GeezerVison is showing github.com — Kotlin: problems with data classes · Issue #2281 · mapstruct/mapstruct
Kotlin: problems with data classes #2281Description
- Is this an issue (and hence not a question)?
I'm trying to use MapStruct with kotlin codebase. For the most part i'm pretty successfull, but i have 2 issues that are weird and require workarounds that shouldn't be necessary.
First issue that i have is that if i try to map data class with single field i have to explicitly ignore "copy" target in mapper annotation because i'm getting warning "Unmapped target property: "copy"". When i add second field - it's no longer needed. Example:
data class SinglePropertyTest(val property: String)
data class SinglePropertyTestDto(val property: String)
data class DoublePropertyTest(val property1: String, val property2: Int)
data class DoublePropertyTestDto(val property1: String, val property2: Int)
@Mapper
interface SinglePropertyMapper {
// this gives me warning - Unmapped target property: "copy"
fun toDto(singlePropertyTest: SinglePropertyTest): SinglePropertyTestDto
// this works just fine
fun toDto(doublePropertyTest: DoublePropertyTest): DoublePropertyTestDto
}Seconds issue is more dangerous. If class have all fields nullable with default value set to null - mapper is silently failing to map any properties (all properties in target class are null). Example:
fun main(args: Array<String>) {
val allNullClass = AllNullClass("test", null)
val mapper = Mappers.getMapper(AllNullMapper::class.java)
val mappedDto = mapper.toDto(allNullClass)
// property1 here is null !
assert(mappedDto.property1 != null)
assert(mappedDto.property2 == null)
}
data class AllNullClass(
val property1: String? = null,
val property2: Int? = null
)
data class AllNullClassDto(
val property1: String? = null,
val property2: Int? = null
)
@Mapper
interface AllNullMapper {
fun toDto(allNullClass: AllNullClass): AllNullClassDto
}Generated code looks like that:
public class AllNullMapperImpl implements AllNullMapper {
@Override
public AllNullClassDto toDto(AllNullClass allNullClass) {
if ( allNullClass == null ) {
return null;
}
AllNullClassDto allNullClassDto = new AllNullClassDto();
return allNullClassDto;
}
}It seems like MapStruct is using the empty constructor in this case.
All in all - great library, almost perfect for my current usage! Reacted by Arne Seime, Morten Løcka, Nikolay Metchev, Olivier Voortman, Sergey Shumov, Julian Jarecki, edee111, Pasha Dymytriiev, Anderson Xavier, Jens and 36 more Reacted by Nikhil Sidhaye Activityfiliphr commented on Nov 25, 2020 Both problems seem related to the way the Kotlin byte code (AST) looks like when we try to create the mappers.
I guess that the first issue is due to the fact that Kotlin creates some method that looks like:
SinglePropertyTest#copy(String)
and since this is a fluent property we treat it as property. Seems like this property is not there when having multiple properties.
The second problem is the fact that Kotlin creates multiple constructors (one of them being the empty constructor) and thus MapStruct picking that one.
For both cases I have really limited knowledge of Kotlin and would appreciate any help. It would be good to know if there is some way to determine that a class is a kotlin data class during our compilation. If we can detect that then we can do something here. Reacted by Szukács Marcell and Martijn van der Blom jakoss commented on Nov 25, 2020 There are ways to detect the data class type in JVM, but they rely on internal implementation that might change (https://stackoverflow.com/a/29077162/939133). Nontheless i believe that's still better than nothing. data classes are really one of the best kotlin features and one that's almost always used for models and dtos. So this issues might be quite often hahmjt commented on Dec 8, 2020 I am having similar issue where Impl function does not have any sort of value assignment and does not use the Constructor to initialize the object. In Kotlin case, it would create an exception.
Here is the Impl method generated
public Dto entityToDto(Entity entity) {
if ( entity == null ) {
return null;
}
long arg0 = 0L;
String arg1 = null;
long arg2 = 0L;
int arg3 = 0;
Dto dto = new Dto( arg0, arg1, arg2, arg3 );
return dto;
}
Reacted by Svyatoslav Lebetsky, Łukasz Kokot and Danielle mortenlocka commented on Feb 13, 2021 I'm experiencing the same issue. Hoping for a fix! Iamdreamlover commented on May 6, 2021 I have same problem. Do someone has a solution? thank you a-ivanov commented on Jul 23, 2021 Easy fix is to ignore copy "property":
@Mapper
interface SomeMapper {
@Mapping(target = "copy", ignore = true)
fun toDto(src: Src): Dto
}This is nevertheless misleading. jakoss commented on Jul 23, 2021 @a-ivanov It's a workaround we have to live with, but i wouldn't call it a fix :) Reacted by Steve Hoang, Vladimir Kim and David Kovář Reacted by Anton Ivanov, d844022f2a734789aa637500a757671e, Ki and loalexzzzz sskrla commented on Jan 12, 2022 I also commented on #2577 which is a duplicate of one of the stated problems. Specifically Kotlin will generate an implicit constructor when all properties have a default. This causes model mapper to "fall out of" constructor mapping mode.
A relatively simple workaround is to annotate the constructor with @Default and introduce an explicit no-args constructor without this annotation. Simply supplying the annotation will copy it into the implicit no-arg constructor as well, causing Mapstruct to (I think I observed) non-deterministically select a mapping mode.
data class AllNullClassDto @Default constructor(
val property1: String? = null,
val property2: Int? = null
) {
constructor() : this(null, null)
}
It isn't pretty, and would be nice if the default behavior was the correct behavior, but it works. Reacted by Anton Koscejev and Sergey Kolychev Reacted by Nikhil Sidhaye and Vladimir Kim acoconut commented on Jan 27, 2022 @sskrla which package is this @Default you're using? filiphr commented on Jan 29, 2022 @acoconut have a look in the Non-shipped annotations section of our documentation. The @Default annotation can be in any package, it can be your own annotation as well. MapStruct only looks in the simple name of the annotation Reacted by Ana Cutillas and Kirill Gavrilov haghani-mehran commented on Feb 25, 2022
@sskrla which package is this @Default you're using?
Alternatively you could only make the no-args constructor private then you don't need to add the @Default annotation. Reacted by Anton Koscejev and Kirill Gavrilov koscejev commented on Mar 10, 2022 Here's another small improvement to the workaround. It's not necessary to call the constructor with values for all the fields that have defaults, but at least 1 field is required to distinguish the constructors:
data class AllNullClassDto(
val property1: String? = null,
val property2: Int? = null,
...
val property28: Long? = null,
) {
private constructor() : this(null)
}Unfortunately I haven't figured out how to apply this workaround to generated DTOs. :( Reacted by Steve Skrla, minyu-luo, dloukusa-bc and dirkbolte-otto grzegorz-grabarz commented on Apr 11, 2022 Hi, we also had problems with Kotlin's copy method. Similar error occurs when we define an instance method that takes one parameter as input and returns the containing class as a return type. (so any signature similar to the copy method)
The root cause of this behaviour is DefaultAccessorNamingStrategy's isSetterMethod.
It recognizes such functions as fluent setters.
Our solution was defining a CustomAccessorNamingStrategy and forgetting that fluent setters ever existed :)
class CustomAccessorNamingStrategy : DefaultAccessorNamingStrategy() {
override fun isSetterMethod(method: ExecutableElement): Boolean {
val methodName = method.simpleName.toString()
return methodName.startsWith("set") && methodName.length > 3
}
}Reacted by Lucas Laurindo dos Santos stravag commented on May 18, 2022
Easy fix is to ignore copy "property":
@Mapper
interface SomeMapper {
@Mapping(target = "copy", ignore = true)
fun toDto(src: Src): Dto
}This is nevertheless misleading.
This workaround only works if the data class has a single property, it fails when there is more than one:
error: Unknown property "copy" in result type
wirekang commented on Apr 22, 2023
Our solution was defining a CustomAccessorNamingStrategy and forgetting that fluent setters ever existed :)
We already using a spi-impl, so I can't implement a custom Strategy. It seems there is no way to use multiple Strategies. Do you have an idea? NikolayMetchev commented on Apr 22, 2023
Our solution was defining a CustomAccessorNamingStrategy and forgetting that fluent setters ever existed :)
We already using a spi-impl, so I can't implement a custom Strategy. It seems there is no way to use multiple Strategies. Do you have an idea?
We use a hack to incorporate both. Gradle can ransform the protobuf spi jar to remove one of the files in there and then we subclass the protobuf version wirekang commented on Apr 22, 2023 NikolayMetchev commented on Apr 22, 2023
@NikolayMetchev Wow Can you give more information about the gradle hack?
The idea is to delete the /META-INF/services/org.mapstruct.ap.spi.AccessorNamingStrategy file from protobuf-spi-impl.jar
build.gradle.kts
val artifactType = Attribute.of("artifactType", String::class.java)
val processed = Attribute.of("processed", Boolean::class.javaObjectType)
dependencies {
attributesSchema {
attribute(processed)
}
artifactTypes.getByName("jar") {
attributes.attribute(processed, false)
}
registerTransform(DeleteJarEntry::class) {
from.attribute(artifactType, "jar").attribute(processed, false)
to.attribute(artifactType, "jar").attribute(processed, true)
parameters {
fileNamePrefix = "protobuf-spi-impl"
fileToDelete = "/META-INF/services/org.mapstruct.ap.spi.AccessorNamingStrategy"
}
}
implementation(libs.mapstruct.protobuf)
}
configurations.all {
afterEvaluate {
if (isCanBeResolved) {
attributes.attribute(processed, true)
}
}
}
buildSrc/src/main/kotlin/DeleteJarEntry.kt
abstract class DeleteJarEntry : TransformAction<Parameters> {
interface Parameters : TransformParameters {
@get:Input
var fileToDelete: String
@get:Input
var fileNamePrefix: String
}
@InputArtifact
abstract fun getInputArtifact(): Provider<FileSystemLocation>
override fun transform(outputs: TransformOutputs) {
val input = getInputArtifact().get().asFile
if (input.name.startsWith(parameters.fileNamePrefix)) {
deleteJarEntry(input, parameters.fileToDelete)
}
outputs.file(getInputArtifact())
}
private fun deleteJarEntry(inputJar: File, fileToDelete: String) {
FileSystems.newFileSystem(inputJar.toPath(), mapOf("create" to "false")).use {
val pathToDelete = it.getPath(fileToDelete)
if (Files.exists(pathToDelete)) {
Files.delete(pathToDelete)
}
}
}
}
Then in your own gradle module have your own /META-INF/services/org.mapstruct.ap.spi.AccessorNamingStrategy file in module/src/main/resources/META-INF/services/org.mapstruct.ap.spi.AccessorNamingStrategy which will point to your class which will subclass no.entur.mapstruct.spi.protobuf.ProtobufAccessorNamingStrategy wirekang commented on Apr 22, 2023 birnbuazn commented on Jun 15, 2023
I don't know why people are downvoting the reply from @jakoss. This thread here has multiple workarounds for the constructor and copy problems.
I guess the reason is that the workarounds mentioned here are exactly what they are: workarounds. They work, but they are ugly.
This has nothing to do that I want a solution served on a silver plate. I'm just wondering why I have to add boiler plate to my Kotlin data classes, just for MapStruct to pick up the correct constructor. Reacted by Leon-Manolo, Ana Cutillas, Danielle , Soraia Basso, rafal-danowski-allegro, Steve Hoang, Ondřej Sedlár, kiturutin, Jakub Łaba and Vladimir Kim jakoss commented on Jun 15, 2023
I'm just wondering why I have to add boiler plate to my Kotlin data classes, just for MapStruct to pick up the correct constructor.
You don't have to, you can use something else. Konvert is (in my opinion) already ahead in some features in addition to being first class kotlin citizen Reacted by Ana Cutillas, i-avalos, Raphael De Lio, Vladimir Kim and Martin Timko filiphr commented on Jun 16, 2023
This has nothing to do that I want a solution served on a silver plate. I'm just wondering why I have to add boiler plate to my Kotlin data classes, just for MapStruct to pick up the correct constructor.
We are all working on MapStruct in our free time, so things get prioritised based on what we can do. Currently we are focused on 1.6. I have seen that people are interested in Kotlin, so I am going to see if we can focus on that for 1.7. Reacted by Thom, GeoffreyStemys, Bernard Laveaux, evef and Natalia Goncharova lucasls commented on Aug 16, 2023
Hi, we also had problems with Kotlin's copy method. Similar error occurs when we define an instance method that takes one parameter as input and returns the containing class as a return type. (so any signature similar to the copy method)
The root cause of this behaviour is DefaultAccessorNamingStrategy's isSetterMethod. It recognizes such functions as fluent setters.
Our solution was defining a CustomAccessorNamingStrategy and forgetting that fluent setters ever existed :)
class CustomAccessorNamingStrategy : DefaultAccessorNamingStrategy() {
override fun isSetterMethod(method: ExecutableElement): Boolean {
val methodName = method.simpleName.toString()
return methodName.startsWith("set") && methodName.length > 3
}
}
I have a question about the SPI solution from @grzegorz-grabarz and the hack by @NikolayMetchev. Does the SPI solution require the META-INF to be placed in a separate JAR? Is this a limitation of Java's SPI mechanism or a decision from MapStruct?
Defining a custom accessor strategy feels like the correct solution here, but I would prefer not to have to create a separate lib or module just for that. That's specially painful on Maven, given moving from single to submodule requires changing the whole structure of the project to have a parent project. Reacted by wirekang and Steve Hoang NikolayMetchev commented on Aug 16, 2023 In theory the protobuf-spi could have allowed extension on their end by whatever means they think most appropriate. Might be worth filing a feature request with them. I don't think it is an inherent problem with Java SPI. stevezuju commented on Oct 10, 2023 Any update on this guys ? added a commit that references this issue on Jan 29, 2026This issue will close once commit d300664 is merged into the 'main' branch.d300664
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|