fix: allow type converter inheritance again (#377)

* fix: Make ORMField(type_) work in case there is no registered converter

* revert/feat!: Type Converters support subtypes again.
this feature adjusts the conversion system to use the MRO of a supplied class

* tests: add test cases for mro & orm field fixes

* tests: use custom type instead of BIGINT due to version incompatibilities

Author: erikwrede

Diff for: graphene_sqlalchemy/converter.py

@@ -261,16 +261,17 @@ def inner(fn):
 
 def convert_sqlalchemy_column(column_prop, registry, resolver, **field_kwargs):
     column = column_prop.columns[0]
-    column_type = getattr(column, "type", None)
     # The converter expects a type to find the right conversion function.
     # If we get an instance instead, we need to convert it to a type.
     # The conversion function will still be able to access the instance via the column argument.
-    if not isinstance(column_type, type):
-        column_type = type(column_type)
-    field_kwargs.setdefault(
-        "type_",
-        convert_sqlalchemy_type(column_type, column=column, registry=registry),
-    )
+    if "type_" not in field_kwargs:
+        column_type = getattr(column, "type", None)
+        if not isinstance(column_type, type):
+            column_type = type(column_type)
+        field_kwargs.setdefault(
+            "type_",
+            convert_sqlalchemy_type(column_type, column=column, registry=registry),
+        )
     field_kwargs.setdefault("required", not is_column_nullable(column))
     field_kwargs.setdefault("description", get_column_doc(column))
 

Diff for: graphene_sqlalchemy/tests/models.py

@@ -21,6 +21,8 @@
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.orm import backref, column_property, composite, mapper, relationship
+from sqlalchemy.sql.sqltypes import _LookupExpressionAdapter
+from sqlalchemy.sql.type_api import TypeEngine
 
 PetKind = Enum("cat", "dog", name="pet_kind")
 
@@ -328,3 +330,39 @@ class Employee(Person):
     __mapper_args__ = {
         "polymorphic_identity": "employee",
     }
+
+
+############################################
+# Custom Test Models
+############################################
+
+
+class CustomIntegerColumn(_LookupExpressionAdapter, TypeEngine):
+    """
+    Custom Column Type that our converters don't recognize
+    Adapted from sqlalchemy.Integer
+    """
+
+    """A type for ``int`` integers."""
+
+    __visit_name__ = "integer"
+
+    def get_dbapi_type(self, dbapi):
+        return dbapi.NUMBER
+
+    @property
+    def python_type(self):
+        return int
+
+    def literal_processor(self, dialect):
+        def process(value):
+            return str(int(value))
+
+        return process
+
+
+class CustomColumnModel(Base):
+    __tablename__ = "customcolumnmodel"
+
+    id = Column(Integer(), primary_key=True)
+    custom_col = Column(CustomIntegerColumn)

Diff for: graphene_sqlalchemy/tests/test_converter.py

@@ -3,6 +3,7 @@
 from typing import Dict, Tuple, Union
 
 import pytest
+import sqlalchemy
 import sqlalchemy_utils as sqa_utils
 from sqlalchemy import Column, func, select, types
 from sqlalchemy.dialects import postgresql
@@ -29,6 +30,7 @@
 from .models import (
     Article,
     CompositeFullName,
+    CustomColumnModel,
     Pet,
     Reporter,
     ShoppingCart,
@@ -81,7 +83,6 @@ def use_legacy_many_relationships():
         set_non_null_many_relationships(True)
 
 
-
 def test_hybrid_prop_int():
     @hybrid_property
     def prop_method() -> int:
@@ -745,6 +746,42 @@ def __init__(self, col1, col2):
         )
 
 
+def test_raise_exception_unkown_column_type():
+    with pytest.raises(
+        Exception,
+        match="Don't know how to convert the SQLAlchemy field customcolumnmodel.custom_col",
+    ):
+
+        class A(SQLAlchemyObjectType):
+            class Meta:
+                model = CustomColumnModel
+
+
+def test_prioritize_orm_field_unkown_column_type():
+    class A(SQLAlchemyObjectType):
+        class Meta:
+            model = CustomColumnModel
+
+        custom_col = ORMField(type_=graphene.Int)
+
+    assert A._meta.fields["custom_col"].type == graphene.Int
+
+
+def test_match_supertype_from_mro_correct_order():
+    """
+    BigInt and Integer are both superclasses of BIGINT, but a custom converter exists for BigInt that maps to Float.
+    We expect the correct MRO order to be used and conversion by the nearest match. BIGINT should be converted to Float,
+    just like BigInt, not to Int like integer which is further up in the MRO.
+    """
+
+    class BIGINT(sqlalchemy.types.BigInteger):
+        pass
+
+    field = get_field_from_column(Column(BIGINT))
+
+    assert field.type == graphene.Float
+
+
 def test_sqlalchemy_hybrid_property_type_inference():
     class ShoppingCartItemType(SQLAlchemyObjectType):
         class Meta:

Diff for: graphene_sqlalchemy/utils.py

@@ -1,6 +1,7 @@
 import re
 import warnings
 from collections import OrderedDict
+from functools import _c3_mro
 from typing import Any, Callable, Dict, Optional
 
 import pkg_resources
@@ -188,10 +189,19 @@ def __init__(self, default: Callable):
         self.default = default
 
     def __call__(self, *args, **kwargs):
-        for matcher_function, final_method in self.registry.items():
-            # Register order is important. First one that matches, runs.
-            if matcher_function(args[0]):
-                return final_method(*args, **kwargs)
+        matched_arg = args[0]
+        try:
+            mro = _c3_mro(matched_arg)
+        except Exception:
+            # In case of tuples or similar types, we can't use the MRO.
+            # Fall back to just matching the original argument.
+            mro = [matched_arg]
+
+        for cls in mro:
+            for matcher_function, final_method in self.registry.items():
+                # Register order is important. First one that matches, runs.
+                if matcher_function(cls):
+                    return final_method(*args, **kwargs)
 
         # No match, using default.
         return self.default(*args, **kwargs)