From 80d3f5be1c6708682578de4289ccd9b26938086e Mon Sep 17 00:00:00 2001 From: sveisvei Date: Tue, 6 Dec 2016 09:19:15 +0100 Subject: [PATCH 1/5] client applications --- lib/client-metrics/index.js | 2 +- lib/db/client-applications-store.js | 74 +++++++++++++++++++ lib/db/index.js | 2 + lib/routes/metrics.js | 74 +++++++++++++------ ...161205203516-create-client-applications.js | 3 + .../011-create-client-applications.down.sql | 1 + .../sql/011-create-client-applications.up.sql | 9 +++ 7 files changed, 143 insertions(+), 22 deletions(-) create mode 100644 lib/db/client-applications-store.js create mode 100644 migrations/20161205203516-create-client-applications.js create mode 100644 migrations/sql/011-create-client-applications.down.sql create mode 100644 migrations/sql/011-create-client-applications.up.sql diff --git a/lib/client-metrics/index.js b/lib/client-metrics/index.js index be25954b9b..3067faf45c 100644 --- a/lib/client-metrics/index.js +++ b/lib/client-metrics/index.js @@ -54,7 +54,7 @@ module.exports = class UnleashClientMetrics { if (!toggles[seenToggleName]) { toggles[seenToggleName] = []; } - toggles[seenToggleName].push(appName); + toggles[seenToggleName].push({ appName }); }); }); return toggles; diff --git a/lib/db/client-applications-store.js b/lib/db/client-applications-store.js new file mode 100644 index 0000000000..31ba7c5bf3 --- /dev/null +++ b/lib/db/client-applications-store.js @@ -0,0 +1,74 @@ +'use strict'; + +const COLUMNS = ['app_name', 'created_at', 'updated_at', 'description', 'url', 'color', 'icon']; +const TABLE = 'client_applications'; + +const mapRow = (row) => ({ + appName: row.app_name, + createdAt: row.created_at, + updatedAt: row.updated_at, + description: row.description, + url: row.url, + color: row.color, + icon: row.icon, +}); + +const remapRow = (input, old = {}) => ({ + app_name: input.appName, + updated_at: input.updatedAt, + description: input.description || old.description, + url: input.url || old.url, + color: input.color || old.color, + icon: input.icon || old.icon, +}); + + +class ClientApplicationsDb { + constructor (db) { + this.db = db; + } + + updateRow (details, prev) { + details.updatedAt = 'now()'; + return this.db(TABLE) + .where('app_name', details.appName) + .update(remapRow(details, prev)); + } + + insertNewRow (details) { + return this.db(TABLE).insert(remapRow(details)); + } + + upsert (data) { + if (!data) { + throw new Error('Missing data to add / update'); + } + return this.db(TABLE) + .select(COLUMNS) + .where('app_name', data.appName) + .then(result => { + if (result && result[0]) { + return this.updateRow(data, result[0]); + } else { + return this.insertNewRow(data); + } + }); + } + + getApplicationMetaData (appName) { + if (appName) { + return this.db + .select(COLUMNS) + .where('app_name', appName) + .from(TABLE) + .map(mapRow); + } + return this.db + .select(COLUMNS) + .from(TABLE) + .orderBy('created_at', 'asc') + .map(mapRow); + } +}; + +module.exports = ClientApplicationsDb; diff --git a/lib/db/index.js b/lib/db/index.js index 738fc9acea..7f880d27c9 100644 --- a/lib/db/index.js +++ b/lib/db/index.js @@ -8,6 +8,7 @@ const ClientInstanceStore = require('./client-instance-store'); const ClientMetricsDb = require('./client-metrics-db'); const ClientMetricsStore = require('./client-metrics-store'); const ClientStrategyStore = require('./client-strategy-store'); +const ClientApplicationsStore = require('./client-applications-store'); module.exports.createStores = (config) => { const db = createDb(config); @@ -19,6 +20,7 @@ module.exports.createStores = (config) => { eventStore, featureToggleStore: new FeatureToggleStore(db, eventStore), strategyStore: new StrategyStore(db, eventStore), + clientApplicationsStore: new ClientApplicationsStore(db), clientInstanceStore: new ClientInstanceStore(db), clientMetricsStore: new ClientMetricsStore(clientMetricsDb), clientStrategyStore: new ClientStrategyStore(db), diff --git a/lib/routes/metrics.js b/lib/routes/metrics.js index 1a575c94c4..2c5bd471fb 100644 --- a/lib/routes/metrics.js +++ b/lib/routes/metrics.js @@ -11,6 +11,7 @@ module.exports = function (app, config) { clientMetricsStore, clientStrategyStore, clientInstanceStore, + clientApplicationsStore, } = config.stores; const metrics = new ClientMetrics(clientMetricsStore); @@ -22,7 +23,19 @@ module.exports = function (app, config) { app.get('/client/seen-apps', (req, res) => { const seenApps = metrics.getSeenAppsPerToggle(); - res.json(seenApps); + clientApplicationsStore.getApplicationMetaData() + .then(toLookup) + .then(metaData => { + Object.keys(seenApps).forEach(key => { + seenApps[key] = seenApps[key].map(entry => { + if (metaData[entry.appName]) { + entry.data = metaData[entry.appName]; + } + return entry; + }); + }); + res.json(seenApps); + }); }); app.get('/client/metrics/feature-toggles', (req, res) => { @@ -99,27 +112,43 @@ module.exports = function (app, config) { } }); + app.post('/client/applications/:appName', (req, res) => { + const input = Object.assign({}, req.body, { + appName: req.params.appName, + }); + clientApplicationsStore + .upsert(input) + .then(() => res.status(202).end()) + .catch((e) => { + logger.error(e); + res.status(500).end(); + }); + }); + + function toLookup (metaData) { + return metaData.reduce((result, entry) => { + result[entry.appName] = entry; + return result; + }, {}); + } + app.get('/client/applications/', (req, res) => { const strategyName = req.query.strategyName; - let appsPromise; - - if (strategyName) { - appsPromise = clientStrategyStore.getAppsForStrategy(strategyName); - } else { - appsPromise = clientStrategyStore.getApplications(); - } - - appsPromise - .then(apps => { - const applications = apps.map(appName => ({ - appName, - links: { - appDetails: `/api/client/applications/${appName}`, - }, - })); - res.json({ applications }); - }) - .catch(err => catchLogAndSendErrorResponse(err, res)); + Promise.all([ + strategyName ? clientStrategyStore.getAppsForStrategy(strategyName) : clientStrategyStore.getApplications(), + clientApplicationsStore.getApplicationMetaData().then(toLookup), + ]) + .then(([apps, metaData]) => { + const applications = apps.map(({ appName }) => ({ + appName, + data: metaData[appName], + links: { + appDetails: `/api/client/applications/${appName}`, + }, + })); + res.json({ applications }); + }) + .catch(err => catchLogAndSendErrorResponse(err, res)); }); app.get('/client/applications/:appName', (req, res) => { @@ -128,8 +157,11 @@ module.exports = function (app, config) { Promise.all([ clientInstanceStore.getByAppName(appName), clientStrategyStore.getByAppName(appName), + clientApplicationsStore.getApplicationMetaData(appName), ]) - .then(([instances, strategies]) => res.json({ appName, instances, strategies, seenToggles })) + .then(([instances, strategies, [metaData]]) => + res.json({ appName, instances, strategies, seenToggles, data: metaData }) + ) .catch(err => catchLogAndSendErrorResponse(err, res)); }); }; diff --git a/migrations/20161205203516-create-client-applications.js b/migrations/20161205203516-create-client-applications.js new file mode 100644 index 0000000000..fe15cbefe1 --- /dev/null +++ b/migrations/20161205203516-create-client-applications.js @@ -0,0 +1,3 @@ +'use strict'; + +module.exports = require('../scripts/migration-runner').create('011-create-client-applications'); diff --git a/migrations/sql/011-create-client-applications.down.sql b/migrations/sql/011-create-client-applications.down.sql new file mode 100644 index 0000000000..2dabc37181 --- /dev/null +++ b/migrations/sql/011-create-client-applications.down.sql @@ -0,0 +1 @@ +DROP TABLE client_applications; diff --git a/migrations/sql/011-create-client-applications.up.sql b/migrations/sql/011-create-client-applications.up.sql new file mode 100644 index 0000000000..0b6aa15134 --- /dev/null +++ b/migrations/sql/011-create-client-applications.up.sql @@ -0,0 +1,9 @@ +CREATE TABLE client_applications ( + app_name varchar(255) PRIMARY KEY NOT NULL, + created_at timestamp default now(), + updated_at timestamp default now(), + description varchar(255), + icon varchar(255), + url varchar(255), + color varchar(255) +); From 50f3cf9a826cbd5d053c7a393e6ad5e08c46acb8 Mon Sep 17 00:00:00 2001 From: sveisvei Date: Tue, 6 Dec 2016 09:27:32 +0100 Subject: [PATCH 2/5] fix lint and mock --- lib/db/client-applications-store.js | 1 + test/unit/routes/fixtures/fake-client-applications-store.js | 6 ++++++ test/unit/routes/fixtures/store.js | 2 ++ 3 files changed, 9 insertions(+) create mode 100644 test/unit/routes/fixtures/fake-client-applications-store.js diff --git a/lib/db/client-applications-store.js b/lib/db/client-applications-store.js index 31ba7c5bf3..bfbb08373c 100644 --- a/lib/db/client-applications-store.js +++ b/lib/db/client-applications-store.js @@ -1,3 +1,4 @@ +/* eslint camelcase:off */ 'use strict'; const COLUMNS = ['app_name', 'created_at', 'updated_at', 'description', 'url', 'color', 'icon']; diff --git a/test/unit/routes/fixtures/fake-client-applications-store.js b/test/unit/routes/fixtures/fake-client-applications-store.js new file mode 100644 index 0000000000..f400038154 --- /dev/null +++ b/test/unit/routes/fixtures/fake-client-applications-store.js @@ -0,0 +1,6 @@ +'use strict'; + +module.exports = () => ({ + upsert: () => Promise.resolve(), + getApplicationMetaData: () => Promise.resolve([]), +}); diff --git a/test/unit/routes/fixtures/store.js b/test/unit/routes/fixtures/store.js index 3e24840288..daef06f189 100644 --- a/test/unit/routes/fixtures/store.js +++ b/test/unit/routes/fixtures/store.js @@ -3,6 +3,7 @@ const ClientMetricsStore = require('./fake-metrics-store'); const clientStrategyStore = require('./fake-client-strategy-store'); const clientInstanceStore = require('./fake-client-instance-store'); +const clientApplicationsStore = require('./fake-client-applications-store'); const featureToggleStore = require('./fake-feature-toggle-store'); const eventStore = require('./fake-event-store'); const strategyStore = require('./fake-strategies-store'); @@ -19,6 +20,7 @@ module.exports = { return { db, + clientApplicationsStore: clientApplicationsStore(), clientMetricsStore: new ClientMetricsStore(), clientStrategyStore: clientStrategyStore(), clientInstanceStore: clientInstanceStore(), From ac846b9879198b75152aa789e0c87b8a82b3a7d3 Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Fri, 9 Dec 2016 16:02:06 +0100 Subject: [PATCH 3/5] Add client_applications table --- ...161205203516-create-client-applications.js | 19 ++++++++++++++++++- .../011-create-client-applications.down.sql | 1 - .../sql/011-create-client-applications.up.sql | 9 --------- 3 files changed, 18 insertions(+), 11 deletions(-) delete mode 100644 migrations/sql/011-create-client-applications.down.sql delete mode 100644 migrations/sql/011-create-client-applications.up.sql diff --git a/migrations/20161205203516-create-client-applications.js b/migrations/20161205203516-create-client-applications.js index fe15cbefe1..e665fda6c3 100644 --- a/migrations/20161205203516-create-client-applications.js +++ b/migrations/20161205203516-create-client-applications.js @@ -1,3 +1,20 @@ +/* eslint camelcase: "off" */ 'use strict'; -module.exports = require('../scripts/migration-runner').create('011-create-client-applications'); +exports.up = function (db, cb) { + db.createTable('client_applications', { + app_name: { type: 'varchar', length: 255, primaryKey: true, notNull: true }, + created_at: { type: 'timestamp', defaultValue: 'now()' }, + updated_at: { type: 'timestamp', defaultValue: 'now()' }, + seen_at: { type: 'timestamp' }, + strategies: { type: 'json' }, + description: { type: 'varchar', length: 255 }, + icon: { type: 'varchar', length: 255 }, + url: { type: 'varchar', length: 255 }, + color: { type: 'varchar', length: 255 }, + }, cb); +}; + +exports.down = function (db, cb) { + return db.dropTable('client_applications', cb); +}; diff --git a/migrations/sql/011-create-client-applications.down.sql b/migrations/sql/011-create-client-applications.down.sql deleted file mode 100644 index 2dabc37181..0000000000 --- a/migrations/sql/011-create-client-applications.down.sql +++ /dev/null @@ -1 +0,0 @@ -DROP TABLE client_applications; diff --git a/migrations/sql/011-create-client-applications.up.sql b/migrations/sql/011-create-client-applications.up.sql deleted file mode 100644 index 0b6aa15134..0000000000 --- a/migrations/sql/011-create-client-applications.up.sql +++ /dev/null @@ -1,9 +0,0 @@ -CREATE TABLE client_applications ( - app_name varchar(255) PRIMARY KEY NOT NULL, - created_at timestamp default now(), - updated_at timestamp default now(), - description varchar(255), - icon varchar(255), - url varchar(255), - color varchar(255) -); From 46c8d83dc178d51d4546ef3459aec60f38c4a3da Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Fri, 9 Dec 2016 16:25:18 +0100 Subject: [PATCH 4/5] A client-register should upsert client_applications table --- lib/db/client-applications-store.js | 5 ++++- lib/routes/metrics.js | 17 +++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/db/client-applications-store.js b/lib/db/client-applications-store.js index bfbb08373c..6f4c4eec8f 100644 --- a/lib/db/client-applications-store.js +++ b/lib/db/client-applications-store.js @@ -1,7 +1,8 @@ /* eslint camelcase:off */ 'use strict'; -const COLUMNS = ['app_name', 'created_at', 'updated_at', 'description', 'url', 'color', 'icon']; +const COLUMNS = [ + 'app_name', 'created_at', 'updated_at', 'description', 'strategies', 'url', 'color', 'icon']; const TABLE = 'client_applications'; const mapRow = (row) => ({ @@ -9,6 +10,7 @@ const mapRow = (row) => ({ createdAt: row.created_at, updatedAt: row.updated_at, description: row.description, + strategies: row.strategies, url: row.url, color: row.color, icon: row.icon, @@ -21,6 +23,7 @@ const remapRow = (input, old = {}) => ({ url: input.url || old.url, color: input.color || old.color, icon: input.icon || old.icon, + strategies: JSON.stringify(input.strategies || old.strategies), }); diff --git a/lib/routes/metrics.js b/lib/routes/metrics.js index 2c5bd471fb..52b921c39c 100644 --- a/lib/routes/metrics.js +++ b/lib/routes/metrics.js @@ -77,22 +77,19 @@ module.exports = function (app, config) { app.post('/client/register', (req, res) => { const data = req.body; - const clientIp = req.ip; - joi.validate(data, clientRegisterSchema, (err, cleaned) => { + joi.validate(data, clientRegisterSchema, (err, clientRegistration) => { if (err) { return res.status(400).json(err); } - clientStrategyStore - .insert(cleaned.appName, cleaned.strategies) - .then(() => clientInstanceStore.insert({ - appName: cleaned.appName, - instanceId: cleaned.instanceId, - clientIp, - })) + clientRegistration.clientIp = req.ip; + + clientApplicationsStore + .upsert(clientRegistration) + .then(() => clientInstanceStore.insert(clientRegistration)) .then(() => logger.info(`New client registered with - appName=${cleaned.appName} and instanceId=${cleaned.instanceId}`)) + appName=${clientRegistration.appName} and instanceId=${clientRegistration.instanceId}`)) .catch(err => logger.error('failed to register client', err)); res.status(202).end(); From ab3694cc94c50448463e0d9b9ebd09ae7856838c Mon Sep 17 00:00:00 2001 From: ivaosthu Date: Fri, 9 Dec 2016 17:30:12 +0100 Subject: [PATCH 5/5] Remove client_strategies table We can just have a strategies column in the client_applications table. This solves all our needs, and thus avoids the need for an extra table. --- docs/api/metrics-api.md | 26 +++--- docs/developer-guide.md | 11 ++- lib/db/client-applications-store.js | 44 +++++++--- lib/db/client-instance-store.js | 2 +- lib/db/client-strategy-store.js | 86 ------------------- lib/db/index.js | 2 - lib/routes/metrics.js | 72 ++++++++-------- ...20161102212415-create-client-strategies.js | 3 - .../sql/009-create-client-strategies.down.sql | 2 - .../sql/009-create-client-strategies.up.sql | 6 -- test/e2e/helpers/test-helper.js | 20 +++-- test/e2e/metrics-api.test.js | 18 +--- .../fake-client-applications-store.js | 2 +- .../fixtures/fake-client-strategy-store.js | 14 --- test/unit/routes/fixtures/store.js | 2 - test/unit/routes/metrics.test.js | 12 --- 16 files changed, 108 insertions(+), 214 deletions(-) delete mode 100644 lib/db/client-strategy-store.js delete mode 100644 migrations/20161102212415-create-client-strategies.js delete mode 100644 migrations/sql/009-create-client-strategies.down.sql delete mode 100644 migrations/sql/009-create-client-strategies.up.sql delete mode 100644 test/unit/routes/fixtures/fake-client-strategy-store.js diff --git a/docs/api/metrics-api.md b/docs/api/metrics-api.md index da58a96e3e..f44a1f13ba 100644 --- a/docs/api/metrics-api.md +++ b/docs/api/metrics-api.md @@ -157,21 +157,27 @@ a link to follow for more datails. { "applications": [ { - "appName": "test", + "appName": "another", + "strategies": [ + "default", + "other", + "brother" + ], + "createdAt": "2016-12-09T14:56:36.730Z", "links": { - "appDetails": "/api/client/applications/test" + "appDetails": "/api/client/applications/another" } }, { - "appName": "demo-app-2", + "appName": "bow", + "strategies": [ + "default", + "other", + "brother" + ], + "createdAt": "2016-12-09T14:56:36.730Z", "links": { - "appDetails": "/api/client/applications/demo-app-2" - } - }, - { - "appName": "demo-app", - "links": { - "appDetails": "/api/client/applications/demo-app" + "appDetails": "/api/client/applications/bow" } } ] diff --git a/docs/developer-guide.md b/docs/developer-guide.md index b8dd3d58ed..67903fb22a 100644 --- a/docs/developer-guide.md +++ b/docs/developer-guide.md @@ -60,10 +60,13 @@ We use database migrations to track database changes. ### Making a schema change -1. Create `migrations/sql/NNN-your-migration-name.up.sql` with your change in SQL. -2. Create `migrations/sql/NNN-your-migration-name.down.sql` with the rollback of your change in SQL. -3. Run `db-migrate create your-migration-name` and edit the generated file to have this line: `module.exports = require('../scripts/migration-runner').create('NNN-your-migration-name');` -4. Run `db-migrate up`. +Use db-migrate to create new migrations file. + +```bash +> ./node_modules/.bin/db-migrate create your-migration-name +``` + + ## Publishing / Releasing new packages diff --git a/lib/db/client-applications-store.js b/lib/db/client-applications-store.js index 6f4c4eec8f..ff2d95751f 100644 --- a/lib/db/client-applications-store.js +++ b/lib/db/client-applications-store.js @@ -1,8 +1,8 @@ /* eslint camelcase:off */ 'use strict'; -const COLUMNS = [ - 'app_name', 'created_at', 'updated_at', 'description', 'strategies', 'url', 'color', 'icon']; +const COLUMNS = ['app_name', 'created_at', 'updated_at', 'description', 'strategies', 'url', 'color', 'icon']; +const COLUMNS_LIST = ['app_name', 'created_at', 'strategies']; const TABLE = 'client_applications'; const mapRow = (row) => ({ @@ -59,19 +59,43 @@ class ClientApplicationsDb { }); } - getApplicationMetaData (appName) { - if (appName) { - return this.db + getAll () { + return this.db + .select(COLUMNS_LIST) + .from(TABLE) + .map(mapRow); + } + + getApplication (appName) { + return this.db .select(COLUMNS) .where('app_name', appName) .from(TABLE) - .map(mapRow); - } + .map(mapRow) + .then(list => list[0]); + } + + /** + * Could also be done in SQL: + * (not sure if it is faster though) + * + * SELECT app_name from ( + * SELECT app_name, json_array_elements(strategies)::text as strategyName from client_strategies + * ) as foo + * WHERE foo.strategyName = '"other"'; + */ + getAppsForStrategy (strategyName) { return this.db - .select(COLUMNS) + .select(COLUMNS_LIST) .from(TABLE) - .orderBy('created_at', 'asc') - .map(mapRow); + .map(mapRow) + .then(apps => apps + .filter(app => app.strategies.includes(strategyName))); + } + + getApplications (filter) { + return filter && filter.strategyName ? + this.getAppsForStrategy(filter.strategyName) : this.getAll(); } }; diff --git a/lib/db/client-instance-store.js b/lib/db/client-instance-store.js index 5e946b3c15..6f99285a22 100644 --- a/lib/db/client-instance-store.js +++ b/lib/db/client-instance-store.js @@ -76,7 +76,7 @@ class ClientInstanceStore { getByAppName (appName) { return this.db - .select(COLUMNS) + .select() .from(TABLE) .where('app_name', appName) .orderBy('last_seen', 'desc') diff --git a/lib/db/client-strategy-store.js b/lib/db/client-strategy-store.js deleted file mode 100644 index 68ae5d049e..0000000000 --- a/lib/db/client-strategy-store.js +++ /dev/null @@ -1,86 +0,0 @@ -'use strict'; - -const COLUMNS = ['app_name', 'strategies']; -const TABLE = 'client_strategies'; - -const mapRow = (row) => ({ - appName: row.app_name, - strategies: row.strategies, -}); - -class ClientStrategyStore { - constructor (db) { - this.db = db; - } - - updateRow (appName, strategies) { - return this.db(TABLE) - .where('app_name', appName) // eslint-disable-line - .update({ - strategies: JSON.stringify(strategies), - updated_at: 'now()', // eslint-disable-line - }); - } - - insertNewRow (appName, strategies) { - return this.db(TABLE).insert({ - app_name: appName, // eslint-disable-line - strategies: JSON.stringify(strategies), - }); - } - - insert (appName, strategies) { - return this.db(TABLE) - .count('*') - .where('app_name', appName) - .map(row => ({ count: row.count })) - .then(rows => { - if (rows[0].count > 0) { - return this.updateRow(appName, strategies); - } else { - return this.insertNewRow(appName, strategies); - } - }); - } - - getAll () { - return this.db - .select(COLUMNS) - .from(TABLE) - .map(mapRow); - } - - getByAppName (appName) { - return this.db - .select('strategies') - .where('app_name', appName) - .from(TABLE) - .map((row) => row.strategies) - .then(arr => arr[0]); - } - - /** - * Could also be done in SQL: - * (not sure if it is faster though) - * - * SELECT app_name from ( - * SELECT app_name, json_array_elements(strategies)::text as strategyName from client_strategies - * ) as foo - * WHERE foo.strategyName = '"other"'; - */ - getAppsForStrategy (strategyName) { - return this.getAll() - .then(apps => apps - .filter(app => app.strategies.includes(strategyName)) - .map(app => app.appName)); - } - - getApplications () { - return this.db - .select('app_name') - .from(TABLE) - .map((row) => row.app_name); - } -}; - -module.exports = ClientStrategyStore; diff --git a/lib/db/index.js b/lib/db/index.js index 7f880d27c9..c7f196ddef 100644 --- a/lib/db/index.js +++ b/lib/db/index.js @@ -7,7 +7,6 @@ const StrategyStore = require('./strategy-store'); const ClientInstanceStore = require('./client-instance-store'); const ClientMetricsDb = require('./client-metrics-db'); const ClientMetricsStore = require('./client-metrics-store'); -const ClientStrategyStore = require('./client-strategy-store'); const ClientApplicationsStore = require('./client-applications-store'); module.exports.createStores = (config) => { @@ -23,6 +22,5 @@ module.exports.createStores = (config) => { clientApplicationsStore: new ClientApplicationsStore(db), clientInstanceStore: new ClientInstanceStore(db), clientMetricsStore: new ClientMetricsStore(clientMetricsDb), - clientStrategyStore: new ClientStrategyStore(db), }; }; diff --git a/lib/routes/metrics.js b/lib/routes/metrics.js index 52b921c39c..d618aca4c4 100644 --- a/lib/routes/metrics.js +++ b/lib/routes/metrics.js @@ -9,7 +9,6 @@ const { catchLogAndSendErrorResponse } = require('./route-utils'); module.exports = function (app, config) { const { clientMetricsStore, - clientStrategyStore, clientInstanceStore, clientApplicationsStore, } = config.stores; @@ -23,7 +22,7 @@ module.exports = function (app, config) { app.get('/client/seen-apps', (req, res) => { const seenApps = metrics.getSeenAppsPerToggle(); - clientApplicationsStore.getApplicationMetaData() + clientApplicationsStore.getApplications() .then(toLookup) .then(metaData => { Object.keys(seenApps).forEach(key => { @@ -96,19 +95,6 @@ module.exports = function (app, config) { }); }); - app.get('/client/strategies', (req, res) => { - const appName = req.query.appName; - if (appName) { - clientStrategyStore.getByAppName(appName) - .then(data => res.json(data)) - .catch(err => catchLogAndSendErrorResponse(err, res)); - } else { - clientStrategyStore.getAll() - .then(data => res.json(data)) - .catch(err => catchLogAndSendErrorResponse(err, res)); - } - }); - app.post('/client/applications/:appName', (req, res) => { const input = Object.assign({}, req.body, { appName: req.params.appName, @@ -130,35 +116,47 @@ module.exports = function (app, config) { } app.get('/client/applications/', (req, res) => { - const strategyName = req.query.strategyName; - Promise.all([ - strategyName ? clientStrategyStore.getAppsForStrategy(strategyName) : clientStrategyStore.getApplications(), - clientApplicationsStore.getApplicationMetaData().then(toLookup), - ]) - .then(([apps, metaData]) => { - const applications = apps.map(({ appName }) => ({ - appName, - data: metaData[appName], - links: { - appDetails: `/api/client/applications/${appName}`, - }, - })); - res.json({ applications }); - }) - .catch(err => catchLogAndSendErrorResponse(err, res)); + clientApplicationsStore + .getApplications(req.query) + .then(apps => { + const applications = apps.map(({ appName, createdAt, strategies }) => ({ + appName, + strategies, + createdAt, + links: { + appDetails: `/api/client/applications/${appName}`, + }, + })); + res.json({ applications }); + }) + .catch(err => catchLogAndSendErrorResponse(err, res)); }); app.get('/client/applications/:appName', (req, res) => { const appName = req.params.appName; const seenToggles = metrics.getSeenTogglesByAppName(appName); + Promise.all([ + clientApplicationsStore.getApplication(appName), clientInstanceStore.getByAppName(appName), - clientStrategyStore.getByAppName(appName), - clientApplicationsStore.getApplicationMetaData(appName), ]) - .then(([instances, strategies, [metaData]]) => - res.json({ appName, instances, strategies, seenToggles, data: metaData }) - ) - .catch(err => catchLogAndSendErrorResponse(err, res)); + .then(([application, instances]) => { + const appDetails = { + appName: application.appName, + createdAt: application.createdAt, + description: application.description, + url: application.url, + color: application.color, + icon: application.icon, + strategies: application.strategies, + instances, + seenToggles, + links: { + self: `/api/client/applications/${application.appName}`, + }, + }; + res.json(appDetails); + }) + .catch(err => catchLogAndSendErrorResponse(err, res)); }); }; diff --git a/migrations/20161102212415-create-client-strategies.js b/migrations/20161102212415-create-client-strategies.js deleted file mode 100644 index 6a6ef7f26a..0000000000 --- a/migrations/20161102212415-create-client-strategies.js +++ /dev/null @@ -1,3 +0,0 @@ -'use strict'; - -module.exports = require('../scripts/migration-runner').create('009-create-client-strategies'); diff --git a/migrations/sql/009-create-client-strategies.down.sql b/migrations/sql/009-create-client-strategies.down.sql deleted file mode 100644 index 85f83c3d35..0000000000 --- a/migrations/sql/009-create-client-strategies.down.sql +++ /dev/null @@ -1,2 +0,0 @@ ---create client_strategies -DROP TABLE client_strategies; diff --git a/migrations/sql/009-create-client-strategies.up.sql b/migrations/sql/009-create-client-strategies.up.sql deleted file mode 100644 index 27e412b957..0000000000 --- a/migrations/sql/009-create-client-strategies.up.sql +++ /dev/null @@ -1,6 +0,0 @@ ---create new client_strategies table -CREATE TABLE client_strategies ( - app_name varchar(255) PRIMARY KEY NOT NULL, - updated_at timestamp default now(), - strategies json -); diff --git a/test/e2e/helpers/test-helper.js b/test/e2e/helpers/test-helper.js index 34bf24c604..958e49ddb3 100644 --- a/test/e2e/helpers/test-helper.js +++ b/test/e2e/helpers/test-helper.js @@ -60,22 +60,24 @@ function createStrategies (stores) { ].map(strategy => stores.strategyStore._createStrategy(strategy)); } -function createClientStrategy (stores) { +function createApplications (stores) { return [ { - appName: 'demo-sed', - instanceId: 'test-1', + appName: 'demo-app-1', strategies: ['default'], - started: Date.now(), - interval: 10, }, - ].map(client => stores.clientStrategyStore.insert(client)); + { + appName: 'demo-app-2', + strategies: ['default', 'extra'], + description: 'hello', + }, + ].map(client => stores.clientApplicationsStore.upsert(client)); } function createClientInstance (stores) { return [ { - appName: 'demo-seed', + appName: 'demo-app-1', instanceId: 'test-1', strategies: ['default'], started: Date.now(), @@ -159,7 +161,7 @@ function resetDatabase (stores) { return Promise.all([ stores.db('strategies').del(), stores.db('features').del(), - stores.db('client_strategies').del(), + stores.db('client_applications').del(), stores.db('client_instances').del(), ]); } @@ -169,7 +171,7 @@ function setupDatabase (stores) { createStrategies(stores) .concat(createFeatures(stores) .concat(createClientInstance(stores)) - .concat(createClientStrategy(stores)))); + .concat(createApplications(stores)))); } module.exports = { diff --git a/test/e2e/metrics-api.test.js b/test/e2e/metrics-api.test.js index 9f3562c0df..585426f29f 100644 --- a/test/e2e/metrics-api.test.js +++ b/test/e2e/metrics-api.test.js @@ -60,26 +60,14 @@ test.serial('should accept client metrics', async t => { .then(destroy); }); -test.serial('should get client strategies', async t => { - const { request, destroy } = await setupApp('metrics_serial'); - return request - .get('/api/client/strategies') - .expect('Content-Type', /json/) - .expect((res) => { - t.true(res.status === 200); - t.true(res.body.length === 1); - }) - .then(destroy); -}); - test.serial('should get application details', async t => { const { request, destroy } = await setupApp('metrics_serial'); return request - .get('/api/client/applications/demo-seed') + .get('/api/client/applications/demo-app-1') .expect('Content-Type', /json/) .expect((res) => { t.true(res.status === 200); - t.true(res.body.appName === 'demo-seed'); + t.true(res.body.appName === 'demo-app-1'); t.true(res.body.instances.length === 1); }) .then(destroy); @@ -92,7 +80,7 @@ test.serial('should get list of applications', async t => { .expect('Content-Type', /json/) .expect((res) => { t.true(res.status === 200); - t.true(res.body.applications.length === 1); + t.true(res.body.applications.length === 2); }) .then(destroy); }); diff --git a/test/unit/routes/fixtures/fake-client-applications-store.js b/test/unit/routes/fixtures/fake-client-applications-store.js index f400038154..7d5d49ebfd 100644 --- a/test/unit/routes/fixtures/fake-client-applications-store.js +++ b/test/unit/routes/fixtures/fake-client-applications-store.js @@ -2,5 +2,5 @@ module.exports = () => ({ upsert: () => Promise.resolve(), - getApplicationMetaData: () => Promise.resolve([]), + getApplications: () => Promise.resolve([]), }); diff --git a/test/unit/routes/fixtures/fake-client-strategy-store.js b/test/unit/routes/fixtures/fake-client-strategy-store.js deleted file mode 100644 index f2ce28bc25..0000000000 --- a/test/unit/routes/fixtures/fake-client-strategy-store.js +++ /dev/null @@ -1,14 +0,0 @@ -'use strict'; - -module.exports = () => { - const _instances = []; - - return { - insert: () => { - _instances.push(); - return Promise.resolve(); - }, - getAll: () => Promise.resolve(_instances), - getApplications: () => Promise.resolve([]), - }; -}; diff --git a/test/unit/routes/fixtures/store.js b/test/unit/routes/fixtures/store.js index daef06f189..4037f3360f 100644 --- a/test/unit/routes/fixtures/store.js +++ b/test/unit/routes/fixtures/store.js @@ -1,7 +1,6 @@ 'use strict'; const ClientMetricsStore = require('./fake-metrics-store'); -const clientStrategyStore = require('./fake-client-strategy-store'); const clientInstanceStore = require('./fake-client-instance-store'); const clientApplicationsStore = require('./fake-client-applications-store'); const featureToggleStore = require('./fake-feature-toggle-store'); @@ -22,7 +21,6 @@ module.exports = { db, clientApplicationsStore: clientApplicationsStore(), clientMetricsStore: new ClientMetricsStore(), - clientStrategyStore: clientStrategyStore(), clientInstanceStore: clientInstanceStore(), featureToggleStore: featureToggleStore(), eventStore: eventStore(), diff --git a/test/unit/routes/metrics.test.js b/test/unit/routes/metrics.test.js index 6600f46b6c..759b6bc036 100644 --- a/test/unit/routes/metrics.test.js +++ b/test/unit/routes/metrics.test.js @@ -151,24 +151,12 @@ test('should return metrics for all toggles', t => { .get('/api/client/metrics/feature-toggles') .expect(200) .expect((res) => { - const metrics = res.body; t.true(metrics.lastHour !== undefined); t.true(metrics.lastMinute !== undefined); }); }); - -test('should return list of client strategies', t => { - const { request } = getSetup(); - return request - .get('/api/client/strategies') - .expect(200) - .expect((res) => { - t.true(res.body.length === 0); - }); -}); - test('should return list of client applications', t => { const { request } = getSetup(); return request