-
Notifications
You must be signed in to change notification settings - Fork 0
fix nullability customizer #48
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
Conversation
ThoSap
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM 👍🏼
Let's only clarify these things first.
| public static class BaseClass { | ||
| @org.springframework.lang.Nullable | ||
| private String baseField; | ||
|
|
||
| public String getBaseField() { | ||
| return baseField; | ||
| } | ||
| } | ||
|
|
||
| public static class SubClass extends BaseClass { | ||
| private String subField; | ||
|
|
||
| public String getSubField() { | ||
| return subField; | ||
| } | ||
| } | ||
|
|
||
| public static class MethodAnnotated { | ||
| private String annotatedGetter; | ||
|
|
||
| @jakarta.annotation.Nullable | ||
| public String getAnnotatedGetter() { | ||
| return annotatedGetter; | ||
| } | ||
| } | ||
|
|
||
| public static class DirectMethodAnnotated { | ||
| private String directMethod; | ||
|
|
||
| @org.jspecify.annotations.Nullable | ||
| public String directMethod() { | ||
| return directMethod; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you have a mix of different @Nullable annotations.
|
|
||
| var schema = new Schema<Object>(); | ||
| // Simulating the concatenated FQN pattern described in the issue | ||
| var concatenatedFqn = SubClass.class.getName() + "Com.finstral.something"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like you have done it in PR commit https://github.com/aboutbits/spring-boot-toolbox/pull/47/changes#diff-ad71dc1ec29d6eb4c590a1e3e61049b7de72b369f976eabf6485d7eef1c210c6R121
| var concatenatedFqn = SubClass.class.getName() + "Com.finstral.something"; | |
| var concatenatedFqn = SubClass.class.getName() + "It.aboutbits.something"; |
| components.addSchemas(SubClass.class.getName(), subClassSchema); | ||
| openApi.setComponents(components); | ||
|
|
||
| assertDoesNotThrow(() -> customizer.customise(openApi)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we usually use AssertJ
assertThatCode(() -> call()).doesNotThrowAnyException();
instead of this JUnit one?
Same also for the ones below and assertTrue.
backport to support jspecify annotation. will be release
1.7.1