feat: Remove applications (#635)

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)

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;

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);

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);
+});

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);
+        });
+});

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);
+});

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": [

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);
+        },
     };
 };

test/fixtures/fake-client-instance-store.js

@@ -3,4 +3,5 @@
 module.exports = () => ({
     insert: () => Promise.resolve(),
     getApplications: () => Promise.resolve([]),
+    deleteForApplication: () => Promise.resolve(),
 });