From 94e0e0ea98bcce684e089317c1ac04494a0a26ed Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Mon, 5 Sep 2016 16:50:52 +0200 Subject: [PATCH] Added version indicator to collection responses to ease migration of client parsers relates to #102 --- .../lib/helper/legacy-feature-mapper.js | 2 +- packages/unleash-api/lib/routes/event.js | 3 +- packages/unleash-api/lib/routes/feature.js | 3 +- packages/unleash-api/lib/routes/strategy.js | 3 +- packages/unleash-api/notes/api.md | 6 ++- packages/unleash-api/package.json | 1 + .../unleash-api/test/e2e/featureApiSpec.js | 10 ++-- .../unit/helper/legacy-feature-mapper.test.js | 2 +- .../test/unit/routes/feature.test.js | 19 ++++++-- .../test/unit/routes/strategies.test.js | 47 +++++++++++++++++++ 10 files changed, 82 insertions(+), 14 deletions(-) create mode 100644 packages/unleash-api/test/unit/routes/strategies.test.js diff --git a/packages/unleash-api/lib/helper/legacy-feature-mapper.js b/packages/unleash-api/lib/helper/legacy-feature-mapper.js index 7a0844287c..35f93237c5 100644 --- a/packages/unleash-api/lib/helper/legacy-feature-mapper.js +++ b/packages/unleash-api/lib/helper/legacy-feature-mapper.js @@ -1,7 +1,7 @@ 'use strict'; function addOldFields (feature) { - let modifiedFeature = Object.assign({}, feature); + const modifiedFeature = Object.assign({}, feature); modifiedFeature.strategy = feature.strategies[0].name; modifiedFeature.parameters = Object.assign({}, feature.strategies[0].parameters); return modifiedFeature; diff --git a/packages/unleash-api/lib/routes/event.js b/packages/unleash-api/lib/routes/event.js index fe9ec09bb8..568ee9e8ff 100644 --- a/packages/unleash-api/lib/routes/event.js +++ b/packages/unleash-api/lib/routes/event.js @@ -1,5 +1,6 @@ 'use strict'; const eventDiffer = require('../eventDiffer'); +const version = 1; module.exports = function (app, config) { const eventDb = config.eventDb; @@ -7,7 +8,7 @@ module.exports = function (app, config) { app.get('/events', (req, res) => { eventDb.getEvents().then(events => { eventDiffer.addDiffs(events); - res.json({ events }); + res.json({ version, events }); }); }); diff --git a/packages/unleash-api/lib/routes/feature.js b/packages/unleash-api/lib/routes/feature.js index a5c6bb082f..9ac253928c 100644 --- a/packages/unleash-api/lib/routes/feature.js +++ b/packages/unleash-api/lib/routes/feature.js @@ -9,6 +9,7 @@ const validateRequest = require('../error/validateRequest'); const extractUser = require('../extractUser'); const legacyFeatureMapper = require('../helper/legacy-feature-mapper'); +const version = 1; module.exports = function (app, config) { const featureDb = config.featureDb; @@ -17,7 +18,7 @@ module.exports = function (app, config) { app.get('/features', (req, res) => { featureDb.getFeatures() .then((features) => features.map(legacyFeatureMapper.addOldFields)) - .then(features => res.json({ features })); + .then(features => res.json({ version, features })); }); app.get('/features/:featureName', (req, res) => { diff --git a/packages/unleash-api/lib/routes/strategy.js b/packages/unleash-api/lib/routes/strategy.js index 8eacc55c0d..00356aad5b 100644 --- a/packages/unleash-api/lib/routes/strategy.js +++ b/packages/unleash-api/lib/routes/strategy.js @@ -7,6 +7,7 @@ const ValidationError = require('../error/ValidationError'); const NotFoundError = require('../error/NotFoundError'); const validateRequest = require('../error/validateRequest'); const extractUser = require('../extractUser'); +const version = 1; module.exports = function (app, config) { const strategyDb = config.strategyDb; @@ -14,7 +15,7 @@ module.exports = function (app, config) { app.get('/strategies', (req, res) => { strategyDb.getStrategies().then(strategies => { - res.json({ strategies }); + res.json({ version, strategies }); }); }); diff --git a/packages/unleash-api/notes/api.md b/packages/unleash-api/notes/api.md index 262caab77a..47662e8108 100644 --- a/packages/unleash-api/notes/api.md +++ b/packages/unleash-api/notes/api.md @@ -11,6 +11,7 @@ GET: http://unleash.host.com/features ```json { + "version": 1, "features": [ { "name": "Feature.A", @@ -53,7 +54,9 @@ GET: http://unleash.host.com/features ``` **Important:** -_strategy_ and _paramters_ are depercated fields and will go away in the next version. They are kept for backward compability with older unleash-clients. +_strategy_ and _paramters_ are deprecated fields and will go away in the next version, 2. +They are kept for backward compatibility with older unleash-clients. _version_ property indicates +the json-response version, making it easier for clients to parse the response. ## Fetch a feature @@ -76,4 +79,5 @@ GET: http://unleash.host.com/features/[featureName] } ``` +_Notice that you will not get a version property when fetching a specific feature toggle by name_. diff --git a/packages/unleash-api/package.json b/packages/unleash-api/package.json index 7ecff6c41e..d6a9fa63c0 100644 --- a/packages/unleash-api/package.json +++ b/packages/unleash-api/package.json @@ -36,6 +36,7 @@ "start:dev:pg-chain": "export DATABASE_URL=postgres://$PGUSER:$PGPASSWORD@localhost:$PGPORT/postgres ; db-migrate up && npm run start:dev", "db-migrate": "db-migrate up", "test": "export PORT=4243 ; mocha test/**/*.js && npm run test:coverage", + "test:unit": "mocha test/unit/**/*.js ", "test:ci": "npm run db-migrate && npm run test", "test:watch": "mocha --watch test test/*", "test:pg-virtualenv": "pg_virtualenv npm run test:pg-virtualenv-chai", diff --git a/packages/unleash-api/test/e2e/featureApiSpec.js b/packages/unleash-api/test/e2e/featureApiSpec.js index 1d245fca0b..5c383125d3 100644 --- a/packages/unleash-api/test/e2e/featureApiSpec.js +++ b/packages/unleash-api/test/e2e/featureApiSpec.js @@ -112,12 +112,12 @@ describe('The features api', () => { .expect(403, done); }); - describe('new strategies api', function () { - it('automatically map existing strategy to strategies array', function (done) { + describe('new strategies api', () => { + it('automatically map existing strategy to strategies array', (done) => { request .get('/features/featureY') .expect('Content-Type', /json/) - .end(function (err, res) { + .end((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); @@ -125,7 +125,7 @@ describe('The features api', () => { }); }); - it('can add two strategies to a feature toggle', function (done) { + it('can add two strategies to a feature toggle', (done) => { request .put('/features/featureY') .send({ @@ -142,7 +142,7 @@ describe('The features api', () => { .expect(200, done); }); - it('should not be allowed to post both strategy and strategies', function (done) { + it('should not be allowed to post both strategy and strategies', (done) => { logger.setLevel('FATAL'); request .post('/features') diff --git a/packages/unleash-api/test/unit/helper/legacy-feature-mapper.test.js b/packages/unleash-api/test/unit/helper/legacy-feature-mapper.test.js index 11e91a1fff..1b288da2af 100644 --- a/packages/unleash-api/test/unit/helper/legacy-feature-mapper.test.js +++ b/packages/unleash-api/test/unit/helper/legacy-feature-mapper.test.js @@ -16,7 +16,7 @@ describe('legacy-feature-mapper', () => { }], }; - let mappedFeature = mapper.addOldFields(feature); + const mappedFeature = mapper.addOldFields(feature); assert.equal(mappedFeature.name, feature.name); assert.equal(mappedFeature.enabled, feature.enabled); diff --git a/packages/unleash-api/test/unit/routes/feature.test.js b/packages/unleash-api/test/unit/routes/feature.test.js index 7ecd03c8ef..e4695ab16f 100644 --- a/packages/unleash-api/test/unit/routes/feature.test.js +++ b/packages/unleash-api/test/unit/routes/feature.test.js @@ -1,6 +1,6 @@ 'use strict'; -let supertest = require('supertest'); +const supertest = require('supertest'); const BPromise = require('bluebird'); BPromise.promisifyAll(supertest); const assert = require('assert'); @@ -18,7 +18,7 @@ describe('Unit: The features api', () => { db: sinon.stub(), eventDb: sinon.stub(), eventStore: sinon.stub(), - featureDb: featureDb, + featureDb, strategyDb: sinon.stub(), }); @@ -49,10 +49,23 @@ describe('Unit: The features api', () => { done(); }); }); + + it('should add version numbers for /features', (done) => { + featureDb.addFeature( { name: 'test', strategies: [{ name: 'default' }] } ); + + request + .get('/features') + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + assert.equal(res.body.version, 1); + done(); + }); + }); }); function createFeatureDb () { - let _features = []; + const _features = []; return { getFeatures: () => BPromise.resolve(_features), addFeature: (feature) => _features.push(feature), diff --git a/packages/unleash-api/test/unit/routes/strategies.test.js b/packages/unleash-api/test/unit/routes/strategies.test.js new file mode 100644 index 0000000000..2b5ae03bb5 --- /dev/null +++ b/packages/unleash-api/test/unit/routes/strategies.test.js @@ -0,0 +1,47 @@ +'use strict'; + +const supertest = require('supertest'); +const BPromise = require('bluebird'); +BPromise.promisifyAll(supertest); +const assert = require('assert'); +const sinon = require('sinon'); + +let request; +let stratDb; + +describe('Unit: The strategies api', () => { + beforeEach(done => { + stratDb = createStrategyDb(); + + const app = require('../../../app')({ + baseUriPath: '', + db: sinon.stub(), + eventDb: sinon.stub(), + eventStore: sinon.stub(), + featureDb: sinon.stub(), + strategyDb: stratDb, + }); + + request = supertest(app); + done(); + }); + + it('should add version numbers for /stategies', (done) => { + request + .get('/strategies') + .expect('Content-Type', /json/) + .expect(200) + .end((err, res) => { + assert.equal(res.body.version, 1); + done(); + }); + }); +}); + +function createStrategyDb () { + const _strategies = [{ name: 'default', parameters: {} }]; + return { + getStrategies: () => BPromise.resolve(_strategies), + addStrategy: (strat) => _strategies.push(strat), + }; +}