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