diff --git a/pkg/diff/namespace/diffexpr.go b/pkg/diff/namespace/diffexpr.go index fc1304e3b9..359728ce72 100644 --- a/pkg/diff/namespace/diffexpr.go +++ b/pkg/diff/namespace/diffexpr.go @@ -17,6 +17,11 @@ const ( // ExpressionChildrenChanged indicates that the children of the expression were changed. ExpressionChildrenChanged ExpressionChangeType = "children-changed" + + // ExpressionOperationExpanded indicates that the operation type of the expression was expanded + // from a union of a single child to multiple children under a union, intersection or another + // operation. + ExpressionOperationExpanded ExpressionChangeType = "operation-expanded" ) // ExpressionDiff holds the diff between two expressions. @@ -144,12 +149,20 @@ func DiffExpressions(existing *core.UsersetRewrite, updated *core.UsersetRewrite return nil, spiceerrors.MustBugf("unknown operation type %T", updated.RewriteOperation) } + childChangeKind := ExpressionChildrenChanged if existingType != updatedType { - return &ExpressionDiff{ - existing: existing, - updated: updated, - change: ExpressionOperationChanged, - }, nil + // If the expression has changed from a union with a single child, then + // treat this as a special case, since there wasn't really an operation + // before. + if existingType != "union" || len(existingOperation.Child) != 1 { + return &ExpressionDiff{ + existing: existing, + updated: updated, + change: ExpressionOperationChanged, + }, nil + } + + childChangeKind = ExpressionOperationExpanded } childDiffs := make([]*OperationDiff, 0, abs(len(updatedOperation.Child)-len(existingOperation.Child))) @@ -186,7 +199,7 @@ func DiffExpressions(existing *core.UsersetRewrite, updated *core.UsersetRewrite return &ExpressionDiff{ existing: existing, updated: updated, - change: ExpressionChildrenChanged, + change: childChangeKind, childDiffs: childDiffs, }, nil } diff --git a/pkg/diff/namespace/diffexpr_test.go b/pkg/diff/namespace/diffexpr_test.go index 59f9a1c09d..1622d599a7 100644 --- a/pkg/diff/namespace/diffexpr_test.go +++ b/pkg/diff/namespace/diffexpr_test.go @@ -97,6 +97,35 @@ func TestDiffExpressions(t *testing.T) { expected: `expression-unchanged `, }, + { + name: "item added to intersection", + existing: `viewer & editor`, + updated: `viewer & editor & admin`, + expected: `children-changed + operation-added`, + }, + { + name: "intersection added", + existing: `viewer`, + updated: `viewer & editor`, + expected: `operation-expanded + operation-added`, + }, + { + name: "exclusion added", + existing: `viewer`, + updated: `viewer - editor`, + expected: `operation-expanded + operation-added`, + }, + { + name: "item added to exclusion", + existing: `viewer - editor`, + updated: `viewer - editor - banned`, + expected: `children-changed + operation-type-changed + operation-computed-userset-changed`, + }, } for _, tc := range tcs {