Initial support for Intersection arrow #1937
Conversation
github-actions
bot
added
area/schema
josephschorr
force-pushed
the
intersection-arrow
branch
from
41f0eb8
to
44c6555
Compare
| tuple.MustParse("document:doc2#parent@folder:folder2-2"), | ||
| tuple.MustParse("document:doc2#parent@folder:folder2-3"), | ||
| tuple.MustParse("folder:folder2-1#viewer@user:tom"), | ||
| tuple.MustParse("folder:folder2-2#viewer@user:tom"), |
There was a problem hiding this comment.
add a 0 case: a document that is in no folders.
| ), | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
I don't see any test cases for (not just for LR, the other methods too):
- an
allover a recursive relationship - an
allthat is part of an expression
internal/graph/check.go
Outdated
| return checkIntersectionTupleToUserset(ctx, cc, crc, child.FunctionedTupleToUserset) | ||
|
|
||
| default: | ||
| return checkResultError(spiceerrors.MustBugf("unknown functioned tuple to userset function `%s`", child.FunctionedTupleToUserset.Function), emptyMetadata) |
There was a problem hiding this comment.
nit/suggestion:
| return checkResultError(spiceerrors.MustBugf("unknown functioned tuple to userset function `%s`", child.FunctionedTupleToUserset.Function), emptyMetadata) | |
| return checkResultError(spiceerrors.MustBugf("unknown tuple to userset function `%s`", child.FunctionedTupleToUserset.Function), emptyMetadata) |
| func (ce *ConcurrentExpander) expandTupleToUserset(_ context.Context, req ValidatedExpandRequest, ttu *core.TupleToUserset) ReduceableExpandFunc { | ||
| type expandFunc func(ctx context.Context, start *core.ObjectAndRelation, requests []ReduceableExpandFunc) ExpandResult | ||
|
|
||
| func expandTupleToUserset[T relation]( |
There was a problem hiding this comment.
I don't see any tests for expand?
internal/graph/lookupsubjects.go
Outdated
| it.Close() | ||
|
|
||
| // Wait for all dispatched operations to complete. | ||
| if err := g.Wait(); err != nil { |
There was a problem hiding this comment.
You shouldn't need to wait here - since this is an AND operation, you can just add in the sets as you get them (it's associative and commutative). That should keep memory lower too.
| ns.MustFunctionedTupleToUserset("owner", "any", "something"), | ||
| )), | ||
| ns.MustRelation("thirdwithall", ns.Union( | ||
| ns.MustFunctionedTupleToUserset("owner", "all", "something"), |
There was a problem hiding this comment.
Could you add some cases in here with expressions containing the fn arrows to make sure they canonicalize the same?
i.e. owner.all(something) + foo == foo + owner.all(something)
|
|
||
| case core.FunctionedTupleToUserset_FUNCTION_ALL: | ||
| // Mark as a conditional result. | ||
| operationResultState = core.ReachabilityEntrypoint_REACHABLE_CONDITIONAL_RESULT |
There was a problem hiding this comment.
Why is this conditional? I would think it would be marked the same as a regular arrow.
There was a problem hiding this comment.
The intersection results in a check being required. This ensures that all is checked post reachable resources.
| } | ||
|
|
||
| // Create a membership set per-subject-type, representing the membership for each of the dispatched subjects. | ||
| resultsByDispatchedSubject := map[string]*MembershipSet{} |
There was a problem hiding this comment.
I made a similar comment on one of the other graph methods, but you don't have to collect up all of the results before you combine them; you can combine as you get responses back.
josephschorr
force-pushed
the
intersection-arrow
branch
from
44c6555
to
1cb8e3d
Compare
|
Updated |
josephschorr
force-pushed
the
intersection-arrow
branch
from
1cb8e3d
to
263823b
Compare
Adds support for intersection arrow and new syntax for existing arrows.
Existing arrow, two syntaxes:
New syntax for intersection arrow:
The
folder.all(view)syntax will ensure that a subject is found forviewin all folders for the parent resourceFixes #597