Merge pull request #14 from graphql-python/feature/improved_key_property_behaviour

Improved conversion of ndb.KeyProperty to GraphQL

Author: ekampf

HISTORY.rst

@@ -3,7 +3,11 @@
 History
 -------
 
-0.1.4 (TBD)
+0.1.5 (2016-06-08)
+---------------------
+* Fixed behavior of ndb.KeyProperty ([PR #14](https://github.com/graphql-python/graphene-gae/pull/14))
+
+0.1.4 (2016-06-02)
 ---------------------
 * NdbConnectionField added arguments that can be used in quert:
     * keys_only - to execute a keys only query
@@ -14,6 +18,7 @@ History
       map to a Field(SomethingType) - SomethingType has to be part of the schema.
     * Support for `repeated` and `required` propeties.
 
+
 0.1.3 (2016-05-27)
 ---------------------
 * Added `graphene_gae.webapp2.GraphQLHandler` - a basic HTTP Handler to process GraphQL requests

graphene_gae/__init__.py

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

graphene_gae/ndb/converter.py

@@ -8,7 +8,7 @@
 from graphene.core.types.definitions import List
 from graphene.core.types.scalars import String, Boolean, Int, Float
 from graphene.core.types.custom_scalars import JSONString, DateTime
-from graphene_gae.ndb.fields import NdbKeyField
+from graphene_gae.ndb.fields import NdbKeyField, NdbKeyStringField
 
 __author__ = 'ekampf'
 
@@ -17,16 +17,16 @@
 p = inflect.engine()
 
 
-def convert_ndb_scalar_property(graphene_type, ndb_prop):
+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)
     if ndb_prop._repeated:
-        l = graphene_type(description=description).List
-        return l if not ndb_prop._required else l.NonNull
+        result = result.List
 
     if ndb_prop._required:
-        return graphene_type(description=description).NonNull
+        result = result.NonNull
 
-    return graphene_type(description=description)
+    return result
 
 
 def convert_ndb_string_property(ndb_prop, meta):
@@ -54,22 +54,69 @@ def convert_ndb_datetime_property(ndb_prop, meta):
 
 
 def convert_ndb_key_propety(ndb_key_prop, meta):
-    remove_key_suffix = meta.remove_key_property_suffix if meta else True
+    """
+    Two conventions for handling KeyProperties:
+    #1.
+        Given:
+            store_key = ndb.KeyProperty(...)
 
+        Result is 2 fields:
+            store_key = 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     = NdbKeyField()     -> resolves to entity
+
+    """
     name = ndb_key_prop._code_name
-    if remove_key_suffix:
-        if name.endswith('_key'):
-            name = name[:-4]
 
-        if name.endswith('_keys'):
-            name = name[:-5]
-            name = p.plural(name)
+    if name.endswith('_key') or name.endswith('_keys'):
+        # Case #1 - name is of form 'store_key' or 'store_keys'
+        string_prop_name = name
+        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'
+        resolved_prop_name = name
 
-    field = NdbKeyField(ndb_key_prop._code_name, ndb_key_prop._kind)
-    if ndb_key_prop._repeated:
-        field = field.List
+    string_field = NdbKeyStringField(name)
+    resolved_field = NdbKeyField(name, ndb_key_prop._kind)
 
-    return ConversionResult(name=name, field=field)
+    if ndb_key_prop._repeated:
+        string_field = string_field.List
+        resolved_field = resolved_field.List
+
+    if ndb_key_prop._required:
+        string_field = string_field.NonNull
+        resolved_field = resolved_field.NonNull
+
+    string_key_field_result = ConversionResult(name=string_prop_name, field=string_field)
+    resolve_key_field_result = ConversionResult(name=resolved_prop_name, field=resolved_field)
+
+    return [
+        string_key_field_result,
+        resolve_key_field_result
+    ]
+
+    # if remove_key_suffix:
+    #     if name.endswith('_key'):
+    #         name = name[:-4]
+    #
+    #     if name.endswith('_keys'):
+    #         name = name[:-5]
+    #         name = p.plural(name)
+    #
+    # field = NdbKeyField(ndb_key_prop._code_name, ndb_key_prop._kind)
+    # if ndb_key_prop._repeated:
+    #     field = field.List
+    #
+    # return ConversionResult(name=name, field=field)
 
 
 def convert_local_structured_property(ndb_structured_prop, meta):
@@ -114,7 +161,7 @@ def convert_ndb_property(prop, meta=None):
     if not result:
         raise Exception("Failed to convert NDB field %s (%s)" % (prop._code_name, prop))
 
-    if isinstance(result, ConversionResult):
+    if isinstance(result, (list, ConversionResult,)):
         return result
 
     return ConversionResult(name=prop._code_name, field=result)

graphene_gae/ndb/fields.py

@@ -4,7 +4,7 @@
 from graphene import relay
 from graphene.core.exceptions import SkipField
 from graphene.core.types.base import FieldType
-from graphene.core.types.scalars import Boolean, Int
+from graphene.core.types.scalars import Boolean, Int, String
 
 __author__ = 'ekampf'
 
@@ -89,6 +89,25 @@ def model(self):
         return self.type._meta.model
 
 
+class NdbKeyStringField(String):
+    def __init__(self, name, *args, **kwargs):
+        self.name = name
+
+        if 'resolver' not in kwargs:
+            kwargs['resolver'] = self.default_resolver
+
+        super(NdbKeyStringField, self).__init__(*args, **kwargs)
+
+    def default_resolver(self, node, args, info):
+        entity = node.instance
+        key = getattr(entity, self.name)
+
+        if isinstance(key, list):
+            return [k.urlsafe() for k in key]
+
+        return key.urlsafe() if key else None
+
+
 class NdbKeyField(FieldType):
     def __init__(self, name, kind, *args, **kwargs):
         self.name = name
@@ -107,7 +126,7 @@ def internal_type(self, schema):
                 "You can either register the type manually "
                 "using @schema.register. "
                 "Or disable the field in %s" % (
-                    self.model,
+                    self.kind,
                     self.parent,
                 )
             )
@@ -131,30 +150,7 @@ def default_resolver(self, node, args, info):
         key = getattr(entity, self.name)
 
         if isinstance(key, list):
-            return self.__auto_resolve_repeated(entity, key)
-
-        return self.__auto_resolve_key(entity, key)
-
-    def __auto_resolve_repeated(self, entity, keys):
-        if not self.name.endswith('_keys'):
-            return ndb.get_multi(keys)
-
-        cache_name = self.name[:-4]  # TODO: pluralise
-        if hasattr(entity, cache_name):
-            return getattr(entity, cache_name)
-
-        values = ndb.get_multi(keys)
-        setattr(entity, cache_name, values)
-        return values
-
-    def __auto_resolve_key(self, entity, key):
-        if not self.name.endswith('_key'):
-            return key.get()
-
-        cache_name = self.name[:-4]
-        if hasattr(entity, cache_name):
-            return getattr(entity, cache_name)
+            entities = ndb.get_multi(key)
+            return entities
 
-        value = key.get()
-        setattr(entity, cache_name, value)
-        return value
+        return key.get()

graphene_gae/ndb/options.py

@@ -11,9 +11,6 @@ class NdbOptions(ObjectTypeOptions):
     * model - which model to convert
     * only_fields - only convert the following property names
     * exclude_fields - exclude specified properties from conversion
-    * remove_key_property_suffix - remove '_key' suffix from KeyProperty
-        * user_key => user
-        * user_keys => users
 
     """
 
@@ -25,7 +22,6 @@ def __init__(self, *args, **kwargs):
         self.valid_attrs += self.VALID_ATTRS
         self.only_fields = None
         self.exclude_fields = []
-        self.remove_key_property_suffix = True
 
     def contribute_to_class(self, cls, name):
         super(NdbOptions, self).contribute_to_class(cls, name)

graphene_gae/ndb/types.py

@@ -31,8 +31,12 @@ def construct_fields(cls):
             if is_not_in_only or is_excluded:
                 continue
 
-            conversion_result = convert_ndb_property(prop, cls._meta)
-            cls.add_to_class(conversion_result.name, conversion_result.field)
+            conversion_results = convert_ndb_property(prop, cls._meta)
+            if not isinstance(conversion_results, list):
+                conversion_results = [conversion_results]
+
+            for r in conversion_results:
+                cls.add_to_class(r.name, r.field)
 
     def construct(cls, *args, **kwargs):
         super(NdbObjectTypeMeta, cls).construct(*args, **kwargs)

tests/_ndb/test_converter.py

@@ -5,7 +5,9 @@
 
 import graphene
 from graphene.core.types.custom_scalars import DateTime, JSONString
+from graphene.core.types.definitions import List, NonNull
 
+from graphene_gae.ndb.fields import NdbKeyStringField, NdbKeyField
 from graphene_gae.ndb.converter import convert_ndb_property
 
 __author__ = 'ekampf'
@@ -73,30 +75,62 @@ def testDateTimeProperty_shouldConvertToString(self):
     def testJsonProperty_shouldConvertToString(self):
         self.__assert_conversion(ndb.JsonProperty, JSONString)
 
-    def testKeyProperty_withSuffixRemoval_removesSuffix(self):
+    def testKeyProperty_withSuffix(self):
         prop = ndb.KeyProperty()
-        prop._code_name = "user_key"
+        prop._code_name = 'user_key'
 
         conversion = convert_ndb_property(prop)
-        self.assertEqual(conversion.name, "user")
 
-    def testKeyProperty_repeatedPlural_withSuffixRemoval_removesSuffixAndPluralName(self):
-        prop = ndb.KeyProperty()
-        prop._code_name = "user_keys"
-        conversion = convert_ndb_property(prop)
-        self.assertEqual(conversion.name, "users")
+        self.assertLength(conversion, 2)
+
+        self.assertEqual(conversion[0].name, 'user_key')
+        self.assertIsInstance(conversion[0].field, NdbKeyStringField)
+
+        self.assertEqual(conversion[1].name, 'user')
+        self.assertIsInstance(conversion[1].field, NdbKeyField)
+
+    def testKeyProperty_withSuffix_repeated(self):
+        prop = ndb.KeyProperty(repeated=True)
+        prop._code_name = 'user_keys'
 
-        prop = ndb.KeyProperty()
-        prop._code_name = "tag_name_keys"
         conversion = convert_ndb_property(prop)
-        self.assertEqual(conversion.name, "tag_names")
 
-        prop = ndb.KeyProperty()
-        prop._code_name = "person_keys"
+        self.assertLength(conversion, 2)
+
+        self.assertEqual(conversion[0].name, 'user_keys')
+        self.assertIsInstance(conversion[0].field, List)
+        self.assertIsInstance(conversion[0].field.of_type, NdbKeyStringField)
+
+        self.assertEqual(conversion[1].name, 'users')
+        self.assertIsInstance(conversion[1].field, List)
+        self.assertIsInstance(conversion[1].field.of_type, NdbKeyField)
+
+    def testKeyProperty_withSuffix_required(self):
+        prop = ndb.KeyProperty(required=True)
+        prop._code_name = 'user_key'
+
         conversion = convert_ndb_property(prop)
-        self.assertEqual(conversion.name, "people")
 
+        self.assertLength(conversion, 2)
+
+        self.assertEqual(conversion[0].name, 'user_key')
+        self.assertIsInstance(conversion[0].field, NonNull)
+        self.assertIsInstance(conversion[0].field.of_type, NdbKeyStringField)
+
+        self.assertEqual(conversion[1].name, 'user')
+        self.assertIsInstance(conversion[1].field, NonNull)
+        self.assertIsInstance(conversion[1].field.of_type, NdbKeyField)
+
+    def testKeyProperty_withoutSuffix(self):
         prop = ndb.KeyProperty()
-        prop._code_name = "universal_category_keys"
+        prop._code_name = 'user'
+
         conversion = convert_ndb_property(prop)
-        self.assertEqual(conversion.name, "universal_categories")
+
+        self.assertLength(conversion, 2)
+
+        self.assertEqual(conversion[0].name, 'user_key')
+        self.assertIsInstance(conversion[0].field, NdbKeyStringField)
+
+        self.assertEqual(conversion[1].name, 'user')
+        self.assertIsInstance(conversion[1].field, NdbKeyField)