From 26428df65ae05b7590f14956e4ff53c122eaf8cf Mon Sep 17 00:00:00 2001 From: Garrett Date: Wed, 12 Oct 2022 20:57:21 +0000 Subject: [PATCH 1/9] feat: add pagination to getWorkspaces --- coderd/database/databasefake/databasefake.go | 13 +++++ coderd/database/modelqueries.go | 2 + coderd/database/queries.sql.go | 54 ++++++++++++-------- coderd/database/queries/workspaces.sql | 6 +++ coderd/workspaces.go | 26 ++++++---- coderd/workspaces_internal_test.go | 3 +- coderd/workspaces_test.go | 41 +++++++++++++++ codersdk/workspaces.go | 10 +++- 8 files changed, 121 insertions(+), 34 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 5a652164d3a6f..bdb4c8e0f0e40 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -695,6 +695,19 @@ func (q *fakeQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg database. workspaces = append(workspaces, workspace) } + if arg.Offset > 0 { + if int(arg.Offset) > len(workspaces) { + return []database.Workspace{}, nil + } + workspaces = workspaces[arg.Offset:] + } + if arg.Limit > 0 { + if int(arg.Limit) > len(workspaces) { + return workspaces, nil + } + workspaces = workspaces[:arg.Limit] + } + return workspaces, nil } diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index 9fc6e37176dc1..7f10019097c3e 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -167,6 +167,8 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa // The name comment is for metric tracking query := fmt.Sprintf("-- name: GetAuthorizedWorkspaces :many\n%s AND %s", getWorkspaces, authorizedFilter.SQLString(rbac.NoACLConfig())) rows, err := q.db.QueryContext(ctx, query, + arg.Limit, + arg.Offset, arg.Deleted, arg.Status, arg.OwnerID, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 16613ee7d5ef5..9316918e60837 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5457,60 +5457,60 @@ LEFT JOIN LATERAL ( ) latest_build ON TRUE WHERE -- Optionally include deleted workspaces - workspaces.deleted = $1 + workspaces.deleted = $3 AND CASE - WHEN $2 :: text != '' THEN + WHEN $4 :: text != '' THEN CASE - WHEN $2 = 'pending' THEN + WHEN $4 = 'pending' THEN latest_build.started_at IS NULL - WHEN $2 = 'starting' THEN + WHEN $4 = 'starting' THEN latest_build.started_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.completed_at IS NULL AND latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND latest_build.transition = 'start'::workspace_transition - WHEN $2 = 'running' THEN + WHEN $4 = 'running' THEN latest_build.completed_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.error IS NULL AND latest_build.transition = 'start'::workspace_transition - WHEN $2 = 'stopping' THEN + WHEN $4 = 'stopping' THEN latest_build.started_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.completed_at IS NULL AND latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND latest_build.transition = 'stop'::workspace_transition - WHEN $2 = 'stopped' THEN + WHEN $4 = 'stopped' THEN latest_build.completed_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.error IS NULL AND latest_build.transition = 'stop'::workspace_transition - WHEN $2 = 'failed' THEN + WHEN $4 = 'failed' THEN (latest_build.canceled_at IS NOT NULL AND latest_build.error IS NOT NULL) OR (latest_build.completed_at IS NOT NULL AND latest_build.error IS NOT NULL) - WHEN $2 = 'canceling' THEN + WHEN $4 = 'canceling' THEN latest_build.canceled_at IS NOT NULL AND latest_build.completed_at IS NULL - WHEN $2 = 'canceled' THEN + WHEN $4 = 'canceled' THEN latest_build.canceled_at IS NOT NULL AND latest_build.completed_at IS NOT NULL - WHEN $2 = 'deleted' THEN + WHEN $4 = 'deleted' THEN latest_build.started_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.completed_at IS NOT NULL AND latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND latest_build.transition = 'delete'::workspace_transition - WHEN $2 = 'deleting' THEN + WHEN $4 = 'deleting' THEN latest_build.completed_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.error IS NULL AND @@ -5523,39 +5523,47 @@ WHERE END -- Filter by owner_id AND CASE - WHEN $3 :: uuid != '00000000-00000000-00000000-00000000' THEN - owner_id = $3 + WHEN $5 :: uuid != '00000000-00000000-00000000-00000000' THEN + owner_id = $5 ELSE true END -- Filter by owner_name AND CASE - WHEN $4 :: text != '' THEN - owner_id = (SELECT id FROM users WHERE lower(username) = lower($4) AND deleted = false) + WHEN $6 :: text != '' THEN + owner_id = (SELECT id FROM users WHERE lower(username) = lower($6) AND deleted = false) ELSE true END -- Filter by template_name -- There can be more than 1 template with the same name across organizations. -- Use the organization filter to restrict to 1 org if needed. AND CASE - WHEN $5 :: text != '' THEN - template_id = ANY(SELECT id FROM templates WHERE lower(name) = lower($5) AND deleted = false) + WHEN $7 :: text != '' THEN + template_id = ANY(SELECT id FROM templates WHERE lower(name) = lower($7) AND deleted = false) ELSE true END -- Filter by template_ids AND CASE - WHEN array_length($6 :: uuid[], 1) > 0 THEN - template_id = ANY($6) + WHEN array_length($8 :: uuid[], 1) > 0 THEN + template_id = ANY($8) ELSE true END -- Filter by name, matching on substring AND CASE - WHEN $7 :: text != '' THEN - name ILIKE '%' || $7 || '%' + WHEN $9 :: text != '' THEN + name ILIKE '%' || $9 || '%' ELSE true END +ORDER BY + last_used_at DESC +LIMIT + $1 +OFFSET + $2 ` type GetWorkspacesParams struct { + Limit int32 `db:"limit" json:"limit"` + Offset int32 `db:"offset" json:"offset"` Deleted bool `db:"deleted" json:"deleted"` Status string `db:"status" json:"status"` OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` @@ -5567,6 +5575,8 @@ type GetWorkspacesParams struct { func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) ([]Workspace, error) { rows, err := q.db.QueryContext(ctx, getWorkspaces, + arg.Limit, + arg.Offset, arg.Deleted, arg.Status, arg.OwnerID, diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 2cd6743b6825d..6552a647dacf1 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -132,6 +132,12 @@ WHERE name ILIKE '%' || @name || '%' ELSE true END +ORDER BY + last_used_at DESC +LIMIT + $1 +OFFSET + $2 ; -- name: GetWorkspaceByOwnerIDAndName :one diff --git a/coderd/workspaces.go b/coderd/workspaces.go index fda034dc6eb88..c96967b1266ed 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -98,8 +98,13 @@ func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() apiKey := httpmw.APIKey(r) + page, ok := parsePagination(rw, r) + if !ok { + return + } + queryStr := r.URL.Query().Get("q") - filter, errs := workspaceSearchQuery(queryStr) + filter, errs := workspaceSearchQuery(queryStr, page) if len(errs) > 0 { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid workspace search query.", @@ -1072,11 +1077,15 @@ func validWorkspaceSchedule(s *string, min time.Duration) (sql.NullString, error // workspaceSearchQuery takes a query string and returns the workspace filter. // It also can return the list of validation errors to return to the api. -func workspaceSearchQuery(query string) (database.GetWorkspacesParams, []codersdk.ValidationError) { +func workspaceSearchQuery(query string, page codersdk.Pagination) (database.GetWorkspacesParams, []codersdk.ValidationError) { + filter := database.GetWorkspacesParams{ + Offset: int32(page.Offset), + Limit: int32(page.Limit), + } searchParams := make(url.Values) if query == "" { // No filter - return database.GetWorkspacesParams{}, nil + return filter, nil } query = strings.ToLower(query) // Because we do this in 2 passes, we want to maintain quotes on the first @@ -1112,13 +1121,10 @@ func workspaceSearchQuery(query string) (database.GetWorkspacesParams, []codersd // Using the query param parser here just returns consistent errors with // other parsing. parser := httpapi.NewQueryParamParser() - filter := database.GetWorkspacesParams{ - Deleted: false, - OwnerUsername: parser.String(searchParams, "", "owner"), - TemplateName: parser.String(searchParams, "", "template"), - Name: parser.String(searchParams, "", "name"), - Status: parser.String(searchParams, "", "status"), - } + filter.OwnerUsername = parser.String(searchParams, "", "owner") + filter.TemplateName = parser.String(searchParams, "", "template") + filter.Name = parser.String(searchParams, "", "name") + filter.Status = parser.String(searchParams, "", "status") return filter, parser.Errors } diff --git a/coderd/workspaces_internal_test.go b/coderd/workspaces_internal_test.go index f9617031f68bd..03a74b29f00ae 100644 --- a/coderd/workspaces_internal_test.go +++ b/coderd/workspaces_internal_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/codersdk" "github.com/stretchr/testify/require" ) @@ -135,7 +136,7 @@ func TestSearchWorkspace(t *testing.T) { c := c t.Run(c.Name, func(t *testing.T) { t.Parallel() - values, errs := workspaceSearchQuery(c.Query) + values, errs := workspaceSearchQuery(c.Query, codersdk.Pagination{}) if c.ExpectedErrorContains != "" { require.True(t, len(errs) > 0, "expect some errors") var s strings.Builder diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 648973d505197..1026dcdd6dd76 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -781,6 +781,47 @@ func TestWorkspaceFilterManual(t *testing.T) { }) } +func TestOffsetLimit(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(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) + _ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + _ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + _ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + + // empty finds all workspaces + ws, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{}) + require.NoError(t, err) + require.Len(t, ws, 3) + + // offset 1 finds 2 workspaces + ws, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ + Offset: 1, + }) + require.NoError(t, err) + require.Len(t, ws, 2) + + // offset 1 limit 1 finds 1 workspace + ws, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ + Offset: 1, + Limit: 1, + }) + require.NoError(t, err) + require.Len(t, ws, 1) + + // offset 3 finds no workspaces + ws, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{ + Offset: 3, + }) + require.NoError(t, err) + require.Len(t, ws, 0) +} + func TestPostWorkspaceBuild(t *testing.T) { t.Parallel() t.Run("NoTemplateVersion", func(t *testing.T) { diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index 7899fb33f8bd6..a019504ad9d09 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -254,6 +254,10 @@ type WorkspaceFilter struct { Name string `json:"name,omitempty" typescript:"-"` // Status is a workspace status, which is really the status of the latest build Status string `json:"status,omitempty" typescript:"-"` + // Offset is the number of workspaces to skip before returning results. + Offset int `json:"offset,omitempty" typescript:"-"` + // Limit is a limit on the number of workspaces returned. + Limit int `json:"limit,omitempty" typescript:"-"` // FilterQuery supports a raw filter query string FilterQuery string `json:"q,omitempty"` } @@ -290,7 +294,11 @@ func (f WorkspaceFilter) asRequestOption() RequestOption { // Workspaces returns all workspaces the authenticated user has access to. func (c *Client) Workspaces(ctx context.Context, filter WorkspaceFilter) ([]Workspace, error) { - res, err := c.Request(ctx, http.MethodGet, "/api/v2/workspaces", nil, filter.asRequestOption()) + page := Pagination{ + Offset: filter.Offset, + Limit: filter.Limit, + } + res, err := c.Request(ctx, http.MethodGet, "/api/v2/workspaces", nil, filter.asRequestOption(), page.asRequestOption()) if err != nil { return nil, err } From 3aa72a99d05c002b8e68c173b1e1933fb7fca6d4 Mon Sep 17 00:00:00 2001 From: Garrett Date: Wed, 12 Oct 2022 21:32:52 +0000 Subject: [PATCH 2/9] dont sent bad limits to db --- coderd/workspaces.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index c96967b1266ed..2c7867ecbbab4 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -1078,6 +1078,9 @@ func validWorkspaceSchedule(s *string, min time.Duration) (sql.NullString, error // workspaceSearchQuery takes a query string and returns the workspace filter. // It also can return the list of validation errors to return to the api. func workspaceSearchQuery(query string, page codersdk.Pagination) (database.GetWorkspacesParams, []codersdk.ValidationError) { + if page.Limit < 1 { + page.Limit = 65536 + } filter := database.GetWorkspacesParams{ Offset: int32(page.Offset), Limit: int32(page.Limit), From 8850b12ec11a37b8e9776f86d5877de1fb22cb2a Mon Sep 17 00:00:00 2001 From: Garrett Date: Thu, 13 Oct 2022 14:40:54 +0000 Subject: [PATCH 3/9] inject authorize filter properly --- coderd/database/modelqueries.go | 6 +++++- coderd/database/queries.sql.go | 2 ++ coderd/database/queries/workspaces.sql | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index 7f10019097c3e..f10fdd8478baa 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "strings" "github.com/lib/pq" @@ -164,8 +165,11 @@ type workspaceQuerier interface { // This code is copied from `GetWorkspaces` and adds the authorized filter WHERE // clause. func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]Workspace, error) { + // In order to properly use ORDER BY, OFFSET, and LIMIT, we need to inject the + // authorizedFilter between the end of the where claus and those statements. + filter := strings.Replace(getWorkspaces, "-- @authorize_filter", fmt.Sprintf(" AND %s", authorizedFilter.SQLString(rbac.NoACLConfig())), 1) // The name comment is for metric tracking - query := fmt.Sprintf("-- name: GetAuthorizedWorkspaces :many\n%s AND %s", getWorkspaces, authorizedFilter.SQLString(rbac.NoACLConfig())) + query := fmt.Sprintf("-- name: GetAuthorizedWorkspaces :many\n%s", filter) rows, err := q.db.QueryContext(ctx, query, arg.Limit, arg.Offset, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 9316918e60837..bd8ca58a80f4d 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5553,6 +5553,8 @@ WHERE name ILIKE '%' || $9 || '%' ELSE true END + -- Authorize Filter clause will be injected below in GetAuthorizedWorkspaces + -- @authorize_filter ORDER BY last_used_at DESC LIMIT diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 6552a647dacf1..3a35f2188afe4 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -132,6 +132,8 @@ WHERE name ILIKE '%' || @name || '%' ELSE true END + -- Authorize Filter clause will be injected below in GetAuthorizedWorkspaces + -- @authorize_filter ORDER BY last_used_at DESC LIMIT From a445e7820bc2c6d36534565f05f874686fcd9e1a Mon Sep 17 00:00:00 2001 From: Garrett Date: Thu, 13 Oct 2022 15:07:38 +0000 Subject: [PATCH 4/9] make limit null if < 1 --- coderd/database/queries.sql.go | 59 ++++++++++++++------------ coderd/database/queries/workspaces.sql | 7 ++- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index bd8ca58a80f4d..c31b4a276492a 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5457,60 +5457,60 @@ LEFT JOIN LATERAL ( ) latest_build ON TRUE WHERE -- Optionally include deleted workspaces - workspaces.deleted = $3 + workspaces.deleted = $1 AND CASE - WHEN $4 :: text != '' THEN + WHEN $2 :: text != '' THEN CASE - WHEN $4 = 'pending' THEN + WHEN $2 = 'pending' THEN latest_build.started_at IS NULL - WHEN $4 = 'starting' THEN + WHEN $2 = 'starting' THEN latest_build.started_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.completed_at IS NULL AND latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND latest_build.transition = 'start'::workspace_transition - WHEN $4 = 'running' THEN + WHEN $2 = 'running' THEN latest_build.completed_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.error IS NULL AND latest_build.transition = 'start'::workspace_transition - WHEN $4 = 'stopping' THEN + WHEN $2 = 'stopping' THEN latest_build.started_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.completed_at IS NULL AND latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND latest_build.transition = 'stop'::workspace_transition - WHEN $4 = 'stopped' THEN + WHEN $2 = 'stopped' THEN latest_build.completed_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.error IS NULL AND latest_build.transition = 'stop'::workspace_transition - WHEN $4 = 'failed' THEN + WHEN $2 = 'failed' THEN (latest_build.canceled_at IS NOT NULL AND latest_build.error IS NOT NULL) OR (latest_build.completed_at IS NOT NULL AND latest_build.error IS NOT NULL) - WHEN $4 = 'canceling' THEN + WHEN $2 = 'canceling' THEN latest_build.canceled_at IS NOT NULL AND latest_build.completed_at IS NULL - WHEN $4 = 'canceled' THEN + WHEN $2 = 'canceled' THEN latest_build.canceled_at IS NOT NULL AND latest_build.completed_at IS NOT NULL - WHEN $4 = 'deleted' THEN + WHEN $2 = 'deleted' THEN latest_build.started_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.completed_at IS NOT NULL AND latest_build.updated_at - INTERVAL '30 seconds' < NOW() AND latest_build.transition = 'delete'::workspace_transition - WHEN $4 = 'deleting' THEN + WHEN $2 = 'deleting' THEN latest_build.completed_at IS NOT NULL AND latest_build.canceled_at IS NULL AND latest_build.error IS NULL AND @@ -5523,34 +5523,34 @@ WHERE END -- Filter by owner_id AND CASE - WHEN $5 :: uuid != '00000000-00000000-00000000-00000000' THEN - owner_id = $5 + WHEN $3 :: uuid != '00000000-00000000-00000000-00000000' THEN + owner_id = $3 ELSE true END -- Filter by owner_name AND CASE - WHEN $6 :: text != '' THEN - owner_id = (SELECT id FROM users WHERE lower(username) = lower($6) AND deleted = false) + WHEN $4 :: text != '' THEN + owner_id = (SELECT id FROM users WHERE lower(username) = lower($4) AND deleted = false) ELSE true END -- Filter by template_name -- There can be more than 1 template with the same name across organizations. -- Use the organization filter to restrict to 1 org if needed. AND CASE - WHEN $7 :: text != '' THEN - template_id = ANY(SELECT id FROM templates WHERE lower(name) = lower($7) AND deleted = false) + WHEN $5 :: text != '' THEN + template_id = ANY(SELECT id FROM templates WHERE lower(name) = lower($5) AND deleted = false) ELSE true END -- Filter by template_ids AND CASE - WHEN array_length($8 :: uuid[], 1) > 0 THEN - template_id = ANY($8) + WHEN array_length($6 :: uuid[], 1) > 0 THEN + template_id = ANY($6) ELSE true END -- Filter by name, matching on substring AND CASE - WHEN $9 :: text != '' THEN - name ILIKE '%' || $9 || '%' + WHEN $7 :: text != '' THEN + name ILIKE '%' || $7 || '%' ELSE true END -- Authorize Filter clause will be injected below in GetAuthorizedWorkspaces @@ -5558,14 +5558,15 @@ WHERE ORDER BY last_used_at DESC LIMIT - $1 + CASE + WHEN $9 :: integer > 0 THEN + $9 + END OFFSET - $2 + $8 ` type GetWorkspacesParams struct { - Limit int32 `db:"limit" json:"limit"` - Offset int32 `db:"offset" json:"offset"` Deleted bool `db:"deleted" json:"deleted"` Status string `db:"status" json:"status"` OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` @@ -5573,12 +5574,12 @@ type GetWorkspacesParams struct { TemplateName string `db:"template_name" json:"template_name"` TemplateIds []uuid.UUID `db:"template_ids" json:"template_ids"` Name string `db:"name" json:"name"` + Offset int32 `db:"offset_" json:"offset_"` + Limit int32 `db:"limit_" json:"limit_"` } func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) ([]Workspace, error) { rows, err := q.db.QueryContext(ctx, getWorkspaces, - arg.Limit, - arg.Offset, arg.Deleted, arg.Status, arg.OwnerID, @@ -5586,6 +5587,8 @@ func (q *sqlQuerier) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) arg.TemplateName, pq.Array(arg.TemplateIds), arg.Name, + arg.Offset, + arg.Limit, ) if err != nil { return nil, err diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 3a35f2188afe4..4cd08b19f98a5 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -137,9 +137,12 @@ WHERE ORDER BY last_used_at DESC LIMIT - $1 + CASE + WHEN @limit_ :: integer > 0 THEN + @limit_ + END OFFSET - $2 + @offset_ ; -- name: GetWorkspaceByOwnerIDAndName :one From 370f9c73c1043295e4b471c83c5637725bde3ef7 Mon Sep 17 00:00:00 2001 From: Garrett Date: Thu, 13 Oct 2022 15:10:09 +0000 Subject: [PATCH 5/9] fmt --- coderd/database/queries/workspaces.sql | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 4cd08b19f98a5..6e7f0436dbff8 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -138,9 +138,9 @@ ORDER BY last_used_at DESC LIMIT CASE - WHEN @limit_ :: integer > 0 THEN - @limit_ - END + WHEN @limit_ :: integer > 0 THEN + @limit_ + END OFFSET @offset_ ; From 704574fc14d18dd2eb92748892bfc26fba0b2247 Mon Sep 17 00:00:00 2001 From: Garrett Date: Thu, 13 Oct 2022 15:11:38 +0000 Subject: [PATCH 6/9] fix hack in query --- coderd/workspaces.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 2c7867ecbbab4..c96967b1266ed 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -1078,9 +1078,6 @@ func validWorkspaceSchedule(s *string, min time.Duration) (sql.NullString, error // workspaceSearchQuery takes a query string and returns the workspace filter. // It also can return the list of validation errors to return to the api. func workspaceSearchQuery(query string, page codersdk.Pagination) (database.GetWorkspacesParams, []codersdk.ValidationError) { - if page.Limit < 1 { - page.Limit = 65536 - } filter := database.GetWorkspacesParams{ Offset: int32(page.Offset), Limit: int32(page.Limit), From a45e6c2c465f0ddb11f4ff54fcee9b12c1dc45c4 Mon Sep 17 00:00:00 2001 From: Garrett Date: Thu, 13 Oct 2022 15:15:19 +0000 Subject: [PATCH 7/9] typos --- coderd/database/modelqueries.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index f10fdd8478baa..c2b7003ae6b3d 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -166,7 +166,7 @@ type workspaceQuerier interface { // clause. func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspacesParams, authorizedFilter rbac.AuthorizeFilter) ([]Workspace, error) { // In order to properly use ORDER BY, OFFSET, and LIMIT, we need to inject the - // authorizedFilter between the end of the where claus and those statements. + // authorizedFilter between the end of the where clause and those statements. filter := strings.Replace(getWorkspaces, "-- @authorize_filter", fmt.Sprintf(" AND %s", authorizedFilter.SQLString(rbac.NoACLConfig())), 1) // The name comment is for metric tracking query := fmt.Sprintf("-- name: GetAuthorizedWorkspaces :many\n%s", filter) From e1e91d6f40722d450cb230f2402eff8ab7eb86e0 Mon Sep 17 00:00:00 2001 From: Garrett Date: Thu, 13 Oct 2022 15:41:09 +0000 Subject: [PATCH 8/9] fix order of fields --- coderd/database/modelqueries.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index c2b7003ae6b3d..3383b6af96e39 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -171,8 +171,6 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa // The name comment is for metric tracking query := fmt.Sprintf("-- name: GetAuthorizedWorkspaces :many\n%s", filter) rows, err := q.db.QueryContext(ctx, query, - arg.Limit, - arg.Offset, arg.Deleted, arg.Status, arg.OwnerID, @@ -180,6 +178,8 @@ func (q *sqlQuerier) GetAuthorizedWorkspaces(ctx context.Context, arg GetWorkspa arg.TemplateName, pq.Array(arg.TemplateIds), arg.Name, + arg.Offset, + arg.Limit, ) if err != nil { return nil, xerrors.Errorf("get authorized workspaces: %w", err) From 53136410d5e091ba44e656d678ca4dda7ee4056a Mon Sep 17 00:00:00 2001 From: Garrett Date: Thu, 13 Oct 2022 15:43:30 +0000 Subject: [PATCH 9/9] make gen --- coderd/database/queries.sql.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index c31b4a276492a..2ff1805cd47e1 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5559,9 +5559,9 @@ ORDER BY last_used_at DESC LIMIT CASE - WHEN $9 :: integer > 0 THEN - $9 - END + WHEN $9 :: integer > 0 THEN + $9 + END OFFSET $8 `