Skip to content

feat: Implement unified pagination and add template versions support #1308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
May 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
475dcf9
feat: Implement pagination for template versions
mafredri May 5, 2022
01a52d1
Use codersdk.Pagination in users endpoint
mafredri May 5, 2022
d1478a9
Avoid user sort side-effect in databasefake
mafredri May 5, 2022
7a36610
Return sql.ErrNoRows for GetUsers in databasefake
mafredri May 5, 2022
61c60c6
Sync codepaths in databasefake
mafredri May 5, 2022
62fa273
Fix test with better handling of sql.ErrNoRows in coderd
mafredri May 5, 2022
878d633
Remove an unused require.NoError
mafredri May 5, 2022
5c840fd
Return empty list for sql.ErrNoRows in coderd users endpoint
mafredri May 5, 2022
07e590c
Add t.Parallel() to sub tests
mafredri May 5, 2022
be8ba6c
Reinit tt due to parallel test
mafredri May 5, 2022
5ab9ad5
Remove unused variable
mafredri May 5, 2022
e2a3741
Fix copy pasta in query comments
mafredri May 5, 2022
61410a5
Move ParsePagination from httpapi to coderd and unexport, return code…
mafredri May 5, 2022
8a67746
codersdk: Create requestOption type
mafredri May 5, 2022
55028d2
codersdk: Add test for Pagination.asRequestOption
mafredri May 5, 2022
ea2371e
coderd: Handle http response errors in parsePagination
mafredri May 5, 2022
66af4ec
codersdk: Use parallel test
mafredri May 5, 2022
067f912
Fix created_at edge case for pagination cursor in queries
mafredri May 5, 2022
b8be7a8
Fix copy paste issue
mafredri May 5, 2022
13fc406
Run make gen
mafredri May 10, 2022
aab400c
feat: Add support for json omitempty and embedded structs in apitypin…
mafredri May 10, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ site/out/index.html: $(shell find ./site -not -path './site/node_modules/*' -typ
# Restores GITKEEP files!
git checkout HEAD site/out

site/src/api/typesGenerated.ts: $(shell find codersdk -type f -name '*.go')
site/src/api/typesGenerated.ts: scripts/apitypings/main.go $(shell find codersdk -type f -name '*.go')
go run scripts/apitypings/main.go > site/src/api/typesGenerated.ts
cd site && yarn run format:types

Expand Down
79 changes: 59 additions & 20 deletions coderd/database/databasefake/databasefake.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,25 +172,25 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams
q.mutex.RLock()
defer q.mutex.RUnlock()

users := q.users
// Avoid side-effect of sorting.
users := make([]database.User, len(q.users))
copy(users, q.users)

// Database orders by created_at
sort.Slice(users, func(i, j int) bool {
if users[i].CreatedAt.Equal(users[j].CreatedAt) {
slices.SortFunc(users, func(a, b database.User) bool {
if a.CreatedAt.Equal(b.CreatedAt) {
// Technically the postgres database also orders by uuid. So match
// that behavior
return users[i].ID.String() < users[j].ID.String()
return a.ID.String() < b.ID.String()
}
return users[i].CreatedAt.Before(users[j].CreatedAt)
return a.CreatedAt.Before(b.CreatedAt)
})

if params.AfterUser != uuid.Nil {
if params.AfterID != uuid.Nil {
found := false
for i := range users {
if users[i].ID == params.AfterUser {
for i, v := range users {
if v.ID == params.AfterID {
// We want to return all users after index i.
if i+1 >= len(users) {
return []database.User{}, nil
}
users = users[i+1:]
found = true
break
Expand All @@ -199,7 +199,7 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams

// If no users after the time, then we return an empty list.
if !found {
return []database.User{}, nil
return nil, sql.ErrNoRows
}
}

Expand Down Expand Up @@ -227,7 +227,7 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams

if params.OffsetOpt > 0 {
if int(params.OffsetOpt) > len(users)-1 {
return []database.User{}, nil
return nil, sql.ErrNoRows
}
users = users[params.OffsetOpt:]
}
Expand All @@ -239,10 +239,7 @@ func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams
users = users[:params.LimitOpt]
}

tmp := make([]database.User, len(users))
copy(tmp, users)

return tmp, nil
return users, nil
}

func (q *fakeQuerier) GetAllUserRoles(_ context.Context, userID uuid.UUID) (database.GetAllUserRolesRow, error) {
Expand Down Expand Up @@ -621,20 +618,62 @@ func (q *fakeQuerier) GetTemplateByOrganizationAndName(_ context.Context, arg da
return database.Template{}, sql.ErrNoRows
}

func (q *fakeQuerier) GetTemplateVersionsByTemplateID(_ context.Context, templateID uuid.UUID) ([]database.TemplateVersion, error) {
func (q *fakeQuerier) GetTemplateVersionsByTemplateID(_ context.Context, arg database.GetTemplateVersionsByTemplateIDParams) (version []database.TemplateVersion, err error) {
q.mutex.RLock()
defer q.mutex.RUnlock()

version := make([]database.TemplateVersion, 0)
for _, templateVersion := range q.templateVersions {
if templateVersion.TemplateID.UUID.String() != templateID.String() {
if templateVersion.TemplateID.UUID.String() != arg.TemplateID.String() {
continue
}
version = append(version, templateVersion)
}

// Database orders by created_at
slices.SortFunc(version, func(a, b database.TemplateVersion) bool {
if a.CreatedAt.Equal(b.CreatedAt) {
// Technically the postgres database also orders by uuid. So match
// that behavior
return a.ID.String() < b.ID.String()
}
return a.CreatedAt.Before(b.CreatedAt)
})

if arg.AfterID != uuid.Nil {
found := false
for i, v := range version {
if v.ID == arg.AfterID {
// We want to return all users after index i.
version = version[i+1:]
found = true
break
}
}

// If no users after the time, then we return an empty list.
if !found {
return nil, sql.ErrNoRows
}
}

if arg.OffsetOpt > 0 {
if int(arg.OffsetOpt) > len(version)-1 {
return nil, sql.ErrNoRows
}
version = version[arg.OffsetOpt:]
}

if arg.LimitOpt > 0 {
if int(arg.LimitOpt) > len(version) {
arg.LimitOpt = int32(len(version))
}
version = version[:arg.LimitOpt]
}

if len(version) == 0 {
return nil, sql.ErrNoRows
}

return version, nil
}

Expand Down
2 changes: 1 addition & 1 deletion coderd/database/querier.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

73 changes: 54 additions & 19 deletions coderd/database/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 27 additions & 1 deletion coderd/database/queries/templateversions.sql
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,33 @@ SELECT
FROM
template_versions
WHERE
template_id = $1 :: uuid;
template_id = @template_id :: uuid
AND CASE
-- This allows using the last element on a page as effectively a cursor.
-- This is an important option for scripts that need to paginate without
-- duplicating or missing data.
WHEN @after_id :: uuid != '00000000-00000000-00000000-00000000' THEN (
-- The pagination cursor is the last ID of the previous page.
-- The query is ordered by the created_at field, so select all
-- rows after the cursor.
(created_at, id) > (
SELECT
created_at, id
FROM
template_versions
WHERE
id = @after_id
)
)
ELSE true
END
ORDER BY
-- Deterministic and consistent ordering of all rows, even if they share
-- a timestamp. This is to ensure consistent pagination.
(created_at, id) ASC OFFSET @offset_opt
LIMIT
-- A null limit means "no limit", so -1 means return all
NULLIF(@limit_opt :: int, -1);

-- name: GetTemplateVersionByJobID :one
SELECT
Expand Down
29 changes: 13 additions & 16 deletions coderd/database/queries/users.sql
Original file line number Diff line number Diff line change
Expand Up @@ -77,23 +77,20 @@ WHERE
-- This allows using the last element on a page as effectively a cursor.
-- This is an important option for scripts that need to paginate without
-- duplicating or missing data.
WHEN @after_user :: uuid != '00000000-00000000-00000000-00000000' THEN (
-- The pagination cursor is the last user of the previous page.
-- The query is ordered by the created_at field, so select all
-- users after the cursor. We also want to include any users
-- that share the created_at (super rare).
created_at >= (
SELECT
created_at
FROM
users
WHERE
id = @after_user
)
-- Omit the cursor from the final.
AND id != @after_user
WHEN @after_id :: uuid != '00000000-00000000-00000000-00000000' THEN (
-- The pagination cursor is the last ID of the previous page.
-- The query is ordered by the created_at field, so select all
-- rows after the cursor.
(created_at, id) > (
SELECT
created_at, id
FROM
users
WHERE
id = @after_id
)
ELSE true
)
ELSE true
END
-- Start filters
-- Filter by name, email or username
Expand Down
57 changes: 57 additions & 0 deletions coderd/pagination.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package coderd

import (
"fmt"
"net/http"
"strconv"

"github.com/google/uuid"

"github.com/coder/coder/coderd/httpapi"
"github.com/coder/coder/codersdk"
)

// parsePagination extracts pagination query params from the http request.
// If an error is encountered, the error is written to w and ok is set to false.
func parsePagination(w http.ResponseWriter, r *http.Request) (p codersdk.Pagination, ok bool) {
var (
afterID = uuid.Nil
limit = -1 // Default to no limit and return all results.
offset = 0
)

var err error
if s := r.URL.Query().Get("after_id"); s != "" {
afterID, err = uuid.Parse(r.URL.Query().Get("after_id"))
if err != nil {
httpapi.Write(w, http.StatusBadRequest, httpapi.Response{
Message: fmt.Sprintf("after_id must be a valid uuid: %s", err.Error()),
})
return p, false
}
}
if s := r.URL.Query().Get("limit"); s != "" {
limit, err = strconv.Atoi(s)
if err != nil {
httpapi.Write(w, http.StatusBadRequest, httpapi.Response{
Message: fmt.Sprintf("limit must be an integer: %s", err.Error()),
})
return p, false
}
}
if s := r.URL.Query().Get("offset"); s != "" {
offset, err = strconv.Atoi(s)
if err != nil {
httpapi.Write(w, http.StatusBadRequest, httpapi.Response{
Message: fmt.Sprintf("offset must be an integer: %s", err.Error()),
})
return p, false
}
}

return codersdk.Pagination{
AfterID: afterID,
Limit: limit,
Offset: offset,
}, true
}
Loading