Skip to content

Commit 5ccd7a7

Browse files
committed
fix: Properly handle deletion / affected rows
1 parent 871f4e2 commit 5ccd7a7

File tree

4 files changed

+26
-20
lines changed

4 files changed

+26
-20
lines changed

coderd/database/querier.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/database/queries.sql.go

Lines changed: 9 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/database/queries/workspaces.sql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,13 +112,15 @@ SET
112112
WHERE
113113
id = $1;
114114

115-
-- name: UpdateWorkspace :exec
115+
-- name: UpdateWorkspace :execrows
116116
UPDATE
117117
workspaces
118118
SET
119119
name = $2
120120
WHERE
121121
id = $1
122+
-- This can result in rows affected being zero, the caller should
123+
-- handle this case.
122124
AND deleted = false;
123125

124126
-- name: UpdateWorkspaceAutostart :exec

coderd/workspaces.go

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -494,24 +494,11 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) {
494494
name = req.Name
495495
}
496496

497-
err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{
497+
n, err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{
498498
ID: workspace.ID,
499499
Name: name,
500500
})
501501
if err != nil {
502-
// The query protects against updating deleted workspaces and
503-
// the existence of the workspace is checked in the request,
504-
// the only conclusion we can make is that we're trying to
505-
// update a deleted workspace.
506-
//
507-
// We could do this check earlier but since we're not in a
508-
// transaction, it's pointless.
509-
if errors.Is(err, sql.ErrNoRows) {
510-
httpapi.Write(rw, http.StatusMethodNotAllowed, codersdk.Response{
511-
Message: fmt.Sprintf("Workspace %q is deleted and cannot be updated.", workspace.Name),
512-
})
513-
return
514-
}
515502
// Check if the name was already in use.
516503
if database.IsUniqueViolation(err, database.UniqueWorkspacesOwnerIDLowerIdx) {
517504
httpapi.Write(rw, http.StatusConflict, codersdk.Response{
@@ -529,6 +516,18 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) {
529516
})
530517
return
531518
}
519+
// If no rows were affected, the workspace must have been deleted
520+
// between the start of the request and the query updating the
521+
// workspace name.
522+
//
523+
// We could do this check earlier but we'd have to start a
524+
// transaction to ensure consistency.
525+
if n == 0 {
526+
httpapi.Write(rw, http.StatusMethodNotAllowed, codersdk.Response{
527+
Message: fmt.Sprintf("Workspace %q is deleted and cannot be updated.", workspace.Name),
528+
})
529+
return
530+
}
532531

533532
rw.WriteHeader(http.StatusNoContent)
534533
}

0 commit comments

Comments
 (0)