From a442c180f1956fa0365bca4617fcedb8ea2199f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Thu, 26 Dec 2024 17:45:17 +0100 Subject: [PATCH] Refine null-safety in the spring-test module Closes gh-34161 --- .../jdbc/SqlScriptsTestExecutionListener.java | 2 +- .../context/junit/jupiter/SpringExtension.java | 2 +- ...tractDirtiesContextTestExecutionListener.java | 4 ++-- .../test/context/support/ContextLoaderUtils.java | 2 +- .../context/support/TestPropertySourceUtils.java | 2 +- .../TransactionalTestExecutionListener.java | 2 +- .../test/http/MediaTypeAssert.java | 1 - .../test/json/AbstractJsonContentAssert.java | 2 +- .../test/util/ReflectionTestUtils.java | 4 ++-- .../test/web/ModelAndViewAssert.java | 2 +- .../org/springframework/test/web/UriAssert.java | 16 +++++++++++++--- .../client/match/MockRestRequestMatchers.java | 4 ++-- .../web/reactive/server/WiretapConnector.java | 4 ++-- .../test/web/servlet/DefaultMvcResult.java | 5 ++--- .../test/web/servlet/MvcResult.java | 4 ++-- .../web/servlet/assertj/MvcTestResultAssert.java | 2 +- .../web/servlet/client/MockMvcHttpConnector.java | 4 ++-- .../test/web/servlet/StubMvcResult.java | 6 ++++-- .../http/InvalidMediaTypeException.java | 8 ++++++++ 19 files changed, 47 insertions(+), 29 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java index f3131669a75..5e741cec4a9 100644 --- a/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/jdbc/SqlScriptsTestExecutionListener.java @@ -410,7 +410,7 @@ public class SqlScriptsTestExecutionListener extends AbstractTestExecutionListen * Detect a default SQL script by implementing the algorithm defined in * {@link Sql#scripts}. */ - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation private String detectDefaultScript(Class testClass, @Nullable Method testMethod, boolean classLevel) { Assert.state(classLevel || testMethod != null, "Method-level @Sql requires a testMethod"); diff --git a/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java b/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java index af497e57ec7..7810d5616d2 100644 --- a/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java +++ b/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java @@ -373,7 +373,7 @@ public class SpringExtension implements BeforeAllCallback, AfterAllCallback, Tes * the supplied {@link TestContextManager}. * @since 6.1 */ - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // org.junit.jupiter.api.extension.ExecutableInvoker is not null marked private static void registerMethodInvoker(TestContextManager testContextManager, ExtensionContext context) { testContextManager.getTestContext().setMethodInvoker(context.getExecutableInvoker()::invoke); } diff --git a/spring-test/src/main/java/org/springframework/test/context/support/AbstractDirtiesContextTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/support/AbstractDirtiesContextTestExecutionListener.java index 11335315938..24407401e20 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/AbstractDirtiesContextTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/AbstractDirtiesContextTestExecutionListener.java @@ -84,7 +84,7 @@ public abstract class AbstractDirtiesContextTestExecutionListener extends Abstra * @since 4.2 * @see #dirtyContext */ - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation protected void beforeOrAfterTestMethod(TestContext testContext, MethodMode requiredMethodMode, ClassMode requiredClassMode) throws Exception { @@ -136,7 +136,7 @@ public abstract class AbstractDirtiesContextTestExecutionListener extends Abstra * @since 4.2 * @see #dirtyContext */ - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation protected void beforeOrAfterTestClass(TestContext testContext, ClassMode requiredClassMode) throws Exception { Assert.notNull(testContext, "TestContext must not be null"); Assert.notNull(requiredClassMode, "requiredClassMode must not be null"); diff --git a/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java b/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java index d768a8cfaf2..9988a98eaed 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/ContextLoaderUtils.java @@ -232,7 +232,7 @@ abstract class ContextLoaderUtils { * @throws IllegalArgumentException if the supplied class is {@code null} or if * {@code @ContextConfiguration} is not present on the supplied class */ - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation static List resolveContextConfigurationAttributes(Class testClass) { Assert.notNull(testClass, "Class must not be null"); diff --git a/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java b/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java index b8f03691386..bd1dbc96745 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java @@ -134,7 +134,7 @@ public abstract class TestPropertySourceUtils { return mergedAttributes; } - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation private static boolean duplicationDetected(TestPropertySourceAttributes currentAttributes, @Nullable TestPropertySourceAttributes previousAttributes) { diff --git a/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionalTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionalTestExecutionListener.java index c19eb72f667..26ca58dcc3b 100644 --- a/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionalTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/transaction/TransactionalTestExecutionListener.java @@ -194,7 +194,7 @@ public class TransactionalTestExecutionListener extends AbstractTestExecutionLis * @see #getTransactionManager(TestContext, String) */ @Override - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation public void beforeTestMethod(final TestContext testContext) throws Exception { Method testMethod = testContext.getTestMethod(); Class testClass = testContext.getTestClass(); diff --git a/spring-test/src/main/java/org/springframework/test/http/MediaTypeAssert.java b/spring-test/src/main/java/org/springframework/test/http/MediaTypeAssert.java index 277419daf7e..08f7c097290 100644 --- a/spring-test/src/main/java/org/springframework/test/http/MediaTypeAssert.java +++ b/spring-test/src/main/java/org/springframework/test/http/MediaTypeAssert.java @@ -103,7 +103,6 @@ public class MediaTypeAssert extends AbstractObjectAssert targetClass, @Nullable String name, @Nullable Object value, @Nullable Class type) { @@ -258,7 +258,7 @@ public abstract class ReflectionTestUtils { * @see ReflectionUtils#getField(Field, Object) * @see AopTestUtils#getUltimateTargetObject(Object) */ - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation public static @Nullable Object getField(@Nullable Object targetObject, @Nullable Class targetClass, String name) { Assert.isTrue(targetObject != null || targetClass != null, "Either targetObject or targetClass for the field must be specified"); diff --git a/spring-test/src/main/java/org/springframework/test/web/ModelAndViewAssert.java b/spring-test/src/main/java/org/springframework/test/web/ModelAndViewAssert.java index 0d87fdfe257..5e553b6f026 100644 --- a/spring-test/src/main/java/org/springframework/test/web/ModelAndViewAssert.java +++ b/spring-test/src/main/java/org/springframework/test/web/ModelAndViewAssert.java @@ -109,7 +109,7 @@ public abstract class ModelAndViewAssert { * @param mav the ModelAndView to test against (never {@code null}) * @param expectedModel the expected model */ - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation public static void assertModelAttributeValues(ModelAndView mav, Map expectedModel) { Map model = mav.getModel(); diff --git a/spring-test/src/main/java/org/springframework/test/web/UriAssert.java b/spring-test/src/main/java/org/springframework/test/web/UriAssert.java index 03e63dedfa3..1373fc3c9b9 100644 --- a/spring-test/src/main/java/org/springframework/test/web/UriAssert.java +++ b/spring-test/src/main/java/org/springframework/test/web/UriAssert.java @@ -80,22 +80,32 @@ public class UriAssert extends AbstractStringAssert { return this; } - @SuppressWarnings("NullAway") + private String buildUri(String uriTemplate, Object... uriVars) { try { return UriComponentsBuilder.fromUriString(uriTemplate) .buildAndExpand(uriVars).encode().toUriString(); } catch (Exception ex) { + String message = ex.getMessage(); throw Failures.instance().failure(this.info, - new ShouldBeValidUriTemplate(uriTemplate, ex.getMessage())); + message == null ? + new ShouldBeValidUriTemplate(uriTemplate) : + new ShouldBeValidUriTemplateWithMessage(uriTemplate, message)); } } private static final class ShouldBeValidUriTemplate extends BasicErrorMessageFactory { - private ShouldBeValidUriTemplate(String uriTemplate, String errorMessage) { + private ShouldBeValidUriTemplate(String uriTemplate) { + super("%nExpecting:%n %s%nTo be a valid URI template%n", uriTemplate); + } + } + + private static final class ShouldBeValidUriTemplateWithMessage extends BasicErrorMessageFactory { + + private ShouldBeValidUriTemplateWithMessage(String uriTemplate, String errorMessage) { super("%nExpecting:%n %s%nTo be a valid URI template but got:%n %s%n", uriTemplate, errorMessage); } } diff --git a/spring-test/src/main/java/org/springframework/test/web/client/match/MockRestRequestMatchers.java b/spring-test/src/main/java/org/springframework/test/web/client/match/MockRestRequestMatchers.java index e68e169aa7a..a7397f039aa 100644 --- a/spring-test/src/main/java/org/springframework/test/web/client/match/MockRestRequestMatchers.java +++ b/spring-test/src/main/java/org/springframework/test/web/client/match/MockRestRequestMatchers.java @@ -159,7 +159,7 @@ public abstract class MockRestRequestMatchers { * @see #queryParam(String, String...) */ @SafeVarargs - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation public static RequestMatcher queryParam(String name, Matcher... matchers) { return request -> { MultiValueMap params = getQueryParams(request); @@ -187,7 +187,7 @@ public abstract class MockRestRequestMatchers { * @see #queryParamList(String, Matcher) * @see #queryParam(String, Matcher...) */ - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation public static RequestMatcher queryParam(String name, String... expectedValues) { return request -> { MultiValueMap params = getQueryParams(request); diff --git a/spring-test/src/main/java/org/springframework/test/web/reactive/server/WiretapConnector.java b/spring-test/src/main/java/org/springframework/test/web/reactive/server/WiretapConnector.java index 4c13e93e773..20460df9d56 100644 --- a/spring-test/src/main/java/org/springframework/test/web/reactive/server/WiretapConnector.java +++ b/spring-test/src/main/java/org/springframework/test/web/reactive/server/WiretapConnector.java @@ -62,7 +62,7 @@ class WiretapConnector implements ClientHttpConnector { @Override - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation public Mono connect(HttpMethod method, URI uri, Function> requestCallback) { @@ -180,7 +180,7 @@ class WiretapConnector implements ClientHttpConnector { return this.publisherNested; } - @SuppressWarnings("NullAway") + @SuppressWarnings("NullAway") // Dataflow analysis limitation public Mono getContent() { return Mono.defer(() -> { if (this.content.scan(Scannable.Attr.TERMINATED) == Boolean.TRUE) { diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/DefaultMvcResult.java b/spring-test/src/main/java/org/springframework/test/web/servlet/DefaultMvcResult.java index 82a5aa8e019..937b2e203bc 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/DefaultMvcResult.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/DefaultMvcResult.java @@ -124,13 +124,12 @@ class DefaultMvcResult implements MvcResult { } @Override - public Object getAsyncResult() { + public @Nullable Object getAsyncResult() { return getAsyncResult(-1); } @Override - @SuppressWarnings("NullAway") - public Object getAsyncResult(long timeToWait) { + public @Nullable Object getAsyncResult(long timeToWait) { if (this.mockRequest.getAsyncContext() != null && timeToWait == -1) { long requestTimeout = this.mockRequest.getAsyncContext().getTimeout(); timeToWait = requestTimeout == -1 ? Long.MAX_VALUE : requestTimeout; diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/MvcResult.java b/spring-test/src/main/java/org/springframework/test/web/servlet/MvcResult.java index 9688bcd3213..00cdea90884 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/MvcResult.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/MvcResult.java @@ -85,7 +85,7 @@ public interface MvcResult { * {@link #getAsyncResult(long)} to specify the amount of time to wait. * @throws IllegalStateException if the async result was not set */ - Object getAsyncResult(); + @Nullable Object getAsyncResult(); /** * Get the result of async execution and wait if necessary. @@ -96,6 +96,6 @@ public interface MvcResult { * MockAsyncContext#setTimeout} for more details. * @throws IllegalStateException if the async result was not set */ - Object getAsyncResult(long timeToWait); + @Nullable Object getAsyncResult(long timeToWait); } diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MvcTestResultAssert.java b/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MvcTestResultAssert.java index 46c50fd4d70..18bf3a9aff8 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MvcTestResultAssert.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/assertj/MvcTestResultAssert.java @@ -212,7 +212,7 @@ public class MvcTestResultAssert extends AbstractMockHttpServletResponseAssert