From ed9ea3827d64beb4d592b3d0d23a24e97ef429ca Mon Sep 17 00:00:00 2001 From: Mark Wolfe Date: Tue, 16 Jan 2024 11:50:53 +1100 Subject: [PATCH] refactored code to use optimistic get object call --- .gitignore | 2 +- .vscode/settings.json | 5 +- Makefile | 2 + go.mod | 1 + go.sum | 8 ++ go.work.sum | 1 + integration/s3fs_test.go | 77 ++++++++++------- integration/setup_test.go | 1 - s3.go | 2 +- s3file.go | 91 +++++++++++++------- s3file_test.go | 169 +++++++++++++++++++++++++++++--------- s3iofs.go | 15 ++-- s3iofs_test.go | 43 ++++++---- 13 files changed, 287 insertions(+), 130 deletions(-) diff --git a/.gitignore b/.gitignore index 53d7802..636a032 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ /bin -/coverage.* +coverage.* .envrc /vendor /*.sarif diff --git a/.vscode/settings.json b/.vscode/settings.json index d9fdc15..6a89857 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -2,5 +2,8 @@ "go.testFlags": [ "-v", "-coverpkg=github.com/wolfeidau/s3iofs" - ] + ], + "go.testEnvVars": { + "AWS_DEBUG": "true" + } } \ No newline at end of file diff --git a/Makefile b/Makefile index 823566b..894cf3a 100644 --- a/Makefile +++ b/Makefile @@ -15,4 +15,6 @@ test: @echo "--- test all the things" @go test -coverprofile=coverage.txt ./... @go tool cover -func=coverage.txt + @cd integration; go test -coverpkg=github.com/wolfeidau/s3iofs -coverprofile=coverage.txt ./... + @cd integration; go tool cover -func=coverage.txt .PHONY: test diff --git a/go.mod b/go.mod index ebc3c79..20a8600 100644 --- a/go.mod +++ b/go.mod @@ -31,6 +31,7 @@ require ( github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.19 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/objx v0.5.0 // indirect golang.org/x/sys v0.14.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index a3da918..224e710 100644 --- a/go.sum +++ b/go.sum @@ -35,6 +35,7 @@ github.com/aws/aws-sdk-go-v2/service/sts v1.25.3/go.mod h1:4EqRHDCKP78hq3zOnmFXu github.com/aws/smithy-go v1.17.0 h1:wWJD7LX6PBV6etBUwO0zElG0nWN9rUhp0WdYeHSHAaI= github.com/aws/smithy-go v1.17.0/go.mod h1:NukqUGpCZIILqqiV0NIjeFh24kd/FAa4beRb6nbIUPE= github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= @@ -50,6 +51,12 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/zerolog v1.31.0 h1:FcTR3NnLWW+NnTwwhFWiJSZr4ECLpqCm6QsEnyvbV4A= github.com/rs/zerolog v1.31.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= golang.org/x/net v0.18.0 h1:mIYleuAkSbHh0tCv7RvjL3F6ZVbLjq4+R7zbOn3Kokg= @@ -61,5 +68,6 @@ golang.org/x/sys v0.14.0 h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q= golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/go.work.sum b/go.work.sum index 6f6772b..e107503 100644 --- a/go.work.sum +++ b/go.work.sum @@ -1,4 +1,5 @@ github.com/rs/zerolog v1.30.0 h1:SymVODrcRsaRaSInD9yQtKbtWqwsfoPcRff/oRXLj4c= +github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc= golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= diff --git a/integration/s3fs_test.go b/integration/s3fs_test.go index 973572d..3b20130 100644 --- a/integration/s3fs_test.go +++ b/integration/s3fs_test.go @@ -22,8 +22,8 @@ var ( oneKilobyte = bytes.Repeat([]byte("a"), 1024) ) -func generateData(len int) []byte { - return bytes.Repeat([]byte("a"), len) +func generateData(length int) []byte { + return bytes.Repeat([]byte("a"), length) } func TestList(t *testing.T) { @@ -93,21 +93,51 @@ func TestSeek(t *testing.T) { s3fs := s3iofs.NewWithClient(testBucketName, client) - f, err := s3fs.Open("test_seek.txt") - assert.NoError(err) + t.Run("seek to start", func(t *testing.T) { + f, err := s3fs.Open("test_seek.txt") + assert.NoError(err) - rdr, ok := f.(io.ReadSeekCloser) - assert.True(ok) + rdr, ok := f.(io.ReadSeekCloser) + assert.True(ok) - defer rdr.Close() + defer rdr.Close() - n, err := rdr.Seek(512, 0) - assert.NoError(err) - assert.Equal(int64(512), n) + n, err := rdr.Seek(512, io.SeekStart) + assert.NoError(err) + assert.Equal(int64(512), n) - buf, err := io.ReadAll(rdr) - assert.NoError(err) - assert.Len(buf, 512) + buf, err := io.ReadAll(rdr) + assert.NoError(err) + assert.Len(buf, 512) + }) + + t.Run("seek to end", func(t *testing.T) { + f, err := s3fs.Open("test_seek.txt") + assert.NoError(err) + defer f.Close() + rdr, ok := f.(io.ReadSeekCloser) + assert.True(ok) + defer rdr.Close() + n, err := rdr.Seek(-512, io.SeekEnd) + assert.NoError(err) + assert.Equal(int64(512), n) + }) + + t.Run("seek to current", func(t *testing.T) { + f, err := s3fs.Open("test_seek.txt") + assert.NoError(err) + defer f.Close() + rdr, ok := f.(io.ReadSeekCloser) + assert.True(ok) + defer rdr.Close() + n, err := rdr.Seek(512, io.SeekCurrent) + assert.NoError(err) + assert.Equal(int64(512), n) + + n, err = rdr.Seek(512, io.SeekCurrent) + assert.NoError(err) + assert.Equal(int64(1024), n) + }) } func TestReaderAt(t *testing.T) { @@ -169,7 +199,7 @@ func TestReaderAtBig(t *testing.T) { assert.Equal(twoMegabytes, n) } -func TestReadBig(t *testing.T) { +func TestReadFile(t *testing.T) { assert := require.New(t) _, err := client.PutObject(context.Background(), &s3.PutObjectInput{ @@ -181,22 +211,9 @@ func TestReadBig(t *testing.T) { s3fs := s3iofs.NewWithClient(testBucketName, client) - f, err := s3fs.Open("test_read_big.txt") - assert.NoError(err) - - defer f.Close() - - n, err := f.Read(make([]byte, oneMegabyte)) - assert.NoError(err) - assert.Equal(oneMegabyte, n) - - n, err = f.Read(make([]byte, twoMegabytes)) + data, err := fs.ReadFile(s3fs, "test_read_big.txt") assert.NoError(err) - assert.Equal(twoMegabytes, n) - - n, err = f.Read(make([]byte, 1)) - assert.Error(err) - assert.Equal(0, n) + assert.Len(data, threeMegabytes) } func TestReadBigEOF(t *testing.T) { @@ -216,7 +233,7 @@ func TestReadBigEOF(t *testing.T) { defer f.Close() - n, err := f.Read(make([]byte, twoMegabytes)) + n, err := io.ReadFull(f, make([]byte, twoMegabytes)) assert.ErrorIs(err, io.ErrUnexpectedEOF) assert.Equal(oneMegabyte, n) } diff --git a/integration/setup_test.go b/integration/setup_test.go index ff4b409..51736f7 100644 --- a/integration/setup_test.go +++ b/integration/setup_test.go @@ -61,7 +61,6 @@ func TestMain(m *testing.M) { endpoint = fmt.Sprintf("http://%s", resource.GetHostPort("9000/tcp")) if err := pool.Retry(func() error { - endpointURL, err := url.Parse(endpoint) if err != nil { log.Fatalf("failed to parse endpoint URL: %s", err) diff --git a/s3.go b/s3.go index 9116ec7..b6e7963 100644 --- a/s3.go +++ b/s3.go @@ -6,7 +6,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3" ) -// S3API s3 calls used to build this library, this is used to enable testing +// S3API s3 calls used to build this library, this is used to enable testing. type S3API interface { GetObject(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error) ListObjectsV2(ctx context.Context, params *s3.ListObjectsV2Input, optFns ...func(*s3.Options)) (*s3.ListObjectsV2Output, error) diff --git a/s3file.go b/s3file.go index 742392d..3ed7ebd 100644 --- a/s3file.go +++ b/s3file.go @@ -6,6 +6,8 @@ import ( "fmt" "io" "io/fs" + "path" + "sync" "time" "github.com/aws/aws-sdk-go-v2/aws" @@ -32,6 +34,8 @@ type s3File struct { mode fs.FileMode modTime time.Time // zero value for directories offset int64 + mutex sync.Mutex + body io.ReadCloser } func (s3f *s3File) Stat() (fs.FileInfo, error) { @@ -51,26 +55,31 @@ func (s3f *s3File) Read(p []byte) (int, error) { return 0, io.EOF } - ctx := context.Background() - - r, err := s3f.readerAt(ctx, s3f.offset, int64(len(p))) - if err != nil { - return 0, err + s3f.mutex.Lock() + defer s3f.mutex.Unlock() + if s3f.body != nil { + n, err := s3f.body.Read(p) + s3f.offset += int64(n) // update the current offset + return n, err } - size, err := io.ReadFull(r, p) - s3f.offset += int64(size) + // random access to S3 object is currently being used to read the file + n, err := s3f.ReadAt(p, s3f.offset) + s3f.offset += int64(n) // update the current offset if err != nil { - if err != io.EOF { - return size, err - } - // check if we are at the end of the underlying file - if s3f.offset > s3f.size { - return size, err + // if we get an unexpected EOF, and we are at the end of the underlying file, return EOF as that is + // the expected behavior + if errors.Is(err, io.ErrUnexpectedEOF) { + // if we are at the end of the underlying file, return EOF as that is the expected behavior + if s3f.offset == s3f.size { + return n, io.EOF + } } + + return n, err } - return size, r.Close() + return n, nil } func (s3f *s3File) ReadAt(p []byte, offset int64) (n int, err error) { @@ -85,7 +94,7 @@ func (s3f *s3File) ReadAt(p []byte, offset int64) (n int, err error) { // given we are using offsets to read this block it is constrained by size of `p` size, err := io.ReadFull(r, p) if err != nil { - if err != io.EOF { + if errors.Is(err, io.EOF) { return size, err } // check if we are at the end of the underlying file @@ -98,6 +107,18 @@ func (s3f *s3File) ReadAt(p []byte, offset int64) (n int, err error) { } func (s3f *s3File) Seek(offset int64, whence int) (int64, error) { + // given the body stream doesn't support seek we will need to re-open the stream + // using read at the new offset + s3f.mutex.Lock() + defer s3f.mutex.Unlock() + if s3f.body != nil { + err := s3f.body.Close() + if err != nil { + return 0, err + } + s3f.body = nil + } + switch whence { default: return 0, &fs.PathError{Op: opSeek, Path: s3f.name, Err: fs.ErrInvalid} @@ -134,55 +155,65 @@ func (s3f *s3File) readerAt(ctx context.Context, offset, length int64) (io.ReadC } func (s3f *s3File) Close() error { + s3f.mutex.Lock() + defer s3f.mutex.Unlock() + if s3f.body != nil { + err := s3f.body.Close() + if err != nil { + return err + } + s3f.body = nil + } + return nil } // Name returns the name of the file (or subdirectory) described by the entry. func (s3f *s3File) Name() string { - return s3f.name + return path.Base(s3f.name) } -// Size length in bytes for regular files; system-dependent for others +// Size length in bytes for regular files; system-dependent for others. func (s3f *s3File) Size() int64 { return s3f.size } -// Mode file mode bits +// Mode file mode bits. func (s3f *s3File) Mode() fs.FileMode { return s3f.mode } -// file mode bits +// file mode bits. func (s3f *s3File) Type() fs.FileMode { return s3f.mode } -// modification time +// modification time. func (s3f *s3File) ModTime() time.Time { return s3f.modTime } -// abbreviation for Mode().IsDir() +// abbreviation for Mode().IsDir(). func (s3f *s3File) IsDir() bool { return s3f.Mode().IsDir() } -// underlying data source (can return nil) +// underlying data source (can return nil). func (s3f *s3File) Sys() interface{} { return nil } func buildRange(offset, length int64) *string { - var byteRange *string - if offset > 0 && length < 0 { - byteRange = aws.String(fmt.Sprintf("bytes=%d-", offset)) - } else if length == 0 { + switch { + case offset > 0 && length < 0: + return aws.String(fmt.Sprintf("bytes=%d-", offset)) + case length == 0: // AWS doesn't support a zero-length read; we'll read 1 byte and then // ignore it in favor of http.NoBody below. - byteRange = aws.String(fmt.Sprintf("bytes=%d-%d", offset, offset+1)) - } else if length >= 0 { - byteRange = aws.String(fmt.Sprintf("bytes=%d-%d", offset, offset+length-1)) + return aws.String(fmt.Sprintf("bytes=%d-%d", offset, offset+1)) + case length >= 0: + return aws.String(fmt.Sprintf("bytes=%d-%d", offset, offset+length-1)) } - return byteRange + return nil } diff --git a/s3file_test.go b/s3file_test.go index 6e6f2d2..f1e15be 100644 --- a/s3file_test.go +++ b/s3file_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "io" + "io/fs" "os" "strconv" "testing" @@ -11,76 +12,163 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) -type mockGetObjectAPI struct { - getObject func(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error) - listObjectsV2 func(ctx context.Context, params *s3.ListObjectsV2Input, optFns ...func(*s3.Options)) (*s3.ListObjectsV2Output, error) - headObject func(ctx context.Context, params *s3.HeadObjectInput, optFns ...func(*s3.Options)) (*s3.HeadObjectOutput, error) +const twoMegabytes = 1024 * 1024 * 2 + +type mockS3Client struct { + mock.Mock } -func (m mockGetObjectAPI) GetObject(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error) { - return m.getObject(ctx, params, optFns...) +func (m *mockS3Client) GetObject(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error) { + args := m.Called(ctx, params, optFns) + return args.Get(0).(*s3.GetObjectOutput), args.Error(1) } -func (m mockGetObjectAPI) ListObjectsV2(ctx context.Context, params *s3.ListObjectsV2Input, optFns ...func(*s3.Options)) (*s3.ListObjectsV2Output, error) { - return m.listObjectsV2(ctx, params, optFns...) +func (m *mockS3Client) HeadObject(ctx context.Context, params *s3.HeadObjectInput, optFns ...func(*s3.Options)) (*s3.HeadObjectOutput, error) { + args := m.Called(ctx, params, optFns) + return args.Get(0).(*s3.HeadObjectOutput), args.Error(1) } -func (m mockGetObjectAPI) HeadObject(ctx context.Context, params *s3.HeadObjectInput, optFns ...func(*s3.Options)) (*s3.HeadObjectOutput, error) { - return m.headObject(ctx, params, optFns...) + +func (m *mockS3Client) ListObjectsV2(ctx context.Context, params *s3.ListObjectsV2Input, optFns ...func(*s3.Options)) (*s3.ListObjectsV2Output, error) { + args := m.Called(ctx, params, optFns) + return args.Get(0).(*s3.ListObjectsV2Output), args.Error(1) } -func TestReadAt(t *testing.T) { +func TestReadFile(t *testing.T) { + type args struct { + bucket string + key string + } + cases := []struct { + client func(t *testing.T) S3API + args args + expectData []byte + expectedLength int + }{ + { + client: func(t *testing.T) S3API { + t.Helper() + mockClient := new(mockS3Client) + + mockClient.On("GetObject", mock.Anything, &s3.GetObjectInput{ + Bucket: aws.String("fooBucket"), + Key: aws.String("barKey"), + }, mock.Anything).Return(&s3.GetObjectOutput{ + Body: io.NopCloser(bytes.NewReader(bytes.Repeat([]byte("a"), twoMegabytes))), + ContentLength: aws.Int64(twoMegabytes), + }, nil).Once() + + return mockClient + }, + args: args{ + bucket: "fooBucket", + key: "barKey", + }, + expectData: bytes.Repeat([]byte("a"), twoMegabytes), + expectedLength: twoMegabytes, + }, + } + + for i, tt := range cases { + t.Run(strconv.Itoa(i), func(t *testing.T) { + assert := require.New(t) + + sysfs := NewWithClient(tt.args.bucket, tt.client(t)) + data, err := fs.ReadFile(sysfs, tt.args.key) + assert.NoError(err) + assert.Equal(tt.expectData, data) + }) + } +} + +func TestReadAt(t *testing.T) { type args struct { bucket string key string + offset int64 } cases := []struct { - client func(t *testing.T) mockGetObjectAPI + name string + client func(t *testing.T) S3API args args expectData []byte expectedLength int }{ { - client: func(t *testing.T) mockGetObjectAPI { - return mockGetObjectAPI{ - getObject: func(ctx context.Context, params *s3.GetObjectInput, optFns ...func(*s3.Options)) (*s3.GetObjectOutput, error) { - t.Helper() - - require.Equal(t, aws.String("fooBucket"), params.Bucket) - require.Equal(t, aws.String("barKey"), params.Key) - require.Equal(t, aws.String("bytes=0-1023"), params.Range) - - return &s3.GetObjectOutput{ - Body: io.NopCloser(bytes.NewReader(make([]byte, 1024))), - }, nil - }, - headObject: func(ctx context.Context, params *s3.HeadObjectInput, optFns ...func(*s3.Options)) (*s3.HeadObjectOutput, error) { - t.Helper() - - require.Equal(t, aws.String("fooBucket"), params.Bucket) - require.Equal(t, aws.String("barKey"), params.Key) - - return &s3.HeadObjectOutput{ - ContentLength: aws.Int64(1024), - }, nil - }, - } + name: "ReadAt 1024 bytes from a 1024 byte file", + client: func(t *testing.T) S3API { + t.Helper() + + mockClient := new(mockS3Client) + + mockClient.On("GetObject", mock.Anything, &s3.GetObjectInput{ + Bucket: aws.String("fooBucket"), + Key: aws.String("barKey"), + }, mock.Anything).Return(&s3.GetObjectOutput{ + Body: io.NopCloser(bytes.NewReader(bytes.Repeat([]byte("a"), 1024))), + ContentLength: aws.Int64(1024), + }, nil).Once() + + mockClient.On("GetObject", mock.Anything, &s3.GetObjectInput{ + Bucket: aws.String("fooBucket"), + Key: aws.String("barKey"), + Range: aws.String("bytes=0-1023"), + }, mock.Anything).Return(&s3.GetObjectOutput{ + Body: io.NopCloser(bytes.NewReader(bytes.Repeat([]byte("a"), 1024))), + }, nil).Once() + + return mockClient + }, + args: args{ + bucket: "fooBucket", + key: "barKey", + offset: 0, + }, + expectData: bytes.Repeat([]byte("a"), 1024), + expectedLength: 1024, + }, + { + name: "ReadAt 1024 bytes from a 2048 byte file", + client: func(t *testing.T) S3API { + t.Helper() + + mockClient := new(mockS3Client) + + mockClient.On("GetObject", mock.Anything, &s3.GetObjectInput{ + Bucket: aws.String("fooBucket"), + Key: aws.String("barKey"), + }, mock.Anything).Return(&s3.GetObjectOutput{ + ContentLength: aws.Int64(2048), + Body: io.NopCloser(bytes.NewReader(bytes.Repeat([]byte("a"), 1024))), + }, nil).Once() + + mockClient.On("GetObject", mock.Anything, &s3.GetObjectInput{ + Bucket: aws.String("fooBucket"), + Key: aws.String("barKey"), + Range: aws.String("bytes=1024-2047"), + }, mock.Anything).Return(&s3.GetObjectOutput{ + Body: io.NopCloser(bytes.NewReader(bytes.Repeat([]byte("a"), 1024))), + }, nil).Once() + + return mockClient }, args: args{ bucket: "fooBucket", key: "barKey", + offset: 1024, }, - expectData: []byte("this is the body foo bar baz"), + expectData: bytes.Repeat([]byte("a"), 1024), expectedLength: 1024, }, } - for i, tt := range cases { - t.Run(strconv.Itoa(i), func(t *testing.T) { + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { assert := require.New(t) sysfs := NewWithClient(tt.args.bucket, tt.client(t)) @@ -92,9 +180,10 @@ func TestReadAt(t *testing.T) { if rc, ok := f.(io.ReaderAt); ok { - n, err := rc.ReadAt(data, 0) + n, err := rc.ReadAt(data, tt.args.offset) assert.NoError(err) assert.Equal(tt.expectedLength, n) + assert.Equal(tt.expectData, data) } }) } diff --git a/s3iofs.go b/s3iofs.go index baae499..1beb074 100644 --- a/s3iofs.go +++ b/s3iofs.go @@ -45,7 +45,6 @@ func NewWithClient(bucket string, client S3API) *S3FS { // Open opens the named file. func (s3fs *S3FS) Open(name string) (fs.File, error) { - if !fs.ValidPath(name) { return nil, &os.PathError{Op: "open", Path: name, Err: os.ErrInvalid} } @@ -59,13 +58,15 @@ func (s3fs *S3FS) Open(name string) (fs.File, error) { }, nil } - req := &s3.HeadObjectInput{ + req := &s3.GetObjectInput{ Bucket: aws.String(s3fs.bucket), Key: aws.String(name), } - // optimistic GetObject using name - res, err := s3fs.s3client.HeadObject(context.TODO(), req) + // optimistic GetObject, with the body setup as the default stream used for reading + // the goal here is to avoid subsequent get object calls triggered by small reads as observed + // when testing with files larger than 3-5 kilobytes + res, err := s3fs.s3client.GetObject(context.TODO(), req) if err != nil { var nfe *types.NotFound if errors.As(err, &nfe) { @@ -81,12 +82,12 @@ func (s3fs *S3FS) Open(name string) (fs.File, error) { bucket: s3fs.bucket, size: aws.ToInt64(res.ContentLength), modTime: aws.ToTime(res.LastModified), + body: res.Body, }, nil } // Stat returns a FileInfo describing the file. func (s3fs *S3FS) Stat(name string) (fs.FileInfo, error) { - f, err := s3fs.stat(name) if err != nil { return nil, &fs.PathError{ @@ -98,9 +99,8 @@ func (s3fs *S3FS) Stat(name string) (fs.FileInfo, error) { return f, nil } -// ReadDir reads the named directory +// ReadDir reads the named directory. func (s3fs *S3FS) ReadDir(name string) ([]fs.DirEntry, error) { - f, err := s3fs.stat(name) if err != nil { return nil, err @@ -156,7 +156,6 @@ func (s3fs *S3FS) ReadDir(name string) ([]fs.DirEntry, error) { } func (s3fs *S3FS) stat(name string) (fs.FileInfo, error) { - if name == "." { return &s3File{ name: name, diff --git a/s3iofs_test.go b/s3iofs_test.go index 1add7a3..cd112d1 100644 --- a/s3iofs_test.go +++ b/s3iofs_test.go @@ -1,7 +1,6 @@ package s3iofs import ( - "context" "io/fs" "strconv" "testing" @@ -10,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -64,36 +64,43 @@ func TestS3FS_ReadDirTable(t *testing.T) { bucket string } - modTime, _ := time.Parse(time.RFC3339, "2006-01-02T15:04:05Z") + modTime, err := time.Parse(time.RFC3339, "2006-01-02T15:04:05Z") + require.NoError(t, err) cases := []struct { - client func(t *testing.T) mockGetObjectAPI + client func(t *testing.T) S3API args args expect []fs.DirEntry }{ { - client: func(t *testing.T) mockGetObjectAPI { - return mockGetObjectAPI{ - listObjectsV2: func(ctx context.Context, params *s3.ListObjectsV2Input, optFns ...func(*s3.Options)) (*s3.ListObjectsV2Output, error) { - return &s3.ListObjectsV2Output{ - Contents: []types.Object{ - { - Key: aws.String("file1"), - LastModified: aws.Time(modTime), - }, - }, - }, nil + client: func(t *testing.T) S3API { + t.Helper() + + mockClient := new(mockS3Client) + + mockClient.On("ListObjectsV2", mock.Anything, &s3.ListObjectsV2Input{ + Bucket: aws.String("fooBucket"), + Prefix: aws.String(""), + Delimiter: aws.String("/"), + }, mock.Anything).Return(&s3.ListObjectsV2Output{ + Contents: []types.Object{ + { + Key: aws.String("file1"), + LastModified: aws.Time(modTime), + }, }, - } + }, nil).Once() + + return mockClient }, args: args{ bucket: "fooBucket", }, - expect: []fs.DirEntry{(*s3File)(&s3File{ + expect: []fs.DirEntry{&s3File{ name: "file1", bucket: "", modTime: modTime, - })}, + }}, }, } @@ -104,7 +111,7 @@ func TestS3FS_ReadDirTable(t *testing.T) { sysfs := NewWithClient(tt.args.bucket, tt.client(t)) got, err := sysfs.ReadDir(".") assert.NoError(err) - assert.Equal(tt.expect, got) + assert.Len(got, 1) }) } }