From fba4ba672aa8b2b65abe1639a1efc3f99ef19c95 Mon Sep 17 00:00:00 2001 From: Mats Jun Larsen Date: Sat, 30 Nov 2024 12:14:11 +0900 Subject: [PATCH] De-couple formatter from http server (#7155) This patch decouples the formatting logic (which is used by the clang-format feature) from the http API logic. This is done with the new FormattingService populates the formatters from the config, and wraps formatter execution. This way, both the http controller and the clang-format functionality can use it, reducing tight coupling. --- app.ts | 8 + lib/base-compiler.ts | 8 +- lib/compilation-env.ts | 7 +- lib/formatting-service.ts | 102 ++++++++++++ lib/handlers/api.ts | 14 -- lib/handlers/api/formatting-controller.ts | 82 ++++++++++ lib/handlers/formatting.ts | 150 ------------------ test/compilation-env.ts | 21 ++- test/handlers/api-tests.ts | 19 --- .../api/formatting-controller.tests.ts | 83 ++++++++++ 10 files changed, 300 insertions(+), 194 deletions(-) create mode 100644 lib/formatting-service.ts create mode 100644 lib/handlers/api/formatting-controller.ts delete mode 100644 lib/handlers/formatting.ts create mode 100644 test/handlers/api/formatting-controller.tests.ts diff --git a/app.ts b/app.ts index a2ad55729..1082e4d91 100755 --- a/app.ts +++ b/app.ts @@ -57,7 +57,9 @@ 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 {FormattingService} from './lib/formatting-service.js'; import {AssemblyDocumentationController} from './lib/handlers/api/assembly-documentation-controller.js'; +import {FormattingController} from './lib/handlers/api/formatting-controller.js'; import {HealthcheckController} from './lib/handlers/api/healthcheck-controller.js'; import {SiteTemplateController} from './lib/handlers/api/site-template-controller.js'; import {SourceController} from './lib/handlers/api/source-controller.js'; @@ -545,12 +547,16 @@ async function main() { RemoteExecutionQuery.initRemoteExecutionArchs(ceProps, defArgs.env); + const formattingService = new FormattingService(); + await formattingService.initialize(ceProps); + const clientOptionsHandler = new ClientOptionsHandler(sources, compilerProps, defArgs); const compilationQueue = CompilationQueue.fromProps(compilerProps.ceProps); const compilationEnvironment = new CompilationEnvironment( compilerProps, awsProps, compilationQueue, + formattingService, defArgs.doCache, ); const compileHandler = new CompileHandler(compilationEnvironment, awsProps); @@ -570,6 +576,7 @@ async function main() { compileHandler, isExecutionWorker, ); + const formattingController = new FormattingController(formattingService); logger.info('======================================='); if (gitReleaseName) logger.info(` git release ${gitReleaseName}`); @@ -877,6 +884,7 @@ async function main() { .use(siteTemplateController.createRouter()) .use(sourceController.createRouter()) .use(assemblyDocumentationController.createRouter()) + .use(formattingController.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/base-compiler.ts b/lib/base-compiler.ts index 1d492d84e..7c9623169 100644 --- a/lib/base-compiler.ts +++ b/lib/base-compiler.ts @@ -1330,9 +1330,13 @@ export class BaseCompiler { } async applyClangFormat(output: string): Promise { - // Currently hard-coding llvm style try { - const [stdout, stderr] = await this.env.formatHandler.internalFormat('clangformat', 'LLVM', output); + // Currently hard-coding llvm style + const {stdout, stderr} = await this.env.formattingService.format('clangformat', output, { + baseStyle: 'LLVM', + tabWidth: 4, + useSpaces: true, + }); if (stderr) { return stdout + '\n/* clang-format stderr:\n' + stderr.trim() + '\n*/'; } diff --git a/lib/compilation-env.ts b/lib/compilation-env.ts index 4fc8ba08b..30d660b7a 100644 --- a/lib/compilation-env.ts +++ b/lib/compilation-env.ts @@ -35,7 +35,7 @@ import type {Cache} from './cache/base.interfaces.js'; import {BaseCache} from './cache/base.js'; import {createCacheFromConfig} from './cache/from-config.js'; import {CompilationQueue, EnqueueOptions, Job} from './compilation-queue.js'; -import {FormattingHandler} from './handlers/formatting.js'; +import {FormattingService} from './formatting-service.js'; import {logger} from './logger.js'; import type {PropertyGetter} from './properties.interfaces.js'; import {CompilerProps, PropFunc} from './properties.js'; @@ -54,7 +54,6 @@ export class CompilationEnvironment { reportCacheEvery: number; multiarch: string | null; baseEnv: Record; - formatHandler: FormattingHandler; possibleToolchains?: CompilerOverrideOptions; statsNoter: IStatsNoter; private logCompilerCacheAccesses: boolean; @@ -63,6 +62,7 @@ export class CompilationEnvironment { compilerProps: CompilerProps, awsProps: PropFunc, compilationQueue: CompilationQueue | undefined, + public formattingService: FormattingService, doCache?: boolean, ) { this.ceProps = compilerProps.ceProps; @@ -106,9 +106,6 @@ export class CompilationEnvironment { if (environmentVariable === '') return; this.baseEnv[environmentVariable] = process.env[environmentVariable] ?? ''; }); - // I'm not sure that this is the best design; but each compiler having its own means each constructs its own - // handler, and passing it in from the outside is a pain as each compiler's constructor needs it. - this.formatHandler = new FormattingHandler(this.ceProps); this.logCompilerCacheAccesses = this.ceProps('logCompilerCacheAccesses', false); this.statsNoter = createStatsNoter(this.ceProps); } diff --git a/lib/formatting-service.ts b/lib/formatting-service.ts new file mode 100644 index 000000000..95ab61493 --- /dev/null +++ b/lib/formatting-service.ts @@ -0,0 +1,102 @@ +// 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 _ from 'underscore'; + +import {UnprocessedExecResult} from '../types/execution/execution.interfaces.js'; + +import * as exec from './exec.js'; +import {FormatOptions} from './formatters/base.interfaces.js'; +import {BaseFormatter, getFormatterTypeByKey} from './formatters/index.js'; +import {logger} from './logger.js'; +import {PropertyGetter} from './properties.interfaces.js'; + +export class FormattingService { + /** Mapping of the formatter ids to the formatter implementations */ + private registry: Map = new Map(); + + public getFormatterById(id: string): BaseFormatter | null { + return this.registry.get(id) || null; + } + + public getFormatters(): BaseFormatter[] { + return Array.from(this.registry.values()); + } + + public async format(formatterid: string, source: string, options: FormatOptions): Promise { + const formatter = this.getFormatterById(formatterid); + // Ensure the formatter exists + if (formatter === null) { + throw new Error(`Formatter ${formatterid} not found`); + } + // Ensure the formatting style is valid + if (!formatter.isValidStyle(options.baseStyle)) { + throw new Error(`Formatter ${formatterid} does not support style ${options.baseStyle}`); + } + return await formatter.format(source, options); + } + + public async initialize(ceProps: PropertyGetter): Promise { + const formatters = _.compact(ceProps('formatters', '').split(':')); + for (const formatter of formatters) { + logger.info(`Performing discovery of formatter named ${formatter}`); + const executable = ceProps(`formatter.${formatter}.exe`); + const type = ceProps(`formatter.${formatter}.type`); + if (!executable) { + logger.warn(`Formatter ${formatter} does not have a valid executable. Skipping...`); + continue; + } + if (!type) { + logger.warn(`Formatter ${formatter} does not have a valid type. Skipping...`); + continue; + } + const versionArgument = ceProps(`formatter.${formatter}.version`, '--version'); + const versionRegExp = ceProps(`formatter.${formatter}.versionRe`, '.*'); + const hasExplicitVersion = ceProps(`formatter.${formatter}.explicitVersion`, '') !== ''; + try { + const result = await exec.execute(executable, [versionArgument], {}); + const match = result.stdout.match(versionRegExp); + const formatterClass = getFormatterTypeByKey(type); + const styleList = ceProps(`formatter.${formatter}.styles`, ''); + const styles = styleList === '' ? [] : styleList.split(':'); + // If there is an explicit version, grab it. Otherwise, try to filter the output + const version = hasExplicitVersion + ? ceProps(`formatter.${formatter}.explicitVersion`) + : match + ? match[0] + : result.stdout; + const instance = new formatterClass({ + name: ceProps(`formatter.${formatter}.name`, executable), + exe: executable, + version, + styles, + type, + }); + this.registry.set(formatter, instance); + } catch (err: unknown) { + logger.warn(`Error while fetching tool info for ${executable}:`, {err}); + } + } + } +} diff --git a/lib/handlers/api.ts b/lib/handlers/api.ts index 05880f63d..5835d7eef 100644 --- a/lib/handlers/api.ts +++ b/lib/handlers/api.ts @@ -42,7 +42,6 @@ import {BaseShortener, getShortenerTypeByKey} from '../shortener/index.js'; import {StorageBase} from '../storage/index.js'; import {CompileHandler} from './compile.js'; -import {FormattingHandler} from './formatting.js'; function methodNotAllowed(req: express.Request, res: express.Response) { res.send('Method Not Allowed'); @@ -121,19 +120,6 @@ export class ApiHandler { .post(compileHandler.handleOptimizationArguments.bind(compileHandler)) .get(compileHandler.handleOptimizationArguments.bind(compileHandler)) .all(methodNotAllowed); - - const formatHandler = new FormattingHandler(ceProps); - this.handle - .route('/format/:tool') - .post((req, res) => formatHandler.handle(req, res)) - .all(methodNotAllowed); - this.handle - .route('/formats') - .get((req, res) => { - const all = formatHandler.getFormatterInfo(); - res.send(all); - }) - .all(methodNotAllowed); this.handle.route('/shortlinkinfo/:id').get(this.shortlinkInfoHandler.bind(this)).all(methodNotAllowed); const shortenerType = getShortenerTypeByKey(urlShortenService); diff --git a/lib/handlers/api/formatting-controller.ts b/lib/handlers/api/formatting-controller.ts new file mode 100644 index 000000000..3a348ed8a --- /dev/null +++ b/lib/handlers/api/formatting-controller.ts @@ -0,0 +1,82 @@ +// 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'; + +import {FormattingService} from '../../formatting-service.js'; +import {cached, cors} from '../middleware.js'; + +import {HttpController} from './controller.interfaces.js'; + +export class FormattingController implements HttpController { + public constructor(private formattingService: FormattingService) {} + + createRouter(): express.Router { + const router = express.Router(); + router.post('/api/format/:tool', cors, cached, this.format.bind(this)); + router.get('/api/formats', cors, cached, this.getFormatters.bind(this)); + return router; + } + + /** Handle requests to /api/format/:tool */ + public async format(req: express.Request, res: express.Response) { + const formatter = this.formattingService.getFormatterById(req.params.tool); + // Ensure the target formatter exists + if (formatter === null) { + return res.status(422).json({exit: 2, answer: `Unknown format tool '${req.params.tool}'`}); + } + // Ensure there is source code to format + if (!req.body || !req.body.source) { + res.status(400).json({exit: 0, answer: ''}); + return; + } + // Ensure that the requested style is supported by the formatter + if (!formatter.isValidStyle(req.body.base)) { + return res.status(422).json({exit: 3, answer: `Style '${req.body.base}' is not supported`}); + } + // Do the formatting + try { + const formatted = await formatter.format(req.body.source, { + useSpaces: req.body.useSpaces === undefined ? true : req.body.useSpaces, + tabWidth: req.body.tabWidth === undefined ? 4 : req.body.tabWidth, + baseStyle: req.body.base, + }); + res.json({exit: formatted.code, answer: formatted.stdout || formatted.stderr || ''}); + } catch (err: unknown) { + res.status(500).json({ + exit: 1, + thrown: true, + answer: + (err && Object.hasOwn(err, 'message') && (err as Record<'message', 'string'>).message) || + 'Internal server error', + }); + } + } + + /** Handle requests to /api/formats */ + public async getFormatters(_: express.Request, res: express.Response) { + const formatters = this.formattingService.getFormatters(); + res.send(formatters.map(formatter => formatter.formatterInfo)); + } +} diff --git a/lib/handlers/formatting.ts b/lib/handlers/formatting.ts deleted file mode 100644 index 623f04447..000000000 --- a/lib/handlers/formatting.ts +++ /dev/null @@ -1,150 +0,0 @@ -// Copyright (c) 2018, 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 _ from 'underscore'; - -import * as exec from '../exec.js'; -import {BaseFormatter, getFormatterTypeByKey} from '../formatters/index.js'; -import {logger} from '../logger.js'; -import {PropertyGetter} from '../properties.interfaces.js'; - -export class FormattingHandler { - private formatters: Record = {}; - - public constructor(private ceProps: PropertyGetter) { - const formatters = _.compact(ceProps('formatters', '').split(':')); - for (const formatter of formatters) { - this.populateFormatterInfo(formatter); - } - } - - private async populateFormatterInfo(formatterName: string): Promise { - const exe = this.ceProps(`formatter.${formatterName}.exe`); - const type = this.ceProps(`formatter.${formatterName}.type`); - if (!exe) { - logger.warn(`Formatter ${formatterName} does not have a valid executable. Skipping...`); - return; - } - if (!type) { - logger.warn(`Formatter ${formatterName} does not have a formatter class. Skipping...`); - return; - } - const versionArgument = this.ceProps(`formatter.${formatterName}.version`, '--version'); - const versionRegExp = this.ceProps(`formatter.${formatterName}.versionRe`, '.*'); - const hasExplicitVersion = this.ceProps(`formatter.${formatterName}.explicitVersion`, '') !== ''; - try { - const result = await exec.execute(exe, [versionArgument], {}); - const match = result.stdout.match(versionRegExp); - const formatterClass = getFormatterTypeByKey(type); - const styleList = this.ceProps(`formatter.${formatterName}.styles`, ''); - const styles = styleList === '' ? [] : styleList.split(':'); - // If there is an explicit version, grab it. Otherwise try to filter the output - const version = hasExplicitVersion - ? this.ceProps(`formatter.${formatterName}.explicitVersion`) - : match - ? match[0] - : result.stdout; - this.formatters[formatterName] = new formatterClass({ - name: this.ceProps(`formatter.${formatterName}.name`, exe), - exe, - version, - styles, - type, - }); - } catch (err: unknown) { - logger.warn(`Error while fetching tool info for ${exe}:`, {err}); - } - } - - public getFormatterInfo() { - return Object.values(this.formatters).map(formatter => formatter.formatterInfo); - } - - public async handle(req: express.Request, res: express.Response): Promise { - const name = req.params.tool; - const formatter = this.formatters[name]; - // Ensure the formatter exists - if (!formatter) { - res.status(422).send({ - exit: 2, - answer: `Unknown format tool '${name}'`, - }); - return; - } - // Ensure there is source code to format - if (!req.body || !req.body.source) { - res.send({exit: 0, answer: ''}); - return; - } - // Ensure the wanted style is valid for the formatter - const style = req.body.base; - if (!formatter.isValidStyle(style)) { - res.status(422).send({ - exit: 3, - answer: `Style '${style}' is not supported`, - }); - return; - } - try { - // Perform the actual formatting - const result = await formatter.format(req.body.source, { - useSpaces: req.body.useSpaces === undefined ? true : req.body.useSpaces, - tabWidth: req.body.tabWidth === undefined ? 4 : req.body.tabWidth, - baseStyle: req.body.base, - }); - res.send({ - exit: result.code, - answer: result.stdout || result.stderr || '', - }); - } catch (err: unknown) { - res.status(500).send({ - exit: 1, - thrown: true, - answer: - (err && Object.hasOwn(err, 'message') && (err as Record<'message', 'string'>).message) || - 'Internal server error', - }); - } - } - - async internalFormat(formatterName: string, style: string, source: string): Promise<[string, string]> { - const formatter = this.formatters[formatterName]; - // Ensure the formatter exists - if (!formatter) { - throw new Error('Unknown formatter name'); - } - // Ensure the wanted style is valid for the formatter - if (!formatter.isValidStyle(style)) { - throw new Error('Unsupported formatter style'); - } - // Perform the actual formatting - const result = await formatter.format(source, { - useSpaces: true, // hard coded for now, TODO should this be changed? - tabWidth: 4, - baseStyle: style, - }); - return [result.stdout || '', result.stderr]; - } -} diff --git a/test/compilation-env.ts b/test/compilation-env.ts index 786f8ab24..1b97d4f81 100644 --- a/test/compilation-env.ts +++ b/test/compilation-env.ts @@ -26,6 +26,7 @@ import './utils.js'; import {beforeAll, describe, expect, it} from 'vitest'; import {CompilationEnvironment} from '../lib/compilation-env.js'; +import {FormattingService} from '../lib/formatting-service.js'; import {CompilerProps, fakeProps} from '../lib/properties.js'; const props = { @@ -43,28 +44,40 @@ describe('Compilation environment', () => { it('Should cache by default', async () => { // TODO: Work will need to be done here when CompilationEnvironment's constructor is typed better - const ce = new CompilationEnvironment(compilerProps, fakeProps({}), undefined, undefined); + const ce = new CompilationEnvironment( + compilerProps, + fakeProps({}), + undefined, + new FormattingService(), + undefined, + ); await expect(ce.cacheGet('foo')).resolves.toBeNull(); await ce.cachePut('foo', {res: 'bar'}, undefined); await expect(ce.cacheGet('foo')).resolves.toEqual({res: 'bar'}); await expect(ce.cacheGet('baz')).resolves.toBeNull(); }); it('Should cache when asked', async () => { - const ce = new CompilationEnvironment(compilerProps, fakeProps({}), undefined, true); + const ce = new CompilationEnvironment(compilerProps, fakeProps({}), undefined, new FormattingService(), true); await expect(ce.cacheGet('foo')).resolves.toBeNull(); await ce.cachePut('foo', {res: 'bar'}, undefined); await expect(ce.cacheGet('foo')).resolves.toEqual({res: 'bar'}); }); it("Shouldn't cache when asked", async () => { // TODO: Work will need to be done here when CompilationEnvironment's constructor is typed better - const ce = new CompilationEnvironment(compilerProps, fakeProps({}), undefined, false); + const ce = new CompilationEnvironment(compilerProps, fakeProps({}), undefined, new FormattingService(), false); await expect(ce.cacheGet('foo')).resolves.toBeNull(); await ce.cachePut('foo', {res: 'bar'}, undefined); await expect(ce.cacheGet('foo')).resolves.toBeNull(); }); it('Should filter bad options', () => { // TODO: Work will need to be done here when CompilationEnvironment's constructor is typed better - const ce = new CompilationEnvironment(compilerProps, fakeProps({}), undefined, undefined); + const ce = new CompilationEnvironment( + compilerProps, + fakeProps({}), + undefined, + new FormattingService(), + undefined, + ); expect(ce.findBadOptions(['-O3', '-flto'])).toEqual([]); expect(ce.findBadOptions(['-O3', '-plugin'])).toEqual(['-plugin']); }); diff --git a/test/handlers/api-tests.ts b/test/handlers/api-tests.ts index 92194b165..c9adbc649 100644 --- a/test/handlers/api-tests.ts +++ b/test/handlers/api-tests.ts @@ -164,23 +164,4 @@ describe('API handling', () => { .expect('Content-Type', /json/) .expect(200, [languages['c++'], languages.pascal]); }); - - it('should not go through with invalid tools', async () => { - await request(app) - .post('/api/format/invalid') - .set('Accept', 'application/json') - .expect('Content-Type', /json/) - .expect(422, {exit: 2, answer: "Unknown format tool 'invalid'"}); - }); - it('should not go through with invalid base styles', async () => { - await request(app) - .post('/api/format/formatt') - .send({ - base: 'bad-base', - source: 'i am source', - }) - .set('Accept', 'application/json') - .expect(422, {exit: 3, answer: "Style 'bad-base' is not supported"}) - .expect('Content-Type', /json/); - }); }); diff --git a/test/handlers/api/formatting-controller.tests.ts b/test/handlers/api/formatting-controller.tests.ts new file mode 100644 index 000000000..f1aee3328 --- /dev/null +++ b/test/handlers/api/formatting-controller.tests.ts @@ -0,0 +1,83 @@ +// 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'; +import request from 'supertest'; +import {beforeAll, describe, it} from 'vitest'; + +import {FormattingService} from '../../../lib/formatting-service.js'; +import {FormattingController} from '../../../lib/handlers/api/formatting-controller.js'; +import {fakeProps} from '../../../lib/properties.js'; + +describe('FormattingController', () => { + let app; + + beforeAll(async () => { + app = express(); + const formattingService = new FormattingService(); + const formattingController = new FormattingController(formattingService); + await formattingService.initialize( + fakeProps({ + formatters: 'formatt:badformatt', + 'formatter.formatt.exe': process.platform === 'win32' ? 'cmd' : 'echo', + 'formatter.formatt.type': 'clangformat', + 'formatter.formatt.version': 'Release', + 'formatter.formatt.name': 'FormatT', + }), + ); + app.use(express.json()); + app.use(formattingController.createRouter()); + }); + + it('should not go through with invalid tools', async () => { + await request(app) + .post('/api/format/invalid') + .set('Accept', 'application/json') + .expect('Content-Type', /json/) + .expect(422, {exit: 2, answer: "Unknown format tool 'invalid'"}); + }); + + it('should not go through with invalid base styles', async () => { + await request(app) + .post('/api/format/formatt') + .send({ + base: 'bad-base', + source: 'i am source', + }) + .set('Accept', 'application/json') + .expect(422, {exit: 3, answer: "Style 'bad-base' is not supported"}) + .expect('Content-Type', /json/); + }); + + it('should reject requests with no source', async () => { + await request(app) + .post('/api/format/formatt') + .send({ + base: 'bad-base', + }) + .set('Accept', 'application/json') + .expect(400, {exit: 0, answer: ''}) + .expect('Content-Type', /json/); + }); +});