Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Change WatchedItemStore to cache an array instead of MapCacheLRU

Browse source 
Updated the closure in WatchedItemStore::resetNotificationTimestamp() to return
an array obtained from MapCacheLRU->toArray(), instead of an object.
Updated the two places that fetch from the cache to read cache entries in either format,
for backwards compatibility.

Added testResetNotificationTimestamp_stashItemTypeCheck test to
WatchedItemStoreUnitTest which verifies correct behaviour with
cache values supplied by the test case.

Bug: T282105
Change-Id: Ibfe4c205ffdf7af7087f6f862004b95b29bdc394
This commit is contained in:
drynok 2021-09-19 20:27:24 +03:00 committed by Petr Pchelko
parent 6fd447783a
commit 9b202c52f4
2 changed files with 68 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
includes/watcheditem/WatchedItemStore.php
View file

@ -1296,12 +1296,12 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
}
$seenTimestamps = $this->getPageSeenTimestamps( $user );
if (
$seenTimestamps &&
$seenTimestamps->get( $this->getPageSeenKey( $target ) ) >= $timestamp
) {
// If a reset job did not yet run, then the "seen" timestamp will be higher
return null;
if ( $seenTimestamps ) {
$seenKey = $this->getPageSeenKey( $target );
if ( isset( $seenTimestamps[$seenKey] ) && $seenTimestamps[$seenKey] >= $timestamp ) {
// If a reset job did not yet run, then the "seen" timestamp will be higher
return null;
}
}
return $timestamp;
@ -1458,9 +1458,9 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
if ( !$current ) {
$value = new MapCacheLRU( 300 );
} elseif ( is_array( $current ) ) {
// Forwards compatibility for T282105
$value = MapCacheLRU::newFromArray( $current, 300 );
} else {
// Backwards compatibility for T282105
$value = $current;
}
$subKey = $this->getPageSeenKey( $title );
@ -1468,16 +1468,19 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
if ( $seenTime > $value->get( $subKey ) ) {
// Revision is newer than the last one seen
$value->set( $subKey, $seenTime );
$this->latestUpdateCache->set( $key, $value, BagOStuff::TTL_PROC_LONG );
$this->latestUpdateCache->set( $key, $value->toArray(), BagOStuff::TTL_PROC_LONG );
} elseif ( $seenTime === false ) {
// Revision does not exist
$value->set( $subKey, wfTimestamp( TS_MW ) );
$this->latestUpdateCache->set( $key, $value, BagOStuff::TTL_PROC_LONG );
$this->latestUpdateCache->set( $key,
$value->toArray(),
BagOStuff::TTL_PROC_LONG );
} else {
return false; // nothing to update
}
return $value;
return $value->toArray();
},
BagOStuff::TTL_HOUR
);
@ -1503,7 +1506,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
/**
* @param UserIdentity $user
* @return MapCacheLRU|null The map contains prefixed title keys and TS_MW values
* @return array|null The map contains prefixed title keys and TS_MW values
*/
private function getPageSeenTimestamps( UserIdentity $user ) {
$key = $this->getPageSeenTimestampsKey( $user );
@ -1515,9 +1518,9 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
return $this->stash->get( $key ) ?: null;
}
);
if ( is_array( $cache ) ) {
// Forwards compatibility for T282105
$cache = MapCacheLRU::newFromArray( $cache, 300 );
// Backwards compatibility for T282105
if ( $cache instanceof MapCacheLRU ) {
$cache = $cache->toArray();
}
return $cache;
}

52
tests/phpunit/unit/includes/watcheditem/WatchedItemStoreUnitTest.php
View file

@ -3192,6 +3192,8 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
$oldMap->set( '0:Title', '20090101000000' );
$newMap = new MapCacheLRU( 300 );
$newMap->set( '0:Title', '20110101000000' );
$wrongKeyMap = new MapCacheLRU( 300 );
$wrongKeyMap->set( '0:Wrong', '20110101000000' );
// Arrays are used for stash values after T282105. We test forwards and
// backwards compatibility. The MapCacheLRU cases can be removed after
// deployment of T282105 has finished.
@ -3223,7 +3225,15 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
'new array' => [
$newMap,
false
]
],
'wrong key MapCacheLRU' => [
$wrongKeyMap,
true
],
'wrong key array' => [
$wrongKeyMap->toArray(),
true
],
];
}
@ -3245,4 +3255,44 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
'20100101000000', $user, $title );
$this->assertSame( $expectNonNull, $result !== null );
}
/**
* @dataProvider provideTestPageFactory
*/
public function testResetNotificationTimestamp_stashItemTypeCheck( $testPageFactory ) {
$user = new UserIdentityValue( 1, 'MockUser' );
$oldid = 22;
$title = $testPageFactory( 100, 0, 'SomeDbKey' );
$stash = new HashBagOStuff;
$mockCache = $this->getMockCache();
$mockRevision = $this->createNoOpMock( RevisionRecord::class );
$mockNextRevision = $this->createNoOpMock( RevisionRecord::class );
$mockRevisionLookup = $this->getMockRevisionLookup(
[
'getTimestampFromId' => static function () {
return '00000000000000';
},
'getRevisionByTitle' => static function () {
return null;
},
'getRevisionById' => static function ( $id ) use ( $mockRevision ) {
return $mockRevision;
},
'getNextRevision' => static function ( RevisionRecord $rev ) use ( $mockNextRevision ) {
return $mockNextRevision;
},
]
);
$store = $this->newWatchedItemStore( [
'cache' => $mockCache,
'revisionLookup' => $mockRevisionLookup,
'stash' => $stash,
'queueGroup' => $this->getMockJobQueueGroup( false ),
] );
$store->resetNotificationTimestamp( $user,
$title,
'force',
$oldid );
$this->assertIsArray( $stash->get( 'global:watchlist-recent-updates::1' ) );
}
}