atos1990 / orika

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

Javolution dependency causing classloader problems on Websphere #60

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Javolution dependency introduced in 1.3.0 is causing classloader problems on 
Websphere 6.1. Mainly the overridden org.xml.sax.* classes in the library. 
While these could probably be resolved by fiddling with the classloader 
configuration it is generally advised against including libraries that override 
classes provided by the application server unless necessary. 

I noticed you're using this library to cache mapped objects - please don't do 
that. Mapping frameworks are mostly used for mapping single-use objects so 
there's no need to cache them. Also there are cases where this could produce 
very unexpected results - consider this simple scenario:

SomeObj o = new SomeObj();
// populate the object
SomeObjDto dto = mapperFacade.map(o, SomeObjDto.class);
dto.setField("str");
SomeObjDto dto2 = mapperFacade.map(o, SomeObjDto.class);
// I don't want the same instance returned by the mapper again!

IMHO Any caching should be done outside the library if necessary.

Please consider removing this dependency. I wouldn't normally argue against a 
dependency but this is a library designed with real-time applications in mind - 
you should not force people into it.

Original issue reported on code.google.com by lighteater on 12 Oct 2012 at 9:18

GoogleCodeExporter commented 9 years ago
Matt was working on this issue we delayed 1.3.1 release until the fix.

We do not cache object for reuse. Cache live inside the MappingContext (single 
mapping operation) life time, in mainly, to avoid infinite recursion when 
dealing with Object graphs (with cycles). In the other case you can even use 
another MappingContext implementation that do not cache objects.

when you call mapperFacade.map(o, SomeObjDto.class) (here new MappingContext is 
created) so dto and dto2 will be two different objects.

PS: 1.3.1 should be out in the beginning of the next week.

Original comment by elaat...@gmail.com on 12 Oct 2012 at 9:52

GoogleCodeExporter commented 9 years ago
As Sidi mentioned, caching (within the library) is required to avoid infinite 
recursion for cyclic object graphs. If you know that your object graph is 
non-cyclic, there's a new option to get a faster mapper which skips the caching.

The scope of the caching is limited to a single mapping call via the API. If 
you look closely, you'll notice that within the map methods in 
MapperFacadeImpl, a fresh MappingContext is obtained, and released at the end 
of the method (so you would have to construct and reuse your own copy of 
MappingContext to cause such caching to live beyond a single mapping request)

That being said, we'll remove javolution dependency as soon as possible : )

Original comment by matt.deb...@gmail.com on 12 Oct 2012 at 5:14

GoogleCodeExporter commented 9 years ago
javolution dependency is removed in current github master (1.3.1-SNAPSHOT)

Original comment by matt.deb...@gmail.com on 12 Oct 2012 at 6:02

GoogleCodeExporter commented 9 years ago
Thanks. 

Sorry for the rant about caching then - I didn't have the time to carefully 
study the calls but wanted to mention it just in case. Guess I should have made 
a testcase first.

Original comment by lighteater on 15 Oct 2012 at 9:23

GoogleCodeExporter commented 9 years ago
The fix is 1.3.4 release,
Kindest regards

Original comment by elaat...@gmail.com on 17 Oct 2012 at 12:42

GoogleCodeExporter commented 9 years ago

Original comment by elaat...@gmail.com on 17 Oct 2012 at 12:42