Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

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

Closed
wants to merge 2 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be56c9e
feat($resource): add support for timeout in cancellable actions
mrac Jan 22, 2016
c1c6e9c
fixup: simplify $timeout handler
mrac Jan 25, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions src/ngResource/resource.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simulatedTimeout is a little confusing imo (what is "simulated" about it).
(On the other hand, I'm notiriously bad with names 😃)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to find a different name, not to confuse with timeout which is passed to $http. There are quite a few variables that deal with $http timeout.

So for me it means "non-$http" timeout.


if (!angular.isNumber(action.timeout)) {
if (action.timeout) {
Expand All @@ -586,9 +590,10 @@ 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;
}
}

Resource[name] = function(a1, a2, a3, a4) {
Expand Down Expand Up @@ -639,6 +644,7 @@ angular.module('ngResource', ['ng']).
var responseErrorInterceptor = action.interceptor && action.interceptor.responseError ||
undefined;
var timeoutDeferred;
var simulatedTimeoutPromise;

forEach(action, function(value, key) {
switch (key) {
Expand All @@ -656,6 +662,10 @@ angular.module('ngResource', ['ng']).
if (!isInstanceCall && cancellable) {
timeoutDeferred = $q.defer();
httpConfig.timeout = timeoutDeferred.promise;

if(simulatedTimeout) {
simulatedTimeoutPromise = $timeout(timeoutDeferred.resolve, simulatedTimeout);
}
}

if (hasBody) httpConfig.data = data;
Expand Down Expand Up @@ -707,6 +717,8 @@ angular.module('ngResource', ['ng']).
if (!isInstanceCall && cancellable) {
value.$cancelRequest = angular.noop;
timeoutDeferred = httpConfig.timeout = null;
$timeout.cancel(simulatedTimeoutPromise);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also set simulatedTimeoutPromise to null (just in case).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

simulatedTimeoutPromise = null;
}
});

Expand Down
34 changes: 30 additions & 4 deletions test/ngResource/resourceSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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() {
Expand Down Expand Up @@ -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',
Expand All @@ -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() {
Expand All @@ -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({});

Expand Down