Skip to content
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

Bug with polymorphic marshalling / unmarshalling objects with composite structure. #234

Closed
apimenau opened this issue May 28, 2013 · 10 comments
Milestone

Comments

@apimenau
Copy link

It is the copy of http://jira.codehaus.org/browse/JACKSON-896
There is a bug with polymorphic marshalling / unmarshalling objects with composite structure.
Using ObjectMapper.DefaultTyping.OBJECT_AND_NON_CONCRETE setting can't guarantee correct marshalling for classes with quite a complex structure. The test case is in the attached file http://jira.codehaus.org/secure/attachment/62953/jackson.zip. There you can find class ItemMap with field private Map<String, List> childItems;. After trying to unmarshall such an object we will catch an exception:
com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException: Unrecognized field "@Class" (class ItemMap), not marked as ignorable (2 known properties: , "childItems", "value"])
at [Source: java.io.StringReader@4cd67e21; line: 1, column: 110](through reference chain: ItemMap["childItems"]->ItemMap["@Class"])
at com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException.from(UnrecognizedPropertyException.java:79)
P.S. In our legacy system we use version 1.9 and 2.1. in the new one, so it will be great to have fixes for both branches.

@apimenau
Copy link
Author

import java.util.*;

public class ItemList {
  public String value;
  public List<ItemList> childItems = new LinkedList<ItemList>();

  public void addChildItem(ItemList l) { childItems.add(l); }
}

import java.util.*;

public class ItemMap {
  public String value;
  public Map<String, List<ItemMap>> childItems = new HashMap<>();

  public void addChildItem(String key, ItemMap childItem) {
    List<ItemMap> items;
    if (childItems.containsKey(key)) {
        items = childItems.get(key);
    } else {
        items = new ArrayList<>();
    }
    items.add(childItem);
    childItems.put(key, items);
  }
}

import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.databind.ObjectMapper;

public class Runner {

public static void main(String[] args) {
    try {
        testList();
    } catch (Exception e) {
        System.out.println("ItemList not passed");
        e.printStackTrace();
    }
    try {
        testMap();
    } catch (Exception e) {
        System.out.println("ItemMap not passed");
        e.printStackTrace();
    }
}

private static void testList() throws Exception {
    String json = getMapper().writeValueAsString(generateItemList());
    Object o = getMapper().readValue(json, ItemList.class);
    System.out.println("ItemList passed:" + o);
}


private static void testMap() throws Exception {
    String json = getMapper().writeValueAsString(generateItemMap());
    Object o = getMapper().readValue(json, ItemMap.class);
    System.out.println("ItemMap passed: " + o);
}

private static ItemList generateItemList() {
    ItemList child = new ItemList();
    child.value = "I am child";

    ItemList parent = new ItemList();
    parent.value  = "I am parent");
    parent.addChildItem(child);
    return parent;
}

private static ItemMap generateItemMap() {
    ItemMap child = new ItemMap();
    child.value = "I am child";

    ItemMap parent = new ItemMap();
    parent.value = "I am parent";
    parent.addChildItem("child", child);
    return parent;
}

 private static ObjectMapper getMapper() {
    final ObjectMapper objectMapper = new ObjectMapper();
    objectMapper.enableDefaultTyping(ObjectMapper.DefaultTyping.OBJECT_AND_NON_CONCRETE, JsonTypeInfo.As.PROPERTY);
    return objectMapper;
 }
}

@Siarhei-Yarkavy
Copy link

Is any news related to this issue? Will it be fixed in 2.3 version?

@cowtowncoder
Copy link
Member

Trying out the provided test (slightly simplified, I edited sample above). I can replicate failure of Map handling.

My first guess is that this is probably due to nested nature of types and handlers. I hope it is something that can be resolved.

@cowtowncoder
Copy link
Member

One thing I note is the inclusion mechanism used: use of JsonTypeInfo.As.PROPERTY is not recommended for default typing. That does not change things (changing it to, say, WRAPPER_ARRAY will not fix this problem) here, but I think it is safer to use either WRAPPER_ARRAY or WRAPPER_OBJECT mechanisms.

@cowtowncoder
Copy link
Member

Serialization is incorrect in failing test case: values in childItems should contain type information as they are declared to be of type ItemMap, which is concrete (and as such should not include type info).
This can be seen by changing criteria in tests to NON_FINAL, in which case test would pass (since inclusion is correctly and consistently determined for serialization and deserialization).

I will try to figure out why type info inclusion is incorrectly determined.

@cowtowncoder
Copy link
Member

Following code flow, the problems comes form dynamic discovery of serializers for Map entries; runtime (type-erased) type of List is used, instead of statically declared type. Problem is that while latter may be more specific (and is the reason it is used), it can not contain generic type information, something that former typically has.

I'll see if I can find a simple fix here (plausible), or if a bigger rework is needed.

@cowtowncoder
Copy link
Member

Ok. Looks like this has similar root cause as:

http://jira.codehaus.org/browse/JACKSON-508

but only affects one specific code path: that of a generic type as a value of Map. Good thing is that this should actually be possible to fix easily and reliably.

cowtowncoder added a commit that referenced this issue Aug 13, 2013
@cowtowncoder
Copy link
Member

Backport fix in 1.9.x as well (for 1.9.14), but note that there is no firm release schedule for 1.9.x, and this is the first fix since 1.9.13. So it may take a while until new version is released there -- in the meantime, you may want to do local build if you need a fixed version.

@apimenau
Copy link
Author

Thanks a lot for you work. It is very useful.

@cowtowncoder
Copy link
Member

You are welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants