Merge pull request #115 from graphql-python/features/global-registry

Improved the global registry

docs/index.rst

@@ -11,3 +11,4 @@ Contents:
    authorization
    debug
    introspection
+   registry

docs/registry.rst

@@ -0,0 +1,60 @@
+Graphene-Django Registry
+========================
+
+Graphene-Django uses a Registry to keep track of all the Django Models
+and the ``DjangoObjectTypes`` associated to them.
+
+This way, we make the library smart enough to convert automatically the 
+relations between models to Graphene fields automatically (when possible).
+
+
+Global registry
+---------------
+
+By default, all model/objecttype relations will live in the global registry.
+You retrieve using the function ``get_global_registry`` in
+``graphene_django.registry``.
+
+.. code:: python
+
+    from graphene_django.registry get_global_registry
+
+    class Reporter(DjangoObjectType):
+        '''Reporter description'''
+        class Meta:
+            model = ReporterModel
+
+    global_registry = get_global_registry
+    global_registry.get_unique_type_for_model(ReporterModel) # == Reporter
+
+
+Multiple types for one model
+----------------------------
+
+There will be some cases where we need one Django Model to
+have multiple graphene ``ObjectType``s associated to it.
+
+In this case, we can either use ``skip_global_registry`` to create
+a new isolated registry for that type (so it doesn't interfere with
+the global registry), or we can create a custom registry for it.
+
+.. code:: python
+
+    from graphene_django.registry import Registry
+
+    class Reporter(DjangoObjectType):
+        '''Reporter description'''
+        class Meta:
+            model = ReporterModel
+
+    class Reporter2(DjangoObjectType):
+        '''Reporter2 description'''
+        class Meta:
+            model = ReporterModel
+            skip_global_registry = True
+            # We can also specify a custom registry with
+            # registry = Registry()
+
+
+This way, the ``ReporterModel`` could have two different types living in the same
+schema.

graphene_django/converter.py

@@ -126,7 +126,7 @@ def convert_onetoone_field_to_djangomodel(field, registry=None):
     model = get_related_model(field)
 
     def dynamic_type():
-        _type = registry.get_type_for_model(model)
+        _type = registry.get_unique_type_for_model(model)
         if not _type:
             return
 
@@ -145,7 +145,7 @@ def convert_field_to_list_or_connection(field, registry=None):
     model = get_related_model(field)
 
     def dynamic_type():
-        _type = registry.get_type_for_model(model)
+        _type = registry.get_unique_type_for_model(model)
         if not _type:
             return
 
@@ -163,7 +163,7 @@ def convert_relatedfield_to_djangomodel(field, registry=None):
     model = field.model
 
     def dynamic_type():
-        _type = registry.get_type_for_model(model)
+        _type = registry.get_unique_type_for_model(model)
         if not _type:
             return
 
@@ -183,7 +183,7 @@ def convert_field_to_djangomodel(field, registry=None):
     model = get_related_model(field)
 
     def dynamic_type():
-        _type = registry.get_type_for_model(model)
+        _type = registry.get_unique_type_for_model(model)
         if not _type:
             return
 

graphene_django/debug/tests/test_query.py

@@ -6,10 +6,15 @@ from graphene_django import DjangoConnectionField, DjangoObjectType
 from graphene_django.utils import DJANGO_FILTER_INSTALLED
 
 from ...tests.models import Reporter
+from ...registry import reset_global_registry
 from ..middleware import DjangoDebugMiddleware
 from ..types import DjangoDebug
 
 
+def setup_function(function):
+    reset_global_registry()
+
+
 class context(object):
     pass
 

graphene_django/filter/tests/test_fields.py

@@ -9,6 +9,7 @@ from graphene_django.forms import (GlobalIDFormField,
                                    GlobalIDMultipleChoiceField)
 from graphene_django.tests.models import Article, Pet, Reporter
 from graphene_django.utils import DJANGO_FILTER_INSTALLED
+from graphene_django.registry import Registry, reset_global_registry
 
 pytestmark = []
 
@@ -24,6 +25,8 @@ pytestmark.append(pytest.mark.django_db)
 
 
 if DJANGO_FILTER_INSTALLED:
+    reset_global_registry()
+
     class ArticleNode(DjangoObjectType):
 
         class Meta:
@@ -47,6 +50,10 @@ if DJANGO_FILTER_INSTALLED:
 
     # schema = Schema()
 
+@pytest.fixture
+def _registry():
+    return Registry()
+
 
 def get_args(field):
     return field.args
@@ -134,26 +141,28 @@ def test_filter_shortcut_filterset_extra_meta():
     assert 'headline' not in field.filterset_class.get_fields()
 
 
-def test_filter_filterset_information_on_meta():
+def test_filter_filterset_information_on_meta(_registry):
     class ReporterFilterNode(DjangoObjectType):
 
         class Meta:
             model = Reporter
             interfaces = (Node, )
             filter_fields = ['first_name', 'articles']
+            registry = _registry
 
     field = DjangoFilterConnectionField(ReporterFilterNode)
     assert_arguments(field, 'first_name', 'articles')
     assert_not_orderable(field)
 
 
-def test_filter_filterset_information_on_meta_related():
+def test_filter_filterset_information_on_meta_related(_registry):
     class ReporterFilterNode(DjangoObjectType):
 
         class Meta:
             model = Reporter
             interfaces = (Node, )
             filter_fields = ['first_name', 'articles']
+            registry = _registry
 
     class ArticleFilterNode(DjangoObjectType):
 
@@ -161,6 +170,7 @@ def test_filter_filterset_information_on_meta_related():
             model = Article
             interfaces = (Node, )
             filter_fields = ['headline', 'reporter']
+            registry = _registry
 
     class Query(ObjectType):
         all_reporters = DjangoFilterConnectionField(ReporterFilterNode)
@@ -174,13 +184,14 @@ def test_filter_filterset_information_on_meta_related():
     assert_not_orderable(articles_field)
 
 
-def test_filter_filterset_related_results():
+def test_filter_filterset_related_results(_registry):
     class ReporterFilterNode(DjangoObjectType):
 
         class Meta:
             model = Reporter
             interfaces = (Node, )
             filter_fields = ['first_name', 'articles']
+            registry = _registry
 
     class ArticleFilterNode(DjangoObjectType):
 
@@ -188,6 +199,7 @@ def test_filter_filterset_related_results():
             interfaces = (Node, )
             model = Article
             filter_fields = ['headline', 'reporter']
+            registry = _registry
 
     class Query(ObjectType):
         all_reporters = DjangoFilterConnectionField(ReporterFilterNode)
@@ -315,7 +327,7 @@ def test_global_id_multiple_field_explicit_reverse():
     assert multiple_filter.field_class == GlobalIDMultipleChoiceField
 
 
-def test_filter_filterset_related_results():
+def test_filter_filterset_related_results(_registry):
     class ReporterFilterNode(DjangoObjectType):
 
         class Meta:
@@ -324,6 +336,7 @@ def test_filter_filterset_related_results():
             filter_fields = {
                 'first_name': ['icontains']
             }
+            registry = _registry
 
     class Query(ObjectType):
         all_reporters = DjangoFilterConnectionField(ReporterFilterNode)

graphene_django/registry.py

@@ -1,22 +1,40 @@
+from collections import defaultdict
+
+
 class Registry(object):
 
     def __init__(self):
-        self._registry = {}
-        self._registry_models = {}
+        self._registry = defaultdict(list)
 
     def register(self, cls):
         from .types import DjangoObjectType
+        model = cls._meta.model
         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)
-        # )
-        if not getattr(cls._meta, 'skip_registry', False):
-            self._registry[cls._meta.model] = cls
+        self._registry[model].append(cls)
 
-    def get_type_for_model(self, model):
+    def get_unique_type_for_model(self, model):
+        types = self.get_types_for_model(model)
+        if not types:
+            return None
+
+        # If there is more than one type for the model, we should
+        # raise an error so both types don't collide in the same schema.
+        assert len(types) == 1, (
+            'Found multiple ObjectTypes associated with the same Django Model "{}.{}": {}. '
+            'You can use a different registry for each or skip '
+            'the global Registry with Meta.skip_global_registry = True". '
+            'Read more at http://docs.graphene-python.org/projects/django/en/latest/registry/ .'
+        ).format(
+            model._meta.app_label,
+            model._meta.object_name,
+            repr(types),
+        )
+        return types[0]
+
+    def get_types_for_model(self, model):
         return self._registry.get(model)
 
 

graphene_django/tests/test_converter.py

@@ -11,12 +11,13 @@ from graphene.types.json import JSONString
 from ..compat import (ArrayField, HStoreField, JSONField, MissingType,
                       RangeField, UUIDField, DurationField)
 from ..converter import convert_django_field, convert_django_field_with_choices
-from ..registry import Registry
+from ..registry import Registry, reset_global_registry
 from ..types import DjangoObjectType
 from .models import Article, Film, FilmDetails, Reporter
 
 
-# from graphene.core.types.custom_scalars import DateTime, Time, JSONString
+def setup_function(function):
+  reset_global_registry()
 
 
 def assert_conversion(django_field, graphene_field, *args, **kwargs):

graphene_django/tests/test_query.py

@@ -12,11 +12,16 @@ from ..utils import DJANGO_FILTER_INSTALLED
 from ..compat import MissingType, RangeField
 from ..fields import DjangoConnectionField
 from ..types import DjangoObjectType
+from ..registry import reset_global_registry
 from .models import Article, Reporter
 
 pytestmark = pytest.mark.django_db
 
 
+def setup_function(function):
+    reset_global_registry()
+
+
 def test_should_query_only_fields():
     with raises(Exception):
         class ReporterType(DjangoObjectType):

graphene_django/tests/test_registry.py

@@ -0,0 +1,64 @@
+from pytest import raises
+
+from ..registry import Registry, get_global_registry, reset_global_registry
+from ..types import DjangoObjectType
+from .models import Reporter as ReporterModel
+
+
+def setup_function(function):
+  reset_global_registry()
+
+
+def test_registry_basic():
+    global_registry = get_global_registry()
+    
+    class Reporter(DjangoObjectType):
+        '''Reporter description'''
+        class Meta:
+            model = ReporterModel
+
+    assert Reporter._meta.registry == global_registry
+    assert global_registry.get_unique_type_for_model(ReporterModel) == Reporter
+
+
+def test_registry_multiple_types():
+    global_registry = get_global_registry()
+
+    class Reporter(DjangoObjectType):
+        '''Reporter description'''
+        class Meta:
+            model = ReporterModel
+
+    class Reporter2(DjangoObjectType):
+        '''Reporter2 description'''
+        class Meta:
+            model = ReporterModel
+
+    assert global_registry.get_types_for_model(ReporterModel) == [Reporter, Reporter2]
+
+    with raises(Exception) as exc_info:
+        global_registry.get_unique_type_for_model(ReporterModel) == [Reporter, Reporter2]
+
+    assert str(exc_info.value) == (
+       'Found multiple ObjectTypes associated with the same '
+       'Django Model "tests.Reporter": {}. You can use a different '
+       'registry for each or skip the global Registry with '
+       'Meta.skip_global_registry = True". '
+       'Read more at http://docs.graphene-python.org/projects/django/en/latest/registry/ .'
+    ).format(repr([Reporter, Reporter2]))
+
+
+def test_registry_multiple_types_dont_collision_if_skip_global_registry():
+    class Reporter(DjangoObjectType):
+        '''Reporter description'''
+        class Meta:
+            model = ReporterModel
+
+    class Reporter2(DjangoObjectType):
+        '''Reporter2 description'''
+        class Meta:
+            model = ReporterModel
+            skip_global_registry = True
+
+    assert Reporter._meta.registry != Reporter2._meta.registry
+    assert Reporter2._meta.registry != get_global_registry()

graphene_django/tests/test_schema.py

@@ -1,10 +1,14 @@
 from py.test import raises
 
-from ..registry import Registry
+from ..registry import Registry, reset_global_registry
 from ..types import DjangoObjectType
 from .models import Reporter
 
 
+def setup_function(function):
+    reset_global_registry()
+
+
 def test_should_raise_if_no_model():
     with raises(Exception) as excinfo:
         class Character1(DjangoObjectType):

graphene_django/tests/test_types.py

@@ -3,11 +3,12 @@ from mock import patch
 from graphene import Interface, ObjectType, Schema
 from graphene.relay import Node
 
-from ..registry import reset_global_registry
+from ..registry import Registry, reset_global_registry
 from ..types import DjangoObjectType
 from .models import Article as ArticleModel
 from .models import Reporter as ReporterModel
 
+
 reset_global_registry()
 
 

graphene_django/types.py

@@ -58,7 +58,7 @@ class DjangoObjectTypeMeta(ObjectTypeMeta):
             only_fields=(),
             exclude_fields=(),
             interfaces=(),
-            skip_registry=False,
+            skip_global_registry=False,
             registry=None
         )
         if DJANGO_FILTER_INSTALLED:
@@ -72,6 +72,14 @@ class DjangoObjectTypeMeta(ObjectTypeMeta):
             attrs.pop('Meta', None),
             **defaults
         )
+        # If the DjangoObjectType wants to skip the global registry
+        # we will automatically create one, so the model is isolated
+        # there.
+        if options.skip_global_registry:
+            assert not options.registry, (
+                "The attribute skip_global_registry requires have an empty registry in {}.Meta"
+            ).format(name)
+            options.registry = Registry()
         if not options.registry:
             options.registry = get_global_registry()
         assert isinstance(options.registry, Registry), (