diff --git a/src/backups.js b/src/backups.js index 3634b8d4f..53cc893de 100644 --- a/src/backups.js +++ b/src/backups.js @@ -685,23 +685,23 @@ function cleanupAppBackups(backupConfig, referencedAppBackups, callback) { if (referencedAppBackups.indexOf(backup.id) !== -1) return iteratorDone(); if ((now - backup.creationTime) < (backupConfig.retentionSecs * 1000)) return iteratorDone(); - debug('cleanup: removing %s', backup.id); + debug('cleanupAppBackups: removing %s', backup.id); - api(backupConfig.provider).removeMany(backupConfig, [ getBackupFilePath(backupConfig, backup.id) ], function (error) { + api(backupConfig.provider).remove(backupConfig, getBackupFilePath(backupConfig, backup.id), function (error) { if (error) { - debug('cleanup: error removing backup %j : %s', backup, error.message); + debug('cleanupAppBackups: error removing backup %j : %s', backup, error.message); iteratorDone(); } backupdb.del(backup.id, function (error) { - if (error) debug('cleanup: error removing from database', error); - else debug('cleanup: removed %s', backup.id); + if (error) debug('cleanupAppBackups: error removing from database', error); + else debug('cleanupAppBackups: removed %s', backup.id); iteratorDone(); }); }); }, function () { - debug('cleanup: done cleaning app backups'); + debug('cleanupAppBackups: done'); callback(); }); @@ -728,11 +728,11 @@ function cleanupBoxBackups(backupConfig, callback) { // keep the first valid backup if (i !== boxBackups.length) { - debug('cleanup: preserving box backup %j', boxBackups[i]); + debug('cleanupBoxBackups: preserving box backup %j', boxBackups[i]); referencedAppBackups = boxBackups[i].dependsOn; boxBackups.splice(i, 1); } else { - debug('cleanup: no box backup to preserve'); + debug('cleanupBoxBackups: no box backup to preserve'); } async.eachSeries(boxBackups, function iterator(backup, iteratorDone) { @@ -742,24 +742,26 @@ function cleanupBoxBackups(backupConfig, callback) { // have to be careful not to remove any backup currently being created if ((now - backup.creationTime) < (backupConfig.retentionSecs * 1000)) return iteratorDone(); - debug('cleanup: removing %s', backup.id); + debug('cleanupBoxBackups: removing %s', backup.id); - var backupIds = [].concat(backup.id, backup.dependsOn); + var filePaths = [].concat(backup.id, backup.dependsOn).map(getBackupFilePath.bind(null, backupConfig)); - api(backupConfig.provider).removeMany(backupConfig, backupIds, function (error) { + async.eachSeries(filePaths, api(backupConfig.provider).remove.bind(null, backupConfig), function (error) { if (error) { - debug('cleanup: error removing backup %j : %s', backup, error.message); + debug('cleanupBoxBackups: error removing backup %j : %s', backup, error.message); iteratorDone(); } backupdb.del(backup.id, function (error) { - if (error) debug('cleanup: error removing from database', error); - else debug('cleanup: removed %j', backupIds); + if (error) debug('cleanupBoxBackups: error removing from database', error); + else debug('cleanupBoxBackups: removed %j', filePaths); iteratorDone(); }); }); }, function () { + debug('cleanupBoxBackups: done'); + return callback(null, referencedAppBackups); }); }); @@ -779,7 +781,7 @@ function cleanupSnapshots(backupConfig, callback) { apps.get(appId, function (error /*, app */) { if (!error || error.reason !== AppsError.NOT_FOUND) return iteratorDone(); - api(backupConfig.provider).removeMany(backupConfig, [ `snapshot/app_${appId}` ], function (/* ignoredError */) { + api(backupConfig.provider).remove(backupConfig, getBackupFilePath(backupConfig, `snapshot/app_${appId}`), function (/* ignoredError */) { setSnapshotInfo(appId, null); iteratorDone(); @@ -804,13 +806,9 @@ function cleanup(callback) { cleanupBoxBackups(backupConfig, function (error, referencedAppBackups) { if (error) return callback(error); - debug('cleanup: done cleaning box backups'); - cleanupAppBackups(backupConfig, referencedAppBackups, function (error) { if (error) return callback(error); - debug('cleanup: done cleaning app backups'); - cleanupSnapshots(backupConfig, callback); }); }); diff --git a/src/storage/caas.js b/src/storage/caas.js index a01dcf8e0..ae7e3d69e 100644 --- a/src/storage/caas.js +++ b/src/storage/caas.js @@ -4,7 +4,7 @@ exports = module.exports = { upload: upload, download: download, copy: copy, - removeMany: removeMany, + remove: remove, backupDone: backupDone, @@ -12,6 +12,7 @@ exports = module.exports = { }; var assert = require('assert'), + async = require('async'), AWS = require('aws-sdk'), BackupsError = require('../backups.js').BackupsError, config = require('../config.js'), @@ -141,29 +142,51 @@ function copy(apiConfig, oldFilePath, newFilePath, callback) { }); } -function removeMany(apiConfig, filePaths, callback) { +function remove(apiConfig, pathPrefix, callback) { assert.strictEqual(typeof apiConfig, 'object'); - assert(Array.isArray(filePaths)); + assert.strictEqual(typeof pathPrefix, 'string'); assert.strictEqual(typeof callback, 'function'); getBackupCredentials(apiConfig, function (error, credentials) { if (error) return callback(error); - var params = { + var s3 = new AWS.S3(credentials); + var listParams = { Bucket: apiConfig.bucket, - Delete: { - Objects: [ ] // { Key } - } + Prefix: pathPrefix }; - filePaths.forEach(function (filePath) { - params.Delete.Objects.push({ Key: filePath }); - }); + async.forever(function listAndDelete(iteratorCallback) { + s3.listObjectsV2(listParams, function (error, listData) { + if (error) { + debug('remove: Failed to list %s. Not fatal.', error); + return iteratorCallback(error); + } - var s3 = new AWS.S3(credentials); - s3.deleteObjects(params, function (error, data) { - if (error) debug('Unable to remove %s. Not fatal.', params.Key, error); - else debug('removeMany: Deleted: %j Errors: %j', data.Deleted, data.Errors); + var deleteParams = { + Bucket: apiConfig.bucket, + Delete: { + Objects: listData.Contents.map(function (c) { return { Key: c.Key }; }) + } + }; + + s3.deleteObjects(deleteParams, function (error, deleteData) { + if (error) { + debug('remove: Unable to remove %s. Not fatal.', deleteParams.Key, error); + return iteratorCallback(error); + } + + debug('remove: Deleted: %j Errors: %j', deleteData.Deleted, deleteData.Errors); + + listParams.Marker = listData[listData.length - 1].Key; // NextMarker is returned only with delimiter + + if (deleteData.IsTruncated) return iteratorCallback(); + + iteratorCallback(new Error('Done')); + }); + }); + }, function (/*ignoredError*/) { + if (error.message === 'Done') return callback(null); callback(null); }); diff --git a/src/storage/filesystem.js b/src/storage/filesystem.js index 10d0a2d03..ac1f56370 100644 --- a/src/storage/filesystem.js +++ b/src/storage/filesystem.js @@ -5,7 +5,7 @@ exports = module.exports = { download: download, copy: copy, - removeMany: removeMany, + remove: remove, backupDone: backupDone, @@ -13,7 +13,6 @@ exports = module.exports = { }; var assert = require('assert'), - async = require('async'), BackupsError = require('../backups.js').BackupsError, config = require('../config.js'), debug = require('debug')('box:storage/filesystem'), @@ -95,20 +94,18 @@ function copy(apiConfig, oldFilePath, newFilePath, callback) { }); } -function removeMany(apiConfig, filePaths, callback) { +function remove(apiConfig, pathPrefix, callback) { assert.strictEqual(typeof apiConfig, 'object'); - assert(Array.isArray(filePaths)); + assert.strictEqual(typeof pathPrefix, 'string'); assert.strictEqual(typeof callback, 'function'); - async.eachSeries(filePaths, function (filePath, iteratorCallback) { - if (!safe.fs.unlinkSync(filePath)) { - debug('removeMany: Unable to remove %s : %s', filePath, safe.error.message); - } + shell.exec('remove', '/bin/rm', [ '-rf', pathPrefix ], { }, function (error) { + if (error) return callback(new BackupsError(BackupsError.EXTERNAL_ERROR, error.message)); - safe.fs.rmdirSync(path.dirname(filePath)); // try to cleanup empty directories + safe.fs.rmdirSync(path.dirname(pathPrefix)); // try to cleanup empty directories - iteratorCallback(); - }, callback); + callback(); + }); } function testConfig(apiConfig, callback) { diff --git a/src/storage/interface.js b/src/storage/interface.js index e907f8564..78a7af2f5 100644 --- a/src/storage/interface.js +++ b/src/storage/interface.js @@ -11,7 +11,7 @@ exports = module.exports = { download: download, copy: copy, - removeMany: removeMany, + remove: remove, backupDone: backupDone, @@ -51,9 +51,9 @@ function copy(apiConfig, oldFilePath, newFilePath, callback) { callback(new Error('not implemented')); } -function removeMany(apiConfig, filePaths, callback) { +function remove(apiConfig, pathPrefix, callback) { assert.strictEqual(typeof apiConfig, 'object'); - assert(Array.isArray(filePaths)); + assert.strictEqual(typeof pathPrefix, 'string'); assert.strictEqual(typeof callback, 'function'); // Result: none diff --git a/src/storage/noop.js b/src/storage/noop.js index 4b29793a6..c4d9a4b78 100644 --- a/src/storage/noop.js +++ b/src/storage/noop.js @@ -5,7 +5,7 @@ exports = module.exports = { download: download, copy: copy, - removeMany: removeMany, + remove: remove, backupDone: backupDone, @@ -47,12 +47,12 @@ function copy(apiConfig, oldFilePath, newFilePath, callback) { callback(); } -function removeMany(apiConfig, filePaths, callback) { +function remove(apiConfig, pathPrefix, callback) { assert.strictEqual(typeof apiConfig, 'object'); - assert(Array.isArray(filePaths)); + assert.strictEqual(typeof pathPrefix, 'string'); assert.strictEqual(typeof callback, 'function'); - debug('removeMany: %j', filePaths); + debug('remove: %s', pathPrefix); callback(); } diff --git a/src/storage/s3.js b/src/storage/s3.js index cef510b87..679b4f486 100644 --- a/src/storage/s3.js +++ b/src/storage/s3.js @@ -5,7 +5,7 @@ exports = module.exports = { download: download, copy: copy, - removeMany: removeMany, + remove: remove, backupDone: backupDone, @@ -17,6 +17,7 @@ exports = module.exports = { }; var assert = require('assert'), + async = require('async'), AWS = require('aws-sdk'), BackupsError = require('../backups.js').BackupsError, debug = require('debug')('box:storage/s3'), @@ -150,30 +151,49 @@ function copy(apiConfig, oldFilePath, newFilePath, callback) { }); } -function removeMany(apiConfig, filePaths, callback) { +function remove(apiConfig, pathPrefix, callback) { assert.strictEqual(typeof apiConfig, 'object'); - assert(Array.isArray(filePaths)); + assert.strictEqual(typeof pathPrefix, 'string'); assert.strictEqual(typeof callback, 'function'); getBackupCredentials(apiConfig, function (error, credentials) { if (error) return callback(error); - var params = { + var s3 = new AWS.S3(credentials); + var listParams = { Bucket: apiConfig.bucket, - Delete: { - Objects: [ ] // { Key } - } + Prefix: pathPrefix }; - filePaths.forEach(function (filePath) { - params.Delete.Objects.push({ Key: filePath }); - }); + async.forever(function listAndDelete(iteratorCallback) { + s3.listObjectsV2(listParams, function (error, listData) { + if (error) { + debug('remove: Failed to list %s. Not fatal.', error); + return iteratorCallback(error); + } - var s3 = new AWS.S3(credentials); - s3.deleteObjects(params, function (error, data) { - if (error) debug('removeMany: Unable to remove %s. Not fatal.', params.Key, error); - else debug('removeMany: Deleted: %j Errors: %j', data.Deleted, data.Errors); + var deleteParams = { + Bucket: apiConfig.bucket, + Delete: { + Objects: listData.Contents.map(function (c) { return { Key: c.Key }; }) + } + }; + s3.deleteObjects(deleteParams, function (error, deleteData) { + if (error) { + debug('remove: Unable to remove %s. Not fatal.', deleteParams.Key, error); + return iteratorCallback(error); + } + debug(': Deleted: %j Errors: %j', deleteData.Deleted, deleteData.Errors); + + listParams.Marker = listData.Contents[listData.Contents.length - 1].Key; // NextMarker is returned only with delimiter + + if (deleteData.IsTruncated) return iteratorCallback(); + + iteratorCallback(new Error('Done')); + }); + }); + }, function (/*ignoredError*/) { callback(null); }); }); diff --git a/src/test/storage-test.js b/src/test/storage-test.js index 24b10ac44..7b1a8c00e 100644 --- a/src/test/storage-test.js +++ b/src/test/storage-test.js @@ -184,7 +184,7 @@ describe('Storage', function () { it('can remove backup', function (done) { // will be verified with next test trying to download the removed one - filesystem.removeMany(gBackupConfig, [ gBackupId_1 ], done); + filesystem.remove(gBackupConfig, gBackupId_1, done); }); it('cannot download deleted backup', function (done) { @@ -221,7 +221,7 @@ describe('Storage', function () { }); it('can remove backup copy', function (done) { - filesystem.removeMany(gBackupConfig, [ gBackupId_2 ], done); + filesystem.remove(gBackupConfig, gBackupId_2, done); }); }); @@ -309,7 +309,7 @@ describe('Storage', function () { it('can remove backup', function (done) { // will be verified with next test trying to download the removed one - s3.removeMany(gBackupConfig, [ gBackupId_1 ], done); + s3.remove(gBackupConfig, gBackupId_1, done); }); it('cannot download deleted backup', function (done) { @@ -346,7 +346,7 @@ describe('Storage', function () { }); it('can remove backup copy', function (done) { - s3.removeMany(gBackupConfig, [ gBackupId_2 ], done); + s3.remove(gBackupConfig, gBackupId_2, done); }); }); });