Merge pull request #185 from Unleash/update_strategy

Update strategy

docs/api/strategies-api.md

@@ -76,4 +76,35 @@ Used to fetch all defined strategies and their defined paramters.
 },
 ```
 
-Used to create a new Strategy. Name is required and must be unique. It is also required to have a parameters array, but it can be empty. 
+Used to create a new Strategy. Name is required and must be unique. It is also required to have a parameters array, but it can be empty. 
+
+
+### Update strategy
+
+`PUT: http://unleash.host.com/api/strategies/:name`
+
+**Body**
+
+```json
+{
+    "name": "gradualRollout",
+    "description": "Gradual rollout to logged in users with updated desc",
+    "parameters": [
+        {
+            "name": "percentage",
+            "type": "percentage",
+            "description": "How many percent should the new feature be active for.",
+            "required": false
+        },
+        {
+            "name": "group",
+            "type": "string",
+            "description": "Group key to use when hasing the userId. Makes sure that the same user get different value for different groups",
+            "required": false
+        }
+    ]
+},
+```
+
+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** 

lib/db/strategy-store.js

@@ -1,6 +1,6 @@
 'use strict';
 
-const { STRATEGY_CREATED, STRATEGY_DELETED } = require('../event-type');
+const { STRATEGY_CREATED, STRATEGY_DELETED, STRATEGY_UPDATED } = require('../event-type');
 const logger = require('../logger');
 const NotFoundError = require('../error/notfound-error');
 const STRATEGY_COLUMNS = ['name', 'description', 'parameters'];
@@ -10,6 +10,7 @@ class StrategyStore {
     constructor (db, eventStore) {
         this.db = db;
         eventStore.on(STRATEGY_CREATED, event => this._createStrategy(event.data));
+        eventStore.on(STRATEGY_UPDATED, event => this._updateStrategy(event.data));
         eventStore.on(STRATEGY_DELETED, event => {
             db(TABLE)
                 .where('name', event.data.name)
@@ -61,6 +62,13 @@ class StrategyStore {
             .insert(this.eventDataToRow(data))
             .catch(err => logger.error('Could not insert strategy, error was: ', err));
     }
+
+    _updateStrategy (data) {
+        this.db(TABLE)
+            .where({ name: data.name })
+            .update(this.eventDataToRow(data))
+            .catch(err => logger.error('Could not update strategy, error was: ', err));
+    }
 };
 
 module.exports = StrategyStore;

lib/event-differ.js

@@ -3,6 +3,7 @@
 const {
     STRATEGY_CREATED,
     STRATEGY_DELETED,
+    STRATEGY_UPDATED,
     FEATURE_CREATED,
     FEATURE_UPDATED,
     FEATURE_ARCHIVED,
@@ -13,6 +14,7 @@ const diff = require('deep-diff').diff;
 const strategyTypes = [
     STRATEGY_CREATED,
     STRATEGY_DELETED,
+    STRATEGY_UPDATED,
 ];
 
 const featureTypes  = [

lib/event-type.js

@@ -7,4 +7,5 @@ module.exports = {
     FEATURE_REVIVED: 'feature-revived',
     STRATEGY_CREATED: 'strategy-created',
     STRATEGY_DELETED: 'strategy-deleted',
+    STRATEGY_UPDATED: 'strategy-updated',
 };

lib/routes/strategy.js

@@ -75,6 +75,23 @@ module.exports = function (app, config) {
             .catch(error => handleError(req, res, error));
     });
 
+    app.put('/strategies/:strategyName', (req, res) => {
+        const strategyName = req.params.strategyName;
+        const updatedStrategy = req.body;
+
+        updatedStrategy.name = strategyName;
+
+        strategyStore.getStrategy(strategyName)
+            .then(() => validateInput(updatedStrategy))
+            .then(() => eventStore.store({
+                type: eventType.STRATEGY_UPDATED,
+                createdBy: extractUser(req),
+                data: updatedStrategy,
+            }))
+            .then(() => res.status(200).end())
+            .catch(error => handleError(req, res, error));
+    });
+
     function validateStrategyName (data) {
         return new Promise((resolve, reject) => {
             strategyStore.getStrategy(data.name)

test/e2e/strategy-api.test.js

@@ -82,3 +82,23 @@ test.serial('can\'t delete a strategy that dose not exist', async (t) => {
         .delete('/api/strategies/unknown')
         .expect(404);
 });
+
+test.serial('updates a exiting strategy', async (t) => {
+    const { request, destroy } = await setupApp('strategy_api_serial');
+    return request
+        .put('/api/strategies/default')
+        .send({ name: 'default', description: 'Default is the best!', parameters: [] })
+        .set('Content-Type', 'application/json')
+        .expect(200)
+        .then(destroy);
+});
+
+test.serial('cant update a unknown strategy', async (t) => {
+    const { request, destroy } = await setupApp('strategy_api_serial');
+    return request
+        .put('/api/strategies/unknown')
+        .send({ name: 'unkown', parameters: [] })
+        .set('Content-Type', 'application/json')
+        .expect(404)
+        .then(destroy);
+});

test/unit/routes/strategies.test.js

@@ -78,3 +78,35 @@ test('should not be possible to override name', () => {
         .send({ name: 'Testing', parameters: [] })
         .expect(403);
 });
+
+test('should update strategy', () => {
+    const name = 'AnotherStrat';
+    const { request, base, strategyStore } = getSetup();
+    strategyStore.addStrategy({ name, parameters: [] });
+
+    return request
+        .put(`${base}/api/strategies/${name}`)
+        .send({ name, parameters: [], description: 'added' })
+        .expect(200);
+});
+
+test('should not update uknown strategy', () => {
+    const name = 'UnknownStrat';
+    const { request, base } = getSetup();
+
+    return request
+        .put(`${base}/api/strategies/${name}`)
+        .send({ name, parameters: [], description: 'added' })
+        .expect(404);
+});
+
+test('should validate format when updating strategy', () => {
+    const name = 'AnotherStrat';
+    const { request, base, strategyStore } = getSetup();
+    strategyStore.addStrategy({ name, parameters: [] });
+
+    return request
+        .put(`${base}/api/strategies/${name}`)
+        .send({  })
+        .expect(400);
+});