Introduce HttpController interface for http routers (#7142)

By requiring controllers to declare their own routes, we ensure
consistency between tests and the actual server
This commit is contained in:
Mats Jun Larsen
2024-11-29 00:12:06 +09:00
committed by GitHub
parent a895b7b365
commit b29a0809ad
8 changed files with 69 additions and 16 deletions

10
app.ts
View File

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

View File

@@ -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
*/

View File

@@ -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;
}

View File

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

View File

@@ -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
*/

View File

@@ -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 () => {

View File

@@ -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 () => {

View File

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