diff --git a/backend/src/imageRotation.ts b/backend/src/imageRotation.ts new file mode 100644 index 0000000..37efea9 --- /dev/null +++ b/backend/src/imageRotation.ts @@ -0,0 +1,81 @@ +import path from 'path'; +import { pool } from './db'; +import { typeForExtension } from './uploadTypes'; +import { rotateInPlace, RotateDirection } from './imageProcessing'; + +/** + * Turning one stored photo, and everything that photo is stored alongside. + * + * Its own module rather than another export on backgroundRemoval.ts: the two + * features share a table and nothing else. Rotation touches no sidecar, records + * no provenance, and is not idempotent — bundling them would put a function + * with none of that module's invariants under its documentation. + */ + +interface ImageRow { + image_path: string; + original_image_path: string | null; +} + +/** + * Thrown when the photo is not on that item. + * + * A named class for the same reason NoOriginalToRestoreError is one: the route + * has to tell "no such photo" apart from "the application is in trouble", and + * the two need opposite replies. Everything else stays loud. + */ +export class ImageNotOnItemError extends Error {} + +/** + * basename only. `image_path` is stored as `/uploads/` and the directory + * it lives in is a server constant — the same rule readPhotos and + * removeImageBackground both follow. + */ +async function rotateStoredFile(storedPath: string, direction: RotateDirection): Promise { + const name = path.basename(storedPath); + const mediaType = typeForExtension(path.extname(name)); + if (mediaType === null) { + throw new Error(`cannot rotate ${name}: unrecognised extension`); + } + await rotateInPlace(path.join(process.env.UPLOADS_DIR ?? '', name), mediaType, direction); +} + +/** + * Turns one photo of one item, and its pristine original when it has one. + * + * Both files or neither is not achievable here and is not claimed. What matters + * is that they cannot end up disagreeing silently: an image that has been + * through #281 has a displayed cut-out and a recorded original, and rotating + * only the first would leave Restore original quietly un-rotating the photo — + * the undo of one feature becoming a regression of another. + * + * The displayed file goes first, so a failure on the original leaves the admin + * looking at a photo that visibly moved, with the failure reported. A retry + * would turn the displayed file a second time: rotation is not idempotent the + * way removeImageBackground is, because nothing records how far the last + * attempt got. That is accepted rather than engineered around — it takes the + * disk failing between two writes, and the remedy is one press in the other + * direction. + * + * No column is written. The paths are the same afterwards; only the bytes + * differ. + */ +export async function rotateItemImage( + itemId: number, + imageId: number, + direction: RotateDirection +): Promise { + const { rows } = await pool.query( + `SELECT image_path, original_image_path FROM item_images WHERE id = $1 AND item_id = $2`, + [imageId, itemId] + ); + const row = rows[0]; + if (!row) { + throw new ImageNotOnItemError(`no image ${imageId} on item ${itemId}`); + } + + await rotateStoredFile(row.image_path, direction); + if (row.original_image_path !== null) { + await rotateStoredFile(row.original_image_path, direction); + } +} diff --git a/backend/src/routes/admin.ts b/backend/src/routes/admin.ts index f6021e0..06da459 100755 --- a/backend/src/routes/admin.ts +++ b/backend/src/routes/admin.ts @@ -8,6 +8,8 @@ import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilter import { readId, tagColorFor } from '../utils'; import { notifyFavoritersOfSale, notifyFavoritersOfRemoval, collectFavoriteRecipients } from '../favoriteAlerts'; import { removeBackgroundsForItem, restoreOriginalsForItem } from '../intake/backgroundRemoval'; +import { rotateItemImage, ImageNotOnItemError } from '../imageRotation'; +import { RotateDirection } from '../imageProcessing'; // The upload pipeline moved to src/imageUpload.ts when #222's public intake // endpoint became a second caller. Mounting uploadImages gets the type // allowlist, the magic-byte check, and the EXIF-stripping re-encode together — @@ -411,4 +413,61 @@ router.post('/items/:id/restore-originals', asyncRoute(async (req: Request, res: res.json(await restoreOriginalsForItem(itemId)); })); +/** + * Turn one photo a quarter turn. + * + * On the item rather than on the draft, which is the decision that makes the + * inventory editor free later: an image belongs to an item whether or not a + * draft row exists, so the second screen to want this is the same call from a + * different place, with no new backend at all. + * + * Unlike the per-item background endpoints, this acts on exactly one file, so + * it can honestly answer whether it worked. 204 rather than 200 because + * rotation changes no column — the paths are identical afterwards and only the + * bytes differ, so there is no row worth returning, which is also why + * DELETE /items/:id/images/:imageId is a 204. + * + * A factory rather than two copied handlers: the direction is the only thing + * that differs. Two paths rather than one endpoint taking a direction in the + * body matches how remove-background and restore-original are already spelled. + */ +function rotationRoute(direction: RotateDirection) { + return asyncRoute(async (req: Request, res: Response) => { + // Both ids, not just the first. A route carrying two of them can guard one + // and forget the other, and the forgotten one fails as a 500 rather than + // the 404 that "no such photo" actually means (#207). + const itemId = readId(req.params.id); + const imageId = readId(req.params.imageId); + if (itemId === null || imageId === null) { + return res.status(404).json({ error: 'no such photo on this item' }); + } + + try { + await rotateItemImage(itemId, imageId, direction); + } catch (err) { + // Only "not on this item" is a 404, and it is indistinguishable from an + // absent one on purpose: an image id is a serial, and confirming which + // ids exist is not something this endpoint should do. Everything else is + // a real fault and stays loud — the file is untouched in every one of + // those cases, because rotateInPlace renames over the original only once + // the new file has been written successfully. + if (err instanceof ImageNotOnItemError) { + return res.status(404).json({ error: 'no such photo on this item' }); + } + console.error(`[rotation] item ${itemId}, image ${imageId}:`, err); + return res.status(500).json({ + error: + err instanceof Error + ? `this photo could not be rotated: ${err.message}` + : 'this photo could not be rotated' + }); + } + + res.status(204).end(); + }); +} + +router.post('/items/:id/images/:imageId/rotate-left', rotationRoute('left')); +router.post('/items/:id/images/:imageId/rotate-right', rotationRoute('right')); + export default router; diff --git a/backend/tests/integration/itemImageRotation.integration.test.ts b/backend/tests/integration/itemImageRotation.integration.test.ts new file mode 100644 index 0000000..d76e229 --- /dev/null +++ b/backend/tests/integration/itemImageRotation.integration.test.ts @@ -0,0 +1,141 @@ +import request from 'supertest'; +import sharp from 'sharp'; +import { promises as fs } from 'fs'; +import path from 'path'; +import app from '../../src/app'; +import { pool } from '../../src/db'; +import { resetDb, closeDb } from './setup/testDb'; + +const UPLOADS_DIR = process.env.UPLOADS_DIR as string; + +beforeAll(async () => { + await fs.mkdir(UPLOADS_DIR, { recursive: true }); +}); + +beforeEach(async () => { + await resetDb(); +}); + +afterAll(async () => { + await pool.end(); + await closeDb(); +}); + +/** A 400x200 landscape JPEG — wide enough that a quarter turn is unmistakable. */ +async function landscapeJpeg(): Promise { + return sharp({ + create: { width: 400, height: 200, channels: 3, background: { r: 20, g: 60, b: 120 } } + }) + .jpeg() + .toBuffer(); +} + +async function writeUpload(name: string): Promise { + await fs.writeFile(path.join(UPLOADS_DIR, name), await landscapeJpeg()); + return `/uploads/${name}`; +} + +async function sizeOf(storedPath: string): Promise { + const meta = await sharp(path.join(UPLOADS_DIR, path.basename(storedPath))).metadata(); + return `${meta.width}x${meta.height}`; +} + +/** An item with one photo on disk. The prefix keeps each test's file its own. */ +async function seedItem(prefix: string): Promise<{ itemId: number; imageId: number }> { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name, status) VALUES ('Blue vase', 'available') RETURNING id` + ); + const itemId = rows[0]!.id; + const imagePath = await writeUpload(`${prefix}-front.jpg`); + const image = await pool.query<{ id: number }>( + `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, 0) RETURNING id`, + [itemId, imagePath] + ); + return { itemId, imageId: image.rows[0]!.id }; +} + +describe('rotating one photo of an item', () => { + it('turns the file and answers 204', async () => { + const { itemId, imageId } = await seedItem('turns'); + + const res = await request(app).post( + `/api/admin/items/${itemId}/images/${imageId}/rotate-right` + ); + + expect(res.status).toBe(204); + expect(await sizeOf('/uploads/turns-front.jpg')).toBe('200x400'); + }); + + // The undo story. One press back is why the control offers both directions + // rather than making somebody turn the same button three times, each turn a + // further generation of JPEG loss. + it('turns back the other way', async () => { + const { itemId, imageId } = await seedItem('back'); + + await request(app).post(`/api/admin/items/${itemId}/images/${imageId}/rotate-right`); + await request(app).post(`/api/admin/items/${itemId}/images/${imageId}/rotate-left`); + + expect(await sizeOf('/uploads/back-front.jpg')).toBe('400x200'); + }); + + // Without this, Restore original would silently un-rotate the photo, turning + // the undo of #281 into a regression of #301. + it('turns the pristine original too', async () => { + const { itemId, imageId } = await seedItem('cutout'); + const originalPath = await writeUpload('cutout-original.jpg'); + await pool.query(`UPDATE item_images SET original_image_path = $2 WHERE id = $1`, [ + imageId, + originalPath + ]); + + const res = await request(app).post( + `/api/admin/items/${itemId}/images/${imageId}/rotate-right` + ); + + expect(res.status).toBe(204); + expect(await sizeOf('/uploads/cutout-front.jpg')).toBe('200x400'); + expect(await sizeOf(originalPath)).toBe('200x400'); + }); + + // An image id is a serial, so guessing one is easy — the photo has to belong + // to the item named in the path (#207). + it('refuses a photo belonging to another item', async () => { + const mine = await seedItem('mine'); + const theirs = await seedItem('theirs'); + + const res = await request(app).post( + `/api/admin/items/${mine.itemId}/images/${theirs.imageId}/rotate-right` + ); + + expect(res.status).toBe(404); + expect(await sizeOf('/uploads/theirs-front.jpg')).toBe('400x200'); + }); + + it('answers 404 for an unreadable id', async () => { + const { itemId } = await seedItem('unreadable'); + + const res = await request(app).post(`/api/admin/items/${itemId}/images/abc/rotate-right`); + + expect(res.status).toBe(404); + }); + + it('answers 404 for an absent item', async () => { + const res = await request(app).post('/api/admin/items/999999/images/1/rotate-left'); + + expect(res.status).toBe(404); + }); + + // Reported rather than swallowed: a row pointing at a file that is not there + // is a real fault, and answering "done" would hide it. + it('answers 500 when the file is missing from disk', async () => { + const { itemId, imageId } = await seedItem('missing'); + await fs.rm(path.join(UPLOADS_DIR, 'missing-front.jpg')); + + const res = await request(app).post( + `/api/admin/items/${itemId}/images/${imageId}/rotate-right` + ); + + expect(res.status).toBe(500); + expect(res.body.error).toMatch(/could not be rotated/); + }); +});