From b0bdfd7e9f7fab3fb2eae37da5d953d03362c15f Mon Sep 17 00:00:00 2001 From: daniel Date: Tue, 6 Mar 2018 15:42:43 +0100 Subject: [PATCH] MCR: replace slot_inherited with slot_origin Bug: T189004 Change-Id: Ie9dbda3296a71f584c82a5f275098adc225a53d5 --- includes/Storage/RevisionStore.php | 37 ++++++---- includes/Storage/SlotRecord.php | 69 +++++++++++++++---- includes/installer/MssqlUpdater.php | 1 + includes/installer/MysqlUpdater.php | 1 + includes/installer/OracleUpdater.php | 1 + includes/installer/PostgresUpdater.php | 21 ++++++ includes/installer/SqliteUpdater.php | 1 + maintenance/archives/patch-slot-origin.sql | 15 ++++ maintenance/archives/patch-slots.sql | 7 +- .../mssql/archives/patch-slot-origin.sql | 14 ++++ maintenance/mssql/archives/patch-slots.sql | 7 +- maintenance/mssql/tables.sql | 7 +- .../oracle/archives/patch-slot-origin.sql | 14 ++++ maintenance/oracle/archives/patch-slots.sql | 4 +- maintenance/oracle/tables.sql | 4 +- .../postgres/archives/patch-slots-table.sql | 4 +- maintenance/postgres/tables.sql | 4 +- .../sqlite/archives/patch-slot-origin.sql | 34 +++++++++ maintenance/tables.sql | 7 +- .../includes/Storage/RevisionStoreDbTest.php | 10 +++ .../includes/Storage/SlotRecordTest.php | 39 +++++++++-- 21 files changed, 250 insertions(+), 51 deletions(-) create mode 100644 maintenance/archives/patch-slot-origin.sql create mode 100644 maintenance/mssql/archives/patch-slot-origin.sql create mode 100644 maintenance/oracle/archives/patch-slot-origin.sql create mode 100644 maintenance/sqlite/archives/patch-slot-origin.sql diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php index 98ad2876c8..1b933c34d3 100644 --- a/includes/Storage/RevisionStore.php +++ b/includes/Storage/RevisionStore.php @@ -594,16 +594,17 @@ class RevisionStore if ( $current ) { $fields = [ - 'page' => $title->getArticleID(), - 'user_text' => $user->getName(), - 'user' => $user->getId(), - 'actor' => $user->getActorId(), - 'comment' => $comment, - 'minor_edit' => $minor, - 'text_id' => $current->rev_text_id, - 'parent_id' => $current->page_latest, - 'len' => $current->rev_len, - 'sha1' => $current->rev_sha1 + 'page' => $title->getArticleID(), + 'user_text' => $user->getName(), + 'user' => $user->getId(), + 'actor' => $user->getActorId(), + 'comment' => $comment, + 'minor_edit' => $minor, + 'text_id' => $current->rev_text_id, + 'parent_id' => $current->page_latest, + 'slot_origin' => $current->page_latest, + 'len' => $current->rev_len, + 'sha1' => $current->rev_sha1 ]; if ( $this->contentHandlerUseDB ) { @@ -771,6 +772,11 @@ class RevisionStore $mainSlotRow->content_address = 'tt:' . $row->rev_text_id; } + // This is used by null-revisions + $mainSlotRow->slot_origin = isset( $row->slot_origin ) + ? intval( $row->slot_origin ) + : null; + if ( isset( $row->old_text ) ) { // this happens when the text-table gets joined directly, in the pre-1.30 schema $blobData = isset( $row->old_text ) ? strval( $row->old_text ) : null; @@ -798,6 +804,9 @@ class RevisionStore $mainSlotRow->slot_content_id = isset( $row['text_id'] ) ? intval( $row['text_id'] ) : null; + $mainSlotRow->slot_origin = isset( $row['slot_origin'] ) + ? intval( $row['slot_origin'] ) + : null; $mainSlotRow->content_address = isset( $row['text_id'] ) ? 'tt:' . intval( $row['text_id'] ) : null; @@ -835,9 +844,11 @@ class RevisionStore throw new MWException( 'Revision constructor passed invalid row format.' ); } - // With the old schema, the content changes with every revision. - // ...except for null-revisions. Would be nice if we could detect them. - $mainSlotRow->slot_inherited = 0; + // With the old schema, the content changes with every revision, + // except for null-revisions. + if ( !isset( $mainSlotRow->slot_origin ) ) { + $mainSlotRow->slot_origin = $mainSlotRow->slot_revision_id; + } if ( $mainSlotRow->model_name === null ) { $mainSlotRow->model_name = function ( SlotRecord $slot ) use ( $title ) { diff --git a/includes/Storage/SlotRecord.php b/includes/Storage/SlotRecord.php index b59d92f065..50d1100547 100644 --- a/includes/Storage/SlotRecord.php +++ b/includes/Storage/SlotRecord.php @@ -96,8 +96,13 @@ class SlotRecord { * @return SlotRecord */ public static function newInherited( SlotRecord $slot ) { + // Sanity check - we can't inherit from a Slot that's not attached to a revision. + $slot->getRevision(); + $slot->getOrigin(); + $slot->getAddress(); + + // NOTE: slot_origin and content_address are copied from $slot. return self::newDerived( $slot, [ - 'slot_inherited' => true, 'slot_revision_id' => null, ] ); } @@ -122,7 +127,7 @@ class SlotRecord { $row = [ 'slot_id' => null, // not yet known 'slot_revision_id' => null, // not yet known - 'slot_inherited' => 0, // not inherited + 'slot_origin' => null, // not yet known, will be set in newSaved() 'content_size' => null, // compute later 'content_sha1' => null, // compute later 'slot_content_id' => null, // not yet known, will be set in newSaved() @@ -183,15 +188,26 @@ class SlotRecord { ); } - if ( $protoSlot->isInherited() && !$protoSlot->hasAddress() ) { - throw new InvalidArgumentException( - "An inherited blob should have a content address!" - ); + if ( $protoSlot->isInherited() ) { + if ( !$protoSlot->hasAddress() ) { + throw new InvalidArgumentException( + "An inherited blob should have a content address!" + ); + } + if ( !$protoSlot->hasField( 'slot_origin' ) ) { + throw new InvalidArgumentException( + "A saved inherited slot should have an origin set!" + ); + } + $origin = $protoSlot->getOrigin(); + } else { + $origin = $revisionId; } return self::newDerived( $protoSlot, [ 'slot_revision_id' => $revisionId, 'slot_content_id' => $contentId, + 'slot_origin' => $origin, 'content_address' => $contentAddress, ] ); } @@ -225,11 +241,6 @@ class SlotRecord { '$row->slot_revision_id', 'must exist' ); - Assert::parameter( - property_exists( $row, 'slot_inherited' ), - '$row->slot_inherited', - 'must exist' - ); Assert::parameter( property_exists( $row, 'slot_content_id' ), '$row->slot_content_id', @@ -245,6 +256,21 @@ class SlotRecord { '$row->model_name', 'must exist' ); + Assert::parameter( + property_exists( $row, 'slot_origin' ), + '$row->slot_origin', + 'must exist' + ); + Assert::parameter( + !property_exists( $row, 'slot_inherited' ), + '$row->slot_inherited', + 'must not exist' + ); + Assert::parameter( + !property_exists( $row, 'slot_revision' ), + '$row->slot_revision', + 'must not exist' + ); $this->row = $row; $this->content = $content; @@ -365,13 +391,32 @@ class SlotRecord { return $this->getIntField( 'slot_revision_id' ); } + /** + * Returns the revision ID of the revision that originated the slot's content. + * + * @return int + */ + public function getOrigin() { + return $this->getIntField( 'slot_origin' ); + } + /** * Whether this slot was inherited from an older revision. * + * If this SlotRecord is already attached to a revision, this returns true + * if the slot's revision of origin is the same as the revision it belongs to. + * + * If this SlotRecord is not yet attached to a revision, this returns true + * if the slot already has an address. + * * @return bool */ public function isInherited() { - return $this->getIntField( 'slot_inherited' ) !== 0; + if ( $this->hasRevision() ) { + return $this->getRevision() !== $this->getOrigin(); + } else { + return $this->hasAddress(); + } } /** diff --git a/includes/installer/MssqlUpdater.php b/includes/installer/MssqlUpdater.php index 38a9ede4b1..a70cab1530 100644 --- a/includes/installer/MssqlUpdater.php +++ b/includes/installer/MssqlUpdater.php @@ -112,6 +112,7 @@ class MssqlUpdater extends DatabaseUpdater { // 1.31 [ 'addTable', 'slots', 'patch-slots.sql' ], + [ 'addField', 'slots', 'slot_origin', 'patch-slot-origin.sql' ], [ 'addTable', 'content', 'patch-content.sql' ], [ 'addTable', 'slot_roles', 'patch-slot_roles.sql' ], [ 'addTable', 'content_models', 'patch-content_models.sql' ], diff --git a/includes/installer/MysqlUpdater.php b/includes/installer/MysqlUpdater.php index bce469053b..46ff747611 100644 --- a/includes/installer/MysqlUpdater.php +++ b/includes/installer/MysqlUpdater.php @@ -332,6 +332,7 @@ class MysqlUpdater extends DatabaseUpdater { // 1.31 [ 'addTable', 'slots', 'patch-slots.sql' ], + [ 'addField', 'slots', 'slot_origin', 'patch-slot-origin.sql' ], [ 'addTable', 'content', 'patch-content.sql' ], [ 'addTable', 'slot_roles', 'patch-slot_roles.sql' ], [ 'addTable', 'content_models', 'patch-content_models.sql' ], diff --git a/includes/installer/OracleUpdater.php b/includes/installer/OracleUpdater.php index 60ac23c1e0..43b74f1f38 100644 --- a/includes/installer/OracleUpdater.php +++ b/includes/installer/OracleUpdater.php @@ -133,6 +133,7 @@ class OracleUpdater extends DatabaseUpdater { // 1.31 [ 'addTable', 'slots', 'patch-slots.sql' ], + [ 'addField', 'slots', 'slot_origin', 'patch-slot-origin.sql' ], [ 'addTable', 'content', 'patch-content.sql' ], [ 'addTable', 'slot_roles', 'patch-slot_roles.sql' ], [ 'addTable', 'content_models', 'patch-content_models.sql' ], diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php index 2bfadf4adf..48f47f5f61 100644 --- a/includes/installer/PostgresUpdater.php +++ b/includes/installer/PostgresUpdater.php @@ -487,6 +487,15 @@ class PostgresUpdater extends DatabaseUpdater { // 1.31 [ 'addTable', 'slots', 'patch-slots-table.sql' ], + [ 'dropPgIndex', 'slots', 'slot_role_inherited' ], + [ 'dropPgField', 'slots', 'slot_inherited' ], + [ 'addPgField', 'slots', 'slot_origin', 'INTEGER NOT NULL' ], + [ + 'addPgIndex', + 'slots', + 'slot_revision_origin_role', + '( slot_revision_id, slot_origin, slot_role_id )', + ], [ 'addTable', 'content', 'patch-content-table.sql' ], [ 'addTable', 'content_models', 'patch-content_models-table.sql' ], [ 'addTable', 'slot_roles', 'patch-slot_roles-table.sql' ], @@ -763,6 +772,18 @@ END; $this->db->query( "ALTER INDEX $old RENAME TO $new" ); } + protected function dropPgField( $table, $field ) { + $fi = $this->db->fieldInfo( $table, $field ); + if ( is_null( $fi ) ) { + $this->output( "...$table table does not contain $field field.\n" ); + + return; + } else { + $this->output( "Dropping column '$table.$field'\n" ); + $this->db->query( "ALTER TABLE $table DROP COLUMN $field" ); + } + } + protected function addPgField( $table, $field, $type ) { $fi = $this->db->fieldInfo( $table, $field ); if ( !is_null( $fi ) ) { diff --git a/includes/installer/SqliteUpdater.php b/includes/installer/SqliteUpdater.php index 3a755b6d40..d396ce2846 100644 --- a/includes/installer/SqliteUpdater.php +++ b/includes/installer/SqliteUpdater.php @@ -198,6 +198,7 @@ class SqliteUpdater extends DatabaseUpdater { [ 'addTable', 'content', 'patch-content.sql' ], [ 'addTable', 'content_models', 'patch-content_models.sql' ], [ 'addTable', 'slots', 'patch-slots.sql' ], + [ 'addField', 'slots', 'slot_origin', 'patch-slot-origin.sql' ], [ 'addTable', 'slot_roles', 'patch-slot_roles.sql' ], [ 'migrateArchiveText' ], [ 'addTable', 'actor', 'patch-actor-table.sql' ], diff --git a/maintenance/archives/patch-slot-origin.sql b/maintenance/archives/patch-slot-origin.sql new file mode 100644 index 0000000000..ee06923195 --- /dev/null +++ b/maintenance/archives/patch-slot-origin.sql @@ -0,0 +1,15 @@ +-- +-- Replace slot_inherited with slot_origin. +-- +-- NOTE: There is no release that has slot_inherited. This is only needed to transition between +-- snapshot versions of 1.30. +-- +-- NOTE: No code that writes to the slots table was merged yet, the table is assumed to be empty. +-- +DROP INDEX /*i*/slot_role_inherited ON /*_*/slots; + +ALTER TABLE /*_*/slots + DROP COLUMN slot_inherited, + ADD COLUMN slot_origin bigint unsigned NOT NULL; + +CREATE INDEX /*i*/slot_revision_origin_role ON /*_*/slots (slot_revision_id, slot_origin, slot_role_id); diff --git a/maintenance/archives/patch-slots.sql b/maintenance/archives/patch-slots.sql index 1a51bb9ef3..5fafe6d39d 100644 --- a/maintenance/archives/patch-slots.sql +++ b/maintenance/archives/patch-slots.sql @@ -14,11 +14,12 @@ CREATE TABLE /*_*/slots ( -- reference to content_id slot_content_id bigint unsigned NOT NULL, - -- whether the content is inherited (1) or new in this revision (0) - slot_inherited tinyint unsigned NOT NULL DEFAULT 0, + -- The revision ID of the revision that originated the slot's content. + -- To find revisions that changed slots, look for slot_origin = slot_revision_id. + slot_origin bigint unsigned NOT NULL, PRIMARY KEY ( slot_revision_id, slot_role_id ) ) /*$wgDBTableOptions*/; -- Index for finding revisions that modified a specific slot -CREATE INDEX /*i*/slot_role_inherited ON /*_*/slots (slot_revision_id, slot_role_id, slot_inherited); \ No newline at end of file +CREATE INDEX /*i*/slot_revision_origin_role ON /*_*/slots (slot_revision_id, slot_origin, slot_role_id); diff --git a/maintenance/mssql/archives/patch-slot-origin.sql b/maintenance/mssql/archives/patch-slot-origin.sql new file mode 100644 index 0000000000..bba3be4cdc --- /dev/null +++ b/maintenance/mssql/archives/patch-slot-origin.sql @@ -0,0 +1,14 @@ +-- +-- Replace slot_inherited with slot_origin. +-- +-- NOTE: There is no release that has slot_inherited. This is only needed to transition between +-- snapshot versions of 1.30. +-- +-- NOTE: No code that writes to the slots table was merge yet, the table is assumed to be empty. +-- +DROP INDEX /*i*/slot_role_inherited ON /*_*/slots; + +ALTER TABLE /*_*/slots DROP CONSTRAINT DF_slot_inherited, COLUMN slot_inherited; +ALTER TABLE /*_*/slots ADD COLUMN slot_origin bigint NOT NULL; + +CREATE INDEX /*i*/slot_revision_origin_role ON /*_*/slots (slot_revision_id, slot_origin, slot_role_id); diff --git a/maintenance/mssql/archives/patch-slots.sql b/maintenance/mssql/archives/patch-slots.sql index 91d3168238..e9ec7c3188 100644 --- a/maintenance/mssql/archives/patch-slots.sql +++ b/maintenance/mssql/archives/patch-slots.sql @@ -14,11 +14,12 @@ CREATE TABLE /*_*/slots ( -- reference to content_id slot_content_id bigint unsigned NOT NULL CONSTRAINT FK_slots_content_id FOREIGN KEY REFERENCES content(content_id), - -- whether the content is inherited (1) or new in this revision (0) - slot_inherited tinyint unsigned NOT NULL CONSTRAINT DF_slot_inherited DEFAULT 0, + -- The revision ID of the revision that originated the slot's content. + -- To find revisions that changed slots, look for slot_origin = slot_revision_id. + slot_origin bigint unsigned NOT NULL, CONSTRAINT PK_slots PRIMARY KEY (slot_revision_id, slot_role_id) ); -- Index for finding revisions that modified a specific slot -CREATE INDEX /*i*/slot_role_inherited ON /*_*/slots (slot_revision_id, slot_role_id, slot_inherited); \ No newline at end of file +CREATE INDEX /*i*/slot_revision_origin_role ON /*_*/slots (slot_revision_id, slot_origin, slot_role_id); diff --git a/maintenance/mssql/tables.sql b/maintenance/mssql/tables.sql index 5348c47d9b..b948772de4 100644 --- a/maintenance/mssql/tables.sql +++ b/maintenance/mssql/tables.sql @@ -311,14 +311,15 @@ CREATE TABLE /*_*/slots ( -- reference to content_id slot_content_id bigint unsigned NOT NULL CONSTRAINT FK_slots_content_id FOREIGN KEY REFERENCES content(content_id), - -- whether the content is inherited (1) or new in this revision (0) - slot_inherited tinyint unsigned NOT NULL CONSTRAINT DF_slot_inherited DEFAULT 0, + -- The revision ID of the revision that originated the slot's content. + -- To find revisions that changed slots, look for slot_origin = slot_revision_id. + slot_origin bigint NOT NULL, CONSTRAINT PK_slots PRIMARY KEY (slot_revision_id, slot_role_id) ); -- Index for finding revisions that modified a specific slot -CREATE INDEX /*i*/slot_role_inherited ON /*_*/slots (slot_revision_id, slot_role_id, slot_inherited); +CREATE INDEX /*i*/slot_revision_origin_role ON /*_*/slots (slot_revision_id, slot_origin, slot_role_id); -- -- The content table represents content objects. It's primary purpose is to provide the necessary diff --git a/maintenance/oracle/archives/patch-slot-origin.sql b/maintenance/oracle/archives/patch-slot-origin.sql new file mode 100644 index 0000000000..1b398cd50c --- /dev/null +++ b/maintenance/oracle/archives/patch-slot-origin.sql @@ -0,0 +1,14 @@ +-- +-- Replace slot_inherited with slot_origin. +-- +-- NOTE: There is no release that has slot_inherited. This is only needed to transition between +-- snapshot versions of 1.30. +-- +-- NOTE: No code that writes to the slots table was merge yet, the table is assumed to be empty. +-- +DROP INDEX &mw_prefix.slot_role_inherited; + +ALTER TABLE &mw_prefix.slots DROP COLUMN slot_inherited; +ALTER TABLE &mw_prefix.slots ADD ( slot_origin NUMBER NOT NULL ); + +CREATE INDEX &mw_prefix.slot_revision_origin_role ON &mw_prefix.slots (slot_revision_id, slot_origin, slot_role_id); diff --git a/maintenance/oracle/archives/patch-slots.sql b/maintenance/oracle/archives/patch-slots.sql index 094ab68dca..fde35d5a0a 100644 --- a/maintenance/oracle/archives/patch-slots.sql +++ b/maintenance/oracle/archives/patch-slots.sql @@ -2,9 +2,9 @@ CREATE TABLE &mw_prefix.slots ( slot_revision_id NUMBER NOT NULL, slot_role_id NUMBER NOT NULL, slot_content_id NUMBER NOT NULL, - slot_inherited CHAR(1) DEFAULT '0' NOT NULL + slot_origin NUMBER NOT NULL ); ALTER TABLE &mw_prefix.slots ADD CONSTRAINT &mw_prefix.slots_pk PRIMARY KEY (slot_revision_id, slot_role_id); -CREATE INDEX &mw_prefix.slot_role_inherited ON &mw_prefix.slots (slot_revision_id, slot_role_id, slot_inherited); \ No newline at end of file +CREATE INDEX &mw_prefix.slot_revision_origin_role ON &mw_prefix.slots (slot_revision_id, slot_origin, slot_role_id); diff --git a/maintenance/oracle/tables.sql b/maintenance/oracle/tables.sql index 110188d300..058ef15321 100644 --- a/maintenance/oracle/tables.sql +++ b/maintenance/oracle/tables.sql @@ -287,12 +287,12 @@ CREATE TABLE &mw_prefix.slots ( slot_revision_id NUMBER NOT NULL, slot_role_id NUMBER NOT NULL, slot_content_id NUMBER NOT NULL, - slot_inherited CHAR(1) DEFAULT '0' NOT NULL + slot_origin NUMBER NOT NULL ); ALTER TABLE &mw_prefix.slots ADD CONSTRAINT &mw_prefix.slots_pk PRIMARY KEY (slot_revision_id, slot_role_id); -CREATE INDEX &mw_prefix.slot_role_inherited ON &mw_prefix.slots (slot_revision_id, slot_role_id, slot_inherited); +CREATE INDEX &mw_prefix.slot_revision_origin_role ON &mw_prefix.slots (slot_revision_id, slot_origin, slot_role_id); CREATE SEQUENCE content_content_id_seq; diff --git a/maintenance/postgres/archives/patch-slots-table.sql b/maintenance/postgres/archives/patch-slots-table.sql index 9cad0d19b8..514921f41d 100644 --- a/maintenance/postgres/archives/patch-slots-table.sql +++ b/maintenance/postgres/archives/patch-slots-table.sql @@ -2,8 +2,8 @@ CREATE TABLE slots ( slot_revision_id INTEGER NOT NULL, slot_role_id SMALLINT NOT NULL, slot_content_id INTEGER NOT NULL, - slot_inherited SMALLINT NOT NULL DEFAULT 0, + slot_origin INTEGER NOT NULL, PRIMARY KEY (slot_revision_id, slot_role_id) ); -CREATE INDEX slot_role_inherited ON slots (slot_revision_id, slot_role_id, slot_inherited); \ No newline at end of file +CREATE INDEX slot_revision_origin_role ON slots (slot_revision_id, slot_origin, slot_role_id); diff --git a/maintenance/postgres/tables.sql b/maintenance/postgres/tables.sql index 01177d86b7..1e1c434a47 100644 --- a/maintenance/postgres/tables.sql +++ b/maintenance/postgres/tables.sql @@ -264,11 +264,11 @@ CREATE TABLE slots ( slot_revision_id INTEGER NOT NULL, slot_role_id SMALLINT NOT NULL, slot_content_id INTEGER NOT NULL, - slot_inherited SMALLINT NOT NULL DEFAULT 0, + slot_origin INTEGER NOT NULL, PRIMARY KEY (slot_revision_id, slot_role_id) ); -CREATE INDEX slot_role_inherited ON slots (slot_revision_id, slot_role_id, slot_inherited); +CREATE INDEX slot_revision_origin_role ON slots (slot_revision_id, slot_origin, slot_role_id); CREATE SEQUENCE content_content_id_seq; diff --git a/maintenance/sqlite/archives/patch-slot-origin.sql b/maintenance/sqlite/archives/patch-slot-origin.sql new file mode 100644 index 0000000000..f6d8ebf870 --- /dev/null +++ b/maintenance/sqlite/archives/patch-slot-origin.sql @@ -0,0 +1,34 @@ +-- +-- Replace slot_inherited with slot_origin. +-- +-- NOTE: There is no release that has slot_inherited. This is only needed to transition between +-- snapshot versions of 1.30. +-- +-- NOTE: No code that writes to the slots table was merge yet, the table is assumed to be empty. +-- +BEGIN TRANSACTION; + +DROP TABLE /*_*/slots; + +CREATE TABLE /*_*/slots ( + + -- reference to rev_id + slot_revision_id bigint unsigned NOT NULL, + + -- reference to role_id + slot_role_id smallint unsigned NOT NULL, + + -- reference to content_id + slot_content_id bigint unsigned NOT NULL, + + -- The revision ID of the revision that originated the slot's content. + -- To find revisions that changed slots, look for slot_origin = slot_revision_id. + slot_origin bigint unsigned NOT NULL, + + PRIMARY KEY ( slot_revision_id, slot_role_id ) +) /*$wgDBTableOptions*/; + +-- Index for finding revisions that modified a specific slot +CREATE INDEX /*i*/slot_revision_origin_role ON /*_*/slots (slot_revision_id, slot_origin, slot_role_id); + +COMMIT TRANSACTION; \ No newline at end of file diff --git a/maintenance/tables.sql b/maintenance/tables.sql index d633a9c209..4120a64d5d 100644 --- a/maintenance/tables.sql +++ b/maintenance/tables.sql @@ -676,14 +676,15 @@ CREATE TABLE /*_*/slots ( -- reference to content_id slot_content_id bigint unsigned NOT NULL, - -- whether the content is inherited (1) or new in this revision (0) - slot_inherited tinyint unsigned NOT NULL DEFAULT 0, + -- The revision ID of the revision that originated the slot's content. + -- To find revisions that changed slots, look for slot_origin = slot_revision_id. + slot_origin bigint unsigned NOT NULL, PRIMARY KEY ( slot_revision_id, slot_role_id ) ) /*$wgDBTableOptions*/; -- Index for finding revisions that modified a specific slot -CREATE INDEX /*i*/slot_role_inherited ON /*_*/slots (slot_revision_id, slot_role_id, slot_inherited); +CREATE INDEX /*i*/slot_revision_origin_role ON /*_*/slots (slot_revision_id, slot_origin, slot_role_id); -- -- The content table represents content objects. It's primary purpose is to provide the necessary diff --git a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php index c713e2c70b..719a3bf67c 100644 --- a/tests/phpunit/includes/Storage/RevisionStoreDbTest.php +++ b/tests/phpunit/includes/Storage/RevisionStoreDbTest.php @@ -412,6 +412,8 @@ class RevisionStoreDbTest extends MediaWikiTestCase { public function testNewNullRevision( Title $title, $comment, $minor ) { $store = MediaWikiServices::getInstance()->getRevisionStore(); $user = TestUserRegistry::getMutableTestUser( __METHOD__ )->getUser(); + + $parent = $store->getRevisionByTitle( $title ); $record = $store->newNullRevision( wfGetDB( DB_MASTER ), $title, @@ -425,6 +427,14 @@ class RevisionStoreDbTest extends MediaWikiTestCase { $this->assertEquals( $comment, $record->getComment() ); $this->assertEquals( $minor, $record->isMinor() ); $this->assertEquals( $user->getName(), $record->getUser()->getName() ); + $this->assertEquals( $parent->getId(), $record->getParentId() ); + + $parentSlot = $parent->getSlot( 'main' ); + $slot = $record->getSlot( 'main' ); + + $this->assertTrue( $slot->isInherited(), 'isInherited' ); + $this->assertSame( $parentSlot->getOrigin(), $slot->getOrigin(), 'getOrigin' ); + $this->assertSame( $parentSlot->getAddress(), $slot->getAddress(), 'getAddress' ); } /** diff --git a/tests/phpunit/includes/Storage/SlotRecordTest.php b/tests/phpunit/includes/Storage/SlotRecordTest.php index ef313156f8..8f26494dc6 100644 --- a/tests/phpunit/includes/Storage/SlotRecordTest.php +++ b/tests/phpunit/includes/Storage/SlotRecordTest.php @@ -25,7 +25,7 @@ class SlotRecordTest extends MediaWikiTestCase { 'model_name' => CONTENT_MODEL_WIKITEXT, 'format_name' => CONTENT_FORMAT_WIKITEXT, 'slot_revision_id' => '2', - 'slot_inherited' => '1', + 'slot_origin' => '1', 'role_name' => 'myRole', ]; return (object)$data; @@ -43,6 +43,7 @@ class SlotRecordTest extends MediaWikiTestCase { $this->assertSame( 'someHash', $record->getSha1() ); $this->assertSame( CONTENT_MODEL_WIKITEXT, $record->getModel() ); $this->assertSame( 2, $record->getRevision() ); + $this->assertSame( 1, $record->getOrigin() ); $this->assertSame( 'tt:456', $record->getAddress() ); $this->assertSame( 33, $record->getContentId() ); $this->assertSame( CONTENT_FORMAT_WIKITEXT, $record->getFormat() ); @@ -56,7 +57,8 @@ class SlotRecordTest extends MediaWikiTestCase { 'format_name' => function () { return CONTENT_FORMAT_WIKITEXT; }, - 'slot_inherited' => '0' + 'slot_revision_id' => '2', + 'slot_origin' => '2', ] ); $content = function () { @@ -73,6 +75,7 @@ class SlotRecordTest extends MediaWikiTestCase { $this->assertNotNull( $record->getSha1() ); $this->assertSame( CONTENT_MODEL_WIKITEXT, $record->getModel() ); $this->assertSame( 2, $record->getRevision() ); + $this->assertSame( 2, $record->getRevision() ); $this->assertSame( 'tt:456', $record->getAddress() ); $this->assertSame( 33, $record->getContentId() ); $this->assertSame( CONTENT_FORMAT_WIKITEXT, $record->getFormat() ); @@ -122,13 +125,35 @@ class SlotRecordTest extends MediaWikiTestCase { $record->getAddress(); } - public function testGetRevision_fails() { + public function provideIncomplete() { + $unsaved = SlotRecord::newUnsaved( 'main', new WikitextContent( 'A' ) ); + yield 'unsaved' => [ $unsaved ]; + + $parent = new SlotRecord( $this->makeRow(), new WikitextContent( 'A' ) ); + $inherited = SlotRecord::newInherited( $parent ); + yield 'inherited' => [ $inherited ]; + } + + /** + * @dataProvider provideIncomplete + */ + public function testGetRevision_fails( SlotRecord $record ) { $record = SlotRecord::newUnsaved( 'main', new WikitextContent( 'A' ) ); $this->setExpectedException( IncompleteRevisionException::class ); $record->getRevision(); } + /** + * @dataProvider provideIncomplete + */ + public function testGetOrigin_fails( SlotRecord $record ) { + $record = SlotRecord::newUnsaved( 'main', new WikitextContent( 'A' ) ); + $this->setExpectedException( IncompleteRevisionException::class ); + + $record->getOrigin(); + } + public function provideHashStability() { yield [ '', 'phoiac9h4m842xq45sp7s6u21eteeq1' ]; yield [ 'Lorem ipsum', 'hcr5u40uxr81d3nx89nvwzclfz6r9c5' ]; @@ -155,7 +180,7 @@ class SlotRecordTest extends MediaWikiTestCase { } public function testNewInherited() { - $row = $this->makeRow( [ 'slot_revision_id' => 7, 'slot_inherited' => 0 ] ); + $row = $this->makeRow( [ 'slot_revision_id' => 7, 'slot_origin' => 7 ] ); $parent = new SlotRecord( $row, new WikitextContent( 'A' ) ); // This would happen while doing an edit, before saving revision meta-data. @@ -205,6 +230,7 @@ class SlotRecordTest extends MediaWikiTestCase { $this->assertSame( 20, $saved->getContentId() ); $this->assertSame( 'A', $saved->getContent()->getNativeData() ); $this->assertSame( 10, $saved->getRevision() ); + $this->assertSame( 10, $saved->getOrigin() ); // make sure we didn't mess with the internal state of $unsaved $this->assertFalse( $unsaved->hasAddress() ); @@ -215,7 +241,7 @@ class SlotRecordTest extends MediaWikiTestCase { $freshRow = $this->makeRow( [ 'content_id' => 10, 'content_address' => 'address:1', - 'slot_inherited' => 0, + 'slot_origin' => 1, 'slot_revision_id' => 1, ] ); @@ -227,7 +253,8 @@ class SlotRecordTest extends MediaWikiTestCase { $inheritedRow = $this->makeRow( [ 'content_id' => null, 'content_address' => null, - 'slot_inherited' => 1 + 'slot_origin' => 0, + 'slot_revision_id' => 1, ] ); $inheritedSlot = new SlotRecord( $inheritedRow, new WikitextContent( 'A' ) ); -- 2.20.1