r96280 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r96279‎ | r96280 | r96281 >
Date:15:25, 5 September 2011
Author:dantman
Status:ok (Comments)
Tags:
Comment:
Add a new RequestContext::newExtraneousContext() to create a context filled with data not derived from the current session. Also modify setLang to accept a language code.
Modified paths:
  • /trunk/phase3/includes/RequestContext.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/RequestContext.php
——@@ -192,12 +192,39 @@
193193 }
194194
195195 /**
 196+ * Accepts a language code and ensures it's sane. Outputs a cleaned up language
 197+ * code and replaces with $wgLanguageCode if not sane.
 198+ */
 199+ private static function sanitizeLangCode( $code ) {
 200+ global $wgLanguageCode;
 201+
 202+ // BCP 47 - letter case MUST NOT carry meaning
 203+ $code = strtolower( $code );
 204+
 205+ # Validate $code
 206+ if( empty( $code ) || !Language::isValidCode( $code ) || ( $code === 'qqq' ) ) {
 207+ wfDebug( "Invalid user language code\n" );
 208+ $code = $wgLanguageCode;
 209+ }
 210+
 211+ return $code;
 212+ }
 213+
 214+ /**
196215 * Set the Language object
197216 *
198 - * @param $l Language
 217+ * @param $l Mixed Language instance or language code
199218 */
200 - public function setLang( Language $l ) {
201 - $this->lang = $l;
 219+ public function setLang( $l ) {
 220+ if ( $l instanceof Language ) {
 221+ $this->lang = $l;
 222+ } elseif ( is_string( $l ) ) {
 223+ $l = self::sanitizeLangCode( $l );
 224+ $obj = Language::factory( $l );
 225+ $this->lang = $obj;
 226+ } else {
 227+ throw new MWException( __METHOD__ . " was passed an invalid type of data." );
 228+ }
202229 }
203230
204231 /**
——@@ -212,15 +239,8 @@
213240 'uselang',
214241 $this->getUser()->getOption( 'language' )
215242 );
216 - // BCP 47 - letter case MUST NOT carry meaning
217 - $code = strtolower( $code );
 243+ $code = self::sanitizeLangCode( $code );
218244
219 - # Validate $code
220 - if( empty( $code ) || !Language::isValidCode( $code ) || ( $code === 'qqq' ) ) {
221 - wfDebug( "Invalid user language code\n" );
222 - $code = $wgLanguageCode;
223 - }
224 -
225245 wfRunHooks( 'UserGetLanguageObject', array( $this->getUser(), &$code ) );
226246
227247 if( $code === $wgLanguageCode ) {
——@@ -297,6 +317,33 @@
298318 }
299319 return $instance;
300320 }
 321+
 322+ /**
 323+ * Create a new extraneous context. The context is filled with information
 324+ * external to the current session.
 325+ * - Title is specified by argument
 326+ * - Request is a FauxRequest, or a FauxRequest can be specified by argument
 327+ * - User is an anonymous user, for separation IPv4 localhost is used
 328+ * - Language will be based on the anonymous user and request, may be content
 329+ * language or a uselang param in the fauxrequest data may change the lang
 330+ * - Skin will be based on the anonymous user, should be the wiki's default skin
 331+ *
 332+ * @param $title Title Title to use for the extraneous request
 333+ * @param $request Mixed A WebRequest or data to use for a FauxRequest
 334+ * @return RequestContext
 335+ */
 336+ public static function newExtraneousContext( Title $title, $request=array() ) {
 337+ $context = new self;
 338+ $context->setTitle( $title );
 339+ if ( $request instanceof WebRequest ) {
 340+ $context->setRequest( $request );
 341+ } else {
 342+ $context->setRequest( new FauxRequest( $request ) );
 343+ }
 344+ $context->user = User::newFromName( '127.0.0.1', false );
 345+ return $context;
 346+ }
 347+
301348 }
302349
303350 /**

Comments

#Comment by Aaron Schulz (talk | contribs)   22:19, 3 November 2011

+ if ( $request instanceof WebRequest ) {

Should that be FauxRequest? Otherwise getIP() and getCookie() functions will be based on the current session.

#Comment by Dantman (talk | contribs)   22:24, 3 November 2011

FauxRequest is a subclass of WebRequest. We don't care whether a WebRequest or a FauxRequest is being passed in, but if either is passed in we shouldn't be creating a new FauxRequest.

#Comment by Aaron Schulz (talk | contribs)   22:27, 3 November 2011

So then the function comment should be changed. Passing in WebRequest *will* give info not external to the session.

#Comment by Dantman (talk | contribs)   22:47, 3 November 2011

Fine want to change 'or a FauxRequest can be specified by argument' to 'or a FauxRequest or WebRequest can be specified by argument'?

The precise specifics of this method aren't rigid, if the caller has a reason to use a WebRequest then there's no reason to make it not work.

#Comment by Aaron Schulz (talk | contribs)   22:49, 3 November 2011

IMO, I'd demand a FauxRequest or a data array...unless we already have things using a WebRequest and that have a very good reason for doing so.

#Comment by Tim Starling (talk | contribs)   05:28, 20 March 2012

"Extraneous" usually means "unnecessary" or "irrelevant". The dictionary tells me that it can be used to mean "external", but we have less ambiguous words for that, like "external" for instance. I suggest changing the function name to avoid confusion.

#Comment by Dantman (talk | contribs)   05:36, 20 March 2012

"unnecessary" may not fit. But "irrelevant" as in "not relevant to the main request context" does fit. "external" has a bit of ambiguity to it as well. eg: RequestContext::newExternalContext() sounds kind of like it creates context for a request to something external to the entire current wiki. It's almost as bad as newForeignContext would be.

newOutlyingContext? newIsolatedContext?

This kind of sounds like one of those scenarios where nothing ever seams right.

#Comment by Tim Starling (talk | contribs)   05:41, 20 March 2012 
 86 Moby Thesaurus words for "extraneous":
    accidental, added, additional, adrift, adventitious, alien, apart,
    barbarian, barbaric, barbarous, beside the mark, beside the point,
    beside the question, detached, disconnected, discrete, disjunct,
    disrelated, dissociated, exotic, exterior, external, extra,
    extraorganismal, extraterrestrial, extrinsic, foreign,
    foreign-born, immaterial, impersonal, impertinent, inadmissible,
    inapplicable, inapposite, inappropriate, inapt, incidental,
    incommensurable, incomparable, inconsequent, independent,
    inessential, insular, intrusive, irrelative, irrelevant, isolated,
    needless, nihil ad rem, nonessential, nonsubjective, not at issue,
    objective, off the subject, other, out of place, out-of-the-way,
    outland, outlandish, outlying, outside, outward, parenthetical,
    peripheral, pointless, remote, removed, segregate, separate,
    separated, strange, superfluous, supernumerary, ulterior,
    unaffiliated, unallied, unapt, unassociated, unconnected,
    unearthly, unessential, unfitting, unnecessary, unneeded,
    unrelatable, unrelated

 59 Moby Thesaurus words for "external":
    alien, apparent, border, circumference, cortex, cortical, covering,
    crust, envelope, epidermic, epidermis, exomorphic, exotic,
    exterior, extraneous, extraorganismal, extrinsic, facade, face,
    facet, foreign, fringe, front, impersonal, integument, lineaments,
    nonsubjective, objective, open, out, outer, outer face,
    outer layer, outer side, outer skin, outermost, outline, outlying,
    outmost, outside, outstanding, outward, outward-facing, over,
    perceptible, peripheral, periphery, public, rind, roundabout,
    seeming, shell, skin, superficial, superficies, superstratum,
    surface, top, visible
#Comment by Tim Starling (talk | contribs)   05:43, 20 March 2012

Also there's 433 synonyms for "separate", I won't quote them here.

Status & tagging log