refactored state-service, added e2e tests Unleash#395

.eslintrc

@@ -8,6 +8,8 @@
         "ecmaVersion": "2017"
     },
     "rules": {
+        "no-param-reassign": "error",
+        "no-return-await": "error",
         "max-nested-callbacks": "off",
         "new-cap": [
             "error",

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)
             );

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)
             );

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;

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,

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 }));
     }
 }
 

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');