Skipping empty string in update expressions as it used to convert to …

dynamo_repository.go

@@ -89,6 +89,14 @@ func (repository Repository) SaveItemWithContext(ctx context.Context, key KeyInt
 	return nil
 }
 
+// isEmptyString returns true if value is a string with zero length.
+// Empty strings cannot be marshaled to valid DynamoDB AttributeValues by guregu/dynamo
+// (they marshal to nil), which causes SerializationException errors from DynamoDB.
+func isEmptyString(value interface{}) bool {
+	str, ok := value.(string)
+	return ok && str == ""
+}
+
 // UpdateWithContext updates item by key; it accepts an expression (Set, SetSet, SetIfNotExists, SetExpr); key is the key to be updated;
 // values contains the values that should be used in the update; context which used to enable log with context
 // returns error in case of error
@@ -109,6 +117,11 @@ func (repository Repository) UpdateWithContext(ctx context.Context, expression U
 	}
 
 	for expr, value := range values {
+		// Skip empty strings: guregu/dynamo marshals them to nil, which causes
+		// DynamoDB SerializationException errors.
+		if isEmptyString(value) {
+			continue
+		}
 		if expression == Add {
 			update.Add(expr, value)
 		}
@@ -159,6 +172,11 @@ func (repository Repository) prepareUpdateWithUpdateExpressions(
 		expression := UpdateExpression(updateExpression)
 
 		for expr, value := range v {
+			// Skip empty strings: guregu/dynamo marshals them to nil, which causes
+			// DynamoDB SerializationException errors.
+			if isEmptyString(value) {
+				continue
+			}
 			if expression == Add {
 				update.Add(expr, value)
 			}

key.go

@@ -100,5 +100,8 @@ func isValidHashKey(key KeyInterface) error {
 	if key.HashKey() == nil {
 		return ErrInvalidHashKeyValue
 	}
+	if str, ok := key.HashKey().(string); ok && str == "" {
+		return ErrInvalidHashKeyValue
+	}
 	return nil
 }

repository_update_test.go

@@ -67,6 +67,17 @@ var _ = Describe("Repository", func() {
 					"TraceID":  "name4",
 				}
 
+				err := repository.UpdateWithContext(context.Background(), djoemo.Set, key, updates)
+				Expect(err).To(Equal(djoemo.ErrInvalidHashKeyValue))
+			})
+			It("should fail with hash key value is empty string", func() {
+				key := djoemo.Key().WithTableName(UserTableName).WithHashKeyName("UUID").WithHashKey("")
+				metricsMock.EXPECT().Record(gomock.Any(), djoemo.OpUpdate, key, gomock.Any(), false)
+				updates := map[string]interface{}{
+					"UserName": "name2",
+					"TraceID":  "name4",
+				}
+
 				err := repository.UpdateWithContext(context.Background(), djoemo.Set, key, updates)
 				Expect(err).To(Equal(djoemo.ErrInvalidHashKeyValue))
 			})
@@ -98,6 +109,54 @@ var _ = Describe("Repository", func() {
 			Expect(err).To(BeNil())
 		})
 
+		It("should skip empty string values in Set update", func() {
+			key := djoemo.Key().WithTableName(UserTableName).
+				WithHashKeyName("UUID").
+				WithHashKey("uuid")
+
+			dMock.Should().Update(
+				dMock.WithTable(key.TableName()),
+				dMock.WithMatch(
+					mock.InputExpect().
+						FieldEq("UserName", "name2"),
+				),
+			).Exec()
+
+			metricsMock.EXPECT().Record(gomock.Any(), djoemo.OpUpdate, key, gomock.Any(), true)
+
+			updates := map[string]interface{}{
+				"UserName": "name2",
+				"DeviceID": "",
+			}
+
+			err := repository.UpdateWithContext(context.Background(), djoemo.Set, key, updates)
+			Expect(err).To(BeNil())
+		})
+
+		It("should skip empty string values in SetIfNotExists update", func() {
+			key := djoemo.Key().WithTableName(UserTableName).
+				WithHashKeyName("UUID").
+				WithHashKey("uuid")
+
+			dMock.Should().Update(
+				dMock.WithTable(key.TableName()),
+				dMock.WithMatch(
+					mock.InputExpect().
+						FieldEq("SDKHash", "hash123"),
+				),
+			).Exec()
+
+			metricsMock.EXPECT().Record(gomock.Any(), djoemo.OpUpdate, key, gomock.Any(), true)
+
+			updates := map[string]interface{}{
+				"SDKHash":    "hash123",
+				"DeviceName": "",
+			}
+
+			err := repository.UpdateWithContext(context.Background(), djoemo.SetIfNotExists, key, updates)
+			Expect(err).To(BeNil())
+		})
+
 		It("should Update item with SetSet", func() {
 			key := djoemo.Key().WithTableName(UserTableName).
 				WithHashKeyName("UUID").
@@ -215,6 +274,82 @@ var _ = Describe("Repository", func() {
 		})
 	})
 
+	Describe("UpdateWithUpdateExpressions", func() {
+		It("should update item with mixed Set and SetIfNotExists expressions", func() {
+			key := djoemo.Key().WithTableName(UserTableName).
+				WithHashKeyName("UUID").
+				WithHashKey("uuid")
+
+			dMock.Should().Update(
+				dMock.WithTable(key.TableName()),
+				dMock.WithMatch(
+					mock.InputExpect().
+						FieldEq("UserName", "name2").FieldEq("SDKHash", "hash123"),
+				),
+			).Exec()
+
+			metricsMock.EXPECT().Record(gomock.Any(), djoemo.OpUpdate, key, gomock.Any(), true)
+
+			updateExpressions := djoemo.UpdateExpressions{
+				djoemo.Set: {
+					"UserName": "name2",
+				},
+				djoemo.SetIfNotExists: {
+					"SDKHash": "hash123",
+				},
+			}
+
+			err := repository.UpdateWithUpdateExpressions(context.Background(), key, updateExpressions)
+			Expect(err).To(BeNil())
+		})
+
+		It("should fail with hash key value is empty string", func() {
+			key := djoemo.Key().WithTableName(UserTableName).WithHashKeyName("UUID").WithHashKey("")
+			metricsMock.EXPECT().Record(gomock.Any(), djoemo.OpUpdate, key, gomock.Any(), false)
+
+			updateExpressions := djoemo.UpdateExpressions{
+				djoemo.Set: {
+					"UserName": "name2",
+				},
+			}
+
+			err := repository.UpdateWithUpdateExpressions(context.Background(), key, updateExpressions)
+			Expect(err).To(Equal(djoemo.ErrInvalidHashKeyValue))
+		})
+
+		It("should skip empty string values in Set and SetIfNotExists expressions", func() {
+			key := djoemo.Key().WithTableName(UserTableName).
+				WithHashKeyName("UUID").
+				WithHashKey("uuid")
+
+			dMock.Should().Update(
+				dMock.WithTable(key.TableName()),
+				dMock.WithMatch(
+					mock.InputExpect().
+						FieldEq("UserName", "name2").FieldEq("SDKHash", "hash123"),
+				),
+			).Exec()
+
+			metricsMock.EXPECT().Record(gomock.Any(), djoemo.OpUpdate, key, gomock.Any(), true)
+
+			updateExpressions := djoemo.UpdateExpressions{
+				djoemo.Set: {
+					"UserName":    "name2",
+					"DeviceID":    "",
+					"ProductName": "",
+				},
+				djoemo.SetIfNotExists: {
+					"SDKHash":    "hash123",
+					"DeviceName": "",
+					"DeviceType": "",
+				},
+			}
+
+			err := repository.UpdateWithUpdateExpressions(context.Background(), key, updateExpressions)
+			Expect(err).To(BeNil())
+		})
+	})
+
 	Describe("UpdateItem with condition", func() {
 		It("should save an item if the condition is met", func() {
 			key := djoemo.Key().WithTableName(UserTableName).