From b3e4a9c456ef7b04c745923238fca895f70907bc Mon Sep 17 00:00:00 2001 From: Aryeh Gregor Date: Sun, 5 May 2019 16:45:19 +0300 Subject: [PATCH] Use RepoGroup service in MovePage Change-Id: I2b59de374d49652b61f074a9da5483c0d991e7a6 --- includes/MovePage.php | 19 ++++++++++++------ includes/ServiceWiring.php | 3 ++- includes/page/MovePageFactory.php | 10 ++++++++-- tests/phpunit/includes/MovePageTest.php | 26 +++++++++++++++++++++---- 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/includes/MovePage.php b/includes/MovePage.php index f48d92e0a2..578c96d177 100644 --- a/includes/MovePage.php +++ b/includes/MovePage.php @@ -70,6 +70,11 @@ class MovePage { */ protected $permMgr; + /** + * @var RepoGroup + */ + protected $repoGroup; + /** * Calling this directly is deprecated in 1.34. Use MovePageFactory instead. * @@ -88,7 +93,8 @@ class MovePage { LoadBalancer $loadBalancer = null, NamespaceInfo $nsInfo = null, WatchedItemStore $watchedItems = null, - PermissionManager $permMgr = null + PermissionManager $permMgr = null, + RepoGroup $repoGroup = null ) { $this->oldTitle = $oldTitle; $this->newTitle = $newTitle; @@ -101,6 +107,7 @@ class MovePage { $this->watchedItems = $watchedItems ?? MediaWikiServices::getInstance()->getWatchedItemStore(); $this->permMgr = $permMgr ?? MediaWikiServices::getInstance()->getPermissionManager(); + $this->repoGroup = $repoGroup ?? MediaWikiServices::getInstance()->getRepoGroup(); } /** @@ -230,7 +237,7 @@ class MovePage { */ protected function isValidFileMove() { $status = new Status(); - $file = wfLocalFile( $this->oldTitle ); + $file = $this->repoGroup->getLocalRepo()->newFile( $this->oldTitle ); $file->load( File::READ_LATEST ); if ( $file->exists() ) { if ( $this->newTitle->getText() != wfStripIllegalFilenameChars( $this->newTitle->getText() ) ) { @@ -258,7 +265,7 @@ class MovePage { protected function isValidMoveTarget() { # Is it an existing file? if ( $this->newTitle->inNamespace( NS_FILE ) ) { - $file = wfLocalFile( $this->newTitle ); + $file = $this->repoGroup->getLocalRepo()->newFile( $this->newTitle ); $file->load( File::READ_LATEST ); if ( $file->exists() ) { wfDebug( __METHOD__ . ": file exists\n" ); @@ -679,15 +686,15 @@ class MovePage { $oldTitle->getPrefixedText() ); - $file = wfLocalFile( $oldTitle ); + $file = $this->repoGroup->getLocalRepo()->newFile( $oldTitle ); $file->load( File::READ_LATEST ); if ( $file->exists() ) { $status = $file->move( $newTitle ); } // Clear RepoGroup process cache - RepoGroup::singleton()->clearCache( $oldTitle ); - RepoGroup::singleton()->clearCache( $newTitle ); # clear false negative cache + $this->repoGroup->clearCache( $oldTitle ); + $this->repoGroup->clearCache( $newTitle ); # clear false negative cache return $status; } diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index fb44722c11..bb6c2c26ef 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -398,7 +398,8 @@ return [ $services->getDBLoadBalancer(), $services->getNamespaceInfo(), $services->getWatchedItemStore(), - $services->getPermissionManager() + $services->getPermissionManager(), + $services->getRepoGroup() ); }, diff --git a/includes/page/MovePageFactory.php b/includes/page/MovePageFactory.php index 144e75b560..26da151844 100644 --- a/includes/page/MovePageFactory.php +++ b/includes/page/MovePageFactory.php @@ -25,6 +25,7 @@ use MediaWiki\Config\ServiceOptions; use MediaWiki\Permissions\PermissionManager; use MovePage; use NamespaceInfo; +use RepoGroup; use Title; use WatchedItemStore; use Wikimedia\Rdbms\LoadBalancer; @@ -48,6 +49,9 @@ class MovePageFactory { /** @var PermissionManager */ private $permMgr; + /** @var RepoGroup */ + private $repoGroup; + /** * @todo Make this a const when we drop HHVM support (T192166) * @var array @@ -62,7 +66,8 @@ class MovePageFactory { LoadBalancer $loadBalancer, NamespaceInfo $nsInfo, WatchedItemStore $watchedItems, - PermissionManager $permMgr + PermissionManager $permMgr, + RepoGroup $repoGroup ) { $options->assertRequiredOptions( self::$constructorOptions ); @@ -71,6 +76,7 @@ class MovePageFactory { $this->nsInfo = $nsInfo; $this->watchedItems = $watchedItems; $this->permMgr = $permMgr; + $this->repoGroup = $repoGroup; } /** @@ -80,6 +86,6 @@ class MovePageFactory { */ public function newMovePage( Title $from, Title $to ) : MovePage { return new MovePage( $from, $to, $this->options, $this->loadBalancer, $this->nsInfo, - $this->watchedItems, $this->permMgr ); + $this->watchedItems, $this->permMgr, $this->repoGroup ); } } diff --git a/tests/phpunit/includes/MovePageTest.php b/tests/phpunit/includes/MovePageTest.php index 25cb8073a0..afce0925d5 100644 --- a/tests/phpunit/includes/MovePageTest.php +++ b/tests/phpunit/includes/MovePageTest.php @@ -2,6 +2,7 @@ use MediaWiki\Config\ServiceOptions; use MediaWiki\Page\MovePageFactory; +use MediaWiki\Permissions\PermissionManager; use Wikimedia\Rdbms\IDatabase; use Wikimedia\Rdbms\LoadBalancer; @@ -22,9 +23,9 @@ class MovePageTest extends MediaWikiTestCase { /** * @param LinkTarget $old * @param LinkTarget $new - * @param array $params Valid keys are: db, options, nsInfo, wiStore. options is an indexed - * array that will overwrite our defaults, not a ServiceOptions, so it need not contain all - * keys. + * @param array $params Valid keys are: db, options, nsInfo, wiStore, repoGroup. + * options is an indexed array that will overwrite our defaults, not a ServiceOptions, so it + * need not contain all keys. * @return MovePage */ private function newMovePage( $old, $new, array $params = [] ) : MovePage { @@ -34,6 +35,21 @@ class MovePageTest extends MediaWikiTestCase { $mockLB->expects( $this->never() ) ->method( $this->anythingBut( 'getConnection', '__destruct' ) ); + $mockLocalFile = $this->createMock( LocalFile::class ); + $mockLocalFile->method( 'exists' )->willReturn( false ); + $mockLocalFile->expects( $this->never() ) + ->method( $this->anythingBut( 'exists', 'load', '__destruct' ) ); + + $mockLocalRepo = $this->createMock( LocalRepo::class ); + $mockLocalRepo->method( 'newFile' )->willReturn( $mockLocalFile ); + $mockLocalRepo->expects( $this->never() ) + ->method( $this->anythingBut( 'newFile', '__destruct' ) ); + + $mockRepoGroup = $this->createMock( RepoGroup::class ); + $mockRepoGroup->method( 'getLocalRepo' )->willReturn( $mockLocalRepo ); + $mockRepoGroup->expects( $this->never() ) + ->method( $this->anythingBut( 'getLocalRepo', '__destruct' ) ); + return new MovePage( $old, $new, @@ -47,7 +63,9 @@ class MovePageTest extends MediaWikiTestCase { ), $mockLB, $params['nsInfo'] ?? $this->getNoOpMock( NamespaceInfo::class ), - $params['wiStore'] ?? $this->getNoOpMock( WatchedItemStore::class ) + $params['wiStore'] ?? $this->getNoOpMock( WatchedItemStore::class ), + $params['permMgr'] ?? $this->getNoOpMock( PermissionManager::class ), + $params['repoGroup'] ?? $mockRepoGroup ); } -- 2.20.1