From c2c4a778c1fcde330239a145b6dc0e972c4abbf4 Mon Sep 17 00:00:00 2001 From: Christoph Zwerschke Date: Fri, 12 Apr 2019 01:22:07 +0200 Subject: [PATCH] Improve auto creation of Graphene Enums. The created Graphene Enums are now registered and reused, because their names must be unique in a GraphQL schema. Also the naming conventions for Enum type names (CamelCase) and options (UPPER_CASE) are applied when creating them. --- graphene_sqlalchemy/__init__.py | 2 +- graphene_sqlalchemy/converter.py | 18 +-- graphene_sqlalchemy/registry.py | 43 ++++++ graphene_sqlalchemy/tests/models.py | 9 +- graphene_sqlalchemy/tests/test_converter.py | 41 +++-- graphene_sqlalchemy/tests/test_query.py | 161 ++++++++++++++------ graphene_sqlalchemy/tests/test_schema.py | 1 + graphene_sqlalchemy/tests/test_types.py | 5 +- graphene_sqlalchemy/tests/test_utils.py | 15 +- graphene_sqlalchemy/utils.py | 17 +++ 10 files changed, 236 insertions(+), 76 deletions(-) diff --git a/graphene_sqlalchemy/__init__.py b/graphene_sqlalchemy/__init__.py index eee98090..b83cecf7 100644 --- a/graphene_sqlalchemy/__init__.py +++ b/graphene_sqlalchemy/__init__.py @@ -1,5 +1,5 @@ -from .types import SQLAlchemyObjectType from .fields import SQLAlchemyConnectionField +from .types import SQLAlchemyObjectType from .utils import get_query, get_session __version__ = "2.1.1" diff --git a/graphene_sqlalchemy/converter.py b/graphene_sqlalchemy/converter.py index 6d021b88..e2f676ce 100644 --- a/graphene_sqlalchemy/converter.py +++ b/graphene_sqlalchemy/converter.py @@ -147,17 +147,13 @@ def convert_column_to_float(type, column, registry=None): @convert_sqlalchemy_type.register(types.Enum) def convert_enum_to_enum(type, column, registry=None): - enum_class = getattr(type, 'enum_class', None) - if enum_class: # Check if an enum.Enum type is used - graphene_type = Enum.from_enum(enum_class) - else: # Nope, just a list of string options - items = zip(type.enums, type.enums) - graphene_type = Enum(type.name, items) - return Field( - graphene_type, - description=get_column_doc(column), - required=not (is_column_nullable(column)), - ) + if registry is None: + from .registry import get_global_registry + registry = get_global_registry() + graphene_type = registry.get_type_for_enum(type) + return Field(graphene_type, + description=get_column_doc(column), + required=not(is_column_nullable(column))) @convert_sqlalchemy_type.register(ChoiceType) diff --git a/graphene_sqlalchemy/registry.py b/graphene_sqlalchemy/registry.py index 460053f2..a75db032 100644 --- a/graphene_sqlalchemy/registry.py +++ b/graphene_sqlalchemy/registry.py @@ -1,8 +1,19 @@ + +from collections import OrderedDict + +from sqlalchemy.types import Enum as SQLAlchemyEnumType + +from graphene import Enum + +from .utils import to_enum_value_name, to_type_name + + class Registry(object): def __init__(self): self._registry = {} self._registry_models = {} self._registry_composites = {} + self._registry_enums = {} def register(self, cls): from .types import SQLAlchemyObjectType @@ -27,6 +38,38 @@ def register_composite_converter(self, composite, converter): def get_converter_for_composite(self, composite): return self._registry_composites.get(composite) + def get_type_for_enum(self, sql_type): + if not isinstance(sql_type, SQLAlchemyEnumType): + raise TypeError( + 'Only sqlalchemy.Enum objects can be registered as enum, ' + 'received "{}"'.format(sql_type)) + enum_class = sql_type.enum_class + if enum_class: + name = enum_class.__name__ + members = OrderedDict( + (to_enum_value_name(key), value.value) + for key, value in enum_class.__members__.items()) + else: + name = sql_type.name + name = to_type_name(name) if name else 'Enum{}'.format( + len(self._registry_enums) + 1) + members = OrderedDict( + (to_enum_value_name(key), key) for key in sql_type.enums) + graphene_type = self._registry_enums.get(name) + if graphene_type: + existing_members = { + key: value.value for key, value + in graphene_type._meta.enum.__members__.items()} + if members != existing_members: + raise TypeError( + 'Different enums with the same name "{}":' + ' tried to register {}, but {} existed already.'.format( + name, members, existing_members)) + else: + graphene_type = Enum(name, members) + self._registry_enums[name] = graphene_type + return graphene_type + registry = None diff --git a/graphene_sqlalchemy/tests/models.py b/graphene_sqlalchemy/tests/models.py index 3ba23a8a..12781cc5 100644 --- a/graphene_sqlalchemy/tests/models.py +++ b/graphene_sqlalchemy/tests/models.py @@ -6,8 +6,10 @@ from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.orm import mapper, relationship +PetKind = Enum("cat", "dog", name="pet_kind") -class Hairkind(enum.Enum): + +class HairKind(enum.Enum): LONG = 'long' SHORT = 'short' @@ -32,8 +34,8 @@ class Pet(Base): __tablename__ = "pets" 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) + pet_kind = Column(PetKind, nullable=False) + hair_kind = Column(Enum(HairKind, name="hair_kind"), nullable=False) reporter_id = Column(Integer(), ForeignKey("reporters.id")) @@ -43,6 +45,7 @@ class Reporter(Base): first_name = Column(String(30)) last_name = Column(String(30)) email = Column(String()) + favorite_pet_kind = Column(PetKind) pets = relationship("Pet", secondary=association_table, backref="reporters") articles = relationship("Article", backref="reporter") favorite_article = relationship("Article", uselist=False) diff --git a/graphene_sqlalchemy/tests/test_converter.py b/graphene_sqlalchemy/tests/test_converter.py index d205427b..8f31e5f0 100644 --- a/graphene_sqlalchemy/tests/test_converter.py +++ b/graphene_sqlalchemy/tests/test_converter.py @@ -85,19 +85,37 @@ def test_should_unicodetext_convert_string(): def test_should_enum_convert_enum(): - field = assert_column_conversion( - types.Enum(enum.Enum("one", "two")), graphene.Field - ) + field = assert_column_conversion(types.Enum("one", "two"), graphene.Field) field_type = field.type() + assert field_type.__class__.__name__.startswith("Enum") assert isinstance(field_type, graphene.Enum) - assert hasattr(field_type, "two") + assert hasattr(field_type, "ONE") + assert not hasattr(field_type, "one") + assert hasattr(field_type, "TWO") + field = assert_column_conversion( types.Enum("one", "two", name="two_numbers"), graphene.Field ) field_type = field.type() - assert field_type.__class__.__name__ == "two_numbers" + assert field_type.__class__.__name__ == "TwoNumbers" + assert isinstance(field_type, graphene.Enum) + assert hasattr(field_type, "ONE") + assert not hasattr(field_type, "one") + assert hasattr(field_type, "TWO") + + +def test_conflicting_enum_should_raise_error(): + some_type = types.Enum(enum.Enum("ConflictingEnum", "cat cow")) + field = assert_column_conversion(some_type, graphene.Field) + field_type = field.type() assert isinstance(field_type, graphene.Enum) - assert hasattr(field_type, "two") + assert hasattr(field_type, "COW") + same_type = types.Enum(enum.Enum("ConflictingEnum", "cat cow")) + field = assert_column_conversion(same_type, graphene.Field) + assert field_type == field.type() + conflicting_type = types.Enum(enum.Enum("ConflictingEnum", "cat horse")) + with raises(TypeError): + assert_column_conversion(conflicting_type, graphene.Field) def test_should_small_integer_convert_int(): @@ -272,19 +290,20 @@ def test_should_postgresql_enum_convert(): postgresql.ENUM("one", "two", name="two_numbers"), graphene.Field ) field_type = field.type() - assert field_type.__class__.__name__ == "two_numbers" + assert field_type.__class__.__name__ == "TwoNumbers" assert isinstance(field_type, graphene.Enum) - assert hasattr(field_type, "two") + assert hasattr(field_type, "TWO") def test_should_postgresql_py_enum_convert(): field = assert_column_conversion( - postgresql.ENUM(enum.Enum("TwoNumbers", "one two"), name="two_numbers"), graphene.Field + postgresql.ENUM(enum.Enum("TwoNumbersEnum", "one two"), name="two_numbers"), + graphene.Field, ) field_type = field.type() - assert field_type.__class__.__name__ == "TwoNumbers" + assert field_type.__class__.__name__ == "TwoNumbersEnum" assert isinstance(field_type, graphene.Enum) - assert hasattr(field_type, "two") + assert hasattr(field_type, "TWO") def test_should_postgresql_array_convert(): diff --git a/graphene_sqlalchemy/tests/test_query.py b/graphene_sqlalchemy/tests/test_query.py index fc4ccc14..9286624e 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 ..types import SQLAlchemyObjectType from ..utils import sort_argument_for_model, sort_enum_for_model -from .models import Article, Base, Editor, Hairkind, Pet, Reporter +from .models import Article, Base, Editor, HairKind, Pet, Reporter db = create_engine("sqlite://") # use in-memory database @@ -34,16 +34,22 @@ def session(): def setup_fixtures(session): - pet = Pet(name="Lassie", pet_kind="dog", hair_kind=Hairkind.LONG) - session.add(pet) - reporter = Reporter(first_name="ABA", last_name="X") + reporter = Reporter( + first_name='John', last_name='Doe', favorite_pet_kind='cat') session.add(reporter) - reporter2 = Reporter(first_name="ABO", last_name="Y") - session.add(reporter2) - article = Article(headline="Hi!") + pet = Pet(name='Garfield', pet_kind='cat', hair_kind=HairKind.SHORT) + session.add(pet) + pet.reporters.append(reporter) + article = Article(headline='Hi!') article.reporter = reporter session.add(article) - editor = Editor(name="John") + reporter = Reporter( + first_name='Jane', last_name='Roe', favorite_pet_kind='dog') + session.add(reporter) + pet = Pet(name='Lassie', pet_kind='dog', hair_kind=HairKind.LONG) + pet.reporters.append(reporter) + session.add(pet) + editor = Editor(name="Jack") session.add(editor) session.commit() @@ -51,6 +57,11 @@ def setup_fixtures(session): def test_should_query_well(session): setup_fixtures(session) + class PetType(SQLAlchemyObjectType): + + class Meta: + model = Pet + class ReporterType(SQLAlchemyObjectType): class Meta: model = Reporter @@ -58,33 +69,68 @@ class Meta: class Query(graphene.ObjectType): reporter = graphene.Field(ReporterType) reporters = graphene.List(ReporterType) + pets = graphene.List(PetType, kind=graphene.Argument( + PetType._meta.fields['pet_kind'].type)) - def resolve_reporter(self, *args, **kwargs): + def resolve_reporter(self, _info): return session.query(Reporter).first() - def resolve_reporters(self, *args, **kwargs): + def resolve_reporters(self, _info): return session.query(Reporter) + def resolve_pets(self, _info, kind): + query = session.query(Pet) + if kind: + query = query.filter_by(pet_kind=kind) + return query + query = """ query ReporterQuery { reporter { firstName, lastName, - email + email, + favoritePetKind, + pets { + name + petKind + } } reporters { firstName } + pets(kind: DOG) { + name + petKind + } } """ expected = { - "reporter": {"firstName": "ABA", "lastName": "X", "email": None}, - "reporters": [{"firstName": "ABA"}, {"firstName": "ABO"}], + 'reporter': { + 'firstName': 'John', + 'lastName': 'Doe', + 'email': None, + 'favoritePetKind': 'CAT', + 'pets': [{ + 'name': 'Garfield', + 'petKind': 'CAT' + }] + }, + 'reporters': [{ + 'firstName': 'John', + }, { + 'firstName': 'Jane', + }], + 'pets': [{ + 'name': 'Lassie', + 'petKind': 'DOG' + }] } schema = graphene.Schema(query=Query) result = schema.execute(query) assert not result.errors - assert result.data == expected + result = to_std_dicts(result.data) + assert result == expected def test_should_query_enums(session): @@ -97,7 +143,7 @@ class Meta: class Query(graphene.ObjectType): pet = graphene.Field(PetType) - def resolve_pet(self, *args, **kwargs): + def resolve_pet(self, _info): return session.query(Pet).first() query = """ @@ -109,11 +155,12 @@ def resolve_pet(self, *args, **kwargs): } } """ - expected = {"pet": {"name": "Lassie", "petKind": "dog", "hairKind": "LONG"}} + expected = {"pet": {"name": "Garfield", "petKind": "CAT", "hairKind": "SHORT"}} schema = graphene.Schema(query=Query) result = schema.execute(query) assert not result.errors - assert result.data == expected, result.data + result = to_std_dicts(result.data) + assert result == expected def test_enum_parameter(session): @@ -124,16 +171,18 @@ class Meta: model = Pet class Query(graphene.ObjectType): - pet = graphene.Field(PetType, kind=graphene.Argument(PetType._meta.fields['pet_kind'].type.of_type)) + pet = graphene.Field( + PetType, + kind=graphene.Argument(PetType._meta.fields['pet_kind'].type.of_type)) - def resolve_pet(self, info, kind=None, *args, **kwargs): + def resolve_pet(self, info, kind=None): query = session.query(Pet) if kind: query = query.filter(Pet.pet_kind == kind) return query.first() query = """ - query PetQuery($kind: pet_kind) { + query PetQuery($kind: PetKind) { pet(kind: $kind) { name, petKind @@ -141,14 +190,15 @@ def resolve_pet(self, info, kind=None, *args, **kwargs): } } """ - expected = {"pet": {"name": "Lassie", "petKind": "dog", "hairKind": "LONG"}} schema = graphene.Schema(query=Query) - result = schema.execute(query, variables={"kind": "cat"}) + result = schema.execute(query, variables={"kind": "CAT"}) assert not result.errors - assert result.data == {"pet": None} - result = schema.execute(query, variables={"kind": "dog"}) + expected = {"pet": {"name": "Garfield", "petKind": "CAT", "hairKind": "SHORT"}} + assert result.data == expected + result = schema.execute(query, variables={"kind": "DOG"}) assert not result.errors - assert result.data == expected, result.data + expected = {"pet": {"name": "Lassie", "petKind": "DOG", "hairKind": "LONG"}} + assert result.data == expected def test_py_enum_parameter(session): @@ -161,15 +211,15 @@ class Meta: class Query(graphene.ObjectType): pet = graphene.Field(PetType, kind=graphene.Argument(PetType._meta.fields['hair_kind'].type.of_type)) - def resolve_pet(self, info, kind=None, *args, **kwargs): + def resolve_pet(self, _info, kind=None): query = session.query(Pet) if kind: # XXX Why kind passed in as a str instead of a Hairkind instance? - query = query.filter(Pet.hair_kind == Hairkind(kind)) + query = query.filter(Pet.hair_kind == HairKind(kind)) return query.first() query = """ - query PetQuery($kind: Hairkind) { + query PetQuery($kind: HairKind) { pet(kind: $kind) { name, petKind @@ -177,14 +227,15 @@ def resolve_pet(self, info, kind=None, *args, **kwargs): } } """ - expected = {"pet": {"name": "Lassie", "petKind": "dog", "hairKind": "LONG"}} schema = graphene.Schema(query=Query) result = schema.execute(query, variables={"kind": "SHORT"}) assert not result.errors - assert result.data == {"pet": None} + expected = {"pet": {"name": "Garfield", "petKind": "CAT", "hairKind": "SHORT"}} + assert result.data == expected result = schema.execute(query, variables={"kind": "LONG"}) assert not result.errors - assert result.data == expected, result.data + expected = {"pet": {"name": "Lassie", "petKind": "DOG", "hairKind": "LONG"}} + assert result.data == expected def test_should_node(session): @@ -218,10 +269,10 @@ class Query(graphene.ObjectType): article = graphene.Field(ArticleNode) all_articles = SQLAlchemyConnectionField(ArticleConnection) - def resolve_reporter(self, *args, **kwargs): + def resolve_reporter(self, _info): return session.query(Reporter).first() - def resolve_article(self, *args, **kwargs): + def resolve_article(self, _info): return session.query(Article).first() query = """ @@ -260,8 +311,8 @@ def resolve_article(self, *args, **kwargs): expected = { "reporter": { "id": "UmVwb3J0ZXJOb2RlOjE=", - "firstName": "ABA", - "lastName": "X", + "firstName": "John", + "lastName": "Doe", "email": None, "articles": {"edges": [{"node": {"headline": "Hi!"}}]}, }, @@ -271,7 +322,8 @@ def resolve_article(self, *args, **kwargs): schema = graphene.Schema(query=Query) result = schema.execute(query, context_value={"session": session}) assert not result.errors - assert result.data == expected + result = to_std_dicts(result.data) + assert result == expected def test_should_custom_identifier(session): @@ -308,14 +360,15 @@ class Query(graphene.ObjectType): } """ expected = { - "allEditors": {"edges": [{"node": {"id": "RWRpdG9yTm9kZTox", "name": "John"}}]}, - "node": {"name": "John"}, + "allEditors": {"edges": [{"node": {"id": "RWRpdG9yTm9kZTox", "name": "Jack"}}]}, + "node": {"name": "Jack"}, } schema = graphene.Schema(query=Query) result = schema.execute(query, context_value={"session": session}) assert not result.errors - assert result.data == expected + result = to_std_dicts(result.data) + assert result == expected def test_should_mutate_well(session): @@ -385,7 +438,7 @@ class Mutation(graphene.ObjectType): "ok": True, "article": { "headline": "My Article", - "reporter": {"id": "UmVwb3J0ZXJOb2RlOjE=", "firstName": "ABA"}, + "reporter": {"id": "UmVwb3J0ZXJOb2RlOjE=", "firstName": "John"}, }, } } @@ -393,14 +446,15 @@ class Mutation(graphene.ObjectType): schema = graphene.Schema(query=Query, mutation=Mutation) result = schema.execute(query, context_value={"session": session}) assert not result.errors - assert result.data == expected + result = to_std_dicts(result.data) + assert result == expected def sort_setup(session): pets = [ - 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), + 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() @@ -493,9 +547,9 @@ def makeNodes(nodeList): ), "multipleSort": makeNodes( [ - {"name": "Alf", "petKind": "cat"}, - {"name": "Lassie", "petKind": "dog"}, - {"name": "Barf", "petKind": "dog"}, + {"name": "Alf", "petKind": "CAT"}, + {"name": "Lassie", "petKind": "DOG"}, + {"name": "Barf", "petKind": "DOG"}, ] ), "descSort": makeNodes([{"name": "Lassie"}, {"name": "Barf"}, {"name": "Alf"}]), @@ -507,7 +561,8 @@ def makeNodes(nodeList): schema = graphene.Schema(query=Query) result = schema.execute(query, context_value={"session": session}) assert not result.errors - assert result.data == expected + result = to_std_dicts(result.data) + assert result == expected queryError = """ query sortTest { @@ -555,3 +610,13 @@ def makeNodes(nodeList): assert set(node["node"]["name"] for node in value["edges"]) == set( node["node"]["name"] for node in expectedNoSort[key]["edges"] ) + + +def to_std_dicts(value): + """Convert nested ordered dicts to normal dicts for better comparison.""" + if isinstance(value, dict): + return {k: to_std_dicts(v) for k, v in value.items()} + elif isinstance(value, list): + return [to_std_dicts(v) for v in value] + else: + return value diff --git a/graphene_sqlalchemy/tests/test_schema.py b/graphene_sqlalchemy/tests/test_schema.py index 628da185..87739bdb 100644 --- a/graphene_sqlalchemy/tests/test_schema.py +++ b/graphene_sqlalchemy/tests/test_schema.py @@ -35,6 +35,7 @@ class Meta: "first_name", "last_name", "email", + "favorite_pet_kind", "pets", "articles", "favorite_article", diff --git a/graphene_sqlalchemy/tests/test_types.py b/graphene_sqlalchemy/tests/test_types.py index 5eaf0137..b0375c60 100644 --- a/graphene_sqlalchemy/tests/test_types.py +++ b/graphene_sqlalchemy/tests/test_types.py @@ -54,6 +54,7 @@ def test_objecttype_registered(): "first_name", "last_name", "email", + "favorite_pet_kind", "pets", "articles", "favorite_article", @@ -121,6 +122,7 @@ def test_custom_objecttype_registered(): "first_name", "last_name", "email", + "favorite_pet_kind", "pets", "articles", "favorite_article", @@ -165,6 +167,7 @@ def test_objecttype_with_custom_options(): "first_name", "last_name", "email", + "favorite_pet_kind", "pets", "articles", "favorite_article", @@ -178,7 +181,7 @@ class TestConnection(Connection): class Meta: node = ReporterWithCustomOptions - def resolver(*args, **kwargs): + def resolver(_obj, _info): return Promise.resolve([]) result = SQLAlchemyConnectionField.connection_resolver( diff --git a/graphene_sqlalchemy/tests/test_utils.py b/graphene_sqlalchemy/tests/test_utils.py index a7b902fe..67771633 100644 --- a/graphene_sqlalchemy/tests/test_utils.py +++ b/graphene_sqlalchemy/tests/test_utils.py @@ -2,7 +2,8 @@ from graphene import Enum, List, ObjectType, Schema, String -from ..utils import get_session, sort_argument_for_model, sort_enum_for_model +from ..utils import (get_session, sort_argument_for_model, sort_enum_for_model, + to_enum_value_name, to_type_name) from .models import Editor, Pet @@ -27,6 +28,18 @@ def resolve_x(self, info): assert result.data["x"] == session +def test_to_type_name(): + assert to_type_name('make_camel_case') == 'MakeCamelCase' + assert to_type_name('AlreadyCamelCase') == 'AlreadyCamelCase' + + +def test_to_enum_value_name(): + assert to_enum_value_name('make_enum_value_name') == 'MAKE_ENUM_VALUE_NAME' + assert to_enum_value_name('makeEnumValueName') == 'MAKE_ENUM_VALUE_NAME' + assert to_enum_value_name('HTTPStatus400Message') == 'HTTP_STATUS400_MESSAGE' + assert to_enum_value_name('ALREADY_ENUM_VALUE_NAME') == 'ALREADY_ENUM_VALUE_NAME' + + def test_sort_enum_for_model(): enum = sort_enum_for_model(Pet) assert isinstance(enum, type(Enum)) diff --git a/graphene_sqlalchemy/utils.py b/graphene_sqlalchemy/utils.py index 276a8075..26a889ea 100644 --- a/graphene_sqlalchemy/utils.py +++ b/graphene_sqlalchemy/utils.py @@ -1,3 +1,5 @@ +import re + from sqlalchemy.exc import ArgumentError from sqlalchemy.inspection import inspect from sqlalchemy.orm import class_mapper, object_mapper @@ -41,6 +43,21 @@ def is_mapped_instance(cls): return True +def to_type_name(name): + """Convert the given name to a GraphQL type name.""" + return ''.join(part[:1].upper() + part[1:] for part in name.split('_')) + + +_re_enum_value_name_1 = re.compile('(.)([A-Z][a-z]+)') +_re_enum_value_name_2 = re.compile('([a-z0-9])([A-Z])') + + +def to_enum_value_name(name): + """Convert the given name to a GraphQL enum value name.""" + return _re_enum_value_name_2.sub( + r'\1_\2', _re_enum_value_name_1.sub(r'\1_\2', name)).upper() + + def _symbol_name(column_name, is_asc): return column_name + ("_asc" if is_asc else "_desc")