Orika fails to map classes with lists which do not return internal references

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Create a class with a field of type List.
2. Initialize the field with an empty ArrayList.
3. Create a getter which will not return the internal reference but wrap the 
list entries in a new instance: return new ArrayList(myList);
4. Write a test case where you add some elements to the list, and then try to 
convert the class. 

I'd expect orika to easily copy this list. Instead the resulting object 
contains an empty list. This code in the generated mapper is responsible for 
the behaviour:

if (((java.util.List) destination.getValues()) == null)
  destination
    .setValues(((java.util.List) new java.util.ArrayList(
        ((java.util.List) source.getValues()).size())));
((java.util.List) destination.getValues()).clear();
((java.util.List) destination.getValues()).addAll(mapperFacade
  .mapAsList(((java.util.List) source.getValues()),
      ((ma.glasnost.orika.metadata.Type) usedTypes[0]),
      ((ma.glasnost.orika.metadata.Type) usedTypes[0]),
        mappingContext));

getValues() is the method returning the list. Since the internal field is 
initialized with an empty list destination.getValues() will return != null. 
Calling getValues() on destination will always create a new ArrayList instance, 
so the later addAll() will add the entries to an list which is not referenced 
anywhere. 

This behavior of not returning internal references is usual for writing stable 
classes which do not allow to modify internals. See FindBugs EI_EXPOSE_REP 
rule: http://findbugs.sourceforge.net/bugDescriptions.html#EI_EXPOSE_REP

Using orica 1.3.5.

Suggested fix: Just call the setter. If the intended behavior was to keep List 
implementations on the destination object, call 
destination.setValues(destination.getValues().addAll(source.getValues())). But 
this will fail if a implementation will return a list wrapped in 
Collections.unmodifiableList().

Original issue reported on code.google.com by wirch.ed...@gmail.com on 20 Nov 2012 at 2:56

[GoogleCodeExporter]

In fact, this was intentional since a lot of us wan't to override internal 
implementation, users usually want to preserve their internal collection 
implementations.

To customize this, by adding to MapperFactory a CustomMapper of Collection to 
Collection, this can solve the issue (think about it as collection's merge 
strategy).

Any way I will let the issue open to to give a deeper look, we're working on 
modularization of generator so it's possible to extend it and/or override 
default generated code.

Thank you.

Original comment by elaat...@gmail.com on 20 Nov 2012 at 8:13

[GoogleCodeExporter]

An update has been applied in current master which will attempt to map using 
the setter, unless the variable is not assignable, in which case, it will fall 
back to mapping against the (assumed) reference retrieved from the getter.

Original comment by matt.deb...@gmail.com on 24 Nov 2012 at 6:30

• Changed state: Started

[GoogleCodeExporter]

This change was applied in the 1.4.0 release. 
Could you tell us if you're seeing improved behavior with use of the 1.4.0 
version?

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

[GoogleCodeExporter]

This issue fix works. But another of my test cases failed when I updated: 
http://code.google.com/p/orika/issues/detail?id=77 May be related? 

Original comment by wirch.ed...@gmail.com on 16 Jan 2013 at 9:06

[GoogleCodeExporter]

fixed in 1.4.1 (along with associated issue #77)

Original comment by matt.deb...@gmail.com on 10 Feb 2013 at 5:54

• Changed state: Fixed