From 16219eb0812aad42664e1262d20abee224f76dbe Mon Sep 17 00:00:00 2001 From: Nikita Menkovich Date: Fri, 9 Oct 2026 10:38:39 +0000 Subject: [PATCH 1/2] fix: retry legacy inline image repair on startup --- scripts/legacy-image-repair.test.js | 33 ++++++++ server/index.js | 4 +- server/lib/img-url-migration.js | 126 +++++----------------------- 3 files changed, 56 insertions(+), 107 deletions(-) create mode 100644 scripts/legacy-image-repair.test.js diff --git a/scripts/legacy-image-repair.test.js b/scripts/legacy-image-repair.test.js new file mode 100644 index 00000000..8119077f --- /dev/null +++ b/scripts/legacy-image-repair.test.js @@ -0,0 +1,33 @@ +import assert from 'node:assert/strict'; +import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import test, { after } from 'node:test'; + +const temp = mkdtempSync(join(tmpdir(), 'nt-legacy-images-')); +process.env.DB_PATH = join(temp, 'test.db'); +process.env.UPLOADS_PATH = join(temp, 'uploads'); +process.env.LOG_LEVEL = 'error'; +const { default: db } = await import('../server/db.js'); +const { migrateDataUrlImages } = await import('../server/lib/img-url-migration.js'); +after(() => { db.close(); rmSync(temp, { recursive: true, force: true }); }); + +test('legacy repair ignores the old flag, retries failures and advances the sync cursor', async () => { + const bytes = Buffer.concat([Buffer.from('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+jRZkAAAAASUVORK5CYII=', 'base64'), Buffer.alloc(64)]); + const inline = `data:image/png;base64,${bytes.toString('base64')}`; + db.prepare("INSERT OR REPLACE INTO app_config (key,value) VALUES ('img_url_data_urls_migrated_v1','done')").run(); + const id = db.prepare("INSERT INTO meals (name,img_url,is_recipe,updated_at) VALUES ('Legacy photo',?,1,'2000-01-01 00:00:00')").run(inline).lastInsertRowid; + const bad = db.prepare("INSERT INTO foods (name,img_url) VALUES ('Broken legacy','data:image/png;base64,bad')").run().lastInsertRowid; + db.prepare("UPDATE meals SET changed_at='2000-01-01 00:00:00' WHERE id=?").run(id); + assert.deepEqual(await migrateDataUrlImages(), { migrated: 1, failed: 1 }); + const row = db.prepare('SELECT * FROM meals WHERE id=?').get(id); + assert.match(row.img_url, /^\/uploads\//); + assert.deepEqual(readFileSync(join(temp, row.img_url)), bytes); + assert.equal(row.updated_at, '2000-01-01 00:00:00'); + assert.ok(row.changed_at > '2000-01-01 00:00:00'); + assert.deepEqual(await migrateDataUrlImages(), { migrated: 0, failed: 1 }); + assert.equal(db.prepare('SELECT img_url FROM meals WHERE id=?').get(id).img_url, row.img_url); + db.prepare('UPDATE foods SET img_url=? WHERE id=?').run(inline, bad); + assert.deepEqual(await migrateDataUrlImages(), { migrated: 1, failed: 0 }); + assert.deepEqual(await migrateDataUrlImages(), { migrated: 0, failed: 0 }); +}); diff --git a/server/index.js b/server/index.js index 7834b9ca..175d04f4 100644 --- a/server/index.js +++ b/server/index.js @@ -462,9 +462,9 @@ app.listen(PORT, async () => { logger.warn(`[scheduler] failed to start: ${e.message}`); }); - // #199 one-shot: localize any data-URL img_urls in foods/meals to + // Repair legacy data-URL img_urls in foods/meals on every startup to // /uploads/ files so the diary hydrator stops amplifying them. - // Guarded by app_config flag; idempotent; fire-and-forget so it + // Scans inline images only; idempotent; fire-and-forget so it // doesn't delay accepting traffic. import('./lib/img-url-migration.js').then(({ migrateDataUrlImages }) => migrateDataUrlImages()).catch(e => { logger.warn(`[img-url-migration] failed to start: ${e.message}`); diff --git a/server/lib/img-url-migration.js b/server/lib/img-url-migration.js index 8323961f..72f08ca5 100644 --- a/server/lib/img-url-migration.js +++ b/server/lib/img-url-migration.js @@ -1,119 +1,35 @@ -/** - * img-url-migration.js — one-shot boot migration for #199. - * - * Foods and meals whose img_url column holds a base64 data URL get - * localized to /uploads/ files on server startup, so the diary's - * freshenItemImages hydrator (which was stamping data URLs onto every - * referencing diary item) stops silently amplifying them into 50 MB - * /api/diary payloads. - * - * Idempotent: guarded by an app_config flag so it runs at most once - * per instance. Async because localizeImage does file IO; can't sit - * inside db.js's synchronous migration block. Invoked from server - * bootstrap after `app.listen` (see server/index.js). - * - * Design choices worth naming: - * - * - **Runs after listen, not during.** Server accepts traffic while - * the migration works. Existing installs with hundreds of data-URL - * rows don't delay boot. The freshenItemImages filter (added in the - * same fix) already keeps the payload clean during the window - * between listen and migration completion, so serving traffic is - * safe throughout. - * - **Does NOT bump updated_at.** Same reasoning as the diary shrink - * migration in db.js: a mass updated_at bump would show every row - * as changed on the next Android sync/pull. Foods/meals dbUpsert - * guards pending edits, but skipping the bump avoids the risk - * entirely. Native clients pick up the /uploads/ path on the next - * natural edit (their PUT sends the local data URL, server - * re-localizes fresh via POST/PUT's own localizeImage path). - * - **Per-row try/catch.** One malformed data URL (truncated, - * unknown mime type, disk full mid-write) can't abort the whole - * pass. Failures are counted and logged; the row stays as-is and - * the read-side filter keeps it out of the diary payload. +/** Repair legacy embedded food/recipe photos on each startup. + * The old one-shot flag missed later recipe edits and imports. Only data URLs + * are scanned; ordinary upload paths and external URLs are never re-downloaded. */ import db from '../db.js'; import { logger } from '../logger.js'; import { localizeImage } from './image-localizer.js'; -const FLAG_KEY = 'img_url_data_urls_migrated_v1'; - -let _promise = null; - +let pending; export function migrateDataUrlImages() { - if (_promise) return _promise; - _promise = _run(); - return _promise; + if (!pending) pending = run().finally(() => { pending = null; }); + return pending; } -async function _run() { - try { - const done = db.prepare(`SELECT value FROM app_config WHERE key = ?`).get(FLAG_KEY); - if (done) return { skipped: true, reason: 'already-migrated' }; - - const foods = db.prepare( - `SELECT id, img_url FROM foods WHERE deleted_at IS NULL AND img_url LIKE 'data:%'` - ).all(); - const meals = db.prepare( - `SELECT id, img_url FROM meals WHERE deleted_at IS NULL AND img_url LIKE 'data:%'` - ).all(); - - if (foods.length === 0 && meals.length === 0) { - db.prepare(`INSERT OR REPLACE INTO app_config (key, value) VALUES (?, ?)`) - .run(FLAG_KEY, new Date().toISOString()); - return { skipped: true, reason: 'no-data-urls', migrated: 0 }; - } - - const foodUpdate = db.prepare(`UPDATE foods SET img_url = ? WHERE id = ?`); - const mealUpdate = db.prepare(`UPDATE meals SET img_url = ? WHERE id = ?`); - - let migrated = 0, failed = 0; - for (const r of foods) { +async function run() { + let migrated = 0, failed = 0; + for (const table of ['foods', 'meals']) { + const rows = db.prepare(`SELECT id, img_url FROM ${table} WHERE img_url LIKE 'data:%'`).all(); + // changed_at triggers make the corrected URL visible to Android pulls. + // Preserve updated_at (the edit time) and never overwrite a concurrent edit. + const update = db.prepare(`UPDATE ${table} SET img_url = ? WHERE id = ? AND img_url = ?`); + for (const row of rows) { try { - const local = await localizeImage(r.img_url); - if (local && !local.startsWith('data:')) { - foodUpdate.run(local, r.id); - migrated++; - } else { - failed++; - } - } catch (e) { + const image = await localizeImage(row.img_url); + if (!image || /^data:/i.test(image)) throw new Error('Image conversion failed'); + migrated += update.run(image, row.id, row.img_url).changes; + } catch { failed++; - logger.warn(`[img-url-migration] food id=${r.id} failed: ${e?.message || e}`); + logger.warn(`[img-url-migration] ${table} id=${row.id}: could not store image; will retry next startup`); } } - for (const r of meals) { - try { - const local = await localizeImage(r.img_url); - if (local && !local.startsWith('data:')) { - mealUpdate.run(local, r.id); - migrated++; - } else { - failed++; - } - } catch (e) { - failed++; - logger.warn(`[img-url-migration] meal id=${r.id} failed: ${e?.message || e}`); - } - } - - // Only stamp the flag if EVERY row succeeded. A partial pass leaves - // the flag off so the next boot retries the failures. That is safe - // because localizeImage is idempotent (data URLs hash to the same - // filename), and the already-migrated rows short-circuit via - // freshenItemImages's data: filter regardless. - if (failed === 0) { - db.prepare(`INSERT OR REPLACE INTO app_config (key, value) VALUES (?, ?)`) - .run(FLAG_KEY, new Date().toISOString()); - } - - logger.info( - `[img-url-migration] localized ${migrated} data-URL image(s) to /uploads/` + - (failed ? `, ${failed} failed (will retry on next boot)` : '') - ); - return { migrated, failed }; - } catch (e) { - logger.warn(`[img-url-migration] pass failed: ${e?.message || e}`); - return { failed: true, error: e?.message || String(e) }; } + if (migrated || failed) logger.info(`[img-url-migration] localized ${migrated} image(s), ${failed} failed`); + return { migrated, failed }; } From fb629885d2ee2f1bdb8a1b38af69472ab51ce13a Mon Sep 17 00:00:00 2001 From: thebigjoe1 <26659217+thebigjoe1@users.noreply.github.com> Date: Fri, 9 Oct 2026 07:56:42 -0400 Subject: [PATCH 2/2] Skip deleted foods and recipes in the inline image repair --- scripts/legacy-image-repair.test.js | 2 ++ server/lib/img-url-migration.js | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/legacy-image-repair.test.js b/scripts/legacy-image-repair.test.js index 8119077f..27435065 100644 --- a/scripts/legacy-image-repair.test.js +++ b/scripts/legacy-image-repair.test.js @@ -18,6 +18,7 @@ test('legacy repair ignores the old flag, retries failures and advances the sync db.prepare("INSERT OR REPLACE INTO app_config (key,value) VALUES ('img_url_data_urls_migrated_v1','done')").run(); const id = db.prepare("INSERT INTO meals (name,img_url,is_recipe,updated_at) VALUES ('Legacy photo',?,1,'2000-01-01 00:00:00')").run(inline).lastInsertRowid; const bad = db.prepare("INSERT INTO foods (name,img_url) VALUES ('Broken legacy','data:image/png;base64,bad')").run().lastInsertRowid; + const gone = db.prepare("INSERT INTO meals (name,img_url,is_recipe,deleted_at) VALUES ('Deleted photo',?,1,datetime('now'))").run(inline).lastInsertRowid; db.prepare("UPDATE meals SET changed_at='2000-01-01 00:00:00' WHERE id=?").run(id); assert.deepEqual(await migrateDataUrlImages(), { migrated: 1, failed: 1 }); const row = db.prepare('SELECT * FROM meals WHERE id=?').get(id); @@ -30,4 +31,5 @@ test('legacy repair ignores the old flag, retries failures and advances the sync db.prepare('UPDATE foods SET img_url=? WHERE id=?').run(inline, bad); assert.deepEqual(await migrateDataUrlImages(), { migrated: 1, failed: 0 }); assert.deepEqual(await migrateDataUrlImages(), { migrated: 0, failed: 0 }); + assert.equal(db.prepare('SELECT img_url FROM meals WHERE id=?').get(gone).img_url, inline, 'a deleted row is left alone'); }); diff --git a/server/lib/img-url-migration.js b/server/lib/img-url-migration.js index 72f08ca5..05d075e3 100644 --- a/server/lib/img-url-migration.js +++ b/server/lib/img-url-migration.js @@ -1,6 +1,8 @@ /** Repair legacy embedded food/recipe photos on each startup. * The old one-shot flag missed later recipe edits and imports. Only data URLs * are scanned; ordinary upload paths and external URLs are never re-downloaded. + * Deleted rows are skipped: nobody sees their photo, and converting it would + * send them to every phone again for nothing. */ import db from '../db.js'; import { logger } from '../logger.js'; @@ -15,7 +17,7 @@ export function migrateDataUrlImages() { async function run() { let migrated = 0, failed = 0; for (const table of ['foods', 'meals']) { - const rows = db.prepare(`SELECT id, img_url FROM ${table} WHERE img_url LIKE 'data:%'`).all(); + const rows = db.prepare(`SELECT id, img_url FROM ${table} WHERE deleted_at IS NULL AND img_url LIKE 'data:%'`).all(); // changed_at triggers make the corrected URL visible to Android pulls. // Preserve updated_at (the edit time) and never overwrite a concurrent edit. const update = db.prepare(`UPDATE ${table} SET img_url = ? WHERE id = ? AND img_url = ?`);