From 07b3029f4d409cf955780113df92e36401b47580 Mon Sep 17 00:00:00 2001
From: Nate Abele <nate.abele@gmail.com>
Date: Tue, 15 Apr 2014 22:29:39 -0400
Subject: [PATCH] feat(UrlMatcher): add per-param config support

Implements optional parameters and default parameter values. [BC-BREAK]: the `params` option in state configurations must now be an object keyed by parameter name.
---
 src/common.js                 |  5 +-
 src/state.js                  | 42 ++++++++--------
 src/urlMatcherFactory.js      | 90 ++++++++++++++++++++++-------------
 test/stateSpec.js             | 14 +++---
 test/urlMatcherFactorySpec.js | 66 +++++++++++++++++++++++--
 5 files changed, 149 insertions(+), 68 deletions(-)

diff --git a/src/common.js b/src/common.js
index 7e2209cb8..bd2b97a5e 100644
--- a/src/common.js
+++ b/src/common.js
@@ -96,8 +96,9 @@ function inheritParams(currentParams, newParams, $current, $to) {
   var parents = ancestors($current, $to), parentParams, inherited = {}, inheritList = [];
 
   for (var i in parents) {
-    if (!parents[i].params || !parents[i].params.length) continue;
-    parentParams = parents[i].params;
+    if (!parents[i].params) continue;
+    parentParams = keys(parents[i].params);
+    if (!parentParams.length) continue;
 
     for (var j in parentParams) {
       if (arraySearch(inheritList, parentParams[j]) >= 0) continue;
diff --git a/src/state.js b/src/state.js
index abf794439..88d066824 100644
--- a/src/state.js
+++ b/src/state.js
@@ -48,18 +48,14 @@ function $StateProvider(   $urlRouterProvider,   $urlMatcherFactory) {
 
     // Build a URLMatcher if necessary, either via a relative or absolute URL
     url: function(state) {
-      var url = state.url;
+      var url = state.url, config = { params: state.params || {} };
 
       if (isString(url)) {
-        if (url.charAt(0) == '^') {
-          return $urlMatcherFactory.compile(url.substring(1));
-        }
-        return (state.parent.navigable || root).url.concat(url);
+        if (url.charAt(0) == '^') return $urlMatcherFactory.compile(url.substring(1), config);
+        return (state.parent.navigable || root).url.concat(url, config);
       }
 
-      if ($urlMatcherFactory.isMatcher(url) || url == null) {
-        return url;
-      }
+      if (!url || $urlMatcherFactory.isMatcher(url)) return url;
       throw new Error("Invalid url '" + url + "' in state '" + state + "'");
     },
 
@@ -71,10 +67,8 @@ function $StateProvider(   $urlRouterProvider,   $urlMatcherFactory) {
     // Derive parameters for this state and ensure they're a super-set of parent's parameters
     params: function(state) {
       if (!state.params) {
-        return state.url ? state.url.parameters() : state.parent.params;
+        return state.url ? state.url.params : state.parent.params;
       }
-      if (!isArray(state.params)) throw new Error("Invalid params in state '" + state + "'");
-      if (state.url) throw new Error("Both params and url specicified in state '" + state + "'");
       return state.params;
     },
 
@@ -94,16 +88,18 @@ function $StateProvider(   $urlRouterProvider,   $urlMatcherFactory) {
     },
 
     ownParams: function(state) {
+      state.params = state.params || {};
+
       if (!state.parent) {
-        return state.params;
+          return keys(state.params);
       }
-      var paramNames = {}; forEach(state.params, function (p) { paramNames[p] = true; });
+      var paramNames = {}; forEach(state.params, function (v, k) { paramNames[k] = true; });
 
-      forEach(state.parent.params, function (p) {
-        if (!paramNames[p]) {
-          throw new Error("Missing required parameter '" + p + "' in state '" + state.name + "'");
+      forEach(state.parent.params, function (v, k) {
+        if (!paramNames[k]) {
+          throw new Error("Missing required parameter '" + k + "' in state '" + state.name + "'");
         }
-        paramNames[p] = false;
+        paramNames[k] = false;
       });
       var ownParams = [];
 
@@ -782,8 +778,8 @@ function $StateProvider(   $urlRouterProvider,   $urlMatcherFactory) {
         toState = findState(to, options.relative);
 
         if (!isDefined(toState)) {
-          if (options.relative) throw new Error("Could not resolve '" + to + "' from state '" + options.relative + "'");
-          throw new Error("No such state '" + to + "'");
+          if (!options.relative) throw new Error("No such state '" + to + "'");
+          throw new Error("Could not resolve '" + to + "' from state '" + options.relative + "'");
         }
       }
       if (toState[abstractKey]) throw new Error("Cannot transition to abstract state '" + to + "'");
@@ -808,14 +804,14 @@ function $StateProvider(   $urlRouterProvider,   $urlMatcherFactory) {
       // TODO: We may not want to bump 'transition' if we're called from a location change
       // that we've initiated ourselves, because we might accidentally abort a legitimate
       // transition initiated from code?
-      if (shouldTriggerReload(to, from, locals, options) ) {
+      if (shouldTriggerReload(to, from, locals, options)) {
         if (to.self.reloadOnSearch !== false) $urlRouter.update();
         $state.transition = null;
         return $q.when($state.current);
       }
 
       // Filter parameters before we pass them to event handlers etc.
-      toParams = filterByKeys(to.params, toParams || {});
+      toParams = filterByKeys(keys(to.params), toParams || {});
 
       // Broadcast start event and cancel the transition if requested
       if (options.notify) {
@@ -1102,7 +1098,7 @@ function $StateProvider(   $urlRouterProvider,   $urlMatcherFactory) {
       if (!nav || !nav.url) {
         return null;
       }
-      return $urlRouter.href(nav.url, filterByKeys(state.params, params || {}), { absolute: options.absolute });
+      return $urlRouter.href(nav.url, filterByKeys(keys(state.params), params || {}), { absolute: options.absolute });
     };
 
     /**
@@ -1132,7 +1128,7 @@ function $StateProvider(   $urlRouterProvider,   $urlMatcherFactory) {
       // necessary. In addition to being available to the controller and onEnter/onExit callbacks,
       // we also need $stateParams to be available for any $injector calls we make during the
       // dependency resolution process.
-      var $stateParams = (paramsAreFiltered) ? params : filterByKeys(state.params, params);
+      var $stateParams = (paramsAreFiltered) ? params : filterByKeys(keys(state.params), params);
       var locals = { $stateParams: $stateParams };
 
       // Resolve 'global' dependencies for the state, i.e. those not specific to a view.
diff --git a/src/urlMatcherFactory.js b/src/urlMatcherFactory.js
index da17fd2c8..d02fe506b 100644
--- a/src/urlMatcherFactory.js
+++ b/src/urlMatcherFactory.js
@@ -38,8 +38,11 @@
  *   path into the parameter 'path'.
  * * `'/files/*path'` - ditto.
  *
- * @param {string} pattern  the pattern to compile into a matcher.
- * @param {bool} caseInsensitiveMatch true if url matching should be case insensitive, otherwise false, the default value (for backward compatibility) is false.
+ * @param {string} pattern  The pattern to compile into a matcher.
+ * @param {config} config  A configuration object hash:
+ *
+ * * `caseInsensitive` - `true` if URL matching should be case insensitive, otherwise `false`, the default value (for backward compatibility) is `false`.
+ * * `strict` - `false` if matching against a URL with a trailing slash should be treated as equivalent to a URL without a trailing slash, the default value is `true`.
  *
  * @property {string} prefix  A static prefix of this pattern. The matcher guarantees that any
  *   URL matching this matcher (i.e. any string for which {@link ui.router.util.type:UrlMatcher#methods_exec exec()} returns
@@ -54,9 +57,10 @@
  * @property {string} regex  The constructed regex that will be used to match against the url when 
  *   it is time to determine which url will match.
  *
- * @returns {Object}  New UrlMatcher object
+ * @returns {Object}  New `UrlMatcher` object
  */
-function UrlMatcher(pattern, caseInsensitiveMatch) {
+function UrlMatcher(pattern, config) {
+  config = angular.isObject(config) ? config : {};
 
   // Find all placeholders and create a compiled pattern, using either classic or curly syntax:
   //   '*' name
@@ -76,32 +80,41 @@ function UrlMatcher(pattern, caseInsensitiveMatch) {
       segments = this.segments = [],
       params = this.params = {};
 
-  function addParameter(id, type) {
+  function addParameter(id, type, config) {
     if (!/^\w+(-+\w+)*$/.test(id)) throw new Error("Invalid parameter name '" + id + "' in pattern '" + pattern + "'");
     if (params[id]) throw new Error("Duplicate parameter name '" + id + "' in pattern '" + pattern + "'");
-    params[id] = angular.isNumber(type) ? new Type() : type;
+    params[id] = extend({ type: type || new Type() }, config);
+  }
+
+  function quoteRegExp(string, pattern, isOptional) {
+    var result = string.replace(/[\\\[\]\^$*+?.()|{}]/g, "\\$&");
+    if (!pattern) return result;
+    var flag = isOptional ? '?' : '';
+    return result + flag + '(' + pattern + ')' + flag;
   }
 
-  function quoteRegExp(string) {
-    return string.replace(/[\\\[\]\^$*+?.()|{}]/g, "\\$&");
+  function paramConfig(param) {
+    if (!config.params || !config.params[param]) return {};
+    return config.params[param];
   }
 
   this.source = pattern;
 
   // Split into static segments separated by path parameter placeholders.
   // The number of segments is always 1 more than the number of parameters.
-  var id, regexp, segment, type;
+  var id, regexp, segment, type, cfg;
 
   while ((m = placeholder.exec(pattern))) {
     id      = m[2] || m[3]; // IE[78] returns '' for unmatched groups instead of null
     regexp  = m[4] || (m[1] == '*' ? '.*' : '[^/]*');
     segment = pattern.substring(last, m.index);
     type    = this.$types[regexp] || new Type({ pattern: new RegExp(regexp) });
+    cfg     = paramConfig(id);
 
     if (segment.indexOf('?') >= 0) break; // we're into the search part
 
-    compiled += quoteRegExp(segment) + '(' + type.$subPattern() + ')';
-    addParameter(id, type);
+    compiled += quoteRegExp(segment, type.$subPattern(), isDefined(cfg.value));
+    addParameter(id, type, cfg);
     segments.push(segment);
     last = placeholder.lastIndex;
   }
@@ -116,7 +129,9 @@ function UrlMatcher(pattern, caseInsensitiveMatch) {
     this.sourcePath = pattern.substring(0, last + i);
 
     // Allow parameters to be separated by '?' as well as '&' to make concat() easier
-    forEach(search.substring(1).split(/[&?]/), addParameter);
+    forEach(search.substring(1).split(/[&?]/), function(key) {
+      addParameter(key, null, paramConfig(key));
+    });
   } else {
     this.sourcePath = pattern;
     this.sourceSearch = '';
@@ -125,7 +140,7 @@ function UrlMatcher(pattern, caseInsensitiveMatch) {
   compiled += quoteRegExp(segment) + '$';
   segments.push(segment);
 
-  this.regexp = (caseInsensitiveMatch) ? new RegExp(compiled, 'i') : new RegExp(compiled);
+  this.regexp = RegExp(compiled, config.caseInsensitive ? 'i' : undefined);
   this.prefix = segments[0];
 }
 
@@ -148,13 +163,14 @@ function UrlMatcher(pattern, caseInsensitiveMatch) {
  * ```
  *
  * @param {string} pattern  The pattern to append.
+ * @param {object} config  An object hash of the configuration for the matcher.
  * @returns {ui.router.util.type:UrlMatcher}  A matcher for the concatenated pattern.
  */
-UrlMatcher.prototype.concat = function (pattern) {
+UrlMatcher.prototype.concat = function (pattern, config) {
   // Because order of search parameters is irrelevant, we can add our own search
   // parameters to the end of the new pattern. Parse the new pattern by itself
   // and then join the bits together, but it's much easier to do this on a string level.
-  return new UrlMatcher(this.sourcePath + pattern + this.sourceSearch);
+  return new UrlMatcher(this.sourcePath + pattern + this.sourceSearch, config);
 };
 
 UrlMatcher.prototype.toString = function () {
@@ -189,14 +205,14 @@ UrlMatcher.prototype.exec = function (path, searchParams) {
 
   var params = this.parameters(), nTotal = params.length,
     nPath = this.segments.length - 1,
-    values = {}, i, type, param;
+    values = {}, i, cfg, param;
 
   if (nPath !== m.length - 1) throw new Error("Unbalanced capture group in route '" + this.source + "'");
 
   for (i = 0; i < nPath; i++) {
     param = params[i];
-    type = this.params[param];
-    values[param] = type.decode(m[i + 1]);
+    cfg = this.params[param];
+    values[param] = cfg.type.decode(isDefined(m[i + 1]) ? m[i + 1] : cfg.value);
   }
   for (/**/; i < nTotal; i++) values[params[i]] = searchParams[params[i]];
 
@@ -214,8 +230,9 @@ UrlMatcher.prototype.exec = function (path, searchParams) {
  * @returns {Array.<string>}  An array of parameter names. Must be treated as read-only. If the
  *    pattern has no parameters, an empty array is returned.
  */
-UrlMatcher.prototype.parameters = function () {
-  return keys(this.params);
+UrlMatcher.prototype.parameters = function (param) {
+  if (!isDefined(param)) return keys(this.params);
+  return this.params[param] || null;
 };
 
 /**
@@ -231,11 +248,13 @@ UrlMatcher.prototype.parameters = function () {
  * @returns {Boolean} Returns `true` if `params` validates, otherwise `false`.
  */
 UrlMatcher.prototype.validates = function (params) {
-  var result = true, self = this;
+  var result = true, isOptional, cfg, self = this;
 
   forEach(params, function(val, key) {
     if (!self.params[key]) return;
-    result = result && self.params[key].is(val);
+    cfg = self.params[key];
+    isOptional = !val && isDefined(cfg.value);
+    result = result && (isOptional || cfg.type.is(val));
   });
   return result;
 };
@@ -261,18 +280,21 @@ UrlMatcher.prototype.validates = function (params) {
  */
 UrlMatcher.prototype.format = function (values) {
   var segments = this.segments, params = this.parameters();
-  if (!values) return segments.join('');
+
+  if (!values) return segments.join('').replace('//', '/');
 
   var nPath = segments.length - 1, nTotal = params.length,
-    result = segments[0], i, search, value, param, type;
+    result = segments[0], i, search, value, param, cfg;
+
+  if (!this.validates(values)) return null;
 
   for (i = 0; i < nPath; i++) {
     param = params[i];
     value = values[param];
-    type  = this.params[param];
+    cfg   = this.params[param];
 
-    if (!type.is(value)) return null;
-    if (value != null) result += encodeURIComponent(type.encode(value));
+    if (!isDefined(value) && (segments[i] === '/' || segments[i + 1] === '/')) continue;
+    if (value != null) result += encodeURIComponent(cfg.type.encode(value));
     result += segments[i + 1];
   }
 
@@ -324,7 +346,7 @@ Type.prototype.pattern = /.*/;
  */
 function $UrlMatcherFactory() {
 
-  var useCaseInsensitiveMatch = false;
+  var isCaseInsensitive = false;
 
   var enqueue = true, typeQueue = [], injector, defaultTypes = {
     int: {
@@ -332,6 +354,7 @@ function $UrlMatcherFactory() {
         return parseInt(val, 10);
       },
       is: function(val) {
+        if (!isDefined(val)) return false;
         return this.decode(val.toString()) === val;
       },
       pattern: /\d+/
@@ -371,7 +394,7 @@ function $UrlMatcherFactory() {
 
   /**
    * @ngdoc function
-   * @name ui.router.util.$urlMatcherFactory#caseInsensitiveMatch
+   * @name ui.router.util.$urlMatcherFactory#caseInsensitive
    * @methodOf ui.router.util.$urlMatcherFactory
    *
    * @description
@@ -379,8 +402,8 @@ function $UrlMatcherFactory() {
    *   
    * @param {bool} value false to match URL in a case sensitive manner; otherwise true;
    */
-  this.caseInsensitiveMatch = function(value) {
-    useCaseInsensitiveMatch = value;
+  this.caseInsensitive = function(value) {
+    isCaseInsensitive = value;
   };
 
   /**
@@ -392,10 +415,11 @@ function $UrlMatcherFactory() {
    * Creates a {@link ui.router.util.type:UrlMatcher} for the specified pattern.
    *   
    * @param {string} pattern  The URL pattern.
+   * @param {object} config  The config object hash.
    * @returns {ui.router.util.type:UrlMatcher}  The UrlMatcher.
    */
-  this.compile = function (pattern) {
-    return new UrlMatcher(pattern, useCaseInsensitiveMatch);
+  this.compile = function (pattern, config) {
+    return new UrlMatcher(pattern, extend({ caseInsensitive: isCaseInsensitive }, config));
   };
 
   /**
diff --git a/test/stateSpec.js b/test/stateSpec.js
index 9fa432c9c..3d504f538 100644
--- a/test/stateSpec.js
+++ b/test/stateSpec.js
@@ -20,9 +20,9 @@ describe('state', function () {
   var A = { data: {} },
       B = {},
       C = {},
-      D = { params: [ 'x', 'y' ] },
-      DD = { parent: D, params: [ 'x', 'y', 'z' ] },
-      E = { params: [ 'i' ] },
+      D = { params: { x: {}, y: {} } },
+      DD = { parent: D, params: { x: {}, y: {}, z: {} } },
+      E = { params: { i: {} } },
       H = { data: {propA: 'propA', propB: 'propB'} },
       HH = { parent: H },
       HHH = {parent: HH, data: {propA: 'overriddenA', propC: 'propC'} },
@@ -261,7 +261,7 @@ describe('state', function () {
       initStateTo(DD, { x: 1, y: 2, z: 3 });
       var called;
       $rootScope.$on('$stateNotFound', function (ev, redirect) {
-        stateProvider.state(redirect.to, { parent: DD, params: [ 'x', 'y', 'z', 'w' ]});
+        stateProvider.state(redirect.to, { parent: DD, params: { x: {}, y: {}, z: {}, w: {} }});
         called = true;
       });
       var promise = $state.go('DDD', { w: 4 });
@@ -280,7 +280,7 @@ describe('state', function () {
         called = true;
       });
       var promise = $state.go('AA', { a: 1 });
-      stateProvider.state('AA', { parent: A, params: [ 'a' ]});
+      stateProvider.state('AA', { parent: A, params: { a: {} }});
       deferred.resolve();
       $q.flush();
       expect(called).toBeTruthy();
@@ -298,7 +298,7 @@ describe('state', function () {
       });
       var promise = $state.go('AA', { a: 1 });
       $state.go(B);
-      stateProvider.state('AA', { parent: A, params: [ 'a' ]});
+      stateProvider.state('AA', { parent: A, params: { a: {} }});
       deferred.resolve();
       $q.flush();
       expect(called).toBeTruthy();
@@ -439,7 +439,7 @@ describe('state', function () {
 
   describe('.go()', function () {
     it('transitions to a relative state', inject(function ($state, $q) {
-      $state.transitionTo('about.person.item', { id: 5 }); $q.flush();
+      $state.transitionTo('about.person.item', { person: "bob", id: 5 }); $q.flush();
       $state.go('^.^.sidebar'); $q.flush();
       expect($state.$current.name).toBe('about.sidebar');
 
diff --git a/test/urlMatcherFactorySpec.js b/test/urlMatcherFactorySpec.js
index b5c04efbf..0654d98fc 100644
--- a/test/urlMatcherFactorySpec.js
+++ b/test/urlMatcherFactorySpec.js
@@ -5,7 +5,7 @@ describe("UrlMatcher", function () {
   });
 
   it("should match static case insensitive URLs", function () {
-    expect(new UrlMatcher('/hello/world', true).exec('/heLLo/World')).toEqual({});
+    expect(new UrlMatcher('/hello/world', { caseInsensitive: true }).exec('/heLLo/World')).toEqual({});
   });
 
   it("should match against the entire path", function () {
@@ -155,7 +155,7 @@ describe("urlMatcherFactory", function () {
   });
 
   it("should handle case insensistive URL", function () {
-    $umf.caseInsensitiveMatch(true);
+    $umf.caseInsensitive(true);
     expect($umf.compile('/hello/world').exec('/heLLo/WORLD')).toEqual({});
   });
 
@@ -197,7 +197,7 @@ describe("urlMatcherFactory", function () {
       expect(m.format({ date: new Date(2014, 2, 26) })).toBe("/calendar/2014-03-26");
     });
 
-    it("should not match invalid typed parameter values", function () {
+    it("should not match invalid typed parameter values", function() {
       var m = new UrlMatcher('/users/{id:int}');
 
       expect(m.exec('/users/1138').id).toBe(1138);
@@ -207,4 +207,64 @@ describe("urlMatcherFactory", function () {
       expect(m.format({ id: "alpha" })).toBeNull();
     });
   });
+
+  describe("optional parameters", function() {
+    it("should match with or without values", function () {
+      var m = new UrlMatcher('/users/{id:int}', {
+        params: { id: { value: null } }
+      });
+      expect(m.exec('/users/1138')).toEqual({ id: 1138 });
+      expect(m.exec('/users/').id.toString()).toBe("NaN");
+      expect(m.exec('/users').id.toString()).toBe("NaN");
+    });
+
+    it("should correctly match multiple", function() {
+      var m = new UrlMatcher('/users/{id:int}/{state:[A-Z]+}', {
+        params: { id: { value: null }, state: { value: null } }
+      });
+      expect(m.exec('/users/1138')).toEqual({ id: 1138, state: null });
+      expect(m.exec('/users/1138/NY')).toEqual({ id: 1138, state: "NY" });
+
+      expect(m.exec('/users/').id.toString()).toBe("NaN");
+      expect(m.exec('/users/').state).toBeNull();
+
+      expect(m.exec('/users').id.toString()).toBe("NaN");
+      expect(m.exec('/users').state).toBeNull();
+
+      expect(m.exec('/users/NY').state).toBe("NY");
+      expect(m.exec('/users/NY').id.toString()).toBe("NaN");
+    });
+
+    it("should correctly format with or without values", function() {
+      var m = new UrlMatcher('/users/{id:int}', {
+        params: { id: { value: null } }
+      });
+      expect(m.format()).toBe('/users/');
+      expect(m.format({ id: 1138 })).toBe('/users/1138');
+    });
+
+    it("should correctly format multiple", function() {
+      var m = new UrlMatcher('/users/{id:int}/{state:[A-Z]+}', {
+        params: { id: { value: null }, state: { value: null } }
+      });
+
+      expect(m.format()).toBe("/users/");
+      expect(m.format({ id: 1138 })).toBe("/users/1138/");
+      expect(m.format({ state: "NY" })).toBe("/users/NY");
+      expect(m.format({ id: 1138, state: "NY" })).toBe("/users/1138/NY");
+    });
+
+    describe("default values", function() {
+      it("should populate if not supplied in URL", function() {
+        var m = new UrlMatcher('/users/{id:int}/{test}', {
+          params: { id: { value: 0 }, test: { value: "foo" } }
+        });
+        expect(m.exec('/users')).toEqual({ id: 0, test: "foo" });
+        expect(m.exec('/users/2')).toEqual({ id: 2, test: "foo" });
+        expect(m.exec('/users/bar')).toEqual({ id: 0, test: "bar" });
+        expect(m.exec('/users/2/bar')).toEqual({ id: 2, test: "bar" });
+        expect(m.exec('/users/bar/2')).toBeNull();
+      });
+    });
+  });
 });