Skip to content

Commit

Permalink
feat: start separation between dependency-graph and expression-manage…
Browse files Browse the repository at this point in the history
…ment
  • Loading branch information
lukasjarosch committed Apr 9, 2024
1 parent c61301d commit 90fa1fe
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 55 deletions.
5 changes: 4 additions & 1 deletion expression.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
package skipper

type ExpressionManager struct{}
import "github.com/lukasjarosch/skipper/expression"

Check failure on line 3 in expression.go

View workflow job for this annotation

GitHub Actions / golangci-lint

could not import github.com/lukasjarosch/skipper/expression (-: # github.com/lukasjarosch/skipper/expression

// TODO: move to expression/dep.go ???
type ExpressionRegistry map[string]*expression.ExpressionNode
44 changes: 28 additions & 16 deletions expression/dep.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import (
"errors"
"fmt"

"github.com/davecgh/go-spew/spew"
"github.com/dominikbraun/graph"
)

// expressionNodeHash returns a string which allows the graph to
// distinguish expression nodes.
func expressionNodeHash(expr *ExpressionNode) string {
return expr.Text()
}
// PathMap maps path strings (dot-separated) and maps it to the expressions which occur at the path.
type PathMap map[string][]*ExpressionNode

// VariableMap holds variable-name to value mappings
type VariableMap map[string]any

var dependencyGraph graph.Graph[string, *ExpressionNode] = graph.New(expressionNodeHash, graph.Acyclic(), graph.Directed(), graph.PreventCycles())

Expand All @@ -29,15 +30,21 @@ func InitializeDependencyGraph(expressions []*ExpressionNode) error {
return nil
}

// PathMap maps path strings (dot-separated) and maps it to the expressions which occur at the path.
type PathMap map[string][]*ExpressionNode
// expressionNodeHash returns a string which allows the graph to
// distinguish expression nodes.
func expressionNodeHash(expr *ExpressionNode) string {
return expr.Text()
}

// Dependencies takes an expression node and a [PathMap]
// and returns those on which the passed expression depends on.
// Dependencies takes an expression node, a [PathMap] and a [VariableMap]
// and returns the skipper paths from within the allExpressions PathMap on which the expression depends on.
//
// An ExpressionNode is dependent on another if
// it has a PathNode at which's value another expression exists.
func Dependencies(expr *ExpressionNode, allExpressions PathMap, variables map[string]interface{}) ([]*ExpressionNode, error) {
// An ExpressionNode is dependent on another if it has a PathNode
// of which the skipper path (dot-separated) matches a key in the PathMap.
// Thus, the ExpressionNode is dependent on all ExpressionNodes of that path.
//
// TODO: return a MapMap instead of only a slice to prevent information loss
func Dependencies(expr *ExpressionNode, allExpressions PathMap, variables VariableMap) ([]string, error) {
pathNodes := PathsInExpression(expr)
resolvedPathNodes := make([]*PathNode, len(pathNodes))

Expand All @@ -53,10 +60,12 @@ func Dependencies(expr *ExpressionNode, allExpressions PathMap, variables map[st

// If the skipper path occurs as key in allExpressions,
// then all those expressions are direct dependencies to the current one.
var dependingOnExpressions []*ExpressionNode
for _, path := range resolvedPathNodes {
if expr, ok := allExpressions[path.SkipperPath().String()]; ok {
dependingOnExpressions = append(dependingOnExpressions, expr...)
var dependingOnExpressions []string
for _, pathNode := range resolvedPathNodes {
spew.Dump(pathNode.Text())
pathMapKey := pathNode.SkipperPath().String()
if _, ok := allExpressions[pathMapKey]; ok {
dependingOnExpressions = append(dependingOnExpressions, pathMapKey)
}
}

Expand All @@ -65,6 +74,9 @@ func Dependencies(expr *ExpressionNode, allExpressions PathMap, variables map[st

// PathsInExpression returns all PathNodes which occur in the expression
func PathsInExpression(expr *ExpressionNode) (paths []*PathNode) {
if expr == nil {
return nil
}
pathsInCall := func(call *CallNode) (paths []*PathNode) {
for _, argNode := range call.Arguments {
switch node := argNode.(type) {
Expand Down
53 changes: 42 additions & 11 deletions expression/dep_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,49 @@ func TestInitializeDependencyGraph(t *testing.T) {
}

func TestDependencies(t *testing.T) {
pathMap := expression.PathMap{
"some.path": expression.Parse(`${some:$target:path}`),
"some.other.path": expression.Parse(`${yet:another:$variable_path}`),
tests := []struct {
name string
errExpected error
subject *expression.ExpressionNode
pathMap expression.PathMap
variableMap expression.VariableMap
expected []string
}{
{
name: "One dependency",
subject: expression.Parse("${foo:bar}")[0], // <- note that
pathMap: expression.PathMap{
"foo.bar": expression.Parse("${bar:baz}"),
"bar.baz": expression.Parse("hello"), // not a dependency, scalar value
},
expected: []string{"foo.bar"},
},
{
name: "Multiple dependencies",
subject: expression.Parse("${default(foo:bar, bar:baz) || set_env(foo:qux)}")[0],
pathMap: expression.PathMap{
"foo.bar": expression.Parse("${bar:baz}"),
"bar.baz": expression.Parse("${foo:qux}"),
"foo.qux": expression.Parse("${ohai:there}"),
"ohai.there": expression.Parse("ohai"),
},
expected: []string{"foo.bar", "bar.baz", "foo.qux"},
},
}
variableMap := map[string]any{
"target": "other",
}
expected := []*expression.ExpressionNode{}
expected = append(expected, pathMap["some.other.path"]...)

deps, err := expression.Dependencies(pathMap["some.path"][0], pathMap, variableMap)
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ret, err := expression.Dependencies(tt.subject, tt.pathMap, tt.variableMap)

assert.NoError(t, err)
assert.ElementsMatch(t, expected, deps)
if tt.errExpected != nil {
assert.ErrorIs(t, err, tt.errExpected)
return
}

assert.NoError(t, err)
assert.Equal(t, ret, tt.expected)

return
})
}
}
9 changes: 1 addition & 8 deletions expression/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ func ResolveVariablePath(path PathNode, varMap map[string]any) (newPathNode *Pat
// TODO: what about the positions?
pathExpr := fmt.Sprintf("${%s}", strings.Join(newSegmentStrings, ":"))

// handle parser or lexer panics gracefully
defer func() {
if r := recover(); r != nil {
if e, ok := r.(error); ok {
Expand All @@ -114,14 +115,6 @@ func ResolveVariablePath(path PathNode, varMap map[string]any) (newPathNode *Pat
return newPathNode, nil
}

func Map(vs []string, f func(string) string) []string {
vsm := make([]string, len(vs))
for i, v := range vs {
vsm[i] = f(v)
}
return vsm
}

func ResolveVariable(varNode *VariableNode, varMap map[string]any) (interface{}, error) {
if varNode == nil {
return nil, fmt.Errorf("nil VariableNode")
Expand Down
119 changes: 119 additions & 0 deletions graph/dependency.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package graph

import (
"errors"
"fmt"
"strings"

"github.com/dominikbraun/graph"
)

type dependencyGraph graph.Graph[string, string]

var (
ErrSelfReferencingDependency = fmt.Errorf("self-referencing dependency")
ErrCyclicReference = fmt.Errorf("cyclic dependency")
)

type DependencyGraph struct {
graph dependencyGraph
}

func NewDependencyGraph() *DependencyGraph {
g := graph.New(graph.StringHash, graph.Acyclic(), graph.Directed(), graph.PreventCycles())
return &DependencyGraph{graph: g}
}

func (dg *DependencyGraph) AddVertex(vertexHash string) error {
err := dg.graph.AddVertex(vertexHash)
if err != nil {
// ignore duplicate vertex errors
if !errors.Is(err, graph.ErrVertexAlreadyExists) {
return fmt.Errorf("cannot add vertex: %w", err)
}
}

return nil
}

func (dg *DependencyGraph) RegisterDependencies(dependerVertexHash string, dependeeVertexHashes []string) error {
for _, dependency := range dependeeVertexHashes {
dependeeVertex, err := dg.graph.Vertex(dependency)
if err != nil {
return fmt.Errorf("unexpectedly could not fetch dependency vertex %s: %w", dependency, err)
}

// prevent self-referencing references
if strings.EqualFold(dependerVertexHash, dependeeVertex) {
return fmt.Errorf("%s: %w", dependerVertexHash, ErrSelfReferencingDependency)
}

err = dg.graph.AddEdge(dependerVertexHash, dependeeVertex)
if err != nil {
if errors.Is(err, graph.ErrEdgeCreatesCycle) {
return fmt.Errorf("%s -> %s: %w", dependerVertexHash, dependeeVertex, ErrCyclicReference)
}
return fmt.Errorf("failed to register dependency %s: %w", dependency, err)
}
}

return nil
}

func (dg *DependencyGraph) RemoveVertex(vertexHash string) error {
edges, err := dg.graph.Edges()
if err != nil {
return err
}

// Find all edges with either originate from or point to the reference and remove them.
for _, edge := range edges {
if edge.Source == vertexHash {
err = dg.graph.RemoveEdge(edge.Source, edge.Target)
if err != nil {
return err
}
continue
}
if edge.Target == vertexHash {
err = dg.graph.RemoveEdge(edge.Source, edge.Target)
if err != nil {
return err
}
continue
}
}

return dg.graph.RemoveVertex(vertexHash)
}

func (dg *DependencyGraph) TopologicalSort() (vertexHashes []string, err error) {
// Perform a stable topological sort of the dependency graph.
// The returned orderedHashes is stable in that the hashes are sorted
// by their length or alphabetically if they are the same length.
// This eliminates the issue that the actual topological sorting algorithm usually
// has multiple valid solutions.
orderedHashes, err := graph.StableTopologicalSort[string, string](dg.graph, func(s1, s2 string) bool {
// Strings are of different length, sort by length
if len(s1) != len(s2) {
return len(s1) < len(s2)
}
// Otherwise, sort alphabetically
return s1 > s2
})
if err != nil {
return nil, fmt.Errorf("failed to sort reference graph: %w", err)
}

// The result of the topological sorting is reverse to what we want.
// We expect the vertex without any dependencies to be at the top.
for i := len(orderedHashes) - 1; i >= 0; i-- {
ref, err := dg.graph.Vertex(orderedHashes[i])
if err != nil {
return nil, err
}
vertexHashes = append(vertexHashes, ref)
}

return
}
25 changes: 25 additions & 0 deletions graph/visualize.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package graph

import (
"os"

"github.com/dominikbraun/graph"
"github.com/dominikbraun/graph/draw"
)

// Visualize is mainly a debugging function which will render a DOT file
// of the graph into the given filePath and add the label as graph description.
func Visualize[K comparable, T any](graph graph.Graph[K, T], filePath string, label string) error {
file, err := os.Create(filePath)
if err != nil {
return err
}
err = draw.DOT(graph, file,
draw.GraphAttribute("label", label),
draw.GraphAttribute("overlap", "prism"))
if err != nil {
return err
}

return nil
}
19 changes: 0 additions & 19 deletions reference/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ package reference
import (
"errors"
"fmt"
"os"
"regexp"
"strings"

"github.com/dominikbraun/graph"
"github.com/dominikbraun/graph/draw"

"github.com/lukasjarosch/skipper/data"
)
Expand Down Expand Up @@ -417,20 +415,3 @@ func DeduplicateValueReferences(references []ValueReference) []ValueReference {

return deduplicated
}

// VisualizeDependencyGraph is mainly a debugging function which will render a DOT file
// of the dependencyGraph into the given filePath and add the label as graph description.
func VisualizeDependencyGraph(graph graph.Graph[string, ValueReference], filePath string, label string) error {
file, err := os.Create(filePath)
if err != nil {
return err
}
err = draw.DOT(graph, file,
draw.GraphAttribute("label", label),
draw.GraphAttribute("overlap", "prism"))
if err != nil {
return err
}

return nil
}

0 comments on commit 90fa1fe

Please sign in to comment.