feat($resource): add support for cancelling requests

Introduced changes:

- Deprecate passing a promise as `timeout` (for `$resource` actions).
  It never worked correctly anyway.
  Now a warnign is logged (using `$log.debug()`) and the property is
  removed.
- Add support for a boolean `cancellable` property to actions'
  configuration, the `$resource` classes `options` of the
  `$resourceProvider`'s defaults.
  If true, the `$cancelRequest` method (added to all returned values for
  non-instance calls) will abort the request (if it's not already
  completed or aborted).
  If there is `timeout` specified on the action's configuration, the value
  of `cancellable` is ignored.

Example usage:

```js
var Post = $resource('/posts/:id', {id: '@id'}, {
  get: {
    method: 'GET',
    cancellable: true
  }
});

var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
currentPost.$cancelRequest();
currentPost = Post.get({id: 2});
...
```

Fixes angular#9332
Closes angular#13050
Closes angular#13058

Author: gkalpak

src/ngResource/resource.js

@@ -365,7 +365,7 @@ angular.module('ngResource', ['ng']).
       }
     };
 
-    this.$get = ['$http', '$q', function($http, $q) {
+    this.$get = ['$http', '$log', '$q', function($http, $log, $q) {
 
       var noop = angular.noop,
         forEach = angular.forEach,
@@ -525,6 +525,22 @@ angular.module('ngResource', ['ng']).
         forEach(actions, function(action, name) {
           var hasBody = /^(POST|PUT|PATCH)$/i.test(action.method);
 
+          var hasTimeout = action.hasOwnProperty('timeout');
+          if (hasTimeout && !angular.isNumber(action.timeout)) {
+            $log.debug('ngResource:\n' +
+                       '  Only numeric values are allowed as `timeout`.\n' +
+                       '  Promises are not supported in $resource, because the same value has to ' +
+                       'be re-used for multiple requests. If you are looking for a way to cancel ' +
+                       'requests, you should use the `cancellable` option.');
+            delete action.timeout;
+            hasTimeout = false;
+          }
+          action.cancellable = hasTimeout ?
+              false : action.hasOwnProperty('cancellable') ?
+              action.cancellable : (options && options.hasOwnProperty('cancellable')) ?
+              options.cancellable :
+              provider.defaults.cancellable;
+
           Resource[name] = function(a1, a2, a3, a4) {
             var params = {}, data, success, error;
 
@@ -581,21 +597,35 @@ angular.module('ngResource', ['ng']).
                 case 'params':
                 case 'isArray':
                 case 'interceptor':
+                case 'cancellable':
                   break;
                 case 'timeout':
                   httpConfig[key] = value;
                   break;
               }
             });
 
+            if (!isInstanceCall) {
+              if (!action.cancellable) {
+                value.$cancelRequest = angular.noop;
+              } else {
+                var deferred = $q.defer();
+                httpConfig.timeout = deferred.promise;
+                value.$cancelRequest = deferred.resolve.bind(deferred);
+              }
+            }
+
             if (hasBody) httpConfig.data = data;
             route.setUrlParams(httpConfig,
               extend({}, extractParams(data, action.params || {}), params),
               action.url);
 
-            var promise = $http(httpConfig).then(function(response) {
+            var promise = $http(httpConfig).finally(function() {
+              if (value.$cancelRequest) value.$cancelRequest = angular.noop;
+            }).then(function(response) {
               var data = response.data,
-                promise = value.$promise;
+                promise = value.$promise,
+                cancelRequest = value.$cancelRequest;
 
               if (data) {
                 // Need to convert action.isArray to boolean in case it is undefined
@@ -625,6 +655,7 @@ angular.module('ngResource', ['ng']).
                 }
               }
 
+              value.$cancelRequest = cancelRequest;
               value.$resolved = true;
 
               response.resource = value;

test/ngResource/resourceSpec.js

@@ -1364,35 +1364,158 @@ describe('resource', function() {
         /^\[\$resource:badcfg\] Error in resource configuration for action `get`\. Expected response to contain an object but got an array \(Request: GET \/Customer\/123\)/
       );
   });
+});
+
+describe('resource wrt cancelling requests', function() {
+  var httpSpy;
+  var $httpBackend;
+  var $resource;
+
+  beforeEach(module('ngResource', function($provide) {
+    $provide.decorator('$http', function($delegate) {
+      httpSpy = jasmine.createSpy('$http').andCallFake($delegate);
+      return httpSpy;
+    });
+  }));
+
+  beforeEach(inject(function(_$httpBackend_, _$resource_) {
+    $httpBackend = _$httpBackend_;
+    $resource = _$resource_;
+  }));
+
+  it('should accept numeric timeouts in actions and pass them to $http', function() {
+    $httpBackend.whenGET('/CreditCard').respond({});
+
+    var CreditCard = $resource('/CreditCard', {}, {
+      get: {
+        method: 'GET',
+        timeout: 10000
+      }
+    });
+
+    CreditCard.get();
+    $httpBackend.flush();
+
+    expect(httpSpy).toHaveBeenCalledOnce();
+    expect(httpSpy.calls[0].args[0].timeout).toBe(10000);
+  });
+
+  it('should delete non-numeric timeouts in actions and log a $debug message',
+    inject(function($log, $q) {
+      spyOn($log, 'debug');
+      $httpBackend.whenGET('/CreditCard').respond({});
+
+      var CreditCard = $resource('/CreditCard', {}, {
+        get: {
+          method: 'GET',
+          timeout: $q.defer().promise
+        }
+      });
+
+      CreditCard.get();
+      $httpBackend.flush();
+
+      expect(httpSpy).toHaveBeenCalledOnce();
+      expect(httpSpy.calls[0].args[0].timeout).toBeUndefined();
+      expect($log.debug).toHaveBeenCalledOnceWith('ngResource:\n' +
+          '  Only numeric values are allowed as `timeout`.\n' +
+          '  Promises are not supported in $resource, because the same value has to ' +
+          'be re-used for multiple requests. If you are looking for a way to cancel ' +
+          'requests, you should use the `cancellable` option.');
+    })
+  );
+
+  it('should not create a `$cancelRequest` method for instance calls', function() {
+    var CreditCard = $resource('/CreditCard', {}, {
+      save1: {
+        method: 'POST',
+        cancellable: false
+      },
+      save2: {
+        method: 'POST',
+        cancellable: true
+      }
+    });
+
+    var creditCard = new CreditCard();
+
+    var promise1 = creditCard.$save1();
+    expect(promise1.$cancelRequest).toBeUndefined();
+    expect(creditCard.$cancelRequest).toBeUndefined();
+
+    var promise2 = creditCard.$save2();
+    expect(promise2.$cancelRequest).toBeUndefined();
+    expect(creditCard.$cancelRequest).toBeUndefined();
+  });
+
+  it('should always create a (possibly noop) `$cancelRequest` method for non-instance calls',
+    function() {
+      var CreditCard = $resource('/CreditCard', {}, {
+        get1: {
+          method: 'GET',
+          cancellable: false
+        },
+        get2: {
+          method: 'GET',
+          cancellable: true
+        }
+      });
 
-  it('should cancel the request if timeout promise is resolved', function() {
-    var canceler = $q.defer();
+      var creditCard1 = CreditCard.get1();
+      var creditCard2 = CreditCard.get2();
 
-    $httpBackend.when('GET', '/CreditCard').respond({data: '123'});
+      expect(creditCard1.$cancelRequest).toBe(noop);
+      expect(creditCard2.$cancelRequest).toBeDefined();
+    }
+  );
 
+  it('should not make the request cancellable if there is a timeout', function() {
     var CreditCard = $resource('/CreditCard', {}, {
-      query: {
+      get: {
         method: 'GET',
-        timeout: canceler.promise
+        timeout: 10000,
+        cancellable: true
       }
     });
 
-    CreditCard.query();
+    var creditCard = CreditCard.get();
 
-    canceler.resolve();
-    expect($httpBackend.flush).toThrow(new Error("No pending request to flush !"));
+    expect(creditCard.$cancelRequest).toBe(noop);
+  });
+
+  it('should cancel the request (if cancellable), when calling `$cancelRequest`', function() {
+    $httpBackend.whenGET('/CreditCard').respond({});
 
-    canceler = $q.defer();
-    CreditCard = $resource('/CreditCard', {}, {
-      query: {
+    var CreditCard = $resource('/CreditCard', {}, {
+      get: {
         method: 'GET',
-        timeout: canceler.promise
+        cancellable: true
       }
     });
 
-    CreditCard.query();
+    CreditCard.get().$cancelRequest();
+    expect($httpBackend.flush).toThrow(new Error('No pending request to flush !'));
+
+    CreditCard.get();
     expect($httpBackend.flush).not.toThrow();
   });
 
+  it('should reset `$cancelRequest` after the response arrives', function() {
+    $httpBackend.whenGET('/CreditCard').respond({});
+
+    var CreditCard = $resource('/CreditCard', {}, {
+      get: {
+        method: 'GET',
+        cancellable: true
+      }
+    });
+
+    var creditCard = CreditCard.get();
+
+    expect(creditCard.$cancelRequest).not.toBe(noop);
 
+    $httpBackend.flush();
+
+    expect(creditCard.$cancelRequest).toBe(noop);
+  });
 });