Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ Vendor-agnostic, pluggable interfaces with implementations in subdirectories:
2. Implementations at `extension/{ext}/{impl}/`
3. Factory interface for dependency injection and lifecycle management

**Extensions hold contracts and impls only — never factory implementations or routing.** An `extension/{ext}` package owns the behavioral interface, its `Config`, the `Factory` *interface*, and impl constructors `New(...)` that return the interface. It must NOT contain `Factory` *implementations* (no `NewFactory()` constructors or factory structs) and no queue-selection/routing logic. The reason: an impl package (e.g. `scorer/heuristic`) cannot know the queue topology or the other impls, so any "which impl for which queue" decision baked there is wrong by construction. A `Factory` whose `For(cfg)` ignores `cfg.QueueName` is just an impl→Factory adapter — that adapting, and all per-queue routing, belongs in the **wiring/integrator layer** (e.g. `example/{domain}/{service}/server/main.go`), which is the one place that knows the queue set. There, adapt `New(...)` impls to the `Factory` contract and route on `Config.QueueName`. Keep this seam strict: if you're about to add a `NewFactory()` or a `map[queue]impl` under `extension/`, it belongs in the wiring layer instead.

**Design interfaces for the technology *space*, not the implementation in front of you.** The interface is a contract every backend will have to satisfy — SQL, key-value (DynamoDB, Bigtable), document, message queue, search, RPC, in-memory, mocks. If the contract assumes a capability that some plausible backend can't provide cheaply, you've baked the current impl's strengths into the API.

Common over-constraints to avoid:
Expand Down
4 changes: 4 additions & 0 deletions example/submitqueue/gateway/server/queues.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,7 @@ queues:
- name: test-queue
- name: e2e-test-queue
- name: e2e-cancel-queue
# Routes to an analyzer that always errors (conflictfake.FailAlways) so e2e can
# exercise the conflict-analysis error path. See newQueueRegistry in the
# orchestrator example server.
- name: e2e-conflict-error-queue
9 changes: 8 additions & 1 deletion example/submitqueue/orchestrator/server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,23 @@ go_library(
"//submitqueue/core/consumer",
"//submitqueue/entity",
"//submitqueue/extension/buildrunner",
"//submitqueue/extension/buildrunner/noop",
"//submitqueue/extension/buildrunner/fake",
"//submitqueue/extension/changeprovider",
"//submitqueue/extension/changeprovider/fake",
"//submitqueue/extension/changeprovider/github",
"//submitqueue/extension/conflict",
"//submitqueue/extension/conflict/all",
"//submitqueue/extension/conflict/fake",
"//submitqueue/extension/conflict/none",
"//submitqueue/extension/mergechecker",
"//submitqueue/extension/mergechecker/fake",
"//submitqueue/extension/mergechecker/github",
"//submitqueue/extension/pusher",
"//submitqueue/extension/pusher/fake",
"//submitqueue/extension/pusher/git",
"//submitqueue/extension/scorer",
"//submitqueue/extension/scorer/composite",
"//submitqueue/extension/scorer/fake",
"//submitqueue/extension/scorer/heuristic",
"//submitqueue/extension/storage",
"//submitqueue/extension/storage/mysql",
Expand Down
309 changes: 216 additions & 93 deletions example/submitqueue/orchestrator/server/main.go

Large diffs are not rendered by default.

24 changes: 0 additions & 24 deletions submitqueue/extension/buildrunner/noop/BUILD.bazel

This file was deleted.

56 changes: 0 additions & 56 deletions submitqueue/extension/buildrunner/noop/noop.go

This file was deleted.

61 changes: 0 additions & 61 deletions submitqueue/extension/buildrunner/noop/noop_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion submitqueue/orchestrator/controller/build/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ go_test(
"//submitqueue/core/consumer",
"//submitqueue/entity",
"//submitqueue/extension/buildrunner",
"//submitqueue/extension/buildrunner/fake",
"//submitqueue/extension/buildrunner/mock",
"//submitqueue/extension/buildrunner/noop",
"//submitqueue/extension/storage",
"//submitqueue/extension/storage/mock",
"@com_github_stretchr_testify//assert",
Expand Down
14 changes: 7 additions & 7 deletions submitqueue/orchestrator/controller/build/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import (
"github.com/uber/submitqueue/submitqueue/core/consumer"
"github.com/uber/submitqueue/submitqueue/entity"
"github.com/uber/submitqueue/submitqueue/extension/buildrunner"
buildfake "github.com/uber/submitqueue/submitqueue/extension/buildrunner/fake"
buildrunnermock "github.com/uber/submitqueue/submitqueue/extension/buildrunner/mock"
buildnoop "github.com/uber/submitqueue/submitqueue/extension/buildrunner/noop"
"github.com/uber/submitqueue/submitqueue/extension/storage"
storagemock "github.com/uber/submitqueue/submitqueue/extension/storage/mock"
"go.uber.org/mock/gomock"
Expand Down Expand Up @@ -74,7 +74,7 @@ func newMockStorage(ctrl *gomock.Controller, batch entity.Batch) *storagemock.Mo
}

// newTestController creates a controller with test dependencies. br is the
// build runner to inject; pass buildnoop.New() for the pass-through default.
// build runner to inject; pass buildfake.New() for the pass-through default.
// staticBuildRunnerFactory is a test factory that returns a fixed BuildRunner
// for any entityqueue.
type staticBuildRunnerFactory struct{ r buildrunner.BuildRunner }
Expand Down Expand Up @@ -111,7 +111,7 @@ func TestNewController(t *testing.T) {
ctrl := gomock.NewController(t)
batch := testBatch()
store := newMockStorage(ctrl, batch)
controller := newTestController(t, ctrl, store, buildnoop.New(), nil)
controller := newTestController(t, ctrl, store, buildfake.New(), nil)

require.NotNil(t, controller)
assert.Equal(t, consumer.TopicKeyBuild, controller.TopicKey())
Expand All @@ -124,7 +124,7 @@ func TestController_Process_Success(t *testing.T) {

batch := testBatch()
store := newMockStorage(ctrl, batch)
controller := newTestController(t, ctrl, store, buildnoop.New(), nil)
controller := newTestController(t, ctrl, store, buildfake.New(), nil)

msg := entityqueue.NewMessage(batch.ID, batchIDPayload(t, batch.ID), batch.Queue, nil)
delivery := queuemock.NewMockDelivery(ctrl)
Expand Down Expand Up @@ -315,7 +315,7 @@ func TestController_Process_StorageFailure(t *testing.T) {
store.EXPECT().GetRequestStore().Return(storagemock.NewMockRequestStore(ctrl)).AnyTimes()
store.EXPECT().GetBuildStore().Return(storagemock.NewMockBuildStore(ctrl)).AnyTimes()

controller := newTestController(t, ctrl, store, buildnoop.New(), nil)
controller := newTestController(t, ctrl, store, buildfake.New(), nil)

msg := entityqueue.NewMessage("test-queue/batch/1", batchIDPayload(t, "test-queue/batch/1"), "test-queue", nil)
delivery := queuemock.NewMockDelivery(ctrl)
Expand All @@ -332,7 +332,7 @@ func TestController_Process_PublishFailure(t *testing.T) {

batch := testBatch()
store := newMockStorage(ctrl, batch)
controller := newTestController(t, ctrl, store, buildnoop.New(), fmt.Errorf("publish failed"))
controller := newTestController(t, ctrl, store, buildfake.New(), fmt.Errorf("publish failed"))

msg := entityqueue.NewMessage(batch.ID, batchIDPayload(t, batch.ID), batch.Queue, nil)
delivery := queuemock.NewMockDelivery(ctrl)
Expand All @@ -347,7 +347,7 @@ func TestController_InterfaceImplementation(t *testing.T) {
ctrl := gomock.NewController(t)
batch := testBatch()
store := newMockStorage(ctrl, batch)
controller := newTestController(t, ctrl, store, buildnoop.New(), nil)
controller := newTestController(t, ctrl, store, buildfake.New(), nil)

var _ consumer.Controller = controller
}
Expand Down
Loading