From d23cd197e1e06c43106101188e36146f691f4ed5 Mon Sep 17 00:00:00 2001 From: Tommy van der Vorst Date: Tue, 8 Apr 2025 14:39:39 +0200 Subject: [PATCH] chore(fs): changes to allow Filesystem to be implemented externally (#10040) ### Purpose The `fs.Filesystem` interface contains two parts that cannot be implemented externally because they are private: * `filesystemWrapperType`: this PR changes `unwrapFilesystem` to downcast to a specific concrete type * `underlying`: this PR simply moves it to an unexported interface ### Testing Regular tests pass. --- lib/fs/basicfs.go | 4 ---- lib/fs/casefs.go | 4 ---- lib/fs/casefs_test.go | 22 +++++++++++++--------- lib/fs/errorfs.go | 4 ---- lib/fs/fakefs.go | 4 ---- lib/fs/filesystem.go | 36 ++++++++++++++++-------------------- lib/fs/logfs.go | 4 ---- lib/fs/metrics.go | 4 ---- lib/fs/mtimefs.go | 10 +--------- lib/fs/mtimefs_test.go | 6 +++--- lib/fs/walkfs.go | 4 ---- 11 files changed, 33 insertions(+), 69 deletions(-) diff --git a/lib/fs/basicfs.go b/lib/fs/basicfs.go index 225eb8132..098769870 100644 --- a/lib/fs/basicfs.go +++ b/lib/fs/basicfs.go @@ -336,10 +336,6 @@ func (*BasicFilesystem) underlying() (Filesystem, bool) { return nil, false } -func (*BasicFilesystem) wrapperType() filesystemWrapperType { - return filesystemWrapperTypeNone -} - // basicFile implements the fs.File interface on top of an os.File type basicFile struct { *os.File diff --git a/lib/fs/casefs.go b/lib/fs/casefs.go index c8288f8b2..d88ccc0a4 100644 --- a/lib/fs/casefs.go +++ b/lib/fs/casefs.go @@ -357,10 +357,6 @@ func (f *caseFilesystem) underlying() (Filesystem, bool) { return f.Filesystem, true } -func (*caseFilesystem) wrapperType() filesystemWrapperType { - return filesystemWrapperTypeCase -} - func (f *caseFilesystem) checkCase(name string) error { var err error if name, err = Canonicalize(name); err != nil { diff --git a/lib/fs/casefs_test.go b/lib/fs/casefs_test.go index 851d79508..8bd78cb57 100644 --- a/lib/fs/casefs_test.go +++ b/lib/fs/casefs_test.go @@ -161,10 +161,11 @@ func BenchmarkWalkCaseFakeFS100k(b *testing.B) { b.Fatal(err) } b.Run("rawfs", func(b *testing.B) { - var fakefs *fakeFS - if ffs, ok := unwrapFilesystem(fsys, filesystemWrapperTypeNone); ok { - fakefs = ffs.(*fakeFS) + fakefs, ok := unwrapFilesystem[*fakeFS](fsys) + if !ok { + panic("expected unwrap to fakefs") } + fakefs.resetCounters() benchmarkWalkFakeFS(b, fsys, paths, 0, "") fakefs.reportMetricsPerOp(b) @@ -180,9 +181,10 @@ func BenchmarkWalkCaseFakeFS100k(b *testing.B) { cache: newCaseCache(), }, } - var fakefs *fakeFS - if ffs, ok := unwrapFilesystem(fsys, filesystemWrapperTypeNone); ok { - fakefs = ffs.(*fakeFS) + + fakefs, ok := unwrapFilesystem[*fakeFS](fsys) + if !ok { + panic("expected unwrap to fakefs") } fakefs.resetCounters() benchmarkWalkFakeFS(b, casefs, paths, 0, "") @@ -209,10 +211,12 @@ func BenchmarkWalkCaseFakeFS100k(b *testing.B) { cache: newCaseCache(), }, } - var fakefs *fakeFS - if ffs, ok := unwrapFilesystem(fsys, filesystemWrapperTypeNone); ok { - fakefs = ffs.(*fakeFS) + + fakefs, ok := unwrapFilesystem[*fakeFS](fsys) + if !ok { + panic("expected unwrap to fakefs") } + fakefs.resetCounters() benchmarkWalkFakeFS(b, casefs, paths, otherOpEvery, otherOpPath) fakefs.reportMetricsPerOp(b) diff --git a/lib/fs/errorfs.go b/lib/fs/errorfs.go index 8ed3d8040..439efdffd 100644 --- a/lib/fs/errorfs.go +++ b/lib/fs/errorfs.go @@ -69,7 +69,3 @@ func (fs *errorFilesystem) PlatformData(_ string, _, _ bool, _ XattrFilter) (pro func (*errorFilesystem) underlying() (Filesystem, bool) { return nil, false } - -func (*errorFilesystem) wrapperType() filesystemWrapperType { - return filesystemWrapperTypeError -} diff --git a/lib/fs/fakefs.go b/lib/fs/fakefs.go index 9429350eb..af508f18c 100644 --- a/lib/fs/fakefs.go +++ b/lib/fs/fakefs.go @@ -724,10 +724,6 @@ func (*fakeFS) underlying() (Filesystem, bool) { return nil, false } -func (*fakeFS) wrapperType() filesystemWrapperType { - return filesystemWrapperTypeNone -} - func (fs *fakeFS) resetCounters() { fs.mut.Lock() fs.counters = fakeFSCounters{} diff --git a/lib/fs/filesystem.go b/lib/fs/filesystem.go index 2d7ffbadd..fa34fe9fd 100644 --- a/lib/fs/filesystem.go +++ b/lib/fs/filesystem.go @@ -21,18 +21,6 @@ import ( "github.com/syncthing/syncthing/lib/protocol" ) -type filesystemWrapperType int32 - -const ( - filesystemWrapperTypeNone filesystemWrapperType = iota - filesystemWrapperTypeMtime - filesystemWrapperTypeCase - filesystemWrapperTypeError - filesystemWrapperTypeWalk - filesystemWrapperTypeLog - filesystemWrapperTypeMetrics -) - type XattrFilter interface { Permit(string) bool GetMaxSingleEntrySize() int @@ -75,10 +63,11 @@ type Filesystem interface { PlatformData(name string, withOwnership, withXattrs bool, xattrFilter XattrFilter) (protocol.PlatformData, error) GetXattr(name string, xattrFilter XattrFilter) ([]protocol.Xattr, error) SetXattr(path string, xattrs []protocol.Xattr, xattrFilter XattrFilter) error +} +type wrappingFilesystem interface { // Used for unwrapping things underlying() (Filesystem, bool) - wrapperType() filesystemWrapperType } // The File interface abstracts access to a regular file, being a somewhat @@ -353,16 +342,23 @@ func Canonicalize(file string) (string, error) { return file, nil } -// unwrapFilesystem removes "wrapping" filesystems to expose the filesystem of the requested wrapperType, if it exists. -func unwrapFilesystem(fs Filesystem, wrapperType filesystemWrapperType) (Filesystem, bool) { - var ok bool +// unwrapFilesystem removes "wrapping" filesystems to expose the filesystem of the requested wrapper type T, if it exists. +func unwrapFilesystem[T Filesystem](fs Filesystem) (T, bool) { for { - if fs.wrapperType() == wrapperType { - return fs, true + if unwrapped, ok := fs.(T); ok { + return unwrapped, true } - fs, ok = fs.underlying() + + wrappingFs, ok := fs.(wrappingFilesystem) if !ok { - return nil, false + var x T + return x, false + } + + fs, ok = wrappingFs.underlying() + if !ok { + var x T + return x, false } } } diff --git a/lib/fs/logfs.go b/lib/fs/logfs.go index bb90d797f..073a98888 100644 --- a/lib/fs/logfs.go +++ b/lib/fs/logfs.go @@ -177,7 +177,3 @@ func (fs *logFilesystem) Usage(name string) (Usage, error) { func (fs *logFilesystem) underlying() (Filesystem, bool) { return fs.Filesystem, true } - -func (*logFilesystem) wrapperType() filesystemWrapperType { - return filesystemWrapperTypeLog -} diff --git a/lib/fs/metrics.go b/lib/fs/metrics.go index a97660ce0..5e25d6819 100644 --- a/lib/fs/metrics.go +++ b/lib/fs/metrics.go @@ -273,10 +273,6 @@ func (m *metricsFS) underlying() (Filesystem, bool) { return m.next, true } -func (m *metricsFS) wrapperType() filesystemWrapperType { - return filesystemWrapperTypeMetrics -} - type metricsFile struct { fs *metricsFS next File diff --git a/lib/fs/mtimefs.go b/lib/fs/mtimefs.go index ba1d1ecc5..5af16cb2a 100644 --- a/lib/fs/mtimefs.go +++ b/lib/fs/mtimefs.go @@ -146,10 +146,6 @@ func (f *mtimeFS) underlying() (Filesystem, bool) { return f.Filesystem, true } -func (*mtimeFS) wrapperType() filesystemWrapperType { - return filesystemWrapperTypeMtime -} - func (f *mtimeFS) save(name string, real, virtual time.Time) { if f.caseInsensitive { name = UnicodeLowercaseNormalized(name) @@ -255,13 +251,9 @@ func (t *MtimeMapping) Unmarshal(bs []byte) error { } func GetMtimeMapping(fs Filesystem, file string) (MtimeMapping, error) { - fs, ok := unwrapFilesystem(fs, filesystemWrapperTypeMtime) + mtimeFs, ok := unwrapFilesystem[*mtimeFS](fs) if !ok { return MtimeMapping{}, errors.New("failed to unwrap") } - mtimeFs, ok := fs.(*mtimeFS) - if !ok { - return MtimeMapping{}, errors.New("unwrapping failed") - } return mtimeFs.load(file) } diff --git a/lib/fs/mtimefs_test.go b/lib/fs/mtimefs_test.go index 22f02b383..4e0687666 100644 --- a/lib/fs/mtimefs_test.go +++ b/lib/fs/mtimefs_test.go @@ -261,7 +261,7 @@ func newMtimeFS(path string, db database, options ...MtimeFSOption) *mtimeFS { func newMtimeFSWithWalk(path string, db database, options ...MtimeFSOption) (*mtimeFS, *walkFilesystem) { fs := NewFilesystem(FilesystemTypeBasic, path, NewMtimeOption(db, options...)) - wfs, _ := unwrapFilesystem(fs, filesystemWrapperTypeWalk) - mfs, _ := unwrapFilesystem(fs, filesystemWrapperTypeMtime) - return mfs.(*mtimeFS), wfs.(*walkFilesystem) + wfs, _ := unwrapFilesystem[*walkFilesystem](fs) + mfs, _ := unwrapFilesystem[*mtimeFS](fs) + return mfs, wfs } diff --git a/lib/fs/walkfs.go b/lib/fs/walkfs.go index 11d0203d6..36bce97a7 100644 --- a/lib/fs/walkfs.go +++ b/lib/fs/walkfs.go @@ -153,7 +153,3 @@ func (f *walkFilesystem) Walk(root string, walkFn WalkFunc) error { func (f *walkFilesystem) underlying() (Filesystem, bool) { return f.Filesystem, true } - -func (*walkFilesystem) wrapperType() filesystemWrapperType { - return filesystemWrapperTypeWalk -}