ModelMapper mapper.skip() doesn't work for pojo objects with circular dependency

[imtiazShakil]

I have two pojo objects: Husband, Wife which reference each other.

Husband.java

public class Husband {
   private String name;
   private int age;
   private String man;
   private Wife wife;

   // getter, setter, builder, constructor are removed for berevity
}

Wife.java

public class Wife {
   private String name;
   private int age;
   private String woman;
   private Husband husband;

   // getter, setter, builder, constructor are removed for berevity
}

I've created simple typeMap rulings for both objects, where the referenced object is skipped.

My test class:

public class ModelTest {

@Test
public void test() {
    ModelMapper modelMapper = new ModelMapper();
    TypeMap<Wife, Wife> typeWife = modelMapper.createTypeMap(Wife.class, Wife.class);
    typeWife.addMappings(mapper -> {
        mapper.skip(Wife::setHusband);
    });

    TypeMap<Husband, Husband> typeHusband = modelMapper.createTypeMap(Husband.class, Husband.class);
    typeHusband.addMappings(mapper -> {
        mapper.skip(Husband::setWife);
    });

    Wife wife = Wife.builder().age(25).name("Sarah").woman("good woman").build();
    Husband husband = Husband.builder().age(28).name("Imtiaz").man("good man").build();
    wife.setHusband(husband);
    husband.setWife(wife);

    Husband updatedHusband = Husband.builder().age(28).name("Imtiaz Shakil").man("slightly good man").build();
    modelMapper.map(updatedHusband, husband);
    System.out.println(husband.toString());
    System.out.println(husband.getWife().toString());
    }

}

When mapping updatedHusband to husband, setWife() method is not skipped. But, if I remove typeWife mapping from modelMapper the code works fine.

I'm using ModelMapper 1.1.3

Thanks.

[imtiazShakil]

I think the problem is with the mappings modelmapper generates. When I print the mappings of each typeMap this is what I get:

[PropertyMapping[Wife.age -> Wife.age], PropertyMapping[ -> Wife.husband], PropertyMapping[Wife.name -> Wife.name], PropertyMapping[Wife.woman -> Wife.woman]]
[PropertyMapping[Husband.age -> Husband.age], PropertyMapping[Husband.man -> Husband.man], PropertyMapping[Husband.name -> Husband.name], PropertyMapping[ -> Husband.wife], PropertyMapping[Husband.wife.age -> Husband.wife.age], PropertyMapping[Husband.wife -> Husband.wife.husband], PropertyMapping[Husband.wife.name -> Husband.wife.name], PropertyMapping[Husband.wife.woman -> Husband.wife.woman]]

TypeMap typeHusband picks mapping from typeWife during mapping process.

[imtiazShakil]

I found no clean way of solving this problem. But we can add PropertyCondition to skip mapping of objects.

// this method checks the source Object Class Name during property mapping
// if the class name is contained in the skipClassesSet then mapping is skipped
private <S, D> Condition<S, D> addSkipCondition(TypeMap<S, D> typeMap, HashSet<String> skipClassesSet) {
    if (typeMap == null)
        return null;
    Condition<S, D> skipCondition = new Condition<S, D>() {
        @Override
        public boolean applies(MappingContext<S, D> context) {
            String destinationType = context.getMapping().getDestinationProperties().get(0).getType()
                    .getSimpleName();

            return !skipClassesSet.contains(destinationType);
        }
    };
    return skipCondition;
}

Now, our test works perfectly.

@Test
public void test5() {
    ModelMapper modelMapper = new ModelMapper();
    TypeMap<Wife, Wife> typeWife = modelMapper.createTypeMap(Wife.class, Wife.class);
    typeWife.setPropertyCondition(
            addSkipCondition(typeWife, new HashSet<>(Arrays.asList(Husband.class.getSimpleName()))) );

    TypeMap<Husband, Husband> typeHusband = modelMapper.createTypeMap(Husband.class, Husband.class);
    typeHusband.setPropertyCondition(
            addSkipCondition(typeHusband, new HashSet<>(Arrays.asList(Wife.class.getSimpleName()))) );


    Wife wife = Wife.builder().age(25).name("Sarah").woman("good woman").build();
    Husband husband = Husband.builder().age(28).name("Imtiaz").man("good man").build();
    wife.setHusband(husband);
    husband.setWife(wife);

    Wife modifiedWife = Wife.builder().age(26).name("Sarah").woman("very good woman").build();
    modelMapper.map(modifiedWife, wife);
    assertEquals(wife.getHusband(), husband);

    Husband modifiedHusband = Husband.builder().age(28).name("Imtiaz Shakil").man("slightly good man").build();
    modelMapper.map(modifiedHusband, husband);
    assertEquals(husband.getWife(),wife);

}

But this approach also has some drawbacks, as we can't add different types of conditions together and use it as PropertyCondition. Because, the builtin Conditions.add(cond1, cond2) doesn't support mixing different types of conditions.

Thanks!

[chhsiao90]

I think it's annoying that implicit mapping(auto property mapping) do what we don't want it do, but we also need implicit mapping for convenience.

I think it's ordering problem that there will be less issue if the implicit mapping is invoked AFTER our manual mappings. For your example, the implicit mappings are created when you create type map, so it's no use to skip the wife after that.

I will try to create an api that you can perform implicit mapping after manually mapping.
WDYT?

[imtiazShakil]

Are there any other solutions like,
Before applying implicit mapping , we should check if the object is already skipped by parentMapping.

For example, in my case,
when creating typeHusband map, we first apply current mapping rules before performing implicit mapping.

[chhsiao90]

when creating typeHusband map, we first apply current mapping rules before performing implicit mapping.

That's what my solution want to solve. Currently, modelmapper don't have any approach to do this.

[chhsiao90]

hi @imtiazShakil ,
I'm working on the implementation.
I can reproduce that the TypeMap will be weird after unexpected implicit mapping. But I'm not sure what actual the issue is (I'm not challenging it's not a bug, I just want to write an unit test to reproduce the issue). Is that your problem is that: the husband of wife or the wife of husband will be null after mapping?

Thanks.

[chhsiao90]

Please take a look #337 and GH336.java#L100-L119 that I want to write a test to reproduce your issue with old way.

[chhsiao90]

hi @imtiazShakil ,

Can you try our new api?

ModelMapper modelMapper = new ModelMapper();
modelMapper.getConfiguration().setImplicitMappingEnabled(false);
modelMapper.typeMap(Husband.class, Husband.class)
    .addMappings(husbandPropertyMap)
    .implicitMappings();
modelMapper.typeMap(Wife.class, Wife.class)
    .addMappings(wifePropertyMap)
    .implicitMappings();

[imtiazShakil]

Hi @chhsiao90 ,

Thanks for the update! I'll give it a try and let you know.

[imtiazShakil]

Hello, @chhsiao90,

I checked the code and tried understanding.
With the new api everything works perfectly! Thanks.

I've some questions that I don't understand.

Approach 1

Disabling implicitMapping.

ModelMapper modelMapper = new ModelMapper();
modelMapper.getConfiguration().setImplicitMappingEnabled(false);
modelMapper.typeMap(Husband.class, Husband.class)
    .addMappings(husbandPropertyMap)
    .implicitMappings();
modelMapper.typeMap(Wife.class, Wife.class)
    .addMappings(wifePropertyMap)
    .implicitMappings();

The mapping is exactly What I meant.

[PropertyMapping[Husband.name -> Husband.name], ConstantMapping[null -> Husband.wife]]
[ConstantMapping[null -> Wife.husband], PropertyMapping[Wife.name -> Wife.name]]

Approach 2

Default mapping

ModelMapper modelMapper = new ModelMapper();
modelMapper.typeMap(Husband.class, Husband.class)
        .addMappings(husbandPropertyMap);
modelMapper.typeMap(Wife.class, Wife.class)
        .addMappings(wifePropertyMap);

What I get is this:

[PropertyMapping[Husband.name -> Husband.name], ConstantMapping[null -> Husband.wife]]
[ConstantMapping[null -> Wife.husband], PropertyMapping[Wife.husband.name -> Wife.husband.name], ConstantMapping[null -> Wife.husband.wife], PropertyMapping[Wife.name -> Wife.name]]

The mapping is like before, but still the tests work ok! Can you explain why?

When should we use what approach?

If implicitMapping is disabled, will modelmapper implicitly create typemaps where no user defined mappings exist? Will it throw exceptions?

Thanks!

[chhsiao90]

About approach 2. that's what I'm asking in my previous comment here that I try to reproduce your bug but I can't. My fix should not affect the approach 2. Maybe there is some changes between old and latest modelmapper that fix the problem.

When should we use what approach?

I think this approach was used when you had some custom type maps, and each type map is based on other type map.

If implicitMapping is disabled, will modelmapper implicitly create typemaps where no user defined mappings exist? Will it throw exceptions?

The modelmapper will create empty TypeMap, and there will be no exception.

It's a good question, that you sometimes need to disable implicit mappings, but you still need implicit mappings. I will think about how to improve the API.

[imtiazShakil]

With this patch #337 , I also can't reproduce the bug.

It's a good question, that you sometimes need to disable implicit mappings, but you still need implicit mappings. I will think about how to improve the API.

Can this problem be fixed if we write configuration like this?

ModelMapper modelMapper = new ModelMapper();

modelMapper.getConfiguration().setImplicitMappingEnabled(false);

// add custom property mappings as necessary
// add custom property conditions as necessary
// ... ... ... ...

modelMapper.getConfiguration().setImplicitMappingEnabled(true);

[chhsiao90]

Can this problem be fixed if we write configuration like this?

Yes, it can fix the problem. But it looks ugly :(

[chhsiao90]

hi @imtiazShakil ,

I provide a new API: modelMapper.emptyTypeMap in #359, is this API can help for your use case?

[chhsiao90]

Just release v2.1.0, please take a look.

[imtiazShakil]

Thanks. I'll let you know.