Page MenuHomePhabricator

[Task] WikiPageEntityRevisionLookup::getLatestRevisionId() should throw an UnsersolvedRedirectException when encountering a redirect.
Closed, ResolvedPublic

Description

WikiPageEntityRevisionLookup::getLatestRevisionId() should throw an UnsersolvedRedirectException when encountering a redirect.
It currently just returns false in this case. That leads to RevisionBasedEntityLookup::hasEntity to return false in this case, making it impossible to distinguish between a redirect and a non-existing entity. That in turn leads to RedirectResolvingEntityLookup violating its contract for hasEntity, which states that "If the given entity ID points to a redirect, that redirect is resolved and the existence of the target entity is checked."

Throwing an UnsersolvedRedirectException from getLatestRevisionId if $row->page_is_redirect is true should fix all this. We should however double-check that nothing relies on hasEntity returning false for redirects.

Related Objects

Event Timeline

daniel raised the priority of this task from to Needs Triage.
daniel updated the task description. (Show Details)
daniel subscribed.
daniel set Security to None.
JanZerebecki renamed this task from WikiPageEntityRevisionLookup::getLatestRevisionId() should throw an UnsersolvedRedirectException when encountering a redirect. to [Task] WikiPageEntityRevisionLookup::getLatestRevisionId() should throw an UnsersolvedRedirectException when encountering a redirect. .Sep 18 2015, 1:35 PM
JanZerebecki triaged this task as High priority.
JanZerebecki moved this task from incoming to ready to go on the Wikidata board.

Change 433952 had a related patch set uploaded (by Eranroz; owner: Eranroz):
[mediawiki/extensions/Wikibase@master] getLatestRevisionId redirect handling

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

eranroz subscribed.

I got to this task while debugging a related issue with mw.wikibase.entityExists(REDIRECT-Qid) - T192462

Change 453118 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE); owner: Aleksey Bekh-Ivanov (WMDE)):
[mediawiki/extensions/Wikibase@master] Introduce LatestRevisionIdResult

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

Change 453148 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE); owner: Aleksey Bekh-Ivanov (WMDE)):
[mediawiki/extensions/Wikibase@master] Use LatestRevisionIdResult in EntityRevisionLookup::getLatestRevisionId

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

Change 453149 had a related patch set uploaded (by Aleksey Bekh-Ivanov (WMDE); owner: Aleksey Bekh-Ivanov (WMDE)):
[mediawiki/extensions/WikibaseLexeme@master] Use LatestRevisionIdResult in EntityRevisionLookup::getLatestRevisionId()

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

Change 453118 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Introduce LatestRevisionIdResult

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

Change 453149 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] Use LatestRevisionIdResult in EntityRevisionLookup::getLatestRevisionId

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

Change 453148 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Use LatestRevisionIdResult in EntityRevisionLookup::getLatestRevisionId

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

This looks resolved to me. The sub-task does not need to be a sub-task of this one (the other way around would've made more sense probably).

Before I make any changes, anything left open on this (apart from fixing Lua's endpoint)?

Change 433952 abandoned by Addshore:
getLatestRevisionId redirect handling

Reason:
I'm going to abandon this for now and continue discussing on the ticket.

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

Change 593460 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/Wikibase@master] docs: WikiPageEntityRevisionLookup::getLatestRevisionId and redirects

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

Change 593460 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] docs: WikiPageEntityRevisionLookup::getLatestRevisionId and redirects

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