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

JSR-305 @Nullable cannot be used to indicate that a parameter to an endpoint operation is optional #24647

Closed
nathankooij opened this issue Jan 5, 2021 · 6 comments
Assignees
Milestone

Comments

@nathankooij
Copy link

@nathankooij nathankooij commented Jan 5, 2021

In #12020 support for optional parameters was introduced which is great. We have been using this happily for a while internally. Recently we decided as part of streamlining efforts to deduplicate several annotations that have been used (mostly) interchangeably within our code base. One such annotation was @Nullable. As we also have (Maven) modules that do not rely on Spring, we decided to only use javax.annotation.Nullable from that point on, because as far as we know, this is supported by Spring. After settling on this rule, and migrating our code base (where we have now banned the other annotations at the build level), a custom endpoint of the following form started to fail:

@Endpoint(id = "endpoint")
final class Endpoint {

    @WriteOperation
    void adjust(@Selector someSelector, @Nullable optionalParameter) {
        ...
    }

}

The @Nullable annotation was migrated from org.springframework.lang.Nullable to javax.annotation.Nullable, and thus support for the optional parameter broke as a result of:

Now we realize that this is indeed documented as such here, so this is definitely not a bug report, but rather a feature request. This would allow us to keep the consistency within our code base and keep our strict rule set without having to support exceptions. Moreover, it could be less of a surprise for others since there's support for it in other parts of Spring. If it helps, I'm willing to do the legwork required and file a PR.

If any more info is required, let me know.

@philwebb
Copy link
Member

@philwebb philwebb commented Jan 5, 2021

I think we could consider this a bug. It seems reasonable that you should be able to use javax.annotation.Nullable.

@wilkinsona wilkinsona changed the title Support optional parameters for actuator Endpoints with javax.annotation.Nullable javax.annotation.Nullable cannot be used to indicate that a parameter to an endpoint operation is optional Jan 5, 2021
@philwebb philwebb self-assigned this Jan 5, 2021
@philwebb philwebb changed the title javax.annotation.Nullable cannot be used to indicate that a parameter to an endpoint operation is optional Support JSR305 @Nullable annotations on endpoint methods Jan 5, 2021
@philwebb philwebb changed the title Support JSR305 @Nullable annotations on endpoint methods Support JSR-305 @Nullable annotations on endpoint methods Jan 5, 2021
@philwebb philwebb changed the title Support JSR-305 @Nullable annotations on endpoint methods Support JSR-305 @Nullable annotations on endpoint methods Jan 5, 2021
@philwebb philwebb closed this in 2ad9a47 Jan 5, 2021
@philwebb philwebb modified the milestones: 2.3.x, 2.3.8 Jan 5, 2021
snicoll added a commit that referenced this issue Jan 6, 2021
snicoll added a commit that referenced this issue Jan 6, 2021
See gh-24647
@snicoll
Copy link
Member

@snicoll snicoll commented Jan 6, 2021

I've updated the reference guide to mention the support of JSR-305 in 2bd7835

@nathankooij
Copy link
Author

@nathankooij nathankooij commented Jan 6, 2021

Thanks for the quick fix!

@knoobie
Copy link

@knoobie knoobie commented Jan 6, 2021

@snicoll Am I missing something or did you mention the wrong annotation? Nonnull instead of Nullable - it means quite the opposite?

@snicoll
Copy link
Member

@snicoll snicoll commented Jan 6, 2021

@knoobie thanks for the nudge! This is me trying to be helpful without having my coffee. I'll polish to mention both @Nullable. I'll wait for @philwebb as there is something that uses Nonnull explicitly.

snicoll added a commit that referenced this issue Jan 6, 2021
See gh-24647
snicoll added a commit that referenced this issue Jan 6, 2021
See gh-24647
@philwebb philwebb reopened this Jan 6, 2021
@philwebb philwebb closed this in 848ed65 Jan 6, 2021
@philwebb
Copy link
Member

@philwebb philwebb commented Jan 6, 2021

I've added a test that shows @Nonnull works as expected. Thanks for the doc updates @snicoll

philwebb added a commit that referenced this issue Jan 6, 2021
Closes gh-24647
@philwebb philwebb changed the title Support JSR-305 @Nullable annotations on endpoint methods JSR-305 @Nullable cannot be used to indicate that a parameter to an endpoint operation is optional Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants