paknahad / jsonapi-bundle

the fastest way to generate API based on jsonapi.org using woohoolabs/yin
MIT License
70 stars 25 forks source link

Support more advanced filtering in query #5

Closed mnugter closed 5 years ago

mnugter commented 5 years ago

The current filtering only supports a limited set of possibilities.

I would love the ability to have advanced filtering options available like AND/OR, greater than, etc.

I have done some research and JSON API does not dictate any standards and nothing is close to beeing the default implementation.

Some interesting options for standards I found:

To me searching for a standard outside JSONAPI makes the most sense for implementation although even there there doesn't seem to be a reliable standard.

paknahad commented 5 years ago

Thanks for the suggestion. it's a very good idea, I like this one. but I think it's too complicated. I'm looking for the simplest way to get this feature

mnugter commented 5 years ago

The easiest way forward is the best way forward to me. If there is a library doing the heavy work that would be awesome.

I have been doing some more research into this and it really seems there is no accepted standard and nothing seems to be emerging. It seems weird for the json-api spec to miss this as to me it's a fundamental part of an API specification.

I did find the oData standard but that describes a full format for an API and we're only looking for a query string filter format. Other types of backends describe an advanced format as well, like elastic search. The format is interesting but there are no libraries available.

The more I think about it, it should be best if this package would not be too opinionated on which filter structure should be used. If would therefore be very nice to be able to plug in different Finders which can handle different types of filtering. In a few days I'm going to research what would be the best way to make in pluggable. As I need advanced filtering for a project (range filter and aggregated fields filter) I'll try to make a PR with the changes as I described.

First step would be to make the Finder pluggable, I would like to maintain the current version as it's really simple to use. Adding variants for the filtering by either separate bundles or package them in this bundle would be fine, not really sure on preference here.

Choices to make in making the Finder pluggable:

  1. Will it be something you can configure (1 active Finder at the time)
  2. Will it be choosable from the URL (type=basic, type=rql, etc)
  3. Will it be like a strategy pattern, having more finders active at the same time.

Options I found for different filtering types:

mnugter commented 5 years ago

i created a fork with pull request for this issue. It doesn't actually add any more advanced filters but it's a step towards it.

Please review my PR and let me know if there is anything I should change!

https://github.com/paknahad/jsonapi-bundle/pull/7

mnugter commented 5 years ago

I added comments in the PR to clarify the changes I have made, I hope that helps in understanding the code.

paknahad commented 5 years ago

thanks for your PR I'll check it as soon as possible.

today I had free time and planned a new one please check this and let me know your opinion:

Simple query

simple query makes by this structure:

url?filter[FIELD_NAME]=VALUE

Example:

http://example.com/books?filter[title]=php&filter[author.name]=hamid

SQL:

SELECT * FROM books AS b INNER JOIN authors AS a ...
WHERE
    b.title = 'php'
    AND
    a.name = 'hamid'

Advanced query:

1- Define conditions.

url?filter[CONDITION NAME][FIELD NAME][OPERATOR]=VALUE

2- Combine these conditions together.

url?filter[CONDITION NAME][COMBINE]=CONDITIONS SEPARATED BY “,”

Example:

books?filter[title]=php&filter[_c1][author.name][_like]=%hamid%&filter[_c2][publish_date][_gt]=2017-1-1&filter[_c3][publish_date][_lt]=2017-6-1]&filter[_c4][_cmb_and]=_c2,_c3&filter[_cmb_or]=_c4,_c1

SQL:

SELECT * FROM books AS b INNER JOIN authors  AS a …
WHERE
    b.title = 'php' 
    AND
    (
        (
            b.publish_date > '2017-1-1'
            AND
            B.publish_date < '2017-6-1'
        )
        OR
        a.name LIKE '%hamid%'
     )
mnugter commented 5 years ago

I like your suggestion as well, it seems to support everything that you'd want in filtering.

That makes for 4 good implementations for the filtering:

  1. The current one, this is fine for a lot of use cases I think
  2. The Fancy filters one
  3. RQL implementation
  4. Your last implementation

Another one could be a serialized json string (for example the json format from one of the last comments on the Fancy Filters definition), difficult to type manually in the url but great to use from code.

As the JsonApi standard doesn't define a way of filtering maybe this package shouldn't either out of the box? If the basic implementation is separated into another package (jsonapi-bundle-basic-filter?) it would enable users to be in full control which filtering options they would like to include. The readme could of course explain this point and provide guidelines to adding basic filtering.

We can then create other packages for the more advanced use cases. I am already working on a RQL implementation (https://github.com/mnugter/jsonapi-rql-finder-bundle) based on this PR.

Krilo89 commented 5 years ago

I validated the code and can verify that with the new code it's possible to add advanced filtering. Would be nice if this gets merged to start adding advanced filter functions.

mnugter commented 5 years ago

So now that the PR is in (thanks for the commit!) we're actually able to provide multiple Finders I think it's maybe better to open new issues to update the README to describe this feature?

Also: it would be possible to move the current Finder out from this package itself and into a separate package. I'd be happy to help out there but I think you'd have to create a new repository first as otherwise the namespace would be mine instead of yours (and it's your code). Should I open a new issue to discuss this?

btw: do you know what the above message with the X means? Are there tests now failing?

paknahad commented 5 years ago

I think it's better this finder stay in this project as default finder and others be as optional. I create a new repository as one of the optional finders: query_parser.

yes please open a new issue to update README and also create new PR to update that. because you create this PR and you can explain this better than me.

I checked code's quality by Sensiolabs and found some issues in coding style. it's not very important. I'll fix it as soon as possible.