spring-projects / spring-data-relational

Spring Data Relational. Home of Spring Data JDBC and Spring Data R2DBC.
https://spring.io/projects/spring-data-jdbc
Apache License 2.0
740 stars 342 forks source link

Kotlin value classes in entities fail with IllegalStateException #1762

Closed hajdamak closed 3 months ago

hajdamak commented 3 months ago

Although #1093 suggest otherwise following code fails on 3.2.4:

@JvmInline
value class EmailAddress(val theAddress: String)

@org.springframework.data.relational.core.mapping.Table
data class Contact(val id: String, val name:String, val emailAddress: EmailAddress)

@Repository
interface ContactRepo : CrudRepository<Contact, String> {
}

with:

Caused by: java.lang.IllegalStateException: A constructor parameter name must not be null to be used with Spring Data JDBC; Offending parameter: org.springframework.data.mapping.Parameter@a4a14c26
        at org.springframework.util.Assert.state(Assert.java:97)
        at org.springframework.data.jdbc.core.mapping.JdbcMappingContext.createPersistentEntity(JdbcMappingContext.java:73)
        at org.springframework.data.jdbc.core.mapping.JdbcMappingContext.createPersistentEntity(JdbcMappingContext.java:40)
        at org.springframework.data.mapping.context.AbstractMappingContext.doAddPersistentEntity(AbstractMappingContext.java:407)
        at org.springframework.data.mapping.context.AbstractMappingContext.addPersistentEntity(AbstractMappingContext.java:383)
        at org.springframework.data.mapping.context.AbstractMappingContext.addPersistentEntity(AbstractMappingContext.java:343)
        at java.base/java.lang.Iterable.forEach(Iterable.java:75)
        at org.springframework.data.relational.RelationalManagedTypes.forEach(RelationalManagedTypes.java:81)
        at org.springframework.data.mapping.context.AbstractMappingContext.initialize(AbstractMappingContext.java:519)
        at org.springframework.data.mapping.context.AbstractMappingContext.afterPropertiesSet(AbstractMappingContext.java:511)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1833)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1782)
mp911de commented 3 months ago

If you would like us to spend some time helping you to diagnose the problem, please spend some time describing it and, ideally, providing a minimal yet complete sample that reproduces the problem. You can share it with us by pushing it to a separate repository on GitHub or by zipping it up and attaching it to this issue.

spring-projects-issues commented 3 months ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

hajdamak commented 3 months ago

@mp911de Hi, here is minimal reproducible example: sdj-kotlin-value Just run it with mvn spring-boot:run and it will log exception. If you remove value class (val emailAddress: EmailAddress) from Contact DTO it will start just fine.

mp911de commented 3 months ago

@schauder For a reason, JdbcMappingContext.createPersistentEntity(…) checks constructor parameter names. This is at odds with the rest of Spring Data. Can we remove this check?

schauder commented 3 months ago

On one hand: git blame says it was introduced by @odrotbohm for performance reasons, so the harm in removing it would be limited. I don't even understand how this improves performance, except when on considers time until eventual failure.

On the other hand: Wouldn't it fail later on anyway, since we do need the names?

mp911de commented 3 months ago

Not necessarily as we have multiple instantiators that understand the Kotlin-specifics. For the parameter in question, DefaultConstructorMarker, the parameter is in the bytecode so it needs to be considered. However, the Kotlin-specific instantiator statically provides a null value to provide a value into the creation process.

schauder commented 3 months ago

Ok, I'll take the check out.

hajdamak commented 3 months ago

@schauder @mp911de Thanks.