Skip to content

FilterSetMetaclass doesn't allow for additional arguments to __init_subclass__ #1713

@ProJoelVik

Description

@ProJoelVik

Hi Django-filters,

I have a small issue I'd like to discuss and hopefully get fixed. In my project I've created some custom FilterSets that add/change some behaviour of FilterSet. One thing I want to do is to add __init_subclass__ to one of the FilterSets that is quite high up in the hierarchy. FilterSet uses a metaclass, FilterSetMetaclass, which I've extended to add some additional behaviour as well. However, I feel like some logic is better to keep within the FilterSet, hence the use of __init_subclass__. Here's an attempt to break down the issue:

FilterSetMetaclass.__new__ has three arguments defined, but no **kwargs:

class FilterSetMetaclass(type):
    def __new__(cls, name, bases, attrs):
        attrs["declared_filters"] = cls.get_declared_filters(bases, attrs)

        new_class = super().__new__(cls, name, bases, attrs)
        new_class._meta = FilterSetOptions(getattr(new_class, "Meta", None))
        new_class.base_filters = new_class.get_filters()

        return new_class

Say I want to extend FilterSet and add an __init_subclass__ to my FilterSet:

class MyFilterSet(FilterSet):
    def __init_subclass__(cls, *args, abstract=False, **kwargs):
        super().__init_subclass__(*args, **kwargs)
       if not abstract:
           # Do something for non-abstract subclasses.

class MyOtherFilterSet(MyFilterSet, abstract=True):
    ...

Any argument I add to MyOtherFilterSet will not get passed down to __init_subclass__, since the __new__ method of FilterSetMetaclass doesn't pass on any additional arguments besides name, bases, attrs.

A suggested fix is to add **kwargs to FilterSetMetaclass.__new__ and pass it along to super().__new__(cls, name, bases, attrs, **kwargs):

class FilterSetMetaclass(type):
    def __new__(cls, name, bases, attrs, **kwargs):
        attrs["declared_filters"] = cls.get_declared_filters(bases, attrs)

        new_class = super().__new__(cls, name, bases, attrs, **kwargs)
        new_class._meta = FilterSetOptions(getattr(new_class, "Meta", None))
        new_class.base_filters = new_class.get_filters()

        return new_class

Practical Example

One use case we have is that we've created 2 base classes that extends FilterSet. These base classes are considered abstract and are meant to be subclassed by actual FilterSets. We've come across cases where the FilterSets reference each other or themselves and therefore cause circular imports. We've solved this by adding a filterset registry, which holds all the subclasses of our 2 base FilterSets. Here's an example of a FilterSet that makes use of this registry by passing in the FilterSet name as a string instead of an actual reference:

class DocumentFilterSet(StrictFilterSet):
    # NestedFilter lets us build a tree of some sort of which models and filters are referenced
    parent = NestedFilter(filterset_class="DocumentFilterSet")

    class Meta:
        model = Document
        fields = ["id"]

The registry and the base classes look like this:

# A dictionary to hold the registered filter sets.
# The key is the name of the filter set, and the value is the filter set class.
FILTERSET_REGISTRY: dict[str, type[StrictFilterSet]] = {}


class FilterSetAlreadyRegisteredError(ValueError):
    pass


class InvalidFilterSetClassError(TypeError):
    pass


class FilterSetRegistryMeta(FilterSetMetaclass):
    """
    A metaclass for managing the registration of filter sets in the global
    FILTERSET_REGISTRY.
    This metaclass ensures that:
    - Only subclasses of StrictFilterSet are registered.
    - Abstract filter sets (marked with `__abstract__ = True`) and empty filter sets
      (subclasses of EmptyFilterSet) are not registered.
    - Duplicate registrations are prevented by raising a
    FilterSetAlreadyRegisteredError.
    Registered filter sets are stored in the global FILTERSET_REGISTRY dictionary,
    where the key is the name of the filter set and the value is the filter set class.
    """

    def __new__(
        cls, name: str, bases: tuple[FilterSet], attrs: dict[str, object], abstract: bool = False
    ) -> type[StrictFilterSet]:
        filterset_class: type[StrictFilterSet] = super().__new__(cls, name, bases, attrs)
        if not issubclass(filterset_class, FilterSet):
            raise InvalidFilterSetClassError(
                f"FilterSet class '{name}' must subclass FilterSet, got {filterset_class} subclassing {bases}"
            )

        # Avoid registering the base FilterSet classes themselves,
        # DjangoFilter's FilterSet classes and classes based on EmptyFilterSet
        if not abstract and not issubclass(filterset_class, EmptyFilterSet):
            if name in FILTERSET_REGISTRY:
                existing_cls = FILTERSET_REGISTRY[name]
                raise FilterSetAlreadyRegisteredError(
                    f"FilterSet '{name}' already registered by {existing_cls.__module__}.{existing_cls.__name__}"
                )
            FILTERSET_REGISTRY[name] = filterset_class

        return filterset_class


class StrictFilterSet(FilterSet, abstract=True, metaclass=FilterSetRegistryMeta):
    ...


class EmptyFilterSet(StrictFilterSet, abstract=True):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)

    class Meta:
        fields: list[str] = []
        model: type[models.Model]

So far so good! FilterSetRegistryMeta.__new__ can receive the abstract argument and exclude FilterSets based on it.

The issue

Now, we've set up some rules on how we want our FilterSets to be defined and used. This does not go hand-in-hand with how Django-filters let us set up FilterSets, as FilterSet is quite flexible. What we want is to set up more rules and disallow certain functionality. For example, we don't want exclude to be used on any FilterSet as we want each field that is allowed to be filtered on to be explicitly stated in the FilterSet class. A convenient place to set up such rules is the __init_subclass__ method of StrictFilterSet:

class StrictFilterSet(FilterSet, metaclass=FilterSetRegistryMeta):
    def __init_subclass__(cls):
        super().__init_subclass__()

        if not hasattr(cls, "Meta"):
            raise TypeError(f"{cls.__name__} must have a Meta class.")

        if not hasattr(cls.Meta, "fields"):
            raise TypeError(f"{cls.__name__} must have a Meta class with a fields attribute.")

        if not isinstance(cls.Meta.fields, (list, tuple)):
            raise TypeError(
                f"{cls.__name__} must have a Meta class with a fields attribute that is a list or tuple."
            )
        if hasattr(cls.Meta, "exclude"):
            raise TypeError(f"{cls.__name__} must not have a Meta class with an exclude attribute.")

        # EmptyFilterSet is an abstract class that doesn't define a model,
        # so we skip this check for it.
        # Checking cls.__name__ is a hack since abstract=True from EmptyFilterSet
        # isn't passed on from the metaclass to __init_subclass__.
        if cls.__name__ != "EmptyFilterSet" and not hasattr(cls.Meta, "model"):
            raise TypeError(f"{cls.__name__} must have a Meta class with a model attribute.")

Given that we want to keep adding functionality, the limit of not being able to pass arguments to __init_subclass__ makes the code a bit messy. We could expand on the metadata class, but the code would be more readable if we could define our rules inside each class extending FilterSet, instead of creating new metadata classes.

Appreciate all the hard work you're doing!

// Joel

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions