diff --git a/graphene_sqlalchemy/converter.py b/graphene_sqlalchemy/converter.py index 7c5330b..26f5b3a 100644 --- a/graphene_sqlalchemy/converter.py +++ b/graphene_sqlalchemy/converter.py @@ -59,6 +59,39 @@ is_selectin_available = getattr(strategies, "SelectInLoader", None) +""" +Flag for whether to generate stricter non-null fields for many-relationships. + +For many-relationships, both the list element and the list field itself will be +non-null by default. This better matches ORM semantics, where there is always a +list for a many relationship (even if it is empty), and it never contains None. + +This option can be set to False to revert to pre-3.0 behavior. + +For example, given a User model with many Comments: + + class User(Base): + comments = relationship("Comment") + +The Schema will be: + + type User { + comments: [Comment!]! + } + +When set to False, the pre-3.0 behavior gives: + + type User { + comments: [Comment] + } +""" +use_non_null_many_relationships = True + + +def set_non_null_many_relationships(non_null_flag): + global use_non_null_many_relationships + use_non_null_many_relationships = non_null_flag + def get_column_doc(column): return getattr(column, "doc", None) @@ -160,7 +193,14 @@ def _convert_o2m_or_m2m_relationship( ) if not child_type._meta.connection: - return graphene.Field(graphene.List(child_type), **field_kwargs) + # check if we need to use non-null fields + list_type = ( + graphene.NonNull(graphene.List(graphene.NonNull(child_type))) + if use_non_null_many_relationships + else graphene.List(child_type) + ) + + return graphene.Field(list_type, **field_kwargs) # TODO Allow override of connection_field_factory and resolver via ORMField if connection_field_factory is None: diff --git a/graphene_sqlalchemy/tests/test_converter.py b/graphene_sqlalchemy/tests/test_converter.py index e903396..b4c6eb2 100644 --- a/graphene_sqlalchemy/tests/test_converter.py +++ b/graphene_sqlalchemy/tests/test_converter.py @@ -21,6 +21,7 @@ convert_sqlalchemy_hybrid_method, convert_sqlalchemy_relationship, convert_sqlalchemy_type, + set_non_null_many_relationships, ) from ..fields import UnsortedSQLAlchemyConnectionField, default_connection_field_factory from ..registry import Registry, get_global_registry @@ -71,6 +72,16 @@ class Model(declarative_base()): ) +@pytest.fixture +def use_legacy_many_relationships(): + set_non_null_many_relationships(False) + try: + yield + finally: + set_non_null_many_relationships(True) + + + def test_hybrid_prop_int(): @hybrid_property def prop_method() -> int: @@ -501,6 +512,30 @@ class Meta: True, "orm_field_name", ) + # field should be [A!]! + assert isinstance(dynamic_field, graphene.Dynamic) + graphene_type = dynamic_field.get_type() + assert isinstance(graphene_type, graphene.Field) + assert isinstance(graphene_type.type, graphene.NonNull) + assert isinstance(graphene_type.type.of_type, graphene.List) + assert isinstance(graphene_type.type.of_type.of_type, graphene.NonNull) + assert graphene_type.type.of_type.of_type.of_type == A + + +@pytest.mark.usefixtures("use_legacy_many_relationships") +def test_should_manytomany_convert_connectionorlist_list_legacy(): + class A(SQLAlchemyObjectType): + class Meta: + model = Pet + + dynamic_field = convert_sqlalchemy_relationship( + Reporter.pets.property, + A, + default_connection_field_factory, + True, + "orm_field_name", + ) + # field should be [A] assert isinstance(dynamic_field, graphene.Dynamic) graphene_type = dynamic_field.get_type() assert isinstance(graphene_type, graphene.Field) diff --git a/graphene_sqlalchemy/tests/test_types.py b/graphene_sqlalchemy/tests/test_types.py index 6632842..3de443d 100644 --- a/graphene_sqlalchemy/tests/test_types.py +++ b/graphene_sqlalchemy/tests/test_types.py @@ -331,8 +331,10 @@ class Meta: pets_field = ReporterType._meta.fields["pets"] assert isinstance(pets_field, Dynamic) - assert isinstance(pets_field.type().type, List) - assert pets_field.type().type.of_type == PetType + assert isinstance(pets_field.type().type, NonNull) + assert isinstance(pets_field.type().type.of_type, List) + assert isinstance(pets_field.type().type.of_type.of_type, NonNull) + assert pets_field.type().type.of_type.of_type.of_type == PetType assert pets_field.type().description == "Overridden"