Skip to content

Presence check method used only once when multiple source parameters are provided #3601

Description

@thunderhook

Expected behavior

Feel free to change the title...

Given the following mapper having a mapping method with two parameters:

import java.util.List;

import org.mapstruct.Condition;
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
import org.mapstruct.factory.Mappers;

@Mapper
interface WrongConditionMapper {

    @Mapping(target = "currentId", source = "source.uuid")
    @Mapping(target = "targetIds", source = "sourceIds")
    Target map(Source source, List<String> sourceIds);

    @Condition
    default boolean isNotEmpty(List<String> elements) {
        return elements != null && !elements.isEmpty();
    }

    class Source {
        public String uuid;
    }

    class Target {
        public String currentId;
        public List<String> targetIds;
    }

}

The isNotEmpty presence check method should only be applied when mapping sourceIds to targetIds but not source.uuid
to currentId.

Actual behavior

class WrongConditionMapperImpl implements WrongConditionMapper {

    @Override
    public Target map(Source source, List<String> sourceIds) {
        if ( source == null && sourceIds == null ) {
            return null;
        }

        Target target = new Target();

        if ( source != null ) {
            // this isNotEmpty check has nothing to do with source.uuid or target.currentId
            if ( isNotEmpty( sourceIds ) ) {
                target.currentId = source.uuid;
            }
        }
        if ( isNotEmpty( sourceIds ) ) {
            List<String> list = sourceIds;
            target.targetIds = new ArrayList<String>( list );
        }

        return target;
    }
}

We didn't even find a workaround for this and implemented the method ourself.

I tried to find the culprit but I'm not experienced enough to fix this, but it is because it is picked up with the PresenceCheckMethodResolver here:

Steps to reproduce the problem

Here is a test mapper and a corresponding unit test:

import java.util.List;

import org.mapstruct.Condition;
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
import org.mapstruct.factory.Mappers;

@Mapper
interface WrongConditionMapper {

    @Mapping(target = "currentId", source = "source.uuid")
    @Mapping(target = "targetIds", source = "sourceIds")
    Target map(Source source, List<String> sourceIds);

    @Condition
    default boolean isNotEmpty(List<String> elements) {
        return elements != null && !elements.isEmpty();
    }

    @Condition
    default boolean stringCondition(String str) {
        return str != null;
    }

    class Source {
        public String uuid;
    }

    class Target {
        public String currentId;
        public List<String> targetIds;
    }

}
import java.util.ArrayList;

import org.mapstruct.ap.testutil.IssueKey;
import org.mapstruct.ap.testutil.ProcessorTest;
import org.mapstruct.ap.testutil.WithClasses;
import org.mapstruct.factory.Mappers;

import static org.assertj.core.api.Assertions.assertThat;

@WithClasses(WrongConditionMapper.class)
@IssueKey("3601")
class Issue3601Test {

    @ProcessorTest
    void shouldMapCurrentId() {

        Issue3601Mapper mapper = Mappers.getMapper( Issue3601Mapper.class );
        Issue3601Mapper.Source source = new Issue3601Mapper.Source();
        source.uuid = "some-uuid";

        Issue3601Mapper.Target target = mapper.map( source, null );

        assertThat( target ).isNotNull();
        assertThat( target.currentId ).isEqualTo( "some-uuid" );
        assertThat( target.targetIds ).isNull();

        target = mapper.map( source, Collections.emptyList() );

        assertThat( target ).isNotNull();
        assertThat( target.currentId ).isEqualTo( "some-uuid" );
        assertThat( target.targetIds ).isNull();

        ArrayList<String> sourceIds = new ArrayList<>();
        sourceIds.add( "other-uuid" );
        target = mapper.map( source, sourceIds );

        assertThat( target ).isNotNull();
        assertThat( target.currentId ).isEqualTo( "some-uuid" );
        assertThat( target.targetIds ).containsExactly( "other-uuid" );
    }
}

MapStruct Version

1.5.5, 1.6.0.Beta1

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions