diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index 767d3ba7872c..02c298bec636 100644 --- 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,6 +597,7 @@ angular.module('ngResource', ['ng']). case 'params': case 'isArray': case 'interceptor': + case 'cancellable': break; case 'timeout': httpConfig[key] = value; @@ -588,14 +605,27 @@ angular.module('ngResource', ['ng']). } }); + 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 index 41a90d2f1601..b25c4d45524b 100644 --- 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); + }); });