From 120bdeecdc9ecb3f760ed38547991e0b87bbaba4 Mon Sep 17 00:00:00 2001
From: Alex Rickabaugh <alx+alxhub@alxandria.net>
Date: Thu, 19 Oct 2017 09:31:10 -0700
Subject: [PATCH] fix(common): allow HttpInterceptors to inject HttpClient
 (#19809)

Previously, an interceptor attempting to inject HttpClient directly
would receive a circular dependency error, as HttpClient was
constructed via a factory which injected the interceptor instances.
Users want to inject HttpClient into interceptors to make supporting
requests (ex: to retrieve an authentication token). Currently this is
only possible by injecting the Injector and using it to resolve
HttpClient at request time.

Either HttpClient or the user has to deal specially with the circular
dependency. This change moves that responsibility into HttpClient
itself. By utilizing a new class HttpInterceptingHandler which lazily
loads the set of interceptors at request time, it's possible to inject
HttpClient directly into interceptors as construction of HttpClient no
longer requires the interceptor chain to be constructed.

Fixes #18224.

PR Close #19809
---
 packages/common/http/src/module.ts       | 35 ++++++++++++++++++------
 packages/common/http/test/module_spec.ts | 22 ++++++++++++++-
 2 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/packages/common/http/src/module.ts b/packages/common/http/src/module.ts
index 08dce24ed2..b16809c760 100644
--- a/packages/common/http/src/module.ts
+++ b/packages/common/http/src/module.ts
@@ -6,16 +6,41 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Inject, ModuleWithProviders, NgModule, Optional} from '@angular/core';
+import {Injectable, Injector, ModuleWithProviders, NgModule, Optional} from '@angular/core';
+import {Observable} from 'rxjs/Observable';
 
 import {HttpBackend, HttpHandler} from './backend';
 import {HttpClient} from './client';
 import {HTTP_INTERCEPTORS, HttpInterceptor, HttpInterceptorHandler, NoopInterceptor} from './interceptor';
 import {JsonpCallbackContext, JsonpClientBackend, JsonpInterceptor} from './jsonp';
+import {HttpRequest} from './request';
+import {HttpEvent} from './response';
 import {BrowserXhr, HttpXhrBackend, XhrFactory} from './xhr';
 import {HttpXsrfCookieExtractor, HttpXsrfInterceptor, HttpXsrfTokenExtractor, XSRF_COOKIE_NAME, XSRF_HEADER_NAME} from './xsrf';
 
+/**
+ * An `HttpHandler` that applies a bunch of `HttpInterceptor`s
+ * to a request before passing it to the given `HttpBackend`.
+ *
+ * The interceptors are loaded lazily from the injector, to allow
+ * interceptors to themselves inject classes depending indirectly
+ * on `HttpInterceptingHandler` itself.
+ */
+@Injectable()
+export class HttpInterceptingHandler implements HttpHandler {
+  private chain: HttpHandler|null = null;
 
+  constructor(private backend: HttpBackend, private injector: Injector) {}
+
+  handle(req: HttpRequest<any>): Observable<HttpEvent<any>> {
+    if (this.chain === null) {
+      const interceptors = this.injector.get(HTTP_INTERCEPTORS, []);
+      this.chain = interceptors.reduceRight(
+          (next, interceptor) => new HttpInterceptorHandler(next, interceptor), this.backend);
+    }
+    return this.chain.handle(req);
+  }
+}
 
 /**
  * Constructs an `HttpHandler` that applies a bunch of `HttpInterceptor`s
@@ -118,13 +143,7 @@ export class HttpClientXsrfModule {
   ],
   providers: [
     HttpClient,
-    // HttpHandler is the backend + interceptors and is constructed
-    // using the interceptingHandler factory function.
-    {
-      provide: HttpHandler,
-      useFactory: interceptingHandler,
-      deps: [HttpBackend, [new Optional(), new Inject(HTTP_INTERCEPTORS)]],
-    },
+    {provide: HttpHandler, useClass: HttpInterceptingHandler},
     HttpXhrBackend,
     {provide: HttpBackend, useExisting: HttpXhrBackend},
     BrowserXhr,
diff --git a/packages/common/http/test/module_spec.ts b/packages/common/http/test/module_spec.ts
index 7773d115f6..68e91490ae 100644
--- a/packages/common/http/test/module_spec.ts
+++ b/packages/common/http/test/module_spec.ts
@@ -8,7 +8,7 @@
 
 import 'rxjs/add/operator/map';
 
-import {Injector} from '@angular/core';
+import {Injectable, Injector} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {Observable} from 'rxjs/Observable';
 
@@ -47,6 +47,15 @@ class InterceptorB extends TestInterceptor {
   constructor() { super('B'); }
 }
 
+@Injectable()
+class ReentrantInterceptor implements HttpInterceptor {
+  constructor(private client: HttpClient) {}
+
+  intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
+    return next.handle(req);
+  }
+}
+
 {
   describe('HttpClientModule', () => {
     let injector: Injector;
@@ -84,5 +93,16 @@ class InterceptorB extends TestInterceptor {
           });
       injector.get(HttpTestingController).expectOne('/test').flush('ok!');
     });
+    it('allows interceptors to inject HttpClient', (done: DoneFn) => {
+      TestBed.resetTestingModule();
+      injector = TestBed.configureTestingModule({
+        imports: [HttpClientTestingModule],
+        providers: [
+          {provide: HTTP_INTERCEPTORS, useClass: ReentrantInterceptor, multi: true},
+        ],
+      });
+      injector.get(HttpClient).get('/test').subscribe(() => { done(); });
+      injector.get(HttpTestingController).expectOne('/test').flush('ok!');
+    });
   });
 }