r90261 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r90260‎ | r90261 | r90262 >
Date:10:29, 17 June 2011
Author:reedy
Status:resolved (Comments)
Tags:
Comment:
Fixing up most of the magic get/set stuff
Modified paths:
  • /trunk/phase3/includes/Wiki.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Wiki.php
——@@ -13,11 +13,13 @@
1414 private $context;
1515
1616 public function request( WebRequest $x = null ){
17 - return wfSetVar( $this->context->request, $x );
 17+ $this->context->setRequest( $x );
 18+ return $this->context->getRequest();
1819 }
1920
2021 public function output( OutputPage $x = null ){
21 - return wfSetVar( $this->context->output, $x );
 22+ $this->context->setOutput( $x );
 23+ return $this->context->getOutput();
2224 }
2325
2426 public function __construct( RequestContext $context ){
——@@ -78,10 +80,10 @@
7981 * @return Title
8082 */
8183 public function getTitle(){
82 - if( $this->context->title === null ){
83 - $this->context->title = $this->parseTitle();
 84+ if( $this->context->getTitle() === null ){
 85+ $this->context->setTitle( $this->parseTitle() );
8486 }
85 - return $this->context->title;
 87+ return $this->context->getTitle();
8688 }
8789
8890 /**
——@@ -100,60 +102,65 @@
101103
102104 wfProfileIn( __METHOD__ );
103105
104 - if ( $this->context->request->getVal( 'printable' ) === 'yes' ) {
105 - $this->context->output->setPrintable();
 106+ $request = $this->context->getRequest();
 107+ $title = $this->context->getTitle();
 108+ $output = $this->context->getOutput();
 109+ $user = $this->context->getUser();
 110+
 111+ if ( $request->getVal( 'printable' ) === 'yes' ) {
 112+ $output->setPrintable();
106113 }
107114
108115 wfRunHooks( 'BeforeInitialize', array(
109 - &$this->context->title,
 116+ &$title,
110117 null,
111 - &$this->context->output,
112 - &$this->context->user,
113 - $this->context->request,
 118+ &$output,
 119+ &$user,
 120+ $request,
114121 $this
115122 ) );
116123
117124 // Invalid titles. Bug 21776: The interwikis must redirect even if the page name is empty.
118 - if ( $this->context->title instanceof BadTitle ) {
 125+ if ( $title instanceof BadTitle ) {
119126 throw new ErrorPageError( 'badtitle', 'badtitletext' );
120127 // If the user is not logged in, the Namespace:title of the article must be in
121128 // the Read array in order for the user to see it. (We have to check here to
122129 // catch special pages etc. We check again in Article::view())
123 - } else if ( !$this->context->title->userCanRead() ) {
124 - $this->context->output->loginToUse();
 130+ } else if ( !$title->userCanRead() ) {
 131+ $output->loginToUse();
125132 // Interwiki redirects
126 - } else if ( $this->context->title->getInterwiki() != '' ) {
127 - $rdfrom = $this->context->request->getVal( 'rdfrom' );
 133+ } else if ( $title->getInterwiki() != '' ) {
 134+ $rdfrom = $request->getVal( 'rdfrom' );
128135 if ( $rdfrom ) {
129 - $url = $this->context->title->getFullURL( 'rdfrom=' . urlencode( $rdfrom ) );
 136+ $url = $title->getFullURL( 'rdfrom=' . urlencode( $rdfrom ) );
130137 } else {
131 - $query = $this->context->request->getValues();
 138+ $query = $request->getValues();
132139 unset( $query['title'] );
133 - $url = $this->context->title->getFullURL( $query );
 140+ $url = $title->getFullURL( $query );
134141 }
135142 // Check for a redirect loop
136 - if ( !preg_match( '/^' . preg_quote( $wgServer, '/' ) . '/', $url ) && $this->context->title->isLocal() ) {
 143+ if ( !preg_match( '/^' . preg_quote( $wgServer, '/' ) . '/', $url ) && $title->isLocal() ) {
137144 // 301 so google et al report the target as the actual url.
138 - $this->context->output->redirect( $url, 301 );
 145+ $output->redirect( $url, 301 );
139146 } else {
140 - $this->context->title = new BadTitle;
 147+ $this->context->setTitle( new BadTitle );
141148 wfProfileOut( __METHOD__ );
142149 throw new ErrorPageError( 'badtitle', 'badtitletext' );
143150 }
144151 // Redirect loops, no title in URL, $wgUsePathInfo URLs, and URLs with a variant
145 - } else if ( $this->context->request->getVal( 'action', 'view' ) == 'view' && !$this->context->request->wasPosted()
146 - && ( $this->context->request->getVal( 'title' ) === null || $this->context->title->getPrefixedDBKey() != $this->context->request->getVal( 'title' ) )
147 - && !count( array_diff( array_keys( $this->context->request->getValues() ), array( 'action', 'title' ) ) ) )
 152+ } else if ( $request->getVal( 'action', 'view' ) == 'view' && !$request->wasPosted()
 153+ && ( $request->getVal( 'title' ) === null || $title->getPrefixedDBKey() != $request->getVal( 'title' ) )
 154+ && !count( array_diff( array_keys( $request->getValues() ), array( 'action', 'title' ) ) ) )
148155 {
149 - if ( $this->context->title->getNamespace() == NS_SPECIAL ) {
150 - list( $name, $subpage ) = SpecialPageFactory::resolveAlias( $this->context->title->getDBkey() );
 156+ if ( $title->getNamespace() == NS_SPECIAL ) {
 157+ list( $name, $subpage ) = SpecialPageFactory::resolveAlias( $title->getDBkey() );
151158 if ( $name ) {
152 - $this->context->title = SpecialPage::getTitleFor( $name, $subpage );
 159+ $title = SpecialPage::getTitleFor( $name, $subpage );
153160 }
154161 }
155 - $targetUrl = $this->context->title->getFullURL();
 162+ $targetUrl = $title->getFullURL();
156163 // Redirect to canonical url, make it a 301 to allow caching
157 - if ( $targetUrl == $this->context->request->getFullRequestURL() ) {
 164+ if ( $targetUrl == $request->getFullRequestURL() ) {
158165 $message = "Redirect loop detected!\n\n" .
159166 "This means the wiki got confused about what page was " .
160167 "requested; this sometimes happens when moving a wiki " .
——@@ -175,13 +182,13 @@
176183 }
177184 wfHttpError( 500, "Internal error", $message );
178185 } else {
179 - $this->context->output->setSquidMaxage( 1200 );
180 - $this->context->output->redirect( $targetUrl, '301' );
 186+ $output->setSquidMaxage( 1200 );
 187+ $output->redirect( $targetUrl, '301' );
181188 }
182189 // Special pages
183 - } else if ( NS_SPECIAL == $this->context->title->getNamespace() ) {
 190+ } else if ( NS_SPECIAL == $title->getNamespace() ) {
184191 // actions that need to be made when we have a special pages
185 - SpecialPageFactory::executePath( $this->context->title, $this->context );
 192+ SpecialPageFactory::executePath( $title, $this->context );
186193 } else {
187194 // ...otherwise treat it as an article view. The article
188195 // may be a redirect to another article or URL.
——@@ -191,7 +198,7 @@
192199 wfProfileOut( __METHOD__ );
193200 return $article;
194201 } elseif ( is_string( $article ) ) {
195 - $this->context->output->redirect( $article );
 202+ $output->redirect( $article );
196203 } else {
197204 wfProfileOut( __METHOD__ );
198205 throw new MWException( "Shouldn't happen: MediaWiki::initializeArticle() returned neither an object nor a URL" );
——@@ -222,7 +229,8 @@
223230 public function getAction() {
224231 global $wgDisabledActions;
225232
226 - $action = $this->context->request->getVal( 'action', 'view' );
 233+ $request = $this->context->getRequest();
 234+ $action = $request->getVal( 'action', 'view' );
227235
228236 // Check for disabled actions
229237 if ( in_array( $action, $wgDisabledActions ) ) {
——@@ -232,7 +240,7 @@
233241 // Workaround for bug #20966: inability of IE to provide an action dependent
234242 // on which submit button is clicked.
235243 if ( $action === 'historysubmit' ) {
236 - if ( $this->context->request->getBool( 'revisiondelete' ) ) {
 244+ if ( $request->getBool( 'revisiondelete' ) ) {
237245 return 'revisiondelete';
238246 } else {
239247 return 'view';
——@@ -255,21 +263,24 @@
256264
257265 wfProfileIn( __METHOD__ );
258266
259 - $action = $this->context->request->getVal( 'action', 'view' );
260 - $article = Article::newFromTitle( $this->context->title, $this->context );
 267+ $request = $this->context->getRequest();
 268+ $title = $this->context->getTitle();
 269+
 270+ $action = $request->getVal( 'action', 'view' );
 271+ $article = Article::newFromTitle( $title, $this->context );
261272 // NS_MEDIAWIKI has no redirects.
262273 // It is also used for CSS/JS, so performance matters here...
263 - if ( $this->context->title->getNamespace() == NS_MEDIAWIKI ) {
 274+ if ( $title->getNamespace() == NS_MEDIAWIKI ) {
264275 wfProfileOut( __METHOD__ );
265276 return $article;
266277 }
267278 // Namespace might change when using redirects
268279 // Check for redirects ...
269 - $file = ( $this->context->title->getNamespace() == NS_FILE ) ? $article->getFile() : null;
 280+ $file = ( $title->getNamespace() == NS_FILE ) ? $article->getFile() : null;
270281 if ( ( $action == 'view' || $action == 'render' ) // ... for actions that show content
271 - && !$this->context->request->getVal( 'oldid' ) && // ... and are not old revisions
272 - !$this->context->request->getVal( 'diff' ) && // ... and not when showing diff
273 - $this->context->request->getVal( 'redirect' ) != 'no' && // ... unless explicitly told not to
 282+ && !$request->getVal( 'oldid' ) && // ... and are not old revisions
 283+ !$request->getVal( 'diff' ) && // ... and not when showing diff
 284+ $request->getVal( 'redirect' ) != 'no' && // ... unless explicitly told not to
274285 // ... and the article is not a non-redirect image page with associated file
275286 !( is_object( $file ) && $file->exists() && !$file->getRedirected() ) )
276287 {
——@@ -277,7 +288,7 @@
278289 $ignoreRedirect = $target = false;
279290
280291 wfRunHooks( 'InitializeArticleMaybeRedirect',
281 - array( &$this->context->title, &$this->context->request, &$ignoreRedirect, &$target, &$article ) );
 292+ array( &$title, &$request, &$ignoreRedirect, &$target, &$article ) );
282293
283294 // Follow redirects only for... redirects.
284295 // If $target is set, then a hook wanted to redirect.
——@@ -296,13 +307,13 @@
297308 $rarticle = Article::newFromTitle( $target, $this->context );
298309 $rarticle->loadPageData();
299310 if ( $rarticle->exists() || ( is_object( $file ) && !$file->isLocal() ) ) {
300 - $rarticle->setRedirectedFrom( $this->context->title );
 311+ $rarticle->setRedirectedFrom( $title );
301312 $article = $rarticle;
302 - $this->context->title = $target;
 313+ $this->context->setTitle( $target );
303314 }
304315 }
305316 } else {
306 - $this->context->title = $article->getTitle();
 317+ $this->context->setTitle( $article->getTitle() );
307318 }
308319 }
309320 wfProfileOut( __METHOD__ );
——@@ -319,7 +330,7 @@
320331 $factory = wfGetLBFactory();
321332 $factory->commitMasterChanges();
322333 // Output everything!
323 - $this->context->output->output();
 334+ $this->context->getOutput()->output();
324335 // Do any deferred jobs
325336 wfDoUpdates( 'commit' );
326337
——@@ -397,9 +408,14 @@
398409
399410 wfProfileIn( __METHOD__ );
400411
 412+ $request = $this->context->getRequest();
 413+ $output = $this->context->getOutput();
 414+ $title = $this->context->getTitle();
 415+ $user = $this->context->getUser();
 416+
401417 if ( !wfRunHooks( 'MediaWikiPerformAction', array(
402 - $this->context->output, $article, $this->context->title,
403 - $this->context->user, $this->context->request, $this ) ) )
 418+ $output, $article, $title,
 419+ $user, $request, $this ) ) )
404420 {
405421 wfProfileOut( __METHOD__ );
406422 return;
——@@ -416,7 +432,7 @@
417433
418434 switch( $act ) {
419435 case 'view':
420 - $this->context->output->setSquidMaxage( $wgSquidMaxage );
 436+ $output->setSquidMaxage( $wgSquidMaxage );
421437 $article->view();
422438 break;
423439 case 'raw': // includes JS/CSS
——@@ -441,32 +457,32 @@
442458 }
443459 // Continue...
444460 case 'edit':
445 - if ( wfRunHooks( 'CustomEditor', array( $article, $this->context->user ) ) ) {
446 - $internal = $this->context->request->getVal( 'internaledit' );
447 - $external = $this->context->request->getVal( 'externaledit' );
448 - $section = $this->context->request->getVal( 'section' );
449 - $oldid = $this->context->request->getVal( 'oldid' );
 461+ if ( wfRunHooks( 'CustomEditor', array( $article, $user ) ) ) {
 462+ $internal = $request->getVal( 'internaledit' );
 463+ $external = $request->getVal( 'externaledit' );
 464+ $section = $request->getVal( 'section' );
 465+ $oldid = $request->getVal( 'oldid' );
450466 if ( !$wgUseExternalEditor || $act == 'submit' || $internal ||
451 - $section || $oldid || ( !$this->context->user->getOption( 'externaleditor' ) && !$external ) ) {
 467+ $section || $oldid || ( !$user->getOption( 'externaleditor' ) && !$external ) ) {
452468 $editor = new EditPage( $article );
453469 $editor->submit();
454 - } elseif ( $wgUseExternalEditor && ( $external || $this->context->user->getOption( 'externaleditor' ) ) ) {
455 - $mode = $this->context->request->getVal( 'mode' );
 470+ } elseif ( $wgUseExternalEditor && ( $external || $user->getOption( 'externaleditor' ) ) ) {
 471+ $mode = $request->getVal( 'mode' );
456472 $extedit = new ExternalEdit( $article, $mode );
457473 $extedit->edit();
458474 }
459475 }
460476 break;
461477 case 'history':
462 - if ( $this->context->request->getFullRequestURL() == $this->context->title->getInternalURL( 'action=history' ) ) {
463 - $this->context->output->setSquidMaxage( $wgSquidMaxage );
 478+ if ( $request->getFullRequestURL() == $title->getInternalURL( 'action=history' ) ) {
 479+ $output->setSquidMaxage( $wgSquidMaxage );
464480 }
465481 $history = new HistoryPage( $article );
466482 $history->history();
467483 break;
468484 default:
469485 if ( wfRunHooks( 'UnknownAction', array( $act, $article ) ) ) {
470 - $this->context->output->showErrorPage( 'nosuchaction', 'nosuchactiontext' );
 486+ $output->showErrorPage( 'nosuchaction', 'nosuchactiontext' );
471487 }
472488 }
473489 wfProfileOut( __METHOD__ );

Follow-up revisions

RevisionCommit summaryAuthorDate
r90559Followup r90261...reedy23:28, 21 June 2011

Comments

#Comment by Bawolff (talk | contribs)   19:10, 17 June 2011

yay, no more deprecated warnings :D


With the BeforeInitialize hook though, doesn't the whole passing by reference thing not work as expected if its passing the result of $this->context->getTitle(), since it references the intermediate variable, not the one in the RequestContext, so the hook wouldn't be able to replace the argument with a different object? (not sure if that matters).

#Comment by Aaron Schulz (talk | contribs)   21:54, 21 June 2011

wfSetVar return the old value, now you just return the new value.

Status & tagging log