fix issue 194

[hduelme]

I fixed #194. Now conditionQualifiedByName is accepted as a valid source property. ignoreByDefault was already handled before.

[filiphr]

Thanks @hduelme. Looking at this, I think that there are more things missing here.

• dateFormat
• numberFormat
• qualifiedBy
• conditionQualifiedBy
• nullValueCheckStrategy
• nullValuePropertyMappingStrategy
• mappingControl

[filiphr]

Can we perhaps use #194 to align this check with what we are doing in core?

[hduelme]

@filiphr I did a bit of testing. It seems that the properties

• conditionQualifiedByName
• conditionQualifiedBy
• dateFormat
• numberFormat
• nullValueCheckStrategy
• nullValuePropertyMappingStrategy
• mappingControl don't effect the core check.

Here a example:

import org.mapstruct.*;
import org.mapstruct.control.DeepClone;

import java.time.LocalDate;

@Mapper
public interface NoSourceTestMapper {

    public class Target {
        private LocalDate t;

        public LocalDate getT() {
            return t;
        }

        public void setT(LocalDate t) {
            this.t = t;
        }
    }

    class SourceMatch {
        private LocalDate t;

        public LocalDate getT() {
            return t;
        }

        public void setT(LocalDate t) {
            this.t = t;
        }
    }

    class SourceNoMatch {
        private LocalDate n;

        public LocalDate getN() {
            return n;
        }

        public void setN(LocalDate n) {
            this.n = n;
        }
    }

    // Valid
    @Mapping(target = "t")
    Target match(SourceMatch sourceMatch);

    // Not valid
    @Mapping(target = "t", dateFormat = "dd", conditionQualifiedByName = "notEmpty",
            nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS,
            nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE,
            mappingControl = DeepClone.class)
    Target noMatch(SourceNoMatch sourceMatch);

    @Condition
    @Named("notEmpty")
    default boolean notEmpty(LocalDate s) {
        return s != null;
    }
}

The only difference I could find, is that the core implementation doesn't report a error if there is a source property with the same name.

[filiphr]

@hduelme what do you mean by they do not affect core? What I meant is that in this PR it looks go me like if target is matching a source property then the plugin reports an error, even when core doesn't.

Perhaps what you are doing with PR #198 is solving this one. It feels like the plugin should only do the no source defined validation if there is no matching source property. I believe this could make the plugin more robust towards new properties in core.

Just FYI, I'm traveling at the moment, so it might take me some time to come back and review this properly

[hduelme]

@filiphr I meant that adding the properties doesn't change the behavior. Even with the properties I get a error. Only if a source property with the same name exists the error goes away.

No problem. Have a good trip

[filiphr]

@filiphr I meant that adding the properties doesn't change the behavior. Even with the properties I get a error. Only if a source property with the same name exists the error goes away.

OK, I see what you mean. Yes you are right, when a source property with the same name exists, then adding all of those is correct.

My main use case is exactly that, the source and target match and I want to only customize something. Can we perhaps adjust this check to only report the error when there is no matching source property. Basically from your example

@Mapping(target = "t", dateFormat = "dd", conditionQualifiedByName = "notEmpty",
            nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS,
            nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE,
            mappingControl = DeepClone.class)

the mapping above is correct when source and target have the same property. The plugin should behave the same. Currently, the plugin says that there is an error and "No source property defined".

[hduelme]

Yes that is exactly what I meant. I implemented the check if a source property exists in #198. I think this one should be closed as I doesn't fix the bug.

[filiphr]

Sorry, I missed PR #198. That indeed looks like the correct fix for #194. I'll go ahead and close this one in favor of PR #198