Clean up more warnings

Nike-Inc · Sep 13, 2024 · e42cd2c · e42cd2c
1 parent c5c8555
commit e42cd2c
Show file tree

Hide file tree

Showing 53 changed files with 249 additions and 140 deletions.
diff --git a/.../java/com/nike/backstopper/model/util/JsonUtilWithDefaultErrorContractDTOSupportTest.java b/.../java/com/nike/backstopper/model/util/JsonUtilWithDefaultErrorContractDTOSupportTest.java
@@ -157,18 +157,12 @@ public void writeValueAsString_serializes_non_Error_objects_like_a_default_Objec
         assertThat(resultString).isEqualTo(defaultSerialization);
     }
 
-    private static class NonErrorObject {
-        public final String someString;
-        public final Integer someInt;
-        public final Double someDouble;
-        public final Map<String, String> someMap;
-
-        private NonErrorObject(String someString, Integer someInt, Double someDouble, Map<String, String> someMap) {
-            this.someString = someString;
-            this.someInt = someInt;
-            this.someDouble = someDouble;
-            this.someMap = someMap;
-        }
+    private record NonErrorObject(
+        String someString,
+        Integer someInt,
+        Double someDouble,
+        Map<String, String> someMap
+    ) {
     }
 
     @DataProvider(value = {

diff --git a/...er/apierror/contract/jsr303convention/ReflectionBasedJsr303AnnotationTrollerBaseTest.java b/...er/apierror/contract/jsr303convention/ReflectionBasedJsr303AnnotationTrollerBaseTest.java
@@ -116,7 +116,7 @@ public void extractMessageFromAnnotation_throws_wrapped_RuntimeException_if_anno
 
     private static class TestClass {
 
-        public String fooField = "fooField";
+        public final String fooField = "fooField";
 
         public TestClass() {}
 

diff --git a/backstopper-spring-boot3-webmvc/build.gradle b/backstopper-spring-boot3-webmvc/build.gradle
@@ -19,6 +19,8 @@ dependencies {
             "com.tngtech.java:junit-dataprovider:$junitDataproviderVersion",
             "jakarta.servlet:jakarta.servlet-api:$servletApiVersion",
             "io.rest-assured:rest-assured:$restAssuredVersion",
+            // Pulling in commons-codec manually to avoid vulnerability warning coming from RestAssured transitive dep.
+            "commons-codec:commons-codec:$commonsCodecVersion",
             "jakarta.servlet:jakarta.servlet-api:$servletApiVersion",
             "org.springframework.boot:spring-boot-starter-web:$springboot3_3Version",
             "org.hibernate.validator:hibernate-validator:$hibernateValidatorVersion",

diff --git a/backstopper-spring-web-flux/build.gradle b/backstopper-spring-web-flux/build.gradle
@@ -20,6 +20,8 @@ dependencies {
             "org.assertj:assertj-core:$assertJVersion",
             "com.tngtech.java:junit-dataprovider:$junitDataproviderVersion",
             "io.rest-assured:rest-assured:$restAssuredVersion",
+            // Pulling in commons-codec manually to avoid vulnerability warning coming from RestAssured transitive dep.
+            "commons-codec:commons-codec:$commonsCodecVersion",
             "org.springframework.boot:spring-boot-starter-webflux:$springboot3_3Version",
             "jakarta.validation:jakarta.validation-api:$jakartaValidationVersion",
             "org.hibernate.validator:hibernate-validator:$hibernateValidatorVersion",

diff --git a/...c/main/java/com/nike/backstopper/handler/adapter/RequestInfoForLoggingWebFluxAdapter.java b/...c/main/java/com/nike/backstopper/handler/adapter/RequestInfoForLoggingWebFluxAdapter.java
@@ -3,6 +3,7 @@
 import com.nike.backstopper.handler.RequestInfoForLogging;
 
 import org.jetbrains.annotations.NotNull;
+import org.springframework.http.HttpMethod;
 import org.springframework.web.reactive.function.server.ServerRequest;
 
 import java.net.URI;
@@ -28,6 +29,7 @@ public RequestInfoForLoggingWebFluxAdapter(@NotNull ServerRequest request) {
         this.request = request;
         this.requestUri = request.uri();
 
+        //noinspection ConstantValue
         if (requestUri == null) {
             throw new NullPointerException("request.uri() cannot be null");
         }
@@ -40,7 +42,12 @@ public String getRequestUri() {
 
     @Override
     public String getRequestHttpMethod() {
-        return request.methodName();
+        HttpMethod method = request.method();
+        //noinspection ConstantValue
+        if (method == null) {
+            return null;
+        }
+        return method.name();
     }
 
     @Override
@@ -56,6 +63,7 @@ public Map<String, List<String>> getHeadersMap() {
     @Override
     public String getHeader(String headerName) {
         List<String> result = request.headers().header(headerName);
+        //noinspection ConstantValue
         if (result == null || result.isEmpty()) {
             return null;
         }

diff --git a/...in/java/com/nike/backstopper/handler/spring/webflux/SpringWebfluxApiExceptionHandler.java b/...in/java/com/nike/backstopper/handler/spring/webflux/SpringWebfluxApiExceptionHandler.java
@@ -114,7 +114,7 @@ protected Mono<ServerResponse> prepareFrameworkRepresentation(
     }
 
     @Override
-    public Mono<Void> handle(ServerWebExchange exchange, Throwable ex) {
+    public @NotNull Mono<Void> handle(@NotNull ServerWebExchange exchange, @NotNull Throwable ex) {
         ServerRequest fluxRequest = ServerRequest.create(exchange, messageReaders);
         RequestInfoForLogging requestInfoForLogging = new RequestInfoForLoggingWebFluxAdapter(fluxRequest);
 
@@ -125,9 +125,8 @@ public Mono<Void> handle(ServerWebExchange exchange, Throwable ex) {
             );
         }
         catch (UnexpectedMajorExceptionHandlingError ohNoException) {
-            logger.error(
-                "Unexpected major error while handling exception. "
-                + SpringWebfluxUnhandledExceptionHandler.class.getName() + " should handle it.", ohNoException
+            logger.error("Unexpected major error while handling exception. {} should handle it.",
+                         SpringWebfluxUnhandledExceptionHandler.class.getName(), ohNoException
             );
             return Mono.error(ex);
         }

diff --git a/...va/com/nike/backstopper/handler/spring/webflux/SpringWebfluxApiExceptionHandlerUtils.java b/...va/com/nike/backstopper/handler/spring/webflux/SpringWebfluxApiExceptionHandlerUtils.java
@@ -60,8 +60,7 @@ public Mono<ServerResponse> generateServerResponseForError(
                 getErrorResponseContentType(
                     errorContractDTO, httpStatusCode, rawFilteredApiErrors, originalException, request
                 )
-            )
-            .syncBody(serializeErrorContractToString(errorContractDTO));
+            ).bodyValue(serializeErrorContractToString(errorContractDTO));
     }
 
     protected String serializeErrorContractToString(DefaultErrorContractDTO errorContractDTO) {
@@ -76,7 +75,7 @@ protected MediaType getErrorResponseContentType(
         Throwable originalException,
         RequestInfoForLogging request
     ) {
-        // Default to simply application/json in UTF8.
-        return MediaType.APPLICATION_JSON_UTF8;
+        // Default to simply application/json.
+        return MediaType.APPLICATION_JSON;
     }
 }
diff --git a/...a/com/nike/backstopper/handler/spring/webflux/SpringWebfluxUnhandledExceptionHandler.java b/...a/com/nike/backstopper/handler/spring/webflux/SpringWebfluxUnhandledExceptionHandler.java
@@ -141,7 +141,7 @@ protected ErrorResponseInfo<Mono<ServerResponse>> generateLastDitchFallbackError
     }
 
     @Override
-    public Mono<Void> handle(ServerWebExchange exchange, Throwable ex) {
+    public @NotNull Mono<Void> handle(@NotNull ServerWebExchange exchange, @NotNull Throwable ex) {
         ServerRequest fluxRequest = ServerRequest.create(exchange, messageReaders);
         RequestInfoForLogging requestInfoForLogging = new RequestInfoForLoggingWebFluxAdapter(fluxRequest);
 

diff --git a/...st/java/com/nike/backstopper/handler/adapter/RequestInfoForLoggingWebFluxAdapterTest.java b/...st/java/com/nike/backstopper/handler/adapter/RequestInfoForLoggingWebFluxAdapterTest.java
@@ -10,6 +10,7 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpMethod;
 import org.springframework.web.reactive.function.server.ServerRequest;
 
 import java.net.URI;
@@ -34,10 +35,12 @@
  * @author Nic Munroe
  */
 @RunWith(DataProviderRunner.class)
+@SuppressWarnings("ClassEscapesDefinedScope")
 public class RequestInfoForLoggingWebFluxAdapterTest {
 
     private ServerRequest requestMock;
     private URI requestUri;
+    HttpMethod httpMethod;
     private String httpMethodName;
     private ServerRequest.Headers serverRequestHeadersMock;
     private HttpHeaders headers;
@@ -51,12 +54,14 @@ public void beforeMethod() {
         requestMock = mock(ServerRequest.class);
         requestUri = URI.create(String.format("http://localhost:8080%s?%s", REQUEST_PATH, QUERY_STRING));
         httpMethodName = UUID.randomUUID().toString();
+        httpMethod = mock(HttpMethod.class);
         headers = new HttpHeaders();
 
         serverRequestHeadersMock = mock(ServerRequest.Headers.class);
 
         doReturn(requestUri).when(requestMock).uri();
-        doReturn(httpMethodName).when(requestMock).methodName();
+        doReturn(httpMethod).when(requestMock).method();
+        doReturn(httpMethodName).when(httpMethod).name();
         doReturn(serverRequestHeadersMock).when(requestMock).headers();
 
         doReturn(headers).when(serverRequestHeadersMock).asHttpHeaders();
@@ -82,6 +87,7 @@ public void constructor_sets_fields_as_expected() {
     @Test
     public void constructor_throws_NullPointerException_if_passed_null_ServerRequest() {
         // when
+        @SuppressWarnings("DataFlowIssue")
         Throwable ex = catchThrowable(() -> new RequestInfoForLoggingWebFluxAdapter(null));
 
         // then
@@ -122,6 +128,18 @@ public void getRequestHttpMethod_returns_the_request_methodName() {
         assertThat(result).isEqualTo(httpMethodName);
     }
 
+    @Test
+    public void getRequestHttpMethod_returns_null_if_request_method_is_null() {
+        // given
+        doReturn(null).when(requestMock).method();
+
+        // when
+        String result = adapter.getRequestHttpMethod();
+
+        // then
+        assertThat(result).isNull();
+    }
+
     @Test
     public void getQueryString_returns_the_request_query_string() {
         // when

diff --git a/...ava/com/nike/backstopper/handler/spring/webflux/SpringWebfluxApiExceptionHandlerTest.java b/...ava/com/nike/backstopper/handler/spring/webflux/SpringWebfluxApiExceptionHandlerTest.java
@@ -80,14 +80,17 @@ public class SpringWebfluxApiExceptionHandlerTest {
     private List<ViewResolver> viewResolvers;
 
     private ServerWebExchange serverWebExchangeMock;
+    @SuppressWarnings("FieldCanBeLocal")
     private ServerHttpRequest serverHttpRequestMock;
     private ServerHttpResponse serverHttpResponseMock;
     private HttpHeaders serverHttpResponseHeadersMock;
+    @SuppressWarnings("FieldCanBeLocal")
     private URI uri;
 
     private Throwable exMock;
 
     @Before
+    @SuppressWarnings("unchecked")
     public void beforeMethod() {
         projectApiErrorsMock = mock(ProjectApiErrors.class);
         listenerList = new SpringWebFluxApiExceptionHandlerListenerList(
@@ -146,6 +149,7 @@ public void constructor_sets_fields_as_expected() {
     @Test
     public void constructor_throws_IllegalArgumentException_if_passed_null_ProjectApiErrors() {
         // when
+        @SuppressWarnings("DataFlowIssue")
         Throwable ex = catchThrowable(
             () -> new SpringWebfluxApiExceptionHandler(
                 null, listenerList, generalUtils, springUtilsMock, viewResolversProviderMock,
@@ -162,6 +166,7 @@ public void constructor_throws_IllegalArgumentException_if_passed_null_ProjectAp
     @Test
     public void constructor_throws_NullPointerException_if_passed_null_ApiExceptionHandlerListenerList() {
         // when
+        @SuppressWarnings("DataFlowIssue")
         Throwable ex = catchThrowable(
             () -> new SpringWebfluxApiExceptionHandler(
                 projectApiErrorsMock, null, generalUtils, springUtilsMock, viewResolversProviderMock,
@@ -196,6 +201,7 @@ public void constructor_throws_IllegalArgumentException_if_passed_ApiExceptionHa
     @Test
     public void constructor_throws_IllegalArgumentException_if_passed_null_ApiExceptionHandlerUtils() {
         // when
+        @SuppressWarnings("DataFlowIssue")
         Throwable ex = catchThrowable(
             () -> new SpringWebfluxApiExceptionHandler(
                 projectApiErrorsMock, listenerList, null, springUtilsMock, viewResolversProviderMock,
@@ -212,6 +218,7 @@ public void constructor_throws_IllegalArgumentException_if_passed_null_ApiExcept
     @Test
     public void constructor_throws_NullPointerException_if_passed_null_SpringWebfluxApiExceptionHandlerUtils() {
         // when
+        @SuppressWarnings("DataFlowIssue")
         Throwable ex = catchThrowable(
             () -> new SpringWebfluxApiExceptionHandler(
                 projectApiErrorsMock, listenerList, generalUtils, null, viewResolversProviderMock,
@@ -228,6 +235,7 @@ public void constructor_throws_NullPointerException_if_passed_null_SpringWebflux
     @Test
     public void constructor_throws_NullPointerException_if_passed_null_ViewResolver_ObjectProvider() {
         // when
+        @SuppressWarnings("DataFlowIssue")
         Throwable ex = catchThrowable(
             () -> new SpringWebfluxApiExceptionHandler(
                 projectApiErrorsMock, listenerList, generalUtils, springUtilsMock, null,
@@ -244,6 +252,7 @@ public void constructor_throws_NullPointerException_if_passed_null_ViewResolver_
     @Test
     public void constructor_throws_NullPointerException_if_passed_null_ServerCodecConfigurer() {
         // when
+        @SuppressWarnings("DataFlowIssue")
         Throwable ex = catchThrowable(
             () -> new SpringWebfluxApiExceptionHandler(
                 projectApiErrorsMock, listenerList, generalUtils, springUtilsMock, viewResolversProviderMock,
@@ -260,10 +269,12 @@ public void constructor_throws_NullPointerException_if_passed_null_ServerCodecCo
     @Test
     public void prepareFrameworkRepresentation_delegates_to_SpringWebfluxApiExceptionHandlerUtils() {
         // given
+        @SuppressWarnings("unchecked")
         Mono<ServerResponse> expectedResult = mock(Mono.class);
 
         DefaultErrorContractDTO errorContractDTOMock = mock(DefaultErrorContractDTO.class);
         int httpStatusCode = 400;
+        @SuppressWarnings("unchecked")
         Collection<ApiError> rawFilteredApiErrors = mock(Collection.class);
         Throwable originalException = mock(Throwable.class);
         RequestInfoForLogging request = mock(RequestInfoForLogging.class);
@@ -360,7 +371,7 @@ public void handle_returns_unhandled_Mono_if_maybeHandleException_returns_null()
     }
 
     private void verifyMonoIsErrorMono(Mono<?> mono, Throwable expectedCause) {
-        Throwable ex = catchThrowable(() -> mono.block());
+        Throwable ex = catchThrowable(mono::block);
 
         assertThat(ex).isNotNull();
 
@@ -394,8 +405,10 @@ public void processWebFluxResponse_works_as_expected() {
             .builder("foo", Arrays.asList("bar1", "bar2"))
             .put("blah", Collections.singletonList(UUID.randomUUID().toString()))
             .build();
+        @SuppressWarnings("unchecked")
+        Mono<ServerResponse> monoMock = mock(Mono.class);
         ErrorResponseInfo<Mono<ServerResponse>> errorResponseInfo = new ErrorResponseInfo<>(
-            400, mock(Mono.class), headersToAddToResponse
+            400, monoMock, headersToAddToResponse
         );
 
         // when

diff --git a/...om/nike/backstopper/handler/spring/webflux/SpringWebfluxApiExceptionHandlerUtilsTest.java b/...om/nike/backstopper/handler/spring/webflux/SpringWebfluxApiExceptionHandlerUtilsTest.java
@@ -24,6 +24,7 @@
 
 import reactor.core.publisher.Mono;
 
+import static java.util.Objects.requireNonNull;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
@@ -51,6 +52,7 @@ public void generateServerResponseForError_works_as_expected() {
         // given
         DefaultErrorContractDTO errorContractDtoMock = mock(DefaultErrorContractDTO.class);
         int statusCode = 400;
+        @SuppressWarnings("unchecked")
         Collection<ApiError> errors = mock(Collection.class);
         Throwable ex = mock(Throwable.class);
         RequestInfoForLogging requestMock = mock(RequestInfoForLogging.class);
@@ -72,7 +74,7 @@ public void generateServerResponseForError_works_as_expected() {
         verify(utilsSpy).getErrorResponseContentType(errorContractDtoMock, statusCode, errors, ex, requestMock);
         verify(utilsSpy).serializeErrorContractToString(errorContractDtoMock);
         ServerResponse result = resultMono.block();
-        assertThat(result.statusCode().value()).isEqualTo(statusCode);
+        assertThat(requireNonNull(result).statusCode().value()).isEqualTo(statusCode);
         assertThat(result.headers().getContentType()).isEqualTo(expectedResponseContentType);
         // Yes this is awful. But figuring out how to spit out the ServerResponse's body to something assertable
         //      in this test is also awful.
@@ -124,6 +126,7 @@ public void getErrorResponseContentType_returns_APPLICATION_JSON_UTF8_by_default
         // given
         DefaultErrorContractDTO errorContractDtoMock = mock(DefaultErrorContractDTO.class);
         int statusCode = 400;
+        @SuppressWarnings("unchecked")
         Collection<ApiError> errors = mock(Collection.class);
         Throwable ex = mock(Throwable.class);
         RequestInfoForLogging requestMock = mock(RequestInfoForLogging.class);
@@ -134,7 +137,7 @@ public void getErrorResponseContentType_returns_APPLICATION_JSON_UTF8_by_default
         );
 
         // then
-        assertThat(result).isEqualTo(MediaType.APPLICATION_JSON_UTF8);
+        assertThat(result).isEqualTo(MediaType.APPLICATION_JSON);
         verifyNoMoreInteractions(errorContractDtoMock, errors, ex, requestMock);
     }