Merge branch 'main' into graphiql-headers-fix

graphene_django/__init__.py

@@ -1,7 +1,7 @@
 from .fields import DjangoConnectionField, DjangoListField
 from .types import DjangoObjectType
 
-__version__ = "3.0.1"
+__version__ = "3.0.2"
 
 __all__ = [
     "__version__",

graphene_django/converter.py

@@ -315,26 +315,7 @@ def convert_field_to_djangomodel(field, registry=None):
         if not _type:
             return
 
-        class CustomField(Field):
+        return Field(
-            def wrap_resolve(self, parent_resolver):
-                """
-                Implements a custom resolver which go through the `get_node` method to ensure that
-                it goes through the `get_queryset` method of the DjangoObjectType.
-                """
-                resolver = super().wrap_resolve(parent_resolver)
-
-                def custom_resolver(root, info, **args):
-                    fk_obj = resolver(root, info, **args)
-                    if not isinstance(fk_obj, model):
-                        # In case the resolver is a custom one that overwrites
-                        # the default Django resolver
-                        # This happens, for example, when using custom awaitable resolvers.
-                        return fk_obj
-                    return _type.get_node(info, fk_obj.pk)
-
-                return custom_resolver
-
-        return CustomField(
             _type,
             description=get_django_field_description(field),
             required=not field.null,

graphene_django/tests/test_fields.py

@@ -1,5 +1,6 @@
 import datetime
-from django.db.models import Count
+import re
+from django.db.models import Count, Prefetch
 
 import pytest
 
@@ -7,8 +8,12 @@ from graphene import List, NonNull, ObjectType, Schema, String
 
 from ..fields import DjangoListField
 from ..types import DjangoObjectType
-from .models import Article as ArticleModel
+from .models import (
-from .models import Reporter as ReporterModel
+    Article as ArticleModel,
+    Film as FilmModel,
+    FilmDetails as FilmDetailsModel,
+    Reporter as ReporterModel,
+)
 
 
 class TestDjangoListField:
@@ -500,3 +505,145 @@ class TestDjangoListField:
 
         assert not result.errors
         assert result.data == {"reporters": [{"firstName": "Tara"}]}
+
+    def test_select_related_and_prefetch_related_are_respected(
+        self, django_assert_num_queries
+    ):
+        class Article(DjangoObjectType):
+            class Meta:
+                model = ArticleModel
+                fields = ("headline", "editor", "reporter")
+
+        class Film(DjangoObjectType):
+            class Meta:
+                model = FilmModel
+                fields = ("genre", "details")
+
+        class FilmDetail(DjangoObjectType):
+            class Meta:
+                model = FilmDetailsModel
+                fields = ("location",)
+
+        class Reporter(DjangoObjectType):
+            class Meta:
+                model = ReporterModel
+                fields = ("first_name", "articles", "films")
+
+        class Query(ObjectType):
+            articles = DjangoListField(Article)
+
+            @staticmethod
+            def resolve_articles(root, info):
+                # Optimize for querying associated editors and reporters, and the films and film
+                # details of those reporters. This is similar to what would happen using a library
+                # like https://github.com/tfoxy/graphene-django-optimizer for a query like the one
+                # below (albeit simplified and hardcoded here).
+                return ArticleModel.objects.select_related(
+                    "editor", "reporter"
+                ).prefetch_related(
+                    Prefetch(
+                        "reporter__films",
+                        queryset=FilmModel.objects.select_related("details"),
+                    ),
+                )
+
+        schema = Schema(query=Query)
+
+        query = """
+            query {
+                articles {
+                    headline
+
+                    editor {
+                        firstName
+                    }
+
+                    reporter {
+                        firstName
+
+                        films {
+                            genre
+
+                            details {
+                                location
+                            }
+                        }
+                    }
+                }
+            }
+        """
+
+        r1 = ReporterModel.objects.create(first_name="Tara", last_name="West")
+        r2 = ReporterModel.objects.create(first_name="Debra", last_name="Payne")
+
+        ArticleModel.objects.create(
+            headline="Amazing news",
+            reporter=r1,
+            pub_date=datetime.date.today(),
+            pub_date_time=datetime.datetime.now(),
+            editor=r2,
+        )
+        ArticleModel.objects.create(
+            headline="Not so good news",
+            reporter=r2,
+            pub_date=datetime.date.today(),
+            pub_date_time=datetime.datetime.now(),
+            editor=r1,
+        )
+
+        film1 = FilmModel.objects.create(genre="ac")
+        film2 = FilmModel.objects.create(genre="ot")
+        film3 = FilmModel.objects.create(genre="do")
+        FilmDetailsModel.objects.create(location="Hollywood", film=film1)
+        FilmDetailsModel.objects.create(location="Antarctica", film=film3)
+        r1.films.add(film1, film2)
+        r2.films.add(film3)
+
+        # We expect 2 queries to be performed based on the above resolver definition: one for all
+        # articles joined with the reporters model (for associated editors and reporters), and one
+        # for the films prefetch (which includes its `select_related` JOIN logic in its queryset)
+        with django_assert_num_queries(2) as captured:
+            result = schema.execute(query)
+
+        assert not result.errors
+        assert result.data == {
+            "articles": [
+                {
+                    "headline": "Amazing news",
+                    "editor": {"firstName": "Debra"},
+                    "reporter": {
+                        "firstName": "Tara",
+                        "films": [
+                            {"genre": "AC", "details": {"location": "Hollywood"}},
+                            {"genre": "OT", "details": None},
+                        ],
+                    },
+                },
+                {
+                    "headline": "Not so good news",
+                    "editor": {"firstName": "Tara"},
+                    "reporter": {
+                        "firstName": "Debra",
+                        "films": [
+                            {"genre": "DO", "details": {"location": "Antarctica"}},
+                        ],
+                    },
+                },
+            ]
+        }
+
+        assert len(captured.captured_queries) == 2  # Sanity-check
+
+        # First we should have queried for all articles in a single query, joining on the reporters
+        # model (for the editors and reporters ForeignKeys)
+        assert re.match(
+            r'SELECT .* "tests_article" INNER JOIN "tests_reporter"',
+            captured.captured_queries[0]["sql"],
+        )
+
+        # Then we should have queried for all of the films of all reporters, joined with the film
+        # details for each film, using a single query
+        assert re.match(
+            r'SELECT .* FROM "tests_film" INNER JOIN "tests_film_reporters" .* LEFT OUTER JOIN "tests_filmdetails"',
+            captured.captured_queries[1]["sql"],
+        )

graphene_django/tests/test_get_queryset.py

@@ -16,6 +16,12 @@ class TestShouldCallGetQuerySetOnForeignKey:
     Check that the get_queryset method is called in both forward and reversed direction
     of a foreignkey on types.
     (see issue #1111)
+
+    NOTE: For now, we do not expect this get_queryset method to be called for nested
+    objects, as the original attempt to do so prevented SQL query-optimization with
+    `select_related`/`prefetch_related` and caused N+1 queries. See discussions here
+    https://github.com/graphql-python/graphene-django/pull/1315/files#r1015659857
+    and here https://github.com/graphql-python/graphene-django/pull/1401.
     """
 
     @pytest.fixture(autouse=True)
@@ -121,69 +127,6 @@ class TestShouldCallGetQuerySetOnForeignKey:
         assert not result.errors
         assert result.data == {"reporter": {"firstName": "Jane"}}
 
-    def test_get_queryset_called_on_foreignkey(self):
-        # If a user tries to access a reporter through an article they should get our authorization error
-        query = """
-            query getArticle($id: ID!) {
-                article(id: $id) {
-                    headline
-                    reporter {
-                        firstName
-                    }
-                }
-            }
-        """
-
-        result = self.schema.execute(query, variables={"id": self.articles[0].id})
-        assert len(result.errors) == 1
-        assert result.errors[0].message == "Not authorized to access reporters."
-
-        # An admin user should be able to get reporters through an article
-        query = """
-            query getArticle($id: ID!) {
-                article(id: $id) {
-                    headline
-                    reporter {
-                        firstName
-                    }
-                }
-            }
-        """
-
-        result = self.schema.execute(
-            query,
-            variables={"id": self.articles[0].id},
-            context_value={"admin": True},
-        )
-        assert not result.errors
-        assert result.data["article"] == {
-            "headline": "A fantastic article",
-            "reporter": {"firstName": "Jane"},
-        }
-
-        # An admin user should not be able to access draft article through a reporter
-        query = """
-            query getReporter($id: ID!) {
-                reporter(id: $id) {
-                    firstName
-                    articles {
-                        headline
-                    }
-                }
-            }
-        """
-
-        result = self.schema.execute(
-            query,
-            variables={"id": self.reporter.id},
-            context_value={"admin": True},
-        )
-        assert not result.errors
-        assert result.data["reporter"] == {
-            "firstName": "Jane",
-            "articles": [{"headline": "A fantastic article"}],
-        }
-
 
 class TestShouldCallGetQuerySetOnForeignKeyNode:
     """
@@ -290,72 +233,3 @@ class TestShouldCallGetQuerySetOnForeignKeyNode:
         )
         assert not result.errors
         assert result.data == {"reporter": {"firstName": "Jane"}}
-
-    def test_get_queryset_called_on_foreignkey(self):
-        # If a user tries to access a reporter through an article they should get our authorization error
-        query = """
-            query getArticle($id: ID!) {
-                article(id: $id) {
-                    headline
-                    reporter {
-                        firstName
-                    }
-                }
-            }
-        """
-
-        result = self.schema.execute(
-            query, variables={"id": to_global_id("ArticleType", self.articles[0].id)}
-        )
-        assert len(result.errors) == 1
-        assert result.errors[0].message == "Not authorized to access reporters."
-
-        # An admin user should be able to get reporters through an article
-        query = """
-            query getArticle($id: ID!) {
-                article(id: $id) {
-                    headline
-                    reporter {
-                        firstName
-                    }
-                }
-            }
-        """
-
-        result = self.schema.execute(
-            query,
-            variables={"id": to_global_id("ArticleType", self.articles[0].id)},
-            context_value={"admin": True},
-        )
-        assert not result.errors
-        assert result.data["article"] == {
-            "headline": "A fantastic article",
-            "reporter": {"firstName": "Jane"},
-        }
-
-        # An admin user should not be able to access draft article through a reporter
-        query = """
-            query getReporter($id: ID!) {
-                reporter(id: $id) {
-                    firstName
-                    articles {
-                        edges {
-                            node {
-                                headline
-                            }
-                        }
-                    }
-                }
-            }
-        """
-
-        result = self.schema.execute(
-            query,
-            variables={"id": to_global_id("ReporterType", self.reporter.id)},
-            context_value={"admin": True},
-        )
-        assert not result.errors
-        assert result.data["reporter"] == {
-            "firstName": "Jane",
-            "articles": {"edges": [{"node": {"headline": "A fantastic article"}}]},
-        }