Skip to content

Commit

Permalink
Fail on output(depended-by relationship in a DAG) from an undefined node
Browse files Browse the repository at this point in the history
  • Loading branch information
mumoshu committed Dec 17, 2021
1 parent 445f86c commit b0537b4
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 11 deletions.
54 changes: 43 additions & 11 deletions pkg/dag/dag.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,32 @@ type DAG struct {
cap int
initNodes []string

nodes []string
outputs map[string]map[string]bool
labels map[string]map[string]bool
nodes []string
// a.k.a dependents of the node denoted by the key
// `outputs["api"]["web"] = true` means api's sole dependent is "web"
// i.e. "web" depends on "api"
outputs map[string]map[string]bool
labels map[string]map[string]bool
// a.k.a number of dependenciesthat the node denoted by the key has.
// `numInputs["web"] = 2` means "web" has 2 dependencies.
numInputs map[string]int
}

func (g *DAG) AddNode(name string) bool {
if _, ok := g.outputs[name]; ok {
if _, ok := g.numInputs[name]; ok {
return false
}

g.nodes = append(g.nodes, name)
g.outputs[name] = make(map[string]bool)
g.numInputs[name] = 0

if _, ok := g.outputs[name]; !ok {
g.outputs[name] = make(map[string]bool)
}

if _, ok := g.numInputs[name]; ok {
g.numInputs[name] = 0
}

return true
}

Expand Down Expand Up @@ -76,7 +89,8 @@ func (g *DAG) AddNodes(names ...string) bool {
func (g *DAG) AddEdge(from, to string) bool {
m, ok := g.outputs[from]
if !ok {
return false
m = map[string]bool{}
g.outputs[from] = m
}

m[to] = true
Expand Down Expand Up @@ -140,10 +154,6 @@ func (g *DAG) Add(node string, opt ...AddOption) bool {

g.AddNode(node)

for _, d := range opts.deps {
g.Add(d)
}

deps := g.AddDependencies(node, opts.deps)

g.AddLabels(node, opts.labels)
Expand Down Expand Up @@ -244,6 +254,15 @@ func (e *Error) Error() string {
return fmt.Sprintf("cycle detected: %v", e.Cycle)
}

type UndefinedDependencyError struct {
UndefinedNode string
Dependents []string
}

func (e *UndefinedDependencyError) Error() string {
return fmt.Sprintf("undefined node %q is depended by node(s): %s", e.UndefinedNode, strings.Join(e.Dependents, ", "))
}

type UnhandledDependencyError struct {
UnhandledDependencies []UnhandledDependency
}
Expand Down Expand Up @@ -367,6 +386,19 @@ func (g *DAG) Sort(opts ...SortOption) (Topology, error) {
}
}

for dep, dependents := range outputs {
if _, ok := nodes[dep]; !ok {
var dependentsNames []string
for d := range dependents {
dependentsNames = append(dependentsNames, d)
}
return nil, &UndefinedDependencyError{
UndefinedNode: dep,
Dependents: dependentsNames,
}
}
}

// We sort sets of nodes rather than nodes themselves,
// so that we know which items can be processed in parallel in the DAG
// See https://cs.stackexchange.com/questions/2524/getting-parallel-items-in-dependency-resolution
Expand Down
29 changes: 29 additions & 0 deletions pkg/dag/dag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package dag

import (
"bytes"
"fmt"
"log"
"testing"
)
Expand Down Expand Up @@ -94,6 +95,8 @@ func TestDAG_DagCleanAPI(t *testing.T) {
g2.Add("api", Dependencies([]string{"db", "cache", "net"}))
g2.Add("db", Dependencies([]string{"net"}))
g2.Add("mesh", Dependencies([]string{"net"}))
g2.Add("net")
g2.Add("cache")

res, err := g2.Plan()
if err != nil {
Expand Down Expand Up @@ -291,6 +294,8 @@ func TestDAG_Dag_Dot(t *testing.T) {
g2.Add("release/api", Dependencies([]string{"release/db", "release/cache", "release/net"}), Labels([]string{"tier:api"}))
g2.Add("release/db", Dependencies([]string{"release/net"}), Labels([]string{"tier:db"}))
g2.Add("release/mesh", Dependencies([]string{"release/net"}), Labels([]string{"tier:net"}))
g2.Add("release/net")
g2.Add("release/cache")

w := &bytes.Buffer{}

Expand Down Expand Up @@ -365,3 +370,27 @@ func TestDAG_GraphAPICycle(t *testing.T) {
t.Errorf("unexpected result: expected=%q, got=%q", expected, actual)
}
}

func TestDAG_UndefinedDependency_MaybeTypo(t *testing.T) {
g2 := New()
g2.Add(
"web",
Dependencies([]string{
"ok",
"ng",
}),
)
g2.Add(
"ok",
)

_, err := g2.Plan()

if err == nil {
t.Fatalf("expected error didnt occur")
}

if fmt.Sprintf("%v", err) != `undefined node "ng" is depended by node(s): web` {
t.Fatalf("%v", err)
}
}

0 comments on commit b0537b4

Please sign in to comment.