Merge pull request #16 from graphql-python/feature/fix_key_property_behavior

Breaking change: ndb KeyProperties should be externalised as Global IDs

Author: ekampf

HISTORY.rst

@@ -3,6 +3,10 @@
 History
 -------
 
+0.1.7 (TBD)
+---------------------
+
+
 0.1.6 (2016-06-10)
 ---------------------
 * Changing development status to Beta

graphene_gae/__init__.py

@@ -12,7 +12,7 @@
 )
 
 __author__ = 'Eran Kampf'
-__version__ = '0.1.6'
+__version__ = '0.1.7'
 
 __all__ = [
     NdbObjectType,

graphene_gae/ndb/converter.py

@@ -17,6 +17,11 @@
 p = inflect.engine()
 
 
+def rreplace(s, old, new, occurrence):
+    li = s.rsplit(old, occurrence)
+    return new.join(li)
+
+
 def convert_ndb_scalar_property(graphene_type, ndb_prop, **kwargs):
     description = "%s %s property" % (ndb_prop._name, graphene_type)
     result = graphene_type(description=description, **kwargs)
@@ -61,31 +66,31 @@ def convert_ndb_key_propety(ndb_key_prop, meta):
             store_key = ndb.KeyProperty(...)
 
         Result is 2 fields:
-            store_key = graphene.String() -> resolves to store_key.urlsafe()
+            store_id  = graphene.String() -> resolves to store_key.urlsafe()
             store     = NdbKeyField()     -> resolves to entity
 
     #2.
         Given:
             store = ndb.KeyProperty(...)
 
         Result is 2 fields:
-            store_key = graphene.String() -> resolves to store_key.urlsafe()
+            store_id = graphene.String() -> resolves to store_key.urlsafe()
             store     = NdbKeyField()     -> resolves to entity
 
     """
     name = ndb_key_prop._code_name
 
     if name.endswith('_key') or name.endswith('_keys'):
         # Case #1 - name is of form 'store_key' or 'store_keys'
-        string_prop_name = name
+        string_prop_name = rreplace(name, '_key', '_id', 1)
         resolved_prop_name = name[:-4] if name.endswith('_key') else p.plural(name[:-5])
     else:
         # Case #2 - name is of form 'store'
         singular_name = p.singular_noun(name) if p.singular_noun(name) else name
-        string_prop_name = singular_name + '_keys' if ndb_key_prop._repeated else singular_name + '_key'
+        string_prop_name = singular_name + '_ids' if ndb_key_prop._repeated else singular_name + '_id'
         resolved_prop_name = name
 
-    string_field = NdbKeyStringField(name)
+    string_field = NdbKeyStringField(name, ndb_key_prop._kind)
     resolved_field = NdbKeyField(name, ndb_key_prop._kind)
 
     if ndb_key_prop._repeated:
@@ -105,7 +110,6 @@ def convert_ndb_key_propety(ndb_key_prop, meta):
     ]
 
 
-
 def convert_local_structured_property(ndb_structured_prop, meta):
     is_required = ndb_structured_prop._required
     is_repeated = ndb_structured_prop._repeated

graphene_gae/ndb/fields.py

@@ -5,6 +5,7 @@
 from graphene.core.exceptions import SkipField
 from graphene.core.types.base import FieldType
 from graphene.core.types.scalars import Boolean, Int, String
+from graphql_relay import to_global_id
 
 __author__ = 'ekampf'
 
@@ -90,22 +91,55 @@ def model(self):
 
 
 class NdbKeyStringField(String):
-    def __init__(self, name, *args, **kwargs):
+    def __init__(self, name, kind, *args, **kwargs):
         self.name = name
+        self.kind = kind
 
         if 'resolver' not in kwargs:
             kwargs['resolver'] = self.default_resolver
 
         super(NdbKeyStringField, self).__init__(*args, **kwargs)
 
+    def internal_type(self, schema):
+        _type = self.get_object_type(schema)
+        if not _type and self.parent._meta.only_fields:
+            raise Exception(
+                "Model %r is not accessible by the schema. "
+                "You can either register the type manually "
+                "using @schema.register. "
+                "Or disable the field in %s" % (
+                    self.kind,
+                    self.parent,
+                )
+            )
+
+        if not _type:
+            raise SkipField()
+
+        from graphql import GraphQLString
+        return GraphQLString
+
+    def get_object_type(self, schema):
+        for _type in schema.types.values():
+            type_model = hasattr(_type, '_meta') and getattr(_type._meta, 'model', None)
+            if not type_model:
+                continue
+
+            if self.kind == type_model or self.kind == type_model.__name__:
+                return _type
+
     def default_resolver(self, node, args, info):
         entity = node.instance
         key = getattr(entity, self.name)
+        if not key:
+            return None
 
         if isinstance(key, list):
-            return [k.urlsafe() for k in key]
+            t = self.get_object_type(info.schema.graphene_schema)._meta.type_name
+            return [to_global_id(t, k.urlsafe()) for k in key]
 
-        return key.urlsafe() if key else None
+        t = self.get_object_type(info.schema.graphene_schema)._meta.type_name
+        return to_global_id(t, key.urlsafe()) if key else None
 
 
 class NdbKeyField(FieldType):

tests/_ndb/test_converter.py

@@ -83,7 +83,7 @@ def testKeyProperty_withSuffix(self):
 
         self.assertLength(conversion, 2)
 
-        self.assertEqual(conversion[0].name, 'user_key')
+        self.assertEqual(conversion[0].name, 'user_id')
         self.assertIsInstance(conversion[0].field, NdbKeyStringField)
 
         self.assertEqual(conversion[1].name, 'user')
@@ -97,7 +97,7 @@ def testKeyProperty_withSuffix_repeated(self):
 
         self.assertLength(conversion, 2)
 
-        self.assertEqual(conversion[0].name, 'user_keys')
+        self.assertEqual(conversion[0].name, 'user_ids')
         self.assertIsInstance(conversion[0].field, List)
         self.assertIsInstance(conversion[0].field.of_type, NdbKeyStringField)
 
@@ -113,7 +113,7 @@ def testKeyProperty_withSuffix_required(self):
 
         self.assertLength(conversion, 2)
 
-        self.assertEqual(conversion[0].name, 'user_key')
+        self.assertEqual(conversion[0].name, 'user_id')
         self.assertIsInstance(conversion[0].field, NonNull)
         self.assertIsInstance(conversion[0].field.of_type, NdbKeyStringField)
 
@@ -129,7 +129,7 @@ def testKeyProperty_withoutSuffix(self):
 
         self.assertLength(conversion, 2)
 
-        self.assertEqual(conversion[0].name, 'user_key')
+        self.assertEqual(conversion[0].name, 'user_id')
         self.assertIsInstance(conversion[0].field, NdbKeyStringField)
 
         self.assertEqual(conversion[1].name, 'user')

tests/_ndb/test_types.py

@@ -1,3 +1,5 @@
+from graphene_gae.ndb.fields import NdbKeyStringField
+from graphql_relay import to_global_id
 from tests.base_test import BaseTest
 
 import graphene
@@ -103,6 +105,27 @@ def resolve_articles(self):
 
         self.assertIn("Model 'bar' is not accessible by the schema.", str(context.exception.message))
 
+    def testNdbObjectType_keyProperty_stringRepresentation_kindDoesntExist_raisesException(self):
+        with self.assertRaises(Exception) as context:
+            class ArticleType(NdbObjectType):
+                class Meta:
+                    model = Article
+                    only_fields = ('prop',)
+
+                prop = NdbKeyStringField('foo', 'bar')
+
+            class QueryType(graphene.ObjectType):
+                articles = graphene.List(ArticleType)
+
+                @graphene.resolve_only_args
+                def resolve_articles(self):
+                    return Article.query()
+
+            schema = graphene.Schema(query=QueryType)
+            schema.execute('query test {  articles { prop } }')
+
+        self.assertIn("Model 'bar' is not accessible by the schema.", str(context.exception.message))
+
     def testQuery_excludedField(self):
         Article(headline="h1", summary="s1").put()
 
@@ -280,7 +303,7 @@ def testQuery_keyProperty(self):
             query ArticleWithAuthorID {
                 articles {
                     headline
-                    authorKey
+                    authorId
                     author {
                         name, email
                     }
@@ -293,7 +316,8 @@ def testQuery_keyProperty(self):
         article = dict(result.data['articles'][0])
         author = dict(article['author'])
         self.assertDictEqual(author, {'name': u'john dow', 'email': u'[email protected]'})
-        self.assertDictContainsSubset(dict(headline='h1', authorKey=author_key.urlsafe()), article)
+        self.assertEqual('h1', article['headline'])
+        self.assertEqual(to_global_id('AuthorType', author_key.urlsafe()), article['authorId'])
 
     def testQuery_repeatedKeyProperty(self):
         tk1 = Tag(name="t1").put()
@@ -302,14 +326,12 @@ def testQuery_repeatedKeyProperty(self):
         tk4 = Tag(name="t4").put()
         Article(headline="h1", summary="s1", tags=[tk1, tk2, tk3, tk4]).put()
 
-        print str(schema)
-
         result = schema.execute('''
             query ArticleWithAuthorID {
                 articles {
                     headline
-                    authorKey
-                    tagKeys
+                    authorId
+                    tagIds
                     tags {
                         name
                     }
@@ -320,7 +342,7 @@ def testQuery_repeatedKeyProperty(self):
         self.assertEmpty(result.errors)
 
         article = dict(result.data['articles'][0])
-        self.assertListEqual(map(lambda k: k.urlsafe(), [tk1, tk2, tk3, tk4]), article['tagKeys'])
+        self.assertListEqual(map(lambda k: to_global_id('TagType', k.urlsafe()), [tk1, tk2, tk3, tk4]), article['tagIds'])
 
         self.assertLength(article['tags'], 4)
         for i in range(0, 3):

tests/base_test.py

@@ -61,7 +61,7 @@ def get_filtered_tasks(self, url=None, name=None, queue_names=None):
 
     # region Extra Assertions
     def assertEmpty(self, l, msg=None):
-        self.assertEqual(0, len(list(l)), msg=msg)
+        self.assertEqual(0, len(list(l)), msg=msg or str(l))
 
     def assertLength(self, l, expectation, msg=None):
         self.assertEqual(len(l), expectation, msg)