Add code to write to change_tag_def table

Bug: T193868
Change-Id: Ia61b9878595dedd2725148e1078063bf1f127d2a

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 87ca016..78d336a 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -8910,6 +8910,20 @@ $wgActorTableSchemaMigrationStage = MIGRATION_OLD;
  */
 $wgExpiryWidgetNoDatePicker = false;
 
+/**
+ * change_tag table schema migration stage.
+ *
+ * - MIGRATION_OLD: Do not use change_tag_def table or ct_tag_id.
+ * - MIGRATION_WRITE_BOTH: Write to the change_tag_def table and ct_tag_id, but read from
+ *   the old schema. This is different from the formal definition of the constants
+ * - MIGRATION_WRITE_NEW: Behaves the same as MIGRATION_WRITE_BOTH
+ * - MIGRATION_NEW: Use the change_tag_def table and ct_tag_id, do not read/write ct_tag
+ *
+ * @since 1.32
+ * @var int One of the MIGRATION_* constants
+ */
+$wgChangeTagsSchemaMigrationStage = MIGRATION_OLD;
+
 /**
  * For really cool vim folding this needs to be at the end:
  * vim: foldmarker=@{,@} foldmethod=marker

diff --git a/includes/changetags/ChangeTags.php b/includes/changetags/ChangeTags.php
index b64f85a..8f8426f 100644 (file)
--- a/includes/changetags/ChangeTags.php
+++ b/includes/changetags/ChangeTags.php
@@ -261,6 +261,8 @@ class ChangeTags {
                &$rev_id = null, &$log_id = null, $params = null, RecentChange $rc = null,
                User $user = null
        ) {
+               global $wgChangeTagsSchemaMigrationStage;
+
                $tagsToAdd = array_filter( (array)$tagsToAdd ); // Make sure we're submitting all tags...
                $tagsToRemove = array_filter( (array)$tagsToRemove );
 
@@ -342,6 +344,35 @@ class ChangeTags {
 
                // insert a row into change_tag for each new tag
                if ( count( $tagsToAdd ) ) {
+                       $changeTagMapping = [];
+                       if ( $wgChangeTagsSchemaMigrationStage > MIGRATION_OLD ) {
+                               $tagDefRows = [];
+                               foreach ( $tagsToAdd as $tag ) {
+                                       $tagDefRows[] = [
+                                               'ctd_name' => $tag,
+                                               'ctd_user_defined' => 0,
+                                               'ctd_count' => 1
+                                       ];
+                               }
+
+                               $dbw->upsert(
+                                       'change_tag_def',
+                                       $tagDefRows,
+                                       [ 'ctd_name' ],
+                                       [ 'ctd_count = ctd_count + 1' ],
+                                       __METHOD__
+                               );
+
+                               $res = $dbw->select(
+                                       'change_tag_def',
+                                       [ 'ctd_name', 'ctd_id' ],
+                                       [ 'ctd_name' => $tagsToAdd ]
+                               );
+                               foreach ( $res as $row ) {
+                                       $changeTagMapping[$row->ctd_name] = $row->ctd_id;
+                               }
+                       }
+
                        $tagsRows = [];
                        foreach ( $tagsToAdd as $tag ) {
                                // Filter so we don't insert NULLs as zero accidentally.
@@ -354,9 +385,11 @@ class ChangeTags {
                                                'ct_rc_id' => $rc_id,
                                                'ct_log_id' => $log_id,
                                                'ct_rev_id' => $rev_id,
-                                               'ct_params' => $params
+                                               'ct_params' => $params,
+                                               'ct_tag_id' => isset( $changeTagMapping[$tag] ) ? $changeTagMapping[$tag] : null,
                                        ]
                                );
+
                        }
 
                        $dbw->insert( 'change_tag', $tagsRows, __METHOD__, [ 'IGNORE' ] );
@@ -374,6 +407,20 @@ class ChangeTags {
                                        ]
                                );
                                $dbw->delete( 'change_tag', $conds, __METHOD__ );
+                               if ( $dbw->affectedRows() && $wgChangeTagsSchemaMigrationStage > MIGRATION_OLD ) {
+                                       $dbw->update(
+                                               'change_tag_def',
+                                               [ 'ctd_count = ctd_count - 1' ],
+                                               [ 'ctd_name' => $tag ],
+                                               __METHOD__
+                                       );
+
+                                       $dbw->delete(
+                                               'change_tag_def',
+                                               [ 'ctd_name' => $tag, 'ctd_count' => 0, 'ctd_user_defined' => 0 ],
+                                               __METHOD__
+                                       );
+                               }
                        }
                }
 
@@ -828,8 +875,8 @@ class ChangeTags {
        }
 
        /**
-        * Defines a tag in the valid_tag table, without checking that the tag name
-        * is valid.
+        * Defines a tag in the valid_tag table and/or update ctd_user_defined field in change_tag_def,
+        * without checking that the tag name is valid.
         * Extensions should NOT use this function; they can use the ListDefinedTags
         * hook instead.
         *
@@ -837,26 +884,63 @@ class ChangeTags {
         * @since 1.25
         */
        public static function defineTag( $tag ) {
+               global $wgChangeTagsSchemaMigrationStage;
+
                $dbw = wfGetDB( DB_MASTER );
-               $dbw->replace( 'valid_tag',
+               if ( $wgChangeTagsSchemaMigrationStage > MIGRATION_OLD ) {
+                       $tagDef = [
+                               'ctd_name' => $tag,
+                               'ctd_user_defined' => 1,
+                               'ctd_count' => 0
+                       ];
+                       $dbw->upsert(
+                               'change_tag_def',
+                               $tagDef,
+                               [ 'ctd_name' ],
+                               [ 'ctd_user_defined' => 1 ],
+                               __METHOD__
+                       );
+               }
+
+               $dbw->replace(
+                       'valid_tag',
                        [ 'vt_tag' ],
                        [ 'vt_tag' => $tag ],
-                       __METHOD__ );
+                       __METHOD__
+               );
 
                // clear the memcache of defined tags
                self::purgeTagCacheAll();
        }
 
        /**
-        * Removes a tag from the valid_tag table. The tag may remain in use by
-        * extensions, and may still show up as 'defined' if an extension is setting
-        * it from the ListDefinedTags hook.
+        * Removes a tag from the valid_tag table and/or update ctd_user_defined field in change_tag_def.
+        * The tag may remain in use by extensions, and may still show up as 'defined'
+        * if an extension is setting it from the ListDefinedTags hook.
         *
         * @param string $tag Tag to remove
         * @since 1.25
         */
        public static function undefineTag( $tag ) {
+               global $wgChangeTagsSchemaMigrationStage;
+
                $dbw = wfGetDB( DB_MASTER );
+
+               if ( $wgChangeTagsSchemaMigrationStage > MIGRATION_OLD ) {
+                       $dbw->update(
+                               'change_tag_def',
+                               [ 'ctd_name' => $tag ],
+                               [ 'ctd_user_defined' => 0 ],
+                               __METHOD__
+                       );
+
+                       $dbw->delete(
+                               'change_tag_def',
+                               [ 'ctd_name' => $tag, 'ctd_count' => 0 ],
+                               __METHOD__
+                       );
+               }
+
                $dbw->delete( 'valid_tag', [ 'vt_tag' => $tag ], __METHOD__ );
 
                // clear the memcache of defined tags
@@ -1108,6 +1192,7 @@ class ChangeTags {
 
        /**
         * Creates a tag by adding a row to the `valid_tag` table.
+        * and/or add it to `change_tag_def` table.
         *
         * Extensions should NOT use this function; they can use the ListDefinedTags
         * hook instead.
@@ -1158,10 +1243,11 @@ class ChangeTags {
         * @since 1.25
         */
        public static function deleteTagEverywhere( $tag ) {
+               global $wgChangeTagsSchemaMigrationStage;
                $dbw = wfGetDB( DB_MASTER );
                $dbw->startAtomic( __METHOD__ );
 
-               // delete from valid_tag
+               // delete from valid_tag and/or set ctd_user_defined = 0
                self::undefineTag( $tag );
 
                // find out which revisions use this tag, so we can delete from tag_summary
@@ -1180,6 +1266,10 @@ class ChangeTags {
                // delete from change_tag
                $dbw->delete( 'change_tag', [ 'ct_tag' => $tag ], __METHOD__ );
 
+               if ( $wgChangeTagsSchemaMigrationStage > MIGRATION_OLD ) {
+                       $dbw->delete( 'change_tag_def', [ 'ctd_name' => $tag ], __METHOD__ );
+               }
+
                $dbw->endAtomic( __METHOD__ );
 
                // give extensions a chance

diff --git a/tests/phpunit/includes/changetags/ChangeTagsTest.php b/tests/phpunit/includes/changetags/ChangeTagsTest.php
index 63e0ec2..215cdfd 100644 (file)
--- a/tests/phpunit/includes/changetags/ChangeTagsTest.php
+++ b/tests/phpunit/includes/changetags/ChangeTagsTest.php
@@ -2,9 +2,18 @@
 
 /**
  * @covers ChangeTags
+ * @group Database
  */
 class ChangeTagsTest extends MediaWikiTestCase {
 
+       public function setUp() {
+               parent::setUp();
+
+               $this->tablesUsed[] = 'change_tag';
+               $this->tablesUsed[] = 'change_tag_def';
+               $this->tablesUsed[] = 'tag_summary';
+       }
+
        // TODO only modifyDisplayQuery and getSoftwareTags are tested, nothing else is
 
        /** @dataProvider provideModifyDisplayQuery */
@@ -306,4 +315,221 @@ class ChangeTagsTest extends MediaWikiTestCase {
                sort( $actual );
                $this->assertEquals( $expected, $actual );
        }
+
+       public function testUpdateTagsMigrationOld() {
+               $this->setMwGlobals( 'wgChangeTagsSchemaMigrationStage', MIGRATION_OLD );
+               $dbw = wfGetDB( DB_MASTER );
+               $dbw->delete( 'change_tag', '*' );
+               $dbw->delete( 'change_tag_def', '*' );
+
+               $rcId = 123;
+               ChangeTags::updateTags( [ 'tag1', 'tag2' ], [], $rcId );
+
+               $dbr = wfGetDB( DB_REPLICA );
+
+               $res = $dbr->select( 'change_tag_def', [ 'ctd_name', 'ctd_id', 'ctd_count' ], '' );
+               $this->assertEquals( [], iterator_to_array( $res, false ) );
+
+               $expected2 = [
+                       (object)[
+                               'ct_tag' => 'tag1',
+                               'ct_tag_id' => null,
+                               'ct_rc_id' => 123
+                       ],
+                       (object)[
+                               'ct_tag' => 'tag2',
+                               'ct_tag_id' => null,
+                               'ct_rc_id' => 123
+                       ],
+               ];
+               $res2 = $dbr->select( 'change_tag', [ 'ct_tag', 'ct_tag_id', 'ct_rc_id' ], '' );
+               $this->assertEquals( $expected2, iterator_to_array( $res2, false ) );
+
+               $rcId = 124;
+               ChangeTags::updateTags( [ 'tag1', 'tag3' ], [], $rcId );
+
+               $dbr = wfGetDB( DB_REPLICA );
+
+               $res = $dbr->select( 'change_tag_def', [ 'ctd_name', 'ctd_id', 'ctd_count' ], '' );
+               $this->assertEquals( [], iterator_to_array( $res, false ) );
+
+               $expected2 = [
+                       (object)[
+                               'ct_tag' => 'tag1',
+                               'ct_tag_id' => null,
+                               'ct_rc_id' => 123
+                       ],
+                       (object)[
+                               'ct_tag' => 'tag2',
+                               'ct_tag_id' => null,
+                               'ct_rc_id' => 123
+                       ],
+                       (object)[
+                               'ct_tag' => 'tag1',
+                               'ct_tag_id' => null,
+                               'ct_rc_id' => 124
+                       ],
+                       (object)[
+                               'ct_tag' => 'tag3',
+                               'ct_tag_id' => null,
+                               'ct_rc_id' => 124
+                       ],
+               ];
+               $res2 = $dbr->select( 'change_tag', [ 'ct_tag', 'ct_tag_id', 'ct_rc_id' ], '' );
+               $this->assertEquals( $expected2, iterator_to_array( $res2, false ) );
+       }
+
+       public function testUpdateTagsMigrationWriteBoth() {
+               $this->setMwGlobals( 'wgChangeTagsSchemaMigrationStage', MIGRATION_WRITE_BOTH );
+               $dbw = wfGetDB( DB_MASTER );
+               $dbw->delete( 'change_tag', '*' );
+               $dbw->delete( 'change_tag_def', '*' );
+
+               $rcId = 123;
+               ChangeTags::updateTags( [ 'tag1', 'tag2' ], [], $rcId );
+
+               $dbr = wfGetDB( DB_REPLICA );
+
+               $expected = [
+                       (object)[
+                               'ctd_name' => 'tag1',
+                               'ctd_id' => 1,
+                               'ctd_count' => 1
+                       ],
+                       (object)[
+                               'ctd_name' => 'tag2',
+                               'ctd_id' => 2,
+                               'ctd_count' => 1
+                       ],
+               ];
+               $res = $dbr->select( 'change_tag_def', [ 'ctd_name', 'ctd_id', 'ctd_count' ], '' );
+               $this->assertEquals( $expected, iterator_to_array( $res, false ) );
+
+               $expected2 = [
+                       (object)[
+                               'ct_tag' => 'tag1',
+                               'ct_tag_id' => 1,
+                               'ct_rc_id' => 123
+                       ],
+                       (object)[
+                               'ct_tag' => 'tag2',
+                               'ct_tag_id' => 2,
+                               'ct_rc_id' => 123
+                       ],
+               ];
+               $res2 = $dbr->select( 'change_tag', [ 'ct_tag', 'ct_tag_id', 'ct_rc_id' ], '' );
+               $this->assertEquals( $expected2, iterator_to_array( $res2, false ) );
+
+               $rcId = 124;
+               ChangeTags::updateTags( [ 'tag1', 'tag3' ], [], $rcId );
+
+               $dbr = wfGetDB( DB_REPLICA );
+
+               $expected = [
+                       (object)[
+                               'ctd_name' => 'tag1',
+                               'ctd_id' => 1,
+                               'ctd_count' => 2
+                       ],
+                       (object)[
+                               'ctd_name' => 'tag2',
+                               'ctd_id' => 2,
+                               'ctd_count' => 1
+                       ],
+                       (object)[
+                               'ctd_name' => 'tag3',
+                               'ctd_id' => 3,
+                               'ctd_count' => 1
+                       ],
+               ];
+               $res = $dbr->select( 'change_tag_def', [ 'ctd_name', 'ctd_id', 'ctd_count' ], '' );
+               $this->assertEquals( $expected, iterator_to_array( $res, false ) );
+
+               $expected2 = [
+                       (object)[
+                               'ct_tag' => 'tag1',
+                               'ct_tag_id' => 1,
+                               'ct_rc_id' => 123
+                       ],
+                       (object)[
+                               'ct_tag' => 'tag2',
+                               'ct_tag_id' => 2,
+                               'ct_rc_id' => 123
+                       ],
+                       (object)[
+                               'ct_tag' => 'tag1',
+                               'ct_tag_id' => 1,
+                               'ct_rc_id' => 124
+                       ],
+                       (object)[
+                               'ct_tag' => 'tag3',
+                               'ct_tag_id' => 3,
+                               'ct_rc_id' => 124
+                       ],
+               ];
+               $res2 = $dbr->select( 'change_tag', [ 'ct_tag', 'ct_tag_id', 'ct_rc_id' ], '' );
+               $this->assertEquals( $expected2, iterator_to_array( $res2, false ) );
+       }
+
+       public function testDeleteTagsMigrationOld() {
+               $this->setMwGlobals( 'wgChangeTagsSchemaMigrationStage', MIGRATION_OLD );
+               $dbw = wfGetDB( DB_MASTER );
+               $dbw->delete( 'change_tag', '*' );
+               $dbw->delete( 'change_tag_def', '*' );
+
+               $rcId = 123;
+               ChangeTags::updateTags( [ 'tag1', 'tag2' ], [], $rcId );
+
+               ChangeTags::updateTags( [], [ 'tag2' ], $rcId );
+
+               $dbr = wfGetDB( DB_REPLICA );
+
+               $res = $dbr->select( 'change_tag_def', [ 'ctd_name', 'ctd_id', 'ctd_count' ], '' );
+               $this->assertEquals( [], iterator_to_array( $res, false ) );
+
+               $expected2 = [
+                       (object)[
+                               'ct_tag' => 'tag1',
+                               'ct_tag_id' => null,
+                               'ct_rc_id' => 123
+                       ]
+               ];
+               $res2 = $dbr->select( 'change_tag', [ 'ct_tag', 'ct_tag_id', 'ct_rc_id' ], '' );
+               $this->assertEquals( $expected2, iterator_to_array( $res2, false ) );
+       }
+
+       public function testDeleteTagsMigrationWriteBoth() {
+               $this->setMwGlobals( 'wgChangeTagsSchemaMigrationStage', MIGRATION_WRITE_BOTH );
+               $dbw = wfGetDB( DB_MASTER );
+               $dbw->delete( 'change_tag', '*' );
+               $dbw->delete( 'change_tag_def', '*' );
+
+               $rcId = 123;
+               ChangeTags::updateTags( [ 'tag1', 'tag2' ], [], $rcId );
+
+               ChangeTags::updateTags( [], [ 'tag2' ], $rcId );
+
+               $dbr = wfGetDB( DB_REPLICA );
+
+               $expected = [
+                       (object)[
+                               'ctd_name' => 'tag1',
+                               'ctd_id' => 1,
+                               'ctd_count' => 1
+                       ],
+               ];
+               $res = $dbr->select( 'change_tag_def', [ 'ctd_name', 'ctd_id', 'ctd_count' ], '' );
+               $this->assertEquals( $expected, iterator_to_array( $res, false ) );
+
+               $expected2 = [
+                       (object)[
+                               'ct_tag' => 'tag1',
+                               'ct_tag_id' => 1,
+                               'ct_rc_id' => 123
+                       ]
+               ];
+               $res2 = $dbr->select( 'change_tag', [ 'ct_tag', 'ct_tag_id', 'ct_rc_id' ], '' );
+               $this->assertEquals( $expected2, iterator_to_array( $res2, false ) );
+       }
+
 }