Merge "rdbms: Drop unused constructor property and default method arg"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Mon, 25 Mar 2019 14:59:11 +0000 (14:59 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Mon, 25 Mar 2019 14:59:11 +0000 (14:59 +0000)
20 files changed:
RELEASE-NOTES-1.33
docs/hooks.txt
includes/ActorMigration.php
includes/DefaultSettings.php
includes/MagicWordArray.php
includes/api/ApiFormatBase.php
includes/config/ConfigRepository.php
includes/db/DatabaseOracle.php
includes/db/MWLBFactory.php
includes/filerepo/file/ForeignAPIFile.php
includes/libs/objectcache/BagOStuff.php
includes/libs/objectcache/MultiWriteBagOStuff.php
includes/libs/objectcache/RESTBagOStuff.php
includes/media/MediaHandlerFactory.php
includes/rcfeed/RedisPubSubFeedEngine.php
includes/registration/ExtensionProcessor.php
includes/upload/UploadBase.php
resources/src/mediawiki.widgets.visibleLengthLimit/mediawiki.widgets.visibleLengthLimit.js
tests/parser/ParserTestRunner.php
tests/phpunit/includes/libs/objectcache/BagOStuffTest.php

index 93d3253..72a468b 100644 (file)
@@ -84,6 +84,8 @@ For notes on 1.32.x and older releases, see HISTORY.
   is no longer a problem, because the code now ensures the timestamp is always
   higher than the previous one. The writes are guarded with CAS logic (check
   and set), which prevents updates that would overlap.
+* $wgDBmysql5 (T196185) - This experimental setting, deprecated in 1.31, has
+  been removed.
 
 === New user-facing features in 1.33 ===
 * (T96041) __EXPECTUNUSEDCATEGORY__ on a category page causes the category
index 4ef680a..139123d 100644 (file)
@@ -2446,10 +2446,14 @@ $userLang: the user language (Language or StubUserLang object)
 $wikiPage: the WikiPage (object) being saved
 $user: the user (object) saving the article
 $content: the new article content, as a Content object
-$summary: the article summary (comment)
-$isminor: minor flag
-$iswatch: watch flag
-$section: section #
+&$summary: CommentStoreComment object containing the edit comment. Can be replaced with a new one.
+$isminor: Boolean flag specifying if the edit was marked as minor.
+$iswatch: Previously a watch flag. Currently unused, always null.
+$section: Previously the section number being edited. Currently unused, always null.
+$flags: All EDIT_… flags (including EDIT_MINOR) as an integer number. See WikiPage::doEditContent
+  documentation for flags' definition.
+$status: StatusValue object for the hook handlers resulting status. Either set $status->fatal() or
+  return false to abort the save action.
 
 'PageContentSaveComplete': After an article has been updated.
 $wikiPage: WikiPage modified
index 0c33eb9..597b8e7 100644 (file)
@@ -136,11 +136,7 @@ class ActorMigration {
         * @return string[] [ $text, $actor ]
         */
        private static function getFieldNames( $key ) {
-               if ( isset( self::$specialFields[$key] ) ) {
-                       return self::$specialFields[$key];
-               }
-
-               return [ $key . '_text', substr( $key, 0, -5 ) . '_actor' ];
+               return self::$specialFields[$key] ?? [ $key . '_text', substr( $key, 0, -5 ) . '_actor' ];
        }
 
        /**
index 3afa593..7a645a6 100644 (file)
@@ -2114,26 +2114,6 @@ $wgDBerrorLog = false;
  */
 $wgDBerrorLogTZ = false;
 
-/**
- * Set to true to engage MySQL 4.1/5.0 charset-related features;
- * for now will just cause sending of 'SET NAMES=utf8' on connect.
- *
- * @warning THIS IS EXPERIMENTAL!
- *
- * May break if you're not using the table defs from mysql5/tables.sql.
- * May break if you're upgrading an existing wiki if set differently.
- * Broken symptoms likely to include incorrect behavior with page titles,
- * usernames, comments etc containing non-ASCII characters.
- * Might also cause failures on the object cache and other things.
- *
- * Even correct usage may cause failures with Unicode supplementary
- * characters (those not in the Basic Multilingual Plane) unless MySQL
- * has enhanced their Unicode support.
- *
- * @deprecated since 1.31
- */
-$wgDBmysql5 = false;
-
 /**
  * Set true to enable Oracle DCRP (supported from 11gR1 onward)
  *
index fde32ce..707c644 100644 (file)
@@ -268,10 +268,7 @@ class MagicWordArray {
                        return $hash[1][$text];
                }
                $lc = $this->factory->getContentLanguage()->lc( $text );
-               if ( isset( $hash[0][$lc] ) ) {
-                       return $hash[0][$lc];
-               }
-               return false;
+               return $hash[0][$lc] ?? false;
        }
 
        /**
index e033525..bff9fd0 100644 (file)
@@ -158,11 +158,9 @@ abstract class ApiFormatBase extends ApiBase {
 
                if ( !is_array( $paramSettings ) ) {
                        return $paramSettings;
-               } elseif ( isset( $paramSettings[self::PARAM_DFLT] ) ) {
-                       return $paramSettings[self::PARAM_DFLT];
-               } else {
-                       return null;
                }
+
+               return $paramSettings[self::PARAM_DFLT] ?? null;
        }
 
        /**
index 96dc51c..2874c33 100644 (file)
@@ -71,10 +71,8 @@ class ConfigRepository implements SalvageableService {
                if ( !$this->has( $name, true ) ) {
                        throw new \ConfigException( 'The configuration option ' . $name . ' does not exist.' );
                }
-               if ( isset( $this->configItems['public'][$name] ) ) {
-                       return $this->configItems['public'][$name];
-               }
-               return $this->configItems['private'][$name];
+
+               return $this->configItems['public'][$name] ?? $this->configItems['private'][$name];
        }
 
        /**
index 16bde4b..bc3873d 100644 (file)
@@ -700,14 +700,6 @@ class DatabaseOracle extends Database {
                return new Blob( $b );
        }
 
-       function decodeBlob( $b ) {
-               if ( $b instanceof Blob ) {
-                       $b = $b->fetch();
-               }
-
-               return $b;
-       }
-
        function unionQueries( $sqls, $all ) {
                $glue = ' UNION ALL ';
 
@@ -1350,10 +1342,6 @@ class DatabaseOracle extends Database {
                return 'BITOR(' . $fieldLeft . ', ' . $fieldRight . ')';
        }
 
-       function getServer() {
-               return $this->server;
-       }
-
        public function buildGroupConcatField(
                $delim, $table, $field, $conds = '', $join_conds = []
        ) {
index 6ed693e..9851460 100644 (file)
@@ -122,7 +122,6 @@ abstract class MWLBFactory {
                                                'tablePrefix' => $mainConfig->get( 'DBprefix' ),
                                                'flags' => DBO_DEFAULT,
                                                'sqlMode' => $mainConfig->get( 'SQLMode' ),
-                                               'utf8Mode' => $mainConfig->get( 'DBmysql5' )
                                        ];
 
                                        $lbConf['servers'][$i] = $server;
@@ -142,7 +141,6 @@ abstract class MWLBFactory {
                                        'load' => 1,
                                        'flags' => $flags,
                                        'sqlMode' => $mainConfig->get( 'SQLMode' ),
-                                       'utf8Mode' => $mainConfig->get( 'DBmysql5' )
                                ];
                                if ( in_array( $server['type'], $typesWithSchema, true ) ) {
                                        $server += [ 'schema' => $mainConfig->get( 'DBmwschema' ) ];
@@ -168,7 +166,6 @@ abstract class MWLBFactory {
                                        $lbConf['serverTemplate']['schema'] = $mainConfig->get( 'DBmwschema' );
                                }
                                $lbConf['serverTemplate']['sqlMode'] = $mainConfig->get( 'SQLMode' );
-                               $lbConf['serverTemplate']['utf8Mode'] = $mainConfig->get( 'DBmysql5' );
                        }
                }
 
index c49810c..3a75720 100644 (file)
@@ -184,11 +184,7 @@ class ForeignAPIFile extends File {
         *   null on error
         */
        public function getExtendedMetadata() {
-               if ( isset( $this->mInfo['extmetadata'] ) ) {
-                       return $this->mInfo['extmetadata'];
-               }
-
-               return null;
+               return $this->mInfo['extmetadata'] ?? null;
        }
 
        /**
index e2b0212..4fe64f2 100644 (file)
@@ -278,7 +278,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
         * @throws InvalidArgumentException
         */
        public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) {
-               return $this->mergeViaLock( $key, $callback, $exptime, $attempts, $flags );
+               return $this->mergeViaCas( $key, $callback, $exptime, $attempts, $flags );
        }
 
        /**
@@ -311,11 +311,13 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
 
                        // Derive the new value from the old value
                        $value = call_user_func( $callback, $this, $key, $currentValue, $exptime );
+                       $hadNoCurrentValue = ( $currentValue === false );
+                       unset( $currentValue ); // free RAM in case the value is large
 
                        $this->clearLastError();
                        if ( $value === false ) {
                                $success = true; // do nothing
-                       } elseif ( $currentValue === false ) {
+                       } elseif ( $hadNoCurrentValue ) {
                                // Try to create the key, failing if it gets created in the meantime
                                $success = $this->add( $key, $value, $exptime, $flags );
                        } else {
@@ -369,58 +371,6 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
                return $success;
        }
 
-       /**
-        * @see BagOStuff::merge()
-        *
-        * @param string $key
-        * @param callable $callback Callback method to be executed
-        * @param int $exptime Either an interval in seconds or a unix timestamp for expiry
-        * @param int $attempts The amount of times to attempt a merge in case of failure
-        * @param int $flags Bitfield of BagOStuff::WRITE_* constants
-        * @return bool Success
-        */
-       protected function mergeViaLock( $key, $callback, $exptime = 0, $attempts = 10, $flags = 0 ) {
-               if ( $attempts <= 1 ) {
-                       $timeout = 0; // clearly intended to be "non-blocking"
-               } else {
-                       $timeout = 3;
-               }
-
-               if ( !$this->lock( $key, $timeout ) ) {
-                       return false;
-               }
-
-               $this->clearLastError();
-               $reportDupes = $this->reportDupes;
-               $this->reportDupes = false;
-               $currentValue = $this->get( $key, self::READ_LATEST );
-               $this->reportDupes = $reportDupes;
-
-               if ( $this->getLastError() ) {
-                       $this->logger->warning(
-                               __METHOD__ . ' failed due to I/O error on get() for {key}.',
-                               [ 'key' => $key ]
-                       );
-
-                       $success = false;
-               } else {
-                       // Derive the new value from the old value
-                       $value = call_user_func( $callback, $this, $key, $currentValue, $exptime );
-                       if ( $value === false ) {
-                               $success = true; // do nothing
-                       } else {
-                               $success = $this->set( $key, $value, $exptime, $flags ); // set the new value
-                       }
-               }
-
-               if ( !$this->unlock( $key ) ) {
-                       // this should never happen
-                       trigger_error( "Could not release lock for key '$key'." );
-               }
-
-               return $success;
-       }
-
        /**
         * Change the expiration on a key if it exists
         *
index 5cf9de4..2d3eed5 100644 (file)
@@ -104,7 +104,7 @@ class MultiWriteBagOStuff extends BagOStuff {
                if ( ( $flags & self::READ_LATEST ) == self::READ_LATEST ) {
                        // If the latest write was a delete(), we do NOT want to fallback
                        // to the other tiers and possibly see the old value. Also, this
-                       // is used by mergeViaLock(), which only needs to hit the primary.
+                       // is used by merge(), which only needs to hit the primary.
                        return $this->caches[0]->get( $key, $flags );
                }
 
index c127ec6..7e578f9 100644 (file)
@@ -84,12 +84,15 @@ class RESTBagOStuff extends BagOStuff {
                $this->client->setLogger( $logger );
        }
 
-       /**
-        * @param string $key
-        * @param int $flags Bitfield of BagOStuff::READ_* constants [optional]
-        * @return mixed Returns false on failure and if the item does not exist
-        */
        protected function doGet( $key, $flags = 0 ) {
+               $casToken = null;
+
+               return $this->getWithToken( $key, $casToken, $flags );
+       }
+
+       protected function getWithToken( $key, &$casToken, $flags = 0 ) {
+               $casToken = null;
+
                $req = [
                        'method' => 'GET',
                        'url' => $this->url . rawurlencode( $key ),
@@ -98,7 +101,11 @@ class RESTBagOStuff extends BagOStuff {
                list( $rcode, $rdesc, $rhdrs, $rbody, $rerr ) = $this->client->run( $req );
                if ( $rcode === 200 ) {
                        if ( is_string( $rbody ) ) {
-                               return unserialize( $rbody );
+                               $value = unserialize( $rbody );
+                               /// @FIXME: use some kind of hash or UUID header as CAS token
+                               $casToken = ( $value !== false ) ? $rbody : null;
+
+                               return $value;
                        }
                        return false;
                }
@@ -108,22 +115,6 @@ class RESTBagOStuff extends BagOStuff {
                return false;
        }
 
-       /**
-        * Handle storage error
-        * @param string $msg Error message
-        * @param int $rcode Error code from client
-        * @param string $rerr Error message from client
-        * @return false
-        */
-       protected function handleError( $msg, $rcode, $rerr ) {
-               $this->logger->error( "$msg : ({code}) {error}", [
-                       'code' => $rcode,
-                       'error' => $rerr
-               ] );
-               $this->setLastError( $rcode === 0 ? self::ERR_UNREACHABLE : self::ERR_UNEXPECTED );
-               return false;
-       }
-
        public function set( $key, $value, $exptime = 0, $flags = 0 ) {
                // @TODO: respect WRITE_SYNC (e.g. EACH_QUORUM)
                // @TODO: respect $exptime
@@ -172,4 +163,24 @@ class RESTBagOStuff extends BagOStuff {
 
                return false;
        }
+
+       public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) {
+               return $this->mergeViaCas( $key, $callback, $exptime, $attempts, $flags );
+       }
+
+       /**
+        * Handle storage error
+        * @param string $msg Error message
+        * @param int $rcode Error code from client
+        * @param string $rerr Error message from client
+        * @return false
+        */
+       protected function handleError( $msg, $rcode, $rerr ) {
+               $this->logger->error( "$msg : ({code}) {error}", [
+                       'code' => $rcode,
+                       'error' => $rerr
+               ] );
+               $this->setLastError( $rcode === 0 ? self::ERR_UNREACHABLE : self::ERR_UNEXPECTED );
+               return false;
+       }
 }
index 543dc80..82e8d1f 100644 (file)
@@ -66,11 +66,7 @@ class MediaHandlerFactory {
        }
 
        protected function getHandlerClass( $type ) {
-               if ( isset( $this->registry[$type] ) ) {
-                       return $this->registry[$type];
-               } else {
-                       return false;
-               }
+               return $this->registry[$type] ?? false;
        }
 
        /**
index 8a3aa0c..c954df1 100644 (file)
@@ -37,7 +37,7 @@
  *
  * @since 1.22
  */
-class RedisPubSubFeedEngine extends RCFeedEngine {
+class RedisPubSubFeedEngine extends FormattedRCFeed {
 
        /**
         * @see FormattedRCFeed::send
@@ -68,8 +68,8 @@ class RedisPubSubFeedEngine extends RCFeedEngine {
                if ( $conn !== false ) {
                        $conn->publish( $channel, $line );
                        return true;
-               } else {
-                       return false;
                }
+
+               return false;
        }
 }
index 1d3fd86..b474ddc 100644 (file)
@@ -189,7 +189,6 @@ class ExtensionProcessor implements Processor {
         * @param string $path
         * @param array $info
         * @param int $version manifest_version for info
-        * @return array
         */
        public function extractInfo( $path, array $info, $version ) {
                $dir = dirname( $path );
index c42584c..d00ad97 100644 (file)
@@ -947,8 +947,8 @@ abstract class UploadBase {
                 */
                list( $partname, $ext ) = $this->splitExtensions( $this->mFilteredName );
 
-               if ( count( $ext ) ) {
-                       $this->mFinalExtension = trim( $ext[count( $ext ) - 1] );
+               if ( $ext !== [] ) {
+                       $this->mFinalExtension = trim( end( $ext ) );
                } else {
                        $this->mFinalExtension = '';
 
index 63da95a..453bf03 100644 (file)
         *
         * @param {OO.ui.TextInputWidget} textInputWidget Text input widget
         * @param {number} [limit] Byte limit, defaults to $input's maxlength
+        * @param {Function} [filterFunction] Function to call on the string before assessing the length.
         */
-       mw.widgets.visibleByteLimit = function ( textInputWidget, limit ) {
+       mw.widgets.visibleByteLimit = function ( textInputWidget, limit, filterFunction ) {
                limit = limit || +textInputWidget.$input.attr( 'maxlength' );
+               if ( !filterFunction || typeof filterFunction !== 'function' ) {
+                       filterFunction = undefined;
+               }
 
                function updateCount() {
-                       var remaining = limit - byteLength( textInputWidget.getValue() );
+                       var value = textInputWidget.getValue(),
+                               remaining;
+                       if ( filterFunction ) {
+                               value = filterFunction( value );
+                       }
+                       remaining = limit - byteLength( value );
                        if ( remaining > 99 ) {
                                remaining = '';
                        } else {
@@ -32,7 +41,7 @@
                updateCount();
 
                // Actually enforce limit
-               textInputWidget.$input.byteLimit( limit );
+               textInputWidget.$input.byteLimit( limit, filterFunction );
        };
 
        /**
         * Uses jQuery#codePointLimit to enforce the limit.
         *
         * @param {OO.ui.TextInputWidget} textInputWidget Text input widget
-        * @param {number} [limit] Byte limit, defaults to $input's maxlength
+        * @param {number} [limit] Code point limit, defaults to $input's maxlength
+        * @param {Function} [filterFunction] Function to call on the string before assessing the length.
         */
-       mw.widgets.visibleCodePointLimit = function ( textInputWidget, limit ) {
+       mw.widgets.visibleCodePointLimit = function ( textInputWidget, limit, filterFunction ) {
                limit = limit || +textInputWidget.$input.attr( 'maxlength' );
+               if ( !filterFunction || typeof filterFunction !== 'function' ) {
+                       filterFunction = undefined;
+               }
 
                function updateCount() {
-                       var remaining = limit - codePointLength( textInputWidget.getValue() );
+                       var value = textInputWidget.getValue(),
+                               remaining;
+                       if ( filterFunction ) {
+                               value = filterFunction( value );
+                       }
+                       remaining = limit - codePointLength( value );
                        if ( remaining > 99 ) {
                                remaining = '';
                        } else {
@@ -60,7 +78,7 @@
                updateCount();
 
                // Actually enforce limit
-               textInputWidget.$input.codePointLimit( limit );
+               textInputWidget.$input.codePointLimit( limit, filterFunction );
        };
 
 }() );
index 699de95..1c93261 100644 (file)
@@ -938,12 +938,7 @@ class ParserTestRunner {
         */
        private static function getOptionValue( $key, $opts, $default ) {
                $key = strtolower( $key );
-
-               if ( isset( $opts[$key] ) ) {
-                       return $opts[$key];
-               } else {
-                       return $default;
-               }
+               return $opts[$key] ?? $default;
        }
 
        /**
index b68ffaf..3d8c9cb 100644 (file)
@@ -65,20 +65,10 @@ class BagOStuffTest extends MediaWikiTestCase {
 
        /**
         * @covers BagOStuff::merge
-        * @covers BagOStuff::mergeViaLock
         * @covers BagOStuff::mergeViaCas
         */
        public function testMerge() {
                $key = $this->cache->makeKey( self::TEST_KEY );
-               $locks = false;
-               $checkLockingCallback = function ( BagOStuff $cache, $key, $oldVal ) use ( &$locks ) {
-                       $locks = $cache->get( "$key:lock" );
-
-                       return false;
-               };
-
-               $this->cache->merge( $key, $checkLockingCallback, 5 );
-               $this->assertFalse( $this->cache->get( $key ) );
 
                $calls = 0;
                $casRace = false; // emulate a race
@@ -103,31 +93,19 @@ class BagOStuffTest extends MediaWikiTestCase {
                $this->assertEquals( 'mergedmerged', $this->cache->get( $key ) );
 
                $calls = 0;
-               if ( $locks ) {
-                       // merge were something else already was merging (e.g. had the lock)
-                       $this->cache->lock( $key );
-                       $this->assertFalse(
-                               $this->cache->merge( $key, $callback, 5, 1 ),
-                               'Non-blocking merge (locking)'
-                       );
-                       $this->cache->unlock( $key );
-                       $this->assertEquals( 0, $calls );
-               } else {
-                       $casRace = true;
-                       $this->assertFalse(
-                               $this->cache->merge( $key, $callback, 5, 1 ),
-                               'Non-blocking merge (CAS)'
-                       );
-                       $this->assertEquals( 1, $calls );
-               }
+               $casRace = true;
+               $this->assertFalse(
+                       $this->cache->merge( $key, $callback, 5, 1 ),
+                       'Non-blocking merge (CAS)'
+               );
+               $this->assertEquals( 1, $calls );
        }
 
        /**
         * @covers BagOStuff::merge
-        * @covers BagOStuff::mergeViaLock
         * @dataProvider provideTestMerge_fork
         */
-       public function testMerge_fork( $exists, $winsLocking, $resLocking, $resCAS ) {
+       public function testMerge_fork( $exists, $childWins, $resCAS ) {
                $key = $this->cache->makeKey( self::TEST_KEY );
                $pCallback = function ( BagOStuff $cache, $key, $oldVal ) {
                        return ( $oldVal === false ) ? 'init-parent' : $oldVal . '-merged-parent';
@@ -153,16 +131,12 @@ class BagOStuffTest extends MediaWikiTestCase {
                $fork &= !$this->cache instanceof MultiWriteBagOStuff;
                if ( $fork ) {
                        $pid = null;
-                       $locked = false;
                        // Function to start merge(), run another merge() midway through, then finish
-                       $func = function ( BagOStuff $cache, $key, $cur )
-                               use ( $pCallback, $cCallback, &$pid, &$locked )
-                       {
+                       $func = function ( $cache, $key, $cur ) use ( $pCallback, $cCallback, &$pid ) {
                                $pid = pcntl_fork();
                                if ( $pid == -1 ) {
                                        return false;
                                } elseif ( $pid ) {
-                                       $locked = $cache->get( "$key:lock" ); // parent has lock?
                                        pcntl_wait( $status );
 
                                        return $pCallback( $cache, $key, $cur );
@@ -182,15 +156,9 @@ class BagOStuffTest extends MediaWikiTestCase {
                                return; // can't fork, ignore this test...
                        }
 
-                       if ( $locked ) {
-                               // merge succeed since child was locked out
-                               $this->assertEquals( $winsLocking, $merged );
-                               $this->assertEquals( $this->cache->get( $key ), $resLocking );
-                       } else {
-                               // merge has failed because child process was merging (and we only attempted once)
-                               $this->assertEquals( !$winsLocking, $merged );
-                               $this->assertEquals( $this->cache->get( $key ), $resCAS );
-                       }
+                       // merge has failed because child process was merging (and we only attempted once)
+                       $this->assertEquals( !$childWins, $merged );
+                       $this->assertEquals( $this->cache->get( $key ), $resCAS );
                } else {
                        $this->markTestSkipped( 'No pcntl methods available' );
                }
@@ -198,9 +166,9 @@ class BagOStuffTest extends MediaWikiTestCase {
 
        function provideTestMerge_fork() {
                return [
-                       // (already exists, parent wins if locking, result if locking, result if CAS)
-                       [ false, true, 'init-parent', 'init-child' ],
-                       [ true, true, 'x-merged-parent', 'x-merged-child' ]
+                       // (already exists, child wins CAS, result of CAS)
+                       [ false, true, 'init-child' ],
+                       [ true, true, 'x-merged-child' ]
                ];
        }