From c18b16255494e3019244247c32781dc99d3222ce Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 17 May 2022 18:06:31 -0500 Subject: [PATCH 1/9] feat: Add single query for all workspaces using a filter --- cli/list.go | 2 +- coderd/coderd.go | 38 +++++------ coderd/database/databasefake/databasefake.go | 23 +++++++ coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 66 ++++++++++++++++++++ coderd/database/queries/workspaces.sql | 26 ++++++++ coderd/users_test.go | 8 ++- coderd/workspaces.go | 52 +++++++++++---- codersdk/users.go | 16 ----- codersdk/workspaces.go | 38 +++++++++++ site/src/api/typesGenerated.ts | 6 ++ 11 files changed, 226 insertions(+), 50 deletions(-) diff --git a/cli/list.go b/cli/list.go index 6fb2a369d63fd..23454bc85674e 100644 --- a/cli/list.go +++ b/cli/list.go @@ -29,7 +29,7 @@ func list() *cobra.Command { if err != nil { return err } - workspaces, err := client.WorkspacesByUser(cmd.Context(), codersdk.Me) + workspaces, err := client.Workspaces(cmd.Context(), codersdk.WorkspaceFilter{}) if err != nil { return err } diff --git a/coderd/coderd.go b/coderd/coderd.go index 1f91e97ccd889..23b65d94bec5a 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -253,7 +253,6 @@ func New(options *Options) (http.Handler, func()) { }) r.Get("/gitsshkey", api.gitSSHKey) r.Put("/gitsshkey", api.regenerateGitSSHKey) - r.Get("/workspaces", api.workspacesByUser) }) }) }) @@ -289,23 +288,26 @@ func New(options *Options) (http.Handler, func()) { ) r.Get("/", api.workspaceResource) }) - r.Route("/workspaces/{workspace}", func(r chi.Router) { - r.Use( - apiKeyMiddleware, - authRolesMiddleware, - httpmw.ExtractWorkspaceParam(options.Database), - ) - r.Get("/", api.workspace) - r.Route("/builds", func(r chi.Router) { - r.Get("/", api.workspaceBuilds) - r.Post("/", api.postWorkspaceBuilds) - r.Get("/{workspacebuildname}", api.workspaceBuildByName) - }) - r.Route("/autostart", func(r chi.Router) { - r.Put("/", api.putWorkspaceAutostart) - }) - r.Route("/autostop", func(r chi.Router) { - r.Put("/", api.putWorkspaceAutostop) + r.Route("/workspaces", func(r chi.Router) { + r.Get("/", api.workspaces) + r.Route("/{workspace}", func(r chi.Router) { + r.Use( + apiKeyMiddleware, + authRolesMiddleware, + httpmw.ExtractWorkspaceParam(options.Database), + ) + r.Get("/", api.workspace) + r.Route("/builds", func(r chi.Router) { + r.Get("/", api.workspaceBuilds) + r.Post("/", api.postWorkspaceBuilds) + r.Get("/{workspacebuildname}", api.workspaceBuildByName) + }) + r.Route("/autostart", func(r chi.Router) { + r.Put("/", api.putWorkspaceAutostart) + }) + r.Route("/autostop", func(r chi.Router) { + r.Put("/", api.putWorkspaceAutostop) + }) }) }) r.Route("/workspacebuilds/{workspacebuild}", func(r chi.Router) { diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 1e80910d97417..2bad57ac8aa4e 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -291,6 +291,29 @@ func (q *fakeQuerier) GetAllUserRoles(_ context.Context, userID uuid.UUID) (data }, nil } +func (q *fakeQuerier) GetWorkspacesWithFilter(_ context.Context, arg database.GetWorkspacesWithFilterParams) ([]database.Workspace, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + workspaces := make([]database.Workspace, 0) + for _, workspace := range q.workspaces { + if arg.OrganizationID != uuid.Nil && workspace.OrganizationID != arg.OrganizationID { + continue + } + if arg.OwnerID != uuid.Nil && workspace.OwnerID != arg.OwnerID { + continue + } + if !arg.IncludeDeleted && workspace.Deleted { + continue + } + workspaces = append(workspaces, workspace) + } + if len(workspaces) == 0 { + return nil, sql.ErrNoRows + } + return workspaces, nil +} + func (q *fakeQuerier) GetWorkspacesByTemplateID(_ context.Context, arg database.GetWorkspacesByTemplateIDParams) ([]database.Workspace, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index faff010d11759..1f1df2c77af8e 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -75,6 +75,7 @@ type querier interface { GetWorkspacesByOrganizationIDs(ctx context.Context, arg GetWorkspacesByOrganizationIDsParams) ([]Workspace, error) GetWorkspacesByOwnerID(ctx context.Context, arg GetWorkspacesByOwnerIDParams) ([]Workspace, error) GetWorkspacesByTemplateID(ctx context.Context, arg GetWorkspacesByTemplateIDParams) ([]Workspace, error) + GetWorkspacesWithFilter(ctx context.Context, arg GetWorkspacesWithFilterParams) ([]Workspace, error) InsertAPIKey(ctx context.Context, arg InsertAPIKeyParams) (APIKey, error) InsertAuditLog(ctx context.Context, arg InsertAuditLogParams) (AuditLog, error) InsertFile(ctx context.Context, arg InsertFileParams) (File, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index de4ac465660dc..479636b3e710a 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3498,6 +3498,72 @@ func (q *sqlQuerier) GetWorkspacesByTemplateID(ctx context.Context, arg GetWorks return items, nil } +const getWorkspacesWithFilter = `-- name: GetWorkspacesWithFilter :many +SELECT + id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, autostop_schedule +FROM + workspaces +WHERE + -- Optionally include deleted workspaces + CASE + WHEN $1 :: boolean = true THEN + true + ELSE deleted = false + END + -- Filter by organization_id + AND CASE + WHEN $2 :: uuid != '00000000-00000000-00000000-00000000' THEN + organization_id = $2 + ELSE true + END + -- Filter by owner_id + AND CASE + WHEN $3 :: uuid != '00000000-00000000-00000000-00000000' THEN + owner_id = $3 + ELSE true + END +` + +type GetWorkspacesWithFilterParams struct { + IncludeDeleted bool `db:"include_deleted" json:"include_deleted"` + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` +} + +func (q *sqlQuerier) GetWorkspacesWithFilter(ctx context.Context, arg GetWorkspacesWithFilterParams) ([]Workspace, error) { + rows, err := q.db.QueryContext(ctx, getWorkspacesWithFilter, arg.IncludeDeleted, arg.OrganizationID, arg.OwnerID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []Workspace + for rows.Next() { + var i Workspace + if err := rows.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.OwnerID, + &i.OrganizationID, + &i.TemplateID, + &i.Deleted, + &i.Name, + &i.AutostartSchedule, + &i.AutostopSchedule, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const insertWorkspace = `-- name: InsertWorkspace :one INSERT INTO workspaces ( diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index f8e68b110656d..ce0caae63676e 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -8,6 +8,32 @@ WHERE LIMIT 1; +-- name: GetWorkspacesWithFilter :many +SELECT + * +FROM + workspaces +WHERE + -- Optionally include deleted workspaces + CASE + WHEN @include_deleted :: boolean = true THEN + true + ELSE deleted = false + END + -- Filter by organization_id + AND CASE + WHEN @organization_id :: uuid != '00000000-00000000-00000000-00000000' THEN + organization_id = @organization_id + ELSE true + END + -- Filter by owner_id + AND CASE + WHEN @owner_id :: uuid != '00000000-00000000-00000000-00000000' THEN + owner_id = @owner_id + ELSE true + END +; + -- name: GetWorkspacesByOrganizationID :many SELECT * FROM workspaces WHERE organization_id = $1 AND deleted = $2; diff --git a/coderd/users_test.go b/coderd/users_test.go index a39f733cf87d0..18aebb9040c13 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -599,7 +599,9 @@ func TestWorkspacesByUser(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) _ = coderdtest.CreateFirstUser(t, client) - workspaces, err := client.WorkspacesByUser(context.Background(), codersdk.Me) + workspaces, err := client.Workspaces(context.Background(), codersdk.WorkspaceFilter{ + OwnerID: codersdk.Me, + }) require.NoError(t, err) require.Len(t, workspaces, 0) }) @@ -628,11 +630,11 @@ func TestWorkspacesByUser(t *testing.T) { template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - workspaces, err := newUserClient.WorkspacesByUser(context.Background(), codersdk.Me) + workspaces, err := newUserClient.Workspaces(context.Background(), codersdk.WorkspaceFilter{OwnerID: codersdk.Me}) require.NoError(t, err) require.Len(t, workspaces, 0) - workspaces, err = client.WorkspacesByUser(context.Background(), codersdk.Me) + workspaces, err = client.Workspaces(context.Background(), codersdk.WorkspaceFilter{OwnerID: codersdk.Me}) require.NoError(t, err) require.Len(t, workspaces, 1) }) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 0e473fbdbcd79..6768346b3909f 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -70,9 +70,9 @@ func (api *api) workspace(rw http.ResponseWriter, r *http.Request) { func (api *api) workspacesByOrganization(rw http.ResponseWriter, r *http.Request) { organization := httpmw.OrganizationParam(r) roles := httpmw.UserRoles(r) - workspaces, err := api.Database.GetWorkspacesByOrganizationID(r.Context(), database.GetWorkspacesByOrganizationIDParams{ + workspaces, err := api.Database.GetWorkspacesWithFilter(r.Context(), database.GetWorkspacesWithFilterParams{ OrganizationID: organization.ID, - Deleted: false, + IncludeDeleted: false, }) if errors.Is(err, sql.ErrNoRows) { err = nil @@ -104,30 +104,58 @@ func (api *api) workspacesByOrganization(rw http.ResponseWriter, r *http.Request httpapi.Write(rw, http.StatusOK, apiWorkspaces) } -func (api *api) workspacesByUser(rw http.ResponseWriter, r *http.Request) { - user := httpmw.UserParam(r) +// workspaces returns all workspaces a user can read. +// Optional filters with query params +func (api *api) workspaces(rw http.ResponseWriter, r *http.Request) { roles := httpmw.UserRoles(r) + apiKey := httpmw.APIKey(r) - allWorkspaces := make([]database.Workspace, 0) - userWorkspaces, err := api.Database.GetWorkspacesByOwnerID(r.Context(), database.GetWorkspacesByOwnerIDParams{ - OwnerID: user.ID, - }) + // Empty strings mean no filter + orgFilter := r.URL.Query().Get("organization_id") + ownerFilter := r.URL.Query().Get("owner_id") + + filter := database.GetWorkspacesWithFilterParams{IncludeDeleted: false} + if orgFilter != "" { + orgID, err := uuid.Parse(orgFilter) + if err != nil { + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: fmt.Sprintf("organization_id must be a uuid: %s", err.Error()), + }) + return + } + filter.OrganizationID = orgID + } + if ownerFilter == "me" { + filter.OwnerID = apiKey.UserID + } else if ownerFilter != "" { + userID, err := uuid.Parse(orgFilter) + if err != nil { + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: fmt.Sprintf("owner_id must be a uuid: %s", err.Error()), + }) + return + } + filter.OwnerID = userID + } + + allowedWorkspaces := make([]database.Workspace, 0) + allWorkspaces, err := api.Database.GetWorkspacesWithFilter(r.Context(), filter) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: fmt.Sprintf("get workspaces for user: %s", err), }) return } - for _, ws := range userWorkspaces { + for _, ws := range allWorkspaces { ws := ws - err = api.Authorizer.ByRoleName(r.Context(), user.ID.String(), roles.Roles, rbac.ActionRead, + err = api.Authorizer.ByRoleName(r.Context(), roles.ID.String(), roles.Roles, rbac.ActionRead, rbac.ResourceWorkspace.InOrg(ws.OrganizationID).WithOwner(ws.OwnerID.String()).WithID(ws.ID.String())) if err == nil { - allWorkspaces = append(allWorkspaces, ws) + allowedWorkspaces = append(allowedWorkspaces, ws) } } - apiWorkspaces, err := convertWorkspaces(r.Context(), api.Database, allWorkspaces) + apiWorkspaces, err := convertWorkspaces(r.Context(), api.Database, allowedWorkspaces) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: fmt.Sprintf("convert workspaces: %s", err), diff --git a/codersdk/users.go b/codersdk/users.go index 228885d48fe7f..61faa398d0fa6 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -438,19 +438,3 @@ func (c *Client) AuthMethods(ctx context.Context) (AuthMethods, error) { var userAuth AuthMethods return userAuth, json.NewDecoder(res.Body).Decode(&userAuth) } - -// WorkspacesByUser returns all workspaces a user has access to. -func (c *Client) WorkspacesByUser(ctx context.Context, user string) ([]Workspace, error) { - res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/workspaces", user), nil) - if err != nil { - return nil, err - } - defer res.Body.Close() - - if res.StatusCode != http.StatusOK { - return nil, readBodyAsError(res) - } - - var workspaces []Workspace - return workspaces, json.NewDecoder(res.Body).Decode(&workspaces) -} diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index 76693d6efad11..95bfc2cc58649 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -131,3 +131,41 @@ func (c *Client) UpdateWorkspaceAutostop(ctx context.Context, id uuid.UUID, req } return nil } + +type WorkspaceFilter struct { + OrganizationID uuid.UUID + // OwnerID must be a uuid or "me" + OwnerID string +} + +// asRequestOption returns a function that can be used in (*Client).Request. +// It modifies the request query parameters. +func (f WorkspaceFilter) asRequestOption() requestOption { + return func(r *http.Request) { + q := r.URL.Query() + if f.OrganizationID != uuid.Nil { + q.Set("organization_id", f.OrganizationID.String()) + } + if f.OwnerID != "" { + q.Set("owner_id", f.OwnerID) + } + r.URL.RawQuery = q.Encode() + } +} + +// 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()) + + if err != nil { + return nil, err + } + defer res.Body.Close() + + if res.StatusCode != http.StatusOK { + return nil, readBodyAsError(res) + } + + var workspaces []Workspace + return workspaces, json.NewDecoder(res.Body).Decode(&workspaces) +} diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 6f35a5236e187..e45461e50f85b 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -430,6 +430,12 @@ export interface WorkspaceBuild { readonly job: ProvisionerJob } +// From codersdk/workspaces.go:135:6 +export interface WorkspaceFilter { + readonly OrganizationID: string + readonly OwnerID: string +} + // From codersdk/workspaceresources.go:23:6 export interface WorkspaceResource { readonly id: string From f0bf9346f80446e99153676b2fc73152eaada15f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 17 May 2022 18:20:46 -0500 Subject: [PATCH 2/9] Remove extra database functions --- coderd/coderd.go | 6 +- coderd/coderd_test.go | 12 ++- coderd/database/databasefake/databasefake.go | 41 +-------- coderd/database/querier.go | 2 - coderd/database/queries.sql.go | 92 -------------------- coderd/database/queries/workspaces.sql | 12 --- coderd/workspaces.go | 5 +- 7 files changed, 18 insertions(+), 152 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 23b65d94bec5a..1f1a4ff18dfac 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -289,11 +289,13 @@ func New(options *Options) (http.Handler, func()) { r.Get("/", api.workspaceResource) }) r.Route("/workspaces", func(r chi.Router) { + r.Use( + apiKeyMiddleware, + authRolesMiddleware, + ) r.Get("/", api.workspaces) r.Route("/{workspace}", func(r chi.Router) { r.Use( - apiKeyMiddleware, - authRolesMiddleware, httpmw.ExtractWorkspaceParam(options.Database), ) r.Get("/", api.workspace) diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index d096682957538..cc34d89ed6bee 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -78,7 +78,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "GET:/api/v2/workspaceagents/me/metadata": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/me/turn": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/{workspaceagent}": {NoAuthorize: true}, - "GET:/api/v2/workspaceagents/{workspaceagent}/": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/{workspaceagent}/dial": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/{workspaceagent}/iceservers": {NoAuthorize: true}, "GET:/api/v2/workspaceagents/{workspaceagent}/pty": {NoAuthorize: true}, @@ -95,7 +94,6 @@ func TestAuthorizeAllEndpoints(t *testing.T) { "GET:/api/v2/users/oauth2/github/callback": {NoAuthorize: true}, - "POST:/api/v2/users/{user}/organizations/": {NoAuthorize: true}, "PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true}, "GET:/api/v2/organizations/{organization}/provisionerdaemons": {NoAuthorize: true}, "POST:/api/v2/organizations/{organization}/templates": {NoAuthorize: true}, @@ -143,11 +141,21 @@ func TestAuthorizeAllEndpoints(t *testing.T) { AssertObject: rbac.ResourceWorkspace.InOrg(organization.ID).WithID(workspace.ID.String()).WithOwner(workspace.OwnerID.String()), }, "GET:/api/v2/organizations/{organization}/workspaces": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace}, + "GET:/api/v2/workspaces": {StatusCode: http.StatusOK, AssertObject: rbac.ResourceWorkspace}, // These endpoints need payloads to get to the auth part. "PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, } + for k, v := range assertRoute { + noTrailSlash := strings.TrimRight(k, "/") + if _, ok := assertRoute[noTrailSlash]; ok && noTrailSlash != k { + t.Errorf("route %q & %q is declared twice", noTrailSlash, k) + t.FailNow() + } + assertRoute[noTrailSlash] = v + } + c, _ := srv.Config.Handler.(*chi.Mux) err = chi.Walk(c, func(method string, route string, handler http.Handler, middlewares ...func(http.Handler) http.Handler) error { name := method + ":" + route diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 2bad57ac8aa4e..be5a4255c9720 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -308,9 +308,7 @@ func (q *fakeQuerier) GetWorkspacesWithFilter(_ context.Context, arg database.Ge } workspaces = append(workspaces, workspace) } - if len(workspaces) == 0 { - return nil, sql.ErrNoRows - } + return workspaces, nil } @@ -509,26 +507,6 @@ func (q *fakeQuerier) GetWorkspaceBuildByWorkspaceIDAndName(_ context.Context, a return database.WorkspaceBuild{}, sql.ErrNoRows } -func (q *fakeQuerier) GetWorkspacesByOrganizationID(_ context.Context, req database.GetWorkspacesByOrganizationIDParams) ([]database.Workspace, error) { - q.mutex.RLock() - defer q.mutex.RUnlock() - - workspaces := make([]database.Workspace, 0) - for _, workspace := range q.workspaces { - if workspace.OrganizationID != req.OrganizationID { - continue - } - if workspace.Deleted != req.Deleted { - continue - } - workspaces = append(workspaces, workspace) - } - if len(workspaces) == 0 { - return nil, sql.ErrNoRows - } - return workspaces, nil -} - func (q *fakeQuerier) GetWorkspacesByOrganizationIDs(_ context.Context, req database.GetWorkspacesByOrganizationIDsParams) ([]database.Workspace, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -548,23 +526,6 @@ func (q *fakeQuerier) GetWorkspacesByOrganizationIDs(_ context.Context, req data return workspaces, nil } -func (q *fakeQuerier) GetWorkspacesByOwnerID(_ context.Context, req database.GetWorkspacesByOwnerIDParams) ([]database.Workspace, error) { - q.mutex.RLock() - defer q.mutex.RUnlock() - - workspaces := make([]database.Workspace, 0) - for _, workspace := range q.workspaces { - if workspace.OwnerID != req.OwnerID { - continue - } - if workspace.Deleted != req.Deleted { - continue - } - workspaces = append(workspaces, workspace) - } - return workspaces, nil -} - func (q *fakeQuerier) GetOrganizations(_ context.Context) ([]database.Organization, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 1f1df2c77af8e..308c1a14aeb1a 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -71,9 +71,7 @@ type querier interface { GetWorkspaceResourceByID(ctx context.Context, id uuid.UUID) (WorkspaceResource, error) GetWorkspaceResourcesByJobID(ctx context.Context, jobID uuid.UUID) ([]WorkspaceResource, error) GetWorkspacesAutostartAutostop(ctx context.Context) ([]Workspace, error) - GetWorkspacesByOrganizationID(ctx context.Context, arg GetWorkspacesByOrganizationIDParams) ([]Workspace, error) GetWorkspacesByOrganizationIDs(ctx context.Context, arg GetWorkspacesByOrganizationIDsParams) ([]Workspace, error) - GetWorkspacesByOwnerID(ctx context.Context, arg GetWorkspacesByOwnerIDParams) ([]Workspace, error) GetWorkspacesByTemplateID(ctx context.Context, arg GetWorkspacesByTemplateIDParams) ([]Workspace, error) GetWorkspacesWithFilter(ctx context.Context, arg GetWorkspacesWithFilterParams) ([]Workspace, error) InsertAPIKey(ctx context.Context, arg InsertAPIKeyParams) (APIKey, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 479636b3e710a..6887eb53dc333 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3314,49 +3314,6 @@ func (q *sqlQuerier) GetWorkspacesAutostartAutostop(ctx context.Context) ([]Work return items, nil } -const getWorkspacesByOrganizationID = `-- name: GetWorkspacesByOrganizationID :many -SELECT id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, autostop_schedule FROM workspaces WHERE organization_id = $1 AND deleted = $2 -` - -type GetWorkspacesByOrganizationIDParams struct { - OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` - Deleted bool `db:"deleted" json:"deleted"` -} - -func (q *sqlQuerier) GetWorkspacesByOrganizationID(ctx context.Context, arg GetWorkspacesByOrganizationIDParams) ([]Workspace, error) { - rows, err := q.db.QueryContext(ctx, getWorkspacesByOrganizationID, arg.OrganizationID, arg.Deleted) - if err != nil { - return nil, err - } - defer rows.Close() - var items []Workspace - for rows.Next() { - var i Workspace - if err := rows.Scan( - &i.ID, - &i.CreatedAt, - &i.UpdatedAt, - &i.OwnerID, - &i.OrganizationID, - &i.TemplateID, - &i.Deleted, - &i.Name, - &i.AutostartSchedule, - &i.AutostopSchedule, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const getWorkspacesByOrganizationIDs = `-- name: GetWorkspacesByOrganizationIDs :many SELECT id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, autostop_schedule FROM workspaces WHERE organization_id = ANY($1 :: uuid [ ]) AND deleted = $2 ` @@ -3400,55 +3357,6 @@ func (q *sqlQuerier) GetWorkspacesByOrganizationIDs(ctx context.Context, arg Get return items, nil } -const getWorkspacesByOwnerID = `-- name: GetWorkspacesByOwnerID :many -SELECT - id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, autostop_schedule -FROM - workspaces -WHERE - owner_id = $1 - AND deleted = $2 -` - -type GetWorkspacesByOwnerIDParams struct { - OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` - Deleted bool `db:"deleted" json:"deleted"` -} - -func (q *sqlQuerier) GetWorkspacesByOwnerID(ctx context.Context, arg GetWorkspacesByOwnerIDParams) ([]Workspace, error) { - rows, err := q.db.QueryContext(ctx, getWorkspacesByOwnerID, arg.OwnerID, arg.Deleted) - if err != nil { - return nil, err - } - defer rows.Close() - var items []Workspace - for rows.Next() { - var i Workspace - if err := rows.Scan( - &i.ID, - &i.CreatedAt, - &i.UpdatedAt, - &i.OwnerID, - &i.OrganizationID, - &i.TemplateID, - &i.Deleted, - &i.Name, - &i.AutostartSchedule, - &i.AutostopSchedule, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const getWorkspacesByTemplateID = `-- name: GetWorkspacesByTemplateID :many SELECT id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, autostop_schedule diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index ce0caae63676e..9ad12707141ed 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -34,9 +34,6 @@ WHERE END ; --- name: GetWorkspacesByOrganizationID :many -SELECT * FROM workspaces WHERE organization_id = $1 AND deleted = $2; - -- name: GetWorkspacesByOrganizationIDs :many SELECT * FROM workspaces WHERE organization_id = ANY(@ids :: uuid [ ]) AND deleted = @deleted; @@ -63,15 +60,6 @@ WHERE template_id = $1 AND deleted = $2; --- name: GetWorkspacesByOwnerID :many -SELECT - * -FROM - workspaces -WHERE - owner_id = $1 - AND deleted = $2; - -- name: GetWorkspaceByOwnerIDAndName :one SELECT * diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 6768346b3909f..b23a2349ae83c 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -168,8 +168,9 @@ func (api *api) workspaces(rw http.ResponseWriter, r *http.Request) { func (api *api) workspacesByOwner(rw http.ResponseWriter, r *http.Request) { owner := httpmw.UserParam(r) roles := httpmw.UserRoles(r) - workspaces, err := api.Database.GetWorkspacesByOwnerID(r.Context(), database.GetWorkspacesByOwnerIDParams{ - OwnerID: owner.ID, + workspaces, err := api.Database.GetWorkspacesWithFilter(r.Context(), database.GetWorkspacesWithFilterParams{ + OwnerID: owner.ID, + IncludeDeleted: false, }) if errors.Is(err, sql.ErrNoRows) { err = nil From 817bae161fd6cbed54c4cc2b5c8e601a953f73d2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 17 May 2022 18:25:05 -0500 Subject: [PATCH 3/9] Also accept username or email --- coderd/workspaces.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index b23a2349ae83c..a751e5afb16b7 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -128,12 +128,21 @@ func (api *api) workspaces(rw http.ResponseWriter, r *http.Request) { if ownerFilter == "me" { filter.OwnerID = apiKey.UserID } else if ownerFilter != "" { - userID, err := uuid.Parse(orgFilter) + userID, err := uuid.Parse(ownerFilter) if err != nil { - httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: fmt.Sprintf("owner_id must be a uuid: %s", err.Error()), + // Maybe it's a username + user, err := api.Database.GetUserByEmailOrUsername(r.Context(), database.GetUserByEmailOrUsernameParams{ + // Why not just accept 1 arg and use it for both in the sql? + Username: ownerFilter, + Email: ownerFilter, }) - return + if err != nil { + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: "owner must be a uuid or username", + }) + return + } + userID = user.ID } filter.OwnerID = userID } From 8cc4a2c8e014948b2e3a20f63a75c4df16d9de04 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 17 May 2022 18:29:38 -0500 Subject: [PATCH 4/9] Make a unit test use a username --- coderd/users_test.go | 6 +++--- coderd/workspaces_test.go | 9 ++++++++- codersdk/workspaces.go | 8 ++++---- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/coderd/users_test.go b/coderd/users_test.go index 18aebb9040c13..9c2846ed96cbd 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -600,7 +600,7 @@ func TestWorkspacesByUser(t *testing.T) { client := coderdtest.New(t, nil) _ = coderdtest.CreateFirstUser(t, client) workspaces, err := client.Workspaces(context.Background(), codersdk.WorkspaceFilter{ - OwnerID: codersdk.Me, + Owner: codersdk.Me, }) require.NoError(t, err) require.Len(t, workspaces, 0) @@ -630,11 +630,11 @@ func TestWorkspacesByUser(t *testing.T) { template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - workspaces, err := newUserClient.Workspaces(context.Background(), codersdk.WorkspaceFilter{OwnerID: codersdk.Me}) + workspaces, err := newUserClient.Workspaces(context.Background(), codersdk.WorkspaceFilter{Owner: codersdk.Me}) require.NoError(t, err) require.Len(t, workspaces, 0) - workspaces, err = client.Workspaces(context.Background(), codersdk.WorkspaceFilter{OwnerID: codersdk.Me}) + workspaces, err = client.Workspaces(context.Background(), codersdk.WorkspaceFilter{Owner: codersdk.Me}) require.NoError(t, err) require.Len(t, workspaces, 1) }) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 6183b853c1733..e996d27da7609 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -139,11 +139,18 @@ func TestWorkspacesByOwner(t *testing.T) { client := coderdtest.New(t, nil) coderdtest.NewProvisionerDaemon(t, client) user := coderdtest.CreateFirstUser(t, client) + me, err := client.User(context.Background(), codersdk.Me) + require.NoError(t, err) + 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) - workspaces, err := client.WorkspacesByOwner(context.Background(), user.OrganizationID, codersdk.Me) + // Use a username + workspaces, err := client.Workspaces(context.Background(), codersdk.WorkspaceFilter{ + OrganizationID: user.OrganizationID, + Owner: me.Username, + }) require.NoError(t, err) require.Len(t, workspaces, 1) }) diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index 95bfc2cc58649..a7bcff37b40a9 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -134,8 +134,8 @@ func (c *Client) UpdateWorkspaceAutostop(ctx context.Context, id uuid.UUID, req type WorkspaceFilter struct { OrganizationID uuid.UUID - // OwnerID must be a uuid or "me" - OwnerID string + // Owner can be a user_id (uuid), "me", or a username + Owner string } // asRequestOption returns a function that can be used in (*Client).Request. @@ -146,8 +146,8 @@ func (f WorkspaceFilter) asRequestOption() requestOption { if f.OrganizationID != uuid.Nil { q.Set("organization_id", f.OrganizationID.String()) } - if f.OwnerID != "" { - q.Set("owner_id", f.OwnerID) + if f.Owner != "" { + q.Set("owner_id", f.Owner) } r.URL.RawQuery = q.Encode() } From 67423fb23c710a13c33ca3c70fee62f21126106b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 17 May 2022 18:30:48 -0500 Subject: [PATCH 5/9] Add extra assertion to unit test --- coderd/workspaces_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index e996d27da7609..724be6106d3e1 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -134,7 +134,8 @@ func TestWorkspacesByOwner(t *testing.T) { _, err := client.WorkspacesByOwner(context.Background(), user.OrganizationID, codersdk.Me) require.NoError(t, err) }) - t.Run("List", func(t *testing.T) { + + t.Run("ListMine", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) coderdtest.NewProvisionerDaemon(t, client) @@ -146,6 +147,11 @@ func TestWorkspacesByOwner(t *testing.T) { coderdtest.AwaitTemplateVersionJob(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) _ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + + // Create noise workspace that should be filtered out + other := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + _ = coderdtest.CreateWorkspace(t, other, user.OrganizationID, template.ID) + // Use a username workspaces, err := client.Workspaces(context.Background(), codersdk.WorkspaceFilter{ OrganizationID: user.OrganizationID, From ba3f15d807f6b37ec97ea6ef77f4ab8971e79180 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 17 May 2022 18:49:37 -0500 Subject: [PATCH 6/9] Update ui api call --- site/src/api/api.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 5151166e8f1a9..4d34c9c7fed86 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -115,8 +115,10 @@ export const getWorkspace = async (workspaceId: string): Promise => { - const response = await axios.get(`/api/v2/users/${userID}/workspaces`) +// TODO: @emyrk add query params as arguments. Supports 'organization_id' and 'owner' +// 'owner' can be a username, user_id, or 'me' +export const getWorkspaces = async (): Promise => { + const response = await axios.get(`/api/v2/workspaces`) return response.data } From 3142a22993eb7c732ec25cac8d296173bad7d2ca Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 May 2022 09:08:13 -0500 Subject: [PATCH 7/9] Fix js test --- site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index df6151c857380..8c26e1c13cf25 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -15,7 +15,7 @@ describe("WorkspacesPage", () => { it("renders an empty workspaces page", async () => { // Given server.use( - rest.get("/api/v2/users/me/workspaces", async (req, res, ctx) => { + rest.get("/api/v2/workspaces", async (req, res, ctx) => { return res(ctx.status(200), ctx.json([])) }), ) From c1bb602bcb6f0d4b778ac63d58cbb32193aef327 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 May 2022 09:17:15 -0500 Subject: [PATCH 8/9] Fix js mocks --- site/src/testHelpers/handlers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index 1f65874616dc1..9fdc30c1d2c6f 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -33,7 +33,7 @@ export const handlers = [ rest.post("/api/v2/users/me/workspaces", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(M.MockWorkspace)) }), - rest.get("/api/v2/users/me/workspaces", async (req, res, ctx) => { + rest.get("/api/v2/workspaces", async (req, res, ctx) => { return res(ctx.status(200), ctx.json([M.MockWorkspace])) }), rest.get("/api/v2/users/me/organizations", (req, res, ctx) => { From 32cf21fb37f13fd30c7db95d3ab0dd15b0ee4ec1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 18 May 2022 09:36:05 -0500 Subject: [PATCH 9/9] Deleted instead of include deleted --- coderd/database/databasefake/databasefake.go | 2 +- coderd/database/queries.sql.go | 10 +++------- coderd/database/queries/workspaces.sql | 6 +----- coderd/workspaces.go | 8 ++++---- 4 files changed, 9 insertions(+), 17 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index be5a4255c9720..6f213048d81b8 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -303,7 +303,7 @@ func (q *fakeQuerier) GetWorkspacesWithFilter(_ context.Context, arg database.Ge if arg.OwnerID != uuid.Nil && workspace.OwnerID != arg.OwnerID { continue } - if !arg.IncludeDeleted && workspace.Deleted { + if !arg.Deleted && workspace.Deleted { continue } workspaces = append(workspaces, workspace) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 6887eb53dc333..24e96f792c9d4 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3413,11 +3413,7 @@ FROM workspaces WHERE -- Optionally include deleted workspaces - CASE - WHEN $1 :: boolean = true THEN - true - ELSE deleted = false - END + deleted = $1 -- Filter by organization_id AND CASE WHEN $2 :: uuid != '00000000-00000000-00000000-00000000' THEN @@ -3433,13 +3429,13 @@ WHERE ` type GetWorkspacesWithFilterParams struct { - IncludeDeleted bool `db:"include_deleted" json:"include_deleted"` + Deleted bool `db:"deleted" json:"deleted"` OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` OwnerID uuid.UUID `db:"owner_id" json:"owner_id"` } func (q *sqlQuerier) GetWorkspacesWithFilter(ctx context.Context, arg GetWorkspacesWithFilterParams) ([]Workspace, error) { - rows, err := q.db.QueryContext(ctx, getWorkspacesWithFilter, arg.IncludeDeleted, arg.OrganizationID, arg.OwnerID) + rows, err := q.db.QueryContext(ctx, getWorkspacesWithFilter, arg.Deleted, arg.OrganizationID, arg.OwnerID) if err != nil { return nil, err } diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 9ad12707141ed..eb87ad9a51d41 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -15,11 +15,7 @@ FROM workspaces WHERE -- Optionally include deleted workspaces - CASE - WHEN @include_deleted :: boolean = true THEN - true - ELSE deleted = false - END + deleted = @deleted -- Filter by organization_id AND CASE WHEN @organization_id :: uuid != '00000000-00000000-00000000-00000000' THEN diff --git a/coderd/workspaces.go b/coderd/workspaces.go index a751e5afb16b7..37b8c7d054b5b 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -72,7 +72,7 @@ func (api *api) workspacesByOrganization(rw http.ResponseWriter, r *http.Request roles := httpmw.UserRoles(r) workspaces, err := api.Database.GetWorkspacesWithFilter(r.Context(), database.GetWorkspacesWithFilterParams{ OrganizationID: organization.ID, - IncludeDeleted: false, + Deleted: false, }) if errors.Is(err, sql.ErrNoRows) { err = nil @@ -114,7 +114,7 @@ func (api *api) workspaces(rw http.ResponseWriter, r *http.Request) { orgFilter := r.URL.Query().Get("organization_id") ownerFilter := r.URL.Query().Get("owner_id") - filter := database.GetWorkspacesWithFilterParams{IncludeDeleted: false} + filter := database.GetWorkspacesWithFilterParams{Deleted: false} if orgFilter != "" { orgID, err := uuid.Parse(orgFilter) if err != nil { @@ -178,8 +178,8 @@ func (api *api) workspacesByOwner(rw http.ResponseWriter, r *http.Request) { owner := httpmw.UserParam(r) roles := httpmw.UserRoles(r) workspaces, err := api.Database.GetWorkspacesWithFilter(r.Context(), database.GetWorkspacesWithFilterParams{ - OwnerID: owner.ID, - IncludeDeleted: false, + OwnerID: owner.ID, + Deleted: false, }) if errors.Is(err, sql.ErrNoRows) { err = nil