From 227d00f540068d583bf75ee6b2e6eb68985512c5 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Tue, 3 Sep 2024 13:30:41 -0700 Subject: [PATCH] Addressed pr feedback --- .../internal/EnhancedClientUtils.java | 6 ++-- .../operations/UpdateItemOperation.java | 13 ++------- .../update/UpdateExpressionUtils.java | 2 +- .../functionaltests/UpdateBehaviorTest.java | 28 +++++++++++++++---- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/EnhancedClientUtils.java b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/EnhancedClientUtils.java index 33e56e87475a..61d750e98a7e 100644 --- a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/EnhancedClientUtils.java +++ b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/EnhancedClientUtils.java @@ -43,7 +43,7 @@ public final class EnhancedClientUtils { private static final Set SPECIAL_CHARACTERS = Stream.of( '*', '.', '-', '#', '+', ':', '/', '(', ')', ' ', '&', '<', '>', '?', '=', '!', '@', '%', '$', '|').collect(Collectors.toSet()); - private static final Pattern PATTERN = Pattern.compile(NESTED_OBJECT_UPDATE); + private static final Pattern NESTED_OBJECT_PATTERN = Pattern.compile(NESTED_OBJECT_UPDATE); private EnhancedClientUtils() { @@ -81,7 +81,7 @@ private static boolean isNestedAttribute(String key) { public static String keyRef(String key) { String cleanAttributeName = cleanAttributeName(key); cleanAttributeName = isNestedAttribute(cleanAttributeName) ? - PATTERN.matcher(cleanAttributeName).replaceAll(".#AMZN_MAPPED_") + NESTED_OBJECT_PATTERN.matcher(cleanAttributeName).replaceAll(".#AMZN_MAPPED_") : cleanAttributeName; return "#AMZN_MAPPED_" + cleanAttributeName; } @@ -92,7 +92,7 @@ public static String keyRef(String key) { public static String valueRef(String value) { String cleanAttributeName = cleanAttributeName(value); cleanAttributeName = isNestedAttribute(cleanAttributeName) ? - PATTERN.matcher(cleanAttributeName).replaceAll("_") + NESTED_OBJECT_PATTERN.matcher(cleanAttributeName).replaceAll("_") : cleanAttributeName; return ":AMZN_MAPPED_" + cleanAttributeName; } diff --git a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/operations/UpdateItemOperation.java b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/operations/UpdateItemOperation.java index 9eb33bef3ad6..0f0d9cb170b3 100644 --- a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/operations/UpdateItemOperation.java +++ b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/operations/UpdateItemOperation.java @@ -195,17 +195,8 @@ private Map nestedItemToMap(Map } private boolean isNotEmptyMap(Map map) { - if (map.isEmpty()) { - return false; - } - - // Checks if a fully empty map is being set. If that is the case, no input transformations are applied to the map - for (Map.Entry entry : map.entrySet()) { - if (attributeValueNonNullOrShouldWriteNull(entry.getValue())) { - return true; - } - } - return false; + return !map.isEmpty() && map.values().stream() + .anyMatch(this::attributeValueNonNullOrShouldWriteNull); } private boolean attributeValueNonNullOrShouldWriteNull(AttributeValue attributeValue) { diff --git a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/update/UpdateExpressionUtils.java b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/update/UpdateExpressionUtils.java index eb9e6f5c90ac..1d47400ab2e6 100644 --- a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/update/UpdateExpressionUtils.java +++ b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/update/UpdateExpressionUtils.java @@ -103,7 +103,7 @@ private static List removeActionsFor(Map a private static RemoveAction remove(String attributeName) { return RemoveAction.builder() .path(keyRef(attributeName)) - .expressionNames(expressionNamesFor(attributeName)) + .expressionNames(Collections.singletonMap(keyRef(attributeName), attributeName)) .build(); } diff --git a/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/UpdateBehaviorTest.java b/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/UpdateBehaviorTest.java index 595579133d9b..a85ce465aab9 100644 --- a/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/UpdateBehaviorTest.java +++ b/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/UpdateBehaviorTest.java @@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.time.Instant; +import java.util.Collections; import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.After; @@ -18,7 +19,10 @@ import software.amazon.awssdk.enhanced.dynamodb.functionaltests.models.NestedRecordWithUpdateBehavior; import software.amazon.awssdk.enhanced.dynamodb.functionaltests.models.RecordWithUpdateBehaviors; import software.amazon.awssdk.enhanced.dynamodb.internal.client.ExtensionResolver; +import software.amazon.awssdk.services.dynamodb.model.AttributeValue; import software.amazon.awssdk.services.dynamodb.model.DynamoDbException; +import software.amazon.awssdk.services.dynamodb.model.GetItemRequest; +import software.amazon.awssdk.services.dynamodb.model.GetItemResponse; public class UpdateBehaviorTest extends LocalDynamoDbSyncTestBase { private static final Instant INSTANT_1 = Instant.parse("2020-05-03T10:00:00Z"); @@ -303,17 +307,31 @@ public void when_updatingNestedNonScalarObject_DynamoDBExceptionIsThrown() { } @Test - public void test() { - String randomId = "id123"; + public void when_emptyNestedRecordIsSet_emotyMapIsStoredInTable() { + String key = "id123"; RecordWithUpdateBehaviors record = new RecordWithUpdateBehaviors(); - record.setId(randomId); + record.setId(key); record.setNestedRecord(new NestedRecordWithUpdateBehavior()); mappedTable.updateItem(r -> r.item(record).ignoreNulls(true)); - RecordWithUpdateBehaviors persistedRecord = mappedTable.getItem(r -> r.key(k -> k.partitionValue("id123"))); - assertThat(persistedRecord.getNestedRecord()).isNull(); + GetItemResponse getItemResponse = getDynamoDbClient().getItem(GetItemRequest.builder() + .key(Collections.singletonMap("id", + AttributeValue.fromS(key))) + .tableName(getConcreteTableName("table-name")) + .build()); + + assertThat(getItemResponse.item().get("nestedRecord")).isNotNull(); + assertThat(getItemResponse.item().get("nestedRecord").toString()).isEqualTo("AttributeValue(M={nestedTimeAttribute" + + "=AttributeValue(NUL=true), " + + "nestedRecord=AttributeValue(NUL=true), " + + "attribute=AttributeValue(NUL=true), " + + "id=AttributeValue(NUL=true), " + + "nestedUpdateBehaviorAttribute=AttributeValue" + + "(NUL=true), nestedCounter=AttributeValue" + + "(NUL=true), nestedVersionedAttribute" + + "=AttributeValue(NUL=true)})"); }