refactor: Migrate S3 Client from AWS SDK v2 to v3

[vahidalizad]

Summary

This pull request migrates the S3 client implementation from AWS SDK v2 to v3.

Changes

1. Initialization: Updated the S3 client initialization to use S3Client from @aws-sdk/client-s3 package.
2. Commands: Replaced v2 commands (getObject, putObject, etc.) with their v3 equivalents (GetObjectCommand, PutObjectCommand, etc.).
3. Response Handling: Modified response handling to accommodate changes in the v3 API, particularly in how streams and buffers are managed.
4. Testing: Added unit tests to cover new functionality and ensure compatibility with the existing codebase. Implemented mocks for offline testing using Jasmine.

Notes

• getSignedUrl Function: The getSignedUrl function in AWS SDK v3 became asynchronous due to new authentication mechanisms. However, because of the way parse-server is designed, this function should not be asynchronous. To address this, deasync was used to make the function synchronous. It might be better to change parse-server to support the asynchronous nature of this function in the future.

References

• AWS SDK for JavaScript v3 Documentation

Please review the changes and provide feedback. Thank you for your time and consideration.

Closes: #197

[parse-github-assistant]

Thanks for opening this pull request!

[mtrezza]

100ms seems quite long, what is this value based on?

[vahidalizad]

The 100ms value was an estimate to prevent CPU overload from a tight loop, as there's no specific AWS documentation on the exact timing needed for this function. Reducing it to 10ms might be possible. Additionally, I suggest opening an issue on parse-server to make the getFileLocation function async, eliminating the need for the deasync library altogether.

[vahidalizad]

I reduced it to 10ms

[mtrezza]

If we estimate the response time for an intra-region call from EC2 to S3 (i.e. Parse Server also running on AWS) to be ~10-100ms, then a 100ms sleep would on avg. double the response time. Maybe setting to 50ms could be a good compromise.

We can only guess a value here, because the best value depends on the actual response times of the specific infrastructure scenario. Using deasync is really more of a hack and we don't know how this responds under heavy concurrent load given that deasync.sleep blocks the event loop. Maybe the correct approach here would be to modify Parse Server first to support sync and async calling of getFileLocation, and then modify the S3 adapter. Would be open to submit a PR to Parse Server? We could add another bounty for that.

[vahidalizad]

Thank you for the feedback! I don't currently have a Parse Server test setup, but I can certainly set one up. It might take some time, but I'm definitely up for the task. I'm happy to work on this without needing the bounty.

Could you please create an issue on the Parse Server repository for this?

[mtrezza]

Done, see parse-community/parse-server#9268.

[vahidalizad]

Thank you! 🙏 I'll get started on it. 🚀

[mtrezza]

How should we proceed here; I suggest to first make the change in Parse Server parse-community/parse-server#9268, release, and then update this PR?

[vahidalizad]

How should we proceed here; I suggest to first make the change in Parse Server parse-community/parse-server#9268, release, and then update this PR?

Yes, I agree that we should update the Parse Server first, release it, and then come back to this PR. I've also fixed the bug when running npm run test on this repo. However, I noticed that the Istanbul configuration might need some adjustments to fully support async functions. I suggest we address that after merging this PR.

[vahidalizad]

I've added a new pull request in the parse-server repo for the async getFileLocation function: #9271. Also, I've created a new pull request for the S3 migration to AWS SDK v3: #221.

You can decide which to merge before or after the release of parse-server.

[mtrezza]

Great, let's take care of parse-community/parse-server#9271 first.

[mtrezza]

@vahidalizad How should we proceed? We merged parse-community/parse-server#9271 and released it as part of Parse Server 7.3.0. Can this PR be closed in favor of #221, and can #221 be merged yet?

[vahidalizad]

@vahidalizad How should we proceed? We merged parse-community/parse-server#9271 and released it as part of Parse Server 7.3.0. Can this PR be closed in favor of #221, and can #221 be merged yet?

Thanks for merging and reaching out! Yes, this PR can be closed in favor of #221. As far as I know, that PR is ready for CI testing, review, and merge.