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 warning is logged (using `$log.debug()`) and the property is removed. - Add support for a boolean `cancellable` property in actions' configuration, the `$resource` factory's `options` parameter and the `$resourceProvider`'s `defaults` property. 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 a numeric `timeout` specified on the action's configuration, the value of `cancellable` will be 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}); ... ``` BREAKING CHANGE: Using a promise as `timeout` is no longer supported and will log a warning. It never worked the way it was supposed to anyway. Before: ```js var deferred = $q.defer(); var User = $resource('/api/user/:id', {id: '@id'}, { get: {method: 'GET', timeout: deferred.promise} }); var user = User.get({id: 1}); // sends a request deferred.resolve(); // aborts the request // Now, we need to re-define `User` passing a new promise as `timeout` // or else all subsequent requests from `someAction` will be aborted User = $resource(...); user = User.get({id: 2}); ``` After: ```js var User = $resource('/api/user/:id', {id: '@id'}, { get: {method: 'GET', cancellable: true} }); var user = User.get({id: 1}); // sends a request instance.$cancelRequest(); // aborts the request user = User.get({id: 2}); ``` Fixes angular#9332 Closes angular#13050 Closes angular#13058
petebacondarwin · Nov 23, 2015 · 15de1d5 · 15de1d5
1 parent d9ec995
commit 15de1d5
Show file tree

Hide file tree

Showing 2 changed files with 170 additions and 16 deletions.
diff --git a/src/ngResource/resource.js b/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;

diff --git a/test/ngResource/resourceSpec.js b/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);
+  });
 });