ApiPageSet uses a lot of different services, and does not work as its own api module, but rather is used to help other api modules. I propose that it be converted to a dedicated service where all dependencies can be injected.
Description
Details
Subject | Repo | Branch | Lines +/- | |
---|---|---|---|---|
api: Get all services for ApiPageSet in constructor | mediawiki/core | master | +65 -38 |
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Open | None | T259960 Inject services into API modules and special pages | |||
Open | None | T283314 Convert ApiPageSet to a service rather than a subclass of ApiBase |
Event Timeline
@BPirkle can you clarify if Platform Engineering would support such a change? I'm happy to try and write the code, but would prefer to get signoff that it would be approved before investing the time
We discussed this briefly at the tech planning meeting.
My take is that ApiPageSet should indeed not be a subclass of ApiBase, since it's not an API module. It can't be a service object though, since it is stateful. It's basically a fancy container object that wraps database logic for looking up and normalizing titles.
My proposal is to create a factory service for ApiPageSets, and treat the ApiPageSet itself like (rather complex) a query builder / result set.
An own service for this seems overkill, because it is only usable in context of Api.
It also not belongs to the ModuleManager, because it is not a module
Maybe a factory function on ApiMain (ApiMain::createPageSet( ... )) could be used here, but ApiMain itself is not ready for injection (T265644)
I have moved some of the services into the constructor with https://gerrit.wikimedia.org/r/c/mediawiki/core/+/703064
HookRunner and DB is missing and still used from ApiBase
Needs fallback in the constructor for all ideas - but it seems ApiPageSet is not part of the stable interface!?
Change 703064 had a related patch set uploaded (by Umherirrender; author: Umherirrender):
[mediawiki/core@master] api: Get all services for ApiPageSet in constructor
Change 703064 merged by jenkins-bot:
[mediawiki/core@master] api: Get all services for ApiPageSet in constructor