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

Support Kotlin nullable information in Spring MVC parameters [SPR-14165] #18737

Closed
spring-issuemaster opened this issue Apr 13, 2016 · 13 comments

Comments

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

commented Apr 13, 2016

Raman Gupta opened SPR-14165 and commented

Spring MVC now supports defining fields with the JDK 8 Optional type to indicate that the parameter is optional (cool!).

Kotlin has fields which can be marked as a nullable type e.g. String? vs. String. This information should be used by Spring MVC in the same way as Optional.

See: KType documentation


Affects: 4.2.5

Issue Links:

  • #19518 Support Kotlin nullable information for @Autowired and @Inject
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 14, 2016

Sébastien Deleuze commented

Very good idea! I would maybe suggest to put such Kotlin specific functionalities in https://github.com/sdeleuze/spring-kotlin for now. This would allow us to add it with other various Kotlin related extensions, and evaluate later if we integrate it as an official https://github.com/spring-projects project or not. If that fine to you, could you please create an issue here?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 18, 2016

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 18, 2016

Sébastien Deleuze commented

Raman Gupta Thanks for creating the other issue, but after discussing with the team we are ok to work on this issue directly as a pull request for https://github.com/spring-projects/spring-framework. Depending on the amount of changes needed, we will evaluate if we can integrate that directly in Spring Framework or not. Are you interested in contributing a PR that I could review using existing Optional support as a template?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 18, 2016

Raman Gupta commented

@Sebastien Deleuze, great! I can take a look at writing up a pull request over the next week or two, if the effort required is not too great. One question: is it ok to have/introduce a dependency on Kotlin? I believe I would need to use Kotlin reflection in order to implement this, so the equivalent of the ObjectToOptionalConverter for Kotlin nullables (ObjectToKtNullableConverter ?) would probably be best written in Kotlin, or at least in Java with some type of Kotlin helper for the reflection bits.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 18, 2016

Sébastien Deleuze commented

Awesome, thanks. It is fine to introduce an optional dependency in build.gradle, and to enable such feature only when Kotlin is on the classpath, as I did here.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 14, 2016

Raman Gupta commented

@Sébastien Deleuze Please check out this pull request for my draft implementation: #1060

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 8, 2016

Sébastien Deleuze commented

Raman Gupta Thanks for your PR, I have reviewed it and added my comments on GitHub. Please let me know if you can take my feedbacks in account, in order to move forward and see with the team if we can integrate that support in Spring Framework 5.0.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 8, 2016

Raman Gupta commented

Thanks for the review. I will take a look over the next few days and update the PR.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 22, 2016

Raman Gupta commented

Sébastien Deleuze I've updated the pull request as requested.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 21, 2016

Sébastien Deleuze commented

I have polished, refactored and added more tests to your commit via this new PR, and have requested a review from Juergen since these are non-trivial additions.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 24, 2016

Sébastien Deleuze commented

I merged a refactored version that reuses MethodParameter#isOptional(), with more tests and polish (see this additional commit)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 24, 2016

Sébastien Deleuze commented

Raman Gupta Thanks for your contribution, I will work on @Autowired and @Inject support as part of #19518 (should be part of 5.0.0.M4).

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 24, 2016

Raman Gupta commented

Sébastien Deleuze Awesome, thanks!

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.