Page MenuHomePhabricator

Remove Module/Router dependency from Handler
Open, MediumPublic8 Estimated Story Points

Description

Split off from my comment on T405373:

I wonder how hard it would be to remove the following methods/dependencies from Handler:

  • getModule(): callers use this to get the module prefix or deprecation flag/timestamp. These can be injected instead. There could be a Hander::getModulePathPrefix() that returns the injected module prefix string.
  • getRouter(): callers use this to then call Router::isRestbaseCompatEnabled() or Router::getRouteUrl()/Router::getRoutePath(). The former could be replace by a static RestbaseCompat::isRestbaseCompatEnabled() helper method. The later could be replaced by injecting a RouteLocator object for making paths/URLs for routes, which would not depend on Router. CreationHandler::mapActionModuleResponse could use $this->getRouteLocator()->getRouteUrl( '/v1/page/' . $title ). RedirectHandler could use $this->getRouteLocator()->getRoutePath( $path, $pathParams, $queryParams. It's interesting how many Handlers assume that some specific Handler exists at some hard-coded path (e.g. AddItemStatementRouteHandler::setLocationHeader or CreationHandler::mapActionModuleResponse). I think that these should be injected via route config instead of being hard-coded in the Handler. Handlers should not assume the location of themselves nor other Handlers.

Getting Module/Router out of Handler would make Handler construction much easier. It would also better justify a HandlerFactory since there would not already be a Router instance for use in ModuleSpecHandler.

  • Replace Handler module calls with injected deprecation settings form the module's info
  • Replace router calls with the new injected RouteLocator object and RestbaseCompat::isRestbaseCompatEnabled helper method
  • Update Handler initialization calls in Module and Router with the new injectable args
  • Start the deprecation process for $this->module, $this->getModule, $this->getRouter from Handler Base class.

The last 2 items are subject to the Stable interface policy . Expected to take atleast 6 months with 3 months of them($this->module, $this->getModule, $this->getRouter) being hard deprecated in master and atleast one major release.

  • Ensure all builds are tests not broken
  • Update all documentations/wikis with this new Base Handler changes

Event Timeline

aaron added a subscriber: • Atieno.

I've spent some time looking at this.

TL;DR from the description into a checklist
Edit: Moved the checklist to the task description

OWresch-WMF set the point value for this task to 8.Feb 12 2026, 4:42 PM
KineticPelagic renamed this task from Remove Module/Route dependency from Handler to Remove Module/Router dependency from Handler.Sep 15 2026, 9:29 AM

Context

While working on T433516: REST vNext: ModuleManager should encapsulate module configuration, we explored two approaches to resolve module base and spec URLs:

  1. Change 1333145 (active approach): Place module base and spec URL resolution in Router (Router::getModuleBaseUrl() and Router::getModuleSpecUrl()) to reuse Router::getRouteUrl() and keep ModuleInfo a pure value object. Change-Id: I7695d3
  2. Change 1341112 (alternative approach): Inject UrlUtils and $wgCanonicalServer into ModuleManager to pre-render absolute URLs upfront on ModuleInfo (getAbsoluteBaseUrl() and getAbsoluteSpecUrl()), eliminating the need for handlers to call Router for these URLs.

Why I abandoned 1341112

As a team, we chose the approach in 1333145 for T433516 because DiscoveryHandler and ModuleSpecHandler already depend on Router::getRouteUrl(). Having ModuleManager hand-roll route URLs via string concatenation risked diverging from core router path rules.

However, as @aaron noted during code review, codifying Router usage in handlers is exactly what this task (T411521).

Implementation note

When we work on T411521 to decouple Handler from Router, change 1341112 may be a useful reference for how to extract URL construction away from Router without circular dependencies.