Allow extra slots in write-both/read-new mode.
authordaniel <daniel.kinzler@wikimedia.de>
Thu, 28 Jun 2018 18:32:03 +0000 (20:32 +0200)
committerdaniel <daniel.kinzler@wikimedia.de>
Mon, 2 Jul 2018 17:44:05 +0000 (19:44 +0200)
Bug: T198413
Change-Id: I3125d5c7ef8d12936d4332cd1602f2eaab2482b2

includes/Storage/PageUpdater.php
includes/Storage/RevisionStore.php
tests/phpunit/includes/Storage/McrReadNewRevisionStoreDbTest.php
tests/phpunit/includes/Storage/McrRevisionStoreDbTest.php
tests/phpunit/includes/Storage/McrWriteBothRevisionStoreDbTest.php
tests/phpunit/includes/Storage/PreMcrRevisionStoreDbTest.php

index 7900210..d6330e8 100644 (file)
@@ -343,6 +343,10 @@ class PageUpdater {
                // TODO: MCR: check the role and the content's model against the list of supported
                // roles, see T194046.
 
+               if ( $role !== 'main' ) {
+                       throw new InvalidArgumentException( 'Only the main slot is presently supported' );
+               }
+
                $this->slotsUpdate->modifyContent( $role, $content );
        }
 
index 07329c9..c92752c 100644 (file)
@@ -412,10 +412,17 @@ class RevisionStore
                        );
                }
 
-               // While inserting into the old schema make sure only the main slot is allowed.
-               if ( $this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_OLD ) && $slotRoles !== [ 'main' ] ) {
+               // If we are not writing into the new schema, we can't support extra slots.
+               if ( !$this->hasMcrSchemaFlags( SCHEMA_COMPAT_WRITE_NEW ) && $slotRoles !== [ 'main' ] ) {
                        throw new InvalidArgumentException(
-                               'Only the main slot is supported when writing to the pre-MCR schema!'
+                               'Only the main slot is supported when not writing to the MCR enabled schema!'
+                       );
+               }
+
+               // As long as we are not reading from the new schema, we don't want to write extra slots.
+               if ( !$this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_NEW ) && $slotRoles !== [ 'main' ] ) {
+                       throw new InvalidArgumentException(
+                               'Only the main slot is supported when not reading from the MCR enabled schema!'
                        );
                }
 
index 0c8094c..cbae4c7 100644 (file)
@@ -1,11 +1,12 @@
 <?php
 namespace MediaWiki\Tests\Storage;
 
-use InvalidArgumentException;
+use CommentStoreComment;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Storage\RevisionRecord;
 use MediaWiki\Storage\SlotRecord;
 use TextContent;
+use Title;
 use WikitextContent;
 
 /**
@@ -23,18 +24,38 @@ class McrReadNewRevisionStoreDbTest extends RevisionStoreDbTestBase {
        use McrReadNewSchemaOverride;
 
        protected function assertRevisionExistsInDatabase( RevisionRecord $rev ) {
+               $numberOfSlots = count( $rev->getSlotRoles() );
+
+               // new schema is written
                $this->assertSelect(
                        'slots',
                        [ 'count(*)' ],
                        [ 'slot_revision_id' => $rev->getId() ],
-                       [ [ '1' ] ]
+                       [ [ (string)$numberOfSlots ] ]
                );
 
+               $store = MediaWikiServices::getInstance()->getRevisionStore();
+               $revQuery = $store->getSlotsQueryInfo( [ 'content' ] );
+
                $this->assertSelect(
-                       'content',
+                       $revQuery['tables'],
                        [ 'count(*)' ],
-                       [ 'content_address' => $rev->getSlot( 'main' )->getAddress() ],
-                       [ [ '1' ] ]
+                       [
+                               'slot_revision_id' => $rev->getId(),
+                       ],
+                       [ [ (string)$numberOfSlots ] ],
+                       [],
+                       $revQuery['joins']
+               );
+
+               // Legacy schema is still being written
+               $this->assertSelect(
+                       [ 'revision', 'text' ],
+                       [ 'count(*)' ],
+                       [ 'rev_id' => $rev->getId(), 'rev_text_id > 0' ],
+                       [ [ 1 ] ],
+                       [],
+                       [ 'text' => [ 'INNER JOIN', [ 'rev_text_id = old_id' ] ] ]
                );
 
                parent::assertRevisionExistsInDatabase( $rev );
@@ -51,6 +72,42 @@ class McrReadNewRevisionStoreDbTest extends RevisionStoreDbTestBase {
                $this->assertSame( $a->getContentId(), $b->getContentId() );
        }
 
+       public function provideInsertRevisionOn_successes() {
+               foreach ( parent::provideInsertRevisionOn_successes() as $case ) {
+                       yield $case;
+               }
+
+               yield 'Multi-slot revision insertion' => [
+                       [
+                               'content' => [
+                                       'main' => new WikitextContent( 'Chicken' ),
+                                       'aux' => new TextContent( 'Egg' ),
+                               ],
+                               'page' => true,
+                               'comment' => $this->getRandomCommentStoreComment(),
+                               'timestamp' => '20171117010101',
+                               'user' => true,
+                       ],
+               ];
+       }
+
+       public function provideNewNullRevision() {
+               foreach ( parent::provideNewNullRevision() as $case ) {
+                       yield $case;
+               }
+
+               yield [
+                       Title::newFromText( 'UTPage_notAutoCreated' ),
+                       [
+                               'content' => [
+                                       'main' => new WikitextContent( 'Chicken' ),
+                                       'aux' => new WikitextContent( 'Omelet' ),
+                               ],
+                       ],
+                       CommentStoreComment::newUnsavedComment( __METHOD__ . ' comment multi' ),
+               ];
+       }
+
        public function testGetQueryInfo_NoSlotDataJoin() {
                $store = MediaWikiServices::getInstance()->getRevisionStore();
                $queryInfo = $store->getQueryInfo();
@@ -166,25 +223,6 @@ class McrReadNewRevisionStoreDbTest extends RevisionStoreDbTestBase {
                ];
        }
 
-       public function provideInsertRevisionOn_failures() {
-               foreach ( parent::provideInsertRevisionOn_failures() as $case ) {
-                       yield $case;
-               }
-
-               yield 'slot that is not main slot' => [
-                       [
-                               'content' => [
-                                       'main' => new WikitextContent( 'Chicken' ),
-                                       'lalala' => new WikitextContent( 'Duck' ),
-                               ],
-                               'comment' => $this->getRandomCommentStoreComment(),
-                               'timestamp' => '20171117010101',
-                               'user' => true,
-                       ],
-                       new InvalidArgumentException( 'Only the main slot is supported' )
-               ];
-       }
-
        public function provideNewMutableRevisionFromArray() {
                foreach ( parent::provideNewMutableRevisionFromArray() as $case ) {
                        yield $case;
index 5bf49d3..9a118d7 100644 (file)
@@ -26,6 +26,7 @@ class McrRevisionStoreDbTest extends RevisionStoreDbTestBase {
        protected function assertRevisionExistsInDatabase( RevisionRecord $rev ) {
                $numberOfSlots = count( $rev->getSlotRoles() );
 
+               // new schema is written
                $this->assertSelect(
                        'slots',
                        [ 'count(*)' ],
@@ -47,13 +48,6 @@ class McrRevisionStoreDbTest extends RevisionStoreDbTestBase {
                        $revQuery['joins']
                );
 
-               $this->assertSelect(
-                       'content',
-                       [ 'count(*)' ],
-                       [ 'content_address' => $rev->getSlot( 'main' )->getAddress() ],
-                       [ [ 1 ] ]
-               );
-
                parent::assertRevisionExistsInDatabase( $rev );
        }
 
index c984142..2653c14 100644 (file)
@@ -32,6 +32,7 @@ class McrWriteBothRevisionStoreDbTest extends RevisionStoreDbTestBase {
        }
 
        protected function assertRevisionExistsInDatabase( RevisionRecord $rev ) {
+               // New schema is being written
                $this->assertSelect(
                        'slots',
                        [ 'count(*)' ],
@@ -46,6 +47,16 @@ class McrWriteBothRevisionStoreDbTest extends RevisionStoreDbTestBase {
                        [ [ '1' ] ]
                );
 
+               // Legacy schema is still being written
+               $this->assertSelect(
+                       [ 'revision', 'text' ],
+                       [ 'count(*)' ],
+                       [ 'rev_id' => $rev->getId(), 'rev_text_id > 0' ],
+                       [ [ 1 ] ],
+                       [],
+                       [ 'text' => [ 'INNER JOIN', [ 'rev_text_id = old_id' ] ] ]
+               );
+
                parent::assertRevisionExistsInDatabase( $rev );
        }
 
index a27d2bb..e625dd0 100644 (file)
@@ -2,6 +2,7 @@
 namespace MediaWiki\Tests\Storage;
 
 use InvalidArgumentException;
+use MediaWiki\Storage\RevisionRecord;
 use Revision;
 use WikitextContent;
 
@@ -29,6 +30,20 @@ class PreMcrRevisionStoreDbTest extends RevisionStoreDbTestBase {
                return $row;
        }
 
+       protected function assertRevisionExistsInDatabase( RevisionRecord $rev ) {
+               // Legacy schema is still being written
+               $this->assertSelect(
+                       [ 'revision', 'text' ],
+                       [ 'count(*)' ],
+                       [ 'rev_id' => $rev->getId(), 'rev_text_id > 0' ],
+                       [ [ 1 ] ],
+                       [],
+                       [ 'text' => [ 'INNER JOIN', [ 'rev_text_id = old_id' ] ] ]
+               );
+
+               parent::assertRevisionExistsInDatabase( $rev );
+       }
+
        public function provideGetArchiveQueryInfo() {
                yield [
                        [