Merge "Avoid using stale data for revision visibility and actor data"

diff --combined includes/Revision/RevisionStore.php
index ee6bf67,23189cc..632bd31
--- 1/includes/Revision/RevisionStore.php
--- 2/includes/Revision/RevisionStore.php
+++ b/includes/Revision/RevisionStore.php
@@@ -1760,10 -1760,16 +1760,16 @@@ class RevisionStor
         * @param object $row
         * @param int $queryFlags
         * @param Title|null $title
-        *
+        * @param bool $fromCache if true, the returned RevisionRecord will ensure that no stale
+        *   data is returned from getters, by querying the database as needed
         * @return RevisionRecord
         */
-       public function newRevisionFromRow( $row, $queryFlags = 0, Title $title = null ) {
+       public function newRevisionFromRow(
+               $row,
+               $queryFlags = 0,
+               Title $title = null,
+               $fromCache = false
+       ) {
                Assert::parameterType( 'object', $row, '$row' );
  
                if ( !$title ) {
@@@ -1797,7 -1803,23 +1803,23 @@@
  
                $slots = $this->newRevisionSlots( $row->rev_id, $row, $queryFlags, $title );
  
-               return new RevisionStoreRecord( $title, $user, $comment, $row, $slots, $this->wikiId );
+               // If this is a cached row, instantiate a cache-aware revision class to avoid stale data.
+               if ( $fromCache ) {
+                       $rev = new RevisionStoreCacheRecord(
+                               function ( $revId ) use ( $queryFlags ) {
+                                       $db = $this->getDBConnectionRefForQueryFlags( $queryFlags );
+                                       return $this->fetchRevisionRowFromConds(
+                                               $db,
+                                               [ 'rev_id' => intval( $revId ) ]
+                                       );
+                               },
+                               $title, $user, $comment, $row, $slots, $this->wikiId
+                       );
+               } else {
+                       $rev = new RevisionStoreRecord(
+                               $title, $user, $comment, $row, $slots, $this->wikiId );
+               }
+               return $rev;
        }
  
        /**
@@@ -2308,7 -2330,7 +2330,7 @@@
                                'page_is_redirect',
                                'page_len',
                        ] );
 -                      $ret['joins']['page'] = [ 'INNER JOIN', [ 'page_id = rev_page' ] ];
 +                      $ret['joins']['page'] = [ 'JOIN', [ 'page_id = rev_page' ] ];
                }
  
                if ( in_array( 'user', $options, true ) ) {
@@@ -2337,7 -2359,7 +2359,7 @@@
                                'old_text',
                                'old_flags'
                        ] );
 -                      $ret['joins']['text'] = [ 'INNER JOIN', [ 'rev_text_id=old_id' ] ];
 +                      $ret['joins']['text'] = [ 'JOIN', [ 'rev_text_id=old_id' ] ];
                }
  
                return $ret;
@@@ -2413,7 -2435,7 +2435,7 @@@
                                        'content_address',
                                        'content_model',
                                ] );
 -                              $ret['joins']['content'] = [ 'INNER JOIN', [ 'slot_content_id = content_id' ] ];
 +                              $ret['joins']['content'] = [ 'JOIN', [ 'slot_content_id = content_id' ] ];
  
                                if ( in_array( 'model', $options, true ) ) {
                                        // Use left join to attach model name, so we still find the revision row even
@@@ -2773,27 -2795,29 +2795,29 @@@
                        return false;
                }
  
+               // Load the row from cache if possible.  If not possible, populate the cache.
+               // As a minor optimization, remember if this was a cache hit or miss.
+               // We can sometimes avoid a database query later if this is a cache miss.
+               $fromCache = true;
                $row = $this->cache->getWithSetCallback(
                        // Page/rev IDs passed in from DB to reflect history merges
                        $this->getRevisionRowCacheKey( $db, $pageId, $revId ),
                        WANObjectCache::TTL_WEEK,
-                       function ( $curValue, &$ttl, array &$setOpts ) use ( $db, $pageId, $revId ) {
+                       function ( $curValue, &$ttl, array &$setOpts ) use (
+                               $db, $pageId, $revId, &$fromCache
+                       ) {
                                $setOpts += Database::getCacheSetOptions( $db );
- 
-                               $conds = [
-                                       'rev_page' => intval( $pageId ),
-                                       'page_id' => intval( $pageId ),
-                                       'rev_id' => intval( $revId ),
-                               ];
- 
-                               $row = $this->fetchRevisionRowFromConds( $db, $conds );
-                               return $row ?: false; // don't cache negatives
+                               $row = $this->fetchRevisionRowFromConds( $db, [ 'rev_id' => intval( $revId ) ] );
+                               if ( $row ) {
+                                       $fromCache = false;
+                               }
+                               return $row; // don't cache negatives
                        }
                );
  
-               // Reflect revision deletion and user renames
+               // Reflect revision deletion and user renames.
                if ( $row ) {
-                       return $this->newRevisionFromRow( $row, 0, $title );
+                       return $this->newRevisionFromRow( $row, 0, $title, $fromCache );
                } else {
                        return false;
                }

diff --combined tests/common/TestsAutoLoader.php
index 153af50,96bd6ae..680b8c3
--- 1/tests/common/TestsAutoLoader.php
--- 2/tests/common/TestsAutoLoader.php
+++ b/tests/common/TestsAutoLoader.php
@@@ -156,7 -156,6 +156,7 @@@ $wgAutoloadClasses += 
        # tests/phpunit/includes/specialpage
        'SpecialPageTestHelper' => "$testDir/phpunit/includes/specialpage/SpecialPageTestHelper.php",
        'AbstractChangesListSpecialPageTestCase' => "$testDir/phpunit/includes/specialpage/AbstractChangesListSpecialPageTestCase.php",
 +      'FormSpecialPageTestCase' => "$testDir/phpunit/includes/specialpage/FormSpecialPageTestCase.php",
  
        # tests/phpunit/includes/specials
        'SpecialPageTestBase' => "$testDir/phpunit/includes/specials/SpecialPageTestBase.php",
@@@ -171,6 -170,7 +171,7 @@@
        'MediaWiki\Tests\Revision\RevisionRecordTests' => "$testDir/phpunit/includes/Revision/RevisionRecordTests.php",
        'MediaWiki\Tests\Revision\RevisionStoreDbTestBase' => "$testDir/phpunit/includes/Revision/RevisionStoreDbTestBase.php",
        'MediaWiki\Tests\Revision\PreMcrSchemaOverride' => "$testDir/phpunit/includes/Revision/PreMcrSchemaOverride.php",
+       'MediaWiki\Tests\Revision\RevisionStoreRecordTest' => "$testDir/phpunit/includes/Revision/RevisionStoreRecordTest.php",
  
        # tests/phpunit/languages
        'LanguageClassesTestCase' => "$testDir/phpunit/languages/LanguageClassesTestCase.php",