From 96fca0a24daf531867ae56c79359f6c0bbbdd665 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Joulaud?= <francois.joulaud@radiofrance.com>
Date: Fri, 10 Jan 2025 16:18:39 +0100
Subject: [PATCH] clean(dag): cleanup unused Filter function

Following 7ef5705 the "Filter" function is not used anymore.

After thinking I think it will not ever be used in this form as it is
not generic and thus it is better to completely delete it.

Indeed the function is buggy, difficult to understand (and to fix). E.g. I
still have not grasped what it try to do with the orphans and their
childs.

I think (without being sure) the idea behind this was to be able to work
on a smaller graph for performance but as the small graph is used just
once, the cost of constructing it in the first place completely offsets
the potential gains.
---
 pkg/dag/dag.go      | 51 ---------------------------------------------
 pkg/dag/dag_test.go | 26 -----------------------
 2 files changed, 77 deletions(-)

diff --git a/pkg/dag/dag.go b/pkg/dag/dag.go
index c96ff31cc..7521cfa9a 100644
--- a/pkg/dag/dag.go
+++ b/pkg/dag/dag.go
@@ -107,57 +107,6 @@ func (d *DAG) WalkParallel(visitor NodeVisitorFunc) {
 	waitGroup.Wait()
 }
 
-// Filter creates a new DAG only populated by nodes that fulfil the condition checked by filterFunc.
-// All children nodes that no longer have any parents in the resulting graph will become a root node of the DAG.
-func (d *DAG) Filter(filterFunc func(*Node) bool) *DAG {
-	filteredGraph := DAG{}
-	replacements := make(map[*Node]*Node)
-	var orphans []*Node
-
-	d.Walk(func(node *Node) {
-		if !filterFunc(node) {
-			// The node does not fulfil the condition, skip it.
-			return
-		}
-
-		// Create a replacement node containing the same Image.
-		newNode := NewNode(node.Image)
-		replacements[node] = newNode
-		// Check if the node is orphan
-		for _, parent := range node.Parents() {
-			if _, ok := replacements[parent]; ok {
-				// The node has at least one parent, nothing to do.
-				return
-			}
-		}
-		// The node no longer has parents, we add it to the list of orphans.
-		orphans = append(orphans, node)
-	})
-
-	for _, orphan := range orphans {
-		orphan.walkInDepth(createUniqueVisitor(func(node *Node) {
-			replacement, ok := replacements[node]
-			if !ok {
-				// This node on the source graph should not be in the filtered graph.
-				return
-			}
-
-			// Add new child nodes
-			for _, child := range node.Children() {
-				childReplacement, ok := replacements[child]
-				if !ok {
-					// This child node on the source graph should not be in the filtered graph.
-					continue
-				}
-				replacement.AddChild(childReplacement)
-			}
-		}))
-		// Add the orphan node to the root nodes of the graph.
-		filteredGraph.AddNode(replacements[orphan])
-	}
-
-	return &filteredGraph
-}
 
 //nolint:musttag
 func (d *DAG) ListImage() string {
diff --git a/pkg/dag/dag_test.go b/pkg/dag/dag_test.go
index 15f93c688..2baf02b06 100644
--- a/pkg/dag/dag_test.go
+++ b/pkg/dag/dag_test.go
@@ -214,32 +214,6 @@ func Test_WalkParallel_RunsAllNodes(t *testing.T) {
 	assert.Equal(t, 6, length) // Total number of nodes is 6
 }
 
-func Test_Filter(t *testing.T) {
-	t.Parallel()
-
-	DAG := createDAG()
-
-	withoutParents := DAG.Filter(func(node *dag.Node) bool {
-		return len(node.Parents()) > 0 // Filter out nodes having parents.
-	})
-	assert.Len(t, withoutParents.Nodes(), 3)
-
-	withParents := DAG.Filter(func(node *dag.Node) bool {
-		return len(node.Parents()) == 0 // Filter out nodes having no parent.
-	})
-	assert.Len(t, withParents.Nodes(), 2)
-
-	withoutChildren := DAG.Filter(func(node *dag.Node) bool {
-		return len(node.Children()) > 0 // Filter out nodes having children.
-	})
-	assert.Len(t, withoutChildren.Nodes(), 2)
-
-	withChildren := DAG.Filter(func(node *dag.Node) bool {
-		return len(node.Children()) == 0 // Filter out nodes having no children.
-	})
-	assert.Len(t, withChildren.Nodes(), 3)
-}
-
 func Test_ListImage(t *testing.T) {
 	t.Parallel()