Skip to content

refactor(extensions): per-queue Factory; drop queue from method signatures#188

Merged
behinddwalls merged 1 commit into
mainfrom
ext-factory
Jun 4, 2026
Merged

refactor(extensions): per-queue Factory; drop queue from method signatures#188
behinddwalls merged 1 commit into
mainfrom
ext-factory

Conversation

@behinddwalls
Copy link
Copy Markdown
Collaborator

@behinddwalls behinddwalls commented Jun 3, 2026

Summary

Why?

Behavioral extensions were global singletons frozen at process start, and two leaked the queue into vendor-agnostic method signatures (mergechecker.Check, buildrunner.Trigger). Each extension should instead be produced by a Factory the controller asks for using the queue, identified only by name, so an implementation can be selected/configured per queue.

What?

  • Reduce entity.QueueConfig to {Name} (the registry of valid queues); all behavioral/VCS config moves into the factory implementations.
  • Each extension (changeprovider, pusher, scorer, conflict, mergechecker, buildrunner) ships a narrow Config{QueueName} and a Factory interface (For(cfg) (T, error)); concrete factories are written by integrators in the example wiring and tests. Generated mocks updated.
  • Drop the queue argument from mergechecker.Check(ctx, change) and buildrunner.Trigger(ctx, base, head, metadata); delete mergechecker.MultiChecker. Status/Cancel stay buildID-keyed.
  • Add storage.Factory (default mysql) and thread it into every controller; the gateway land controller resolves For(req.Queue) — the one place per-queue storage routing is actionable. Orchestrator controllers carry a TODO(queue-aware) to derive the queue from the loaded entity for logging/metrics/storage.
  • Controllers resolve each extension from its Factory using the loaded entity's queue; buildsignal fetches the batch before polling Status. Example servers wire concrete static factory structs.

Stacked on #187 (now merged); rebased onto main.

Test Plan

make build, make test (40 pass)
make fmt / lint / check-gazelle / check-tidy / check-mocks
✅ Integration + e2e (Docker) verified locally: counter, gateway, orchestrator, e2e suites pass

@behinddwalls behinddwalls marked this pull request as ready for review June 3, 2026 22:55
@behinddwalls behinddwalls requested review from a team and sbalabanov as code owners June 3, 2026 22:56
@behinddwalls behinddwalls marked this pull request as draft June 3, 2026 22:57
@behinddwalls behinddwalls marked this pull request as ready for review June 3, 2026 23:08
Base automatically changed from messagequeue-moves to main June 3, 2026 23:13
Copy link
Copy Markdown

@JamyDev JamyDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit 🥲

Comment thread submitqueue/orchestrator/controller/batch/batch.go Outdated
Comment thread submitqueue/orchestrator/controller/batch/batch.go Outdated
Comment thread submitqueue/orchestrator/controller/batch/batch.go Outdated
Comment thread submitqueue/orchestrator/controller/batch/batch.go Outdated
Comment thread submitqueue/orchestrator/controller/batch/batch.go Outdated
Comment thread submitqueue/orchestrator/controller/speculate/speculate.go Outdated
Comment thread submitqueue/orchestrator/controller/speculate/speculate.go Outdated
Comment thread submitqueue/orchestrator/controller/speculate/speculate.go Outdated
Comment thread submitqueue/orchestrator/controller/speculate/speculate.go Outdated
Comment thread submitqueue/orchestrator/controller/validate/validate.go Outdated
…tures

## Summary

### Why?

Behavioral extensions were global singletons frozen at process start, and two
leaked the queue into vendor-agnostic method signatures (mergechecker.Check,
buildrunner.Trigger). Each extension should instead be produced by a Factory
the controller asks for using the queue, identified only by name, so an
implementation can be selected/configured per queue.

### What?

- Reduce entity.QueueConfig to {Name} (the registry of valid queues); all
  behavioral/VCS config moves into the factory implementations.
- Each extension (changeprovider, pusher, scorer, conflict, mergechecker,
  buildrunner) ships a narrow Config{QueueName} and a Factory interface
  (For(cfg) (T, error)); concrete factories are written by integrators in the
  example wiring and tests. Generated mocks updated.
- Drop the queue argument from mergechecker.Check(ctx, change) and
  buildrunner.Trigger(ctx, base, head, metadata); delete mergechecker
  MultiChecker. Status/Cancel stay buildID-keyed.
- Add storage.Factory (default mysql) and thread it into every controller; the
  gateway land controller resolves For(req.Queue) — the one place per-queue
  storage routing is actionable. Orchestrator controllers carry a
  TODO(queue-aware) to derive the queue from the loaded entity for
  logging/metrics/storage.
- Controllers resolve each extension from its Factory using the loaded entity's
  queue; buildsignal fetches the batch before polling Status. Example servers
  wire concrete static factory structs.

## Test Plan

✅ make build, make test (40 pass)
✅ make fmt / lint / check-gazelle / check-tidy / check-mocks
@behinddwalls
Copy link
Copy Markdown
Collaborator Author

nit 🥲

Fixed

@behinddwalls behinddwalls requested a review from JamyDev June 4, 2026 02:28
@behinddwalls behinddwalls merged commit 61dd1ae into main Jun 4, 2026
14 checks passed
@behinddwalls behinddwalls deleted the ext-factory branch June 4, 2026 02:30
behinddwalls added a commit that referenced this pull request Jun 4, 2026
## Summary

### Why?

PR #188 added `storage.Factory` (`For(queue) (Storage, error)`) and
threaded it into every controller, leaving a `TODO(queue-aware)` in all
11 orchestrator controllers: each fell back to `stores.For("")` because
storage is needed *before* the by-ID entity load, so the queue isn't
known yet. That chicken-and-egg was the sole reason for the TODO.

Behavioral extensions (mergechecker, buildrunner, scorer, conflict,
changeprovider, pusher) are already correctly per-queue — resolved
*after* the load from the loaded entity's queue. Storage, by contrast,
does not vary by queue: every queue shares the same entity schema and
tables. Per-queue storage, if ever needed, is better achieved by
splitting the environment and wiring different stores per deployment,
not an in-process Factory.

### What?

- Remove the `storage.Factory` interface and delete the `staticFactory`
/ `NewStaticFactory` implementation (+ its test). Regenerate the storage
mock (drops `MockFactory`).
- Switch the 11 orchestrator controllers and the gateway `land`
controller from `storage.Factory` to a single injected
`storage.Storage`; drop the `stores.For("")` hack and the
`TODO(queue-aware)` comments. Queue for logging and behavioral-extension
resolution continues to come from the loaded entity.
- Update example wiring (gateway + orchestrator `main.go`) to pass the
store directly instead of `NewStaticFactory(store)`.
- Behavioral-extension Factories are unchanged.

No behavior change under today's single-store deployment — `For("")`
already returned this exact store.

## Test Plan

✅ `make build`, `make test` (39 pass)
✅ `make gazelle` / `make mocks` / `make tidy` / `make fmt` (all in sync)
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.

2 participants