Allow recreating soft-deleted repos
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
5df8d85b84476e9427af97303ddf50885ec82cd3- Parents
-
cabd341 - Tree
e31b3dc
5df8d85
5df8d85b84476e9427af97303ddf50885ec82cd3cabd341
e31b3dc| Status | File | + | - |
|---|---|---|---|
| M |
docs/internal/repo-create.md
|
5 | 2 |
| M |
docs/internal/repo-lifecycle.md
|
17 | 8 |
| M |
internal/infra/storage/reposfs.go
|
19 | 0 |
| A |
internal/migrationsfs/migrations/0058_repo_name_reuse_after_soft_delete.sql
|
32 | 0 |
| M |
internal/repos/create.go
|
77 | 2 |
| M |
internal/repos/create_test.go
|
97 | 0 |
| M |
internal/repos/lifecycle/hard_delete.go
|
54 | 31 |
| M |
internal/repos/lifecycle/lifecycle_test.go
|
52 | 3 |
| A |
internal/repos/lifecycle/paths.go
|
102 | 0 |
| M |
internal/repos/lifecycle/soft_delete.go
|
149 | 6 |
| M |
internal/repos/queries/repos.sql
|
33 | 0 |
| M |
internal/repos/sqlc/querier.go
|
7 | 0 |
| M |
internal/repos/sqlc/repos.sql.go
|
123 | 0 |
docs/internal/repo-create.mdmodified@@ -4,7 +4,7 @@ S11 ships the create-a-repo flow end-to-end: a logged-in user clicks **New**, fi | ||
| 4 | 4 | |
| 5 | 5 | ## What's wired |
| 6 | 6 | |
| 7 | -- **Migration:** `0017_repos.sql` adds the `repos` table (with `repo_visibility` enum, owner XOR check, per-owner unique-by-name partial indexes, soft-delete column). | |
| 7 | +- **Migration:** `0017_repos.sql` adds the `repos` table (with `repo_visibility` enum, owner XOR check, per-owner unique-by-name partial indexes, soft-delete column). `0058_repo_name_reuse_after_soft_delete.sql` narrows those uniqueness indexes to active repos so deleted names can be reused. | |
| 8 | 8 | - **Source remotes:** `0052_repo_source_remotes.sql` adds one optional public fetch URL per repo. Creation and settings can save this URL, fetch heads/tags, and use it later for submodule gitlink backfill. |
| 9 | 9 | - **sqlc package:** `internal/repos/sqlc` (`reposdb`) — Create, Get-by-owner-and-name, Exists, List-by-owner, Count, SoftDelete, UpdateDiskUsed. |
| 10 | 10 | - `internal/repos/validate.go` — name shape (≤100 chars, `[a-z0-9._-]`, non-separator edges, no dot-dot, no leading dot) + reserved-name list. |
@@ -52,8 +52,11 @@ POST /new | ||
| 52 | 52 | │ (refuse with ErrNoVerifiedEmail when init is requested AND missing) |
| 53 | 53 | ├─ RepoFS.RepoPath(owner, name) → defense-in-depth path validation |
| 54 | 54 | ├─ tx.Begin() |
| 55 | + │ ├─ LockRepoOwnerName(owner/name) advisory lock | |
| 55 | 56 | │ └─ reposdb.CreateRepo(...) ← unique-violation surfaces as ErrTaken |
| 56 | 57 | ├─ RepoFS.InitBare(diskPath) ← `git init --bare --initial-branch=trunk` |
| 58 | + │ └─ if a legacy soft-deleted repo still occupies diskPath: | |
| 59 | + │ move it to `.deleted/<old-repo-id>.git` and retry | |
| 57 | 60 | ├─ if init flag set: |
| 58 | 61 | │ buildInitialCommit(ic) → commit OID |
| 59 | 62 | │ (hash-object → update-index → write-tree → commit-tree → update-ref) |
@@ -70,7 +73,7 @@ POST /new | ||
| 70 | 73 | Failure handling at each step: |
| 71 | 74 | |
| 72 | 75 | - DB insert error: tx already rolled back via the deferred Rollback closure; nothing on disk to clean. |
| 73 | -- FS InitBare error: tx still uncommitted (we Rollback via defer); best-effort `os.RemoveAll(diskPath)` clears any partially-mkdir'd directory. | |
| 76 | +- FS InitBare error: tx still uncommitted (we Rollback via defer); best-effort `os.RemoveAll(diskPath)` clears any partially-mkdir'd directory. `storage.ErrAlreadyExists` is not blindly removed because it can be a legacy soft-deleted repo path; create first tries to displace that path to the deleted tombstone. | |
| 74 | 77 | - Initial-commit error: same as above — Rollback + RemoveAll. |
| 75 | 78 | - tx.Commit error: post-FS-success but DB couldn't commit. We RemoveAll the bare repo dir to keep DB and disk consistent. |
| 76 | 79 | - Audit error: logged at WARN, not propagated — we don't fail the create just because audit logging blipped. |
docs/internal/repo-lifecycle.mdmodified@@ -86,12 +86,18 @@ data layer; visibility flips don't cascade. | ||
| 86 | 86 | |
| 87 | 87 | * **Soft delete**: `POST /{owner}/{repo}/settings/delete` → |
| 88 | 88 | `lifecycle.SoftDelete`. Sets `deleted_at = now()`. Repo disappears |
| 89 | - from listings, profile pinned slot, search. `/{owner}/{repo}` 404s | |
| 90 | - for non-owners (auth-aware via S15 policy's `DenyRepoDeleted`). | |
| 89 | + from listings, profile pinned slot, search. The bare repo is moved | |
| 90 | + from the canonical `<owner>/<name>.git` path to an internal | |
| 91 | + `.deleted/<repo-id>.git` tombstone so a fresh repo can reuse the same | |
| 92 | + owner/name during the grace window. `/{owner}/{repo}` 404s for | |
| 93 | + non-owners (auth-aware via S15 policy's `DenyRepoDeleted`). | |
| 91 | 94 | * **Restore**: owner sees the soft-deleted repo at |
| 92 | 95 | `/settings/repositories`. POST to |
| 93 | - `/settings/repositories/restore/{id}` clears `deleted_at`. Past the | |
| 94 | - 7-day grace, restore returns `ErrPastGrace` (410). | |
| 96 | + `/settings/repositories/restore/{id}` moves the tombstone back to the | |
| 97 | + canonical path and clears `deleted_at`. If the owner/name was reused, | |
| 98 | + restore returns `ErrNameTaken` and leaves the deleted repo in the | |
| 99 | + restore list. Past the 7-day grace, restore returns `ErrPastGrace` | |
| 100 | + (410). | |
| 95 | 101 | * **Hard delete**: `lifecycle:sweep` worker job (registered in |
| 96 | 102 | `cmd/shithubd/worker.go`) runs periodically. The handler: |
| 97 | 103 | 1. `ListRepoIDsPastSoftDeleteGrace` — finds rows past 7 days. |
@@ -101,7 +107,9 @@ data layer; visibility flips don't cascade. | ||
| 101 | 107 | `push_events`, `repo_collaborators`, `repo_redirects` (rows |
| 102 | 108 | pointing at this repo), `repo_transfer_requests`, and |
| 103 | 109 | `webhook_events_pending`. |
| 104 | - * `RemoveAll` the bare repo on disk. | |
| 110 | + * Remove the tombstoned bare repo on disk. Legacy soft-deleted | |
| 111 | + repos whose bare data still sits at the canonical path are | |
| 112 | + removed only when no active repo has reused the owner/name. | |
| 105 | 113 | * Audit `repo_hard_deleted` with the row snapshot in `meta`, |
| 106 | 114 | since the repo_id no longer resolves. |
| 107 | 115 | 3. The same job also flips pending transfers past their TTL via |
@@ -129,9 +137,10 @@ from "redirected" to "never existed." | ||
| 129 | 137 | INSERT INTO jobs (kind, payload) VALUES ('lifecycle:sweep', '{}'::jsonb); |
| 130 | 138 | NOTIFY shithub_jobs; |
| 131 | 139 | ``` |
| 132 | -* Soft-deleted repos take their disk path with them — the bare repo | |
| 133 | - on disk stays for the full grace window. If disk pressure becomes | |
| 134 | - an issue, configure the grace window down (it's a constant in | |
| 140 | +* Soft-deleted repos keep their bare data for the full grace window, | |
| 141 | + but under the owner-local `.deleted/<repo-id>.git` tombstone path | |
| 142 | + rather than the active canonical path. If disk pressure becomes an | |
| 143 | + issue, configure the grace window down (it's a constant in | |
| 135 | 144 | `lifecycle.go::softDeleteGrace`; promote to config in S37 if needed). |
| 136 | 145 | * Renaming a repo doesn't require restarting any worker or hook. |
| 137 | 146 | The atomic FS move + DB update means the next hook invocation will |
internal/infra/storage/reposfs.gomodified@@ -119,6 +119,25 @@ func (r *RepoFS) RepoPath(owner, name string) (string, error) { | ||
| 119 | 119 | return p, nil |
| 120 | 120 | } |
| 121 | 121 | |
| 122 | +// DeletedRepoPath returns the internal tombstone path used while a | |
| 123 | +// soft-deleted repo is inside its restore grace window. Keeping | |
| 124 | +// tombstones outside the canonical <owner>/<name>.git path lets a new | |
| 125 | +// active repo reuse the name without losing the old row's restore data. | |
| 126 | +func (r *RepoFS) DeletedRepoPath(owner, name string, repoID int64) (string, error) { | |
| 127 | + if repoID <= 0 { | |
| 128 | + return "", fmt.Errorf("%w: repo id required", ErrInvalidPath) | |
| 129 | + } | |
| 130 | + canonical, err := r.RepoPath(owner, name) | |
| 131 | + if err != nil { | |
| 132 | + return "", err | |
| 133 | + } | |
| 134 | + p := filepath.Join(filepath.Dir(canonical), ".deleted", fmt.Sprintf("%d.git", repoID)) | |
| 135 | + if err := r.containedInRoot(p); err != nil { | |
| 136 | + return "", err | |
| 137 | + } | |
| 138 | + return p, nil | |
| 139 | +} | |
| 140 | + | |
| 122 | 141 | // containedInRoot returns ErrEscapesRoot when p does not resolve under r.root. |
| 123 | 142 | // Defense-in-depth: validateName already rejects ".." and absolute paths, |
| 124 | 143 | // but a future caller might compose paths differently. |
internal/migrationsfs/migrations/0058_repo_name_reuse_after_soft_delete.sqladded@@ -0,0 +1,32 @@ | ||
| 1 | +-- SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | +-- | |
| 3 | +-- Soft-deleted repos must no longer reserve their owner/name forever. | |
| 4 | +-- Runtime lifecycle code moves the bare repo to a tombstone path so | |
| 5 | +-- the canonical on-disk path can be reused by a fresh repo. | |
| 6 | + | |
| 7 | +-- +goose Up | |
| 8 | +DROP INDEX IF EXISTS repos_owner_user_name_idx; | |
| 9 | +DROP INDEX IF EXISTS repos_owner_org_name_idx; | |
| 10 | + | |
| 11 | +CREATE UNIQUE INDEX repos_owner_user_name_idx | |
| 12 | + ON repos (owner_user_id, name) | |
| 13 | + WHERE owner_user_id IS NOT NULL AND deleted_at IS NULL; | |
| 14 | + | |
| 15 | +CREATE UNIQUE INDEX repos_owner_org_name_idx | |
| 16 | + ON repos (owner_org_id, name) | |
| 17 | + WHERE owner_org_id IS NOT NULL AND deleted_at IS NULL; | |
| 18 | + | |
| 19 | +-- +goose Down | |
| 20 | +DROP INDEX IF EXISTS repos_owner_user_name_idx; | |
| 21 | +DROP INDEX IF EXISTS repos_owner_org_name_idx; | |
| 22 | + | |
| 23 | +-- This rollback can fail if a name has been reused while the prior row | |
| 24 | +-- remains soft-deleted. That is intentional: reverting the old invariant | |
| 25 | +-- requires resolving those duplicates first. | |
| 26 | +CREATE UNIQUE INDEX repos_owner_user_name_idx | |
| 27 | + ON repos (owner_user_id, name) | |
| 28 | + WHERE owner_user_id IS NOT NULL; | |
| 29 | + | |
| 30 | +CREATE UNIQUE INDEX repos_owner_org_name_idx | |
| 31 | + ON repos (owner_org_id, name) | |
| 32 | + WHERE owner_org_id IS NOT NULL; | |
internal/repos/create.gomodified@@ -11,6 +11,7 @@ import ( | ||
| 11 | 11 | "strings" |
| 12 | 12 | "time" |
| 13 | 13 | |
| 14 | + "github.com/jackc/pgx/v5" | |
| 14 | 15 | "github.com/jackc/pgx/v5/pgconn" |
| 15 | 16 | "github.com/jackc/pgx/v5/pgtype" |
| 16 | 17 | "github.com/jackc/pgx/v5/pgxpool" |
@@ -192,6 +193,13 @@ func Create(ctx context.Context, deps Deps, p Params) (Result, error) { | ||
| 192 | 193 | }() |
| 193 | 194 | |
| 194 | 195 | q := reposdb.New() |
| 196 | + lockKey, err := createRepoNameLockKey(p) | |
| 197 | + if err != nil { | |
| 198 | + return Result{}, err | |
| 199 | + } | |
| 200 | + if err := q.LockRepoOwnerName(ctx, tx, lockKey); err != nil { | |
| 201 | + return Result{}, fmt.Errorf("repos: lock owner/name: %w", err) | |
| 202 | + } | |
| 195 | 203 | row, err := q.CreateRepo(ctx, tx, reposdb.CreateRepoParams{ |
| 196 | 204 | OwnerUserID: pgtype.Int8{Int64: p.OwnerUserID, Valid: p.OwnerUserID != 0}, |
| 197 | 205 | OwnerOrgID: pgtype.Int8{Int64: p.OwnerOrgID, Valid: p.OwnerOrgID != 0}, |
@@ -213,8 +221,21 @@ func Create(ctx context.Context, deps Deps, p Params) (Result, error) { | ||
| 213 | 221 | // reverses the row; we also best-effort RemoveAll the directory in |
| 214 | 222 | // case it got partially created. |
| 215 | 223 | if err := deps.RepoFS.InitBare(ctx, diskPath); err != nil { |
| 216 | - _ = os.RemoveAll(diskPath) | |
| 217 | - return Result{}, fmt.Errorf("repos: init bare: %w", err) | |
| 224 | + if errors.Is(err, storage.ErrAlreadyExists) { | |
| 225 | + displaced, displaceErr := displaceDeletedRepoPath(ctx, deps, q, tx, p, ownerSlug, diskPath) | |
| 226 | + if displaceErr != nil { | |
| 227 | + return Result{}, fmt.Errorf("repos: reclaim deleted repo path: %w", displaceErr) | |
| 228 | + } | |
| 229 | + if displaced { | |
| 230 | + err = deps.RepoFS.InitBare(ctx, diskPath) | |
| 231 | + } | |
| 232 | + } | |
| 233 | + if err != nil { | |
| 234 | + if !errors.Is(err, storage.ErrAlreadyExists) { | |
| 235 | + _ = os.RemoveAll(diskPath) | |
| 236 | + } | |
| 237 | + return Result{}, fmt.Errorf("repos: init bare: %w", err) | |
| 238 | + } | |
| 218 | 239 | } |
| 219 | 240 | |
| 220 | 241 | // Install push-pipeline hooks. Skipped when ShithubdPath is empty |
@@ -362,6 +383,60 @@ func resolveAuthor(ctx context.Context, pool *pgxpool.Pool, userID int64) (name, | ||
| 362 | 383 | return display, string(em.Email), nil |
| 363 | 384 | } |
| 364 | 385 | |
| 386 | +func createRepoNameLockKey(p Params) (string, error) { | |
| 387 | + name := strings.ToLower(p.Name) | |
| 388 | + switch { | |
| 389 | + case p.OwnerUserID != 0 && p.OwnerOrgID == 0: | |
| 390 | + return fmt.Sprintf("repo-name:user:%d:%s", p.OwnerUserID, name), nil | |
| 391 | + case p.OwnerOrgID != 0 && p.OwnerUserID == 0: | |
| 392 | + return fmt.Sprintf("repo-name:org:%d:%s", p.OwnerOrgID, name), nil | |
| 393 | + default: | |
| 394 | + return "", errors.New("repos: owner is XOR — set OwnerUserID OR OwnerOrgID, not both") | |
| 395 | + } | |
| 396 | +} | |
| 397 | + | |
| 398 | +func displaceDeletedRepoPath( | |
| 399 | + ctx context.Context, | |
| 400 | + deps Deps, | |
| 401 | + q *reposdb.Queries, | |
| 402 | + db reposdb.DBTX, | |
| 403 | + p Params, | |
| 404 | + ownerSlug string, | |
| 405 | + diskPath string, | |
| 406 | +) (bool, error) { | |
| 407 | + deleted, err := softDeletedRepoForCreate(ctx, q, db, p) | |
| 408 | + if errors.Is(err, pgx.ErrNoRows) { | |
| 409 | + return false, nil | |
| 410 | + } | |
| 411 | + if err != nil { | |
| 412 | + return false, err | |
| 413 | + } | |
| 414 | + deletedPath, err := deps.RepoFS.DeletedRepoPath(ownerSlug, p.Name, deleted.ID) | |
| 415 | + if err != nil { | |
| 416 | + return false, err | |
| 417 | + } | |
| 418 | + if err := deps.RepoFS.Move(diskPath, deletedPath); err != nil { | |
| 419 | + if errors.Is(err, os.ErrNotExist) { | |
| 420 | + return false, nil | |
| 421 | + } | |
| 422 | + return false, err | |
| 423 | + } | |
| 424 | + return true, nil | |
| 425 | +} | |
| 426 | + | |
| 427 | +func softDeletedRepoForCreate(ctx context.Context, q *reposdb.Queries, db reposdb.DBTX, p Params) (reposdb.Repo, error) { | |
| 428 | + if p.OwnerUserID != 0 { | |
| 429 | + return q.GetSoftDeletedRepoByOwnerUserAndName(ctx, db, reposdb.GetSoftDeletedRepoByOwnerUserAndNameParams{ | |
| 430 | + OwnerUserID: pgtype.Int8{Int64: p.OwnerUserID, Valid: true}, | |
| 431 | + Name: p.Name, | |
| 432 | + }) | |
| 433 | + } | |
| 434 | + return q.GetSoftDeletedRepoByOwnerOrgAndName(ctx, db, reposdb.GetSoftDeletedRepoByOwnerOrgAndNameParams{ | |
| 435 | + OwnerOrgID: pgtype.Int8{Int64: p.OwnerOrgID, Valid: true}, | |
| 436 | + Name: p.Name, | |
| 437 | + }) | |
| 438 | +} | |
| 439 | + | |
| 365 | 440 | // isUniqueViolation matches Postgres SQLSTATE 23505. Used to surface |
| 366 | 441 | // the friendly "name taken" error from the unique-by-owner-and-name |
| 367 | 442 | // indexes when the pre-check raced. |
internal/repos/create_test.gomodified@@ -8,6 +8,7 @@ import ( | ||
| 8 | 8 | "fmt" |
| 9 | 9 | "io" |
| 10 | 10 | "log/slog" |
| 11 | + "os" | |
| 11 | 12 | "os/exec" |
| 12 | 13 | "path/filepath" |
| 13 | 14 | "strings" |
@@ -22,6 +23,7 @@ import ( | ||
| 22 | 23 | "github.com/tenseleyFlow/shithub/internal/infra/storage" |
| 23 | 24 | "github.com/tenseleyFlow/shithub/internal/orgs" |
| 24 | 25 | "github.com/tenseleyFlow/shithub/internal/repos" |
| 26 | + "github.com/tenseleyFlow/shithub/internal/repos/lifecycle" | |
| 25 | 27 | "github.com/tenseleyFlow/shithub/internal/testing/dbtest" |
| 26 | 28 | usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc" |
| 27 | 29 | ) |
@@ -175,6 +177,101 @@ func TestCreate_RejectsDuplicate(t *testing.T) { | ||
| 175 | 177 | } |
| 176 | 178 | } |
| 177 | 179 | |
| 180 | +func TestCreate_ReusesSoftDeletedRepoName(t *testing.T) { | |
| 181 | + t.Parallel() | |
| 182 | + pool, deps, uid, uname, _ := setupCreateEnv(t) | |
| 183 | + ctx := context.Background() | |
| 184 | + first, err := repos.Create(ctx, deps, repos.Params{ | |
| 185 | + OwnerUserID: uid, OwnerUsername: uname, Name: "reuse", Visibility: "public", | |
| 186 | + InitReadme: true, | |
| 187 | + }) | |
| 188 | + if err != nil { | |
| 189 | + t.Fatalf("first create: %v", err) | |
| 190 | + } | |
| 191 | + deletedPath, err := deps.RepoFS.DeletedRepoPath(uname, "reuse", first.Repo.ID) | |
| 192 | + if err != nil { | |
| 193 | + t.Fatalf("DeletedRepoPath: %v", err) | |
| 194 | + } | |
| 195 | + ldeps := lifecycle.Deps{Pool: pool, RepoFS: deps.RepoFS, Audit: audit.NewRecorder(), Logger: deps.Logger} | |
| 196 | + if err := lifecycle.SoftDelete(ctx, ldeps, uid, first.Repo.ID); err != nil { | |
| 197 | + t.Fatalf("SoftDelete: %v", err) | |
| 198 | + } | |
| 199 | + if _, err := os.Stat(first.DiskPath); !os.IsNotExist(err) { | |
| 200 | + t.Fatalf("canonical path after soft-delete: err = %v, want not exist", err) | |
| 201 | + } | |
| 202 | + if _, err := os.Stat(deletedPath); err != nil { | |
| 203 | + t.Fatalf("deleted tombstone missing: %v", err) | |
| 204 | + } | |
| 205 | + | |
| 206 | + second, err := repos.Create(ctx, deps, repos.Params{ | |
| 207 | + OwnerUserID: uid, OwnerUsername: uname, Name: "reuse", Visibility: "public", | |
| 208 | + }) | |
| 209 | + if err != nil { | |
| 210 | + t.Fatalf("recreate: %v", err) | |
| 211 | + } | |
| 212 | + if second.Repo.ID == first.Repo.ID { | |
| 213 | + t.Fatalf("recreate reused old repo id %d", second.Repo.ID) | |
| 214 | + } | |
| 215 | + if _, err := os.Stat(second.DiskPath); err != nil { | |
| 216 | + t.Fatalf("replacement canonical path missing: %v", err) | |
| 217 | + } | |
| 218 | + | |
| 219 | + if _, err := pool.Exec(ctx, | |
| 220 | + `UPDATE repos SET deleted_at = now() - interval '8 days' WHERE id = $1`, first.Repo.ID); err != nil { | |
| 221 | + t.Fatalf("backdate deleted repo: %v", err) | |
| 222 | + } | |
| 223 | + if err := lifecycle.HardDelete(ctx, ldeps, 0, first.Repo.ID); err != nil { | |
| 224 | + t.Fatalf("HardDelete old repo: %v", err) | |
| 225 | + } | |
| 226 | + if _, err := os.Stat(second.DiskPath); err != nil { | |
| 227 | + t.Fatalf("replacement path should survive hard-delete of old repo: %v", err) | |
| 228 | + } | |
| 229 | +} | |
| 230 | + | |
| 231 | +func TestCreate_DisplacesLegacySoftDeletedOrgRepoPath(t *testing.T) { | |
| 232 | + t.Parallel() | |
| 233 | + pool, deps, uid, _, _ := setupCreateEnv(t) | |
| 234 | + ctx := context.Background() | |
| 235 | + org, err := orgs.Create(ctx, orgs.Deps{Pool: pool}, orgs.CreateParams{ | |
| 236 | + Slug: "gardesk", DisplayName: "gardesk", CreatedByUserID: uid, | |
| 237 | + }) | |
| 238 | + if err != nil { | |
| 239 | + t.Fatalf("orgs.Create: %v", err) | |
| 240 | + } | |
| 241 | + first, err := repos.Create(ctx, deps, repos.Params{ | |
| 242 | + OwnerOrgID: org.ID, OwnerSlug: string(org.Slug), ActorUserID: uid, | |
| 243 | + Name: "garterm", Visibility: "public", InitReadme: true, | |
| 244 | + }) | |
| 245 | + if err != nil { | |
| 246 | + t.Fatalf("first create: %v", err) | |
| 247 | + } | |
| 248 | + if _, err := pool.Exec(ctx, | |
| 249 | + `UPDATE repos SET deleted_at = now(), updated_at = now() WHERE id = $1`, first.Repo.ID); err != nil { | |
| 250 | + t.Fatalf("legacy soft delete: %v", err) | |
| 251 | + } | |
| 252 | + | |
| 253 | + second, err := repos.Create(ctx, deps, repos.Params{ | |
| 254 | + OwnerOrgID: org.ID, OwnerSlug: string(org.Slug), ActorUserID: uid, | |
| 255 | + Name: "garterm", Visibility: "public", | |
| 256 | + }) | |
| 257 | + if err != nil { | |
| 258 | + t.Fatalf("recreate after legacy soft delete: %v", err) | |
| 259 | + } | |
| 260 | + if second.Repo.ID == first.Repo.ID { | |
| 261 | + t.Fatalf("recreate reused old repo id %d", second.Repo.ID) | |
| 262 | + } | |
| 263 | + deletedPath, err := deps.RepoFS.DeletedRepoPath(string(org.Slug), "garterm", first.Repo.ID) | |
| 264 | + if err != nil { | |
| 265 | + t.Fatalf("DeletedRepoPath: %v", err) | |
| 266 | + } | |
| 267 | + if _, err := os.Stat(deletedPath); err != nil { | |
| 268 | + t.Fatalf("legacy repo was not moved to tombstone: %v", err) | |
| 269 | + } | |
| 270 | + if _, err := os.Stat(second.DiskPath); err != nil { | |
| 271 | + t.Fatalf("replacement canonical path missing: %v", err) | |
| 272 | + } | |
| 273 | +} | |
| 274 | + | |
| 178 | 275 | func TestCreate_RejectsReservedName(t *testing.T) { |
| 179 | 276 | t.Parallel() |
| 180 | 277 | _, deps, uid, uname, _ := setupCreateEnv(t) |
internal/repos/lifecycle/hard_delete.gomodified@@ -5,13 +5,11 @@ package lifecycle | ||
| 5 | 5 | import ( |
| 6 | 6 | "context" |
| 7 | 7 | "fmt" |
| 8 | - "os" | |
| 9 | 8 | |
| 10 | 9 | "github.com/jackc/pgx/v5/pgtype" |
| 11 | 10 | |
| 12 | 11 | "github.com/tenseleyFlow/shithub/internal/auth/audit" |
| 13 | 12 | reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc" |
| 14 | - usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc" | |
| 15 | 13 | ) |
| 16 | 14 | |
| 17 | 15 | // HardDelete is the worker-driven cascade that runs after the soft- |
@@ -26,7 +24,9 @@ import ( | ||
| 26 | 24 | // 3. DELETE FROM repos — FK cascades handle push_events, |
| 27 | 25 | // webhook_events_pending, repo_collaborators, transfer requests, |
| 28 | 26 | // and any redirect rows pointing at this id. |
| 29 | -// 4. RemoveAll the bare repo on disk. | |
| 27 | +// 4. Remove the bare repo tombstone on disk. For legacy soft-deleted | |
| 28 | +// rows that still occupy the canonical path, remove that path only | |
| 29 | +// when no new active repo has reused the owner/name. | |
| 30 | 30 | // 5. Audit-log the hard-delete with a snapshot of the removed row in |
| 31 | 31 | // the meta payload so the audit row is self-contained even after |
| 32 | 32 | // the repo_id is gone. |
@@ -37,11 +37,24 @@ import ( | ||
| 37 | 37 | // id encoded in some way; pass 0 to record "system" in audit. |
| 38 | 38 | func HardDelete(ctx context.Context, deps Deps, actorUserID, repoID int64) error { |
| 39 | 39 | rq := reposdb.New() |
| 40 | - uq := usersdb.New() | |
| 41 | - repo, err := rq.GetRepoByID(ctx, deps.Pool, repoID) | |
| 40 | + tx, err := deps.Pool.Begin(ctx) | |
| 41 | + if err != nil { | |
| 42 | + return fmt.Errorf("begin: %w", err) | |
| 43 | + } | |
| 44 | + committed := false | |
| 45 | + defer func() { | |
| 46 | + if !committed { | |
| 47 | + _ = tx.Rollback(ctx) | |
| 48 | + } | |
| 49 | + }() | |
| 50 | + | |
| 51 | + repo, err := rq.GetRepoByID(ctx, tx, repoID) | |
| 42 | 52 | if err != nil { |
| 43 | 53 | return fmt.Errorf("load repo: %w", err) |
| 44 | 54 | } |
| 55 | + if err := lockRepoName(ctx, rq, tx, repo); err != nil { | |
| 56 | + return err | |
| 57 | + } | |
| 45 | 58 | if !repo.DeletedAt.Valid { |
| 46 | 59 | return ErrNotDeleted |
| 47 | 60 | } |
@@ -61,23 +74,17 @@ func HardDelete(ctx context.Context, deps Deps, actorUserID, repoID int64) error | ||
| 61 | 74 | if repo.OwnerUserID.Valid { |
| 62 | 75 | snapshot["owner_user_id"] = repo.OwnerUserID.Int64 |
| 63 | 76 | } |
| 64 | - ownerName := "" | |
| 65 | - if repo.OwnerUserID.Valid { | |
| 66 | - if u, err := uq.GetUserByID(ctx, deps.Pool, repo.OwnerUserID.Int64); err == nil { | |
| 67 | - ownerName = u.Username | |
| 68 | - } | |
| 77 | + if repo.OwnerOrgID.Valid { | |
| 78 | + snapshot["owner_org_id"] = repo.OwnerOrgID.Int64 | |
| 69 | 79 | } |
| 70 | - | |
| 71 | - tx, err := deps.Pool.Begin(ctx) | |
| 80 | + if ownerSlug, err := ownerSlugForRepo(ctx, deps, repo); err == nil { | |
| 81 | + snapshot["owner"] = ownerSlug | |
| 82 | + } | |
| 83 | + paths, pathErr := diskPathsForRepo(ctx, deps, repo) | |
| 84 | + activeNameExists, err := activeRepoNameExists(ctx, rq, tx, repo) | |
| 72 | 85 | if err != nil { |
| 73 | - return fmt.Errorf("begin: %w", err) | |
| 86 | + return fmt.Errorf("hard-delete name check: %w", err) | |
| 74 | 87 | } |
| 75 | - committed := false | |
| 76 | - defer func() { | |
| 77 | - if !committed { | |
| 78 | - _ = tx.Rollback(ctx) | |
| 79 | - } | |
| 80 | - }() | |
| 81 | 88 | |
| 82 | 89 | // 1. Orphan child forks. |
| 83 | 90 | if _, err := rq.OrphanForksOf(ctx, tx, pgtype.Int8{Int64: repoID, Valid: true}); err != nil { |
@@ -93,23 +100,25 @@ func HardDelete(ctx context.Context, deps Deps, actorUserID, repoID int64) error | ||
| 93 | 100 | if err := rq.HardDeleteRepo(ctx, tx, repoID); err != nil { |
| 94 | 101 | return fmt.Errorf("delete repo: %w", err) |
| 95 | 102 | } |
| 103 | + | |
| 104 | + // 4. Remove the bare repo while the owner/name advisory lock is | |
| 105 | + // still held. That matters for legacy soft-deleted rows whose data | |
| 106 | + // is still at the canonical path: a concurrent recreate cannot race | |
| 107 | + // this cleanup and have its new bare repo removed. | |
| 108 | + if pathErr == nil { | |
| 109 | + if err := removeDeletedRepoPath(deps, paths, activeNameExists); err != nil && deps.Logger != nil { | |
| 110 | + deps.Logger.WarnContext(ctx, "hard delete: fs delete failed", | |
| 111 | + "repo_id", repoID, "path", paths.deleted, "canonical_path", paths.canonical, "error", err) | |
| 112 | + } | |
| 113 | + } else if deps.Logger != nil { | |
| 114 | + deps.Logger.WarnContext(ctx, "hard delete: compute fs path failed", "repo_id", repoID, "error", pathErr) | |
| 115 | + } | |
| 116 | + | |
| 96 | 117 | if err := tx.Commit(ctx); err != nil { |
| 97 | 118 | return fmt.Errorf("commit: %w", err) |
| 98 | 119 | } |
| 99 | 120 | committed = true |
| 100 | 121 | |
| 101 | - // 4. Remove the bare repo on disk. Best-effort: log and continue | |
| 102 | - // so a missing path doesn't leave a "deleted but DB row gone" | |
| 103 | - // inconsistency that's impossible to recover from. | |
| 104 | - if ownerName != "" { | |
| 105 | - if path, err := deps.RepoFS.RepoPath(ownerName, repo.Name); err == nil { | |
| 106 | - if err := os.RemoveAll(path); err != nil && deps.Logger != nil { | |
| 107 | - deps.Logger.WarnContext(ctx, "hard delete: fs RemoveAll failed", | |
| 108 | - "repo_id", repoID, "path", path, "error", err) | |
| 109 | - } | |
| 110 | - } | |
| 111 | - } | |
| 112 | - | |
| 113 | 122 | // 5. Audit. Use a fresh pool conn since the repo_id no longer |
| 114 | 123 | // exists; the meta blob carries the snapshot. |
| 115 | 124 | if deps.Audit != nil { |
@@ -118,3 +127,17 @@ func HardDelete(ctx context.Context, deps Deps, actorUserID, repoID int64) error | ||
| 118 | 127 | } |
| 119 | 128 | return nil |
| 120 | 129 | } |
| 130 | + | |
| 131 | +func removeDeletedRepoPath(deps Deps, paths repoDiskPaths, activeNameExists bool) error { | |
| 132 | + deletedExists, err := deps.RepoFS.Exists(paths.deleted) | |
| 133 | + if err != nil { | |
| 134 | + return err | |
| 135 | + } | |
| 136 | + if deletedExists { | |
| 137 | + return deps.RepoFS.Delete(paths.deleted) | |
| 138 | + } | |
| 139 | + if activeNameExists { | |
| 140 | + return nil | |
| 141 | + } | |
| 142 | + return deps.RepoFS.Delete(paths.canonical) | |
| 143 | +} | |
internal/repos/lifecycle/lifecycle_test.gomodified@@ -196,20 +196,65 @@ func TestSetVisibility(t *testing.T) { | ||
| 196 | 196 | func TestSoftDeleteAndRestore(t *testing.T) { |
| 197 | 197 | t.Parallel() |
| 198 | 198 | env := setup(t) |
| 199 | + deletedPath, err := env.deps.RepoFS.DeletedRepoPath("alice", "demo", env.repoID) | |
| 200 | + if err != nil { | |
| 201 | + t.Fatalf("DeletedRepoPath: %v", err) | |
| 202 | + } | |
| 199 | 203 | if err := lifecycle.SoftDelete(context.Background(), env.deps, env.alice.ID, env.repoID); err != nil { |
| 200 | 204 | t.Fatalf("SoftDelete: %v", err) |
| 201 | 205 | } |
| 206 | + if _, err := os.Stat(env.originalFS); !os.IsNotExist(err) { | |
| 207 | + t.Fatalf("canonical path after soft-delete: err = %v, want not exist", err) | |
| 208 | + } | |
| 209 | + if _, err := os.Stat(deletedPath); err != nil { | |
| 210 | + t.Fatalf("deleted tombstone missing: %v", err) | |
| 211 | + } | |
| 202 | 212 | if err := lifecycle.SoftDelete(context.Background(), env.deps, env.alice.ID, env.repoID); !errors.Is(err, lifecycle.ErrAlreadyDeleted) { |
| 203 | 213 | t.Errorf("double soft-delete: err=%v", err) |
| 204 | 214 | } |
| 205 | 215 | if err := lifecycle.Restore(context.Background(), env.deps, env.alice.ID, env.repoID); err != nil { |
| 206 | 216 | t.Fatalf("Restore: %v", err) |
| 207 | 217 | } |
| 218 | + if _, err := os.Stat(env.originalFS); err != nil { | |
| 219 | + t.Fatalf("canonical path after restore missing: %v", err) | |
| 220 | + } | |
| 221 | + if _, err := os.Stat(deletedPath); !os.IsNotExist(err) { | |
| 222 | + t.Fatalf("deleted tombstone after restore: err = %v, want not exist", err) | |
| 223 | + } | |
| 208 | 224 | if err := lifecycle.Restore(context.Background(), env.deps, env.alice.ID, env.repoID); !errors.Is(err, lifecycle.ErrNotDeleted) { |
| 209 | 225 | t.Errorf("double restore: err=%v", err) |
| 210 | 226 | } |
| 211 | 227 | } |
| 212 | 228 | |
| 229 | +func TestRestore_RefusesWhenNameReused(t *testing.T) { | |
| 230 | + t.Parallel() | |
| 231 | + env := setup(t) | |
| 232 | + ctx := context.Background() | |
| 233 | + if err := lifecycle.SoftDelete(ctx, env.deps, env.alice.ID, env.repoID); err != nil { | |
| 234 | + t.Fatalf("SoftDelete: %v", err) | |
| 235 | + } | |
| 236 | + replacement, err := repos.Create(ctx, env.rdeps, repos.Params{ | |
| 237 | + OwnerUserID: env.alice.ID, OwnerUsername: env.alice.Username, | |
| 238 | + Name: "demo", Visibility: "public", | |
| 239 | + }) | |
| 240 | + if err != nil { | |
| 241 | + t.Fatalf("replacement create: %v", err) | |
| 242 | + } | |
| 243 | + if err := lifecycle.Restore(ctx, env.deps, env.alice.ID, env.repoID); !errors.Is(err, lifecycle.ErrNameTaken) { | |
| 244 | + t.Fatalf("Restore: err = %v, want ErrNameTaken", err) | |
| 245 | + } | |
| 246 | + if _, err := os.Stat(replacement.DiskPath); err != nil { | |
| 247 | + t.Fatalf("replacement canonical path missing: %v", err) | |
| 248 | + } | |
| 249 | + deletedPath, err := env.deps.RepoFS.DeletedRepoPath("alice", "demo", env.repoID) | |
| 250 | + if err != nil { | |
| 251 | + t.Fatalf("DeletedRepoPath: %v", err) | |
| 252 | + } | |
| 253 | + if _, err := os.Stat(deletedPath); err != nil { | |
| 254 | + t.Fatalf("old tombstone should remain restorable: %v", err) | |
| 255 | + } | |
| 256 | +} | |
| 257 | + | |
| 213 | 258 | func TestRestore_PastGraceRefuses(t *testing.T) { |
| 214 | 259 | t.Parallel() |
| 215 | 260 | env := setup(t) |
@@ -311,6 +356,10 @@ func TestTransfer_ExpireSweepFlipsPending(t *testing.T) { | ||
| 311 | 356 | func TestHardDelete_PastGraceCascades(t *testing.T) { |
| 312 | 357 | t.Parallel() |
| 313 | 358 | env := setup(t) |
| 359 | + deletedPath, err := env.deps.RepoFS.DeletedRepoPath("alice", "demo", env.repoID) | |
| 360 | + if err != nil { | |
| 361 | + t.Fatalf("DeletedRepoPath: %v", err) | |
| 362 | + } | |
| 314 | 363 | if err := lifecycle.SoftDelete(context.Background(), env.deps, env.alice.ID, env.repoID); err != nil { |
| 315 | 364 | t.Fatal(err) |
| 316 | 365 | } |
@@ -327,8 +376,8 @@ func TestHardDelete_PastGraceCascades(t *testing.T) { | ||
| 327 | 376 | if _, err := rq.GetRepoByID(context.Background(), env.deps.Pool, env.repoID); err == nil { |
| 328 | 377 | t.Errorf("repo row still present after hard-delete") |
| 329 | 378 | } |
| 330 | - // FS dir gone. | |
| 331 | - if _, err := os.Stat(env.originalFS); !os.IsNotExist(err) { | |
| 332 | - t.Errorf("FS path still present: err=%v", err) | |
| 379 | + // Tombstone dir gone. | |
| 380 | + if _, err := os.Stat(deletedPath); !os.IsNotExist(err) { | |
| 381 | + t.Errorf("deleted tombstone still present: err=%v", err) | |
| 333 | 382 | } |
| 334 | 383 | } |
internal/repos/lifecycle/paths.goadded@@ -0,0 +1,102 @@ | ||
| 1 | +// SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | |
| 3 | +package lifecycle | |
| 4 | + | |
| 5 | +import ( | |
| 6 | + "context" | |
| 7 | + "errors" | |
| 8 | + "fmt" | |
| 9 | + "strings" | |
| 10 | + | |
| 11 | + "github.com/jackc/pgx/v5/pgconn" | |
| 12 | + "github.com/jackc/pgx/v5/pgtype" | |
| 13 | + | |
| 14 | + orgsdb "github.com/tenseleyFlow/shithub/internal/orgs/sqlc" | |
| 15 | + reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc" | |
| 16 | + usersdb "github.com/tenseleyFlow/shithub/internal/users/sqlc" | |
| 17 | +) | |
| 18 | + | |
| 19 | +type repoDiskPaths struct { | |
| 20 | + canonical string | |
| 21 | + deleted string | |
| 22 | +} | |
| 23 | + | |
| 24 | +func lockRepoName(ctx context.Context, q *reposdb.Queries, db reposdb.DBTX, repo reposdb.Repo) error { | |
| 25 | + key, err := repoNameLockKey(repo.OwnerUserID, repo.OwnerOrgID, repo.Name) | |
| 26 | + if err != nil { | |
| 27 | + return err | |
| 28 | + } | |
| 29 | + if err := q.LockRepoOwnerName(ctx, db, key); err != nil { | |
| 30 | + return fmt.Errorf("lock repo owner/name: %w", err) | |
| 31 | + } | |
| 32 | + return nil | |
| 33 | +} | |
| 34 | + | |
| 35 | +func repoNameLockKey(ownerUserID, ownerOrgID pgtype.Int8, name string) (string, error) { | |
| 36 | + name = strings.ToLower(name) | |
| 37 | + switch { | |
| 38 | + case ownerUserID.Valid && !ownerOrgID.Valid: | |
| 39 | + return fmt.Sprintf("repo-name:user:%d:%s", ownerUserID.Int64, name), nil | |
| 40 | + case ownerOrgID.Valid && !ownerUserID.Valid: | |
| 41 | + return fmt.Sprintf("repo-name:org:%d:%s", ownerOrgID.Int64, name), nil | |
| 42 | + default: | |
| 43 | + return "", errors.New("lifecycle: repo owner is not xor") | |
| 44 | + } | |
| 45 | +} | |
| 46 | + | |
| 47 | +func diskPathsForRepo(ctx context.Context, deps Deps, repo reposdb.Repo) (repoDiskPaths, error) { | |
| 48 | + owner, err := ownerSlugForRepo(ctx, deps, repo) | |
| 49 | + if err != nil { | |
| 50 | + return repoDiskPaths{}, err | |
| 51 | + } | |
| 52 | + canonical, err := deps.RepoFS.RepoPath(owner, repo.Name) | |
| 53 | + if err != nil { | |
| 54 | + return repoDiskPaths{}, fmt.Errorf("canonical repo path: %w", err) | |
| 55 | + } | |
| 56 | + deleted, err := deps.RepoFS.DeletedRepoPath(owner, repo.Name, repo.ID) | |
| 57 | + if err != nil { | |
| 58 | + return repoDiskPaths{}, fmt.Errorf("deleted repo path: %w", err) | |
| 59 | + } | |
| 60 | + return repoDiskPaths{canonical: canonical, deleted: deleted}, nil | |
| 61 | +} | |
| 62 | + | |
| 63 | +func ownerSlugForRepo(ctx context.Context, deps Deps, repo reposdb.Repo) (string, error) { | |
| 64 | + switch { | |
| 65 | + case repo.OwnerUserID.Valid && !repo.OwnerOrgID.Valid: | |
| 66 | + user, err := usersdb.New().GetUserIncludingDeleted(ctx, deps.Pool, repo.OwnerUserID.Int64) | |
| 67 | + if err != nil { | |
| 68 | + return "", fmt.Errorf("load repo owner user: %w", err) | |
| 69 | + } | |
| 70 | + return user.Username, nil | |
| 71 | + case repo.OwnerOrgID.Valid && !repo.OwnerUserID.Valid: | |
| 72 | + org, err := orgsdb.New().GetOrgByID(ctx, deps.Pool, repo.OwnerOrgID.Int64) | |
| 73 | + if err != nil { | |
| 74 | + return "", fmt.Errorf("load repo owner org: %w", err) | |
| 75 | + } | |
| 76 | + return string(org.Slug), nil | |
| 77 | + default: | |
| 78 | + return "", errors.New("lifecycle: repo owner is not xor") | |
| 79 | + } | |
| 80 | +} | |
| 81 | + | |
| 82 | +func activeRepoNameExists(ctx context.Context, q *reposdb.Queries, db reposdb.DBTX, repo reposdb.Repo) (bool, error) { | |
| 83 | + switch { | |
| 84 | + case repo.OwnerUserID.Valid && !repo.OwnerOrgID.Valid: | |
| 85 | + return q.ExistsRepoForOwnerUser(ctx, db, reposdb.ExistsRepoForOwnerUserParams{ | |
| 86 | + OwnerUserID: pgtype.Int8{Int64: repo.OwnerUserID.Int64, Valid: true}, | |
| 87 | + Name: repo.Name, | |
| 88 | + }) | |
| 89 | + case repo.OwnerOrgID.Valid && !repo.OwnerUserID.Valid: | |
| 90 | + return q.ExistsRepoForOwnerOrg(ctx, db, reposdb.ExistsRepoForOwnerOrgParams{ | |
| 91 | + OwnerOrgID: pgtype.Int8{Int64: repo.OwnerOrgID.Int64, Valid: true}, | |
| 92 | + Name: repo.Name, | |
| 93 | + }) | |
| 94 | + default: | |
| 95 | + return false, errors.New("lifecycle: repo owner is not xor") | |
| 96 | + } | |
| 97 | +} | |
| 98 | + | |
| 99 | +func isUniqueViolation(err error) bool { | |
| 100 | + var pgErr *pgconn.PgError | |
| 101 | + return errors.As(err, &pgErr) && pgErr.Code == "23505" | |
| 102 | +} | |
internal/repos/lifecycle/soft_delete.gomodified@@ -4,28 +4,61 @@ package lifecycle | ||
| 4 | 4 | |
| 5 | 5 | import ( |
| 6 | 6 | "context" |
| 7 | + "errors" | |
| 7 | 8 | "fmt" |
| 9 | + "os" | |
| 8 | 10 | |
| 9 | 11 | "github.com/tenseleyFlow/shithub/internal/auth/audit" |
| 12 | + "github.com/tenseleyFlow/shithub/internal/infra/storage" | |
| 10 | 13 | reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc" |
| 11 | 14 | ) |
| 12 | 15 | |
| 13 | 16 | // SoftDelete sets repos.deleted_at to now. The repo disappears from |
| 14 | 17 | // listings and the home page returns 404 for non-owners (auth-aware). |
| 15 | -// The bare repo on disk is left alone until the hard-delete worker | |
| 16 | -// runs at the end of the grace window. | |
| 18 | +// The bare repo is moved from the canonical owner/name path into an | |
| 19 | +// internal tombstone path so the owner can recreate the same repo name | |
| 20 | +// during the grace window without colliding with the deleted repo. | |
| 17 | 21 | func SoftDelete(ctx context.Context, deps Deps, actorUserID, repoID int64) error { |
| 18 | 22 | rq := reposdb.New() |
| 19 | - repo, err := rq.GetRepoByID(ctx, deps.Pool, repoID) | |
| 23 | + | |
| 24 | + tx, err := deps.Pool.Begin(ctx) | |
| 25 | + if err != nil { | |
| 26 | + return fmt.Errorf("begin: %w", err) | |
| 27 | + } | |
| 28 | + committed := false | |
| 29 | + defer func() { | |
| 30 | + if !committed { | |
| 31 | + _ = tx.Rollback(ctx) | |
| 32 | + } | |
| 33 | + }() | |
| 34 | + | |
| 35 | + repo, err := rq.GetRepoByID(ctx, tx, repoID) | |
| 20 | 36 | if err != nil { |
| 21 | 37 | return fmt.Errorf("load repo: %w", err) |
| 22 | 38 | } |
| 39 | + if err := lockRepoName(ctx, rq, tx, repo); err != nil { | |
| 40 | + return err | |
| 41 | + } | |
| 23 | 42 | if repo.DeletedAt.Valid { |
| 24 | 43 | return ErrAlreadyDeleted |
| 25 | 44 | } |
| 26 | - if err := rq.SoftDeleteRepoLifecycle(ctx, deps.Pool, repoID); err != nil { | |
| 45 | + moved, err := moveCanonicalToDeleted(ctx, deps, repo) | |
| 46 | + if err != nil { | |
| 47 | + return err | |
| 48 | + } | |
| 49 | + if err := rq.SoftDeleteRepoLifecycle(ctx, tx, repoID); err != nil { | |
| 50 | + if moved { | |
| 51 | + moveDeletedBackToCanonical(ctx, deps, repo) | |
| 52 | + } | |
| 27 | 53 | return fmt.Errorf("soft delete: %w", err) |
| 28 | 54 | } |
| 55 | + if err := tx.Commit(ctx); err != nil { | |
| 56 | + if moved { | |
| 57 | + moveDeletedBackToCanonical(ctx, deps, repo) | |
| 58 | + } | |
| 59 | + return fmt.Errorf("commit: %w", err) | |
| 60 | + } | |
| 61 | + committed = true | |
| 29 | 62 | if deps.Audit != nil { |
| 30 | 63 | _ = deps.Audit.Record(ctx, deps.Pool, actorUserID, |
| 31 | 64 | audit.ActionRepoSoftDeleted, audit.TargetRepo, repoID, |
@@ -39,19 +72,61 @@ func SoftDelete(ctx context.Context, deps Deps, actorUserID, repoID int64) error | ||
| 39 | 72 | // refuse — the operator-visible contract is "7 days, then it's gone". |
| 40 | 73 | func Restore(ctx context.Context, deps Deps, actorUserID, repoID int64) error { |
| 41 | 74 | rq := reposdb.New() |
| 42 | - repo, err := rq.GetRepoByID(ctx, deps.Pool, repoID) | |
| 75 | + | |
| 76 | + tx, err := deps.Pool.Begin(ctx) | |
| 77 | + if err != nil { | |
| 78 | + return fmt.Errorf("begin: %w", err) | |
| 79 | + } | |
| 80 | + committed := false | |
| 81 | + defer func() { | |
| 82 | + if !committed { | |
| 83 | + _ = tx.Rollback(ctx) | |
| 84 | + } | |
| 85 | + }() | |
| 86 | + | |
| 87 | + repo, err := rq.GetRepoByID(ctx, tx, repoID) | |
| 43 | 88 | if err != nil { |
| 44 | 89 | return fmt.Errorf("load repo: %w", err) |
| 45 | 90 | } |
| 91 | + if err := lockRepoName(ctx, rq, tx, repo); err != nil { | |
| 92 | + return err | |
| 93 | + } | |
| 46 | 94 | if !repo.DeletedAt.Valid { |
| 47 | 95 | return ErrNotDeleted |
| 48 | 96 | } |
| 49 | 97 | if deps.now().Sub(repo.DeletedAt.Time) > softDeleteGrace { |
| 50 | 98 | return ErrPastGrace |
| 51 | 99 | } |
| 52 | - if err := rq.RestoreRepo(ctx, deps.Pool, repoID); err != nil { | |
| 100 | + taken, err := activeRepoNameExists(ctx, rq, tx, repo) | |
| 101 | + if err != nil { | |
| 102 | + return fmt.Errorf("restore name check: %w", err) | |
| 103 | + } | |
| 104 | + if taken { | |
| 105 | + return ErrNameTaken | |
| 106 | + } | |
| 107 | + moved, err := moveDeletedToCanonical(ctx, deps, repo) | |
| 108 | + if err != nil { | |
| 109 | + if errors.Is(err, storage.ErrAlreadyExists) { | |
| 110 | + return ErrNameTaken | |
| 111 | + } | |
| 112 | + return err | |
| 113 | + } | |
| 114 | + if err := rq.RestoreRepo(ctx, tx, repoID); err != nil { | |
| 115 | + if moved { | |
| 116 | + moveCanonicalBackToDeleted(ctx, deps, repo) | |
| 117 | + } | |
| 118 | + if isUniqueViolation(err) { | |
| 119 | + return ErrNameTaken | |
| 120 | + } | |
| 53 | 121 | return fmt.Errorf("restore: %w", err) |
| 54 | 122 | } |
| 123 | + if err := tx.Commit(ctx); err != nil { | |
| 124 | + if moved { | |
| 125 | + moveCanonicalBackToDeleted(ctx, deps, repo) | |
| 126 | + } | |
| 127 | + return fmt.Errorf("commit: %w", err) | |
| 128 | + } | |
| 129 | + committed = true | |
| 55 | 130 | if deps.Audit != nil { |
| 56 | 131 | _ = deps.Audit.Record(ctx, deps.Pool, actorUserID, |
| 57 | 132 | audit.ActionRepoRestored, audit.TargetRepo, repoID, nil) |
@@ -59,6 +134,74 @@ func Restore(ctx context.Context, deps Deps, actorUserID, repoID int64) error { | ||
| 59 | 134 | return nil |
| 60 | 135 | } |
| 61 | 136 | |
| 137 | +func moveCanonicalToDeleted(ctx context.Context, deps Deps, repo reposdb.Repo) (bool, error) { | |
| 138 | + paths, err := diskPathsForRepo(ctx, deps, repo) | |
| 139 | + if err != nil { | |
| 140 | + return false, err | |
| 141 | + } | |
| 142 | + if err := deps.RepoFS.Move(paths.canonical, paths.deleted); err != nil { | |
| 143 | + if errors.Is(err, os.ErrNotExist) { | |
| 144 | + exists, existsErr := deps.RepoFS.Exists(paths.deleted) | |
| 145 | + if existsErr != nil { | |
| 146 | + return false, existsErr | |
| 147 | + } | |
| 148 | + if exists { | |
| 149 | + return false, nil | |
| 150 | + } | |
| 151 | + } | |
| 152 | + return false, fmt.Errorf("move repo to deleted path: %w", err) | |
| 153 | + } | |
| 154 | + return true, nil | |
| 155 | +} | |
| 156 | + | |
| 157 | +func moveDeletedToCanonical(ctx context.Context, deps Deps, repo reposdb.Repo) (bool, error) { | |
| 158 | + paths, err := diskPathsForRepo(ctx, deps, repo) | |
| 159 | + if err != nil { | |
| 160 | + return false, err | |
| 161 | + } | |
| 162 | + if err := deps.RepoFS.Move(paths.deleted, paths.canonical); err != nil { | |
| 163 | + if errors.Is(err, os.ErrNotExist) { | |
| 164 | + exists, existsErr := deps.RepoFS.Exists(paths.canonical) | |
| 165 | + if existsErr != nil { | |
| 166 | + return false, existsErr | |
| 167 | + } | |
| 168 | + if exists { | |
| 169 | + return false, nil | |
| 170 | + } | |
| 171 | + } | |
| 172 | + return false, fmt.Errorf("move repo to canonical path: %w", err) | |
| 173 | + } | |
| 174 | + return true, nil | |
| 175 | +} | |
| 176 | + | |
| 177 | +func moveDeletedBackToCanonical(ctx context.Context, deps Deps, repo reposdb.Repo) { | |
| 178 | + paths, err := diskPathsForRepo(ctx, deps, repo) | |
| 179 | + if err != nil { | |
| 180 | + if deps.Logger != nil { | |
| 181 | + deps.Logger.WarnContext(ctx, "soft delete: compute rollback path failed", "repo_id", repo.ID, "error", err) | |
| 182 | + } | |
| 183 | + return | |
| 184 | + } | |
| 185 | + if err := deps.RepoFS.Move(paths.deleted, paths.canonical); err != nil && deps.Logger != nil { | |
| 186 | + deps.Logger.WarnContext(ctx, "soft delete: rollback fs move failed", | |
| 187 | + "repo_id", repo.ID, "from", paths.deleted, "to", paths.canonical, "error", err) | |
| 188 | + } | |
| 189 | +} | |
| 190 | + | |
| 191 | +func moveCanonicalBackToDeleted(ctx context.Context, deps Deps, repo reposdb.Repo) { | |
| 192 | + paths, err := diskPathsForRepo(ctx, deps, repo) | |
| 193 | + if err != nil { | |
| 194 | + if deps.Logger != nil { | |
| 195 | + deps.Logger.WarnContext(ctx, "restore: compute rollback path failed", "repo_id", repo.ID, "error", err) | |
| 196 | + } | |
| 197 | + return | |
| 198 | + } | |
| 199 | + if err := deps.RepoFS.Move(paths.canonical, paths.deleted); err != nil && deps.Logger != nil { | |
| 200 | + deps.Logger.WarnContext(ctx, "restore: rollback fs move failed", | |
| 201 | + "repo_id", repo.ID, "from", paths.canonical, "to", paths.deleted, "error", err) | |
| 202 | + } | |
| 203 | +} | |
| 204 | + | |
| 62 | 205 | // int64ValueOrZero unwraps a pgtype.Int8 stored as raw int64+bool. We |
| 63 | 206 | // keep this private helper duplicated across packages rather than |
| 64 | 207 | // pulling pgtype into the audit-meta hot path. |
internal/repos/queries/repos.sqlmodified@@ -15,6 +15,13 @@ RETURNING id, owner_user_id, owner_org_id, name, description, visibility, | ||
| 15 | 15 | star_count, watcher_count, fork_count, init_status, |
| 16 | 16 | last_indexed_oid; |
| 17 | 17 | |
| 18 | +-- name: LockRepoOwnerName :exec | |
| 19 | +-- Serializes DB + filesystem operations for one logical owner/name | |
| 20 | +-- pair. Create, soft-delete, restore, and hard-delete all touch the | |
| 21 | +-- canonical bare path; a transaction-scoped advisory lock keeps those | |
| 22 | +-- cross-resource moves from racing. | |
| 23 | +SELECT pg_advisory_xact_lock(hashtextextended($1, 0)); | |
| 24 | + | |
| 18 | 25 | -- name: GetRepoByID :one |
| 19 | 26 | SELECT id, owner_user_id, owner_org_id, name, description, visibility, |
| 20 | 27 | default_branch, is_archived, archived_at, deleted_at, |
@@ -46,6 +53,19 @@ SELECT id, owner_user_id, owner_org_id, name, description, visibility, | ||
| 46 | 53 | FROM repos |
| 47 | 54 | WHERE owner_user_id = $1 AND name = $2 AND deleted_at IS NULL; |
| 48 | 55 | |
| 56 | +-- name: GetSoftDeletedRepoByOwnerUserAndName :one | |
| 57 | +SELECT id, owner_user_id, owner_org_id, name, description, visibility, | |
| 58 | + default_branch, is_archived, archived_at, deleted_at, | |
| 59 | + disk_used_bytes, fork_of_repo_id, license_key, primary_language, | |
| 60 | + has_issues, has_pulls, created_at, updated_at, default_branch_oid, | |
| 61 | + allow_squash_merge, allow_rebase_merge, allow_merge_commit, default_merge_method, | |
| 62 | + star_count, watcher_count, fork_count, init_status, | |
| 63 | + last_indexed_oid | |
| 64 | +FROM repos | |
| 65 | +WHERE owner_user_id = $1 AND name = $2 AND deleted_at IS NOT NULL | |
| 66 | +ORDER BY deleted_at DESC, id DESC | |
| 67 | +LIMIT 1; | |
| 68 | + | |
| 49 | 69 | -- name: ExistsRepoForOwnerUser :one |
| 50 | 70 | SELECT EXISTS( |
| 51 | 71 | SELECT 1 FROM repos |
@@ -82,6 +102,19 @@ SELECT id, owner_user_id, owner_org_id, name, description, visibility, | ||
| 82 | 102 | FROM repos |
| 83 | 103 | WHERE owner_org_id = $1 AND name = $2 AND deleted_at IS NULL; |
| 84 | 104 | |
| 105 | +-- name: GetSoftDeletedRepoByOwnerOrgAndName :one | |
| 106 | +SELECT id, owner_user_id, owner_org_id, name, description, visibility, | |
| 107 | + default_branch, is_archived, archived_at, deleted_at, | |
| 108 | + disk_used_bytes, fork_of_repo_id, license_key, primary_language, | |
| 109 | + has_issues, has_pulls, created_at, updated_at, default_branch_oid, | |
| 110 | + allow_squash_merge, allow_rebase_merge, allow_merge_commit, default_merge_method, | |
| 111 | + star_count, watcher_count, fork_count, init_status, | |
| 112 | + last_indexed_oid | |
| 113 | +FROM repos | |
| 114 | +WHERE owner_org_id = $1 AND name = $2 AND deleted_at IS NOT NULL | |
| 115 | +ORDER BY deleted_at DESC, id DESC | |
| 116 | +LIMIT 1; | |
| 117 | + | |
| 85 | 118 | -- name: ExistsRepoForOwnerOrg :one |
| 86 | 119 | SELECT EXISTS( |
| 87 | 120 | SELECT 1 FROM repos |
internal/repos/sqlc/querier.gomodified@@ -67,6 +67,8 @@ type Querier interface { | ||
| 67 | 67 | GetRepoOwnerUsernameByID(ctx context.Context, db DBTX, id int64) (GetRepoOwnerUsernameByIDRow, error) |
| 68 | 68 | // SPDX-License-Identifier: AGPL-3.0-or-later |
| 69 | 69 | GetRepoSourceRemote(ctx context.Context, db DBTX, repoID int64) (RepoSourceRemote, error) |
| 70 | + GetSoftDeletedRepoByOwnerOrgAndName(ctx context.Context, db DBTX, arg GetSoftDeletedRepoByOwnerOrgAndNameParams) (Repo, error) | |
| 71 | + GetSoftDeletedRepoByOwnerUserAndName(ctx context.Context, db DBTX, arg GetSoftDeletedRepoByOwnerUserAndNameParams) (Repo, error) | |
| 70 | 72 | GetTransferRequest(ctx context.Context, db DBTX, id int64) (RepoTransferRequest, error) |
| 71 | 73 | HardDeleteRepo(ctx context.Context, db DBTX, id int64) error |
| 72 | 74 | InsertProfilePin(ctx context.Context, db DBTX, arg InsertProfilePinParams) error |
@@ -112,6 +114,11 @@ type Querier interface { | ||
| 112 | 114 | ListSoftDeletedReposForOwner(ctx context.Context, db DBTX, ownerUserID pgtype.Int8) ([]ListSoftDeletedReposForOwnerRow, error) |
| 113 | 115 | // Sender / repo-settings view. |
| 114 | 116 | ListTransfersForRepo(ctx context.Context, db DBTX, repoID int64) ([]RepoTransferRequest, error) |
| 117 | + // Serializes DB + filesystem operations for one logical owner/name | |
| 118 | + // pair. Create, soft-delete, restore, and hard-delete all touch the | |
| 119 | + // canonical bare path; a transaction-scoped advisory lock keeps those | |
| 120 | + // cross-resource moves from racing. | |
| 121 | + LockRepoOwnerName(ctx context.Context, db DBTX, hashtextextended string) error | |
| 115 | 122 | // Returns the current repo_id when (old_owner_user_id, old_name) hits |
| 116 | 123 | // a redirect row. |
| 117 | 124 | LookupRedirectByUserOwner(ctx context.Context, db DBTX, arg LookupRedirectByUserOwnerParams) (int64, error) |
internal/repos/sqlc/repos.sql.gomodified@@ -448,6 +448,116 @@ func (q *Queries) GetRepoOwnerUsernameByID(ctx context.Context, db DBTX, id int6 | ||
| 448 | 448 | return i, err |
| 449 | 449 | } |
| 450 | 450 | |
| 451 | +const getSoftDeletedRepoByOwnerOrgAndName = `-- name: GetSoftDeletedRepoByOwnerOrgAndName :one | |
| 452 | +SELECT id, owner_user_id, owner_org_id, name, description, visibility, | |
| 453 | + default_branch, is_archived, archived_at, deleted_at, | |
| 454 | + disk_used_bytes, fork_of_repo_id, license_key, primary_language, | |
| 455 | + has_issues, has_pulls, created_at, updated_at, default_branch_oid, | |
| 456 | + allow_squash_merge, allow_rebase_merge, allow_merge_commit, default_merge_method, | |
| 457 | + star_count, watcher_count, fork_count, init_status, | |
| 458 | + last_indexed_oid | |
| 459 | +FROM repos | |
| 460 | +WHERE owner_org_id = $1 AND name = $2 AND deleted_at IS NOT NULL | |
| 461 | +ORDER BY deleted_at DESC, id DESC | |
| 462 | +LIMIT 1 | |
| 463 | +` | |
| 464 | + | |
| 465 | +type GetSoftDeletedRepoByOwnerOrgAndNameParams struct { | |
| 466 | + OwnerOrgID pgtype.Int8 | |
| 467 | + Name string | |
| 468 | +} | |
| 469 | + | |
| 470 | +func (q *Queries) GetSoftDeletedRepoByOwnerOrgAndName(ctx context.Context, db DBTX, arg GetSoftDeletedRepoByOwnerOrgAndNameParams) (Repo, error) { | |
| 471 | + row := db.QueryRow(ctx, getSoftDeletedRepoByOwnerOrgAndName, arg.OwnerOrgID, arg.Name) | |
| 472 | + var i Repo | |
| 473 | + err := row.Scan( | |
| 474 | + &i.ID, | |
| 475 | + &i.OwnerUserID, | |
| 476 | + &i.OwnerOrgID, | |
| 477 | + &i.Name, | |
| 478 | + &i.Description, | |
| 479 | + &i.Visibility, | |
| 480 | + &i.DefaultBranch, | |
| 481 | + &i.IsArchived, | |
| 482 | + &i.ArchivedAt, | |
| 483 | + &i.DeletedAt, | |
| 484 | + &i.DiskUsedBytes, | |
| 485 | + &i.ForkOfRepoID, | |
| 486 | + &i.LicenseKey, | |
| 487 | + &i.PrimaryLanguage, | |
| 488 | + &i.HasIssues, | |
| 489 | + &i.HasPulls, | |
| 490 | + &i.CreatedAt, | |
| 491 | + &i.UpdatedAt, | |
| 492 | + &i.DefaultBranchOid, | |
| 493 | + &i.AllowSquashMerge, | |
| 494 | + &i.AllowRebaseMerge, | |
| 495 | + &i.AllowMergeCommit, | |
| 496 | + &i.DefaultMergeMethod, | |
| 497 | + &i.StarCount, | |
| 498 | + &i.WatcherCount, | |
| 499 | + &i.ForkCount, | |
| 500 | + &i.InitStatus, | |
| 501 | + &i.LastIndexedOid, | |
| 502 | + ) | |
| 503 | + return i, err | |
| 504 | +} | |
| 505 | + | |
| 506 | +const getSoftDeletedRepoByOwnerUserAndName = `-- name: GetSoftDeletedRepoByOwnerUserAndName :one | |
| 507 | +SELECT id, owner_user_id, owner_org_id, name, description, visibility, | |
| 508 | + default_branch, is_archived, archived_at, deleted_at, | |
| 509 | + disk_used_bytes, fork_of_repo_id, license_key, primary_language, | |
| 510 | + has_issues, has_pulls, created_at, updated_at, default_branch_oid, | |
| 511 | + allow_squash_merge, allow_rebase_merge, allow_merge_commit, default_merge_method, | |
| 512 | + star_count, watcher_count, fork_count, init_status, | |
| 513 | + last_indexed_oid | |
| 514 | +FROM repos | |
| 515 | +WHERE owner_user_id = $1 AND name = $2 AND deleted_at IS NOT NULL | |
| 516 | +ORDER BY deleted_at DESC, id DESC | |
| 517 | +LIMIT 1 | |
| 518 | +` | |
| 519 | + | |
| 520 | +type GetSoftDeletedRepoByOwnerUserAndNameParams struct { | |
| 521 | + OwnerUserID pgtype.Int8 | |
| 522 | + Name string | |
| 523 | +} | |
| 524 | + | |
| 525 | +func (q *Queries) GetSoftDeletedRepoByOwnerUserAndName(ctx context.Context, db DBTX, arg GetSoftDeletedRepoByOwnerUserAndNameParams) (Repo, error) { | |
| 526 | + row := db.QueryRow(ctx, getSoftDeletedRepoByOwnerUserAndName, arg.OwnerUserID, arg.Name) | |
| 527 | + var i Repo | |
| 528 | + err := row.Scan( | |
| 529 | + &i.ID, | |
| 530 | + &i.OwnerUserID, | |
| 531 | + &i.OwnerOrgID, | |
| 532 | + &i.Name, | |
| 533 | + &i.Description, | |
| 534 | + &i.Visibility, | |
| 535 | + &i.DefaultBranch, | |
| 536 | + &i.IsArchived, | |
| 537 | + &i.ArchivedAt, | |
| 538 | + &i.DeletedAt, | |
| 539 | + &i.DiskUsedBytes, | |
| 540 | + &i.ForkOfRepoID, | |
| 541 | + &i.LicenseKey, | |
| 542 | + &i.PrimaryLanguage, | |
| 543 | + &i.HasIssues, | |
| 544 | + &i.HasPulls, | |
| 545 | + &i.CreatedAt, | |
| 546 | + &i.UpdatedAt, | |
| 547 | + &i.DefaultBranchOid, | |
| 548 | + &i.AllowSquashMerge, | |
| 549 | + &i.AllowRebaseMerge, | |
| 550 | + &i.AllowMergeCommit, | |
| 551 | + &i.DefaultMergeMethod, | |
| 552 | + &i.StarCount, | |
| 553 | + &i.WatcherCount, | |
| 554 | + &i.ForkCount, | |
| 555 | + &i.InitStatus, | |
| 556 | + &i.LastIndexedOid, | |
| 557 | + ) | |
| 558 | + return i, err | |
| 559 | +} | |
| 560 | + | |
| 451 | 561 | const insertProfilePin = `-- name: InsertProfilePin :exec |
| 452 | 562 | INSERT INTO profile_pins (set_id, repo_id, position) |
| 453 | 563 | VALUES ($1, $2, $3) |
@@ -919,6 +1029,19 @@ func (q *Queries) ListReposNeedingReindex(ctx context.Context, db DBTX, limit in | ||
| 919 | 1029 | return items, nil |
| 920 | 1030 | } |
| 921 | 1031 | |
| 1032 | +const lockRepoOwnerName = `-- name: LockRepoOwnerName :exec | |
| 1033 | +SELECT pg_advisory_xact_lock(hashtextextended($1, 0)) | |
| 1034 | +` | |
| 1035 | + | |
| 1036 | +// Serializes DB + filesystem operations for one logical owner/name | |
| 1037 | +// pair. Create, soft-delete, restore, and hard-delete all touch the | |
| 1038 | +// canonical bare path; a transaction-scoped advisory lock keeps those | |
| 1039 | +// cross-resource moves from racing. | |
| 1040 | +func (q *Queries) LockRepoOwnerName(ctx context.Context, db DBTX, hashtextextended string) error { | |
| 1041 | + _, err := db.Exec(ctx, lockRepoOwnerName, hashtextextended) | |
| 1042 | + return err | |
| 1043 | +} | |
| 1044 | + | |
| 922 | 1045 | const replaceRepoTopics = `-- name: ReplaceRepoTopics :exec |
| 923 | 1046 | DELETE FROM repo_topics WHERE repo_id = $1 |
| 924 | 1047 | ` |