From 0a50854e1af7b78ff1ae63f142cc426c492ce2d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Fri, 22 Mar 2024 16:09:18 +0100 Subject: [PATCH] Perform NullAway build-time checks in spring-webflux See gh-32475 --- gradle/spring-module.gradle | 3 ++- .../springframework/web/reactive/DispatcherHandler.java | 5 +++-- .../web/reactive/config/ResourceChainRegistration.java | 1 + .../function/client/DefaultClientResponseBuilder.java | 8 ++++---- .../web/reactive/resource/ResourceWebHandler.java | 2 ++ .../result/method/AbstractHandlerMethodMapping.java | 1 + .../reactive/result/method/InvocableHandlerMethod.java | 4 +++- .../annotation/AbstractMessageWriterResultHandler.java | 2 +- .../method/annotation/ControllerMethodResolver.java | 1 + .../method/annotation/ResponseEntityResultHandler.java | 2 +- .../socket/adapter/Netty5WebSocketSessionSupport.java | 5 ++++- .../socket/adapter/NettyWebSocketSessionSupport.java | 5 ++++- .../socket/adapter/StandardWebSocketHandlerAdapter.java | 1 + .../socket/server/support/HandshakeWebSocketService.java | 1 + 14 files changed, 29 insertions(+), 12 deletions(-) diff --git a/gradle/spring-module.gradle b/gradle/spring-module.gradle index 5f410410074..9970a25019e 100644 --- a/gradle/spring-module.gradle +++ b/gradle/spring-module.gradle @@ -117,7 +117,8 @@ tasks.withType(JavaCompile).configureEach { options.errorprone { disableAllChecks = true option("NullAway:CustomContractAnnotations", "org.springframework.lang.Contract") - option("NullAway:AnnotatedPackages", "org.springframework.core,org.springframework.expression") + option("NullAway:AnnotatedPackages", "org.springframework.core,org.springframework.expression," + + "org.springframework.web.reactive") option("NullAway:UnannotatedSubPackages", "org.springframework.instrument,org.springframework.context.index," + "org.springframework.asm,org.springframework.cglib,org.springframework.objenesis," + "org.springframework.javapoet,org.springframework.aot.nativex.substitution") diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java index c51a5f5026e..996c672c00d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java @@ -171,9 +171,10 @@ public class DispatcherHandler implements WebHandler, PreFlightRequestHandler, A } return resultMono.flatMap(result -> { Mono voidMono = handleResult(exchange, result, "Handler " + result.getHandler()); - if (result.getExceptionHandler() != null) { + DispatchExceptionHandler exceptionHandler = result.getExceptionHandler(); + if (exceptionHandler != null) { voidMono = voidMono.onErrorResume(ex -> - result.getExceptionHandler().handleError(exchange, ex).flatMap(result2 -> + exceptionHandler.handleError(exchange, ex).flatMap(result2 -> handleResult(exchange, result2, "Exception handler " + result2.getHandler() + ", error=\"" + ex.getMessage() + "\""))); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceChainRegistration.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceChainRegistration.java index fe31f196361..1102b974cde 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceChainRegistration.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/ResourceChainRegistration.java @@ -64,6 +64,7 @@ public class ResourceChainRegistration { this(cacheResources, cacheResources ? new ConcurrentMapCache(DEFAULT_CACHE_NAME) : null); } + @SuppressWarnings("NullAway") public ResourceChainRegistration(boolean cacheResources, @Nullable Cache cache) { Assert.isTrue(!cacheResources || cache != null, "'cache' is required when cacheResources=true"); if (cacheResources) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java index 29845f26f1c..24f41905d40 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java @@ -137,7 +137,7 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { return this; } - @SuppressWarnings("ConstantConditions") + @SuppressWarnings({"ConstantConditions", "NullAway"}) private HttpHeaders getHeaders() { if (this.headers == null) { this.headers = new HttpHeaders(this.originalResponse.headers().asHttpHeaders()); @@ -159,7 +159,7 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { return this; } - @SuppressWarnings("ConstantConditions") + @SuppressWarnings({"ConstantConditions", "NullAway"}) private MultiValueMap getCookies() { if (this.cookies == null) { this.cookies = new LinkedMultiValueMap<>(this.originalResponse.cookies()); @@ -256,13 +256,13 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { } @Override - @SuppressWarnings("ConstantConditions") + @SuppressWarnings({"ConstantConditions", "NullAway"}) public HttpHeaders getHeaders() { return (this.headers != null ? this.headers : this.originalResponse.headers().asHttpHeaders()); } @Override - @SuppressWarnings("ConstantConditions") + @SuppressWarnings({"ConstantConditions", "NullAway"}) public MultiValueMap getCookies() { return (this.cookies != null ? this.cookies : this.originalResponse.cookies()); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java index 8c6819517d7..3f4721a5b02 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java @@ -334,6 +334,7 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { * @param mediaTypes media type mappings * @since 5.3.2 */ + @SuppressWarnings("NullAway") public void setMediaTypes(Map mediaTypes) { if (this.mediaTypes == null) { this.mediaTypes = new HashMap<>(mediaTypes.size()); @@ -483,6 +484,7 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { }); } + @SuppressWarnings("NullAway") protected Mono getResource(ServerWebExchange exchange) { String rawPath = getResourcePath(exchange); String path = processPath(rawPath); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java index a42cbdc9536..4b454084b63 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java @@ -361,6 +361,7 @@ public abstract class AbstractHandlerMethodMapping extends AbstractHandlerMap } } + @SuppressWarnings("NullAway") private void addMatchingMappings(Collection mappings, List matches, ServerWebExchange exchange) { for (T mapping : mappings) { T match = getMatchingMapping(mapping, exchange); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java index f812d49d27e..89fd48c2f59 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java @@ -46,6 +46,7 @@ import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.http.HttpStatusCode; import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.lang.Contract; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; @@ -161,7 +162,7 @@ public class InvocableHandlerMethod extends HandlerMethod { * @param providedArgs optional list of argument values to match by type * @return a Mono with a {@link HandlerResult} */ - @SuppressWarnings("unchecked") + @SuppressWarnings({"unchecked", "NullAway"}) public Mono invoke( ServerWebExchange exchange, BindingContext bindingContext, Object... providedArgs) { @@ -261,6 +262,7 @@ public class InvocableHandlerMethod extends HandlerMethod { } } + @Contract("_, null -> false") private static boolean isAsyncVoidReturnType(MethodParameter returnType, @Nullable ReactiveAdapter adapter) { if (adapter != null && adapter.supportsEmpty()) { if (adapter.isNoValue()) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java index e5f0672eec8..f01fc44f325 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageWriterResultHandler.java @@ -165,7 +165,7 @@ public abstract class AbstractMessageWriterResultHandler extends HandlerResultHa * @return indicates completion or error * @since 5.0.2 */ - @SuppressWarnings({"rawtypes", "unchecked", "ConstantConditions"}) + @SuppressWarnings({"rawtypes", "unchecked", "ConstantConditions", "NullAway"}) protected Mono writeBody(@Nullable Object body, MethodParameter bodyParameter, @Nullable MethodParameter actualParam, ServerWebExchange exchange) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java index 18e380178df..94f745a74f3 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java @@ -375,6 +375,7 @@ class ControllerMethodResolver { * if {@code null}, check only {@code @ControllerAdvice} classes. */ @Nullable + @SuppressWarnings("NullAway") public InvocableHandlerMethod getExceptionHandlerMethod(Throwable ex, @Nullable HandlerMethod handlerMethod) { Class handlerType = (handlerMethod != null ? handlerMethod.getBeanType() : null); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandler.java index e5017ad344a..3d91c62a7de 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityResultHandler.java @@ -128,7 +128,7 @@ public class ResponseEntityResultHandler extends AbstractMessageWriterResultHand @Override - @SuppressWarnings("ConstantConditions") + @SuppressWarnings({"ConstantConditions", "NullAway"}) public Mono handleResult(ServerWebExchange exchange, HandlerResult result) { Mono returnValueMono; diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/Netty5WebSocketSessionSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/Netty5WebSocketSessionSupport.java index 8fcfff5eda6..3bfc7715baf 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/Netty5WebSocketSessionSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/Netty5WebSocketSessionSupport.java @@ -28,6 +28,7 @@ import io.netty5.handler.codec.http.websocketx.WebSocketFrame; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.Netty5DataBufferFactory; +import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; import org.springframework.web.reactive.socket.HandshakeInfo; import org.springframework.web.reactive.socket.WebSocketMessage; @@ -76,7 +77,9 @@ public abstract class Netty5WebSocketSessionSupport extends AbstractWebSocket protected WebSocketMessage toMessage(WebSocketFrame frame) { DataBuffer payload = bufferFactory().wrap(frame.binaryData()); - return new WebSocketMessage(messageTypes.get(frame.getClass()), payload, frame); + WebSocketMessage.Type messageType = messageTypes.get(frame.getClass()); + Assert.state(messageType != null, "Unexpected message type"); + return new WebSocketMessage(messageType, payload, frame); } protected WebSocketFrame toFrame(WebSocketMessage message) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/NettyWebSocketSessionSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/NettyWebSocketSessionSupport.java index d331d31dc3d..158ad985223 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/NettyWebSocketSessionSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/NettyWebSocketSessionSupport.java @@ -28,6 +28,7 @@ import io.netty.handler.codec.http.websocketx.WebSocketFrame; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.NettyDataBufferFactory; +import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; import org.springframework.web.reactive.socket.HandshakeInfo; import org.springframework.web.reactive.socket.WebSocketMessage; @@ -74,7 +75,9 @@ public abstract class NettyWebSocketSessionSupport extends AbstractWebSocketS protected WebSocketMessage toMessage(WebSocketFrame frame) { DataBuffer payload = bufferFactory().wrap(frame.content()); - return new WebSocketMessage(messageTypes.get(frame.getClass()), payload, frame); + WebSocketMessage.Type messageType = messageTypes.get(frame.getClass()); + Assert.state(messageType != null, "Unexpected message type"); + return new WebSocketMessage(messageType, payload, frame); } protected WebSocketFrame toFrame(WebSocketMessage message) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/StandardWebSocketHandlerAdapter.java b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/StandardWebSocketHandlerAdapter.java index 2f78f073e71..43ca7f12b3b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/StandardWebSocketHandlerAdapter.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/adapter/StandardWebSocketHandlerAdapter.java @@ -65,6 +65,7 @@ public class StandardWebSocketHandlerAdapter extends Endpoint { @Override + @SuppressWarnings("NullAway") public void onOpen(Session session, EndpointConfig config) { this.delegateSession = this.sessionFactory.apply(session); Assert.state(this.delegateSession != null, "No delegate session"); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java index 7392a8c1885..81b5326e817 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/socket/server/support/HandshakeWebSocketService.java @@ -244,6 +244,7 @@ public class HandshakeWebSocketService implements WebSocketService, Lifecycle { return null; } + @SuppressWarnings("NullAway") private Mono> initAttributes(ServerWebExchange exchange) { if (this.sessionAttributePredicate == null) { return EMPTY_ATTRIBUTES;