strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension
https://strawberry.rocks/docs/django
MIT License
412 stars 118 forks source link

A solution to make filters support 'NOT' 'AND' 'OR' #303

Closed star2000 closed 1 year ago

star2000 commented 1 year ago

Feature Request Type

Description

Because there may be bugs, or it can be solved more elegantly, so I post here for discussion.

Here is how I do it.

Where apply_filters only needs to remove the two asterisks.

from __future__ import annotations

from enum import Enum
from typing import TYPE_CHECKING, Generic, Optional, TypeVar

import strawberry
import strawberry_django.filters
from django.db.models import Q
from django.db.models.sql.query import get_field_names_from_opts  # type:ignore
from strawberry import UNSET
from strawberry.type import has_object_definition
from strawberry_django.filters import (_resolve_global_id,
                                       function_allow_passing_info,
                                       lookup_name_conversion_map)
from strawberry_django.utils.typing import has_django_definition

if TYPE_CHECKING:
    from typing import Any, Callable, List, Tuple

    from django.db.models import Model, QuerySet
    from strawberry_django.filters import WithStrawberryObjectDefinition

    _M = TypeVar('_M', bound=Model)

T = TypeVar("T")

@strawberry.input
class FilterLookup(Generic[T]):
    exact: Optional[T] = UNSET
    i_exact: Optional[T] = UNSET
    contains: Optional[T] = UNSET
    i_contains: Optional[T] = UNSET
    in_list: Optional[list[T]] = UNSET
    gt: Optional[T] = UNSET
    gte: Optional[T] = UNSET
    lt: Optional[T] = UNSET
    lte: Optional[T] = UNSET
    starts_with: Optional[T] = UNSET
    i_starts_with: Optional[T] = UNSET
    ends_with: Optional[T] = UNSET
    i_ends_with: Optional[T] = UNSET
    range: Optional[list[T]] = UNSET
    is_null: Optional[bool] = UNSET
    regex: Optional[str] = UNSET
    i_regex: Optional[str] = UNSET
    n_exact: Optional[T] = UNSET
    n_i_exact: Optional[T] = UNSET
    n_contains: Optional[T] = UNSET
    n_i_contains: Optional[T] = UNSET
    n_in_list: Optional[list[T]] = UNSET
    n_gt: Optional[T] = UNSET
    n_gte: Optional[T] = UNSET
    n_lt: Optional[T] = UNSET
    n_lte: Optional[T] = UNSET
    n_starts_with: Optional[T] = UNSET
    n_i_starts_with: Optional[T] = UNSET
    n_ends_with: Optional[T] = UNSET
    n_i_ends_with: Optional[T] = UNSET
    n_range: Optional[list[T]] = UNSET
    n_is_null: Optional[bool] = UNSET
    n_regex: Optional[str] = UNSET
    n_i_regex: Optional[str] = UNSET

strawberry_django.filters.FilterLookup = FilterLookup

def build_filter_kwargs(
    filters: WithStrawberryObjectDefinition,
    path='',
) -> Tuple[Q, List[Callable[..., Any]]]:
    filter_kwargs = Q()
    filter_methods: List[Callable[..., Any]] = []
    django_model = (
        filters.__strawberry_django_definition__.model
        if has_django_definition(filters)
        else None
    )

    for f in filters.__strawberry_definition__.fields:
        field_name = f.name
        field_value = _resolve_global_id(getattr(filters, field_name))

        # Unset means we are not filtering this. None is still acceptable
        if field_value is UNSET:
            continue

        if isinstance(field_value, Enum):
            field_value = field_value.value

        negated = False
        if field_name.startswith('n_'):
            field_name = field_name[2:]
            negated = True

        field_name = lookup_name_conversion_map.get(field_name, field_name)
        filter_method = getattr(
            filters, f"filter_{'n_' if negated else ''}{field_name}", None)
        if filter_method:
            filter_methods.append(filter_method)
            continue

        if django_model:
            if field_name == 'OR':
                (
                    subfield_filter_kwargs,
                    subfield_filter_methods,
                ) = build_filter_kwargs(field_value, path)
                filter_kwargs |= subfield_filter_kwargs
                filter_methods.extend(subfield_filter_methods)
                continue

            if field_name not in get_field_names_from_opts(
                django_model._meta,
            ):
                continue

        if has_object_definition(field_value):
            subfield_filter_kwargs, subfield_filter_methods = build_filter_kwargs(
                field_value,
                f'{path}{field_name}__',
            )
            filter_kwargs &= subfield_filter_kwargs
            filter_methods.extend(subfield_filter_methods)
        else:
            filter_kwarg = Q(**{f'{path}{field_name}': field_value})
            if negated:
                filter_kwarg = ~filter_kwarg
            filter_kwargs &= filter_kwarg

    return filter_kwargs, filter_methods

strawberry_django.filters.build_filter_kwargs = build_filter_kwargs

def apply_filters(filters, queryset: QuerySet[_M], info=UNSET, pk=UNSET) -> QuerySet[_M]:
    if pk not in (None, strawberry.UNSET):
        queryset = queryset.filter(pk=pk)

    if filters in (None, strawberry.UNSET) or not has_django_definition(filters):
        return queryset

    # Custom filter function in the filters object
    filter_method = getattr(filters, "filter", None)
    if filter_method:
        kwargs = {}
        if function_allow_passing_info(
            # Pass the original __func__ which is always the same
            getattr(filter_method, "__func__", filter_method),
        ):
            kwargs["info"] = info

        return filter_method(queryset=queryset, **kwargs)

    filter_kwargs, filter_methods = build_filter_kwargs(filters)
    queryset = queryset.filter(filter_kwargs)
    for filter_method in filter_methods:
        kwargs = {}
        if function_allow_passing_info(
            # Pass the original __func__ which is always the same
            getattr(filter_method, "__func__", filter_method),
        ):
            kwargs["info"] = info

        queryset = filter_method(queryset=queryset, **kwargs)

    return queryset

strawberry_django.filters.apply = apply_filters

Upvote & Fund

Fund with Polar

bellini666 commented 1 year ago

Hey @star2000 ,

The ideas in here are interesting! Support for both "OR" and "NOT" are totally issues that I want to solve (I also miss having those in my projects).

We had a discussion about that on discord the order day, and Colin (don't know his github handler) shared his custom implementation as well: https://discord.com/channels/689806334337482765/689861980776955948/1126205282440204378

If I understood your solution correctly, it expects a field called OR to apply with a | to the rest of the filters. You didn't give any example on how the user would define that, but I imagine it would be very similar to Colin's idea of typing it as Self.

One interesting thing he proposed is to add flag to the filters function which would allows users to opt in to this feature, and we could inject the or and and fields and their resolvers.

Regarding the "NOT", I like this! I was previously wondering about a similar solution (not to say identical =P) or exposing an extra exclude of the same type as the filters, but I don't think it would work very well when doing complex and/or stuff, so probably your implementation is the way to go here.

star2000 commented 1 year ago

@bellini666 Regarding the definition of OR , I just do. But I think it's best to integrate this OR directly. e.g.

import strawberry_django

@strawberry_django.filter(m.Group, lookups=True)
class GroupFilter:
    permissions: 'PermissionFilter | None'
    name: auto
    id: auto
    user: 'UserFilter | None'
    OR: 'GroupFilter | None'
star2000 commented 1 year ago

I saw this message. https://discord.com/channels/689806334337482765/689861980776955948/1129431715752976425

Regarding the problem of a && (b || c), although it can be transformed into an equivalent (a && b) || (a && c), the behavior of repeatedly entering the condition a is not elegant after all. So it's better to add an AND field.

            if field_name in ('AND', 'OR'):
                (
                    subfield_filter_kwargs,
                    subfield_filter_methods,
                ) = build_filter_kwargs(field_value, path)
                if field_name == 'AND':
                    filter_kwargs &= subfield_filter_kwargs
                else:
                    filter_kwargs |= subfield_filter_kwargs
from typing import Self

import strawberry_django

@strawberry_django.filter(m.Group, lookups=True)
class GroupFilter:
    permissions: 'PermissionFilter | None'
    name: auto
    id: auto
    user: 'UserFilter | None'
    AND: 'Self | None'
    OR: 'Self | None'
query onlyOR {
  Groups(
    filters: {
      name: {startsWith: "a"},
      id: {range: [1, 2]},
      OR: {
        name: {startsWith: "a"},
        id: {range: [3, 4]}
      }
    }
  ) {
    id
  }
}

query useAND {
  Groups(
    filters: {
      name: {startsWith: "a"},
      AND: {
        id: {range: [1, 2]},
        OR: {
          id: {range: [3, 4]}
        }
      }
    }
  ) {
    id
  }
}
bellini666 commented 1 year ago

@star2000 nice!

I would just call those and and or in the schema though. Also, maybe "injecting" those fields at the filter would make sense so that the user doesn't have to define an and/or by himself.

star2000 commented 1 year ago

@bellini666 How to implement the filter injection attribute you mentioned, is there a code example?

bellini666 commented 1 year ago

@bellini666 How to implement the filter injection attribute you mentioned, is there a code example?

Probably something similar to how this PR implemented fields/exclude.

Inside _process_type, if the type is a filter you can inject the annotations and also set setattr(cls, "<attr>", UNSET)

star2000 commented 1 year ago

@bellini666 How to implement the filter injection attribute you mentioned, is there a code example?

Probably something similar to how this PR implemented fields/exclude.

Inside _process_type, if the type is a filter you can inject the annotations and also set setattr(cls, "<attr>", UNSET)

I tried it and it was successful. I just need to add this code after checking __annotations__ in _process_type.

    if is_filter:
        cls.__annotations__.update({
            'AND': Optional[Self],
            'OR': Optional[Self],
        })

There is no need to set a default value, because here is set. https://github.com/strawberry-graphql/strawberry-graphql-django/blob/5e146adb5bab1ef1ddd87ab07465d20c1e8e422b/strawberry_django/type.py#L173