Opt-out of using builders via a processor option

[twentylemon]

Possibly related to #1547

I've recently been playing with 1.3 (from 1.2), testing out builder/immutable object support. I'm trying to map between 2 classes, a service layer DTO and our internal model representation. Currently, our internal model is a lombok @Data class, as mapstruct requires mutable objects. Ideally, we'd move to @Value or otherwise make an immutable object, as part of the 1.3 upgrade.

The DTO is generated, mutable and comes with a builder class, the builder has setters named withAttribute. Mapstruct 1.3 doesn't know how to map these out of the box -- it tries to use the builder then complains about not mapping withAttribute. I started trying out the AccessorNamingStrategy spi. It works, but I have to support all strategies with a single class (mapstruct throws if you provide more than 1 implementation).
Aside: I found this to be odd, as ServiceLoaders typically get multiple implementations of a service (like Processor). This would allow me to write a single class to support a withAttribute naming strategy, without having to worry about supporting setAttribute or attribute still. Mapstruct could in theory support multiple by trying all implementations before being unable to find a method.
While, strictly speaking, this universal naming strategy worked, I didn't feel confident that the strategy would cover all existing cases across all of our code, and future cases may be hard to integrate and be blocked.

So. Not overly confident in that, I searched for some way to simply turn off builder support. It seems this is only possible mapstruct-wide.

In case you want to disable using builders then you can use the NoOpBuilderProvider by creating a org.mapstruct.ap.spi.BuilderProvider file in the META-INF/services directory with org.mapstruct.ap.spi.NoOpBuilderProvider as it’s content.

Not ideal, as I wanted to change our internal model to be immutable and don't want to support withAttribute through the spi. For my situation, I'd rather see a way to specify this method, use the builder; this method, it's just a bean. Specifying an arg as "don't use builders" would be a seamless way for me to upgrade, but mapstruct's default could be "autodetect builders" as it currently does. Overriding this config at a mapper or method level would allow me to opt-in to new features, so the new version is fully backward compatible.

[filiphr]

I started trying out the AccessorNamingStrategy spi. It works, but I have to support all strategies with a single class (mapstruct throws if you provide more than 1 implementation).

Not sure how you implemented the AccessorNamingStrategy and whether you extended from the DefaultAccessorNamingStrategy it is quite easy to write one when doing the extension. Have a look at CustomAccessorNamingStrategy from our tests. That one supports prefix with for accessors.

Yes we know that the SPI is designed for multiple implementations. However, our SPI is not like that. The reason is that the SPI has methods that return names of the property for a certain accessor. If there are more, how do we know which one to use to get the name as all of them would return a certain name.

For my situation, I'd rather see a way to specify this method, use the builder; this method, it's just a bean.

I think that we could actually do this by adding a new property to @BeanMapping, @Mapper and @MapperConfig. As the locations where we need to get the getters / setters is in a place where we have access to the method.

Overriding this config at a mapper or method level would allow me to opt-in to new features, so the new version is fully backward compatible.

We made it on purpose opt-out now, otherwise it won't work ootb for people. And as you noticed you can still opt out (globally) via the NoOpBuilderProvider

[twentylemon]

Not sure how you implemented the AccessorNamingStrategy and whether you extended from the DefaultAccessorNamingStrategy it is quite easy to write one when doing the extension. Have a look at CustomAccessorNamingStrategy from our tests. That one supports prefix with for accessors.

That was my approach, following the example toward the end of the reference guide, but essentially decorating the default one to account for withAttribute in addition to whatever the default covers. I mentioned I did get it to work, though wasn't confident it'd work universally in our entire code base.

Yes we know that the SPI is designed for multiple implementations. However, our SPI is not like that. The reason is that the SPI has methods that return names of the property for a certain accessor. If there are more, how do we know which one to use to get the name as all of them would return a certain name.

Fair point. It seems like that problem would exist given a single implementation which covers several naming strategies. For example, mixing a java-bean with a fluent strategy, the attribute name would be ambiguous for a attribute like isStuff. Granted, a bad name, but I see booleans like that all the time.
With multiple implementations, you could re-call isGetterMethod before extracting the attribute name out of the method name so you don't operate on garbage. Naively, they seem equivalent then, but the implementations are easier to write, test, etc, all the stuff that comes with smaller classes.
I suppose the difference here is you force the single implementation to resolve any ambiguities, rather than mapstruct trying to figure that out for the general case.

For my situation, I'd rather see a way to specify this method, use the builder; this method, it's just a bean.

I think that we could actually do this by adding a new property to @BeanMapping, @Mapper and @MapperConfig. As the locations where we need to get the getters / setters is in a place where we have access to the method.

Great, that's along what I had envisioned. Globally via a processor option seems reasonable to me as well, though it steps on the toes of the NoOpBuilderProvider a little bit.

We made it on purpose opt-out now, otherwise it won't work ootb for people. And as you noticed you can still opt out (globally) via the NoOpBuilderProvider

Fair point! I suppose of the two options, upgrade almost-blocker vs onboard blocker, almost-blocking my upgrade is better than telling others they can't use mapstruct.

[filiphr]

This would be resolved once we merge #1811

[sjaakd]

resolved.. Not sure we have it on all levels yet.. but we have it on bean mapping and mapper level.

[sjaakd]

Processor option and mapperconfig missing

[filiphr]

The mapper config option was added in ef270ca as part of the implementation for #1479.

The final thing that is missing is the global processor option