Skip to content

Commit 6eb7d1f

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

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
@@ -494,11 +494,24 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) {
494494
name = req.Name
495495
}
496496

497-
n, err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{
497+
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+
}
502515
// Check if the name was already in use.
503516
if database.IsUniqueViolation(err, database.UniqueWorkspacesOwnerIDLowerIdx) {
504517
httpapi.Write(rw, http.StatusConflict, codersdk.Response{
@@ -516,18 +529,6 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) {
516529
})
517530
return
518531
}
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-
}
531532

532533
rw.WriteHeader(http.StatusNoContent)
533534
}

0 commit comments

Comments
 (0)