Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Reject null for non-optional arguments

Browse Source 
Closes gh-33339
This commit is contained in:
Olga Maciaszek-Sharma
2024-08-05 15:30:10 +02:00
committed by rstoyanchev
parent 4ac4c1b868
commit 51de84e148
14 changed files with 378 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
spring-web/src/test/java/org/springframework/web/service/invoker/HttpMethodArgumentResolverTests.java
View File

@@ -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 @@ class HttpMethodArgumentResolverTests {
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 @@ class HttpMethodArgumentResolverTests {
@GetExchange
void executeNotHttpMethod(String test);
@GetExchange
void executeNullableHttpMethod(@Nullable HttpMethod method);
@GetExchange
void executeOptionalHttpMethod(Optional<HttpMethod> method);
}
}

12
spring-web/src/test/java/org/springframework/web/service/invoker/NamedValueArgumentResolverTests.java
View File

@@ -48,6 +48,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException
* {@link TestValue @TestValue} annotation and {@link TestNamedValueArgumentResolver}.
*
* @author Rossen Stoyanchev
* @author Olga Maciaszek-Sharma
*/
class NamedValueArgumentResolverTests {
@@ -134,7 +135,7 @@ class NamedValueArgumentResolverTests {
}
@Test
void optionalEmpthyWithDefaultValue() {
void optionalEmptyWithDefaultValue() {
this.service.executeOptionalWithDefaultValue(Optional.empty());
assertTestValue("value", "default");
}
@@ -157,6 +158,12 @@ class NamedValueArgumentResolverTests {
assertTestValue("value", "test");
}
@Test
void nullTestValueWithNullable() {
this.service.executeNullable(null);
assertTestValue("value");
}
private void assertTestValue(String key, String... values) {
List<String> actualValues = this.argumentResolver.getTestValues().get(key);
if (ObjectUtils.isEmpty(values)) {
@@ -207,6 +214,9 @@ class NamedValueArgumentResolverTests {
@GetExchange
void executeMapWithOptionalValue(@TestValue Map<String, Optional<String>> values);
@GetExchange
void executeNullable(@Nullable @TestValue String value);
}

76
spring-web/src/test/java/org/springframework/web/service/invoker/RequestBodyArgumentResolverTests.java
View File

@@ -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 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
* 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 @@ class RequestBodyArgumentResolverTests {
}
@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.executeMono(null);
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();
}
@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 @@ class RequestBodyArgumentResolverTests {
}
@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<String> body);
@GetExchange
void executeMono(@RequestBody Mono<String> body);
@GetExchange
void executeNullableMono(@Nullable @RequestBody Mono<String> body);
@GetExchange
void executeNotRequiredMono(@RequestBody(required = false) Mono<String> body);
@GetExchange
void executeSingle(@RequestBody Single<String> body);

51
spring-web/src/test/java/org/springframework/web/service/invoker/UriBuilderFactoryArgumentResolverTests.java
View File

@@ -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.DefaultUriBuilderFactory;
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 @@ class UriBuilderFactoryArgumentResolverTests {
}
@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<UriBuilderFactory> uri);
}
}

50
spring-web/src/test/java/org/springframework/web/service/invoker/UrlArgumentResolverTests.java
View File

@@ -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.lang.Nullable;
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 @@ class UrlArgumentResolverTests {
}
@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> uri);
@GetExchange
void executeNotUri(String other);