From 78004a6470c1f2d39357da4e5632abfd244cbe90 Mon Sep 17 00:00:00 2001 From: Frederick Polgardy Date: Mon, 24 Oct 2022 14:34:51 -0600 Subject: [PATCH 1/9] Support GQL interfaces for polymorphic SQLA models --- graphene_sqlalchemy/__init__.py | 3 +- graphene_sqlalchemy/registry.py | 21 +++----- graphene_sqlalchemy/tests/test_registry.py | 4 +- graphene_sqlalchemy/types.py | 57 +++++++++++++++++----- 4 files changed, 55 insertions(+), 30 deletions(-) diff --git a/graphene_sqlalchemy/__init__.py b/graphene_sqlalchemy/__init__.py index 33345815..fb32379c 100644 --- a/graphene_sqlalchemy/__init__.py +++ b/graphene_sqlalchemy/__init__.py @@ -1,11 +1,12 @@ from .fields import SQLAlchemyConnectionField -from .types import SQLAlchemyObjectType +from .types import SQLAlchemyInterface, SQLAlchemyObjectType from .utils import get_query, get_session __version__ = "3.0.0b3" __all__ = [ "__version__", + "SQLAlchemyInterface", "SQLAlchemyObjectType", "SQLAlchemyConnectionField", "get_query", diff --git a/graphene_sqlalchemy/registry.py b/graphene_sqlalchemy/registry.py index 8f2bc9e7..cc4b02b7 100644 --- a/graphene_sqlalchemy/registry.py +++ b/graphene_sqlalchemy/registry.py @@ -18,15 +18,10 @@ def __init__(self): self._registry_unions = {} def register(self, obj_type): + from .types import SQLAlchemyBase - from .types import SQLAlchemyObjectType - - if not isinstance(obj_type, type) or not issubclass( - obj_type, SQLAlchemyObjectType - ): - raise TypeError( - "Expected SQLAlchemyObjectType, but got: {!r}".format(obj_type) - ) + if not isinstance(obj_type, type) or not issubclass(obj_type, SQLAlchemyBase): + raise TypeError("Expected SQLAlchemyBase, but got: {!r}".format(obj_type)) assert obj_type._meta.registry == self, "Registry for a Model have to match." # assert self.get_type_for_model(cls._meta.model) in [None, cls], ( # 'SQLAlchemy model "{}" already associated with ' @@ -38,14 +33,10 @@ def get_type_for_model(self, model): return self._registry.get(model) def register_orm_field(self, obj_type, field_name, orm_field): - from .types import SQLAlchemyObjectType + from .types import SQLAlchemyBase - if not isinstance(obj_type, type) or not issubclass( - obj_type, SQLAlchemyObjectType - ): - raise TypeError( - "Expected SQLAlchemyObjectType, but got: {!r}".format(obj_type) - ) + if not isinstance(obj_type, type) or not issubclass(obj_type, SQLAlchemyBase): + raise TypeError("Expected SQLAlchemyBase, but got: {!r}".format(obj_type)) if not field_name or not isinstance(field_name, str): raise TypeError("Expected a field name, but got: {!r}".format(field_name)) self._registry_orm_fields[obj_type][field_name] = orm_field diff --git a/graphene_sqlalchemy/tests/test_registry.py b/graphene_sqlalchemy/tests/test_registry.py index cb7e9034..68b5404f 100644 --- a/graphene_sqlalchemy/tests/test_registry.py +++ b/graphene_sqlalchemy/tests/test_registry.py @@ -28,7 +28,7 @@ def test_register_incorrect_object_type(): class Spam: pass - re_err = "Expected SQLAlchemyObjectType, but got: .*Spam" + re_err = "Expected SQLAlchemyBase, but got: .*Spam" with pytest.raises(TypeError, match=re_err): reg.register(Spam) @@ -51,7 +51,7 @@ def test_register_orm_field_incorrect_types(): class Spam: pass - re_err = "Expected SQLAlchemyObjectType, but got: .*Spam" + re_err = "Expected SQLAlchemyBase, but got: .*Spam" with pytest.raises(TypeError, match=re_err): reg.register_orm_field(Spam, "name", Pet.name) diff --git a/graphene_sqlalchemy/types.py b/graphene_sqlalchemy/types.py index fe48e9eb..55f59773 100644 --- a/graphene_sqlalchemy/types.py +++ b/graphene_sqlalchemy/types.py @@ -7,6 +7,8 @@ from graphene import Field from graphene.relay import Connection, Node +from graphene.types.base import BaseType +from graphene.types.interface import Interface, InterfaceOptions from graphene.types.objecttype import ObjectType, ObjectTypeOptions from graphene.types.utils import yank_fields_from_attrs from graphene.utils.orderedtype import OrderedType @@ -211,14 +213,7 @@ def construct_fields( return fields -class SQLAlchemyObjectTypeOptions(ObjectTypeOptions): - model = None # type: sqlalchemy.Model - registry = None # type: sqlalchemy.Registry - connection = None # type: sqlalchemy.Type[sqlalchemy.Connection] - id = None # type: str - - -class SQLAlchemyObjectType(ObjectType): +class SQLAlchemyBase(BaseType): @classmethod def __init_subclass_with_meta__( cls, @@ -237,6 +232,11 @@ def __init_subclass_with_meta__( _meta=None, **options ): + # We always want to bypass this hook unless we're defining a concrete + # `SQLAlchemyObjectType` or `SQLAlchemyInterface`. + if not _meta: + return + # Make sure model is a valid SQLAlchemy model if not is_mapped_class(model): raise ValueError( @@ -290,9 +290,6 @@ def __init_subclass_with_meta__( "The connection must be a Connection. Received {}" ).format(connection.__name__) - if not _meta: - _meta = SQLAlchemyObjectTypeOptions(cls) - _meta.model = model _meta.registry = registry @@ -306,7 +303,7 @@ def __init_subclass_with_meta__( cls.connection = connection # Public way to get the connection - super(SQLAlchemyObjectType, cls).__init_subclass_with_meta__( + super(SQLAlchemyBase, cls).__init_subclass_with_meta__( _meta=_meta, interfaces=interfaces, **options ) @@ -345,3 +342,39 @@ def enum_for_field(cls, field_name): sort_enum = classmethod(sort_enum_for_object_type) sort_argument = classmethod(sort_argument_for_object_type) + + +class SQLAlchemyObjectTypeOptions(ObjectTypeOptions): + model = None # type: sqlalchemy.Model + registry = None # type: sqlalchemy.Registry + connection = None # type: sqlalchemy.Type[sqlalchemy.Connection] + id = None # type: str + + +class SQLAlchemyObjectType(SQLAlchemyBase, ObjectType): + @classmethod + def __init_subclass_with_meta__(cls, _meta=None, **options): + if not _meta: + _meta = SQLAlchemyObjectTypeOptions(cls) + + super(SQLAlchemyObjectType, cls).__init_subclass_with_meta__( + _meta=_meta, **options + ) + + +class SQLAlchemyInterfaceOptions(InterfaceOptions): + model = None # type: sqlalchemy.Model + registry = None # type: sqlalchemy.Registry + connection = None # type: sqlalchemy.Type[sqlalchemy.Connection] + id = None # type: str + + +class SQLAlchemyInterface(SQLAlchemyBase, Interface): + @classmethod + def __init_subclass_with_meta__(cls, _meta=None, **options): + if not _meta: + _meta = SQLAlchemyInterfaceOptions(cls) + + super(SQLAlchemyInterface, cls).__init_subclass_with_meta__( + _meta=_meta, **options + ) From d95dcc5e50595d1792b77a498233f6595d204a24 Mon Sep 17 00:00:00 2001 From: Frederick Polgardy Date: Sat, 26 Nov 2022 17:08:25 +0000 Subject: [PATCH 2/9] add a doc for using interfaces --- docs/inheritance.rst | 74 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 docs/inheritance.rst diff --git a/docs/inheritance.rst b/docs/inheritance.rst new file mode 100644 index 00000000..05c3b9a1 --- /dev/null +++ b/docs/inheritance.rst @@ -0,0 +1,74 @@ +Inheritance Examples +==================== + +Create interfaces from inheritance relationships +------------------------------------------------ + +SQLAlchemy has excellent support for class inheritance hierarchies. +These hierarchies can be represented in your GraphQL schema by means +of interfaces_. Much like ObjectTypes, Interfaces in +Graphene-SQLAlchemy are able to infer their fields and relationships +from the attributes of their underlying SQLAlchemy model: + +.. _interfaces: https://docs.graphene-python.org/en/latest/types/interfaces/ + +.. code:: python + + from sqlalchemy import Column, DateTime, Integer, String + from sqlalchemy.ext.declarative import declarative_base + + import graphene + from graphene import relay + from graphene_sqlalchemy import SQLAlchemyInterface, SQLAlchemyObjectType + + Base = declarative_base() + + class PersonModel(Base): + id = Column(Integer, primary_key=True) + type = Column(String, nullable=False) + name = Column(String, nullable=False) + birth_date = Column(DateTime, nullable=False) + + __tablename__ = "person" + __mapper_args__ = { + "polymorphic_on": type, + "polymorphic_identity": "person", + } + + class EmployeeModel(PersonModel): + hire_date = Column(DateTime, nullable=False) + + __mapper_args__ = { + "polymorphic_identity": "employee", + } + + class Person(SQLAlchemyInterface): + class Meta: + model = PersonModel + + class Employee(SQLAlchemyObjectType): + class Meta: + model = EmployeeModel + interfaces = (relay.Node, Person) + + +When querying on the base type, you can refer directly to common fields, +and fields on concrete implementations using the `... on` syntax: + +.. code:: + + people { + name + birthDate + ... on Employee { + hireDate + } + } + +Note that if your SQLAlchemy model only specifies a relationship to the +base type, you will need to explicitly pass your concrete implementation +class to the Schema constructor via the `types=` argument: + +.. code:: python + + schema = graphene.Schema(..., types=[Person, Employee]) From 694c1229813ae88d15c786e0102224d0b7e8a4aa Mon Sep 17 00:00:00 2001 From: Frederick Polgardy Date: Sun, 27 Nov 2022 06:23:35 +0000 Subject: [PATCH 3/9] tweak docs --- docs/inheritance.rst | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/docs/inheritance.rst b/docs/inheritance.rst index 05c3b9a1..f2ea5754 100644 --- a/docs/inheritance.rst +++ b/docs/inheritance.rst @@ -14,7 +14,7 @@ from the attributes of their underlying SQLAlchemy model: .. code:: python - from sqlalchemy import Column, DateTime, Integer, String + from sqlalchemy import Column, Date, Integer, String from sqlalchemy.ext.declarative import declarative_base import graphene @@ -23,11 +23,11 @@ from the attributes of their underlying SQLAlchemy model: Base = declarative_base() - class PersonModel(Base): - id = Column(Integer, primary_key=True) - type = Column(String, nullable=False) - name = Column(String, nullable=False) - birth_date = Column(DateTime, nullable=False) + class Person(Base): + id = Column(Integer(), primary_key=True) + type = Column(String()) + name = Column(String()) + birth_date = Column(Date()) __tablename__ = "person" __mapper_args__ = { @@ -35,21 +35,21 @@ from the attributes of their underlying SQLAlchemy model: "polymorphic_identity": "person", } - class EmployeeModel(PersonModel): - hire_date = Column(DateTime, nullable=False) + class Employee(Person): + hire_date = Column(Date()) __mapper_args__ = { "polymorphic_identity": "employee", } - class Person(SQLAlchemyInterface): + class PersonType(SQLAlchemyInterface): class Meta: - model = PersonModel + model = Person - class Employee(SQLAlchemyObjectType): + class EmployeeType(SQLAlchemyObjectType): class Meta: - model = EmployeeModel - interfaces = (relay.Node, Person) + model = Employee + interfaces = (relay.Node, PersonType) When querying on the base type, you can refer directly to common fields, @@ -60,7 +60,7 @@ and fields on concrete implementations using the `... on` syntax: people { name birthDate - ... on Employee { + ... on EmployeeType { hireDate } } @@ -71,4 +71,4 @@ class to the Schema constructor via the `types=` argument: .. code:: python - schema = graphene.Schema(..., types=[Person, Employee]) + schema = graphene.Schema(..., types=[PersonType, EmployeeType]) From e5f0f8af6e6854a02af41a1c04346a1cf205d65f Mon Sep 17 00:00:00 2001 From: Frederick Polgardy Date: Sun, 27 Nov 2022 06:24:20 +0000 Subject: [PATCH 4/9] unit tests for interfaces --- graphene_sqlalchemy/tests/models.py | 26 ++++++++++ graphene_sqlalchemy/tests/test_query.py | 67 ++++++++++++++++++++++++- graphene_sqlalchemy/tests/test_types.py | 33 +++++++++++- 3 files changed, 122 insertions(+), 4 deletions(-) diff --git a/graphene_sqlalchemy/tests/models.py b/graphene_sqlalchemy/tests/models.py index b433982d..410c6b0a 100644 --- a/graphene_sqlalchemy/tests/models.py +++ b/graphene_sqlalchemy/tests/models.py @@ -288,3 +288,29 @@ class KeyedModel(Base): __tablename__ = "test330" id = Column(Integer(), primary_key=True) reporter_number = Column("% reporter_number", Numeric, key="reporter_number") + + +############################################ +# For interfaces +############################################ + + +class Person(Base): + id = Column(Integer(), primary_key=True) + type = Column(String()) + name = Column(String()) + birth_date = Column(Date()) + + __tablename__ = "person" + __mapper_args__ = { + "polymorphic_on": type, + "polymorphic_identity": "person", + } + + +class Employee(Person): + hire_date = Column(Date()) + + __mapper_args__ = { + "polymorphic_identity": "employee", + } diff --git a/graphene_sqlalchemy/tests/test_query.py b/graphene_sqlalchemy/tests/test_query.py index c7a173df..456254fc 100644 --- a/graphene_sqlalchemy/tests/test_query.py +++ b/graphene_sqlalchemy/tests/test_query.py @@ -1,10 +1,21 @@ +from datetime import date + import graphene from graphene.relay import Node from ..converter import convert_sqlalchemy_composite from ..fields import SQLAlchemyConnectionField -from ..types import ORMField, SQLAlchemyObjectType -from .models import Article, CompositeFullName, Editor, HairKind, Pet, Reporter +from ..types import ORMField, SQLAlchemyInterface, SQLAlchemyObjectType +from .models import ( + Article, + CompositeFullName, + Editor, + Employee, + HairKind, + Person, + Pet, + Reporter, +) from .utils import to_std_dicts @@ -334,3 +345,55 @@ class Mutation(graphene.ObjectType): assert not result.errors result = to_std_dicts(result.data) assert result == expected + + +def add_person_data(session): + bob = Employee(name="Bob", birth_date=date(1990, 1, 1), hire_date=date(2015, 1, 1)) + session.add(bob) + joe = Employee(name="Joe", birth_date=date(1980, 1, 1), hire_date=date(2010, 1, 1)) + session.add(joe) + jen = Employee(name="Jen", birth_date=date(1995, 1, 1), hire_date=date(2020, 1, 1)) + session.add(jen) + session.commit() + + +def test_interface_query_on_base_type(session): + add_person_data(session) + + class PersonType(SQLAlchemyInterface): + class Meta: + model = Person + + class EmployeeType(SQLAlchemyObjectType): + class Meta: + model = Employee + interfaces = (Node, PersonType) + + class Query(graphene.ObjectType): + people = graphene.Field(graphene.List(PersonType)) + + def resolve_people(self, _info): + return session.query(Person).all() + + schema = graphene.Schema(query=Query, types=[PersonType, EmployeeType]) + result = schema.execute( + """ + query { + people { + __typename + name + birthDate + ... on EmployeeType { + hireDate + } + } + } + """ + ) + + assert not result.errors + assert len(result.data["people"]) == 3 + assert result.data["people"][0]["__typename"] == "EmployeeType" + assert result.data["people"][0]["name"] == "Bob" + assert result.data["people"][0]["birthDate"] == "1990-01-01" + assert result.data["people"][0]["hireDate"] == "2015-01-01" diff --git a/graphene_sqlalchemy/tests/test_types.py b/graphene_sqlalchemy/tests/test_types.py index 4afb120d..e21546ba 100644 --- a/graphene_sqlalchemy/tests/test_types.py +++ b/graphene_sqlalchemy/tests/test_types.py @@ -29,8 +29,13 @@ registerConnectionFieldFactory, unregisterConnectionFieldFactory, ) -from ..types import ORMField, SQLAlchemyObjectType, SQLAlchemyObjectTypeOptions -from .models import Article, CompositeFullName, Pet, Reporter +from ..types import ( + ORMField, + SQLAlchemyInterface, + SQLAlchemyObjectType, + SQLAlchemyObjectTypeOptions, +) +from .models import Article, CompositeFullName, Employee, Person, Pet, Reporter def test_should_raise_if_no_model(): @@ -509,6 +514,30 @@ class Meta: assert ReporterWithCustomOptions._meta.custom_option == "custom_option" +def test_interface_inherited_fields(): + class PersonType(SQLAlchemyInterface): + class Meta: + model = Person + + class EmployeeType(SQLAlchemyObjectType): + class Meta: + model = Employee + interfaces = (Node, PersonType) + + assert PersonType in EmployeeType._meta.interfaces + + name_field = EmployeeType._meta.fields["name"] + assert name_field.type == String + + assert list(EmployeeType._meta.fields.keys()) == [ + "id", + "type", + "name", + "birth_date", + "hire_date", + ] + + # Tests for connection_field_factory From 57b86fe4dfbfc4bb70463708517cabfcbdd9b3f1 Mon Sep 17 00:00:00 2001 From: Frederick Polgardy Date: Sun, 27 Nov 2022 15:50:41 +0000 Subject: [PATCH 5/9] docstrings for interface and object type --- graphene_sqlalchemy/types.py | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/graphene_sqlalchemy/types.py b/graphene_sqlalchemy/types.py index 55f59773..4bfab351 100644 --- a/graphene_sqlalchemy/types.py +++ b/graphene_sqlalchemy/types.py @@ -214,6 +214,11 @@ def construct_fields( class SQLAlchemyBase(BaseType): + """ + This class contains initialization code that is common to both ObjectTypes + and Interfaces. You typically don't need to use it directly. + """ + @classmethod def __init_subclass_with_meta__( cls, @@ -352,6 +357,22 @@ class SQLAlchemyObjectTypeOptions(ObjectTypeOptions): class SQLAlchemyObjectType(SQLAlchemyBase, ObjectType): + """ + This type represents the GraphQL ObjectType. It reflects on the + given SQLAlchemy model, and automatically generates an ObjectType + using the column and relationship information defined there. + + Usage: + + class MyModel(Base): + id = Column(Integer(), primary_key=True) + name = Column(String()) + + class MyType(SQLAlchemyObjectType): + class Meta: + model = MyModel + """ + @classmethod def __init_subclass_with_meta__(cls, _meta=None, **options): if not _meta: @@ -370,6 +391,41 @@ class SQLAlchemyInterfaceOptions(InterfaceOptions): class SQLAlchemyInterface(SQLAlchemyBase, Interface): + """ + This type represents the GraphQL Interface. It reflects on the + given SQLAlchemy model, and automatically generates an Interface + using the column and relationship information defined there. This + is used to construct interface relationships based on polymorphic + inheritance hierarchies in SQLAlchemy. + + Usage (using joined table inheritance): + + class MyBaseModel(Base): + id = Column(Integer(), primary_key=True) + name = Column(String()) + + __mapper_args__ = { + "polymorphic_on": type, + "polymorphic_identity": "base", + } + + class MyChildModel(Base): + date = Column(Date()) + + __mapper_args__ = { + "polymorphic_identity": "child", + } + + class MyBaseType(SQLAlchemyInterface): + class Meta: + model = MyBaseModel + + class MyChildType(SQLAlchemyObjectType): + class Meta: + model = MyChildModel + interfaces = (MyBaseType,) + """ + @classmethod def __init_subclass_with_meta__(cls, _meta=None, **options): if not _meta: From 44a17f999408f27ebf712f2cd752b0739e7333a2 Mon Sep 17 00:00:00 2001 From: Frederick Polgardy Date: Mon, 28 Nov 2022 05:32:09 +0000 Subject: [PATCH 6/9] don't autogenerate a field for polymorphic discriminators --- graphene_sqlalchemy/tests/test_types.py | 3 ++- graphene_sqlalchemy/types.py | 26 +++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/graphene_sqlalchemy/tests/test_types.py b/graphene_sqlalchemy/tests/test_types.py index e21546ba..fd5fe0ba 100644 --- a/graphene_sqlalchemy/tests/test_types.py +++ b/graphene_sqlalchemy/tests/test_types.py @@ -529,9 +529,10 @@ class Meta: name_field = EmployeeType._meta.fields["name"] assert name_field.type == String + # `type` should *not* be in this list because it's the polymorphic_on + # discriminator for Person assert list(EmployeeType._meta.fields.keys()) == [ "id", - "type", "name", "birth_date", "hire_date", diff --git a/graphene_sqlalchemy/types.py b/graphene_sqlalchemy/types.py index 4bfab351..f422be72 100644 --- a/graphene_sqlalchemy/types.py +++ b/graphene_sqlalchemy/types.py @@ -96,6 +96,18 @@ class Meta: self.kwargs.update(common_kwargs) +def get_polymorphic_on(model): + """ + Check whether this model is a polymorphic type, and if so return the name + of the discriminator field (`polymorphic_on`), so that it won't be automatically + generated as an ORMField. + """ + if hasattr(model, "__mapper__") and model.__mapper__.polymorphic_on is not None: + polymorphic_on = model.__mapper__.polymorphic_on + if isinstance(polymorphic_on, sqlalchemy.Column): + return polymorphic_on.name + + def construct_fields( obj_type, model, @@ -135,10 +147,13 @@ def construct_fields( ) # Filter out excluded fields + polymorphic_on = get_polymorphic_on(model) auto_orm_field_names = [] for attr_name, attr in all_model_attrs.items(): - if (only_fields and attr_name not in only_fields) or ( - attr_name in exclude_fields + if ( + (only_fields and attr_name not in only_fields) + or (attr_name in exclude_fields) + or attr_name == polymorphic_on ): continue auto_orm_field_names.append(attr_name) @@ -398,10 +413,17 @@ class SQLAlchemyInterface(SQLAlchemyBase, Interface): is used to construct interface relationships based on polymorphic inheritance hierarchies in SQLAlchemy. + Please note that by default, the "polymorphic_on" column is *not* + generated as a field on types that use polymorphic inheritance, as + this is considered an implentation detail. The idiomatic way to + retrieve the concrete GraphQL type of an object is to query for the + `__typename` field. + Usage (using joined table inheritance): class MyBaseModel(Base): id = Column(Integer(), primary_key=True) + type = Column(String()) name = Column(String()) __mapper_args__ = { From 59d1d35c5fdd9ce3e7c90d6b8f75d228f910e4f3 Mon Sep 17 00:00:00 2001 From: Frederick Polgardy Date: Mon, 28 Nov 2022 05:46:04 +0000 Subject: [PATCH 7/9] don't allow interfaces to have a polymorphic_identity --- graphene_sqlalchemy/tests/models.py | 1 - graphene_sqlalchemy/types.py | 10 +++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/graphene_sqlalchemy/tests/models.py b/graphene_sqlalchemy/tests/models.py index 410c6b0a..15524ebe 100644 --- a/graphene_sqlalchemy/tests/models.py +++ b/graphene_sqlalchemy/tests/models.py @@ -304,7 +304,6 @@ class Person(Base): __tablename__ = "person" __mapper_args__ = { "polymorphic_on": type, - "polymorphic_identity": "person", } diff --git a/graphene_sqlalchemy/types.py b/graphene_sqlalchemy/types.py index f422be72..85fafaac 100644 --- a/graphene_sqlalchemy/types.py +++ b/graphene_sqlalchemy/types.py @@ -428,7 +428,6 @@ class MyBaseModel(Base): __mapper_args__ = { "polymorphic_on": type, - "polymorphic_identity": "base", } class MyChildModel(Base): @@ -456,3 +455,12 @@ def __init_subclass_with_meta__(cls, _meta=None, **options): super(SQLAlchemyInterface, cls).__init_subclass_with_meta__( _meta=_meta, **options ) + + # make sure that the model doesn't have a polymorphic_identity defined + if hasattr(_meta.model, "__mapper__"): + polymorphic_identity = _meta.model.__mapper__.polymorphic_identity + assert ( + polymorphic_identity is None + ), 'An interface cannot map to a concrete type (polymorphic_identity is "{}")'.format( + polymorphic_identity + ) From fa039311463ab97df90fa3d3b519c830d0d6d1b9 Mon Sep 17 00:00:00 2001 From: Erik Wrede Date: Mon, 28 Nov 2022 09:40:37 +0100 Subject: [PATCH 8/9] docs: refactor inheritance.rst - Added a note that polymorphic_on fields are excluded by default. - Added a note that a polymorphic_identity is unsupported for interfaces --- docs/inheritance.rst | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/docs/inheritance.rst b/docs/inheritance.rst index f2ea5754..ee16f062 100644 --- a/docs/inheritance.rst +++ b/docs/inheritance.rst @@ -32,7 +32,6 @@ from the attributes of their underlying SQLAlchemy model: __tablename__ = "person" __mapper_args__ = { "polymorphic_on": type, - "polymorphic_identity": "person", } class Employee(Person): @@ -41,6 +40,13 @@ from the attributes of their underlying SQLAlchemy model: __mapper_args__ = { "polymorphic_identity": "employee", } + + class Customer(Person): + first_purchase_date = Column(Date()) + + __mapper_args__ = { + "polymorphic_identity": "customer", + } class PersonType(SQLAlchemyInterface): class Meta: @@ -50,11 +56,23 @@ from the attributes of their underlying SQLAlchemy model: class Meta: model = Employee interfaces = (relay.Node, PersonType) + + class CustomerType(SQLAlchemyObjectType): + class Meta: + model = Customer + interfaces = (relay.Node, PersonType) +Keep in mind that `PersonType` is a `SQLAlchemyInterface`. Interfaces must +be linked to an abstract Model that does not specify a `polymorphic_identity`, +because we cannot return instances of interfaces from a GraphQL query. +If Person specified a `polymorphic_identity`, instances of Person could +be inserted into and returned by the database, potentially causing +Persons to be returned to the resolvers. When querying on the base type, you can refer directly to common fields, and fields on concrete implementations using the `... on` syntax: + .. code:: people { @@ -63,12 +81,27 @@ and fields on concrete implementations using the `... on` syntax: ... on EmployeeType { hireDate } + ... on CustomerType { + firstPurchaseDate + } } - -Note that if your SQLAlchemy model only specifies a relationship to the + + +Please note that by default, the "polymorphic_on" column is *not* +generated as a field on types that use polymorphic inheritance, as +this is considered an implentation detail. The idiomatic way to +retrieve the concrete GraphQL type of an object is to query for the +`__typename` field. +To override this behavior, an `ORMField` needs to be created +for the custom type field on the corresponding `SQLAlchemyInterface`. This is *not recommended* +as it promotes abiguous schema design + +If your SQLAlchemy model only specifies a relationship to the base type, you will need to explicitly pass your concrete implementation class to the Schema constructor via the `types=` argument: .. code:: python - schema = graphene.Schema(..., types=[PersonType, EmployeeType]) + schema = graphene.Schema(..., types=[PersonType, EmployeeType, CustomerType]) + +See also: `Graphene Interfaces `_ From 1ed9dbeb74674884bd5722e2591782175ed8f6ea Mon Sep 17 00:00:00 2001 From: Erik Wrede Date: Mon, 28 Nov 2022 10:18:58 +0100 Subject: [PATCH 9/9] tests: added test cases for ORMField Override & Assertion error on non-abstract Interface models chore: added type name to assertion error for easier debugging during development --- graphene_sqlalchemy/tests/models.py | 11 ++++ graphene_sqlalchemy/tests/test_types.py | 72 ++++++++++++++++++++++--- graphene_sqlalchemy/types.py | 4 +- 3 files changed, 79 insertions(+), 8 deletions(-) diff --git a/graphene_sqlalchemy/tests/models.py b/graphene_sqlalchemy/tests/models.py index 15524ebe..4fe91462 100644 --- a/graphene_sqlalchemy/tests/models.py +++ b/graphene_sqlalchemy/tests/models.py @@ -306,6 +306,17 @@ class Person(Base): "polymorphic_on": type, } +class NonAbstractPerson(Base): + id = Column(Integer(), primary_key=True) + type = Column(String()) + name = Column(String()) + birth_date = Column(Date()) + + __tablename__ = "non_abstract_person" + __mapper_args__ = { + "polymorphic_on": type, + "polymorphic_identity": "person", + } class Employee(Person): hire_date = Column(Date()) diff --git a/graphene_sqlalchemy/tests/test_types.py b/graphene_sqlalchemy/tests/test_types.py index fd5fe0ba..813fb134 100644 --- a/graphene_sqlalchemy/tests/test_types.py +++ b/graphene_sqlalchemy/tests/test_types.py @@ -1,9 +1,9 @@ +import re from unittest import mock import pytest import sqlalchemy.exc import sqlalchemy.orm.exc - from graphene import ( Boolean, Dynamic, @@ -20,6 +20,7 @@ ) from graphene.relay import Connection +from .models import Article, CompositeFullName, Employee, Person, Pet, Reporter, NonAbstractPerson from .. import utils from ..converter import convert_sqlalchemy_composite from ..fields import ( @@ -35,13 +36,11 @@ SQLAlchemyObjectType, SQLAlchemyObjectTypeOptions, ) -from .models import Article, CompositeFullName, Employee, Person, Pet, Reporter def test_should_raise_if_no_model(): re_err = r"valid SQLAlchemy Model" with pytest.raises(Exception, match=re_err): - class Character1(SQLAlchemyObjectType): pass @@ -49,7 +48,6 @@ class Character1(SQLAlchemyObjectType): def test_should_raise_if_model_is_invalid(): re_err = r"valid SQLAlchemy Model" with pytest.raises(Exception, match=re_err): - class Character(SQLAlchemyObjectType): class Meta: model = 1 @@ -322,7 +320,6 @@ def test_invalid_model_attr(): "Cannot map ORMField to a model attribute.\n" "Field: 'ReporterType.first_name'" ) with pytest.raises(ValueError, match=err_msg): - class ReporterType(SQLAlchemyObjectType): class Meta: model = Reporter @@ -376,7 +373,6 @@ class Meta: def test_only_and_exclude_fields(): re_err = r"'only_fields' and 'exclude_fields' cannot be both set" with pytest.raises(Exception, match=re_err): - class ReporterType(SQLAlchemyObjectType): class Meta: model = Reporter @@ -514,6 +510,14 @@ class Meta: assert ReporterWithCustomOptions._meta.custom_option == "custom_option" +def test_interface_with_polymorphic_identity(): + with pytest.raises(AssertionError, + match=re.escape('PersonType: An interface cannot map to a concrete type (polymorphic_identity is "person")')): + class PersonType(SQLAlchemyInterface): + class Meta: + model = NonAbstractPerson + + def test_interface_inherited_fields(): class PersonType(SQLAlchemyInterface): class Meta: @@ -539,6 +543,62 @@ class Meta: ] +def test_interface_type_field_orm_override(): + class PersonType(SQLAlchemyInterface): + class Meta: + model = Person + + type = ORMField() + + class EmployeeType(SQLAlchemyObjectType): + class Meta: + model = Employee + interfaces = (Node, PersonType) + + assert PersonType in EmployeeType._meta.interfaces + + name_field = EmployeeType._meta.fields["name"] + assert name_field.type == String + + # type should be in this list because we used ORMField + # to force its presence on the model + assert sorted(list(EmployeeType._meta.fields.keys())) == sorted([ + "id", + "name", + "type", + "birth_date", + "hire_date", + ]) + + +def test_interface_custom_resolver(): + class PersonType(SQLAlchemyInterface): + class Meta: + model = Person + + custom_field = Field(String) + + class EmployeeType(SQLAlchemyObjectType): + class Meta: + model = Employee + interfaces = (Node, PersonType) + + assert PersonType in EmployeeType._meta.interfaces + + name_field = EmployeeType._meta.fields["name"] + assert name_field.type == String + + # type should be in this list because we used ORMField + # to force its presence on the model + assert sorted(list(EmployeeType._meta.fields.keys())) == sorted([ + "id", + "name", + "custom_field", + "birth_date", + "hire_date", + ]) + + # Tests for connection_field_factory diff --git a/graphene_sqlalchemy/types.py b/graphene_sqlalchemy/types.py index 85fafaac..e0ada38e 100644 --- a/graphene_sqlalchemy/types.py +++ b/graphene_sqlalchemy/types.py @@ -461,6 +461,6 @@ def __init_subclass_with_meta__(cls, _meta=None, **options): polymorphic_identity = _meta.model.__mapper__.polymorphic_identity assert ( polymorphic_identity is None - ), 'An interface cannot map to a concrete type (polymorphic_identity is "{}")'.format( - polymorphic_identity + ), '{}: An interface cannot map to a concrete type (polymorphic_identity is "{}")'.format( + cls.__name__, polymorphic_identity )