Merge "Break the cyclic dependency between SearchEngine and SearchResult"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 20 Aug 2019 22:48:24 +0000 (22:48 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 20 Aug 2019 22:48:24 +0000 (22:48 +0000)
docs/hooks.txt
includes/block/BlockManager.php
includes/libs/rdbms/loadbalancer/ILoadBalancer.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php
includes/resourceloader/ResourceLoaderClientHtml.php
includes/specials/SpecialMute.php
tests/phpunit/integration/includes/db/DatabaseSqliteTest.php

index 6207b12..09b2c97 100644 (file)
@@ -3211,6 +3211,9 @@ $request: WebRequest object for getting the value provided by the current user
 $sp: SpecialPage object, for context
 &$fields: Current HTMLForm fields descriptors
 
+'SpecialMuteSubmit': DEPRECATED since 1.34! Used only for instrumentation on SpecialMute
+$data: Array containing information about submitted options on SpecialMute form
+
 'SpecialNewpagesConditions': Called when building sql query for
 Special:NewPages.
 &$special: NewPagesPager object (subclass of ReverseChronologicalPager)
index b67703c..f92dd1e 100644 (file)
@@ -21,6 +21,7 @@
 namespace MediaWiki\Block;
 
 use DateTime;
+use DateTimeZone;
 use DeferredUpdates;
 use IP;
 use MediaWiki\Config\ServiceOptions;
@@ -223,7 +224,7 @@ class BlockManager {
 
        /**
         * Try to load a block from an ID given in a cookie value. If the block is invalid
-        * or doesn't exist, remove the cookie.
+        * doesn't exist, or the cookie value is malformed, remove the cookie.
         *
         * @param UserIdentity $user
         * @param WebRequest $request
@@ -233,9 +234,13 @@ class BlockManager {
                UserIdentity $user,
                WebRequest $request
        ) {
-               $blockCookieId = $this->getIdFromCookieValue( $request->getCookie( 'BlockID' ) );
+               $cookieValue = $request->getCookie( 'BlockID' );
+               if ( is_null( $cookieValue ) ) {
+                       return false;
+               }
 
-               if ( $blockCookieId !== null ) {
+               $blockCookieId = $this->getIdFromCookieValue( $cookieValue );
+               if ( !is_null( $blockCookieId ) ) {
                        // TODO: remove dependency on DatabaseBlock
                        $block = DatabaseBlock::newFromID( $blockCookieId );
                        if (
@@ -244,9 +249,10 @@ class BlockManager {
                        ) {
                                return $block;
                        }
-                       $this->clearBlockCookie( $request->response() );
                }
 
+               $this->clearBlockCookie( $request->response() );
+
                return false;
        }
 
@@ -435,7 +441,11 @@ class BlockManager {
                }
 
                // Set the cookie. Reformat the MediaWiki datetime as a Unix timestamp for the cookie.
-               $expiryValue = DateTime::createFromFormat( 'YmdHis', $expiryTime )->format( 'U' );
+               $expiryValue = DateTime::createFromFormat(
+                       'YmdHis',
+                       $expiryTime,
+                       new DateTimeZone( 'UTC' )
+               )->format( 'U' );
                $cookieOptions = [ 'httpOnly' => false ];
                $cookieValue = $this->getCookieValue( $block );
                $response->setCookie( 'BlockID', $cookieValue, $expiryValue, $cookieOptions );
index 990705c..1125572 100644 (file)
@@ -167,8 +167,7 @@ interface ILoadBalancer {
         *
         * @param string|bool $group Query group or false for the generic group
         * @param string|bool $domain DB domain ID or false for the local domain
-        * @throws DBError If no live handle can be obtained
-        * @return bool|int|string
+        * @return int|bool Returns false if no live handle can be obtained
         */
        public function getReaderIndex( $group = false, $domain = false );
 
index 98607ce..d088aa9 100644 (file)
@@ -592,8 +592,7 @@ class LoadBalancer implements ILoadBalancer {
                        $this->connLogger->debug( __METHOD__ . ": Using reader #$i: $serverName..." );
 
                        // Get a connection to this server without triggering other server connections
-                       $flags = self::CONN_SILENCE_ERRORS;
-                       $conn = $this->getServerConnection( $i, $domain, $flags );
+                       $conn = $this->getServerConnection( $i, $domain, self::CONN_SILENCE_ERRORS );
                        if ( !$conn ) {
                                $this->connLogger->warning( __METHOD__ . ": Failed connecting to $i/$domain" );
                                unset( $currentLoads[$i] ); // avoid this server next iteration
@@ -1919,13 +1918,8 @@ class LoadBalancer implements ILoadBalancer {
                }
 
                if ( $this->hasStreamingReplicaServers() ) {
-                       try {
-                               // Set "laggedReplicaMode"
-                               $this->getReaderIndex( self::GROUP_GENERIC, $domain );
-                       } catch ( DBConnectionError $e ) {
-                               // Sanity: avoid expensive re-connect attempts and failures
-                               $this->laggedReplicaMode = true;
-                       }
+                       // This will set "laggedReplicaMode" as needed
+                       $this->getReaderIndex( self::GROUP_GENERIC, $domain );
                }
 
                return $this->laggedReplicaMode;
index d59e1c8..e17b393 100644 (file)
@@ -143,15 +143,15 @@ class ResourceLoaderClientHtml {
 
                        $group = $module->getGroup();
                        $context = $this->getContext( $group, ResourceLoaderModule::TYPE_COMBINED );
-                       if ( $module->isKnownEmpty( $context ) ) {
-                               // Avoid needless request or embed for empty module
-                               $data['states'][$name] = 'ready';
-                               continue;
-                       }
+                       $shouldEmbed = $module->shouldEmbedModule( $this->context );
 
-                       if ( $group === 'user' || $module->shouldEmbedModule( $this->context ) ) {
-                               // Call makeLoad() to decide how to load these, instead of
-                               // loading via mw.loader.load().
+                       if ( ( $group === 'user' || $shouldEmbed ) && $module->isKnownEmpty( $context ) ) {
+                               // This is a user-specific or embedded module, which means its output
+                               // can be specific to the current page or user. As such, we can optimise
+                               // the way we load it based on the current version of the module.
+                               // Avoid needless embed for empty module, preset ready state.
+                               $data['states'][$name] = 'ready';
+                       } elseif ( $group === 'user' || $shouldEmbed ) {
                                // - For group=user: We need to provide a pre-generated load.php
                                //   url to the client that has the 'user' and 'version' parameters
                                //   filled in. Without this, the client would wrongly use the static
@@ -187,15 +187,30 @@ class ResourceLoaderClientHtml {
 
                        $group = $module->getGroup();
                        $context = $this->getContext( $group, ResourceLoaderModule::TYPE_STYLES );
-                       // Avoid needless request for empty module
-                       if ( !$module->isKnownEmpty( $context ) ) {
-                               if ( $module->shouldEmbedModule( $this->context ) ) {
-                                       // Embed via style element
+                       if ( $module->shouldEmbedModule( $this->context ) ) {
+                               // Avoid needless embed for private embeds we know are empty.
+                               // (Set "ready" state directly instead, which we do a few lines above.)
+                               if ( !$module->isKnownEmpty( $context ) ) {
+                                       // Embed via <style> element
                                        $data['embed']['styles'][] = $name;
-                               } else {
-                                       // Load from load.php?only=styles via <link rel=stylesheet>
-                                       $data['styles'][] = $name;
                                }
+                       // For other style modules, always request them, regardless of whether they are
+                       // currently known to be empty. Because:
+                       // 1. Those modules are requested in batch, so there is no extra request overhead
+                       //    or extra HTML element to be avoided.
+                       // 2. Checking isKnownEmpty for those can be expensive and slow down page view
+                       //    generation (T230260).
+                       // 3. We don't want cached HTML to vary on the current state of a module.
+                       //    If the module becomes non-empty a few minutes later, it should start working
+                       //    on cached HTML without requiring a purge.
+                       //
+                       // But, user-specific modules:
+                       // * ... are used on page views not publicly cached.
+                       // * ... are in their own group and thus a require a request we can avoid
+                       // * ... have known-empty status preloaded by ResourceLoader.
+                       } elseif ( $group !== 'user' || !$module->isKnownEmpty( $context ) ) {
+                               // Load from load.php?only=styles via <link rel=stylesheet>
+                               $data['styles'][] = $name;
                        }
                        $deprecation = $module->getDeprecationInformation();
                        if ( $deprecation ) {
index f3ae31a..77c0710 100644 (file)
@@ -99,14 +99,20 @@ class SpecialMute extends FormSpecialPage {
         * @return bool
         */
        public function onSubmit( array $data, HTMLForm $form = null ) {
+               $hookData = [];
                foreach ( $data as $userOption => $value ) {
+                       $hookData[$userOption]['before'] = $this->isTargetBlacklisted( $userOption );
                        if ( $value ) {
                                $this->muteTarget( $userOption );
                        } else {
                                $this->unmuteTarget( $userOption );
                        }
+                       $hookData[$userOption]['after'] = (bool)$value;
                }
 
+               // NOTE: this hook is temporary
+               Hooks::run( 'SpecialMuteSubmit', [ $hookData ] );
+
                return true;
        }
 
index 6fa911b..53edbf2 100644 (file)
@@ -26,6 +26,7 @@ class DatabaseSqliteTest extends \MediaWikiIntegrationTestCase {
                $this->db = $this->getMockBuilder( DatabaseSqlite::class )
                        ->setConstructorArgs( [ [
                                'dbFilePath' => ':memory:',
+                               'dbname' => 'Foo',
                                'schema' => false,
                                'host' => false,
                                'user' => false,