From d8c3dc99ff88698c6d38255dd20c2b98c8c570f2 Mon Sep 17 00:00:00 2001 From: Dominique PERETTI Date: Tue, 18 Jun 2019 22:56:25 +0200 Subject: [PATCH 1/3] Enforce NonNull for returned related Sets and their content. https://github.com/graphql-python/graphene-django/issues/448 --- graphene_django/converter.py | 4 +++- graphene_django/fields.py | 3 ++- graphene_django/tests/test_converter.py | 17 +++++++++++++---- graphene_django/tests/test_types.py | 2 +- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/graphene_django/converter.py b/graphene_django/converter.py index 4d0b45f95..6b687fe45 100644 --- a/graphene_django/converter.py +++ b/graphene_django/converter.py @@ -198,7 +198,9 @@ def dynamic_type(): return DjangoConnectionField(_type, description=description) - return DjangoListField(_type, description=description) + return DjangoListField(_type, + required=True, # A Set is always returned, never None. + description=description) return Dynamic(dynamic_type) diff --git a/graphene_django/fields.py b/graphene_django/fields.py index 791e78544..8c8fa2bd7 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -15,7 +15,8 @@ class DjangoListField(Field): def __init__(self, _type, *args, **kwargs): - super(DjangoListField, self).__init__(List(_type), *args, **kwargs) + # Django would never return a Set of None vvvvvvv + super(DjangoListField, self).__init__(List(NonNull(_type)), *args, **kwargs) @property def model(self): diff --git a/graphene_django/tests/test_converter.py b/graphene_django/tests/test_converter.py index 5542c90a7..c819a6566 100644 --- a/graphene_django/tests/test_converter.py +++ b/graphene_django/tests/test_converter.py @@ -1,6 +1,7 @@ import pytest from django.db import models from django.utils.translation import ugettext_lazy as _ +from graphene import NonNull from py.test import raises import graphene @@ -234,8 +235,12 @@ class Meta: assert isinstance(graphene_field, graphene.Dynamic) dynamic_field = graphene_field.get_type() assert isinstance(dynamic_field, graphene.Field) - assert isinstance(dynamic_field.type, graphene.List) - assert dynamic_field.type.of_type == A + # A NonNull List of NonNull A ([A!]!) + # https://github.com/graphql-python/graphene-django/issues/448 + assert isinstance(dynamic_field.type, NonNull) + assert isinstance(dynamic_field.type.of_type, graphene.List) + assert isinstance(dynamic_field.type.of_type.of_type, NonNull) + assert dynamic_field.type.of_type.of_type.of_type == A def test_should_manytomany_convert_connectionorlist_connection(): @@ -262,8 +267,12 @@ class Meta: assert isinstance(graphene_field, graphene.Dynamic) dynamic_field = graphene_field.get_type() assert isinstance(dynamic_field, graphene.Field) - assert isinstance(dynamic_field.type, graphene.List) - assert dynamic_field.type.of_type == A + # a NonNull List of NonNull A ([A!]!) + assert isinstance(dynamic_field.type, NonNull) + assert isinstance(dynamic_field.type.of_type, graphene.List) + assert isinstance(dynamic_field.type.of_type.of_type, NonNull) + assert isinstance(dynamic_field.type.of_type.of_type, NonNull) + assert dynamic_field.type.of_type.of_type.of_type == A def test_should_onetoone_reverse_convert_model(): diff --git a/graphene_django/tests/test_types.py b/graphene_django/tests/test_types.py index c1ac6c2bc..6f5ab7ed7 100644 --- a/graphene_django/tests/test_types.py +++ b/graphene_django/tests/test_types.py @@ -170,7 +170,7 @@ def test_schema_representation(): firstName: String! lastName: String! email: String! - pets: [Reporter] + pets: [Reporter!]! aChoice: ReporterAChoice! reporterType: ReporterReporterType articles(before: String, after: String, first: Int, last: Int): ArticleConnection From 2030aa2716002a02bff98170a3a5d7fd5cd5e2e5 Mon Sep 17 00:00:00 2001 From: Dominique PERETTI Date: Tue, 18 Jun 2019 23:32:16 +0200 Subject: [PATCH 2/3] Run format. --- graphene_django/converter.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/graphene_django/converter.py b/graphene_django/converter.py index 6b687fe45..64bf341a9 100644 --- a/graphene_django/converter.py +++ b/graphene_django/converter.py @@ -198,9 +198,11 @@ def dynamic_type(): return DjangoConnectionField(_type, description=description) - return DjangoListField(_type, - required=True, # A Set is always returned, never None. - description=description) + return DjangoListField( + _type, + required=True, # A Set is always returned, never None. + description=description, + ) return Dynamic(dynamic_type) From 861d0d1472e9bf4d5088797a7dc4fba2fc5d3ebf Mon Sep 17 00:00:00 2001 From: Jonathan Kim Date: Tue, 25 Jun 2019 11:14:21 +0100 Subject: [PATCH 3/3] Remove duplicate assertion --- graphene_django/tests/test_converter.py | 1 - 1 file changed, 1 deletion(-) diff --git a/graphene_django/tests/test_converter.py b/graphene_django/tests/test_converter.py index c819a6566..00467b4f9 100644 --- a/graphene_django/tests/test_converter.py +++ b/graphene_django/tests/test_converter.py @@ -271,7 +271,6 @@ class Meta: assert isinstance(dynamic_field.type, NonNull) assert isinstance(dynamic_field.type.of_type, graphene.List) assert isinstance(dynamic_field.type.of_type.of_type, NonNull) - assert isinstance(dynamic_field.type.of_type.of_type, NonNull) assert dynamic_field.type.of_type.of_type.of_type == A