Unclear MCR replacement for WikiPage::prepareContentForEdit
Open, MediumPublic
Actions

Assigned To

None

Authored By

	Daimona
	Jan 8 2020, 5:54 PM

Description

The docs of WikiPage::prepareContentForEdit say:

@deprecated since 1.32, use getDerivedDataUpdater instead

But WikiPage::getDerivedDataUpdater is private, and so has been since its introduction in r405015. What should we use as replacement?

Details

	Subject	Repo	Branch	Lines +/-
	Introduced PreparedUpdate to replace PreparedEdit	mediawiki/core	master	+341 -43
	Use WikiPage::getCurrentUpdate	mediawiki/extensions/SpamBlacklist	master	+2 -3

Customize query in gerrit

Related Objects

Mentioned In: T304238: Replace the EditFilterMergedContent hook with a more modern one
T65632: AbuseFilter *_links variables incorrect on item edits.
T288639: SpamBlacklistHooks::onEditFilterMergedContent causes every edit to be rendered twice
T286140: AbuseLog no longer recording revids of saved edits
T264104: Verify AbuseFilter code that claims to share and re-use ParserOutput from core
Mentioned Here: T288639: SpamBlacklistHooks::onEditFilterMergedContent causes every edit to be rendered twice

Event Timeline

Daimona created this task.Jan 8 2020, 5:54 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 8 2020, 5:54 PM

Anomie subscribed.Jan 9 2020, 2:41 PM

Eevans triaged this task as Medium priority.Jan 10 2020, 5:22 PM

Eevans edited projects, added Platform Team Workboards (Clinic Duty Team); removed Platform Engineering.Jan 10 2020, 5:40 PM

daniel moved this task from Inbox to Backlog on the Platform Team Workboards (Clinic Duty Team) board.Feb 5 2020, 7:32 PM

• AMooney edited projects, added Platform Engineering (Icebox); removed Platform Team Workboards (Clinic Duty Team).Mar 13 2020, 1:33 PM

CCicalese_WMF removed a project: Platform Engineering (Icebox).Mar 24 2020, 10:30 PM

Daimona mentioned this in T264104: Verify AbuseFilter code that claims to share and re-use ParserOutput from core.Sep 29 2020, 3:46 PM

Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:02 PM

{{ping}} This method was deprecated 3 versions ago and it still doesn't have a documented replacement.

@daniel can you recommend a strategy to move this forward?

• AMooney moved this task from Backlog to Later on the Platform Team Workboards (Clinic Duty Team) board.Jan 19 2021, 2:18 PM

@daniel: Any comments which public function to use instead? Thanks in advance!

Change 701074 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] WIP: introduced PreparedUpdate to replace PreparedEdit

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

gerritbot added a project: Patch-For-Review.Jun 23 2021, 10:45 AM

This was stalled for a long time, since I was hoping to avoid WikiPage maintaining state about ongoing edits. That would however mean replacing several hooks that are fired during edits with a new version that would expose the relevant state directly. We never got around to scheduling that effort.

My proposal for now is to continue the pattern set by PreparedEdit and WikiPage::prepareContentFor edit, by introducing PreparedUpdate and WikiPage::prepareUpdate(). The PreparedUpdate interface seems find for the future. Access to WikiPage::prepareUpdate() should eventually become unnecessary when we have better hooks.

I made a WIP patch to explore the idea. Comments on the approach I took are very welcome.

Pinging @Addshore since we discussed this in the context of Wikibase recently, in particular the need to defer rendering until after the response has been sent.

Besides AbuseFilter, other extensions affected are TemplateData, SpamBlacklist, MediaWiki-extensions-FlaggedRevs, and ConfirmEdit (CAPTCHA extension).

daniel mentioned this in T286140: AbuseLog no longer recording revids of saved edits.Jul 13 2021, 7:13 PM

Change 705854 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/extensions/SpamBlacklist@master] Use WikiPage::getCurrentUpdate

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

Ladsgroup subscribed.Aug 16 2021, 5:37 PM

Krinkle mentioned this in T288639: SpamBlacklistHooks::onEditFilterMergedContent causes every edit to be rendered twice.Aug 16 2021, 8:22 PM

In T288639#7286483, @Krinkle wrote:

I suggest reverting the changes that moved away from the (soft-deprecated) WikiPage::prepareContentForEdit method. This method is to my knowledge the only appropiate way we have for extension hooks during an edit stash or edit submit request to obtain the ParserOutput in a shared and cached way.

Please comment at T288639 if you disagree. Copying here for awareness.

The prepareContentForEdit signature is very close to what the new proposed thing looks like. And re-adopting that first will be an easy way to re-centralise the call structure, as well as to validate out hypothesis with regards to save timing regression.

Ideally, extensions would get a PreparedUpdate as a hook parameter. Until we have that, we can expose the "current" PreparedUpdate via a service. This is better than relying on a WikiPage instance coming from RequestContext (especially since we want to kill RequestContext::getWikiPage).

Change 705854 abandoned by Daniel Kinzler:

[mediawiki/extensions/SpamBlacklist@master] Use WikiPage::getCurrentUpdate

Reason:

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

Change 701074 merged by jenkins-bot:

[mediawiki/core@master] Introduced PreparedUpdate to replace PreparedEdit

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

ReleaseTaggerBot added a project: MW-1.38-notes (1.38.0-wmf.16; 2022-01-03).Dec 20 2021, 9:00 PM

matej_suchanek mentioned this in T65632: AbuseFilter *_links variables incorrect on item edits..Feb 4 2022, 10:38 AM

Daimona mentioned this in T304238: Replace the EditFilterMergedContent hook with a more modern one.Mar 20 2022, 5:56 PM

Addshore unsubscribed.Jun 27 2023, 12:40 PM

Maintenance_bot removed a project: Patch-For-Review.Jun 27 2023, 1:11 PM