Skip to content

Commit

Permalink
fix: All values can be blank. (#247)
Browse files Browse the repository at this point in the history 
* fix: Values can be blank.

We only need to validate they're not nil.

* refactor: Normalize validation.

* Extract validateNotNil
  * Except for slice types, they're hard to cram into one function.

* test: Add tests for empty values.
  • Loading branch information
@schwern
schwern authored Mar 8, 2023
1 parent d50c981 commit 1b9579b
Show file tree
Hide file tree
Showing 6 changed files with 187 additions and 64 deletions.
10 changes: 9 additions & 1 deletion momento/dictionary_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,15 @@ var _ = Describe("Dictionary methods", func() {
map[string]string{"myField1": "myValue1", "myField2": "myValue2"},
map[string][]byte{"myField1": []byte("myValue1"), "myField2": []byte("myValue2")},
),
Entry(
"with empty values",
[]DictionaryElement{
{Field: Bytes("myField1"), Value: String("")},
{Field: String("myField2"), Value: Bytes("")},
},
map[string]string{"myField1": "", "myField2": ""},
map[string][]byte{"myField1": []byte(""), "myField2": []byte("")},
),
)

It("returns an error if an item field is empty", func() {
Expand Down Expand Up @@ -635,7 +644,6 @@ var _ = Describe("Dictionary methods", func() {
})

When("removing multiple fields", func() {

It("properly removes multiple fields", func() {
removeResp, err := sharedContext.Client.DictionaryRemoveFields(sharedContext.Ctx, &DictionaryRemoveFieldsRequest{
CacheName: sharedContext.CacheName,
Expand Down
97 changes: 96 additions & 1 deletion momento/list_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,22 @@ var _ = Describe("List methods", func() {
).Error().To(HaveMomentoErrorCode(InvalidArgumentError))
})

It("accepts an empty value", func() {
Expect(
sharedContext.Client.ListPushBack(sharedContext.Ctx, &ListPushBackRequest{
CacheName: sharedContext.CacheName,
ListName: sharedContext.CollectionName,
Value: String(""),
}),
).To(BeAssignableToTypeOf(&ListPushBackSuccess{}))

fetchResp, err := sharedContext.Client.ListFetch(sharedContext.Ctx, &ListFetchRequest{
CacheName: sharedContext.CacheName,
ListName: sharedContext.CollectionName,
})
Expect(err).To(BeNil())
Expect(fetchResp).To(HaveListLength(1))
})
})

When("pushing to the back of the list", func() {
Expand Down Expand Up @@ -262,6 +278,22 @@ var _ = Describe("List methods", func() {
).Error().To(HaveMomentoErrorCode(InvalidArgumentError))
})

It("accepts an empty value", func() {
Expect(
sharedContext.Client.ListPushBack(sharedContext.Ctx, &ListPushBackRequest{
CacheName: sharedContext.CacheName,
ListName: sharedContext.CollectionName,
Value: String(""),
}),
).To(BeAssignableToTypeOf(&ListPushBackSuccess{}))

fetchResp, err := sharedContext.Client.ListFetch(sharedContext.Ctx, &ListFetchRequest{
CacheName: sharedContext.CacheName,
ListName: sharedContext.CollectionName,
})
Expect(err).To(BeNil())
Expect(fetchResp).To(HaveListLength(1))
})
})

})
Expand Down Expand Up @@ -336,6 +368,22 @@ var _ = Describe("List methods", func() {
).Error().To(HaveMomentoErrorCode(InvalidArgumentError))
})

It("accepts an empty value", func() {
Expect(
sharedContext.Client.ListConcatenateFront(sharedContext.Ctx, &ListConcatenateFrontRequest{
CacheName: sharedContext.CacheName,
ListName: sharedContext.CollectionName,
Values: []Value{String("")},
}),
).To(BeAssignableToTypeOf(&ListConcatenateFrontSuccess{}))

fetchResp, err := sharedContext.Client.ListFetch(sharedContext.Ctx, &ListFetchRequest{
CacheName: sharedContext.CacheName,
ListName: sharedContext.CollectionName,
})
Expect(err).To(BeNil())
Expect(fetchResp).To(HaveListLength(1))
})
})

When("concatenating to the back of the list", func() {
Expand Down Expand Up @@ -406,8 +454,23 @@ var _ = Describe("List methods", func() {
).Error().To(HaveMomentoErrorCode(InvalidArgumentError))
})

})
It("accepts an empty value", func() {
Expect(
sharedContext.Client.ListConcatenateBack(sharedContext.Ctx, &ListConcatenateBackRequest{
CacheName: sharedContext.CacheName,
ListName: sharedContext.CollectionName,
Values: []Value{String("")},
}),
).To(BeAssignableToTypeOf(&ListConcatenateBackSuccess{}))

fetchResp, err := sharedContext.Client.ListFetch(sharedContext.Ctx, &ListFetchRequest{
CacheName: sharedContext.CacheName,
ListName: sharedContext.CollectionName,
})
Expect(err).To(BeNil())
Expect(fetchResp).To(HaveListLength(1))
})
})
})

Describe("list pop", func() {
Expand Down Expand Up @@ -546,6 +609,38 @@ var _ = Describe("List methods", func() {
).Error().To(HaveMomentoErrorCode(InvalidArgumentError))
})

It("accepts an empty value", func() {
Expect(
sharedContext.Client.ListConcatenateFront(
sharedContext.Ctx,
&ListConcatenateFrontRequest{
CacheName: sharedContext.CacheName,
ListName: sharedContext.CollectionName,
Values: []Value{String("one"), String(""), String("three")},
},
),
).To(BeAssignableToTypeOf(&ListConcatenateFrontSuccess{}))

Expect(
sharedContext.Client.ListRemoveValue(sharedContext.Ctx, &ListRemoveValueRequest{
CacheName: sharedContext.CacheName,
ListName: sharedContext.CollectionName,
Value: String(""),
}),
).To(BeAssignableToTypeOf(&ListRemoveValueSuccess{}))

fetchResp, err := sharedContext.Client.ListFetch(sharedContext.Ctx, &ListFetchRequest{
CacheName: sharedContext.CacheName,
ListName: sharedContext.CollectionName,
})
Expect(err).To(BeNil())
switch result := fetchResp.(type) {
case *ListFetchHit:
Expect(
result.ValueListString(),
).To(Equal([]string{"one", "three"}))
}
})
})

When("removing a value that appears multiple times", func() {
Expand Down
102 changes: 42 additions & 60 deletions momento/requester.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,85 +84,54 @@ func buildError(errorCode string, errorMessage string, originalError error) Mome

func prepareName(name string, label string) (string, error) {
if len(strings.TrimSpace(name)) < 1 {
errStr := fmt.Sprintf("%v cannot be empty", label)
errStr := fmt.Sprintf("%v cannot be empty or blank", label)
return "", buildError(momentoerrors.InvalidArgumentError, errStr, nil)
}
return name, nil
}

func prepareElementValue(value Value) ([]byte, error) {
if value == nil {
return nil, buildError(
momentoerrors.InvalidArgumentError, "element value cannot be nil", nil,
)
}

// just validate not empty using prepareName
_, err := prepareName(value.asString(), "element value")
if err != nil {
return nil, err
}

return value.asBytes(), nil
}

func prepareCacheName(r hasCacheName) (string, error) {
return prepareName(r.cacheName(), "Cache name")
}

func prepareKey(r hasKey) ([]byte, error) {
if r.key() == nil {
return nil, buildError(momentoerrors.InvalidArgumentError, "key cannot be nil or empty", nil)
if err := validateNotEmpty(r.key(), "key"); err != nil {
return nil, err
}

key := r.key().asBytes()
if len(key) == 0 {
return nil, buildError(momentoerrors.InvalidArgumentError, "key cannot be nil or empty", nil)
}
return key, nil
return r.key().asBytes(), 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 {
if err := validateNotEmpty(r.field(), "field"); err != nil {
return nil, err
}
return field, nil
return r.field().asBytes(), nil
}

func prepareFields(r hasFields) ([][]byte, error) {
if r.fields() == nil {
return nil, buildError(momentoerrors.InvalidArgumentError, "fields cannot be nil or empty", nil)
return nil, buildError(InvalidArgumentError, "fields cannot be nil", 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()
for _, field := range r.fields() {
if err := validateNotEmpty(field, "field"); err != nil {
return nil, buildError(momentoerrors.InvalidArgumentError, "fields cannot be nil or empty", nil)
return nil, err
}
fields = append(fields, field)
fields = append(fields, field.asBytes())
}
return fields, nil
}

func prepareValue(r hasValue) ([]byte, momentoerrors.MomentoSvcErr) {
if r.value() == nil {
return []byte{}, buildError(
momentoerrors.InvalidArgumentError, "value may not be nil", nil,
)
func prepareValue(r hasValue) ([]byte, error) {
if err := validateNotNil(r.value(), "value"); err != nil {
return []byte{}, err
}
return r.value().asBytes(), nil
}

func prepareValues(r hasValues) ([][]byte, momentoerrors.MomentoSvcErr) {
func prepareValues(r hasValues) ([][]byte, error) {
values, err := momentoValuesToPrimitiveByteList(r.values())
if err != nil {
return [][]byte{}, err
Expand All @@ -172,12 +141,10 @@ func prepareValues(r hasValues) ([][]byte, momentoerrors.MomentoSvcErr) {

func prepareDictionaryElements(r hasDictionaryElements) ([]DictionaryElement, error) {
for _, v := range r.dictionaryElements() {
if v.Value == nil || v.Field == nil {
return nil, buildError(
momentoerrors.InvalidArgumentError, "element fields and values may not be nil", nil,
)
if err := validateNotNil(v.Value, "value"); err != nil {
return nil, err
}
if err := validateNotEmpty(v.Field.asBytes(), "element field"); err != nil {
if err := validateNotEmpty(v.Field, "element field"); err != nil {
return nil, err
}
}
Expand Down Expand Up @@ -210,29 +177,44 @@ func prepareTtl(r hasTtl, defaultTtl time.Duration) (uint64, error) {
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)
func momentoValuesToPrimitiveByteList(values []Value) ([][]byte, error) {
if values == nil {
return nil, buildError(momentoerrors.InvalidArgumentError, "values cannot be nil", nil)
}

var rList [][]byte
for _, mb := range i {
if mb == nil {
return [][]byte{}, buildError(momentoerrors.InvalidArgumentError, "values may not be nil", nil)
for _, mb := range values {
if err := validateNotNil(mb, "value"); err != nil {
return [][]byte{}, err
}
rList = append(rList, mb.asBytes())
}
return rList, nil
}

func validateNotEmpty(field []byte, label string) error {
if len(field) == 0 {
func validateNotEmpty(thing Value, label string) error {
if err := validateNotNil(thing, label); err != nil {
return err
}

if len(thing.asBytes()) == 0 {
return buildError(
momentoerrors.InvalidArgumentError, fmt.Sprintf("%s cannot be empty", label), nil,
momentoerrors.InvalidArgumentError, fmt.Sprintf("%v cannot be empty", label), nil,
)
}
return nil
}

func validateNotNil(value Value, label string) error {
if value == nil {
return buildError(
momentoerrors.InvalidArgumentError, fmt.Sprintf("%v cannot be nil", label), nil,
)
}

return nil
}

func ElementsFromMapStringString(theMap map[string]string) []DictionaryElement {
var elements []DictionaryElement
for k, v := range theMap {
Expand Down
4 changes: 3 additions & 1 deletion momento/sorted_set_get_rank.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ func (r *SortedSetGetRankRequest) cacheName() string { return r.CacheName }

func (r *SortedSetGetRankRequest) requestName() string { return "Sorted set get rank" }

func (r *SortedSetGetRankRequest) value() Value { return r.Value }

func (r *SortedSetGetRankRequest) initGrpcRequest(scsDataClient) error {
var err error

Expand All @@ -30,7 +32,7 @@ func (r *SortedSetGetRankRequest) initGrpcRequest(scsDataClient) error {
}

var value []byte
if value, err = prepareElementValue(r.Value); err != nil {
if value, err = prepareValue(r); err != nil {
return err
}

Expand Down
4 changes: 3 additions & 1 deletion momento/sorted_set_increment_score.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ func (r *SortedSetIncrementScoreRequest) cacheName() string { return r.CacheName

func (r *SortedSetIncrementScoreRequest) requestName() string { return "Sorted set increment" }

func (r *SortedSetIncrementScoreRequest) value() Value { return r.Value }

func (r *SortedSetIncrementScoreRequest) ttl() time.Duration { return r.Ttl.Ttl }

func (r *SortedSetIncrementScoreRequest) collectionTtl() *utils.CollectionTtl { return r.Ttl }
Expand All @@ -47,7 +49,7 @@ func (r *SortedSetIncrementScoreRequest) initGrpcRequest(client scsDataClient) e
}

var value []byte
if value, err = prepareElementValue(r.Value); err != nil {
if value, err = prepareValue(r); err != nil {
return err
}

Expand Down
Loading

0 comments on commit 1b9579b

Please sign in to comment.