EditResult: only stash reverts

Why:
- In I38cb5622238bc674 we started to write EditResults to the stash
  unconditionally. That's several orders of magnitude more than
  before and would flood the cache. We should only store the
  EditResult for reverts, and only if RC patrolling is enabled.
- While this logic should really be in ChangeTrackingEventIngress,
  that currently doesn't work because some extensions need the
  cache to be updated before their hooks run, but the ingress is only
  triggered post-send.

What:
- Make DerivedPageDataUpdater skip writing to EditResultCache if the
  edit is not a revert or RC patrolling is not enabled.

Bug: T388573
Bug: T386217
Change-Id: I4ac6efbdc1713c87153e66e0520c9f749a7a2a9d

2 files changed, 38 insertions, 30 deletions

diff --git a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
index e516f7cfdcce..06fe953e8061 100644
--- a/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
+++ b/tests/phpunit/includes/Storage/DerivedPageDataUpdaterTest.php
@@ -1475,11 +1475,18 @@ class DerivedPageDataUpdaterTest extends MediaWikiIntegrationTestCase {
 			)
 		);
 
-		$this->assertEquals(
-			$editResult,
-			$editResultCache->get( $rev->getId() ),
-			'EditResult should be cached when the revert is not approved'
-		);
+		if ( $useRcPatrol ) {
+			$this->assertEquals(
+				$editResult,
+				$editResultCache->get( $rev->getId() ),
+				'EditResult should be cached if patrolling is enabled'
+			);
+		} else {
+			$this->assertNull(
+				$editResultCache->get( $rev->getId() ),
+				'EditResult should not be cached if patrolling is disabled'
+			);
+		}
 
 		$jobQueueGroup = $this->getServiceContainer()->getJobQueueGroup();
 		$jobQueue = $jobQueueGroup->get( 'revertedTagUpdate' );
diff --git a/tests/phpunit/unit/includes/Storage/RevertedTagUpdateManagerTest.php b/tests/phpunit/unit/includes/Storage/RevertedTagUpdateManagerTest.php
index c1834ffd3a56..b692aa15108e 100644
--- a/tests/phpunit/unit/includes/Storage/RevertedTagUpdateManagerTest.php
+++ b/tests/phpunit/unit/includes/Storage/RevertedTagUpdateManagerTest.php
@@ -15,10 +15,29 @@ use MediaWikiUnitTestCase;
  */
 class RevertedTagUpdateManagerTest extends MediaWikiUnitTestCase {
 
-	public function testApproveRevertedTagForRevision() {
+	public function provideApproveRevertedTagForRevision() {
+		yield 'not revert' => [
+			new EditResult( false, 1234, 0, null, null, false, false, [] ),
+			false
+		];
+		yield 'is revert' => [
+			new EditResult( false, 1234, 0, 1230, 1233, true, false, [] ),
+			false
+		];
+		yield 'no EditResult' => [
+			null,
+			false
+		];
+	}
+
+	/**
+	 * @dataProvider provideApproveRevertedTagForRevision
+	 */
+	public function testApproveRevertedTagForRevision( $editResult, $expectedOutcome ) {
 		$revisionId = 123;
 
-		$editResult = $this->createMock( EditResult::class );
+		$editResult = new EditResult( false, 1234, 0,
+			null, null, false, false, [] );
 		$editResultCache = $this->createMock( EditResultCache::class );
 		$editResultCache->expects( $this->once() )
 			->method( 'get' )
@@ -26,37 +45,19 @@ class RevertedTagUpdateManagerTest extends MediaWikiUnitTestCase {
 			->willReturn( $editResult );
 
 		$jobQueueGroup = $this->createMock( JobQueueGroup::class );
-		$jobQueueGroup->expects( $this->once() )
+		$jobQueueGroup
+			->expects( $expectedOutcome ? $this->once() : $this->never() )
 			->method( 'lazyPush' );
 
 		$manager = new RevertedTagUpdateManager(
 			$editResultCache,
 			$jobQueueGroup
 		);
-		$this->assertTrue(
+		$this->assertSame(
+			$expectedOutcome,
 			$manager->approveRevertedTagForRevision( $revisionId ),
-			'The operation is successful'
+			'return value of approveRevertedTagForRevision()'
 		);
 	}
 
-	public function testApproveRevertedTagForRevision_missingEditResult() {
-		$revisionId = 123;
-
-		$editResultCache = $this->createMock( EditResultCache::class );
-		$editResultCache->expects( $this->once() )
-			->method( 'get' )
-			->with( $revisionId )
-			->willReturn( null );
-
-		$jobQueueGroup = $this->createNoOpMock( JobQueueGroup::class );
-
-		$manager = new RevertedTagUpdateManager(
-			$editResultCache,
-			$jobQueueGroup
-		);
-		$this->assertFalse(
-			$manager->approveRevertedTagForRevision( $revisionId ),
-			'The operation is not successful'
-		);
-	}
 }