Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 2a21234

Browse files
marknadigvojtajina
authored andcommitted
fix($resource): params should expand array values properly
Today, calling e.g. var R = $resource('/Path/:a'); R.get({a: 'foo', bar: ['baz1', 'baz2']}); results in a query string like "/Path/doh?bar=baz1,baz2" which is undesirable. This commit enhances resource to use $http to encode any non-url parameters resulting in a query string like "/Path/doh?bar=baz1&bar=baz2". BREAKING CHANGE: if the server relied on the buggy behavior then either the backend should be fixed or a simple serialization of the array should be done on the client before calling the resource service.
1 parent 1d7a95d commit 2a21234

File tree

2 files changed

+27
-17
lines changed

2 files changed

+27
-17
lines changed

src/ngResource/resource.js

+9-8
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ angular.module('ngResource', ['ng']).
316316
}
317317

318318
Route.prototype = {
319-
url: function(params) {
319+
setUrlParams: function(config, params) {
320320
var self = this,
321321
url = this.template,
322322
val,
@@ -339,16 +339,17 @@ angular.module('ngResource', ['ng']).
339339
});
340340
}
341341
});
342-
url = url.replace(/\/?#$/, '');
343-
var query = [];
342+
343+
// set the url
344+
config.url = url.replace(/\/?#$/, '').replace(/\/*$/, '');
345+
346+
// set params - delegate param encoding to $http
344347
forEach(params, function(value, key){
345348
if (!self.urlParams[key]) {
346-
query.push(encodeUriQuery(key) + '=' + encodeUriQuery(value));
349+
config.params = config.params || {};
350+
config.params[key] = value;
347351
}
348352
});
349-
query.sort();
350-
url = url.replace(/\/*$/, '');
351-
return url + (query.length ? '?' + query.join('&') : '');
352353
}
353354
};
354355

@@ -426,7 +427,7 @@ angular.module('ngResource', ['ng']).
426427
}
427428
});
428429
httpConfig.data = data;
429-
httpConfig.url = route.url(extend({}, extractParams(data, action.params || {}), params))
430+
route.setUrlParams(httpConfig, extend({}, extractParams(data, action.params || {}), params));
430431

431432
function markResolved() { value.$resolved = true; }
432433

test/ngResource/resourceSpec.js

+18-9
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,24 @@ describe("resource", function() {
118118
});
119119

120120

121-
it('should not encode @ in url params', function() {
122-
//encodeURIComponent is too agressive and doesn't follow http://www.ietf.org/rfc/rfc3986.txt
123-
//with regards to the character set (pchar) allowed in path segments
124-
//so we need this test to make sure that we don't over-encode the params and break stuff like
125-
//buzz api which uses @self
121+
// In order to get this passed, we need to fix $http - another breaking change,
122+
// so I'm gonna submit that as a separate CL.
123+
xit('should not encode @ in url params', function() {
124+
//encodeURIComponent is too agressive and doesn't follow http://www.ietf.org/rfc/rfc3986.txt
125+
//with regards to the character set (pchar) allowed in path segments
126+
//so we need this test to make sure that we don't over-encode the params and break stuff like
127+
//buzz api which uses @self
126128

129+
var R = $resource('/Path/:a');
130+
$httpBackend.expect('GET', '/Path/doh@fo%20o?!do%26h=g%3Da+h&:bar=$baz@1').respond('{}');
131+
R.get({a: 'doh@fo o', ':bar': '$baz@1', '!do&h': 'g=a h'});
132+
});
133+
134+
135+
it('should encode array params', function() {
127136
var R = $resource('/Path/:a');
128-
$httpBackend.expect('GET', '/Path/doh@fo%20o?!do%26h=g%3Da+h&:bar=$baz@1').respond('{}');
129-
R.get({a: 'doh@fo o', ':bar': '$baz@1', '!do&h': 'g=a h'});
137+
$httpBackend.expect('GET', '/Path/doh&foo?bar=baz1&bar=baz2').respond('{}');
138+
R.get({a: 'doh&foo', bar: ['baz1', 'baz2']});
130139
});
131140

132141
it('should allow relative paths in resource url', function () {
@@ -554,7 +563,7 @@ describe("resource", function() {
554563
expect(response).toEqualData({
555564
data: [{id: 1}, {id :2}],
556565
status: 200,
557-
config: {method: 'GET', data: undefined, url: '/CreditCard?key=value'},
566+
config: {method: 'GET', data: undefined, url: '/CreditCard', params: {key: 'value'}},
558567
resource: [ { id : 1 }, { id : 2 } ]
559568
});
560569
expect(typeof response.resource[0].$save).toBe('function');
@@ -603,7 +612,7 @@ describe("resource", function() {
603612
expect(response).toEqualData({
604613
data : 'resource not found',
605614
status : 404,
606-
config : { method : 'GET', data : undefined, url : '/CreditCard?key=value' }
615+
config : { method : 'GET', data : undefined, url : '/CreditCard', params: {key: 'value'}}
607616
});
608617
});
609618

0 commit comments

Comments
 (0)