From 4a3c136167c8b61e68ddd427e6ef103db9559628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Fri, 25 Sep 2020 09:39:12 +0200 Subject: [PATCH] feat: Remove applications (#635) --- lib/db/client-applications-store.js | 6 ++ lib/db/client-instance-store.js | 6 ++ lib/routes/admin-api/metrics.js | 26 ++++++ lib/routes/admin-api/metrics.test.js | 24 ++++++ test/e2e/api/admin/metrics.e2e.test.js | 81 ++++++------------- test/e2e/api/client/metrics.e2e.test.js | 55 +++++++++++++ test/e2e/helpers/database.json | 12 +++ .../fake-client-applications-store.js | 13 ++- test/fixtures/fake-client-instance-store.js | 1 + 9 files changed, 167 insertions(+), 57 deletions(-) diff --git a/lib/db/client-applications-store.js b/lib/db/client-applications-store.js index 63a10b46cb..9c6748d664 100644 --- a/lib/db/client-applications-store.js +++ b/lib/db/client-applications-store.js @@ -87,6 +87,12 @@ class ClientApplicationsDb { return mapRow(row); } + async deleteApplication(appName) { + return this.db(TABLE) + .where('app_name', appName) + .del(); + } + /** * Could also be done in SQL: * (not sure if it is faster though) diff --git a/lib/db/client-instance-store.js b/lib/db/client-instance-store.js index 608dc63495..950e20240a 100644 --- a/lib/db/client-instance-store.js +++ b/lib/db/client-instance-store.js @@ -127,6 +127,12 @@ class ClientInstanceStore { return rows.map(mapRow); } + + async deleteForApplication(appName) { + return this.db(TABLE) + .where('app_name', appName) + .del(); + } } module.exports = ClientInstanceStore; diff --git a/lib/routes/admin-api/metrics.js b/lib/routes/admin-api/metrics.js index 05149e9873..e0efc1b9ba 100644 --- a/lib/routes/admin-api/metrics.js +++ b/lib/routes/admin-api/metrics.js @@ -32,6 +32,11 @@ class MetricsController extends Controller { this.createApplication, UPDATE_APPLICATION, ); + this.delete( + '/applications/:appName', + this.deleteApplication, + UPDATE_APPLICATION, + ); this.get('/applications/', this.getApplications); this.get('/applications/:appName', this.getApplication); } @@ -76,6 +81,27 @@ class MetricsController extends Controller { }); } + async deleteApplication(req, res) { + const { appName } = req.params; + + try { + await this.clientApplicationsStore.getApplication(appName); + } catch (e) { + this.logger.error(e); + res.status(409).end(); + return; + } + + try { + await this.clientInstanceStore.deleteForApplication(appName); + await this.clientApplicationsStore.deleteApplication(appName); + res.status(200).end(); + } catch (e) { + this.logger.error(e); + res.status(500).end(); + } + } + async createApplication(req, res) { const input = { ...req.body, appName: req.params.appName }; const { value: applicationData, error } = schema.validate(input); diff --git a/lib/routes/admin-api/metrics.test.js b/lib/routes/admin-api/metrics.test.js index f5326b9e71..1854f4959b 100644 --- a/lib/routes/admin-api/metrics.test.js +++ b/lib/routes/admin-api/metrics.test.js @@ -154,3 +154,27 @@ test('should store application details wihtout strategies', t => { .send({ appName, url: 'htto://asd.com' }) .expect(202); }); + +test('should not delete unknown application', t => { + t.plan(0); + const { request, perms } = getSetup(); + const appName = 'unknown'; + perms.withPermissions(UPDATE_APPLICATION); + + return request + .delete(`/api/admin/metrics/applications/${appName}`) + .expect(409); +}); + +test('should delete application', t => { + t.plan(0); + const { request, stores, perms } = getSetup(); + const appName = 'deletable-test'; + + perms.withPermissions(UPDATE_APPLICATION); + stores.clientApplicationsStore.upsert({ appName }); + + return request + .delete(`/api/admin/metrics/applications/${appName}`) + .expect(200); +}); diff --git a/test/e2e/api/admin/metrics.e2e.test.js b/test/e2e/api/admin/metrics.e2e.test.js index f8691f066b..871ab379ed 100644 --- a/test/e2e/api/admin/metrics.e2e.test.js +++ b/test/e2e/api/admin/metrics.e2e.test.js @@ -22,61 +22,6 @@ test.afterEach(async () => { await reset(); }); -test.serial('should register client', async t => { - t.plan(0); - const request = await setupApp(stores); - return request - .post('/api/client/register') - .send({ - appName: 'demo', - instanceId: 'test', - strategies: ['default'], - started: Date.now(), - interval: 10, - }) - .expect(202); -}); - -test.serial('should allow client to register multiple times', async t => { - t.plan(0); - const request = await setupApp(stores); - const clientRegistration = { - appName: 'multipleRegistration', - instanceId: 'test', - strategies: ['default', 'another'], - started: Date.now(), - interval: 10, - }; - - return request - .post('/api/client/register') - .send(clientRegistration) - .expect(202) - .then(() => - request - .post('/api/client/register') - .send(clientRegistration) - .expect(202), - ); -}); - -test.serial('should accept client metrics', async t => { - t.plan(0); - const request = await setupApp(stores); - return request - .post('/api/client/metrics') - .send({ - appName: 'demo', - instanceId: '1', - bucket: { - start: Date.now(), - stop: Date.now(), - toggles: {}, - }, - }) - .expect(202); -}); - test.serial('should get application details', async t => { t.plan(3); const request = await setupApp(stores); @@ -98,6 +43,32 @@ test.serial('should get list of applications', async t => { .expect('Content-Type', /json/) .expect(res => { t.true(res.status === 200); + t.is(res.body.applications.length, 3); + }); +}); + +test.serial('should delete application', async t => { + t.plan(2); + const request = await setupApp(stores); + await request + .delete('/api/admin/metrics/applications/deletable-app') + .expect(res => { + t.is(res.status, 200); + }); + return request + .get('/api/admin/metrics/applications') + .expect('Content-Type', /json/) + .expect(res => { t.is(res.body.applications.length, 2); }); }); + +test.serial('should get 409 when deleting unknwn application', async t => { + t.plan(1); + const request = await setupApp(stores); + return request + .delete('/api/admin/metrics/applications/unkown') + .expect(res => { + t.is(res.status, 409); + }); +}); diff --git a/test/e2e/api/client/metrics.e2e.test.js b/test/e2e/api/client/metrics.e2e.test.js index 6ddaab0dfa..0fbf14bc6c 100644 --- a/test/e2e/api/client/metrics.e2e.test.js +++ b/test/e2e/api/client/metrics.e2e.test.js @@ -37,3 +37,58 @@ test.serial('should require valid send metrics', async t => { }) .expect(400); }); + +test.serial('should register client', async t => { + t.plan(0); + const request = await setupApp(stores); + return request + .post('/api/client/register') + .send({ + appName: 'demo', + instanceId: 'test', + strategies: ['default'], + started: Date.now(), + interval: 10, + }) + .expect(202); +}); + +test.serial('should allow client to register multiple times', async t => { + t.plan(0); + const request = await setupApp(stores); + const clientRegistration = { + appName: 'multipleRegistration', + instanceId: 'test', + strategies: ['default', 'another'], + started: Date.now(), + interval: 10, + }; + + return request + .post('/api/client/register') + .send(clientRegistration) + .expect(202) + .then(() => + request + .post('/api/client/register') + .send(clientRegistration) + .expect(202), + ); +}); + +test.serial('should accept client metrics', async t => { + t.plan(0); + const request = await setupApp(stores); + return request + .post('/api/client/metrics') + .send({ + appName: 'demo', + instanceId: '1', + bucket: { + start: Date.now(), + stop: Date.now(), + toggles: {}, + }, + }) + .expect(202); +}); diff --git a/test/e2e/helpers/database.json b/test/e2e/helpers/database.json index 7fd6e96db9..7ae76eb5f6 100644 --- a/test/e2e/helpers/database.json +++ b/test/e2e/helpers/database.json @@ -30,6 +30,11 @@ "appName": "demo-app-2", "strategies": ["default", "extra"], "description": "hello" + }, + { + "appName": "deletable-app", + "strategies": ["default"], + "description": "Some desc" } ], "clientInstances": [ @@ -46,6 +51,13 @@ "strategies": ["default"], "started": 1516026938494, "interval": 10 + }, + { + "appName": "deletable-app", + "instanceId": "inst-1", + "strategies": ["default"], + "started": 1516026938494, + "interval": 10 } ], "features": [ diff --git a/test/fixtures/fake-client-applications-store.js b/test/fixtures/fake-client-applications-store.js index 4921c07d00..f58c9bd5ee 100644 --- a/test/fixtures/fake-client-applications-store.js +++ b/test/fixtures/fake-client-applications-store.js @@ -1,7 +1,7 @@ 'use strict'; module.exports = () => { - const apps = []; + let apps = []; return { upsert: app => { @@ -9,6 +9,15 @@ module.exports = () => { return Promise.resolve(); }, getApplications: () => Promise.resolve(apps), - getApplication: appName => apps.filter(a => a.name === appName)[0], + getApplication: appName => { + const app = apps.filter(a => a.appName === appName)[0]; + if (!app) { + throw new Error(`Could not find app=${appName}`); + } + return app; + }, + deleteApplication: appName => { + apps = apps.filter(app => app.appName !== appName); + }, }; }; diff --git a/test/fixtures/fake-client-instance-store.js b/test/fixtures/fake-client-instance-store.js index b8d48b78e6..b163c5eb36 100644 --- a/test/fixtures/fake-client-instance-store.js +++ b/test/fixtures/fake-client-instance-store.js @@ -3,4 +3,5 @@ module.exports = () => ({ insert: () => Promise.resolve(), getApplications: () => Promise.resolve([]), + deleteForApplication: () => Promise.resolve(), });