Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix and test schema generation interface #220

Merged
merged 20 commits into from Mar 22, 2019
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 19 additions & 12 deletions graphql_compiler/__init__.py
Expand Up @@ -141,9 +141,7 @@ def get_graphql_schema_from_orientdb_schema_data(schema_data, class_to_field_typ

Args:
schema_data: list of dicts describing the classes in the OrientDB schema. The following
format is the way the data is structured in OrientDB 2. See
test_get_graphql_schema_from_orientdb_schema in
graphql_compiler/tests/integration_tests/test_backends_integration.py
format is the way the data is structured in OrientDB 2. See the README.md file
for an example of how to query this data.
Each dict has the following string fields:
- name: string, the name of the class.
Expand All @@ -159,22 +157,31 @@ def get_graphql_schema_from_orientdb_schema_data(schema_data, class_to_field_typ
- properties: list of dicts, describing the class's properties.
Each property dictionary has the following string fields:
- name: string, the name of the property.
- type_id: int, builtin OrientDB type ID of the field.
- type: int, builtin OrientDB type ID of the property.
See schema_properties.py for the mapping.
- linked_type (optional): int, if the property is a
collection of builtin OrientDB
objects, then it indicates their
type ID.
- linked_class (optional): string, if the property is a
collection of class instances,
then it indicates the name of the
class. If class is an edge class,
and the field name is either 'in'
or 'out', then it describes the
name of an endpoint of the edge.
collection of class instances,
then it indicates the name of
the class. If class is an edge
class, and the field name is
either 'in' or 'out', then it
describes the name of an
endpoint of the edge.
- defaultValue: string, the textual representation of the
default value for the property, as
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the visual indentation is off by one space (compare string vs default)

returned by OrientDB's schema
introspection code, e.g., '{}' for
the embedded set type. Note that if the
property is a collection type is must
have a default value.
class_to_field_type_overrides: optional dict, class name -> {field name -> field type},
(string -> {string -> GraphQLType}). Used to override the
type of a field in the class where it's first defined and all
the class's subclasses.
type of a field in the class where it's first
defined and all the class's subclasses.

Returns:
tuple of (GraphQL schema object, GraphQL type equivalence hints dict).
Expand Down
30 changes: 18 additions & 12 deletions graphql_compiler/schema_generation/schema_graph.py
Expand Up @@ -207,9 +207,7 @@ def __init__(self, schema_data):
Args:
schema_data: list of dicts describing the classes in the OrientDB schema. The following
format is the way the data is structured in OrientDB 2. See
test_get_graphql_schema_from_orientdb_schema in
graphql_compiler/tests/integration_tests/test_backends_integration.py
for an example of how to query this data.
the README.md file for an example of how to query this data.
Each dict has the following string fields:
- name: string, the name of the class.
- superClasses (optional): list of strings, the name of the class's
Expand All @@ -224,20 +222,28 @@ class instead of instances of the class.
- properties: list of dicts, describing the class's properties.
Each property dictionary has the following string fields:
- name: string, the name of the property.
- type_id: int, builtin OrientDB type ID of the field.
- type: int, builtin OrientDB type ID of the field.
See schema_properties.py for the mapping.
- linked_type (optional): int, if the property is a
collection of builtin
OrientDB objects, then it
indicates their type ID.
- linked_class (optional): string, if the property is a
collection of class
instances, then it indicates
the name of the class. If
class is an edge class, and
the field name is either 'in'
or 'out', then it describes
the name of an endpoint of
the edge.
collection of class
instances, then it indicates
the name of the class. If
class is an edge class, and
the field name is either
'in' or 'out', then it
describes the name of an
endpoint of the edge.
- defaultValue: string, the textual representation
of the default value for the
property, as returned by OrientDB's
schema introspection code, e.g.,
'{}' for the embedded set type. Note
that if the property is a collection
type is must have a default value.

Returns:
fully-constructed SchemaGraph object
Expand Down
@@ -0,0 +1,55 @@
# -*- coding: utf-8 -*-
# snapshottest: v1 - https://goo.gl/zC4yUc
from __future__ import unicode_literals

from snapshottest import Snapshot


snapshots = Snapshot()

snapshots['GraphQLSchemaGenerationTests::test_graphql_schema_generation_from_schema_data_api 1'] = '''schema {
query: RootSchemaQuery
}

directive @filter(op_name: String!, value: [String!]!) on FIELD | INLINE_FRAGMENT

directive @tag(tag_name: String!) on FIELD

directive @output(out_name: String!) on FIELD

directive @output_source on FIELD

directive @optional on FIELD

directive @recurse(depth: Int!) on FIELD

directive @fold on FIELD

scalar Decimal

interface Entity {
_x_count: Int
name: String
}

type Location implements Entity {
_x_count: Int
description: String
in_Person_LivesIn: [Person]
name: String
}

type Person implements Entity {
_x_count: Int
alias: [String]
name: String
net_worth: Decimal
out_Person_LivesIn: [Location]
}

type RootSchemaQuery {
Entity: Entity
Location: Location
Person: Person
}
'''
@@ -0,0 +1,88 @@
# Copyright 2018-present Kensho Technologies, LLC.
from graphql.utils.schema_printer import print_schema
from snapshottest import TestCase

from ... import get_graphql_schema_from_orientdb_schema_data
from ...schema_generation.schema_properties import (
ORIENTDB_BASE_EDGE_CLASS_NAME, ORIENTDB_BASE_VERTEX_CLASS_NAME, PROPERTY_TYPE_DECIMAL_ID,
PROPERTY_TYPE_EMBEDDED_SET_ID, PROPERTY_TYPE_LINK_ID, PROPERTY_TYPE_STRING_ID
)


class GraphQLSchemaGenerationTests(TestCase):

def test_graphql_schema_generation_from_schema_data_api(self):
orientdb_schema_data = [
{
'name': ORIENTDB_BASE_VERTEX_CLASS_NAME,
'abstract': False,
'properties': []
},
{
'name': ORIENTDB_BASE_EDGE_CLASS_NAME,
'abstract': False,
'properties': []
},
{
'name': 'Entity',
'abstract': True,
'superClasses': [ORIENTDB_BASE_VERTEX_CLASS_NAME],
'properties': [
{
'name': 'name',
'type': PROPERTY_TYPE_STRING_ID,
}
]
},
{
'name': 'Person',
'abstract': False,
'superClasses': ['Entity'],
'properties': [
{
'name': 'net_worth',
'type': PROPERTY_TYPE_DECIMAL_ID,
},
{
'name': 'alias',
'type': PROPERTY_TYPE_EMBEDDED_SET_ID,
'linkedType': PROPERTY_TYPE_STRING_ID,
'defaultValue': '{}'
}
],
},
{
'name': 'Person_LivesIn',
'abstract': False,
'customFields': {
'human_name_in': 'Person',
'human_name_out': 'Location where person lives',
},
'properties': [
{
'name': 'in',
'type': PROPERTY_TYPE_LINK_ID,
'linkedClass': 'Location',
},
{
'name': 'out',
'type': PROPERTY_TYPE_LINK_ID,
'linkedClass': 'Person',
}
],
'superClass': ORIENTDB_BASE_EDGE_CLASS_NAME
},
{
'name': 'Location',
'abstract': False,
'superClasses': ['Entity'],
'properties': [
{
'name': 'description',
'type': PROPERTY_TYPE_STRING_ID,
}
]
},
]
schema, _ = get_graphql_schema_from_orientdb_schema_data(orientdb_schema_data)
self.assertMatchSnapshot(print_schema(schema))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need this to be a snapshot test here? This feels like a unit test to me since we have well-defined input and output, and a single function call being tested. Invoking all the snapshot testing machinery feels like overkill for this use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good idea because every time I've made a small change to the way we generate the schema, I've had to print it and then copy paste it into test_helpers. If I introduce this test as an unit test, every time we change the schema we now have to change two things.

2 changes: 2 additions & 0 deletions graphql_compiler/tests/test_data_tools/schema.sql
Expand Up @@ -64,6 +64,8 @@ CREATE INDEX Animal.net_worth NOTUNIQUE
CREATE CLASS Animal_ParentOf EXTENDS E
CREATE PROPERTY Animal_ParentOf.in LINK Animal
CREATE PROPERTY Animal_ParentOf.out LINK Animal
ALTER CLASS Animal_ParentOf CUSTOM human_name_in="Parent"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test on the generated schema graph that these properties exist and have their expected values?

ALTER CLASS Animal_ParentOf CUSTOM human_name_out="Child"
CREATE INDEX Animal_ParentOf ON Animal_ParentOf (in, out) UNIQUE_HASH_INDEX

CREATE CLASS Animal_FedAt EXTENDS E
Expand Down