Experimental Reflection apis #98
Conversation
josephschorr
force-pushed
the
reflection-apis
branch
2 times, most recently
from
3cce8c6 to
b717a13
Compare
| // permissions that compute based off a set of relations. For example, if a schema has a relation | ||
| // `viewer` and a permission `view` defined as `permission view = viewer + editor`, then the | ||
| // computable permissions for the relation `viewer` will include `view`. | ||
| rpc ExperimentalComputablePermissions(ExperimentalComputablePermissionsRequest) |
There was a problem hiding this comment.
this name confused me on the first read (and the inverse one) - can you explain why this API is better than e.g. returning an expression tree?
There was a problem hiding this comment.
Because understanding how to walk the expression tree is a non-trivial task, especially as we add more operations to the schema
There was a problem hiding this comment.
It would just be a AST and the leaf nodes will all be relations, right? So even as we add operations we still just have a tree with relations as leaves. Internal nodes are (op, []children) where a child is either another internal node or a leaf (relation) node?
There was a problem hiding this comment.
Not quite; it would have to take into account subject relations/permissions as well, and distinguish between the relation used on the left hand side of an arrow vs the right side ones. In particular, the right side of arrows are inferred based on the type system, and an AST would not show that information directly
There was a problem hiding this comment.
I still think we could come up with an AST that:
- has enough info that consumers can understand the shape of the expressions
- is simple enough that you can use it to determine the answers to basic questions (like what permission is affected by what relation) without needing to understand the full details of the tree
but this is experimental and nothing stops us from adding that in the future, so I'll leave it.
| // permissions that compute based off a set of relations. For example, if a schema has a relation | ||
| // `viewer` and a permission `view` defined as `permission view = viewer + editor`, then the | ||
| // computable permissions for the relation `viewer` will include `view`. | ||
| rpc ExperimentalComputablePermissions(ExperimentalComputablePermissionsRequest) |
There was a problem hiding this comment.
I still think we could come up with an AST that:
- has enough info that consumers can understand the shape of the expressions
- is simple enough that you can use it to determine the answers to basic questions (like what permission is affected by what relation) without needing to understand the full details of the tree
but this is experimental and nothing stops us from adding that in the future, so I'll leave it.
|
|
||
| message ExperimentalComputablePermissionsRequest { | ||
| Consistency consistency = 1; | ||
| repeated ExpRelationReference relations = 3; |
There was a problem hiding this comment.
what about including a filter so that you can avoid objects you're not interested in? i.e. when looking up the effects of a relationship that is used in an arrow
There was a problem hiding this comment.
A filter for the response you mean?
There was a problem hiding this comment.
Yeah - you may be querying for changes to folder#parent and want to ignore changes in document#view for example
josephschorr
force-pushed
the
reflection-apis
branch
from
b717a13 to
f9bbb28
Compare
|
|
||
| // optional_definition_name_match is a regex that is matched against the definition or caveat name. | ||
| // If not specified, will be ignored. | ||
| string optional_definition_name_match = 1; |
There was a problem hiding this comment.
Why are these regexes and not strings? I would've expected them to be more like the filters in the Permissions service
There was a problem hiding this comment.
Purposefully to allow for prefix matching or other forms of matching. For example, you might want to filter to definitions under prefix someteam/
| // optional_definition_name_match is a regex that is matched against the definition or caveat name. | ||
| // If not specified, will be ignored. | ||
| string optional_definition_name_match = 1; | ||
|
|
||
| // optional_relation_or_permission_name_match is a regex that is matched against the relation or permission name. | ||
| // If not specified, will be ignored. | ||
| string optional_relation_or_permission_name_match = 2; | ||
|
|
||
| // kind_filters is a list of kinds to filter on. If not specified, will be ignored. If multiple are specified, | ||
| // the filter will be applied in an OR fashion. | ||
| repeated KindFilter kind_filters = 3; |
There was a problem hiding this comment.
@josephschorr when kind==caveat, the optional_definition makes sense, but optional_relation does not. IMHO a more type-safe way to model this is to have multiple types of KindFilter (CaveatFilter, ObjectFilter, RelationFilter), then remove both optional arguments, and only have repeated KindFilter kind_filters be a one of those types. The part I'm not sure is if you can create a repeated oneof
However, the ExperimentalReflectSchemaRequest already has a repeated ExpSchemaFilter, so why having the repeated KindFilter in ExpSchemaFilter? If superfluous, we could make kind_filters as I described and drop the repeated.
There was a problem hiding this comment.
The part I'm not sure is if you can create a repeated oneof
You cannot; we'd have to create another message for the field and then repeat that
There was a problem hiding this comment.
What are you thoughts on this I wrote?
However, the ExperimentalReflectSchemaRequest already has a repeated ExpSchemaFilter, so why having the repeated KindFilter in ExpSchemaFilter? If superfluous, we could make kind_filters as I described and drop the repeated.
There was a problem hiding this comment.
We can likely drop the repeated from in here, yes
| ZedToken read_at = 3; | ||
| } | ||
|
|
||
| message ExpSchemaFilter { |
There was a problem hiding this comment.
All types should probably have a comment describing it
No description provided.