Skip to content

Commit f1e1ff7

Browse files
cowtowncodercowtowncoder
committed
Fix #3338 
1 parent 84ae854 commit f1e1ff7

File tree

4 files changed

+177
-7
lines changed

4 files changed

+177
-7
lines changed

‎release-notes/CREDITS-2.x

+4
Original file line numberDiff line numberDiff line change
@@ -1449,6 +1449,10 @@ ZeyuCai@github:
14491449
* Contributed #3314: Four Flaky Tests Detected in 2.14
14501450
(2.14.0)
14511451
1452+
Ernst-Jan van der Laan (ejl888@github)
1453+
* Reported #3338: `configOverride.setMergeable(false)` not supported by `ArrayNode`
1454+
(2.14.0)
1455+
14521456
Gary Morgan (morganga@github)
14531457
* Contributed #3419: Improve performance of `UnresolvedForwardReference` for
14541458
forward reference resolution

‎release-notes/VERSION-2.x

+2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ Project: jackson-databind
99
#2541: Cannot merge polymorphic objects
1010
(reported by Matthew A)
1111
(fix contributed by James W)
12+
#3338: `configOverride.setMergeable(false)` not supported by `ArrayNode`
13+
(requested by Ernst-Jan vdL)
1214
#3357: `@JsonIgnore` does not if together with `@JsonProperty` or `@JsonFormat`
1315
(reported by lizongbo@github)
1416
#3373: Change `TypeSerializerBase` to skip `generator.writeTypePrefix()`

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

+93-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import com.fasterxml.jackson.databind.*;
99
import com.fasterxml.jackson.databind.cfg.JsonNodeFeature;
10+
import com.fasterxml.jackson.databind.deser.ContextualDeserializer;
1011
import com.fasterxml.jackson.databind.jsontype.TypeDeserializer;
1112
import com.fasterxml.jackson.databind.node.*;
1213
import com.fasterxml.jackson.databind.type.LogicalType;
@@ -36,6 +37,17 @@ protected JsonNodeDeserializer() {
3637
super(JsonNode.class, null);
3738
}
3839

40+
protected JsonNodeDeserializer(JsonNodeDeserializer base,
41+
boolean mergeArrays, boolean mergeObjects) {
42+
super(base, mergeArrays, mergeObjects);
43+
}
44+
45+
@Override
46+
protected JsonDeserializer<?> _createWithMerge(boolean mergeArrays,
47+
boolean mergeObjects) {
48+
return new JsonNodeDeserializer(this, mergeArrays, mergeObjects);
49+
}
50+
3951
/**
4052
* Factory method for accessing deserializer for specific node type
4153
*/
@@ -95,6 +107,11 @@ public JsonNode deserialize(JsonParser p, DeserializationContext ctxt) throws IO
95107
return _deserializeAnyScalar(p, ctxt);
96108
}
97109

110+
@Override
111+
public Boolean supportsUpdate(DeserializationConfig config) {
112+
return _supportsUpdates;
113+
}
114+
98115
/*
99116
/**********************************************************************
100117
/* Specific instances for more accurate types
@@ -115,6 +132,17 @@ final static class ObjectDeserializer
115132

116133
public static ObjectDeserializer getInstance() { return _instance; }
117134

135+
protected ObjectDeserializer(ObjectDeserializer base,
136+
boolean mergeArrays, boolean mergeObjects) {
137+
super(base, mergeArrays, mergeObjects);
138+
}
139+
140+
@Override
141+
protected JsonDeserializer<?> _createWithMerge(boolean mergeArrays,
142+
boolean mergeObjects) {
143+
return new ObjectDeserializer(this, mergeArrays, mergeObjects);
144+
}
145+
118146
@Override
119147
public ObjectNode deserialize(JsonParser p, DeserializationContext ctxt) throws IOException
120148
{
@@ -166,6 +194,17 @@ final static class ArrayDeserializer
166194

167195
public static ArrayDeserializer getInstance() { return _instance; }
168196

197+
protected ArrayDeserializer(ArrayDeserializer base,
198+
boolean mergeArrays, boolean mergeObjects) {
199+
super(base, mergeArrays, mergeObjects);
200+
}
201+
202+
@Override
203+
protected JsonDeserializer<?> _createWithMerge(boolean mergeArrays,
204+
boolean mergeObjects) {
205+
return new ArrayDeserializer(this, mergeArrays, mergeObjects);
206+
}
207+
169208
@Override
170209
public ArrayNode deserialize(JsonParser p, DeserializationContext ctxt) throws IOException
171210
{
@@ -205,12 +244,27 @@ public ArrayNode deserialize(JsonParser p, DeserializationContext ctxt,
205244
@SuppressWarnings("serial")
206245
abstract class BaseNodeDeserializer<T extends JsonNode>
207246
extends StdDeserializer<T>
247+
implements ContextualDeserializer
208248
{
209249
protected final Boolean _supportsUpdates;
210250

251+
protected final boolean _mergeArrays;
252+
protected final boolean _mergeObjects;
253+
211254
public BaseNodeDeserializer(Class<T> vc, Boolean supportsUpdates) {
212255
super(vc);
213256
_supportsUpdates = supportsUpdates;
257+
_mergeArrays = true;
258+
_mergeObjects = true;
259+
}
260+
261+
protected BaseNodeDeserializer(BaseNodeDeserializer<?> base,
262+
boolean mergeArrays, boolean mergeObjects)
263+
{
264+
super(base);
265+
_supportsUpdates = base._supportsUpdates;
266+
_mergeArrays = mergeArrays;
267+
_mergeObjects = mergeObjects;
214268
}
215269

216270
@Override
@@ -238,6 +292,43 @@ public Boolean supportsUpdate(DeserializationConfig config) {
238292
return _supportsUpdates;
239293
}
240294

295+
@Override // @since 2.14
296+
public JsonDeserializer<?> createContextual(DeserializationContext ctxt,
297+
BeanProperty property)
298+
throws JsonMappingException
299+
{
300+
// 13-Jun-2022, tatu: Should we care about property? For now, let's not yet.
301+
// (merge info there accessible via "property.getMetadata().getMergeInfo()")
302+
final DeserializationConfig cfg = ctxt.getConfig();
303+
Boolean mergeArr = cfg.getDefaultMergeable(ArrayNode.class);
304+
Boolean mergeObj = cfg.getDefaultMergeable(ObjectNode.class);
305+
Boolean mergeNode = cfg.getDefaultMergeable(JsonNode.class);
306+
307+
final boolean mergeArrays = _shouldMerge(mergeArr, mergeNode);
308+
final boolean mergeObjects = _shouldMerge(mergeObj, mergeNode);
309+
310+
if ((mergeArrays != _mergeArrays)
311+
|| (mergeObjects != _mergeObjects)) {
312+
return _createWithMerge(mergeArrays, mergeObjects);
313+
}
314+
315+
return this;
316+
}
317+
318+
private static boolean _shouldMerge(Boolean specificMerge, Boolean generalMerge) {
319+
if (specificMerge != null) {
320+
return specificMerge.booleanValue();
321+
}
322+
if (generalMerge != null) {
323+
return generalMerge.booleanValue();
324+
}
325+
return true;
326+
}
327+
328+
// @since 2.14
329+
protected abstract JsonDeserializer<?> _createWithMerge(boolean mergeArrays,
330+
boolean mergeObjects);
331+
241332
/*
242333
/**********************************************************************
243334
/* Duplicate handling
@@ -359,7 +450,7 @@ protected final JsonNode updateObject(JsonParser p, DeserializationContext ctxt,
359450
if (old instanceof ObjectNode) {
360451
// [databind#3056]: merging only if had Object and
361452
// getting an Object
362-
if (t == JsonToken.START_OBJECT) {
453+
if ((t == JsonToken.START_OBJECT) && _mergeObjects) {
363454
JsonNode newValue = updateObject(p, ctxt, (ObjectNode) old, stack);
364455
if (newValue != old) {
365456
node.set(key, newValue);
@@ -369,7 +460,7 @@ protected final JsonNode updateObject(JsonParser p, DeserializationContext ctxt,
369460
} else if (old instanceof ArrayNode) {
370461
// [databind#3056]: related to Object handling, ensure
371462
// Array values also match for mergeability
372-
if (t == JsonToken.START_ARRAY) {
463+
if ((t == JsonToken.START_ARRAY) && _mergeArrays) {
373464
// 28-Mar-2021, tatu: We'll only append entries so not very different
374465
// from "regular" deserializeArray...
375466
_deserializeContainerNoRecursion(p, ctxt, nodeFactory,

‎src/test/java/com/fasterxml/jackson/failing/ArrayNode3338MergeTest.java

+78-5
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ public class ArrayNode3338MergeTest extends BaseMapTest
1010
public void testEnabledArrayNodeMerge() throws Exception {
1111
final ObjectMapper mapperWithMerge = sharedMapper();
1212

13-
JsonNode merged = _updateTree(mapperWithMerge);
13+
JsonNode merged = _updateTreeWithArray(mapperWithMerge);
1414

1515
ObjectNode expected = mapperWithMerge.createObjectNode();
1616
expected.put("number", 888);
@@ -26,23 +26,32 @@ public void testEnabledArrayNodeMerge() throws Exception {
2626
}
2727

2828
public void testDisabledArrayNodeMerge() throws Exception {
29-
ObjectMapper mapperNoMerge = jsonMapperBuilder()
29+
ObjectMapper mapperNoArrayMerge = jsonMapperBuilder()
3030
.withConfigOverride(ArrayNode.class,
3131
cfg -> cfg.setMergeable(false))
3232
.build();
3333

34-
JsonNode merged = _updateTree(mapperNoMerge);
34+
JsonNode merged = _updateTreeWithArray(mapperNoArrayMerge);
3535

36-
ObjectNode expected = mapperNoMerge.createObjectNode();
36+
ObjectNode expected = mapperNoArrayMerge.createObjectNode();
3737
ArrayNode array = expected.putArray("array");
3838
array.add("Mister");
3939
array.add("Miss");
4040
expected.put("number", 888);
4141

4242
assertEquals(expected, merged);
43+
44+
// Also should work via JsonNode
45+
ObjectMapper mapperNoJsonNodeMerge = jsonMapperBuilder()
46+
.withConfigOverride(JsonNode.class,
47+
cfg -> cfg.setMergeable(false))
48+
.build();
49+
50+
JsonNode merged2 = _updateTreeWithArray(mapperNoJsonNodeMerge);
51+
assertEquals(expected, merged2);
4352
}
4453

45-
private JsonNode _updateTree(ObjectMapper mapper) throws Exception
54+
private JsonNode _updateTreeWithArray(ObjectMapper mapper) throws Exception
4655
{
4756
JsonNode mergeTarget = mapper.readTree(a2q("{"
4857
+ "'array': ['Mr.', 'Ms.' ],"
@@ -53,4 +62,68 @@ private JsonNode _updateTree(ObjectMapper mapper) throws Exception
5362
+ "}"));
5463
return mapper.readerForUpdating(mergeTarget).readValue(updateNode);
5564
}
65+
66+
public void testEnabledObjectNodeMerge() throws Exception {
67+
final ObjectMapper mapperWithMerge = sharedMapper();
68+
69+
JsonNode merged = _updateTreeWithObject(mapperWithMerge);
70+
71+
// default behavior is to enable merge:
72+
ObjectNode expected = mapperWithMerge.createObjectNode();
73+
ObjectNode obj = expected.putObject("object");
74+
obj.put("a", "1");
75+
obj.put("b", "xyz");
76+
77+
expected.put("number", 42);
78+
ArrayNode array = expected.putArray("array");
79+
array.add(1);
80+
array.add(2);
81+
array.add(3);
82+
83+
assertEquals(expected, merged);
84+
}
85+
86+
public void testDisabledObjectNodeMerge() throws Exception {
87+
ObjectMapper mapperNoObjectMerge = jsonMapperBuilder()
88+
.withConfigOverride(ObjectNode.class,
89+
cfg -> cfg.setMergeable(false))
90+
.build();
91+
92+
JsonNode merged = _updateTreeWithObject(mapperNoObjectMerge);
93+
94+
// but that can be disabled
95+
ObjectNode expected = mapperNoObjectMerge.createObjectNode();
96+
ObjectNode obj = expected.putObject("object");
97+
obj.put("b", "xyz");
98+
99+
expected.put("number", 42);
100+
ArrayNode array = expected.putArray("array");
101+
array.add(1);
102+
array.add(2);
103+
array.add(3);
104+
105+
assertEquals(expected, merged);
106+
107+
// Also: verify that `JsonNode` target also works:
108+
ObjectMapper mapperNoJsonNodeMerge = jsonMapperBuilder()
109+
.withConfigOverride(JsonNode.class,
110+
cfg -> cfg.setMergeable(false))
111+
.build();
112+
113+
JsonNode merged2 = _updateTreeWithObject(mapperNoJsonNodeMerge);
114+
assertEquals(expected, merged2);
115+
}
116+
117+
private JsonNode _updateTreeWithObject(ObjectMapper mapper) throws Exception
118+
{
119+
JsonNode mergeTarget = mapper.readTree(a2q("{"
120+
+ "'object': {'a':'1', 'b':'2' },"
121+
+ "'array': [1, 2, 3],"
122+
+ "'number': 42"
123+
+ "}"));
124+
JsonNode updateNode = mapper.readTree(a2q("{"
125+
+ "'object': {'b':'xyz'}"
126+
+ "}"));
127+
return mapper.readerForUpdating(mergeTarget).readValue(updateNode);
128+
}
56129
}

0 commit comments

Comments
 (0)