Support GQL interfaces for polymorphic SQLA models

[polgfred]

Hi!

Would love any feedback on this.

My background assumption here is that, for most folks using graphene_sqlalchemy bindings, GraphQL interfaces are going to take the form of polymorphic base classes. So it would be great if you could extend SQLAlchemyInterface the same way you extend SQLAlchemyObjectType for concrete types -- and wire up the attributes using the same reflection mechanism.

So, in order to accomplish this, I:

• moved the logic in SQLAlchemyObjectType up into a SQLAlchemyBase type
• made SQLAlchemyObjectType and (a new) SQLAlchemyInterface extend SQLAlchemyBase
• updated Registry to accept objects of type SQLAlchemyBase

I've tested this in a work project that depends heavily on a few polymorphic JTI base types, and this mechanism works perfectly. However, I'm sure I haven't thought through potential limitations and incorrect assumptions, which is why I'm submitting this for review and feedback.

Thanks!

[erikwrede]

Thanks for the PR! Will check it out this week 🙂

[codecov[bot]]

Codecov Report

Base: 96.97% // Head: 97.09% // Increases project coverage by +0.11% :tada:

Coverage data is based on head (1ed9dbe) compared to base (8bfa1e9). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #365 +/- ## ========================================== + Coverage 96.97% 97.09% +0.11% ========================================== Files 7 7 Lines 694 722 +28 ========================================== + Hits 673 701 +28 Misses 21 21 ``` | [Impacted Files](https://codecov.io/gh/graphql-python/graphene-sqlalchemy/pull/365?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python) | Coverage Δ | | |---|---|---| | [graphene\_sqlalchemy/registry.py](https://codecov.io/gh/graphql-python/graphene-sqlalchemy/pull/365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python#diff-Z3JhcGhlbmVfc3FsYWxjaGVteS9yZWdpc3RyeS5weQ==) | `100.00% <100.00%> (ø)` | | | [graphene\_sqlalchemy/types.py](https://codecov.io/gh/graphql-python/graphene-sqlalchemy/pull/365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python#diff-Z3JhcGhlbmVfc3FsYWxjaGVteS90eXBlcy5weQ==) | `95.39% <100.00%> (+1.03%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[polgfred]

Can you please add a usage example, unit tests and some docstrings to give some guidance on when and how to use the new Base and Interface types?

@erikwrede Thanks! And sorry for the delay, been busy over here. I've added a doc, unit tests, and a docstring for ObjectType and Interface. Let me know if this is what you're looking for, and please feel free to tweak if desired.

[erikwrede]

@polgfred thanks for the update! Appreciate the weekend-effort 🙂

There's two things we should discuss before merging this:

1. Should we explicitly exclude the field linked in polymorphic_on (in this case type) from the GQL schema? The type field is verbose as GraphQL already provides the __typename field including the GQL type name. API-Design wise the type field does not bring any value as it is only relevant for server internals. We could enable overriding the behavior using the ORMField: if an ORMField was created on the base/explicit type for the field containing the polymorphic identity, the field would not be excluded. I think this leads to better schema design while still giving users the option to customize.

2. Removing polymorphic_identity: "person" from the examples PersonType is an Interface in this case, so pure instances of PersonType are not allowed. However, the underlying DB Model allows pure instances of our base class Person because it provides a polymorphic_identity. This can lead to the following error if a pure Person instance is returned from the resolvers:

 "message": "Abstract type 'PersonType' must resolve to an Object type at runtime for field 'Query.people'. Either the 'PersonType' type should provide a 'resolve_type' function or each possible type should provide an 'is_type_of' function."

If the polymorphic_identity is not set for Person, SQLAlchemy will not allow such inserts.

We need to clarify that the Interface type is only suitable for models that have an abstract (maybe there's better wording) base that is guaranteed to not appear in the records. Same goes for multi-table inhertance with a distinct table for the base (which is an uncommon case but it might still be used in existing designs).

Please let me know what you think!

[polgfred]

@erikwrede Yeah, I agree with both those observations. An interface isn't exactly a polymorphic type for the reason you mentioned, but in my experience it's much more common for a polymorphic base class to be abstract, so we should call that out in the docs and examples. And I agree that it makes sense to hide the polymorphic_on field in the schema. I'll look into that tonight :)

[polgfred]

@erikwrede Added checks for the stuff you called out.

[polgfred]

Awesome, looks great to me!