Simplified how NodeIDField is managed. Improved tests

graphene/contrib/django/options.py

@@ -24,6 +24,8 @@ class DjangoOptions(Options):
 
     def contribute_to_class(self, cls, name):
         super(DjangoOptions, self).contribute_to_class(cls, name)
+        if is_node(cls):
+            self.exclude_fields += ['id']
         if not is_node(cls) and not is_base(cls):
             return
         if not self.model:

graphene/contrib/django/types.py

@@ -6,6 +6,7 @@ from graphene.contrib.django.options import DjangoOptions
 from graphene.contrib.django.converter import convert_django_field
 
 from graphene.relay.types import Node, BaseNode
+from graphene.relay.fields import NodeIDField
 
 
 def get_reverse_fields(model):
@@ -50,6 +51,8 @@ class DjangoInterface(six.with_metaclass(DjangoObjectTypeMeta, BaseObjectType)):
 
 
 class DjangoNode(BaseNode, DjangoInterface):
+    id = NodeIDField()
+
     @classmethod
     def get_node(cls, id):
         instance = cls._meta.model.objects.filter(id=id).first()

graphene/core/fields.py

@@ -25,11 +25,12 @@ class Empty(object):
 class Field(object):
     SKIP = GraphQLSkipField
     creation_counter = 0
+    required = False
 
     def __init__(self, field_type, name=None, resolve=None, required=False, args=None, description='', **extra_args):
         self.field_type = field_type
         self.resolve_fn = resolve
-        self.required = required
+        self.required = self.required or required
         self.args = args or {}
         self.extra_args = extra_args
         self._type = None
@@ -39,13 +40,14 @@ class Field(object):
         self.creation_counter = Field.creation_counter
         Field.creation_counter += 1
 
-    def contribute_to_class(self, cls, name):
+    def contribute_to_class(self, cls, name, add=True):
         if not self.name:
             self.name = to_camel_case(name)
         self.field_name = name
         self.object_type = cls
         if isinstance(self.field_type, Field) and not self.field_type.object_type:
-            self.field_type.contribute_to_class(cls, name)
+            self.field_type.contribute_to_class(cls, name, False)
+        if add:
             cls._meta.add_field(self)
 
     def resolve(self, instance, args, info):

graphene/core/types.py

@@ -75,13 +75,13 @@ class ObjectTypeMeta(type):
             for field in parent_fields:
                 if field.name in field_names and field.__class__ != field_names[field.name].__class__:
                     raise Exception(
-                        'Local field %r in class %r clashes '
+                        'Local field %r in class %r (%r) clashes '
                         'with field with similar name from '
-                        'Interface %s (%r != %r)' % (
+                        'Interface %s (%r)' % (
                             field.name,
                             new_class.__name__,
-                            base.__name__,
                             field.__class__,
+                            base.__name__,
                             field_names[field.name].__class__)
                     )
                 new_field = copy.copy(field)
@@ -99,6 +99,8 @@ class ObjectTypeMeta(type):
         pass
 
     def _prepare(cls):
+        if hasattr(cls, '_prepare_class'):
+            cls._prepare_class()
         signals.class_prepared.send(cls)
 
     def add_to_class(cls, name, value):

graphene/relay/__init__.py

@@ -3,8 +3,6 @@ from graphene.relay.fields import (
     NodeField
 )
 
-import graphene.relay.connections
-
 from graphene.relay.types import (
     Node
 )

graphene/relay/connections.py

@@ -1,17 +0,0 @@
-from graphql_relay.node.node import (
-    global_id_field
-)
-
-from graphene import signals
-from graphene.relay.fields import NodeIDField
-from graphene.relay.utils import is_node
-
-
-@signals.class_prepared.connect
-def object_type_created(object_type):
-    if is_node(object_type):
-        type_name = object_type._meta.type_name
-        field = NodeIDField()
-        object_type.add_to_class('id', field)
-        assert hasattr(
-            object_type, 'get_node'), 'get_node classmethod not found in %s Node' % type_name

graphene/relay/fields.py

@@ -8,10 +8,11 @@ from graphql_relay.connection.connection import (
 )
 from graphql_relay.node.node import (
     global_id_field,
+    to_global_id,
     from_global_id
 )
 
-from graphene.core.fields import Field, LazyNativeField, LazyField
+from graphene.core.fields import Field, LazyNativeField, IDField
 from graphene.utils import cached_property
 from graphene.utils import memoize
 
@@ -70,6 +71,9 @@ class NodeField(LazyNativeField):
         return BaseNode.get_definitions(schema).node_field
 
 
-class NodeIDField(LazyNativeField):
-    def get_field(self, schema):
-        return global_id_field(self.object_type._meta.type_name)
+class NodeIDField(IDField):
+    required = True
+
+    def resolve(self, instance, args, info):
+        type_name = self.object_type._meta.type_name
+        return to_global_id(type_name, instance.id)

graphene/relay/types.py

@@ -8,6 +8,7 @@ from graphql_relay.connection.connection import (
 
 from graphene.core.types import Interface
 from graphene.core.fields import LazyNativeField
+from graphene.relay.fields import NodeIDField
 from graphene.utils import memoize
 
 
@@ -47,6 +48,13 @@ class BaseNode(object):
             return BaseNode.get_definitions(schema).node_interface
         return super(BaseNode, cls).internal_type(schema)
 
+    @classmethod
+    def _prepare_class(cls):
+        from graphene.relay.utils import is_node
+        if is_node(cls):
+            assert hasattr(
+                cls, 'get_node'), 'get_node classmethod not found in %s Node' % cls
+
 
 class Node(BaseNode, Interface):
-    pass
+    id = NodeIDField()

tests/contrib_django/test_types.py

@@ -1,6 +1,9 @@
 from py.test import raises
 from collections import namedtuple
 from pytest import raises
+from graphene.relay.fields import (
+    NodeIDField
+)
 from graphene.core.fields import (
     Field,
     StringField,
@@ -56,6 +59,16 @@ def test_pseudo_interface():
     )
 
 
+def test_djangonode_idfield():
+    idfield = DjangoNode._meta.fields_map['id']
+    assert isinstance(idfield, NodeIDField)
+
+
+def test_node_idfield():
+    idfield = Human._meta.fields_map['id']
+    assert isinstance(idfield, NodeIDField)
+
+
 def test_interface_resolve_type():
     resolve_type = Character.resolve_type(schema, Human(object()))
     assert isinstance(resolve_type, GraphQLObjectType)

tests/relay/test_relayfields.py

@@ -1,4 +1,8 @@
 from pytest import raises
+from graphql.core.type import (
+    GraphQLNonNull,
+    GraphQLID
+)
 
 import graphene
 from graphene import relay
@@ -41,3 +45,9 @@ def test_nodefield_query():
     result = schema.execute(query)
     assert not result.errors
     assert result.data == expected
+
+
+def test_nodeidfield():
+    id_field = MyNode._meta.fields_map['id']
+    assert isinstance(id_field.internal_field(schema).type, GraphQLNonNull)
+    assert id_field.internal_field(schema).type.of_type == GraphQLID