Skip to content

feat: Single query for all workspaces with optional filter #1537

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 18, 2022
Merged

feat: Single query for all workspaces with optional filter #1537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c18b162
feat: Add single query for all workspaces using a filter
Emyrk May 17, 2022
f0bf934
Remove extra database functions
Emyrk May 17, 2022
817bae1
Also accept username or email
Emyrk May 17, 2022
8cc4a2c
Make a unit test use a username
Emyrk May 17, 2022
67423fb
Add extra assertion to unit test
Emyrk May 17, 2022
ba3f15d
Update ui api call
Emyrk May 17, 2022
3142a22
Fix js test
Emyrk May 18, 2022
c1bb602
Fix js mocks
Emyrk May 18, 2022
32cf21f
Deleted instead of include deleted
Emyrk May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
32 changes: 18 additions & 14 deletions coderd/coderd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
Expand Down Expand Up @@ -289,23 +288,28 @@ func New(options *Options) (http.Handler, func()) {
)
r.Get("/", api.workspaceResource)
})
r.Route("/workspaces/{workspace}", func(r chi.Router) {
r.Route("/workspaces", 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.Get("/", api.workspaces)
r.Route("/{workspace}", func(r chi.Router) {
r.Use(
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) {
Expand Down
12 changes: 10 additions & 2 deletions coderd/coderd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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},
Expand Down Expand Up @@ -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
}
Comment on lines +150 to +157
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Routes were being duplicated with a trailing slash. Just catching dev errors.


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
Expand Down
58 changes: 21 additions & 37 deletions coderd/database/databasefake/databasefake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,27 @@ 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.Deleted && workspace.Deleted {
continue
}
workspaces = append(workspaces, workspace)
}

return workspaces, nil
}

func (q *fakeQuerier) GetWorkspacesByTemplateID(_ context.Context, arg database.GetWorkspacesByTemplateIDParams) ([]database.Workspace, error) {
q.mutex.RLock()
defer q.mutex.RUnlock()
Expand Down Expand Up @@ -486,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()
Expand All @@ -525,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()
Expand Down
3 changes: 1 addition & 2 deletions coderd/database/querier.go
View file
Open in desktop

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

90 changes: 30 additions & 60 deletions coderd/database/queries.sql.go
View file
Open in desktop

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

32 changes: 21 additions & 11 deletions coderd/database/queries/workspaces.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,27 @@ WHERE
LIMIT
1;

-- name: GetWorkspacesByOrganizationID :many
SELECT * FROM workspaces WHERE organization_id = $1 AND deleted = $2;
-- name: GetWorkspacesWithFilter :many
SELECT
*
FROM
workspaces
WHERE
-- Optionally include deleted workspaces
deleted = @deleted
-- 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: GetWorkspacesByOrganizationIDs :many
SELECT * FROM workspaces WHERE organization_id = ANY(@ids :: uuid [ ]) AND deleted = @deleted;
Expand Down Expand Up @@ -37,15 +56,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
*
Expand Down
8 changes: 5 additions & 3 deletions coderd/users_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Owner: codersdk.Me,
})
require.NoError(t, err)
require.Len(t, workspaces, 0)
})
Expand Down Expand Up @@ -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{Owner: 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{Owner: codersdk.Me})
require.NoError(t, err)
require.Len(t, workspaces, 1)
})
Expand Down
Loading