Skip to content

Commit 2e06068

Browse files
carterkozakcarterkozak
authored
Test coverage and fix for ACCEPT_SINGLE_VALUE_AS_ARRAY (#2922)
This fixes a regresion introduced by fb72f93 which caused unexpected interactions between ACCEPT_SINGLE_VALUE_AS_ARRAY and handling for ACCEPT_EMPTY_STRING_AS_NULL_OBJECT, causing unwrapped string values to fail deserialization.
1 parent ecd23e2 commit 2e06068

File tree

3 files changed

+130
-3
lines changed

3 files changed

+130
-3
lines changed

‎src/main/java/com/fasterxml/jackson/databind/deser/std/CollectionDeserializer.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
import com.fasterxml.jackson.databind.*;
1111
import com.fasterxml.jackson.databind.annotation.JacksonStdImpl;
12+
import com.fasterxml.jackson.databind.cfg.CoercionAction;
13+
import com.fasterxml.jackson.databind.cfg.CoercionInputShape;
1214
import com.fasterxml.jackson.databind.deser.*;
1315
import com.fasterxml.jackson.databind.deser.impl.ReadableObjectId.Referring;
1416
import com.fasterxml.jackson.databind.jsontype.TypeDeserializer;
@@ -246,7 +248,23 @@ public Collection<Object> deserialize(JsonParser p, DeserializationContext ctxt)
246248
// there is also possibility of "auto-wrapping" of single-element arrays.
247249
// Hence we only accept empty String here.
248250
if (p.hasToken(JsonToken.VALUE_STRING)) {
249-
return _deserializeFromString(p, ctxt);
251+
// 05-Nov-2020, ckozak: As per [jackson-databind#2922] string values may be handled
252+
// using handleNonArray, however empty strings may result in a null or empty collection
253+
// depending on configuration.
254+
final CoercionAction act = ctxt.findCoercionAction(logicalType(), handledType(),
255+
CoercionInputShape.EmptyString);
256+
// 05-Nov-2020, ckozak: Unclear if TryConvert should return the default
257+
// conversion (null) or fall through to handleNonArray.
258+
if (act != null
259+
// handleNonArray may successfully deserialize the result (if
260+
// ACCEPT_SINGLE_VALUE_AS_ARRAY is enabled, for example) otherwise it
261+
// is capable of failing just as well as _deserializeFromEmptyString.
262+
&& act != CoercionAction.Fail
263+
// getValueAsString call is ordered last to avoid unnecessarily building a string value.
264+
&& p.getValueAsString().isEmpty()) {
265+
return (Collection<Object>) _deserializeFromEmptyString(
266+
p, ctxt, act, handledType(), "empty String (\"\")");
267+
}
250268
}
251269
return handleNonArray(p, ctxt, createDefaultInstance(ctxt));
252270
}

‎src/test/java/com/fasterxml/jackson/databind/convert/CoerceContainersTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ private void _verifyNoCoercion(JavaType targetType) throws Exception {
161161
fail("Should not pass");
162162
} catch (Exception e) {
163163
verifyException(e, "Cannot deserialize value of type");
164-
verifyException(e, "from empty String");
164+
verifyException(e, "from empty String", "from String value (token `JsonToken.VALUE_STRING`)");
165165
}
166166
}
167167

‎src/test/java/com/fasterxml/jackson/databind/struct/FormatFeatureAcceptSingleTest.java

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
package com.fasterxml.jackson.databind.struct;
22

3+
import java.util.ArrayList;
4+
import java.util.Collections;
35
import java.util.EnumSet;
46
import java.util.List;
57

8+
import com.fasterxml.jackson.annotation.JsonCreator;
69
import com.fasterxml.jackson.annotation.JsonFormat;
10+
import com.fasterxml.jackson.annotation.JsonProperty;
711
import com.fasterxml.jackson.databind.BaseMapTest;
812
import com.fasterxml.jackson.databind.ObjectMapper;
13+
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
914

1015
public class FormatFeatureAcceptSingleTest extends BaseMapTest
1116
{
@@ -51,6 +56,33 @@ static class StringListWrapper {
5156
public List<String> values;
5257
}
5358

59+
@JsonDeserialize(builder = StringListWrapperWithBuilder.Builder.class)
60+
static class StringListWrapperWithBuilder {
61+
public final List<String> values;
62+
63+
private StringListWrapperWithBuilder(List<String> values) {
64+
this.values = values;
65+
}
66+
67+
static class Builder {
68+
private List<String> values = Collections.emptyList();
69+
70+
@JsonProperty
71+
@JsonFormat(with = JsonFormat.Feature.ACCEPT_SINGLE_VALUE_AS_ARRAY)
72+
public Builder values(Iterable<? extends String> elements) {
73+
values = new ArrayList<>();
74+
for (String value : elements) {
75+
values.add(value);
76+
}
77+
return this;
78+
}
79+
80+
public StringListWrapperWithBuilder build() {
81+
return new StringListWrapperWithBuilder(values);
82+
}
83+
}
84+
}
85+
5486
static class EnumSetWrapper {
5587
@JsonFormat(with = JsonFormat.Feature.ACCEPT_SINGLE_VALUE_AS_ARRAY)
5688
public EnumSet<ABC> values;
@@ -66,11 +98,60 @@ static class RolesInList {
6698
public List<Role> roles;
6799
}
68100

101+
@JsonDeserialize(builder = RolesInListWithBuilder.Builder.class)
102+
static class RolesInListWithBuilder {
103+
public final List<Role> roles;
104+
105+
private RolesInListWithBuilder(List<Role> roles) {
106+
this.roles = roles;
107+
}
108+
109+
static class Builder {
110+
private List<Role> values = Collections.emptyList();
111+
112+
@JsonProperty
113+
@JsonFormat(with = JsonFormat.Feature.ACCEPT_SINGLE_VALUE_AS_ARRAY)
114+
public Builder roles(Iterable<? extends Role> elements) {
115+
values = new ArrayList<>();
116+
for (Role value : elements) {
117+
values.add(value);
118+
}
119+
return this;
120+
}
121+
122+
public RolesInListWithBuilder build() {
123+
return new RolesInListWithBuilder(values);
124+
}
125+
}
126+
}
127+
128+
static class WrapperWithStringFactoryInList {
129+
@JsonFormat(with = JsonFormat.Feature.ACCEPT_SINGLE_VALUE_AS_ARRAY)
130+
public List<WrapperWithStringFactory> values;
131+
}
132+
69133
static class Role {
70134
public String ID;
71135
public String Name;
72136
}
73137

138+
@JsonDeserialize
139+
static class WrapperWithStringFactory {
140+
private final Role role;
141+
142+
private WrapperWithStringFactory(Role role) {
143+
this.role = role;
144+
}
145+
146+
@JsonCreator
147+
static WrapperWithStringFactory from(String value) {
148+
Role role = new Role();
149+
role.ID = "1";
150+
role.Name = value;
151+
return new WrapperWithStringFactory(role);
152+
}
153+
}
154+
74155
private final ObjectMapper MAPPER = new ObjectMapper();
75156

76157
/*
@@ -150,7 +231,7 @@ public void testSingleElementArrayRead() throws Exception {
150231
assertEquals(1, response.roles.length);
151232
assertEquals("333", response.roles[0].ID);
152233
}
153-
234+
154235
public void testSingleStringListRead() throws Exception {
155236
String json = aposToQuotes(
156237
"{ 'values': 'first' }");
@@ -160,6 +241,16 @@ public void testSingleStringListRead() throws Exception {
160241
assertEquals("first", result.values.get(0));
161242
}
162243

244+
public void testSingleStringListReadWithBuilder() throws Exception {
245+
String json = aposToQuotes(
246+
"{ 'values': 'first' }");
247+
StringListWrapperWithBuilder result =
248+
MAPPER.readValue(json, StringListWrapperWithBuilder.class);
249+
assertNotNull(result.values);
250+
assertEquals(1, result.values.size());
251+
assertEquals("first", result.values.get(0));
252+
}
253+
163254
public void testSingleElementListRead() throws Exception {
164255
String json = aposToQuotes(
165256
"{ 'roles': { 'Name': 'User', 'ID': '333' } }");
@@ -169,6 +260,24 @@ public void testSingleElementListRead() throws Exception {
169260
assertEquals("333", response.roles.get(0).ID);
170261
}
171262

263+
public void testSingleElementListReadWithBuilder() throws Exception {
264+
String json = aposToQuotes(
265+
"{ 'roles': { 'Name': 'User', 'ID': '333' } }");
266+
RolesInListWithBuilder response = MAPPER.readValue(json, RolesInListWithBuilder.class);
267+
assertNotNull(response.roles);
268+
assertEquals(1, response.roles.size());
269+
assertEquals("333", response.roles.get(0).ID);
270+
}
271+
272+
public void testSingleElementWithStringFactoryRead() throws Exception {
273+
String json = aposToQuotes(
274+
"{ 'values': '333' }");
275+
WrapperWithStringFactoryInList response = MAPPER.readValue(json, WrapperWithStringFactoryInList.class);
276+
assertNotNull(response.values);
277+
assertEquals(1, response.values.size());
278+
assertEquals("333", response.values.get(0).role.Name);
279+
}
280+
172281
public void testSingleEnumSetRead() throws Exception {
173282
EnumSetWrapper result = MAPPER.readValue(aposToQuotes("{ 'values': 'B' }"),
174283
EnumSetWrapper.class);

0 commit comments

Comments
 (0)