updated tests

core/app.go

@@ -1264,6 +1264,7 @@ type App interface {
 
 	// ---------------------------------------------------------------
 	// Filesystem event hooks
+	// (not publicly exposed until file_field refactoring)
 	// ---------------------------------------------------------------
 
 	// onFilesystemNewWriter is an internal hook for app.NewFilesystem()

core/backup_create.go

@@ -299,7 +299,7 @@ func copyFileToZip(w *zip.Writer, localPath string, zipPath string) error {
 	}
 
 	if info.IsDir() {
-		return nil
+		return fmt.Errorf("%s is a directory and not a regular file", localPath)
 	}
 
 	h, err := zip.FileInfoHeader(info)

tools/filesystem/filesystem.go

@@ -26,7 +26,7 @@ import (
 	"github.com/pocketbase/pocketbase/tools/list"
 	"github.com/pocketbase/pocketbase/tools/routine"
 
-	// explicit webp decoder because disintegration/imaging does not support webp
+	// manually register the webp decoder because disintegration/imaging does not support webp
 	_ "golang.org/x/image/webp"
 )
 
@@ -55,6 +55,7 @@ type System struct {
 	ctx    context.Context
 	bucket *blob.Bucket
 
+	// @todo consider with the refactoring to bind on driver level
 	onNewWriter *hook.Hook[*NewWriterEvent]
 	onDelete    *hook.Hook[*DeleteEvent]
 }
@@ -117,14 +118,23 @@ func (s *System) SetContext(ctx context.Context) {
 
 // Close releases any resources used for the related filesystem.
 func (s *System) Close() error {
-	s.onNewWriter = nil
-	s.onDelete = nil
+	if s.onNewWriter != nil {
+		s.onNewWriter.UnbindAll()
+		s.onNewWriter = nil
+	}
+
+	if s.onDelete != nil {
+		s.onDelete.UnbindAll()
+		s.onDelete = nil
+	}
 
 	return s.bucket.Close()
 }
 
 // OnNewWriter is a low level hook that is triggered on every writer initialization
 // (aka. when attempting to create a new file with [system.NewWriter], [system.Upload], etc.).
+//
+// Note that currently it doesn't trigger on [System.Copy] but this may change in future releases.
 func (s *System) OnNewWriter() *hook.Hook[*NewWriterEvent] {
 	if s.onNewWriter == nil {
 		s.onNewWriter = &hook.Hook[*NewWriterEvent]{}

tools/filesystem/filesystem_test.go

@@ -30,6 +30,8 @@ func TestFilesystemExists(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	scenarios := []struct {
 		file   string
 		exists bool
@@ -53,6 +55,8 @@ func TestFilesystemExists(t *testing.T) {
 			}
 		})
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 0})
 }
 
 func TestFilesystemAttributes(t *testing.T) {
@@ -65,6 +69,8 @@ func TestFilesystemAttributes(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	scenarios := []struct {
 		file              string
 		expectError       bool
@@ -95,6 +101,8 @@ func TestFilesystemAttributes(t *testing.T) {
 			}
 		})
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 0})
 }
 
 func TestFilesystemNewWriter(t *testing.T) {
@@ -107,6 +115,8 @@ func TestFilesystemNewWriter(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	testKey := "test_writer"
 
 	opts := &blob.WriterOptions{
@@ -156,6 +166,8 @@ func TestFilesystemNewWriter(t *testing.T) {
 	if result != expectedContent {
 		t.Fatalf("Expected file content\n%s\ngot\n%s", expectedContent, result)
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 1})
 }
 
 func TestFilesystemDelete(t *testing.T) {
@@ -168,6 +180,8 @@ func TestFilesystemDelete(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	if err := fsys.Delete("missing.txt"); err == nil || !errors.Is(err, filesystem.ErrNotFound) {
 		t.Fatalf("Expected ErrNotFound error, got %v", err)
 	}
@@ -175,6 +189,8 @@ func TestFilesystemDelete(t *testing.T) {
 	if err := fsys.Delete("image.png"); err != nil {
 		t.Fatalf("Expected nil, got error %v", err)
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 2, "OnNewWriter": 0})
 }
 
 func TestFilesystemDeletePrefixWithoutTrailingSlash(t *testing.T) {
@@ -187,6 +203,8 @@ func TestFilesystemDeletePrefixWithoutTrailingSlash(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	if errs := fsys.DeletePrefix(""); len(errs) == 0 {
 		t.Fatal("Expected error, got nil", errs)
 	}
@@ -211,6 +229,8 @@ func TestFilesystemDeletePrefixWithoutTrailingSlash(t *testing.T) {
 	if _, err := os.Stat(filepath.Join(dir, "test")); os.IsNotExist(err) {
 		t.Fatal("Expected the prefix dir to remain")
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 2, "OnNewWriter": 0})
 }
 
 func TestFilesystemDeletePrefixWithTrailingSlash(t *testing.T) {
@@ -223,6 +243,8 @@ func TestFilesystemDeletePrefixWithTrailingSlash(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	if errs := fsys.DeletePrefix("missing/"); len(errs) != 0 {
 		t.Fatalf("Not existing prefix shouldn't error, got %v", errs)
 	}
@@ -243,6 +265,8 @@ func TestFilesystemDeletePrefixWithTrailingSlash(t *testing.T) {
 	if _, err := os.Stat(filepath.Join(dir, "test")); !os.IsNotExist(err) {
 		t.Fatal("Expected the prefix dir to be deleted")
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 4, "OnNewWriter": 0})
 }
 
 func TestFilesystemIsEmptyDir(t *testing.T) {
@@ -255,6 +279,8 @@ func TestFilesystemIsEmptyDir(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	scenarios := []struct {
 		dir      string
 		expected bool
@@ -278,6 +304,8 @@ func TestFilesystemIsEmptyDir(t *testing.T) {
 			}
 		})
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 0})
 }
 
 func TestFilesystemUploadMultipart(t *testing.T) {
@@ -310,6 +338,8 @@ func TestFilesystemUploadMultipart(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	fileKey := "newdir/newkey.txt"
 
 	uploadErr := fsys.UploadMultipart(fh, fileKey)
@@ -328,6 +358,8 @@ func TestFilesystemUploadMultipart(t *testing.T) {
 	if name, ok := attrs.Metadata["original-filename"]; !ok || name != "test" {
 		t.Fatalf("Expected original-filename to be %q, got %q", "test", name)
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 1})
 }
 
 func TestFilesystemUploadFile(t *testing.T) {
@@ -340,6 +372,8 @@ func TestFilesystemUploadFile(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	fileKey := "newdir/newkey.txt"
 
 	file, err := filesystem.NewFileFromPath(filepath.Join(dir, "image.svg"))
@@ -365,6 +399,8 @@ func TestFilesystemUploadFile(t *testing.T) {
 	if name, ok := attrs.Metadata["original-filename"]; !ok || name != file.OriginalName {
 		t.Fatalf("Expected original-filename to be %q, got %q", file.OriginalName, name)
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 1})
 }
 
 func TestFilesystemUpload(t *testing.T) {
@@ -377,6 +413,8 @@ func TestFilesystemUpload(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	fileKey := "newdir/newkey.txt"
 
 	uploadErr := fsys.Upload([]byte("demo"), fileKey)
@@ -387,6 +425,8 @@ func TestFilesystemUpload(t *testing.T) {
 	if exists, _ := fsys.Exists(fileKey); !exists {
 		t.Fatalf("Expected %s to exist", fileKey)
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 1})
 }
 
 func TestFilesystemServe(t *testing.T) {
@@ -399,6 +439,8 @@ func TestFilesystemServe(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	csp := "default-src 'none'; media-src 'self'; style-src 'unsafe-inline'; sandbox"
 	cacheControl := "max-age=2592000, stale-while-revalidate=86400"
 
@@ -631,6 +673,8 @@ func TestFilesystemServe(t *testing.T) {
 			}
 		})
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 0})
 }
 
 func TestFilesystemGetReader(t *testing.T) {
@@ -643,6 +687,8 @@ func TestFilesystemGetReader(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	scenarios := []struct {
 		file            string
 		expectError     bool
@@ -683,6 +729,8 @@ func TestFilesystemGetReader(t *testing.T) {
 			}
 		})
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 0})
 }
 
 func TestFilesystemGetReuploadableFile(t *testing.T) {
@@ -695,6 +743,8 @@ func TestFilesystemGetReuploadableFile(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	t.Run("missing.txt", func(t *testing.T) {
 		_, err := fsys.GetReuploadableFile("missing.txt", false)
 		if err == nil {
@@ -761,6 +811,8 @@ func TestFilesystemGetReuploadableFile(t *testing.T) {
 
 		testReader(t, file, "sub1")
 	})
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 0})
 }
 
 func TestFilesystemCopy(t *testing.T) {
@@ -773,6 +825,8 @@ func TestFilesystemCopy(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	src := "image.png"
 	dst := "image.png_copy"
 
@@ -795,6 +849,8 @@ func TestFilesystemCopy(t *testing.T) {
 	if f.Size() != 77 {
 		t.Fatalf("Expected file size %d, got %d", 77, f.Size())
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 0})
 }
 
 func TestFilesystemList(t *testing.T) {
@@ -807,6 +863,8 @@ func TestFilesystemList(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	scenarios := []struct {
 		prefix   string
 		expected []string
@@ -869,6 +927,8 @@ func TestFilesystemList(t *testing.T) {
 			}
 		})
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 0})
 }
 
 func TestFilesystemServeSingleRange(t *testing.T) {
@@ -881,6 +941,8 @@ func TestFilesystemServeSingleRange(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	res := httptest.NewRecorder()
 	req := httptest.NewRequest("GET", "/", nil)
 	req.Header.Add("Range", "bytes=0-20")
@@ -903,6 +965,8 @@ func TestFilesystemServeSingleRange(t *testing.T) {
 	if l := result.Header.Get("Content-Length"); l != "21" {
 		t.Fatalf("Expected Content-Length %v, got %v", 21, l)
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 0})
 }
 
 func TestFilesystemServeMultiRange(t *testing.T) {
@@ -915,6 +979,8 @@ func TestFilesystemServeMultiRange(t *testing.T) {
 	}
 	defer fsys.Close()
 
+	hookCalls := bindHooks(fsys)
+
 	res := httptest.NewRecorder()
 	req := httptest.NewRequest("GET", "/", nil)
 	req.Header.Add("Range", "bytes=0-20, 25-30")
@@ -932,18 +998,14 @@ func TestFilesystemServeMultiRange(t *testing.T) {
 	if ct := result.Header.Get("Content-Type"); !strings.HasPrefix(ct, "multipart/byteranges; boundary=") {
 		t.Fatalf("Expected Content-Type to be multipart/byteranges, got %v", ct)
 	}
+
+	checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 0})
 }
 
 func TestFilesystemCreateThumb(t *testing.T) {
 	dir := createTestDir(t)
 	defer os.RemoveAll(dir)
 
-	fsys, err := filesystem.NewLocal(dir)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer fsys.Close()
-
 	scenarios := []struct {
 		file             string
 		thumb            string
@@ -980,10 +1042,18 @@ func TestFilesystemCreateThumb(t *testing.T) {
 
 	for _, s := range scenarios {
 		t.Run(s.file+"_"+s.thumb+"_"+s.size, func(t *testing.T) {
-			err := fsys.CreateThumb(s.file, s.thumb, s.size)
+			fsys, err := filesystem.NewLocal(dir)
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer fsys.Close()
+
+			hookCalls := bindHooks(fsys)
 
 			expectErr := s.expectedMimeType == ""
 
+			err = fsys.CreateThumb(s.file, s.thumb, s.size)
+
 			hasErr := err != nil
 			if hasErr != expectErr {
 				t.Fatalf("Expected hasErr to be %v, got %v (%v)", expectErr, hasErr, err)
@@ -1014,10 +1084,45 @@ func TestFilesystemCreateThumb(t *testing.T) {
 			if attrsMimeType != s.expectedMimeType {
 				t.Fatalf("Expected thumb attrs %s MimeType %q, got %q", s.thumb, s.expectedMimeType, attrsMimeType)
 			}
+
+			checkHooks(t, hookCalls, map[string]int{"OnDelete": 0, "OnNewWriter": 1})
 		})
 	}
 }
 
+func TestFilesystemClose(t *testing.T) {
+	dir := createTestDir(t)
+	defer os.RemoveAll(dir)
+
+	fsys, err := filesystem.NewLocal(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	bindHooks(fsys)
+
+	if v := fsys.OnDelete().Length(); v != 1 {
+		t.Fatalf("Expected 1 OnDelete listener, got %d", v)
+	}
+
+	if v := fsys.OnNewWriter().Length(); v != 1 {
+		t.Fatalf("Expected 1 OnNewWriter listener, got %d", v)
+	}
+
+	err = fsys.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if v := fsys.OnDelete().Length(); v != 0 {
+		t.Fatalf("Expected 0 OnDelete listeners after close, got %d", v)
+	}
+
+	if v := fsys.OnNewWriter().Length(); v != 0 {
+		t.Fatalf("Expected 0 OnNewWriter listeners after close, got %d", v)
+	}
+}
+
 // ---
 
 func createTestDir(t *testing.T) string {
@@ -1180,3 +1285,28 @@ func createTestDir(t *testing.T) string {
 
 	return dir
 }
+
+func bindHooks(fsys *filesystem.System) map[string]int {
+	hookCalls := map[string]int{}
+
+	fsys.OnDelete().BindFunc(func(e *filesystem.DeleteEvent) error {
+		hookCalls["OnDelete"]++
+		return e.Next()
+	})
+
+	fsys.OnNewWriter().BindFunc(func(e *filesystem.NewWriterEvent) error {
+		hookCalls["OnNewWriter"]++
+		return e.Next()
+	})
+
+	return hookCalls
+}
+
+func checkHooks(t *testing.T, hookCalls, expectations map[string]int) {
+	for event, expected := range expectations {
+		got, _ := hookCalls[event]
+		if got != expected {
+			t.Fatalf("Expected event %q to be called %d, got %d", event, expected, got)
+		}
+	}
+}