setMethodOverride - get / getList

[SamanthaAdrichem]

Once i enable setMethodOverride for get ( you want to do this if the requests parameters become quite long ). It doesn't work for all the getList calls.

[SamanthaAdrichem]

Additionally, it doesn't put the request params in the POST body, but still puts them in the URI.

[pedromarce]

Hi @SamanthaAdrichem, it seems a curious use case where we need to post something to get some results, could you expand a little bit more? I would like to take a look at your pull request (thanks !!), but would rather understand what is the need for this operation in first place. As it is (I would say) quite controversial operation.

[SamanthaAdrichem]

Hi, as far as i know, it's actually a very common practice with large data systems. A good example is this one:

The system has a transaction overview with 1000's of transactions per day. Now someone has a list of 5000 transaction id's which he wants to export from the system.

This is a normal get request, for it's a get service with a filter on transaction ID, however, the amount of transaction id's is too large thus it exceeds the maximum URI length of the browser, (this case occurs even sooner if you use a query type like &id[]=1&id[]=2 then when you use a query type like id=1,2).

To solve this, you POST the GET request with an override header so that you will have up to (by default) 8MB or even more for your request.

Normally you would configure it like, if uri becomes longer than, solve this issue for me, otherwise do a get. But you could just always POST all GET request, and it would work, saving a lot of "is the uri too long in this browser hassle".

( see first answer here: http://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers )

[pedromarce]

I understand the convenience of using it, still would argue that contradicts restful practices.

Anyway if I have to decide between a theoretic principle or a practical one, I would lean to the practical one every time.

So, I'll check with @ConcurrentHashMap and @daviesgeek but I will support to merge it, but @SamanthaAdrichem, could you add test cases also in your pull request?

[daviesgeek]

That makes sense to me from a practical standpoint, however I'd want to be careful about changing this (if I'm understanding this correctly) across the board. I don't use method overriding, so I don't know the precise details of how this works, and if it's changing the way the params are sent, I'd want to be careful about making breaking changes.

[SamanthaAdrichem]

I understand that @daviesgeek and totally agree:

On the one hand: maybe i should add a configuration option that determines wether you want to put the params in the uri or in the body,

On the other hand, there is in my opinion no other single possible use of overriding a GET request to a POST request than that the request URI data is too large. So that would mean this solution is okay, but i should only implement this on GET requests.

Some research is required here.

p.s. I will write the tests soon.

[SamanthaAdrichem]

@pedromarce ok so today i have time once again. I've adjusted it so that only the request will be send as a body on GET requests. Now i want to write the tests, but on what framework wore they written?

Looks to me like mocha maybe? but when i run them i get

/restangular/test/restangularSpec.js:9
beforeEach(angular.mock.module("restangular"));
             ^
ReferenceError: angular is not defined

[SamanthaAdrichem]

Ok nvm figured it out using your travis job ;)

[SamanthaAdrichem]

That should do it!

[stephengardner]

Hey I'm not sure if this is fixed but RestangularProvider.getMethodOverriders(['getList', 'get']); and then subsequently calling .getList is still performing a GET request. But, calling a .get DOES infact perform a POST.
however, it puts the params in the URL, which is absolutely not intended due to it being POST now.

Any quick example?

[SamanthaAdrichem]

We use it all the time and it works, but it has never been merged in the master branch

[bostrom]

I guess we're talking about PR #1338 here, since it's not really referenced in this issue anywhere?

I'm going to thing aloud here, bare with me.

I'm not familiar with method overriding, but intuitively it seems that if I use POST as HTTP method, then why would I not want the data in the body, but left in the URL? If I was making a server that accepts POST operations, I would probably look for the data in the body, and not in the query parameters. Having Restangular make POST requests, some of which have their data in the body and some in the query parameters seem a little messy to me.

However, one can argue that since it's possible to send POST/PUT/PATCH/DELETE requests with data both in the body and in query parameters, the query parameters should not be merged with the body data in a POST request, should they both exist.

Since a body in GET requests is not really supported, according to the RFC:

A payload within a GET request message has no defined semantics; sending a payload body on a GET request might cause some existing implementations to reject the request.

it would be safe to convert such a request to a POST and put the query data in the body.

The use case that @SamanthaAdrichem presents is reasonable and I don't see any reason for doing a method override from GET to POST if you wouldn't want the query parameters to be sent in the POST body. Please prove me wrong!

An option to switch between data in body vs data in query params would perhaps appeal to some people, but I'm a bit reluctant to adding even more switch-flipping to this library than we already have.

So to me it sounds logical and reasonable enough to "fix" the setMethodOverriders (strange name too) to

• be able to use POST for any HTTP method
• include the X-HTTP-Method-Override header
• keep body data in the body for requests supporting body data
• keep query parameters as query parameters for other methods than GET
• convert the query parameters to a body for GET requests sent as POST

The last bullet would naturally include all Restangular methods that perform a HTTP GET request, getList also.

@SamanthaAdrichem do the above hold true for your PR, are they all included in the spec? What about keeping the body as body, I suppose that's handled also, but would it be possible to get a test for that as well?

What does @pedromarce and @daviesgeek say about my reasoning? The data placement (body vs query) would introduce a breaking change, but I'm willing to live with that.

And @stephengardner the method is setMethodOverriders and not getMethodOverriders, but maybe that was just a typo.

[SamanthaAdrichem]

wow i never saw your comment 🙈
at your question, the way it was designed back then the answer whould be yes. The change only did "convert the query parameters to a body for GET requests sent as POST". So all the earlier remained the same.

We've just forked it back then and kept our own copy. Today i'm actually going to update since we want to use cancel request and i'm going to make the changes again in your latest version

[SamanthaAdrichem]

Created new PR #1482