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

Commit

Permalink
fix($resource): params should expand array values properly
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
marknadig authored and vojtajina committed Feb 14, 2013
1 parent 1d7a95d commit 2a21234
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 17 deletions.
17 changes: 9 additions & 8 deletions src/ngResource/resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ angular.module('ngResource', ['ng']).
}

Route.prototype = {
url: function(params) {
setUrlParams: function(config, params) {
var self = this,
url = this.template,
val,
Expand All @@ -339,16 +339,17 @@ angular.module('ngResource', ['ng']).
});
}
});
url = url.replace(/\/?#$/, '');
var query = [];

// set the url
config.url = url.replace(/\/?#$/, '').replace(/\/*$/, '');

// set params - delegate param encoding to $http
forEach(params, function(value, key){
if (!self.urlParams[key]) {
query.push(encodeUriQuery(key) + '=' + encodeUriQuery(value));
config.params = config.params || {};
config.params[key] = value;
}
});
query.sort();
url = url.replace(/\/*$/, '');
return url + (query.length ? '?' + query.join('&') : '');
}
};

Expand Down Expand Up @@ -426,7 +427,7 @@ angular.module('ngResource', ['ng']).
}
});
httpConfig.data = data;
httpConfig.url = route.url(extend({}, extractParams(data, action.params || {}), params))
route.setUrlParams(httpConfig, extend({}, extractParams(data, action.params || {}), params));

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

Expand Down
27 changes: 18 additions & 9 deletions test/ngResource/resourceSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,24 @@ describe("resource", function() {
});


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

var R = $resource('/Path/:a');
$httpBackend.expect('GET', '/Path/doh@fo%20o?!do%26h=g%3Da+h&:bar=$baz@1').respond('{}');
R.get({a: 'doh@fo o', ':bar': '$baz@1', '!do&h': 'g=a h'});
});


it('should encode array params', function() {
var R = $resource('/Path/:a');
$httpBackend.expect('GET', '/Path/doh@fo%20o?!do%26h=g%3Da+h&:bar=$baz@1').respond('{}');
R.get({a: 'doh@fo o', ':bar': '$baz@1', '!do&h': 'g=a h'});
$httpBackend.expect('GET', '/Path/doh&foo?bar=baz1&bar=baz2').respond('{}');
R.get({a: 'doh&foo', bar: ['baz1', 'baz2']});
});

it('should allow relative paths in resource url', function () {
Expand Down Expand Up @@ -554,7 +563,7 @@ describe("resource", function() {
expect(response).toEqualData({
data: [{id: 1}, {id :2}],
status: 200,
config: {method: 'GET', data: undefined, url: '/CreditCard?key=value'},
config: {method: 'GET', data: undefined, url: '/CreditCard', params: {key: 'value'}},
resource: [ { id : 1 }, { id : 2 } ]
});
expect(typeof response.resource[0].$save).toBe('function');
Expand Down Expand Up @@ -603,7 +612,7 @@ describe("resource", function() {
expect(response).toEqualData({
data : 'resource not found',
status : 404,
config : { method : 'GET', data : undefined, url : '/CreditCard?key=value' }
config : { method : 'GET', data : undefined, url : '/CreditCard', params: {key: 'value'}}
});
});

Expand Down

0 comments on commit 2a21234

Please sign in to comment.