Skip to content

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

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

Open
fangzhen opened this issue Jul 9, 2014 · 14 comments
Labels
has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue

Comments

@fangzhen
Copy link

fangzhen commented Jul 9, 2014

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

@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
Copy link
Member

@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 added a commit that referenced this issue Jul 11, 2014
cowtowncoder added a commit that referenced this issue Jul 11, 2014
@cowtowncoder
Copy link
Member

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
Copy link

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

@kopax
Copy link

kopax commented Oct 25, 2017

I have this issue

@cowtowncoder cowtowncoder removed the 2.9 label Sep 12, 2019
@vpatil1311
Copy link

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

@JooHyukKim
Copy link
Member

@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
Copy link
Member

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

@srusanvin
Copy link

Hi @JooHyukKim @cowtowncoder my test code is here FasterXML/jackson-module-kotlin#809 I am using version 2.17.1

@cowtowncoder cowtowncoder added the has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue label Jun 21, 2024
@cowtowncoder
Copy link
Member

cowtowncoder commented Jun 21, 2024

@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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Projects
None yet
Development

No branches or pull requests

7 participants