v7 Proposal

[funkyshu]

Proposed changes to vfs in next major release

This is meant to be a starting point for discussion, not a final draft. Nothing int his discussion should be thought of as set in stone. Indeed, much of what is initially added will need to be fleshed out further.

Improve Standardization between backends

Create new behavior-driven test suite

Currently each backend implementation have mostly the same behavior but because a rickety and incomplete test suite, it's impossible to ensure compliance. There are some subtle differences with authority behavior that needs to be worked in the test suite. For instance, os and mem allow for empty authority (volume) while the reset do not.

Proposed

• create behavior drive tests that can be applied against any backend
• suite should test the public API, the interfaces.

Standardize Errors

To help facilitate the proposed test suite, we should be using standard errors that can be universally checked using Go 1.13+ errors functions (Is, As, Unwrap). I really like Dave Cheney's thoughts on this https://dave.cheney.net/tag/error-handling about creating immutable but most importantly fungible errors.

Proposed

• create sentinel error constants
• use fmt.Errorf("%w") error wrapping when error decoration is desired
• use go 1.13+ error functions (Is, As, Unwrap) in tests and code where possible

Breaking Changes

Deprecate Volume and ChangeDir terminology and functions

Initial development of vfs tried to constrain functionality to what we could accomplish in all filesystems we could think of (ie., unix os, windows os [we are dismally broken in windows], AWS S3, Google Storage, Azure Blob storage, SFTP, etc). We decided that when default behaviors between filesystems conflicted, we'd choose the os behavior as pattern we'd most closely follow. Along with this, we borrowed some terminology from os that may not apply everywhere, including File, Volume, ChangeDir. However, as it turns out, that while our use of URI as the means for resource locating has worked extremely well, the terms Volume and ChangeDir are not universally applicable.

Volume

     scheme             host
     __/             ___/____  port
    /  \            /        \ /\
    sftp://[email protected]:22/path/to/file.txt
           \____________________/\_______________/
           \______/       \               \
               /     authority section    path
         username       (Volume)

In the pattern above, for sftp (and ftp) authority is user + host + port. For s3 and gs the authority is a bucket. For mem, namespaces are the optional authority. For os (mainly for windows, unix doesn't distinguish between volumes and directories), it is optionally a volume or mount. Azure is complicated because of its hostname and its container concept and its URI pattern we might revisit but will remain out-of-scope for this proposal.

ChangDir

ChangeDir(relLocPath) is the only place we actually use the term "Dir" (vs Location). It updates the existing Location's path to the provided relative location path. As far as I can see, its behavior is redundant.
We can accomplish the same thing by doing:

// change location
changedDir, err := currentDir.NewLocation(relLocPath)

I can't see a valid reason for keeping this in the Location interface. Smaller interface = better interface.

Proposed:

• add Authority() function
• update all internal references from Volume() to Authority
• deprecate Volume() function in v7 to be removed in v8.
• replace all documentation reference to Volume both in public functions and concept be changes to use authority.
• deprecate ChangDir to be removed in v8

Support ​context.Context

Most network-based backends' underlying API's support context but vfs does not. We should add context to support to our interface, providing timeout and cancellation. This would largely entail adding a ctx to the front of most arg lists.

Proposed:

• Add clone methods that support context.
• Point the old methods to the new ones passing a todo context.
• Deprecate old methods in v7. Remove in v8 or later.

Redesign Options

We currently pass options an empty interface called vfs.Options and then cast it as implementation-specific stuct like s3.Options, applying globally.
We should consider using variadic functional options instead (https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis) and apply them in various contexts. In other words, we could have Filesystem-level options, Location-level options, and File-level.

type FilesytemOptions func(fs *s3.Filesystem)
type FileOptions func(fs *s3.File)

func DoSomething() error {
        // here, anything satisfies the s3FilesyetemOptions interface is acceptable
        fsOptions := []s3.FilesyetemOptions{
        	s3.WithAuth(myAuthSettings),
		s3.WithLogger(someLogger),
		s3.WithRetryer(myRetryLogic),
		s3.WithGlobalSSE(),
		s3.WithGlobalFilePermissions('0755'),
	}
	fs := s3.NewFileSystem(fsOptions...)

	existingFile, _ := fs.NewFile("mybucket", "/path/to/location/file1.txt")

	fileOptions := []FileOptions{
		s3.WithFilePermissions('0644'),
	}
	newfile, _ := fs.NewFile("mybucket", "/path/to/location/file2.txt", fileOptions...)

	err := existingFile.CopyToFile(newfile)
}

Here we've set some global option including authentications, logging, retry, at-rest encryption, and file permissions defaults. However, when setting up newfile we decide to override the global settings a different set of permissions.
Note that existingFile didn't have to pass any options because the variadic signature allowed for passing nothing. So it will just use any relevant global settings.

Proposed

• add variadic optional function signature to relevant functions
• create some Optional Functions for existing options

Redesign List functions

TBD

[seeroush]

Great suggestions! So for adding context to each method signature, are you saying that it'd look something like this in the deprecation phase?

v7 - add Context version of method

// Deprecated: old function
func (*File) Write([]byte) error {
   f.Write(context.Background(), []bytes))
}

// WriteWithContext - use me instead
func (*File) WriteWithContext(ctx, []byte) error { ... }

v8 - rename back to Write()

// Write - use me instead
func (*File) Write(ctx, []byte) error { ... }

[funkyshu]

@seeroush
That IS what I was suggesting but, honestly, I hadn't really thought it through. Personally, I'd prefer a hard cutover rather than introduce a new function that we'd just have to deprecate again. :/

[funkyshu]

Altering Read or Write is actually a bad idea since it would break the implementation of the io.* interfaces :(

[funkyshu]

In the last several years, google released readonly filesystem interface in package io/fs called fs.FS that could easily be adopted into the vfs with little effort and no breaking changes.