Skip to content

Commit

Permalink
Change function order in abortMultipartUpload to avoid code duplicati…
Browse files Browse the repository at this point in the history 
…on and MD orphans

Remove duplicated import in tests

Issue: CLDSRV-570
  • Loading branch information
@KillianG
KillianG committed Nov 18, 2024
1 parent bc34e74 commit a934fe1
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 44 deletions.
56 changes: 22 additions & 34 deletions lib/api/apiUtils/object/abortMultipartUpload.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,31 +92,9 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
skipDataDelete);
});
},
function deleteData(mpuBucket, storedParts, destBucket, objectMD,
skipDataDelete, next) {
if (skipDataDelete) {
return next(null, mpuBucket, storedParts, destBucket, objectMD);
}
// The locations were sent to metadata as an array
// under partLocations. Pull the partLocations.
let locations = storedParts.map(item => item.value.partLocations);
if (locations.length === 0) {
return next(null, mpuBucket, storedParts, destBucket, objectMD);
}
// flatten the array
locations = [].concat(...locations);
return async.eachLimit(locations, 5, (loc, cb) => {
data.delete(loc, log, err => {
if (err) {
log.fatal('delete ObjectPart failed', { err });
}
cb();
});
}, () => next(null, mpuBucket, storedParts, destBucket, objectMD));
},
function deleteObjectMetadata(mpuBucket, storedParts, destBucket, objectMD, next) {
function deleteObjectMetadata(mpuBucket, storedParts, destBucket, objectMD, skipDataDelete, next) {
if (!objectMD || metadataValMPUparams.uploadId !== objectMD.uploadId) {
return next(null, mpuBucket, storedParts, destBucket, objectMD);
return next(null, mpuBucket, storedParts, destBucket, objectMD, skipDataDelete);
}
// In case there has been an error during cleanup after a complete MPU
// (e.g. failure to delete MPU MD in shadow bucket),
Expand All @@ -132,23 +110,33 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
if (err) {
log.error('error deleting object metadata', { error: err });
}
return next(err, mpuBucket, storedParts, destBucket, objectMD);
return next(err, mpuBucket, storedParts, destBucket, objectMD, skipDataDelete);
});
},
function deleteObjectData(mpuBucket, storedParts, destBucket, objectMD, next) {
if (!objectMD?.location) {
function deleteData(mpuBucket, storedParts, destBucket, objectMD,
skipDataDelete, next) {
if (skipDataDelete) {
return next(null, mpuBucket, storedParts, destBucket);
}
// Filtering parts that has already been delete by the previous step
const partLocations = new Set(
storedParts.flatMap(item => item.value.partLocations.map(loc => loc.key))
);
const objectLocationLeft = objectMD.location.filter(loc => !partLocations.has(loc.key));
// The locations were sent to metadata as an array
// under partLocations. Pull the partLocations.
let locations = storedParts.map(item => item.value.partLocations);
if (locations.length === 0) {
return next(null, mpuBucket, storedParts, destBucket);
}
// flatten the array
locations = [].concat(...locations);

return async.eachLimit(objectLocationLeft, 5, (loc, cb) => {
if (objectMD?.location) {
const objectLocationLeft = objectMD.location.filter(loc =>
!locations.some(existingLoc => existingLoc.key === loc.key));
locations = locations.concat(objectLocationLeft);
}

return async.eachLimit(locations, 5, (loc, cb) => {
data.delete(loc, log, err => {
if (err) {
log.fatal('delete object data failed', { err });
log.fatal('delete ObjectPart failed', { err });
}
cb();
});
Expand Down
19 changes: 9 additions & 10 deletions tests/unit/api/multipartUpload.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const metadataswitch = require('../metadataswitch');
const { metadata } = storage.metadata.inMemory.metadata;
const metadataBackend = storage.metadata.inMemory.metastore;
const { ds } = storage.data.inMemory.datastore;
const ms = storage.metadata.inMemory.metastore;

const log = new DummyRequestLogger();

Expand Down Expand Up @@ -1919,8 +1918,8 @@ describe('Multipart Upload API', () => {
});

it('should abort an MPU and delete its MD if it has been created by a failed complete before', done => {
const delMeta = ms.deleteObject;
ms.deleteObject = (bucketName, objName, params, log, cb) => cb(errors.InternalError);
const delMeta = metadataBackend.deleteObject;
metadataBackend.deleteObject = (bucketName, objName, params, log, cb) => cb(errors.InternalError);
const partBody = Buffer.from('I am a part\n', 'utf8');
initiateRequest.headers['x-amz-meta-stuff'] =
'I am some user metadata';
Expand Down Expand Up @@ -1976,7 +1975,7 @@ describe('Multipart Upload API', () => {
'I am some user metadata');
assert.strictEqual(MD.uploadId, testUploadId);

ms.deleteObject = delMeta;
metadataBackend.deleteObject = delMeta;
const deleteRequest = {
bucketName,
namespace,
Expand All @@ -1998,8 +1997,8 @@ describe('Multipart Upload API', () => {
});

it('should complete an MPU and promote its MD if it has been created by a failed complete before', done => {
const delMeta = ms.deleteObject;
ms.deleteObject = (bucketName, objName, params, log, cb) => cb(errors.InternalError);
const delMeta = metadataBackend.deleteObject;
metadataBackend.deleteObject = (bucketName, objName, params, log, cb) => cb(errors.InternalError);
const partBody = Buffer.from('I am a part\n', 'utf8');
initiateRequest.headers['x-amz-meta-stuff'] =
'I am some user metadata';
Expand Down Expand Up @@ -2053,7 +2052,7 @@ describe('Multipart Upload API', () => {
assert.strictEqual(MD['x-amz-meta-stuff'],
'I am some user metadata');
assert.strictEqual(MD.uploadId, testUploadId);
ms.deleteObject = delMeta;
metadataBackend.deleteObject = delMeta;
assert.strictEqual(metadata.keyMaps.get(mpuBucket).size, 2);
completeMultipartUpload(authInfo,
completeRequest, log, err => {
Expand Down Expand Up @@ -2306,8 +2305,8 @@ describe('complete mpu with versioning', () => {

it('should complete an MPU and promote its MD if it has been created by a failed complete before' +
'without creating a new version', done => {
const delMeta = ms.deleteObject;
ms.deleteObject = (bucketName, objName, params, log, cb) => cb(errors.InternalError);
const delMeta = metadataBackend.deleteObject;
metadataBackend.deleteObject = (bucketName, objName, params, log, cb) => cb(errors.InternalError);
const partBody = Buffer.from('I am a part\n', 'utf8');
initiateRequest.headers['x-amz-meta-stuff'] =
'I am some user metadata';
Expand Down Expand Up @@ -2364,7 +2363,7 @@ describe('complete mpu with versioning', () => {
assert.strictEqual(MD['x-amz-meta-stuff'],
'I am some user metadata');
assert.strictEqual(MD.uploadId, testUploadId);
ms.deleteObject = delMeta;
metadataBackend.deleteObject = delMeta;
assert.strictEqual(metadata.keyMaps.get(mpuBucket).size, 2);
completeMultipartUpload(authInfo,
completeRequest, log, err => {
Expand Down

0 comments on commit a934fe1

Please sign in to comment.