chore: simplify Processor.Process() (#1354)

Do no return anything from it since it is mutation of a plan anyway.
Cosmo already does not use the returned value.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Restructured query plan processing workflow to separate caching and
post-processing logic.
* Updated plan processor architecture with new internal components for
enhanced plan optimization.

* **Tests**
  * Updated test cases to reflect changes in plan processing flow.

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: ysmolski

execution/engine/execution_engine.go

@@ -260,9 +260,9 @@ func (e *ExecutionEngine) getCachedPlan(ctx *internalExecutionContext, operation
 		return nil
 	}
 
-	p := ctx.postProcessor.Process(planResult)
-	e.executionPlanCache.Add(cacheKey, p)
-	return p
+	ctx.postProcessor.Process(planResult)
+	e.executionPlanCache.Add(cacheKey, planResult)
+	return planResult
 }
 
 func (e *ExecutionEngine) GetWebsocketBeforeStartHook() WebsocketBeforeStartHook {

v2/pkg/engine/plan/visitor.go

@@ -1313,6 +1313,7 @@ func (v *Visitor) configureObjectFetch(config *objectFetchConfiguration) {
 	v.response.RawFetches = append(v.response.RawFetches, fetchItem)
 }
 
+// configureFetch builds and assembles all fields of resolve.SingleFetch.
 func (v *Visitor) configureFetch(internal *objectFetchConfiguration, external resolve.FetchConfiguration) *resolve.SingleFetch {
 	dataSourceType := reflect.TypeOf(external.DataSource).String()
 	dataSourceType = strings.TrimPrefix(dataSourceType, "*")

v2/pkg/engine/postprocess/postprocess.go

@@ -16,6 +16,8 @@ type FetchTreeProcessor interface {
 	ProcessFetchTree(root *resolve.FetchTreeNode)
 }
 
+// Processor transforms and optimizes the query plan after
+// it's been created by the planner but before execution.
 type Processor struct {
 	disableExtractFetches bool
 	collectDataSourceInfo bool
@@ -131,7 +133,11 @@ func NewProcessor(options ...ProcessorOption) *Processor {
 	}
 }
 
-func (p *Processor) Process(pre plan.Plan) plan.Plan {
+// Process takes a raw query plan and optimizes it by deduplicating fetches,
+// ordering them correctly by dependencies, and resolving any templated inputs.
+// It groups already-ordered fetches into parallel execution batches
+// when they have the same dependency requirements satisfied.
+func (p *Processor) Process(pre plan.Plan) {
 	switch t := pre.(type) {
 	case *plan.SynchronousResponsePlan:
 		for i := range p.processResponseTree {
@@ -162,10 +168,9 @@ func (p *Processor) Process(pre plan.Plan) plan.Plan {
 			p.processFetchTree[i].ProcessFetchTree(t.Response.Response.Fetches)
 		}
 	}
-	return pre
 }
 
-// createFetchTree creates an inital fetch tree from the raw fetches in the GraphQL response.
+// createFetchTree creates an initial fetch tree from the raw fetches in the GraphQL response.
 // The initial fetch tree is a node of sequence fetch kind, with a flat list of fetches as children.
 func (p *Processor) createFetchTree(res *resolve.GraphQLResponse) {
 	if p.disableExtractFetches {

v2/pkg/engine/postprocess/postprocess_test.go

@@ -16,14 +16,14 @@ import (
 func TestProcess_ExtractFetches(t *testing.T) {
 	type TestCase struct {
 		name     string
-		pre      plan.Plan
+		plan     plan.Plan
 		expected plan.Plan
 	}
 
 	cases := []TestCase{
 		{
 			name: "1",
-			pre: &plan.SynchronousResponsePlan{
+			plan: &plan.SynchronousResponsePlan{
 				Response: &resolve.GraphQLResponse{
 					RawFetches: []*resolve.FetchItem{
 						{Fetch: &resolve.SingleFetch{FetchDependencies: resolve.FetchDependencies{FetchID: 1}}},
@@ -64,7 +64,7 @@ func TestProcess_ExtractFetches(t *testing.T) {
 		},
 		{
 			name: "2",
-			pre: &plan.SynchronousResponsePlan{
+			plan: &plan.SynchronousResponsePlan{
 				Response: &resolve.GraphQLResponse{
 					RawFetches: []*resolve.FetchItem{
 						{
@@ -161,7 +161,7 @@ func TestProcess_ExtractFetches(t *testing.T) {
 		},
 		{
 			name: "3",
-			pre: &plan.SynchronousResponsePlan{
+			plan: &plan.SynchronousResponsePlan{
 				Response: &resolve.GraphQLResponse{
 					RawFetches: []*resolve.FetchItem{
 						{
@@ -274,7 +274,7 @@ func TestProcess_ExtractFetches(t *testing.T) {
 		},
 		{
 			name: "4",
-			pre: &plan.SynchronousResponsePlan{
+			plan: &plan.SynchronousResponsePlan{
 				Response: &resolve.GraphQLResponse{
 					RawFetches: []*resolve.FetchItem{
 						{
@@ -368,9 +368,9 @@ func TestProcess_ExtractFetches(t *testing.T) {
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			actual := processor.Process(c.pre)
+			processor.Process(c.plan)
 
-			if !assert.Equal(t, c.expected, actual) {
+			if !assert.Equal(t, c.expected, c.plan) {
 				formatterConfig := map[reflect.Type]interface{}{
 					reflect.TypeOf([]byte{}): func(b []byte) string { return fmt.Sprintf(`"%s"`, string(b)) },
 				}
@@ -381,7 +381,7 @@ func TestProcess_ExtractFetches(t *testing.T) {
 					Formatter:         formatterConfig,
 				}
 
-				if diff := prettyCfg.Compare(c.expected, actual); diff != "" {
+				if diff := prettyCfg.Compare(c.expected, c.plan); diff != "" {
 					t.Errorf("Plan does not match(-want +got)\n%s", diff)
 				}
 			}
@@ -392,14 +392,14 @@ func TestProcess_ExtractFetches(t *testing.T) {
 func TestProcess_ExtractServiceNames(t *testing.T) {
 	type TestCase struct {
 		name     string
-		pre      plan.Plan
+		plan     plan.Plan
 		expected plan.Plan
 	}
 
 	cases := []TestCase{
 		{
 			name: "Collect all service names",
-			pre: &plan.SynchronousResponsePlan{
+			plan: &plan.SynchronousResponsePlan{
 				Response: &resolve.GraphQLResponse{
 					RawFetches: []*resolve.FetchItem{
 						{
@@ -533,7 +533,7 @@ func TestProcess_ExtractServiceNames(t *testing.T) {
 		},
 		{
 			name: "Deduplicate the same service names",
-			pre: &plan.SynchronousResponsePlan{
+			plan: &plan.SynchronousResponsePlan{
 				Response: &resolve.GraphQLResponse{
 					RawFetches: []*resolve.FetchItem{
 						{
@@ -679,9 +679,9 @@ func TestProcess_ExtractServiceNames(t *testing.T) {
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			actual := processor.Process(c.pre)
+			processor.Process(c.plan)
 
-			if !assert.Equal(t, c.expected, actual) {
+			if !assert.Equal(t, c.expected, c.plan) {
 				formatterConfig := map[reflect.Type]interface{}{
 					reflect.TypeOf([]byte{}): func(b []byte) string { return fmt.Sprintf(`"%s"`, string(b)) },
 				}
@@ -692,7 +692,7 @@ func TestProcess_ExtractServiceNames(t *testing.T) {
 					Formatter:         formatterConfig,
 				}
 
-				if diff := prettyCfg.Compare(c.expected, actual); diff != "" {
+				if diff := prettyCfg.Compare(c.expected, c.plan); diff != "" {
 					t.Errorf("Plan does not match(-want +got)\n%s", diff)
 				}
 			}