• createLogRecentChange: 2 callers
  • LogPage::saveContent -> LogPage::addEntry where some other extra context is floating around ->  a limited slightly obscure set of extensions which generally have a context available
  • ManualLogEntry::getRecentChange
    • [[ https://codesearch.wmcloud.org/search/?q=getRecentChange%5C%28&files=&excludeFiles=&repos= | some extensions which may or may not have a context available ]] and a bunch of false positives
    • ManualLogEntry::publish -> a lot
    • many instantiations of ManualLogEntry as well
  • soft-deprecated RecentChange::newLogEntry
    •  basically nothing

• createCategorizationRecentChange: 1 caller
  • CategoryMembershipChange::notifyCategorization -> CategoryMembershipChange::createRecentChangesEntry -> CategoryMembershipChange::triggerCategoryAddedNotification / CategoryMembershipChange::triggerCategoryRemovedNotification -> CategoryMembershipChangeJob::notifyUpdatesForRevision -> CategoryMembershipChangeJob::run

• createNewPageRecentChange / createEditRecentChange
  • maintenance/importTextFiles.php script
  • ChangeTrackingEventIngress::updateRecentChangesAfterPageUpdated -> ChangeTrackingEventIngress::handlePageLatestRevisionChangedEvent -> ???
  • soft-deprecated RecentChange::notifyNew / RecentChange::notifyEdit -> exclusively the ExternalArticles extension

• insertRecentChange
  • maintenance/importTextFiles.php script
  • ChangeTrackingEventIngress::updateRecentChangesAfterPageUpdated
  • CategoryMembershipChange::notifyCategorization
  • LogPage::saveContent
  • soft-deprecated RecentChange::save -> no callers

I do not quite understand these Ingress things, but neither the maintenance script nor the Job should have a meaningful requests available anyway, so they could pass a trivial FauxRequest. LogPage::addEntry should just take a WebRequest as a parameter, for how to deal with ManualLogEntry I am not so sure.

Also, hard-deprecate these compatibility shims in RecentChange, preferably before the 1.46 branch cut, to have one less thing to worry about.

Both checkIPAddress() and createLogRecentChange() access the request as a fallback. Would it be feasible to deprecate not passing an IP address and bot flag, instead of trying to pass a WebRequest along?

This is a good point.

For the bot flag, I do not understand how it gets populated in the standard scenarios, once again leading to this event ingress pattern. Anyway, it feels to me like on some level, this information must be read from the request to avoid changing the behavior compared to now.

The IP seems to be specified by more or less none of the callers. It would need to be injected from the request, at a similar level where we would otherwise inject the request in its entirety.
Either way, for insertRecentChange, limiting the interface does not seem feasible due to the diverse hook usage in extensions (which would still require a complex migration, as I have learnt today...)

Considering this, all callers of createNewPageRecentChange / createEditRecentChange / createCategorizationRecentChange would need to have the full request available anyway. Still, for the three methods it would be nicer to just pass in the IP rather than the entire request. This back and forth with empty strings seems redundant and unnecessary.

While createLogRecentChange does not visit ManualLogEntry::getRecentChange, the IP would still need to be passed in at a similar level as the request.

Change #1271077 had a related patch set uploaded (by MGChecker; author: MGChecker):

[mediawiki/core@master] Hard-deprecate static RecentChange methods

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

Change #1271077 merged by jenkins-bot:

[mediawiki/core@master] Hard-deprecate static RecentChange methods

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

Change #1275896 had a related patch set uploaded (by MGChecker; author: MGChecker):

[mediawiki/core@master] [WIP] Prepare removal of request fallbacks in RecentChange context

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

In T423274#11819590, @MGChecker wrote:

• soft-deprecated RecentChange::save -> no callers

That one is actually not true, see e.g.
https://codesearch.wmcloud.org/deployed/?q=rc-%3Esave%5C%28&files=&excludeFiles=&repos=
https://codesearch.wmcloud.org/deployed/?q=recentChange-%3Esave%5C%28&files=&excludeFiles=&repos=&i=fosho
I overlooked that one was not static.

@daniel I read some documentation at MediaWiki.org, but I still feel no smarter.
Can you enlighten me whether RequestContext::getMain would be populated at ChangeTrackingEventIngress?
And is there any proper way to get a context source or at least the request in question there?

Change #1276288 had a related patch set uploaded (by MGChecker; author: MGChecker):

[mediawiki/core@master] Replace calls of deprecated RecentChange methods

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

Change #1276288 merged by jenkins-bot:

[mediawiki/core@master] Replace calls of deprecated RecentChange methods

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

@Tascsipacsi Are you sure adding a parameter to a hook is a breaking change?
E.g. https://gerrit.wikimedia.org/r/c/mediawiki/core/+/347143 was not.

Yes, I’m sure. Adding new parameters to an interface method breaks interface implementations. rMW72c969d6774eebf9ce70ab9b9cd3be7f2426de2f was committed before hook handler interfaces were introduced, so there were no interface implementations to break back then.