Merge pull request #1689 from josephschorr/diff-source-position

Ignore the source position when diffing permission expressions

pkg/diff/namespace/diff.go

@@ -1,9 +1,10 @@
 package namespace
 
 import (
+	"github.com/google/go-cmp/cmp"
 	"github.com/scylladb/go-set/strset"
 	"golang.org/x/exp/slices"
-	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/testing/protocmp"
 
 	nsinternal "github.com/authzed/spicedb/internal/namespace"
 	"github.com/authzed/spicedb/pkg/genutil/mapz"
@@ -301,5 +302,12 @@ func isPermission(relation *core.Relation) bool {
 }
 
 func areDifferentExpressions(existing *core.UsersetRewrite, updated *core.UsersetRewrite) bool {
-	return !proto.Equal(existing, updated)
+	// Return whether the rewrites are different, ignoring the SourcePosition message type.
+	delta := cmp.Diff(
+		existing,
+		updated,
+		protocmp.Transform(),
+		protocmp.IgnoreMessages(&core.SourcePosition{}),
+	)
+	return delta != ""
 }

pkg/diff/namespace/diff_test.go

@@ -556,6 +556,22 @@ func TestNamespaceDiff(t *testing.T) {
 				},
 			},
 		},
+		{
+			"location change does not cause expression change",
+			ns.Namespace(
+				"document",
+				ns.MustRelation("somerel", ns.Union(
+					ns.MustComputesUsersetWithSourcePosition("editor", 1),
+				)),
+			),
+			ns.Namespace(
+				"document",
+				ns.MustRelation("somerel", ns.Union(
+					ns.MustComputesUsersetWithSourcePosition("editor", 2),
+				)),
+			),
+			[]Delta{},
+		},
 	}
 
 	for _, tc := range testCases {

pkg/namespace/builder.go

@@ -224,6 +224,23 @@ func ComputedUserset(relation string) *core.SetOperation_Child {
 	}
 }
 
+// MustComputesUsersetWithSourcePosition creates a child for a set operation that follows a relation on the given starting object.
+func MustComputesUsersetWithSourcePosition(relation string, lineNumber uint64) *core.SetOperation_Child {
+	cu := &core.ComputedUserset{
+		Relation: relation,
+	}
+	cu.SourcePosition = &core.SourcePosition{
+		ZeroIndexedLineNumber:     lineNumber,
+		ZeroIndexedColumnPosition: 0,
+	}
+
+	return &core.SetOperation_Child{
+		ChildType: &core.SetOperation_Child_ComputedUserset{
+			ComputedUserset: cu,
+		},
+	}
+}
+
 // TupleToUserset creates a child which first loads all tuples with the specific relation,
 // and then unions all children on the usersets found by following a relation on those loaded
 // tuples.