Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 47 additions & 31 deletions tests/functional/aws-node-sdk/lib/utility/customS3Request.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,59 @@
const { S3 } = require('aws-sdk');
const { S3Client } = require('@aws-sdk/client-s3');
const { HttpRequest } = require('@smithy/protocol-http');
const querystring = require('querystring');

const getConfig = require('../../test/support/config');

const config = getConfig('default', { signatureVersion: 'v4' });
const s3 = new S3(config);
const config = getConfig('default');

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

nits


// Custom middleware to modify requests without mutating args
const customRequestMiddleware = buildParams => next => async args => {

function customS3Request(action, params, buildParams, callback) {
const method = action.bind(s3);
const request = method(params);
const { headers, query } = buildParams;
// modify underlying http request object created by aws sdk
request.on('build', () => {
Object.assign(request.httpRequest.headers, headers);
if (query) {
const qs = querystring.stringify(query);
// NOTE: that this relies on there not being a query string in the
// first place; if there is a qs then we have to search for ? and
// append &qs at the end of the string, if ? is not followed by ''
request.httpRequest.path = `${request.httpRequest.path}?${qs}`;
}
});
request.on('success', response => {
const resData = {
statusCode: response.httpResponse.statusCode,
headers: response.httpResponse.headers,
body: response.httpResponse.body.toString('utf8'),
};
callback(null, resData);

const prevReq = args.request;
const base = prevReq instanceof HttpRequest ? prevReq : new HttpRequest(prevReq);

let newHeaders = base.headers || {};
if (headers) {
newHeaders = { ...newHeaders, ...headers };
}

let newQuery = base.query || {};
if (query) {
const extra = querystring.parse(querystring.stringify(query));
newQuery = { ...newQuery, ...extra };
}

const newReq = new HttpRequest({
...base,
headers: newHeaders,
query: newQuery,
});
request.on('error', err => {

return next({ ...args, request: newReq });
};

async function customS3Request(CommandClass, params, buildParams) {
const customS3 = new S3Client({ ...config });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation issue


customS3.middlewareStack.add(
customRequestMiddleware(buildParams),
{ step: 'build', name: 'customRequestMiddleware', tags: ['CUSTOM'] }
);

const command = new CommandClass(params);
const response = await customS3.send(command);

const resData = {
statusCode: request.response.httpResponse.statusCode,
headers: request.response.httpResponse.headers,
body: request.response.httpResponse.body.toString('utf8'),
statusCode: 200,
headers: response.$metadata?.httpHeaders || {},
body: JSON.stringify(response),
};
callback(err, resData);
});
request.send();

return resData;

}

module.exports = customS3Request;
7 changes: 3 additions & 4 deletions tests/functional/aws-node-sdk/lib/utility/versioning-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,14 @@ function _deleteVersionList(versionList, bucket, callback) {
return s3Client.send(new DeleteObjectsCommand(params)).then(() => callback()).catch(err => callback(err));
}

function checkOneVersion(s3, bucket, versionId, callback) {
function checkOneVersion(s3, bucket, versionId) {
return s3Client.send(new ListObjectVersionsCommand({ Bucket: bucket })).then(data => {
Comment on lines +31 to 32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
function checkOneVersion(s3, bucket, versionId) {
return s3Client.send(new ListObjectVersionsCommand({ Bucket: bucket })).then(data => {
async function checkOneVersion(s3, bucket, versionId) {
return await s3Client.send(new ListObjectVersionsCommand({ Bucket: bucket })).then(data => {

This will keep the checkOneVersion in the stack trace. It's a "good" practice for async/await. But nits

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));
assert.strictEqual(data.DeleteMarkers?.length, undefined);
});
}

function removeAllVersions(params, callback) {
Expand Down
104 changes: 52 additions & 52 deletions tests/functional/aws-node-sdk/test/versioning/bucketDelete.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
const assert = require('assert');
const async = require('async');
const {
CreateBucketCommand,
PutBucketVersioningCommand,
DeleteBucketCommand,
PutObjectCommand,
DeleteObjectCommand,
} = require('@aws-sdk/client-s3');

const withV4 = require('../support/withV4');
const BucketUtility = require('../../lib/utility/bucket-util');
Expand All @@ -12,11 +18,7 @@ const key = 'anObject';

function checkError(err, code) {
assert.notEqual(err, null, 'Expected failure but got success');
assert.strictEqual(err.code, code);
}

function checkNoError(err) {
assert.ifError(err, `Expected success, got error ${JSON.stringify(err)}`);
assert.strictEqual(err.Code, code);
}

describe('aws-node-sdk test delete bucket', () => {
Expand All @@ -25,72 +27,70 @@ describe('aws-node-sdk test delete bucket', () => {
const s3 = bucketUtil.s3;

// setup test
beforeEach(done => {
async.waterfall([
next => s3.createBucket({ Bucket: bucketName },
err => next(err)),
next => s3.putBucketVersioning({
Bucket: bucketName,
VersioningConfiguration: {
Status: 'Enabled',
},
}, err => next(err)),
], done);
beforeEach(async () => {
await s3.send(new CreateBucketCommand({ Bucket: bucketName }));
await s3.send(new PutBucketVersioningCommand({
Bucket: bucketName,
VersioningConfiguration: {
Status: 'Enabled',
},
}));
});

// empty and delete bucket after testing if bucket exists
afterEach(done => {
afterEach(done => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
afterEach(done => {
afterEach(done => {

indentation ? Weird that the CI pass 😬

removeAllVersions({ Bucket: bucketName }, err => {
if (err && err.code === 'NoSuchBucket') {
if (err?.name === 'NoSuchBucket') {
return done();
} else if (err) {
return done(err);
}
return s3.deleteBucket({ Bucket: bucketName }, done);
return s3.send(new DeleteBucketCommand({ Bucket: bucketName }))
.then(() => done()).catch(err => {
if (err.name === 'NoSuchBucket') {
return done();
}
return done(err);
});
});
});

it('should be able to delete empty bucket with version enabled',
done => {
s3.deleteBucket({ Bucket: bucketName }, err => {
checkNoError(err);
return done();
});
async () => {
await s3.send(new DeleteBucketCommand({ Bucket: bucketName }));
});

it('should return error 409 BucketNotEmpty if trying to delete bucket' +
' containing delete marker', done => {
s3.deleteObject({ Bucket: bucketName, Key: key }, err => {
if (err) {
return done(err);
}
return s3.deleteBucket({ Bucket: bucketName }, err => {
checkError(err, 'BucketNotEmpty');
return done();
});
});
' containing delete marker', async () => {
await s3.send(new DeleteObjectCommand({ Bucket: bucketName, Key: key }));

try {
await s3.send(new DeleteBucketCommand({ Bucket: bucketName }));
assert.fail('Expected BucketNotEmpty error but got success');
} catch (err) {
checkError(err, 'BucketNotEmpty');
}
});

it('should return error 409 BucketNotEmpty if trying to delete bucket' +
' containing version and delete marker', done => {
async.waterfall([
next => s3.putObject({ Bucket: bucketName, Key: key },
err => next(err)),
next => s3.deleteObject({ Bucket: bucketName, Key: key },
err => next(err)),
next => s3.deleteBucket({ Bucket: bucketName }, err => {
checkError(err, 'BucketNotEmpty');
return next();
}),
], done);
' containing version and delete marker', async () => {
await s3.send(new PutObjectCommand({ Bucket: bucketName, Key: key }));
await s3.send(new DeleteObjectCommand({ Bucket: bucketName, Key: key }));

try {
await s3.send(new DeleteBucketCommand({ Bucket: bucketName }));
assert.fail('Expected BucketNotEmpty error but got success');
} catch (err) {
checkError(err, 'BucketNotEmpty');
}
});

it('should return error 404 NoSuchBucket if the bucket name is invalid',
done => {
s3.deleteBucket({ Bucket: 'bucketA' }, err => {
async () => {
try {
await s3.send(new DeleteBucketCommand({ Bucket: 'bucketA' }));
assert.fail('Expected NoSuchBucket error but got success');
} catch (err) {
checkError(err, 'NoSuchBucket');
return done();
});
}
});
});
});
Loading
Loading