Merge "Add missing title info in RenderedRevision::outputVariesOnRevisionMetaData()"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 1 Aug 2019 17:40:05 +0000 (17:40 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 1 Aug 2019 17:40:05 +0000 (17:40 +0000)
14 files changed:
includes/api/ApiHelp.php
includes/libs/lockmanager/DBLockManager.php
includes/parser/ParserOutput.php
includes/resourceloader/MessageBlobStore.php
includes/resourceloader/ResourceLoaderStartUpModule.php
includes/user/UserGroupMembership.php
resources/Resources.php
resources/src/mediawiki.apihelp.css [deleted file]
resources/src/mediawiki.apipretty.css [deleted file]
resources/src/mediawiki.apipretty/apihelp.css [new file with mode: 0644]
resources/src/mediawiki.apipretty/apipretty.css [new file with mode: 0644]
resources/src/mediawiki.inspect.js
resources/src/startup/mediawiki.js
tests/phpunit/structure/ResourcesTest.php

index cc96f90..988957b 100644 (file)
@@ -100,7 +100,7 @@ class ApiHelp extends ApiBase {
                $out = $context->getOutput();
                $out->addModuleStyles( [
                        'mediawiki.hlist',
-                       'mediawiki.apihelp',
+                       'mediawiki.apipretty',
                ] );
                if ( !empty( $options['toc'] ) ) {
                        $out->addModuleStyles( 'mediawiki.toc.styles' );
index 5e8c22b..37d53e2 100644 (file)
@@ -150,12 +150,12 @@ abstract class DBLockManager extends QuorumLockManager {
                        } elseif ( is_array( $this->dbServers[$lockDb] ) ) {
                                // Parameters to construct a new database connection
                                $config = $this->dbServers[$lockDb];
+                               $config['flags'] = ( $config['flags'] ?? 0 );
+                               $config['flags'] &= ~( IDatabase::DBO_TRX | IDatabase::DBO_DEFAULT );
                                $db = Database::factory( $config['type'], $config );
                        } else {
                                throw new UnexpectedValueException( "No server called '$lockDb'." );
                        }
-
-                       $db->clearFlag( DBO_TRX );
                        # If the connection drops, try to avoid letting the DB rollback
                        # and release the locks before the file operations are finished.
                        # This won't handle the case of DB server restarts however.
index cbba80a..1922f7d 100644 (file)
@@ -1326,9 +1326,18 @@ class ParserOutput extends CacheTime {
        }
 
        public function __sleep() {
-               return array_diff(
-                       array_keys( get_object_vars( $this ) ),
-                       [ 'mParseStartTime' ]
+               return array_filter( array_keys( get_object_vars( $this ) ),
+                       function ( $field ) {
+                               if ( $field === 'mParseStartTime' ) {
+                                       return false;
+                               } elseif ( strpos( $field, "\0" ) !== false ) {
+                                       // Unserializing unknown private fields in HHVM causes
+                                       // member variables with nulls in their names (T229366)
+                                       return false;
+                               } else {
+                                       return true;
+                               }
+                       }
                );
        }
 
index fca06c9..d0f6729 100644 (file)
@@ -224,7 +224,7 @@ class MessageBlobStore implements LoggerAwareInterface {
                        }
                }
 
-               $json = FormatJson::encode( (object)$messages );
+               $json = FormatJson::encode( (object)$messages, false, FormatJson::UTF8_OK );
                // @codeCoverageIgnoreStart
                if ( $json === false ) {
                        $this->logger->warning( 'Failed to encode message blob for {module} ({lang})', [
index 6b38ee4..dbf6b21 100644 (file)
@@ -283,8 +283,9 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                        }
 
                        if ( $versionHash !== '' && strlen( $versionHash ) !== 7 ) {
-                               $this->getLogger()->warning(
-                                       "Module '{module}' produced an invalid version hash: '{version}'.",
+                               $e = new RuntimeException( "Badly formatted module version hash" );
+                               $resourceLoader->outputErrorAndLog( $e,
+                                               "Module '{module}' produced an invalid version hash: '{version}'.",
                                        [
                                                'module' => $name,
                                                'version' => $versionHash,
index fdac4a2..2261fcb 100644 (file)
@@ -148,72 +148,66 @@ class UserGroupMembership {
         * the function fails if there is a conflicting membership entry (same user and
         * group) already in the table.
         *
-        * @throws MWException
+        * @throws UnexpectedValueException
         * @param bool $allowUpdate Whether to perform "upsert" instead of INSERT
         * @param IDatabase|null $dbw If you have one available
         * @return bool Whether or not anything was inserted
         */
        public function insert( $allowUpdate = false, IDatabase $dbw = null ) {
-               if ( $dbw === null ) {
-                       $dbw = wfGetDB( DB_MASTER );
-               }
-
-               // Purge old, expired memberships from the DB
-               $hasExpiredRow = $dbw->selectField(
-                       'user_groups',
-                       '1',
-                       [ 'ug_expiry < ' . $dbw->addQuotes( $dbw->timestamp() ) ],
-                       __METHOD__
-               );
-               if ( $hasExpiredRow ) {
-                       JobQueueGroup::singleton()->lazyPush( new UserGroupExpiryJob() );
-               }
-
-               // Check that the values make sense
                if ( $this->group === null ) {
                        throw new UnexpectedValueException(
-                               'Don\'t try inserting an uninitialized UserGroupMembership object' );
+                               'Cannot insert an uninitialized UserGroupMembership instance'
+                       );
                } elseif ( $this->userId <= 0 ) {
                        throw new UnexpectedValueException(
                                'UserGroupMembership::insert() needs a positive user ID. ' .
-                               'Did you forget to add your User object to the database before calling addGroup()?' );
+                               'Perhaps addGroup() was called before the user was added to the database.'
+                       );
                }
 
+               $dbw = $dbw ?: wfGetDB( DB_MASTER );
                $row = $this->getDatabaseArray( $dbw );
+
+               $dbw->startAtomic( __METHOD__ );
                $dbw->insert( 'user_groups', $row, __METHOD__, [ 'IGNORE' ] );
                $affected = $dbw->affectedRows();
-
-               // Don't collide with expired user group memberships
-               // Do this after trying to insert, in order to avoid locking
                if ( !$affected ) {
-                       $conds = [
-                               'ug_user' => $row['ug_user'],
-                               'ug_group' => $row['ug_group'],
-                       ];
-                       // if we're unconditionally updating, check that the expiry is not already the
-                       // same as what we are trying to update it to; otherwise, only update if
-                       // the expiry date is in the past
+                       // Conflicting row already exists; it should be overriden if it is either expired
+                       // or if $allowUpdate is true and the current row is different than the loaded row.
+                       $conds = [ 'ug_user' => $row['ug_user'], 'ug_group' => $row['ug_group'] ];
                        if ( $allowUpdate ) {
-                               if ( $this->expiry ) {
-                                       $conds[] = 'ug_expiry IS NULL OR ug_expiry != ' .
-                                               $dbw->addQuotes( $dbw->timestamp( $this->expiry ) );
-                               } else {
-                                       $conds[] = 'ug_expiry IS NOT NULL';
-                               }
+                               // Update the current row if its expiry does not match that of the loaded row
+                               $conds[] = $this->expiry
+                                       ? 'ug_expiry IS NULL OR ug_expiry != ' .
+                                               $dbw->addQuotes( $dbw->timestamp( $this->expiry ) )
+                                       : 'ug_expiry IS NOT NULL';
                        } else {
+                               // Update the current row if it is expired
                                $conds[] = 'ug_expiry < ' . $dbw->addQuotes( $dbw->timestamp() );
                        }
+                       $dbw->update(
+                               'user_groups',
+                               [ 'ug_expiry' => $this->expiry ? $dbw->timestamp( $this->expiry ) : null ],
+                               $conds,
+                               __METHOD__
+                       );
+                       $affected = $dbw->affectedRows();
+               }
+               $dbw->endAtomic( __METHOD__ );
 
-                       $row = $dbw->selectRow( 'user_groups', $this::selectFields(), $conds, __METHOD__ );
-                       if ( $row ) {
-                               $dbw->update(
-                                       'user_groups',
-                                       [ 'ug_expiry' => $this->expiry ? $dbw->timestamp( $this->expiry ) : null ],
-                                       [ 'ug_user' => $row->ug_user, 'ug_group' => $row->ug_group ],
-                                       __METHOD__ );
-                               $affected = $dbw->affectedRows();
+               // Purge old, expired memberships from the DB
+               $fname = __METHOD__;
+               DeferredUpdates::addCallableUpdate( function () use ( $dbw, $fname ) {
+                       $hasExpiredRow = $dbw->selectField(
+                               'user_groups',
+                               '1',
+                               [ 'ug_expiry < ' . $dbw->addQuotes( $dbw->timestamp() ) ],
+                               $fname
+                       );
+                       if ( $hasExpiredRow ) {
+                               JobQueueGroup::singleton()->push( new UserGroupExpiryJob() );
                        }
-               }
+               } );
 
                return $affected > 0;
        }
index e29d658..133ba0f 100644 (file)
@@ -763,10 +763,6 @@ return [
        ],
 
        /* MediaWiki */
-       'mediawiki.apihelp' => [
-               'styles' => 'resources/src/mediawiki.apihelp.css',
-               'targets' => [ 'desktop' ],
-       ],
        'mediawiki.template' => [
                'scripts' => 'resources/src/mediawiki.template.js',
                'targets' => [ 'desktop', 'mobile' ],
@@ -785,7 +781,10 @@ return [
                'dependencies' => 'mediawiki.template',
        ],
        'mediawiki.apipretty' => [
-               'styles' => 'resources/src/mediawiki.apipretty.css',
+               'styles' => [
+                       'resources/src/mediawiki.apipretty/apipretty.css',
+                       'resources/src/mediawiki.apipretty/apihelp.css',
+               ],
                'targets' => [ 'desktop', 'mobile' ],
        ],
        'mediawiki.api' => [
@@ -1154,6 +1153,7 @@ return [
                        'upload-form-label-usage-filename',
                        'action-upload',
                        'apierror-mustbeloggedin',
+                       'apierror-permissiondenied',
                        'badaccess-groups',
                        'apierror-timeout',
                        'apierror-offline',
diff --git a/resources/src/mediawiki.apihelp.css b/resources/src/mediawiki.apihelp.css
deleted file mode 100644 (file)
index d1f32ab..0000000
+++ /dev/null
@@ -1,109 +0,0 @@
-/* stylelint-disable selector-class-pattern */
-
-.apihelp-header {
-       clear: both;
-       margin-bottom: 0.1em;
-}
-
-.apihelp-header.apihelp-module-name {
-       /*
-        * This element is explicitly set to dir="ltr" in HTML.
-        * Set explicit alignment so that CSSJanus will flip it to "right";
-        * otherwise the alignment will be automatically set to "left" according
-        * to the element's direction, and this will have an inconsistent look.
-        */
-       text-align: left;
-}
-
-div.apihelp-linktrail {
-       font-size: smaller;
-}
-
-.apihelp-block {
-       margin-top: 0.5em;
-}
-
-.apihelp-block-head {
-       font-weight: bold;
-}
-
-.apihelp-flags {
-       font-size: smaller;
-       float: right;
-       border: 1px solid #000;
-       padding: 0.25em;
-       width: 20em;
-}
-
-.apihelp-deprecated,
-.apihelp-flag-deprecated,
-.apihelp-flag-internal strong {
-       font-weight: bold;
-       color: #d33;
-}
-
-.apihelp-deprecated-value {
-       text-decoration: line-through;
-}
-
-.apihelp-unknown {
-       color: #72777d;
-}
-
-.apihelp-empty {
-       color: #72777d;
-}
-
-.apihelp-help-urls ul {
-       list-style-image: none;
-       list-style-type: none;
-       margin-left: 0;
-}
-
-.apihelp-parameters dl,
-.apihelp-examples dl,
-.apihelp-permissions dl {
-       margin-left: 2em;
-}
-
-.apihelp-parameters dt {
-       float: left;
-       clear: left;
-       min-width: 10em;
-       white-space: nowrap;
-       line-height: 1.5em;
-}
-
-.apihelp-parameters dt:after {
-       content: ':\A0';
-}
-
-.apihelp-parameters dd {
-       margin: 0 0 0.5em 10em;
-       line-height: 1.5em;
-}
-
-.apihelp-parameters dd p:first-child {
-       margin-top: 0;
-}
-
-.apihelp-parameters dd.info {
-       margin-left: 12em;
-       text-indent: -2em;
-}
-
-.apihelp-examples dt {
-       font-weight: normal;
-}
-
-.api-main-links {
-       text-align: center;
-}
-
-.api-main-links ul:before {
-       content: '[';
-}
-
-.api-main-links ul:after {
-       content: ']';
-}
diff --git a/resources/src/mediawiki.apipretty.css b/resources/src/mediawiki.apipretty.css
deleted file mode 100644 (file)
index 3e921f4..0000000
+++ /dev/null
@@ -1,13 +0,0 @@
-/* stylelint-disable selector-class-pattern */
-
-.mw-special-ApiHelp h1.firstHeading {
-       display: none;
-}
-
-.api-pretty-header {
-       font-size: small;
-}
-
-.api-pretty-content {
-       white-space: pre-wrap;
-}
diff --git a/resources/src/mediawiki.apipretty/apihelp.css b/resources/src/mediawiki.apipretty/apihelp.css
new file mode 100644 (file)
index 0000000..d1f32ab
--- /dev/null
@@ -0,0 +1,109 @@
+/* stylelint-disable selector-class-pattern */
+
+.apihelp-header {
+       clear: both;
+       margin-bottom: 0.1em;
+}
+
+.apihelp-header.apihelp-module-name {
+       /*
+        * This element is explicitly set to dir="ltr" in HTML.
+        * Set explicit alignment so that CSSJanus will flip it to "right";
+        * otherwise the alignment will be automatically set to "left" according
+        * to the element's direction, and this will have an inconsistent look.
+        */
+       text-align: left;
+}
+
+div.apihelp-linktrail {
+       font-size: smaller;
+}
+
+.apihelp-block {
+       margin-top: 0.5em;
+}
+
+.apihelp-block-head {
+       font-weight: bold;
+}
+
+.apihelp-flags {
+       font-size: smaller;
+       float: right;
+       border: 1px solid #000;
+       padding: 0.25em;
+       width: 20em;
+}
+
+.apihelp-deprecated,
+.apihelp-flag-deprecated,
+.apihelp-flag-internal strong {
+       font-weight: bold;
+       color: #d33;
+}
+
+.apihelp-deprecated-value {
+       text-decoration: line-through;
+}
+
+.apihelp-unknown {
+       color: #72777d;
+}
+
+.apihelp-empty {
+       color: #72777d;
+}
+
+.apihelp-help-urls ul {
+       list-style-image: none;
+       list-style-type: none;
+       margin-left: 0;
+}
+
+.apihelp-parameters dl,
+.apihelp-examples dl,
+.apihelp-permissions dl {
+       margin-left: 2em;
+}
+
+.apihelp-parameters dt {
+       float: left;
+       clear: left;
+       min-width: 10em;
+       white-space: nowrap;
+       line-height: 1.5em;
+}
+
+.apihelp-parameters dt:after {
+       content: ':\A0';
+}
+
+.apihelp-parameters dd {
+       margin: 0 0 0.5em 10em;
+       line-height: 1.5em;
+}
+
+.apihelp-parameters dd p:first-child {
+       margin-top: 0;
+}
+
+.apihelp-parameters dd.info {
+       margin-left: 12em;
+       text-indent: -2em;
+}
+
+.apihelp-examples dt {
+       font-weight: normal;
+}
+
+.api-main-links {
+       text-align: center;
+}
+
+.api-main-links ul:before {
+       content: '[';
+}
+
+.api-main-links ul:after {
+       content: ']';
+}
diff --git a/resources/src/mediawiki.apipretty/apipretty.css b/resources/src/mediawiki.apipretty/apipretty.css
new file mode 100644 (file)
index 0000000..3e921f4
--- /dev/null
@@ -0,0 +1,13 @@
+/* stylelint-disable selector-class-pattern */
+
+.mw-special-ApiHelp h1.firstHeading {
+       display: none;
+}
+
+.api-pretty-header {
+       font-size: small;
+}
+
+.api-pretty-content {
+       white-space: pre-wrap;
+}
index a500226..7f4af5b 100644 (file)
                        if ( stats.enabled ) {
                                $.extend( stats, mw.loader.store.stats );
                                try {
-                                       raw = localStorage.getItem( mw.loader.store.getStoreKey() );
+                                       raw = localStorage.getItem( mw.loader.store.key );
                                        stats.totalSizeInBytes = byteLength( raw );
                                        stats.totalSize = humanSize( byteLength( raw ) );
                                } catch ( e ) {}
index b0355b0..ace92f0 100644 (file)
                                         * @return {Object} Module store contents.
                                         */
                                        toJSON: function () {
-                                               return { items: mw.loader.store.items, vary: mw.loader.store.getVary() };
+                                               return { items: mw.loader.store.items, vary: mw.loader.store.vary };
                                        },
 
                                        /**
-                                        * Get the localStorage key for the entire module store. The key references
+                                        * The localStorage key for the entire module store. The key references
                                         * $wgDBname to prevent clashes between wikis which share a common host.
                                         *
-                                        * @return {string} localStorage item key
+                                        * @property {string}
                                         */
-                                       getStoreKey: function () {
-                                               return $VARS.storeKey;
-                                       },
+                                       key: $VARS.storeKey,
 
                                        /**
-                                        * Get a key on which to vary the module cache.
+                                        * A string containing various factors on which to the module cache should vary.
                                         *
-                                        * @return {string} String of concatenated vary conditions.
+                                        * @property {string}
                                         */
-                                       getVary: function () {
-                                               return $VARS.storeVary;
-                                       },
+                                       vary: $VARS.storeVary,
 
                                        /**
                                         * Initialize the store.
 
                                                try {
                                                        // This a string we stored, or `null` if the key does not (yet) exist.
-                                                       raw = localStorage.getItem( this.getStoreKey() );
+                                                       raw = localStorage.getItem( this.key );
                                                        // If we get here, localStorage is available; mark enabled
                                                        this.enabled = true;
                                                        // If null, JSON.parse() will cast to string and re-parse, still null.
                                                        data = JSON.parse( raw );
-                                                       if ( data && typeof data.items === 'object' && data.vary === this.getVary() ) {
+                                                       if ( data && typeof data.items === 'object' && data.vary === this.vary ) {
                                                                this.items = data.items;
                                                                return;
                                                        }
                                                //    The store was enabled, and `items` starts fresh.
                                                //
                                                // 2. localStorage contained parseable data under our store key,
-                                               //    but it's not applicable to our current context (see getVary).
+                                               //    but it's not applicable to our current context (see #vary).
                                                //    The store was enabled, and `items` starts fresh.
                                                //
                                                // 3. JSON.parse threw (localStorage contained corrupt data).
                                        clear: function () {
                                                this.items = {};
                                                try {
-                                                       localStorage.removeItem( this.getStoreKey() );
+                                                       localStorage.removeItem( this.key );
                                                } catch ( e ) {}
                                        },
 
                                                                mw.loader.store.set( mw.loader.store.queue.shift() );
                                                        }
 
-                                                       key = mw.loader.store.getStoreKey();
+                                                       key = mw.loader.store.key;
                                                        try {
                                                                // Replacing the content of the module store might fail if the new
                                                                // contents would exceed the browser's localStorage size limit. To
index 4f9664f..acd8a19 100644 (file)
@@ -36,14 +36,6 @@ class ResourcesTest extends MediaWikiTestCase {
                );
        }
 
-       public function testVersionHash() {
-               $data = self::getAllModules();
-               foreach ( $data['modules'] as $moduleName => $module ) {
-                       $version = $module->getVersionHash( $data['context'] );
-                       $this->assertEquals( 7, strlen( $version ), "$moduleName must use ResourceLoader::makeHash" );
-               }
-       }
-
        /**
         * Verify that all modules specified as dependencies of other modules actually
         * exist and are not illegal.