From 4de77c95e12a4a9838654f9198db2ecc6a27d8d5 Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Sat, 18 Jun 2016 14:33:04 -0700 Subject: [PATCH] Improved django integration --- graphene-django/django_test_settings.py | 6 +- graphene-django/examples/starwars/schema.py | 37 +++-- graphene-django/graphene_django/converter.py | 23 +-- .../graphene_django/form_converter.py | 2 +- graphene-django/graphene_django/registry.py | 29 ++++ .../graphene_django/tests/test_converter.py | 21 +-- .../tests/test_form_converter.py | 3 +- .../graphene_django/tests/test_query.py | 9 +- .../graphene_django/tests/test_types.py | 142 ++++++++++-------- .../graphene_django/tests/test_views.py | 8 +- .../tests/{test_urls.py => urls.py} | 4 +- graphene-django/graphene_django/types.py | 61 +++++--- 12 files changed, 195 insertions(+), 150 deletions(-) create mode 100644 graphene-django/graphene_django/registry.py rename graphene-django/graphene_django/tests/{test_urls.py => urls.py} (88%) diff --git a/graphene-django/django_test_settings.py b/graphene-django/django_test_settings.py index 3be92ade..12efbf60 100644 --- a/graphene-django/django_test_settings.py +++ b/graphene-django/django_test_settings.py @@ -1,9 +1,13 @@ +import sys, os +ROOT_PATH = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, ROOT_PATH + '/examples/') + SECRET_KEY = 1 INSTALLED_APPS = [ 'graphene_django', 'graphene_django.tests', - 'examples.starwars', + 'starwars', ] DATABASES = { diff --git a/graphene-django/examples/starwars/schema.py b/graphene-django/examples/starwars/schema.py index 88407103..b30ebcc0 100644 --- a/graphene-django/examples/starwars/schema.py +++ b/graphene-django/examples/starwars/schema.py @@ -1,24 +1,26 @@ import graphene -from graphene import relay, resolve_only_args -from graphene.contrib.django import DjangoNode, DjangoObjectType +from graphene import relay, resolve_only_args, Schema +from graphene_django import DjangoNode, DjangoObjectType from .data import (create_ship, get_empire, get_faction, get_rebels, get_ship, get_ships) -from .models import Character as CharacterModel -from .models import Faction as FactionModel -from .models import Ship as ShipModel - -schema = graphene.Schema(name='Starwars Django Relay Schema') +from .models import ( + Character as CharacterModel, + Faction as FactionModel, + Ship as ShipModel +) -class Ship(DjangoNode): +class Ship(DjangoNode, DjangoObjectType): class Meta: model = ShipModel @classmethod - def get_node(cls, id, info): - return Ship(get_ship(id)) + def get_node(cls, id, context, info): + node = get_ship(id) + print(node) + return node class Character(DjangoObjectType): @@ -27,14 +29,14 @@ class Character(DjangoObjectType): model = CharacterModel -class Faction(DjangoNode): +class Faction(DjangoNode, DjangoObjectType): class Meta: model = FactionModel @classmethod - def get_node(cls, id, info): - return Faction(get_faction(id)) + def get_node(cls, id, context, info): + return get_faction(id) class IntroduceShip(relay.ClientIDMutation): @@ -52,13 +54,13 @@ class IntroduceShip(relay.ClientIDMutation): faction_id = input.get('faction_id') ship = create_ship(ship_name, faction_id) faction = get_faction(faction_id) - return IntroduceShip(ship=Ship(ship), faction=Faction(faction)) + return IntroduceShip(ship=ship, faction=faction) class Query(graphene.ObjectType): rebels = graphene.Field(Faction) empire = graphene.Field(Faction) - node = relay.NodeField() + node = relay.Node.Field() ships = relay.ConnectionField(Ship, description='All the ships.') @resolve_only_args @@ -80,7 +82,4 @@ class Mutation(graphene.ObjectType): # We register the Character Model because if not would be # inaccessible for the schema -schema.register(Character) - -schema.query = Query -schema.mutation = Mutation +schema = Schema(query=Query, mutation=Mutation, types=[]) diff --git a/graphene-django/graphene_django/converter.py b/graphene-django/graphene_django/converter.py index 2fad5f63..bbeaa379 100644 --- a/graphene-django/graphene_django/converter.py +++ b/graphene-django/graphene_django/converter.py @@ -2,6 +2,7 @@ from django.db import models from django.utils.encoding import force_text from graphene import Enum, List, ID, Boolean, Float, Int, String, Field +from graphene.types.datetime import DateTime from graphene.utils.str_converters import to_const from graphene.relay import Node, ConnectionField # from ...core.types.custom_scalars import DateTime, JSONString @@ -13,22 +14,6 @@ from .fields import DjangoConnectionField singledispatch = import_single_dispatch() -class Registry(object): - def __init__(self): - self._registry = {} - self._registry_models = {} - - def register(self, cls): - from .types import DjangoObjectType - print(cls.get_registry(), self) - assert issubclass(cls, DjangoObjectType), 'Only DjangoObjectTypes can be registered, received "{}"'.format(cls.__name__) - assert cls.get_registry() == self, 'Registry for a Model have to match.' - self._registry[cls._meta.model] = cls - - def get_type_for_model(self, model): - return self._registry.get(model) - - def convert_choices(choices): for value, name in choices: if isinstance(name, (tuple, list)): @@ -42,9 +27,11 @@ def convert_django_field_with_choices(field, registry=None): choices = getattr(field, 'choices', None) if choices: meta = field.model._meta - name = '{}_{}_{}'.format(meta.app_label, meta.object_name, field.name) + name = '{}{}'.format(meta.object_name, field.name.capitalize()) graphql_choices = list(convert_choices(choices)) - return Enum(name.upper(), graphql_choices, description=field.help_text) + from collections import OrderedDict + enum = Enum(name, OrderedDict(graphql_choices)) + return enum(description=field.help_text) return convert_django_field(field, registry) diff --git a/graphene-django/graphene_django/form_converter.py b/graphene-django/graphene_django/form_converter.py index ae32b63c..2ddb912b 100644 --- a/graphene-django/graphene_django/form_converter.py +++ b/graphene-django/graphene_django/form_converter.py @@ -61,7 +61,7 @@ def convert_form_field_to_float(field): @convert_form_field.register(forms.ModelMultipleChoiceField) @convert_form_field.register(GlobalIDMultipleChoiceField) def convert_form_field_to_list(field): - return List(ID()) + return List(ID) @convert_form_field.register(forms.ModelChoiceField) diff --git a/graphene-django/graphene_django/registry.py b/graphene-django/graphene_django/registry.py new file mode 100644 index 00000000..464e8263 --- /dev/null +++ b/graphene-django/graphene_django/registry.py @@ -0,0 +1,29 @@ +class Registry(object): + def __init__(self): + self._registry = {} + self._registry_models = {} + + def register(self, cls): + from .types import DjangoObjectType + assert issubclass(cls, DjangoObjectType), 'Only DjangoObjectTypes can be registered, received "{}"'.format(cls.__name__) + assert cls._meta.registry == self, 'Registry for a Model have to match.' + # assert self.get_type_for_model(cls._meta.model) == cls, 'Multiple DjangoObjectTypes registered for "{}"'.format(cls._meta.model) + self._registry[cls._meta.model] = cls + + def get_type_for_model(self, model): + return self._registry.get(model) + + +registry = None + + +def get_global_registry(): + global registry + if not registry: + registry = Registry() + return registry + + +def reset_global_registry(): + global registry + registry = None diff --git a/graphene-django/graphene_django/tests/test_converter.py b/graphene-django/graphene_django/tests/test_converter.py index 70ca46f9..59beff2f 100644 --- a/graphene-django/graphene_django/tests/test_converter.py +++ b/graphene-django/graphene_django/tests/test_converter.py @@ -5,12 +5,14 @@ from py.test import raises import graphene from graphene.relay import Node, ConnectionField +from graphene.types.datetime import DateTime from graphene.utils.get_graphql_type import get_graphql_type # from graphene.core.types.custom_scalars import DateTime, JSONString from ..compat import (ArrayField, HStoreField, JSONField, MissingType, RangeField) -from ..converter import convert_django_field, convert_django_field_with_choices, Registry +from ..converter import convert_django_field, convert_django_field_with_choices +from ..registry import Registry from .models import Article, Reporter, Film, FilmDetails, Pet from ..types import DjangoObjectType, DjangoNode @@ -163,27 +165,22 @@ def test_should_manytomany_convert_connectionorlist(): def test_should_manytomany_convert_connectionorlist_list(): - registry = Registry() - class A(DjangoObjectType): class Meta: model = Reporter - registry.register(A) - graphene_field = convert_django_field(Reporter._meta.local_many_to_many[0], registry) + graphene_field = convert_django_field(Reporter._meta.local_many_to_many[0], A._meta.registry) assert isinstance(graphene_field, graphene.Field) assert isinstance(graphene_field.type, graphene.List) assert graphene_field.type.of_type == get_graphql_type(A) def test_should_manytomany_convert_connectionorlist_connection(): - registry = Registry() class A(DjangoNode, DjangoObjectType): class Meta: model = Reporter - registry.register(A) - graphene_field = convert_django_field(Reporter._meta.local_many_to_many[0], registry) + graphene_field = convert_django_field(Reporter._meta.local_many_to_many[0], A._meta.registry) assert isinstance(graphene_field, ConnectionField) assert graphene_field.type == get_graphql_type(A.get_default_connection()) @@ -192,14 +189,12 @@ def test_should_manytoone_convert_connectionorlist(): # Django 1.9 uses 'rel', <1.9 uses 'related related = getattr(Reporter.articles, 'rel', None) or \ getattr(Reporter.articles, 'related') - registry = Registry() class A(DjangoObjectType): class Meta: model = Article - registry.register(A) - graphene_field = convert_django_field(related, registry) + graphene_field = convert_django_field(related, A._meta.registry) assert isinstance(graphene_field, graphene.Field) assert isinstance(graphene_field.type, graphene.List) assert graphene_field.type.of_type == get_graphql_type(A) @@ -214,9 +209,7 @@ def test_should_onetoone_reverse_convert_model(): class Meta: model = FilmDetails - registry = Registry() - registry.register(A) - graphene_field = convert_django_field(related, registry) + graphene_field = convert_django_field(related, A._meta.registry) assert isinstance(graphene_field, graphene.Field) assert graphene_field.type == get_graphql_type(A) diff --git a/graphene-django/graphene_django/tests/test_form_converter.py b/graphene-django/graphene_django/tests/test_form_converter.py index db5e0dd8..42fcc53e 100644 --- a/graphene-django/graphene_django/tests/test_form_converter.py +++ b/graphene-django/graphene_django/tests/test_form_converter.py @@ -4,6 +4,7 @@ from py.test import raises import graphene from ..form_converter import convert_form_field from graphene import ID, List +from graphene.utils.get_graphql_type import get_graphql_type from .models import Reporter @@ -94,7 +95,7 @@ def test_should_multiple_choice_convert_connectionorlist(): field = forms.ModelMultipleChoiceField(Reporter.objects.all()) graphene_type = convert_form_field(field) assert isinstance(graphene_type, List) - assert isinstance(graphene_type.of_type, ID) + assert graphene_type.of_type == get_graphql_type(ID) def test_should_manytoone_convert_connectionorlist(): diff --git a/graphene-django/graphene_django/tests/test_query.py b/graphene-django/graphene_django/tests/test_query.py index 6d6b7540..c8fd6439 100644 --- a/graphene-django/graphene_django/tests/test_query.py +++ b/graphene-django/graphene_django/tests/test_query.py @@ -9,6 +9,7 @@ from graphene import relay from ..compat import MissingType, RangeField from ..types import DjangoNode, DjangoObjectType +from ..registry import reset_global_registry from .models import Article, Reporter pytestmark = pytest.mark.django_db @@ -118,6 +119,8 @@ def test_should_query_postgres_fields(): def test_should_node(): + reset_global_registry() + class ReporterNode(DjangoNode): class Meta: @@ -128,7 +131,7 @@ def test_should_node(): return ReporterNode(Reporter(id=2, first_name='Cookie Monster')) def resolve_articles(self, *args, **kwargs): - return [ArticleNode(Article(headline='Hi!'))] + return [Article(headline='Hi!')] class ArticleNode(DjangoNode): @@ -137,10 +140,10 @@ def test_should_node(): @classmethod def get_node(cls, id, info): - return ArticleNode(Article(id=1, headline='Article node', pub_date=datetime.date(2002, 3, 11))) + return Article(id=1, headline='Article node', pub_date=datetime.date(2002, 3, 11)) class Query(graphene.ObjectType): - node = relay.NodeField() + node = relay.Node.Field() reporter = graphene.Field(ReporterNode) article = graphene.Field(ArticleNode) diff --git a/graphene-django/graphene_django/tests/test_types.py b/graphene-django/graphene_django/tests/test_types.py index a930b78d..76e34f7b 100644 --- a/graphene-django/graphene_django/tests/test_types.py +++ b/graphene-django/graphene_django/tests/test_types.py @@ -1,102 +1,120 @@ from graphql.type import GraphQLObjectType from mock import patch -from graphene import Schema, Interface +from graphene import ObjectType, Field, Int, ID, Schema, Interface +from graphene.relay import Node, ConnectionField from ..types import DjangoNode, DjangoObjectType -from graphene.core.fields import Field -from graphene.core.types.scalars import Int -from graphene.relay.fields import GlobalIDField -from tests.utils import assert_equal_lists -from .models import Article, Reporter - -schema = Schema() +from .models import Article as ArticleModel, Reporter as ReporterModel -@schema.register -class Character(DjangoObjectType): +class Reporter(DjangoObjectType): '''Character description''' class Meta: - model = Reporter + model = ReporterModel -@schema.register -class Human(DjangoNode): +class Article(DjangoNode, DjangoObjectType): '''Human description''' pub_date = Int() class Meta: - model = Article + model = ArticleModel + + +class RootQuery(ObjectType): + node = DjangoNode.Field() + + +schema = Schema(query=RootQuery, types=[Article, Reporter]) def test_django_interface(): - assert DjangoNode._meta.interface is True + assert issubclass(DjangoNode, Interface) + assert issubclass(DjangoNode, Node) @patch('graphene_django.tests.models.Article.objects.get', return_value=Article(id=1)) def test_django_get_node(get): - human = Human.get_node(1, None) + article = Article.get_node(1, None, None) get.assert_called_with(id=1) - assert human.id == 1 + assert article.id == 1 -def test_djangonode_idfield(): - idfield = DjangoNode._meta.fields_map['id'] - assert isinstance(idfield, GlobalIDField) +def test_django_objecttype_map_correct_fields(): + graphql_type = Reporter._meta.graphql_type + assert list(graphql_type.get_fields().keys()) == ['id', 'firstName', 'lastName', 'email', 'pets', 'aChoice', 'articles'] -def test_node_idfield(): - idfield = Human._meta.fields_map['id'] - assert isinstance(idfield, GlobalIDField) +def test_django_objecttype_with_node_have_correct_fields(): + graphql_type = Article._meta.graphql_type + assert list(graphql_type.get_fields().keys()) == ['id', 'pubDate', 'headline', 'reporter', 'lang', 'importance'] -def test_node_replacedfield(): - idfield = Human._meta.fields_map['pub_date'] - assert isinstance(idfield, Field) - assert schema.T(idfield).type == schema.T(Int()) +def test_schema_representation(): + expected = """ +schema { + query: RootQuery +} +type Article implements Node { + id: ID! + pubDate: Int + headline: String + reporter: Reporter + lang: ArticleLang + importance: ArticleImportance +} -def test_objecttype_init_none(): - h = Human() - assert h._root is None +type ArticleConnection { + pageInfo: PageInfo! + edges: [ArticleEdge] +} +type ArticleEdge { + node: Article + cursor: String! +} -def test_objecttype_init_good(): - instance = Article() - h = Human(instance) - assert h._root == instance +enum ArticleImportance { + VERY_IMPORTANT + NOT_AS_IMPORTANT +} +enum ArticleLang { + SPANISH + ENGLISH +} -def test_object_type(): - object_type = schema.T(Human) - Human._meta.fields_map - assert Human._meta.interface is False - assert isinstance(object_type, GraphQLObjectType) - assert_equal_lists( - object_type.get_fields().keys(), - ['headline', 'id', 'reporter', 'pubDate'] - ) - assert schema.T(DjangoNode) in object_type.get_interfaces() +interface Node { + id: ID! +} +type PageInfo { + hasNextPage: Boolean! + hasPreviousPage: Boolean! + startCursor: String + endCursor: String +} -def test_node_notinterface(): - assert Human._meta.interface is False - assert DjangoNode in Human._meta.interfaces +type Reporter { + id: ID + firstName: String + lastName: String + email: String + pets: [Reporter] + aChoice: ReporterA_choice + articles(before: String, after: String, first: Int, last: Int): ArticleConnection +} +enum ReporterA_choice { + THIS + THAT +} -def test_django_objecttype_could_extend_interface(): - schema = Schema() - - @schema.register - class Customer(Interface): - id = Int() - - @schema.register - class UserType(DjangoObjectType): - class Meta: - model = Reporter - interfaces = [Customer] - - object_type = schema.T(UserType) - assert schema.T(Customer) in object_type.get_interfaces() +type RootQuery { + node(id: ID!): Node +} +""".lstrip() + assert str(schema) == expected diff --git a/graphene-django/graphene_django/tests/test_views.py b/graphene-django/graphene_django/tests/test_views.py index 46179943..efa0381f 100644 --- a/graphene-django/graphene_django/tests/test_views.py +++ b/graphene-django/graphene_django/tests/test_views.py @@ -6,7 +6,7 @@ def format_response(response): def test_client_get_good_query(settings, client): - settings.ROOT_URLCONF = 'graphene_django.tests.test_urls' + settings.ROOT_URLCONF = 'graphene_django.tests.urls' response = client.get('/graphql', {'query': '{ human { headline } }'}) json_response = format_response(response) expected_json = { @@ -20,7 +20,7 @@ def test_client_get_good_query(settings, client): def test_client_get_good_query_with_raise(settings, client): - settings.ROOT_URLCONF = 'graphene_django.tests.test_urls' + settings.ROOT_URLCONF = 'graphene_django.tests.urls' response = client.get('/graphql', {'query': '{ human { raises } }'}) json_response = format_response(response) assert json_response['errors'][0]['message'] == 'This field should raise exception' @@ -28,7 +28,7 @@ def test_client_get_good_query_with_raise(settings, client): def test_client_post_good_query_json(settings, client): - settings.ROOT_URLCONF = 'graphene_django.tests.test_urls' + settings.ROOT_URLCONF = 'graphene_django.tests.urls' response = client.post( '/graphql', json.dumps({'query': '{ human { headline } }'}), 'application/json') json_response = format_response(response) @@ -43,7 +43,7 @@ def test_client_post_good_query_json(settings, client): def test_client_post_good_query_graphql(settings, client): - settings.ROOT_URLCONF = 'graphene_django.tests.test_urls' + settings.ROOT_URLCONF = 'graphene_django.tests.urls' response = client.post( '/graphql', '{ human { headline } }', 'application/graphql') json_response = format_response(response) diff --git a/graphene-django/graphene_django/tests/test_urls.py b/graphene-django/graphene_django/tests/urls.py similarity index 88% rename from graphene-django/graphene_django/tests/test_urls.py rename to graphene-django/graphene_django/tests/urls.py index 2d5ca4ca..5caa4a82 100644 --- a/graphene-django/graphene_django/tests/test_urls.py +++ b/graphene-django/graphene_django/tests/urls.py @@ -13,7 +13,7 @@ class Character(DjangoNode): class Meta: model = Reporter - def get_node(self, id): + def get_node(self, id, context, info): pass @@ -33,7 +33,7 @@ class Human(DjangoNode): class Query(graphene.ObjectType): human = graphene.Field(Human) - def resolve_human(self, args, info): + def resolve_human(self, args, context, info): return Human() diff --git a/graphene-django/graphene_django/types.py b/graphene-django/graphene_django/types.py index daca36ec..8b02c8b1 100644 --- a/graphene-django/graphene_django/types.py +++ b/graphene-django/graphene_django/types.py @@ -1,4 +1,6 @@ import inspect +from collections import OrderedDict +from functools import partial import six from django.db import models @@ -8,18 +10,38 @@ from graphene.types.objecttype import ObjectType, ObjectTypeMeta, attrs_without_ from graphene.types.interface import InterfaceTypeMeta from graphene.relay import Connection, Node from graphene.relay.node import NodeMeta -from .converter import convert_django_field_with_choices, Registry +from .converter import convert_django_field_with_choices from graphene.types.options import Options -from graphene import String from .utils import get_model_fields +from .registry import Registry, get_global_registry from graphene.utils.is_base_type import is_base_type - from graphene.utils.copy_fields import copy_fields from graphene.utils.get_fields import get_fields -from graphene.utils.is_base_type import is_base_type +from graphene.utils.as_field import as_field class DjangoObjectTypeMeta(ObjectTypeMeta): + def _construct_fields(cls, fields, options): + _model_fields = get_model_fields(options.model) + + for field in _model_fields: + name = field.name + is_not_in_only = options.fields and name not in options.fields + is_already_created = name in fields + is_excluded = field.name in options.exclude or is_already_created + if is_not_in_only or is_excluded: + # We skip this field if we specify only_fields and is not + # in there. Or when we exclude this field in exclude_fields + continue + converted = convert_django_field_with_choices(field, options.registry) + if not converted: + continue + fields[name] = as_field(converted) + + fields = copy_fields(Field, fields, parent=cls) + + return fields + def __new__(cls, name, bases, attrs): # super_new = super(DjangoObjectTypeMeta, cls).__new__ super_new = type.__new__ @@ -37,45 +59,35 @@ class DjangoObjectTypeMeta(ObjectTypeMeta): fields=(), exclude=(), interfaces=(), + registry=None ) + if not options.registry: + options.registry = get_global_registry() + assert isinstance(options.registry, Registry), 'The attribute registry in {}.Meta needs to be an instance of Registry.'.format(name) assert options.model, 'You need to pass a valid Django Model in {}.Meta'.format(name) - get_model_fields(options.model) interfaces = tuple(options.interfaces) fields = get_fields(ObjectType, attrs, bases, interfaces) attrs = attrs_without_fields(attrs, fields) cls = super_new(cls, name, bases, dict(attrs, _meta=options)) - fields = copy_fields(Field, fields, parent=cls) base_interfaces = tuple(b for b in bases if issubclass(b, Interface)) options.graphql_type = GrapheneObjectType( graphene_type=cls, name=options.name or cls.__name__, description=options.description or cls.__doc__, - fields=fields, + fields=partial(cls._construct_fields, fields, options), interfaces=tuple(get_interfaces(interfaces + base_interfaces)) ) - # for field in all_fields: - # is_not_in_only = only_fields and field.name not in only_fields - # is_already_created = field.name in already_created_fields - # is_excluded = field.name in cls._meta.exclude_fields or is_already_created - # if is_not_in_only or is_excluded: - # # We skip this field if we specify only_fields and is not - # # in there. Or when we exclude this field in exclude_fields - # continue - # converted_field = convert_django_field_with_choices(field) + if issubclass(cls, DjangoObjectType): + options.registry.register(cls) + return cls class DjangoObjectType(six.with_metaclass(DjangoObjectTypeMeta, ObjectType)): - _registry = None - - @classmethod - def get_registry(cls): - if not DjangoObjectType._registry: - DjangoObjectType._registry = Registry() - return DjangoObjectType._registry + pass class DjangoNodeMeta(DjangoObjectTypeMeta, NodeMeta): @@ -86,7 +98,6 @@ class DjangoNode(six.with_metaclass(DjangoNodeMeta, Node)): @classmethod def get_node(cls, id, context, info): try: - instance = cls._meta.model.objects.get(id=id) - return cls(instance) + return cls._meta.model.objects.get(id=id) except cls._meta.model.DoesNotExist: return None