diff --git a/AGENTS.md b/AGENTS.md index 2c7ee0c9..96dd4b72 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -70,6 +70,7 @@ For any nostr related work, please review the nip documentation located in /docs * **Whole-file hash before upload.** When the modal doesn't pre-seed an info-hash, `resolveUploadIdentifier` runs `calculateTorrentInfoHash(file)`, which reads the *entire* file before the upload begins and before any progress is emitted — on multi-GB files this looks like a hang. Emit a "preparing/hashing" status or parallelize. * **Publish can report success unconfirmed.** `publishEventToRelay` (`js/nostrPublish.js`) has an optimistic-success fallthrough when the relay handle exposes no `on()`/`then()` — a note may show "Published" with no relay ACK. Don't tighten blindly (legacy `seen`-only relays rely on it), but log it. * **Silent partial failures.** Thumbnail and `.torrent` upload failures are caught and `warn`-only; the video note still publishes with no user signal. Empty (0-byte) files hit `CompleteMultipartUpload` with `Parts:[]` and surface a cryptic error — guard `file.size === 0`. +* **Object cleanup (deletes/edits).** `R2Service.deleteVideoStorage()` removes the backing objects so "deleted" videos aren't left publicly downloadable. Contract to preserve: it's **best-effort and never throws** (must not block note deletion), it derives keys via `collectVideoStorageKeys()` and **only ever touches URLs under the owner's bucket base** (never external/3rd-party URLs), and it skips cleanly when storage is locked/credential-less. Wired into delete (`app.handleDeleteModalConfirm`) and edit-with-replaced-URL (`editModalController.handleSubmit`). Edit cleanup intentionally does **not** delete thumbnails (the new note may still reference them) and only fires when both old and new URLs are present and different. --- diff --git a/js/ui/editModalController.js b/js/ui/editModalController.js index 5b762ca0..51e2a0f1 100644 --- a/js/ui/editModalController.js +++ b/js/ui/editModalController.js @@ -1,4 +1,5 @@ import { devLogger } from "../utils/logger.js"; +import r2Service from "../services/r2Service.js"; export default class EditModalController { constructor({ services, state, ui, callbacks, helpers }) { @@ -90,6 +91,34 @@ export default class EditModalController { updatedData, 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. + try { + const oldUrl = detail.video?.url || ""; + const newUrl = updatedData?.url || ""; + if (updatedData?.urlEdited && oldUrl && newUrl && oldUrl !== newUrl) { + const cleanup = await r2Service.deleteVideoStorage({ + videos: [{ url: oldUrl }], + pubkey, + }); + if (cleanup?.deleted?.length) { + devLogger.log( + `[edit] Removed ${cleanup.deleted.length} superseded storage object(s).` + ); + } + } + } catch (cleanupErr) { + devLogger.warn( + "[edit] Storage cleanup failed (edit still succeeded):", + cleanupErr + ); + } + await this.callbacks.loadVideos(); const videosMap = this.state.getVideosMap(); diff --git a/tests/ui/edit-modal-storage-cleanup.test.mjs b/tests/ui/edit-modal-storage-cleanup.test.mjs new file mode 100644 index 00000000..75d8fe48 --- /dev/null +++ b/tests/ui/edit-modal-storage-cleanup.test.mjs @@ -0,0 +1,78 @@ +// Scenario (SCN-edit-storage-cleanup): +// Editing a video with a genuinely replaced hosted URL should remove the OLD +// R2/S3 object (now orphaned), but only when both old and new URLs are present +// and different — never when the URL was merely cleared or only metadata +// changed (so we don't delete an object the new note still references). + +import "../test-helpers/setup-localstorage.mjs"; +import test from "node:test"; +import assert from "node:assert/strict"; + +globalThis.__BITVID_DISABLE_NETWORK_IMPORTS__ = true; +const { default: EditModalController } = await import( + "../../js/ui/editModalController.js" +); +const r2Service = (await import("../../js/services/r2Service.js")).default; + +const BASE = "https://pub.bitvid.network"; + +function makeController() { + const calls = []; + r2Service.deleteVideoStorage = async (args) => { + calls.push(args); + return { deleted: ["x"], failed: [], skipped: false, reason: "" }; + }; + const controller = new EditModalController({ + services: { nostrService: { handleEditVideoSubmit: async () => {} } }, + state: { getPubkey: () => "ownerhex", getVideosMap: () => null }, + ui: { + getEditModal: () => ({ setSubmitState() {}, close() {} }), + showError() {}, + showSuccess() {}, + }, + callbacks: { loadVideos: async () => {}, forceRefreshAllProfiles: () => {} }, + helpers: {}, + }); + return { controller, calls }; +} + +const submit = (controller, { oldUrl, newUrl, urlEdited }) => + controller.handleSubmit({ + detail: { + originalEvent: { id: "1", pubkey: "ownerhex", videoRootId: "root" }, + updatedData: { url: newUrl, urlEdited }, + video: { url: oldUrl }, + }, + }); + +test("cleans the old object when the URL is genuinely replaced", async () => { + const { controller, calls } = makeController(); + await submit(controller, { + oldUrl: `${BASE}/u/np/OLD/clip.mp4`, + newUrl: `${BASE}/u/np/NEW/clip.mp4`, + urlEdited: true, + }); + assert.equal(calls.length, 1, "should request cleanup once"); + assert.equal(calls[0].videos[0].url, `${BASE}/u/np/OLD/clip.mp4`); + assert.equal(calls[0].pubkey, "ownerhex"); +}); + +test("does NOT clean when only metadata changed (urlEdited false)", async () => { + const { controller, calls } = makeController(); + await submit(controller, { + oldUrl: `${BASE}/u/np/OLD/clip.mp4`, + newUrl: `${BASE}/u/np/OLD/clip.mp4`, + urlEdited: false, + }); + assert.equal(calls.length, 0); +}); + +test("does NOT clean when the URL was cleared (new URL empty)", async () => { + const { controller, calls } = makeController(); + await submit(controller, { + oldUrl: `${BASE}/u/np/OLD/clip.mp4`, + newUrl: "", + urlEdited: true, + }); + assert.equal(calls.length, 0, "clearing the URL must not delete the object (magnet web-seed may still use it)"); +});