Merge "Allow titles with falsy title text in suggestions"

diff --git a/includes/WatchedItem.php b/includes/WatchedItem.php
index 5b4a4fc..0495536 100644 (file)
--- a/includes/WatchedItem.php
+++ b/includes/WatchedItem.php
@@ -179,23 +179,31 @@ class WatchedItem {
         */
        public static function batchAddWatch( array $items ) {
                // wfDeprecated( __METHOD__, '1.27' );
-               $userTargetCombinations = [];
+               if ( !$items ) {
+                       return false;
+               }
+
+               $targets = [];
+               $users = [];
                /** @var WatchedItem $watchedItem */
                foreach ( $items as $watchedItem ) {
-                       if ( $watchedItem->checkRights && !$watchedItem->getUser()->isAllowed( 'editmywatchlist' ) ) {
+                       $user = $watchedItem->getUser();
+                       if ( $watchedItem->checkRights && !$user->isAllowed( 'editmywatchlist' ) ) {
                                continue;
                        }
-                       $userTargetCombinations[] = [
-                               $watchedItem->getUser(),
-                               $watchedItem->getTitle()->getSubjectPage()
-                       ];
-                       $userTargetCombinations[] = [
-                               $watchedItem->getUser(),
-                               $watchedItem->getTitle()->getTalkPage()
-                       ];
+                       $userId = $user->getId();
+                       $users[$userId] = $user;
+                       $targets[$userId][] = $watchedItem->getTitle()->getSubjectPage();
+                       $targets[$userId][] = $watchedItem->getTitle()->getTalkPage();
                }
+
                $store = WatchedItemStore::getDefaultInstance();
-               return $store->addWatchBatch( $userTargetCombinations );
+               $success = true;
+               foreach ( $users as $userId => $user ) {
+                       $success &= $store->addWatchBatchForUser( $user, $targets[$userId] );
+               }
+
+               return $success;
        }
 
        /**

diff --git a/includes/WatchedItemStore.php b/includes/WatchedItemStore.php
index 4eea54d..c4340ad 100644 (file)
--- a/includes/WatchedItemStore.php
+++ b/includes/WatchedItemStore.php
@@ -615,30 +615,30 @@ class WatchedItemStore {
         * @param LinkTarget $target
         */
        public function addWatch( User $user, LinkTarget $target ) {
-               $this->addWatchBatch( [ [ $user, $target ] ] );
+               $this->addWatchBatchForUser( $user, [ $target ] );
        }
 
        /**
-        * @param array[] $userTargetCombinations array of arrays containing [0] => User [1] => LinkTarget
+        * @param User $user
+        * @param LinkTarget[] $targets
         *
         * @return bool success
         */
-       public function addWatchBatch( array $userTargetCombinations ) {
+       public function addWatchBatchForUser( User $user, array $targets ) {
                if ( $this->loadBalancer->getReadOnlyReason() !== false ) {
                        return false;
                }
+               // Only loggedin user can have a watchlist
+               if ( $user->isAnon() ) {
+                       return false;
+               }
+
+               if ( !$targets ) {
+                       return true;
+               }
 
                $rows = [];
-               foreach ( $userTargetCombinations as list( $user, $target ) ) {
-                       /**
-                        * @var User $user
-                        * @var LinkTarget $target
-                        */
-
-                       // Only loggedin user can have a watchlist
-                       if ( $user->isAnon() ) {
-                               continue;
-                       }
+               foreach ( $targets as $target ) {
                        $rows[] = [
                                'wl_user' => $user->getId(),
                                'wl_namespace' => $target->getNamespace(),
@@ -648,10 +648,6 @@ class WatchedItemStore {
                        $this->uncache( $user, $target );
                }
 
-               if ( !$rows ) {
-                       return false;
-               }
-
                $dbw = $this->getConnection( DB_MASTER );
                foreach ( array_chunk( $rows, 100 ) as $toInsert ) {
                        // Use INSERT IGNORE to avoid overwriting the notification timestamp

diff --git a/includes/media/DjVu.php b/includes/media/DjVu.php
index cd249a8..9add138 100644 (file)
--- a/includes/media/DjVu.php
+++ b/includes/media/DjVu.php
@@ -393,25 +393,24 @@ class DjVuHandler extends ImageHandler {
        }
 
        protected function getDimensionInfo( File $file ) {
-               $that = $this;
-
-               return ObjectCache::getMainWANInstance()->getWithSetCallback(
-                       wfMemcKey( 'file-djvu', 'dimensions', $file->getSha1() ),
-                       WANObjectCache::TTL_INDEFINITE,
-                       function () use ( $that, $file ) {
-                               $tree = $that->getMetaTree( $file );
+               $cache = ObjectCache::getMainWANInstance();
+               return $cache->getWithSetCallback(
+                       $cache->makeKey( 'file-djvu', 'dimensions', $file->getSha1() ),
+                       $cache::TTL_INDEFINITE,
+                       function () use ( $file ) {
+                               $tree = $this->getMetaTree( $file );
                                if ( !$tree ) {
                                        return false;
                                }
 
                                $dimsByPage = [];
                                $count = count( $tree->xpath( '//OBJECT' ) );
-                               for ( $i = 0; $i < $count; ++$i ) {
+                               for ( $i = 0; $i < $count; $i++ ) {
                                        $o = $tree->BODY[0]->OBJECT[$i];
                                        if ( $o ) {
                                                $dimsByPage[$i] = [
                                                        'width' => (int)$o['width'],
-                                                       'height' => (int)$o['height']
+                                                       'height' => (int)$o['height'],
                                                ];
                                        } else {
                                                $dimsByPage[$i] = false;
@@ -420,7 +419,7 @@ class DjVuHandler extends ImageHandler {
 
                                return [ 'pageCount' => $count, 'dimensionsByPage' => $dimsByPage ];
                        },
-                       [ 'pcTTL' => WANObjectCache::TTL_INDEFINITE ]
+                       [ 'pcTTL' => $cache::TTL_INDEFINITE ]
                );
        }
 

diff --git a/includes/page/Article.php b/includes/page/Article.php
index d57d3fd..7592017 100644 (file)
--- a/includes/page/Article.php
+++ b/includes/page/Article.php
@@ -1644,6 +1644,7 @@ class Article implements Page {
                $title = $this->getTitle();
                $context = $this->getContext();
                $user = $context->getUser();
+               $request = $context->getRequest();
 
                # Check permissions
                $permissionErrors = $title->getUserPermissionsErrors( 'delete', $user );
@@ -1657,7 +1658,9 @@ class Article implements Page {
                }
 
                # Better double-check that it hasn't been deleted yet!
-               $this->mPage->loadPageData( 'fromdbmaster' );
+               $this->mPage->loadPageData(
+                       $request->wasPosted() ? WikiPage::READ_LATEST : WikiPage::READ_NORMAL
+               );
                if ( !$this->mPage->exists() ) {
                        $deleteLogPage = new LogPage( 'delete' );
                        $outputPage = $context->getOutput();
@@ -1677,7 +1680,6 @@ class Article implements Page {
                        return;
                }
 
-               $request = $context->getRequest();
                $deleteReasonList = $request->getText( 'wpDeleteReasonList', 'other' );
                $deleteReason = $request->getText( 'wpReason' );
 

diff --git a/includes/specials/SpecialEditWatchlist.php b/includes/specials/SpecialEditWatchlist.php
index a9753ae..c1abd6e 100644 (file)
--- a/includes/specials/SpecialEditWatchlist.php
+++ b/includes/specials/SpecialEditWatchlist.php
@@ -49,10 +49,26 @@ class SpecialEditWatchlist extends UnlistedSpecialPage {
 
        private $badItems = [];
 
+       /**
+        * @var TitleParser
+        */
+       private $titleParser;
+
        public function __construct() {
                parent::__construct( 'EditWatchlist', 'editmywatchlist' );
        }
 
+       /**
+        * Initialize any services we'll need (unless it has already been provided via a setter).
+        * This allows for dependency injection even though we don't control object creation.
+        */
+       private function initServices() {
+               if ( !$this->titleParser ) {
+                       $lang = $this->getContext()->getLanguage();
+                       $this->titleParser = new MediaWikiTitleCodec( $lang, GenderCache::singleton() );
+               }
+       }
+
        public function doesWrites() {
                return true;
        }
@@ -63,6 +79,7 @@ class SpecialEditWatchlist extends UnlistedSpecialPage {
         * @param int $mode
         */
        public function execute( $mode ) {
+               $this->initServices();
                $this->setHeaders();
 
                # Anons don't get a watchlist
@@ -432,39 +449,32 @@ class SpecialEditWatchlist extends UnlistedSpecialPage {
        }
 
        /**
-        * Add a list of titles to a user's watchlist
-        *
-        * $titles can be an array of strings or Title objects; the former
-        * is preferred, since Titles are very memory-heavy
+        * Add a list of targets to a user's watchlist
         *
-        * @param array $titles Array of strings, or Title objects
+        * @param string[]|LinkTarget[] $targets
         */
-       private function watchTitles( $titles ) {
-               $dbw = wfGetDB( DB_MASTER );
-               $rows = [];
-
-               foreach ( $titles as $title ) {
-                       if ( !$title instanceof Title ) {
-                               $title = Title::newFromText( $title );
+       private function watchTitles( $targets ) {
+               $expandedTargets = [];
+               foreach ( $targets as $target ) {
+                       if ( !$target instanceof LinkTarget ) {
+                               try {
+                                       $target = $this->titleParser->parseTitle( $target, NS_MAIN );
+                               }
+                               catch ( MalformedTitleException $e ) {
+                                       continue;
+                               }
                        }
 
-                       if ( $title instanceof Title ) {
-                               $rows[] = [
-                                       'wl_user' => $this->getUser()->getId(),
-                                       'wl_namespace' => MWNamespace::getSubject( $title->getNamespace() ),
-                                       'wl_title' => $title->getDBkey(),
-                                       'wl_notificationtimestamp' => null,
-                               ];
-                               $rows[] = [
-                                       'wl_user' => $this->getUser()->getId(),
-                                       'wl_namespace' => MWNamespace::getTalk( $title->getNamespace() ),
-                                       'wl_title' => $title->getDBkey(),
-                                       'wl_notificationtimestamp' => null,
-                               ];
-                       }
+                       $ns = $target->getNamespace();
+                       $dbKey = $target->getDBkey();
+                       $expandedTargets[] = new TitleValue( MWNamespace::getSubject( $ns ), $dbKey );
+                       $expandedTargets[] = new TitleValue( MWNamespace::getTalk( $ns ), $dbKey );
                }
 
-               $dbw->insert( 'watchlist', $rows, __METHOD__, 'IGNORE' );
+               WatchedItemStore::getDefaultInstance()->addWatchBatchForUser(
+                       $this->getUser(),
+                       $expandedTargets
+               );
        }
 
        /**

diff --git a/includes/user/User.php b/includes/user/User.php
index 027edf9..a272b37 100644 (file)
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -3467,10 +3467,9 @@ class User implements IDBAccessObject {
         */
        public function addWatch( $title, $checkRights = self::CHECK_USER_RIGHTS ) {
                if ( !$checkRights || $this->isAllowed( 'editmywatchlist' ) ) {
-                       WatchedItemStore::getDefaultInstance()->addWatchBatch( [
-                               [ $this, $title->getSubjectPage() ],
-                               [ $this, $title->getTalkPage() ],
-                       ]
+                       WatchedItemStore::getDefaultInstance()->addWatchBatchForUser(
+                               $this,
+                               [ $title->getSubjectPage(), $title->getTalkPage() ]
                        );
                }
                $this->invalidateCache();

diff --git a/tests/phpunit/includes/WatchedItemStoreUnitTest.php b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
index afde88c..5f07dbf 100644 (file)
--- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
@@ -934,7 +934,7 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
                );
        }
 
-       public function testAddWatchBatch_nonAnonymousUser() {
+       public function testAddWatchBatchForUser_nonAnonymousUser() {
                $mockDb = $this->getMockDb();
                $mockDb->expects( $this->once() )
                        ->method( 'insert' )
@@ -974,52 +974,14 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
                $mockUser = $this->getMockNonAnonUserWithId( 1 );
 
                $this->assertTrue(
-                       $store->addWatchBatch(
-                               [
-                                       [ $mockUser, new TitleValue( 0, 'Some_Page' ) ],
-                                       [ $mockUser, new TitleValue( 1, 'Some_Page' ) ],
-                               ]
-                       )
-               );
-       }
-
-       public function testAddWatchBatch_anonymousUserCombinationsAreSkipped() {
-               $mockDb = $this->getMockDb();
-               $mockDb->expects( $this->once() )
-                       ->method( 'insert' )
-                       ->with(
-                               'watchlist',
-                               [
-                                       [
-                                               'wl_user' => 1,
-                                               'wl_namespace' => 0,
-                                               'wl_title' => 'Some_Page',
-                                               'wl_notificationtimestamp' => null,
-                                       ]
-                               ]
-                       );
-
-               $mockCache = $this->getMockCache();
-               $mockCache->expects( $this->once() )
-                       ->method( 'delete' )
-                       ->with( '0:Some_Page:1' );
-
-               $store = new WatchedItemStore(
-                       $this->getMockLoadBalancer( $mockDb ),
-                       $mockCache
-               );
-
-               $this->assertTrue(
-                       $store->addWatchBatch(
-                               [
-                                       [ $this->getMockNonAnonUserWithId( 1 ), new TitleValue( 0, 'Some_Page' ) ],
-                                       [ $this->getAnonUser(), new TitleValue( 0, 'Other_Page' ) ],
-                               ]
+                       $store->addWatchBatchForUser(
+                               $mockUser,
+                               [ new TitleValue( 0, 'Some_Page' ), new TitleValue( 1, 'Some_Page' ) ]
                        )
                );
        }
 
-       public function testAddWatchBatchReturnsFalse_whenOnlyGivenAnonymousUserCombinations() {
+       public function testAddWatchBatchForUser_anonymousUsersAreSkipped() {
                $mockDb = $this->getMockDb();
                $mockDb->expects( $this->never() )
                        ->method( 'insert' );
@@ -1033,18 +995,16 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
                        $mockCache
                );
 
-               $anonUser = $this->getAnonUser();
                $this->assertFalse(
-                       $store->addWatchBatch(
-                               [
-                                       [ $anonUser, new TitleValue( 0, 'Some_Page' ) ],
-                                       [ $anonUser, new TitleValue( 1, 'Other_Page' ) ],
-                               ]
+                       $store->addWatchBatchForUser(
+                               $this->getAnonUser(),
+                               [ new TitleValue( 0, 'Other_Page' ) ]
                        )
                );
        }
 
-       public function testAddWatchBatchReturnsFalse_whenGivenEmptyList() {
+       public function testAddWatchBatchReturnsTrue_whenGivenEmptyList() {
+               $user = $this->getMockNonAnonUserWithId( 1 );
                $mockDb = $this->getMockDb();
                $mockDb->expects( $this->never() )
                        ->method( 'insert' );
@@ -1058,8 +1018,8 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
                        $mockCache
                );
 
-               $this->assertFalse(
-                       $store->addWatchBatch( [] )
+               $this->assertTrue(
+                       $store->addWatchBatchForUser( $user, [] )
                );
        }
 

diff --git a/tests/phpunit/includes/WatchedItemUnitTest.php b/tests/phpunit/includes/WatchedItemUnitTest.php
index 58984cf..b4eaa76 100644 (file)
--- a/tests/phpunit/includes/WatchedItemUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemUnitTest.php
@@ -165,25 +165,37 @@ class WatchedItemUnitTest extends PHPUnit_Framework_TestCase {
        }
 
        public function testBatchAddWatch() {
-               /** @var WatchedItem[] $items */
-               $items = [
-                       new WatchedItem( User::newFromId( 1 ), new TitleValue( 0, 'Title1' ), null ),
-                       new WatchedItem( User::newFromId( 3 ), Title::newFromText( 'Title2' ), '20150101010101' ),
-               ];
-
-               $userTargetCombinations = [];
-               foreach ( $items as $item ) {
-                       $userTargetCombinations[] = [ $item->getUser(), $item->getTitle()->getSubjectPage() ];
-                       $userTargetCombinations[] = [ $item->getUser(), $item->getTitle()->getTalkPage() ];
-               }
+               $itemOne = new WatchedItem( User::newFromId( 1 ), new TitleValue( 0, 'Title1' ), null );
+               $itemTwo = new WatchedItem(
+                       User::newFromId( 3 ),
+                       Title::newFromText( 'Title2' ),
+                       '20150101010101'
+               );
 
                $store = $this->getMockWatchedItemStore();
-               $store->expects( $this->once() )
-                       ->method( 'addWatchBatch' )
-                       ->with( $userTargetCombinations );
+               $store->expects( $this->exactly( 2 ) )
+                       ->method( 'addWatchBatchForUser' );
+               $store->expects( $this->at( 0 ) )
+                       ->method( 'addWatchBatchForUser' )
+                       ->with(
+                               $itemOne->getUser(),
+                               [
+                                       $itemOne->getTitle()->getSubjectPage(),
+                                       $itemOne->getTitle()->getTalkPage(),
+                               ]
+                       );
+               $store->expects( $this->at( 1 ) )
+                       ->method( 'addWatchBatchForUser' )
+                       ->with(
+                               $itemTwo->getUser(),
+                               [
+                                       $itemTwo->getTitle()->getSubjectPage(),
+                                       $itemTwo->getTitle()->getTalkPage(),
+                               ]
+                       );
                $scopedOverride = WatchedItemStore::overrideDefaultInstance( $store );
 
-               WatchedItem::batchAddWatch( $items );
+               WatchedItem::batchAddWatch( [ $itemOne, $itemTwo ] );
 
                ScopedCallback::consume( $scopedOverride );
        }