Page MenuHomePhabricator

Wikidata Client usage tracking for redirect resolving Lua functions
Closed, ResolvedPublic8 Estimated Story Points

Description

Most (all?) Lua functions that return part of an entity are meant to resolve redirects, e.g. given Q123 is a redirect to Q321, mw.wikibase.getLabel('Q123') should return the label of Q321. Some of the redirect resolving functionality got introduced by accident and so the question of usage tracking did not come up. These methods currently track only the redirect but not the redirect target.

What should probably happen is that if the given entity is a redirect, track an all usage for the redirect (if anything at all changes about it, we need to know), and track the relevant aspect (= same as what would be tracked if it wasn't a redirect) of the target.

Acceptance criteria:

  • usages for both the redirect and target are correctly tracked

Event Timeline

@Manuel We should check here if this affects what is taken into account for watchlist notifications and purging articles. My assumption is we're missing out on showing som things in watchlist that should be shown and we're not purging some articles when we should. That'd be not great...

@Ladsgroup @hoo do you think tracking an “all” usage in this case would be okay? Or should it maybe be some other usage (“other”, or maybe a new type)?

"X" usage is deprecated and has caused all sorts of issues including a major outage in ruwiki, we really should avoid using it unless absolutely necessary.

If I understand it correctly, the usage is transient so if I check P123 of Q1 but Q1 is a redirect to Q2, I'll get just entity usage aspect of Q2#C.P123 instead of Q1#C.P123 so nothing major is lost. The only problem I can see is that when Q1 stops being a redirect and then the client wiki won't be notified to pick up the new data but that's pretty rare.

One suggestion would be to have it in O (e.g. being Q1#O) and reverting a redirect making (what's the correct name for it?) would count as the same group as changing alias (by setting otherchanges to true in the change object). Would that work?

"X" usage is deprecated and has caused all sorts of issues including a major outage in ruwiki, we really should avoid using it unless absolutely necessary.

If I understand it correctly, the usage is transient so if I check P123 of Q1 but Q1 is a redirect to Q2, I'll get just entity usage aspect of Q2#C.P123 instead of Q1#C.P123 so nothing major is lost.

No, you get a usage on Q1 (not quite sure what kind of usage – my sandbox page apparently uses “some labels”, rather than specifically the English label), and nothing on Q2. So edits on Q2 have no effect.

okay, that's not great and needs fixing indeed but I suggest fixing it by making it transient

Manuel renamed this task from Usage tracking for redirect resolving Lua functions to Wikidata Client usage tracking for redirect resolving Lua functions.Jul 7 2021, 10:43 AM
Manuel updated the task description. (Show Details)

No, you get a usage on Q1 (not quite sure what kind of usage – my sandbox page apparently uses “some labels”, rather than specifically the English label), and nothing on Q2. So edits on Q2 have no effect.

The usage of "some labels" instead of the English label looks fine, because Template:Q uses the user language for the label and that language can be something other than English. That usage should still be tracked for the redirect target instead though (or to both of them).

Change 703873 had a related patch set uploaded (by Ladsgroup; author: Ladsgroup):

[mediawiki/extensions/Wikibase@master] DataAccess: Add usage for statements when entity id is different

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

So the patch I made above would fix the statement usage and tested locally ^.
Fixing label and other usages has an intricacy. We look up values of other usages through other means (like term store) meaning we should not follow redirect directly, unless we load the whole item using entity lookup which we really shouldn't. Maybe there is just a check to see if the item id resolves in a redirect (by looking up the page table?) and I missed it.

Change 704930 had a related patch set uploaded (by Tobias Andersson; author: Tobias Andersson):

[mediawiki/extensions/Wikibase@master] wip: resolve redirects for usage

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

I'm gonna put this back in todo as it will not be very likely finished by the end of today.

I've uploaded some thoughts on how we could re-use the already existing redirect resolution that happens when looking up terms from the termfallbackcache. In the WikibaseLanguageIndependentLuaBindings we are using two different redirect resolvers that only differ by what they return as far as I can tell. By re-using them we can do the redirect lookup for free and update the usage with that? Never got it to actually update the thing in the database so I'm probably lacking in the understanding on how these usages get updated once they already exists.

This method of updating(currently not updating) was inspired by looking at https://gerrit.wikimedia.org/r/703873 but I'm not sure if this is the correct way to update all of these usages? To some extent I'd probably prefer notifying the client of redirects via some other method, like a job might be a valid alternative?

Change 704930 abandoned by Tobias Andersson:

[mediawiki/extensions/Wikibase@master] wip: resolve redirects for usage

Reason:

/ignored

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

Change 705711 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/Wikibase@master] Track usage on redirects

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

Change 705711 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Track usage on redirects

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

Entity usage on my sandbox page looks sensible now:

Q104056384: Other (Statements), Wikidata Sandbox: Some labels

Promotion: After seeing this I changed the message for other aspect to reflect reality: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/709711