Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set 'filter' parameter to HTTP headers #151

Closed
jesperbruunhansen opened this issue Oct 16, 2016 · 24 comments · Fixed by #270
Closed

Set 'filter' parameter to HTTP headers #151

jesperbruunhansen opened this issue Oct 16, 2016 · 24 comments · Fixed by #270

Comments

@jesperbruunhansen
Copy link

jesperbruunhansen commented Oct 16, 2016

This is a rather "appearance"-related question, but is it possible to set the filter parameters to HTTP headers, instead of having them at parsed at the url?

I know for a fact that Loopback supports filters like this:

GET /api/Users HTTP/1.1
HOST: localhost:3000
filter: {"where":{"schoolId":"test-school-1"}, "limit":5, "order": "createdAt DESC"}

which to me is a lot more readable than eg.

GET /api/Users?filter%5Bwhere%5D%5BschoolId%5D=test-school-1&filter%5Blimit%5D=5&filter%5Border%5D=createdAt%20DESC HTTP/1.1
HOST: localhost:3000

This is especially helpfull when inspecting request in the browsers developer tools.

btw, really cool tool - this SDK builder has helped me a bunch! 👍
Jesper.

@jonathan-casarrubias
Copy link
Collaborator

Hi @jesperbruunhansen thanks for reaching out,

I can take a look a it, it may not be so difficult to do it.. if tests passes I may be adding this request to the next release.

cheers
Jon

@jonathan-casarrubias jonathan-casarrubias self-assigned this Oct 17, 2016
@jonathan-casarrubias jonathan-casarrubias added this to the 2.1.0-beta.11 milestone Oct 18, 2016
jonathan-casarrubias pushed a commit that referenced this issue Oct 18, 2016
- Fix: #157
- Fix: #155
- Fix: #154
- Fix: #153
- Fix: #151
- Fix: #147
jonathan-casarrubias pushed a commit that referenced this issue Oct 18, 2016
- Fix: #157
- Fix: #155
- Fix: #154
- Fix: #153
- Fix: #151
- Fix: #147
jonathan-casarrubias pushed a commit that referenced this issue Oct 18, 2016
- Fix: #157
- Fix: #155
- Fix: #154
- Fix: #153
- Fix: #151
- Fix: #147
jonathan-casarrubias pushed a commit that referenced this issue Oct 18, 2016
- Fix: #157
- Fix: #155
- Fix: #154
- Fix: #153
- Fix: #151
- Fix: #147
jonathan-casarrubias pushed a commit that referenced this issue Oct 18, 2016
- Fix: #157
- Fix: #155
- Fix: #154
- Fix: #153
- Fix: #152
- Fix: #151
- Fix: #148
- Fix: #147
@jonathan-casarrubias
Copy link
Collaborator

Fixed

@jesperbruunhansen
Copy link
Author

Hi again @jonathan-casarrubias and thanks for a swift implementation of this request!

Sorry if I am missing anything, by how do I actually tell the SDK to build the urls with the filter header instead of parsed urls?

@PhilippeCorreges
Copy link

Hi @jesperbruunhansen
If you see an answer to your question, I would be happy to know it as I am on it now !!
I am currently trying to do a find(filter) in vain ...

@jesperbruunhansen
Copy link
Author

Hi @PhilippeCorreges as I understand @jonathan-casarrubias has implemented this feature in release 2.1.0-beta.11 (please correct me if I'm wrong).

I've looked through the commits, but I'm not able to figure out myself how the implementation is working.

@PhilippeCorreges
Copy link

@jesperbruunhansen
Mmmm I am using 2.1.0-rc2 and it seems to be implemented as I am getting a popup to ask for a filter when I write my find().
I am much more facing a syntax issue...

@jesperbruunhansen
Copy link
Author

jesperbruunhansen commented Dec 20, 2016

How does your request urls look, @PhilippeCorreges?

Mine still look like something:

GET /api/Users?filter%5Bwhere%5D%5BschoolId%5D=test-school-1&filter%5Blimit%5D=5&filter%5Border%5D=createdAt%20DESC HTTP/1.1
HOST: localhost:3000

whereas I was looking for ex:

GET /api/Users HTTP/1.1
HOST: localhost:3000
filter: {"where":{"schoolId":"test-school-1"}, "limit":5, "order": "createdAt DESC"}

@PhilippeCorreges
Copy link

seems to be implemented in 2.1.0-rc.5 ...
Let me look for my request ...

@PhilippeCorreges
Copy link

@jesperbruunhansen
this.category.find("sdfsdf").subscribe((data)=>{ this.categories = data; console.log(data); }, err =>{ console.log(err); });
I am looking for categories with a particular Id.
In fact the url is:
http://0.0.0.0:3000/api/Categories?filter=%22sdfsdf%22
So the inside parenthesis is converted into a string. Maybe I do not have to enter a string as a parameter but an Filter Object whom syntax I have no idea .. :(

@PhilippeCorreges
Copy link

but concretely this message shows the issue:
"Invalid value for argument 'filter' of type 'object': sdfsdf. Received type was converted to string." name : "Error" stack : "Error: Invalid value for argument 'filter' of type 'object': sdfsdf. Received type was converted to string.↵ at badArgumentError

@jesperbruunhansen
Copy link
Author

@PhilippeCorreges okay, so your version does also parse the filter-object to the url string. I guess we'll have to wait for @jonathan-casarrubias's reply on how to use the filter header instead.

Regarding your error you have two options:
Using the find() :

this.category.find({
  where: {categoryId:"123xyx"}
});

The other option is to use findById which may be more suited here:
this.category.findById("123xyz")

@PhilippeCorreges
Copy link

@jesperbruunhansen here is my working where clause without parsing the url:
this.category.find({where: {clientId:this.userApi.getCurrentId()}}).subscribe((data)=>{
userApi is a UserApi()

@jesperbruunhansen
Copy link
Author

What URL does this generate? Can I see the headers of your request please.

@jonathan-casarrubias
Copy link
Collaborator

Hey guys, thanks for reaching out...

Something really strange happened in here, it seems the patch was missed while merging the build, I have reviewed the base.service and it does miss this functionality.

The only thing is sending through the headers is the token, but not the filter I will need to re-open and make sure it is available by next version

@jonathan-casarrubias
Copy link
Collaborator

@jesperbruunhansen
Copy link
Author

Again, @jonathan-casarrubias thank you very much for quick reply and implementation!

Last question, will the SDK automatically set the filter to the header in RC6, or should we activate/deactivate globally somewhere?

@jonathan-casarrubias
Copy link
Collaborator

Hi @jesperbruunhansen,

The sdk will automatically set the filter into the header, the query string led to a bunch of issues in the past.

For now will be by default sent through the headers, but if more than 1 person complains about this approach I may add an option to decide.

Not for now. lol

Cheers
Jon

@jesperbruunhansen
Copy link
Author

Haha, great!

Thanks for the help, really appreciate it :)

Jesper.

@JonnyBGod
Copy link
Contributor

Is this working with cors?

I am getting the following error that seams related to this change:

XMLHttpRequest cannot load http://localhost:54447/api/users/findOne. Request header field filter is not allowed by Access-Control-Allow-Headers in preflight response.

@JonnyBGod
Copy link
Contributor

Ok fixed it by adding filter to allowedHeaders in cors middleware config.

@bonaventoora
Copy link

Hi,
@jonathan-casarrubias: I have a use case where it's more convenient to use filter in query params, so count me as one complainant :)

@barocsi
Copy link

barocsi commented Feb 15, 2017

this yields #357

@Sturgelose
Copy link

@jonathan-casarrubias I also found another use case to have a switch option. With Progresive Web Apps and specifically service workers, caching is usually done at URL level. Thus, when having the filter query in he headers, urls to the same API endpoint and different filters are considered as the same calls.

If we had the query filters in the URL, caching would be per API endpoint + per filter, which does proper caching for most use cases. Should I make a Pull request to have a switch on this in lb.config.ts?

@jonathan-casarrubias
Copy link
Collaborator

@Sturgelose thanks for reach out... Hey so, there is already a way to set filters over url within the lb.config.ts

https://github.com/mean-expert-official/loopback-sdk-builder/blob/master/lib/angular2/shared/config.ejs#L63

You just need to execute that method on your app init.

Cheers
Jon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants