diff --git a/apps/clubs/src/clubs-db.ts b/apps/clubs/src/clubs-db.ts index dddddb5..d18c05c 100644 --- a/apps/clubs/src/clubs-db.ts +++ b/apps/clubs/src/clubs-db.ts @@ -14,7 +14,7 @@ * can build the tables directly. */ -import { getSavedImagesByIds } from '@repo/domain' +import { getSavedImagesByNames, placeholderSavedImage } from '@repo/domain' import type { SavedImage } from '@repo/domain' @@ -130,12 +130,11 @@ interface StoredClub extends Club { CreatedAt: string CustomTags?: string[] /** - * The club's gallery, by slot (the client PUTs to `/additionalimage/{index}`). - * Entries are `SavedImage` ids — image names are R2 keys and change when an image - * is re-uploaded, so the id is what stays true — with `0` for an empty slot. - * Positional, so clearing a middle slot doesn't shift the images after it. + * The club's gallery images, by slot (the client PUTs to + * `/additionalimage/{index}`). Positional, so a cleared middle slot stays as an + * empty string rather than shifting the images after it. */ - AdditionalImages?: number[] + AdditionalImages?: string[] } /** How many gallery images a club has room for (slots 0..2). */ @@ -675,48 +674,44 @@ function dedupeTags(tags: string[]): string[] { } /** - * A club's gallery slots as stored (image ids, `0` for empty). Trailing empty slots - * are trimmed, so a club with nothing set reads as `[]` while a club with only slot 1 - * filled still reports `[0, 42]` — the index a client PUT to is the index it reads - * back. + * A club's gallery images (stored on the blob). Trailing empty slots are trimmed, so + * a club with nothing set reads as `[]` while a club with only slot 1 filled still + * reports `['', 'name.jpg']` — the index a client PUT to is the index it reads back. */ -export async function getClubAdditionalImages(db: D1Database, clubId: number): Promise { +export async function getClubAdditionalImages(db: D1Database, clubId: number): Promise { const row = await db .prepare('SELECT data FROM club WHERE club_id = ?1') .bind(clubId) .first() const images = row === null ? [] : ((JSON.parse(row.data) as StoredClub).AdditionalImages ?? []) let end = images.length - while (end > 0 && images[end - 1] === 0) end-- + while (end > 0 && images[end - 1] === '') end-- return images.slice(0, end) } /** * A club's gallery as the client reads it: the image record behind each filled slot, - * in slot order. Empty slots are left out — the records carry no index, so a hole - * would just be a blank image — and so are ids whose image has since been deleted, - * which is the point of storing ids: the gallery follows the image rather than a - * filename that may now belong to nothing. + * in slot order. Empty slots are left out (the records carry no index, so a hole + * would just be a blank image), and a name whose metadata row is missing falls back + * to a placeholder record so the picture still renders. */ export async function getClubGallery(db: D1Database, clubId: number): Promise { - const ids = (await getClubAdditionalImages(db, clubId)).filter((id) => id !== 0) - if (ids.length === 0) return [] - const byId = await getSavedImagesByIds(db, ids) - return ids - .map((id) => byId.get(id)) - .filter((image): image is SavedImage => image !== undefined) + const names = (await getClubAdditionalImages(db, clubId)).filter((n) => n !== '') + if (names.length === 0) return [] + const records = await getSavedImagesByNames(db, names) + return names.map((name) => records.get(name) ?? placeholderSavedImage(name)) } /** - * Set (or clear, with a null `imageId`) one of a club's gallery image slots. Returns - * null when the club doesn't exist. The slot must be in range, and the image must - * exist — callers validate both before getting here. + * Set (or clear, with an empty `imageName`) one of a club's gallery image slots. + * Returns null when the club doesn't exist. The slot must be in range — callers + * validate the index before getting here. */ export async function setClubAdditionalImage( db: D1Database, clubId: number, index: number, - imageId: number | null + imageName: string ): Promise { const row = await db .prepare('SELECT data FROM club WHERE club_id = ?1') @@ -726,10 +721,10 @@ export async function setClubAdditionalImage( const stored = JSON.parse(row.data) as StoredClub // Pad rather than assign past the end: a sparse array would serialize its holes as - // nulls, and every slot the client reads should be an id (0 when empty). + // nulls, and the client expects strings in every slot it reads. const images = [...(stored.AdditionalImages ?? [])] - while (images.length <= index) images.push(0) - images[index] = imageId ?? 0 + while (images.length <= index) images.push('') + images[index] = imageName const updated: StoredClub = { ...stored, AdditionalImages: images } await db diff --git a/apps/clubs/src/clubs.app.ts b/apps/clubs/src/clubs.app.ts index 8a37bc9..ac0a802 100644 --- a/apps/clubs/src/clubs.app.ts +++ b/apps/clubs/src/clubs.app.ts @@ -1,7 +1,6 @@ import { Hono } from 'hono' import { useWorkersLogger } from 'workers-tagged-logger' -import { getSavedImageByName } from '@repo/domain' import { withNotFound, withOnError } from '@repo/hono-helpers' import { validateAndGetAccountId } from '@repo/jwt' @@ -577,13 +576,9 @@ const app = new Hono() // One of the club's gallery images, by slot (`/additionalimage/{index}`, 0-based — // the client PUTs the first image to 0, the second to 1). Takes the same - // `imageName` the `storage` worker handed back (or an `imageId` directly); an empty - // one clears that slot. Co-owner or above, like the main image. The slots are - // positional, so clearing one doesn't shift the others; they come back on - // `value.AdditionalImages` as whole image records. - // - // The name is resolved to the image's id and *that* is what's stored: names are R2 - // keys, so a re-upload changes them, while the id keeps pointing at the image. + // `imageName` the `storage` worker handed back; an empty one clears that slot. + // Co-owner or above, like the main image. The slots are positional, so clearing + // one doesn't shift the others; they come back on `value.AdditionalImages`. .put('/club/:clubId{[0-9]+}/additionalimage/:index{[0-9]+}', async (c) => { const id = await authedId(c) if (id === null) return c.body(null, 401) @@ -603,26 +598,10 @@ const app = new Hono() } const body = (await c.req.parseBody().catch(() => ({}))) as Record - const field = (name: string): string => { - const key = Object.keys(body).find((k) => k.toLowerCase() === name) - return typeof body[key ?? ''] === 'string' ? (body[key ?? ''] as string).trim() : '' - } - const rawId = field('imageid') - const imageName = field('imagename') + const key = Object.keys(body).find((k) => k.toLowerCase() === 'imagename') + const imageName = typeof body[key ?? ''] === 'string' ? (body[key ?? ''] as string).trim() : '' - // Neither field (or an empty one) clears the slot; otherwise resolve to an id and - // refuse an image we don't know, rather than storing a dangling reference. - let imageId: number | null = null - if (rawId !== '') { - imageId = Number.parseInt(rawId, 10) - if (Number.isNaN(imageId)) return clubError(c, 'Invalid imageId.') - } else if (imageName !== '') { - const image = await getSavedImageByName(c.env.DB, imageName) - if (image === null) return clubError(c, 'No such image.') - imageId = image.Id - } - - const updated = await setClubAdditionalImage(c.env.DB, clubId, index, imageId) + const updated = await setClubAdditionalImage(c.env.DB, clubId, index, imageName) if (updated === null) return c.notFound() return c.json({ error: '', diff --git a/apps/clubs/src/test/integration/api.test.ts b/apps/clubs/src/test/integration/api.test.ts index 3059071..87e39a7 100644 --- a/apps/clubs/src/test/integration/api.test.ts +++ b/apps/clubs/src/test/integration/api.test.ts @@ -585,7 +585,7 @@ describe('clubs endpoints', () => { const first = 'sharecamera/2026-07-21/e37fc41f-005e-4216-8f1e-a37dca953981.jpg' const insertImage = env.DB.prepare('INSERT OR IGNORE INTO image (data) VALUES (?1)') await env.DB.batch( - [first, 'b.jpg', 'c.jpg', 'legacy.jpg'].map((ImageName, i) => + [first, 'b.jpg', 'c.jpg'].map((ImageName, i) => insertImage.bind( JSON.stringify({ Id: 500 + i, @@ -641,63 +641,23 @@ describe('clubs endpoints', () => { const second = (await (await setImage(1, 'b.jpg')).json()) as Details expect(names(second.value.AdditionalImages)).toEqual([first, 'b.jpg', 'c.jpg']) - // What's stored is the image's id, not its name: renaming the image (a re-upload - // gets a new R2 key) leaves the gallery pointing at the same picture. - await env.DB.prepare("UPDATE image SET data = json_set(data, '$.ImageName', ?2) WHERE id = ?1") - .bind(501, 'b-renamed.jpg') - .run() - const renamed = (await ( - await exports.default.fetch(`${ORIGIN}/club/${clubId}/details`) - ).json()) as { AdditionalImages: Image[] } - expect(names(renamed.AdditionalImages)).toEqual([first, 'b-renamed.jpg', 'c.jpg']) - - // Re-PUTting a slot replaces just that image; an empty name clears it. - const replaced = (await (await setImage(0, 'c.jpg')).json()) as Details - expect(names(replaced.value.AdditionalImages)).toEqual(['c.jpg', 'b-renamed.jpg', 'c.jpg']) + // Re-PUTting a slot replaces just that image; an empty name clears it. A name with + // no image row still renders, as a placeholder record. + const replaced = (await (await setImage(0, 'a2.jpg')).json()) as Details + expect(names(replaced.value.AdditionalImages)).toEqual(['a2.jpg', 'b.jpg', 'c.jpg']) + expect(replaced.value.AdditionalImages[0]).toMatchObject({ Id: 0, ImageName: 'a2.jpg' }) const cleared = (await (await setImage(1, '')).json()) as Details - expect(names(cleared.value.AdditionalImages)).toEqual(['c.jpg', 'c.jpg']) - - // An image name nothing was uploaded under is refused rather than stored as a - // dangling reference, and a deleted image drops out of the gallery. - expect((await setImage(1, 'never-uploaded.jpg')).status).toBe(400) - await env.DB.prepare('DELETE FROM image WHERE id = ?1').bind(502).run() - const afterDelete = (await ( - await exports.default.fetch(`${ORIGIN}/club/${clubId}/details`) - ).json()) as { AdditionalImages: Image[] } - expect(afterDelete.AdditionalImages).toEqual([]) - // Put one back so the rest of the test has a gallery to read. - await setImage(0, first) + expect(names(cleared.value.AdditionalImages)).toEqual(['a2.jpg', 'c.jpg']) // They're on the club's details payload, for everyone reading the club. const details = (await ( await exports.default.fetch(`${ORIGIN}/club/${clubId}/details`) ).json()) as { AdditionalImages: Image[] } - expect(names(details.AdditionalImages)).toEqual([first]) - - // A club stored before ids (its slots hold image names) still resolves, and the - // next PUT to that slot rewrites it as an id. - await env.DB.prepare( - "UPDATE club SET data = json_set(data, '$.AdditionalImages', json(?2)) WHERE club_id = ?1" - ) - .bind(clubId, JSON.stringify(['legacy.jpg'])) - .run() - const legacy = (await ( - await exports.default.fetch(`${ORIGIN}/club/${clubId}/details`) - ).json()) as { AdditionalImages: Image[] } - expect(legacy.AdditionalImages).toEqual([ - expect.objectContaining({ Id: 503, ImageName: 'legacy.jpg' }), - ]) - await setImage(0, first) - const row = await env.DB.prepare('SELECT data FROM club WHERE club_id = ?1') - .bind(clubId) - .first<{ data: string }>() - expect((JSON.parse(row!.data) as { AdditionalImages: unknown[] }).AdditionalImages).toEqual([ - 500, - ]) + expect(names(details.AdditionalImages)).toEqual(['a2.jpg', 'c.jpg']) // There are only three slots, and only co-owners may set them. - expect((await setImage(3, first)).status).toBe(400) - expect((await setImage(0, first, '7101')).status).toBe(403) + expect((await setImage(3, 'd.jpg')).status).toBe(400) + expect((await setImage(0, 'hijack.jpg', '7101')).status).toBe(403) expect( ( await exports.default.fetch(`${ORIGIN}/club/${clubId}/additionalimage/0`, { diff --git a/packages/domain/src/images-db.ts b/packages/domain/src/images-db.ts index 6e884eb..4adbc83 100644 --- a/packages/domain/src/images-db.ts +++ b/packages/domain/src/images-db.ts @@ -30,37 +30,45 @@ const placeholders = (n: number): string => Array.from({ length: n }, (_, i) => `?${i + 1}`).join(',') /** - * Look up image records by id, returned keyed by Id. Ids with no record (the image - * was deleted since) are simply absent from the map. + * Look up image records by name (the R2 key), returned keyed by ImageName. One query + * for the whole set; names with no record are simply absent from the map. */ -export async function getSavedImagesByIds( +export async function getSavedImagesByNames( db: D1Database, - ids: number[] -): Promise> { - if (ids.length === 0) return new Map() + names: string[] +): Promise> { + if (names.length === 0) return new Map() const { results } = await db - .prepare(`SELECT data FROM image WHERE id IN (${placeholders(ids.length)})`) - .bind(...ids) + .prepare(`SELECT data FROM image WHERE image_name IN (${placeholders(names.length)})`) + .bind(...names) .all<{ data: string }>() return new Map( results.map((r) => { const image = JSON.parse(r.data) as SavedImage - return [image.Id, image] + return [image.ImageName, image] }) ) } /** - * Look up a single image record by name (the R2 key), or null. Only for turning a - * name a client sent into the image's id — store the id, never the name. + * A minimal SavedImage for an image name with no metadata row — enough for the client + * to render the picture. Uploads normally write a row first, so this only covers a + * name that was set directly (or whose row was since deleted). */ -export async function getSavedImageByName( - db: D1Database, - name: string -): Promise { - const row = await db - .prepare('SELECT data FROM image WHERE image_name = ?1') - .bind(name) - .first<{ data: string }>() - return row ? (JSON.parse(row.data) as SavedImage) : null +export function placeholderSavedImage(imageName: string): SavedImage { + return { + Id: 0, + Type: 1, + Accessibility: 1, + AccessibilityLocked: false, + ImageName: imageName, + Description: null, + PlayerId: 0, + TaggedPlayerIds: [], + RoomId: null, + PlayerEventId: null, + CreatedAt: new Date(0).toISOString(), + CheerCount: 0, + CommentCount: 0, + } }