artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 20:51:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

permissions-center: add pagination support for permission sync jobs store. (#47350)

Browse Source 
This commit also removes an error in `QueryArgs` when there is only `OrderBy` argument provided without `First` or `Last`.

Test plan:
Tests added.
This commit is contained in:
Alex Ostrikov 2023-02-03 10:26:37 +04:00 committed by GitHub
parent 7efb0d9727
commit 8604bfba21
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 116 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
enterprise/internal/insights/scheduler/backfill.go
View File

@ -282,10 +282,7 @@ func (s *BackfillStore) GetBackfillQueueInfo(ctx context.Context, args BackfillQ
if args.PaginationArgs != nil {
pagination = *args.PaginationArgs
}
p, err := pagination.SQL()
if err != nil {
return nil, err
}
p := pagination.SQL()
// Add in pagination where clause
if p.Where != nil {
where = append(where, p.Where)

5
internal/database/conf.go
View File

@ -158,10 +158,7 @@ func (s *confStore) ListSiteConfigs(ctx context.Context, paginationArgs *Paginat
return scanSiteConfigs(rows, err)
}
args, err := paginationArgs.SQL()
if err != nil {
return []*SiteConfig{}, nil
}
args := paginationArgs.SQL()
if args.Where != nil {
where = append(where, args.Where)

10
internal/database/helpers.go
View File

@ -8,8 +8,6 @@ import (
"github.com/graph-gophers/graphql-go"
"github.com/graph-gophers/graphql-go/relay"
"github.com/keegancsmith/sqlf"
"github.com/sourcegraph/sourcegraph/lib/errors"
)
// LimitOffset specifies SQL LIMIT and OFFSET counts. A pointer to it is typically embedded in other options
@ -130,12 +128,12 @@ type PaginationArgs struct {
After *string
Before *string
// TODDO(naman): explain default
// TODO(naman): explain default
OrderBy OrderBy
Ascending bool
}
func (p *PaginationArgs) SQL() (*QueryArgs, error) {
func (p *PaginationArgs) SQL() *QueryArgs {
queryArgs := &QueryArgs{}
var conditions []*sqlf.Query
@ -177,10 +175,10 @@ func (p *PaginationArgs) SQL() (*QueryArgs, error) {
queryArgs.Order = orderBy.SQL(!p.Ascending)
queryArgs.Limit = sqlf.Sprintf("LIMIT %d", *p.Last)
} else {
return nil, errors.New("First or Last must be set")
queryArgs.Order = orderBy.SQL(p.Ascending)
}
return queryArgs, nil
return queryArgs
}
func copyPtr[T any](n *T) *T {

28
internal/database/permission_sync_jobs.go
View File

@ -290,6 +290,9 @@ type ListPermissionSyncJobOpts struct {
NullProcessAfter bool
NotNullProcessAfter bool
NotCanceled bool
// Cursor-based pagination arguments.
PaginationArgs *PaginationArgs
}
func (opts ListPermissionSyncJobOpts) sqlConds() []*sqlf.Query {
@ -322,9 +325,25 @@ func (opts ListPermissionSyncJobOpts) sqlConds() []*sqlf.Query {
return conds
}
const listPermissionSyncJobQueryFmtstr = `
SELECT %s
FROM permission_sync_jobs
%s -- whereClause
`
func (s *permissionSyncJobStore) List(ctx context.Context, opts ListPermissionSyncJobOpts) ([]*PermissionSyncJob, error) {
conds := opts.sqlConds()
paginationArgs := PaginationArgs{OrderBy: []OrderByOption{{Field: "id"}}, Ascending: true}
if opts.PaginationArgs != nil {
paginationArgs = *opts.PaginationArgs
}
pagination := paginationArgs.SQL()
if pagination.Where != nil {
conds = append(conds, pagination.Where)
}
var whereClause *sqlf.Query
if len(conds) != 0 {
whereClause = sqlf.Sprintf("WHERE %s", sqlf.Join(conds, "\n AND "))
@ -337,6 +356,8 @@ func (s *permissionSyncJobStore) List(ctx context.Context, opts ListPermissionSy
sqlf.Join(PermissionSyncJobColumns, ", "),
whereClause,
)
q = pagination.AppendOrderToQuery(q)
q = pagination.AppendLimitToQuery(q)
rows, err := s.Query(ctx, q)
if err != nil {
@ -356,13 +377,6 @@ func (s *permissionSyncJobStore) List(ctx context.Context, opts ListPermissionSy
return syncJobs, nil
}
const listPermissionSyncJobQueryFmtstr = `
SELECT %s
FROM permission_sync_jobs
%s -- whereClause
ORDER BY id ASC
`
type PermissionSyncJob struct {
ID int
State string

89
internal/database/permission_sync_jobs_test.go
View File

@ -33,13 +33,13 @@ func TestPermissionSyncJobs_CreateAndList(t *testing.T) {
usersStore := UsersWith(logger, db)
reposStore := ReposWith(logger, db)
// create users
// Create users.
user1, err := usersStore.Create(ctx, NewUser{Username: "test-user-1"})
require.NoError(t, err)
user2, err := usersStore.Create(ctx, NewUser{Username: "test-user-2"})
require.NoError(t, err)
// create repo
// Create a repo.
repo1 := types.Repo{Name: "test-repo-1", ID: 101}
err = reposStore.Create(ctx, &repo1)
require.NoError(t, err)
@ -316,7 +316,7 @@ func TestPermissionSyncJobs_CancelQueuedJob(t *testing.T) {
store := PermissionSyncJobsWith(logger, db)
reposStore := ReposWith(logger, db)
// create repo
// Create a repo.
repo1 := types.Repo{Name: "test-repo-1", ID: 101}
err := reposStore.Create(ctx, &repo1)
require.NoError(t, err)
@ -365,7 +365,7 @@ func TestPermissionSyncJobs_CascadeOnRepoDelete(t *testing.T) {
store := PermissionSyncJobsWith(logger, db)
reposStore := ReposWith(logger, db)
// Create repo.
// Create a repo.
repo1 := types.Repo{Name: "test-repo-1", ID: 101}
err := reposStore.Create(ctx, &repo1)
require.NoError(t, err)
@ -423,3 +423,84 @@ func TestPermissionSyncJobs_CascadeOnUserDelete(t *testing.T) {
require.NoError(t, err)
require.Empty(t, jobs)
}
func TestPermissionSyncJobs_Pagination(t *testing.T) {
if testing.Short() {
t.Skip()
}
ctx := context.Background()
logger := logtest.Scoped(t)
db := NewDB(logger, dbtest.NewDB(logger, t))
user, err := db.Users().Create(ctx, NewUser{Username: "horse"})
require.NoError(t, err)
store := PermissionSyncJobsWith(logger, db)
// Create 10 sync jobs.
createSyncJobs(t, ctx, user.ID, store)
jobs, err := store.List(ctx, ListPermissionSyncJobOpts{})
require.NoError(t, err)
paginationTests := []struct {
name string
paginationArgs PaginationArgs
wantJobs []*PermissionSyncJob
}{
{
name: "After",
paginationArgs: PaginationArgs{OrderBy: []OrderByOption{{Field: "user_id"}}, Ascending: true, After: strptr("1")},
wantJobs: []*PermissionSyncJob{},
},
{
name: "Before",
paginationArgs: PaginationArgs{OrderBy: []OrderByOption{{Field: "user_id"}}, Ascending: true, Before: strptr("2")},
wantJobs: jobs,
},
{
name: "First",
paginationArgs: PaginationArgs{Ascending: true, First: intPtr(5)},
wantJobs: jobs[:5],
},
{
name: "OrderBy",
paginationArgs: PaginationArgs{OrderBy: []OrderByOption{{Field: "queued_at"}}, Ascending: false},
wantJobs: reverse(jobs),
},
}
for _, tt := range paginationTests {
t.Run(tt.name, func(t *testing.T) {
have, err := store.List(ctx, ListPermissionSyncJobOpts{PaginationArgs: &tt.paginationArgs})
require.NoError(t, err)
if len(have) != len(tt.wantJobs) {
t.Fatalf("wrong number of jobs returned. want=%d, have=%d", len(tt.wantJobs), len(have))
}
if len(tt.wantJobs) > 0 {
if diff := cmp.Diff(tt.wantJobs, have); diff != "" {
t.Fatalf("unexpected jobs. diff: %s", diff)
}
}
})
}
}
func createSyncJobs(t *testing.T, ctx context.Context, userID int32, store PermissionSyncJobStore) {
t.Helper()
clock := timeutil.NewFakeClock(time.Now(), 0)
for i := 0; i < 10; i++ {
processAfter := clock.Now().Add(5 * time.Minute)
opts := PermissionSyncJobOpts{Priority: MediumPriorityPermissionSync, InvalidateCaches: true, ProcessAfter: processAfter, Reason: ReasonManualUserSync}
err := store.CreateUserSyncJob(ctx, userID, opts)
require.NoError(t, err)
}
}
func reverse(jobs []*PermissionSyncJob) []*PermissionSyncJob {
reversed := make([]*PermissionSyncJob, 0, len(jobs))
for i := 0; i < len(jobs); i++ {
reversed = append(reversed, jobs[len(jobs)-i-1])
}
return reversed
}

5
internal/database/repos.go
View File

@ -939,10 +939,7 @@ func (s *repoStore) listSQL(ctx context.Context, tr *trace.Trace, opt ReposListO
querySuffix := sqlf.Sprintf("%s %s", opt.OrderBy.SQL(), opt.LimitOffset.SQL())
if opt.PaginationArgs != nil {
p, err := opt.PaginationArgs.SQL()
if err != nil {
return nil, err
}
p := opt.PaginationArgs.SQL()
if p.Where != nil {
where = append(where, p.Where)

7
internal/database/saved_searches.go
View File

@ -247,14 +247,11 @@ func (s *savedSearchStore) ListSavedSearchesByOrgID(ctx context.Context, orgID i
// organization for the user.
//
// 🚨 SECURITY: This method does NOT verify the user's identity or that the
// user is an admin. It is the callers responsibility to ensure only admins or
// user is an admin. It is the caller's responsibility to ensure only admins or
// members of the specified organization can access the returned saved
// searches.
func (s *savedSearchStore) ListSavedSearchesByOrgOrUser(ctx context.Context, userID, orgID *int32, paginationArgs *PaginationArgs) ([]*types.SavedSearch, error) {
p, err := paginationArgs.SQL()
if err != nil {
return nil, err
}
p := paginationArgs.SQL()
var where []*sqlf.Query

5
internal/database/users.go
View File

@ -986,10 +986,7 @@ func (u *userStore) ListByOrg(ctx context.Context, orgID int32, paginationArgs *
where = append(where, cond)
}
p, err := paginationArgs.SQL()
if err != nil {
return nil, err
}
p := paginationArgs.SQL()
if p.Where != nil {
where = append(where, p.Where)