From 6a6b68178f018b29490bf7f6d7f5aa1e77331af9 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Thu, 20 Nov 2025 10:01:17 -0500 Subject: [PATCH 1/2] Improve error compatibility, update README --- README.md | 14 +- integration/integration_test.go | 15 +- pkg/datastore/cursor_coverage_test.go | 2 +- pkg/datastore/entity_coverage_test.go | 2 +- pkg/datastore/errors.go | 50 ++++++- pkg/datastore/iterator.go | 4 +- pkg/datastore/iterator_coverage_test.go | 4 +- pkg/datastore/iterator_test.go | 6 +- pkg/datastore/operations.go | 189 +++++++++++++++++------- pkg/datastore/operations_delete_test.go | 46 ++++-- pkg/datastore/operations_get_test.go | 21 ++- pkg/datastore/operations_misc_test.go | 15 +- pkg/datastore/operations_put_test.go | 17 ++- pkg/datastore/query.go | 2 +- pkg/datastore/query_coverage_test.go | 4 +- pkg/datastore/transaction.go | 11 +- pkg/mock/mock_test.go | 3 +- 17 files changed, 299 insertions(+), 106 deletions(-) diff --git a/README.md b/README.md index f8f5f2b..4ae5adb 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,15 @@ # ds9 -Zero-dependency Google Cloud Datastore client for Go. Drop-in replacement for `cloud.google.com/go/datastore` basic operations. In-memory mock implementation. Comprehensive testing. +ds9 logo -**Why?** The official client has 50+ dependencies. `ds9` uses only Go stdlib—ideal for lightweight services and minimizing supply chain risk. +[![Go Reference](https://pkg.go.dev/badge/github.com/codeGROOVE-dev/ds9.svg)](https://pkg.go.dev/github.com/codeGROOVE-dev/ds9) +[![Go Report Card](https://goreportcard.com/badge/github.com/codeGROOVE-dev/ds9)](https://goreportcard.com/report/github.com/codeGROOVE-dev/ds9) +[![License: Apache 2.0](https://img.shields.io/badge/License-Apache%202.0-blue.svg)](LICENSE) +[![Go Version](https://img.shields.io/github/go-mod/go-version/codeGROOVE-dev/ds9)](go.mod) + +Zero-dependency Google Cloud Datastore client for Go. Drop-in replacement for `cloud.google.com/go/datastore` with CRUD, transactions, queries, cursors, and mutations. In-memory mock implementation. Comprehensive testing. + +**Why?** The official client has 50+ dependencies. `ds9` has zero—ideal for lightweight services and minimizing supply chain risk. ## Installation @@ -36,6 +43,7 @@ Just switch the import path from `cloud.google.com/go/datastore` to `github.com/ - **Cursors**: Start, End, DecodeCursor - **Keys**: NameKey, IDKey, IncompleteKey, AllocateIDs, parent keys - **Mutations**: NewInsert, NewUpdate, NewUpsert, NewDelete, Mutate +- **Errors**: ErrNoSuchEntity, ErrInvalidKey, ErrInvalidEntityType, ErrConcurrentTransaction, Done, MultiError - **Types**: string, int, int64, int32, bool, float64, time.Time, slices ([]string, []int64, []int, []float64, []bool) **Unsupported Features** @@ -46,6 +54,6 @@ These features are unsupported just because we haven't found a use for the featu ## Testing -* Use `github.com/codeGROOVE-dev/ds9/pkg/mock` package for in-memory testing. It should work even if you choose not to use ds9. +* Use `datastore.NewMockClient(t)` for in-memory testing. Works even if you're still using the official client. * See [TESTING.md](TESTING.md) for integration tests. * We aim to maintain 85% test coverage - please don't send PRs without tests. diff --git a/integration/integration_test.go b/integration/integration_test.go index 1c9ecb3..6afd3ab 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -186,8 +186,19 @@ func TestIntegrationBatchOperations(t *testing.T) { var retrieved []integrationEntity err = client.GetMulti(ctx, keys, &retrieved) - if !errors.Is(err, datastore.ErrNoSuchEntity) { - t.Errorf("expected datastore.ErrNoSuchEntity after DeleteMulti, got %v", err) + if err == nil { + t.Fatal("expected error after DeleteMulti, got nil") + } + + var multiErr datastore.MultiError + if !errors.As(err, &multiErr) { + t.Fatalf("expected MultiError after DeleteMulti, got %T: %v", err, err) + } + + for i, e := range multiErr { + if !errors.Is(e, datastore.ErrNoSuchEntity) { + t.Errorf("expected ErrNoSuchEntity at index %d, got %v", i, e) + } } }) } diff --git a/pkg/datastore/cursor_coverage_test.go b/pkg/datastore/cursor_coverage_test.go index 06c146d..575c030 100644 --- a/pkg/datastore/cursor_coverage_test.go +++ b/pkg/datastore/cursor_coverage_test.go @@ -115,7 +115,7 @@ func TestCursorWithLimitedResults(t *testing.T) { for { var entity testEntity _, err := it.Next(&entity) - if errors.Is(err, datastore.ErrDone) { + if errors.Is(err, datastore.Done) { break } if err != nil { diff --git a/pkg/datastore/entity_coverage_test.go b/pkg/datastore/entity_coverage_test.go index 3875bfa..aeaac23 100644 --- a/pkg/datastore/entity_coverage_test.go +++ b/pkg/datastore/entity_coverage_test.go @@ -212,7 +212,7 @@ func TestIterator_Coverage(t *testing.T) { Time time.Time } _, err := it.Next(&entity) - if errors.Is(err, ErrDone) { + if errors.Is(err, Done) { break } if err != nil { diff --git a/pkg/datastore/errors.go b/pkg/datastore/errors.go index d7b09cd..20f0866 100644 --- a/pkg/datastore/errors.go +++ b/pkg/datastore/errors.go @@ -1,11 +1,53 @@ package datastore -import "errors" +import ( + "errors" + "fmt" +) var ( - // ErrNoSuchEntity is returned when an entity is not found. + // ErrInvalidEntityType is returned when functions like Get or Next are + // passed a dst or src argument of invalid type. + ErrInvalidEntityType = errors.New("datastore: invalid entity type") + + // ErrInvalidKey is returned when an invalid key is presented. + ErrInvalidKey = errors.New("datastore: invalid key") + + // ErrNoSuchEntity is returned when no entity was found for a given key. ErrNoSuchEntity = errors.New("datastore: no such entity") - // ErrDone is returned by Iterator.Next when no more results are available. - ErrDone = errors.New("datastore: no more results") + // ErrConcurrentTransaction is returned when a transaction is used concurrently. + ErrConcurrentTransaction = errors.New("datastore: concurrent transaction") + + // Done is returned by Iterator.Next when no more results are available. + // This matches the official cloud.google.com/go/datastore API. + // + //nolint:revive,errname,staticcheck // Name must match official API (iterator.Done) + Done = errors.New("datastore: no more items in iterator") ) + +// MultiError is returned by batch operations when there are errors with +// particular elements. Errors will be in a one-to-one correspondence with +// the input elements; successful elements will have a nil entry. +type MultiError []error + +func (m MultiError) Error() string { + s, n := "", 0 + for _, e := range m { + if e != nil { + if n == 0 { + s = e.Error() + } + n++ + } + } + switch n { + case 0: + return "(0 errors)" + case 1: + return s + case 2: + return s + " (and 1 other error)" + } + return fmt.Sprintf("%s (and %d other errors)", s, n-1) +} diff --git a/pkg/datastore/iterator.go b/pkg/datastore/iterator.go index f9b7c53..2e39d9f 100644 --- a/pkg/datastore/iterator.go +++ b/pkg/datastore/iterator.go @@ -41,7 +41,7 @@ func (it *Iterator) Next(dst any) (*Key, error) { return nil, it.err } if !it.fetchNext { - return nil, ErrDone + return nil, Done } // Fetch next batch @@ -51,7 +51,7 @@ func (it *Iterator) Next(dst any) (*Key, error) { } if len(it.results) == 0 { - return nil, ErrDone + return nil, Done } } diff --git a/pkg/datastore/iterator_coverage_test.go b/pkg/datastore/iterator_coverage_test.go index 8b2ab5c..d5a4368 100644 --- a/pkg/datastore/iterator_coverage_test.go +++ b/pkg/datastore/iterator_coverage_test.go @@ -64,7 +64,7 @@ func TestIteratorMultipleFetches(t *testing.T) { Time time.Time } _, err := it.Next(&entity) - if errors.Is(err, ErrDone) { + if errors.Is(err, Done) { break } if err != nil { @@ -209,7 +209,7 @@ func TestIteratorKeysOnly(t *testing.T) { Data string } key, err := it.Next(&entity) - if errors.Is(err, ErrDone) { + if errors.Is(err, Done) { break } if err != nil { diff --git a/pkg/datastore/iterator_test.go b/pkg/datastore/iterator_test.go index f77f179..2d94723 100644 --- a/pkg/datastore/iterator_test.go +++ b/pkg/datastore/iterator_test.go @@ -35,7 +35,7 @@ func TestIterator(t *testing.T) { for { var entity testEntity key, err := it.Next(&entity) - if errors.Is(err, datastore.ErrDone) { + if errors.Is(err, datastore.Done) { break } if err != nil { @@ -89,8 +89,8 @@ func TestIterator(t *testing.T) { var entity testEntity _, err := it.Next(&entity) - if !errors.Is(err, datastore.ErrDone) { - t.Errorf("Expected datastore.ErrDone, got %v", err) + if !errors.Is(err, datastore.Done) { + t.Errorf("Expected datastore.Done, got %v", err) } }) } diff --git a/pkg/datastore/operations.go b/pkg/datastore/operations.go index 77d768a..726bf3b 100644 --- a/pkg/datastore/operations.go +++ b/pkg/datastore/operations.go @@ -19,7 +19,11 @@ func (c *Client) Get(ctx context.Context, key *Key, dst any) error { if key == nil { c.logger.WarnContext(ctx, "Get called with nil key") - return errors.New("key cannot be nil") + return ErrInvalidKey + } + + if dst == nil { + return fmt.Errorf("%w: dst cannot be nil", ErrInvalidEntityType) } c.logger.DebugContext(ctx, "getting entity", "kind", key.Kind, "name", key.Name, "id", key.ID) @@ -78,7 +82,7 @@ func (c *Client) Put(ctx context.Context, key *Key, src any) (*Key, error) { ctx = c.withClientConfig(ctx) if key == nil { c.logger.WarnContext(ctx, "Put called with nil key") - return nil, errors.New("key cannot be nil") + return nil, ErrInvalidKey } c.logger.DebugContext(ctx, "putting entity", "kind", key.Kind, "name", key.Name, "id", key.ID) @@ -125,7 +129,7 @@ func (c *Client) Delete(ctx context.Context, key *Key) error { ctx = c.withClientConfig(ctx) if key == nil { c.logger.WarnContext(ctx, "Delete called with nil key") - return errors.New("key cannot be nil") + return ErrInvalidKey } c.logger.DebugContext(ctx, "deleting entity", "kind", key.Kind, "name", key.Name, "id", key.ID) @@ -163,17 +167,39 @@ func (c *Client) Delete(ctx context.Context, key *Key) error { // GetMulti retrieves multiple entities by their keys. // dst must be a pointer to a slice of structs. -// Returns ErrNoSuchEntity if any key is not found. +// Returns MultiError with ErrNoSuchEntity for missing keys, or other errors for specific items. // This matches the API of cloud.google.com/go/datastore. func (c *Client) GetMulti(ctx context.Context, keys []*Key, dst any) error { ctx = c.withClientConfig(ctx) if len(keys) == 0 { c.logger.WarnContext(ctx, "GetMulti called with no keys") - return errors.New("keys cannot be empty") + return fmt.Errorf("%w: keys cannot be empty", ErrInvalidKey) } c.logger.DebugContext(ctx, "getting multiple entities", "count", len(keys)) + // Validate keys first + multiErr := make(MultiError, len(keys)) + hasErr := false + for i, key := range keys { + if key == nil { + c.logger.WarnContext(ctx, "GetMulti called with nil key", "index", i) + multiErr[i] = fmt.Errorf("%w: key at index %d cannot be nil", ErrInvalidKey, i) + hasErr = true + } + } + if hasErr { + return multiErr + } + + // Decode into slice + dstValue := reflect.ValueOf(dst) + if dstValue.Kind() != reflect.Ptr || dstValue.Elem().Kind() != reflect.Slice { + return fmt.Errorf("%w: dst must be a pointer to slice", ErrInvalidEntityType) + } + + sliceType := dstValue.Elem().Type() + token, err := auth.AccessToken(ctx) if err != nil { c.logger.ErrorContext(ctx, "failed to get access token", "error", err) @@ -183,10 +209,6 @@ func (c *Client) GetMulti(ctx context.Context, keys []*Key, dst any) error { // Build keys array jsonKeys := make([]map[string]any, len(keys)) for i, key := range keys { - if key == nil { - c.logger.WarnContext(ctx, "GetMulti called with nil key", "index", i) - return fmt.Errorf("key at index %d cannot be nil", i) - } jsonKeys[i] = keyToJSON(key) } @@ -225,46 +247,89 @@ func (c *Client) GetMulti(ctx context.Context, keys []*Key, dst any) error { return fmt.Errorf("failed to parse response: %w", err) } - if len(result.Missing) > 0 { - c.logger.DebugContext(ctx, "some entities not found", "missing_count", len(result.Missing)) - return ErrNoSuchEntity - } - - // Decode into slice - v := reflect.ValueOf(dst) - if v.Kind() != reflect.Ptr || v.Elem().Kind() != reflect.Slice { - return errors.New("dst must be a pointer to slice") + // Create a map of keys to their indices + keyMap := make(map[string][]int) + for i, key := range keys { + keyStr := key.String() + keyMap[keyStr] = append(keyMap[keyStr], i) } - sliceType := v.Elem().Type() - elemType := sliceType.Elem() + // Create slice to hold results, sized to match keys + resultSlice := reflect.MakeSlice(sliceType, len(keys), len(keys)) - // Create new slice of correct size - slice := reflect.MakeSlice(sliceType, 0, len(result.Found)) + // Mark all as not found initially + for i := range keys { + multiErr[i] = ErrNoSuchEntity + } + // Process found entities for _, found := range result.Found { - elem := reflect.New(elemType).Elem() - if err := decodeEntity(found.Entity, elem.Addr().Interface()); err != nil { - c.logger.ErrorContext(ctx, "failed to decode entity", "error", err) + key, err := keyFromJSON(found.Entity["key"]) + if err != nil { + c.logger.ErrorContext(ctx, "failed to parse key from response", "error", err) return err } - slice = reflect.Append(slice, elem) + + keyStr := key.String() + indices, ok := keyMap[keyStr] + if !ok { + continue + } + + for _, index := range indices { + elem := resultSlice.Index(index) + if err := decodeEntity(found.Entity, elem.Addr().Interface()); err != nil { + c.logger.ErrorContext(ctx, "failed to decode entity", "index", index, "error", err) + multiErr[index] = err + hasErr = true + } else { + multiErr[index] = nil // Success + } + } + } + + // Process missing entities - they already have ErrNoSuchEntity set + for _, missing := range result.Missing { + key, err := keyFromJSON(missing.Entity["key"]) + if err != nil { + continue + } + keyStr := key.String() + if indices, ok := keyMap[keyStr]; ok { + for range indices { + hasErr = true + // multiErr[index] already set to ErrNoSuchEntity above + } + } + } + + // Check if any entities are still marked as missing + for i, e := range multiErr { + if errors.Is(e, ErrNoSuchEntity) { + hasErr = true + c.logger.DebugContext(ctx, "entity not found", "index", i, "key", keys[i].String()) + } } - v.Elem().Set(slice) - c.logger.DebugContext(ctx, "entities retrieved successfully", "count", len(result.Found)) + // Set the result slice + dstValue.Elem().Set(resultSlice) + + if hasErr { + return multiErr + } + + c.logger.DebugContext(ctx, "entities retrieved successfully", "count", len(keys)) return nil } // PutMulti stores multiple entities with their keys. // keys and src must have the same length. -// Returns the keys (same as input) and any error. +// Returns the keys (same as input) and MultiError if any operations failed. // This matches the API of cloud.google.com/go/datastore. func (c *Client) PutMulti(ctx context.Context, keys []*Key, src any) ([]*Key, error) { ctx = c.withClientConfig(ctx) if len(keys) == 0 { - c.logger.WarnContext(ctx, "PutMulti called with no keys") - return nil, errors.New("keys cannot be empty") + return nil, nil } c.logger.DebugContext(ctx, "putting multiple entities", "count", len(keys)) @@ -275,31 +340,32 @@ func (c *Client) PutMulti(ctx context.Context, keys []*Key, src any) ([]*Key, er v = v.Elem() } if v.Kind() != reflect.Slice { - return nil, errors.New("src must be a slice") + return nil, fmt.Errorf("%w: src must be a slice", ErrInvalidEntityType) } if v.Len() != len(keys) { return nil, fmt.Errorf("keys and src length mismatch: %d != %d", len(keys), v.Len()) } - token, err := auth.AccessToken(ctx) - if err != nil { - c.logger.ErrorContext(ctx, "failed to get access token", "error", err) - return nil, fmt.Errorf("failed to get access token: %w", err) - } - - // Build mutations + // Validate keys and encode entities upfront + multiErr := make(MultiError, len(keys)) + hasErr := false mutations := make([]map[string]any, len(keys)) + for i, key := range keys { if key == nil { c.logger.WarnContext(ctx, "PutMulti called with nil key", "index", i) - return nil, fmt.Errorf("key at index %d cannot be nil", i) + multiErr[i] = fmt.Errorf("%w: key at index %d cannot be nil", ErrInvalidKey, i) + hasErr = true + continue } entity, err := encodeEntity(key, v.Index(i).Interface()) if err != nil { c.logger.ErrorContext(ctx, "failed to encode entity", "error", err, "index", i) - return nil, fmt.Errorf("failed to encode entity at index %d: %w", i, err) + multiErr[i] = err + hasErr = true + continue } mutations[i] = map[string]any{ @@ -307,6 +373,17 @@ func (c *Client) PutMulti(ctx context.Context, keys []*Key, src any) ([]*Key, er } } + // If encoding failed, return MultiError + if hasErr { + return nil, multiErr + } + + token, err := auth.AccessToken(ctx) + if err != nil { + c.logger.ErrorContext(ctx, "failed to get access token", "error", err) + return nil, fmt.Errorf("failed to get access token: %w", err) + } + reqBody := map[string]any{ "mode": "NON_TRANSACTIONAL", "mutations": mutations, @@ -333,28 +410,27 @@ func (c *Client) PutMulti(ctx context.Context, keys []*Key, src any) ([]*Key, er } // DeleteMulti deletes multiple entities with their keys. +// Returns MultiError if any keys are invalid. // This matches the API of cloud.google.com/go/datastore. func (c *Client) DeleteMulti(ctx context.Context, keys []*Key) error { ctx = c.withClientConfig(ctx) if len(keys) == 0 { - c.logger.WarnContext(ctx, "DeleteMulti called with no keys") - return errors.New("keys cannot be empty") + return nil } c.logger.DebugContext(ctx, "deleting multiple entities", "count", len(keys)) - token, err := auth.AccessToken(ctx) - if err != nil { - c.logger.ErrorContext(ctx, "failed to get access token", "error", err) - return fmt.Errorf("failed to get access token: %w", err) - } - - // Build mutations + // Validate keys upfront + multiErr := make(MultiError, len(keys)) + hasErr := false mutations := make([]map[string]any, len(keys)) + for i, key := range keys { if key == nil { c.logger.WarnContext(ctx, "DeleteMulti called with nil key", "index", i) - return fmt.Errorf("key at index %d cannot be nil", i) + multiErr[i] = fmt.Errorf("%w: key at index %d cannot be nil", ErrInvalidKey, i) + hasErr = true + continue } mutations[i] = map[string]any{ @@ -362,6 +438,17 @@ func (c *Client) DeleteMulti(ctx context.Context, keys []*Key) error { } } + // If validation failed, return MultiError + if hasErr { + return multiErr + } + + token, err := auth.AccessToken(ctx) + if err != nil { + c.logger.ErrorContext(ctx, "failed to get access token", "error", err) + return fmt.Errorf("failed to get access token: %w", err) + } + reqBody := map[string]any{ "mode": "NON_TRANSACTIONAL", "mutations": mutations, diff --git a/pkg/datastore/operations_delete_test.go b/pkg/datastore/operations_delete_test.go index 55923ac..70a7fab 100644 --- a/pkg/datastore/operations_delete_test.go +++ b/pkg/datastore/operations_delete_test.go @@ -80,8 +80,19 @@ func TestMultiDelete(t *testing.T) { // Verify they're gone by trying to get them var retrieved []testEntity err = client.GetMulti(ctx, keys, &retrieved) - if !errors.Is(err, datastore.ErrNoSuchEntity) { - t.Errorf("expected datastore.ErrNoSuchEntity after delete, got %v", err) + if err == nil { + t.Error("expected error after delete, got nil") + } + // Should get MultiError with all ErrNoSuchEntity + var multiErr datastore.MultiError + if !errors.As(err, &multiErr) { + t.Errorf("expected MultiError after delete, got %T", err) + } else { + for i, e := range multiErr { + if !errors.Is(e, datastore.ErrNoSuchEntity) { + t.Errorf("expected ErrNoSuchEntity at index %d, got %v", i, e) + } + } } } @@ -94,8 +105,9 @@ func TestMultiDeleteEmptyKeys(t *testing.T) { var keys []*datastore.Key err := client.DeleteMulti(ctx, keys) - if err == nil { - t.Error("expected error for empty keys, got nil") + // Empty keys should return nil (matches official API) + if err != nil { + t.Errorf("expected nil for empty keys, got %v", err) } } @@ -134,10 +146,10 @@ func TestMultiDeleteEmptySlice(t *testing.T) { ctx := context.Background() - // Call MultiDelete with empty slice - should return error + // Call MultiDelete with empty slice - should return nil (matches official API) err := client.DeleteMulti(ctx, []*datastore.Key{}) - if err == nil { - t.Error("expected error for MultiDelete with empty keys, got nil") + if err != nil { + t.Errorf("expected nil for empty keys, got %v", err) } } @@ -387,8 +399,9 @@ func TestDeleteMultiEmptySlice(t *testing.T) { ctx := context.Background() err := client.DeleteMulti(ctx, []*datastore.Key{}) - if err == nil { - t.Error("expected error for empty keys") + // Empty keys should return nil (matches official API) + if err != nil { + t.Errorf("expected nil for empty keys, got %v", err) } } @@ -524,8 +537,19 @@ func TestDeleteMultiPartialSuccess(t *testing.T) { // Verify deletion var retrieved []testEntity err = client.GetMulti(ctx, keys, &retrieved) - if !errors.Is(err, datastore.ErrNoSuchEntity) { - t.Errorf("expected datastore.ErrNoSuchEntity after delete, got: %v", err) + if err == nil { + t.Error("expected error after delete, got nil") + } + // Should get MultiError with all ErrNoSuchEntity + var multiErr datastore.MultiError + if !errors.As(err, &multiErr) { + t.Errorf("expected MultiError after delete, got %T", err) + } else { + for i, e := range multiErr { + if !errors.Is(e, datastore.ErrNoSuchEntity) { + t.Errorf("expected ErrNoSuchEntity at index %d, got %v", i, e) + } + } } } diff --git a/pkg/datastore/operations_get_test.go b/pkg/datastore/operations_get_test.go index c341436..df58dd8 100644 --- a/pkg/datastore/operations_get_test.go +++ b/pkg/datastore/operations_get_test.go @@ -50,8 +50,13 @@ func TestMultiGetNotFound(t *testing.T) { var retrieved []testEntity err = client.GetMulti(ctx, keys, &retrieved) - if !errors.Is(err, datastore.ErrNoSuchEntity) { - t.Errorf("expected datastore.ErrNoSuchEntity when some keys missing, got %v", err) + // Should get MultiError with some ErrNoSuchEntity + var multiErr datastore.MultiError + if !errors.As(err, &multiErr) { + t.Fatalf("expected MultiError when some keys missing, got %T: %v", err, err) + } + if !errors.Is(multiErr[1], datastore.ErrNoSuchEntity) { + t.Errorf("expected ErrNoSuchEntity at index 1, got %v", multiErr[1]) } } @@ -213,7 +218,8 @@ func TestMultiGetWithDatabaseID(t *testing.T) { var entities []testEntity err = client.GetMulti(ctx, keys, &entities) // Expect error since entities don't exist - if !errors.Is(err, datastore.ErrNoSuchEntity) { + var multiErr datastore.MultiError + if !errors.As(err, &multiErr) { t.Errorf("expected datastore.ErrNoSuchEntity, got: %v", err) } } @@ -273,7 +279,8 @@ func TestGetMultiMixedResults(t *testing.T) { var retrieved []testEntity err = client.GetMulti(ctx, keys, &retrieved) - if !errors.Is(err, datastore.ErrNoSuchEntity) { + var multiErr datastore.MultiError + if !errors.As(err, &multiErr) { t.Errorf("expected datastore.ErrNoSuchEntity for mixed results, got: %v", err) } } @@ -293,7 +300,8 @@ func TestGetMultiAllMissing(t *testing.T) { var entities []testEntity err := client.GetMulti(ctx, keys, &entities) - if !errors.Is(err, datastore.ErrNoSuchEntity) { + var multiErr datastore.MultiError + if !errors.As(err, &multiErr) { t.Errorf("expected datastore.ErrNoSuchEntity when all keys missing, got: %v", err) } } @@ -451,7 +459,8 @@ func TestGetMultiWithNilInResults(t *testing.T) { var entities []testEntity err = client.GetMulti(ctx, keys, &entities) - if !errors.Is(err, datastore.ErrNoSuchEntity) { + var multiErr datastore.MultiError + if !errors.As(err, &multiErr) { t.Errorf("expected datastore.ErrNoSuchEntity when some keys missing, got: %v", err) } } diff --git a/pkg/datastore/operations_misc_test.go b/pkg/datastore/operations_misc_test.go index 777e2f2..9509fac 100644 --- a/pkg/datastore/operations_misc_test.go +++ b/pkg/datastore/operations_misc_test.go @@ -213,8 +213,19 @@ func TestLargeEntityBatch(t *testing.T) { // Verify deletion var retrieved2 []testEntity err = client.GetMulti(ctx, keys, &retrieved2) - if !errors.Is(err, datastore.ErrNoSuchEntity) { - t.Errorf("expected datastore.ErrNoSuchEntity after batch delete, got %v", err) + if err == nil { + t.Fatal("expected error after batch delete, got nil") + } + + var multiErr datastore.MultiError + if !errors.As(err, &multiErr) { + t.Fatalf("expected MultiError after batch delete, got %T: %v", err, err) + } + + for i, e := range multiErr { + if !errors.Is(e, datastore.ErrNoSuchEntity) { + t.Errorf("expected ErrNoSuchEntity at index %d, got %v", i, e) + } } } diff --git a/pkg/datastore/operations_put_test.go b/pkg/datastore/operations_put_test.go index 7f36ff8..325e297 100644 --- a/pkg/datastore/operations_put_test.go +++ b/pkg/datastore/operations_put_test.go @@ -146,8 +146,9 @@ func TestMultiPutEmptyKeys(t *testing.T) { var keys []*datastore.Key _, err := client.PutMulti(ctx, keys, entities) - if err == nil { - t.Error("expected error for empty keys, got nil") + // Empty keys should return nil (matches official API) + if err != nil { + t.Errorf("expected nil for empty keys, got %v", err) } } @@ -213,10 +214,10 @@ func TestMultiPutEmptySlices(t *testing.T) { ctx := context.Background() - // Call MultiPut with empty slices - should return error + // Call MultiPut with empty slices - should return nil (matches official API) _, err := client.PutMulti(ctx, []*datastore.Key{}, []testEntity{}) - if err == nil { - t.Error("expected error for MultiPut with empty keys, got nil") + if err != nil { + t.Errorf("expected nil for empty keys, got %v", err) } } @@ -304,10 +305,10 @@ func TestPutMultiEmptySlice(t *testing.T) { ctx := context.Background() - // Empty slices + // Empty slices - should return nil (matches official API) _, err := client.PutMulti(ctx, []*datastore.Key{}, []testEntity{}) - if err == nil { - t.Error("expected error for empty slices") + if err != nil { + t.Errorf("expected nil for empty slices, got %v", err) } } diff --git a/pkg/datastore/query.go b/pkg/datastore/query.go index 7cc648b..ada56ad 100644 --- a/pkg/datastore/query.go +++ b/pkg/datastore/query.go @@ -434,7 +434,7 @@ func (c *Client) GetAll(ctx context.Context, query *Query, dst any) ([]*Key, err // Verify dst is a pointer to slice v := reflect.ValueOf(dst) if v.Kind() != reflect.Ptr || v.Elem().Kind() != reflect.Slice { - return nil, errors.New("dst must be a pointer to slice") + return nil, fmt.Errorf("%w: dst must be a pointer to slice", ErrInvalidEntityType) } sliceType := v.Elem().Type() diff --git a/pkg/datastore/query_coverage_test.go b/pkg/datastore/query_coverage_test.go index 139111e..2ab9b09 100644 --- a/pkg/datastore/query_coverage_test.go +++ b/pkg/datastore/query_coverage_test.go @@ -193,8 +193,8 @@ func TestGetAll_Coverage(t *testing.T) { if err == nil { t.Error("Expected error for invalid dst, got nil") } - if !errors.Is(err, errors.New("dst must be a pointer to slice")) && err.Error() != "dst must be a pointer to slice" { - t.Errorf("Expected 'dst must be a pointer to slice' error, got: %v", err) + if !errors.Is(err, datastore.ErrInvalidEntityType) { + t.Errorf("Expected ErrInvalidEntityType, got: %v", err) } }) diff --git a/pkg/datastore/transaction.go b/pkg/datastore/transaction.go index ad6a679..93a5a51 100644 --- a/pkg/datastore/transaction.go +++ b/pkg/datastore/transaction.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "io" "net/http" @@ -302,7 +301,7 @@ func (c *Client) RunInTransaction(ctx context.Context, f func(*Transaction) erro // API compatible with cloud.google.com/go/datastore. func (tx *Transaction) Get(key *Key, dst any) error { if key == nil { - return errors.New("key cannot be nil") + return ErrInvalidKey } token, err := auth.AccessToken(tx.ctx) @@ -387,7 +386,7 @@ func (tx *Transaction) Get(key *Key, dst any) error { // Put stores an entity within the transaction. func (tx *Transaction) Put(key *Key, src any) (*Key, error) { if key == nil { - return nil, errors.New("key cannot be nil") + return nil, ErrInvalidKey } // Encode the entity @@ -411,7 +410,7 @@ func (tx *Transaction) Put(key *Key, src any) (*Key, error) { // API compatible with cloud.google.com/go/datastore. func (tx *Transaction) Delete(key *Key) error { if key == nil { - return errors.New("key cannot be nil") + return ErrInvalidKey } // Create delete mutation @@ -441,7 +440,7 @@ func (tx *Transaction) DeleteMulti(keys []*Key) error { func (tx *Transaction) GetMulti(keys []*Key, dst any) error { dstVal := reflect.ValueOf(dst) if dstVal.Kind() != reflect.Ptr || dstVal.Elem().Kind() != reflect.Slice { - return errors.New("dst must be a pointer to a slice") + return fmt.Errorf("%w: dst must be a pointer to a slice", ErrInvalidEntityType) } slice := dstVal.Elem() @@ -476,7 +475,7 @@ func (tx *Transaction) GetMulti(keys []*Key, dst any) error { func (tx *Transaction) PutMulti(keys []*Key, src any) ([]*Key, error) { srcVal := reflect.ValueOf(src) if srcVal.Kind() != reflect.Slice { - return nil, errors.New("src must be a slice") + return nil, fmt.Errorf("%w: src must be a slice", ErrInvalidEntityType) } if len(keys) != srcVal.Len() { diff --git a/pkg/mock/mock_test.go b/pkg/mock/mock_test.go index 8403778..984cfe4 100644 --- a/pkg/mock/mock_test.go +++ b/pkg/mock/mock_test.go @@ -107,7 +107,7 @@ func TestMockMultiOperations(t *testing.T) { } // Test GetMulti - var retrieved []TestEntity + retrieved := make([]TestEntity, len(keys)) err = client.GetMulti(ctx, keys, &retrieved) if err != nil { t.Fatalf("GetMulti failed: %v", err) @@ -133,6 +133,7 @@ func TestMockMultiOperations(t *testing.T) { } // Verify all deleted + retrieved = make([]TestEntity, len(keys)) err = client.GetMulti(ctx, keys, &retrieved) if err == nil { t.Error("expected error after DeleteMulti, got nil") From a9de5fa492ad938b1f7bcccc47c904963e227f45 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Thu, 20 Nov 2025 10:07:38 -0500 Subject: [PATCH 2/2] simplify tests --- .github/workflows/test.yml | 5 +---- Makefile | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 979027d..6caf080 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -9,9 +9,6 @@ on: jobs: test: runs-on: ubuntu-latest - strategy: - matrix: - go-version: [ '1.22', '1.23' ] steps: - uses: actions/checkout@v4 @@ -19,7 +16,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: ${{ matrix.go-version }} + go-version: '1.25' - name: Run tests run: make test diff --git a/Makefile b/Makefile index d27e066..056892e 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ .PHONY: test integration lint build clean test: - go test -v -race -cover ./... + go test -race -cover ./... DS9_TEST_PROJECT ?= integration-testing-476513 DS9_TEST_DATABASE ?= ds9-test