From d8ecb4198195b1a48d4cce48cb53a998b1ee3e81 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Fri, 18 Sep 2026 20:00:31 +0200 Subject: [PATCH] feat(05-04): add attach delete lifecycle and static file handler - DeleteForOwner removes system_files rows in-tx; blobs after commit - Soft-delete of an owner keeps rows and blobs - StaticHandler serves exact partition+disk_name keys and 404s traversal --- lagoon/attach/file.go | 95 ++++++++++++++- lagoon/attach/lifecycle_test.go | 199 ++++++++++++++++++++++++++++++++ lagoon/attach/static.go | 92 +++++++++++++++ lagoon/attach/static_test.go | 71 ++++++++++++ 4 files changed, 456 insertions(+), 1 deletion(-) create mode 100644 lagoon/attach/lifecycle_test.go create mode 100644 lagoon/attach/static.go create mode 100644 lagoon/attach/static_test.go diff --git a/lagoon/attach/file.go b/lagoon/attach/file.go index ff1b427..5e6e85a 100644 --- a/lagoon/attach/file.go +++ b/lagoon/attach/file.go @@ -1,6 +1,16 @@ package attach -import "time" +import ( + "context" + "fmt" + "io" + "strings" + "time" + + "gocloud.dev/blob" + "gocloud.dev/gcerrors" + "gorm.io/gorm" +) // Owner is implemented by models that own system_files rows. MorphName // must return the PHP class string so cutover-copied attachment_type @@ -46,3 +56,86 @@ func All() []any { func init() { Register(&File{}) } + +func blobKeysFor(f File) []string { + part := PartitionDirectory(f.DiskName) + return []string{ + part + f.DiskName, + part + fmt.Sprintf("thumb_%d_", f.ID), + } +} + +// DeleteForOwner removes system_files rows for owner inside tx. +// +// Two-phase contract (GORM has no post-commit hook): +// 1. Inside tx this function SELECTs disk_name values, DELETEs the rows, +// and invokes afterCommit with the blob keys so the caller can record +// them. afterCommit must not delete blobs — a rollback cannot restore +// bytes. +// 2. After the top-level Unscoped().Delete(...) returns without error +// (the transaction has committed), the caller passes those keys to +// DeleteKeys to remove originals and thumbs from the bucket. +// +// Soft-deleting an owner must not call this helper: rows and blobs stay. +func DeleteForOwner(tx *gorm.DB, owner Owner, ownerID string, afterCommit func(blobKeys []string) error) error { + if tx == nil { + return fmt.Errorf("attach: delete tx is nil") + } + if owner == nil { + return fmt.Errorf("attach: delete owner is nil") + } + morph := owner.MorphName() + var files []File + if err := tx.Where("attachment_type = ? AND attachment_id = ?", morph, ownerID).Find(&files).Error; err != nil { + return err + } + var keys []string + for _, f := range files { + keys = append(keys, blobKeysFor(f)...) + } + if err := tx.Where("attachment_type = ? AND attachment_id = ?", morph, ownerID).Delete(&File{}).Error; err != nil { + return err + } + if afterCommit != nil { + return afterCommit(keys) + } + return nil +} + +// DeleteKeys removes blob objects after a committed force-delete. +// Keys that end in '_' are treated as List prefixes (thumb__). +func DeleteKeys(ctx context.Context, bucket *blob.Bucket, keys []string) error { + if bucket == nil { + return fmt.Errorf("attach: bucket is nil") + } + for _, key := range keys { + if strings.HasSuffix(key, "_") || strings.HasSuffix(key, "/") { + iter := bucket.List(&blob.ListOptions{Prefix: key}) + for { + obj, err := iter.Next(ctx) + if err == io.EOF { + break + } + if err != nil { + return fmt.Errorf("attach: list %q: %w", key, err) + } + if err := deleteKey(ctx, bucket, obj.Key); err != nil { + return err + } + } + continue + } + if err := deleteKey(ctx, bucket, key); err != nil { + return err + } + } + return nil +} + +func deleteKey(ctx context.Context, bucket *blob.Bucket, key string) error { + err := bucket.Delete(ctx, key) + if err == nil || gcerrors.Code(err) == gcerrors.NotFound { + return nil + } + return fmt.Errorf("attach: delete %q: %w", key, err) +} diff --git a/lagoon/attach/lifecycle_test.go b/lagoon/attach/lifecycle_test.go new file mode 100644 index 0000000..0a16df8 --- /dev/null +++ b/lagoon/attach/lifecycle_test.go @@ -0,0 +1,199 @@ +package attach_test + +import ( + "context" + "database/sql" + "fmt" + "strconv" + "testing" + "time" + + "git.golem15.com/golem15/summercms/lagoon" + "git.golem15.com/golem15/summercms/lagoon/attach" + _ "github.com/jackc/pgx/v5/stdlib" + "github.com/testcontainers/testcontainers-go" + "github.com/testcontainers/testcontainers-go/modules/postgres" + "gocloud.dev/blob" + "gocloud.dev/blob/memblob" + "gorm.io/gorm" +) + +type lifecycleOwner struct { + ID uint `gorm:"column:id;primaryKey"` + Name string `gorm:"column:name"` + DeletedAt gorm.DeletedAt `gorm:"column:deleted_at"` +} + +func (lifecycleOwner) TableName() string { return "attach_lifecycle_owners" } + +func (lifecycleOwner) MorphName() string { + return `Golem15\Fonoteka\Models\Album` +} + +func TestFileLifecycle(t *testing.T) { + if testing.Short() { + t.Skip("requires testcontainers postgres") + } + ctx := t.Context() + gdb := attachGorm(t) + if err := lagoon.Migrate(gdb, nil); err != nil { + t.Fatal(err) + } + if err := gdb.Exec(` +CREATE TABLE attach_lifecycle_owners ( + id SERIAL PRIMARY KEY, + name TEXT NOT NULL, + deleted_at TIMESTAMPTZ +)`).Error; err != nil { + t.Fatal(err) + } + + bucket := memblob.OpenBucket(nil) + t.Cleanup(func() { _ = bucket.Close() }) + + owner := lifecycleOwner{Name: "album"} + if err := gdb.Create(&owner).Error; err != nil { + t.Fatal(err) + } + file := attach.File{ + DiskName: "abc123xyz.jpg", + FileName: "cover.jpg", + FileSize: 12, + ContentType: "image/jpeg", + Field: "photos", + AttachmentID: strconv.FormatUint(uint64(owner.ID), 10), + AttachmentType: owner.MorphName(), + IsPublic: true, + } + if err := gdb.Create(&file).Error; err != nil { + t.Fatal(err) + } + origKey := attach.BlobKey(file.DiskName) + thumbKey := attach.PartitionDirectory(file.DiskName) + attach.ThumbFilename(file.ID, 200, 200, 0, 0, "crop", "jpg") + if err := bucket.WriteAll(ctx, origKey, []byte("original"), &blob.WriterOptions{ContentType: "image/jpeg"}); err != nil { + t.Fatal(err) + } + if err := bucket.WriteAll(ctx, thumbKey, []byte("thumb"), &blob.WriterOptions{ContentType: "image/jpeg"}); err != nil { + t.Fatal(err) + } + + if err := gdb.Delete(&owner).Error; err != nil { + t.Fatal(err) + } + var softOwner lifecycleOwner + if err := gdb.Unscoped().First(&softOwner, owner.ID).Error; err != nil { + t.Fatal(err) + } + if !softOwner.DeletedAt.Valid { + t.Fatal("soft-delete must set deleted_at") + } + assertFileRow(t, gdb, file.ID, true) + assertBlob(t, ctx, bucket, origKey, true) + assertBlob(t, ctx, bucket, thumbKey, true) + + if err := gdb.Transaction(func(tx *gorm.DB) error { + return attach.DeleteForOwner(tx, owner, file.AttachmentID, func(keys []string) error { + assertBlob(t, ctx, bucket, origKey, true) + assertBlob(t, ctx, bucket, thumbKey, true) + return fmt.Errorf("rollback after collecting keys") + }) + }); err == nil { + t.Fatal("expected rollback") + } + assertFileRow(t, gdb, file.ID, true) + assertBlob(t, ctx, bucket, origKey, true) + assertBlob(t, ctx, bucket, thumbKey, true) + + var pending []string + if err := gdb.Transaction(func(tx *gorm.DB) error { + if err := tx.Unscoped().Delete(&owner).Error; err != nil { + return err + } + return attach.DeleteForOwner(tx, owner, file.AttachmentID, func(keys []string) error { + pending = append([]string(nil), keys...) + var n int64 + if err := tx.Model(&attach.File{}).Where("id = ?", file.ID).Count(&n).Error; err != nil { + return err + } + if n != 0 { + return fmt.Errorf("system_files row must be gone inside the force-delete transaction") + } + exists, err := bucket.Exists(ctx, origKey) + if err != nil { + return err + } + if !exists { + return fmt.Errorf("blob must still exist before commit") + } + return nil + }) + }); err != nil { + t.Fatal(err) + } + assertFileRow(t, gdb, file.ID, false) + assertBlob(t, ctx, bucket, origKey, true) + if err := attach.DeleteKeys(ctx, bucket, pending); err != nil { + t.Fatal(err) + } + assertBlob(t, ctx, bucket, origKey, false) + assertBlob(t, ctx, bucket, thumbKey, false) +} + +func assertFileRow(t *testing.T, gdb *gorm.DB, id uint, want bool) { + t.Helper() + var n int64 + if err := gdb.Model(&attach.File{}).Where("id = ?", id).Count(&n).Error; err != nil { + t.Fatal(err) + } + if got := n > 0; got != want { + t.Fatalf("system_files id %d exists=%v, want %v", id, got, want) + } +} + +func assertBlob(t *testing.T, ctx context.Context, bucket *blob.Bucket, key string, want bool) { + t.Helper() + got, err := bucket.Exists(ctx, key) + if err != nil { + t.Fatal(err) + } + if got != want { + t.Fatalf("blob %q exists=%v, want %v", key, got, want) + } +} + +func attachGorm(t *testing.T) *gorm.DB { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + t.Cleanup(cancel) + ctr, err := postgres.Run(ctx, + "postgres:16-alpine", + postgres.WithDatabase("attach"), + postgres.WithUsername("attach"), + postgres.WithPassword("attach"), + postgres.BasicWaitStrategies(), + testcontainers.WithEnv(map[string]string{ + "POSTGRES_INITDB_ARGS": "--locale-provider=icu --icu-locale=pl-PL --encoding=UTF8", + }), + ) + if err != nil { + t.Fatalf("postgres: %v", err) + } + t.Cleanup(func() { _ = testcontainers.TerminateContainer(ctr) }) + dsn, err := ctr.ConnectionString(ctx, "sslmode=disable") + if err != nil { + t.Fatal(err) + } + db, err := sql.Open("pgx", dsn) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := db.PingContext(ctx); err != nil { + t.Fatal(err) + } + gdb, err := lagoon.Use(ctx, db) + if err != nil { + t.Fatalf("lagoon.Use: %v", err) + } + return gdb +} diff --git a/lagoon/attach/static.go b/lagoon/attach/static.go new file mode 100644 index 0000000..f644d15 --- /dev/null +++ b/lagoon/attach/static.go @@ -0,0 +1,92 @@ +package attach + +import ( + "io" + "net/http" + "strings" + + "gocloud.dev/blob" +) + +const defaultStaticContentType = "application/octet-stream" + +// StaticHandler serves GET prefix// from bucket. +// The blob key is rebuilt from disk_name via PartitionDirectory; request +// path segments never reach NewReader unvalidated (T-05-13). +func StaticHandler(bucket *blob.Bucket, prefix string) http.Handler { + prefix = strings.TrimSuffix(prefix, "/") + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet && r.Method != http.MethodHead { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + if bucket == nil { + http.NotFound(w, r) + return + } + rel, ok := stripStaticPrefix(r.URL.Path, prefix) + if !ok { + http.NotFound(w, r) + return + } + diskName, ok := parsePublicBlobPath(rel) + if !ok { + http.NotFound(w, r) + return + } + key := BlobKey(diskName) + reader, err := bucket.NewReader(r.Context(), key, nil) + if err != nil { + http.NotFound(w, r) + return + } + defer reader.Close() + ct := reader.ContentType() + if ct == "" { + ct = defaultStaticContentType + } + w.Header().Set("Content-Type", ct) + if r.Method == http.MethodHead { + return + } + _, _ = io.Copy(w, reader) + }) +} + +func stripStaticPrefix(path, prefix string) (string, bool) { + if prefix == "" { + return strings.TrimPrefix(path, "/"), true + } + if path == prefix { + return "", false + } + if strings.HasPrefix(path, prefix+"/") { + return path[len(prefix)+1:], true + } + return "", false +} + +// parsePublicBlobPath accepts exactly 3 partition groups plus disk_name +// whose PartitionDirectory matches those groups. Rejects "..", empty +// segments, extra slashes, and mismatched partitions. +func parsePublicBlobPath(p string) (string, bool) { + if p == "" || strings.Contains(p, "\\") || strings.Contains(p, "..") || strings.Contains(p, "//") { + return "", false + } + parts := strings.Split(p, "/") + if len(parts) != 4 { + return "", false + } + for _, part := range parts { + if part == "" || part == "." || part == ".." { + return "", false + } + } + diskName := parts[3] + got := strings.Join(parts[:3], "/") + want := strings.TrimSuffix(PartitionDirectory(diskName), "/") + if got != want { + return "", false + } + return diskName, true +} diff --git a/lagoon/attach/static_test.go b/lagoon/attach/static_test.go new file mode 100644 index 0000000..6c1c8e2 --- /dev/null +++ b/lagoon/attach/static_test.go @@ -0,0 +1,71 @@ +package attach + +import ( + "io" + "net/http" + "net/http/httptest" + "testing" + + "gocloud.dev/blob" + "gocloud.dev/blob/memblob" +) + +func TestStaticHandler(t *testing.T) { + ctx := t.Context() + bucket := memblob.OpenBucket(nil) + t.Cleanup(func() { _ = bucket.Close() }) + + diskName := "abc123xyz.jpg" + key := BlobKey(diskName) + body := []byte("cover-bytes") + if err := bucket.WriteAll(ctx, key, body, &blob.WriterOptions{ContentType: "image/jpeg"}); err != nil { + t.Fatal(err) + } + + h := StaticHandler(bucket, "/storage/uploads") + srv := httptest.NewServer(h) + t.Cleanup(srv.Close) + + res, err := http.Get(srv.URL + "/storage/uploads/abc/123/xyz/abc123xyz.jpg") + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + t.Fatalf("status = %d", res.StatusCode) + } + if ct := res.Header.Get("Content-Type"); ct != "image/jpeg" { + t.Fatalf("Content-Type = %q, want image/jpeg", ct) + } + got, err := io.ReadAll(res.Body) + if err != nil { + t.Fatal(err) + } + if string(got) != string(body) { + t.Fatalf("body = %q", got) + } + + missing, err := http.Get(srv.URL + "/storage/uploads/mis/sin/g.j/missing.jpg") + if err != nil { + t.Fatal(err) + } + defer missing.Body.Close() + if missing.StatusCode != http.StatusNotFound { + t.Fatalf("missing status = %d, want 404", missing.StatusCode) + } + + for _, path := range []string{ + "/storage/uploads/abc/123/xyz/../abc123xyz.jpg", + "/storage/uploads/abc/123/xyz//abc123xyz.jpg", + "/storage/uploads/foo/bar/baz/abc123xyz.jpg", + "/storage/uploads/abc/123/xyz/abc123xyz.jpg/extra", + "/storage/uploads", + } { + rr := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, path, nil) + h.ServeHTTP(rr, req) + if rr.Code != http.StatusNotFound { + t.Fatalf("path %q status = %d, want 404", path, rr.Code) + } + } +}