-
Notifications
You must be signed in to change notification settings - Fork 290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement partials compilation #2212
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,55 +86,39 @@ type Option func(*config) | |
|
||
type ObjectPrefixOption func(*config) | ||
|
||
type compilationContext struct { | ||
// The set of definition names that we've seen as we compile. | ||
// If these collide we throw an error. | ||
existingNames *mapz.Set[string] | ||
// The global set of files we've visited in the import process. | ||
// If these collide we short circuit, preventing duplicate imports. | ||
globallyVisitedFiles *mapz.Set[string] | ||
// The set of files that we've visited on a particular leg of the recursion. | ||
// This allows for detection of circular imports. | ||
// NOTE: This depends on an assumption that a depth-first search will always | ||
// find a cycle, even if we're otherwise marking globally visited nodes. | ||
locallyVisitedFiles *mapz.Set[string] | ||
} | ||
|
||
// Compile compilers the input schema into a set of namespace definition protos. | ||
func Compile(schema InputSchema, prefix ObjectPrefixOption, opts ...Option) (*CompiledSchema, error) { | ||
cctx := compilationContext{ | ||
existingNames: mapz.NewSet[string](), | ||
globallyVisitedFiles: mapz.NewSet[string](), | ||
locallyVisitedFiles: mapz.NewSet[string](), | ||
} | ||
return compileImpl(schema, cctx, prefix, opts...) | ||
} | ||
|
||
func compileImpl(schema InputSchema, cctx compilationContext, prefix ObjectPrefixOption, opts ...Option) (*CompiledSchema, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flattening these since there's no longer an invocation in the import logic |
||
cfg := &config{} | ||
prefix(cfg) // required option | ||
|
||
for _, fn := range opts { | ||
fn(cfg) | ||
} | ||
|
||
mapper := newPositionMapper(schema) | ||
root := parser.Parse(createAstNode, schema.Source, schema.SchemaString).(*dslNode) | ||
errs := root.FindAll(dslshape.NodeTypeError) | ||
if len(errs) > 0 { | ||
err := errorNodeToError(errs[0], mapper) | ||
root, mapper, err := parseSchema(schema) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
compiled, err := translate(translationContext{ | ||
objectTypePrefix: cfg.objectTypePrefix, | ||
mapper: mapper, | ||
schemaString: schema.SchemaString, | ||
skipValidate: cfg.skipValidation, | ||
// NOTE: import translation is done separately so that partial references | ||
// and definitions defined in separate files can correctly resolve. | ||
err = translateImports(importResolutionContext{ | ||
globallyVisitedFiles: mapz.NewSet[string](), | ||
locallyVisitedFiles: mapz.NewSet[string](), | ||
sourceFolder: cfg.sourceFolder, | ||
existingNames: cctx.existingNames, | ||
locallyVisitedFiles: cctx.locallyVisitedFiles, | ||
globallyVisitedFiles: cctx.globallyVisitedFiles, | ||
}, root) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
compiled, err := translate(translationContext{ | ||
objectTypePrefix: cfg.objectTypePrefix, | ||
mapper: mapper, | ||
schemaString: schema.SchemaString, | ||
skipValidate: cfg.skipValidation, | ||
existingNames: mapz.NewSet[string](), | ||
compiledPartials: make(map[string][]*core.Relation), | ||
unresolvedPartials: mapz.NewMultiMap[string, *dslNode](), | ||
}, root) | ||
if err != nil { | ||
var withNodeError withNodeError | ||
|
@@ -148,6 +132,17 @@ func compileImpl(schema InputSchema, cctx compilationContext, prefix ObjectPrefi | |
return compiled, nil | ||
} | ||
|
||
func parseSchema(schema InputSchema) (*dslNode, input.PositionMapper, error) { | ||
mapper := newPositionMapper(schema) | ||
root := parser.Parse(createAstNode, schema.Source, schema.SchemaString).(*dslNode) | ||
errs := root.FindAll(dslshape.NodeTypeError) | ||
if len(errs) > 0 { | ||
err := errorNodeToError(errs[0], mapper) | ||
return nil, nil, err | ||
} | ||
return root, mapper, nil | ||
} | ||
Comment on lines
+135
to
+144
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I pulled this out into a separate function because the import functionality parses as a part of its work. |
||
|
||
func errorNodeToError(node *dslNode, mapper input.PositionMapper) error { | ||
if node.GetType() != dslshape.NodeTypeError { | ||
return fmt.Errorf("given none error node") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -88,6 +88,238 @@ func TestCompile(t *testing.T) { | |
), | ||
}, | ||
}, | ||
{ | ||
"simple partial", | ||
withTenantPrefix, | ||
`partial view_partial { | ||
relation user: user; | ||
} | ||
|
||
definition simple { | ||
...view_partial | ||
}`, | ||
"", | ||
[]SchemaDefinition{ | ||
namespace.Namespace("sometenant/simple", | ||
namespace.MustRelation("user", nil, | ||
namespace.AllowedRelation("sometenant/user", "..."), | ||
), | ||
), | ||
}, | ||
}, | ||
{ | ||
"more complex partial", | ||
withTenantPrefix, | ||
` | ||
definition user {} | ||
definition organization {} | ||
|
||
partial view_partial { | ||
relation user: user; | ||
permission view = user | ||
} | ||
|
||
definition resource { | ||
relation organization: organization | ||
permission manage = organization | ||
|
||
...view_partial | ||
} | ||
`, | ||
"", | ||
[]SchemaDefinition{ | ||
namespace.Namespace("sometenant/user"), | ||
namespace.Namespace("sometenant/organization"), | ||
namespace.Namespace("sometenant/resource", | ||
namespace.MustRelation("organization", nil, | ||
namespace.AllowedRelation("sometenant/organization", "..."), | ||
), | ||
namespace.MustRelation("manage", | ||
namespace.Union( | ||
namespace.ComputedUserset("organization"), | ||
), | ||
), | ||
namespace.MustRelation("user", nil, | ||
namespace.AllowedRelation("sometenant/user", "..."), | ||
), | ||
namespace.MustRelation("view", | ||
namespace.Union( | ||
namespace.ComputedUserset("user"), | ||
), | ||
), | ||
), | ||
}, | ||
}, | ||
{ | ||
"partial defined after reference", | ||
withTenantPrefix, | ||
`definition simple { | ||
...view_partial | ||
} | ||
|
||
partial view_partial { | ||
relation user: user; | ||
}`, | ||
"", | ||
[]SchemaDefinition{ | ||
namespace.Namespace("sometenant/simple", | ||
namespace.MustRelation("user", nil, | ||
namespace.AllowedRelation("sometenant/user", "..."), | ||
), | ||
), | ||
}, | ||
}, | ||
{ | ||
"transitive partials", | ||
withTenantPrefix, | ||
` | ||
partial view_partial { | ||
relation user: user; | ||
} | ||
|
||
partial transitive_partial { | ||
...view_partial | ||
} | ||
|
||
definition simple { | ||
...view_partial | ||
} | ||
`, | ||
"", | ||
[]SchemaDefinition{ | ||
namespace.Namespace("sometenant/simple", | ||
namespace.MustRelation("user", nil, | ||
namespace.AllowedRelation("sometenant/user", "..."), | ||
), | ||
), | ||
}, | ||
}, | ||
{ | ||
"transitive partials out of order", | ||
withTenantPrefix, | ||
` | ||
partial transitive_partial { | ||
...view_partial | ||
} | ||
|
||
partial view_partial { | ||
relation user: user; | ||
} | ||
|
||
definition simple { | ||
...view_partial | ||
} | ||
`, | ||
"", | ||
[]SchemaDefinition{ | ||
namespace.Namespace("sometenant/simple", | ||
namespace.MustRelation("user", nil, | ||
namespace.AllowedRelation("sometenant/user", "..."), | ||
), | ||
), | ||
}, | ||
}, | ||
{ | ||
"transitive partials in reverse order", | ||
withTenantPrefix, | ||
` | ||
definition simple { | ||
...view_partial | ||
} | ||
|
||
partial transitive_partial { | ||
...view_partial | ||
} | ||
|
||
partial view_partial { | ||
relation user: user; | ||
} | ||
`, | ||
"", | ||
[]SchemaDefinition{ | ||
namespace.Namespace("sometenant/simple", | ||
namespace.MustRelation("user", nil, | ||
namespace.AllowedRelation("sometenant/user", "..."), | ||
), | ||
), | ||
}, | ||
}, | ||
{ | ||
"forking transitive partials out of order", | ||
withTenantPrefix, | ||
` | ||
partial transitive_partial { | ||
...view_partial | ||
...group_partial | ||
} | ||
|
||
partial view_partial { | ||
relation user: user; | ||
} | ||
|
||
partial group_partial { | ||
relation group: group; | ||
} | ||
|
||
definition simple { | ||
...transitive_partial | ||
} | ||
`, | ||
"", | ||
[]SchemaDefinition{ | ||
namespace.Namespace("sometenant/simple", | ||
namespace.MustRelation("user", nil, | ||
namespace.AllowedRelation("sometenant/user", "..."), | ||
), | ||
namespace.MustRelation("group", nil, | ||
namespace.AllowedRelation("sometenant/group", "..."), | ||
), | ||
), | ||
}, | ||
}, | ||
{ | ||
"circular reference in partials", | ||
withTenantPrefix, | ||
` | ||
partial one_partial { | ||
...another_partial | ||
} | ||
|
||
partial another_partial { | ||
...one_partial | ||
} | ||
|
||
definition simple { | ||
...one_partial | ||
} | ||
`, | ||
"could not resolve partials", | ||
[]SchemaDefinition{}, | ||
}, | ||
{ | ||
"definition reference to nonexistent partial", | ||
withTenantPrefix, | ||
` | ||
definition simple { | ||
...some_partial | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add some tests that have concrete impls next to the partials |
||
} | ||
`, | ||
"could not find partial reference", | ||
[]SchemaDefinition{}, | ||
}, | ||
{ | ||
"definition reference to another definition errors", | ||
withTenantPrefix, | ||
` | ||
definition some_definition {} | ||
|
||
definition simple { | ||
...some_definition | ||
} | ||
`, | ||
"could not find partial reference", | ||
[]SchemaDefinition{}, | ||
}, | ||
{ | ||
"explicit relation", | ||
withTenantPrefix, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These were previously values that we passed through the tree, because the import logic invoked compilation. That isn't possible with partials since a partial can be defined in one file and used in another, so among other changes, this PR makes the import logic operate on the DSL tree as an intermediate step.