r86775 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r86774‎ | r86775 | r86776 >
Date:15:19, 23 April 2011
Author:happy-melon
Status:resolved (Comments)
Tags:
Comment:
Stop stubbing $wgLang and $wgContLang. There are no major code paths which do not call either $wgLang or $wgContLang at least once. All index.php calls unstub $wgContLang from MediaWiki::parseTitle() except in the edgecase of viewing pages referenced only by "curid=123", and since those will end up calling OutputPage::output() they will eventually be unstubbed at some point as well. All calls through load.php unstub $wgLang in ResourceLoaderContext::getLanguage() from ResouceLoader::respond() --> ResourceLoader::preloadModuleInfo(). All calls through api.php unstub $wgContLang in ApiResult::cleanUpUTF8() from ApiMain::printResult().
Modified paths:
  • /trunk/phase3/includes/Message.php (modified) (history)
  • /trunk/phase3/includes/Setup.php (modified) (history)
  • /trunk/phase3/includes/StubObject.php (modified) (history)
  • /trunk/phase3/languages/Language.php (modified) (history)
  • /trunk/phase3/tests/parser/parserTest.inc (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/ArticleTablesTest.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/ParserOptionsTest.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/parser/NewParserTest.php (modified) (history)
  • /trunk/phase3/tests/phpunit/suites/UploadFromUrlTestSuite.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/ArticleTablesTest.php
@@ -15,8 +15,8 @@
1616 function tearDown() {
1717 global $wgLanguageCode, $wgContLang, $wgLang;
1818 $wgLanguageCode = $this->languageCode;
19 - $wgContLang = new StubContLang;
20 - $wgLang = new StubUserLang;
 19+ $wgContLang = Language::factory( $wgLanguageCode );
 20+ $wgLang = RequestContext::getMain()->getLang();
2121 }
2222
2323 /**
Index: trunk/phase3/tests/phpunit/includes/parser/NewParserTest.php
@@ -76,7 +76,7 @@
7777
7878 // $tmpGlobals['wgContLang'] = new StubContLang;
7979 $tmpGlobals['wgUser'] = new User;
80 - $tmpGlobals['wgLang'] = new StubUserLang;
 80+ $tmpGlobals['wgLang'] = Language::factory( 'en' );
8181 $tmpGlobals['wgOut'] = new StubObject( 'wgOut', 'OutputPage' );
8282 $tmpGlobals['wgParser'] = new StubObject( 'wgParser', $GLOBALS['wgParserConf']['class'], array( $GLOBALS['wgParserConf'] ) );
8383 $tmpGlobals['wgRequest'] = new WebRequest;
Index: trunk/phase3/tests/phpunit/includes/ParserOptionsTest.php
@@ -7,8 +7,8 @@
88
99 function setUp() {
1010 ParserTest::setUp(); //reuse setup from parser tests
11 - global $wgContLang, $wgUser;
12 - $wgContLang = new StubContLang;
 11+ global $wgContLang, $wgUser, $wgLanguageCode;
 12+ $wgContLang = Language::factory( $wgLanguageCode );
1313 $this->popts = new ParserOptions( $wgUser );
1414 $this->pcache = ParserCache::singleton();
1515 }
Index: trunk/phase3/tests/phpunit/suites/UploadFromUrlTestSuite.php
@@ -48,7 +48,7 @@
4949
5050 // $wgContLang = new StubContLang;
5151 $wgUser = new User;
52 - $wgLang = new StubUserLang;
 52+ $wgLang = Language::factory( 'en' );
5353 $wgOut = new StubObject( 'wgOut', 'OutputPage' );
5454 $wgParser = new StubObject( 'wgParser', $wgParserConf['class'], array( $wgParserConf ) );
5555 $wgRequest = new WebRequest;
Index: trunk/phase3/tests/parser/parserTest.inc
@@ -166,7 +166,7 @@
167167
168168 // $wgContLang = new StubContLang;
169169 $wgUser = new User;
170 - $wgLang = new StubUserLang;
 170+ $wgLang = Language::factory( 'en' );
171171 $wgOut = new StubObject( 'wgOut', 'OutputPage' );
172172 $wgParser = new StubObject( 'wgParser', $wgParserConf['class'], array( $wgParserConf ) );
173173 $wgRequest = new WebRequest;
Index: trunk/phase3/includes/StubObject.php
@@ -110,6 +110,8 @@
111111 /**
112112 * Stub object for the content language of this wiki. This object have to be in
113113 * $wgContLang global.
 114+ *
 115+ * @deprecated since 1.18
114116 */
115117 class StubContLang extends StubObject {
116118
@@ -134,6 +136,8 @@
135137 * Stub object for the user language. It depends of the user preferences and
136138 * "uselang" parameter that can be passed to index.php. This object have to be
137139 * in $wgLang global.
 140+ *
 141+ * @deprecated since 1.18
138142 */
139143 class StubUserLang extends StubObject {
140144
Index: trunk/phase3/includes/Setup.php
@@ -397,7 +397,9 @@
398398 wfProfileOut( $fname . '-session' );
399399 wfProfileIn( $fname . '-globals' );
400400
401 -$wgContLang = new StubContLang;
 401+$wgContLang = Language::factory( $wgLanguageCode );
 402+$wgContLang->initEncoding();
 403+$wgContLang->initContLang();
402404
403405 // Now that variant lists may be available...
404406 $wgRequest->interpolateTitle();
@@ -406,7 +408,7 @@
407409 /**
408410 * @var Language
409411 */
410 -$wgLang = new StubUserLang;
 412+$wgLang = RequestContext::getMain()->getLang();
411413
412414 /**
413415 * @var OutputPage
Index: trunk/phase3/includes/Message.php
@@ -208,7 +208,7 @@
209209 * @return Message: $this
210210 */
211211 public function inLanguage( $lang ) {
212 - if ( $lang instanceof Language || $lang instanceof StubContLang || $lang instanceof StubUserLang ) {
 212+ if ( $lang instanceof Language ) {
213213 $this->language = $lang;
214214 } elseif ( is_string( $lang ) ) {
215215 if( $this->language->getCode() != $lang ) {
Index: trunk/phase3/languages/Language.php
@@ -73,7 +73,11 @@
7474 */
7575 var $transformData = array();
7676
 77+ /**
 78+ * @var LocalisationCache
 79+ */
7780 static public $dataCache;
 81+
7882 static public $mLangObjCache = array();
7983
8084 static public $mWeekdayMsgs = array(

Follow-up revisions

RevisionCommit summaryAuthorDate
r86801Follow-up r86775: the execution order is slightly changed by that commit; and...happy-melon21:55, 23 April 2011
r86809Follow-up r86775: restub $wgLang. Not because it's a good idea, but because ...happy-melon10:50, 24 April 2011
r87126The $wgLocalisationCacheConf was being overwritten by the default value....platonides16:26, 29 April 2011

Comments

#Comment by Siebrand (talk | contribs)   17:38, 23 April 2011

PHP Warning: in_array() expects parameter 2 to be array, null given in /www/w/extensions/LanguageSelector/LanguageSelector.php on line 162

#Comment by Happy-melon (talk | contribs)   20:14, 23 April 2011
162:		if ( in_array( $reqCode, $wgLanguageSelectorLanguages ) ) {

Parameter 2 there is a global config variable. Are you sure this is related to this commit?

#Comment by Nikerabbit (talk | contribs)   21:09, 23 April 2011

Yes. The relevant code is now called earlier and looks like some initializations are not done properly at that point.

#Comment by Happy-melon (talk | contribs)   21:16, 23 April 2011

Do you have a stack trace?

#Comment by Nikerabbit (talk | contribs)   21:21, 23 April 2011
[23-Apr-2011 21:20:09] PHP Warning:  in_array() expects parameter 2 to be array, null given in /www/sandwiki/extensions/LanguageSelector/LanguageSelector.php on line 164
[23-Apr-2011 21:20:09] <ul>
<li>LanguageSelector.php line 127 calls wfBacktrace()</li>
<li>- line - calls wfLanguageSelectorGetLanguageCode()</li>
<li>Hooks.php line 235 calls call_user_func_array()</li>
<li>Hooks.php line 38 calls Hooks::run()</li>
<li>RequestContext.php line 119 calls wfRunHooks()</li>
<li>Setup.php line 411 calls RequestContext::getLang()</li>
<li>WebStart.php line 169 calls require_once()</li>
<li>load.php line 39 calls require()</li>
</ul>

[23-Apr-2011 21:20:09] PHP Warning:  in_array() expects parameter 2 to be array, null given in /www/sandwiki/extensions/LanguageSelector/LanguageSelector.php on line 164
#Comment by Happy-melon (talk | contribs)   22:04, 23 April 2011

Should be fixed by r86801.

#Comment by MarkAHershberger (talk | contribs)   23:56, 23 April 2011

Problem running install.php, starting with this revision:

$ php maintenance/install.php --dbtype=sqlite --dbpath=/tmp --pass=testpass test test
DB connection error: Access denied for user 'wikiuser'@'localhost' (using password: NO) (localhost)
Backtrace:
#0 /home/ci/test-server/cc-home/projects/mw/source/includes/db/DatabaseMysql.php(131): DatabaseBase->reportConnectionError(' Access denied ...')
#1 /home/ci/test-server/cc-home/projects/mw/source/includes/db/Database.php(518): DatabaseMysql->open('localhost', 'wikiuser', '', 'my_wiki')
#2 /home/ci/test-server/cc-home/projects/mw/source/includes/db/Database.php(566): DatabaseBase->__construct('localhost', 'wikiuser', '',  'my_wiki', 16, 'get from global')
#3 /home/ci/test-server/cc-home/projects/mw/source/includes/db/LoadBalancer.php(658): DatabaseBase::newFromType('mysql', Array)
#4 /home/ci/test-server/cc-home/projects/mw/source/includes/db/LoadBalancer.php(539): LoadBalancer->reallyOpenConnection(Array, false)
#5 /home/ci/test-server/cc-home/projects/mw/source/includes/db/LoadBalancer.php(460): LoadBalancer->openConnection(0, false)
#6 /home/ci/test-server/cc-home/projects/mw/source/includes/GlobalFunctions.php(3117): LoadBalancer->getConnection(-1, Array, false)
#7 /home/ci/test-server/cc-home/projects/mw/source/includes/LocalisationCache.php(768): wfGetDB(-1)
#8 /home/ci/test-server/cc-home/projects/mw/source/includes/LocalisationCache.php(318): LCStore_DB->get('en', 'deps')
#9 /home/ci/test-server/cc-home/projects/mw/source/includes/LocalisationCache.php(353): LocalisationCache->isExpired('en')
#10 /home/ci/test-server/cc-home/projects/mw/source/includes/LocalisationCache.php(259): LocalisationCache->initLanguage('en')
#11 /home/ci/test-server/cc-home/projects/mw/source/includes/LocalisationCache.php(205): LocalisationCache->loadItem('en', 'defaultUserOpti...')
#12 /home/ci/test-server/cc-home/projects/mw/source/languages/Language.php(529): LocalisationCache->getItem('en', 'defaultUserOpti...')
#13 /home/ci/test-server/cc-home/projects/mw/source/includes/User.php(1052): Language->getDefaultUserOptionOverrides()
#14 /home/ci/test-server/cc-home/projects/mw/source/includes/User.php(1944): User::getDefaultOptions()
#15 /home/ci/test-server/cc-home/projects/mw/source/includes/RequestContext.php(108): User->getOption('language')
#16 /home/ci/test-server/cc-home/projects/mw/source/includes/Setup.php(411): RequestContext->getLang()
#17 /home/ci/test-server/cc-home/projects/mw/source/maintenance/doMaintenance.php(108): require_once('/home/ci/test-s...')
#18 /home/ci/test-server/cc-home/projects/mw/source/maintenance/install.php(94): require_once('/home/ci/test-s...')
#19 {main}
#Comment by Happy-melon (talk | contribs)   23:58, 23 April 2011

Ewww...

#Comment by Platonides (talk | contribs)   20:50, 26 April 2011

Breaks databaseless phpunit tests. The Language::factory() at Setup.php(400) tries to load the localization from the db.

With the StubContLang, phpunit was able to disable Database messages before the unstubbing.

Stack trace:
#0 phase3/includes/db/Database.php(518): DatabaseMysql->open('10.0.6.47', 'wikiuser', 'jgmeidj28gms', 'eswiki')
#1 phase3/includes/db/Database.php(566): DatabaseBase->__construct('10.0.6.47', 'wikiuser', 'jgmeidj28gms', 'eswiki', 16, 'get from global')
#2 phase3/includes/db/LoadBalancer.php(658): DatabaseBase::newFromType('mysql', Array)
#3 phase3/includes/db/LoadBalancer.php(539): LoadBalancer->reallyOpenConnection(Array, false)
#4 phase3/includes/db/LoadBalancer.php(460): LoadBalancer->openConnection(0, false)
#5 phase3/includes/GlobalFunctions.php(3128): LoadBalancer->getConnection(-1, Array, false)
#6 phase3/includes/LocalisationCache.php(768): wfGetDB(-1)
#7 phase3/includes/LocalisationCache.php(318): LCStore_DB->get('es', 'deps')
#8 phase3/includes/LocalisationCache.php(353): LocalisationCache->isExpired('es')
#9 phase3/includes/LocalisationCache.php(259): LocalisationCache->initLanguage('es')
#10 phase3/includes/LocalisationCache.php(205): LocalisationCache->loadItem('es', 'fallback')
#11 phase3/languages/Language.php(2968): LocalisationCache->getItem('es', 'fallback')
#12 phase3/languages/Language.php(197): Language::getFallbackFor('es')
#13 phase3/languages/Language.php(148): Language::newFromCode('es')
#14 phase3/includes/Setup.php(400): Language::factory('es')
#15 phase3/maintenance/doMaintenance.php(108): require_once('.../Setup.php')
#16 phase3/tests/phpunit/phpunit.php(38): require('.../doMaintenance.php')
#17 {main}
#Comment by Happy-melon (talk | contribs)   22:29, 26 April 2011

This is nothing to do with $wgUseDatabaseMessages, it's because LocalisationCache doesn't respect that as meaning it shouldn't try and safe its cache data in the database. The PHPUnitMaintClass::finalSetup() call is made before Setup.php is included, so that global is already set, AFAICT anyway.

The unit tests run fine on my setup, at least as far as the parser tests. Which test generates that error for you?

#Comment by Platonides (talk | contribs)   22:37, 26 April 2011

You can see that the db access spawned from the call $wgContLang = Language::factory( $wgLanguageCode ); added here. It will be the BagOStuff replacement what arrives late, then.

To test it, stop mysqld and run make databaseless in phpunit folder

#Comment by Brion VIBBER (talk | contribs)   23:08, 14 June 2011

This seems to have been resolved between here and r90000 or so; current REL1_18 branch and trunk both seem happy running databaseless tests with a broken DB config now.

#Comment by Platonides (talk | contribs)   14:08, 15 June 2011

I fixed it in r87126

Status & tagging log