Skip to content
Merged
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: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ GoBricks is a **production-grade framework for building MVPs fast**. It provides
- **logger/** — Structured logging (zerolog)
- **messaging/** — AMQP client for RabbitMQ
- **scheduler/** — gocron-based job scheduling with observability and CIDR-restricted APIs
- **server/** — Echo-based HTTP server. Echo stays the engine but no `echo.*` type appears on the consumer surface (ADR-034): custom middleware is `server.MiddlewareFunc` (flat `func(c HandlerContext, next func() error) error`), raw/ready handlers are `server.Handler`, and request access goes through `ctx.RequestContext()` / `ctx.Request()` accessors (the `Echo` field is removed); route template + ordered path params via `ctx.RouteTemplate()` / `ctx.PathParams()` / `ctx.SetPathParams()` (v0.46). Set `server.logroutes` (env `SERVER_LOGROUTES`; tri-state, default dev-on/prod-off) to emit one Info line per registered route at startup (`Route registered module=… method=… path=…`) — see [startup_defaults.md](wiki/startup_defaults.md#startup-route-logging).
- **server/** — Echo-based HTTP server. Echo stays the engine but no `echo.*` type appears on the consumer surface (ADR-034): custom middleware is `server.MiddlewareFunc` (flat `func(c HandlerContext, next func() error) error`), raw/ready handlers are `server.Handler`, and request access goes through `ctx.RequestContext()` / `ctx.Request()` accessors (the `Echo` field is removed); route template + ordered path params via `ctx.RouteTemplate()` / `ctx.PathParams()` / `ctx.SetPathParams()` (v0.46). Set `server.logroutes` (env `SERVER_LOGROUTES`; tri-state, default dev-on/prod-off) to emit one Info line per registered route at startup (`Route registered module=… method=… path=…`) — see [startup_defaults.md](wiki/startup_defaults.md#startup-route-logging). Duplicate route registration (same method + full path) fails startup with an aggregate error naming both registrants.
- **migration/** — Flyway integration with single- and multi-tenant runners; pairs with `tools/migration` CLI (`go-bricks-migrate`) for CI/CD fleet rollouts. Emits `migration.applied` audit events on every migrate invocation via OTel by default; opt-in `AuditRecorder` for compliance-grade durable delivery. PostgreSQL migrator-vs-runtime role separation (`ProvisionPGRoles` / `PGRoleProvisioningSQL`) gives auditors a flat *no* to "can the running service alter its own schema?". The `migration/provisioning/` subpackage carries a durable, crash-recoverable per-tenant state machine (`pending → schema_created → role_created → migrated → seeded → ready`, with `cleanup → failed` branches) for dynamic tenant provisioning. A deployment **quiesce flag** (`QuiesceController` / `Executor.WithQuiesce` / `MigrateAllOptions.Quiesce`) lets a deployment-time migration pause worker pickup and tenant fan-out — in-flight work drains, nothing is interrupted — with read-side TTL auto-release (crash-safe, no sweeper) and fail-open on control-plane errors. See [multi_tenant_migration.md](wiki/multi_tenant_migration.md), [migration_roles.md](wiki/migration_roles.md), [migration_provisioning.md](wiki/migration_provisioning.md), [migration_quiesce.md](wiki/migration_quiesce.md), [migration_audit.md](wiki/migration_audit.md), [ADR-018](wiki/adr_018_multi_tenant_migration_cli.md), [ADR-019](wiki/adr_019_migration_audit_delivery.md), and [ADR-021](wiki/adr_021_provisioning_state_machine.md).
- **multitenant/** — Tenant identifier resolution from incoming HTTP requests. Four resolver types: `header` (default `X-Tenant-ID`), `subdomain` (`<tenant>.<domain>`), `path` (1-indexed segment with optional prefix gate; e.g. `/itsp/{tenantID}/...`), and `composite` (first-match fallback chain; `resolver.order` is **required** — no default, composite fails at startup without it. Recommended `[subdomain, path, header]`; header-first if a trusted gateway owns `X-Tenant-ID` — ADR-039). Resolution is identification, not authorization: all three sources are caller-written, so the deployment must still authorize the resolved tenant. All run before route matching so the resolved tenant is in `context.Context` for every middleware and handler. Per-tenant DB/cache/messaging accessors (`deps.DB(ctx)`, etc.) consume the value transparently. See [multi_tenant_resolvers.md](wiki/multi_tenant_resolvers.md).
- **observability/** — OpenTelemetry tracing and metrics
Expand Down
27 changes: 27 additions & 0 deletions app/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,38 @@ func (a *App) prepareRuntime() error {
}

a.registry.RegisterRoutes(a.server.ModuleGroup())
if err := a.checkRouteConflicts(); err != nil {
return err
}
a.startMaintenanceLoops()

return nil
}

// checkRouteConflicts fails startup when two registrations claimed the same
// method+path: echo's router silently overwrites (last one wins), so the first
// handler would be dead on arrival. Fail Fast: surface every collision at once.
// Servers that don't expose conflict tracking (test fakes) are skipped.
func (a *App) checkRouteConflicts() error {
cs, ok := a.server.(interface{ RouteConflicts() []server.RouteConflict })
if !ok {
return nil
}
conflicts := cs.RouteConflicts()
if len(conflicts) == 0 {
return nil
}
errs := make([]error, 0, len(conflicts)+1)
errs = append(errs, fmt.Errorf("duplicate route registration (%d conflict(s))", len(conflicts)))
for _, c := range conflicts {
errs = append(errs, fmt.Errorf("%s %s — first: %s (%s), duplicate: %s (%s)",
c.Method, c.Path,
c.First.HandlerName, c.First.Package,
c.Duplicate.HandlerName, c.Duplicate.Package))
}
return errors.Join(errs...)
}

// applyGlobalMiddleware registers module-contributed global middleware on the server. It
// fails closed: if any module registered middleware but the server cannot install it, the
// gate (canonically auth) would be silently absent, so startup aborts rather than serving
Expand Down
77 changes: 77 additions & 0 deletions app/lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package app

import (
"context"
"net/http"
"testing"
"time"

Expand Down Expand Up @@ -477,3 +478,79 @@ func TestStartMaintenanceLoopsWarnsOnLateCleanupInterval(t *testing.T) {
return manager.Stats()["active_publishers"] == 0
}, time.Second, 10*time.Millisecond, "cleanup loop must still run despite the late-cleanupinterval WARN")
}

// TestCheckRouteConflictsAggregatesAndSkips drives checkRouteConflicts directly against a
// real server.Server (white-box field access — the mock server used elsewhere in this file
// has no-op registrars and never implements RouteConflicts(), so positive cases can only be
// exercised against a real server), covering report, aggregate, and skip-path behaviors.
func TestCheckRouteConflictsAggregatesAndSkips(t *testing.T) {
log := logger.New("error", false)
noop := func(c server.HandlerContext) error { return c.String(http.StatusOK, "") }

tests := []struct {
name string
buildApp func() *App
wantErr bool
wantContains []string
}{
{
name: "one_conflict",
buildApp: func() *App {
srv := server.New(&config.Config{}, log)
mg := srv.ModuleGroup()
mg.Add(http.MethodGet, "/dup", noop)
mg.Add(http.MethodGet, "/dup", noop)
return &App{server: srv}
},
wantErr: true,
wantContains: []string{"duplicate route registration (1 conflict(s))", "GET /dup"},
},
{
name: "two_conflicts",
buildApp: func() *App {
srv := server.New(&config.Config{}, log)
mg := srv.ModuleGroup()
mg.Add(http.MethodGet, "/one", noop)
mg.Add(http.MethodGet, "/one", noop)
mg.Add(http.MethodPost, "/two", noop)
mg.Add(http.MethodPost, "/two", noop)
return &App{server: srv}
},
wantErr: true,
wantContains: []string{"(2 conflict(s))", "GET /one", "POST /two"},
},
{
name: "no_conflicts",
buildApp: func() *App {
srv := server.New(&config.Config{}, log)
mg := srv.ModuleGroup()
mg.Add(http.MethodGet, "/one", noop)
mg.Add(http.MethodPost, "/two", noop)
return &App{server: srv}
},
wantErr: false,
},
{
name: "fake_server_skipped",
buildApp: func() *App {
return &App{server: newMockServer()}
},
wantErr: false,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
a := tc.buildApp()
err := a.checkRouteConflicts()
if !tc.wantErr {
require.NoError(t, err)
return
}
require.Error(t, err)
for _, want := range tc.wantContains {
assert.Contains(t, err.Error(), want)
}
})
}
}
7 changes: 5 additions & 2 deletions server/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -1432,7 +1432,7 @@ type RouteRegistrar interface {
// ADR-026), while consumers use the echo-free RouteRegistrar.Add. It is an optional
// interface upgrade in the style of io.ReaderFrom.
type echoAdder interface {
addEcho(method, path string, h echo.HandlerFunc)
addEcho(method, path string, h echo.HandlerFunc, reg RouteRegistrant)
}

// HandlerRegistry manages enhanced handlers and provides registration utilities.
Expand Down Expand Up @@ -1521,7 +1521,10 @@ func RegisterHandler[T any, R any](
// non-routeGroup registrars (test fakes); it round-trips through the unexported escape
// hatch and never executes on the framework's real registrar.
if er, ok := r.(echoAdder); ok {
er.addEcho(method, path, wrappedHandler)
er.addEcho(method, path, wrappedHandler, RouteRegistrant{
HandlerName: descriptor.HandlerName,
Package: descriptor.Package,
})
} else {
r.Add(method, path, func(c HandlerContext) error {
ec := c.echoContext()
Expand Down
63 changes: 63 additions & 0 deletions server/route_conflicts.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package server

import "sync"

// serverPackagePath attributes framework-registered routes (health/ready probes)
// in conflict reports.
const serverPackagePath = "github.com/gaborage/go-bricks/server"

// RouteRegistrant identifies who registered a route, for conflict reporting.
// ModuleName is intentionally absent: no registration path populates it
// (attribution in route logging uses registration-order spans instead).
type RouteRegistrant struct {
HandlerName string
Package string
}

// RouteConflict reports two registrations of the same method + full path.
type RouteConflict struct {
Method string
Path string
First RouteRegistrant
Duplicate RouteRegistrant
}

// routeConflictTracker records every route added through a Server's routeGroups
// and accumulates conflicts. One instance per Server; a nil tracker disables
// recording (bare newRouteGroup construction in tests).
type routeConflictTracker struct {
mu sync.Mutex
seen map[string]RouteRegistrant // key: formatHandlerID(method, fullPath)
conflicts []RouteConflict
}

func newRouteConflictTracker() *routeConflictTracker {
return &routeConflictTracker{seen: make(map[string]RouteRegistrant)}
}

func (t *routeConflictTracker) record(method, fullPath string, reg RouteRegistrant) {
if t == nil {
return
}
t.mu.Lock()
defer t.mu.Unlock()
key := formatHandlerID(method, fullPath)
if first, dup := t.seen[key]; dup {
t.conflicts = append(t.conflicts, RouteConflict{
Method: method, Path: fullPath, First: first, Duplicate: reg,
})
return
}
t.seen[key] = reg
}

func (t *routeConflictTracker) snapshot() []RouteConflict {
if t == nil {
return nil
}
t.mu.Lock()
defer t.mu.Unlock()
out := make([]RouteConflict, len(t.conflicts))
copy(out, t.conflicts)
return out
}
101 changes: 101 additions & 0 deletions server/route_conflicts_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package server

import (
"net/http"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestRouteConflictTrackerRecordsDuplicate(t *testing.T) {
tr := newRouteConflictTracker()

first := RouteRegistrant{HandlerName: "handlerA", Package: "pkg/a"}
dup := RouteRegistrant{HandlerName: "handlerB", Package: "pkg/b"}

tr.record(http.MethodGet, "/users", first)
tr.record(http.MethodGet, "/users", dup)

conflicts := tr.snapshot()
require.Len(t, conflicts, 1)
assert.Equal(t, RouteConflict{
Method: http.MethodGet, Path: "/users",
First: first, Duplicate: dup,
}, conflicts[0])

// Different methods, same path — no conflict.
trMethods := newRouteConflictTracker()
trMethods.record(http.MethodGet, "/users", first)
trMethods.record(http.MethodPost, "/users", dup)
assert.Empty(t, trMethods.snapshot())

// A nil tracker must not panic and must report no conflicts.
var nilTracker *routeConflictTracker
assert.NotPanics(t, func() {
nilTracker.record(http.MethodGet, "/x", first)
})
assert.Nil(t, nilTracker.snapshot())
}

func TestServerRouteConflictsAcrossGroups(t *testing.T) {
srv := newTestServer("", "", "")

moduleGroup := srv.ModuleGroup()
rootGroup := srv.RootGroup()

noop := func(c HandlerContext) error { return c.String(http.StatusOK, "") }

moduleGroup.Add(http.MethodGet, "/shared", noop)
rootGroup.Add(http.MethodGet, "/shared", noop)

conflicts := srv.RouteConflicts()
require.Len(t, conflicts, 1, "same method+path registered via ModuleGroup and RootGroup should collide")
assert.Equal(t, http.MethodGet, conflicts[0].Method)
assert.Equal(t, "/shared", conflicts[0].Path)

// Nested Group() registration colliding with a flat registration of the identical
// full path proves tracker propagation through Group().
sub := moduleGroup.Group("/sub")
sub.Add(http.MethodGet, "/leaf", noop)
moduleGroup.Add(http.MethodGet, "/sub/leaf", noop)

conflicts = srv.RouteConflicts()
require.Len(t, conflicts, 2, "nested-group registration colliding with an equivalent flat path should be detected")
}

func TestRouteConflictDetectsProbeCollision(t *testing.T) {
srv := newTestServer("", "", "") // health defaults to /health, ready to /ready

noop := func(c HandlerContext) error { return c.String(http.StatusOK, "") }
srv.ModuleGroup().Add(http.MethodGet, "/health", noop)

conflicts := srv.RouteConflicts()
require.Len(t, conflicts, 1, "module route shadowing the health probe must be detected")
assert.Equal(t, http.MethodGet, conflicts[0].Method)
assert.Equal(t, "/health", conflicts[0].Path)
assert.Equal(t, "healthCheck", conflicts[0].First.HandlerName)
assert.Equal(t, serverPackagePath, conflicts[0].First.Package)
}

func TestRouteConflictTypedAndRawBothTracked(t *testing.T) {
srv := newTestServer("", "", "")
hr := NewHandlerRegistry(srv.cfg)
moduleGroup := srv.ModuleGroup()

GET(hr, moduleGroup, "/dup", func(_ EmptyRequest, _ HandlerContext) (helloResp, IAPIError) {
return helloResp{Message: "typed"}, nil
})

moduleGroup.Add(http.MethodGet, "/dup", func(c HandlerContext) error {
return c.String(http.StatusOK, "raw")
})

conflicts := srv.RouteConflicts()
require.Len(t, conflicts, 1)
c := conflicts[0]
assert.Equal(t, http.MethodGet, c.Method)
assert.Equal(t, "/dup", c.Path)
assert.NotEmpty(t, c.First.HandlerName, "typed registration's provenance must thread through the addEcho seam")
assert.NotEmpty(t, c.First.Package)
}
38 changes: 28 additions & 10 deletions server/route_registrar.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import (
)

type routeGroup struct {
group *echo.Group
prefix string
cfg *config.Config // populates HandlerContext.Config in the go-bricks↔echo adapters
group *echo.Group
prefix string
cfg *config.Config // populates HandlerContext.Config in the go-bricks↔echo adapters
tracker *routeConflictTracker
}

func newRouteGroup(group *echo.Group, prefix string, cfg *config.Config) RouteRegistrar {
Expand All @@ -21,11 +22,22 @@ func newRouteGroup(group *echo.Group, prefix string, cfg *config.Config) RouteRe
}
}

// newTrackedRouteGroup is newRouteGroup plus a shared conflict tracker: Server's group
// factories use it so every routeGroup they hand out (including nested Group() children)
// records into the same per-Server tracker.
func newTrackedRouteGroup(group *echo.Group, prefix string, cfg *config.Config, tracker *routeConflictTracker) RouteRegistrar {
rg := newRouteGroup(group, prefix, cfg).(*routeGroup)
rg.tracker = tracker
return rg
}

// addEcho implements the unexported echoAdder seam: it registers a pre-built
// echo.HandlerFunc directly, so the framework's typed-handler hot path pays no
// per-request adapter cost (ADR-026).
func (rg *routeGroup) addEcho(method, path string, h echo.HandlerFunc) {
rg.group.Add(method, rg.relativePath(path), h)
func (rg *routeGroup) addEcho(method, path string, h echo.HandlerFunc, reg RouteRegistrant) {
relative := rg.relativePath(path)
rg.group.Add(method, relative, h)
rg.tracker.record(method, rg.fullPathFromRelative(relative), reg)
}

// Add registers an echo-free Handler with optional flat middleware. The go-bricks→echo
Expand All @@ -43,12 +55,17 @@ func (rg *routeGroup) Add(method, path string, handler Handler, middleware ...Mi
rg.group.Add(method, relative, adaptHandler(handler, rg.cfg), rg.adaptAll(middleware)...)

fullPath := rg.fullPathFromRelative(relative)
handlerName := extractHandlerName(handler)
pkg := getCallerPackage(2) // getCallerPackage → Add → module (best-effort, as typed routes)

rg.tracker.record(method, fullPath, RouteRegistrant{HandlerName: handlerName, Package: pkg})

DefaultRouteRegistry.Register(&RouteDescriptor{
Method: method,
Path: fullPath,
HandlerID: formatHandlerID(method, fullPath),
HandlerName: extractHandlerName(handler),
Package: getCallerPackage(2), // getCallerPackage → Add → module (best-effort, as typed routes)
HandlerName: handlerName,
Package: pkg,
})
}

Expand All @@ -57,9 +74,10 @@ func (rg *routeGroup) Group(prefix string, middleware ...MiddlewareFunc) RouteRe
newGroup := rg.group.Group(normalized, rg.adaptAll(middleware)...)

return &routeGroup{
group: newGroup,
prefix: rg.combinePrefix(normalized),
cfg: rg.cfg,
group: newGroup,
prefix: rg.combinePrefix(normalized),
cfg: rg.cfg,
tracker: rg.tracker,
}
}

Expand Down
Loading