Page MenuHomePhabricator

Research to create service for DeferredUpdates::addUpdate()
Open, Needs TriagePublic


The class DeferredUpdates is using some services for database

Event Timeline

I've looked a bit into this. First of all, the DeferredUpdates class seems to be "ready" for servicification, in that the class itself already has well-defined responsibilities, methods have the proper visibility modifiers etc. Thus, creating a service from it would mostly be a matter of injecting some dependencies.

The only thing that needs a little bit of thought is how to handle the persistency of the state. Right now this is achieved with three static properties. The new service could either keep this behaviour, or become a singleton class. I don't have the knowledge necessary to pick an option here.

Either way, I think a general scheme might be to create a DeferredUpdatesRegistry (or Manager, or whatnot) with the same public interface as the current DeferredUpdates class, except not static. The DeferredUpdates class would simply proxy the calls to the new service, and become deprecated.

@aaron You're probably the best person to ask to. What do you think about this?

It would be good to get cleanup patches like merged first.

Makes sense. I might review some of these cleanups, but I don't know that code enough to review the patch in its current form (i.e. as a single patch which does lots of things).

See also for an early stab at this.

Yeah, that seems similar to what I was thinking about. As a note, the queue properties were declared non-static there. While this is probably going to work (mostly) because of MediaWikiServices being a singleton, I think it wouldn't be bad to make them static, because we really don't want to lose any update.