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

fix($resource): remove (broken) support for promises as timeout #13462

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 14 additions & 4 deletions src/ngResource/resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,11 @@ function shallowClearAndCopy(src, dst) {
* GET request, otherwise if a cache instance built with
* {@link ng.$cacheFactory $cacheFactory}, this cache will be used for
* caching.
* - **`timeout`** – `{number|Promise}` – timeout in milliseconds, or {@link ng.$q promise} that
* should abort the request when resolved.
* - **`timeout`** – `{number}` – timeout in milliseconds.<br />
* **Note:** In contrast to {@link ng.$http#usage $http.config}, {@link ng.$q promises} are
* **not** supported in $resource, because the same value would be used for multiple requests.
* If you need support for cancellable $resource actions, you should upgrade to version 1.5 or
* higher.
* - **`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 @@ -365,7 +368,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,
Expand Down Expand Up @@ -577,7 +580,14 @@ angular.module('ngResource', ['ng']).
case 'interceptor':
break;
case 'timeout':
httpConfig[key] = value;
if (value && !angular.isNumber(value)) {
$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.\n' +
' If you need support for cancellable $resource actions, you should ' +
'upgrade to version 1.5 or higher.');
}
break;
}
});
Expand Down
62 changes: 37 additions & 25 deletions test/ngResource/resourceSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1308,7 +1308,7 @@ describe("resource", function() {
});

describe('resource', function() {
var $httpBackend, $resource, $q;
var $httpBackend, $resource;

beforeEach(module(function($exceptionHandlerProvider) {
$exceptionHandlerProvider.mode('log');
Expand All @@ -1319,7 +1319,6 @@ describe('resource', function() {
beforeEach(inject(function($injector) {
$httpBackend = $injector.get('$httpBackend');
$resource = $injector.get('$resource');
$q = $injector.get('$q');
}));


Expand Down Expand Up @@ -1356,35 +1355,48 @@ 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\)/
);
});
});

it('should cancel the request if timeout promise is resolved', function() {
var canceler = $q.defer();

$httpBackend.when('GET', '/CreditCard').respond({data: '123'});
describe('resource with promises as `timeout`', function() {
var httpSpy;
var $httpBackend;
var $resource;

var CreditCard = $resource('/CreditCard', {}, {
query: {
method: 'GET',
timeout: canceler.promise
}
beforeEach(module('ngResource', function($provide) {
$provide.decorator('$http', function($delegate) {
httpSpy = jasmine.createSpy('$http').andCallFake($delegate);
return httpSpy;
});
}));

CreditCard.query();

canceler.resolve();
expect($httpBackend.flush).toThrow(new Error("No pending request to flush !"));
beforeEach(inject(function(_$httpBackend_, _$resource_) {
$httpBackend = _$httpBackend_;
$resource = _$resource_;
}));

canceler = $q.defer();
CreditCard = $resource('/CreditCard', {}, {
query: {
method: 'GET',
timeout: canceler.promise
}
});
it('should ignore non-numeric timeouts in actions and log a $debug message',
inject(function($log, $q) {
spyOn($log, 'debug');
$httpBackend.whenGET('/CreditCard').respond({});

CreditCard.query();
expect($httpBackend.flush).not.toThrow();
});
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 would ' +
'be used for multiple requests.\n' +
' If you need support for cancellable $resource actions, you should ' +
'upgrade to version 1.5 or higher.');
})
);
});