Skip to content

Commit ab7b530

Browse files
vroldanbetjzelinskie
vroldanbet
authored and
jzelinskie
committed
address PR feedback
- check_test.go: move dispatch chunking in its own test file - chunking_test.go: test bigger number than MaxUInt16 - check_test.go: indentation of test schema
1 parent 93331d0 commit ab7b530

File tree

3 files changed

+127
-100
lines changed

3 files changed

+127
-100
lines changed

‎internal/dispatch/graph/check_test.go‎

Lines changed: 0 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package graph
33
import (
44
"context"
55
"fmt"
6-
"math"
76
"strings"
87
"testing"
98

@@ -411,91 +410,6 @@ func TestCheckDebugging(t *testing.T) {
411410
}
412411
}
413412

414-
func TestDispatchChunking(t *testing.T) {
415-
schema := `
416-
definition user {
417-
relation self: user
418-
}
419-
420-
definition res {
421-
relation owner : user
422-
permission view = owner->self
423-
}
424-
`
425-
426-
resources := make([]*core.RelationTuple, 0, math.MaxUint16+1)
427-
enabled := make([]*core.RelationTuple, 0, math.MaxUint16+1)
428-
for i := 0; i < math.MaxUint16+1; i++ {
429-
resources = append(resources, tuple.Parse(fmt.Sprintf("res:res1#owner@user:user%d", i)))
430-
enabled = append(enabled, tuple.Parse(fmt.Sprintf("user:user%d#self@user:user%d", i, i)))
431-
}
432-
433-
ctx, dispatcher, revision := newLocalDispatcherWithSchemaAndRels(t, schema, append(enabled, resources...))
434-
435-
t.Run("check", func(t *testing.T) {
436-
for _, tpl := range resources[:1] {
437-
checkResult, err := dispatcher.DispatchCheck(ctx, &v1.DispatchCheckRequest{
438-
ResourceRelation: RR(tpl.ResourceAndRelation.Namespace, "view"),
439-
ResourceIds: []string{tpl.ResourceAndRelation.ObjectId},
440-
ResultsSetting: v1.DispatchCheckRequest_ALLOW_SINGLE_RESULT,
441-
Subject: ONR(tpl.Subject.Namespace, tpl.Subject.ObjectId, graph.Ellipsis),
442-
Metadata: &v1.ResolverMeta{
443-
AtRevision: revision.String(),
444-
DepthRemaining: 50,
445-
},
446-
})
447-
448-
require.NoError(t, err)
449-
require.NotNil(t, checkResult)
450-
require.NotEmpty(t, checkResult.ResultsByResourceId, "expected membership for resource %s", tpl.ResourceAndRelation.ObjectId)
451-
require.Equal(t, v1.ResourceCheckResult_MEMBER, checkResult.ResultsByResourceId[tpl.ResourceAndRelation.ObjectId].Membership)
452-
}
453-
})
454-
455-
t.Run("lookup-resources", func(t *testing.T) {
456-
for _, tpl := range resources[:1] {
457-
stream := dispatch.NewCollectingDispatchStream[*v1.DispatchLookupResourcesResponse](ctx)
458-
err := dispatcher.DispatchLookupResources(&v1.DispatchLookupResourcesRequest{
459-
ObjectRelation: RR(tpl.ResourceAndRelation.Namespace, "view"),
460-
Subject: ONR(tpl.Subject.Namespace, tpl.Subject.ObjectId, graph.Ellipsis),
461-
Metadata: &v1.ResolverMeta{
462-
AtRevision: revision.String(),
463-
DepthRemaining: 50,
464-
},
465-
OptionalLimit: veryLargeLimit,
466-
}, stream)
467-
468-
require.NoError(t, err)
469-
470-
foundResources, _, _, _ := processResults(stream)
471-
require.Len(t, foundResources, 1)
472-
}
473-
})
474-
475-
t.Run("lookup-subjects", func(t *testing.T) {
476-
for _, tpl := range resources[:1] {
477-
stream := dispatch.NewCollectingDispatchStream[*v1.DispatchLookupSubjectsResponse](ctx)
478-
479-
err := dispatcher.DispatchLookupSubjects(&v1.DispatchLookupSubjectsRequest{
480-
ResourceRelation: RR(tpl.ResourceAndRelation.Namespace, "view"),
481-
ResourceIds: []string{tpl.ResourceAndRelation.ObjectId},
482-
SubjectRelation: RR(tpl.Subject.Namespace, graph.Ellipsis),
483-
Metadata: &v1.ResolverMeta{
484-
AtRevision: revision.String(),
485-
DepthRemaining: 50,
486-
},
487-
}, stream)
488-
489-
require.NoError(t, err)
490-
res := stream.Results()
491-
require.Len(t, res, 1)
492-
require.Len(t, res[0].FoundSubjectsByResourceId, 1)
493-
require.NotNil(t, res[0].FoundSubjectsByResourceId["res1"])
494-
require.Len(t, res[0].FoundSubjectsByResourceId["res1"].FoundSubjects, math.MaxUint16+1)
495-
}
496-
})
497-
}
498-
499413
func newLocalDispatcherWithConcurrencyLimit(t testing.TB, concurrencyLimit uint16) (context.Context, dispatch.Dispatcher, datastore.Revision) {
500414
rawDS, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC)
501415
require.NoError(t, err)

‎internal/dispatch/graph/dispatch_test.go‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package graph
2+
3+
import (
4+
"fmt"
5+
"math"
6+
"testing"
7+
8+
"github.com/authzed/spicedb/internal/dispatch"
9+
"github.com/authzed/spicedb/internal/graph"
10+
core "github.com/authzed/spicedb/pkg/proto/core/v1"
11+
v1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1"
12+
"github.com/authzed/spicedb/pkg/tuple"
13+
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestDispatchChunking(t *testing.T) {
18+
schema := `
19+
definition user {
20+
relation self: user
21+
}
22+
23+
definition res {
24+
relation owner : user
25+
permission view = owner->self
26+
}`
27+
28+
resources := make([]*core.RelationTuple, 0, math.MaxUint16+1)
29+
enabled := make([]*core.RelationTuple, 0, math.MaxUint16+1)
30+
for i := 0; i < math.MaxUint16+1; i++ {
31+
resources = append(resources, tuple.Parse(fmt.Sprintf("res:res1#owner@user:user%d", i)))
32+
enabled = append(enabled, tuple.Parse(fmt.Sprintf("user:user%d#self@user:user%d", i, i)))
33+
}
34+
35+
ctx, dispatcher, revision := newLocalDispatcherWithSchemaAndRels(t, schema, append(enabled, resources...))
36+
37+
t.Run("check", func(t *testing.T) {
38+
for _, tpl := range resources[:1] {
39+
checkResult, err := dispatcher.DispatchCheck(ctx, &v1.DispatchCheckRequest{
40+
ResourceRelation: RR(tpl.ResourceAndRelation.Namespace, "view"),
41+
ResourceIds: []string{tpl.ResourceAndRelation.ObjectId},
42+
ResultsSetting: v1.DispatchCheckRequest_ALLOW_SINGLE_RESULT,
43+
Subject: ONR(tpl.Subject.Namespace, tpl.Subject.ObjectId, graph.Ellipsis),
44+
Metadata: &v1.ResolverMeta{
45+
AtRevision: revision.String(),
46+
DepthRemaining: 50,
47+
},
48+
})
49+
50+
require.NoError(t, err)
51+
require.NotNil(t, checkResult)
52+
require.NotEmpty(t, checkResult.ResultsByResourceId, "expected membership for resource %s", tpl.ResourceAndRelation.ObjectId)
53+
require.Equal(t, v1.ResourceCheckResult_MEMBER, checkResult.ResultsByResourceId[tpl.ResourceAndRelation.ObjectId].Membership)
54+
}
55+
})
56+
57+
t.Run("lookup-resources", func(t *testing.T) {
58+
for _, tpl := range resources[:1] {
59+
stream := dispatch.NewCollectingDispatchStream[*v1.DispatchLookupResourcesResponse](ctx)
60+
err := dispatcher.DispatchLookupResources(&v1.DispatchLookupResourcesRequest{
61+
ObjectRelation: RR(tpl.ResourceAndRelation.Namespace, "view"),
62+
Subject: ONR(tpl.Subject.Namespace, tpl.Subject.ObjectId, graph.Ellipsis),
63+
Metadata: &v1.ResolverMeta{
64+
AtRevision: revision.String(),
65+
DepthRemaining: 50,
66+
},
67+
OptionalLimit: veryLargeLimit,
68+
}, stream)
69+
70+
require.NoError(t, err)
71+
72+
foundResources, _, _, _ := processResults(stream)
73+
require.Len(t, foundResources, 1)
74+
}
75+
})
76+
77+
t.Run("lookup-subjects", func(t *testing.T) {
78+
for _, tpl := range resources[:1] {
79+
stream := dispatch.NewCollectingDispatchStream[*v1.DispatchLookupSubjectsResponse](ctx)
80+
81+
err := dispatcher.DispatchLookupSubjects(&v1.DispatchLookupSubjectsRequest{
82+
ResourceRelation: RR(tpl.ResourceAndRelation.Namespace, "view"),
83+
ResourceIds: []string{tpl.ResourceAndRelation.ObjectId},
84+
SubjectRelation: RR(tpl.Subject.Namespace, graph.Ellipsis),
85+
Metadata: &v1.ResolverMeta{
86+
AtRevision: revision.String(),
87+
DepthRemaining: 50,
88+
},
89+
}, stream)
90+
91+
require.NoError(t, err)
92+
res := stream.Results()
93+
require.Len(t, res, 1)
94+
require.Len(t, res[0].FoundSubjectsByResourceId, 1)
95+
require.NotNil(t, res[0].FoundSubjectsByResourceId["res1"])
96+
require.Len(t, res[0].FoundSubjectsByResourceId["res1"].FoundSubjects, math.MaxUint16+1)
97+
}
98+
})
99+
}

‎pkg/genutil/slicez/chunking_test.go‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,21 @@ import (
99
)
1010

1111
func TestForEachChunk(t *testing.T) {
12+
t.Parallel()
13+
1214
for _, datasize := range []int{0, 1, 5, 10, 50, 100, 250} {
1315
datasize := datasize
1416
for _, chunksize := range []uint16{1, 2, 3, 5, 10, 50} {
1517
chunksize := chunksize
1618
t.Run(fmt.Sprintf("test-%d-%d", datasize, chunksize), func(t *testing.T) {
17-
data := []int{}
19+
t.Parallel()
20+
21+
data := make([]int, 0, datasize)
1822
for i := 0; i < datasize; i++ {
1923
data = append(data, i)
2024
}
2125

22-
found := []int{}
26+
found := make([]int, 0, datasize)
2327
ForEachChunk(data, chunksize, func(items []int) {
2428
found = append(found, items...)
2529
require.True(t, len(items) <= int(chunksize))
@@ -32,6 +36,8 @@ func TestForEachChunk(t *testing.T) {
3236
}
3337

3438
func TestForEachChunkOverflowPanic(t *testing.T) {
39+
t.Parallel()
40+
3541
datasize := math.MaxUint16
3642
chunksize := uint16(50)
3743
data := make([]int, 0, datasize)
@@ -50,19 +56,27 @@ func TestForEachChunkOverflowPanic(t *testing.T) {
5056
}
5157

5258
func TestForEachChunkOverflowIncorrect(t *testing.T) {
59+
t.Parallel()
60+
5361
chunksize := uint16(50)
54-
datasize := math.MaxUint16 + int(chunksize)
55-
data := make([]int, 0, datasize)
56-
for i := 0; i < datasize; i++ {
57-
data = append(data, i)
58-
}
62+
for _, datasize := range []int{math.MaxUint16 + int(chunksize), 10_000_000} {
63+
datasize := datasize
64+
t.Run(fmt.Sprintf("test-%d-%d", datasize, chunksize), func(t *testing.T) {
65+
t.Parallel()
5966

60-
found := make([]int, 0, datasize)
61-
ForEachChunk(data, chunksize, func(items []int) {
62-
found = append(found, items...)
63-
require.True(t, len(items) <= int(chunksize))
64-
require.True(t, len(items) > 0)
65-
})
67+
data := make([]int, 0, datasize)
68+
for i := 0; i < datasize; i++ {
69+
data = append(data, i)
70+
}
6671

67-
require.Equal(t, data, found)
72+
found := make([]int, 0, datasize)
73+
ForEachChunk(data, chunksize, func(items []int) {
74+
found = append(found, items...)
75+
require.True(t, len(items) <= int(chunksize))
76+
require.True(t, len(items) > 0)
77+
})
78+
79+
require.Equal(t, data, found)
80+
})
81+
}
6882
}

0 commit comments

Comments
 (0)