Extract DAG creation out, address review comments

Author: rahulgurnani

pkg/epp/requestcontrol/dag.go

@@ -53,24 +53,23 @@ func buildDAG(plugins []PrepareDataPlugin) map[string][]string {
 	return dag
 }
 
-// prepareDataGraph builds the dependency graph and returns the plugins ordered in topological order.
+// sortPlugins builds the dependency graph and returns the plugins ordered in topological order.
 // If there is a cycle, it returns an error.
-func prepareDataGraph(plugins []PrepareDataPlugin) (map[string][]string, []PrepareDataPlugin, error) {
-	dag := buildDAG(plugins)
+func sortPlugins(dag map[string][]string, plugins []PrepareDataPlugin) ([]PrepareDataPlugin, error) {
 	nameToPlugin := map[string]PrepareDataPlugin{}
 	for _, plugin := range plugins {
 		nameToPlugin[plugin.TypedName().String()] = plugin
 	}
 	sortedPlugins, err := topologicalSort(dag)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 	orderedPlugins := []PrepareDataPlugin{}
 	for _, pluginName := range sortedPlugins {
 		orderedPlugins = append(orderedPlugins, nameToPlugin[pluginName])
 	}
 
-	return dag, orderedPlugins, err
+	return orderedPlugins, err
 }
 
 // TopologicalSort performs Kahn's Algorithm on a DAG.

pkg/epp/requestcontrol/dag_test.go

@@ -113,33 +113,33 @@ func TestPrepareDataGraph(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			dag, orderedPlugins, err := prepareDataGraph(tc.plugins)
+			dag := buildDAG(tc.plugins)
+			orderedPlugins, err := sortPlugins(dag, tc.plugins)
 
 			if tc.expectError {
 				assert.Error(t, err)
-				assert.Nil(t, dag)
 				assert.Contains(t, err.Error(), "cycle detected")
-			} else {
-				assert.NoError(t, err)
-
-				// Normalize the slices in the maps for consistent comparison
-				normalizedDAG := make(map[string][]string)
-				maps.Copy(normalizedDAG, dag)
-				normalizedExpectedDAG := make(map[string][]string)
-				for k, v := range tc.expectedDAG {
-					normalizedExpectedDAG[k] = v
-				}
-
-				if diff := cmp.Diff(normalizedExpectedDAG, normalizedDAG); diff != "" {
-					t.Errorf("prepareDataGraph() mismatch (-want +got):\n%s", diff)
-				}
-
-				orderedPluginNames := make([]string, len(orderedPlugins))
-				for i, p := range orderedPlugins {
-					orderedPluginNames[i] = p.TypedName().String()
-				}
-				assertTopologicalOrder(t, dag, orderedPlugins)
+				return
 			}
+			assert.NoError(t, err)
+
+			// Normalize the slices in the maps for consistent comparison
+			normalizedDAG := make(map[string][]string)
+			maps.Copy(normalizedDAG, dag)
+			normalizedExpectedDAG := make(map[string][]string)
+			for k, v := range tc.expectedDAG {
+				normalizedExpectedDAG[k] = v
+			}
+
+			if diff := cmp.Diff(normalizedExpectedDAG, normalizedDAG); diff != "" {
+				t.Errorf("prepareDataGraph() mismatch (-want +got):\n%s", diff)
+			}
+
+			orderedPluginNames := make([]string, len(orderedPlugins))
+			for i, p := range orderedPlugins {
+				orderedPluginNames[i] = p.TypedName().String()
+			}
+			assertTopologicalOrder(t, dag, orderedPlugins)
 		})
 	}
 }

pkg/epp/requestcontrol/request_control_config.go

@@ -108,7 +108,8 @@ func (c *Config) AddPlugins(pluginObjects ...plugins.Plugin) {
 // PrepareDataPluginGraph creates data dependency graph and sorts the plugins in topological order.
 // If a cycle is detected, it returns an error.
 func (c *Config) PrepareDataPluginGraph() error {
-	_, plugins, err := prepareDataGraph(c.prepareDataPlugins)
+	dag := buildDAG(c.prepareDataPlugins)
+	plugins, err := sortPlugins(dag, c.prepareDataPlugins)
 	if err != nil {
 		return err
 	}