Merge pull request nestjs#1328 from nestjs/bugfix/interceptor-flow

bugfix(core) fix interceptors execution flow

Author: kamilmysliwiec

packages/common/cache/interceptors/cache.interceptor.ts

@@ -2,6 +2,7 @@ import { Observable, of } from 'rxjs';
 import { tap } from 'rxjs/operators';
 import { Inject, Injectable, Optional } from '../../decorators';
 import {
+  CallHandler,
   ExecutionContext,
   HttpServer,
   NestInterceptor,
@@ -28,22 +29,22 @@ export class CacheInterceptor implements NestInterceptor {
 
   async intercept(
     context: ExecutionContext,
-    call$: Observable<any>,
+    next: CallHandler,
   ): Promise<Observable<any>> {
     const key = this.trackBy(context);
     if (!key) {
-      return call$;
+      return next.handle();
     }
     try {
       const value = await this.cacheManager.get(key);
       if (value) {
         return of(value);
       }
-      return call$.pipe(
-        tap((response: any) => this.cacheManager.set(key, response)),
-      );
+      return next
+        .handle()
+        .pipe(tap(response => this.cacheManager.set(key, response)));
     } catch {
-      return call$;
+      return next.handle();
     }
   }
 

packages/common/files/interceptors/file-fields.interceptor.ts

@@ -7,7 +7,10 @@ import {
   MulterField,
   MulterOptions,
 } from '../../interfaces/external/multer-options.interface';
-import { NestInterceptor } from '../../interfaces/features/nest-interceptor.interface';
+import {
+  CallHandler,
+  NestInterceptor,
+} from '../../interfaces/features/nest-interceptor.interface';
 import { MULTER_MODULE_OPTIONS } from '../files.constants';
 import { MulterModuleOptions } from '../interfaces';
 import { transformException } from '../multer/multer.utils';
@@ -34,7 +37,7 @@ export function FileFieldsInterceptor(
 
     async intercept(
       context: ExecutionContext,
-      call$: Observable<any>,
+      next: CallHandler,
     ): Promise<Observable<any>> {
       const ctx = context.switchToHttp();
 
@@ -51,7 +54,7 @@ export function FileFieldsInterceptor(
           },
         ),
       );
-      return call$;
+      return next.handle();
     }
   }
   const Interceptor = mixin(MixinInterceptor);

packages/common/files/interceptors/file.interceptor.ts

@@ -4,7 +4,10 @@ import { Inject, Optional } from '../../decorators';
 import { mixin } from '../../decorators/core/injectable.decorator';
 import { ExecutionContext, Type } from '../../interfaces';
 import { MulterOptions } from '../../interfaces/external/multer-options.interface';
-import { NestInterceptor } from '../../interfaces/features/nest-interceptor.interface';
+import {
+  CallHandler,
+  NestInterceptor,
+} from '../../interfaces/features/nest-interceptor.interface';
 import { MULTER_MODULE_OPTIONS } from '../files.constants';
 import { MulterModuleOptions } from '../interfaces';
 import { transformException } from '../multer/multer.utils';
@@ -31,7 +34,7 @@ export function FileInterceptor(
 
     async intercept(
       context: ExecutionContext,
-      call$: Observable<any>,
+      next: CallHandler,
     ): Promise<Observable<any>> {
       const ctx = context.switchToHttp();
 
@@ -48,7 +51,7 @@ export function FileInterceptor(
           },
         ),
       );
-      return call$;
+      return next.handle();
     }
   }
   const Interceptor = mixin(MixinInterceptor);

packages/common/files/interceptors/files.interceptor.ts

@@ -4,7 +4,10 @@ import { Inject, Optional } from '../../decorators';
 import { mixin } from '../../decorators/core/injectable.decorator';
 import { ExecutionContext, Type } from '../../interfaces';
 import { MulterOptions } from '../../interfaces/external/multer-options.interface';
-import { NestInterceptor } from '../../interfaces/features/nest-interceptor.interface';
+import {
+  CallHandler,
+  NestInterceptor,
+} from '../../interfaces/features/nest-interceptor.interface';
 import { MULTER_MODULE_OPTIONS } from '../files.constants';
 import { MulterModuleOptions } from '../interfaces';
 import { transformException } from '../multer/multer.utils';
@@ -32,7 +35,7 @@ export function FilesInterceptor(
 
     async intercept(
       context: ExecutionContext,
-      call$: Observable<any>,
+      next: CallHandler,
     ): Promise<Observable<any>> {
       const ctx = context.switchToHttp();
 
@@ -49,7 +52,7 @@ export function FilesInterceptor(
           },
         ),
       );
-      return call$;
+      return next.handle();
     }
   }
   const Interceptor = mixin(MixinInterceptor);

packages/common/index.ts

@@ -15,6 +15,7 @@ export * from './http';
 export {
   ArgumentMetadata,
   ArgumentsHost,
+  CallHandler,
   CanActivate,
   DynamicModule,
   ExceptionFilter,

packages/common/interfaces/features/nest-interceptor.interface.ts

@@ -1,9 +1,13 @@
 import { Observable } from 'rxjs';
 import { ExecutionContext } from './execution-context.interface';
 
+export interface CallHandler<T = any> {
+  handle(): Observable<T>;
+}
+
 export interface NestInterceptor<T = any, R = any> {
   intercept(
     context: ExecutionContext,
-    call$: Observable<T>,
+    next: CallHandler<T>,
   ): Observable<R> | Promise<Observable<R>>;
 }

packages/common/serializer/class-serializer.interceptor.ts

@@ -4,7 +4,11 @@ import { Inject, Injectable } from '../decorators/core';
 import { ClassTransformOptions } from '../interfaces/external/class-transform-options.interface';
 import { loadPackage } from '../utils/load-package.util';
 import { isObject } from '../utils/shared.utils';
-import { ExecutionContext, NestInterceptor } from './../interfaces';
+import {
+  CallHandler,
+  ExecutionContext,
+  NestInterceptor,
+} from './../interfaces';
 import { CLASS_SERIALIZER_OPTIONS } from './class-serializer.constants';
 
 let classTransformer: any = {};
@@ -26,16 +30,15 @@ export class ClassSerializerInterceptor implements NestInterceptor {
     classTransformer = loadPkg('class-transformer');
   }
 
-  intercept(
-    context: ExecutionContext,
-    call$: Observable<any>,
-  ): Observable<any> {
+  intercept(context: ExecutionContext, next: CallHandler): Observable<any> {
     const options = this.getContextOptions(context);
-    return call$.pipe(
-      map((res: PlainLiteralObject | Array<PlainLiteralObject>) =>
-        this.serialize(res, options),
-      ),
-    );
+    return next
+      .handle()
+      .pipe(
+        map((res: PlainLiteralObject | Array<PlainLiteralObject>) =>
+          this.serialize(res, options),
+        ),
+      );
   }
 
   serialize(

packages/common/test/files/interceptors/file-fields.interceptor.spec.ts

@@ -3,6 +3,7 @@ import { expect } from 'chai';
 import { of } from 'rxjs';
 import * as sinon from 'sinon';
 import { FileFieldsInterceptor } from '../../../files/interceptors/file-fields.interceptor';
+import { CallHandler } from '../../../interfaces/features/nest-interceptor.interface';
 
 describe('FileFieldsInterceptor', () => {
   it('should return metatype with expected structure', async () => {
@@ -13,9 +14,11 @@ describe('FileFieldsInterceptor', () => {
     expect(targetClass.prototype.intercept).to.not.be.undefined;
   });
   describe('intercept', () => {
-    let stream$;
+    let handler: CallHandler;
     beforeEach(() => {
-      stream$ = of('test');
+      handler = {
+        handle: () => of('test'),
+      };
     });
     it('should call object with expected params', async () => {
       const fieldName1 = 'file';
@@ -33,7 +36,7 @@ describe('FileFieldsInterceptor', () => {
         .stub((target as any).multer, 'fields')
         .returns(callback);
 
-      await target.intercept(new ExecutionContextHost([]), stream$);
+      await target.intercept(new ExecutionContextHost([]), handler);
 
       expect(fieldsSpy.called).to.be.true;
       expect(fieldsSpy.calledWith(argument)).to.be.true;
@@ -55,7 +58,7 @@ describe('FileFieldsInterceptor', () => {
         array: () => callback,
       };
       expect(
-        target.intercept(new ExecutionContextHost([]), stream$),
+        target.intercept(new ExecutionContextHost([]), handler),
       ).to.eventually.throw();
     });
   });

packages/common/test/files/interceptors/file.interceptor.spec.ts

@@ -3,16 +3,19 @@ import { expect } from 'chai';
 import { of } from 'rxjs';
 import * as sinon from 'sinon';
 import { FileInterceptor } from '../../../files/interceptors/file.interceptor';
+import { CallHandler } from '../../../interfaces/features/nest-interceptor.interface';
 
 describe('FileInterceptor', () => {
   it('should return metatype with expected structure', async () => {
     const targetClass = FileInterceptor('file');
     expect(targetClass.prototype.intercept).to.not.be.undefined;
   });
   describe('intercept', () => {
-    let stream$;
+    let handler: CallHandler;
     beforeEach(() => {
-      stream$ = of('test');
+      handler = {
+        handle: () => of('test'),
+      };
     });
     it('should call single() with expected params', async () => {
       const fieldName = 'file';
@@ -22,7 +25,7 @@ describe('FileInterceptor', () => {
         .stub((target as any).multer, 'single')
         .returns(callback);
 
-      await target.intercept(new ExecutionContextHost([]), stream$);
+      await target.intercept(new ExecutionContextHost([]), handler);
 
       expect(singleSpy.called).to.be.true;
       expect(singleSpy.calledWith(fieldName)).to.be.true;
@@ -37,7 +40,7 @@ describe('FileInterceptor', () => {
         single: () => callback,
       };
       expect(
-        target.intercept(new ExecutionContextHost([]), stream$),
+        target.intercept(new ExecutionContextHost([]), handler),
       ).to.eventually.throw();
     });
   });

packages/common/test/files/interceptors/files.interceptor.spec.ts

@@ -3,16 +3,19 @@ import { expect } from 'chai';
 import { of } from 'rxjs';
 import * as sinon from 'sinon';
 import { FilesInterceptor } from '../../../files/interceptors/files.interceptor';
+import { CallHandler } from '../../../interfaces/features/nest-interceptor.interface';
 
 describe('FilesInterceptor', () => {
   it('should return metatype with expected structure', async () => {
     const targetClass = FilesInterceptor('file');
     expect(targetClass.prototype.intercept).to.not.be.undefined;
   });
   describe('intercept', () => {
-    let stream$;
+    let handler: CallHandler;
     beforeEach(() => {
-      stream$ = of('test');
+      handler = {
+        handle: () => of('test'),
+      };
     });
     it('should call array() with expected params', async () => {
       const fieldName = 'file';
@@ -24,7 +27,7 @@ describe('FilesInterceptor', () => {
         .stub((target as any).multer, 'array')
         .returns(callback);
 
-      await target.intercept(new ExecutionContextHost([]), stream$);
+      await target.intercept(new ExecutionContextHost([]), handler);
 
       expect(arraySpy.called).to.be.true;
       expect(arraySpy.calledWith(fieldName, maxCount)).to.be.true;
@@ -39,7 +42,7 @@ describe('FilesInterceptor', () => {
         array: () => callback,
       };
       expect(
-        target.intercept(new ExecutionContextHost([]), stream$),
+        target.intercept(new ExecutionContextHost([]), handler),
       ).to.eventually.throw();
     });
   });