From: daniel <dkinzler@wikimedia.org>
Date: Thu, 15 Nov 2018 14:49:23 +0000 (+0100)
Subject: Reuse DerivedPageDataUpdater during null-edits.
X-Git-Tag: 1.34.0-rc.0~3491^2
X-Git-Url: http://git.cyclocoop.org/?a=commitdiff_plain;h=b8e58c039aa8dd8f2501fe6e704cf0d42715d747;p=lhc%2Fweb%2Fwiklou.git

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 ad29f91e99..b45aeba7f4 100644
--- 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 c175e2fd09..732030522b 100644
--- 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,