r92339 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r92338‎ | r92339 | r92340 >
Date:23:08, 15 July 2011
Author:reedy
Status:deferred (Comments)
Tags:
Comment:
REL1_18 MFT r89401, r89451, r89512, r89513, r89523, r89529, r89532, r89549, r89553, r89559, r89574, r89598
Modified paths:
  • /branches/REL1_18/phase3/includes/api/ApiPageSet.php (modified) (history)
  • /branches/REL1_18/phase3/includes/api/ApiQuery.php (modified) (history)
  • /branches/REL1_18/phase3/includes/installer/CliInstaller.php (modified) (history)
  • /branches/REL1_18/phase3/includes/installer/DatabaseUpdater.php (modified) (history)
  • /branches/REL1_18/phase3/includes/installer/Installer.php (modified) (history)
  • /branches/REL1_18/phase3/includes/installer/LocalSettingsGenerator.php (modified) (history)
  • /branches/REL1_18/phase3/includes/installer/WebInstallerPage.php (modified) (history)
  • /branches/REL1_18/phase3/includes/json/Services_JSON.php (modified) (history)
  • /branches/REL1_18/phase3/includes/specials/SpecialVersion.php (modified) (history)
  • /branches/REL1_18/phase3/maintenance/install.php (modified) (history)
  • /branches/REL1_18/phase3/resources/Resources.php (modified) (history)
  • /branches/REL1_18/phase3/tests/phpunit/includes/libs/IEUrlExtensionTest.php (added) (history)

Diff [purge]

Index: branches/REL1_18/phase3/maintenance/install.php
——@@ -27,6 +27,7 @@
2828 }
2929
3030 define( 'MW_CONFIG_CALLBACK', 'Installer::overrideConfig' );
 31+define( 'MEDIAWIKI_INSTALL', true );
3132
3233 require_once( dirname( dirname( __FILE__ ) )."/maintenance/Maintenance.php" );
3334
Index: branches/REL1_18/phase3/tests/phpunit/includes/libs/IEUrlExtensionTest.php
——@@ -0,0 +1,118 @@
 2+<?php
 3+
 4+/**
 5+ * Tests for IEUrlExtension::findIE6Extension
 6+ */
 7+class IEUrlExtensionTest extends MediaWikiTestCase {
 8+ function testSimple() {
 9+ $this->assertEquals(
 10+ 'y',
 11+ IEUrlExtension::findIE6Extension( 'x.y' ),
 12+ 'Simple extension'
 13+ );
 14+ }
 15+
 16+ function testSimpleNoExt() {
 17+ $this->assertEquals(
 18+ '',
 19+ IEUrlExtension::findIE6Extension( 'x' ),
 20+ 'No extension'
 21+ );
 22+ }
 23+
 24+ function testEmpty() {
 25+ $this->assertEquals(
 26+ '',
 27+ IEUrlExtension::findIE6Extension( '' ),
 28+ 'Empty string'
 29+ );
 30+ }
 31+
 32+ function testQuestionMark() {
 33+ $this->assertEquals(
 34+ '',
 35+ IEUrlExtension::findIE6Extension( '?' ),
 36+ 'Question mark only'
 37+ );
 38+ }
 39+
 40+ function testExtQuestionMark() {
 41+ $this->assertEquals(
 42+ 'x',
 43+ IEUrlExtension::findIE6Extension( '.x?' ),
 44+ 'Extension then question mark'
 45+ );
 46+ }
 47+
 48+ function testQuestionMarkExt() {
 49+ $this->assertEquals(
 50+ 'x',
 51+ IEUrlExtension::findIE6Extension( '?.x' ),
 52+ 'Question mark then extension'
 53+ );
 54+ }
 55+
 56+ function testInvalidChar() {
 57+ $this->assertEquals(
 58+ '',
 59+ IEUrlExtension::findIE6Extension( '.x*' ),
 60+ 'Extension with invalid character'
 61+ );
 62+ }
 63+
 64+ function testInvalidCharThenExtension() {
 65+ $this->assertEquals(
 66+ 'x',
 67+ IEUrlExtension::findIE6Extension( '*.x' ),
 68+ 'Invalid character followed by an extension'
 69+ );
 70+ }
 71+
 72+ function testMultipleQuestionMarks() {
 73+ $this->assertEquals(
 74+ 'c',
 75+ IEUrlExtension::findIE6Extension( 'a?b?.c?.d?e?f' ),
 76+ 'Multiple question marks'
 77+ );
 78+ }
 79+
 80+ function testExeException() {
 81+ $this->assertEquals(
 82+ 'd',
 83+ IEUrlExtension::findIE6Extension( 'a?b?.exe?.d?.e' ),
 84+ '.exe exception'
 85+ );
 86+ }
 87+
 88+ function testExeException2() {
 89+ $this->assertEquals(
 90+ 'exe',
 91+ IEUrlExtension::findIE6Extension( 'a?b?.exe' ),
 92+ '.exe exception 2'
 93+ );
 94+ }
 95+
 96+ function testHash() {
 97+ $this->assertEquals(
 98+ '',
 99+ IEUrlExtension::findIE6Extension( 'a#b.c' ),
 100+ 'Hash character preceding extension'
 101+ );
 102+ }
 103+
 104+ function testHash2() {
 105+ $this->assertEquals(
 106+ '',
 107+ IEUrlExtension::findIE6Extension( 'a?#b.c' ),
 108+ 'Hash character preceding extension 2'
 109+ );
 110+ }
 111+
 112+ function testDotAtEnd() {
 113+ $this->assertEquals(
 114+ '',
 115+ IEUrlExtension::findIE6Extension( '.' ),
 116+ 'Dot at end of string'
 117+ );
 118+ }
 119+}
Property changes on: branches/REL1_18/phase3/tests/phpunit/includes/libs/IEUrlExtensionTest.php
___________________________________________________________________
Added: svn:eol-style
1120 + native
Index: branches/REL1_18/phase3/includes/json/Services_JSON.php
——@@ -870,7 +870,12 @@
871871 function Services_JSON_Error($message = 'unknown error', $code = null,
872872 $mode = null, $options = null, $userinfo = null)
873873 {
874 -
 874+ $this->message = $message;
875875 }
 876+
 877+ function __toString()
 878+ {
 879+ return $this->message;
 880+ }
876881 }
877882 }
Property changes on: branches/REL1_18/phase3/includes/json/Services_JSON.php
___________________________________________________________________
Modified: svn:mergeinfo
878883 Merged /trunk/phase3/includes/json/Services_JSON.php:r89401,89451,89512-89513,89523,89529,89532,89549,89553,89559,89574,89598
Index: branches/REL1_18/phase3/includes/installer/DatabaseUpdater.php
——@@ -66,6 +66,7 @@
6767 $this->maintenance = new FakeMaintenance;
6868 }
6969 $this->initOldGlobals();
 70+ $this->loadExtensions();
7071 wfRunHooks( 'LoadExtensionSchemaUpdates', array( $this ) );
7172 }
7273
——@@ -88,6 +89,25 @@
8990 }
9091
9192 /**
 93+ * Loads LocalSettings.php, if needed, and initialises everything needed for LoadExtensionSchemaUpdates hook
 94+ */
 95+ private function loadExtensions() {
 96+ if ( !defined( 'MEDIAWIKI_INSTALL' ) ) {
 97+ return; // already loaded
 98+ }
 99+ $vars = Installer::getExistingLocalSettings();
 100+ if ( !$vars ) {
 101+ return; // no LocalSettings found
 102+ }
 103+ if ( !isset( $vars['wgHooks'] ) && !isset( $vars['wgHooks']['LoadExtensionSchemaUpdates'] ) ) {
 104+ return;
 105+ }
 106+ global $wgHooks, $wgAutoloadClasses;
 107+ $wgHooks['LoadExtensionSchemaUpdates'] = $vars['wgHooks']['LoadExtensionSchemaUpdates'];
 108+ $wgAutoloadClasses = $wgAutoloadClasses + $vars['wgAutoloadClasses'];
 109+ }
 110+
 111+ /**
92112 * @throws MWException
93113 * @param DatabaseBase $db
94114 * @param bool $shared
Index: branches/REL1_18/phase3/includes/installer/Installer.php
——@@ -456,7 +456,7 @@
457457 *
458458 * @return Array
459459 */
460 - public function getExistingLocalSettings() {
 460+ public static function getExistingLocalSettings() {
461461 global $IP;
462462
463463 wfSuppressWarnings();
Index: branches/REL1_18/phase3/includes/installer/LocalSettingsGenerator.php
——@@ -183,6 +183,19 @@
184184 $metaNamespace = "\$wgMetaNamespace = \"{$this->values['wgMetaNamespace']}\";\n";
185185 }
186186
 187+ $groupRights = '';
 188+ if( $this->groupPermissions ) {
 189+ $groupRights .= "# The following permissions were set based on your choice in the installer\n";
 190+ foreach( $this->groupPermissions as $group => $rightArr ) {
 191+ $group = self::escapePhpString( $group );
 192+ foreach( $rightArr as $right => $perm ) {
 193+ $right = self::escapePhpString( $right );
 194+ $groupRights .= "\$wgGroupPermissions['$group']['$right'] = " .
 195+ wfBoolToStr( $perm ) . ";\n";
 196+ }
 197+ }
 198+ }
 199+
187200 switch( $this->values['wgMainCacheType'] ) {
188201 case 'anything':
189202 case 'db':
Index: branches/REL1_18/phase3/includes/installer/CliInstaller.php
——@@ -88,7 +88,7 @@
8989 * Main entry point.
9090 */
9191 public function execute() {
92 - $vars = $this->getExistingLocalSettings();
 92+ $vars = Installer::getExistingLocalSettings();
9393 if( $vars ) {
9494 $this->showStatusMessage(
9595 Status::newFatal( "config-localsettings-cli-upgrade" )
Index: branches/REL1_18/phase3/includes/installer/WebInstallerPage.php
——@@ -225,7 +225,7 @@
226226 class WebInstaller_ExistingWiki extends WebInstallerPage {
227227 public function execute() {
228228 // If there is no LocalSettings.php, continue to the installer welcome page
229 - $vars = $this->parent->getExistingLocalSettings();
 229+ $vars = Installer::getExistingLocalSettings();
230230 if ( !$vars ) {
231231 return 'skip';
232232 }
Index: branches/REL1_18/phase3/includes/api/ApiQuery.php
——@@ -478,10 +478,6 @@
479479 }
480480 }
481481 }
482 - // only export when there are titles
483 - if ( !count( $exportTitles ) ) {
484 - return;
485 - }
486482
487483 $exporter = new WikiExporter( $this->getDB() );
488484 // WikiExporter writes to stdout, so catch its
Index: branches/REL1_18/phase3/includes/api/ApiPageSet.php
——@@ -446,18 +446,24 @@
447447 }
448448
449449 $pageids = array_map( 'intval', $pageids ); // paranoia
450 - $set = array(
451 - 'page_id' => $pageids
452 - );
453 - $db = $this->getDB();
 450+ $remaining = array_flip( $pageids );
454451
455 - // Get pageIDs data from the `page` table
456 - $this->profileDBIn();
457 - $res = $db->select( 'page', $this->getPageTableFields(), $set,
458 - __METHOD__ );
459 - $this->profileDBOut();
 452+ $pageids = self::getPositiveIntegers( $pageids );
460453
461 - $remaining = array_flip( $pageids );
 454+ $res = null;
 455+ if ( count( $pageids ) ) {
 456+ $set = array(
 457+ 'page_id' => $pageids
 458+ );
 459+ $db = $this->getDB();
 460+
 461+ // Get pageIDs data from the `page` table
 462+ $this->profileDBIn();
 463+ $res = $db->select( 'page', $this->getPageTableFields(), $set,
 464+ __METHOD__ );
 465+ $this->profileDBOut();
 466+ }
 467+
462468 $this->initFromQueryResult( $res, $remaining, false ); // process PageIDs
463469
464470 // Resolve any found redirects
——@@ -479,20 +485,22 @@
480486 ApiBase::dieDebug( __METHOD__, 'Missing $processTitles parameter when $remaining is provided' );
481487 }
482488
483 - foreach ( $res as $row ) {
484 - $pageId = intval( $row->page_id );
 489+ if ( $res ) {
 490+ foreach ( $res as $row ) {
 491+ $pageId = intval( $row->page_id );
485492
486 - // Remove found page from the list of remaining items
487 - if ( isset( $remaining ) ) {
488 - if ( $processTitles ) {
489 - unset( $remaining[$row->page_namespace][$row->page_title] );
490 - } else {
491 - unset( $remaining[$pageId] );
 493+ // Remove found page from the list of remaining items
 494+ if ( isset( $remaining ) ) {
 495+ if ( $processTitles ) {
 496+ unset( $remaining[$row->page_namespace][$row->page_title] );
 497+ } else {
 498+ unset( $remaining[$pageId] );
 499+ }
492500 }
 501+
 502+ // Store any extra fields requested by modules
 503+ $this->processDbRow( $row );
493504 }
494 -
495 - // Store any extra fields requested by modules
496 - $this->processDbRow( $row );
497505 }
498506
499507 if ( isset( $remaining ) ) {
——@@ -534,21 +542,25 @@
535543 $pageids = array();
536544 $remaining = array_flip( $revids );
537545
538 - $tables = array( 'revision', 'page' );
539 - $fields = array( 'rev_id', 'rev_page' );
540 - $where = array( 'rev_id' => $revids, 'rev_page = page_id' );
 546+ $revids = self::getPositiveIntegers( $revids );
541547
542 - // Get pageIDs data from the `page` table
543 - $this->profileDBIn();
544 - $res = $db->select( $tables, $fields, $where, __METHOD__ );
545 - foreach ( $res as $row ) {
546 - $revid = intval( $row->rev_id );
547 - $pageid = intval( $row->rev_page );
548 - $this->mGoodRevIDs[$revid] = $pageid;
549 - $pageids[$pageid] = '';
550 - unset( $remaining[$revid] );
 548+ if ( count( $revids ) ) {
 549+ $tables = array( 'revision', 'page' );
 550+ $fields = array( 'rev_id', 'rev_page' );
 551+ $where = array( 'rev_id' => $revids, 'rev_page = page_id' );
 552+
 553+ // Get pageIDs data from the `page` table
 554+ $this->profileDBIn();
 555+ $res = $db->select( $tables, $fields, $where, __METHOD__ );
 556+ foreach ( $res as $row ) {
 557+ $revid = intval( $row->rev_id );
 558+ $pageid = intval( $row->rev_page );
 559+ $this->mGoodRevIDs[$revid] = $pageid;
 560+ $pageids[$pageid] = '';
 561+ unset( $remaining[$revid] );
 562+ }
 563+ $this->profileDBOut();
551564 }
552 - $this->profileDBOut();
553565
554566 $this->mMissingRevIDs = array_keys( $remaining );
555567
——@@ -710,6 +722,25 @@
711723 return $linkBatch;
712724 }
713725
 726+ /**
 727+ * Returns the input array of integers with all values < 0 removed
 728+ *
 729+ * @param $array array
 730+ * @return array
 731+ */
 732+ private static function getPositiveIntegers( $array ) {
 733+ // bug 25734 API: possible issue with revids validation
 734+ // It seems with a load of revision rows, MySQL gets upset
 735+ // Remove any < 0 integers, as they can't be valid
 736+ foreach( $array as $i => $int ) {
 737+ if ( $int < 0 ) {
 738+ unset( $array[$i] );
 739+ }
 740+ }
 741+
 742+ return $array;
 743+ }
 744+
714745 protected function getAllowedParams() {
715746 return array(
716747 'titles' => array(
Index: branches/REL1_18/phase3/includes/specials/SpecialVersion.php
——@@ -109,7 +109,7 @@
110110 'Aryeh Gregor', 'Aaron Schulz', 'Andrew Garrett', 'Raimond Spekking',
111111 'Alexandre Emsenhuber', 'Siebrand Mazeland', 'Chad Horohoe',
112112 'Roan Kattouw', 'Trevor Parscal', 'Bryan Tong Minh', 'Sam Reed',
113 - 'Victor Vasiliev', 'Rotem Liss', 'Platonides',
 113+ 'Victor Vasiliev', 'Rotem Liss', 'Platonides', 'Ashar Voultoiz',
114114 wfMsg( 'version-poweredby-others' )
115115 );
116116
Index: branches/REL1_18/phase3/resources/Resources.php
——@@ -135,6 +135,7 @@
136136 'jquery.suggestions' => array(
137137 'scripts' => 'resources/jquery/jquery.suggestions.js',
138138 'styles' => 'resources/jquery/jquery.suggestions.css',
 139+ 'dependencies' => 'jquery.autoEllipsis',
139140 ),
140141 'jquery.tabIndex' => array(
141142 'scripts' => 'resources/jquery/jquery.tabIndex.js',

Sign-offs

UserFlagDate
Hasharinspected11:23, 17 July 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r89401(bug 29121) jquery.suggestions uses jquery.autoEllipsis but didn't have it in...catrope09:51, 3 June 2011
r89451Followup r88124: fix the most glaring bug of the weekdemon02:54, 4 June 2011
r89512* (bug 25734) API: possible issue with revids validation...reedy17:49, 5 June 2011
r89513Followup r89512...reedy18:01, 5 June 2011
r89523* (bug 28002) Internal error in ApiFormatRaw::getMimeType...reedy19:22, 5 June 2011
r89529Follow-up r89254 and r89481: re-did loading extension updates properly, now u...maxsem19:52, 5 June 2011
r89532Followup r89529...reedy21:03, 5 June 2011
r89549follow up r89513: avoid internal error when only invalid revids/pageids are p...gurch08:45, 6 June 2011
r89553Fix the non-PEAR alternative for Services_JSON_Error to be less useless. bug ...catrope09:41, 6 June 2011
r89559Test filename updates for r89558.tstarling12:00, 6 June 2011
r89574Fix fixme on r89549reedy15:58, 6 June 2011
r89598;demon20:01, 6 June 2011

Comments

#Comment by Hashar (talk | contribs)   10:48, 16 July 2011

somehow this merge merged some part of r89311 and missed another part ! See r1=92338&r2=92339& http://svn.wikimedia.org/viewvc/mediawiki/branches/REL1_18/phase3/includes/installer/LocalSettingsGenerator.php?r1=92338&r2=92339&

I finally merged r89311 in REL1_18 with r92351

#Comment by Hashar (talk | contribs)   21:39, 16 July 2011

See also comment on r92351

#Comment by Reedy (talk | contribs)   00:06, 17 July 2011

(Computers|SVN|Merging) sucks.

Status & tagging log