mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
feat(edit): clean up superseded thumbnail on edit (orphan removal)
Previously editing a video deleted the old hosted video + its .torrent when the
URL changed, but intentionally left the old thumbnail orphaned in the bucket.
Now that the Edit modal can replace a thumbnail (and we have a clear "thumbnail
changed" signal), also delete the superseded thumbnail: build an orphan list with
the old video ({url}) and/or old thumbnail ({thumbnail}) and pass both to
deleteVideoStorage. Each guarded on old & new being non-empty and different, so we
never delete an object the new note still references. Best-effort; never blocks
the edit.
Tests (collectVideoStorageKeys): a {url} orphan yields the video key + sibling
.torrent; a {thumbnail} orphan yields the thumbnail key; both collect together;
objects under a different publicBaseUrl are never collected (bucket-switch safety).
Also documents the pre-existing uploadModal-reset.test.mjs flake in KNOWN_ISSUES
(reproduced on 1b11cb1b; not from this work).
Build + full lint green; full unit suite green except the documented pre-existing
flake.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
c8a63c57c6
commit
3aef97f08a
3 files changed
+81
-7
No files matched your search
@@ -33,3 +33,8 @@
|
||||
- **Status:** Active (pre-existing — reproduced on pre-refactor commit `1b11cb1b`)
|
||||
- **Description:** "plays directly from HTML link (CDN mode)" fails on `--project=e2e-firefox` with `#modalVideo` `src` empty instead of the test's archive.org URL. The Chromium variant and the WebTorrent-fallback variant (`:63`) both pass. Firefox-headless-specific (external URL reachability/codec), not an app regression.
|
||||
- **Impact:** One Firefox-only e2e failure; CDN playback works on Chromium and in manual testing.
|
||||
|
||||
### `uploadModal-reset.test.mjs` ("UploadModal Reset Logic") hangs/cancels
|
||||
- **Status:** Active (pre-existing — reproduced on pre-refactor commit `1b11cb1b`)
|
||||
- **Description:** The jsdom-based UploadModal reset suite intermittently fails with `cancelledByParent` / "Promise resolution is still pending but the event loop has already resolved" (~12s hang). It passes or fails depending on full-suite test ordering/timing (an async-hang flake, likely a torrent-metadata/upload promise not settling under the mocked services). Not a regression from the MediaUploader extraction — it fails identically on `1b11cb1b`.
|
||||
- **Impact:** One unit suite can report `not ok 1`; the runner still exits 0. Other tests unaffected.
|
||||
@@ -92,18 +92,31 @@ export default class EditModalController {
|
||||
pubkey,
|
||||
});
|
||||
|
||||
// Best-effort: if the hosted video URL was genuinely replaced, the old
|
||||
// R2/S3 object (and its .torrent) is now orphaned — remove it. We require
|
||||
// both URLs to be non-empty and different so we never delete an object
|
||||
// that's still referenced (e.g. via a magnet web-seed when the URL was
|
||||
// merely cleared). Thumbnails are intentionally left alone here to avoid
|
||||
// deleting one the new note still points at. Never blocks the edit.
|
||||
// Best-effort: when the hosted video URL and/or thumbnail were genuinely
|
||||
// replaced, the old R2/S3 objects are now orphaned — remove them. For each,
|
||||
// we require both old and new values to be non-empty and different so we
|
||||
// never delete an object the new note still references (e.g. the URL was
|
||||
// merely cleared, or the thumbnail was left unchanged). The video entry
|
||||
// also takes its sibling .torrent. Never blocks the edit.
|
||||
try {
|
||||
const orphans = [];
|
||||
|
||||
const oldUrl = detail.video?.url || "";
|
||||
const newUrl = updatedData?.url || "";
|
||||
if (updatedData?.urlEdited && oldUrl && newUrl && oldUrl !== newUrl) {
|
||||
// collectVideoStorageKeys derives the video key + its .torrent.
|
||||
orphans.push({ url: oldUrl });
|
||||
}
|
||||
|
||||
const oldThumb = detail.video?.thumbnail || "";
|
||||
const newThumb = updatedData?.thumbnail || "";
|
||||
if (oldThumb && newThumb && oldThumb !== newThumb) {
|
||||
orphans.push({ thumbnail: oldThumb });
|
||||
}
|
||||
|
||||
if (orphans.length) {
|
||||
const cleanup = await r2Service.deleteVideoStorage({
|
||||
videos: [{ url: oldUrl }],
|
||||
videos: orphans,
|
||||
pubkey,
|
||||
});
|
||||
if (cleanup?.deleted?.length) {
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
// On edit, superseded R2/S3 objects are removed by passing orphan descriptors to
|
||||
// deleteVideoStorage, which derives the bucket keys via collectVideoStorageKeys.
|
||||
// This guards the descriptors the edit-cleanup builds map to the right keys:
|
||||
// - a replaced video entry -> the video key AND its sibling .torrent
|
||||
// - a replaced thumbnail entry -> the thumbnail key
|
||||
// and that only objects under the configured publicBaseUrl are ever collected.
|
||||
|
||||
import "./test-helpers/setup-localstorage.mjs";
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
import { collectVideoStorageKeys } from "../js/utils/storagePointer.js";
|
||||
|
||||
const BASE = "https://pub-abc.r2.dev";
|
||||
|
||||
test("a replaced video entry yields the video key + its .torrent", () => {
|
||||
const keys = collectVideoStorageKeys({
|
||||
videos: [{ url: `${BASE}/u/npub1x/deadbeef/my-clip.mp4` }],
|
||||
publicBaseUrl: BASE,
|
||||
});
|
||||
assert.deepEqual(
|
||||
keys.sort(),
|
||||
["u/npub1x/deadbeef/my-clip.mp4", "u/npub1x/deadbeef/my-clip.torrent"].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
test("a replaced thumbnail entry yields the thumbnail key", () => {
|
||||
const keys = collectVideoStorageKeys({
|
||||
videos: [{ thumbnail: `${BASE}/npub1x/thumbnails/123-old.png` }],
|
||||
publicBaseUrl: BASE,
|
||||
});
|
||||
assert.deepEqual(keys, ["npub1x/thumbnails/123-old.png"]);
|
||||
});
|
||||
|
||||
test("both video and thumbnail orphans collect together", () => {
|
||||
const keys = collectVideoStorageKeys({
|
||||
videos: [
|
||||
{ url: `${BASE}/u/npub1x/hash/clip.mp4` },
|
||||
{ thumbnail: `${BASE}/npub1x/thumbnails/9-old.jpg` },
|
||||
],
|
||||
publicBaseUrl: BASE,
|
||||
});
|
||||
assert.ok(keys.includes("u/npub1x/hash/clip.mp4"));
|
||||
assert.ok(keys.includes("u/npub1x/hash/clip.torrent"));
|
||||
assert.ok(keys.includes("npub1x/thumbnails/9-old.jpg"));
|
||||
});
|
||||
|
||||
test("objects under a DIFFERENT base are never collected (bucket switch safety)", () => {
|
||||
const keys = collectVideoStorageKeys({
|
||||
videos: [
|
||||
{ url: "https://other-bucket.r2.dev/u/npub1x/hash/clip.mp4" },
|
||||
{ thumbnail: "https://other-bucket.r2.dev/npub1x/thumbnails/9.jpg" },
|
||||
],
|
||||
publicBaseUrl: BASE,
|
||||
});
|
||||
assert.deepEqual(keys, []);
|
||||
});
|
||||
Reference in new issue
Block a user