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
Labels
type: bug A general bug
Milestone

Comments

@nathankooij
Copy link

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.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 5, 2021
@philwebb philwebb added type: enhancement A general enhancement type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Jan 5, 2021
@philwebb philwebb added this to the 2.3.x milestone Jan 5, 2021
@philwebb
Copy link
Member

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 modified the milestones: 2.3.x, 2.3.8 Jan 5, 2021
snicoll added a commit that referenced this issue Jan 6, 2021
@snicoll
Copy link
Member

snicoll commented Jan 6, 2021

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

@nathankooij
Copy link
Author

Thanks for the quick fix!

@knoobie
Copy link

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 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.

@philwebb
Copy link
Member

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
@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 7, 2021
@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
@philwebb philwebb removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

6 participants