diff --git a/src/ngRoute/route.js b/src/ngRoute/route.js index 82c6cf3b6126..d1d7bba6528c 100644 --- a/src/ngRoute/route.js +++ b/src/ngRoute/route.js @@ -35,11 +35,13 @@ function $RouteProvider(){ * * * `path` can contain named groups starting with a colon (`:name`). All characters up * to the next slash are matched and stored in `$routeParams` under the given `name` - * after the route is resolved. - * * `path` can contain named groups starting with a star (`*name`). All characters are - * eagerly stored in `$routeParams` under the given `name` after the route is resolved. + * when the route matches. + * * `path` can contain named groups starting with a colon and ending with a star (`:name*`). + * All characters are eagerly stored in `$routeParams` under the given `name` + * when the route matches. + * * `path` can contain optional named groups with a question mark (`:name?`). * - * For example, routes like `/color/:color/largecode/*largecode/edit` will match + * For example, routes like `/color/:color/largecode/:largecode*\/edit` will match * `/color/brown/largecode/code/with/slashs/edit` and extract: * * * `color: brown` @@ -117,7 +119,11 @@ function $RouteProvider(){ * Adds a new route definition to the `$route` service. */ this.when = function(path, route) { - routes[path] = extend({reloadOnSearch: true, caseInsensitiveMatch: false}, route); + routes[path] = extend( + {reloadOnSearch: true}, + route, + path && pathRegExp(path, route) + ); // create redirection for trailing slashes if (path) { @@ -125,12 +131,54 @@ function $RouteProvider(){ ? path.substr(0, path.length-1) : path +'/'; - routes[redirectPath] = {redirectTo: path}; + routes[redirectPath] = extend( + {redirectTo: path}, + pathRegExp(redirectPath, route) + ); } return this; }; + /** + * @param path {string} path + * @param opts {Object} options + * @return {?Object} + * + * @description + * Normalizes the given path, returning a regular expression + * and the original path. + * + * Inspired by pathRexp in visionmedia/express/lib/utils.js. + */ + function pathRegExp(path, opts) { + var insensitive = opts.caseInsensitiveMatch, + ret = { + originalPath: path, + regexp: path + }, + keys = ret.keys = []; + + path = path + .replace(/([().])/g, '\\$1') + .replace(/(\/)?:(\w+)([\?|\*])?/g, function(_, slash, key, option){ + var optional = option === '?' ? option : null; + var star = option === '*' ? option : null; + keys.push({ name: key, optional: !!optional }); + slash = slash || ''; + return '' + + (optional ? '' : slash) + + '(?:' + + (optional ? slash : '') + + (star && '(.+)?' || '([^/]+)?') + ')' + + (optional || ''); + }) + .replace(/([\/$\*])/g, '\\$1'); + + ret.regexp = new RegExp('^' + path + '$', insensitive ? 'i' : ''); + return ret; + } + /** * @ngdoc method * @name ngRoute.$routeProvider#otherwise @@ -362,50 +410,37 @@ function $RouteProvider(){ /** * @param on {string} current url - * @param when {string} route when template to match the url against - * @param whenProperties {Object} properties to define when's matching behavior + * @param route {Object} route regexp to match the url against * @return {?Object} + * + * @description + * Check if the route matches the current url. + * + * Inspired by match in + * visionmedia/express/lib/router/router.js. */ - function switchRouteMatcher(on, when, whenProperties) { - // TODO(i): this code is convoluted and inefficient, we should construct the route matching - // regex only once and then reuse it - - // Escape regexp special characters. - when = '^' + when.replace(/[-\/\\^$:*+?.()|[\]{}]/g, "\\$&") + '$'; - - var regex = '', - params = [], - dst = {}; - - var re = /\\([:*])(\w+)/g, - paramMatch, - lastMatchedIndex = 0; - - while ((paramMatch = re.exec(when)) !== null) { - // Find each :param in `when` and replace it with a capturing group. - // Append all other sections of when unchanged. - regex += when.slice(lastMatchedIndex, paramMatch.index); - switch(paramMatch[1]) { - case ':': - regex += '([^\\/]*)'; - break; - case '*': - regex += '(.*)'; - break; + function switchRouteMatcher(on, route) { + var keys = route.keys, + params = {}; + + if (!route.regexp) return null; + + var m = route.regexp.exec(on); + if (!m) return null; + + var N = 0; + for (var i = 1, len = m.length; i < len; ++i) { + var key = keys[i - 1]; + + var val = 'string' == typeof m[i] + ? decodeURIComponent(m[i]) + : m[i]; + + if (key && val) { + params[key.name] = val; } - params.push(paramMatch[2]); - lastMatchedIndex = re.lastIndex; - } - // Append trailing path part. - regex += when.substr(lastMatchedIndex); - - var match = on.match(new RegExp(regex, whenProperties.caseInsensitiveMatch ? 'i' : '')); - if (match) { - forEach(params, function(name, index) { - dst[name] = match[index + 1]; - }); } - return match ? dst : null; + return params; } function updateRoute() { @@ -489,7 +524,7 @@ function $RouteProvider(){ // Match a route var params, match; forEach(routes, function(route, path) { - if (!match && (params = switchRouteMatcher($location.path(), path, route))) { + if (!match && (params = switchRouteMatcher($location.path(), route))) { match = inherit(route, { params: extend({}, $location.search(), params), pathParams: params}); diff --git a/test/ngRoute/routeParamsSpec.js b/test/ngRoute/routeParamsSpec.js index 1391151cbc49..7c10a922302d 100644 --- a/test/ngRoute/routeParamsSpec.js +++ b/test/ngRoute/routeParamsSpec.js @@ -45,4 +45,37 @@ describe('$routeParams', function() { expect($routeParams).toEqual({barId: 'barvalue', fooId: 'foovalue'}); }); }); + + it('should correctly extract the params when an optional param name is part of the route', function() { + module(function($routeProvider) { + $routeProvider.when('/bar/:foo?', {}); + $routeProvider.when('/baz/:foo?/edit', {}); + $routeProvider.when('/qux/:bar?/:baz?', {}); + }); + + inject(function($rootScope, $route, $location, $routeParams) { + $location.path('/bar'); + $rootScope.$digest(); + expect($routeParams).toEqual({}); + + $location.path('/bar/fooValue'); + $rootScope.$digest(); + expect($routeParams).toEqual({foo: 'fooValue'}); + + $location.path('/baz/fooValue/edit'); + $rootScope.$digest(); + expect($routeParams).toEqual({foo: 'fooValue'}); + + $location.path('/baz/edit'); + $rootScope.$digest(); + expect($routeParams).toEqual({}); + + $location.path('/qux//bazValue'); + $rootScope.$digest(); + expect($routeParams).toEqual({baz: 'bazValue', bar: undefined}); + + }); + }); + + }); diff --git a/test/ngRoute/routeSpec.js b/test/ngRoute/routeSpec.js index 29c2b7984167..0064c26c60c1 100644 --- a/test/ngRoute/routeSpec.js +++ b/test/ngRoute/routeSpec.js @@ -68,9 +68,9 @@ describe('$route', function() { nextRoute; module(function($routeProvider) { - $routeProvider.when('/Book1/:book/Chapter/:chapter/*highlight/edit', + $routeProvider.when('/Book1/:book/Chapter/:chapter/:highlight*/edit', {controller: noop, templateUrl: 'Chapter.html'}); - $routeProvider.when('/Book2/:book/*highlight/Chapter/:chapter', + $routeProvider.when('/Book2/:book/:highlight*/Chapter/:chapter', {controller: noop, templateUrl: 'Chapter.html'}); $routeProvider.when('/Blank', {}); }); @@ -127,9 +127,9 @@ describe('$route', function() { nextRoute; module(function($routeProvider) { - $routeProvider.when('/Book1/:book/Chapter/:chapter/*highlight/edit', + $routeProvider.when('/Book1/:book/Chapter/:chapter/:highlight*/edit', {controller: noop, templateUrl: 'Chapter.html', caseInsensitiveMatch: true}); - $routeProvider.when('/Book2/:book/*highlight/Chapter/:chapter', + $routeProvider.when('/Book2/:book/:highlight*/Chapter/:chapter', {controller: noop, templateUrl: 'Chapter.html'}); $routeProvider.when('/Blank', {}); }); @@ -245,6 +245,31 @@ describe('$route', function() { }); + describe('should match a route that contains optional params in the path', function() { + beforeEach(module(function($routeProvider) { + $routeProvider.when('/test/:opt?/:baz/edit', {templateUrl: 'test.html'}); + })); + + it('matches a URL with optional params', inject(function($route, $location, $rootScope) { + $location.path('/test/optValue/bazValue/edit'); + $rootScope.$digest(); + expect($route.current).toBeDefined(); + })); + + it('matches a URL without optional param', inject(function($route, $location, $rootScope) { + $location.path('/test//bazValue/edit'); + $rootScope.$digest(); + expect($route.current).toBeDefined(); + })); + + it('not match a URL with a required param', inject(function($route, $location, $rootScope) { + $location.path('///edit'); + $rootScope.$digest(); + expect($route.current).not.toBeDefined(); + })); + }); + + it('should change route even when only search param changes', function() { module(function($routeProvider) { $routeProvider.when('/test', {templateUrl: 'test.html'}); @@ -723,6 +748,8 @@ describe('$route', function() { module(function($routeProvider) { $routeProvider.when('/foo/:id/foo/:subid/:extraId', {redirectTo: '/bar/:id/:subid/23'}); $routeProvider.when('/bar/:id/:subid/:subsubid', {templateUrl: 'bar.html'}); + $routeProvider.when('/baz/:id/:path*', {redirectTo: '/path/:path/:id'}); + $routeProvider.when('/path/:path*/:id', {templateUrl: 'foo.html'}); }); inject(function($route, $location, $rootScope) { @@ -732,6 +759,11 @@ describe('$route', function() { expect($location.path()).toEqual('/bar/id1/subid3/23'); expect($location.search()).toEqual({extraId: 'gah'}); expect($route.current.templateUrl).toEqual('bar.html'); + + $location.path('/baz/1/foovalue/barvalue'); + $rootScope.$digest(); + expect($location.path()).toEqual('/path/foovalue/barvalue/1'); + expect($route.current.templateUrl).toEqual('foo.html'); }); });