From adb985c98c5b150280bdf2592aced3c5d3b1fc78 Mon Sep 17 00:00:00 2001 From: thaddmt <68032955+thaddmt@users.noreply.github.com> Date: Thu, 25 Jul 2024 19:17:11 -0700 Subject: [PATCH] fix(storage): fix looping calls to processFile (#5477) * fix(storage): fix looping calls to processFile * Create four-apricots-compete.md * update unit tests, ensure onProcessFile is called with path or accessFile --- .changeset/four-apricots-compete.md | 5 +++ .../process-file-access-level/index.page.tsx | 2 +- .../__tests__/StorageManager.test.tsx | 45 +++++++++++++++++++ .../utils/__tests__/getInput.spec.ts | 32 ++++++++++++- .../StorageManager/utils/getInput.ts | 29 +++++++----- 5 files changed, 100 insertions(+), 13 deletions(-) create mode 100644 .changeset/four-apricots-compete.md diff --git a/.changeset/four-apricots-compete.md b/.changeset/four-apricots-compete.md new file mode 100644 index 00000000000..d893bfd0cff --- /dev/null +++ b/.changeset/four-apricots-compete.md @@ -0,0 +1,5 @@ +--- +"@aws-amplify/ui-react-storage": patch +--- + +fix(storage): fix looping calls to processFile diff --git a/examples/next/pages/ui/components/storage/storage-manager/process-file-access-level/index.page.tsx b/examples/next/pages/ui/components/storage/storage-manager/process-file-access-level/index.page.tsx index 6d9c307e4f0..b7e61805b11 100644 --- a/examples/next/pages/ui/components/storage/storage-manager/process-file-access-level/index.page.tsx +++ b/examples/next/pages/ui/components/storage/storage-manager/process-file-access-level/index.page.tsx @@ -28,7 +28,7 @@ export function StorageManagerExample() { return ( `private/${identityId}/`} maxFileCount={3} showThumbnails={true} processFile={processFile} diff --git a/packages/react-storage/src/components/StorageManager/__tests__/StorageManager.test.tsx b/packages/react-storage/src/components/StorageManager/__tests__/StorageManager.test.tsx index daa9d5a5c49..8128408d32f 100644 --- a/packages/react-storage/src/components/StorageManager/__tests__/StorageManager.test.tsx +++ b/packages/react-storage/src/components/StorageManager/__tests__/StorageManager.test.tsx @@ -374,6 +374,51 @@ describe('StorageManager', () => { }); }); + it('provides the processed file key on a remove file event after upload when processFile is provided with a path function', async () => { + const onFileRemove = jest.fn(); + + const processedKey = 'processedKey'; + const processFile: StorageManagerProps['processFile'] = (input) => ({ + ...input, + key: processedKey, + }); + + const { container } = render( + 'my-path'} + accessLevel={undefined} + /> + ); + + const hiddenInput = document.querySelector( + 'input[type="file"]' + ) as HTMLInputElement; + + expect(hiddenInput).toBeInTheDocument(); + const file = new File(['file content'], 'file.txt', { type: 'text/plain' }); + + fireEvent.change(hiddenInput, { target: { files: [file] } }); + + // Wait for the file to be uploaded + await waitFor(() => { + expect(uploadDataSpy).toHaveBeenCalled(); + + const removeButton = getByTestId( + container, + 'storage-manager-remove-button' + ); + expect(removeButton).toBeDefined(); + + fireEvent.click(removeButton); + + expect(onFileRemove).toHaveBeenCalledTimes(1); + expect(onFileRemove).toHaveBeenCalledWith({ key: processedKey }); + }); + }); + it('logs a warning if maxFileCount is zero', () => { render(); diff --git a/packages/react-storage/src/components/StorageManager/utils/__tests__/getInput.spec.ts b/packages/react-storage/src/components/StorageManager/utils/__tests__/getInput.spec.ts index 2f419bc4893..0ff0394999f 100644 --- a/packages/react-storage/src/components/StorageManager/utils/__tests__/getInput.spec.ts +++ b/packages/react-storage/src/components/StorageManager/utils/__tests__/getInput.spec.ts @@ -3,7 +3,9 @@ import { UploadDataWithPathInput, UploadDataInput } from 'aws-amplify/storage'; import { getInput, GetInputParams } from '../getInput'; const identityId = 'identity-id'; -jest.spyOn(AuthModule, 'fetchAuthSession').mockResolvedValue({ identityId }); +const fetchAuthSpy = jest + .spyOn(AuthModule, 'fetchAuthSession') + .mockResolvedValue({ identityId }); const file = new File(['hello'], 'hello.png', { type: 'image/png' }); const key = file.name; @@ -54,6 +56,7 @@ const accessLevelWithPathInput: GetInputParams = { describe('getInput', () => { beforeEach(() => { onProcessFileSuccess.mockClear(); + fetchAuthSpy.mockClear(); }); it('resolves an UploadDataWithPathInput with a string `path` as expected', async () => { @@ -186,6 +189,33 @@ describe('getInput', () => { contentDisposition ); }); + + it('calls `onProcessFileSuccess` after fetchAuthSession', async () => { + const processedKey = `processedKey`; + + const input = getInput({ + ...pathCallbackInput, + processFile: ({ key: _, ...rest }) => ({ + key: processedKey, + ...rest, + }), + }); + + await input(); + + const fetchAuthSessionCallOrder = fetchAuthSpy.mock.invocationCallOrder[0]; + const onProcessFileSuccessCallORder = + onProcessFileSuccess.mock.invocationCallOrder[0]; + expect(fetchAuthSessionCallOrder).toBeLessThan( + onProcessFileSuccessCallORder + ); + + expect(fetchAuthSpy).toHaveBeenCalledTimes(1); + expect(onProcessFileSuccess).toHaveBeenCalledTimes(1); + expect(onProcessFileSuccess).toHaveBeenCalledWith({ + processedKey, + }); + }); }); it('defaults `options.contentType` to "binary/octet-stream" when no file type is provided', async () => { diff --git a/packages/react-storage/src/components/StorageManager/utils/getInput.ts b/packages/react-storage/src/components/StorageManager/utils/getInput.ts index ca86bc073bd..146aa07f4bd 100644 --- a/packages/react-storage/src/components/StorageManager/utils/getInput.ts +++ b/packages/react-storage/src/components/StorageManager/utils/getInput.ts @@ -39,30 +39,37 @@ export const getInput = ({ ...rest } = await resolveFile({ file, key, processFile }); - if (processFile) { - // provide post-processing value of target `key` - onProcessFileSuccess({ processedKey }); - } - const contentType = file.type || 'binary/octet-stream'; // IMPORTANT: always pass `...rest` here for backwards compatibility const options = { contentType, onProgress, ...rest }; + let inputResult: PathInput | UploadDataInput; if (hasKeyInput) { // legacy handling of `path` is to prefix to `fileKey` const resolvedKey = hasStringPath ? `${path}${processedKey}` : processedKey; - return { data, key: resolvedKey, options: { ...options, accessLevel } }; + inputResult = { + data, + key: resolvedKey, + options: { ...options, accessLevel }, + }; + } else { + const { identityId } = await fetchAuthSession(); + const resolvedPath = `${ + hasCallbackPath ? path({ identityId }) : path + }${processedKey}`; + + inputResult = { data: file, path: resolvedPath, options }; } - const { identityId } = await fetchAuthSession(); - const resolvedPath = `${ - hasCallbackPath ? path({ identityId }) : path - }${processedKey}`; + if (processFile) { + // provide post-processing value of target `key` + onProcessFileSuccess({ processedKey }); + } - return { data: file, path: resolvedPath, options }; + return inputResult; }; };