From be56c9ef5afe720af976111a19cb3ac071e43bac Mon Sep 17 00:00:00 2001 From: Michal Raczkowski Date: Fri, 22 Jan 2016 18:24:19 +0100 Subject: [PATCH 1/2] feat($resource): add support for timeout in cancellable actions 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 ``` --- src/ngResource/resource.js | 28 +++++++++++++++++++++------ test/ngResource/resourceSpec.js | 34 +++++++++++++++++++++++++++++---- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index de84f1ff4728..473db505a8af 100644 --- 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.
- * **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,7 +575,11 @@ angular.module('ngResource', ['ng']). forEach(actions, function(action, name) { var hasBody = /^(POST|PUT|PATCH)$/i.test(action.method); - var cancellable = false; + var cancellable = angular.isDefined(action.cancellable) ? action.cancellable : + (options && angular.isDefined(options.cancellable)) ? options.cancellable : + provider.defaults.cancellable; + + var simulatedTimeout; if (!angular.isNumber(action.timeout)) { if (action.timeout) { @@ -586,9 +590,11 @@ angular.module('ngResource', ['ng']). '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; + } else { + if(cancellable) { + simulatedTimeout = action.timeout; + delete action.timeout; + } } Resource[name] = function(a1, a2, a3, a4) { @@ -639,6 +645,7 @@ angular.module('ngResource', ['ng']). var responseErrorInterceptor = action.interceptor && action.interceptor.responseError || undefined; var timeoutDeferred; + var simulatedTimeoutPromise; forEach(action, function(value, key) { switch (key) { @@ -656,6 +663,14 @@ angular.module('ngResource', ['ng']). if (!isInstanceCall && cancellable) { timeoutDeferred = $q.defer(); httpConfig.timeout = timeoutDeferred.promise; + + if(simulatedTimeout) { + simulatedTimeoutPromise = $timeout(function() { + if(timeoutDeferred) { + timeoutDeferred.resolve(); + } + }, simulatedTimeout); + } } if (hasBody) httpConfig.data = data; @@ -707,6 +722,7 @@ angular.module('ngResource', ['ng']). if (!isInstanceCall && cancellable) { value.$cancelRequest = angular.noop; timeoutDeferred = httpConfig.timeout = null; + $timeout.cancel(simulatedTimeoutPromise); } }); diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js index b9f39a70cde7..c032b2ad63a1 100644 --- 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({}); From c1c6e9ca687309f60fe979574ad41a0a5b27effd Mon Sep 17 00:00:00 2001 From: Michal Raczkowski Date: Mon, 25 Jan 2016 18:43:18 +0100 Subject: [PATCH 2/2] fixup: simplify $timeout handler simplify $timeout handler set `simulatedTimeoutPromise` to null don't delete action.timeout - not necessary --- src/ngResource/resource.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index 473db505a8af..cc1ffc4a3ce9 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -593,7 +593,6 @@ angular.module('ngResource', ['ng']). } else { if(cancellable) { simulatedTimeout = action.timeout; - delete action.timeout; } } @@ -665,11 +664,7 @@ angular.module('ngResource', ['ng']). httpConfig.timeout = timeoutDeferred.promise; if(simulatedTimeout) { - simulatedTimeoutPromise = $timeout(function() { - if(timeoutDeferred) { - timeoutDeferred.resolve(); - } - }, simulatedTimeout); + simulatedTimeoutPromise = $timeout(timeoutDeferred.resolve, simulatedTimeout); } } @@ -723,6 +718,7 @@ angular.module('ngResource', ['ng']). value.$cancelRequest = angular.noop; timeoutDeferred = httpConfig.timeout = null; $timeout.cancel(simulatedTimeoutPromise); + simulatedTimeoutPromise = null; } });