feat(uploads): backfill re-encoding over already-stored photos (#226)
Linting / lint (pull_request) Successful in 1m56s
SonarQube Analysis / sonarqube (pull_request) Failing after 16m51s

Stripping new uploads does nothing for the catalogue that is already on the storefront, which is where the exposure actually lives today. This is the other half.

Reports by default and rewrites nothing without --apply, because the transform is lossy and there is no undo. Idempotency comes from `needsProcessing` rather than from a marker or a schema change: a file with no EXIF already inside the bounds is already in its final state, so a second run skips it instead of putting it through another lossy pass. Proven rather than assumed — a second --apply immediately after the first reports skipped 1, processed 0.

Verified end to end against a real row and a real file. 3000x2000 carrying GPS EXIF became 2000x1333 with the metadata gone, 35760 bytes down to 16019, the format preserved, no temporary file left behind, and `item_images.image_path` untouched. That last part is what preserving the format bought: the backfill rewrites bytes and writes nothing to the database, so there is no window where a row points at a file that no longer exists.

Both degenerate branches are exercised too, since a script that dies partway through a catalogue leaves the rest of it exposed: a row pointing at a missing file and a row with an extension the application would refuse to serve are each reported and counted, and the run continues.

`handleRow` is split out of `run` for cognitive complexity, and while doing that a miscount was introduced and caught — incrementing `processed` before the rewrite meant a file that threw would have been counted as both processed and failed, which makes the summary unreadable at the moment it matters most.

Ref #226
This commit is contained in:
2026-08-29 13:51:26 -05:00
parent aecccef418
commit 502d56d9fd
2 changed files with 161 additions and 0 deletions
+1
View File
@@ -19,6 +19,7 @@
"test:integration:json": "jest -c jest.integration.config.js --runInBand --json --outputFile=integration-results.json",
"test:integration:cov": "jest -c jest.integration.config.js --runInBand --coverage --forceExit",
"bench:hashing": "tsx scripts/bench-hash-latency.ts",
"backfill:images": "tsx scripts/backfill-image-reencode.ts",
"db:test:up": "docker compose -f docker-compose.test.yml up -d",
"db:test:down": "docker compose -f docker-compose.test.yml down -v",
"migrate:up": "node migrate.js up",
+160
View File
@@ -0,0 +1,160 @@
/**
* Applies #226's re-encoding to the photos that were stored before it existed.
*
* New uploads are handled in the request path. Everything already on the volume
* still carries whatever the camera wrote, including the coordinates the photo
* was taken at, and is still served publicly. This is the other half.
*
* The transform is lossy and there is no undo, so:
*
* - It reports by default and changes nothing without --apply.
* - It is idempotent. `needsProcessing` skips a file that is already stripped
* and already within bounds, so a second run is not a second lossy pass.
* - `reencodeInPlace` writes to a temporary file and renames, so an
* interruption cannot leave a half-written image being served.
* - It never renames the stored file, so item_images.image_path stays correct
* and no database write is needed at all.
*
* Usage
* -----
* npm run backfill:images # report only
* npm run backfill:images -- --apply # rewrite the files
*
* Point it at QA first. Compare a handful of images by eye before production,
* and take a backup that you have confirmed restores.
*/
import sharp from 'sharp';
import { promises as fs } from 'fs';
import path from 'path';
import { pool } from '../src/db';
import { needsProcessing, reencodeInPlace } from '../src/imageProcessing';
const UPLOADS_DIR = process.env.UPLOADS_DIR || '/app/uploads';
const APPLY = process.argv.includes('--apply');
// The stored extension is the file's real type — uploadTypes.ts derives it from
// the validated content type on the way in, so it can be trusted on the way
// back out. Anything else is a file this application would refuse to serve.
const TYPE_FOR_EXTENSION: Readonly<Record<string, string>> = {
'.jpg': 'image/jpeg',
'.png': 'image/png',
'.webp': 'image/webp'
};
interface Totals {
seen: number;
missing: number;
skipped: number;
unrecognised: number;
processed: number;
failed: number;
bytesBefore: number;
bytesAfter: number;
}
/**
* One stored image: classify it, and rewrite it when it needs rewriting.
*
* Split out of `run` so that the loop reads as a loop. Every outcome is
* counted rather than thrown, because one unreadable file in a catalogue is
* not a reason to leave the rest of it exposed.
*/
async function handleRow(imagePath: string, totals: Totals): Promise<void> {
// basename only: image_path is '/uploads/<name>', and the directory it is
// served from is a server constant rather than part of the stored value.
const filePath = path.join(UPLOADS_DIR, path.basename(imagePath));
const mimetype = TYPE_FOR_EXTENSION[path.extname(filePath).toLowerCase()];
if (!mimetype) {
console.warn(`[backfill] unrecognised extension, skipping: ${imagePath}`);
totals.unrecognised++;
return;
}
let before: number;
try {
before = (await fs.stat(filePath)).size;
} catch {
// A row pointing at nothing is a pre-existing inconsistency. Reported
// rather than fatal: it is not this script's job to fix, and stopping
// would leave the rest of the catalogue exposed.
console.warn(`[backfill] file missing for ${imagePath}`);
totals.missing++;
return;
}
try {
const meta = await sharp(filePath).metadata();
if (!needsProcessing(meta)) {
totals.skipped++;
return;
}
if (!APPLY) {
console.info(
`[backfill] would process ${imagePath} ` +
`(${meta.width}x${meta.height}, exif ${meta.exif ? 'present' : 'absent'}, ${before} bytes)`
);
totals.processed++;
totals.bytesBefore += before;
return;
}
await reencodeInPlace(filePath, mimetype);
const after = (await fs.stat(filePath)).size;
// Counted only once the rewrite succeeded, so a file that threw is `failed`
// and nothing else. A row counted as both processed and failed would make
// the summary unreadable at exactly the moment it matters.
totals.processed++;
totals.bytesBefore += before;
totals.bytesAfter += after;
console.info(`[backfill] ${imagePath}: ${before} -> ${after} bytes`);
} catch (err) {
console.error(`[backfill] failed on ${imagePath}:`, err);
totals.failed++;
}
}
async function run(): Promise<void> {
const totals: Totals = {
seen: 0,
missing: 0,
skipped: 0,
unrecognised: 0,
processed: 0,
failed: 0,
bytesBefore: 0,
bytesAfter: 0
};
const { rows } = await pool.query<{ image_path: string }>(
`SELECT image_path FROM item_images ORDER BY id`
);
console.info(
`[backfill] ${rows.length} image rows in ${UPLOADS_DIR}, ` +
`${APPLY ? 'APPLYING CHANGES' : 'reporting only (pass --apply to rewrite)'}`
);
for (const row of rows) {
totals.seen++;
await handleRow(row.image_path, totals);
}
console.info('[backfill] done', totals);
if (totals.failed > 0) {
// A non-zero exit so a partial run is visible to whatever invoked it,
// rather than reading as success because the summary printed.
process.exitCode = 1;
}
}
run()
.catch((err) => {
console.error(err);
process.exitCode = 1;
})
.finally(() => pool.end());