mapstruct / mapstruct

An annotation processor for generating type-safe bean mappers
https://mapstruct.org/
Other
7.05k stars 946 forks source link

Lombok's @Wither is affecting MapStruct #1740

Open apapenko opened 5 years ago

apapenko commented 5 years ago

So for some reason after update from 1.2.0.Final to 1.3.0.Final we experience this error:

/Users/username/workspace/project-name/src/main/java/com/company-name/packages/model
/mapping/BlahEntityMapping.java:36: error: Unmapped target properties: "withBlahField".
    BlahEntity toEntity(BlahDto blah, UserDetails userDetails);

With such class (actual contains 9 fields so it's best not to add ignore to @Mapping:

@Data
@Entity
@Wither <--- the creation of `with` methods is achieved by this annotation
@Builder
@NoArgsConstructor
@AllArgsConstructor
@Table(name = "\"blah_table\"")
@FieldDefaults(level = PRIVATE)
public class BlahEntity {
    String blahField;
}

Gradle: 5.2.1 Java: 1.8.0_161 OS: macOS 10.13.5 (17F77) Lombok's version: 1.18.6 Mapstruct version: 1.3.0.Final

build.gradle config:

dependencies {
    implementation('org.projectlombok:lombok')
    annotationProcessor('org.projectlombok:lombok')

    implementation("org.mapstruct:mapstruct-jdk8:${mapstructVersion}")
    implementation("org.mapstruct:mapstruct-processor:${mapstructVersion}")
    annotationProcessor("org.mapstruct:mapstruct-processor:${mapstructVersion}")
}
configurations {
    testImplementation.extendsFrom implementation
    testAnnotationProcessor.extendsFrom annotationProcessor
}

Maybe it makes sense to improve tests under https://github.com/mapstruct/mapstruct-examples/tree/master/mapstruct-lombok

As a temporary solution will just avoid usage of @Wither.

Thanks in advance!

filiphr commented 5 years ago

I presume the problem is that the wither methods are considered as properties. Are the withXXX properties added on the BlahEntity or on the builder?

I would advise in adding a custom AccessorNamingStrategy for that in which you won't consider them as setters.

Since 1.3 MapStruct would use the builder for creating the BlahEntity.

Decat-SimonA commented 5 years ago

The withXXX methods are on the BlahEntity:

@Wither
@Builder
@AllArgsConstructor
public class BlahEntity {

  String blahField;
}

is equivalent to


public class BlahEntity {

  String blahField;

  public BlahEntity(String blahField) {
    this.blahField = blahField;
  }

  public static BlahEntityBuilder builder() {
    return new BlahEntityBuilder();
  }

  public BlahEntity withBlahField(String blahField) {
    return this.blahField == blahField ? this : new BlahEntity(blahField);
  }

  public static class BlahEntityBuilder {

    private String blahField;

    BlahEntityBuilder() {
    }

    public BlahEntity.BlahEntityBuilder blahField(String blahField) {
      this.blahField = blahField;
      return this;
    }

    public BlahEntity build() {
      return new BlahEntity(blahField);
    }

    public String toString() {
      return "BlahEntity.BlahEntityBuilder(blahField=" + this.blahField + ")";
    }
  }
}
filiphr commented 5 years ago

Thanks @Decat-SimonA. I will try it out. However, if the builder support is not explicitly disabled then the builder class should be used for matching the properties. If the builder is not explicitly disabled this might be affected by https://github.com/mapstruct/mapstruct/issues/1581 as well

filiphr commented 5 years ago

Reading this again, I really don't see a way (short of a custom accessor naming strategy) for us to fix this. Since 1.3 we support fluent setters and the wither methods look like fluent setters so we consider them as fluent setters. Therefore I am going to close this one as won't fix

Decat-SimonA commented 5 years ago

Could you make an example repo with the custom AccessorNamingStrategy please ? I can't get it to work on my machine

baurceanu commented 4 years ago

@Decat-SimonA https://github.com/mapstruct/mapstruct-examples/tree/master/mapstruct-spi-accessor-naming

phazebroek commented 4 years ago

I'm running into the same issue. I tried to apply the code from the example but it's still throwing the warnings.

What else do I need to do other than defining META-INF/services with a reference to the CustomAccessorNamingStrategy class?

sjaakd commented 4 years ago

I'm running into the same issue. I tried to apply the code from the example but it's still throwing the warnings.

What else do I need to do other than defining META-INF/services with a reference to the CustomAccessorNamingStrategy class?

Bit difficult to gauge from this side. Can you adapt the SPI project above to your needs and leave the structure in-tact. The SPI for instance, needs to be in a separate project..

from this example, start with a minimal example that you can share so we can take a look.

filiphr commented 4 years ago

@sjaakd perhaps we should introduce a LombokAccessorNamingStrategy that would ignore those with methods if the class for the method is annotated with @Wither?

sjaakd commented 4 years ago

Hmm. checkout this. @Wither used to be experimental and is replaced with @With. So I guess both annotations need to be spotted based on name..

filiphr commented 4 years ago

@sjaakd nice for spotting it. Yes it would mean we need to support both. Although you could argue that we only support the Lombok supported feature, so @With in this case. @Wither is / was experimental

sjaakd commented 4 years ago

@filiphr . shall we reopen this issue? Or create a new one.

filiphr commented 4 years ago

I say let's reopen and improve the support here

phazebroek commented 4 years ago

Please note that it's not just only Lombok. It's also possible to generate objects with the jsonschema2pojo maven plugin for instance, and generate with... methods as well. No lombok involved, same issue. Let me try and setup something up.

phazebroek commented 4 years ago

I'm running into the same issue. I tried to apply the code from the example but it's still throwing the warnings. What else do I need to do other than defining META-INF/services with a reference to the CustomAccessorNamingStrategy class?

Bit difficult to gauge from this side. Can you adapt the SPI project above to your needs and leave the structure in-tact. The SPI for instance, needs to be in a separate project..

from this example, start with a minimal example that you can share so we can take a look.

Nevermind, fixed it / got it working. I somehow missed the pom.xml configuration including the annotationProcessorPaths.

Some notes though, it worked because the with... methods in the example work like setters. When using lombok with @Getter/@Setter and @With the with... methods return a new instance and the generated code in the MapperImpl is not working because it treats the with methods as setters. Ignoring the new instance that is actually returned.

To be more precise, here are some code snippets: Generated using lombok's @With:

  public GolfPlayer withName(String name) {
    return this.name == name ? this : new GolfPlayer(this.handicap, name);
  }

Generated mapping logic:

        GolfPlayer golfPlayer = new GolfPlayer();
        golfPlayer.withName( player.getName() ); // withName returning new instance instead of modifying instance referenced by golfPlayer.
        return golfPlayer;
shollander commented 4 years ago

I would be nice if MapStruct supported "wither" methods out of the box as an alternative to setters and builders. Using withXXX methods that return an immutable copy is an established pattern. The use of the "with" prefix is to distinguish it from a setter where the original copy is modified. With indicates that a new copy of the object is returned "with" the requested modification.

polarfish commented 4 years ago

Another thing to keep in mind is that jsonschema2pojo generates regular fluent setters when Lombok's @With generates copy-on-write fluent setters. So if you implement custom accessor naming strategy treating your with* methods as setters, make sure they are not taken for code generation (instead of plain set* methods), otherwise your generated mapper will not work (see example below).

if ( source.getCreatedBy() != null ) {
    target.withCreatedBy( source.getCreatedBy() );
    // new instance of target was created and ignored
}
if ( source.getCreatedAt() != null ) {
    // at this point target's createdBy is null
    target.withCreatedAt( source.getCreatedAt() );
}
polarfish commented 4 years ago

My CustomAccessorNamingStrategy that doesn't recognize with methods as setters:

public class CustomAccessorNamingStrategy extends DefaultAccessorNamingStrategy {

    @Override
    protected boolean isFluentSetter(ExecutableElement method) {
        return !isWitherMethod(method) && super.isFluentSetter(method);
    }

    protected boolean isWitherMethod(ExecutableElement method) {
        String methodName = method.getSimpleName().toString();
        return methodName.length() > 4 && methodName.startsWith("with") && Character.isUpperCase(methodName.charAt(4));
    }
}
sukys commented 1 year ago

Sorry but the documentations is very poor. Is there an example with gradle? Without using parent would also be great. thanks.

nbrugger-tgm commented 10 months ago

Related/Duplicate: #3211

mors741 commented 7 months ago

If you have not too many uses of @With annotation, then as a workaround you can replace @With with @Builder(toBuilder = true) MapStruct works correctly with @Builder annotation.

In this case you will replace this client code

blahEntity
    .withBlahField1()
    .withBlahField2()

with this

blahEntity.toBuilder()
    .blahField1()
    .blahField2()
    .build();

As far as I understand, you will need to add the following mapper configuration @Mapper(builder = @Builder(disableBuilder = true))