Skip to content

Commit 020dacd

Browse files
iulianbudauiulianbudau
committed
Fixed DynamoDbEnhancedClient TableSchema::itemToMap to handle null flattened members when ignoreNulls is false
1 parent 1594ddd commit 020dacd

File tree

7 files changed

+496
-3
lines changed

7 files changed

+496
-3
lines changed

‎.changes/next-release/bugfix-AmazonDynamoDBEnhancedClient-752432f.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "Amazon DynamoDB Enhanced Client",
4+
"contributor": "",
5+
"description": "Fixed DynamoDbEnhancedClient TableSchema::itemToMap to return a map that contains a consistent representation of null top-level (non-flattened) attributes and flattened attributes when their enclosing member is null and ignoreNulls is set to false."
6+
}

‎services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/mapper/StaticImmutableTableSchema.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ private B mapToItem(B thisBuilder,
126126
private Map<String, AttributeValue> itemToMap(T item, boolean ignoreNulls) {
127127
T1 otherItem = this.otherItemGetter.apply(item);
128128

129-
if (otherItem == null) {
129+
if (otherItem == null && ignoreNulls) {
130130
return Collections.emptyMap();
131131
}
132132

@@ -515,15 +515,21 @@ public Map<String, AttributeValue> itemToMap(T item, boolean ignoreNulls) {
515515

516516
attributeMappers.forEach(attributeMapper -> {
517517
String attributeKey = attributeMapper.attributeName();
518-
AttributeValue attributeValue = attributeMapper.attributeGetterMethod().apply(item);
518+
AttributeValue attributeValue = item == null ?
519+
AttributeValue.fromNul(true) :
520+
attributeMapper.attributeGetterMethod().apply(item);
519521

520522
if (!ignoreNulls || !isNullAttributeValue(attributeValue)) {
521523
attributeValueMap.put(attributeKey, attributeValue);
522524
}
523525
});
524526

525527
indexedFlattenedMappers.forEach((name, flattenedMapper) -> {
526-
attributeValueMap.putAll(flattenedMapper.itemToMap(item, ignoreNulls));
528+
if (item != null) {
529+
attributeValueMap.putAll(flattenedMapper.itemToMap(item, ignoreNulls));
530+
} else if (!ignoreNulls) {
531+
attributeValueMap.put(name, AttributeValue.fromNul(true));
532+
}
527533
});
528534

529535
return unmodifiableMap(attributeValueMap);

‎services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/mapper/BeanTableSchemaTest.java‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import software.amazon.awssdk.enhanced.dynamodb.internal.AttributeValues;
4646
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.AbstractBean;
4747
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.AbstractImmutable;
48+
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.AbstractNestedImmutable;
4849
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.AttributeConverterBean;
4950
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.AttributeConverterNoConstructorBean;
5051
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.CommonTypesBean;
@@ -54,7 +55,10 @@
5455
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.EnumBean;
5556
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.ExtendedBean;
5657
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.FlattenedBeanBean;
58+
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.FlattenedFirstNestedBean;
59+
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.FlattenedFirstNestedBean.FlattenedSecondNestedBean;
5760
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.FlattenedImmutableBean;
61+
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.FlattenedNestedImmutableBean;
5862
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.FluentSetterBean;
5963
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.IgnoredAttributeBean;
6064
import software.amazon.awssdk.enhanced.dynamodb.mapper.testbeans.InvalidBean;
@@ -257,6 +261,128 @@ public void dynamoDbFlatten_correctlyFlattensImmutableAttributes() {
257261
assertThat(itemMap, hasEntry("attribute2", stringValue("two")));
258262
}
259263

264+
@Test
265+
public void dynamoDbFlatten_correctlyFlattensNullImmutableAttributes() {
266+
BeanTableSchema<FlattenedImmutableBean> beanTableSchema = BeanTableSchema.create(FlattenedImmutableBean.class);
267+
AbstractImmutable abstractImmutable = AbstractImmutable.builder().build();
268+
FlattenedImmutableBean flattenedImmutableBean = new FlattenedImmutableBean();
269+
flattenedImmutableBean.setId("id-value");
270+
flattenedImmutableBean.setAbstractImmutable(abstractImmutable);
271+
272+
Map<String, AttributeValue> itemMap = beanTableSchema.itemToMap(flattenedImmutableBean, false);
273+
assertThat(itemMap.size(), is(3));
274+
assertThat(itemMap, hasEntry("id", stringValue("id-value")));
275+
assertThat(itemMap, hasEntry("attribute1", AttributeValue.fromNul(true)));
276+
assertThat(itemMap, hasEntry("attribute2", AttributeValue.fromNul(true)));
277+
}
278+
279+
@Test
280+
public void dynamoDbFlatten_correctlyFlattensNestedImmutableAttributes() {
281+
BeanTableSchema<FlattenedNestedImmutableBean> beanTableSchema =
282+
BeanTableSchema.create(FlattenedNestedImmutableBean.class);
283+
AbstractNestedImmutable abstractNestedImmutable2 =
284+
AbstractNestedImmutable.builder().attribute2("nested-two").build();
285+
AbstractNestedImmutable abstractNestedImmutable1 =
286+
AbstractNestedImmutable.builder().attribute2("two").abstractNestedImmutableOne(abstractNestedImmutable2).build();
287+
FlattenedNestedImmutableBean flattenedNestedImmutableBean = new FlattenedNestedImmutableBean();
288+
flattenedNestedImmutableBean.setId("id-value");
289+
flattenedNestedImmutableBean.setAttribute1("one");
290+
flattenedNestedImmutableBean.setAbstractNestedImmutable(abstractNestedImmutable1);
291+
292+
Map<String, AttributeValue> nestedAttributesMap = new HashMap<>();
293+
nestedAttributesMap.put("abstractNestedImmutableOne", AttributeValue.fromNul(true));
294+
nestedAttributesMap.put("attribute2", stringValue("nested-two"));
295+
296+
AttributeValue expectedNestedAttribute =
297+
AttributeValue.builder().m(Collections.unmodifiableMap(nestedAttributesMap)).build();
298+
299+
Map<String, AttributeValue> itemMap = beanTableSchema.itemToMap(flattenedNestedImmutableBean, false);
300+
assertThat(itemMap.size(), is(4));
301+
assertThat(itemMap, hasEntry("id", stringValue("id-value")));
302+
assertThat(itemMap, hasEntry("attribute1", stringValue("one")));
303+
assertThat(itemMap, hasEntry("attribute2", stringValue("two")));
304+
assertThat(itemMap, hasEntry("abstractNestedImmutableOne", expectedNestedAttribute));
305+
}
306+
307+
@Test
308+
public void dynamoDbFlatten_correctlyFlattensNullNestedImmutableAttributes() {
309+
BeanTableSchema<FlattenedNestedImmutableBean> beanTableSchema =
310+
BeanTableSchema.create(FlattenedNestedImmutableBean.class);
311+
AbstractNestedImmutable abstractNestedImmutable = AbstractNestedImmutable.builder().build();
312+
FlattenedNestedImmutableBean flattenedNestedImmutableBean = new FlattenedNestedImmutableBean();
313+
flattenedNestedImmutableBean.setId("id-value");
314+
flattenedNestedImmutableBean.setAttribute1("one");
315+
flattenedNestedImmutableBean.setAbstractNestedImmutable(abstractNestedImmutable);
316+
317+
Map<String, AttributeValue> itemMap = beanTableSchema.itemToMap(flattenedNestedImmutableBean, false);
318+
assertThat(itemMap.size(), is(4));
319+
assertThat(itemMap, hasEntry("id", stringValue("id-value")));
320+
assertThat(itemMap, hasEntry("attribute1", stringValue("one")));
321+
assertThat(itemMap, hasEntry("attribute2", AttributeValue.fromNul(true)));
322+
assertThat(itemMap, hasEntry("abstractNestedImmutableOne", AttributeValue.fromNul(true)));
323+
}
324+
325+
@Test
326+
public void dynamoDbFlatten_correctlyFlattensSecondNullNestedAttributes_IgnoreNullsFalse() {
327+
BeanTableSchema<FlattenedFirstNestedBean> beanTableSchema =
328+
BeanTableSchema.create(FlattenedFirstNestedBean.class);
329+
FlattenedFirstNestedBean flattenedFirstNestedBean = new FlattenedFirstNestedBean();
330+
flattenedFirstNestedBean.setId("id-value");
331+
332+
Map<String, AttributeValue> itemMap = beanTableSchema.itemToMap(flattenedFirstNestedBean, false);
333+
assertThat(itemMap.size(), is(4));
334+
assertThat(itemMap, hasEntry("id", stringValue("id-value")));
335+
assertThat(itemMap, hasEntry("secondId", AttributeValue.fromNul(true)));
336+
assertThat(itemMap, hasEntry("thirdId", AttributeValue.fromNul(true)));
337+
assertThat(itemMap, hasEntry("flattenedFourthBean", AttributeValue.fromNul(true)));
338+
}
339+
340+
@Test
341+
public void dynamoDbFlatten_correctlyFlattensSecondNullNestedAttributes_IgnoreNullsTrue() {
342+
BeanTableSchema<FlattenedFirstNestedBean> beanTableSchema =
343+
BeanTableSchema.create(FlattenedFirstNestedBean.class);
344+
FlattenedFirstNestedBean flattenedFirstNestedBean = new FlattenedFirstNestedBean();
345+
flattenedFirstNestedBean.setId("id-value");
346+
347+
Map<String, AttributeValue> itemMap = beanTableSchema.itemToMap(flattenedFirstNestedBean, true);
348+
assertThat(itemMap.size(), is(1));
349+
assertThat(itemMap, hasEntry("id", stringValue("id-value")));
350+
}
351+
352+
@Test
353+
public void dynamoDbFlatten_correctlyFlattensThirdNullNestedAttributes_IgnoreNullsFalse() {
354+
BeanTableSchema<FlattenedFirstNestedBean> beanTableSchema =
355+
BeanTableSchema.create(FlattenedFirstNestedBean.class);
356+
FlattenedSecondNestedBean flattenedSecondNestedBean = new FlattenedSecondNestedBean();
357+
flattenedSecondNestedBean.setSecondId("second-id-value");
358+
FlattenedFirstNestedBean flattenedFirstNestedBean = new FlattenedFirstNestedBean();
359+
flattenedFirstNestedBean.setId("id-value");
360+
flattenedFirstNestedBean.setFlattenedSecondNestedBean(flattenedSecondNestedBean);
361+
362+
Map<String, AttributeValue> itemMap = beanTableSchema.itemToMap(flattenedFirstNestedBean, false);
363+
assertThat(itemMap.size(), is(4));
364+
assertThat(itemMap, hasEntry("id", stringValue("id-value")));
365+
assertThat(itemMap, hasEntry("secondId", stringValue("second-id-value")));
366+
assertThat(itemMap, hasEntry("thirdId", AttributeValue.fromNul(true)));
367+
assertThat(itemMap, hasEntry("flattenedFourthBean", AttributeValue.fromNul(true)));
368+
}
369+
370+
@Test
371+
public void dynamoDbFlatten_correctlyFlattensThirdNullNestedAttributes_IgnoreNullsTrue() {
372+
BeanTableSchema<FlattenedFirstNestedBean> beanTableSchema =
373+
BeanTableSchema.create(FlattenedFirstNestedBean.class);
374+
FlattenedSecondNestedBean flattenedSecondNestedBean = new FlattenedSecondNestedBean();
375+
flattenedSecondNestedBean.setSecondId("second-id-value");
376+
FlattenedFirstNestedBean flattenedFirstNestedBean = new FlattenedFirstNestedBean();
377+
flattenedFirstNestedBean.setId("id-value");
378+
flattenedFirstNestedBean.setFlattenedSecondNestedBean(flattenedSecondNestedBean);
379+
380+
Map<String, AttributeValue> itemMap = beanTableSchema.itemToMap(flattenedFirstNestedBean, true);
381+
assertThat(itemMap.size(), is(2));
382+
assertThat(itemMap, hasEntry("id", stringValue("id-value")));
383+
assertThat(itemMap, hasEntry("secondId", stringValue("second-id-value")));
384+
}
385+
260386
@Test
261387
public void documentBean_correctlyMapsBeanAttributes() {
262388
BeanTableSchema<DocumentBean> beanTableSchema = BeanTableSchema.create(DocumentBean.class);

‎services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/mapper/StaticImmutableTableSchemaTest.java‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import software.amazon.awssdk.enhanced.dynamodb.functionaltests.models.FakeItem;
6060
import software.amazon.awssdk.enhanced.dynamodb.functionaltests.models.FakeItemComposedClass;
6161
import software.amazon.awssdk.enhanced.dynamodb.functionaltests.models.FakeItemWithSort;
62+
import software.amazon.awssdk.enhanced.dynamodb.mapper.annotations.DynamoDbFlatten;
6263
import software.amazon.awssdk.enhanced.dynamodb.mapper.testimmutables.EntityEnvelopeImmutable;
6364
import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
6465

@@ -782,6 +783,62 @@ public Consumer<StaticTableMetadata.Builder> modifyMetadata() {
782783
}
783784
}
784785

786+
public static final class FakeMappedItemWithDeep {
787+
private String documentString;
788+
private FakeDocumentWithDeep aFakeDocument;
789+
790+
public String getDocumentString() {
791+
return documentString;
792+
}
793+
794+
public void setDocumentString(String documentString) {
795+
this.documentString = documentString;
796+
}
797+
798+
@DynamoDbFlatten
799+
public FakeDocumentWithDeep getAFakeDocument() {
800+
return aFakeDocument;
801+
}
802+
803+
public void setAFakeDocument(FakeDocumentWithDeep aFakeDocument) {
804+
this.aFakeDocument = aFakeDocument;
805+
}
806+
}
807+
808+
public static final class FakeDocumentWithDeep {
809+
private Integer documentInteger;
810+
private DeepFakeDocument deepFakeDocument;
811+
812+
public Integer getDocumentInteger() {
813+
return documentInteger;
814+
}
815+
816+
public void setDocumentInteger(Integer documentInteger) {
817+
this.documentInteger = documentInteger;
818+
}
819+
820+
@DynamoDbFlatten
821+
public DeepFakeDocument getDeepFakeDocument() {
822+
return deepFakeDocument;
823+
}
824+
825+
public void setDeepFakeDocument(DeepFakeDocument deepFakeDocument) {
826+
this.deepFakeDocument = deepFakeDocument;
827+
}
828+
}
829+
830+
public static final class DeepFakeDocument {
831+
private String deepString;
832+
833+
public String getDeepString() {
834+
return deepString;
835+
}
836+
837+
public void setDeepString(String deepString) {
838+
this.deepString = deepString;
839+
}
840+
}
841+
785842
@Mock
786843
private AttributeConverterProvider provider1;
787844

@@ -1388,6 +1445,67 @@ public void buildAbstractWithFlatten() {
13881445
is(singletonMap("documentString", AttributeValue.builder().s("test-string").build())));
13891446
}
13901447

1448+
@Test
1449+
public void buildAbstractWithFlattenAndIgnoreNullAsFalse() {
1450+
StaticTableSchema<FakeMappedItem> tableSchema =
1451+
StaticTableSchema.builder(FakeMappedItem.class)
1452+
.flatten(FAKE_DOCUMENT_TABLE_SCHEMA,
1453+
FakeMappedItem::getAFakeDocument,
1454+
FakeMappedItem::setAFakeDocument)
1455+
.build();
1456+
1457+
FakeDocument document = FakeDocument.of("test-string", null);
1458+
FakeMappedItem item = FakeMappedItem.builder().aFakeDocument(document).build();
1459+
1460+
Map<String, AttributeValue> attributeMapWithNulls = tableSchema.itemToMap(item, false);
1461+
assertThat(attributeMapWithNulls.size(), is(2));
1462+
assertThat(attributeMapWithNulls, hasEntry("documentString", AttributeValue.builder().s("test-string").build()));
1463+
assertThat(attributeMapWithNulls, hasEntry("documentInteger", AttributeValue.fromNul(true)));
1464+
}
1465+
1466+
@Test
1467+
public void buildAbstractWithNestedFlattenAndIgnoreNullAsFalse() {
1468+
StaticTableSchema<DeepFakeDocument> deepSchema =
1469+
StaticTableSchema.builder(DeepFakeDocument.class)
1470+
.newItemSupplier(DeepFakeDocument::new)
1471+
.addAttribute(String.class, a -> a.name("deepString")
1472+
.getter(DeepFakeDocument::getDeepString)
1473+
.setter(DeepFakeDocument::setDeepString))
1474+
.build();
1475+
1476+
StaticTableSchema<FakeDocumentWithDeep> nestedSchema =
1477+
StaticTableSchema.builder(FakeDocumentWithDeep.class)
1478+
.newItemSupplier(FakeDocumentWithDeep::new)
1479+
.addAttribute(Integer.class, a -> a.name("documentInteger")
1480+
.getter(FakeDocumentWithDeep::getDocumentInteger)
1481+
.setter(FakeDocumentWithDeep::setDocumentInteger))
1482+
.flatten(deepSchema,
1483+
FakeDocumentWithDeep::getDeepFakeDocument,
1484+
FakeDocumentWithDeep::setDeepFakeDocument)
1485+
.build();
1486+
1487+
StaticTableSchema<FakeMappedItemWithDeep> tableSchema =
1488+
StaticTableSchema.builder(FakeMappedItemWithDeep.class)
1489+
.newItemSupplier(FakeMappedItemWithDeep::new)
1490+
.addAttribute(String.class, a -> a.name("documentString")
1491+
.getter(FakeMappedItemWithDeep::getDocumentString)
1492+
.setter(FakeMappedItemWithDeep::setDocumentString))
1493+
.flatten(nestedSchema,
1494+
FakeMappedItemWithDeep::getAFakeDocument,
1495+
FakeMappedItemWithDeep::setAFakeDocument)
1496+
.build();
1497+
1498+
FakeMappedItemWithDeep item = new FakeMappedItemWithDeep();
1499+
item.setDocumentString("top-level-string");
1500+
1501+
Map<String, AttributeValue> attributeMap = tableSchema.itemToMap(item, false);
1502+
1503+
assertThat(attributeMap.size(), is(3));
1504+
assertThat(attributeMap, hasEntry("documentString", AttributeValue.builder().s("top-level-string").build()));
1505+
assertThat(attributeMap, hasEntry("documentInteger", AttributeValue.fromNul(true)));
1506+
assertThat(attributeMap, hasEntry("deepString", AttributeValue.fromNul(true)));
1507+
}
1508+
13911509
@Test
13921510
public void buildAbstractExtends() {
13931511
StaticTableSchema<FakeAbstractSuperclass> superclassTableSchema =

0 commit comments

Comments
 (0)