parse-server-s3-adapter icon indicating copy to clipboard operation
parse-server-s3-adapter copied to clipboard

fix: Upgrade to AWS JS SDK v3

Open mman opened this issue 11 months ago • 3 comments

Closes: https://github.com/parse-community/parse-server-s3-adapter/issues/197

The changes appear to be rather minimal and for now working in my staging environment.

Need a way to actually run proper tests and fix any remaining issues.

@mtrezza Could you help me understand how to run the tests? Or will they run automatically on PR?

mman avatar Feb 27 '24 16:02 mman

Thanks for opening this pull request!

@mman Did you try npm test locally? The tests won't run automatically in this PR, I believe because this is your first PR in this repo so we need to approve them manually to run. But if you get them to pass locally, then the CI here is just to ensure it works with multiple Node versions.

mtrezza avatar Feb 27 '24 23:02 mtrezza

@mtrezza So I made some good progress on making npm test pass on my local machine talking to AWS S3. Couple issues remain:

1/ The interface for FilesAdapter.getFileLocation (https://github.com/mman/parse-server/blob/af72a4753967ad5c015f800830510bf928bdc107/src/Adapters/Files/FilesAdapter.js#L64) declares the method to return Absolute URL string synchronously.

However notes on migration to v3 mention that there is no sync method to generate pre-signed URL anymore (https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/migrating/notable-changes/). My JS experience is lacking in what to do now. Basically all remaining tests now fail because I was not able to make getFileLocation work as intended, and I am not sure I even understand what the intended behaviour in case of S3 is.

2/ Using V3 putObject method instead of V2 upload method returns different Body type back.

The original V2 upload (https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html#upload-property) returns some kind of stream or Buffer?

V3 returns https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-smithy-types/TypeAlias/NodeJsRuntimeStreamingBlobPayloadOutputTypes/ which is basically a http response wrapper that can be converted to byte array or string (https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-smithy-types/TypeAlias/SdkStream/). I have for now used conversion to string, but I feel it is not appropriate.

The FilesAdatapter interface does not mention what getFileData should return, but one of the tests assumes it is instance of Buffer.

Need some help here to make progress.

mman avatar Feb 28 '24 17:02 mman

Hi @mman Did you get time to fix this?

thphuccoder avatar Jul 30 '24 09:07 thphuccoder

See https://github.com/parse-community/parse-server-s3-adapter/pull/220

mtrezza avatar Aug 10 '24 00:08 mtrezza

See #220

Much cleaner implementation, will close this PR in favor of #220

mman avatar Aug 10 '24 06:08 mman