Skip to content

Commit

Permalink
chore(rest): review - 1
Browse files Browse the repository at this point in the history
  • Loading branch information
raymondfeng committed Dec 3, 2018
1 parent 9d55526 commit a99ad6a
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 35 deletions.
6 changes: 6 additions & 0 deletions packages/rest/src/keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ export namespace RestBindings {
export const HTTPS_OPTIONS = BindingKey.create<https.ServerOptions>(
'rest.httpsOptions',
);

/**
* Internal binding key for basePath
*/
export const BASE_PATH = BindingKey.create<string>('rest.basePath');

/**
* Internal binding key for http-handler
*/
Expand Down
8 changes: 8 additions & 0 deletions packages/rest/src/rest.application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ export class RestApplication extends Application implements HttpServerLike {
return this.restServer.bodyParser(bodyParserClass, address);
}

/**
* Configure the `basePath` for the rest server
* @param path Base path
*/
basePath(path: string = '') {
this.restServer.basePath(path);
}

/**
* Register a new Controller-based route.
*
Expand Down
42 changes: 32 additions & 10 deletions packages/rest/src/rest.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,18 @@ export class RestServer extends Context implements Server, HttpServerLike {
* @param req The request.
* @param res The response.
*/
public requestHandler: HttpRequestListener;

protected _requestHandler: HttpRequestListener;
public get requestHandler(): HttpRequestListener {
if (this._requestHandler == null) {
this._setupRequestHandlerIfNeeded();
}
return this._requestHandler;
}

public readonly config: RestServerConfig;
private _basePath: string;

protected _httpHandler: HttpHandler;
protected get httpHandler(): HttpHandler {
this._setupHandlerIfNeeded();
Expand Down Expand Up @@ -187,18 +195,14 @@ export class RestServer extends Context implements Server, HttpServerLike {
this.sequence(config.sequence);
}

let basePath = config.basePath || '';
// Trim leading and trailing `/`
basePath = basePath.replace(/(^\/)|(\/$)/, '');
if (basePath) basePath = '/' + basePath;
this._basePath = basePath;

this._setupRequestHandler();
this.basePath(config.basePath);

this.bind(RestBindings.BASE_PATH).toDynamicValue(() => this._basePath);
this.bind(RestBindings.HANDLER).toDynamicValue(() => this.httpHandler);
}

protected _setupRequestHandler() {
protected _setupRequestHandlerIfNeeded() {
if (this._expressApp) return;
this._expressApp = express();

// Disable express' built-in query parser, we parse queries ourselves
Expand All @@ -209,7 +213,7 @@ export class RestServer extends Context implements Server, HttpServerLike {
// that property to be defined. A static singleton object to the rescue!
this._expressApp.set('query parser fn', (str: string) => QUERY_NOT_PARSED);

this.requestHandler = this._expressApp;
this._requestHandler = this._expressApp;

// Allow CORS support for all endpoints so that users
// can test with online SwaggerUI instance
Expand Down Expand Up @@ -756,13 +760,31 @@ export class RestServer extends Context implements Server, HttpServerLike {
return binding;
}

/**
* Configure the `basePath` for the rest server
* @param path Base path
*/
basePath(path: string = '') {
if (this._requestHandler) {
throw new Error(
'Base path cannot be set as the request handler has been created',
);
}
// Trim leading and trailing `/`
path = path.replace(/(^\/)|(\/$)/, '');
if (path) path = '/' + path;
this._basePath = path;
}

/**
* Start this REST API's HTTP/HTTPS server.
*
* @returns {Promise<void>}
* @memberof RestServer
*/
async start(): Promise<void> {
// Set up the Express app if not done yet
this._setupRequestHandlerIfNeeded();
// Setup the HTTP handler so that we can verify the configuration
// of API spec, controllers and routes at startup time.
this._setupHandlerIfNeeded();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,15 @@ describe('RestApplication (integration)', () => {
.expect('Hello');
});

it('honors basePath', async () => {
givenApplication();
restApp.basePath('/html');
restApp.static('/', ASSETS);
await restApp.start();
client = createRestAppClient(restApp);
await client.get('/html/index.html').expect(200);
});

it('returns RestServer instance', async () => {
givenApplication();
const restServer = restApp.restServer;
Expand Down
67 changes: 42 additions & 25 deletions packages/rest/test/integration/rest.server.integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -701,37 +701,54 @@ paths:
await server.stop();
});

it('allows `basePath` for routes', async () => {
describe('basePath', () => {
const root = ASSETS;
const server = await givenAServer({
rest: {
basePath: '/api',
port: 0,
},
let server: RestServer;

beforeEach(async () => {
server = await givenAServer({
rest: {
basePath: '/api',
port: 0,
},
});
});
server.static('/html', root);
server.controller(DummyController);

const content = fs
.readFileSync(path.join(root, 'index.html'))
.toString('utf-8');
await createClientForHandler(server.requestHandler)
.get('/api/html/index.html')
.expect('Content-Type', /text\/html/)
.expect(200, content);
it('controls static assets', async () => {
server.static('/html', root);

await createClientForHandler(server.requestHandler)
.get('/api/html')
.expect(200, 'Hi');
const content = fs
.readFileSync(path.join(root, 'index.html'))
.toString('utf-8');
await createClientForHandler(server.requestHandler)
.get('/api/html/index.html')
.expect('Content-Type', /text\/html/)
.expect(200, content);
});

await createClientForHandler(server.requestHandler)
.get('/html')
.expect(404);
it('controls controller routes', async () => {
server.controller(DummyController);

const response = await createClientForHandler(server.requestHandler).get(
'/openapi.json',
);
expect(response.body.servers).to.containEql({url: '/api'});
await createClientForHandler(server.requestHandler)
.get('/api/html')
.expect(200, 'Hi');
});

it('reports 404 if not found', async () => {
server.static('/html', root);
server.controller(DummyController);

await createClientForHandler(server.requestHandler)
.get('/html')
.expect(404);
});

it('controls server urls', async () => {
const response = await createClientForHandler(server.requestHandler).get(
'/openapi.json',
);
expect(response.body.servers).to.containEql({url: '/api'});
});
});

async function givenAServer(options?: {rest: RestServerConfig}) {
Expand Down
34 changes: 34 additions & 0 deletions packages/rest/test/unit/rest.server/rest.server.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,40 @@ describe('RestServer', () => {
expect(server.getSync(RestBindings.PORT)).to.equal(4000);
expect(server.getSync(RestBindings.HOST)).to.equal('my-host');
});

it('honors basePath in config', async () => {
const app = new Application({
rest: {port: 0, basePath: '/api'},
});
app.component(RestComponent);
const server = await app.getServer(RestServer);
expect(server.getSync(RestBindings.BASE_PATH)).to.equal('/api');
});

it('honors basePath via api', async () => {
const app = new Application({
rest: {port: 0},
});
app.component(RestComponent);
const server = await app.getServer(RestServer);
server.basePath('/api');
expect(server.getSync(RestBindings.BASE_PATH)).to.equal('/api');
});

it('rejects basePath if request handler is created', async () => {
const app = new Application({
rest: {port: 0},
});
app.component(RestComponent);
const server = await app.getServer(RestServer);
expect(() => {
if (server.requestHandler) {
server.basePath('/api');
}
}).to.throw(
/Base path cannot be set as the request handler has been created/,
);
});
});

async function givenRequestContext() {
Expand Down

0 comments on commit a99ad6a

Please sign in to comment.