From 7b730febfbd059513134da34afb0ca9c94bb3122 Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Tue, 7 Jan 2025 09:45:15 -0500 Subject: [PATCH] Cleanup stairway serialization tests and fix a few other warnings (#199) * cleanup stairway serialization tests and fix a few other warnings * more test cleanups * remove the quality plugin (aka pmd) as we are running sonar which is performing the same code checks --- build.gradle | 2 - gradle/config/checkstyle/suppressions.xml | 71 ------ gradle/config/pmd/pmd.xml | 202 ------------------ gradle/quality.gradle | 51 ----- .../common/db/DatabaseRetryUtilsTest.java | 53 ++--- .../iam/AuthenticatedUserRequestTest.java | 51 +---- .../bio/terra/common/iam/BearerTokenTest.java | 51 +---- .../bio/terra/common/iam/SamUserTest.java | 54 +---- .../bio/terra/common/logging/LoggingTest.java | 97 ++++----- .../transaction/TransactionRetryTest.java | 21 +- .../stairway/test/StairwayTestUtils.java | 20 ++ 11 files changed, 122 insertions(+), 551 deletions(-) delete mode 100644 gradle/config/checkstyle/suppressions.xml delete mode 100644 gradle/config/pmd/pmd.xml delete mode 100644 gradle/quality.gradle diff --git a/build.gradle b/build.gradle index 831c083b..af25b062 100644 --- a/build.gradle +++ b/build.gradle @@ -10,7 +10,6 @@ plugins { id 'org.sonarqube' version '5.1.0.4882' id 'io.spring.dependency-management' version '1.1.7' id 'org.springframework.boot' version '3.4.1' - id 'ru.vyarus.quality' version '5.0.0' id 'com.srcclr.gradle' version '3.1.12' } @@ -133,7 +132,6 @@ apply from: "$gradleIncDir/application.gradle" apply from: "$gradleIncDir/jacoco.gradle" apply from: "$gradleIncDir/javadoc.gradle" apply from: "$gradleIncDir/publishing.gradle" -apply from: "$gradleIncDir/quality.gradle" apply from: "$gradleIncDir/srcclr.gradle" apply from: "$gradleIncDir/sonarqube.gradle" apply from: "$gradleIncDir/spotless.gradle" diff --git a/gradle/config/checkstyle/suppressions.xml b/gradle/config/checkstyle/suppressions.xml deleted file mode 100644 index d7aa3496..00000000 --- a/gradle/config/checkstyle/suppressions.xml +++ /dev/null @@ -1,71 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/gradle/config/pmd/pmd.xml b/gradle/config/pmd/pmd.xml deleted file mode 100644 index a5268efb..00000000 --- a/gradle/config/pmd/pmd.xml +++ /dev/null @@ -1,202 +0,0 @@ - - - - - General Java quality rules. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/gradle/quality.gradle b/gradle/quality.gradle deleted file mode 100644 index 8214b91b..00000000 --- a/gradle/quality.gradle +++ /dev/null @@ -1,51 +0,0 @@ -/** - * Terra configs for the Gradle quality plugin. - * - * See docs at http://xvik.github.io/gradle-quality-plugin/4.5.0/ - * - * Guiding principles behind our use of static code analyzers: - * - * 1) Shift left. We value code analyzers because they can accelerate and - * automate our detection of clear-cut violations of Java style expectations - * and good coding practice. Automated enforcement should be scoped to - * non-controversial matters of code health. - * - * 2) Automated enforcement is for programming errors or mechanical code - * formatting. Style and good judgment can not be replaced by automatic - * checks. - * - * Good candidates for automated enforcement include code formatting, - * such as white space or camel case variable naming, or programmer errors - * such as unused or uninitialized variables. - * - * Bad candidates for automated enforcement include trying to determine when - * a method is too complex or coding conventions with many exceptions to any - * rule, such as class member ordering. - * - * For inspiration, see Error Prone's guidelines for new automated checks: - * https://github.com/google/error-prone/wiki/Criteria-for-new-checks - * - * 3) Suppress early, discuss later. We prefer fewer, more meaningful findings. - * At the first sign of a noisy or controversial finding, add a global - * suppression and follow-up with discussion among the team. We prefer global - * exclusions to many identical `@SuppressWarning` statements. - * - * Practical matters: - * - * For override files, see gradle/config/{toolName}/*.xml . These files were - * auto-generated with `./gradlew initQualityConfig`. - * - * All overrides should be tagged with a Terra-specific justification comment - * that explains why we are excluding the finding. Cross-links to tickets or - * discussion threads for context are encouraged. - * - * To suppress a violation in code, use `@SuppressWarning("ViolationName")`. - */ -quality { - // FYI-only mode for now. Switch to strict=true once all sub-tasks are green. - strict = false - - pmd = true - pmdIncremental = false - pmdVersion = '6.31.0' -} diff --git a/src/test/java/bio/terra/common/db/DatabaseRetryUtilsTest.java b/src/test/java/bio/terra/common/db/DatabaseRetryUtilsTest.java index d8422381..5bf3415f 100644 --- a/src/test/java/bio/terra/common/db/DatabaseRetryUtilsTest.java +++ b/src/test/java/bio/terra/common/db/DatabaseRetryUtilsTest.java @@ -1,77 +1,70 @@ package bio.terra.common.db; -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.time.Duration; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; -import org.springframework.dao.CannotSerializeTransactionException; +import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.dao.DataAccessException; +import org.springframework.dao.PessimisticLockingFailureException; /** Test for {@link DatabaseRetryUtils} */ @Tag("unit") -public class DatabaseRetryUtilsTest { - private static final CannotSerializeTransactionException RETRY_EXCEPTION = - new CannotSerializeTransactionException("test"); +@ExtendWith(MockitoExtension.class) +class DatabaseRetryUtilsTest { + private static final PessimisticLockingFailureException RETRY_EXCEPTION = + new PessimisticLockingFailureException("test"); + private static final Duration RETRY_SLEEP = Duration.ofMillis(1); - @SuppressWarnings("unchecked") - @Mock - DatabaseRetryUtils.DatabaseOperation mockDatabaseOperation = - mock(DatabaseRetryUtils.DatabaseOperation.class); + @Mock DatabaseRetryUtils.DatabaseOperation mockDatabaseOperation; @Test - public void succeedAfterRetry() throws Exception { + void succeedAfterRetry() throws InterruptedException { // Throw retryable exception 3 times then succeed. when(mockDatabaseOperation.execute()) .thenThrow(RETRY_EXCEPTION) .thenThrow(RETRY_EXCEPTION) .thenThrow(RETRY_EXCEPTION) .thenReturn(true); - assertTrue( - () -> { - try { - return DatabaseRetryUtils.executeAndRetry( - mockDatabaseOperation, Duration.ofMillis(1), 10); - } catch (InterruptedException e) { - fail("Should not throw exception"); - return false; - } - }); + assertTrue(DatabaseRetryUtils.executeAndRetry(mockDatabaseOperation, RETRY_SLEEP, 10)); verify(mockDatabaseOperation, times(4)).execute(); } @Test - public void exceedMaxRetry() throws Exception { + void exceedMaxRetry() { when(mockDatabaseOperation.execute()).thenThrow(RETRY_EXCEPTION); DataAccessException finalException = assertThrows( DataAccessException.class, - () -> - DatabaseRetryUtils.executeAndRetry(mockDatabaseOperation, Duration.ofMillis(1), 3)); + () -> DatabaseRetryUtils.executeAndRetry(mockDatabaseOperation, RETRY_SLEEP, 3)); verify(mockDatabaseOperation, times(3)).execute(); assertEquals(RETRY_EXCEPTION, finalException); } @Test - public void nonRetryableExecute() throws Exception { + void nonRetryableExecute() { when(mockDatabaseOperation.execute()).thenThrow(new RuntimeException("non-retryable")); RuntimeException finalException = assertThrows( RuntimeException.class, - () -> - DatabaseRetryUtils.executeAndRetry(mockDatabaseOperation, Duration.ofMillis(1), 3)); + () -> DatabaseRetryUtils.executeAndRetry(mockDatabaseOperation, RETRY_SLEEP, 3)); verify(mockDatabaseOperation, times(1)).execute(); assertEquals("non-retryable", finalException.getMessage()); } @Test - public void zeroMaxAttemptsInvalid() throws Exception { - when(mockDatabaseOperation.execute()).thenReturn(true); + void zeroMaxAttemptsInvalid() { assertThrows( IllegalArgumentException.class, - () -> DatabaseRetryUtils.executeAndRetry(mockDatabaseOperation, Duration.ofMillis(1), 0)); + () -> DatabaseRetryUtils.executeAndRetry(mockDatabaseOperation, RETRY_SLEEP, 0)); } } diff --git a/src/test/java/bio/terra/common/iam/AuthenticatedUserRequestTest.java b/src/test/java/bio/terra/common/iam/AuthenticatedUserRequestTest.java index 239cce87..639c6e1f 100644 --- a/src/test/java/bio/terra/common/iam/AuthenticatedUserRequestTest.java +++ b/src/test/java/bio/terra/common/iam/AuthenticatedUserRequestTest.java @@ -4,47 +4,24 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import bio.terra.common.stairway.test.StairwayTestUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.DeserializationFeature; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; -import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; -import com.fasterxml.jackson.module.paramnames.ParameterNamesModule; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; -import org.openapitools.jackson.nullable.JsonNullableModule; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @Tag("unit") -public class AuthenticatedUserRequestTest { +class AuthenticatedUserRequestTest { private static final Logger logger = LoggerFactory.getLogger(AuthenticatedUserRequestTest.class); - /** - * ObjectMapper testing configuration - * - * 
-   * Align ObjectMapper to Stairway ObjectMapper configuration in order to attempt to catch any
-   * serialization incompatibilities at dev time.
-   *
- */ - private static final ObjectMapper objectMapper = - new ObjectMapper() - .registerModule(new ParameterNamesModule()) - .registerModule(new Jdk8Module()) - .registerModule(new JavaTimeModule()) - .registerModule(new JsonNullableModule()) - .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) - .enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL); - private static final String EMAIL_ADDRESS = "test@example.com"; private static final String SUBJECT_ID = "Subject"; private static final String TOKEN = "0123.456-789AbCd"; @Test - public void builder() throws Exception { + void builder() throws Exception { AuthenticatedUserRequest.Builder builder = AuthenticatedUserRequest.builder(); // Build fails due to no Email @@ -68,7 +45,7 @@ public void builder() throws Exception { } @Test - public void equality() { + void equality() { AuthenticatedUserRequest req = AuthenticatedUserRequest.builder() .setEmail(EMAIL_ADDRESS) @@ -93,26 +70,18 @@ public void equality() { assertNotEquals(req, "test"); } - private static void validateJsonDeserialization(String json, AuthenticatedUserRequest request) - throws JsonProcessingException { - AuthenticatedUserRequest deserialized = - objectMapper.readValue(json, AuthenticatedUserRequest.class); - assertEquals(request, deserialized); - assertEquals(request.hashCode(), deserialized.hashCode()); - } - private static void validateJsonSerialization(AuthenticatedUserRequest request) throws JsonProcessingException { - String asString = objectMapper.writeValueAsString(request); + String asString = StairwayTestUtils.serializeToJson(request); logger.debug(String.format("Serialized AuthenticatedUserRequest: '%s'", asString)); - validateJsonDeserialization(asString, request); + StairwayTestUtils.validateJsonDeserialization(asString, request); } @Test - public void testVectors() throws JsonProcessingException { - - validateJsonDeserialization( - "[\"bio.terra.common.iam.AuthenticatedUserRequest\",{\"email\":\"test@example.com\",\"reqId\":\"78f84562-c442-49be-951a-a0a56230c35f\",\"subjectId\":\"Subject\",\"token\":\"0123.456-789AbCd\"}]", + void testVectors() throws JsonProcessingException { + StairwayTestUtils.validateJsonDeserialization( + """ + ["bio.terra.common.iam.AuthenticatedUserRequest",{"email":"test@example.com","reqId":"78f84562-c442-49be-951a-a0a56230c35f","subjectId":"Subject","token":"0123.456-789AbCd"}]""", AuthenticatedUserRequest.builder() .setEmail("test@example.com") .setSubjectId("Subject") diff --git a/src/test/java/bio/terra/common/iam/BearerTokenTest.java b/src/test/java/bio/terra/common/iam/BearerTokenTest.java index 2824d949..2fa8e152 100644 --- a/src/test/java/bio/terra/common/iam/BearerTokenTest.java +++ b/src/test/java/bio/terra/common/iam/BearerTokenTest.java @@ -2,45 +2,22 @@ import static org.junit.jupiter.api.Assertions.*; +import bio.terra.common.stairway.test.StairwayTestUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.DeserializationFeature; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; -import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; -import com.fasterxml.jackson.module.paramnames.ParameterNamesModule; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; -import org.openapitools.jackson.nullable.JsonNullableModule; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @Tag("unit") -public class BearerTokenTest { +class BearerTokenTest { private static final Logger logger = LoggerFactory.getLogger(BearerTokenTest.class); - /** - * ObjectMapper testing configuration - * - * 
-   * Align ObjectMapper to Stairway ObjectMapper configuration in order to attempt to catch any
-   * serialization incompatibilities at dev time.
-   *
- */ - private static final ObjectMapper objectMapper = - new ObjectMapper() - .registerModule(new ParameterNamesModule()) - .registerModule(new Jdk8Module()) - .registerModule(new JavaTimeModule()) - .registerModule(new JsonNullableModule()) - .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) - .enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL); - private static final BearerToken TOKEN = new BearerToken("0123.456-789AbCd"); @Test - public void equality() { + void equality() { // Positive test BearerToken cmp = new BearerToken(TOKEN.getToken()); assertEquals(TOKEN, cmp); @@ -53,27 +30,21 @@ public void equality() { assertEquals(TOKEN, TOKEN); // Explicit test for off-type comparison - assertNotEquals(TOKEN, "test"); - } - - private static void validateJsonDeserialization(String json, BearerToken request) - throws JsonProcessingException { - BearerToken deserialized = objectMapper.readValue(json, BearerToken.class); - assertEquals(request, deserialized); - assertEquals(request.hashCode(), deserialized.hashCode()); + assertNotEquals("test", TOKEN); } @Test void testJsonSerialization() throws JsonProcessingException { - String asString = objectMapper.writeValueAsString(TOKEN); + String asString = StairwayTestUtils.serializeToJson(TOKEN); logger.debug(String.format("Serialized TokenAuthenticatedRequest: '%s'", asString)); - validateJsonDeserialization(asString, TOKEN); + StairwayTestUtils.validateJsonDeserialization(asString, TOKEN); } @Test - public void testJsonDeserialization() throws JsonProcessingException { - - validateJsonDeserialization( - "[\"bio.terra.common.iam.BearerToken\",{\"token\":\"0123.456-789AbCd\"}]", TOKEN); + void testJsonDeserialization() throws JsonProcessingException { + StairwayTestUtils.validateJsonDeserialization( + """ + ["bio.terra.common.iam.BearerToken",{"token":"0123.456-789AbCd"}]""", + TOKEN); } } diff --git a/src/test/java/bio/terra/common/iam/SamUserTest.java b/src/test/java/bio/terra/common/iam/SamUserTest.java index 1c8f55b5..0abdaa0d 100644 --- a/src/test/java/bio/terra/common/iam/SamUserTest.java +++ b/src/test/java/bio/terra/common/iam/SamUserTest.java @@ -2,40 +2,13 @@ import static org.junit.jupiter.api.Assertions.*; +import bio.terra.common.stairway.test.StairwayTestUtils; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.DeserializationFeature; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; -import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; -import com.fasterxml.jackson.module.paramnames.ParameterNamesModule; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; -import org.openapitools.jackson.nullable.JsonNullableModule; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Tag("unit") -public class SamUserTest { - - private static final Logger logger = LoggerFactory.getLogger(SamUserTest.class); - - /** - * ObjectMapper testing configuration - * - * 
-   * Align ObjectMapper to Stairway ObjectMapper configuration in order to attempt to catch any
-   * serialization incompatibilities at dev time.
-   *
- */ - private static final ObjectMapper objectMapper = - new ObjectMapper() - .registerModule(new ParameterNamesModule()) - .registerModule(new Jdk8Module()) - .registerModule(new JavaTimeModule()) - .registerModule(new JsonNullableModule()) - .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) - .enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL); +class SamUserTest { private static final String EMAIL_ADDRESS = "test@example.com"; private static final String SUBJECT_ID = "Subject"; @@ -43,7 +16,7 @@ public class SamUserTest { private static final SamUser TEST_SAM_USER = new SamUser(EMAIL_ADDRESS, SUBJECT_ID, TOKEN); @Test - public void equality() { + void equality() { // Positive test SamUser cmp = new SamUser(EMAIL_ADDRESS, SUBJECT_ID, TOKEN); assertEquals(TEST_SAM_USER, cmp); @@ -61,24 +34,17 @@ public void equality() { assertNotEquals(TEST_SAM_USER, "test"); } - private static void validateJsonDeserialization(String json, SamUser request) - throws JsonProcessingException { - SamUser deserialized = objectMapper.readValue(json, SamUser.class); - assertEquals(request, deserialized); - assertEquals(request.hashCode(), deserialized.hashCode()); - } - @Test - public void validateJsonSerialization() throws JsonProcessingException { - String asString = objectMapper.writeValueAsString(TEST_SAM_USER); - logger.debug(String.format("Serialized SamUserAuthenticatedRequest: '%s'", asString)); - validateJsonDeserialization(asString, TEST_SAM_USER); + void validateJsonSerialization() throws JsonProcessingException { + String asString = StairwayTestUtils.serializeToJson(TEST_SAM_USER); + StairwayTestUtils.validateJsonDeserialization(asString, TEST_SAM_USER); } @Test - public void testJsonDeserialize() throws JsonProcessingException { - validateJsonDeserialization( - "[\"bio.terra.common.iam.SamUser\",{\"email\":\"test@example.com\",\"subjectId\":\"Subject\",\"bearerToken\":[\"bio.terra.common.iam.BearerToken\",{\"token\":\"0123.456-789AbCd\"}]}]", + void testJsonDeserialize() throws JsonProcessingException { + StairwayTestUtils.validateJsonDeserialization( + """ + ["bio.terra.common.iam.SamUser",{"email":"test@example.com","subjectId":"Subject","bearerToken":["bio.terra.common.iam.BearerToken",{"token":"0123.456-789AbCd"}]}]""", TEST_SAM_USER); } } diff --git a/src/test/java/bio/terra/common/logging/LoggingTest.java b/src/test/java/bio/terra/common/logging/LoggingTest.java index 7c693a17..82f84369 100644 --- a/src/test/java/bio/terra/common/logging/LoggingTest.java +++ b/src/test/java/bio/terra/common/logging/LoggingTest.java @@ -8,9 +8,6 @@ import com.jayway.jsonpath.Configuration; import com.jayway.jsonpath.JsonPath; import com.jayway.jsonpath.Option; -import jakarta.annotation.Nullable; -import jakarta.servlet.ServletException; -import java.io.IOException; import java.util.Arrays; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; @@ -31,14 +28,14 @@ * Tests the functionality of the common logging package, including request ID generation and * propagation, JSON formatting, and trace / span correlation. * - *

We use @SpringBootTest with a actual local servlet (WebEnvironment.RANDOM_PORT) to create as + *

We use @SpringBootTest with an actual local servlet (WebEnvironment.RANDOM_PORT) to create as * close of an approximation to a full Spring Boot application as possible. Still, some of the * initialization and auto-registration of Servlet filters had to be hard-coded within the * ContextConfiguration annotation. See the LoggingTestApplication for an example of what an actual * service needs to do in order to initialize logging. * - *

Inspired by - * https://github.com/eugenp/tutorials/blob/master/spring-boot-modules/spring-boot-testing/src/test/java/com/baeldung/testloglevel/LogbackMultiProfileTestLogLevelIntegrationTest.java + *

Inspired by */ @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, classes = LoggingTestApplication.class) @SpringJUnitConfig( @@ -51,14 +48,14 @@ @ActiveProfiles("logging-test") @ExtendWith(OutputCaptureExtension.class) @Tag("unit") -public class LoggingTest { +class LoggingTest { @Autowired private TestRestTemplate testRestTemplate; // Spy bean to allow us to mock out the RequestIdFilter ID generator. @MockitoSpyBean private RequestIdFilter requestIdFilter; @BeforeEach - public void setUp() throws IOException, ServletException { + public void setUp() { // Ensure the request ID is always set to a known value. when(requestIdFilter.generateRequestId()).thenReturn("12345"); // Ensure the GCP project ID is always set to a known value. See @@ -68,7 +65,7 @@ public void setUp() throws IOException, ServletException { } @Test - public void testRequestLogging(CapturedOutput capturedOutput) { + void testRequestLogging(CapturedOutput capturedOutput) { ResponseEntity response = testRestTemplate.getForEntity("/testRequestLogging", String.class); assertThat(response.getStatusCode().value()).isEqualTo(200); @@ -76,31 +73,32 @@ public void testRequestLogging(CapturedOutput capturedOutput) { String line = lastLoggedLine(capturedOutput); // Timestamp fields - assertThat((Integer) readJson(line, "$.timestampSeconds")).isNotNull(); - assertThat((Integer) readJson(line, "$.timestampNanos")).isNotNull(); + assertThat(readJson(line, "$.timestampSeconds", Integer.class)).isNotNull(); + assertThat(readJson(line, "$.timestampNanos", Integer.class)).isNotNull(); // Log message & source fields - assertThat((String) readJson(line, "$.severity")).isEqualTo("INFO"); - assertThat((String) readJson(line, "$.message")).isEqualTo("GET /testRequestLogging 200"); - assertThat((String) readJson(line, "$['logging.googleapis.com/sourceLocation'].file")) + assertThat((readJson(line, "$.severity", String.class))).isEqualTo("INFO"); + assertThat(readJson(line, "$.message", String.class)).isEqualTo("GET /testRequestLogging 200"); + assertThat(readJson(line, "$['logging.googleapis.com/sourceLocation'].file", String.class)) .isNotNull(); - assertThat((String) readJson(line, "$.serviceContext.service")).isEqualTo("loggingTest"); - assertThat((String) readJson(line, "$.serviceContext.version")).isEqualTo("1.2.3-SNAPSHOT"); + assertThat(readJson(line, "$.serviceContext.service", String.class)).isEqualTo("loggingTest"); + assertThat(readJson(line, "$.serviceContext.version", String.class)) + .isEqualTo("1.2.3-SNAPSHOT"); // Request-related fields - assertThat((String) readJson(line, "$.requestId")).isEqualTo("12345"); - assertThat((String) readJson(line, "$.httpRequest.requestMethod")).isEqualTo("GET"); - assertThat((String) readJson(line, "$.httpRequest.requestUrl")) + assertThat(readJson(line, "$.requestId", String.class)).isEqualTo("12345"); + assertThat(readJson(line, "$.httpRequest.requestMethod", String.class)).isEqualTo("GET"); + assertThat(readJson(line, "$.httpRequest.requestUrl", String.class)) .isEqualTo("/testRequestLogging"); - assertThat((Integer) readJson(line, "$.httpRequest.status")).isEqualTo(200); + assertThat(readJson(line, "$.httpRequest.status", Integer.class)).isEqualTo(200); // We also log all HTTP request headers. These aren't directly interpreted by Google, but are // available via jsonPayload.requestHeaders.* - assertThat((Object) readJson(line, "$.requestHeaders")).isNotNull(); + assertThat(readJson(line, "$.requestHeaders", Object.class)).isNotNull(); // Tracing-related fields - assertThat((String) readJson(line, "$.['logging.googleapis.com/trace']")) + assertThat(readJson(line, "$.['logging.googleapis.com/trace']", String.class)) .startsWith("projects/my-project-1234/traces/"); - assertThat((String) readJson(line, "$.['logging.googleapis.com/spanId']")).isNotBlank(); + assertThat(readJson(line, "$.['logging.googleapis.com/spanId']", String.class)).isNotBlank(); // The JSON format will also output the trace_sampled value if that value exists in the context // span. Due to the way we're creating a span from scratch, there wasn't an easy way to set it // to true within this test. @@ -111,7 +109,7 @@ public void testRequestLogging(CapturedOutput capturedOutput) { * for details on the invocation. */ @Test - public void testStructuredLogging(CapturedOutput capturedOutput) { + void testStructuredLogging(CapturedOutput capturedOutput) { ResponseEntity response = testRestTemplate.getForEntity("/testStructuredLogging", String.class); assertThat(response.getStatusCode().value()).isEqualTo(200); @@ -125,10 +123,10 @@ public void testStructuredLogging(CapturedOutput capturedOutput) { // The first log statement included a single key-value pair. Ensure that data is included in // the log output. assertThat(event1).isNotNull(); - assertThat((String) readJson(event1, "$.foo")).isEqualTo("bar"); + assertThat(readJson(event1, "$.foo", String.class)).isEqualTo("bar"); assertThat(event2).isNotNull(); - assertThat((Integer) readJson(event2, "$.a")).isEqualTo(1); - assertThat((Integer) readJson(event2, "$.b")).isEqualTo(2); + assertThat(readJson(event2, "$.a", Integer.class)).isEqualTo(1); + assertThat(readJson(event2, "$.b", Integer.class)).isEqualTo(2); assertThat(event3).isNotNull(); StructuredDataPojo pojo = readJson(event3, "$.pojo", StructuredDataPojo.class); @@ -137,23 +135,23 @@ public void testStructuredLogging(CapturedOutput capturedOutput) { assertThat(pojo.id).isEqualTo(1234); assertThat(event4).isNotNull(); - assertThat((String) readJson(event4, "$.foo.bar")).isEqualTo("baz"); + assertThat(readJson(event4, "$.foo.bar", String.class)).isEqualTo("baz"); } @Test - public void testAlertLogging(CapturedOutput capturedOutput) { + void testAlertLogging(CapturedOutput capturedOutput) { ResponseEntity response = testRestTemplate.getForEntity("/testAlertLogging", String.class); assertThat(response.getStatusCode().value()).isEqualTo(200); String[] lines = capturedOutput.getAll().split("\n"); String logLine = getLogContainingMessage(lines, "test alert message"); assertThat(logLine).isNotNull(); - assertThat((String) readJson(logLine, "$.severity")).isEqualTo("ERROR"); - assertTrue((Boolean) readJson(logLine, "$." + LoggingUtils.ALERT_KEY)); + assertThat(readJson(logLine, "$.severity", String.class)).isEqualTo("ERROR"); + assertTrue(readJson(logLine, "$." + LoggingUtils.ALERT_KEY, Boolean.class)); } @Test - public void testAlertLoggingWithObject(CapturedOutput capturedOutput) { + void testAlertLoggingWithObject(CapturedOutput capturedOutput) { ResponseEntity response = testRestTemplate.getForEntity("/testAlertLoggingWithObject", String.class); assertThat(response.getStatusCode().value()).isEqualTo(200); @@ -164,47 +162,26 @@ public void testAlertLoggingWithObject(CapturedOutput capturedOutput) { assertThat(pojo).isNotNull(); assertThat(logLine).isNotNull(); - assertThat((String) readJson(logLine, "$.severity")).isEqualTo("ERROR"); - assertTrue((Boolean) readJson(logLine, "$." + LoggingUtils.ALERT_KEY)); + assertThat(readJson(logLine, "$.severity", String.class)).isEqualTo("ERROR"); + assertTrue(readJson(logLine, "$." + LoggingUtils.ALERT_KEY, Boolean.class)); assertThat(pojo.name).isEqualTo("asdf"); assertThat(pojo.id).isEqualTo(1234); } - // Uses the JsonPath library to extract data from a given path within a JSON string. - @Nullable - private T readJson(String line, String path) { - return readJson(line, path, null); - } - /** * Uses the JsonPath library to extract data from a given path within a JSON string. * * @param line The line of text to extract from * @param path The JSON object path to read * @param clazz Class to return. If null, this is inferred by the JsonPath library. - * @return */ - @Nullable - private T readJson(String line, String path, @Nullable Class clazz) { - if (line.isEmpty()) { - // JsonPath does not allow empty strings to be parsed. - return null; - } + private T readJson(String line, String path, Class clazz) { // Suppress exceptions, otherwise JsonPath will throw an exception when we look for a path that // doesn't exist. It's better to assert a null return value in that case. - if (clazz != null) { - return (T) - JsonPath.using( - Configuration.defaultConfiguration().addOptions(Option.SUPPRESS_EXCEPTIONS)) - .parse(line) - .read(path, clazz); - } else { - return (T) - JsonPath.using( - Configuration.defaultConfiguration().addOptions(Option.SUPPRESS_EXCEPTIONS)) - .parse(line) - .read(path); - } + return JsonPath.using( + Configuration.defaultConfiguration().addOptions(Option.SUPPRESS_EXCEPTIONS)) + .parse(line) + .read(path, clazz); } private String lastLoggedLine(CapturedOutput capturedOutput) { diff --git a/src/test/java/bio/terra/common/retry/transaction/TransactionRetryTest.java b/src/test/java/bio/terra/common/retry/transaction/TransactionRetryTest.java index 7c5ee319..b9a67e86 100644 --- a/src/test/java/bio/terra/common/retry/transaction/TransactionRetryTest.java +++ b/src/test/java/bio/terra/common/retry/transaction/TransactionRetryTest.java @@ -7,14 +7,14 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; -import org.springframework.dao.CannotSerializeTransactionException; +import org.springframework.dao.PessimisticLockingFailureException; import org.springframework.test.context.ActiveProfiles; import org.springframework.transaction.CannotCreateTransactionException; @SpringBootTest(classes = TransactionRetryTestApplication.class) @ActiveProfiles("retry-transaction-test") @Tag("unit") -public class TransactionRetryTest { +class TransactionRetryTest { @Autowired private TransactionRetryProbe transactionRetryProbe; @@ -24,26 +24,27 @@ public void setup() { } @Test - public void testFastRetry() { + void testFastRetry() { + var exception = new PessimisticLockingFailureException("test"); assertThrows( - CannotSerializeTransactionException.class, - () -> transactionRetryProbe.throwMe(new CannotSerializeTransactionException("test"))); + PessimisticLockingFailureException.class, () -> transactionRetryProbe.throwMe(exception)); assertEquals(10, transactionRetryProbe.getCount()); } @Test - public void testSlowRetry() { + void testSlowRetry() { + var exception = new CannotCreateTransactionException("test"); assertThrows( - CannotCreateTransactionException.class, - () -> transactionRetryProbe.throwMe(new CannotCreateTransactionException("test"))); + CannotCreateTransactionException.class, () -> transactionRetryProbe.throwMe(exception)); assertEquals(4, transactionRetryProbe.getCount()); } @Test - public void testNoRetry() { - assertThrows(Exception.class, () -> transactionRetryProbe.throwMe(new Exception("test"))); + void testNoRetry() { + var exception = new Exception("test"); + assertThrows(Exception.class, () -> transactionRetryProbe.throwMe(exception)); assertEquals(1, transactionRetryProbe.getCount()); } diff --git a/src/test/java/bio/terra/common/stairway/test/StairwayTestUtils.java b/src/test/java/bio/terra/common/stairway/test/StairwayTestUtils.java index be60270c..77a89540 100644 --- a/src/test/java/bio/terra/common/stairway/test/StairwayTestUtils.java +++ b/src/test/java/bio/terra/common/stairway/test/StairwayTestUtils.java @@ -1,5 +1,7 @@ package bio.terra.common.stairway.test; +import static org.junit.jupiter.api.Assertions.assertEquals; + import bio.terra.common.db.BaseDatabaseProperties; import bio.terra.common.db.DataSourceInitializer; import bio.terra.stairway.Flight; @@ -7,8 +9,12 @@ import bio.terra.stairway.FlightState; import bio.terra.stairway.Stairway; import bio.terra.stairway.StairwayBuilder; +import bio.terra.stairway.StairwayMapper; import bio.terra.stairway.exception.DatabaseOperationException; import bio.terra.stairway.exception.StairwayException; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; import java.time.Duration; import java.time.Instant; import javax.sql.DataSource; @@ -16,6 +22,9 @@ /** Test utilities for testing integrations with {@link Stairway}. */ public class StairwayTestUtils { + /** Use Stairway objectmapper. */ + private static final ObjectMapper OBJECT_MAPPER = StairwayMapper.getObjectMapper(); + /** Returns an initialized and started Stairway instance from the Stairway.Builder. */ public static Stairway setupStairway(StairwayBuilder builder) { try { @@ -77,4 +86,15 @@ public static FlightState pollUntilComplete( throw new InterruptedException( String.format("Flight [%s] did not complete in the allowed wait time.", flightId)); } + + public static void validateJsonDeserialization(String json, T request) + throws JsonProcessingException { + T deserialized = OBJECT_MAPPER.readValue(json, new TypeReference<>() {}); + assertEquals(request, deserialized); + assertEquals(request.hashCode(), deserialized.hashCode()); + } + + public static String serializeToJson(Object object) throws JsonProcessingException { + return OBJECT_MAPPER.writeValueAsString(object); + } }