mirrors/compiler-explorer
1
0
Fork 0
mirror of https://github.com/compiler-explorer/compiler-explorer.git synced 2025-12-27 09:23:52 -05:00
Code Issues Packages Projects Releases Wiki Activity

Separate list and load actions for source API (#7090)

Browse Source 
This is the first PR in API consistency with more to come:

1. The source API urls have been properly defined with
`/source/:source/list` and `/source/:source/load/:language/:filename`
- These used to be two API endpoints mushed together into one `handle()`
function. They are now separate
- While it may appear from the tests that this is an API breakage,
there's currently no way to have the source API spit out responses for
stuff like `/source/moose/load/Grunkle`.
2. The frontend code calling the list api now has shared types
3. Code has been migrated to `/lib/handlers/api/source.ts`
- I've moved code here, because I would like to separate the stuff
that's under the API and the stuff that isn't (e.g., healthcheck)
- The source endpoint is currently not under /api, but I believe it
makes sense to move it.
4. GitHub is terrible at showing the diff, but the `browser.ts` file has
been removed, since it's never actually used. Our frontend only calls
`/source/builtin/...` and it never had any entries. Besides, the type
used on the frontend for the locally stored stuff is different...
This commit is contained in:
Mats
2024-11-27 13:54:47 +09:00
committed by GitHub
parent a711eb6544
commit ea29a6e9d6
8 changed files with 103 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
app.ts
View File

@@ -57,11 +57,11 @@ 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 {SourceController} from './lib/handlers/api/source-controller.js';
import {CompileHandler} from './lib/handlers/compile.js';
import * as healthCheck from './lib/handlers/health-check.js';
import {NoScriptHandler} from './lib/handlers/noscript.js';
import {RouteAPI, ShortLinkMetaData} from './lib/handlers/route-api.js';
import {SourceHandler} from './lib/handlers/source.js';
import {languages as allLanguages} from './lib/languages.js';
import {logger, logToLoki, logToPapertrail, makeLogStream, suppressConsoleLog} from './lib/logger.js';
import {setupMetricsServer} from './lib/metrics-server.js';
@@ -335,9 +335,13 @@ const httpRoot = urljoin(ceProps('httpRoot', '/'), '/');
const staticUrl = ceProps<string | undefined>('staticUrl');
const staticRoot = urljoin(staticUrl || urljoin(httpRoot, 'static'), '/');
function staticHeaders(res: express.Response) {
/**
* Sets appropriate headers for static resources. This is based on the staticMaxAgeSecs property from the
* compiler-explorer.properties file.
*/
export function addStaticHeaders(res: express.Response) {
if (staticMaxAgeSecs) {
res.setHeader('Cache-Control', 'public, max-age=' + staticMaxAgeSecs + ', must-revalidate');
res.setHeader('Cache-Control', `public, max-age=${staticMaxAgeSecs}, must-revalidate`);
}
}
@@ -550,7 +554,7 @@ async function main() {
const compileHandler = new CompileHandler(compilationEnvironment, awsProps);
const storageType = getStorageTypeByKey(storageSolution);
const storageHandler = new storageType(httpRoot, compilerProps, awsProps);
const sourceHandler = new SourceHandler(sources, staticHeaders);
const sourceHandler = new SourceController(sources);
const compilerFinder = new CompilerFinder(compileHandler, compilerProps, awsProps, defArgs, clientOptionsHandler);
logger.info('=======================================');
@@ -615,7 +619,7 @@ async function main() {
defArgs,
renderConfig,
renderGoldenLayout,
staticHeaders,
staticHeaders: addStaticHeaders,
contentPolicyHeader,
};
@@ -736,7 +740,7 @@ async function main() {
req: express.Request,
res: express.Response,
) {
staticHeaders(res);
addStaticHeaders(res);
contentPolicyHeader(res);
const embedded = req.query.embedded === 'true' ? true : false;
@@ -757,7 +761,7 @@ async function main() {
}
const embeddedHandler = function (req: express.Request, res: express.Response) {
staticHeaders(res);
addStaticHeaders(res);
contentPolicyHeader(res);
res.render(
'embed',
@@ -802,7 +806,7 @@ async function main() {
)
.use(compression())
.get('/', (req, res) => {
staticHeaders(res);
addStaticHeaders(res);
contentPolicyHeader(res);
res.render(
'index',
@@ -819,7 +823,7 @@ async function main() {
// legacy. not a 301 to prevent any redirect loops between old e links and embed.html
.get('/embed.html', embeddedHandler)
.get('/embed-ro', (req, res) => {
staticHeaders(res);
addStaticHeaders(res);
contentPolicyHeader(res);
res.render(
'embed',
@@ -834,22 +838,22 @@ async function main() {
);
})
.get('/robots.txt', (req, res) => {
staticHeaders(res);
addStaticHeaders(res);
res.end('User-agent: *\nSitemap: https://godbolt.org/sitemap.xml\nDisallow:');
})
.get('/sitemap.xml', (req, res) => {
staticHeaders(res);
addStaticHeaders(res);
res.set('Content-Type', 'application/xml');
res.render('sitemap');
})
.use(sFavicon(utils.resolvePathFromAppRoot('static/favicons', getFaviconFilename())))
.get('/client-options.js', (req, res) => {
staticHeaders(res);
addStaticHeaders(res);
res.set('Content-Type', 'application/javascript');
res.end(`window.compilerExplorerOptions = ${clientOptionsHandler.getJSON()};`);
})
.use('/bits/:bits(\\w+).html', (req, res) => {
staticHeaders(res);
addStaticHeaders(res);
contentPolicyHeader(res);
res.render(
`bits/${sanitize(req.params.bits)}`,
@@ -863,7 +867,8 @@ async function main() {
);
})
.use(bodyParser.json({limit: ceProps('bodyParserLimit', maxUploadSize)}))
.use('/source', sourceHandler.handle.bind(sourceHandler))
.use('/source/:source/list', sourceHandler.listEntries.bind(sourceHandler))
.use('/source/:source/load/:language/:filename', sourceHandler.loadEntry.bind(sourceHandler))
.get('/g/:id', oldGoogleUrlHandler)
// Deprecated old route for this -- TODO remove in late 2021
.post('/shortener', routeApi.apiHandler.shortener.handle.bind(routeApi.apiHandler.shortener));

70
lib/handlers/source.ts → lib/handlers/api/source-controller.ts
View File

@@ -1,4 +1,4 @@
// Copyright (c) 2017, Compiler Explorer Authors
// Copyright (c) 2024, Compiler Explorer Authors
// All rights reserved.
//
// Redistribution and use in source and binary forms, with or without
@@ -25,46 +25,42 @@
import express from 'express';
import _ from 'underscore';
import {Source} from '../sources/index.js';
import {addStaticHeaders} from '../../../app.js';
import {Source} from '../../../types/source.interfaces.js';
// TODO(supergrecko): Maybe provide a more elegant way to do this instead of accessing keys?
const ALLOWED_ACTIONS = new Set(['list', 'load']);
export class SourceController {
public constructor(private readonly sources: Source[]) {}
export class SourceHandler {
public constructor(
private fileSources: Source[],
private addStaticHeaders: (res: express.Response) => void,
) {}
/**
* Handle request to `/source/<source>/list` endpoint
*/
public async listEntries(req: express.Request, res: express.Response) {
const source = this.getSourceForHandler(req.params.source);
if (source === null) {
res.sendStatus(404);
return;
}
const entries = await source.list();
addStaticHeaders(res);
res.json(entries);
}
/**
* Handle request to `/source/<source>/load/<language>/<filename>` endpoint
*/
public async loadEntry(req: express.Request, res: express.Response) {
const source = this.getSourceForHandler(req.params.source);
if (source === null) {
res.sendStatus(404);
return;
}
const entry = await source.load(req.params.language, req.params.filename);
addStaticHeaders(res);
res.json(entry);
}
private getSourceForHandler(handler: string): Source | null {
const records = _.indexBy(this.fileSources, 'urlpart');
const records = _.indexBy(this.sources, 'urlpart');
return records[handler] || null;
}
private getActionForSource(source: Source, action: string): ((...args: unknown[]) => Promise<unknown>) | null {
return ALLOWED_ACTIONS.has(action) ? (source as any)[action] : null;
}
public handle(req: express.Request, res: express.Response, next: express.NextFunction): void {
// Split URLs with the scheme /source/browser/list into the source and the action to perform
const [_, handler, action, ...rest] = req.url.split('/');
const source = this.getSourceForHandler(handler);
if (source === null) {
next();
return;
}
const callback = this.getActionForSource(source, action);
if (callback === null) {
next();
return;
}
callback(...rest)
.then(response => {
this.addStaticHeaders(res);
res.send(response);
})
.catch(err => {
res.send({err: err});
});
}
}

2
lib/options-handler.ts
View File

@@ -32,12 +32,12 @@ import _ from 'underscore';
import {AppDefaultArguments} from '../app.js';
import {CompilerInfo} from '../types/compiler.interfaces.js';
import type {LanguageKey} from '../types/languages.interfaces.js';
import type {Source} from '../types/source.interfaces.js';
import type {ToolTypeKey} from '../types/tool.interfaces.js';
import {logger} from './logger.js';
import type {PropertyGetter, PropertyValue} from './properties.interfaces.js';
import {CompilerProps} from './properties.js';
import {Source} from './sources/index.js';
import {BaseTool, getToolTypeByKey} from './tooling/index.js';
import {asSafeVer, getHash, splitArguments, splitIntoArray} from './utils.js';

5
lib/sources/builtin.ts
View File

@@ -26,10 +26,9 @@ import fs from 'fs';
import fsp from 'fs/promises';
import path from 'path';
import type {Source, SourceApiEntry, SourceEntry} from '../../types/source.interfaces.js';
import * as props from '../properties.js';
import type {Source, SourceEntry} from './index.js';
const EXAMPLES_PATH = props.get('builtin', 'sourcePath', './examples/') as string;
const NAME_SUBSTUTION_PATTERN = new RegExp('_', 'g');
const ALL_EXAMPLES: SourceEntry[] = fs.readdirSync(EXAMPLES_PATH).flatMap(folder => {
@@ -62,7 +61,7 @@ export const builtin: Source = {
return {file: 'Could not read file'};
}
},
async list(): Promise<Omit<SourceEntry, 'path'>[]> {
async list(): Promise<SourceApiEntry[]> {
return ALL_EXAMPLES.map(e => ({
lang: e.lang,
name: e.name,

21
lib/sources/index.ts
View File

@@ -22,25 +22,6 @@
// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
// POSSIBILITY OF SUCH DAMAGE.
import {browser} from './browser.js';
import {builtin} from './builtin.js';
export interface SourceEntry {
/** The file name of the source file */
file: string;
/** The programming language the source file is written in */
lang: string;
/** The "nice" name of the source file, replacing _ with spaces */
name: string;
/** The path to the source file */
path: string;
}
export interface Source {
name: string;
urlpart: string;
list(): Promise<Omit<SourceEntry, 'path'>[]>;
load(language: string, filename: string): Promise<{file: string}>;
}
export const sources = [browser, builtin];
export const sources = [builtin];

7
static/widgets/load-save.ts
View File

@@ -30,6 +30,7 @@ import {Language} from '../../types/languages.interfaces.js';
import {unwrap, unwrapString} from '../assert.js';
import {escapeHTML} from '../../shared/common-utils.js';
import {localStorage} from '../local.js';
import {SourceApiEntry} from '../../types/source.interfaces';
const history = require('../history');
@@ -71,8 +72,8 @@ export class LoadSave {
localStorage.set('files', JSON.stringify(files));
}
private async fetchBuiltins(): Promise<Record<string, any>[]> {
return new Promise<Record<string, any>[]>(resolve => {
private async fetchBuiltins(): Promise<SourceApiEntry[]> {
return new Promise(resolve => {
$.getJSON(window.location.origin + this.base + 'source/builtin/list', resolve);
});
}
@@ -118,7 +119,7 @@ export class LoadSave {
private async populateBuiltins() {
const builtins = (await this.fetchBuiltins()).filter(entry => this.currentLanguage?.id === entry.lang);
return LoadSave.populate(
LoadSave.populate(
unwrap(this.modal).find('.examples'),
builtins.map(elem => {
return {

45
test/handlers/source-tests.ts
View File

@@ -26,35 +26,40 @@ import express from 'express';
import request from 'supertest';
import {describe, expect, it} from 'vitest';
import {SourceHandler} from '../../lib/handlers/source.js';
import {SourceController} from '../../lib/handlers/api/source-controller.js';
describe('Sources', () => {
const app = express();
const handler = new SourceHandler(
[
{
name: 'moose',
urlpart: 'moose',
list: async () => [{file: 'file', lang: 'lang', name: 'name'}],
load: name => Promise.resolve({file: `File called ${name}`}),
},
],
res => res.setHeader('Yibble', 'boing'),
);
app.use('/source', handler.handle.bind(handler));
const handler = new SourceController([
{
name: 'moose',
urlpart: 'moose',
list: async () => [{file: 'file', lang: 'lang', name: 'Grunkle'}],
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));
it('should list', async () => {
const res = await request(app)
await request(app)
.get('/source/moose/list')
.expect('Content-Type', /json/)
.expect(200, [{file: 'file', lang: 'lang', name: 'name'}]);
expect(res.headers['yibble']).toEqual('boing');
.expect(200, [{file: 'file', lang: 'lang', name: 'Grunkle'}]);
});
it('should fetch files', async () => {
const res = await request(app)
.get('/source/moose/load/Grunkle')
await request(app)
.get('/source/moose/load/lang/Grunkle')
.expect('Content-Type', /json/)
.expect(200, {file: 'File called Grunkle'});
expect(res.headers['yibble']).toEqual('boing');
.expect(200, {file: 'File called Grunkle in lang'});
});
it('should have a max-age cache control header', async () => {
const res = await request(app)
.get('/source/moose/load/lang/Grunkle')
.expect('Content-Type', /json/)
.expect(200, {file: 'File called Grunkle in lang'});
expect(res.headers['cache-control']).toContain('public, max-age=');
});
});

33
lib/sources/browser.ts → types/source.interfaces.ts
View File

@@ -1,4 +1,4 @@
// Copyright (c) 2012, Compiler Explorer Authors
// Copyright (c) 2024, Compiler Explorer Authors
// All rights reserved.
//
// Redistribution and use in source and binary forms, with or without
@@ -22,19 +22,22 @@
// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
// POSSIBILITY OF SUCH DAMAGE.
import type {Source, SourceEntry} from './index.js';
export interface SourceEntry {
/** The file name of the source file */
file: string;
/** The programming language the source file is written in */
lang: string;
/** The "nice" name of the source file, replacing _ with spaces */
name: string;
/** The path to the source file */
path: string;
}
// This is a fake plugin. All of the functionality is in the browser code.
export const browser: Source = {
name: 'Browser',
urlpart: 'browser',
list(): Promise<Omit<SourceEntry, 'path'>[]> {
return Promise.resolve([]);
},
load(language: string, filename: string): Promise<{file: string}> {
return Promise.resolve({file: ''});
},
};
export interface Source {
name: string;
urlpart: string;
list(): Promise<SourceApiEntry[]>;
load(language: string, filename: string): Promise<{file: string}>;
}
export const name = 'Browser';
export const urlpart = 'browser';
export type SourceApiEntry = Pick<SourceEntry, 'lang' | 'name' | 'file'>;