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

ngResource does not apply request interceptor #5146

Closed
onsails opened this issue Nov 26, 2013 · 49 comments
Closed

ngResource does not apply request interceptor #5146

onsails opened this issue Nov 26, 2013 · 49 comments

Comments

@onsails
Copy link

onsails commented Nov 26, 2013

ngResource removed transformRequest support but does not apply interceptor.request provided in actions. Shouldn't it?

@scharf
Copy link

scharf commented Dec 5, 2013

the documentation of http://docs.angularjs.org/api/ngResource.$resource specifies transformRequest but it has no effect

@ghost ghost assigned tbosch Jan 4, 2014
@tbosch
Copy link
Contributor

tbosch commented Jan 4, 2014

Hi,
could you create a plunker for this? You can start with an example from the docs...

@scharf
Copy link

scharf commented Jan 5, 2014

Is there any plunker example I can start with? I can't see any plunker in the documentation of $resource to start with...

@tbosch
Copy link
Contributor

tbosch commented Jan 11, 2014

Sorry, you have to start your own... But you can start with another example, e.g. for <input>.

@linclark
Copy link
Contributor

No response from OP, so I'm going to close this one out.

@gitnik
Copy link

gitnik commented Apr 19, 2014

Why? It's still a valid concern/bug.

@linclark
Copy link
Contributor

If you provide a plunker demonstrating the issue, then I will reopen.

@gitnik
Copy link

gitnik commented Apr 19, 2014

here you go http://plnkr.co/edit/WAd11CG7GvBWDSXkLKRh
once you look at the code (ng-resource), you will notice the flaw pretty fast

@linclark
Copy link
Contributor

The code works as documented, so this isn't a bug.

interceptor - {Object=} - The interceptor object has two optional methods - response and 
responseError. Both response and responseError interceptors get called with http response 
object. See $http interceptors.

I'll open it back up as a feature request.

@philBrown
Copy link
Contributor

Any reason ngResource doesn't make use of request interceptors on the action.interceptor object?

As a feature request, this certainly gets my vote

@btford btford removed the gh: issue label Aug 20, 2014
@moderndegree
Copy link

+1 for the feature request

@grabus
Copy link

grabus commented Sep 3, 2014

+1 for the request

@caitp
Copy link
Contributor

caitp commented Sep 3, 2014

proper interceptors can't really be supported without a major refactoring of $http, because interceptors for $http are setup during configuration, not per-request.

If it were going to be possible, would the $resource request interceptor run before, or after, the pre-configured interceptors? Neither would work for everybody. Providing an option to transform the request could be doable, but it would look very different from $http interceptors, so it might be problematic to make them look like they were one in the same.

Looking at the history, I don't see where transformRequest was ever supported in resource before, though, so I'm not sure if we ever had a good reason to remove it, if it was ever there to begin with.

wking added a commit to azurestandard/azure-angular-providers that referenced this issue Jan 21, 2015
To figure out how many items match a given query.  I couldn't figure
out where to attach the data as part of an AzureAPI.<model>.query
request, so I added this new method.  Not as efficient as extracting
the count from the query, but it will get the job done.

I'd like to use transformRequest [1] to make sure we set limit to 0
(no need to render results, because we're using a HEAD request).
However, Angular doesn't currently let you set default data [1],
transform action parameters [2], or handle per-resource request
interceptors [3].

The response interceptor [4,5] does let us extract the 'Count' header
from the response and attach it to the $request instance (we're using
HEAD, so there's no data in the response body).  We need to keep the
$request object, because the mutable object is already attached to the
$scope (or held somehow by the caller), so I attach the value under a
'count' attribute ($scope.favorites_count.count in example.html).

[1]: angular/angular.js#7473
[2]: angular/angular.js#5521
[3]: angular/angular.js#5146
[4]: https://docs.angularjs.org/api/ngResource/service/$resource#usage
[5]: https://docs.angularjs.org/api/ng/service/$http#interceptors
@jtheoof
Copy link

jtheoof commented Apr 27, 2015

👍 This would be great feature added. I need this to convert query params from camel case to snake case, per request. I can't do it with transformRequest unfortunately.

@gkalpak
Copy link
Member

gkalpak commented Apr 29, 2015

@jtheoof, why can't you do it with transformRequest ?

@jtheoof
Copy link

jtheoof commented Apr 29, 2015

Because transformRequest passes the payload of the request, not the whole config object.

GET /somepath/objects/aa0094f040120f708144e5f09d51978a?id_type=deviceid HTTP/1.1
Host: localhost
Connection: keep-alive
Pragma: no-cache
Cache-Control: no-cache
Accept: application/json, text/plain, */*
X-CSRF-TOKEN: blabla
...

In the ObjectsResource:

get: {
    method: 'GET',
    url: url + '/:id',
    transformRequest: function(request) {
        console.log(arguments);
        return request;
    },
}

Prints out: [undefined, function, undefined, callee: (...), caller: (...)].

The query params are gone, no way to do anything with it.

@gkalpak
Copy link
Member

gkalpak commented Apr 29, 2015

How are you setting the query params ?

@jtheoof
Copy link

jtheoof commented Apr 29, 2015

In the service. Our backend only supports snake case, but our jshint rules are not happy about it. So I would like to create a transformer but ngResource does not allow to transform the query params in transformRequest.

In objectService:

        /**
         * Fetches an object based on id and and id type.
         * @param {String} id The id of the object to get.
         * @param {String} idType The type corresponding to the id. Could be 'objectid' by default
         * or 'deviceid'.
         * @return {Resource} An angular $resource instance.
         */
        this.get = function(id, idType) {
            var options = {
                id: id
            };

            if (_.isString(idType)) {
                options.idType = idType;
            }

            // Converting `idType` to `id_type`. This should be done in transformRequest.
            var resource = ObjectsResource.get(utilsService.camelCaseToSnakeCase(options));

            return resource;
        };

@gkalpak
Copy link
Member

gkalpak commented Apr 30, 2015

@jtheoof, I see what you mean. (Although, it sounds much simpler to fix the jshint rules, than adding an interceptor.)

It is probably not the case, but if the query params where fixed, you could do:

var ObjectsSource = new $resource('/some/url?id=:id&id_type=:idType', ...);

@jtheoof
Copy link

jtheoof commented Apr 30, 2015

Good point, but they are not fixed... In my opinion, transformRequest should allow to transform the whole request, not only the payload. Or the function should be named transformRequestPayload. 😉

@qstrahl
Copy link

qstrahl commented Sep 21, 2015

+1 for this feature request. In my case, I need to fetch a token (which may or may not be cached) from a 3rd party REST service and send it with each request made by the resource. Intercepting the request seems like the obvious answer.

@phil-hachey
Copy link

+1 for this feature request

@aebabis
Copy link

aebabis commented Oct 13, 2015

+1

@webpartisan
Copy link

+1
I need request interceptor in resource too

@gitnik
Copy link

gitnik commented Nov 13, 2015

@qstrahl just FYI, but what you're trying to achieve does not depend on this non-existent feature but can be achieved via $http interceptors like so:

$httpProvider.interceptors.push(
        ($injector, $q) => {
            return {
                request(config) {
                    $injector.invoke((UserService) => {
                        config.headers = config.headers || {};

                        config.headers['x-token'] = UserService.getToken();
                    });
                    return config;
                }
            };
        }
    );

@qstrahl
Copy link

qstrahl commented Nov 13, 2015

@gitnik You misunderstand my problem. Your suggestion will call the interceptor for every request ever made by $http. That's exactly what I want to avoid. I need the interceptor on the resource method only.

@gitnik
Copy link

gitnik commented Nov 13, 2015

Ah sorry misread some parts of your question

@qstrahl
Copy link

qstrahl commented Nov 13, 2015

No worries

On Fri, 13 Nov 2015, 17:07 Nik Karbaum notifications@github.com wrote:

Ah sorry misread some parts of your question


Reply to this email directly or view it on GitHub
#5146 (comment)
.

@jdrorrer
Copy link

+1

@k3v3n
Copy link

k3v3n commented Feb 19, 2016

You get my vote for this one +1

@almothafar
Copy link

almothafar commented Mar 16, 2016

+1 for basic feature asked from 3 years !!

@romankierzkowski
Copy link

+1

3 similar comments
@Potseluiko
Copy link

+1

@bencpeters
Copy link

+1

@alexeden
Copy link

+1

@dinuchiriac
Copy link

Will this be implemented anytime ?

@gkalpak
Copy link
Member

gkalpak commented Jul 18, 2016

It will be in 1.6.x (not sure about 1.5.x). You can track #13273 as well.

@qstrahl
Copy link

qstrahl commented Aug 25, 2016

@gkalpak That's awesome to hear, big thanks to everyone involved :D

gkalpak pushed a commit to gkalpak/angular.js that referenced this issue Feb 3, 2017
…ptors

This patch adds `request` and `requestError` interceptors for `$resource`, as
per the documentation found for `$http` interceptors. It is important to note
that returning an error at this stage of the request - before the call to
`$http` - will completely bypass any global interceptors and/or recovery
handlers, as those are added to a separate context. This is intentional;
intercepting a request before it is passed to `$http` indicates that the
resource itself has made a decision, and that it accepts the responsibility for
recovery.

Closes angular#5146
@Narretz Narretz modified the milestones: Backlog, 1.7.0 Nov 1, 2017
gkalpak pushed a commit to gkalpak/angular.js that referenced this issue Dec 5, 2017
…ptors

This patch adds `request` and `requestError` interceptors for `$resource`, as
per the documentation found for `$http` interceptors. It is important to note
that returning an error at this stage of the request - before the call to
`$http` - will completely bypass any global interceptors and/or recovery
handlers, as those are added to a separate context. This is intentional;
intercepting a request before it is passed to `$http` indicates that the
resource itself has made a decision, and that it accepts the responsibility for
recovery.

Closes angular#5146
gkalpak pushed a commit to gkalpak/angular.js that referenced this issue Dec 5, 2017
…ptors

This patch adds `request` and `requestError` interceptors for `$resource`, as
per the documentation found for `$http` interceptors. It is important to note
that returning an error at this stage of the request - before the call to
`$http` - will completely bypass any global interceptors and/or recovery
handlers, as those are added to a separate context. This is intentional;
intercepting a request before it is passed to `$http` indicates that the
resource itself has made a decision, and that it accepts the responsibility for
recovery.

Closes angular#5146
gkalpak pushed a commit to gkalpak/angular.js that referenced this issue Dec 5, 2017
…ptors

This commit adds `request` and `requestError` interceptors for `$resource`, as
per the documentation found for `$http` interceptors. It is important to note
that returning an error at this stage of the request - before the call to
`$http` - will completely bypass any global interceptors and/or recovery
handlers, as those are added to a separate context. This is intentional;
intercepting a request before it is passed to `$http` indicates that the
resource itself has made a decision, and that it accepts the responsibility for
recovery.

Closes angular#5146

BREAKING CHANGE:

Previously, calling a `$resource` method would synchronously call
`$http`. Now, it will be called asynchronously (regardless if a
`request`/`requestError` interceptor has been defined.

This is not expected to affect applications at runtime, since the
overall operation is asynchronous already, but may affect assertions in
tests. For example, if you want to assert that `$http` has been called
with specific arguments as a result of a `$resource` call, you now need
to run a `$digest` first, to ensure the (possibly empty) request
interceptor promise has been resolved.

Before:
```js
it('...', function() {
  $httpBackend.expectGET('/api/things').respond(...);
  var Things = $resource('/api/things');
  Things.query();

  expect($http).toHaveBeenCalledWith(...);
});
```

After:
```js
it('...', function() {
  $httpBackend.expectGET('/api/things').respond(...);
  var Things = $resource('/api/things');
  Things.query();
  $rootScope.$digest();

  expect($http).toHaveBeenCalledWith(...);
});
```
gkalpak pushed a commit to gkalpak/angular.js that referenced this issue Dec 18, 2017
…ptors

This commit adds `request` and `requestError` interceptors for `$resource`, as
per the documentation found for `$http` interceptors. It is important to note
that returning an error at this stage of the request - before the call to
`$http` - will completely bypass any global interceptors and/or recovery
handlers, as those are added to a separate context. This is intentional;
intercepting a request before it is passed to `$http` indicates that the
resource itself has made a decision, and that it accepts the responsibility for
recovery.

Closes angular#5146

BREAKING CHANGE:

Previously, calling a `$resource` method would synchronously call
`$http`. Now, it will be called asynchronously (regardless if a
`request`/`requestError` interceptor has been defined.

This is not expected to affect applications at runtime, since the
overall operation is asynchronous already, but may affect assertions in
tests. For example, if you want to assert that `$http` has been called
with specific arguments as a result of a `$resource` call, you now need
to run a `$digest` first, to ensure the (possibly empty) request
interceptor promise has been resolved.

Before:
```js
it('...', function() {
  $httpBackend.expectGET('/api/things').respond(...);
  var Things = $resource('/api/things');
  Things.query();

  expect($http).toHaveBeenCalledWith(...);
});
```

After:
```js
it('...', function() {
  $httpBackend.expectGET('/api/things').respond(...);
  var Things = $resource('/api/things');
  Things.query();
  $rootScope.$digest();

  expect($http).toHaveBeenCalledWith(...);
});
```
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests