engine: graph: autogroup: Improve the autogrouping algorithm

This improves the autogrouping algorithm to support hierarchical
autogrouping. It's not guaranteed to work if we replace the reachability
grouper with something more efficient, but it's good enough for now.

engine/graph/autogroup/autogroup_test.go

@@ -741,17 +741,30 @@ func TestPgraphGrouping16(t *testing.T) {
 		g1.AddEdge(b1, c1, e2)
 		g1.AddEdge(a2, c1, e3)
 	}
-	g2, _ := pgraph.NewGraph("g2") // expected result
+	//g2, _ := pgraph.NewGraph("g2") // expected result
+	//{
+	//	a := NewNoopResTest("a1,a2")
+	//	b1 := NewNoopResTest("b1")
+	//	c1 := NewNoopResTest("c1")
+	//	e1 := NE("e1,e3")
+	//	e2 := NE("e2,e3") // e3 gets "merged through" to BOTH edges!
+	//	g2.AddEdge(a, b1, e1)
+	//	g2.AddEdge(b1, c1, e2)
+	//}
+	//runGraphCmp(t, g1, g2)
+	g3, _ := pgraph.NewGraph("g3") // alternative expected result
 	{
 		a := NewNoopResTest("a1,a2")
 		b1 := NewNoopResTest("b1")
 		c1 := NewNoopResTest("c1")
-		e1 := NE("e1,e3")
+		e1 := NE("e1")
-		e2 := NE("e2,e3") // e3 gets "merged through" to BOTH edges!
+		e2 := NE("e2")
-		g2.AddEdge(a, b1, e1)
+		e3 := NE("e3")
-		g2.AddEdge(b1, c1, e2)
+		g3.AddEdge(a, b1, e1)
+		g3.AddEdge(b1, c1, e2)
+		g3.AddEdge(a, c1, e3)
 	}
-	runGraphCmp(t, g1, g2)
+	runGraphCmp(t, g1, g3)
 }
 
 /*
@@ -792,11 +805,12 @@ func TestPgraphGrouping17(t *testing.T) {
 /*
 // re-attach 3 (double)
 // similar to "re-attach 1", technically there is a second possibility for this
-// a2   a1    b2         a1,a2
+// TODO: verify this second possibility manually
-//   \   |   /             |
+// a2   a1    b2         a1,a2        a1,a2
-//    \ b1  /      >>>   b1,b2   (arrows point downwards)
+//   \   |   /             |            |   \
-//     \ | /               |
+//    \ b1  /      >>>   b1,b2   OR   b1,b2 /   (arrows point downwards)
-//      c1                c1
+//     \ | /               |            |  /
+//      c1                c1           c1
 */
 func TestPgraphGrouping18(t *testing.T) {
 	g1, _ := pgraph.NewGraph("g1") // original graph
@@ -815,17 +829,30 @@ func TestPgraphGrouping18(t *testing.T) {
 		g1.AddEdge(a2, c1, e3)
 		g1.AddEdge(b2, c1, e4)
 	}
-	g2, _ := pgraph.NewGraph("g2") // expected result
+	//g2, _ := pgraph.NewGraph("g2") // expected result
+	//{
+	//	a := NewNoopResTest("a1,a2")
+	//	b := NewNoopResTest("b1,b2")
+	//	c1 := NewNoopResTest("c1")
+	//	e1 := NE("e1,e3")
+	//	e2 := NE("e2,e3,e4") // e3 gets "merged through" to BOTH edges!
+	//	g2.AddEdge(a, b, e1)
+	//	g2.AddEdge(b, c1, e2)
+	//}
+	//runGraphCmp(t, g1, g2)
+	g3, _ := pgraph.NewGraph("g3") // alternative expected result
 	{
 		a := NewNoopResTest("a1,a2")
 		b := NewNoopResTest("b1,b2")
 		c1 := NewNoopResTest("c1")
-		e1 := NE("e1,e3")
+		e1 := NE("e1")
-		e2 := NE("e2,e3,e4") // e3 gets "merged through" to BOTH edges!
+		e2 := NE("e2,e4")
-		g2.AddEdge(a, b, e1)
+		e3 := NE("e3")
-		g2.AddEdge(b, c1, e2)
+		g3.AddEdge(a, b, e1)
+		g3.AddEdge(b, c1, e2)
+		g3.AddEdge(a, c1, e3)
 	}
-	runGraphCmp(t, g1, g2)
+	runGraphCmp(t, g1, g3)
 }
 
 /*

engine/graph/autogroup/base.go

@@ -43,8 +43,24 @@ func (ag *baseGrouper) Init(g *pgraph.Graph) error {
 	if ag.graph != nil {
 		return fmt.Errorf("the init method has already been called")
 	}
-	ag.graph = g                            // pointer
+	ag.graph = g // pointer
-	ag.vertices = ag.graph.VerticesSorted() // cache in deterministic order!
+
+	// We sort deterministically, first by kind, and then by name. In
+	// particular, longer kind chunks sort first. So http:ui:text should
+	// appear before http:server and http:ui. This is a hack so that if we
+	// are doing hierarchical automatic grouping, it gives the http:ui:text
+	// a chance to get grouped into http:ui, before http:ui gets grouped
+	// into http:server, because once that happens, http:ui:text will never
+	// get grouped, and this won't work properly. This works, because when
+	// we start comparing iteratively the list of resources, it does this
+	// with a O(n^2) loop that compares the X and Y zero indexes first, and
+	// and then continues along. If the "longer" resources appear first,
+	// then they'll group together first. We should probably put this into
+	// a new Grouper struct, but for now we might as well leave it here.
+	//vertices := ag.graph.VerticesSorted() // formerly
+	vertices := RHVSort(ag.graph.Vertices())
+
+	ag.vertices = vertices // cache in deterministic order!
 	ag.i = 0
 	ag.j = 0
 	if len(ag.vertices) == 0 { // empty graph

engine/graph/autogroup/util.go

@@ -19,6 +19,8 @@ package autogroup
 
 import (
 	"fmt"
+	"sort"
+	"strings"
 
 	"github.com/purpleidea/mgmt/engine"
 	"github.com/purpleidea/mgmt/pgraph"
@@ -136,3 +138,67 @@ func VertexMerge(g *pgraph.Graph, v1, v2 pgraph.Vertex, vertexMergeFn func(pgrap
 	}
 	return nil // success
 }
+
+// RHVSlice is a linear list of vertices. It can be sorted by the Kind, taking
+// into account the hierarchy of names separated by colons. Afterwards, it uses
+// String() to avoid the non-determinism in the map type. RHV stands for Reverse
+// Hierarchical Vertex, meaning the hierarchical topology of the vertex
+// (resource) names are used.
+type RHVSlice []pgraph.Vertex
+
+// Len returns the length of the slice of vertices.
+func (obj RHVSlice) Len() int { return len(obj) }
+
+// Swap swaps two elements in the slice.
+func (obj RHVSlice) Swap(i, j int) { obj[i], obj[j] = obj[j], obj[i] }
+
+// Less returns the smaller element in the sort order according to the
+// aforementioned rules.
+// XXX: Add some tests to make sure I didn't get any "reverse" part backwards.
+func (obj RHVSlice) Less(i, j int) bool {
+	resi, oki := obj[i].(engine.Res)
+	resj, okj := obj[j].(engine.Res)
+	if !oki || !okj || resi.Kind() == "" || resj.Kind() == "" {
+		// One of these isn't a normal Res, so just compare normally.
+		return obj[i].String() > obj[j].String() // reverse
+	}
+
+	si := strings.Split(resi.Kind(), ":")
+	sj := strings.Split(resj.Kind(), ":")
+	// both lengths should each be at least one now
+	li := len(si)
+	lj := len(sj)
+
+	if li != lj { // eg: http:ui vs. http:ui:text
+		return li > lj // reverse
+	}
+
+	// same number of chunks
+	for k := 0; k < li; k++ {
+		if si[k] != sj[k] { // lhs chunk differs
+			return si[k] > sj[k] // reverse
+		}
+
+		// if the chunks are the same, we continue...
+	}
+
+	// They must all have the same chunks, so finally we compare the names.
+	return resi.Name() > resj.Name() // reverse
+}
+
+// Sort is a convenience method.
+func (obj RHVSlice) Sort() { sort.Sort(obj) }
+
+// RHVSort returns a deterministically sorted slice of all vertices in the list.
+// The order is sorted by the Kind, taking into account the hierarchy of names
+// separated by colons. Afterwards, it uses String() to avoid the
+// non-determinism in the map type. RHV stands for Reverse Hierarchical Vertex,
+// meaning the hierarchical topology of the vertex (resource) names are used.
+func RHVSort(vertices []pgraph.Vertex) []pgraph.Vertex {
+	var vs []pgraph.Vertex
+	for _, v := range vertices { // copy first
+		vs = append(vs, v)
+	}
+	sort.Sort(RHVSlice(vs)) // add determinism
+	return vs
+}

lang/interpret_test/TestAstFunc3/sendrecv-autogroup.txtar

@@ -33,17 +33,17 @@ Value["value2"].any -> Print["print2"].msg
 Value["value3"].any -> Print["print3"].msg
 -- OUTPUT --
 Edge: value[value1] -> print[print1] # value[value1] -> print[print1]
-Edge: value[value2] -> print[print2] # value[value2] -> print[print2]
+Edge: value[value2] -> print[print3] # value[value2] -> print[print2]
-Edge: value[value3] -> print[print2] # value[value3] -> print[print3]
+Edge: value[value3] -> print[print3] # value[value3] -> print[print3]
 Field: print[print1].Msg = "i am value1"
-Field: print[print2].Msg = "i am value2"
+Field: print[print3].Msg = "i am value3"
 Field: value[value1].Any = "i am value1"
 Field: value[value2].Any = "i am value2"
 Field: value[value3].Any = "i am value3"
-Group: print[print2]: Field: print[print3].Msg = "i am value3"
+Group: print[print3]: Field: print[print2].Msg = "i am value2"
-Group: print[print2]: Vertex: print[print3]
+Group: print[print3]: Vertex: print[print2]
 Vertex: print[print1]
-Vertex: print[print2]
+Vertex: print[print3]
 Vertex: value[value1]
 Vertex: value[value2]
 Vertex: value[value3]