diff --git a/apps/admin/src/app/(authenticated)/posts/page.test.tsx b/apps/admin/src/app/(authenticated)/posts/page.test.tsx index 6f2ca839..fe933142 100644 --- a/apps/admin/src/app/(authenticated)/posts/page.test.tsx +++ b/apps/admin/src/app/(authenticated)/posts/page.test.tsx @@ -9,6 +9,47 @@ import { describe, expect, it } from 'vitest'; import { render } from '@testing-library/react'; import { Headline } from '@/components/ui/headline'; +import { adaptApiPost, type ApiPost } from './page'; + +describe('adaptApiPost — author display fallback (issue #515)', () => { + it('falls back to last 8 chars of the UUID when the API omits a display name', () => { + // 36-char UUID — the realistic shape coming back from the list + // endpoint today (no `users` join, so no display_name). + const apiPost: ApiPost = { + id: 'post-1', + title: 'Hello', + status: 'publish', + author_id: '550e8400-e29b-41d4-a716-446655440000', + }; + const adapted = adaptApiPost(apiPost); + + expect(adapted.author.id).toBe('550e8400-e29b-41d4-a716-446655440000'); + // Last 8 chars of the UUID — short enough for the table cell, + // long enough to disambiguate. + expect(adapted.author.displayName).toBe('55440000'); + }); + + it('uses the API-supplied display name when present (no fallback)', () => { + const apiPost: ApiPost = { + id: 'post-2', + title: 'Hi', + status: 'publish', + author_id: '550e8400-e29b-41d4-a716-446655440000', + author: { display_name: 'Ada Lovelace' }, + }; + expect(adaptApiPost(apiPost).author.displayName).toBe('Ada Lovelace'); + }); + + it('leaves commentsCount at 0 — list endpoint does not compute it', () => { + const apiPost: ApiPost = { + id: 'post-3', + title: 'C', + status: 'draft', + author_id: 'abcdefgh', + }; + expect(adaptApiPost(apiPost).commentsCount).toBe(0); + }); +}); describe('Posts page head', () => { it('renders the brand "All posts." headline with the italic accent', () => { diff --git a/apps/admin/src/app/(authenticated)/posts/page.tsx b/apps/admin/src/app/(authenticated)/posts/page.tsx index 7e4eb971..5dfaee18 100644 --- a/apps/admin/src/app/(authenticated)/posts/page.tsx +++ b/apps/admin/src/app/(authenticated)/posts/page.tsx @@ -44,6 +44,66 @@ import styles from './posts.module.css'; export const dynamic = 'force-dynamic'; +/** + * Last 8 chars of a UUID — enough to disambiguate authors in the list + * table without occupying half the column. Mirrors the helper of the + * same name in `posts/[id]/revisions/page.tsx` (issue #515 fixes the + * blank-Author-cell bug by routing through this fallback when the list + * API doesn't include an author display name). + */ +function shortAuthorId(id: string): string { + if (id.length <= 8) return id; + return id.slice(-8); +} + +/** Wire shape we expect from `GET /api/v1/posts`. */ +export type ApiPost = { + id: string; + title: string; + status: string; + published_at?: string | null; + updated_at?: string; + created_at?: string; + author_id?: string; + author?: { id?: string; display_name?: string; displayName?: string } | null; +}; + +/** + * Adapt an API post to the flatter `Post` shape the list UI expects. + * + * Pulled out of `fetchInitialPosts` so it can be unit tested without + * spinning up the whole server component (issue #515). + * + * Author display name: the list endpoint doesn't currently JOIN + * `users`, so `author.display_name` is usually absent. We fall back + * to the last 8 chars of the author UUID rather than rendering a + * blank Author cell — matches the pattern used on the revisions page + * (`shortId` helper). + * + * Comments count: a separate aggregate the list endpoint doesn't + * compute. Left at 0 until the API gains the column / sub-select + * (tracked as a follow-up in #515). + */ +export function adaptApiPost(p: ApiPost): PostListResponse['posts'][number] { + const apiName = + p.author?.display_name ?? p.author?.displayName ?? ''; + const authorId = p.author?.id ?? p.author_id ?? ''; + return { + id: p.id, + title: p.title ?? '(untitled)', + status: + (p.status as PostListResponse['posts'][number]['status']) ?? 'draft', + date: p.published_at ?? p.updated_at ?? p.created_at ?? '', + author: { + id: authorId, + displayName: apiName || (authorId ? shortAuthorId(authorId) : ''), + }, + // Comments aggregate isn't part of the list payload — would + // require a SELECT COUNT(*) join we don't run yet. See #515. + commentsCount: 0, + }; +} + /** Loading skeleton for the Suspense fallback. */ function PostsSkeleton(): ReactElement { return ( @@ -101,15 +161,6 @@ async function fetchInitialPosts(): Promise<{ // `{posts, nextCursor, total}` form with a flatter Post shape, so // adapt here. Be defensive: a missing field shouldn't crash the // page (issue #76 — contract still evolving). - type ApiPost = { - id: string; - title: string; - status: string; - published_at?: string | null; - updated_at?: string; - created_at?: string; - author_id?: string; - }; type ApiEnvelope = { data?: ApiPost[]; posts?: ApiPost[]; @@ -129,14 +180,7 @@ async function fetchInitialPosts(): Promise<{ json.nextCursor ?? null; // Map ApiPost → the flatter Post shape the UI expects. - const posts = rows.map((p) => ({ - id: p.id, - title: p.title ?? '(untitled)', - status: (p.status as PostListResponse['posts'][number]['status']) ?? 'draft', - date: p.published_at ?? p.updated_at ?? p.created_at ?? '', - author: { id: p.author_id ?? '', displayName: '' }, - commentsCount: 0, - })); + const posts = rows.map(adaptApiPost); return { data: { posts: posts as PostListResponse['posts'], diff --git a/apps/api/cmd/server/main.go b/apps/api/cmd/server/main.go index a0f81dd2..d7a121e6 100644 --- a/apps/api/cmd/server/main.go +++ b/apps/api/cmd/server/main.go @@ -792,18 +792,28 @@ func buildRouter(cfg *config.Config, pool *pgxpool.Pool, rdb *goredis.Client, se // A read error is logged and surfaced as "no active theme" — the // handler turns that into a 404 rather than a 500 because a // transient DB hiccup should not bring down public CSS. + // + // The raw resolver hits Postgres on every call. Public CSS traffic + // scales with page views, so we wrap it in a 60s TTL cache (issue + // #526). The cache is best-effort: theme switches will lag by up + // to one TTL until a follow-up wires Invalidate() through the + // admin activate handler. Sixty seconds is short enough that an + // operator never waits long for a theme swap to propagate and long + // enough that bursty traffic doesn't punch through to the DB. + rawThemeActiveResolver := func() string { + slug, err := themeActiveStore.Get(context.Background()) + if err != nil { + logger.Debug("themes/static: active resolver failed", + slog.Any("err", err)) + return "" + } + return slug + } + cachedThemeActiveResolver := themesstatic.NewCachedResolver(rawThemeActiveResolver, 60*time.Second) if err := themesstatic.Mount(mux, "/themes", themesstatic.Deps{ - ThemeDir: themeDir, - ActiveResolver: func() string { - slug, err := themeActiveStore.Get(context.Background()) - if err != nil { - logger.Debug("themes/static: active resolver failed", - slog.Any("err", err)) - return "" - } - return slug - }, - Logger: logger, + ThemeDir: themeDir, + ActiveResolver: cachedThemeActiveResolver.Get, + Logger: logger, }); err != nil { logger.Warn("themes/static: failed to mount", slog.Any("err", err)) } else { @@ -1497,17 +1507,35 @@ func buildRouter(cfg *config.Config, pool *pgxpool.Pool, rdb *goredis.Client, se } if jobsInspector == nil { logger.Warn("admin/jobs: skipping mount; asynq inspector unavailable") - } else if err := adminjobs.Mount(mux, "/api/v1/admin/jobs", adminjobs.Deps{ - Inspector: jobsInspectorAdapter{insp: jobsInspector}, - Redactions: adminjobs.NewMemoryRedactionStore(), - Policy: adminPolicy, - Logger: logger, - }); err != nil { - logger.Warn("admin/jobs: failed to mount", slog.Any("err", err)) } else { - logger.Info("admin/jobs: routes mounted", - slog.String("base", "/api/v1/admin/jobs"), - ) + // Mount on a sub-mux and wrap with RequireSession so the handler's + // gate() sees a principal on context. The global middleware chain + // no longer carries OptionalSession (the regression fixed in #31), + // so admin endpoints have to do their own session validation — + // mirroring how /api/v1/settings and /api/v1/auth/sessions wire + // themselves. Without this wrap, every admin/jobs request landed + // at the gate with no principal and returned 401, hiding the + // inspector behind a permanent auth wall (issue #502). + jobsMux := http.NewServeMux() + if err := adminjobs.Mount(jobsMux, "/api/v1/admin/jobs", adminjobs.Deps{ + Inspector: jobsInspectorAdapter{insp: jobsInspector}, + Redactions: adminjobs.NewMemoryRedactionStore(), + Policy: adminPolicy, + Logger: logger, + }); err != nil { + logger.Warn("admin/jobs: failed to mount", slog.Any("err", err)) + } else { + var jobsHandler http.Handler = jobsMux + if sessions != nil { + jobsHandler = authmw.RequireSession(sessions)(jobsMux) + } else { + logger.Warn("admin/jobs: session manager nil; mounting without RequireSession") + } + mux.Handle("/api/v1/admin/jobs/", jobsHandler) + logger.Info("admin/jobs: routes mounted", + slog.String("base", "/api/v1/admin/jobs"), + ) + } } // Admin marketplace surface (/api/v1/admin/marketplace). The diff --git a/apps/api/internal/admin/comments/list.go b/apps/api/internal/admin/comments/list.go index b913fb60..dda538ba 100644 --- a/apps/api/internal/admin/comments/list.go +++ b/apps/api/internal/admin/comments/list.go @@ -6,8 +6,21 @@ import ( "github.com/Singleton-Solution/GoNext/apps/api/internal/rest/router" "github.com/Singleton-Solution/GoNext/packages/go/policy" + "github.com/Singleton-Solution/GoNext/packages/go/util/queryparse" ) +// validCommentStatuses is the lookup table queryparse.ParseStatus +// uses to validate the ?status= query parameter on the list endpoint. +// Built once at package load from AllStatuses so the source of truth +// for the moderation enum stays in model.go. +var validCommentStatuses = func() map[string]struct{} { + m := make(map[string]struct{}, len(AllStatuses)) + for _, s := range AllStatuses { + m[string(s)] = struct{}{} + } + return m +}() + // listResponse is the envelope returned by GET /api/v1/admin/comments. // We reuse router.Page so the shape matches the rest of the admin // surface (posts, jobs, etc.). Cursor encoding: a plain page number @@ -35,15 +48,16 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request, _ policy.Princip // Status filter. Empty string and the literal "any" both mean // "no filter"; any other value must be in AllStatuses or we 400 // so the client doesn't accidentally typo "approve" (the bulk - // verb) instead of "approved" (the state). - if s := q.Get("status"); s != "" && s != "any" { - st := Status(s) - if !IsValidStatus(st) { - router.WriteError(w, http.StatusBadRequest, "invalid_status", - "status must be one of pending, approved, spam, trash") - return - } - filter.Status = st + // verb) instead of "approved" (the state). queryparse.ParseStatus + // owns the alias rule so the three list endpoints can't drift. + parsedStatus, err := queryparse.ParseStatus(q.Get("status"), validCommentStatuses) + if err != nil { + router.WriteError(w, http.StatusBadRequest, "invalid_status", + "status must be one of pending, approved, spam, trash") + return + } + if parsedStatus != "" { + filter.Status = Status(parsedStatus) } if pid := q.Get("post_id"); pid != "" { diff --git a/apps/api/internal/admin/jobs/handler.go b/apps/api/internal/admin/jobs/handler.go index a2be53ee..e9539151 100644 --- a/apps/api/internal/admin/jobs/handler.go +++ b/apps/api/internal/admin/jobs/handler.go @@ -205,6 +205,19 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request, _ policy.Princip // its list response; the overshoot is the standard trick. tasks, err := h.inspector.ListArchivedTasks(queue, asynq.PageSize(limit+1), asynq.Page(page)) if err != nil { + // On a freshly-booted system the queue's Redis keys don't exist + // yet because no task has ever been enqueued to it. Asynq treats + // that as ErrQueueNotFound, but for the admin DLQ surface it's + // semantically "no archived tasks" — return an empty page rather + // than a 500. Without this, the admin Jobs page renders its + // FailureState UI on a clean install (issue #502). + if errors.Is(err, asynq.ErrQueueNotFound) { + router.WriteJSON(w, http.StatusOK, router.Page[ArchivedTask]{ + Data: []ArchivedTask{}, + Pagination: router.PageInfo{}, + }) + return + } h.logger.ErrorContext(r.Context(), "admin/jobs: list failed", slog.String("queue", queue), slog.Any("err", err), diff --git a/apps/api/internal/admin/jobs/handler_test.go b/apps/api/internal/admin/jobs/handler_test.go index a1b41529..93243453 100644 --- a/apps/api/internal/admin/jobs/handler_test.go +++ b/apps/api/internal/admin/jobs/handler_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io" "log/slog" "net/http" @@ -313,6 +314,41 @@ func TestList_InspectorError(t *testing.T) { } } +// TestList_QueueNotFoundIsEmpty pins the fix for issue #502: a clean +// install has never enqueued anything to the "default" queue, so Asynq's +// ListArchivedTasks returns an error wrapping ErrQueueNotFound. The +// handler must translate that into an empty page (200), not a 500. +// Otherwise the admin Jobs page renders its FailureState UI for every +// fresh deployment. +func TestList_QueueNotFoundIsEmpty(t *testing.T) { + h := newTestHarness(t) + // Asynq wraps ErrQueueNotFound with fmt.Errorf, so mirror that here + // — errors.Is must still report a match. + h.inspector.listErr = fmt.Errorf("asynq: %w", asynq.ErrQueueNotFound) + + req := httptest.NewRequest("GET", "/api/v1/admin/jobs/dlq?queue=default", nil) + rec := h.do(req, ptr(adminPrincipal())) + if rec.Code != http.StatusOK { + t.Fatalf("status: got %d, want 200; body=%s", rec.Code, rec.Body.String()) + } + var page router.Page[ArchivedTask] + if err := json.Unmarshal(rec.Body.Bytes(), &page); err != nil { + t.Fatalf("unmarshal: %v; body=%s", err, rec.Body.String()) + } + if len(page.Data) != 0 { + t.Errorf("data: got %d, want 0", len(page.Data)) + } + // Empty page must serialise as [] (not null) so the UI can render it + // without a nil-check; router.Page already guarantees this, but pin + // the wire shape here too. + if !strings.Contains(rec.Body.String(), `"data":[]`) { + t.Errorf("body: want data:[] in body, got %s", rec.Body.String()) + } + if page.Pagination.NextCursor != "" { + t.Errorf("next_cursor: got %q, want empty", page.Pagination.NextCursor) + } +} + // ----------------------------------------------------------------------------- // AUTH // ----------------------------------------------------------------------------- diff --git a/apps/api/internal/admin/pluginpages/handler_test.go b/apps/api/internal/admin/pluginpages/handler_test.go index fdabe6f1..688b567c 100644 --- a/apps/api/internal/admin/pluginpages/handler_test.go +++ b/apps/api/internal/admin/pluginpages/handler_test.go @@ -120,3 +120,29 @@ func TestList_AnonymousDenied(t *testing.T) { t.Fatalf("expected 401, got %d", rec.Code) } } + +// Clean install: no plugins are installed yet, so the underlying +// lifecycle.List call returns an empty slice. The sidebar polls this +// endpoint on every authenticated layout render; it must respond 200 +// with {"pages":[]} rather than failing the layout. Regression for +// issue #503. +func TestList_EmptyOnCleanInstall(t *testing.T) { + mux := newHarness(t, nil) + req := httptest.NewRequest(http.MethodGet, "/api/v1/admin/plugin-pages", nil) + req = req.WithContext(policy.WithPrincipal(req.Context(), policy.Principal{UserID: "u:1"})) + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) + } + var resp listResponse + if err := json.NewDecoder(rec.Body).Decode(&resp); err != nil { + t.Fatalf("decode: %v", err) + } + if resp.Pages == nil { + t.Fatal("Pages should be a non-nil empty slice so JSON encodes as [], not null") + } + if len(resp.Pages) != 0 { + t.Fatalf("expected zero pages, got %d: %+v", len(resp.Pages), resp.Pages) + } +} diff --git a/apps/api/internal/admin/search/handler.go b/apps/api/internal/admin/search/handler.go index 370102d2..1f807680 100644 --- a/apps/api/internal/admin/search/handler.go +++ b/apps/api/internal/admin/search/handler.go @@ -9,10 +9,26 @@ import ( "strings" "github.com/Singleton-Solution/GoNext/apps/api/internal/rest/router" - pkgsearch "github.com/Singleton-Solution/GoNext/packages/go/search" "github.com/Singleton-Solution/GoNext/packages/go/policy" + pkgsearch "github.com/Singleton-Solution/GoNext/packages/go/search" + "github.com/Singleton-Solution/GoNext/packages/go/util/queryparse" ) +// validSearchStatuses is the closed set the admin search endpoint +// accepts for ?status=. Matches the post_status enum in +// 000001_init.up.sql — searchable rows live in the posts table, so +// the valid filter values are the same ones REST /api/v1/posts +// accepts. queryparse.ParseStatus also recognises "" and "any" as +// "no filter" before consulting this set. +var validSearchStatuses = map[string]struct{}{ + "draft": {}, + "pending": {}, + "published": {}, + "scheduled": {}, + "private": {}, + "trash": {}, +} + // Searcher is the read-only contract the handler needs. The // concrete *pkgsearch.Store satisfies it. Keeping the interface // local lets unit tests stub one method without bringing the SQL @@ -66,7 +82,19 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - opts := parseOpts(r) + opts, err := parseOpts(r) + if err != nil { + // parseOpts only fails on invalid_status today; route everything + // through a single branch so future validations can extend + // parseOpts without growing the handler. + if errors.Is(err, queryparse.ErrInvalidStatus) { + router.WriteError(w, http.StatusBadRequest, "invalid_status", + "status must be one of draft, pending, published, scheduled, private, trash (or omitted / 'any')") + return + } + router.WriteError(w, http.StatusBadRequest, "invalid_request", err.Error()) + return + } res, err := h.search.Search(r.Context(), q, opts) if err != nil { @@ -88,10 +116,18 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // parseOpts maps URL query params onto a SearchOpts. The admin // surface deliberately does NOT default-filter by status; callers // who want only published rows pass &status=published explicitly. -func parseOpts(r *http.Request) pkgsearch.SearchOpts { +// The "any" alias and the empty string both mean "no filter"; an +// unknown status returns queryparse.ErrInvalidStatus so the handler +// surfaces a 400 instead of slipping it into the SQL parameter and +// returning empty results. +func parseOpts(r *http.Request) (pkgsearch.SearchOpts, error) { q := r.URL.Query() + status, err := queryparse.ParseStatus(q.Get("status"), validSearchStatuses) + if err != nil { + return pkgsearch.SearchOpts{}, err + } opts := pkgsearch.SearchOpts{ - Status: q.Get("status"), + Status: status, } if t := q.Get("types"); t != "" { // Split on comma so the typical "?types=post,page" works. @@ -114,7 +150,7 @@ func parseOpts(r *http.Request) pkgsearch.SearchOpts { opts.Offset = n } } - return opts + return opts, nil } // Mount registers the handler at base + "/search" behind the diff --git a/apps/api/internal/rest/posts/handlers.go b/apps/api/internal/rest/posts/handlers.go index bc4f9d3a..fc0ddedf 100644 --- a/apps/api/internal/rest/posts/handlers.go +++ b/apps/api/internal/rest/posts/handlers.go @@ -14,6 +14,7 @@ import ( "github.com/Singleton-Solution/GoNext/apps/api/internal/rest/router" "github.com/Singleton-Solution/GoNext/packages/go/audit" "github.com/Singleton-Solution/GoNext/packages/go/policy" + "github.com/Singleton-Solution/GoNext/packages/go/util/queryparse" ) // HeaderPostPassword is the request header carrying the password for @@ -155,19 +156,18 @@ func (h *handlers) list(w http.ResponseWriter, r *http.Request, pr policy.Princi func parseListQuery(r *http.Request) (ListFilter, error) { q := r.URL.Query() var f ListFilter - f.Status = q.Get("status") // `status=any` is a documented client convention meaning "all // statuses" — the admin posts page uses it to render drafts + - // published in one list. Treat it as the absence of a status - // filter rather than a validation error. Empty string is the same. - if f.Status == "any" { - f.Status = "" - } - if f.Status != "" { - if _, ok := validStatuses[f.Status]; !ok { - return f, validation{Code: "invalid_status", Detail: fmt.Sprintf("unknown status %q", f.Status)} - } + // published in one list. The shared queryparse helper normalises + // the alias and the empty string to "" so we can drop the filter + // downstream without a separate branch. Anything else must be in + // validStatuses or we 400. + rawStatus := q.Get("status") + status, err := queryparse.ParseStatus(rawStatus, validStatuses) + if err != nil { + return f, validation{Code: "invalid_status", Detail: fmt.Sprintf("unknown status %q", rawStatus)} } + f.Status = status f.AuthorID = q.Get("author") f.Search = q.Get("search") diff --git a/apps/api/internal/themes/static/cache.go b/apps/api/internal/themes/static/cache.go new file mode 100644 index 00000000..33b80df9 --- /dev/null +++ b/apps/api/internal/themes/static/cache.go @@ -0,0 +1,85 @@ +package static + +import ( + "sync" + "time" +) + +// CachedResolver wraps an ActiveResolver with a TTL-based cache so +// repeated CSS requests don't hammer Postgres. The cache is +// invalidated either by TTL expiry or by an explicit Invalidate() +// call (wired from the theme-activate handler in a follow-up). +// +// The implementation favors the read path: the common case is a cache +// hit served under an RLock with no allocation. On miss (or expiry) +// we drop to the write lock, re-check the deadline under it (a peer +// goroutine may have refilled between RUnlock and Lock), and call the +// inner resolver exactly once per expiry window. The inner resolver's +// result — even the empty string the production wiring uses to signal +// "no active theme / read error" — is cached verbatim; we deliberately +// do NOT special-case empty so a transient DB hiccup doesn't degrade +// to "hammer Postgres until it recovers." +// +// Invalidate is best-effort: it resets the deadline to zero so the +// next Get() forces a refill. It does NOT block on an in-flight +// refill; callers (the admin activate handler) treat it as a fire-and +// -forget signal. +type CachedResolver struct { + inner ActiveResolver + mu sync.RWMutex + value string + until time.Time + ttl time.Duration + now func() time.Time // injectable for tests +} + +// NewCachedResolver returns a CachedResolver that memoizes the result +// of inner for ttl. A ttl of zero or negative disables caching (every +// Get hits inner) — useful for tests that want to exercise the inner +// call path without rebuilding the wrapper. +func NewCachedResolver(inner ActiveResolver, ttl time.Duration) *CachedResolver { + return &CachedResolver{ + inner: inner, + ttl: ttl, + now: time.Now, + } +} + +// Get returns the memoized active-theme slug, refilling from inner if +// the TTL has expired or Invalidate was called since the last refill. +func (c *CachedResolver) Get() string { + // Fast path: read under RLock. The common case is a cache hit and + // we want zero contention with peer readers. + c.mu.RLock() + if c.ttl > 0 && c.now().Before(c.until) { + v := c.value + c.mu.RUnlock() + return v + } + c.mu.RUnlock() + + // Slow path: refill. Take the write lock and re-check the deadline + // — a peer goroutine may have refilled between our RUnlock and + // Lock, in which case we serve their cached value rather than + // piling a second inner call onto the same expiry window. + c.mu.Lock() + defer c.mu.Unlock() + if c.ttl > 0 && c.now().Before(c.until) { + return c.value + } + c.value = c.inner() + if c.ttl > 0 { + c.until = c.now().Add(c.ttl) + } + return c.value +} + +// Invalidate forces the next Get() call to bypass the cache and +// re-invoke inner. Safe to call concurrently with Get; the worst case +// is a Get that started before Invalidate returns a stale value, but +// the next Get sees the fresh one. +func (c *CachedResolver) Invalidate() { + c.mu.Lock() + c.until = time.Time{} + c.mu.Unlock() +} diff --git a/apps/api/internal/themes/static/cache_test.go b/apps/api/internal/themes/static/cache_test.go new file mode 100644 index 00000000..973d3eca --- /dev/null +++ b/apps/api/internal/themes/static/cache_test.go @@ -0,0 +1,234 @@ +package static + +import ( + "sync" + "sync/atomic" + "testing" + "time" +) + +// newCounter returns an ActiveResolver that records call count and +// returns the given slug on every invocation. Tests inspect the +// counter to assert how many times the inner resolver was actually +// hit — the whole point of the cache is to keep this number small. +func newCounter(slug string) (ActiveResolver, *int64) { + var n int64 + return func() string { + atomic.AddInt64(&n, 1) + return slug + }, &n +} + +func TestCachedResolver_HitReturnsMemoizedValue(t *testing.T) { + inner, calls := newCounter("gn-hello") + c := NewCachedResolver(inner, time.Minute) + + // First call populates the cache (inner fires once). + if got := c.Get(); got != "gn-hello" { + t.Fatalf("first Get: got %q, want gn-hello", got) + } + // Subsequent calls within the TTL must not hit inner. + for i := 0; i < 100; i++ { + if got := c.Get(); got != "gn-hello" { + t.Fatalf("iter %d: got %q, want gn-hello", i, got) + } + } + if n := atomic.LoadInt64(calls); n != 1 { + t.Errorf("inner calls: got %d, want 1 (100 cached reads should not refill)", n) + } +} + +func TestCachedResolver_ExpiresAfterTTL(t *testing.T) { + inner, calls := newCounter("gn-hello") + c := NewCachedResolver(inner, 100*time.Millisecond) + + // Inject a controllable clock so the test is deterministic and + // doesn't actually sleep. The cache uses c.now() everywhere. + var nowVal time.Time + c.now = func() time.Time { return nowVal } + nowVal = time.Unix(1_000_000, 0) + + // First call: cold cache, fires inner, sets until = now + ttl. + if got := c.Get(); got != "gn-hello" { + t.Fatalf("first Get: got %q, want gn-hello", got) + } + if n := atomic.LoadInt64(calls); n != 1 { + t.Fatalf("after first Get: inner calls = %d, want 1", n) + } + + // Advance clock half the TTL — still inside the window, no refill. + nowVal = nowVal.Add(50 * time.Millisecond) + if got := c.Get(); got != "gn-hello" { + t.Fatalf("mid-ttl Get: got %q", got) + } + if n := atomic.LoadInt64(calls); n != 1 { + t.Errorf("mid-ttl: inner calls = %d, want 1 (still cached)", n) + } + + // Advance past the TTL — next Get must refill. + nowVal = nowVal.Add(200 * time.Millisecond) + if got := c.Get(); got != "gn-hello" { + t.Fatalf("post-ttl Get: got %q", got) + } + if n := atomic.LoadInt64(calls); n != 2 { + t.Errorf("post-ttl: inner calls = %d, want 2 (refill on expiry)", n) + } +} + +func TestCachedResolver_InvalidateForcesRefill(t *testing.T) { + inner, calls := newCounter("gn-hello") + c := NewCachedResolver(inner, time.Hour) + + // Populate the cache. + _ = c.Get() + _ = c.Get() + if n := atomic.LoadInt64(calls); n != 1 { + t.Fatalf("after 2 Gets: inner calls = %d, want 1", n) + } + + // Invalidate, then Get — must hit inner. + c.Invalidate() + _ = c.Get() + if n := atomic.LoadInt64(calls); n != 2 { + t.Errorf("after Invalidate + Get: inner calls = %d, want 2", n) + } + + // Subsequent Gets within the (refreshed) TTL are cached again. + _ = c.Get() + _ = c.Get() + if n := atomic.LoadInt64(calls); n != 2 { + t.Errorf("after Invalidate + 3 Gets: inner calls = %d, want 2", n) + } +} + +func TestCachedResolver_InvalidateBeforeFirstGet(t *testing.T) { + // Invalidate on a fresh resolver must not crash and must not + // confuse the first Get: it should still hit inner exactly once + // and then memoize. + inner, calls := newCounter("gn-hello") + c := NewCachedResolver(inner, time.Minute) + + c.Invalidate() + if got := c.Get(); got != "gn-hello" { + t.Fatalf("first Get after Invalidate: got %q", got) + } + _ = c.Get() + if n := atomic.LoadInt64(calls); n != 1 { + t.Errorf("inner calls = %d, want 1", n) + } +} + +func TestCachedResolver_ZeroTTLAlwaysHitsInner(t *testing.T) { + // A non-positive TTL disables caching — every Get calls inner. + // Useful for callers who want to opt out without rebuilding the + // dependency wiring. + inner, calls := newCounter("gn-hello") + c := NewCachedResolver(inner, 0) + + for i := 0; i < 5; i++ { + if got := c.Get(); got != "gn-hello" { + t.Fatalf("iter %d: got %q", i, got) + } + } + if n := atomic.LoadInt64(calls); n != 5 { + t.Errorf("inner calls = %d, want 5 (ttl=0 disables cache)", n) + } +} + +func TestCachedResolver_CachesEmptyValue(t *testing.T) { + // The production wiring returns "" on a DB read error and the + // handler treats that as "no active theme → 404". We deliberately + // cache the empty string for the TTL so a transient DB hiccup + // doesn't degrade to "hammer Postgres on every request until it + // recovers." Invalidate is the explicit escape hatch. + inner, calls := newCounter("") + c := NewCachedResolver(inner, time.Minute) + + for i := 0; i < 10; i++ { + if got := c.Get(); got != "" { + t.Fatalf("iter %d: got %q, want empty", i, got) + } + } + if n := atomic.LoadInt64(calls); n != 1 { + t.Errorf("inner calls = %d, want 1 (empty values cache too)", n) + } +} + +func TestCachedResolver_ConcurrentAccess(t *testing.T) { + // Race-detector workout: many goroutines hit Get concurrently with + // occasional Invalidate calls. We don't assert an exact inner- + // call count (the timing is racy by design) — the contract is + // (a) every Get returns the configured slug, and + // (b) the inner count stays in a sane bound. + // Run with `go test -race` to catch any unguarded access. + inner, calls := newCounter("gn-hello") + c := NewCachedResolver(inner, 10*time.Millisecond) + + const readers = 64 + const itersPerReader = 500 + + var wg sync.WaitGroup + wg.Add(readers) + for i := 0; i < readers; i++ { + go func(i int) { + defer wg.Done() + for j := 0; j < itersPerReader; j++ { + if got := c.Get(); got != "gn-hello" { + t.Errorf("reader %d iter %d: got %q", i, j, got) + return + } + // One in 50 iterations, force an invalidation. This + // mimics the production pattern where the admin + // activate handler signals a switch — rare relative + // to public CSS reads. + if j%50 == 0 { + c.Invalidate() + } + } + }(i) + } + wg.Wait() + + // Sanity check: the inner resolver must have been called at least + // once (cold start) but well below the total Get count (caching + // should have absorbed the vast majority). The exact upper bound + // depends on goroutine scheduling and the 10ms TTL; 64*500=32000 + // total Gets, and inner should have fired at most a few hundred + // times. We pick a generous ceiling so the test isn't flaky on a + // slow CI box. + total := int64(readers * itersPerReader) + got := atomic.LoadInt64(calls) + if got < 1 { + t.Errorf("inner never called (counter = %d)", got) + } + if got >= total { + t.Errorf("cache ineffective: inner called %d / %d times", got, total) + } +} + +func TestCachedResolver_TracksValueChangesAfterInvalidate(t *testing.T) { + // The inner resolver returns a different slug on each call. The + // cache should pin the first value until Invalidate fires, then + // pick up the next inner result. + var n int64 + inner := func() string { + v := atomic.AddInt64(&n, 1) + if v == 1 { + return "gn-hello" + } + return "gn-pro" + } + c := NewCachedResolver(inner, time.Hour) + + if got := c.Get(); got != "gn-hello" { + t.Fatalf("first Get: got %q, want gn-hello", got) + } + if got := c.Get(); got != "gn-hello" { + t.Errorf("cached Get: got %q, want gn-hello (still cached)", got) + } + + c.Invalidate() + if got := c.Get(); got != "gn-pro" { + t.Errorf("post-invalidate Get: got %q, want gn-pro", got) + } +} diff --git a/cli/gonext/cmd/migrate/migrate.go b/cli/gonext/cmd/migrate/migrate.go index 1ae7624c..a7bc76c4 100644 --- a/cli/gonext/cmd/migrate/migrate.go +++ b/cli/gonext/cmd/migrate/migrate.go @@ -36,6 +36,10 @@ Subcommands: --seed-default-theme=BOOL Install the bundled default theme (gn-hello) on first boot. Default: true. + --seed-themes=BOOL Copy image-bundled themes + into the runtime themes + volume on first boot. + Default: true. down [N] Roll back N migrations (default 1). Pass 0 to roll back ALL. to Migrate up or down to reach the given version (positive integer matching the migration filename prefix). @@ -46,10 +50,15 @@ Subcommands: See 'migrate replacements --help'. Environment: - DATABASE_URL Required. Postgres DSN. - GONEXT_MIGRATION_DIR Migration directory. Default: ./migrations. - GONEXT_THEME_DIR Runtime theme directory used by the seeder. - Default: ./themes. + DATABASE_URL Required. Postgres DSN. + GONEXT_MIGRATION_DIR Migration directory. Default: ./migrations. + GONEXT_THEME_DIR Runtime theme directory used by the DB + seeder (gn-hello). Default: ./themes. + GONEXT_BUNDLED_THEMES_DIR Source directory for the file seeder + (image-baked themes). Default: /themes. + GONEXT_VOLUME_THEMES_DIR Destination directory for the file seeder + (named-volume mount on the migrate + service). Default: /var/lib/gonext-themes. Exit codes: 0 success @@ -107,6 +116,8 @@ func runUp(args []string, stdout, stderr io.Writer) int { fs.SetOutput(stderr) seedDefaultTheme := fs.Bool("seed-default-theme", true, "install the bundled default theme (gn-hello) on first boot") + seedThemesFlag := fs.Bool("seed-themes", true, + "copy image-bundled themes into the runtime themes volume on first boot") if err := fs.Parse(args); err != nil { // flag.ContinueOnError already prints the error; emit usage so // the operator sees the surrounding command help too. @@ -129,6 +140,24 @@ func runUp(args []string, stdout, stderr io.Writer) int { } fmt.Fprintln(stdout, "migrate: up OK") + // File seed first: copy image-bundled themes into the named volume + // before the DB seeder runs so the gn-hello directory exists on + // disk by the time the renderer wakes up and resolves the + // active_theme slug. The two seeders are deliberately separate — + // the file seed is operator-facing (what the renderer reads), the + // DB seed is product-facing (which theme is active). + if *seedThemesFlag { + src := resolveBundledThemesDir() + dst := resolveVolumeThemesDir() + if err := seedThemes(src, dst, logger); err != nil { + fmt.Fprintf(stderr, "gonext migrate up: seed themes: %v\n", err) + return ExitFail + } + fmt.Fprintln(stdout, "migrate: themes seed OK") + } else { + logger.Info("themes file seed skipped via --seed-themes=false") + } + if !*seedDefaultTheme { logger.Info("theme seed skipped via --seed-default-theme=false") return ExitOK diff --git a/cli/gonext/cmd/migrate/themes.go b/cli/gonext/cmd/migrate/themes.go new file mode 100644 index 00000000..92f53da6 --- /dev/null +++ b/cli/gonext/cmd/migrate/themes.go @@ -0,0 +1,238 @@ +package migrate + +import ( + "errors" + "fmt" + "io" + "io/fs" + "log/slog" + "os" + "path/filepath" +) + +// Default paths used by seedThemes. They map to the layout described in +// docs/09-deployment-ops.md and the Compose overlay: +// +// - DefaultBundledThemesDir is where the cli/gonext Dockerfile drops +// the canonical /themes payload (image-baked, read-only at runtime). +// - DefaultVolumeThemesDir is the path the migrate service mounts the +// api-themes named volume at, so the seeded files persist across +// `compose down -v` and become visible to the api container (which +// mounts the same volume at /themes). +// +// Both are overridable via env so an operator running the CLI outside +// Compose (bare-metal, kube initContainer with a different mount path) +// doesn't have to rebuild the image. +const ( + DefaultBundledThemesDir = "/themes" + DefaultVolumeThemesDir = "/var/lib/gonext-themes" + + // EnvBundledThemesDir overrides DefaultBundledThemesDir. Set this + // when the source themes live somewhere other than /themes (e.g. a + // dev-loop bind mount). + EnvBundledThemesDir = "GONEXT_BUNDLED_THEMES_DIR" + + // EnvVolumeThemesDir overrides DefaultVolumeThemesDir. Set this when + // the persistent volume is mounted at a non-default path (e.g. kube + // initContainer that uses /mnt/themes). + EnvVolumeThemesDir = "GONEXT_VOLUME_THEMES_DIR" +) + +// seedThemes copies the image-bundled themes tree at src into dst so a +// freshly-created named volume becomes immediately usable by the api +// container without an out-of-band `docker run --rm alpine cp` step. +// +// Idempotence is by destination directory, not by content hash: +// +// - If dst already contains at least one theme directory (a child dir +// with theme.json at its root), seedThemes is a no-op. Operators +// who curate the volume — child themes, hand-rolled overrides, a +// production theme rolled out via the admin UI — are protected. +// The single switch they need to flip to *re-seed* is wiping the +// volume. +// - If dst is empty (or contains only non-theme cruft like a lost+found +// directory), every theme under src is copied verbatim. The seeder +// does not merge. +// +// This intentionally does NOT touch the options-table active-theme row. +// That row is owned by the database-level seeder in +// packages/go/theme/seed; this function only ensures the renderer- +// readable filesystem has the bytes available. The two seeders are +// composed in runUp so the order is: SQL migrations → file seed → +// embedded DB seed (gn-hello via active_theme row). +// +// Errors are wrapped with the offending path so an operator sees +// "seed-themes: copy gn-hello/templates/index.html: permission denied" +// rather than a bare errno. +func seedThemes(src, dst string, logger *slog.Logger) error { + if logger == nil { + logger = slog.Default() + } + logger = logger.With(slog.String("component", "migrate.seed-themes"), + slog.String("src", src), slog.String("dst", dst)) + + // Source must exist — the cli/gonext Dockerfile is supposed to + // drop the themes tree there. If it's missing we treat it as a + // build-time bug, not a runtime no-op: surface the error so the + // operator knows to rebuild the image. + srcInfo, err := os.Stat(src) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + logger.Warn("bundled themes directory missing; skipping seed", + slog.String("hint", "rebuild cli/gonext image; expected COPY themes/ /themes")) + return nil + } + return fmt.Errorf("stat bundled themes dir: %w", err) + } + if !srcInfo.IsDir() { + return fmt.Errorf("bundled themes path %q is not a directory", src) + } + + // Destination is created with parents so a fresh volume mount lands + // cleanly. 0o755 lets the api service user traverse the tree at + // request time without needing extra perms. + if err := os.MkdirAll(dst, 0o755); err != nil { + return fmt.Errorf("mkdir volume themes dir: %w", err) + } + + populated, err := destinationHasThemes(dst) + if err != nil { + return fmt.Errorf("inspect volume themes dir: %w", err) + } + if populated { + logger.Info("themes volume already populated; skipping seed") + return nil + } + + entries, err := os.ReadDir(src) + if err != nil { + return fmt.Errorf("read bundled themes dir: %w", err) + } + + seeded := 0 + for _, entry := range entries { + // We only seed theme directories. Top-level files in the bundle + // (e.g. README.md) are documentation, not payload — copying + // them would pollute the renderer's slug enumeration. + if !entry.IsDir() { + continue + } + slug := entry.Name() + srcTheme := filepath.Join(src, slug) + // Sanity: the directory must actually look like a theme. + // Without theme.json the renderer would skip it anyway, so + // we filter here to keep the log line honest. + if _, statErr := os.Stat(filepath.Join(srcTheme, "theme.json")); statErr != nil { + logger.Debug("skipping non-theme directory under bundled themes", + slog.String("slug", slug)) + continue + } + dstTheme := filepath.Join(dst, slug) + if err := copyTree(srcTheme, dstTheme); err != nil { + return fmt.Errorf("copy theme %q: %w", slug, err) + } + logger.Info("seeded theme", slog.String("slug", slug)) + seeded++ + } + if seeded == 0 { + logger.Warn("no themes found under bundled themes dir; nothing seeded") + } + return nil +} + +// destinationHasThemes returns true when dst contains at least one +// child directory that looks like a theme (has a theme.json). We +// don't gate on "is the directory empty" because container-managed +// volumes can ship with a lost+found stub or .gitkeep that would +// fool a naive emptiness check into bailing out forever. +func destinationHasThemes(dst string) (bool, error) { + entries, err := os.ReadDir(dst) + if err != nil { + return false, err + } + for _, entry := range entries { + if !entry.IsDir() { + continue + } + if _, err := os.Stat(filepath.Join(dst, entry.Name(), "theme.json")); err == nil { + return true, nil + } + } + return false, nil +} + +// copyTree mirrors srcDir into dstDir, preserving the relative layout. +// Modes are normalised: directories at 0o755, files at 0o644. We don't +// preserve the source mode because the bundled image layer may carry +// over the build-time umask (root-owned 0o600 after the Dockerfile's +// --chown), and the api service user needs read access at runtime. +// +// Symlinks are not expected in the bundle; if encountered they're +// resolved to their target (Stat, not Lstat). This matches the rest of +// the theme tooling, which treats theme directories as plain trees. +func copyTree(srcDir, dstDir string) error { + return filepath.WalkDir(srcDir, func(path string, d fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + rel, err := filepath.Rel(srcDir, path) + if err != nil { + return fmt.Errorf("rel %q: %w", path, err) + } + target := filepath.Join(dstDir, rel) + if d.IsDir() { + if err := os.MkdirAll(target, 0o755); err != nil { + return fmt.Errorf("mkdir %q: %w", target, err) + } + return nil + } + return copyFile(path, target) + }) +} + +// copyFile streams src → dst with a fresh 0o644 mode. We stream rather +// than ReadFile/WriteFile so an unusually large theme asset (a hero +// image, a webfont) doesn't have to materialise in memory. +func copyFile(src, dst string) error { + in, err := os.Open(src) //nolint:gosec // path comes from filepath.WalkDir over a controlled tree + if err != nil { + return fmt.Errorf("open src %q: %w", src, err) + } + defer in.Close() + + // O_TRUNC because callers have already gated on "destination is + // empty"; if we're here the file shouldn't exist yet, but being + // defensive against a partial prior run is cheap. + out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) + if err != nil { + return fmt.Errorf("open dst %q: %w", dst, err) + } + if _, err := io.Copy(out, in); err != nil { + out.Close() + return fmt.Errorf("copy %q -> %q: %w", src, dst, err) + } + if err := out.Close(); err != nil { + return fmt.Errorf("close dst %q: %w", dst, err) + } + return nil +} + +// resolveBundledThemesDir returns the source directory the seeder reads +// from. The env override takes precedence so an operator running the +// CLI outside Compose can point at a checkout. +func resolveBundledThemesDir() string { + if v := os.Getenv(EnvBundledThemesDir); v != "" { + return v + } + return DefaultBundledThemesDir +} + +// resolveVolumeThemesDir returns the destination directory the seeder +// writes to. The env override takes precedence so a kube initContainer +// with a different mount path is supported without code changes. +func resolveVolumeThemesDir() string { + if v := os.Getenv(EnvVolumeThemesDir); v != "" { + return v + } + return DefaultVolumeThemesDir +} diff --git a/cli/gonext/cmd/migrate/themes_test.go b/cli/gonext/cmd/migrate/themes_test.go new file mode 100644 index 00000000..97233014 --- /dev/null +++ b/cli/gonext/cmd/migrate/themes_test.go @@ -0,0 +1,234 @@ +package migrate + +import ( + "log/slog" + "os" + "path/filepath" + "testing" +) + +// makeBundledThemes builds a fixture under root that resembles the +// /themes tree the cli/gonext Dockerfile bakes into the image: a +// top-level README, plus N theme directories each carrying a +// theme.json and a couple of payload files. Returns the root path so +// callers can pass it as src to seedThemes. +func makeBundledThemes(t *testing.T, root string, slugs ...string) { + t.Helper() + if err := os.MkdirAll(root, 0o755); err != nil { + t.Fatalf("mkdir bundled root: %v", err) + } + // A top-level README that must NOT be copied — it's documentation, + // not a theme. seedThemes filters non-directory entries. + if err := os.WriteFile(filepath.Join(root, "README.md"), []byte("docs\n"), 0o644); err != nil { + t.Fatalf("write README: %v", err) + } + for _, slug := range slugs { + dir := filepath.Join(root, slug) + if err := os.MkdirAll(filepath.Join(dir, "templates"), 0o755); err != nil { + t.Fatalf("mkdir %q: %v", dir, err) + } + if err := os.WriteFile(filepath.Join(dir, "theme.json"), + []byte(`{"slug":"`+slug+`"}`), 0o644); err != nil { + t.Fatalf("write theme.json: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, "style.css"), + []byte("body{}"), 0o644); err != nil { + t.Fatalf("write style.css: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, "templates", "index.html"), + []byte(""), 0o644); err != nil { + t.Fatalf("write index.html: %v", err) + } + } +} + +func TestSeedThemes_CopiesEverythingIntoEmptyDst(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + makeBundledThemes(t, src, "gn-hello", "gn-pro") + + if err := seedThemes(src, dst, slog.Default()); err != nil { + t.Fatalf("seedThemes: %v", err) + } + + for _, slug := range []string{"gn-hello", "gn-pro"} { + if _, err := os.Stat(filepath.Join(dst, slug, "theme.json")); err != nil { + t.Errorf("missing %s/theme.json after seed: %v", slug, err) + } + if _, err := os.Stat(filepath.Join(dst, slug, "templates", "index.html")); err != nil { + t.Errorf("missing %s/templates/index.html after seed: %v", slug, err) + } + } + // README.md is documentation, not payload: it must not pollute the + // runtime volume. + if _, err := os.Stat(filepath.Join(dst, "README.md")); !os.IsNotExist(err) { + t.Errorf("README.md should not be copied to destination; err=%v", err) + } +} + +func TestSeedThemes_IsNoOpWhenDestinationHasThemes(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + makeBundledThemes(t, src, "gn-hello") + + // Pre-populate destination with an operator-curated theme. Its + // payload uses a sentinel value the bundled copy never carries — + // after a second seedThemes run we expect the sentinel to be + // intact (i.e. no overwrite). + curated := filepath.Join(dst, "gn-hello") + if err := os.MkdirAll(curated, 0o755); err != nil { + t.Fatalf("mkdir curated: %v", err) + } + sentinel := []byte("OPERATOR-CURATED") + if err := os.WriteFile(filepath.Join(curated, "theme.json"), sentinel, 0o644); err != nil { + t.Fatalf("write curated theme.json: %v", err) + } + + if err := seedThemes(src, dst, slog.Default()); err != nil { + t.Fatalf("seedThemes: %v", err) + } + + got, err := os.ReadFile(filepath.Join(curated, "theme.json")) + if err != nil { + t.Fatalf("read curated theme.json: %v", err) + } + if string(got) != string(sentinel) { + t.Errorf("curated theme overwritten: got=%q, want=%q", string(got), string(sentinel)) + } +} + +func TestSeedThemes_IgnoresNonThemeChildDirectories(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + makeBundledThemes(t, src, "gn-hello") + // A bare directory with no theme.json — must be skipped, not copied. + if err := os.MkdirAll(filepath.Join(src, "leftover"), 0o755); err != nil { + t.Fatalf("mkdir leftover: %v", err) + } + if err := os.WriteFile(filepath.Join(src, "leftover", "junk.txt"), + []byte("junk"), 0o644); err != nil { + t.Fatalf("write junk: %v", err) + } + + if err := seedThemes(src, dst, slog.Default()); err != nil { + t.Fatalf("seedThemes: %v", err) + } + if _, err := os.Stat(filepath.Join(dst, "leftover")); !os.IsNotExist(err) { + t.Errorf("non-theme directory %q should not be copied; err=%v", + "leftover", err) + } +} + +func TestSeedThemes_TolerantOfCruftInDestination(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + makeBundledThemes(t, src, "gn-hello") + // A file at the destination root (e.g. .gitkeep) and an empty + // directory (lost+found-style) must NOT inhibit the seed: they're + // not theme directories and a renderer would ignore them anyway. + if err := os.WriteFile(filepath.Join(dst, ".gitkeep"), nil, 0o644); err != nil { + t.Fatalf("write .gitkeep: %v", err) + } + if err := os.MkdirAll(filepath.Join(dst, "lost+found"), 0o755); err != nil { + t.Fatalf("mkdir lost+found: %v", err) + } + + if err := seedThemes(src, dst, slog.Default()); err != nil { + t.Fatalf("seedThemes: %v", err) + } + if _, err := os.Stat(filepath.Join(dst, "gn-hello", "theme.json")); err != nil { + t.Errorf("seed should have run despite cruft; missing gn-hello: %v", err) + } +} + +func TestSeedThemes_MissingSourceIsSoftWarning(t *testing.T) { + dst := t.TempDir() + // Point at a path that does not exist. seedThemes treats this as a + // soft warning (logged, no error returned) so an operator running + // the CLI outside the Compose image — where /themes is genuinely + // absent — doesn't see a hard boot failure. + src := filepath.Join(t.TempDir(), "does-not-exist") + if err := seedThemes(src, dst, slog.Default()); err != nil { + t.Errorf("missing source should be soft warning, got error: %v", err) + } +} + +func TestSeedThemes_SourceIsFile_HardError(t *testing.T) { + dst := t.TempDir() + srcFile := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(srcFile, []byte("x"), 0o644); err != nil { + t.Fatalf("write srcFile: %v", err) + } + if err := seedThemes(srcFile, dst, slog.Default()); err == nil { + t.Errorf("expected error when source is a file, got nil") + } +} + +func TestResolveBundledThemesDir_EnvOverride(t *testing.T) { + t.Setenv(EnvBundledThemesDir, "/custom/src") + if got := resolveBundledThemesDir(); got != "/custom/src" { + t.Errorf("got %q, want %q", got, "/custom/src") + } + t.Setenv(EnvBundledThemesDir, "") + if got := resolveBundledThemesDir(); got != DefaultBundledThemesDir { + t.Errorf("default fallthrough: got %q, want %q", got, DefaultBundledThemesDir) + } +} + +func TestResolveVolumeThemesDir_EnvOverride(t *testing.T) { + t.Setenv(EnvVolumeThemesDir, "/custom/dst") + if got := resolveVolumeThemesDir(); got != "/custom/dst" { + t.Errorf("got %q, want %q", got, "/custom/dst") + } + t.Setenv(EnvVolumeThemesDir, "") + if got := resolveVolumeThemesDir(); got != DefaultVolumeThemesDir { + t.Errorf("default fallthrough: got %q, want %q", got, DefaultVolumeThemesDir) + } +} + +// TestRun_Up_AcceptsSeedThemesFlag is the migrate-level smoke test for +// the new flag: it must parse cleanly. We drive the misconfiguration +// path with an unset DATABASE_URL so loadConfig refuses *after* the +// flag has been accepted. +func TestRun_Up_AcceptsSeedThemesFlag(t *testing.T) { + t.Setenv("DATABASE_URL", "") + for _, arg := range []string{ + "--seed-themes=false", + "--seed-themes=true", + "-seed-themes=false", + } { + t.Run(arg, func(t *testing.T) { + var stdout, stderr testWriter + code := Run([]string{"up", arg}, &stdout, &stderr) + if code != ExitUsage { + t.Errorf("exit: got %d, want %d", code, ExitUsage) + } + if !contains(stderr.String(), "DATABASE_URL") { + t.Errorf("expected DATABASE_URL error, got stderr=%q", stderr.String()) + } + }) + } +} + +// testWriter is a tiny strings.Builder wrapper. We avoid pulling in +// bytes.Buffer/strings.Contains in this file because the package-level +// migrate_test.go already uses those names; keeping local helpers +// avoids redeclaration noise. +type testWriter struct { + b []byte +} + +func (w *testWriter) Write(p []byte) (int, error) { w.b = append(w.b, p...); return len(p), nil } +func (w *testWriter) String() string { return string(w.b) } + +func contains(s, sub string) bool { + if len(sub) == 0 { + return true + } + for i := 0; i+len(sub) <= len(s); i++ { + if s[i:i+len(sub)] == sub { + return true + } + } + return false +} diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 022a3204..3036505c 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -82,6 +82,20 @@ services: <<: *go-env GONEXT_MIGRATION_DIR: /migrations GONEXT_THEME_DIR: /themes + # The file-seed step copies image-bundled themes (baked into the + # cli image at /themes by cli/gonext/Dockerfile) into the named + # api-themes volume mounted below. The api container mounts the + # same volume at /themes, so anything seeded here becomes visible + # to it on next boot. Idempotent: skipped when the volume already + # has theme directories. + GONEXT_BUNDLED_THEMES_DIR: /themes + GONEXT_VOLUME_THEMES_DIR: /var/lib/gonext-themes + volumes: + # api-themes is the shared volume the api consumes at /themes. + # Mounting it here at a distinct path lets the seeder copy + # /themes (image-baked) into the volume without the mount + # shadowing the source directory. + - api-themes:/var/lib/gonext-themes command: ["migrate", "up"] # ------------------------------------------------------------------------- diff --git a/packages/go/plugins/lifecycle/postgres.go b/packages/go/plugins/lifecycle/postgres.go index b30b3b13..1359ecd9 100644 --- a/packages/go/plugins/lifecycle/postgres.go +++ b/packages/go/plugins/lifecycle/postgres.go @@ -153,9 +153,20 @@ func (s *PostgresStorage) Get(ctx context.Context, slug string) (Plugin, error) } // List returns every plugin row, ordered by slug. +// +// On a clean install where the plugins table hasn't been created yet +// (SQLSTATE 42P01 — undefined_table), List returns an empty slice +// instead of an error. Semantically "no table" and "no rows" are the +// same thing for the lifecycle reader path: no plugins are installed. +// This keeps read-only callers (e.g. the admin sidebar's +// /api/v1/admin/plugin-pages endpoint) from failing with 500 on a +// fresh database before any plugin has been installed. func (s *PostgresStorage) List(ctx context.Context) ([]Plugin, error) { rows, err := s.db.Query(ctx, `SELECT `+selectColumns+` FROM plugins ORDER BY slug ASC`) if err != nil { + if isUndefinedTable(err) { + return []Plugin{}, nil + } return nil, fmt.Errorf("lifecycle/postgres: list: %w", err) } defer rows.Close() @@ -310,6 +321,18 @@ func isUniqueViolation(err error) bool { return false } +// isUndefinedTable reports whether err is a Postgres undefined-table +// (SQLSTATE 42P01). Used by List to treat a missing plugins table on a +// clean install as an empty result rather than a 500-worthy error. +func isUndefinedTable(err error) bool { + for ; err != nil; err = errors.Unwrap(err) { + if s, ok := err.(sqlStater); ok { + return s.SQLState() == "42P01" + } + } + return false +} + // sqlStater is the minimal interface our isUniqueViolation needs. The // concrete *pgconn.PgError type implements it. Re-declaring it locally // keeps PostgresStorage testable without importing pgconn solely for diff --git a/packages/go/plugins/lifecycle/postgres_test.go b/packages/go/plugins/lifecycle/postgres_test.go index 6778b551..88f173c7 100644 --- a/packages/go/plugins/lifecycle/postgres_test.go +++ b/packages/go/plugins/lifecycle/postgres_test.go @@ -370,6 +370,27 @@ func TestPostgresStorage_List_QueryError(t *testing.T) { } } +// On a fresh database where the plugins table hasn't been created yet, +// List should return an empty slice rather than a 500-worthy error. +// Regression for issue #503 — the admin sidebar polls +// /api/v1/admin/plugin-pages on every render and a missing table on +// clean install used to surface as "failed to list plugins" 500. +func TestPostgresStorage_List_UndefinedTableIsEmpty(t *testing.T) { + q := &fakeQuerier{ + queryFn: func(_ string, _ []any) (pgx.Rows, error) { + return nil, &stubPgError{code: "42P01"} + }, + } + st := NewPostgresStorageWithQuerier(q) + rows, err := st.List(context.Background()) + if err != nil { + t.Fatalf("List: unexpected error for undefined-table: %v", err) + } + if len(rows) != 0 { + t.Fatalf("expected empty slice, got %d rows", len(rows)) + } +} + func TestPostgresStorage_List_ScanError(t *testing.T) { q := &fakeQuerier{ queryFn: func(_ string, _ []any) (pgx.Rows, error) { diff --git a/packages/go/settings/core.go b/packages/go/settings/core.go index d0af8934..c3c6144f 100644 --- a/packages/go/settings/core.go +++ b/packages/go/settings/core.go @@ -109,6 +109,126 @@ func CoreSettings() []Setting { Group: GroupPermalinks, RequiresCapability: policy.CapManageOptions, }, + + // Reading group — issue #525. Mirrors the form rendered by + // apps/admin/.../settings/reading/ReadingForm.tsx. Keys here are + // the canonical names the admin client sends; renaming requires + // a coordinated change in both places. + { + Key: "core.reading.homepage_type", + Description: "What visitors see at the site root: the latest-posts index or a designated static page.", + Type: SettingTypeEnum, + Schema: json.RawMessage(`{"type":"string","enum":["latest_posts","static_page"]}`), + Default: "latest_posts", + Autoload: true, + Group: GroupReading, + RequiresCapability: policy.CapManageOptions, + }, + { + Key: "core.reading.homepage_page_id", + Description: "Slug or ID of the static page used as the homepage when homepage_type=static_page. Empty otherwise.", + Type: SettingTypeString, + Schema: json.RawMessage(`{"type":"string","maxLength":200}`), + Default: "", + Autoload: true, + Group: GroupReading, + RequiresCapability: policy.CapManageOptions, + }, + { + Key: "core.reading.posts_per_page", + Description: "Number of posts shown on the blog index and archive pages.", + Type: SettingTypeInt, + Schema: json.RawMessage(`{"type":"integer","minimum":1,"maximum":100}`), + Default: float64(10), + Autoload: true, + Group: GroupReading, + RequiresCapability: policy.CapManageOptions, + }, + { + Key: "core.reading.show_summary", + Description: "When true, archive pages show post excerpts instead of the full body.", + Type: SettingTypeBool, + Schema: json.RawMessage(`{"type":"boolean"}`), + Default: true, + Autoload: true, + Group: GroupReading, + RequiresCapability: policy.CapManageOptions, + }, + { + Key: "core.reading.rss_items", + Description: "Number of entries served at /feed/.", + Type: SettingTypeInt, + Schema: json.RawMessage(`{"type":"integer","minimum":1,"maximum":100}`), + Default: float64(10), + Autoload: true, + Group: GroupReading, + RequiresCapability: policy.CapManageOptions, + }, + { + Key: "core.reading.rss_full_text", + Description: "When true, RSS items include the full post body; when false, only excerpts.", + Type: SettingTypeBool, + Schema: json.RawMessage(`{"type":"boolean"}`), + Default: false, + Autoload: true, + Group: GroupReading, + RequiresCapability: policy.CapManageOptions, + }, + + // Writing group — issue #525. Mirrors + // apps/admin/.../settings/writing/WritingForm.tsx. The enum lists + // here are seed defaults; richer category/format pickers arrive + // with Posts (#31) and Taxonomies (#32). + { + Key: "core.writing.default_category", + Description: "Default taxonomy term applied to new posts that do not explicitly set one.", + Type: SettingTypeEnum, + Schema: json.RawMessage(`{"type":"string","enum":["uncategorized","news","blog","updates"]}`), + Default: "uncategorized", + Autoload: true, + Group: GroupWriting, + RequiresCapability: policy.CapManageOptions, + }, + { + Key: "core.writing.default_format", + Description: "Default post-format hint themes use to pick a template for new posts.", + Type: SettingTypeEnum, + Schema: json.RawMessage(`{"type":"string","enum":["standard","aside","gallery","link","quote"]}`), + Default: "standard", + Autoload: true, + Group: GroupWriting, + RequiresCapability: policy.CapManageOptions, + }, + { + Key: "core.writing.default_editor", + Description: "Which editor opens when an author clicks New post: the block editor or the classic editor.", + Type: SettingTypeEnum, + Schema: json.RawMessage(`{"type":"string","enum":["block","classic"]}`), + Default: "block", + Autoload: true, + Group: GroupWriting, + RequiresCapability: policy.CapManageOptions, + }, + { + Key: "core.writing.post_by_email_enabled", + Description: "When true, messages sent to the inbound address are converted to draft posts.", + Type: SettingTypeBool, + Schema: json.RawMessage(`{"type":"boolean"}`), + Default: false, + Autoload: true, + Group: GroupWriting, + RequiresCapability: policy.CapManageOptions, + }, + { + Key: "core.writing.post_by_email_address", + Description: "Inbound mailbox that receives incoming post-by-email messages. Only used when post_by_email_enabled is true.", + Type: SettingTypeString, + Schema: json.RawMessage(`{"type":"string","maxLength":254}`), + Default: "", + Autoload: true, + Group: GroupWriting, + RequiresCapability: policy.CapManageOptions, + }, } } diff --git a/packages/go/settings/core_test.go b/packages/go/settings/core_test.go new file mode 100644 index 00000000..14425def --- /dev/null +++ b/packages/go/settings/core_test.go @@ -0,0 +1,153 @@ +package settings + +import ( + "context" + "strings" + "testing" +) + +// TestRegisterCore_RegistersAllKeys asserts every key returned by +// CoreSettings shows up in a freshly seeded registry. Guards against +// silently dropping a Setting from the slice during a refactor. +func TestRegisterCore_RegistersAllKeys(t *testing.T) { + reg := NewRegistry() + if err := RegisterCore(reg); err != nil { + t.Fatalf("RegisterCore: %v", err) + } + + for _, want := range CoreSettings() { + got, err := reg.Get(want.Key) + if err != nil { + t.Errorf("Get(%q): %v", want.Key, err) + continue + } + if got.Key != want.Key { + t.Errorf("Get(%q): got key %q", want.Key, got.Key) + } + } +} + +// TestRegisterCore_ReadingKeysPresent locks in the keys the admin +// Reading page renders against. Issue #525 — if any of these go +// missing, the form renders blank and saves 400. +func TestRegisterCore_ReadingKeysPresent(t *testing.T) { + reg := NewRegistry() + if err := RegisterCore(reg); err != nil { + t.Fatalf("RegisterCore: %v", err) + } + + wantKeys := []string{ + "core.reading.homepage_type", + "core.reading.homepage_page_id", + "core.reading.posts_per_page", + "core.reading.show_summary", + "core.reading.rss_items", + "core.reading.rss_full_text", + } + for _, key := range wantKeys { + s, err := reg.Get(key) + if err != nil { + t.Errorf("Get(%q): %v", key, err) + continue + } + if s.Group != GroupReading { + t.Errorf("%s: Group = %q, want %q", key, s.Group, GroupReading) + } + if !strings.HasPrefix(s.Key, "core.reading.") { + t.Errorf("%s: key should be under core.reading.* namespace", key) + } + } +} + +// TestRegisterCore_WritingKeysPresent locks in the keys the admin +// Writing page renders against. See the Reading equivalent. +func TestRegisterCore_WritingKeysPresent(t *testing.T) { + reg := NewRegistry() + if err := RegisterCore(reg); err != nil { + t.Fatalf("RegisterCore: %v", err) + } + + wantKeys := []string{ + "core.writing.default_category", + "core.writing.default_format", + "core.writing.default_editor", + "core.writing.post_by_email_enabled", + "core.writing.post_by_email_address", + } + for _, key := range wantKeys { + s, err := reg.Get(key) + if err != nil { + t.Errorf("Get(%q): %v", key, err) + continue + } + if s.Group != GroupWriting { + t.Errorf("%s: Group = %q, want %q", key, s.Group, GroupWriting) + } + if !strings.HasPrefix(s.Key, "core.writing.") { + t.Errorf("%s: key should be under core.writing.* namespace", key) + } + } +} + +// TestRegisterCore_DefaultsReadable seeds the registry, wraps it in a +// memory store, and confirms every core key returns its declared default +// from Read (no Write needed). Equivalent to the +// TestRegisterPrivacy_AllRegister belt-and-braces check. +func TestRegisterCore_DefaultsReadable(t *testing.T) { + reg := NewRegistry() + if err := RegisterCore(reg); err != nil { + t.Fatalf("RegisterCore: %v", err) + } + store := NewMemoryStore(reg) + for _, s := range CoreSettings() { + v, err := store.Read(context.Background(), s.Key) + if err != nil { + t.Errorf("Read %s: %v", s.Key, err) + continue + } + if v == nil && s.Default != nil { + t.Errorf("expected default for %s, got nil", s.Key) + } + } +} + +// TestRegisterCore_ReadingPostsPerPageWriteRoundtrip confirms the +// posts_per_page schema accepts the documented range. The admin form +// renders a number input; we make sure a typical value round-trips +// through the registry without a validation error. +func TestRegisterCore_ReadingPostsPerPageWriteRoundtrip(t *testing.T) { + reg := NewRegistry() + if err := RegisterCore(reg); err != nil { + t.Fatalf("RegisterCore: %v", err) + } + store := NewMemoryStore(reg) + + if err := store.Write(context.Background(), "core.reading.posts_per_page", float64(25)); err != nil { + t.Fatalf("Write posts_per_page=25: %v", err) + } + got, err := store.Read(context.Background(), "core.reading.posts_per_page") + if err != nil { + t.Fatalf("Read posts_per_page: %v", err) + } + if got.(float64) != 25 { + t.Errorf("posts_per_page roundtrip: got %v, want 25", got) + } +} + +// TestRegisterCore_WritingDefaultEditorEnum confirms the default_editor +// enum schema rejects an unknown value. Cheap insurance that the +// validation hook is actually wired. +func TestRegisterCore_WritingDefaultEditorEnum(t *testing.T) { + reg := NewRegistry() + if err := RegisterCore(reg); err != nil { + t.Fatalf("RegisterCore: %v", err) + } + store := NewMemoryStore(reg) + + if err := store.Write(context.Background(), "core.writing.default_editor", "block"); err != nil { + t.Fatalf("Write default_editor=block: %v", err) + } + if err := store.Write(context.Background(), "core.writing.default_editor", "vim"); err == nil { + t.Errorf("Write default_editor=vim: want validation error, got nil") + } +} diff --git a/packages/go/util/queryparse/status.go b/packages/go/util/queryparse/status.go new file mode 100644 index 00000000..f1aae3e3 --- /dev/null +++ b/packages/go/util/queryparse/status.go @@ -0,0 +1,40 @@ +// Package queryparse holds tiny, dependency-free helpers for parsing +// URL query parameters that several handlers share. +// +// The package exists to keep the same alias rules ("status=any" means +// "no filter", empty string also means "no filter") from drifting +// across endpoints. Three list handlers — REST posts, admin comments, +// and admin search — used to hand-roll the same shape; every new list +// endpoint should call into here instead so the contract stays +// authoritative in one file. +package queryparse + +import "errors" + +// ErrInvalidStatus is returned by ParseStatus when the raw value is +// non-empty, not the "any" alias, and not present in the caller's +// valid set. Handlers map this to a 400 with an error code of their +// choosing — typically "invalid_status". +var ErrInvalidStatus = errors.New("queryparse: invalid status") + +// ParseStatus parses a "status" query param against the caller's set +// of valid values. +// +// Both "" and the literal "any" mean "no filter": the caller wants +// every status. Both return ("", nil) so the handler can drop the +// filter from its downstream call without a separate branch. +// +// Any other value must be in valid; otherwise ParseStatus returns +// ErrInvalidStatus. The caller supplies its own set rather than +// importing a shared one because the valid statuses differ by +// resource (posts use the WordPress post_status enum, comments use a +// moderation-state enum, etc.). +func ParseStatus(raw string, valid map[string]struct{}) (string, error) { + if raw == "" || raw == "any" { + return "", nil + } + if _, ok := valid[raw]; !ok { + return "", ErrInvalidStatus + } + return raw, nil +} diff --git a/packages/go/util/queryparse/status_test.go b/packages/go/util/queryparse/status_test.go new file mode 100644 index 00000000..2d64b4c1 --- /dev/null +++ b/packages/go/util/queryparse/status_test.go @@ -0,0 +1,112 @@ +package queryparse_test + +import ( + "errors" + "testing" + + "github.com/Singleton-Solution/GoNext/packages/go/util/queryparse" +) + +// validPosts mirrors the post_status enum the REST posts handler uses. +// Kept as a fixture rather than imported so this test file has zero +// dependencies outside the package under test. +var validPosts = map[string]struct{}{ + "draft": {}, + "pending": {}, + "published": {}, + "scheduled": {}, + "private": {}, + "trash": {}, +} + +// TestParseStatus_EmptyMeansNoFilter documents the contract for the +// empty-string case: the handler treats it as "all statuses". +func TestParseStatus_EmptyMeansNoFilter(t *testing.T) { + got, err := queryparse.ParseStatus("", validPosts) + if err != nil { + t.Fatalf("ParseStatus(\"\"): unexpected error %v", err) + } + if got != "" { + t.Errorf("ParseStatus(\"\") = %q, want \"\"", got) + } +} + +// TestParseStatus_AnyIsAlias is the regression test for issue #516 — +// the literal "any" must map to the empty filter, identically to "". +func TestParseStatus_AnyIsAlias(t *testing.T) { + got, err := queryparse.ParseStatus("any", validPosts) + if err != nil { + t.Fatalf("ParseStatus(\"any\"): unexpected error %v", err) + } + if got != "" { + t.Errorf("ParseStatus(\"any\") = %q, want \"\" (alias for no filter)", got) + } +} + +// TestParseStatus_ValidPassesThrough asserts that a value present in +// the valid set is returned verbatim. +func TestParseStatus_ValidPassesThrough(t *testing.T) { + for _, raw := range []string{"draft", "pending", "published", "scheduled", "private", "trash"} { + t.Run(raw, func(t *testing.T) { + got, err := queryparse.ParseStatus(raw, validPosts) + if err != nil { + t.Fatalf("ParseStatus(%q): unexpected error %v", raw, err) + } + if got != raw { + t.Errorf("ParseStatus(%q) = %q, want %q", raw, got, raw) + } + }) + } +} + +// TestParseStatus_UnknownReturnsErr covers the rejection path: an +// unknown value yields ErrInvalidStatus and an empty string. +func TestParseStatus_UnknownReturnsErr(t *testing.T) { + got, err := queryparse.ParseStatus("approve", validPosts) // typo of "approved"; not in set + if !errors.Is(err, queryparse.ErrInvalidStatus) { + t.Fatalf("ParseStatus(\"approve\") err = %v, want ErrInvalidStatus", err) + } + if got != "" { + t.Errorf("ParseStatus(\"approve\") = %q, want \"\" on error", got) + } +} + +// TestParseStatus_NilValidMapRejectsNonAlias documents that a caller +// passing a nil valid set still gets the "" / "any" aliases for free +// but rejects everything else. The implementation reads valid only +// inside the map-lookup branch, so a nil map is safe. +func TestParseStatus_NilValidMapRejectsNonAlias(t *testing.T) { + // empty + alias should still pass with a nil map + if got, err := queryparse.ParseStatus("", nil); err != nil || got != "" { + t.Errorf("ParseStatus(\"\", nil) = (%q, %v), want (\"\", nil)", got, err) + } + if got, err := queryparse.ParseStatus("any", nil); err != nil || got != "" { + t.Errorf("ParseStatus(\"any\", nil) = (%q, %v), want (\"\", nil)", got, err) + } + // anything else must error — the lookup against a nil map yields + // (zero, false) just like an empty map. + if got, err := queryparse.ParseStatus("draft", nil); !errors.Is(err, queryparse.ErrInvalidStatus) || got != "" { + t.Errorf("ParseStatus(\"draft\", nil) = (%q, %v), want (\"\", ErrInvalidStatus)", got, err) + } +} + +// TestParseStatus_CaseSensitive guards the design decision that +// status values are matched byte-exactly. The handlers that call in +// emit lowercase enums, so "Draft" must be rejected — relying on +// case-insensitive matching here would diverge from the underlying +// CHECK constraints in the schema migrations. +func TestParseStatus_CaseSensitive(t *testing.T) { + if _, err := queryparse.ParseStatus("Draft", validPosts); !errors.Is(err, queryparse.ErrInvalidStatus) { + t.Errorf("ParseStatus(\"Draft\"): err = %v, want ErrInvalidStatus (case-sensitive match)", err) + } +} + +// TestParseStatus_AnyCaseSensitive guards a subtle edge: only the +// lowercase "any" is the alias. "ANY", "Any", etc. fall through to +// the valid-set check and (assuming they're not in the set) are +// rejected. Documents the design rather than the implementation. +func TestParseStatus_AnyCaseSensitive(t *testing.T) { + if _, err := queryparse.ParseStatus("ANY", validPosts); !errors.Is(err, queryparse.ErrInvalidStatus) { + t.Errorf("ParseStatus(\"ANY\"): err = %v, want ErrInvalidStatus", err) + } +}