Skip to content

Commit 014691e

Browse files
authored
fix: ensure resolved references in schema v2 are updated when flattening (#2662)
1 parent 50bd8e7 commit 014691e

File tree

2 files changed

+295
-1
lines changed

2 files changed

+295
-1
lines changed

pkg/schema/v2/flatten.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,27 @@ func flattenOperation(op Operation, def *Definition, baseName string, options Fl
134134

135135
switch o := op.(type) {
136136
case *ResolvedRelationReference:
137-
// Leaf node, no flattening needed
137+
// Leaf node, but we need to update the resolved reference to point to the cloned schema
138+
// Check if this references a relation or permission in the definition
139+
if rel, ok := o.resolved.(*Relation); ok {
140+
// Find the corresponding relation in the cloned definition
141+
if clonedRel, exists := def.relations[rel.name]; exists {
142+
return &ResolvedRelationReference{
143+
relationName: o.relationName,
144+
resolved: clonedRel,
145+
}, nil, nil
146+
}
147+
} else if perm, ok := o.resolved.(*Permission); ok {
148+
// Find the corresponding permission in the cloned definition
149+
if clonedPerm, exists := def.permissions[perm.name]; exists {
150+
return &ResolvedRelationReference{
151+
relationName: o.relationName,
152+
resolved: clonedPerm,
153+
}, nil, nil
154+
}
155+
}
156+
// If we can't find the referenced object, just return the original reference
157+
// This shouldn't happen in a properly resolved schema
138158
return o, nil, nil
139159

140160
case *ResolvedArrowReference:

pkg/schema/v2/flatten_test.go

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,78 @@ definition user {}`,
977977
978978
definition user {}`,
979979
},
980+
{
981+
name: "union with arrow and nil",
982+
schemaString: `definition organization {
983+
relation member: user
984+
}
985+
definition document {
986+
relation org: organization
987+
permission view = org->member + nil
988+
}
989+
990+
definition user {}`,
991+
flattenNonUnionOperations: true,
992+
flattenArrows: true,
993+
expectedString: `definition organization {
994+
relation member: user
995+
}
996+
definition document {
997+
relation org: organization
998+
permission view = view__c1dc49f4d1e680d0 + nil
999+
permission view__c1dc49f4d1e680d0 = org->member
1000+
}
1001+
1002+
definition user {}`,
1003+
},
1004+
{
1005+
name: "multi-flatten with arrows and non-union ops",
1006+
schemaString: `definition user {}
1007+
1008+
definition document {
1009+
relation owner: user
1010+
relation editor: user
1011+
relation parent: folder
1012+
relation viewer: user
1013+
1014+
permission edit = editor + owner
1015+
permission view = viewer + edit + parent->view
1016+
permission view_and_edit = view & edit
1017+
}
1018+
1019+
definition folder {
1020+
relation parent: folder
1021+
relation owner: user
1022+
relation editor: user
1023+
relation viewer: user | folder#view
1024+
1025+
permission view = viewer + editor + owner + parent->view
1026+
}`,
1027+
flattenNonUnionOperations: true,
1028+
flattenArrows: true,
1029+
expectedString: `definition document {
1030+
permission edit = editor + owner
1031+
relation editor: user
1032+
relation owner: user
1033+
relation parent: folder
1034+
permission view = viewer + edit + view__0b0ed894546431d9
1035+
permission view__0b0ed894546431d9 = parent->view
1036+
permission view_and_edit = view & edit
1037+
relation viewer: user
1038+
}
1039+
1040+
definition folder {
1041+
relation editor: user
1042+
relation owner: user
1043+
relation parent: folder
1044+
permission view = viewer + editor + owner + view__0b0ed894546431d9
1045+
permission view__0b0ed894546431d9 = parent->view
1046+
relation viewer: user | folder#view
1047+
}
1048+
1049+
definition user {}
1050+
`,
1051+
},
9801052
}
9811053

9821054
for _, tt := range tests {
@@ -1376,3 +1448,205 @@ definition user {}`,
13761448
})
13771449
}
13781450
}
1451+
1452+
func TestFlattenSchema_ResolvedReferences(t *testing.T) {
1453+
tests := []struct {
1454+
name string
1455+
schemaString string
1456+
}{
1457+
{
1458+
name: "nested operations create resolved references",
1459+
schemaString: `definition document {
1460+
relation viewer: user
1461+
relation editor: user
1462+
relation owner: user
1463+
permission view = (viewer + editor) & owner
1464+
}
1465+
1466+
definition user {}`,
1467+
},
1468+
{
1469+
name: "deeply nested operations",
1470+
schemaString: `definition document {
1471+
relation alpha: user
1472+
relation beta: user
1473+
relation gamma: user
1474+
relation delta: user
1475+
permission view = ((alpha & beta) - gamma) & delta
1476+
}
1477+
1478+
definition user {}`,
1479+
},
1480+
{
1481+
name: "arrow operations flattened",
1482+
schemaString: `definition document {
1483+
relation foo: folder
1484+
relation bar: folder
1485+
permission view = foo->bar & bar->baz
1486+
}
1487+
1488+
definition folder {
1489+
relation bar: folder
1490+
permission baz = bar
1491+
}`,
1492+
},
1493+
{
1494+
name: "functioned arrow operations",
1495+
schemaString: `definition document {
1496+
relation parent: folder
1497+
relation viewer: user
1498+
permission view = parent.any(viewer) + viewer
1499+
}
1500+
1501+
definition folder {
1502+
relation viewer: user
1503+
}
1504+
1505+
definition user {}`,
1506+
},
1507+
}
1508+
1509+
for _, tt := range tests {
1510+
t.Run(tt.name, func(t *testing.T) {
1511+
// Step 1: Compile the schema
1512+
compiled, err := compiler.Compile(compiler.InputSchema{
1513+
Source: input.Source("test"),
1514+
SchemaString: tt.schemaString,
1515+
}, compiler.AllowUnprefixedObjectType())
1516+
require.NoError(t, err)
1517+
1518+
// Step 2: Convert to *Schema
1519+
schema, err := BuildSchemaFromCompiledSchema(*compiled)
1520+
require.NoError(t, err)
1521+
require.NotNil(t, schema)
1522+
1523+
// Step 3: Resolve the schema
1524+
resolved, err := ResolveSchema(schema)
1525+
require.NoError(t, err)
1526+
require.NotNil(t, resolved)
1527+
1528+
// Step 4: Flatten the schema
1529+
flattened, err := FlattenSchema(resolved, FlattenSeparatorDoubleUnderscore)
1530+
require.NoError(t, err)
1531+
require.NotNil(t, flattened)
1532+
1533+
// Step 5: Verify all ResolvedRelationReferences point to valid permissions
1534+
flattenedSchema := flattened.ResolvedSchema().Schema()
1535+
for _, def := range flattenedSchema.definitions {
1536+
for _, perm := range def.permissions {
1537+
verifyResolvedReferencesInOperation(t, perm.operation, def)
1538+
}
1539+
}
1540+
})
1541+
}
1542+
}
1543+
1544+
// verifyResolvedReferencesInOperation recursively checks that all ResolvedRelationReferences
1545+
// point to valid relations or permissions in the definition.
1546+
func verifyResolvedReferencesInOperation(t *testing.T, op Operation, def *Definition) {
1547+
if op == nil {
1548+
return
1549+
}
1550+
1551+
switch o := op.(type) {
1552+
case *ResolvedRelationReference:
1553+
// Verify the resolved field points to either a relation or permission
1554+
require.NotNil(t, o.resolved, "ResolvedRelationReference.resolved should not be nil for %s", o.relationName)
1555+
1556+
// Check if it's a relation
1557+
if rel, ok := o.resolved.(*Relation); ok {
1558+
// Verify the relation exists in the definition
1559+
foundRel, exists := def.relations[o.relationName]
1560+
require.True(t, exists, "Relation %s should exist in definition %s", o.relationName, def.name)
1561+
// Verify they have the same name (pointer equality won't work after cloning)
1562+
require.Equal(t, foundRel.name, rel.name, "Resolved relation should have same name")
1563+
require.Equal(t, foundRel.parent, rel.parent, "Resolved relation should have same parent")
1564+
} else if perm, ok := o.resolved.(*Permission); ok {
1565+
// Verify the permission exists in the definition
1566+
foundPerm, exists := def.permissions[o.relationName]
1567+
require.True(t, exists, "Permission %s should exist in definition %s", o.relationName, def.name)
1568+
// Verify they have the same name (pointer equality won't work after cloning)
1569+
require.Equal(t, foundPerm.name, perm.name, "Resolved permission should have same name")
1570+
require.Equal(t, foundPerm.parent, perm.parent, "Resolved permission should have same parent")
1571+
// Most importantly, verify that the resolved field points to the exact same permission
1572+
// object that's in the definition map (same pointer)
1573+
require.Same(t, foundPerm, o.resolved, "ResolvedRelationReference should point to the exact permission object in the definition")
1574+
} else {
1575+
t.Fatalf("ResolvedRelationReference.resolved should be either *Relation or *Permission, got %T", o.resolved)
1576+
}
1577+
1578+
case *UnionOperation:
1579+
for _, child := range o.children {
1580+
verifyResolvedReferencesInOperation(t, child, def)
1581+
}
1582+
1583+
case *IntersectionOperation:
1584+
for _, child := range o.children {
1585+
verifyResolvedReferencesInOperation(t, child, def)
1586+
}
1587+
1588+
case *ExclusionOperation:
1589+
verifyResolvedReferencesInOperation(t, o.left, def)
1590+
verifyResolvedReferencesInOperation(t, o.right, def)
1591+
1592+
case *ResolvedArrowReference, *ResolvedFunctionedArrowReference, *RelationReference, *ArrowReference, *FunctionedArrowReference, *NilReference:
1593+
// These are leaf nodes, no further verification needed
1594+
return
1595+
1596+
default:
1597+
t.Fatalf("Unknown operation type: %T", op)
1598+
}
1599+
}
1600+
1601+
func TestFlattenSchema_SyntheticPermissionsAreResolved(t *testing.T) {
1602+
schemaString := `definition document {
1603+
relation viewer: user
1604+
relation editor: user
1605+
relation owner: user
1606+
permission view = (viewer + editor) & owner
1607+
}
1608+
1609+
definition user {}`
1610+
1611+
// Compile the schema
1612+
compiled, err := compiler.Compile(compiler.InputSchema{
1613+
Source: input.Source("test"),
1614+
SchemaString: schemaString,
1615+
}, compiler.AllowUnprefixedObjectType())
1616+
require.NoError(t, err)
1617+
1618+
// Convert to *Schema
1619+
schema, err := BuildSchemaFromCompiledSchema(*compiled)
1620+
require.NoError(t, err)
1621+
1622+
// Resolve the schema
1623+
resolved, err := ResolveSchema(schema)
1624+
require.NoError(t, err)
1625+
1626+
// Flatten the schema
1627+
flattened, err := FlattenSchema(resolved, FlattenSeparatorDoubleUnderscore)
1628+
require.NoError(t, err)
1629+
1630+
// Get the flattened definition
1631+
flattenedDef := flattened.ResolvedSchema().Schema().definitions["document"]
1632+
1633+
// Find the view permission
1634+
viewPerm, exists := flattenedDef.permissions["view"]
1635+
require.True(t, exists)
1636+
1637+
// The view permission should have an intersection operation
1638+
intersection, ok := viewPerm.operation.(*IntersectionOperation)
1639+
require.True(t, ok, "view permission should be an intersection")
1640+
1641+
// The first child should be a resolved reference to a synthetic permission
1642+
firstChild, ok := intersection.children[0].(*ResolvedRelationReference)
1643+
require.True(t, ok, "first child should be a ResolvedRelationReference")
1644+
1645+
// Verify the synthetic permission exists
1646+
synthPerm, exists := flattenedDef.permissions[firstChild.relationName]
1647+
require.True(t, exists, "synthetic permission %s should exist", firstChild.relationName)
1648+
require.True(t, synthPerm.IsSynthetic(), "permission should be marked as synthetic")
1649+
1650+
// Verify the resolved field points to the correct synthetic permission
1651+
require.Equal(t, synthPerm, firstChild.resolved, "ResolvedRelationReference should point to the synthetic permission")
1652+
}

0 commit comments

Comments
 (0)