fixups post pre review

Author: benzekrimaha

tests/functional/aws-node-sdk/lib/utility/versioning-util.js

@@ -1,10 +1,16 @@
 const async = require('async');
 const assert = require('assert');
-const { S3 } = require('aws-sdk');
+const { S3Client, 
+    ListObjectVersionsCommand, 
+    GetObjectCommand,
+    DeleteObjectsCommand,
+    PutBucketVersioningCommand,
+    PutObjectCommand,
+    DeleteObjectCommand } = require('@aws-sdk/client-s3');
 
 const getConfig = require('../../test/support/config');
-const config = getConfig('default', { signatureVersion: 'v4' });
-const s3 = new S3(config);
+const config = getConfig('default');
+const s3Client = new S3Client(config);
 
 const versioningEnabled = { Status: 'Enabled' };
 const versioningSuspended = { Status: 'Suspended' };
@@ -19,28 +25,25 @@ function _deleteVersionList(versionList, bucket, callback) {
             Key: version.Key, VersionId: version.VersionId });
     });
 
-    return s3.deleteObjects(params, callback);
+    return s3Client.send(new DeleteObjectsCommand(params)).then(() => callback()).catch(err => callback(err));
 }
 
 function checkOneVersion(s3, bucket, versionId, callback) {
-    return s3.listObjectVersions({ Bucket: bucket },
-        (err, data) => {
-            if (err) {
-                callback(err);
-            }
+    return s3Client.send(new ListObjectVersionsCommand({ Bucket: bucket })).then(data => {
             assert.strictEqual(data.Versions.length, 1);
             if (versionId) {
                 assert.strictEqual(data.Versions[0].VersionId, versionId);
             }
             assert.strictEqual(data.DeleteMarkers.length, 0);
             callback();
-        });
+        }).catch(err => callback(err));
 }
 
 function removeAllVersions(params, callback) {
     const bucket = params.Bucket;
     async.waterfall([
-        cb => s3.listObjectVersions(params, cb),
+        cb => s3Client.send(new ListObjectVersionsCommand(params)).then(data =>
+            cb(null, data)).catch(err => cb(err)),
         (data, cb) => _deleteVersionList(data.DeleteMarkers, bucket,
             err => cb(err, data)),
         (data, cb) => _deleteVersionList(data.Versions, bucket,
@@ -60,25 +63,26 @@ function removeAllVersions(params, callback) {
 }
 
 function suspendVersioning(bucket, callback) {
-    s3.putBucketVersioning({
+    s3Client.send(new PutBucketVersioningCommand({
         Bucket: bucket,
         VersioningConfiguration: versioningSuspended,
-    }, callback);
+    })).then(() => callback()).catch(err => callback(err));
 }
 
 function enableVersioning(bucket, callback) {
-    s3.putBucketVersioning({
+    s3Client.send(new PutBucketVersioningCommand({
         Bucket: bucket,
         VersioningConfiguration: versioningEnabled,
-    }, callback);
+    })).then(() => callback()).catch(err => callback(err));
 }
 
 function enableVersioningThenPutObject(bucket, object, callback) {
     enableVersioning(bucket, err => {
         if (err) {
             callback(err);
         }
-        s3.putObject({ Bucket: bucket, Key: object }, callback);
+        s3Client.send(new PutObjectCommand({ Bucket: bucket, Key: object })).then(() => 
+            callback()).catch(err => callback(err));
     });
 }
 
@@ -102,33 +106,35 @@ function enableVersioningThenPutObject(bucket, object, callback) {
 function createDualNullVersion(s3, bucketName, keyName, cb) {
     async.waterfall([
         // put null version
-        next => s3.putObject({ Bucket: bucketName, Key: keyName },
-            err => next(err)),
+        next => s3Client.send(new PutObjectCommand({ Bucket: bucketName, Key: keyName, Body: null })).then(() => 
+            next()).catch(err => next(err)),
         next => enableVersioning(bucketName, err => next(err)),
         // should store null version as separate version before
         // putting new version
-        next => s3.putObject({ Bucket: bucketName, Key: keyName },
-            (err, data) => {
-                assert.strictEqual(err, null,
-                    'Unexpected err putting new version');
-                assert(data.VersionId);
-                next(null, data.VersionId);
-            }),
+        next => s3Client.send(new PutObjectCommand({ Bucket: bucketName, Key: keyName })).then(data => {
+            assert(data.VersionId);
+            next(null, data.VersionId);
+        }).catch(err => {
+            assert.strictEqual(err, null,
+                'Unexpected err putting new version');
+            next(err);
+        }),
         // delete version we just created, master version should be updated
         // with value of next most recent version: null version previously put
-        (versionId, next) => s3.deleteObject({
+        (versionId, next) => s3Client.send(new DeleteObjectCommand({
             Bucket: bucketName,
             Key: keyName,
             VersionId: versionId,
-        }, err => next(err)),
+        })).then(() => next()).catch(err => next(err)),
         // getting object should return null version now
-        next => s3.getObject({ Bucket: bucketName, Key: keyName },
-            (err, data) => {
-                assert.strictEqual(err, null,
-                    'Unexpected err getting latest version');
-                assert.strictEqual(data.VersionId, 'null');
-                next();
-            }),
+        next => s3Client.send(new GetObjectCommand({ Bucket: bucketName, Key: keyName })).then(data => {
+            assert.strictEqual(data.VersionId, 'null');
+            next();
+        }).catch(err => {
+            assert.strictEqual(err, null,
+                'Unexpected err getting latest version');
+            next(err);
+        }),
     ], err => cb(err));
 }
 

tests/functional/aws-node-sdk/test/multipleBackend/delete/delete.js

@@ -70,13 +70,21 @@ describeSkipIfNotMultiple('Multiple backend delete', () => {
                 Metadata: { 'scal-location-constraint': awsLocation } 
             }));
 
-            process.stdout.write('Putting big object to AWS\n');
+            process.stdout.write('Putting large object to AWS\n');
             await s3.send(new PutObjectCommand({ 
                 Bucket: bucket, 
                 Key: bigObject, 
                 Body: bigBody,
                 Metadata: { 'scal-location-constraint': awsLocation } 
             }));
+
+            process.stdout.write('Putting object to AWS\n');
+            await s3.send(new PutObjectCommand({ 
+                Bucket: bucket, 
+                Key: mismatchObject, 
+                Body: body,
+                Metadata: { 'scal-location-constraint': awsLocationMismatch } 
+            }));
         });
 
         after(async () => {
@@ -91,7 +99,7 @@ describeSkipIfNotMultiple('Multiple backend delete', () => {
 
             try {
                 await s3.send(new GetObjectCommand({ Bucket: bucket, Key: memObject }));
-                throw new Error('Expected NoSuchKey error but got success');
+                assert.fail('Expected NoSuchKey error but got success');
             } catch (err) {
                 assert.strictEqual(err.code, 'NoSuchKey');
             }
@@ -102,7 +110,7 @@ describeSkipIfNotMultiple('Multiple backend delete', () => {
 
             try {
                 await s3.send(new GetObjectCommand({ Bucket: bucket, Key: fileObject }));
-                throw new Error('Expected NoSuchKey error but got success');
+                assert.fail('Expected NoSuchKey error but got success');
             } catch (err) {
                 assert.strictEqual(err.code, 'NoSuchKey');
             }
@@ -113,7 +121,7 @@ describeSkipIfNotMultiple('Multiple backend delete', () => {
 
             try {
                 await s3.send(new GetObjectCommand({ Bucket: bucket, Key: awsObject }));
-                throw new Error('Expected NoSuchKey error but got success');
+                assert.fail('Expected NoSuchKey error but got success');
             } catch (err) {
                 assert.strictEqual(err.code, 'NoSuchKey');
             }
@@ -124,7 +132,7 @@ describeSkipIfNotMultiple('Multiple backend delete', () => {
 
             try {
                 await s3.send(new GetObjectCommand({ Bucket: bucket, Key: emptyObject }));
-                throw new Error('Expected NoSuchKey error but got success');
+                assert.fail('Expected NoSuchKey error but got success');
             } catch (err) {
                 assert.strictEqual(err.code, 'NoSuchKey');
             }
@@ -135,24 +143,23 @@ describeSkipIfNotMultiple('Multiple backend delete', () => {
 
             try {
                 await s3.send(new GetObjectCommand({ Bucket: bucket, Key: bigObject }));
-                throw new Error('Expected NoSuchKey error but got success');
+                assert.fail('Expected NoSuchKey error but got success');
             } catch (err) {
                 assert.strictEqual(err.code, 'NoSuchKey');
             }
         });
 
-        it('should return an InvalidLocationConstraint ' +
-            'error for mismatch location', async () => {
+         it('should delete object from AWS location with bucketMatch set to ' +
+            'false', async () => {
             try {
-                await s3.send(new PutObjectCommand({ 
+                await s3.send(new DeleteObjectCommand({ Bucket: bucket, Key: mismatchObject }));
+                await s3.send(new GetObjectCommand({ 
                     Bucket: bucket, 
-                    Key: mismatchObject, 
-                    Body: body,
-                    Metadata: { 'scal-location-constraint': awsLocationMismatch } 
+                    Key: mismatchObject
                 }));
-                throw new Error('Expected InvalidLocationConstraint error but got success');
+                assert.fail('Expected NoSuchKey error but got success');
             } catch (err) {
-                assert.strictEqual(err.code, 'InvalidLocationConstraint');
+                assert.strictEqual(err.code, 'NoSuchKey');
             }
         });
     });

tests/functional/aws-node-sdk/test/multipleBackend/get/get.js

@@ -71,9 +71,7 @@ describe('Multiple backend get object', function testSuite() {
             done => {
                 const command = new GetObjectCommand({ Bucket: '', Key: 'somekey' });
                 s3.send(command)
-                    .then(() => {
-                        done(new Error('Expected failure but got success'));
-                    })
+                    .then(() => done(new Error('Expected failure but got success')))
                     .catch(err => {
                         assert.notEqual(err, null,
                             'Expected failure but got success');
@@ -85,9 +83,7 @@ describe('Multiple backend get object', function testSuite() {
             done => {
                 const command = new GetObjectCommand({ Bucket: bucket, Key: 'nope' });
                 s3.send(command)
-                    .then(() => {
-                        done(new Error('Expected failure but got success'));
-                    })
+                    .then(() => done(new Error('Expected failure but got success')))
                     .catch(err => {
                         assert.notEqual(err, null,
                             'Expected failure but got success');

tests/functional/aws-node-sdk/test/multipleBackend/mpuComplete/mpuAwsVersioning.js

@@ -112,9 +112,14 @@ function testSuite() {
                 LocationConstraint: awsLocation,
             },
         })).then(() => done()).catch(err => done(err)));
-        afterEach(async () => {
-            await removeAllVersions({ Bucket: bucket });
-            await s3.send(new DeleteBucketCommand({ Bucket: bucket }));
+        afterEach(done => {
+            removeAllVersions({ Bucket: bucket }, err => {
+                if (err) {
+                    return done(err);
+                }
+                return s3.send(new DeleteBucketCommand({ Bucket: bucket }))
+                .then(() => done()).catch(done);
+            });
         });
 
         it('versioning not configured: should not return version id ' +