From ddd7c1f6065810c8bbdb86042bfe01923d63c554 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sun, 26 Apr 2026 16:41:50 +0300 Subject: [PATCH] savedsearch: add /api/v1/saved-searches CRUD with 0002 migration --- cmd/lethe/main.go | 14 +- internal/domain/savedsearch/handler.go | 236 +++++++++++++ internal/domain/savedsearch/handler_test.go | 332 ++++++++++++++++++ internal/domain/savedsearch/repository.go | 198 +++++++++++ .../domain/savedsearch/repository_test.go | 321 +++++++++++++++++ .../migrations/0002_saved_searches.down.sql | 2 + .../migrations/0002_saved_searches.up.sql | 10 + internal/server/server.go | 13 +- 8 files changed, 1117 insertions(+), 9 deletions(-) create mode 100644 internal/domain/savedsearch/handler.go create mode 100644 internal/domain/savedsearch/handler_test.go create mode 100644 internal/domain/savedsearch/repository.go create mode 100644 internal/domain/savedsearch/repository_test.go create mode 100644 internal/platform/database/migrations/0002_saved_searches.down.sql create mode 100644 internal/platform/database/migrations/0002_saved_searches.up.sql diff --git a/cmd/lethe/main.go b/cmd/lethe/main.go index 7c0823d0313139a7783fb052050df4710e5be6d7..424d041b55f357fdba29131b5b768a9ce678ac7d 100644 --- a/cmd/lethe/main.go +++ b/cmd/lethe/main.go @@ -26,6 +26,7 @@ import ( "sourcecraft.dev/bigbes/lethe/internal/config" "sourcecraft.dev/bigbes/lethe/internal/domain/ingest" "sourcecraft.dev/bigbes/lethe/internal/domain/project" + "sourcecraft.dev/bigbes/lethe/internal/domain/savedsearch" "sourcecraft.dev/bigbes/lethe/internal/domain/session" "sourcecraft.dev/bigbes/lethe/internal/domain/stats" "sourcecraft.dev/bigbes/lethe/internal/platform/database" @@ -100,15 +101,18 @@ func run() int { sessionHnd = &session.Handler{} projectRepo = &project.Repository{} projectHnd = &project.Handler{} - statsRepo = &stats.Repository{} - statsHnd = &stats.Handler{} - serverSvc = &server.Server{} + statsRepo = &stats.Repository{} + statsHnd = &stats.Handler{} + savedSearchRepo = &savedsearch.Repository{} + savedSearchHnd = &savedsearch.Handler{} + serverSvc = &server.Server{} ) registered := []any{ loggerSvc, metricsSvc, dbSvc, dbCheckSvc, healthSetSvc, authSvc, ingestRepo, ingestSvc, ingestHnd, - sessionRepo, sessionHnd, projectRepo, projectHnd, statsRepo, statsHnd, serverSvc, + sessionRepo, sessionHnd, projectRepo, projectHnd, statsRepo, statsHnd, + savedSearchRepo, savedSearchHnd, serverSvc, } mgr.AddComponent(ctx, @@ -128,6 +132,8 @@ func run() int { steward.MustServiceAsset(projectHnd), steward.MustServiceAsset(statsRepo), steward.MustServiceAsset(statsHnd), + steward.MustServiceAsset(savedSearchRepo), + steward.MustServiceAsset(savedSearchHnd), steward.MustServiceAsset(serverSvc, steward.Root()), ) diff --git a/internal/domain/savedsearch/handler.go b/internal/domain/savedsearch/handler.go new file mode 100644 index 0000000000000000000000000000000000000000..513d181aae0107237a64dc6a3a6f3d2af9c881c8 --- /dev/null +++ b/internal/domain/savedsearch/handler.go @@ -0,0 +1,236 @@ +package savedsearch + +import ( + "context" + "encoding/json" + "log/slog" + "net/http" + "strings" + "time" + + "github.com/go-chi/chi/v5" + "go.bigb.es/auxilia/culpa" + "go.bigb.es/auxilia/scribe" + + "sourcecraft.dev/bigbes/lethe/internal/pkg/apierror" + "sourcecraft.dev/bigbes/lethe/internal/pkg/httputil" + "sourcecraft.dev/bigbes/lethe/internal/server/auth" +) + +// Handler is the steward-managed HTTP boundary for the saved-searches CRUD API. +// Repo is the injected SQL steward; the Handler holds no other state. +type Handler struct { + Repo *Repository `inject:""` +} + +// Init satisfies the steward Initer contract. The Handler is stateless beyond +// its injected dependencies. +func (h *Handler) Init(_ context.Context) error { return nil } + +// Mount registers the four CRUD routes under r. Server.Init mounts this inside +// the /api/v1 group, so the effective paths are: +// +// GET /api/v1/saved-searches +// POST /api/v1/saved-searches +// PUT /api/v1/saved-searches/{name} +// DELETE /api/v1/saved-searches/{name} +func (h *Handler) Mount(r chi.Router) { + r.Get("/saved-searches", h.List) + r.Post("/saved-searches", h.Create) + r.Put("/saved-searches/{name}", h.Update) + r.Delete("/saved-searches/{name}", h.Delete) +} + +// ownerOf derives the owner from the authenticated identity on the context. +// The ?owner= query parameter is intentionally ignored (IV2). +func (h *Handler) ownerOf(r *http.Request) string { + return auth.MustIdentity(r.Context()).User +} + +// validateName enforces the name constraints (IV7, UK1): +// - non-empty +// - at most 64 characters +// - no "/" character +func validateName(name string) error { + if name == "" { + return culpa.WithCode( + culpa.WithPublic(culpa.New("name is empty"), "name must not be empty"), + "VALIDATION", + ) + } + if len(name) > 64 { + return culpa.WithCode( + culpa.WithPublic(culpa.New("name too long"), "name must be 64 characters or fewer"), + "VALIDATION", + ) + } + if strings.Contains(name, "/") { + return culpa.WithCode( + culpa.WithPublic(culpa.New("name contains slash"), "name must not contain '/'"), + "VALIDATION", + ) + } + return nil +} + +// validateQuery enforces the query constraints: non-empty. +func validateQuery(query string) error { + if query == "" { + return culpa.WithCode( + culpa.WithPublic(culpa.New("query is empty"), "query must not be empty"), + "VALIDATION", + ) + } + return nil +} + +// listResponse is the JSON body returned by List. +type listResponse struct { + SavedSearches []SavedSearch `json:"saved_searches"` +} + +// List handles GET /saved-searches. The owner is derived from the auth context; +// ?owner= is silently ignored (IV2). Writes { saved_searches: [...] }. +func (h *Handler) List(w http.ResponseWriter, r *http.Request) { + owner := h.ownerOf(r) + rows, err := h.Repo.List(r.Context(), owner) + if err != nil { + apierror.Render(w, r, err) + return + } + if writeErr := httputil.WriteJSON(w, http.StatusOK, listResponse{SavedSearches: rows}); writeErr != nil { + slog.Default().ErrorContext(r.Context(), "write saved-searches list response", scribe.Err(writeErr)) + } +} + +// createRequest is the decoded JSON body for POST /saved-searches. +type createRequest struct { + Name string `json:"name"` + Query string `json:"query"` +} + +// Create handles POST /saved-searches. Decodes { name, query }, validates both +// fields, inserts the row with now as both timestamps, and returns 201 Created +// with the new row body. +func (h *Handler) Create(w http.ResponseWriter, r *http.Request) { + // Defensive: reject any explicit ?owner= param to prevent confusion (IV2). + if r.URL.Query().Get("owner") != "" { + apierror.Render(w, r, culpa.WithCode( + culpa.WithPublic(culpa.New("?owner= not accepted on write paths"), "?owner= is not accepted on saved-search write paths"), + "INVALID", + )) + return + } + + var req createRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + apierror.Render(w, r, culpa.WithCode( + culpa.WithPublic(culpa.Wrap(err, "decode body"), "invalid request body"), + "INVALID", + )) + return + } + + if err := validateName(req.Name); err != nil { + apierror.Render(w, r, err) + return + } + if err := validateQuery(req.Query); err != nil { + apierror.Render(w, r, err) + return + } + + now := time.Now().Unix() + s := SavedSearch{ + Owner: h.ownerOf(r), + Name: req.Name, + Query: req.Query, + CreatedAt: now, + UpdatedAt: now, + } + if err := h.Repo.Create(r.Context(), s); err != nil { + apierror.Render(w, r, err) + return + } + + if writeErr := httputil.WriteJSON(w, http.StatusCreated, s); writeErr != nil { + slog.Default().ErrorContext(r.Context(), "write saved-search create response", scribe.Err(writeErr)) + } +} + +// updateRequest is the decoded JSON body for PUT /saved-searches/{name}. +// Both fields are optional; at least one must be non-nil. +type updateRequest struct { + Name *string `json:"name"` + Query *string `json:"query"` +} + +// Update handles PUT /saved-searches/{name}. Decodes { name?, query? }, requires +// at least one field, validates non-nil fields, and returns 200 with the updated row. +func (h *Handler) Update(w http.ResponseWriter, r *http.Request) { + // Defensive: reject any explicit ?owner= param (IV2). + if r.URL.Query().Get("owner") != "" { + apierror.Render(w, r, culpa.WithCode( + culpa.WithPublic(culpa.New("?owner= not accepted on write paths"), "?owner= is not accepted on saved-search write paths"), + "INVALID", + )) + return + } + + urlName := chi.URLParam(r, "name") + + var req updateRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + apierror.Render(w, r, culpa.WithCode( + culpa.WithPublic(culpa.Wrap(err, "decode body"), "invalid request body"), + "INVALID", + )) + return + } + + if req.Name == nil && req.Query == nil { + apierror.Render(w, r, culpa.WithCode( + culpa.WithPublic(culpa.New("no fields to update"), "at least one of name or query must be provided"), + "VALIDATION", + )) + return + } + + if req.Name != nil { + if err := validateName(*req.Name); err != nil { + apierror.Render(w, r, err) + return + } + } + if req.Query != nil { + if err := validateQuery(*req.Query); err != nil { + apierror.Render(w, r, err) + return + } + } + + owner := h.ownerOf(r) + now := time.Now().Unix() + updated, err := h.Repo.Update(r.Context(), owner, urlName, req.Name, req.Query, now) + if err != nil { + apierror.Render(w, r, err) + return + } + + if writeErr := httputil.WriteJSON(w, http.StatusOK, updated); writeErr != nil { + slog.Default().ErrorContext(r.Context(), "write saved-search update response", scribe.Err(writeErr)) + } +} + +// Delete handles DELETE /saved-searches/{name}. Returns 204 No Content on +// success, 404 when the row does not exist. +func (h *Handler) Delete(w http.ResponseWriter, r *http.Request) { + owner := h.ownerOf(r) + name := chi.URLParam(r, "name") + + if err := h.Repo.Delete(r.Context(), owner, name); err != nil { + apierror.Render(w, r, err) + return + } + w.WriteHeader(http.StatusNoContent) +} diff --git a/internal/domain/savedsearch/handler_test.go b/internal/domain/savedsearch/handler_test.go new file mode 100644 index 0000000000000000000000000000000000000000..93ca78570eb33fc6461f07595e6bb0c7faf3d145 --- /dev/null +++ b/internal/domain/savedsearch/handler_test.go @@ -0,0 +1,332 @@ +package savedsearch_test + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + + "sourcecraft.dev/bigbes/lethe/internal/domain/savedsearch" + "sourcecraft.dev/bigbes/lethe/internal/server/auth" +) + +// fakeAuthMiddleware injects a fixed Identity onto the request context so +// the handler can call auth.MustIdentity without a real Authenticator. +func fakeAuthMiddleware(id auth.Identity) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := auth.WithIdentity(r.Context(), id) + next.ServeHTTP(w, r.WithContext(ctx)) + }) + } +} + +// newHandler wires a Handler against a fresh in-memory database. +func newHandler(t *testing.T) *savedsearch.Handler { + t.Helper() + repo := newRepo(t) + h := &savedsearch.Handler{Repo: repo} + if err := h.Init(context.Background()); err != nil { + t.Fatalf("handler.Init: %v", err) + } + return h +} + +// mountWithIdentity builds a chi router with the fake auth middleware and +// the savedsearch handler mounted under /api/v1. +func mountWithIdentity(h *savedsearch.Handler, id auth.Identity) http.Handler { + r := chi.NewRouter() + r.Route("/api/v1", func(r chi.Router) { + r.Use(fakeAuthMiddleware(id)) + h.Mount(r) + }) + return r +} + +// savedSearchListBody is the decoded JSON body from GET /saved-searches. +type savedSearchListBody struct { + SavedSearches []json.RawMessage `json:"saved_searches"` +} + +// problemBody captures RFC 7807 fields tests assert on. +type problemBody struct { + Status int `json:"status"` + Code string `json:"code"` +} + +func doGET(t *testing.T, router http.Handler, path string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodGet, path, nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec +} + +func doPOST(t *testing.T, router http.Handler, path string, body any) *httptest.ResponseRecorder { + t.Helper() + b, err := json.Marshal(body) + if err != nil { + t.Fatalf("marshal POST body: %v", err) + } + req := httptest.NewRequest(http.MethodPost, path, bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec +} + +func doPUT(t *testing.T, router http.Handler, path string, body any) *httptest.ResponseRecorder { + t.Helper() + b, err := json.Marshal(body) + if err != nil { + t.Fatalf("marshal PUT body: %v", err) + } + req := httptest.NewRequest(http.MethodPut, path, bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec +} + +func doDELETE(t *testing.T, router http.Handler, path string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodDelete, path, nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec +} + +// TestHandler_List_Authenticated verifies GET /saved-searches returns 200 +// with { saved_searches: [] } when no rows exist. +func TestHandler_List_Authenticated(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + rec := doGET(t, router, "/api/v1/saved-searches") + if rec.Code != http.StatusOK { + t.Fatalf("status=%d; want 200; body=%s", rec.Code, rec.Body.String()) + } + var body savedSearchListBody + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unmarshal: %v (body=%s)", err, rec.Body.String()) + } + if body.SavedSearches == nil { + t.Fatal("saved_searches key missing or null") + } + if len(body.SavedSearches) != 0 { + t.Fatalf("expected empty list; got %d items", len(body.SavedSearches)) + } +} + +// TestHandler_List_OwnerParamIgnored verifies that ?owner= is silently ignored +// (IV2) and the response is identical to a call without the param. +func TestHandler_List_OwnerParamIgnored(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + rec1 := doGET(t, router, "/api/v1/saved-searches") + rec2 := doGET(t, router, "/api/v1/saved-searches?owner=alice") + + if rec1.Code != http.StatusOK || rec2.Code != http.StatusOK { + t.Fatalf("status1=%d status2=%d; both want 200", rec1.Code, rec2.Code) + } + if rec1.Body.String() != rec2.Body.String() { + t.Fatalf("bodies differ:\n no-param: %s\n with-param: %s", + rec1.Body.String(), rec2.Body.String()) + } +} + +// TestHandler_List_OwnerFieldAbsentInJSON verifies that Owner field is not +// present in the JSON output (json:"-"). +func TestHandler_List_OwnerFieldAbsentInJSON(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + // Create a row first. + doPOST(t, router, "/api/v1/saved-searches", map[string]string{"name": "test", "query": "q"}) + + rec := doGET(t, router, "/api/v1/saved-searches") + if rec.Code != http.StatusOK { + t.Fatalf("status=%d; want 200; body=%s", rec.Code, rec.Body.String()) + } + bodyStr := rec.Body.String() + if strings.Contains(bodyStr, `"owner"`) { + t.Fatalf("owner field should be absent in JSON; body=%s", bodyStr) + } +} + +// TestHandler_Create_EmptyNameReturns400 verifies validation of empty name. +func TestHandler_Create_EmptyNameReturns400(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + rec := doPOST(t, router, "/api/v1/saved-searches", map[string]string{"name": "", "query": "x"}) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status=%d; want 400; body=%s", rec.Code, rec.Body.String()) + } + var p problemBody + _ = json.Unmarshal(rec.Body.Bytes(), &p) + if p.Code != "VALIDATION" { + t.Fatalf("expected VALIDATION; got %q (body=%s)", p.Code, rec.Body.String()) + } +} + +// TestHandler_Create_SlashInNameReturns400 verifies that a name with "/" is +// rejected (IV7). +func TestHandler_Create_SlashInNameReturns400(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + rec := doPOST(t, router, "/api/v1/saved-searches", map[string]string{"name": "a/b", "query": "x"}) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status=%d; want 400; body=%s", rec.Code, rec.Body.String()) + } + var p problemBody + _ = json.Unmarshal(rec.Body.Bytes(), &p) + if p.Code != "VALIDATION" { + t.Fatalf("expected VALIDATION; got %q (body=%s)", p.Code, rec.Body.String()) + } +} + +// TestHandler_Create_NameTooLongReturns400 verifies the 64-char cap (UK1). +func TestHandler_Create_NameTooLongReturns400(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + longName := strings.Repeat("a", 65) + rec := doPOST(t, router, "/api/v1/saved-searches", map[string]string{"name": longName, "query": "x"}) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status=%d; want 400; body=%s", rec.Code, rec.Body.String()) + } + var p problemBody + _ = json.Unmarshal(rec.Body.Bytes(), &p) + if p.Code != "VALIDATION" { + t.Fatalf("expected VALIDATION; got %q (body=%s)", p.Code, rec.Body.String()) + } +} + +// TestHandler_Create_EmptyQueryReturns400 verifies that an empty query is +// rejected. +func TestHandler_Create_EmptyQueryReturns400(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + rec := doPOST(t, router, "/api/v1/saved-searches", map[string]string{"name": "ok", "query": ""}) + if rec.Code != http.StatusBadRequest { + t.Fatalf("status=%d; want 400; body=%s", rec.Code, rec.Body.String()) + } + var p problemBody + _ = json.Unmarshal(rec.Body.Bytes(), &p) + if p.Code != "VALIDATION" { + t.Fatalf("expected VALIDATION; got %q (body=%s)", p.Code, rec.Body.String()) + } +} + +// TestHandler_Create_Valid verifies that a valid POST returns 201 with the new row. +func TestHandler_Create_Valid(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + rec := doPOST(t, router, "/api/v1/saved-searches", map[string]string{"name": "mysearch", "query": "model:gpt-4"}) + if rec.Code != http.StatusCreated { + t.Fatalf("status=%d; want 201; body=%s", rec.Code, rec.Body.String()) + } + var got savedsearch.SavedSearch + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("unmarshal: %v (body=%s)", err, rec.Body.String()) + } + if got.Name != "mysearch" { + t.Errorf("Name: got %q; want %q", got.Name, "mysearch") + } + if got.Query != "model:gpt-4" { + t.Errorf("Query: got %q; want %q", got.Query, "model:gpt-4") + } +} + +// TestHandler_Create_DuplicateReturns409 verifies that a duplicate name for the +// same owner returns 409 CONFLICT with problem+json. +func TestHandler_Create_DuplicateReturns409(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + doPOST(t, router, "/api/v1/saved-searches", map[string]string{"name": "x", "query": "q1"}) + rec := doPOST(t, router, "/api/v1/saved-searches", map[string]string{"name": "x", "query": "q2"}) + if rec.Code != http.StatusConflict { + t.Fatalf("status=%d; want 409; body=%s", rec.Code, rec.Body.String()) + } + var p problemBody + _ = json.Unmarshal(rec.Body.Bytes(), &p) + if p.Code != "CONFLICT" { + t.Fatalf("expected CONFLICT; got %q (body=%s)", p.Code, rec.Body.String()) + } + // Verify content-type is problem+json. + ct := rec.Header().Get("Content-Type") + if !strings.Contains(ct, "application/problem+json") { + t.Fatalf("expected application/problem+json; got %q", ct) + } +} + +// TestHandler_Update_NotFound verifies that PUT /saved-searches/missing returns 404. +func TestHandler_Update_NotFound(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + rec := doPUT(t, router, "/api/v1/saved-searches/missing", map[string]*string{"query": ptrStr("new")}) + if rec.Code != http.StatusNotFound { + t.Fatalf("status=%d; want 404; body=%s", rec.Code, rec.Body.String()) + } + var p problemBody + _ = json.Unmarshal(rec.Body.Bytes(), &p) + if p.Code != "NOT_FOUND" { + t.Fatalf("expected NOT_FOUND; got %q (body=%s)", p.Code, rec.Body.String()) + } +} + +// TestHandler_Update_RenameConflict verifies that renaming onto an existing name +// for the same owner returns 409 CONFLICT. +func TestHandler_Update_RenameConflict(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + doPOST(t, router, "/api/v1/saved-searches", map[string]string{"name": "x", "query": "q1"}) + doPOST(t, router, "/api/v1/saved-searches", map[string]string{"name": "y", "query": "q2"}) + + rec := doPUT(t, router, "/api/v1/saved-searches/x", map[string]*string{"name": ptrStr("y")}) + if rec.Code != http.StatusConflict { + t.Fatalf("status=%d; want 409; body=%s", rec.Code, rec.Body.String()) + } + var p problemBody + _ = json.Unmarshal(rec.Body.Bytes(), &p) + if p.Code != "CONFLICT" { + t.Fatalf("expected CONFLICT; got %q (body=%s)", p.Code, rec.Body.String()) + } +} + +// TestHandler_Delete_Sequence verifies 204 on first delete and 404 on second. +func TestHandler_Delete_Sequence(t *testing.T) { + h := newHandler(t) + router := mountWithIdentity(h, auth.Identity{User: "alice"}) + + // Create a row first. + doPOST(t, router, "/api/v1/saved-searches", map[string]string{"name": "todel", "query": "q"}) + + rec1 := doDELETE(t, router, "/api/v1/saved-searches/todel") + if rec1.Code != http.StatusNoContent { + t.Fatalf("first delete: status=%d; want 204; body=%s", rec1.Code, rec1.Body.String()) + } + + rec2 := doDELETE(t, router, "/api/v1/saved-searches/todel") + if rec2.Code != http.StatusNotFound { + t.Fatalf("second delete: status=%d; want 404; body=%s", rec2.Code, rec2.Body.String()) + } + var p problemBody + _ = json.Unmarshal(rec2.Body.Bytes(), &p) + if p.Code != "NOT_FOUND" { + t.Fatalf("expected NOT_FOUND; got %q", p.Code) + } +} diff --git a/internal/domain/savedsearch/repository.go b/internal/domain/savedsearch/repository.go new file mode 100644 index 0000000000000000000000000000000000000000..facdc1d11f6b6afacfe37ff1db970b8b4a7b6b6f --- /dev/null +++ b/internal/domain/savedsearch/repository.go @@ -0,0 +1,198 @@ +// Package savedsearch implements the saved-search CRUD API: a per-owner list of +// named queries that can be stored, renamed, updated, and deleted. The package +// layers as Repository (raw SQL) and Handler (HTTP boundary). Both are steward +// services. +// +// Owner-scope invariant (IV2): the owner is always derived from the +// authenticated identity; no ?owner= query parameter is accepted on any +// saved-search route. +// +// Uniqueness invariant (IV3): (owner, name) is a composite primary key; +// duplicate names per owner produce a CONFLICT-coded error, never a silent +// overwrite. +package savedsearch + +import ( + "context" + "database/sql" + "errors" + "strings" + + "go.bigb.es/auxilia/culpa" + "modernc.org/sqlite" + sqlite3 "modernc.org/sqlite/lib" + + "sourcecraft.dev/bigbes/lethe/internal/platform/database" +) + +// SavedSearch is the row shape for the saved_searches table. The Owner field +// carries json:"-" so it is never included in wire responses (IV2); all other +// fields use snake_case JSON tags. +type SavedSearch struct { + Owner string `db:"owner" json:"-"` + Name string `db:"name" json:"name"` + Query string `db:"query" json:"query"` + CreatedAt int64 `db:"created_at" json:"created_at"` + UpdatedAt int64 `db:"updated_at" json:"updated_at"` +} + +// Repository is the SQL steward for the saved_searches table. It is stateless +// beyond its injected dependencies; Init is empty. +type Repository struct { + Database *database.Database `inject:""` +} + +// Init satisfies the steward Initer contract. Nothing to set up — the +// underlying *sqlx.DB is owned by the Database steward. +func (r *Repository) Init(_ context.Context) error { return nil } + +// List returns all saved searches for the given owner ordered by updated_at +// DESC. Returns a non-nil zero-length slice when no rows are present so +// JSON-encoding produces [] rather than null. +func (r *Repository) List(ctx context.Context, owner string) ([]SavedSearch, error) { + const q = `SELECT owner, name, query, created_at, updated_at + FROM saved_searches + WHERE owner = ? + ORDER BY updated_at DESC` + out := make([]SavedSearch, 0) + if err := r.Database.DB.SelectContext(ctx, &out, q, owner); err != nil { + return nil, culpa.WithCode(culpa.Wrap(err, "list saved searches"), "DB_QUERY") + } + return out, nil +} + +// Create inserts a new saved search row. On primary-key collision (same owner +// + name already exists) it returns a CONFLICT-coded error with a public +// message suitable for surfacing to clients (IV3). +func (r *Repository) Create(ctx context.Context, s SavedSearch) error { + const q = `INSERT INTO saved_searches (owner, name, query, created_at, updated_at) + VALUES (?, ?, ?, ?, ?)` + _, err := r.Database.DB.ExecContext(ctx, q, s.Owner, s.Name, s.Query, s.CreatedAt, s.UpdatedAt) + if err != nil { + if isSQLiteConstraint(err) { + return culpa.WithCode( + culpa.WithPublic(culpa.Wrap(err, "create saved search"), "saved search with that name already exists"), + "CONFLICT", + ) + } + return culpa.WithCode(culpa.Wrap(err, "create saved search"), "DB_QUERY") + } + return nil +} + +// Update modifies an existing saved search identified by (owner, oldName). +// newName and newQuery are optional; at least one must be non-nil (the handler +// enforces this precondition). updated_at is always set to now. +// +// The rename + read are performed inside a single transaction (BeginTxx) so +// the returned SavedSearch reflects the post-update state atomically. +// +// RowsAffected == 0 → NOT_FOUND; primary-key collision on rename → CONFLICT. +func (r *Repository) Update(ctx context.Context, owner, oldName string, newName *string, newQuery *string, now int64) (SavedSearch, error) { + tx, err := r.Database.DB.BeginTxx(ctx, nil) + if err != nil { + return SavedSearch{}, culpa.WithCode(culpa.Wrap(err, "begin update transaction"), "DB_QUERY") + } + defer func() { _ = tx.Rollback() }() + + // Build a dynamic SET clause: name and/or query, plus updated_at. + var sets []string + var args []any + if newName != nil { + sets = append(sets, "name = ?") + args = append(args, *newName) + } + if newQuery != nil { + sets = append(sets, "query = ?") + args = append(args, *newQuery) + } + sets = append(sets, "updated_at = ?") + args = append(args, now) + // WHERE clause args. + args = append(args, owner, oldName) + + updateQ := "UPDATE saved_searches SET " + strings.Join(sets, ", ") + " WHERE owner = ? AND name = ?" + res, err := tx.ExecContext(ctx, updateQ, args...) + if err != nil { + if isSQLiteConstraint(err) { + return SavedSearch{}, culpa.WithCode( + culpa.WithPublic(culpa.Wrap(err, "update saved search"), "saved search with that name already exists"), + "CONFLICT", + ) + } + return SavedSearch{}, culpa.WithCode(culpa.Wrap(err, "update saved search"), "DB_QUERY") + } + + n, err := res.RowsAffected() + if err != nil { + return SavedSearch{}, culpa.WithCode(culpa.Wrap(err, "rows affected"), "DB_QUERY") + } + if n == 0 { + return SavedSearch{}, culpa.WithCode( + culpa.WithPublic(culpa.New("saved search not found"), "saved search not found"), + "NOT_FOUND", + ) + } + + // Determine the effective name (may have changed on rename). + effectiveName := oldName + if newName != nil { + effectiveName = *newName + } + + const selectQ = `SELECT owner, name, query, created_at, updated_at + FROM saved_searches WHERE owner = ? AND name = ?` + var updated SavedSearch + if err := tx.GetContext(ctx, &updated, selectQ, owner, effectiveName); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return SavedSearch{}, culpa.WithCode(culpa.New("saved search not found after update"), "NOT_FOUND") + } + return SavedSearch{}, culpa.WithCode(culpa.Wrap(err, "read updated saved search"), "DB_QUERY") + } + + if err := tx.Commit(); err != nil { + return SavedSearch{}, culpa.WithCode(culpa.Wrap(err, "commit update transaction"), "DB_QUERY") + } + return updated, nil +} + +// Delete removes the saved search identified by (owner, name). Returns +// NOT_FOUND when no row is deleted. +func (r *Repository) Delete(ctx context.Context, owner, name string) error { + const q = `DELETE FROM saved_searches WHERE owner = ? AND name = ?` + res, err := r.Database.DB.ExecContext(ctx, q, owner, name) + if err != nil { + return culpa.WithCode(culpa.Wrap(err, "delete saved search"), "DB_QUERY") + } + n, err := res.RowsAffected() + if err != nil { + return culpa.WithCode(culpa.Wrap(err, "rows affected"), "DB_QUERY") + } + if n == 0 { + return culpa.WithCode( + culpa.WithPublic(culpa.New("saved search not found"), "saved search not found"), + "NOT_FOUND", + ) + } + return nil +} + +// isSQLiteConstraint returns true when err is a SQLite constraint violation +// (PRIMARY KEY, UNIQUE, NOT NULL, or generic CONSTRAINT). This mirrors the +// pattern used in internal/domain/ingest/repository.go. +func isSQLiteConstraint(err error) bool { + var se *sqlite.Error + if !errors.As(err, &se) { + return false + } + switch se.Code() { + case sqlite3.SQLITE_CONSTRAINT, + sqlite3.SQLITE_CONSTRAINT_CHECK, + sqlite3.SQLITE_CONSTRAINT_NOTNULL, + sqlite3.SQLITE_CONSTRAINT_PRIMARYKEY, + sqlite3.SQLITE_CONSTRAINT_UNIQUE: + return true + } + return false +} + diff --git a/internal/domain/savedsearch/repository_test.go b/internal/domain/savedsearch/repository_test.go new file mode 100644 index 0000000000000000000000000000000000000000..3beceff46eccf043907a9f82bd7bbed42a12cdc0 --- /dev/null +++ b/internal/domain/savedsearch/repository_test.go @@ -0,0 +1,321 @@ +package savedsearch_test + +import ( + "context" + "testing" + "time" + + "go.bigb.es/auxilia/culpa" + _ "modernc.org/sqlite" + + "sourcecraft.dev/bigbes/lethe/internal/config" + "sourcecraft.dev/bigbes/lethe/internal/domain/savedsearch" + "sourcecraft.dev/bigbes/lethe/internal/platform/database" +) + +// newTestDatabase builds a Database steward against :memory: (one DB per +// test, isolated). Cleanup runs Destroy. +func newTestDatabase(t *testing.T) *database.Database { + t.Helper() + d := &database.Database{ + Cfg: config.DatabaseConfig{ + Path: ":memory:", + BusyTimeout: 5 * time.Second, + }, + } + if err := d.Init(context.Background()); err != nil { + t.Fatalf("database.Init: %v", err) + } + t.Cleanup(func() { _ = d.Destroy(context.Background()) }) + return d +} + +// newRepo wires a Repository against a fresh in-memory database. +func newRepo(t *testing.T) *savedsearch.Repository { + t.Helper() + d := newTestDatabase(t) + repo := &savedsearch.Repository{Database: d} + if err := repo.Init(context.Background()); err != nil { + t.Fatalf("repo.Init: %v", err) + } + return repo +} + +// codeOf walks the culpa chain for a CodeDetail and returns the string code, +// or "" if there isn't one. +func codeOf(err error) string { + var cd culpa.CodeDetail + if !culpa.FindDetail(err, &cd) { + return "" + } + s, ok := cd.Code.(string) + if !ok { + return "" + } + return s +} + +func ptrStr(s string) *string { return &s } + +// TestList_EmptyDB verifies that List on an empty DB returns a non-nil, +// zero-length slice (JSON [] rather than null). +func TestList_EmptyDB(t *testing.T) { + repo := newRepo(t) + got, err := repo.List(context.Background(), "alice") + if err != nil { + t.Fatalf("List: %v", err) + } + if got == nil { + t.Fatal("expected non-nil slice; got nil") + } + if len(got) != 0 { + t.Fatalf("expected zero-length slice; got len=%d", len(got)) + } +} + +// TestCreate_ThenList verifies that a Created row is returned by List with +// correct field values and that created_at == updated_at == the injected now. +func TestCreate_ThenList(t *testing.T) { + repo := newRepo(t) + now := int64(1700000000) + s := savedsearch.SavedSearch{ + Owner: "alice", + Name: "my search", + Query: "model:gpt-4", + CreatedAt: now, + UpdatedAt: now, + } + if err := repo.Create(context.Background(), s); err != nil { + t.Fatalf("Create: %v", err) + } + + rows, err := repo.List(context.Background(), "alice") + if err != nil { + t.Fatalf("List: %v", err) + } + if len(rows) != 1 { + t.Fatalf("expected 1 row; got %d", len(rows)) + } + got := rows[0] + if got.Name != "my search" { + t.Errorf("Name: got %q; want %q", got.Name, "my search") + } + if got.Query != "model:gpt-4" { + t.Errorf("Query: got %q; want %q", got.Query, "model:gpt-4") + } + if got.CreatedAt != now { + t.Errorf("CreatedAt: got %d; want %d", got.CreatedAt, now) + } + if got.UpdatedAt != now { + t.Errorf("UpdatedAt: got %d; want %d", got.UpdatedAt, now) + } + // Owner is stored in DB but json:"-"; verify the struct still has it. + if got.Owner != "alice" { + t.Errorf("Owner: got %q; want %q", got.Owner, "alice") + } +} + +// TestCreate_Duplicate verifies that a second Create with the same (owner, name) +// returns a CONFLICT-coded error. +func TestCreate_Duplicate(t *testing.T) { + repo := newRepo(t) + now := int64(1700000000) + s := savedsearch.SavedSearch{ + Owner: "alice", + Name: "dupe", + Query: "q1", + CreatedAt: now, + UpdatedAt: now, + } + if err := repo.Create(context.Background(), s); err != nil { + t.Fatalf("first Create: %v", err) + } + s.Query = "q2" + err := repo.Create(context.Background(), s) + if err == nil { + t.Fatal("expected CONFLICT error; got nil") + } + if code := codeOf(err); code != "CONFLICT" { + t.Fatalf("expected code CONFLICT; got %q", code) + } +} + +// TestCreate_SameNameDifferentOwner verifies that the same name can be used by +// two different owners without conflict. +func TestCreate_SameNameDifferentOwner(t *testing.T) { + repo := newRepo(t) + now := int64(1700000000) + if err := repo.Create(context.Background(), savedsearch.SavedSearch{ + Owner: "alice", Name: "shared", Query: "q1", + CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("alice Create: %v", err) + } + if err := repo.Create(context.Background(), savedsearch.SavedSearch{ + Owner: "bob", Name: "shared", Query: "q2", + CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("bob Create: %v", err) + } + + aliceRows, _ := repo.List(context.Background(), "alice") + bobRows, _ := repo.List(context.Background(), "bob") + if len(aliceRows) != 1 { + t.Errorf("alice: expected 1 row; got %d", len(aliceRows)) + } + if len(bobRows) != 1 { + t.Errorf("bob: expected 1 row; got %d", len(bobRows)) + } +} + +// TestUpdate_NotFound verifies that updating a non-existent (owner, name) +// returns a NOT_FOUND-coded error. +func TestUpdate_NotFound(t *testing.T) { + repo := newRepo(t) + _, err := repo.Update(context.Background(), "alice", "missing", nil, ptrStr("newq"), 1700000001) + if err == nil { + t.Fatal("expected NOT_FOUND error; got nil") + } + if code := codeOf(err); code != "NOT_FOUND" { + t.Fatalf("expected code NOT_FOUND; got %q", code) + } +} + +// TestUpdate_RenameConflict verifies that renaming onto an existing name for +// the same owner returns a CONFLICT-coded error. +func TestUpdate_RenameConflict(t *testing.T) { + repo := newRepo(t) + now := int64(1700000000) + if err := repo.Create(context.Background(), savedsearch.SavedSearch{ + Owner: "alice", Name: "a", Query: "q1", + CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("Create a: %v", err) + } + if err := repo.Create(context.Background(), savedsearch.SavedSearch{ + Owner: "alice", Name: "b", Query: "q2", + CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("Create b: %v", err) + } + + // Try to rename "a" to "b" (which already exists for alice). + _, err := repo.Update(context.Background(), "alice", "a", ptrStr("b"), nil, now+1) + if err == nil { + t.Fatal("expected CONFLICT error; got nil") + } + if code := codeOf(err); code != "CONFLICT" { + t.Fatalf("expected code CONFLICT; got %q", code) + } +} + +// TestUpdate_QueryOnly verifies that passing newName=nil only updates the query +// and updated_at, leaving the name unchanged. +func TestUpdate_QueryOnly(t *testing.T) { + repo := newRepo(t) + now := int64(1700000000) + if err := repo.Create(context.Background(), savedsearch.SavedSearch{ + Owner: "alice", Name: "myq", Query: "old query", + CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("Create: %v", err) + } + + later := now + 100 + updated, err := repo.Update(context.Background(), "alice", "myq", nil, ptrStr("new query"), later) + if err != nil { + t.Fatalf("Update: %v", err) + } + if updated.Name != "myq" { + t.Errorf("Name should be unchanged: got %q; want %q", updated.Name, "myq") + } + if updated.Query != "new query" { + t.Errorf("Query: got %q; want %q", updated.Query, "new query") + } + if updated.UpdatedAt != later { + t.Errorf("UpdatedAt: got %d; want %d", updated.UpdatedAt, later) + } + // created_at should remain the original. + if updated.CreatedAt != now { + t.Errorf("CreatedAt should be unchanged: got %d; want %d", updated.CreatedAt, now) + } +} + +// TestDelete_NotFound verifies that deleting a non-existent row returns a +// NOT_FOUND-coded error. +func TestDelete_NotFound(t *testing.T) { + repo := newRepo(t) + err := repo.Delete(context.Background(), "alice", "ghost") + if err == nil { + t.Fatal("expected NOT_FOUND error; got nil") + } + if code := codeOf(err); code != "NOT_FOUND" { + t.Fatalf("expected code NOT_FOUND; got %q", code) + } +} + +// TestDelete_ExistingRow verifies that deleting an existing row succeeds and +// that the row is absent on a subsequent List. +func TestDelete_ExistingRow(t *testing.T) { + repo := newRepo(t) + now := int64(1700000000) + if err := repo.Create(context.Background(), savedsearch.SavedSearch{ + Owner: "alice", Name: "bye", Query: "q", + CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("Create: %v", err) + } + + if err := repo.Delete(context.Background(), "alice", "bye"); err != nil { + t.Fatalf("Delete: %v", err) + } + + rows, err := repo.List(context.Background(), "alice") + if err != nil { + t.Fatalf("List after Delete: %v", err) + } + if len(rows) != 0 { + t.Fatalf("expected 0 rows after delete; got %d", len(rows)) + } + + // Subsequent delete should be NOT_FOUND. + err = repo.Delete(context.Background(), "alice", "bye") + if err == nil { + t.Fatal("expected NOT_FOUND on second delete; got nil") + } + if code := codeOf(err); code != "NOT_FOUND" { + t.Fatalf("expected NOT_FOUND; got %q", code) + } +} + +// TestList_OrderByUpdatedAtDesc verifies that rows are returned newest first +// according to updated_at. +func TestList_OrderByUpdatedAtDesc(t *testing.T) { + repo := newRepo(t) + base := int64(1700000000) + // Insert three rows with staggered updated_at values. + for i, name := range []string{"first", "second", "third"} { + ts := base + int64(i)*100 // first=base, second=base+100, third=base+200 + if err := repo.Create(context.Background(), savedsearch.SavedSearch{ + Owner: "alice", + Name: name, + Query: "q", + CreatedAt: ts, + UpdatedAt: ts, + }); err != nil { + t.Fatalf("Create %s: %v", name, err) + } + } + + rows, err := repo.List(context.Background(), "alice") + if err != nil { + t.Fatalf("List: %v", err) + } + if len(rows) != 3 { + t.Fatalf("expected 3 rows; got %d", len(rows)) + } + // Expected order: third (newest), second, first (oldest). + if rows[0].Name != "third" || rows[1].Name != "second" || rows[2].Name != "first" { + t.Fatalf("unexpected order: %s, %s, %s", rows[0].Name, rows[1].Name, rows[2].Name) + } +} diff --git a/internal/platform/database/migrations/0002_saved_searches.down.sql b/internal/platform/database/migrations/0002_saved_searches.down.sql new file mode 100644 index 0000000000000000000000000000000000000000..887985af866498b014cd70b8e58c0efaffc08954 --- /dev/null +++ b/internal/platform/database/migrations/0002_saved_searches.down.sql @@ -0,0 +1,2 @@ +DROP INDEX IF EXISTS idx_saved_searches_owner_updated; +DROP TABLE IF EXISTS saved_searches; diff --git a/internal/platform/database/migrations/0002_saved_searches.up.sql b/internal/platform/database/migrations/0002_saved_searches.up.sql new file mode 100644 index 0000000000000000000000000000000000000000..06f9dc2c3ad695f4d2b85717b8d050274aca7063 --- /dev/null +++ b/internal/platform/database/migrations/0002_saved_searches.up.sql @@ -0,0 +1,10 @@ +CREATE TABLE saved_searches ( + owner TEXT NOT NULL, + name TEXT NOT NULL, + query TEXT NOT NULL, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + PRIMARY KEY (owner, name) +) WITHOUT ROWID; +CREATE INDEX idx_saved_searches_owner_updated + ON saved_searches (owner, updated_at DESC); diff --git a/internal/server/server.go b/internal/server/server.go index 7195d1e6b07b623df95d9000ca26112991a393d9..249c035ba6fadacdc465430990a728df2838bd86 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -31,6 +31,7 @@ import ( "sourcecraft.dev/bigbes/lethe/internal/config" "sourcecraft.dev/bigbes/lethe/internal/domain/ingest" "sourcecraft.dev/bigbes/lethe/internal/domain/project" + "sourcecraft.dev/bigbes/lethe/internal/domain/savedsearch" "sourcecraft.dev/bigbes/lethe/internal/domain/session" "sourcecraft.dev/bigbes/lethe/internal/domain/stats" "sourcecraft.dev/bigbes/lethe/internal/pkg/apierror" @@ -55,11 +56,12 @@ type Server struct { Metrics *observability.Metrics `inject:""` Health *health.Set `inject:""` - Auth *authpkg.Authenticator `inject:""` - Ingest *ingest.Handler `inject:""` - Sessions *session.Handler `inject:""` - Projects *project.Handler `inject:""` - Stats *stats.Handler `inject:""` + Auth *authpkg.Authenticator `inject:""` + Ingest *ingest.Handler `inject:""` + Sessions *session.Handler `inject:""` + Projects *project.Handler `inject:""` + Stats *stats.Handler `inject:""` + SavedSearches *savedsearch.Handler `inject:""` router *chi.Mux httpSrv *http.Server @@ -103,6 +105,7 @@ func (s *Server) Init(_ context.Context) error { s.Sessions.Mount(r) s.Projects.Mount(r) s.Stats.Mount(r) + s.SavedSearches.Mount(r) }) // SPA catch-all: serves the embedded React app for all non-API GET paths.