Skip to content

Commit

Permalink
Option to specify explicit ContentType on file create (#208)
Browse files Browse the repository at this point in the history
* Shorten DeleteAllVersions API

* New file ContentType option

* Added changlog entry. udpated s3 and gs backends to use src content-type when copying to same target type, like azure does.

* fix lint issue

---------

Co-authored-by: John Judd <[email protected]>
  • Loading branch information
NathanBaulch and funkyshu authored Jan 8, 2025
1 parent 27306f4 commit ff6bf4d
Show file tree
Hide file tree
Showing 46 changed files with 401 additions and 142 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]
### Added
- Added NewFile option for Content-Type.
- Windows support in the os backend.
### Fixed
- Ability to run all unit tests on Windows.
- Deprecated delete.WithDeleteAllVersions in favor of delete.WithAllVersions.

## [6.24.0] - 2024-12-16
### Security
Expand Down
6 changes: 3 additions & 3 deletions backend/azure/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type Client interface {

// Upload should create or update the blob specified by the file parameter with the contents of the content
// parameter
Upload(file vfs.File, content io.ReadSeeker) error
Upload(file vfs.File, content io.ReadSeeker, contentType string) error

// Download should return a reader for the blob specified by the file parameter
Download(file vfs.File) (io.ReadCloser, error)
Expand Down Expand Up @@ -102,15 +102,15 @@ func (a *DefaultClient) Properties(containerURI, filePath string) (*BlobProperti
}

// Upload uploads a new file to Azure Blob Storage
func (a *DefaultClient) Upload(file vfs.File, content io.ReadSeeker) error {
func (a *DefaultClient) Upload(file vfs.File, content io.ReadSeeker, contentType string) error {
URL, err := url.Parse(file.Location().(*Location).ContainerURL())
if err != nil {
return err
}

containerURL := azblob.NewContainerURL(*URL, a.pipeline)
blobURL := containerURL.NewBlockBlobURL(utils.RemoveLeadingSlash(file.Path()))
_, err = blobURL.Upload(context.Background(), content, azblob.BlobHTTPHeaders{}, azblob.Metadata{},
_, err = blobURL.Upload(context.Background(), content, azblob.BlobHTTPHeaders{ContentType: contentType}, azblob.Metadata{},
azblob.BlobAccessConditions{}, azblob.DefaultAccessTier, nil, azblob.ClientProvidedKeyOptions{}, azblob.ImmutabilityPolicyOptions{})
return err
}
Expand Down
17 changes: 9 additions & 8 deletions backend/azure/client_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package azure
import (
"context"
"fmt"
"io"
"net/url"
"os"
"strings"
Expand Down Expand Up @@ -64,7 +65,7 @@ func (s *ClientIntegrationTestSuite) TestAllTheThings_FileWithNoPath() {
s.NoError(err, "Env variables (AZURE_STORAGE_ACCOUNT, AZURE_STORAGE_ACCESS_KEY) should contain valid azure account credentials")

// Create the new file
err = client.Upload(f, strings.NewReader("Hello world!"))
err = client.Upload(f, strings.NewReader("Hello world!"), "")
s.NoError(err, "The file should be successfully uploaded to azure")

// make sure it exists
Expand Down Expand Up @@ -112,7 +113,7 @@ func (s *ClientIntegrationTestSuite) TestAllTheThings_FileWithPath() {
s.NoError(err, "Env variables (AZURE_STORAGE_ACCOUNT, AZURE_STORAGE_ACCESS_KEY) should contain valid azure account credentials")

// create a new file
err = client.Upload(f, strings.NewReader("Hello world!"))
err = client.Upload(f, strings.NewReader("Hello world!"), "")
s.NoError(err, "The file should be successfully uploaded to azure")

// check to see if it exists
Expand Down Expand Up @@ -144,11 +145,11 @@ func (s *ClientIntegrationTestSuite) TestDeleteAllVersions() {
s.NoError(err, "Env variables (AZURE_STORAGE_ACCOUNT, AZURE_STORAGE_ACCESS_KEY) should contain valid azure account credentials")

// Create the new file
err = client.Upload(f, strings.NewReader("Hello!"))
err = client.Upload(f, strings.NewReader("Hello!"), "")
s.NoError(err, "The file should be successfully uploaded to azure")

// Recreate the file
err = client.Upload(f, strings.NewReader("Hello world!"))
err = client.Upload(f, strings.NewReader("Hello world!"), "")
s.NoError(err, "The file should be successfully uploaded to azure")

// make sure it exists
Expand All @@ -172,7 +173,7 @@ func (s *ClientIntegrationTestSuite) TestProperties() {
client, err := fs.Client()
s.NoError(err, "Env variables (AZURE_STORAGE_ACCOUNT, AZURE_STORAGE_ACCESS_KEY) should contain valid azure account credentials")

err = client.Upload(f, strings.NewReader("Hello world!"))
err = client.Upload(f, strings.NewReader("Hello world!"), "")
s.NoError(err, "The file should be successfully uploaded to azure so we shouldn't get an error")
props, err := client.Properties(f.Location().(*Location).ContainerURL(), f.Path())
s.NoError(err, "The file exists so we shouldn't get an error")
Expand All @@ -188,7 +189,7 @@ func (s *ClientIntegrationTestSuite) TestProperties_Location() {
l, _ := fs.NewLocation("test-container", "/")
client, _ := fs.Client()

err = client.Upload(f, strings.NewReader("Hello world!"))
err = client.Upload(f, strings.NewReader("Hello world!"), "")
s.NoError(err, "The file should be successfully uploaded to azure so we shouldn't get an error")

props, err := client.Properties(l.URI(), "")
Expand Down Expand Up @@ -226,7 +227,7 @@ func (s *ClientIntegrationTestSuite) TestTouch_NonexistentContainer() {
client, err := fs.Client()
s.NoError(err, "Env variables (AZURE_STORAGE_ACCOUNT, AZURE_STORAGE_ACCESS_KEY) should contain valid azure account credentials")

err = client.Upload(f, strings.NewReader(""))
err = client.Upload(f, strings.NewReader(""), "")
s.Error(err, "The container doesn't exist so we should get an error")
}

Expand All @@ -237,7 +238,7 @@ func (s *ClientIntegrationTestSuite) TestTouch_FileAlreadyExists() {
client, err := fs.Client()
s.NoError(err)

err = client.Upload(f, strings.NewReader("One fish, two fish, red fish, blue fish."))
err = client.Upload(f, strings.NewReader("One fish, two fish, red fish, blue fish."), "")
s.NoError(err)
originalProps, err := client.Properties(f.Location().(*Location).ContainerURL(), f.Path())
s.NoError(err, "Should get properties back from azure with no error")
Expand Down
36 changes: 28 additions & 8 deletions backend/azure/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/c2fo/vfs/v6/backend"
"github.com/c2fo/vfs/v6/options"
"github.com/c2fo/vfs/v6/options/delete"
"github.com/c2fo/vfs/v6/options/newfile"
"github.com/c2fo/vfs/v6/utils"
)

Expand All @@ -23,6 +24,7 @@ type File struct {
fileSystem *FileSystem
container string
name string
opts []options.NewFileOption
tempFile *os.File
isDirty bool
}
Expand All @@ -47,7 +49,16 @@ func (f *File) Close() error {
}

if f.isDirty {
if err := client.Upload(f, f.tempFile); err != nil {
var contentType string
for _, o := range f.opts {
switch o := o.(type) {
case *newfile.ContentType:
contentType = *(*string)(o)
default:
}
}

if err := client.Upload(f, f.tempFile, contentType); err != nil {
return utils.WrapCloseError(err)
}
}
Expand Down Expand Up @@ -141,7 +152,7 @@ func (f *File) Location() vfs.Location {
// name at the given location. If the given location is also azure, the azure API for copying
// files will be utilized, otherwise, standard io.Copy will be done to the new file.
func (f *File) CopyToLocation(location vfs.Location) (vfs.File, error) {
newFile, err := location.NewFile(utils.RemoveLeadingSlash(f.Name()))
newFile, err := location.NewFile(utils.RemoveLeadingSlash(f.Name()), f.opts...)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -225,7 +236,7 @@ func (f *File) MoveToFile(file vfs.File) error {
}

// Delete deletes the file.
// If DeleteAllVersions option is provided, each version of the file is deleted. NOTE: if soft deletion is enabled,
// If delete.AllVersions option is provided, each version of the file is deleted. NOTE: if soft deletion is enabled,
// it will mark all versions as soft deleted, and they will be removed by Azure as per soft deletion policy.
// Returns any error returned by the API.
func (f *File) Delete(opts ...options.DeleteOption) error {
Expand All @@ -238,11 +249,11 @@ func (f *File) Delete(opts ...options.DeleteOption) error {
return err
}

var deleteAllVersions bool
var allVersions bool
for _, o := range opts {
switch o.(type) {
case delete.DeleteAllVersions:
deleteAllVersions = true
case delete.AllVersions, delete.DeleteAllVersions:
allVersions = true
default:
}
}
Expand All @@ -251,7 +262,7 @@ func (f *File) Delete(opts ...options.DeleteOption) error {
return err
}

if deleteAllVersions {
if allVersions {
return client.DeleteAllVersions(f)
}

Expand Down Expand Up @@ -308,7 +319,16 @@ func (f *File) Touch() error {
}

if !exists {
return client.Upload(f, strings.NewReader(""))
var contentType string
for _, o := range f.opts {
switch o := o.(type) {
case *newfile.ContentType:
contentType = *(*string)(o)
default:
}
}

return client.Upload(f, strings.NewReader(""), contentType)
}

props, err := client.Properties(f.Location().(*Location).ContainerURL(), f.Path())
Expand Down
4 changes: 3 additions & 1 deletion backend/azure/fileSystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/c2fo/vfs/v6"
"github.com/c2fo/vfs/v6/backend"
"github.com/c2fo/vfs/v6/options"
"github.com/c2fo/vfs/v6/utils"
)

Expand Down Expand Up @@ -57,7 +58,7 @@ func (fs *FileSystem) Client() (Client, error) {
}

// NewFile returns the azure implementation of vfs.File
func (fs *FileSystem) NewFile(volume, absFilePath string) (vfs.File, error) {
func (fs *FileSystem) NewFile(volume, absFilePath string, opts ...options.NewFileOption) (vfs.File, error) {
if fs == nil {
return nil, errors.New(errNilFileSystemReceiver)
}
Expand All @@ -74,6 +75,7 @@ func (fs *FileSystem) NewFile(volume, absFilePath string) (vfs.File, error) {
fileSystem: fs,
container: volume,
name: path.Clean(absFilePath),
opts: opts,
}, nil
}

Expand Down
28 changes: 24 additions & 4 deletions backend/azure/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/c2fo/vfs/v6"
"github.com/c2fo/vfs/v6/options/delete"
"github.com/c2fo/vfs/v6/options/newfile"
"github.com/c2fo/vfs/v6/utils"
)

Expand Down Expand Up @@ -114,6 +115,15 @@ func (s *FileTestSuite) TestExists_NonExistentFile() {
s.False(exists)
}

func (s *FileTestSuite) TestCloseWithContentType() {
client := MockAzureClient{PropertiesError: MockStorageError{}}
fs := NewFileSystem().WithClient(&client)
f, _ := fs.NewFile("test-container", "/foo.txt", newfile.WithContentType("text/plain"))
_, _ = f.Write([]byte("Hello, World!"))
s.NoError(f.Close())
s.Equal("text/plain", client.UploadContentType)
}

func (s *FileTestSuite) TestLocation() {
fs := NewFileSystem().WithOptions(Options{AccountName: "test-account"})
f, _ := fs.NewFile("test-container", "/file.txt")
Expand Down Expand Up @@ -189,22 +199,22 @@ func (s *FileTestSuite) TestDelete() {
s.NoError(f.Delete(), "The delete should succeed so there should be no error")
}

func (s *FileTestSuite) TestDeleteWithDeleteAllVersionsOption() {
func (s *FileTestSuite) TestDeleteWithAllVersionsOption() {
client := MockAzureClient{}
fs := NewFileSystem().WithClient(&client)

f, err := fs.NewFile("test-container", "/foo.txt")
s.NoError(err, "The path is valid so no error should be returned")
s.NoError(f.Delete(delete.WithDeleteAllVersions()), "The delete should succeed so there should be no error")
s.NoError(f.Delete(delete.WithAllVersions()), "The delete should succeed so there should be no error")
}

func (s *FileTestSuite) TestDeleteWithDeleteAllVersionsOption_Error() {
func (s *FileTestSuite) TestDeleteWithAllVersionsOption_Error() {
client := MockAzureClient{ExpectedError: errors.New("i always error")}
fs := NewFileSystem().WithClient(&client)

f, err := fs.NewFile("test-container", "/foo.txt")
s.NoError(err, "The path is valid so no error should be returned")
err = f.Delete(delete.WithDeleteAllVersions())
err = f.Delete(delete.WithAllVersions())
s.Error(err, "If the file does not exist we get an error")
}

Expand Down Expand Up @@ -287,6 +297,16 @@ func (s *FileTestSuite) TestTouch_NonexistentContainer() {
s.Error(f.Touch(), "The container does not exist so creating the new file should error")
}

func (s *FileTestSuite) TestTouchWithContentType() {
client := MockAzureClient{ExpectedResult: &BlobProperties{}, PropertiesError: MockStorageError{}}
fs := NewFileSystem().WithClient(&client)

f, err := fs.NewFile("test-container", "/foo.txt", newfile.WithContentType("text/plain"))
s.NoError(err, "The path is valid so no error should be returned")
s.NoError(f.Touch())
s.Equal("text/plain", client.UploadContentType)
}

func (s *FileTestSuite) TestURI() {
fs := NewFileSystem().WithOptions(Options{AccountName: "test-container"})
f, _ := fs.NewFile("temp", "/foo/bar/blah.txt")
Expand Down
3 changes: 2 additions & 1 deletion backend/azure/location.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (l *Location) FileSystem() vfs.FileSystem {
}

// NewFile returns a new file instance at the given path, relative to the current location.
func (l *Location) NewFile(relFilePath string) (vfs.File, error) {
func (l *Location) NewFile(relFilePath string, opts ...options.NewFileOption) (vfs.File, error) {
if l == nil {
return nil, errors.New(errNilLocationReceiver)
}
Expand All @@ -180,6 +180,7 @@ func (l *Location) NewFile(relFilePath string) (vfs.File, error) {
name: utils.EnsureLeadingSlash(path.Join(l.path, relFilePath)),
container: l.container,
fileSystem: l.fileSystem,
opts: opts,
}, nil
}

Expand Down
12 changes: 7 additions & 5 deletions backend/azure/mock_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ import (

// MockAzureClient is a mock implementation of azure.Client.
type MockAzureClient struct {
PropertiesError error
PropertiesResult *BlobProperties
ExpectedError error
ExpectedResult interface{}
PropertiesError error
PropertiesResult *BlobProperties
ExpectedError error
ExpectedResult interface{}
UploadContentType string
}

// Properties returns a PropertiesResult if it exists, otherwise it will return the value of PropertiesError
Expand All @@ -31,7 +32,8 @@ func (a *MockAzureClient) SetMetadata(file vfs.File, metadata map[string]string)
}

// Upload returns the value of ExpectedError
func (a *MockAzureClient) Upload(file vfs.File, content io.ReadSeeker) error {
func (a *MockAzureClient) Upload(file vfs.File, content io.ReadSeeker, contentType string) error {
a.UploadContentType = contentType
return a.ExpectedError
}

Expand Down
1 change: 1 addition & 0 deletions backend/ftp/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type File struct {
fileSystem *FileSystem
authority utils.Authority
path string
opts []options.NewFileOption
offset int64
}

Expand Down
4 changes: 3 additions & 1 deletion backend/ftp/fileSystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/c2fo/vfs/v6"
"github.com/c2fo/vfs/v6/backend"
"github.com/c2fo/vfs/v6/backend/ftp/types"
"github.com/c2fo/vfs/v6/options"
"github.com/c2fo/vfs/v6/utils"
)

Expand All @@ -34,7 +35,7 @@ func (fs *FileSystem) Retry() vfs.Retry {
}

// NewFile function returns the FTP implementation of vfs.File.
func (fs *FileSystem) NewFile(authority, filePath string) (vfs.File, error) {
func (fs *FileSystem) NewFile(authority, filePath string, opts ...options.NewFileOption) (vfs.File, error) {
if fs == nil {
return nil, errors.New("non-nil ftp.FileSystem pointer is required")
}
Expand All @@ -54,6 +55,7 @@ func (fs *FileSystem) NewFile(authority, filePath string) (vfs.File, error) {
fileSystem: fs,
authority: auth,
path: path.Clean(filePath),
opts: opts,
}, nil
}

Expand Down
3 changes: 2 additions & 1 deletion backend/ftp/location.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func (l *Location) ChangeDir(relativePath string) error {

// NewFile uses the properties of the calling location to generate a vfs.File (backed by an ftp.File). The filePath
// argument is expected to be a relative path to the location's current path.
func (l *Location) NewFile(filePath string) (vfs.File, error) {
func (l *Location) NewFile(filePath string, opts ...options.NewFileOption) (vfs.File, error) {
err := utils.ValidateRelativeFilePath(filePath)
if err != nil {
return nil, err
Expand All @@ -202,6 +202,7 @@ func (l *Location) NewFile(filePath string) (vfs.File, error) {
fileSystem: l.fileSystem,
authority: l.Authority,
path: utils.EnsureLeadingSlash(path.Join(l.path, filePath)),
opts: opts,
}
return newFile, nil
}
Expand Down
Loading

0 comments on commit ff6bf4d

Please sign in to comment.