store images as int

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