From 1dc1ca69a20a11d9f4b450eb4c0b984c48155d69 Mon Sep 17 00:00:00 2001 From: Yusuf Demir Date: Fri, 17 Oct 2025 03:35:47 +0300 Subject: [PATCH] fix: reviews applied --- firestore/README.md | 4 +++- firestore/config.go | 9 +++------ firestore/firestore.go | 12 +++++++----- firestore/firestore_test.go | 6 +++++- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/firestore/README.md b/firestore/README.md index 83fab286..84dae2eb 100644 --- a/firestore/README.md +++ b/firestore/README.md @@ -22,7 +22,7 @@ A Firestore storage driver using [cloud.google.com/go/firestore](https://pkg.go. ### Signatures ```go -func New(config ...Config) Storage +func New(config ...Config) *Storage func NewFromConnection(client *firestore.Client, collection string) *Storage func (s *Storage) Get(key string) ([]byte, error) func (s *Storage) GetWithContext(ctx context.Context, key string) ([]byte, error) @@ -95,9 +95,11 @@ If you already have a Firestore client configured in your application, you can c ```go import ( "cloud.google.com/go/firestore" + "context" firestorage "github.com/gofiber/storage/firestore/v2" ) +ctx := context.Background() client, err := firestore.NewClient(ctx, "my-gcp-project") if err != nil { panic(err) diff --git a/firestore/config.go b/firestore/config.go index a5d37956..e85b830f 100644 --- a/firestore/config.go +++ b/firestore/config.go @@ -50,14 +50,11 @@ var ConfigDefault = Config{ // configDefault is a helper function to set default values func configDefault(config ...Config) Config { - // Return default config if nothing provided - if len(config) < 1 { - return ConfigDefault + var cfg Config + if len(config) > 0 { + cfg = config[0] } - // Override default config - cfg := config[0] - // Validate required fields if cfg.ProjectID == "" { panic("firestore: ProjectID is required") diff --git a/firestore/firestore.go b/firestore/firestore.go index b1b9ab24..c75f7b8e 100644 --- a/firestore/firestore.go +++ b/firestore/firestore.go @@ -11,11 +11,11 @@ import ( "context" "encoding/json" "fmt" - "io" "os" "time" "cloud.google.com/go/firestore" + "google.golang.org/api/iterator" "google.golang.org/api/option" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -32,7 +32,7 @@ type Storage struct { type document struct { Key string `firestore:"k"` Value []byte `firestore:"v"` - ExpiresAt time.Time `firestore:"exp_at"` + ExpiresAt time.Time `firestore:"exp_at,omitempty"` } // New creates a new Firestore storage instance @@ -197,14 +197,16 @@ func (s *Storage) ResetWithContext(ctx context.Context) error { for { doc, err := docs.Next() - if err == io.EOF { + if err == iterator.Done { break } if err != nil { - break + return fmt.Errorf("failed to iterate documents: %w", err) } - _, _ = bulkWriter.Delete(doc.Ref) + if _, err := bulkWriter.Delete(doc.Ref); err != nil { + return fmt.Errorf("failed to schedule delete for doc %s: %w", doc.Ref.ID, err) + } } bulkWriter.Flush() diff --git a/firestore/firestore_test.go b/firestore/firestore_test.go index c301ebba..e5160e05 100644 --- a/firestore/firestore_test.go +++ b/firestore/firestore_test.go @@ -39,7 +39,11 @@ func newTestStore(t testing.TB) *Storage { ContainerRequest: req, Started: true, }) - testcontainers.CleanupContainer(t, c) + t.Cleanup(func() { + if err := c.Terminate(ctx); err != nil { + t.Fatalf("failed to terminate container: %s", err) + } + }) require.NoError(t, err) host, err := c.Host(ctx)