Repository: compiler-explorer/compiler-explorer
Pull request #7130 (2024-11-27 06:04:50)

Rewrite assembly documentation handler to controller form
Follow-up to #7090


Patch:
diff --git a/app.ts b/app.ts
index d8923c376..db8537215 100755
--- a/app.ts
+++ b/app.ts
@@ -57,6 +57,7 @@ import {startWineInit} from './lib/exec.js';
 import {RemoteExecutionQuery} from './lib/execution/execution-query.js';
 import {initHostSpecialties} from './lib/execution/execution-triple.js';
 import {startExecutionWorkerThread} from './lib/execution/sqs-execution-queue.js';
+import {AssemblyDocumentationController} from './lib/handlers/api/assembly-documentation-controller.js';
 import {SiteTemplateController} from './lib/handlers/api/site-template-controller.js';
 import {SourceController} from './lib/handlers/api/source-controller.js';
 import {CompileHandler} from './lib/handlers/compile.js';
@@ -555,10 +556,11 @@ async function main() {
     const compileHandler = new CompileHandler(compilationEnvironment, awsProps);
     const storageType = getStorageTypeByKey(storageSolution);
     const storageHandler = new storageType(httpRoot, compilerProps, awsProps);
-    const sourceHandler = new SourceController(sources);
     const compilerFinder = new CompilerFinder(compileHandler, compilerProps, awsProps, defArgs, clientOptionsHandler);
 
     const siteTemplateController = new SiteTemplateController();
+    const sourceController = new SourceController(sources);
+    const assemblyDocumentationController = new AssemblyDocumentationController();
 
     logger.info('=======================================');
     if (gitReleaseName) logger.info(`  git release ${gitReleaseName}`);
@@ -870,9 +872,13 @@ async function main() {
             );
         })
         .use(bodyParser.json({limit: ceProps('bodyParserLimit', maxUploadSize)}))
-        .use('/source/:source/list', sourceHandler.listEntries.bind(sourceHandler))
-        .use('/source/:source/load/:language/:filename', sourceHandler.loadEntry.bind(sourceHandler))
         .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),
+        )
         .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.ts b/lib/handlers/api.ts
index 735eefc55..05880f63d 100644
--- a/lib/handlers/api.ts
+++ b/lib/handlers/api.ts
@@ -41,7 +41,6 @@ import {SentryCapture} from '../sentry.js';
 import {BaseShortener, getShortenerTypeByKey} from '../shortener/index.js';
 import {StorageBase} from '../storage/index.js';
 
-import {withAssemblyDocumentationProviders} from './assembly-documentation.js';
 import {CompileHandler} from './compile.js';
 import {FormattingHandler} from './formatting.js';
 
@@ -91,9 +90,6 @@ export class ApiHandler {
 
         this.handle.route('/libraries').get(this.handleAllLibraries.bind(this)).all(methodNotAllowed);
 
-        // Binding for assembly documentation
-        withAssemblyDocumentationProviders(this.handle);
-        // Legacy binding for old clients.
         this.handle
             .route('/asm/:opcode')
             .get((req, res) => res.redirect(`amd64/${req.params.opcode}`))
diff --git a/lib/handlers/api/assembly-documentation-controller.ts b/lib/handlers/api/assembly-documentation-controller.ts
new file mode 100644
index 000000000..f4560591f
--- /dev/null
+++ b/lib/handlers/api/assembly-documentation-controller.ts
@@ -0,0 +1,67 @@
+// Copyright (c) 2021, 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';
+
+import {addStaticHeaders} from '../../../app.js';
+import {getDocumentationProviderTypeByKey} from '../../asm-docs/index.js';
+
+export class AssemblyDocumentationController {
+    /**
+     * Handle request to `/asm/:arch/:opcode` endpoint
+     */
+    public async getOpcodeDocumentation(req: express.Request, res: express.Response) {
+        try {
+            const Provider = getDocumentationProviderTypeByKey(req.params.arch);
+            const provider = new Provider();
+            // If there is no opcode, we should just fail with 404 anyways... Assumes that no assembly language has
+            // a __unknown_opcode instruction.
+            const instruction = (req.params.opcode || '__UNKNOWN_OPCODE').toUpperCase();
+            const information = provider.getInstructionInformation(instruction);
+            if (information === null) {
+                return res.status(404).send({error: `Unknown opcode '${instruction}'`});
+            }
+
+            const contentType = req.accepts(['text', 'json']);
+            switch (contentType) {
+                case 'text': {
+                    addStaticHeaders(res);
+                    res.send(information.html);
+                    break;
+                }
+                case 'json': {
+                    addStaticHeaders(res);
+                    res.send(information);
+                    break;
+                }
+                default: {
+                    res.status(406).send({error: 'Not Acceptable'});
+                    break;
+                }
+            }
+        } catch {
+            return res.status(404).send({error: `No documentation for '${req.params.arch}'`});
+        }
+    }
+}
diff --git a/lib/handlers/assembly-documentation.ts b/lib/handlers/assembly-documentation.ts
deleted file mode 100644
index 5230864c3..000000000
--- a/lib/handlers/assembly-documentation.ts
+++ /dev/null
@@ -1,79 +0,0 @@
-// Copyright (c) 2021, 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';
-
-import {BaseAssemblyDocumentationProvider, getDocumentationProviderTypeByKey} from '../asm-docs/index.js';
-import {propsFor} from '../properties.js';
-
-const MAX_STATIC_AGE = propsFor('asm-docs')('staticMaxAgeSecs', 10);
-
-const onDocumentationProviderRequest = (
-    provider: BaseAssemblyDocumentationProvider,
-    request: express.Request,
-    response: express.Response,
-) => {
-    // If the request had no opcode parameter, we should fail. This assumes
-    // no assembly language has a __unknown_opcode instruction.
-    const instruction = (request.params.opcode || '__UNKNOWN_OPCODE').toUpperCase();
-    const information = provider.getInstructionInformation(instruction);
-    if (information === null) {
-        return response.status(404).send({error: `Unknown opcode '${instruction}'`});
-    }
-    // Accept either JSON or Plaintext Content-Type
-    const requestedContentType = request.accepts(['text', 'json']);
-    switch (requestedContentType) {
-        case 'text': {
-            response.send(information.html);
-            break;
-        }
-        case 'json': {
-            response.send(information);
-            break;
-        }
-        default: {
-            response.status(406).send({error: 'Not Acceptable'});
-            break;
-        }
-    }
-};
-
-/** Initialize API routes for assembly documentation */
-export const withAssemblyDocumentationProviders = (router: express.Router) =>
-    router.get('/asm/:arch/:opcode', (req, res) => {
-        if (MAX_STATIC_AGE > 0) {
-            res.setHeader('Cache-Control', `public, max-age=${MAX_STATIC_AGE}`);
-        }
-        const arch = req.params.arch;
-        // makeKeyedTypeGetter throws if the key is not found. We do not wish
-        // crash CE if this happens, so we catch the error and return a 404.
-        try {
-            const providerClass = getDocumentationProviderTypeByKey(arch);
-            onDocumentationProviderRequest(new providerClass(), req, res);
-        } catch {
-            res.status(404)
-                .json({error: `No documentation for '${arch}'`})
-                .send();
-        }
-    });


Test patch:
diff --git a/test/handlers/asm-docs-tests.ts b/test/handlers/asm-docs-tests.ts
index 46f8a1738..9593eb078 100644
--- a/test/handlers/asm-docs-tests.ts
+++ b/test/handlers/asm-docs-tests.ts
@@ -26,7 +26,7 @@ import express from 'express';
 import request from 'supertest';
 import {beforeAll, describe, expect, it} from 'vitest';
 
-import {withAssemblyDocumentationProviders} from '../../lib/handlers/assembly-documentation.js';
+import {AssemblyDocumentationController} from '../../lib/handlers/api/assembly-documentation-controller.js';
 
 /** Test matrix of architecture to [opcode, tooptip, html, url] */
 export const TEST_MATRIX: Record<PropertyKey, [string, string, string, string][]> = {
@@ -112,7 +112,8 @@ describe('Assembly Documentation API', () => {
     beforeAll(() => {
         app = express();
         const router = express.Router();
-        withAssemblyDocumentationProviders(router);
+        const controller = new AssemblyDocumentationController();
+        router.get('/asm/:arch/:opcode', controller.getOpcodeDocumentation.bind(controller));
         app.use('/api', router);
     });