mirror of
https://github.com/sourcegraph/sourcegraph.git
synced 2026-02-06 17:31:43 +00:00
add optional flag to skip pagination arg validation (#48101)
There are some resources we want to optionally pagination, for example, RBAC permissions. When a user loads the Sourcegraph web app, we need to fetch all permissions assigned to that user to figure out access control. This PR adds a field to `ConnectionResolverOptions` called `SkipArgsValidation`, which skips the check to ensure `First` or `Last` is set when accessing a paginated resource. The field is false by default to enforce pagination for new and existing resources and I've added a comment so anyone making use of it understands the performance implications of setting it to `true`. ## Test plan <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles --> * Manually tested * Added unit tests
This commit is contained in:
parent
40c53bf01b
commit
544f0f1986
@ -64,6 +64,11 @@ type ConnectionResolverOptions struct {
|
||||
OrderBy database.OrderBy
|
||||
// Order direction.
|
||||
Ascending bool
|
||||
|
||||
// If set to true, the resolver won't throw an error when `first` or `last` isn't provided
|
||||
// in `ConnectionResolverArgs`. Be careful when setting this to true, as this could cause
|
||||
// performance issues when fetching large data.
|
||||
AllowNoLimit bool
|
||||
}
|
||||
|
||||
// MaxPageSizeOrDefault returns the configured max page limit for the connection.
|
||||
@ -118,7 +123,7 @@ func (r *ConnectionResolver[N]) paginationArgs() (*database.PaginationArgs, erro
|
||||
paginationArgs.First = &limit
|
||||
} else if r.args.Last != nil {
|
||||
paginationArgs.Last = &limit
|
||||
} else {
|
||||
} else if !r.options.AllowNoLimit {
|
||||
return nil, errors.New("you must provide a `first` or `last` value to properly paginate")
|
||||
}
|
||||
|
||||
|
||||
@ -144,12 +144,19 @@ func TestConnectionTotalCount(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func testResolverNodesResponse(t *testing.T, resolver *ConnectionResolver[*testConnectionNode], store *testConnectionStore, count int) {
|
||||
func testResolverNodesResponse(t *testing.T, resolver *ConnectionResolver[*testConnectionNode], store *testConnectionStore, count int, wantErr bool) {
|
||||
ctx := context.Background()
|
||||
nodes, err := resolver.Nodes(ctx)
|
||||
if err != nil {
|
||||
if wantErr {
|
||||
if err == nil {
|
||||
t.Fatalf("expected error, got %v", err)
|
||||
}
|
||||
return
|
||||
}
|
||||
if err != nil && !wantErr {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if diff := cmp.Diff(count, len(nodes)); diff != "" {
|
||||
t.Fatal(diff)
|
||||
}
|
||||
@ -172,7 +179,9 @@ func TestConnectionNodes(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
connectionArgs *ConnectionResolverArgs
|
||||
options *ConnectionResolverOptions
|
||||
|
||||
wantError bool
|
||||
wantPaginationArgs *database.PaginationArgs
|
||||
wantNodes int
|
||||
}{
|
||||
@ -206,15 +215,28 @@ func TestConnectionNodes(t *testing.T) {
|
||||
connectionArgs: withBeforeCA("0", withLastCA(1, &ConnectionResolverArgs{})),
|
||||
wantNodes: 1,
|
||||
},
|
||||
{
|
||||
name: "no args supplied (skipArgValidation is false)",
|
||||
connectionArgs: &ConnectionResolverArgs{},
|
||||
options: &ConnectionResolverOptions{AllowNoLimit: false},
|
||||
wantError: true,
|
||||
},
|
||||
{
|
||||
name: "no args supplied (skipArgValidation is true)",
|
||||
connectionArgs: &ConnectionResolverArgs{},
|
||||
options: &ConnectionResolverOptions{AllowNoLimit: true},
|
||||
wantError: false,
|
||||
wantNodes: 2,
|
||||
},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
store := &testConnectionStore{t: t, expectedPaginationArgs: test.wantPaginationArgs}
|
||||
resolver, err := NewConnectionResolver[*testConnectionNode](store, test.connectionArgs, nil)
|
||||
resolver, err := NewConnectionResolver[*testConnectionNode](store, test.connectionArgs, test.options)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
testResolverNodesResponse(t, resolver, store, test.wantNodes)
|
||||
testResolverNodesResponse(t, resolver, store, test.wantNodes, test.wantError)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user