diff --git a/framework-docs/modules/ROOT/pages/integration/rest-clients.adoc b/framework-docs/modules/ROOT/pages/integration/rest-clients.adoc index ecfbd49f82a7..969d12c44e12 100644 --- a/framework-docs/modules/ROOT/pages/integration/rest-clients.adoc +++ b/framework-docs/modules/ROOT/pages/integration/rest-clients.adoc @@ -978,6 +978,9 @@ method parameters: `MultiValueMap` 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`. + |=== diff --git a/framework-docs/modules/ROOT/pages/rsocket.adoc b/framework-docs/modules/ROOT/pages/rsocket.adoc index 402c213898df..6a6654a314f6 100644 --- a/framework-docs/modules/ROOT/pages/rsocket.adoc +++ b/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 diff --git a/spring-messaging/src/main/java/org/springframework/messaging/rsocket/service/PayloadArgumentResolver.java b/spring-messaging/src/main/java/org/springframework/messaging/rsocket/service/PayloadArgumentResolver.java index 8511f675f25f..e8081efcedea 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/rsocket/service/PayloadArgumentResolver.java +++ b/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; } - } diff --git a/spring-messaging/src/test/java/org/springframework/messaging/rsocket/service/PayloadArgumentResolverTests.java b/spring-messaging/src/test/java/org/springframework/messaging/rsocket/service/PayloadArgumentResolverTests.java index d3ba5ba03113..f1f5ac9527d1 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/rsocket/service/PayloadArgumentResolverTests.java +++ b/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,9 +50,7 @@ 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 @@ -57,10 +58,7 @@ void monoPayload() { Mono 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() {}); + assertPayloadMono(resolved, payloadMono); } @Test @@ -92,7 +90,7 @@ void completable() { } @Test - void notRequestBody() { + void notPayload() { MethodParameter parameter = initMethodParameter(Service.class, "executeNotAnnotated", 0); boolean resolved = execute("value", parameter); @@ -100,23 +98,69 @@ void notRequestBody() { } @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 payloadMono) { + assertThat(resolved).isTrue(); + assertThat(getRequestValues().getPayloadValue()).isNull(); + assertThat(getRequestValues().getPayload()).isSameAs(payloadMono); + assertThat(getRequestValues().getPayloadElementType()).isEqualTo(new ParameterizedTypeReference() { }); + } + + 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 body); + void executeNullableMono(@Nullable @Payload Mono body); + + void executeNotRequiredMono(@Payload(required = false) Mono body); + void executeSingle(@Payload Single body); void executeMonoVoid(@Payload Mono body); diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/AbstractNamedValueArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/service/invoker/AbstractNamedValueArgumentResolver.java index de8ad19ce2a7..97288c2b5ea3 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/AbstractNamedValueArgumentResolver.java +++ b/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); } diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpMethodArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpMethodArgumentResolver.java index 9a6f966f08fe..4a8cdd161e85 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpMethodArgumentResolver.java +++ b/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; } diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/RequestBodyArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/service/invoker/RequestBodyArgumentResolver.java index cdd3c6f0c05f..e35edb9171b2 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/RequestBodyArgumentResolver.java +++ b/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; } diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/UriBuilderFactoryArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/service/invoker/UriBuilderFactoryArgumentResolver.java index d5d40f0bad78..0c89f905e593 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/UriBuilderFactoryArgumentResolver.java +++ b/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; } } diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/UrlArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/service/invoker/UrlArgumentResolver.java index 3ca43c85051e..e3840a55347c 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/UrlArgumentResolver.java +++ b/spring-web/src/main/java/org/springframework/web/service/invoker/UrlArgumentResolver.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. @@ -17,15 +17,18 @@ package org.springframework.web.service.invoker; import java.net.URI; +import java.util.Optional; import org.springframework.core.MethodParameter; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** * {@link HttpServiceArgumentResolver} that resolves the URL for the request * from a {@link URI} argument. * * @author Rossen Stoyanchev + * @author Olga Maciaszek-Sharma * @since 6.0 */ public class UrlArgumentResolver implements HttpServiceArgumentResolver { @@ -34,14 +37,22 @@ public class UrlArgumentResolver implements HttpServiceArgumentResolver { public boolean resolve( @Nullable Object argument, MethodParameter parameter, HttpRequestValues.Builder requestValues) { - if (!parameter.getParameterType().equals(URI.class)) { + parameter = parameter.nestedIfOptional(); + + if (!parameter.getNestedParameterType().equals(URI.class)) { return false; } - if (argument != null) { - requestValues.setUri((URI) argument); + if (argument instanceof Optional optionalValue) { + argument = optionalValue.orElse(null); + } + + if (argument == null) { + Assert.isTrue(parameter.isOptional(), "URI is required"); + return true; } + requestValues.setUri((URI) argument); return true; } diff --git a/spring-web/src/test/java/org/springframework/web/service/invoker/HttpMethodArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/service/invoker/HttpMethodArgumentResolverTests.java index d060b550af7b..e2f87f01cca4 100644 --- a/spring-web/src/test/java/org/springframework/web/service/invoker/HttpMethodArgumentResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/service/invoker/HttpMethodArgumentResolverTests.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.junit.jupiter.api.Test; import org.springframework.http.HttpMethod; @@ -68,12 +70,38 @@ void nullHttpMethod() { assertThatIllegalArgumentException().isThrownBy(() -> this.service.execute(null)); } + @Test + void nullHttpMethodWithNullable() { + this.service.executeNullableHttpMethod(null); + assertThat(getActualMethod()).isEqualTo(HttpMethod.GET); + } + + @Test + void nullHttpMethodWithOptional() { + this.service.executeOptionalHttpMethod(null); + assertThat(getActualMethod()).isEqualTo(HttpMethod.GET); + } + + @Test + void emptyOptionalHttpMethod() { + this.service.executeOptionalHttpMethod(Optional.empty()); + assertThat(getActualMethod()).isEqualTo(HttpMethod.GET); + } + + @Test + void optionalHttpMethod() { + this.service.executeOptionalHttpMethod(Optional.of(HttpMethod.POST)); + assertThat(getActualMethod()).isEqualTo(HttpMethod.POST); + } + + @Nullable private HttpMethod getActualMethod() { return this.client.getRequestValues().getHttpMethod(); } + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") private interface Service { @HttpExchange @@ -85,6 +113,12 @@ private interface Service { @GetExchange void executeNotHttpMethod(String test); + @GetExchange + void executeNullableHttpMethod(@Nullable HttpMethod method); + + @GetExchange + void executeOptionalHttpMethod(Optional method); + } } diff --git a/spring-web/src/test/java/org/springframework/web/service/invoker/NamedValueArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/service/invoker/NamedValueArgumentResolverTests.java index 399b5c3f4f5d..cbac555179ab 100644 --- a/spring-web/src/test/java/org/springframework/web/service/invoker/NamedValueArgumentResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/service/invoker/NamedValueArgumentResolverTests.java @@ -48,6 +48,7 @@ * {@link TestValue @TestValue} annotation and {@link TestNamedValueArgumentResolver}. * * @author Rossen Stoyanchev + * @author Olga Maciaszek-Sharma */ class NamedValueArgumentResolverTests { @@ -134,7 +135,7 @@ void optionalEmpty() { } @Test - void optionalEmpthyWithDefaultValue() { + void optionalEmptyWithDefaultValue() { this.service.executeOptionalWithDefaultValue(Optional.empty()); assertTestValue("value", "default"); } @@ -157,6 +158,12 @@ void mapOfTestValuesHasOptionalValue() { assertTestValue("value", "test"); } + @Test + void nullTestValueWithNullable() { + this.service.executeNullable(null); + assertTestValue("value"); + } + private void assertTestValue(String key, String... values) { List actualValues = this.argumentResolver.getTestValues().get(key); if (ObjectUtils.isEmpty(values)) { @@ -207,6 +214,9 @@ private interface Service { @GetExchange void executeMapWithOptionalValue(@TestValue Map> values); + @GetExchange + void executeNullable(@Nullable @TestValue String value); + } diff --git a/spring-web/src/test/java/org/springframework/web/service/invoker/RequestBodyArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/service/invoker/RequestBodyArgumentResolverTests.java index 12b8b81575d9..3b82d00164a1 100644 --- a/spring-web/src/test/java/org/springframework/web/service/invoker/RequestBodyArgumentResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/service/invoker/RequestBodyArgumentResolverTests.java @@ -16,6 +16,8 @@ package org.springframework.web.service.invoker; +import java.util.Optional; + import io.reactivex.rxjava3.core.Completable; import io.reactivex.rxjava3.core.Single; import org.junit.jupiter.api.Test; @@ -35,7 +37,9 @@ * Tests for {@link RequestBodyArgumentResolver}. * * @author Rossen Stoyanchev + * @author Olga Maciaszek-Sharma */ +@SuppressWarnings({"DataFlowIssue", "OptionalAssignedToNull"}) class RequestBodyArgumentResolverTests { private final TestReactorExchangeAdapter client = new TestReactorExchangeAdapter(); @@ -102,18 +106,68 @@ void notRequestBody() { } @Test - void ignoreNull() { - this.service.execute(null); + void nullRequestBody() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.service.execute(null)) + .withMessage("RequestBody is required"); + + assertThatIllegalArgumentException() + .isThrownBy(() -> this.service.executeMono(null)) + .withMessage("RequestBody is required"); + } + + @Test + void nullRequestBodyWithNullable() { + this.service.executeNullable(null); + + assertThat(getBodyValue()).isNull(); + assertThat(getPublisherBody()).isNull(); + + this.service.executeNullableMono(null); + + assertThat(getBodyValue()).isNull(); + assertThat(getPublisherBody()).isNull(); + } + + @Test + void nullRequestBodyWithNotRequired() { + this.service.executeNotRequired(null); + + assertThat(getBodyValue()).isNull(); + assertThat(getPublisherBody()).isNull(); + + this.service.executeNotRequiredMono(null); assertThat(getBodyValue()).isNull(); assertThat(getPublisherBody()).isNull(); + } - this.service.executeMono(null); + @Test + void nullRequestBodyWithOptional() { + this.service.executeOptional(null); assertThat(getBodyValue()).isNull(); assertThat(getPublisherBody()).isNull(); } + @Test + void emptyOptionalRequestBody() { + this.service.executeOptional(Optional.empty()); + + assertThat(getBodyValue()).isNull(); + assertThat(getPublisherBody()).isNull(); + } + + @Test + void optionalStringBody() { + String body = "bodyValue"; + this.service.executeOptional(Optional.of(body)); + + assertThat(getBodyValue()).isEqualTo(body); + assertThat(getPublisherBody()).isNull(); + } + + @Nullable private Object getBodyValue() { return getReactiveRequestValues().getBodyValue(); @@ -134,14 +188,30 @@ private ReactiveHttpRequestValues getReactiveRequestValues() { } + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") private interface Service { @GetExchange void execute(@RequestBody String body); + @GetExchange + void executeNullable(@Nullable @RequestBody String body); + + @GetExchange + void executeNotRequired(@RequestBody(required = false) String body); + + @GetExchange + void executeOptional(@RequestBody Optional body); + @GetExchange void executeMono(@RequestBody Mono body); + @GetExchange + void executeNullableMono(@Nullable @RequestBody Mono body); + + @GetExchange + void executeNotRequiredMono(@RequestBody(required = false) Mono body); + @GetExchange void executeSingle(@RequestBody Single body); diff --git a/spring-web/src/test/java/org/springframework/web/service/invoker/UriBuilderFactoryArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/service/invoker/UriBuilderFactoryArgumentResolverTests.java index 670d680929aa..36b8c9ec562c 100644 --- a/spring-web/src/test/java/org/springframework/web/service/invoker/UriBuilderFactoryArgumentResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/service/invoker/UriBuilderFactoryArgumentResolverTests.java @@ -16,6 +16,8 @@ package org.springframework.web.service.invoker; +import java.util.Optional; + import org.junit.jupiter.api.Test; import org.springframework.lang.Nullable; @@ -24,12 +26,14 @@ import org.springframework.web.util.UriBuilderFactory; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** * Tests for {@link UriBuilderFactoryArgumentResolver}. * * @author Olga Maciaszek-Sharma */ +@SuppressWarnings({"DataFlowIssue", "OptionalAssignedToNull"}) class UriBuilderFactoryArgumentResolverTests { private final TestExchangeAdapter client = new TestExchangeAdapter(); @@ -49,24 +53,65 @@ void uriBuilderFactory(){ } @Test - void ignoreNullUriBuilderFactory(){ - this.service.execute(null); + void nullUriBuilderFactory() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.service.execute(null)) + .withMessage("UriBuilderFactory is required"); + } + + @Test + void nullUriBuilderFactoryWithNullable(){ + this.service.executeNullable(null); assertThat(getRequestValues().getUriBuilderFactory()).isEqualTo(null); assertThat(getRequestValues().getUriTemplate()).isEqualTo("/path"); assertThat(getRequestValues().getUri()).isNull(); } + @Test + void nullUriBuilderFactoryWithOptional(){ + this.service.executeOptional(null); + + assertThat(getRequestValues().getUriBuilderFactory()).isEqualTo(null); + assertThat(getRequestValues().getUriTemplate()).isEqualTo("/path"); + assertThat(getRequestValues().getUri()).isNull(); + } + + @Test + void emptyOptionalUriBuilderFactory(){ + this.service.executeOptional(Optional.empty()); + + assertThat(getRequestValues().getUriBuilderFactory()).isEqualTo(null); + assertThat(getRequestValues().getUriTemplate()).isEqualTo("/path"); + assertThat(getRequestValues().getUri()).isNull(); + } + + @Test + void optionalUriBuilderFactory(){ + UriBuilderFactory factory = new DefaultUriBuilderFactory("https://example.com"); + this.service.executeOptional(Optional.of(factory)); + + assertThat(getRequestValues().getUriBuilderFactory()).isEqualTo(factory); + assertThat(getRequestValues().getUriTemplate()).isEqualTo("/path"); + assertThat(getRequestValues().getUri()).isNull(); + } private HttpRequestValues getRequestValues() { return this.client.getRequestValues(); } + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") private interface Service { @GetExchange("/path") - void execute(@Nullable UriBuilderFactory uri); + void execute(UriBuilderFactory uri); + + @GetExchange("/path") + void executeNullable(@Nullable UriBuilderFactory uri); + + @GetExchange("/path") + void executeOptional(Optional uri); } } diff --git a/spring-web/src/test/java/org/springframework/web/service/invoker/UrlArgumentResolverTests.java b/spring-web/src/test/java/org/springframework/web/service/invoker/UrlArgumentResolverTests.java index 668373ccf133..2039e851a48c 100644 --- a/spring-web/src/test/java/org/springframework/web/service/invoker/UrlArgumentResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/service/invoker/UrlArgumentResolverTests.java @@ -17,6 +17,7 @@ package org.springframework.web.service.invoker; import java.net.URI; +import java.util.Optional; import org.junit.jupiter.api.Test; @@ -24,13 +25,16 @@ import org.springframework.web.service.annotation.GetExchange; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** * Tests for {@link UrlArgumentResolver}. * * @author Rossen Stoyanchev + * @author Olga Maciaszek-Sharma */ +@SuppressWarnings({"DataFlowIssue", "OptionalAssignedToNull"}) class UrlArgumentResolverTests { private final TestExchangeAdapter client = new TestExchangeAdapter(); @@ -59,22 +63,62 @@ void notUrl() { } @Test - void ignoreNull() { - this.service.execute(null); + void nullUrl() { + assertThatIllegalArgumentException() + .isThrownBy(() -> this.service.execute(null)) + .withMessage("URI is required"); + } + + @Test + void nullUrlWithNullable() { + this.service.executeNullable(null); + + assertThat(getRequestValues().getUri()).isNull(); + assertThat(getRequestValues().getUriTemplate()).isEqualTo("/path"); + } + + @Test + void nullUrlWithOptional() { + this.service.executeOptional(null); assertThat(getRequestValues().getUri()).isNull(); assertThat(getRequestValues().getUriTemplate()).isEqualTo("/path"); } + @Test + void emptyOptionalUrl() { + this.service.executeOptional(Optional.empty()); + + assertThat(getRequestValues().getUri()).isNull(); + assertThat(getRequestValues().getUriTemplate()).isEqualTo("/path"); + } + + @Test + void optionalUrl() { + URI dynamicUrl = URI.create("dynamic-path"); + this.service.executeOptional(Optional.of(dynamicUrl)); + + assertThat(getRequestValues().getUri()).isEqualTo(dynamicUrl); + assertThat(getRequestValues().getUriTemplate()).isEqualTo("/path"); + } + + private HttpRequestValues getRequestValues() { return this.client.getRequestValues(); } + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") private interface Service { @GetExchange("/path") - void execute(@Nullable URI uri); + void execute(URI uri); + + @GetExchange("/path") + void executeNullable(@Nullable URI uri); + + @GetExchange("/path") + void executeOptional(Optional uri); @GetExchange void executeNotUri(String other);