Merge "Allow extra slots in write-both/read-new mode."

diff --combined includes/Storage/PageUpdater.php
index 2376d16,d6330e8..67928f9
--- 1/includes/Storage/PageUpdater.php
--- 2/includes/Storage/PageUpdater.php
+++ b/includes/Storage/PageUpdater.php
@@@ -45,7 -45,6 +45,7 @@@ use User
  use Wikimedia\Assert\Assert;
  use Wikimedia\Rdbms\DBConnRef;
  use Wikimedia\Rdbms\DBUnexpectedError;
 +use Wikimedia\Rdbms\IDatabase;
  use Wikimedia\Rdbms\LoadBalancer;
  use WikiPage;
  
@@@ -344,6 -343,10 +344,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 );
        }
  
@@@ -640,7 -643,7 +644,7 @@@
                // NOTE: This grabs the parent revision as the CAS token, if grabParentRevision
                // wasn't called yet. If the page is modified by another process before we are done with
                // it, this method must fail (with status 'edit-conflict')!
 -              // NOTE: The parent revision may be different from $this->baseRevisionId.
 +              // NOTE: The parent revision may be different from $this->originalRevisionId.
                $this->grabParentRevision();
                $flags = $this->checkFlags( $flags );
  
@@@ -923,6 -926,7 +927,6 @@@
  
                // XXX: we may want a flag that allows a null revision to be forced!
                $changed = $this->derivedDataUpdater->isChange();
 -              $mainContent = $newRevisionRecord->getContent( 'main' );
  
                $dbw = $this->getDBConnectionRef( DB_MASTER );
  
@@@ -991,19 -995,22 +995,19 @@@
                        $user->incEditCount();
  
                        $dbw->endAtomic( __METHOD__ );
 -              } else {
 -                      // T34948: revision ID must be set to page {{REVISIONID}} and
 -                      // related variables correctly. Likewise for {{REVISIONUSER}} (T135261).
 -                      // Since we don't insert a new revision into the database, the least
 -                      // error-prone way is to reuse given old revision.
 -                      $newRevisionRecord = $oldRev;
 -                      $newLegacyRevision = new Revision( $newRevisionRecord );
 -              }
  
 -              if ( $changed ) {
                        // Return the new revision to the caller
                        $status->value['revision-record'] = $newRevisionRecord;
  
                        // TODO: globally replace usages of 'revision' with getNewRevision()
                        $status->value['revision'] = $newLegacyRevision;
                } else {
 +                      // T34948: revision ID must be set to page {{REVISIONID}} and
 +                      // related variables correctly. Likewise for {{REVISIONUSER}} (T135261).
 +                      // Since we don't insert a new revision into the database, the least
 +                      // error-prone way is to reuse given old revision.
 +                      $newRevisionRecord = $oldRev;
 +
                        $status->warning( 'edit-no-change' );
                        // Update page_touched as updateRevisionOn() was not called.
                        // Other cache updates are managed in WikiPage::onArticleEdit()
@@@ -1018,15 -1025,30 +1022,15 @@@
                // importantly, before the parser cache has been updated. This would cause the
                // content to be parsed a second time, or may cause stale content to be shown.
                DeferredUpdates::addUpdate(
 -                      new AtomicSectionUpdate(
 +                      $this->getAtomicSectionUpdate(
                                $dbw,
 -                              __METHOD__,
 -                              function () use (
 -                                      $wikiPage, $newRevisionRecord, $newLegacyRevision, $user, $mainContent,
 -                                      $summary, $flags, $changed, $status
 -                              ) {
 -                                      // Update links tables, site stats, etc.
 -                                      $this->derivedDataUpdater->prepareUpdate(
 -                                              $newRevisionRecord,
 -                                              [
 -                                                      'changed' => $changed,
 -                                              ]
 -                                      );
 -                                      $this->derivedDataUpdater->doUpdates();
 -
 -                                      // Trigger post-save hook
 -                                      // TODO: replace legacy hook!
 -                                      // TODO: avoid pass-by-reference, see T193950
 -                                      $params = [ &$wikiPage, &$user, $mainContent, $summary->text, $flags & EDIT_MINOR,
 -                                              null, null, &$flags, $newLegacyRevision, &$status, $this->getOriginalRevisionId(),
 -                                              $this->undidRevId ];
 -                                      Hooks::run( 'PageContentSaveComplete', $params );
 -                              }
 +                              $wikiPage,
 +                              $newRevisionRecord,
 +                              $user,
 +                              $summary,
 +                              $flags,
 +                              $status,
 +                              [ 'changed' => $changed, ]
                        ),
                        DeferredUpdates::PRESEND
                );
@@@ -1144,65 -1166,47 +1148,65 @@@
                $status->value['revision'] = $newLegacyRevision;
                $status->value['revision-record'] = $newRevisionRecord;
  
 -              // XXX: make sure we are not loading the Content from the DB
 -              $mainContent = $newRevisionRecord->getContent( 'main' );
 -
                // Do secondary updates once the main changes have been committed...
                DeferredUpdates::addUpdate(
 -                      new AtomicSectionUpdate(
 +                      $this->getAtomicSectionUpdate(
                                $dbw,
 -                              __METHOD__,
 -                              function () use (
 -                                      $wikiPage,
 -                                      $newRevisionRecord,
 -                                      $newLegacyRevision,
 -                                      $user,
 -                                      $mainContent,
 -                                      $summary,
 -                                      $flags,
 -                                      $status
 -                              ) {
 -                                      // Update links, etc.
 -                                      $this->derivedDataUpdater->prepareUpdate(
 -                                              $newRevisionRecord,
 -                                              [ 'created' => true ]
 -                                      );
 -                                      $this->derivedDataUpdater->doUpdates();
 +                              $wikiPage,
 +                              $newRevisionRecord,
 +                              $user,
 +                              $summary,
 +                              $flags,
 +                              $status,
 +                              [ 'created' => true ]
 +                      ),
 +                      DeferredUpdates::PRESEND
 +              );
  
 +              return $status;
 +      }
 +
 +      private function getAtomicSectionUpdate(
 +              IDatabase $dbw,
 +              WikiPage $wikiPage,
 +              RevisionRecord $newRevisionRecord,
 +              User $user,
 +              CommentStoreComment $summary,
 +              $flags,
 +              Status $status,
 +              $hints = []
 +      ) {
 +              return new AtomicSectionUpdate(
 +                      $dbw,
 +                      __METHOD__,
 +                      function () use (
 +                              $wikiPage, $newRevisionRecord, $user,
 +                              $summary, $flags, $status, $hints
 +                      ) {
 +                              $newLegacyRevision = new Revision( $newRevisionRecord );
 +                              $mainContent = $newRevisionRecord->getContent( 'main', RevisionRecord::RAW );
 +
 +                              // Update links tables, site stats, etc.
 +                              $this->derivedDataUpdater->prepareUpdate( $newRevisionRecord, $hints );
 +                              $this->derivedDataUpdater->doUpdates();
 +
 +                              // TODO: replace legacy hook!
 +                              // TODO: avoid pass-by-reference, see T193950
 +
 +                              if ( $hints['created'] ?? false ) {
                                        // Trigger post-create hook
 -                                      // TODO: replace legacy hook!
 -                                      // TODO: avoid pass-by-reference, see T193950
                                        $params = [ &$wikiPage, &$user, $mainContent, $summary->text,
                                                $flags & EDIT_MINOR, null, null, &$flags, $newLegacyRevision ];
                                        Hooks::run( 'PageContentInsertComplete', $params );
 -                                      // Trigger post-save hook
 -                                      // TODO: replace legacy hook!
 -                                      $params = array_merge( $params, [ &$status, $this->getOriginalRevisionId(), 0 ] );
 -                                      Hooks::run( 'PageContentSaveComplete', $params );
                                }
 -                      ),
 -                      DeferredUpdates::PRESEND
 -              );
  
 -              return $status;
 +                              // Trigger post-save hook
 +                              $params = [ &$wikiPage, &$user, $mainContent, $summary->text,
 +                                              $flags & EDIT_MINOR, null, null, &$flags, $newLegacyRevision,
 +                                              &$status, $this->getOriginalRevisionId(), $this->undidRevId ];
 +                              Hooks::run( 'PageContentSaveComplete', $params );
 +                      }
 +              );
        }
  
  }

diff --combined includes/Storage/RevisionStore.php
index 119af1d,c92752c..3e779fb
--- 1/includes/Storage/RevisionStore.php
--- 2/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@@ -83,7 -83,6 +83,7 @@@ class RevisionStor
  
        /**
         * @var boolean
 +       * @see $wgContentHandlerUseDB
         */
        private $contentHandlerUseDB = true;
  
@@@ -221,7 -220,6 +221,7 @@@
        }
  
        /**
 +       * @see $wgContentHandlerUseDB
         * @param bool $contentHandlerUseDB
         * @throws MWException
         */
@@@ -414,10 -412,17 +414,17 @@@
                        );
                }
  
-               // 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!'
                        );
                }
  

diff --combined tests/phpunit/includes/Storage/McrWriteBothRevisionStoreDbTest.php
index f03d403,2653c14..9d5dc29
--- 1/tests/phpunit/includes/Storage/McrWriteBothRevisionStoreDbTest.php
--- 2/tests/phpunit/includes/Storage/McrWriteBothRevisionStoreDbTest.php
+++ b/tests/phpunit/includes/Storage/McrWriteBothRevisionStoreDbTest.php
@@@ -32,6 -32,7 +32,7 @@@ class McrWriteBothRevisionStoreDbTest e
        }
  
        protected function assertRevisionExistsInDatabase( RevisionRecord $rev ) {
+               // New schema is being written
                $this->assertSelect(
                        'slots',
                        [ 'count(*)' ],
@@@ -46,6 -47,16 +47,16 @@@
                        [ [ '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 );
        }
  
@@@ -159,8 -170,8 +170,8 @@@
                                                'role_name' => $db->addQuotes( 'main' ),
                                                'content_size' => 'slots.rev_len',
                                                'content_sha1' => 'slots.rev_sha1',
 -                                              'content_address' =>
 -                                                      'CONCAT(' . $db->addQuotes( 'tt:' ) . ',slots.rev_text_id)',
 +                                              'content_address' => $db->buildConcat( [
 +                                                      $db->addQuotes( 'tt:' ), 'slots.rev_text_id' ] ),
                                                'model_name' => 'slots.rev_content_model',
                                        ]
                                ),

diff --combined tests/phpunit/includes/Storage/PreMcrRevisionStoreDbTest.php
index 7cc3131,e625dd0..cee5f96
--- 1/tests/phpunit/includes/Storage/PreMcrRevisionStoreDbTest.php
--- 2/tests/phpunit/includes/Storage/PreMcrRevisionStoreDbTest.php
+++ b/tests/phpunit/includes/Storage/PreMcrRevisionStoreDbTest.php
@@@ -2,6 -2,7 +2,7 @@@
  namespace MediaWiki\Tests\Storage;
  
  use InvalidArgumentException;
+ use MediaWiki\Storage\RevisionRecord;
  use Revision;
  use WikitextContent;
  
@@@ -29,6 -30,20 +30,20 @@@ class PreMcrRevisionStoreDbTest extend
                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 [
                        [
@@@ -130,7 -145,7 +145,7 @@@
                                                'content_size' => 'slots.rev_len',
                                                'content_sha1' => 'slots.rev_sha1',
                                                'content_address' =>
 -                                                      'CONCAT(' . $db->addQuotes( 'tt:' ) . ',slots.rev_text_id)',
 +                                                      $db->buildConcat( [ $db->addQuotes( 'tt:' ), 'slots.rev_text_id' ] ),
                                                'model_name' => 'slots.rev_content_model',
                                        ]
                                ),