feat($resource): expose promise instead of only $then

- Instance or collection have `$promise` property which is the initial promise.
- Add per-action `interceptor`, which has access to entire $http response object.

BREAKING CHANGE: resource instance does not have `$then` function anymore.

Before:

Resource.query().$then(callback);

After:

Resource.query().$promise.then(callback);

BREAKING CHANGE: instance methods return the promise rather than the instance itself.

Before:

resource.$save().chaining = true;

After:

resource.$save();
resourve.chaining = true;

BREAKING CHANGE: On success, promise is resolved with the resource instance rather than http
response object.

Use interceptor to access the http response object.

Before:

Resource.query().$then(function(response) {...});

After:

var Resource = $resource('/url', {}, {
  get: {
    method: 'get',
    interceptor: {
      response: function(response) {
        // expose response
        return response;
      }
    }
  }
});

2 files changed, 227 insertions, 101 deletions

diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js
index 59ac3b3a..abb2bc56 100644
--- a/src/ngResource/resource.js
+++ b/src/ngResource/resource.js
@@ -92,6 +92,9 @@
  *     requests with credentials} for more information.
  *   - **`responseType`** - `{string}` - see {@link
  *     https://developer.mozilla.org/en-US/docs/DOM/XMLHttpRequest#responseType requestType}.
+ *   - **`interceptor`** - `{Object=}` - The interceptor object has two optional methods -
+ *     `response` and `responseError`. Both `response` and `responseError` interceptors get called
+ *     with `http response` object. See {@link ng.$http $http interceptors}.
  *
  * @returns {Object} A resource "class" object with methods for the default set of resource actions
  *   optionally extended with custom `actions`. The default set contains these actions:
@@ -130,24 +133,27 @@
  *   - non-GET "class" actions: `Resource.action([parameters], postData, [success], [error])`
  *   - non-GET instance actions:  `instance.$action([parameters], [success], [error])`
  *
+ *   Success callback is called with (value, responseHeaders) arguments. Error callback is called
+ *   with (httpResponse) argument.
  *
- *   The Resource instances and collection have these additional properties:
+ *   Class actions return empty instance (with additional properties below).
+ *   Instance actions return promise of the action.
  *
- *   - `$then`: the `then` method of a {@link ng.$q promise} derived from the underlying
- *     {@link ng.$http $http} call.
+ *   The Resource instances and collection have these additional properties:
  *
- *     The success callback for the `$then` method will be resolved if the underlying `$http` requests
- *     succeeds.
+ *   - `$promise`: the {@link ng.$q promise} of the original server interaction that created this
+ *     instance or collection.
  *
- *     The success callback is called with a single object which is the {@link ng.$http http response}
- *     object extended with a new property `resource`. This `resource` property is a reference to the
- *     result of the resource action — resource object or array of resources.
+ *     On success, the promise is resolved with the same resource instance or collection object,
+ *     updated with data from server. This makes it easy to use in
+ *     {@link ng.$routeProvider resolve section of $routeProvider.when()} to defer view rendering
+ *     until the resource(s) are loaded.
  *
- *     The error callback is called with the {@link ng.$http http response} object when an http
- *     error occurs.
+ *     On failure, the promise is resolved with the {@link ng.$http http response} object,
+ *     without the `resource` property.
  *
- *   - `$resolved`: true if the promise has been resolved (either with success or rejection);
- *     Knowing if the Resource has been resolved is useful in data-binding.
+ *   - `$resolved`: `true` after first server interaction is completed (either with success or rejection),
+ *     `false` before that. Knowing if the Resource has been resolved is useful in data-binding.
  *
  * @example
  *
@@ -268,7 +274,7 @@
     </doc:example>
  */
 angular.module('ngResource', ['ng']).
-  factory('$resource', ['$http', '$parse', function($http, $parse) {
+  factory('$resource', ['$http', '$parse', '$q', function($http, $parse, $q) {
     var DEFAULT_ACTIONS = {
       'get':    {method:'GET'},
       'save':   {method:'POST'},
@@ -398,19 +404,19 @@ angular.module('ngResource', ['ng']).
         return ids;
       }
 
+      function defaultResponseInterceptor(response) {
+        return response.resource;
+      }
+
       function Resource(value){
         copy(value || {}, this);
       }
 
       forEach(actions, function(action, name) {
-        action.method = angular.uppercase(action.method);
-        var hasBody = action.method == 'POST' || action.method == 'PUT' || action.method == 'PATCH';
+        var hasBody = /^(POST|PUT|PATCH)$/i.test(action.method);
+
         Resource[name] = function(a1, a2, a3, a4) {
-          var params = {};
-          var data;
-          var success = noop;
-          var error = null;
-          var promise;
+          var params = {}, data, success, error;
 
           switch(arguments.length) {
           case 4:
@@ -442,31 +448,28 @@ angular.module('ngResource', ['ng']).
             break;
           case 0: break;
           default:
-            throw "Expected between 0-4 arguments [params, data, success, error], got " +
+            throw "Expected up to 4 arguments [params, data, success, error], got " +
               arguments.length + " arguments.";
           }
 
-          var value = this instanceof Resource ? this : (action.isArray ? [] : new Resource(data));
-          var httpConfig = {},
-              promise;
+          var isInstanceCall = data instanceof Resource;
+          var value = isInstanceCall ? data : (action.isArray ? [] : new Resource(data));
+          var httpConfig = {};
+          var responseInterceptor = action.interceptor && action.interceptor.response || defaultResponseInterceptor;
+          var responseErrorInterceptor = action.interceptor && action.interceptor.responseError || undefined;
 
           forEach(action, function(value, key) {
-            if (key != 'params' && key != 'isArray' ) {
+            if (key != 'params' && key != 'isArray' && key != 'interceptor') {
               httpConfig[key] = copy(value);
             }
           });
+
           httpConfig.data = data;
           route.setUrlParams(httpConfig, extend({}, extractParams(data, action.params || {}), params), action.url);
 
-          function markResolved() { value.$resolved = true; }
-
-          promise = $http(httpConfig);
-          value.$resolved = false;
-
-          promise.then(markResolved, markResolved);
-          value.$then = promise.then(function(response) {
-            var data = response.data;
-            var then = value.$then, resolved = value.$resolved;
+          var promise = $http(httpConfig).then(function(response) {
+            var data = response.data,
+                promise = value.$promise;
 
             if (data) {
               if (action.isArray) {
@@ -476,44 +479,47 @@ angular.module('ngResource', ['ng']).
                 });
               } else {
                 copy(data, value);
-                value.$then = then;
-                value.$resolved = resolved;
+                value.$promise = promise;
               }
             }
 
+            value.$resolved = true;
+
             (success||noop)(value, response.headers);
 
             response.resource = value;
+
             return response;
-          }, error).then;
+          }, function(response) {
+            value.$resolved = true;
 
-          return value;
-        };
+            (error||noop)(response);
 
+            return $q.reject(response);
+          }).then(responseInterceptor, responseErrorInterceptor);
 
-        Resource.prototype['$' + name] = function(a1, a2, a3) {
-          var params = extractParams(this),
-              success = noop,
-              error;
 
-          switch(arguments.length) {
-          case 3: params = a1; success = a2; error = a3; break;
-          case 2:
-          case 1:
-            if (isFunction(a1)) {
-              success = a1;
-              error = a2;
-            } else {
-              params = a1;
-              success = a2 || noop;
-            }
-          case 0: break;
-          default:
-            throw "Expected between 1-3 arguments [params, success, error], got " +
-              arguments.length + " arguments.";
+          if (!isInstanceCall) {
+            // we are creating instance / collection
+            // - set the initial promise
+            // - return the instance / collection
+            value.$promise = promise;
+            value.$resolved = false;
+
+            return value;
+          }
+
+          // instance call
+          return promise;
+        };
+
+
+        Resource.prototype['$' + name] = function(params, success, error) {
+          if (isFunction(params)) {
+            error = success; success = params; params = {};
           }
-          var data = hasBody ? this : undefined;
-          Resource[name].call(this, params, data, success, error);
+          var result = Resource[name](params, this, success, error);
+          return result.$promise || result;
         };
       });
 
diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js
index ec7f1476..6a709fb7 100644
--- a/test/ngResource/resourceSpec.js
+++ b/test/ngResource/resourceSpec.js
@@ -467,58 +467,66 @@ describe("resource", function() {
 
     describe('single resource', function() {
 
-      it('should add promise $then method to the result object', function() {
+      it('should add $promise to the result object', function() {
         $httpBackend.expect('GET', '/CreditCard/123').respond({id: 123, number: '9876'});
         var cc = CreditCard.get({id: 123});
 
-        cc.$then(callback);
+        cc.$promise.then(callback);
         expect(callback).not.toHaveBeenCalled();
 
         $httpBackend.flush();
 
-        var response = callback.mostRecentCall.args[0];
-
-        expect(response.data).toEqual({id: 123, number: '9876'});
-        expect(response.status).toEqual(200);
-        expect(response.resource).toEqualData({id: 123, number: '9876', $resolved: true});
-        expect(typeof response.resource.$save).toBe('function');
+        expect(callback).toHaveBeenCalledOnce();
+        expect(callback.mostRecentCall.args[0]).toBe(cc);
       });
 
 
-      it('should keep $then around after promise resolution', function() {
+      it('should keep $promise around after resolution', function() {
         $httpBackend.expect('GET', '/CreditCard/123').respond({id: 123, number: '9876'});
         var cc = CreditCard.get({id: 123});
 
-        cc.$then(callback);
+        cc.$promise.then(callback);
         $httpBackend.flush();
 
-        var response = callback.mostRecentCall.args[0];
-
         callback.reset();
 
-        cc.$then(callback);
+        cc.$promise.then(callback);
         $rootScope.$apply(); //flush async queue
 
-        expect(callback).toHaveBeenCalledOnceWith(response);
+        expect(callback).toHaveBeenCalledOnce();
+      });
+
+
+      it('should keep the original promise after instance action', function() {
+        $httpBackend.expect('GET', '/CreditCard/123').respond({id: 123, number: '9876'});
+        $httpBackend.expect('POST', '/CreditCard/123').respond({id: 123, number: '9876'});
+
+        var cc = CreditCard.get({id: 123});
+        var originalPromise = cc.$promise;
+
+        cc.number = '666';
+        cc.$save({id: 123});
+
+        expect(cc.$promise).toBe(originalPromise);
       });
 
 
-      it('should allow promise chaining via $then method', function() {
+      it('should allow promise chaining', function() {
         $httpBackend.expect('GET', '/CreditCard/123').respond({id: 123, number: '9876'});
         var cc = CreditCard.get({id: 123});
 
-        cc.$then(function(response) { return 'new value'; }).then(callback);
+        cc.$promise.then(function(value) { return 'new value'; }).then(callback);
         $httpBackend.flush();
 
         expect(callback).toHaveBeenCalledOnceWith('new value');
       });
 
 
-      it('should allow error callback registration via $then method', function() {
+      it('should allow $promise error callback registration', function() {
         $httpBackend.expect('GET', '/CreditCard/123').respond(404, 'resource not found');
         var cc = CreditCard.get({id: 123});
 
-        cc.$then(null, callback);
+        cc.$promise.then(null, callback);
         $httpBackend.flush();
 
         var response = callback.mostRecentCall.args[0];
@@ -534,7 +542,7 @@ describe("resource", function() {
 
         expect(cc.$resolved).toBe(false);
 
-        cc.$then(callback);
+        cc.$promise.then(callback);
         expect(cc.$resolved).toBe(false);
 
         $httpBackend.flush();
@@ -547,69 +555,125 @@ describe("resource", function() {
         $httpBackend.expect('GET', '/CreditCard/123').respond(404, 'resource not found');
         var cc = CreditCard.get({id: 123});
 
-        cc.$then(null, callback);
+        cc.$promise.then(null, callback);
         $httpBackend.flush();
         expect(callback).toHaveBeenCalledOnce();
         expect(cc.$resolved).toBe(true);
       });
+
+
+      it('should keep $resolved true in all subsequent interactions', function() {
+        $httpBackend.expect('GET', '/CreditCard/123').respond({id: 123, number: '9876'});
+        var cc = CreditCard.get({id: 123});
+        $httpBackend.flush();
+        expect(cc.$resolved).toBe(true);
+
+        $httpBackend.expect('POST', '/CreditCard/123').respond();
+        cc.$save({id: 123});
+        expect(cc.$resolved).toBe(true);
+        $httpBackend.flush();
+        expect(cc.$resolved).toBe(true);
+      });
+
+
+      it('should return promise from action method calls', function() {
+        $httpBackend.expect('GET', '/CreditCard/123').respond({id: 123, number: '9876'});
+        var cc = new CreditCard({name: 'Mojo'});
+
+        expect(cc).toEqualData({name: 'Mojo'});
+
+        cc.$get({id:123}).then(callback);
+
+        $httpBackend.flush();
+        expect(callback).toHaveBeenCalledOnce();
+        expect(cc).toEqualData({id: 123, number: '9876'});
+        callback.reset();
+
+        $httpBackend.expect('POST', '/CreditCard').respond({id: 1, number: '9'});
+
+        cc.$save().then(callback);
+
+        $httpBackend.flush();
+        expect(callback).toHaveBeenCalledOnce();
+        expect(cc).toEqualData({id: 1, number: '9'});
+      });
+
+
+      it('should allow parsing a value from headers', function() {
+        // https://github.com/angular/angular.js/pull/2607#issuecomment-17759933
+        $httpBackend.expect('POST', '/CreditCard').respond(201, '', {'Location': '/new-id'});
+
+        var parseUrlFromHeaders = function(response) {
+          var resource = response.resource;
+          resource.url = response.headers('Location');
+          return resource;
+        };
+
+        var CreditCard = $resource('/CreditCard', {}, {
+          save: {
+            method: 'post',
+            interceptor: {response: parseUrlFromHeaders}
+          }
+        });
+
+        var cc = new CreditCard({name: 'Me'});
+
+        cc.$save();
+        $httpBackend.flush();
+
+        expect(cc.url).toBe('/new-id');
+      });
     });
 
 
     describe('resource collection', function() {
 
-      it('should add promise $then method to the result object', function() {
+      it('should add $promise to the result object', function() {
         $httpBackend.expect('GET', '/CreditCard?key=value').respond([{id: 1}, {id: 2}]);
         var ccs = CreditCard.query({key: 'value'});
 
-        ccs.$then(callback);
+        ccs.$promise.then(callback);
         expect(callback).not.toHaveBeenCalled();
 
         $httpBackend.flush();
 
-        var response = callback.mostRecentCall.args[0];
-
-        expect(response.data).toEqual([{id: 1}, {id :2}]);
-        expect(response.status).toEqual(200);
-        expect(response.resource).toEqualData([ { id : 1 }, { id : 2 } ]);
-        expect(typeof response.resource[0].$save).toBe('function');
-        expect(typeof response.resource[1].$save).toBe('function');
+        expect(callback).toHaveBeenCalledOnce();
+        expect(callback.mostRecentCall.args[0]).toBe(ccs);
       });
 
 
-      it('should keep $then around after promise resolution', function() {
+      it('should keep $promise around after resolution', function() {
         $httpBackend.expect('GET', '/CreditCard?key=value').respond([{id: 1}, {id: 2}]);
         var ccs = CreditCard.query({key: 'value'});
 
-        ccs.$then(callback);
+        ccs.$promise.then(callback);
         $httpBackend.flush();
 
-        var response = callback.mostRecentCall.args[0];
-
         callback.reset();
 
-        ccs.$then(callback);
+        ccs.$promise.then(callback);
         $rootScope.$apply(); //flush async queue
 
-        expect(callback).toHaveBeenCalledOnceWith(response);
+        expect(callback).toHaveBeenCalledOnce();
       });
 
 
-      it('should allow promise chaining via $then method', function() {
+      it('should allow promise chaining', function() {
         $httpBackend.expect('GET', '/CreditCard?key=value').respond([{id: 1}, {id: 2}]);
         var ccs = CreditCard.query({key: 'value'});
 
-        ccs.$then(function(response) { return 'new value'; }).then(callback);
+        ccs.$promise.then(function(value) { return 'new value'; }).then(callback);
         $httpBackend.flush();
 
         expect(callback).toHaveBeenCalledOnceWith('new value');
       });
 
 
-      it('should allow error callback registration via $then method', function() {
+      it('should allow $promise error callback registration', function() {
         $httpBackend.expect('GET', '/CreditCard?key=value').respond(404, 'resource not found');
         var ccs = CreditCard.query({key: 'value'});
 
-        ccs.$then(null, callback);
+        ccs.$promise.then(null, callback);
         $httpBackend.flush();
 
         var response = callback.mostRecentCall.args[0];
@@ -625,7 +689,7 @@ describe("resource", function() {
 
         expect(ccs.$resolved).toBe(false);
 
-        ccs.$then(callback);
+        ccs.$promise.then(callback);
         expect(ccs.$resolved).toBe(false);
 
         $httpBackend.flush();
@@ -638,12 +702,68 @@ describe("resource", function() {
         $httpBackend.expect('GET', '/CreditCard?key=value').respond(404, 'resource not found');
         var ccs = CreditCard.query({key: 'value'});
 
-        ccs.$then(null, callback);
+        ccs.$promise.then(null, callback);
         $httpBackend.flush();
         expect(callback).toHaveBeenCalledOnce();
         expect(ccs.$resolved).toBe(true);
       });
     });
+
+    it('should allow per action response interceptor that gets full response', function() {
+      CreditCard = $resource('/CreditCard', {}, {
+        query: {
+          method: 'get',
+          isArray: true,
+          interceptor: {
+            response: function(response) {
+              return response;
+            }
+          }
+        }
+      });
+
+      $httpBackend.expect('GET', '/CreditCard').respond([{id: 1}]);
+
+      var ccs = CreditCard.query();
+
+      ccs.$promise.then(callback);
+
+      $httpBackend.flush();
+      expect(callback).toHaveBeenCalledOnce();
+
+      var response = callback.mostRecentCall.args[0];
+      expect(response.resource).toBe(ccs);
+      expect(response.status).toBe(200);
+      expect(response.config).toBeDefined();
+    });
+
+
+    it('should allow per action responseError interceptor that gets full response', function() {
+      CreditCard = $resource('/CreditCard', {}, {
+        query: {
+          method: 'get',
+          isArray: true,
+          interceptor: {
+            responseError: function(response) {
+              return response;
+            }
+          }
+        }
+      });
+
+      $httpBackend.expect('GET', '/CreditCard').respond(404);
+
+      var ccs = CreditCard.query();
+
+      ccs.$promise.then(callback);
+
+      $httpBackend.flush();
+      expect(callback).toHaveBeenCalledOnce();
+
+      var response = callback.mostRecentCall.args[0];
+      expect(response.status).toBe(404);
+      expect(response.config).toBeDefined();
+    });
   });