From 60aa943e2dd4654600c0e53e51b8d7a1ac3730da Mon Sep 17 00:00:00 2001 From: Vikram Subramanian Date: Thu, 31 May 2018 10:35:51 -0700 Subject: [PATCH] fix(platform-server): avoid dependency cycle when using http interceptor (#24229) Fixes #23023. When a HTTP Interceptor injects HttpClient it causes a DI cycle. This fix is to use Injector to lazily inject HTTP_INTERCEPTORS while setting up the HttpHandler on the server so as to break the cycle. PR Close #24229 --- packages/common/http/public_api.ts | 2 +- packages/common/http/src/module.ts | 17 -------- packages/platform-server/src/http.ts | 11 +++-- .../platform-server/test/integration_spec.ts | 40 ++++++++++++++++++- 4 files changed, 44 insertions(+), 26 deletions(-) diff --git a/packages/common/http/public_api.ts b/packages/common/http/public_api.ts index 8ac2b95de8..9975cfc8f5 100644 --- a/packages/common/http/public_api.ts +++ b/packages/common/http/public_api.ts @@ -11,7 +11,7 @@ export {HttpClient} from './src/client'; export {HttpHeaders} from './src/headers'; export {HTTP_INTERCEPTORS, HttpInterceptor} from './src/interceptor'; export {JsonpClientBackend, JsonpInterceptor} from './src/jsonp'; -export {HttpClientJsonpModule, HttpClientModule, HttpClientXsrfModule, interceptingHandler as ɵinterceptingHandler} from './src/module'; +export {HttpClientJsonpModule, HttpClientModule, HttpClientXsrfModule, HttpInterceptingHandler as ɵHttpInterceptingHandler} from './src/module'; export {HttpParameterCodec, HttpParams, HttpUrlEncodingCodec} from './src/params'; export {HttpRequest} from './src/request'; export {HttpDownloadProgressEvent, HttpErrorResponse, HttpEvent, HttpEventType, HttpHeaderResponse, HttpProgressEvent, HttpResponse, HttpResponseBase, HttpSentEvent, HttpUserEvent} from './src/response'; diff --git a/packages/common/http/src/module.ts b/packages/common/http/src/module.ts index 08ebe63e36..83a9fbfc94 100644 --- a/packages/common/http/src/module.ts +++ b/packages/common/http/src/module.ts @@ -42,23 +42,6 @@ export class HttpInterceptingHandler implements HttpHandler { } } -/** - * Constructs an `HttpHandler` that applies a bunch of `HttpInterceptor`s - * to a request before passing it to the given `HttpBackend`. - * - * Meant to be used as a factory function within `HttpClientModule`. - * - * - */ -export function interceptingHandler( - backend: HttpBackend, interceptors: HttpInterceptor[] | null = []): HttpHandler { - if (!interceptors) { - return backend; - } - return interceptors.reduceRight( - (next, interceptor) => new HttpInterceptorHandler(next, interceptor), backend); -} - /** * Factory function that determines where to store JSONP callbacks. * diff --git a/packages/platform-server/src/http.ts b/packages/platform-server/src/http.ts index 56e135c4b2..1fc55db209 100644 --- a/packages/platform-server/src/http.ts +++ b/packages/platform-server/src/http.ts @@ -8,10 +8,10 @@ const xhr2: any = require('xhr2'); -import {Injectable, Optional, Provider} from '@angular/core'; +import {Injectable, Injector, Optional, Provider, InjectFlags} from '@angular/core'; import {BrowserXhr, Connection, ConnectionBackend, Http, ReadyState, Request, RequestOptions, Response, XHRBackend, XSRFStrategy} from '@angular/http'; -import {HttpEvent, HttpRequest, HttpHandler, HttpInterceptor, HTTP_INTERCEPTORS, HttpBackend, XhrFactory, ɵinterceptingHandler as interceptingHandler} from '@angular/common/http'; +import {HttpEvent, HttpRequest, HttpHandler, HttpInterceptor, HTTP_INTERCEPTORS, HttpBackend, XhrFactory, ɵHttpInterceptingHandler as HttpInterceptingHandler} from '@angular/common/http'; import {Observable, Observer, Subscription} from 'rxjs'; @@ -156,9 +156,8 @@ export function httpFactory(xhrBackend: XHRBackend, options: RequestOptions) { return new Http(macroBackend, options); } -export function zoneWrappedInterceptingHandler( - backend: HttpBackend, interceptors: HttpInterceptor[] | null) { - const realBackend: HttpBackend = interceptingHandler(backend, interceptors); +export function zoneWrappedInterceptingHandler(backend: HttpBackend, injector: Injector) { + const realBackend: HttpBackend = new HttpInterceptingHandler(backend, injector); return new ZoneClientBackend(realBackend); } @@ -168,6 +167,6 @@ export const SERVER_HTTP_PROVIDERS: Provider[] = [ {provide: XhrFactory, useClass: ServerXhr}, { provide: HttpHandler, useFactory: zoneWrappedInterceptingHandler, - deps: [HttpBackend, [new Optional(), HTTP_INTERCEPTORS]] + deps: [HttpBackend, Injector] } ]; diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts index 11c836bfe3..4d8d8240c0 100644 --- a/packages/platform-server/test/integration_spec.ts +++ b/packages/platform-server/test/integration_spec.ts @@ -8,15 +8,16 @@ import {AnimationBuilder, animate, style, transition, trigger} from '@angular/animations'; import {APP_BASE_HREF, PlatformLocation, isPlatformServer} from '@angular/common'; -import {HttpClient, HttpClientModule} from '@angular/common/http'; +import {HTTP_INTERCEPTORS, HttpClient, HttpClientModule, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest} from '@angular/common/http'; import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing'; -import {ApplicationRef, CompilerFactory, Component, HostListener, Inject, Input, NgModule, NgModuleRef, NgZone, PLATFORM_ID, PlatformRef, ViewEncapsulation, destroyPlatform, getPlatform} from '@angular/core'; +import {ApplicationRef, CompilerFactory, Component, HostListener, Inject, Injectable, Input, NgModule, NgModuleRef, NgZone, PLATFORM_ID, PlatformRef, ViewEncapsulation, destroyPlatform, getPlatform} from '@angular/core'; import {TestBed, async, inject} from '@angular/core/testing'; import {Http, HttpModule, Response, ResponseOptions, XHRBackend} from '@angular/http'; import {MockBackend, MockConnection} from '@angular/http/testing'; import {BrowserModule, DOCUMENT, StateKey, Title, TransferState, makeStateKey} from '@angular/platform-browser'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; import {BEFORE_APP_SERIALIZED, INITIAL_CONFIG, PlatformState, ServerModule, ServerTransferStateModule, platformDynamicServer, renderModule, renderModuleFactory} from '@angular/platform-server'; +import {Observable} from 'rxjs'; import {first} from 'rxjs/operators'; @Component({selector: 'app', template: `Works!`}) @@ -202,6 +203,26 @@ export class HttpAfterExampleModule { export class HttpClientExampleModule { } +@Injectable() +export class MyHttpInterceptor implements HttpInterceptor { + constructor(private http: HttpClient) {} + + intercept(req: HttpRequest, next: HttpHandler): Observable> { + return next.handle(req); + } +} + +@NgModule({ + bootstrap: [MyServerApp], + declarations: [MyServerApp], + imports: [ServerModule, HttpClientModule, HttpClientTestingModule], + providers: [ + {provide: HTTP_INTERCEPTORS, multi: true, useClass: MyHttpInterceptor}, + ], +}) +export class HttpInterceptorExampleModule { +} + @Component({selector: 'app', template: ``}) class ImageApp { } @@ -750,6 +771,21 @@ class EscapedTransferStoreModule { }); }); })); + it('can use HttpInterceptor that injects HttpClient', () => { + const platform = + platformDynamicServer([{provide: INITIAL_CONFIG, useValue: {document: ''}}]); + platform.bootstrapModule(HttpInterceptorExampleModule).then(ref => { + const mock = ref.injector.get(HttpTestingController) as HttpTestingController; + const http = ref.injector.get(HttpClient); + ref.injector.get(NgZone).run(() => { + http.get('http://localhost/testing').subscribe(body => { + NgZone.assertInAngularZone(); + expect(body).toEqual('success!'); + }); + mock.expectOne('http://localhost/testing').flush('success!'); + }); + }); + }); }); describe('ServerTransferStoreModule', () => {