Fix serial execution not always executing in order.

The executor will now choose to use an ordered dict implementation when collecting the root fields of query/mutation, rather than an unordered-dict implementation to guarantee the order of execution when being executed serially is the same as it is defined in the query. No order guarantees are given for non-serial execution, as per the spec.

Author: jhgg

graphql/core/execution/base.py

@@ -82,24 +82,31 @@ def __init__(self, data=None, errors=None, invalid=False):
                 error.value if isinstance(error, DeferredException) else error
                 for error in errors
             ]
+
         self.errors = errors
+
         if invalid:
             assert data is None
+
         self.invalid = invalid
 
 
 def get_operation_root_type(schema, operation):
     op = operation.operation
     if op == 'query':
         return schema.get_query_type()
+
     elif op == 'mutation':
         mutation_type = schema.get_mutation_type()
+
         if not mutation_type:
             raise GraphQLError(
                 'Schema is not configured for mutations',
                 [operation]
             )
+
         return mutation_type
+
     raise GraphQLError(
         'Can only execute queries and mutations',
         [operation]
@@ -109,35 +116,36 @@ def get_operation_root_type(schema, operation):
 def collect_fields(ctx, type, selection_set, fields, prev_fragment_names):
     for selection in selection_set.selections:
         directives = selection.directives
+
         if isinstance(selection, ast.Field):
             if not should_include_node(ctx, directives):
                 continue
+
             name = get_field_entry_key(selection)
-            if name not in fields:
-                fields[name] = []
             fields[name].append(selection)
+
         elif isinstance(selection, ast.InlineFragment):
-            if not should_include_node(ctx, directives) or \
-                    not does_fragment_condition_match(ctx, selection, type):
+            if not should_include_node(ctx, directives) or not does_fragment_condition_match(ctx, selection, type):
                 continue
-            collect_fields(
-                ctx, type, selection.selection_set,
-                fields, prev_fragment_names)
+
+            collect_fields(ctx, type, selection.selection_set, fields, prev_fragment_names)
+
         elif isinstance(selection, ast.FragmentSpread):
             frag_name = selection.name.value
-            if frag_name in prev_fragment_names or \
-                    not should_include_node(ctx, directives):
+
+            if frag_name in prev_fragment_names or not should_include_node(ctx, directives):
                 continue
+
             prev_fragment_names.add(frag_name)
             fragment = ctx.fragments.get(frag_name)
             frag_directives = fragment.directives
-            if not fragment or \
-                    not should_include_node(ctx, frag_directives) or \
-                    not does_fragment_condition_match(ctx, fragment, type):
+            if not fragment or not \
+                    should_include_node(ctx, frag_directives) or not \
+                    does_fragment_condition_match(ctx, fragment, type):
                 continue
-            collect_fields(
-                ctx, type, fragment.selection_set,
-                fields, prev_fragment_names)
+
+            collect_fields(ctx, type, fragment.selection_set, fields, prev_fragment_names)
+
     return fields
 
 
@@ -146,10 +154,12 @@ def should_include_node(ctx, directives):
     @skip directives, where @skip has higher precidence than @include."""
     if directives:
         skip_ast = None
+
         for directive in directives:
             if directive.name.value == GraphQLSkipDirective.name:
                 skip_ast = directive
                 break
+
         if skip_ast:
             args = get_argument_values(
                 GraphQLSkipDirective.args,
@@ -159,16 +169,19 @@ def should_include_node(ctx, directives):
             return not args.get('if')
 
         include_ast = None
+
         for directive in directives:
             if directive.name.value == GraphQLIncludeDirective.name:
                 include_ast = directive
                 break
+
         if include_ast:
             args = get_argument_values(
                 GraphQLIncludeDirective.args,
                 include_ast.arguments,
                 ctx.variables,
             )
+
             return bool(args.get('if'))
 
     return True

graphql/core/execution/executor.py

@@ -6,6 +6,7 @@
 from ..language import ast
 from ..language.parser import parse
 from ..language.source import Source
+from ..pyutils.default_ordered_dict import DefaultOrderedDict
 from ..type import GraphQLEnumType, GraphQLInterfaceType, GraphQLList, GraphQLNonNull, GraphQLObjectType, \
     GraphQLScalarType, GraphQLUnionType
 from ..utils.is_nullish import is_nullish
@@ -76,9 +77,14 @@ def _execute_graphql_query(self, root, ast, operation_name, args, request_contex
 
     def _execute_operation(self, ctx, root, operation, execute_serially):
         type = get_operation_root_type(ctx.schema, operation)
-        fields = collect_fields(ctx, type, operation.selection_set, {}, set())
 
         if operation.operation == 'mutation' or execute_serially:
+            execute_serially = True
+
+        field_map = DefaultOrderedDict(list) if execute_serially else collections.defaultdict(list)
+        fields = collect_fields(ctx, type, operation.selection_set, field_map, set())
+
+        if execute_serially:
             return self._execute_fields_serially(ctx, type, root, fields)
 
         return self._execute_fields(ctx, type, root, fields)
@@ -277,7 +283,7 @@ def complete_value(self, ctx, return_type, field_asts, info, result):
             )
 
         # Collect sub-fields to execute to complete this value.
-        subfield_asts = {}
+        subfield_asts = collections.defaultdict(list)
         visited_fragment_names = set()
         for field_ast in field_asts:
             selection_set = field_ast.selection_set