diff --git a/engine/graph/autogroup/autogroup_test.go b/engine/graph/autogroup/autogroup_test.go index 5c3fb899..4ae9f9a7 100644 --- a/engine/graph/autogroup/autogroup_test.go +++ b/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") - e2 := NE("e2,e3") // e3 gets "merged through" to BOTH edges! - g2.AddEdge(a, b1, e1) - g2.AddEdge(b1, c1, e2) + e1 := NE("e1") + e2 := NE("e2") + e3 := NE("e3") + 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 -// \ | / | -// \ b1 / >>> b1,b2 (arrows point downwards) -// \ | / | -// c1 c1 +// TODO: verify this second possibility manually +// a2 a1 b2 a1,a2 a1,a2 +// \ | / | | \ +// \ b1 / >>> b1,b2 OR b1,b2 / (arrows point downwards) +// \ | / | | / +// 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") - e2 := NE("e2,e3,e4") // e3 gets "merged through" to BOTH edges! - g2.AddEdge(a, b, e1) - g2.AddEdge(b, c1, e2) + e1 := NE("e1") + e2 := NE("e2,e4") + e3 := NE("e3") + g3.AddEdge(a, b, e1) + g3.AddEdge(b, c1, e2) + g3.AddEdge(a, c1, e3) } - runGraphCmp(t, g1, g2) + runGraphCmp(t, g1, g3) } /* diff --git a/engine/graph/autogroup/base.go b/engine/graph/autogroup/base.go index 756968a6..488f1a3e 100644 --- a/engine/graph/autogroup/base.go +++ b/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.vertices = ag.graph.VerticesSorted() // cache in deterministic order! + ag.graph = g // pointer + + // 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 diff --git a/engine/graph/autogroup/util.go b/engine/graph/autogroup/util.go index 99b8194d..7e0508a7 100644 --- a/engine/graph/autogroup/util.go +++ b/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 +} diff --git a/lang/interpret_test/TestAstFunc3/sendrecv-autogroup.txtar b/lang/interpret_test/TestAstFunc3/sendrecv-autogroup.txtar index 2f2e9160..f9088f5e 100644 --- a/lang/interpret_test/TestAstFunc3/sendrecv-autogroup.txtar +++ b/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[value3] -> print[print2] # value[value3] -> print[print3] +Edge: value[value2] -> print[print3] # value[value2] -> print[print2] +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[print2]: Vertex: print[print3] +Group: print[print3]: Field: print[print2].Msg = "i am value2" +Group: print[print3]: Vertex: print[print2] Vertex: print[print1] -Vertex: print[print2] +Vertex: print[print3] Vertex: value[value1] Vertex: value[value2] Vertex: value[value3]