* @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 ) {
$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;
}
/**
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;
}
--- /dev/null
+<?php
+/**
+ * A RevisionStoreRecord loaded from the cache.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+namespace MediaWiki\Revision;
+
+use MediaWiki\User\UserIdentity;
+use MediaWiki\User\UserIdentityValue;
+use CommentStoreComment;
+use InvalidArgumentException;
+use Title;
+use User;
+
+/**
+ * A cached RevisionStoreRecord. Ensures that changes performed "behind the back"
+ * of the cache do not cause the revision record to deliver stale data.
+ *
+ * @since 1.33
+ */
+class RevisionStoreCacheRecord extends RevisionStoreRecord {
+
+ /**
+ * @var callable
+ */
+ private $mCallback;
+
+ /**
+ * @note Avoid calling this constructor directly. Use the appropriate methods
+ * in RevisionStore instead.
+ *
+ * @param callable $callback Callback for loading data. Signature: function ( $id ): object
+ * @param Title $title The title of the page this Revision is associated with.
+ * @param UserIdentity $user
+ * @param CommentStoreComment $comment
+ * @param object $row A row from the revision table. Use RevisionStore::getQueryInfo() to build
+ * a query that yields the required fields.
+ * @param RevisionSlots $slots The slots of this revision.
+ * @param bool|string $wikiId the wiki ID of the site this Revision belongs to,
+ * or false for the local site.
+ */
+ function __construct(
+ $callback,
+ Title $title,
+ UserIdentity $user,
+ CommentStoreComment $comment,
+ $row,
+ RevisionSlots $slots,
+ $wikiId = false
+ ) {
+ parent::__construct( $title, $user, $comment, $row, $slots, $wikiId );
+ $this->mCallback = $callback;
+ }
+
+ /**
+ * Overridden to ensure that we return a fresh value and not a cached one.
+ *
+ * @return int
+ */
+ public function getVisibility() {
+ if ( $this->mCallback ) {
+ $this->loadFreshRow();
+ }
+ return parent::getVisibility();
+ }
+
+ /**
+ * Overridden to ensure that we return a fresh value and not a cached one.
+ *
+ * @param int $audience
+ * @param User|null $user
+ *
+ * @return UserIdentity The identity of the revision author, null if access is forbidden.
+ */
+ public function getUser( $audience = self::FOR_PUBLIC, User $user = null ) {
+ if ( $this->mCallback ) {
+ $this->loadFreshRow();
+ }
+ return parent::getUser( $audience, $user );
+ }
+
+ /**
+ * Load a fresh row from the database to ensure we return updated information
+
+ * @throws RevisionAccessException if the row could not be loaded
+ */
+ private function loadFreshRow() {
+ $freshRow = call_user_func( $this->mCallback, $this->mId );
+
+ // Set to null to ensure we do not make unnecessary queries for subsequent getter calls,
+ // and to allow the closure to be freed.
+ $this->mCallback = null;
+
+ if ( $freshRow ) {
+ $this->mDeleted = intval( $freshRow->rev_deleted );
+
+ try {
+ $this->mUser = User::newFromAnyId(
+ $freshRow->rev_user ?? null,
+ $freshRow->rev_user_text ?? null,
+ $freshRow->rev_actor ?? null
+ );
+ } catch ( InvalidArgumentException $ex ) {
+ wfWarn(
+ __METHOD__
+ . ': '
+ . $this->mTitle->getPrefixedDBkey()
+ . ': '
+ . $ex->getMessage()
+ );
+ $this->mUser = new UserIdentityValue( 0, 'Unknown user', 0 );
+ }
+ } else {
+ throw new RevisionAccessException(
+ 'Unable to load fresh row for rev_id: ' . $this->mId
+ );
+ }
+ }
+
+}
'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",
--- /dev/null
+<?php
+namespace MediaWiki\Tests\Revision;
+
+use Title;
+use MediaWiki\User\UserIdentityValue;
+use TextContent;
+use CommentStoreComment;
+use MediaWiki\Revision\RevisionStoreCacheRecord;
+use MediaWiki\Revision\RevisionSlots;
+use MediaWiki\Revision\SlotRecord;
+use MediaWiki\Revision\RevisionRecord;
+use MediaWiki\Revision\RevisionStoreRecord;
+
+/**
+ * @covers \MediaWiki\Revision\RevisionStoreCacheRecord
+ * @covers \MediaWiki\Revision\RevisionStoreRecord
+ * @covers \MediaWiki\Revision\RevisionRecord
+ */
+class RevisionStoreCacheRecordTest extends RevisionStoreRecordTest {
+
+ /**
+ * @param array $rowOverrides
+ * @param bool|callable callback function to use, or false for a default no-op callback
+ *
+ * @return RevisionStoreRecord
+ */
+ protected function newRevision( array $rowOverrides = [], $callback = false ) {
+ $title = Title::newFromText( 'Dummy' );
+ $title->resetArticleID( 17 );
+
+ $user = new UserIdentityValue( 11, 'Tester', 0 );
+ $comment = CommentStoreComment::newUnsavedComment( 'Hello World' );
+
+ $main = SlotRecord::newUnsaved( SlotRecord::MAIN, new TextContent( 'Lorem Ipsum' ) );
+ $aux = SlotRecord::newUnsaved( 'aux', new TextContent( 'Frumious Bandersnatch' ) );
+ $slots = new RevisionSlots( [ $main, $aux ] );
+
+ $row = [
+ 'rev_id' => '7',
+ 'rev_page' => strval( $title->getArticleID() ),
+ 'rev_timestamp' => '20200101000000',
+ 'rev_deleted' => 0,
+ 'rev_minor_edit' => 0,
+ 'rev_parent_id' => '5',
+ 'rev_len' => $slots->computeSize(),
+ 'rev_sha1' => $slots->computeSha1(),
+ 'rev_user' => '11',
+ 'page_latest' => '18',
+ ];
+
+ $row = array_merge( $row, $rowOverrides );
+
+ if ( !$callback ) {
+ $callback = function ( $revId ) use ( $row ) {
+ return (object)$row;
+ };
+ }
+
+ return new RevisionStoreCacheRecord(
+ $callback,
+ $title,
+ $user,
+ $comment,
+ (object)$row,
+ $slots
+ );
+ }
+
+ public function testCallback() {
+ // Provide a callback that returns non-default values. Asserting the revision returns
+ // these values confirms callback execution and behavior. Also confirm the callback
+ // is only invoked once, even for multiple getter calls.
+ $rowOverrides = [
+ 'rev_deleted' => RevisionRecord::DELETED_TEXT,
+ 'rev_user' => 12,
+ ];
+ $callbackInvoked = 0;
+ $callback = function ( $revId ) use ( &$callbackInvoked, $rowOverrides ) {
+ $callbackInvoked++;
+ return (object)$rowOverrides;
+ };
+ $rev = $this->newRevision( [], $callback );
+
+ $this->assertSame( RevisionRecord::DELETED_TEXT, $rev->getVisibility() );
+ $this->assertSame( 12, $rev->getUser()->getId() );
+ $this->assertSame( 1, $callbackInvoked );
+ }
+
+}
use MediaWiki\Revision\MutableRevisionRecord;
use MediaWiki\Revision\RevisionArchiveRecord;
use MediaWiki\Revision\RevisionRecord;
+use MediaWiki\Revision\RevisionStoreRecord;
use MediaWiki\Revision\RevisionSlots;
use MediaWiki\Revision\RevisionStore;
use MediaWiki\Revision\SlotRecord;
$this->testNewMutableRevisionFromArray( $array );
}
+ /**
+ * Creates a new revision for testing caching behavior
+ *
+ * @param WikiPage $page the page for the new revision
+ * @param RevisionStore $store store object to use for creating the revision
+ * @return bool|RevisionStoreRecord the revision created, or false if missing
+ */
+ private function createRevisionStoreCacheRecord( $page, $store ) {
+ $user = MediaWikiTestCase::getMutableTestUser()->getUser();
+ $updater = $page->newPageUpdater( $user );
+ $updater->setContent( SlotRecord::MAIN, new WikitextContent( __METHOD__ ) );
+ $summary = CommentStoreComment::newUnsavedComment( __METHOD__ );
+ $rev = $updater->saveRevision( $summary, EDIT_NEW );
+ return $store->getKnownCurrentRevision( $page->getTitle(), $rev->getId() );
+ }
+
+ /**
+ * @covers \MediaWiki\Revision\RevisionStore::getKnownCurrentRevision
+ */
+ public function testGetKnownCurrentRevision_userNameChange() {
+ global $wgActorTableSchemaMigrationStage;
+
+ $this->overrideMwServices();
+ $cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ] );
+ $this->setService( 'MainWANObjectCache', $cache );
+
+ $store = MediaWikiServices::getInstance()->getRevisionStore();
+ $page = $this->getNonexistingTestPage();
+ $rev = $this->createRevisionStoreCacheRecord( $page, $store );
+
+ // Grab the user name
+ $userNameBefore = $rev->getUser()->getName();
+
+ // Change the user name in the database, "behind the back" of the cache
+ $newUserName = "Renamed $userNameBefore";
+ $this->db->update( 'revision',
+ [ 'rev_user_text' => $newUserName ],
+ [ 'rev_id' => $rev->getId() ] );
+ $this->db->update( 'user',
+ [ 'user_name' => $newUserName ],
+ [ 'user_id' => $rev->getUser()->getId() ] );
+ if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) {
+ $this->db->update( 'actor',
+ [ 'actor_name' => $newUserName ],
+ [ 'actor_user' => $rev->getUser()->getId() ] );
+ }
+
+ // Reload the revision and regrab the user name.
+ $revAfter = $store->getKnownCurrentRevision( $page->getTitle(), $rev->getId() );
+ $userNameAfter = $revAfter->getUser()->getName();
+
+ // The two user names should be different.
+ // If they are the same, we are seeing a cached value, which is bad.
+ $this->assertNotSame( $userNameBefore, $userNameAfter );
+
+ // This is implied by the above assertion, but explicitly check it, for completeness
+ $this->assertSame( $newUserName, $userNameAfter );
+ }
+
+ /**
+ * @covers \MediaWiki\Revision\RevisionStore::getKnownCurrentRevision
+ */
+ public function testGetKnownCurrentRevision_revDelete() {
+ $this->overrideMwServices();
+ $cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ] );
+ $this->setService( 'MainWANObjectCache', $cache );
+
+ $store = MediaWikiServices::getInstance()->getRevisionStore();
+ $page = $this->getNonexistingTestPage();
+ $rev = $this->createRevisionStoreCacheRecord( $page, $store );
+
+ // Grab the deleted bitmask
+ $deletedBefore = $rev->getVisibility();
+
+ // Change the deleted bitmask in the database, "behind the back" of the cache
+ $this->db->update( 'revision',
+ [ 'rev_deleted' => RevisionRecord::DELETED_TEXT ],
+ [ 'rev_id' => $rev->getId() ] );
+
+ // Reload the revision and regrab the visibility flag.
+ $revAfter = $store->getKnownCurrentRevision( $page->getTitle(), $rev->getId() );
+ $deletedAfter = $revAfter->getVisibility();
+
+ // The two deleted flags should be different.
+ // If they are the same, we are seeing a cached value, which is bad.
+ $this->assertNotSame( $deletedBefore, $deletedAfter );
+
+ // This is implied by the above assertion, but explicitly check it, for completeness
+ $this->assertSame( RevisionRecord::DELETED_TEXT, $deletedAfter );
+ }
+
+ /**
+ * @covers \MediaWiki\Revision\RevisionStore::newRevisionFromRow
+ */
+ public function testNewRevisionFromRow_userNameChange() {
+ global $wgActorTableSchemaMigrationStage;
+
+ $page = $this->getTestPage();
+ $text = __METHOD__;
+ /** @var Revision $rev */
+ $rev = $page->doEditContent(
+ new WikitextContent( $text ),
+ __METHOD__
+ )->value['revision'];
+
+ $store = MediaWikiServices::getInstance()->getRevisionStore();
+ $record = $store->newRevisionFromRow(
+ $this->revisionToRow( $rev ),
+ [],
+ $page->getTitle()
+ );
+
+ // Grab the user name
+ $userNameBefore = $record->getUser()->getName();
+
+ // Change the user name in the database
+ $newUserName = "Renamed $userNameBefore";
+ $this->db->update( 'revision',
+ [ 'rev_user_text' => $newUserName ],
+ [ 'rev_id' => $record->getId() ] );
+ $this->db->update( 'user',
+ [ 'user_name' => $newUserName ],
+ [ 'user_id' => $record->getUser()->getId() ] );
+ if ( $wgActorTableSchemaMigrationStage & SCHEMA_COMPAT_WRITE_NEW ) {
+ $this->db->update( 'actor',
+ [ 'actor_name' => $newUserName ],
+ [ 'actor_user' => $record->getUser()->getId() ] );
+ }
+
+ // Reload the record, passing $fromCache as true to force fresh info from the db,
+ // and regrab the user name
+ $recordAfter = $store->newRevisionFromRow(
+ $this->revisionToRow( $rev ),
+ [],
+ $page->getTitle(),
+ true
+ );
+ $userNameAfter = $recordAfter->getUser()->getName();
+
+ // The two user names should be different.
+ // If they are the same, we are seeing a cached value, which is bad.
+ $this->assertNotSame( $userNameBefore, $userNameAfter );
+
+ // This is implied by the above assertion, but explicitly check it, for completeness
+ $this->assertSame( $newUserName, $userNameAfter );
+ }
+
+ /**
+ * @covers \MediaWiki\Revision\RevisionStore::newRevisionFromRow
+ */
+ public function testNewRevisionFromRow_revDelete() {
+ $page = $this->getTestPage();
+ $text = __METHOD__;
+ /** @var Revision $rev */
+ $rev = $page->doEditContent(
+ new WikitextContent( $text ),
+ __METHOD__
+ )->value['revision'];
+
+ $store = MediaWikiServices::getInstance()->getRevisionStore();
+ $record = $store->newRevisionFromRow(
+ $this->revisionToRow( $rev ),
+ [],
+ $page->getTitle()
+ );
+
+ // Grab the deleted bitmask
+ $deletedBefore = $record->getVisibility();
+
+ // Change the deleted bitmask in the database
+ $this->db->update( 'revision',
+ [ 'rev_deleted' => RevisionRecord::DELETED_TEXT ],
+ [ 'rev_id' => $record->getId() ] );
+
+ // Reload the record, passing $fromCache as true to force fresh info from the db,
+ // and regrab the deleted bitmask
+ $recordAfter = $store->newRevisionFromRow(
+ $this->revisionToRow( $rev ),
+ [],
+ $page->getTitle(),
+ true
+ );
+ $deletedAfter = $recordAfter->getVisibility();
+
+ // The two deleted flags should be different, because we modified the database.
+ $this->assertNotSame( $deletedBefore, $deletedAfter );
+
+ // This is implied by the above assertion, but explicitly check it, for completeness
+ $this->assertSame( RevisionRecord::DELETED_TEXT, $deletedAfter );
+ }
+
}
use Wikimedia\TestingAccessWrapper;
use WikitextContent;
+/**
+ * Tests RevisionStore
+ */
class RevisionStoreTest extends MediaWikiTestCase {
private function useTextId() {
dépôts / lhc / web / wiklou.git / commitdiff