Skip to content

Serializing native Python enums does not work #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wichert opened this issue Jul 31, 2018 · 6 comments · Fixed by #154
Closed

Serializing native Python enums does not work #152

wichert opened this issue Jul 31, 2018 · 6 comments · Fixed by #154

Comments

@wichert
Copy link
Contributor

wichert commented Jul 31, 2018

Currently using SQL enums works well, unless you use a enum.Enum as base for the enum. For example using this model:

class Hairkind(enum.Enum):
    LONG = 'long'
    SHORT = 'short'

class Pet(Base):
    __tablename__ = 'pets'
    id = Column(Integer(), primary_key=True)
    hair_kind = Column(Enum(Hairkind, name='hair_kind'), nullable=False)

will fail badly if you try to query the hairKind field:

  File "lib/python3.7/site-packages/graphql/execution/executor.py", line 622, in complete_leaf_value
    path=path,
graphql.error.base.GraphQLError: Expected a value of type "hair_kind" but received: Hairkind.LONG
@wichert wichert changed the title Native Python enums do not work Serializing native Python enums does not work Jul 31, 2018
@wichert
Copy link
Contributor Author

wichert commented Jul 31, 2018

What is happening here is that graphqln_sqlalchemy.converter.convert_sqlalchemy_type.register creates a GraphQL Enum using the enum instances using items = type.enum_class.__members__.items(). But in graphql.type.definition.GraphQLEnumType.serialize this happens:

    def serialize(self, value):
        # type: (Union[str, PyEnum]) -> Optional[str]
        if isinstance(value, PyEnum):
            # We handle PyEnum values
            value = value.value
        if isinstance(value, collections.Hashable):
            enum_value = self._value_lookup.get(value)
            if enum_value:
                return enum_value.name

        return None

at that point the enum.Enum instance is converted to its value, which is then looked up. But the lookup table has the enum instances, causing the lookup to fail and an error to be returned.

@wichert
Copy link
Contributor Author

wichert commented Jul 31, 2018

This diff adds a failing test demonstrating the problem:

diff --git a/graphene_sqlalchemy/tests/models.py b/graphene_sqlalchemy/tests/models.py
index be7d5cd..3ba23a8 100644
--- a/graphene_sqlalchemy/tests/models.py
+++ b/graphene_sqlalchemy/tests/models.py
@@ -6,6 +6,12 @@ from sqlalchemy import Column, Date, Enum, ForeignKey, Integer, String, Table
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.orm import mapper, relationship
 
+
+class Hairkind(enum.Enum):
+    LONG = 'long'
+    SHORT = 'short'
+
+
 Base = declarative_base()
 
 association_table = Table(
@@ -27,6 +33,7 @@ class Pet(Base):
     id = Column(Integer(), primary_key=True)
     name = Column(String(30))
     pet_kind = Column(Enum("cat", "dog", name="pet_kind"), nullable=False)
+    hair_kind = Column(Enum(Hairkind, name="hair_kind"), nullable=False)
     reporter_id = Column(Integer(), ForeignKey("reporters.id"))
 
 
diff --git a/graphene_sqlalchemy/tests/test_query.py b/graphene_sqlalchemy/tests/test_query.py
index f8bc840..2e2d602 100644
--- a/graphene_sqlalchemy/tests/test_query.py
+++ b/graphene_sqlalchemy/tests/test_query.py
@@ -9,7 +9,7 @@ from ..registry import reset_global_registry
 from ..fields import SQLAlchemyConnectionField
 from ..types import SQLAlchemyObjectType
 from ..utils import sort_argument_for_model, sort_enum_for_model
-from .models import Article, Base, Editor, Pet, Reporter
+from .models import Article, Base, Editor, Pet, Reporter, Hairkind
 
 db = create_engine("sqlite:///test_sqlalchemy.sqlite3")
 
@@ -34,7 +34,7 @@ def session():
 
 
 def setup_fixtures(session):
-    pet = Pet(name="Lassie", pet_kind="dog")
+    pet = Pet(name="Lassie", pet_kind="dog", hair_kind=Hairkind.LONG)
     session.add(pet)
     reporter = Reporter(first_name="ABA", last_name="X")
     session.add(reporter)
@@ -105,6 +105,7 @@ def test_should_query_enums(session):
           pet {
             name,
             petKind
+            hairKind
           }
         }
     """
@@ -326,9 +327,9 @@ def test_should_mutate_well(session):
 
 def sort_setup(session):
     pets = [
-        Pet(id=2, name="Lassie", pet_kind="dog"),
-        Pet(id=22, name="Alf", pet_kind="cat"),
-        Pet(id=3, name="Barf", pet_kind="dog"),
+        Pet(id=2, name="Lassie", pet_kind="dog", hair_kind=Hairkind.LONG),
+        Pet(id=22, name="Alf", pet_kind="cat", hair_kind=Hairkind.LONG),
+        Pet(id=3, name="Barf", pet_kind="dog", hair_kind=Hairkind.LONG),
     ]
     session.add_all(pets)
     session.commit()

@wichert
Copy link
Contributor Author

wichert commented Jul 31, 2018

The situation turns out to be worse now than with the last release. With the previous release you could add a resolver, like so:

    def resolve_type(self, info):
        return self.type.name

that workaround no longer works now, since graphql now expects to receive an enum and aborts when it gets a string instead.

@congocongo
Copy link

look at:
graphql-python/graphql-core#199

@robjampar
Copy link

robjampar commented Aug 30, 2018

Yeah this has broken enums in one of my libraries and the only way i can fix it is to monkey patch the method!

id suggest a non breaking change, for example:

def serialize(self, value):
    if isinstance(value, collections.Hashable):
        if value in self._value_lookup:
            enum_value = self._value_lookup.get(value)
    
        elif isinstance(value, PyEnum):
            # We handle PyEnum values
            enum_value = self._value_lookup.get(value.value)
    
        if enum_value:
            return enum_value.name
    
    return None```

kigen pushed a commit to kigen/graphene-sqlalchemy that referenced this issue Dec 9, 2018
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related topics referencing this issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants