Skip to content

Commit

Permalink
fix: schema graph logic handles duplicate edges (#4208)
Browse files Browse the repository at this point in the history
  • Loading branch information
matt2e authored Jan 28, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 2fd1992 commit d58a986
Showing 2 changed files with 51 additions and 32 deletions.
10 changes: 6 additions & 4 deletions common/schema/edges.go
Original file line number Diff line number Diff line change
@@ -3,6 +3,8 @@ package schema
import (
"slices"
"strings"

"golang.org/x/exp/maps"
)

// GraphNode provides inbound and outbound edges for a node.
@@ -63,15 +65,15 @@ func Graph(s *Schema) map[RefKey]GraphNode {

// outboundEdges returns all the outbound edges of a node.
func outboundEdges(n Node, ignoredRefs map[RefKey]bool) []RefKey {
out := []RefKey{}
out := map[RefKey]bool{}
if r, ok := n.(*Ref); ok {
out = append(out, r.ToRefKey())
out[r.ToRefKey()] = true
}
Visit(n, func(n Node, next func() error) error { //nolint:errcheck
if r, ok := n.(*Ref); ok && !ignoredRefs[r.ToRefKey()] {
out = append(out, r.ToRefKey())
out[r.ToRefKey()] = true
}
return next()
})
return out
return maps.Keys(out)
}
73 changes: 45 additions & 28 deletions common/schema/edges_test.go
Original file line number Diff line number Diff line change
@@ -30,7 +30,10 @@ module a {
+database calls a.db
export verb inboundWithExternalTypes(builtin.HttpRequest<Unit, b.Location, Unit>) builtin.HttpResponse<b.Address, String>
+ingress http GET /todo/destroy/{name}
+ingress http GET /todo/external/{name}
export verb inboundWithDupes(builtin.HttpRequest<Unit, b.Location, Unit>) builtin.HttpResponse<b.Location, String>
+ingress http GET /todo/dupes/{name}
}
module b {
export data Location {
@@ -74,74 +77,80 @@ module c {
expected := map[RefKey]GraphNode{}
// builtins
addExpectedNode(t, expected, "builtin.Empty",
[]string{},
[]string{},
nil,
nil,
)
addExpectedNode(t, expected, "builtin.Ref",
[]string{"builtin.CatchRequest"},
[]string{},
nil,
)
addExpectedNode(t, expected, "builtin.HttpRequest",
[]string{"a.inboundWithExternalTypes"},
[]string{},
[]string{"a.inboundWithExternalTypes", "a.inboundWithDupes"},
nil,
)
addExpectedNode(t, expected, "builtin.HttpResponse",
[]string{"a.inboundWithExternalTypes"},
[]string{},
[]string{"a.inboundWithExternalTypes", "a.inboundWithDupes"},
nil,
)
addExpectedNode(t, expected, "builtin.CatchRequest",
[]string{},
nil,
[]string{"builtin.Ref"},
)
addExpectedNode(t, expected, "builtin.FailedEvent",
[]string{},
[]string{},
nil,
nil,
)

// module a
addExpectedNode(t, expected, "a.employeeOfTheMonth",
[]string{}, []string{"a.User"})
nil,
[]string{"a.User"})
addExpectedNode(t, expected, "a.myFavoriteChild",
[]string{}, []string{"a.User"})
nil,
[]string{"a.User"})
addExpectedNode(t, expected, "a.db",
[]string{"a.getUsers"},
[]string{})
nil)
addExpectedNode(t, expected, "a.User",
[]string{"a.Event", "a.myFavoriteChild", "a.employeeOfTheMonth", "a.getUsers", "c.AliasedUser", "c.end"},
[]string{},
nil,
)
addExpectedNode(t, expected, "a.Event",
[]string{"a.postEvent"},
[]string{"a.User"})
addExpectedNode(t, expected, "a.empty",
[]string{},
[]string{})
nil,
nil)
addExpectedNode(t, expected, "a.postEvent",
[]string{}, []string{"a.Event"})
nil, []string{"a.Event"})
addExpectedNode(t, expected, "a.getUsers",
[]string{},
nil,
[]string{"a.User", "a.db"},
)
addExpectedNode(t, expected, "a.inboundWithExternalTypes",
[]string{},
nil,
[]string{"b.Location", "b.Address", "builtin.HttpRequest", "builtin.HttpResponse"},
)
addExpectedNode(t, expected, "a.inboundWithDupes",
nil,
[]string{"b.Location", "builtin.HttpRequest", "builtin.HttpResponse"},
)

// module b
addExpectedNode(t, expected, "b.Location",
[]string{"a.inboundWithExternalTypes", "b.consume", "b.locations", "c.middle", "c.start"},
[]string{},
[]string{"a.inboundWithExternalTypes", "a.inboundWithDupes", "b.consume", "b.locations", "c.middle", "c.start"},
nil,
)
addExpectedNode(t, expected, "b.Address",
[]string{"a.inboundWithExternalTypes", "c.middle"},
[]string{},
nil,
)
addExpectedNode(t, expected, "b.locations",
[]string{"b.consume"},
[]string{"b.Location"},
)
addExpectedNode(t, expected, "b.consume",
[]string{},
nil,
[]string{"b.Location", "b.locations"},
)

@@ -159,8 +168,8 @@ module c {
[]string{"a.User", "c.start"},
)
addExpectedNode(t, expected, "c.Color",
[]string{},
[]string{},
nil,
nil,
)
addExpectedNode(t, expected, "c.AliasedUser",
[]string{"c.start"},
@@ -185,8 +194,8 @@ module c {
assert.True(t, ok, "did not expect node %s, but got:\nIn: %v\nOut: %v", ref, graph[ref].In, graph[ref].Out)
graphNode, ok := graph[ref]
assert.True(t, ok, "expected node %s but graph did not include it", ref)
assert.Equal(t, expectedNode.In, graphNode.In, "inbound edges for %s should match", ref)
assert.Equal(t, expectedNode.Out, graphNode.Out, "outbound edges for %s should match", ref)
assertEqualOrBothEmpty(t, expectedNode.In, graphNode.In, "inbound edges for %s should match", ref)
assertEqualOrBothEmpty(t, expectedNode.Out, graphNode.Out, "outbound edges for %s should match", ref)
})
}
}
@@ -220,3 +229,11 @@ func addExpectedNode(t *testing.T, m map[RefKey]GraphNode, refStr string, in, ou
Out: outRefs,
}
}

func assertEqualOrBothEmpty(t *testing.T, a, b []RefKey, msgAndArgs ...any) {
t.Helper()
if len(a) == 0 && len(b) == 0 {
return
}
assert.Equal(t, a, b, msgAndArgs...)
}

0 comments on commit d58a986

Please sign in to comment.