diff --git a/lib/api/apiUtils/object/abortMultipartUpload.js b/lib/api/apiUtils/object/abortMultipartUpload.js index c5928b7229..4926a65733 100644 --- a/lib/api/apiUtils/object/abortMultipartUpload.js +++ b/lib/api/apiUtils/object/abortMultipartUpload.js @@ -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), @@ -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(); }); diff --git a/tests/unit/api/multipartUpload.js b/tests/unit/api/multipartUpload.js index e132cc597d..4d4d37e0d1 100644 --- a/tests/unit/api/multipartUpload.js +++ b/tests/unit/api/multipartUpload.js @@ -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(); @@ -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'; @@ -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, @@ -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'; @@ -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 => { @@ -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'; @@ -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 => {