anacronw / multer-s3

multer storage engine for amazon s3
MIT License
660 stars 190 forks source link

Upgrade to AWS SDK for JavaScript, v3 with modular S3 #146

Closed Takaitra closed 2 years ago

Takaitra commented 3 years ago

AWS SDK for JavaScript, v3, a rewrite of the AWS SDK, was announced in December 2020: https://aws.amazon.com/blogs/developer/modular-aws-sdk-for-javascript-is-now-generally-available/. V2 was monolithic whereas V3 introduces separate packages for each service. This is a feature request to upgrade to v3 of the SDK, importing only the S3 module. This would probably be accompanied by a major version bump for multer-s3.

I'm interested in working on this if you're accepting pull requests. Just let me know.

calvincs commented 3 years ago

Any progress on an aws-sdk3 implementation?

1valdis commented 2 years ago

There's an abstraction library @aws-sdk/lib-storage which allows for similar syntax to the one used currently with aws-sdk v2; might be helpful in making stream uploads on sdk v3. Currently the fact that multer-s3 uses aws sdk v2 blocks me from upgrading my project to aws sdk v3.

dangolbeeker commented 2 years ago

There's an abstraction library @aws-sdk/lib-storage which allows for similar syntax to the one used currently with aws-sdk v2; might be helpful in making stream uploads on sdk v3. Currently the fact that multer-s3 uses aws sdk v2 blocks me from upgrading my project to aws sdk v3.

I'm in the same boat.

Takaitra commented 2 years ago

I haven't been working on this since I ended up downgrading to v2 as the path of least resistance. I'll at least take a look at the scope and see if this is something that is reasonable for me to do in a few weekends.

Takaitra commented 2 years ago

I got it working: https://github.com/Takaitra/multer-s3/commit/282db914963b759e42683e270382269ac38c22ba

It's not ready for a pull request yet

  1. Tests need to be updated
  2. Upload progress isn't implemented. AWS SDK v3 doesn't support the callback.

Not the fault of the original author since this is an older library but it would be nice to start from scratch with TypeScript and ES6 with async/await.

Takaitra commented 2 years ago

There's an abstraction library @aws-sdk/lib-storage which allows for similar syntax to the one used currently with aws-sdk v2; might be helpful in making stream uploads on sdk v3. Currently the fact that multer-s3 uses aws sdk v2 blocks me from upgrading my project to aws sdk v3.

Thanks for the @aws-sdk/lib-storage idea. I'll take a look and see if it simplifies the diff.

dangolbeeker commented 2 years ago

Here's a bridge I found

const { S3Client, DeleteObjectCommand } = require('@aws-sdk/client-s3');
const { Upload } = require('@aws-sdk/lib-storage');

class S3V2ToV3Bridge {

    constructor(configuration) {
        this.s3Client = new S3Client(configuration || {});
    }

    upload(params) {
        return new UploadV2ToV3Bridge({client: this.s3Client, params});
    }

    deleteObject(args, cb) {
        const deleteObjectCommand = new DeleteObjectCommand(args);
        this.s3Client.send(deleteObjectCommand, cb);
    }

}

class UploadV2ToV3Bridge {

    constructor(options) {
        this.upload = new Upload(options);
    }

    on(event, listener) {
        if (event !== 'httpUploadProgress') {
            LOGGER.error(`Something tried to register an event listener for event type '{}' on an instance of UploadV2ToV3Bridge. ` +
                `The only event type permitted to have a registered event listener is 'httpUploadProcess'`, event);
            return;
        }
        this.upload.on('httpUploadProgress', listener);
    }

    async send(cb) {
        try {
            const result = await this.upload.done();
            cb(null, result);
        } catch (err) {
            cb(err, null);
        }
    }

}

module.exports = S3V2ToV3Bridge;
Takaitra commented 2 years ago

@aws-sdk/lib-storage enabled uploading multipart in parallel and fixes the upload progress. Here it is working in my branch: https://github.com/anacronw/multer-s3/compare/master...Takaitra:aws-sdk-v3

Todo

  1. Fix tests
  2. Update README
1valdis commented 2 years ago

@aws-sdk/lib-storage enabled uploading multipart in parallel and fixes the upload progress. Here it is working in my branch: master...Takaitra:aws-sdk-v3

Todo

  1. Fix tests
  2. Update README

As far as I understand, you end up with concatenating the entire upload into a buffer, and only then uploading to s3? That's not good for uploading files of a few GB in size.

Takaitra commented 2 years ago

@aws-sdk/lib-storage enabled uploading multipart in parallel and fixes the upload progress. Here it is working in my branch: master...Takaitra:aws-sdk-v3 Todo

  1. Fix tests
  2. Update README

As far as I understand, you end up with concatenating the entire upload into a buffer, and only then uploading to s3? That's not good for uploading files of a few GB in size.

I agree. I'll investigate dropping streamToBuffer or at least find a replacement that doesn't concat the entire file to a buffer.

Takaitra commented 2 years ago

@1valdis, @aws-sdk/lib-storage is able to work on the stream directly. I removed the streamToBuffer.

Takaitra commented 2 years ago

README is updated and tests are fixed. Pull request is here: https://github.com/anacronw/multer-s3/pull/162

1valdis commented 2 years ago

This issue can now be closed I think? It was released as v3.0.0

LinusU commented 2 years ago

Nice catch! 👍