Make WatchedItemQueryService depend on PermissionManager

Bug: T220191
Change-Id: I714d2b33b83dab230a8168765d5523cb14971371

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index 4cd3d85..8807d04 100644 (file)
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -784,7 +784,8 @@ return [
                        $services->getDBLoadBalancer(),
                        $services->getCommentStore(),
                        $services->getActorMigration(),
-                       $services->getWatchedItemStore()
+                       $services->getWatchedItemStore(),
+                       $services->getPermissionManager()
                );
        },
 

diff --git a/includes/watcheditem/WatchedItemQueryService.php b/includes/watcheditem/WatchedItemQueryService.php
index 13f2b52..d8e4985 100644 (file)
--- a/includes/watcheditem/WatchedItemQueryService.php
+++ b/includes/watcheditem/WatchedItemQueryService.php
@@ -1,6 +1,7 @@
 <?php
 
 use MediaWiki\Linker\LinkTarget;
+use MediaWiki\Permissions\PermissionManager;
 use MediaWiki\Revision\RevisionRecord;
 use MediaWiki\User\UserIdentity;
 use Wikimedia\Assert\Assert;
@@ -70,16 +71,21 @@ class WatchedItemQueryService {
        /** @var WatchedItemStoreInterface */
        private $watchedItemStore;
 
+       /** @var PermissionManager */
+       private $permissionManager;
+
        public function __construct(
                ILoadBalancer $loadBalancer,
                CommentStore $commentStore,
                ActorMigration $actorMigration,
-               WatchedItemStoreInterface $watchedItemStore
+               WatchedItemStoreInterface $watchedItemStore,
+               PermissionManager $permissionManager
        ) {
                $this->loadBalancer = $loadBalancer;
                $this->commentStore = $commentStore;
                $this->actorMigration = $actorMigration;
                $this->watchedItemStore = $watchedItemStore;
+               $this->permissionManager = $permissionManager;
        }
 
        /**
@@ -549,7 +555,7 @@ class WatchedItemQueryService {
                return $conds;
        }
 
-       private function getUserRelatedConds( IDatabase $db, User $user, array $options ) {
+       private function getUserRelatedConds( IDatabase $db, UserIdentity $user, array $options ) {
                if ( !array_key_exists( 'onlyByUser', $options ) && !array_key_exists( 'notByUser', $options ) ) {
                        return [];
                }
@@ -566,9 +572,11 @@ class WatchedItemQueryService {
 
                // Avoid brute force searches (T19342)
                $bitmask = 0;
-               if ( !$user->isAllowed( 'deletedhistory' ) ) {
+               if ( !$this->permissionManager->userHasRight( $user, 'deletedhistory' ) ) {
                        $bitmask = RevisionRecord::DELETED_USER;
-               } elseif ( !$user->isAllowedAny( 'suppressrevision', 'viewsuppressed' ) ) {
+               } elseif ( !$this->permissionManager
+                       ->userHasAnyRight( $user, 'suppressrevision', 'viewsuppressed' )
+               ) {
                        $bitmask = RevisionRecord::DELETED_USER | RevisionRecord::DELETED_RESTRICTED;
                }
                if ( $bitmask ) {
@@ -578,13 +586,15 @@ class WatchedItemQueryService {
                return $conds;
        }
 
-       private function getExtraDeletedPageLogEntryRelatedCond( IDatabase $db, User $user ) {
+       private function getExtraDeletedPageLogEntryRelatedCond( IDatabase $db, UserIdentity $user ) {
                // LogPage::DELETED_ACTION hides the affected page, too. So hide those
                // entirely from the watchlist, or someone could guess the title.
                $bitmask = 0;
-               if ( !$user->isAllowed( 'deletedhistory' ) ) {
+               if ( !$this->permissionManager->userHasRight( $user, 'deletedhistory' ) ) {
                        $bitmask = LogPage::DELETED_ACTION;
-               } elseif ( !$user->isAllowedAny( 'suppressrevision', 'viewsuppressed' ) ) {
+               } elseif ( !$this->permissionManager
+                       ->userHasAnyRight( $user, 'suppressrevision', 'viewsuppressed' )
+               ) {
                        $bitmask = LogPage::DELETED_ACTION | LogPage::DELETED_RESTRICTED;
                }
                if ( $bitmask ) {

diff --git a/tests/phpunit/includes/watcheditem/WatchedItemQueryServiceUnitTest.php b/tests/phpunit/includes/watcheditem/WatchedItemQueryServiceUnitTest.php
index 0b1d013..45fa143 100644 (file)
--- a/tests/phpunit/includes/watcheditem/WatchedItemQueryServiceUnitTest.php
+++ b/tests/phpunit/includes/watcheditem/WatchedItemQueryServiceUnitTest.php
@@ -1,5 +1,6 @@
 <?php
 
+use MediaWiki\Permissions\PermissionManager;
 use MediaWiki\User\UserIdentityValue;
 use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\LoadBalancer;
@@ -66,14 +67,16 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase {
 
        /**
         * @param PHPUnit_Framework_MockObject_MockObject|Database $mockDb
+        * @param PHPUnit_Framework_MockObject_MockObject|PermissionManager $mockPM
         * @return WatchedItemQueryService
         */
-       private function newService( $mockDb ) {
+       private function newService( $mockDb, $mockPM = null ) {
                return new WatchedItemQueryService(
                        $this->getMockLoadBalancer( $mockDb ),
                        $this->getMockCommentStore(),
                        $this->getMockActorMigration(),
-                       $this->getMockWatchedItemStore()
+                       $this->getMockWatchedItemStore(),
+                       $mockPM ?: $this->getMockPermissionManager()
                );
        }
 
@@ -153,6 +156,25 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase {
                return $mock;
        }
 
+       /**
+        * @param string $notAllowedAction
+        * @return PHPUnit_Framework_MockObject_MockObject|PermissionManager
+        */
+       private function getMockPermissionManager( $notAllowedAction = null ) {
+               $mock = $this->getMockBuilder( PermissionManager::class )
+                       ->disableOriginalConstructor()
+                       ->getMock();
+               $mock->method( 'userHasRight' )
+                       ->will( $this->returnCallback( function ( $user, $action ) use ( $notAllowedAction ) {
+                               return $action !== $notAllowedAction;
+                       } ) );
+               $mock->method( 'userHasAnyRight' )
+                       ->will( $this->returnCallback( function ( $user, ...$actions ) use ( $notAllowedAction ) {
+                               return !in_array( $notAllowedAction, $actions );
+                       } ) );
+               return $mock;
+       }
+
        /**
         * @param int $id
         * @param string[] $extraMethods Extra methods that are expected might be called
@@ -177,9 +199,7 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase {
         */
        private function getMockUnrestrictedNonAnonUserWithId( $id, array $extraMethods = [] ) {
                $mock = $this->getMockNonAnonUserWithId( $id,
-                       array_merge( [ 'isAllowed', 'isAllowedAny', 'useRCPatrol' ], $extraMethods ) );
-               $mock->method( 'isAllowed' )->willReturn( true );
-               $mock->method( 'isAllowedAny' )->willReturn( true );
+                       array_merge( [ 'useRCPatrol' ], $extraMethods ) );
                $mock->method( 'useRCPatrol' )->willReturn( true );
                return $mock;
        }
@@ -189,18 +209,9 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase {
         * @param string $notAllowedAction
         * @return PHPUnit_Framework_MockObject_MockObject|User
         */
-       private function getMockNonAnonUserWithIdAndRestrictedPermissions( $id, $notAllowedAction ) {
+       private function getMockNonAnonUserWithIdAndRestrictedPermissions( $id ) {
                $mock = $this->getMockNonAnonUserWithId( $id,
-                       [ 'isAllowed', 'isAllowedAny', 'useRCPatrol', 'useNPPatrol' ] );
-
-               $mock->method( 'isAllowed' )
-                       ->will( $this->returnCallback( function ( $action ) use ( $notAllowedAction ) {
-                               return $action !== $notAllowedAction;
-                       } ) );
-               $mock->method( 'isAllowedAny' )
-                       ->will( $this->returnCallback( function ( ...$actions ) use ( $notAllowedAction ) {
-                               return !in_array( $notAllowedAction, $actions );
-                       } ) );
+                       [ 'useRCPatrol', 'useNPPatrol' ] );
                $mock->method( 'useRCPatrol' )->willReturn( false );
                $mock->method( 'useNPPatrol' )->willReturn( false );
 
@@ -213,15 +224,7 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase {
         */
        private function getMockNonAnonUserWithIdAndNoPatrolRights( $id ) {
                $mock = $this->getMockNonAnonUserWithId( $id,
-                       [ 'isAllowed', 'isAllowedAny', 'useRCPatrol', 'useNPPatrol' ] );
-
-               $mock->expects( $this->any() )
-                       ->method( 'isAllowed' )
-                       ->will( $this->returnValue( true ) );
-               $mock->expects( $this->any() )
-                       ->method( 'isAllowedAny' )
-                       ->will( $this->returnValue( true ) );
-
+                       [ 'useRCPatrol', 'useNPPatrol' ] );
                $mock->expects( $this->any() )
                        ->method( 'useRCPatrol' )
                        ->will( $this->returnValue( false ) );
@@ -1132,9 +1135,10 @@ class WatchedItemQueryServiceUnitTest extends MediaWikiTestCase {
                        )
                        ->will( $this->returnValue( [] ) );
 
-               $user = $this->getMockNonAnonUserWithIdAndRestrictedPermissions( 1, $notAllowedAction );
+               $permissionManager = $this->getMockPermissionManager( $notAllowedAction );
+               $user = $this->getMockNonAnonUserWithIdAndRestrictedPermissions( 1 );
 
-               $queryService = $this->newService( $mockDb );
+               $queryService = $this->newService( $mockDb, $permissionManager );
                $items = $queryService->getWatchedItemsWithRecentChangeInfo( $user, $options );
 
                $this->assertEmpty( $items );