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/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..ff2d95751f --- /dev/null +++ b/lib/db/client-applications-store.js @@ -0,0 +1,102 @@ +/* eslint camelcase:off */ +'use strict'; + +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) => ({ + appName: row.app_name, + createdAt: row.created_at, + updatedAt: row.updated_at, + description: row.description, + strategies: row.strategies, + 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, + strategies: JSON.stringify(input.strategies || old.strategies), +}); + + +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); + } + }); + } + + 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) + .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_LIST) + .from(TABLE) + .map(mapRow) + .then(apps => apps + .filter(app => app.strategies.includes(strategyName))); + } + + getApplications (filter) { + return filter && filter.strategyName ? + this.getAppsForStrategy(filter.strategyName) : this.getAll(); + } +}; + +module.exports = ClientApplicationsDb; 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 738fc9acea..c7f196ddef 100644 --- a/lib/db/index.js +++ b/lib/db/index.js @@ -7,7 +7,7 @@ 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) => { const db = createDb(config); @@ -19,8 +19,8 @@ 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..d618aca4c4 100644 --- a/lib/routes/metrics.js +++ b/lib/routes/metrics.js @@ -9,8 +9,8 @@ const { catchLogAndSendErrorResponse } = require('./route-utils'); module.exports = function (app, config) { const { clientMetricsStore, - clientStrategyStore, clientInstanceStore, + clientApplicationsStore, } = config.stores; const metrics = new ClientMetrics(clientMetricsStore); @@ -22,7 +22,19 @@ module.exports = function (app, config) { app.get('/client/seen-apps', (req, res) => { const seenApps = metrics.getSeenAppsPerToggle(); - res.json(seenApps); + clientApplicationsStore.getApplications() + .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) => { @@ -64,55 +76,53 @@ 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(); }); }); - 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, + }); + 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 + clientApplicationsStore + .getApplications(req.query) .then(apps => { - const applications = apps.map(appName => ({ + const applications = apps.map(({ appName, createdAt, strategies }) => ({ appName, + strategies, + createdAt, links: { appDetails: `/api/client/applications/${appName}`, }, @@ -125,11 +135,28 @@ module.exports = function (app, config) { 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), ]) - .then(([instances, strategies]) => res.json({ appName, instances, strategies, seenToggles })) - .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/20161205203516-create-client-applications.js b/migrations/20161205203516-create-client-applications.js new file mode 100644 index 0000000000..e665fda6c3 --- /dev/null +++ b/migrations/20161205203516-create-client-applications.js @@ -0,0 +1,20 @@ +/* eslint camelcase: "off" */ +'use strict'; + +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/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 new file mode 100644 index 0000000000..7d5d49ebfd --- /dev/null +++ b/test/unit/routes/fixtures/fake-client-applications-store.js @@ -0,0 +1,6 @@ +'use strict'; + +module.exports = () => ({ + upsert: () => 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 3e24840288..4037f3360f 100644 --- a/test/unit/routes/fixtures/store.js +++ b/test/unit/routes/fixtures/store.js @@ -1,8 +1,8 @@ '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'); const eventStore = require('./fake-event-store'); const strategyStore = require('./fake-strategies-store'); @@ -19,8 +19,8 @@ module.exports = { return { 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