Skip to content

Commit ff20bf5

Browse files
BenWhiteheadBenWhitehead
committed
chore: fix xml parsing of StringEnumValue's so that the enum contract is not broken
1 parent 3869259 commit ff20bf5

File tree

3 files changed

+204
-26
lines changed

3 files changed

+204
-26
lines changed

‎google-cloud-storage/src/main/java/com/google/cloud/storage/StorageClass.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package com.google.cloud.storage;
1717

18-
import com.fasterxml.jackson.annotation.JsonCreator;
1918
import com.google.api.core.ApiFunction;
2019
import com.google.cloud.StringEnumType;
2120
import com.google.cloud.StringEnumValue;
@@ -112,11 +111,7 @@ public static StorageClass valueOfStrict(String constant) {
112111
}
113112

114113
/** Get the StorageClass for the given String constant, and allow unrecognized values. */
115-
@JsonCreator
116114
public static StorageClass valueOf(String constant) {
117-
if (constant == null || constant.isEmpty()) {
118-
return null;
119-
}
120115
return type.valueOf(constant);
121116
}
122117

‎google-cloud-storage/src/main/java/com/google/cloud/storage/XmlObjectParser.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,31 @@
1515
*/
1616
package com.google.cloud.storage;
1717

18+
import com.fasterxml.jackson.core.JacksonException;
19+
import com.fasterxml.jackson.core.JsonGenerator;
20+
import com.fasterxml.jackson.core.JsonParser;
21+
import com.fasterxml.jackson.core.Version;
22+
import com.fasterxml.jackson.databind.DeserializationContext;
23+
import com.fasterxml.jackson.databind.Module;
24+
import com.fasterxml.jackson.databind.SerializerProvider;
25+
import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
26+
import com.fasterxml.jackson.databind.module.SimpleDeserializers;
27+
import com.fasterxml.jackson.databind.module.SimpleSerializers;
28+
import com.fasterxml.jackson.databind.ser.std.StdSerializer;
1829
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
1930
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
2031
import com.google.api.client.util.ObjectParser;
32+
import com.google.cloud.StringEnumValue;
2133
import com.google.common.annotations.VisibleForTesting;
34+
import com.google.common.collect.ImmutableList;
35+
import com.google.common.collect.ImmutableMap;
2236
import java.io.IOException;
2337
import java.io.InputStream;
2438
import java.io.InputStreamReader;
2539
import java.io.Reader;
2640
import java.lang.reflect.Type;
2741
import java.nio.charset.Charset;
42+
import java.util.function.Function;
2843

2944
final class XmlObjectParser implements ObjectParser {
3045
private final XmlMapper xmlMapper;
@@ -33,6 +48,31 @@ final class XmlObjectParser implements ObjectParser {
3348
public XmlObjectParser(XmlMapper xmlMapper) {
3449
this.xmlMapper = xmlMapper;
3550
this.xmlMapper.registerModule(new JavaTimeModule());
51+
this.xmlMapper.registerModule(
52+
new Module() {
53+
@Override
54+
public String getModuleName() {
55+
return this.getClass().getPackage().getName();
56+
}
57+
58+
@Override
59+
public Version version() {
60+
return Version.unknownVersion();
61+
}
62+
63+
@Override
64+
public void setupModule(SetupContext context) {
65+
context.addSerializers(
66+
new SimpleSerializers(
67+
ImmutableList.of(new StringEnumValueSerializer<>(StorageClass.class))));
68+
context.addDeserializers(
69+
new SimpleDeserializers(
70+
ImmutableMap.of(
71+
StorageClass.class,
72+
new StringEnumValueDeserializer<>(
73+
StorageClass.class, StorageClass::valueOf))));
74+
}
75+
});
3676
}
3777

3878
@Override
@@ -62,4 +102,39 @@ public Object parseAndClose(Reader reader, Type dataType) throws IOException {
62102
"XmlObjectParse#"
63103
+ CrossTransportUtils.fmtMethodName("parseAndClose", Reader.class, Type.class));
64104
}
105+
106+
private static final class StringEnumValueDeserializer<E extends StringEnumValue>
107+
extends StdDeserializer<E> {
108+
109+
private final Function<String, E> constructor;
110+
111+
private StringEnumValueDeserializer(Class<E> cl, Function<String, E> constructor) {
112+
super(cl);
113+
this.constructor = constructor;
114+
}
115+
116+
@Override
117+
public E deserialize(JsonParser p, DeserializationContext ctxt)
118+
throws IOException, JacksonException {
119+
String s = p.readValueAs(String.class);
120+
if (s == null || s.trim().isEmpty()) {
121+
return null;
122+
}
123+
return constructor.apply(s);
124+
}
125+
}
126+
127+
private static final class StringEnumValueSerializer<E extends StringEnumValue>
128+
extends StdSerializer<E> {
129+
130+
private StringEnumValueSerializer(Class<E> cl) {
131+
super(cl);
132+
}
133+
134+
@Override
135+
public void serialize(E value, JsonGenerator gen, SerializerProvider provider)
136+
throws IOException {
137+
gen.writeString(value.name());
138+
}
139+
}
65140
}

‎google-cloud-storage/src/test/java/com/google/cloud/storage/XmlObjectParserTest.java‎

Lines changed: 129 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,48 +17,156 @@
1717
package com.google.cloud.storage;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20-
import static org.mockito.ArgumentMatchers.any;
21-
import static org.mockito.Mockito.when;
2220

2321
import com.fasterxml.jackson.dataformat.xml.XmlMapper;
22+
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty;
23+
import com.google.cloud.storage.multipartupload.model.ListPartsResponse;
24+
import com.google.common.base.MoreObjects;
2425
import java.io.ByteArrayInputStream;
2526
import java.io.IOException;
2627
import java.io.InputStream;
27-
import java.io.Reader;
28+
import java.io.StringReader;
2829
import java.nio.charset.StandardCharsets;
29-
import org.junit.After;
30+
import java.util.Objects;
3031
import org.junit.Before;
3132
import org.junit.Test;
32-
import org.mockito.Mock;
33-
import org.mockito.MockitoAnnotations;
3433

3534
public class XmlObjectParserTest {
3635

37-
@Mock private XmlMapper xmlMapper;
38-
39-
private AutoCloseable mocks;
4036
private XmlObjectParser xmlObjectParser;
4137

4238
@Before
4339
public void setUp() {
44-
mocks = MockitoAnnotations.openMocks(this);
45-
xmlObjectParser = new XmlObjectParser(xmlMapper);
40+
xmlObjectParser = new XmlObjectParser(new XmlMapper());
41+
}
42+
43+
@Test
44+
public void testParseStringValueEnum() throws IOException {
45+
// language=xml
46+
String xml =
47+
"<TestXmlObject2>\n" + " <storageClass>STANDARD</storageClass>" + "</TestXmlObject2>";
48+
InputStream in = new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8));
49+
TestXmlObject2 expected = new TestXmlObject2(StorageClass.STANDARD);
50+
TestXmlObject2 actual =
51+
xmlObjectParser.parseAndClose(in, StandardCharsets.UTF_8, TestXmlObject2.class);
52+
assertThat(actual).isEqualTo(expected);
4653
}
4754

48-
@After
49-
public void tearDown() throws Exception {
50-
mocks.close();
55+
@Test
56+
public void testNestedParseStringValueEnum_undefined() throws IOException {
57+
// language=xml
58+
String xml =
59+
"<ListPartsResponse>\n"
60+
+ " <truncated>false</truncated>\n"
61+
+ " <Bucket>bucket</Bucket>\n"
62+
+ " <Key>key</Key>\n"
63+
+ " <UploadId/>\n"
64+
+ " <PartNumberMarker>0</PartNumberMarker>\n"
65+
+ " <NextPartNumberMarker>0</NextPartNumberMarker>\n"
66+
+ " <MaxParts>0</MaxParts>\n"
67+
+ " <IsTruncated>false</IsTruncated>\n"
68+
+ " <Owner/>\n"
69+
+ " <Part>\n"
70+
+ " <PartNumber>1</PartNumber>\n"
71+
+ " <ETag>etag</ETag>\n"
72+
+ " <Size>33</Size>\n"
73+
+ " <LastModified/>\n"
74+
+ " </Part>\n"
75+
+ "</ListPartsResponse>";
76+
System.out.println(xml);
77+
ListPartsResponse listPartsResponse =
78+
xmlObjectParser.parseAndClose(new StringReader(xml), ListPartsResponse.class);
79+
assertThat(listPartsResponse.getStorageClass()).isNull();
80+
}
81+
82+
@Test
83+
public void testNestedParseStringValueEnum_null() throws IOException {
84+
// language=xml
85+
String xml =
86+
"<ListPartsResponse>\n"
87+
+ " <truncated>false</truncated>\n"
88+
+ " <Bucket>bucket</Bucket>\n"
89+
+ " <Key>key</Key>\n"
90+
+ " <UploadId/>\n"
91+
+ " <PartNumberMarker>0</PartNumberMarker>\n"
92+
+ " <NextPartNumberMarker>0</NextPartNumberMarker>\n"
93+
+ " <MaxParts>0</MaxParts>\n"
94+
+ " <IsTruncated>false</IsTruncated>\n"
95+
+ " <Owner/>\n"
96+
+ " <StorageClass/>"
97+
+ " <Part>\n"
98+
+ " <PartNumber>1</PartNumber>\n"
99+
+ " <ETag>etag</ETag>\n"
100+
+ " <Size>33</Size>\n"
101+
+ " <LastModified/>\n"
102+
+ " </Part>\n"
103+
+ "</ListPartsResponse>";
104+
System.out.println(xml);
105+
ListPartsResponse listPartsResponse =
106+
xmlObjectParser.parseAndClose(new StringReader(xml), ListPartsResponse.class);
107+
assertThat(listPartsResponse.getStorageClass()).isNull();
51108
}
52109

53110
@Test
54-
public void testParseAndClose() throws IOException {
55-
InputStream in = new ByteArrayInputStream("<Test/>".getBytes(StandardCharsets.UTF_8));
56-
TestXmlObject expected = new TestXmlObject();
57-
when(xmlMapper.readValue(any(Reader.class), any(Class.class))).thenReturn(expected);
58-
TestXmlObject actual =
59-
xmlObjectParser.parseAndClose(in, StandardCharsets.UTF_8, TestXmlObject.class);
60-
assertThat(actual).isSameInstanceAs(expected);
111+
public void testNestedParseStringValueEnum_nonNull() throws IOException {
112+
// language=xml
113+
String xml =
114+
"<ListPartsResponse>\n"
115+
+ " <truncated>false</truncated>\n"
116+
+ " <Bucket>bucket</Bucket>\n"
117+
+ " <Key>key</Key>\n"
118+
+ " <UploadId/>\n"
119+
+ " <PartNumberMarker>0</PartNumberMarker>\n"
120+
+ " <NextPartNumberMarker>0</NextPartNumberMarker>\n"
121+
+ " <MaxParts>0</MaxParts>\n"
122+
+ " <IsTruncated>false</IsTruncated>\n"
123+
+ " <Owner/>\n"
124+
+ " <StorageClass>STANDARD</StorageClass>"
125+
+ " <Part>\n"
126+
+ " <PartNumber>1</PartNumber>\n"
127+
+ " <ETag>etag</ETag>\n"
128+
+ " <Size>33</Size>\n"
129+
+ " <LastModified/>\n"
130+
+ " </Part>\n"
131+
+ "</ListPartsResponse>";
132+
System.out.println(xml);
133+
ListPartsResponse listPartsResponse =
134+
xmlObjectParser.parseAndClose(new StringReader(xml), ListPartsResponse.class);
135+
assertThat(listPartsResponse.getStorageClass()).isEqualTo(StorageClass.STANDARD);
61136
}
62137

63138
private static class TestXmlObject {}
139+
140+
private static final class TestXmlObject2 {
141+
@JacksonXmlProperty(localName = "storageClass")
142+
private StorageClass storageClass;
143+
144+
private TestXmlObject2() {}
145+
146+
public TestXmlObject2(StorageClass storageClass) {
147+
this.storageClass = storageClass;
148+
}
149+
150+
@Override
151+
public boolean equals(Object o) {
152+
if (this == o) {
153+
return true;
154+
}
155+
if (!(o instanceof TestXmlObject2)) {
156+
return false;
157+
}
158+
TestXmlObject2 that = (TestXmlObject2) o;
159+
return Objects.equals(storageClass, that.storageClass);
160+
}
161+
162+
@Override
163+
public int hashCode() {
164+
return Objects.hashCode(storageClass);
165+
}
166+
167+
@Override
168+
public String toString() {
169+
return MoreObjects.toStringHelper(this).add("storageClass", storageClass).toString();
170+
}
171+
}
64172
}

0 commit comments

Comments
 (0)