strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension
MIT License
391 stars 115 forks source link

The get_queryset method is called twice when using relay connections #528

Closed moritz89 closed 1 month ago

moritz89 commented 1 month ago

When implementing types that override the get_queryset() method as well as using relay connection, the get_queryset() method is called twice. This has the result, that when applying filters to the queryset they are applied twice. Since they are idempotent it does not change the query output, but adds unnecessary overhead.

Describe the Bug

The core of bug appears to be strawberry_django/fields/field.py:283-289 in StrawberryDjangoField:get_queryset(). When not using the relay connection the bug does not appear, but with it the if statement resolves to True

    get_queryset = getattr(type_, "get_queryset", None)
    if get_queryset:
        queryset = get_queryset(queryset, info, **kwargs)

and it is also called on the next line with

    queryset = super().get_queryset(
        filter_with_perms(queryset, info), info, **kwargs
    )

System Information

Additional Context

This behavior is reproducible in the example app. If you already have a relay app, simply add a print in the overriding get_queryset method.

# example/django/app/types.py

@strawberry_django.type(
    models.Fruit,
    filters=FruitFilter,
    order=FruitOrder,
    pagination=True,
)
class Fruit(relay.Node):
    name: auto
    # color: Optional["Color"]

    @classmethod
    def get_queryset(
        cls,
        queryset: QuerySet,
        info: Info,
        **kwargs,
    ):
        print("Getting QS")
        return queryset.filter(name__startswith="s")
# example/django/app/schema.py

@strawberry.type
class Query:
    # fruit: Fruit = strawberry_django.field()
    # fruits: List[Fruit] = strawberry_django.field()
    all_fruits: ListConnection[Fruit] = strawberry_django.connection()

Attached is the diff, applied to commit 21c14e4

bug.zip

Upvote & Fund

Fund with Polar

bellini666 commented 1 month ago

Hey @moritz89 ,

Thanks for the report.

It wasn't actually that line, as one is the get_queryset from the field and the other from the type, but I found the issue :)

Should be fixed in the next release that I'm going to make in a few minutes