atos1990 / orika

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

StackOverflowError when using inheritance mapping #52

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In Orika 1.2.0 there is a problem when having a lot of inheritance mapping. 

I'm having trouble cooking up a simple test case where this error occurs, it 
doesn't seem to come up with only a few mappers. However I managed to narrow up 
the cause with a debugger so I'll try to describe what happens. During the 
initialization process the DefaultMapperFactory.initializeUsedMappers method 
will mess up the parent mapper:

private void initializeUsedMappers(ClassMap<?, ?> classMap) {        
        Mapper<Object, Object> mapper = lookupMapper(new MapperKey(classMap.getAType(), classMap.getBType()));

       List<Mapper<Object, Object>> parentMappers = new ArrayList<Mapper<Object, Object>>();
// .. find parent mappers
        mapper.setUsedMappers(parentMappers.toArray(new Mapper[parentMappers.size()]));
}

Now the problem is the lookupMapper method might return the parent mapper since 
the getRegisteredMapper method uses 'isAssignableFrom' to find the first hit. 
So when searching for a mapper for Child.class/ChildDto.class it might return a 
mapper for Parent.class/ParentDto.class. In 

this case it causes the parent mapper to be set with itself as a usedMapper and 
forms an infinite recursive call, but it looks like it might cause other errors 
as well.

Original issue reported on code.google.com by lighteater on 14 Sep 2012 at 3:41

GoogleCodeExporter commented 9 years ago
Thanks for the heads up; I'll take a look

Original comment by matt.deb...@gmail.com on 14 Sep 2012 at 6:38

GoogleCodeExporter commented 9 years ago
Ok, I was able to reproduce this;
I found an error in MapperKey's compare function that could result in the 
parent mapper being looked up first; after fixing this, parent mapper cannot be 
looked up through this path.
However, it revealed a different problem which is possible:
if "used mappers" are specified with types flipped, they aren't properly 
flipped in the generated mapper, causing ClassCastException; 
both of these issues have been addressed in the current trunk.
Could you take a look at the latest snapshot and see if this fixes it for you?

Original comment by matt.deb...@gmail.com on 16 Sep 2012 at 1:55

GoogleCodeExporter commented 9 years ago
Still getting the error on the current trunk:

Caused by: java.lang.StackOverflowError
    at ma.glasnost.orika.impl.GeneratedMapperBase.mapAtoB(GeneratedMapperBase.java:94)

I'll try poking at it a bit more and try to pin it down with a testcase, but 
I'm on a tight schedule so no promises.

Original comment by lighteater on 17 Sep 2012 at 8:26

GoogleCodeExporter commented 9 years ago
I managed to reproduce the issue with a testcase :)

Original comment by lighteater on 17 Sep 2012 at 2:15

Attachments:

GoogleCodeExporter commented 9 years ago
The funny thing is the test also fails on version 1.9.1, but with a different 
error:

java.lang.ClassCastException: my.test.OrikaInheritanceSOETest$ChildDto111
    at ma.glasnost.orika.generated.OrikaChild11ChildDto11Mapper1005998931020178.mapBtoA(OrikaChild11ChildDto11Mapper1005998931020178.java)
    at ma.glasnost.orika.impl.GeneratedMapperBase.mapBtoA(GeneratedMapperBase.java:89)
    at ma.glasnost.orika.generated.OrikaChild111ChildDto111Mapper1824811429310343.mapBtoA(OrikaChild111ChildDto111Mapper1824811429310343.java)
    at ma.glasnost.orika.impl.MapperFacadeImpl.mapDeclaredProperties(MapperFacadeImpl.java:376)
    at ma.glasnost.orika.impl.MapperFacadeImpl.map(MapperFacadeImpl.java:218)
    at ma.glasnost.orika.impl.MapperFacadeImpl.map(MapperFacadeImpl.java:113)
    at ma.glasnost.orika.impl.MapperFacadeImpl.map(MapperFacadeImpl.java:447)
    at my.test.OrikaInheritanceSOETest.test(OrikaInheritanceSOETest.java:27)

Original comment by lighteater on 17 Sep 2012 at 2:22

GoogleCodeExporter commented 9 years ago
Are you sure you're running with current trunk? We recently moved to githib, 
and the version is 1.2.2-SNAPSHOT...the reason I ask is that this test passes 
fine with no errors for me.

If not, check out our updated Downloads page for details on how to get it.

Original comment by matt.deb...@gmail.com on 17 Sep 2012 at 3:00

GoogleCodeExporter commented 9 years ago
Yeah, I went and downloaded again to be extra sure. 

Out of curiosity I went and copied the test onto the orika sources and indeed 
it passes when doing a full build. It still fails when run just by itself (mvn 
test -Dtest=my.test.OrikaInheritanceSOETest)

Original comment by lighteater on 17 Sep 2012 at 3:42

GoogleCodeExporter commented 9 years ago
Ok, what about OS and jdk vendor/version?

Original comment by matt.deb...@gmail.com on 17 Sep 2012 at 3:58

GoogleCodeExporter commented 9 years ago
I'll try with a new standalone project...

Original comment by matt.deb...@gmail.com on 17 Sep 2012 at 4:21

GoogleCodeExporter commented 9 years ago
Windows XP, JDK 1.5_17.

Original comment by lighteater on 17 Sep 2012 at 4:29

GoogleCodeExporter commented 9 years ago
Ok, thanks for the test and the info.
There's been a new fix pushed to the trunk; could you confirm?

Original comment by matt.deb...@gmail.com on 18 Sep 2012 at 6:51

GoogleCodeExporter commented 9 years ago
Seems ok now. Thanks :)

Original comment by lighteater on 18 Sep 2012 at 8:28

GoogleCodeExporter commented 9 years ago
Fixed in current trunk

Original comment by matt.deb...@gmail.com on 19 Sep 2012 at 8:09

GoogleCodeExporter commented 9 years ago
This problem seems to have reappeared in version 1.3.0 and is still present in 
1.3.5.

Original comment by christia...@gmail.com on 11 Dec 2012 at 3:25

GoogleCodeExporter commented 9 years ago
Could you provide more details? Anything you can do to help us reproduce this 
would be great...

Original comment by matt.deb...@gmail.com on 11 Dec 2012 at 4:23

GoogleCodeExporter commented 9 years ago
I use a custom mapper to map the response from a webservice to a dto because 
great richness of the modeled entities which rely on a heavy use of 
inheritance. Anyway, the response may contain class A or class A1 which 
inherits from A. To test both possibilities in the same test class, I wrote a 
test for each of these possibilities. The mapper is instantiated through 
autowiring.

If I place the test for A before the test for A1 I get ClassCastException from 
A to A1.
If I swap the order, the A1-specific properties aren't mapped.

This does not happen if I instantiate the mapper manually in a @Before only if 
I use @Autowire. I haven't been able to test it with live code yet since the 
GUI isn't ready yet so the jUnit tests are my best take at that.

Original comment by christia...@gmail.com on 12 Dec 2012 at 7:46

GoogleCodeExporter commented 9 years ago
is it possible to send your JUnit test?

Original comment by matt.deb...@gmail.com on 12 Dec 2012 at 4:21

GoogleCodeExporter commented 9 years ago
When you instantiated the mapper manually in the @Before method, were you
registering any class maps or converters or anything?

Original comment by matt.deb...@gmail.com on 14 Dec 2012 at 5:42

GoogleCodeExporter commented 9 years ago
Now I have merged the 2 tests I mentioned to assure that the same instance of 
the mapper is used and I get the errors. So the @Before worked only because it 
creates a new mapper for each test method (despite I tried to avoid it).

I cannot just send you JUnit test as it is now because of all the dependencies 
but I can try to reproduce it in another project this weekend and send it to 
you.

It works with v1.2.2 so it's not a showstopper for my part but I considered 
best to report it.

Original comment by christia...@gmail.com on 14 Dec 2012 at 10:06

GoogleCodeExporter commented 9 years ago
Ok, thanks.
I'll try to manufacture a test based on what you've described to see if we
can reproduce....

Original comment by matt.deb...@gmail.com on 14 Dec 2012 at 4:16

GoogleCodeExporter commented 9 years ago
One last question -- have you by chance checked your situation with latest
1.4.0-SNAPSHOT build? (it would be good to know whether we've already fixed
it, as I think this is currently the last outstanding issue before our
release)

Original comment by matt.deb...@gmail.com on 14 Dec 2012 at 4:21

GoogleCodeExporter commented 9 years ago
Does the attached test case seem to reflect your scenario?

I've noticed that the 'parentBeforeChild' test fails in 1.2.1 up to 1.3.5,
but passes with the latest 1.4.0-SNAPSHOT

Original comment by matt.deb...@gmail.com on 14 Dec 2012 at 8:55

GoogleCodeExporter commented 9 years ago
I changed the version to 1.4.0 and it works now :-)

Original comment by christia...@gmail.com on 2 Jan 2013 at 7:36

GoogleCodeExporter commented 9 years ago
Thanks for confirming

Original comment by matt.deb...@gmail.com on 2 Jan 2013 at 2:44