Skip to content

Commit bdec395

Browse files
RanVakninRanVaknin
committed
Refactoring based on comments from maintainer
1 parent 33037e1 commit bdec395

File tree

5 files changed

+135
-147
lines changed

5 files changed

+135
-147
lines changed

‎pom.xml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,9 @@
105105
<awsjavasdk.version>${project.version}</awsjavasdk.version>
106106
<awsjavasdk.previous.version>2.36.2</awsjavasdk.previous.version>
107107
<awsjavasdk.previous-previous.version>2.36.1</awsjavasdk.previous-previous.version>
108-
<jackson.version>2.15.2</jackson.version>
109-
<jackson.databind.version>2.15.2</jackson.databind.version>
110-
<jacksonjr.version>2.17.3</jacksonjr.version>
108+
<jackson.version>2.18.0</jackson.version>
109+
<jackson.databind.version>2.18.0</jackson.databind.version>
110+
<jacksonjr.version>2.18.0</jacksonjr.version>
111111
<eventstream.version>1.0.1</eventstream.version>
112112
<commons.lang.version>3.14.0</commons.lang.version>
113113
<wiremock.version>2.32.0</wiremock.version>

‎services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/document/DefaultEnhancedDocument.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ public String getJson(String attributeName) {
191191
if (attributeValue == null) {
192192
return null;
193193
}
194-
return StrategyJsonSerializer.serializeSingleAttributeValue(attributeValue);
194+
return DocumentJsonSerializer.serializeSingleAttributeValue(attributeValue);
195195
}
196196

197197
@Override
@@ -228,7 +228,7 @@ public String toJson() {
228228
if (nonAttributeValueMap.isEmpty()) {
229229
return "{}";
230230
}
231-
return StrategyJsonSerializer.serializeAttributeValueMap(attributeValueMap.getValue());
231+
return DocumentJsonSerializer.serializeAttributeValueMap(attributeValueMap.getValue());
232232
}
233233

234234
@Override

services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/document/StrategyJsonSerializer.java renamed to services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/internal/document/DocumentJsonSerializer.java

Lines changed: 26 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -15,32 +15,30 @@
1515

1616
package software.amazon.awssdk.enhanced.dynamodb.internal.document;
1717

18+
import java.io.IOException;
19+
import java.io.OutputStream;
1820
import java.nio.charset.StandardCharsets;
1921
import java.util.Map;
2022
import software.amazon.awssdk.annotations.SdkInternalApi;
2123
import software.amazon.awssdk.core.SdkBytes;
2224
import software.amazon.awssdk.protocols.json.SdkJsonGenerator;
2325
import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
2426
import software.amazon.awssdk.thirdparty.jackson.core.JsonFactory;
27+
import software.amazon.awssdk.thirdparty.jackson.core.JsonGenerator;
2528

29+
/**
30+
* JSON serializer for DynamoDB Enhanced Client document operations.
31+
*/
2632
@SdkInternalApi
27-
public final class StrategyJsonSerializer {
28-
private static final JsonFactory JSON_FACTORY = new JsonFactory();
29-
30-
private StrategyJsonSerializer() {
31-
}
33+
public final class DocumentJsonSerializer {
34+
private static final JsonFactory JSON_FACTORY = new JsonFactory() {
35+
@Override
36+
public JsonGenerator createGenerator(OutputStream out) throws IOException {
37+
return super.createGenerator(out).enable(JsonGenerator.Feature.COMBINE_UNICODE_SURROGATES_IN_UTF8);
38+
}
39+
};
3240

33-
private enum JsonSerializationStrategy {
34-
NULL,
35-
STRING,
36-
NUMBER,
37-
BOOLEAN,
38-
BYTES,
39-
LIST,
40-
MAP,
41-
STRING_SET,
42-
NUMBER_SET,
43-
BYTES_SET
41+
private DocumentJsonSerializer() {
4442
}
4543

4644
public static String serializeAttributeValueMap(Map<String, AttributeValue> map) {
@@ -63,96 +61,61 @@ public static String serializeSingleAttributeValue(AttributeValue av) {
6361
}
6462

6563
public static void serializeAttributeValue(SdkJsonGenerator generator, AttributeValue av) {
66-
JsonSerializationStrategy strategy = getStrategy(av);
67-
68-
switch (strategy) {
69-
case NULL:
64+
switch (av.type()) {
65+
case NUL:
7066
generator.writeNull();
7167
break;
72-
case STRING:
68+
case S:
7369
generator.writeValue(av.s());
7470
break;
75-
case NUMBER:
71+
case N:
7672
generator.writeNumber(av.n());
7773
break;
78-
case BOOLEAN:
74+
case BOOL:
7975
generator.writeValue(av.bool());
8076
break;
81-
case BYTES:
77+
case B:
8278
generator.writeValue(av.b().asByteBuffer());
8379
break;
84-
case LIST:
80+
case L:
8581
generator.writeStartArray();
8682
for (AttributeValue item : av.l()) {
8783
serializeAttributeValue(generator, item);
8884
}
8985
generator.writeEndArray();
9086
break;
91-
case MAP:
87+
case M:
9288
generator.writeStartObject();
9389
for (Map.Entry<String, AttributeValue> entry : av.m().entrySet()) {
9490
generator.writeFieldName(entry.getKey());
9591
serializeAttributeValue(generator, entry.getValue());
9692
}
9793
generator.writeEndObject();
9894
break;
99-
case STRING_SET:
95+
case SS:
10096
generator.writeStartArray();
10197
for (String s : av.ss()) {
10298
generator.writeValue(s);
10399
}
104100
generator.writeEndArray();
105101
break;
106-
case NUMBER_SET:
102+
case NS:
107103
generator.writeStartArray();
108104
for (String n : av.ns()) {
109105
generator.writeNumber(n);
110106
}
111107
generator.writeEndArray();
112108
break;
113-
case BYTES_SET:
109+
case BS:
114110
generator.writeStartArray();
115111
for (SdkBytes b : av.bs()) {
116112
generator.writeValue(b.asByteBuffer());
117113
}
118114
generator.writeEndArray();
119115
break;
120116
default:
121-
throw new IllegalStateException("Unsupported strategy: " + strategy);
117+
throw new IllegalArgumentException("Unsupported AttributeValue type: " + av.type());
122118
}
123119
}
124120

125-
private static JsonSerializationStrategy getStrategy(AttributeValue av) {
126-
if (av.nul() != null && av.nul()) {
127-
return JsonSerializationStrategy.NULL;
128-
}
129-
if (av.s() != null) {
130-
return JsonSerializationStrategy.STRING;
131-
}
132-
if (av.n() != null) {
133-
return JsonSerializationStrategy.NUMBER;
134-
}
135-
if (av.bool() != null) {
136-
return JsonSerializationStrategy.BOOLEAN;
137-
}
138-
if (av.b() != null) {
139-
return JsonSerializationStrategy.BYTES;
140-
}
141-
if (av.hasL()) {
142-
return JsonSerializationStrategy.LIST;
143-
}
144-
if (av.hasM()) {
145-
return JsonSerializationStrategy.MAP;
146-
}
147-
if (av.hasSs()) {
148-
return JsonSerializationStrategy.STRING_SET;
149-
}
150-
if (av.hasNs()) {
151-
return JsonSerializationStrategy.NUMBER_SET;
152-
}
153-
if (av.hasBs()) {
154-
return JsonSerializationStrategy.BYTES_SET;
155-
}
156-
throw new IllegalStateException("Unknown AttributeValue type: " + av);
157-
}
158121
}

‎services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/document/EnhancedDocumentTest.java‎

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static software.amazon.awssdk.enhanced.dynamodb.document.EnhancedDocumentTestData.testDataInstance;
2525

2626
import com.fasterxml.jackson.core.JsonProcessingException;
27+
import com.fasterxml.jackson.databind.JsonNode;
2728
import com.fasterxml.jackson.databind.ObjectMapper;
2829
import java.math.BigDecimal;
2930
import java.time.LocalDate;
@@ -52,6 +53,8 @@
5253

5354
class EnhancedDocumentTest {
5455

56+
ObjectMapper mapper = new ObjectMapper();
57+
5558
private static Stream<Arguments> escapeDocumentStrings() {
5659
char c = 0x0a;
5760
return Stream.of(
@@ -498,7 +501,7 @@ void error_when_usingClassGetPut_for_CollectionValues(){
498501
}
499502

500503
@Test
501-
void toJson_numberFormatting_veryLargeNumbers() {
504+
void toJson_numberFormatting_veryLargeNumbers() throws JsonProcessingException {
502505
EnhancedDocument doc = EnhancedDocument.builder()
503506
.attributeConverterProviders(defaultProvider())
504507
.putNumber("longMax", Long.MAX_VALUE)
@@ -509,104 +512,124 @@ void toJson_numberFormatting_veryLargeNumbers() {
509512
.build();
510513

511514
String json = doc.toJson();
512-
assertThat(json).isEqualTo("{\"longMax\":9223372036854775807,\"longMin\":-9223372036854775808,"
513-
+ "\"doubleMax\":1.7976931348623157E308,\"scientific\":1.23E100,"
514-
+ "\"manyDecimals\":1.123456789012345}");
515+
JsonNode actual = mapper.readTree(json);
516+
JsonNode expected = mapper.readTree("{\"longMax\":9223372036854775807,\"longMin\":-9223372036854775808,"
517+
+ "\"doubleMax\":1.7976931348623157E308,\"scientific\":1.23E100,"
518+
+ "\"manyDecimals\":1.123456789012345}");
519+
assertThat(expected).isEqualTo(actual);
515520
}
516521

517522
@Test
518-
void toJson_numberFormatting_trailingZeros() {
523+
void toJson_numberFormatting_trailingZeros() throws JsonProcessingException {
519524
EnhancedDocument doc = EnhancedDocument.builder()
520525
.attributeConverterProviders(defaultProvider())
521526
.putNumber("twoPointZero", 2.0)
522527
.putNumber("tenPointZeroZero", 10.00)
523528
.build();
524529

525530
String json = doc.toJson();
526-
assertThat(json).isEqualTo("{\"twoPointZero\":2.0,\"tenPointZeroZero\":10.0}");
531+
JsonNode actual = mapper.readTree(json);
532+
JsonNode expected = mapper.readTree("{\"twoPointZero\":2.0,\"tenPointZeroZero\":10.0}");
533+
assertThat(expected).isEqualTo(actual);
527534
}
528535

529536
@Test
530-
void toJson_stringEscaping_allControlCharacters() {
537+
void toJson_stringEscaping_allControlCharacters() throws JsonProcessingException {
531538
EnhancedDocument doc = EnhancedDocument.builder()
532539
.attributeConverterProviders(defaultProvider())
533540
.putString("allEscapes", "line1\nline2\ttab\"quote\\backslash\r\f")
534541
.build();
535542

536543
String json = doc.toJson();
537-
assertThat(json).isEqualTo("{\"allEscapes\":\"line1\\nline2\\ttab\\\"quote\\\\backslash\\r\\f\"}");
544+
JsonNode actual = mapper.readTree(json);
545+
JsonNode expected = mapper.readTree("{\"allEscapes\":\"line1\\nline2\\ttab\\\"quote\\\\backslash\\r\\f\"}");
546+
assertThat(expected).isEqualTo(actual);
538547
}
539548

540549
@Test
541-
void toJson_stringEscaping_forwardSlash() {
550+
void toJson_stringEscaping_forwardSlash() throws JsonProcessingException {
542551
EnhancedDocument doc = EnhancedDocument.builder()
543552
.attributeConverterProviders(defaultProvider())
544553
.putString("slash", "path/to/resource")
545554
.build();
546555

547556
String json = doc.toJson();
548-
assertThat(json).isEqualTo("{\"slash\":\"path/to/resource\"}");
557+
JsonNode actual = mapper.readTree(json);
558+
JsonNode expected = mapper.readTree("{\"slash\":\"path/to/resource\"}");
559+
assertThat(expected).isEqualTo(actual);
549560
}
550561

551562
@Test
552-
void toJson_emptyString() {
563+
void toJson_emptyString() throws JsonProcessingException {
553564
EnhancedDocument doc = EnhancedDocument.builder()
554565
.attributeConverterProviders(defaultProvider())
555566
.putString("empty", "")
556567
.build();
557568

558569
String json = doc.toJson();
559-
assertThat(json).isEqualTo("{\"empty\":\"\"}");
570+
JsonNode actual = mapper.readTree(json);
571+
JsonNode expected = mapper.readTree("{\"empty\":\"\"}");
572+
assertThat(expected).isEqualTo(actual);
560573
}
561574

562575
@Test
563-
void toJson_bytesEncoding_emptyBytes() {
576+
void toJson_bytesEncoding_emptyBytes() throws JsonProcessingException {
564577
EnhancedDocument doc = EnhancedDocument.builder()
565578
.attributeConverterProviders(defaultProvider())
566579
.putBytes("empty", SdkBytes.fromByteArray(new byte[0]))
567580
.build();
568581

569582
String json = doc.toJson();
570-
assertThat(json).isEqualTo("{\"empty\":\"\"}");
583+
JsonNode actual = mapper.readTree(json);
584+
JsonNode expected = mapper.readTree("{\"empty\":\"\"}");
585+
assertThat(expected).isEqualTo(actual);
571586
}
572587

573588
@Test
574-
void toJson_listWithAllNulls() {
589+
void toJson_listWithAllNulls() throws JsonProcessingException {
575590
EnhancedDocument doc = EnhancedDocument.builder()
576591
.attributeConverterProviders(defaultProvider())
577592
.putJson("nullList", "[null,null,null]")
578593
.build();
579594

580595
String json = doc.toJson();
581-
assertThat(json).isEqualTo("{\"nullList\":[null,null,null]}");
596+
JsonNode actual = mapper.readTree(json);
597+
JsonNode expected = mapper.readTree("{\"nullList\":[null,null,null]}");
598+
assertThat(expected).isEqualTo(actual);
582599
}
583600

584601
@Test
585-
void toJson_mapWithNullValues() {
602+
void toJson_mapWithNullValues() throws JsonProcessingException {
586603
EnhancedDocument doc = EnhancedDocument.builder()
587604
.attributeConverterProviders(defaultProvider())
588605
.putJson("nullValues", "{\"key1\":null,\"key2\":\"value\",\"key3\":null}")
589606
.build();
590607

591608
String json = doc.toJson();
592-
assertThat(json).isEqualTo("{\"nullValues\":{\"key1\":null,\"key2\":\"value\",\"key3\":null}}");
609+
JsonNode actual = mapper.readTree(json);
610+
JsonNode expected = mapper.readTree("{\"nullValues\":{\"key1\":null,\"key2\":\"value\",\"key3\":null}}");
611+
assertThat(expected).isEqualTo(actual);
593612
}
594613

595614
@Test
596-
void toJson_deeplyNestedStructure() {
597-
String deepJson = "{\"level1\":{\"level2\":{\"level3\":{\"level4\":{\"level5\":{\"level6\":{\"level7\":{\"value\":\"deep\"}}}}}}}}";
615+
void toJson_deeplyNestedStructure() throws JsonProcessingException {
616+
String deepJson = "{\"level1\":{\"level2\":{\"level3\":{\"level4\":"
617+
+ "{\"level5\":{\"level6\":{\"level7\":{\"value\":\"deep\"}}}}}}}}";
598618

599619
EnhancedDocument doc = EnhancedDocument.builder()
600620
.attributeConverterProviders(defaultProvider())
601621
.putJson("nested", deepJson)
602622
.build();
603623

604624
String json = doc.toJson();
605-
assertThat(json).isEqualTo("{\"nested\":" + deepJson + "}");
625+
626+
JsonNode actual = mapper.readTree(json);
627+
JsonNode expected = mapper.readTree("{\"nested\":" + deepJson + "}");
628+
assertThat(expected).isEqualTo(actual);
606629
}
607630

608631
@Test
609-
void toJson_deeplyNestedArrays() {
632+
void toJson_deeplyNestedArrays() throws JsonProcessingException {
610633
String deepArrayJson = "[[[[[[\"innermost\"]]]]]]";
611634

612635
EnhancedDocument doc = EnhancedDocument.builder()
@@ -615,7 +638,9 @@ void toJson_deeplyNestedArrays() {
615638
.build();
616639

617640
String json = doc.toJson();
618-
assertThat(json).isEqualTo("{\"nestedArrays\":" + deepArrayJson + "}");
641+
JsonNode actual = mapper.readTree(json);
642+
JsonNode expected = mapper.readTree("{\"nestedArrays\":" + deepArrayJson + "}");
643+
assertThat(expected).isEqualTo(actual);
619644
}
620645

621646
@Test

0 commit comments

Comments
 (0)