feat(router): allow guards to return UrlTree as well as boolean (#26521)

* Removed `andObservable` helper function in favor of inline implementation
* Flow `boolean | UrlTree` through guards check
* Add tests to verify behavior of `checkGuards` function flowing `UrlTree` properly

PR Close #26521

packages/router/src/apply_redirects.ts

@@ -8,14 +8,14 @@
 
 import {Injector, NgModuleRef} from '@angular/core';
 import {EmptyError, Observable, Observer, from, of } from 'rxjs';
-import {catchError, concatAll, first, map, mergeMap} from 'rxjs/operators';
+import {catchError, concatAll, every, first, map, mergeMap} from 'rxjs/operators';
 
 import {LoadedRouterConfig, Route, Routes} from './config';
 import {CanLoadFn} from './interfaces';
 import {RouterConfigLoader} from './router_config_loader';
 import {PRIMARY_OUTLET, Params, defaultUrlMatcher, navigationCancelingError} from './shared';
 import {UrlSegment, UrlSegmentGroup, UrlSerializer, UrlTree} from './url_tree';
-import {andObservables, forEach, waitForMap, wrapIntoObservable} from './utils/collection';
+import {forEach, waitForMap, wrapIntoObservable} from './utils/collection';
 import {isCanLoad, isFunction} from './utils/type_guards';
 
 class NoMatch {
@@ -421,7 +421,7 @@ function runCanLoadGuard(
     return wrapIntoObservable(guardVal);
   }));
 
-  return andObservables(obs);
+  return obs.pipe(concatAll(), every(result => result === true));
 }
 
 function match(segmentGroup: UrlSegmentGroup, route: Route, segments: UrlSegment[]): {

packages/router/src/operators/check_guards.ts

@@ -7,17 +7,19 @@
  */
 
 import {Injector} from '@angular/core';
-import {MonoTypeOperatorFunction, Observable, from, of } from 'rxjs';
+import {MonoTypeOperatorFunction, Observable, defer, from, of } from 'rxjs';
-import {concatMap, every, first, map, mergeMap} from 'rxjs/operators';
+import {concatAll, concatMap, first, map, mergeMap} from 'rxjs/operators';
 
 import {ActivationStart, ChildActivationStart, Event} from '../events';
 import {CanActivateChildFn, CanActivateFn, CanDeactivateFn} from '../interfaces';
 import {NavigationTransition} from '../router';
 import {ActivatedRouteSnapshot, RouterStateSnapshot} from '../router_state';
 import {UrlTree} from '../url_tree';
-import {andObservables, wrapIntoObservable} from '../utils/collection';
+import {wrapIntoObservable} from '../utils/collection';
 import {CanActivate, CanDeactivate, getCanActivateChild, getToken} from '../utils/preactivation';
-import {isCanActivate, isCanActivateChild, isCanDeactivate, isFunction} from '../utils/type_guards';
+import {isCanActivate, isCanActivateChild, isCanDeactivate, isFunction, isBoolean} from '../utils/type_guards';
+
+import {prioritizedGuardValue} from './prioritized_guard_value';
 
 export function checkGuards(moduleInjector: Injector, forwardEvent?: (evt: Event) => void):
     MonoTypeOperatorFunction<NavigationTransition> {
@@ -33,10 +35,10 @@ export function checkGuards(moduleInjector: Injector, forwardEvent?: (evt: Event
                  canDeactivateChecks, targetSnapshot !, currentSnapshot, moduleInjector)
           .pipe(
               mergeMap(canDeactivate => {
-                return canDeactivate ?
+                return canDeactivate && isBoolean(canDeactivate) ?
                     runCanActivateChecks(
                         targetSnapshot !, canActivateChecks, moduleInjector, forwardEvent) :
-                    of (false);
+                    of (canDeactivate);
               }),
               map(guardsResult => ({...t, guardsResult})));
     }));
@@ -45,25 +47,30 @@ export function checkGuards(moduleInjector: Injector, forwardEvent?: (evt: Event
 
 function runCanDeactivateChecks(
     checks: CanDeactivate[], futureRSS: RouterStateSnapshot, currRSS: RouterStateSnapshot,
-    moduleInjector: Injector): Observable<boolean|UrlTree> {
+    moduleInjector: Injector) {
   return from(checks).pipe(
-      mergeMap(check => {
+      mergeMap(
-        return runCanDeactivate(check.component, check.route, currRSS, futureRSS, moduleInjector);
+          check =>
-      }),
+              runCanDeactivate(check.component, check.route, currRSS, futureRSS, moduleInjector)),
-      every(result => result === true));
+      first(result => { return result !== true; }, true as boolean | UrlTree));
 }
 
 function runCanActivateChecks(
     futureSnapshot: RouterStateSnapshot, checks: CanActivate[], moduleInjector: Injector,
-    forwardEvent?: (evt: Event) => void): Observable<boolean> {
+    forwardEvent?: (evt: Event) => void) {
   return from(checks).pipe(
-      concatMap((check: CanActivate) => andObservables(from([
+      concatMap((check: CanActivate) => {
-                  fireChildActivationStart(check.route.parent, forwardEvent),
+        return from([
-                  fireActivationStart(check.route, forwardEvent),
+                 fireChildActivationStart(check.route.parent, forwardEvent),
-                  runCanActivateChild(futureSnapshot, check.path, moduleInjector),
+                 fireActivationStart(check.route, forwardEvent),
-                  runCanActivate(futureSnapshot, check.route, moduleInjector)
+                 runCanActivateChild(futureSnapshot, check.path, moduleInjector),
-                ]))),
+                 runCanActivate(futureSnapshot, check.route, moduleInjector)
-      every((result: boolean) => result === true));
+               ])
+            .pipe(concatAll(), first(result => {
+                    return result !== true;
+                  }, true as boolean | UrlTree));
+      }),
+      first(result => { return result !== true; }, true as boolean | UrlTree));
 }
 
 /**
@@ -102,27 +109,30 @@ function fireChildActivationStart(
 
 function runCanActivate(
     futureRSS: RouterStateSnapshot, futureARS: ActivatedRouteSnapshot,
-    moduleInjector: Injector): Observable<boolean> {
+    moduleInjector: Injector): Observable<boolean|UrlTree> {
   const canActivate = futureARS.routeConfig ? futureARS.routeConfig.canActivate : null;
   if (!canActivate || canActivate.length === 0) return of (true);
-  const obs = from(canActivate).pipe(map((c: any) => {
+
-    const guard = getToken(c, futureARS, moduleInjector);
+  const canActivateObservables = canActivate.map((c: any) => {
-    let observable;
+    return defer(() => {
-    if (isCanActivate(guard)) {
+      const guard = getToken(c, futureARS, moduleInjector);
-      observable = wrapIntoObservable(guard.canActivate(futureARS, futureRSS));
+      let observable;
-    } else if (isFunction<CanActivateFn>(guard)) {
+      if (isCanActivate(guard)) {
-      observable = wrapIntoObservable(guard(futureARS, futureRSS));
+        observable = wrapIntoObservable(guard.canActivate(futureARS, futureRSS));
-    } else {
+      } else if (isFunction<CanActivateFn>(guard)) {
-      throw new Error('Invalid canActivate guard');
+        observable = wrapIntoObservable(guard(futureARS, futureRSS));
-    }
+      } else {
-    return observable.pipe(first());
+        throw new Error('Invalid CanActivate guard');
-  }));
+      }
-  return andObservables(obs);
+      return observable.pipe(first());
+    });
+  });
+  return of (canActivateObservables).pipe(prioritizedGuardValue());
 }
 
 function runCanActivateChild(
     futureRSS: RouterStateSnapshot, path: ActivatedRouteSnapshot[],
-    moduleInjector: Injector): Observable<boolean> {
+    moduleInjector: Injector): Observable<boolean|UrlTree> {
   const futureARS = path[path.length - 1];
 
   const canActivateChildGuards = path.slice(0, path.length - 1)
@@ -130,29 +140,32 @@ function runCanActivateChild(
                                      .map(p => getCanActivateChild(p))
                                      .filter(_ => _ !== null);
 
-  return andObservables(from(canActivateChildGuards).pipe(map((d: any) => {
+  const canActivateChildGuardsMapped = canActivateChildGuards.map((d: any) => {
-    const obs = from(d.guards).pipe(map((c: any) => {
+    return defer(() => {
-      const guard = getToken(c, d.node, moduleInjector);
+      const guardsMapped = d.guards.map((c: any) => {
-      let observable;
+        const guard = getToken(c, d.node, moduleInjector);
-      if (isCanActivateChild(guard)) {
+        let observable;
-        observable = wrapIntoObservable(guard.canActivateChild(futureARS, futureRSS));
+        if (isCanActivateChild(guard)) {
-      } else if (isFunction<CanActivateChildFn>(guard)) {
+          observable = wrapIntoObservable(guard.canActivateChild(futureARS, futureRSS));
-        observable = wrapIntoObservable(guard(futureARS, futureRSS));
+        } else if (isFunction<CanActivateChildFn>(guard)) {
-      } else {
+          observable = wrapIntoObservable(guard(futureARS, futureRSS));
-        throw new Error('Invalid CanActivateChild guard');
+        } else {
-      }
+          throw new Error('Invalid CanActivateChild guard');
-      return observable.pipe(first());
+        }
-    }));
+        return observable.pipe(first());
-    return andObservables(obs);
+      });
-  })));
+      return of (guardsMapped).pipe(prioritizedGuardValue());
+    });
+  });
+  return of (canActivateChildGuardsMapped).pipe(prioritizedGuardValue());
 }
 
 function runCanDeactivate(
-    component: Object| null, currARS: ActivatedRouteSnapshot, currRSS: RouterStateSnapshot,
+    component: Object | null, currARS: ActivatedRouteSnapshot, currRSS: RouterStateSnapshot,
     futureRSS: RouterStateSnapshot, moduleInjector: Injector): Observable<boolean|UrlTree> {
   const canDeactivate = currARS && currARS.routeConfig ? currARS.routeConfig.canDeactivate : null;
   if (!canDeactivate || canDeactivate.length === 0) return of (true);
-  const canDeactivate$ = from(canDeactivate).pipe(mergeMap((c: any) => {
+  const canDeactivateObservables = canDeactivate.map((c: any) => {
     const guard = getToken(c, currARS, moduleInjector);
     let observable;
     if (isCanDeactivate(guard)) {
@@ -164,6 +177,6 @@ function runCanDeactivate(
       throw new Error('Invalid CanDeactivate guard');
     }
     return observable.pipe(first());
-  }));
+  });
-  return canDeactivate$.pipe(every((result: any) => result === true));
+  return of (canDeactivateObservables).pipe(prioritizedGuardValue());
 }

packages/router/src/router.ts

@@ -186,7 +186,7 @@ export type NavigationTransition = {
   currentRouterState: RouterState,
   targetRouterState: RouterState | null,
   guards: Checks,
-  guardsResult: boolean | null,
+  guardsResult: boolean | UrlTree | null,
 };
 
 /**

packages/router/src/utils/collection.ts

@@ -8,7 +8,7 @@
 
 import {NgModuleFactory, ɵisObservable as isObservable, ɵisPromise as isPromise} from '@angular/core';
 import {Observable, from, of } from 'rxjs';
-import {concatAll, every, last as lastValue, map, mergeAll} from 'rxjs/operators';
+import {concatAll, last as lastValue, map} from 'rxjs/operators';
 
 import {PRIMARY_OUTLET} from '../shared';
 
@@ -88,14 +88,6 @@ export function waitForMap<A, B>(
   return of .apply(null, waitHead.concat(waitTail)).pipe(concatAll(), lastValue(), map(() => res));
 }
 
-/**
- * ANDs Observables by merging all input observables, reducing to an Observable verifying all
- * input Observables return `true`.
- */
-export function andObservables(observables: Observable<Observable<any>>): Observable<boolean> {
-  return observables.pipe(mergeAll(), every((result: any) => result === true));
-}
-
 export function wrapIntoObservable<T>(value: T | NgModuleFactory<T>| Promise<T>| Observable<T>) {
   if (isObservable(value)) {
     return value;

packages/router/src/utils/type_guards.ts

@@ -6,7 +6,6 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Type} from '@angular/core';
 import {CanActivate, CanActivateChild, CanDeactivate, CanLoad} from '../interfaces';
 
 /**
@@ -26,6 +25,10 @@ export function isFunction<T>(v: any): v is T {
   return typeof v === 'function';
 }
 
+export function isBoolean(v: any): v is boolean {
+  return typeof v === 'boolean';
+}
+
 export function isCanLoad(guard: any): guard is CanLoad {
   return guard && isFunction<CanLoad>(guard.canLoad);
 }

packages/router/test/helpers.ts

@@ -19,7 +19,7 @@ export class Logger {
   empty() { this.logs.length = 0; }
 }
 
-export function provideTokenLogger(token: string, returnValue = true) {
+export function provideTokenLogger(token: string, returnValue = true as boolean | UrlTree) {
   return {
     provide: token,
     useFactory: (logger: Logger) => () => (logger.add(token), returnValue),

packages/router/test/router.spec.ts

@@ -17,7 +17,7 @@ import {resolveData as resolveDataOperator} from '../src/operators/resolve_data'
 import {NavigationTransition, Router} from '../src/router';
 import {ChildrenOutletContexts} from '../src/router_outlet_context';
 import {RouterStateSnapshot, createEmptyStateSnapshot} from '../src/router_state';
-import {DefaultUrlSerializer} from '../src/url_tree';
+import {DefaultUrlSerializer, UrlTree} from '../src/url_tree';
 import {getAllRouteGuards} from '../src/utils/preactivation';
 import {TreeNode} from '../src/utils/tree';
 import {RouterTestingModule} from '../testing/src/router_testing_module';
@@ -103,23 +103,38 @@ describe('Router', () => {
 
     const CA_CHILD = 'canActivate_child';
     const CA_CHILD_FALSE = 'canActivate_child_false';
+    const CA_CHILD_REDIRECT = 'canActivate_child_redirect';
     const CAC_CHILD = 'canActivateChild_child';
     const CAC_CHILD_FALSE = 'canActivateChild_child_false';
+    const CAC_CHILD_REDIRECT = 'canActivateChild_child_redirect';
     const CA_GRANDCHILD = 'canActivate_grandchild';
     const CA_GRANDCHILD_FALSE = 'canActivate_grandchild_false';
+    const CA_GRANDCHILD_REDIRECT = 'canActivate_grandchild_redirect';
     const CDA_CHILD = 'canDeactivate_child';
     const CDA_CHILD_FALSE = 'canDeactivate_child_false';
+    const CDA_CHILD_REDIRECT = 'canDeactivate_child_redirect';
     const CDA_GRANDCHILD = 'canDeactivate_grandchild';
     const CDA_GRANDCHILD_FALSE = 'canDeactivate_grandchild_false';
+    const CDA_GRANDCHILD_REDIRECT = 'canDeactivate_grandchild_redirect';
 
     beforeEach(() => {
+
       TestBed.configureTestingModule({
+        imports: [RouterTestingModule],
         providers: [
           Logger, provideTokenLogger(CA_CHILD), provideTokenLogger(CA_CHILD_FALSE, false),
+          provideTokenLogger(CA_CHILD_REDIRECT, serializer.parse('/canActivate_child_redirect')),
           provideTokenLogger(CAC_CHILD), provideTokenLogger(CAC_CHILD_FALSE, false),
+          provideTokenLogger(
+              CAC_CHILD_REDIRECT, serializer.parse('/canActivateChild_child_redirect')),
           provideTokenLogger(CA_GRANDCHILD), provideTokenLogger(CA_GRANDCHILD_FALSE, false),
+          provideTokenLogger(
+              CA_GRANDCHILD_REDIRECT, serializer.parse('/canActivate_grandchild_redirect')),
           provideTokenLogger(CDA_CHILD), provideTokenLogger(CDA_CHILD_FALSE, false),
-          provideTokenLogger(CDA_GRANDCHILD), provideTokenLogger(CDA_GRANDCHILD_FALSE, false)
+          provideTokenLogger(CDA_CHILD_REDIRECT, serializer.parse('/canDeactivate_child_redirect')),
+          provideTokenLogger(CDA_GRANDCHILD), provideTokenLogger(CDA_GRANDCHILD_FALSE, false),
+          provideTokenLogger(
+              CDA_GRANDCHILD_REDIRECT, serializer.parse('/canDeactivate_grandchild_redirect'))
         ]
       });
 
@@ -389,11 +404,11 @@ describe('Router', () => {
 
       it('should not run activate if deactivate fails guards', () => {
         /**
-         *      R  -->  R
+         *      R  -->     R
-         *     /         \
+         *     /            \
-         *    prev (CDA)  child (CA)
+         *    prev (CDA: x)  child (CA)
-         *                 \
+         *                    \
-         *                  grandchild (CA)
+         *                     grandchild (CA)
          */
 
         const prevSnapshot = createActivatedRouteSnapshot(
@@ -460,6 +475,114 @@ describe('Router', () => {
           expect(logger.logs).toEqual([]);
         });
       });
+
+      describe('UrlTree', () => {
+        it('should allow return of UrlTree from CanActivate', () => {
+          /**
+           * R  -->  R
+           *          \
+           *           child (CA: redirect)
+           */
+
+          const childSnapshot = createActivatedRouteSnapshot({
+            component: 'child',
+            routeConfig: {
+
+              canActivate: [CA_CHILD_REDIRECT]
+            }
+          });
+
+          const futureState = new (RouterStateSnapshot as any)(
+              'url', new TreeNode(empty.root, [new TreeNode(childSnapshot, [])]));
+
+          checkGuards(futureState, empty, TestBed, (result) => {
+            expect(serializer.serialize(result as UrlTree)).toBe('/' + CA_CHILD_REDIRECT);
+            expect(logger.logs).toEqual([CA_CHILD_REDIRECT]);
+          });
+        });
+
+        it('should allow return of UrlTree from CanActivateChild', () => {
+          /**
+           * R  -->  R
+           *          \
+           *           child (CAC: redirect)
+           *            \
+           *             grandchild (CA)
+           */
+
+          const childSnapshot = createActivatedRouteSnapshot(
+              {component: 'child', routeConfig: {canActivateChild: [CAC_CHILD_REDIRECT]}});
+          const grandchildSnapshot = createActivatedRouteSnapshot(
+              {component: 'grandchild', routeConfig: {canActivate: [CA_GRANDCHILD]}});
+
+          const futureState = new (RouterStateSnapshot as any)(
+              'url', new TreeNode(
+                         empty.root,
+                         [new TreeNode(childSnapshot, [new TreeNode(grandchildSnapshot, [])])]));
+
+          checkGuards(futureState, empty, TestBed, (result) => {
+            expect(serializer.serialize(result as UrlTree)).toBe('/' + CAC_CHILD_REDIRECT);
+            expect(logger.logs).toEqual([CAC_CHILD_REDIRECT]);
+          });
+        });
+
+        it('should allow return of UrlTree from a child CanActivate', () => {
+          /**
+           * R  -->  R
+           *          \
+           *           child (CAC)
+           *            \
+           *             grandchild (CA: redirect)
+           */
+
+          const childSnapshot = createActivatedRouteSnapshot(
+              {component: 'child', routeConfig: {canActivateChild: [CAC_CHILD]}});
+          const grandchildSnapshot = createActivatedRouteSnapshot(
+              {component: 'grandchild', routeConfig: {canActivate: [CA_GRANDCHILD_REDIRECT]}});
+
+          const futureState = new (RouterStateSnapshot as any)(
+              'url', new TreeNode(
+                         empty.root,
+                         [new TreeNode(childSnapshot, [new TreeNode(grandchildSnapshot, [])])]));
+
+          checkGuards(futureState, empty, TestBed, (result) => {
+            expect(serializer.serialize(result as UrlTree)).toBe('/' + CA_GRANDCHILD_REDIRECT);
+            expect(logger.logs).toEqual([CAC_CHILD, CA_GRANDCHILD_REDIRECT]);
+          });
+        });
+
+        it('should allow return of UrlTree from a child CanDeactivate', () => {
+          /**
+           *      R  -->            R
+           *     /                   \
+           *    prev (CDA: redirect)  child (CA)
+           *                           \
+           *                            grandchild (CA)
+           */
+
+          const prevSnapshot = createActivatedRouteSnapshot(
+              {component: 'prev', routeConfig: {canDeactivate: [CDA_CHILD_REDIRECT]}});
+          const childSnapshot = createActivatedRouteSnapshot({
+            component: 'child',
+            routeConfig: {canActivate: [CA_CHILD], canActivateChild: [CAC_CHILD]}
+          });
+          const grandchildSnapshot = createActivatedRouteSnapshot(
+              {component: 'grandchild', routeConfig: {canActivate: [CA_GRANDCHILD]}});
+
+          const currentState = new (RouterStateSnapshot as any)(
+              'prev', new TreeNode(empty.root, [new TreeNode(prevSnapshot, [])]));
+          const futureState = new (RouterStateSnapshot as any)(
+              'url', new TreeNode(
+                         empty.root,
+                         [new TreeNode(childSnapshot, [new TreeNode(grandchildSnapshot, [])])]));
+
+          checkGuards(futureState, currentState, TestBed, (result) => {
+            expect(serializer.serialize(result as UrlTree)).toBe('/' + CDA_CHILD_REDIRECT);
+            expect(logger.logs).toEqual([CDA_CHILD_REDIRECT]);
+          });
+        });
+
+      });
     });
 
     describe('resolve', () => {
@@ -544,10 +667,15 @@ function checkResolveData(
 
 function checkGuards(
     future: RouterStateSnapshot, curr: RouterStateSnapshot, injector: any,
-    check: (result: boolean) => void): void {
+    check: (result: boolean | UrlTree) => void): void {
   of ({
     guards: getAllRouteGuards(future, curr, new ChildrenOutletContexts())
   } as Partial<NavigationTransition>)
       .pipe(checkGuardsOperator(injector))
-      .subscribe(t => check(!!t.guardsResult), (e) => { throw e; });
+      .subscribe(
+          t => {
+            if (t.guardsResult === null) throw new Error('Guard result expected');
+            return check(t.guardsResult);
+          },
+          (e) => { throw e; });
 }