Option to prevent fluent setter style

[davidkron]

Use case

We are using MapStruct to map between our applications model and the model of an external library we have no control over. For convenience, the authors of the external library have created additional withers for each property, something like this:

public class TargetType {

    public void setX(String x) {
        this.x = x;
    }
      
    public TargetType withX(String x) {
        setX(x);
        return this;
    }
}

The problem is, that MapStruct automatically accepts fluent setters as target properties and with a strict target policy, we have to additionally exclude every single wither, which can (depending on the model) immensely bloat up the list of ignored properties:

@Mapper
public interface MyMapper {

    @Mapping(target = "withX", ignore = true)
    @Mapping(target = "withY", ignore = true)
    @Mapping(target = "withZ", ignore = true)
    TargetType toTargetType(SourceType source);
}

Generated Code

I don't think any additional code generation is necessary, but more like an option, that could be set for a mapper:

@Mapper(namingPolicy = @NamingPolicy(allowFluentSetters = false, allowSetters = true))
public interface MyMapper {  
    TargetType toTargetType(SourceType source);
}

The default AccessorNamingStrategy should then respect these options.

Possible workarounds

Custom AccessorNamingStrategy which is a bit cumbersome as we can't just create a class in the project itself. It has to be in another project, which then gets put on the annotation processors classpath.

MapStruct Version

1.5.3

[filiphr]

I am a bit reluctant to expose the naming policy in an annotation.

Why is writing a custom AccessorNamingStrategy cumbersome? Adding a new module in the project and using it is not really difficult.

[davidkron]

First, sorry already for the long read. I hope this comment doesn't look like a big rant because I absolutely love MapStruct and wouldn't want to miss it in my projects. I just want to give some detailed thoughts where I or colleagues have encountered some special use-cases in real projects where we were wondering why this wasn't handled already by MapStruct in the default configuration. Maybe there are good reasons like technical limitations or the fact that our use-cases are just special snowflakes not worth the investment. Obviously it is the decision of the MapStruct team if my points are worth pursuing.

About implementing an AccessorNamingStrategy Maybe that was incorrectly worded by me. I don't think it is hard per-se to implement a custom strategy, the SPI is actually pretty straight forward. BUT only if you are knowledgeable about annotation processing and build debugging, and I don't think this is taught in Java 101, so this already limits the audience of developers capable of doing it.

I'm not sure about the multi-module argument. If the project is already structured that way, I agree. But nowadays with a lot of projects starting as microservices or even serverless functions, I don't agree that multi-module projects are necessarily the undisputed norm anymore like some years ago. And converting a perfectly fine single-module project that has been like this for many years to a multi-module one only to implement some special-case naming convention is IMHO not justifiable. JBang too might be some additional "single-module" use-case, where one writes some batch processing scripts to transform between esotheric models with different naming conventions.

Problem: AccessorNamingStrategy is global Another assumption from MapStruct is, that a naming strategy inside a project is global. Might be true in very small projects, but if they get bigger and have to interact with other systems, other ecosystems and 3rd party libraries, the chance is high that multiple naming conventions exist between different models. Although definitely not realistic, but in theory every single class in a project could have a different naming strategy. Also looking ahead at a future with project Panama, a lot of native library bindings with snake_case conventions might be usable in Java, which again might not be mapped straight-forward with the default MapStruct configuration. Another thing that disturbs me about setting a global naming strategy is the fact that it is a relatively high risk to break the whole application. Take for example my simple use-case from my original comment: if I wanted to implement a custom naming strategy for this 5% part of the application I would have to override the default naming strategy and could affect the generated mappers of the other 95%, which is not very elegant.

Idea I gave it some more thought and I do agree with you that my quick shot suggestion with concrete settings like allowFluentSetters seems pretty limiting and not flexible or easily extensible. But why couldn't MapStruct just choose on a per-mapper basis, which naming strategy should be used. Maybe even considering using the unix philosophy of "do one thing and do it well" so we could have predefined naming strategies of the ecosystem available like bean-style, fluent-style, wither-style, component-style and so on, instead of the current default one that handles multiple variants.

Also for special cases the per-mapper strategy could simply be changed by the developer:

@Mapper(unmappedTargetPolicy = ReportingPolicy.ERROR)
@MapperConfig(namingStrategy = "wither-style")
public interface MyMapper {  
    TargetType toTargetType(SourceType source);
}

I also think such an approach could solve many of the integration problems with other libraries like Lombok or Immutables as the default strategy for a specific target could simply be selected by the MapStruct compiler using some criteria like the presence of a Lombok annotation. This could even result in some handcrafted Lombok strategy that perfectly integrates.

Other options Even if this is not a direction MapStruct is willing to go, this would then beg some other questions:

• Withers are very common in the ecosystem, wouldn't it be justified to include it in the DefaultAccessorNamingStrategy?
• Even before JDK17, component-style accessors (without get prefix) were reasonably common and maybe even more now with records available. Shouldn't this then be also a default?

For component-style accessors there is even a nifty refactoring in IntelliJ called "convert record to class" which will break mapper compilation because records are presumably directly handled in MapStruct and component-style accessors on normal classes are not recognized by the DefaultAccessorNamingStrategy.

Even some popular libraries like Vert.x are using component-style accessors, so a simple mapper like this will not work by default for HttpServerRequest accessors like path() or uri():

@Mapper(unmappedTargetPolicy = ReportingPolicy.ERROR)
public interface RequestMapper {
    RequestInfo toRequestInfo(HttpServerRequest request);
}

[filiphr]

First, sorry already for the long read. I hope this comment doesn't look like a big rant because I absolutely love MapStruct and wouldn't want to miss it in my projects. I just want to give some detailed thoughts where I or colleagues have encountered some special use-cases in real projects where we were wondering why this wasn't handled already by MapStruct in the default configuration

Getting a constructive criticism and sharing of ideas is always good to improve the project.

Problem: AccessorNamingStrategy is global

I do agree about the problems of the accessor naming strategy being global.

Regarding your idea of exposing which naming strategy to use on a mapper level. If you think about it a bit it is something you'll want to expose on a type level and not a mapper level.

e.g. whether wither-style, bean-style, fluent-style, Lombok, Immutables etc. needs to be used is entirely based on the type that be being mapped to / from.

Currently, we only have a single strategy and based on whether Lombok / Immutables are available we pick a different global strategy. Ideally, we would be doing this based on the type from which we are getting the method. This way for the Lombok classes we are going to use the Lombok accessor naming strategy, for the Immutables we are going to use the immutables one etc.

This means that we might expose some properties (e.g. a property file in your project) that can instruct MapStruct which strategy to use for which types. An additional approach would be to allow to use any annotation on your types directly.

e.g.

Property file:

com.example.TargetType.setterTypes=bean
com.example.TargetType.getterTypes=bean
com.example.TargetType.setterPrefix=with
com.example.TargetType.strategy=snake_case

On a more granular level we can even go so far and look for annotations on the methods themselves

Withers are very common in the ecosystem, wouldn't it be justified to include it in the DefaultAccessorNamingStrategy?

We haven't really had anyone ask for withers support out-of-the-box in MapStruct.

Even before JDK17, component-style accessors (without get prefix) were reasonably common and maybe even more now with records available. Shouldn't this then be also a default?

We do have support for getters. For them we don't even go through the accessor naming strategy, since records are holders of data and they have an API that provides the the data that they hold.

Regular classes with component style getters is something that would be more tricky to implement, since any method with a return will become eligible for being a getter.

[davidkron]

I agree, naming strategy on a per-type level would certainly be better.

Also I really like your suggestion to configure the naming strategy using a property file. A helper annotation that does the same thing could also be an option for a better developer experience. But I think the properties file should be the baseline as I think it is more flexible, because you can define strategies for types you are not the owner of (definitely a use-case we have had).

In theory, this approach also seems to work well to integrate with libraries as the property file could simply be extended by an annotation processor (or SPI?):

• from MapStruct itself (out-of-the-box support for common libraries in the ecosystem)
• from the library itself by provoding an annotation processor to perfectly integrate with MapStruct
• from a 3rd party developer that provides a standalone integration

[filiphr]

But I think the properties file should be the baseline as I think it is more flexible, because you can define strategies for types you are not the owner of (definitely a use-case we have had).

I indeed had that option in mind. Most of the time you don't have control over the classes you are mapping, so having a property file where you can configure this would be quite easy. In addition per type for this I can also see that using a definition for particular package can also be interesting.

In theory, this approach also seems to work well to integrate with libraries as the property file could simply be extended by an annotation processor (or SPI?):

Exactly, the configuration can be multi fold (property file, user annotation, SPI). This allows for good flexibility and opens the door for the community to more easily configure things.

[durcon]

Maybe I can add something to the discussion.

I use jsonschema2pojo-maven-plugin which generates classes like:

@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonPropertyOrder({
    "url"
})
@Generated("jsonschema2pojo")
public class MyObject implements Serializable
{
    @JsonProperty("url")
    @JsonPropertyDescription("")
    @NotNull
    private String url;

    @JsonProperty("url")
    public String getUrl() {
        return url;
    }

    @JsonProperty("url")
    public void setUrl(String url) {
        this.url = url;
    }

    public MyObject withUrl(String url) {
        this.url = url;
        return this;
    }

If I map this class with:

@Mapper
public interface MyObjectMapper {

  @Mapping(source = "myOtherObject.childObject.url", target = "url")
  MyObject map(MyOtherObject myOtherObject);
}

Spring Tools Suite shows a warning (in tool tip and error tab) and yellow signs:

Unmapped target property: "withUrl".

At least it would be helpful, if setter and wither not handled as two different properties. If there is a setter and a builder method, only the setter should be used.

Maybe I could write an AccessorNamingStrategy, but I don't want to write Maven multi module projects for other reasons.

[filiphr]

We do not want to go in the direction and start treating withers as properties. We would instead prefer for people to write custom AccessorNamingStrategy or some other alternative (in a separate issue) where you would be able to define some rules for this

[jearton]

Any update?

[thunderhook]

No updates yet. The corresponding issue #1740 has the 1.7.0 milestone. We are currently concentrating on 1.6.0.