Support deprecating and reactivating strategies

fixes: #682

CHANGELOG.md

@@ -5,6 +5,7 @@
 - feat: add tags (#655)
 - feat: add tag-types (#655)
 - feat: Added servicelayer (#685)
+- feat: Allow deprecation of strategies (#682)
 - fix: upgrade knex to 0.21.15
 - fix: Updated docs about event-types (#684)
 - fix: Add application-created event (#595)

docs/api/admin/strategies-api.md

@@ -113,3 +113,23 @@ Used to create a new Strategy. Name is required and must be unique. It is also r
 ```
 
 Used to update a Strategy definition. Name can't be changed. **PS! I can be dangerous to change a implemnted strategy as the implementation also might need to be changed**
+
+### Deprecate strategy
+
+`POST: https://unleash.host.com/api/admin/strategies/:name/deprecate`
+
+Used to deprecate a strategy definition. This will set the deprecated flag to true. If the strategy is already deprecated, this will be a noop.
+
+#### Errors
+
+_404 NOT FOUND_ - if `:name` does not exist
+
+### Reactivate strategy
+
+`POST: https://unleash.host.com/api/admin/strategies/:name/reactivate`
+
+Used to reactivate a deprecated strategy defintion. This will set the deprecated flag back to false. If the strategy is not deprecated this is a noop and will still return 200.
+
+#### Errors
+
+_404 NOT FOUND_ - if if `:name` does not exist

lib/db/strategy-store.js

@@ -2,7 +2,13 @@
 
 const NotFoundError = require('../error/notfound-error');
 
-const STRATEGY_COLUMNS = ['name', 'description', 'parameters', 'built_in'];
+const STRATEGY_COLUMNS = [
+    'name',
+    'description',
+    'parameters',
+    'built_in',
+    'deprecated',
+];
 const TABLE = 'strategies';
 
 class StrategyStore {
@@ -46,6 +52,7 @@ class StrategyStore {
             editable: row.built_in !== 1,
             description: row.description,
             parameters: row.parameters,
+            deprecated: row.deprecated,
         };
     }
 
@@ -57,6 +64,7 @@ class StrategyStore {
             name: row.name,
             description: row.description,
             parameters: row.parameters,
+            deprecated: row.deprecated,
         };
     }
 
@@ -85,6 +93,27 @@ class StrategyStore {
             );
     }
 
+    async deprecateStrategy({ name }) {
+        this.db(TABLE)
+            .where({ name })
+            .update({ deprecated: true })
+            .catch(err =>
+                this.logger.error('Could not deprecate strategy, error: ', err),
+            );
+    }
+
+    async reactivateStrategy({ name }) {
+        this.db(TABLE)
+            .where({ name })
+            .update({ deprecated: false })
+            .catch(err =>
+                this.logger.error(
+                    'Could not reactivate strategy, error: ',
+                    err,
+                ),
+            );
+    }
+
     async deleteStrategy({ name }) {
         return this.db(TABLE)
             .where({ name })

lib/event-differ.js

@@ -8,6 +8,8 @@ const {
     STRATEGY_DELETED,
     STRATEGY_UPDATED,
     STRATEGY_IMPORT,
+    STRATEGY_DEPRECATED,
+    STRATEGY_REACTIVATED,
     DROP_STRATEGIES,
     FEATURE_CREATED,
     FEATURE_UPDATED,
@@ -35,6 +37,8 @@ const strategyTypes = [
     STRATEGY_DELETED,
     STRATEGY_UPDATED,
     STRATEGY_IMPORT,
+    STRATEGY_DEPRECATED,
+    STRATEGY_REACTIVATED,
     DROP_STRATEGIES,
 ];
 

lib/event-type.js

@@ -12,6 +12,8 @@ module.exports = {
     DROP_FEATURES: 'drop-features',
     STRATEGY_CREATED: 'strategy-created',
     STRATEGY_DELETED: 'strategy-deleted',
+    STRATEGY_DEPRECATED: 'strategy-deprecated',
+    STRATEGY_REACTIVATED: 'strategy-reactivated',
     STRATEGY_UPDATED: 'strategy-updated',
     STRATEGY_IMPORT: 'strategy-import',
     DROP_STRATEGIES: 'drop-strategies',

lib/routes/admin-api/strategy.js

@@ -23,6 +23,16 @@ class StrategyController extends Controller {
         this.delete('/:name', this.removeStrategy, DELETE_STRATEGY);
         this.post('/', this.createStrategy, CREATE_STRATEGY);
         this.put('/:strategyName', this.updateStrategy, UPDATE_STRATEGY);
+        this.post(
+            '/:strategyName/deprecate',
+            this.deprecateStrategy,
+            UPDATE_STRATEGY,
+        );
+        this.post(
+            '/:strategyName/reactivate',
+            this.reactivateStrategy,
+            UPDATE_STRATEGY,
+        );
     }
 
     async getAllStratgies(req, res) {
@@ -71,6 +81,34 @@ class StrategyController extends Controller {
             handleErrors(res, this.logger, error);
         }
     }
+
+    async deprecateStrategy(req, res) {
+        const userName = extractUser(req);
+        const { strategyName } = req.params;
+        try {
+            await this.strategyService.deprecateStrategy(
+                strategyName,
+                userName,
+            );
+            res.status(200).end();
+        } catch (error) {
+            handleErrors(res, this.logger, error);
+        }
+    }
+
+    async reactivateStrategy(req, res) {
+        const userName = extractUser(req);
+        const { strategyName } = req.params;
+        try {
+            await this.strategyService.reactivateStrategy(
+                strategyName,
+                userName,
+            );
+            res.status(200).end();
+        } catch (error) {
+            handleErrors(res, this.logger, error);
+        }
+    }
 }
 
 module.exports = StrategyController;

lib/routes/admin-api/strategy.test.js

@@ -196,3 +196,55 @@ test('editable=true will allow edit request', t => {
         .send({ name, parameters: [] })
         .expect(200);
 });
+
+test('deprecating a strategy works', async t => {
+    t.plan(1);
+    const name = 'editStrat';
+    const { request, base, strategyStore, perms } = getSetup();
+    perms.withPermissions(UPDATE_STRATEGY);
+    strategyStore.createStrategy({ name, parameters: [] });
+
+    await request
+        .post(`${base}/api/admin/strategies/${name}/deprecate`)
+        .send()
+        .expect(200);
+    return request
+        .get(`${base}/api/admin/strategies/${name}`)
+        .expect(200)
+        .expect(res => t.is(res.body.deprecated, true));
+});
+
+test('deprecating a non-existent strategy yields 404', t => {
+    t.plan(0);
+    const { request, base, perms } = getSetup();
+    perms.withPermissions(UPDATE_STRATEGY);
+    return request
+        .post(`${base}/api/admin/strategies/non-existent-strategy/deprecate`)
+        .expect(404);
+});
+
+test('reactivating a strategy works', async t => {
+    t.plan(1);
+    const name = 'editStrat';
+    const { request, base, strategyStore, perms } = getSetup();
+    perms.withPermissions(UPDATE_STRATEGY);
+    strategyStore.createStrategy({ name, parameters: [] });
+
+    await request
+        .post(`${base}/api/admin/strategies/${name}/reactivate`)
+        .send()
+        .expect(200);
+    return request
+        .get(`${base}/api/admin/strategies/${name}`)
+        .expect(200)
+        .expect(res => t.is(res.body.deprecated, false));
+});
+
+test('reactivating a non-existent strategy yields 404', t => {
+    t.plan(0);
+    const { request, base, perms } = getSetup();
+    perms.withPermissions(UPDATE_STRATEGY);
+    return request
+        .post(`${base}/api/admin/strategies/non-existent-strategy/reactivate`)
+        .expect(404);
+});

lib/routes/client-api/feature.js

@@ -6,10 +6,10 @@ const { filter } = require('./util');
 const version = 1;
 
 class FeatureController extends Controller {
-    constructor({ featureToggleService }) {
+    constructor({ featureToggleService }, getLogger) {
         super();
         this.toggleService = featureToggleService;
-
+        this.logger = getLogger('client-api/feature.js');
         this.get('/', this.getAll);
         this.get('/:featureName', this.getFeatureToggle);
     }

lib/routes/client-api/metrics.js

@@ -19,7 +19,7 @@ class ClientMetricsController extends Controller {
             await this.metrics.registerClientMetrics(data, clientIp);
             return res.status(202).end();
         } catch (e) {
-            this.logger.error('Failed to store metrics', e);
+            this.logger.warn('Failed to store metrics', e);
             switch (e.name) {
                 case 'ValidationError':
                     return res.status(400).end();

lib/routes/client-api/register.js

@@ -17,7 +17,7 @@ class RegisterController extends Controller {
             await this.metrics.registerClient(data, clientIp);
             return res.status(202).end();
         } catch (err) {
-            this.logger.error('failed to register client', err);
+            this.logger.warn('failed to register client', err);
             switch (err.name) {
                 case 'ValidationError':
                     return res.status(400).end();

lib/services/strategy-service.js

@@ -3,6 +3,8 @@ const NameExistsError = require('../error/name-exists-error');
 const {
     STRATEGY_CREATED,
     STRATEGY_DELETED,
+    STRATEGY_DEPRECATED,
+    STRATEGY_REACTIVATED,
     STRATEGY_UPDATED,
 } = require('../event-type');
 
@@ -34,8 +36,33 @@ class StrategyService {
         });
     }
 
+    async deprecateStrategy(strategyName, userName) {
+        await this.strategyStore.getStrategy(strategyName); // Check existence
+        await this.strategyStore.deprecateStrategy({ name: strategyName });
+        await this.eventStore.store({
+            type: STRATEGY_DEPRECATED,
+            createdBy: userName,
+            data: {
+                name: strategyName,
+            },
+        });
+    }
+
+    async reactivateStrategy(strategyName, userName) {
+        await this.strategyStore.getStrategy(strategyName); // Check existence
+        await this.strategyStore.reactivateStrategy({ name: strategyName });
+        await this.eventStore.store({
+            type: STRATEGY_REACTIVATED,
+            createdBy: userName,
+            data: {
+                name: strategyName,
+            },
+        });
+    }
+
     async createStrategy(value, userName) {
         const strategy = await strategySchema.validateAsync(value);
+        strategy.deprecated = false;
         await this._validateStrategyName(strategy);
         await this.strategyStore.createStrategy(strategy);
         await this.eventStore.store({

migrations/20210121115438-add-deprecated-column-to-strategies.js

@@ -0,0 +1,18 @@
+'use strict';
+
+exports.up = function(db, cb) {
+    db.runSql(
+        `
+    ALTER TABLE strategies ADD COLUMN deprecated boolean default false
+  `,
+        cb,
+    );
+};
+
+exports.down = function(db, cb) {
+    db.runSql(`ALTER TABLE strategies DROP COLUMN deprecated`, cb);
+};
+
+exports._meta = {
+    version: 1,
+};

test/e2e/api/admin/strategy.e2e.test.js

@@ -119,3 +119,50 @@ test.serial('cant update a unknown strategy', async t => {
         .set('Content-Type', 'application/json')
         .expect(404);
 });
+
+test.serial('deprecating a strategy works', async t => {
+    const request = await setupApp(stores);
+    const name = 'deprecate';
+    await request
+        .post('/api/admin/strategies')
+        .send({ name, description: 'Should deprecate this', parameters: [] })
+        .set('Content-Type', 'application/json')
+        .expect(201);
+    await request
+        .post(`/api/admin/strategies/${name}/deprecate`)
+        .send()
+        .expect(200);
+    await request
+        .get(`/api/admin/strategies/${name}`)
+        .expect('Content-Type', /json/)
+        .expect(200)
+        .expect(res => t.is(res.body.deprecated, true));
+});
+
+test.serial('can reactivate a deprecated strategy', async t => {
+    const request = await setupApp(stores);
+    const name = 'reactivate';
+    await request
+        .post('/api/admin/strategies')
+        .send({ name, description: 'Should deprecate this', parameters: [] })
+        .set('Content-Type', 'application/json')
+        .expect(201);
+    await request
+        .post(`/api/admin/strategies/${name}/deprecate`)
+        .send()
+        .expect(200);
+    await request
+        .get(`/api/admin/strategies/${name}`)
+        .expect('Content-Type', /json/)
+        .expect(200)
+        .expect(res => t.is(res.body.deprecated, true));
+    await request
+        .post(`/api/admin/strategies/${name}/reactivate`)
+        .send()
+        .expect(200);
+    await request
+        .get(`/api/admin/strategies/${name}`)
+        .expect('Content-Type', /json/)
+        .expect(200)
+        .expect(res => t.is(res.body.deprecated, false));
+});

test/fixtures/fake-strategies-store.js

@@ -3,7 +3,9 @@
 const NotFoundError = require('../../lib/error/notfound-error');
 
 module.exports = () => {
-    const _strategies = [{ name: 'default', editable: false, parameters: {} }];
+    const _strategies = [
+        { name: 'default', editable: false, parameters: {}, deprecated: false },
+    ];
 
     return {
         getStrategies: () => Promise.resolve(_strategies),
@@ -31,5 +33,23 @@ module.exports = () => {
                 _strategies.indexOf(({ name }) => name === strat.name),
                 1,
             ),
+        deprecateStrategy: ({ name }) => {
+            const deprecatedStrat = _strategies.find(s => s.name === name);
+            deprecatedStrat.deprecated = true;
+            _strategies.splice(
+                _strategies.indexOf(s => name === s.name),
+                1,
+            );
+            _strategies.push(deprecatedStrat);
+        },
+        reactivateStrategy: ({ name }) => {
+            const reactivatedStrat = _strategies.find(s => s.name === name);
+            reactivatedStrat.deprecated = false;
+            _strategies.splice(
+                _strategies.indexOf(s => name === s.name),
+                1,
+            );
+            _strategies.push(reactivatedStrat);
+        },
     };
 };