From 5735b0931ad9b30f7c52ea4e6720e9a346bc9a53 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Fri, 11 Nov 2016 15:46:59 +0100 Subject: [PATCH] Cleanup route/metrics a bit --- lib/db/client-metrics-store.js | 2 +- lib/routes/metrics-schema.js | 24 ++++++++ lib/routes/metrics.js | 52 +++++------------ test/unit/routes/feature.test.js | 21 ++----- test/unit/routes/metrics.test.js | 56 +++++++++++++------ .../mocks/fake-client-instance-store.js | 1 + .../mocks/fake-client-strategy-store.js | 1 + test/unit/routes/mocks/fake-metrics-store.js | 2 + test/unit/routes/mocks/store.js | 27 +++++++++ test/unit/routes/strategies.test.js | 21 ++----- 10 files changed, 121 insertions(+), 86 deletions(-) create mode 100644 lib/routes/metrics-schema.js create mode 100644 test/unit/routes/mocks/store.js diff --git a/lib/db/client-metrics-store.js b/lib/db/client-metrics-store.js index 7ed0d54c5b..bafa031ed7 100644 --- a/lib/db/client-metrics-store.js +++ b/lib/db/client-metrics-store.js @@ -22,7 +22,7 @@ class ClientMetricsStore { this.db(TABLE) .whereRaw('created_at < now() - interval \'1 hour\'') .del() - .then((res) => logger.info(`Delted ${res} metrics`)); + .then((res) => logger.info(`Deleted ${res} metrics`)); } // Insert new client metrics diff --git a/lib/routes/metrics-schema.js b/lib/routes/metrics-schema.js new file mode 100644 index 0000000000..c24b6c1358 --- /dev/null +++ b/lib/routes/metrics-schema.js @@ -0,0 +1,24 @@ +const joi = require('joi'); + +const clientMetricsSchema = joi.object().keys({ + appName: joi.string().required(), + instanceId: joi.string().required(), + bucket: joi.object().required() + .keys({ + start: joi.date().required(), + stop: joi.date().required(), + toggles: joi.object(), + }), +}); + +const clientRegisterSchema = joi.object().keys({ + appName: joi.string().required(), + instanceId: joi.string().required(), + strategies: joi.array() + .required() + .items(joi.string(), joi.any().strip()), + started: joi.date().required(), + interval: joi.number().required(), +}); + +module.exports = { clientMetricsSchema, clientRegisterSchema } \ No newline at end of file diff --git a/lib/routes/metrics.js b/lib/routes/metrics.js index 236192c22c..6212eafea8 100644 --- a/lib/routes/metrics.js +++ b/lib/routes/metrics.js @@ -4,6 +4,7 @@ const logger = require('../logger'); const ClientMetrics = require('../client-metrics'); const ClientMetricsService = require('../client-metrics/service'); const joi = require('joi'); +const { clientMetricsSchema, clientRegisterSchema } = require('./metrics-schema'); module.exports = function (app, config) { const { @@ -11,6 +12,7 @@ module.exports = function (app, config) { clientStrategyStore, clientInstanceStore, } = config.stores; + const metrics = new ClientMetrics(); const service = new ClientMetricsService(clientMetricsStore); @@ -28,45 +30,17 @@ module.exports = function (app, config) { res.json(metrics.getTogglesMetrics()); }); - const clientMetricsSchema = joi.object().keys({ - appName: joi.string().required(), - instanceId: joi.string().required(), - bucket: joi.object().required() - .keys({ - start: joi.date().required(), - stop: joi.date().required(), - toggles: joi.object() - .required() - .unknown() - .min(1) - .max(1000), - }), - }); - app.post('/client/metrics', (req, res) => { - try { - const data = typeof req.body === 'string' ? JSON.parse(req.body) : req.body; - const result = joi.validate(data, clientMetricsSchema); - if (result.error) { - throw result.error; + const data = req.body; + joi.validate(data, clientMetricsSchema, (err, cleaned) => { + if (err) { + return res.status(400).json(err); } - service - .insert(result.value) + service.insert(cleaned) .catch(e => logger.error('Error inserting metrics data', e)); - } catch (e) { - logger.error('Error receiving metrics', e); - } - res.end(); - }); - - const clientRegisterSchema = joi.object().keys({ - appName: joi.string().required(), - instanceId: joi.string().required(), - strategies: joi.array() - .required() - .items(joi.string(), joi.any().strip()), - started: joi.date().required(), - interval: joi.number().required(), + + res.status(202).end(); + }); }); app.post('/client/register', (req, res) => { @@ -84,10 +58,10 @@ module.exports = function (app, config) { instanceId: cleaned.instanceId, clientIp, })) - .then(() => console.log('new client registerd')) + .then(() => logger.info('New client registered!')) .catch((error) => logger.error('Error registering client', error)); - res.end(); + res.status(202).end(); }); }); @@ -98,6 +72,6 @@ module.exports = function (app, config) { app.get('/client/instances', (req, res) => { clientInstanceStore.getAll() .then(data => res.json(data)) - .catch(err => console.error(err)); + .catch(err => logger.error(err)); }); }; diff --git a/test/unit/routes/feature.test.js b/test/unit/routes/feature.test.js index 671b8226f0..5ab93cde85 100644 --- a/test/unit/routes/feature.test.js +++ b/test/unit/routes/feature.test.js @@ -1,32 +1,23 @@ 'use strict'; -const clientMetricsStore = require('./mocks/fake-metrics-store'); -const featureToggleStore = require('./mocks/fake-feature-toggle-store'); -const strategyStore = require('./mocks/fake-strategies-store'); +const store = require('./mocks/store'); const supertest = require('supertest'); const assert = require('assert'); -const sinon = require('sinon'); + let request; +let featureToggleStore; describe('Unit: The features api', () => { beforeEach(done => { - featureToggleStore.reset(); - + const stores = store.createStores(); const app = require('../../../app')({ baseUriPath: '', - stores: { - db: sinon.stub(), - eventStore: sinon.stub(), - featureToggleStore, - clientMetricsStore, - strategyStore, - clientStrategyStore: sinon.stub(), - clientInstanceStore: sinon.stub(), - }, + stores: stores, }); + featureToggleStore = stores.featureToggleStore; request = supertest(app); done(); }); diff --git a/test/unit/routes/metrics.test.js b/test/unit/routes/metrics.test.js index fd012e7296..227ce4d0ff 100644 --- a/test/unit/routes/metrics.test.js +++ b/test/unit/routes/metrics.test.js @@ -1,28 +1,17 @@ 'use strict'; -const clientMetricsStore = require('./mocks/fake-metrics-store'); -const clientStrategyStore = require('./mocks/fake-client-strategy-store'); -const clientInstanceStore = require('./mocks/fake-client-instance-store'); - +const store = require('./mocks/store'); const supertest = require('supertest'); const assert = require('assert'); -const sinon = require('sinon'); let request; describe('Unit: The metrics api', () => { beforeEach(done => { + const stores = store.createStores(); const app = require('../../../app')({ baseUriPath: '', - stores: { - db: sinon.stub(), - eventStore: sinon.stub(), - featureToggleStore: sinon.stub(), - clientMetricsStore, - strategyStore: sinon.stub(), - clientStrategyStore, - clientInstanceStore, - }, + stores: stores, }); request = supertest(app); @@ -32,8 +21,14 @@ describe('Unit: The metrics api', () => { it('should register client', (done) => { request .post('/api/client/register') - .send({ appName: 'demo', instanceId: 'test', strategies: ['default'], started: Date.now(), interval: 10 }) - .expect(200, done); + .send({ + appName: 'demo', + instanceId: 'test', + strategies: ['default'], + started: Date.now(), + interval: 10 + }) + .expect(202, done); }); it('should require appName field', (done) => { @@ -41,4 +36,33 @@ describe('Unit: The metrics api', () => { .post('/api/client/register') .expect(400, done) }); + + it('should require strategies field', (done) => { + request + .post('/api/client/register') + .send({ + appName: 'demo', + instanceId: 'test', + //strategies: ['default'], + started: Date.now(), + interval: 10 + }) + .expect(400, done) + }); + + + it('should accept client metrics', (done) => { + request + .post('/api/client/metrics') + .send({ + appName: 'demo', + instanceId: '1', + bucket: { + start: Date.now(), + stop: Date.now(), + toggles: {} + } + }) + .expect(202, done) + }); }); diff --git a/test/unit/routes/mocks/fake-client-instance-store.js b/test/unit/routes/mocks/fake-client-instance-store.js index e57c13425c..954a2e2ab2 100644 --- a/test/unit/routes/mocks/fake-client-instance-store.js +++ b/test/unit/routes/mocks/fake-client-instance-store.js @@ -1,5 +1,6 @@ 'use strict'; module.exports = { + reset: () => {}, insert: () => Promise.resolve(), }; diff --git a/test/unit/routes/mocks/fake-client-strategy-store.js b/test/unit/routes/mocks/fake-client-strategy-store.js index e57c13425c..954a2e2ab2 100644 --- a/test/unit/routes/mocks/fake-client-strategy-store.js +++ b/test/unit/routes/mocks/fake-client-strategy-store.js @@ -1,5 +1,6 @@ 'use strict'; module.exports = { + reset: () => {}, insert: () => Promise.resolve(), }; diff --git a/test/unit/routes/mocks/fake-metrics-store.js b/test/unit/routes/mocks/fake-metrics-store.js index 94c134f068..6f43ad12ed 100644 --- a/test/unit/routes/mocks/fake-metrics-store.js +++ b/test/unit/routes/mocks/fake-metrics-store.js @@ -1,5 +1,7 @@ 'use strict'; module.exports = { + reset: () => {}, getMetricsLastHour: () => Promise.resolve([]), + insert: () => Promise.resolve(), }; diff --git a/test/unit/routes/mocks/store.js b/test/unit/routes/mocks/store.js new file mode 100644 index 0000000000..05f1400785 --- /dev/null +++ b/test/unit/routes/mocks/store.js @@ -0,0 +1,27 @@ +const sinon = require('sinon'); + +const clientMetricsStore = require('./fake-metrics-store'); +const clientStrategyStore = require('./fake-client-strategy-store'); +const clientInstanceStore = require('./fake-client-instance-store'); +const featureToggleStore = require('./fake-feature-toggle-store'); +const strategyStore = require('./fake-strategies-store'); + +module.exports = { + createStores: () => { + clientMetricsStore.reset(); + clientStrategyStore.reset(); + clientInstanceStore.reset(); + featureToggleStore.reset(); + strategyStore.reset(); + + return { + db: sinon.stub(), + clientMetricsStore, + clientStrategyStore, + clientInstanceStore, + featureToggleStore, + strategyStore, + } + + } +}; \ No newline at end of file diff --git a/test/unit/routes/strategies.test.js b/test/unit/routes/strategies.test.js index 87a0526a4b..463ec338d4 100644 --- a/test/unit/routes/strategies.test.js +++ b/test/unit/routes/strategies.test.js @@ -1,32 +1,23 @@ 'use strict'; -const clientMetricsStore = require('./mocks/fake-metrics-store'); -const featureToggleStore = require('./mocks/fake-feature-toggle-store'); -const strategyStore = require('./mocks/fake-strategies-store'); +const store = require('./mocks/store'); + const supertest = require('supertest'); const assert = require('assert'); const sinon = require('sinon'); let request; +let strategyStore; describe('Unit: The strategies api', () => { beforeEach(done => { - strategyStore.reset(); - + const stores = store.createStores(); const app = require('../../../app')({ baseUriPath: '', - stores: { - db: sinon.stub(), - eventStore: sinon.stub(), - featureToggleStore, - clientMetricsStore, - strategyStore, - clientStrategyStore: sinon.stub(), - clientInstanceStore: sinon.stub(), - }, + stores: stores, }); - + strategyStore = stores.strategyStore; request = supertest(app); done(); });