mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-16 06:39:01 +02:00
Problem ------- The v2 workspace URL refactor (#1141) switched the frontend from sending X-Workspace-ID (UUID) to X-Workspace-Slug. The workspace middleware was updated to accept the slug and translate it via GetWorkspaceBySlug. But the handler package maintained a PARALLEL resolver (`resolveWorkspaceID` in handler.go) used by endpoints that sit outside the workspace middleware — and that resolver was never updated. It only checked context / ?workspace_id / X-Workspace-ID, never the slug. /api/upload-file is the one production route that hit the broken path: it's user-scoped (not behind workspace middleware) because it also serves avatar uploads (no workspace). Post-refactor requests from the frontend arrived with only X-Workspace-Slug; the handler resolver returned "", the code fell into the "no workspace context" branch, and every file upload since v2 landed in S3 with no corresponding DB attachment row — files orphaned, invisible to the UI. Root cause is structural: two resolvers doing the same job, written independently, diverged silently when one was updated. Fix --- Collapse to a single shared helper. middleware.ResolveWorkspaceIDFromRequest is the new canonical resolver; both the middleware's internal `resolveWorkspaceUUID` (for middleware gating) and the handler-side `(h *Handler).resolveWorkspaceID` (promoted from a package function) now delegate to it. Priority order matches what the middleware has had since v2: context > X-Workspace-Slug header > ?workspace_slug query > X-Workspace-ID header > ?workspace_id query. Impact analysis --------------- 47 call sites of the old `resolveWorkspaceID(r)` are renamed to `h.resolveWorkspaceID(r)`. 46 of them sit behind workspace middleware, so they hit the context fast path and see zero behavior change. The one caller that actually gains capability is UploadFile — which now correctly recognizes slug requests and creates DB attachment rows. Tests ----- - New table-driven unit test for ResolveWorkspaceIDFromRequest covers all priority levels and the unknown-slug fallback. - Regression tests for UploadFile: once with X-Workspace-Slug only (the broken path), once with X-Workspace-ID only (legacy CLI/daemon compat path). Both assert that a DB attachment row is created. - Full Go test suite passes; typecheck + pnpm test unaffected. Plan ---- See docs/plans/2026-04-16-unify-workspace-identity-resolver.md for the full first-principles writeup. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
170 lines
4.7 KiB
Go
170 lines
4.7 KiB
Go
package middleware
|
|
|
|
import (
|
|
"context"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"os"
|
|
"testing"
|
|
|
|
"github.com/jackc/pgx/v5/pgxpool"
|
|
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
|
)
|
|
|
|
const testResolverSlug = "middleware-resolver-test"
|
|
|
|
// openPool returns a connected pgxpool, or skips the test if the database is
|
|
// unreachable. Mirrors the handler package's fixture approach so tests don't
|
|
// require a DB in environments where one isn't available.
|
|
func openPool(t *testing.T) *pgxpool.Pool {
|
|
t.Helper()
|
|
dbURL := os.Getenv("DATABASE_URL")
|
|
if dbURL == "" {
|
|
dbURL = "postgres://multica:multica@localhost:5432/multica?sslmode=disable"
|
|
}
|
|
pool, err := pgxpool.New(context.Background(), dbURL)
|
|
if err != nil {
|
|
t.Skipf("skipping: could not connect to database: %v", err)
|
|
}
|
|
if err := pool.Ping(context.Background()); err != nil {
|
|
pool.Close()
|
|
t.Skipf("skipping: database not reachable: %v", err)
|
|
}
|
|
return pool
|
|
}
|
|
|
|
// setupResolverFixture inserts a workspace with a known slug and returns its
|
|
// UUID. The caller is responsible for calling the returned cleanup func.
|
|
func setupResolverFixture(t *testing.T, pool *pgxpool.Pool) (workspaceID string, cleanup func()) {
|
|
t.Helper()
|
|
ctx := context.Background()
|
|
// Pre-cleanup in case a previous run didn't finish.
|
|
_, _ = pool.Exec(ctx, `DELETE FROM workspace WHERE slug = $1`, testResolverSlug)
|
|
|
|
if err := pool.QueryRow(ctx,
|
|
`INSERT INTO workspace (name, slug, description, issue_prefix) VALUES ($1, $2, '', 'MRT') RETURNING id`,
|
|
"Middleware Resolver Test", testResolverSlug,
|
|
).Scan(&workspaceID); err != nil {
|
|
t.Fatalf("insert workspace: %v", err)
|
|
}
|
|
return workspaceID, func() {
|
|
_, _ = pool.Exec(ctx, `DELETE FROM workspace WHERE slug = $1`, testResolverSlug)
|
|
}
|
|
}
|
|
|
|
// TestResolveWorkspaceIDFromRequest pins down the priority order of the
|
|
// shared resolver. Every handler-level lookup of workspace identity — whether
|
|
// a route sits inside or outside the workspace middleware — must produce
|
|
// identical results, in the same priority, across all five supported
|
|
// mechanisms. Breaking any row here is a behavioral regression.
|
|
func TestResolveWorkspaceIDFromRequest(t *testing.T) {
|
|
pool := openPool(t)
|
|
defer pool.Close()
|
|
queries := db.New(pool)
|
|
|
|
workspaceID, cleanup := setupResolverFixture(t, pool)
|
|
defer cleanup()
|
|
|
|
const (
|
|
uuidA = "00000000-0000-0000-0000-000000000001"
|
|
uuidB = "00000000-0000-0000-0000-000000000002"
|
|
)
|
|
|
|
cases := []struct {
|
|
name string
|
|
setup func(r *http.Request)
|
|
want string
|
|
wantEmpty bool
|
|
}{
|
|
{
|
|
name: "context UUID wins over everything else",
|
|
setup: func(r *http.Request) {
|
|
ctx := context.WithValue(r.Context(), ctxKeyWorkspaceID, uuidA)
|
|
*r = *r.WithContext(ctx)
|
|
r.Header.Set("X-Workspace-Slug", testResolverSlug)
|
|
r.Header.Set("X-Workspace-ID", uuidB)
|
|
},
|
|
want: uuidA,
|
|
},
|
|
{
|
|
name: "X-Workspace-Slug header resolves to UUID via DB lookup",
|
|
setup: func(r *http.Request) {
|
|
r.Header.Set("X-Workspace-Slug", testResolverSlug)
|
|
},
|
|
want: workspaceID,
|
|
},
|
|
{
|
|
name: "X-Workspace-Slug wins over X-Workspace-ID (post-refactor priority)",
|
|
setup: func(r *http.Request) {
|
|
r.Header.Set("X-Workspace-Slug", testResolverSlug)
|
|
r.Header.Set("X-Workspace-ID", uuidB)
|
|
},
|
|
want: workspaceID,
|
|
},
|
|
{
|
|
name: "unknown X-Workspace-Slug falls through to UUID header",
|
|
setup: func(r *http.Request) {
|
|
r.Header.Set("X-Workspace-Slug", "does-not-exist")
|
|
r.Header.Set("X-Workspace-ID", uuidB)
|
|
},
|
|
want: uuidB,
|
|
},
|
|
{
|
|
name: "?workspace_slug query resolves to UUID via DB lookup",
|
|
setup: func(r *http.Request) {
|
|
q := r.URL.Query()
|
|
q.Set("workspace_slug", testResolverSlug)
|
|
r.URL.RawQuery = q.Encode()
|
|
},
|
|
want: workspaceID,
|
|
},
|
|
{
|
|
name: "X-Workspace-ID header is returned when no slug provided",
|
|
setup: func(r *http.Request) {
|
|
r.Header.Set("X-Workspace-ID", uuidA)
|
|
},
|
|
want: uuidA,
|
|
},
|
|
{
|
|
name: "?workspace_id query is the last-resort fallback",
|
|
setup: func(r *http.Request) {
|
|
q := r.URL.Query()
|
|
q.Set("workspace_id", uuidA)
|
|
r.URL.RawQuery = q.Encode()
|
|
},
|
|
want: uuidA,
|
|
},
|
|
{
|
|
name: "no identifier at all returns empty",
|
|
setup: func(r *http.Request) {},
|
|
wantEmpty: true,
|
|
},
|
|
{
|
|
name: "unknown slug with no UUID fallback returns empty",
|
|
setup: func(r *http.Request) {
|
|
r.Header.Set("X-Workspace-Slug", "does-not-exist")
|
|
},
|
|
wantEmpty: true,
|
|
},
|
|
}
|
|
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
req := httptest.NewRequest("GET", "/api/anything", nil)
|
|
tc.setup(req)
|
|
|
|
got := ResolveWorkspaceIDFromRequest(req, queries)
|
|
|
|
if tc.wantEmpty {
|
|
if got != "" {
|
|
t.Fatalf("expected empty, got %q", got)
|
|
}
|
|
return
|
|
}
|
|
if got != tc.want {
|
|
t.Fatalf("expected %q, got %q", tc.want, got)
|
|
}
|
|
})
|
|
}
|
|
}
|