diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82b166c..ec82efa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,5 +58,5 @@ jobs: uses: codecov/codecov-action@v5 with: token: ${{ secrets.CODECOV_TOKEN }} - files: coverage.out,coverage-examples.out + files: .coverage/coverage.out,.coverage/coverage-examples.out fail_ci_if_error: false diff --git a/.gitignore b/.gitignore index 63df557..c3709e5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,8 +1,10 @@ .worktrees/ # Coverage files -coverage.out -coverage-examples.out +.coverage/ + +# Build cache +.cache/ # Internal planning files .plans/ diff --git a/Makefile b/Makefile index 1fa374c..6dac2e2 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ SHELL := /bin/sh -.PHONY: fmt lint vuln test test-coverage tools setup-hooks lint-commit +.PHONY: fmt lint vuln test test-coverage test-patch-coverage tools setup-hooks lint-commit GOPATH ?= $(shell go env GOPATH) GOIMPORTS ?= $(GOPATH)/bin/goimports @@ -23,8 +23,28 @@ test: go test ./... test-coverage: - go test -race -coverprofile=coverage.out -covermode=atomic ./... - go test -race -coverprofile=coverage-examples.out -covermode=atomic ./examples/hello-mysql/... + @mkdir -p .coverage + go test -race -coverprofile=.coverage/coverage.out -covermode=atomic ./... + go test -race -coverprofile=.coverage/coverage-examples.out -covermode=atomic ./examples/hello-mysql/... + +test-patch-coverage: + @mkdir -p .coverage + @changed=$$(git diff --name-only origin/main...HEAD --diff-filter=AM | grep '\.go$$' || true); \ + if [ -z "$$changed" ]; then \ + echo "No changed Go files detected vs origin/main."; \ + exit 0; \ + fi; \ + pkgs=$$(echo "$$changed" | xargs -n1 dirname | sort -u | awk '{ if ($$0 == ".") { print "./..." } else { print "./"$$0"/..." } }' | tr '\n' ' '); \ + echo "Running patch coverage for packages:"; \ + echo "$$pkgs" | tr ' ' '\n'; \ + go test -race -coverprofile=.coverage/coverage-patch.out -covermode=atomic $$pkgs; \ + exclude_pattern=$$(awk '/^ignore:/{flag=1; next} /^[a-z]/ && !/^ignore:/{flag=0} flag && /^ -/{gsub(/^[[:space:]]*-[[:space:]]*"/,""); gsub(/"$$/,""); print}' codecov.yml 2>/dev/null | tr '\n' '|' | sed 's/|$$//'); \ + if [ -n "$$exclude_pattern" ]; then \ + grep -vE "$$exclude_pattern" .coverage/coverage-patch.out > .coverage/coverage-patch-filtered.out || cp .coverage/coverage-patch.out .coverage/coverage-patch-filtered.out; \ + go tool cover -func=.coverage/coverage-patch-filtered.out; \ + else \ + go tool cover -func=.coverage/coverage-patch.out; \ + fi # Install all development tools (tracked in tools/tools.go) tools: diff --git a/codecov.yml b/codecov.yml index bce6003..5a5d669 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,6 +1,13 @@ # Codecov configuration # https://docs.codecov.com/docs/codecovyml-reference +# Exclude files from coverage calculation +ignore: + # Integration-only code (requires real database) + - "examples/hello-mysql/internal/modules/users/repo_mysql.go" + # Untestable entry point + - "examples/hello-mysql/cmd/api/main.go" + coverage: status: # Project coverage: overall repository coverage check diff --git a/docs/plans/2026-02-05-auth-example-design.md b/docs/plans/2026-02-05-auth-example-design.md deleted file mode 100644 index 7a26fc4..0000000 --- a/docs/plans/2026-02-05-auth-example-design.md +++ /dev/null @@ -1,56 +0,0 @@ -# Auth Example Design - -**Goal:** Add a runnable, example-focused JWT authentication module to hello-mysql with a login endpoint, middleware validation, and typed context helpers. - -**Architecture:** A dedicated `auth` module provides a login handler and a JWT middleware provider. Configuration is explicit via example config/env. The middleware validates tokens and stores user info in a typed context helper, which handlers can read. User write routes (`POST /users`, `PUT /users/{id}`, `DELETE /users/{id}`) are protected by the auth middleware, while read routes (`GET /users`, `GET /users/{id}`) remain public. - -**Tech Stack:** Go, chi router via modkit http adapter, standard library + minimal JWT dependency. - ---- - -## Section 1 — Architecture Summary - -We add `examples/hello-mysql/internal/modules/auth` with a deterministic module definition and provider scaffolding. The module exports two primary providers: a JWT validation middleware and a login handler/controller. Configuration is explicit and local to the example (`JWT_SECRET`, `JWT_ISSUER`, `JWT_TTL`, `AUTH_USERNAME`, `AUTH_PASSWORD`). The login endpoint verifies demo credentials (no DB, no hashing) and returns a signed HS256 JWT with a minimal subject/email claim. The middleware validates the `Authorization: Bearer ` header, verifies signature + expiry, and stores authenticated user info in the request context via typed helpers. Downstream handlers access user info using those helpers only; no global state. - -## Section 2 — Components and Data Flow - -**Config:** Extend `examples/hello-mysql/internal/platform/config` with JWT + demo auth fields. `Load()` pulls from env with defaults (e.g., username `demo`, password `demo`, issuer `hello-mysql`, TTL `1h`). - -**Auth Module:** -- `module.go`: registers module name, exports provider tokens. -- `providers.go`: builds middleware and login handler using config. -- `config.go`: holds auth config struct sourced from platform config. - -**JWT Middleware:** -- Extracts bearer token, returns 401 on missing/invalid tokens. -- Verifies signature and expiry using HS256. -- On success, stores `User{ID, Email}` in context. - -**Login Handler:** -- `POST /auth/login` expects JSON with username/password. -- Validates against demo config values. -- Returns `{ "token": "" }` on success. - -**Typed Context Helpers:** -- `WithUser(ctx, user)` and `UserFromContext(ctx)` in `context.go`. -- Used by handlers and tests to show how to access authenticated user. - -## Section 3 — Error Handling, Tests, and Docs - -**Errors:** Use existing `httpapi.WriteProblem` for auth errors with status `401`. Validation errors for login payload are `400`. Internal issues return `500` with explicit context wrapping. - -**Tests:** -- Unit tests for middleware and context helpers (valid/invalid token, missing header). -- Integration tests for `/auth/login` and protected routes (valid/invalid creds). -- Use table-driven tests for token validation cases. - -**Documentation:** Update `examples/hello-mysql/README.md` with login example, token usage, and which `/users` routes require auth. Keep examples aligned with code paths. - ---- - -**Defaults (chosen):** -- `AUTH_USERNAME=demo` -- `AUTH_PASSWORD=demo` -- `JWT_ISSUER=hello-mysql` -- `JWT_TTL=1h` -- `JWT_SECRET=dev-secret-change-me` diff --git a/examples/hello-mysql/README.md b/examples/hello-mysql/README.md index 938ad68..1d6c5fd 100644 --- a/examples/hello-mysql/README.md +++ b/examples/hello-mysql/README.md @@ -129,6 +129,16 @@ Example response: } ``` +## Lifecycle and Cleanup + +Cleanup hooks are registered on providers via `ProviderDef.Cleanup`. The database module uses this hook to close the `*sql.DB` pool. + +On shutdown, the API server: +- Stops accepting new requests and waits for in-flight requests to finish. +- Runs cleanup hooks in **LIFO** order (last registered, first cleaned). + +The users service includes a context cancellation example via `Service.LongOperation`, which exits early with `context.Canceled` when the request is canceled. + ## Test ```bash diff --git a/examples/hello-mysql/cmd/api/main.go b/examples/hello-mysql/cmd/api/main.go index 4d2c35f..c6bf94a 100644 --- a/examples/hello-mysql/cmd/api/main.go +++ b/examples/hello-mysql/cmd/api/main.go @@ -1,11 +1,17 @@ package main import ( + "context" "log" + "net/http" + "os" + "os/signal" + "syscall" "time" _ "github.com/go-modkit/modkit/examples/hello-mysql/docs" "github.com/go-modkit/modkit/examples/hello-mysql/internal/httpserver" + "github.com/go-modkit/modkit/examples/hello-mysql/internal/lifecycle" "github.com/go-modkit/modkit/examples/hello-mysql/internal/modules/app" "github.com/go-modkit/modkit/examples/hello-mysql/internal/modules/auth" "github.com/go-modkit/modkit/examples/hello-mysql/internal/platform/config" @@ -21,7 +27,7 @@ func main() { cfg := config.Load() jwtTTL := parseJWTTTL(cfg.JWTTTL) - handler, err := httpserver.BuildHandler(buildAppOptions(cfg, jwtTTL)) + boot, handler, err := httpserver.BuildAppHandler(buildAppOptions(cfg, jwtTTL)) if err != nil { log.Fatalf("bootstrap failed: %v", err) } @@ -29,7 +35,25 @@ func main() { logger := logging.New() logStartup(logger, cfg.HTTPAddr) - if err := modkithttp.Serve(cfg.HTTPAddr, handler); err != nil { + server := &http.Server{ + Addr: cfg.HTTPAddr, + Handler: handler, + } + + sigCh := make(chan os.Signal, 1) + signal.Notify(sigCh, os.Interrupt, syscall.SIGTERM) + defer func() { + signal.Stop(sigCh) + close(sigCh) + }() + + errCh := make(chan error, 1) + go func() { + errCh <- server.ListenAndServe() + }() + + hooks := lifecycle.FromFuncs(boot.CleanupHooks()) + if err := runServer(modkithttp.ShutdownTimeout, server, sigCh, errCh, hooks); err != nil { log.Fatalf("server failed: %v", err) } } @@ -64,3 +88,32 @@ func parseJWTTTL(raw string) time.Duration { } return ttl } + +type shutdownServer interface { + ListenAndServe() error + Shutdown(context.Context) error +} + +func runServer(shutdownTimeout time.Duration, server shutdownServer, sigCh <-chan os.Signal, errCh <-chan error, hooks []lifecycle.CleanupHook) error { + select { + case err := <-errCh: + if err == http.ErrServerClosed { + return nil + } + return err + case <-sigCh: + ctx, cancel := context.WithTimeout(context.Background(), shutdownTimeout) + defer cancel() + + shutdownErr := lifecycle.ShutdownServer(ctx, server, hooks) + + err := <-errCh + if err == http.ErrServerClosed { + err = nil + } + if shutdownErr != nil { + return shutdownErr + } + return err + } +} diff --git a/examples/hello-mysql/cmd/api/main_test.go b/examples/hello-mysql/cmd/api/main_test.go index 46efc30..397b4e3 100644 --- a/examples/hello-mysql/cmd/api/main_test.go +++ b/examples/hello-mysql/cmd/api/main_test.go @@ -1,95 +1,133 @@ package main import ( + "context" + "errors" + "net/http" + "os" "testing" "time" + "github.com/go-modkit/modkit/examples/hello-mysql/internal/lifecycle" "github.com/go-modkit/modkit/examples/hello-mysql/internal/platform/config" ) -func TestParseJWTTTL_DefaultOnInvalid(t *testing.T) { - got := parseJWTTTL("bad-value") - if got != time.Hour { - t.Fatalf("ttl = %v", got) - } +type stubServer struct { + shutdownCalled bool + shutdownErr error + shutdownCh chan struct{} } -func TestParseJWTTTL_Valid(t *testing.T) { - got := parseJWTTTL("30m") - if got != 30*time.Minute { - t.Fatalf("ttl = %v", got) - } +func (s *stubServer) ListenAndServe() error { + return nil } -func TestParseJWTTTL_RejectsNonPositive(t *testing.T) { - for _, value := range []string{"0s", "-1s"} { - got := parseJWTTTL(value) - if got != time.Hour { - t.Fatalf("ttl for %q = %v", value, got) - } +func (s *stubServer) Shutdown(ctx context.Context) error { + s.shutdownCalled = true + if s.shutdownCh != nil { + close(s.shutdownCh) } + return s.shutdownErr } -func TestBuildAuthConfig(t *testing.T) { - cfg := config.Config{ - JWTSecret: "secret", - JWTIssuer: "issuer", - AuthUsername: "demo", - AuthPassword: "s3cret", +func TestRunServer_ShutdownPath(t *testing.T) { + server := &stubServer{} + sigCh := make(chan os.Signal, 1) + errCh := make(chan error, 1) + cleanupCalled := false + hooks := []lifecycle.CleanupHook{ + func(ctx context.Context) error { + cleanupCalled = true + return nil + }, } - ttl := 2 * time.Minute - got := buildAuthConfig(cfg, ttl) + server.shutdownCh = make(chan struct{}) + go func() { + <-server.shutdownCh + errCh <- http.ErrServerClosed + }() + sigCh <- os.Interrupt - if got.Secret != cfg.JWTSecret { - t.Fatalf("secret = %q", got.Secret) - } - if got.Issuer != cfg.JWTIssuer { - t.Fatalf("issuer = %q", got.Issuer) - } - if got.TTL != ttl { - t.Fatalf("ttl = %v", got.TTL) + err := runServer(50*time.Millisecond, server, sigCh, errCh, hooks) + if err != nil { + t.Fatalf("expected nil error, got %v", err) } - if got.Username != cfg.AuthUsername { - t.Fatalf("username = %q", got.Username) + if !server.shutdownCalled { + t.Fatal("expected shutdown to be called") } - if got.Password != cfg.AuthPassword { - t.Fatalf("password = %q", got.Password) + if !cleanupCalled { + t.Fatal("expected cleanup to be called") } } -func TestBuildAppOptions(t *testing.T) { - cfg := config.Config{ - HTTPAddr: ":9999", - MySQLDSN: "dsn", - JWTSecret: "secret", - JWTIssuer: "issuer", - AuthUsername: "demo", - AuthPassword: "s3cret", +func TestRunServer_ReturnsListenError(t *testing.T) { + server := &stubServer{} + sigCh := make(chan os.Signal, 1) + errCh := make(chan error, 1) + errCh <- errors.New("listen failed") + + err := runServer(50*time.Millisecond, server, sigCh, errCh, nil) + if err == nil || err.Error() != "listen failed" { + t.Fatalf("expected listen error, got %v", err) + } + if server.shutdownCalled { + t.Fatal("shutdown should not be called") } - ttl := 3 * time.Minute +} - opts := buildAppOptions(cfg, ttl) +func TestRunServer_ShutdownReturnsError(t *testing.T) { + server := &stubServer{shutdownErr: errors.New("shutdown failed"), shutdownCh: make(chan struct{})} + sigCh := make(chan os.Signal, 1) + errCh := make(chan error, 1) + go func() { + <-server.shutdownCh + errCh <- http.ErrServerClosed + }() + sigCh <- os.Interrupt - if opts.HTTPAddr != cfg.HTTPAddr { - t.Fatalf("http addr = %q", opts.HTTPAddr) + err := runServer(50*time.Millisecond, server, sigCh, errCh, nil) + if err == nil || err.Error() != "shutdown failed" { + t.Fatalf("expected shutdown error, got %v", err) } - if opts.MySQLDSN != cfg.MySQLDSN { - t.Fatalf("mysql dsn = %q", opts.MySQLDSN) - } - if opts.Auth.Secret != cfg.JWTSecret { - t.Fatalf("auth secret = %q", opts.Auth.Secret) +} + +func TestParseJWTTTL_InvalidFallsBack(t *testing.T) { + got := parseJWTTTL("nope") + if got != time.Hour { + t.Fatalf("expected 1h fallback, got %v", got) } - if opts.Auth.Issuer != cfg.JWTIssuer { - t.Fatalf("auth issuer = %q", opts.Auth.Issuer) +} + +func TestParseJWTTTL_NonPositiveFallsBack(t *testing.T) { + got := parseJWTTTL("0s") + if got != time.Hour { + t.Fatalf("expected 1h fallback, got %v", got) } - if opts.Auth.TTL != ttl { - t.Fatalf("auth ttl = %v", opts.Auth.TTL) +} + +func TestParseJWTTTL_Valid(t *testing.T) { + got := parseJWTTTL("2h") + if got != 2*time.Hour { + t.Fatalf("expected 2h, got %v", got) } - if opts.Auth.Username != cfg.AuthUsername { - t.Fatalf("auth username = %q", opts.Auth.Username) +} + +func TestBuildAuthConfig_MapsFields(t *testing.T) { + cfg := config.Config{JWTSecret: "s", JWTIssuer: "i", AuthUsername: "u", AuthPassword: "p"} + got := buildAuthConfig(cfg, 5*time.Minute) + if got.Secret != "s" || got.Issuer != "i" || got.Username != "u" || got.Password != "p" || got.TTL != 5*time.Minute { + t.Fatalf("unexpected auth config: %+v", got) } - if opts.Auth.Password != cfg.AuthPassword { - t.Fatalf("auth password = %q", opts.Auth.Password) +} + +func TestBuildAppOptions_MapsFields(t *testing.T) { + cfg := config.Config{HTTPAddr: ":1234", MySQLDSN: "dsn", JWTSecret: "s", JWTIssuer: "i", AuthUsername: "u", AuthPassword: "p"} + got := buildAppOptions(cfg, 10*time.Minute) + if got.HTTPAddr != ":1234" || got.MySQLDSN != "dsn" { + t.Fatalf("unexpected options: %+v", got) + } + if got.Auth.Secret != "s" || got.Auth.Issuer != "i" || got.Auth.Username != "u" || got.Auth.Password != "p" || got.Auth.TTL != 10*time.Minute { + t.Fatalf("unexpected auth: %+v", got.Auth) } } diff --git a/examples/hello-mysql/cmd/api/startup_test.go b/examples/hello-mysql/cmd/api/startup_test.go index ff3d404..864d431 100644 --- a/examples/hello-mysql/cmd/api/startup_test.go +++ b/examples/hello-mysql/cmd/api/startup_test.go @@ -71,3 +71,8 @@ func TestLogStartup_EmitsMessage(t *testing.T) { t.Fatalf("expected scope api, got %q", scope) } } + +func TestLogStartup_NilLogger(t *testing.T) { + // Ensure no panic when logger is nil. + logStartup(nil, ":9090") +} diff --git a/examples/hello-mysql/internal/httpserver/server.go b/examples/hello-mysql/internal/httpserver/server.go index b9f71d8..22a00dc 100644 --- a/examples/hello-mysql/internal/httpserver/server.go +++ b/examples/hello-mysql/internal/httpserver/server.go @@ -11,18 +11,20 @@ import ( httpSwagger "github.com/swaggo/http-swagger/v2" ) -func BuildHandler(opts app.Options) (http.Handler, error) { +var registerRoutes = modkithttp.RegisterRoutes + +func BuildAppHandler(opts app.Options) (*kernel.App, http.Handler, error) { mod := app.NewModule(opts) boot, err := kernel.Bootstrap(mod) if err != nil { - return nil, err + return nil, nil, err } logger := logging.New().With(slog.String("scope", "httpserver")) router := modkithttp.NewRouter() router.Use(modkithttp.RequestLogger(logger)) - if err := modkithttp.RegisterRoutes(modkithttp.AsRouter(router), boot.Controllers); err != nil { - return nil, err + if err := registerRoutes(modkithttp.AsRouter(router), boot.Controllers); err != nil { + return boot, nil, err } router.Get("/swagger/*", httpSwagger.WrapHandler) router.Get("/docs/*", httpSwagger.WrapHandler) @@ -30,5 +32,10 @@ func BuildHandler(opts app.Options) (http.Handler, error) { http.Redirect(w, r, "/docs/index.html", http.StatusMovedPermanently) })) - return router, nil + return boot, router, nil +} + +func BuildHandler(opts app.Options) (http.Handler, error) { + _, handler, err := BuildAppHandler(opts) + return handler, err } diff --git a/examples/hello-mysql/internal/httpserver/server_test.go b/examples/hello-mysql/internal/httpserver/server_test.go index 870c609..30dc94d 100644 --- a/examples/hello-mysql/internal/httpserver/server_test.go +++ b/examples/hello-mysql/internal/httpserver/server_test.go @@ -2,6 +2,7 @@ package httpserver import ( "bytes" + "errors" "io" "net/http" "net/http/httptest" @@ -11,6 +12,7 @@ import ( "github.com/go-modkit/modkit/examples/hello-mysql/internal/modules/app" "github.com/go-modkit/modkit/examples/hello-mysql/internal/modules/auth" + modkithttp "github.com/go-modkit/modkit/modkit/http" ) func TestBuildHandler_LogsRequest(t *testing.T) { @@ -25,17 +27,7 @@ func TestBuildHandler_LogsRequest(t *testing.T) { _ = r.Close() }() - h, err := BuildHandler(app.Options{ - HTTPAddr: ":8080", - MySQLDSN: "root:password@tcp(localhost:3306)/app?parseTime=true&multiStatements=true", - Auth: auth.Config{ - Secret: "dev-secret-change-me", - Issuer: "hello-mysql", - TTL: time.Hour, - Username: "demo", - Password: "demo", - }, - }) + h, err := BuildHandler(testAppOptions()) if err != nil { _ = w.Close() t.Fatalf("build handler: %v", err) @@ -55,3 +47,58 @@ func TestBuildHandler_LogsRequest(t *testing.T) { t.Fatalf("expected log output, got %s", string(output)) } } + +func TestBuildAppHandler_ReturnsAppAndHandler(t *testing.T) { + boot, handler, err := BuildAppHandler(testAppOptions()) + if err != nil { + t.Fatalf("build app handler: %v", err) + } + if boot == nil { + t.Fatal("expected app, got nil") + } + if len(boot.Controllers) == 0 { + t.Fatal("expected controllers to be registered") + } + if handler == nil { + t.Fatal("expected handler, got nil") + } + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/health", nil) + handler.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", rec.Code) + } +} + +func TestBuildAppHandler_ReturnsBootOnRouteError(t *testing.T) { + origRegister := registerRoutes + registerRoutes = func(_ modkithttp.Router, _ map[string]any) error { + return errors.New("routes failed") + } + defer func() { registerRoutes = origRegister }() + + boot, handler, err := BuildAppHandler(testAppOptions()) + if err == nil { + t.Fatal("expected error") + } + if boot == nil { + t.Fatal("expected boot to be returned on error") + } + if handler != nil { + t.Fatal("expected nil handler on error") + } +} + +func testAppOptions() app.Options { + return app.Options{ + HTTPAddr: ":8080", + MySQLDSN: "root:password@tcp(localhost:3306)/app?parseTime=true&multiStatements=true", + Auth: auth.Config{ + Secret: "dev-secret-change-me", + Issuer: "hello-mysql", + TTL: time.Hour, + Username: "demo", + Password: "demo", + }, + } +} diff --git a/examples/hello-mysql/internal/lifecycle/cleanup.go b/examples/hello-mysql/internal/lifecycle/cleanup.go new file mode 100644 index 0000000..0f8d525 --- /dev/null +++ b/examples/hello-mysql/internal/lifecycle/cleanup.go @@ -0,0 +1,55 @@ +package lifecycle + +import ( + "context" + "errors" +) + +// CleanupHook defines a shutdown cleanup function. +type CleanupHook func(ctx context.Context) error + +// RunCleanup executes hooks in LIFO order and returns any combined errors. +func RunCleanup(ctx context.Context, hooks []CleanupHook) error { + var joined error + for i := len(hooks) - 1; i >= 0; i-- { + if hooks[i] == nil { + continue + } + if err := hooks[i](ctx); err != nil { + joined = errors.Join(joined, err) + } + } + return joined +} + +// FromFuncs wraps raw cleanup functions into CleanupHook values. +func FromFuncs(funcs []func(context.Context) error) []CleanupHook { + if len(funcs) == 0 { + return nil + } + hooks := make([]CleanupHook, len(funcs)) + for i, fn := range funcs { + if fn == nil { + continue + } + hooks[i] = CleanupHook(fn) + } + return hooks +} + +type shutdowner interface { + Shutdown(ctx context.Context) error +} + +// ShutdownServer shuts down the server, then runs cleanup hooks. +func ShutdownServer(ctx context.Context, server shutdowner, hooks []CleanupHook) error { + shutdownErr := server.Shutdown(ctx) + cleanupErr := RunCleanup(ctx, hooks) + if shutdownErr != nil && cleanupErr != nil { + return errors.Join(shutdownErr, cleanupErr) + } + if shutdownErr != nil { + return shutdownErr + } + return cleanupErr +} diff --git a/examples/hello-mysql/internal/lifecycle/lifecycle_test.go b/examples/hello-mysql/internal/lifecycle/lifecycle_test.go new file mode 100644 index 0000000..d11dc01 --- /dev/null +++ b/examples/hello-mysql/internal/lifecycle/lifecycle_test.go @@ -0,0 +1,205 @@ +package lifecycle + +import ( + "context" + "errors" + "net" + "net/http" + "strings" + "testing" + "time" +) + +func TestShutdown_InvokesCleanupHooksInLIFO(t *testing.T) { + calls := make([]string, 0, 2) + hooks := []CleanupHook{ + func(ctx context.Context) error { + calls = append(calls, "first") + return nil + }, + func(ctx context.Context) error { + calls = append(calls, "second") + return nil + }, + } + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + if err := RunCleanup(ctx, hooks); err != nil { + t.Fatalf("cleanup failed: %v", err) + } + + if got, want := strings.Join(calls, ","), "second,first"; got != want { + t.Fatalf("expected %s, got %s", want, got) + } +} + +func TestRunCleanup_JoinsErrorsAndSkipsNil(t *testing.T) { + calls := make([]string, 0, 2) + errFirst := errors.New("first") + errSecond := errors.New("second") + hooks := []CleanupHook{ + nil, + func(ctx context.Context) error { + calls = append(calls, "first") + return errFirst + }, + func(ctx context.Context) error { + calls = append(calls, "second") + return errSecond + }, + } + + if err := RunCleanup(context.Background(), hooks); err == nil { + t.Fatal("expected error, got nil") + } else if !errors.Is(err, errFirst) || !errors.Is(err, errSecond) { + t.Fatalf("expected joined errors, got %v", err) + } + + if got, want := strings.Join(calls, ","), "second,first"; got != want { + t.Fatalf("expected %s, got %s", want, got) + } +} + +func TestFromFuncs_WrapsFuncsAndSkipsNil(t *testing.T) { + calls := make([]string, 0, 2) + fnFirst := func(ctx context.Context) error { + calls = append(calls, "first") + return nil + } + fnSecond := func(ctx context.Context) error { + calls = append(calls, "second") + return nil + } + + hooks := FromFuncs([]func(context.Context) error{fnFirst, nil, fnSecond}) + if len(hooks) != 3 { + t.Fatalf("expected 3 hooks, got %d", len(hooks)) + } + if hooks[0] == nil || hooks[2] == nil { + t.Fatal("expected non-nil hooks for non-nil funcs") + } + if hooks[1] != nil { + t.Fatal("expected nil hook for nil func") + } + + if err := RunCleanup(context.Background(), hooks); err != nil { + t.Fatalf("cleanup failed: %v", err) + } + + if got, want := strings.Join(calls, ","), "second,first"; got != want { + t.Fatalf("expected %s, got %s", want, got) + } +} + +func TestShutdown_WaitsForInFlightRequest(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + done := make(chan struct{}) + cleanupCalled := make(chan struct{}) + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + close(started) + <-release + w.WriteHeader(http.StatusOK) + close(done) + }) + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen failed: %v", err) + } + server := &http.Server{Handler: handler} + go func() { + _ = server.Serve(ln) + }() + + reqDone := make(chan struct{}) + go func() { + _, _ = http.Get("http://" + ln.Addr().String()) + close(reqDone) + }() + + <-started + + hooks := []CleanupHook{ + func(ctx context.Context) error { + close(cleanupCalled) + return nil + }, + } + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + shutdownDone := make(chan error, 1) + go func() { + shutdownDone <- ShutdownServer(ctx, server, hooks) + }() + + select { + case <-cleanupCalled: + t.Fatal("cleanup ran before in-flight request completed") + default: + } + + close(release) + <-done + <-reqDone + + select { + case <-cleanupCalled: + case <-time.After(time.Second): + t.Fatal("cleanup did not run after in-flight request completed") + } + + if err := <-shutdownDone; err != nil { + t.Fatalf("shutdown failed: %v", err) + } +} + +type stubServer struct { + err error + called bool +} + +func (s *stubServer) Shutdown(ctx context.Context) error { + s.called = true + return s.err +} + +func TestShutdownServer_ReturnsShutdownErrorAndRunsCleanup(t *testing.T) { + shutdownErr := errors.New("shutdown failed") + server := &stubServer{err: shutdownErr} + cleanupCalled := false + hooks := []CleanupHook{ + func(ctx context.Context) error { + cleanupCalled = true + return nil + }, + } + + if err := ShutdownServer(context.Background(), server, hooks); !errors.Is(err, shutdownErr) { + t.Fatalf("expected shutdown error, got %v", err) + } + if !server.called { + t.Fatal("expected shutdown to be called") + } + if !cleanupCalled { + t.Fatal("expected cleanup to run even when shutdown fails") + } +} + +func TestShutdownServer_ReturnsCleanupErrorWhenShutdownOk(t *testing.T) { + cleanupErr := errors.New("cleanup failed") + server := &stubServer{} + hooks := []CleanupHook{ + func(ctx context.Context) error { + return cleanupErr + }, + } + + if err := ShutdownServer(context.Background(), server, hooks); !errors.Is(err, cleanupErr) { + t.Fatalf("expected cleanup error, got %v", err) + } +} diff --git a/examples/hello-mysql/internal/modules/audit/module_test.go b/examples/hello-mysql/internal/modules/audit/module_test.go new file mode 100644 index 0000000..35023bd --- /dev/null +++ b/examples/hello-mysql/internal/modules/audit/module_test.go @@ -0,0 +1,99 @@ +package audit + +import ( + "context" + "errors" + "testing" + + "github.com/go-modkit/modkit/examples/hello-mysql/internal/modules/users" + "github.com/go-modkit/modkit/modkit/module" +) + +var errUsersServiceNotFound = errors.New("users service not found") + +type stubResolver struct { + values map[module.Token]any + errors map[module.Token]error +} + +func (r stubResolver) Get(token module.Token) (any, error) { + if err := r.errors[token]; err != nil { + return nil, err + } + if val, ok := r.values[token]; ok { + return val, nil + } + return nil, nil +} + +type stubUserService struct{} + +func (stubUserService) GetUser(ctx context.Context, id int64) (users.User, error) { + return users.User{}, nil +} +func (stubUserService) CreateUser(ctx context.Context, input users.CreateUserInput) (users.User, error) { + return users.User{}, nil +} +func (stubUserService) ListUsers(ctx context.Context) ([]users.User, error) { + return nil, nil +} +func (stubUserService) UpdateUser(ctx context.Context, id int64, input users.UpdateUserInput) (users.User, error) { + return users.User{}, nil +} +func (stubUserService) DeleteUser(ctx context.Context, id int64) error { + return nil +} +func (stubUserService) LongOperation(ctx context.Context) error { + return nil +} + +func TestAuditModule_Definition_WiresUsersImport(t *testing.T) { + usersMod := &users.Module{} + mod := NewModule(Options{Users: usersMod}) + def := mod.(*Module).Definition() + if def.Name != "audit" { + t.Fatalf("expected name audit, got %q", def.Name) + } + if len(def.Imports) != 1 { + t.Fatalf("expected 1 import, got %d", len(def.Imports)) + } + if def.Imports[0].Definition().Name != "users" { + t.Fatalf("expected users import, got %q", def.Imports[0].Definition().Name) + } +} + +func TestAuditModule_ProviderBuildInvokesUsersService(t *testing.T) { + mod := NewModule(Options{Users: &users.Module{}}) + def := mod.(*Module).Definition() + provider := def.Providers[0] + resolver := stubResolver{ + values: map[module.Token]any{ + users.TokenService: stubUserService{}, + }, + } + res, err := provider.Build(resolver) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, ok := res.(Service); !ok { + t.Fatalf("expected Service, got %T", res) + } +} + +func TestAuditModule_ProviderBuildError(t *testing.T) { + mod := NewModule(Options{Users: &users.Module{}}) + def := mod.(*Module).Definition() + provider := def.Providers[0] + + _, err := provider.Build(stubResolver{ + errors: map[module.Token]error{ + users.TokenService: errUsersServiceNotFound, + }, + }) + if err == nil { + t.Fatal("expected error for missing users service") + } + if !errors.Is(err, errUsersServiceNotFound) { + t.Fatalf("expected users service error, got %v", err) + } +} diff --git a/examples/hello-mysql/internal/modules/audit/service_test.go b/examples/hello-mysql/internal/modules/audit/service_test.go index 08f785e..021d1e5 100644 --- a/examples/hello-mysql/internal/modules/audit/service_test.go +++ b/examples/hello-mysql/internal/modules/audit/service_test.go @@ -31,6 +31,10 @@ func (s stubUsersService) DeleteUser(ctx context.Context, id int64) error { return nil } +func (s stubUsersService) LongOperation(ctx context.Context) error { + return nil +} + func TestAuditService_FormatsEntry(t *testing.T) { svc := NewService(stubUsersService{user: users.User{ID: 3, Name: "Jo", Email: "jo@example.com"}}) diff --git a/examples/hello-mysql/internal/modules/database/cleanup.go b/examples/hello-mysql/internal/modules/database/cleanup.go new file mode 100644 index 0000000..54400f6 --- /dev/null +++ b/examples/hello-mysql/internal/modules/database/cleanup.go @@ -0,0 +1,16 @@ +package database + +import ( + "context" + "database/sql" +) + +func CleanupDB(ctx context.Context, db *sql.DB) error { + if ctx.Err() != nil { + return ctx.Err() + } + if db == nil { + return nil + } + return db.Close() +} diff --git a/examples/hello-mysql/internal/modules/database/cleanup_test.go b/examples/hello-mysql/internal/modules/database/cleanup_test.go new file mode 100644 index 0000000..8598986 --- /dev/null +++ b/examples/hello-mysql/internal/modules/database/cleanup_test.go @@ -0,0 +1,37 @@ +package database + +import ( + "context" + "database/sql" + "errors" + "testing" +) + +func TestCleanupDB_ReturnsContextError(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := CleanupDB(ctx, nil) + if err == nil { + t.Fatal("expected error") + } + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected context.Canceled, got %v", err) + } +} + +func TestCleanupDB_AllowsNilDB(t *testing.T) { + err := CleanupDB(context.Background(), nil) + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } +} + +func TestCleanupDB_ReturnsContextErrorBeforeClose(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + if err := CleanupDB(ctx, &sql.DB{}); !errors.Is(err, context.Canceled) { + t.Fatalf("expected context.Canceled, got %v", err) + } +} diff --git a/examples/hello-mysql/internal/modules/database/module.go b/examples/hello-mysql/internal/modules/database/module.go index 4e8f921..a2beb88 100644 --- a/examples/hello-mysql/internal/modules/database/module.go +++ b/examples/hello-mysql/internal/modules/database/module.go @@ -1,6 +1,9 @@ package database import ( + "context" + "database/sql" + "github.com/go-modkit/modkit/examples/hello-mysql/internal/platform/mysql" "github.com/go-modkit/modkit/modkit/module" ) @@ -22,13 +25,22 @@ func NewModule(opts Options) module.Module { } func (m Module) Definition() module.ModuleDef { + var db *sql.DB return module.ModuleDef{ Name: "database", Providers: []module.ProviderDef{ { Token: TokenDB, Build: func(r module.Resolver) (any, error) { - return mysql.Open(m.opts.DSN) + var err error + db, err = mysql.Open(m.opts.DSN) + if err != nil { + return nil, err + } + return db, nil + }, + Cleanup: func(ctx context.Context) error { + return CleanupDB(ctx, db) }, }, }, diff --git a/examples/hello-mysql/internal/modules/database/module_test.go b/examples/hello-mysql/internal/modules/database/module_test.go new file mode 100644 index 0000000..08b7e84 --- /dev/null +++ b/examples/hello-mysql/internal/modules/database/module_test.go @@ -0,0 +1,61 @@ +package database + +import ( + "context" + "errors" + "testing" +) + +func TestModuleDefinition_ProviderCleanupHook_CanceledContext(t *testing.T) { + def := Module{}.Definition() + if len(def.Providers) == 0 { + t.Fatal("expected at least one provider") + } + var cleanup func(ctx context.Context) error + for _, provider := range def.Providers { + if provider.Token == TokenDB { + cleanup = provider.Cleanup + break + } + } + if cleanup == nil { + t.Fatal("expected provider cleanup hook") + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if err := cleanup(ctx); !errors.Is(err, context.Canceled) { + t.Fatalf("expected context.Canceled, got %v", err) + } +} + +func TestDatabaseModule_Definition_ProvidesDB(t *testing.T) { + mod := NewModule(Options{DSN: "dsn"}) + def := mod.(*Module).Definition() + if def.Name != "database" { + t.Fatalf("expected name database, got %q", def.Name) + } + if len(def.Providers) != 1 { + t.Fatalf("expected 1 provider, got %d", len(def.Providers)) + } + if def.Providers[0].Token != TokenDB { + t.Fatalf("expected TokenDB, got %q", def.Providers[0].Token) + } + if def.Providers[0].Cleanup == nil { + t.Fatal("expected cleanup hook") + } +} + +func TestDatabaseModule_ProviderBuildError(t *testing.T) { + mod := NewModule(Options{DSN: ""}) + def := mod.(*Module).Definition() + provider := def.Providers[0] + + // Use a stub resolver - the error will come from mysql.Open with empty DSN + _, err := provider.Build(nil) + if err == nil { + t.Fatal("expected error for empty DSN") + } + if err.Error() != "mysql dsn is required" { + t.Fatalf("expected 'mysql dsn is required' error, got %q", err.Error()) + } +} diff --git a/examples/hello-mysql/internal/modules/users/controller_test.go b/examples/hello-mysql/internal/modules/users/controller_test.go index c0497a8..3b43364 100644 --- a/examples/hello-mysql/internal/modules/users/controller_test.go +++ b/examples/hello-mysql/internal/modules/users/controller_test.go @@ -49,6 +49,10 @@ func (s stubService) DeleteUser(ctx context.Context, id int64) error { return s.deleteFn(ctx, id) } +func (s stubService) LongOperation(ctx context.Context) error { + return nil +} + func TestController_CreateUser(t *testing.T) { svc := stubService{ createFn: func(ctx context.Context, input CreateUserInput) (User, error) { @@ -370,7 +374,9 @@ func TestController_UpdateUser_InternalError(t *testing.T) { svc := stubService{ createFn: func(ctx context.Context, input CreateUserInput) (User, error) { return User{}, nil }, listFn: func(ctx context.Context) ([]User, error) { return nil, nil }, - updateFn: func(ctx context.Context, id int64, input UpdateUserInput) (User, error) { return User{}, errors.New("boom") }, + updateFn: func(ctx context.Context, id int64, input UpdateUserInput) (User, error) { + return User{}, errors.New("boom") + }, deleteFn: func(ctx context.Context, id int64) error { return nil }, } diff --git a/examples/hello-mysql/internal/modules/users/module_test.go b/examples/hello-mysql/internal/modules/users/module_test.go index dfa1056..0c9ea96 100644 --- a/examples/hello-mysql/internal/modules/users/module_test.go +++ b/examples/hello-mysql/internal/modules/users/module_test.go @@ -60,6 +60,10 @@ func (serviceStub) DeleteUser(ctx context.Context, id int64) error { return nil } +func (serviceStub) LongOperation(ctx context.Context) error { + return nil +} + func TestUsersModule_ControllerBuildErrors(t *testing.T) { mod := NewModule(Options{Database: &database.Module{}, Auth: auth.NewModule(auth.Options{})}) def := mod.(*Module).Definition() @@ -96,3 +100,39 @@ func TestUsersModule_ControllerBuildErrors(t *testing.T) { t.Fatalf("unexpected error: %v", err) } } + +func TestUsersModule_RepositoryBuildError(t *testing.T) { + mod := NewModule(Options{Database: &database.Module{}, Auth: auth.NewModule(auth.Options{})}) + def := mod.(*Module).Definition() + provider := def.Providers[0] // TokenRepository + + _, err := provider.Build(stubResolver{ + errors: map[module.Token]error{ + database.TokenDB: errors.New("database connection failed"), + }, + }) + if err == nil { + t.Fatal("expected error for missing database") + } + if err.Error() != "database connection failed" { + t.Fatalf("expected 'database connection failed' error, got %q", err.Error()) + } +} + +func TestUsersModule_ServiceBuildError(t *testing.T) { + mod := NewModule(Options{Database: &database.Module{}, Auth: auth.NewModule(auth.Options{})}) + def := mod.(*Module).Definition() + provider := def.Providers[1] // TokenService + + _, err := provider.Build(stubResolver{ + errors: map[module.Token]error{ + TokenRepository: errors.New("repository not found"), + }, + }) + if err == nil { + t.Fatal("expected error for missing repository") + } + if err.Error() != "repository not found" { + t.Fatalf("expected 'repository not found' error, got %q", err.Error()) + } +} diff --git a/examples/hello-mysql/internal/modules/users/service.go b/examples/hello-mysql/internal/modules/users/service.go index 4922df5..182e512 100644 --- a/examples/hello-mysql/internal/modules/users/service.go +++ b/examples/hello-mysql/internal/modules/users/service.go @@ -3,6 +3,7 @@ package users import ( "context" "log/slog" + "time" modkitlogging "github.com/go-modkit/modkit/modkit/logging" ) @@ -13,11 +14,13 @@ type Service interface { ListUsers(ctx context.Context) ([]User, error) UpdateUser(ctx context.Context, id int64, input UpdateUserInput) (User, error) DeleteUser(ctx context.Context, id int64) error + LongOperation(ctx context.Context) error } type service struct { - repo Repository - logger modkitlogging.Logger + repo Repository + logger modkitlogging.Logger + longOperationDelay time.Duration } func NewService(repo Repository, logger modkitlogging.Logger) Service { @@ -25,7 +28,11 @@ func NewService(repo Repository, logger modkitlogging.Logger) Service { logger = modkitlogging.NewNopLogger() } logger = logger.With(slog.String("scope", "users")) - return &service{repo: repo, logger: logger} + return &service{ + repo: repo, + logger: logger, + longOperationDelay: 2 * time.Second, + } } func (s *service) GetUser(ctx context.Context, id int64) (User, error) { @@ -52,3 +59,13 @@ func (s *service) DeleteUser(ctx context.Context, id int64) error { s.logger.Debug("delete user", slog.Int64("id", id)) return s.repo.DeleteUser(ctx, id) } + +func (s *service) LongOperation(ctx context.Context) error { + s.logger.Debug("long operation") + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(s.longOperationDelay): + return nil + } +} diff --git a/examples/hello-mysql/internal/modules/users/service_test.go b/examples/hello-mysql/internal/modules/users/service_test.go index e1ab0ad..44960c0 100644 --- a/examples/hello-mysql/internal/modules/users/service_test.go +++ b/examples/hello-mysql/internal/modules/users/service_test.go @@ -2,7 +2,9 @@ package users import ( "context" + "errors" "testing" + "time" ) type stubRepo struct { @@ -99,3 +101,28 @@ func TestService_DeleteUser(t *testing.T) { t.Fatalf("expected delete id 9, got %d", repo.deleteID) } } + +func TestService_LongOperation_RespectsContextCancel(t *testing.T) { + svc := NewService(&stubRepo{}, nil) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := svc.LongOperation(ctx) + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected context.Canceled, got %v", err) + } +} + +func TestService_LongOperation_Completes(t *testing.T) { + svc := NewService(&stubRepo{}, nil).(*service) + origDelay := svc.longOperationDelay + svc.longOperationDelay = 2 * time.Millisecond + t.Cleanup(func() { svc.longOperationDelay = origDelay }) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + if err := svc.LongOperation(ctx); err != nil { + t.Fatalf("expected nil error, got %v", err) + } +} diff --git a/modkit/kernel/bootstrap.go b/modkit/kernel/bootstrap.go index 7d38443..cfd19f9 100644 --- a/modkit/kernel/bootstrap.go +++ b/modkit/kernel/bootstrap.go @@ -1,6 +1,10 @@ package kernel -import "github.com/go-modkit/modkit/modkit/module" +import ( + "context" + + "github.com/go-modkit/modkit/modkit/module" +) type App struct { Graph *Graph @@ -64,3 +68,8 @@ func (a *App) Resolver() module.Resolver { func (a *App) Get(token module.Token) (any, error) { return a.Resolver().Get(token) } + +// CleanupHooks returns provider cleanup hooks in LIFO order. +func (a *App) CleanupHooks() []func(context.Context) error { + return a.container.cleanupHooksLIFO() +} diff --git a/modkit/kernel/bootstrap_test.go b/modkit/kernel/bootstrap_test.go index 51f093e..574de2b 100644 --- a/modkit/kernel/bootstrap_test.go +++ b/modkit/kernel/bootstrap_test.go @@ -1,6 +1,7 @@ package kernel_test import ( + "context" "errors" "testing" @@ -176,6 +177,67 @@ func TestBootstrapRejectsDuplicateControllerNames(t *testing.T) { } } +func TestBootstrap_CollectsCleanupHooksInLIFO(t *testing.T) { + tokenB := module.Token("test.tokenB") + tokenA := module.Token("test.tokenA") + calls := make([]string, 0, 2) + + modA := mod("A", nil, + []module.ProviderDef{{ + Token: tokenB, + Build: func(r module.Resolver) (any, error) { + return "b", nil + }, + Cleanup: func(ctx context.Context) error { + calls = append(calls, "B") + return nil + }, + }, { + Token: tokenA, + Build: func(r module.Resolver) (any, error) { + _, err := r.Get(tokenB) + if err != nil { + return nil, err + } + return "a", nil + }, + Cleanup: func(ctx context.Context) error { + calls = append(calls, "A") + return nil + }, + }}, + nil, + nil, + ) + + app, err := kernel.Bootstrap(modA) + if err != nil { + t.Fatalf("Bootstrap failed: %v", err) + } + + if _, err := app.Get(tokenA); err != nil { + t.Fatalf("Get failed: %v", err) + } + + hooks := app.CleanupHooks() + if len(hooks) != 2 { + t.Fatalf("expected 2 cleanup hooks, got %d", len(hooks)) + } + + for _, hook := range hooks { + if err := hook(context.Background()); err != nil { + t.Fatalf("cleanup failed: %v", err) + } + } + + if len(calls) != 2 { + t.Fatalf("expected 2 cleanup calls, got %d", len(calls)) + } + if calls[0] != "A" || calls[1] != "B" { + t.Fatalf("unexpected cleanup order: %v", calls) + } +} + func TestBootstrapRegistersControllers(t *testing.T) { modA := mod("A", nil, nil, []module.ControllerDef{{ diff --git a/modkit/kernel/container.go b/modkit/kernel/container.go index 6c9e86e..ea6603d 100644 --- a/modkit/kernel/container.go +++ b/modkit/kernel/container.go @@ -1,6 +1,7 @@ package kernel import ( + "context" "sync" "github.com/go-modkit/modkit/modkit/module" @@ -9,15 +10,17 @@ import ( type providerEntry struct { moduleName string build func(r module.Resolver) (any, error) + cleanup func(ctx context.Context) error } type Container struct { - providers map[module.Token]providerEntry - instances map[module.Token]any - visibility Visibility - locks map[module.Token]*sync.Mutex - waitingOn map[module.Token]module.Token - mu sync.Mutex + providers map[module.Token]providerEntry + instances map[module.Token]any + visibility Visibility + locks map[module.Token]*sync.Mutex + waitingOn map[module.Token]module.Token + cleanupHooks []func(context.Context) error + mu sync.Mutex } func newContainer(graph *Graph, visibility Visibility) (*Container, error) { @@ -33,16 +36,18 @@ func newContainer(graph *Graph, visibility Visibility) (*Container, error) { providers[provider.Token] = providerEntry{ moduleName: node.Name, build: provider.Build, + cleanup: provider.Cleanup, } } } return &Container{ - providers: providers, - instances: make(map[module.Token]any), - visibility: visibility, - locks: make(map[module.Token]*sync.Mutex), - waitingOn: make(map[module.Token]module.Token), + providers: providers, + instances: make(map[module.Token]any), + visibility: visibility, + locks: make(map[module.Token]*sync.Mutex), + waitingOn: make(map[module.Token]module.Token), + cleanupHooks: make([]func(context.Context) error, 0), }, nil } @@ -102,10 +107,24 @@ func (c *Container) getWithStack(token module.Token, requester string, stack []m c.mu.Lock() c.instances[token] = instance + if entry.cleanup != nil { + c.cleanupHooks = append(c.cleanupHooks, entry.cleanup) + } c.mu.Unlock() return instance, nil } +func (c *Container) cleanupHooksLIFO() []func(context.Context) error { + c.mu.Lock() + defer c.mu.Unlock() + + hooks := make([]func(context.Context) error, len(c.cleanupHooks)) + for i, hook := range c.cleanupHooks { + hooks[len(c.cleanupHooks)-1-i] = hook + } + return hooks +} + type moduleResolver struct { container *Container moduleName string diff --git a/modkit/kernel/container_test.go b/modkit/kernel/container_test.go index b6d89bf..b6fbb72 100644 --- a/modkit/kernel/container_test.go +++ b/modkit/kernel/container_test.go @@ -1,6 +1,7 @@ package kernel_test import ( + "context" "errors" "sync" "sync/atomic" @@ -199,3 +200,182 @@ func TestContainerDetectsConcurrentMutualCycle(t *testing.T) { } } } + +// TestContainerGetWrapsProviderBuildError verifies that Container.Get wraps +// provider build errors in ProviderBuildError and preserves the original error. +// This tests the error wrapping path when a provider's build function fails. +func TestContainerGetWrapsProviderBuildError(t *testing.T) { + badToken := module.Token("bad") + sentinel := errors.New("build failed sentinel") + + modA := mod("A", nil, + []module.ProviderDef{{ + Token: badToken, + Build: func(r module.Resolver) (any, error) { + return nil, sentinel + }, + }}, + nil, + nil, + ) + + app, err := kernel.Bootstrap(modA) + if err != nil { + t.Fatalf("Bootstrap failed: %v", err) + } + + // Trigger the build error by requesting the token + _, err = app.Get(badToken) + if err == nil { + t.Fatalf("expected error for build failure") + } + + var buildErr *kernel.ProviderBuildError + if !errors.As(err, &buildErr) { + t.Fatalf("unexpected error type: %T, wanted ProviderBuildError", err) + } + + if buildErr.Token != badToken { + t.Fatalf("expected Token %q, got %q", badToken, buildErr.Token) + } + if buildErr.Module != "A" { + t.Fatalf("expected Module %q, got %q", "A", buildErr.Module) + } + + // Verify the original error is preserved in the error chain + if !errors.Is(err, sentinel) { + t.Fatalf("expected error chain to include sentinel, got: %v", err) + } +} + +// TestContainerGetMissingTokenError verifies that Container.Get reports errors +// correctly when a requested token is not found. We test this by creating a +// non-root module and bypassing it via module imports. +func TestContainerGetMissingTokenError(t *testing.T) { + modB := mod("B", nil, nil, nil, nil) + modA := mod("A", []module.Module{modB}, nil, nil, nil) + + app, err := kernel.Bootstrap(modA) + if err != nil { + t.Fatalf("Bootstrap failed: %v", err) + } + + missingToken := module.Token("missing") + // Try to get a missing token from a non-root module context + // This should trigger TokenNotVisibleError for a token outside visibility + resolver := app.Resolver() + _, err = resolver.Get(missingToken) + if err == nil { + t.Fatalf("expected error for missing token") + } + + var notVisible *kernel.TokenNotVisibleError + if !errors.As(err, ¬Visible) { + t.Fatalf("expected TokenNotVisibleError, got %T", err) + } + if notVisible.Module != "A" || notVisible.Token != missingToken { + t.Fatalf("unexpected error fields: %+v", notVisible) + } +} + +// TestContainerGetSingletonBehavior verifies that Container.Get caches instances +// and reuses them on subsequent calls, demonstrating singleton semantics. +func TestContainerGetSingletonBehavior(t *testing.T) { + token := module.Token("cached") + var buildCount int32 + type cachedInstance struct{} + + modA := mod("A", nil, + []module.ProviderDef{{ + Token: token, + Build: func(r module.Resolver) (any, error) { + buildCount++ + return &cachedInstance{}, nil + }, + }}, + nil, + nil, + ) + + app, err := kernel.Bootstrap(modA) + if err != nil { + t.Fatalf("Bootstrap failed: %v", err) + } + + // First call should build + val1, err := app.Get(token) + if err != nil { + t.Fatalf("first Get failed: %v", err) + } + instance1, ok := val1.(*cachedInstance) + if !ok { + t.Fatalf("expected *cachedInstance, got %T", val1) + } + if buildCount != 1 { + t.Fatalf("expected 1 build call, got %d", buildCount) + } + + // Second call should use cache + val2, err := app.Get(token) + if err != nil { + t.Fatalf("second Get failed: %v", err) + } + instance2, ok := val2.(*cachedInstance) + if !ok { + t.Fatalf("expected *cachedInstance, got %T", val2) + } + if buildCount != 1 { + t.Fatalf("expected still 1 build call (cached), got %d", buildCount) + } + + // Both should be the same instance + if instance1 != instance2 { + t.Fatalf("expected same cached instance") + } +} + +// TestContainerGetRegistersCleanupHooks verifies that Container.Get registers +// cleanup hooks when a provider has a cleanup function. +func TestContainerGetRegistersCleanupHooks(t *testing.T) { + token := module.Token("with.cleanup") + var cleanupCalled bool + + modA := mod("A", nil, + []module.ProviderDef{{ + Token: token, + Build: func(r module.Resolver) (any, error) { + return "instance", nil + }, + Cleanup: func(ctx context.Context) error { + cleanupCalled = true + return nil + }, + }}, + nil, + nil, + ) + + app, err := kernel.Bootstrap(modA) + if err != nil { + t.Fatalf("Bootstrap failed: %v", err) + } + + _, err = app.Get(token) + if err != nil { + t.Fatalf("Get failed: %v", err) + } + + hooks := app.CleanupHooks() + if len(hooks) != 1 { + t.Fatalf("expected 1 cleanup hook, got %d", len(hooks)) + } + + err = hooks[0](context.Background()) + if err != nil { + t.Fatalf("cleanup failed: %v", err) + } + + if !cleanupCalled { + t.Fatalf("expected cleanup to be called") + } +} diff --git a/modkit/kernel/errors_test.go b/modkit/kernel/errors_test.go new file mode 100644 index 0000000..a1247aa --- /dev/null +++ b/modkit/kernel/errors_test.go @@ -0,0 +1,49 @@ +package kernel + +import ( + "errors" + "testing" +) + +func TestKernelErrorStrings(t *testing.T) { + tests := []struct { + name string + err error + }{ + {"RootModuleNil", &RootModuleNilError{}}, + {"InvalidModuleName", &InvalidModuleNameError{Name: "mod"}}, + {"ModuleNotPointer", &ModuleNotPointerError{Module: "mod"}}, + {"InvalidModuleDef", &InvalidModuleDefError{Module: "mod", Reason: "bad"}}, + {"NilImport", &NilImportError{Module: "mod", Index: 1}}, + {"DuplicateModuleName", &DuplicateModuleNameError{Name: "mod"}}, + {"ModuleCycle", &ModuleCycleError{Path: []string{"a", "b"}}}, + {"DuplicateProviderToken", &DuplicateProviderTokenError{Token: "t", Modules: []string{"a", "b"}}}, + {"DuplicateControllerName", &DuplicateControllerNameError{Module: "mod", Name: "ctrl"}}, + {"TokenNotVisible", &TokenNotVisibleError{Module: "mod", Token: "t"}}, + {"ExportNotVisible", &ExportNotVisibleError{Module: "mod", Token: "t"}}, + {"ProviderNotFound", &ProviderNotFoundError{Module: "mod", Token: "t"}}, + {"ProviderCycle", &ProviderCycleError{Token: "t"}}, + {"ProviderBuild", &ProviderBuildError{Module: "mod", Token: "t", Err: errors.New("boom")}}, + {"ControllerBuild", &ControllerBuildError{Module: "mod", Controller: "c", Err: errors.New("boom")}}, + } + for _, tc := range tests { + if tc.err == nil { + t.Fatalf("%s produced nil error", tc.name) + } + if tc.err.Error() == "" { + t.Fatalf("%s produced empty error string", tc.name) + } + } +} + +func TestErrorWraps(t *testing.T) { + inner := errors.New("inner") + err := &ProviderBuildError{Module: "m", Token: "t", Err: inner} + if !errors.Is(err.Unwrap(), inner) { + t.Fatalf("expected unwrap to return inner error, got %v", err.Unwrap()) + } + err2 := &ControllerBuildError{Module: "m", Controller: "c", Err: inner} + if !errors.Is(err2.Unwrap(), inner) { + t.Fatalf("expected unwrap to return inner error, got %v", err2.Unwrap()) + } +} diff --git a/modkit/kernel/graph_test.go b/modkit/kernel/graph_test.go index 2622e20..e522e9e 100644 --- a/modkit/kernel/graph_test.go +++ b/modkit/kernel/graph_test.go @@ -8,40 +8,6 @@ import ( "github.com/go-modkit/modkit/modkit/module" ) -type testModule struct { - def module.ModuleDef -} - -func (m *testModule) Definition() module.ModuleDef { - return m.def -} - -type valueModule struct { - def module.ModuleDef -} - -func (m valueModule) Definition() module.ModuleDef { - return m.def -} - -func mod( - name string, - imports []module.Module, - providers []module.ProviderDef, - controllers []module.ControllerDef, - exports []module.Token, -) module.Module { - return &testModule{ - def: module.ModuleDef{ - Name: name, - Imports: imports, - Providers: providers, - Controllers: controllers, - Exports: exports, - }, - } -} - func TestBuildGraphRejectsNilRoot(t *testing.T) { _, err := kernel.BuildGraph(nil) if err == nil { @@ -308,7 +274,7 @@ func TestBuildGraphRejectsCycles(t *testing.T) { modA := mod("A", nil, nil, nil, nil) modB := mod("B", []module.Module{modA}, nil, nil, nil) - root := modA.(*testModule) + root := modA.(*modHelper) root.def.Imports = []module.Module{modB} _, err := kernel.BuildGraph(modA) diff --git a/modkit/kernel/mod_helper_test.go b/modkit/kernel/mod_helper_test.go new file mode 100644 index 0000000..ab4d836 --- /dev/null +++ b/modkit/kernel/mod_helper_test.go @@ -0,0 +1,45 @@ +package kernel_test + +import "github.com/go-modkit/modkit/modkit/module" + +type testModule struct { + def module.ModuleDef +} + +func (m *testModule) Definition() module.ModuleDef { + return m.def +} + +type valueModule struct { + def module.ModuleDef +} + +func (m valueModule) Definition() module.ModuleDef { + return m.def +} + +type modHelper struct { + def module.ModuleDef +} + +func (m *modHelper) Definition() module.ModuleDef { + return m.def +} + +func mod( + name string, + imports []module.Module, + providers []module.ProviderDef, + controllers []module.ControllerDef, + exports []module.Token, +) module.Module { + return &modHelper{ + def: module.ModuleDef{ + Name: name, + Imports: imports, + Providers: providers, + Controllers: controllers, + Exports: exports, + }, + } +} diff --git a/modkit/module/provider.go b/modkit/module/provider.go index d1a2f03..b334922 100644 --- a/modkit/module/provider.go +++ b/modkit/module/provider.go @@ -1,7 +1,10 @@ package module +import "context" + // ProviderDef describes how to build a provider for a token. type ProviderDef struct { - Token Token - Build func(r Resolver) (any, error) + Token Token + Build func(r Resolver) (any, error) + Cleanup func(ctx context.Context) error }