Support type-use @Nullable annotation in annotated service #6457
Support type-use @Nullable annotation in annotated service #6457
@Nullable annotation in annotated service #6457Conversation
Signed-off-by: Seonghyeon Cho <seonghyeoncho96@gmail.com>
Signed-off-by: Seonghyeon Cho <seonghyeoncho96@gmail.com>
Signed-off-by: Seonghyeon Cho <seonghyeoncho96@gmail.com>
Signed-off-by: Seonghyeon Cho <seonghyeoncho96@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6457 +/- ##
============================================
- Coverage 74.46% 74.11% -0.35%
- Complexity 22234 23005 +771
============================================
Files 1963 2062 +99
Lines 82437 86144 +3707
Branches 10764 11314 +550
============================================
+ Hits 61385 63846 +2461
- Misses 15918 16883 +965
- Partials 5134 5415 +281 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| static boolean isAnnotatedNullable(AnnotatedElement annotatedElement) { | ||
| // 1) declaration annotation |
There was a problem hiding this comment.
Could you add examples so that readers can distinguish them easily? e.g.
@Nullable // declaration annotation
public String declarationAnnotatedMethod() {
return null;
}
// type-use annotation
public @Nullable String typeUseAnnotatedMethod() {
return null;
}
| } | ||
|
|
||
| @Get("/type_use_nullable") | ||
| public String typeUseNullable(@Header @org.jspecify.annotations.Nullable String value) { |
There was a problem hiding this comment.
Could you also add a test with this?
public @Nullable String typeUseAnnotatedMethod() {
return null;
}
There was a problem hiding this comment.
We may use @RequestObject to test it.
There was a problem hiding this comment.
Added test for nullable response
8d850d9 (#6457)
Also I tried to test with @RequestObject like below, but looks like @Nullable doesn't affect 👀 (empty body is rejected even if there's @Nullable)
@Post("/foo")
@ProducesJson
public Map<String, Object> foo(@RequestObject Map<String, Object> map) {
return map;
}
@Post("/bar")
@ProducesJson
public Map<String, Object> bar(@RequestObject @Nullable Map<String, Object> map) {
if (map == null) {
return Map.of("this is", "null");
}
return map;
}There was a problem hiding this comment.
empty body is rejected even if there's @nullable
I think it shouldn't throw an exception if it's annotated with the @Nullable.
What do you think of it? @line/dx
There was a problem hiding this comment.
I agree the behavior sounds reasonable - I'm not sure whether this is enforceable for all cases though. I have no objection if for Jackson's case where a literal null body is received, a null value is passed as the request object.
Having said this, I'm fine with going ahead with merging this PR and handling this separately as they seem to be separate issues
There was a problem hiding this comment.
I'm fine with going ahead with merging this PR and handling this separately as they seem to be separate issues
I agree. We don't have to deal with this in this PR. Let me merge this. 😉
Signed-off-by: Seonghyeon Cho <seonghyeoncho96@gmail.com>
Signed-off-by: Seonghyeon Cho <seonghyeoncho96@gmail.com>
Signed-off-by: Seonghyeon Cho <seonghyeoncho96@gmail.com>
Signed-off-by: Seonghyeon Cho <seonghyeoncho96@gmail.com>
|
Thanks a lot, @sh-cho! 😄 |
Motivation:
In annotated service, type-use
@Nullableannotation (ex. jspecify) is not respected.Modifications:
AnnotatedValueResolver#isAnnotatedNullable, add checks for type-use nullable annotationResult:
@NullableTYPE_USE annotation is not respected in annotated service #6454