yigityus / orika

Automatically exported from code.google.com/p/orika
0 stars 0 forks source link

mapAsMap breaks Object graphs #140

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
Given 2 classes ParentA and ChildA with a bi-directional link between them:

class ParentA
    ChildA child

class ChildA
    ParentA parent

Create 2 instances and connect them:

ParentA a = new ParentA();
a.child = new ChildA();
a.child.parent = a;

a == a.child.parent is true

Given also 2 equivalent classes ParentB and ChildB:

class ParentB
    ChildB child

class ChildB
    ParentB parent

Using Orika to convert a from ParentA to ParentB:

ParentB b = mapper.map(a, ParentB.class);

We end up with the same graph:

b == b.child.parent is true

But if mapAsMap is used instead, the graph ends up broken:

//Create a map and place the parent and its child in it:

Map<String, Object> m = new HashMap<String, Object>();
m.put("p", a);
m.put("c", a.child);

//use mapAsMap to convert it:
Type<Map<String, Object>> mapType = new TypeBuilder<Map<String, Object>>() 
{}.build();
Map<String, Object> m2 = mapper.mapAsMap(m, mapType, mapType);

ParentB b = (ParentB) m2.get("p");

And here's the bug:

b == b.child.parent is false

Please see the attached java file for a complete example to reproduce the bug 
(with Orika 1.4.4).

Digging through the code, I think that this bug is caused by the type cache. 
Here's a rundown of what I noticed while debugging the mapping process:

- Orika iterates over the map entries. In my case, the "c" entry is the first 
entry processed.
- The ChildA instance gets mapped to a ChildB instance
- Indirectly, ChildA.parent also gets mapped to a ParentB instance. This 
instance is stored in the typeCache under its type ParentB.uniqueIndex (say 15 
for example)
- The second entry with the key "p", which is an instance of ParentA is 
processed next
- Orika checks in the typeCache. The problem is that it uses the wrong type:
Dv mappedValue = (Dv) context.getMappedObject(entry.getValue(), 
destinationType.<Dv> getNestedType(1));

Here, the destinationType is Object (the type parameter of values in the 
generic Map type) and not ParentB (the actual value type), which has a 
different uniqueIndex. And so the instead of using the cached object we end up 
with a different instance.

I'm not sure I'm following everything in the codebase, but it seems to me that 
the cache lookup shouldn't use the map generic type but instead use the 
resolved concrete type (using  mapperFactory.lookupConcreteDestinationType I 
think).

The same bug also affects mapAsCollection:

Type<Object> listType = new TypeBuilder<Object>() {}.build();
List<Object> l2 = mapper.mapAsList(l, listType, listType);
b = (ParentB) l2.get(1);
System.err.println(b == b.child.parent);//prints false, graph was broken

Original issue reported on code.google.com by jawher.m...@gmail.com on 19 Dec 2013 at 5:54

Attachments:

GoogleCodeExporter commented 8 years ago
Please take a look at the lastest snapshot; a fix has been pushed for this 
issue. Let us know how it works for you.

Original comment by matt.deb...@gmail.com on 11 Jun 2014 at 4:37