diff --git a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/update/UpdateExpressionConverter.java b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/update/UpdateExpressionConverter.java index d3c3c748e563..2579d88efa58 100644 --- a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/update/UpdateExpressionConverter.java +++ b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/update/UpdateExpressionConverter.java @@ -17,6 +17,7 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -114,7 +115,7 @@ private static List groupExpressions(UpdateExpression expression) { List groupExpressions = new ArrayList<>(); if (!expression.setActions().isEmpty()) { groupExpressions.add(SET + expression.setActions().stream() - .map(a -> String.format("%s = %s", a.path(), a.value())) + .map(a -> a.path() + " = " + a.value()) .collect(Collectors.joining(ACTION_SEPARATOR))); } if (!expression.removeActions().isEmpty()) { @@ -124,42 +125,78 @@ private static List groupExpressions(UpdateExpression expression) { } if (!expression.deleteActions().isEmpty()) { groupExpressions.add(DELETE + expression.deleteActions().stream() - .map(a -> String.format("%s %s", a.path(), a.value())) + .map(a -> a.path() + " " + a.value()) .collect(Collectors.joining(ACTION_SEPARATOR))); } if (!expression.addActions().isEmpty()) { groupExpressions.add(ADD + expression.addActions().stream() - .map(a -> String.format("%s %s", a.path(), a.value())) + .map(a -> a.path() + " " + a.value()) .collect(Collectors.joining(ACTION_SEPARATOR))); } return groupExpressions; } - private static Stream> streamOfExpressionNames(UpdateExpression expression) { - return Stream.concat(expression.setActions().stream().map(SetAction::expressionNames), - Stream.concat(expression.removeActions().stream().map(RemoveAction::expressionNames), - Stream.concat(expression.deleteActions().stream() - .map(DeleteAction::expressionNames), - expression.addActions().stream() - .map(AddAction::expressionNames)))); + private static Map mergeExpressionValues(UpdateExpression expression) { + Map merged = new HashMap<>(); + + for (SetAction action : expression.setActions()) { + mergeValuesInto(merged, action.expressionValues()); + } + for (DeleteAction action : expression.deleteActions()) { + mergeValuesInto(merged, action.expressionValues()); + } + for (AddAction action : expression.addActions()) { + mergeValuesInto(merged, action.expressionValues()); + } + + return merged.isEmpty() ? Collections.emptyMap() : Collections.unmodifiableMap(merged); } - private static Map mergeExpressionValues(UpdateExpression expression) { - return streamOfExpressionValues(expression) - .reduce(Expression::joinValues) - .orElseGet(Collections::emptyMap); + private static Map mergeExpressionNames(UpdateExpression expression) { + Map merged = new HashMap<>(); + + for (SetAction action : expression.setActions()) { + mergeNamesInto(merged, action.expressionNames()); + } + for (RemoveAction action : expression.removeActions()) { + mergeNamesInto(merged, action.expressionNames()); + } + for (DeleteAction action : expression.deleteActions()) { + mergeNamesInto(merged, action.expressionNames()); + } + for (AddAction action : expression.addActions()) { + mergeNamesInto(merged, action.expressionNames()); + } + + return merged.isEmpty() ? Collections.emptyMap() : Collections.unmodifiableMap(merged); } - private static Stream> streamOfExpressionValues(UpdateExpression expression) { - return Stream.concat(expression.setActions().stream().map(SetAction::expressionValues), - Stream.concat(expression.deleteActions().stream().map(DeleteAction::expressionValues), - expression.addActions().stream().map(AddAction::expressionValues))); + private static void mergeNamesInto(Map target, Map source) { + if (source == null || source.isEmpty()) { + return; + } + source.forEach((key, value) -> { + String oldValue = target.put(key, value); + if (oldValue != null && !oldValue.equals(value)) { + throw new IllegalArgumentException( + String.format("Attempt to coalesce two expressions with conflicting expression names. " + + "Expression name key = '%s'", key)); + } + }); } - private static Map mergeExpressionNames(UpdateExpression expression) { - return streamOfExpressionNames(expression) - .reduce(Expression::joinNames) - .orElseGet(Collections::emptyMap); + private static void mergeValuesInto(Map target, Map source) { + if (source == null || source.isEmpty()) { + return; + } + source.forEach((key, value) -> { + AttributeValue oldValue = target.put(key, value); + if (oldValue != null && !oldValue.equals(value)) { + throw new IllegalArgumentException( + String.format("Attempt to coalesce two expressions with conflicting expression values. " + + "Expression value key = '%s'", key)); + } + }); } private static List listPathsWithoutTokens(UpdateExpression expression) {