fix: safer cleanUpFinishedBatch in batch helpers

protect from batch not found in state when clean is called
rpldy · Sep 21, 2020 · 33bec1e · 33bec1e
1 parent 09c9731
commit 33bec1e
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 15 deletions.
diff --git a/packages/core/uploader/src/queue/batchHelpers.js b/packages/core/uploader/src/queue/batchHelpers.js
@@ -102,7 +102,7 @@ const cleanUpFinishedBatch = (queue: QueueState) => {
     const state = queue.getState();
     const batchId = state.currentBatch;
 
-    if (batchId && isBatchFinished(queue)) {
+    if (batchId && state.batches[batchId] && isBatchFinished(queue)) {
         triggerUploaderBatchEvent(queue, batchId, UPLOADER_EVENTS.BATCH_FINISH);
         removeBatchItems(queue, batchId);
         removeBatch(queue, batchId);

diff --git a/packages/core/uploader/src/queue/processBatchItems.js b/packages/core/uploader/src/queue/processBatchItems.js
@@ -133,7 +133,7 @@ const getAllowedItem = (id: string, queue: QueueState) =>
 	queue.getState().items[id];
 
 //send group of items to be uploaded
-const processBatchItems = (queue: QueueState, ids: string[], next: ProcessNextMethod) => {
+const processBatchItems = (queue: QueueState, ids: string[], next: ProcessNextMethod): Promise<void> => {
     const state = queue.getState();
     //ids will have more than one when grouping is allowed
     let items: any[] = Object.values(state.items);

diff --git a/packages/core/uploader/src/queue/processFinishedRequest.js b/packages/core/uploader/src/queue/processFinishedRequest.js
@@ -27,7 +27,7 @@ type FinishData = { id: string, info: UploadData };
 const getIsFinalized = (item: BatchItem) =>
 	!!~ITEM_FINALIZE_STATES.indexOf(item.state);
 
-export default (queue: QueueState, finishedData: FinishData[], next: ProcessNextMethod) => {
+const processFinishedRequest = (queue: QueueState, finishedData: FinishData[], next: ProcessNextMethod) => {
     finishedData.forEach((itemData: FinishData) => {
         const state = queue.getState();
         const { id, info } = itemData;
@@ -41,8 +41,8 @@ export default (queue: QueueState, finishedData: FinishData[], next: ProcessNext
                 item.uploadResponse = info.response;
 
                 if (getIsFinalized(item)) {
-					delete state.aborts[id];
-				}
+                    delete state.aborts[id];
+                }
             });
 
             //get most up-to-date item data
@@ -80,3 +80,5 @@ export default (queue: QueueState, finishedData: FinishData[], next: ProcessNext
 
     return next(queue);
 };
+
+export default processFinishedRequest;
diff --git a/packages/core/uploader/src/queue/tests/batchHelpers.test.js b/packages/core/uploader/src/queue/tests/batchHelpers.test.js
@@ -147,10 +147,21 @@ describe("batchHelpers tests", () => {
 
 			expect(queueState.updateState).not.toHaveBeenCalled();
 			expect(queueState.state.batches.b1).toBeDefined();
-			expect(queueState.trigger).not.toHaveBeenCalled(); //dWith(UPLOADER_EVENTS.BATCH_FINISH, batch);
+			expect(queueState.trigger).not.toHaveBeenCalled();
 		});
 
-	});
+        it("should'nt finalize batch if no longer in state", () => {
+            const queueState = getQueueState({
+                currentBatch: "b1",
+                batches: {}
+            });
+
+            batchHelpers.cleanUpFinishedBatch(queueState);
+
+            expect(queueState.updateState).not.toHaveBeenCalled();
+            expect(queueState.trigger).not.toHaveBeenCalled();
+        });
+    });
 
 	describe("loadNewBatchForItem tests", () => {
 		it("should load allowed batch", async () => {

diff --git a/story-helpers/useStoryUploadySetup.js b/story-helpers/useStoryUploadySetup.js
@@ -19,13 +19,15 @@ const DEV_DEST_OPTIONS = [
     DESTINATION_TYPES.local,
 ];
 
-const mockSenderEnhancer = getMockSenderEnhancer({ delay: 1000 });
+export const mockDestination = () => {
+    const delay = number("mock send delay", 1000, {}, KNOB_GROUPS.DESTINATION);
 
-export const mockDestination = () => ({
-    destinationType: DESTINATION_TYPES.mock,
-    destination: { url: "http://react-uploady-dummy-server.comm" },
-    enhancer: mockSenderEnhancer
-});
+    return {
+        destinationType: DESTINATION_TYPES.mock,
+        destination: { url: "http://react-uploady-dummy-server.comm" },
+        enhancer: getMockSenderEnhancer({ delay }),
+    }
+};
 
 export const localDestination = ({noLong = false} = {}) => {
 	console.log("GETTING LOCAL DESTINATION ", noLong);
@@ -49,8 +51,7 @@ export const localDestination = ({noLong = false} = {}) => {
 };
 
 const cldDestination = () => {
-    const cloudName = text("(cloudinary) cloud name", process.env.CLD_CLOUD || "", KNOB_GROUPS.DESTINATION);
-    const uploadPreset = text("(cloudinary) upload preset", process.env.CLD_PRESET || "", KNOB_GROUPS.DESTINATION);
+    const cloudName = text("(cloudinary) cloud name", process.env.CLD_CLOUD || "", KNOB_GROUPS.DESTINATION);    const uploadPreset = text("(cloudinary) upload preset", process.env.CLD_PRESET || "", KNOB_GROUPS.DESTINATION);
     const folder = text("(cloudinary) folder", process.env.CLD_TEST_FOLDER || "", KNOB_GROUPS.DESTINATION);
 
     return {