From 54cc6a4b13c18b8efebccaacd8ac8df93bf56949 Mon Sep 17 00:00:00 2001 From: Jonathan Kim Date: Tue, 25 Jun 2019 16:30:30 +0100 Subject: [PATCH] Enforce NonNull for returned related Sets and their content (#690) * Enforce NonNull for returned related Sets and their content. https://github.com/graphql-python/graphene-django/issues/448 * Run format. * Remove duplicate assertion --- graphene_django/converter.py | 6 +++++- graphene_django/fields.py | 3 ++- graphene_django/tests/test_converter.py | 16 ++++++++++++---- graphene_django/tests/test_types.py | 2 +- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/graphene_django/converter.py b/graphene_django/converter.py index 4d0b45f..64bf341 100644 --- a/graphene_django/converter.py +++ b/graphene_django/converter.py @@ -198,7 +198,11 @@ def convert_field_to_list_or_connection(field, registry=None): 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 791e785..8c8fa2b 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -15,7 +15,8 @@ from .utils import maybe_queryset 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 5542c90..00467b4 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 @@ def test_should_manytomany_convert_connectionorlist_list(): 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,11 @@ def test_should_manytoone_convert_connectionorlist(): 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 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 c1ac6c2..6f5ab7e 100644 --- a/graphene_django/tests/test_types.py +++ b/graphene_django/tests/test_types.py @@ -170,7 +170,7 @@ type Reporter { firstName: String! lastName: String! email: String! - pets: [Reporter] + pets: [Reporter!]! aChoice: ReporterAChoice! reporterType: ReporterReporterType articles(before: String, after: String, first: Int, last: Int): ArticleConnection