terraform: remove ConnectDependents and related interfaces

terraform/graph.go

@@ -5,7 +5,6 @@ import (
 	"log"
 	"runtime/debug"
 	"strings"
-	"sync"
 
 	"github.com/hashicorp/terraform/dag"
 )
@@ -17,8 +16,7 @@ const RootModuleName = "root"
 var RootModulePath = []string{RootModuleName}
 
 // Graph represents the graph that Terraform uses to represent resources
-// and their dependencies. Each graph represents only one module, but it
+// and their dependencies.
-// can contain further modules, which themselves have their own graph.
 type Graph struct {
 	// Graph is the actual DAG. This is embedded so you can call the DAG
 	// methods directly.
@@ -29,127 +27,16 @@ type Graph struct {
 	// RootModuleName
 	Path []string
 
-	// dependableMap is a lookaside table for fast lookups for connecting
-	// dependencies by their GraphNodeDependable value to avoid O(n^3)-like
-	// situations and turn them into O(1) with respect to the number of new
-	// edges.
-	dependableMap map[string]dag.Vertex
-
 	// debugName is a name for reference in the debug output. This is usually
 	// to indicate what topmost builder was, and if this graph is a shadow or
 	// not.
 	debugName string
-
-	once sync.Once
 }
 
 func (g *Graph) DirectedGraph() dag.Grapher {
 	return &g.AcyclicGraph
 }
 
-// Add is the same as dag.Graph.Add.
-func (g *Graph) Add(v dag.Vertex) dag.Vertex {
-	g.once.Do(g.init)
-
-	// Call upwards to add it to the actual graph
-	g.Graph.Add(v)
-
-	// If this is a depend-able node, then store the lookaside info
-	if dv, ok := v.(GraphNodeDependable); ok {
-		for _, n := range dv.DependableName() {
-			g.dependableMap[n] = v
-		}
-	}
-
-	return v
-}
-
-// Remove is the same as dag.Graph.Remove
-func (g *Graph) Remove(v dag.Vertex) dag.Vertex {
-	g.once.Do(g.init)
-
-	// If this is a depend-able node, then remove the lookaside info
-	if dv, ok := v.(GraphNodeDependable); ok {
-		for _, n := range dv.DependableName() {
-			delete(g.dependableMap, n)
-		}
-	}
-
-	// Call upwards to remove it from the actual graph
-	return g.Graph.Remove(v)
-}
-
-// Replace is the same as dag.Graph.Replace
-func (g *Graph) Replace(o, n dag.Vertex) bool {
-	g.once.Do(g.init)
-
-	// Go through and update our lookaside to point to the new vertex
-	for k, v := range g.dependableMap {
-		if v == o {
-			if _, ok := n.(GraphNodeDependable); ok {
-				g.dependableMap[k] = n
-			} else {
-				delete(g.dependableMap, k)
-			}
-		}
-	}
-
-	return g.Graph.Replace(o, n)
-}
-
-// ConnectDependent connects a GraphNodeDependent to all of its
-// GraphNodeDependables. It returns the list of dependents it was
-// unable to connect to.
-func (g *Graph) ConnectDependent(raw dag.Vertex) []string {
-	v, ok := raw.(GraphNodeDependent)
-	if !ok {
-		return nil
-	}
-
-	return g.ConnectTo(v, v.DependentOn())
-}
-
-// ConnectDependents goes through the graph, connecting all the
-// GraphNodeDependents to GraphNodeDependables. This is safe to call
-// multiple times.
-//
-// To get details on whether dependencies could be found/made, the more
-// specific ConnectDependent should be used.
-func (g *Graph) ConnectDependents() {
-	for _, v := range g.Vertices() {
-		if dv, ok := v.(GraphNodeDependent); ok {
-			g.ConnectDependent(dv)
-		}
-	}
-}
-
-// ConnectFrom creates an edge by finding the source from a DependableName
-// and connecting it to the specific vertex.
-func (g *Graph) ConnectFrom(source string, target dag.Vertex) {
-	g.once.Do(g.init)
-
-	if source := g.dependableMap[source]; source != nil {
-		g.Connect(dag.BasicEdge(source, target))
-	}
-}
-
-// ConnectTo connects a vertex to a raw string of targets that are the
-// result of DependableName, and returns the list of targets that are missing.
-func (g *Graph) ConnectTo(v dag.Vertex, targets []string) []string {
-	g.once.Do(g.init)
-
-	var missing []string
-	for _, t := range targets {
-		if dest := g.dependableMap[t]; dest != nil {
-			g.Connect(dag.BasicEdge(v, dest))
-		} else {
-			missing = append(missing, t)
-		}
-	}
-
-	return missing
-}
-
 // Walk walks the graph with the given walker for callbacks. The graph
 // will be walked with full parallelism, so the walker should expect
 // to be called in concurrently.
@@ -157,12 +44,6 @@ func (g *Graph) Walk(walker GraphWalker) error {
 	return g.walk(walker)
 }
 
-func (g *Graph) init() {
-	if g.dependableMap == nil {
-		g.dependableMap = make(map[string]dag.Vertex)
-	}
-}
-
 func (g *Graph) walk(walker GraphWalker) error {
 	// The callbacks for enter/exiting a graph
 	ctx := walker.EnterPath(g.Path)
@@ -289,20 +170,3 @@ func (g *Graph) walk(walker GraphWalker) error {
 
 	return g.AcyclicGraph.Walk(walkFn)
 }
-
-// GraphNodeDependable is an interface which says that a node can be
-// depended on (an edge can be placed between this node and another) according
-// to the well-known name returned by DependableName.
-//
-// DependableName can return multiple names it is known by.
-type GraphNodeDependable interface {
-	DependableName() []string
-}
-
-// GraphNodeDependent is an interface which says that a node depends
-// on another GraphNodeDependable by some name. By implementing this
-// interface, Graph.ConnectDependents() can be called multiple times
-// safely and efficiently.
-type GraphNodeDependent interface {
-	DependentOn() []string
-}

terraform/graph_dot_test.go

@@ -33,23 +33,27 @@ digraph {
 				root := &testDrawableOrigin{"root"}
 				g.Add(root)
 
-				levelOne := []string{"foo", "bar"}
+				levelOne := []interface{}{"foo", "bar"}
-				for _, s := range levelOne {
+				for i, s := range levelOne {
-					g.Add(&testDrawable{
+					levelOne[i] = &testDrawable{
-						VertexName:      s,
+						VertexName: s.(string),
-						DependentOnMock: []string{"root"},
+					}
-					})
+					v := levelOne[i]
+
+					g.Add(v)
+					g.Connect(dag.BasicEdge(v, root))
 				}
 
 				levelTwo := []string{"baz", "qux"}
 				for i, s := range levelTwo {
-					g.Add(&testDrawable{
+					v := &testDrawable{
 						VertexName: s,
-						DependentOnMock: levelOne[i : i+1],
-					})
 					}
 
-				g.ConnectDependents()
+					g.Add(v)
+					g.Connect(dag.BasicEdge(v, levelOne[i]))
+				}
+
 				return &g
 			},
 			Expect: `
@@ -81,22 +85,23 @@ digraph {
 				root := &testDrawableOrigin{"root"}
 				g.Add(root)
 
-				g.Add(&testDrawable{
+				vA := g.Add(&testDrawable{
 					VertexName: "A",
-					DependentOnMock: []string{"root", "C"},
 				})
 
-				g.Add(&testDrawable{
+				vB := g.Add(&testDrawable{
 					VertexName: "B",
-					DependentOnMock: []string{"A"},
 				})
 
-				g.Add(&testDrawable{
+				vC := g.Add(&testDrawable{
 					VertexName: "C",
-					DependentOnMock: []string{"B"},
 				})
 
-				g.ConnectDependents()
+				g.Connect(dag.BasicEdge(vA, root))
+				g.Connect(dag.BasicEdge(vA, vC))
+				g.Connect(dag.BasicEdge(vB, vA))
+				g.Connect(dag.BasicEdge(vC, vB))
+
 				return &g
 			},
 			Expect: `
@@ -131,23 +136,23 @@ digraph {
 				g.Add(root)
 
 				var sub Graph
-				sub.Add(&testDrawableOrigin{"sub_root"})
+				vSubRoot := sub.Add(&testDrawableOrigin{"sub_root"})
 
 				var subsub Graph
 				subsub.Add(&testDrawableOrigin{"subsub_root"})
-				sub.Add(&testDrawableSubgraph{
+				vSubV := sub.Add(&testDrawableSubgraph{
 					VertexName:   "subsub",
 					SubgraphMock: &subsub,
-					DependentOnMock: []string{"sub_root"},
-				})
-				g.Add(&testDrawableSubgraph{
-					VertexName:      "sub",
-					SubgraphMock:    &sub,
-					DependentOnMock: []string{"root"},
 				})
 
-				g.ConnectDependents()
+				vSub := g.Add(&testDrawableSubgraph{
-				sub.ConnectDependents()
+					VertexName:   "sub",
+					SubgraphMock: &sub,
+				})
+
+				g.Connect(dag.BasicEdge(vSub, root))
+				sub.Connect(dag.BasicEdge(vSubV, vSubRoot))
+
 				return &g
 			},
 			Expect: `
@@ -184,23 +189,22 @@ digraph {
 				g.Add(root)
 
 				var sub Graph
-				sub.Add(&testDrawableOrigin{"sub_root"})
+				rootSub := sub.Add(&testDrawableOrigin{"sub_root"})
 
 				var subsub Graph
 				subsub.Add(&testDrawableOrigin{"subsub_root"})
-				sub.Add(&testDrawableSubgraph{
+
+				subV := sub.Add(&testDrawableSubgraph{
 					VertexName:   "subsub",
 					SubgraphMock: &subsub,
-					DependentOnMock: []string{"sub_root"},
 				})
-				g.Add(&testDrawableSubgraph{
+				vSub := g.Add(&testDrawableSubgraph{
 					VertexName:   "sub",
 					SubgraphMock: &sub,
-					DependentOnMock: []string{"root"},
 				})
 
-				g.ConnectDependents()
+				g.Connect(dag.BasicEdge(vSub, root))
-				sub.ConnectDependents()
+				sub.Connect(dag.BasicEdge(subV, rootSub))
 				return &g
 			},
 			Expect: `

terraform/graph_test.go

@@ -1,76 +1,11 @@
 package terraform
 
 import (
-	"reflect"
-	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform/dag"
 )
 
-func TestGraphAdd(t *testing.T) {
-	// Test Add since we override it and want to make sure we don't break it.
-	var g Graph
-	g.Add(42)
-	g.Add(84)
-
-	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(testGraphAddStr)
-	if actual != expected {
-		t.Fatalf("bad: %s", actual)
-	}
-}
-
-func TestGraphConnectDependent(t *testing.T) {
-	var g Graph
-	g.Add(&testGraphDependable{VertexName: "a"})
-	b := g.Add(&testGraphDependable{
-		VertexName:      "b",
-		DependentOnMock: []string{"a"},
-	})
-
-	if missing := g.ConnectDependent(b); len(missing) > 0 {
-		t.Fatalf("bad: %#v", missing)
-	}
-
-	actual := strings.TrimSpace(g.String())
-	expected := strings.TrimSpace(testGraphConnectDepsStr)
-	if actual != expected {
-		t.Fatalf("bad: %s", actual)
-	}
-}
-
-func TestGraphReplace_DependableWithNonDependable(t *testing.T) {
-	var g Graph
-	a := g.Add(&testGraphDependable{VertexName: "a"})
-	b := g.Add(&testGraphDependable{
-		VertexName:      "b",
-		DependentOnMock: []string{"a"},
-	})
-	newA := "non-dependable-a"
-
-	if missing := g.ConnectDependent(b); len(missing) > 0 {
-		t.Fatalf("bad: %#v", missing)
-	}
-
-	if !g.Replace(a, newA) {
-		t.Fatalf("failed to replace")
-	}
-
-	c := g.Add(&testGraphDependable{
-		VertexName:      "c",
-		DependentOnMock: []string{"a"},
-	})
-
-	// This should fail by reporting missing, since a node with dependable
-	// name "a" is no longer in the graph.
-	missing := g.ConnectDependent(c)
-	expected := []string{"a"}
-	if !reflect.DeepEqual(expected, missing) {
-		t.Fatalf("expected: %#v, got: %#v", expected, missing)
-	}
-}
-
 func TestGraphWalk_panicWrap(t *testing.T) {
 	var g Graph