From 47a3ac2758825e952a36c9797293817b51373102 Mon Sep 17 00:00:00 2001 From: Paul Craciunoiu Date: Tue, 19 May 2020 13:17:27 -0600 Subject: [PATCH 1/7] Use max_limit first, if set. --- graphene_django/fields.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/graphene_django/fields.py b/graphene_django/fields.py index 7539cf29c..02e4a9c1d 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -127,12 +127,12 @@ def resolve_queryset(cls, connection, queryset, info, args): return connection._meta.node.get_queryset(queryset, info) @classmethod - def resolve_connection(cls, connection, args, iterable): + def resolve_connection(cls, connection, args, iterable, max_limit=None): iterable = maybe_queryset(iterable) if isinstance(iterable, QuerySet): - _len = iterable.count() + _len = max_limit or iterable.count() else: - _len = len(iterable) + _len = max_limit or len(iterable) connection = connection_from_list_slice( iterable, args, @@ -189,7 +189,7 @@ def connection_resolver( # 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) + on_resolve = partial(cls.resolve_connection, connection, args, max_limit=max_limit) if Promise.is_thenable(iterable): return Promise.resolve(iterable).then(on_resolve) From 87c2730ab0b280e1cd09a8cd7c84e3df788e768b Mon Sep 17 00:00:00 2001 From: Paul Craciunoiu Date: Mon, 25 May 2020 23:06:57 -0600 Subject: [PATCH 2/7] Fix tests and bug with 'last' argument --- graphene_django/debug/tests/test_query.py | 81 ++++++++++++++++------- graphene_django/fields.py | 7 +- graphene_django/tests/test_query.py | 2 +- 3 files changed, 63 insertions(+), 27 deletions(-) diff --git a/graphene_django/debug/tests/test_query.py b/graphene_django/debug/tests/test_query.py index 7226f9b8a..8cc0a3b5f 100644 --- a/graphene_django/debug/tests/test_query.py +++ b/graphene_django/debug/tests/test_query.py @@ -1,4 +1,5 @@ import graphene +import pytest from graphene.relay import Node from graphene_django import DjangoConnectionField, DjangoObjectType @@ -24,7 +25,7 @@ class Meta: class Query(graphene.ObjectType): reporter = graphene.Field(ReporterType) - debug = graphene.Field(DjangoDebug, name="_debug") + debug = graphene.Field(DjangoDebug, name="__debug") def resolve_reporter(self, info, **args): return Reporter.objects.first() @@ -34,7 +35,7 @@ def resolve_reporter(self, info, **args): reporter { lastName } - _debug { + __debug { sql { rawSql } @@ -43,7 +44,7 @@ def resolve_reporter(self, info, **args): """ expected = { "reporter": {"lastName": "ABA"}, - "_debug": {"sql": [{"rawSql": str(Reporter.objects.order_by("pk")[:1].query)}]}, + "__debug": {"sql": [{"rawSql": str(Reporter.objects.order_by("pk")[:1].query)}]}, } schema = graphene.Schema(query=Query) result = schema.execute( @@ -53,7 +54,10 @@ def resolve_reporter(self, info, **args): assert result.data == expected -def test_should_query_nested_field(): +@pytest.mark.parametrize("max_limit,does_count", [(None, True), (100, False)]) +def test_should_query_nested_field(graphene_settings, max_limit, does_count): + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = max_limit + r1 = Reporter(last_name="ABA") r1.save() r2 = Reporter(last_name="Griffin") @@ -111,11 +115,18 @@ def resolve_reporter(self, info, **args): assert not result.errors query = str(Reporter.objects.order_by("pk")[:1].query) assert result.data["__debug"]["sql"][0]["rawSql"] == query - assert "COUNT" in result.data["__debug"]["sql"][1]["rawSql"] - assert "tests_reporter_pets" in result.data["__debug"]["sql"][2]["rawSql"] - assert "COUNT" in result.data["__debug"]["sql"][3]["rawSql"] - assert "tests_reporter_pets" in result.data["__debug"]["sql"][4]["rawSql"] - assert len(result.data["__debug"]["sql"]) == 5 + if does_count: + assert "COUNT" in result.data["__debug"]["sql"][1]["rawSql"] + assert "tests_reporter_pets" in result.data["__debug"]["sql"][2]["rawSql"] + assert "COUNT" in result.data["__debug"]["sql"][3]["rawSql"] + assert "tests_reporter_pets" in result.data["__debug"]["sql"][4]["rawSql"] + assert len(result.data["__debug"]["sql"]) == 5 + else: + assert len(result.data["__debug"]["sql"]) == 3 + for i in range(len(result.data["__debug"]["sql"])): + assert "COUNT" not in result.data["__debug"]["sql"][i]["rawSql"] + assert "tests_reporter_pets" in result.data["__debug"]["sql"][1]["rawSql"] + assert "tests_reporter_pets" in result.data["__debug"]["sql"][2]["rawSql"] assert result.data["reporter"] == expected["reporter"] @@ -133,7 +144,7 @@ class Meta: class Query(graphene.ObjectType): all_reporters = graphene.List(ReporterType) - debug = graphene.Field(DjangoDebug, name="_debug") + debug = graphene.Field(DjangoDebug, name="__debug") def resolve_all_reporters(self, info, **args): return Reporter.objects.all() @@ -143,7 +154,7 @@ def resolve_all_reporters(self, info, **args): allReporters { lastName } - _debug { + __debug { sql { rawSql } @@ -152,7 +163,7 @@ def resolve_all_reporters(self, info, **args): """ expected = { "allReporters": [{"lastName": "ABA"}, {"lastName": "Griffin"}], - "_debug": {"sql": [{"rawSql": str(Reporter.objects.all().query)}]}, + "__debug": {"sql": [{"rawSql": str(Reporter.objects.all().query)}]}, } schema = graphene.Schema(query=Query) result = schema.execute( @@ -162,7 +173,10 @@ def resolve_all_reporters(self, info, **args): assert result.data == expected -def test_should_query_connection(): +@pytest.mark.parametrize("max_limit,does_count", [(None, True), (100, False)]) +def test_should_query_connection(graphene_settings, max_limit, does_count): + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = max_limit + r1 = Reporter(last_name="ABA") r1.save() r2 = Reporter(last_name="Griffin") @@ -175,7 +189,7 @@ class Meta: class Query(graphene.ObjectType): all_reporters = DjangoConnectionField(ReporterType) - debug = graphene.Field(DjangoDebug, name="_debug") + debug = graphene.Field(DjangoDebug, name="__debug") def resolve_all_reporters(self, info, **args): return Reporter.objects.all() @@ -189,7 +203,7 @@ def resolve_all_reporters(self, info, **args): } } } - _debug { + __debug { sql { rawSql } @@ -203,12 +217,22 @@ def resolve_all_reporters(self, info, **args): ) assert not result.errors assert result.data["allReporters"] == expected["allReporters"] - assert "COUNT" in result.data["_debug"]["sql"][0]["rawSql"] - query = str(Reporter.objects.all()[:1].query) - assert result.data["_debug"]["sql"][1]["rawSql"] == query - + if does_count: + assert len(result.data["__debug"]["sql"]) == 2 + assert "COUNT" in result.data["__debug"]["sql"][0]["rawSql"] + query = str(Reporter.objects.all()[:1].query) + assert result.data["__debug"]["sql"][1]["rawSql"] == query + else: + assert len(result.data["__debug"]["sql"]) == 1 + assert "COUNT" not in result.data["__debug"]["sql"][0]["rawSql"] + query = str(Reporter.objects.all()[:1].query) + assert result.data["__debug"]["sql"][0]["rawSql"] == query + + +@pytest.mark.parametrize("max_limit,does_count", [(None, True), (100, False)]) +def test_should_query_connectionfilter(graphene_settings, max_limit, does_count): + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = max_limit -def test_should_query_connectionfilter(): from ...filter import DjangoFilterConnectionField r1 = Reporter(last_name="ABA") @@ -224,7 +248,7 @@ class Meta: class Query(graphene.ObjectType): all_reporters = DjangoFilterConnectionField(ReporterType, fields=["last_name"]) s = graphene.String(resolver=lambda *_: "S") - debug = graphene.Field(DjangoDebug, name="_debug") + debug = graphene.Field(DjangoDebug, name="__debug") def resolve_all_reporters(self, info, **args): return Reporter.objects.all() @@ -238,7 +262,7 @@ def resolve_all_reporters(self, info, **args): } } } - _debug { + __debug { sql { rawSql } @@ -252,6 +276,13 @@ def resolve_all_reporters(self, info, **args): ) assert not result.errors assert result.data["allReporters"] == expected["allReporters"] - assert "COUNT" in result.data["_debug"]["sql"][0]["rawSql"] - query = str(Reporter.objects.all()[:1].query) - assert result.data["_debug"]["sql"][1]["rawSql"] == query + if does_count: + assert len(result.data["__debug"]["sql"]) == 2 + assert "COUNT" in result.data["__debug"]["sql"][0]["rawSql"] + query = str(Reporter.objects.all()[:1].query) + assert result.data["__debug"]["sql"][1]["rawSql"] == query + else: + assert len(result.data["__debug"]["sql"]) == 1 + assert "COUNT" not in result.data["__debug"]["sql"][0]["rawSql"] + query = str(Reporter.objects.all()[:1].query) + assert result.data["__debug"]["sql"][0]["rawSql"] == query diff --git a/graphene_django/fields.py b/graphene_django/fields.py index 02e4a9c1d..9b102bdc8 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -129,6 +129,9 @@ def resolve_queryset(cls, connection, queryset, info, args): @classmethod def resolve_connection(cls, connection, args, iterable, max_limit=None): iterable = maybe_queryset(iterable) + # When slicing from the end, need to retrieve the iterable length. + if args.get("last"): + max_limit = None if isinstance(iterable, QuerySet): _len = max_limit or iterable.count() else: @@ -189,7 +192,9 @@ def connection_resolver( # 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, max_limit=max_limit) + on_resolve = partial( + cls.resolve_connection, connection, args, max_limit=max_limit + ) if Promise.is_thenable(iterable): return Promise.resolve(iterable).then(on_resolve) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index bb9cc8840..fcc53e846 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1130,7 +1130,7 @@ def resolve_films(root, info): } """ schema = graphene.Schema(query=Query) - with django_assert_num_queries(3) as captured: + with django_assert_num_queries(2) as captured: result = schema.execute(query) assert not result.errors From 14ae2f44d3d39bc5a93e72dd3c6d818794683027 Mon Sep 17 00:00:00 2001 From: Paul Craciunoiu Date: Mon, 25 May 2020 23:08:24 -0600 Subject: [PATCH 3/7] black --- graphene_django/debug/tests/test_query.py | 56 ++++++++++++----------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/graphene_django/debug/tests/test_query.py b/graphene_django/debug/tests/test_query.py index 8cc0a3b5f..4c057ed93 100644 --- a/graphene_django/debug/tests/test_query.py +++ b/graphene_django/debug/tests/test_query.py @@ -44,7 +44,9 @@ def resolve_reporter(self, info, **args): """ expected = { "reporter": {"lastName": "ABA"}, - "__debug": {"sql": [{"rawSql": str(Reporter.objects.order_by("pk")[:1].query)}]}, + "__debug": { + "sql": [{"rawSql": str(Reporter.objects.order_by("pk")[:1].query)}] + }, } schema = graphene.Schema(query=Query) result = schema.execute( @@ -116,17 +118,17 @@ def resolve_reporter(self, info, **args): query = str(Reporter.objects.order_by("pk")[:1].query) assert result.data["__debug"]["sql"][0]["rawSql"] == query if does_count: - assert "COUNT" in result.data["__debug"]["sql"][1]["rawSql"] - assert "tests_reporter_pets" in result.data["__debug"]["sql"][2]["rawSql"] - assert "COUNT" in result.data["__debug"]["sql"][3]["rawSql"] - assert "tests_reporter_pets" in result.data["__debug"]["sql"][4]["rawSql"] - assert len(result.data["__debug"]["sql"]) == 5 + assert "COUNT" in result.data["__debug"]["sql"][1]["rawSql"] + assert "tests_reporter_pets" in result.data["__debug"]["sql"][2]["rawSql"] + assert "COUNT" in result.data["__debug"]["sql"][3]["rawSql"] + assert "tests_reporter_pets" in result.data["__debug"]["sql"][4]["rawSql"] + assert len(result.data["__debug"]["sql"]) == 5 else: - assert len(result.data["__debug"]["sql"]) == 3 - for i in range(len(result.data["__debug"]["sql"])): - assert "COUNT" not in result.data["__debug"]["sql"][i]["rawSql"] - assert "tests_reporter_pets" in result.data["__debug"]["sql"][1]["rawSql"] - assert "tests_reporter_pets" in result.data["__debug"]["sql"][2]["rawSql"] + assert len(result.data["__debug"]["sql"]) == 3 + for i in range(len(result.data["__debug"]["sql"])): + assert "COUNT" not in result.data["__debug"]["sql"][i]["rawSql"] + assert "tests_reporter_pets" in result.data["__debug"]["sql"][1]["rawSql"] + assert "tests_reporter_pets" in result.data["__debug"]["sql"][2]["rawSql"] assert result.data["reporter"] == expected["reporter"] @@ -218,15 +220,15 @@ def resolve_all_reporters(self, info, **args): assert not result.errors assert result.data["allReporters"] == expected["allReporters"] if does_count: - assert len(result.data["__debug"]["sql"]) == 2 - assert "COUNT" in result.data["__debug"]["sql"][0]["rawSql"] - query = str(Reporter.objects.all()[:1].query) - assert result.data["__debug"]["sql"][1]["rawSql"] == query + assert len(result.data["__debug"]["sql"]) == 2 + assert "COUNT" in result.data["__debug"]["sql"][0]["rawSql"] + query = str(Reporter.objects.all()[:1].query) + assert result.data["__debug"]["sql"][1]["rawSql"] == query else: - assert len(result.data["__debug"]["sql"]) == 1 - assert "COUNT" not in result.data["__debug"]["sql"][0]["rawSql"] - query = str(Reporter.objects.all()[:1].query) - assert result.data["__debug"]["sql"][0]["rawSql"] == query + assert len(result.data["__debug"]["sql"]) == 1 + assert "COUNT" not in result.data["__debug"]["sql"][0]["rawSql"] + query = str(Reporter.objects.all()[:1].query) + assert result.data["__debug"]["sql"][0]["rawSql"] == query @pytest.mark.parametrize("max_limit,does_count", [(None, True), (100, False)]) @@ -277,12 +279,12 @@ def resolve_all_reporters(self, info, **args): assert not result.errors assert result.data["allReporters"] == expected["allReporters"] if does_count: - assert len(result.data["__debug"]["sql"]) == 2 - assert "COUNT" in result.data["__debug"]["sql"][0]["rawSql"] - query = str(Reporter.objects.all()[:1].query) - assert result.data["__debug"]["sql"][1]["rawSql"] == query + assert len(result.data["__debug"]["sql"]) == 2 + assert "COUNT" in result.data["__debug"]["sql"][0]["rawSql"] + query = str(Reporter.objects.all()[:1].query) + assert result.data["__debug"]["sql"][1]["rawSql"] == query else: - assert len(result.data["__debug"]["sql"]) == 1 - assert "COUNT" not in result.data["__debug"]["sql"][0]["rawSql"] - query = str(Reporter.objects.all()[:1].query) - assert result.data["__debug"]["sql"][0]["rawSql"] == query + assert len(result.data["__debug"]["sql"]) == 1 + assert "COUNT" not in result.data["__debug"]["sql"][0]["rawSql"] + query = str(Reporter.objects.all()[:1].query) + assert result.data["__debug"]["sql"][0]["rawSql"] == query From 201fd194517e8c3bd3c3cf096c574b0b757994c4 Mon Sep 17 00:00:00 2001 From: Paul Craciunoiu Date: Sat, 6 Jun 2020 11:39:03 -0600 Subject: [PATCH 4/7] Add test for max_limit. --- graphene_django/tests/test_query.py | 42 +++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index fcc53e846..92b810115 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1084,6 +1084,48 @@ class Query(graphene.ObjectType): assert result.data == expected +REPORTERS = [ + dict( + first_name=f"First {i}", + last_name=f"Last {i}", + email=f"johndoe+{i}@example.com", + a_choice=1, + ) + for i in range(6) +] + + +def test_should_return_max_limit(graphene_settings): + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = 4 + reporters = [Reporter(**kwargs) for kwargs in REPORTERS] + Reporter.objects.bulk_create(reporters) + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query AllReporters { + allReporters { + edges { + node { + id + } + } + } + } + """ + + result = schema.execute(query) + assert not result.errors + assert len(result.data["allReporters"]["edges"]) == 4 + + def test_should_preserve_prefetch_related(django_assert_num_queries): class ReporterType(DjangoObjectType): class Meta: From 82c944a89a4e80ada97d0d7ad05b6967b5301795 Mon Sep 17 00:00:00 2001 From: Paul Craciunoiu Date: Sat, 6 Jun 2020 11:42:44 -0600 Subject: [PATCH 5/7] Older python syntax. --- graphene_django/tests/test_query.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 92b810115..3d480b233 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1086,9 +1086,9 @@ class Query(graphene.ObjectType): REPORTERS = [ dict( - first_name=f"First {i}", - last_name=f"Last {i}", - email=f"johndoe+{i}@example.com", + first_name="First {}".format(i), + last_name=f"Last {}".format(i), + email="johndoe+{i}@example.com".format(i), a_choice=1, ) for i in range(6) From 038fbe62b0f45c9f12647b900192f1849e19831b Mon Sep 17 00:00:00 2001 From: Paul Craciunoiu Date: Sat, 6 Jun 2020 11:44:56 -0600 Subject: [PATCH 6/7] Stray f-string --- graphene_django/tests/test_query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 3d480b233..477d14758 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1087,7 +1087,7 @@ class Query(graphene.ObjectType): REPORTERS = [ dict( first_name="First {}".format(i), - last_name=f"Last {}".format(i), + last_name="Last {}".format(i), email="johndoe+{i}@example.com".format(i), a_choice=1, ) From 66a09ae34fc483394c87c7270634a907c248f05e Mon Sep 17 00:00:00 2001 From: Paul Craciunoiu Date: Sat, 6 Jun 2020 11:47:28 -0600 Subject: [PATCH 7/7] Format --- graphene_django/tests/test_query.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 477d14758..e6ed49e8e 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1088,7 +1088,7 @@ class Query(graphene.ObjectType): dict( first_name="First {}".format(i), last_name="Last {}".format(i), - email="johndoe+{i}@example.com".format(i), + email="johndoe+{}@example.com".format(i), a_choice=1, ) for i in range(6)