chore: Cleaning up the request module in engine-v2 (#1375)

This PR only moves around code, no business logic is changed.

I'm mostly simplifying the `Operation` build logic, exposing only a
`Operation::build` to engine which also does the parsing and separating
all the validation logic inside a module. I'm also updating it to
traverse the selection sets through `fields()` which is a lot simpler.

engine/crates/engine-v2/src/engine.rs

@@ -10,7 +10,7 @@ use schema::Schema;
 use crate::{
     execution::{ExecutionCoordinator, PreparedExecution},
     plan::OperationPlan,
-    request::{parse_operation, Operation},
+    request::{bind_variables, Operation},
     response::{ExecutionMetadata, GraphqlError, Response},
 };
 
@@ -102,8 +102,7 @@ impl Engine {
             Err(error) => return Err(Response::from_error(error, ExecutionMetadata::default())),
         };
 
-        let input_values = operation_plan
-            .bind_variables(self.schema.as_ref(), &mut request.variables)
+        let input_values = bind_variables(self.schema.as_ref(), &operation_plan, &mut request.variables)
             .map_err(|errors| Response::from_errors(errors, ExecutionMetadata::build(&operation_plan)))?;
 
         Ok(ExecutionCoordinator::new(
@@ -121,15 +120,8 @@ impl Engine {
                 return Ok(cached);
             }
         }
-        let parsed_operation = parse_operation(request)?;
-        let bound_operation = Operation::build(
-            &self.schema,
-            parsed_operation,
-            !request.operation_limits_disabled(),
-            request.introspection_state(),
-            request,
-        )?;
-        let prepared = Arc::new(OperationPlan::prepare(&self.schema, bound_operation)?);
+        let operation = Operation::build(&self.schema, request)?;
+        let prepared = Arc::new(OperationPlan::prepare(&self.schema, operation)?);
         #[cfg(feature = "plan_cache")]
         {
             self.plan_cache

engine/crates/engine-v2/src/plan/collected.rs

@@ -1,12 +1,15 @@
 use schema::{FieldId, ObjectId, ScalarType, Wrapping};
 
 use crate::{
-    request::{BoundFieldId, FlatTypeCondition, SelectionSetType},
+    request::{BoundFieldId, SelectionSetType},
     response::{ResponseEdge, SafeResponseKey},
     utils::IdRange,
 };
 
-use super::{CollectedFieldId, CollectedSelectionSetId, ConditionalFieldId, ConditionalSelectionSetId, PlanBoundaryId};
+use super::{
+    CollectedFieldId, CollectedSelectionSetId, ConditionalFieldId, ConditionalSelectionSetId, FlatTypeCondition,
+    PlanBoundaryId,
+};
 
 // TODO: The two AnyCollectedSelectionSet aren't great, need to split better the ones which are computed
 // during planning and the others.

engine/crates/engine-v2/src/request/flat.rs → engine/crates/engine-v2/src/plan/flat.rs

@@ -7,84 +7,83 @@ use std::{
 use itertools::Itertools;
 use schema::{Definition, InterfaceId, ObjectId, Schema};
 
-use crate::request::{BoundFieldId, BoundSelectionSetId, SelectionSetType, TypeCondition};
-
-use super::{BoundSelection, OperationWalker};
-
-impl<'a> OperationWalker<'a> {
-    pub fn flatten_selection_sets(&self, root_selection_set_ids: Vec<BoundSelectionSetId>) -> FlatSelectionSet {
-        let ty = {
-            let selection_set_types = root_selection_set_ids
+use crate::request::{BoundFieldId, BoundSelection, BoundSelectionSetId, Operation, SelectionSetType, TypeCondition};
+
+pub fn flatten_selection_sets(
+    schema: &Schema,
+    operation: &Operation,
+    root_selection_set_ids: Vec<BoundSelectionSetId>,
+) -> FlatSelectionSet {
+    let ty = {
+        let selection_set_types = root_selection_set_ids
+            .iter()
+            .map(|id| operation[*id].ty)
+            .collect::<HashSet<SelectionSetType>>();
+        assert_eq!(
+            selection_set_types.len(),
+            1,
+            "{}",
+            selection_set_types
+                .into_iter()
+                .map(|ty| schema.walk(Definition::from(ty)).name())
+                .join(", ")
+        );
+        selection_set_types.into_iter().next().unwrap()
+    };
+    let mut flat_selection_set = FlatSelectionSet {
+        root_selection_set_ids,
+        ty,
+        fields: Vec::new(),
+    };
+    let mut selections = VecDeque::from_iter(flat_selection_set.root_selection_set_ids.iter().flat_map(
+        |&selection_set_id| {
+            operation[selection_set_id]
+                .items
                 .iter()
-                .map(|id| self.operation[*id].ty)
-                .collect::<HashSet<SelectionSetType>>();
-            assert_eq!(
-                selection_set_types.len(),
-                1,
-                "{}",
-                selection_set_types
-                    .into_iter()
-                    .map(|ty| self.schema_walker.walk(Definition::from(ty)).name())
-                    .join(", ")
-            );
-            selection_set_types.into_iter().next().unwrap()
-        };
-        let mut flat_selection_set = FlatSelectionSet {
-            root_selection_set_ids,
-            ty,
-            fields: Vec::new(),
-        };
-        let mut selections = VecDeque::from_iter(flat_selection_set.root_selection_set_ids.iter().flat_map(
-            |&selection_set_id| {
-                self.operation[selection_set_id]
-                    .items
-                    .iter()
-                    .map(move |selection| (Vec::<TypeCondition>::new(), vec![selection_set_id], selection))
-            },
-        ));
-        while let Some((mut type_condition_chain, mut selection_set_path, selection)) = selections.pop_front() {
-            match selection {
-                &BoundSelection::Field(bound_field_id) => {
-                    let type_condition =
-                        FlatTypeCondition::flatten(&self.schema_walker, flat_selection_set.ty, type_condition_chain);
-                    if FlatTypeCondition::is_possible(&type_condition) {
-                        flat_selection_set.fields.push(FlatField {
-                            type_condition,
-                            selection_set_path,
-                            bound_field_id,
-                        });
-                    }
+                .map(move |selection| (Vec::<TypeCondition>::new(), vec![selection_set_id], selection))
+        },
+    ));
+    while let Some((mut type_condition_chain, mut selection_set_path, selection)) = selections.pop_front() {
+        match selection {
+            &BoundSelection::Field(bound_field_id) => {
+                let type_condition = FlatTypeCondition::flatten(schema, flat_selection_set.ty, type_condition_chain);
+                if FlatTypeCondition::is_possible(&type_condition) {
+                    flat_selection_set.fields.push(FlatField {
+                        type_condition,
+                        selection_set_path,
+                        bound_field_id,
+                    });
                 }
-                BoundSelection::FragmentSpread(spread_id) => {
-                    let spread = &self.operation[*spread_id];
-                    let fragment = &self.operation[spread.fragment_id];
-                    type_condition_chain.push(fragment.type_condition);
-                    selection_set_path.push(spread.selection_set_id);
-                    selections.extend(
-                        self.operation[spread.selection_set_id]
-                            .items
-                            .iter()
-                            .map(|selection| (type_condition_chain.clone(), selection_set_path.clone(), selection)),
-                    );
-                }
-                BoundSelection::InlineFragment(inline_fragment_id) => {
-                    let inline_fragment = &self.operation[*inline_fragment_id];
-                    if let Some(type_condition) = inline_fragment.type_condition {
-                        type_condition_chain.push(type_condition);
-                    }
-                    selection_set_path.push(inline_fragment.selection_set_id);
-                    selections.extend(
-                        self.operation[inline_fragment.selection_set_id]
-                            .items
-                            .iter()
-                            .map(|selection| (type_condition_chain.clone(), selection_set_path.clone(), selection)),
-                    );
+            }
+            BoundSelection::FragmentSpread(spread_id) => {
+                let spread = &operation[*spread_id];
+                let fragment = &operation[spread.fragment_id];
+                type_condition_chain.push(fragment.type_condition);
+                selection_set_path.push(spread.selection_set_id);
+                selections.extend(
+                    operation[spread.selection_set_id]
+                        .items
+                        .iter()
+                        .map(|selection| (type_condition_chain.clone(), selection_set_path.clone(), selection)),
+                );
+            }
+            BoundSelection::InlineFragment(inline_fragment_id) => {
+                let inline_fragment = &operation[*inline_fragment_id];
+                if let Some(type_condition) = inline_fragment.type_condition {
+                    type_condition_chain.push(type_condition);
                 }
+                selection_set_path.push(inline_fragment.selection_set_id);
+                selections.extend(
+                    operation[inline_fragment.selection_set_id]
+                        .items
+                        .iter()
+                        .map(|selection| (type_condition_chain.clone(), selection_set_path.clone(), selection)),
+                );
             }
         }
-
-        flat_selection_set
     }
+
+    flat_selection_set
 }
 
 #[derive(Debug, Clone)]

engine/crates/engine-v2/src/plan/mod.rs

@@ -1,18 +1,20 @@
 use schema::{ResolverId, Schema};
 
 use crate::{
-    request::{EntityType, FlatTypeCondition, OpInputValues, Operation, QueryPath},
+    request::{OpInputValues, Operation, QueryPath},
     response::ReadSelectionSet,
     sources::Plan,
     utils::IdRange,
 };
 
 mod collected;
+mod flat;
 mod ids;
 mod planning;
 mod state;
 mod walkers;
 pub(crate) use collected::*;
+pub(crate) use flat::*;
 pub(crate) use ids::*;
 pub(crate) use planning::*;
 pub(crate) use state::*;

engine/crates/engine-v2/src/plan/planning/boundary.rs

@@ -7,10 +7,9 @@ use schema::{FieldId, FieldResolverWalker, FieldSet, FieldSetItem, ResolverId, R
 
 use super::{logic::PlanningLogic, planner::Planner, PlanningError, PlanningResult};
 use crate::{
-    plan::{ParentToChildEdge, PlanId},
+    plan::{flatten_selection_sets, EntityType, FlatField, FlatSelectionSet, ParentToChildEdge, PlanId},
     request::{
-        BoundField, BoundFieldId, BoundSelection, BoundSelectionSet, BoundSelectionSetId, EntityType, FlatField,
-        FlatSelectionSet, QueryPath, SelectionSetType,
+        BoundField, BoundFieldId, BoundSelection, BoundSelectionSet, BoundSelectionSetId, QueryPath, SelectionSetType,
     },
     response::{ReadField, ReadSelectionSet, UnpackedResponseEdge},
 };
@@ -375,7 +374,7 @@ impl<'schema, 'a> BoundaryPlanner<'schema, 'a> {
                         .iter()
                         .filter_map(|id| self.operation[*id].selection_set_id())
                         .collect();
-                    let flat_selection_set = self.walker().flatten_selection_sets(subselection_set_ids);
+                    let flat_selection_set = flatten_selection_sets(self.schema, &self.operation, subselection_set_ids);
                     let fields = self.create_boundary_fields(flat_selection_set)?;
                     boundary_field.lazy_subselection = Some(fields)
                 }

engine/crates/engine-v2/src/plan/planning/collect.rs

@@ -4,13 +4,12 @@ use std::collections::HashSet;
 
 use crate::{
     plan::{
-        AnyCollectedSelectionSet, AnyCollectedSelectionSetId, CollectedField, CollectedFieldId, CollectedSelectionSet,
-        CollectedSelectionSetId, ConditionalField, ConditionalFieldId, ConditionalSelectionSet,
-        ConditionalSelectionSetId, FieldType, OperationPlan, PlanBoundaryId, PlanId,
-    },
-    request::{
-        BoundFieldId, BoundSelectionSetId, EntityType, FlatField, FlatTypeCondition, OperationWalker, SelectionSetType,
+        flatten_selection_sets, AnyCollectedSelectionSet, AnyCollectedSelectionSetId, CollectedField, CollectedFieldId,
+        CollectedSelectionSet, CollectedSelectionSetId, ConditionalField, ConditionalFieldId, ConditionalSelectionSet,
+        ConditionalSelectionSetId, EntityType, FieldType, FlatField, FlatTypeCondition, OperationPlan, PlanBoundaryId,
+        PlanId,
     },
+    request::{BoundFieldId, BoundSelectionSetId, OperationWalker, SelectionSetType},
     utils::IdRange,
 };
 
@@ -78,7 +77,7 @@ impl<'schema, 'a> Collector<'schema, 'a> {
         selection_set_ids: Vec<BoundSelectionSetId>,
         concrete_parent: bool,
     ) -> PlanningResult<AnyCollectedSelectionSet> {
-        let selection_set = self.walker().flatten_selection_sets(selection_set_ids);
+        let selection_set = flatten_selection_sets(self.schema, self.operation, selection_set_ids);
 
         let mut maybe_boundary_id = None;
         let mut plan_fields = Vec::new();

engine/crates/engine-v2/src/plan/planning/planner.rs

@@ -13,10 +13,13 @@ use super::{
     PlanningError, PlanningResult,
 };
 use crate::{
-    plan::{OperationPlan, ParentToChildEdge, PlanBoundaryId, PlanId, PlanInput, PlanOutput, PlannedResolver},
+    plan::{
+        flatten_selection_sets, EntityType, FlatField, FlatSelectionSet, FlatTypeCondition, OperationPlan,
+        ParentToChildEdge, PlanBoundaryId, PlanId, PlanInput, PlanOutput, PlannedResolver,
+    },
     request::{
-        BoundField, BoundFieldId, BoundSelection, BoundSelectionSet, BoundSelectionSetId, EntityType, FlatField,
-        FlatSelectionSet, FlatTypeCondition, Operation, OperationWalker, QueryPath,
+        BoundField, BoundFieldId, BoundSelection, BoundSelectionSet, BoundSelectionSetId, Operation, OperationWalker,
+        QueryPath,
     },
     response::{ReadSelectionSet, ResponseKeys, SafeResponseKey},
     sources::Plan,
@@ -122,21 +125,20 @@ impl<'schema> Planner<'schema> {
         // The root plan is always introspection which also lets us handle operations like:
         // query { __typename }
         let introspection_resolver_id = self.schema.introspection_resolver_id();
-        let (introspection_selection_set, selection_set) = self
-            .walker()
-            .flatten_selection_sets(vec![self.operation.root_selection_set_id])
-            .partition_fields(|flat_field| {
-                let bound_field = &self.operation[flat_field.bound_field_id];
-                if let Some(schema_field_id) = bound_field.schema_field_id() {
-                    self.schema
-                        .walker()
-                        .walk(schema_field_id)
-                        .resolvers()
-                        .any(|FieldResolverWalker { resolver, .. }| resolver.id() == introspection_resolver_id)
-                } else {
-                    true
-                }
-            });
+        let (introspection_selection_set, selection_set) =
+            flatten_selection_sets(self.schema, &self.operation, vec![self.operation.root_selection_set_id])
+                .partition_fields(|flat_field| {
+                    let bound_field = &self.operation[flat_field.bound_field_id];
+                    if let Some(schema_field_id) = bound_field.schema_field_id() {
+                        self.schema
+                            .walker()
+                            .walk(schema_field_id)
+                            .resolvers()
+                            .any(|FieldResolverWalker { resolver, .. }| resolver.id() == introspection_resolver_id)
+                    } else {
+                        true
+                    }
+                });
 
         if !introspection_selection_set.is_empty() {
             self.push_plan(
@@ -252,7 +254,7 @@ impl<'schema> Planner<'schema> {
                 let schema_field_id = self.operation[bound_field_ids[0]]
                     .schema_field_id()
                     .expect("wouldn't have a subselection");
-                let flat_selection_set = self.walker().flatten_selection_sets(subselection_set_ids);
+                let flat_selection_set = flatten_selection_sets(self.schema, &self.operation, subselection_set_ids);
                 self.attribute_selection_sets(&flat_selection_set.root_selection_set_ids, plan_id);
                 self.recursive_plan_subselections(&path.child(key), &logic.child(schema_field_id), flat_selection_set)?;
             }

engine/crates/engine-v2/src/plan/state.rs

@@ -2,12 +2,11 @@ use std::sync::Arc;
 
 use schema::Schema;
 
-use crate::request::FlatTypeCondition;
 use crate::response::{ResponseBoundaryItem, ResponseBuilder};
 
 use crate::plan::{OperationPlan, PlanBoundaryId};
 
-use super::PlanId;
+use super::{FlatTypeCondition, PlanId};
 
 /// Holds the current state of the operation execution:
 /// - which plans have been executed

engine/crates/engine-v2/src/plan/walkers/mod.rs

@@ -4,16 +4,13 @@ use schema::{FieldId, ObjectId, Schema, SchemaWalker};
 
 use crate::{
     plan::{CollectedField, FieldType, RuntimeMergedConditionals},
-    request::{
-        BoundFieldId, FlatTypeCondition, OpInputValues, Operation, OperationWalker, SelectionSetType,
-        VariableDefinitionId,
-    },
+    request::{BoundFieldId, OpInputValues, Operation, OperationWalker, SelectionSetType, VariableDefinitionId},
     response::{ResponseEdge, ResponseKey, ResponseKeys, ResponsePart, ResponsePath, SafeResponseKey, SeedContext},
 };
 
 use super::{
-    AnyCollectedSelectionSet, CollectedSelectionSetId, ConditionalSelectionSetId, OperationPlan, PlanId, PlanInput,
-    PlanOutput, RuntimeCollectedSelectionSet,
+    AnyCollectedSelectionSet, CollectedSelectionSetId, ConditionalSelectionSetId, FlatTypeCondition, OperationPlan,
+    PlanId, PlanInput, PlanOutput, RuntimeCollectedSelectionSet,
 };
 
 mod argument;

engine/crates/engine-v2/src/request/bind/coercion/value.rs

@@ -4,8 +4,9 @@ use schema::{
     Type, Wrapping,
 };
 
-use crate::request::{bind::Binder, BoundFieldId, Location, OpInputValue, OpInputValueId};
+use crate::request::{BoundFieldId, Location, OpInputValue, OpInputValueId};
 
+use super::super::Binder;
 use super::{
     error::InputValueError,
     path::{value_path_to_string, ValuePathSegment},