Merge pull request #20 from graphql-python/strict-key-ordering

Add option to specify `map_type` in the Executor.

Closes #19

Author: jhgg

graphql/core/execution/executor.py

@@ -15,9 +15,21 @@
 
 
 class Executor(object):
-    def __init__(self, execution_middlewares=None, default_resolver=default_resolve_fn):
-        self.execution_middlewares = execution_middlewares or []
-        self.default_resolve_fn = default_resolver
+    def __init__(self, execution_middlewares=None, default_resolver=default_resolve_fn, map_type=dict):
+        assert issubclass(map_type, collections.MutableMapping)
+
+        self._execution_middlewares = execution_middlewares or []
+        self._default_resolve_fn = default_resolver
+        self._map_type = map_type
+        self._enforce_strict_ordering = issubclass(map_type, collections.OrderedDict)
+
+    @property
+    def enforce_strict_ordering(self):
+        return self._enforce_strict_ordering
+
+    @property
+    def map_type(self):
+        return self._map_type
 
     def execute(self, schema, request='', root=None, args=None, operation_name=None, request_context=None,
                 execute_serially=False, validate_ast=True):
@@ -34,7 +46,7 @@ def execute(self, schema, request='', root=None, args=None, operation_name=None,
             validate_ast
         )
 
-        for middleware in self.execution_middlewares:
+        for middleware in self._execution_middlewares:
             if hasattr(middleware, 'execution_result'):
                 curried_execution_function = functools.partial(middleware.execution_result, curried_execution_function)
 
@@ -81,7 +93,10 @@ def _execute_operation(self, ctx, root, operation, execute_serially):
         if operation.operation == 'mutation' or execute_serially:
             execute_serially = True
 
-        fields = DefaultOrderedDict(list) if execute_serially else collections.defaultdict(list)
+        fields = DefaultOrderedDict(list) \
+            if (execute_serially or self._enforce_strict_ordering) \
+            else collections.defaultdict(list)
+
         fields = collect_fields(ctx, type, operation.selection_set, fields, set())
 
         if execute_serially:
@@ -101,20 +116,20 @@ def collect_result(resolved_result):
                 return results
 
             if isinstance(result, Deferred):
-                return result.add_callback(collect_result)
+                return succeed(result).add_callback(collect_result)
 
             else:
                 return collect_result(result)
 
         def execute_field(prev_deferred, response_name):
             return prev_deferred.add_callback(execute_field_callback, response_name)
 
-        return functools.reduce(execute_field, fields.keys(), succeed({}))
+        return functools.reduce(execute_field, fields.keys(), succeed(self._map_type()))
 
     def _execute_fields(self, execution_context, parent_type, source_value, fields):
         contains_deferred = False
 
-        results = {}
+        results = self._map_type()
         for response_name, field_asts in fields.items():
             result = self._resolve_field(execution_context, parent_type, source_value, field_asts)
             if result is Undefined:
@@ -138,7 +153,7 @@ def _resolve_field(self, execution_context, parent_type, source, field_asts):
             return Undefined
 
         return_type = field_def.type
-        resolve_fn = field_def.resolver or self.default_resolve_fn
+        resolve_fn = field_def.resolver or self._default_resolve_fn
 
         # Build a dict of arguments from the field.arguments AST, using the variables scope to
         # fulfill any variable references.
@@ -283,7 +298,7 @@ def complete_value(self, ctx, return_type, field_asts, info, result):
             )
 
         # Collect sub-fields to execute to complete this value.
-        subfield_asts = collections.defaultdict(list)
+        subfield_asts = DefaultOrderedDict(list) if self._enforce_strict_ordering else collections.defaultdict(list)
         visited_fragment_names = set()
         for field_ast in field_asts:
             selection_set = field_ast.selection_set
@@ -298,7 +313,7 @@ def run_resolve_fn(self, resolve_fn, source, args, info):
         curried_resolve_fn = functools.partial(resolve_fn, source, args, info)
 
         try:
-            for middleware in self.execution_middlewares:
+            for middleware in self._execution_middlewares:
                 if hasattr(middleware, 'run_resolve_fn'):
                     curried_resolve_fn = functools.partial(middleware.run_resolve_fn, curried_resolve_fn, resolve_fn)
 

graphql/core/pyutils/defer.py

@@ -466,16 +466,19 @@ class _ResultCollector(Deferred):
     objects_remaining_to_resolve = 0
     _result = None
 
-    def _schedule_callbacks(self, items, result, objects_remaining_to_resolve=None):
+    def _schedule_callbacks(self, items, result, objects_remaining_to_resolve=None, preserve_insert_ordering=False):
         self.objects_remaining_to_resolve = \
             objects_remaining_to_resolve if objects_remaining_to_resolve is not None else len(items)
         self._result = result
         for key, value in items:
             if isinstance(value, Deferred):
+                # We will place a value in place of the resolved key, so that insert order is preserved.
+                if preserve_insert_ordering:
+                    result[key] = None
+
                 value.add_callbacks(self._cb_deferred, self._cb_deferred,
                                     callback_args=(key, True),
                                     errback_args=(key, False))
-
             else:
                 self.objects_remaining_to_resolve -= 1
                 result[key] = value
@@ -509,7 +512,8 @@ class DeferredDict(_ResultCollector):
     def __init__(self, mapping):
         super(DeferredDict, self).__init__()
         assert isinstance(mapping, collections.Mapping)
-        self._schedule_callbacks(mapping.items(), {})
+        self._schedule_callbacks(mapping.items(), type(mapping)(),
+                                 preserve_insert_ordering=isinstance(mapping, collections.OrderedDict))
 
 
 class DeferredList(_ResultCollector):

graphql/core/utils/build_ast_schema.py

@@ -68,7 +68,6 @@ def build_ast_schema(document, query_type_name, mutation_type_name=None):
 
     def produce_type_def(type_ast):
         type_name = _get_inner_type_name(type_ast)
-        print('ptd', type_name)
         if type_name in inner_type_map:
             return _build_wrapped_type(inner_type_map[type_name], type_ast)
 

tests/core_execution/test_concurrent_executor.py

@@ -1,3 +1,4 @@
+from collections import OrderedDict
 from graphql.core.error import format_error
 from graphql.core.execution import Executor
 from graphql.core.execution.middlewares.sync import SynchronousExecutionMiddleware
@@ -233,3 +234,36 @@ def promise(self):
     assert not isinstance(result, Deferred)
     assert result.data == {"promise": 'I should work'}
     assert not result.errors
+
+
+def test_executor_can_enforce_strict_ordering():
+    Type = GraphQLObjectType('Type', lambda: {
+        'a': GraphQLField(GraphQLString,
+                          resolver=lambda *_: succeed('Apple')),
+        'b': GraphQLField(GraphQLString,
+                          resolver=lambda *_: succeed('Banana')),
+        'c': GraphQLField(GraphQLString,
+                          resolver=lambda *_: succeed('Cherry')),
+        'deep': GraphQLField(Type, resolver=lambda *_: succeed({})),
+    })
+    schema = GraphQLSchema(query=Type)
+    executor = Executor(map_type=OrderedDict)
+
+    query = '{ a b c aa: c cc: c bb: b aaz: a bbz: b deep { b a c deeper: deep { c a b } } ' \
+            'ccz: c zzz: c aaa: a }'
+
+    def handle_results(result):
+        assert not result.errors
+
+        data = result.data
+        assert isinstance(data, OrderedDict)
+        assert list(data.keys()) == ['a', 'b', 'c', 'aa', 'cc', 'bb', 'aaz', 'bbz', 'deep', 'ccz', 'zzz', 'aaa']
+        deep = data['deep']
+        assert isinstance(deep, OrderedDict)
+        assert list(deep.keys()) == ['b', 'a', 'c', 'deeper']
+        deeper = deep['deeper']
+        assert isinstance(deeper, OrderedDict)
+        assert list(deeper.keys()) == ['c', 'a', 'b']
+
+    raise_callback_results(executor.execute(schema, query), handle_results)
+    raise_callback_results(executor.execute(schema, query, execute_serially=True), handle_results)

tests/core_execution/test_executor.py

@@ -1,6 +1,8 @@
+from collections import OrderedDict
 import json
 from pytest import raises
-from graphql.core.execution import execute
+from graphql.core.execution import execute, Executor
+from graphql.core.execution.middlewares.sync import SynchronousExecutionMiddleware
 from graphql.core.language.parser import parse
 from graphql.core.type import (GraphQLSchema, GraphQLObjectType, GraphQLField,
                                GraphQLArgument, GraphQLList, GraphQLInt, GraphQLString,
@@ -450,3 +452,35 @@ def test_fails_to_execute_a_query_containing_a_type_definition():
         result = execute(schema, None, query)
 
     assert excinfo.value.message == 'GraphQL cannot execute a request containing a ObjectTypeDefinition.'
+
+
+def test_executor_can_enforce_strict_ordering():
+    Type = GraphQLObjectType('Type', lambda: {
+        'a': GraphQLField(GraphQLString,
+                          resolver=lambda *_: 'Apple'),
+        'b': GraphQLField(GraphQLString,
+                          resolver=lambda *_: 'Banana'),
+        'c': GraphQLField(GraphQLString,
+                          resolver=lambda *_: 'Cherry'),
+        'deep': GraphQLField(Type, resolver=lambda *_: {}),
+    })
+    schema = GraphQLSchema(query=Type)
+    executor = Executor(execution_middlewares=[SynchronousExecutionMiddleware], map_type=OrderedDict)
+    query = '{ a b c aa: c cc: c bb: b aaz: a bbz: b deep { b a c deeper: deep { c a b } } ' \
+            'ccz: c zzz: c aaa: a }'
+
+    def check_result(result):
+        assert not result.errors
+
+        data = result.data
+        assert isinstance(data, OrderedDict)
+        assert list(data.keys()) == ['a', 'b', 'c', 'aa', 'cc', 'bb', 'aaz', 'bbz', 'deep', 'ccz', 'zzz', 'aaa']
+        deep = data['deep']
+        assert isinstance(deep, OrderedDict)
+        assert list(deep.keys()) == ['b', 'a', 'c', 'deeper']
+        deeper = deep['deeper']
+        assert isinstance(deeper, OrderedDict)
+        assert list(deeper.keys()) == ['c', 'a', 'b']
+
+    check_result(executor.execute(schema, query))
+    check_result(executor.execute(schema, query, execute_serially=True))