fix: cancelling an upload must not commit its result

package.json

@@ -1,6 +1,6 @@
 {
     "name": "openstack-uicore-foundation",
-    "version": "5.0.50",
+    "version": "5.0.52-beta.0",
     "description": "ui reactjs components for openstack marketing site",
     "main": "lib/openstack-uicore-foundation.js",
     "scripts": {

src/components/inputs/dropzone/__tests__/dropzone.test.js

@@ -285,6 +285,236 @@ describe('DropzoneJS - HTTP 202 Polling UX', () => {
       }, 2500);
     }, 10);
   }, 10000);
+  /**
+   * Resolves a handler DropzoneJS registered on the underlying Dropzone instance,
+   * so a test can drive the real wiring instead of poking at internals.
+   */
+  const getEventHandler = (instance, eventName) => {
+    const call = instance.dropzone.on.mock.calls
+      .slice()
+      .reverse()
+      .find(([evt]) => evt === eventName);
+    return call ? call[1] : null;
+  };
+
+  /**
+   * Test Case 6: cancelling a file stops ITS status polling - and only its own
+   *
+   * While the server processes an upload asynchronously (HTTP 202) the row sits in
+   * "Loading" with a delete button. Removing the file there used to leave the status
+   * interval running, so when processing finished the result was still pushed to the
+   * parent and the "cancelled" file ended up committed to the form.
+   */
+  test('test_dropzone_cancel_stops_polling_for_that_file_only', (done) => {
+    // Server is still processing: every poll answers 'uploading', so polling keeps going.
+    global.fetch = jest.fn(() =>
+      Promise.resolve({ json: () => Promise.resolve({ status: 'uploading' }) })
+    );
+
+    const ref = React.createRef();
+
+    render(
+      <DropzoneJS
+        {...defaultProps}
+        ref={ref}
+        onUploadComplete={onUploadCompleteMock}
+        onError={onErrorMock}
+      />
+    );
+
+    setTimeout(() => {
+      const instance = ref.current;
+      const fileA = { name: 'a.png', size: 1024 };
+      const fileB = { name: 'b.png', size: 2048 };
+
+      instance.pollUploadStatus('file-a', 'https://example.com/upload', fileA);
+      instance.pollUploadStatus('file-b', 'https://example.com/upload', fileB);
+
+      setTimeout(() => {
+        expect(global.fetch).toHaveBeenCalled();
+
+        const onRemovedFile = getEventHandler(instance, 'removedfile');
+        expect(typeof onRemovedFile).toBe('function');
+        onRemovedFile(fileA);
+
+        const callsAtCancel = global.fetch.mock.calls.length;
+
+        setTimeout(() => {
+          const urlsAfterCancel = global.fetch.mock.calls
+            .slice(callsAtCancel)
+            .map(([url]) => url);
+
+          // fileB is untouched and keeps polling on its own interval...
+          expect(urlsAfterCancel.length).toBeGreaterThan(0);
+          // ...while the cancelled file never asks for its status again.
+          expect(urlsAfterCancel.some((url) => url.endsWith('/status/file-a'))).toBe(false);
+
+          done();
+        }, 2500);
+      }, 2500);
+    }, 10);
+  }, 15000);
+
+  /**
+   * Test Case 7: a status response that lands AFTER the cancel must not be committed
+   *
+   * Clearing the interval is not enough on its own: the tick that was already awaiting
+   * its status request resolves after the user cancelled, and used to fire the deferred
+   * success callback plus onUploadComplete.
+   */
+  test('test_dropzone_cancel_while_status_request_in_flight_does_not_commit_result', (done) => {
+    let respondComplete = null;
+    global.fetch = jest.fn(
+      () =>
+        new Promise((resolve) => {
+          respondComplete = () =>
+            resolve({
+              json: () =>
+                Promise.resolve({ status: 'complete', name: 'test.pdf', size: 1024000 })
+            });
+        })
+    );
+
+    const ref = React.createRef();
+
+    render(
+      <DropzoneJS
+        {...defaultProps}
+        ref={ref}
+        onUploadComplete={onUploadCompleteMock}
+        onError={onErrorMock}
+      />
+    );
+
+    setTimeout(() => {
+      const instance = ref.current;
+      const chunksUploadedDone = jest.fn();
+      const mockFile = {
+        name: 'test.pdf',
+        size: 1024000,
+        _asyncProcessing: true,
+        _chunksUploadedDone: chunksUploadedDone
+      };
+
+      instance.pollUploadStatus('file-123', 'https://example.com/upload', mockFile);
+
+      setTimeout(() => {
+        // The first tick fired and is parked on the status request.
+        expect(typeof respondComplete).toBe('function');
+
+        // The user cancels while that request is still in flight...
+        getEventHandler(instance, 'removedfile')(mockFile);
+        // ...and only then does the server report the upload as processed.
+        respondComplete();
+
+        setTimeout(() => {
+          expect(chunksUploadedDone).not.toHaveBeenCalled();
+          expect(onUploadCompleteMock).not.toHaveBeenCalled();
+          done();
+        }, 100);
+      }, 2500);
+    }, 10);
+  }, 15000);
+
+  /**
+   * Test Case 8: unmounting stops polling for EVERY file in flight
+   *
+   * The interval id used to live in a single component-level slot, so a second file
+   * starting to poll orphaned the first one's interval and it outlived the component.
+   */
+  test('test_dropzone_unmount_stops_polling_for_every_file', (done) => {
+    global.fetch = jest.fn(() =>
+      Promise.resolve({ json: () => Promise.resolve({ status: 'uploading' }) })
+    );
+
+    const ref = React.createRef();
+
+    const { unmount } = render(
+      <DropzoneJS
+        {...defaultProps}
+        ref={ref}
+        onUploadComplete={onUploadCompleteMock}
+        onError={onErrorMock}
+      />
+    );
+
+    setTimeout(() => {
+      const instance = ref.current;
+
+      instance.pollUploadStatus('file-a', 'https://example.com/upload', {
+        name: 'a.png',
+        size: 1024
+      });
+      instance.pollUploadStatus('file-b', 'https://example.com/upload', {
+        name: 'b.png',
+        size: 2048
+      });
+
+      setTimeout(() => {
+        expect(global.fetch).toHaveBeenCalled();
+
+        unmount();
+        const callsAtUnmount = global.fetch.mock.calls.length;
+
+        setTimeout(() => {
+          expect(global.fetch.mock.calls.length).toBe(callsAtUnmount);
+          done();
+        }, 2500);
+      }, 2500);
+    }, 10);
+  }, 15000);
+
+  /**
+   * Test Case 9: an upload response that arrives after the cancel must not be committed
+   *
+   * xhr.abort() on a request already in DONE state is a no-op, so the synchronous (200)
+   * path could still reach onUploadComplete for a file the user had just removed.
+   */
+  test('test_dropzone_upload_response_after_cancel_does_not_commit_result', (done) => {
+    const ref = React.createRef();
+
+    render(
+      <DropzoneJS
+        {...defaultProps}
+        ref={ref}
+        onUploadComplete={onUploadCompleteMock}
+        onError={onErrorMock}
+      />
+    );
+
+    setTimeout(() => {
+      const instance = ref.current;
+      const mockFile = {
+        name: 'test.pdf',
+        size: 1024000,
+        accessToken: 'mock-token',
+        md5: 'mock-md5'
+      };
+      const mockXhr = {
+        readyState: XMLHttpRequest.DONE,
+        status: 200,
+        responseText: JSON.stringify({
+          name: 'test.pdf',
+          path: 'uploads/',
+          size: 1024000
+        }),
+        setRequestHeader: jest.fn(),
+        onload: jest.fn(),
+        onerror: jest.fn(),
+        abort: jest.fn()
+      };
+
+      // 'sending' is what installs the wrapper deciding what to do with the response.
+      getEventHandler(instance, 'sending')(mockFile, mockXhr, { append: jest.fn() });
+
+      // The user cancels; the response for the last chunk is already on its way back.
+      getEventHandler(instance, 'removedfile')(mockFile);
+      mockXhr.onload({});
+
+      expect(onUploadCompleteMock).not.toHaveBeenCalled();
+      done();
+    }, 10);
+  });
 });
 
 describe('DropzoneJS - Progress Bar Monotonicity', () => {

src/components/inputs/dropzone/index.js

@@ -22,6 +22,26 @@ export class DropzoneJS extends React.Component {
         this.activeXHRs = new Map(); // Track active XHR requests per file
         this.chunkQueue = [];
         this.chunksInFlight = 0;
+        // Status-poll interval ids, one per file in flight. Kept as a set (and mirrored on
+        // the file itself) rather than a single slot so a second file starting to poll
+        // cannot orphan the first one's interval.
+        this._pollIntervals = new Set();
+    }
+
+    /**
+     * Stops the status polling started by pollUploadStatus for this file, if any.
+     * Cancelling an upload has to reach the interval too: a file the user removed while the
+     * server was still processing it must stop asking for its status, otherwise the result
+     * lands later and gets committed as if the upload had been kept.
+     */
+    stopPolling(file) {
+        if (!file) return;
+        if (file._pollIntervalId) {
+            clearInterval(file._pollIntervalId);
+            this._pollIntervals.delete(file._pollIntervalId);
+            file._pollIntervalId = null;
+        }
+        file._pollingActive = false;
     }
 
     onError(e, status){
@@ -77,12 +97,15 @@ export class DropzoneJS extends React.Component {
         const maxAttempts = 300; // 10 minutes at 2s intervals
         let attempts = 0;
 
-        this._pollInterval = setInterval(async () => {
+        const intervalId = setInterval(async () => {
+            // The file may have been removed since the last tick.
+            if (file._canceled) {
+                this.stopPolling(file);
+                return;
+            }
             attempts++;
             if (attempts > maxAttempts) {
-                clearInterval(this._pollInterval);
-                this._pollInterval = null;
-                file._pollingActive = false;
+                this.stopPolling(file);
                 this.onError({ message: 'Upload timed out' });
                 return;
             }
@@ -92,28 +115,32 @@ export class DropzoneJS extends React.Component {
                     headers: { 'Authorization': `Bearer ${accessToken}` }
                 });
                 const data = await response.json();
+                // Clearing the interval is not enough on its own: this tick was already
+                // awaiting its response when the user cancelled, and committing it now
+                // would restore a file they removed.
+                if (file._canceled) {
+                    this.stopPolling(file);
+                    return;
+                }
                 if (data.status === 'complete') {
-                    clearInterval(this._pollInterval);
-                    this._pollInterval = null;
-                    file._pollingActive = false;
+                    this.stopPolling(file);
                     // Call the stored done callback to trigger Dropzone's success event
                     if (file?._chunksUploadedDone) {
                         file._chunksUploadedDone();
                     }
                     this.onUploadComplete(data);
                 } else if (data.status === 'error') {
-                    clearInterval(this._pollInterval);
-                    this._pollInterval = null;
-                    file._pollingActive = false;
+                    this.stopPolling(file);
                     this.onError(data);
                 }
             } catch (error) {
-                clearInterval(this._pollInterval);
-                this._pollInterval = null;
-                file._pollingActive = false;
+                this.stopPolling(file);
                 this.onError(error);
             }
         }, 2000);
+
+        file._pollIntervalId = intervalId;
+        this._pollIntervals.add(intervalId);
     }
 
     /**
@@ -204,10 +231,8 @@ export class DropzoneJS extends React.Component {
      * Removes dropzone.js (and all its globals) if the component is being unmounted
      */
     componentWillUnmount () {
-        if (this._pollInterval) {
-            clearInterval(this._pollInterval);
-            this._pollInterval = null;
-        }
+        this._pollIntervals.forEach(intervalId => clearInterval(intervalId));
+        this._pollIntervals.clear();
 
         // Clear chunk queue and cancel all pending XHR requests
         this.chunkQueue = [];
@@ -343,6 +368,14 @@ export class DropzoneJS extends React.Component {
         this.dropzone.on('removedfile', (file) => {
             if (!file) return;
 
+            // Mark the file dead FIRST: both commit points (xhr.onload below and the status
+            // poll) check this flag, so a result that lands after the user cancelled is
+            // dropped instead of being pushed to the parent.
+            file._canceled = true;
+            this.stopPolling(file);
+            // A removed file must not fire Dropzone's deferred success event either.
+            file._chunksUploadedDone = null;
+
             // Cancel all active XHR requests for this file
             const xhrs = this.activeXHRs.get(file);
             if (xhrs) {
@@ -434,6 +467,13 @@ export class DropzoneJS extends React.Component {
 
                 dropzoneOnLoad(e);
 
+                // The user may have cancelled while this response was in flight: abort() on an
+                // already-DONE xhr is a no-op, so without this check the result would still be
+                // committed for a file that is no longer in the list. 'canceled' is the value of
+                // Dropzone.CANCELED, compared as a literal so the guard does not depend on the
+                // Dropzone module being loaded.
+                if (file._canceled || file.status === 'canceled') return;
+
                 if(xhr?.status == 200) {
                     if (typeof uploadResponse.name === 'string') {
                         _this.onUploadComplete(uploadResponse);