From 7afcff6d78e33bce2e2290e080fe1136f23b27fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Wed, 9 May 2018 16:58:57 +0200 Subject: [PATCH 1/2] refactor(rest): rename internal-types to types The types defined in "internal-types.ts" are not internal at all, for example they include signatures of sequence actions. This commit renames the file to "types.ts", which is a file name already used in other packages (let's be consistent). --- packages/rest/docs.json | 2 +- packages/rest/src/http-handler.ts | 2 +- packages/rest/src/index.ts | 2 +- packages/rest/src/keys.ts | 2 +- packages/rest/src/parser.ts | 6 +----- packages/rest/src/providers/bind-element.provider.ts | 2 +- packages/rest/src/providers/find-route.provider.ts | 3 +-- .../rest/src/providers/get-from-context.provider.ts | 2 +- packages/rest/src/providers/invoke-method.provider.ts | 2 +- packages/rest/src/providers/log-error.provider.ts | 2 +- packages/rest/src/providers/reject.provider.ts | 2 +- packages/rest/src/rest.server.ts | 10 ++-------- packages/rest/src/router/routing-table.ts | 2 +- packages/rest/src/sequence.ts | 2 +- packages/rest/src/{internal-types.ts => types.ts} | 0 packages/rest/src/writer.ts | 2 +- 16 files changed, 16 insertions(+), 27 deletions(-) rename packages/rest/src/{internal-types.ts => types.ts} (100%) diff --git a/packages/rest/docs.json b/packages/rest/docs.json index ea4673e1b978..51c8b07cc945 100644 --- a/packages/rest/docs.json +++ b/packages/rest/docs.json @@ -3,7 +3,7 @@ "index.ts", "src/http-handler.ts", "src/index.ts", - "src/internal-types.ts", + "src/types.ts", "src/keys.ts", "src/parser.ts", "src/rest.application.ts", diff --git a/packages/rest/src/http-handler.ts b/packages/rest/src/http-handler.ts index a3e0dffa3a38..ace4e6cd787d 100644 --- a/packages/rest/src/http-handler.ts +++ b/packages/rest/src/http-handler.ts @@ -17,7 +17,7 @@ import { ControllerClass, ControllerFactory, } from './router/routing-table'; -import {ParsedRequest} from './internal-types'; +import {ParsedRequest} from './types'; import {RestBindings} from './keys'; diff --git a/packages/rest/src/index.ts b/packages/rest/src/index.ts index 450481d764fd..1c8145bc5954 100644 --- a/packages/rest/src/index.ts +++ b/packages/rest/src/index.ts @@ -35,7 +35,7 @@ export {writeResultToResponse} from './writer'; export {HttpErrors}; export * from './http-handler'; -export * from './internal-types'; +export * from './types'; export * from './keys'; export * from './rest.application'; export * from './rest.component'; diff --git a/packages/rest/src/keys.ts b/packages/rest/src/keys.ts index 87648bd2714b..db91b7e25437 100644 --- a/packages/rest/src/keys.ts +++ b/packages/rest/src/keys.ts @@ -20,7 +20,7 @@ import { ParseParams, Reject, Send, -} from './internal-types'; +} from './types'; // NOTE(bajtos) The following import is required to satisfy TypeScript compiler // tslint:disable-next-line:no-unused-variable diff --git a/packages/rest/src/parser.ts b/packages/rest/src/parser.ts index 210c361ed2fc..8fbb88ec9f21 100644 --- a/packages/rest/src/parser.ts +++ b/packages/rest/src/parser.ts @@ -12,11 +12,7 @@ import { } from '@loopback/openapi-v3-types'; import {REQUEST_BODY_INDEX} from '@loopback/openapi-v3'; import {promisify} from 'util'; -import { - OperationArgs, - ParsedRequest, - PathParameterValues, -} from './internal-types'; +import {OperationArgs, ParsedRequest, PathParameterValues} from './types'; import {ResolvedRoute} from './router/routing-table'; type HttpError = HttpErrors.HttpError; diff --git a/packages/rest/src/providers/bind-element.provider.ts b/packages/rest/src/providers/bind-element.provider.ts index 699c1847d133..289b15ac304f 100644 --- a/packages/rest/src/providers/bind-element.provider.ts +++ b/packages/rest/src/providers/bind-element.provider.ts @@ -4,7 +4,7 @@ // License text available at https://opensource.org/licenses/MIT import {Context, inject, Provider, Binding} from '@loopback/context'; -import {BindElement} from '../internal-types'; +import {BindElement} from '../types'; import {RestBindings} from '../keys'; export class BindElementProvider implements Provider { diff --git a/packages/rest/src/providers/find-route.provider.ts b/packages/rest/src/providers/find-route.provider.ts index 0bfc5cb65837..fb3cdc235b5f 100644 --- a/packages/rest/src/providers/find-route.provider.ts +++ b/packages/rest/src/providers/find-route.provider.ts @@ -4,10 +4,9 @@ // License text available at https://opensource.org/licenses/MIT import {Context, inject, Provider} from '@loopback/context'; -import {FindRoute} from '../internal-types'; +import {FindRoute, ParsedRequest} from '../types'; import {HttpHandler} from '../http-handler'; import {RestBindings} from '../keys'; -import {ParsedRequest} from '../internal-types'; import {ResolvedRoute} from '../router/routing-table'; export class FindRouteProvider implements Provider { diff --git a/packages/rest/src/providers/get-from-context.provider.ts b/packages/rest/src/providers/get-from-context.provider.ts index e3ed0957dd7c..9693dc775728 100644 --- a/packages/rest/src/providers/get-from-context.provider.ts +++ b/packages/rest/src/providers/get-from-context.provider.ts @@ -4,7 +4,7 @@ // License text available at https://opensource.org/licenses/MIT import {Context, inject, Provider, BoundValue} from '@loopback/context'; -import {GetFromContext} from '../internal-types'; +import {GetFromContext} from '../types'; import {RestBindings} from '../keys'; export class GetFromContextProvider implements Provider { diff --git a/packages/rest/src/providers/invoke-method.provider.ts b/packages/rest/src/providers/invoke-method.provider.ts index 3b85944b6739..42289269f9ad 100644 --- a/packages/rest/src/providers/invoke-method.provider.ts +++ b/packages/rest/src/providers/invoke-method.provider.ts @@ -4,7 +4,7 @@ // License text available at https://opensource.org/licenses/MIT import {Context, inject, Provider} from '@loopback/context'; -import {InvokeMethod, OperationArgs, OperationRetval} from '../internal-types'; +import {InvokeMethod, OperationArgs, OperationRetval} from '../types'; import {RestBindings} from '../keys'; import {RouteEntry} from '../router/routing-table'; diff --git a/packages/rest/src/providers/log-error.provider.ts b/packages/rest/src/providers/log-error.provider.ts index b2aebfb19595..077befa700a8 100644 --- a/packages/rest/src/providers/log-error.provider.ts +++ b/packages/rest/src/providers/log-error.provider.ts @@ -5,7 +5,7 @@ import {Provider} from '@loopback/context'; import {ServerRequest} from 'http'; -import {LogError} from '../internal-types'; +import {LogError} from '../types'; export class LogErrorProvider implements Provider { value(): LogError { diff --git a/packages/rest/src/providers/reject.provider.ts b/packages/rest/src/providers/reject.provider.ts index 6aee9ac512e2..4103bf391a0f 100644 --- a/packages/rest/src/providers/reject.provider.ts +++ b/packages/rest/src/providers/reject.provider.ts @@ -3,7 +3,7 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {LogError, Reject} from '../internal-types'; +import {LogError, Reject} from '../types'; import {inject, Provider} from '@loopback/context'; import {ServerResponse, ServerRequest} from 'http'; import {HttpError} from 'http-errors'; diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts index 3652f7947b68..b9d03d036be5 100644 --- a/packages/rest/src/rest.server.ts +++ b/packages/rest/src/rest.server.ts @@ -15,7 +15,7 @@ import { ControllerInstance, createControllerFactoryForBinding, } from './router/routing-table'; -import {ParsedRequest} from './internal-types'; +import {ParsedRequest} from './types'; import {OpenApiSpec, OperationObject} from '@loopback/openapi-v3-types'; import {ServerRequest, ServerResponse, createServer} from 'http'; import * as Http from 'http'; @@ -24,13 +24,7 @@ import {Application, CoreBindings, Server} from '@loopback/core'; import {getControllerSpec} from '@loopback/openapi-v3'; import {HttpHandler} from './http-handler'; import {DefaultSequence, SequenceHandler, SequenceFunction} from './sequence'; -import { - FindRoute, - InvokeMethod, - Send, - Reject, - ParseParams, -} from './internal-types'; +import {FindRoute, InvokeMethod, Send, Reject, ParseParams} from './types'; import {RestBindings} from './keys'; export type HttpRequestListener = ( diff --git a/packages/rest/src/router/routing-table.ts b/packages/rest/src/router/routing-table.ts index 8c166925ed4c..3e68902b2209 100644 --- a/packages/rest/src/router/routing-table.ts +++ b/packages/rest/src/router/routing-table.ts @@ -24,7 +24,7 @@ import { PathParameterValues, OperationArgs, OperationRetval, -} from '../internal-types'; +} from '../types'; import {ControllerSpec} from '@loopback/openapi-v3'; diff --git a/packages/rest/src/sequence.ts b/packages/rest/src/sequence.ts index e57da1f97430..25a566dad74d 100644 --- a/packages/rest/src/sequence.ts +++ b/packages/rest/src/sequence.ts @@ -13,7 +13,7 @@ import { Send, Reject, ParseParams, -} from './internal-types'; +} from './types'; import {RestBindings} from './keys'; const SequenceActions = RestBindings.SequenceActions; diff --git a/packages/rest/src/internal-types.ts b/packages/rest/src/types.ts similarity index 100% rename from packages/rest/src/internal-types.ts rename to packages/rest/src/types.ts diff --git a/packages/rest/src/writer.ts b/packages/rest/src/writer.ts index 9c012265cad4..f1532d56af4f 100644 --- a/packages/rest/src/writer.ts +++ b/packages/rest/src/writer.ts @@ -4,7 +4,7 @@ // License text available at https://opensource.org/licenses/MIT import {ServerResponse as Response} from 'http'; -import {OperationRetval} from './internal-types'; +import {OperationRetval} from './types'; import {HttpError} from 'http-errors'; import {Readable} from 'stream'; From c7095719d07cc0d9c09aa5755f9d611bcff9fe5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 10 May 2018 08:35:25 +0200 Subject: [PATCH 2/2] refactor(rest): introduce RequestContext Rework all places where we were accepting a pair of `(ParsedRequest, ServerResponse)` to use an context object instead. Two context types are introduced: `HandlerContext` is an interface describing objects required to handle an incoming HTTP request. There are two properties for starter: - context.request - context.response Low-level entities like Sequence Actions should be using this interface to allow easy interoperability with potentially any HTTP framework and more importantly, to keep the code easy to test. `RequestContext` is a class extending our IoC `Context` and implementing `HandlerContext` interface. This object is used when invoking the sequence handler. By combining both IoC and HTTP contexts into a single object, there will be (hopefully) less confusion among LB4 users about what shape a "context" object has in different places. This is a breaking change affecting the following users: - Custom sequence classes - Custom sequence handlers registered via `app.handler` - Custom implementations of the built-in sequence action `Reject` --- .prettierignore | 1 + docs/site/Sequence.md | 24 +++++------ examples/hello-world/src/application.ts | 2 +- .../acceptance/log.extension.acceptance.ts | 24 +++++------ examples/todo/src/sequence.ts | 16 +++---- .../test/acceptance/basic-auth.acceptance.ts | 18 ++++---- .../app/templates/src/sequence.ts.ejs | 17 ++++---- packages/rest/README.md | 2 +- packages/rest/src/http-handler.ts | 20 +++------ packages/rest/src/index.ts | 1 + .../rest/src/providers/reject.provider.ts | 7 ++- packages/rest/src/request-context.ts | 39 +++++++++++++++++ packages/rest/src/rest.server.ts | 16 +++---- packages/rest/src/sequence.ts | 39 ++++++++--------- packages/rest/src/types.ts | 19 +++++--- .../bootstrapping/rest.acceptance.ts | 5 +-- .../acceptance/routing/routing.acceptance.ts | 2 +- .../sequence/sequence.acceptance.ts | 43 +++++++++---------- .../integration/rest.server.integration.ts | 6 +-- .../test/unit/router/reject.provider.unit.ts | 18 ++++++-- 20 files changed, 180 insertions(+), 139 deletions(-) create mode 100644 packages/rest/src/request-context.ts diff --git a/.prettierignore b/.prettierignore index 75511ca86f10..a338cbbd096c 100644 --- a/.prettierignore +++ b/.prettierignore @@ -4,5 +4,6 @@ packages/cli/generators/*/templates packages/cli/test/integration/.sandbox packages/*/dist* examples/*/dist* +sandbox/**/dist* *.json *.md diff --git a/docs/site/Sequence.md b/docs/site/Sequence.md index 10e0590143ef..119b53667a9b 100644 --- a/docs/site/Sequence.md +++ b/docs/site/Sequence.md @@ -24,14 +24,14 @@ instances handle requests and responses. The `DefaultSequence` looks like this: ```ts class DefaultSequence { - async handle(request: ParsedRequest, response: ServerResponse) { + async handle(context: RequestContext) { try { - const route = this.findRoute(request); - const params = await this.parseParams(request, route); + const route = this.findRoute(context.request); + const params = await this.parseParams(context.request, route); const result = await this.invoke(route, params); - await this.send(response, result); - } catch (err) { - await this.reject(response, err); + await this.send(context.response, result); + } catch (error) { + await this.reject(context, error); } } } @@ -61,15 +61,15 @@ Actions: ```ts class MySequence extends DefaultSequence { - async handle(request: ParsedRequest, response: ServerResponse) { + async handle(context: RequestContext) { // findRoute() produces an element - const route = this.findRoute(request); + const route = this.findRoute(context.request); // parseParams() uses the route element and produces the params element - const params = await this.parseParams(request, route); + const params = await this.parseParams(context.request, route); // invoke() uses both the route and params elements to produce the result (OperationRetVal) element const result = await this.invoke(route, params); // send() uses the result element - await this.send(response, result); + await this.send(context.response, result); } } ``` @@ -91,9 +91,9 @@ class MySequence extends DefaultSequence { log(msg: string) { console.log(msg); } - async handle(request: ParsedRequest, response: ServerResponse) { + async handle(context: RequestContext) { this.log('before request'); - await super.handle(request, response); + await super.handle(context); this.log('after request'); } } diff --git a/examples/hello-world/src/application.ts b/examples/hello-world/src/application.ts index bfa570c88383..53472d32f596 100644 --- a/examples/hello-world/src/application.ts +++ b/examples/hello-world/src/application.ts @@ -13,7 +13,7 @@ export class HelloWorldApplication extends RestApplication { // returns the same HTTP response: Hello World! // Learn more about the concept of Sequence in our docs: // http://loopback.io/doc/en/lb4/Sequence.html - this.handler((sequence, request, response) => { + this.handler(({response}, sequence) => { sequence.send(response, 'Hello World!'); }); } diff --git a/examples/log-extension/test/acceptance/log.extension.acceptance.ts b/examples/log-extension/test/acceptance/log.extension.acceptance.ts index 5cc752dc4337..4178c9e81937 100644 --- a/examples/log-extension/test/acceptance/log.extension.acceptance.ts +++ b/examples/log-extension/test/acceptance/log.extension.acceptance.ts @@ -12,8 +12,7 @@ import { InvokeMethod, Send, Reject, - ParsedRequest, - ServerResponse, + RequestContext, } from '@loopback/rest'; import {get, param} from '@loopback/openapi-v3'; import { @@ -31,7 +30,7 @@ import { createClientForHandler, expect, } from '@loopback/testlab'; -import {Context, inject} from '@loopback/context'; +import {inject} from '@loopback/context'; import chalk from 'chalk'; const SequenceActions = RestBindings.SequenceActions; @@ -204,7 +203,6 @@ describe('log extension acceptance test', () => { function createSequence() { class LogSequence implements SequenceHandler { constructor( - @inject(RestBindings.Http.CONTEXT) public ctx: Context, @inject(SequenceActions.FIND_ROUTE) protected findRoute: FindRoute, @inject(SequenceActions.PARSE_PARAMS) protected parseParams: ParseParams, @@ -214,23 +212,25 @@ describe('log extension acceptance test', () => { @inject(EXAMPLE_LOG_BINDINGS.LOG_ACTION) protected logger: LogFn, ) {} - async handle(req: ParsedRequest, res: ServerResponse) { + async handle(context: RequestContext): Promise { + const {request, response} = context; + // tslint:disable-next-line:no-any let args: any = []; // tslint:disable-next-line:no-any let result: any; try { - const route = this.findRoute(req); - args = await this.parseParams(req, route); + const route = this.findRoute(request); + args = await this.parseParams(request, route); result = await this.invoke(route, args); - this.send(res, result); - } catch (err) { - this.reject(res, req, err); - result = err; + this.send(response, result); + } catch (error) { + this.reject(context, error); + result = error; } - await this.logger(req, args, result); + await this.logger(request, args, result); } } diff --git a/examples/todo/src/sequence.ts b/examples/todo/src/sequence.ts index 55f2eb029e03..238a4520c8db 100644 --- a/examples/todo/src/sequence.ts +++ b/examples/todo/src/sequence.ts @@ -7,14 +7,13 @@ import {Context, inject} from '@loopback/context'; import { FindRoute, InvokeMethod, - ParsedRequest, ParseParams, Reject, + RequestContext, RestBindings, Send, SequenceHandler, } from '@loopback/rest'; -import {ServerResponse} from 'http'; const SequenceActions = RestBindings.SequenceActions; @@ -28,14 +27,15 @@ export class MySequence implements SequenceHandler { @inject(SequenceActions.REJECT) public reject: Reject, ) {} - async handle(req: ParsedRequest, res: ServerResponse) { + async handle(context: RequestContext) { try { - const route = this.findRoute(req); - const args = await this.parseParams(req, route); + const {request, response} = context; + const route = this.findRoute(request); + const args = await this.parseParams(request, route); const result = await this.invoke(route, args); - this.send(res, result); - } catch (err) { - this.reject(res, req, err); + this.send(response, result); + } catch (error) { + this.reject(context, error); } } } diff --git a/packages/authentication/test/acceptance/basic-auth.acceptance.ts b/packages/authentication/test/acceptance/basic-auth.acceptance.ts index cb26a4d569b7..29746836c2b9 100644 --- a/packages/authentication/test/acceptance/basic-auth.acceptance.ts +++ b/packages/authentication/test/acceptance/basic-auth.acceptance.ts @@ -6,8 +6,6 @@ import {Application} from '@loopback/core'; import { RestBindings, - ServerResponse, - ParsedRequest, ParseParams, FindRoute, InvokeMethod, @@ -16,6 +14,7 @@ import { SequenceHandler, RestServer, RestComponent, + RequestContext, } from '@loopback/rest'; import {api, get} from '@loopback/openapi-v3'; import {Client, createClientForHandler} from '@loopback/testlab'; @@ -137,19 +136,20 @@ describe('Basic Authentication', () => { protected authenticateRequest: AuthenticateFn, ) {} - async handle(req: ParsedRequest, res: ServerResponse) { + async handle(context: RequestContext) { try { - const route = this.findRoute(req); + const {request, response} = context; + const route = this.findRoute(request); // Authenticate - await this.authenticateRequest(req); + await this.authenticateRequest(request); // Authentication successful, proceed to invoke controller - const args = await this.parseParams(req, route); + const args = await this.parseParams(request, route); const result = await this.invoke(route, args); - this.send(res, result); - } catch (err) { - this.reject(res, req, err); + this.send(response, result); + } catch (error) { + this.reject(context, error); return; } } diff --git a/packages/cli/generators/app/templates/src/sequence.ts.ejs b/packages/cli/generators/app/templates/src/sequence.ts.ejs index 962946f894ce..24339017ff1e 100644 --- a/packages/cli/generators/app/templates/src/sequence.ts.ejs +++ b/packages/cli/generators/app/templates/src/sequence.ts.ejs @@ -1,21 +1,19 @@ -import {Context, inject} from '@loopback/context'; +import {inject} from '@loopback/context'; import { FindRoute, InvokeMethod, - ParsedRequest, ParseParams, Reject, + RequestContext, RestBindings, Send, SequenceHandler, } from '@loopback/rest'; -import {ServerResponse} from 'http'; const SequenceActions = RestBindings.SequenceActions; export class MySequence implements SequenceHandler { constructor( - @inject(RestBindings.Http.CONTEXT) public ctx: Context, @inject(SequenceActions.FIND_ROUTE) protected findRoute: FindRoute, @inject(SequenceActions.PARSE_PARAMS) protected parseParams: ParseParams, @inject(SequenceActions.INVOKE_METHOD) protected invoke: InvokeMethod, @@ -23,14 +21,15 @@ export class MySequence implements SequenceHandler { @inject(SequenceActions.REJECT) public reject: Reject, ) {} - async handle(req: ParsedRequest, res: ServerResponse) { + async handle(context: RequestContext) { try { - const route = this.findRoute(req); - const args = await this.parseParams(req, route); + const {request, response} = context; + const route = this.findRoute(request); + const args = await this.parseParams(request, route); const result = await this.invoke(route, args); - this.send(res, result); + this.send(response, result); } catch (err) { - this.reject(res, req, err); + this.reject(context, err); } } } diff --git a/packages/rest/README.md b/packages/rest/README.md index 09d61dcdee45..491a3658f5de 100644 --- a/packages/rest/README.md +++ b/packages/rest/README.md @@ -29,7 +29,7 @@ Here's a basic "Hello World" application using `@loopback/rest`: import {RestApplication, RestServer} from '@loopback/rest'; const app = new RestApplication(); -app.handler((sequence, request, response) => { +app.handler(({request, response}, sequence) => { sequence.send(response, 'hello world'); }); diff --git a/packages/rest/src/http-handler.ts b/packages/rest/src/http-handler.ts index ace4e6cd787d..d90c600162fb 100644 --- a/packages/rest/src/http-handler.ts +++ b/packages/rest/src/http-handler.ts @@ -20,6 +20,7 @@ import { import {ParsedRequest} from './types'; import {RestBindings} from './keys'; +import {RequestContext} from './request-context'; export class HttpHandler { protected _routes: RoutingTable = new RoutingTable(); @@ -67,22 +68,15 @@ export class HttpHandler { response: ServerResponse, ): Promise { const parsedRequest: ParsedRequest = parseRequestUrl(request); - const requestContext = this._createRequestContext(parsedRequest, response); + const requestContext = new RequestContext( + parsedRequest, + response, + this._rootContext, + ); const sequence = await requestContext.get( RestBindings.SEQUENCE, ); - await sequence.handle(parsedRequest, response); - } - - protected _createRequestContext( - req: ParsedRequest, - res: ServerResponse, - ): Context { - const requestContext = new Context(this._rootContext); - requestContext.bind(RestBindings.Http.REQUEST).to(req); - requestContext.bind(RestBindings.Http.RESPONSE).to(res); - requestContext.bind(RestBindings.Http.CONTEXT).to(requestContext); - return requestContext; + await sequence.handle(requestContext); } } diff --git a/packages/rest/src/index.ts b/packages/rest/src/index.ts index 1c8145bc5954..80f06b45a196 100644 --- a/packages/rest/src/index.ts +++ b/packages/rest/src/index.ts @@ -35,6 +35,7 @@ export {writeResultToResponse} from './writer'; export {HttpErrors}; export * from './http-handler'; +export * from './request-context'; export * from './types'; export * from './keys'; export * from './rest.application'; diff --git a/packages/rest/src/providers/reject.provider.ts b/packages/rest/src/providers/reject.provider.ts index 4103bf391a0f..3164f70f7225 100644 --- a/packages/rest/src/providers/reject.provider.ts +++ b/packages/rest/src/providers/reject.provider.ts @@ -3,9 +3,8 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {LogError, Reject} from '../types'; +import {LogError, Reject, HandlerContext} from '../types'; import {inject, Provider} from '@loopback/context'; -import {ServerResponse, ServerRequest} from 'http'; import {HttpError} from 'http-errors'; import {writeErrorToResponse} from '../writer'; import {RestBindings} from '../keys'; @@ -17,10 +16,10 @@ export class RejectProvider implements Provider { ) {} value(): Reject { - return (response, request, error) => this.action(response, request, error); + return (context, error) => this.action(context, error); } - action(response: ServerResponse, request: ServerRequest, error: Error) { + action({request, response}: HandlerContext, error: Error) { const err = error; const statusCode = err.statusCode || err.status || 500; writeErrorToResponse(response, err); diff --git a/packages/rest/src/request-context.ts b/packages/rest/src/request-context.ts new file mode 100644 index 000000000000..38092754802a --- /dev/null +++ b/packages/rest/src/request-context.ts @@ -0,0 +1,39 @@ +// Copyright IBM Corp. 2017. All Rights Reserved. +// Node module: @loopback/rest +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +import {Context} from '@loopback/context'; +import {ServerResponse} from 'http'; +import {HandlerContext, ParsedRequest} from './types'; +import {RestBindings} from './keys'; + +/** + * A per-request Context combining an IoC container with handler context + * (request, response, etc.). + */ +export class RequestContext extends Context implements HandlerContext { + constructor( + public readonly request: ParsedRequest, + public readonly response: ServerResponse, + parent: Context, + name?: string, + ) { + super(parent, name); + this._setupBindings(request, response); + } + + private _setupBindings(request: ParsedRequest, response: ServerResponse) { + this.bind(RestBindings.Http.REQUEST) + .to(request) + .lock(); + + this.bind(RestBindings.Http.RESPONSE) + .to(response) + .lock(); + + this.bind(RestBindings.Http.CONTEXT) + .to(this) + .lock(); + } +} diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts index b9d03d036be5..0e32318111f8 100644 --- a/packages/rest/src/rest.server.ts +++ b/packages/rest/src/rest.server.ts @@ -15,7 +15,6 @@ import { ControllerInstance, createControllerFactoryForBinding, } from './router/routing-table'; -import {ParsedRequest} from './types'; import {OpenApiSpec, OperationObject} from '@loopback/openapi-v3-types'; import {ServerRequest, ServerResponse, createServer} from 'http'; import * as Http from 'http'; @@ -26,6 +25,7 @@ import {HttpHandler} from './http-handler'; import {DefaultSequence, SequenceHandler, SequenceFunction} from './sequence'; import {FindRoute, InvokeMethod, Send, Reject, ParseParams} from './types'; import {RestBindings} from './keys'; +import {RequestContext} from '.'; export type HttpRequestListener = ( req: ServerRequest, @@ -507,7 +507,7 @@ export class RestServer extends Context implements Server, HttpServerLike { * @inject('send) public send: Send)) { * } * - * public async handle(request: ParsedRequest, response: ServerResponse) { + * public async handle({response}: RequestContext) { * send(response, 'hello world'); * } * } @@ -523,7 +523,7 @@ export class RestServer extends Context implements Server, HttpServerLike { * Configure a custom sequence function for handling incoming requests. * * ```ts - * app.handler((sequence, request, response) => { + * app.handler(({request, response}, sequence) => { * sequence.send(response, 'hello world'); * }); * ``` @@ -535,7 +535,6 @@ export class RestServer extends Context implements Server, HttpServerLike { // NOTE(bajtos) Unfortunately, we have to duplicate the constructor // in order for our DI/IoC framework to inject constructor arguments constructor( - @inject(RestBindings.Http.CONTEXT) public ctx: Context, @inject(SequenceActions.FIND_ROUTE) protected findRoute: FindRoute, @inject(SequenceActions.PARSE_PARAMS) protected parseParams: ParseParams, @@ -543,14 +542,11 @@ export class RestServer extends Context implements Server, HttpServerLike { @inject(SequenceActions.SEND) public send: Send, @inject(SequenceActions.REJECT) public reject: Reject, ) { - super(ctx, findRoute, parseParams, invoke, send, reject); + super(findRoute, parseParams, invoke, send, reject); } - async handle( - request: ParsedRequest, - response: ServerResponse, - ): Promise { - await Promise.resolve(handlerFn(this, request, response)); + async handle(context: RequestContext): Promise { + await Promise.resolve(handlerFn(context, this)); } } diff --git a/packages/rest/src/sequence.ts b/packages/rest/src/sequence.ts index 25a566dad74d..31de58f5c689 100644 --- a/packages/rest/src/sequence.ts +++ b/packages/rest/src/sequence.ts @@ -4,8 +4,7 @@ // License text available at https://opensource.org/licenses/MIT const debug = require('debug')('loopback:core:sequence'); -import {ServerResponse} from 'http'; -import {inject, Context} from '@loopback/context'; +import {inject} from '@loopback/context'; import { FindRoute, InvokeMethod, @@ -15,6 +14,7 @@ import { ParseParams, } from './types'; import {RestBindings} from './keys'; +import {RequestContext} from './request-context'; const SequenceActions = RestBindings.SequenceActions; @@ -23,9 +23,8 @@ const SequenceActions = RestBindings.SequenceActions; * sequence of actions to handle an incoming request. */ export type SequenceFunction = ( + context: RequestContext, sequence: DefaultSequence, - request: ParsedRequest, - response: ServerResponse, ) => Promise | void; /** @@ -36,10 +35,10 @@ export interface SequenceHandler { /** * Handle the request by running the configured sequence of actions. * - * @param request The incoming HTTP request - * @param response The HTTP server response where to write the result + * @param context The request context: HTTP request and response objects, + * per-request IoC container and more. */ - handle(request: ParsedRequest, response: ServerResponse): Promise; + handle(context: RequestContext): Promise; } /** @@ -63,8 +62,6 @@ export class DefaultSequence implements SequenceHandler { * Constructor: Injects findRoute, invokeMethod & logError * methods as promises. * - * @param {Context} ctx The context for the sequence (injected via - * RestBindings.Http.CONTEXT). * @param {FindRoute} findRoute Finds the appropriate controller method, * spec and args for invocation (injected via SequenceActions.FIND_ROUTE). * @param {ParseParams} parseParams The parameter parsing function (injected @@ -77,7 +74,6 @@ export class DefaultSequence implements SequenceHandler { * promise result (injected via SequenceActions.REJECT). */ constructor( - @inject(RestBindings.Http.CONTEXT) public ctx: Context, @inject(SequenceActions.FIND_ROUTE) protected findRoute: FindRoute, @inject(SequenceActions.PARSE_PARAMS) protected parseParams: ParseParams, @inject(SequenceActions.INVOKE_METHOD) protected invoke: InvokeMethod, @@ -86,8 +82,8 @@ export class DefaultSequence implements SequenceHandler { ) {} /** - * Runs the default sequence. Given a request and response, running the - * sequence will produce a response or an error. + * Runs the default sequence. Given a handler context (request and response), + * running the sequence will produce a response or an error. * * Default sequence executes these steps * - Finds the appropriate controller method, swagger spec @@ -97,20 +93,21 @@ export class DefaultSequence implements SequenceHandler { * - Writes the result from API into the HTTP response * - Error is caught and logged using 'logError' if any of the above steps * in the sequence fails with an error. - * @param req Parsed incoming HTTP request - * @param res HTTP server response with result from Application controller - * method invocation + * + * @param context The request context: HTTP request and response objects, + * per-request IoC container and more. */ - async handle(req: ParsedRequest, res: ServerResponse) { + async handle(context: RequestContext): Promise { try { - const route = this.findRoute(req); - const args = await this.parseParams(req, route); + const {request, response} = context; + const route = this.findRoute(request); + const args = await this.parseParams(request, route); const result = await this.invoke(route, args); debug('%s result -', route.describe(), result); - this.send(res, result); - } catch (err) { - this.reject(res, req, err); + this.send(response, result); + } catch (error) { + this.reject(context, error); } } } diff --git a/packages/rest/src/types.ts b/packages/rest/src/types.ts index 74e9694e9c36..03430bfa7946 100644 --- a/packages/rest/src/types.ts +++ b/packages/rest/src/types.ts @@ -18,6 +18,15 @@ export interface ParsedRequest extends ServerRequest { method: string; } +/** + * An object holding HTTP request, response and other data + * needed to handle an incoming HTTP request. + */ +export interface HandlerContext { + readonly request: ParsedRequest; + readonly response: ServerResponse; +} + /** * Find a route matching the incoming request. * Throw an error when no route was found. @@ -57,15 +66,11 @@ export type Send = (response: ServerResponse, result: OperationRetval) => void; /** * Reject the request with an error. * - * @param response The response the response to send to. - * @param request The request that triggered the error. + * @param handlerContext The context object holding HTTP request, response + * and other data needed to handle an incoming HTTP request. * @param err The error. */ -export type Reject = ( - response: ServerResponse, - request: ServerRequest, - err: Error, -) => void; +export type Reject = (handlerContext: HandlerContext, err: Error) => void; /** * Log information about a failed request. diff --git a/packages/rest/test/acceptance/bootstrapping/rest.acceptance.ts b/packages/rest/test/acceptance/bootstrapping/rest.acceptance.ts index 17315f0aa8d4..002a041cf1e6 100644 --- a/packages/rest/test/acceptance/bootstrapping/rest.acceptance.ts +++ b/packages/rest/test/acceptance/bootstrapping/rest.acceptance.ts @@ -10,9 +10,9 @@ import { RestServer, RestComponent, RestApplication, + RequestContext, } from '../../..'; import {Application} from '@loopback/core'; -import {ServerResponse, ServerRequest} from 'http'; describe('Bootstrapping with RestComponent', () => { context('with a user-defined sequence', () => { @@ -76,9 +76,8 @@ async function startServerCheck(app: Application) { } function sequenceHandler( + {request, response}: RequestContext, sequence: DefaultSequence, - request: ServerRequest, - response: ServerResponse, ) { sequence.send(response, 'hello world'); } diff --git a/packages/rest/test/acceptance/routing/routing.acceptance.ts b/packages/rest/test/acceptance/routing/routing.acceptance.ts index 6ee9d6fae8e4..b7be41381634 100644 --- a/packages/rest/test/acceptance/routing/routing.acceptance.ts +++ b/packages/rest/test/acceptance/routing/routing.acceptance.ts @@ -694,7 +694,7 @@ describe('Routing', () => { it('provides httpHandler compatible with HTTP server API', async () => { const app = new RestApplication(); - app.handler((sequence, req, res) => res.end('hello')); + app.handler(({request, response}, sequence) => response.end('hello')); await createClientForHandler(app.requestHandler) .get('/') diff --git a/packages/rest/test/acceptance/sequence/sequence.acceptance.ts b/packages/rest/test/acceptance/sequence/sequence.acceptance.ts index f9c49c18fd98..157065d08984 100644 --- a/packages/rest/test/acceptance/sequence/sequence.acceptance.ts +++ b/packages/rest/test/acceptance/sequence/sequence.acceptance.ts @@ -4,7 +4,6 @@ // License text available at https://opensource.org/licenses/MIT import { - ServerResponse, ParsedRequest, FindRoute, InvokeMethod, @@ -18,12 +17,13 @@ import { RestComponent, RestApplication, HttpServerLike, + RequestContext, } from '../../..'; import {api} from '@loopback/openapi-v3'; import {Application} from '@loopback/core'; -import {expect, Client, createClientForHandler} from '@loopback/testlab'; +import {Client, createClientForHandler} from '@loopback/testlab'; import {anOpenApiSpec} from '@loopback/openapi-spec-builder'; -import {inject, Context} from '@loopback/context'; +import {inject} from '@loopback/context'; import { ControllerClass, ControllerInstance, @@ -47,7 +47,7 @@ describe('Sequence', () => { }); it('allows users to define a custom sequence as a function', () => { - server.handler((sequence, request, response) => { + server.handler(({request, response}, sequence) => { sequence.send(response, 'hello world'); }); return whenIRequest() @@ -59,8 +59,8 @@ describe('Sequence', () => { class MySequence implements SequenceHandler { constructor(@inject(SequenceActions.SEND) private send: Send) {} - async handle(req: ParsedRequest, res: ServerResponse) { - this.send(res, 'hello world'); + async handle({response}: RequestContext) { + this.send(response, 'hello world'); } } // bind user defined sequence @@ -72,7 +72,7 @@ describe('Sequence', () => { }); it('allows users to bind a custom sequence class', () => { - class MySequence { + class MySequence implements SequenceHandler { constructor( @inject(SequenceActions.FIND_ROUTE) protected findRoute: FindRoute, @inject(SequenceActions.PARSE_PARAMS) @@ -81,11 +81,12 @@ describe('Sequence', () => { @inject(SequenceActions.SEND) protected send: Send, ) {} - async handle(req: ParsedRequest, res: ServerResponse) { - const route = this.findRoute(req); - const args = await this.parseParams(req, route); + async handle(context: RequestContext) { + const {request, response} = context; + const route = this.findRoute(request); + const args = await this.parseParams(request, route); const result = await this.invoke(route, args); - this.send(res, `MySequence ${result}`); + this.send(response, `MySequence ${result}`); } } @@ -97,11 +98,11 @@ describe('Sequence', () => { }); it('allows users to bind a custom sequence class via app.sequence()', async () => { - class MySequence { + class MySequence implements SequenceHandler { constructor(@inject(SequenceActions.SEND) protected send: Send) {} - async handle(req: ParsedRequest, res: ServerResponse) { - this.send(res, 'MySequence was invoked.'); + async handle({request, response}: RequestContext) { + this.send(response, 'MySequence was invoked.'); } } @@ -126,7 +127,7 @@ describe('Sequence', () => { }); it('user-defined Reject', () => { - const reject: Reject = (response, request, error) => { + const reject: Reject = ({response}, error) => { response.statusCode = 418; // I'm a teapot response.end(); }; @@ -139,9 +140,8 @@ describe('Sequence', () => { it('makes ctx available in a custom sequence handler function', () => { app.bind('test').to('hello world'); - server.handler((sequence, request, response) => { - expect.exists(sequence.ctx); - sequence.send(response, sequence.ctx.getSync('test')); + server.handler((context, sequence) => { + sequence.send(context.response, context.getSync('test')); }); return whenIRequest() @@ -152,7 +152,6 @@ describe('Sequence', () => { it('makes ctx available in a custom sequence class', () => { class MySequence extends DefaultSequence { constructor( - @inject(RestBindings.Http.CONTEXT) public ctx: Context, @inject(SequenceActions.FIND_ROUTE) protected findRoute: FindRoute, @inject(SequenceActions.PARSE_PARAMS) protected parseParams: ParseParams, @@ -160,11 +159,11 @@ describe('Sequence', () => { @inject(SequenceActions.SEND) public send: Send, @inject(SequenceActions.REJECT) public reject: Reject, ) { - super(ctx, findRoute, parseParams, invoke, send, reject); + super(findRoute, parseParams, invoke, send, reject); } - async handle(req: ParsedRequest, res: ServerResponse) { - this.send(res, this.ctx.getSync('test')); + async handle(context: RequestContext) { + this.send(context.response, context.getSync('test')); } } diff --git a/packages/rest/test/integration/rest.server.integration.ts b/packages/rest/test/integration/rest.server.integration.ts index 4edda6e0f817..cfa4d569296f 100644 --- a/packages/rest/test/integration/rest.server.integration.ts +++ b/packages/rest/test/integration/rest.server.integration.ts @@ -18,7 +18,7 @@ describe('RestServer (integration)', () => { it('responds with 500 when Sequence fails with unhandled error', async () => { const server = await givenAServer({rest: {port: 0}}); - server.handler((sequence, request, response) => { + server.handler((context, sequence) => { return Promise.reject(new Error('unhandled test error')); }); @@ -39,7 +39,7 @@ describe('RestServer (integration)', () => { it('allows cors', async () => { const server = await givenAServer({rest: {port: 0}}); - server.handler((sequence, request, response) => { + server.handler(({response}, sequence) => { response.write('Hello'); response.end(); }); @@ -53,7 +53,7 @@ describe('RestServer (integration)', () => { it('allows cors preflight', async () => { const server = await givenAServer({rest: {port: 0}}); - server.handler((sequence, request, response) => { + server.handler(({response}, sequence) => { response.write('Hello'); response.end(); }); diff --git a/packages/rest/test/unit/router/reject.provider.unit.ts b/packages/rest/test/unit/router/reject.provider.unit.ts index 38d65abe2bf2..11b3322dd655 100644 --- a/packages/rest/test/unit/router/reject.provider.unit.ts +++ b/packages/rest/test/unit/router/reject.provider.unit.ts @@ -3,7 +3,12 @@ // This file is licensed under the MIT License. // License text available at https://opensource.org/licenses/MIT -import {RejectProvider, LogError} from '../../..'; +import { + RejectProvider, + LogError, + ParsedRequest, + HandlerContext, +} from '../../..'; import { expect, @@ -17,12 +22,14 @@ describe('reject', () => { const noopLogger: LogError = () => {}; const testError = new Error('test error'); let mock: ShotResponseMock; + let mockedContext: HandlerContext; beforeEach(givenMockedResponse); it('returns HTTP response with status code 500 by default', async () => { const reject = new RejectProvider(noopLogger).value(); - reject(mock.response, mock.request, testError); + + reject(mockedContext, testError); const result = await mock.result; expect(result).to.have.property('statusCode', 500); @@ -32,7 +39,7 @@ describe('reject', () => { const logger = sinon.spy() as LogError & SinonSpy; const reject = new RejectProvider(logger).value(); - reject(mock.response, mock.request, testError); + reject(mockedContext, testError); await mock.result; sinon.assert.calledWith(logger, testError, 500, mock.request); @@ -40,5 +47,10 @@ describe('reject', () => { function givenMockedResponse() { mock = mockResponse(); + mockedContext = { + // FIXME(bajtos) Remove this explicit cast once we switch to Express + request: mock.request as ParsedRequest, + response: mock.response, + }; } });