Skip to content

Commit 82785e6

Browse files
thorn0thorn0
committed
fix(ngMock/$httpBackend): correctly ignore query params in {expect,when}Route
Fixes angular#14173
1 parent a7de87d commit 82785e6

File tree

5 files changed

+95
-89
lines changed

5 files changed

+95
-89
lines changed

Diff for: ‎angularFiles.js

+2
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ var angularFiles = {
130130
],
131131
'ngRoute': [
132132
'src/shallowCopy.js',
133+
'src/routeToRegExp.js',
133134
'src/ngRoute/route.js',
134135
'src/ngRoute/routeParams.js',
135136
'src/ngRoute/directive/ngView.js'
@@ -139,6 +140,7 @@ var angularFiles = {
139140
'src/ngSanitize/filter/linky.js'
140141
],
141142
'ngMock': [
143+
'src/routeToRegExp.js',
142144
'src/ngMock/angular-mocks.js',
143145
'src/ngMock/browserTrigger.js'
144146
],

Diff for: ‎src/ngMock/angular-mocks.js

+15-41
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
'use strict';
22

3+
/* global routeToRegExp: false */
4+
35
/**
46
* @ngdoc object
57
* @name angular.mock
@@ -1282,7 +1284,7 @@ angular.mock.dump = function(object) {
12821284
* ## Matching route requests
12831285
*
12841286
* For extra convenience, `whenRoute` and `expectRoute` shortcuts are available. These methods offer colon
1285-
* delimited matching of the url path, ignoring the query string. This allows declarations
1287+
* delimited matching of the url path, ignoring the query string and trailing slashes. This allows declarations
12861288
* similar to how application routes are configured with `$routeProvider`. Because these methods convert
12871289
* the definition url to regex, declaration order is important. Combined with query parameter parsing,
12881290
* the following is possible:
@@ -1481,8 +1483,9 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
14811483
* ```
14821484
* – The respond method takes a set of static data to be returned or a function that can
14831485
* return an array containing response status (number), response data (Array|Object|string),
1484-
* response headers (Object), and the text for the status (string). The respond method returns
1485-
* the `requestHandler` object for possible overrides.
1486+
* response headers (Object), HTTP status text (string), and XMLHttpRequest status (string:
1487+
* `complete`, `error`, `timeout` or `abort`). The respond method returns the `requestHandler`
1488+
* object for possible overrides.
14861489
*/
14871490
$httpBackend.when = function(method, url, data, headers, keys) {
14881491

@@ -1663,40 +1666,10 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
16631666
* See {@link ngMock.$httpBackend#when `when`} for more info.
16641667
*/
16651668
$httpBackend.whenRoute = function(method, url) {
1666-
var pathObj = parseRoute(url);
1669+
var pathObj = routeToRegExp(url, {caseInsensitiveMatch: true, ignoreTrailingSlashes: true});
16671670
return $httpBackend.when(method, pathObj.regexp, undefined, undefined, pathObj.keys);
16681671
};
16691672

1670-
function parseRoute(url) {
1671-
var ret = {
1672-
regexp: url
1673-
},
1674-
keys = ret.keys = [];
1675-
1676-
if (!url || !angular.isString(url)) return ret;
1677-
1678-
url = url
1679-
.replace(/([().])/g, '\\$1')
1680-
.replace(/(\/)?:(\w+)([?*])?/g, function(_, slash, key, option) {
1681-
var optional = option === '?' ? option : null;
1682-
var star = option === '*' ? option : null;
1683-
keys.push({ name: key, optional: !!optional });
1684-
slash = slash || '';
1685-
return ''
1686-
+ (optional ? '' : slash)
1687-
+ '(?:'
1688-
+ (optional ? slash : '')
1689-
+ (star && '(.+?)' || '([^/]+)')
1690-
+ (optional || '')
1691-
+ ')'
1692-
+ (optional || '');
1693-
})
1694-
.replace(/([/$*])/g, '\\$1');
1695-
1696-
ret.regexp = new RegExp('^' + url, 'i');
1697-
return ret;
1698-
}
1699-
17001673
/**
17011674
* @ngdoc method
17021675
* @name $httpBackend#expect
@@ -1717,14 +1690,15 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
17171690
* order to change how a matched request is handled.
17181691
*
17191692
* - respond –
1720-
* ```
1721-
* { function([status,] data[, headers, statusText])
1722-
* | function(function(method, url, data, headers, params)}
1723-
* ```
1693+
* ```js
1694+
* {function([status,] data[, headers, statusText])
1695+
* | function(function(method, url, data, headers, params)}
1696+
* ```
17241697
* – The respond method takes a set of static data to be returned or a function that can
17251698
* return an array containing response status (number), response data (Array|Object|string),
1726-
* response headers (Object), and the text for the status (string). The respond method returns
1727-
* the `requestHandler` object for possible overrides.
1699+
* response headers (Object), HTTP status text (string), and XMLHttpRequest status (string:
1700+
* `complete`, `error`, `timeout` or `abort`). The respond method returns the `requestHandler`
1701+
* object for possible overrides.
17281702
*/
17291703
$httpBackend.expect = function(method, url, data, headers, keys) {
17301704

@@ -1876,7 +1850,7 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
18761850
* See {@link ngMock.$httpBackend#expect `expect`} for more info.
18771851
*/
18781852
$httpBackend.expectRoute = function(method, url) {
1879-
var pathObj = parseRoute(url);
1853+
var pathObj = routeToRegExp(url, {caseInsensitiveMatch: true, ignoreTrailingSlashes: true});
18801854
return $httpBackend.expect(method, pathObj.regexp, undefined, undefined, pathObj.keys);
18811855
};
18821856

Diff for: ‎src/ngRoute/route.js

+3-43
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict';
22

3+
/* global routeToRegExp: false */
34
/* global shallowCopy: false */
45

56
// `isArray` and `isObject` are necessary for `shallowCopy()` (included via `src/shallowCopy.js`).
@@ -210,7 +211,7 @@ function $RouteProvider() {
210211
}
211212
routes[path] = angular.extend(
212213
routeCopy,
213-
path && pathRegExp(path, routeCopy)
214+
path && routeToRegExp(path, routeCopy)
214215
);
215216

216217
// create redirection for trailing slashes
@@ -221,7 +222,7 @@ function $RouteProvider() {
221222

222223
routes[redirectPath] = angular.extend(
223224
{redirectTo: path},
224-
pathRegExp(redirectPath, routeCopy)
225+
routeToRegExp(redirectPath, routeCopy)
225226
);
226227
}
227228

@@ -239,47 +240,6 @@ function $RouteProvider() {
239240
*/
240241
this.caseInsensitiveMatch = false;
241242

242-
/**
243-
* @param path {string} path
244-
* @param opts {Object} options
245-
* @return {?Object}
246-
*
247-
* @description
248-
* Normalizes the given path, returning a regular expression
249-
* and the original path.
250-
*
251-
* Inspired by pathRexp in visionmedia/express/lib/utils.js.
252-
*/
253-
function pathRegExp(path, opts) {
254-
var insensitive = opts.caseInsensitiveMatch,
255-
ret = {
256-
originalPath: path,
257-
regexp: path
258-
},
259-
keys = ret.keys = [];
260-
261-
path = path
262-
.replace(/([().])/g, '\\$1')
263-
.replace(/(\/)?:(\w+)(\*\?|[?*])?/g, function(_, slash, key, option) {
264-
var optional = (option === '?' || option === '*?') ? '?' : null;
265-
var star = (option === '*' || option === '*?') ? '*' : null;
266-
keys.push({ name: key, optional: !!optional });
267-
slash = slash || '';
268-
return ''
269-
+ (optional ? '' : slash)
270-
+ '(?:'
271-
+ (optional ? slash : '')
272-
+ (star && '(.+?)' || '([^/]+)')
273-
+ (optional || '')
274-
+ ')'
275-
+ (optional || '');
276-
})
277-
.replace(/([/$*])/g, '\\$1');
278-
279-
ret.regexp = new RegExp('^' + path + '$', insensitive ? 'i' : '');
280-
return ret;
281-
}
282-
283243
/**
284244
* @ngdoc method
285245
* @name $routeProvider#otherwise

Diff for: ‎src/routeToRegExp.js

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
'use strict';
2+
3+
/* global routeToRegExp: true */
4+
5+
/**
6+
* @param path {string} path
7+
* @param opts {Object} options
8+
* @return {?Object}
9+
*
10+
* @description
11+
* Normalizes the given path, returning a regular expression
12+
* and the original path.
13+
*
14+
* Inspired by pathRexp in visionmedia/express/lib/utils.js.
15+
*/
16+
function routeToRegExp(path, opts) {
17+
var keys = [];
18+
19+
var pattern = path
20+
.replace(/([().])/g, '\\$1')
21+
.replace(/(\/)?:(\w+)(\*\?|[?*])?/g, function(_, slash, key, option) {
22+
var optional = option === '?' || option === '*?';
23+
var star = option === '*' || option === '*?';
24+
keys.push({ name: key, optional: optional });
25+
slash = slash || '';
26+
return (
27+
(optional ? '(?:' + slash : slash + '(?:') +
28+
(star ? '([^?#]+?)' : '([^/?#]+)') +
29+
(optional ? '?)?' : ')')
30+
);
31+
})
32+
.replace(/([/$*])/g, '\\$1');
33+
34+
if (opts.ignoreTrailingSlashes) {
35+
pattern = pattern.replace(/\/+$/, '') + '/*';
36+
}
37+
38+
return {
39+
originalPath: path,
40+
keys: keys,
41+
regexp: new RegExp(
42+
'^' + pattern + '(?:[?#]|$)',
43+
opts.caseInsensitiveMatch ? 'i' : ''
44+
)
45+
};
46+
}

Diff for: ‎test/ngMock/angular-mocksSpec.js

+29-5
Original file line numberDiff line numberDiff line change
@@ -1941,12 +1941,36 @@ describe('ngMock', function() {
19411941
expect(callback).toHaveBeenCalledOnceWith(200, 'path', '', '', 'complete');
19421942
}
19431943
);
1944-
they('should ignore query param when matching in ' + routeShortcut + ' $prop method', methods,
1944+
they('should ignore query params when matching in ' + routeShortcut + ' $prop method', methods,
19451945
function() {
1946-
hb[routeShortcut](this, '/route/:id').respond('path');
1947-
hb(this, '/route/123?q=str&foo=bar', undefined, callback);
1948-
hb.flush();
1949-
expect(callback).toHaveBeenCalledOnceWith(200, 'path', '', '', 'complete');
1946+
angular.forEach([
1947+
{route: '/route1/:id', url: '/route1/Alpha', expectedParams: {id: 'Alpha'}},
1948+
{route: '/route2/:id', url: '/route2/Bravo/?', expectedParams: {id: 'Bravo'}},
1949+
{route: '/route3/:id', url: '/route3/Charlie?q=str&foo=bar', expectedParams: {id: 'Charlie', q: 'str', foo: 'bar'}},
1950+
{route: '/:x/route4', url: '/Delta/route4?q=str&foo=bar', expectedParams: {x: 'Delta', q: 'str', foo: 'bar'}},
1951+
{route: '/route5/:id*', url: '/route5/Echo/456?q=str&foo=bar', expectedParams: {id: 'Echo/456', q: 'str', foo: 'bar'}},
1952+
{route: '/route6/:id*', url: '/route6/Foxtrot/456/?q=str&foo=bar', expectedParams: {id: 'Foxtrot/456', q: 'str', foo: 'bar'}},
1953+
{route: '/route7/:id*', url: '/route7/Golf/456//?q=str&foo=bar', expectedParams: {id: 'Golf/456', q: 'str', foo: 'bar'}},
1954+
{route: '/:x*/route8', url: '/Hotel/123/456/route8/?q=str&foo=bar', expectedParams: {x: 'Hotel/123/456', q: 'str', foo: 'bar'}},
1955+
{route: '/:x*/route9/:id', url: '/India/456/route9/0?q=str&foo=bar', expectedParams: {x: 'India/456', id: '0', q: 'str', foo: 'bar'}},
1956+
{route: '/route10', url: '/route10?q=Juliet&foo=bar', expectedParams: {q: 'Juliet', foo: 'bar'}},
1957+
{route: '/route11', url: '/route11///?q=Kilo', expectedParams: {q: 'Kilo'}},
1958+
{route: '/route12', url: '/route12///', expectedParams: {}}
1959+
], function(testDataEntry) {
1960+
callback.calls.reset();
1961+
var paramsSpy = jasmine.createSpy('params');
1962+
hb[routeShortcut](this, testDataEntry.route).respond(
1963+
function(method, url, data, headers, params) {
1964+
paramsSpy(params);
1965+
// status, response, headers, statusText, xhrStatus
1966+
return [200, 'path', { 'x-header': 'foo' }, 'OK', 'complete'];
1967+
}
1968+
);
1969+
hb(this, testDataEntry.url, undefined, callback);
1970+
hb.flush();
1971+
expect(callback).toHaveBeenCalledOnceWith(200, 'path', 'x-header: foo', 'OK', 'complete');
1972+
expect(paramsSpy).toHaveBeenCalledOnceWith(testDataEntry.expectedParams);
1973+
});
19501974
}
19511975
);
19521976
});

0 commit comments

Comments
 (0)