FasterXML / jackson-databind

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

Exceptions when deserialize Setterless properties with type info(polymorphic deserialization) #501

Open fangzhen opened 10 years ago

fangzhen commented 10 years ago

When deserialize setterless properties with type info

 mapper.enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL)
the deserializer throws an exception:
com.fasterxml.jackson.databind.JsonMappingException: Invalid type id 'item' (for id type 'Id.class'): no such class found (through reference chain: com.fasterxml.jackson.databind.deser.PropNoSetter["list"]) 
Here's an example to make it clear:

@Test
public void testVectorDeser() throws JsonGenerationException, JsonMappingException, IOException{
    try{
    ObjectMapper mapper = new ObjectMapper();
    mapper.enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL);
    StringWriter out;
    out = new StringWriter();
    PropNoSetter data = new PropNoSetter();
    data.getList().add("item");
    mapper.writeValue(out, data);
    System.out.println(out);
    //OUTPUT: ["com.fasterxml.jackson.databind.deser.TestSetterlessTypeId$PropNoSetter",{"list":["java.util.ArrayList",["item"]],"map":["java.util.HashMap",{}]}]
    PropNoSetter data2 = mapper.readValue(new StringReader(out.toString()), PropNoSetter.class);
    //Here will throw the exception above
    assertEquals(data.getList(), data2.getList());
    assertEquals(data.getMap(), data2.getMap());
    }catch (Exception e){
        e.printStackTrace();
    }
}

@SuppressWarnings("rawtypes")
static class PropNoSetter{
    private Map amap = new HashMap();
    private List alist = new ArrayList();
    public List getList(){
        return alist;
    }
    public Map getMap(){
        return amap;
    }
}

I traced the execution, and have found something.

BeanDeserializerFactory#constructSetterlessProperty(DeserializationContext ctxt,
        BeanDescription beanDesc, BeanPropertyDefinition propDef)

doesn't initiate _valueTypeDeserializer for setterlessProperty. And

SetterlessProperty#deserializeAndSet(JsonParser jp, DeserializationContext ctxt,
        Object instance)

doesn't test if the _valueTypeDeserializer exists, probably correspond with the former. However, I think it should test it as SettableBeanProperty does. Unfortunately, the TypeDeserializer didn't give interface to deserialize to a given Object.

cowtowncoder commented 10 years ago

Ok. I assume that if you added actual setter for List, things would work? If so it sounds like there is a bug in SetterlessProperty.

Thank you for reporting this.

fangzhen commented 10 years ago

Yes. When there's actual setter or the getter is corresponding to the field name behind(that's, getAlist() instead of getList()), it works just fine. And I had a temporary workaround fork here: https://github.com/fangzhen/jackson-databind/commit/deb02f0313691c8fe176584f6178e126ca2b4111, But it changed API of TypeDeserializer .

cowtowncoder commented 10 years ago

Yeah, there should be no need to change the api here, nor assume that type information comes from an array -- it's not related to inclusion of values, but for inclusion of type id. But I hope test case can help reproduce the problem.

fangzhen commented 10 years ago

I'm not quite clear of the relation of valueDeserializer and typeDeserializer. So I just tried to follow what SettableBeanProperty does. The jsonDesrializer abstract class defined three methods for deserialize:

deserialize(JsonParser jp, DeserializationContext ctxt)
deserialize(JsonParser jp, DeserializationContext ctxt, T intoValue)
deserializeWithType(JsonParser jp, DeserializationContext ctxt, TypeDeserializer typeDeserializer)

At my point of view, I think that the first is for deserializing normal cases without type info and setterless properties, the second is for those with setterless properties and the third for type info. But we dont have one for those both with type info and setterless properties. And similarly, the TypeDeserializer dont have API for setterless properties.

And the third one in CollectionDeserializer are impled as:

@Override
public Object deserializeWithType(JsonParser jp, DeserializationContext ctxt,
        TypeDeserializer typeDeserializer)
    throws IOException, JsonProcessingException
{
    // In future could check current token... for now this should be enough:
    return typeDeserializer.deserializeTypedFromArray(jp, ctxt);
}

And I just followed it... But I think it happens to work since currently deserializeTypedFromXXX method are the same.

The test case should reproduce the problem for version 2.4.0. Please let me know if it didn't. Thanks.

cowtowncoder commented 10 years ago

@fangzhen understood, relationship is not trivial to understand; and API may well be missing things since earlier versions did not handle as many things as newer ones (i.e. API not designed to pass more information than what was needed at a time).

Wrt methods: second method does not exist to support setterless properties, but for ObjectMapper.updateValue() call; this is at least the original reason for adding it. But I can see why it would also make sense for setterless case, as handling needs to differ.

Anyway: I did not mean to criticize your approach, but just try to help understand original design idea. I appreciate your work here in debugging the issue, and I think it will make it much easier to resolve the problem. I also understand your approach, figuring out ways that work first, and then trying to make sure they follow design as much as possible.

So: thank you for test case, proposed patch; I will try to get working on this issue today.

cowtowncoder commented 10 years ago

@fangzhen Looking at code I now understand your comments, and yes, you are right in that some pieces are missing. But this might be even trickier in that type information for container must basically be ignored as it can not be used for constructing new container (since existing one is to be used).

cowtowncoder commented 7 years ago

Just adding label so that I remember to re-evaluate this as part of 2.9 work; no guarantees it can be fixed but should be attempted.

jacarrichan commented 7 years ago

It cause can not storage org.apache.shiro.subject.SimplePrincipalCollection in redis

kopax commented 6 years ago

I have this issue

vpatil1311 commented 2 months ago

Hey @cowtowncoder I am facing this issue in version 2.15 , I am using kotlin data classes any pointers would help.

JooHyukKim commented 2 months ago

@vpatil1311 this issue, is reproduced in java. You may do either of the following

  1. Either provide reproduction in java (like this issue with full configuration etc)...
  2. Or ask over at kotlin-module discussions channel also make sure to bring reproduction.
cowtowncoder commented 2 months ago

What @JooHyukKim said, but additionally upgrade first to 2.17.1 (even if just for testing) to see if something has been fixed.

srusanvin commented 2 months ago

Hi @JooHyukKim @cowtowncoder my test code is here https://github.com/FasterXML/jackson-module-kotlin/discussions/809 I am using version 2.17.1

cowtowncoder commented 2 months ago

@srusanvin Assuming this is the same issue, you are better of figuring how to avoid use of "setterless" getters. First, disable

MapperFeature.USE_GETTERS_AS_SETTERS

and then you won't be hitting the same issue. And I doubt it's something you'd expect to happen; there needs to be either Field to set, or setter method, for whichever property was attempted to be set via getter (that is: getter used to access List, which is modified; something JAXB does but is really not a proper way to do anything).