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

feat($resource): add support for cancelling requests #13210

Closed
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
123 changes: 103 additions & 20 deletions src/ngResource/resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ function shallowClearAndCopy(src, dst) {
* @ngdoc service
* @name $resource
* @requires $http
* @requires ng.$log
* @requires $q
*
* @description
* A factory which creates a resource object that lets you interact with
Expand Down Expand Up @@ -107,9 +109,9 @@ function shallowClearAndCopy(src, dst) {
* URL `/path/greet?salutation=Hello`.
*
* If the parameter value is prefixed with `@` then the value for that parameter will be extracted
* from the corresponding property on the `data` object (provided when calling an action method). For
* example, if the `defaultParam` object is `{someParam: '@someProp'}` then the value of `someParam`
* will be `data.someProp`.
* from the corresponding property on the `data` object (provided when calling an action method).
* For example, if the `defaultParam` object is `{someParam: '@someProp'}` then the value of
* `someParam` will be `data.someProp`.
*
* @param {Object.<Object>=} actions Hash with declaration of custom actions that should extend
* the default set of resource actions. The declaration should be created in the format of {@link
Expand Down Expand Up @@ -143,15 +145,23 @@ function shallowClearAndCopy(src, dst) {
* `{function(data, headersGetter)|Array.<function(data, headersGetter)>}` –
* transform function or an array of such functions. The transform function takes the http
* response body and headers and returns its transformed (typically deserialized) version.
* By default, transformResponse will contain one function that checks if the response looks like
* a JSON string and deserializes it using `angular.fromJson`. To prevent this behavior, set
* `transformResponse` to an empty array: `transformResponse: []`
* By default, transformResponse will contain one function that checks if the response looks
* like a JSON string and deserializes it using `angular.fromJson`. To prevent this behavior,
* set `transformResponse` to an empty array: `transformResponse: []`
Copy link
Member Author

Choose a reason for hiding this comment

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

Here and above, I just truncated the lines to 100 chars max.
Sorry, I couldn't help myself 😃

* - **`cache`** – `{boolean|Cache}` – If true, a default $http cache will be used to cache the
* 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 has to be re-used for multiple
* requests. If you are looking for a way to cancel requests, you should use the `cancellable`
* option.
* - **`cancellable`** – `{boolean}` – if set to true, the request made by a "non-instance" call
* 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is that so?

Copy link
Member Author

Choose a reason for hiding this comment

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

By design 😄

Basically, both need to pass a value as timeout to $http's config (either the number of milliseconds or a promise).
You can't use both a numeric timeout and a promise, so I chose for timeout to take precedence.
(Note that calcellable can be configured as a global default or as per resource default (using the options argument), while timeout can't.)

* - **`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 All @@ -163,12 +173,13 @@ function shallowClearAndCopy(src, dst) {
* with `http response` object. See {@link ng.$http $http interceptors}.
*
* @param {Object} options Hash with custom settings that should extend the
* default `$resourceProvider` behavior. The only supported option is
*
* Where:
* default `$resourceProvider` behavior. The supported options are:
*
* - **`stripTrailingSlashes`** – {boolean} – If true then the trailing
* slashes from any calculated URL will be stripped. (Defaults to true.)
* - **`cancellable`** – {boolean} – If true, the request made by a "non-instance" call will be
* cancelled (if not already completed) by calling `$cancelRequest()` on the call's return value.
* This can be overwritten per action. (Defaults to false.)
*
* @returns {Object} A resource "class" object with methods for the default set of resource actions
* optionally extended with custom `actions`. The default set contains these actions:
Expand Down Expand Up @@ -216,7 +227,7 @@ function shallowClearAndCopy(src, dst) {
* Class actions return empty instance (with additional properties below).
* Instance actions return promise of the action.
*
* The Resource instances and collection have these additional properties:
* The Resource instances and collections have these additional properties:
*
* - `$promise`: the {@link ng.$q promise} of the original server interaction that created this
* instance or collection.
Expand All @@ -236,6 +247,11 @@ function shallowClearAndCopy(src, dst) {
* rejection), `false` before that. Knowing if the Resource has been resolved is useful in
* data-binding.
*
* The Resource instances and collections have these additional methods:
*
* - `$cancelRequest`: If there is a cancellable, pending request related to the instance or
* collection, calling this method will abort the request.
*
* @example
*
* # Credit card resource
Expand Down Expand Up @@ -280,6 +296,11 @@ function shallowClearAndCopy(src, dst) {
*
* Calling these methods invoke `$http` on the `url` template with the given `method`, `params` and
* `headers`.
*
* @example
*
* # User resource
*
* When the data is returned from the server then the object is an instance of the resource type and
* all of the non-GET methods are available with `$` prefix. This allows you to easily support CRUD
* operations (create, read, update, delete) on server-side data.
Expand All @@ -298,10 +319,10 @@ function shallowClearAndCopy(src, dst) {
*
```js
var User = $resource('/user/:userId', {userId:'@id'});
User.get({userId:123}, function(u, getResponseHeaders){
u.abc = true;
u.$save(function(u, putResponseHeaders) {
//u => saved user object
User.get({userId:123}, function(user, getResponseHeaders){
user.abc = true;
user.$save(function(user, putResponseHeaders) {
//user => saved user object
//putResponseHeaders => $http header getter
});
});
Expand All @@ -316,8 +337,11 @@ function shallowClearAndCopy(src, dst) {
$scope.user = user;
});
```

*
* @example
*
* # Creating a custom 'PUT' request
*
* In this example we create a custom method on our resource to make a PUT request
* ```js
* var app = angular.module('app', ['ngResource', 'ngRoute']);
Expand Down Expand Up @@ -345,6 +369,34 @@ function shallowClearAndCopy(src, dst) {
* // This will PUT /notes/ID with the note object in the request payload
* }]);
* ```
*
* @example
*
* # Cancelling requests
*
* If an action's configuration specifies that it is cancellable, you can cancel the request related
* to an instance or collection (as long as it is a result of a "non-instance" call):
*
```js
// ...defining the `Hotel` resource...
var Hotel = $resource('/api/hotel/:id', {id: '@id'}, {
// Let's make the `query()` method cancellable
query: {method: 'get', isArray: true, cancellable: true}
});

// ...somewhere in the PlanVacationController...
...
this.onDestinationChanged = function onDestinationChanged(destination) {
// We don't care about any pending request for hotels
// in a different destination any more
this.availableHotels.$cancelRequest();

// Let's query for hotels in '<destination>'
// (calls: /api/hotel?location=<destination>)
this.availableHotels = Hotel.query({location: destination});
};
```
*
*/
angular.module('ngResource', ['ng']).
provider('$resource', function() {
Expand All @@ -365,7 +417,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 @@ -525,6 +577,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;

Expand Down Expand Up @@ -581,21 +649,35 @@ angular.module('ngResource', ['ng']).
case 'params':
case 'isArray':
case 'interceptor':
case 'cancellable':
break;
case 'timeout':
httpConfig[key] = value;
break;
}
});

if (!isInstanceCall) {
if (!action.cancellable) {
value.$cancelRequest = angular.noop;
} else {
var deferred = $q.defer();
httpConfig.timeout = deferred.promise;
value.$cancelRequest = deferred.resolve;
}
}

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;
Copy link
Member

Choose a reason for hiding this comment

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

Do we even need to do this? Since resolving the timeout after the request completes would have no impact right?

Copy link
Member Author

Choose a reason for hiding this comment

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

See #13210 (comment).
We need this in order to enable deferred to be gc'ed and we need $cancelRequest() to be a function (that does nothing).

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that we can't take care of it inside the then success callback, because there might be an error (in which case we'll never make it to the success callback).

}).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
Expand Down Expand Up @@ -625,6 +707,7 @@ angular.module('ngResource', ['ng']).
}
}

value.$cancelRequest = cancelRequest;
Copy link
Member

Choose a reason for hiding this comment

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

we can just set this to noop here and then we don't need to hold on to the promise up above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not holding to the promise is taken care of in the finally block.

At this point cancelRequest will either be angular.noop or undefined.
The idea was to add noop back if it was there or set the property to undefined if it wasn't defined.

On a second thought, it would make more sense to avoid adding a property that didn't exist in the first place:

if (cancelRequest) value.$cancelRequest = cancelRequest;

value.$resolved = true;

response.resource = value;
Expand Down