feat($resource): add support for timeout in cancellable actions

mrac · gkalpak · commit d641901be688 · 2016-01-26T13:59:28.000+02:00
Old behavior: actions can be either cancellable or have a numeric timeout. When having both defined, cancellable was ignored. With this commit: it's possible for actions to have both cancellable:true and numeric timeout defined. Example usage: ```js var Post = $resource('/posts/:id', {id: '@id'}, { get: { method: 'GET', cancellable: true, timeout: 10000 } }); var currentPost = Post.get({id: 1}); ... // the next request can cancel the previous one currentPost.$cancelRequest(); currentPost = Post.get({id: 2}); // any of those requests will also timeout, if the response // doesn't come within 10 seconds ``` Closes angular#13824
diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js
@@ -65,6 +65,7 @@ function shallowClearAndCopy(src, dst) {
  * @requires $http
  * @requires ng.$log
  * @requires $q
+ * @requires $timeout
  *
  * @description
  * A factory which creates a resource object that lets you interact with
@@ -160,7 +161,6 @@ function shallowClearAndCopy(src, dst) {
  *     will be cancelled (if not already completed) by calling `$cancelRequest()` on the call's
  *     return value. Calling `$cancelRequest()` for a non-cancellable or an already
  *     completed/cancelled request will have no effect.<br />
- *     **Note:** If a timeout is specified in millisecondes, `cancellable` is ignored.
  *   - **`withCredentials`** - `{boolean}` - whether to set the `withCredentials` flag on the
  *     XHR object. See
  *     [requests with credentials](https://developer.mozilla.org/en/http_access_control#section_5)
@@ -416,7 +416,7 @@ angular.module('ngResource', ['ng']).
       }
     };
 
-    this.$get = ['$http', '$log', '$q', function($http, $log, $q) {
+    this.$get = ['$http', '$log', '$q', '$timeout', function($http, $log, $q, $timeout) {
 
       var noop = angular.noop,
         forEach = angular.forEach,
@@ -575,20 +575,19 @@ angular.module('ngResource', ['ng']).
 
         forEach(actions, function(action, name) {
           var hasBody = /^(POST|PUT|PATCH)$/i.test(action.method);
-          var cancellable = false;
-
-          if (!angular.isNumber(action.timeout)) {
-            if (action.timeout) {
-              $log.debug('ngResource:\n' +
-                         '  Only numeric values are allowed as `timeout`.\n' +
-                         '  Promises are not supported in $resource, because the same value would ' +
-                         'be used for multiple requests. If you are looking for a way to cancel ' +
-                         'requests, you should use the `cancellable` option.');
-              delete action.timeout;
-            }
-            cancellable = angular.isDefined(action.cancellable) ? action.cancellable :
-                         (options && angular.isDefined(options.cancellable)) ? options.cancellable :
-                         provider.defaults.cancellable;
+          var numericTimeout = action.timeout;
+          var cancellable = angular.isDefined(action.cancellable) ? action.cancellable :
+              (options && angular.isDefined(options.cancellable)) ? options.cancellable :
+              provider.defaults.cancellable;
+
+          if (numericTimeout && !angular.isNumber(numericTimeout)) {
+            $log.debug('ngResource:\n' +
+                       '  Only numeric values are allowed as `timeout`.\n' +
+                       '  Promises are not supported in $resource, because the same value would ' +
+                       'be used for multiple requests. If you are looking for a way to cancel ' +
+                       'requests, you should use the `cancellable` option.');
+            delete action.timeout;
+            numericTimeout = null;
           }
 
           Resource[name] = function(a1, a2, a3, a4) {
@@ -639,6 +638,7 @@ angular.module('ngResource', ['ng']).
             var responseErrorInterceptor = action.interceptor && action.interceptor.responseError ||
               undefined;
             var timeoutDeferred;
+            var numericTimeoutPromise;
 
             forEach(action, function(value, key) {
               switch (key) {
@@ -656,6 +656,10 @@ angular.module('ngResource', ['ng']).
             if (!isInstanceCall && cancellable) {
               timeoutDeferred = $q.defer();
               httpConfig.timeout = timeoutDeferred.promise;
+
+              if (numericTimeout) {
+                numericTimeoutPromise = $timeout(timeoutDeferred.resolve, numericTimeout);
+              }
             }
 
             if (hasBody) httpConfig.data = data;
@@ -706,7 +710,8 @@ angular.module('ngResource', ['ng']).
               value.$resolved = true;
               if (!isInstanceCall && cancellable) {
                 value.$cancelRequest = angular.noop;
-                timeoutDeferred = httpConfig.timeout = null;
+                $timeout.cancel(numericTimeoutPromise);
+                timeoutDeferred = numericTimeoutPromise = httpConfig.timeout = null;
               }
             });
 
diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js
@@ -1372,6 +1372,7 @@ describe('cancelling requests', function() {
   var httpSpy;
   var $httpBackend;
   var $resource;
+  var $timeout;
 
   beforeEach(module('ngResource', function($provide) {
     $provide.decorator('$http', function($delegate) {
@@ -1380,9 +1381,10 @@ describe('cancelling requests', function() {
     });
   }));
 
-  beforeEach(inject(function(_$httpBackend_, _$resource_) {
+  beforeEach(inject(function(_$httpBackend_, _$resource_, _$timeout_) {
     $httpBackend = _$httpBackend_;
     $resource = _$resource_;
+    $timeout = _$timeout_;
   }));
 
   it('should accept numeric timeouts in actions and pass them to $http', function() {
@@ -1531,7 +1533,9 @@ describe('cancelling requests', function() {
     expect(creditCard3.$cancelRequest).toBeUndefined();
   });
 
-  it('should not make the request cancellable if there is a timeout', function() {
+  it('should accept numeric timeouts in cancellable actions and cancel the request when timeout occurs', function() {
+    $httpBackend.whenGET('/CreditCard').respond({});
+
     var CreditCard = $resource('/CreditCard', {}, {
       get: {
         method: 'GET',
@@ -1540,9 +1544,13 @@ describe('cancelling requests', function() {
       }
     });
 
-    var creditCard = CreditCard.get();
+    CreditCard.get();
+    $timeout.flush();
+    expect($httpBackend.flush).toThrow(new Error('No pending request to flush !'));
+
+    CreditCard.get();
+    expect($httpBackend.flush).not.toThrow();
 
-    expect(creditCard.$cancelRequest).toBeUndefined();
   });
 
   it('should cancel the request (if cancellable), when calling `$cancelRequest`', function() {
@@ -1562,6 +1570,24 @@ describe('cancelling requests', function() {
     expect($httpBackend.flush).not.toThrow();
   });
 
+  it('should cancel the request, when calling `$cancelRequest` in cancellable actions with timeout defined', function() {
+    $httpBackend.whenGET('/CreditCard').respond({});
+
+    var CreditCard = $resource('/CreditCard', {}, {
+      get: {
+        method: 'GET',
+        timeout: 10000,
+        cancellable: true
+      }
+    });
+
+    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({});
 
