Merge "filerepo: only trigger maybeUpgradeRow() on action=purge"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Fri, 29 Mar 2019 18:00:44 +0000 (18:00 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Fri, 29 Mar 2019 18:00:44 +0000 (18:00 +0000)
14 files changed:
RELEASE-NOTES-1.33
docs/hooks.txt
includes/actions/HistoryAction.php
includes/libs/objectcache/APCBagOStuff.php
includes/libs/objectcache/APCUBagOStuff.php
languages/i18n/en.json
languages/i18n/qqq.json
maintenance/Maintenance.php
maintenance/doMaintenance.php
maintenance/getConfiguration.php
maintenance/update.php
resources/Resources.php
tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
tests/selenium/specs/rollback.js

index ddd6da9..6e5de2c 100644 (file)
@@ -109,6 +109,7 @@ For notes on 1.32.x and older releases, see HISTORY.
   Content::getNativeData() for text-based content models.
 * (T214706) LinksUpdate::getAddedExternalLinks() and
   LinksUpdate::getRemovedExternalLinks() were introduced.
+* (T213893) Added 'MaintenanceUpdateAddParams' hook
 
 === External library changes in 1.33 ===
 ==== New external libraries ====
index 139123d..e9ceb95 100644 (file)
@@ -2198,6 +2198,16 @@ Special:LonelyPages.
 'MagicWordwgVariableIDs': When defining new magic words IDs.
 &$variableIDs: array of strings
 
+'MaintenanceUpdateAddParams': allow extensions to add params to the update.php
+maintenance script.
+&$params: array to populate with the params to be added. Array elements are keyed by
+the param name. Each param is an associative array that must include the following keys:
+  - desc The description of the param to show on --help
+  - require Is the param required? Defaults to false if not set.
+  - withArg Is an argument required with this option?  Defaults to false if not set.
+  - shortName Character to use as short name, or false if none.  Defaults to false if not set.
+  - multiOccurrence Can this option be passed multiple times?  Defaults to false if not set.
+
 'MaintenanceRefreshLinksInit': before executing the refreshLinks.php maintenance
 script.
 $refreshLinks: RefreshLinks object
index e9f8b6f..7ff6f7d 100644 (file)
@@ -229,7 +229,6 @@ class HistoryAction extends FormlessAction {
                }
 
                // Add the general form.
-               $action = htmlspecialchars( wfScript() );
                $fields = [
                        [
                                'name' => 'title',
@@ -268,7 +267,7 @@ class HistoryAction extends FormlessAction {
                $htmlForm = HTMLForm::factory( 'ooui', $fields, $this->getContext() );
                $htmlForm
                        ->setMethod( 'get' )
-                       ->setAction( $action )
+                       ->setAction( wfScript() )
                        ->setId( 'mw-history-searchform' )
                        ->setSubmitText( $this->msg( 'historyaction-submit' )->text() )
                        ->setWrapperLegend( $this->msg( 'history-fieldset-title' )->text() );
index 902cd6a..9a5a433 100644 (file)
 /**
  * This is a wrapper for APC's shared memory functions
  *
+ * Use PHP serialization to avoid bugs and easily create CAS tokens.
+ * APCu has a memory corruption bug when the serializer is set to 'default'.
+ * See T120267, and upstream bug reports:
+ *  - https://github.com/krakjoe/apcu/issues/38
+ *  - https://github.com/krakjoe/apcu/issues/35
+ *  - https://github.com/krakjoe/apcu/issues/111
+ *
  * @ingroup Cache
  */
 class APCBagOStuff extends BagOStuff {
-
-       /**
-        * @var bool If true, trust the APC implementation to serialize and
-        * deserialize objects correctly. If false, (de-)serialize in PHP.
-        */
-       protected $nativeSerialize;
-
        /**
         * @var string String to append to each APC key. This may be changed
         *  whenever the handling of values is changed, to prevent existing code
         *  from encountering older values which it cannot handle.
         */
-       const KEY_SUFFIX = ':2';
-
-       /**
-        * Available parameters are:
-        *   - nativeSerialize:     If true, pass objects to apc_store(), and trust it
-        *                          to serialize them correctly. If false, serialize
-        *                          all values in PHP.
-        *
-        * @param array $params
-        */
-       public function __construct( array $params = [] ) {
-               parent::__construct( $params );
-
-               if ( isset( $params['nativeSerialize'] ) ) {
-                       $this->nativeSerialize = $params['nativeSerialize'];
-               } elseif ( extension_loaded( 'apcu' ) && ini_get( 'apc.serializer' ) === 'default' ) {
-                       // APCu has a memory corruption bug when the serializer is set to 'default'.
-                       // See T120267, and upstream bug reports:
-                       //  - https://github.com/krakjoe/apcu/issues/38
-                       //  - https://github.com/krakjoe/apcu/issues/35
-                       //  - https://github.com/krakjoe/apcu/issues/111
-                       $this->logger->warning(
-                               'The APCu extension is loaded and the apc.serializer INI setting ' .
-                               'is set to "default". This can cause memory corruption! ' .
-                               'You should change apc.serializer to "php" instead. ' .
-                               'See <https://github.com/krakjoe/apcu/issues/38>.'
-                       );
-                       $this->nativeSerialize = false;
-               } else {
-                       $this->nativeSerialize = true;
-               }
-       }
+       const KEY_SUFFIX = ':3';
 
        protected function doGet( $key, $flags = 0, &$casToken = null ) {
                $casToken = null;
@@ -117,18 +86,10 @@ class APCBagOStuff extends BagOStuff {
        }
 
        protected function serialize( $value ) {
-               if ( !$this->nativeSerialize && !$this->isInteger( $value ) ) {
-                       $value = serialize( $value );
-               }
-               return $value;
+               return $this->isInteger( $value ) ? (int)$value : serialize( $value );
        }
 
        protected function unserialize( $value ) {
-               if ( is_string( $value ) && !$this->nativeSerialize ) {
-                       $value = $this->isInteger( $value )
-                               ? intval( $value )
-                               : unserialize( $value );
-               }
-               return $value;
+               return $this->isInteger( $value ) ? (int)$value : unserialize( $value );
        }
 }
index da6544b..0483ee7 100644 (file)
 /**
  * This is a wrapper for APCU's shared memory functions
  *
+ * Use PHP serialization to avoid bugs and easily create CAS tokens.
+ * APCu has a memory corruption bug when the serializer is set to 'default'.
+ * See T120267, and upstream bug reports:
+ *  - https://github.com/krakjoe/apcu/issues/38
+ *  - https://github.com/krakjoe/apcu/issues/35
+ *  - https://github.com/krakjoe/apcu/issues/111
+ *
  * @ingroup Cache
  */
-class APCUBagOStuff extends APCBagOStuff {
+class APCUBagOStuff extends BagOStuff {
        /**
-        * Available parameters are:
-        *   - nativeSerialize:     If true, pass objects to apcu_store(), and trust it
-        *                          to serialize them correctly. If false, serialize
-        *                          all values in PHP.
-        *
-        * @param array $params
+        * @var string String to append to each APC key. This may be changed
+        *  whenever the handling of values is changed, to prevent existing code
+        *  from encountering older values which it cannot handle.
         */
-       public function __construct( array $params = [] ) {
-               parent::__construct( $params );
-       }
+       const KEY_SUFFIX = ':3';
 
        protected function doGet( $key, $flags = 0, &$casToken = null ) {
                $casToken = null;
@@ -100,4 +102,12 @@ class APCUBagOStuff extends APCBagOStuff {
                        return false;
                }
        }
+
+       protected function serialize( $value ) {
+               return $this->isInteger( $value ) ? (int)$value : serialize( $value );
+       }
+
+       protected function unserialize( $value ) {
+               return $this->isInteger( $value ) ? (int)$value : unserialize( $value );
+       }
 }
index 65b956e..c4c018b 100644 (file)
        "right-reupload-own": "Overwrite existing files uploaded by oneself",
        "right-reupload-shared": "Override files on the shared media repository locally",
        "right-upload_by_url": "Upload files from a URL",
-       "right-purge": "Purge the site cache for a page without confirmation",
+       "right-purge": "Purge the site cache for a page",
        "right-autoconfirmed": "Not be affected by IP-based rate limits",
        "right-bot": "Be treated as an automated process",
        "right-nominornewtalk": "Not have minor edits to discussion pages trigger the new messages prompt",
index f37b5c7..fa66817 100644 (file)
        "right-reupload-own": "{{doc-right|reupload-own}}\nRight to upload a file under a file name that already exists, and that the same user has uploaded.\n\nRelated messages:\n* {{msg-mw|right-upload}}\n* {{msg-mw|right-reupload}}",
        "right-reupload-shared": "{{doc-right|reupload-shared}}\nThe right to upload a file locally under a file name that already exists in a shared database (for example Commons).\n\nRelated messages:\n* {{msg-mw|right-upload}}\n* {{msg-mw|right-reupload}}",
        "right-upload_by_url": "{{doc-right|upload_by_url}}",
-       "right-purge": "{{doc-right|purge}}\nThe right to use <code>&action=purge</code> in the URL, without needing to confirm it (by default, anonymous users need to confirm it).",
+       "right-purge": "{{doc-right|purge}}\nThe right to use <code>&action=purge</code> in the URL.",
        "right-autoconfirmed": "{{doc-right|autoconfirmed}}\nIf your account is older than [[mw:Manual:$wgAutoConfirmAge|wgAutoConfirmAge]] and if you have at least [[mw:Manual:$wgAutoConfirmCount|$wgAutoConfirmCount]] edits, you are in the '''group \"autoconfirmed\"''' (note that you can't see this group at [[Special:ListUsers]]).\nIf you are in that group, you have (by default) the '''right \"autoconfirmed\"''', which exempts you from certain rate limits (those based on your IP address or otherwise intended solely for new users). Other rate limits may still apply; see {{msg-mw|right-noratelimit}}.",
        "right-bot": "{{doc-right|bot}}",
        "right-nominornewtalk": "{{doc-right|nominornewtalk}}\nIf someone with this right (bots by default) edits a user talk page and marks it as minor (requires {{msg-mw|right-minoredit}}), the user will not get a notification \"You have new messages\".",
index 3476a32..b3e958f 100644 (file)
@@ -54,6 +54,18 @@ use Wikimedia\Rdbms\IMaintainableDatabase;
  * is the execute() method. See docs/maintenance.txt for more info
  * and a quick demo of how to use it.
  *
+ * Terminology:
+ *   params: registry of named values that may be passed to the script
+ *   arg list: registry of positional values that may be passed to the script
+ *   options: passed param values
+ *   args: passed positional values
+ *
+ * In the command:
+ *   mwscript somescript.php --foo=bar baz
+ * foo is a param
+ * bar is the option value of the option for param foo
+ * baz is the arg value at index 0 in the arg list
+ *
  * @since 1.16
  * @ingroup Maintenance
  */
@@ -69,13 +81,13 @@ abstract class Maintenance {
        // Const for getStdin()
        const STDIN_ALL = 'all';
 
-       // This is the desired params
+       // Array of desired/allowed params
        protected $mParams = [];
 
        // Array of mapping short parameters to long ones
        protected $mShortParamsMap = [];
 
-       // Array of desired args
+       // Array of desired/allowed args
        protected $mArgList = [];
 
        // This is the list of options that were actually passed
@@ -738,7 +750,6 @@ abstract class Maintenance {
                }
 
                $this->loadParamsAndArgs();
-               $this->maybeHelp();
 
                # Set the memory limit
                # Note we need to set it again later in cache LocalSettings changed it
@@ -758,8 +769,6 @@ abstract class Maintenance {
                while ( ob_get_level() > 0 ) {
                        ob_end_flush();
                }
-
-               $this->validateParamsAndArgs();
        }
 
        /**
@@ -972,7 +981,7 @@ abstract class Maintenance {
        /**
         * Run some validation checks on the params, etc
         */
-       protected function validateParamsAndArgs() {
+       public function validateParamsAndArgs() {
                $die = false;
                # Check to make sure we've got all the required options
                foreach ( $this->mParams as $opt => $info ) {
@@ -998,9 +1007,7 @@ abstract class Maintenance {
                        }
                }
 
-               if ( $die ) {
-                       $this->maybeHelp( true );
-               }
+               $this->maybeHelp( $die );
        }
 
        /**
index 1f1a4c7..1c53fe8 100644 (file)
@@ -90,6 +90,8 @@ $maintenance->checkRequiredExtensions();
 // This avoids having long running scripts just OOM and lose all the updates.
 $maintenance->setAgentAndTriggers();
 
+$maintenance->validateParamsAndArgs();
+
 // Do the work
 $success = $maintenance->execute();
 
index de6e87a..f56729c 100644 (file)
@@ -56,7 +56,7 @@ class GetConfiguration extends Maintenance {
                $this->addOption( 'format', implode( ', ', self::$outFormats ), false, true );
        }
 
-       protected function validateParamsAndArgs() {
+       public function validateParamsAndArgs() {
                $error_out = false;
 
                # Get the format and make sure it is set to a valid default value
index 2a1feb4..50fb6dc 100755 (executable)
@@ -242,6 +242,24 @@ class UpdateMediaWiki extends Maintenance {
                        'manualRecache' => false,
                ];
        }
+
+       public function validateParamsAndArgs() {
+               // Allow extensions to add additional params.
+               $params = [];
+               Hooks::run( 'MaintenanceUpdateAddParams', [ &$params ] );
+               foreach ( $params as $name => $param ) {
+                       $this->addOption(
+                               $name,
+                               $param['desc'],
+                               $param['require'] ?? false,
+                               $param['withArg'] ?? false,
+                               $param['shortName'] ?? false,
+                               $param['multiOccurrence'] ?? false
+                       );
+               }
+
+               parent::validateParamsAndArgs();
+       }
 }
 
 $maintClass = UpdateMediaWiki::class;
index bfa80a8..af40b73 100644 (file)
@@ -1781,14 +1781,16 @@ return [
        /* MediaWiki Special pages */
 
        'mediawiki.rcfilters.filters.base.styles' => [
-               'styles' => [
-                       'resources/src/mediawiki.rcfilters/styles/mw.rcfilters.less',
+               'skinStyles' => [
+                       'default' => 'resources/src/mediawiki.rcfilters/styles/mw.rcfilters.less',
                ],
        ],
        'mediawiki.rcfilters.highlightCircles.seenunseen.styles' => [
-               'styles' => [
-                       'resources/src/mediawiki.rcfilters/' .
-                       'styles/mw.rcfilters.ui.ChangesListWrapperWidget.highlightCircles.seenunseen.less',
+               'skinStyles' => [
+                       'default' => [
+                               'resources/src/mediawiki.rcfilters/' .
+                               'styles/mw.rcfilters.ui.ChangesListWrapperWidget.highlightCircles.seenunseen.less',
+                       ],
                ],
        ],
        'mediawiki.rcfilters.filters.dm' => [
index e6b277b..4a09a2e 100644 (file)
@@ -107,77 +107,6 @@ class BagOStuffTest extends MediaWikiTestCase {
                $this->assertEquals( $n, $calls );
        }
 
-       /**
-        * @covers BagOStuff::merge
-        * @dataProvider provideTestMerge_fork
-        */
-       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';
-               };
-               $cCallback = function ( BagOStuff $cache, $key, $oldVal ) {
-                       return ( $oldVal === false ) ? 'init-child' : $oldVal . '-merged-child';
-               };
-
-               if ( $exists ) {
-                       $this->cache->set( $key, 'x', 5 );
-               }
-
-               /*
-                * Test concurrent merges by forking this process, if:
-                * - not manually called with --use-bagostuff
-                * - pcntl_fork is supported by the system
-                * - cache type will correctly support calls over forks
-                */
-               $fork = (bool)$this->getCliArg( 'use-bagostuff' );
-               $fork &= function_exists( 'pcntl_fork' );
-               $fork &= !$this->cache instanceof HashBagOStuff;
-               $fork &= !$this->cache instanceof EmptyBagOStuff;
-               $fork &= !$this->cache instanceof MultiWriteBagOStuff;
-               if ( $fork ) {
-                       $pid = null;
-                       // Function to start merge(), run another merge() midway through, then finish
-                       $func = function ( $cache, $key, $cur ) use ( $pCallback, $cCallback, &$pid ) {
-                               $pid = pcntl_fork();
-                               if ( $pid == -1 ) {
-                                       return false;
-                               } elseif ( $pid ) {
-                                       pcntl_wait( $status );
-
-                                       return $pCallback( $cache, $key, $cur );
-                               } else {
-                                       $this->cache->merge( $key, $cCallback, 0, 1 );
-                                       // Bail out of the outer merge() in the child process since it does not
-                                       // need to attempt to write anything. Success is checked by the parent.
-                                       parent::tearDown(); // avoid phpunit notices
-                                       exit;
-                               }
-                       };
-
-                       // attempt a merge - this should fail
-                       $merged = $this->cache->merge( $key, $func, 0, 1 );
-
-                       if ( $pid == -1 ) {
-                               return; // can't fork, ignore this test...
-                       }
-
-                       // 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' );
-               }
-       }
-
-       function provideTestMerge_fork() {
-               return [
-                       // (already exists, child wins CAS, result of CAS)
-                       [ false, true, 'init-child' ],
-                       [ true, true, 'x-merged-child' ]
-               ];
-       }
-
        /**
         * @covers BagOStuff::changeTTL
         */
index d54641b..805b793 100644 (file)
@@ -50,6 +50,7 @@ describe( 'Rollback with confirmation', function () {
 
        it( 'should offer a way to cancel rollbacks', function () {
                HistoryPage.rollback.click();
+               browser.pause( 300 );
                HistoryPage.rollbackConfirmableNo.click();
 
                browser.pause( 500 );