ChangeTags: turn private getPrevTags() into public getTags()

This makes the most convenient way to get all the change tags for a
given recent change, revision, and/or log entry public. The database is
injected as an additional parameter so that callers can choose to read
from a replica, and the revision and log ID parameters are swapped for
consistency with other methods in the same class. Also, while we’re
already touching the method, let’s extract the change_tag_def store.

Change-Id: Id8f14044ba5d926447aedd60e8cc8eb7dc864899

diff --git a/includes/changetags/ChangeTags.php b/includes/changetags/ChangeTags.php
index 40f7180..8c8125b 100644 (file)
--- a/includes/changetags/ChangeTags.php
+++ b/includes/changetags/ChangeTags.php
@@ -24,6 +24,7 @@
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Storage\NameTableAccessException;
 use Wikimedia\Rdbms\Database;
+use Wikimedia\Rdbms\IDatabase;
 
 class ChangeTags {
        /**
@@ -358,7 +359,7 @@ class ChangeTags {
                        );
                }
 
-               $prevTags = self::getPrevTags( $rc_id, $log_id, $rev_id );
+               $prevTags = self::getTags( $dbw, $rc_id, $rev_id, $log_id );
 
                // add tags
                $tagsToAdd = array_values( array_diff( $tagsToAdd, $prevTags ) );
@@ -452,21 +453,36 @@ class ChangeTags {
                return [ $tagsToAdd, $tagsToRemove, $prevTags ];
        }
 
-       private static function getPrevTags( $rc_id = null, $log_id = null, $rev_id = null ) {
+       /**
+        * Return all the tags associated with the given recent change ID,
+        * revision ID, and/or log entry ID.
+        *
+        * @param IDatabase $db the database to query
+        * @param int|null $rc_id
+        * @param int|null $rev_id
+        * @param int|null $log_id
+        * @return string[]
+        */
+       public static function getTags( IDatabase $db, $rc_id = null, $rev_id = null, $log_id = null ) {
                $conds = array_filter(
                        [
                                'ct_rc_id' => $rc_id,
-                               'ct_log_id' => $log_id,
                                'ct_rev_id' => $rev_id,
+                               'ct_log_id' => $log_id,
                        ]
                );
 
-               $dbw = wfGetDB( DB_MASTER );
-               $tagIds = $dbw->selectFieldValues( 'change_tag', 'ct_tag_id', $conds, __METHOD__ );
+               $tagIds = $db->selectFieldValues(
+                       'change_tag',
+                       'ct_tag_id',
+                       $conds,
+                       __METHOD__
+               );
 
                $tags = [];
+               $changeTagDefStore = MediaWikiServices::getInstance()->getChangeTagDefStore();
                foreach ( $tagIds as $tagId ) {
-                       $tags[] = MediaWikiServices::getInstance()->getChangeTagDefStore()->getName( (int)$tagId );
+                       $tags[] = $changeTagDefStore->getName( (int)$tagId );
                }
 
                return $tags;

diff --git a/tests/phpunit/includes/changetags/ChangeTagsTest.php b/tests/phpunit/includes/changetags/ChangeTagsTest.php
index 1405680..71870e1 100644 (file)
--- a/tests/phpunit/includes/changetags/ChangeTagsTest.php
+++ b/tests/phpunit/includes/changetags/ChangeTagsTest.php
@@ -23,7 +23,7 @@ class ChangeTagsTest extends MediaWikiTestCase {
                $this->tablesUsed[] = 'archive';
        }
 
-       // TODO only modifyDisplayQuery and getSoftwareTags are tested, nothing else is
+       // TODO most methods are not tested
 
        /** @dataProvider provideModifyDisplayQuery */
        public function testModifyDisplayQuery( $origQuery, $filter_tag, $useTags, $modifiedQuery ) {
@@ -555,6 +555,48 @@ class ChangeTagsTest extends MediaWikiTestCase {
                $this->assertEquals( $expected2, iterator_to_array( $res2, false ) );
        }
 
+       public function provideTags() {
+               $tags = [ 'tag 1', 'tag 2', 'tag 3' ];
+               $rcId = 123;
+               $revId = 456;
+               $logId = 789;
+
+               yield [ $tags, $rcId, null, null ];
+               yield [ $tags, null, $revId, null ];
+               yield [ $tags, null, null, $logId ];
+               yield [ $tags, $rcId, $revId, null ];
+               yield [ $tags, $rcId, null, $logId ];
+               yield [ $tags, $rcId, $revId, $logId ];
+       }
+
+       /**
+        * @dataProvider provideTags
+        */
+       public function testGetTags( array $tags, $rcId, $revId, $logId ) {
+               ChangeTags::addTags( $tags, $rcId, $revId, $logId );
+
+               $actualTags = ChangeTags::getTags( $this->db, $rcId, $revId, $logId );
+
+               $this->assertSame( $tags, $actualTags );
+       }
+
+       public function testGetTags_multiple_arguments() {
+               $rcId = 123;
+               $revId = 456;
+               $logId = 789;
+
+               ChangeTags::addTags( [ 'tag 1' ], $rcId );
+               ChangeTags::addTags( [ 'tag 2' ], $rcId, $revId );
+               ChangeTags::addTags( [ 'tag 3' ], $rcId, $revId, $logId );
+
+               $tags3 = [ 'tag 3' ];
+               $tags2 = array_merge( $tags3, [ 'tag 2' ] );
+               $tags1 = array_merge( $tags2, [ 'tag 1' ] );
+               $this->assertArrayEquals( $tags3, ChangeTags::getTags( $this->db, $rcId, $revId, $logId ) );
+               $this->assertArrayEquals( $tags2, ChangeTags::getTags( $this->db, $rcId, $revId ) );
+               $this->assertArrayEquals( $tags1, ChangeTags::getTags( $this->db, $rcId ) );
+       }
+
        public function testTagUsageStatistics() {
                $dbw = wfGetDB( DB_MASTER );
                $dbw->delete( 'change_tag', '*' );