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 Optional in PayloadMethodArgumentResolver #28945

Closed
artembilan opened this issue Aug 9, 2022 · 3 comments
Closed

Support Optional in PayloadMethodArgumentResolver #28945

artembilan opened this issue Aug 9, 2022 · 3 comments
Assignees
Labels
in: messaging Issues in messaging modules (jms, messaging) type: enhancement A general enhancement
Milestone

Comments

@artembilan
Copy link
Member

Sometime the data for payload method argument is optional and we cover that with a @Payload(required = false).
It would be nice if this PayloadMethodArgumentResolver would treat a @Nullable payload same way as that required = false.

Another improvement is around an Optional: the isEmptyPayload(@Nullable Object payload) should add a check for Optional.isEmpty().
Technically we cannot transfer a null in messaging at the moment, so the Optional.empty() is good compromise for us.
However this one probably should have an effect only if method param type is not an Optional, so no checks no conversions necessary. I assume this is an existing behavior.
What we need is just an automatic Optional resolution to null if @Nullable or @Payload(required = false).

Thanks

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 9, 2022
@rstoyanchev rstoyanchev added the in: messaging Issues in messaging modules (jms, messaging) label Jan 24, 2023
@rstoyanchev rstoyanchev self-assigned this Jan 24, 2023
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jan 24, 2023

I think you're suggesting to turn Optional into null if the method parameter is not Optional, and is not required. I think we could do that. Wouldn't we want the opposite too, i.e. wrap with Optional if the method parameter is Optional?

As for @Nullable @Payload payload this has to be decided more broadly for such annotated handler methods, including web where we also don't support that currently.

@artembilan
Copy link
Member Author

Hi @rstoyanchev !

Right, I concur with all your arguments.
Plus we'd like to have that check for Optional.isEmpty() in the isEmptyPayload(). In Spring Integration we are going to implement a Null payload feature: some users are requesting to be able to produce nulls from their POJO methods as valid result for a reply message.
Yes, we can revise @Nullable payload in the separate issue.

@rstoyanchev rstoyanchev added this to the 6.0.5 milestone Jan 30, 2023
@rstoyanchev rstoyanchev removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 30, 2023
@rstoyanchev
Copy link
Contributor

I've scheduled it for the Optional support improvements but I'll discuss the @Nullable and provide and update or create another issue if necessary.

@sbrannen sbrannen changed the title Improve org.springframework.messaging.handler.annotation.support.PayloadMethodArgumentResolver for better end-user experience Improve PayloadMethodArgumentResolver for better end-user experience Jan 30, 2023
@rstoyanchev rstoyanchev changed the title Improve PayloadMethodArgumentResolver for better end-user experience Support Optional in PayloadMethodArgumentResolver Feb 7, 2023
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed for: team-meeting labels Feb 7, 2023
rstoyanchev added a commit that referenced this issue Feb 13, 2023
mdeinum pushed a commit to mdeinum/spring-framework that referenced this issue Jun 29, 2023
mdeinum pushed a commit to mdeinum/spring-framework that referenced this issue Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: messaging Issues in messaging modules (jms, messaging) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants