Skip to content

Commit

Permalink
Compare tuples without stringifying
Browse files Browse the repository at this point in the history
  • Loading branch information
bradengroom committed Oct 3, 2023
1 parent e97bbf7 commit 430ff42
Show file tree
Hide file tree
Showing 5 changed files with 196 additions and 12 deletions.
3 changes: 1 addition & 2 deletions internal/datastore/mysql/readwrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,7 @@ func (rwt *mysqlReadWriteTXN) WriteRelationships(ctx context.Context, mutations
}

// Ensure the tuples are the same.
// TODO(jschorr): Use a faster method then string comparison.
if tuple.MustString(mut.Tuple) == tuple.MustString(foundTpl) {
if tuple.Equal(mut.Tuple, foundTpl) {
delete(createAndTouchMutationsByTuple, tplString)
continue
}
Expand Down
11 changes: 1 addition & 10 deletions internal/graph/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,15 +197,6 @@ func (cc *ConcurrentChecker) checkInternal(ctx context.Context, req ValidatedChe
return combineResultWithFoundResources(cc.checkUsersetRewrite(ctx, crc, relation.UsersetRewrite), membershipSet)
}

func onrEqual(lhs, rhs *core.ObjectAndRelation) bool {
// Properties are sorted by highest to lowest cardinality to optimize for short-circuiting.
return lhs.ObjectId == rhs.ObjectId && lhs.Relation == rhs.Relation && lhs.Namespace == rhs.Namespace
}

func onrEqualOrWildcard(tpl, target *core.ObjectAndRelation) bool {
return onrEqual(tpl, target) || (tpl.ObjectId == tuple.PublicWildcard && tpl.Namespace == target.Namespace)
}

type directDispatch struct {
resourceType *core.RelationReference
resourceIds []string
Expand Down Expand Up @@ -298,7 +289,7 @@ func (cc *ConcurrentChecker) checkDirect(ctx context.Context, crc currentRequest

// If the subject of the relationship matches the target subject, then we've found
// a result.
if !onrEqualOrWildcard(tpl.Subject, crc.parentReq.Subject) {
if !tuple.OnrEqualOrWildcard(tpl.Subject, crc.parentReq.Subject) {
tplString, err := tuple.String(tpl)
if err != nil {
return checkResultError(err, emptyMetadata)
Expand Down
9 changes: 9 additions & 0 deletions pkg/tuple/onr.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,12 @@ func StringsONRs(onrs []*core.ObjectAndRelation) []string {
sort.Strings(onrstrings)
return onrstrings
}

func OnrEqual(lhs, rhs *core.ObjectAndRelation) bool {
// Properties are sorted by highest to lowest cardinality to optimize for short-circuiting.
return lhs.ObjectId == rhs.ObjectId && lhs.Relation == rhs.Relation && lhs.Namespace == rhs.Namespace
}

func OnrEqualOrWildcard(tpl, target *core.ObjectAndRelation) bool {
return OnrEqual(tpl, target) || (tpl.ObjectId == PublicWildcard && tpl.Namespace == target.Namespace)
}
13 changes: 13 additions & 0 deletions pkg/tuple/tuple.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,14 @@ func StringWithoutCaveat(tpl *core.RelationTuple) string {
return fmt.Sprintf("%s@%s", StringONR(tpl.ResourceAndRelation), StringONR(tpl.Subject))
}

func MustStringCaveat(caveat *core.ContextualizedCaveat) string {
caveatString, err := StringCaveat(caveat)
if err != nil {
panic(err)
}
return caveatString
}

// StringCaveat converts a contextualized caveat to a string. If the caveat is nil or empty, returns empty string.
func StringCaveat(caveat *core.ContextualizedCaveat) (string, error) {
if caveat == nil || caveat.CaveatName == "" {
Expand Down Expand Up @@ -263,6 +271,11 @@ func Delete(tpl *core.RelationTuple) *core.RelationTupleUpdate {
}
}

func Equal(lhs, rhs *core.RelationTuple) bool {
// TODO(jschorr): Use a faster method then string comparison for caveats.
return OnrEqual(lhs.ResourceAndRelation, rhs.ResourceAndRelation) && OnrEqual(lhs.Subject, rhs.Subject) && MustStringCaveat(lhs.Caveat) == MustStringCaveat(rhs.Caveat)
}

// MustToRelationship converts a RelationTuple into a Relationship. Will panic if
// the RelationTuple does not validate.
func MustToRelationship(tpl *core.RelationTuple) *v1.Relationship {
Expand Down
172 changes: 172 additions & 0 deletions pkg/tuple/tuple_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -546,3 +546,175 @@ func TestCopyRelationTupleToRelationship(t *testing.T) {
})
}
}

func TestEqual(t *testing.T) {
equalTestCases := []*core.RelationTuple{
makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user", "testusr", "..."),
),
MustWithCaveat(
makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user", "testusr", "..."),
),
"somecaveat",
map[string]any{
"context": map[string]any{
"deeply": map[string]any{
"nested": true,
},
},
},
),
}

for _, tc := range equalTestCases {
t.Run(MustString(tc), func(t *testing.T) {
require := require.New(t)
require.True(Equal(tc, tc.CloneVT()))
})
}

notEqualTestCases := []struct {
name string
lhs *core.RelationTuple
rhs *core.RelationTuple
}{
{
name: "Mismatch Resource Type",
lhs: makeTuple(
ObjectAndRelation("testns1", "testobj", "testrel"),
ObjectAndRelation("user", "testusr", "..."),
),
rhs: makeTuple(
ObjectAndRelation("testns2", "testobj", "testrel"),
ObjectAndRelation("user", "testusr", "..."),
),
},
{
name: "Mismatch Resource ID",
lhs: makeTuple(
ObjectAndRelation("testns", "testobj1", "testrel"),
ObjectAndRelation("user", "testusr", "..."),
),
rhs: makeTuple(
ObjectAndRelation("testns", "testobj2", "testrel"),
ObjectAndRelation("user", "testusr", "..."),
),
},
{
name: "Mismatch Resource Relationship",
lhs: makeTuple(
ObjectAndRelation("testns", "testobj", "testrel1"),
ObjectAndRelation("user", "testusr", "..."),
),
rhs: makeTuple(
ObjectAndRelation("testns", "testobj", "testrel2"),
ObjectAndRelation("user", "testusr", "..."),
),
},
{
name: "Mismatch Subject Type",
lhs: makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user1", "testusr", "..."),
),
rhs: makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user2", "testusr", "..."),
),
},
{
name: "Mismatch Subject ID",
lhs: makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user", "testusr1", "..."),
),
rhs: makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user", "testusr2", "..."),
),
},
{
name: "Mismatch Subject Relationship",
lhs: makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user", "testusr", "testrel1"),
),
rhs: makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user", "testusr", "testrel2"),
),
},
{
name: "Mismatch Caveat Name",
lhs: MustWithCaveat(
makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user", "testusr", "..."),
),
"somecaveat1",
map[string]any{
"context": map[string]any{
"deeply": map[string]any{
"nested": true,
},
},
},
),
rhs: MustWithCaveat(
makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user", "testusr", "..."),
),
"somecaveat2",
map[string]any{
"context": map[string]any{
"deeply": map[string]any{
"nested": true,
},
},
},
),
},
{
name: "Mismatch Caveat Content",
lhs: MustWithCaveat(
makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user", "testusr", "..."),
),
"somecaveat",
map[string]any{
"context": map[string]any{
"deeply": map[string]any{
"nested": "1",
},
},
},
),
rhs: MustWithCaveat(
makeTuple(
ObjectAndRelation("testns", "testobj", "testrel"),
ObjectAndRelation("user", "testusr", "..."),
),
"somecaveat",
map[string]any{
"context": map[string]any{
"deeply": map[string]any{
"nested": "2",
},
},
},
),
},
}

for _, tc := range notEqualTestCases {
t.Run(tc.name, func(t *testing.T) {
require := require.New(t)
require.False(Equal(tc.lhs, tc.rhs))
})
}
}

0 comments on commit 430ff42

Please sign in to comment.