#3539 Support BeanMapping#ignoredTargets instead of repeating @Mapping(target = "foo", ignore = true)

[chenzijia12300]

This commit attempts to implement #2599, mapping BeanMapping#ignoredTargets to multiple Mapping#ignore annotations.

If there are any necessary changes, I'll do my best to address them.

[filiphr]

Thanks for your PR @chenzijia12300.

Can we perhaps add a test case when an unknown property is used in BeanMapping#ignoreTargets? How would the compile error look like? Where would it be reported?

[chenzijia12300]

Hi @filiphr, thanks for your review, I have added a test case

[zyberzebra]

Just checked it out and had a look :)

[zyberzebra]

Shouldn't there be an assertion? :)

    public void shouldIgnoreTargetProperty() {
        Source source = new Source();
        Source.NestedSource nested = new Source.NestedSource();
        source.setAge( 18 );
        source.setName( "John" );
        source.setNested( nested );
        nested.setScore( 50.0 );
        Target convert = BeanMappingIgnoreTargetMapper.INSTANCE.convert( source );
        assertThat(convert).hasNoNullFieldsOrPropertiesExcept( "address" );
        assertThat(convert.getNested()).hasNoNullFieldsOrPropertiesExcept( "flag" );
    }

SHouldn't we add a test like that? :) Or is it enough thats it does not generate an error that we have unmapped targets

[filiphr]

I agree about this one. We should add an assertion as well.

[zyberzebra]

Maybe a new method?

[zyberzebra]

I guess we have to update the intelliJ plugin if this is implemented ;)

[zyberzebra]

    @Mapping( target = "address", ignore = true)
    @BeanMapping( ignoreTargets = {"nested.flag", "address"} )
    Target convert( Source source );

Should this lead to warning? What happens if we do that.

[zyberzebra]

Shouldn't this also lead to: Target property "address" must not be mapped more than once.

    @BeanMapping( ignoreTargets = {"nested.flag", "address", "address"} )

this leads to: Target property "address" must not be mapped more than once.

    @Mapping( target = "address", ignore = true)
    @Mapping( target = "address", ignore = true)

does

[filiphr]

I think that the best solution for this would be to do it in the MapperCreationProcessor similar to how we are doing it for @BeanMapping( ignoreByDefault = true ).

Additionally, as @zyberzebra mentioned in some other comments. We need to make sure that people do not use duplicates and that you can't define things in @BeanMapping#ignoreTargets and @Mapping in the same time.

[filiphr]

The reported line should be on the @BeanMapping not on the method.

[filiphr]

I agree about this one. We should add an assertion as well.

[chenzijia12300]

@zyberzebra @filiphr Thank you very much for your review. I will try to modify these problems.