From 16a65fb1855f165f5f4f8e81bdf252ca4dd39d79 Mon Sep 17 00:00:00 2001 From: Girish Ramakrishnan Date: Sun, 3 Apr 2016 22:29:16 -0700 Subject: [PATCH] drop configJson The initial idea was to store exactly where the backups are stored. But this only causes problems for migrations where the bucket might change and clones where the prefix (box.id) changes. Thus, it's best to leave the url creation to the caas side. (That has to be done in another change) --- .../20160404052453-backups-drop-configJson.js | 17 ++++++++ migrations/schema.sql | 1 - src/backupdb.js | 9 ++--- src/backups.js | 39 +++++++------------ src/storage/caas.js | 11 +++--- 5 files changed, 39 insertions(+), 38 deletions(-) create mode 100644 migrations/20160404052453-backups-drop-configJson.js diff --git a/migrations/20160404052453-backups-drop-configJson.js b/migrations/20160404052453-backups-drop-configJson.js new file mode 100644 index 000000000..3741354a5 --- /dev/null +++ b/migrations/20160404052453-backups-drop-configJson.js @@ -0,0 +1,17 @@ +var dbm = dbm || require('db-migrate'); +var type = dbm.dataType; + +exports.up = function(db, callback) { + db.runSql('ALTER TABLE backups DROP COLUMN configJson', function (error) { + if (error) console.error(error); + callback(error); + }); +}; + +exports.down = function(db, callback) { + db.runSql('ALTER TABLE backups ADD COLUMN configJson TEXT', function (error) { + if (error) console.error(error); + callback(error); + }); +}; + diff --git a/migrations/schema.sql b/migrations/schema.sql index 4ef1e5c63..0edd8e662 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -105,6 +105,5 @@ CREATE TABLE IF NOT EXISTS backups( type VARCHAR(16) NOT NULL, /* 'box' or 'app' */ dependsOn VARCHAR(4096), /* comma separate list of objects this backup depends on */ state VARCHAR(16) NOT NULL, - configJson TEXT, /* configuration - bucket, prefix, key, provider */ PRIMARY KEY (filename)); diff --git a/src/backupdb.js b/src/backupdb.js index 83eb20a6c..f66eda006 100644 --- a/src/backupdb.js +++ b/src/backupdb.js @@ -6,7 +6,7 @@ var assert = require('assert'), safe = require('safetydance'), util = require('util'); -var BACKUPS_FIELDS = [ 'filename', 'creationTime', 'version', 'type', 'dependsOn', 'state', 'configJson' ]; +var BACKUPS_FIELDS = [ 'filename', 'creationTime', 'version', 'type', 'dependsOn', 'state', ]; exports = module.exports = { add: add, @@ -27,8 +27,6 @@ function postProcess(result) { assert.strictEqual(typeof result, 'object'); result.dependsOn = result.dependsOn ? result.dependsOn.split(',') : [ ]; - result.config = safe.JSON.parse(result.configJson); - delete result.configJson; } function getPaged(page, perPage, callback) { @@ -83,13 +81,12 @@ function add(backup, callback) { assert.strictEqual(typeof backup.version, 'string'); assert(backup.type === exports.BACKUP_TYPE_APP || backup.type === exports.BACKUP_TYPE_BOX); assert(util.isArray(backup.dependsOn)); - assert(backup.config && typeof backup.config === 'object'); assert.strictEqual(typeof callback, 'function'); var creationTime = backup.creationTime || new Date(); // allow tests to set the time - database.query('INSERT INTO backups (filename, version, type, creationTime, state, dependsOn, configJson) VALUES (?, ?, ?, ?, ?, ?, ?)', - [ backup.filename, backup.version, backup.type, creationTime, exports.BACKUP_STATE_NORMAL, backup.dependsOn.join(','), JSON.stringify(backup.config) ], + database.query('INSERT INTO backups (filename, version, type, creationTime, state, dependsOn) VALUES (?, ?, ?, ?, ?, ?)', + [ backup.filename, backup.version, backup.type, creationTime, exports.BACKUP_STATE_NORMAL, backup.dependsOn.join(',') ], function (error) { if (error && error.code === 'ER_DUP_ENTRY') return callback(new DatabaseError(DatabaseError.ALREADY_EXISTS)); if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); diff --git a/src/backups.js b/src/backups.js index b2ec9f0aa..81cff6863 100644 --- a/src/backups.js +++ b/src/backups.js @@ -20,8 +20,7 @@ var assert = require('assert'), debug = require('debug')('box:backups'), s3 = require('./storage/s3.js'), settings = require('./settings.js'), - util = require('util'), - _ = require('underscore'); + util = require('util'); function BackupsError(reason, errorOrMessage) { assert.strictEqual(typeof reason, 'string'); @@ -67,11 +66,6 @@ function getPaged(page, perPage, callback) { }); } -// this should probably be provider specific -function cleanBackupConfig(backupConfig) { - return _.pick(backupConfig, 'provider', 'key', 'bucket', 'prefix', 'region'); -} - function getByAppIdPaged(page, perPage, appId, callback) { assert(typeof page === 'number' && page > 0); assert(typeof perPage === 'number' && perPage > 0); @@ -109,7 +103,7 @@ function getBackupUrl(appBackupIds, callback) { debug('getBackupUrl: id:%s url:%s sessionToken:%s backupKey:%s', obj.id, obj.url, obj.sessionToken, obj.backupKey); backupdb.add({ filename: filename, creationTime: now, version: config.version(), type: backupdb.BACKUP_TYPE_BOX, - dependsOn: appBackupIds, config: cleanBackupConfig(backupConfig) }, function (error) { + dependsOn: appBackupIds }, function (error) { if (error) return callback(new BackupsError(BackupsError.INTERNAL_ERROR, error)); callback(null, obj); @@ -146,7 +140,7 @@ function getAppBackupUrl(app, callback) { debug('getAppBackupUrl: %j', obj); backupdb.add({ filename: dataFilename, creationTime: now, version: app.manifest.version, type: backupdb.BACKUP_TYPE_APP, - dependsOn: [ ], config: cleanBackupConfig(backupConfig) }, function (error) { + dependsOn: [ ] }, function (error) { if (error) return callback(new BackupsError(BackupsError.INTERNAL_ERROR, error)); callback(null, obj); @@ -161,27 +155,22 @@ function getRestoreUrl(backupId, callback) { assert.strictEqual(typeof backupId, 'string'); assert.strictEqual(typeof callback, 'function'); - settings.getBackupConfig(function (error, apiConfig) { + settings.getBackupConfig(function (error, backupConfig) { if (error) return callback(new BackupsError(BackupsError.INTERNAL_ERROR, error)); - backupdb.get(backupId, function (error, result) { - if (error) return callback(new BackupsError(BackupsError.INTERNAL_ERROR, error)); + api(backupConfig.provider).getSignedDownloadUrl(backupConfig, backupId, function (error, result) { + if (error) return callback(error); - var backupConfig = result.config; - api(backupConfig.provider).getSignedDownloadUrl(apiConfig, backupConfig, backupId, function (error, result) { - if (error) return callback(error); + var obj = { + id: backupId, + url: result.url, + sessionToken: result.sessionToken, + backupKey: backupConfig.key + }; - var obj = { - id: backupId, - url: result.url, - sessionToken: result.sessionToken, - backupKey: backupConfig.key - }; + debug('getRestoreUrl: id:%s url:%s sessionToken:%s backupKey:%s', obj.id, obj.url, obj.sessionToken, obj.backupKey); - debug('getRestoreUrl: id:%s url:%s sessionToken:%s backupKey:%s', obj.id, obj.url, obj.sessionToken, obj.backupKey); - - callback(null, obj); - }); + callback(null, obj); }); }); } diff --git a/src/storage/caas.js b/src/storage/caas.js index fd0eddec7..458ca3fb7 100644 --- a/src/storage/caas.js +++ b/src/storage/caas.js @@ -79,23 +79,22 @@ function getSignedUploadUrl(apiConfig, filename, callback) { }); } -function getSignedDownloadUrl(apiConfig, info, filename, callback) { +function getSignedDownloadUrl(apiConfig, filename, callback) { assert.strictEqual(typeof apiConfig, 'object'); - assert.strictEqual(typeof info, 'object'); assert.strictEqual(typeof filename, 'string'); assert.strictEqual(typeof callback, 'function'); - if (!info.bucket || !info.prefix || !info.region) return new Error('Invalid configuration'); // prevent error in s3 + if (!apiConfig.bucket || !apiConfig.prefix) return new Error('Invalid configuration'); // prevent error in s3 getBackupCredentials(apiConfig, function (error, credentials) { if (error) return callback(error); - credentials.region = info.region; // use same region as where we uploaded + credentials.region = apiConfig.region; // use same region as where we uploaded var s3 = new AWS.S3(credentials); var params = { - Bucket: info.bucket, - Key: info.prefix + '/' + filename, + Bucket: apiConfig.bucket, + Key: apiConfig.prefix + '/' + filename, Expires: 60 * 30 /* 30 minutes */ };