Reuse DerivedPageDataUpdater during null-edits.

Checking the acting user against the revision's author in
DerivedPageDataUpdater::isReusableFor would lead to false
negatives.

Also removes a check of the current acting user against the
cached revision's author, for the same reason: for null edits,
the acting user and the revision author are unrelated.

Bug: T205369
Change-Id: I48f59dce6c25062b3d6ff4248e1171269766c507

diff --git a/includes/Storage/DerivedPageDataUpdater.php b/includes/Storage/DerivedPageDataUpdater.php
index ad29f91..b45aeba 100644 (file)
--- a/includes/Storage/DerivedPageDataUpdater.php
+++ b/includes/Storage/DerivedPageDataUpdater.php
@@ -351,13 +351,9 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                        throw new InvalidArgumentException( '$parentId should match the parent of $revision' );
                }
 
-               if ( $revision
-                       && $user
-                       && $revision->getUser( RevisionRecord::RAW )->getName() !== $user->getName()
-               ) {
-                       throw new InvalidArgumentException( '$user should match the author of $revision' );
-               }
-
+               // NOTE: For null revisions, $user may be different from $this->revision->getUser
+               // and also from $revision->getUser.
+               // But $user should always match $this->user.
                if ( $user && $this->user && $user->getName() !== $this->user->getName() ) {
                        return false;
                }
@@ -368,10 +364,6 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                        return false;
                }
 
-               if ( $revision && !$user ) {
-                       $user = $revision->getUser( RevisionRecord::RAW );
-               }
-
                if ( $this->pageState
                        && $revision
                        && $revision->getParentId() !== null
@@ -387,22 +379,6 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                        return false;
                }
 
-               if ( $this->revision
-                       && $user
-                       && $this->revision->getUser( RevisionRecord::RAW )
-                       && $this->revision->getUser( RevisionRecord::RAW )->getName() !== $user->getName()
-               ) {
-                       return false;
-               }
-
-               if ( $revision
-                       && $this->user
-                       && $this->revision->getUser( RevisionRecord::RAW )
-                       && $revision->getUser( RevisionRecord::RAW )->getName() !== $this->user->getName()
-               ) {
-                       return false;
-               }
-
                // NOTE: this check is the primary reason for having the $this->slotsUpdate field!
                if ( $this->slotsUpdate
                        && $slotsUpdate

diff --git a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
index c175e2f..7320305 100644 (file)
--- a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
+++ b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
@@ -803,16 +803,6 @@ class DerivedPageDataUpdaterTest extends MediaWikiTestCase {
                        '$forParent' => 0,
                        '$isReusable' => false,
                ];
-               yield 'mismatch prepareUpdate revision user' => [
-                       '$prepUser' => null,
-                       '$prepRevision' => $rev2,
-                       '$prepUpdate' => null,
-                       '$forUser' => null,
-                       '$forRevision' => $rev2x,
-                       '$forUpdate' => null,
-                       '$forParent' => 0,
-                       '$isReusable' => false,
-               ];
                yield 'mismatch prepareUpdate revision id' => [
                        '$prepUser' => null,
                        '$prepRevision' => $rev2,