feat(lib-storage): rewrite lib-storage upload

[alexforsyth]

Issue

Description

As part of a review of our codebase I rewrote Upload(). This PR fixes a range of issues and should address most of the Issues filed (below). This rewrite is backwards compatible.

Upload is a wrapper on s3.multipart upload. This is largely analogous to managed upload in V2 of the JS SDK.
Here's how you use Upload:

import { Upload } from "@aws-sdk/lib-storage";
import { S3 } from "@aws-sdk/client-s3";

(async () => {
  const upload = new Upload({
    params: {
      Bucket: configuration.Bucket,
      Key: configuration.Key,
      Body: String | Buffer | Uint8Array | Stream | ReadableStream,
    },
    client: new S3({}),
    queueSize: 3,
  });

  upload.on("httpUploadProgress", (progress) => {
    console.log(progress);
  });

  await upload.done();
})();

Testing

How was this change tested?

• Test cases enumerating chunking
• Test cases making sure multipart upload is called as expected
• Upload should work if you pass in a version of S3 that is older than the version of S3 that Upload depends on. Upload constructor in @aws-sdk/lib-storage appears to be broken #1973, @aws-sdk/lib-storage: client is not compatible if lib-storage version and client version is different #1729
• Upload should allow you to set MetaData: Cannot set Metadata when using lib-storage Upload. #1891

Manual Browser testing

• correctly uploads a string
• correctly uploads an ArrayBuffer
• correctly uploads a stream of unknown length (audio stream)
• correctly provides status updates

Manual Node testing

• correctly uploads a string
• correctly uploads an Buffer
• correctly uploads a stream of unknown length (audio stream)
• correctly provides status updates

Additional context

Add any other context about the PR here.

PRs:
Fixes: #1990
Fixes: #1955

Issues:
Fixes: #1973
Fixes: #1891
Fixes: #1729
Fixes: #1688

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[monken]

Hi @alexforsyth, will you incorporate the performance improvements from my PR at #1955 ?

[alexforsyth]

@monken yes! I'll request you as a reviewer when this PR is in better shape. Everything should be using for...await syntax here. I'm looking at the buffer concat in readable chunker right now. The way you do it is much better, I'll only concat once.

[codecov-io]

Codecov Report

Merging #2039 (05b6f8a) into master (f2a47e8) will increase coverage by 0.26%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2039      +/-   ##
==========================================
+ Coverage   79.30%   79.57%   +0.26%     
==========================================
  Files         368      365       -3     
  Lines       15132    15130       -2     
  Branches     3222     3251      +29     
==========================================
+ Hits        12001    12039      +38     
+ Misses       3131     3091      -40     

Impacted Files	Coverage Δ
packages/util-waiter/src/utils/validate.ts	81.81% <0.00%> (-18.19%)	⬇️
...process-arnables-plugin/update-arnables-request.ts	91.30% <0.00%> (-8.70%)	⬇️
packages/s3-request-presigner/src/presigner.ts	94.73% <0.00%> (-5.27%)	⬇️
...s/middleware-bucket-endpoint/src/bucketHostname.ts	98.27% <0.00%> (-1.73%)	⬇️
packages/util-dynamodb/src/convertToAttr.ts	98.94% <0.00%> (-1.06%)	⬇️
lib/storage/src/index.ts	100.00% <0.00%> (ø)
packages/util-waiter/src/poller.ts	100.00% <0.00%> (ø)
packages/util-waiter/src/waiter.ts	100.00% <0.00%> (ø)
protocol_tests/aws-ec2/endpoints.ts	81.48% <0.00%> (ø)
packages/smithy-client/src/client.ts	76.47% <0.00%> (ø)
... and 74 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16214be...05b6f8a. Read the comment docs.

[alexforsyth]

@monken it looks like I cannot add you as a reviewer to this PR. I'd still love your feedback at any rate

[samsullivan]

Thank you so much for your attention here, since I only was able to give partial care previously and we've been waiting for these performance improvements to switch over to the new SDK.

If you care, I'll get this tested with my use case some point this week.

[alexforsyth]

@samsullivan that'd be great! We're looking to get this out with the next release

[markb-trustifi]

Please check that it supports server-side encryption with SSECustomerKey.

[alexforsyth]

@markb-trustifi it should, all the params you normally pass to multipart upload should work just the same as they are simply passed through to the underlying client.

[samsullivan]

Don't have time to review the code changes, but I 💯 agree that this needed to be rewritten and I appreciate your work. I built you're branch in my test build and threw this into our stage AWS environment just now and it worked fine!

Super excited to have this released. v2 of the JS SDK has an ECS Fargate bug where it doesn't use the IAM role on the container, so I'll get to remove some access keys once upgraded 🙂

[alexforsyth]

@monken @samsullivan @xfournet Thanks so much for the engagement. It's awesome to see the involvement. We're aiming at releasing these changes soon

[aws-sdk-js-automation]

AWS CodeBuild CI Report

• CodeBuild project: sdk-staging-test
• Commit ID: 6fe4156
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[alexforsyth]

FYI: This should be out with 3.6.0

[skozer]

Confirmed working in 3.6.0, thanks!

[samsullivan]

☝️ same here, really appreciate the effort by everybody on this one. Happy to be off of v2 SDK!

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.