From 2d3dc436a81d2fe6fa8af7d4958b7a1f51f56caa Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 10 May 2022 10:44:09 +0300 Subject: [PATCH] feat: Implement unified pagination and add template versions support (#1308) * feat: Implement pagination for template versions * feat: Use unified pagination between users and template versions * Sync codepaths between users and template versions * Create requestOption type in codersdk and add test * Fix created_at edge case for pagination cursor in queries * feat: Add support for json omitempty and embedded structs in apitypings (#1318) * Add scripts/apitypings/main.go to Makefile --- Makefile | 2 +- coderd/database/databasefake/databasefake.go | 79 +++++++++++---- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 73 ++++++++++---- coderd/database/queries/templateversions.sql | 28 +++++- coderd/database/queries/users.sql | 29 +++--- coderd/pagination.go | 57 +++++++++++ coderd/templates.go | 17 +++- coderd/templates_test.go | 100 ++++++++++++++++++- coderd/users.go | 48 ++------- coderd/users_test.go | 24 +++-- codersdk/client.go | 4 +- codersdk/pagination.go | 45 +++++++++ codersdk/pagination_test.go | 60 +++++++++++ codersdk/templates.go | 11 +- codersdk/users.go | 36 ++----- scripts/apitypings/main.go | 31 +++++- site/src/api/typesGenerated.ts | 61 ++++++----- 18 files changed, 540 insertions(+), 167 deletions(-) create mode 100644 coderd/pagination.go create mode 100644 codersdk/pagination.go create mode 100644 codersdk/pagination_test.go diff --git a/Makefile b/Makefile index ffc08fe329..9ea8aa90a1 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 850f5605a4..f1ec50bbbb 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -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 @@ -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 } } @@ -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:] } @@ -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) { @@ -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 } diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 216db6dcd9..4182d15284 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -47,7 +47,7 @@ type querier interface { GetTemplateVersionByID(ctx context.Context, id uuid.UUID) (TemplateVersion, error) GetTemplateVersionByJobID(ctx context.Context, jobID uuid.UUID) (TemplateVersion, error) GetTemplateVersionByTemplateIDAndName(ctx context.Context, arg GetTemplateVersionByTemplateIDAndNameParams) (TemplateVersion, error) - GetTemplateVersionsByTemplateID(ctx context.Context, dollar_1 uuid.UUID) ([]TemplateVersion, error) + GetTemplateVersionsByTemplateID(ctx context.Context, arg GetTemplateVersionsByTemplateIDParams) ([]TemplateVersion, error) GetTemplatesByIDs(ctx context.Context, ids []uuid.UUID) ([]Template, error) GetTemplatesByOrganization(ctx context.Context, arg GetTemplatesByOrganizationParams) ([]Template, error) GetUserByEmailOrUsername(ctx context.Context, arg GetUserByEmailOrUsernameParams) (User, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 8392ea2783..362bae614c 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1908,10 +1908,48 @@ FROM template_versions WHERE template_id = $1 :: 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 $2 :: 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 = $2 + ) + ) + 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 $3 +LIMIT + -- A null limit means "no limit", so -1 means return all + NULLIF($4 :: int, -1) ` -func (q *sqlQuerier) GetTemplateVersionsByTemplateID(ctx context.Context, dollar_1 uuid.UUID) ([]TemplateVersion, error) { - rows, err := q.db.QueryContext(ctx, getTemplateVersionsByTemplateID, dollar_1) +type GetTemplateVersionsByTemplateIDParams struct { + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + AfterID uuid.UUID `db:"after_id" json:"after_id"` + OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` + LimitOpt int32 `db:"limit_opt" json:"limit_opt"` +} + +func (q *sqlQuerier) GetTemplateVersionsByTemplateID(ctx context.Context, arg GetTemplateVersionsByTemplateIDParams) ([]TemplateVersion, error) { + rows, err := q.db.QueryContext(ctx, getTemplateVersionsByTemplateID, + arg.TemplateID, + arg.AfterID, + arg.OffsetOpt, + arg.LimitOpt, + ) if err != nil { return nil, err } @@ -2125,22 +2163,19 @@ WHERE -- This is an important option for scripts that need to paginate without -- duplicating or missing data. WHEN $1 :: 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 = $1 - ) - -- Omit the cursor from the final. - AND id != $1 + -- 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 = $1 ) - ELSE true + ) + ELSE true END -- Start filters -- Filter by name, email or username @@ -2171,7 +2206,7 @@ LIMIT ` type GetUsersParams struct { - AfterUser uuid.UUID `db:"after_user" json:"after_user"` + AfterID uuid.UUID `db:"after_id" json:"after_id"` Search string `db:"search" json:"search"` Status string `db:"status" json:"status"` OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` @@ -2180,7 +2215,7 @@ type GetUsersParams struct { func (q *sqlQuerier) GetUsers(ctx context.Context, arg GetUsersParams) ([]User, error) { rows, err := q.db.QueryContext(ctx, getUsers, - arg.AfterUser, + arg.AfterID, arg.Search, arg.Status, arg.OffsetOpt, diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index dccdc0a54b..b6a3550d5f 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -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 diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index d0c3d4c3e7..c3d72b3637 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -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 diff --git a/coderd/pagination.go b/coderd/pagination.go new file mode 100644 index 0000000000..2fbc2a0df6 --- /dev/null +++ b/coderd/pagination.go @@ -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 +} diff --git a/coderd/templates.go b/coderd/templates.go index fe087c9f1b..153c8aba9c 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -75,9 +75,21 @@ func (api *api) deleteTemplate(rw http.ResponseWriter, r *http.Request) { func (api *api) templateVersionsByTemplate(rw http.ResponseWriter, r *http.Request) { template := httpmw.TemplateParam(r) - versions, err := api.Database.GetTemplateVersionsByTemplateID(r.Context(), template.ID) + paginationParams, ok := parsePagination(rw, r) + if !ok { + return + } + + apiVersion := []codersdk.TemplateVersion{} + versions, err := api.Database.GetTemplateVersionsByTemplateID(r.Context(), database.GetTemplateVersionsByTemplateIDParams{ + TemplateID: template.ID, + AfterID: paginationParams.AfterID, + LimitOpt: int32(paginationParams.Limit), + OffsetOpt: int32(paginationParams.Offset), + }) if errors.Is(err, sql.ErrNoRows) { - err = nil + httpapi.Write(rw, http.StatusOK, apiVersion) + return } if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -101,7 +113,6 @@ func (api *api) templateVersionsByTemplate(rw http.ResponseWriter, r *http.Reque jobByID[job.ID.String()] = job } - apiVersion := make([]codersdk.TemplateVersion, 0) for _, version := range versions { job, exists := jobByID[version.JobID.String()] if !exists { diff --git a/coderd/templates_test.go b/coderd/templates_test.go index d7b0e67a65..f0082408f3 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -6,10 +6,13 @@ import ( "testing" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" + "github.com/coder/coder/provisioner/echo" ) func TestTemplate(t *testing.T) { @@ -63,7 +66,9 @@ func TestTemplateVersionsByTemplate(t *testing.T) { user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - versions, err := client.TemplateVersionsByTemplate(context.Background(), template.ID) + versions, err := client.TemplateVersionsByTemplate(context.Background(), codersdk.TemplateVersionsByTemplateRequest{ + TemplateID: template.ID, + }) require.NoError(t, err) require.Len(t, versions, 1) }) @@ -137,3 +142,96 @@ func TestPatchActiveTemplateVersion(t *testing.T) { require.NoError(t, err) }) } + +// TestPaginatedTemplateVersions creates a list of template versions and paginate. +func TestPaginatedTemplateVersions(t *testing.T) { + t.Parallel() + ctx := context.Background() + + client := coderdtest.New(t, &coderdtest.Options{APIRateLimit: -1}) + // Prepare database. + user := coderdtest.CreateFirstUser(t, client) + coderdtest.NewProvisionerDaemon(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + + // Populate database with template versions. + total := 9 + for i := 0; i < total; i++ { + data, err := echo.Tar(nil) + require.NoError(t, err) + file, err := client.Upload(context.Background(), codersdk.ContentTypeTar, data) + require.NoError(t, err) + templateVersion, err := client.CreateTemplateVersion(ctx, user.OrganizationID, codersdk.CreateTemplateVersionRequest{ + TemplateID: template.ID, + StorageSource: file.Hash, + StorageMethod: database.ProvisionerStorageMethodFile, + Provisioner: database.ProvisionerTypeEcho, + }) + require.NoError(t, err) + + _ = coderdtest.AwaitTemplateVersionJob(t, client, templateVersion.ID) + } + + templateVersions, err := client.TemplateVersionsByTemplate(ctx, + codersdk.TemplateVersionsByTemplateRequest{ + TemplateID: template.ID, + }, + ) + require.NoError(t, err) + require.Len(t, templateVersions, 10, "wrong number of template versions created") + + type args struct { + ctx context.Context + pagination codersdk.Pagination + } + tests := []struct { + name string + args args + want []codersdk.TemplateVersion + }{ + { + name: "Single result", + args: args{ctx: ctx, pagination: codersdk.Pagination{Limit: 1}}, + want: templateVersions[:1], + }, + { + name: "Single result, second page", + args: args{ctx: ctx, pagination: codersdk.Pagination{Limit: 1, Offset: 1}}, + want: templateVersions[1:2], + }, + { + name: "Last two results", + args: args{ctx: ctx, pagination: codersdk.Pagination{Limit: 2, Offset: 8}}, + want: templateVersions[8:10], + }, + { + name: "AfterID returns next two results", + args: args{ctx: ctx, pagination: codersdk.Pagination{Limit: 2, AfterID: templateVersions[1].ID}}, + want: templateVersions[2:4], + }, + { + name: "No result after last AfterID", + args: args{ctx: ctx, pagination: codersdk.Pagination{Limit: 2, AfterID: templateVersions[9].ID}}, + want: []codersdk.TemplateVersion{}, + }, + { + name: "No result after last Offset", + args: args{ctx: ctx, pagination: codersdk.Pagination{Limit: 2, Offset: 10}}, + want: []codersdk.TemplateVersion{}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := client.TemplateVersionsByTemplate(tt.args.ctx, codersdk.TemplateVersionsByTemplateRequest{ + TemplateID: template.ID, + Pagination: tt.args.pagination, + }) + assert.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/coderd/users.go b/coderd/users.go index 2d984c8ff9..c85a23f3f7 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "net/http" - "strconv" "time" "github.com/go-chi/chi/v5" @@ -106,55 +105,26 @@ func (api *api) postFirstUser(rw http.ResponseWriter, r *http.Request) { func (api *api) users(rw http.ResponseWriter, r *http.Request) { var ( - afterArg = r.URL.Query().Get("after_user") - limitArg = r.URL.Query().Get("limit") - offsetArg = r.URL.Query().Get("offset") searchName = r.URL.Query().Get("search") statusFilter = r.URL.Query().Get("status") ) - // createdAfter is a user uuid. - createdAfter := uuid.Nil - if afterArg != "" { - after, err := uuid.Parse(afterArg) - if err != nil { - httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: fmt.Sprintf("after_user must be a valid uuid: %s", err.Error()), - }) - return - } - createdAfter = after - } - - // Default to no limit and return all users. - pageLimit := -1 - if limitArg != "" { - limit, err := strconv.Atoi(limitArg) - if err != nil { - httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: fmt.Sprintf("limit must be an integer: %s", err.Error()), - }) - return - } - pageLimit = limit - } - - // The default for empty string is 0. - offset, err := strconv.ParseInt(offsetArg, 10, 64) - if offsetArg != "" && err != nil { - httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: fmt.Sprintf("offset must be an integer: %s", err.Error()), - }) + paginationParams, ok := parsePagination(rw, r) + if !ok { return } users, err := api.Database.GetUsers(r.Context(), database.GetUsersParams{ - AfterUser: createdAfter, - OffsetOpt: int32(offset), - LimitOpt: int32(pageLimit), + AfterID: paginationParams.AfterID, + OffsetOpt: int32(paginationParams.Offset), + LimitOpt: int32(paginationParams.Limit), Search: searchName, Status: statusFilter, }) + if errors.Is(err, sql.ErrNoRows) { + httpapi.Write(rw, http.StatusOK, []codersdk.User{}) + return + } if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: err.Error(), diff --git a/coderd/users_test.go b/coderd/users_test.go index f075e670c4..99d1849ca6 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -722,8 +722,6 @@ func TestPaginatedUsers(t *testing.T) { allUsers = append(allUsers, me) specialUsers := make([]codersdk.User, 0) - require.NoError(t, err) - // When 100 users exist total := 100 // Create users @@ -795,7 +793,9 @@ func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client // Check the first page page, err := client.Users(ctx, opt(codersdk.UsersRequest{ - Limit: limit, + Pagination: codersdk.Pagination{ + Limit: limit, + }, })) require.NoError(t, err, "first page") require.Equalf(t, page, allUsers[:limit], "first page, limit=%d", limit) @@ -811,15 +811,19 @@ func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client // This is using a cursor, and only works if all users created_at // is unique. page, err = client.Users(ctx, opt(codersdk.UsersRequest{ - Limit: limit, - AfterUser: afterCursor, + Pagination: codersdk.Pagination{ + Limit: limit, + AfterID: afterCursor, + }, })) require.NoError(t, err, "next cursor page") // Also check page by offset offsetPage, err := client.Users(ctx, opt(codersdk.UsersRequest{ - Limit: limit, - Offset: count, + Pagination: codersdk.Pagination{ + Limit: limit, + Offset: count, + }, })) require.NoError(t, err, "next offset page") @@ -834,8 +838,10 @@ func assertPagination(ctx context.Context, t *testing.T, client *codersdk.Client // Also check the before prevPage, err := client.Users(ctx, opt(codersdk.UsersRequest{ - Offset: count - limit, - Limit: limit, + Pagination: codersdk.Pagination{ + Offset: count - limit, + Limit: limit, + }, })) require.NoError(t, err, "prev page") require.Equal(t, allUsers[count-limit:count], prevPage, "prev users") diff --git a/codersdk/client.go b/codersdk/client.go index d13927513a..1654c141e6 100644 --- a/codersdk/client.go +++ b/codersdk/client.go @@ -33,9 +33,11 @@ type Client struct { URL *url.URL } +type requestOption func(*http.Request) + // request performs an HTTP request with the body provided. // The caller is responsible for closing the response body. -func (c *Client) request(ctx context.Context, method, path string, body interface{}, opts ...func(r *http.Request)) (*http.Response, error) { +func (c *Client) request(ctx context.Context, method, path string, body interface{}, opts ...requestOption) (*http.Response, error) { serverURL, err := c.URL.Parse(path) if err != nil { return nil, xerrors.Errorf("parse url: %w", err) diff --git a/codersdk/pagination.go b/codersdk/pagination.go new file mode 100644 index 0000000000..a4adee6b6e --- /dev/null +++ b/codersdk/pagination.go @@ -0,0 +1,45 @@ +package codersdk + +import ( + "net/http" + "strconv" + + "github.com/google/uuid" +) + +// Pagination sets pagination options for the endpoints that support it. +type Pagination struct { + // AfterID returns all or up to Limit results after the given + // UUID. This option can be used with or as an alternative to + // Offset for better performance. To use it as an alternative, + // set AfterID to the last UUID returned by the previous + // request. + AfterID uuid.UUID `json:"after_id,omitempty"` + // Limit sets the maximum number of users to be returned + // in a single page. If the limit is <= 0, there is no limit + // and all users are returned. + Limit int `json:"limit,omitempty"` + // Offset is used to indicate which page to return. An offset of 0 + // returns the first 'limit' number of users. + // To get the next page, use offset=*. + // Offset is 0 indexed, so the first record sits at offset 0. + Offset int `json:"offset,omitempty"` +} + +// asRequestOption returns a function that can be used in (*Client).request. +// It modifies the request query parameters. +func (p Pagination) asRequestOption() requestOption { + return func(r *http.Request) { + q := r.URL.Query() + if p.AfterID != uuid.Nil { + q.Set("after_id", p.AfterID.String()) + } + if p.Limit > 0 { + q.Set("limit", strconv.Itoa(p.Limit)) + } + if p.Offset > 0 { + q.Set("offset", strconv.Itoa(p.Offset)) + } + r.URL.RawQuery = q.Encode() + } +} diff --git a/codersdk/pagination_test.go b/codersdk/pagination_test.go new file mode 100644 index 0000000000..53a3fcaebc --- /dev/null +++ b/codersdk/pagination_test.go @@ -0,0 +1,60 @@ +//nolint:testpackage +package codersdk + +import ( + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" +) + +func TestPagination_asRequestOption(t *testing.T) { + t.Parallel() + + uuid1 := uuid.New() + type fields struct { + AfterID uuid.UUID + Limit int + Offset int + } + tests := []struct { + name string + fields fields + want url.Values + }{ + { + name: "Test AfterID is set", + fields: fields{AfterID: uuid1}, + want: url.Values{"after_id": []string{uuid1.String()}}, + }, + { + name: "Test Limit is set", + fields: fields{Limit: 10}, + want: url.Values{"limit": []string{"10"}}, + }, + { + name: "Test Offset is set", + fields: fields{Offset: 10}, + want: url.Values{"offset": []string{"10"}}, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + p := Pagination{ + AfterID: tt.fields.AfterID, + Limit: tt.fields.Limit, + Offset: tt.fields.Offset, + } + req := httptest.NewRequest(http.MethodGet, "/", nil) + p.asRequestOption()(req) + got := req.URL.Query() + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/codersdk/templates.go b/codersdk/templates.go index 0f8355a47f..d1f3361c85 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -69,9 +69,16 @@ func (c *Client) UpdateActiveTemplateVersion(ctx context.Context, template uuid. return nil } +// TemplateVersionsByTemplateRequest defines the request parameters for +// TemplateVersionsByTemplate. +type TemplateVersionsByTemplateRequest struct { + TemplateID uuid.UUID `json:"template_id" validate:"required"` + Pagination +} + // TemplateVersionsByTemplate lists versions associated with a template. -func (c *Client) TemplateVersionsByTemplate(ctx context.Context, template uuid.UUID) ([]TemplateVersion, error) { - res, err := c.request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/templates/%s/versions", template), nil) +func (c *Client) TemplateVersionsByTemplate(ctx context.Context, req TemplateVersionsByTemplateRequest) ([]TemplateVersion, error) { + res, err := c.request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/templates/%s/versions", req.TemplateID), nil, req.Pagination.asRequestOption()) if err != nil { return nil, err } diff --git a/codersdk/users.go b/codersdk/users.go index 1747a300cf..4388b0cfd4 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "net/http" - "strconv" "time" "github.com/google/uuid" @@ -22,19 +21,10 @@ const ( ) type UsersRequest struct { - AfterUser uuid.UUID `json:"after_user"` - Search string `json:"search"` - // Limit sets the maximum number of users to be returned - // in a single page. If the limit is <= 0, there is no limit - // and all users are returned. - Limit int `json:"limit"` - // Offset is used to indicate which page to return. An offset of 0 - // returns the first 'limit' number of users. - // To get the next page, use offset=*. - // Offset is 0 indexed, so the first record sits at offset 0. - Offset int `json:"offset"` + Search string `json:"search"` // Filter users by status Status string `json:"status"` + Pagination } // User represents a user in Coder. @@ -317,19 +307,15 @@ func (c *Client) userByIdentifier(ctx context.Context, ident string) (User, erro // Users returns all users according to the request parameters. If no parameters are set, // the default behavior is to return all users in a single page. func (c *Client) Users(ctx context.Context, req UsersRequest) ([]User, error) { - res, err := c.request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users"), nil, func(r *http.Request) { - q := r.URL.Query() - if req.AfterUser != uuid.Nil { - q.Set("after_user", req.AfterUser.String()) - } - if req.Limit > 0 { - q.Set("limit", strconv.Itoa(req.Limit)) - } - q.Set("offset", strconv.Itoa(req.Offset)) - q.Set("search", req.Search) - q.Set("status", req.Status) - r.URL.RawQuery = q.Encode() - }) + res, err := c.request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users"), nil, + req.Pagination.asRequestOption(), + func(r *http.Request) { + q := r.URL.Query() + q.Set("search", req.Search) + q.Set("status", req.Status) + r.URL.RawQuery = q.Encode() + }, + ) if err != nil { return []User{}, err } diff --git a/scripts/apitypings/main.go b/scripts/apitypings/main.go index 529b7ae0f4..0891439e72 100644 --- a/scripts/apitypings/main.go +++ b/scripts/apitypings/main.go @@ -237,10 +237,31 @@ func (g *Generator) posLine(obj types.Object) string { func (g *Generator) buildStruct(obj types.Object, st *types.Struct) (string, error) { var s strings.Builder _, _ = s.WriteString(g.posLine(obj)) + _, _ = s.WriteString(fmt.Sprintf("export interface %s ", obj.Name())) - _, _ = s.WriteString(fmt.Sprintf("export interface %s {\n", obj.Name())) + // Handle named embedded structs in the codersdk package via extension. + var extends []string + extendedFields := make(map[int]bool) + for i := 0; i < st.NumFields(); i++ { + field := st.Field(i) + tag := reflect.StructTag(st.Tag(i)) + // Adding a json struct tag causes the json package to consider + // the field unembedded. + if field.Embedded() && tag.Get("json") == "" && field.Pkg().Name() == "codersdk" { + extendedFields[i] = true + extends = append(extends, field.Name()) + } + } + if len(extends) > 0 { + _, _ = s.WriteString(fmt.Sprintf("extends %s ", strings.Join(extends, ", "))) + } + + _, _ = s.WriteString("{\n") // For each field in the struct, we print 1 line of the typescript interface for i := 0; i < st.NumFields(); i++ { + if extendedFields[i] { + continue + } field := st.Field(i) tag := reflect.StructTag(st.Tag(i)) @@ -251,6 +272,10 @@ func (g *Generator) buildStruct(obj types.Object, st *types.Struct) (string, err if jsonName == "" { jsonName = field.Name() } + jsonOptional := false + if len(arr) > 1 && arr[1] == "omitempty" { + jsonOptional = true + } var tsType TypescriptType // If a `typescript:"string"` exists, we take this, and do not try to infer. @@ -273,7 +298,7 @@ func (g *Generator) buildStruct(obj types.Object, st *types.Struct) (string, err _, _ = s.WriteRune('\n') } optional := "" - if tsType.Optional { + if jsonOptional || tsType.Optional { optional = "?" } _, _ = s.WriteString(fmt.Sprintf("%sreadonly %s%s: %s\n", indent, jsonName, optional, tsType.ValueType)) @@ -322,7 +347,7 @@ func (g *Generator) typescriptType(ty types.Type) (TypescriptType, error) { return TypescriptType{ ValueType: "any", AboveTypeLine: fmt.Sprintf("%s\n%s", - indentedComment("Embedded struct, please fix by naming it"), + indentedComment("Embedded anonymous struct, please fix by naming it"), indentedComment("eslint-disable-next-line @typescript-eslint/no-explicit-any"), ), }, nil diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 846211a109..e30c94e866 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -12,7 +12,7 @@ export interface AgentGitSSHKey { readonly private_key: string } -// From codersdk/users.go:110:6 +// From codersdk/users.go:100:6 export interface AuthMethods { readonly password: boolean readonly github: boolean @@ -30,7 +30,7 @@ export interface BuildInfoResponse { readonly version: string } -// From codersdk/users.go:51:6 +// From codersdk/users.go:41:6 export interface CreateFirstUserRequest { readonly email: string readonly username: string @@ -38,13 +38,13 @@ export interface CreateFirstUserRequest { readonly organization: string } -// From codersdk/users.go:59:6 +// From codersdk/users.go:49:6 export interface CreateFirstUserResponse { readonly user_id: string readonly organization_id: string } -// From codersdk/users.go:105:6 +// From codersdk/users.go:95:6 export interface CreateOrganizationRequest { readonly name: string } @@ -77,7 +77,7 @@ export interface CreateTemplateVersionRequest { readonly parameter_values: CreateParameterRequest[] } -// From codersdk/users.go:64:6 +// From codersdk/users.go:54:6 export interface CreateUserRequest { readonly email: string readonly username: string @@ -91,7 +91,7 @@ export interface CreateWorkspaceBuildRequest { // This is likely an enum in an external package ("github.com/coder/coder/coderd/database.WorkspaceTransition") readonly transition: string readonly dry_run: boolean - readonly state: string + readonly state?: string } // From codersdk/organizations.go:52:6 @@ -101,7 +101,7 @@ export interface CreateWorkspaceRequest { readonly parameter_values: CreateParameterRequest[] } -// From codersdk/users.go:101:6 +// From codersdk/users.go:91:6 export interface GenerateAPIKeyResponse { readonly key: string } @@ -119,13 +119,13 @@ export interface GoogleInstanceIdentityToken { readonly json_web_token: string } -// From codersdk/users.go:90:6 +// From codersdk/users.go:80:6 export interface LoginWithPasswordRequest { readonly email: string readonly password: string } -// From codersdk/users.go:96:6 +// From codersdk/users.go:86:6 export interface LoginWithPasswordResponse { readonly session_token: string } @@ -147,6 +147,13 @@ export interface OrganizationMember { readonly roles: string[] } +// From codersdk/pagination.go:11:6 +export interface Pagination { + readonly after_id?: string + readonly limit?: number + readonly offset?: number +} + // From codersdk/parameters.go:26:6 export interface Parameter { readonly id: string @@ -178,7 +185,7 @@ export interface ProvisionerJob { readonly created_at: string readonly started_at?: string readonly completed_at?: string - readonly error: string + readonly error?: string readonly status: ProvisionerJobStatus readonly worker_id?: string } @@ -195,7 +202,7 @@ export interface ProvisionerJobLog { readonly output: string } -// From codersdk/roles.go:13:6 +// From codersdk/roles.go:12:6 export interface Role { readonly name: string readonly display_name: string @@ -256,22 +263,27 @@ export interface TemplateVersionParameterSchema { readonly validation_value_type: string } +// From codersdk/templates.go:74:6 +export interface TemplateVersionsByTemplateRequest extends Pagination { + readonly template_id: string +} + // From codersdk/templates.go:28:6 export interface UpdateActiveTemplateVersion { readonly id: string } -// From codersdk/users.go:80:6 +// From codersdk/users.go:70:6 export interface UpdateRoles { readonly roles: string[] } -// From codersdk/users.go:76:6 +// From codersdk/users.go:66:6 export interface UpdateUserPasswordRequest { readonly password: string } -// From codersdk/users.go:71:6 +// From codersdk/users.go:61:6 export interface UpdateUserProfileRequest { readonly email: string readonly username: string @@ -292,7 +304,7 @@ export interface UploadResponse { readonly hash: string } -// From codersdk/users.go:41:6 +// From codersdk/users.go:31:6 export interface User { readonly id: string readonly email: string @@ -303,18 +315,15 @@ export interface User { readonly roles: Role[] } -// From codersdk/users.go:84:6 +// From codersdk/users.go:74:6 export interface UserRoles { readonly roles: string[] readonly organization_roles: Record } -// From codersdk/users.go:24:6 -export interface UsersRequest { - readonly after_user: string +// From codersdk/users.go:23:6 +export interface UsersRequest extends Pagination { readonly search: string - readonly limit: number - readonly offset: number readonly status: string } @@ -344,12 +353,12 @@ export interface WorkspaceAgent { readonly status: WorkspaceAgentStatus readonly name: string readonly resource_id: string - readonly instance_id: string + readonly instance_id?: string readonly architecture: string readonly environment_variables: Record readonly operating_system: string - readonly startup_script: string - readonly directory: string + readonly startup_script?: string + readonly directory?: string } // From codersdk/workspaceagents.go:47:6 @@ -404,7 +413,7 @@ export interface WorkspaceResource { readonly workspace_transition: string readonly type: string readonly name: string - readonly agents: WorkspaceAgent[] + readonly agents?: WorkspaceAgent[] } // From codersdk/parameters.go:16:6 @@ -413,7 +422,7 @@ export type ParameterScope = "organization" | "template" | "user" | "workspace" // From codersdk/provisionerdaemons.go:26:6 export type ProvisionerJobStatus = "canceled" | "canceling" | "failed" | "pending" | "running" | "succeeded" -// From codersdk/users.go:17:6 +// From codersdk/users.go:16:6 export type UserStatus = "active" | "suspended" // From codersdk/workspaceresources.go:15:6