Skip to content

Commit 256b7ef

Browse files
mafredrimafredri
committed
Revert "fix: Properly handle deletion / affected rows"
This reverts commit 5ccd7a7.
1 parent 9320850 commit 256b7ef

File tree

4 files changed

+20
-26
lines changed

4 files changed

+20
-26
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: 4 additions & 9 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: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,13 @@ SET
112112
WHERE
113113
id = $1;
114114

115-
-- name: UpdateWorkspace :execrows
115+
-- name: UpdateWorkspace :exec
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.
124122
AND deleted = false;
125123

126124
-- name: UpdateWorkspaceAutostart :exec

‎coderd/workspaces.go

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

504-
n, err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{
504+
err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{
505505
ID: workspace.ID,
506506
Name: name,
507507
})
508508
if err != nil {
509+
// The query protects against updating deleted workspaces and
510+
// the existence of the workspace is checked in the request,
511+
// the only conclusion we can make is that we're trying to
512+
// update a deleted workspace.
513+
//
514+
// We could do this check earlier but since we're not in a
515+
// transaction, it's pointless.
516+
if errors.Is(err, sql.ErrNoRows) {
517+
httpapi.Write(rw, http.StatusMethodNotAllowed, codersdk.Response{
518+
Message: fmt.Sprintf("Workspace %q is deleted and cannot be updated.", workspace.Name),
519+
})
520+
return
521+
}
509522
// Check if the name was already in use.
510523
if database.IsUniqueViolation(err, database.UniqueWorkspacesOwnerIDLowerIdx) {
511524
httpapi.Write(rw, http.StatusConflict, codersdk.Response{
@@ -523,18 +536,6 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) {
523536
})
524537
return
525538
}
526-
// If no rows were affected, the workspace must have been deleted
527-
// between the start of the request and the query updating the
528-
// workspace name.
529-
//
530-
// We could do this check earlier but we'd have to start a
531-
// transaction to ensure consistency.
532-
if n == 0 {
533-
httpapi.Write(rw, http.StatusMethodNotAllowed, codersdk.Response{
534-
Message: fmt.Sprintf("Workspace %q is deleted and cannot be updated.", workspace.Name),
535-
})
536-
return
537-
}
538539

539540
rw.WriteHeader(http.StatusNoContent)
540541
}

0 commit comments

Comments
 (0)