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.
@@ -16,6 +16,8 @@
 
 package org.springframework.messaging.rsocket.service;
 
+import java.util.Optional;
+
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ParameterizedTypeReference;
 import org.springframework.core.ReactiveAdapter;
@@ -29,6 +31,7 @@
  * annotated arguments.
  *
  * @author Rossen Stoyanchev
+ * @author Olga Maciaszek-Sharma
  * @since 6.0
  */
 public class PayloadArgumentResolver implements RSocketServiceArgumentResolver {
@@ -54,8 +57,15 @@ public boolean resolve(
 			return false;
 		}
 
+		parameter = parameter.nestedIfOptional();
+
+		if (argument instanceof Optional<?> optionalValue) {
+			argument = optionalValue.orElse(null);
+		}
+
 		if (argument != null) {
-			ReactiveAdapter reactiveAdapter = this.reactiveAdapterRegistry.getAdapter(parameter.getParameterType());
+			ReactiveAdapter reactiveAdapter = this.reactiveAdapterRegistry
+					.getAdapter(parameter.getNestedParameterType());
 			if (reactiveAdapter == null) {
 				requestValues.setPayloadValue(argument);
 			}
@@ -70,7 +80,10 @@ public boolean resolve(
 						reactiveAdapter.toPublisher(argument),
 						ParameterizedTypeReference.forType(nestedParameter.getNestedGenericParameterType()));
 			}
+			return true;
 		}
+		boolean required = (annot == null || annot.required()) && !parameter.isOptional();
+		Assert.isTrue(!required, () -> "Missing payload");
 
 		return true;
 	}

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

@@ -16,6 +16,8 @@
 
 package org.springframework.messaging.rsocket.service;
 
+import java.util.Optional;
+
 import io.reactivex.rxjava3.core.Completable;
 import io.reactivex.rxjava3.core.Single;
 import org.junit.jupiter.api.Test;
@@ -25,6 +27,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 +37,9 @@
  * Tests for {@link PayloadArgumentResolver}.
  *
  * @author Rossen Stoyanchev
+ * @author Olga Maciaszek-Sharma
  */
+@SuppressWarnings("DataFlowIssue")
 class PayloadArgumentResolverTests extends RSocketServiceArgumentResolverTestSupport {
 
 	@Override
@@ -47,20 +52,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 +92,116 @@ 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);
+	}
+
+	@Test
+	void nullPayloadWithOptional() {
+		boolean resolved = execute(null, initMethodParameter(Service.class, "executeOptional", 0));
+		assertNullValues(resolved);
+
+		boolean resolvedMono = execute(null, initMethodParameter(Service.class, "executeOptionalMono", 0));
+		assertNullValues(resolvedMono);
+	}
+
+	@Test
+	void emptyPayloadWithOptional() {
+		boolean resolved = execute(Optional.empty(), initMethodParameter(Service.class, "executeOptional", 0));
+		assertNullValues(resolved);
+
+		boolean resolvedMono = execute(Optional.empty(), initMethodParameter(Service.class, "executeOptionalMono", 0));
+		assertNullValues(resolvedMono);
+	}
+
+	@Test
+	void optionalStringPayload() {
+		String payload = "payloadValue";
+		boolean resolved = execute(Optional.of(payload), initMethodParameter(Service.class, "executeOptional", 0));
+
+		assertPayload(resolved, payload);
+	}
+
+	@Test
+	void optionalMonoPayload() {
+		Mono<String> payloadMono = Mono.just("payloadValue");
+		boolean resolved = execute(Optional.of(payloadMono), initMethodParameter(Service.class, "executeOptionalMono", 0));
+
+		assertPayloadMono(resolved, payloadMono);
+	}
+
+
+	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", "OptionalUsedAsFieldOrParameterType"})
 	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 executeOptional(@Payload Optional<String> body);
+
+		void executeNullableMono(@Nullable @Payload Mono<String> body);
+
+		void executeNotRequiredMono(@Payload(required = false) Mono<String> body);
+
+		void executeOptionalMono(@Payload Optional<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,25 @@ 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");
-		HttpMethod httpMethod = (HttpMethod) argument;
-		requestValues.setHttpMethod(httpMethod);
-		if (logger.isTraceEnabled()) {
-			logger.trace("Resolved HTTP method to: " + httpMethod.name());
+		if (argument instanceof Optional<?> optionalValue) {
+			argument = optionalValue.orElse(null);
 		}
 
+		if(argument != null) {
+			HttpMethod httpMethod = (HttpMethod) argument;
+			requestValues.setHttpMethod(httpMethod);
+			if (logger.isTraceEnabled()) {
+				logger.trace("Resolved HTTP method to: " + httpMethod.name());
+			}
+			return true;
+		}
+		Assert.isTrue(parameter.isOptional(), "HttpMethod is required");
 		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,9 +71,16 @@ public boolean resolve(
 			return false;
 		}
 
+		parameter = parameter.nestedIfOptional();
+
+		if (argument instanceof Optional<?> optionalValue) {
+			argument = optionalValue.orElse(null);
+		}
+
 		if (argument != null) {
 			if (this.reactiveAdapterRegistry != null) {
-				ReactiveAdapter adapter = this.reactiveAdapterRegistry.getAdapter(parameter.getParameterType());
+				ReactiveAdapter adapter = this.reactiveAdapterRegistry
+						.getAdapter(parameter.getNestedParameterType());
 				if (adapter != null) {
 					MethodParameter nestedParameter = parameter.nested();
 
@@ -93,7 +103,9 @@ public boolean resolve(
 
 			// Not a reactive type
 			requestValues.setBodyValue(argument);
+			return true;
 		}
+		Assert.isTrue(!annot.required() || parameter.isOptional(), "RequestBody is required");
 
 		return true;
 	}