feat(admin): remove and restore a photo's background per image (#281)
Adds two admin-gated endpoints on the review queue router: POST /:itemId/images/:imageId/remove-background and POST /:itemId/images/:imageId/restore-original. Both run synchronously and reuse the same backgroundRemoval module the drafting worker uses, so a cut-out obtained either way is identical and either can be undone by Restore.
Ownership is scoped by item as well as by image (imageOfItem selects on id AND item_id), because the image id is a serial and guessing one is easy — a photo belonging to a different submission must not be reachable through another item's URL. A sidecar failure returns 502, not 500, and leaves the row untouched, since removeImageBackground only writes the row after the cut-out file already exists on disk.
GET /api/admin/item-drafts now returns { drafts, backgroundRemoval } instead of { drafts }, and each image in the payload gains original_image_path, which is what the review queue UI will use to decide between "Remove background" and "Restore original". DRAFT_SELECT's images aggregate is extended accordingly, keeping the deliberate column spelling that guards against the upload_links token digest leaking into the response.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,6 +3,8 @@ import { pool } from '../db';
|
|||||||
import { asyncRoute } from '../asyncRoute';
|
import { asyncRoute } from '../asyncRoute';
|
||||||
import { draftQueued } from '../intake/draftingWorker';
|
import { draftQueued } from '../intake/draftingWorker';
|
||||||
import { nextPriceSource, PriceSource } from '../intake/priceSource';
|
import { nextPriceSource, PriceSource } from '../intake/priceSource';
|
||||||
|
import { removeImageBackground, restoreImageOriginal } from '../intake/backgroundRemoval';
|
||||||
|
import { isRembgConfigured } from '../intake/rembgClient';
|
||||||
|
|
||||||
const router = Router();
|
const router = Router();
|
||||||
|
|
||||||
@@ -27,7 +29,10 @@ const DRAFT_SELECT = `
|
|||||||
i.price_cents, i.status,
|
i.price_cents, i.status,
|
||||||
l.label AS upload_link_label,
|
l.label AS upload_link_label,
|
||||||
COALESCE((
|
COALESCE((
|
||||||
SELECT json_agg(json_build_object('id', img.id, 'image_path', img.image_path)
|
SELECT json_agg(json_build_object(
|
||||||
|
'id', img.id,
|
||||||
|
'image_path', img.image_path,
|
||||||
|
'original_image_path', img.original_image_path)
|
||||||
ORDER BY img.sort_order)
|
ORDER BY img.sort_order)
|
||||||
FROM item_images img WHERE img.item_id = d.item_id
|
FROM item_images img WHERE img.item_id = d.item_id
|
||||||
), '[]'::json) AS images
|
), '[]'::json) AS images
|
||||||
@@ -52,7 +57,9 @@ router.get(
|
|||||||
? await pool.query(`${DRAFT_SELECT} WHERE d.state = $1 ORDER BY d.created_at DESC`, [state])
|
? await pool.query(`${DRAFT_SELECT} WHERE d.state = $1 ORDER BY d.created_at DESC`, [state])
|
||||||
: await pool.query(`${DRAFT_SELECT} WHERE d.state <> 'discarded' ORDER BY d.created_at DESC`);
|
: await pool.query(`${DRAFT_SELECT} WHERE d.state <> 'discarded' ORDER BY d.created_at DESC`);
|
||||||
|
|
||||||
res.json({ drafts: rows });
|
// Whether the control has anything behind it, alongside the rows. A second
|
||||||
|
// endpoint for one boolean would be a round trip the queue already makes.
|
||||||
|
res.json({ drafts: rows, backgroundRemoval: isRembgConfigured() });
|
||||||
})
|
})
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -220,4 +227,82 @@ router.post(
|
|||||||
})
|
})
|
||||||
);
|
);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* One photo's current paths, if it belongs to this item.
|
||||||
|
*
|
||||||
|
* Scoped by item as well as by image so an image id from a different
|
||||||
|
* submission cannot be acted on through this item's URL — the id is a serial,
|
||||||
|
* so guessing one is not hard.
|
||||||
|
*/
|
||||||
|
async function imageOfItem(
|
||||||
|
itemId: string,
|
||||||
|
imageId: string
|
||||||
|
): Promise<{ image_path: string; original_image_path: string | null } | null> {
|
||||||
|
const { rows } = await pool.query<{ image_path: string; original_image_path: string | null }>(
|
||||||
|
`SELECT image_path, original_image_path
|
||||||
|
FROM item_images WHERE id = $1 AND item_id = $2`,
|
||||||
|
[imageId, itemId]
|
||||||
|
);
|
||||||
|
return rows[0] ?? null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Remove the background from one photo.
|
||||||
|
*
|
||||||
|
* The other half of the submitter's checkbox: for the photos nobody ticked it
|
||||||
|
* for, and for the ones where the worker could not reach the sidecar. Both go
|
||||||
|
* through the same module, so a cut-out obtained either way is identical and
|
||||||
|
* either can be undone by Restore.
|
||||||
|
*
|
||||||
|
* Synchronous, unlike the worker's path. A warm request measures 1.1–2.3 s and
|
||||||
|
* this is an admin who just clicked a button and is watching for the result.
|
||||||
|
* The reason drafting was moved off the request path — that a stranger can
|
||||||
|
* trigger it and must never wait — does not apply behind the admin gate.
|
||||||
|
*/
|
||||||
|
router.post(
|
||||||
|
'/:itemId/images/:imageId/remove-background',
|
||||||
|
asyncRoute(async (req: Request, res: Response) => {
|
||||||
|
const { itemId = '', imageId = '' } = req.params;
|
||||||
|
if ((await imageOfItem(itemId, imageId)) === null) {
|
||||||
|
return res.status(404).json({ error: 'no such photo on this item' });
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
await removeImageBackground(Number(imageId));
|
||||||
|
} catch (err) {
|
||||||
|
// 502, not 500. The request was fine and so is this app — the service it
|
||||||
|
// depends on did not answer. The message says the photo is unchanged,
|
||||||
|
// because that is the thing the admin actually needs to know.
|
||||||
|
console.error(`[drafts] background removal for image ${imageId}:`, err);
|
||||||
|
return res
|
||||||
|
.status(502)
|
||||||
|
.json({ error: 'the background-removal service did not answer — the photo is unchanged' });
|
||||||
|
}
|
||||||
|
|
||||||
|
res.json(await imageOfItem(itemId, imageId));
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Put the original photo back.
|
||||||
|
*
|
||||||
|
* The reason a cut-out is safe to try at all. Background removal produces the
|
||||||
|
* occasional poor result on an unusual object, and this makes that survivable
|
||||||
|
* rather than something to prevent. Nothing is deleted: the cut-out file stays
|
||||||
|
* on disk, because somebody restoring one is quite likely to try again.
|
||||||
|
*/
|
||||||
|
router.post(
|
||||||
|
'/:itemId/images/:imageId/restore-original',
|
||||||
|
asyncRoute(async (req: Request, res: Response) => {
|
||||||
|
const { itemId = '', imageId = '' } = req.params;
|
||||||
|
const existing = await imageOfItem(itemId, imageId);
|
||||||
|
if (existing === null || existing.original_image_path === null) {
|
||||||
|
return res.status(404).json({ error: 'this photo has no original to restore' });
|
||||||
|
}
|
||||||
|
|
||||||
|
await restoreImageOriginal(Number(imageId));
|
||||||
|
res.json(await imageOfItem(itemId, imageId));
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
export default router;
|
export default router;
|
||||||
|
|||||||
@@ -2,6 +2,11 @@ import request from 'supertest';
|
|||||||
import app from '../../src/app';
|
import app from '../../src/app';
|
||||||
import { pool } from '../../src/db';
|
import { pool } from '../../src/db';
|
||||||
import { resetDb, closeDb } from './setup/testDb';
|
import { resetDb, closeDb } from './setup/testDb';
|
||||||
|
import http from 'http';
|
||||||
|
import { AddressInfo } from 'net';
|
||||||
|
import { promises as fsp } from 'fs';
|
||||||
|
import os from 'os';
|
||||||
|
import path from 'path';
|
||||||
|
|
||||||
beforeEach(async () => {
|
beforeEach(async () => {
|
||||||
await resetDb();
|
await resetDb();
|
||||||
@@ -304,3 +309,144 @@ describe('the other three actions', () => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('the review queue’s background-removal control', () => {
|
||||||
|
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]);
|
||||||
|
|
||||||
|
let uploads = '';
|
||||||
|
let stub: http.Server | null = null;
|
||||||
|
|
||||||
|
/** A stub sidecar on an ephemeral port, and a temporary uploads directory. */
|
||||||
|
async function startStub(status: number, body: Buffer | string): Promise<void> {
|
||||||
|
uploads = await fsp.mkdtemp(path.join(os.tmpdir(), 'adminbg-'));
|
||||||
|
process.env.UPLOADS_DIR = uploads;
|
||||||
|
|
||||||
|
stub = http.createServer((req, res) => {
|
||||||
|
req.on('data', () => undefined);
|
||||||
|
req.on('end', () => {
|
||||||
|
res.writeHead(status, { 'Content-Type': 'image/png' });
|
||||||
|
res.end(body);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
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}`;
|
||||||
|
}
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
delete process.env.REMBG_URL;
|
||||||
|
if (stub) {
|
||||||
|
await new Promise<void>((resolve) => stub!.close(() => resolve()));
|
||||||
|
stub = null;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
/** A ready draft with one photo, on disk, named to match the assertions. */
|
||||||
|
async function seedDraftWithImage(): Promise<{ itemId: number; imageId: number }> {
|
||||||
|
const { rows } = await pool.query<{ id: number }>(
|
||||||
|
`INSERT INTO items (name, status) VALUES ('Submission placeholder', 'pending') RETURNING id`
|
||||||
|
);
|
||||||
|
const itemId = rows[0]!.id;
|
||||||
|
await pool.query(`INSERT INTO item_drafts (item_id, state) VALUES ($1, 'ready')`, [itemId]);
|
||||||
|
const image = await pool.query<{ id: number }>(
|
||||||
|
`INSERT INTO item_images (item_id, image_path, sort_order)
|
||||||
|
VALUES ($1, '/uploads/original.jpg', 0) RETURNING id`,
|
||||||
|
[itemId]
|
||||||
|
);
|
||||||
|
if (uploads !== '') await fsp.writeFile(path.join(uploads, 'original.jpg'), JPEG_BYTES);
|
||||||
|
return { itemId, imageId: image.rows[0]!.id };
|
||||||
|
}
|
||||||
|
|
||||||
|
it('says whether there is a sidecar behind the control at all', async () => {
|
||||||
|
process.env.REMBG_URL = 'http://rembg-syn:7000';
|
||||||
|
const on = await request(app).get('/api/admin/item-drafts');
|
||||||
|
expect(on.body.backgroundRemoval).toBe(true);
|
||||||
|
|
||||||
|
delete process.env.REMBG_URL;
|
||||||
|
const off = await request(app).get('/api/admin/item-drafts');
|
||||||
|
expect(off.body.backgroundRemoval).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
// The UI decides between "Remove background" and "Restore original" from
|
||||||
|
// this field alone, so it has to be in the payload the queue is built from.
|
||||||
|
it('includes original_image_path on every image', async () => {
|
||||||
|
await seedDraftWithImage();
|
||||||
|
|
||||||
|
const res = await request(app).get('/api/admin/item-drafts');
|
||||||
|
|
||||||
|
expect(res.body.drafts[0].images[0]).toHaveProperty('original_image_path', null);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('cuts out one photo and answers with its new paths', async () => {
|
||||||
|
await startStub(200, PNG_BYTES);
|
||||||
|
const { itemId, imageId } = await seedDraftWithImage();
|
||||||
|
|
||||||
|
const res = await request(app).post(
|
||||||
|
`/api/admin/item-drafts/${itemId}/images/${imageId}/remove-background`
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(res.body.image_path).toBe('/uploads/original-cutout.png');
|
||||||
|
expect(res.body.original_image_path).toBe('/uploads/original.jpg');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('puts the original back', async () => {
|
||||||
|
await startStub(200, PNG_BYTES);
|
||||||
|
const { itemId, imageId } = await seedDraftWithImage();
|
||||||
|
await request(app).post(
|
||||||
|
`/api/admin/item-drafts/${itemId}/images/${imageId}/remove-background`
|
||||||
|
);
|
||||||
|
|
||||||
|
const res = await request(app).post(
|
||||||
|
`/api/admin/item-drafts/${itemId}/images/${imageId}/restore-original`
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(res.body.image_path).toBe('/uploads/original.jpg');
|
||||||
|
expect(res.body.original_image_path).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
// 502 rather than 500: the request was fine and the app is fine, and saying
|
||||||
|
// which of the two failed is what stops somebody searching the application
|
||||||
|
// logs for a fault that is not there.
|
||||||
|
it('answers 502 when the sidecar will not, and leaves the photo alone', async () => {
|
||||||
|
await startStub(500, 'boom');
|
||||||
|
const { itemId, imageId } = await seedDraftWithImage();
|
||||||
|
|
||||||
|
const res = await request(app).post(
|
||||||
|
`/api/admin/item-drafts/${itemId}/images/${imageId}/remove-background`
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(res.status).toBe(502);
|
||||||
|
const { rows } = await pool.query<{ image_path: string }>(
|
||||||
|
`SELECT image_path FROM item_images WHERE id = $1`,
|
||||||
|
[imageId]
|
||||||
|
);
|
||||||
|
expect(rows[0]?.image_path).toBe('/uploads/original.jpg');
|
||||||
|
});
|
||||||
|
|
||||||
|
// Scoped by item as well as by image. The id is a serial, so guessing one is
|
||||||
|
// not hard, and a photo from another submission must not be reachable
|
||||||
|
// through this item's URL.
|
||||||
|
it('refuses an image that does not belong to the item', async () => {
|
||||||
|
await startStub(200, PNG_BYTES);
|
||||||
|
const first = await seedDraftWithImage();
|
||||||
|
const second = await seedDraftWithImage();
|
||||||
|
|
||||||
|
const res = await request(app).post(
|
||||||
|
`/api/admin/item-drafts/${first.itemId}/images/${second.imageId}/remove-background`
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(res.status).toBe(404);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('refuses to restore a photo that was never cut out', async () => {
|
||||||
|
const { itemId, imageId } = await seedDraftWithImage();
|
||||||
|
|
||||||
|
const res = await request(app).post(
|
||||||
|
`/api/admin/item-drafts/${itemId}/images/${imageId}/restore-original`
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(res.status).toBe(404);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user