Repository: compiler-explorer/compiler-explorer
Pull request #7142 (2024-11-28 14:05:03)

Introduce HttpController interface for http routers
By requiring controllers to declare their own routes, we ensure consistency between tests and the actual server


Patch:
diff --git a/app.ts b/app.ts
index db8537215..d5e55801f 100755
--- a/app.ts
+++ b/app.ts
@@ -872,13 +872,9 @@ async function main() {
             );
         })
         .use(bodyParser.json({limit: ceProps('bodyParserLimit', maxUploadSize)}))
-        .get('/api/siteTemplates', siteTemplateController.getSiteTemplates.bind(siteTemplateController))
-        .get('/source/:source/list', sourceController.listEntries.bind(sourceController))
-        .get('/source/:source/load/:language/:filename', sourceController.loadEntry.bind(sourceController))
-        .get(
-            '/api/asm/:arch/:opcode',
-            assemblyDocumentationController.getOpcodeDocumentation.bind(assemblyDocumentationController),
-        )
+        .use(siteTemplateController.createRouter())
+        .use(sourceController.createRouter())
+        .use(assemblyDocumentationController.createRouter())
         .get('/g/:id', oldGoogleUrlHandler)
         // Deprecated old route for this -- TODO remove in late 2021
         .post('/shortener', routeApi.apiHandler.shortener.handle.bind(routeApi.apiHandler.shortener));
diff --git a/lib/handlers/api/assembly-documentation-controller.ts b/lib/handlers/api/assembly-documentation-controller.ts
index f4560591f..9893105f4 100644
--- a/lib/handlers/api/assembly-documentation-controller.ts
+++ b/lib/handlers/api/assembly-documentation-controller.ts
@@ -27,7 +27,15 @@ import express from 'express';
 import {addStaticHeaders} from '../../../app.js';
 import {getDocumentationProviderTypeByKey} from '../../asm-docs/index.js';
 
-export class AssemblyDocumentationController {
+import {HttpController} from './controller.interfaces.js';
+
+export class AssemblyDocumentationController implements HttpController {
+    createRouter(): express.Router {
+        const router = express.Router();
+        router.get('/api/asm/:arch/:opcode', this.getOpcodeDocumentation.bind(this));
+        return router;
+    }
+
     /**
      * Handle request to `/asm/:arch/:opcode` endpoint
      */
diff --git a/lib/handlers/api/controller.interfaces.ts b/lib/handlers/api/controller.interfaces.ts
new file mode 100644
index 000000000..52ab0e28f
--- /dev/null
+++ b/lib/handlers/api/controller.interfaces.ts
@@ -0,0 +1,35 @@
+// Copyright (c) 2024, Compiler Explorer Authors
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+//     * Redistributions of source code must retain the above copyright notice,
+//       this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above copyright
+//       notice, this list of conditions and the following disclaimer in the
+//       documentation and/or other materials provided with the distribution.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+// POSSIBILITY OF SUCH DAMAGE.
+
+import express from 'express';
+
+export interface HttpController {
+    /**
+     * Create a separate express router for this controller.
+     *
+     * The purpose of this is to allow for a consistent API surface across both test and production, as well as being
+     * able to create multiple mergable routes for the final server.
+     */
+    createRouter(): express.Router;
+}
diff --git a/lib/handlers/api/site-template-controller.ts b/lib/handlers/api/site-template-controller.ts
index 6f49a9732..38a5703f3 100644
--- a/lib/handlers/api/site-template-controller.ts
+++ b/lib/handlers/api/site-template-controller.ts
@@ -26,7 +26,15 @@ import express from 'express';
 
 import {getSiteTemplates} from '../site-templates.js';
 
-export class SiteTemplateController {
+import {HttpController} from './controller.interfaces.js';
+
+export class SiteTemplateController implements HttpController {
+    createRouter(): express.Router {
+        const router = express.Router();
+        router.get('/api/siteTemplates', this.getSiteTemplates.bind(this));
+        return router;
+    }
+
     public async getSiteTemplates(req: express.Request, res: express.Response) {
         const templates = await getSiteTemplates();
         res.send(templates);
diff --git a/lib/handlers/api/source-controller.ts b/lib/handlers/api/source-controller.ts
index 065689401..b7a48f609 100644
--- a/lib/handlers/api/source-controller.ts
+++ b/lib/handlers/api/source-controller.ts
@@ -28,9 +28,18 @@ import _ from 'underscore';
 import {addStaticHeaders} from '../../../app.js';
 import {Source} from '../../../types/source.interfaces.js';
 
-export class SourceController {
+import {HttpController} from './controller.interfaces.js';
+
+export class SourceController implements HttpController {
     public constructor(private readonly sources: Source[]) {}
 
+    createRouter(): express.Router {
+        const router = express.Router();
+        router.get('/source/:source/list', this.listEntries.bind(this));
+        router.get('/source/:source/load/:language/:filename', this.loadEntry.bind(this));
+        return router;
+    }
+
     /**
      * Handle request to `/source/<source>/list` endpoint
      */


Test patch:
diff --git a/test/handlers/asm-docs-tests.ts b/test/handlers/asm-docs-tests.ts
index 9593eb078..168557b7c 100644
--- a/test/handlers/asm-docs-tests.ts
+++ b/test/handlers/asm-docs-tests.ts
@@ -111,10 +111,8 @@ describe('Assembly Documentation API', () => {
 
     beforeAll(() => {
         app = express();
-        const router = express.Router();
         const controller = new AssemblyDocumentationController();
-        router.get('/asm/:arch/:opcode', controller.getOpcodeDocumentation.bind(controller));
-        app.use('/api', router);
+        app.use('/', controller.createRouter());
     });
 
     it('should return 404 for unknown architecture', async () => {
diff --git a/test/handlers/site-templates-tests.ts b/test/handlers/site-templates-tests.ts
index aca4dc2ef..2a0ad2e19 100644
--- a/test/handlers/site-templates-tests.ts
+++ b/test/handlers/site-templates-tests.ts
@@ -10,7 +10,7 @@ describe('Site Templates Backend', () => {
     beforeAll(() => {
         app = express();
         const controller = new SiteTemplateController();
-        app.use('/api/siteTemplates', controller.getSiteTemplates.bind(controller));
+        app.use('/', controller.createRouter());
     });
 
     it('should load site templates properly', async () => {
diff --git a/test/handlers/source-tests.ts b/test/handlers/source-tests.ts
index a8708a1a7..61e95e005 100644
--- a/test/handlers/source-tests.ts
+++ b/test/handlers/source-tests.ts
@@ -38,8 +38,7 @@ describe('Sources', () => {
             load: (lang, name) => Promise.resolve({file: `File called ${name} in ${lang}`}),
         },
     ]);
-    app.use('/source/:source/list', handler.listEntries.bind(handler));
-    app.use('/source/:source/load/:language/:filename', handler.loadEntry.bind(handler));
+    app.use('/', handler.createRouter());
 
     it('should list', async () => {
         await request(app)