diff --git a/internal/services/v1/relationships.go b/internal/services/v1/relationships.go index cce700a173..b76e5efd6d 100644 --- a/internal/services/v1/relationships.go +++ b/internal/services/v1/relationships.go @@ -28,7 +28,6 @@ import ( "github.com/authzed/spicedb/pkg/datastore/pagination" "github.com/authzed/spicedb/pkg/genutil/mapz" "github.com/authzed/spicedb/pkg/middleware/consistency" - core "github.com/authzed/spicedb/pkg/proto/core/v1" dispatchv1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1" "github.com/authzed/spicedb/pkg/tuple" "github.com/authzed/spicedb/pkg/zedtoken" @@ -351,12 +350,19 @@ func (ps *permissionServer) DeleteRelationships(ctx context.Context, req *v1.Del return err } - var deleteMutations []*core.RelationTupleUpdate + usagemetrics.SetInContext(ctx, &dispatchv1.ResponseMeta{ + // One request per precondition and one request for the actual delete. + DispatchCount: uint32(len(req.OptionalPreconditions)) + 1, + }) - if req.OptionalLimit > 0 { - limit := uint64(req.OptionalLimit) - deleteMutations = make([]*core.RelationTupleUpdate, 0, limit) + if err := checkPreconditions(ctx, rwt, req.OptionalPreconditions); err != nil { + return err + } + // If a limit was specified but partial deletion is not allowed, we need to check if the + // number of relationships to be deleted exceeds the limit. + if req.OptionalLimit > 0 && !req.OptionalAllowPartialDeletions { + limit := uint64(req.OptionalLimit) limitPlusOne := limit + 1 filter := datastore.RelationshipsFilterFromPublicFilter(req.RelationshipFilter) @@ -366,38 +372,37 @@ func (ps *permissionServer) DeleteRelationships(ctx context.Context, req *v1.Del } defer iter.Close() + counter := 0 for tpl := iter.Next(); tpl != nil; tpl = iter.Next() { if iter.Err() != nil { return ps.rewriteError(ctx, err) } - if len(deleteMutations) == int(limit) { - deletionProgress = v1.DeleteRelationshipsResponse_DELETION_PROGRESS_PARTIAL - if !req.OptionalAllowPartialDeletions { - return ps.rewriteError(ctx, NewCouldNotTransactionallyDeleteErr(req.RelationshipFilter, req.OptionalLimit)) - } - - break + if counter == int(limit) { + return ps.rewriteError(ctx, NewCouldNotTransactionallyDeleteErr(req.RelationshipFilter, req.OptionalLimit)) } - deleteMutations = append(deleteMutations, tuple.Delete(tpl)) + counter++ } iter.Close() } - usagemetrics.SetInContext(ctx, &dispatchv1.ResponseMeta{ - // One request per precondition and one request for the actual delete. - DispatchCount: uint32(len(req.OptionalPreconditions)) + 1, - }) + // Delete with the specified limit. + if req.OptionalLimit > 0 { + deleteLimit := uint64(req.OptionalLimit) + reachedLimit, err := rwt.DeleteRelationships(ctx, req.RelationshipFilter, options.WithDeleteLimit(&deleteLimit)) + if err != nil { + return err + } - if err := checkPreconditions(ctx, rwt, req.OptionalPreconditions); err != nil { - return err - } + if reachedLimit { + deletionProgress = v1.DeleteRelationshipsResponse_DELETION_PROGRESS_PARTIAL + } - if len(deleteMutations) > 0 { - return rwt.WriteRelationships(ctx, deleteMutations) + return nil } + // Otherwise, kick off an unlimited deletion. _, err := rwt.DeleteRelationships(ctx, req.RelationshipFilter) return err }) diff --git a/internal/services/v1/relationships_test.go b/internal/services/v1/relationships_test.go index 2683fdfbf4..e3993dc9e0 100644 --- a/internal/services/v1/relationships_test.go +++ b/internal/services/v1/relationships_test.go @@ -1122,10 +1122,7 @@ func TestDeleteRelationshipsBeyondLimitPartial(t *testing.T) { }) require.NoError(err) - headRev, err = ds.HeadRevision(context.Background()) - require.NoError(err) - - afterDelete := readOfType(require, "document", client, zedtoken.MustNewFromRevision(headRev)) + afterDelete := readOfType(require, "document", client, resp.DeletedAt) require.LessOrEqual(len(beforeDelete)-len(afterDelete), batchSize) if i == 0 {