Skip to content

Commit 9309adc

Browse files
committed
Exporter cleanup & tests
refs #6301 - change knex getter def to be configurable, else it is not testable - remove exportPath and lang from config - neither are used - add client_trusted_domains to tables which shouldn't be exported as there are no clients in the export - change export signature to be an object with `doExport` function consistent with import & easier to test - cleanup export code so it is clearer, easier to read & to test: - use mapSeries instead of sequence - use Promise.props instead of Promise.join - split functionality into smaller functions - add test coverage
1 parent 411dd47 commit 9309adc

File tree

9 files changed

+208
-48
lines changed

9 files changed

+208
-48
lines changed

core/server/api/db.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// API for DB operations
33
var _ = require('lodash'),
44
Promise = require('bluebird'),
5-
dataExport = require('../data/export'),
5+
exporter = require('../data/export'),
66
importer = require('../data/importer'),
77
backupDatabase = require('../data/migration').backupDatabase,
88
models = require('../models'),
@@ -38,7 +38,7 @@ db = {
3838

3939
// Export data, otherwise send error 500
4040
function exportContent() {
41-
return dataExport().then(function (exportedData) {
41+
return exporter.doExport().then(function (exportedData) {
4242
return {db: [exportedData]};
4343
}).catch(function (error) {
4444
return Promise.reject(new errors.InternalServerError(error.message || error));

core/server/api/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ var _ = require('lodash'),
2121
slugs = require('./slugs'),
2222
authentication = require('./authentication'),
2323
uploads = require('./upload'),
24-
dataExport = require('../data/export'),
24+
exporter = require('../data/export'),
2525

2626
http,
2727
addHeaders,
@@ -138,7 +138,7 @@ locationHeader = function locationHeader(req, result) {
138138
* @return {string}
139139
*/
140140
contentDispositionHeader = function contentDispositionHeader() {
141-
return dataExport.fileName().then(function then(filename) {
141+
return exporter.fileName().then(function then(filename) {
142142
return 'Attachment; filename="' + filename + '"';
143143
});
144144
};

core/server/config/index.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,6 @@ ConfigManager.prototype.set = function (config) {
167167

168168
adminViews: path.join(corePath, '/server/views/'),
169169
helperTemplates: path.join(corePath, '/server/helpers/tpl/'),
170-
exportPath: path.join(corePath, '/server/data/export/'),
171-
lang: path.join(corePath, '/shared/lang/'),
172170

173171
availableThemes: this._config.paths.availableThemes || {},
174172
availableApps: this._config.paths.availableApps || {},

core/server/data/db/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ var connection;
22

33
Object.defineProperty(exports, 'knex', {
44
enumerable: true,
5-
configurable: false,
5+
configurable: true,
66
get: function get() {
77
connection = connection || require('./connection');
88
return connection;

core/server/data/export/index.js

Lines changed: 55 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,22 @@ var _ = require('lodash'),
88
settings = require('../../api/settings'),
99
i18n = require('../../i18n'),
1010

11-
excludedTables = ['accesstokens', 'refreshtokens', 'clients'],
12-
exporter,
11+
excludedTables = ['accesstokens', 'refreshtokens', 'clients', 'client_trusted_domains'],
12+
modelOptions = {context: {internal: true}},
13+
14+
// private
15+
getVersionAndTables,
16+
exportTable,
17+
18+
// public
19+
doExport,
1320
exportFileName;
1421

15-
exportFileName = function () {
22+
exportFileName = function exportFileName() {
1623
var datetime = (new Date()).toJSON().substring(0, 10),
1724
title = '';
1825

19-
return settings.read({key: 'title', context: {internal: true}}).then(function (result) {
26+
return settings.read(_.extend({}, {key: 'title'}, modelOptions)).then(function (result) {
2027
if (result) {
2128
title = serverUtils.safeString(result.settings[0].value) + '.';
2229
}
@@ -27,37 +34,51 @@ exportFileName = function () {
2734
});
2835
};
2936

30-
exporter = function () {
31-
return Promise.join(versioning.getDatabaseVersion(), commands.getTables()).then(function (results) {
32-
var version = results[0],
33-
tables = results[1],
34-
selectOps = _.map(tables, function (name) {
35-
if (excludedTables.indexOf(name) < 0) {
36-
return db.knex(name).select();
37-
}
38-
});
39-
40-
return Promise.all(selectOps).then(function (tableData) {
41-
var exportData = {
42-
meta: {
43-
exported_on: new Date().getTime(),
44-
version: version
45-
},
46-
data: {
47-
// Filled below
48-
}
49-
};
50-
51-
_.each(tables, function (name, i) {
52-
exportData.data[name] = tableData[i];
53-
});
54-
55-
return exportData;
56-
}).catch(function (err) {
57-
errors.logAndThrowError(err, i18n.t('errors.data.export.errorExportingData'), '');
37+
getVersionAndTables = function getVersionAndTables() {
38+
var props = {
39+
version: versioning.getDatabaseVersion(),
40+
tables: commands.getTables()
41+
};
42+
43+
return Promise.props(props);
44+
};
45+
46+
exportTable = function exportTable(tableName) {
47+
if (excludedTables.indexOf(tableName) < 0) {
48+
return db.knex(tableName).select();
49+
}
50+
};
51+
52+
doExport = function doExport() {
53+
var tables, version;
54+
55+
return getVersionAndTables().then(function exportAllTables(result) {
56+
tables = result.tables;
57+
version = result.version;
58+
59+
return Promise.mapSeries(tables, exportTable);
60+
}).then(function formatData(tableData) {
61+
var exportData = {
62+
meta: {
63+
exported_on: new Date().getTime(),
64+
version: version
65+
},
66+
data: {
67+
// Filled below
68+
}
69+
};
70+
71+
_.each(tables, function (name, i) {
72+
exportData.data[name] = tableData[i];
5873
});
74+
75+
return exportData;
76+
}).catch(function (err) {
77+
errors.logAndThrowError(err, i18n.t('errors.data.export.errorExportingData'), '');
5978
});
6079
};
6180

62-
module.exports = exporter;
63-
module.exports.fileName = exportFileName;
81+
module.exports = {
82+
doExport: doExport,
83+
fileName: exportFileName
84+
};

core/server/data/migration/index.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ var _ = require('lodash'),
88
schema = require('../schema').tables,
99
commands = require('../schema').commands,
1010
versioning = require('../schema').versioning,
11-
dataExport = require('../export'),
11+
exporter = require('../export'),
1212
config = require('../../config'),
1313
errors = require('../../errors'),
1414
models = require('../../models'),
@@ -46,9 +46,9 @@ populateDefaultSettings = function populateDefaultSettings() {
4646

4747
backupDatabase = function backupDatabase() {
4848
logInfo('Creating database backup');
49-
return dataExport().then(function (exportedData) {
49+
return exporter.doExport().then(function (exportedData) {
5050
// Save the exported data to the file system for download
51-
return dataExport.fileName().then(function (fileName) {
51+
return exporter.fileName().then(function (fileName) {
5252
fileName = path.resolve(config.paths.contentPath + '/data/' + fileName);
5353

5454
return Promise.promisify(fs.writeFile)(fileName, JSON.stringify(exportedData)).then(function () {

core/test/integration/export_spec.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ var testUtils = require('../utils/index'),
77

88
// Stuff we are testing
99
versioning = require('../../server/data/schema').versioning,
10-
exporter = require('../../server/data/export/index'),
10+
exporter = require('../../server/data/export'),
1111

1212
DEF_DB_VERSION = versioning.getDefaultDatabaseVersion(),
1313
sandbox = sinon.sandbox.create();
@@ -28,7 +28,7 @@ describe('Exporter', function () {
2828
return Promise.resolve(DEF_DB_VERSION);
2929
});
3030

31-
exporter().then(function (exportData) {
31+
exporter.doExport().then(function (exportData) {
3232
var tables = ['posts', 'users', 'roles', 'roles_users', 'permissions', 'permissions_roles',
3333
'permissions_users', 'settings', 'tags', 'posts_tags'],
3434
dbVersionSetting;

core/test/unit/config_spec.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,6 @@ describe('Config', function () {
7272
'imagesRelPath',
7373
'adminViews',
7474
'helperTemplates',
75-
'exportPath',
76-
'lang',
7775
'availableThemes',
7876
'availableApps',
7977
'clientAssets'

core/test/unit/exporter_spec.js

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*globals describe, afterEach, beforeEach, it*/
2+
var should = require('should'),
3+
sinon = require('sinon'),
4+
Promise = require('bluebird'),
5+
6+
// Stuff we're testing
7+
db = require('../../server/data/db'),
8+
errors = require('../../server/errors'),
9+
exporter = require('../../server/data/export'),
10+
schema = require('../../server/data/schema'),
11+
settings = require('../../server/api/settings'),
12+
13+
schemaTables = Object.keys(schema.tables),
14+
15+
sandbox = sinon.sandbox.create();
16+
17+
require('should-sinon');
18+
19+
describe('Exporter', function () {
20+
var versionStub, tablesStub, queryMock, knexMock, knexStub;
21+
22+
afterEach(function () {
23+
sandbox.restore();
24+
knexStub.restore();
25+
});
26+
27+
describe('doExport', function () {
28+
beforeEach(function () {
29+
versionStub = sandbox.stub(schema.versioning, 'getDatabaseVersion').returns(new Promise.resolve('004'));
30+
tablesStub = sandbox.stub(schema.commands, 'getTables').returns(schemaTables);
31+
32+
queryMock = {
33+
select: sandbox.stub()
34+
};
35+
36+
knexMock = sandbox.stub().returns(queryMock);
37+
38+
// this MUST use sinon, not sandbox, see sinonjs/sinon#781
39+
knexStub = sinon.stub(db, 'knex', {get: function () { return knexMock; }});
40+
});
41+
42+
it('should try to export all the correct tables', function (done) {
43+
// Setup for success
44+
queryMock.select.returns(new Promise.resolve({}));
45+
46+
// Execute
47+
exporter.doExport().then(function (exportData) {
48+
// No tables, less the number of excluded tables
49+
var expectedCallCount = schemaTables.length - 4;
50+
51+
should.exist(exportData);
52+
53+
versionStub.should.be.calledOnce();
54+
tablesStub.should.be.calledOnce();
55+
knexStub.get.should.be.called();
56+
knexMock.should.be.called();
57+
queryMock.select.should.be.called();
58+
59+
knexMock.should.have.callCount(expectedCallCount);
60+
queryMock.select.should.have.callCount(expectedCallCount);
61+
62+
knexMock.getCall(0).should.be.calledWith('posts');
63+
knexMock.getCall(1).should.be.calledWith('users');
64+
knexMock.getCall(2).should.be.calledWith('roles');
65+
knexMock.getCall(3).should.be.calledWith('roles_users');
66+
knexMock.getCall(4).should.be.calledWith('permissions');
67+
knexMock.getCall(5).should.be.calledWith('permissions_users');
68+
knexMock.getCall(6).should.be.calledWith('permissions_roles');
69+
knexMock.getCall(7).should.be.calledWith('permissions_apps');
70+
knexMock.getCall(8).should.be.calledWith('settings');
71+
knexMock.getCall(9).should.be.calledWith('tags');
72+
knexMock.getCall(10).should.be.calledWith('posts_tags');
73+
knexMock.getCall(11).should.be.calledWith('apps');
74+
knexMock.getCall(12).should.be.calledWith('app_settings');
75+
knexMock.getCall(13).should.be.calledWith('app_fields');
76+
77+
knexMock.should.not.be.calledWith('clients');
78+
knexMock.should.not.be.calledWith('client_trusted_domains');
79+
knexMock.should.not.be.calledWith('refreshtokens');
80+
knexMock.should.not.be.calledWith('accesstokens');
81+
82+
done();
83+
}).catch(done);
84+
});
85+
86+
it('should catch and log any errors', function (done) {
87+
// Setup for failure
88+
var errorStub = sandbox.stub(errors, 'logAndThrowError');
89+
queryMock.select.returns(new Promise.reject({}));
90+
91+
// Execute
92+
exporter.doExport().then(function (exportData) {
93+
should.not.exist(exportData);
94+
errorStub.should.be.calledOnce();
95+
done();
96+
}).catch(done);
97+
});
98+
});
99+
100+
describe('exportFileName', function () {
101+
it('should return a correctly structured filename', function (done) {
102+
var settingsStub = sandbox.stub(settings, 'read').returns(
103+
new Promise.resolve({settings: [{value: 'testblog'}]})
104+
);
105+
106+
exporter.fileName().then(function (result) {
107+
should.exist(result);
108+
settingsStub.should.be.calledOnce();
109+
result.should.match(/^testblog\.ghost\.[0-9]{4}-[0-9]{2}-[0-9]{2}\.json$/);
110+
111+
done();
112+
}).catch(done);
113+
});
114+
115+
it('should return a correctly structured filename if settings is empty', function (done) {
116+
var settingsStub = sandbox.stub(settings, 'read').returns(
117+
new Promise.resolve()
118+
);
119+
120+
exporter.fileName().then(function (result) {
121+
should.exist(result);
122+
settingsStub.should.be.calledOnce();
123+
result.should.match(/^ghost\.[0-9]{4}-[0-9]{2}-[0-9]{2}\.json$/);
124+
125+
done();
126+
}).catch(done);
127+
});
128+
129+
it('should return a correctly structured filename if settings errors', function (done) {
130+
var settingsStub = sandbox.stub(settings, 'read').returns(
131+
new Promise.reject()
132+
);
133+
134+
exporter.fileName().then(function (result) {
135+
should.exist(result);
136+
settingsStub.should.be.calledOnce();
137+
result.should.match(/^ghost\.[0-9]{4}-[0-9]{2}-[0-9]{2}\.json$/);
138+
139+
done();
140+
}).catch(done);
141+
});
142+
});
143+
});

0 commit comments

Comments
 (0)