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

ActivityUpdateJob: accept PageReference

Browse source 
For both LinkTarget and PageReference, just extract
namespace and dbkey and pass those in the array
of parameters to Job::__construct().

Allows a bunch of simplification to WatchedItemStore.

Bug: T291531
Change-Id: Id150d0c62af38d4b3d17e5698866127c6e04717e
This commit is contained in:
DannyS712 2021-09-21 23:22:19 +00:00
parent 20aa78f213
commit db49026745
4 changed files with 17 additions and 98 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
includes/ServiceWiring.php
View file

@ -1795,8 +1795,7 @@ return [
$services->getReadOnlyMode(),
$services->getNamespaceInfo(),
$services->getRevisionLookup(),
$services->getLinkBatchFactory(),
$services->getTitleFactory()
$services->getLinkBatchFactory()
);
$store->setStatsdDataFactory( $services->getStatsdDataFactory() );

15
includes/jobqueue/jobs/ActivityUpdateJob.php
View file

@ -20,6 +20,7 @@
*/
use MediaWiki\Linker\LinkTarget;
use MediaWiki\Page\PageReference;
/**
* Job for updating user activity like "last viewed" timestamps
@ -34,10 +35,18 @@ use MediaWiki\Linker\LinkTarget;
* @since 1.26
*/
class ActivityUpdateJob extends Job {
public function __construct( LinkTarget $title, array $params ) {
$title = Title::newFromLinkTarget( $title );
/**
* @param LinkTarget|PageReference $title
* @param array $params
*/
public function __construct( $title, array $params ) {
// If we know its a PageReference, we could just pass that to the parent
// constructor, but its simpler to just extract namespace and dbkey, and
// that works for both LinkTarget and PageReference
$params['namespace'] = $title->getNamespace();
$params['title'] = $title->getDBkey();
parent::__construct( 'activityUpdateJob', $title, $params );
parent::__construct( 'activityUpdateJob', $params );
static $required = [ 'type', 'userid', 'notifTime', 'curTime' ];
$missing = implode( ', ', array_diff( $required, array_keys( $this->params ) ) );

22
includes/watcheditem/WatchedItemStore.php
View file

@ -114,9 +114,6 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
*/
private $linkBatchFactory;
/** @var TitleFactory */
private $titleFactory;
/**
* @var string|null Maximum configured relative expiry.
*/
@ -135,7 +132,6 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
* @param NamespaceInfo $nsInfo
* @param RevisionLookup $revisionLookup
* @param LinkBatchFactory $linkBatchFactory
* @param TitleFactory $titleFactory
*/
public function __construct(
ServiceOptions $options,
@ -146,8 +142,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
ReadOnlyMode $readOnlyMode,
NamespaceInfo $nsInfo,
RevisionLookup $revisionLookup,
LinkBatchFactory $linkBatchFactory,
TitleFactory $titleFactory
LinkBatchFactory $linkBatchFactory
) {
$options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
$this->updateRowsPerQuery = $options->get( 'UpdateRowsPerQuery' );
@ -167,7 +162,6 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
$this->nsInfo = $nsInfo;
$this->revisionLookup = $revisionLookup;
$this->linkBatchFactory = $linkBatchFactory;
$this->titleFactory = $titleFactory;
$this->latestUpdateCache = new HashBagOStuff( [ 'maxKeys' => 3 ] );
}
@ -1482,19 +1476,9 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
);
// If the page is watched by the user (or may be watched), update the timestamp
// ActivityUpdateJob requires a LinkTarget, and creates a Title object from that,
// to pass to Job, which only needs a PageReference. TODO clean that up, T291531.
// If we already have a LinkTarget, we still convert to a Title object so that
// the Title::newFromLinkTarget() call in ActivityUpdateJob doesn't break things
// in unit tests
if ( $title instanceof LinkTarget ) {
$titleObj = $this->titleFactory->castFromLinkTarget( $title );
} else {
// instanceof PageIdentity
$titleObj = $this->titleFactory->castFromPageIdentity( $title );
}
// ActivityUpdateJob accepts both LinkTarget and PageReference
$job = new ActivityUpdateJob(
$titleObj,
$title,
[
'type' => 'updateWatchlistNotification',
'userid' => $user->getId(),

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

@ -147,54 +147,6 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
);
}
/**
* @param LinkTarget|PageIdentity|null $target
* @param Title|null $title
* @return MockObject|TitleFactory
*/
private function getTitleFactory( $target = null, $title = null ) {
// TitleFactory only needed for castFromLinkTarget or castFromPageIdentity - if this is
// called with a link target or page identity and a title, the mock expects the function
// invocation and returns the title, otherwise the mock expects never to be called.
// If no title is provided here, we create a placeholder mock that passes the ->equals()
// check, and thats it
$titleFactory = $this->createNoOpMock(
TitleFactory::class,
[
'castFromLinkTarget',
'castFromPageIdentity'
]
);
if ( $target !== null ) {
if ( $title === null ) {
$title = $this->makeMockTitle(
$target->getDBkey(),
[
'namespace' => $target->getNamespace()
]
);
}
$title->method( 'equals' )
->with( $target )
->willReturn( true );
if ( $target instanceof LinkTarget ) {
$titleFactory->method( 'castFromLinkTarget' )
->with( $target )
->willReturn( $title );
$titleFactory->expects( $this->never() )->method( 'castFromPageIdentity' );
} else {
$titleFactory->method( 'castFromPageIdentity' )
->with( $target )
->willReturn( $title );
$titleFactory->expects( $this->never() )->method( 'castFromLinkTarget' );
}
} else {
$titleFactory->expects( $this->never() )->method( 'castFromLinkTarget' );
$titleFactory->expects( $this->never() )->method( 'castFromPageIdentity' );
}
return $titleFactory;
}
/**
* @param array $mocks Associative array providing mocks to use when constructing the
* WatchedItemStore. Anything not provided will fall back to a default. Valid keys:
@ -205,7 +157,6 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
* * readOnlyMode
* * nsInfo
* * revisionLookup
* * titleFactory
* * expiryEnabled
* * maxExpiryDuration
* * watchlistPurgeRate
@ -235,8 +186,7 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
$mocks['readOnlyMode'] ?? $this->getDummyReadOnlyMode( false ),
$nsInfo,
$mocks['revisionLookup'] ?? $this->getMockRevisionLookup(),
$this->getMockLinkBatchFactory( $db ),
$mocks['titleFactory'] ?? $this->getTitleFactory()
$this->getMockLinkBatchFactory( $db )
);
}
@ -2288,12 +2238,10 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
$user = new UserIdentityValue( 1, 'MockUser' );
$title = $testPageFactory( 100, 0, 'SomeDbKey' );
$titleFactory = $this->getTitleFactory( $title );
$store = $this->newWatchedItemStore( [
'db' => $mockDb,
'cache' => $mockCache,
'titleFactory' => $titleFactory,
] );
$this->assertFalse(
@ -2367,14 +2315,11 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
},
] );
$titleFactory = $this->getTitleFactory( $title );
$store = $this->newWatchedItemStore( [
'db' => $mockDb,
'queueGroup' => $mockQueueGroup,
'cache' => $mockCache,
'revisionLookup' => $mockRevisionLookup,
'titleFactory' => $titleFactory,
] );
$this->assertTrue(
@ -2415,14 +2360,11 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
},
] );
$titleFactory = $this->getTitleFactory( $title );
$store = $this->newWatchedItemStore( [
'db' => $mockDb,
'queueGroup' => $mockQueueGroup,
'cache' => $mockCache,
'revisionLookup' => $mockRevisionLookup,
'titleFactory' => $titleFactory,
] );
$mockQueueGroup->method( 'lazyPush' )
@ -2504,14 +2446,11 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
'getNextRevision' => 1,
] );
$titleFactory = $this->getTitleFactory( $title );
$store = $this->newWatchedItemStore( [
'db' => $mockDb,
'queueGroup' => $mockQueueGroup,
'cache' => $mockCache,
'revisionLookup' => $mockRevisionLookup,
'titleFactory' => $titleFactory,
] );
$mockQueueGroup->method( 'lazyPush' )
@ -2609,14 +2548,11 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
]
);
$titleFactory = $this->getTitleFactory( $title );
$store = $this->newWatchedItemStore( [
'db' => $mockDb,
'queueGroup' => $mockQueueGroup,
'cache' => $mockCache,
'revisionLookup' => $mockRevisionLookup,
'titleFactory' => $titleFactory,
] );
$mockQueueGroup->method( 'lazyPush' )
@ -2706,14 +2642,11 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
]
);
$titleFactory = $this->getTitleFactory( $title );
$store = $this->newWatchedItemStore( [
'db' => $mockDb,
'queueGroup' => $mockQueueGroup,
'cache' => $mockCache,
'revisionLookup' => $mockRevisionLookup,
'titleFactory' => $titleFactory,
] );
$mockQueueGroup->method( 'lazyPush' )
@ -2811,14 +2744,11 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
]
);
$titleFactory = $this->getTitleFactory( $title );
$store = $this->newWatchedItemStore( [
'db' => $mockDb,
'queueGroup' => $mockQueueGroup,
'cache' => $mockCache,
'revisionLookup' => $mockRevisionLookup,
'titleFactory' => $titleFactory,
] );
$mockQueueGroup->method( 'lazyPush' )
@ -2916,14 +2846,11 @@ class WatchedItemStoreUnitTest extends MediaWikiUnitTestCase {
]
);
$titleFactory = $this->getTitleFactory( $title );
$store = $this->newWatchedItemStore( [
'db' => $mockDb,
'queueGroup' => $mockQueueGroup,
'cache' => $mockCache,
'revisionLookup' => $mockRevisionLookup,
'titleFactory' => $titleFactory,
] );
$mockQueueGroup->method( 'lazyPush' )