From eadd63d0963bf6153384cb37dfc8c5896b0be2de Mon Sep 17 00:00:00 2001 From: Syrus Akbary Date: Mon, 20 Feb 2017 00:47:54 -0800 Subject: [PATCH] Improved registry, raising only when constructing the schema. --- docs/registry.rst | 2 +- graphene_django/converter.py | 8 +++--- graphene_django/registry.py | 40 ++++++++++++++++---------- graphene_django/tests/test_registry.py | 27 ++++++++++------- 4 files changed, 46 insertions(+), 31 deletions(-) diff --git a/docs/registry.rst b/docs/registry.rst index da42b35..9c4b56f 100644 --- a/docs/registry.rst +++ b/docs/registry.rst @@ -25,7 +25,7 @@ You retrieve using the function ``get_global_registry`` in model = ReporterModel global_registry = get_global_registry - global_registry.get_type_for_model(ReporterModel) # == Reporter + global_registry.get_unique_type_for_model(ReporterModel) # == Reporter Multiple types for one model diff --git a/graphene_django/converter.py b/graphene_django/converter.py index 92812d1..74f5944 100644 --- a/graphene_django/converter.py +++ b/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 diff --git a/graphene_django/registry.py b/graphene_django/registry.py index a2021ff..b4b8806 100644 --- a/graphene_django/registry.py +++ b/graphene_django/registry.py @@ -1,30 +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 self._registry.get(model, cls) == cls, ( - 'Django Model "{}.{}" already associated with {}. ' - 'You can use a different registry for {} or skip ' - 'the global Registry with "{}.Meta.skip_global_registry = True".' - ).format( - model._meta.app_label, - model._meta.object_name, - repr(self.get_type_for_model(cls._meta.model)), - repr(cls), - cls - ) 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.' - 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) diff --git a/graphene_django/tests/test_registry.py b/graphene_django/tests/test_registry.py index 6527b32..eece4cf 100644 --- a/graphene_django/tests/test_registry.py +++ b/graphene_django/tests/test_registry.py @@ -18,29 +18,34 @@ def test_registry_basic(): model = ReporterModel assert Reporter._meta.registry == global_registry - assert global_registry.get_type_for_model(ReporterModel) == Reporter + 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(object): - pass + 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: - class Reporter2(DjangoObjectType): - '''Reporter2 description''' - class Meta: - model = ReporterModel + global_registry.get_unique_type_for_model(ReporterModel) == [Reporter, Reporter2] assert str(exc_info.value) == ( - 'Django Model "tests.Reporter" already associated with {}. ' - 'You can use a different registry for {} ' - 'or skip the global Registry with "Reporter2.Meta.skip_global_registry = True".' - ).format(repr(Reporter), repr(Reporter2)) + '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():