fix(router): correctly sort route matches with children by specificity

This changes the way we calculate specificity. Instead of using a number,
we use a string, so that combining specificity across parent-child instructions
becomes a matter of concatenating them

Fixes #5848

Closes #6011

modules/angular2/src/facade/lang.dart

@@ -11,6 +11,7 @@ class Math {
   static final _random = new math.Random();
   static int floor(num n) => n.floor();
   static double random() => _random.nextDouble();
+  static num min(num a, num b) => math.min(a, b);
 }
 
 class CONST {

modules/angular2/src/router/instruction.ts

@@ -115,8 +115,8 @@ export abstract class Instruction {
 
   get urlParams(): string[] { return isPresent(this.component) ? this.component.urlParams : []; }
 
-  get specificity(): number {
+  get specificity(): string {
-    var total = 0;
+    var total = '';
     if (isPresent(this.component)) {
       total += this.component.specificity;
     }
@@ -305,7 +305,7 @@ export class ComponentInstruction {
   public routeData: RouteData;
 
   constructor(public urlPath: string, public urlParams: string[], data: RouteData,
-              public componentType, public terminal: boolean, public specificity: number,
+              public componentType, public terminal: boolean, public specificity: string,
               public params: {[key: string]: any} = null) {
     this.routeData = isPresent(data) ? data : BLANK_ROUTE_DATA;
   }

modules/angular2/src/router/path_recognizer.ts

@@ -96,21 +96,22 @@ function parsePathString(route: string): {[key: string]: any} {
 
   var segments = splitBySlash(route);
   var results = [];
-  var specificity = 0;
+
+  var specificity = '';
+
+  // a single slash (or "empty segment" is as specific as a static segment
+  if (segments.length == 0) {
+    specificity += '2';
+  }
 
   // The "specificity" of a path is used to determine which route is used when multiple routes match
-  // a URL.
+  // a URL. Static segments (like "/foo") are the most specific, followed by dynamic segments (like
-  // Static segments (like "/foo") are the most specific, followed by dynamic segments (like
+  // "/:id"). Star segments add no specificity. Segments at the start of the path are more specific
-  // "/:id"). Star segments
+  // than proceeding ones.
-  // add no specificity. Segments at the start of the path are more specific than proceeding ones.
+  //
   // The code below uses place values to combine the different types of segments into a single
-  // integer that we can
+  // string that we can sort later. Each static segment is marked as a specificity of "2," each
-  // sort later. Each static segment is worth hundreds of points of specificity (10000, 9900, ...,
+  // dynamic segment is worth "1" specificity, and stars are worth "0" specificity.
-  // 200), and each
-  // dynamic segment is worth single points of specificity (100, 99, ... 2).
-  if (segments.length > 98) {
-    throw new BaseException(`'${route}' has more than the maximum supported number of segments.`);
-  }
 
   var limit = segments.length - 1;
   for (var i = 0; i <= limit; i++) {
@@ -118,9 +119,10 @@ function parsePathString(route: string): {[key: string]: any} {
 
     if (isPresent(match = RegExpWrapper.firstMatch(paramMatcher, segment))) {
       results.push(new DynamicSegment(match[1]));
-      specificity += (100 - i);
+      specificity += '1';
     } else if (isPresent(match = RegExpWrapper.firstMatch(wildcardMatcher, segment))) {
       results.push(new StarSegment(match[1]));
+      specificity += '0';
     } else if (segment == '...') {
       if (i < limit) {
         throw new BaseException(`Unexpected "..." before the end of the path for "${route}".`);
@@ -128,13 +130,11 @@ function parsePathString(route: string): {[key: string]: any} {
       results.push(new ContinuationSegment());
     } else {
       results.push(new StaticSegment(segment));
-      specificity += 100 * (100 - i);
+      specificity += '2';
     }
   }
-  var result = StringMapWrapper.create();
+
-  StringMapWrapper.set(result, 'segments', results);
+  return {'segments': results, 'specificity': specificity};
-  StringMapWrapper.set(result, 'specificity', specificity);
-  return result;
 }
 
 // this function is used to determine whether a route config path like `/foo/:id` collides with
@@ -177,7 +177,7 @@ function assertPath(path: string) {
  */
 export class PathRecognizer {
   private _segments: Segment[];
-  specificity: number;
+  specificity: string;
   terminal: boolean = true;
   hash: string;
 

modules/angular2/src/router/route_recognizer.ts

@@ -59,7 +59,7 @@ export class RedirectRecognizer implements AbstractRecognizer {
 
 // represents something like '/foo/:bar'
 export class RouteRecognizer implements AbstractRecognizer {
-  specificity: number;
+  specificity: string;
   terminal: boolean = true;
   hash: string;
 

modules/angular2/src/router/route_registry.ts

@@ -8,6 +8,8 @@ import {
   isString,
   isStringMap,
   Type,
+  StringWrapper,
+  Math,
   getTypeNameForDebugging,
   CONST_EXPR
 } from 'angular2/src/facade/lang';
@@ -492,7 +494,39 @@ function splitAndFlattenLinkParams(linkParams: any[]): any[] {
  * Given a list of instructions, returns the most specific instruction
  */
 function mostSpecific(instructions: Instruction[]): Instruction {
-  return ListWrapper.maximum(instructions, (instruction: Instruction) => instruction.specificity);
+  instructions = instructions.filter((instruction) => isPresent(instruction));
+  if (instructions.length == 0) {
+    return null;
+  }
+  if (instructions.length == 1) {
+    return instructions[0];
+  }
+  var first = instructions[0];
+  var rest = instructions.slice(1);
+  return rest.reduce((instruction: Instruction, contender: Instruction) => {
+    if (compareSpecificityStrings(contender.specificity, instruction.specificity) == -1) {
+      return contender;
+    }
+    return instruction;
+  }, first);
+}
+
+/*
+ * Expects strings to be in the form of "[0-2]+"
+ * Returns -1 if string A should be sorted above string B, 1 if it should be sorted after,
+ * or 0 if they are the same.
+ */
+function compareSpecificityStrings(a: string, b: string): number {
+  var l = Math.min(a.length, b.length);
+  for (var i = 0; i < l; i += 1) {
+    var ai = StringWrapper.charCodeAt(a, i);
+    var bi = StringWrapper.charCodeAt(b, i);
+    var difference = bi - ai;
+    if (difference != 0) {
+      return difference;
+    }
+  }
+  return a.length - b.length;
 }
 
 function assertTerminalComponent(component, path) {

modules/angular2/test/router/route_registry_spec.ts

@@ -181,6 +181,21 @@ export function main() {
              });
        }));
 
+    it('should prefer routes with high specificity over routes with children with lower specificity',
+       inject([AsyncTestCompleter], (async) => {
+         registry.config(RootHostCmp, new Route({path: '/first', component: DummyCmpA}));
+
+         // terminates to DummyCmpB
+         registry.config(RootHostCmp,
+                         new Route({path: '/:second/...', component: SingleSlashChildCmp}));
+
+         registry.recognize('/first', [])
+             .then((instruction) => {
+               expect(instruction.component.componentType).toBe(DummyCmpA);
+               async.done();
+             });
+       }));
+
     it('should match the full URL using child components', inject([AsyncTestCompleter], (async) => {
          registry.config(RootHostCmp, new Route({path: '/first/...', component: DummyParentCmp}));
 
@@ -322,6 +337,10 @@ class DummyCmpB {}
 class DefaultRouteCmp {
 }
 
+@RouteConfig([new Route({path: '/', component: DummyCmpB, name: 'ThirdCmp'})])
+class SingleSlashChildCmp {
+}
+
 
 @RouteConfig([
   new Route(

modules/angular2/test/router/router_link_spec.ts

@@ -33,8 +33,8 @@ import {
 import {DOM} from 'angular2/src/platform/dom/dom_adapter';
 import {ResolvedInstruction} from 'angular2/src/router/instruction';
 
-let dummyInstruction =
+let dummyInstruction = new ResolvedInstruction(
-    new ResolvedInstruction(new ComponentInstruction('detail', [], null, null, true, 0), null, {});
+    new ComponentInstruction('detail', [], null, null, true, '0'), null, {});
 
 export function main() {
   describe('routerLink directive', function() {