digitalsadhu / loopback-component-jsonapi

JSONAPI support for loopback.
MIT License
101 stars 32 forks source link

Loopback vs JSONAPI filter syntax #70

Open digitalsadhu opened 8 years ago

digitalsadhu commented 8 years ago

Description

Loopback provides the necessary syntax for filtering results.

JSON API is agnostic about how filtering is implemented and only specifies that the filter key is used. In some ways, since loopback does use the filter key, the implementation is already correct though perhaps not ideal.

Loopback example:

https://docs.strongloop.com/display/public/LB/Where+filter

filter[where][property]=value
filter[where][property][op]=value

Note the use of the where namespace which, while not violating anything in JSON API, could be removed for a better API.

JSON API example

http://jsonapi.org/format/#fetching-filtering

The filter query parameter is reserved for filtering data. Servers and clients SHOULD use this key for filtering operations. Note: JSON API is agnostic about the strategies supported by a server. The filter query parameter can be used as the basis for any number of filtering strategies.

The following is how I would expect an ideal solution to work:

filter[property]=value
filter[property][op]=value

Also important to note is that it may cause us issues to use the filter key in a non standard way since all the url based operations in loopback use this key. Eg. Offset, limit, where, fields. It may simply be easier to treat this as technically compliant and leave it at that in the short term.

tsteuwer commented 8 years ago

I also agree that we should leave this as is and it should continue just using what loopback does.

tsteuwer commented 8 years ago

Maybe we should leave that as a comment in the README about using loopback filters?

digitalsadhu commented 8 years ago

Yea good idea. Perhaps link to this issue in the readme and then we can close it for the time being and if anyone feels the need to revisit they can reopen the issue.

maorhayoun commented 8 years ago

can't we rely on content negotiation to indicate which syntax should be parsed? according to the spec, json api content type supersedes any other content-type once it's provided in the Accept header by the Client.

loopback where filter is great but common adapters implement according to spec example. e.g. http://example.com/contacts?filter[name_last]=Smith

digitalsadhu commented 8 years ago

Perhaps that is the way to go. We'd need to do a bit of work to get the content negotiation cleaned up. Also worth checking that there's nothing that's modified at boot time that might get in the way. There are some things that have to be modified when the app boots up so switching with content negotiation becomes difficult.

behnoodk commented 8 years ago

I started using loopback-component-jsonapi a few days ago and it works great. May I suggest that while it's being decided which way to go for filtering, we mention that the filtering follows loopback where filter in the README. I was lost and since everything was supposed to be jsonapi compliant, I kept trying to filter my data based on docs on jsonapi.org docs. I have one concern if someone can address it please. I am using Ember JSONAPIAdapter right now and am writing filters like this: this.store.queryRecord('article',{'filter[where][slug]': params.article_slug});

how likely is it that I will have to refactor my code in the future if it is decided that loopback-component-jsonapi will not follow the built in "where filter" of loopback?

digitalsadhu commented 8 years ago

I certainly hope we can support both forms making refactors unnecessary. That's the plan anyway and I don't see any reason why that wouldn't be possible.

for your interest and feedback!

digitalsadhu commented 8 years ago

Changed "binary" to "both" in the comment above. Blasted autocorrect on phone.

digitalsadhu commented 8 years ago

Hmm, the more I think about this, the more I think it might be best to go one of the other. I do think we should preserve backwards compat if possible however so perhaps we may need to use an options flag to allow the user to choose which style they prefer. The ideal default would be to go with jsonapi but that will break for people on update so probably what we should do is default to loopback syntax and opt in to jsonapi in options with clear instructions on readme. We could even go so far as logging a sort of deprecation notice to the console to inform that we will be switching with the next major so that jsonapi is default and that they will need to specify which syntax in options they want or risk a breakage.

behnoodk commented 8 years ago

probably what we should do is default to loopback syntax and opt in to jsonapi in options with clear instructions on readme

It would be great. most people using this library have been using loopback syntax and are familiar with it and since the response will be jsonapi compliant anyway, it shouldn't break any of our API consumers.