RevisionStore::getRevisionByTitle can fail for foreign wikis when called with TitleValue
Closed, ResolvedPublicBUG REPORT
Actions

Assigned To

Authored By

	TK-999
	Mar 28 2020, 7:14 PM

Description

Steps to Reproduce:
Use RevisionStore::getRevisionByTitle to attempt to obtain a RevisionRecord for a foreign wiki, for a title that exists both on the local and the foreign wiki, like so:

$revisionStoreFactory = MediaWiki\MediaWikiServices::getInstance()->getRevisionStoreFactory();
$foreignRevisionStore = $revisionStoreFactory->getRevisionStore( 'some_foreign_db_name' );

$title = new TitleValue( NS_MAIN, 'Main_Page' );
$revision = $foreignRevisionStore->getRevisionByTitle( $title );

Actual Results:
InvalidArgumentException with message: "The given Title (Main Page) does not belong to page ID XXX but actually belongs to YYY.", where XXX is the page ID of the title on the local wiki and YYY is the page ID of the title on the foreign wiki.

Expected Results:
A revision instance should be correctly returned for the foreign wiki.

Details

Related Changes in Gerrit:

	Subject	Repo	Branch	Lines +/-
	Revision: Ensure getRevisionByTitle() works for foreign wikis	mediawiki/core	master	+61 -4

Customize query in gerrit

Related Objects

Mentioned Here: T206498: MediaWiki needlessly queries page data twice on page views
rMWaed6be47ac27: Fully utilize LinkTarget passed to getRevisionByTitle

Event Timeline

TK-999 created this task.Mar 28 2020, 7:14 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 28 2020, 7:14 PM

This looks to be caused by rMWaed6be4, which changed RevisionStore::getRevisionByTitle to convert the passed LinkTarget to a Title using Title::newFromLinkTarget, to resolve T206498. Unfortunately, this breaks when the given RevisionStore is operating in the context of a foreign wiki. In this scenario, when RevisionStoreRecord::__construct calls Title::getArticleID on the Title instance to perform a sanity check of the page ID against the page ID reference stored in the revision table for the given revision record, then Title::getArticleID will lazily fetch the page ID on the local, rather than the foreign wiki. This results in the aforementioned InvalidArgumentException being thrown.

The easiest fix I see would be to change RevisionStore::getRevisionByTitle such that it would only create a Title object when operating in the context of the local wiki, similar to how RevisionStore::getTitle and friends do it already. For foreign wikis, we can just pass null downstream and a Title object will be fetched from the foreign wiki DB for us. This still isn't optimal, given how dependent Title is on global state, but it should get rid of the exception.

Change 584158 had a related patch set uploaded (by TK-999; owner: TK-999):
[mediawiki/core@master] Ensure RevisionStore::getRevisionByTitle works for foreign wikis

https://gerrit.wikimedia.org/r/584158

gerritbot added a project: Patch-For-Review.Mar 28 2020, 7:53 PM

daniel added a subscriber: Amire80.Mar 29 2020, 9:13 AM

• Pchelolo added a project: Platform Team Workboards (External Code Reviews).May 4 2020, 11:50 PM

Change 584158 merged by jenkins-bot:
[mediawiki/core@master] Revision: Ensure getRevisionByTitle() works for foreign wikis

https://gerrit.wikimedia.org/r/584158

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.34; 2020-05-26).May 25 2020, 2:00 AM

Maintenance_bot removed a project: Patch-For-Review.May 25 2020, 2:10 AM

Untagging, since there is nothing to review right now. Feel free to re-tag if there's a new patch.

This should be resolved now, thanks!

RevisionStore::getRevisionByTitle can fail for foreign wikis when called with TitleValueClosed, ResolvedPublicBUG REPORTActions

Description

Details

Related Objects

Event Timeline

RevisionStore::getRevisionByTitle can fail for foreign wikis when called with TitleValue
Closed, ResolvedPublicBUG REPORT
Actions