jhalterman/modelmapper — pull request #39 by jmbreuer (2013-02-18 10:17:56+00:00)
Title: Fix "constructors call setters", "map/convert to String"

This is my first pull request, so please excuse any n00bness and just tell me what I should do.

The attached commits fix two issues we came across when using modelmapper here.

Sorry, there are no proper tests for the changes so far.

1) When (default) constructors of mapped classes call setters, this confuses the configuration interception stage - the setter call is instrumented and recorded as if it were part of the mapping's configure().
My fix is to track whether execution is within a constructor; and if it is to not record intercepted setters.
As a side effect, this disallows 'null' as a MappingContext, which is required by one existing test. For use here, I've disabled the test; to fix this properly I'd probably mock the MappingContext.

I see there is no mocking framework in place yet, how do you feel about https://code.google.com/p/mockito/

I do not feel that checking for a null MappingContext in the production code is valid, since it only concerns testing and not productive use as far as I understand.

2) Mapping using a converter to string values breaks because the Strings are considered "mutable" by the mapping process, and a default empty constructed value is put in the destination cache.
I've not tried to figure out exactly when/how/why this value is sometimes used and sometimes a correctly mapped value is used.
But I found a very succinct fix: Returning 'false' from Types.isInstantiable() for Strings causes modelmapper to do what I'd expect in our use case.

The mapping behind this would look something like this:

enum EnumVal {
FOO, BAR;

public String getExportName() {
return foo(name());
}
}

class MappedFrom {
UUID id;
EnumVal eVal;
}

class MappedTo {
String id;
String eVal;
}

with a Converter in the mapping context which returns source.getExportName().
UUID -> ID apparently works implicitly using .toString().

I use MatchingStrategies.STRICT; the mappings are defined explicitly using the type conversion syntax:
map(source.getId()).setId(null);
map(source.getEVal()).setEVal(null);

If I'm simply using modelmapper wrong and there's a simpler way to achieve the same goal just show me what it is.

Nonetheless, I think "least surprise" WRT String type is improved with this fix.