removeBackgroundsForItem answered void and threw on the first failure, which is enough for the drafting worker — it catches and logs, and a draft is not worth failing over — and not enough for an admin standing in front of a screen who needs to know whether the thing they pressed happened. It now returns a summary: how many photos the item has, how many carry a cut-out, and whether it stopped early. It still stops at the first failure. 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 a retry resumes rather than starting over. The count is what turns that retry into an informed choice instead of a guess. Adds restoreOriginalsForItem alongside it. A photo that was never cut out is skipped rather than refused, because the mixed state a partial removal leaves behind is exactly when somebody reaches for this. The one existing caller does not change: the worker ignores the result, and ignoring a returned value is legal, which is what makes this additive rather than breaking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
418 lines
15 KiB
TypeScript
418 lines
15 KiB
TypeScript
import http from 'http';
|
||
import { AddressInfo } from 'net';
|
||
import { promises as fsp } from 'fs';
|
||
import os from 'os';
|
||
import path from 'path';
|
||
import { pool } from '../../src/db';
|
||
import { resetDb, closeDb } from './setup/testDb';
|
||
import {
|
||
removeImageBackground,
|
||
restoreImageOriginal,
|
||
removeBackgroundsForItem,
|
||
restoreOriginalsForItem
|
||
} from '../../src/intake/backgroundRemoval';
|
||
|
||
beforeEach(async () => {
|
||
await resetDb();
|
||
});
|
||
|
||
afterAll(async () => {
|
||
await pool.end();
|
||
await closeDb();
|
||
});
|
||
|
||
/** An item with a draft row and one image, which is what a submission leaves. */
|
||
async function seedSubmission(
|
||
imagePath = '/uploads/photo.jpg'
|
||
): Promise<{ itemId: number; imageId: number }> {
|
||
const item = await pool.query<{ id: number }>(
|
||
`INSERT INTO items (name, status) VALUES ('Submission placeholder', 'pending') RETURNING id`
|
||
);
|
||
const itemId = item.rows[0]!.id;
|
||
await pool.query(`INSERT INTO item_drafts (item_id) VALUES ($1)`, [itemId]);
|
||
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('the background-removal columns', () => {
|
||
// Default true because the submitter's checkbox is ticked by default, and
|
||
// because a row written by any path that does not mention the column should
|
||
// behave like the new default rather than needing a backfill.
|
||
it('defaults remove_background to true', async () => {
|
||
const { itemId } = await seedSubmission();
|
||
|
||
const { rows } = await pool.query<{ remove_background: boolean }>(
|
||
`SELECT remove_background FROM item_drafts WHERE item_id = $1`,
|
||
[itemId]
|
||
);
|
||
expect(rows[0]?.remove_background).toBe(true);
|
||
});
|
||
|
||
// Null is also the answer to "can this be restored?", which is why there is
|
||
// no separate flag: one fact, one place.
|
||
it('leaves original_image_path null until a photo has been cut out', async () => {
|
||
const { imageId } = await seedSubmission();
|
||
|
||
const { rows } = await pool.query<{ original_image_path: string | null }>(
|
||
`SELECT original_image_path FROM item_images WHERE id = $1`,
|
||
[imageId]
|
||
);
|
||
expect(rows[0]?.original_image_path).toBeNull();
|
||
});
|
||
});
|
||
|
||
const PNG_BYTES = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 0]);
|
||
const JPEG_BYTES = Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0, 0, 0, 0, 0, 0, 0, 0]);
|
||
|
||
interface ImagePaths {
|
||
image_path: string;
|
||
original_image_path: string | null;
|
||
}
|
||
|
||
let uploads = '';
|
||
let stub: http.Server | null = null;
|
||
let previousUploadsDir: string | undefined;
|
||
|
||
/**
|
||
* A real uploads directory and a stub sidecar.
|
||
*
|
||
* A temporary directory rather than the configured one, because these tests
|
||
* write files and a suite that leaves rubbish in a developer's uploads volume
|
||
* is a suite people stop running. `afterEach` removes it and puts back
|
||
* whatever UPLOADS_DIR held before, so this suite does not leak either the
|
||
* directory or the environment variable into whatever runs after it.
|
||
*
|
||
* No test here contacts the real sidecar. It takes forty seconds to start, and
|
||
* a suite that depends on that is broken by construction.
|
||
*/
|
||
async function startStub(
|
||
handler: (req: http.IncomingMessage, res: http.ServerResponse) => void
|
||
): Promise<void> {
|
||
uploads = await fsp.mkdtemp(path.join(os.tmpdir(), 'bgremoval-'));
|
||
previousUploadsDir = process.env.UPLOADS_DIR;
|
||
process.env.UPLOADS_DIR = uploads;
|
||
|
||
stub = http.createServer((req, res) => {
|
||
// Drain the body before answering, or the client sees a reset rather than
|
||
// the status this test is about.
|
||
req.on('data', () => undefined);
|
||
req.on('end', () => handler(req, res));
|
||
});
|
||
await new Promise<void>((resolve) => stub!.listen(0, '127.0.0.1', resolve));
|
||
process.env.REMBG_URL = `http://127.0.0.1:${(stub!.address() as AddressInfo).port}`;
|
||
}
|
||
|
||
function answerWithPng(_req: http.IncomingMessage, res: http.ServerResponse): void {
|
||
res.writeHead(200, { 'Content-Type': 'image/png' });
|
||
res.end(PNG_BYTES);
|
||
}
|
||
|
||
afterEach(async () => {
|
||
delete process.env.REMBG_URL;
|
||
if (stub) {
|
||
await new Promise<void>((resolve) => stub!.close(() => resolve()));
|
||
stub = null;
|
||
}
|
||
if (uploads !== '') {
|
||
await fsp.rm(uploads, { recursive: true, force: true });
|
||
uploads = '';
|
||
}
|
||
if (previousUploadsDir === undefined) delete process.env.UPLOADS_DIR;
|
||
else process.env.UPLOADS_DIR = previousUploadsDir;
|
||
previousUploadsDir = undefined;
|
||
});
|
||
|
||
async function seedWithFile(): Promise<{ itemId: number; imageId: number; name: string }> {
|
||
const name = 'original.jpg';
|
||
const seeded = await seedSubmission(`/uploads/${name}`);
|
||
await fsp.writeFile(path.join(uploads, name), JPEG_BYTES);
|
||
return { ...seeded, name };
|
||
}
|
||
|
||
async function pathsOf(imageId: number): Promise<ImagePaths> {
|
||
const { rows } = await pool.query<ImagePaths>(
|
||
`SELECT image_path, original_image_path FROM item_images WHERE id = $1`,
|
||
[imageId]
|
||
);
|
||
return rows[0]!;
|
||
}
|
||
|
||
describe('removing one photo’s background', () => {
|
||
it('points the row at the cut-out and records where the original went', async () => {
|
||
await startStub(answerWithPng);
|
||
const { imageId } = await seedWithFile();
|
||
|
||
await removeImageBackground(imageId);
|
||
|
||
const paths = await pathsOf(imageId);
|
||
expect(paths.image_path).toBe('/uploads/original-cutout.png');
|
||
expect(paths.original_image_path).toBe('/uploads/original.jpg');
|
||
});
|
||
|
||
// The original is the only copy of an item that may no longer be in the
|
||
// sender's hands. Nothing in this module is allowed to remove it.
|
||
it('leaves the original file on disk', async () => {
|
||
await startStub(answerWithPng);
|
||
const { imageId, name } = await seedWithFile();
|
||
|
||
await removeImageBackground(imageId);
|
||
|
||
await expect(fsp.access(path.join(uploads, name))).resolves.toBeUndefined();
|
||
});
|
||
|
||
it('writes the cut-out where the row now says it is', async () => {
|
||
await startStub(answerWithPng);
|
||
const { imageId } = await seedWithFile();
|
||
|
||
await removeImageBackground(imageId);
|
||
|
||
await expect(fsp.readFile(path.join(uploads, 'original-cutout.png'))).resolves.toEqual(
|
||
PNG_BYTES
|
||
);
|
||
});
|
||
|
||
// The guard that stops a second pass recording the cut-out as the original
|
||
// and losing the real one for good.
|
||
it('is a no-op on a photo that has already been cut out', async () => {
|
||
await startStub(answerWithPng);
|
||
const { imageId } = await seedWithFile();
|
||
|
||
await removeImageBackground(imageId);
|
||
await removeImageBackground(imageId);
|
||
|
||
expect((await pathsOf(imageId)).original_image_path).toBe('/uploads/original.jpg');
|
||
});
|
||
});
|
||
|
||
describe('when the sidecar will not answer', () => {
|
||
it('leaves the row untouched on a 500', async () => {
|
||
await startStub((_req, res) => {
|
||
res.writeHead(500);
|
||
res.end('boom');
|
||
});
|
||
const { imageId } = await seedWithFile();
|
||
|
||
await expect(removeImageBackground(imageId)).rejects.toThrow(/500/);
|
||
|
||
const paths = await pathsOf(imageId);
|
||
expect(paths.image_path).toBe('/uploads/original.jpg');
|
||
expect(paths.original_image_path).toBeNull();
|
||
});
|
||
|
||
it('leaves the row untouched when it returns something that is not an image', async () => {
|
||
await startStub((_req, res) => {
|
||
res.writeHead(200, { 'Content-Type': 'image/png' });
|
||
res.end('<html>gateway error</html>');
|
||
});
|
||
const { imageId } = await seedWithFile();
|
||
|
||
await expect(removeImageBackground(imageId)).rejects.toThrow(/not a PNG/);
|
||
|
||
const paths = await pathsOf(imageId);
|
||
expect(paths.image_path).toBe('/uploads/original.jpg');
|
||
expect(paths.original_image_path).toBeNull();
|
||
});
|
||
|
||
it('leaves the row untouched when it is unreachable', async () => {
|
||
await startStub(answerWithPng);
|
||
// Closed before the call, so the connection is refused rather than hung.
|
||
// The URL stays set, which is the case worth modelling: configured, and
|
||
// not there.
|
||
await new Promise<void>((resolve) => stub!.close(() => resolve()));
|
||
stub = null;
|
||
const { imageId } = await seedWithFile();
|
||
|
||
await expect(removeImageBackground(imageId)).rejects.toThrow();
|
||
|
||
const paths = await pathsOf(imageId);
|
||
expect(paths.image_path).toBe('/uploads/original.jpg');
|
||
expect(paths.original_image_path).toBeNull();
|
||
});
|
||
});
|
||
|
||
describe('putting the original back', () => {
|
||
it('swaps the paths back and clears the record', async () => {
|
||
await startStub(answerWithPng);
|
||
const { imageId } = await seedWithFile();
|
||
await removeImageBackground(imageId);
|
||
|
||
await restoreImageOriginal(imageId);
|
||
|
||
const paths = await pathsOf(imageId);
|
||
expect(paths.image_path).toBe('/uploads/original.jpg');
|
||
expect(paths.original_image_path).toBeNull();
|
||
});
|
||
|
||
it('refuses a photo that was never cut out, rather than blanking its path', async () => {
|
||
await startStub(answerWithPng);
|
||
const { imageId } = await seedWithFile();
|
||
|
||
await expect(restoreImageOriginal(imageId)).rejects.toThrow(/no original/);
|
||
|
||
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 });
|
||
});
|
||
});
|