orika-mapper / orika

Simpler, better and faster Java bean mapping framework
http://orika-mapper.github.io/orika-docs/
Apache License 2.0
1.29k stars 269 forks source link

mapNulls(false) is ignored if a parameterised constructor is found in destination class #335

Open robabod opened 4 years ago

robabod commented 4 years ago

We have found an issue when mapping using mapNulls(false) if the destination class has both a parameterised constructor (as well as a default constructor), and a default value for a field (as set by the default constructor or by direct assignment). Instead of calling the default constructor, the parameterised constructor is called, with the null values passed in as parameters.

The following provides a simple example:

public class Source {
  private String value;
  public Source() { }
  public Source(String value) { this.value = value; }
  public String getValue() { return value; }
  public void setValue(String value) { this.value = value; }
}
public class Destination {
  private String value = "default-value";
  public Destination() { }
  public Destination(String value) { this.value = value; }
  public String getValue(){ return value; }
  public void setValue(String value) { this.value = value; }
}

The only difference between Source and Destination are that Destination has a default value for the contained String. The mapper is configured as follows:

public class SourceDestMapper extends ConfigurableMapper {
  @Override
  protected void configure(MapperFactory mapperFactory) {
    mapperFactory.classMap(Source.class, Destination.class)
        .mapNulls(false).mapNullsInReverse(false)
//        .constructorA().constructorB()
        .byDefault()
        .register();
  }
}

We can test this with the following in a simple JUnit test:

  public void shouldNotMapNullsKeepingDefault() {
    Destination mapped = new SourceDestMapper().map(new Source(), Destination.class);
    Assertions.assertThat(mapped).extracting(Destination::getValue).isEqualTo("default-value");
  }

When used as-is, the mapper calls the parameterised constructor of Destination, passing in the null value from Source, resulting in the test failing, despite specifying .mapNulls(false).

The test can be made to work by uncommenting the lines that force the mapper to use the default constructors.

Using the parametised constructors is a good thing, and should be used for speed in most cases. However I would expect that if mapping of null is disabled, and there are null fields in the source object, then the default constructor should be used so that any default values are kept.

Note that in the example code I have used String, but we found the issue when the default value of a field was an empty List.

We found the issue in 1.5.1, but I have tested in 1.5.4 and the issue remains.

This is also similar to #135 but specific to excluding the mapping of null values.