mirror of
				https://github.com/Unleash/unleash.git
				synced 2025-10-27 11:02:16 +01:00 
			
		
		
		
	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.
This commit is contained in:
		
							parent
							
								
									8764b952b3
								
							
						
					
					
						commit
						51442a6cc4
					
				| @ -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" | ||||
|       } | ||||
|     } | ||||
|   ] | ||||
|  | ||||
| @ -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 | ||||
|  | ||||
| @ -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(); | ||||
|     } | ||||
| }; | ||||
| 
 | ||||
|  | ||||
| @ -76,7 +76,7 @@ class ClientInstanceStore { | ||||
| 
 | ||||
|     getByAppName (appName) { | ||||
|         return this.db | ||||
|             .select(COLUMNS) | ||||
|             .select() | ||||
|             .from(TABLE) | ||||
|             .where('app_name', appName) | ||||
|             .orderBy('last_seen', 'desc') | ||||
|  | ||||
| @ -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; | ||||
| @ -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), | ||||
|     }; | ||||
| }; | ||||
|  | ||||
| @ -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)); | ||||
|     }); | ||||
| }; | ||||
|  | ||||
| @ -1,3 +0,0 @@ | ||||
| 'use strict'; | ||||
| 
 | ||||
| module.exports = require('../scripts/migration-runner').create('009-create-client-strategies'); | ||||
| @ -1,2 +0,0 @@ | ||||
| --create client_strategies | ||||
| DROP TABLE client_strategies; | ||||
| @ -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 | ||||
| ); | ||||
| @ -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 = { | ||||
|  | ||||
| @ -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); | ||||
| }); | ||||
|  | ||||
| @ -2,5 +2,5 @@ | ||||
| 
 | ||||
| module.exports = () => ({ | ||||
|     upsert: () => Promise.resolve(), | ||||
|     getApplicationMetaData: () => Promise.resolve([]), | ||||
|     getApplications: () => Promise.resolve([]), | ||||
| }); | ||||
|  | ||||
| @ -1,14 +0,0 @@ | ||||
| 'use strict'; | ||||
| 
 | ||||
| module.exports = () => { | ||||
|     const _instances = []; | ||||
| 
 | ||||
|     return { | ||||
|         insert: () => { | ||||
|             _instances.push(); | ||||
|             return Promise.resolve(); | ||||
|         }, | ||||
|         getAll: () => Promise.resolve(_instances), | ||||
|         getApplications: () => Promise.resolve([]), | ||||
|     }; | ||||
| }; | ||||
| @ -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(), | ||||
|  | ||||
| @ -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 | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user