openvibers/openvibe.community Commits

Community: retire old v1 screenshot files after a reference check (#266)

OP OpenVibers committed 7 days ago · 4e9b6baaaf · 5 files, +146 −33

The screenshot move treated a 409 media.object.legacy_managed from Media as a refusal, so the old Community v1 file objects of 28 pastes were retried on every run. Retire that shape through Media's internal legacy route, but only when no other paste, post, attachment or submission still references the Media id: those rows are read-only SELECTs in Community's own tables, and a hit leaves previous_media_ref for a later retry and names the tables in the summary. The --retire-ids path uses the same check.

Changes

ddiff --git a/CHANGELOG.md b/CHANGELOG.md
iindex 7ddc6c5..cac5eab 100644
---- a/CHANGELOG.md
++++ b/CHANGELOG.md
@@@ -6,6 +6,12 @@
`legacy:live:paste:<slug>` object it keeps `previous_media_ref` and retires the object through Media's internal legacy-paste
route. `--apply` pages with an id cursor and reaches every paste in one run, `--retire-ids <file>` retires named objects
whose paste record was lost (one Media id per line), and the dry run prints the moves, retires and refusals it would make.
++- The screenshot move script also retires an old Community v1 file object (`legacy:community:file:<key>`, app
++ `community`, kind `file`) through Media's internal legacy route when DELETE answers 409
++ `media.object.legacy_managed`. Before retiring it, the script reads Community's own tables (pastes, posts,
++ attachments, submissions) and leaves the object in place when any other row still references the Media id, keeping
++ `previous_media_ref` for a later retry; the summary counts those as `referenced` and names the tables. Test:
++ `test/move-screenshots.test.js`.
- Review follow-up: a failed review keeps the owner's requested visibility/audience/NSFW choice in
`review_requested_*`; `server/pastes/review-retry.js` retries pending reviews with backoff and restores the
visibility and share audience when Vision later says fine. Migration `0018_paste_review_retry.sql`, Contracts
ddiff --git a/scripts/move-screenshots-private.js b/scripts/move-screenshots-private.js
iindex 19afdfd..06f57bf 100644
---- a/scripts/move-screenshots-private.js
++++ b/scripts/move-screenshots-private.js
@@@ -13,11 +13,15 @@
* The old id is recorded as `previous_media_ref` before the delete, so a failed delete is retried on
* the next run instead of being forgotten.
*
-- * The old object may be Community's own (delete it through the community tenant) or an inherited
-- * Live-era paste screenshot (`legacy:live:paste:<slug>` in Media's live tenant). Media's
-- * tenant-scoped delete answers 404 for the latter and its v2 delete refuses every legacy object, so
-- * the script retires those through Media's internal legacy-paste route. A 404 is therefore a failure
-- * that keeps `previous_media_ref`: it never means "already gone".
++ * The old object may be Community's own native object (delete it through the community tenant), an
++ * inherited Live-era paste screenshot (`legacy:live:paste:<slug>` in Media's live tenant), or an older
++ * Community v1 file (`legacy:community:file:<key>` in the community tenant). Media's tenant-scoped
++ * delete answers 404 for the Live one and its v2 delete refuses every legacy object with 409
++ * media.object.legacy_managed, so the script retires both through Media's internal legacy route. A
++ * legacy object in this community tenant is retired only after checking that no other paste, post,
++ * attachment or submission still references its Media id: deleting bytes another page still shows
++ * would break it. A 404, 409 or a reference is therefore a failure that keeps `previous_media_ref`:
++ * it never means "already gone".
*
* --apply pages through every screenshot paste with an id cursor, in batches of --limit (500 by
* default), so one run reaches every row instead of re-reading the first batch forever. A dry run
@@@ -66,6 +70,34 @@ function readRetireIds(file) {
return ids;
}
++/**
++ * The Community rows that still point at one Media id: another paste's media_ref, screenshot_url or
++ * metadata, a post body, an attachment, or a submission. Read-only SELECTs against Community's own
++ * tables; `exceptPasteId` leaves the paste row being moved out (its previous_media_ref names the
++ * object on purpose). Returns [{ table, count }] for the tables with a hit, empty when it is safe to
++ * retire the old bytes.
++ */
++async function referencesToMediaId(db, id, { exceptPasteId = null } = {}) {
++ const count = async (sql, ...args) => Number((await db.prepare(sql).get(...args)).n) || 0;
++ const hits = [];
++ const pastes = await count(`SELECT count(*) AS n FROM pastes
++ WHERE deleted_at IS NULL AND id <> ? AND (media_ref = ? OR strpos(coalesce(screenshot_url, ''), ?) > 0 OR strpos(coalesce(metadata, ''), ?) > 0)`,
++ exceptPasteId == null ? -1 : exceptPasteId, id, id, id);
++ if (pastes) hits.push({ table: 'pastes', count: pastes });
++ const posts = await count(`SELECT count(*) AS n FROM posts
++ WHERE deleted_at IS NULL AND strpos(coalesce(body_markdown, ''), ?) > 0`, id);
++ if (posts) hits.push({ table: 'posts', count: posts });
++ const attachments = await count(`SELECT count(*) AS n FROM attachments
++ WHERE media_id = ? OR strpos(coalesce(url, ''), ?) > 0`, id, id);
++ if (attachments) hits.push({ table: 'attachments', count: attachments });
++ const submissions = await count(`SELECT count(*) AS n FROM submissions
++ WHERE media_ref = ? OR strpos(coalesce(url, ''), ?) > 0`, id, id);
++ if (submissions) hits.push({ table: 'submissions', count: submissions });
++ return hits;
++}
++
++const describeReferences = (hits) => hits.map((h) => `${h.table} (${h.count})`).join(', ');
++
/**
* Move every unprivate screenshot paste, or retire the ids in --retire-ids, or (a dry run) say what
* would happen. Returns the counts printed by main(); failures never clear previous_media_ref.
@@@ -74,7 +106,7 @@ async function moveScreenshotsPrivate({ db, mediaObjects, fetchImpl = globalThis
const pageSize = Math.max(1, parseInt(limit, 10) || 50);
const out = {
seen: 0, moved: 0, deleted: 0, retired: 0, alreadyRetired: 0, deleteFailed: 0, retireFailed: 0,
-- refused: 0, notDeletable: 0, skipped: 0, failed: 0, batches: 0, dryRun: !apply,
++ referenced: 0, refused: 0, notDeletable: 0, skipped: 0, failed: 0, batches: 0, dryRun: !apply,
plan: { move: 0, retire: 0, refuse: 0 }, failures: [],
};
@@@ -85,32 +117,47 @@ async function moveScreenshotsPrivate({ db, mediaObjects, fetchImpl = globalThis
};
/**
-- * Delete Community's own object, or retire a legacy Live paste screenshot on the 404. A 409 or
-- * any other answer is a refusal/failure, never "gone". previous_media_ref was written first.
++ * Delete Community's own object, or retire a legacy one. A 404 is a legacy Live paste
++ * screenshot (another tenant); a 409 media.object.legacy_managed is a legacy object in this
++ * tenant (an old Community v1 file). The latter is retired only when no other paste, post or
++ * attachment still references it. Any other answer, or a referenced object, is a refusal that
++ * keeps previous_media_ref for the next run; none of them means "gone". The reference was
++ * written before the delete.
*/
const removeOld = async (row, meta, id) => {
const withRef = { ...meta, previous_media_ref: id };
await db.prepare('UPDATE pastes SET metadata = ?, updated_at = ov_now() WHERE id = ?').run(JSON.stringify(withRef), row.id);
++ const retire = async () => {
++ try {
++ const r = await mediaObjects.retireLegacyPaste(id);
++ out.retired++;
++ if (r && r.changed === false) out.alreadyRetired++;
++ log.log(`${r && r.changed === false ? 'already retired' : 'retired'} legacy old object ${id} of ${row.slug}`);
++ } catch (retireErr) {
++ out.retireFailed++;
++ if (retireErr.status === 409 || retireErr.status === 403 || retireErr.status === 404) out.refused++;
++ throw retireErr;
++ }
++ };
try {
try {
await mediaObjects.deleteObject(id);
out.deleted++;
log.log(`deleted old object ${id} of ${row.slug}`);
} catch (err) {
-- if (err.status !== 404) {
++ if (err.status === 404) {
++ await retire();
++ } else if (err.status === 409 && err.code === 'media.object.legacy_managed') {
++ const refs = await referencesToMediaId(db, id, { exceptPasteId: row.id });
++ if (refs.length) {
++ out.referenced++;
++ throw new Error(`still referenced by ${describeReferences(refs)}`);
++ }
++ await retire();
++ } else {
if (err.status === 409 || err.status === 403) out.refused++;
throw err;
}
-- try {
-- const r = await mediaObjects.retireLegacyPaste(id);
-- out.retired++;
-- if (r && r.changed === false) out.alreadyRetired++;
-- log.log(`retired legacy old object ${id} of ${row.slug}`);
-- } catch (retireErr) {
-- out.retireFailed++;
-- if (retireErr.status === 409 || retireErr.status === 403 || retireErr.status === 404) out.refused++;
-- throw retireErr;
-- }
}
const cleared = { ...withRef };
delete cleared.previous_media_ref;
@@@ -175,7 +222,7 @@ async function moveScreenshotsPrivate({ db, mediaObjects, fetchImpl = globalThis
log.log(`moved ${row.slug} → ${stored.id}`);
if (!keepOld) {
if (old) await removeOld(row, next, old);
-- // A legacy:community:file object has no v2 id to delete; it is left in place and said so.
++ // No Media v2 id (an old /f/ URL or similar): there is nothing to delete this way.
else refuse(row, null, 'the old screenshot is not a Media v2 object');
}
} catch (err) {
@@@ -185,7 +232,8 @@ async function moveScreenshotsPrivate({ db, mediaObjects, fetchImpl = globalThis
}
};
-- // A lost paste record: retire exactly the named objects, no paste scan.
++ // A lost paste record: retire exactly the named objects, no paste scan. A named object is still
++ // left alone when a paste, post, attachment or submission points at it.
if (retireIds) {
for (const id of retireIds) {
out.seen++;
@@@ -194,6 +242,16 @@ async function moveScreenshotsPrivate({ db, mediaObjects, fetchImpl = globalThis
refuse(null, id, 'not a Media object id');
continue;
}
++ const refs = await referencesToMediaId(db, id);
++ if (refs.length) {
++ out.referenced++;
++ out.refused++;
++ out.plan.refuse++;
++ const why = `still referenced by ${describeReferences(refs)}`;
++ out.failures.push({ object: id, error: why });
++ log.warn(`${id} is ${why}; left in place`);
++ continue;
++ }
if (!apply) { out.plan.retire++; log.log(`would retire ${id}`); continue; }
try {
const r = await mediaObjects.retireLegacyPaste(id);
@@@ -254,4 +312,4 @@ async function main(argv = process.argv.slice(2)) {
if (require.main === module) main().catch((err) => { console.error(err.message || err); process.exitCode = 1; });
--module.exports = { moveScreenshotsPrivate, oldObjectId, readRetireIds };
++module.exports = { moveScreenshotsPrivate, oldObjectId, readRetireIds, referencesToMediaId };
ddiff --git a/server/app.js b/server/app.js
iindex 3099c7d..f2dea03 100644
---- a/server/app.js
++++ b/server/app.js
@@@ -204,9 +204,10 @@ async function createApp(opts = {}) {
},
/**
* The screenshot follows a transfer: re-host the same bytes privately, owned by the new owner,
-- * then delete the old object (retire it when it is a legacy Live paste screenshot).
-- * previous_media_ref is recorded before the delete, so a failure is retried by
-- * scripts/move-screenshots-private.js instead of leaving the old URL readable.
++ * then delete the old object (a legacy object is left to the move script, which checks other
++ * references before retiring it). previous_media_ref is recorded before the delete, so a
++ * failure is retried by scripts/move-screenshots-private.js instead of leaving the old URL
++ * readable.
*/
async moveOwner(row, owner) {
if (!owner || !mediaObjects.configured) return false;
@@@ -224,7 +225,9 @@ async function createApp(opts = {}) {
try {
await mediaObjects.deleteObject(old);
} catch (err) {
-- // A 404 is a legacy Live paste screenshot, not "already gone": retire it.
++ // A 404 is a legacy Live paste screenshot, not "already gone": retire it. A 409
++ // legacy_managed (an old Community v1 file) waits for the move script, which checks
++ // that no other paste, post or attachment still references the id.
if (err.status !== 404) throw err;
await mediaObjects.retireLegacyPaste(old);
}
ddiff --git a/server/media/objects.js b/server/media/objects.js
iindex c8b2833..4b96cc1 100644
---- a/server/media/objects.js
++++ b/server/media/objects.js
@@@ -87,7 +87,9 @@ function createMediaObjects({ config, fetchImpl = globalThis.fetch } = {}) {
* Needs media.object.delete in the community tenant. A 404 is NOT "already gone": Media answers
* it for an object outside the caller's tenant (a legacy Live paste, media.object.legacy_managed
* on the direct route), and the move script must keep the reference and retire that object
-- * through retireLegacyPaste instead.
++ * through retireLegacyPaste instead. An older Community v1 file is in this tenant and answers
++ * 409 media.object.legacy_managed; the move script retires it the same way, after its reference
++ * check.
*/
async function deleteObject(id) {
await call('DELETE', `/${encodeURIComponent(String(id))}`);
@@@ -95,10 +97,10 @@ function createMediaObjects({ config, fetchImpl = globalThis.fetch } = {}) {
}
/**
-- * Retire one legacy Live paste screenshot through Media's internal route (the object is in the
-- * live tenant, so deleteObject answers 404 and the v2 delete would refuse it). Only the exact
-- * legacy:live:paste:<slug> screenshot kind is accepted by Media, only for Community's service
-- * principal; a repeat is a no-op (changed:false). Needs media.object.delete.
++ * Retire one legacy object through Media's internal route: a Live paste screenshot in the live
++ * tenant (deleteObject answers 404) or a Community v1 file in this tenant (deleteObject answers
++ * 409 media.object.legacy_managed). Media accepts those two legacy shapes only, only for
++ * Community's service principal; a repeat is a no-op (changed:false). Needs media.object.delete.
*/
async function retireLegacyPaste(id) {
if (!configured) { const e = new Error('Retiring a Media object needs Community\'s service principal (OV_OAUTH_CLIENT_SECRET)'); e.status = 503; throw e; }
ddiff --git a/test/move-screenshots.test.js b/test/move-screenshots.test.js
iindex 1ad3e73..28b8265 100644
---- a/test/move-screenshots.test.js
++++ b/test/move-screenshots.test.js
@@@ -82,7 +82,8 @@ const { testDb } = require('./helpers/db');
async deleteObject(id) {
const mode = this.mode.get(id) || 'ok';
if (mode === '404') { const e = new Error(`Media DELETE 404: not in tenant`); e.status = 404; throw e; }
-- if (mode === '409') { const e = new Error(`Media DELETE 409: legacy managed`); e.status = 409; e.code = 'media.object.legacy_managed'; throw e; }
++ if (mode === '409') { const e = new Error(`Media DELETE 409: held`); e.status = 409; e.code = 'media.object.held'; throw e; }
++ if (mode === 'legacy') { const e = new Error(`Media DELETE 409: legacy managed`); e.status = 409; e.code = 'media.object.legacy_managed'; throw e; }
this.deleted.push(id);
},
async retireLegacyPaste(id) {
@@@ -93,7 +94,8 @@ const { testDb } = require('./helpers/db');
return { retired: true, changed };
},
};
-- // A legacy Live object answers 404 to Community's delete; the held one is refused.
++ // A legacy Live object answers 404 to Community's delete; the held one is refused; an old
++ // Community v1 file answers 409 legacy_managed and waits for the reference check.
fake.mode.set(old.legacy, '404');
fake.mode.set(old.held, '409');
const fetchImpl = async () => new Response(Buffer.from('OLD'), { status: 200, headers: { 'content-type': 'image/png' } });
@@@ -134,6 +136,48 @@ const { testDb } = require('./helpers/db');
assert.strictEqual(JSON.parse((await row('row-held')).metadata).previous_media_ref, undefined, 'the retry cleared it');
console.log('✅ a later run retries the kept reference and clears it');
++ // ── A legacy Community v1 file is retired only when nothing else references it ──
++ const commFile = ids.newId('media');
++ const refPaste = ids.newId('media');
++ const refPost = ids.newId('media');
++ const refAttachment = ids.newId('media');
++ const refSubmission = ids.newId('media');
++ for (const [slug, object] of [
++ ['row-community-file', commFile], ['row-ref-paste', refPaste], ['row-ref-post', refPost],
++ ['row-ref-attachment', refAttachment], ['row-ref-submission', refSubmission],
++ ]) {
++ await add(slug, object, JSON.stringify({ media_visibility: 'private', previous_media_ref: object }));
++ fake.mode.set(object, 'legacy');
++ }
++ await db.prepare(`INSERT INTO pastes (slug, type, title, content, visibility, media_ref)
++ VALUES ('ref-holder', 'paste', 'holder', '', 'public', ?)`).run(refPaste);
++ const space = (await db.prepare("INSERT INTO spaces (slug, name) VALUES ('ref-space', 'Reference space') RETURNING id").run()).lastInsertRowid;
++ const thread = (await db.prepare("INSERT INTO threads (space_id, slug, title) VALUES (?, 'ref-thread', 'Reference thread') RETURNING id").run(space)).lastInsertRowid;
++ await db.prepare('INSERT INTO posts (thread_id, body_markdown) VALUES (?, ?)').run(thread, `![old](${mediaUrl}/o/${refPost})`);
++ const owner = ids.newId('user');
++ await db.prepare(`INSERT INTO attachments (media_id, owner_subject, filename, mime, size_bytes, url)
++ VALUES (?, ?, 'old.png', 'image/png', 3, ?)`).run(refAttachment, owner, `${mediaUrl}/o/${refAttachment}`);
++ await db.prepare(`INSERT INTO submissions (slug, kind, title, body, media_ref, author_subject)
++ VALUES ('ref-submission', 'art', 'Reference art', '', ?, ?)`).run(refSubmission, owner);
++
++ out = await moveScreenshotsPrivate({ db, mediaObjects: fake, fetchImpl, log, limit: 100, apply: true });
++ assert.deepStrictEqual([out.retired, out.referenced, out.deleteFailed, out.refused], [1, 4, 4, 0], JSON.stringify(out));
++ assert.ok(fake.retired.includes(commFile), 'the unreferenced legacy community file was retired');
++ for (const id of [refPaste, refPost, refAttachment, refSubmission]) assert.strictEqual(fake.retired.includes(id), false, `${id} stays`);
++ assert.strictEqual(JSON.parse((await row('row-community-file')).metadata).previous_media_ref, undefined, 'the retired file clears its record');
++ const expected = { 'row-ref-paste': refPaste, 'row-ref-post': refPost, 'row-ref-attachment': refAttachment, 'row-ref-submission': refSubmission };
++ for (const [slug, id] of Object.entries(expected)) {
++ assert.strictEqual(JSON.parse((await row(slug)).metadata).previous_media_ref, id, `${slug} keeps its reference`);
++ }
++ for (const where of ['pastes (1)', 'posts (1)', 'attachments (1)', 'submissions (1)']) {
++ assert.ok(logs.some((l) => l.includes(`still referenced by ${where}`)), `the summary says ${where}`);
++ }
++ console.log('✅ a legacy Community file is retired only when no paste, post, attachment or submission references it');
++
++ out = await moveScreenshotsPrivate({ db, mediaObjects: fake, fetchImpl, log, apply: true, retireIds: [refPaste] });
++ assert.deepStrictEqual([out.referenced, out.refused, out.retired, out.retireFailed], [1, 1, 0, 0], JSON.stringify(out));
++ console.log('✅ --retire-ids also leaves a referenced object alone');
++
// --retire-ids: only those ids, no paste scan, dry run first; a repeat is Media's no-op.
const lost1 = ids.newId('media');
const lost2 = ids.newId('media');