Add URLFilter for URLField.

[jeking3]

Problem

Consider this fragment:

class GrantFilter(filters.FilterSet):
    """Filter grants by principal, scope, or role."""
    principal = CharFilter(method="filter_by_principal", label="Filter by principal (User).")

With drf_spectacular emitting a string type out, we lose the uri formatting designation. Due to the way drf_spectacular works, once it matches CharFilter (see: code) then it assumes a standard OpenApiTypes.STR type. There is no way to annotate your way around it due to the logic there.

Resulting generated web client with swagger-typescript-api shows:

+  /** Filter by principal (User). */
+  principal?: string

Solution

1. Add a django_filters.filters.URLFilter with a field type of URLField.
2. drf_spectacular has no match for this currently (a follow-up to this work will be to remedy that) so it falls back to looking at type annotations on the method's value argument.
3. Add a type hint to the filter method value argument so that it is an OpenApiTypes.URI (note: this will be unnecessary when item 2 is done).
4. It is up to the filter method to figure out the type, identity, and manipulate the queryset appropriately.

Resulting generated web client with swagger-typescript-api shows:

+  /**
+   * Filter by principal (User).
+   * @format uri
+   */
+  principal?: string

[carltongibson]

@tfranzel can I ask you to comment? (I guess I don't mind adding a URLFilter but... 🤷‍♂️)

[tfranzel]

hey! sure @carltongibson, hope you doing fine. this actually has a bit of history behind it.

django-filter fields can have a different type from the underlying model field they operate against. drf-spectacular incorrectly makes the schema type match the model field instead of the filter.
tfranzel/drf-spectacular#317 (comment)

in the beginning we went all out with the model introspection because we simply had the code available, sometimes doing a better job than people wanted/anticipated. that is why we now stop at unambiguous_mapping although more would be possible. as you can guess from the code, reliably parsing django-filters is not an easy feat.

There is no way to annotate your way around it due to the logic there

not entirely true @jeking3, but it is a bit awkward. the test do this like so (for you of course OpenApiTypes.URI):

https://github.com/tfranzel/drf-spectacular/blob/52325f956422e8fc1bf0d9a89b460e6e6c891aa4/tests/contrib/test_django_filters.py#L115-L117

now when I think about it, allowing a bit more flexibility on unambiguous_mapping (via value arg's hint) could have been an option, but we have no mapping for a native "URL" type and so a type hint wouldn't have saved you anyway. also mypy would throw a hissy fit if do func(self, queryset, name, value: OpenApiTypes.URI). I believe this was the reasoning for not adding it due to the limited options for type hints. not 100% sure though... bad memory 😅

TLDR: It can't hurt adding the standard field types that DRF provides anyway. on our side, adding the field would be a one-liner.

[jeking3]

@tfranzel I had to "type: ignore" that one to make it work, after I made my own local URLFilter based on URLField. Already got this working locally so thought I would enhance filters then spectacular, so we could have more spectacular filters and API specs. :) Love both projects.

[jeking3]

@tfranzel consider perhaps allowing @extend_schema_field as a decorator on the filter method as an alternative.... I tried that first and then when it didn't work I dug into the code.

[carltongibson]

So does the example here work (when adapted for URI):

https://github.com/tfranzel/drf-spectacular/blob/52325f956422e8fc1bf0d9a89b460e6e6c891aa4/tests/contrib/test_django_filters.py#L115-L117

    price_range_vat_decorated = extend_schema_field(OpenApiTypes.INT)(
        RangeFilter(field_name='price_vat')
    )

?

This seems like a general problem:

django-filter fields can have a different type from the underlying model field they operate against.

And I'm not sure adding a filter for every model field is the way forward 🤔

Taking away the schema generation question, a potential URLFilter isn't actually serving any purpose (is it? 😬) — so I'm wary of adding it here only to solve a schema generation problem over there (so to speak). 🤔

[tfranzel]

@tfranzel consider perhaps allowing @extend_schema_field as a decorator on the filter method as an alternative.... I tried that first and then when it didn't work I dug into the code.

that sounds like a good idea. i'll look into it.

So does the example here work (when adapted for URI): ?

yes, it does work! extend_schema_field will always take precedence over introspection.

And I'm not sure adding a filter for every model field is the way forward thinking

can't really judge this, but since django-filters is very tightly coupled to the ORM, it wouldn't be the craziest idea to support all Django-native model fields I suppose.

I'm wary of adding it here only to solve a schema generation problem over there

I think of it as a collaborative effort to help people getting most out of their filters with correct schema generation. if I could solve it I would, but due to the way django-filter is designed, it's not always 100% clear to me which is the "correct exposed type". Having all model fields (and using them when mapping on your end) would provide us with more info to work with, taking out some of the heuristic guesswork.

[jeking3]

URLFilter which leverages URLField uses a URLValidator that ensures the content is a valid URL, so it is a stronger validation than using a CharFilter. Additionally, the separate type allows downstream code generators to annotate the format of the string field properly. Having it would be a similar reason as to why UUIDFilter exists already

[tfranzel]

@jeking3 I totally agree. Imho the added validation makes django-filter more robust while also having a more specific interface. Having that info downstream is just a nice perk that helps introspection. So the specificity should be beneficial here regardless of what we do downstream.

[carltongibson]

Now, that is more down my line of concern... 😜

I'm worried this is a breaking change for folks expecting a CharFilter and using contains (for example) — I have a bookmarking app; I want to match URLs containing twitter.com ...

Let me ponder a while. It's not a No. 😄

[jeking3]

Here is one possible way to do that with a URLFilter. It could also be done with a CharFilter, however the downstream intention is lost unless you decorate things for downstream tools; URLFilter (like UUIDFilter) allows downstream tools to discern a string from a url (or a uuid) format as the intended input. At least for drf-spectacular, adding URLFilter would cause no change to its behavior as it falls back to other methods to determine the intention, ending up at str eventually. Further, implementers can choose not to use URLFilter at all, but it would be there if they want to.

from django.core.validators import URLValidator
from django.forms import URLField
from django_filters import rest_framework as filters


class TwitterOnlyURLValidator(URLValidator):
    host_re = r"(twitter\.com)"


class TwitterOnlyURLField(URLField):
    default_validators = [TwitterOnlyURLValidator]


class TwitterOnlyURLFilter(filters.URLFilter):
    field_class = TwitterOnlyURLField


class MyFilter(filters.FilterSet):
    destination = TwitterOnlyURLFilter(method="filter_by_destination", label="Filter by destination.")