Commit 4c5cdcb
Antonio Mika
·
2026-10-09 12:49:44 -0400 EDT
parent 1ac67bc
feat(storage): remove dir marker
12 files changed,
+401,
-163
+73,
-0
| ... | ... | @@ -0,0 +1,73 @@ | |
| 1 | + | // clean-keep-dirs removes the ._pico_keep_dir markers pgs used to write to | |
| 2 | + | // keep empty directories. Filesystem storage keeps real directories now, so | |
| 3 | + | // removing a marker leaves its directory in place. | |
| 4 | + | package main | |
| 5 | + | ||
| 6 | + | import ( | |
| 7 | + | "flag" | |
| 8 | + | "fmt" | |
| 9 | + | "io/fs" | |
| 10 | + | "log/slog" | |
| 11 | + | "os" | |
| 12 | + | "path/filepath" | |
| 13 | + | ||
| 14 | + | "github.com/picosh/pico/pkg/storage" | |
| 15 | + | ) | |
| 16 | + | ||
| 17 | + | const keepDirName = "._pico_keep_dir" | |
| 18 | + | ||
| 19 | + | func main() { | |
| 20 | + | deleteFlag := flag.Bool("delete", false, "delete the markers found") | |
| 21 | + | flag.Parse() | |
| 22 | + | ||
| 23 | + | logger := slog.Default() | |
| 24 | + | ||
| 25 | + | adapter := storage.GetStorageTypeFromEnv() | |
| 26 | + | if adapter != "fs" { | |
| 27 | + | logger.Error("markers only exist on filesystem storage", "adapter", adapter) | |
| 28 | + | os.Exit(1) | |
| 29 | + | } | |
| 30 | + | st, err := storage.NewStorage(logger, adapter) | |
| 31 | + | if err != nil { | |
| 32 | + | logger.Error("failed to create storage", "err", err) | |
| 33 | + | os.Exit(1) | |
| 34 | + | } | |
| 35 | + | root := st.(*storage.StorageFS).Dir | |
| 36 | + | ||
| 37 | + | found, deleted, failed := 0, 0, 0 | |
| 38 | + | err = filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { | |
| 39 | + | if err != nil { | |
| 40 | + | logger.Error("failed to read", "path", p, "err", err) | |
| 41 | + | failed++ | |
| 42 | + | return nil | |
| 43 | + | } | |
| 44 | + | if d.IsDir() || d.Name() != keepDirName { | |
| 45 | + | return nil | |
| 46 | + | } | |
| 47 | + | found++ | |
| 48 | + | if !*deleteFlag { | |
| 49 | + | fmt.Println(p) | |
| 50 | + | return nil | |
| 51 | + | } | |
| 52 | + | if err := os.Remove(p); err != nil { | |
| 53 | + | logger.Error("failed to delete marker", "path", p, "err", err) | |
| 54 | + | failed++ | |
| 55 | + | return nil | |
| 56 | + | } | |
| 57 | + | deleted++ | |
| 58 | + | return nil | |
| 59 | + | }) | |
| 60 | + | if err != nil { | |
| 61 | + | logger.Error("failed to walk storage", "root", root, "err", err) | |
| 62 | + | os.Exit(1) | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | if !*deleteFlag { | |
| 66 | + | fmt.Printf("\nFound %d marker(s) under %s. Use --delete to remove them.\n", found, root) | |
| 67 | + | return | |
| 68 | + | } | |
| 69 | + | fmt.Printf("\nDeleted %d marker(s) under %s, %d failed\n", deleted, root, failed) | |
| 70 | + | if failed > 0 { | |
| 71 | + | os.Exit(1) | |
| 72 | + | } | |
| 73 | + | } |
+23,
-3
| ... | ... | @@ -7,6 +7,7 @@ import ( | |
| 7 | 7 | "io" | |
| 8 | 8 | "log/slog" | |
| 9 | 9 | "path/filepath" | |
| 10 | + | "slices" | |
| 10 | 11 | "strings" | |
| 11 | 12 | "text/tabwriter" | |
| 12 | 13 | "time" |
| ... | ... | @@ -101,6 +102,9 @@ func (c *Cmd) RmProjectAssets(projectName string) error { | |
| 101 | 102 | ||
| 102 | 103 | if len(fileList) == 0 { | |
| 103 | 104 | c.output(fmt.Sprintf("no assets found for project (%s)", projectName)) | |
| 105 | + | if c.Write { | |
| 106 | + | return c.Store.DeleteObject(bucket, projectName) | |
| 107 | + | } | |
| 104 | 108 | return nil | |
| 105 | 109 | } | |
| 106 | 110 | c.output(fmt.Sprintf("found (%d) assets for project (%s), removing", len(fileList), projectName)) |
| ... | ... | @@ -133,6 +137,25 @@ func (c *Cmd) RmProjectAssets(projectName string) error { | |
| 133 | 137 | c.output(intent) | |
| 134 | 138 | } | |
| 135 | 139 | } | |
| 140 | + | if !c.Write { | |
| 141 | + | return nil | |
| 142 | + | } | |
| 143 | + | ||
| 144 | + | // Directories outlive their files, so remove them deepest first. | |
| 145 | + | dirs := []string{projectName} | |
| 146 | + | for _, file := range fileList { | |
| 147 | + | if file.IsDir() { | |
| 148 | + | dirs = append(dirs, filepath.Join(projectName, file.Name())) | |
| 149 | + | } | |
| 150 | + | } | |
| 151 | + | slices.SortStableFunc(dirs, func(a, b string) int { | |
| 152 | + | return strings.Count(b, "/") - strings.Count(a, "/") | |
| 153 | + | }) | |
| 154 | + | for _, dir := range dirs { | |
| 155 | + | if err := c.Store.DeleteObject(bucket, dir); err != nil { | |
| 156 | + | return err | |
| 157 | + | } | |
| 158 | + | } | |
| 136 | 159 | return nil | |
| 137 | 160 | } | |
| 138 | 161 |
| ... | ... | @@ -325,9 +348,6 @@ func (c *Cmd) fzf(projectName string) error { | |
| 325 | 348 | } | |
| 326 | 349 | ||
| 327 | 350 | for _, obj := range objs { | |
| 328 | - | if strings.Contains(obj.Name(), "._pico_keep_dir") { | |
| 329 | - | continue | |
| 330 | - | } | |
| 331 | 351 | url := c.Cfg.AssetURL( | |
| 332 | 352 | c.User.Name, | |
| 333 | 353 | project.Name, |
+39,
-0
| ... | ... | @@ -4,6 +4,9 @@ import ( | |
| 4 | 4 | "bytes" | |
| 5 | 5 | "io" | |
| 6 | 6 | "log/slog" | |
| 7 | + | "os" | |
| 8 | + | "path/filepath" | |
| 9 | + | "strings" | |
| 7 | 10 | "testing" | |
| 8 | 11 | ||
| 9 | 12 | "github.com/picosh/pico/pkg/shared" |
| ... | ... | @@ -61,3 +64,39 @@ func TestCliPurges(t *testing.T) { | |
| 61 | 64 | } | |
| 62 | 65 | } | |
| 63 | 66 | } | |
| 67 | + | ||
| 68 | + | // Removing a project's assets also removes its directories, which storage | |
| 69 | + | // keeps after their last file is deleted. | |
| 70 | + | func TestRmProjectAssetsRemovesDirs(t *testing.T) { | |
| 71 | + | logger := slog.New(slog.DiscardHandler) | |
| 72 | + | dbpool := NewPgsDb(logger) | |
| 73 | + | user := dbpool.Users[0] | |
| 74 | + | st, err := storage.NewStorageFS(logger, t.TempDir()) | |
| 75 | + | if err != nil { | |
| 76 | + | t.Fatal(err) | |
| 77 | + | } | |
| 78 | + | bucket, err := st.UpsertBucket(shared.GetAssetBucketName(user.ID)) | |
| 79 | + | if err != nil { | |
| 80 | + | t.Fatal(err) | |
| 81 | + | } | |
| 82 | + | for _, name := range []string{"test/index.html", "test/css/deep/site.css", "other/index.html"} { | |
| 83 | + | if _, _, err := st.PutObject(bucket, name, strings.NewReader("x"), &storage.ObjectInfo{}); err != nil { | |
| 84 | + | t.Fatal(err) | |
| 85 | + | } | |
| 86 | + | } | |
| 87 | + | if err := st.PutDir(bucket, "test/empty/nested"); err != nil { | |
| 88 | + | t.Fatal(err) | |
| 89 | + | } | |
| 90 | + | ||
| 91 | + | cfg := NewPgsConfig(logger, dbpool, st, discardPubsub{}) | |
| 92 | + | cmd := &Cmd{User: user, Session: &fakeCmdSession{}, Log: logger, Store: st, Dbpool: dbpool, Write: true, Cfg: cfg} | |
| 93 | + | if err := cmd.RmProjectAssets("test"); err != nil { | |
| 94 | + | t.Fatal(err) | |
| 95 | + | } | |
| 96 | + | if _, err := os.Stat(filepath.Join(bucket.Path, "test")); !os.IsNotExist(err) { | |
| 97 | + | t.Fatalf("project directory still exists: %v", err) | |
| 98 | + | } | |
| 99 | + | if _, err := os.Stat(filepath.Join(bucket.Path, "other/index.html")); err != nil { | |
| 100 | + | t.Fatalf("another project was touched: %v", err) | |
| 101 | + | } | |
| 102 | + | } |
+1,
-82
| ... | ... | @@ -1,16 +1,12 @@ | |
| 1 | 1 | package pgs | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | - | "bytes" | |
| 5 | 4 | "context" | |
| 6 | 5 | "fmt" | |
| 7 | 6 | "io" | |
| 8 | - | "io/fs" | |
| 9 | 7 | "log/slog" | |
| 10 | 8 | "os" | |
| 11 | - | "path" | |
| 12 | 9 | "path/filepath" | |
| 13 | - | "slices" | |
| 14 | 10 | "strings" | |
| 15 | 11 | "sync" | |
| 16 | 12 | "time" |
| ... | ... | @@ -307,17 +303,8 @@ func (h *UploadAssetHandler) Write(s *pssh.SSHServerConnSession, entry *sendutil | |
| 307 | 303 | return "", fmt.Errorf(msg, project.Blocked) | |
| 308 | 304 | } | |
| 309 | 305 | ||
| 310 | - | info := &storage.ObjectInfo{ | |
| 311 | - | LastModified: mtimeToTime(entry), | |
| 312 | - | } | |
| 313 | 306 | if entry.Mode.IsDir() { | |
| 314 | - | _, _, err := h.Cfg.Storage.PutObject( | |
| 315 | - | bucket, | |
| 316 | - | path.Join(shared.GetAssetFileName(entry), "._pico_keep_dir"), | |
| 317 | - | bytes.NewReader([]byte{}), | |
| 318 | - | info, | |
| 319 | - | ) | |
| 320 | - | return "", err | |
| 307 | + | return "", h.Cfg.Storage.PutDir(bucket, shared.GetAssetFileName(entry)) | |
| 321 | 308 | } | |
| 322 | 309 | ||
| 323 | 310 | // calculate the filsize difference between the same file already |
| ... | ... | @@ -448,74 +435,6 @@ func (h *UploadAssetHandler) Delete(s *pssh.SSHServerConnSession, entry *senduti | |
| 448 | 435 | ||
| 449 | 436 | logger.Info("deleting file") | |
| 450 | 437 | ||
| 451 | - | // Check if this path represents a directory (has a . _pico_keep_dir marker) | |
| 452 | - | keepDirPath := filepath.Join(assetFilepath, "._pico_keep_dir") | |
| 453 | - | if _, err := h.Cfg.Storage.StatObject(bucket, keepDirPath); err == nil { | |
| 454 | - | // This is a directory being deleted. We must delete all nested | |
| 455 | - | // . _pico_keep_dir files first, otherwise os.Remove() on the | |
| 456 | - | // directory will fail with "directory not empty". | |
| 457 | - | nested, err := h.Cfg.Storage.ListObjects(bucket, assetFilepath+"/", true) | |
| 458 | - | if err != nil { | |
| 459 | - | return err | |
| 460 | - | } | |
| 461 | - | ||
| 462 | - | baseDepth := strings.Count(assetFilepath, string(os.PathSeparator)) | |
| 463 | - | for _, nestedEntry := range nested { | |
| 464 | - | if filepath.Base(nestedEntry.Name()) != "._pico_keep_dir" { | |
| 465 | - | continue | |
| 466 | - | } | |
| 467 | - | // Only delete keep_dir files that are direct or nested children | |
| 468 | - | nestedDepth := strings.Count(nestedEntry.Name(), string(os.PathSeparator)) | |
| 469 | - | if nestedDepth <= baseDepth { | |
| 470 | - | continue | |
| 471 | - | } | |
| 472 | - | nestedKeepDirPath := filepath.Join(assetFilepath, nestedEntry.Name()) | |
| 473 | - | if delErr := h.Cfg.Storage.DeleteObject(bucket, nestedKeepDirPath); delErr != nil { | |
| 474 | - | return delErr | |
| 475 | - | } | |
| 476 | - | } | |
| 477 | - | ||
| 478 | - | // Delete this directory's own . _pico_keep_dir | |
| 479 | - | err = h.Cfg.Storage.DeleteObject(bucket, keepDirPath) | |
| 480 | - | if err != nil { | |
| 481 | - | return err | |
| 482 | - | } | |
| 483 | - | ||
| 484 | - | // Delete the directory itself (no-op for S3-style storage, removes the dir for fs storage) | |
| 485 | - | _ = h.Cfg.Storage.DeleteObject(bucket, assetFilepath) | |
| 486 | - | ||
| 487 | - | surrogate := getSurrogateKey(user.Name, projectName) | |
| 488 | - | h.Cfg.CacheClearingQueue <- surrogate | |
| 489 | - | return nil | |
| 490 | - | } | |
| 491 | - | ||
| 492 | - | // Regular file deletion: create . _pico_keep_dir if the directory becomes empty | |
| 493 | - | pathDir := filepath.Dir(assetFilepath) | |
| 494 | - | fileName := filepath.Base(assetFilepath) | |
| 495 | - | ||
| 496 | - | sibs, err := h.Cfg.Storage.ListObjects(bucket, pathDir+"/", false) | |
| 497 | - | if err != nil { | |
| 498 | - | return err | |
| 499 | - | } | |
| 500 | - | ||
| 501 | - | sibs = slices.DeleteFunc(sibs, func(sib fs.FileInfo) bool { | |
| 502 | - | return sib.Name() == fileName | |
| 503 | - | }) | |
| 504 | - | ||
| 505 | - | if len(sibs) == 0 { | |
| 506 | - | info := &storage.ObjectInfo{ | |
| 507 | - | LastModified: mtimeToTime(entry), | |
| 508 | - | } | |
| 509 | - | _, _, err := h.Cfg.Storage.PutObject( | |
| 510 | - | bucket, | |
| 511 | - | filepath.Join(pathDir, "._pico_keep_dir"), | |
| 512 | - | bytes.NewReader([]byte{}), | |
| 513 | - | info, | |
| 514 | - | ) | |
| 515 | - | if err != nil { | |
| 516 | - | return err | |
| 517 | - | } | |
| 518 | - | } | |
| 519 | 438 | info, statErr := h.Cfg.Storage.StatObject(bucket, assetFilepath) | |
| 520 | 439 | err = h.Cfg.Storage.DeleteObject(bucket, assetFilepath) | |
| 521 | 440 | if err == nil && statErr == nil { |
+98,
-15
| ... | ... | @@ -4,6 +4,7 @@ import ( | |
| 4 | 4 | "encoding/pem" | |
| 5 | 5 | "fmt" | |
| 6 | 6 | "io" | |
| 7 | + | "io/fs" | |
| 7 | 8 | "log/slog" | |
| 8 | 9 | "net" | |
| 9 | 10 | "os" |
| ... | ... | @@ -22,13 +23,9 @@ import ( | |
| 22 | 23 | "github.com/prometheus/client_golang/prometheus" | |
| 23 | 24 | ) | |
| 24 | 25 | ||
| 25 | - | // TestRsyncDeleteDirectoryWithKeepDir verifies that rsync --delete can | |
| 26 | - | // successfully delete a directory that contains . _pico_keep_dir markers. | |
| 27 | - | // | |
| 28 | - | // Regression test for: "remove /storage/.../project/... directory not empty" | |
| 29 | - | // The bug occurred because . _pico_keep_dir files were not cleaned up when | |
| 30 | - | // their parent directory was explicitly deleted, causing os.Remove() to fail. | |
| 31 | - | func TestRsyncDeleteDirectoryWithKeepDir(t *testing.T) { | |
| 26 | + | // TestRsyncDeleteDirectory verifies that rsync --delete removes a directory | |
| 27 | + | // tree on filesystem storage without failing with "directory not empty". | |
| 28 | + | func TestRsyncDeleteDirectory(t *testing.T) { | |
| 32 | 29 | opts := &slog.HandlerOptions{ | |
| 33 | 30 | AddSource: true, | |
| 34 | 31 | Level: slog.LevelDebug, |
| ... | ... | @@ -205,12 +202,6 @@ func TestRsyncDeleteDirectoryWithKeepDir(t *testing.T) { | |
| 205 | 202 | t.Fatalf("expected 'not found' error, got: %v", err) | |
| 206 | 203 | } | |
| 207 | 204 | ||
| 208 | - | // Verify the . _pico_keep_dir markers are also cleaned up | |
| 209 | - | _, err = client.Lstat("/testdir/subdir/._pico_keep_dir") | |
| 210 | - | if err == nil { | |
| 211 | - | t.Fatal("subdir/._pico_keep_dir should have been deleted but still exists") | |
| 212 | - | } | |
| 213 | - | ||
| 214 | 205 | // Verify index.html still exists (it wasn't deleted) | |
| 215 | 206 | fi, err := client.Lstat("/testdir/index.html") | |
| 216 | 207 | if err != nil { |
| ... | ... | @@ -225,8 +216,7 @@ func TestRsyncDeleteDirectoryWithKeepDir(t *testing.T) { | |
| 225 | 216 | } | |
| 226 | 217 | ||
| 227 | 218 | // TestRsyncDeleteNestedEmptyDirectories verifies that rsync --delete handles | |
| 228 | - | // deeply nested directory structures where intermediate directories become empty | |
| 229 | - | // and need . _pico_keep_dir markers managed correctly. | |
| 219 | + | // deeply nested directory structures where intermediate directories become empty. | |
| 230 | 220 | func TestRsyncDeleteNestedEmptyDirectories(t *testing.T) { | |
| 231 | 221 | opts := &slog.HandlerOptions{ | |
| 232 | 222 | AddSource: true, |
| ... | ... | @@ -720,3 +710,96 @@ func TestPgsFlagAllowsUpload(t *testing.T) { | |
| 720 | 710 | t.Errorf("uploaded file has zero size") | |
| 721 | 711 | } | |
| 722 | 712 | } | |
| 713 | + | ||
| 714 | + | // SFTP directories behave like a filesystem's on storage that has real | |
| 715 | + | // directories, with no marker files left behind. | |
| 716 | + | func TestSftpDirectories(t *testing.T) { | |
| 717 | + | logger := slog.New(slog.DiscardHandler) | |
| 718 | + | dbpool := pgsdb.NewDBMemory(logger) | |
| 719 | + | dbpool.SetupTestData() | |
| 720 | + | storageDir := t.TempDir() | |
| 721 | + | st, err := storage.NewStorageFS(logger, storageDir) | |
| 722 | + | if err != nil { | |
| 723 | + | t.Fatal(err) | |
| 724 | + | } | |
| 725 | + | _ = os.Setenv("PGS_SSH_PORT", "0") | |
| 726 | + | cfg := NewPgsConfig(logger, dbpool, st, discardPubsub{}) | |
| 727 | + | done := make(chan error) | |
| 728 | + | defer close(done) | |
| 729 | + | readyCh := make(chan *pssh.SSHServer) | |
| 730 | + | prometheus.DefaultRegisterer = prometheus.NewRegistry() | |
| 731 | + | go StartSshServerForTesting(cfg, done, readyCh) | |
| 732 | + | server := <-readyCh | |
| 733 | + | if server == nil { | |
| 734 | + | t.Fatal("failed to create ssh server") | |
| 735 | + | } | |
| 736 | + | var addr string | |
| 737 | + | for range 100 { | |
| 738 | + | server.Mu.Lock() | |
| 739 | + | listener := server.Listener | |
| 740 | + | server.Mu.Unlock() | |
| 741 | + | if listener != nil { | |
| 742 | + | addr = listener.Addr().String() | |
| 743 | + | break | |
| 744 | + | } | |
| 745 | + | time.Sleep(10 * time.Millisecond) | |
| 746 | + | } | |
| 747 | + | ||
| 748 | + | user := GenerateUser() | |
| 749 | + | dbpool.Pubkeys = append(dbpool.Pubkeys, &db.PublicKey{ | |
| 750 | + | ID: "test-pubkey-dirs", | |
| 751 | + | UserID: dbpool.Users[0].ID, | |
| 752 | + | Key: shared.KeyForKeyText(user.signer.PublicKey()), | |
| 753 | + | }) | |
| 754 | + | conn, err := user.NewClientAddr(addr) | |
| 755 | + | if err != nil { | |
| 756 | + | t.Fatal(err) | |
| 757 | + | } | |
| 758 | + | defer func() { _ = conn.Close() }() | |
| 759 | + | client, err := sftp.NewClient(conn) | |
| 760 | + | if err != nil { | |
| 761 | + | t.Fatal(err) | |
| 762 | + | } | |
| 763 | + | defer func() { _ = client.Close() }() | |
| 764 | + | ||
| 765 | + | if err := client.MkdirAll("/site/empty"); err != nil { | |
| 766 | + | t.Fatal(err) | |
| 767 | + | } | |
| 768 | + | f, err := client.Create("/site/css/site.css") | |
| 769 | + | if err != nil { | |
| 770 | + | t.Fatal(err) | |
| 771 | + | } | |
| 772 | + | if _, err := f.Write([]byte("body{}")); err != nil { | |
| 773 | + | t.Fatal(err) | |
| 774 | + | } | |
| 775 | + | _ = f.Close() | |
| 776 | + | ||
| 777 | + | if err := client.RemoveDirectory("/site/css"); err == nil { | |
| 778 | + | t.Fatal("removing a non-empty directory succeeded") | |
| 779 | + | } | |
| 780 | + | if err := client.Remove("/site/css/site.css"); err != nil { | |
| 781 | + | t.Fatal(err) | |
| 782 | + | } | |
| 783 | + | for _, dir := range []string{"/site/empty", "/site/css"} { | |
| 784 | + | fi, err := client.Stat(dir) | |
| 785 | + | if err != nil || !fi.IsDir() { | |
| 786 | + | t.Fatalf("%s should be an empty directory: %v", dir, err) | |
| 787 | + | } | |
| 788 | + | if err := client.RemoveDirectory(dir); err != nil { | |
| 789 | + | t.Fatalf("rmdir %s: %v", dir, err) | |
| 790 | + | } | |
| 791 | + | if _, err := client.Stat(dir); err == nil { | |
| 792 | + | t.Fatalf("%s still exists after rmdir", dir) | |
| 793 | + | } | |
| 794 | + | } | |
| 795 | + | ||
| 796 | + | err = filepath.WalkDir(storageDir, func(p string, d fs.DirEntry, err error) error { | |
| 797 | + | if err == nil && !d.IsDir() { | |
| 798 | + | t.Errorf("unexpected file left in storage: %s", p) | |
| 799 | + | } | |
| 800 | + | return err | |
| 801 | + | }) | |
| 802 | + | if err != nil { | |
| 803 | + | t.Fatal(err) | |
| 804 | + | } | |
| 805 | + | } |
+1,
-4
| ... | ... | @@ -14,9 +14,6 @@ import ( | |
| 14 | 14 | "github.com/picosh/pico/pkg/send/utils" | |
| 15 | 15 | ) | |
| 16 | 16 | ||
| 17 | - | // keepDirName marks an otherwise empty directory in object storage. | |
| 18 | - | const keepDirName = "._pico_keep_dir" | |
| 19 | - | ||
| 20 | 17 | // storageFS exposes a CopyFromClientHandler as an rsync.FS. Handlers take | |
| 21 | 18 | // absolute paths and differ in how they name listing results, so this | |
| 22 | 19 | // normalizes both directions. |
| ... | ... | @@ -73,7 +70,7 @@ func (s *storageFS) ReadDir(dir string, recursive bool) ([]rsync.FileInfo, error | |
| 73 | 70 | continue | |
| 74 | 71 | } | |
| 75 | 72 | name := strings.Trim(e.Name(), "/") | |
| 76 | - | if name == "" || path.Base(name) == keepDirName { | |
| 73 | + | if name == "" { | |
| 77 | 74 | continue | |
| 78 | 75 | } | |
| 79 | 76 | out = append(out, rsync.FileInfo{Name: name, Size: e.Size(), ModTime: e.ModTime(), IsDir: e.IsDir()}) |
| ... | ... | @@ -133,7 +133,6 @@ func TestReadDirNormalizesListing(t *testing.T) { | |
| 133 | 133 | &mockFileInfo{name: "index.html", size: 10}, | |
| 134 | 134 | &mockFileInfo{name: "/css/site.css", size: 5}, | |
| 135 | 135 | &mockFileInfo{name: "css", isDir: true}, | |
| 136 | - | &mockFileInfo{name: "empty/._pico_keep_dir"}, | |
| 137 | 136 | &mockFileInfo{name: "empty.txt"}, | |
| 138 | 137 | }, | |
| 139 | 138 | }} |
+42,
-1
| ... | ... | @@ -235,12 +235,53 @@ func TestStorageConformance(t *testing.T) { | |
| 235 | 235 | t.Fatalf("StatObject after delete returned %v", err) | |
| 236 | 236 | } | |
| 237 | 237 | got := list(t, st, b, "/proj/css/", true) | |
| 238 | - | want := []string{entry("site.css", false, 15, t1)} | |
| 238 | + | want := []string{"deep/", entry("site.css", false, 15, t1)} | |
| 239 | 239 | if !slices.Equal(got, want) { | |
| 240 | 240 | t.Fatalf("after delete got %q, want %q", got, want) | |
| 241 | 241 | } | |
| 242 | 242 | }) | |
| 243 | 243 | ||
| 244 | + | t.Run("directories", func(t *testing.T) { | |
| 245 | + | if err := st.PutDir(b, "/proj/empty/nested"); err != nil { | |
| 246 | + | t.Fatal(err) | |
| 247 | + | } | |
| 248 | + | if got, want := list(t, st, b, "/proj/empty/", true), []string{"nested/"}; !slices.Equal(got, want) { | |
| 249 | + | t.Fatalf("list after PutDir got %q, want %q", got, want) | |
| 250 | + | } | |
| 251 | + | if got := list(t, st, b, "/proj/empty/nested", false); !slices.Equal(got, []string{"/"}) { | |
| 252 | + | t.Fatalf("empty directory not found: %q", got) | |
| 253 | + | } | |
| 254 | + | if _, err := st.StatObject(b, "/proj/empty/nested"); !errors.Is(err, fs.ErrNotExist) { | |
| 255 | + | t.Fatalf("StatObject on a directory returned %v", err) | |
| 256 | + | } | |
| 257 | + | ||
| 258 | + | if err := st.DeleteObject(b, "/proj/empty"); err == nil { | |
| 259 | + | t.Fatal("deleting a non-empty directory succeeded") | |
| 260 | + | } | |
| 261 | + | for _, dir := range []string{"/proj/empty/nested", "/proj/empty", "/proj/css/deep"} { | |
| 262 | + | if err := st.DeleteObject(b, dir); err != nil { | |
| 263 | + | t.Fatalf("delete %s: %v", dir, err) | |
| 264 | + | } | |
| 265 | + | if got := list(t, st, b, dir, false); len(got) != 0 { | |
| 266 | + | t.Fatalf("%s still listed as %q", dir, got) | |
| 267 | + | } | |
| 268 | + | } | |
| 269 | + | ||
| 270 | + | put(t, st, b, "/solo/only.txt", []byte("x"), t1) | |
| 271 | + | if err := st.DeleteObject(b, "/solo/only.txt"); err != nil { | |
| 272 | + | t.Fatal(err) | |
| 273 | + | } | |
| 274 | + | if got := list(t, st, b, "/", false); !slices.Contains(got, "solo/") { | |
| 275 | + | t.Fatalf("directory went with its last file: %q", got) | |
| 276 | + | } | |
| 277 | + | if err := st.DeleteObject(b, "/solo"); err != nil { | |
| 278 | + | t.Fatal(err) | |
| 279 | + | } | |
| 280 | + | if got := list(t, st, b, "/", false); slices.Contains(got, "solo/") { | |
| 281 | + | t.Fatalf("deleted directory still listed: %q", got) | |
| 282 | + | } | |
| 283 | + | }) | |
| 284 | + | ||
| 244 | 285 | t.Run("quota", func(t *testing.T) { | |
| 245 | 286 | size, err := st.GetBucketQuota(b) | |
| 246 | 287 | if err != nil { |
+17,
-40
| ... | ... | @@ -35,6 +35,10 @@ func dirSize(path string) (int64, error) { | |
| 35 | 35 | return size, err | |
| 36 | 36 | } | |
| 37 | 37 | ||
| 38 | + | // legacyKeepDir is the marker pgs used to write to keep empty directories. | |
| 39 | + | // Listings hide it until cmd/scripts/clean-keep-dirs has removed them all. | |
| 40 | + | const legacyKeepDir = "._pico_keep_dir" | |
| 41 | + | ||
| 38 | 42 | type StorageFS struct { | |
| 39 | 43 | Dir string | |
| 40 | 44 | Logger *slog.Logger |
| ... | ... | @@ -172,51 +176,18 @@ func (s *StorageFS) PutObject(bucket Bucket, fpath string, contents io.Reader, i | |
| 172 | 176 | return loc, size, nil | |
| 173 | 177 | } | |
| 174 | 178 | ||
| 179 | + | func (s *StorageFS) PutDir(bucket Bucket, dir string) error { | |
| 180 | + | return os.MkdirAll(filepath.Join(bucket.Path, dir), os.ModePerm) | |
| 181 | + | } | |
| 182 | + | ||
| 175 | 183 | func (s *StorageFS) DeleteObject(bucket Bucket, fpath string) error { | |
| 176 | 184 | loc := filepath.Join(bucket.Path, fpath) | |
| 185 | + | // A leftover marker would keep an otherwise empty directory in place. | |
| 186 | + | _ = os.Remove(filepath.Join(loc, legacyKeepDir)) | |
| 177 | 187 | err := os.Remove(loc) | |
| 178 | - | if err != nil { | |
| 179 | - | if os.IsNotExist(err) { | |
| 180 | - | return nil | |
| 181 | - | } | |
| 188 | + | if err != nil && !os.IsNotExist(err) { | |
| 182 | 189 | return err | |
| 183 | 190 | } | |
| 184 | - | ||
| 185 | - | // traverse up the folder tree and remove all empty folders | |
| 186 | - | dir := filepath.Dir(loc) | |
| 187 | - | for dir != "" { | |
| 188 | - | f, err := os.Open(dir) | |
| 189 | - | if err != nil { | |
| 190 | - | s.Logger.Info("open dir", "dir", dir, "err", err) | |
| 191 | - | break | |
| 192 | - | } | |
| 193 | - | defer func() { | |
| 194 | - | _ = f.Close() | |
| 195 | - | }() | |
| 196 | - | ||
| 197 | - | // https://stackoverflow.com/a/30708914 | |
| 198 | - | contents, err := f.Readdirnames(-1) | |
| 199 | - | if err != nil { | |
| 200 | - | s.Logger.Info("read dir", "dir", dir, "err", err) | |
| 201 | - | break | |
| 202 | - | } | |
| 203 | - | if len(contents) > 0 { | |
| 204 | - | break | |
| 205 | - | } | |
| 206 | - | ||
| 207 | - | err = os.Remove(dir) | |
| 208 | - | if err != nil { | |
| 209 | - | s.Logger.Info("remove dir", "dir", dir, "err", err) | |
| 210 | - | break | |
| 211 | - | } | |
| 212 | - | fp := strings.Split(dir, "/") | |
| 213 | - | prefix := "" | |
| 214 | - | if strings.HasPrefix(loc, "/") { | |
| 215 | - | prefix = "/" | |
| 216 | - | } | |
| 217 | - | dir = prefix + filepath.Join(fp[:len(fp)-1]...) | |
| 218 | - | } | |
| 219 | - | ||
| 220 | 191 | return nil | |
| 221 | 192 | } | |
| 222 | 193 |
| ... | ... | @@ -267,6 +238,9 @@ func (s *StorageFS) ListObjects(bucket Bucket, dir string, recursive bool) ([]os | |
| 267 | 238 | if err != nil { | |
| 268 | 239 | return err | |
| 269 | 240 | } | |
| 241 | + | if d.Name() == legacyKeepDir { | |
| 242 | + | return nil | |
| 243 | + | } | |
| 270 | 244 | info, err := d.Info() | |
| 271 | 245 | if err != nil { | |
| 272 | 246 | return nil |
| ... | ... | @@ -296,6 +270,9 @@ func (s *StorageFS) ListObjects(bucket Bucket, dir string, recursive bool) ([]os | |
| 296 | 270 | return fileList, nil | |
| 297 | 271 | } | |
| 298 | 272 | for _, d := range fls { | |
| 273 | + | if d.Name() == legacyKeepDir { | |
| 274 | + | continue | |
| 275 | + | } | |
| 299 | 276 | info, err := d.Info() | |
| 300 | 277 | if err != nil { | |
| 301 | 278 | continue |
+42,
-11
| ... | ... | @@ -179,10 +179,10 @@ func TestFsAdapter(t *testing.T) { | |
| 179 | 179 | t.Fatal("file should have been deleted") | |
| 180 | 180 | } | |
| 181 | 181 | ||
| 182 | - | // ensure containing folder was also deleted | |
| 182 | + | // the containing folder stays, as it would on any filesystem | |
| 183 | 183 | _, err = os.Stat(filepath.Join(bucket.Path, "nice")) | |
| 184 | - | if !os.IsNotExist(err) { | |
| 185 | - | t.Fatal("containing folder should have been deleted") | |
| 184 | + | if err != nil { | |
| 185 | + | t.Fatal("containing folder should have been kept") | |
| 186 | 186 | } | |
| 187 | 187 | ||
| 188 | 188 | // it should not error if file doesn't exist |
| ... | ... | @@ -200,18 +200,14 @@ func TestFsAdapter(t *testing.T) { | |
| 200 | 200 | t.Fatal(err) | |
| 201 | 201 | } | |
| 202 | 202 | ||
| 203 | - | // delete deeply nested file and all parent folders that are now empty | |
| 203 | + | // delete deeply nested file; its now empty parent folders stay | |
| 204 | 204 | err = st.DeleteObject(bucket, "here/yes/we/can.txt") | |
| 205 | 205 | if err != nil { | |
| 206 | 206 | t.Fatal(err) | |
| 207 | 207 | } | |
| 208 | - | _, err = os.Stat(filepath.Join(bucket.Path, "here")) | |
| 209 | - | if os.IsNotExist(err) { | |
| 210 | - | t.Fatal("this folder had multiple files and should not have been deleted") | |
| 211 | - | } | |
| 212 | - | _, err = os.Stat(filepath.Join(bucket.Path, "here/yes")) | |
| 213 | - | if !os.IsNotExist(err) { | |
| 214 | - | t.Fatal("containing folder should have been deleted") | |
| 208 | + | _, err = os.Stat(filepath.Join(bucket.Path, "here/yes/we")) | |
| 209 | + | if err != nil { | |
| 210 | + | t.Fatal("containing folder should have been kept") | |
| 215 | 211 | } | |
| 216 | 212 | ||
| 217 | 213 | // delete bucket even with file contents |
| ... | ... | @@ -220,3 +216,38 @@ func TestFsAdapter(t *testing.T) { | |
| 220 | 216 | t.Fatal(err) | |
| 221 | 217 | } | |
| 222 | 218 | } | |
| 219 | + | ||
| 220 | + | func TestFsLegacyKeepDir(t *testing.T) { | |
| 221 | + | st, err := NewStorageFS(slog.New(slog.DiscardHandler), t.TempDir()) | |
| 222 | + | if err != nil { | |
| 223 | + | t.Fatal(err) | |
| 224 | + | } | |
| 225 | + | bucket, err := st.UpsertBucket("legacy") | |
| 226 | + | if err != nil { | |
| 227 | + | t.Fatal(err) | |
| 228 | + | } | |
| 229 | + | for _, name := range []string{"proj/empty/" + legacyKeepDir, "proj/index.html"} { | |
| 230 | + | if _, _, err := st.PutObject(bucket, name, strings.NewReader(""), &ObjectInfo{}); err != nil { | |
| 231 | + | t.Fatal(err) | |
| 232 | + | } | |
| 233 | + | } | |
| 234 | + | ||
| 235 | + | for _, recursive := range []bool{false, true} { | |
| 236 | + | infos, err := st.ListObjects(bucket, "proj/", recursive) | |
| 237 | + | if err != nil { | |
| 238 | + | t.Fatal(err) | |
| 239 | + | } | |
| 240 | + | for _, info := range infos { | |
| 241 | + | if strings.Contains(info.Name(), legacyKeepDir) { | |
| 242 | + | t.Fatalf("listing (recursive=%v) shows %s", recursive, info.Name()) | |
| 243 | + | } | |
| 244 | + | } | |
| 245 | + | } | |
| 246 | + | ||
| 247 | + | if err := st.DeleteObject(bucket, "proj/empty"); err != nil { | |
| 248 | + | t.Fatalf("deleting a directory holding only a marker: %v", err) | |
| 249 | + | } | |
| 250 | + | if _, err := os.Stat(filepath.Join(bucket.Path, "proj/empty")); !os.IsNotExist(err) { | |
| 251 | + | t.Fatalf("directory still exists: %v", err) | |
| 252 | + | } | |
| 253 | + | } |
+60,
-6
| ... | ... | @@ -6,6 +6,7 @@ import ( | |
| 6 | 6 | "io" | |
| 7 | 7 | "io/fs" | |
| 8 | 8 | "os" | |
| 9 | + | "path" | |
| 9 | 10 | "path/filepath" | |
| 10 | 11 | "strings" | |
| 11 | 12 | "sync" |
| ... | ... | @@ -51,6 +52,22 @@ func memoryKey(fpath string) string { | |
| 51 | 52 | return fpath | |
| 52 | 53 | } | |
| 53 | 54 | ||
| 55 | + | // dirKey is the key that keeps a directory listed while it's empty. Keys are | |
| 56 | + | // flat like S3's, so a directory is a "dir/" key with no contents. | |
| 57 | + | func dirKey(dir string) string { | |
| 58 | + | return strings.TrimSuffix(memoryKey(dir), "/") + "/" | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | // hasPrefix reports whether any key other than skip starts with prefix. | |
| 62 | + | func hasPrefix(objects map[string]string, prefix, skip string) bool { | |
| 63 | + | for key := range objects { | |
| 64 | + | if key != skip && strings.HasPrefix(key, prefix) { | |
| 65 | + | return true | |
| 66 | + | } | |
| 67 | + | } | |
| 68 | + | return false | |
| 69 | + | } | |
| 70 | + | ||
| 54 | 71 | func (s *StorageMemory) GetBucket(name string) (Bucket, error) { | |
| 55 | 72 | s.mu.RLock() | |
| 56 | 73 | defer s.mu.RUnlock() |
| ... | ... | @@ -111,7 +128,7 @@ func (s *StorageMemory) GetObject(bucket Bucket, fpath string) (utils.ReadAndRea | |
| 111 | 128 | objInfo := &ObjectInfo{} | |
| 112 | 129 | ||
| 113 | 130 | dat, ok := s.storage[bucket.Path][fpath] | |
| 114 | - | if !ok { | |
| 131 | + | if !ok || strings.HasSuffix(fpath, "/") { | |
| 115 | 132 | return nil, objInfo, fmt.Errorf("object does not exist: %s", fpath) | |
| 116 | 133 | } | |
| 117 | 134 |
| ... | ... | @@ -126,7 +143,7 @@ func (s *StorageMemory) StatObject(bucket Bucket, fpath string) (*ObjectInfo, er | |
| 126 | 143 | ||
| 127 | 144 | fpath = memoryKey(fpath) | |
| 128 | 145 | dat, ok := s.storage[bucket.Path][fpath] | |
| 129 | - | if !ok { | |
| 146 | + | if !ok || strings.HasSuffix(fpath, "/") { | |
| 130 | 147 | return nil, fmt.Errorf("object does not exist: %s: %w", fpath, fs.ErrNotExist) | |
| 131 | 148 | } | |
| 132 | 149 | return &ObjectInfo{ |
| ... | ... | @@ -157,13 +174,42 @@ func (s *StorageMemory) PutObject(bucket Bucket, fpath string, contents io.Reade | |
| 157 | 174 | return fmt.Sprintf("%s%s", bucket.Path, fpath), int64(len(d)), nil | |
| 158 | 175 | } | |
| 159 | 176 | ||
| 177 | + | func (s *StorageMemory) PutDir(bucket Bucket, dir string) error { | |
| 178 | + | s.mu.Lock() | |
| 179 | + | defer s.mu.Unlock() | |
| 180 | + | ||
| 181 | + | key := dirKey(dir) | |
| 182 | + | if key == "/" { | |
| 183 | + | return nil | |
| 184 | + | } | |
| 185 | + | s.storage[bucket.Path][key] = "" | |
| 186 | + | s.mtimes[bucket.Path][key] = time.Now() | |
| 187 | + | return nil | |
| 188 | + | } | |
| 189 | + | ||
| 160 | 190 | func (s *StorageMemory) DeleteObject(bucket Bucket, fpath string) error { | |
| 161 | 191 | s.mu.Lock() | |
| 162 | 192 | defer s.mu.Unlock() | |
| 163 | 193 | ||
| 194 | + | objects := s.storage[bucket.Path] | |
| 195 | + | mtimes := s.mtimes[bucket.Path] | |
| 164 | 196 | fpath = memoryKey(fpath) | |
| 165 | - | delete(s.storage[bucket.Path], fpath) | |
| 166 | - | delete(s.mtimes[bucket.Path], fpath) | |
| 197 | + | if _, ok := objects[fpath]; ok && !strings.HasSuffix(fpath, "/") { | |
| 198 | + | delete(objects, fpath) | |
| 199 | + | delete(mtimes, fpath) | |
| 200 | + | if parent := dirKey(path.Dir(fpath)); parent != "/" && !hasPrefix(objects, parent, "") { | |
| 201 | + | objects[parent] = "" | |
| 202 | + | mtimes[parent] = time.Now() | |
| 203 | + | } | |
| 204 | + | return nil | |
| 205 | + | } | |
| 206 | + | ||
| 207 | + | key := dirKey(fpath) | |
| 208 | + | if hasPrefix(objects, key, key) { | |
| 209 | + | return fmt.Errorf("%s: directory not empty", fpath) | |
| 210 | + | } | |
| 211 | + | delete(objects, key) | |
| 212 | + | delete(mtimes, key) | |
| 167 | 213 | return nil | |
| 168 | 214 | } | |
| 169 | 215 |
| ... | ... | @@ -190,7 +236,7 @@ func (s *StorageMemory) ListObjects(bucket Bucket, dir string, recursive bool) ( | |
| 190 | 236 | objects := s.storage[bucket.Path] | |
| 191 | 237 | mtimes := s.mtimes[bucket.Path] | |
| 192 | 238 | ||
| 193 | - | if val, ok := objects[resolved]; ok { | |
| 239 | + | if val, ok := objects[resolved]; ok && !strings.HasSuffix(resolved, "/") { | |
| 194 | 240 | fileList = append(fileList, &utils.VirtualFile{ | |
| 195 | 241 | FName: filepath.Base(resolved), | |
| 196 | 242 | FSize: int64(len(val)), |
| ... | ... | @@ -227,9 +273,17 @@ func (s *StorageMemory) ListObjects(bucket Bucket, dir string, recursive bool) ( | |
| 227 | 273 | continue | |
| 228 | 274 | } | |
| 229 | 275 | } | |
| 230 | - | for d := filepath.Dir(rel); d != "." && !seen[d]; d = filepath.Dir(d) { | |
| 276 | + | name, isDir := strings.CutSuffix(rel, "/") | |
| 277 | + | start := filepath.Dir(name) | |
| 278 | + | if isDir { | |
| 279 | + | start = name | |
| 280 | + | } | |
| 281 | + | for d := start; d != "." && !seen[d]; d = filepath.Dir(d) { | |
| 231 | 282 | addDir(d) | |
| 232 | 283 | } | |
| 284 | + | if isDir { | |
| 285 | + | continue | |
| 286 | + | } | |
| 233 | 287 | fileList = append(fileList, &utils.VirtualFile{ | |
| 234 | 288 | FName: rel, | |
| 235 | 289 | FSize: int64(len(val)), |
+5,
-0
| ... | ... | @@ -34,6 +34,11 @@ type ObjectStorage interface { | |
| 34 | 34 | // StatObject returns an object's info without opening it. | |
| 35 | 35 | StatObject(bucket Bucket, fpath string) (*ObjectInfo, error) | |
| 36 | 36 | PutObject(bucket Bucket, fpath string, contents io.Reader, info *ObjectInfo) (string, int64, error) | |
| 37 | + | // PutDir creates a directory, and its parents, that is listed even | |
| 38 | + | // while it's empty. | |
| 39 | + | PutDir(bucket Bucket, dir string) error | |
| 40 | + | // DeleteObject removes a file or an empty directory. Deleting a missing | |
| 41 | + | // path succeeds. A directory stays after its last file is deleted. | |
| 37 | 42 | DeleteObject(bucket Bucket, fpath string) error | |
| 38 | 43 | ListObjects(bucket Bucket, dir string, recursive bool) ([]os.FileInfo, error) | |
| 39 | 44 | } |