From 787d8a86820fbee49f342cb11200f31283322afc Mon Sep 17 00:00:00 2001 From: Mike Pickfield Date: Mon, 27 Jul 2020 16:28:31 -0400 Subject: [PATCH 1/3] Alerted is_mapped_class to allow the original sqlalchemy error to propogate. --- graphene_sqlalchemy/tests/test_types.py | 67 +++++++++++++++++++++++++ graphene_sqlalchemy/types.py | 8 +-- graphene_sqlalchemy/utils.py | 7 ++- 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/graphene_sqlalchemy/tests/test_types.py b/graphene_sqlalchemy/tests/test_types.py index bf563b6e..139a91ba 100644 --- a/graphene_sqlalchemy/tests/test_types.py +++ b/graphene_sqlalchemy/tests/test_types.py @@ -1,6 +1,11 @@ import mock import pytest import six # noqa F401 +import sqlalchemy.exc +import sqlalchemy.orm.exc +from promise import Promise + +from .. import utils from graphene import (Dynamic, Field, GlobalID, Int, List, Node, NonNull, ObjectType, Schema, String) @@ -492,3 +497,65 @@ class Meta: def test_deprecated_createConnectionField(): with pytest.warns(DeprecationWarning): createConnectionField(None) + + +@mock.patch(utils.__name__ + '.class_mapper') +def test_unique_errors_propagate(class_mapper_mock): + # Define unique error to detect + class UniqueError(Exception): + pass + + # Mock class_mapper effect + class_mapper_mock.side_effect = UniqueError + + # Make sure that errors are propagated from class_mapper when instantiating new classes + error = None + try: + class ArticleOne(SQLAlchemyObjectType): + class Meta(object): + model = Article + except UniqueError as e: + error = e + + # Check that an error occured, and that it was the unique error we gave + assert error is not None + assert isinstance(error, UniqueError) + + +@mock.patch(utils.__name__ + '.class_mapper') +def test_argument_errors_propagate(class_mapper_mock): + # Mock class_mapper effect + class_mapper_mock.side_effect = sqlalchemy.exc.ArgumentError + + # Make sure that errors are propagated from class_mapper when instantiating new classes + error = None + try: + class ArticleTwo(SQLAlchemyObjectType): + class Meta(object): + model = Article + except sqlalchemy.exc.ArgumentError as e: + error = e + + # Check that an error occured, and that it was the unique error we gave + assert error is not None + assert isinstance(error, sqlalchemy.exc.ArgumentError) + + +@mock.patch(utils.__name__ + '.class_mapper') +def test_unmapped_errors_reformat(class_mapper_mock): + # Mock class_mapper effect + class_mapper_mock.side_effect = sqlalchemy.orm.exc.UnmappedClassError(object) + + # Make sure that errors are propagated from class_mapper when instantiating new classes + error = None + try: + class ArticleThree(SQLAlchemyObjectType): + class Meta(object): + model = Article + except ValueError as e: + error = e + + # Check that an error occured, and that it was the unique error we gave + assert error is not None + assert isinstance(error, ValueError) + assert "You need to pass a valid SQLAlchemy Model" in str(error) diff --git a/graphene_sqlalchemy/types.py b/graphene_sqlalchemy/types.py index ff22cded..1a1dbd33 100644 --- a/graphene_sqlalchemy/types.py +++ b/graphene_sqlalchemy/types.py @@ -207,10 +207,10 @@ def __init_subclass_with_meta__( _meta=None, **options ): - assert is_mapped_class(model), ( - "You need to pass a valid SQLAlchemy Model in " '{}.Meta, received "{}".' - ).format(cls.__name__, model) - + if not is_mapped_class(model): + raise ValueError( + "You need to pass a valid SQLAlchemy Model in " '{}.Meta, received "{}".'.format(cls.__name__, model) + ) if not registry: registry = get_global_registry() diff --git a/graphene_sqlalchemy/utils.py b/graphene_sqlalchemy/utils.py index 7139eefc..7a47bde8 100644 --- a/graphene_sqlalchemy/utils.py +++ b/graphene_sqlalchemy/utils.py @@ -26,7 +26,12 @@ def get_query(model, context): def is_mapped_class(cls): try: class_mapper(cls) - except (ArgumentError, UnmappedClassError): + except ArgumentError as error: + # Only handle ArgumentErrors for non-class objects + if "Class object expected" in str(error): + return False + raise error + except UnmappedClassError: return False else: return True From 023ac83a2422ee04efd8b70cc1ee883aaddfd858 Mon Sep 17 00:00:00 2001 From: Mike Pickfield Date: Mon, 27 Jul 2020 16:45:19 -0400 Subject: [PATCH 2/3] Removed unused import (promise.Promise) --- graphene_sqlalchemy/tests/test_types.py | 1 - 1 file changed, 1 deletion(-) diff --git a/graphene_sqlalchemy/tests/test_types.py b/graphene_sqlalchemy/tests/test_types.py index 139a91ba..83747435 100644 --- a/graphene_sqlalchemy/tests/test_types.py +++ b/graphene_sqlalchemy/tests/test_types.py @@ -3,7 +3,6 @@ import six # noqa F401 import sqlalchemy.exc import sqlalchemy.orm.exc -from promise import Promise from .. import utils From dad2abe7a6788cc9bcb2739785afa2e35524e41b Mon Sep 17 00:00:00 2001 From: Mike Pickfield Date: Mon, 27 Jul 2020 16:54:19 -0400 Subject: [PATCH 3/3] sorted imports in test_types --- graphene_sqlalchemy/tests/test_types.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/graphene_sqlalchemy/tests/test_types.py b/graphene_sqlalchemy/tests/test_types.py index 83747435..e9baf206 100644 --- a/graphene_sqlalchemy/tests/test_types.py +++ b/graphene_sqlalchemy/tests/test_types.py @@ -4,12 +4,11 @@ import sqlalchemy.exc import sqlalchemy.orm.exc -from .. import utils - from graphene import (Dynamic, Field, GlobalID, Int, List, Node, NonNull, ObjectType, Schema, String) from graphene.relay import Connection +from .. import utils from ..converter import convert_sqlalchemy_composite from ..fields import (SQLAlchemyConnectionField, UnsortedSQLAlchemyConnectionField, createConnectionField,