diff --git a/internal/graph/check.go b/internal/graph/check.go index b9880acda8..77f498ed29 100644 --- a/internal/graph/check.go +++ b/internal/graph/check.go @@ -19,7 +19,6 @@ import ( "github.com/authzed/spicedb/internal/taskrunner" "github.com/authzed/spicedb/pkg/datastore" "github.com/authzed/spicedb/pkg/genutil/mapz" - "github.com/authzed/spicedb/pkg/genutil/slicez" nspkg "github.com/authzed/spicedb/pkg/namespace" core "github.com/authzed/spicedb/pkg/proto/core/v1" v1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1" @@ -296,11 +295,6 @@ func combineWithCheckHints(result CheckResult, req ValidatedCheckRequest) CheckR return result } -type directDispatch struct { - resourceType *core.RelationReference - resourceIds []string -} - func (cc *ConcurrentChecker) checkDirect(ctx context.Context, crc currentRequestContext, relation *core.Relation) CheckResult { ctx, span := tracer.Start(ctx, "checkDirect") defer span.End() @@ -447,56 +441,30 @@ func (cc *ConcurrentChecker) checkDirect(ctx context.Context, crc currentRequest defer it.Close() queryCount += 1.0 - // Find the subjects over which to dispatch. - subjectsToDispatch := tuple.NewONRByTypeSet() - relationshipsBySubjectONR := mapz.NewMultiMap[string, *core.RelationTuple]() - hasCaveats := false - + // Build the set of subjects over which to dispatch, along with metadata for + // mapping over caveats (if any). + checksToDispatch := newCheckDispatchSet() for tpl := it.Next(); tpl != nil; tpl = it.Next() { if it.Err() != nil { return checkResultError(NewCheckFailureErr(it.Err()), emptyMetadata) } - - spiceerrors.DebugAssert(func() bool { return tpl.Subject.Relation != Ellipsis }, "got a terminal for a non-terminal query") - - // Add the subject as an object over which to dispatch. - subjectsToDispatch.Add(tpl.Subject) - relationshipsBySubjectONR.Add(tuple.StringONR(tpl.Subject), tpl) - if tpl.Caveat != nil && tpl.Caveat.CaveatName != "" { - hasCaveats = true - } + checksToDispatch.addForRelationship(tpl) } it.Close() - // Convert the subjects into batched requests. - // To simplify the logic, +1 is added to account for the situation where - // the number of elements is less than the chunk size, and spare us some annoying code. - expectedNumberOfChunks := subjectsToDispatch.ValueLen()/int(crc.dispatchChunkSize) + 1 - toDispatch := make([]directDispatch, 0, expectedNumberOfChunks) - subjectsToDispatch.ForEachType(func(rr *core.RelationReference, resourceIds []string) { - chunkCount := 0.0 - slicez.ForEachChunk(resourceIds, crc.dispatchChunkSize, func(resourceIdChunk []string) { - chunkCount++ - toDispatch = append(toDispatch, directDispatch{ - resourceType: rr, - resourceIds: resourceIdChunk, - }) - }) - dispatchChunkCountHistogram.Observe(chunkCount) - }) - - // If there are caveats on the incoming relationships, then we must require all results to be - // found, as we need to ensure that all caveats are used for building the final expression. - resultsSetting := crc.resultsSetting - if hasCaveats { - resultsSetting = v1.DispatchCheckRequest_REQUIRE_ALL_RESULTS - } - // Dispatch and map to the associated resource ID(s). - result := union(ctx, crc, toDispatch, func(ctx context.Context, crc currentRequestContext, dd directDispatch) CheckResult { + toDispatch := checksToDispatch.dispatchChunks(crc.dispatchChunkSize) + result := union(ctx, crc, toDispatch, func(ctx context.Context, crc currentRequestContext, dd checkDispatchChunk) CheckResult { + // If there are caveats on any of the incoming relationships for the subjects to dispatch, then we must require all + // results to be found, as we need to ensure that all caveats are used for building the final expression. + resultsSetting := crc.resultsSetting + if dd.hasIncomingCaveats { + resultsSetting = v1.DispatchCheckRequest_REQUIRE_ALL_RESULTS + } + childResult := cc.dispatch(ctx, crc, ValidatedCheckRequest{ &v1.DispatchCheckRequest{ - ResourceRelation: dd.resourceType, + ResourceRelation: tuple.RelationReference(dd.resourceType.namespace, dd.resourceType.relation), ResourceIds: dd.resourceIds, Subject: crc.parentReq.Subject, ResultsSetting: resultsSetting, @@ -513,21 +481,24 @@ func (cc *ConcurrentChecker) checkDirect(ctx context.Context, crc currentRequest return childResult } - return mapFoundResources(childResult, dd.resourceType, relationshipsBySubjectONR) + return mapFoundResources(childResult, dd.resourceType, checksToDispatch) }, cc.concurrencyLimit) return combineResultWithFoundResources(result, foundResources) } -func mapFoundResources(result CheckResult, resourceType *core.RelationReference, relationshipsBySubjectONR *mapz.MultiMap[string, *core.RelationTuple]) CheckResult { +func mapFoundResources(result CheckResult, resourceType relationRef, checksToDispatch *checkDispatchSet) CheckResult { // Map any resources found to the parent resource IDs. membershipSet := NewMembershipSet() for foundResourceID, result := range result.Resp.ResultsByResourceId { - subjectKey := tuple.StringONRStrings(resourceType.Namespace, foundResourceID, resourceType.Relation) + resourceIDAndCaveats := checksToDispatch.mappingsForSubject(resourceType.namespace, foundResourceID, resourceType.relation) + + spiceerrors.DebugAssert(func() bool { + return len(resourceIDAndCaveats) > 0 + }, "found resource ID without associated caveats") - tuples, _ := relationshipsBySubjectONR.Get(subjectKey) - for _, relationTuple := range tuples { - membershipSet.AddMemberViaRelationship(relationTuple.ResourceAndRelation.ObjectId, result.Expression, relationTuple) + for _, riac := range resourceIDAndCaveats { + membershipSet.AddMemberWithParentCaveat(riac.resourceID, result.Expression, riac.caveat) } } @@ -595,7 +566,7 @@ func (cc *ConcurrentChecker) runSetOperation(ctx context.Context, crc currentReq } } -func (cc *ConcurrentChecker) checkComputedUserset(ctx context.Context, crc currentRequestContext, cu *core.ComputedUserset, rr *core.RelationReference, resourceIds []string) CheckResult { +func (cc *ConcurrentChecker) checkComputedUserset(ctx context.Context, crc currentRequestContext, cu *core.ComputedUserset, rr *relationRef, resourceIds []string) CheckResult { ctx, span := tracer.Start(ctx, cu.Relation) defer span.End() @@ -606,7 +577,7 @@ func (cc *ConcurrentChecker) checkComputedUserset(ctx context.Context, crc curre return checkResultError(spiceerrors.MustBugf("computed userset for tupleset without tuples"), emptyMetadata) } - startNamespace = rr.Namespace + startNamespace = rr.namespace targetResourceIds = resourceIds } else if cu.Object == core.ComputedUserset_TUPLE_OBJECT { if rr != nil { @@ -693,7 +664,7 @@ type ttu[T relation] interface { type checkResultWithType struct { CheckResult - relationType *core.RelationReference + relationType relationRef } func checkIntersectionTupleToUserset( @@ -719,38 +690,21 @@ func checkIntersectionTupleToUserset( } defer it.Close() - subjectsToDispatch := tuple.NewONRByTypeSet() - relationshipsBySubjectONR := mapz.NewMultiMap[string, *core.RelationTuple]() + checksToDispatch := newCheckDispatchSet() subjectsByResourceID := mapz.NewMultiMap[string, *core.ObjectAndRelation]() for tpl := it.Next(); tpl != nil; tpl = it.Next() { if it.Err() != nil { return checkResultError(NewCheckFailureErr(it.Err()), emptyMetadata) } - subjectsToDispatch.Add(tpl.Subject) - relationshipsBySubjectONR.Add(tuple.StringONR(tpl.Subject), tpl) + checksToDispatch.addForRelationship(tpl) subjectsByResourceID.Add(tpl.ResourceAndRelation.ObjectId, tpl.Subject) } it.Close() // Convert the subjects into batched requests. - // To simplify the logic, +1 is added to account for the situation where - // the number of elements is less than the chunk size, and spare us some annoying code. - expectedNumberOfChunks := uint16(subjectsToDispatch.ValueLen())/crc.dispatchChunkSize + 1 - toDispatch := make([]directDispatch, 0, expectedNumberOfChunks) - subjectsToDispatch.ForEachType(func(rr *core.RelationReference, resourceIds []string) { - chunkCount := 0.0 - slicez.ForEachChunk(resourceIds, crc.dispatchChunkSize, func(resourceIdChunk []string) { - chunkCount++ - toDispatch = append(toDispatch, directDispatch{ - resourceType: rr, - resourceIds: resourceIdChunk, - }) - }) - dispatchChunkCountHistogram.Observe(chunkCount) - }) - - if subjectsToDispatch.IsEmpty() { + toDispatch := checksToDispatch.dispatchChunks(crc.dispatchChunkSize) + if len(toDispatch) == 0 { return noMembers() } @@ -766,8 +720,9 @@ func checkIntersectionTupleToUserset( dispatchChunkSize: crc.dispatchChunkSize, }, toDispatch, - func(ctx context.Context, crc currentRequestContext, dd directDispatch) checkResultWithType { - childResult := cc.checkComputedUserset(ctx, crc, ttu.GetComputedUserset(), dd.resourceType, dd.resourceIds) + func(ctx context.Context, crc currentRequestContext, dd checkDispatchChunk) checkResultWithType { + resourceType := dd.resourceType + childResult := cc.checkComputedUserset(ctx, crc, ttu.GetComputedUserset(), &resourceType, dd.resourceIds) return checkResultWithType{ CheckResult: childResult, relationType: dd.resourceType, @@ -780,19 +735,18 @@ func checkIntersectionTupleToUserset( } // Create a membership set per-subject-type, representing the membership for each of the dispatched subjects. - resultsByDispatchedSubject := map[string]*MembershipSet{} + resultsByDispatchedSubject := map[relationRef]*MembershipSet{} combinedMetadata := emptyMetadata for _, result := range chunkResults { if result.Err != nil { return checkResultError(result.Err, emptyMetadata) } - typeKey := tuple.StringRR(result.relationType) - if _, ok := resultsByDispatchedSubject[typeKey]; !ok { - resultsByDispatchedSubject[typeKey] = NewMembershipSet() + if _, ok := resultsByDispatchedSubject[result.relationType]; !ok { + resultsByDispatchedSubject[result.relationType] = NewMembershipSet() } - resultsByDispatchedSubject[typeKey].UnionWith(result.Resp.ResultsByResourceId) + resultsByDispatchedSubject[result.relationType].UnionWith(result.Resp.ResultsByResourceId) combinedMetadata = combineResponseMetadata(combinedMetadata, result.Resp.Metadata) } @@ -813,11 +767,7 @@ func checkIntersectionTupleToUserset( // was found for each. If any are not found, then the resource ID is not a member. // We also collect up the caveats for each subject, as they will be added to the final result. for _, subject := range subjects { - subjectTypeKey := tuple.StringRR(&core.RelationReference{ - Namespace: subject.Namespace, - Relation: subject.Relation, - }) - + subjectTypeKey := relationRef{subject.Namespace, subject.Relation} results, ok := resultsByDispatchedSubject[subjectTypeKey] if !ok { hasAllSubjects = false @@ -835,11 +785,10 @@ func checkIntersectionTupleToUserset( } // Add any caveats on the subject from the starting relationship(s) as well. - subjectKey := tuple.StringONR(subject) - tuples, _ := relationshipsBySubjectONR.Get(subjectKey) - for _, relationTuple := range tuples { - if relationTuple.Caveat != nil { - caveats = append(caveats, wrapCaveat(relationTuple.Caveat)) + resourceIDAndCaveats := checksToDispatch.mappingsForSubject(subject.Namespace, subject.ObjectId, subject.Relation) + for _, riac := range resourceIDAndCaveats { + if riac.caveat != nil { + caveats = append(caveats, wrapCaveat(riac.caveat)) } } } @@ -904,46 +853,28 @@ func checkTupleToUserset[T relation]( } defer it.Close() - subjectsToDispatch := tuple.NewONRByTypeSet() - relationshipsBySubjectONR := mapz.NewMultiMap[string, *core.RelationTuple]() + checksToDispatch := newCheckDispatchSet() for tpl := it.Next(); tpl != nil; tpl = it.Next() { if it.Err() != nil { return checkResultError(NewCheckFailureErr(it.Err()), emptyMetadata) } - - subjectsToDispatch.Add(tpl.Subject) - relationshipsBySubjectONR.Add(tuple.StringONR(tpl.Subject), tpl) + checksToDispatch.addForRelationship(tpl) } it.Close() - // Convert the subjects into batched requests. - // To simplify the logic, +1 is added to account for the situation where - // the number of elements is less than the chunk size, and spare us some annoying code. - expectedNumberOfChunks := uint16(subjectsToDispatch.ValueLen())/crc.dispatchChunkSize + 1 - toDispatch := make([]directDispatch, 0, expectedNumberOfChunks) - subjectsToDispatch.ForEachType(func(rr *core.RelationReference, resourceIds []string) { - chunkCount := 0.0 - slicez.ForEachChunk(resourceIds, crc.dispatchChunkSize, func(resourceIdChunk []string) { - chunkCount++ - toDispatch = append(toDispatch, directDispatch{ - resourceType: rr, - resourceIds: resourceIdChunk, - }) - }) - dispatchChunkCountHistogram.Observe(chunkCount) - }) - + toDispatch := checksToDispatch.dispatchChunks(crc.dispatchChunkSize) return combineWithComputedHints(union( ctx, crc, toDispatch, - func(ctx context.Context, crc currentRequestContext, dd directDispatch) CheckResult { - childResult := cc.checkComputedUserset(ctx, crc, ttu.GetComputedUserset(), dd.resourceType, dd.resourceIds) + func(ctx context.Context, crc currentRequestContext, dd checkDispatchChunk) CheckResult { + resourceType := dd.resourceType + childResult := cc.checkComputedUserset(ctx, crc, ttu.GetComputedUserset(), &resourceType, dd.resourceIds) if childResult.Err != nil { return childResult } - return mapFoundResources(childResult, dd.resourceType, relationshipsBySubjectONR) + return mapFoundResources(childResult, dd.resourceType, checksToDispatch) }, cc.concurrencyLimit, ), hintsToReturn) diff --git a/internal/graph/checkdispatchset.go b/internal/graph/checkdispatchset.go new file mode 100644 index 0000000000..331157e580 --- /dev/null +++ b/internal/graph/checkdispatchset.go @@ -0,0 +1,166 @@ +package graph + +import ( + "sort" + + "github.com/authzed/spicedb/pkg/genutil/mapz" + "github.com/authzed/spicedb/pkg/genutil/slicez" + core "github.com/authzed/spicedb/pkg/proto/core/v1" + "github.com/authzed/spicedb/pkg/spiceerrors" +) + +// checkDispatchSet is the set of subjects over which check will need to dispatch +// as subproblems in order to answer the parent problem. +type checkDispatchSet struct { + // bySubjectType is a map from the type of subject to the set of subjects of that type + // over which to dispatch, along with information indicating whether caveats are present + // for that chunk. + bySubjectType map[relationRef]map[string]bool + + // bySubject is a map from the subject to the set of resources for which the subject + // has a relationship, along with the caveats that apply to that relationship. + bySubject *mapz.MultiMap[subjectRef, resourceIDAndCaveat] +} + +// checkDispatchChunk is a chunk of subjects over which to dispatch a check operation. +type checkDispatchChunk struct { + // resourceType is the type of the subjects in this chunk. + resourceType relationRef + + // resourceIds is the set of subjects in this chunk. + resourceIds []string + + // hasIncomingCaveats is true if any of the subjects in this chunk have incoming caveats. + // This is used to determine whether the check operation should be dispatched requiring + // all results. + hasIncomingCaveats bool +} + +// subjectIDAndHasCaveat is a tuple of a subject ID and whether it has a caveat. +type subjectIDAndHasCaveat struct { + // objectID is the ID of the subject. + objectID string + + // hasIncomingCaveats is true if the subject has a caveat. + hasIncomingCaveats bool +} + +// resourceIDAndCaveat is a tuple of a resource ID and a caveat. +type resourceIDAndCaveat struct { + // resourceID is the ID of the resource. + resourceID string + + // caveat is the caveat that applies to the relationship between the subject and the resource. + // May be nil. + caveat *core.ContextualizedCaveat +} + +// relationRef is a tuple of a namespace and a relation. +type relationRef struct { + namespace string + relation string +} + +// subjectRef is a tuple of a namespace, an object ID, and a relation. +type subjectRef struct { + namespace string + objectID string + relation string +} + +// newCheckDispatchSet creates and returns a new checkDispatchSet. +func newCheckDispatchSet() *checkDispatchSet { + return &checkDispatchSet{ + bySubjectType: map[relationRef]map[string]bool{}, + bySubject: mapz.NewMultiMap[subjectRef, resourceIDAndCaveat](), + } +} + +// Add adds the specified ObjectAndRelation to the set. +func (s *checkDispatchSet) addForRelationship(tpl *core.RelationTuple) { + // Add an entry for the subject pointing to the resource ID and caveat for the subject. + riac := resourceIDAndCaveat{ + resourceID: tpl.ResourceAndRelation.ObjectId, + caveat: tpl.Caveat, + } + subjectRef := subjectRef{ + namespace: tpl.Subject.Namespace, + objectID: tpl.Subject.ObjectId, + relation: tpl.Subject.Relation, + } + s.bySubject.Add(subjectRef, riac) + + // Add the subject ID to the map of subjects for the type of subject. + siac := subjectIDAndHasCaveat{ + objectID: tpl.Subject.ObjectId, + hasIncomingCaveats: tpl.Caveat != nil && tpl.Caveat.CaveatName != "", + } + subjectTypeRef := relationRef{namespace: tpl.Subject.Namespace, relation: tpl.Subject.Relation} + + subjectIDsForType, ok := s.bySubjectType[subjectTypeRef] + if !ok { + subjectIDsForType = make(map[string]bool) + s.bySubjectType[subjectTypeRef] = subjectIDsForType + } + + // If a caveat exists for the subject ID in any branch, the whole branch is considered caveated. + subjectIDsForType[tpl.Subject.ObjectId] = siac.hasIncomingCaveats || subjectIDsForType[tpl.Subject.ObjectId] +} + +func (s *checkDispatchSet) dispatchChunks(dispatchChunkSize uint16) []checkDispatchChunk { + // Start with an estimate of one chunk per type, plus one for the remainder. + expectedNumberOfChunks := len(s.bySubjectType) + 1 + toDispatch := make([]checkDispatchChunk, 0, expectedNumberOfChunks) + + // For each type of subject, create chunks of the IDs over which to dispatch. + for subjectType, subjectIDsAndHasCaveats := range s.bySubjectType { + entries := make([]subjectIDAndHasCaveat, 0, len(subjectIDsAndHasCaveats)) + for objectID, hasIncomingCaveats := range subjectIDsAndHasCaveats { + entries = append(entries, subjectIDAndHasCaveat{objectID: objectID, hasIncomingCaveats: hasIncomingCaveats}) + } + + // Sort the list of subject IDs by whether they have caveats and then the ID itself. + sort.Slice(entries, func(i, j int) bool { + iHasCaveat := entries[i].hasIncomingCaveats + jHasCaveat := entries[j].hasIncomingCaveats + if iHasCaveat == jHasCaveat { + return entries[i].objectID < entries[j].objectID + } + return iHasCaveat && !jHasCaveat + }) + + chunkCount := 0.0 + slicez.ForEachChunk(entries, dispatchChunkSize, func(subjectIdChunk []subjectIDAndHasCaveat) { + chunkCount++ + + subjectIDsToDispatch := make([]string, 0, len(subjectIdChunk)) + hasIncomingCaveats := false + for _, entry := range subjectIdChunk { + subjectIDsToDispatch = append(subjectIDsToDispatch, entry.objectID) + hasIncomingCaveats = hasIncomingCaveats || entry.hasIncomingCaveats + } + + toDispatch = append(toDispatch, checkDispatchChunk{ + resourceType: subjectType, + resourceIds: subjectIDsToDispatch, + hasIncomingCaveats: hasIncomingCaveats, + }) + }) + dispatchChunkCountHistogram.Observe(chunkCount) + } + + return toDispatch +} + +// mappingsForSubject returns the mappings that apply to the relationship between the specified +// subject and any of its resources. The returned caveats include the resource ID of the resource +// that the subject has a relationship with. +func (s *checkDispatchSet) mappingsForSubject(subjectType string, subjectObjectID string, subjectRelation string) []resourceIDAndCaveat { + results, ok := s.bySubject.Get(subjectRef{ + namespace: subjectType, + objectID: subjectObjectID, + relation: subjectRelation, + }) + spiceerrors.DebugAssert(func() bool { return ok }, "no caveats found for subject %s:%s:%s", subjectType, subjectObjectID, subjectRelation) + return results +} diff --git a/internal/graph/checkdispatchset_test.go b/internal/graph/checkdispatchset_test.go new file mode 100644 index 0000000000..cefbd17aed --- /dev/null +++ b/internal/graph/checkdispatchset_test.go @@ -0,0 +1,390 @@ +package graph + +import ( + "sort" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/authzed/spicedb/internal/caveats" + core "github.com/authzed/spicedb/pkg/proto/core/v1" + "github.com/authzed/spicedb/pkg/tuple" +) + +var caveatForTesting = caveats.CaveatForTesting + +func TestCheckDispatchSet(t *testing.T) { + tcs := []struct { + name string + relationships []*core.RelationTuple + dispatchChunkSize uint16 + expectedChunks []checkDispatchChunk + expectedMappings map[string][]resourceIDAndCaveat + }{ + { + "basic", + []*core.RelationTuple{ + tuple.MustParse("document:somedoc#viewer@group:1#member"), + tuple.MustParse("document:somedoc#viewer@group:2#member"), + tuple.MustParse("document:somedoc#viewer@group:3#member"), + }, + 100, + []checkDispatchChunk{ + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"1", "2", "3"}, + hasIncomingCaveats: false, + }, + }, + map[string][]resourceIDAndCaveat{ + "group:1#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:2#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:3#member": { + {resourceID: "somedoc", caveat: nil}, + }, + }, + }, + { + "basic chunking", + []*core.RelationTuple{ + tuple.MustParse("document:somedoc#viewer@group:1#member"), + tuple.MustParse("document:somedoc#viewer@group:2#member"), + tuple.MustParse("document:somedoc#viewer@group:3#member"), + }, + 2, + []checkDispatchChunk{ + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"1", "2"}, + hasIncomingCaveats: false, + }, + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"3"}, + hasIncomingCaveats: false, + }, + }, + map[string][]resourceIDAndCaveat{ + "group:1#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:2#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:3#member": { + {resourceID: "somedoc", caveat: nil}, + }, + }, + }, + { + "different subject types", + []*core.RelationTuple{ + tuple.MustParse("document:somedoc#viewer@group:1#member"), + tuple.MustParse("document:somedoc#viewer@group:2#member"), + tuple.MustParse("document:somedoc#viewer@group:3#member"), + tuple.MustParse("document:somedoc#viewer@anothertype:1#member"), + tuple.MustParse("document:somedoc#viewer@anothertype:2#member"), + tuple.MustParse("document:somedoc#viewer@anothertype:3#member"), + }, + 100, + []checkDispatchChunk{ + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"1", "2", "3"}, + hasIncomingCaveats: false, + }, + { + resourceType: relationRef{namespace: "anothertype", relation: "member"}, + resourceIds: []string{"1", "2", "3"}, + hasIncomingCaveats: false, + }, + }, + map[string][]resourceIDAndCaveat{ + "group:1#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:2#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:3#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "anothertype:1#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "anothertype:2#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "anothertype:3#member": { + {resourceID: "somedoc", caveat: nil}, + }, + }, + }, + { + "different subject types mixed", + []*core.RelationTuple{ + tuple.MustParse("document:somedoc#viewer@group:1#member"), + tuple.MustParse("document:somedoc#viewer@anothertype:1#member"), + tuple.MustParse("document:somedoc#viewer@anothertype:2#member"), + tuple.MustParse("document:somedoc#viewer@group:2#member"), + tuple.MustParse("document:somedoc#viewer@group:3#member"), + tuple.MustParse("document:somedoc#viewer@anothertype:3#member"), + }, + 100, + []checkDispatchChunk{ + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"1", "2", "3"}, + hasIncomingCaveats: false, + }, + { + resourceType: relationRef{namespace: "anothertype", relation: "member"}, + resourceIds: []string{"1", "2", "3"}, + hasIncomingCaveats: false, + }, + }, + map[string][]resourceIDAndCaveat{ + "group:1#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:2#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:3#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "anothertype:1#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "anothertype:2#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "anothertype:3#member": { + {resourceID: "somedoc", caveat: nil}, + }, + }, + }, + { + "different subject types with chunking", + []*core.RelationTuple{ + tuple.MustParse("document:somedoc#viewer@group:1#member"), + tuple.MustParse("document:somedoc#viewer@group:2#member"), + tuple.MustParse("document:somedoc#viewer@group:3#member"), + tuple.MustParse("document:somedoc#viewer@anothertype:1#member"), + tuple.MustParse("document:somedoc#viewer@anothertype:2#member"), + tuple.MustParse("document:somedoc#viewer@anothertype:3#member"), + }, + 2, + []checkDispatchChunk{ + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"1", "2"}, + hasIncomingCaveats: false, + }, + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"3"}, + hasIncomingCaveats: false, + }, + { + resourceType: relationRef{namespace: "anothertype", relation: "member"}, + resourceIds: []string{"1", "2"}, + hasIncomingCaveats: false, + }, + { + resourceType: relationRef{namespace: "anothertype", relation: "member"}, + resourceIds: []string{"3"}, + hasIncomingCaveats: false, + }, + }, + map[string][]resourceIDAndCaveat{ + "group:1#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:2#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:3#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "anothertype:1#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "anothertype:2#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "anothertype:3#member": { + {resourceID: "somedoc", caveat: nil}, + }, + }, + }, + { + "some caveated members", + []*core.RelationTuple{ + tuple.MustParse("document:somedoc#viewer@group:1#member[somecaveat]"), + tuple.MustParse("document:somedoc#viewer@group:2#member"), + tuple.MustParse("document:somedoc#viewer@group:3#member"), + }, + 100, + []checkDispatchChunk{ + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"1", "2", "3"}, + hasIncomingCaveats: true, + }, + }, + map[string][]resourceIDAndCaveat{ + "group:1#member": { + {resourceID: "somedoc", caveat: caveatForTesting("somecaveat")}, + }, + "group:2#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:3#member": { + {resourceID: "somedoc", caveat: nil}, + }, + }, + }, + { + "caveated members combined when chunking", + []*core.RelationTuple{ + tuple.MustParse("document:somedoc#viewer@group:1#member[somecaveat]"), + tuple.MustParse("document:somedoc#viewer@group:2#member"), + tuple.MustParse("document:somedoc#viewer@group:3#member"), + tuple.MustParse("document:somedoc#viewer@group:4#member[somecaveat]"), + }, + 2, + []checkDispatchChunk{ + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"2", "3"}, + hasIncomingCaveats: false, + }, + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"1", "4"}, + hasIncomingCaveats: true, + }, + }, + map[string][]resourceIDAndCaveat{ + "group:1#member": { + {resourceID: "somedoc", caveat: caveatForTesting("somecaveat")}, + }, + "group:2#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:3#member": { + {resourceID: "somedoc", caveat: nil}, + }, + "group:4#member": { + {resourceID: "somedoc", caveat: caveatForTesting("somecaveat")}, + }, + }, + }, + { + "different resources leading to the same subject", + []*core.RelationTuple{ + tuple.MustParse("document:somedoc#viewer@group:1#member"), + tuple.MustParse("document:anotherdoc#viewer@group:1#member"), + tuple.MustParse("document:somedoc#viewer@group:2#member"), + tuple.MustParse("document:anotherdoc#viewer@group:2#member"), + }, + 2, + []checkDispatchChunk{ + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"1", "2"}, + hasIncomingCaveats: false, + }, + }, + map[string][]resourceIDAndCaveat{ + "group:1#member": { + {resourceID: "somedoc", caveat: nil}, + {resourceID: "anotherdoc", caveat: nil}, + }, + "group:2#member": { + {resourceID: "somedoc", caveat: nil}, + {resourceID: "anotherdoc", caveat: nil}, + }, + }, + }, + { + "different resources leading to the same subject with caveats", + []*core.RelationTuple{ + tuple.MustParse("document:somedoc#viewer@group:1#member[somecaveat]"), + tuple.MustParse("document:anotherdoc#viewer@group:1#member"), + tuple.MustParse("document:somedoc#viewer@group:2#member"), + tuple.MustParse("document:anotherdoc#viewer@group:2#member[somecaveat]"), + }, + 2, + []checkDispatchChunk{ + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"1", "2"}, + hasIncomingCaveats: true, + }, + }, + map[string][]resourceIDAndCaveat{ + "group:1#member": { + {resourceID: "somedoc", caveat: caveatForTesting("somecaveat")}, + {resourceID: "anotherdoc", caveat: nil}, + }, + "group:2#member": { + {resourceID: "somedoc", caveat: nil}, + {resourceID: "anotherdoc", caveat: caveatForTesting("somecaveat")}, + }, + }, + }, + { + "different resource leading to the same subject with caveats", + []*core.RelationTuple{ + tuple.MustParse("document:anotherdoc#viewer@group:1#member"), + tuple.MustParse("document:thirddoc#viewer@group:1#member"), + tuple.MustParse("document:somedoc#viewer@group:1#member[somecaveat]"), + }, + 2, + []checkDispatchChunk{ + { + resourceType: relationRef{namespace: "group", relation: "member"}, + resourceIds: []string{"1"}, + hasIncomingCaveats: true, + }, + }, + map[string][]resourceIDAndCaveat{ + "group:1#member": { + {resourceID: "somedoc", caveat: caveatForTesting("somecaveat")}, + {resourceID: "anotherdoc", caveat: nil}, + {resourceID: "thirddoc", caveat: nil}, + }, + }, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + set := newCheckDispatchSet() + for _, rel := range tc.relationships { + set.addForRelationship(rel) + } + + chunks := set.dispatchChunks(tc.dispatchChunkSize) + for _, c := range chunks { + sort.Strings(c.resourceIds) + } + + require.ElementsMatch(t, tc.expectedChunks, chunks, "difference in expected chunks. found: %v", chunks) + + for subjectString, expectedMappings := range tc.expectedMappings { + parsed := tuple.ParseSubjectONR(subjectString) + require.NotNil(t, parsed) + + mappings := set.mappingsForSubject(parsed.Namespace, parsed.ObjectId, parsed.Relation) + require.ElementsMatch(t, expectedMappings, mappings) + } + }) + } +} diff --git a/internal/graph/membershipset.go b/internal/graph/membershipset.go index 1eacf1fd86..6c9157a329 100644 --- a/internal/graph/membershipset.go +++ b/internal/graph/membershipset.go @@ -58,7 +58,17 @@ func (ms *MembershipSet) AddMemberViaRelationship( resourceCaveatExpression *core.CaveatExpression, parentRelationship *core.RelationTuple, ) { - intersection := caveatAnd(wrapCaveat(parentRelationship.Caveat), resourceCaveatExpression) + ms.AddMemberWithParentCaveat(resourceID, resourceCaveatExpression, parentRelationship.Caveat) +} + +// AddMemberWithParentCaveat adds the given resource ID as a member with the parent caveat +// combined via intersection with the resource's caveat. The parent caveat may be nil. +func (ms *MembershipSet) AddMemberWithParentCaveat( + resourceID string, + resourceCaveatExpression *core.CaveatExpression, + parentCaveat *core.ContextualizedCaveat, +) { + intersection := caveatAnd(wrapCaveat(parentCaveat), resourceCaveatExpression) ms.addMember(resourceID, intersection) } diff --git a/pkg/tuple/onrbytypeset.go b/pkg/tuple/onrbytypeset.go deleted file mode 100644 index 56da96ddbc..0000000000 --- a/pkg/tuple/onrbytypeset.go +++ /dev/null @@ -1,82 +0,0 @@ -package tuple - -import ( - "github.com/samber/lo" - - core "github.com/authzed/spicedb/pkg/proto/core/v1" -) - -// ONRByTypeSet is a set of ObjectAndRelation's, grouped by namespace+relation. -type ONRByTypeSet struct { - byType map[string][]string -} - -// NewONRByTypeSet creates and returns a new ONRByTypeSet. -func NewONRByTypeSet() *ONRByTypeSet { - return &ONRByTypeSet{ - byType: map[string][]string{}, - } -} - -// Add adds the specified ObjectAndRelation to the set. -func (s *ONRByTypeSet) Add(onr *core.ObjectAndRelation) { - key := JoinRelRef(onr.Namespace, onr.Relation) - if _, ok := s.byType[key]; !ok { - s.byType[key] = []string{} - } - - s.byType[key] = append(s.byType[key], onr.ObjectId) -} - -// ForEachType invokes the handler for each type of ObjectAndRelation found in the set, along -// with all IDs of objects of that type. -func (s *ONRByTypeSet) ForEachType(handler func(rr *core.RelationReference, objectIds []string)) { - for key, objectIds := range s.byType { - ns, rel := MustSplitRelRef(key) - handler(&core.RelationReference{ - Namespace: ns, - Relation: rel, - }, lo.Uniq(objectIds)) - } -} - -// Map runs the mapper function over each type of object in the set, returning a new ONRByTypeSet with -// the object type replaced by that returned by the mapper function. -func (s *ONRByTypeSet) Map(mapper func(rr *core.RelationReference) (*core.RelationReference, error)) (*ONRByTypeSet, error) { - mapped := NewONRByTypeSet() - for key, objectIds := range s.byType { - ns, rel := MustSplitRelRef(key) - updatedType, err := mapper(&core.RelationReference{ - Namespace: ns, - Relation: rel, - }) - if err != nil { - return nil, err - } - if updatedType == nil { - continue - } - mapped.byType[JoinRelRef(updatedType.Namespace, updatedType.Relation)] = lo.Uniq(objectIds) - } - return mapped, nil -} - -// IsEmpty returns true if the set is empty. -func (s *ONRByTypeSet) IsEmpty() bool { - return len(s.byType) == 0 -} - -// KeyLen returns the number of keys in the set. -func (s *ONRByTypeSet) KeyLen() int { - return len(s.byType) -} - -// ValueLen returns the number of values in the set. -func (s *ONRByTypeSet) ValueLen() int { - var total int - for _, vals := range s.byType { - total += len(vals) - } - - return total -} diff --git a/pkg/tuple/onrbytypeset_test.go b/pkg/tuple/onrbytypeset_test.go deleted file mode 100644 index 00254e0330..0000000000 --- a/pkg/tuple/onrbytypeset_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package tuple - -import ( - "sort" - "testing" - - core "github.com/authzed/spicedb/pkg/proto/core/v1" - - "github.com/stretchr/testify/require" -) - -func RR(namespaceName string, relationName string) *core.RelationReference { - return &core.RelationReference{ - Namespace: namespaceName, - Relation: relationName, - } -} - -func TestONRByTypeSet(t *testing.T) { - assertHasObjectIds := func(s *ONRByTypeSet, rr *core.RelationReference, expected []string) { - wasFound := false - s.ForEachType(func(foundRR *core.RelationReference, objectIds []string) { - if rr.EqualVT(foundRR) { - sort.Strings(objectIds) - require.Equal(t, expected, objectIds) - wasFound = true - } - }) - require.True(t, wasFound) - } - - set := NewONRByTypeSet() - require.True(t, set.IsEmpty()) - - // Add some ONRs. - set.Add(ParseONR("document:foo#viewer")) - set.Add(ParseONR("document:bar#viewer")) - set.Add(ParseONR("team:something#member")) - set.Add(ParseONR("team:other#member")) - set.Add(ParseONR("team:other#manager")) - require.False(t, set.IsEmpty()) - - // Run for each type over the set - assertHasObjectIds(set, RR("document", "viewer"), []string{"bar", "foo"}) - assertHasObjectIds(set, RR("team", "member"), []string{"other", "something"}) - assertHasObjectIds(set, RR("team", "manager"), []string{"other"}) - - // Map - mapped, err := set.Map(func(rr *core.RelationReference) (*core.RelationReference, error) { - if rr.Namespace == "document" { - return RR("doc", rr.Relation), nil - } - - return rr, nil - }) - require.NoError(t, err) - - assertHasObjectIds(mapped, RR("doc", "viewer"), []string{"bar", "foo"}) - assertHasObjectIds(mapped, RR("team", "member"), []string{"other", "something"}) - assertHasObjectIds(mapped, RR("team", "manager"), []string{"other"}) -}