r88247 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r88246‎ | r88247 | r88248 >
Date:16:46, 16 May 2011
Author:ialex
Status:ok (Comments)
Tags:
Comment:
Removed configuration storage in the MediaWiki class:
* It serves no purpose, since to be useful we would need to propagate it to all to all objects called by the MediaWiki class
* It is in the wrong place; the MediaWiki class is an helper class for the index.php script, not the base class for the software (and the class could maybe even be renamed)
Modified paths:
  • /trunk/phase3/includes/Wiki.php (modified) (history)
  • /trunk/phase3/index.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/MediaWikiTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/MediaWikiTest.php
——@@ -19,49 +19,6 @@
2020 }
2121
2222 /**
23 - * Test case insentiveness for get / set
24 - */
25 - public function testSetGetValKeyInsentiveness() {
26 -
27 - // set with lower case key
28 - $value = 'SomeValue';
29 - $this->object->setVal( 'foobar', $value );
30 -
31 - $this->assertEquals(
32 - $this->object->getVal( 'foobar' ), 'SomeValue',
33 - 'lower case key set, getting lower case key'
34 - );
35 - $this->assertEquals(
36 - $this->object->getVal( 'FOOBAR' ), 'SomeValue',
37 - 'lower case key set, getting upper case key'
38 - );
39 -
40 - // set with Mixed case key
41 - $value = 'SomeValue2';
42 - $this->object->setVal( 'FooBar', $value );
43 -
44 - $this->assertEquals(
45 - $this->object->getVal( 'foobar' ), 'SomeValue2',
46 - 'mixed case key set, getting lower case key'
47 - );
48 - $this->assertEquals(
49 - $this->object->getVal( 'FOOBAR' ), 'SomeValue2',
50 - 'mixed case key set, getting upper case key'
51 - );
52 - }
53 -
54 - public function testGetValWithDefault() {
55 - $this->assertEmpty(
56 - $this->object->getVal( 'NonExistent' ),
57 - 'Non existent key return empty string'
58 - );
59 - $this->assertEquals(
60 - $this->object->getVal( 'NonExistent2', 'Default Value' ), 'Default Value',
61 - 'Non existent key with default given, should give default'
62 - );
63 - }
64 -
65 - /**
6623 * @todo Implement testPerformRequestForTitle().
6724 */
6825 public function testPerformRequestForTitle() {
Index: trunk/phase3/includes/Wiki.php
——@@ -7,47 +7,11 @@
88 class MediaWiki {
99
1010 /**
11 - * Array of options which may or may not be used
12 - * FIXME: this seems currently to be a messy halfway-house between globals
13 - * and a config object. Pick one and run with it
14 - * @var array
15 - */
16 - private $params = array();
17 -
18 - /**
1911 * TODO: fold $output, etc, into this
2012 * @var RequestContext
2113 */
2214 private $context;
2315
24 - /**
25 - * Stores key/value pairs to circumvent global variables
26 - * Note that keys are case-insensitive!
27 - *
28 - * @param $key String: key to store
29 - * @param $value Mixed: value to put for the key
30 - */
31 - public function setVal( $key, &$value ) {
32 - $key = strtolower( $key );
33 - $this->params[$key] =& $value;
34 - }
35 -
36 - /**
37 - * Retrieves key/value pairs to circumvent global variables
38 - * Note that keys are case-insensitive!
39 - *
40 - * @param $key String: key to get
41 - * @param $default string default value, defaults to empty string
42 - * @return $default Mixed: default value if if the key doesn't exist
43 - */
44 - public function getVal( $key, $default = '' ) {
45 - $key = strtolower( $key );
46 - if ( isset( $this->params[$key] ) ) {
47 - return $this->params[$key];
48 - }
49 - return $default;
50 - }
51 -
5216 public function request( WebRequest $x = null ){
5317 return wfSetVar( $this->context->request, $x );
5418 }
——@@ -180,6 +144,8 @@
181145 * @return bool true if the request is already executed
182146 */
183147 private function handleSpecialCases() {
 148+ global $wgServer, $wgUsePathInfo;
 149+
184150 wfProfileIn( __METHOD__ );
185151
186152 // Invalid titles. Bug 21776: The interwikis must redirect even if the page name is empty.
——@@ -198,7 +164,7 @@
199165 $url = $this->context->title->getFullURL( $query );
200166 }
201167 // Check for a redirect loop
202 - if ( !preg_match( '/^' . preg_quote( $this->getVal( 'Server' ), '/' ) . '/', $url ) && $this->context->title->isLocal() ) {
 168+ if ( !preg_match( '/^' . preg_quote( $wgServer, '/' ) . '/', $url ) && $this->context->title->isLocal() ) {
203169 // 301 so google et al report the target as the actual url.
204170 $this->context->output->redirect( $url, 301 );
205171 } else {
——@@ -225,7 +191,7 @@
226192 "requested; this sometimes happens when moving a wiki " .
227193 "to a new server or changing the server configuration.\n\n";
228194
229 - if ( $this->getVal( 'UsePathInfo' ) ) {
 195+ if ( $wgUsePathInfo ) {
230196 $message .= "The wiki is trying to interpret the page " .
231197 "title from the URL path portion (PATH_INFO), which " .
232198 "sometimes fails depending on the web server. Try " .
——@@ -332,6 +298,8 @@
333299 * @return mixed an Article, or a string to redirect to another URL
334300 */
335301 private function initializeArticle() {
 302+ global $wgDisableHardRedirects;
 303+
336304 wfProfileIn( __METHOD__ );
337305
338306 $action = $this->context->request->getVal( 'action', 'view' );
——@@ -364,7 +332,7 @@
365333 // Is the target already set by an extension?
366334 $target = $target ? $target : $article->followRedirect();
367335 if ( is_string( $target ) ) {
368 - if ( !$this->getVal( 'DisableHardRedirects' ) ) {
 336+ if ( !$wgDisableHardRedirects ) {
369337 // we'll need to redirect
370338 wfProfileOut( __METHOD__ );
371339 return $target;
——@@ -460,6 +428,9 @@
461429 * @param $article Article
462430 */
463431 private function performAction( $article ) {
 432+ global $wgSquidMaxage, $wgUseExternalEditor,
 433+ $wgEnableDublinCoreRdf, $wgEnableCreativeCommonsRdf;
 434+
464435 wfProfileIn( __METHOD__ );
465436
466437 if ( !wfRunHooks( 'MediaWikiPerformAction', array(
——@@ -481,7 +452,7 @@
482453
483454 switch( $act ) {
484455 case 'view':
485 - $this->context->output->setSquidMaxage( $this->getVal( 'SquidMaxage' ) );
 456+ $this->context->output->setSquidMaxage( $wgSquidMaxage );
486457 $article->view();
487458 break;
488459 case 'raw': // includes JS/CSS
——@@ -502,7 +473,7 @@
503474 $article->$act();
504475 break;
505476 case 'dublincore':
506 - if ( !$this->getVal( 'EnableDublinCoreRdf' ) ) {
 477+ if ( !$wgEnableDublinCoreRdf ) {
507478 wfHttpError( 403, 'Forbidden', wfMsg( 'nodublincore' ) );
508479 } else {
509480 $rdf = new DublinCoreRdf( $article );
——@@ -510,7 +481,7 @@
511482 }
512483 break;
513484 case 'creativecommons':
514 - if ( !$this->getVal( 'EnableCreativeCommonsRdf' ) ) {
 485+ if ( !$wgEnableCreativeCommonsRdf ) {
515486 wfHttpError( 403, 'Forbidden', wfMsg( 'nocreativecommons' ) );
516487 } else {
517488 $rdf = new CreativeCommonsRdf( $article );
——@@ -529,11 +500,11 @@
530501 $external = $this->context->request->getVal( 'externaledit' );
531502 $section = $this->context->request->getVal( 'section' );
532503 $oldid = $this->context->request->getVal( 'oldid' );
533 - if ( !$this->getVal( 'UseExternalEditor' ) || $act == 'submit' || $internal ||
 504+ if ( !$wgUseExternalEditor || $act == 'submit' || $internal ||
534505 $section || $oldid || ( !$this->context->user->getOption( 'externaleditor' ) && !$external ) ) {
535506 $editor = new EditPage( $article );
536507 $editor->submit();
537 - } elseif ( $this->getVal( 'UseExternalEditor' ) && ( $external || $this->context->user->getOption( 'externaleditor' ) ) ) {
 508+ } elseif ( $wgUseExternalEditor && ( $external || $this->context->user->getOption( 'externaleditor' ) ) ) {
538509 $mode = $this->context->request->getVal( 'mode' );
539510 $extedit = new ExternalEdit( $article, $mode );
540511 $extedit->edit();
——@@ -542,7 +513,7 @@
543514 break;
544515 case 'history':
545516 if ( $this->context->request->getFullRequestURL() == $this->context->title->getInternalURL( 'action=history' ) ) {
546 - $this->context->output->setSquidMaxage( $this->getVal( 'SquidMaxage' ) );
 517+ $this->context->output->setSquidMaxage( $wgSquidMaxage );
547518 }
548519 $history = new HistoryPage( $article );
549520 $history->history();
Index: trunk/phase3/index.php
——@@ -133,15 +133,6 @@
134134 wfProfileOut( 'index.php-filecache' );
135135 }
136136
137 -# Setting global variables in mediaWiki
138 -$mediaWiki->setVal( 'DisableHardRedirects', $wgDisableHardRedirects );
139 -$mediaWiki->setVal( 'EnableCreativeCommonsRdf', $wgEnableCreativeCommonsRdf );
140 -$mediaWiki->setVal( 'EnableDublinCoreRdf', $wgEnableDublinCoreRdf );
141 -$mediaWiki->setVal( 'Server', $wgServer );
142 -$mediaWiki->setVal( 'SquidMaxage', $wgSquidMaxage );
143 -$mediaWiki->setVal( 'UseExternalEditor', $wgUseExternalEditor );
144 -$mediaWiki->setVal( 'UsePathInfo', $wgUsePathInfo );
145 -
146137 $mediaWiki->performRequestForTitle( $wgArticle );
147138 $mediaWiki->finalCleanup();
148139
——@@ -206,4 +197,4 @@
207198 </html>
208199 <?php
209200 die( 1 );
210 -}
\ No newline at end of file
 201+}

Comments

#Comment by 😂 (talk | contribs)   16:51, 16 May 2011

\o/ Hopefully we can get real config management soon.

Status & tagging log