Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kotlin value classes in entities fail with IllegalStateException #1762

Closed
hajdamak opened this issue Apr 9, 2024 · 8 comments
Closed

Kotlin value classes in entities fail with IllegalStateException #1762

hajdamak opened this issue Apr 9, 2024 · 8 comments
Assignees
Labels
type: bug A general bug

Comments

@hajdamak
Copy link

hajdamak commented Apr 9, 2024

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)
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 9, 2024
@hajdamak hajdamak changed the title Kotlin value classes in entieties fail with Kotlin value classes in entireties fail with IllegalStateException Apr 9, 2024
@mp911de
Copy link
Member

mp911de commented Apr 10, 2024

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.

@mp911de mp911de added the status: waiting-for-feedback We need additional information before we can continue label Apr 10, 2024
@spring-projects-issues
Copy link

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.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Apr 17, 2024
@hajdamak
Copy link
Author

hajdamak commented Apr 22, 2024

@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.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Apr 22, 2024
@schauder schauder changed the title Kotlin value classes in entireties fail with IllegalStateException Kotlin value classes in entities fail with IllegalStateException Apr 22, 2024
@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Apr 24, 2024
@mp911de
Copy link
Member

mp911de commented Apr 24, 2024

@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
Copy link
Contributor

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
Copy link
Member

mp911de commented Apr 24, 2024

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
Copy link
Contributor

Ok, I'll take the check out.

schauder added a commit that referenced this issue Apr 24, 2024
For Kotlin constructors and possibly others parameters maybe unnamed and still work with our infrastructure.

Closes #1762
schauder added a commit that referenced this issue Apr 24, 2024
For Kotlin constructors and possibly others parameters maybe unnamed and still work with our infrastructure.

Closes #1762
@schauder schauder added this to the 3.1.12 (2023.0.12) milestone Apr 24, 2024
@hajdamak
Copy link
Author

@schauder @mp911de Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants