Skip to content

Stricter non-null fields for relationships #367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion graphene_sqlalchemy/converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down
35 changes: 35 additions & 0 deletions graphene_sqlalchemy/tests/test_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions graphene_sqlalchemy/tests/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"


Expand Down