mirror of
				https://github.com/Unleash/unleash.git
				synced 2025-10-27 11:02:16 +01:00 
			
		
		
		
	Merge pull request #689 from Unleash/feat-682-deprecate-strategies
Support deprecating and reactivating strategies
This commit is contained in:
		
						commit
						88dcbffe74
					
				@ -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)
 | 
			
		||||
 | 
			
		||||
@ -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
 | 
			
		||||
 | 
			
		||||
@ -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 })
 | 
			
		||||
 | 
			
		||||
@ -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,
 | 
			
		||||
];
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
@ -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',
 | 
			
		||||
 | 
			
		||||
@ -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,38 @@ class StrategyController extends Controller {
 | 
			
		||||
            handleErrors(res, this.logger, error);
 | 
			
		||||
        }
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    async deprecateStrategy(req, res) {
 | 
			
		||||
        const userName = extractUser(req);
 | 
			
		||||
        const { strategyName } = req.params;
 | 
			
		||||
        if (strategyName === 'default') {
 | 
			
		||||
            res.status(403).end();
 | 
			
		||||
        } else {
 | 
			
		||||
            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;
 | 
			
		||||
 | 
			
		||||
@ -196,3 +196,63 @@ 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);
 | 
			
		||||
});
 | 
			
		||||
test(`deprecating 'default' strategy will yield 403`, t => {
 | 
			
		||||
    t.plan(0);
 | 
			
		||||
    const { request, base, perms } = getSetup();
 | 
			
		||||
    perms.withPermissions(UPDATE_STRATEGY);
 | 
			
		||||
    return request
 | 
			
		||||
        .post(`${base}/api/admin/strategies/default/deprecate`)
 | 
			
		||||
        .expect(403);
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
@ -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);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
@ -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();
 | 
			
		||||
 | 
			
		||||
@ -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();
 | 
			
		||||
 | 
			
		||||
@ -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({
 | 
			
		||||
 | 
			
		||||
@ -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,
 | 
			
		||||
};
 | 
			
		||||
@ -119,3 +119,56 @@ 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.serial('cannot deprecate default strategy', async t => {
 | 
			
		||||
    t.plan(0);
 | 
			
		||||
    const request = await setupApp(stores);
 | 
			
		||||
    await request.post('/api/admin/strategies/default/deprecate').expect(403);
 | 
			
		||||
});
 | 
			
		||||
 | 
			
		||||
							
								
								
									
										22
									
								
								test/fixtures/fake-strategies-store.js
									
									
									
									
										vendored
									
									
								
							
							
						
						
									
										22
									
								
								test/fixtures/fake-strategies-store.js
									
									
									
									
										vendored
									
									
								
							@ -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);
 | 
			
		||||
        },
 | 
			
		||||
    };
 | 
			
		||||
};
 | 
			
		||||
 | 
			
		||||
		Loading…
	
		Reference in New Issue
	
	Block a user