fix: Sort workspace by name by created_at (#2214)

* fix: Sort workspace by name by created_at

Fix bug where deleting workspaces with the same name returns the
oldest deleted workspace
This commit is contained in:
Steven Masley
2022-06-10 09:58:42 -05:00
committed by GitHub
parent 953e8c8fe6
commit 6bee180bb3
10 changed files with 60 additions and 29 deletions
+2 -2
View File
@@ -49,7 +49,7 @@ func create() *cobra.Command {
workspaceName, err = cliui.Prompt(cmd, cliui.PromptOptions{
Text: "Specify a name for your workspace:",
Validate: func(workspaceName string) error {
_, err = client.WorkspaceByOwnerAndName(cmd.Context(), codersdk.Me, workspaceName, codersdk.WorkspaceByOwnerAndNameParams{})
_, err = client.WorkspaceByOwnerAndName(cmd.Context(), codersdk.Me, workspaceName, codersdk.WorkspaceOptions{})
if err == nil {
return xerrors.Errorf("A workspace already exists named %q!", workspaceName)
}
@@ -61,7 +61,7 @@ func create() *cobra.Command {
}
}
_, err = client.WorkspaceByOwnerAndName(cmd.Context(), codersdk.Me, workspaceName, codersdk.WorkspaceByOwnerAndNameParams{})
_, err = client.WorkspaceByOwnerAndName(cmd.Context(), codersdk.Me, workspaceName, codersdk.WorkspaceOptions{})
if err == nil {
return xerrors.Errorf("A workspace already exists named %q!", workspaceName)
}
+1 -1
View File
@@ -214,7 +214,7 @@ func namedWorkspace(cmd *cobra.Command, client *codersdk.Client, identifier stri
return codersdk.Workspace{}, xerrors.Errorf("invalid workspace name: %q", identifier)
}
return client.WorkspaceByOwnerAndName(cmd.Context(), owner, name, codersdk.WorkspaceByOwnerAndNameParams{})
return client.WorkspaceByOwnerAndName(cmd.Context(), owner, name, codersdk.WorkspaceOptions{})
}
// createConfig consumes the global configuration flag to produce a config root.
+10 -1
View File
@@ -375,7 +375,9 @@ func (q *fakeQuerier) GetWorkspaceByOwnerIDAndName(_ context.Context, arg databa
q.mutex.RLock()
defer q.mutex.RUnlock()
var found *database.Workspace
for _, workspace := range q.workspaces {
workspace := workspace
if workspace.OwnerID != arg.OwnerID {
continue
}
@@ -385,7 +387,14 @@ func (q *fakeQuerier) GetWorkspaceByOwnerIDAndName(_ context.Context, arg databa
if workspace.Deleted != arg.Deleted {
continue
}
return workspace, nil
// Return the most recent workspace with the given name
if found == nil || workspace.CreatedAt.After(found.CreatedAt) {
found = &workspace
}
}
if found != nil {
return *found, nil
}
return database.Workspace{}, sql.ErrNoRows
}
+1
View File
@@ -3480,6 +3480,7 @@ WHERE
owner_id = $1
AND deleted = $2
AND LOWER("name") = LOWER($3)
ORDER BY created_at DESC
`
type GetWorkspaceByOwnerIDAndNameParams struct {
+2 -1
View File
@@ -70,7 +70,8 @@ FROM
WHERE
owner_id = @owner_id
AND deleted = @deleted
AND LOWER("name") = LOWER(@name);
AND LOWER("name") = LOWER(@name)
ORDER BY created_at DESC;
-- name: GetWorkspaceOwnerCountsByTemplateIDs :many
SELECT
+2 -4
View File
@@ -35,10 +35,8 @@ func (api *API) workspace(rw http.ResponseWriter, r *http.Request) {
return
}
// The `deleted` query parameter (which defaults to `false`) MUST match the
// `Deleted` field on the workspace otherwise you will get a 410 Gone.
var (
deletedStr = r.URL.Query().Get("deleted")
deletedStr = r.URL.Query().Get("include_deleted")
showDeleted = false
)
if deletedStr != "" {
@@ -46,7 +44,7 @@ func (api *API) workspace(rw http.ResponseWriter, r *http.Request) {
showDeleted, err = strconv.ParseBool(deletedStr)
if err != nil {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: fmt.Sprintf("Invalid boolean value %q for \"deleted\" query param.", deletedStr),
Message: fmt.Sprintf("Invalid boolean value %q for \"include_deleted\" query param.", deletedStr),
Validations: []httpapi.Error{
{Field: "deleted", Detail: "Must be a valid boolean"},
},
+35 -4
View File
@@ -258,7 +258,7 @@ func TestWorkspaceByOwnerAndName(t *testing.T) {
t.Run("NotFound", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, "something", codersdk.WorkspaceByOwnerAndNameParams{})
_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, "something", codersdk.WorkspaceOptions{})
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode())
@@ -271,7 +271,7 @@ func TestWorkspaceByOwnerAndName(t *testing.T) {
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, workspace.Name, codersdk.WorkspaceByOwnerAndNameParams{})
_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, workspace.Name, codersdk.WorkspaceOptions{})
require.NoError(t, err)
})
t.Run("Deleted", func(t *testing.T) {
@@ -294,12 +294,43 @@ func TestWorkspaceByOwnerAndName(t *testing.T) {
// Then:
// When we call without includes_deleted, we don't expect to get the workspace back
_, err = client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceByOwnerAndNameParams{})
_, err = client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{})
require.ErrorContains(t, err, "403")
// Then:
// When we call with includes_deleted, we should get the workspace back
workspaceNew, err := client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceByOwnerAndNameParams{IncludeDeleted: true})
workspaceNew, err := client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{IncludeDeleted: true})
require.NoError(t, err)
require.Equal(t, workspace.ID, workspaceNew.ID)
// Given:
// We recreate the workspace with the same name
workspace, err = client.CreateWorkspace(context.Background(), user.OrganizationID, codersdk.CreateWorkspaceRequest{
TemplateID: workspace.TemplateID,
Name: workspace.Name,
AutostartSchedule: workspace.AutostartSchedule,
TTLMillis: workspace.TTLMillis,
})
require.NoError(t, err)
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
// Then:
// We can fetch the most recent workspace
workspaceNew, err = client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{})
require.NoError(t, err)
require.Equal(t, workspace.ID, workspaceNew.ID)
// Given:
// We delete the workspace again
build, err = client.CreateWorkspaceBuild(context.Background(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{
Transition: codersdk.WorkspaceTransitionDelete,
})
require.NoError(t, err, "delete the workspace")
coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)
// Then:
// When we fetch the deleted workspace, we get the most recently deleted one
workspaceNew, err = client.WorkspaceByOwnerAndName(context.Background(), workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{IncludeDeleted: true})
require.NoError(t, err)
require.Equal(t, workspace.ID, workspaceNew.ID)
})
+5 -9
View File
@@ -39,7 +39,7 @@ type CreateWorkspaceBuildRequest struct {
}
type WorkspaceOptions struct {
Deleted bool `json:"deleted,omitempty"`
IncludeDeleted bool `json:"include_deleted,omitempty"`
}
// asRequestOption returns a function that can be used in (*Client).Request.
@@ -47,8 +47,8 @@ type WorkspaceOptions struct {
func (o WorkspaceOptions) asRequestOption() requestOption {
return func(r *http.Request) {
q := r.URL.Query()
if o.Deleted {
q.Set("deleted", "true")
if o.IncludeDeleted {
q.Set("include_deleted", "true")
}
r.URL.RawQuery = q.Encode()
}
@@ -62,7 +62,7 @@ func (c *Client) Workspace(ctx context.Context, id uuid.UUID) (Workspace, error)
// DeletedWorkspace returns a single workspace that was deleted.
func (c *Client) DeletedWorkspace(ctx context.Context, id uuid.UUID) (Workspace, error) {
o := WorkspaceOptions{
Deleted: true,
IncludeDeleted: true,
}
return c.getWorkspace(ctx, id, o.asRequestOption())
}
@@ -258,12 +258,8 @@ func (c *Client) Workspaces(ctx context.Context, filter WorkspaceFilter) ([]Work
return workspaces, json.NewDecoder(res.Body).Decode(&workspaces)
}
type WorkspaceByOwnerAndNameParams struct {
IncludeDeleted bool `json:"include_deleted,omitempty"`
}
// WorkspaceByOwnerAndName returns a workspace by the owner's UUID and the workspace's name.
func (c *Client) WorkspaceByOwnerAndName(ctx context.Context, owner string, name string, params WorkspaceByOwnerAndNameParams) (Workspace, error) {
func (c *Client) WorkspaceByOwnerAndName(ctx context.Context, owner string, name string, params WorkspaceOptions) (Workspace, error) {
res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/workspace/%s", owner, name), nil, func(r *http.Request) {
q := r.URL.Query()
q.Set("include_deleted", fmt.Sprintf("%t", params.IncludeDeleted))
+1 -1
View File
@@ -144,7 +144,7 @@ export const getWorkspaces = async (filter?: TypesGen.WorkspaceFilter): Promise<
export const getWorkspaceByOwnerAndName = async (
username = "me",
workspaceName: string,
params?: TypesGen.WorkspaceByOwnerAndNameParams,
params?: TypesGen.WorkspaceOptions,
): Promise<TypesGen.Workspace> => {
const response = await axios.get<TypesGen.Workspace>(`/api/v2/users/${username}/workspace/${workspaceName}`, {
params,
+1 -6
View File
@@ -464,11 +464,6 @@ export interface WorkspaceBuildsRequest extends Pagination {
readonly WorkspaceID: string
}
// From codersdk/workspaces.go:261:6
export interface WorkspaceByOwnerAndNameParams {
readonly include_deleted?: boolean
}
// From codersdk/workspaces.go:219:6
export interface WorkspaceFilter {
readonly organization_id?: string
@@ -478,7 +473,7 @@ export interface WorkspaceFilter {
// From codersdk/workspaces.go:41:6
export interface WorkspaceOptions {
readonly deleted?: boolean
readonly include_deleted?: boolean
}
// From codersdk/workspaceresources.go:21:6