From 8b93cc081e5a1f262595945a20c99b8915e1f87f Mon Sep 17 00:00:00 2001 From: pgautier404 Date: Tue, 28 Feb 2023 16:48:21 -0800 Subject: [PATCH] chore: audit and test for nils (#206) * chore: audit request structs for nils and bad defaults * audited dictionaries * audited remaining request types * fix request property name * add pubsub nil value check in topic publish * don't precompute error responses --- momento/dictionary_test.go | 99 ++++++++++-- momento/list_test.go | 25 ++- momento/pubsub_test.go | 12 +- momento/requester.go | 75 ++++++--- momento/scalar_test.go | 32 ++++ momento/set_test.go | 217 +++++++++++++++++--------- momento/simple_cache_client.go | 29 ++++ momento/sorted_set_get_rank.go | 7 +- momento/sorted_set_increment_score.go | 7 +- momento/sorted_set_test.go | 94 ++++++++++- 10 files changed, 486 insertions(+), 111 deletions(-) diff --git a/momento/dictionary_test.go b/momento/dictionary_test.go index 0958535e..43bb8e72 100644 --- a/momento/dictionary_test.go +++ b/momento/dictionary_test.go @@ -96,6 +96,8 @@ var _ = Describe("Dictionary methods", func() { Entry("nonexistent cache name", uuid.NewString(), uuid.NewString(), NotFoundError), Entry("empty cache name", "", sharedContext.CollectionName, InvalidArgumentError), Entry("empty dictionary name", sharedContext.CacheName, "", InvalidArgumentError), + Entry("nil dictionary name", sharedContext.CacheName, nil, InvalidArgumentError), + Entry("nil cache name", nil, sharedContext.CollectionName, InvalidArgumentError), ) DescribeTable("add string and bytes value for single field happy path", @@ -129,16 +131,22 @@ var _ = Describe("Dictionary methods", func() { Entry("using bytes value and field", Bytes("myField"), Bytes("myValue"), "myField", []byte("myField"), "myValue", []byte("myValue")), ) - It("returns an error for set field when field is empty", func() { - Expect( - sharedContext.Client.DictionarySetField(sharedContext.Ctx, &DictionarySetFieldRequest{ - CacheName: sharedContext.CacheName, - DictionaryName: sharedContext.CollectionName, - Field: String(""), - Value: String("myValue"), - }), - ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) - }) + DescribeTable("try using empty and nil fields and values for set", + func(field Value, value Value) { + Expect( + sharedContext.Client.DictionarySetField(sharedContext.Ctx, &DictionarySetFieldRequest{ + CacheName: sharedContext.CacheName, + DictionaryName: sharedContext.CollectionName, + Field: field, + Value: value, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }, + Entry("empty field", String(""), String("value")), + Entry("nil field", nil, String("value")), + Entry("nil value", String("field"), nil), + Entry("both nil", nil, nil), + ) DescribeTable("add string fields and string and bytes values for set fields happy path", func(elements map[string]Value, expectedItemsStringValue map[string]string, expectedItemsByteValue map[string][]byte) { @@ -193,6 +201,16 @@ var _ = Describe("Dictionary methods", func() { ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) }) + It("returns an error if an item value is nil", func() { + Expect( + sharedContext.Client.DictionarySetFields(sharedContext.Ctx, &DictionarySetFieldsRequest{ + CacheName: sharedContext.CacheName, + DictionaryName: sharedContext.CollectionName, + Elements: map[string]Value{"myField": String("myValue"), "myOtherField": nil}, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + Describe("dictionary increment", func() { It("populates nonexistent field", func() { @@ -266,6 +284,17 @@ var _ = Describe("Dictionary methods", func() { Fail("expected a hit for get field but got a miss") } }) + + It("returns an error when field is nil", func() { + Expect( + sharedContext.Client.DictionaryIncrement(sharedContext.Ctx, &DictionaryIncrementRequest{ + CacheName: sharedContext.CacheName, + DictionaryName: sharedContext.CollectionName, + Field: nil, + Amount: 1, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) }) Describe("dictionary get", func() { @@ -325,6 +354,16 @@ var _ = Describe("Dictionary methods", func() { Expect(getResp).To(BeAssignableToTypeOf(&DictionaryGetFieldMiss{})) }) + It("returns an error for a nil field", func() { + Expect( + sharedContext.Client.DictionaryGetField(sharedContext.Ctx, &DictionaryGetFieldRequest{ + CacheName: sharedContext.CacheName, + DictionaryName: sharedContext.CollectionName, + Field: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + }) When("getting multiple fields", func() { @@ -393,6 +432,16 @@ var _ = Describe("Dictionary methods", func() { } }) + It("returns an error for a nil field", func() { + Expect( + sharedContext.Client.DictionaryGetFields(sharedContext.Ctx, &DictionaryGetFieldsRequest{ + CacheName: sharedContext.CacheName, + DictionaryName: sharedContext.CollectionName, + Fields: []Value{String("myField"), nil}, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + }) }) @@ -496,6 +545,16 @@ var _ = Describe("Dictionary methods", func() { Expect(removeResp).To(BeAssignableToTypeOf(&DictionaryRemoveFieldSuccess{})) }) + It("returns an error when trying to remove a nil field", func() { + Expect( + sharedContext.Client.DictionaryRemoveField(sharedContext.Ctx, &DictionaryRemoveFieldRequest{ + CacheName: sharedContext.CacheName, + DictionaryName: sharedContext.CollectionName, + Field: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + }) When("removing multiple fields", func() { @@ -544,6 +603,26 @@ var _ = Describe("Dictionary methods", func() { Expect(removeResp).To(BeAssignableToTypeOf(&DictionaryRemoveFieldsSuccess{})) }) + It("returns an error when Fields is nil", func() { + Expect( + sharedContext.Client.DictionaryRemoveFields(sharedContext.Ctx, &DictionaryRemoveFieldsRequest{ + CacheName: sharedContext.CacheName, + DictionaryName: sharedContext.CollectionName, + Fields: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + + It("returns an error when one field is nil", func() { + Expect( + sharedContext.Client.DictionaryRemoveFields(sharedContext.Ctx, &DictionaryRemoveFieldsRequest{ + CacheName: sharedContext.CacheName, + DictionaryName: sharedContext.CollectionName, + Fields: []Value{String("myField"), nil, String("myField2")}, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + }) }) diff --git a/momento/list_test.go b/momento/list_test.go index de12f954..a843706d 100644 --- a/momento/list_test.go +++ b/momento/list_test.go @@ -312,7 +312,7 @@ var _ = Describe("List methods", func() { It("returns an invalid argument for a nil value", func() { populateList(sharedContext, 5) - concatValues := []Value{nil, nil} + concatValues := []Value{nil, String("aRealValue"), nil} Expect( sharedContext.Client.ListConcatenateFront(sharedContext.Ctx, &ListConcatenateFrontRequest{ CacheName: sharedContext.CacheName, @@ -382,7 +382,7 @@ var _ = Describe("List methods", func() { It("returns an invalid argument for a nil value", func() { populateList(sharedContext, 5) - concatValues := []Value{nil, nil} + concatValues := []Value{nil, String("aRealValue"), nil} Expect( sharedContext.Client.ListConcatenateBack(sharedContext.Ctx, &ListConcatenateBackRequest{ CacheName: sharedContext.CacheName, @@ -523,6 +523,16 @@ var _ = Describe("List methods", func() { } }) + It("returns an error for a nil value", func() { + Expect( + sharedContext.Client.ListRemoveValue(sharedContext.Ctx, &ListRemoveValueRequest{ + CacheName: sharedContext.CacheName, + ListName: sharedContext.CollectionName, + Value: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + }) When("removing a value that appears multiple times", func() { @@ -560,6 +570,16 @@ var _ = Describe("List methods", func() { } }) + It("returns an error for a nil value", func() { + Expect( + sharedContext.Client.ListRemoveValue(sharedContext.Ctx, &ListRemoveValueRequest{ + CacheName: sharedContext.CacheName, + ListName: sharedContext.CollectionName, + Value: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + }) When("removing a value that isn't in the list", func() { @@ -597,6 +617,7 @@ var _ = Describe("List methods", func() { ).To(BeAssignableToTypeOf(&ListRemoveValueSuccess{})) }) + }) }) diff --git a/momento/pubsub_test.go b/momento/pubsub_test.go index 7acc6b90..0a2ce81a 100644 --- a/momento/pubsub_test.go +++ b/momento/pubsub_test.go @@ -64,7 +64,7 @@ var _ = Describe("Pubsub", func() { } cancelContext, cancelFunction := context.WithCancel(sharedContext.Ctx) - receivedValues := []TopicValue{} + var receivedValues []TopicValue ready := make(chan int, 1) go func() { ready <- 1 @@ -100,6 +100,16 @@ var _ = Describe("Pubsub", func() { Expect(receivedValues).To(Equal(publishedValues)) }) + It("returns an error when trying to publish a nil topic value", func() { + Expect( + sharedContext.Client.TopicPublish(sharedContext.Ctx, &TopicPublishRequest{ + CacheName: sharedContext.CacheName, + TopicName: sharedContext.CollectionName, + Value: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + Describe(`TopicSubscribe`, func() { It(`Does not error on a non-existent topic`, func() { Expect( diff --git a/momento/requester.go b/momento/requester.go index d551287d..a8846491 100644 --- a/momento/requester.go +++ b/momento/requester.go @@ -70,29 +70,59 @@ type hasTTL interface { ttl() time.Duration } +func buildError(errorCode string, errorMessage string, originalError error) MomentoError { + return convertMomentoSvcErrorToCustomerError( + momentoerrors.NewMomentoSvcErr(errorCode, errorMessage, originalError), + ) +} + func prepareName(name string, label string) (string, error) { if len(strings.TrimSpace(name)) < 1 { errStr := fmt.Sprintf("%v cannot be empty", label) - return "", momentoerrors.NewMomentoSvcErr(momentoerrors.InvalidArgumentError, errStr, nil) + return "", buildError(momentoerrors.InvalidArgumentError, errStr, nil) } return name, nil } +func prepareElementName(name Value) ([]byte, error) { + if name == nil { + return nil, buildError( + momentoerrors.InvalidArgumentError, "element name cannot be nil or empty", nil, + ) + } + + // just validate not empty using prepareName + nameBytes := name.asBytes() + _, err := prepareName(string(nameBytes), "element name") + if err != nil { + return nil, err + } + + return nameBytes, nil +} + func prepareCacheName(r hasCacheName) (string, error) { return prepareName(r.cacheName(), "Cache name") } func prepareKey(r hasKey) ([]byte, error) { - key := r.key().asBytes() + if r.key() == nil { + return nil, buildError(momentoerrors.InvalidArgumentError, "key cannot be nil or empty", nil) + } + key := r.key().asBytes() if len(key) == 0 { - err := momentoerrors.NewMomentoSvcErr(momentoerrors.InvalidArgumentError, "key cannot be empty", nil) - return nil, convertMomentoSvcErrorToCustomerError(err) + return nil, buildError(momentoerrors.InvalidArgumentError, "key cannot be nil or empty", nil) } return key, nil } func prepareField(r hasField) ([]byte, error) { + if r.field() == nil { + return nil, buildError( + momentoerrors.InvalidArgumentError, "field cannot be nil or empty", nil, + ) + } field := r.field().asBytes() if err := validateNotEmpty(field, "field"); err != nil { return nil, err @@ -101,11 +131,17 @@ func prepareField(r hasField) ([]byte, error) { } func prepareFields(r hasFields) ([][]byte, error) { + if r.fields() == nil { + return nil, buildError(momentoerrors.InvalidArgumentError, "fields cannot be nil or empty", nil) + } var fields [][]byte for _, valueField := range r.fields() { + if valueField == nil { + return nil, buildError(momentoerrors.InvalidArgumentError, "fields cannot be nil or empty", nil) + } field := valueField.asBytes() if err := validateNotEmpty(field, "field"); err != nil { - return nil, err + return nil, buildError(momentoerrors.InvalidArgumentError, "fields cannot be nil or empty", nil) } fields = append(fields, field) } @@ -114,10 +150,8 @@ func prepareFields(r hasFields) ([][]byte, error) { func prepareValue(r hasValue) ([]byte, momentoerrors.MomentoSvcErr) { if r.value() == nil { - return []byte{}, momentoerrors.NewMomentoSvcErr( - momentoerrors.InvalidArgumentError, - "value may not be nil", - nil, + return []byte{}, buildError( + momentoerrors.InvalidArgumentError, "value may not be nil", nil, ) } return r.value().asBytes(), nil @@ -135,10 +169,8 @@ func prepareElements(r hasElements) (map[string][]byte, error) { retMap := make(map[string][]byte) for k, v := range r.elements() { if v == nil { - return map[string][]byte{}, momentoerrors.NewMomentoSvcErr( - momentoerrors.InvalidArgumentError, - "item values may not be nil", - nil, + return map[string][]byte{}, buildError( + momentoerrors.InvalidArgumentError, "item values may not be nil", nil, ) } if err := validateNotEmpty([]byte(k), "item keys"); err != nil { @@ -155,24 +187,21 @@ func prepareTTL(r hasTTL, defaultTtl time.Duration) (uint64, error) { ttl = defaultTtl } if ttl <= time.Duration(0) { - return 0, momentoerrors.NewMomentoSvcErr( - momentoerrors.InvalidArgumentError, - "ttl must be a non-zero positive value", - nil, + return 0, buildError( + momentoerrors.InvalidArgumentError, "ttl must be a non-zero positive value", nil, ) } return uint64(ttl.Milliseconds()), nil } func momentoValuesToPrimitiveByteList(i []Value) ([][]byte, momentoerrors.MomentoSvcErr) { + if i == nil { + return [][]byte{}, buildError(momentoerrors.InvalidArgumentError, "values may not be nil", nil) + } var rList [][]byte for _, mb := range i { if mb == nil { - return [][]byte{}, momentoerrors.NewMomentoSvcErr( - momentoerrors.InvalidArgumentError, - "values may not be nil", - nil, - ) + return [][]byte{}, buildError(momentoerrors.InvalidArgumentError, "values may not be nil", nil) } rList = append(rList, mb.asBytes()) } @@ -181,7 +210,7 @@ func momentoValuesToPrimitiveByteList(i []Value) ([][]byte, momentoerrors.Moment func validateNotEmpty(field []byte, label string) error { if len(field) == 0 { - return momentoerrors.NewMomentoSvcErr( + return buildError( momentoerrors.InvalidArgumentError, fmt.Sprintf("%s cannot be empty", label), nil, ) } diff --git a/momento/scalar_test.go b/momento/scalar_test.go index 1de6f2c9..1dd5ee29 100644 --- a/momento/scalar_test.go +++ b/momento/scalar_test.go @@ -92,6 +92,27 @@ var _ = Describe("Scalar methods", func() { Expect(err).To(HaveMomentoErrorCode(NotFoundError)) }) + It(`errors when the key is nil`, func() { + Expect( + sharedContext.Client.Get(sharedContext.Ctx, &GetRequest{ + CacheName: sharedContext.CacheName, + Key: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + Expect( + sharedContext.Client.Set(sharedContext.Ctx, &SetRequest{ + CacheName: sharedContext.CacheName, + Key: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + Expect( + sharedContext.Client.Delete(sharedContext.Ctx, &DeleteRequest{ + CacheName: sharedContext.CacheName, + Key: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + DescribeTable(`invalid cache names and keys`, func(cacheName string, key Key, value Key) { getResp, err := sharedContext.Client.Get(sharedContext.Ctx, &GetRequest{ @@ -193,5 +214,16 @@ var _ = Describe("Scalar methods", func() { }), ).To(BeAssignableToTypeOf(&GetMiss{})) }) + + It("returns an error for a nil value", func() { + key := String("key") + Expect( + sharedContext.Client.Set(sharedContext.Ctx, &SetRequest{ + CacheName: sharedContext.CacheName, + Key: key, + Value: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) }) }) diff --git a/momento/set_test.go b/momento/set_test.go index afc88957..d8c50c1b 100644 --- a/momento/set_test.go +++ b/momento/set_test.go @@ -66,14 +66,6 @@ var _ = Describe("Set methods", func() { Element: String("astring"), }), ).Error().To(HaveMomentoErrorCode(NotFoundError)) - - Expect( - sharedContext.Client.SetRemoveElements(sharedContext.Ctx, &SetRemoveElementsRequest{ - CacheName: cacheName, - SetName: setName, - Elements: nil, - }), - ).Error().To(HaveMomentoErrorCode(NotFoundError)) }) It("errors on invalid set name", func() { @@ -85,6 +77,30 @@ var _ = Describe("Set methods", func() { }), ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + Expect( + sharedContext.Client.SetAddElement(sharedContext.Ctx, &SetAddElementRequest{ + CacheName: sharedContext.CacheName, + SetName: setName, + Element: String("hi"), + }), + ) + + Expect( + sharedContext.Client.SetAddElements(sharedContext.Ctx, &SetAddElementsRequest{ + CacheName: sharedContext.CacheName, + SetName: setName, + Elements: []Value{String("hi")}, + }), + ) + + Expect( + sharedContext.Client.SetRemoveElement(sharedContext.Ctx, &SetRemoveElementRequest{ + CacheName: sharedContext.CacheName, + SetName: setName, + Element: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + Expect( sharedContext.Client.SetRemoveElements(sharedContext.Ctx, &SetRemoveElementsRequest{ CacheName: sharedContext.CacheName, @@ -94,81 +110,110 @@ var _ = Describe("Set methods", func() { ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) }) - DescribeTable("add string and byte single elements happy path", - func(element Value, expectedStrings []string, expectedBytes [][]byte) { + Describe("add", func() { + DescribeTable("add string and byte single elements happy path", + func(element Value, expectedStrings []string, expectedBytes [][]byte) { + Expect( + sharedContext.Client.SetAddElement(sharedContext.Ctx, &SetAddElementRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + Element: element, + }), + ).To(BeAssignableToTypeOf(&SetAddElementSuccess{})) + + fetchResp, err := sharedContext.Client.SetFetch(sharedContext.Ctx, &SetFetchRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + }) + Expect(err).To(BeNil()) + switch result := fetchResp.(type) { + case *SetFetchHit: + Expect(result.ValueString()).To(Equal(expectedStrings)) + Expect(result.ValueByte()).To(Equal(expectedBytes)) + default: + Fail("Unexpected result for Set Fetch") + } + }, + Entry("when element is a string", String("hello"), []string{"hello"}, [][]byte{[]byte("hello")}), + Entry("when element is bytes", Bytes("hello"), []string{"hello"}, [][]byte{[]byte("hello")}), + Entry("when element is a empty", String(""), []string{""}, [][]byte{[]byte("")}), + ) + + DescribeTable("add string and byte multiple elements happy path", + func(elements []Value, expectedStrings []string, expectedBytes [][]byte) { + Expect( + sharedContext.Client.SetAddElements(sharedContext.Ctx, &SetAddElementsRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + Elements: elements, + }), + ).To(BeAssignableToTypeOf(&SetAddElementsSuccess{})) + fetchResp, err := sharedContext.Client.SetFetch(sharedContext.Ctx, &SetFetchRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + }) + Expect(err).To(BeNil()) + switch result := fetchResp.(type) { + case *SetFetchHit: + Expect(result.ValueString()).To(ConsistOf(expectedStrings)) + Expect(result.ValueByte()).To(ConsistOf(expectedBytes)) + default: + Fail("Unexpected results for Set Fetch") + } + }, + Entry( + "when elements are strings", + []Value{String("hello"), String("world"), String("!"), String("␆")}, + []string{"hello", "world", "!", "␆"}, + [][]byte{[]byte("hello"), []byte("world"), []byte("!"), []byte("␆")}, + ), + Entry( + "when elements are bytes", + []Value{Bytes([]byte("hello")), Bytes([]byte("world")), Bytes([]byte("!")), Bytes([]byte("␆"))}, + []string{"hello", "world", "!", "␆"}, + [][]byte{[]byte("hello"), []byte("world"), []byte("!"), []byte("␆")}, + ), + Entry( + "when elements are mixed", + []Value{Bytes([]byte("hello")), String([]byte("world")), Bytes([]byte("!")), String([]byte("␆"))}, + []string{"hello", "world", "!", "␆"}, + [][]byte{[]byte("hello"), []byte("world"), []byte("!"), []byte("␆")}, + ), + Entry( + "when elements are empty", + []Value{Bytes([]byte("")), Bytes([]byte(""))}, + []string{""}, + [][]byte{[]byte("")}, + ), + ) + + It("returns an error when trying to add nil elements", func() { Expect( sharedContext.Client.SetAddElement(sharedContext.Ctx, &SetAddElementRequest{ CacheName: sharedContext.CacheName, SetName: sharedContext.CollectionName, - Element: element, + Element: nil, }), - ).To(BeAssignableToTypeOf(&SetAddElementSuccess{})) + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) - fetchResp, err := sharedContext.Client.SetFetch(sharedContext.Ctx, &SetFetchRequest{ - CacheName: sharedContext.CacheName, - SetName: sharedContext.CollectionName, - }) - Expect(err).To(BeNil()) - switch result := fetchResp.(type) { - case *SetFetchHit: - Expect(result.ValueString()).To(Equal(expectedStrings)) - Expect(result.ValueByte()).To(Equal(expectedBytes)) - default: - Fail("Unexpected result for Set Fetch") - } - }, - Entry("when element is a string", String("hello"), []string{"hello"}, [][]byte{[]byte("hello")}), - Entry("when element is bytes", Bytes("hello"), []string{"hello"}, [][]byte{[]byte("hello")}), - Entry("when element is a empty", String(""), []string{""}, [][]byte{[]byte("")}), - ) - - DescribeTable("add string and byte multiple elements happy path", - func(elements []Value, expectedStrings []string, expectedBytes [][]byte) { Expect( sharedContext.Client.SetAddElements(sharedContext.Ctx, &SetAddElementsRequest{ CacheName: sharedContext.CacheName, SetName: sharedContext.CollectionName, - Elements: elements, + Elements: nil, }), - ).To(BeAssignableToTypeOf(&SetAddElementsSuccess{})) - fetchResp, err := sharedContext.Client.SetFetch(sharedContext.Ctx, &SetFetchRequest{ - CacheName: sharedContext.CacheName, - SetName: sharedContext.CollectionName, - }) - Expect(err).To(BeNil()) - switch result := fetchResp.(type) { - case *SetFetchHit: - Expect(result.ValueString()).To(ConsistOf(expectedStrings)) - Expect(result.ValueByte()).To(ConsistOf(expectedBytes)) - default: - Fail("Unexpected results for Set Fetch") - } - }, - Entry( - "when elements are strings", - []Value{String("hello"), String("world"), String("!"), String("␆")}, - []string{"hello", "world", "!", "␆"}, - [][]byte{[]byte("hello"), []byte("world"), []byte("!"), []byte("␆")}, - ), - Entry( - "when elements are bytes", - []Value{Bytes([]byte("hello")), Bytes([]byte("world")), Bytes([]byte("!")), Bytes([]byte("␆"))}, - []string{"hello", "world", "!", "␆"}, - [][]byte{[]byte("hello"), []byte("world"), []byte("!"), []byte("␆")}, - ), - Entry( - "when elements are mixed", - []Value{Bytes([]byte("hello")), String([]byte("world")), Bytes([]byte("!")), String([]byte("␆"))}, - []string{"hello", "world", "!", "␆"}, - [][]byte{[]byte("hello"), []byte("world"), []byte("!"), []byte("␆")}, - ), - Entry( - "when elements are empty", - []Value{Bytes([]byte("")), Bytes([]byte(""))}, - []string{""}, - [][]byte{[]byte("")}, - ), - ) + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + + Expect( + sharedContext.Client.SetAddElements(sharedContext.Ctx, &SetAddElementsRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + Elements: []Value{nil, String("aValue"), nil}, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + + }) Describe("remove", func() { @@ -238,6 +283,34 @@ var _ = Describe("Set methods", func() { Entry("as bytes", []Value{Bytes("#3"), Bytes("#4")}, 8), Entry("unmatched", []Value{String("notvalid")}, 10), ) + + It("returns an error when trying to remove nil elements", func() { + Expect( + sharedContext.Client.SetRemoveElement(sharedContext.Ctx, &SetRemoveElementRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + Element: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + + Expect( + sharedContext.Client.SetRemoveElements(sharedContext.Ctx, &SetRemoveElementsRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + Elements: nil, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + + Expect( + sharedContext.Client.SetRemoveElements(sharedContext.Ctx, &SetRemoveElementsRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + Elements: []Value{nil, String("aValue"), nil}, + }), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + + }) + }) Describe("using client default TTL", func() { diff --git a/momento/simple_cache_client.go b/momento/simple_cache_client.go index 0c62d298..3a0ab31e 100644 --- a/momento/simple_cache_client.go +++ b/momento/simple_cache_client.go @@ -246,6 +246,14 @@ func (c defaultScsClient) TopicPublish(ctx context.Context, request *TopicPublis return nil, err } + if request.Value == nil { + return nil, convertMomentoSvcErrorToCustomerError( + momentoerrors.NewMomentoSvcErr( + momentoerrors.InvalidArgumentError, "value cannot be nil", nil, + ), + ) + } + err := c.pubSubClient.TopicPublish(ctx, &TopicPublishRequest{ CacheName: request.CacheName, TopicName: request.TopicName, @@ -281,6 +289,13 @@ func (c defaultScsClient) SortedSetGetScore(ctx context.Context, r *SortedSetGet } func (c defaultScsClient) SortedSetRemove(ctx context.Context, r *SortedSetRemoveRequest) (SortedSetRemoveResponse, error) { + if r.ElementsToRemove == nil { + return nil, convertMomentoSvcErrorToCustomerError( + momentoerrors.NewMomentoSvcErr( + momentoerrors.InvalidArgumentError, "elements to remove cannot be nil", nil, + ), + ) + } if err := c.dataClient.makeRequest(ctx, r); err != nil { return nil, err } @@ -411,6 +426,13 @@ func (c defaultScsClient) ListRemoveValue(ctx context.Context, r *ListRemoveValu } func (c defaultScsClient) DictionarySetField(ctx context.Context, r *DictionarySetFieldRequest) (DictionarySetFieldResponse, error) { + if r.Field == nil { + return nil, convertMomentoSvcErrorToCustomerError( + momentoerrors.NewMomentoSvcErr( + momentoerrors.InvalidArgumentError, "field cannot be nil", nil, + ), + ) + } elements := make(map[string]Value) elements[string(r.Field.asBytes())] = r.Value newRequest := &DictionarySetFieldsRequest{ @@ -483,6 +505,13 @@ func (c defaultScsClient) DictionaryIncrement(ctx context.Context, r *Dictionary } func (c defaultScsClient) DictionaryRemoveField(ctx context.Context, r *DictionaryRemoveFieldRequest) (DictionaryRemoveFieldResponse, error) { + if r.Field == nil { + return nil, convertMomentoSvcErrorToCustomerError( + momentoerrors.NewMomentoSvcErr( + momentoerrors.InvalidArgumentError, "field cannot be nil", nil, + ), + ) + } newRequest := &DictionaryRemoveFieldsRequest{ CacheName: r.CacheName, DictionaryName: r.DictionaryName, diff --git a/momento/sorted_set_get_rank.go b/momento/sorted_set_get_rank.go index 203f1fa2..fa85e3db 100644 --- a/momento/sorted_set_get_rank.go +++ b/momento/sorted_set_get_rank.go @@ -47,9 +47,14 @@ func (r *SortedSetGetRankRequest) initGrpcRequest(scsDataClient) error { return err } + var elementName []byte + if elementName, err = prepareElementName(r.ElementName); err != nil { + return err + } + resp := &pb.XSortedSetGetRankRequest{ SetName: []byte(r.SetName), - ElementName: r.ElementName.asBytes(), + ElementName: elementName, } r.grpcRequest = resp diff --git a/momento/sorted_set_increment_score.go b/momento/sorted_set_increment_score.go index 7db53c13..39ed1b34 100644 --- a/momento/sorted_set_increment_score.go +++ b/momento/sorted_set_increment_score.go @@ -54,6 +54,11 @@ func (r *SortedSetIncrementScoreRequest) initGrpcRequest(client scsDataClient) e return err } + var elementName []byte + if elementName, err = prepareElementName(r.ElementName); err != nil { + return err + } + if r.Amount == 0 { return momentoerrors.NewMomentoSvcErr( momentoerrors.InvalidArgumentError, @@ -64,7 +69,7 @@ func (r *SortedSetIncrementScoreRequest) initGrpcRequest(client scsDataClient) e r.grpcRequest = &pb.XSortedSetIncrementRequest{ SetName: []byte(r.SetName), - ElementName: r.ElementName.asBytes(), + ElementName: elementName, Amount: r.Amount, TtlMilliseconds: ttlMillis, RefreshTtl: r.Ttl.RefreshTtl, diff --git a/momento/sorted_set_test.go b/momento/sorted_set_test.go index b628bcc9..4e88b43d 100644 --- a/momento/sorted_set_test.go +++ b/momento/sorted_set_test.go @@ -97,7 +97,7 @@ var _ = Describe("SortedSet", func() { Entry("Blank cache name", " ", sharedContext.CollectionName, InvalidArgumentError), Entry("Empty collection name", sharedContext.CacheName, "", InvalidArgumentError), Entry("Blank collection name", sharedContext.CacheName, " ", InvalidArgumentError), - Entry("Non-existant cache", uuid.NewString(), uuid.NewString(), NotFoundError), + Entry("Non-existent cache", uuid.NewString(), uuid.NewString(), NotFoundError), ) DescribeTable(`Honors CollectionTtl `, @@ -323,6 +323,19 @@ var _ = Describe("SortedSet", func() { ), ).To(Equal(&SortedSetGetRankHit{Rank: 0})) }) + + It(`returns an error for a nil element name`, func() { + Expect( + sharedContext.Client.SortedSetGetRank( + sharedContext.Ctx, + &SortedSetGetRankRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + ElementName: nil, + }, + ), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) }) Describe(`SortedSetGetScore`, func() { @@ -369,6 +382,30 @@ var _ = Describe("SortedSet", func() { }, )) }) + + It(`returns an error when element names are nil`, func() { + Expect( + sharedContext.Client.SortedSetGetScore( + sharedContext.Ctx, + &SortedSetGetScoreRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + ElementNames: nil, + }, + ), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + + Expect( + sharedContext.Client.SortedSetGetScore( + sharedContext.Ctx, + &SortedSetGetScoreRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + ElementNames: []Value{nil, String("aValue"), nil}, + }, + ), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) }) Describe(`SortedSetIncrementScore`, func() { @@ -446,6 +483,21 @@ var _ = Describe("SortedSet", func() { ), ).To(BeAssignableToTypeOf(&SortedSetIncrementScoreSuccess{Value: 50})) }) + + It("returns an error when element name is nil", func() { + Expect( + sharedContext.Client.SortedSetIncrementScore( + sharedContext.Ctx, + &SortedSetIncrementScoreRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + ElementName: nil, + Amount: 42, + }, + ), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + }) Describe(`SortedSetRemove`, func() { @@ -505,5 +557,45 @@ var _ = Describe("SortedSet", func() { }, )) }) + + It("returns an error when elements are nil", func() { + Expect( + sharedContext.Client.SortedSetRemove( + sharedContext.Ctx, + &SortedSetRemoveRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + ElementsToRemove: nil, + }, + ), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + + Expect( + sharedContext.Client.SortedSetRemove( + sharedContext.Ctx, + &SortedSetRemoveRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + ElementsToRemove: RemoveSomeElements{ + Elements: nil, + }, + }, + ), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + + Expect( + sharedContext.Client.SortedSetRemove( + sharedContext.Ctx, + &SortedSetRemoveRequest{ + CacheName: sharedContext.CacheName, + SetName: sharedContext.CollectionName, + ElementsToRemove: RemoveSomeElements{ + Elements: []Value{nil, String("aValue"), nil}, + }, + }, + ), + ).Error().To(HaveMomentoErrorCode(InvalidArgumentError)) + }) + }) })