Page MenuHomePhabricator

Config on the RequestContext may not be the same as the main config
Open, Needs TriagePublic

Description

Problem
In 92004 a setConfig() method was added to RequestContext.

In 137211 a getConfig() method was added to SpecialPage. Though, the assumption (from the comment) is that this method will always return the "main" config.

However, because of RequestContext::setConfig() that is not always the case. This could create a discrepancy between the config that is retrieved from the RequestContext and the config from the MainConfig service.

Solution
It does not appear that anything in core (outside of tests) is using RequestContext::setConfig(). It may be simplest to deprecate this method. Alternatively, SpecialPage::getConfig() could be altered use the MainConfig service rather than the config on the RequestContext.

Event Timeline

dbarratt created this task.Dec 6 2019, 3:17 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 6 2019, 3:17 PM
Restricted Application added a subscriber: MGChecker. · View Herald TranscriptDec 6 2019, 3:23 PM
dbarratt updated the task description. (Show Details)Dec 6 2019, 3:30 PM
Catrope added a subscriber: Catrope.Dec 6 2019, 8:34 PM

Going one step further: config being both a request property and a service seems weird. I think we should make up our mind about which one it is. I'd suggest that the config should be a service, not a request property, and so we should not just deprecate RequestContext::setConfig(), but also (soft-)deprecate RequestContext::getConfig() and move away from it in favor of the MainConfig service. There is a little bit of precedent for this, we deprecated ResourceLoaderContext::getConfig() this year.

This would be a messy migration though, because RequestContext::getConfig() is used all over the place.

WDoranWMF added a subscriber: WDoranWMF.

Is this an area where documentation can be expanded or instantiated?

@Catrope That totally makes sense to me, it would be great if we picked a direction either way.

Is this an area where documentation can be expanded or instantiated?

I don't think so. I think there is a (theoretical?) problem that should be resolved. At a minimum, a direction should be decided upon.