Skip to content

Commit 85b244f

Browse files
committed
fix: harden workspace deletion cleanup
Move PostgreSQL sequence cleanup after the workspace deletion commits so auxiliary DDL failures cannot roll back valid deletions. Remove the unused transactional helper and pin the historical Zammad migration checksums to prevent accidental edits.
1 parent fe48d21 commit 85b244f

3 files changed

Lines changed: 30 additions & 14 deletions

File tree

internal/database/zammad_schema_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,28 @@ func TestZammadTicketLinkMetadataMigrationBackendParity(t *testing.T) {
190190
}
191191
}
192192

193+
func TestZammadCanonicalWorkspaceScopeMigrationChecksumStability(t *testing.T) {
194+
var migration *Migration
195+
for i := range Catalog {
196+
if Catalog[i].Version == "20260901_zammad_canonical_workspace_scope" {
197+
migration = &Catalog[i]
198+
break
199+
}
200+
}
201+
if migration == nil {
202+
t.Fatal("Zammad canonical workspace scope migration is missing")
203+
}
204+
205+
for driver, want := range map[string]string{
206+
driverSQLite: "164fb8cb79ee0f5705aa5891540576995bc030260effc89c75a6de470e45a33f",
207+
driverPostgres: "4d1561e94cff763531c5c68a749f5f32bc3f3aa2feb2493ac7911d697bd450b4",
208+
} {
209+
if got := migration.checksum(driver); got != want {
210+
t.Fatalf("historical %s migration checksum changed: got %s, want %s", driver, got, want)
211+
}
212+
}
213+
}
214+
193215
func TestZammadTicketSyncLockOwnerMigrationBackendParity(t *testing.T) {
194216
var migration *Migration
195217
for i := range Catalog {

internal/repository/workspace_repository.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -730,14 +730,3 @@ func (r *WorkspaceRepository) DropItemSequence(workspaceID int64) error {
730730
_, err := r.db.ExecWrite(fmt.Sprintf(`DROP SEQUENCE IF EXISTS %q`, seqName))
731731
return err
732732
}
733-
734-
// DropItemSequenceTx removes the per-workspace sequence inside the caller's
735-
// transaction. No-op on SQLite.
736-
func (r *WorkspaceRepository) DropItemSequenceTx(tx database.Tx, workspaceID int64) error {
737-
if r.db.GetDriverName() != "postgres" {
738-
return nil
739-
}
740-
seqName := fmt.Sprintf("workspace_%d_item_seq", workspaceID)
741-
_, err := tx.ExecWrite(fmt.Sprintf(`DROP SEQUENCE IF EXISTS %q`, seqName))
742-
return err
743-
}

internal/services/workspace_service.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"database/sql"
66
"errors"
77
"fmt"
8+
"log/slog"
89
"strings"
910
"time"
1011

@@ -376,14 +377,18 @@ func (s *WorkspaceService) Delete(id int) error {
376377
if err := s.repo.DeleteTx(tx, id); err != nil {
377378
return fmt.Errorf("delete workspace: %w", err)
378379
}
379-
if err := s.repo.DropItemSequenceTx(tx, int64(id)); err != nil {
380-
return fmt.Errorf("drop workspace item sequence: %w", err)
381-
}
382380
return nil
383381
}); err != nil {
384382
return err
385383
}
386384

385+
// The sequence is auxiliary cleanup. Run it after the workspace deletion
386+
// commits so a failed DROP cannot roll back an otherwise valid deletion or
387+
// leave a PostgreSQL transaction permanently aborted.
388+
if err := s.repo.DropItemSequence(int64(id)); err != nil {
389+
slog.Warn("failed to drop item sequence for workspace", "workspace_id", id, "error", err)
390+
}
391+
387392
repository.InvalidateItemListCountCache(s.db, id)
388393

389394
return nil

0 commit comments

Comments
 (0)