diff --git a/backend/src/intake/backgroundRemoval.ts b/backend/src/intake/backgroundRemoval.ts index 31bfa11..2659d68 100644 --- a/backend/src/intake/backgroundRemoval.ts +++ b/backend/src/intake/backgroundRemoval.ts @@ -169,21 +169,103 @@ export async function restoreImageOriginal(imageId: number): Promise { } /** - * Every photo of one item, in order. + * What a whole-item removal actually did. * - * Sequential rather than parallel: the sidecar is assumed to handle one - * request at a time, and the worker it runs inside is not in a hurry. A - * failure on one photo stops the rest, and the caller logs it — the item keeps - * whatever was already done, and nothing is left half-written. + * `void` was enough for the drafting worker, which catches and logs and would + * not fail a draft over a background — but not for an admin standing in front + * of the screen, who needs to know whether the thing they pressed happened. + * Three of four is the normal shape of a bad day here, not an exception, and + * the count is what decides whether pressing it again is worth anything. */ -export async function removeBackgroundsForItem(itemId: number): Promise { - if (!isRembgConfigured()) return; +export interface RemovalSummary { + /** How many images the item has. */ + total: number; + /** How many now carry a cut-out, including any that already did. */ + removed: number; + /** Whether it stopped early because one of them failed. */ + failed: boolean; +} +/** + * What a whole-item restore did. + * + * No `failed`, because restoring cannot fail the way removing can: it is a + * database swap with no sidecar in it, and a photo that was never cut out is + * skipped rather than being an error. + */ +export interface RestoreSummary { + total: number; + /** How many were put back. Photos that were never cut out are not counted. */ + restored: number; +} + +/** Every photo of one item, in order. */ +async function imageIdsFor(itemId: number): Promise { const { rows } = await pool.query<{ id: number }>( `SELECT id FROM item_images WHERE item_id = $1 ORDER BY sort_order`, [itemId] ); - for (const row of rows) { - await removeImageBackground(row.id); + return rows.map((row) => row.id); +} + +/** + * Cuts out every photo of one item. + * + * Sequential rather than parallel: the sidecar is assumed to handle one request + * at a time, and neither caller is in a hurry. + * + * Stops at the first failure rather than pushing on. Six attempts against a + * sidecar that is not answering helps nobody, and stopping costs nothing + * because `removeImageBackground` skips a photo that already has an original + * recorded — so pressing the button again resumes where this stopped instead of + * starting over. The summary is what makes that retry an informed choice rather + * than a guess. + * + * An unconfigured environment is not a failure, here as everywhere else in this + * feature: nothing was attempted, so nothing went wrong. + */ +export async function removeBackgroundsForItem(itemId: number): Promise { + const imageIds = await imageIdsFor(itemId); + if (!isRembgConfigured()) { + return { total: imageIds.length, removed: 0, failed: false }; } + + let removed = 0; + for (const imageId of imageIds) { + try { + await removeImageBackground(imageId); + removed += 1; + } catch (err) { + // Logged rather than thrown. The caller gets the count, which is the + // thing it can act on; the reason belongs in the log, because the admin's + // next move is the same whatever it was. + console.error(`[background-removal] item ${itemId}, image ${imageId}:`, err); + return { total: imageIds.length, removed, failed: true }; + } + } + return { total: imageIds.length, removed, failed: false }; +} + +/** + * Puts every cut-out photo of one item back. + * + * A photo that was never cut out is skipped rather than refused — the mixed + * state a partial removal leaves behind has to be restorable too, and half an + * item is exactly when somebody reaches for this. + */ +export async function restoreOriginalsForItem(itemId: number): Promise { + const imageIds = await imageIdsFor(itemId); + + let restored = 0; + for (const imageId of imageIds) { + try { + await restoreImageOriginal(imageId); + restored += 1; + } catch (err) { + // Only "there was nothing to restore" is skipped. Anything else is a real + // failure and belongs to the caller. + if (!(err instanceof NoOriginalToRestoreError)) throw err; + } + } + return { total: imageIds.length, restored }; } diff --git a/backend/tests/integration/backgroundRemoval.integration.test.ts b/backend/tests/integration/backgroundRemoval.integration.test.ts index a0eba9c..4d3ffd1 100644 --- a/backend/tests/integration/backgroundRemoval.integration.test.ts +++ b/backend/tests/integration/backgroundRemoval.integration.test.ts @@ -5,7 +5,12 @@ import os from 'os'; import path from 'path'; import { pool } from '../../src/db'; import { resetDb, closeDb } from './setup/testDb'; -import { removeImageBackground, restoreImageOriginal } from '../../src/intake/backgroundRemoval'; +import { + removeImageBackground, + restoreImageOriginal, + removeBackgroundsForItem, + restoreOriginalsForItem +} from '../../src/intake/backgroundRemoval'; beforeEach(async () => { await resetDb(); @@ -250,3 +255,163 @@ describe('putting the original back', () => { expect((await pathsOf(imageId)).image_path).toBe('/uploads/original.jpg'); }); }); + +describe('acting on every photo of one item', () => { + /** One item with three photos on disk, which is what an upload leaves. */ + async function seedItemWithPhotos(): Promise<{ itemId: number; imageIds: number[] }> { + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name, status) VALUES ('Three views', 'available') RETURNING id` + ); + const itemId = rows[0]!.id; + + const imageIds: number[] = []; + for (const [index, name] of ['front.jpg', 'back.jpg', 'base.jpg'].entries()) { + const image = await pool.query<{ id: number }>( + `INSERT INTO item_images (item_id, image_path, sort_order) + VALUES ($1, $2, $3) RETURNING id`, + [itemId, `/uploads/${name}`, index] + ); + imageIds.push(image.rows[0]!.id); + await fsp.writeFile(path.join(uploads, name), JPEG_BYTES); + } + return { itemId, imageIds }; + } + + it('cuts out every photo and says how many', async () => { + await startStub(answerWithPng); + const { itemId } = await seedItemWithPhotos(); + + const summary = await removeBackgroundsForItem(itemId); + + expect(summary).toEqual({ total: 3, removed: 3, failed: false }); + }); + + // An item with no photos is not a failure. It is a perfectly ordinary item + // somebody has not photographed yet, and the button should say so rather + // than erroring. + it('reports nothing to do for an item with no photos', async () => { + await startStub(answerWithPng); + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name, status) VALUES ('Unphotographed', 'pending') RETURNING id` + ); + + expect(await removeBackgroundsForItem(rows[0]!.id)).toEqual({ + total: 0, + removed: 0, + failed: false + }); + }); + + // The case the whole summary exists for: the admin needs to know how far it + // got, because the answer decides whether pressing it again is worth it. + it('stops at the first failure and reports how far it got', async () => { + let served = 0; + await startStub((_req, res) => { + served += 1; + // The first photo works; the sidecar dies before the second. + if (served > 1) { + res.writeHead(500); + res.end('boom'); + return; + } + res.writeHead(200, { 'Content-Type': 'image/png' }); + res.end(PNG_BYTES); + }); + const { itemId } = await seedItemWithPhotos(); + + const summary = await removeBackgroundsForItem(itemId); + + expect(summary).toEqual({ total: 3, removed: 1, failed: true }); + }); + + // And the reason stopping early is acceptable: a retry resumes rather than + // starting over, because removeImageBackground skips what it already did. + it('a retry finishes the job without re-cutting what worked', async () => { + let served = 0; + await startStub((_req, res) => { + served += 1; + if (served === 2) { + res.writeHead(500); + res.end('boom'); + return; + } + res.writeHead(200, { 'Content-Type': 'image/png' }); + res.end(PNG_BYTES); + }); + const { itemId } = await seedItemWithPhotos(); + + const first = await removeBackgroundsForItem(itemId); + expect(first.failed).toBe(true); + + const second = await removeBackgroundsForItem(itemId); + + expect(second).toEqual({ total: 3, removed: 3, failed: false }); + }); + + // Unconfigured is not a failure anywhere else in this feature and is not one + // here. Nothing was attempted, so nothing went wrong. + it('does nothing, and calls it nothing, when there is no sidecar', async () => { + await startStub(answerWithPng); + const { itemId } = await seedItemWithPhotos(); + delete process.env.REMBG_URL; + + expect(await removeBackgroundsForItem(itemId)).toEqual({ + total: 3, + removed: 0, + failed: false + }); + }); +}); + +describe('putting every original back', () => { + it('restores each photo that was cut out', async () => { + await startStub(answerWithPng); + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name, status) VALUES ('Two views', 'available') RETURNING id` + ); + const itemId = rows[0]!.id; + for (const [index, name] of ['a.jpg', 'b.jpg'].entries()) { + await pool.query( + `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`, + [itemId, `/uploads/${name}`, index] + ); + await fsp.writeFile(path.join(uploads, name), JPEG_BYTES); + } + await removeBackgroundsForItem(itemId); + + const summary = await restoreOriginalsForItem(itemId); + + expect(summary).toEqual({ total: 2, restored: 2 }); + + const { rows: after } = await pool.query<{ image_path: string }>( + `SELECT image_path FROM item_images WHERE item_id = $1 ORDER BY sort_order`, + [itemId] + ); + expect(after.map((r) => r.image_path)).toEqual(['/uploads/a.jpg', '/uploads/b.jpg']); + }); + + // A photo that was never cut out is skipped rather than being an error — the + // mixed state a partial failure leaves behind has to be restorable too. + it('skips photos that were never cut out', async () => { + await startStub(answerWithPng); + const { rows } = await pool.query<{ id: number }>( + `INSERT INTO items (name, status) VALUES ('Mixed', 'available') RETURNING id` + ); + const itemId = rows[0]!.id; + for (const [index, name] of ['x.jpg', 'y.jpg'].entries()) { + await pool.query( + `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`, + [itemId, `/uploads/${name}`, index] + ); + await fsp.writeFile(path.join(uploads, name), JPEG_BYTES); + } + // Cut out only the first, leaving the second as it arrived. + const { rows: images } = await pool.query<{ id: number }>( + `SELECT id FROM item_images WHERE item_id = $1 ORDER BY sort_order`, + [itemId] + ); + await removeImageBackground(images[0]!.id); + + expect(await restoreOriginalsForItem(itemId)).toEqual({ total: 2, restored: 1 }); + }); +});