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

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))
 

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)

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:

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)