mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-15 06:09:35 +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>
163 lines
5.5 KiB
Go
163 lines
5.5 KiB
Go
package handler
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"encoding/json"
|
|
"fmt"
|
|
"mime/multipart"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
)
|
|
|
|
type mockStorage struct{}
|
|
|
|
func (m *mockStorage) Upload(_ context.Context, key string, _ []byte, _ string, _ string) (string, error) {
|
|
return fmt.Sprintf("https://cdn.example.com/%s", key), nil
|
|
}
|
|
|
|
func (m *mockStorage) Delete(_ context.Context, _ string) {}
|
|
func (m *mockStorage) DeleteKeys(_ context.Context, _ []string) {}
|
|
func (m *mockStorage) KeyFromURL(rawURL string) string { return rawURL }
|
|
func (m *mockStorage) CdnDomain() string { return "cdn.example.com" }
|
|
|
|
func TestUploadFileForeignWorkspace(t *testing.T) {
|
|
origStorage := testHandler.Storage
|
|
testHandler.Storage = &mockStorage{}
|
|
defer func() { testHandler.Storage = origStorage }()
|
|
|
|
var body bytes.Buffer
|
|
writer := multipart.NewWriter(&body)
|
|
part, err := writer.CreateFormFile("file", "test.txt")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
part.Write([]byte("hello world"))
|
|
writer.Close()
|
|
|
|
foreignWorkspaceID := "00000000-0000-0000-0000-000000000099"
|
|
req := httptest.NewRequest("POST", "/api/upload-file", &body)
|
|
req.Header.Set("Content-Type", writer.FormDataContentType())
|
|
req.Header.Set("X-User-ID", testUserID)
|
|
req.Header.Set("X-Workspace-ID", foreignWorkspaceID)
|
|
|
|
w := httptest.NewRecorder()
|
|
testHandler.UploadFile(w, req)
|
|
if w.Code != http.StatusForbidden {
|
|
t.Fatalf("UploadFile with foreign workspace: expected 403, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
}
|
|
|
|
// TestUploadFileResolvesWorkspaceViaSlugHeader is a regression test for the
|
|
// v2 workspace URL refactor (#1141). The frontend switched from sending
|
|
// X-Workspace-ID (UUID) to X-Workspace-Slug. For endpoints that sit outside
|
|
// the workspace middleware — like /api/upload-file — the handler-side
|
|
// resolver must accept the slug and translate it to a UUID, otherwise the
|
|
// handler silently falls through to the "no workspace context" branch and
|
|
// skips creating the DB attachment record. Files end up in S3 with no row
|
|
// in the attachment table, invisible to the UI.
|
|
func TestUploadFileResolvesWorkspaceViaSlugHeader(t *testing.T) {
|
|
origStorage := testHandler.Storage
|
|
testHandler.Storage = &mockStorage{}
|
|
defer func() { testHandler.Storage = origStorage }()
|
|
|
|
var body bytes.Buffer
|
|
writer := multipart.NewWriter(&body)
|
|
part, err := writer.CreateFormFile("file", "slug-upload.txt")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
part.Write([]byte("hello via slug"))
|
|
writer.Close()
|
|
|
|
req := httptest.NewRequest("POST", "/api/upload-file", &body)
|
|
req.Header.Set("Content-Type", writer.FormDataContentType())
|
|
req.Header.Set("X-User-ID", testUserID)
|
|
// Intentionally NOT setting X-Workspace-ID — post-v2 clients only send slug.
|
|
req.Header.Set("X-Workspace-Slug", handlerTestWorkspaceSlug)
|
|
|
|
w := httptest.NewRecorder()
|
|
testHandler.UploadFile(w, req)
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("UploadFile with slug header: expected 200, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
// The workspace-aware branch returns the full AttachmentResponse (with
|
|
// id, workspace_id, uploader, etc.). The no-workspace-context branch
|
|
// returns only {filename, link}. Distinguish by checking the shape.
|
|
var resp map[string]any
|
|
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
|
t.Fatalf("decode response: %v; body: %s", err, w.Body.String())
|
|
}
|
|
if _, ok := resp["id"]; !ok {
|
|
t.Fatalf("expected attachment response with 'id' field (DB row created); got fallback link-only response: %s", w.Body.String())
|
|
}
|
|
if gotWs, _ := resp["workspace_id"].(string); gotWs != testWorkspaceID {
|
|
t.Fatalf("attachment workspace_id mismatch: want %s, got %v", testWorkspaceID, resp["workspace_id"])
|
|
}
|
|
|
|
// Verify the row actually exists in the database.
|
|
var count int
|
|
if err := testPool.QueryRow(
|
|
context.Background(),
|
|
`SELECT count(*) FROM attachment WHERE workspace_id = $1 AND filename = $2`,
|
|
testWorkspaceID,
|
|
"slug-upload.txt",
|
|
).Scan(&count); err != nil {
|
|
t.Fatalf("query attachment count: %v", err)
|
|
}
|
|
if count != 1 {
|
|
t.Fatalf("attachment row count: want 1, got %d", count)
|
|
}
|
|
|
|
// Clean up so reruns don't accumulate rows.
|
|
if _, err := testPool.Exec(
|
|
context.Background(),
|
|
`DELETE FROM attachment WHERE workspace_id = $1 AND filename = $2`,
|
|
testWorkspaceID,
|
|
"slug-upload.txt",
|
|
); err != nil {
|
|
t.Fatalf("cleanup attachment: %v", err)
|
|
}
|
|
}
|
|
|
|
// TestUploadFileResolvesWorkspaceViaIDHeaderStill confirms the legacy path
|
|
// (CLI / daemon clients sending X-Workspace-ID as a UUID) still works after
|
|
// the refactor. Prevents a regression in the CLI/daemon compat branch.
|
|
func TestUploadFileResolvesWorkspaceViaIDHeaderStill(t *testing.T) {
|
|
origStorage := testHandler.Storage
|
|
testHandler.Storage = &mockStorage{}
|
|
defer func() { testHandler.Storage = origStorage }()
|
|
|
|
var body bytes.Buffer
|
|
writer := multipart.NewWriter(&body)
|
|
part, err := writer.CreateFormFile("file", "uuid-upload.txt")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
part.Write([]byte("hello via uuid"))
|
|
writer.Close()
|
|
|
|
req := httptest.NewRequest("POST", "/api/upload-file", &body)
|
|
req.Header.Set("Content-Type", writer.FormDataContentType())
|
|
req.Header.Set("X-User-ID", testUserID)
|
|
req.Header.Set("X-Workspace-ID", testWorkspaceID)
|
|
|
|
w := httptest.NewRecorder()
|
|
testHandler.UploadFile(w, req)
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("UploadFile with UUID header: expected 200, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
// Clean up.
|
|
if _, err := testPool.Exec(
|
|
context.Background(),
|
|
`DELETE FROM attachment WHERE workspace_id = $1 AND filename = $2`,
|
|
testWorkspaceID,
|
|
"uuid-upload.txt",
|
|
); err != nil {
|
|
t.Fatalf("cleanup attachment: %v", err)
|
|
}
|
|
}
|