From ed7b043e71c06a3dd74f887b47033e1bc20c86cf Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 3 Sep 2026 19:17:44 -0300 Subject: [PATCH 1/3] feat: create a retry ledger so that on retries it only uploads missing chunks --- .../__tests__/dropzone-resume.test.js | 304 ++++++++++++++++++ .../dropzone/__tests__/upload-ledger.test.js | 129 ++++++++ src/components/inputs/dropzone/index.js | 105 ++++++ .../inputs/dropzone/upload-ledger.js | 111 +++++++ .../__tests__/upload-input-v3.test.js | 56 +++- .../inputs/upload-input-v3/index.js | 30 ++ src/i18n/en.json | 3 +- 7 files changed, 735 insertions(+), 3 deletions(-) create mode 100644 src/components/inputs/dropzone/__tests__/dropzone-resume.test.js create mode 100644 src/components/inputs/dropzone/__tests__/upload-ledger.test.js create mode 100644 src/components/inputs/dropzone/upload-ledger.js diff --git a/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js b/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js new file mode 100644 index 00000000..9909dd87 --- /dev/null +++ b/src/components/inputs/dropzone/__tests__/dropzone-resume.test.js @@ -0,0 +1,304 @@ +/** + * Copyright 2018 OpenStack Foundation + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +import React from 'react'; +import { render, cleanup } from '@testing-library/react'; +import { DropzoneJS } from '../index'; +import { getOrCreateUploadLedger, acknowledgeChunk, isChunkAcknowledged } from '../upload-ledger'; + +jest.mock('../../../security/methods', () => ({ + getAccessToken: jest.fn(() => Promise.resolve('mock-token')), + initLogOut: jest.fn() +})); + +jest.mock('../../../../utils/crypto', () => ({ + getMD5: jest.fn(() => Promise.resolve('mock-md5-hash')) +})); + +// A separate mock module instance from dropzone.test.js's - each test file gets its own +// jest module registry, so extending this shape (vs. the other suite's) carries no risk +// of changing behavior other tests already rely on. +let mockCapturedOptions = {}; +// _originalUploadData ends up as a bound-native-function (not a jest mock, since +// Function.prototype.bind on a jest.fn() drops its .mock tracking) - assertions use +// this captured reference to the pre-bind mock instead. +let mockUploadDataFn; + +jest.mock('dropzone', () => { + return jest.fn().mockImplementation((element, options) => { + mockCapturedOptions = options; + mockUploadDataFn = jest.fn(); + const dz = { + options, + _uploadData: mockUploadDataFn, + _getChunk: jest.fn((file, xhr) => + (file.upload?.chunks || []).find((c) => c && c.xhr === xhr) + ), + uploadFiles: jest.fn(), + on: jest.fn(), + off: jest.fn(), + destroy: jest.fn(() => null), + getActiveFiles: jest.fn(() => []) + }; + dz.emit = jest.fn((event, ...args) => { + dz.on.mock.calls + .filter(([evt]) => evt === event) + .forEach(([, handler]) => handler(...args)); + }); + return dz; + }); +}); + +const getEventHandler = (instance, eventName) => { + const call = instance.dropzone.on.mock.calls + .slice() + .reverse() + .find(([evt]) => evt === eventName); + return call ? call[1] : null; +}; + +describe('DropzoneJS - Resumable Chunked Uploads', () => { + const defaultProps = { + id: 'test-namespace', + config: { postUrl: 'https://example.com/upload' }, + djsConfig: { chunking: true, chunkSize: 1000, maxFilesize: 100 }, + eventHandlers: {}, + data: {}, + uploadCount: 0 + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockCapturedOptions = {}; + window.localStorage.clear(); + }); + + afterEach(() => { + cleanup(); + }); + + const mountInstance = (props = {}) => { + const ref = React.createRef(); + render(); + return ref.current; + }; + + test('accept() reassigns dzuuid to the ledger id and seeds progress from acked chunks', async () => { + const seeded = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + acknowledgeChunk(seeded, 0); + acknowledgeChunk(seeded, 1); + + const instance = mountInstance(); + const file = { name: 'video.mp4', size: 5000, upload: { uuid: 'dropzone-own-random-uuid' } }; + + await mockCapturedOptions.accept(file, jest.fn()); + + expect(file.upload.uuid).toBe(seeded.uploadId); + expect(file._resumeLedger.ackedChunks).toEqual([0, 1]); + expect(file._completedBytes).toBe(2000); + expect(instance.dropzone.emit).toHaveBeenCalledWith('uploadprogress', file, 40, 2000); + }); + + test('a fresh file (no prior ledger) gets a new random id and no progress seed', async () => { + const instance = mountInstance(); + const file = { name: 'video.mp4', size: 5000, upload: { uuid: 'dropzone-own-random-uuid' } }; + + await mockCapturedOptions.accept(file, jest.fn()); + + expect(file.upload.uuid).not.toBe('dropzone-own-random-uuid'); + expect(file._resumeLedger.ackedChunks).toEqual([]); + expect(file._completedBytes).toBeUndefined(); + expect(instance.dropzone.emit).not.toHaveBeenCalledWith('uploadprogress', expect.anything(), expect.anything(), expect.anything()); + }); + + test('a chunk already acknowledged is skipped: never dispatched, never occupies a slot', () => { + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + acknowledgeChunk(ledger, 0); + + const instance = mountInstance(); + const chunk0 = { index: 0 }; + const chunk1 = { index: 1 }; + const file = { + name: 'video.mp4', + size: 5000, + _resumeLedger: ledger, + upload: { chunks: [chunk0, chunk1], finishedChunkUpload: jest.fn() } + }; + instance.chunksInFlight = 3; + + instance.dropzone._uploadData([file], [{ chunkIndex: 0 }]); + + expect(mockUploadDataFn).not.toHaveBeenCalled(); + expect(file.upload.finishedChunkUpload).toHaveBeenCalledWith(chunk0); + expect(file._completedBytes).toBe(1000); + // never took a concurrency slot, so there is none to release + expect(instance.chunksInFlight).toBe(3); + }); + + test('a chunk not yet acknowledged is queued and dispatched for real', () => { + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + acknowledgeChunk(ledger, 0); + + const instance = mountInstance(); + const file = { + name: 'video.mp4', + size: 5000, + _resumeLedger: ledger, + upload: { chunks: [{ index: 0 }, { index: 1 }], finishedChunkUpload: jest.fn() } + }; + + instance.dropzone._uploadData([file], [{ chunkIndex: 1 }]); + + expect(mockUploadDataFn).toHaveBeenCalledWith([file], [{ chunkIndex: 1 }]); + expect(file.upload.finishedChunkUpload).not.toHaveBeenCalled(); + }); + + test('a chunk response with status 0 (connection drop) is not acknowledged', () => { + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + const instance = mountInstance(); + const mockXhr = { + readyState: XMLHttpRequest.DONE, + status: 0, + responseText: '', + setRequestHeader: jest.fn(), + onload: jest.fn(), + onerror: jest.fn(), + abort: jest.fn() + }; + const file = { + name: 'video.mp4', + size: 5000, + _resumeLedger: ledger, + upload: { chunked: true, chunks: [{ index: 0, xhr: mockXhr }] } + }; + + getEventHandler(instance, 'sending')(file, mockXhr, { append: jest.fn() }); + mockXhr.onload({}); + + expect(isChunkAcknowledged(ledger, 0)).toBe(false); + }); + + test.each([200, 202])('a chunk response with status %i is acknowledged', (status) => { + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + const instance = mountInstance(); + const mockXhr = { + readyState: XMLHttpRequest.DONE, + status, + responseText: JSON.stringify({ done: 40, status: true }), + setRequestHeader: jest.fn(), + onload: jest.fn(), + onerror: jest.fn(), + abort: jest.fn() + }; + const file = { + name: 'video.mp4', + size: 5000, + _resumeLedger: ledger, + upload: { chunked: true, chunks: [{ index: 2, xhr: mockXhr }] } + }; + + getEventHandler(instance, 'sending')(file, mockXhr, { append: jest.fn() }); + mockXhr.onload({}); + + expect(isChunkAcknowledged(ledger, 2)).toBe(true); + }); + + test('an over-claiming ledger self-corrects once under the same id, then gives up cleanly with no error', () => { + const instance = mountInstance(); + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + const originalId = ledger.uploadId; + const file = { + name: 'video.mp4', + size: 5000, + md5: 'mock-md5-hash', + _resumeLedger: ledger, + _resumeSkippedThisAttempt: true, + _asyncProcessing: false, + upload: { uuid: originalId } + }; + const done = jest.fn(); + + // First occurrence: same id, ackedChunks cleared, one clean re-drive. + mockCapturedOptions.chunksUploaded(file, done); + + expect(instance.dropzone.uploadFiles).toHaveBeenCalledWith([file]); + expect(file.upload.uuid).toBe(originalId); + expect(file._resumeLedger.correctionAttempted).toBe(true); + expect(file._resumeLedger.ackedChunks).toEqual([]); + expect(file._completedBytes).toBe(0); + expect(done).not.toHaveBeenCalled(); + expect(instance.dropzone.emit).not.toHaveBeenCalledWith('error', expect.anything(), expect.anything()); + + // Second occurrence: the corrected pass ALSO came up short - discard the id, + // start a clean full upload under a fresh one, still no error surfaced. + file._resumeSkippedThisAttempt = true; + instance.dropzone.uploadFiles.mockClear(); + + mockCapturedOptions.chunksUploaded(file, done); + + expect(instance.dropzone.uploadFiles).toHaveBeenCalledWith([file]); + expect(file.upload.uuid).not.toBe(originalId); + expect(file._resumeLedger.ackedChunks).toEqual([]); + expect(done).not.toHaveBeenCalled(); + expect(instance.dropzone.emit).not.toHaveBeenCalledWith('error', expect.anything(), expect.anything()); + }); + + test('a genuinely completed resume (async 202) never enters the correction path', () => { + const instance = mountInstance(); + const ledger = getOrCreateUploadLedger('test-namespace', 'mock-md5-hash', 5000, 1000, 5); + const file = { + name: 'video.mp4', + size: 5000, + _resumeLedger: ledger, + _resumeSkippedThisAttempt: true, + _asyncProcessing: true, // the last real chunk got a 202 + upload: { uuid: ledger.uploadId } + }; + const done = jest.fn(); + + mockCapturedOptions.chunksUploaded(file, done); + + expect(instance.dropzone.uploadFiles).not.toHaveBeenCalled(); + expect(file._chunksUploadedDone).toBe(done); + expect(done).not.toHaveBeenCalled(); + }); + + test('a chunkSize change starts a fresh upload id instead of reusing the old one', async () => { + const instance = mountInstance(); + const file = { name: 'video.mp4', size: 5000, upload: {} }; + + await mockCapturedOptions.accept(file, jest.fn()); + const firstId = file.upload.uuid; + + instance.dropzone.options.chunkSize = 2000; + const file2 = { name: 'video.mp4', size: 5000, upload: {} }; + await mockCapturedOptions.accept(file2, jest.fn()); + + expect(file2.upload.uuid).not.toBe(firstId); + }); + + test('a retried File keeps its md5 across accept() calls, so the same ledger is found', async () => { + const instance = mountInstance(); + const file = { name: 'video.mp4', size: 5000, upload: {} }; + + await mockCapturedOptions.accept(file, jest.fn()); + const firstId = file.upload.uuid; + + // Simulate removeFile + addFile reusing the same object: dropzone's native addFile + // resets file.upload, but never touches file.md5. + file.upload = { uuid: 'brand-new-native-random-uuid' }; + await mockCapturedOptions.accept(file, jest.fn()); + + expect(file.upload.uuid).toBe(firstId); + }); +}); diff --git a/src/components/inputs/dropzone/__tests__/upload-ledger.test.js b/src/components/inputs/dropzone/__tests__/upload-ledger.test.js new file mode 100644 index 00000000..c7943e7e --- /dev/null +++ b/src/components/inputs/dropzone/__tests__/upload-ledger.test.js @@ -0,0 +1,129 @@ +/** + * Copyright 2018 OpenStack Foundation + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +import { + getOrCreateUploadLedger, + acknowledgeChunk, + isChunkAcknowledged, + markCorrectionAttempted, + clearLedger, +} from '../upload-ledger'; + +describe('upload-ledger', () => { + beforeEach(() => { + window.localStorage.clear(); + }); + + test('creates a fresh random id with an empty ledger on first use', () => { + const ledger = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + + expect(typeof ledger.uploadId).toBe('string'); + expect(ledger.uploadId).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/); + expect(ledger.ackedChunks).toEqual([]); + expect(ledger.correctionAttempted).toBe(false); + }); + + test('a second call for the same file returns the same id and preserves acked chunks', () => { + const first = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(first, 0); + acknowledgeChunk(first, 1); + + const second = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + + expect(second.uploadId).toBe(first.uploadId); + expect(second.ackedChunks).toEqual([0, 1]); + }); + + test('acknowledging the same index twice does not duplicate it', () => { + const ledger = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(ledger, 3); + acknowledgeChunk(ledger, 3); + + expect(ledger.ackedChunks).toEqual([3]); + expect(isChunkAcknowledged(ledger, 3)).toBe(true); + expect(isChunkAcknowledged(ledger, 4)).toBe(false); + }); + + test('isChunkAcknowledged is false for a null ledger', () => { + expect(isChunkAcknowledged(null, 0)).toBe(false); + }); + + test('a ledger older than the TTL is discarded for a fresh one', () => { + const nowSpy = jest.spyOn(Date, 'now').mockReturnValue(1000); + const first = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10, 5000); + acknowledgeChunk(first, 0); + + nowSpy.mockReturnValue(1000 + 5001); + const second = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10, 5000); + + expect(second.uploadId).not.toBe(first.uploadId); + expect(second.ackedChunks).toEqual([]); + + nowSpy.mockRestore(); + }); + + test('a chunkSize change discards the ledger for a fresh one', () => { + const first = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(first, 0); + + const second = getOrCreateUploadLedger('ns', 'md5-a', 1000, 200, 5); + + expect(second.uploadId).not.toBe(first.uploadId); + expect(second.ackedChunks).toEqual([]); + }); + + test('a totalChunks change discards the ledger for a fresh one', () => { + const first = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(first, 0); + + const second = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 11); + + expect(second.uploadId).not.toBe(first.uploadId); + }); + + test('different namespaces for the same file do not collide', () => { + const a = getOrCreateUploadLedger('slot-a', 'md5-a', 1000, 100, 10); + const b = getOrCreateUploadLedger('slot-b', 'md5-a', 1000, 100, 10); + + expect(a.uploadId).not.toBe(b.uploadId); + }); + + test('markCorrectionAttempted clears ackedChunks and keeps the same id', () => { + const ledger = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(ledger, 0); + acknowledgeChunk(ledger, 1); + const uploadId = ledger.uploadId; + + const corrected = markCorrectionAttempted(ledger); + + expect(corrected.uploadId).toBe(uploadId); + expect(corrected.ackedChunks).toEqual([]); + expect(corrected.correctionAttempted).toBe(true); + + // and it's actually persisted, not just mutated in memory + const reread = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + expect(reread.correctionAttempted).toBe(true); + expect(reread.ackedChunks).toEqual([]); + }); + + test('clearLedger removes the entry so the next call creates a brand new one', () => { + const first = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + acknowledgeChunk(first, 0); + + clearLedger('ns', 'md5-a', 1000); + + const second = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + expect(second.uploadId).not.toBe(first.uploadId); + expect(second.ackedChunks).toEqual([]); + }); +}); diff --git a/src/components/inputs/dropzone/index.js b/src/components/inputs/dropzone/index.js index b6cb2f19..669c69cd 100644 --- a/src/components/inputs/dropzone/index.js +++ b/src/components/inputs/dropzone/index.js @@ -6,6 +6,14 @@ import PropTypes from 'prop-types'; import {getAccessToken, initLogOut} from '../../security/methods'; import {AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR} from '../../security/constants'; import {getMD5} from "../../../utils/crypto"; +import { + getOrCreateUploadLedger, + acknowledgeChunk, + isChunkAcknowledged, + markCorrectionAttempted, + clearLedger, + UPLOAD_LEDGER_TTL_MS, +} from "./upload-ledger"; let Dropzone = null; /** @@ -80,6 +88,14 @@ export class DropzoneJS extends React.Component { this.dropzone._uploadData = (files, dataBlocks) => { // Only throttle chunked uploads (single dataBlock with chunkIndex) if (dataBlocks.length === 1 && dataBlocks[0].chunkIndex !== undefined) { + const [file] = files; + const chunkIndex = dataBlocks[0].chunkIndex; + // A resumed chunk the server already acked is resolved here, before it + // ever takes a concurrency slot - never queued, never sent over the wire. + if (isChunkAcknowledged(file._resumeLedger, chunkIndex)) { + this.skipAcknowledgedChunk(file, chunkIndex); + return; + } this.chunkQueue.push({ files, dataBlocks }); this.processChunkQueue(); } else { @@ -89,11 +105,28 @@ export class DropzoneJS extends React.Component { }; } + // Synthesizes a successful chunk without a network round-trip, using Dropzone's own + // finishedChunkUpload to drive its native chunk state machine (next chunk / chunksUploaded) + // exactly as a real success would. + skipAcknowledgedChunk(file, chunkIndex) { + const chunk = file.upload?.chunks?.[chunkIndex]; + if (!chunk) return; + file._resumeSkippedThisAttempt = true; + const chunkSize = this.dropzone?.options?.chunkSize || 2000000; + file._completedBytes = Math.min((file._completedBytes || 0) + chunkSize, file.size); + file.upload.finishedChunkUpload(chunk); + } + onChunkComplete() { this.chunksInFlight = Math.max(0, this.chunksInFlight - 1); this.processChunkQueue(); } + clearResumeLedger(file) { + clearLedger(this.props.id, file.md5, file.size); + file._resumeLedger = null; + } + pollUploadStatus(fileId, baseUrl, file) { // Guard against multiple polling intervals for the same file if (file._pollingActive) { @@ -139,6 +172,7 @@ export class DropzoneJS extends React.Component { } if (data.status === 'complete') { this.stopPolling(file); + if (file._resumeLedger) this.clearResumeLedger(file); // Call the stored done callback to trigger Dropzone's success event if (file?._chunksUploadedDone) { file._chunksUploadedDone(); @@ -202,11 +236,67 @@ export class DropzoneJS extends React.Component { return; } + // A retried File (removeFile + addFile) keeps whatever these were set to on its + // previous failed pass - addFile/removeFile never clear custom properties. + file._asyncProcessing = false; + file._chunksUploadedDone = null; + file._resumeSkippedThisAttempt = false; + + if (options.chunking) { + const chunkSize = options.chunkSize || 2000000; + const totalChunks = Math.ceil(file.size / chunkSize) || 1; + const ledger = getOrCreateUploadLedger( + this.props.id, file.md5, file.size, chunkSize, totalChunks, + this.props.resumeLedgerTtlMs || UPLOAD_LEDGER_TTL_MS + ); + // Overwrites Dropzone's own randomly-generated dzuuid (already set by + // addFile() before accept() ever runs) with our persisted, stable one - + // the only thing that lets a retry reuse the server's in-progress upload. + file.upload.uuid = ledger.uploadId; + file._resumeLedger = ledger; + + if (ledger.ackedChunks.length > 0) { + const acknowledgedBytes = Math.min(ledger.ackedChunks.length * chunkSize, file.size); + file._completedBytes = acknowledgedBytes; + // Reuses the existing uploadprogress bridge so a resumed row shows its + // real starting percentage immediately, with no new event/plumbing. + if (typeof this.dropzone.emit === 'function') { + this.dropzone.emit( + 'uploadprogress', file, (acknowledgedBytes / file.size) * 100, acknowledgedBytes + ); + } + } + } + done(); }; // Override chunksUploaded to defer success event for async processing (HTTP 202) options.chunksUploaded = (file, done) => { + // Every local chunk succeeded (real or resume-skipped) but the file never got + // a real 202 - proof a skipped chunk wasn't actually held by the server. The + // ledger's belief was wrong, not just incomplete: self-correct, bounded to one + // retry under the same id, then one clean full upload under a fresh id - never + // loop, and never surface this to the user, since a clean pass can't skip + // anything and so can't re-trigger this branch. + if (file._resumeLedger && file._resumeSkippedThisAttempt && !file._asyncProcessing) { + file._resumeSkippedThisAttempt = false; + file._completedBytes = 0; + if (!file._resumeLedger.correctionAttempted) { + file._resumeLedger = markCorrectionAttempted(file._resumeLedger); + } else { + const chunkSize = file._resumeLedger.chunkSize; + const totalChunks = file._resumeLedger.totalChunks; + this.clearResumeLedger(file); + file._resumeLedger = getOrCreateUploadLedger( + this.props.id, file.md5, file.size, chunkSize, totalChunks, + this.props.resumeLedgerTtlMs || UPLOAD_LEDGER_TTL_MS + ); + file.upload.uuid = file._resumeLedger.uploadId; + } + this.dropzone.uploadFiles([file]); + return; + } if (file._asyncProcessing) { // Store the done callback for later execution after polling completes file._chunksUploadedDone = done; @@ -456,6 +546,13 @@ export class DropzoneJS extends React.Component { // load callback from dropzone let dropzoneOnLoad = xhr.onload; xhr.onload = function (e) { + // Resolved BEFORE dropzoneOnLoad: on a successful chunked response, Dropzone's + // native finishedChunkUpload (called from within dropzoneOnLoad) nulls + // chunk.xhr, so _getChunk can no longer find it afterwards. + const chunk = (file?.upload?.chunked && _this.dropzone?._getChunk) + ? _this.dropzone._getChunk(file, xhr) + : null; + // Remove this XHR from active tracking const xhrs = _this.activeXHRs.get(file); if (xhrs) { @@ -481,6 +578,13 @@ export class DropzoneJS extends React.Component { file._asyncProcessing = true; } + // Acknowledge only on a real 200/202 - never on a connection failure (status 0, + // xhr.onerror/ontimeout below), so an unacknowledged chunk always gets re-sent + // on the next resume, which is safe because the server dedups a held chunk. + if (chunk && (xhr?.status === 200 || xhr?.status === 202) && file._resumeLedger) { + acknowledgeChunk(file._resumeLedger, chunk.index); + } + dropzoneOnLoad(e); // The user may have cancelled while this response was in flight: abort() on an @@ -492,6 +596,7 @@ export class DropzoneJS extends React.Component { if(xhr?.status == 200) { if (typeof uploadResponse.name === 'string') { + if (file._resumeLedger) _this.clearResumeLedger(file); _this.onUploadComplete(uploadResponse); } } diff --git a/src/components/inputs/dropzone/upload-ledger.js b/src/components/inputs/dropzone/upload-ledger.js new file mode 100644 index 00000000..55e57982 --- /dev/null +++ b/src/components/inputs/dropzone/upload-ledger.js @@ -0,0 +1,111 @@ +/** + * Copyright 2018 OpenStack Foundation + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + **/ + +import { putOnLocalStorage, getFromLocalStorage, removeFromLocalStorage } from '../../../utils/methods'; + +const LEDGER_KEY_PREFIX = 'dz_resume_ledger_v1'; + +// Must stay shorter than file-upload-api's ABANDONED_UPLOAD_RETENTION_SECONDS +// (1hr default) so the client never resumes against chunks the server's reaper +// already deleted. +export const UPLOAD_LEDGER_TTL_MS = 30 * 60 * 1000; + +// Mirrors Dropzone.uuidv4() byte-for-byte without importing the 'dropzone' package +// here - a plain uuid4 is what file-upload-api's sanitize_upload_id (Django slugify) +// passes through unchanged. +const uuidv4 = () => + 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => { + const r = (Math.random() * 16) | 0; + const v = c === 'x' ? r : (r & 0x3) | 0x8; + return v.toString(16); + }); + +const buildKey = (namespace, fileMd5, fileSize) => + `${LEDGER_KEY_PREFIX}:${namespace || 'default'}:${fileMd5}:${fileSize}`; + +const readLedger = (key) => { + const raw = getFromLocalStorage(key); + if (!raw) return null; + try { + const parsed = JSON.parse(raw); + return parsed && typeof parsed === 'object' ? parsed : null; + } catch (e) { + return null; + } +}; + +const writeLedger = (ledger) => { + const { _key, ...persisted } = ledger; + putOnLocalStorage(_key, JSON.stringify(persisted)); +}; + +/** + * Loads the persisted ledger for a physical file (identified by content, not name, + * so a retry resumes even if the file is re-selected under a different name) within + * a caller-supplied namespace, or creates a fresh one. A stored ledger is discarded + * (fresh random id, empty ackedChunks) when it is older than UPLOAD_LEDGER_TTL_MS, or + * when chunkSize/totalChunks no longer match - reusing an id under a different chunk + * size is what triggers the server's `dztotalchunkcount changed` 412. + */ +export const getOrCreateUploadLedger = (namespace, fileMd5, fileSize, chunkSize, totalChunks, ttlMs = UPLOAD_LEDGER_TTL_MS) => { + const key = buildKey(namespace, fileMd5, fileSize); + const existing = readLedger(key); + const now = Date.now(); + + const isFresh = + !!existing && + now - existing.createdAt < ttlMs && + existing.chunkSize === chunkSize && + existing.totalChunks === totalChunks; + + if (isFresh) return { ...existing, _key: key }; + + const ledger = { + uploadId: uuidv4(), + chunkSize, + totalChunks, + ackedChunks: [], + createdAt: now, + correctionAttempted: false, + _key: key, + }; + writeLedger(ledger); + return ledger; +}; + +export const acknowledgeChunk = (ledger, chunkIndex) => { + if (!ledger || ledger.ackedChunks.includes(chunkIndex)) return ledger; + ledger.ackedChunks.push(chunkIndex); + writeLedger(ledger); + return ledger; +}; + +export const isChunkAcknowledged = (ledger, chunkIndex) => + !!ledger && ledger.ackedChunks.includes(chunkIndex); + +/** + * One bounded self-correction pass: the client believed every acked index was held + * by the server, but it wasn't. Clears that belief without discarding the upload id, + * so the next pass re-sends exactly the previously-skipped chunks under the same id. + */ +export const markCorrectionAttempted = (ledger) => { + if (!ledger) return ledger; + ledger.correctionAttempted = true; + ledger.ackedChunks = []; + writeLedger(ledger); + return ledger; +}; + +export const clearLedger = (namespace, fileMd5, fileSize) => { + removeFromLocalStorage(buildKey(namespace, fileMd5, fileSize)); +}; diff --git a/src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js b/src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js index 05973137..11939a76 100644 --- a/src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js +++ b/src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js @@ -298,8 +298,10 @@ describe('UploadInputV3', () => { expect(screen.getByText(/File is too big/)).toBeInTheDocument(); expect(container.querySelector('.dropzone-mock')).not.toBeVisible(); + // The error row has two buttons (Retry, then dismiss) - dismiss is last. + const errorRowButtons = screen.getAllByRole('button'); act(() => { - fireEvent.click(screen.getByRole('button')); + fireEvent.click(errorRowButtons[errorRowButtons.length - 1]); }); expect(screen.queryByText(/File is too big/)).not.toBeInTheDocument(); @@ -363,6 +365,54 @@ describe('UploadInputV3', () => { }); expect(container.querySelector('.dropzone-mock')).not.toBeVisible(); }); + + test('retrying an error resumes: removeFile + addFile on the same dropzone file object, flags reset', () => { + const removeFile = jest.fn(); + const addFile = jest.fn(); + // still carrying flags from its previous pass, same as after a real interrupted upload + const dzFile = { name: 'video.mp4', size: 9999999, accepted: true, _canceled: true }; + + render(); + act(() => { + dropzoneCallbacks.onDropzoneReady({ files: [dzFile], removeFile, addFile }); + }); + act(() => { + dropzoneCallbacks.onFileError({ name: 'video.mp4', size: 9999999 }, 'Network error'); + }); + + act(() => { + fireEvent.click(screen.getByTitle('Retry')); + }); + + expect(removeFile).toHaveBeenCalledWith(dzFile); + expect(addFile).toHaveBeenCalledWith(dzFile); + // dropzone's own accept() gate and our xhr.onload guard both key off these - + // stale values from the failed attempt must not survive into the retry. + expect(dzFile.accepted).toBe(false); + expect(dzFile._canceled).toBe(false); + expect(screen.queryByText('video.mp4')).not.toBeInTheDocument(); + }); + + test('retrying an error is a no-op when the underlying dropzone file is gone', () => { + const removeFile = jest.fn(); + const addFile = jest.fn(); + + render(); + act(() => { + dropzoneCallbacks.onDropzoneReady({ files: [], removeFile, addFile }); + }); + act(() => { + dropzoneCallbacks.onFileError({ name: 'video.mp4', size: 9999999 }, 'Network error'); + }); + + act(() => { + fireEvent.click(screen.getByTitle('Retry')); + }); + + expect(removeFile).not.toHaveBeenCalled(); + expect(addFile).not.toHaveBeenCalled(); + expect(screen.queryByText('video.mp4')).not.toBeInTheDocument(); + }); }); describe('Configuration', () => { @@ -590,7 +640,9 @@ describe('UploadInputV3', () => { }); expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:photo-a.jpg'); - act(() => { fireEvent.click(screen.getByRole('button')); }); + // The error row now has two buttons (Retry, then dismiss) - the dismiss one is last. + const errorRowButtons = screen.getAllByRole('button'); + act(() => { fireEvent.click(errorRowButtons[errorRowButtons.length - 1]); }); act(() => { dropzoneCallbacks.onAddedFile({ name: 'photo-b.jpg', size: 20000, type: 'image/jpeg' }); dropzoneCallbacks.onFileCompleted({ name: 'photo-b.jpg', size: 20000 }); diff --git a/src/components/inputs/upload-input-v3/index.js b/src/components/inputs/upload-input-v3/index.js index e0d36ef2..410ac3e6 100644 --- a/src/components/inputs/upload-input-v3/index.js +++ b/src/components/inputs/upload-input-v3/index.js @@ -25,6 +25,7 @@ import DeleteIcon from "@mui/icons-material/Delete"; import CheckCircleIcon from "@mui/icons-material/CheckCircle"; import ErrorOutlineIcon from "@mui/icons-material/ErrorOutline"; import CloseIcon from "@mui/icons-material/Close"; +import ReplayIcon from "@mui/icons-material/Replay"; import { DropzoneV3 } from './dropzone-v3'; import ProgressiveImg from '../../progressive-img'; import file_icon from '../upload-input/file.png'; @@ -249,6 +250,27 @@ const UploadInputV3 = ({ setErrorFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size))); }, []); + // An errored file is never auto-removed from dropzone's own file list, so the same + // (name,size) lookup handleDismissError uses still finds it here - re-adding it + // re-triggers accept(), which is where the resume ledger lookup happens. + const handleRetryError = useCallback((file) => { + const dz = dropzoneInstanceRef.current; + const dzFile = dz?.files?.find(f => f.name === file.name && f.size === file.size); + setErrorFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size))); + if (!dz || !dzFile) return; // file object gone (e.g. full remount) - re-selecting the + // same file still resumes via the md5-keyed ledger the next time it hits accept() + dz.removeFile(dzFile); + // Reusing the same File object means it still carries flags from its previous pass. + // dropzone's OWN accept() gate runs before our custom options.accept ever does, and + // rejects the file as "too many files" if accepted stays true from its original + // successful accept. Our removedfile handler also sets _canceled, which later blocks + // xhr.onload from ever calling onUploadComplete/pollUploadStatus once resent chunks + // succeed. Neither is cleared by removeFile/addFile - must reset both here. + dzFile.accepted = false; + dzFile._canceled = false; + dz.addFile(dzFile); + }, []); + const handleDeleteUploading = useCallback((file) => { const entry = uploadingFilesRef.current.find(f => f.name === file.name && f.size === file.size); if (entry?.previewUrl) URL.revokeObjectURL(entry.previewUrl); @@ -452,6 +474,14 @@ const UploadInputV3 = ({ + handleRetryError(file)} + title={T.translate("upload_input_v3.retry")} + sx={{ color: 'primary.main' }} + > + + handleDismissError(file)} diff --git a/src/i18n/en.json b/src/i18n/en.json index 7eb14cba..ef3b4480 100644 --- a/src/i18n/en.json +++ b/src/i18n/en.json @@ -193,7 +193,8 @@ "upload_failed": "Upload failed. Please retry.", "auth_error": "Authentication error. Please sign in again.", "max_files_reached": "Maximum number of files reached.", - "loading": "Uploading" + "loading": "Uploading", + "retry": "Retry" }, "grid_filter": { "filter_by": "Filter by ", From 69de42092ae3c3acd993c1f3ab978edb72d506db Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Thu, 3 Sep 2026 19:18:29 -0300 Subject: [PATCH 2/3] v5.0.58-beta.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1e862b46..b4538fe4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openstack-uicore-foundation", - "version": "5.0.58-beta.0", + "version": "5.0.58-beta.1", "description": "ui reactjs components for openstack marketing site", "main": "lib/openstack-uicore-foundation.js", "scripts": { From 064d8e315bc466921bcf7061d88c84ae28bbcd4e Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Fri, 4 Sep 2026 09:45:22 -0300 Subject: [PATCH 3/3] fix: never let a localStorage failure stall a resumable upload getOrCreateUploadLedger was called outside options.accept's try/catch, so a localStorage error (blocked/full storage) would leave the file stuck without ever surfacing an error. readLedger/writeLedger/clearLedger now swallow storage errors and fall back to an in-memory ledger. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GSiT8RW5vTG8gzP9zEnJxt --- .../dropzone/__tests__/upload-ledger.test.js | 43 +++++++++++++++++++ .../inputs/dropzone/upload-ledger.js | 20 ++++++--- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/src/components/inputs/dropzone/__tests__/upload-ledger.test.js b/src/components/inputs/dropzone/__tests__/upload-ledger.test.js index c7943e7e..e189b43e 100644 --- a/src/components/inputs/dropzone/__tests__/upload-ledger.test.js +++ b/src/components/inputs/dropzone/__tests__/upload-ledger.test.js @@ -126,4 +126,47 @@ describe('upload-ledger', () => { expect(second.uploadId).not.toBe(first.uploadId); expect(second.ackedChunks).toEqual([]); }); + + describe('when localStorage throws (blocked/full storage)', () => { + let getItemSpy; + let setItemSpy; + let removeItemSpy; + + beforeEach(() => { + getItemSpy = jest.spyOn(window.localStorage.__proto__, 'getItem').mockImplementation(() => { + throw new Error('storage blocked'); + }); + setItemSpy = jest.spyOn(window.localStorage.__proto__, 'setItem').mockImplementation(() => { + throw new Error('storage blocked'); + }); + removeItemSpy = jest.spyOn(window.localStorage.__proto__, 'removeItem').mockImplementation(() => { + throw new Error('storage blocked'); + }); + }); + + afterEach(() => { + getItemSpy.mockRestore(); + setItemSpy.mockRestore(); + removeItemSpy.mockRestore(); + }); + + test('getOrCreateUploadLedger still returns a fresh in-memory ledger instead of throwing', () => { + expect(() => getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10)).not.toThrow(); + + const ledger = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + expect(typeof ledger.uploadId).toBe('string'); + expect(ledger.ackedChunks).toEqual([]); + }); + + test('acknowledgeChunk does not throw when the underlying write fails', () => { + const ledger = getOrCreateUploadLedger('ns', 'md5-a', 1000, 100, 10); + + expect(() => acknowledgeChunk(ledger, 0)).not.toThrow(); + expect(ledger.ackedChunks).toEqual([0]); + }); + + test('clearLedger does not throw when the underlying removal fails', () => { + expect(() => clearLedger('ns', 'md5-a', 1000)).not.toThrow(); + }); + }); }); diff --git a/src/components/inputs/dropzone/upload-ledger.js b/src/components/inputs/dropzone/upload-ledger.js index 55e57982..7a252306 100644 --- a/src/components/inputs/dropzone/upload-ledger.js +++ b/src/components/inputs/dropzone/upload-ledger.js @@ -34,9 +34,9 @@ const buildKey = (namespace, fileMd5, fileSize) => `${LEDGER_KEY_PREFIX}:${namespace || 'default'}:${fileMd5}:${fileSize}`; const readLedger = (key) => { - const raw = getFromLocalStorage(key); - if (!raw) return null; try { + const raw = getFromLocalStorage(key); + if (!raw) return null; const parsed = JSON.parse(raw); return parsed && typeof parsed === 'object' ? parsed : null; } catch (e) { @@ -44,9 +44,15 @@ const readLedger = (key) => { } }; +// Best-effort: a write failure (storage blocked/full) must not break the upload itself - +// the caller already has the ledger object in memory and keeps using it for this attempt. const writeLedger = (ledger) => { - const { _key, ...persisted } = ledger; - putOnLocalStorage(_key, JSON.stringify(persisted)); + try { + const { _key, ...persisted } = ledger; + putOnLocalStorage(_key, JSON.stringify(persisted)); + } catch (e) { + // no-op - persistence is opportunistic + } }; /** @@ -107,5 +113,9 @@ export const markCorrectionAttempted = (ledger) => { }; export const clearLedger = (namespace, fileMd5, fileSize) => { - removeFromLocalStorage(buildKey(namespace, fileMd5, fileSize)); + try { + removeFromLocalStorage(buildKey(namespace, fileMd5, fileSize)); + } catch (e) { + // no-op - a failed clear is harmless, the ledger just expires via its TTL + } };