-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix #4680 : Custom key deserialiser registered for Object.class
is ignored on nested JSON
#4684
base: 2.19
Are you sure you want to change the base?
Changes from 1 commit
f3d38b0
e832d13
06f8b83
a1fe9cc
9222ade
6e04144
149180c
03b365a
f91f0a3
8976fef
e80b337
1f7dfbc
874dd82
39aa9a1
672c69e
2792f28
83af378
c09b191
517edba
ef0e18c
1a0cdbf
212bc64
3c35bcd
f58cdd6
d48f5f6
2989044
b32f234
95e7887
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
import com.fasterxml.jackson.databind.deser.ResolvableDeserializer; | ||
import com.fasterxml.jackson.databind.jsontype.TypeDeserializer; | ||
import com.fasterxml.jackson.databind.type.LogicalType; | ||
import com.fasterxml.jackson.databind.type.SimpleType; | ||
import com.fasterxml.jackson.databind.type.TypeFactory; | ||
import com.fasterxml.jackson.databind.util.ClassUtil; | ||
import com.fasterxml.jackson.databind.util.ObjectBuffer; | ||
|
@@ -48,6 +49,13 @@ public class UntypedObjectDeserializer | |
|
||
protected JsonDeserializer<Object> _numberDeserializer; | ||
|
||
/** | ||
* Object.class may also have custom key deserializer | ||
* | ||
* @since 2.19 | ||
*/ | ||
private KeyDeserializer _customKeyDeserializer; | ||
|
||
/** | ||
* If {@link java.util.List} has been mapped to non-default implementation, | ||
* we'll store type here | ||
|
@@ -74,7 +82,7 @@ public class UntypedObjectDeserializer | |
*/ | ||
@Deprecated | ||
public UntypedObjectDeserializer() { | ||
this(null, null); | ||
this(null, (JavaType) null); | ||
} | ||
|
||
public UntypedObjectDeserializer(JavaType listType, JavaType mapType) { | ||
|
@@ -96,6 +104,7 @@ public UntypedObjectDeserializer(UntypedObjectDeserializer base, | |
_numberDeserializer = (JsonDeserializer<Object>) numberDeser; | ||
_listType = base._listType; | ||
_mapType = base._mapType; | ||
_customKeyDeserializer = base._customKeyDeserializer; | ||
_nonMerging = base._nonMerging; | ||
} | ||
|
||
|
@@ -112,9 +121,27 @@ protected UntypedObjectDeserializer(UntypedObjectDeserializer base, | |
_numberDeserializer = base._numberDeserializer; | ||
_listType = base._listType; | ||
_mapType = base._mapType; | ||
_customKeyDeserializer = base._customKeyDeserializer; | ||
_nonMerging = nonMerging; | ||
} | ||
|
||
/** | ||
* @since 2.19 | ||
*/ | ||
protected UntypedObjectDeserializer(UntypedObjectDeserializer base, | ||
KeyDeserializer keyDeser) | ||
{ | ||
super(Object.class); | ||
_mapDeserializer = base._mapDeserializer; | ||
_listDeserializer = base._listDeserializer; | ||
_stringDeserializer = base._stringDeserializer; | ||
_numberDeserializer = base._numberDeserializer; | ||
_listType = base._listType; | ||
_mapType = base._mapType; | ||
_nonMerging = base._nonMerging; | ||
_customKeyDeserializer = keyDeser; | ||
} | ||
|
||
/* | ||
/********************************************************** | ||
/* Initialization | ||
|
@@ -191,18 +218,32 @@ public JsonDeserializer<?> createContextual(DeserializationContext ctxt, | |
// 14-Jun-2017, tatu: [databind#1625]: may want to block merging, for root value | ||
boolean preventMerge = (property == null) | ||
&& Boolean.FALSE.equals(ctxt.getConfig().getDefaultMergeable(Object.class)); | ||
// 31-Aug-2024: [databind#4680] Allow custom key deserializer for Object.class deserialization | ||
KeyDeserializer keyDeser = ctxt.findKeyDeserializer(SimpleType.constructUnsafe(Object.class), property); | ||
// 20-Apr-2014, tatu: If nothing custom, let's use "vanilla" instance, | ||
// simpler and can avoid some of delegation | ||
if ((_stringDeserializer == null) && (_numberDeserializer == null) | ||
&& (_mapDeserializer == null) && (_listDeserializer == null) | ||
&& getClass() == UntypedObjectDeserializer.class) { | ||
return UntypedObjectDeserializerNR.instance(preventMerge); | ||
if (keyDeser == null) { | ||
return UntypedObjectDeserializerNR.instance(preventMerge); | ||
} | ||
} | ||
|
||
UntypedObjectDeserializer untyped = null; | ||
if (preventMerge != _nonMerging) { | ||
return new UntypedObjectDeserializer(this, preventMerge); | ||
untyped = new UntypedObjectDeserializer(this, preventMerge); | ||
} | ||
if (keyDeser != null) { | ||
if (untyped == null) { | ||
untyped = new UntypedObjectDeserializer(this, keyDeser); | ||
} else { | ||
untyped = new UntypedObjectDeserializer(untyped, keyDeser); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cowtowncoder This is what I came up with atm. I was hoping maybe you might think of better approach than this? 😅 First, null/non-null check for |
||
if (untyped != null) { | ||
return untyped; | ||
} | ||
|
||
return this; | ||
} | ||
|
||
|
@@ -496,6 +537,8 @@ protected Object mapObject(JsonParser p, DeserializationContext ctxt) throws IOE | |
if (key1 == null) { | ||
// empty map might work; but caller may want to modify... so better just give small modifiable | ||
return new LinkedHashMap<>(2); | ||
} else { | ||
key1 = (String) _customKeyDeserializer.deserializeKey(key1, ctxt); | ||
} | ||
// minor optimization; let's handle 1 and 2 entry cases separately | ||
// 24-Mar-2015, tatu: Ideally, could use one of 'nextXxx()' methods, but for | ||
|
@@ -508,6 +551,8 @@ protected Object mapObject(JsonParser p, DeserializationContext ctxt) throws IOE | |
LinkedHashMap<String, Object> result = new LinkedHashMap<>(2); | ||
result.put(key1, value1); | ||
return result; | ||
} else { | ||
key2 = (String) _customKeyDeserializer.deserializeKey(key2, ctxt); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @JooHyukKim I think same check needed for 2 cases further down too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, seems like it. Will do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Applied ur review and did minor plummeting (with a new helper function) via 1f7dfbc. Thanks! |
||
} | ||
p.nextToken(); | ||
Object value2 = deserialize(p, ctxt); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
package com.fasterxml.jackson.databind.deser; | ||
|
||
import java.util.Map; | ||
|
||
import com.fasterxml.jackson.core.type.TypeReference; | ||
import com.fasterxml.jackson.databind.DeserializationContext; | ||
import com.fasterxml.jackson.databind.KeyDeserializer; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.databind.json.JsonMapper; | ||
import com.fasterxml.jackson.databind.module.SimpleModule; | ||
|
||
import org.junit.jupiter.api.Assertions; | ||
import org.junit.jupiter.api.Test; | ||
|
||
public class CustomKeyDeserializer4680Test | ||
{ | ||
|
||
@Test | ||
void customKeyDeserializerShouldBeUsedWhenTypeNotDefined() throws Exception { | ||
// GIVEN | ||
String json = "{\n" + | ||
" \"name*\": \"Erik\",\n" + | ||
" \"address*\": {\n" + | ||
" \"city*\": {\n" + | ||
" \"id*\": 1,\n" + | ||
" \"name*\": \"Berlin\"\n" + | ||
" },\n" + | ||
" \"street*\": \"Elvirastr\"\n" + | ||
" }\n" + | ||
" }"; | ||
|
||
SimpleModule keySanitizationModule = new SimpleModule("key-sanitization"); | ||
keySanitizationModule.addKeyDeserializer(String.class, new KeyDeserializer() { | ||
@Override | ||
public String deserializeKey(String key, DeserializationContext ctxt) { | ||
return key.replace("*", "_"); | ||
} | ||
}); | ||
|
||
keySanitizationModule.addKeyDeserializer(Object.class, new KeyDeserializer() { | ||
@Override | ||
public Object deserializeKey(String key, DeserializationContext ctxt) { | ||
return key.replace("*", "_"); | ||
} | ||
}); | ||
|
||
ObjectMapper mapper = JsonMapper.builder().addModule(keySanitizationModule).build(); | ||
|
||
// WHEN | ||
Map<String, Object> result = mapper.readValue(json, new TypeReference<Map<String, Object>>() { | ||
}); | ||
|
||
// THEN | ||
// depth 1 works as expected | ||
Assertions.assertEquals("Erik", result.get("name_")); | ||
|
||
// depth 2 does NOT work as expected | ||
Map<String, Object> addressMap = (Map<String, Object>) result.get("address_"); | ||
// null?? Fails here | ||
Assertions.assertEquals("Elvirastr", addressMap.get("street_")); | ||
Map<String, Object> cityMap = (Map<String, Object>) addressMap.get("city_"); | ||
Assertions.assertEquals(1, cityMap.get("id_")); | ||
Assertions.assertEquals("Berlin", cityMap.get("name_")); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might need to see if we get the "default" key deserializer -- I forget how exactly it is done, maybe regular
MapDeserializer
has an example -- and if so, ignore it (re-set tonull
).(I may be wrong wrt above, but basically wondering if
ctxt.findKeyDeserializer()
ever returnsnull
or not)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Added commit according to your suggestio and we are down to one last case 👌🏼👌🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, simple mistake, didn't check if we had _customKeyDeserializer not null