Kotlin data classes and mapstruct

[Pozo]

Related to #1298 and #782

A few months ago I came to the same conclusion with the all-open and the no-arg plugin like @thePhil and @nilotpal1981. However I really wanted to avoid using something like

data class Person(var firstName: String?, var lastName: String?, var phoneNumber: String?, var birthdate: LocalDate?) {
    // Necessary for MapStruct
    constructor() : this(null, null, null, null)
}

Luckily since mapstruct 1.3.0.Beta2 it's possible to use builders for immutable classes. According to the documentation we can implement our custom builder provider logic. And the holiday season just came :) and I could create a proof of concept which is focusing on Kotlin's data classes.

So you can write down something like this:

@KotlinBuilder
data class Person(var firstName: String, var lastName: String, var phoneNumber: String, var birthdate: LocalDate)

The main idea is generating a builder for each data class and pass these builders to mapstruct via a custom BuilderProvider.

As I mentioned this is a proof of concept repository but I want to develop it further. If you think that this might be a way to use mapstruct with kotlin please share your idea or request and create an Issue here:

https://github.com/Pozo/mapstruct-kotlin

[filiphr]

I had a brief look at the repository. The idea is OK. However, I think that implementing #73 is the correct way for implementing this. When MapStruct can use constructors for creating objects there won't be any difference between a normal Java class with constructor and Kotlin data classes.

What do you think about #73?

[ahulyk]

@filiphr any news on that?

[ahulyk]

@Pozo i like your approach. It looks ok for now. Do you have plans to release it?

[gildor]

However I really wanted to avoid using something like

Why not just add default params to primary constructor?

data class Person(
    var firstName: String? = null, // may be not only null
    var lastName: String? = null,
    var phoneNumber: String? = null,
    var birthdate: LocalDate? = null
)

In this case default constructor also will be created

[ahulyk]

@gildor
In most cases we want to avoid mutability (especially on DTO layer):

data class Person(
    val firstName: String,
    val lastName: String,
    val phoneNumber: String,
    val birthdate: LocalDate
)

[Pozo]

@filiphr Thank you for the information. I read through the #73 and I must agree with you. That would be a more generic way to achieve immutable class mapping, however my work was focusing on Kotlin’s data classes.

When I started the implementation I’ve found #847 PR which seemed abandoned, and a few comments #73 (comment) #73 (comment) about the difficulties of a generic solution, so I decided to go only for Kotlin’s data classes.

@ahulyk Thank you for your interest, however as I mentioned this is only a proof of concept right now. I just want to see how many people agree with this idea so I don’t plan to create any release yet.

@gildor Thank you four your suggestion, however in this case I'm agreeing with @ahulyk, When I’m using pure Kotlin I really want to avoid something like you just wrote. I understand that it’s working but I think we should use Kotlin in an idiomatic way when It’s possible and one of It’s strengths is the immutability by default.

[FearlessHyena]

Although @Pozo 's implementation is still PoC I still agree with @ahulyk that it's a cleaner and better approach than having to define a mutable data class
Any plans on releasing or maybe having something similar for Kotlin?

[Pozo]

@FearlessHyena I decided to push it a bit forward. I've made several adjustment on the repository today, so you can try it out with the help of jitpack.io. Don't forget to raise a ticket or open an issue if you have something to share.

[FearlessHyena]

@Pozo This is great!!! :D
Currently using this in my project and it works without any issues 👍
I'll raise a ticket if I notice any problems
It would be really nice if this or atleast something similar were merged into the main project. So much cleaner to use!

[Sam-Kruglov]

@Pozo works for me, thank you!
my configuration:

...
<dependency>
    <groupId>org.mapstruct</groupId>
    <artifactId>mapstruct</artifactId>
    <version>${mapstruct.version}</version>
</dependency>
<dependency>
    <groupId>com.github.pozo.mapstruct-kotlin</groupId>
    <artifactId>mapstruct-kotlin</artifactId>
    <version>${mapstruct.version}</version>
</dependency>
...
<executions>
    <execution>
        <id>compile</id>
        <goals>
            <goal>compile</goal>
        </goals>
    </execution>
    <execution>
        <id>kapt</id>
        <goals>
            <goal>kapt</goal>
        </goals>
        <configuration>
            <sourceDirs>
                <sourceDir>src/main/java</sourceDir>
            </sourceDirs>
            <annotationProcessorPaths>
                <annotationProcessorPath>
                    <groupId>com.github.pozo.mapstruct-kotlin</groupId>
                    <artifactId>mapstruct-kotlin-processor</artifactId>
                    <version>${mapstruct.version}</version>
                </annotationProcessorPath>
            </annotationProcessorPaths>
            <annotationProcessorArgs>
                <arg>mapstruct.defaultComponentModel=spring</arg>
                <arg>mapstruct.unmappedTargetPolicy=ERROR</arg>
            </annotationProcessorArgs>
        </configuration>
    </execution>
</executions>

Also, it creates an empty folder under target/generated-sources/kaptKotlin.

[FearlessHyena]

Hi @Pozo I've been using it for a few weeks now and haven't seen any issues
Any plans on removing that Beta tag and making this official? :)

[Sam-Kruglov]

Hey, @Pozo one more vote for production!

[lkjh654]

@Pozo +1

[Sam-Kruglov]

I would make 1 comment: instead of MyClassBuilder.builder().setField("value").create() I would prefer MyClassBuilder.builder().field("value").build(). Don't know if mapstruct will work with that though

[skuznets0v]

@Pozo, +1

[Sam-Kruglov]

Found one thing:

@KotlinBuilder
data class Outer(
        val id: String
){
        @KotlinBuilder
        data class Inner(val id: String)
}

generates this:

import java.lang.String;

public final class InnerBuilder {
  private String id;

  public InnerBuilder setId(String id) {
    this.id = id;
    return this;
  }

  public Inner create() {
    return new Inner(id);
  }

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

Which does not compile because method create does not import the inner object. Correct would be this:

import java.lang.String;

import static bla.Outer.*;

public final class InnerBuilder {
  private String id;

  public InnerBuilder setId(String id) {
    this.id = id;
    return this;
  }

  public Inner create() {
    return new Inner(id);
  }

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

note: hand-written examples

[Pozo]

I appreciate your interest guys but we should keep this thread clean. To my mind this topic is only for collecting ideas about mapstruct with kotlin, and my approach is just one implementation.

So if you have an issue/idea with my approach please create a ticket there. In this case others can find the relevant information more easily.

[Sam-Kruglov]

It's been open for almost a year. Any plans on it?

[sjaakd]

It's been open for almost a year. Any plans on it?

For sure.. To do it right is a major piece of work and we sure like to have it. But it is not as simple as the abandoned PR suggests 😄. I even think @filiphr was working on it.

[Pozo]

I just released https://github.com/Pozo/mapstruct-kotlin to the maven central. @filiphr I think we can close this issue.

[filiphr]

Since 1.4 MapStruct has support for using constructor arguments when instantiating mapping targets. This also works with Kotlin data classes.

Therefore, I am closing this issue

[HomoEfficio]

@filiphr It works good on Entity(not data class) -> DTO(data class).
But It does not seem to work on the other way around(DTO -> Entity).

Or if I missed something please let me know what is wrong.

version: mapstruct 1.4.2.Final

[filiphr]

@HomoEfficio I do not know much about Kotlin, but looking at the difference between val and var it seems that val cannot be assigned more than once, i.e. they don't have setters.

I have no idea how the generated constructor looks like for this class in Kotlin. We are only looking into what the Java compiler sees and we use that.

[HomoEfficio]

@filiphr As you can see in the upper right side of the picture, values of DTO(WorldOut) are initiated using constructor with params. And it works OK.
But on the bottom right side of the picture, values of Entity(World) are set using setter with default constructor. And it does not work.

So to fix this issue, why don't you use constructor with params instead of default constructor with setters to create an Entity, just like creating DTO with constructor with params. Then It will work OK regardless of val or var.

[cujo]

The problem with Kotlin classes is when all contructor properties are optional:

data class User(
    val id: UUID? = null,
    val email: String? = null,
    val phoneNumber: String? = null
)

In this case compiler creates additional parameterless constructor which Mapstruct will use then by design, and so no actual mapping happens:

User user = new User();

return user;

As a possible workaround we can directly annotate data class constructor:

data class User @Default constructor(
    val id: UUID? = null,
    val email: String? = null,
    val phoneNumber: String? = null
)

But moving this annotation to domain layer just for compatibility with some infrastructure code seems to be very wrong. My proposal is to add special Mapper parameter to avoid choosing empty constructor as default one when another with params exists. This should help in most cases.

[npwork]

The problem with Kotlin classes is when all contructor properties are optional:

data class User(
    val id: UUID? = null,
    val email: String? = null,
    val phoneNumber: String? = null
)

In this case compiler creates additional parameterless constructor which Mapstruct will use then by design, and so no actual mapping happens:

User user = new User();

return user;

As a possible workaround we can directly annotate data class constructor:

data class User @Default constructor(
    val id: UUID? = null,
    val email: String? = null,
    val phoneNumber: String? = null
)

But moving this annotation to domain layer just for compatibility with some infrastructure code seems to be very wrong. My proposal is to add special Mapper parameter to avoid choosing empty constructor as default one when another with params exists. This should help in most cases.

Still reproduces :(

[filiphr]

So to fix this issue, why don't you use constructor with params instead of default constructor with setters to create an Entity, just like creating DTO with constructor with params. Then It will work OK regardless of val or var.

@HomoEfficio we can't do that due to backwards compatibility. I think that for your example there is more than one constructor visible to us.

My proposal is to add special Mapper parameter to avoid choosing empty constructor as default one when another with params exists. This should help in most cases.

This is not as trivial as you think @cujo.

I would suggest that you follow #2378 that is meant to address this particular problem.