diff --git a/.changeset/media-signed-upload-pending-rows.md b/.changeset/media-signed-upload-pending-rows.md new file mode 100644 index 0000000000..d2f6526614 --- /dev/null +++ b/.changeset/media-signed-upload-pending-rows.md @@ -0,0 +1,5 @@ +--- +"emdash": patch +--- + +Fixes the media table filling up with hidden, unusable `pending` records — one per upload attempt — when storage cannot create signed upload URLs, which is always the case for local storage and for R2 accessed through a Worker binding. diff --git a/packages/core/src/astro/routes/api/media/upload-url.ts b/packages/core/src/astro/routes/api/media/upload-url.ts index edfa95ff2e..6d07ea2a8c 100644 --- a/packages/core/src/astro/routes/api/media/upload-url.ts +++ b/packages/core/src/astro/routes/api/media/upload-url.ts @@ -103,6 +103,15 @@ export const POST: APIRoute = async ({ request, locals }) => { const ext = path.extname(body.filename) || ""; const storageKey = `${id}${ext}`; + // Get the signed upload URL before creating the pending row, so adapters + // that cannot pre-sign don't leave an orphaned `pending` record. + const signedUrl = await emdash.storage.getSignedUploadUrl({ + key: storageKey, + contentType: body.contentType, + size: body.size, + expiresIn: 3600, // 1 hour + }); + // Create pending media record with content hash const mediaItem = await repo.createPending({ filename: body.filename, @@ -113,14 +122,6 @@ export const POST: APIRoute = async ({ request, locals }) => { authorId: user?.id, }); - // Get signed upload URL from storage - const signedUrl = await emdash.storage.getSignedUploadUrl({ - key: storageKey, - contentType: body.contentType, - size: body.size, - expiresIn: 3600, // 1 hour - }); - const response: UploadUrlResponse = { uploadUrl: signedUrl.url, method: signedUrl.method, diff --git a/packages/core/tests/integration/api/media-upload-url-pending.test.ts b/packages/core/tests/integration/api/media-upload-url-pending.test.ts new file mode 100644 index 0000000000..8f78595ecc --- /dev/null +++ b/packages/core/tests/integration/api/media-upload-url-pending.test.ts @@ -0,0 +1,83 @@ +/** The signed-upload endpoint must not leave a pending media row behind when storage cannot pre-sign. */ +import { Role } from "@emdash-cms/auth"; +import type { APIContext } from "astro"; +import type { Kysely } from "kysely"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { POST as requestUploadUrl } from "../../../src/astro/routes/api/media/upload-url.js"; +import type { DatabaseSchema } from "../../../src/database/types.js"; +import { EmDashStorageError } from "../../../src/storage/types.js"; +import type { Storage } from "../../../src/storage/types.js"; +import { setupTestDatabase, teardownTestDatabase } from "../../utils/test-db.js"; + +/** Storage that behaves like local storage / an R2 binding: it cannot pre-sign. */ +function storageThatCannotPresign(): Storage { + return { + getSignedUploadUrl() { + throw new EmDashStorageError( + "Local storage does not support signed upload URLs. Upload files directly through the API.", + "NOT_SUPPORTED", + ); + }, + } as unknown as Storage; +} + +function callRoute(db: Kysely, storage: Storage) { + const request = new Request("http://localhost:4321/_emdash/api/media/upload-url", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ filename: "photo.png", contentType: "image/png", size: 1024 }), + }); + + return requestUploadUrl({ + request, + url: new URL(request.url), + params: {}, + locals: { + emdash: { db, storage, config: {} }, + user: { id: "admin-1", role: Role.ADMIN }, + }, + // eslint-disable-next-line typescript/no-unsafe-type-assertion -- minimal stub for tests + } as unknown as APIContext); +} + +async function countMedia(db: Kysely, status: string): Promise { + const rows = await db.selectFrom("media").select("id").where("status", "=", status).execute(); + return rows.length; +} + +describe("POST /_emdash/api/media/upload-url with storage that cannot pre-sign", () => { + let db: Kysely; + + beforeEach(async () => { + db = await setupTestDatabase(); + }); + + afterEach(async () => { + await teardownTestDatabase(db); + }); + + it("answers 501 NOT_SUPPORTED so the client falls back to direct upload", async () => { + const response = await callRoute(db, storageThatCannotPresign()); + + expect(response.status).toBe(501); + const body = (await response.json()) as { error: { code: string } }; + expect(body.error.code).toBe("NOT_SUPPORTED"); + }); + + it("does not create a pending media row", async () => { + await callRoute(db, storageThatCannotPresign()); + + expect(await countMedia(db, "pending")).toBe(0); + }); + + it("does not accumulate rows across repeated attempts", async () => { + for (let i = 0; i < 5; i++) { + // oxlint-disable-next-line no-await-in-loop -- sequential on purpose: the point is that repeats don't accumulate + await callRoute(db, storageThatCannotPresign()); + } + + const all = await db.selectFrom("media").select("id").execute(); + expect(all).toHaveLength(0); + }); +});