Page MenuHomePhabricator

AuthManager should use dependency injection
Closed, ResolvedPublic

Description

There are two somewhat distinct applications of dependency injection that should be considered:

  • Injecting the providers into AuthManager (and making it a service instead of a singleton). Right now the AuthManager class and the configuration logic deciding what providers to use are strongly coupled. It should be possible to create other AuthManager instances with different sets of providers. This would be helpful for extensions which do programmatic account creation (such as TranslateSandbox) and right now have to hope that no provider is registered which would interfere with the non-interactive account creation projects.
  • Injecting dependencies into each provider, either by making them into separate services, or by providing a dedicated ServiceContainer for AuthManager in which the providers can be items. The main use case for this is extension configuration: providers defined by extensions need two Config instances, one for core and one for the extension. Currently the core config is supplied via setter injection and the extension one via ugly hacks.

Event Timeline

Tgr renamed this task from AuthManager should use dependency injection to load the prociders to AuthManager should use dependency injection to load the providers.Jul 27 2016, 10:59 PM
Tgr renamed this task from AuthManager should use dependency injection to load the providers to AuthManager should use dependency injection.Aug 3 2016, 1:06 AM
Tgr updated the task description. (Show Details)

The big work for the service was done with https://gerrit.wikimedia.org/r/c/mediawiki/core/+/549934, some injection are still missing.

For extension it is possible to use ObjectFactory specs on extension.json for the provider - for example https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AntiSpoof/+/618623/2/extension.json

Change 633330 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] Inject ReadOnlyMode service into AuthManager service

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

Change 587575 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] Use UserNameUtils in AuthManager

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

Change 619123 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] Use SpecialPageFactory in AuthManager

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

Change 633330 merged by jenkins-bot:
[mediawiki/core@master] Inject ReadOnlyMode service into AuthManager service

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

Change 587575 merged by jenkins-bot:
[mediawiki/core@master] Use UserNameUtils in AuthManager

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

Change 619123 abandoned by Umherirrender:
[mediawiki/core@master] Use SpecialPageFactory in AuthManager

Reason:
let's keep it as is

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

Change 706397 had a related patch set uploaded (by Zabe; author: Zabe):

[mediawiki/core@master] AuthManager: inject remaining services

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

Change 707347 had a related patch set uploaded (by Zabe; author: Zabe):

[mediawiki/core@master] AuthManager: stop injecting unused BlockErrorFormatter service

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

Change 707347 merged by jenkins-bot:

[mediawiki/core@master] AuthManager: stop injecting unused BlockErrorFormatter service

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

Change 706397 merged by jenkins-bot:

[mediawiki/core@master] AuthManager: inject remaining services

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

Change 710104 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/core@master] AuthManager: inject more services

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

Change 710104 merged by jenkins-bot:

[mediawiki/core@master] AuthManager: inject more services

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

Zabe added a project: User-Zabe.

As far as I can tell all used services are now being injected into AuthManager.