Improve optional parameter checks for HTTP Interface argument

framework-docs/modules/ROOT/pages/integration/rest-clients.adoc

@@ -977,6 +977,9 @@ method parameters:
   `MultiValueMap<String, ?>` with multiple cookies, a `Collection<?>` of values, or an
   individual value. Type conversion is supported for non-String values.
 
+The parameters can't be null unless they are set as not required through the annotation,
+annotated with `@Nullable` or they are `Optional`.
+
 |===
 
 

framework-docs/modules/ROOT/pages/rsocket.adoc

@@ -1115,7 +1115,8 @@ method parameters:
 | `@Payload`
 | Set the input payload(s) for the request. This can be a concrete value, or any producer
   of values that can be adapted to a Reactive Streams `Publisher` via
-  `ReactiveAdapterRegistry`
+  `ReactiveAdapterRegistry`. The payload can't be null unless it's set as not required
+through the annotation, annotated with `@Nullable` or it is `Optional`.
 
 | `Object`, if followed by `MimeType`
 | The value for a metadata entry in the input payload. This can be any `Object` as long

spring-messaging/src/main/java/org/springframework/messaging/rsocket/service/PayloadArgumentResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@
  * annotated arguments.
  *
  * @author Rossen Stoyanchev
+ * @author Olga Maciaszek-Sharma
  * @since 6.0
  */
 public class PayloadArgumentResolver implements RSocketServiceArgumentResolver {
@@ -54,25 +55,28 @@ public boolean resolve(
 			return false;
 		}
 
-		if (argument != null) {
-			ReactiveAdapter reactiveAdapter = this.reactiveAdapterRegistry.getAdapter(parameter.getParameterType());
-			if (reactiveAdapter == null) {
-				requestValues.setPayloadValue(argument);
-			}
-			else {
-				MethodParameter nestedParameter = parameter.nested();
-
-				String message = "Async type for @Payload should produce value(s)";
-				Assert.isTrue(nestedParameter.getNestedParameterType() != Void.class, message);
-				Assert.isTrue(!reactiveAdapter.isNoValue(), message);
+		if (argument == null) {
+			boolean required = (annot == null || annot.required()) && !parameter.isOptional();
+			Assert.isTrue(!required, () -> "Missing payload");
+			return true;
+		}
 
-				requestValues.setPayload(
-						reactiveAdapter.toPublisher(argument),
-						ParameterizedTypeReference.forType(nestedParameter.getNestedGenericParameterType()));
-			}
+		ReactiveAdapter reactiveAdapter = this.reactiveAdapterRegistry
+				.getAdapter(parameter.getParameterType());
+		if (reactiveAdapter == null) {
+			requestValues.setPayloadValue(argument);
 		}
+		else {
+			MethodParameter nestedParameter = parameter.nested();
+
+			String message = "Async type for @Payload should produce value(s)";
+			Assert.isTrue(nestedParameter.getNestedParameterType() != Void.class, message);
+			Assert.isTrue(!reactiveAdapter.isNoValue(), message);
 
+			requestValues.setPayload(
+					reactiveAdapter.toPublisher(argument),
+					ParameterizedTypeReference.forType(nestedParameter.getNestedGenericParameterType()));
+		}
 		return true;
 	}
-
 }

spring-messaging/src/test/java/org/springframework/messaging/rsocket/service/PayloadArgumentResolverTests.java

@@ -25,6 +25,7 @@
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ParameterizedTypeReference;
 import org.springframework.core.ReactiveAdapterRegistry;
+import org.springframework.lang.Nullable;
 import org.springframework.messaging.handler.annotation.Payload;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -34,7 +35,9 @@
  * Tests for {@link PayloadArgumentResolver}.
  *
  * @author Rossen Stoyanchev
+ * @author Olga Maciaszek-Sharma
  */
+@SuppressWarnings("DataFlowIssue")
 class PayloadArgumentResolverTests extends RSocketServiceArgumentResolverTestSupport {
 
 	@Override
@@ -47,20 +50,15 @@ void stringPayload() {
 		String payload = "payloadValue";
 		boolean resolved = execute(payload, initMethodParameter(Service.class, "execute", 0));
 
-		assertThat(resolved).isTrue();
-		assertThat(getRequestValues().getPayloadValue()).isEqualTo(payload);
-		assertThat(getRequestValues().getPayload()).isNull();
+		assertPayload(resolved, payload);
 	}
 
 	@Test
 	void monoPayload() {
 		Mono<String> payloadMono = Mono.just("payloadValue");
 		boolean resolved = execute(payloadMono, initMethodParameter(Service.class, "executeMono", 0));
 
-		assertThat(resolved).isTrue();
-		assertThat(getRequestValues().getPayloadValue()).isNull();
-		assertThat(getRequestValues().getPayload()).isSameAs(payloadMono);
-		assertThat(getRequestValues().getPayloadElementType()).isEqualTo(new ParameterizedTypeReference<String>() {});
+		assertPayloadMono(resolved, payloadMono);
 	}
 
 	@Test
@@ -92,31 +90,77 @@ void completable() {
 	}
 
 	@Test
-	void notRequestBody() {
+	void notPayload() {
 		MethodParameter parameter = initMethodParameter(Service.class, "executeNotAnnotated", 0);
 		boolean resolved = execute("value", parameter);
 
 		assertThat(resolved).isFalse();
 	}
 
 	@Test
-	void ignoreNull() {
-		boolean resolved = execute(null, initMethodParameter(Service.class, "execute", 0));
+	void nullPayload() {
+		assertThatIllegalArgumentException()
+				.isThrownBy(() -> execute(null, initMethodParameter(Service.class, "execute", 0)))
+				.withMessage("Missing payload");
+
+		assertThatIllegalArgumentException()
+				.isThrownBy(() -> execute(null, initMethodParameter(Service.class, "executeMono", 0)))
+				.withMessage("Missing payload");
+	}
+
+	@Test
+	void nullPayloadWithNullable() {
+		boolean resolved = execute(null, initMethodParameter(Service.class, "executeNullable", 0));
+		assertNullValues(resolved);
+
+		boolean resolvedMono = execute(null, initMethodParameter(Service.class, "executeNullableMono", 0));
+		assertNullValues(resolvedMono);
+	}
+
+	@Test
+	void nullPayloadWithNotRequired() {
+		boolean resolved = execute(null, initMethodParameter(Service.class, "executeNotRequired", 0));
+		assertNullValues(resolved);
 
+		boolean resolvedMono = execute(null, initMethodParameter(Service.class, "executeNotRequiredMono", 0));
+		assertNullValues(resolvedMono);
+	}
+
+	private void assertPayload(boolean resolved, String payload) {
+		assertThat(resolved).isTrue();
+		assertThat(getRequestValues().getPayloadValue()).isEqualTo(payload);
+		assertThat(getRequestValues().getPayload()).isNull();
+	}
+
+	private void assertPayloadMono(boolean resolved, Mono<String> payloadMono) {
+		assertThat(resolved).isTrue();
+		assertThat(getRequestValues().getPayloadValue()).isNull();
+		assertThat(getRequestValues().getPayload()).isSameAs(payloadMono);
+		assertThat(getRequestValues().getPayloadElementType()).isEqualTo(new ParameterizedTypeReference<String>() { });
+	}
+
+	private void assertNullValues(boolean resolved) {
 		assertThat(resolved).isTrue();
 		assertThat(getRequestValues().getPayloadValue()).isNull();
 		assertThat(getRequestValues().getPayload()).isNull();
 		assertThat(getRequestValues().getPayloadElementType()).isNull();
 	}
 
-
-	@SuppressWarnings("unused")
+	@SuppressWarnings({"unused"})
 	private interface Service {
 
 		void execute(@Payload String body);
 
+		void executeNotRequired(@Payload(required = false) String body);
+
+		void executeNullable(@Nullable @Payload String body);
+
 		void executeMono(@Payload Mono<String> body);
 
+		void executeNullableMono(@Nullable @Payload Mono<String> body);
+
+		void executeNotRequiredMono(@Payload(required = false) Mono<String> body);
+
 		void executeSingle(@Payload Single<String> body);
 
 		void executeMonoVoid(@Payload Mono<Void> body);

spring-web/src/main/java/org/springframework/web/service/invoker/AbstractNamedValueArgumentResolver.java

@@ -39,6 +39,7 @@
  * request header, path variable, cookie, and others.
  *
  * @author Rossen Stoyanchev
+ * @author Olga Maciaszek-Sharma
  * @since 6.0
  */
 public abstract class AbstractNamedValueArgumentResolver implements HttpServiceArgumentResolver {
@@ -145,7 +146,7 @@ private NamedValueInfo updateNamedValueInfo(MethodParameter parameter, NamedValu
 							.formatted(parameter.getNestedParameterType().getName()));
 			}
 		}
-		boolean required = (info.required && !parameter.getParameterType().equals(Optional.class));
+		boolean required = (info.required && !parameter.isOptional());
 		String defaultValue = (ValueConstants.DEFAULT_NONE.equals(info.defaultValue) ? null : info.defaultValue);
 		return info.update(name, required, defaultValue);
 	}

spring-web/src/main/java/org/springframework/web/service/invoker/HttpMethodArgumentResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,8 @@
 
 package org.springframework.web.service.invoker;
 
+import java.util.Optional;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -41,17 +43,26 @@ public class HttpMethodArgumentResolver implements HttpServiceArgumentResolver {
 	public boolean resolve(
 			@Nullable Object argument, MethodParameter parameter, HttpRequestValues.Builder requestValues) {
 
-		if (!parameter.getParameterType().equals(HttpMethod.class)) {
+		parameter = parameter.nestedIfOptional();
+
+		if (!parameter.getNestedParameterType().equals(HttpMethod.class)) {
 			return false;
 		}
 
-		Assert.notNull(argument, "HttpMethod is required");
+		if (argument instanceof Optional<?> optionalValue) {
+			argument = optionalValue.orElse(null);
+		}
+
+		if (argument == null) {
+			Assert.isTrue(parameter.isOptional(), "HttpMethod is required");
+			return true;
+		}
+
 		HttpMethod httpMethod = (HttpMethod) argument;
 		requestValues.setHttpMethod(httpMethod);
 		if (logger.isTraceEnabled()) {
 			logger.trace("Resolved HTTP method to: " + httpMethod.name());
 		}
-
 		return true;
 	}
 

spring-web/src/main/java/org/springframework/web/service/invoker/RequestBodyArgumentResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,8 @@
 
 package org.springframework.web.service.invoker;
 
+import java.util.Optional;
+
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ParameterizedTypeReference;
 import org.springframework.core.ReactiveAdapter;
@@ -30,6 +32,7 @@
  * annotated arguments.
  *
  * @author Rossen Stoyanchev
+ * @author Olga Maciaszek-Sharma
  * @since 6.0
  */
 public class RequestBodyArgumentResolver implements HttpServiceArgumentResolver {
@@ -68,33 +71,39 @@ public boolean resolve(
 			return false;
 		}
 
-		if (argument != null) {
-			if (this.reactiveAdapterRegistry != null) {
-				ReactiveAdapter adapter = this.reactiveAdapterRegistry.getAdapter(parameter.getParameterType());
-				if (adapter != null) {
-					MethodParameter nestedParameter = parameter.nested();
-
-					String message = "Async type for @RequestBody should produce value(s)";
-					Assert.isTrue(!adapter.isNoValue(), message);
-					Assert.isTrue(nestedParameter.getNestedParameterType() != Void.class, message);
-
-					if (requestValues instanceof ReactiveHttpRequestValues.Builder reactiveRequestValues) {
-						reactiveRequestValues.setBodyPublisher(
-								adapter.toPublisher(argument), asParameterizedTypeRef(nestedParameter));
-					}
-					else {
-						throw new IllegalStateException(
-								"RequestBody with a reactive type is only supported with reactive client");
-					}
-
-					return true;
-				}
-			}
+		if (argument instanceof Optional<?> optionalValue) {
+			argument = optionalValue.orElse(null);
+		}
 
-			// Not a reactive type
-			requestValues.setBodyValue(argument);
+		if (argument == null) {
+			Assert.isTrue(!annot.required() || parameter.isOptional(), "RequestBody is required");
+			return true;
 		}
 
+		if (this.reactiveAdapterRegistry != null) {
+			ReactiveAdapter adapter = this.reactiveAdapterRegistry
+					.getAdapter(parameter.getParameterType());
+			if (adapter != null) {
+				MethodParameter nestedParameter = parameter.nested();
+
+				String message = "Async type for @RequestBody should produce value(s)";
+				Assert.isTrue(!adapter.isNoValue(), message);
+				Assert.isTrue(nestedParameter.getNestedParameterType() != Void.class, message);
+
+				if (requestValues instanceof ReactiveHttpRequestValues.Builder reactiveRequestValues) {
+					reactiveRequestValues.setBodyPublisher(
+							adapter.toPublisher(argument), asParameterizedTypeRef(nestedParameter));
+				}
+				else {
+					throw new IllegalStateException(
+							"RequestBody with a reactive type is only supported with reactive client");
+				}
+
+				return true;
+			}
+		}
+		// Not a reactive type
+		requestValues.setBodyValue(argument);
 		return true;
 	}
 

spring-web/src/main/java/org/springframework/web/service/invoker/UriBuilderFactoryArgumentResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,9 +17,11 @@
 package org.springframework.web.service.invoker;
 
 import java.net.URL;
+import java.util.Optional;
 
 import org.springframework.core.MethodParameter;
 import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 import org.springframework.web.util.UriBuilderFactory;
 import org.springframework.web.util.UriTemplate;
 
@@ -42,14 +44,22 @@ public class UriBuilderFactoryArgumentResolver implements HttpServiceArgumentRes
 	public boolean resolve(
 			@Nullable Object argument, MethodParameter parameter, HttpRequestValues.Builder requestValues) {
 
-		if (!parameter.getParameterType().equals(UriBuilderFactory.class)) {
+		parameter = parameter.nestedIfOptional();
+
+		if (!parameter.getNestedParameterType().equals(UriBuilderFactory.class)) {
 			return false;
 		}
 
-		if (argument != null) {
-			requestValues.setUriBuilderFactory((UriBuilderFactory) argument);
+		if (argument instanceof Optional<?> optionalValue) {
+			argument = optionalValue.orElse(null);
+		}
+
+		if (argument == null) {
+			Assert.isTrue(parameter.isOptional(), "UriBuilderFactory is required");
+			return true;
 		}
 
+		requestValues.setUriBuilderFactory((UriBuilderFactory) argument);
 		return true;
 	}
 }