Skip to content

WIP: Filter Backends #455

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
[unreleased]

* Add testing configuration to `REST_FRAMEWORK` configuration as described in [DRF](https://www.django-rest-framework.org/api-guide/testing/#configuration)
* Add sorting configuration to `REST_FRAMEWORK` as defined in [json api spec](http://jsonapi.org/format/#fetching-sorting)
* Add `HyperlinkedRelatedField` and `SerializerMethodHyperlinkedRelatedField`. See [usage docs](docs/usage.md#related-fields)
* Add optional [jsonapi-style](http://jsonapi.org/format/) filter backends. See [usage docs](docs/usage.md#filter-backends)
* query parameter validation -- raises 400 errors rather than silently ignoring "bad" parameters
* sort - based on `rest_framework.filters.OrderingFilter`
* keyword filter across multiple fields based on `rest_framework.filters.SearchFilter`
* field-level filter based on `django_filters.rest_framework.DjangoFilterBackend`


v2.5.0 - Released July 11, 2018
Expand Down
6 changes: 4 additions & 2 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,11 @@ override ``settings.REST_FRAMEWORK``
),
'DEFAULT_METADATA_CLASS': 'rest_framework_json_api.metadata.JSONAPIMetadata',
'DEFAULT_FILTER_BACKENDS': (
'rest_framework.filters.OrderingFilter',
'rest_framework_json_api.backends.JsonApiQueryValidationFilter',
'rest_framework_json_api.backends.JsonApiOrderingFilter',
'rest_framework_json_api.backends.JsonApiFilterFilter',
'rest_framework_json_api.backends.JsonApiSearchFilter',
),
'ORDERING_PARAM': 'sort',
'TEST_REQUEST_RENDERER_CLASSES': (
'rest_framework_json_api.renderers.JSONRenderer',
),
Expand Down
155 changes: 151 additions & 4 deletions docs/usage.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@

# Usage

The DJA package implements a custom renderer, parser, exception handler, and
The DJA package implements a custom renderer, parser, exception handler, query filter backends, and
pagination. To get started enable the pieces in `settings.py` that you want to use.

Many features of the JSON:API format standard have been implemented using Mixin classes in `serializers.py`.
Many features of the [JSON:API](http://jsonapi.org/format) format standard have been implemented using
Mixin classes in `serializers.py`.
The easiest way to make use of those features is to import ModelSerializer variants
from `rest_framework_json_api` instead of the usual `rest_framework`

Expand Down Expand Up @@ -32,9 +33,11 @@ REST_FRAMEWORK = {
),
'DEFAULT_METADATA_CLASS': 'rest_framework_json_api.metadata.JSONAPIMetadata',
'DEFAULT_FILTER_BACKENDS': (
'rest_framework.filters.OrderingFilter',
'rest_framework_json_api.backends.JsonApiQueryValidationFilter',
'rest_framework_json_api.backends.JsonApiOrderingFilter',
'rest_framework_json_api.backends.JsonApiFilterFilter',
'rest_framework_json_api.backends.JsonApiSearchFilter',
),
'ORDERING_PARAM': 'sort',
'TEST_REQUEST_RENDERER_CLASSES': (
'rest_framework_json_api.renderers.JSONRenderer',
),
Expand Down Expand Up @@ -90,6 +93,150 @@ class MyLimitPagination(JsonApiLimitOffsetPagination):
max_limit = None
```

### Filter Backends

[JSON:API](http://jsonapi.org) specifies certain query parameter names but not always the meaning of a particular name.
The following four filter backends extend existing common filter backends found in DRF and the `django-filter` package
to comply with the required parts of the [spec](http://jsonapi.org/format) and offer up a Django-flavored
implementation of parts that are left undefined (like what a `filter` looks like.) The four backends may be
[configured](http://django-rest-framework.readthedocs.io/en/latest/api-guide/filtering/#setting-filter-backends)
either as `REST_FRAMWORK['DEFAULT_FILTER_BACKENDS']` or as a list of `filter_backends`. Following is an example
with explanations of each filter backend after it:

**TODO: change to match example model**

```python
from rest_framework_json_api.backends import *

class LineItemViewSet(viewsets.ModelViewSet):
queryset = LineItem.objects
serializer_class = LineItemSerializer
filter_backends = (JsonApiQueryValidationFilter, JsonApiOrderingFilter, JsonApiFilterFilter, JsonApiSearchFilter,)
filterset_fields = {
'subject_area_code': usual_rels,
'course_name': ('exact', ) + text_rels,
'course_description': text_rels + usual_rels,
'course_identifier': text_rels + usual_rels,
'course_number': ('exact', ),
'course_terms__term_identifier': usual_rels,
'school_bulletin_prefix_code': ('exact', 'regex'),
}
search_fields = ('course_name', 'course_description', 'course_identifier', 'course_number')
```

#### `JsonApiQueryValidationFilter`
`JsonApiQueryValidationFilter` checks the query parameters to make sure they are all valid per JSON:API
and returns a `400 Bad Request` if they are not. By default it also flags duplicated `filter` parameters (it is
generally meaningless to have two of the same filter as filters are ANDed together). You can override these
attributes if you need to step outside the spec:

**TODO: check this**
```python
jsonapi_query_keywords = ('sort', 'filter', 'fields', 'page', 'include')
allow_duplicated_filters = False
```

If, for example, your client sends in a query with an invalid parameter (`?sort` misspelled as `?snort`),
this error will be returned:
```json
{
"errors": [
{
"detail": "invalid query parameter: snort",
"source": {
"pointer": "/data"
},
"status": "400"
}
]
}
```

And if two conflicting filters are provided (`?filter[foo]=123&filter[foo]=456`), this error:
```json
{
"errors": [
{
"detail": "repeated query parameter not allowed: filter[foo]",
"source": {
"pointer": "/data"
},
"status": "400"
}
]
}
```

If you would rather have your API silently ignore incorrect parameters, simply leave this filter backend out
and set `allow_duplicated_filters = True`.

#### `JsonApiOrderingFilter`
`JsonApiOrderingFilter` implements the [JSON:API `sort`](http://jsonapi.org/format/#fetching-sorting) just uses
DRF's [ordering filter](http://django-rest-framework.readthedocs.io/en/latest/api-guide/filtering/#orderingfilter).
You can use a non-standard parameter name isntead of `sort` by setting `ordering_param`:
```json
ordering_param = 'sort'
ignore_bad_sort_fields = False
```
Per the JSON:API, "If the server does not support sorting as specified in the query parameter `sort`,
it **MUST** return `400 Bad Request`." This error looks like ()for `?sort=`abc,foo,def` where `foo` is a valid
field name and the other two are not):
```json
{
"errors": [
{
"detail": "invalid sort parameters: abc,def",
"source": {
"pointer": "/data"
},
"status": "400"
}
]
}
```
If you want to silently ignore bad sort fields, set `ignore_bad_sort_fields = True`

#### `JsonApiFilterFilter`
`JsonApiFilterFilter` exploits the power of the [django-filter DjangoFilterBackend](https://django-filter.readthedocs.io/en/latest/guide/rest_framework.html).
The JSON:API spec explicitly does not define the syntax or meaning of a filter beyond requiring use of the `filter`
query parameter. This filter implementation is "just a suggestion", but hopefully a useful one with these features:
- A resource field exact match test: `?filter[foo]=123`
- Apply other [relational operators](https://docs.djangoproject.com/en/2.0/ref/models/querysets/#field-lookups):
`?filter[foo.icontains]=bar or ?filter[count.gt]=7...`
- Membership in a list of values (OR): `?filter[foo]=abc,123,zzz` (foo in ['abc','123','zzz'])
- Filters can be combined for intersection (AND): `?filter[foo]=123&filter[bar]=abc,123,zzz&filter[...]`
- Chaining related resource fields for above filters: `?filter[foo.rel.baz]=123` (where `rel` is the relationship name)

Both the usual Django ORM double-underscore notation (`foo__bar__eq`) and a more JSON:API-flavored dotted
notation (`foo.bar.eq`) are supported.

See the [django-filter](https://django-filter.readthedocs.io/en/latest/guide/rest_framework.html) documentation
for more details. See an example using a dictionary of filter definitions [above](#filter-backends).

A `400 Bad Request` like the following is returned if the requested filter is not defined:
```json
{
"errors": [
{
"detail": "invalid filter[foo]",
"source": {
"pointer": "/data"
},
"status": "400"
}
]
}
```

#### `JsonApiSearchFilter`
`JsonApiSearchFilter` implements keyword searching across multiple text fields using
[`rest_framework.filters.SearchFilter`](http://django-rest-framework.readthedocs.io/en/latest/api-guide/filtering/#searchfilter)
You configure this filter with `search_fields` and name the filter with `search_param`. For lack of a better name,
the default is:
```python
search_param = 'filter[all]'
```

### Performance Testing

If you are trying to see if your viewsets are configured properly to optimize performance,
Expand Down
2 changes: 2 additions & 0 deletions requirements-development.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
-e .
django>=2.0
django-debug-toolbar
django-polymorphic>=2.0
django-filter>=2.0
factory-boy
Faker
isort
Expand Down
148 changes: 148 additions & 0 deletions rest_framework_json_api/backends.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
from django.db.models.sql.constants import ORDER_PATTERN
from django_filters.rest_framework import DjangoFilterBackend
from rest_framework.exceptions import ValidationError
from rest_framework.filters import (BaseFilterBackend, OrderingFilter,
SearchFilter)
from rest_framework.settings import api_settings


class JsonApiFilterMixin(object):
"""
class to share data among filtering backends
"""
jsonapi_query_keywords = ('sort', 'filter', 'fields', 'page', 'include')
search_param = api_settings.SEARCH_PARAM
ordering_param = 'sort'

def __init__(self):
self.filter_keys = []


class JsonApiQueryValidationFilter(JsonApiFilterMixin, BaseFilterBackend):
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite sure how to get this right. Especially when an API designer wants to define additional query params with different naming per view it would also need to be configurable.

All would be possible to do somehow but I kind of think it makes the life more difficult of an api designer and the gain is very little in the default case.

Wouldn't it be good enough just to verify the params in the other filter backend classes whether they exists and leave this filter class out for now?

From a maintenance perspective I would prefer not to provide this for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems inappropriate to verify the params in other classes as they only "own" their own params. Verification is already happening there. For instance, filter[blah] checks to see if blah is a defined filter.
Some of the params are not even backend filters, like include which is elsewhere in the serializer code.

The problem this class addresses is that a client would inadvertently misspell a param and then (if they don't notice that) wonder why the action they requested isn't happening. I'm all for making it easier on the client developer to notice these kinds of errors. If they want to "break" JSON:API and add additional parameters they can:

  1. Not include this backend filter at all and the default silent behavior of not complaining about params that are not recognized is achieved -- which was kind of the reason for the ignore_bad_sort_fields as well; If you really want your API to be silent to all query parameter errors, this is needed.
  2. Extend this backend filter to add additional params to the list by changing the jsonapi_query_keywords attribute (or add a JSON_API_QUERY_KEYWORDS to settings although I've currently left that out to make it difficult to break the standard.)

Needless to say I feel pretty strongly that this is a useful backend filter to help achieve JSON:API compliance for query parameter handling:

"If a server encounters a query parameter that does not follow the naming conventions above, and the server does not know how to process it as a query parameter from this specification, it MUST return 400 Bad Request."

Copy link
Contributor Author

@n2ygk n2ygk Aug 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't address the different naming per view part. I believe the attribute is actually defined for the view. It's certainly declared that way. I'm guessing backend_filters classes are mixed in with the view class? I need to write unit tests for these permutations to confirm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this would make DJA better comply with JSON API spec which is one of ours goals of DJA.

I think though there is simple no hurry to implement this in DJA and I think we shouldn't rush into implementing this without having a proper understanding how to do it well.
Because once its landed in DJA and we will find that it is not a good fit we will have to go through a deprecation cycle to change it which makes maintaining DJA harder and the code more complex.

Another reason why I think this is hard to implement is that query params can be accessed wherever request is accessible. Query params are not only used in filter backends but also in other parts e.g. pagination is one example but I am sure there are others.
Also it kind of feels odd to add validation into a filter backend which should be there for filtering.
Maybe a middleware might be the better spot but I am not sure and would try to experiment with it first to get a clear understanding.

We could think of an experimental area for this. We haven't done this in the past and would need to rethink how to structure it though.

So why do we not just try it out in our projects (I can also think of one of our projects where it could be helpful)? Once we have it running in production and it is solid we can still consider adding it to DJA. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to finish up this PR, leaving it marked WIP, and then maybe we can make an experimental branch and merge it there? We can always rebase later. I have a working example and test case in my local code but wanted to merge in to the example and test cases here so others can try it and provide feedback.

I would argue that filtering for valid query parameters is a legit use of a filter;-) But this could be done as a view mixin too. I think middleware is too early in the stack as we haven't even gotten to DRF at that point, whereas backend filters are a DRF layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sliverc FYI - I've closed the PR while I keep working on this. Do we want to create an experiment branch so others can test/contribute?

BTW, using DjangoFilterBackend adds a new dependency for django-filter. That could be a problem. It's not core Django or DRF but is the top-rated filtering package and the one reference from DRF docs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of django-filter. We have the same issue with django-polymorphic. This is marked as optional dependency, we can do the same with django-filter. We just have to make sure that DJA can still be used when this dependency is not installed - meaning we have to be careful with imports.

Actually django-filter has the exact same issue just the other way around as it is also usable without Django REST Framework. All rest_framework related classes are part of the rest_framework module. We could create a django_filters module in our case.

In terms of how to move forward:
I am an advocate of small PRs as they are simpler to review and there quicker to get merged.

What I would suggest how we move forward:

  1. Create a PR for JSONAPIOrderingFilter - this is a fairly straight forward change and can be merged easily
  2. Create a PR for JSONAPIDjangoFilterBackend with its own module approach
  3. Create a PR for JsonApiQueryValidationFilter where we can experiment how this works best. (I will comment on the experimental discussion you started later on)

How does this sound?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sliverc you wrote:

In terms of django-filter. We have the same issue with django-polymorphic. This is marked as optional dependency, we can do the same with django-filter. We just have to make sure that DJA can still be used when this dependency is not installed - meaning we have to be careful with imports.

Actually there are no imports for django-polymorphic. I don't quite understand how this code works:

class PolymorphicSerializerMetaclass(SerializerMetaclass):
w.r.t. django-polymorphic.

Actually django-filter has the exact same issue just the other way around as it is also usable without Django REST Framework. All rest_framework related classes are part of the rest_framework module. We could create a django_filters module in our case.

I don't quite understand what you mean by creating a django_filters module. Are you saying submit this as a PR to django-filter? As it stands right now, I'll either have to wrap the JSONAPIDjangoFilter implementation with a try/except of from django_filters.rest_framework import DjangoFilterBackend or just make django-filter a dependency for DJA. Please advise.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, what I mean is to create a folder django_filters in DJA and add a init,py and backends.py to implement a custom backend. (Hint: Simplified each python file is called a module but more details at https://docs.python.org/3.6/tutorial/modules.html)

If a user of DJA doesn't import this newly created django_filters module or uses it in settings then he doesn't have to install django_filters.

A backend filter that validates query parameters for jsonapi spec conformance and raises a 400 error
rather than silently ignoring unknown parameters or incorrect usage.

set `allow_duplicate_filters = True` if you are OK with the same filter being repeated.

TODO: For jsonapi error object compliance, must set jsonapi errors "parameter" for the ValidationError.
This requires extending DRF/DJA Exceptions.
"""
allow_duplicated_filters = False

def validate_query_params(self, request):
"""
Validate that query params are in the list of valid `jsonapi_query_keywords`
Raises ValidationError if not.
"""
for qp in request.query_params.keys():
bracket = qp.find('[')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a regex would be better here such as:

match = re.match(r'filter\[(.*?)\].*', qp)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to care about performance as this code executes for every request. Also the match would be something more complicated since it's searching for all keywords, whether or not they have brackets (e.g. filter[foo], sort, ...). It seems a regex is overkill and detracts from readability.

if bracket >= 0:
if qp[-1] != ']':
raise ValidationError(
'invalid query parameter: {}'.format(qp))
keyword = qp[:bracket]
else:
keyword = qp
if keyword not in self.jsonapi_query_keywords:
raise ValidationError(
'invalid query parameter: {}'.format(keyword))
if not self.allow_duplicated_filters and len(
request.query_params.getlist(qp)) > 1:
raise ValidationError(
'repeated query parameter not allowed: {}'.format(qp))

def filter_queryset(self, request, queryset, view):
self.validate_query_params(request)
return queryset


class JsonApiOrderingFilter(JsonApiFilterMixin, OrderingFilter):
"""
The standard rest_framework.filters.OrderingFilter works mostly fine as is, but with .ordering_param = 'sort'.

This implements http://jsonapi.org/format/#fetching-sorting and raises 400 if any sort field is invalid.
"""
ignore_bad_sort_fields = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't provide such a option as I think DJA should simply follow the specification. If someone doesn't want to raise an error when sort field is invalid they can simply use the OrderingFilter of DRF.

Copy link
Contributor Author

@n2ygk n2ygk Aug 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Serves me right for trying to anticipate your review;-) I originally left it out:-) But see below...


def remove_invalid_fields(self, queryset, fields, view, request):
"""
override remove_invalid_fields to raise a 400 exception instead of silently removing them.
"""
valid_fields = [item[0] for item in self.get_valid_fields(queryset, view, {'request': request})]
bad_terms = [term for term in fields if term.lstrip('-') not in valid_fields and ORDER_PATTERN.match(term)]
if bad_terms and not self.ignore_bad_sort_fields:
raise ValidationError(
'invalid sort parameter{}: {}'.format(('s' if len(bad_terms) > 1 else ''), ','.join(bad_terms)))
return super(JsonApiOrderingFilter, self).remove_invalid_fields(queryset, fields, view, request)


class JsonApiSearchFilter(JsonApiFilterMixin, SearchFilter):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you think it is better not to provide specific json api classes where there is no need? In this case we could simply provide the settings how to configure SearchFilter.

I would call the search param filter[search] kind of feels natural and follows the specification.

Copy link
Contributor Author

@n2ygk n2ygk Aug 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we just include JSONAPISearchFilter in DEFAULT_FILTER_BACKENDS or the filter_backends attribute then the default value of filter[search], filter[all] or whatever is predefined. Otherwise, including plain SearchFilter will require one to also set settings.JSON_API_SEARCH_PARAM or search_param attribute in order to comply with the requirement to use filter. So it's less typing to just use the JSONAPI-flavored class even if it doesn't do much.

Regarding the name of the filter, I like all (adverb) because it implies search all the (searchable) fields whereas search feels more like a verb usage to me which is anti-REST. But you can use one and I can use the other as it's configurable.

all is also more djangoistic: see __all__ in DRF serializers, so maybe filter[__all__]...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of think filter[all] is misleading as it implies that it takes into account all fields for filtering. This is actually not true as it only search in the defined search_fields. filter[search] might also be a bit odd though I agree.

Even though everyone can define the name as they like if we add it to DJA we have to come up with a recommendation. But this doesn't seem to be easy as the json api is silent on it and we are just two users and have two different opinions already 😄

While thinking it seems that adding a JSONAPISearchFilter to DJA doesn't add any value as someone can already use the DRF SearchFilter today without this change and define whatever parameter he likes.

Also in DJA we don't have to reimplement all features which are in DRF but only implement what is different according to the json api specification.

For this reasons I think we shouldn't provide a JSONAPISearchFilter. We might add a reference to the DRF documentation that there are other filter backends though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's call it filter[all-the-things] ;-)

I agree with your reasoning for calling it something that doesn't imply every field. How about filter[fields] or filter[search_fields] as the default?

I'm OK with documenting to just use SearchFilter and set search_param. I just needed a way to make sure when validating filter field names that the one we use for keyword searching doesn't come up as an error. I can just as easily change the code to look for DRF's settings.REST_FRAMEWORK['SEARCH_PARAM'].

I'll drop the code and document example usage of SearchFilter.

"""
The (multi-field) rest_framework.filters.SearchFilter works just fine as is, but with a
defined `filter[NAME]` such as `filter[all]` or `filter[_all_]` or something like that.

This is not part of the jsonapi standard per-se, other than the requirement to use the `filter`
keyword: This is an optional implementation of a style of filtering in which a single filter
can implement a keyword search across multiple fields of a model as implemented by SearchFilter.
"""
pass


class JsonApiFilterFilter(JsonApiFilterMixin, DjangoFilterBackend):
"""
Overrides django_filters.rest_framework.DjangoFilterBackend to use `filter[field]` query parameter.

This is not part of the jsonapi standard per-se, other than the requirement to use the `filter`
keyword: This is an optional implementation of style of filtering in which each filter is an ORM
expression as implemented by DjangoFilterBackend and seems to be in alignment with an interpretation of
http://jsonapi.org/recommendations/#filtering, including relationship chaining.
Filters can be:
- A resource field equality test: ?filter[foo]=123
- Apply other relational operators: ?filter[foo.in]=bar,baz or ?filter[count.ge]=7...
- Membership in a list of values (OR): ?filter[foo]=abc,123,zzz (foo in ['abc','123','zzz'])
- Filters can be combined for intersection (AND): ?filter[foo]=123&filter[bar]=abc,123,zzz&filter[...]
- A related resource field for above tests: ?filter[foo.rel.baz]=123 (where `rel` is the relationship name)

It is meaningless to intersect the same filter: ?filter[foo]=123&filter[foo]=abc will always yield nothing so
detect this repeated appearance of the same filter in JsonApiQueryValidationFilter and complain there.
"""

def get_filterset(self, request, queryset, view):
"""
Validate that the `filter[field]` is defined in the filters and raise ValidationError if it's missing.

While `filter` syntax and semantics is undefined by the jsonapi 1.0 spec, this behavior is consistent with the
style used for missing query parameters: http://jsonapi.org/format/#query-parameters. In general, unlike
django/DRF, jsonapi raises 400 rather than ignoring "bad" query parameters.
"""
fs = super(JsonApiFilterFilter, self).get_filterset(
request, queryset, view)
# TODO: change to have option to silently ignore bad filters
for k in self.filter_keys:
if k not in fs.filters:
raise ValidationError("invalid filter[{}]".format(k))
return fs

def get_filterset_kwargs(self, request, queryset, view):
"""
Turns filter[<field>]=<value> into <field>=<value> which is what DjangoFilterBackend expects
"""
self.filter_keys = []
# rewrite `filter[field]` query parameters to make DjangoFilterBackend work.
data = request.query_params.copy()
for qp, val in data.items():
if qp[:
7] == 'filter[' and qp[-1] == ']' and qp != self.search_param:
key = qp[7:-1].replace(
'.', '__'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above a regex would make this more readable and the slices obsolete.

Thinking about it again I guess replacing dots with __ wouldn't hurt for users who don't care about this notation as dots are disallowed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, I'm going to stop using yapf. It messed up the line breaks. It should be this:

if qp[:7] == 'filter[' and qp[-1] == ']' and qp != self.search_param:

) # convert jsonapi relationship path to Django ORM's __ notation
# TODO: implement JSON_API_FORMAT_FIELD_NAMES conversions inbound.
data[key] = val
self.filter_keys.append(key)
del data[qp]
return {
'data': data,
'queryset': queryset,
'request': request,
}