Merge "WikiPage: Return false from hasDifferencesOutsideMainSlot if no differences...
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Wed, 18 Jul 2018 19:20:08 +0000 (19:20 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Wed, 18 Jul 2018 19:20:08 +0000 (19:20 +0000)
includes/Category.php
includes/deferred/LinksDeletionUpdate.php
includes/htmlform/fields/HTMLTitleTextField.php
includes/htmlform/fields/HTMLUserTextField.php
includes/libs/MapCacheLRU.php
includes/libs/objectcache/WANObjectCache.php
includes/libs/rdbms/database/Database.php
includes/logging/LogEventsList.php
includes/page/WikiPage.php
includes/specials/SpecialEditTags.php
tests/phpunit/includes/libs/MapCacheLRUTest.php

index f8ac8ae..3352c2c 100644 (file)
@@ -420,4 +420,45 @@ class Category {
 
                return true;
        }
+
+       /**
+        * Call refreshCounts() if there are no entries in the categorylinks table
+        * or if the category table has a row that states that there are no entries
+        *
+        * Due to lock errors or other failures, the precomputed counts can get out of sync,
+        * making it hard to know when to delete the category row without checking the
+        * categorylinks table.
+        *
+        * @return bool Whether links were refreshed
+        * @since 1.32
+        */
+       public function refreshCountsIfEmpty() {
+               $dbw = wfGetDB( DB_MASTER );
+
+               $hasLink = $dbw->selectField(
+                       'categorylinks',
+                       '1',
+                       [ 'cl_to' => $this->getName() ],
+                       __METHOD__
+               );
+               if ( !$hasLink ) {
+                       $this->refreshCounts(); // delete any category table entry
+
+                       return true;
+               }
+
+               $hasBadRow = $dbw->selectField(
+                       'category',
+                       '1',
+                       [ 'cat_title' => $this->getName(), 'cat_pages <= 0' ],
+                       __METHOD__
+               );
+               if ( $hasBadRow ) {
+                       $this->refreshCounts(); // clean up this row
+
+                       return true;
+               }
+
+               return false;
+       }
 }
index 9f6257c..f370e43 100644 (file)
@@ -101,7 +101,8 @@ class LinksDeletionUpdate extends DataUpdate implements EnqueueableDataUpdate {
                if ( $title->getNamespace() === NS_CATEGORY ) {
                        // T166757: do the update after the main job DB commit
                        DeferredUpdates::addCallableUpdate( function () use ( $title ) {
-                               $this->refreshCategoryIfEmpty( $title );
+                               $cat = Category::newFromName( $title->getDBkey() );
+                               $cat->refreshCountsIfEmpty();
                        } );
                }
 
@@ -187,35 +188,6 @@ class LinksDeletionUpdate extends DataUpdate implements EnqueueableDataUpdate {
                ScopedCallback::consume( $scopedLock );
        }
 
-       /**
-        * @param Title $title
-        */
-       private function refreshCategoryIfEmpty( Title $title ) {
-               $dbw = $this->getDB();
-
-               $row = $dbw->selectRow(
-                       'category',
-                       [ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ],
-                       [ 'cat_title' => $title->getDBkey(), 'cat_pages <= 100' ],
-                       __METHOD__
-               );
-
-               if ( !$row ) {
-                       return; // nothing to delete
-               }
-
-               $cat = Category::newFromRow( $row, $title );
-               $hasLink = $dbw->selectField(
-                       'categorylinks',
-                       '1',
-                       [ 'cl_to' => $title->getDBkey() ],
-                       __METHOD__
-               );
-               if ( !$hasLink ) {
-                       $cat->refreshCounts(); // delete the category table entry
-               }
-       }
-
        private function batchDeleteByPK( $table, array $conds, array $pk, $bSize ) {
                $services = MediaWikiServices::getInstance();
                $lbFactory = $services->getDBLoadBalancerFactory();
index 602ddee..0ad41d4 100644 (file)
@@ -41,6 +41,11 @@ class HTMLTitleTextField extends HTMLTextField {
                        return parent::validate( $value, $alldata );
                }
 
+               // Default value (from getDefault()) is null, which breaks Title::newFromTextThrow() below
+               if ( $value === null ) {
+                       $value = '';
+               }
+
                if ( !$this->mParams['required'] && $value === '' ) {
                        // If this field is not required and the value is empty, that's okay, skip validation
                        return parent::validate( $value, $alldata );
index e193970..d672314 100644 (file)
@@ -34,6 +34,11 @@ class HTMLUserTextField extends HTMLTextField {
        }
 
        public function validate( $value, $alldata ) {
+               // Default value (from getDefault()) is null, User::newFromName() expects a string
+               if ( $value === null ) {
+                       $value = '';
+               }
+
                // check, if a user exists with the given username
                $user = User::newFromName( $value, false );
                $rangeError = null;
@@ -43,7 +48,7 @@ class HTMLUserTextField extends HTMLTextField {
                } elseif (
                        // check, if the user exists, if requested
                        ( $this->mParams['exists'] && $user->getId() === 0 ) &&
-                       // check, if the username is a valid IP address, otherweise save the error message
+                       // check, if the username is a valid IP address, otherwise save the error message
                        !( $this->mParams['ipallowed'] && IP::isValid( $value ) ) &&
                        // check, if the username is a valid IP range, otherwise save the error message
                        !( $this->mParams['iprange'] && ( $rangeError = $this->isValidIPRange( $value ) ) === true )
index e891c9e..ad5e58d 100644 (file)
@@ -135,7 +135,7 @@ class MapCacheLRU implements IExpiringStore, Serializable {
         * Check if a key exists
         *
         * @param string $key
-        * @param float $maxAge Ignore items older than this many seconds (since 1.32)
+        * @param float $maxAge Ignore items older than this many seconds [optional] (since 1.32)
         * @return bool
         */
        public function has( $key, $maxAge = 0.0 ) {
@@ -157,10 +157,11 @@ class MapCacheLRU implements IExpiringStore, Serializable {
         * If the item is already set, it will be pushed to the top of the cache.
         *
         * @param string $key
-        * @return mixed Returns null if the key was not found
+        * @param float $maxAge Ignore items older than this many seconds [optional] (since 1.32)
+        * @return mixed Returns null if the key was not found or is older than $maxAge
         */
-       public function get( $key ) {
-               if ( !$this->has( $key ) ) {
+       public function get( $key, $maxAge = 0.0 ) {
+               if ( !$this->has( $key, $maxAge ) ) {
                        return null;
                }
 
@@ -193,7 +194,7 @@ class MapCacheLRU implements IExpiringStore, Serializable {
        /**
         * @param string|int $key
         * @param string|int $field
-        * @param float $maxAge
+        * @param float $maxAge Ignore items older than this many seconds [optional] (since 1.32)
         * @return bool
         */
        public function hasField( $key, $field, $maxAge = 0.0 ) {
@@ -205,8 +206,18 @@ class MapCacheLRU implements IExpiringStore, Serializable {
                return ( $maxAge <= 0 || $this->getAge( $key, $field ) <= $maxAge );
        }
 
-       public function getField( $key, $field ) {
-               return $this->get( $key )[$field] ?? null;
+       /**
+        * @param string|int $key
+        * @param string|int $field
+        * @param float $maxAge Ignore items older than this many seconds [optional] (since 1.32)
+        * @return mixed Returns null if the key was not found or is older than $maxAge
+        */
+       public function getField( $key, $field, $maxAge = 0.0 ) {
+               if ( !$this->hasField( $key, $field, $maxAge ) ) {
+                       return null;
+               }
+
+               return $this->cache[$key][$field];
        }
 
        /**
index e30e061..2989c81 100644 (file)
@@ -87,7 +87,7 @@ use Psr\Log\NullLogger;
 class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
        /** @var BagOStuff The local datacenter cache */
        protected $cache;
-       /** @var HashBagOStuff[] Map of group PHP instance caches */
+       /** @var MapCacheLRU[] Map of group PHP instance caches */
        protected $processCaches = [];
        /** @var string Purge channel name */
        protected $purgeChannel;
@@ -1061,7 +1061,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                if ( $pcTTL >= 0 && $this->callbackDepth == 0 ) {
                        $group = $opts['pcGroup'] ?? self::PC_PRIMARY;
                        $procCache = $this->getProcessCache( $group );
-                       $value = $procCache->get( $key );
+                       $value = $procCache->has( $key, $pcTTL ) ? $procCache->get( $key ) : false;
                } else {
                        $procCache = false;
                        $value = false;
@@ -1117,7 +1117,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 
                        // Update the process cache if enabled
                        if ( $procCache && $value !== false ) {
-                               $procCache->set( $key, $value, $pcTTL );
+                               $procCache->set( $key, $value );
                        }
                }
 
@@ -1385,10 +1385,11 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
        ) {
                $valueKeys = array_keys( $keyedIds->getArrayCopy() );
                $checkKeys = $opts['checkKeys'] ?? [];
+               $pcTTL = $opts['pcTTL'] ?? self::TTL_UNCACHEABLE;
 
                // Load required keys into process cache in one go
                $this->warmupCache = $this->getRawKeysForWarmup(
-                       $this->getNonProcessCachedKeys( $valueKeys, $opts ),
+                       $this->getNonProcessCachedKeys( $valueKeys, $opts, $pcTTL ),
                        $checkKeys
                );
                $this->warmupKeyMisses = 0;
@@ -1480,11 +1481,12 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                $idsByValueKey = $keyedIds->getArrayCopy();
                $valueKeys = array_keys( $idsByValueKey );
                $checkKeys = $opts['checkKeys'] ?? [];
+               $pcTTL = $opts['pcTTL'] ?? self::TTL_UNCACHEABLE;
                unset( $opts['lockTSE'] ); // incompatible
                unset( $opts['busyValue'] ); // incompatible
 
                // Load required keys into process cache in one go
-               $keysGet = $this->getNonProcessCachedKeys( $valueKeys, $opts );
+               $keysGet = $this->getNonProcessCachedKeys( $valueKeys, $opts, $pcTTL );
                $this->warmupCache = $this->getRawKeysForWarmup( $keysGet, $checkKeys );
                $this->warmupKeyMisses = 0;
 
@@ -2103,12 +2105,12 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 
        /**
         * @param string $group
-        * @return HashBagOStuff
+        * @return MapCacheLRU
         */
        protected function getProcessCache( $group ) {
                if ( !isset( $this->processCaches[$group] ) ) {
                        list( , $n ) = explode( ':', $group );
-                       $this->processCaches[$group] = new HashBagOStuff( [ 'maxKeys' => (int)$n ] );
+                       $this->processCaches[$group] = new MapCacheLRU( (int)$n );
                }
 
                return $this->processCaches[$group];
@@ -2117,15 +2119,16 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
        /**
         * @param array $keys
         * @param array $opts
+        * @param int $pcTTL
         * @return array List of keys
         */
-       private function getNonProcessCachedKeys( array $keys, array $opts ) {
+       private function getNonProcessCachedKeys( array $keys, array $opts, $pcTTL ) {
                $keysFound = [];
                if ( isset( $opts['pcTTL'] ) && $opts['pcTTL'] > 0 && $this->callbackDepth == 0 ) {
                        $pcGroup = $opts['pcGroup'] ?? self::PC_PRIMARY;
                        $procCache = $this->getProcessCache( $pcGroup );
                        foreach ( $keys as $key ) {
-                               if ( $procCache->get( $key ) !== false ) {
+                               if ( $procCache->has( $key, $pcTTL ) ) {
                                        $keysFound[] = $key;
                                }
                        }
index d11b51b..dc3260d 100644 (file)
@@ -3844,9 +3844,11 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->assertOpen();
 
                $this->runOnTransactionPreCommitCallbacks();
+
                $writeTime = $this->pendingWriteQueryDuration( self::ESTIMATE_DB_APPLY );
                $this->doCommit( $fname );
                $this->trxStatus = self::STATUS_TRX_NONE;
+
                if ( $this->trxDoneWrites ) {
                        $this->lastWriteTime = microtime( true );
                        $this->trxProfiler->transactionWritingOut(
@@ -3894,14 +3896,18 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        // Avoid fatals if close() was called
                        $this->assertOpen();
 
+                       $writeTime = $this->pendingWriteQueryDuration( self::ESTIMATE_DB_APPLY );
                        $this->doRollback( $fname );
                        $this->trxStatus = self::STATUS_TRX_NONE;
                        $this->trxAtomicLevels = [];
+
                        if ( $this->trxDoneWrites ) {
                                $this->trxProfiler->transactionWritingOut(
                                        $this->server,
                                        $this->dbName,
-                                       $this->trxShortId
+                                       $this->trxShortId,
+                                       $writeTime,
+                                       $this->trxWriteAffectedRows
                                );
                        }
                }
index a7620fe..aecbce9 100644 (file)
@@ -173,6 +173,7 @@ class LogEventsList extends ContextSource {
                $htmlForm = new HTMLForm( $formDescriptor, $this->getContext() );
                $htmlForm
                        ->setSubmitText( $this->msg( 'logeventslist-submit' )->text() )
+                       ->setMethod( 'get' )
                        ->setWrapperLegendMsg( 'log' );
 
                // TODO This will should be removed at some point. See T199495.
index 424f6b7..a1b2e57 100644 (file)
@@ -3343,23 +3343,10 @@ class WikiPage implements Page, IDBAccessObject {
                foreach ( $deleted as $catName ) {
                        $cat = Category::newFromName( $catName );
                        Hooks::run( 'CategoryAfterPageRemoved', [ $cat, $this, $id ] );
-               }
-
-               // Refresh counts on categories that should be empty now
-               if ( count( $deleted ) ) {
-                       $rows = $dbw->select(
-                               'category',
-                               [ 'cat_id', 'cat_title', 'cat_pages', 'cat_subcats', 'cat_files' ],
-                               [ 'cat_title' => $deleted, 'cat_pages <= 100' ],
-                               __METHOD__
-                       );
-                       foreach ( $rows as $row ) {
-                               $cat = Category::newFromRow( $row );
-                               // T166757: do the update after this DB commit
-                               DeferredUpdates::addCallableUpdate( function () use ( $cat ) {
-                                       $cat->refreshCounts();
-                               } );
-                       }
+                       // Refresh counts on categories that should be empty now (after commit, T166757)
+                       DeferredUpdates::addCallableUpdate( function () use ( $cat ) {
+                               $cat->refreshCountsIfEmpty();
+                       } );
                }
        }
 
index 3db7eda..b657335 100644 (file)
@@ -166,7 +166,7 @@ class SpecialEditTags extends UnlistedSpecialPage {
                                [],
                                [
                                        'page' => $this->targetObj->getPrefixedText(),
-                                       'hide_tag_log' => '0',
+                                       'wpfilters' => [ 'tag' ],
                                ]
                        );
                        if ( !$this->targetObj->isSpecialPage() ) {
index 79f7b96..a06ef62 100644 (file)
@@ -158,10 +158,12 @@ class MapCacheLRUTest extends PHPUnit\Framework\TestCase {
                $now += 29;
                $this->assertTrue( $cache->has( 'd', 30 ) );
                $this->assertEquals( 'xxx', $cache->get( 'd' ) );
+               $this->assertEquals( 'xxx', $cache->get( 'd', 30 ) );
 
                $now += 1.5;
                $this->assertFalse( $cache->has( 'd', 30 ) );
                $this->assertEquals( 'xxx', $cache->get( 'd' ) );
+               $this->assertNull( $cache->get( 'd', 30 ) );
        }
 
        /**
@@ -180,14 +182,17 @@ class MapCacheLRUTest extends PHPUnit\Framework\TestCase {
                $cache->setField( 'PMs', 'Margaret Thatcher', 'Tory' );
                $this->assertTrue( $cache->hasField( 'PMs', 'Tony Blair', 30 ) );
                $this->assertEquals( 'Labour', $cache->getField( 'PMs', 'Tony Blair' ) );
+               $this->assertTrue( $cache->hasField( 'PMs', 'Tony Blair', 30 ) );
 
                $now += 29;
                $this->assertTrue( $cache->hasField( 'PMs', 'Tony Blair', 30 ) );
                $this->assertEquals( 'Labour', $cache->getField( 'PMs', 'Tony Blair' ) );
+               $this->assertEquals( 'Labour', $cache->getField( 'PMs', 'Tony Blair', 30 ) );
 
                $now += 1.5;
                $this->assertFalse( $cache->hasField( 'PMs', 'Tony Blair', 30 ) );
                $this->assertEquals( 'Labour', $cache->getField( 'PMs', 'Tony Blair' ) );
+               $this->assertNull( $cache->getField( 'PMs', 'Tony Blair', 30 ) );
 
                $this->assertEquals(
                        [ 'Tony Blair' => 'Labour', 'Margaret Thatcher' => 'Tory' ],