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

Refine ProjectedPayload argument handling for an optional input argument #550

Closed
rstoyanchev opened this issue Nov 30, 2022 · 2 comments
Closed
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Nov 30, 2022

A @ProjectedPayload method parameter can be declared with java.util.Optional as of #506, and that allows handling optional input arguments. We should allow it to be declared with ArgumentValue, introduced in #518.

If neither wrapper is used, currently ProjectionFactory fails with IllegalArgumentException if the input value is null. We should change that to allow null to be passed in if we want it to be consistent with the handling of regular @Argument method parameters. This allows the controller method to handle a null argument value.

Alternatively, we could keep the current behavior of the @ProjectionPayload argument handling, in effect requiring an optional wrapper for optional input, and otherwise rejecting it. In this case, we should also align the handling of regular @Argument parameters to do the same. That would be a breaking change, however, for an application that handles the null within he controller method.

@rstoyanchev rstoyanchev added the type: enhancement A general enhancement label Nov 30, 2022
@rstoyanchev rstoyanchev self-assigned this Nov 30, 2022
@rstoyanchev rstoyanchev added the for: team-attention An issue we need to discuss as a team to make progress label Nov 30, 2022
@rstoyanchev
Copy link
Contributor Author

Tentatively scheduling for 1.1.1 since this is a missed aspect of the argument binding improvements in 1.1. However, we should weigh in when to introduce the parts that bring breaking behavior.

@rstoyanchev rstoyanchev added this to the 1.1.1 milestone Nov 30, 2022
@rstoyanchev rstoyanchev changed the title Refine ProjectedPayload argument handling for a missing input value Refine ProjectedPayload argument handling for an optional input argument Nov 30, 2022
@rstoyanchev
Copy link
Contributor Author

We've discussed this and decided to change ProjectedPayloadArgumentResolver to allow null to be passed in. If an argument is required in the first place, it will be rejected by GraphQL Java. If it is optional and the method parameter is declared as an object, then that becomes null.

@rstoyanchev rstoyanchev removed the for: team-attention An issue we need to discuss as a team to make progress label Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

1 participant