FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.52k stars 1.38k forks source link

Jackson-databind caches plain map deserializer and use it even map has `@JsonDeserializer` #1807

Closed lexas2509 closed 6 years ago

lexas2509 commented 7 years ago

Hi.

We face the issue with map deserialization when custom KeyDeserializer is used. Looks like if we have two different Models one with plain map deserializer and another with custom one. And if we have deserialized first model then all upcoming deserialization request will be processed by plain map deserializer instead of using custom map deserializer specified for this model.

Here you can find Unit test to reproduce this issue.

We are using jackson 2.8.9 version.

In attached test.zip

Thank you Alexey

lexas2509 commented 7 years ago

I also checked 2.9.2 it has the same issue.

cowtowncoder commented 7 years ago

Thank you for reporting this. I will have a look at reproduction to see what might be happening.

cowtowncoder commented 7 years ago

Ok, yes, looks legit (wrt bug). I will cut'n paste here for simplicity.

cowtowncoder commented 7 years ago
public class DeserializerCacheTest {
    @Test
    public void testCachedSerialize() throws IOException {
        ObjectMapper objectMapper = new ObjectMapper();

        MapHolder expectedPojo = new MapHolder();
        expectedPojo.data.put("1key", "onedata");
        expectedPojo.data.put("2 key", "twodata");
        String expectedJson = "{\"data\":{\"1key\":\"onedata\",\"2<-space->key\":\"twodata\"}}";

        String actualJson = objectMapper.writeValueAsString(expectedPojo);
        assertEquals(expectedJson, actualJson);

        // Do deserialization with non-annotated map property
        NonAnnotatedMapHolderClass ignored = objectMapper.readValue(actualJson, NonAnnotatedMapHolderClass.class);
        assertTrue(ignored.data.containsKey("2<-space->key"));

        MapHolder model2 = objectMapper.readValue(actualJson, MapHolder.class);
        assertTrue(model2.data.containsKey("2 key"));
        assertFalse(model2.data.containsKey("2<-space->key"));
    }

    @Test
    public void testSerializer() throws IOException {
        ObjectMapper objectMapper = new ObjectMapper();

        MapHolder expectedPojo = new MapHolder();
        expectedPojo.data.put("1key", "onedata");
        expectedPojo.data.put("2 key", "twodata");
        String expectedJson = "{\"data\":{\"1key\":\"onedata\",\"2<-space->key\":\"twodata\"}}";

        String actualJson = objectMapper.writeValueAsString(expectedPojo);
        assertEquals(expectedJson, actualJson);

        MapHolder model2 = objectMapper.readValue(actualJson, MapHolder.class);
        assertTrue(model2.data.containsKey("2 key"));
        assertFalse(model2.data.containsKey("2<-space->key"));
    }

    public static class MyKeySerializer extends JsonSerializer<String> {
        @Override
        public void serialize(String value, JsonGenerator jgen, SerializerProvider provider) throws IOException, JsonProcessingException {
            String result = value.replaceAll(" ", "<-space->");
            jgen.writeFieldName(result);
        }
    }

    public static class MyKeyDeserializer extends KeyDeserializer {
        @Override
        public Object deserializeKey(String key, DeserializationContext ctxt) throws IOException, JsonProcessingException {
            String result = key.replaceAll("<-space->", " ");
            return result;
        }
    }

    public static class NonAnnotatedMapHolderClass {
        public Map<String, String> data = new TreeMap<>();
    }

    public static class MapHolder {
        @JsonSerialize(keyUsing = MyKeySerializer.class)
        @JsonDeserialize(keyUsing = MyKeyDeserializer.class)
        public Map<String, String> data = new TreeMap<>();
    }
cowtowncoder commented 7 years ago

Yes, I can reproduce this. Problem is that although custom value handler (deserializer) and type handler (type deserializer) are checked for contents for all structured types, key type is not yet checked. So will need to add that check to prevent caching as well as lookups for such cases.

cowtowncoder commented 7 years ago

Fixed for 2.8.11 / 2.9.3

Johanneke commented 6 years ago

I have an issue with a class that implements StdSerializer<Map<String, MyObject>>, which is then applied to objects of type Map<String, String> as well. Do you think that is the same issue?

cowtowncoder commented 6 years ago

@Johanneke hard to say with just the description. But if you do return your custom serializer for specific type, caching does mean that this serializer will be assumed to be used for all properties (and root values) of the type you returned it for. Contextualization will still be performed for each instance (if and only if it implements ContextualSerializer), in case you need to change applicability.