Page MenuHomePhabricator

Move redirect lookup logic into a service object
Closed, ResolvedPublic

Description

NOTE: in the light of T296430, this task has changed significantly.

Looking up redirects and maintaining the redirect table is currently done by WikiPage. This logic should be pulled out into a service object. There should be a separate interface that onyl covers the lookup aspect.

Interface draft:

interface RedirectLookup {
   getRedirectTarget( PageIdentity $page ): ?LinkTarget;
}

class RedirectStore implements RedirectLookup {
   getRedirectTarget( PageIdentity $page ): ?LinkTarget;
   updateRedirectTarget( PageIdentity $page, LinkTarget $target );
}

Original Description, for reference:
Looking up redirects is currently implemented in AbstractContent::getRedirectChain() and WikiPage::getRedirectTarget(). It should be handled by a storage layer service object instead.

Interface draft:

interface RedirectLookup {
   getRedirectChain( PageIdentity $page ): LinkTarget[];
   getRedirectTarget( PageIdentity $page ): ?LinkTarget;
   getUltimateRedirectTarget( PageIdentity $page ): ?LinkTarget;
}

The implementation should be based on the redirect table. The interface could be implemented separately, but putting the logic into PageStore may also be ok.

Note that Content should for now keep getRedirectTarget(), though it should not return a Title but a LinkTarget. That probably means deprecating the method and introducing a new one. That is however outside the scope of this task.

Event Timeline

Change 732389 had a related patch set uploaded (by D3r1ck01; author: Derick Alangi):

[mediawiki/core@master] WIP: Introduce RedirectLookup interface

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

Per our conversation, we found that multi-hop redirect logic is broken. We cannot sensibly support it with the current database structure. I filed T296430 for the issue.

We should:

  • deprecated $wgMaxRedirects without replacement
  • deprecated getRedirectChain() and getUltimateRedirectTarget() on Content and AbstractContent without replacement
  • only have getRedirectTarget in the RedirectLookup interface
  • replace calls to Content::getUltimateRedirectTarget with calls to Content::getRedirectTarget
  • treat RedirectLookup::getRedirectTarget() as a replacement for WikiPage::getRedirectTarget(), which should be deprecated.

Change 732389 merged by jenkins-bot:

[mediawiki/core@master] Introduce `Redirect(Lookup&Store)` services to handle redirects

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

daniel claimed this task.

RedirectStore exists now