Browse Source

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.
Tommy van der Vorst 6 months ago
parent
commit
d23cd197e1
11 changed files with 33 additions and 69 deletions
  1. 0 4
      lib/fs/basicfs.go
  2. 0 4
      lib/fs/casefs.go
  3. 13 9
      lib/fs/casefs_test.go
  4. 0 4
      lib/fs/errorfs.go
  5. 0 4
      lib/fs/fakefs.go
  6. 16 20
      lib/fs/filesystem.go
  7. 0 4
      lib/fs/logfs.go
  8. 0 4
      lib/fs/metrics.go
  9. 1 9
      lib/fs/mtimefs.go
  10. 3 3
      lib/fs/mtimefs_test.go
  11. 0 4
      lib/fs/walkfs.go

+ 0 - 4
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

+ 0 - 4
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 {

+ 13 - 9
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)

+ 0 - 4
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
-}

+ 0 - 4
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{}

+ 16 - 20
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 {
+			var x T
+			return x, false
+		}
+
+		fs, ok = wrappingFs.underlying()
 		if !ok {
-			return nil, false
+			var x T
+			return x, false
 		}
 	}
 }

+ 0 - 4
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
-}

+ 0 - 4
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

+ 1 - 9
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)
 }

+ 3 - 3
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
 }

+ 0 - 4
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
-}