Skip to content

Commit 84f6a4c

Browse files
gkalpakgkalpak
committed
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
1 parent 596af70 commit 84f6a4c

File tree

2 files changed

+170
-16
lines changed

2 files changed

+170
-16
lines changed

‎src/ngResource/resource.js

+34-3
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ angular.module('ngResource', ['ng']).
365365
}
366366
};
367367

368-
this.$get = ['$http', '$q', function($http, $q) {
368+
this.$get = ['$http', '$log', '$q', function($http, $log, $q) {
369369

370370
var noop = angular.noop,
371371
forEach = angular.forEach,
@@ -525,6 +525,22 @@ angular.module('ngResource', ['ng']).
525525
forEach(actions, function(action, name) {
526526
var hasBody = /^(POST|PUT|PATCH)$/i.test(action.method);
527527

528+
var hasTimeout = action.hasOwnProperty('timeout');
529+
if (hasTimeout && !angular.isNumber(action.timeout)) {
530+
$log.debug('ngResource:\n' +
531+
' Only numeric values are allowed as `timeout`.\n' +
532+
' Promises are not supported in $resource, because the same value has to ' +
533+
'be re-used for multiple requests. If you are looking for a way to cancel ' +
534+
'requests, you should use the `cancellable` option.');
535+
delete action.timeout;
536+
hasTimeout = false;
537+
}
538+
action.cancellable = hasTimeout ?
539+
false : action.hasOwnProperty('cancellable') ?
540+
action.cancellable : (options && options.hasOwnProperty('cancellable')) ?
541+
options.cancellable :
542+
provider.defaults.cancellable;
543+
528544
Resource[name] = function(a1, a2, a3, a4) {
529545
var params = {}, data, success, error;
530546

@@ -581,21 +597,35 @@ angular.module('ngResource', ['ng']).
581597
case 'params':
582598
case 'isArray':
583599
case 'interceptor':
600+
case 'cancellable':
584601
break;
585602
case 'timeout':
586603
httpConfig[key] = value;
587604
break;
588605
}
589606
});
590607

608+
if (!isInstanceCall) {
609+
if (!action.cancellable) {
610+
value.$cancelRequest = angular.noop;
611+
} else {
612+
var deferred = $q.defer();
613+
httpConfig.timeout = deferred.promise;
614+
value.$cancelRequest = deferred.resolve.bind(deferred);
615+
}
616+
}
617+
591618
if (hasBody) httpConfig.data = data;
592619
route.setUrlParams(httpConfig,
593620
extend({}, extractParams(data, action.params || {}), params),
594621
action.url);
595622

596-
var promise = $http(httpConfig).then(function(response) {
623+
var promise = $http(httpConfig).finally(function() {
624+
if (value.$cancelRequest) value.$cancelRequest = angular.noop;
625+
}).then(function(response) {
597626
var data = response.data,
598-
promise = value.$promise;
627+
promise = value.$promise,
628+
cancelRequest = value.$cancelRequest;
599629

600630
if (data) {
601631
// Need to convert action.isArray to boolean in case it is undefined
@@ -625,6 +655,7 @@ angular.module('ngResource', ['ng']).
625655
}
626656
}
627657

658+
value.$cancelRequest = cancelRequest;
628659
value.$resolved = true;
629660

630661
response.resource = value;

‎test/ngResource/resourceSpec.js

+136-13
Original file line numberDiff line numberDiff line change
@@ -1364,35 +1364,158 @@ describe('resource', function() {
13641364
/^\[\$resource:badcfg\] Error in resource configuration for action `get`\. Expected response to contain an object but got an array \(Request: GET \/Customer\/123\)/
13651365
);
13661366
});
1367+
});
1368+
1369+
describe('resource wrt cancelling requests', function() {
1370+
var httpSpy;
1371+
var $httpBackend;
1372+
var $resource;
1373+
1374+
beforeEach(module('ngResource', function($provide) {
1375+
$provide.decorator('$http', function($delegate) {
1376+
httpSpy = jasmine.createSpy('$http').andCallFake($delegate);
1377+
return httpSpy;
1378+
});
1379+
}));
1380+
1381+
beforeEach(inject(function(_$httpBackend_, _$resource_) {
1382+
$httpBackend = _$httpBackend_;
1383+
$resource = _$resource_;
1384+
}));
1385+
1386+
it('should accept numeric timeouts in actions and pass them to $http', function() {
1387+
$httpBackend.whenGET('/CreditCard').respond({});
1388+
1389+
var CreditCard = $resource('/CreditCard', {}, {
1390+
get: {
1391+
method: 'GET',
1392+
timeout: 10000
1393+
}
1394+
});
1395+
1396+
CreditCard.get();
1397+
$httpBackend.flush();
1398+
1399+
expect(httpSpy).toHaveBeenCalledOnce();
1400+
expect(httpSpy.calls[0].args[0].timeout).toBe(10000);
1401+
});
1402+
1403+
it('should delete non-numeric timeouts in actions and log a $debug message',
1404+
inject(function($log, $q) {
1405+
spyOn($log, 'debug');
1406+
$httpBackend.whenGET('/CreditCard').respond({});
1407+
1408+
var CreditCard = $resource('/CreditCard', {}, {
1409+
get: {
1410+
method: 'GET',
1411+
timeout: $q.defer().promise
1412+
}
1413+
});
1414+
1415+
CreditCard.get();
1416+
$httpBackend.flush();
1417+
1418+
expect(httpSpy).toHaveBeenCalledOnce();
1419+
expect(httpSpy.calls[0].args[0].timeout).toBeUndefined();
1420+
expect($log.debug).toHaveBeenCalledOnceWith('ngResource:\n' +
1421+
' Only numeric values are allowed as `timeout`.\n' +
1422+
' Promises are not supported in $resource, because the same value has to ' +
1423+
'be re-used for multiple requests. If you are looking for a way to cancel ' +
1424+
'requests, you should use the `cancellable` option.');
1425+
})
1426+
);
1427+
1428+
it('should not create a `$cancelRequest` method for instance calls', function() {
1429+
var CreditCard = $resource('/CreditCard', {}, {
1430+
save1: {
1431+
method: 'POST',
1432+
cancellable: false
1433+
},
1434+
save2: {
1435+
method: 'POST',
1436+
cancellable: true
1437+
}
1438+
});
1439+
1440+
var creditCard = new CreditCard();
1441+
1442+
var promise1 = creditCard.$save1();
1443+
expect(promise1.$cancelRequest).toBeUndefined();
1444+
expect(creditCard.$cancelRequest).toBeUndefined();
1445+
1446+
var promise2 = creditCard.$save2();
1447+
expect(promise2.$cancelRequest).toBeUndefined();
1448+
expect(creditCard.$cancelRequest).toBeUndefined();
1449+
});
1450+
1451+
it('should always create a (possibly noop) `$cancelRequest` method for non-instance calls',
1452+
function() {
1453+
var CreditCard = $resource('/CreditCard', {}, {
1454+
get1: {
1455+
method: 'GET',
1456+
cancellable: false
1457+
},
1458+
get2: {
1459+
method: 'GET',
1460+
cancellable: true
1461+
}
1462+
});
13671463

1368-
it('should cancel the request if timeout promise is resolved', function() {
1369-
var canceler = $q.defer();
1464+
var creditCard1 = CreditCard.get1();
1465+
var creditCard2 = CreditCard.get2();
13701466

1371-
$httpBackend.when('GET', '/CreditCard').respond({data: '123'});
1467+
expect(creditCard1.$cancelRequest).toBe(noop);
1468+
expect(creditCard2.$cancelRequest).toBeDefined();
1469+
}
1470+
);
13721471

1472+
it('should not make the request cancellable if there is a timeout', function() {
13731473
var CreditCard = $resource('/CreditCard', {}, {
1374-
query: {
1474+
get: {
13751475
method: 'GET',
1376-
timeout: canceler.promise
1476+
timeout: 10000,
1477+
cancellable: true
13771478
}
13781479
});
13791480

1380-
CreditCard.query();
1481+
var creditCard = CreditCard.get();
13811482

1382-
canceler.resolve();
1383-
expect($httpBackend.flush).toThrow(new Error("No pending request to flush !"));
1483+
expect(creditCard.$cancelRequest).toBe(noop);
1484+
});
1485+
1486+
it('should cancel the request (if cancellable), when calling `$cancelRequest`', function() {
1487+
$httpBackend.whenGET('/CreditCard').respond({});
13841488

1385-
canceler = $q.defer();
1386-
CreditCard = $resource('/CreditCard', {}, {
1387-
query: {
1489+
var CreditCard = $resource('/CreditCard', {}, {
1490+
get: {
13881491
method: 'GET',
1389-
timeout: canceler.promise
1492+
cancellable: true
13901493
}
13911494
});
13921495

1393-
CreditCard.query();
1496+
CreditCard.get().$cancelRequest();
1497+
expect($httpBackend.flush).toThrow(new Error('No pending request to flush !'));
1498+
1499+
CreditCard.get();
13941500
expect($httpBackend.flush).not.toThrow();
13951501
});
13961502

1503+
it('should reset `$cancelRequest` after the response arrives', function() {
1504+
$httpBackend.whenGET('/CreditCard').respond({});
1505+
1506+
var CreditCard = $resource('/CreditCard', {}, {
1507+
get: {
1508+
method: 'GET',
1509+
cancellable: true
1510+
}
1511+
});
1512+
1513+
var creditCard = CreditCard.get();
1514+
1515+
expect(creditCard.$cancelRequest).not.toBe(noop);
13971516

1517+
$httpBackend.flush();
1518+
1519+
expect(creditCard.$cancelRequest).toBe(noop);
1520+
});
13981521
});

0 commit comments

Comments
 (0)