Django-filter: Filter fields are technically allowed for all DRF view actions. This messes with CoreAPI schema.

Created on 16 May 2018  Â·  3Comments  Â·  Source: carltongibson/django-filter

When using CoreAPI with DRF and DjangoFilterBackend enabled, the client attempts to map all fields for a create, update or partial_update actions to the query params as well. This breaks the client's ability to use these methods.

Example

If we have a request that attempts to create a book with a name, and name is also a filterable field on our model:

# book views
class BookViewSet():
    filter_fields = ('name',)
   # ...
// schema
client.action('book', 'create', { 'name': 'My Awesome Book' });

This will create a request with both name in the URL param as well as the request body. This is not expected behavior.

POST /api/book?name=My%20Awesome%20Book

name=My Awesome Book

Proposed Solution

We've fixed this in our code by instead pointing our DRF filter backend to a custom override with the following code:

class OnlyFilterOnReadDjangoFilterBackend(DjangoFilterBackend):
    """ A filter backend that only allows for filtering by properties on
    actions deemed as safe. This means that create, update, and partial_update
    actions will not provide filter options.
    """
    SAFE_ACTIONS = ('list', 'retrieve')

    def get_schema_fields(self, view):
        if view.action in self.SAFE_ACTIONS:
            return super().get_schema_fields(view)
        return []

Most helpful comment

Hi @Sonictherocketman. Thanks for the report.

My first thought is that this is a bug with the client API — it's not suitably distinguishing between query string parameters and request body when constructing the request.

In general, just disabling filtering for non-safe actions isn't correct. Filtering occurs when establishing the base queryset that an update will be made against.

I'll leave this open for now, just to have a think about it.

All 3 comments

Hi @Sonictherocketman. Thanks for the report.

My first thought is that this is a bug with the client API — it's not suitably distinguishing between query string parameters and request body when constructing the request.

In general, just disabling filtering for non-safe actions isn't correct. Filtering occurs when establishing the base queryset that an update will be made against.

I'll leave this open for now, just to have a think about it.

I'm also kinda think that it's a client issue, but the more I thought about it, I feel like the actions don't really make any sense on write queries (especially writing to a specific resource) and imo the schema shouldn't have those fields if they shouldn't be used.

Thanks for the quick reply and I'm super interested in what your thoughts are on this.

More or less, I think we're limited here by our desires for _automagic_ code generation outstripping the tech (in it's current state).

There's lots we can't (yet) do with schemas (in any of the available formats) that we'd like to — for example, with CoreAPI, we're not yet able to group parameters together for a date range filter to show that they're related. (There's a lot more we can do writing schemas by hand, but no one wants to do that, of course.)

But it evolves, and as it does so does the tooling. It'll improve.

That this particular line:

client.action('book', 'create', { 'name': 'My Awesome Book' });

uses the same dict for both query string and the request body isn't great. I guess it's the abstraction leaking. You really want to be building this request _by-hand_, specifying query params and body in separate places.

This isn't currently anything we can address here. Thanks again for the report though.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

chromakey picture chromakey  Â·  5Comments

madelyneriksen picture madelyneriksen  Â·  4Comments

ses4j picture ses4j  Â·  4Comments

GuillaumeCisco picture GuillaumeCisco  Â·  3Comments

gotexis picture gotexis  Â·  4Comments