From d4433fb8860428650144763e580db3a7aeb78afc Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 25 Jan 2026 21:24:01 +0000 Subject: [PATCH] fix(UploadModal): cancel and reset uploads on exit, preserve metadata --- js/ui/components/UploadModal.js | 116 +++++++++++--- tests/ui/uploadModal-reset.test.mjs | 224 ++++++++++++++++++++++++++++ 2 files changed, 320 insertions(+), 20 deletions(-) create mode 100644 tests/ui/uploadModal-reset.test.mjs diff --git a/js/ui/components/UploadModal.js b/js/ui/components/UploadModal.js index 1317e725..f486f474 100644 --- a/js/ui/components/UploadModal.js +++ b/js/ui/components/UploadModal.js @@ -123,6 +123,10 @@ export class UploadModal { // Automation this.summaryLocked = true; this.isUploading = false; // Global lock + + // Upload Session IDs (to guard against zombie callbacks) + this.videoUploadId = 0; + this.thumbnailUploadId = 0; } // --- Core Lifecycle --- @@ -472,6 +476,9 @@ export class UploadModal { return; } + // Start new session + const currentUploadId = ++this.videoUploadId; + // Reset Video State this.videoUploadState = { status: 'uploading', @@ -508,6 +515,8 @@ export class UploadModal { const { settings, bucketEntry } = await service.prepareUpload(npub, { credentials: this.activeCredentials }); + if (this.videoUploadId !== currentUploadId) return; + // 2. Determine Keys const videoKey = buildR2Key(npub, file); const baseDomain = bucketEntry.publicBaseUrl; @@ -539,7 +548,9 @@ export class UploadModal { forcePathStyle: settings.forcePathStyle, createBucketIfMissing: true, onProgress: (fraction) => { - this.updateVideoProgress(fraction); + if (this.videoUploadId === currentUploadId) { + this.updateVideoProgress(fraction); + } } }); @@ -552,6 +563,8 @@ export class UploadModal { const [uploadResult, torrentResult] = await Promise.all([uploadPromise, torrentPromise]); + if (this.videoUploadId !== currentUploadId) return; + // Video Complete this.videoUploadState.status = 'complete'; this.videoUploadState.url = videoPublicUrl; @@ -585,6 +598,8 @@ export class UploadModal { createBucketIfMissing: true, }); + if (this.videoUploadId !== currentUploadId) return; + this.torrentState.status = 'complete'; this.torrentState.infoHash = torrentResult.infoHash; this.torrentState.url = torrentPublicUrl; @@ -610,6 +625,8 @@ export class UploadModal { } } catch (err) { + if (this.videoUploadId !== currentUploadId) return; + userLogger.error("Video upload sequence failed:", err); this.videoUploadState.status = 'error'; this.updateVideoProgress(null, "Upload failed."); @@ -628,6 +645,8 @@ export class UploadModal { return; } + const currentUploadId = ++this.thumbnailUploadId; + this.thumbnailUploadState = { status: 'uploading', progress: 0, @@ -648,6 +667,8 @@ export class UploadModal { const { settings, bucketEntry } = await service.prepareUpload(npub, { credentials: this.activeCredentials }); + if (this.thumbnailUploadId !== currentUploadId) return; + // Derive Key (randomish or based on file) // We don't have the video key here easily if video isn't selected yet. // Use a standalone key structure: npub/thumbnails/timestamp-name @@ -673,9 +694,15 @@ export class UploadModal { secretAccessKey: settings.secretAccessKey, forcePathStyle: settings.forcePathStyle, createBucketIfMissing: true, - onProgress: (fraction) => this.updateThumbnailProgress(fraction) + onProgress: (fraction) => { + if (this.thumbnailUploadId === currentUploadId) { + this.updateThumbnailProgress(fraction); + } + } }); + if (this.thumbnailUploadId !== currentUploadId) return; + this.thumbnailUploadState.status = 'complete'; this.thumbnailUploadState.url = publicUrl; this.inputs.thumbnail.value = publicUrl; @@ -684,10 +711,14 @@ export class UploadModal { // Hide progress after a delay setTimeout(() => { - this.sourceSections.thumbnailProgress.classList.add("hidden"); + if (this.thumbnailUploadId === currentUploadId) { + this.sourceSections.thumbnailProgress.classList.add("hidden"); + } }, 2000); } catch (err) { + if (this.thumbnailUploadId !== currentUploadId) return; + userLogger.error("Thumbnail upload failed:", err); this.thumbnailUploadState.status = 'error'; this.updateThumbnailProgress(null, "Failed."); @@ -1041,6 +1072,66 @@ export class UploadModal { } } + resetUploads() { + // Invalidate active uploads + this.videoUploadId++; + this.thumbnailUploadId++; + + // Reset State + this.videoUploadState = { + status: 'idle', + progress: 0, + url: '', + key: '', + file: null, + }; + this.thumbnailUploadState = { + status: 'idle', + progress: 0, + url: '', + key: '', + file: null, + }; + this.torrentState = { + status: 'idle', + infoHash: '', + magnet: '', + url: '', // xs (torrent file url) + file: null, + }; + + // Reset UI + if (this.sourceSections.progress) { + this.sourceSections.progress.classList.add("hidden"); + } + if (this.sourceSections.thumbnailProgress) { + this.sourceSections.thumbnailProgress.classList.add("hidden"); + } + if (this.sourceSections.results) { + this.sourceSections.results.classList.add("hidden"); + } + + // Clear Result Inputs + if (this.results.videoUrl) this.results.videoUrl.value = ""; + if (this.results.magnet) this.results.magnet.value = ""; + if (this.results.torrentUrl) this.results.torrentUrl.value = ""; + + // Reset Inputs + if (this.inputs.file) { + this.inputs.file.value = ""; + } + if (this.inputs.thumbnailFile) { + this.inputs.thumbnailFile.value = ""; + } + // Re-enable thumbnail input if it was disabled by an upload + if (this.inputs.thumbnail) { + this.inputs.thumbnail.disabled = false; + if (this.inputs.thumbnail.placeholder && this.inputs.thumbnail.placeholder.startsWith("Selected:")) { + this.inputs.thumbnail.placeholder = "https://example.com/thumbnail.jpg"; + } + } + } + resetForm() { this.form.reset(); this.nip71FormManager.resetSection("main"); @@ -1050,23 +1141,7 @@ export class UploadModal { this.toggles.nsfw.checked = false; this.toggles.kids.checked = false; - // Reset State - this.videoUploadState = { status: 'idle', progress: 0, url: '', key: '', file: null }; - this.thumbnailUploadState = { status: 'idle', progress: 0, url: '', key: '', file: null }; - this.torrentState = { status: 'idle', infoHash: '', magnet: '', url: '', file: null }; - - this.sourceSections.progress.classList.add("hidden"); - this.sourceSections.thumbnailProgress.classList.add("hidden"); - this.sourceSections.results.classList.add("hidden"); - - // Reset thumbnail UI - if (this.inputs.thumbnail) { - this.inputs.thumbnail.disabled = false; - this.inputs.thumbnail.placeholder = "https://example.com/thumbnail.jpg"; - } - if (this.inputs.thumbnailFile) { - this.inputs.thumbnailFile.value = ""; - } + this.resetUploads(); } // --- Modal Control --- @@ -1093,6 +1168,7 @@ export class UploadModal { close() { if (!this.root) return; + this.resetUploads(); this.root.classList.add("hidden"); this.setGlobalModalState("upload", false); this.isVisible = false; diff --git a/tests/ui/uploadModal-reset.test.mjs b/tests/ui/uploadModal-reset.test.mjs new file mode 100644 index 00000000..6f4a2149 --- /dev/null +++ b/tests/ui/uploadModal-reset.test.mjs @@ -0,0 +1,224 @@ +import { test, describe, it, before, after } from "node:test"; +import assert from "node:assert"; +import { createUiDom } from "./helpers/jsdom-test-helpers.mjs"; + +describe("UploadModal Reset Logic", () => { + let dom; + let UploadModal; + + // Mock StorageService + const mockStorageService = { + isUnlocked: () => true, + unlock: async () => {}, + listConnections: async () => [], + getConnection: async () => null, + saveConnection: async () => {}, + isMock: true + }; + + // Mock R2Service + const mockR2Service = { + prepareUpload: async () => ({ + settings: {}, + bucketEntry: { bucket: 'test', publicBaseUrl: 'https://test.com' } + }), + uploadFile: async () => { + // Default immediate resolution + return {}; + } + }; + + before(async () => { + dom = createUiDom(); + global.window = dom.window; + global.document = dom.document; + global.EventTarget = dom.window.EventTarget; + global.alert = (msg) => { console.log("ALERT CALLED:", msg); }; + global.HTMLElement = dom.window.HTMLElement; + global.HTMLInputElement = dom.window.HTMLInputElement; + global.HTMLButtonElement = dom.window.HTMLButtonElement; + global.HTMLDivElement = dom.window.HTMLDivElement; + + const module = await import("../../js/ui/components/UploadModal.js"); + UploadModal = module.UploadModal; + }); + + after(() => { + if (dom) dom.cleanup(); + }); + + const mockHtml = ` + + `; + + it("should reset upload state and inputs when resetUploads is called", async () => { + const container = document.createElement("div"); + container.id = "modalContainer"; + document.body.appendChild(container); + + global.fetch = async () => ({ ok: true, text: async () => mockHtml }); + + const modal = new UploadModal({ + storageService: mockStorageService, + r2Service: mockR2Service, + getCurrentPubkey: () => "pk1", + removeTrackingScripts: () => {}, + setGlobalModalState: () => {}, + }); + + await modal.load({ container }); + + // Simulate modified state + modal.videoUploadState.status = 'complete'; + modal.inputs.file = { value: 'fake-path' }; // Mock input value + modal.inputs.thumbnail.disabled = true; + modal.inputs.thumbnail.placeholder = "Selected: file.jpg"; + + const initialVideoId = modal.videoUploadId; + + modal.resetUploads(); + + // Assertions + assert.strictEqual(modal.videoUploadState.status, 'idle', "State should be idle"); + assert.strictEqual(modal.inputs.file.value, "", "File input should be cleared"); + assert.strictEqual(modal.inputs.thumbnail.disabled, false, "Thumbnail input should be enabled"); + assert.strictEqual(modal.inputs.thumbnail.placeholder, "https://example.com/thumbnail.jpg", "Thumbnail placeholder should be reset"); + assert.strictEqual(modal.videoUploadId, initialVideoId + 1, "Video upload ID should be incremented"); + }); + + it("should guard against zombie callbacks when upload is reset during process", async () => { + const container = document.createElement("div"); + container.id = "modalContainer2"; + document.body.appendChild(container); + + let resolveUpload; + const delayedUpload = new Promise(r => { resolveUpload = r; }); + + const slowR2Service = { + prepareUpload: async () => ({ + settings: {}, + bucketEntry: { bucket: 'test', publicBaseUrl: 'https://test.com' } + }), + uploadFile: () => delayedUpload // Return promise that waits + }; + + const modal = new UploadModal({ + storageService: mockStorageService, + r2Service: slowR2Service, + getCurrentPubkey: () => "pk1", + removeTrackingScripts: () => {}, + setGlobalModalState: () => {}, + }); + + await modal.load({ container }); + + // Force configured state + modal.storageConfigured = true; + modal.isStorageUnlocked = true; + modal.activeProvider = "cloudflare_r2"; + modal.activeCredentials = { accountId: 'acc', accessKeyId: 'key', secretAccessKey: 'sec' }; + + // Start upload + const file = { name: "video.mp4" }; + const event = { target: { files: [file], value: "video.mp4" } }; + + // Start the async process without awaiting it yet + const uploadPromise = modal.handleVideoSelection(event); + + // Verify it started + assert.strictEqual(modal.videoUploadState.status, 'uploading'); + + // RESET immediately + modal.resetUploads(); + + // Verify it reset + assert.strictEqual(modal.videoUploadState.status, 'idle'); + + // Now let the upload promise resolve + resolveUpload({ bucket: 'b', key: 'k' }); + + // Also need to handle generateTorrentMetadata which might be awaited + // Assuming it resolves quickly or we mock it too. + // UploadModal mocks `generateTorrentMetadata` internally? No, it's a method on instance or imported. + // It calls `this.generateTorrentMetadata`. We can spy/mock it. + modal.generateTorrentMetadata = async () => ({ hasValidInfoHash: false }); + + await uploadPromise; + + // Assert that state remains 'idle' and wasn't overwritten by 'complete' + assert.strictEqual(modal.videoUploadState.status, 'idle', "State should remain idle after zombie completion"); + assert.strictEqual(modal.results.videoUrl.value, "", "Result URL should be empty"); + }); + + it("should call resetUploads when close is called", async () => { + const container = document.createElement("div"); + container.id = "modalContainer3"; + document.body.appendChild(container); + + const modal = new UploadModal({ + storageService: mockStorageService, + r2Service: mockR2Service, + setGlobalModalState: () => {}, + }); + await modal.load({ container }); + + let resetCalled = false; + const originalReset = modal.resetUploads.bind(modal); + modal.resetUploads = () => { + resetCalled = true; + originalReset(); + }; + + modal.close(); + assert.strictEqual(resetCalled, true, "resetUploads should be called on close"); + }); +});