From a06d2c04bb0d83d9c8c5bf2a90e9dace50f0b10a Mon Sep 17 00:00:00 2001 From: Benjamin Ludewig Date: Thu, 14 Mar 2019 17:56:02 +0100 Subject: [PATCH] Refactored state-service, schemas, added e2e tests #395 --- .eslintrc | 2 + docs/import-export.md | 87 +++++++++++++++++++ lib/db/feature-toggle-store.js | 16 ++-- lib/db/strategy-store.js | 33 +++++-- lib/routes/admin-api/feature-schema.js | 10 ++- lib/routes/admin-api/state.js | 24 +++--- lib/routes/admin-api/strategy-schema.js | 12 ++- lib/server-impl.js | 6 +- lib/state-service.js | 110 ++++++++++++------------ lib/state-service.test.js | 4 +- test/e2e/api/admin/state.e2e.test.js | 80 +++++++++++++++++ test/e2e/helpers/test-helper.js | 2 + test/examples/import.json | 72 ++++++++++++++++ test/examples/import.yml | 42 +++++++++ test/fixtures/fake-strategies-store.js | 2 + 15 files changed, 410 insertions(+), 92 deletions(-) create mode 100644 docs/import-export.md create mode 100644 test/e2e/api/admin/state.e2e.test.js create mode 100644 test/examples/import.json create mode 100644 test/examples/import.yml diff --git a/.eslintrc b/.eslintrc index ecfcaa749a4..5add26ae439 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/docs/import-export.md b/docs/import-export.md new file mode 100644 index 00000000000..ab25a8a1356 --- /dev/null +++ b/docs/import-export.md @@ -0,0 +1,87 @@ +--- +id: import_export +title: Import & Export +--- + +Unleash supports import and export of feature-toggles and strategies at startup and during runtime. The import mechanism will guarantee that all imported features will be non-archived, as well as updates to strategies and features are included in the event history. + +All import mechanisms support a `drop` parameter which will clean the database before import (all strategies and features will be removed).\ +**You should never use this in production environments.** + +_since v3.3.0_ + +## Runtime import & export + +### State Service + +Unleash returns a StateService when started, you can use this to import and export data at any time. + +```javascript +const unleash = require('unleash-server'); + +unleash.start({...}) + .then(async ({ stateService }) => { + const exportedData = await stateService.export({includeStrategies: false, includeFeatureToggles: true}); + await stateService.import({data: exportedData, userName: 'import', dropBeforeImport: false}); + await stateService.importFile({file: 'exported-data.yml', userName: 'import', dropBeforeImport: true}) + }); +``` + +If you want the database to be cleaned before import (all strategies and features will be removed), set the `dropBeforeImport` parameter.\ +**You should never use this in production environments.** + +### API Export + +The api endpoint `/api/admin/state/export` will export feature-toggles and strategies as json by default.\ +You can customize the export with queryparameters: + +| Parameter | Default | Description | +| -------------- | ------- | --------------------------------------------------- | +| format | `json` | Export format, either `json` or `yaml` | +| download | `false` | If the exported data should be downloaded as a file | +| featureToggles | `true` | Include feature-toggles in the exported data | +| strategies | `true` | Include strategies in the exported data | + +For example if you want to download all feature-toggles as yaml: + +``` +/api/admin/state/export?format=yaml&featureToggles=1&download=1 +``` + +### API Import + +You can import feature-toggles and strategies by POSTing to the `/api/admin/state/import` endpoint (keep in mind this will require authentication).\ +You can either send the data as JSON in the POST-body or send a `file` parameter with `multipart/form-data` (YAML files are also accepted here). + +If you want the database to be cleaned before import (all strategies and features will be removed), specify a `drop` query parameter.\ +**You should never use this in production environments.** + +Example usage: + +``` +POST /api/admin/state/import +{ + "features": [ + { + "name": "a-feature-toggle", + "enabled": true, + "description": "#1 feature-toggle" + } + ] +} +``` + +## Startup import + +### Import files via config parameter + +You can import a json or yaml file via the configuration option `importFile`. + +Example usage: `unleash-server --databaseUrl ... --importFile export.yml`. + +If you want the database to be cleaned before import (all strategies and features will be removed), specify the `dropBeforeImport` option.\ +**You should never use this in production environments.** + +Example usage: `unleash-server --databaseUrl ... --importFile export.yml --dropBeforeImport`. + +These options can also be passed into the `unleash.start()` entrypoint. diff --git a/lib/db/feature-toggle-store.js b/lib/db/feature-toggle-store.js index 932025ccf8d..2c1adaafb9b 100644 --- a/lib/db/feature-toggle-store.js +++ b/lib/db/feature-toggle-store.js @@ -143,15 +143,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 575403cfaf1..7a9ce825fe8 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/feature-schema.js b/lib/routes/admin-api/feature-schema.js index 262ccdcc693..8039aac4209 100644 --- a/lib/routes/admin-api/feature-schema.js +++ b/lib/routes/admin-api/feature-schema.js @@ -40,7 +40,11 @@ const featureShema = joi .keys({ name: nameType, enabled: joi.boolean().default(false), - description: joi.string(), + description: joi + .string() + .allow('') + .allow(null) + .optional(), strategies: joi .array() .required() @@ -48,10 +52,10 @@ const featureShema = joi .items(strategiesSchema), variants: joi .array() + .allow(null) .unique((a, b) => a.name === b.name) .optional() - .items(variantsSchema) - .allow(null), + .items(variantsSchema), }) .options({ allowUnknown: false, stripUnknown: true }); diff --git a/lib/routes/admin-api/state.js b/lib/routes/admin-api/state.js index e668ee39747..15b384bbd1f 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/routes/admin-api/strategy-schema.js b/lib/routes/admin-api/strategy-schema.js index 3a61c015a98..7175e7bce92 100644 --- a/lib/routes/admin-api/strategy-schema.js +++ b/lib/routes/admin-api/strategy-schema.js @@ -6,7 +6,11 @@ const { nameType } = require('./util'); const strategySchema = joi.object().keys({ name: nameType, editable: joi.boolean().default(true), - description: joi.string(), + description: joi + .string() + .allow(null) + .allow('') + .optional(), parameters: joi .array() .required() @@ -14,7 +18,11 @@ const strategySchema = joi.object().keys({ joi.object().keys({ name: joi.string().required(), type: joi.string().required(), - description: joi.string().allow(''), + description: joi + .string() + .allow(null) + .allow('') + .optional(), required: joi.boolean(), }) ), diff --git a/lib/server-impl.js b/lib/server-impl.js index a446f4562e8..5d35cb92d21 100644 --- a/lib/server-impl.js +++ b/lib/server-impl.js @@ -40,9 +40,9 @@ async function createApp(options) { config.stateService = stateService; if (config.importFile) { await stateService.importFile({ - importFile: config.importFile, + file: config.importFile, dropBeforeImport: config.dropBeforeImport, - userName: 'importer', + userName: 'import', }); } @@ -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 0c9f7a60ee7..b28391141fe 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, @@ -17,6 +17,7 @@ const { const logger = getLogger('state-service.js'); const dataSchema = joi.object().keys({ + version: joi.number(), features: joi .array() .optional() @@ -27,39 +28,36 @@ const dataSchema = joi.object().keys({ .items(strategySchema), }); +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, - }); + 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 +66,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 +87,33 @@ 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]) => ({ + version: 1, + features, + strategies, + })); } } diff --git a/lib/state-service.test.js b/lib/state-service.test.js index 3f1c04e1591..26e3e68cc05 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 00000000000..da7eaa11836 --- /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 fbf5c7f9c84..f36ffd1e788 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 00000000000..ca098684d3a --- /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 00000000000..3976723f4b1 --- /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 5f0d2be8473..29ed68a1189 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) {