Skip to content

Commit

Permalink
fix(storage): fix looping calls to processFile (#5477)
Browse files Browse the repository at this point in the history
* fix(storage): fix looping calls to processFile

* Create four-apricots-compete.md

* update unit tests, ensure onProcessFile is called with path or accessFile
  • Loading branch information
thaddmt authored Jul 26, 2024
1 parent 24eabd9 commit adb985c
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/four-apricots-compete.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@aws-amplify/ui-react-storage": patch
---

fix(storage): fix looping calls to processFile
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export function StorageManagerExample() {
return (
<StorageManager
acceptedFileTypes={['image/*']}
accessLevel="private"
path={({ identityId }) => `private/${identityId}/`}
maxFileCount={3}
showThumbnails={true}
processFile={processFile}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<StorageManager
{...storeManagerProps}
onFileRemove={onFileRemove}
processFile={processFile}
path={() => '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(<StorageManager {...storeManagerProps} maxFileCount={0} />);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
};

0 comments on commit adb985c

Please sign in to comment.