Page MenuHomePhabricator

Replace usages of Content::getSecondaryDataUpdates
Closed, ResolvedPublic

Description

Currently, code that needs to purge or update data derived from page content (during purge, import, undeletion, etc) does so by calling WikiPage::doEditUpdates and/or by running the DataUpdates returned by Content::getSecondaryLinksUpdate directly.

For MCR, calls to Content::getSecondaryLinksUpdate() need to be replaced, since there may be more than one slot (and thus more than one Content object), and callers should not need to know about slots. This also means that calls to ontent::getSecondaryDataUpdates should not be replaced directly with calls to the new ContentHandler::getSecondaryDataUpdates proposed in T194038. Callers should not have to know about individual slots.

Instead, WikiPage::doSecondaryDataUpdates() should be introduced, with an interface similar to WikiPage::doEditUpdates(), which would use a DerivedPageDataUpdater run the combined DataUpdates for all slots.

Note: this task used to call for calls to WikiPage::doEditUpdates(), but there currently is no replacement for this without exposing DerivedPageDataUpdater directly, which we want to avoid. So WikiPage::doEditUpdates() should be un-deprecated, and its signature changed to accept RevisionRecords as well as old style Revision objects.


Considerations for future refactoring:

In the context of the new page update interface for MCR, a new concise interface should be exposed for purging or updating caches and other derived data. This should be a stateless service, exposing the following methods:

  • purgeCaches
  • runSecondaryDataUpdates
  • updateParserCache

However, in order to run the DataUpdates, the Content of Revision needs to be rendered, and the rendering needs to be cached.
Doing that with a stateless services is blocked on having a RenderdRevisionCache, which is blocked on having RenderedRevision, which is blocked on having a RevisionRenderer, which is blocked on SlotRoleHandler.

As an intermediate step, WikiPage could have a getXyzUpdater( Revision ) methods that would return an intermediate implementation of that interface. The instance returned by WikiPage could be specific for the WikiPage, and make use of the state of WikiPage for caching the rendered version.

Related Objects

StatusAssignedTask
Declineddchen
OpenNone
OpenNone
DuplicateNone
OpenNone
ResolvedAbit
OpenNone
OpenNone
OpenNone
DuplicateNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
Resolvedppelberg
ResolvedKrinkle
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
Opendaniel
Resolveddaniel
ResolvedTgr
ResolvedTgr
ResolvedTgr
ResolvedBstorm
ResolvedCCicalese_WMF

Event Timeline

daniel created this task.May 7 2018, 1:22 PM
daniel triaged this task as Normal priority.

Change 418075 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] [MCR][WIP] Replace calls to Content::getSecondaryDataUpdates

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

daniel renamed this task from Replace usages of WikiPage::doUpdates and Content::getSecondaryLinksUpdates to Replace usages of WikiPage::doUpdates and Content::getSecondaryDataUpdates.May 7 2018, 1:28 PM
daniel updated the task description. (Show Details)

After looking at the code again, it seems to me we want to do T196087: Refactored implementation of MCR page update interface first, and then this. The idea is that we would want to split/refactor DerivedPageDataUpdater before we start using it in more parts of the code. If we go that way, this task should move to phase 2.

daniel added a comment.EditedJun 18 2018, 4:59 PM

For some reason, Phabricator won't let me add T196087: Refactored implementation of MCR page update interface as a subtask, after I removed it as a parent task. What the hell?

Oh. Circular dependency: T196087: Refactored implementation of MCR page update interface → T174038: Initial implementation of MCR page update interface → T190063: Tracking dependencies for multiple Content objects per page (MCR) → T194043: Replace usages of WikiPage::doUpdates and Content::getSecondaryDataUpdates → T196087: Refactored implementation of MCR page update interface

Will sort that out later...

daniel updated the task description. (Show Details)

I have rewritten the task description to decouple this from T196087.

It has become clear to me that the proper implementation of this is blocked by the full refactoring of DerivedPageDataUpdater. So I'm proposing an intermediate implementation that matches what we will need later, but can be achieved with the current DerivedPageDataUpdater implementation.

Change 418075 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] [MCR][DNM] Replace calls to Content::getSecondaryDataUpdates

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

Ladsgroup moved this task from incoming to monitoring on the Wikidata board.Jun 28 2018, 3:39 PM
Vvjjkkii renamed this task from Replace usages of WikiPage::doUpdates and Content::getSecondaryDataUpdates to 8gdaaaaaaa.Jul 1 2018, 1:11 AM
Vvjjkkii raised the priority of this task from Normal to High.
Vvjjkkii removed daniel as the assignee of this task.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed subscribers: Aklapper, gerritbot.
CommunityTechBot assigned this task to daniel.
CommunityTechBot lowered the priority of this task from High to Normal.
CommunityTechBot renamed this task from 8gdaaaaaaa to Replace usages of WikiPage::doUpdates and Content::getSecondaryDataUpdates.
CommunityTechBot added subscribers: Aklapper, gerritbot.

This task should be split: for SDC, we need to replace calls to Content::getSecondaryDataUpdates, but we can leave calls to WikiPage::doUpdates to be cleaned up later

daniel renamed this task from Replace usages of WikiPage::doUpdates and Content::getSecondaryDataUpdates to Replace usages of Content::getSecondaryDataUpdates.Aug 1 2018, 3:13 PM
daniel updated the task description. (Show Details)

Instead of splitting, I changed the description to no longer call for WikiPagedoEditUpdates to be replaced. That method is actually OK for now.

CCicalese_WMF reassigned this task from Anomie to Tgr.Aug 27 2018, 2:15 PM
CCicalese_WMF added a subscriber: Anomie.

Change 456182 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] MCR WIP Replace calls to Content::getSecondaryDataUpdates

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

Change 456182 abandoned by Daniel Kinzler:
MCR WIP Replace calls to Content::getSecondaryDataUpdates

Reason:
see Idbe7d582b49

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

Change 418075 abandoned by Daniel Kinzler:
[MCR][DNM] Replace calls to Content::getSecondaryDataUpdates

Reason:
see Idbe7d582b49

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

Change 456204 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] [MCR][WIP] Move getSecondaryDataUpdates to the page level

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

Change 456204 merged by jenkins-bot:
[mediawiki/core@master] [MCR] Move getSecondaryDataUpdates to the page level

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