Enhance graph node's execution context

Each node receives their parent and child nodes for a simpler
exec context logic.

pkg/envoy/graph.go

@@ -17,8 +17,8 @@ type (
 	// When it comes to larger setups (custom CRM) the number of nodes and relations is well below 1000.
 	// Due to the simple interface, we can define a more optimal graph implementation as a sepperate module.
 	Graph struct {
-		// Nodes defines a set of all available structures conforming to the Node interface
-		Nodes []Node
+		// nodes defines a set of all available structures conforming to the Node interface
+		nodes []Node
 		// Since everything is calculated on-the-fly, we need a simple boolean flag to determine if the graph is inverted
 		invert bool
 		// Allows us to keep track of all the nodes that were determined as conflicting.
@@ -26,11 +26,10 @@ type (
 		// If the data input is consistent, the conflicting nodes will be consistent, but there is no guarantee
 		// what node in a cycle will be selected as a conflicting.
 		conflicts set
+		// Allows us to easily keep track of nodes that were already processed.
+		processed set
 	}
 
-	// NodeRelation specifies what nodes from what resource this node is in relation with.
-	// The mapping is in the form of { resource: [nodeID, nodeID, ...] }
-	NodeRelation map[string][]string
 	// A simple "set" implementation for simpler, quicker checks
 	set map[string]bool
 
@@ -48,42 +47,74 @@ type (
 		// Relations provides a list of node's relations
 		// This can be calculated on the fly based on the node's state and don't need to be
 		// built in to the node struct.
-		Relations() NodeRelation
+		Relations() map[string][]string
 
 		// Run should implement the actual operation that should be performend when the node is invoked.
 		// This can be as simple or as complex as needed
-		Run(context.Context) error
+		// It's also provided a set of child and parent nodes so we can easily provide node context
+		Run(ctx context.Context, cc []Node, pp []Node) error
 
 		// ResolveConflict should implement any operation that should occur when the node
 		// causes a dependency conflict. For example -- partially import the records (without relations)
 		// and correct those relations when executing the Run function
-		ResolveConflict(context.Context) error
+		// It's also provided a set of child and parent nodes so we can easily provide node context
+		ResolveConflict(ctx context.Context, cc []Node, pp []Node) error
 	}
 )
 
 // Add registers a given set of nodes nn into the graph g
 func (g *Graph) Add(nn ...Node) {
-	g.Nodes = append(g.Nodes, nn...)
+	g.nodes = append(g.nodes, nn...)
 }
 
 // Remove removes the set of nodes nn from the graph h
 func (g *Graph) Remove(nn ...Node) {
-	mm := make([]Node, 0, len(g.Nodes)-len(nn))
-	for _, m := range g.Nodes {
-		mh := NodeHash(m)
+	if len(nn) <= 0 {
+		return
+	}
+
+	mm := make([]Node, 0, len(g.nodes))
+	for _, m := range g.nodes {
 		for _, n := range nn {
-			if mh != NodeHash(n) {
-				mm = append(mm, m)
+			if NodeHash(m) == NodeHash(n) && g.canRemove(n) {
+				goto skip
 			}
 		}
+		mm = append(mm, m)
+
+	skip:
+	}
+	g.nodes = mm
+}
+
+func (g *Graph) canRemove(n Node) bool {
+	if len(g.nodes) <= 1 {
+		return true
+	}
+
+	// When the first unprocessed child node is found, terminate
+	for _, m := range g.Children(n) {
+		if !g.processed[NodeHash(m)] {
+			return false
+		}
+	}
+
+	return true
+}
+
+func (g *Graph) MarkProcessed(nn ...Node) {
+	if g.processed == nil {
+		g.processed = make(set)
+	}
+	for _, n := range nn {
+		g.processed[NodeHash(n)] = true
 	}
-	g.Nodes = mm
 }
 
 // FindNode returns all nodes that match the given resource and identifiers
 func (g *Graph) FindNode(res string, IDs ...string) []Node {
 	nn := make([]Node, 0, len(IDs))
-	for _, n := range g.Nodes {
+	for _, n := range g.nodes {
 		for _, ID := range IDs {
 			if NodeHash(n) == NodeHashRaw(res, ID) {
 				nn = append(nn, n)
@@ -108,15 +139,24 @@ func (g *Graph) SetNodeConflict(n Node) {
 	g.conflicts[NodeHash(n)] = true
 }
 
+func (g *Graph) removeProcessed(nn []Node) []Node {
+	mm := make([]Node, 0, len(nn))
+	for _, n := range nn {
+		if !g.processed[NodeHash(n)] {
+			mm = append(mm, n)
+		}
+	}
+	return mm
+}
+
 // Children provides a list of children of the node n
 //
 // Child nodes are calculated on the fly based on node Relations()
 func (g *Graph) Children(n Node) []Node {
 	if !g.invert {
-		return g.children(n)
+		return g.removeProcessed(g.children(n))
 	}
-
-	return g.parents(n)
+	return g.removeProcessed(g.parents(n))
 }
 
 func (g *Graph) children(n Node) []Node {
@@ -128,22 +168,32 @@ func (g *Graph) children(n Node) []Node {
 	return nn
 }
 
+func (g *Graph) ChildrenA(n Node) []Node {
+	if !g.invert {
+		return g.children(n)
+	}
+	return g.parents(n)
+}
+
 // Parents provides a list of parents of the node n
 //
 // Parent nodes are calculated on the fly based on node Relations()
 func (g *Graph) Parents(n Node) []Node {
 	if !g.invert {
-		return g.parents(n)
+		return g.removeProcessed(g.parents(n))
 	}
-
-	return g.children(n)
+	return g.removeProcessed(g.children(n))
 }
 
 func (g *Graph) parents(n Node) []Node {
 	nn := make([]Node, 0)
 
-	for _, m := range g.Nodes {
-		if IDs, has := m.Relations()[n.Resource()]; has {
+	for _, m := range g.nodes {
+		r := m.Relations()
+		if r == nil {
+			continue
+		}
+		if IDs, has := r[n.Resource()]; has {
 			for _, ID := range IDs {
 				if n.ID() == ID {
 					nn = append(nn, m)
@@ -169,12 +219,29 @@ func (g *Graph) ParentsC(n Node) []Node {
 	return mm
 }
 
+func (g *Graph) ParentsA(n Node) []Node {
+	if !g.invert {
+		return g.parents(n)
+	}
+	return g.children(n)
+}
+
 // Validate performs a basic data validation over all the nodes.
 func (g *Graph) Validate() error {
 	// @todo...
 	return nil
 }
 
+func (g *Graph) Nodes() []Node {
+	nn := make([]Node, 0, len(g.nodes))
+	for _, n := range g.nodes {
+		if !g.processed[NodeHash(n)] {
+			nn = append(nn, n)
+		}
+	}
+	return nn
+}
+
 // Run invokes all operations while respecting relations (dependencies) and solving
 // dependency conflicts.
 //
@@ -185,10 +252,10 @@ func (g *Graph) Validate() error {
 //
 // The order of above operations is a bit more complex, but the general flow is that.
 func (g *Graph) Run(ctx context.Context) error {
-	for len(g.Nodes) > 0 {
+	for len(g.nodes) > 0 {
 		// Find all root nodes in the current graph state; those nodes are allowed to run.
-		nn := make([]Node, 0, len(g.Nodes)/2)
-		for _, n := range g.Nodes {
+		nn := make([]Node, 0, len(g.Nodes())/2)
+		for _, n := range g.Nodes() {
 			// We should not take into account conflicted parent nodes, as they already resolved
 			// the conflict.
 			if len(g.ParentsC(n)) == 0 {
@@ -216,11 +283,16 @@ func (g *Graph) Run(ctx context.Context) error {
 // are allowed to run.
 func (g *Graph) runRegular(ctx context.Context, nn []Node) error {
 	for _, n := range nn {
-		err := n.Run(ctx)
+		err := n.Run(ctx, g.Children(n), g.ParentsA(n))
 		if err != nil {
 			return err
 		}
 
+		// When a child node is processed check if it's parent can be removed.
+		// A parent node can only be removed if all of it's child nodes have already been
+		// processed
+		g.MarkProcessed(n)
+		g.Remove(g.ParentsA(n)...)
 		g.Remove(n)
 	}
 
@@ -240,14 +312,14 @@ func (g *Graph) runResolution(ctx context.Context) error {
 	// For example: A -> B -> A -> c -> D; where A B C is the cycle and C is a branch from the cycle.
 	// The code will works just fine, but it won't be that optimal so it should be improved to do
 	// actual cycle detection.
-	for _, m := range g.Nodes {
+	for _, m := range g.Nodes() {
 		if !g.conflicts[NodeHash(m)] {
 			n = m
 			break
 		}
 	}
 
-	err := n.ResolveConflict(ctx)
+	err := n.ResolveConflict(ctx, g.Children(n), g.Parents(n))
 	if err != nil {
 		return err
 	}

pkg/envoy/graph_test.go

@@ -12,7 +12,7 @@ var (
 type (
 	NodeTest struct {
 		Id        string
-		relations NodeRelation
+		relations map[string][]string
 		exOrder   *[]string
 	}
 )
@@ -25,16 +25,16 @@ func (n *NodeTest) Resource() string {
 	return "test"
 }
 
-func (n *NodeTest) Relations() NodeRelation {
+func (n *NodeTest) Relations() map[string][]string {
 	return n.relations
 }
 
-func (n *NodeTest) Run(ctx context.Context) error {
+func (n *NodeTest) Run(ctx context.Context, cc []Node, pp []Node) error {
 	ops = append(ops, "R:"+n.ID())
 	return nil
 }
 
-func (n *NodeTest) ResolveConflict(ctx context.Context) error {
+func (n *NodeTest) ResolveConflict(ctx context.Context, cc []Node, pp []Node) error {
 	ops = append(ops, "C:"+n.ID())
 	return nil
 }
@@ -50,8 +50,8 @@ func TestGraphStructure_relations(t *testing.T) {
 		{
 			name: "N1 -> N2",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N2"}}},
-				{Id: "N2", relations: NodeRelation{}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N2"}}},
+				{Id: "N2", relations: map[string][]string{}},
 			},
 			node:     "N1",
 			children: []string{"N2"},
@@ -60,8 +60,8 @@ func TestGraphStructure_relations(t *testing.T) {
 		{
 			name: "N1 -> N2",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N2"}}},
-				{Id: "N2", relations: NodeRelation{}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N2"}}},
+				{Id: "N2", relations: map[string][]string{}},
 			},
 			node:     "N2",
 			children: []string{},
@@ -70,7 +70,7 @@ func TestGraphStructure_relations(t *testing.T) {
 		{
 			name: "N1 -> N1 :: cycle to self",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N1"}}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N1"}}},
 			},
 			node:     "N1",
 			children: []string{"N1"},
@@ -79,8 +79,8 @@ func TestGraphStructure_relations(t *testing.T) {
 		{
 			name: "N1 -> N1 -> N2 <- N2",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N1", "N2"}}},
-				{Id: "N2", relations: NodeRelation{"test": []string{"N2"}}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N1", "N2"}}},
+				{Id: "N2", relations: map[string][]string{"test": []string{"N2"}}},
 			},
 			node:     "N1",
 			children: []string{"N1", "N2"},
@@ -89,8 +89,8 @@ func TestGraphStructure_relations(t *testing.T) {
 		{
 			name: "N1 -> N1 -> N2 <- N2",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N1", "N2"}}},
-				{Id: "N2", relations: NodeRelation{"test": []string{"N2"}}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N1", "N2"}}},
+				{Id: "N2", relations: map[string][]string{"test": []string{"N2"}}},
 			},
 			node:     "N2",
 			children: []string{"N2"},
@@ -104,8 +104,8 @@ func TestGraphStructure_relations(t *testing.T) {
 			g.Add(n)
 		}
 
-		cc := g.Children(g.findNode("test", test.node)[0])
-		pp := g.Parents(g.findNode("test", test.node)[0])
+		cc := g.Children(g.FindNode("test", test.node)[0])
+		pp := g.Parents(g.FindNode("test", test.node)[0])
 
 		if len(cc) != len(test.children) {
 			t.Errorf("[%s] node child missmatch; list range doesnt match; exp=%d got=%d", test.name, len(test.children), len(cc))
@@ -142,8 +142,8 @@ func TestGraphStructure_inversion(t *testing.T) {
 		{
 			name: "N1 -> N2",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N2"}}},
-				{Id: "N2", relations: NodeRelation{}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N2"}}},
+				{Id: "N2", relations: map[string][]string{}},
 			},
 			node:     "N1",
 			children: []string{},
@@ -152,8 +152,8 @@ func TestGraphStructure_inversion(t *testing.T) {
 		{
 			name: "N1 -> N2",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N2"}}},
-				{Id: "N2", relations: NodeRelation{}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N2"}}},
+				{Id: "N2", relations: map[string][]string{}},
 			},
 			node:     "N2",
 			children: []string{"N1"},
@@ -162,7 +162,7 @@ func TestGraphStructure_inversion(t *testing.T) {
 		{
 			name: "N1 -> N1 :: cycle to self",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N1"}}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N1"}}},
 			},
 			node:     "N1",
 			children: []string{"N1"},
@@ -178,8 +178,8 @@ func TestGraphStructure_inversion(t *testing.T) {
 
 		g.Invert()
 
-		cc := g.Children(g.findNode("test", test.node)[0])
-		pp := g.Parents(g.findNode("test", test.node)[0])
+		cc := g.Children(g.FindNode("test", test.node)[0])
+		pp := g.Parents(g.FindNode("test", test.node)[0])
 
 		if len(cc) != len(test.children) {
 			t.Errorf("[%s] node child missmatch; list range doesnt match; exp=%d got=%d", test.name, len(test.children), len(cc))
@@ -214,33 +214,40 @@ func TestGraphStructure_execution(t *testing.T) {
 		{
 			name: "N1 -> N2",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N2"}}},
-				{Id: "N2", relations: NodeRelation{}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N2"}}},
+				{Id: "N2", relations: map[string][]string{}},
 			},
 			ops: []string{"R:N2", "R:N1"},
 		},
+		{
+			name: "N1 -> N1",
+			nodes: []*NodeTest{
+				{Id: "N1", relations: map[string][]string{"test": []string{"N1"}}},
+			},
+			ops: []string{"C:N1", "R:N1"},
+		},
 		{
 			name: "N1 -> N1 -> N2",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N2", "N1"}}},
-				{Id: "N2", relations: NodeRelation{}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N2", "N1"}}},
+				{Id: "N2", relations: map[string][]string{}},
 			},
 			ops: []string{"R:N2", "C:N1", "R:N1"},
 		},
 		{
 			name: "N2 -> N1 -> N1",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N1"}}},
-				{Id: "N2", relations: NodeRelation{"test": []string{"N1"}}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N1"}}},
+				{Id: "N2", relations: map[string][]string{"test": []string{"N1"}}},
 			},
 			ops: []string{"C:N1", "R:N1", "R:N2"},
 		},
 		{
 			name: "N1 -> N1 <-> N2 <-> N3 <- N1",
 			nodes: []*NodeTest{
-				{Id: "N1", relations: NodeRelation{"test": []string{"N2", "N1", "N3"}}},
-				{Id: "N2", relations: NodeRelation{"test": []string{"N1", "N3"}}},
-				{Id: "N3", relations: NodeRelation{"test": []string{"N2"}}},
+				{Id: "N1", relations: map[string][]string{"test": []string{"N2", "N1", "N3"}}},
+				{Id: "N2", relations: map[string][]string{"test": []string{"N1", "N3"}}},
+				{Id: "N3", relations: map[string][]string{"test": []string{"N2"}}},
 			},
 			ops: []string{"C:N1", "C:N2", "R:N3", "R:N1", "R:N2"},
 		},
@@ -253,6 +260,9 @@ func TestGraphStructure_execution(t *testing.T) {
 			g.Add(n)
 		}
 
+		// spew.Dump(g.nodes)
+		// spew.Dump("___", ops)
+
 		g.Invert()
 		g.Run(context.Background())
 
@@ -261,27 +271,5 @@ func TestGraphStructure_execution(t *testing.T) {
 				t.Errorf("[%s] operation missmatch; exp=%s got=%s", test.name, o, ops[i])
 			}
 		}
-
-		// if len(cc) != len(test.children) {
-		// 	t.Errorf("[%s] node child missmatch; list range doesnt match; exp=%d got=%d", test.name, len(test.children), len(cc))
-		// 	return
-		// }
-		// for i, c := range cc {
-		// 	if c.ID() != test.children[i] {
-		// 		t.Errorf("[%s] node child missmatch; exp=%s got=%s pos=%d", test.name, test.children[i], c.ID(), i)
-		// 		return
-		// 	}
-		// }
-
-		// if len(pp) != len(test.parents) {
-		// 	t.Errorf("[%s] node parent missmatch; list range doesnt match; exp=%d got=%d", test.name, len(test.parents), len(pp))
-		// 	return
-		// }
-		// for i, p := range pp {
-		// 	if p.ID() != test.parents[i] {
-		// 		t.Errorf("[%s] node parent missmatch; exp=%s got=%s pos=%d", test.name, test.parents[i], p.ID(), i)
-		// 		return
-		// 	}
-		// }
 	}
 }