From ca23b4c55f13dc66a47f715064ba5a677dacc603 Mon Sep 17 00:00:00 2001
From: vsavkin <avix1000@gmail.com>
Date: Thu, 16 Jun 2016 14:36:51 -0700
Subject: [PATCH] feat(router): add route config validation

---
 modules/@angular/router/src/config.ts       | 21 +++++++++
 modules/@angular/router/src/router.ts       | 11 +++--
 modules/@angular/router/test/config.spec.ts | 50 +++++++++++++++++++++
 modules/@angular/router/tsconfig.json       |  1 +
 4 files changed, 80 insertions(+), 3 deletions(-)
 create mode 100644 modules/@angular/router/test/config.spec.ts

diff --git a/modules/@angular/router/src/config.ts b/modules/@angular/router/src/config.ts
index f40b3c4de5..33e337b1a4 100644
--- a/modules/@angular/router/src/config.ts
+++ b/modules/@angular/router/src/config.ts
@@ -11,4 +11,25 @@ export interface Route {
   canDeactivate?: any[];
   redirectTo?: string;
   children?: Route[];
+}
+
+export function validateConfig(config: RouterConfig): void {
+  config.forEach(validateNode);
+}
+
+function validateNode(route: Route): void {
+  if (!!route.redirectTo && !!route.children) {
+    throw new Error(
+        `Invalid configuration of route '${route.path}': redirectTo and children cannot be used together`);
+  }
+  if (!!route.redirectTo && !!route.component) {
+    throw new Error(
+        `Invalid configuration of route '${route.path}': redirectTo and component cannot be used together`);
+  }
+  if (route.path === undefined) {
+    throw new Error(`Invalid route configuration: routes must have path specified`);
+  }
+  if (route.path.startsWith('/')) {
+    throw new Error(`Invalid route configuration of route '/a': path cannot start with a slash`);
+  }
 }
\ No newline at end of file
diff --git a/modules/@angular/router/src/router.ts b/modules/@angular/router/src/router.ts
index b5347be38d..f6ba9ba277 100644
--- a/modules/@angular/router/src/router.ts
+++ b/modules/@angular/router/src/router.ts
@@ -15,7 +15,7 @@ import {Subscription} from 'rxjs/Subscription';
 import {of } from 'rxjs/observable/of';
 
 import {applyRedirects} from './apply_redirects';
-import {RouterConfig} from './config';
+import {RouterConfig, validateConfig} from './config';
 import {createRouterState} from './create_router_state';
 import {createUrlTree} from './create_url_tree';
 import {RouterOutlet} from './directives/router_outlet';
@@ -99,6 +99,7 @@ export class Router {
   private locationSubscription: Subscription;
   private routerEvents: Subject<Event>;
   private navigationId: number = 0;
+  private config: RouterConfig;
 
   /**
    * @internal
@@ -106,7 +107,8 @@ export class Router {
   constructor(
       private rootComponentType: Type, private resolver: ComponentResolver,
       private urlSerializer: UrlSerializer, private outletMap: RouterOutletMap,
-      private location: Location, private injector: Injector, private config: RouterConfig) {
+      private location: Location, private injector: Injector, config: RouterConfig) {
+    this.resetConfig(config);
     this.routerEvents = new Subject<Event>();
     this.currentUrlTree = createEmptyUrlTree();
     this.currentRouterState = createEmptyState(this.currentUrlTree, this.rootComponentType);
@@ -149,7 +151,10 @@ export class Router {
    * ]);
    * ```
    */
-  resetConfig(config: RouterConfig): void { this.config = config; }
+  resetConfig(config: RouterConfig): void {
+    validateConfig(config);
+    this.config = config;
+  }
 
   /**
    * @internal
diff --git a/modules/@angular/router/test/config.spec.ts b/modules/@angular/router/test/config.spec.ts
new file mode 100644
index 0000000000..9ead702d83
--- /dev/null
+++ b/modules/@angular/router/test/config.spec.ts
@@ -0,0 +1,50 @@
+import {validateConfig} from '../src/config';
+
+describe('config', () => {
+  describe("validateConfig", () => {
+    it("should not throw when no errors", () => {
+      validateConfig([
+        { path: '', redirectTo: 'b' },
+        { path: 'b', component: ComponentA }
+      ]);
+    });
+
+    it("should throw when redirectTo and children are used together", () => {
+      expect(() => {
+        validateConfig([
+          { path: 'a', redirectTo: 'b', children: [
+            {path: 'b', component: ComponentA}
+          ] }
+        ]);
+      }).toThrowError(`Invalid configuration of route 'a': redirectTo and children cannot be used together`);
+    });
+
+    it("should throw when component and redirectTo are used together", () => {
+      expect(() => {
+        validateConfig([
+              { path: 'a', component: ComponentA, redirectTo: 'b' }
+        ]);
+      }).toThrowError(`Invalid configuration of route 'a': redirectTo and component cannot be used together`);
+    });
+
+    it("should throw when path is missing", () => {
+      expect(() => {
+        validateConfig([
+              { component: '', redirectTo: 'b' }
+        ]);
+      }).toThrowError(`Invalid route configuration: routes must have path specified`);
+    });
+
+    it("should throw when path starts with a slash", () => {
+      expect(() => {
+        validateConfig([
+              { path: '/a', componenta: '', redirectTo: 'b' }
+        ]);
+      }).toThrowError(`Invalid route configuration of route '/a': path cannot start with a slash`);
+    });
+  });
+});
+
+class ComponentA {}
+class ComponentB {}
+class ComponentC {}
diff --git a/modules/@angular/router/tsconfig.json b/modules/@angular/router/tsconfig.json
index 2bae4f5a11..168a8fa3f7 100644
--- a/modules/@angular/router/tsconfig.json
+++ b/modules/@angular/router/tsconfig.json
@@ -43,6 +43,7 @@
     "test/recognize.spec.ts",
     "test/create_router_state.spec.ts",
     "test/create_url_tree.spec.ts",
+    "test/config.spec.ts",
     "test/router.spec.ts",
     "typings/index.d.ts"
   ]