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

Querystring behaviour is inconsistent when integrating other Loopback APIs #324

Closed
horiaradu opened this issue Jul 21, 2016 · 7 comments · Fixed by #325
Closed

Querystring behaviour is inconsistent when integrating other Loopback APIs #324

horiaradu opened this issue Jul 21, 2016 · 7 comments · Fixed by #325

Comments

@horiaradu
Copy link
Contributor

horiaradu commented Jul 21, 2016

Loopback has a particular interface for REST APIs. It allows you to write a query like so:

Model.find({where: {id: {inq: [1, 2, 3]}}})

However, this query is also valid: Model.find({where: {id: {inq: []}}}), but it will obviously not return any results.

In this case, the generated query looks like this:

http://localhost:3010/api/model?filter=%7B%22where%22%3A%20%7B%22id%22%3A%20%7B%22inq%22%3A%20%5B%5D%7D%7D%7D

and decoded, like this:

http://localhost:3010/api/model?filter={"where": {"id": {"inq": []}}}

The problem is inside http-invocation.js, line: 215.

// query is an object: query = {where: {id: {inq: []}}}
qs.stringify(query) //this will return an empty string

Querystring behaviour (from node REPL):

> qs.stringify({where: {id: {inq: [1]}}})
'where%5Bid%5D%5Binq%5D%5B0%5D=1'
> qs.stringify({where: {id: {inq: []}}})
''
>

If you are integrating another loopback API, when you do the query with an empty array, you expect to get back an empty collection of items, but instead, because of the behaviour of querystring, you get back all the items.

See related issues:

@BoLaMN
Copy link

BoLaMN commented Jul 21, 2016

same as #320

@richardpringle
Copy link
Contributor

Hey @horiaradu, what version of loopback, strong-remoting, and loopback-datasource-juggler are you using?

I just tried to reproduce the issue and it was working as expected. Either that, or I don't entirely understand the issue. You're saying that when you use an empty array, it returns all the results instead of none?

@BoLaMN
Copy link

BoLaMN commented Aug 11, 2016

The issue I posted is still relevant in how querystring handles arrays throwing object ids on top just makes the whole scenario worse.

I was getting 413 errors due to the querystring being to large for nginx to process, check my referenced issue to find an example in tonicdev you can play with.

In the mean time I too have forked and introduced a similar patch.

cheers

@horiaradu
Copy link
Contributor Author

horiaradu commented Aug 11, 2016

@richardpringle

loopback: 2.29.1
strong-remoting: 2.28.1
loopback-datasource-juggler: 2.47.0

More detailed scenario:

There is a model Car who is backed by a MySQL DB (classic model). This has a belongsTo relationship to another model: Owner. The Owner is backed by a remote datasource (via strong-remoting) which points to a second loopback API.

I am doing a query like bellow:

Car.find({where: {id: -1}, include: ['owner']})

I've put the id == -1 predicate to stress the fact that I don't get any results from Car.

So, I don't get any results from Car, but because I did the include, the juggler, was trying to still query the Owner (this is now optimized via: loopbackio/loopback-datasource-juggler#1007). When doing the query, it wanted to "get all the owners with id included in []", which translated to the REST call I detailed above:

http://localhost:3010/api/model?filter={"where": {"id": {"inq": []}}}

And here is where querystring appears into play and strips out the array and reformulating the query to be: "get all the owners".

@richardpringle
Copy link
Contributor

Sorry @horiaradu, I didn't quite understand what you were describing right away. I still can't reproduce the issue with strong-remoting alone (I'm on 2.29.0, no sure if that makes a difference).

I get an empty array when I try to hit the following:
http://localhost:3010/api/model?filter={"where": {"id": {"inq": []}}}

I do however see that when you pass {where: {id: {inq: []}}} into qs.stringify(), it returns an empty string... I'm just not quite sure where this is happening.

Thanks for your patience, I'm sure this is easier to understand than I am making it.

@horiaradu
Copy link
Contributor Author

@richardpringle

I have updated my PR so that the tests pass (forgot a line...).

jskrzypek pushed a commit to jskrzypek/strong-remoting that referenced this issue Sep 28, 2016
Stringify query params which are objects in a way in which empty ararys
are preserved instead of removed (default querystring implementation).

fix: strongloop#324
@richardpringle richardpringle removed their assignment Oct 6, 2016
@gunjpan
Copy link
Contributor

gunjpan commented Nov 3, 2016

Landed.

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

Successfully merging a pull request may close this issue.

4 participants