Page MenuHomePhabricator

Create ActionFactory, add dependency injection to actions
Open, LowPublic

Description

The different action classes, retrieved from Action::factory, should instead be constructed from a dedicated factory service that has an object factory and provides each action the services it needs.

Actions used by services (I may have missed some):

ContentLanguage (HistoryAction, InfoAction, RevertAction)
HookContainer (FormAction, EditAction, HistoryAction, InfoAction, RawAction, ViewAction, WatchAction)
IContentHandlerFactory (RollbackAction)
ILoadBalancer (HistoryAction, InfoAction, both via wfGetDB)
LanguageNameUtils (InfoAction)
LinkBatchFactory (HistoryAction, InfoAction)
LinkRenderer (CreditsAction, HistoryAction, InfoAction, MarkpatrolledAction)
MagicWordFactory (InfoAction)
MainWANObjectCache (InfoAction)
NamespaceInfo (InfoAction)
PageProps (InfoAction, via PageProps::getInstance)
Parser (RawAction)
PermissionManager (HistoryAction, InfoAction, McrUndoAction, RawAction, WatchAction)
RepoGroup (InfoAction, RevertAction)
RestrictionStore (InfoAction, RawAction)
RevisionLookup (InfoAction, McrUndoAction, RawAction)
RevisionRenderer (McrUndoAction)
RevisionStore (HistoryAction, McrUndoAction)
SpecialPageFactory (SpecialPageAction)
UserFactory (CreditsAction, InfoAction, RawAction, all via User::newFromName)
UserOptionsLookup (RollbackAction, via User::getOption)
WatchedItemStore (InfoAction, WatchAction)
WatchlistManager (HistoryAction)
WikiPageFactory (WatchAction)

SpecialPageFactory::getPage can be used as a model for the desired ActionFactory::getAction

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

@eprodromou is it okay to move forward with this using ObjectFactory specs for DI? This has been in "Tech Planning Review" since May

@eprodromou is it okay to move forward with this using ObjectFactory specs for DI? This has been in "Tech Planning Review" since May

{{ping}}

@eprodromou is it okay to move forward with this using ObjectFactory specs for DI? This has been in "Tech Planning Review" since May

Yes :)

I wouldn't consider this high priority, but moving from a static factory methods to a factory service object is the right thing. It can use ObjectFactory just like we do it for SpecialPages and API modules.

Change 673795 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Add an ActionFactory and start converting to DI

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

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

[mediawiki/core@master] Inject services into WatchAction and UnwatchAction

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

Change 673795 merged by jenkins-bot:

[mediawiki/core@master] Add an ActionFactory and start converting to DI

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

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

[mediawiki/core@master] Inject services into a bunch more actions

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

For services that still need a factory method in the ObjectFactory spec, is there a suggested/recommended method name? In Wikibase we’ve usually used factory(), but of course that doesn’t work for actions, because they already inherit Action::factory().

(We used to call these methods newFromGlobalState(), but that became confusing/misleading when we started injecting dependencies into those methods.)

For services that still need a factory method in the ObjectFactory spec, is there a suggested/recommended method name? In Wikibase we’ve usually used factory(), but of course that doesn’t work for actions, because they already inherit Action::factory().

(We used to call these methods newFromGlobalState(), but that became confusing/misleading when we started injecting dependencies into those methods.)

Hmm, maybe createNew or something? Not sure - I don't think there is an official recommended name, thats just my suggestion

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

[mediawiki/core@master] Clean up SpecialPageAction name handling

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

Change 708626 merged by jenkins-bot:

[mediawiki/core@master] Inject services into WatchAction and UnwatchAction

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

Change 708627 merged by jenkins-bot:

[mediawiki/core@master] Inject services into a bunch more actions

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

Change 709192 merged by jenkins-bot:

[mediawiki/core@master] SpecialPageAction: inject action name and SpecialPageFactory

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

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

[mediawiki/core@master] Inject services into InfoAction

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

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

[mediawiki/core@master] Inject services into Mcr(Undo|Restore)Action

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

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

[mediawiki/core@master] Mark Action::getHookContainer() as @internal

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

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

[mediawiki/core@master] RELEASE-NOTES: document changes to $wgActions

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

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

[mediawiki/extensions/DiscussionTools@master] Injected SubscriptionStore into UnsubscribeAction

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

Change 709266 merged by jenkins-bot:

[mediawiki/core@master] RELEASE-NOTES: document changes to $wgActions

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

Change 709205 merged by jenkins-bot:

[mediawiki/core@master] Mark Action::getHookContainer() as @internal

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

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

[mediawiki/core@master] RELEASE-NOTES: fix typo (where -> were)

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

Change 710086 merged by jenkins-bot:

[mediawiki/core@master] RELEASE-NOTES: fix typo (where -> were)

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

Change 709203 merged by jenkins-bot:

[mediawiki/core@master] Inject services into InfoAction

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

Change 709204 merged by jenkins-bot:

[mediawiki/core@master] Inject services into Mcr(Undo|Restore)Action

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

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

[mediawiki/core@master] McrUndoAction: inject ReadOnlyMode

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

Change 710409 merged by jenkins-bot:

[mediawiki/core@master] McrUndoAction: inject ReadOnlyMode

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

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

[mediawiki/core@master] Action::addHelpLink() - don't call getActionName()

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

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

[mediawiki/core@master] Inject HookContainer into RawAction

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

Change 709267 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Injected SubscriptionStore into UnsubscribeAction

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

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

[mediawiki/core@master] CreditsAction: inject UserFactory

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

Change 712934 merged by jenkins-bot:

[mediawiki/core@master] CreditsAction: inject UserFactory

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

Change 710560 merged by jenkins-bot:

[mediawiki/core@master] Inject HookContainer into RawAction

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

Change 710559 merged by jenkins-bot:

[mediawiki/core@master] Action::addHelpLink() - don't call getActionName()

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

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

[mediawiki/core@master] ActionFactory: restore missing check for non-existent classes

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

Change 713525 merged by jenkins-bot:

[mediawiki/core@master] ActionFactory: restore missing check for non-existent classes

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

Change 957764 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/core@master] actions: Let getActionOverrides() return ObjectFactory specs

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

Change 957764 merged by jenkins-bot:

[mediawiki/core@master] actions: Let getActionOverrides() return ObjectFactory specs

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