Skip to content

[2.x] fix: Route queued jobs at push time instead of via a shared static $onQueue - #4853

Merged
imorland merged 1 commit into
2.xfrom
im/queue-job-routing-registry
Jul 29, 2026
Merged

[2.x] fix: Route queued jobs at push time instead of via a shared static $onQueue#4853
imorland merged 1 commit into
2.xfrom
im/queue-job-routing-registry

Conversation

@imorland

@imorland imorland commented Jul 29, 2026

Copy link
Copy Markdown
Member

The problem

AbstractJob::$onQueue is a static property on AbstractJob. A subclass that doesn't redeclare a static shares its parent's storage — so every job extending AbstractJob without its own $onQueue shares one slot. Routing two job classes collides (last write wins), and the same class can resolve differently across processes. In practice jobs silently land on whichever queue was assigned last. (The pattern dates back to 1.x's $sendOnQueue.)

The fix

Route jobs by class at push time, using Laravel's native queue-route manager.

  • queue.routes — bind Laravel's Illuminate\Queue\QueueRoutes, which resolves a job's queue by its class and hierarchy (parents, interfaces, traits).
  • Extend\Queue->route(JobClass::class, 'queue') — populates it. Routing an abstract/base class covers all subclasses; most specific wins.
  • RoutingQueue wraps flarum.queue.connection: on push()/later()/bulk() with no explicit queue, it applies the route. Laravel only applies routes via the bus dispatcher (dispatch()), but most of Flarum and its extensions use the low-level push() — the wrapper closes that gap so push() routes too. It's applied in QueueServiceProvider::boot(), wrapping whatever connection extensions bind (FoF Redis / Horizon), so routing is driver-independent.

The sync driver is left unwrapped (it runs jobs inline, so routing is moot) — which also preserves the instanceof SyncQueue checks used to detect "no real queue". The two instanceof DatabaseQueue checks unwrap via RoutingQueue::getDriver().

use Flarum\Extend;

return [
    (new Extend\Queue())
        ->route(\Your\Extension\Jobs\SendExportJob::class, 'exports'),
];

Breaking change

AbstractJob::$onQueue is removed; code that set it moves to Extend\Queue::route(). This PR migrates the bundled GDPR and Realtime extensions and updates the GDPR README.

Tests

  • RoutingQueueTest (unit): routing, explicit-queue-wins, unregistered pass-through, sibling classes independent, abstract-base routing, most-specific wins, later()/bulk(), string-job pass-through.
  • QueueTest (integration): Extend\Queue populates queue.routes, extenders compose.
  • QueueServiceProviderTest: the connection is a RoutingQueue wrapping the database driver.

Full core integration suite (760 tests) passes; GDPR + Realtime suites pass.

Docs: flarum/docs#565.

@imorland
imorland requested a review from a team as a code owner July 29, 2026 10:00
@imorland imorland changed the title Route queued jobs via a class-keyed registry instead of a shared static [2.x] fix: Route queued jobs via a class-keyed registry instead of a shared static Jul 29, 2026
@imorland imorland added this to the 2.0.0-rc.6 milestone Jul 29, 2026
@imorland
imorland force-pushed the im/queue-job-routing-registry branch from 0dae3e4 to f46ab12 Compare July 29, 2026 12:05
@imorland imorland changed the title [2.x] fix: Route queued jobs via a class-keyed registry instead of a shared static [2.x] fix: Route queued jobs at push time instead of via a shared static $onQueue Jul 29, 2026
@imorland
imorland force-pushed the im/queue-job-routing-registry branch from f46ab12 to 489b22f Compare July 29, 2026 12:20
@imorland
imorland marked this pull request as draft July 29, 2026 12:49
AbstractJob::$onQueue was a static property on the base class. A subclass
that does not redeclare a static shares its parent's storage, so routing two
job classes made the last assignment win for both — silently mis-routing jobs.
(This pattern dates back to 1.x's $sendOnQueue.)

Route jobs by class at the point they are pushed instead, using Laravel's
native queue-route manager:

- Bind Laravel's queue.routes (Illuminate\Queue\QueueRoutes), which resolves a
  job's queue by class and hierarchy (parents, interfaces, traits).
- Add Extend\Queue->route(JobClass::class, 'queue') to populate it.
- RoutingQueue wraps flarum.queue.connection and, on push()/later()/bulk()
  with no explicit queue, applies the route. Laravel only applies routes via
  the bus dispatcher; most of Flarum and its extensions use the low-level
  push(), so the wrapper closes that gap. Wrapping is done in boot() so it
  decorates whatever connection extensions bind (FoF Redis / Horizon), making
  routing driver-independent. The sync driver is left unwrapped (it runs jobs
  inline, so routing is moot and 'instanceof SyncQueue' checks are preserved);
  the two 'instanceof DatabaseQueue' checks unwrap via getDriver().

Removes AbstractJob::$onQueue; migrates the GDPR and Realtime extensions.
Breaking: code setting $onQueue must move to Extend\Queue::route().
@imorland
imorland force-pushed the im/queue-job-routing-registry branch from 489b22f to bb226ed Compare July 29, 2026 13:40
@imorland
imorland marked this pull request as ready for review July 29, 2026 14:17
@imorland
imorland merged commit e73790b into 2.x Jul 29, 2026
25 checks passed
@imorland
imorland deleted the im/queue-job-routing-registry branch July 29, 2026 14:17
imorland added a commit that referenced this pull request Jul 29, 2026
The queue connection is wrapped in a RoutingQueue (#4853) so pushes can be
routed by job class. identifyQueueDriver() derived the driver name from the
connection's class, so it began reporting 'routing' instead of the real
backend (redis/database/sync) in `flarum info` and the admin dashboard.

Unwrap a RoutingQueue via its public getDriver() before reading the class
name, so the reported driver reflects the actual backend again.
imorland added a commit to FriendsOfFlarum/horizon that referenced this pull request Jul 29, 2026
#34)

Core 2.x removed the shared static AbstractJob::$onQueue in favour of a
per-class queue-route map (flarum/framework#4853). fof/horizon's job
routing wrote to that static, so this switches it onto the new map:

- BuiltInRouting and the Horizon extender now resolve 'queue.routes' and
  call set(class, queue) instead of assigning $class::$onQueue.
- HorizonServiceProvider injects 'queue.routes' into both BuiltInRouting
  construction sites.

Routing is keyed per class and resolves through the class hierarchy, so
routing an abstract base covers its subclasses and multiple job classes
no longer collide on a shared static slot. Tests converted to assert via
the route map, with a regression test enabling realtime+gdpr+geoip
together and asserting each routes independently. The named-RedisQueue
connection test now asserts through the RoutingQueue wrapper's
getDriver().
imorland added a commit to flarum/docs that referenced this pull request Jul 29, 2026
* Document routing jobs to named queues via the Queue extender

Replaces the removed AbstractJob::$onQueue static (see flarum/framework#4853)
with Extend\Queue->route().

* Document abstract-base job routing and most-specific-wins precedence

* Note job routing change in the 2.x upgrade guide

* Document job queue routing in the 1.x to 2.0 upgrade guide

* Frame queue routing upgrade around 1.x $sendOnQueue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant