From 2a2123441c2b749b8f316a24c3ca3f77a9132a01 Mon Sep 17 00:00:00 2001
From: Mark Nadig <mark@nadigs.net>
Date: Wed, 30 Jan 2013 08:41:39 -0700
Subject: [PATCH] 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.
---
 src/ngResource/resource.js      | 17 +++++++++--------
 test/ngResource/resourceSpec.js | 27 ++++++++++++++++++---------
 2 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js
index 484e9b01c92e..6e179827bdd8 100644
--- a/src/ngResource/resource.js
+++ b/src/ngResource/resource.js
@@ -316,7 +316,7 @@ angular.module('ngResource', ['ng']).
     }
 
     Route.prototype = {
-      url: function(params) {
+      setUrlParams: function(config, params) {
         var self = this,
             url = this.template,
             val,
@@ -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('&') : '');
       }
     };
 
@@ -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; }
 
diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js
index 1edb5a959b6f..db6fd4023fb3 100644
--- a/test/ngResource/resourceSpec.js
+++ b/test/ngResource/resourceSpec.js
@@ -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 () {
@@ -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');
@@ -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'}}
         });
       });