From 529ac38e9790ebcc74ce0c3d0aa90bec4dd505a9 Mon Sep 17 00:00:00 2001
From: ivaosthu <ivar.conradi.osthus@finn.no>
Date: Thu, 29 Nov 2018 20:46:24 +0100
Subject: [PATCH] chore(modernize): Modernize ClientRegisterController

---
 lib/routes/client-api/index.js         | 14 +++---
 lib/routes/client-api/register.js      | 62 +++++++++++++++-----------
 lib/routes/client-api/register.test.js | 32 +++++++++++++
 3 files changed, 75 insertions(+), 33 deletions(-)

diff --git a/lib/routes/client-api/index.js b/lib/routes/client-api/index.js
index d42e8161f7..2931edea51 100644
--- a/lib/routes/client-api/index.js
+++ b/lib/routes/client-api/index.js
@@ -2,9 +2,9 @@
 
 const { Router } = require('express');
 const FeatureController = require('./feature.js');
-const ClientMetricsController = require('./metrics.js');
-const register = require('./register.js');
-const clientApi = require('./client-api.json');
+const MetricsController = require('./metrics.js');
+const RegisterController = require('./register.js');
+const clientApiSpec = require('./client-api.json');
 
 class ClientApi {
     constructor(config) {
@@ -13,12 +13,12 @@ class ClientApi {
 
         router.get('/', this.index);
         router.use('/features', new FeatureController(config).router());
-        router.use('/metrics', new ClientMetricsController(config).router());
-        router.use('/register', register.router(config));
+        router.use('/metrics', new MetricsController(config).router());
+        router.use('/register', new RegisterController(config).router());
     }
 
     index(req, res) {
-        res.json(clientApi);
+        res.json(clientApiSpec);
     }
 
     router() {
@@ -26,6 +26,6 @@ class ClientApi {
     }
 }
 
-ClientApi.api = clientApi;
+ClientApi.api = clientApiSpec;
 
 module.exports = ClientApi;
diff --git a/lib/routes/client-api/register.js b/lib/routes/client-api/register.js
index 4b59e45eaa..62937fbc10 100644
--- a/lib/routes/client-api/register.js
+++ b/lib/routes/client-api/register.js
@@ -4,37 +4,47 @@ const { Router } = require('express');
 const joi = require('joi');
 const logger = require('../../logger')('/client-api/register.js');
 
-const { clientRegisterSchema } = require('./register-schema');
+const { clientRegisterSchema: schema } = require('./register-schema');
 
-exports.router = config => {
-    const { clientInstanceStore, clientApplicationsStore } = config.stores;
-    const router = Router();
+class RegisterController {
+    constructor({ stores: { clientInstanceStore, clientApplicationsStore } }) {
+        const router = Router();
+        this._router = router;
+        this.clientInstanceStore = clientInstanceStore;
+        this.clientApplicationsStore = clientApplicationsStore;
 
-    router.post('/', (req, res) => {
+        router.post('/', (req, res) => this.handleRegister(req, res));
+    }
+
+    async handleRegister(req, res) {
         const data = req.body;
+        const { value: clientRegistration, error } = joi.validate(data, schema);
 
-        joi.validate(data, clientRegisterSchema, (err, clientRegistration) => {
-            if (err) {
-                logger.warn('Invalid client data posted', err);
-                return res.status(400).json(err);
-            }
+        if (error) {
+            logger.warn('Invalid client data posted', error);
+            return res.status(400).json(error);
+        }
 
-            clientRegistration.clientIp = req.ip;
+        clientRegistration.clientIp = req.ip;
 
-            clientApplicationsStore
-                .upsert(clientRegistration)
-                .then(() => clientInstanceStore.insert(clientRegistration))
-                .then(() =>
-                    logger.info(`New client registered with
-                        appName=${clientRegistration.appName} and instanceId=${
-                        clientRegistration.instanceId
-                    }`)
-                )
-                .catch(err => logger.error('failed to register client', err));
+        try {
+            await this.clientApplicationsStore.upsert(clientRegistration);
+            await this.clientInstanceStore.insert(clientRegistration);
+            logger.info(
+                `New client registered with appName=${
+                    clientRegistration.appName
+                } and instanceId=${clientRegistration.instanceId}`
+            );
+            return res.status(202).end();
+        } catch (err) {
+            logger.error('failed to register client', err);
+            return res.status(500).end();
+        }
+    }
 
-            res.status(202).end();
-        });
-    });
+    router() {
+        return this._router;
+    }
+}
 
-    return router;
-};
+module.exports = RegisterController;
diff --git a/lib/routes/client-api/register.test.js b/lib/routes/client-api/register.test.js
index 706407ac65..fcbabb0f40 100644
--- a/lib/routes/client-api/register.test.js
+++ b/lib/routes/client-api/register.test.js
@@ -73,3 +73,35 @@ test('should require strategies field', t => {
         })
         .expect(400);
 });
+
+test('should fail if store fails', t => {
+    t.plan(0);
+
+    // --- start custom config
+    const stores = store.createStores();
+    stores.clientApplicationsStore = {
+        upsert: () => {
+            throw new Error('opps');
+        },
+    };
+
+    const app = getApp({
+        baseUriPath: '',
+        stores,
+        eventBus,
+    });
+    // --- end custom config
+
+    const request = supertest(app);
+
+    return request
+        .post('/api/client/register')
+        .send({
+            appName: 'demo',
+            instanceId: 'test',
+            strategies: ['default'],
+            started: Date.now(),
+            interval: 10,
+        })
+        .expect(500);
+});