diff --git a/.eslintrc b/.eslintrc index ecfcaa749a42..5add26ae4396 100644 --- a/.eslintrc +++ b/.eslintrc @@ -8,6 +8,8 @@ "ecmaVersion": "2017" }, "rules": { + "no-param-reassign": "error", + "no-return-await": "error", "max-nested-callbacks": "off", "new-cap": [ "error", diff --git a/lib/db/feature-toggle-store.js b/lib/db/feature-toggle-store.js index 932025ccf8d2..7c265e34cde3 100644 --- a/lib/db/feature-toggle-store.js +++ b/lib/db/feature-toggle-store.js @@ -100,7 +100,6 @@ class FeatureToggleStore { name: data.name, description: data.description, enabled: data.enabled ? 1 : 0, - archived: data.archived ? 1 : 0, strategies: JSON.stringify(data.strategies), variants: data.variants ? JSON.stringify(data.variants) : null, created_at: data.createdAt, // eslint-disable-line @@ -143,15 +142,13 @@ class FeatureToggleStore { } _importFeature(data) { - data = this.eventDataToRow(data); - return this.db - .raw(`? ON CONFLICT (name) DO ?`, [ - this.db(TABLE).insert(data), - this.db - .queryBuilder() - .update(data) - .update('archived', 0), - ]) + const rowData = this.eventDataToRow(data); + return this.db(TABLE) + .where({ name: rowData.name }) + .update(rowData) + .then(result => + result === 0 ? this.db(TABLE).insert(rowData) : result + ) .catch(err => logger.error('Could not import feature, error was: ', err) ); diff --git a/lib/db/strategy-store.js b/lib/db/strategy-store.js index 575403cfaf13..7a9ce825fe86 100644 --- a/lib/db/strategy-store.js +++ b/lib/db/strategy-store.js @@ -38,6 +38,15 @@ class StrategyStore { .map(this.rowToStrategy); } + getEditableStrategies() { + return this.db + .select(STRATEGY_COLUMNS) + .from(TABLE) + .where({ built_in: 0 }) // eslint-disable-line + .orderBy('name', 'asc') + .map(this.rowToEditableStrategy); + } + getStrategy(name) { return this.db .first(STRATEGY_COLUMNS) @@ -58,6 +67,17 @@ class StrategyStore { }; } + rowToEditableStrategy(row) { + if (!row) { + throw new NotFoundError('No strategy found'); + } + return { + name: row.name, + description: row.description, + parameters: row.parameters, + }; + } + eventDataToRow(data) { return { name: data.name, @@ -93,12 +113,13 @@ class StrategyStore { } _importStrategy(data) { - data = this.eventDataToRow(data); - return this.db - .raw(`? ON CONFLICT (name) DO ?`, [ - this.db(TABLE).insert(data), - this.db.queryBuilder().update(data).where(`${TABLE}.built_in`, 0), // eslint-disable-line - ]) + const rowData = this.eventDataToRow(data); + return this.db(TABLE) + .where({ name: rowData.name, built_in: 0 }) // eslint-disable-line + .update(rowData) + .then(result => + result === 0 ? this.db(TABLE).insert(rowData) : result + ) .catch(err => logger.error('Could not import strategy, error was: ', err) ); diff --git a/lib/routes/admin-api/state.js b/lib/routes/admin-api/state.js index e668ee397479..15b384bbd1f6 100644 --- a/lib/routes/admin-api/state.js +++ b/lib/routes/admin-api/state.js @@ -9,7 +9,7 @@ const moment = require('moment'); const multer = require('multer'); const upload = multer({ limits: { fileSize: 5242880 } }); -class ImportController extends Controller { +class StateController extends Controller { constructor(config) { super(config); this.fileupload('/import', upload.single('file'), this.import, ADMIN); @@ -46,27 +46,29 @@ class ImportController extends Controller { async export(req, res) { const { format } = req.query; - let strategies = 'strategies' in req.query; - let featureToggles = 'features' in req.query; + const downloadFile = Boolean(req.query.download); + let includeStrategies = Boolean(req.query.strategies); + let includeFeatureToggles = Boolean(req.query.featureToggles); - if (!strategies && !featureToggles) { - strategies = true; - featureToggles = true; + // if neither is passed as query argument, export both + if (!includeStrategies && !includeFeatureToggles) { + includeStrategies = true; + includeFeatureToggles = true; } try { const data = await this.config.stateService.export({ - strategies, - featureToggles, + includeStrategies, + includeFeatureToggles, }); const timestamp = moment().format('YYYY-MM-DD_HH-mm-ss'); if (format === 'yaml') { - if ('download' in req.query) { + if (downloadFile) { res.attachment(`export-${timestamp}.yml`); } res.type('yaml').send(YAML.safeDump(data)); } else { - if ('download' in req.query) { + if (downloadFile) { res.attachment(`export-${timestamp}.json`); } res.json(data); @@ -77,4 +79,4 @@ class ImportController extends Controller { } } -module.exports = ImportController; +module.exports = StateController; diff --git a/lib/server-impl.js b/lib/server-impl.js index a446f4562e89..7eeda2eba577 100644 --- a/lib/server-impl.js +++ b/lib/server-impl.js @@ -40,7 +40,7 @@ async function createApp(options) { config.stateService = stateService; if (config.importFile) { await stateService.importFile({ - importFile: config.importFile, + file: config.importFile, dropBeforeImport: config.dropBeforeImport, userName: 'importer', }); @@ -50,7 +50,7 @@ async function createApp(options) { logger.info(`Unleash started on port ${server.address().port}`) ); - return await new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { server.on('listening', () => resolve({ app, diff --git a/lib/state-service.js b/lib/state-service.js index 0c9f7a60ee7a..cda08e0b01f0 100644 --- a/lib/state-service.js +++ b/lib/state-service.js @@ -6,7 +6,7 @@ const mime = require('mime'); const { featureShema } = require('./routes/admin-api/feature-schema'); const strategySchema = require('./routes/admin-api/strategy-schema'); const getLogger = require('./logger'); -const yaml = require('js-yaml'); +const YAML = require('js-yaml'); const { FEATURE_IMPORT, DROP_FEATURES, @@ -24,42 +24,39 @@ const dataSchema = joi.object().keys({ strategies: joi .array() .optional() - .items(strategySchema), + .items(strategySchema.forbiddenKeys('editable')), }); +function readFile(file) { + return new Promise((resolve, reject) => + fs.readFile(file, (err, v) => (err ? reject(err) : resolve(v))) + ); +} + +function parseFile(file, data) { + return mime.lookup(file) === 'text/yaml' + ? YAML.safeLoad(data) + : JSON.parse(data); +} + class StateService { constructor(config) { this.config = config; } - async importFile({ importFile, dropBeforeImport, userName }) { - let data = await new Promise((resolve, reject) => - fs.readFile(importFile, (err, v) => - err ? reject(err) : resolve(v) - ) - ); - if (mime.lookup(importFile) === 'text/yaml') { - data = yaml.safeLoad(data); - } - - await this.import({ - data, - dropBeforeImport, - userName, - }); + async importFile({ file, dropBeforeImport, userName }) { + return readFile(file) + .then(data => parseFile(file, data)) + .then(data => this.import({ data, userName, dropBeforeImport })); } async import({ data, userName, dropBeforeImport }) { const { eventStore } = this.config.stores; - if (typeof data === 'string') { - data = JSON.parse(data); - } - - data = await joi.validate(data, dataSchema); + const importData = await joi.validate(data, dataSchema); - if (data.features) { - logger.info(`Importing ${data.features.length} features`); + if (importData.features) { + logger.info(`Importing ${importData.features.length} features`); if (dropBeforeImport) { logger.info(`Dropping existing features`); await eventStore.store({ @@ -68,17 +65,19 @@ class StateService { data: { name: 'all-features' }, }); } - for (const feature of data.features) { - await eventStore.store({ - type: FEATURE_IMPORT, - createdBy: userName, - data: feature, - }); - } + await Promise.all( + importData.features.map(feature => + eventStore.store({ + type: FEATURE_IMPORT, + createdBy: userName, + data: feature, + }) + ) + ); } - if (data.strategies) { - logger.info(`Importing ${data.strategies.length} strategies`); + if (importData.strategies) { + logger.info(`Importing ${importData.strategies.length} strategies`); if (dropBeforeImport) { logger.info(`Dropping existing strategies`); await eventStore.store({ @@ -87,35 +86,29 @@ class StateService { data: { name: 'all-strategies' }, }); } - for (const strategy of data.strategies) { - await eventStore.store({ - type: STRATEGY_IMPORT, - createdBy: userName, - data: strategy, - }); - } + await Promise.all( + importData.strategies.map(strategy => + eventStore.store({ + type: STRATEGY_IMPORT, + createdBy: userName, + data: strategy, + }) + ) + ); } } - async export({ strategies, featureToggles }) { + async export({ includeFeatureToggles = true, includeStrategies = true }) { const { featureToggleStore, strategyStore } = this.config.stores; - const result = {}; - - if (featureToggles) { - result.features = await featureToggleStore.getFeatures(); - } - - if (strategies) { - result.strategies = (await strategyStore.getStrategies()) - .filter(strat => strat.editable) - .map(strat => { - strat = Object.assign({}, strat); - delete strat.editable; - return strat; - }); - } - return result; + return Promise.all([ + includeFeatureToggles + ? featureToggleStore.getFeatures() + : Promise.resolve(), + includeStrategies + ? strategyStore.getEditableStrategies() + : Promise.resolve(), + ]).then(([features, strategies]) => ({ features, strategies })); } } diff --git a/lib/state-service.test.js b/lib/state-service.test.js index 3f1c04e15912..26e3e68cc05e 100644 --- a/lib/state-service.test.js +++ b/lib/state-service.test.js @@ -130,7 +130,7 @@ test('should export featureToggles', async t => { stores.featureToggleStore.addFeature({ name: 'a-feature' }); - const data = await stateService.export({ featureToggles: true }); + const data = await stateService.export({ includeFeatureToggles: true }); t.is(data.features.length, 1); t.is(data.features[0].name, 'a-feature'); @@ -141,7 +141,7 @@ test('should export strategies', async t => { stores.strategyStore.addStrategy({ name: 'a-strategy', editable: true }); - const data = await stateService.export({ strategies: true }); + const data = await stateService.export({ includeStrategies: true }); t.is(data.strategies.length, 1); t.is(data.strategies[0].name, 'a-strategy'); diff --git a/test/e2e/api/admin/state.e2e.test.js b/test/e2e/api/admin/state.e2e.test.js new file mode 100644 index 000000000000..da7eaa118361 --- /dev/null +++ b/test/e2e/api/admin/state.e2e.test.js @@ -0,0 +1,80 @@ +'use strict'; + +const test = require('ava'); +const { setupApp } = require('./../../helpers/test-helper'); +const importData = require('../../../examples/import.json'); + +test.serial('exports strategies and features as json by default', async t => { + t.plan(2); + const { request, destroy } = await setupApp('state_api_serial'); + return request + .get('/api/admin/state/export') + .expect('Content-Type', /json/) + .expect(200) + .expect(res => { + t.true('features' in res.body); + t.true('strategies' in res.body); + }) + .then(destroy); +}); + +test.serial('exports strategies and features as yaml', async t => { + t.plan(0); + const { request, destroy } = await setupApp('state_api_serial'); + return request + .get('/api/admin/state/export?format=yaml') + .expect('Content-Type', /yaml/) + .expect(200) + .then(destroy); +}); + +test.serial('exports strategies and features as attachment', async t => { + t.plan(0); + const { request, destroy } = await setupApp('state_api_serial'); + return request + .get('/api/admin/state/export?download=1') + .expect('Content-Type', /json/) + .expect('Content-Disposition', /attachment/) + .expect(200) + .then(destroy); +}); + +test.serial('imports strategies and features', async t => { + t.plan(0); + const { request, destroy } = await setupApp('state_api_serial'); + return request + .post('/api/admin/state/import') + .send(importData) + .expect(202) + .then(destroy); +}); + +test.serial('does not not accept gibberish', async t => { + t.plan(0); + const { request, destroy } = await setupApp('state_api_serial'); + return request + .post('/api/admin/state/import') + .send({ features: 'nonsense' }) + .expect(400) + .then(destroy); +}); + +test.serial('imports strategies and features from json file', async t => { + t.plan(0); + const { request, destroy } = await setupApp('state_api_serial'); + return request + .post('/api/admin/state/import') + .attach('file', 'test/examples/import.json') + .expect(202) + .then(destroy); +}); + +test.serial('imports strategies and features from yaml file', async t => { + t.plan(0); + const { request, destroy } = await setupApp('state_api_serial'); + return request + .post('/api/admin/state/import') + .attach('file', 'test/examples/import.yml') + .expect(202) + .then(destroy); +}); diff --git a/test/e2e/helpers/test-helper.js b/test/e2e/helpers/test-helper.js index fbf5c7f9c84e..f36ffd1e7886 100644 --- a/test/e2e/helpers/test-helper.js +++ b/test/e2e/helpers/test-helper.js @@ -6,6 +6,7 @@ const supertest = require('supertest'); const getApp = require('../../../lib/app'); const dbInit = require('./database-init'); +const StateService = require('../../../lib/state-service'); const { EventEmitter } = require('events'); const eventBus = new EventEmitter(); @@ -18,6 +19,7 @@ function createApp(stores, adminAuthentication = 'none', preHook) { adminAuthentication, secret: 'super-secret', sessionAge: 4000, + stateService: new StateService({ stores }), }); } diff --git a/test/examples/import.json b/test/examples/import.json new file mode 100644 index 000000000000..ca098684d3af --- /dev/null +++ b/test/examples/import.json @@ -0,0 +1,72 @@ +{ + "strategies": [ + { + "name": "usersWithEmail", + "description": "Active for users defined in the comma-separated emails-parameter.", + "parameters": [ + { + "name": "emails", + "type": "string" + } + ] + }, + { + "name": "country", + "description": "Active for country.", + "parameters": [ + { + "name": "countries", + "type": "list" + } + ] + } + ], + "features": [ + { + "name": "featureX", + "description": "the #1 feature", + "enabled": true, + "strategies": [ + { + "name": "default", + "parameters": {} + } + ] + }, + { + "name": "featureA", + "description": "soon to be the #1 feature", + "enabled": false, + "strategies": [ + { + "name": "baz", + "parameters": { + "foo": "bar" + } + } + ] + }, + { + "name": "featureArchivedX", + "description": "the #1 feature", + "enabled": true, + "strategies": [ + { + "name": "default", + "parameters": {} + } + ] + }, + { + "name": "feature.with.variants", + "description": "A feature toggle with watiants", + "enabled": true, + "archived": false, + "strategies": [{ "name": "default" }], + "variants": [ + { "name": "control", "weight": 50 }, + { "name": "new", "weight": 50 } + ] + } + ] +} diff --git a/test/examples/import.yml b/test/examples/import.yml new file mode 100644 index 000000000000..3976723f4b12 --- /dev/null +++ b/test/examples/import.yml @@ -0,0 +1,42 @@ +strategies: +- name: usersWithEmail + description: Active for users defined in the comma-separated emails-parameter. + parameters: + - name: emails + type: string +- name: country + description: Active for country. + parameters: + - name: countries + type: list +features: +- name: featureX + description: 'the #1 feature' + enabled: true + strategies: + - name: default + parameters: {} +- name: featureA + description: 'soon to be the #1 feature' + enabled: false + strategies: + - name: baz + parameters: + foo: bar +- name: featureArchivedX + description: 'the #1 feature' + enabled: true + strategies: + - name: default + parameters: {} +- name: feature.with.variants + description: A feature toggle with watiants + enabled: true + archived: false + strategies: + - name: default + variants: + - name: control + weight: 50 + - name: new + weight: 50 diff --git a/test/fixtures/fake-strategies-store.js b/test/fixtures/fake-strategies-store.js index 5f0d2be84738..29ed68a1189f 100644 --- a/test/fixtures/fake-strategies-store.js +++ b/test/fixtures/fake-strategies-store.js @@ -7,6 +7,8 @@ module.exports = () => { return { getStrategies: () => Promise.resolve(_strategies), + getEditableStrategies: () => + Promise.resolve(_strategies.filter(s => s.editable)), getStrategy: name => { const strategy = _strategies.find(s => s.name === name); if (strategy) {