refactor: turn the repo into a two-plugin marketplace
tea and tdl were two repositories, each carrying its own
.claude-plugin/marketplace.json — two marketplaces to register for what
is one collection. Fold them into one.
The repo root is now the marketplace and nothing else: a single
.claude-plugin/marketplace.json whose entries point at ./plugins/tea and
./plugins/tdl. A plugin's root is its own directory under plugins/, so
${CLAUDE_PLUGIN_ROOT} still resolves inside it and every path a plugin
uses stays relative to itself — the hooks and the test roots needed no
adjustment beyond the move.
tea's files move with git mv, so its history and blame follow. tdl
arrives as a plain copy; its history stays in claude-skills/threedotslab.
test_payload_root asserted `tmp/` was ignored by REPO/.gitignore. The
rule is that tmp/ is ignored, not which file says so, and git reads every
.gitignore on the way up — so the test now walks up to the repo root the
same way git does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
naudachu
@@ -0,0 +1,480 @@
|
||||
# Architecture Rules (ARCH-01..08)
|
||||
|
||||
## ARCH-01: Standard Directory Layout (CRITICAL)
|
||||
|
||||
Every service MUST follow this directory structure:
|
||||
|
||||
```
|
||||
<service>/
|
||||
├── domain/<aggregate>/ # Pure business logic, entities, value objects, repository interfaces
|
||||
├── app/ # Application struct (app.go) with Commands + Queries
|
||||
│ ├── command/ # Write use cases (command handlers)
|
||||
│ └── query/ # Read use cases (query handlers + read model interfaces)
|
||||
├── ports/ # Inbound adapters: HTTP handlers, gRPC servers, CLI
|
||||
├── adapters/ # Outbound adapters: repository implementations, external clients
|
||||
└── service/ # Composition root: wires all dependencies together
|
||||
```
|
||||
|
||||
**Check procedure:**
|
||||
1. Glob for these directories relative to the service root
|
||||
2. Flag any missing standard directories
|
||||
3. Flag any non-standard directories at the same level (e.g., `controllers/`, `models/`, `handlers/`)
|
||||
4. Multiple aggregates can exist under `domain/` as sub-packages (e.g., `domain/hour/`, `domain/training/`)
|
||||
|
||||
**Reference (wild-workouts):**
|
||||
```
|
||||
internal/trainer/
|
||||
├── domain/hour/
|
||||
├── app/
|
||||
│ ├── command/
|
||||
│ └── query/
|
||||
├── ports/
|
||||
├── adapters/
|
||||
└── service/
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ARCH-02: Dependency Direction (CRITICAL)
|
||||
|
||||
Dependencies MUST flow inward only: `ports/adapters → app → domain`
|
||||
|
||||
The domain layer MUST NOT import from:
|
||||
- `app/`, `app/command/`, `app/query/`
|
||||
- `ports/`
|
||||
- `adapters/`
|
||||
- Any external infrastructure package (database drivers, HTTP frameworks, etc.)
|
||||
|
||||
The app layer MUST NOT import from:
|
||||
- `ports/`
|
||||
- `adapters/`
|
||||
|
||||
**Check procedure:**
|
||||
1. For every `.go` file in `domain/`, scan import statements
|
||||
2. Flag any import that references `app/`, `ports/`, `adapters/`, or the service's own non-domain packages
|
||||
3. For every `.go` file in `app/`, scan imports for `ports/` or `adapters/`
|
||||
4. Domain MAY import standard library and pure utility packages
|
||||
|
||||
**Allowed domain imports:**
|
||||
- Standard library (`context`, `time`, `errors`, `fmt`, `strings`, etc.)
|
||||
- Pure value libraries (e.g., `github.com/google/uuid`)
|
||||
- NOT: database drivers, HTTP routers, gRPC, logging libraries
|
||||
|
||||
---
|
||||
|
||||
## ARCH-03: Composition Root Isolation (CRITICAL)
|
||||
|
||||
All dependency wiring MUST happen exclusively in `service/`. The composition root is the **only** place that knows about concrete adapter types, infrastructure clients, and how dependencies connect.
|
||||
|
||||
**`main.go`** MUST only:
|
||||
1. Initialize cross-cutting concerns (logging)
|
||||
2. Call `service.NewApplication()`
|
||||
3. Wire ports (pass `app.Application` to port constructors)
|
||||
4. Start the server
|
||||
|
||||
`main.go` MUST NOT import `adapters/`, create infrastructure clients, or instantiate command/query handlers directly.
|
||||
|
||||
**Check procedure:**
|
||||
1. Scan `main.go` imports — flag any reference to `adapters/`, database drivers, or external service clients
|
||||
2. Scan all files outside `service/` — flag any call to adapter constructors (e.g., `adapters.New*`)
|
||||
3. Verify `service/` returns `app.Application`
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// main.go — only knows about service and ports
|
||||
func main() {
|
||||
logs.Init()
|
||||
ctx := context.Background()
|
||||
|
||||
app, cleanup := service.NewApplication(ctx)
|
||||
defer cleanup()
|
||||
|
||||
server.RunHTTPServer(func(router chi.Router) http.Handler {
|
||||
return ports.HandlerFromMux(ports.NewHttpServer(app), router)
|
||||
})
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// main.go — VIOLATION: wiring infrastructure directly
|
||||
func main() {
|
||||
client, _ := firestore.NewClient(ctx, os.Getenv("GCP_PROJECT")) // VIOLATION
|
||||
repo := adapters.NewFirestoreRepository(client) // VIOLATION
|
||||
handler := command.NewScheduleTrainingHandler(repo, logger, mc) // VIOLATION
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ARCH-04: Dual Constructor Pattern for Testability (WARNING)
|
||||
|
||||
The composition root MUST provide two constructors sharing a single private wiring function:
|
||||
1. **`NewApplication(ctx) (app.Application, func())`** — production constructor, creates real infrastructure
|
||||
2. **`NewComponentTestApplication(ctx) app.Application`** — test constructor, injects mocks/stubs
|
||||
|
||||
Both MUST delegate to a **private** `newApplication(...)` that accepts dependencies as interfaces, so the real vs test paths only differ in what they pass in.
|
||||
|
||||
This ensures:
|
||||
- Test mocks never leak into production wiring
|
||||
- All wiring logic is shared — no drift between prod and test setups
|
||||
- The private function signature documents the full set of external dependencies
|
||||
|
||||
**Check procedure:**
|
||||
1. Look for exported `NewApplication` and `NewComponentTestApplication` in `service/`
|
||||
2. Verify both call the same unexported function
|
||||
3. The unexported function MUST accept dependencies as interfaces, not concrete types
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// service/service.go
|
||||
func NewApplication(ctx context.Context) (app.Application, func()) {
|
||||
trainerClient, closeTrainer, err := client.NewTrainerClient()
|
||||
if err != nil { panic(err) }
|
||||
|
||||
trainerService := adapters.NewTrainerGrpc(trainerClient)
|
||||
|
||||
return newApplication(ctx, trainerService),
|
||||
func() { _ = closeTrainer() }
|
||||
}
|
||||
|
||||
func NewComponentTestApplication(ctx context.Context) app.Application {
|
||||
return newApplication(ctx, TrainerServiceMock{})
|
||||
}
|
||||
|
||||
func newApplication(ctx context.Context, trainerService command.TrainerService) app.Application {
|
||||
// shared wiring logic — accepts interfaces, not concrete types
|
||||
repo := adapters.NewFirestoreRepository(client)
|
||||
return app.Application{ /* ... */ }
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// VIOLATION: separate wiring paths, no shared private function
|
||||
func NewApplication(ctx context.Context) app.Application {
|
||||
repo := adapters.NewFirestoreRepository(client)
|
||||
return app.Application{
|
||||
Commands: app.Commands{
|
||||
ScheduleTraining: command.NewScheduleTrainingHandler(repo, logger, mc),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func NewTestApplication() app.Application {
|
||||
repo := NewMockRepo() // VIOLATION: duplicated wiring, can drift
|
||||
return app.Application{
|
||||
Commands: app.Commands{
|
||||
ScheduleTraining: command.NewScheduleTrainingHandler(repo, logger, mc),
|
||||
},
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ARCH-05: Cleanup Function for Resource Lifecycle (WARNING)
|
||||
|
||||
When the composition root creates resources that require cleanup (connections, clients, subscriptions), `NewApplication` MUST return a cleanup function alongside the application. The caller owns the lifecycle via `defer`.
|
||||
|
||||
This ensures:
|
||||
- Resources are released even on panic
|
||||
- `main.go` doesn't need to know *what* to clean up — just *that* it must
|
||||
- Adding new infrastructure only changes `service/`, not `main.go`
|
||||
|
||||
**Check procedure:**
|
||||
1. If `NewApplication` creates closeable resources (clients, connections), it MUST return `func()`
|
||||
2. `main.go` MUST call `defer cleanup()` immediately after receiving it
|
||||
3. The cleanup function MUST NOT be ignored (assigned to `_`)
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// service/service.go
|
||||
func NewApplication(ctx context.Context) (app.Application, func()) {
|
||||
trainerClient, closeTrainer, err := client.NewTrainerClient()
|
||||
if err != nil { panic(err) }
|
||||
usersClient, closeUsers, err := client.NewUsersClient()
|
||||
if err != nil { panic(err) }
|
||||
|
||||
return newApplication(ctx, adapters.NewTrainerGrpc(trainerClient), adapters.NewUsersGrpc(usersClient)),
|
||||
func() {
|
||||
_ = closeTrainer()
|
||||
_ = closeUsers()
|
||||
}
|
||||
}
|
||||
|
||||
// main.go
|
||||
app, cleanup := service.NewApplication(ctx)
|
||||
defer cleanup()
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// VIOLATION: caller must know internals to clean up
|
||||
func NewApplication(ctx context.Context) (app.Application, *firestore.Client, *grpc.ClientConn) {
|
||||
// ...
|
||||
}
|
||||
|
||||
// VIOLATION: cleanup responsibility leaks into main
|
||||
app, fsClient, conn := service.NewApplication(ctx)
|
||||
defer fsClient.Close() // main.go shouldn't know about Firestore
|
||||
defer conn.Close() // main.go shouldn't know about gRPC
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ARCH-06: Server Startup via Callback (WARNING)
|
||||
|
||||
Server startup MUST be delegated to a shared `server.Run*Server()` function. `main.go` provides **only the application handler** via a callback. It MUST NOT configure server internals: middleware, routing, listening address, or transport-level concerns.
|
||||
|
||||
This ensures:
|
||||
- Middleware stack (auth, logging, recovery, CORS, security headers) is consistent across all services
|
||||
- Adding or changing middleware is a single change, not per-service
|
||||
- `main.go` remains a thin orchestrator: init → wire app → provide handler → run
|
||||
|
||||
**Check procedure:**
|
||||
1. `main.go` MUST call a shared `Run*Server()` function as the final blocking call
|
||||
2. The callback passed to `Run*Server()` MUST only construct the handler from port constructors — no middleware setup, no router configuration, no listener creation
|
||||
3. `main.go` MUST NOT import server infrastructure packages (e.g., `net/http.ListenAndServe`, `net.Listen`, middleware libraries)
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// main.go — provides handler, delegates everything else
|
||||
func main() {
|
||||
logs.Init()
|
||||
ctx := context.Background()
|
||||
|
||||
app, cleanup := service.NewApplication(ctx)
|
||||
defer cleanup()
|
||||
|
||||
server.RunHTTPServer(func(router chi.Router) http.Handler {
|
||||
return ports.HandlerFromMux(ports.NewHttpServer(app), router)
|
||||
})
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// VIOLATION: main.go configures server internals
|
||||
func main() {
|
||||
app, cleanup := service.NewApplication(ctx)
|
||||
defer cleanup()
|
||||
|
||||
router := chi.NewRouter()
|
||||
router.Use(middleware.Logger) // VIOLATION: middleware in main
|
||||
router.Use(middleware.Recoverer) // VIOLATION: middleware in main
|
||||
router.Mount("/api", ports.NewHttpServer(app))
|
||||
|
||||
http.ListenAndServe(":8080", router) // VIOLATION: listening in main
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ARCH-07: Composition Root Must Not Own Server Lifecycle (CRITICAL)
|
||||
|
||||
The `service/` package wires dependencies and returns `app.Application`. It MUST NOT create transport servers, bind to network ports, handle OS signals, or manage graceful shutdown. Server lifecycle is a **separate concern** that belongs in a shared server package or the entry point.
|
||||
|
||||
`service/` MUST NOT:
|
||||
- Create transport servers (`grpc.NewServer()`, `http.Server{}`, `message.NewRouter()`)
|
||||
- Bind to network ports (`net.Listen()`)
|
||||
- Handle OS signals (`signal.NotifyContext()`, `signal.Notify()`)
|
||||
- Manage graceful shutdown (`GracefulStop()`, `router.Close()`)
|
||||
- Import port packages (`ports/grpc`, `ports/amqp`, `ports/http`)
|
||||
|
||||
`service/` MUST only:
|
||||
- Create infrastructure clients and adapters
|
||||
- Wire command/query handlers with dependencies
|
||||
- Return `app.Application` (and optionally a cleanup function)
|
||||
|
||||
**Check procedure:**
|
||||
1. Scan all files in `service/` for imports of `net`, `os/signal`, `syscall`, transport packages, or `ports/`
|
||||
2. Flag any function in `service/` that accepts or creates a server, listener, or router
|
||||
3. A file named `server.go` in `service/` is a strong signal of violation
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// service/service.go — only wires the application
|
||||
func NewApplication(ctx context.Context, cfg *config.Config) (app.Application, func()) {
|
||||
repo := adapters.NewFirestoreRepository(client)
|
||||
syncer := tokensync.NewSyncer(fetchers, syncRepo, progressTracker)
|
||||
|
||||
return newApplication(repo, syncer),
|
||||
func() { _ = client.Close() }
|
||||
}
|
||||
|
||||
// Server lifecycle lives elsewhere (shared server package or entry point)
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// service/server.go — VIOLATION: server lifecycle in composition root
|
||||
func RunServer(application app.Application, cfg *config.Config) error {
|
||||
ctx, stop := signal.NotifyContext(context.Background(), ...) // VIOLATION: signal handling
|
||||
defer stop()
|
||||
|
||||
grpcServer := grpc.NewServer() // VIOLATION: transport server
|
||||
pb.RegisterCommandsServer(grpcServer, ports.NewServer(app)) // VIOLATION: imports ports/
|
||||
|
||||
lis, _ := net.Listen("tcp", fmt.Sprintf(":%s", cfg.Port)) // VIOLATION: network binding
|
||||
go grpcServer.Serve(lis) // VIOLATION: server lifecycle
|
||||
|
||||
<-ctx.Done()
|
||||
grpcServer.GracefulStop() // VIOLATION: shutdown management
|
||||
return nil
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ARCH-08: Unified Server with Named Components and OnShutdown (WARNING)
|
||||
|
||||
When a project has multiple transports (gRPC, HTTP, AMQP/Watermill), the shared server package SHOULD provide a **single `server.New(...).Run(ctx)`** with functional options per transport and an explicit `OnShutdown` that declares the shutdown sequence.
|
||||
|
||||
### Why explicit shutdown ordering matters
|
||||
|
||||
Different services have different dependency graphs between transports:
|
||||
- A consumer that calls gRPC must stop consuming *before* gRPC clients close
|
||||
- An HTTP API that publishes events must drain HTTP *before* the publisher closes
|
||||
- Two independent ingress points (HTTP + gRPC) can shut down in parallel
|
||||
|
||||
Implicit ordering (LIFO based on registration) is fragile — reordering lines silently changes shutdown behavior. `OnShutdown` makes the sequence a readable, reviewable declaration.
|
||||
|
||||
### Core types
|
||||
|
||||
```go
|
||||
// server/server.go
|
||||
type Server struct {
|
||||
components map[string]component
|
||||
startOrder []string
|
||||
shutdownSteps []ShutdownStep
|
||||
}
|
||||
|
||||
type component struct {
|
||||
name string
|
||||
start func(ctx context.Context) error
|
||||
stop func(ctx context.Context) error
|
||||
}
|
||||
|
||||
type Option func(*Server)
|
||||
|
||||
type ShutdownStep struct {
|
||||
componentNames []string
|
||||
fn func(ctx context.Context) error
|
||||
}
|
||||
```
|
||||
|
||||
### API
|
||||
|
||||
```go
|
||||
// Stop creates a step that stops named components.
|
||||
// Multiple names = parallel shutdown within the step.
|
||||
func Stop(names ...string) ShutdownStep
|
||||
|
||||
// StopFunc creates a step that runs an arbitrary cleanup function.
|
||||
func StopFunc(fn func()) ShutdownStep
|
||||
|
||||
// StopFuncWithErr creates a step with error return.
|
||||
func StopFuncWithErr(fn func(ctx context.Context) error) ShutdownStep
|
||||
|
||||
// OnShutdown declares the shutdown sequence.
|
||||
// Steps execute top-to-bottom. Each step completes before the next starts.
|
||||
// Components not mentioned stop last (with a warning log).
|
||||
func OnShutdown(steps ...ShutdownStep) Option
|
||||
```
|
||||
|
||||
### Shutdown execution
|
||||
|
||||
1. Steps execute sequentially in declaration order
|
||||
2. Within a `Stop("a", "b")` call, components stop in parallel
|
||||
3. Each step's `wg.Wait()` completes before the next step begins
|
||||
4. Components not mentioned in any `Stop()` get a catch-all parallel stop after all explicit steps (with a warning log — every component should be in OnShutdown)
|
||||
5. A global timeout (default 30s) bounds the entire sequence
|
||||
|
||||
### Key design principles
|
||||
|
||||
- Each `With*` option takes a `name string` as first argument — used in `Stop(name)` to reference it
|
||||
- `OnShutdown` reads top-to-bottom as a shutdown script
|
||||
- The factory owns `signal.NotifyContext` — callers never handle signals
|
||||
- `defer cleanup()` from `NewApplication` naturally runs after `Run()` returns — it is the implicit last phase
|
||||
- Duplicate component names panic at startup — caught immediately
|
||||
|
||||
**Check procedure:**
|
||||
1. If a project uses 2+ transports, verify `server.New()` is used (not multiple `Run*Server` calls)
|
||||
2. Verify `OnShutdown` is present and lists all components
|
||||
3. Verify shutdown order makes sense: consumers before servers, servers before clients
|
||||
4. No `signal.NotifyContext`, `net.Listen`, or `GracefulStop` calls outside `common/server/`
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// Trainer: HTTP + gRPC + Watermill consumer
|
||||
func main() {
|
||||
logs.Init()
|
||||
ctx := context.Background()
|
||||
|
||||
app, cleanup := service.NewApplication(ctx)
|
||||
defer cleanup()
|
||||
|
||||
server.New(
|
||||
server.WithWatermillRouter("events", func(r *message.Router, sub message.Subscriber) {
|
||||
ports.RegisterEventHandlers(r, sub, app)
|
||||
}),
|
||||
server.WithHTTPHandler("api", func(router chi.Router) http.Handler {
|
||||
return ports.HandlerFromMux(ports.NewHttpServer(app), router)
|
||||
}),
|
||||
server.WithGRPCServer("grpc", func(s *grpc.Server) {
|
||||
trainer.RegisterTrainerServiceServer(s, ports.NewGrpcServer(app))
|
||||
}),
|
||||
server.OnShutdown(
|
||||
server.Stop("events"), // 1. stop consuming
|
||||
server.Stop("api", "grpc"), // 2. drain both servers in parallel
|
||||
server.StopFunc(cleanup), // 3. close clients & publisher
|
||||
),
|
||||
).Run(ctx)
|
||||
}
|
||||
|
||||
// Trainings: HTTP-only, publishes events (publisher in cleanup)
|
||||
func main() {
|
||||
logs.Init()
|
||||
ctx := context.Background()
|
||||
|
||||
app, cleanup := service.NewApplication(ctx)
|
||||
defer cleanup()
|
||||
|
||||
server.New(
|
||||
server.WithHTTPHandler("api", func(router chi.Router) http.Handler {
|
||||
return ports.HandlerFromMux(ports.NewHttpServer(app), router)
|
||||
}),
|
||||
server.OnShutdown(
|
||||
server.Stop("api"), // 1. drain HTTP (in-flight may publish events)
|
||||
server.StopFunc(cleanup), // 2. close publisher + gRPC clients
|
||||
),
|
||||
).Run(ctx)
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// VIOLATION: implicit LIFO ordering — fragile
|
||||
server.New(
|
||||
server.WithHTTPHandler("api", createHandler),
|
||||
server.WithWatermillRouter("events", configureRouter),
|
||||
// no OnShutdown — relies on registration order
|
||||
).Run(ctx)
|
||||
|
||||
// VIOLATION: manual lifecycle per transport
|
||||
func main() {
|
||||
ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
|
||||
|
||||
grpcServer := grpc.NewServer()
|
||||
go grpcServer.Serve(lis)
|
||||
|
||||
router, _ := message.NewRouter(...)
|
||||
go router.Run(ctx)
|
||||
|
||||
<-ctx.Done()
|
||||
grpcServer.GracefulStop()
|
||||
router.Close()
|
||||
}
|
||||
```
|
||||
@@ -0,0 +1,220 @@
|
||||
# Code Style Rules (STYLE-01..08)
|
||||
|
||||
## STYLE-01: Import Grouping (INFO)
|
||||
|
||||
Imports MUST be organized in groups separated by blank lines:
|
||||
1. Standard library
|
||||
2. External packages (third-party + internal modules)
|
||||
|
||||
```go
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"time"
|
||||
|
||||
"github.com/sirupsen/logrus"
|
||||
"github.com/example/myproject/internal/trainer/domain/hour"
|
||||
)
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
import (
|
||||
"context"
|
||||
"github.com/sirupsen/logrus" // VIOLATION: mixed with stdlib
|
||||
"fmt"
|
||||
"time"
|
||||
)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## STYLE-02: Receiver Conventions (INFO)
|
||||
|
||||
- **Pointer receivers** (`*Type`) for methods that mutate state
|
||||
- **Value receivers** (`Type`) for methods that only read state
|
||||
|
||||
```go
|
||||
// Mutates — pointer receiver
|
||||
func (h *Hour) ScheduleTraining() error {
|
||||
h.availability = TrainingScheduled
|
||||
return nil
|
||||
}
|
||||
|
||||
// Read-only — value receiver
|
||||
func (h Hour) IsAvailable() bool {
|
||||
return h.availability == Available
|
||||
}
|
||||
|
||||
func (a Availability) IsZero() bool {
|
||||
return a == Availability{}
|
||||
}
|
||||
```
|
||||
|
||||
Receiver names should be short (1-2 chars), typically the first letter of the type.
|
||||
|
||||
---
|
||||
|
||||
## STYLE-03: t.Parallel() in Tests (WARNING)
|
||||
|
||||
Every test function and subtest SHOULD call `t.Parallel()` as its first statement.
|
||||
|
||||
```go
|
||||
func TestScheduleTraining(t *testing.T) {
|
||||
t.Parallel()
|
||||
// ... test code
|
||||
}
|
||||
|
||||
func TestRepository(t *testing.T) {
|
||||
t.Parallel()
|
||||
for i := range testCases {
|
||||
tc := testCases[i]
|
||||
t.Run(tc.Name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
// ... test code
|
||||
})
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## STYLE-04: require vs assert (INFO)
|
||||
|
||||
Use the testify library with:
|
||||
- **`require`** for setup/preconditions that must succeed (fatal on failure)
|
||||
- **`assert`** for actual test assertions (non-fatal, continues test)
|
||||
|
||||
```go
|
||||
func TestSomething(t *testing.T) {
|
||||
// Setup — use require (fatal if fails)
|
||||
hour, err := hour.NewAvailableHour(testTime)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Act
|
||||
err = hour.ScheduleTraining()
|
||||
|
||||
// Assert — use assert (non-fatal)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, hour.TrainingScheduled, hour.Availability())
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## STYLE-05: Loop Variable Capture (WARNING)
|
||||
|
||||
When using loop variables in goroutines or subtests, ALWAYS capture them first.
|
||||
|
||||
```go
|
||||
for i := range repositories {
|
||||
r := repositories[i] // capture before subtest
|
||||
t.Run(r.Name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
testUpdateHour(t, r.Repository)
|
||||
})
|
||||
}
|
||||
```
|
||||
|
||||
**Note:** Go 1.22+ fixes loop variable capture for `range` loops, but the explicit capture pattern is still preferred for clarity and backward compatibility.
|
||||
|
||||
---
|
||||
|
||||
## STYLE-06: Table-Driven Tests (INFO)
|
||||
|
||||
Tests with multiple cases SHOULD use table-driven pattern with named test cases.
|
||||
|
||||
```go
|
||||
func TestValidateTime(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
testCases := []struct {
|
||||
Name string
|
||||
Hour time.Time
|
||||
ExpectedErr error
|
||||
}{
|
||||
{
|
||||
Name: "valid_hour",
|
||||
Hour: time.Now().Truncate(time.Hour).Add(24 * time.Hour),
|
||||
ExpectedErr: nil,
|
||||
},
|
||||
{
|
||||
Name: "past_hour",
|
||||
Hour: time.Now().Add(-time.Hour),
|
||||
ExpectedErr: ErrPastHour,
|
||||
},
|
||||
{
|
||||
Name: "not_full_hour",
|
||||
Hour: time.Now().Add(30 * time.Minute),
|
||||
ExpectedErr: ErrNotFullHour,
|
||||
},
|
||||
}
|
||||
|
||||
for i := range testCases {
|
||||
tc := testCases[i]
|
||||
t.Run(tc.Name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
err := validateTime(tc.Hour)
|
||||
assert.ErrorIs(t, err, tc.ExpectedErr)
|
||||
})
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## STYLE-07: Interfaces Where Consumed (WARNING)
|
||||
|
||||
Interfaces MUST be defined in the package that **uses** them, not the package that implements them. This follows Go's implicit interface philosophy.
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// domain/hour/repository.go — consumer defines what it needs
|
||||
package hour
|
||||
|
||||
type Repository interface {
|
||||
GetHour(ctx context.Context, hourTime time.Time) (*Hour, error)
|
||||
UpdateHour(ctx context.Context, hourTime time.Time,
|
||||
updateFn func(h *Hour) (*Hour, error)) error
|
||||
}
|
||||
|
||||
// adapters/ — implicitly implements it
|
||||
package adapters
|
||||
|
||||
type FirestoreHourRepository struct { ... }
|
||||
func (r *FirestoreHourRepository) GetHour(...) (*hour.Hour, error) { ... }
|
||||
func (r *FirestoreHourRepository) UpdateHour(...) error { ... }
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// adapters/interfaces.go ← VIOLATION
|
||||
package adapters
|
||||
|
||||
type HourRepository interface { ... } // interface where implemented, not consumed
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## STYLE-08: Context as First Parameter (WARNING)
|
||||
|
||||
All methods that perform I/O (database, HTTP, gRPC, file) MUST accept `context.Context` as their first parameter.
|
||||
|
||||
```go
|
||||
// Repository methods
|
||||
GetHour(ctx context.Context, hourTime time.Time) (*Hour, error)
|
||||
UpdateHour(ctx context.Context, hourTime time.Time, updateFn func(h *Hour) (*Hour, error)) error
|
||||
|
||||
// Handler methods
|
||||
Handle(ctx context.Context, cmd CancelTraining) error
|
||||
Handle(ctx context.Context, q AvailableHours) ([]Date, error)
|
||||
|
||||
// Adapter methods
|
||||
func (r *FirestoreHourRepository) GetHour(ctx context.Context, hourTime time.Time) (*hour.Hour, error)
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
func (r *Repo) GetHour(hourTime time.Time) (*Hour, error) // VIOLATION: no context
|
||||
func (r *Repo) GetHour(hourTime time.Time, ctx context.Context) (*Hour, error) // VIOLATION: ctx not first
|
||||
```
|
||||
@@ -0,0 +1,276 @@
|
||||
# CQRS Rules (CQRS-01..10)
|
||||
|
||||
## CQRS-01: Command Struct Pattern (CRITICAL)
|
||||
|
||||
Commands MUST be:
|
||||
- Named with imperative verb + noun (domain language, NOT CRUD)
|
||||
- Plain data structs (no methods, no interfaces)
|
||||
- Their handler returns `error` only — no data
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
type ScheduleTraining struct {
|
||||
Hour time.Time
|
||||
}
|
||||
|
||||
type CancelTraining struct {
|
||||
Hour time.Time
|
||||
}
|
||||
|
||||
type MakeHoursAvailable struct {
|
||||
Hours []time.Time
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
type CreateTraining struct { ... } // VIOLATION: CRUD naming
|
||||
type UpdateHour struct { ... } // VIOLATION: CRUD naming
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## CQRS-02: Query Struct Pattern (CRITICAL)
|
||||
|
||||
Queries MUST be:
|
||||
- Named with noun phrases (NOT "Get" + noun)
|
||||
- Plain data structs
|
||||
- Their handler returns `(ResultType, error)`
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
type AvailableHours struct {
|
||||
From time.Time
|
||||
To time.Time
|
||||
}
|
||||
|
||||
type HourAvailability struct {
|
||||
Hour time.Time
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
type GetAvailableHours struct { ... } // VIOLATION: "Get" prefix
|
||||
type FetchTrainings struct { ... } // VIOLATION: "Fetch" prefix
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## CQRS-03: Exported Handler Type Alias (WARNING)
|
||||
|
||||
Each handler file MUST define an exported type alias using the generic decorator interface.
|
||||
|
||||
```go
|
||||
// For commands:
|
||||
type CancelTrainingHandler decorator.CommandHandler[CancelTraining]
|
||||
|
||||
// For queries:
|
||||
type AvailableHoursHandler decorator.QueryHandler[AvailableHours, []Date]
|
||||
```
|
||||
|
||||
This allows callers to depend on the decorated interface, not the concrete struct.
|
||||
|
||||
---
|
||||
|
||||
## CQRS-04: Unexported Handler Struct (WARNING)
|
||||
|
||||
The concrete handler struct MUST be unexported (lowercase). It holds dependencies injected via constructor.
|
||||
|
||||
```go
|
||||
type cancelTrainingHandler struct {
|
||||
hourRepo hour.Repository
|
||||
}
|
||||
|
||||
type availableHoursHandler struct {
|
||||
readModel AvailableHoursReadModel
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## CQRS-05: Constructor Wraps with Decorators (WARNING)
|
||||
|
||||
Handler constructors MUST wrap the concrete handler with `ApplyCommandDecorators` or `ApplyQueryDecorators`.
|
||||
|
||||
```go
|
||||
func NewCancelTrainingHandler(
|
||||
hourRepo hour.Repository,
|
||||
logger *logrus.Entry,
|
||||
metricsClient decorator.MetricsClient,
|
||||
) CancelTrainingHandler {
|
||||
return decorator.ApplyCommandDecorators[CancelTraining](
|
||||
cancelTrainingHandler{hourRepo: hourRepo},
|
||||
logger,
|
||||
metricsClient,
|
||||
)
|
||||
}
|
||||
|
||||
func NewAvailableHoursHandler(
|
||||
readModel AvailableHoursReadModel,
|
||||
logger *logrus.Entry,
|
||||
metricsClient decorator.MetricsClient,
|
||||
) AvailableHoursHandler {
|
||||
return decorator.ApplyQueryDecorators[AvailableHours, []Date](
|
||||
availableHoursHandler{readModel: readModel},
|
||||
logger,
|
||||
metricsClient,
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## CQRS-06: Constructor Nil-Checks with Panic (WARNING)
|
||||
|
||||
Handler constructors SHOULD nil-check all injected dependencies and panic if any are nil. This is a fail-fast pattern — misconfiguration is caught at startup, not at runtime.
|
||||
|
||||
```go
|
||||
func NewCancelTrainingHandler(
|
||||
hourRepo hour.Repository,
|
||||
logger *logrus.Entry,
|
||||
metricsClient decorator.MetricsClient,
|
||||
) CancelTrainingHandler {
|
||||
if hourRepo == nil {
|
||||
panic("nil hourRepo")
|
||||
}
|
||||
if logger == nil {
|
||||
panic("nil logger")
|
||||
}
|
||||
if metricsClient == nil {
|
||||
panic("nil metricsClient")
|
||||
}
|
||||
return decorator.ApplyCommandDecorators[CancelTraining](
|
||||
cancelTrainingHandler{hourRepo: hourRepo},
|
||||
logger,
|
||||
metricsClient,
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## CQRS-07: Application Struct (CRITICAL)
|
||||
|
||||
The `app/app.go` file MUST define an `Application` struct that bundles `Commands` and `Queries` sub-structs.
|
||||
|
||||
```go
|
||||
type Application struct {
|
||||
Commands Commands
|
||||
Queries Queries
|
||||
}
|
||||
|
||||
type Commands struct {
|
||||
CancelTraining command.CancelTrainingHandler
|
||||
ScheduleTraining command.ScheduleTrainingHandler
|
||||
MakeHoursAvailable command.MakeHoursAvailableHandler
|
||||
MakeHoursUnavailable command.MakeHoursUnavailableHandler
|
||||
}
|
||||
|
||||
type Queries struct {
|
||||
HourAvailability query.HourAvailabilityHandler
|
||||
TrainerAvailableHours query.AvailableHoursHandler
|
||||
}
|
||||
```
|
||||
|
||||
**Check:** Look for `app.go` in the `app/` package. Verify it has `Application`, `Commands`, and `Queries` types.
|
||||
|
||||
---
|
||||
|
||||
## CQRS-08: Read Model Interface for Queries (WARNING)
|
||||
|
||||
Query handlers SHOULD depend on a dedicated read model interface, not the write repository.
|
||||
|
||||
```go
|
||||
// In app/query/ — defines what it needs
|
||||
type AvailableHoursReadModel interface {
|
||||
AvailableHours(ctx context.Context, from, to time.Time) ([]Date, error)
|
||||
}
|
||||
```
|
||||
|
||||
This keeps reads and writes separate. The same adapter may implement both the write `Repository` and a read model interface, but the query handler only knows about the read model.
|
||||
|
||||
---
|
||||
|
||||
## CQRS-09: Command/Query Separation (CRITICAL)
|
||||
|
||||
- **Commands** MUST modify state and return only `error`
|
||||
- **Queries** MUST read state and return `(ResultType, error)` — they MUST NOT modify state
|
||||
|
||||
A handler that both reads and writes violates CQRS.
|
||||
|
||||
**Check:** Command handlers returning anything besides `error` is a violation. Query handlers calling mutation methods on repositories is a violation.
|
||||
|
||||
---
|
||||
|
||||
## CQRS-10: No Business Logic in Handlers (WARNING)
|
||||
|
||||
Handlers are orchestrators. Business rules live in domain entities.
|
||||
|
||||
**Correct** — handler delegates to domain:
|
||||
```go
|
||||
func (h cancelTrainingHandler) Handle(ctx context.Context, cmd CancelTraining) error {
|
||||
return h.hourRepo.UpdateHour(ctx, cmd.Hour, func(h *hour.Hour) (*hour.Hour, error) {
|
||||
if err := h.CancelTraining(); err != nil { // domain method
|
||||
return nil, err
|
||||
}
|
||||
return h, nil
|
||||
})
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong** — business logic in handler:
|
||||
```go
|
||||
func (h cancelTrainingHandler) Handle(ctx context.Context, cmd CancelTraining) error {
|
||||
hour, _ := h.hourRepo.GetHour(ctx, cmd.Hour)
|
||||
if hour.Availability != "training_scheduled" { // VIOLATION: logic belongs in domain
|
||||
return errors.New("no training to cancel")
|
||||
}
|
||||
hour.Availability = "available" // VIOLATION: direct field mutation
|
||||
return h.hourRepo.Save(ctx, hour)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Complete Handler File Template
|
||||
|
||||
Every command/query handler file follows this 4-component pattern:
|
||||
|
||||
```go
|
||||
package command
|
||||
|
||||
// 1. Command struct
|
||||
type CancelTraining struct {
|
||||
Hour time.Time
|
||||
}
|
||||
|
||||
// 2. Exported handler type (alias to decorator interface)
|
||||
type CancelTrainingHandler decorator.CommandHandler[CancelTraining]
|
||||
|
||||
// 3. Unexported concrete handler
|
||||
type cancelTrainingHandler struct {
|
||||
hourRepo hour.Repository
|
||||
}
|
||||
|
||||
// 4. Constructor with nil-checks + decorator wrapping
|
||||
func NewCancelTrainingHandler(
|
||||
hourRepo hour.Repository,
|
||||
logger *logrus.Entry,
|
||||
metricsClient decorator.MetricsClient,
|
||||
) CancelTrainingHandler {
|
||||
if hourRepo == nil {
|
||||
panic("nil hourRepo")
|
||||
}
|
||||
return decorator.ApplyCommandDecorators[CancelTraining](
|
||||
cancelTrainingHandler{hourRepo: hourRepo},
|
||||
logger,
|
||||
metricsClient,
|
||||
)
|
||||
}
|
||||
|
||||
// Handle method on unexported struct
|
||||
func (h cancelTrainingHandler) Handle(ctx context.Context, cmd CancelTraining) error {
|
||||
// orchestration only — delegate to domain
|
||||
}
|
||||
```
|
||||
@@ -0,0 +1,265 @@
|
||||
# Domain Rules (DOM-01..09)
|
||||
|
||||
## DOM-01: Private Entity Fields (CRITICAL)
|
||||
|
||||
ALL entity struct fields MUST be unexported (lowercase). Entities are "types with behavior," not data bags.
|
||||
|
||||
**Check:** Scan all structs in `domain/` for exported fields. Any uppercase field name is a violation.
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
type Hour struct {
|
||||
hour time.Time
|
||||
availability Availability
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
type Hour struct {
|
||||
Hour time.Time // VIOLATION: exported field
|
||||
Availability Availability // VIOLATION: exported field
|
||||
}
|
||||
```
|
||||
|
||||
**Exception:** DB model structs in `adapters/` MAY have exported fields for serialization tags.
|
||||
|
||||
---
|
||||
|
||||
## DOM-02: Factory Constructors (WARNING)
|
||||
|
||||
Entities MUST be created through factory constructors, never by direct struct literal.
|
||||
|
||||
Pattern: `func New{Type}(args...) (*Type, error)`
|
||||
|
||||
The constructor:
|
||||
- Validates all invariants
|
||||
- Returns an error if validation fails
|
||||
- Returns a pointer to the new entity
|
||||
|
||||
**Reference:**
|
||||
```go
|
||||
func NewAvailableHour(hour time.Time) (*Hour, error) {
|
||||
if err := validateTime(hour); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &Hour{hour: hour, availability: Available}, nil
|
||||
}
|
||||
|
||||
func NewTraining(uuid, userUUID, userName string, trainingTime time.Time) (*Training, error) {
|
||||
if uuid == "" {
|
||||
return nil, errors.New("empty training uuid")
|
||||
}
|
||||
if userUUID == "" {
|
||||
return nil, errors.New("empty training user uuid")
|
||||
}
|
||||
// ... validate all fields
|
||||
return &Training{uuid: uuid, userUUID: userUUID, userName: userName, time: trainingTime}, nil
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## DOM-03: MustNew Panic Constructors (INFO)
|
||||
|
||||
For use in tests and initialization code, provide `MustNew{Type}` that panics on error.
|
||||
|
||||
```go
|
||||
func MustNewFactory(fc FactoryConfig) Factory {
|
||||
f, err := NewFactory(fc)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
return f
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## DOM-04: UnmarshalFromDatabase (WARNING)
|
||||
|
||||
Entities MUST provide an `Unmarshal{Type}FromDatabase` function for reconstruction from persistence. This function:
|
||||
- Bypasses normal validation (data was already valid when stored)
|
||||
- Accepts all fields needed to reconstruct full state
|
||||
- Is used ONLY by repository adapters
|
||||
|
||||
**Reference:**
|
||||
```go
|
||||
func UnmarshalHourFromDatabase(hour time.Time, availability Availability) *Hour {
|
||||
return &Hour{hour: hour, availability: availability}
|
||||
}
|
||||
|
||||
func UnmarshalTrainingFromDatabase(
|
||||
uuid, userUUID, userName string,
|
||||
trainingTime time.Time,
|
||||
notes string,
|
||||
canceled bool,
|
||||
proposedNewTime time.Time,
|
||||
moveProposedBy UserType,
|
||||
) (*Training, error) {
|
||||
return &Training{
|
||||
uuid: uuid, userUUID: userUUID, userName: userName,
|
||||
time: trainingTime, notes: notes, canceled: canceled,
|
||||
proposedNewTime: proposedNewTime, moveProposedBy: moveProposedBy,
|
||||
}, nil
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## DOM-05: Value Objects as Structs (CRITICAL)
|
||||
|
||||
Value objects MUST be structs wrapping a private field, NOT raw strings, ints, or type aliases.
|
||||
|
||||
This ensures they cannot be constructed with arbitrary values — only through validated constructors or predefined constants.
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
type Availability struct {
|
||||
a string // private — cannot be set directly
|
||||
}
|
||||
|
||||
var (
|
||||
Available = Availability{"available"}
|
||||
NotAvailable = Availability{"not_available"}
|
||||
TrainingScheduled = Availability{"training_scheduled"}
|
||||
)
|
||||
|
||||
type UserType struct {
|
||||
s string
|
||||
}
|
||||
|
||||
var (
|
||||
Trainer = UserType{"trainer"}
|
||||
Attendee = UserType{"attendee"}
|
||||
)
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
type Availability string // VIOLATION: can be set to any string
|
||||
|
||||
const (
|
||||
Available Availability = "available"
|
||||
NotAvailable Availability = "not_available"
|
||||
)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## DOM-06: IsZero Method (WARNING)
|
||||
|
||||
Value objects and factory structs SHOULD implement `IsZero() bool` to check for zero-value state.
|
||||
|
||||
```go
|
||||
func (a Availability) IsZero() bool {
|
||||
return a == Availability{}
|
||||
}
|
||||
|
||||
func (f Factory) IsZero() bool {
|
||||
return f == Factory{}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## DOM-07: Behavior Methods Use Domain Language (CRITICAL)
|
||||
|
||||
Entity methods MUST use domain-specific language, NOT generic CRUD terms.
|
||||
|
||||
| Forbidden | Use Instead |
|
||||
|-----------|------------|
|
||||
| `SetStatus`, `Update` | `ScheduleTraining`, `CancelTraining`, `MakeAvailable` |
|
||||
| `Create` | `Schedule`, `Register`, `Place`, `Submit` |
|
||||
| `Delete` | `Cancel`, `Archive`, `Revoke` |
|
||||
| `Get` | Use query noun phrases |
|
||||
|
||||
**Reference:**
|
||||
```go
|
||||
func (h *Hour) ScheduleTraining() error {
|
||||
if !h.IsAvailable() {
|
||||
return ErrHourNotAvailable
|
||||
}
|
||||
h.availability = TrainingScheduled
|
||||
return nil
|
||||
}
|
||||
|
||||
func (h *Hour) CancelTraining() error { ... }
|
||||
func (h *Hour) MakeAvailable() error { ... }
|
||||
func (h *Hour) MakeNotAvailable() error { ... }
|
||||
|
||||
func (t *Training) ProposeReschedule(newTime time.Time, proposedBy UserType) error { ... }
|
||||
func (t *Training) ApproveReschedule(approvedBy UserType) error { ... }
|
||||
func (t *Training) RejectReschedule() error { ... }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## DOM-08: String Constructors Validate Input (WARNING)
|
||||
|
||||
When a value object can be constructed from a string, use `New{Type}FromString` with validation.
|
||||
|
||||
```go
|
||||
func NewAvailabilityFromString(availabilityStr string) (Availability, error) {
|
||||
switch availabilityStr {
|
||||
case "available":
|
||||
return Available, nil
|
||||
case "not_available":
|
||||
return NotAvailable, nil
|
||||
case "training_scheduled":
|
||||
return TrainingScheduled, nil
|
||||
default:
|
||||
return Availability{}, fmt.Errorf("unknown availability: %s", availabilityStr)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## DOM-09: Factory Struct for Complex Creation (INFO)
|
||||
|
||||
When entity creation requires configuration or external dependencies, use a Factory struct pattern.
|
||||
|
||||
```go
|
||||
type FactoryConfig struct {
|
||||
MaxWeeksInTheFutureToSet int
|
||||
MinUtcHour int
|
||||
MaxUtcHour int
|
||||
}
|
||||
|
||||
func (c FactoryConfig) Validate() error {
|
||||
var errs []error
|
||||
if c.MaxWeeksInTheFutureToSet <= 0 {
|
||||
errs = append(errs, errors.New("MaxWeeksInTheFutureToSet must be > 0"))
|
||||
}
|
||||
// ... more validations
|
||||
return multierr.Combine(errs...)
|
||||
}
|
||||
|
||||
type Factory struct {
|
||||
fc FactoryConfig
|
||||
}
|
||||
|
||||
func NewFactory(fc FactoryConfig) (Factory, error) {
|
||||
if err := fc.Validate(); err != nil {
|
||||
return Factory{}, err
|
||||
}
|
||||
return Factory{fc: fc}, nil
|
||||
}
|
||||
|
||||
func MustNewFactory(fc FactoryConfig) Factory {
|
||||
f, err := NewFactory(fc)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
return f
|
||||
}
|
||||
|
||||
func (f Factory) IsZero() bool {
|
||||
return f == Factory{}
|
||||
}
|
||||
|
||||
func (f Factory) NewAvailableHour(hour time.Time) (*Hour, error) {
|
||||
// uses f.fc for validation bounds
|
||||
}
|
||||
```
|
||||
@@ -0,0 +1,150 @@
|
||||
# Error Rules (ERR-01..05)
|
||||
|
||||
## Three-Tier Error Architecture
|
||||
|
||||
The error system has three tiers:
|
||||
|
||||
1. **Domain errors** — sentinel variables and typed structs in `domain/`
|
||||
2. **Application errors** — `SlugError` with machine-readable slugs in `app/`
|
||||
3. **Port errors** — protocol-specific error mapping in `ports/`
|
||||
|
||||
---
|
||||
|
||||
## ERR-01: Sentinel Error Variables (WARNING)
|
||||
|
||||
Simple domain errors without context SHOULD use sentinel `var` declarations.
|
||||
|
||||
```go
|
||||
// domain/hour/errors.go
|
||||
var (
|
||||
ErrNotFullHour = errors.New("hour should be a full hour")
|
||||
ErrPastHour = errors.New("cannot create hour in the past")
|
||||
ErrTrainingScheduled = errors.New("unable to modify hour, because scheduled training")
|
||||
ErrHourNotAvailable = errors.New("hour is not available")
|
||||
ErrNoTrainingScheduled = errors.New("no training scheduled")
|
||||
)
|
||||
```
|
||||
|
||||
**Naming:** `Err{DescriptiveName}` — always starts with `Err`.
|
||||
|
||||
**Usage in domain methods:**
|
||||
```go
|
||||
func (h *Hour) ScheduleTraining() error {
|
||||
if !h.IsAvailable() {
|
||||
return ErrHourNotAvailable
|
||||
}
|
||||
h.availability = TrainingScheduled
|
||||
return nil
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ERR-02: Typed Error Structs (WARNING)
|
||||
|
||||
Errors that carry context (values for logging/display) SHOULD be typed structs implementing the `error` interface.
|
||||
|
||||
```go
|
||||
type TooDistantDateError struct {
|
||||
MaxWeeksInTheFutureToSet int
|
||||
ProvidedDate time.Time
|
||||
}
|
||||
|
||||
func (e TooDistantDateError) Error() string {
|
||||
return fmt.Sprintf(
|
||||
"schedule can be only set for next %d weeks, provided date: %s",
|
||||
e.MaxWeeksInTheFutureToSet, e.ProvidedDate,
|
||||
)
|
||||
}
|
||||
|
||||
type TooEarlyHourError struct {
|
||||
MinUtcHour int
|
||||
ProvidedTime time.Time
|
||||
}
|
||||
|
||||
type ForbiddenToSeeTrainingError struct {
|
||||
RequestingUserUUID string
|
||||
TrainingOwnerUUID string
|
||||
}
|
||||
|
||||
type NotFoundError struct {
|
||||
TrainingUUID string
|
||||
}
|
||||
```
|
||||
|
||||
**Naming:** `{Condition}Error` — describes the error condition.
|
||||
|
||||
---
|
||||
|
||||
## ERR-03: SlugError for Application Layer (WARNING)
|
||||
|
||||
Application-layer errors (command/query handlers) SHOULD use `SlugError` from the common errors package. SlugErrors carry:
|
||||
- Human-readable error message
|
||||
- Machine-readable slug (used by API clients)
|
||||
- Error type (authorization, incorrect-input, unknown)
|
||||
|
||||
```go
|
||||
// common/errors/errors.go
|
||||
type ErrorType struct {
|
||||
t string
|
||||
}
|
||||
|
||||
var (
|
||||
ErrorTypeUnknown = ErrorType{"unknown"}
|
||||
ErrorTypeAuthorization = ErrorType{"authorization"}
|
||||
ErrorTypeIncorrectInput = ErrorType{"incorrect-input"}
|
||||
)
|
||||
|
||||
type SlugError struct {
|
||||
error string
|
||||
slug string
|
||||
errorType ErrorType
|
||||
}
|
||||
|
||||
func NewSlugError(error string, slug string) SlugError
|
||||
func NewAuthorizationError(error string, slug string) SlugError
|
||||
func NewIncorrectInputError(error string, slug string) SlugError
|
||||
```
|
||||
|
||||
**Usage in handlers:**
|
||||
```go
|
||||
func (h cancelTrainingHandler) Handle(ctx context.Context, cmd CancelTraining) error {
|
||||
if err := h.hourRepo.UpdateHour(ctx, cmd.Hour, func(h *hour.Hour) (*hour.Hour, error) {
|
||||
if err := h.CancelTraining(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return h, nil
|
||||
}); err != nil {
|
||||
return errors.NewSlugError(err.Error(), "unable-to-update-availability")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ERR-04: Error Wrapping with Context (INFO)
|
||||
|
||||
When re-raising errors, wrap them with context using `fmt.Errorf("context: %w", err)` or a wrapping library.
|
||||
|
||||
```go
|
||||
// In adapters
|
||||
if err := doc.DataTo(&model); err != nil {
|
||||
return nil, fmt.Errorf("unmarshaling hour from firestore: %w", err)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## ERR-05: No Bare fmt.Errorf in Domain (CRITICAL)
|
||||
|
||||
The domain package MUST NOT use `fmt.Errorf` for error creation. Domain errors must be either:
|
||||
- Sentinel variables (`var ErrX = errors.New(...)`)
|
||||
- Typed error structs
|
||||
- Standard `errors.New(...)` for simple cases
|
||||
|
||||
**Check:** Grep `domain/` for `fmt.Errorf`. Any match in non-test files is a violation.
|
||||
|
||||
**Rationale:** `fmt.Errorf` creates untyped errors that cannot be checked with `errors.Is` or `errors.As`. Domain errors should be programmatically handleable.
|
||||
|
||||
**Exception:** `fmt.Errorf` with `%w` for wrapping IS acceptable in domain validation helpers that combine multiple checks, but prefer typed errors or sentinel variables.
|
||||
@@ -0,0 +1,52 @@
|
||||
# Naming Rules
|
||||
|
||||
## Strict Naming Convention Table
|
||||
|
||||
| Pattern | Convention | Example |
|
||||
|---------|-----------|---------|
|
||||
| Entity constructor | `New{Type}(args...) (*Type, error)` | `NewTraining(...)`, `NewAvailableHour(...)` |
|
||||
| Panic constructor | `MustNew{Type}(args...) Type` | `MustNewFactory(...)`, `MustNewUser(...)` |
|
||||
| DB reconstruction | `Unmarshal{Type}FromDatabase(...)` | `UnmarshalHourFromDatabase(...)` |
|
||||
| Value from string | `New{Type}FromString(s string) (Type, error)` | `NewAvailabilityFromString(...)` |
|
||||
| Command struct | Imperative verb + noun (PascalCase) | `ScheduleTraining`, `CancelTraining`, `MakeHoursAvailable` |
|
||||
| Query struct | Noun phrase (PascalCase) | `AvailableHours`, `HourAvailability`, `AllTrainings` |
|
||||
| Handler type (exported) | `{ActionName}Handler` | `ScheduleTrainingHandler`, `CancelTrainingHandler` |
|
||||
| Handler struct (unexported) | `{actionName}Handler` | `scheduleTrainingHandler`, `cancelTrainingHandler` |
|
||||
| Handler constructor | `New{ActionName}Handler(...)` | `NewScheduleTrainingHandler(...)` |
|
||||
| Adapter type | Technology suffix | `FirestoreHourRepository`, `MySQLHourRepository`, `MemoryHourRepository` |
|
||||
| Adapter constructor | `New{Tech}{Entity}Repository(...)` | `NewFirestoreHourRepository(...)` |
|
||||
| DB model (SQL) | Tech prefix, unexported | `mysqlHour`, `postgresTraining` |
|
||||
| DB model (NoSQL) | `{Entity}Model` (exported for tags) | `TrainingModel`, `DateModel` |
|
||||
| Sentinel errors | `Err{Name}` | `ErrNotFullHour`, `ErrHourNotAvailable` |
|
||||
| Typed errors | `{Condition}Error` | `TooDistantDateError`, `NotFoundError` |
|
||||
| Zero check | `IsZero() bool` | `Availability.IsZero()`, `Factory.IsZero()` |
|
||||
| Application struct | `Application` in `app/` package | `app.Application` |
|
||||
| App sub-structs | `Commands`, `Queries` | `app.Commands`, `app.Queries` |
|
||||
| Composition root | `NewApplication(...)` in `service/` | `service.NewApplication(ctx)` |
|
||||
| gRPC client adapter | `{Service}Grpc` | `TrainerGrpc`, `UsersGrpc` |
|
||||
| Read model interface | `{Query}ReadModel` | `AvailableHoursReadModel` |
|
||||
|
||||
## CRUD-to-Domain-Language Mapping
|
||||
|
||||
CRUD terms are **forbidden** in domain code, commands, queries, and API endpoints. Use domain-specific language instead.
|
||||
|
||||
| CRUD Term | Replacement Options | Example |
|
||||
|-----------|-------------------|---------|
|
||||
| Create | Schedule, Register, Place, Submit, Open, Enroll | `ScheduleTraining`, not `CreateTraining` |
|
||||
| Read | *(use noun phrase queries)* | `AvailableHours`, not `GetHours` |
|
||||
| Update | Approve, Reject, Reschedule, Move, Modify, Assign | `ApproveReschedule`, not `UpdateTraining` |
|
||||
| Delete | Cancel, Archive, Revoke, Close, Withdraw | `CancelTraining`, not `DeleteTraining` |
|
||||
| Get | *(avoid as prefix)* | `HourAvailability`, not `GetHourAvailability` |
|
||||
| Set | *(use specific verb)* | `MakeAvailable`, not `SetAvailability` |
|
||||
| List | *(use noun phrase)* | `AllTrainings`, not `ListTrainings` |
|
||||
| Fetch | *(avoid entirely)* | Use noun phrase queries |
|
||||
|
||||
## Check Procedure
|
||||
|
||||
1. Scan all type declarations and function names
|
||||
2. Flag any use of Create/Read/Update/Delete/Get/Set/List/Fetch in:
|
||||
- Command struct names
|
||||
- Query struct names
|
||||
- Domain entity method names
|
||||
- Handler type names
|
||||
3. Severity: CRITICAL for command/query names, WARNING for methods
|
||||
@@ -0,0 +1,179 @@
|
||||
# Port Rules (PORT-01..06)
|
||||
|
||||
## PORT-01: Handler Struct Holds Application (WARNING)
|
||||
|
||||
HTTP and gRPC handler structs MUST hold `app.Application` and delegate to it. They are thin wrappers.
|
||||
|
||||
```go
|
||||
// ports/http.go
|
||||
type HttpServer struct {
|
||||
app app.Application
|
||||
}
|
||||
|
||||
// ports/grpc.go
|
||||
type GrpcServer struct {
|
||||
app app.Application
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## PORT-02: Error Mapping (WARNING)
|
||||
|
||||
Ports MUST map application errors to protocol-specific responses. They must NOT leak internal error details.
|
||||
|
||||
**HTTP — using httperr helper:**
|
||||
```go
|
||||
func (h HttpServer) MakeHourAvailable(w http.ResponseWriter, r *http.Request) {
|
||||
err = h.app.Commands.MakeHoursAvailable.Handle(r.Context(), command.MakeHoursAvailable{...})
|
||||
if err != nil {
|
||||
httperr.RespondWithSlugError(err, w, r)
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
```
|
||||
|
||||
**The httperr mapper:**
|
||||
```go
|
||||
func RespondWithSlugError(err error, w http.ResponseWriter, r *http.Request) {
|
||||
slugError, ok := err.(errors.SlugError)
|
||||
if !ok {
|
||||
InternalError("internal-server-error", err, w, r)
|
||||
return
|
||||
}
|
||||
switch slugError.ErrorType() {
|
||||
case errors.ErrorTypeAuthorization:
|
||||
Unauthorised(slugError.Slug(), slugError, w, r) // 401
|
||||
case errors.ErrorTypeIncorrectInput:
|
||||
BadRequest(slugError.Slug(), slugError, w, r) // 400
|
||||
default:
|
||||
InternalError(slugError.Slug(), slugError, w, r) // 500
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**gRPC — using status codes:**
|
||||
```go
|
||||
func (g GrpcServer) ScheduleTraining(ctx context.Context, req *trainer.UpdateHourRequest) (*empty.Empty, error) {
|
||||
if err := g.app.Commands.ScheduleTraining.Handle(ctx, command.ScheduleTraining{...}); err != nil {
|
||||
return nil, status.Error(codes.Internal, err.Error())
|
||||
}
|
||||
return &empty.Empty{}, nil
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## PORT-03: Auth Extracted from Context (WARNING)
|
||||
|
||||
Authentication/authorization data MUST be extracted from the request context using a shared auth package, NOT parsed directly in the handler.
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
func (h HttpServer) MakeHourAvailable(w http.ResponseWriter, r *http.Request) {
|
||||
user, err := auth.UserFromCtx(r.Context())
|
||||
if err != nil {
|
||||
httperr.RespondWithSlugError(err, w, r)
|
||||
return
|
||||
}
|
||||
if user.Role != "trainer" {
|
||||
httperr.Unauthorised("invalid-role", nil, w, r)
|
||||
return
|
||||
}
|
||||
// ... delegate to app
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
func (h HttpServer) MakeHourAvailable(w http.ResponseWriter, r *http.Request) {
|
||||
token := r.Header.Get("Authorization") // VIOLATION: parsing auth in handler
|
||||
claims, err := jwt.Parse(token, keyFunc) // VIOLATION: JWT logic in port
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## PORT-04: No Business Logic in Ports (CRITICAL)
|
||||
|
||||
Port handlers MUST only:
|
||||
1. Parse/decode the request
|
||||
2. Extract auth from context
|
||||
3. Construct command/query struct
|
||||
4. Call `app.Commands.X.Handle()` or `app.Queries.X.Handle()`
|
||||
5. Map the result/error to a response
|
||||
|
||||
They MUST NOT contain:
|
||||
- Domain validation logic
|
||||
- Business rule checks
|
||||
- Direct database calls
|
||||
- State manipulation
|
||||
|
||||
**Check:** Port files should only import `app/`, `app/command/`, `app/query/`, and infrastructure packages (HTTP, gRPC, auth). They should NOT import `domain/` directly (except for response mapping types).
|
||||
|
||||
---
|
||||
|
||||
## PORT-05: Response Model Mapping (INFO)
|
||||
|
||||
Response transformation SHOULD be in separate mapping functions, not inline in handlers.
|
||||
|
||||
```go
|
||||
// Mapping function
|
||||
func dateModelsToResponse(models []query.Date) []Date {
|
||||
var dates []Date
|
||||
for _, m := range models {
|
||||
dates = append(dates, Date{
|
||||
Date: m.Date,
|
||||
Hours: hourModelsToResponse(m.Hours),
|
||||
})
|
||||
}
|
||||
return dates
|
||||
}
|
||||
|
||||
// Handler uses it cleanly
|
||||
func (h HttpServer) GetTrainerAvailableHours(w http.ResponseWriter, r *http.Request, params GetTrainerAvailableHoursParams) {
|
||||
dateModels, err := h.app.Queries.TrainerAvailableHours.Handle(r.Context(), query.AvailableHours{
|
||||
From: params.DateFrom,
|
||||
To: params.DateTo,
|
||||
})
|
||||
if err != nil {
|
||||
httperr.RespondWithSlugError(err, w, r)
|
||||
return
|
||||
}
|
||||
dates := dateModelsToResponse(dateModels)
|
||||
render.Respond(w, r, dates)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## PORT-06: No Unimplemented Embedding in gRPC Servers (CRITICAL)
|
||||
|
||||
gRPC server structs MUST NOT embed `Unimplemented*Server` structs. Omitting the embed enforces **compile-time interface compliance** — if a new RPC is added to the proto definition, the code will fail to compile until the method is explicitly implemented.
|
||||
|
||||
Embedding `Unimplemented*Server` silently returns "unimplemented" at runtime for missing methods, hiding broken contracts until a request hits the missing endpoint in production.
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
type GrpcServer struct {
|
||||
app app.Application
|
||||
}
|
||||
// Compile error if any RPC method from TrainerServiceServer is missing.
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
type GrpcServer struct {
|
||||
trainer.UnimplementedTrainerServiceServer // VIOLATION: hides missing methods at compile time
|
||||
app app.Application
|
||||
}
|
||||
```
|
||||
|
||||
**Check:** Scan all structs in `ports/grpc.go` for embedded `Unimplemented*Server` fields. Any match is a CRITICAL violation.
|
||||
|
||||
**Proto generation:** When generating gRPC code, use `require_unimplemented_servers=false` to keep the interface strict:
|
||||
```
|
||||
protoc --go-grpc_out=require_unimplemented_servers=false:. *.proto
|
||||
```
|
||||
@@ -0,0 +1,181 @@
|
||||
# Repository Rules (REPO-01..07)
|
||||
|
||||
## REPO-01: Interface Defined in Domain (CRITICAL)
|
||||
|
||||
Repository interfaces MUST be defined in the domain package, next to the entity they persist. This follows the Dependency Inversion Principle — the domain defines what it needs, adapters implement it.
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// domain/hour/repository.go
|
||||
package hour
|
||||
|
||||
type Repository interface {
|
||||
GetHour(ctx context.Context, hourTime time.Time) (*Hour, error)
|
||||
UpdateHour(ctx context.Context, hourTime time.Time,
|
||||
updateFn func(h *Hour) (*Hour, error)) error
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// adapters/repository.go ← VIOLATION: interface in adapter layer
|
||||
package adapters
|
||||
|
||||
type HourRepository interface { ... }
|
||||
```
|
||||
|
||||
**Check:** Grep `domain/` for `type.*Repository interface`. Grep `adapters/` for the same — if found in adapters, it's a violation.
|
||||
|
||||
---
|
||||
|
||||
## REPO-02: Update Callback Pattern (WARNING)
|
||||
|
||||
Repository update methods SHOULD use a callback/closure pattern. The repository handles transaction lifecycle; the callback handles domain logic.
|
||||
|
||||
```go
|
||||
// Interface
|
||||
UpdateHour(ctx context.Context, hourTime time.Time,
|
||||
updateFn func(h *Hour) (*Hour, error)) error
|
||||
|
||||
// Usage in handler
|
||||
err := h.hourRepo.UpdateHour(ctx, cmd.Hour, func(h *hour.Hour) (*hour.Hour, error) {
|
||||
if err := h.CancelTraining(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return h, nil
|
||||
})
|
||||
```
|
||||
|
||||
Benefits:
|
||||
- Transaction scope is clear
|
||||
- Domain logic is isolated from persistence details
|
||||
- Enables optimistic locking, retries, etc. transparently
|
||||
|
||||
---
|
||||
|
||||
## REPO-03: Separate DB Model Structs (WARNING)
|
||||
|
||||
Adapter implementations MUST use separate structs for database representation. Domain entities should NOT have serialization tags.
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// adapters/ — DB model
|
||||
type mysqlHour struct {
|
||||
ID int `db:"id"`
|
||||
Hour time.Time `db:"hour"`
|
||||
Availability string `db:"availability"`
|
||||
}
|
||||
|
||||
// or for Firestore (needs exported fields for tags)
|
||||
type TrainingModel struct {
|
||||
UUID string `firestore:"Uuid"`
|
||||
UserUUID string `firestore:"UserUuid"`
|
||||
Time time.Time `firestore:"Time"`
|
||||
}
|
||||
|
||||
// Conversion in adapter
|
||||
func (r *MySQLHourRepository) toHour(m mysqlHour) (*hour.Hour, error) {
|
||||
availability, err := hour.NewAvailabilityFromString(m.Availability)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return hour.UnmarshalHourFromDatabase(m.Hour, availability), nil
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// domain/hour/hour.go
|
||||
type Hour struct {
|
||||
Hour time.Time `json:"hour" db:"hour"` // VIOLATION: DB tags on domain entity
|
||||
Availability string `json:"availability"` // VIOLATION: serialization concern in domain
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## REPO-04: Adapter Constructor Naming (INFO)
|
||||
|
||||
Repository adapter constructors follow: `New{Technology}{Entity}Repository`
|
||||
|
||||
```go
|
||||
func NewFirestoreHourRepository(client *firestore.Client, factory hour.Factory) *FirestoreHourRepository
|
||||
func NewMySQLHourRepository(db *sqlx.DB) *MySQLHourRepository
|
||||
func NewMemoryHourRepository(factory hour.Factory) *MemoryHourRepository
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## REPO-05: Technology Suffix Naming (INFO)
|
||||
|
||||
Adapter types use technology as a suffix/prefix to distinguish implementations.
|
||||
|
||||
```go
|
||||
type FirestoreHourRepository struct { ... }
|
||||
type MySQLHourRepository struct { ... }
|
||||
type MemoryHourRepository struct { ... }
|
||||
|
||||
// For external service clients
|
||||
type TrainerGrpc struct { ... }
|
||||
type UsersGrpc struct { ... }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## REPO-06: Shared Test Suite (WARNING)
|
||||
|
||||
Repository tests SHOULD run the same test logic against ALL implementations (memory, MySQL, Firestore, etc.). This ensures behavioral consistency.
|
||||
|
||||
**Pattern:**
|
||||
```go
|
||||
func createRepositories(t *testing.T) []Repository {
|
||||
return []Repository{
|
||||
{Name: "Firebase", Repository: newFirebaseRepository(t)},
|
||||
{Name: "MySQL", Repository: newMySQLRepository(t)},
|
||||
{Name: "memory", Repository: adapters.NewMemoryHourRepository(testFactory)},
|
||||
}
|
||||
}
|
||||
|
||||
func TestRepository(t *testing.T) {
|
||||
repositories := createRepositories(t)
|
||||
for i := range repositories {
|
||||
r := repositories[i] // capture loop variable
|
||||
t.Run(r.Name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
testUpdateHour(t, r.Repository)
|
||||
testUpdateHour_parallel(t, r.Repository)
|
||||
})
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Check:** Look for test files in `adapters/` that test repository implementations. Verify they use a shared test function or table-driven approach.
|
||||
|
||||
---
|
||||
|
||||
## REPO-07: UnmarshalFromDatabase Usage (WARNING)
|
||||
|
||||
Adapter implementations MUST use the entity's `UnmarshalFromDatabase` function to reconstruct domain objects from persistence, not the regular constructor.
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
func (r *FirestoreHourRepository) toHour(doc *firestore.DocumentSnapshot) (*hour.Hour, error) {
|
||||
var m HourModel
|
||||
if err := doc.DataTo(&m); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
availability, err := hour.NewAvailabilityFromString(m.Availability)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return hour.UnmarshalHourFromDatabase(m.Hour, availability), nil
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
func (r *FirestoreHourRepository) toHour(doc *firestore.DocumentSnapshot) (*hour.Hour, error) {
|
||||
// VIOLATION: using business constructor for DB reconstruction
|
||||
return hour.NewAvailableHour(m.Hour) // This re-validates and may reject valid stored data
|
||||
}
|
||||
```
|
||||
@@ -0,0 +1,407 @@
|
||||
# Watermill Rules (WM-01..10)
|
||||
|
||||
## WM-01: Watermill as a Named Component in Unified Server (CRITICAL)
|
||||
|
||||
Watermill router MUST be registered as a named component via `server.WithWatermillRouter(name, configure)` — same pattern as `WithHTTPHandler` and `WithGRPCServer`. The `With*` option owns AMQP connection, middleware, and router lifecycle. The caller provides **only handler registration** via callback.
|
||||
|
||||
This ensures:
|
||||
- Middleware stack (retry, correlation, recovery) is consistent across all services
|
||||
- Broker config is centralized — swapping AMQP for Kafka changes one file
|
||||
- Shutdown ordering is explicit via `server.OnShutdown(server.Stop(name))`
|
||||
|
||||
**Check procedure:**
|
||||
1. Scan `main.go` for direct Watermill router creation (`message.NewRouter`, `amqp.NewSubscriber`)
|
||||
2. Flag any middleware setup outside `server/watermill.go`
|
||||
3. Verify Watermill component appears in `OnShutdown` with correct ordering
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// internal/common/server/watermill.go
|
||||
func WithWatermillRouter(
|
||||
name string,
|
||||
configure func(*message.Router, message.Subscriber),
|
||||
) Option {
|
||||
return func(s *Server) {
|
||||
wmLogger := watermill.NewStdLoggerWithOut(os.Stdout, true, false)
|
||||
amqpURI := os.Getenv("AMQP_URI")
|
||||
amqpConfig := amqp.NewDurableQueueConfig(amqpURI)
|
||||
|
||||
sub, err := amqp.NewSubscriber(amqpConfig, wmLogger)
|
||||
if err != nil { panic(err) }
|
||||
|
||||
r, err := message.NewRouter(message.RouterConfig{}, wmLogger)
|
||||
if err != nil { panic(err) }
|
||||
|
||||
r.AddMiddleware(
|
||||
wmMiddleware.CorrelationID,
|
||||
wmMiddleware.Recoverer,
|
||||
wmMiddleware.Retry{MaxRetries: 3}.Middleware,
|
||||
)
|
||||
configure(r, sub)
|
||||
|
||||
s.addComponent(name, component{
|
||||
name: name,
|
||||
start: func(ctx context.Context) error {
|
||||
return r.Run(ctx)
|
||||
},
|
||||
stop: func(ctx context.Context) error {
|
||||
return r.Close()
|
||||
},
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// main.go — registered as named component
|
||||
server.New(
|
||||
server.WithWatermillRouter("events", func(r *message.Router, sub message.Subscriber) {
|
||||
ports.RegisterEventHandlers(r, sub, application)
|
||||
}),
|
||||
server.WithHTTPHandler("api", createHandler),
|
||||
server.OnShutdown(
|
||||
server.Stop("events"), // 1. stop consuming
|
||||
server.Stop("api"), // 2. drain HTTP
|
||||
server.StopFunc(cleanup), // 3. close clients
|
||||
),
|
||||
).Run(ctx)
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// main.go — VIOLATION: infrastructure in main
|
||||
func main() {
|
||||
sub, _ := amqp.NewSubscriber(amqpConfig, logger) // VIOLATION
|
||||
r, _ := message.NewRouter(message.RouterConfig{}, logger) // VIOLATION
|
||||
r.AddMiddleware(wmMiddleware.Recoverer) // VIOLATION
|
||||
r.Run(context.Background())
|
||||
}
|
||||
|
||||
// main.go — VIOLATION: standalone RunWatermillRouter without unified server
|
||||
server.RunWatermillRouter(func(r *message.Router, sub message.Subscriber) { ... })
|
||||
// Cannot coordinate shutdown with other transports
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## WM-02: Publisher Factory Returns (Publisher, Close, Error) Triple (CRITICAL)
|
||||
|
||||
Publisher creation MUST follow the same `(client, closeFunc, error)` triple-return pattern as `client.NewTrainerClient()` and `client.NewUsersClient()`. Config comes from environment variables.
|
||||
|
||||
**Check procedure:**
|
||||
1. Verify publisher factory in `internal/common/client/watermill.go`
|
||||
2. Must return `(message.Publisher, func() error, error)`
|
||||
3. Must read `AMQP_URI` from env
|
||||
4. Error case must return a no-op close function, never nil
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// internal/common/client/watermill.go
|
||||
func NewWatermillPublisher() (pub message.Publisher, close func() error, err error) {
|
||||
amqpURI := os.Getenv("AMQP_URI")
|
||||
if amqpURI == "" {
|
||||
return nil, func() error { return nil }, errors.New("empty env AMQP_URI")
|
||||
}
|
||||
|
||||
logger := watermill.NewStdLoggerWithOut(os.Stdout, true, false)
|
||||
config := amqp.NewDurableQueueConfig(amqpURI)
|
||||
|
||||
publisher, err := amqp.NewPublisher(config, logger)
|
||||
if err != nil {
|
||||
return nil, func() error { return nil }, errors.Wrap(err, "cannot create watermill publisher")
|
||||
}
|
||||
|
||||
return publisher, publisher.Close, nil
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// VIOLATION: returns raw connection, no close function
|
||||
func NewPublisher() *amqp.Publisher {
|
||||
pub, _ := amqp.NewPublisher(config, logger)
|
||||
return pub
|
||||
}
|
||||
|
||||
// VIOLATION: nil close function on error path
|
||||
func NewPublisher() (message.Publisher, func() error, error) {
|
||||
// ...
|
||||
return nil, nil, err // nil close panics on defer
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## WM-03: Event Handlers Live in Ports (CRITICAL)
|
||||
|
||||
Watermill event handlers are **inbound adapters** — they are ports, just like HTTP and gRPC handlers. They MUST:
|
||||
- Live in `ports/`
|
||||
- Hold `app.Application`
|
||||
- Delegate to command/query handlers
|
||||
- Contain NO business logic
|
||||
|
||||
**Check procedure:**
|
||||
1. Scan for `message.HandlerFunc` or `func(*message.Message) error` signatures
|
||||
2. These MUST be in `ports/` package
|
||||
3. Must import `app/`, `app/command/`, or `app/query/` — not `domain/` directly
|
||||
4. Must follow the same delegation pattern as HTTP/gRPC handlers
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// ports/event.go
|
||||
type EventHandlers struct {
|
||||
app app.Application
|
||||
}
|
||||
|
||||
func RegisterEventHandlers(r *message.Router, sub message.Subscriber, application app.Application) {
|
||||
handlers := EventHandlers{app: application}
|
||||
|
||||
r.AddNoPublisherHandler(
|
||||
"OnTrainingScheduled",
|
||||
"training.scheduled",
|
||||
sub,
|
||||
handlers.OnTrainingScheduled,
|
||||
)
|
||||
}
|
||||
|
||||
func (h EventHandlers) OnTrainingScheduled(msg *message.Message) error {
|
||||
var event TrainingScheduledEvent
|
||||
if err := json.Unmarshal(msg.Payload, &event); err != nil {
|
||||
return err
|
||||
}
|
||||
return h.app.Commands.ScheduleTraining.Handle(
|
||||
msg.Context(),
|
||||
command.ScheduleTraining{Hour: event.Hour},
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// adapters/event_handler.go — VIOLATION: handler in adapters/
|
||||
func HandleTrainingScheduled(msg *message.Message) error {
|
||||
repo.Save(ctx, training) // VIOLATION: direct repo access
|
||||
}
|
||||
|
||||
// app/command/schedule_training.go — VIOLATION: message parsing in app layer
|
||||
func (h handler) Handle(ctx context.Context, msg *message.Message) error { ... }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## WM-04: Event Publisher Adapter Implements Domain Interface (WARNING)
|
||||
|
||||
Publishing events MUST go through an adapter that implements an interface defined in the app or domain layer. The app layer defines *what* events to publish; the adapter knows *how*.
|
||||
|
||||
This keeps Watermill as a swappable infrastructure detail.
|
||||
|
||||
**Check procedure:**
|
||||
1. Look for `message.Publisher` usage — it MUST NOT appear in `app/` or `domain/`
|
||||
2. An interface like `EventPublisher` should be in `app/command/services.go` or similar
|
||||
3. The concrete adapter in `adapters/` implements it using Watermill
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// app/command/services.go
|
||||
type TrainingEventPublisher interface {
|
||||
TrainingScheduled(ctx context.Context, t training.Training) error
|
||||
TrainingCancelled(ctx context.Context, trainingUUID string) error
|
||||
}
|
||||
|
||||
// adapters/training_event_publisher.go
|
||||
type WatermillTrainingEventPublisher struct {
|
||||
pub message.Publisher
|
||||
}
|
||||
|
||||
func NewWatermillTrainingEventPublisher(pub message.Publisher) WatermillTrainingEventPublisher {
|
||||
return WatermillTrainingEventPublisher{pub: pub}
|
||||
}
|
||||
|
||||
func (p WatermillTrainingEventPublisher) TrainingScheduled(ctx context.Context, t training.Training) error {
|
||||
payload, err := json.Marshal(TrainingScheduledEvent{UUID: t.UUID(), Hour: t.Time()})
|
||||
if err != nil { return err }
|
||||
msg := message.NewMessage(watermill.NewUUID(), payload)
|
||||
middleware.SetCorrelationID(middleware.MessageCorrelationID(msg), msg)
|
||||
return p.pub.Publish("training.scheduled", msg)
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// app/command/schedule_training.go — VIOLATION: Watermill in app layer
|
||||
import "github.com/ThreeDotsLabs/watermill/message"
|
||||
|
||||
func (h handler) Handle(ctx context.Context, cmd ScheduleTraining) error {
|
||||
msg := message.NewMessage(watermill.NewUUID(), payload) // VIOLATION
|
||||
h.publisher.Publish("topic", msg) // VIOLATION: infra detail
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## WM-05: Topic Naming Uses Domain Language (WARNING)
|
||||
|
||||
Topic/queue names MUST use domain language with dot notation: `{aggregate}.{past-tense-event}`. No CRUD names, no technical prefixes.
|
||||
|
||||
**Correct:**
|
||||
```
|
||||
training.scheduled
|
||||
training.cancelled
|
||||
training.reschedule_requested
|
||||
hour.made_available
|
||||
```
|
||||
|
||||
**Wrong:**
|
||||
```
|
||||
create-training // VIOLATION: CRUD name
|
||||
events.training.created // VIOLATION: redundant "events" prefix, CRUD
|
||||
TRAINING_QUEUE // VIOLATION: technical name, not domain event
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## WM-06: Event Structs Live in the Publishing Port or Adapter (INFO)
|
||||
|
||||
Event DTOs (the JSON payloads) are protocol-specific — they belong in `ports/` or `adapters/`, NOT in `domain/`. Domain entities are the canonical model; events are a serialization concern.
|
||||
|
||||
**Check procedure:**
|
||||
1. Look for event structs (e.g., `TrainingScheduledEvent`)
|
||||
2. They MUST be in `ports/` (if consumed by event handlers) or `adapters/` (if produced by publisher adapters)
|
||||
3. They MUST NOT be in `domain/`
|
||||
|
||||
**Correct:**
|
||||
```go
|
||||
// ports/event.go or adapters/training_event_publisher.go
|
||||
type TrainingScheduledEvent struct {
|
||||
UUID string `json:"uuid"`
|
||||
Hour time.Time `json:"hour"`
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## WM-07: Watermill Middleware in With* Option Only (WARNING)
|
||||
|
||||
Watermill middleware (retry, correlation ID, recoverer, throttle, etc.) MUST be configured exclusively inside the `WithWatermillRouter` option in `internal/common/server/watermill.go` — same principle as ARCH-06 for HTTP/gRPC middleware.
|
||||
|
||||
**Check procedure:**
|
||||
1. Scan for `r.AddMiddleware` or `router.AddMiddleware` calls
|
||||
2. All MUST be in `internal/common/server/watermill.go` (inside `WithWatermillRouter`)
|
||||
3. Flag any middleware setup in `main.go`, `ports/`, or `service/`
|
||||
|
||||
---
|
||||
|
||||
## WM-08: Publisher Cleanup via OnShutdown or Composition Root (WARNING)
|
||||
|
||||
When a service publishes events, the publisher's close function MUST be closed as part of the shutdown sequence. Two valid patterns:
|
||||
|
||||
**Pattern A — cleanup in OnShutdown (preferred when using unified server):**
|
||||
```go
|
||||
server.New(
|
||||
server.WithHTTPHandler("api", createHandler),
|
||||
server.OnShutdown(
|
||||
server.Stop("api"), // 1. drain HTTP (in-flight may publish)
|
||||
server.StopFunc(cleanup), // 2. close publisher + clients
|
||||
),
|
||||
).Run(ctx)
|
||||
```
|
||||
|
||||
**Pattern B — cleanup via defer (simpler services):**
|
||||
```go
|
||||
app, cleanup := service.NewApplication(ctx)
|
||||
defer cleanup() // runs after Run() returns
|
||||
|
||||
server.New(
|
||||
server.WithHTTPHandler("api", createHandler),
|
||||
server.OnShutdown(
|
||||
server.Stop("api"),
|
||||
),
|
||||
).Run(ctx)
|
||||
// cleanup() runs here via defer — publisher closes after server drained
|
||||
```
|
||||
|
||||
**Check procedure:**
|
||||
1. If `service/application.go` creates a publisher, verify close is either in `OnShutdown` or in the cleanup function
|
||||
2. Publisher close MUST happen *after* all transports that might publish are stopped
|
||||
3. Closing publisher before draining HTTP/gRPC = lost messages
|
||||
|
||||
**Wrong:**
|
||||
```go
|
||||
// main.go — VIOLATION: publisher lifecycle in main, not ordered
|
||||
func main() {
|
||||
pub, closePub, _ := client.NewWatermillPublisher()
|
||||
defer closePub() // VIOLATION: may close before HTTP drains
|
||||
app := service.NewApplication(ctx, pub) // VIOLATION: infra detail leaked
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## WM-09: Named Components Replace SERVER_TO_RUN Switch (INFO)
|
||||
|
||||
With the unified server pattern (ARCH-08), the `SERVER_TO_RUN` environment variable switch is replaced by composing `With*` options. A service that needs HTTP + Watermill simply registers both.
|
||||
|
||||
**Correct — unified server:**
|
||||
```go
|
||||
// All transports in one process, explicit shutdown order
|
||||
server.New(
|
||||
server.WithWatermillRouter("events", func(r *message.Router, sub message.Subscriber) {
|
||||
ports.RegisterEventHandlers(r, sub, app)
|
||||
}),
|
||||
server.WithHTTPHandler("api", func(router chi.Router) http.Handler {
|
||||
return ports.HandlerFromMux(ports.NewHttpServer(app), router)
|
||||
}),
|
||||
server.OnShutdown(
|
||||
server.Stop("events"),
|
||||
server.Stop("api"),
|
||||
server.StopFunc(cleanup),
|
||||
),
|
||||
).Run(ctx)
|
||||
```
|
||||
|
||||
**Also acceptable — SERVER_TO_RUN for single-transport deployments:**
|
||||
```go
|
||||
// When deploying each transport as a separate container
|
||||
switch serverType {
|
||||
case "http":
|
||||
server.New(
|
||||
server.WithHTTPHandler("api", createHandler),
|
||||
server.OnShutdown(server.Stop("api")),
|
||||
).Run(ctx)
|
||||
case "watermill":
|
||||
server.New(
|
||||
server.WithWatermillRouter("events", configureRouter),
|
||||
server.OnShutdown(server.Stop("events")),
|
||||
).Run(ctx)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## WM-10: No Synchronous Side Effects Replaced by Fire-and-Forget (CRITICAL)
|
||||
|
||||
When replacing synchronous gRPC calls with async events, you MUST ensure the operation tolerates eventual consistency. If the caller needs confirmation that the action succeeded, keep it synchronous (gRPC) or use a saga/process manager — do NOT simply drop the response.
|
||||
|
||||
**Check procedure:**
|
||||
1. For each gRPC adapter being replaced by events, check if the calling command inspects the return value or error
|
||||
2. If the command makes decisions based on the result, it MUST remain synchronous or use a compensation pattern
|
||||
3. Fire-and-forget is only valid for notifications, projections, and truly independent side effects
|
||||
|
||||
**Correct use of async:**
|
||||
```go
|
||||
// Notification — caller doesn't need the result
|
||||
func (h handler) Handle(ctx context.Context, cmd ScheduleTraining) error {
|
||||
// ... create training ...
|
||||
// Fire event — consumer will send email, update dashboard, etc.
|
||||
return h.eventPublisher.TrainingScheduled(ctx, training)
|
||||
}
|
||||
```
|
||||
|
||||
**Wrong use of async:**
|
||||
```go
|
||||
// VIOLATION: caller needs confirmation that hours were reserved
|
||||
func (h handler) Handle(ctx context.Context, cmd ScheduleTraining) error {
|
||||
training, _ := training.NewTraining(...)
|
||||
h.eventPublisher.TrainingScheduled(ctx, training) // VIOLATION: no guarantee hours are available
|
||||
return h.repo.Save(ctx, training) // saved training without confirmed availability
|
||||
}
|
||||
// Previously this was a synchronous gRPC call that could fail and roll back
|
||||
```
|
||||
Reference in New Issue
Block a user