Skip to content

Commit

Permalink
Add support for aggregate_strategies in the API.
Browse files Browse the repository at this point in the history
This commit changes the features-tables:
- drop columns 'strategy' and 'parameters'
- add column 'strategies' of type json.
- migrates existing strategy-mappings in to the new format.

The idea is that the 'strategies' column should contain a json-array
of strategy-configuration for the toggle:

```
[{
 "name" : "strategy1",
 "parameters": { "name": "vale" }
}]

```

To make sure to not break exiting clients the api is extended with a
mapping layer (adding old fields to the json-respons, and mapping
to the new format on create/update a feature toggle.

this commit is first step in solving #102
  • Loading branch information
ivarconr committed Sep 5, 2016
1 parent 4a527a6 commit b4dc2b3
Show file tree
Hide file tree
Showing 10 changed files with 257 additions and 58 deletions.
11 changes: 4 additions & 7 deletions packages/unleash-api/lib/db/feature.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
const eventType = require('../eventType');
const logger = require('../logger');
const NotFoundError = require('../error/NotFoundError');
const FEATURE_COLUMNS = ['name', 'description', 'enabled', 'strategy_name', 'parameters'];
const FEATURE_COLUMNS = ['name', 'description', 'enabled', 'strategies'];

module.exports = function (db, eventStore) {
eventStore.on(eventType.featureCreated, event => createFeature(event.data));
Expand Down Expand Up @@ -39,18 +39,15 @@ module.exports = function (db, eventStore) {
.map(rowToFeature);
}


function rowToFeature (row) {
if (!row) {
throw new NotFoundError('No feature toggle found');
}

return {
name: row.name,
description: row.description,
enabled: row.enabled > 0,
strategy: row.strategy_name, // eslint-disable-line
parameters: row.parameters,
strategies: row.strategies,
};
}

Expand All @@ -60,8 +57,7 @@ module.exports = function (db, eventStore) {
description: data.description,
enabled: data.enabled ? 1 : 0,
archived: data.archived ? 1 : 0,
strategy_name: data.strategy, // eslint-disable-line
parameters: data.parameters,
strategies: JSON.stringify(data.strategies),
};
}

Expand All @@ -74,6 +70,7 @@ module.exports = function (db, eventStore) {
}

function updateFeature (data) {
console.log(data);
return db('features')
.where({ name: data.name })
.update(eventDataToRow(data))
Expand Down
31 changes: 31 additions & 0 deletions packages/unleash-api/lib/helper/legacy-feature-mapper.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use strict';

function addOldFields (feature) {
let modifiedFeature = Object.assign({}, feature);
modifiedFeature.strategy = feature.strategies[0].name;
modifiedFeature.parameters = Object.assign({}, feature.strategies[0].parameters);
return modifiedFeature;
}

function isOldFomrat (feature) {
return feature.strategy && !feature.strategies;
}

function toNewFormat (feature) {
if (isOldFomrat(feature)) {
return {
name: feature.name,
description: feature.description,
enabled: feature.enabled,
strategies: [
{
name: feature.strategy,
parameters: Object.assign({}, feature.parameters),
},
],
};
}
return feature;
}

module.exports = { addOldFields, toNewFormat };
52 changes: 19 additions & 33 deletions packages/unleash-api/lib/routes/feature.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,23 @@ const ValidationError = require('../error/ValidationError');
const validateRequest = require('../error/validateRequest');
const extractUser = require('../extractUser');

const legacyFeatureMapper = require('../helper/legacy-feature-mapper');

module.exports = function (app, config) {
const featureDb = config.featureDb;
const eventStore = config.eventStore;

app.get('/features', (req, res) => {
featureDb.getFeatures().then(features => {
res.json({ features });
});
featureDb.getFeatures()
.then((features) => features.map(legacyFeatureMapper.addOldFields))
.then(features => res.json({ features }));
});

app.get('/features/:featureName', (req, res) => {
featureDb.getFeature(req.params.featureName)
.then(feature => {
res.json(feature);
})
.catch(() => {
res.status(404).json({ error: 'Could not find feature' });
});
.then(legacyFeatureMapper.addOldFields)
.then(feature => res.json(feature).end())
.catch(() => res.status(404).json({ error: 'Could not find feature' }));
});

app.post('/features', (req, res) => {
Expand All @@ -37,20 +36,15 @@ module.exports = function (app, config) {
.then(() => eventStore.create({
type: eventType.featureCreated,
createdBy: extractUser(req),
data: req.body,
data: legacyFeatureMapper.toNewFormat(req.body),
}))
.then(() => {
res.status(201).end();
})
.then(() => res.status(201).end())
.catch(NameExistsError, () => {
res.status(403).json([{
msg: `A feature named '${req.body.name}' already exists. It could be archived.`,
}])
.end();
})
.catch(ValidationError, () => {
res.status(400).json(req.validationErrors());
res.status(403)
.json([{ msg: `A feature named '${req.body.name}' already exists.` }])
.end();
})
.catch(ValidationError, () => res.status(400).json(req.validationErrors()))
.catch(err => {
logger.error('Could not create feature toggle', err);
res.status(500).end();
Expand All @@ -60,7 +54,7 @@ module.exports = function (app, config) {
app.put('/features/:featureName', (req, res) => {
const featureName = req.params.featureName;
const userName = extractUser(req);
const updatedFeature = req.body;
const updatedFeature = legacyFeatureMapper.toNewFormat(req.body);

updatedFeature.name = featureName;

Expand All @@ -70,12 +64,8 @@ module.exports = function (app, config) {
createdBy: userName,
data: updatedFeature,
}))
.then(() => {
res.status(200).end();
})
.catch(NotFoundError, () => {
res.status(404).end();
})
.then(() => res.status(200).end())
.catch(NotFoundError, () => res.status(404).end())
.catch(err => {
logger.error(`Could not update feature toggle=${featureName}`, err);
res.status(500).end();
Expand All @@ -94,12 +84,8 @@ module.exports = function (app, config) {
name: featureName,
},
}))
.then(() => {
res.status(200).end();
})
.catch(NotFoundError, () => {
res.status(404).end();
})
.then(() => res.status(200).end())
.catch(NotFoundError, () => res.status(404).end())
.catch(err => {
logger.error(`Could not archive feature=${featureName}`, err);
res.status(500).end();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
'use strict';
module.exports = require('../scripts/migration-runner').create('007-add-strategies-to-features');
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
--create old columns
ALTER TABLE features ADD "parameters" json;
ALTER TABLE features ADD "strategy_name" varchar(255);

--populate old columns
UPDATE features
SET strategy_name = f.strategies->0->>'name',
parameters = f.strategies->0->'parameters'
FROM features as f
WHERE f.name = features.name;

--drop new column
ALTER TABLE features DROP COLUMN "strategies";
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--create new strategies-column
ALTER TABLE features ADD "strategies" json;

--populate the strategies column
UPDATE features
SET strategies = ('[{"name":"'||f.strategy_name||'","parameters":'||f.parameters||'}]')::json
FROM features as f
WHERE f.name = features.name;

--delete old strategy-columns
ALTER TABLE features DROP COLUMN "strategy_name";
ALTER TABLE features DROP COLUMN "parameters";
71 changes: 71 additions & 0 deletions packages/unleash-api/notes/schema.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# Schema

## Table: _migrations_

Used by db-migrate module to keep track of migrations.

| NAME | TYPE | SIZE | NULLABLE | COLUMN_DEF |
| ----------- | --------- | ----------- | -------- | -------------------------------------- |
| id | serial | 10 | 0 | nextval('migrations_id_seq'::regclass) |
| name | varchar | 255 | 0 | (null) |
| run_on | timestamp | 29 | 0 | (null) |



## Table: _events_
| NAME | TYPE | SIZE | NULLABLE | COLUMN_DEF |
| ----------- | --------- | ----------- | -------- | ---------------------------------- |
| id | serial | 10 | 0 | nextval('events_id_seq'::regclass) |
| created_at | timestamp | 29 | 1 | now() |
| type | varchar | 255 | 0 | (null) |
| created_by | varchar | 255 | 0 | (null) |
| data | json | 2147483647 | 1 | (null) |


## Table: _strategies_loc
| NAME | TYPE | SIZE | NULLABLE | COLUMN_DEF |
| ------------------- | --------- | ----------- | -------- | ---------- |
| created_at | timestamp | 29 | 1 | now() |
| name | varchar | 255 | 0 | (null) |
| description | text | 2147483647 | 1 | (null) |
| parameters_template | json | 2147483647 | 1 | (null) |


## Table: _features_

| **NAME** | **TYPE** | **SIZE** | **NULLABLE** | **COLUMN_DEF** | **COMMENT** |
| ------------- | --------- | ----------- | ------------ | -------------- | ----------- |
| created_at | timestamp | 29 | 1 | now() | |
| name | varchar | 255 | 0 | (null) | |
| enabled | int4 | 10 | 1 | 0 | |
| description | text | 2147483647 | 1 | (null) | |
| archived | int4 | 10 | 1 | 0 | |
| parameters | json | 2147483647 | 1 | (null) | deprecated (*) |
| strategy_name | varchar | 255 | 1 | (null) | deprecated (*) |
| strategies | json | 2147483647 | 1 | (null) | |

(*) we migrated from `parmaters` and `strategy_name` to `strategies` which should contain an array of these.

For [aggregate strategies](https:/finn-no/unleash/issues/102) we had the following sql to migrate to the strategies column:

```sql
ALTER TABLE features ADD "strategies" json;

--populate the strategies column
UPDATE features
SET strategies = ('[{"name":"'||f.strategy_name||'","parameters":'||f.parameters||'}]')::json
FROM features as f
WHERE f.name = features.name;
```

In order to migrate back, one can use the following sql (it will loose all, but the first activation strategy):

```sql
UPDATE features
SET strategy_name = f.strategies->0->>'name',
parameters = f.strategies->0->'parameters'
FROM features as f
WHERE f.name = features.name;

ALTER TABLE features DROP COLUMN "strategies";
```
31 changes: 31 additions & 0 deletions packages/unleash-api/test/featureApiSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,35 @@ describe('The features api', () => {
.set('Content-Type', 'application/json')
.expect(403, done);
});

describe('new strategies api', function () {
it('automatically map existing strategy to strategies array', function (done) {
request
.get('/features/featureY')
.expect('Content-Type', /json/)
.end(function (err, res) {
assert.equal(res.body.strategies.length, 1, 'expected strategy added to strategies');
assert.equal(res.body.strategy, res.body.strategies[0].name);
assert.deepEqual(res.body.parameters, res.body.strategies[0].parameters);
done();
});
});

it('can add two strategies to a feature toggle', function (done) {
request
.put('/features/featureY')
.send({
name: 'featureY',
description: 'soon to be the #14 feature',
enabled: false,
strategies: [
{
name: 'baz',
parameters: { foo: 'bar' },
},
] })
.set('Content-Type', 'application/json')
.expect(200, done);
});
});
});
44 changes: 26 additions & 18 deletions packages/unleash-api/test/specHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,52 +46,60 @@ function createFeatures () {
name: 'featureX',
description: 'the #1 feature',
enabled: true,
strategy: 'default',
strategies: [{ name: 'default', parameters: {} }],
},
{
name: 'featureY',
description: 'soon to be the #1 feature',
enabled: false,
strategy: 'baz',
parameters: {
foo: 'bar',
},
strategies: [{
name: 'baz',
parameters: {
foo: 'bar',
},
}],
},
{
name: 'featureZ',
description: 'terrible feature',
enabled: true,
strategy: 'baz',
parameters: {
foo: 'rab',
},
strategies: [{
name: 'baz',
parameters: {
foo: 'rab',
},
}],
},
{
name: 'featureArchivedX',
description: 'the #1 feature',
enabled: true,
archived: true,
strategy: 'default',
strategies: [{ name: 'default', parameters: {} }],
},
{
name: 'featureArchivedY',
description: 'soon to be the #1 feature',
enabled: false,
archived: true,
strategy: 'baz',
parameters: {
foo: 'bar',
},
strategies: [{
name: 'baz',
parameters: {
foo: 'bar',
},
}],
},
{
name: 'featureArchivedZ',
description: 'terrible feature',
enabled: true,
archived: true,
strategy: 'baz',
parameters: {
foo: 'rab',
},
strategies: [{
name: 'baz',
parameters: {
foo: 'rab',
},
}],
},
], feature => featureDb._createFeature(feature));
}
Expand Down
Loading

0 comments on commit b4dc2b3

Please sign in to comment.