Merge pull request #256 from kensho-technologies/switch-to-grapqhl-ty…

…pe-system

Make the SchemaGraph use GraphQL's type system

graphql_compiler/schema.py

@@ -267,6 +267,14 @@ def _parse_datetime_value(value):
     parse_literal=_unused_function,  # We don't yet support parsing Decimal objects in literals.
 )
 
+GraphQLAny = GraphQLScalarType(
+    name='Any',
+    description='The `Any` scalar type is used to represent a scalar type not representable '
+                'by any other GraphQLScalar type.',
+    serialize=str,
+    parse_value=_unused_function,
+    parse_literal=_unused_function,
+)
 
 DIRECTIVES = (
     FilterDirective,

graphql_compiler/schema_generation/graphql_schema.py

@@ -3,21 +3,14 @@
 from itertools import chain
 
 from graphql.type import (
-    GraphQLBoolean, GraphQLField, GraphQLFloat, GraphQLInt, GraphQLInterfaceType, GraphQLList,
-    GraphQLObjectType, GraphQLSchema, GraphQLString, GraphQLUnionType
+    GraphQLField, GraphQLInterfaceType, GraphQLList, GraphQLObjectType, GraphQLSchema,
+    GraphQLUnionType
 )
 import six
 
-from ..schema import (
-    DIRECTIVES, EXTENDED_META_FIELD_DEFINITIONS, GraphQLDate, GraphQLDateTime, GraphQLDecimal
-)
+from ..schema import DIRECTIVES, EXTENDED_META_FIELD_DEFINITIONS
 from .exceptions import EmptySchemaError
-from .schema_properties import (
-    ORIENTDB_BASE_VERTEX_CLASS_NAME, PROPERTY_TYPE_BOOLEAN_ID, PROPERTY_TYPE_DATE_ID,
-    PROPERTY_TYPE_DATETIME_ID, PROPERTY_TYPE_DECIMAL_ID, PROPERTY_TYPE_DOUBLE_ID,
-    PROPERTY_TYPE_EMBEDDED_LIST_ID, PROPERTY_TYPE_EMBEDDED_SET_ID, PROPERTY_TYPE_FLOAT_ID,
-    PROPERTY_TYPE_INTEGER_ID, PROPERTY_TYPE_STRING_ID
-)
+from .schema_properties import ORIENTDB_BASE_VERTEX_CLASS_NAME
 
 
 def _get_referenced_type_equivalences(graphql_types, type_equivalence_hints):
@@ -60,41 +53,6 @@ def _validate_overriden_fields_are_not_defined_in_superclasses(class_to_field_ty
                             .format(field_name, class_name, superclass_name))
 
 
-def _property_descriptor_to_graphql_type(property_obj):
-    """Return the best GraphQL type representation for an OrientDB property descriptor."""
-    property_type = property_obj.type_id
-    scalar_types = {
-        PROPERTY_TYPE_BOOLEAN_ID: GraphQLBoolean,
-        PROPERTY_TYPE_DATE_ID: GraphQLDate,
-        PROPERTY_TYPE_DATETIME_ID: GraphQLDateTime,
-        PROPERTY_TYPE_DECIMAL_ID: GraphQLDecimal,
-        PROPERTY_TYPE_DOUBLE_ID: GraphQLFloat,
-        PROPERTY_TYPE_FLOAT_ID: GraphQLFloat,
-        PROPERTY_TYPE_INTEGER_ID: GraphQLInt,
-        PROPERTY_TYPE_STRING_ID: GraphQLString,
-    }
-
-    result = scalar_types.get(property_type, None)
-    if result:
-        return result
-
-    mapping_types = {
-        PROPERTY_TYPE_EMBEDDED_SET_ID: GraphQLList,
-        PROPERTY_TYPE_EMBEDDED_LIST_ID: GraphQLList,
-    }
-    wrapping_type = mapping_types.get(property_type, None)
-    if wrapping_type:
-        linked_property_obj = property_obj.qualifier
-        # There are properties that are embedded collections of non-primitive types,
-        # for example, ProxyEventSet.scalar_parameters.
-        # The GraphQL compiler does not currently support these.
-        if linked_property_obj in scalar_types:
-            return wrapping_type(scalar_types[linked_property_obj])
-
-    # We weren't able to represent this property in GraphQL, so we'll hide it instead.
-    return None
-
-
 def _get_union_type_name(type_names_to_union):
     """Construct a unique union type name based on the type names being unioned."""
     if not type_names_to_union:
@@ -110,13 +68,16 @@ def _get_fields_for_class(schema_graph, graphql_types, field_type_overrides, hid
 
     # Add leaf GraphQL fields (class properties).
     all_properties = {
-        property_name: _property_descriptor_to_graphql_type(property_obj)
+        property_name: property_obj.type
         for property_name, property_obj in six.iteritems(properties)
     }
+
+    # Filter collections of classes. They are currently not supported.
     result = {
-        property_name: graphql_representation
-        for property_name, graphql_representation in six.iteritems(all_properties)
-        if graphql_representation is not None
+        property_name: graphql_type
+        for property_name, graphql_type in six.iteritems(all_properties)
+        if not (isinstance(graphql_type, GraphQLList) and
+                isinstance(graphql_type.of_type, GraphQLObjectType))
     }
 
     # Add edge GraphQL fields (edges to other vertex classes).

graphql_compiler/schema_generation/schema_graph.py

@@ -3,6 +3,7 @@
 from itertools import chain
 
 from funcy.py3 import lsplit
+from graphql.type import GraphQLList, GraphQLObjectType
 import six
 
 from graphql_compiler.schema_generation.schema_properties import (
@@ -13,7 +14,7 @@
 from .schema_properties import (
     COLLECTION_PROPERTY_TYPES, ILLEGAL_PROPERTY_NAME_PREFIXES, ORIENTDB_BASE_EDGE_CLASS_NAME,
     ORIENTDB_BASE_VERTEX_CLASS_NAME, PROPERTY_TYPE_LINK_ID, PropertyDescriptor,
-    parse_default_property_value, validate_supported_property_type_id
+    get_graphql_scalar_type_or_raise, parse_default_property_value
 )
 from .utils import toposort_classes
 
@@ -35,16 +36,6 @@ def _validate_property_names(class_name, properties):
                                           u'{}'.format(class_name, property_name))
 
 
-def _validate_collections_have_default_values(class_name, property_name, property_descriptor):
-    """Validate that if the property is of collection type, it has a specified default value."""
-    # We don't want properties of collection type having "null" values, since that may cause
-    # unexpected errors during GraphQL query execution and other operations.
-    if property_descriptor.type_id in COLLECTION_PROPERTY_TYPES:
-        if property_descriptor.default is None:
-            raise IllegalSchemaStateError(u'Class "{}" has a property "{}" of collection type with '
-                                          u'no default value.'.format(class_name, property_name))
-
-
 def get_superclasses_from_class_definition(class_definition):
     """Extract a list of all superclass names from a class definition dict."""
     # New-style superclasses definition, supporting multiple-inheritance.
@@ -624,55 +615,54 @@ def _get_element_properties(self, class_name, non_link_property_definitions):
                                      u'more than once, this is not allowed!'
                                      .format(property_name, class_name))
 
-            property_descriptor = self._create_descriptor_from_property_definition(
-                class_name, property_definition)
+            graphql_type = self._get_graphql_type(class_name, property_definition)
+            default_value = _get_default_value(class_name, property_definition)
+            property_descriptor = PropertyDescriptor(graphql_type, default_value)
             property_name_to_descriptor[property_name] = property_descriptor
         return property_name_to_descriptor
 
-    def _create_descriptor_from_property_definition(self, class_name, property_definition):
-        """Return a PropertyDescriptor corresponding to the non-link property definition."""
+    def _get_graphql_type(self, class_name, property_definition):
+        """Return the GraphQLType corresponding to the non-link property definition."""
         name = property_definition['name']
         type_id = property_definition['type']
         linked_class = property_definition.get('linkedClass', None)
         linked_type = property_definition.get('linkedType', None)
-        qualifier = None
-
-        validate_supported_property_type_id(name, type_id)
 
+        graphql_type = None
         if type_id == PROPERTY_TYPE_LINK_ID:
-            raise AssertionError(u'Found a property of type Link on a non-edge class: '
-                                 u'{} {}'.format(name, class_name))
+            raise AssertionError(u'Found a improperly named property of type Link: '
+                                 u'{} {}. Links must be named either "in" or "out"'
+                                 .format(name, class_name))
         elif type_id in COLLECTION_PROPERTY_TYPES:
             if linked_class is not None and linked_type is not None:
                 raise AssertionError(u'Property "{}" unexpectedly has both a linked class and '
                                      u'a linked type: {}'.format(name, property_definition))
             elif linked_type is not None and linked_class is None:
                 # No linked class, must be a linked native OrientDB type.
-                validate_supported_property_type_id(name + ' inner type', linked_type)
-
-                qualifier = linked_type
+                inner_type = get_graphql_scalar_type_or_raise(name + ' inner type', linked_type)
+                graphql_type = GraphQLList(inner_type)
             elif linked_class is not None and linked_type is None:
                 # No linked type, must be a linked non-graph user-defined type.
                 if linked_class not in self._non_graph_class_names:
                     raise AssertionError(u'Property "{}" is declared as the inner type of '
                                          u'an embedded collection, but is not a non-graph class: '
                                          u'{}'.format(name, linked_class))
-
-                qualifier = linked_class
+                if class_name in self._non_graph_class_names:
+                    raise AssertionError('Class {} is a non-graph class that contains a '
+                                         'collection property {}. Only graph classes are allowed '
+                                         'to have collections as properties.'
+                                         .format(class_name, property_definition))
+                # Don't include the fields and implemented interfaces, this information is already
+                # stored in the SchemaGraph.
+                graphql_type = GraphQLList(GraphQLObjectType(linked_class, {}, []))
             else:
                 raise AssertionError(u'Property "{}" is an embedded collection but has '
                                      u'neither a linked class nor a linked type: '
                                      u'{}'.format(name, property_definition))
+        else:
+            graphql_type = get_graphql_scalar_type_or_raise(name, type_id)
 
-        default_value = None
-        default_value_string = property_definition.get('defaultValue', None)
-        if default_value_string is not None:
-            default_value = parse_default_property_value(name, type_id, default_value_string)
-
-        descriptor = PropertyDescriptor(type_id=type_id, qualifier=qualifier, default=default_value)
-        # Sanity-check the descriptor before returning it.
-        _validate_collections_have_default_values(class_name, name, descriptor)
-        return descriptor
+        return graphql_type
 
     def _link_vertex_and_edge_types(self):
         """For each edge, link it to the vertex types it connects to each other."""
@@ -730,3 +720,22 @@ def _get_class_fields(class_definition):
         # We convert this field back to an empty dict, for our general sanity.
         class_fields = dict()
     return class_fields
+
+
+def _get_default_value(class_name, property_definition):
+    """Return the default value of the OrientDB property."""
+    default_value = None
+    default_value_string = property_definition.get('defaultValue', None)
+    if default_value_string is not None:
+        default_value = parse_default_property_value(
+            property_definition['name'], property_definition['type'], default_value_string)
+
+    # We don't want properties of collection type having "null" values, since that may cause
+    # unexpected errors during GraphQL query execution and other operations.
+    if property_definition['type'] in COLLECTION_PROPERTY_TYPES:
+        if default_value is None:
+            raise IllegalSchemaStateError(u'Class "{}" has a property "{}" of collection type with '
+                                          u'no default value.'
+                                          .format(class_name, property_definition))
+
+    return default_value

graphql_compiler/schema_generation/schema_properties.py

@@ -3,8 +3,11 @@
 import datetime
 import time
 
+from graphql.type import GraphQLBoolean, GraphQLFloat, GraphQLInt, GraphQLString
 import six
 
+from ..schema import GraphQLAny, GraphQLDate, GraphQLDateTime, GraphQLDecimal
+
 
 EDGE_SOURCE_PROPERTY_NAME = 'out'
 EDGE_DESTINATION_PROPERTY_NAME = 'in'
@@ -90,15 +93,30 @@
     PROPERTY_TYPE_ANY_ID: PROPERTY_TYPE_ANY_NAME,
 }
 
+ORIENTDB_TO_GRAPHQL_SCALARS = {
+    PROPERTY_TYPE_ANY_ID: GraphQLAny,
+    PROPERTY_TYPE_BOOLEAN_ID: GraphQLBoolean,
+    PROPERTY_TYPE_DATE_ID: GraphQLDate,
+    PROPERTY_TYPE_DATETIME_ID: GraphQLDateTime,
+    PROPERTY_TYPE_DECIMAL_ID: GraphQLDecimal,
+    PROPERTY_TYPE_DOUBLE_ID: GraphQLFloat,
+    PROPERTY_TYPE_FLOAT_ID: GraphQLFloat,
+    PROPERTY_TYPE_INTEGER_ID: GraphQLInt,
+    PROPERTY_TYPE_LONG_ID: GraphQLInt,
+    PROPERTY_TYPE_STRING_ID: GraphQLString,
+}
+
+
 ORIENTDB_DATETIME_FORMAT = '%Y-%m-%d %H:%M:%S'
 ORIENTDB_DATE_FORMAT = '%Y-%m-%d'
 
 
-def validate_supported_property_type_id(property_name, property_type_id):
-    """Ensure that the given property type_id is supported by the graph."""
-    if property_type_id not in PROPERTY_TYPE_ID_TO_NAME:
+def get_graphql_scalar_type_or_raise(property_name, property_type_id):
+    """Return the matching GraphQLScalarType for the property type id, asserting it exists."""
+    if property_type_id not in ORIENTDB_TO_GRAPHQL_SCALARS:
         raise AssertionError(u'Property "{}" has unsupported property type id: '
                              u'{}'.format(property_name, property_type_id))
+    return ORIENTDB_TO_GRAPHQL_SCALARS[property_type_id]
 
 
 def _parse_bool_default_value(property_name, default_value_string):
@@ -173,16 +191,7 @@ def parse_default_property_value(property_name, property_type_id, default_value_
 
 
 # A way to describe a property's type and associated information:
-#   - type_id: int, the OrientDB property type ID -- can be made human-readable
-#              using the above PROPERTY_TYPE_ID_TO_NAME map.
-#   - qualifier: dependent on the type_id
-#        - For Link properties, string -- the name of the class to which the Link points.
-#        - For EmbeddedSet and EmbeddedList, either:
-#              - int, the property type ID of the native OrientDB type, if the data in
-#                the collection is of a built-in OrientDB type, or
-#              - string, the name of the non-graph class representing the data in the collection.
-#        - For all other property types, None.
+#   - type: GraphQLType, the type of this property
 #   - default: the default value for the property, used when a record is inserted without an
 #              explicit value for this property. Set to None if no default is given in the schema.
-PropertyDescriptor = namedtuple('PropertyDescriptor',
-                                ('type_id', 'qualifier', 'default'))
+PropertyDescriptor = namedtuple('PropertyDescriptor', ('type', 'default'))

graphql_compiler/tests/test_schema_generation.py

@@ -2,7 +2,7 @@
 import unittest
 
 from frozendict import frozendict
-from graphql.type import GraphQLList, GraphQLString
+from graphql.type import GraphQLList, GraphQLObjectType, GraphQLString
 import six
 
 from .. import get_graphql_schema_from_orientdb_schema_data
@@ -76,8 +76,8 @@
             'linkedClass': 'ExternalSource',
             'defaultValue': '[]'
         }
-
-    ]
+    ],
+    'superClass': 'V',
 })
 
 PERSON_LIVES_IN_EDGE_SCHEMA_DATA = frozendict({
@@ -188,14 +188,14 @@ def test_parsed_superclasses_field(self):
         self.assertEqual({'Entity', ORIENTDB_BASE_VERTEX_CLASS_NAME},
                          schema_graph.get_inheritance_set('Entity'))
 
-    def test_parse_property(self):
+    def test_parsed_property(self):
         schema_data = [
             BASE_VERTEX_SCHEMA_DATA,
             ENTITY_SCHEMA_DATA,
         ]
         schema_graph = SchemaGraph(schema_data)
         name_property = schema_graph.get_element_by_class_name('Entity').properties['name']
-        self.assertEqual(name_property.type_id, PROPERTY_TYPE_STRING_ID)
+        self.assertTrue(name_property.type.is_same_type(GraphQLString))
 
     def test_native_orientdb_collection_property(self):
         schema_data = [
@@ -205,20 +205,20 @@ def test_native_orientdb_collection_property(self):
         ]
         schema_graph = SchemaGraph(schema_data)
         alias_property = schema_graph.get_element_by_class_name('Person').properties['alias']
-        self.assertEqual(alias_property.type_id, PROPERTY_TYPE_EMBEDDED_SET_ID)
-        self.assertEqual(alias_property.qualifier, PROPERTY_TYPE_STRING_ID)
+        self.assertTrue(alias_property.type.is_same_type(GraphQLList(GraphQLString)))
         self.assertEqual(alias_property.default, set())
 
     def test_class_collection_property(self):
         schema_data = [
+            BASE_VERTEX_SCHEMA_DATA,
             DATA_POINT_SCHEMA_DATA,
             EXTERNAL_SOURCE_SCHEMA_DATA,
         ]
         schema_graph = SchemaGraph(schema_data)
         friends_property = schema_graph.get_element_by_class_name('DataPoint').properties[
             'data_source']
-        self.assertEqual(friends_property.type_id, PROPERTY_TYPE_EMBEDDED_LIST_ID)
-        self.assertEqual(friends_property.qualifier, 'ExternalSource')
+        self.assertTrue(friends_property.type.is_same_type(
+            GraphQLList(GraphQLObjectType('ExternalSource', {}))))
         self.assertEqual(friends_property.default, list())
 
     def test_link_parsing(self):