refactor convert_sqlalchemy_relationship

Author: jnak

Diff for: graphene_sqlalchemy/converter.py

@@ -34,47 +34,79 @@ def is_column_nullable(column):
 
 
 def convert_sqlalchemy_relationship(relationship_prop, obj_type, connection_field_factory, batching,
-                                    attr_name, orm_field_name, **field_kwargs):
+                                    orm_field_name, **field_kwargs):
     """
     :param sqlalchemy.RelationshipProperty relationship_prop:
-    :param Registry registry:
-    :type function|None connection_field_factory:
-    :type bool batching:
     :param SQLAlchemyObjectType obj_type:
+    :param function|None connection_field_factory:
+    :param bool batching:
     :param str orm_field_name:
+    :param dict field_kwargs:
     :rtype: Dynamic
     """
     def dynamic_type():
+        """:rtype: Field|None"""
         direction = relationship_prop.direction
-        model = relationship_prop.mapper.entity
-        type_ = obj_type._meta.registry.get_type_for_model(model)
-
+        child_type = obj_type._meta.registry.get_type_for_model(relationship_prop.mapper.entity)
         batching_ = batching if is_selectin_available else False
-        connection_field_factory_ = connection_field_factory
 
-        if not type_:
+        if not child_type:
             return None
 
         if direction == interfaces.MANYTOONE or not relationship_prop.uselist:
-            resolver = get_custom_resolver(obj_type, orm_field_name)
-            if resolver is None:
-                resolver = get_batch_resolver(relationship_prop) if batching_ else \
-                    get_attr_resolver(obj_type, relationship_prop.key)
-
-            return Field(type_, resolver=resolver, **field_kwargs)
+            return _convert_o2o_or_m2o_relationship(relationship_prop, obj_type, batching_, orm_field_name,
+                                                    **field_kwargs)
 
         if direction in (interfaces.ONETOMANY, interfaces.MANYTOMANY):
-            if not type_._meta.connection:
-                return Field(List(type_), **field_kwargs)
+            return _convert_o2m_or_m2m_relationship(relationship_prop, obj_type, batching_,
+                                                    connection_field_factory, **field_kwargs)
+
+    return Dynamic(dynamic_type)
 
-            if connection_field_factory_ is None:
-                connection_field_factory_ = BatchSQLAlchemyConnectionField.from_relationship if batching_ else \
-                    default_connection_field_factory
 
-            # TODO Allow override of connection_field_factory and resolver via ORMField
-            return connection_field_factory_(relationship_prop, obj_type._meta.registry, **field_kwargs)
+def _convert_o2o_or_m2o_relationship(relationship_prop, obj_type, batching, orm_field_name, **field_kwargs):
+    """
+    Convert one-to-one or many-to-one relationshsip. Return an object field.
 
-    return Dynamic(dynamic_type)
+    :param sqlalchemy.RelationshipProperty relationship_prop:
+    :param SQLAlchemyObjectType obj_type:
+    :param bool batching:
+    :param str orm_field_name:
+    :param dict field_kwargs:
+    :rtype: Field
+    """
+    child_type = obj_type._meta.registry.get_type_for_model(relationship_prop.mapper.entity)
+
+    resolver = get_custom_resolver(obj_type, orm_field_name)
+    if resolver is None:
+        resolver = get_batch_resolver(relationship_prop) if batching else \
+            get_attr_resolver(obj_type, relationship_prop.key)
+
+    return Field(child_type, resolver=resolver, **field_kwargs)
+
+
+def _convert_o2m_or_m2m_relationship(relationship_prop, obj_type, batching, connection_field_factory, **field_kwargs):
+    """
+    Convert one-to-many or many-to-many relationshsip. Return a list field or a connection field.
+
+    :param sqlalchemy.RelationshipProperty relationship_prop:
+    :param SQLAlchemyObjectType obj_type:
+    :param bool batching:
+    :param function|None connection_field_factory:
+    :param dict field_kwargs:
+    :rtype: Field
+    """
+    child_type = obj_type._meta.registry.get_type_for_model(relationship_prop.mapper.entity)
+
+    if not child_type._meta.connection:
+        return Field(List(child_type), **field_kwargs)
+
+    # TODO Allow override of connection_field_factory and resolver via ORMField
+    if connection_field_factory is None:
+        connection_field_factory = BatchSQLAlchemyConnectionField.from_relationship if batching else \
+            default_connection_field_factory
+
+    return connection_field_factory(relationship_prop, obj_type._meta.registry, **field_kwargs)
 
 
 def convert_sqlalchemy_hybrid_method(hybrid_prop, resolver, **field_kwargs):

Diff for: graphene_sqlalchemy/tests/test_converter.py

@@ -195,7 +195,7 @@ class Meta:
             model = Article
 
     dynamic_field = convert_sqlalchemy_relationship(
-        Reporter.pets.property, A, default_connection_field_factory, True, 'attr_name', 'orm_field_name',
+        Reporter.pets.property, A, default_connection_field_factory, True, 'orm_field_name',
     )
     assert isinstance(dynamic_field, graphene.Dynamic)
     assert not dynamic_field.get_type()
@@ -207,7 +207,7 @@ class Meta:
             model = Pet
 
     dynamic_field = convert_sqlalchemy_relationship(
-        Reporter.pets.property, A, default_connection_field_factory, True, 'attr_name', 'orm_field_name',
+        Reporter.pets.property, A, default_connection_field_factory, True, 'orm_field_name',
     )
     assert isinstance(dynamic_field, graphene.Dynamic)
     graphene_type = dynamic_field.get_type()
@@ -223,7 +223,7 @@ class Meta:
             interfaces = (Node,)
 
     dynamic_field = convert_sqlalchemy_relationship(
-        Reporter.pets.property, A, default_connection_field_factory, True, 'attr_name', 'orm_field_name',
+        Reporter.pets.property, A, default_connection_field_factory, True, 'orm_field_name',
     )
     assert isinstance(dynamic_field, graphene.Dynamic)
     assert isinstance(dynamic_field.get_type(), UnsortedSQLAlchemyConnectionField)
@@ -235,7 +235,7 @@ class Meta:
             model = Article
 
     dynamic_field = convert_sqlalchemy_relationship(
-        Reporter.pets.property, A, default_connection_field_factory, True, 'attr_name', 'orm_field_name',
+        Reporter.pets.property, A, default_connection_field_factory, True, 'orm_field_name',
     )
     assert isinstance(dynamic_field, graphene.Dynamic)
     assert not dynamic_field.get_type()
@@ -247,7 +247,7 @@ class Meta:
             model = Reporter
 
     dynamic_field = convert_sqlalchemy_relationship(
-        Article.reporter.property, A, default_connection_field_factory, True, 'attr_name', 'orm_field_name',
+        Article.reporter.property, A, default_connection_field_factory, True, 'orm_field_name',
     )
     assert isinstance(dynamic_field, graphene.Dynamic)
     graphene_type = dynamic_field.get_type()
@@ -262,7 +262,7 @@ class Meta:
             interfaces = (Node,)
 
     dynamic_field = convert_sqlalchemy_relationship(
-        Article.reporter.property, A, default_connection_field_factory, True, 'attr_name', 'orm_field_name',
+        Article.reporter.property, A, default_connection_field_factory, True, 'orm_field_name',
     )
     assert isinstance(dynamic_field, graphene.Dynamic)
     graphene_type = dynamic_field.get_type()
@@ -277,7 +277,7 @@ class Meta:
             interfaces = (Node,)
 
     dynamic_field = convert_sqlalchemy_relationship(
-        Reporter.favorite_article.property, A, default_connection_field_factory, True, 'attr_name', 'orm_field_name',
+        Reporter.favorite_article.property, A, default_connection_field_factory, True, 'orm_field_name',
     )
     assert isinstance(dynamic_field, graphene.Dynamic)
     graphene_type = dynamic_field.get_type()

Diff for: graphene_sqlalchemy/types.py

@@ -162,8 +162,7 @@ def construct_fields(
         elif isinstance(attr, RelationshipProperty):
             batching_ = orm_field.kwargs.pop('batching', batching)
             field = convert_sqlalchemy_relationship(
-                attr, obj_type, connection_field_factory, batching_, attr_name,
-                orm_field_name, **orm_field.kwargs)
+                attr, obj_type, connection_field_factory, batching_, orm_field_name, **orm_field.kwargs)
         elif isinstance(attr, CompositeProperty):
             if attr_name != orm_field_name or orm_field.kwargs:
                 # TODO Add a way to override composite property fields