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 class instantiation with optional parameters and default values [SPR-15673] #20232

Closed
spring-issuemaster opened this issue Jun 16, 2017 · 7 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Jun 16, 2017

Sébastien Deleuze opened SPR-15673 and commented

Following the work done on #19763, it seems we don't support currently Kotlin classes with default parameters since the bytecode generated contains 2 constructors.

This comment from Jayson Minard provide useful guidance. We could maybe take inspiration of https://github.com/FasterXML/jackson-module-kotlin/ that implements similar support.

This feature should if possible be reusable from Spring Boot for its support for @ConfigurationProperties and Spring Data which currently requires Kotlin noarg compiler plugin (I have validated it is currently required via MiXiT application, would be nice to be able to avoid using this plugin which is basicaly a trick for libraries without Kotlin support).

Juergen Hoeller I am going to try to write Kotlin tests that demonstrate the issue and try to find the right Kotlin API to use for that, I may need your guidance for the steps after that.


Issue Links:

  • #19763 Data binding with immutable objects (Kotlin / Lombok / @ConstructorProperties)
  • #20406 Kotlin bean instantiation regression with default declared constructor
  • #20101 BindingResult support for constructor argument mismatch on immutable data object
  • #20432 Revisit handling of missing fields (without default values) for immutable data classes
  • #20569 Streamline and reduce Kotlin delegates

Referenced from: commits d8e52c0, fa4d139, ab64305, 3991ab4, 40df7b6

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 23, 2017

Sébastien Deleuze commented

For the record, Jayson Minard provided these hints to help us based on his experience on the Jackson Kotlin module:

Jackson Kotlin Module looks for the Primary Constructor (if more than one) or the ONLY constructor (if only one).
https://github.com/FasterXML/jackson-module-kotlin/blob/master/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinModule.kt#L98

It also ensures that all parameters have name attributes.
Creation of object based on getting called back with a set of parameters is done at:
https://github.com/FasterXML/jackson-module-kotlin/blob/master/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 19, 2017

Sébastien Deleuze commented

Pull request submitted: #1482

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 27, 2017

Moritz Schulze commented

I would like to add that after the upgrade to Spring Boot 2.0.0 M3 this actually broke some Kotlin compability for me.

I had a @ConfigurationProperties class with a constructor that had default values for all parameters. This worked fine in M2

@ConfigurationProperties("foo")
class FooProperties(
    var bar: String = "bar",
    var baz: Int = 1
)

With M3 I get

Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [de.techdev.xyz.FooProperties]: No corresponding Kotlin constructor found
        at org.springframework.beans.BeanUtils$KotlinDelegate.instantiateClass(BeanUtils.java:742) ~[spring-beans-5.0.0.RC3.jar:5.0.0.RC3]
        at org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:165) ~[spring-beans-5.0.0.RC3.jar:5.0.0.RC3]
        at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:88) ~[spring-beans-5.0.0.RC3.jar:5.0.0.RC3]

So I had to change to

@ConfigurationProperties("foo")
class FooProperties {
    var bar: String = "bar",
    var baz: Int = 1
}

(which of cause is not that big of a hassle - I didn't use the constructor anywhere manually anyway).

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 27, 2017

Sébastien Deleuze commented

Moritz Schulze Could you please create a dedicated issue describing this regression? I will have a look before RC4.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 27, 2017

Moritz Schulze commented

I guess this ticket for Spring Boot will fix this in the end: spring-projects/spring-boot#8762, so maybe that is not necessary?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 2, 2017

Sébastien Deleuze commented

This is a regression reported already 2 times, so maybe there is something to improve. Please create an issue in order to make sure I have a look before RC4.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 7, 2017

Sébastien Deleuze commented

#20406 has been creted to track and fix this regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.