From ae3d812a25faca89ca93a20c1c6047ceb15e0146 Mon Sep 17 00:00:00 2001 From: Erik Wrede Date: Tue, 24 Jan 2023 23:07:58 +0100 Subject: [PATCH 1/4] fix: Make ORMField(type_) work in case there is no registered converter --- graphene_sqlalchemy/converter.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/graphene_sqlalchemy/converter.py b/graphene_sqlalchemy/converter.py index 26f5b3a..8c7cd7a 100644 --- a/graphene_sqlalchemy/converter.py +++ b/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)) From 5ebc8c873f2659f2d7540d651f0460330195a647 Mon Sep 17 00:00:00 2001 From: Erik Wrede Date: Tue, 24 Jan 2023 23:08:04 +0100 Subject: [PATCH 2/4] revert/feat!: Type Converters support subtypes again. this feature adjusts the conversion system to use the MRO of a supplied class --- graphene_sqlalchemy/utils.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/graphene_sqlalchemy/utils.py b/graphene_sqlalchemy/utils.py index 1bf361f..ac5be88 100644 --- a/graphene_sqlalchemy/utils.py +++ b/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) From 57dc4a09d6deb969e1dede2daabdb54ea1e290bb Mon Sep 17 00:00:00 2001 From: Erik Wrede Date: Wed, 25 Jan 2023 11:58:37 +0100 Subject: [PATCH 3/4] tests: add test cases for mro & orm field fixes --- graphene_sqlalchemy/tests/models.py | 38 +++++++++++++++++++++ graphene_sqlalchemy/tests/test_converter.py | 35 ++++++++++++++++++- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/graphene_sqlalchemy/tests/models.py b/graphene_sqlalchemy/tests/models.py index 9531aaa..5acbc6f 100644 --- a/graphene_sqlalchemy/tests/models.py +++ b/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 --git a/graphene_sqlalchemy/tests/test_converter.py b/graphene_sqlalchemy/tests/test_converter.py index b4c6eb2..6e6733a 100644 --- a/graphene_sqlalchemy/tests/test_converter.py +++ b/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,38 @@ 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. + """ + field = get_field_from_column(Column(sqlalchemy.dialects.mysql.BIGINT)) + + assert field.type == graphene.Float + + def test_sqlalchemy_hybrid_property_type_inference(): class ShoppingCartItemType(SQLAlchemyObjectType): class Meta: From 1595d4e1ca27a9258cac9039c532b4dbd6ea0736 Mon Sep 17 00:00:00 2001 From: Erik Wrede Date: Wed, 25 Jan 2023 12:04:26 +0100 Subject: [PATCH 4/4] tests: use custom type instead of BIGINT due to version incompatibilities --- graphene_sqlalchemy/tests/test_converter.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/graphene_sqlalchemy/tests/test_converter.py b/graphene_sqlalchemy/tests/test_converter.py index 6e6733a..f70a50f 100644 --- a/graphene_sqlalchemy/tests/test_converter.py +++ b/graphene_sqlalchemy/tests/test_converter.py @@ -773,7 +773,11 @@ def test_match_supertype_from_mro_correct_order(): 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. """ - field = get_field_from_column(Column(sqlalchemy.dialects.mysql.BIGINT)) + + class BIGINT(sqlalchemy.types.BigInteger): + pass + + field = get_field_from_column(Column(BIGINT)) assert field.type == graphene.Float