fix(router): preserve specificity for redirects

btford · btford · commit a038bb9ae3a3 · 2016-01-04T20:06:21.000Z
Previously when comparing which of multiple possible routes to choose in an ambiguous case, we looked at the specificity of the target of redirect matches rather than the original match. This meant that if a redirect used a whilecard, but redirected to a target that was a static path, we'd cound the static path's specificity instead of the wildcard. This change stores the specificity of the redirect on the RedirectInstruction. Closes angular#5933
diff --git a/modules/angular2/src/router/instruction.ts b/modules/angular2/src/router/instruction.ts
@@ -281,9 +281,11 @@ export class UnresolvedInstruction extends Instruction {
 
 export class RedirectInstruction extends ResolvedInstruction {
   constructor(component: ComponentInstruction, child: Instruction,
-              auxInstruction: {[key: string]: Instruction}) {
+              auxInstruction: {[key: string]: Instruction}, private _specificity: string) {
     super(component, child, auxInstruction);
   }
+
+  get specificity(): string { return this._specificity; }
 }
 
 
diff --git a/modules/angular2/src/router/route_registry.ts b/modules/angular2/src/router/route_registry.ts
@@ -205,7 +205,7 @@ export class RouteRegistry {
             var instruction =
                 this.generate(candidate.redirectTo, ancestorInstructions.concat([null]));
             return new RedirectInstruction(instruction.component, instruction.child,
-                                           instruction.auxInstruction);
+                                           instruction.auxInstruction, candidate.specificity);
           }
         }));
 
diff --git a/modules/angular2/test/router/integration/impl/fixture_components.ts b/modules/angular2/test/router/integration/impl/fixture_components.ts
@@ -10,6 +10,12 @@ import {
 } from 'angular2/router';
 import {PromiseWrapper} from 'angular2/src/facade/async';
 
+@Component({selector: 'goodbye-cmp', template: `{{farewell}}`})
+export class GoodbyeCmp {
+  farewell: string;
+  constructor() { this.farewell = 'goodbye'; }
+}
+
 @Component({selector: 'hello-cmp', template: `{{greeting}}`})
 export class HelloCmp {
   greeting: string;
diff --git a/modules/angular2/test/router/integration/redirect_route_spec.ts b/modules/angular2/test/router/integration/redirect_route_spec.ts
@@ -25,7 +25,7 @@ import {
 } from 'angular2/src/router/route_config_decorator';
 
 import {TEST_ROUTER_PROVIDERS, RootCmp, compile} from './util';
-import {HelloCmp, RedirectToParentCmp} from './impl/fixture_components';
+import {HelloCmp, GoodbyeCmp, RedirectToParentCmp} from './impl/fixture_components';
 
 var cmpInstanceCount;
 var childCmpInstanceCount;
@@ -117,5 +117,24 @@ export function main() {
                async.done();
              });
        }));
+
+
+    it('should not redirect when redirect is less specific than other matching routes',
+       inject([AsyncTestCompleter, Location], (async, location) => {
+         compile(tcb)
+             .then((rtc) => {rootTC = rtc})
+             .then((_) => rtr.config([
+               new Route({path: '/foo', component: HelloCmp, name: 'Hello'}),
+               new Route({path: '/:param', component: GoodbyeCmp, name: 'Goodbye'}),
+               new Redirect({path: '/*rest', redirectTo: ['/Hello']})
+             ]))
+             .then((_) => rtr.navigateByUrl('/bye'))
+             .then((_) => {
+               rootTC.detectChanges();
+               expect(rootTC.debugElement.nativeElement).toHaveText('goodbye');
+               expect(location.urlChanges).toEqual(['/bye']);
+               async.done();
+             });
+       }));
   });
 }

Original file line number	Diff line number	Diff line change
`@@ -281,9 +281,11 @@ export class UnresolvedInstruction extends Instruction {`
`281`	`281`
`282`	`282`	`export class RedirectInstruction extends ResolvedInstruction {`
`283`	`283`	`constructor(component: ComponentInstruction, child: Instruction,`
`284`		`- auxInstruction: {[key: string]: Instruction}) {`
	`284`	`+ auxInstruction: {[key: string]: Instruction}, private _specificity: string) {`
`285`	`285`	`super(component, child, auxInstruction);`
`286`	`286`	`}`
	`287`	`+`
	`288`	`+ get specificity(): string { return this._specificity; }`
`287`	`289`	`}`
`288`	`290`
`289`	`291`
Original file line number	Diff line number	Diff line change
`@@ -205,7 +205,7 @@ export class RouteRegistry {`
`205`	`205`	`var instruction =`
`206`	`206`	`this.generate(candidate.redirectTo, ancestorInstructions.concat([null]));`
`207`	`207`	`return new RedirectInstruction(instruction.component, instruction.child,`
`208`		`- instruction.auxInstruction);`
	`208`	`+ instruction.auxInstruction, candidate.specificity);`
`209`	`209`	`}`
`210`	`210`	`}));`
`211`	`211`