From 569309c5457579408c4b8efd511a0dfe4536f58d Mon Sep 17 00:00:00 2001 From: Chris Miles Date: Thu, 6 Aug 2026 22:49:24 +0000 Subject: [PATCH 1/8] test: raise storage coverage above 95 percent --- driver/dropboxstorage/dropbox_unit_test.go | 472 +++++++++++++++++++ driver/ftpstorage/ftp_unit_test.go | 213 +++++++++ driver/gcsstorage/gcs_unit_test.go | 464 +++++++++++++++++- driver/localstorage/local_more_test.go | 343 ++++++++++++++ driver/memorystorage/memory_internal_test.go | 209 ++++++++ driver/rclonestorage/rclone_internal_test.go | 126 +++++ driver/redisstorage/redis_miniredis_test.go | 352 ++++++++++++++ driver/redisstorage/redis_test.go | 93 ++++ driver/s3storage/s3_unit_test.go | 220 +++++++++ driver/sftpstorage/sftp_unit_test.go | 300 +++++++++++- 10 files changed, 2781 insertions(+), 11 deletions(-) diff --git a/driver/dropboxstorage/dropbox_unit_test.go b/driver/dropboxstorage/dropbox_unit_test.go index cc48ef2..8a8f0cf 100644 --- a/driver/dropboxstorage/dropbox_unit_test.go +++ b/driver/dropboxstorage/dropbox_unit_test.go @@ -1,6 +1,7 @@ package dropboxstorage import ( + "bytes" "context" "errors" "io" @@ -101,6 +102,12 @@ type fakeDropbox struct { createErr error delErr error deleteHook func(*files.DeleteArg) error + createHook func() + moveHook func() + metaHook func() + listHook func() + continueHook func() + linkHook func() moveErr error metaErr error metaOut files.IsMetadata @@ -109,6 +116,7 @@ type fakeDropbox struct { linkErr error linkURL string continueOut *files.ListFolderResult + continueErr error continueSeq []*files.ListFolderResult continueArgs []*files.ListFolderContinueArg uploaded []byte @@ -121,6 +129,8 @@ type fakeDropbox struct { metadataArgs []*files.GetMetadataArg metaByPath map[string]files.IsMetadata metaErrByPath map[string]error + metaSeq []files.IsMetadata + metaErrSeq []error } // Download records its argument and returns the configured body, payload, or failure. @@ -130,6 +140,9 @@ func (f *fakeDropbox) Download(arg *files.DownloadArg) (*files.FileMetadata, io. f.downloadHook() } if f.getErr != nil { + if f.getReader != nil { + return nil, f.getReader, f.getErr + } return nil, nil, f.getErr } if f.getReader != nil { @@ -167,6 +180,9 @@ func (f *fakeDropbox) DeleteV2(arg *files.DeleteArg) (*files.DeleteResult, error // CreateFolderV2 records folder creation and updates fixture metadata on success. func (f *fakeDropbox) CreateFolderV2(arg *files.CreateFolderArg) (*files.CreateFolderResult, error) { f.createArgs = append(f.createArgs, arg) + if f.createHook != nil { + f.createHook() + } if f.createErr != nil { return nil, f.createErr } @@ -179,6 +195,9 @@ func (f *fakeDropbox) CreateFolderV2(arg *files.CreateFolderArg) (*files.CreateF // MoveV2 records relocation arguments and returns the configured failure. func (f *fakeDropbox) MoveV2(arg *files.RelocationArg) (*files.RelocationResult, error) { f.moveArgs = append(f.moveArgs, arg) + if f.moveHook != nil { + f.moveHook() + } if f.moveErr != nil { return nil, f.moveErr } @@ -188,6 +207,22 @@ func (f *fakeDropbox) MoveV2(arg *files.RelocationArg) (*files.RelocationResult, // GetMetadata resolves path-specific fixture metadata and failures before shared defaults. func (f *fakeDropbox) GetMetadata(arg *files.GetMetadataArg) (files.IsMetadata, error) { f.metadataArgs = append(f.metadataArgs, arg) + if f.metaHook != nil { + f.metaHook() + } + if len(f.metaSeq) > 0 || len(f.metaErrSeq) > 0 { + var metadata files.IsMetadata + var err error + if len(f.metaSeq) > 0 { + metadata = f.metaSeq[0] + f.metaSeq = f.metaSeq[1:] + } + if len(f.metaErrSeq) > 0 { + err = f.metaErrSeq[0] + f.metaErrSeq = f.metaErrSeq[1:] + } + return metadata, err + } if err, ok := f.metaErrByPath[arg.Path]; ok { return nil, err } @@ -206,6 +241,9 @@ func (f *fakeDropbox) GetMetadata(arg *files.GetMetadataArg) (files.IsMetadata, // ListFolder records the initial query and returns its configured page. func (f *fakeDropbox) ListFolder(arg *files.ListFolderArg) (*files.ListFolderResult, error) { f.listArgs = append(f.listArgs, arg) + if f.listHook != nil { + f.listHook() + } if f.listErr != nil { return nil, f.listErr } @@ -218,6 +256,12 @@ func (f *fakeDropbox) ListFolder(arg *files.ListFolderArg) (*files.ListFolderRes // ListFolderContinue records cursors and consumes configured pages iteratively. func (f *fakeDropbox) ListFolderContinue(arg *files.ListFolderContinueArg) (*files.ListFolderResult, error) { f.continueArgs = append(f.continueArgs, arg) + if f.continueHook != nil { + f.continueHook() + } + if f.continueErr != nil { + return nil, f.continueErr + } if len(f.continueSeq) > 0 { out := f.continueSeq[0] f.continueSeq = f.continueSeq[1:] @@ -231,6 +275,9 @@ func (f *fakeDropbox) ListFolderContinue(arg *files.ListFolderContinueArg) (*fil // GetTemporaryLink returns the configured public link or signing failure. func (f *fakeDropbox) GetTemporaryLink(arg *files.GetTemporaryLinkArg) (*files.GetTemporaryLinkResult, error) { + if f.linkHook != nil { + f.linkHook() + } if f.linkErr != nil { return nil, f.linkErr } @@ -924,6 +971,35 @@ func TestDropboxTypedErrors(t *testing.T) { if err := wrapError(dropbox.SDKInternalError{StatusCode: 403}); !errors.Is(err, storagecore.ErrForbidden) { t.Fatalf("HTTP forbidden error = %v", err) } + if err := wrapError(dropbox.SDKInternalError{StatusCode: 404}); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("HTTP not-found error = %v", err) + } + + lookup := &files.LookupError{Tagged: dropbox.Tagged{Tag: files.LookupErrorNotFound}} + lookupErrors := []error{ + files.DownloadAPIError{EndpointError: &files.DownloadError{Path: lookup}}, + files.GetTemporaryLinkAPIError{EndpointError: &files.GetTemporaryLinkError{Path: lookup}}, + files.ListFolderAPIError{EndpointError: &files.ListFolderError{Path: lookup}}, + files.ListFolderContinueAPIError{EndpointError: &files.ListFolderContinueError{Path: lookup}}, + files.MoveV2APIError{EndpointError: &files.RelocationError{FromLookup: lookup}}, + } + for index, typedErr := range lookupErrors { + if err := wrapError(typedErr); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("typed lookup error %d = %v", index, err) + } + } + + write := &files.WriteError{Tagged: dropbox.Tagged{Tag: files.WriteErrorMalformedPath}} + writeErrors := []error{ + files.CreateFolderV2APIError{EndpointError: &files.CreateFolderError{Path: write}}, + files.MoveV2APIError{EndpointError: &files.RelocationError{To: write}}, + files.MoveV2APIError{EndpointError: &files.RelocationError{FromWrite: write}}, + } + for index, typedErr := range writeErrors { + if err := wrapError(typedErr); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("typed write error %d = %v", index, err) + } + } } // TestDropboxListContinuationIsIterative verifies long pagination chains do not consume call stack. @@ -944,6 +1020,402 @@ func TestDropboxListContinuationIsIterative(t *testing.T) { } } +// TestDropboxInvalidPathsAndThinWrappers covers validation before any SDK +// request and the background-context adapters omitted by the shared contract. +func TestDropboxInvalidPathsAndThinWrappers(t *testing.T) { + d := &driver{} + calls := []func() error{ + func() error { _, err := d.GetContext(nil, "../bad"); return err }, + func() error { return d.PutContext(nil, "../bad", nil) }, + func() error { return d.MakeDirContext(nil, "../bad") }, + func() error { return d.DeleteContext(nil, "../bad") }, + func() error { _, err := d.StatContext(nil, "../bad"); return err }, + func() error { _, err := d.ExistsContext(nil, "../bad"); return err }, + func() error { _, err := d.ListContext(nil, "../bad"); return err }, + func() error { _, err := d.ListPageContext(nil, "../bad", 0, 1); return err }, + func() error { return d.WalkContext(nil, "../bad", func(storagecore.Entry) error { return nil }) }, + func() error { return d.CopyContext(nil, "../bad", "dst") }, + func() error { return d.CopyContext(nil, "src", "../bad") }, + func() error { return d.MoveContext(nil, "../bad", "dst") }, + func() error { return d.MoveContext(nil, "src", "../bad") }, + func() error { _, err := d.URLContext(nil, "../bad"); return err }, + } + for index, call := range calls { + if err := call(); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("invalid-path call %d error = %v", index, err) + } + } + if err := d.MakeDir("../bad"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("MakeDir invalid path error = %v", err) + } + if _, err := d.ListPage("../bad", 0, 1); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("ListPage invalid path error = %v", err) + } +} + +// TestDropboxCancellationAndHelperEdges covers operation cancellation and the +// stream, pagination, directory, and cleanup branches outside contract tests. +func TestDropboxCancellationAndHelperEdges(t *testing.T) { + t.Run("canceled operations", func(t *testing.T) { + d := &driver{client: &fakeDropbox{}, prefix: "pre"} + ctx, cancel := context.WithCancel(context.Background()) + cancel() + calls := []func() error{ + func() error { _, err := d.GetContext(ctx, "file"); return err }, + func() error { return d.PutContext(ctx, "file", nil) }, + func() error { return d.MakeDirContext(ctx, "dir") }, + func() error { return d.DeleteContext(ctx, "file") }, + func() error { _, err := d.ExistsContext(ctx, "file"); return err }, + func() error { _, err := d.ListContext(ctx, ""); return err }, + func() error { return d.WalkContext(ctx, "", func(storagecore.Entry) error { return nil }) }, + func() error { return d.CopyContext(ctx, "file", "copy") }, + func() error { return d.MoveContext(ctx, "file", "moved") }, + func() error { _, err := d.URLContext(ctx, "file"); return err }, + } + for index, call := range calls { + if err := call(); !errors.Is(err, context.Canceled) { + t.Fatalf("canceled call %d error = %v", index, err) + } + } + }) + + t.Run("root and callback guards", func(t *testing.T) { + d := &driver{client: &fakeDropbox{}, prefix: "pre"} + if _, err := d.Get(""); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Get root error = %v", err) + } + if err := d.Put("", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Put root error = %v", err) + } + if err := d.MakeDir(""); err != nil { + t.Fatalf("MakeDir root: %v", err) + } + if entry, err := d.Stat(""); err != nil || !entry.IsDir { + t.Fatalf("Stat root = %+v, %v", entry, err) + } + if exists, err := d.Exists(""); err != nil || exists { + t.Fatalf("Exists root = %v, %v", exists, err) + } + if err := d.Walk("", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Walk nil callback error = %v", err) + } + if _, err := d.URL(""); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("URL root error = %v", err) + } + }) + + t.Run("directory creation failures", func(t *testing.T) { + d := &driver{client: &fakeDropbox{metaErr: errors.New("lookup")}} + if err := d.MakeDir("dir"); err == nil { + t.Fatal("MakeDir lookup returned nil error") + } + d = &driver{client: &fakeDropbox{metaErr: errNotFound{}, createErr: errors.New("create")}} + if err := d.MakeDir("dir"); err == nil { + t.Fatal("MakeDir create returned nil error") + } + d = &driver{client: &fakeDropbox{metaOut: &files.FileMetadata{}}} + if err := d.MakeDir("dir"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("MakeDir over file error = %v", err) + } + }) + + t.Run("continuation and helper failures", func(t *testing.T) { + client := &fakeDropbox{ + listOut: &files.ListFolderResult{HasMore: true, Cursor: "next"}, + continueErr: errors.New("continue"), + } + d := &driver{client: client} + if _, err := d.List(""); err == nil { + t.Fatal("List continuation returned nil error") + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + var entries []storagecore.Entry + if err := d.appendEntries(ctx, []files.IsMetadata{&files.FileMetadata{}}, &entries); !errors.Is(err, context.Canceled) { + t.Fatalf("appendEntries canceled error = %v", err) + } + if _, err := (&contextReader{ctx: ctx, reader: bytes.NewReader(nil)}).Read(make([]byte, 1)); !errors.Is(err, context.Canceled) { + t.Fatalf("contextReader error = %v", err) + } + if _, err := copyContext(ctx, io.Discard, bytes.NewReader(nil)); !errors.Is(err, context.Canceled) { + t.Fatalf("copyContext canceled error = %v", err) + } + writeErr := errors.New("write") + if _, err := copyContext(context.Background(), dropboxFailingWriter{err: writeErr}, bytes.NewReader([]byte("x"))); !errors.Is(err, writeErr) { + t.Fatalf("copyContext write error = %v", err) + } + if _, err := copyContext(context.Background(), dropboxShortWriter{}, bytes.NewReader([]byte("x"))); !errors.Is(err, io.ErrShortWrite) { + t.Fatalf("copyContext short write error = %v", err) + } + primary := errors.New("primary") + cleanup := errors.New("cleanup") + if err := joinCleanup(primary, cleanup); !errors.Is(err, primary) || !errors.Is(err, cleanup) { + t.Fatalf("joinCleanup error = %v", err) + } + if err := joinCleanup(nil, cleanup); !errors.Is(err, cleanup) { + t.Fatalf("joinCleanup cleanup error = %v", err) + } + }) +} + +type dropboxFailingWriter struct { + err error +} + +// Write injects a destination failure. +func (w dropboxFailingWriter) Write([]byte) (int, error) { return 0, w.err } + +type dropboxShortWriter struct{} + +// Write accepts no bytes without error to exercise short-write handling. +func (dropboxShortWriter) Write([]byte) (int, error) { return 0, nil } + +// TestDropboxPostSDKCancellation verifies cancellation wins when it arrives +// while a non-context-aware Dropbox SDK call is in flight. +func TestDropboxPostSDKCancellation(t *testing.T) { + tests := []struct { + name string + call func(*driver, context.Context) error + make func(context.CancelFunc) *fakeDropbox + }{ + { + name: "stat", + call: func(d *driver, ctx context.Context) error { _, err := d.StatContext(ctx, "file"); return err }, + make: func(cancel context.CancelFunc) *fakeDropbox { + return &fakeDropbox{metaOut: &files.FileMetadata{}, metaHook: cancel} + }, + }, + { + name: "exists", + call: func(d *driver, ctx context.Context) error { _, err := d.ExistsContext(ctx, "file"); return err }, + make: func(cancel context.CancelFunc) *fakeDropbox { + return &fakeDropbox{metaOut: &files.FileMetadata{}, metaHook: cancel} + }, + }, + { + name: "list", + call: func(d *driver, ctx context.Context) error { _, err := d.ListContext(ctx, ""); return err }, + make: func(cancel context.CancelFunc) *fakeDropbox { return &fakeDropbox{listHook: cancel} }, + }, + { + name: "url", + call: func(d *driver, ctx context.Context) error { _, err := d.URLContext(ctx, "file"); return err }, + make: func(cancel context.CancelFunc) *fakeDropbox { return &fakeDropbox{linkHook: cancel} }, + }, + { + name: "delete metadata", + call: func(d *driver, ctx context.Context) error { return d.DeleteContext(ctx, "file") }, + make: func(cancel context.CancelFunc) *fakeDropbox { + return &fakeDropbox{metaOut: &files.FileMetadata{}, metaHook: cancel} + }, + }, + { + name: "delete mutation", + call: func(d *driver, ctx context.Context) error { return d.DeleteContext(ctx, "file") }, + make: func(cancel context.CancelFunc) *fakeDropbox { + return &fakeDropbox{metaOut: &files.FileMetadata{}, deleteHook: func(*files.DeleteArg) error { cancel(); return nil }} + }, + }, + { + name: "move mutation", + call: func(d *driver, ctx context.Context) error { return d.MoveContext(ctx, "file", "moved") }, + make: func(cancel context.CancelFunc) *fakeDropbox { + return &fakeDropbox{ + metaByPath: map[string]files.IsMetadata{"/file": &files.FileMetadata{}}, + moveHook: cancel, + } + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + d := &driver{client: test.make(cancel)} + if err := test.call(d, ctx); !errors.Is(err, context.Canceled) { + t.Fatalf("error = %v", err) + } + }) + } +} + +// TestDropboxTransferAndConcurrentDirectoryEdges covers combined SDK cleanup, +// post-transfer cancellation, pagination, and concurrent folder creation. +func TestDropboxTransferAndConcurrentDirectoryEdges(t *testing.T) { + t.Run("download error closes response", func(t *testing.T) { + primary := errors.New("download") + cleanup := errors.New("close") + body := &trackedReadCloser{reader: strings.NewReader(""), closeErr: cleanup} + d := &driver{client: &fakeDropbox{getErr: primary, getReader: body}} + _, err := d.Get("file") + if !errors.Is(err, primary) || !errors.Is(err, cleanup) || !body.closed { + t.Fatalf("Get combined error = %v closed=%v", err, body.closed) + } + }) + + t.Run("post-copy cancellation", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + body := &cancelEOFReadCloser{cancel: cancel} + d := &driver{client: &fakeDropbox{getReader: body}} + if _, err := d.GetContext(ctx, "file"); !errors.Is(err, context.Canceled) { + t.Fatalf("GetContext error = %v", err) + } + }) + + t.Run("upload error after existing parent", func(t *testing.T) { + putErr := errors.New("upload") + client := &fakeDropbox{ + metaByPath: map[string]files.IsMetadata{"/pre": &files.FolderMetadata{}}, + putErr: putErr, + } + d := &driver{client: client, prefix: "pre"} + if err := d.Put("file", nil); !errors.Is(err, putErr) { + t.Fatalf("Put upload error = %v", err) + } + }) + + t.Run("concurrent folder creator wins", func(t *testing.T) { + createErr := errors.New("already exists") + client := &fakeDropbox{ + metaSeq: []files.IsMetadata{nil, &files.FolderMetadata{}}, + metaErrSeq: []error{errNotFound{}, nil}, + createErr: createErr, + } + d := &driver{client: client} + if err := d.MakeDir("dir"); err != nil { + t.Fatalf("MakeDir concurrent creator: %v", err) + } + }) + + t.Run("pagination and prefix fallback", func(t *testing.T) { + client := &fakeDropbox{listOut: &files.ListFolderResult{Entries: []files.IsMetadata{ + &files.FileMetadata{Metadata: files.Metadata{PathLower: "/pre/a"}}, + &files.FileMetadata{Metadata: files.Metadata{PathLower: "/pre/b"}}, + }}} + d := &driver{client: client, prefix: "pre"} + page, err := d.ListPage("", 0, 1) + if err != nil || len(page.Entries) != 1 || !page.HasMore { + t.Fatalf("ListPage = %+v, %v", page, err) + } + if got := d.stripPrefix("unrelated/file"); got != "unrelated/file" { + t.Fatalf("stripPrefix unrelated = %q", got) + } + }) +} + +// TestDropboxNestedCancellationEdges verifies pagination and directory repair +// recheck cancellation at each boundary around the context-free SDK methods. +func TestDropboxNestedCancellationEdges(t *testing.T) { + t.Run("continuation boundaries", func(t *testing.T) { + canceled, cancel := context.WithCancel(context.Background()) + cancel() + d := &driver{client: &fakeDropbox{}} + var entries []storagecore.Entry + if err := d.listContinue(canceled, files.NewListFolderContinueArg("cursor"), &entries); !errors.Is(err, context.Canceled) { + t.Fatalf("listContinue initial cancellation = %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + d = &driver{client: &fakeDropbox{continueHook: cancel}} + if err := d.listContinue(ctx, files.NewListFolderContinueArg("cursor"), &entries); !errors.Is(err, context.Canceled) { + t.Fatalf("listContinue post-call cancellation = %v", err) + } + for _, cancelAt := range []int{3, 4} { + ctx := &dropboxStepContext{Context: context.Background(), cancelAt: cancelAt} + d = &driver{client: &fakeDropbox{continueOut: &files.ListFolderResult{Entries: []files.IsMetadata{&files.FileMetadata{}}}}} + if err := d.listContinue(ctx, files.NewListFolderContinueArg("cursor"), &entries); !errors.Is(err, context.Canceled) { + t.Fatalf("listContinue cancellation at %d = %v", cancelAt, err) + } + } + }) + + t.Run("initial page boundaries", func(t *testing.T) { + for _, cancelAt := range []int{3, 4} { + ctx := &dropboxStepContext{Context: context.Background(), cancelAt: cancelAt} + d := &driver{client: &fakeDropbox{listOut: &files.ListFolderResult{Entries: []files.IsMetadata{&files.FileMetadata{}}}}} + var entries []storagecore.Entry + if err := d.listPage(ctx, files.NewListFolderArg(""), &entries); !errors.Is(err, context.Canceled) { + t.Fatalf("listPage cancellation at %d = %v", cancelAt, err) + } + } + }) + + t.Run("directory creation boundaries", func(t *testing.T) { + canceled, cancel := context.WithCancel(context.Background()) + cancel() + d := &driver{client: &fakeDropbox{}} + if err := d.ensureSingleDirectory(canceled, "/dir"); !errors.Is(err, context.Canceled) { + t.Fatalf("ensureSingleDirectory initial cancellation = %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + d = &driver{client: &fakeDropbox{metaHook: cancel}} + if err := d.ensureSingleDirectory(ctx, "/dir"); !errors.Is(err, context.Canceled) { + t.Fatalf("ensureSingleDirectory lookup cancellation = %v", err) + } + + ctx, cancel = context.WithCancel(context.Background()) + d = &driver{client: &fakeDropbox{metaErr: errNotFound{}, createHook: cancel}} + if err := d.ensureSingleDirectory(ctx, "/dir"); !errors.Is(err, context.Canceled) { + t.Fatalf("ensureSingleDirectory create cancellation = %v", err) + } + + ctx = &dropboxStepContext{Context: context.Background(), cancelAt: 4} + d = &driver{client: &fakeDropbox{ + metaSeq: []files.IsMetadata{nil, &files.FolderMetadata{}}, + metaErrSeq: []error{errNotFound{}, nil}, + createErr: errors.New("already exists"), + }} + if err := d.ensureSingleDirectory(ctx, "/dir"); !errors.Is(err, context.Canceled) { + t.Fatalf("ensureSingleDirectory repair cancellation = %v", err) + } + }) + + t.Run("walk emission cancellation", func(t *testing.T) { + ctx := &dropboxStepContext{Context: context.Background(), cancelAt: 5} + d := &driver{client: &fakeDropbox{listOut: &files.ListFolderResult{Entries: []files.IsMetadata{ + &files.FileMetadata{Metadata: files.Metadata{PathDisplay: "/file"}}, + }}}} + if err := d.walkPage(ctx, files.NewListFolderArg(""), func(storagecore.Entry) error { return nil }); !errors.Is(err, context.Canceled) { + t.Fatalf("walkPage cancellation = %v", err) + } + }) +} + +type dropboxStepContext struct { + context.Context + calls int + cancelAt int +} + +// Err begins returning context.Canceled at the configured observation count. +func (c *dropboxStepContext) Err() error { + c.calls++ + if c.calls >= c.cancelAt { + return context.Canceled + } + return nil +} + +type cancelEOFReadCloser struct { + cancel context.CancelFunc + done bool +} + +// Read cancels while returning a complete final chunk so cancellation is +// observed after a successful copy rather than as a transfer error. +func (r *cancelEOFReadCloser) Read(p []byte) (int, error) { + if r.done { + return 0, io.EOF + } + r.done = true + p[0] = 'x' + r.cancel() + return 1, io.EOF +} + +// Close makes response cleanup succeed. +func (*cancelEOFReadCloser) Close() error { return nil } + // createPaths projects recorded folder arguments into their API paths for assertions. func createPaths(args []*files.CreateFolderArg) []string { paths := make([]string, len(args)) diff --git a/driver/ftpstorage/ftp_unit_test.go b/driver/ftpstorage/ftp_unit_test.go index e9f3369..64ffdc5 100644 --- a/driver/ftpstorage/ftp_unit_test.go +++ b/driver/ftpstorage/ftp_unit_test.go @@ -7,6 +7,7 @@ import ( "io" "net" "net/textproto" + "os" "slices" "syscall" "testing" @@ -167,6 +168,37 @@ type fakeFTPConn struct { makeDirErr error } +// TestFTPInvalidPathsAndThinWrappers covers validation before any connection +// request and the background-context adapters omitted by the shared contract. +func TestFTPInvalidPathsAndThinWrappers(t *testing.T) { + d := &driver{} + calls := []func() error{ + func() error { _, err := d.GetContext(nil, "../bad"); return err }, + func() error { return d.PutContext(nil, "../bad", nil) }, + func() error { return d.MakeDirContext(nil, "../bad") }, + func() error { return d.DeleteContext(nil, "../bad") }, + func() error { _, err := d.StatContext(nil, "../bad"); return err }, + func() error { _, err := d.ExistsContext(nil, "../bad"); return err }, + func() error { _, err := d.ListContext(nil, "../bad"); return err }, + func() error { return d.WalkContext(nil, "../bad", func(storagecore.Entry) error { return nil }) }, + func() error { return d.CopyContext(nil, "../bad", "dst") }, + func() error { return d.CopyContext(nil, "src", "../bad") }, + func() error { return d.MoveContext(nil, "../bad", "dst") }, + func() error { return d.MoveContext(nil, "src", "../bad") }, + } + for index, call := range calls { + if err := call(); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("invalid-path call %d error = %v", index, err) + } + } + if err := d.MakeDir("../bad"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("MakeDir invalid path error = %v", err) + } + if _, err := d.ListPage("../bad", 0, 1); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("ListPage error = %v", err) + } +} + // Login returns the configured authentication failure. func (f *fakeFTPConn) Login(string, string) error { return f.loginErr } @@ -516,3 +548,184 @@ func TestFTPFakeWalkAndErrors(t *testing.T) { } }) } + +// TestFTPLifecycleAndMutationEdges covers connection setup, pagination, +// relocation failures, callback validation, and terminal close behavior. +func TestFTPLifecycleAndMutationEdges(t *testing.T) { + t.Run("dial and login failures", func(t *testing.T) { + dialErr := errors.New("dial") + d := &driver{dialFn: func() (ftpConn, error) { return nil, dialErr }} + if _, err := d.Get("file"); !errors.Is(err, dialErr) { + t.Fatalf("Get dial error = %v", err) + } + loginErr := errors.New("login") + d = &driver{user: "user", dialFn: func() (ftpConn, error) { return &fakeFTPConn{loginErr: loginErr}, nil }} + if _, err := d.Get("file"); !errors.Is(err, loginErr) { + t.Fatalf("Get login error = %v", err) + } + }) + + t.Run("pagination callback and mkdir", func(t *testing.T) { + conn := &fakeFTPConn{listEntries: []*ftp.Entry{ + {Name: "a", Type: ftp.EntryTypeFile}, + {Name: "b", Type: ftp.EntryTypeFile}, + }} + d := &driver{conn: conn, dialFn: func() (ftpConn, error) { return conn, nil }} + page, err := d.ListPage("", 0, 1) + if err != nil || len(page.Entries) != 1 || !page.HasMore { + t.Fatalf("ListPage = %+v, %v", page, err) + } + if err := d.Walk("", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Walk nil callback error = %v", err) + } + if err := d.MakeDir(""); err != nil { + t.Fatalf("MakeDir root: %v", err) + } + conn.makeDirErr = errors.New("mkdir") + if err := d.MakeDir("dir"); err == nil { + t.Fatal("MakeDir returned nil error") + } + }) + + t.Run("copy and move failures", func(t *testing.T) { + conn := &fakeFTPConn{retrErr: errors.New("550 missing"), fileSizeErr: errors.New("550 missing")} + d := &driver{conn: conn, dialFn: func() (ftpConn, error) { return conn, nil }} + if err := d.Copy("missing", "copy"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Copy missing error = %v", err) + } + if err := d.Move("missing", "moved"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Move missing error = %v", err) + } + conn = &fakeFTPConn{ + listEntries: []*ftp.Entry{{Name: "file", Type: ftp.EntryTypeFile}}, + storErr: errors.New("rename"), + } + d = &driver{conn: conn, dialFn: func() (ftpConn, error) { return conn, nil }} + if err := d.Move("file", "moved"); err == nil { + t.Fatal("Move rename returned nil error") + } + }) + + t.Run("close is terminal", func(t *testing.T) { + quitErr := errors.New("quit") + conn := &fakeFTPConn{quitErr: quitErr} + d := &driver{conn: conn, dialFn: func() (ftpConn, error) { return conn, nil }} + if err := d.Close(); !errors.Is(err, quitErr) { + t.Fatalf("Close error = %v", err) + } + if err := d.Close(); !errors.Is(err, quitErr) { + t.Fatalf("second Close error = %v", err) + } + calls := []func() error{ + func() error { _, err := d.Get("file"); return err }, + func() error { return d.Put("file", nil) }, + func() error { return d.MakeDir("dir") }, + func() error { return d.Delete("file") }, + func() error { _, err := d.Stat("file"); return err }, + func() error { _, err := d.Exists("file"); return err }, + func() error { _, err := d.List(""); return err }, + func() error { return d.Walk("", func(storagecore.Entry) error { return nil }) }, + func() error { return d.Copy("file", "copy") }, + func() error { return d.Move("file", "moved") }, + } + for index, call := range calls { + if err := call(); err == nil { + t.Fatalf("closed call %d returned nil error", index) + } + } + }) +} + +// TestFTPPureHelperEdges covers configuration and stream helpers that do not +// require an FTP server. +func TestFTPPureHelperEdges(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := newFromDiskConfig(ctx, storagecore.ResolvedConfig{FTPHost: "host"}); !errors.Is(err, context.Canceled) { + t.Fatalf("newFromDiskConfig canceled error = %v", err) + } + for _, port := range []int{-1, 65536} { + if _, err := New(Config{Host: "host", Port: port}); err == nil { + t.Fatalf("New accepted port %d", port) + } + } + if _, err := New(Config{Host: "host", Prefix: "../bad"}); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("New invalid prefix error = %v", err) + } + store, err := New(Config{Host: "host", TLS: true}) + if err != nil { + t.Fatalf("New TLS: %v", err) + } + if d := store.(*driver); !d.tls || d.serverName != "host" { + t.Fatalf("TLS driver = %+v", d) + } + + d := &driver{} + if err := d.runConnLocked(func(ftpConn) error { return nil }); err == nil { + t.Fatal("runConnLocked without connection returned nil error") + } + d.closed = true + if _, err := d.ensureConnLocked(); !errors.Is(err, os.ErrClosed) { + t.Fatalf("ensureConnLocked closed error = %v", err) + } + if isDirectoryExistsError(nil) { + t.Fatal("isDirectoryExistsError(nil) = true") + } + if !shouldReconnectFTP(temporaryNetError{}) { + t.Fatal("shouldReconnectFTP net error = false") + } + + if _, err := (&contextReader{ctx: ctx, reader: bytes.NewReader(nil)}).Read(make([]byte, 1)); !errors.Is(err, context.Canceled) { + t.Fatalf("contextReader canceled error = %v", err) + } + if _, err := copyContext(ctx, io.Discard, bytes.NewReader(nil)); !errors.Is(err, context.Canceled) { + t.Fatalf("copyContext canceled error = %v", err) + } + writeErr := errors.New("write") + if _, err := copyContext(context.Background(), ftpFailingWriter{err: writeErr}, bytes.NewReader([]byte("x"))); !errors.Is(err, writeErr) { + t.Fatalf("copyContext write error = %v", err) + } + if _, err := copyContext(context.Background(), ftpShortWriter{}, bytes.NewReader([]byte("x"))); !errors.Is(err, io.ErrShortWrite) { + t.Fatalf("copyContext short write error = %v", err) + } + if _, err := copyContext(context.Background(), io.Discard, ftpErrorReader{err: writeErr}); !errors.Is(err, writeErr) { + t.Fatalf("copyContext read error = %v", err) + } + cleanup := errors.New("cleanup") + if err := joinCleanup(nil, cleanup); !errors.Is(err, cleanup) { + t.Fatalf("joinCleanup cleanup error = %v", err) + } + if err := joinCleanup(writeErr, cleanup); !errors.Is(err, writeErr) || !errors.Is(err, cleanup) { + t.Fatalf("joinCleanup combined error = %v", err) + } +} + +type temporaryNetError struct{} + +// Error returns a stable network failure message. +func (temporaryNetError) Error() string { return "temporary network failure" } + +// Timeout classifies the fixture as a non-timeout network error. +func (temporaryNetError) Timeout() bool { return false } + +// Temporary classifies the fixture as a retryable network error. +func (temporaryNetError) Temporary() bool { return true } + +type ftpFailingWriter struct { + err error +} + +// Write injects a destination failure. +func (w ftpFailingWriter) Write([]byte) (int, error) { return 0, w.err } + +type ftpShortWriter struct{} + +// Write accepts no bytes without error to exercise short-write handling. +func (ftpShortWriter) Write([]byte) (int, error) { return 0, nil } + +type ftpErrorReader struct { + err error +} + +// Read injects a source failure. +func (r ftpErrorReader) Read([]byte) (int, error) { return 0, r.err } diff --git a/driver/gcsstorage/gcs_unit_test.go b/driver/gcsstorage/gcs_unit_test.go index 991a585..ee65c95 100644 --- a/driver/gcsstorage/gcs_unit_test.go +++ b/driver/gcsstorage/gcs_unit_test.go @@ -134,6 +134,38 @@ type fakeGCSClient struct { bucket *fakeGCSBucket } +// TestGCSInvalidPathsAndThinWrappers covers validation before any SDK request +// and the background-context adapters omitted by the shared contract. +func TestGCSInvalidPathsAndThinWrappers(t *testing.T) { + d := &driver{} + calls := []func() error{ + func() error { _, err := d.GetContext(nil, "../bad"); return err }, + func() error { return d.PutContext(nil, "../bad", nil) }, + func() error { return d.MakeDirContext(nil, "../bad") }, + func() error { return d.DeleteContext(nil, "../bad") }, + func() error { _, err := d.StatContext(nil, "../bad"); return err }, + func() error { _, err := d.ExistsContext(nil, "../bad"); return err }, + func() error { _, err := d.ListContext(nil, "../bad"); return err }, + func() error { return d.WalkContext(nil, "../bad", func(storagecore.Entry) error { return nil }) }, + func() error { return d.CopyContext(nil, "../bad", "dst") }, + func() error { return d.CopyContext(nil, "src", "../bad") }, + func() error { return d.MoveContext(nil, "../bad", "dst") }, + func() error { return d.MoveContext(nil, "src", "../bad") }, + func() error { _, err := d.URLContext(nil, "../bad"); return err }, + } + for index, call := range calls { + if err := call(); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("invalid-path call %d error = %v", index, err) + } + } + if err := d.MakeDir("../bad"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("MakeDir invalid path error = %v", err) + } + if _, err := d.ListPage("../bad", 0, 1); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("ListPage error = %v", err) + } +} + // Bucket returns the fake bucket shared by driver tests. func (f fakeGCSClient) Bucket(string) gcsBucketHandle { return f.bucket } @@ -141,12 +173,13 @@ func (f fakeGCSClient) Bucket(string) gcsBucketHandle { return f.bucket } func (f fakeGCSClient) Close() error { return nil } type fakeGCSBucket struct { - object *fakeGCSObject - objects []*storage.ObjectAttrs - objectsErr error - signedURL string - signedErr error - queryPrefix string + object *fakeGCSObject + objects []*storage.ObjectAttrs + objectsErr error + signedURL string + signedErr error + queryPrefix string + iteratorHook func() } // Object returns the fake object shared by bucket operations. @@ -168,7 +201,7 @@ func (f *fakeGCSBucket) Objects(_ context.Context, q *storage.Query) gcsObjectIt } } } - return &fakeGCSIterator{items: items, err: f.objectsErr} + return &fakeGCSIterator{items: items, err: f.objectsErr, hook: f.iteratorHook} } // SignedURL returns the configured link or signing failure. @@ -181,6 +214,7 @@ type fakeGCSObject struct { readErr error readBody io.ReadCloser writeErr error + writeHook func() abortErr error abortWith error closeCalls int @@ -226,6 +260,9 @@ type fakeGCSWriter struct { // Write captures uploaded bytes unless a configured transfer failure occurs. func (w *fakeGCSWriter) Write(p []byte) (int, error) { + if w.parent.writeHook != nil { + w.parent.writeHook() + } if w.parent.writeErr != nil { return 0, w.parent.writeErr } @@ -249,10 +286,14 @@ type fakeGCSIterator struct { items []*storage.ObjectAttrs err error idx int + hook func() } // Next yields configured metadata in order before returning iterator.Done. func (it *fakeGCSIterator) Next() (*storage.ObjectAttrs, error) { + if it.hook != nil { + it.hook() + } if it.err != nil { return nil, it.err } @@ -536,6 +577,415 @@ func TestGCSFakeWalkAndErrorBranches(t *testing.T) { }) } +// TestGCSDirectoryAndLifecycleEdges covers virtual-directory decisions and the +// terminal client lifecycle without requiring network credentials. +func TestGCSDirectoryAndLifecycleEdges(t *testing.T) { + t.Run("explicit empty directory", func(t *testing.T) { + object := &fakeGCSObject{attrsErr: storage.ErrObjectNotExist} + bucket := &fakeGCSBucket{ + object: object, + objects: []*storage.ObjectAttrs{{Name: "pre/empty/"}}, + } + d := &driver{client: fakeGCSClient{bucket: bucket}, bucket: "bucket", prefix: "pre"} + + entry, err := d.Stat("empty") + if err != nil || !entry.IsDir { + t.Fatalf("Stat directory = %+v, %v", entry, err) + } + if err := d.Delete("empty"); err != nil { + t.Fatalf("Delete empty directory: %v", err) + } + }) + + t.Run("non-empty directory", func(t *testing.T) { + object := &fakeGCSObject{attrsErr: storage.ErrObjectNotExist} + bucket := &fakeGCSBucket{ + object: object, + objects: []*storage.ObjectAttrs{ + {Name: "pre/folder/"}, + {Name: "pre/folder/file.txt", Size: 1}, + }, + } + d := &driver{client: fakeGCSClient{bucket: bucket}, bucket: "bucket", prefix: "pre"} + if err := d.Delete("folder"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Delete non-empty directory error = %v", err) + } + }) + + t.Run("implicit and missing directories", func(t *testing.T) { + object := &fakeGCSObject{attrsErr: storage.ErrObjectNotExist} + bucket := &fakeGCSBucket{object: object, objects: []*storage.ObjectAttrs{{Name: "pre/implicit/file.txt"}}} + d := &driver{client: fakeGCSClient{bucket: bucket}, bucket: "bucket", prefix: "pre"} + entry, err := d.Stat("implicit") + if err != nil || !entry.IsDir { + t.Fatalf("Stat implicit directory = %+v, %v", entry, err) + } + bucket.objects = nil + if _, err := d.Stat("missing"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Stat missing error = %v", err) + } + if exists, err := d.Exists(""); err != nil || exists { + t.Fatalf("Exists root = %v, %v", exists, err) + } + if exists, err := d.Exists("missing"); err != nil || exists { + t.Fatalf("Exists missing = %v, %v", exists, err) + } + }) + + t.Run("listing filters markers and logical root", func(t *testing.T) { + object := &fakeGCSObject{} + bucket := &fakeGCSBucket{object: object, objects: []*storage.ObjectAttrs{ + {Name: "pre/"}, + {Name: "pre/file.txt", Size: 2}, + {Prefix: "pre/dir/"}, + }} + d := &driver{client: fakeGCSClient{bucket: bucket}, bucket: "bucket", prefix: "pre"} + page, err := d.ListPage("", 0, 1) + if err != nil || len(page.Entries) != 1 || !page.HasMore { + t.Fatalf("ListPage = %+v, %v", page, err) + } + if err := d.Walk("", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Walk nil callback error = %v", err) + } + object.attrsErr = storage.ErrObjectNotExist + bucket.objects = nil + if err := d.Walk("missing", func(storagecore.Entry) error { return nil }); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Walk missing error = %v", err) + } + }) + + t.Run("close is terminal", func(t *testing.T) { + bucket := &fakeGCSBucket{object: &fakeGCSObject{}} + d := &driver{client: fakeGCSClient{bucket: bucket}, bucket: "bucket"} + if err := d.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + if err := d.Close(); err != nil { + t.Fatalf("second Close: %v", err) + } + calls := []func() error{ + func() error { _, err := d.Get("file"); return err }, + func() error { return d.Put("file", nil) }, + func() error { return d.MakeDir("dir") }, + func() error { return d.Delete("file") }, + func() error { _, err := d.Stat("file"); return err }, + func() error { _, err := d.Exists("file"); return err }, + func() error { _, err := d.List(""); return err }, + func() error { return d.Walk("", func(storagecore.Entry) error { return nil }) }, + func() error { return d.Copy("file", "copy") }, + func() error { return d.Move("file", "moved") }, + func() error { _, err := d.URL("file"); return err }, + } + for index, call := range calls { + if err := call(); err == nil { + t.Fatalf("closed call %d returned nil error", index) + } + } + }) +} + +// TestGCSPureHelperEdges verifies endpoint validation, portable error mapping, +// and transfer failures independently of the cloud client. +func TestGCSPureHelperEdges(t *testing.T) { + for _, endpoint := range []string{"relative", "ftp://host", "http://user@host", "http://host?query=1", "http://host#fragment"} { + if _, err := normalizeEndpoint(endpoint); err == nil { + t.Fatalf("normalizeEndpoint(%q) returned nil error", endpoint) + } + } + if endpoint, err := normalizeEndpoint("http://localhost"); err != nil || endpoint != "http://localhost/storage/v1/" { + t.Fatalf("normalizeEndpoint default = %q, %v", endpoint, err) + } + if endpoint, err := normalizeEndpoint("https://localhost/custom/"); err != nil || endpoint != "https://localhost/custom/" { + t.Fatalf("normalizeEndpoint custom = %q, %v", endpoint, err) + } + if err := wrapError(nil); err != nil { + t.Fatalf("wrapError nil = %v", err) + } + for _, code := range []int{401, 403} { + if err := wrapError(&googleapi.Error{Code: code}); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("wrapError status %d = %v", code, err) + } + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := copyContext(ctx, io.Discard, strings.NewReader("x")); !errors.Is(err, context.Canceled) { + t.Fatalf("copyContext cancellation error = %v", err) + } + writeErr := errors.New("write") + if _, err := copyContext(context.Background(), gcsFailingWriter{err: writeErr}, strings.NewReader("x")); !errors.Is(err, writeErr) { + t.Fatalf("copyContext write error = %v", err) + } + if _, err := copyContext(context.Background(), gcsShortWriter{}, strings.NewReader("x")); !errors.Is(err, io.ErrShortWrite) { + t.Fatalf("copyContext short write error = %v", err) + } + primary := errors.New("primary") + cleanup := errors.New("cleanup") + if err := joinCleanup(primary, cleanup); !errors.Is(err, primary) || !errors.Is(err, cleanup) { + t.Fatalf("joinCleanup error = %v", err) + } + if err := joinCleanup(nil, cleanup); !errors.Is(err, cleanup) { + t.Fatalf("joinCleanup cleanup = %v", err) + } +} + +// TestGCSListingCancellationAndFilters covers cancellation between iterator +// results and namespace-marker suppression in list and walk operations. +func TestGCSListingCancellationAndFilters(t *testing.T) { + t.Run("list cancellation", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + bucket := &fakeGCSBucket{ + object: &fakeGCSObject{}, + objects: []*storage.ObjectAttrs{{Name: "pre/file"}}, + iteratorHook: cancel, + } + d := &driver{client: fakeGCSClient{bucket: bucket}, bucket: "bucket", prefix: "pre"} + if _, err := d.ListContext(ctx, ""); !errors.Is(err, context.Canceled) { + t.Fatalf("ListContext cancellation = %v", err) + } + }) + t.Run("walk cancellation", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + bucket := &fakeGCSBucket{ + object: &fakeGCSObject{}, + objects: []*storage.ObjectAttrs{{Name: "file"}}, + iteratorHook: cancel, + } + d := &driver{client: fakeGCSClient{bucket: bucket}, bucket: "bucket"} + if err := d.WalkContext(ctx, "", func(storagecore.Entry) error { return nil }); !errors.Is(err, context.Canceled) { + t.Fatalf("WalkContext cancellation = %v", err) + } + }) + t.Run("markers are filtered", func(t *testing.T) { + bucket := &fakeGCSBucket{object: &fakeGCSObject{}, objects: []*storage.ObjectAttrs{ + {Prefix: "pre/"}, + {Name: "pre/"}, + {Name: "pre"}, + }} + d := &driver{client: fakeGCSClient{bucket: bucket}, bucket: "bucket", prefix: "pre"} + entries, err := d.List("") + if err != nil || len(entries) != 0 { + t.Fatalf("List marker entries = %+v, %v", entries, err) + } + }) + t.Run("walk callback cancellation", func(t *testing.T) { + bucket := &fakeGCSBucket{object: &fakeGCSObject{}, objects: []*storage.ObjectAttrs{{Name: "a"}, {Name: "b"}}} + d := &driver{client: fakeGCSClient{bucket: bucket}, bucket: "bucket"} + ctx, cancel := context.WithCancel(context.Background()) + seen := 0 + err := d.WalkContext(ctx, "", func(storagecore.Entry) error { + seen++ + cancel() + return nil + }) + if !errors.Is(err, context.Canceled) || seen != 1 { + t.Fatalf("WalkContext callback cancellation = %v after %d entries", err, seen) + } + }) +} + +// TestGCSBackendFailureEdges verifies cleanup, mutation, and iterator failures +// remain observable instead of being mistaken for missing cloud objects. +func TestGCSBackendFailureEdges(t *testing.T) { + t.Run("constructor failures", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := newFromDiskConfig(ctx, storagecore.ResolvedConfig{GCSBucket: "bucket"}); !errors.Is(err, context.Canceled) { + t.Fatalf("newFromDiskConfig cancellation = %v", err) + } + if _, err := newClient(ctx, storagecore.ResolvedConfig{}); !errors.Is(err, context.Canceled) { + t.Fatalf("newClient cancellation = %v", err) + } + if _, err := newFromDiskConfig(context.Background(), storagecore.ResolvedConfig{GCSBucket: "bucket", GCSEndpoint: "relative"}); err == nil { + t.Fatal("newFromDiskConfig invalid endpoint returned nil error") + } + if _, err := newClient(context.Background(), storagecore.ResolvedConfig{GCSCredentialsJSON: "{"}); err == nil { + t.Fatal("newClient invalid credentials returned nil error") + } + }) + + t.Run("reader close failure", func(t *testing.T) { + closeErr := errors.New("close") + object := &fakeGCSObject{readBody: &gcsCloseReader{Reader: strings.NewReader("data"), err: closeErr}} + d := newFakeGCSDriver(object, nil) + if _, err := d.Get("file"); !errors.Is(err, closeErr) { + t.Fatalf("Get close error = %v", err) + } + }) + + t.Run("post-read cancellation", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + object := &fakeGCSObject{readBody: &gcsCancelEOFReader{cancel: cancel}} + d := newFakeGCSDriver(object, nil) + if _, err := d.GetContext(ctx, "file"); !errors.Is(err, context.Canceled) { + t.Fatalf("GetContext cancellation = %v", err) + } + }) + + t.Run("post-write cancellation", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + object := &fakeGCSObject{writeHook: cancel} + d := newFakeGCSDriver(object, nil) + if err := d.PutContext(ctx, "file", []byte("data")); !errors.Is(err, context.Canceled) || object.abortCalls != 1 { + t.Fatalf("PutContext cancellation = %v aborts=%d", err, object.abortCalls) + } + }) + + t.Run("post-copy cancellation and close failure", func(t *testing.T) { + object := &fakeGCSObject{} + d := newFakeGCSDriver(object, nil) + ctx := &gcsStepContext{Context: context.Background(), cancelAt: 3} + if err := d.PutContext(ctx, "file", nil); !errors.Is(err, context.Canceled) || object.abortCalls != 1 { + t.Fatalf("PutContext post-copy cancellation = %v aborts=%d", err, object.abortCalls) + } + + closeErr := errors.New("close") + object = &fakeGCSObject{writeErr: closeErr} + d = newFakeGCSDriver(object, nil) + if err := d.Put("file", nil); !errors.Is(err, closeErr) { + t.Fatalf("Put close error = %v", err) + } + }) + + t.Run("writer and delete failures", func(t *testing.T) { + writeErr := errors.New("write close") + object := &fakeGCSObject{writeErr: writeErr} + d := newFakeGCSDriver(object, nil) + if err := d.MakeDir("dir"); !errors.Is(err, writeErr) { + t.Fatalf("MakeDir close error = %v", err) + } + + deleteErr := errors.New("delete") + object = &fakeGCSObject{attrs: &storage.ObjectAttrs{}, deleteErr: deleteErr} + d = newFakeGCSDriver(object, nil) + if err := d.Delete("file"); !errors.Is(err, deleteErr) { + t.Fatalf("Delete object error = %v", err) + } + }) + + t.Run("directory iterator failure", func(t *testing.T) { + iterErr := errors.New("iterate") + object := &fakeGCSObject{attrsErr: storage.ErrObjectNotExist} + d := newFakeGCSDriver(object, nil) + d.client.(fakeGCSClient).bucket.objectsErr = iterErr + if err := d.Delete("dir"); !errors.Is(err, iterErr) { + t.Fatalf("Delete iterator error = %v", err) + } + }) + + t.Run("metadata and walk failures", func(t *testing.T) { + backendErr := errors.New("backend") + object := &fakeGCSObject{attrsErr: backendErr} + d := newFakeGCSDriver(object, nil) + if _, err := d.Stat("file"); !errors.Is(err, backendErr) { + t.Fatalf("Stat metadata error = %v", err) + } + if err := d.Walk("file", func(storagecore.Entry) error { return nil }); !errors.Is(err, backendErr) { + t.Fatalf("Walk metadata error = %v", err) + } + + object.attrsErr = storage.ErrObjectNotExist + d.client.(fakeGCSClient).bucket.objectsErr = backendErr + if _, err := d.Stat("file"); !errors.Is(err, backendErr) { + t.Fatalf("Stat iterator error = %v", err) + } + if err := d.Walk("file", func(storagecore.Entry) error { return nil }); !errors.Is(err, backendErr) { + t.Fatalf("Walk iterator error = %v", err) + } + }) + + t.Run("empty walk records", func(t *testing.T) { + object := &fakeGCSObject{attrsErr: storage.ErrObjectNotExist} + d := newFakeGCSDriver(object, []*storage.ObjectAttrs{{Name: ""}}) + if err := d.Walk("", func(storagecore.Entry) error { return nil }); err != nil { + t.Fatalf("Walk empty records: %v", err) + } + }) + + t.Run("empty list records and move failures", func(t *testing.T) { + object := &fakeGCSObject{attrsErr: storage.ErrObjectNotExist} + d := newFakeGCSDriver(object, []*storage.ObjectAttrs{{Name: ""}}) + if entries, err := d.List(""); err != nil || len(entries) != 0 { + t.Fatalf("List empty record = %+v, %v", entries, err) + } + if err := d.Move("", "destination"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Move root error = %v", err) + } + if err := d.Move("missing", "destination"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Move missing source error = %v", err) + } + + readErr := errors.New("read") + object = &fakeGCSObject{attrs: &storage.ObjectAttrs{}, readErr: readErr} + d = newFakeGCSDriver(object, nil) + if err := d.Move("source", "destination"); !errors.Is(err, readErr) { + t.Fatalf("Move copy error = %v", err) + } + }) +} + +// newFakeGCSDriver constructs the minimal in-memory SDK boundary shared by +// backend edge tests. +func newFakeGCSDriver(object *fakeGCSObject, objects []*storage.ObjectAttrs) *driver { + bucket := &fakeGCSBucket{object: object, objects: objects} + return &driver{client: fakeGCSClient{bucket: bucket}, bucket: "bucket"} +} + +type gcsCloseReader struct { + io.Reader + err error +} + +// Close returns the configured cleanup failure. +func (r *gcsCloseReader) Close() error { return r.err } + +type gcsCancelEOFReader struct { + cancel context.CancelFunc + done bool +} + +// Read cancels while completing the final chunk so GetContext observes the +// post-transfer context check. +func (r *gcsCancelEOFReader) Read(p []byte) (int, error) { + if r.done { + return 0, io.EOF + } + r.done = true + p[0] = 'x' + r.cancel() + return 1, io.EOF +} + +// Close completes reader cleanup successfully. +func (*gcsCancelEOFReader) Close() error { return nil } + +type gcsStepContext struct { + context.Context + calls int + cancelAt int +} + +// Err begins returning context.Canceled at the configured observation count. +func (c *gcsStepContext) Err() error { + c.calls++ + if c.calls >= c.cancelAt { + return context.Canceled + } + return nil +} + +type gcsFailingWriter struct { + err error +} + +// Write injects the configured transfer failure. +func (w gcsFailingWriter) Write([]byte) (int, error) { return 0, w.err } + +type gcsShortWriter struct{} + +// Write accepts no bytes without an error to exercise short-write handling. +func (gcsShortWriter) Write([]byte) (int, error) { return 0, nil } + type errReader struct{} // Read injects a deterministic transfer failure after a reader opens successfully. diff --git a/driver/localstorage/local_more_test.go b/driver/localstorage/local_more_test.go index e558d3e..f18574e 100644 --- a/driver/localstorage/local_more_test.go +++ b/driver/localstorage/local_more_test.go @@ -3,6 +3,7 @@ package localstorage import ( "context" "errors" + "io" "os" "path/filepath" "slices" @@ -240,3 +241,345 @@ func TestLocalModTimeAndRelativeEdgeCases(t *testing.T) { t.Fatalf("modTime missing error = %v", err) } } + +// TestLocalInvalidPathsAndThinWrappers covers validation before filesystem +// access and the background-context adapters omitted by the shared contract. +func TestLocalInvalidPathsAndThinWrappers(t *testing.T) { + d := &driver{} + calls := []func() error{ + func() error { _, err := d.GetContext(nil, "../bad"); return err }, + func() error { return d.PutContext(nil, "../bad", nil) }, + func() error { return d.MakeDirContext(nil, "../bad") }, + func() error { return d.DeleteContext(nil, "../bad") }, + func() error { _, err := d.StatContext(nil, "../bad"); return err }, + func() error { _, err := d.ExistsContext(nil, "../bad"); return err }, + func() error { _, err := d.ListContext(nil, "../bad"); return err }, + func() error { _, err := d.ListPageContext(nil, "../bad", 0, 1); return err }, + func() error { return d.WalkContext(nil, "../bad", func(storagecore.Entry) error { return nil }) }, + func() error { return d.CopyContext(nil, "../bad", "dst") }, + func() error { return d.CopyContext(nil, "src", "../bad") }, + func() error { return d.MoveContext(nil, "../bad", "dst") }, + func() error { return d.MoveContext(nil, "src", "../bad") }, + } + for index, call := range calls { + if err := call(); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("invalid-path call %d error = %v", index, err) + } + } + if err := d.MakeDir("../bad"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("MakeDir invalid path error = %v", err) + } + if _, err := d.ListPage("../bad", 0, 1); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("ListPage invalid path error = %v", err) + } +} + +// TestLocalFilesystemAndStreamFailures covers unavailable roots and atomic +// transfer failures without depending on platform-specific permissions. +func TestLocalFilesystemAndStreamFailures(t *testing.T) { + t.Run("unavailable root", func(t *testing.T) { + d := &driver{root: filepath.Join(t.TempDir(), "missing")} + calls := []func() error{ + func() error { _, err := d.Get("file"); return err }, + func() error { return d.Put("file", nil) }, + func() error { return d.MakeDir("dir") }, + func() error { return d.Delete("file") }, + func() error { _, err := d.Stat("file"); return err }, + func() error { _, err := d.Exists("file"); return err }, + func() error { _, err := d.List(""); return err }, + func() error { return d.Walk("", func(storagecore.Entry) error { return nil }) }, + func() error { return d.Copy("file", "copy") }, + func() error { return d.Move("file", "moved") }, + func() error { _, err := d.modTime(context.Background(), "file"); return err }, + } + for index, call := range calls { + if err := call(); err == nil { + t.Fatalf("unavailable-root call %d returned nil error", index) + } + } + }) + + t.Run("stream helpers", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if err := copyContext(ctx, io.Discard, &localErrorReader{}); !errors.Is(err, context.Canceled) { + t.Fatalf("copyContext cancellation error = %v", err) + } + readErr := errors.New("read") + if err := copyContext(context.Background(), io.Discard, &localErrorReader{err: readErr}); !errors.Is(err, readErr) { + t.Fatalf("copyContext read error = %v", err) + } + writeErr := errors.New("write") + if err := copyContext(context.Background(), &localFailingWriter{err: writeErr}, &localDataReader{}); !errors.Is(err, writeErr) { + t.Fatalf("copyContext write error = %v", err) + } + if err := copyContext(context.Background(), &localShortWriter{}, &localDataReader{}); !errors.Is(err, io.ErrShortWrite) { + t.Fatalf("copyContext short write error = %v", err) + } + primary := errors.New("primary") + cleanup := errors.New("cleanup") + if err := joinCleanup(primary, cleanup); !errors.Is(err, primary) || !errors.Is(err, cleanup) { + t.Fatalf("joinCleanup error = %v", err) + } + if err := joinCleanup(nil, cleanup); !errors.Is(err, cleanup) { + t.Fatalf("joinCleanup cleanup error = %v", err) + } + }) + + t.Run("atomic cancellation and helper projections", func(t *testing.T) { + rootPath := t.TempDir() + root, err := os.OpenRoot(rootPath) + if err != nil { + t.Fatalf("OpenRoot: %v", err) + } + defer root.Close() + ctx, cancel := context.WithCancel(context.Background()) + err = atomicWrite(ctx, root, "file", 0o644, func(file *os.File) error { + cancel() + return nil + }) + if !errors.Is(err, context.Canceled) { + t.Fatalf("atomicWrite cancellation error = %v", err) + } + d := &driver{root: rootPath, prefix: "pre"} + if _, err := d.displayName("outside/file"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("displayName outside error = %v", err) + } + if mode := existingMode(root, "missing", 0o600); mode != 0o600 { + t.Fatalf("existingMode fallback = %v", mode) + } + }) +} + +// TestLocalConstructorAndAtomicEdges covers setup failures and operation +// cancellation at filesystem boundaries using only temporary directories. +func TestLocalConstructorAndAtomicEdges(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := NewContext(ctx, Config{Root: t.TempDir()}); !errors.Is(err, context.Canceled) { + t.Fatalf("NewContext canceled error = %v", err) + } + if _, err := New(Config{}); err == nil { + t.Fatal("New missing root returned nil error") + } + fileRoot := filepath.Join(t.TempDir(), "file") + if err := os.WriteFile(fileRoot, []byte("x"), 0o600); err != nil { + t.Fatalf("WriteFile root: %v", err) + } + if _, err := New(Config{Root: fileRoot}); err == nil { + t.Fatal("New file root returned nil error") + } + rootPath := t.TempDir() + if err := os.WriteFile(filepath.Join(rootPath, "pre"), []byte("x"), 0o600); err != nil { + t.Fatalf("WriteFile prefix: %v", err) + } + if _, err := New(Config{Root: rootPath, Prefix: "pre"}); err == nil { + t.Fatal("New colliding prefix returned nil error") + } + + rootPath = t.TempDir() + root, err := os.OpenRoot(rootPath) + if err != nil { + t.Fatalf("OpenRoot: %v", err) + } + defer root.Close() + writeErr := errors.New("write") + if err := atomicWrite(context.Background(), root, "write-failure", 0o600, func(*os.File) error { return writeErr }); !errors.Is(err, writeErr) { + t.Fatalf("atomicWrite callback error = %v", err) + } + if err := atomicWrite(context.Background(), root, "missing/file", 0o600, func(*os.File) error { return nil }); err == nil { + t.Fatal("atomicWrite missing parent returned nil error") + } + if err := root.Mkdir("target", 0o755); err != nil { + t.Fatalf("Mkdir target: %v", err) + } + if err := atomicWrite(context.Background(), root, "target", 0o600, func(file *os.File) error { + _, err := file.Write([]byte("x")) + return err + }); err == nil { + t.Fatal("atomicWrite directory target returned nil error") + } + if mode := existingMode(root, "target", 0o640); mode != 0o640 { + t.Fatalf("existingMode directory fallback = %v", mode) + } +} + +// TestLocalFilesystemCollisionEdges verifies ordinary filesystem collisions +// remain visible through the portable storage error identities. +func TestLocalFilesystemCollisionEdges(t *testing.T) { + rootPath := t.TempDir() + store, err := New(Config{Root: rootPath}) + if err != nil { + t.Fatalf("New: %v", err) + } + d := store.(*driver) + if err := d.Put("source", []byte("source")); err != nil { + t.Fatalf("Put source: %v", err) + } + if err := d.Put("blocker", []byte("blocker")); err != nil { + t.Fatalf("Put blocker: %v", err) + } + + calls := []struct { + name string + call func() error + }{ + {name: "put beneath file", call: func() error { return d.Put("blocker/child", nil) }}, + {name: "mkdir beneath file", call: func() error { return d.MakeDir("blocker/child") }}, + {name: "copy beneath file", call: func() error { return d.Copy("source", "blocker/copy") }}, + {name: "move beneath file", call: func() error { return d.Move("source", "blocker/move") }}, + } + for _, test := range calls { + t.Run(test.name, func(t *testing.T) { + if err := test.call(); err == nil { + t.Fatal("operation returned nil error") + } + }) + } + + if err := d.MakeDir(""); err != nil { + t.Fatalf("MakeDir root: %v", err) + } + if err := d.MakeDir("nonempty"); err != nil { + t.Fatalf("MakeDir nonempty: %v", err) + } + if err := d.Put("nonempty/child", nil); err != nil { + t.Fatalf("Put nonempty child: %v", err) + } + if err := d.Delete("nonempty"); err == nil { + t.Fatalf("Delete nonempty directory error = %v", err) + } + if _, err := d.Get("nonempty"); err == nil { + t.Fatal("Get directory returned nil error") + } + if err := d.Copy("source", "source"); err != nil { + t.Fatalf("Copy same path: %v", err) + } + if err := d.Move("source", "source"); err != nil { + t.Fatalf("Move same path: %v", err) + } + if err := d.Walk("", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Walk nil callback error = %v", err) + } + if _, err := d.userRelative(filepath.Join(filepath.Dir(rootPath), "outside")); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("userRelative outside error = %v", err) + } + if _, err := d.modTime(context.Background(), "../bad"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("modTime invalid path error = %v", err) + } +} + +// TestLocalAtomicPostWriteEdges covers cancellation after synchronization and +// direct file-write failures without installing incomplete content. +func TestLocalAtomicPostWriteEdges(t *testing.T) { + rootPath := t.TempDir() + root, err := os.OpenRoot(rootPath) + if err != nil { + t.Fatalf("OpenRoot: %v", err) + } + defer root.Close() + + ctx := &stepCancelContext{Context: context.Background(), cancelAt: 2} + if err := atomicWrite(ctx, root, "canceled", 0o600, func(file *os.File) error { + _, err := file.Write([]byte("data")) + return err + }); !errors.Is(err, context.Canceled) { + t.Fatalf("atomicWrite post-sync cancellation error = %v", err) + } + if _, err := root.Stat("canceled"); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("canceled target stat error = %v", err) + } + + file, err := os.CreateTemp(rootPath, "closed-") + if err != nil { + t.Fatalf("CreateTemp: %v", err) + } + if err := file.Close(); err != nil { + t.Fatalf("Close temporary file: %v", err) + } + if err := writeBytesContext(context.Background(), file, []byte("data")); err == nil { + t.Fatal("writeBytesContext closed file returned nil error") + } + if err := writeBytesContext(&stepCancelContext{Context: context.Background(), cancelAt: 1}, file, []byte("data")); !errors.Is(err, context.Canceled) { + t.Fatalf("writeBytesContext cancellation error = %v", err) + } +} + +// TestLocalMidOperationCancellation verifies long-running filesystem methods +// observe cancellation after their initial validation check. +func TestLocalMidOperationCancellation(t *testing.T) { + root := t.TempDir() + d := &driver{root: root} + if err := d.Put("a", []byte("a")); err != nil { + t.Fatalf("Put a: %v", err) + } + if err := d.Put("b", []byte("b")); err != nil { + t.Fatalf("Put b: %v", err) + } + + if _, err := d.GetContext(&stepCancelContext{Context: context.Background(), cancelAt: 3}, "a"); !errors.Is(err, context.Canceled) { + t.Fatalf("GetContext mid-read cancellation = %v", err) + } + if _, err := d.ListContext(&stepCancelContext{Context: context.Background(), cancelAt: 2}, ""); !errors.Is(err, context.Canceled) { + t.Fatalf("ListContext mid-list cancellation = %v", err) + } + if err := d.WalkContext(&stepCancelContext{Context: context.Background(), cancelAt: 3}, "", func(storagecore.Entry) error { return nil }); !errors.Is(err, context.Canceled) { + t.Fatalf("WalkContext mid-walk cancellation = %v", err) + } + if err := d.MoveContext(&stepCancelContext{Context: context.Background(), cancelAt: 2}, "a", "nested/a"); !errors.Is(err, context.Canceled) { + t.Fatalf("MoveContext pre-rename cancellation = %v", err) + } +} + +type stepCancelContext struct { + context.Context + calls int + cancelAt int +} + +// Err begins returning context.Canceled at the configured observation count. +func (c *stepCancelContext) Err() error { + c.calls++ + if c.calls >= c.cancelAt { + return context.Canceled + } + return nil +} + +type localErrorReader struct { + err error +} + +// Read returns the configured failure without producing data. +func (r *localErrorReader) Read([]byte) (int, error) { + if r.err == nil { + return 0, io.EOF + } + return 0, r.err +} + +type localDataReader struct { + done bool +} + +// Read produces one byte before ending the stream. +func (r *localDataReader) Read(p []byte) (int, error) { + if r.done { + return 0, io.EOF + } + r.done = true + p[0] = 'x' + return 1, nil +} + +type localFailingWriter struct { + err error +} + +// Write injects the configured destination failure. +func (w *localFailingWriter) Write([]byte) (int, error) { return 0, w.err } + +type localShortWriter struct{} + +// Write accepts no bytes without an error to exercise short-write handling. +func (*localShortWriter) Write([]byte) (int, error) { return 0, nil } diff --git a/driver/memorystorage/memory_internal_test.go b/driver/memorystorage/memory_internal_test.go index 532e1e5..1441c0d 100644 --- a/driver/memorystorage/memory_internal_test.go +++ b/driver/memorystorage/memory_internal_test.go @@ -3,12 +3,93 @@ package memorystorage import ( "context" "errors" + "reflect" "sync" "testing" "github.com/goforj/storage/storagecore" ) +// TestMemoryRemainingBranches exercises validation, pagination, directory, and +// helper branches that are deliberately outside the shared storage contract. +func TestMemoryRemainingBranches(t *testing.T) { + d := &driver{prefix: "tenant", dirs: make(map[string]struct{}), objects: make(map[string]object)} + + if _, err := newFromDiskConfig(nil, storagecore.ResolvedConfig{Prefix: "../bad"}); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("newFromDiskConfig invalid prefix error = %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := newFromDiskConfig(ctx, storagecore.ResolvedConfig{}); !errors.Is(err, context.Canceled) { + t.Fatalf("newFromDiskConfig canceled error = %v", err) + } + + for name, call := range map[string]func() error{ + "get": func() error { _, err := d.GetContext(nil, "../bad"); return err }, + "make directory": func() error { return d.MakeDirContext(nil, "../bad") }, + "delete": func() error { return d.DeleteContext(nil, "../bad") }, + "stat": func() error { _, err := d.StatContext(nil, "../bad"); return err }, + "exists": func() error { _, err := d.ExistsContext(nil, "../bad"); return err }, + "list": func() error { _, err := d.ListContext(nil, "../bad"); return err }, + "list page": func() error { _, err := d.ListPageContext(nil, "../bad", 0, 1); return err }, + "walk": func() error { return d.WalkContext(nil, "../bad", func(storagecore.Entry) error { return nil }) }, + "copy source": func() error { return d.CopyContext(nil, "../bad", "dst") }, + "copy destination": func() error { return d.CopyContext(nil, "src", "../bad") }, + "move source": func() error { return d.MoveContext(nil, "../bad", "dst") }, + "move destination": func() error { return d.MoveContext(nil, "src", "../bad") }, + "mod time": func() error { _, err := d.ModTime(nil, "../bad"); return err }, + } { + t.Run(name, func(t *testing.T) { + if err := call(); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("error = %v", err) + } + }) + } + if err := d.WalkContext(nil, "", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("WalkContext nil callback error = %v", err) + } + if err := d.MakeDir("nested/empty"); err != nil { + t.Fatalf("MakeDir: %v", err) + } + if err := d.Delete("nested"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Delete non-empty directory error = %v", err) + } + if err := d.Delete("nested/empty"); err != nil { + t.Fatalf("Delete empty directory: %v", err) + } + if err := d.Put("file", []byte("x")); err != nil { + t.Fatalf("Put file: %v", err) + } + if page, err := d.ListPage("", -1, 0); err != nil || page.Offset != 0 || page.Limit != 100 { + t.Fatalf("ListPage defaults = %+v, %v", page, err) + } + if page, err := d.ListPage("", 99, 1); err != nil || len(page.Entries) != 0 { + t.Fatalf("ListPage bounded offset = %+v, %v", page, err) + } + if _, err := d.ListPage("file", 0, 1); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("ListPage object error = %v", err) + } + if err := d.Walk("file", func(entry storagecore.Entry) error { + if entry.Path != "file" { + t.Fatalf("Walk file entry = %+v", entry) + } + return nil + }); err != nil { + t.Fatalf("Walk file: %v", err) + } + + dirs := recursiveParentDirs("a/b/c") + if !reflect.DeepEqual(dirs, []string{"a", "a/b"}) { + t.Fatalf("recursiveParentDirs = %v", dirs) + } + if recursiveParentDirs("") != nil || recursiveParentDirs("one") != nil { + t.Fatal("recursiveParentDirs should omit paths without parents") + } + if d.stripPrefix("tenant") != "" || d.stripPrefix("tenant/file") != "file" { + t.Fatalf("stripPrefix returned unexpected values") + } +} + // TestPutContextRechecksCancellationAfterLockWait proves cancellation wins // when it arrives after validation but before a contended mutation can run. func TestPutContextRechecksCancellationAfterLockWait(t *testing.T) { @@ -61,6 +142,134 @@ func TestSamePathCopyPreservesObject(t *testing.T) { } } +// TestMemoryCollisionAndMissingBranches verifies each path-shape conflict and +// absence distinction exposed by the in-memory implementation. +func TestMemoryCollisionAndMissingBranches(t *testing.T) { + d := &driver{dirs: make(map[string]struct{}), objects: make(map[string]object)} + if err := d.MakeDir(""); err != nil { + t.Fatalf("MakeDir root: %v", err) + } + if err := d.Put("parent", []byte("file")); err != nil { + t.Fatalf("Put parent: %v", err) + } + if err := d.Put("parent/child", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Put beneath object error = %v", err) + } + if err := d.MakeDir("parent/child"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("MakeDir beneath object error = %v", err) + } + if err := d.MakeDir("directory"); err != nil { + t.Fatalf("MakeDir directory: %v", err) + } + if err := d.Put("directory", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Put over directory error = %v", err) + } + if err := d.Put("directory/child", nil); err != nil { + t.Fatalf("Put child: %v", err) + } + if err := d.Put("directory", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Put over implied children error = %v", err) + } + if _, err := d.Get("missing"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Get missing error = %v", err) + } + if _, err := d.Stat("missing"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Stat missing error = %v", err) + } + if _, err := d.List("missing"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("List missing error = %v", err) + } + if _, err := d.ListPage("missing", 0, 1); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("ListPage missing error = %v", err) + } + if err := d.Copy("missing", "copy"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Copy missing error = %v", err) + } + if err := d.Copy("parent", "directory"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Copy onto directory error = %v", err) + } + if err := d.Move("parent", "directory"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Move onto directory error = %v", err) + } + if _, err := d.ModTime(context.Background(), "missing"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("ModTime missing error = %v", err) + } + canceled, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := d.ModTime(canceled, "parent"); !errors.Is(err, context.Canceled) { + t.Fatalf("ModTime canceled error = %v", err) + } + if _, err := d.URL("missing"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("URL missing error = %v", err) + } + if _, err := d.URL("parent"); !errors.Is(err, storagecore.ErrUnsupported) { + t.Fatalf("URL existing error = %v", err) + } + + d.objects["implicit/file"] = object{data: []byte("x")} + entry, err := d.Stat("implicit") + if err != nil || !entry.IsDir { + t.Fatalf("Stat implicit directory = %+v, %v", entry, err) + } + if !d.hasChildrenLocked("implicit") { + t.Fatal("hasChildrenLocked did not find object descendant") + } + d.dirs["self"] = struct{}{} + if entries := d.listEntriesLocked("self"); len(entries) != 0 { + t.Fatalf("listEntriesLocked self = %+v", entries) + } + + walkCtx, walkCancel := context.WithCancel(context.Background()) + seen := 0 + err = d.WalkContext(walkCtx, "", func(storagecore.Entry) error { + seen++ + walkCancel() + return nil + }) + if !errors.Is(err, context.Canceled) || seen != 1 { + t.Fatalf("WalkContext cancellation = %v after %d entries", err, seen) + } +} + +// TestMemoryPostLockCancellation verifies every locked operation rechecks the +// caller context after waiting for concurrent storage work. +func TestMemoryPostLockCancellation(t *testing.T) { + tests := []struct { + name string + call func(*driver, context.Context) error + }{ + {name: "get", call: func(d *driver, ctx context.Context) error { _, err := d.GetContext(ctx, "file"); return err }}, + {name: "put", call: func(d *driver, ctx context.Context) error { return d.PutContext(ctx, "file", nil) }}, + {name: "mkdir", call: func(d *driver, ctx context.Context) error { return d.MakeDirContext(ctx, "dir") }}, + {name: "delete", call: func(d *driver, ctx context.Context) error { return d.DeleteContext(ctx, "file") }}, + {name: "stat", call: func(d *driver, ctx context.Context) error { _, err := d.StatContext(ctx, "file"); return err }}, + {name: "exists", call: func(d *driver, ctx context.Context) error { _, err := d.ExistsContext(ctx, "file"); return err }}, + {name: "list", call: func(d *driver, ctx context.Context) error { _, err := d.ListContext(ctx, ""); return err }}, + {name: "list page", call: func(d *driver, ctx context.Context) error { _, err := d.ListPageContext(ctx, "", 0, 1); return err }}, + {name: "walk", call: func(d *driver, ctx context.Context) error { + return d.WalkContext(ctx, "", func(storagecore.Entry) error { return nil }) + }}, + {name: "copy", call: func(d *driver, ctx context.Context) error { return d.CopyContext(ctx, "file", "copy") }}, + {name: "mod time", call: func(d *driver, ctx context.Context) error { _, err := d.ModTime(ctx, "file"); return err }}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + d := &driver{dirs: make(map[string]struct{}), objects: map[string]object{"file": {}}} + base, cancel := context.WithCancel(context.Background()) + ctx := &observedContext{Context: base, checked: make(chan struct{})} + d.mu.Lock() + result := make(chan error, 1) + go func() { result <- test.call(d, ctx) }() + <-ctx.checked + cancel() + d.mu.Unlock() + if err := <-result; !errors.Is(err, context.Canceled) { + t.Fatalf("error = %v", err) + } + }) + } +} + type observedContext struct { context.Context once sync.Once diff --git a/driver/rclonestorage/rclone_internal_test.go b/driver/rclonestorage/rclone_internal_test.go index 5e52645..266007c 100644 --- a/driver/rclonestorage/rclone_internal_test.go +++ b/driver/rclonestorage/rclone_internal_test.go @@ -765,6 +765,24 @@ func TestRcloneCloseOwnsBackendLifecycle(t *testing.T) { if _, err := d.Get("file.txt"); !errors.Is(err, stdfs.ErrClosed) { t.Fatalf("Get after Close error = %v", err) } + closedCalls := []func() error{ + func() error { return d.Put("file.txt", nil) }, + func() error { return d.MakeDir("dir") }, + func() error { return d.Delete("file.txt") }, + func() error { _, err := d.Stat("file.txt"); return err }, + func() error { _, err := d.Exists("file.txt"); return err }, + func() error { _, err := d.List(""); return err }, + func() error { return d.Walk("", func(storagecore.Entry) error { return nil }) }, + func() error { return d.Copy("file.txt", "copy.txt") }, + func() error { return d.Move("file.txt", "moved.txt") }, + func() error { _, err := d.URL("file.txt"); return err }, + func() error { _, err := d.ModTime(context.Background(), "file.txt"); return err }, + } + for index, call := range closedCalls { + if err := call(); !errors.Is(err, stdfs.ErrClosed) { + t.Fatalf("closed call %d error = %v", index, err) + } + } } // resetRcloneInit restores all process-global rclone hooks and identity state after each test. @@ -1026,3 +1044,111 @@ func TestInitRcloneConfigDataSetsConfigPath(t *testing.T) { t.Fatalf("expected config path to end with inline-rclone.conf, got %q", config.GetConfigPath()) } } + +// TestRcloneInvalidPathsAndThinWrappers covers validation before any backend +// request and the background-context adapters omitted by the shared contract. +func TestRcloneInvalidPathsAndThinWrappers(t *testing.T) { + d := &driver{} + calls := []func() error{ + func() error { _, err := d.GetContext(nil, "../bad"); return err }, + func() error { return d.PutContext(nil, "../bad", nil) }, + func() error { return d.MakeDirContext(nil, "../bad") }, + func() error { return d.DeleteContext(nil, "../bad") }, + func() error { _, err := d.StatContext(nil, "../bad"); return err }, + func() error { _, err := d.ExistsContext(nil, "../bad"); return err }, + func() error { _, err := d.ListContext(nil, "../bad"); return err }, + func() error { return d.WalkContext(nil, "../bad", func(storagecore.Entry) error { return nil }) }, + func() error { return d.CopyContext(nil, "../bad", "dst") }, + func() error { return d.CopyContext(nil, "src", "../bad") }, + func() error { return d.MoveContext(nil, "../bad", "dst") }, + func() error { return d.MoveContext(nil, "src", "../bad") }, + func() error { _, err := d.URLContext(nil, "../bad"); return err }, + } + for index, call := range calls { + if err := call(); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("invalid-path call %d error = %v", index, err) + } + } + if err := d.MakeDir("../bad"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("MakeDir invalid path error = %v", err) + } + if _, err := d.ListPage("../bad", 0, 1); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("ListPage error = %v", err) + } +} + +// TestRcloneRemainingHelperEdges covers cancellation, cleanup, stream, and +// backend capability branches without external remotes. +func TestRcloneRemainingHelperEdges(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := newFromDiskConfig(ctx, storagecore.ResolvedConfig{Remote: "remote:"}); !errors.Is(err, context.Canceled) { + t.Fatalf("newFromDiskConfig canceled error = %v", err) + } + + fake := newFakeFs() + d := &driver{fs: fake, prefix: "pre"} + if err := d.MakeDir(""); err != nil { + t.Fatalf("MakeDir root: %v", err) + } + mkdirErr := errors.New("mkdir") + fake.mkdirFunc = func(context.Context, string) error { return mkdirErr } + if err := d.MakeDir("dir"); !errors.Is(err, mkdirErr) { + t.Fatalf("MakeDir error = %v", err) + } + if err := d.Walk("", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Walk nil callback error = %v", err) + } + + closeErr := errors.New("close") + fake.newObjectFunc = func(context.Context, string) (fs.Object, error) { + return &fakeObject{fakeDirEntry: fakeDirEntry{remote: "pre/file", fsys: fake}, openRC: &rcloneCloseReader{closeErr: closeErr}}, nil + } + if _, err := d.Get("file"); !errors.Is(err, closeErr) { + t.Fatalf("Get close error = %v", err) + } + + writeErr := errors.New("write") + if err := copyContext(context.Background(), rcloneFailingWriter{err: writeErr}, strings.NewReader("x")); !errors.Is(err, writeErr) { + t.Fatalf("copyContext write error = %v", err) + } + if err := copyContext(context.Background(), rcloneShortWriter{}, strings.NewReader("x")); !errors.Is(err, io.ErrShortWrite) { + t.Fatalf("copyContext short write error = %v", err) + } + if err := copyContext(ctx, io.Discard, strings.NewReader("x")); !errors.Is(err, context.Canceled) { + t.Fatalf("copyContext canceled error = %v", err) + } + if err := joinCleanup(nil, closeErr); !errors.Is(err, closeErr) { + t.Fatalf("joinCleanup cleanup error = %v", err) + } + if err := joinCleanup(writeErr, closeErr); !errors.Is(err, writeErr) || !errors.Is(err, closeErr) { + t.Fatalf("joinCleanup combined error = %v", err) + } + + withoutShutdown := &driver{fs: newFakeFs()} + if err := withoutShutdown.Close(); err != nil { + t.Fatalf("Close without shutdown: %v", err) + } +} + +type rcloneCloseReader struct { + closeErr error +} + +// Read reports EOF immediately. +func (*rcloneCloseReader) Read([]byte) (int, error) { return 0, io.EOF } + +// Close returns the configured cleanup failure. +func (r *rcloneCloseReader) Close() error { return r.closeErr } + +type rcloneFailingWriter struct { + err error +} + +// Write injects a destination failure. +func (w rcloneFailingWriter) Write([]byte) (int, error) { return 0, w.err } + +type rcloneShortWriter struct{} + +// Write accepts no bytes without error to exercise short-write handling. +func (rcloneShortWriter) Write([]byte) (int, error) { return 0, nil } diff --git a/driver/redisstorage/redis_miniredis_test.go b/driver/redisstorage/redis_miniredis_test.go index 8d8a244..92cbed8 100644 --- a/driver/redisstorage/redis_miniredis_test.go +++ b/driver/redisstorage/redis_miniredis_test.go @@ -17,6 +17,358 @@ import ( "github.com/goforj/storage/storagecore" ) +// TestRedisPublicOperations exercises the complete storage surface against an +// in-process Redis server so default tests cover behavior formerly limited to +// integration-tagged container tests. +func TestRedisPublicOperations(t *testing.T) { + d := newMiniRedisDriver(t, "sandbox") + + if err := d.MakeDir("empty"); err != nil { + t.Fatalf("MakeDir: %v", err) + } + if err := d.Put("docs/readme.txt", []byte("hello")); err != nil { + t.Fatalf("Put: %v", err) + } + got, err := d.Get("docs/readme.txt") + if err != nil || string(got) != "hello" { + t.Fatalf("Get = %q, %v", got, err) + } + if _, err := d.Get(""); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Get root error = %v", err) + } + + entry, err := d.Stat("docs/readme.txt") + if err != nil || entry.IsDir || entry.Size != 5 { + t.Fatalf("Stat file = %+v, %v", entry, err) + } + entry, err = d.Stat("docs") + if err != nil || !entry.IsDir { + t.Fatalf("Stat directory = %+v, %v", entry, err) + } + entry, err = d.Stat("") + if err != nil || !entry.IsDir || entry.Path != "" { + t.Fatalf("Stat root = %+v, %v", entry, err) + } + if _, err := d.Stat("missing"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Stat missing error = %v", err) + } + if exists, err := d.Exists("docs/readme.txt"); err != nil || !exists { + t.Fatalf("Exists file = %v, %v", exists, err) + } + if exists, err := d.Exists(""); err != nil || exists { + t.Fatalf("Exists root = %v, %v", exists, err) + } + + entries, err := d.List("") + if err != nil || len(entries) != 2 { + t.Fatalf("List root = %+v, %v", entries, err) + } + page, err := d.ListPage("", 0, 1) + if err != nil || len(page.Entries) != 1 || !page.HasMore { + t.Fatalf("ListPage = %+v, %v", page, err) + } + if _, err := d.List("docs/readme.txt"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("List file error = %v", err) + } + if _, err := d.List("missing"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("List missing error = %v", err) + } + + var walked []string + if err := d.Walk("docs", func(entry storagecore.Entry) error { + walked = append(walked, entry.Path) + return nil + }); err != nil || !slices.Equal(walked, []string{"docs/readme.txt"}) { + t.Fatalf("Walk = %v, %v", walked, err) + } + callbackErr := errors.New("stop") + if err := d.Walk("", func(storagecore.Entry) error { return callbackErr }); !errors.Is(err, callbackErr) { + t.Fatalf("Walk callback error = %v", err) + } + if err := d.Walk("", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Walk nil callback error = %v", err) + } + if err := d.Walk("missing", func(storagecore.Entry) error { return nil }); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Walk missing error = %v", err) + } + + if err := d.Copy("docs/readme.txt", "copies/readme.txt"); err != nil { + t.Fatalf("Copy: %v", err) + } + if err := d.Copy("missing", "copies/missing"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Copy missing error = %v", err) + } + if err := d.Move("copies/readme.txt", "archive/readme.txt"); err != nil { + t.Fatalf("Move file: %v", err) + } + if err := d.Move("docs", "manual"); err != nil { + t.Fatalf("Move directory: %v", err) + } + if _, err := d.URL("manual/readme.txt"); !errors.Is(err, storagecore.ErrUnsupported) { + t.Fatalf("URL error = %v", err) + } + if modTime, err := d.ModTime(context.Background(), "manual/readme.txt"); err != nil || modTime.IsZero() { + t.Fatalf("ModTime = %v, %v", modTime, err) + } + if _, err := d.ModTime(context.Background(), "missing"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("ModTime missing error = %v", err) + } + + if err := d.Delete("manual/readme.txt"); err != nil { + t.Fatalf("Delete file: %v", err) + } + if err := d.Delete("manual"); err != nil { + t.Fatalf("Delete directory: %v", err) + } + if err := d.Delete("missing"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Delete missing error = %v", err) + } +} + +// TestRedisBackendFailures verifies transport failures remain observable from +// every operation instead of being mistaken for absence or empty results. +func TestRedisBackendFailures(t *testing.T) { + server := miniredis.RunT(t) + d := newMiniRedisDriverForServer(t, server, "sandbox") + server.Close() + + calls := []func() error{ + func() error { _, err := d.Get("file"); return err }, + func() error { return d.Put("file", []byte("data")) }, + func() error { return d.MakeDir("dir") }, + func() error { return d.Delete("file") }, + func() error { _, err := d.Stat("file"); return err }, + func() error { _, err := d.Exists("file"); return err }, + func() error { _, err := d.List(""); return err }, + func() error { _, err := d.ListPage("", 0, 1); return err }, + func() error { return d.Walk("", func(storagecore.Entry) error { return nil }) }, + func() error { return d.Copy("file", "copy") }, + func() error { return d.Move("file", "moved") }, + func() error { _, err := d.URL("file"); return err }, + func() error { _, err := d.ModTime(context.Background(), "file"); return err }, + } + for index, call := range calls { + if err := call(); err == nil { + t.Fatalf("backend failure call %d returned nil error", index) + } + } +} + +// TestRedisMalformedRecords covers defensive parsing of persisted metadata so +// corrupted Redis values produce errors rather than fabricated entries. +func TestRedisMalformedRecords(t *testing.T) { + d := newMiniRedisDriver(t, "") + ctx := context.Background() + + if err := d.client.HSet(ctx, d.objectKey("bad-time"), "data", "ok", "modtime", "not-a-number").Err(); err != nil { + t.Fatalf("seed bad modtime: %v", err) + } + if _, err := d.ModTime(ctx, "bad-time"); err == nil { + t.Fatal("ModTime malformed value returned nil error") + } + if _, err := d.ModTime(ctx, ""); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("ModTime root error = %v", err) + } +} + +// TestRedisInternalFailureEdges verifies corrupted Redis key types and +// adversarial recursion state fail closed throughout index validation. +func TestRedisInternalFailureEdges(t *testing.T) { + d := newMiniRedisDriver(t, "") + ctx := context.Background() + wrongType := func(key string) { + t.Helper() + if err := d.client.Set(ctx, key, "not-a-set-or-hash", 0).Err(); err != nil { + t.Fatalf("seed wrong type %q: %v", key, err) + } + } + + wrongType(d.objectKey("bad-object")) + if _, err := d.objectSize(ctx, "bad-object"); err == nil { + t.Fatal("objectSize wrong type returned nil error") + } + wrongType(d.dirChildrenKey("bad-children")) + if _, err := d.children(ctx, "bad-children"); err == nil { + t.Fatal("children wrong type returned nil error") + } + wrongType(d.dirObjectsKey("bad-descendants")) + if _, err := d.descendants(ctx, "bad-descendants"); err == nil { + t.Fatal("descendants wrong type returned nil error") + } + wrongType(d.dirObjectsKey("bad-legacy-marker")) + if _, err := d.legacyDirectoryMarkerValid(ctx, "bad-legacy-marker"); err == nil { + t.Fatal("legacy marker wrong type returned nil error") + } + wrongType(d.versionedDirObjectsKey("bad-versioned-marker")) + if _, err := d.versionedDirectoryMarkerValid(ctx, "bad-versioned-marker"); err == nil { + t.Fatal("versioned marker wrong type returned nil error") + } + + if _, err := d.listEntries(ctx, []string{encodeFileChild("bad-object")}); err == nil { + t.Fatal("listEntries wrong object type returned nil error") + } + if _, _, err := d.walkEntries(ctx, []indexedMemberRecord{{member: objectMember("bad-object"), schema: versionedIndex}}, ""); err == nil { + t.Fatal("walkEntries wrong object type returned nil error") + } + invalidRecord := indexedMemberRecord{member: "storage:v1:object:%", schema: versionedIndex} + if _, err := d.resolveIndexedMember(ctx, invalidRecord); err == nil { + t.Fatal("resolveIndexedMember invalid value returned nil error") + } + if _, _, err := d.directoryStateFromRecords(ctx, d.client, "", []indexedMemberRecord{invalidRecord}); err == nil { + t.Fatal("directoryStateFromRecords invalid value returned nil error") + } + + if exists, err := d.dirExistsSeen(ctx, "cycle", map[string]struct{}{"cycle": {}}); err != nil || exists { + t.Fatalf("dirExistsSeen cycle = %v, %v", exists, err) + } + deep := make(map[string]struct{}, 1024) + for index := 0; index < 1024; index++ { + deep[fmt.Sprintf("dir-%d", index)] = struct{}{} + } + if exists, err := d.dirExistsSeen(ctx, "overflow", deep); err != nil || exists { + t.Fatalf("dirExistsSeen depth = %v, %v", exists, err) + } + if _, _, err := d.directoryStateInTransactionSeen(ctx, nil, "cycle", map[string]struct{}{"cycle": {}}); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("transaction cycle error = %v", err) + } + if _, _, err := d.directoryStateInTransactionSeen(ctx, nil, "overflow", deep); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("transaction depth error = %v", err) + } +} + +// TestRedisTransactionalIndexHelpers exercises stable watched snapshots and +// stale-member filtering against the same Redis transaction used by deletion. +func TestRedisTransactionalIndexHelpers(t *testing.T) { + d := newMiniRedisDriver(t, "") + ctx := context.Background() + if err := d.MakeDir("empty"); err != nil { + t.Fatalf("MakeDir empty: %v", err) + } + if err := d.Put("parent/file", []byte("data")); err != nil { + t.Fatalf("Put nested file: %v", err) + } + + err := d.client.Watch(ctx, func(tx *redis.Tx) error { + children, err := d.watchedChildMembers(ctx, tx, "") + if err != nil || len(children) != 2 { + t.Fatalf("watchedChildMembers = %v, %v", children, err) + } + records, err := d.watchedDescendants(ctx, tx, "") + if err != nil || len(records) != 2 { + t.Fatalf("watchedDescendants = %v, %v", records, err) + } + exists, contents, err := d.directoryRecordsStateInTransaction(ctx, tx, "parent") + if err != nil || !exists || !contents { + t.Fatalf("directoryRecordsStateInTransaction parent = %v, %v, %v", exists, contents, err) + } + exists, contents, err = d.directoryStateInTransaction(ctx, tx, "empty") + if err != nil || !exists || contents { + t.Fatalf("directoryStateInTransaction empty = %v, %v, %v", exists, contents, err) + } + return nil + }, d.dirChildrenKey(""), d.versionedDirChildrenKey("")) + if err != nil { + t.Fatalf("Watch: %v", err) + } + + entries, err := d.listEntries(ctx, []string{encodeDirChild("ghost"), encodeFileChild("missing")}) + if err != nil || len(entries) != 0 { + t.Fatalf("listEntries stale members = %+v, %v", entries, err) + } + records := []indexedMemberRecord{ + {member: objectMember("outside/file"), schema: versionedIndex}, + {member: objectMember("parent/missing"), schema: versionedIndex}, + {member: dirMarkerMember("parent"), schema: versionedIndex}, + } + entries, found, err := d.walkEntries(ctx, records, "parent") + if err != nil || !found || len(entries) != 0 { + t.Fatalf("walkEntries filtered records = %+v, %v, %v", entries, found, err) + } + + seedLegacyObject(t, d, "duplicate", "data") + if err := d.client.SAdd(ctx, d.versionedDirObjectsKey(""), "duplicate").Err(); err != nil { + t.Fatalf("seed duplicate schema member: %v", err) + } + if records, err := d.descendants(ctx, ""); err != nil || len(records) < 2 { + t.Fatalf("descendants duplicate records = %v, %v", records, err) + } + + primary := errors.New("primary") + cleanup := errors.New("cleanup") + if err := joinCleanup(primary, nil); !errors.Is(err, primary) { + t.Fatalf("joinCleanup primary = %v", err) + } + if err := joinCleanup(nil, cleanup); !errors.Is(err, cleanup) { + t.Fatalf("joinCleanup cleanup = %v", err) + } + if err := joinCleanup(primary, cleanup); !errors.Is(err, primary) || !errors.Is(err, cleanup) { + t.Fatalf("joinCleanup combined = %v", err) + } + canceled, cancel := context.WithCancel(ctx) + cancel() + if err := d.watchTransaction(canceled, nil, func(*redis.Tx) error { return nil }); !errors.Is(err, context.Canceled) { + t.Fatalf("watchTransaction cancellation = %v", err) + } +} + +// TestRedisPublicCorruptTypes verifies every public operation propagates Redis +// WRONGTYPE errors from the particular structure it owns. +func TestRedisPublicCorruptTypes(t *testing.T) { + tests := []struct { + name string + seed func(context.Context, *driver) error + call func(*driver) error + }{ + {name: "get", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.objectKey("file"), "bad", 0).Err() + }, call: func(d *driver) error { _, err := d.Get("file"); return err }}, + {name: "put", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.objectKey("file"), "bad", 0).Err() + }, call: func(d *driver) error { return d.Put("file", nil) }}, + {name: "mkdir", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.objectKey("dir"), "bad", 0).Err() + }, call: func(d *driver) error { return d.MakeDir("dir") }}, + {name: "delete", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.objectKey("file"), "bad", 0).Err() + }, call: func(d *driver) error { return d.Delete("file") }}, + {name: "stat", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.objectKey("file"), "bad", 0).Err() + }, call: func(d *driver) error { _, err := d.Stat("file"); return err }}, + {name: "exists", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.objectKey("file"), "bad", 0).Err() + }, call: func(d *driver) error { _, err := d.Exists("file"); return err }}, + {name: "list", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.dirChildrenKey(""), "bad", 0).Err() + }, call: func(d *driver) error { _, err := d.List(""); return err }}, + {name: "walk", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.dirObjectsKey(""), "bad", 0).Err() + }, call: func(d *driver) error { return d.Walk("", func(storagecore.Entry) error { return nil }) }}, + {name: "copy", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.objectKey("source"), "bad", 0).Err() + }, call: func(d *driver) error { return d.Copy("source", "copy") }}, + {name: "move", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.objectKey("source"), "bad", 0).Err() + }, call: func(d *driver) error { return d.Move("source", "moved") }}, + {name: "url", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.objectKey("file"), "bad", 0).Err() + }, call: func(d *driver) error { _, err := d.URL("file"); return err }}, + {name: "modtime", seed: func(ctx context.Context, d *driver) error { + return d.client.Set(ctx, d.objectKey("file"), "bad", 0).Err() + }, call: func(d *driver) error { _, err := d.ModTime(context.Background(), "file"); return err }}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + d := newMiniRedisDriver(t, "") + if err := test.seed(context.Background(), d); err != nil { + t.Fatalf("seed: %v", err) + } + if err := test.call(d); err == nil { + t.Fatal("operation returned nil error") + } + }) + } +} + // TestRedisIndexedMembersDoNotCollide verifies that directory metadata cannot // hide a valid object whose name matches the legacy marker namespace. func TestRedisIndexedMembersDoNotCollide(t *testing.T) { diff --git a/driver/redisstorage/redis_test.go b/driver/redisstorage/redis_test.go index f46fd2d..f332f05 100644 --- a/driver/redisstorage/redis_test.go +++ b/driver/redisstorage/redis_test.go @@ -3,6 +3,7 @@ package redisstorage import ( "context" "errors" + "slices" "testing" "github.com/goforj/storage/storagecore" @@ -127,3 +128,95 @@ func TestRedisNamespace(t *testing.T) { t.Fatalf("redisNamespace db = %q", got) } } + +// TestRedisInvalidPathsAndThinWrappers covers validation before any Redis +// request and the background-context adapters omitted by the shared contract. +func TestRedisInvalidPathsAndThinWrappers(t *testing.T) { + d := &driver{} + calls := []func() error{ + func() error { _, err := d.GetContext(nil, "../bad"); return err }, + func() error { return d.PutContext(nil, "../bad", nil) }, + func() error { return d.MakeDirContext(nil, "../bad") }, + func() error { return d.DeleteContext(nil, "../bad") }, + func() error { _, err := d.StatContext(nil, "../bad"); return err }, + func() error { _, err := d.ExistsContext(nil, "../bad"); return err }, + func() error { _, err := d.ListContext(nil, "../bad"); return err }, + func() error { _, err := d.ListPageContext(nil, "../bad", 0, 1); return err }, + func() error { return d.WalkContext(nil, "../bad", func(storagecore.Entry) error { return nil }) }, + func() error { return d.CopyContext(nil, "../bad", "dst") }, + func() error { return d.CopyContext(nil, "src", "../bad") }, + func() error { return d.MoveContext(nil, "../bad", "dst") }, + func() error { return d.MoveContext(nil, "src", "../bad") }, + func() error { _, err := d.URLContext(nil, "../bad"); return err }, + } + for index, call := range calls { + if err := call(); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("invalid-path call %d error = %v", index, err) + } + } + if err := d.MakeDir("../bad"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("MakeDir invalid path error = %v", err) + } + if _, err := d.ListPage("../bad", 0, 1); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("ListPage error = %v", err) + } +} + +// TestRedisIndexEncodingEdges verifies malformed persisted index values fail +// safely and every released and versioned encoding remains distinguishable. +func TestRedisIndexEncodingEdges(t *testing.T) { + d := &driver{namespace: "ns", prefix: "sandbox"} + if d.stripPrefix("sandbox") != "" || d.stripPrefix("sandbox/file") != "file" { + t.Fatal("stripPrefix returned unexpected values") + } + if got := (&driver{}).stripPrefix("plain"); got != "plain" { + t.Fatalf("unprefixed stripPrefix = %q", got) + } + if parentDir("") != "" || parentDir("file") != "" || parentDir("a/b") != "a" { + t.Fatal("parentDir returned unexpected values") + } + if objectDirs("") != nil || objectDirs("file") != nil || !slices.Equal(objectDirs("a/b/c"), []string{"a", "a/b"}) { + t.Fatalf("objectDirs returned unexpected values") + } + for _, child := range []string{encodeFileChild("a"), encodeDirChild("b")} { + if _, _, err := parseChildEntry(child); err != nil { + t.Fatalf("parseChildEntry(%q): %v", child, err) + } + } + if _, _, err := parseChildEntry("bad"); err == nil { + t.Fatal("parseChildEntry malformed returned nil error") + } + + for _, member := range []string{objectMember("a"), dirMarkerMember("b"), legacyDirMarkerMember("c"), "plain"} { + if _, _, err := parseIndexedMember(member); err != nil { + t.Fatalf("parseIndexedMember(%q): %v", member, err) + } + } + for _, member := range []string{"storage:v1:object:%", "storage:v1:directory:%"} { + if _, _, err := parseIndexedMember(member); err == nil { + t.Fatalf("parseIndexedMember(%q) returned nil error", member) + } + } + for _, key := range []string{"storage:v1:object:x", "storage:v1:directory:x", "dirmarker:x"} { + if _, ok := unambiguousLegacyObjectMember(key); ok { + t.Fatalf("unambiguousLegacyObjectMember(%q) = true", key) + } + } + if got, ok := unambiguousLegacyObjectMember("ordinary"); !ok || got != "ordinary" { + t.Fatalf("unambiguousLegacyObjectMember ordinary = %q, %v", got, ok) + } + + keys := d.indexedMemberWatchKeys([]indexedMemberRecord{ + {member: "legacy", schema: legacyIndex}, + {member: objectMember("object"), schema: versionedIndex}, + {member: dirMarkerMember("dir"), schema: versionedIndex}, + {member: legacyDirMarkerMember("old-dir"), schema: legacyIndex}, + {member: "storage:v1:object:%", schema: versionedIndex}, + }) + if len(keys) == 0 || !slices.IsSorted(keys) { + t.Fatalf("indexedMemberWatchKeys = %v", keys) + } + if _, err := d.listEntries(context.Background(), []string{"invalid"}); err == nil { + t.Fatal("listEntries malformed child returned nil error") + } +} diff --git a/driver/s3storage/s3_unit_test.go b/driver/s3storage/s3_unit_test.go index 0e47ffa..5a97405 100644 --- a/driver/s3storage/s3_unit_test.go +++ b/driver/s3storage/s3_unit_test.go @@ -38,11 +38,48 @@ type fakeS3 struct { listInputs []*s3.ListObjectsV2Input headByKey map[string]*s3.HeadObjectOutput headErrByKey map[string]error + getHook func() + listHook func() +} + +// TestS3InvalidPathsAndThinWrappers covers validation before any SDK request +// and the background-context adapters omitted by the shared contract. +func TestS3InvalidPathsAndThinWrappers(t *testing.T) { + d := &driver{} + calls := []func() error{ + func() error { _, err := d.GetContext(nil, "../bad"); return err }, + func() error { return d.PutContext(nil, "../bad", nil) }, + func() error { return d.MakeDirContext(nil, "../bad") }, + func() error { return d.DeleteContext(nil, "../bad") }, + func() error { _, err := d.StatContext(nil, "../bad"); return err }, + func() error { _, err := d.ExistsContext(nil, "../bad"); return err }, + func() error { _, err := d.ListContext(nil, "../bad"); return err }, + func() error { return d.WalkContext(nil, "../bad", func(storagecore.Entry) error { return nil }) }, + func() error { return d.CopyContext(nil, "../bad", "dst") }, + func() error { return d.CopyContext(nil, "src", "../bad") }, + func() error { return d.MoveContext(nil, "../bad", "dst") }, + func() error { return d.MoveContext(nil, "src", "../bad") }, + func() error { _, err := d.URLContext(nil, "../bad"); return err }, + } + for index, call := range calls { + if err := call(); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("invalid-path call %d error = %v", index, err) + } + } + if err := d.MakeDir("../bad"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("MakeDir invalid path error = %v", err) + } + if _, err := d.ListPage("../bad", 0, 1); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("ListPage error = %v", err) + } } // GetObject records read ownership and returns the configured body or failure for cleanup assertions. func (f *fakeS3) GetObject(ctx context.Context, in *s3.GetObjectInput, _ ...func(*s3.Options)) (*s3.GetObjectOutput, error) { f.getInputs = append(f.getInputs, in) + if f.getHook != nil { + f.getHook() + } if f.getErr != nil { return nil, f.getErr } @@ -86,6 +123,9 @@ func (f *fakeS3) HeadObject(ctx context.Context, in *s3.HeadObjectInput, _ ...fu // ListObjectsV2 consumes a configured page sequence so pagination and cancellation boundaries stay deterministic. func (f *fakeS3) ListObjectsV2(ctx context.Context, in *s3.ListObjectsV2Input, _ ...func(*s3.Options)) (*s3.ListObjectsV2Output, error) { f.listInputs = append(f.listInputs, in) + if f.listHook != nil { + f.listHook() + } if f.listErr != nil { return nil, f.listErr } @@ -816,3 +856,183 @@ func TestS3GetContextReadFailureStillCloses(t *testing.T) { t.Fatal("GetContext did not close failed response body") } } + +// TestS3ClientFailurePropagation verifies every SDK boundary preserves the +// operation that failed and does not continue into a later mutation. +func TestS3ClientFailurePropagation(t *testing.T) { + boom := errors.New("sdk failed") + + tests := []struct { + name string + driver *driver + call func(*driver) error + }{ + {name: "get", driver: &driver{client: &fakeS3{getErr: boom}}, call: func(d *driver) error { _, err := d.Get("file"); return err }}, + {name: "put", driver: &driver{client: &fakeS3{putErr: boom}}, call: func(d *driver) error { return d.Put("file", nil) }}, + {name: "make directory", driver: &driver{client: &fakeS3{putErr: boom}}, call: func(d *driver) error { return d.MakeDir("dir") }}, + {name: "delete object", driver: &driver{client: &fakeS3{headOK: true, delErr: boom}}, call: func(d *driver) error { return d.Delete("file") }}, + {name: "delete directory list", driver: &driver{client: &fakeS3{headByKey: map[string]*s3.HeadObjectOutput{"dir/": {}}, listErr: boom}}, call: func(d *driver) error { return d.Delete("dir") }}, + {name: "stat object", driver: &driver{client: &fakeS3{headErr: boom}}, call: func(d *driver) error { _, err := d.Stat("file"); return err }}, + {name: "stat directory marker", driver: &driver{client: &fakeS3{headErrByKey: map[string]error{"dir": &types.NotFound{}, "dir/": boom}}}, call: func(d *driver) error { _, err := d.Stat("dir"); return err }}, + {name: "stat implicit directory", driver: &driver{client: &fakeS3{listErr: boom}}, call: func(d *driver) error { _, err := d.Stat("dir"); return err }}, + {name: "exists", driver: &driver{client: &fakeS3{headErr: boom}}, call: func(d *driver) error { _, err := d.Exists("file"); return err }}, + {name: "list", driver: &driver{client: &fakeS3{listErr: boom}}, call: func(d *driver) error { _, err := d.List(""); return err }}, + {name: "list page", driver: &driver{client: &fakeS3{listErr: boom}}, call: func(d *driver) error { _, err := d.ListPage("", 0, 1); return err }}, + {name: "walk list", driver: &driver{client: &fakeS3{listErr: boom}}, call: func(d *driver) error { return d.Walk("", func(storagecore.Entry) error { return nil }) }}, + {name: "copy read", driver: &driver{client: &fakeS3{getErr: boom}}, call: func(d *driver) error { return d.Copy("src", "dst") }}, + {name: "copy write", driver: &driver{client: &fakeS3{getBody: "x", putErr: boom}}, call: func(d *driver) error { return d.Copy("src", "dst") }}, + {name: "move stat", driver: &driver{client: &fakeS3{headErr: boom}}, call: func(d *driver) error { return d.Move("src", "dst") }}, + {name: "move delete", driver: &driver{client: &fakeS3{headOK: true, getBody: "x", delErr: boom}}, call: func(d *driver) error { return d.Move("src", "dst") }}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + test.driver.bucket = "bucket" + if err := test.call(test.driver); !errors.Is(err, boom) { + t.Fatalf("error = %v", err) + } + }) + } + + d := &driver{client: &fakeS3{}, bucket: "bucket"} + if err := d.Walk("", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Walk nil callback error = %v", err) + } + if err := d.Walk("missing", func(storagecore.Entry) error { return nil }); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Walk missing error = %v", err) + } + implicit := &driver{client: &fakeS3{listOut: &s3.ListObjectsV2Output{Contents: []types.Object{{Key: aws.String("dir/file")}}}}, bucket: "bucket"} + entry, err := implicit.Stat("dir") + if err != nil || !entry.IsDir { + t.Fatalf("Stat implicit directory = %+v, %v", entry, err) + } +} + +// TestS3InternalEdgeBranches pins stream, cleanup, root-path, and collision +// behavior that cannot be reached through a successful object lifecycle. +func TestS3InternalEdgeBranches(t *testing.T) { + d := &driver{client: &fakeS3{}, presign: fakePresign{}, bucket: "bucket", prefix: "pre"} + for name, call := range map[string]func() error{ + "get root": func() error { _, err := d.Get(""); return err }, + "url root": func() error { _, err := d.URL(""); return err }, + } { + if err := call(); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("%s error = %v", name, err) + } + } + if exists, err := d.Exists(""); err != nil || exists { + t.Fatalf("Exists root = %v, %v", exists, err) + } + if got := d.stripPrefix("pre"); got != "" { + t.Fatalf("stripPrefix exact prefix = %q", got) + } + + entries := map[string]storagecore.Entry{"file": {Path: "file"}} + addDirectoryEntry(entries, "file") + if entries["file"].IsDir { + t.Fatal("addDirectoryEntry replaced a colliding file") + } + + canceled, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := copyContext(canceled, io.Discard, strings.NewReader("x")); !errors.Is(err, context.Canceled) { + t.Fatalf("copyContext canceled error = %v", err) + } + writeErr := errors.New("write failed") + if _, err := copyContext(context.Background(), failingWriter{err: writeErr}, strings.NewReader("x")); !errors.Is(err, writeErr) { + t.Fatalf("copyContext write error = %v", err) + } + if _, err := copyContext(context.Background(), shortWriter{}, strings.NewReader("xy")); !errors.Is(err, io.ErrShortWrite) { + t.Fatalf("copyContext short write error = %v", err) + } + cleanupErr := errors.New("cleanup") + if !errors.Is(joinCleanup(nil, cleanupErr), cleanupErr) { + t.Fatal("joinCleanup dropped cleanup-only error") + } + if got := joinCleanup(writeErr, nil); !errors.Is(got, writeErr) { + t.Fatal("joinCleanup dropped primary-only error") + } + if got := joinCleanup(writeErr, cleanupErr); !errors.Is(got, writeErr) || !errors.Is(got, cleanupErr) { + t.Fatal("joinCleanup did not preserve both errors") + } +} + +// TestS3PostSDKCancellation verifies cancellation takes precedence when it is +// observed immediately after an AWS operation completes. +func TestS3PostSDKCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + d := &driver{client: &fakeS3{getBody: "data", getHook: cancel}, bucket: "bucket"} + if _, err := d.GetContext(ctx, "file"); !errors.Is(err, context.Canceled) { + t.Fatalf("GetContext error = %v", err) + } +} + +// TestS3ListingCancellationAndFilters covers cancellation between paged +// entries and suppression of the logical namespace marker. +func TestS3ListingCancellationAndFilters(t *testing.T) { + t.Run("common prefix cancellation", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + client := &fakeS3{listHook: cancel, listOut: &s3.ListObjectsV2Output{ + CommonPrefixes: []types.CommonPrefix{{Prefix: aws.String("pre/dir/")}}, + }} + d := &driver{client: client, bucket: "bucket", prefix: "pre"} + if _, err := d.ListContext(ctx, ""); !errors.Is(err, context.Canceled) { + t.Fatalf("ListContext prefix cancellation = %v", err) + } + }) + t.Run("object cancellation", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + client := &fakeS3{listHook: cancel, listOut: &s3.ListObjectsV2Output{ + Contents: []types.Object{{Key: aws.String("pre/file")}}, + }} + d := &driver{client: client, bucket: "bucket", prefix: "pre"} + if _, err := d.ListContext(ctx, ""); !errors.Is(err, context.Canceled) { + t.Fatalf("ListContext object cancellation = %v", err) + } + }) + t.Run("logical marker filtering", func(t *testing.T) { + client := &fakeS3{listOut: &s3.ListObjectsV2Output{ + CommonPrefixes: []types.CommonPrefix{{Prefix: aws.String("pre/")}}, + Contents: []types.Object{{Key: aws.String("pre")}}, + }} + d := &driver{client: client, bucket: "bucket", prefix: "pre"} + entries, err := d.List("") + if err != nil || len(entries) != 0 { + t.Fatalf("List marker entries = %+v, %v", entries, err) + } + }) + t.Run("walk cancellation and callback cancellation", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + client := &fakeS3{listHook: cancel, listOut: &s3.ListObjectsV2Output{ + Contents: []types.Object{{Key: aws.String("file")}}, + }} + d := &driver{client: client, bucket: "bucket"} + if err := d.WalkContext(ctx, "", func(storagecore.Entry) error { return nil }); !errors.Is(err, context.Canceled) { + t.Fatalf("WalkContext listing cancellation = %v", err) + } + + client.listHook = nil + client.listOut = &s3.ListObjectsV2Output{Contents: []types.Object{{Key: aws.String("a")}, {Key: aws.String("b")}}} + ctx, cancel = context.WithCancel(context.Background()) + seen := 0 + err := d.WalkContext(ctx, "", func(storagecore.Entry) error { + seen++ + cancel() + return nil + }) + if !errors.Is(err, context.Canceled) || seen != 1 { + t.Fatalf("WalkContext callback cancellation = %v after %d entries", err, seen) + } + }) +} + +// failingWriter returns a stable failure from Write. +type failingWriter struct{ err error } + +// Write returns the configured failure without accepting bytes. +func (w failingWriter) Write([]byte) (int, error) { return 0, w.err } + +// shortWriter accepts fewer bytes than requested without an explicit error. +type shortWriter struct{} + +// Write reports a deliberately short successful write. +func (shortWriter) Write(p []byte) (int, error) { return len(p) - 1, nil } diff --git a/driver/sftpstorage/sftp_unit_test.go b/driver/sftpstorage/sftp_unit_test.go index 6eff51a..e660bd3 100644 --- a/driver/sftpstorage/sftp_unit_test.go +++ b/driver/sftpstorage/sftp_unit_test.go @@ -6,6 +6,7 @@ import ( "errors" "io" "os" + "path/filepath" "syscall" "testing" "time" @@ -242,10 +243,48 @@ type fakeSFTPClient struct { statErr error readDir []os.FileInfo readDirErr error + closeErr error + openHook func() + removeHook func() + statHook func() +} + +// TestSFTPInvalidPathsAndThinWrappers covers validation before any client +// request and the background-context adapters omitted by the shared contract. +func TestSFTPInvalidPathsAndThinWrappers(t *testing.T) { + d := &driver{} + calls := []func() error{ + func() error { _, err := d.GetContext(nil, "../bad"); return err }, + func() error { return d.PutContext(nil, "../bad", nil) }, + func() error { return d.MakeDirContext(nil, "../bad") }, + func() error { return d.DeleteContext(nil, "../bad") }, + func() error { _, err := d.StatContext(nil, "../bad"); return err }, + func() error { _, err := d.ExistsContext(nil, "../bad"); return err }, + func() error { _, err := d.ListContext(nil, "../bad"); return err }, + func() error { return d.WalkContext(nil, "../bad", func(storagecore.Entry) error { return nil }) }, + func() error { return d.CopyContext(nil, "../bad", "dst") }, + func() error { return d.CopyContext(nil, "src", "../bad") }, + func() error { return d.MoveContext(nil, "../bad", "dst") }, + func() error { return d.MoveContext(nil, "src", "../bad") }, + } + for index, call := range calls { + if err := call(); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("invalid-path call %d error = %v", index, err) + } + } + if err := d.MakeDir("../bad"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("MakeDir invalid path error = %v", err) + } + if _, err := d.ListPage("../bad", 0, 1); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("ListPage error = %v", err) + } } // Open returns the configured download body, payload, or open failure. func (f *fakeSFTPClient) Open(string) (io.ReadCloser, error) { + if f.openHook != nil { + f.openHook() + } if f.openErr != nil { return nil, f.openErr } @@ -269,11 +308,16 @@ func (f *fakeSFTPClient) OpenFile(path string, flags int) (io.WriteCloser, error } // MkdirAll returns the configured parent-creation failure. -func (f *fakeSFTPClient) MkdirAll(string) error { return f.mkdirErr } +func (f *fakeSFTPClient) MkdirAll(string) error { + return f.mkdirErr +} // Remove records file and temporary cleanup paths before returning its configured failure. func (f *fakeSFTPClient) Remove(path string) error { f.removePaths = append(f.removePaths, path) + if f.removeHook != nil { + f.removeHook() + } return f.removeErr } @@ -291,19 +335,27 @@ func (f *fakeSFTPClient) PosixRename(oldname, newname string) error { } // Stat returns configured remote metadata or lookup failure. -func (f *fakeSFTPClient) Stat(string) (os.FileInfo, error) { return f.statInfo, f.statErr } +func (f *fakeSFTPClient) Stat(string) (os.FileInfo, error) { + if f.statHook != nil { + f.statHook() + } + return f.statInfo, f.statErr +} // ReadDir returns configured immediate children or listing failure. -func (f *fakeSFTPClient) ReadDir(string) ([]os.FileInfo, error) { return f.readDir, f.readDirErr } +func (f *fakeSFTPClient) ReadDir(string) ([]os.FileInfo, error) { + return f.readDir, f.readDirErr +} // Close makes fake SFTP client cleanup succeed. -func (f *fakeSFTPClient) Close() error { return nil } +func (f *fakeSFTPClient) Close() error { return f.closeErr } type fakeWriteCloser struct { buf bytes.Buffer writeErr error closeErr error afterWrite func() + zeroWrite bool } // Write captures bytes and can cancel or fail an upload after the write boundary. @@ -311,6 +363,9 @@ func (f *fakeWriteCloser) Write(p []byte) (int, error) { if f.writeErr != nil { return 0, f.writeErr } + if f.zeroWrite { + return 0, nil + } n, err := f.buf.Write(p) if f.afterWrite != nil { f.afterWrite() @@ -652,6 +707,243 @@ func TestSFTPFakeWalkAndErrors(t *testing.T) { }) } +// TestSFTPEdgeFailures covers pagination, transfer cleanup, relocation, and +// closed-client behavior that the shared contract cannot induce. +func TestSFTPEdgeFailures(t *testing.T) { + t.Run("pagination and callback validation", func(t *testing.T) { + client := &fakeSFTPClient{readDir: []os.FileInfo{ + fakeFileInfo{name: "a"}, fakeFileInfo{name: "b"}, + }} + d := &driver{client: client} + page, err := d.ListPage("", 0, 1) + if err != nil || len(page.Entries) != 1 || !page.HasMore { + t.Fatalf("ListPage = %+v, %v", page, err) + } + if err := d.Walk("", nil); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Walk nil callback error = %v", err) + } + if err := d.MakeDir(""); err != nil { + t.Fatalf("MakeDir root: %v", err) + } + }) + + t.Run("copy and move failures", func(t *testing.T) { + client := &fakeSFTPClient{openErr: os.ErrNotExist, statErr: os.ErrNotExist} + d := &driver{client: client} + if err := d.Copy("missing", "copy"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Copy missing error = %v", err) + } + if err := d.Move("missing", "moved"); !errors.Is(err, storagecore.ErrNotFound) { + t.Fatalf("Move missing error = %v", err) + } + + client = &fakeSFTPClient{statInfo: fakeFileInfo{name: "file"}, renameErr: os.ErrPermission} + d = &driver{client: client} + if err := d.Move("file", "moved"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Move rename error = %v", err) + } + }) + + t.Run("temporary cleanup errors are joined", func(t *testing.T) { + writeErr := errors.New("write") + closeErr := errors.New("close") + removeErr := errors.New("remove") + client := &fakeSFTPClient{ + openFile: &fakeWriteCloser{writeErr: writeErr, closeErr: closeErr}, + removeErr: removeErr, + } + d := &driver{client: client} + err := d.Put("file", []byte("data")) + if !errors.Is(err, writeErr) || !errors.Is(err, closeErr) || !errors.Is(err, removeErr) { + t.Fatalf("Put joined error = %v", err) + } + }) + + t.Run("close is terminal", func(t *testing.T) { + closeErr := errors.New("close") + d := &driver{client: &fakeSFTPClient{closeErr: closeErr}} + if err := d.Close(); !errors.Is(err, closeErr) { + t.Fatalf("Close error = %v", err) + } + if err := d.Close(); !errors.Is(err, closeErr) { + t.Fatalf("second Close error = %v", err) + } + calls := []func() error{ + func() error { _, err := d.Get("file"); return err }, + func() error { return d.Put("file", nil) }, + func() error { return d.MakeDir("dir") }, + func() error { return d.Delete("file") }, + func() error { _, err := d.Stat("file"); return err }, + func() error { _, err := d.Exists("file"); return err }, + func() error { _, err := d.List(""); return err }, + func() error { return d.Walk("", func(storagecore.Entry) error { return nil }) }, + func() error { return d.Copy("file", "copy") }, + func() error { return d.Move("file", "moved") }, + } + for index, call := range calls { + if err := call(); err == nil { + t.Fatalf("closed call %d returned nil error", index) + } + } + }) +} + +// TestSFTPPostClientCancellation verifies cancellation wins when it arrives +// across an SFTP call boundary that cannot accept a context directly. +func TestSFTPPostClientCancellation(t *testing.T) { + tests := []struct { + name string + call func(*driver, context.Context) error + make func(context.CancelFunc) *fakeSFTPClient + }{ + {name: "get", call: func(d *driver, ctx context.Context) error { _, err := d.GetContext(ctx, "file"); return err }, make: func(cancel context.CancelFunc) *fakeSFTPClient { + return &fakeSFTPClient{openData: "data", openHook: cancel} + }}, + {name: "delete stat", call: func(d *driver, ctx context.Context) error { return d.DeleteContext(ctx, "file") }, make: func(cancel context.CancelFunc) *fakeSFTPClient { + return &fakeSFTPClient{statInfo: fakeFileInfo{name: "file"}, statHook: cancel} + }}, + {name: "delete remove", call: func(d *driver, ctx context.Context) error { return d.DeleteContext(ctx, "file") }, make: func(cancel context.CancelFunc) *fakeSFTPClient { + return &fakeSFTPClient{statInfo: fakeFileInfo{name: "file"}, removeHook: cancel} + }}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + d := &driver{client: test.make(cancel)} + if err := test.call(d, ctx); !errors.Is(err, context.Canceled) { + t.Fatalf("error = %v", err) + } + }) + } +} + +// TestSFTPPureHelperEdges covers configuration validation, stream cleanup, +// and short writes without requiring an SSH server. +func TestSFTPPureHelperEdges(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := newFromDiskConfig(ctx, storagecore.ResolvedConfig{SFTPHost: "host"}); !errors.Is(err, context.Canceled) { + t.Fatalf("newFromDiskConfig canceled error = %v", err) + } + if _, err := newFromDiskConfig(context.Background(), storagecore.ResolvedConfig{ + SFTPHost: "host", SFTPPassword: "secret", SFTPKnownHostsPath: "known", SFTPInsecureIgnoreHostKey: true, + }); err == nil { + t.Fatal("newFromDiskConfig accepted conflicting host-key settings") + } + for _, port := range []int{-1, 65536} { + if _, err := New(Config{Host: "host", Port: port, Password: "secret", InsecureIgnoreHostKey: true}); err == nil { + t.Fatalf("New accepted port %d", port) + } + } + if _, err := buildAuth(storagecore.ResolvedConfig{SFTPKeyPath: filepath.Join(t.TempDir(), "missing")}); err == nil { + t.Fatal("buildAuth missing key returned nil error") + } + if _, err := buildHostKeyCallback(storagecore.ResolvedConfig{SFTPKnownHostsPath: "known", SFTPInsecureIgnoreHostKey: true}); err == nil { + t.Fatal("buildHostKeyCallback accepted conflicting settings") + } + if _, err := newFromDiskConfig(context.Background(), storagecore.ResolvedConfig{SFTPHost: "host", SFTPInsecureIgnoreHostKey: true}); err == nil { + t.Fatal("newFromDiskConfig missing auth returned nil error") + } + if _, err := newFromDiskConfig(context.Background(), storagecore.ResolvedConfig{SFTPHost: "host", SFTPPassword: "secret"}); err == nil { + t.Fatal("newFromDiskConfig missing host verification returned nil error") + } + + closeErr := errors.New("close") + writeErr := errors.New("write") + d := &driver{client: &fakeSFTPClient{openReader: &sftpTrackedReader{Reader: bytes.NewReader([]byte("data")), closeErr: closeErr}}} + if _, err := d.Get("file"); !errors.Is(err, closeErr) { + t.Fatalf("Get close error = %v", err) + } + d = &driver{client: &fakeSFTPClient{openFile: &fakeWriteCloser{zeroWrite: true}}} + if err := d.Put("file", []byte("data")); !errors.Is(err, io.ErrShortWrite) { + t.Fatalf("Put short write error = %v", err) + } + d = &driver{client: &fakeSFTPClient{mkdirErr: os.ErrPermission}} + if err := d.MakeDir("dir"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("MakeDir error = %v", err) + } + d = &driver{client: &fakeSFTPClient{openFileErr: os.ErrPermission}} + if err := d.Put("file", []byte("data")); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Put open error = %v", err) + } + d = &driver{client: &fakeSFTPClient{openFile: &fakeWriteCloser{closeErr: closeErr}}} + if err := d.Put("file", []byte("data")); !errors.Is(err, closeErr) { + t.Fatalf("Put close error = %v", err) + } + d = &driver{client: &fakeSFTPClient{openFile: &fakeWriteCloser{writeErr: writeErr}, removeErr: os.ErrNotExist}} + if err := d.Put("file", []byte("data")); !errors.Is(err, writeErr) { + t.Fatalf("Put cleanup not-found error = %v", err) + } + d = &driver{client: &fakeSFTPClient{statInfo: fakeFileInfo{name: "file"}, mkdirErr: os.ErrPermission}} + if err := d.Move("file", "nested/moved"); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatalf("Move mkdir error = %v", err) + } + + if _, err := copyContext(context.Background(), sftpFailingWriter{err: writeErr}, bytes.NewReader([]byte("x"))); !errors.Is(err, writeErr) { + t.Fatalf("copyContext write error = %v", err) + } + if _, err := copyContext(context.Background(), sftpShortWriter{}, bytes.NewReader([]byte("x"))); !errors.Is(err, io.ErrShortWrite) { + t.Fatalf("copyContext short write error = %v", err) + } + if err := joinCleanup(nil, closeErr); !errors.Is(err, closeErr) { + t.Fatalf("joinCleanup cleanup error = %v", err) + } + if err := (&realSFTPClient{}).Close(); err != nil { + t.Fatalf("empty real client Close = %v", err) + } +} + +// TestSFTPMidIterationCancellation verifies listing and walking recheck the +// caller context between remote entries. +func TestSFTPMidIterationCancellation(t *testing.T) { + client := &fakeSFTPClient{ + statInfo: fakeFileInfo{name: "dir", isDir: true}, + readDir: []os.FileInfo{fakeFileInfo{name: "a"}, fakeFileInfo{name: "b"}}, + } + d := &driver{client: client} + if _, err := d.ListContext(&sftpStepContext{Context: context.Background(), cancelAt: 2}, ""); !errors.Is(err, context.Canceled) { + t.Fatalf("ListContext cancellation = %v", err) + } + if err := d.WalkContext(&sftpStepContext{Context: context.Background(), cancelAt: 3}, "dir", func(storagecore.Entry) error { return nil }); !errors.Is(err, context.Canceled) { + t.Fatalf("WalkContext cancellation = %v", err) + } +} + +type sftpStepContext struct { + context.Context + calls int + cancelAt int +} + +// Err begins returning context.Canceled at the configured observation count. +func (c *sftpStepContext) Err() error { + c.calls++ + if c.calls >= c.cancelAt { + return context.Canceled + } + return nil +} + +type sftpTrackedReader struct { + io.Reader + closeErr error +} + +// Close returns the configured download cleanup failure. +func (r *sftpTrackedReader) Close() error { return r.closeErr } + +type sftpFailingWriter struct { + err error +} + +// Write injects a destination failure. +func (w sftpFailingWriter) Write([]byte) (int, error) { return 0, w.err } + +type sftpShortWriter struct{} + +// Write accepts no bytes without error to exercise short-write handling. +func (sftpShortWriter) Write([]byte) (int, error) { return 0, nil } + type failingReadCloser struct{} // Read injects a deterministic failure after a download opens. From c2127876d3958f2c5d76550d651769265b318618 Mon Sep 17 00:00:00 2001 From: Chris Miles Date: Sat, 17 Oct 2026 13:04:32 -0500 Subject: [PATCH 2/8] chore(release): prepare storage v0.5.1 --- docs/bench/go.mod | 22 +++++++++++----------- driver/dropboxstorage/go.mod | 2 +- driver/ftpstorage/go.mod | 2 +- driver/gcsstorage/go.mod | 2 +- driver/localstorage/go.mod | 2 +- driver/memorystorage/go.mod | 2 +- driver/rclonestorage/go.mod | 2 +- driver/redisstorage/go.mod | 2 +- driver/s3storage/go.mod | 2 +- driver/sftpstorage/go.mod | 2 +- examples/go.mod | 22 +++++++++++----------- go.mod | 2 +- integration/go.mod | 22 +++++++++++----------- storagetest/go.mod | 6 +++--- 14 files changed, 46 insertions(+), 46 deletions(-) diff --git a/docs/bench/go.mod b/docs/bench/go.mod index 98c3d62..56965b9 100644 --- a/docs/bench/go.mod +++ b/docs/bench/go.mod @@ -4,16 +4,16 @@ go 1.26.0 require ( github.com/fsouza/fake-gcs-server v1.56.1 - github.com/goforj/storage v0.5.0 - github.com/goforj/storage/driver/ftpstorage v0.5.0 - github.com/goforj/storage/driver/gcsstorage v0.5.0 - github.com/goforj/storage/driver/localstorage v0.5.0 - github.com/goforj/storage/driver/memorystorage v0.5.0 - github.com/goforj/storage/driver/rclonestorage v0.5.0 - github.com/goforj/storage/driver/redisstorage v0.5.0 - github.com/goforj/storage/driver/s3storage v0.5.0 - github.com/goforj/storage/driver/sftpstorage v0.5.0 - github.com/goforj/storage/storagetest v0.5.0 + github.com/goforj/storage v0.5.1 + github.com/goforj/storage/driver/ftpstorage v0.5.1 + github.com/goforj/storage/driver/gcsstorage v0.5.1 + github.com/goforj/storage/driver/localstorage v0.5.1 + github.com/goforj/storage/driver/memorystorage v0.5.1 + github.com/goforj/storage/driver/rclonestorage v0.5.1 + github.com/goforj/storage/driver/redisstorage v0.5.1 + github.com/goforj/storage/driver/s3storage v0.5.1 + github.com/goforj/storage/driver/sftpstorage v0.5.1 + github.com/goforj/storage/storagetest v0.5.1 github.com/goftp/server v0.0.0-20200708154336-f64f7c2d8a42 github.com/testcontainers/testcontainers-go v0.44.0 ) @@ -133,7 +133,7 @@ require ( github.com/go-resty/resty/v2 v2.17.2 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/goccy/go-json v0.10.6 // indirect - github.com/goforj/storage/storagecore v0.5.0 // indirect + github.com/goforj/storage/storagecore v0.5.1 // indirect github.com/gofrs/flock v0.13.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang-jwt/jwt/v4 v4.5.2 // indirect diff --git a/driver/dropboxstorage/go.mod b/driver/dropboxstorage/go.mod index 3c9481e..2d81169 100644 --- a/driver/dropboxstorage/go.mod +++ b/driver/dropboxstorage/go.mod @@ -4,7 +4,7 @@ go 1.25.0 require ( github.com/dropbox/dropbox-sdk-go-unofficial/v6 v6.6.1 - github.com/goforj/storage/storagecore v0.5.0 + github.com/goforj/storage/storagecore v0.5.1 ) require golang.org/x/oauth2 v0.30.0 // indirect diff --git a/driver/ftpstorage/go.mod b/driver/ftpstorage/go.mod index 25813d1..89f3060 100644 --- a/driver/ftpstorage/go.mod +++ b/driver/ftpstorage/go.mod @@ -3,7 +3,7 @@ module github.com/goforj/storage/driver/ftpstorage go 1.24.4 require ( - github.com/goforj/storage/storagecore v0.5.0 + github.com/goforj/storage/storagecore v0.5.1 github.com/goftp/server v0.0.0-20200708154336-f64f7c2d8a42 github.com/jlaffaye/ftp v0.2.4 ) diff --git a/driver/gcsstorage/go.mod b/driver/gcsstorage/go.mod index 8346417..5b48e7d 100644 --- a/driver/gcsstorage/go.mod +++ b/driver/gcsstorage/go.mod @@ -5,7 +5,7 @@ go 1.26.0 require ( cloud.google.com/go/storage v1.67.0 github.com/fsouza/fake-gcs-server v1.56.1 - github.com/goforj/storage/storagecore v0.5.0 + github.com/goforj/storage/storagecore v0.5.1 google.golang.org/api v0.297.0 ) diff --git a/driver/localstorage/go.mod b/driver/localstorage/go.mod index 24abeca..43b5a66 100644 --- a/driver/localstorage/go.mod +++ b/driver/localstorage/go.mod @@ -3,6 +3,6 @@ module github.com/goforj/storage/driver/localstorage go 1.25.0 require ( - github.com/goforj/storage/storagecore v0.5.0 + github.com/goforj/storage/storagecore v0.5.1 golang.org/x/sys v0.47.0 ) diff --git a/driver/memorystorage/go.mod b/driver/memorystorage/go.mod index df557fa..816e588 100644 --- a/driver/memorystorage/go.mod +++ b/driver/memorystorage/go.mod @@ -2,4 +2,4 @@ module github.com/goforj/storage/driver/memorystorage go 1.24.4 -require github.com/goforj/storage/storagecore v0.5.0 +require github.com/goforj/storage/storagecore v0.5.1 diff --git a/driver/rclonestorage/go.mod b/driver/rclonestorage/go.mod index ad86abf..5201e40 100644 --- a/driver/rclonestorage/go.mod +++ b/driver/rclonestorage/go.mod @@ -3,7 +3,7 @@ module github.com/goforj/storage/driver/rclonestorage go 1.26.0 require ( - github.com/goforj/storage/storagecore v0.5.0 + github.com/goforj/storage/storagecore v0.5.1 github.com/johannesboyne/gofakes3 v1.2.0 github.com/rclone/rclone v1.75.2 github.com/unknwon/goconfig v1.0.0 diff --git a/driver/redisstorage/go.mod b/driver/redisstorage/go.mod index f25efbb..d06d0a0 100644 --- a/driver/redisstorage/go.mod +++ b/driver/redisstorage/go.mod @@ -4,7 +4,7 @@ go 1.26.0 require ( github.com/alicebob/miniredis/v2 v2.39.0 - github.com/goforj/storage/storagecore v0.5.0 + github.com/goforj/storage/storagecore v0.5.1 github.com/redis/go-redis/v9 v9.22.0 github.com/testcontainers/testcontainers-go v0.44.0 ) diff --git a/driver/s3storage/go.mod b/driver/s3storage/go.mod index 4143a5b..75cbf84 100644 --- a/driver/s3storage/go.mod +++ b/driver/s3storage/go.mod @@ -8,7 +8,7 @@ require ( github.com/aws/aws-sdk-go-v2/credentials v1.20.3 github.com/aws/aws-sdk-go-v2/service/s3 v1.111.0 github.com/aws/smithy-go v1.28.1 - github.com/goforj/storage/storagecore v0.5.0 + github.com/goforj/storage/storagecore v0.5.1 github.com/johannesboyne/gofakes3 v1.2.0 github.com/stretchr/testify v1.12.1 ) diff --git a/driver/sftpstorage/go.mod b/driver/sftpstorage/go.mod index f6dbe3c..6e5b179 100644 --- a/driver/sftpstorage/go.mod +++ b/driver/sftpstorage/go.mod @@ -3,7 +3,7 @@ module github.com/goforj/storage/driver/sftpstorage go 1.26.0 require ( - github.com/goforj/storage/storagecore v0.5.0 + github.com/goforj/storage/storagecore v0.5.1 github.com/pkg/sftp v1.13.11 golang.org/x/crypto v0.56.0 ) diff --git a/examples/go.mod b/examples/go.mod index 03ab561..b62e4e2 100644 --- a/examples/go.mod +++ b/examples/go.mod @@ -3,16 +3,16 @@ module github.com/goforj/storage/examples go 1.26.0 require ( - github.com/goforj/storage v0.5.0 - github.com/goforj/storage/driver/dropboxstorage v0.5.0 - github.com/goforj/storage/driver/ftpstorage v0.5.0 - github.com/goforj/storage/driver/gcsstorage v0.5.0 - github.com/goforj/storage/driver/localstorage v0.5.0 - github.com/goforj/storage/driver/memorystorage v0.5.0 - github.com/goforj/storage/driver/rclonestorage v0.5.0 - github.com/goforj/storage/driver/redisstorage v0.5.0 - github.com/goforj/storage/driver/s3storage v0.5.0 - github.com/goforj/storage/driver/sftpstorage v0.5.0 + github.com/goforj/storage v0.5.1 + github.com/goforj/storage/driver/dropboxstorage v0.5.1 + github.com/goforj/storage/driver/ftpstorage v0.5.1 + github.com/goforj/storage/driver/gcsstorage v0.5.1 + github.com/goforj/storage/driver/localstorage v0.5.1 + github.com/goforj/storage/driver/memorystorage v0.5.1 + github.com/goforj/storage/driver/rclonestorage v0.5.1 + github.com/goforj/storage/driver/redisstorage v0.5.1 + github.com/goforj/storage/driver/s3storage v0.5.1 + github.com/goforj/storage/driver/sftpstorage v0.5.1 ) replace github.com/goforj/storage => ./.. @@ -140,7 +140,7 @@ require ( github.com/go-resty/resty/v2 v2.17.2 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/goccy/go-json v0.10.6 // indirect - github.com/goforj/storage/storagecore v0.5.0 // indirect + github.com/goforj/storage/storagecore v0.5.1 // indirect github.com/gofrs/flock v0.13.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang-jwt/jwt/v4 v4.5.2 // indirect diff --git a/go.mod b/go.mod index 2e0bfbe..48e2c65 100644 --- a/go.mod +++ b/go.mod @@ -2,4 +2,4 @@ module github.com/goforj/storage go 1.24.4 -require github.com/goforj/storage/storagecore v0.5.0 +require github.com/goforj/storage/storagecore v0.5.1 diff --git a/integration/go.mod b/integration/go.mod index 746fa8b..743f398 100644 --- a/integration/go.mod +++ b/integration/go.mod @@ -4,16 +4,16 @@ go 1.26.0 require ( github.com/fsouza/fake-gcs-server v1.56.1 - github.com/goforj/storage v0.5.0 - github.com/goforj/storage/driver/ftpstorage v0.5.0 - github.com/goforj/storage/driver/gcsstorage v0.5.0 - github.com/goforj/storage/driver/localstorage v0.5.0 - github.com/goforj/storage/driver/memorystorage v0.5.0 - github.com/goforj/storage/driver/rclonestorage v0.5.0 - github.com/goforj/storage/driver/redisstorage v0.5.0 - github.com/goforj/storage/driver/s3storage v0.5.0 - github.com/goforj/storage/driver/sftpstorage v0.5.0 - github.com/goforj/storage/storagetest v0.5.0 + github.com/goforj/storage v0.5.1 + github.com/goforj/storage/driver/ftpstorage v0.5.1 + github.com/goforj/storage/driver/gcsstorage v0.5.1 + github.com/goforj/storage/driver/localstorage v0.5.1 + github.com/goforj/storage/driver/memorystorage v0.5.1 + github.com/goforj/storage/driver/rclonestorage v0.5.1 + github.com/goforj/storage/driver/redisstorage v0.5.1 + github.com/goforj/storage/driver/s3storage v0.5.1 + github.com/goforj/storage/driver/sftpstorage v0.5.1 + github.com/goforj/storage/storagetest v0.5.1 github.com/goftp/server v0.0.0-20200708154336-f64f7c2d8a42 github.com/testcontainers/testcontainers-go v0.44.0 ) @@ -155,7 +155,7 @@ require ( github.com/go-resty/resty/v2 v2.17.2 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/goccy/go-json v0.10.6 // indirect - github.com/goforj/storage/storagecore v0.5.0 // indirect + github.com/goforj/storage/storagecore v0.5.1 // indirect github.com/gofrs/flock v0.13.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang-jwt/jwt/v4 v4.5.2 // indirect diff --git a/storagetest/go.mod b/storagetest/go.mod index 4076eda..079fdf5 100644 --- a/storagetest/go.mod +++ b/storagetest/go.mod @@ -9,9 +9,9 @@ require ( github.com/aws/aws-sdk-go-v2/credentials v1.20.3 github.com/aws/aws-sdk-go-v2/service/s3 v1.111.0 github.com/fsouza/fake-gcs-server v1.56.1 - github.com/goforj/storage v0.5.0 - github.com/goforj/storage/driver/memorystorage v0.5.0 - github.com/goforj/storage/storagecore v0.5.0 + github.com/goforj/storage v0.5.1 + github.com/goforj/storage/driver/memorystorage v0.5.1 + github.com/goforj/storage/storagecore v0.5.1 github.com/johannesboyne/gofakes3 v1.2.0 google.golang.org/api v0.297.0 ) From 24ec77d658d30c588b3336fcb235268a996e62f1 Mon Sep 17 00:00:00 2001 From: Chris Miles Date: Sun, 18 Oct 2026 01:29:47 -0500 Subject: [PATCH 3/8] feat: define optional streaming object storage capability --- storagecore/objects.go | 55 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 storagecore/objects.go diff --git a/storagecore/objects.go b/storagecore/objects.go new file mode 100644 index 0000000..7218130 --- /dev/null +++ b/storagecore/objects.go @@ -0,0 +1,55 @@ +package storagecore + +import ( + "context" + "io" + "time" +) + +// ObjectMetadata contains portable HTTP content attributes retained by object stores. +type ObjectMetadata struct { + ContentType string + ContentEncoding string + ContentDisposition string + CacheControl string + Values map[string]string +} + +// ObjectInfo describes an object using its exact key relative to the configured disk prefix. +// Keys are opaque, including dot segments and repeated slashes, and are never filesystem paths. +type ObjectInfo struct { + Key string + Size int64 + ETag string + Version string + ModifiedAt time.Time + Metadata ObjectMetadata +} + +// OpenObjectOptions optionally pins a read to a version or a previously observed entity tag. +type OpenObjectOptions struct { + Version string + IfMatch string +} + +// ObjectReader owns a streaming response that the caller must close, including after partial reads. +type ObjectReader struct { + io.ReadCloser + Info ObjectInfo +} + +// PutObjectOptions declares the exact stream size and portable metadata without buffering the object. +// The caller owns the reader and should provide a seekable reader when retries are required. +type PutObjectOptions struct { + Size int64 + Metadata ObjectMetadata +} + +// ObjectStorage is an optional streaming capability for opaque object keys. +// Existing Storage path semantics and buffered methods remain unchanged. Walk visits +// objects page by page without synthesizing directories or accumulating the whole listing. +type ObjectStorage interface { + WalkObjectsContext(ctx context.Context, prefix string, visit func(ObjectInfo) error) error + OpenObjectContext(ctx context.Context, key string, options OpenObjectOptions) (*ObjectReader, error) + PutObjectContext(ctx context.Context, key string, body io.Reader, options PutObjectOptions) (ObjectInfo, error) +} From b403d13f1e70a2e99e1e8f6e44a9fa88bcab6be0 Mon Sep 17 00:00:00 2001 From: Chris Miles Date: Sun, 18 Oct 2026 01:31:52 -0500 Subject: [PATCH 4/8] feat: stream S3 objects through optional storage capability --- README.md | 18 ++++ driver/s3storage/bucket.go | 49 +++++++++ driver/s3storage/go.mod | 2 +- driver/s3storage/go.sum | 2 + driver/s3storage/objects.go | 150 +++++++++++++++++++++++++++ driver/s3storage/objects_test.go | 167 +++++++++++++++++++++++++++++++ go.mod | 2 +- go.sum | 2 + objects.go | 21 ++++ objects_test.go | 51 ++++++++++ 10 files changed, 462 insertions(+), 2 deletions(-) create mode 100644 driver/s3storage/bucket.go create mode 100644 driver/s3storage/objects.go create mode 100644 driver/s3storage/objects_test.go create mode 100644 objects.go create mode 100644 objects_test.go diff --git a/README.md b/README.md index 66226dd..97be76d 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,24 @@ Choose the construction style that fits your application: All storage operations also expose `*Context` equivalents for deadlines and cancellation. The default methods use `context.Background()`. +### Streaming objects + +`storage.Objects(disk)` discovers the optional `storagecore.ObjectStorage` +capability. The S3 driver supports paged object walking, streaming reads and +writes, metadata preservation, version-pinned reads, and conditional reads using +an observed ETag. Unsupported drivers return `storage.ErrUnsupported`; there is +no implicit fallback that buffers whole objects in memory. + +Object keys are opaque and relative to the configured disk prefix. Unlike the +existing file API, these methods preserve dot segments and repeated slashes. +Each operation takes an explicit context. Close every returned `ObjectReader`. +Writes declare an exact size and leave ownership of the input reader with the +caller; provide a seekable reader when provider retries are required. + +Bucket creation remains explicit through `s3storage.PrepareBucketContext`. +Ordinary storage construction and object reads never provision provider resources. +Existing buffered methods, driver interfaces, and minimum Go versions are unchanged. + ### Common operations ```go diff --git a/driver/s3storage/bucket.go b/driver/s3storage/bucket.go new file mode 100644 index 0000000..697ee69 --- /dev/null +++ b/driver/s3storage/bucket.go @@ -0,0 +1,49 @@ +package s3storage + +import ( + "context" + "errors" + "fmt" + + "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/aws/smithy-go" + "github.com/goforj/storage/storagecore" +) + +// bucketAPI is separate from ordinary object operations because provisioning is explicitly requested. +type bucketAPI interface { + HeadBucket(context.Context, *s3.HeadBucketInput, ...func(*s3.Options)) (*s3.HeadBucketOutput, error) + CreateBucket(context.Context, *s3.CreateBucketInput, ...func(*s3.Options)) (*s3.CreateBucketOutput, error) +} + +// PrepareBucketContext creates a missing bucket using the same configuration and credentials as the driver. +// Existing buckets are unchanged. Permission failures never trigger a creation attempt. +func PrepareBucketContext(ctx context.Context, cfg Config) error { + ctx = normalizeContext(ctx) + store, err := newFromDiskConfig(ctx, cfg.ResolvedConfig()) + if err != nil { + return err + } + client, ok := store.(*driver).client.(bucketAPI) + if !ok { + return storagecore.ErrUnsupported + } + if _, err = client.HeadBucket(ctx, &s3.HeadBucketInput{Bucket: aws.String(cfg.Bucket)}); err == nil { + return nil + } + var api smithy.APIError + if !errors.As(err, &api) || api.ErrorCode() != "NotFound" && api.ErrorCode() != "NoSuchBucket" { + return fmt.Errorf("storage: inspect bucket: %w", wrapError(err)) + } + input := &s3.CreateBucketInput{Bucket: aws.String(cfg.Bucket)} + if cfg.Region != "us-east-1" { + input.CreateBucketConfiguration = &types.CreateBucketConfiguration{LocationConstraint: types.BucketLocationConstraint(cfg.Region)} + } + _, err = client.CreateBucket(ctx, input) + if err != nil { + return fmt.Errorf("storage: prepare bucket: %w", wrapError(err)) + } + return nil +} diff --git a/driver/s3storage/go.mod b/driver/s3storage/go.mod index 042cd4e..481e602 100644 --- a/driver/s3storage/go.mod +++ b/driver/s3storage/go.mod @@ -8,7 +8,7 @@ require ( github.com/aws/aws-sdk-go-v2/credentials v1.18.21 github.com/aws/aws-sdk-go-v2/service/s3 v1.90.0 github.com/aws/smithy-go v1.23.2 - github.com/goforj/storage/storagecore v0.5.0 + github.com/goforj/storage/storagecore v0.5.1-0.20261018062947-24ec77d658d3 github.com/johannesboyne/gofakes3 v0.0.0-20250916175020-ebf3e50324d3 github.com/stretchr/testify v1.11.1 ) diff --git a/driver/s3storage/go.sum b/driver/s3storage/go.sum index c439d0c..1352e76 100644 --- a/driver/s3storage/go.sum +++ b/driver/s3storage/go.sum @@ -101,6 +101,8 @@ github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2 github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/goforj/storage/storagecore v0.5.0 h1:1NHEDS/P6BOEOk0DIq5gQsvgz/Zew3v1ZG2XZ3pZ9Zw= github.com/goforj/storage/storagecore v0.5.0/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= +github.com/goforj/storage/storagecore v0.5.1-0.20261018062947-24ec77d658d3 h1:GmAeqDaGG5B733T8KtOIdOtaug0A7iU23fm0Q/fAHao= +github.com/goforj/storage/storagecore v0.5.1-0.20261018062947-24ec77d658d3/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/groupcache v0.0.0-20191227052852-215e87163ea7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= diff --git a/driver/s3storage/objects.go b/driver/s3storage/objects.go new file mode 100644 index 0000000..6534d2d --- /dev/null +++ b/driver/s3storage/objects.go @@ -0,0 +1,150 @@ +package s3storage + +import ( + "context" + "fmt" + "io" + "strings" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/goforj/storage/storagecore" +) + +// objectKey preserves opaque keys while keeping all access within the configured prefix. +func (d *driver) objectKey(key string) (string, error) { + if strings.ContainsRune(key, '\x00') || d.prefix == "" && key == "" { + return "", fmt.Errorf("%w: invalid object key", storagecore.ErrForbidden) + } + if d.prefix != "" { + return d.prefix + "/" + key, nil + } + return key, nil +} + +// objectPrefix scopes flat listings without treating keys as filesystem paths. +func (d *driver) objectPrefix(prefix string) (string, error) { + if strings.ContainsRune(prefix, '\x00') { + return "", fmt.Errorf("%w: invalid object prefix", storagecore.ErrForbidden) + } + if d.prefix != "" { + return d.prefix + "/" + prefix, nil + } + return prefix, nil +} + +// WalkObjectsContext iterates raw objects one provider page at a time and propagates callback cancellation. +func (d *driver) WalkObjectsContext(ctx context.Context, prefix string, visit func(storagecore.ObjectInfo) error) error { + ctx = normalizeContext(ctx) + if err := ctx.Err(); err != nil { + return err + } + if visit == nil { + return fmt.Errorf("storage: object visitor is required") + } + scoped, err := d.objectPrefix(prefix) + if err != nil { + return err + } + var token *string + for { + page, err := d.client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{Bucket: aws.String(d.bucket), Prefix: aws.String(scoped), ContinuationToken: token}) + if err != nil { + return wrapError(err) + } + for _, object := range page.Contents { + key := aws.ToString(object.Key) + if !strings.HasPrefix(key, scoped) { + return fmt.Errorf("%w: object outside disk prefix", storagecore.ErrForbidden) + } + relative := key + if d.prefix != "" { + relative = strings.TrimPrefix(key, d.prefix+"/") + } + info := storagecore.ObjectInfo{Key: relative, Size: aws.ToInt64(object.Size), ETag: aws.ToString(object.ETag)} + if object.LastModified != nil { + info.ModifiedAt = *object.LastModified + } + if err := ctx.Err(); err != nil { + return err + } + if err := visit(info); err != nil { + return err + } + } + if !aws.ToBool(page.IsTruncated) { + return nil + } + next := aws.ToString(page.NextContinuationToken) + if next == "" || next == aws.ToString(token) { + return fmt.Errorf("storage: object listing did not advance") + } + token = page.NextContinuationToken + } +} + +// OpenObjectContext streams a prefix-scoped object with optional version and entity-tag conditions. +func (d *driver) OpenObjectContext(ctx context.Context, key string, options storagecore.OpenObjectOptions) (*storagecore.ObjectReader, error) { + ctx = normalizeContext(ctx) + if err := ctx.Err(); err != nil { + return nil, err + } + scoped, err := d.objectKey(key) + if err != nil { + return nil, err + } + input := &s3.GetObjectInput{Bucket: aws.String(d.bucket), Key: aws.String(scoped)} + if options.Version != "" { + input.VersionId = aws.String(options.Version) + } + if options.IfMatch != "" { + input.IfMatch = aws.String(options.IfMatch) + } + out, err := d.client.GetObject(ctx, input) + if err != nil { + return nil, wrapError(err) + } + if options.IfMatch != "" && options.IfMatch != "*" && strings.Trim(options.IfMatch, "\"") != strings.Trim(aws.ToString(out.ETag), "\"") { + out.Body.Close() + return nil, fmt.Errorf("storage: object changed during conditional read") + } + info := storagecore.ObjectInfo{Key: key, Size: aws.ToInt64(out.ContentLength), ETag: aws.ToString(out.ETag), Version: aws.ToString(out.VersionId), Metadata: storagecore.ObjectMetadata{ContentType: aws.ToString(out.ContentType), ContentEncoding: aws.ToString(out.ContentEncoding), ContentDisposition: aws.ToString(out.ContentDisposition), CacheControl: aws.ToString(out.CacheControl), Values: out.Metadata}} + if out.LastModified != nil { + info.ModifiedAt = *out.LastModified + } + return &storagecore.ObjectReader{ReadCloser: out.Body, Info: info}, nil +} + +// PutObjectContext passes a bounded stream and its metadata directly to the provider. +func (d *driver) PutObjectContext(ctx context.Context, key string, body io.Reader, options storagecore.PutObjectOptions) (storagecore.ObjectInfo, error) { + ctx = normalizeContext(ctx) + if err := ctx.Err(); err != nil { + return storagecore.ObjectInfo{}, err + } + scoped, err := d.objectKey(key) + if err != nil { + return storagecore.ObjectInfo{}, err + } + if body == nil || options.Size < 0 { + return storagecore.ObjectInfo{}, fmt.Errorf("storage: object body and nonnegative size are required") + } + metadata := options.Metadata + input := &s3.PutObjectInput{Bucket: aws.String(d.bucket), Key: aws.String(scoped), Body: body, ContentLength: aws.Int64(options.Size), Metadata: metadata.Values} + if metadata.ContentType != "" { + input.ContentType = aws.String(metadata.ContentType) + } + if metadata.ContentEncoding != "" { + input.ContentEncoding = aws.String(metadata.ContentEncoding) + } + if metadata.ContentDisposition != "" { + input.ContentDisposition = aws.String(metadata.ContentDisposition) + } + if metadata.CacheControl != "" { + input.CacheControl = aws.String(metadata.CacheControl) + } + out, err := d.client.PutObject(ctx, input) + if err != nil { + return storagecore.ObjectInfo{}, wrapError(err) + } + return storagecore.ObjectInfo{Key: key, Size: options.Size, ETag: aws.ToString(out.ETag), Version: aws.ToString(out.VersionId), Metadata: metadata}, nil +} diff --git a/driver/s3storage/objects_test.go b/driver/s3storage/objects_test.go new file mode 100644 index 0000000..87fcc4d --- /dev/null +++ b/driver/s3storage/objects_test.go @@ -0,0 +1,167 @@ +package s3storage + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "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/goforj/storage/storagecore" + "github.com/johannesboyne/gofakes3" + "github.com/johannesboyne/gofakes3/backend/s3mem" +) + +// TestStreamingObjectsPreserveOpaqueKeysAndMetadata exercises the optional capability against the provider protocol. +func TestStreamingObjectsPreserveOpaqueKeysAndMetadata(t *testing.T) { + server := fakeServer(t, gofakes3.New(s3mem.New())) + defer server.Close() + cfg := Config{Bucket: "streaming", Endpoint: server.URL, Region: "us-east-1", AccessKeyID: "access", SecretAccessKey: "secret", UsePathStyle: true, Prefix: "tenant"} + if err := PrepareBucketContext(context.Background(), cfg); err != nil { + t.Fatal(err) + } + store, err := New(cfg) + if err != nil { + t.Fatal(err) + } + objects := store.(storagecore.ObjectStorage) + ctx := context.Background() + key := "path/../odd//name" + info, err := objects.PutObjectContext(ctx, key, strings.NewReader("payload"), storagecore.PutObjectOptions{Size: 7, Metadata: storagecore.ObjectMetadata{ContentType: "text/plain", Values: map[string]string{"owner": "fixture"}}}) + if err != nil || info.Size != 7 { + t.Fatal(info, err) + } + count := 0 + if err := objects.WalkObjectsContext(ctx, "", func(info storagecore.ObjectInfo) error { + count++ + if info.Key != key || info.Size != 7 { + t.Errorf("key or size changed: %#v", info) + } + return nil + }); err != nil || count != 1 { + t.Fatal(count, err) + } + reader, err := objects.OpenObjectContext(ctx, key, storagecore.OpenObjectOptions{IfMatch: info.ETag}) + if err != nil { + t.Fatal(err) + } + data, err := io.ReadAll(reader) + reader.Close() + if err != nil || string(data) != "payload" || reader.Info.Metadata.ContentType != "text/plain" || reader.Info.Metadata.Values["owner"] != "fixture" { + t.Fatal("stream or metadata changed", err, reader.Info) + } + if _, err := objects.OpenObjectContext(ctx, key, storagecore.OpenObjectOptions{IfMatch: "different"}); err == nil { + t.Fatal("conditional read accepted changed object") + } + stop := errors.New("stop") + if err := objects.WalkObjectsContext(ctx, "", func(storagecore.ObjectInfo) error { return stop }); !errors.Is(err, stop) { + t.Fatal("callback failure lost", err) + } + canceled, cancel := context.WithCancel(ctx) + cancel() + if err := objects.WalkObjectsContext(canceled, "", func(storagecore.ObjectInfo) error { return nil }); !errors.Is(err, context.Canceled) { + t.Fatal(err) + } + if _, err := objects.OpenObjectContext(canceled, key, storagecore.OpenObjectOptions{}); !errors.Is(err, context.Canceled) { + t.Fatal(err) + } + if _, err := objects.PutObjectContext(canceled, key, strings.NewReader(""), storagecore.PutObjectOptions{}); !errors.Is(err, context.Canceled) { + t.Fatal(err) + } + for _, bad := range []string{"bad\x00key"} { + if _, err := objects.OpenObjectContext(ctx, bad, storagecore.OpenObjectOptions{}); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatal(err) + } + if _, err := objects.PutObjectContext(ctx, bad, strings.NewReader(""), storagecore.PutObjectOptions{}); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatal(err) + } + if err := objects.WalkObjectsContext(ctx, bad, func(storagecore.ObjectInfo) error { return nil }); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatal(err) + } + } + if err := objects.WalkObjectsContext(ctx, "", nil); err == nil { + t.Fatal("nil visitor accepted") + } + if _, err := objects.PutObjectContext(ctx, key, nil, storagecore.PutObjectOptions{}); err == nil { + t.Fatal("nil body accepted") + } + if _, err := objects.PutObjectContext(ctx, key, strings.NewReader(""), storagecore.PutObjectOptions{Size: -1}); err == nil { + t.Fatal("negative size accepted") + } +} + +// TestStreamingObjectFailuresKeepScopeAndStopPagination validates provider and pagination failures without buffering. +func TestStreamingObjectFailuresKeepScopeAndStopPagination(t *testing.T) { + ctx := context.Background() + failure := errors.New("provider failed") + for _, op := range []string{"list", "read", "write"} { + fake := &fakeS3{listErr: failure, getErr: failure, putErr: failure} + disk := &driver{client: fake, prefix: "tenant", bucket: "fixture"} + var err error + switch op { + case "list": + err = disk.WalkObjectsContext(ctx, "", func(storagecore.ObjectInfo) error { return nil }) + case "read": + _, err = disk.OpenObjectContext(ctx, "key", storagecore.OpenObjectOptions{Version: "pinned-version"}) + if len(fake.getInputs) != 1 || *fake.getInputs[0].VersionId != "pinned-version" { + t.Fatal("version pin lost") + } + case "write": + _, err = disk.PutObjectContext(ctx, "key", strings.NewReader("data"), storagecore.PutObjectOptions{Size: 4}) + } + if err == nil { + t.Fatal("provider error swallowed", op) + } + } + for _, page := range []*s3.ListObjectsV2Output{ + {IsTruncated: aws.Bool(true)}, + {Contents: []types.Object{{Key: aws.String("tenant-other/key")}}}, + } { + disk := &driver{client: &fakeS3{listOut: page}, prefix: "tenant", bucket: "fixture"} + if err := disk.WalkObjectsContext(ctx, "", func(storagecore.ObjectInfo) error { return nil }); err == nil { + t.Fatal("invalid listing accepted") + } + } + disk := &driver{client: &fakeS3{}, bucket: "fixture"} + if _, err := disk.OpenObjectContext(ctx, "", storagecore.OpenObjectOptions{}); !errors.Is(err, storagecore.ErrForbidden) { + t.Fatal("empty root key accepted", err) + } +} + +// TestPrepareBucketDistinguishesForbiddenAndMissing verifies provisioning stays opt-in and permission failures are preserved. +func TestPrepareBucketDistinguishesForbiddenAndMissing(t *testing.T) { + for _, status := range []int{200, 403, 404} { + t.Run(http.StatusText(status), func(t *testing.T) { + created := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodHead { + w.WriteHeader(status) + return + } + if r.Method == http.MethodPut { + created++ + w.WriteHeader(200) + return + } + w.WriteHeader(500) + })) + defer server.Close() + err := PrepareBucketContext(context.Background(), Config{Bucket: "fixture", Region: "us-east-1", Endpoint: server.URL, AccessKeyID: "fixture", SecretAccessKey: "fixture", UsePathStyle: true}) + if status == 403 { + if err == nil || created != 0 { + t.Fatal("forbidden bucket provisioned", err, created) + } + } else if err != nil { + t.Fatal(err) + } + if status == 404 && created != 1 { + t.Fatal("missing bucket not prepared", created) + } + }) + } +} diff --git a/go.mod b/go.mod index 2e0bfbe..1a3e38d 100644 --- a/go.mod +++ b/go.mod @@ -2,4 +2,4 @@ module github.com/goforj/storage go 1.24.4 -require github.com/goforj/storage/storagecore v0.5.0 +require github.com/goforj/storage/storagecore v0.5.1-0.20261018062947-24ec77d658d3 diff --git a/go.sum b/go.sum index 4b4815f..f82808d 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,4 @@ github.com/goforj/storage/storagecore v0.5.0 h1:1NHEDS/P6BOEOk0DIq5gQsvgz/Zew3v1ZG2XZ3pZ9Zw= github.com/goforj/storage/storagecore v0.5.0/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= +github.com/goforj/storage/storagecore v0.5.1-0.20261018062947-24ec77d658d3 h1:GmAeqDaGG5B733T8KtOIdOtaug0A7iU23fm0Q/fAHao= +github.com/goforj/storage/storagecore v0.5.1-0.20261018062947-24ec77d658d3/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= diff --git a/objects.go b/objects.go new file mode 100644 index 0000000..ec62efd --- /dev/null +++ b/objects.go @@ -0,0 +1,21 @@ +package storage + +import "github.com/goforj/storage/storagecore" + +// Objects returns a driver's optional streaming object capability without changing Storage. +// Every operation takes its own context; a handle's WithContext binding does not replace it. +// Unsupported drivers return ErrUnsupported rather than buffering a stream as a fallback. +func Objects(disk Storage) (storagecore.ObjectStorage, error) { + if bound, ok := disk.(*boundStorage); ok { + if bound.bindErr != nil { + return nil, bound.bindErr + } + if objects, ok := bound.inner.(storagecore.ObjectStorage); ok { + return objects, nil + } + } + if objects, ok := disk.(storagecore.ObjectStorage); ok { + return objects, nil + } + return nil, ErrUnsupported +} diff --git a/objects_test.go b/objects_test.go new file mode 100644 index 0000000..2eab6ab --- /dev/null +++ b/objects_test.go @@ -0,0 +1,51 @@ +package storage + +import ( + "context" + "errors" + "io" + "testing" + + "github.com/goforj/storage/storagecore" +) + +// streamFixture exposes the optional capability without adding it to the base Storage interface. +type streamFixture struct{ stubFS } + +// WithContext preserves the fixture capability across public bindings. +func (s *streamFixture) WithContext(context.Context) Storage { return s } + +// WalkObjectsContext keeps the fixture focused on capability discovery. +func (*streamFixture) WalkObjectsContext(context.Context, string, func(storagecore.ObjectInfo) error) error { + return nil +} + +// OpenObjectContext keeps the fixture focused on capability discovery. +func (*streamFixture) OpenObjectContext(context.Context, string, storagecore.OpenObjectOptions) (*storagecore.ObjectReader, error) { + return nil, nil +} + +// PutObjectContext keeps the fixture focused on capability discovery. +func (*streamFixture) PutObjectContext(context.Context, string, io.Reader, storagecore.PutObjectOptions) (storagecore.ObjectInfo, error) { + return storagecore.ObjectInfo{}, nil +} + +// TestObjectsCapabilityPreservesExistingStorage verifies supported, unsupported, and failed bindings. +func TestObjectsCapabilityPreservesExistingStorage(t *testing.T) { + stream := &streamFixture{} + for _, disk := range []Storage{stream, wrapStorage(stream), wrapStorage(stream).WithContext(context.Background())} { + got, err := Objects(disk) + if err != nil || got != stream { + t.Fatal("streaming capability lost", got, err) + } + } + for _, disk := range []Storage{nil, &stubFS{}, wrapStorage(&stubFS{})} { + if _, err := Objects(disk); !errors.Is(err, ErrUnsupported) { + t.Fatal(err) + } + } + failure := errors.New("binding failed") + if _, err := Objects(&boundStorage{inner: stream, bindErr: failure}); !errors.Is(err, failure) { + t.Fatal(err) + } +} From 6d8e8c25863f0a9e6e202c7d77c05f0cec6750ba Mon Sep 17 00:00:00 2001 From: Chris Miles Date: Sun, 18 Oct 2026 01:42:38 -0500 Subject: [PATCH 5/8] fix: pin streaming core above the published storage release --- driver/s3storage/go.mod | 2 +- driver/s3storage/go.sum | 2 ++ go.mod | 2 +- go.sum | 2 ++ 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/driver/s3storage/go.mod b/driver/s3storage/go.mod index 75cbf84..894d9bb 100644 --- a/driver/s3storage/go.mod +++ b/driver/s3storage/go.mod @@ -8,7 +8,7 @@ require ( github.com/aws/aws-sdk-go-v2/credentials v1.20.3 github.com/aws/aws-sdk-go-v2/service/s3 v1.111.0 github.com/aws/smithy-go v1.28.1 - github.com/goforj/storage/storagecore v0.5.1 + github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b github.com/johannesboyne/gofakes3 v1.2.0 github.com/stretchr/testify v1.12.1 ) diff --git a/driver/s3storage/go.sum b/driver/s3storage/go.sum index 2bd593d..c388eb1 100644 --- a/driver/s3storage/go.sum +++ b/driver/s3storage/go.sum @@ -98,6 +98,8 @@ github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2 github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/goforj/storage/storagecore v0.5.1 h1:q9B6F9ne/KqZtLDM5wqWcmoZz+teWzhu7OLGp3aUCqY= github.com/goforj/storage/storagecore v0.5.1/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= +github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b h1:DoxWa3MWGyCYqF7BngY1KKMZfUpGWCSr7NNMvqZ72Jo= +github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/groupcache v0.0.0-20191227052852-215e87163ea7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= diff --git a/go.mod b/go.mod index 48e2c65..a6fae84 100644 --- a/go.mod +++ b/go.mod @@ -2,4 +2,4 @@ module github.com/goforj/storage go 1.24.4 -require github.com/goforj/storage/storagecore v0.5.1 +require github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b diff --git a/go.sum b/go.sum index 4b2ec25..cb8c39c 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,4 @@ github.com/goforj/storage/storagecore v0.5.1 h1:q9B6F9ne/KqZtLDM5wqWcmoZz+teWzhu7OLGp3aUCqY= github.com/goforj/storage/storagecore v0.5.1/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= +github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b h1:DoxWa3MWGyCYqF7BngY1KKMZfUpGWCSr7NNMvqZ72Jo= +github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= From c7275ca08e400b0349ca0e12aec020235acbb74d Mon Sep 17 00:00:00 2001 From: Chris Miles Date: Sun, 18 Oct 2026 01:50:02 -0500 Subject: [PATCH 6/8] fix: synchronize streaming core module metadata for CI --- docs/bench/go.mod | 2 +- driver/s3storage/go.sum | 2 -- examples/go.mod | 2 +- go.sum | 2 -- integration/go.mod | 2 +- 5 files changed, 3 insertions(+), 7 deletions(-) diff --git a/docs/bench/go.mod b/docs/bench/go.mod index 56965b9..557b8cc 100644 --- a/docs/bench/go.mod +++ b/docs/bench/go.mod @@ -133,7 +133,7 @@ require ( github.com/go-resty/resty/v2 v2.17.2 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/goccy/go-json v0.10.6 // indirect - github.com/goforj/storage/storagecore v0.5.1 // indirect + github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b // indirect github.com/gofrs/flock v0.13.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang-jwt/jwt/v4 v4.5.2 // indirect diff --git a/driver/s3storage/go.sum b/driver/s3storage/go.sum index c388eb1..3dd9115 100644 --- a/driver/s3storage/go.sum +++ b/driver/s3storage/go.sum @@ -96,8 +96,6 @@ github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7 github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= -github.com/goforj/storage/storagecore v0.5.1 h1:q9B6F9ne/KqZtLDM5wqWcmoZz+teWzhu7OLGp3aUCqY= -github.com/goforj/storage/storagecore v0.5.1/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b h1:DoxWa3MWGyCYqF7BngY1KKMZfUpGWCSr7NNMvqZ72Jo= github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= diff --git a/examples/go.mod b/examples/go.mod index b62e4e2..7f04372 100644 --- a/examples/go.mod +++ b/examples/go.mod @@ -140,7 +140,7 @@ require ( github.com/go-resty/resty/v2 v2.17.2 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/goccy/go-json v0.10.6 // indirect - github.com/goforj/storage/storagecore v0.5.1 // indirect + github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b // indirect github.com/gofrs/flock v0.13.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang-jwt/jwt/v4 v4.5.2 // indirect diff --git a/go.sum b/go.sum index cb8c39c..c60f599 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,2 @@ -github.com/goforj/storage/storagecore v0.5.1 h1:q9B6F9ne/KqZtLDM5wqWcmoZz+teWzhu7OLGp3aUCqY= -github.com/goforj/storage/storagecore v0.5.1/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b h1:DoxWa3MWGyCYqF7BngY1KKMZfUpGWCSr7NNMvqZ72Jo= github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b/go.mod h1:n6epEk/dinioVPKa8VkWduaf0b1ggcnycXRmd6nURCY= diff --git a/integration/go.mod b/integration/go.mod index 743f398..70e1981 100644 --- a/integration/go.mod +++ b/integration/go.mod @@ -155,7 +155,7 @@ require ( github.com/go-resty/resty/v2 v2.17.2 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/goccy/go-json v0.10.6 // indirect - github.com/goforj/storage/storagecore v0.5.1 // indirect + github.com/goforj/storage/storagecore v0.5.2-0.20261018064156-39419460440b // indirect github.com/gofrs/flock v0.13.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang-jwt/jwt/v4 v4.5.2 // indirect From d7c4338e0d017984ac477b7ae407a9aed8d9e243 Mon Sep 17 00:00:00 2001 From: Chris Miles Date: Sun, 18 Oct 2026 02:00:33 -0500 Subject: [PATCH 7/8] test: cover streaming object and bucket preparation boundaries --- driver/s3storage/objects_contract_test.go | 150 ++++++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 driver/s3storage/objects_contract_test.go diff --git a/driver/s3storage/objects_contract_test.go b/driver/s3storage/objects_contract_test.go new file mode 100644 index 0000000..93d74b4 --- /dev/null +++ b/driver/s3storage/objects_contract_test.go @@ -0,0 +1,150 @@ +package s3storage + +import ( + "context" + "errors" + "io" + "strings" + "testing" + "time" + + "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/aws/smithy-go" + "github.com/goforj/storage/storagecore" +) + +// bucketFixture records provisioning separately from ordinary object operations. +type bucketFixture struct { + fakeS3 + inspectErr error + createErr error + created []*s3.CreateBucketInput +} + +// HeadBucket supplies provider failure classes without network retry delays. +func (f *bucketFixture) HeadBucket(context.Context, *s3.HeadBucketInput, ...func(*s3.Options)) (*s3.HeadBucketOutput, error) { + return &s3.HeadBucketOutput{}, f.inspectErr +} + +// CreateBucket records the region constraint and preserves provisioning errors. +func (f *bucketFixture) CreateBucket(_ context.Context, input *s3.CreateBucketInput, _ ...func(*s3.Options)) (*s3.CreateBucketOutput, error) { + f.created = append(f.created, input) + return &s3.CreateBucketOutput{}, f.createErr +} + +// TestBucketPreparationFailureBoundaries ensures only missing buckets trigger provisioning. +func TestBucketPreparationFailureBoundaries(t *testing.T) { + original := buildS3Clients + t.Cleanup(func() { buildS3Clients = original }) + cfg := Config{Bucket: "fixture", Region: "eu-west-1", AccessKeyID: "fixture", SecretAccessKey: "fixture"} + for _, test := range []struct { + name string + client s3API + cfg Config + wantError bool + wantCreates int + }{ + {name: "invalid configuration", client: &bucketFixture{}, cfg: Config{}, wantError: true}, + {name: "unsupported provisioning", client: &fakeS3{}, cfg: cfg, wantError: true}, + {name: "transport failure", client: &bucketFixture{inspectErr: errors.New("transport failed")}, cfg: cfg, wantError: true}, + {name: "missing regional bucket", client: &bucketFixture{inspectErr: &smithy.GenericAPIError{Code: "NoSuchBucket"}}, cfg: cfg, wantCreates: 1}, + {name: "create failure", client: &bucketFixture{inspectErr: &smithy.GenericAPIError{Code: "NotFound"}, createErr: errors.New("creation refused")}, cfg: cfg, wantError: true, wantCreates: 1}, + } { + t.Run(test.name, func(t *testing.T) { + buildS3Clients = func(aws.Config, storagecore.ResolvedConfig) (s3API, s3PresignAPI) { return test.client, nil } + err := PrepareBucketContext(context.Background(), test.cfg) + if (err != nil) != test.wantError { + t.Fatal("unexpected provisioning outcome", err) + } + if fixture, ok := test.client.(*bucketFixture); ok { + if len(fixture.created) != test.wantCreates { + t.Fatal("unexpected creation attempt", fixture.created) + } + for _, created := range fixture.created { + if created.CreateBucketConfiguration == nil || string(created.CreateBucketConfiguration.LocationConstraint) != "eu-west-1" { + t.Fatal("regional constraint lost", created) + } + } + } + }) + } +} + +// objectResponseFixture supplies streaming response ownership and metadata independently of list fixtures. +type objectResponseFixture struct { + fakeS3 + response *s3.GetObjectOutput +} + +// GetObject records conditional inputs while returning the configured streaming response. +func (f *objectResponseFixture) GetObject(_ context.Context, input *s3.GetObjectInput, _ ...func(*s3.Options)) (*s3.GetObjectOutput, error) { + f.getInputs = append(f.getInputs, input) + return f.response, nil +} + +// TestObjectPaginationPreservesContinuationAndCancellation checks page boundaries and callback stop behavior. +func TestObjectPaginationPreservesContinuationAndCancellation(t *testing.T) { + modified := time.Date(2026, 10, 18, 0, 0, 0, 0, time.UTC) + fake := &fakeS3{listSeq: []*s3.ListObjectsV2Output{ + {Contents: []types.Object{{Key: aws.String("first"), Size: aws.Int64(1), LastModified: &modified}}, IsTruncated: aws.Bool(true), NextContinuationToken: aws.String("second-page")}, + {Contents: []types.Object{{Key: aws.String("second"), Size: aws.Int64(2)}}}, + }} + disk := &driver{client: fake, bucket: "fixture"} + var found []storagecore.ObjectInfo + if err := disk.WalkObjectsContext(context.Background(), "", func(info storagecore.ObjectInfo) error { found = append(found, info); return nil }); err != nil { + t.Fatal(err) + } + if len(found) != 2 || found[0].Key != "first" || !found[0].ModifiedAt.Equal(modified) || found[1].Key != "second" || len(fake.listInputs) != 2 || aws.ToString(fake.listInputs[1].ContinuationToken) != "second-page" { + t.Fatal("pagination lost data", found, fake.listInputs) + } + fake.listSeq = []*s3.ListObjectsV2Output{{IsTruncated: aws.Bool(true), NextContinuationToken: aws.String("repeated")}, {IsTruncated: aws.Bool(true), NextContinuationToken: aws.String("repeated")}} + if err := disk.WalkObjectsContext(context.Background(), "", func(storagecore.ObjectInfo) error { return nil }); err == nil { + t.Fatal("repeated token accepted") + } + ctx, cancel := context.WithCancel(context.Background()) + fake.listHook = cancel + fake.listOut = &s3.ListObjectsV2Output{Contents: []types.Object{{Key: aws.String("after-cancellation")}}} + called := false + if err := disk.WalkObjectsContext(ctx, "", func(storagecore.ObjectInfo) error { called = true; return nil }); !errors.Is(err, context.Canceled) || called { + t.Fatal("canceled page delivered", called, err) + } +} + +// TestObjectStreamingOwnershipAndMetadata verifies stream lifetime and metadata at the provider boundary. +func TestObjectStreamingOwnershipAndMetadata(t *testing.T) { + modified := time.Date(2026, 10, 18, 0, 0, 0, 0, time.UTC) + body := &trackedReadCloser{reader: strings.NewReader("data")} + fixture := &objectResponseFixture{response: &s3.GetObjectOutput{Body: body, ContentLength: aws.Int64(4), ETag: aws.String("\"observed\""), VersionId: aws.String("version"), LastModified: &modified, ContentType: aws.String("text/plain"), ContentEncoding: aws.String("identity"), ContentDisposition: aws.String("attachment"), CacheControl: aws.String("max-age=60"), Metadata: map[string]string{"owner": "fixture"}}} + disk := &driver{client: fixture, bucket: "fixture"} + reader, err := disk.OpenObjectContext(context.Background(), "path/../opaque", storagecore.OpenObjectOptions{Version: "version", IfMatch: "observed"}) + if err != nil { + t.Fatal(err) + } + if body.closed { + t.Fatal("stream closed before caller finished") + } + data, err := io.ReadAll(reader) + if err != nil || string(data) != "data" || reader.Info.Version != "version" || !reader.Info.ModifiedAt.Equal(modified) { + t.Fatal("stream metadata lost", reader.Info, err) + } + if err := reader.Close(); err != nil || !body.closed { + t.Fatal("caller close lost", err) + } + metadata := reader.Info.Metadata + if metadata.ContentType != "text/plain" || metadata.ContentEncoding != "identity" || metadata.ContentDisposition != "attachment" || metadata.CacheControl != "max-age=60" || metadata.Values["owner"] != "fixture" { + t.Fatal("read metadata lost", metadata) + } + if _, err := disk.PutObjectContext(context.Background(), "path/../opaque", strings.NewReader("data"), storagecore.PutObjectOptions{Size: 4, Metadata: metadata}); err != nil { + t.Fatal(err) + } + put := fixture.putInputs[0] + if aws.ToString(put.Key) != "path/../opaque" || aws.ToString(put.ContentEncoding) != "identity" || aws.ToString(put.ContentDisposition) != "attachment" || aws.ToString(put.CacheControl) != "max-age=60" || put.Metadata["owner"] != "fixture" { + t.Fatal("write metadata or key changed", put) + } + body.closed = false + if _, err := disk.OpenObjectContext(context.Background(), "key", storagecore.OpenObjectOptions{IfMatch: "stale"}); err == nil || !body.closed { + t.Fatal("stale stream retained") + } +} From ca39f268afd3529433882b278e970b25c1614969 Mon Sep 17 00:00:00 2001 From: Chris Miles Date: Sun, 18 Oct 2026 02:26:37 -0500 Subject: [PATCH 8/8] fix: address independent streaming storage review findings --- README.md | 7 + driver/s3storage/objects.go | 65 ++++++- driver/s3storage/objects_protocol_test.go | 197 ++++++++++++++++++++++ driver/s3storage/s3.go | 31 +++- 4 files changed, 289 insertions(+), 11 deletions(-) create mode 100644 driver/s3storage/objects_protocol_test.go diff --git a/README.md b/README.md index ce06517..2a13578 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,13 @@ existing file API, these methods preserve dot segments and repeated slashes. Each operation takes an explicit context. Close every returned `ObjectReader`. Writes declare an exact size and leave ownership of the input reader with the caller; provide a seekable reader when provider retries are required. +For non-seekable S3 uploads, HTTPS retains the SDK's streaming checksums. Plain +HTTP requires explicit `s3storage.Config.UnsignedPayload` opt-in; only that case +skips optional SDK checksum precomputation. Signed HTTP uploads require a seekable +reader. The library never silently disables configured payload signing or spools +whole objects to disk. Cancellation stops subsequent source reads, including SDK +prehashing; a reader that blocks inside its own `Read` must honor cancellation or +be unblocked by its owner. Bucket creation remains explicit through `s3storage.PrepareBucketContext`. Ordinary storage construction and object reads never provision provider resources. diff --git a/driver/s3storage/objects.go b/driver/s3storage/objects.go index 6534d2d..272f6b4 100644 --- a/driver/s3storage/objects.go +++ b/driver/s3storage/objects.go @@ -4,10 +4,12 @@ import ( "context" "fmt" "io" + "net/url" "strings" "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/goforj/storage/storagecore" ) @@ -48,12 +50,18 @@ func (d *driver) WalkObjectsContext(ctx context.Context, prefix string, visit fu } var token *string for { - page, err := d.client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{Bucket: aws.String(d.bucket), Prefix: aws.String(scoped), ContinuationToken: token}) + page, err := d.client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{Bucket: aws.String(d.bucket), Prefix: aws.String(scoped), ContinuationToken: token, EncodingType: types.EncodingTypeUrl}) if err != nil { return wrapError(err) } for _, object := range page.Contents { key := aws.ToString(object.Key) + if page.EncodingType == types.EncodingTypeUrl { + key, err = url.PathUnescape(key) + if err != nil { + return fmt.Errorf("storage: decode object key: %w", err) + } + } if !strings.HasPrefix(key, scoped) { return fmt.Errorf("%w: object outside disk prefix", storagecore.ErrForbidden) } @@ -128,6 +136,18 @@ func (d *driver) PutObjectContext(ctx context.Context, key string, body io.Reade if body == nil || options.Size < 0 { return storagecore.ObjectInfo{}, fmt.Errorf("storage: object body and nonnegative size are required") } + _, seekable := body.(io.Seeker) + var operationOptions []func(*s3.Options) + if !seekable && d.httpEndpoint && options.Size > 0 { + if !d.unsignedPayload { + return storagecore.ObjectInfo{}, fmt.Errorf("%w: non-seekable HTTP uploads require UnsignedPayload or a seekable body", storagecore.ErrUnsupported) + } + // The caller already opted out of payload signing; optional checksums also require rewindable HTTP streams. + operationOptions = append(operationOptions, func(options *s3.Options) { + options.RequestChecksumCalculation = aws.RequestChecksumCalculationWhenRequired + }) + } + body = contextObjectBody(ctx, body) metadata := options.Metadata input := &s3.PutObjectInput{Bucket: aws.String(d.bucket), Key: aws.String(scoped), Body: body, ContentLength: aws.Int64(options.Size), Metadata: metadata.Values} if metadata.ContentType != "" { @@ -142,9 +162,50 @@ func (d *driver) PutObjectContext(ctx context.Context, key string, body io.Reade if metadata.CacheControl != "" { input.CacheControl = aws.String(metadata.CacheControl) } - out, err := d.client.PutObject(ctx, input) + out, err := d.client.PutObject(ctx, input, operationOptions...) if err != nil { return storagecore.ObjectInfo{}, wrapError(err) } return storagecore.ObjectInfo{Key: key, Size: options.Size, ETag: aws.ToString(out.ETag), Version: aws.ToString(out.VersionId), Metadata: metadata}, nil } + +// objectContextReader checks cancellation during SDK hashing as well as transport reads. +type objectContextReader struct { + ctx context.Context + reader io.Reader +} + +// Read stops additional source consumption as soon as cancellation is observed. +func (r objectContextReader) Read(p []byte) (int, error) { + if err := r.ctx.Err(); err != nil { + return 0, err + } + n, err := r.reader.Read(p) + if canceled := r.ctx.Err(); canceled != nil { + return n, canceled + } + return n, err +} + +// objectContextReadSeeker preserves SDK rewind support for caller-owned seekable sources. +type objectContextReadSeeker struct { + objectContextReader + seeker io.Seeker +} + +// Seek preserves the caller's stream position semantics while honoring cancellation. +func (r objectContextReadSeeker) Seek(offset int64, whence int) (int64, error) { + if err := r.ctx.Err(); err != nil { + return 0, err + } + return r.seeker.Seek(offset, whence) +} + +// contextObjectBody retains seekability without exposing a whole-stream copy fast path that bypasses cancellation. +func contextObjectBody(ctx context.Context, body io.Reader) io.Reader { + reader := objectContextReader{ctx: ctx, reader: body} + if seeker, ok := body.(io.Seeker); ok { + return objectContextReadSeeker{objectContextReader: reader, seeker: seeker} + } + return reader +} diff --git a/driver/s3storage/objects_protocol_test.go b/driver/s3storage/objects_protocol_test.go new file mode 100644 index 0000000..37b47d9 --- /dev/null +++ b/driver/s3storage/objects_protocol_test.go @@ -0,0 +1,197 @@ +package s3storage + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/goforj/storage/storagecore" +) + +// TestObjectListingDecodesOpaqueKeysExactlyOnce prevents XML-sensitive keys from poisoning a provider page. +func TestObjectListingDecodesOpaqueKeysExactlyOnce(t *testing.T) { + keys := []string{"tenant/control\x01key", "tenant/plus+percent%2F space", "tenant/雪"} + malformed := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("encoding-type") != "url" { + t.Error("listing did not request URL encoding") + } + w.Header().Set("Content-Type", "application/xml") + fmt.Fprint(w, "urlfalse") + for _, key := range keys { + encoded := strings.ReplaceAll(url.QueryEscape(key), "+", "%20") + if malformed { + encoded = "%ZZ" + } + fmt.Fprintf(w, "%s1", encoded) + } + fmt.Fprint(w, "") + })) + defer server.Close() + disk, err := New(Config{Bucket: "fixture", Region: "us-east-1", Endpoint: server.URL, AccessKeyID: "fixture", SecretAccessKey: "fixture", UsePathStyle: true, Prefix: "tenant"}) + if err != nil { + t.Fatal(err) + } + objects := disk.(storagecore.ObjectStorage) + var found []string + if err := objects.WalkObjectsContext(context.Background(), "", func(info storagecore.ObjectInfo) error { found = append(found, info.Key); return nil }); err != nil { + t.Fatal(err) + } + if len(found) != len(keys) { + t.Fatal("objects omitted", found) + } + for i, key := range keys { + if found[i] != strings.TrimPrefix(key, "tenant/") { + t.Errorf("opaque key changed: got %q want %q", found[i], key) + } + } + malformed = true + called := false + if err := objects.WalkObjectsContext(context.Background(), "", func(storagecore.ObjectInfo) error { called = true; return nil }); err == nil || called { + t.Fatal("malformed encoding accepted", called, err) + } +} + +// TestNonseekableUploadsRespectHTTPPayloadSigning preserves signing policy without buffering entire objects. +func TestNonseekableUploadsRespectHTTPPayloadSigning(t *testing.T) { + for _, mode := range []string{"explicit", "uppercase", "environment", "service-environment"} { + for _, unsigned := range []bool{false, true} { + t.Run(fmt.Sprint(mode, "/unsigned=", unsigned), func(t *testing.T) { + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + body, err := io.ReadAll(r.Body) + if err != nil || string(body) != "payload" { + t.Errorf("stream changed: %q, %v", body, err) + } + if r.Header.Get("X-Amz-Content-Sha256") != "UNSIGNED-PAYLOAD" { + t.Error("explicit unsigned policy not honored") + } + w.Header().Set("ETag", "\"fixture\"") + })) + defer server.Close() + endpoint := server.URL + switch mode { + case "uppercase": + endpoint = strings.Replace(endpoint, "http:", "HTTP:", 1) + case "environment": + t.Setenv("AWS_ENDPOINT_URL", endpoint) + endpoint = "" + case "service-environment": + t.Setenv("AWS_ENDPOINT_URL_S3", endpoint) + endpoint = "" + } + disk, err := New(Config{Bucket: "fixture", Region: "us-east-1", Endpoint: endpoint, AccessKeyID: "fixture", SecretAccessKey: "fixture", UsePathStyle: true, UnsignedPayload: unsigned}) + if err != nil { + t.Fatal(err) + } + _, err = disk.(storagecore.ObjectStorage).PutObjectContext(context.Background(), "key", io.LimitReader(strings.NewReader("payload"), 7), storagecore.PutObjectOptions{Size: 7}) + if unsigned { + if err != nil || requests != 1 { + t.Fatal("opted-in streaming upload failed", requests, err) + } + } else if !errors.Is(err, storagecore.ErrUnsupported) || requests != 0 { + t.Fatal("signed payload policy weakened", requests, err) + } + }) + } + } + +} + +// TestNonseekableHTTPSUploadsRetainStreamingChecksums exercises the normal SDK streaming protocol over TLS. +func TestNonseekableHTTPSUploadsRetainStreamingChecksums(t *testing.T) { + requests := 0 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + body, err := io.ReadAll(r.Body) + if err != nil || !strings.Contains(string(body), "payload") { + t.Errorf("streaming payload lost: %q, %v", body, err) + } + if !strings.Contains(r.Header.Get("X-Amz-Trailer"), "x-amz-checksum-") || r.Header.Get("X-Amz-Content-Sha256") != "STREAMING-UNSIGNED-PAYLOAD-TRAILER" { + t.Errorf("HTTPS checksum headers: algorithm=%q trailer=%q hash=%q encoding=%q", r.Header.Get("X-Amz-Sdk-Checksum-Algorithm"), r.Header.Get("X-Amz-Trailer"), r.Header.Get("X-Amz-Content-Sha256"), r.Header.Get("Content-Encoding")) + } + w.Header().Set("ETag", "\"fixture\"") + })) + defer server.Close() + original := buildS3Clients + t.Cleanup(func() { buildS3Clients = original }) + buildS3Clients = func(cfg aws.Config, resolved storagecore.ResolvedConfig) (s3API, s3PresignAPI) { + cfg.HTTPClient = server.Client() + return original(cfg, resolved) + } + disk, err := New(Config{Bucket: "fixture", Region: "us-east-1", Endpoint: server.URL, AccessKeyID: "fixture", SecretAccessKey: "fixture", UsePathStyle: true}) + if err != nil { + t.Fatal(err) + } + _, err = disk.(storagecore.ObjectStorage).PutObjectContext(context.Background(), "key", io.LimitReader(strings.NewReader("payload"), 7), storagecore.PutObjectOptions{Size: 7}) + if err != nil || requests != 1 { + t.Fatal("TLS stream failed", requests, err) + } +} + +// cancelUploadReader records source consumption when cancellation arrives during SDK checksum computation. +type cancelUploadReader struct { + reader *strings.Reader + cancel context.CancelFunc + bytes int + calls int +} + +// Read cancels the request after its first bounded read to expose uninterruptible prehash loops. +func (r *cancelUploadReader) Read(p []byte) (int, error) { + n, err := r.reader.Read(p) + r.bytes += n + r.calls++ + if r.calls == 1 { + r.cancel() + } + return n, err +} + +// Seek retains the source's rewindability so the SDK takes its prehash path. +func (r *cancelUploadReader) Seek(offset int64, whence int) (int64, error) { + return r.reader.Seek(offset, whence) +} + +// TestUploadCancellationStopsPrehashConsumption verifies cancellation after hashing has started, before any network request. +func TestUploadCancellationStopsPrehashConsumption(t *testing.T) { + requests := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requests++; w.WriteHeader(500) })) + defer server.Close() + disk, err := New(Config{Bucket: "fixture", Region: "us-east-1", Endpoint: server.URL, AccessKeyID: "fixture", SecretAccessKey: "fixture", UsePathStyle: true}) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + body := &cancelUploadReader{reader: strings.NewReader(strings.Repeat("x", 4<<20)), cancel: cancel} + _, err = disk.(storagecore.ObjectStorage).PutObjectContext(ctx, "key", body, storagecore.PutObjectOptions{Size: 4 << 20}) + if !errors.Is(err, context.Canceled) || body.calls != 1 || body.bytes >= 4<<20 || requests != 0 { + t.Fatal("cancellation consumed the remaining source", body.calls, body.bytes, requests, err) + } +} + +// TestCanceledObjectBodyDoesNotTouchSource ensures canceled SDK reads and rewinds leave caller state untouched. +func TestCanceledObjectBodyDoesNotTouchSource(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + source := strings.NewReader("payload") + body := contextObjectBody(ctx, source) + cancel() + if n, err := body.Read(make([]byte, 3)); n != 0 || !errors.Is(err, context.Canceled) { + t.Fatalf("canceled read = %d, %v", n, err) + } + if _, err := body.(io.Seeker).Seek(2, io.SeekStart); !errors.Is(err, context.Canceled) { + t.Fatalf("canceled seek = %v", err) + } + if position, err := source.Seek(0, io.SeekCurrent); err != nil || position != 0 { + t.Fatalf("canceled operation moved source: %d, %v", position, err) + } +} diff --git a/driver/s3storage/s3.go b/driver/s3storage/s3.go index dad5498..41317bc 100644 --- a/driver/s3storage/s3.go +++ b/driver/s3storage/s3.go @@ -33,10 +33,12 @@ func init() { } type driver struct { - client s3API - presign s3PresignAPI - bucket string - prefix string + httpEndpoint bool + unsignedPayload bool + client s3API + presign s3PresignAPI + bucket string + prefix string } type s3API interface { @@ -158,9 +160,11 @@ func newFromDiskConfig(ctx context.Context, cfg storagecore.ResolvedConfig) (sto return nil, fmt.Errorf("storage: s3 access key ID and secret access key must be provided together") } if cfg.S3Endpoint != "" { - if _, err := normalizeEndpoint(cfg.S3Endpoint); err != nil { + endpoint, err := normalizeEndpoint(cfg.S3Endpoint) + if err != nil { return nil, err } + cfg.S3Endpoint = endpoint } prefix, err := storagecore.NormalizePath(cfg.Prefix) @@ -173,12 +177,21 @@ func newFromDiskConfig(ctx context.Context, cfg storagecore.ResolvedConfig) (sto return nil, fmt.Errorf("storage: load aws config: %w", err) } client, presign := buildS3Clients(awsCfg, cfg) + endpoint := cfg.S3Endpoint + // SDK options include endpoint overrides inherited from environment and shared AWS configuration. + if endpoint == "" { + if configured, ok := client.(interface{ Options() s3.Options }); ok { + endpoint = aws.ToString(configured.Options().BaseEndpoint) + } + } return &driver{ - client: client, - presign: presign, - bucket: cfg.S3Bucket, - prefix: prefix, + httpEndpoint: strings.HasPrefix(strings.ToLower(endpoint), "http://"), + unsignedPayload: cfg.S3UnsignedPayload, + client: client, + presign: presign, + bucket: cfg.S3Bucket, + prefix: prefix, }, nil }