From 3ce44908c9ddfcf6a7b603acc6ee7aefb64ce03c Mon Sep 17 00:00:00 2001 From: Jason Kraus Date: Thu, 28 Nov 2019 02:48:03 -0800 Subject: [PATCH 1/8] =?UTF-8?q?django-filter:=20resolve=20field=20along=20?= =?UTF-8?q?with=20lookup=20expression=20to=20pro=E2=80=A6=20(#805)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * django-filter: resolve field along with lookup expression to properly resolve field * bring back django-filter with method test * remove dangling comment * refactor based on better knowledge of django-filters --- graphene_django/filter/utils.py | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index abb03a9..c5f18e2 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -1,5 +1,6 @@ import six +from django_filters.utils import get_model_field from .filterset import custom_filterset_factory, setup_filterset @@ -18,22 +19,12 @@ def get_filtering_args_from_filterset(filterset_class, type): if name in filterset_class.declared_filters: form_field = filter_field.field else: - try: - field_name, filter_type = name.rsplit("__", 1) - except ValueError: - field_name = name - filter_type = None - - # If the filter type is `isnull` then use the filter provided by - # DjangoFilter (a BooleanFilter). - # Otherwise try and get a filter based on the actual model field - if filter_type != "isnull" and hasattr(model, field_name): - model_field = model._meta.get_field(field_name) - - if hasattr(model_field, "formfield"): - form_field = model_field.formfield( - required=filter_field.extra.get("required", False) - ) + model_field = get_model_field(model, filter_field.field_name) + filter_type = filter_field.lookup_expr + if filter_type != "isnull" and hasattr(model_field, "formfield"): + form_field = model_field.formfield( + required=filter_field.extra.get("required", False) + ) # Fallback to field defined on filter if we can't get it from the # model field From a818ec9017c82105d2dcfb605946b890b319fa97 Mon Sep 17 00:00:00 2001 From: Jason Kraus Date: Thu, 28 Nov 2019 02:49:37 -0800 Subject: [PATCH 2/8] replace merge_queryset with resolve_queryset pattern (#796) * replace merge_queryset with resolve_queryset pattern * skip double limit test * Update graphene_django/fields.py Co-Authored-By: Jonathan Kim * yank skipped test * fix bad variable ref * add test for annotations * add test for using queryset with django filters * document ththat one should use defer instead of values with queysets and DjangoObjectTypes --- docs/queries.rst | 7 ++ graphene_django/fields.py | 35 ++++---- graphene_django/filter/fields.py | 68 ++------------- graphene_django/filter/tests/test_fields.py | 96 +++++++++------------ graphene_django/tests/test_query.py | 58 ++++++++++++- 5 files changed, 132 insertions(+), 132 deletions(-) diff --git a/docs/queries.rst b/docs/queries.rst index 67ebb06..36cdab1 100644 --- a/docs/queries.rst +++ b/docs/queries.rst @@ -282,6 +282,13 @@ of Django's ``HTTPRequest`` in your resolve methods, such as checking for authen return Question.objects.none() +DjangoObjectTypes +~~~~~~~~~~~~~~~~~ + +A Resolver that maps to a defined `DjangoObjectType` should only use methods that return a queryset. +Queryset methods like `values` will return dictionaries, use `defer` instead. + + Plain ObjectTypes ----------------- diff --git a/graphene_django/fields.py b/graphene_django/fields.py index e6daa88..47b44f6 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -39,9 +39,9 @@ class DjangoListField(Field): if queryset is None: # Default to Django Model queryset # N.B. This happens if DjangoListField is used in the top level Query object - model = django_object_type._meta.model + model_manager = django_object_type._meta.model.objects queryset = maybe_queryset( - django_object_type.get_queryset(model.objects, info) + django_object_type.get_queryset(model_manager, info) ) return queryset @@ -108,25 +108,13 @@ class DjangoConnectionField(ConnectionField): @classmethod def resolve_queryset(cls, connection, queryset, info, args): + # queryset is the resolved iterable from ObjectType return connection._meta.node.get_queryset(queryset, info) @classmethod - def merge_querysets(cls, default_queryset, queryset): - if default_queryset.query.distinct and not queryset.query.distinct: - queryset = queryset.distinct() - elif queryset.query.distinct and not default_queryset.query.distinct: - default_queryset = default_queryset.distinct() - return queryset & default_queryset - - @classmethod - def resolve_connection(cls, connection, default_manager, args, iterable): - if iterable is None: - iterable = default_manager + def resolve_connection(cls, connection, args, iterable): iterable = maybe_queryset(iterable) if isinstance(iterable, QuerySet): - if iterable.model.objects is not default_manager: - default_queryset = maybe_queryset(default_manager) - iterable = cls.merge_querysets(default_queryset, iterable) _len = iterable.count() else: _len = len(iterable) @@ -150,6 +138,7 @@ class DjangoConnectionField(ConnectionField): resolver, connection, default_manager, + queryset_resolver, max_limit, enforce_first_or_last, root, @@ -177,9 +166,15 @@ class DjangoConnectionField(ConnectionField): ).format(last, info.field_name, max_limit) args["last"] = min(last, max_limit) + # eventually leads to DjangoObjectType's get_queryset (accepts queryset) + # or a resolve_foo (does not accept queryset) iterable = resolver(root, info, **args) - queryset = cls.resolve_queryset(connection, default_manager, info, args) - on_resolve = partial(cls.resolve_connection, connection, queryset, args) + if iterable is None: + iterable = default_manager + # thus the iterable gets refiltered by resolve_queryset + # but iterable might be promise + iterable = queryset_resolver(connection, iterable, info, args) + on_resolve = partial(cls.resolve_connection, connection, args) if Promise.is_thenable(iterable): return Promise.resolve(iterable).then(on_resolve) @@ -192,6 +187,10 @@ class DjangoConnectionField(ConnectionField): parent_resolver, self.connection_type, self.get_manager(), + self.get_queryset_resolver(), self.max_limit, self.enforce_first_or_last, ) + + def get_queryset_resolver(self): + return self.resolve_queryset diff --git a/graphene_django/filter/fields.py b/graphene_django/filter/fields.py index 338becb..9943346 100644 --- a/graphene_django/filter/fields.py +++ b/graphene_django/filter/fields.py @@ -52,69 +52,17 @@ class DjangoFilterConnectionField(DjangoConnectionField): return get_filtering_args_from_filterset(self.filterset_class, self.node_type) @classmethod - def merge_querysets(cls, default_queryset, queryset): - # There could be the case where the default queryset (returned from the filterclass) - # and the resolver queryset have some limits on it. - # We only would be able to apply one of those, but not both - # at the same time. - - # See related PR: https://github.com/graphql-python/graphene-django/pull/126 - - assert not ( - default_queryset.query.low_mark and queryset.query.low_mark - ), "Received two sliced querysets (low mark) in the connection, please slice only in one." - assert not ( - default_queryset.query.high_mark and queryset.query.high_mark - ), "Received two sliced querysets (high mark) in the connection, please slice only in one." - low = default_queryset.query.low_mark or queryset.query.low_mark - high = default_queryset.query.high_mark or queryset.query.high_mark - default_queryset.query.clear_limits() - queryset = super(DjangoFilterConnectionField, cls).merge_querysets( - default_queryset, queryset - ) - queryset.query.set_limits(low, high) - return queryset - - @classmethod - def connection_resolver( - cls, - resolver, - connection, - default_manager, - max_limit, - enforce_first_or_last, - filterset_class, - filtering_args, - root, - info, - **args + def resolve_queryset( + cls, connection, iterable, info, args, filtering_args, filterset_class ): filter_kwargs = {k: v for k, v in args.items() if k in filtering_args} - qs = filterset_class( - data=filter_kwargs, - queryset=default_manager.get_queryset(), - request=info.context, + return filterset_class( + data=filter_kwargs, queryset=iterable, request=info.context ).qs - return super(DjangoFilterConnectionField, cls).connection_resolver( - resolver, - connection, - qs, - max_limit, - enforce_first_or_last, - root, - info, - **args - ) - - def get_resolver(self, parent_resolver): + def get_queryset_resolver(self): return partial( - self.connection_resolver, - parent_resolver, - self.connection_type, - self.get_manager(), - self.max_limit, - self.enforce_first_or_last, - self.filterset_class, - self.filtering_args, + self.resolve_queryset, + filterset_class=self.filterset_class, + filtering_args=self.filtering_args, ) diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py index 1ffa0f4..1eba601 100644 --- a/graphene_django/filter/tests/test_fields.py +++ b/graphene_django/filter/tests/test_fields.py @@ -608,58 +608,6 @@ def test_should_query_filter_node_limit(): assert result.data == expected -def test_should_query_filter_node_double_limit_raises(): - class ReporterFilter(FilterSet): - limit = NumberFilter(method="filter_limit") - - def filter_limit(self, queryset, name, value): - return queryset[:value] - - class Meta: - model = Reporter - fields = ["first_name"] - - class ReporterType(DjangoObjectType): - class Meta: - model = Reporter - interfaces = (Node,) - - class Query(ObjectType): - all_reporters = DjangoFilterConnectionField( - ReporterType, filterset_class=ReporterFilter - ) - - def resolve_all_reporters(self, info, **args): - return Reporter.objects.order_by("a_choice")[:2] - - Reporter.objects.create( - first_name="Bob", last_name="Doe", email="bobdoe@example.com", a_choice=2 - ) - Reporter.objects.create( - first_name="John", last_name="Doe", email="johndoe@example.com", a_choice=1 - ) - - schema = Schema(query=Query) - query = """ - query NodeFilteringQuery { - allReporters(limit: 1) { - edges { - node { - id - firstName - } - } - } - } - """ - - result = schema.execute(query) - assert len(result.errors) == 1 - assert str(result.errors[0]) == ( - "Received two sliced querysets (high mark) in the connection, please slice only in one." - ) - - def test_order_by_is_perserved(): class ReporterType(DjangoObjectType): class Meta: @@ -721,7 +669,7 @@ def test_order_by_is_perserved(): assert reverse_result.data == reverse_expected -def test_annotation_is_perserved(): +def test_annotation_is_preserved(): class ReporterType(DjangoObjectType): full_name = String() @@ -766,6 +714,48 @@ def test_annotation_is_perserved(): assert result.data == expected +def test_annotation_with_only(): + class ReporterType(DjangoObjectType): + full_name = String() + + class Meta: + model = Reporter + interfaces = (Node,) + filter_fields = () + + class Query(ObjectType): + all_reporters = DjangoFilterConnectionField(ReporterType) + + def resolve_all_reporters(self, info, **args): + return Reporter.objects.only("first_name", "last_name").annotate( + full_name=Concat( + "first_name", Value(" "), "last_name", output_field=TextField() + ) + ) + + Reporter.objects.create(first_name="John", last_name="Doe") + + schema = Schema(query=Query) + + query = """ + query NodeFilteringQuery { + allReporters(first: 1) { + edges { + node { + fullName + } + } + } + } + """ + expected = {"allReporters": {"edges": [{"node": {"fullName": "John Doe"}}]}} + + result = schema.execute(query) + + assert not result.errors + assert result.data == expected + + def test_integer_field_filter_type(): class PetType(DjangoObjectType): class Meta: diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index f24f84b..95db2d1 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -638,6 +638,8 @@ def test_should_error_if_first_is_greater_than_max(): class Query(graphene.ObjectType): all_reporters = DjangoConnectionField(ReporterType) + assert Query.all_reporters.max_limit == 100 + r = Reporter.objects.create( first_name="John", last_name="Doe", email="johndoe@example.com", a_choice=1 ) @@ -679,6 +681,8 @@ def test_should_error_if_last_is_greater_than_max(): class Query(graphene.ObjectType): all_reporters = DjangoConnectionField(ReporterType) + assert Query.all_reporters.max_limit == 100 + r = Reporter.objects.create( first_name="John", last_name="Doe", email="johndoe@example.com", a_choice=1 ) @@ -804,7 +808,7 @@ def test_should_query_connectionfields_with_manager(): schema = graphene.Schema(query=Query) query = """ query ReporterLastQuery { - allReporters(first: 2) { + allReporters(first: 1) { edges { node { id @@ -1116,3 +1120,55 @@ def test_should_preserve_prefetch_related(django_assert_num_queries): with django_assert_num_queries(3) as captured: result = schema.execute(query) assert not result.errors + + +def test_should_preserve_annotations(): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (graphene.relay.Node,) + + class FilmType(DjangoObjectType): + reporters = DjangoConnectionField(ReporterType) + reporters_count = graphene.Int() + + class Meta: + model = Film + interfaces = (graphene.relay.Node,) + + class Query(graphene.ObjectType): + films = DjangoConnectionField(FilmType) + + def resolve_films(root, info): + qs = Film.objects.prefetch_related("reporters") + return qs.annotate(reporters_count=models.Count("reporters")) + + r1 = Reporter.objects.create(first_name="Dave", last_name="Smith") + r2 = Reporter.objects.create(first_name="Jane", last_name="Doe") + + f1 = Film.objects.create() + f1.reporters.set([r1, r2]) + f2 = Film.objects.create() + f2.reporters.set([r2]) + + query = """ + query { + films { + edges { + node { + reportersCount + } + } + } + } + """ + schema = graphene.Schema(query=Query) + result = schema.execute(query) + assert not result.errors, str(result) + + expected = { + "films": { + "edges": [{"node": {"reportersCount": 2}}, {"node": {"reportersCount": 1}}] + } + } + assert result.data == expected, str(result.data) From e82a2d75c645989ade5c51b578230f5d313e6a7c Mon Sep 17 00:00:00 2001 From: Jonathan Kim Date: Thu, 28 Nov 2019 19:23:31 +0000 Subject: [PATCH 3/8] v2.7.0 --- graphene_django/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphene_django/__init__.py b/graphene_django/__init__.py index 7650dd2..bc01752 100644 --- a/graphene_django/__init__.py +++ b/graphene_django/__init__.py @@ -1,6 +1,6 @@ from .types import DjangoObjectType from .fields import DjangoConnectionField -__version__ = "2.6.0" +__version__ = "2.7.0" __all__ = ["__version__", "DjangoObjectType", "DjangoConnectionField"] From 7e7f18ee0e96562f57628c2cd5cbd9a5a6941f57 Mon Sep 17 00:00:00 2001 From: Thiago Bellini Ribeiro Date: Fri, 29 Nov 2019 06:13:16 -0300 Subject: [PATCH 4/8] Keep original queryset on DjangoFilterConnectionField (#816) * Keep original queryset on DjangoFilterConnectionField The PR #796 broke DjangoFilterConnectionField making it always get the raw queryset from the model to apply the filters in it. This makes sure that the DjangoObjectType's .get_queryset is called, keeping any filtering it might have made. * Add regression test --- graphene_django/filter/fields.py | 7 ++-- graphene_django/filter/tests/test_fields.py | 38 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/graphene_django/filter/fields.py b/graphene_django/filter/fields.py index 9943346..a46a4b7 100644 --- a/graphene_django/filter/fields.py +++ b/graphene_django/filter/fields.py @@ -55,10 +55,11 @@ class DjangoFilterConnectionField(DjangoConnectionField): def resolve_queryset( cls, connection, iterable, info, args, filtering_args, filterset_class ): + qs = super(DjangoFilterConnectionField, cls).resolve_queryset( + connection, iterable, info, args + ) filter_kwargs = {k: v for k, v in args.items() if k in filtering_args} - return filterset_class( - data=filter_kwargs, queryset=iterable, request=info.context - ).qs + return filterset_class(data=filter_kwargs, queryset=qs, request=info.context).qs def get_queryset_resolver(self): return partial( diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py index 1eba601..de366ba 100644 --- a/graphene_django/filter/tests/test_fields.py +++ b/graphene_django/filter/tests/test_fields.py @@ -756,6 +756,44 @@ def test_annotation_with_only(): assert result.data == expected +def test_node_get_queryset_is_called(): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + filter_fields = () + + @classmethod + def get_queryset(cls, queryset, info): + return queryset.filter(first_name="b") + + class Query(ObjectType): + all_reporters = DjangoFilterConnectionField( + ReporterType, reverse_order=Boolean() + ) + + Reporter.objects.create(first_name="b") + Reporter.objects.create(first_name="a") + + schema = Schema(query=Query) + query = """ + query NodeFilteringQuery { + allReporters(first: 10) { + edges { + node { + firstName + } + } + } + } + """ + expected = {"allReporters": {"edges": [{"node": {"firstName": "b"}}]}} + + result = schema.execute(query) + assert not result.errors + assert result.data == expected + + def test_integer_field_filter_type(): class PetType(DjangoObjectType): class Meta: From 374d8a8a9e6fd2f2fdd373183f35d75229617768 Mon Sep 17 00:00:00 2001 From: Jonathan Kim Date: Fri, 29 Nov 2019 09:13:36 +0000 Subject: [PATCH 5/8] v2.7.1 --- graphene_django/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphene_django/__init__.py b/graphene_django/__init__.py index bc01752..df58a5a 100644 --- a/graphene_django/__init__.py +++ b/graphene_django/__init__.py @@ -1,6 +1,6 @@ from .types import DjangoObjectType from .fields import DjangoConnectionField -__version__ = "2.7.0" +__version__ = "2.7.1" __all__ = ["__version__", "DjangoObjectType", "DjangoConnectionField"] From a73d6532744fac27af049d3aa6fbf7e1acc831fa Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 7 Dec 2019 19:56:40 +0000 Subject: [PATCH 6/8] Bump django from 2.2.4 to 2.2.8 in /examples/cookbook-plain (#822) Bumps [django](https://github.com/django/django) from 2.2.4 to 2.2.8. - [Release notes](https://github.com/django/django/releases) - [Commits](https://github.com/django/django/compare/2.2.4...2.2.8) Signed-off-by: dependabot[bot] --- examples/cookbook-plain/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/cookbook-plain/requirements.txt b/examples/cookbook-plain/requirements.txt index 802aa37..beed53b 100644 --- a/examples/cookbook-plain/requirements.txt +++ b/examples/cookbook-plain/requirements.txt @@ -1,4 +1,4 @@ graphene graphene-django graphql-core>=2.1rc1 -django==2.2.4 +django==2.2.8 From 968002f1554e3a7a1c0617682be64b67823b2581 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 7 Dec 2019 19:56:57 +0000 Subject: [PATCH 7/8] Bump django from 2.2.4 to 2.2.8 in /examples/cookbook (#821) Bumps [django](https://github.com/django/django) from 2.2.4 to 2.2.8. - [Release notes](https://github.com/django/django/releases) - [Commits](https://github.com/django/django/compare/2.2.4...2.2.8) Signed-off-by: dependabot[bot] --- examples/cookbook/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/cookbook/requirements.txt b/examples/cookbook/requirements.txt index 0537103..3209a5e 100644 --- a/examples/cookbook/requirements.txt +++ b/examples/cookbook/requirements.txt @@ -1,5 +1,5 @@ graphene graphene-django graphql-core>=2.1rc1 -django==2.2.4 +django==2.2.8 django-filter>=2 From b66a3f347947804d0ab7d9763309e2977b5bcd5a Mon Sep 17 00:00:00 2001 From: Chibuotu Amadi Date: Thu, 26 Dec 2019 12:45:18 +0100 Subject: [PATCH 8/8] Add headers arg to GraphQLTestCase.query (#827) * Add headers arg to GraphQLTestCase.query * fix headers NoneType case in GraphQLTestCase.query * Run format Co-authored-by: Jonathan Kim --- graphene_django/utils/testing.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/graphene_django/utils/testing.py b/graphene_django/utils/testing.py index 5b694b2..8a9b994 100644 --- a/graphene_django/utils/testing.py +++ b/graphene_django/utils/testing.py @@ -24,7 +24,7 @@ class GraphQLTestCase(TestCase): cls._client = Client() - def query(self, query, op_name=None, input_data=None, variables=None): + def query(self, query, op_name=None, input_data=None, variables=None, headers=None): """ Args: query (string) - GraphQL query to run @@ -36,7 +36,9 @@ class GraphQLTestCase(TestCase): are provided, the ``input`` field in the ``variables`` dict will be overwritten with this value. variables (dict) - If provided, the "variables" field in GraphQL will be - set to this value. + set to this value. + headers (dict) - If provided, the headers in POST request to GRAPHQL_URL + will be set to this value. Returns: Response object from client @@ -51,10 +53,17 @@ class GraphQLTestCase(TestCase): body["variables"]["input"] = input_data else: body["variables"] = {"input": input_data} - - resp = self._client.post( - self.GRAPHQL_URL, json.dumps(body), content_type="application/json" - ) + if headers: + resp = self._client.post( + self.GRAPHQL_URL, + json.dumps(body), + content_type="application/json", + **headers + ) + else: + resp = self._client.post( + self.GRAPHQL_URL, json.dumps(body), content_type="application/json" + ) return resp def assertResponseNoErrors(self, resp):