From 922bbc3e574ebb7694514baa097e54913dceaf99 Mon Sep 17 00:00:00 2001 From: Toon van Strijp Date: Mon, 1 Apr 2019 01:02:09 +0200 Subject: [PATCH 1/5] set http status code before running interceptors added unit test and integration test changes http adapter to not send http status in reply --- .../hello-world/e2e/interceptors.spec.ts | 24 +++++++++++++ .../hello-world/src/hello/hello.controller.ts | 2 +- .../interfaces/http/http-server.interface.ts | 3 +- packages/core/adapters/http-adapter.ts | 3 +- .../core/exceptions/base-exception-filter.ts | 6 ++-- .../core/router/router-execution-context.ts | 26 +++++++------- .../core/router/router-response-controller.ts | 12 +++++-- .../exceptions/exceptions-handler.spec.ts | 12 ++++--- .../router/router-response-controller.spec.ts | 36 +++++++++++++------ packages/core/test/utils/noop-adapter.spec.ts | 3 +- .../adapters/express-adapter.ts | 11 +++--- .../adapters/fastify-adapter.ts | 8 +++-- 12 files changed, 104 insertions(+), 42 deletions(-) diff --git a/integration/hello-world/e2e/interceptors.spec.ts b/integration/hello-world/e2e/interceptors.spec.ts index e5957e3e530..fffbaff1c64 100644 --- a/integration/hello-world/e2e/interceptors.spec.ts +++ b/integration/hello-world/e2e/interceptors.spec.ts @@ -28,6 +28,19 @@ export class TransformInterceptor { } } +@Injectable() +export class StatusInterceptor { + + constructor(private statusCode: number){} + + intercept(context: ExecutionContext, next: CallHandler) { + const ctx = context.switchToHttp(); + const res = ctx.getResponse(); + res.status(this.statusCode); + return next.handle().pipe(map(data => ({ data }))); + } +} + function createTestModule(interceptor) { return Test.createTestingModule({ imports: [ApplicationModule], @@ -87,6 +100,17 @@ describe('Interceptors', () => { .expect(200, { data: 'Hello world!' }); }); + it(`should modify response status`, async () => { + app = (await createTestModule( + new StatusInterceptor(400), + )).createNestApplication(); + + await app.init(); + return request(app.getHttpServer()) + .get('/hello') + .expect(400, { data: 'Hello world!' }); + }); + afterEach(async () => { await app.close(); }); diff --git a/integration/hello-world/src/hello/hello.controller.ts b/integration/hello-world/src/hello/hello.controller.ts index 5b723f4a092..c494aaf3a31 100644 --- a/integration/hello-world/src/hello/hello.controller.ts +++ b/integration/hello-world/src/hello/hello.controller.ts @@ -1,5 +1,5 @@ import { HelloService } from './hello.service'; -import { Controller, Get, Header, Param } from '@nestjs/common'; +import { Controller, Get, Header, Param, Post } from '@nestjs/common'; import { Observable, of } from 'rxjs'; import { UserByIdPipe } from './users/user-by-id.pipe'; diff --git a/packages/common/interfaces/http/http-server.interface.ts b/packages/common/interfaces/http/http-server.interface.ts index cfa5e99fc49..5f66b67d62e 100644 --- a/packages/common/interfaces/http/http-server.interface.ts +++ b/packages/common/interfaces/http/http-server.interface.ts @@ -42,7 +42,8 @@ export interface HttpServer { options(path: string, handler: RequestHandler): any; listen(port: number | string, callback?: () => void): any; listen(port: number | string, hostname: string, callback?: () => void): any; - reply(response: any, body: any, statusCode: number): any; + reply(response: any, body: any): any; + status(response: any, statusCode: number): any; render(response: any, view: string, options: any): any; setHeader(response: any, name: string, value: string): any; setErrorHandler?(handler: Function): any; diff --git a/packages/core/adapters/http-adapter.ts b/packages/core/adapters/http-adapter.ts index fcb918bd7d9..dca16665a5d 100644 --- a/packages/core/adapters/http-adapter.ts +++ b/packages/core/adapters/http-adapter.ts @@ -82,7 +82,8 @@ export abstract class AbstractHttpAdapter< abstract setViewEngine(engine: string); abstract getRequestMethod(request); abstract getRequestUrl(request); - abstract reply(response, body: any, statusCode: number); + abstract status(response, statusCode: number); + abstract reply(response, body: any); abstract render(response, view: string, options: any); abstract setErrorHandler(handler: Function); abstract setNotFoundHandler(handler: Function); diff --git a/packages/core/exceptions/base-exception-filter.ts b/packages/core/exceptions/base-exception-filter.ts index 6701318f5b0..d71ff479b8b 100644 --- a/packages/core/exceptions/base-exception-filter.ts +++ b/packages/core/exceptions/base-exception-filter.ts @@ -31,7 +31,8 @@ export class BaseExceptionFilter implements ExceptionFilter { statusCode: HttpStatus.INTERNAL_SERVER_ERROR, message: MESSAGES.UNKNOWN_EXCEPTION_MESSAGE, }; - applicationRef.reply(host.getArgByIndex(1), body, body.statusCode); + applicationRef.status(host.getArgByIndex(1), body.statusCode); + applicationRef.reply(host.getArgByIndex(1), body); if (this.isExceptionObject(exception)) { return BaseExceptionFilter.logger.error( exception.message, @@ -48,7 +49,8 @@ export class BaseExceptionFilter implements ExceptionFilter { message: res, }; - applicationRef.reply(host.getArgByIndex(1), message, exception.getStatus()); + applicationRef.status(host.getArgByIndex(1), exception.getStatus()); + applicationRef.reply(host.getArgByIndex(1), message); } public isExceptionObject(err: any): err is Error { diff --git a/packages/core/router/router-execution-context.ts b/packages/core/router/router-execution-context.ts index 8bb009b348a..8f77445a43a 100644 --- a/packages/core/router/router-execution-context.ts +++ b/packages/core/router/router-execution-context.ts @@ -83,7 +83,7 @@ export class RouterExecutionContext { fnHandleResponse, paramtypes, getParamsMetadata, - } = this.getMetadata(instance, callback, methodName, module, requestMethod); + } = this.getMetadata(instance, callback, methodName, module); const paramsOptions = this.contextUtils.mergeParamsMetatypes( getParamsMetadata(module, contextId, inquirerId), paramtypes, @@ -131,6 +131,14 @@ export class RouterExecutionContext { const args = this.contextUtils.createNullArray(argsLength); fnCanActivate && (await fnCanActivate([req, res])); + const httpCode = this.reflectHttpStatusCode(callback); + + const httpStatusCode = httpCode + ? httpCode + : this.responseController.getStatusByMethod(requestMethod); + + this.responseController.status(res, httpStatusCode); + const result = await this.interceptorsConsumer.intercept( interceptors, [req, res], @@ -146,8 +154,7 @@ export class RouterExecutionContext { instance: Controller, callback: (...args: any[]) => any, methodName: string, - module: string, - requestMethod: RequestMethod, + module: string ): HandlerMetadata { const cacheMetadata = this.handlerMetadataStorage.get(instance, methodName); if (cacheMetadata) { @@ -165,7 +172,6 @@ export class RouterExecutionContext { instance, methodName, ); - const httpCode = this.reflectHttpStatusCode(callback); const getParamsMetadata = ( moduleKey: string, contextId = STATIC_CONTEXT, @@ -184,14 +190,10 @@ export class RouterExecutionContext { ({ type }) => type === RouteParamtypes.RESPONSE || type === RouteParamtypes.NEXT, ); - const httpStatusCode = httpCode - ? httpCode - : this.responseController.getStatusByMethod(requestMethod); const fnHandleResponse = this.createHandleResponseFn( callback, - isResponseHandled, - httpStatusCode, + isResponseHandled ); const handlerMetadata: HandlerMetadata = { argsLength, @@ -341,8 +343,7 @@ export class RouterExecutionContext { public createHandleResponseFn( callback: (...args: any[]) => any, - isResponseHandled: boolean, - httpStatusCode: number, + isResponseHandled: boolean ) { const renderTemplate = this.reflectRenderTemplate(callback); const responseHeaders = this.reflectResponseHeaders(callback); @@ -358,10 +359,9 @@ export class RouterExecutionContext { return async (result: TResult, res: TResponse) => { hasCustomHeaders && this.responseController.setHeaders(res, responseHeaders); - result = await this.responseController.transformToResult(result); !isResponseHandled && - (await this.responseController.apply(result, res, httpStatusCode)); + (await this.responseController.apply(result, res)); }; } } diff --git a/packages/core/router/router-response-controller.ts b/packages/core/router/router-response-controller.ts index cb14605ba2f..701170e7cd8 100644 --- a/packages/core/router/router-response-controller.ts +++ b/packages/core/router/router-response-controller.ts @@ -11,10 +11,9 @@ export class RouterResponseController { public async apply( result: TInput, - response: TResponse, - httpStatusCode: number, + response: TResponse ) { - return this.applicationRef.reply(response, result, httpStatusCode); + return this.applicationRef.reply(response, result); } public async render( @@ -50,4 +49,11 @@ export class RouterResponseController { this.applicationRef.setHeader(response, name, value), ); } + + public status( + response: TResponse, + statusCode: number + ) { + this.applicationRef.status(response, statusCode); + } } diff --git a/packages/core/test/exceptions/exceptions-handler.spec.ts b/packages/core/test/exceptions/exceptions-handler.spec.ts index bd44a3364c9..7d8d461ef7a 100644 --- a/packages/core/test/exceptions/exceptions-handler.spec.ts +++ b/packages/core/test/exceptions/exceptions-handler.spec.ts @@ -31,14 +31,18 @@ describe('ExceptionsHandler', () => { describe('next', () => { beforeEach(() => { + sinon + .stub(adapter, 'status') + .callsFake((responseRef: any, statusCode: number) => { + return responseRef.status(statusCode); + }); sinon .stub(adapter, 'reply') - .callsFake((responseRef: any, body: any, statusCode: number) => { - const res = responseRef.status(statusCode); + .callsFake((responseRef: any, body: any) => { if (isNil(body)) { - return res.send(); + return responseRef.send(); } - return isObject(body) ? res.json(body) : res.send(String(body)); + return isObject(body) ? responseRef.json(body) : responseRef.send(String(body)); }); }); it('should method send expected response status code and message when exception is unknown', () => { diff --git a/packages/core/test/router/router-response-controller.spec.ts b/packages/core/test/router/router-response-controller.spec.ts index 5aff6910359..98ca6f0b7cd 100644 --- a/packages/core/test/router/router-response-controller.spec.ts +++ b/packages/core/test/router/router-response-controller.spec.ts @@ -18,36 +18,34 @@ describe('RouterResponseController', () => { describe('apply', () => { let response: { send: sinon.SinonSpy; - status?: sinon.SinonSpy; + status: sinon.SinonSpy; json: sinon.SinonSpy; }; beforeEach(() => { - response = { send: sinon.spy(), json: sinon.spy() }; - response.status = sinon.stub().returns(response); + response = { send: sinon.spy(), json: sinon.spy(), status: sinon.spy() }; }); describe('when result is', () => { beforeEach(() => { sinon .stub(adapter, 'reply') - .callsFake((responseRef: any, body: any, statusCode: number) => { - const res = responseRef.status(statusCode); + .callsFake((responseRef: any, body: any) => { if (isNil(body)) { - return res.send(); + return responseRef.send(); } - return isObject(body) ? res.json(body) : res.send(String(body)); + return isObject(body) ? responseRef.json(body) : responseRef.send(String(body)); }); }); describe('nil', () => { it('should call send()', async () => { const value = null; - await routerResponseController.apply(value, response, 200); + await routerResponseController.apply(value, response); expect(response.send.called).to.be.true; }); }); describe('string', () => { it('should call send(value)', async () => { const value = 'string'; - await routerResponseController.apply(value, response, 200); + await routerResponseController.apply(value, response); expect(response.send.called).to.be.true; expect(response.send.calledWith(String(value))).to.be.true; }); @@ -55,7 +53,7 @@ describe('RouterResponseController', () => { describe('object', () => { it('should call json(value)', async () => { const value = { test: 'test' }; - await routerResponseController.apply(value, response, 200); + await routerResponseController.apply(value, response); expect(response.json.called).to.be.true; expect(response.json.calledWith(value)).to.be.true; }); @@ -149,4 +147,22 @@ describe('RouterResponseController', () => { ).to.be.true; }); }); + + describe('status', () => { + let statusStub: sinon.SinonStub; + + beforeEach(() => { + statusStub = sinon.stub(adapter, 'status').callsFake(() => ({})); + }); + + it('should set status', () => { + const response = {}; + const statusCode = 400; + + routerResponseController.setStatus(response, statusCode); + expect( + statusStub.calledWith(response, statusCode), + ).to.be.true; + }); + }); }); diff --git a/packages/core/test/utils/noop-adapter.spec.ts b/packages/core/test/utils/noop-adapter.spec.ts index 81283e79f8d..560d70ac952 100644 --- a/packages/core/test/utils/noop-adapter.spec.ts +++ b/packages/core/test/utils/noop-adapter.spec.ts @@ -11,7 +11,8 @@ export class NoopHttpAdapter extends AbstractHttpAdapter { setViewEngine(engine: string): any {} getRequestMethod(request: any): any {} getRequestUrl(request: any): any {} - reply(response: any, body: any, statusCode: number): any {} + reply(response: any, body: any): any {} + status(response: any, statusCode: number): any {} render(response: any, view: string, options: any): any {} setErrorHandler(handler: Function): any {} setNotFoundHandler(handler: Function): any {} diff --git a/packages/platform-express/adapters/express-adapter.ts b/packages/platform-express/adapters/express-adapter.ts index 94dd1af8286..895b76d7130 100644 --- a/packages/platform-express/adapters/express-adapter.ts +++ b/packages/platform-express/adapters/express-adapter.ts @@ -18,12 +18,15 @@ export class ExpressAdapter extends AbstractHttpAdapter { super(instance || express()); } - public reply(response, body: any, statusCode: number) { - const res = response.status(statusCode); + public reply(response, body: any) { if (isNil(body)) { - return res.send(); + return response.send(); } - return isObject(body) ? res.json(body) : res.send(String(body)); + return isObject(body) ? response.json(body) : response.send(String(body)); + } + + public status(response: any, statusCode: number) { + return response.status(statusCode); } public render(response: any, view: string, options: any) { diff --git a/packages/platform-fastify/adapters/fastify-adapter.ts b/packages/platform-fastify/adapters/fastify-adapter.ts index fbd223a2450..f8e1e3b4530 100644 --- a/packages/platform-fastify/adapters/fastify-adapter.ts +++ b/packages/platform-fastify/adapters/fastify-adapter.ts @@ -29,8 +29,12 @@ export class FastifyAdapter extends AbstractHttpAdapter { return this.instance.listen(port, ...args); } - public reply(response: any, body: any, statusCode: number) { - return response.code(statusCode).send(body); + public reply(response: any, body: any) { + return response.send(body); + } + + public status(response: any, statusCode: number) { + return response.code(statusCode); } public render(response: any, view: string, options: any) { From 6415633097efc6fa15c22f074eab7f9209206202 Mon Sep 17 00:00:00 2001 From: Toon van Strijp Date: Mon, 1 Apr 2019 09:24:09 +0200 Subject: [PATCH 2/5] fixed unit tests --- packages/core/test/router/router-execution-context.spec.ts | 4 ++-- packages/core/test/router/router-response-controller.spec.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/test/router/router-execution-context.spec.ts b/packages/core/test/router/router-execution-context.spec.ts index a4d21be99e4..e618815b97b 100644 --- a/packages/core/test/router/router-execution-context.spec.ts +++ b/packages/core/test/router/router-execution-context.spec.ts @@ -281,7 +281,7 @@ describe('RouterExecutionContext', () => { sinon.stub(contextCreator, 'reflectResponseHeaders').returns([]); sinon.stub(contextCreator, 'reflectRenderTemplate').returns(template); - const handler = contextCreator.createHandleResponseFn(null, true, 100); + const handler = contextCreator.createHandleResponseFn(null, true); await handler(value, response); expect(response.render.calledWith(template, value)).to.be.true; @@ -295,7 +295,7 @@ describe('RouterExecutionContext', () => { sinon.stub(contextCreator, 'reflectResponseHeaders').returns([]); sinon.stub(contextCreator, 'reflectRenderTemplate').returns(undefined); - const handler = contextCreator.createHandleResponseFn(null, true, 100); + const handler = contextCreator.createHandleResponseFn(null, true); handler(result, response); expect(response.render.called).to.be.false; diff --git a/packages/core/test/router/router-response-controller.spec.ts b/packages/core/test/router/router-response-controller.spec.ts index 98ca6f0b7cd..900d3bfabb8 100644 --- a/packages/core/test/router/router-response-controller.spec.ts +++ b/packages/core/test/router/router-response-controller.spec.ts @@ -159,7 +159,7 @@ describe('RouterResponseController', () => { const response = {}; const statusCode = 400; - routerResponseController.setStatus(response, statusCode); + routerResponseController.status(response, statusCode); expect( statusStub.calledWith(response, statusCode), ).to.be.true; From 152140085b2713299cb0383be7d660a21541e7be Mon Sep 17 00:00:00 2001 From: Toon van Strijp Date: Mon, 1 Apr 2019 11:47:37 +0200 Subject: [PATCH 3/5] reverted reply signature to have statusCode --- .../common/interfaces/http/http-server.interface.ts | 2 +- packages/core/adapters/http-adapter.ts | 2 +- packages/core/exceptions/base-exception-filter.ts | 6 ++---- packages/core/router/router-execution-context.ts | 5 +++-- packages/core/router/router-response-controller.ts | 5 +++-- .../core/test/exceptions/exceptions-handler.spec.ts | 10 ++++------ .../test/router/router-execution-context.spec.ts | 4 ++-- .../test/router/router-response-controller.spec.ts | 13 ++++++++----- .../platform-express/adapters/express-adapter.ts | 5 ++++- .../platform-fastify/adapters/fastify-adapter.ts | 5 ++++- 10 files changed, 32 insertions(+), 25 deletions(-) diff --git a/packages/common/interfaces/http/http-server.interface.ts b/packages/common/interfaces/http/http-server.interface.ts index 5f66b67d62e..348b0c01e54 100644 --- a/packages/common/interfaces/http/http-server.interface.ts +++ b/packages/common/interfaces/http/http-server.interface.ts @@ -42,7 +42,7 @@ export interface HttpServer { options(path: string, handler: RequestHandler): any; listen(port: number | string, callback?: () => void): any; listen(port: number | string, hostname: string, callback?: () => void): any; - reply(response: any, body: any): any; + reply(response: any, body: any, statusCode?: number): any; status(response: any, statusCode: number): any; render(response: any, view: string, options: any): any; setHeader(response: any, name: string, value: string): any; diff --git a/packages/core/adapters/http-adapter.ts b/packages/core/adapters/http-adapter.ts index dca16665a5d..163f784a8db 100644 --- a/packages/core/adapters/http-adapter.ts +++ b/packages/core/adapters/http-adapter.ts @@ -83,7 +83,7 @@ export abstract class AbstractHttpAdapter< abstract getRequestMethod(request); abstract getRequestUrl(request); abstract status(response, statusCode: number); - abstract reply(response, body: any); + abstract reply(response, body: any, statusCode?: number); abstract render(response, view: string, options: any); abstract setErrorHandler(handler: Function); abstract setNotFoundHandler(handler: Function); diff --git a/packages/core/exceptions/base-exception-filter.ts b/packages/core/exceptions/base-exception-filter.ts index d71ff479b8b..6701318f5b0 100644 --- a/packages/core/exceptions/base-exception-filter.ts +++ b/packages/core/exceptions/base-exception-filter.ts @@ -31,8 +31,7 @@ export class BaseExceptionFilter implements ExceptionFilter { statusCode: HttpStatus.INTERNAL_SERVER_ERROR, message: MESSAGES.UNKNOWN_EXCEPTION_MESSAGE, }; - applicationRef.status(host.getArgByIndex(1), body.statusCode); - applicationRef.reply(host.getArgByIndex(1), body); + applicationRef.reply(host.getArgByIndex(1), body, body.statusCode); if (this.isExceptionObject(exception)) { return BaseExceptionFilter.logger.error( exception.message, @@ -49,8 +48,7 @@ export class BaseExceptionFilter implements ExceptionFilter { message: res, }; - applicationRef.status(host.getArgByIndex(1), exception.getStatus()); - applicationRef.reply(host.getArgByIndex(1), message); + applicationRef.reply(host.getArgByIndex(1), message, exception.getStatus()); } public isExceptionObject(err: any): err is Error { diff --git a/packages/core/router/router-execution-context.ts b/packages/core/router/router-execution-context.ts index 8f77445a43a..8c2944291f5 100644 --- a/packages/core/router/router-execution-context.ts +++ b/packages/core/router/router-execution-context.ts @@ -343,7 +343,8 @@ export class RouterExecutionContext { public createHandleResponseFn( callback: (...args: any[]) => any, - isResponseHandled: boolean + isResponseHandled: boolean, + httpStatusCode?: number ) { const renderTemplate = this.reflectRenderTemplate(callback); const responseHeaders = this.reflectResponseHeaders(callback); @@ -361,7 +362,7 @@ export class RouterExecutionContext { this.responseController.setHeaders(res, responseHeaders); result = await this.responseController.transformToResult(result); !isResponseHandled && - (await this.responseController.apply(result, res)); + (await this.responseController.apply(result, res, httpStatusCode)); }; } } diff --git a/packages/core/router/router-response-controller.ts b/packages/core/router/router-response-controller.ts index 701170e7cd8..57f810c47b7 100644 --- a/packages/core/router/router-response-controller.ts +++ b/packages/core/router/router-response-controller.ts @@ -11,9 +11,10 @@ export class RouterResponseController { public async apply( result: TInput, - response: TResponse + response: TResponse, + httpStatusCode?: number, ) { - return this.applicationRef.reply(response, result); + return this.applicationRef.reply(response, result, httpStatusCode); } public async render( diff --git a/packages/core/test/exceptions/exceptions-handler.spec.ts b/packages/core/test/exceptions/exceptions-handler.spec.ts index 7d8d461ef7a..9c5c49289d5 100644 --- a/packages/core/test/exceptions/exceptions-handler.spec.ts +++ b/packages/core/test/exceptions/exceptions-handler.spec.ts @@ -31,14 +31,12 @@ describe('ExceptionsHandler', () => { describe('next', () => { beforeEach(() => { - sinon - .stub(adapter, 'status') - .callsFake((responseRef: any, statusCode: number) => { - return responseRef.status(statusCode); - }); sinon .stub(adapter, 'reply') - .callsFake((responseRef: any, body: any) => { + .callsFake((responseRef: any, body: any, statusCode?: number) => { + if (statusCode) { + responseRef.status(statusCode); + } if (isNil(body)) { return responseRef.send(); } diff --git a/packages/core/test/router/router-execution-context.spec.ts b/packages/core/test/router/router-execution-context.spec.ts index e618815b97b..ebe51f2e798 100644 --- a/packages/core/test/router/router-execution-context.spec.ts +++ b/packages/core/test/router/router-execution-context.spec.ts @@ -281,7 +281,7 @@ describe('RouterExecutionContext', () => { sinon.stub(contextCreator, 'reflectResponseHeaders').returns([]); sinon.stub(contextCreator, 'reflectRenderTemplate').returns(template); - const handler = contextCreator.createHandleResponseFn(null, true); + const handler = contextCreator.createHandleResponseFn(null, true, 200); await handler(value, response); expect(response.render.calledWith(template, value)).to.be.true; @@ -295,7 +295,7 @@ describe('RouterExecutionContext', () => { sinon.stub(contextCreator, 'reflectResponseHeaders').returns([]); sinon.stub(contextCreator, 'reflectRenderTemplate').returns(undefined); - const handler = contextCreator.createHandleResponseFn(null, true); + const handler = contextCreator.createHandleResponseFn(null, true, 200); handler(result, response); expect(response.render.called).to.be.false; diff --git a/packages/core/test/router/router-response-controller.spec.ts b/packages/core/test/router/router-response-controller.spec.ts index 900d3bfabb8..11c6431e979 100644 --- a/packages/core/test/router/router-response-controller.spec.ts +++ b/packages/core/test/router/router-response-controller.spec.ts @@ -18,7 +18,7 @@ describe('RouterResponseController', () => { describe('apply', () => { let response: { send: sinon.SinonSpy; - status: sinon.SinonSpy; + status?: sinon.SinonSpy; json: sinon.SinonSpy; }; beforeEach(() => { @@ -28,7 +28,10 @@ describe('RouterResponseController', () => { beforeEach(() => { sinon .stub(adapter, 'reply') - .callsFake((responseRef: any, body: any) => { + .callsFake((responseRef: any, body: any, statusCode?: number) => { + if (statusCode) { + responseRef.status(statusCode); + } if (isNil(body)) { return responseRef.send(); } @@ -38,14 +41,14 @@ describe('RouterResponseController', () => { describe('nil', () => { it('should call send()', async () => { const value = null; - await routerResponseController.apply(value, response); + await routerResponseController.apply(value, response, 200); expect(response.send.called).to.be.true; }); }); describe('string', () => { it('should call send(value)', async () => { const value = 'string'; - await routerResponseController.apply(value, response); + await routerResponseController.apply(value, response, 200); expect(response.send.called).to.be.true; expect(response.send.calledWith(String(value))).to.be.true; }); @@ -53,7 +56,7 @@ describe('RouterResponseController', () => { describe('object', () => { it('should call json(value)', async () => { const value = { test: 'test' }; - await routerResponseController.apply(value, response); + await routerResponseController.apply(value, response, 200); expect(response.json.called).to.be.true; expect(response.json.calledWith(value)).to.be.true; }); diff --git a/packages/platform-express/adapters/express-adapter.ts b/packages/platform-express/adapters/express-adapter.ts index 895b76d7130..e82fdb40f26 100644 --- a/packages/platform-express/adapters/express-adapter.ts +++ b/packages/platform-express/adapters/express-adapter.ts @@ -18,7 +18,10 @@ export class ExpressAdapter extends AbstractHttpAdapter { super(instance || express()); } - public reply(response, body: any) { + public reply(response, body: any, statusCode?: number) { + if (statusCode) { + response.status(statusCode); + } if (isNil(body)) { return response.send(); } diff --git a/packages/platform-fastify/adapters/fastify-adapter.ts b/packages/platform-fastify/adapters/fastify-adapter.ts index f8e1e3b4530..04da6b60530 100644 --- a/packages/platform-fastify/adapters/fastify-adapter.ts +++ b/packages/platform-fastify/adapters/fastify-adapter.ts @@ -29,7 +29,10 @@ export class FastifyAdapter extends AbstractHttpAdapter { return this.instance.listen(port, ...args); } - public reply(response: any, body: any) { + public reply(response: any, body: any, statusCode?: number) { + if (statusCode) { + response.status(statusCode); + } return response.send(body); } From 6618badc23bce03b92656f358822b881d4a44cb4 Mon Sep 17 00:00:00 2001 From: Toon van Strijp Date: Fri, 5 Apr 2019 11:40:16 +0200 Subject: [PATCH 4/5] removed set header code before running interceptors. Added e2e test for testing header override. --- .../hello-world/e2e/interceptors.spec.ts | 25 +++++++++++++++++++ .../core/router/router-execution-context.ts | 12 ++++----- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/integration/hello-world/e2e/interceptors.spec.ts b/integration/hello-world/e2e/interceptors.spec.ts index fffbaff1c64..4dcdb9ab054 100644 --- a/integration/hello-world/e2e/interceptors.spec.ts +++ b/integration/hello-world/e2e/interceptors.spec.ts @@ -41,6 +41,19 @@ export class StatusInterceptor { } } +@Injectable() +export class HeaderInterceptor { + + constructor(private authorization: string){} + + intercept(context: ExecutionContext, next: CallHandler) { + const ctx = context.switchToHttp(); + const res = ctx.getResponse(); + res.header('Authorization', this.authorization); + return next.handle().pipe(map(data => ({ data }))); + } +} + function createTestModule(interceptor) { return Test.createTestingModule({ imports: [ApplicationModule], @@ -111,6 +124,18 @@ describe('Interceptors', () => { .expect(400, { data: 'Hello world!' }); }); + it(`should modify Authorization header`, async () => { + app = (await createTestModule( + new HeaderInterceptor('jwt'), + )).createNestApplication(); + + await app.init(); + return request(app.getHttpServer()) + .get('/hello') + .expect(200) + .expect('Authorization', 'jwt'); + }); + afterEach(async () => { await app.close(); }); diff --git a/packages/core/router/router-execution-context.ts b/packages/core/router/router-execution-context.ts index 8c2944291f5..d63c82c488f 100644 --- a/packages/core/router/router-execution-context.ts +++ b/packages/core/router/router-execution-context.ts @@ -139,6 +139,12 @@ export class RouterExecutionContext { this.responseController.status(res, httpStatusCode); + const responseHeaders = this.reflectResponseHeaders(callback); + const hasCustomHeaders = !isEmpty(responseHeaders); + + hasCustomHeaders && + this.responseController.setHeaders(res, responseHeaders); + const result = await this.interceptorsConsumer.intercept( interceptors, [req, res], @@ -347,19 +353,13 @@ export class RouterExecutionContext { httpStatusCode?: number ) { const renderTemplate = this.reflectRenderTemplate(callback); - const responseHeaders = this.reflectResponseHeaders(callback); - const hasCustomHeaders = !isEmpty(responseHeaders); if (renderTemplate) { return async (result: TResult, res: TResponse) => { - hasCustomHeaders && - this.responseController.setHeaders(res, responseHeaders); await this.responseController.render(result, res, renderTemplate); }; } return async (result: TResult, res: TResponse) => { - hasCustomHeaders && - this.responseController.setHeaders(res, responseHeaders); result = await this.responseController.transformToResult(result); !isResponseHandled && (await this.responseController.apply(result, res, httpStatusCode)); From 21c22d5af3819930321f381663a5d8bfde968635 Mon Sep 17 00:00:00 2001 From: Toon van Strijp Date: Sat, 6 Apr 2019 15:49:42 +0200 Subject: [PATCH 5/5] change header interceptor in hello-world e2e to take multiple headers if needed. --- integration/hello-world/e2e/interceptors.spec.ts | 14 +++++++++++--- scripts/test.sh | 0 2 files changed, 11 insertions(+), 3 deletions(-) mode change 100644 => 100755 scripts/test.sh diff --git a/integration/hello-world/e2e/interceptors.spec.ts b/integration/hello-world/e2e/interceptors.spec.ts index 4dcdb9ab054..dbfb7ed9a7a 100644 --- a/integration/hello-world/e2e/interceptors.spec.ts +++ b/integration/hello-world/e2e/interceptors.spec.ts @@ -44,12 +44,16 @@ export class StatusInterceptor { @Injectable() export class HeaderInterceptor { - constructor(private authorization: string){} + constructor(private headers: object){} intercept(context: ExecutionContext, next: CallHandler) { const ctx = context.switchToHttp(); const res = ctx.getResponse(); - res.header('Authorization', this.authorization); + for (const key in this.headers) { + if (this.headers.hasOwnProperty(key)) { + res.header(key, this.headers[key]); + } + } return next.handle().pipe(map(data => ({ data }))); } } @@ -125,8 +129,12 @@ describe('Interceptors', () => { }); it(`should modify Authorization header`, async () => { + const customHeaders = { + Authorization: 'jwt', + }; + app = (await createTestModule( - new HeaderInterceptor('jwt'), + new HeaderInterceptor(customHeaders), )).createNestApplication(); await app.init(); diff --git a/scripts/test.sh b/scripts/test.sh old mode 100644 new mode 100755