From 5b1772ab1410ed3fc1524ae9b672d6915207036f Mon Sep 17 00:00:00 2001 From: David An Date: Tue, 19 Dec 2023 12:34:14 -0500 Subject: [PATCH 01/14] runtime code compatibility --- build.gradle | 2 +- service/build.gradle | 4 +++- .../InstanceInitializerBean.java | 2 +- .../generated/CapabilitiesApi.java | 6 +++--- .../generated/CapabilitiesServerModel.java | 6 +++--- .../generated/GenericJobAllOfServerModel.java | 6 +++--- .../generated/GenericJobServerModel.java | 6 +++--- .../workspacedataservice/generated/ImportApi.java | 6 +++--- .../generated/ImportRequestServerModel.java | 6 +++--- .../workspacedataservice/generated/JobApi.java | 6 +++--- .../generated/JobV1ServerModel.java | 6 +++--- .../leonardo/LeonardoServiceException.java | 2 +- .../retry/RetryLoggingListener.java | 4 ++-- .../workspacedataservice/sam/BearerTokenFilter.java | 12 ++++++------ .../workspacedataservice/sam/TokenContextUtil.java | 2 +- .../service/MDCResponseHeaderFilter.java | 12 ++++++------ .../sourcewds/WorkspaceDataServiceException.java | 2 +- .../workspacemanager/WorkspaceManagerException.java | 2 +- 18 files changed, 47 insertions(+), 45 deletions(-) diff --git a/build.gradle b/build.gradle index f3e3adbe0..4738919e2 100644 --- a/build.gradle +++ b/build.gradle @@ -1,7 +1,7 @@ import org.springframework.boot.gradle.plugin.SpringBootPlugin plugins { - id 'org.springframework.boot' version '2.7.18' apply false + id 'org.springframework.boot' version '3.2.0' apply false id 'io.spring.dependency-management' version '1.1.4' apply false id 'com.google.cloud.tools.jib' version '3.2.1' apply false id "org.sonarqube" version "4.4.1.3373" apply false diff --git a/service/build.gradle b/service/build.gradle index 5cd418e8f..cd1edb448 100644 --- a/service/build.gradle +++ b/service/build.gradle @@ -70,7 +70,7 @@ dependencies { implementation 'com.google.mug:mug:6.6' // required by openapi-generated models and api interfaces - implementation 'javax.validation:validation-api' + implementation 'jakarta.validation:jakarta.validation-api' implementation 'io.swagger.core.v3:swagger-annotations:2.2.16' // Terra libraries @@ -158,10 +158,12 @@ tasks.register('generateApiInterfaces', GenerateTask) { supportingFilesConstrainedTo.set([]) // empty array means generate none skipOperationExample = true // example responses require the ApiUtil.java supporting file configOptions.set([ + useJakartaEe : "true", // for Spring Boot 3 interfaceOnly : "true", // Java interfaces only; no controllers or implementation classes useTags : "true", // use the OpenAPI tag to classify apis, not the first segment of the api path hideGenerationTimestamp: "true" // hidden to prevent unnecessary diff noise on unrelated changes ]) + } diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/InstanceInitializerBean.java b/service/src/main/java/org/databiosphere/workspacedataservice/InstanceInitializerBean.java index dc602b332..8ce501091 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/InstanceInitializerBean.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/InstanceInitializerBean.java @@ -247,7 +247,7 @@ private String requestRemoteBackup(UUID trackingId) { } catch (WorkspaceDataServiceException wdsE) { if (wdsE.getCause() != null && wdsE.getCause() instanceof RestException restException - && restException.getStatus() == HttpStatus.NOT_FOUND) { + && restException.getStatusCode() == HttpStatus.NOT_FOUND) { LOGGER.error( "Remote source WDS in workspace {} does not support cloning", sourceWorkspaceId); cloneDao.terminateCloneToError( diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/generated/CapabilitiesApi.java b/service/src/main/java/org/databiosphere/workspacedataservice/generated/CapabilitiesApi.java index e043b371d..355a0fb62 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/generated/CapabilitiesApi.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/generated/CapabilitiesApi.java @@ -25,12 +25,12 @@ import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.multipart.MultipartFile; -import javax.validation.Valid; -import javax.validation.constraints.*; +import jakarta.validation.Valid; +import jakarta.validation.constraints.*; import java.util.List; import java.util.Map; import java.util.Optional; -import javax.annotation.Generated; +import jakarta.annotation.Generated; @Generated(value = "org.openapitools.codegen.languages.SpringCodegen") @Validated diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/generated/CapabilitiesServerModel.java b/service/src/main/java/org/databiosphere/workspacedataservice/generated/CapabilitiesServerModel.java index e538d76d9..0e9541818 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/generated/CapabilitiesServerModel.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/generated/CapabilitiesServerModel.java @@ -9,13 +9,13 @@ import java.util.Map; import org.openapitools.jackson.nullable.JsonNullable; import java.time.OffsetDateTime; -import javax.validation.Valid; -import javax.validation.constraints.*; +import jakarta.validation.Valid; +import jakarta.validation.constraints.*; import io.swagger.v3.oas.annotations.media.Schema; import java.util.*; -import javax.annotation.Generated; +import jakarta.annotation.Generated; /** * CapabilitiesServerModel diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/generated/GenericJobAllOfServerModel.java b/service/src/main/java/org/databiosphere/workspacedataservice/generated/GenericJobAllOfServerModel.java index 15ae9b345..91d45c8cf 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/generated/GenericJobAllOfServerModel.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/generated/GenericJobAllOfServerModel.java @@ -7,13 +7,13 @@ import com.fasterxml.jackson.annotation.JsonTypeName; import org.openapitools.jackson.nullable.JsonNullable; import java.time.OffsetDateTime; -import javax.validation.Valid; -import javax.validation.constraints.*; +import jakarta.validation.Valid; +import jakarta.validation.constraints.*; import io.swagger.v3.oas.annotations.media.Schema; import java.util.*; -import javax.annotation.Generated; +import jakarta.annotation.Generated; /** * GenericJobAllOfServerModel diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/generated/GenericJobServerModel.java b/service/src/main/java/org/databiosphere/workspacedataservice/generated/GenericJobServerModel.java index 3ba356ab9..c9920fed1 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/generated/GenericJobServerModel.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/generated/GenericJobServerModel.java @@ -11,13 +11,13 @@ import org.springframework.format.annotation.DateTimeFormat; import org.openapitools.jackson.nullable.JsonNullable; import java.time.OffsetDateTime; -import javax.validation.Valid; -import javax.validation.constraints.*; +import jakarta.validation.Valid; +import jakarta.validation.constraints.*; import io.swagger.v3.oas.annotations.media.Schema; import java.util.*; -import javax.annotation.Generated; +import jakarta.annotation.Generated; /** * Generic representation of a job, no opinion on inputs and result for the job. See individual APIs for more guidance on expected input and result payloads. diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/generated/ImportApi.java b/service/src/main/java/org/databiosphere/workspacedataservice/generated/ImportApi.java index 18c57b95d..edf1831cd 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/generated/ImportApi.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/generated/ImportApi.java @@ -27,12 +27,12 @@ import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.multipart.MultipartFile; -import javax.validation.Valid; -import javax.validation.constraints.*; +import jakarta.validation.Valid; +import jakarta.validation.constraints.*; import java.util.List; import java.util.Map; import java.util.Optional; -import javax.annotation.Generated; +import jakarta.annotation.Generated; @Generated(value = "org.openapitools.codegen.languages.SpringCodegen") @Validated diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/generated/ImportRequestServerModel.java b/service/src/main/java/org/databiosphere/workspacedataservice/generated/ImportRequestServerModel.java index 29d91c108..bc96efc44 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/generated/ImportRequestServerModel.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/generated/ImportRequestServerModel.java @@ -11,13 +11,13 @@ import java.util.Map; import org.openapitools.jackson.nullable.JsonNullable; import java.time.OffsetDateTime; -import javax.validation.Valid; -import javax.validation.constraints.*; +import jakarta.validation.Valid; +import jakarta.validation.constraints.*; import io.swagger.v3.oas.annotations.media.Schema; import java.util.*; -import javax.annotation.Generated; +import jakarta.annotation.Generated; /** * ImportRequestServerModel diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/generated/JobApi.java b/service/src/main/java/org/databiosphere/workspacedataservice/generated/JobApi.java index c5be8385d..316e91ffd 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/generated/JobApi.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/generated/JobApi.java @@ -26,12 +26,12 @@ import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.multipart.MultipartFile; -import javax.validation.Valid; -import javax.validation.constraints.*; +import jakarta.validation.Valid; +import jakarta.validation.constraints.*; import java.util.List; import java.util.Map; import java.util.Optional; -import javax.annotation.Generated; +import jakarta.annotation.Generated; @Generated(value = "org.openapitools.codegen.languages.SpringCodegen") @Validated diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/generated/JobV1ServerModel.java b/service/src/main/java/org/databiosphere/workspacedataservice/generated/JobV1ServerModel.java index 7b9dc4158..e4fe75a53 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/generated/JobV1ServerModel.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/generated/JobV1ServerModel.java @@ -11,13 +11,13 @@ import org.springframework.format.annotation.DateTimeFormat; import org.openapitools.jackson.nullable.JsonNullable; import java.time.OffsetDateTime; -import javax.validation.Valid; -import javax.validation.constraints.*; +import jakarta.validation.Valid; +import jakarta.validation.constraints.*; import io.swagger.v3.oas.annotations.media.Schema; import java.util.*; -import javax.annotation.Generated; +import jakarta.annotation.Generated; /** * JobV1ServerModel diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/leonardo/LeonardoServiceException.java b/service/src/main/java/org/databiosphere/workspacedataservice/leonardo/LeonardoServiceException.java index d4382716a..d566fb0e0 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/leonardo/LeonardoServiceException.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/leonardo/LeonardoServiceException.java @@ -5,6 +5,6 @@ public class LeonardoServiceException extends ResponseStatusException { public LeonardoServiceException(RestException cause) { - super(cause.getStatus(), cause.getMessage(), cause); + super(cause.getStatusCode(), cause.getMessage(), cause); } } diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/retry/RetryLoggingListener.java b/service/src/main/java/org/databiosphere/workspacedataservice/retry/RetryLoggingListener.java index b91e3f0bf..a90c15149 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/retry/RetryLoggingListener.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/retry/RetryLoggingListener.java @@ -4,11 +4,11 @@ import org.slf4j.LoggerFactory; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; -import org.springframework.retry.listener.RetryListenerSupport; +import org.springframework.retry.RetryListener; import org.springframework.stereotype.Component; @Component("retryLoggingListener") -public class RetryLoggingListener extends RetryListenerSupport { +public class RetryLoggingListener implements RetryListener { private final Logger logger = LoggerFactory.getLogger(getClass()); diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/sam/BearerTokenFilter.java b/service/src/main/java/org/databiosphere/workspacedataservice/sam/BearerTokenFilter.java index 0b80e0647..4648fc701 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/sam/BearerTokenFilter.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/sam/BearerTokenFilter.java @@ -2,14 +2,14 @@ import static org.springframework.web.context.request.RequestAttributes.SCOPE_REQUEST; +import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; import java.io.IOException; import java.util.Objects; -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.HttpHeaders; diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/sam/TokenContextUtil.java b/service/src/main/java/org/databiosphere/workspacedataservice/sam/TokenContextUtil.java index c5eb7eab8..47b13aa78 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/sam/TokenContextUtil.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/sam/TokenContextUtil.java @@ -3,9 +3,9 @@ import static org.databiosphere.workspacedataservice.sam.BearerTokenFilter.ATTRIBUTE_NAME_TOKEN; import static org.springframework.web.context.request.RequestAttributes.SCOPE_REQUEST; +import jakarta.validation.constraints.NotNull; import java.util.Map; import java.util.function.Supplier; -import javax.validation.constraints.NotNull; import org.databiosphere.workspacedataservice.jobexec.JobContextHolder; import org.databiosphere.workspacedataservice.shared.model.BearerToken; import org.slf4j.Logger; diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/service/MDCResponseHeaderFilter.java b/service/src/main/java/org/databiosphere/workspacedataservice/service/MDCResponseHeaderFilter.java index 5e9a64238..d849a01b0 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/service/MDCResponseHeaderFilter.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/service/MDCResponseHeaderFilter.java @@ -1,12 +1,12 @@ package org.databiosphere.workspacedataservice.service; +import jakarta.servlet.Filter; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletResponse; import org.slf4j.MDC; import org.springframework.stereotype.Component; diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/sourcewds/WorkspaceDataServiceException.java b/service/src/main/java/org/databiosphere/workspacedataservice/sourcewds/WorkspaceDataServiceException.java index 80fbd0497..e5ff29e47 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/sourcewds/WorkspaceDataServiceException.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/sourcewds/WorkspaceDataServiceException.java @@ -16,6 +16,6 @@ public WorkspaceDataServiceException(ApiException cause) { } public WorkspaceDataServiceException(RestException cause) { - super(cause.getStatus(), cause.getMessage(), cause); + super(cause.getStatusCode(), cause.getMessage(), cause); } } diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/workspacemanager/WorkspaceManagerException.java b/service/src/main/java/org/databiosphere/workspacedataservice/workspacemanager/WorkspaceManagerException.java index 563516124..29d7379b7 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/workspacemanager/WorkspaceManagerException.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/workspacemanager/WorkspaceManagerException.java @@ -5,6 +5,6 @@ public class WorkspaceManagerException extends ResponseStatusException { public WorkspaceManagerException(RestException cause) { - super(cause.getStatus(), cause.getMessage(), cause); + super(cause.getStatusCode(), cause.getMessage(), cause); } } From 1ece9eb8191d90e971466dbc0cc53e4751d5d390 Mon Sep 17 00:00:00 2001 From: David An Date: Tue, 19 Dec 2023 12:39:09 -0500 Subject: [PATCH 02/14] test compatibility --- .../workspacedataservice/datarepo/DataRepoDaoTest.java | 2 +- .../workspacedataservice/leonardo/LeonardoDaoTest.java | 2 +- .../databiosphere/workspacedataservice/pact/WsmPactTest.java | 4 ++-- .../workspacedataservice/retry/RestClientRetryTest.java | 4 ++-- .../service/InstanceServiceSamExceptionTest.java | 4 ++-- .../service/RecordOrchestratorServiceTest.java | 2 +- .../workspacemanager/WorkspaceManagerDaoTest.java | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/datarepo/DataRepoDaoTest.java b/service/src/test/java/org/databiosphere/workspacedataservice/datarepo/DataRepoDaoTest.java index d6393217e..aef23f4cf 100644 --- a/service/src/test/java/org/databiosphere/workspacedataservice/datarepo/DataRepoDaoTest.java +++ b/service/src/test/java/org/databiosphere/workspacedataservice/datarepo/DataRepoDaoTest.java @@ -68,7 +68,7 @@ void testErrorThrown() throws ApiException { .willThrow(new ApiException(statusCode, "Intentional error thrown for unit test")); var exception = assertThrows(DataRepoException.class, () -> dataRepoDao.getSnapshot(UUID.randomUUID())); - assertEquals(statusCode, exception.getRawStatusCode()); + assertEquals(statusCode, exception.getStatusCode().value()); Mockito.clearInvocations(mockRepositoryApi); } } diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/leonardo/LeonardoDaoTest.java b/service/src/test/java/org/databiosphere/workspacedataservice/leonardo/LeonardoDaoTest.java index 104aeab21..ba9686b38 100644 --- a/service/src/test/java/org/databiosphere/workspacedataservice/leonardo/LeonardoDaoTest.java +++ b/service/src/test/java/org/databiosphere/workspacedataservice/leonardo/LeonardoDaoTest.java @@ -57,7 +57,7 @@ void testWdsUrlNotReturned() throws ApiException { statusCode, "Intentional error thrown for unit test")); var exception = assertThrows(LeonardoServiceException.class, () -> leonardoDao.getWdsEndpointUrl(any())); - assertEquals(statusCode, exception.getRawStatusCode()); + assertEquals(statusCode, exception.getStatusCode().value()); } @Test diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/pact/WsmPactTest.java b/service/src/test/java/org/databiosphere/workspacedataservice/pact/WsmPactTest.java index 8e818303b..54c866226 100644 --- a/service/src/test/java/org/databiosphere/workspacedataservice/pact/WsmPactTest.java +++ b/service/src/test/java/org/databiosphere/workspacedataservice/pact/WsmPactTest.java @@ -480,7 +480,7 @@ void getBlobStorageUrl_forbidden(MockServer mockServer) { assertThrows( WorkspaceManagerException.class, () -> wsmDao.getBlobStorageUrl(WORKSPACE_UUID.toString(), BEARER_TOKEN)); - assertEquals(HttpStatus.FORBIDDEN, thrown.getStatus()); + assertEquals(HttpStatus.FORBIDDEN, thrown.getStatusCode()); } @Test @@ -494,7 +494,7 @@ void getBlobStorageUrl_noStorageContainers(MockServer mockServer) { assertThrows( WorkspaceManagerException.class, () -> wsmDao.getBlobStorageUrl(WORKSPACE_UUID.toString(), BEARER_TOKEN)); - assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, thrown.getStatus()); + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, thrown.getStatusCode()); } @Test diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/retry/RestClientRetryTest.java b/service/src/test/java/org/databiosphere/workspacedataservice/retry/RestClientRetryTest.java index 7071bb90d..9d756e0c1 100644 --- a/service/src/test/java/org/databiosphere/workspacedataservice/retry/RestClientRetryTest.java +++ b/service/src/test/java/org/databiosphere/workspacedataservice/retry/RestClientRetryTest.java @@ -304,7 +304,7 @@ private void expectRestExceptionWithStatusCode(int expectedStatusCode, RestCall< assertEquals( expectedStatusCode, - actual.getRawStatusCode(), + actual.getStatusCode().value(), "RestCall: Incorrect status code in RestException"); } @@ -318,7 +318,7 @@ private void expectRestExceptionWithStatusCode( assertEquals( expectedStatusCode, - actual.getRawStatusCode(), + actual.getStatusCode().value(), "voidRestCall: Incorrect status code in RestException"); } } diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/service/InstanceServiceSamExceptionTest.java b/service/src/test/java/org/databiosphere/workspacedataservice/service/InstanceServiceSamExceptionTest.java index 475bb36bb..8aa70e3b7 100644 --- a/service/src/test/java/org/databiosphere/workspacedataservice/service/InstanceServiceSamExceptionTest.java +++ b/service/src/test/java/org/databiosphere/workspacedataservice/service/InstanceServiceSamExceptionTest.java @@ -250,7 +250,7 @@ private void doSamCreateTest(UUID instanceId, int expectedSamExceptionCode) { "createInstance should throw if caller does not have permission to create wds-instance resource in Sam"); assertEquals( expectedSamExceptionCode, - samException.getRawStatusCode(), + samException.getStatusCode().value(), "RestException from createInstance should have same status code as the thrown ApiException"); List allInstances = instanceService.listInstances(VERSION); assertFalse(allInstances.contains(instanceId), "should not have created the instances."); @@ -270,7 +270,7 @@ private void doSamDeleteTest(UUID instanceId, int expectedSamExceptionCode) { "deleteInstance should throw if caller does not have permission to create wds-instance resource in Sam"); assertEquals( expectedSamExceptionCode, - samException.getRawStatusCode(), + samException.getStatusCode().value(), "RestException from deleteInstance should have same status code as the thrown ApiException"); allInstances = instanceService.listInstances(VERSION); assertTrue( diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/service/RecordOrchestratorServiceTest.java b/service/src/test/java/org/databiosphere/workspacedataservice/service/RecordOrchestratorServiceTest.java index 24679f3d7..274328462 100644 --- a/service/src/test/java/org/databiosphere/workspacedataservice/service/RecordOrchestratorServiceTest.java +++ b/service/src/test/java/org/databiosphere/workspacedataservice/service/RecordOrchestratorServiceTest.java @@ -168,7 +168,7 @@ void testValidateVersion() { ResponseStatusException.class, () -> validateVersion("invalidVersion"), "validateVersion should have thrown an error"); - assert (e.getStatus().equals(HttpStatus.BAD_REQUEST)); + assert (e.getStatusCode().equals(HttpStatus.BAD_REQUEST)); } private void testCreateRecord(String newRecordId, String testKey, String testVal) { diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/workspacemanager/WorkspaceManagerDaoTest.java b/service/src/test/java/org/databiosphere/workspacedataservice/workspacemanager/WorkspaceManagerDaoTest.java index 5dba73c3a..a6f7fd6d8 100644 --- a/service/src/test/java/org/databiosphere/workspacedataservice/workspacemanager/WorkspaceManagerDaoTest.java +++ b/service/src/test/java/org/databiosphere/workspacedataservice/workspacemanager/WorkspaceManagerDaoTest.java @@ -94,7 +94,7 @@ void testErrorThrown() throws ApiException { assertThrows( WorkspaceManagerException.class, () -> workspaceManagerDao.linkSnapshotForPolicy(testSnapshot)); - assertEquals(statusCode, exception.getRawStatusCode()); + assertEquals(statusCode, exception.getStatusCode().value()); } @Test From 346610d9491a883ab231fed41bf0b31fbc7b53da Mon Sep 17 00:00:00 2001 From: David An Date: Thu, 21 Dec 2023 09:43:31 -0500 Subject: [PATCH 03/14] WIP --- build.gradle | 10 +++---- client/build.gradle | 6 ++--- service/build.gradle | 26 +++++++++---------- .../service/MDCServletRequestListener.java | 6 ++--- .../HttpWorkspaceManagerClientFactory.java | 2 +- 5 files changed, 25 insertions(+), 25 deletions(-) diff --git a/build.gradle b/build.gradle index 4738919e2..7439c48a5 100644 --- a/build.gradle +++ b/build.gradle @@ -1,7 +1,7 @@ import org.springframework.boot.gradle.plugin.SpringBootPlugin plugins { - id 'org.springframework.boot' version '3.2.0' apply false + id 'org.springframework.boot' version '3.0.13' apply false id 'io.spring.dependency-management' version '1.1.4' apply false id 'com.google.cloud.tools.jib' version '3.2.1' apply false id "org.sonarqube" version "4.4.1.3373" apply false @@ -38,10 +38,10 @@ subprojects { } dependencies { - dependency 'org.yaml:snakeyaml:2.0' - dependency 'org.webjars:webjars-locator-core:0.53' - dependency 'ch.qos.logback:logback-classic:1.2.13+' // CVE-2023-6378 - dependency 'ch.qos.logback:logback-core:1.2.13+' // CVE-2023-6378 +// dependency 'org.yaml:snakeyaml:2.0' +// dependency 'org.webjars:webjars-locator-core:0.53' +// dependency 'ch.qos.logback:logback-classic:1.2.13+' // CVE-2023-6378 +// dependency 'ch.qos.logback:logback-core:1.2.13+' // CVE-2023-6378 } } } diff --git a/client/build.gradle b/client/build.gradle index c13c36f82..d95cae570 100644 --- a/client/build.gradle +++ b/client/build.gradle @@ -21,10 +21,10 @@ ext { // default to javax, since WDS is currently on Spring Boot 2. -Pjakarta=true overrides // the default and creates a Jakarta-based client. -if (project.hasProperty("jakarta") && "true".equalsIgnoreCase(project.getProperty("jakarta"))) { - apply from: "dependencies-jakarta.gradle" -} else { +if (project.hasProperty("jakarta") && "false".equalsIgnoreCase(project.getProperty("jakarta"))) { apply from: "dependencies-javax.gradle" +} else { + apply from: "dependencies-jakarta.gradle" } apply from: 'artifactory.gradle' diff --git a/service/build.gradle b/service/build.gradle index cd1edb448..13cec07f2 100644 --- a/service/build.gradle +++ b/service/build.gradle @@ -62,11 +62,11 @@ dependencies { implementation 'org.webjars:webjars-locator-core' // versioned by spring dependency management implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-csv:2.13.5' implementation 'io.sentry:sentry-logback:6.34.0' - implementation 'org.liquibase:liquibase-core:4.21.1' + implementation 'org.liquibase:liquibase-core:4.25.1' implementation 'javax.cache:cache-api' - implementation 'org.ehcache:ehcache:3.10.8' + implementation 'org.ehcache:ehcache:3.10.8:jakarta' implementation 'org.hashids:hashids:1.0.3' - implementation 'javax.ws.rs:javax.ws.rs-api:2.1.1' + implementation 'jakarta.ws.rs:jakarta.ws.rs-api:3.1.0' implementation 'com.google.mug:mug:6.6' // required by openapi-generated models and api interfaces @@ -74,11 +74,11 @@ dependencies { implementation 'io.swagger.core.v3:swagger-annotations:2.2.16' // Terra libraries - implementation group: 'org.broadinstitute.dsde.workbench', name: 'sam-client_2.13', version: '0.1-08b6588' + implementation group: 'org.broadinstitute.dsde.workbench', name: 'sam-client_2.13', version: '0.1-752b4f3' implementation group: 'org.broadinstitute.dsde.workbench', name: 'leonardo-client_2.13', version: '1.3.6-22ee00b' implementation 'com.squareup.okhttp3:okhttp:4.12.0' // required by Sam client - implementation "bio.terra:datarepo-client:1.537.0-SNAPSHOT" - implementation "bio.terra:workspace-manager-client:0.254.717-SNAPSHOT" + implementation "bio.terra:datarepo-client:1.564.0-SNAPSHOT" + implementation "bio.terra:workspace-manager-client:0.254.983-SNAPSHOT" implementation "bio.terra:java-pfb-library:0.15.0" implementation project(path: ':client') @@ -130,14 +130,14 @@ dependencies { // this is redundant (and ineffective) without an explicit override in build.gradle // dependencyManagement, but left here for documentation purposes - implementation('ch.qos.logback:logback-classic:1.2.13+') { - because("CVE-2023-6378") - } +// implementation('ch.qos.logback:logback-classic:1.2.13+') { +// because("CVE-2023-6378") +// } // this is redundant (and ineffective) without an explicit override in build.gradle // dependencyManagement, but left here for documentation purposes - implementation('ch.qos.logback:logback-core:1.2.13+') { - because("CVE-2023-6378") - } +// implementation('ch.qos.logback:logback-core:1.2.13+') { +// because("CVE-2023-6378") +// } } } @@ -253,7 +253,7 @@ testing { implementation project(':client') implementation 'org.springframework.boot:spring-boot-starter-test' implementation 'org.springframework.boot:spring-boot-starter-jdbc' - implementation 'org.liquibase:liquibase-core:4.21.1' + implementation 'org.liquibase:liquibase-core:4.25.1' implementation "org.glassfish.jersey.core:jersey-client:$jersey_version" implementation "org.glassfish.jersey.core:jersey-common:$jersey_version" implementation "org.glassfish.jersey.inject:jersey-hk2:$jersey_version" diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/service/MDCServletRequestListener.java b/service/src/main/java/org/databiosphere/workspacedataservice/service/MDCServletRequestListener.java index 97d8ce82e..8b1f0e8dc 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/service/MDCServletRequestListener.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/service/MDCServletRequestListener.java @@ -1,11 +1,11 @@ package org.databiosphere.workspacedataservice.service; +import jakarta.servlet.ServletRequestEvent; +import jakarta.servlet.ServletRequestListener; +import jakarta.servlet.http.HttpServletRequest; import java.util.Arrays; import java.util.List; import java.util.concurrent.ThreadLocalRandom; -import javax.servlet.ServletRequestEvent; -import javax.servlet.ServletRequestListener; -import javax.servlet.http.HttpServletRequest; import org.hashids.Hashids; import org.slf4j.MDC; import org.springframework.stereotype.Component; diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/workspacemanager/HttpWorkspaceManagerClientFactory.java b/service/src/main/java/org/databiosphere/workspacedataservice/workspacemanager/HttpWorkspaceManagerClientFactory.java index 70962fa25..b87d1a420 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/workspacemanager/HttpWorkspaceManagerClientFactory.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/workspacemanager/HttpWorkspaceManagerClientFactory.java @@ -4,7 +4,7 @@ import bio.terra.workspace.api.ReferencedGcpResourceApi; import bio.terra.workspace.api.ResourceApi; import bio.terra.workspace.client.ApiClient; -import javax.ws.rs.client.Client; +import jakarta.ws.rs.client.Client; import org.apache.commons.lang3.StringUtils; import org.databiosphere.workspacedataservice.sam.TokenContextUtil; import org.databiosphere.workspacedataservice.shared.model.BearerToken; From 95a8cb2c6d8be313341c373c1bb67553bf836829 Mon Sep 17 00:00:00 2001 From: David An Date: Thu, 21 Dec 2023 16:13:30 -0500 Subject: [PATCH 04/14] update logging configs --- service/src/main/resources/logback-spring.xml | 113 +++++++++--------- service/src/test/resources/logback-spring.xml | 34 +++--- 2 files changed, 69 insertions(+), 78 deletions(-) diff --git a/service/src/main/resources/logback-spring.xml b/service/src/main/resources/logback-spring.xml index 30c346292..bad3b9f1c 100644 --- a/service/src/main/resources/logback-spring.xml +++ b/service/src/main/resources/logback-spring.xml @@ -1,63 +1,60 @@ - - - - - - - - ${WDS_LOG_PATTERN} - ${CONSOLE_LOG_CHARSET} - - - - - - ERROR - - INFO - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + ERROR + + INFO + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/service/src/test/resources/logback-spring.xml b/service/src/test/resources/logback-spring.xml index b064be6b2..427b65e45 100644 --- a/service/src/test/resources/logback-spring.xml +++ b/service/src/test/resources/logback-spring.xml @@ -1,28 +1,22 @@ - - - - %green(%d{ISO8601}) %highlight(%-5level) [%blue(%X{requestId})] %yellow(%C{1.}): %msg%n%throwable - - - + + - - - - + + + + - - - - + + + + - - - - + + + + From e550b0326163935fa6e8a67cb5c8a13dda7f967d Mon Sep 17 00:00:00 2001 From: David An Date: Thu, 21 Dec 2023 16:29:42 -0500 Subject: [PATCH 05/14] update data repo client --- build.gradle | 7 ----- service/build.gradle | 29 +++++-------------- .../datarepo/HttpDataRepoClientFactory.java | 2 +- 3 files changed, 9 insertions(+), 29 deletions(-) diff --git a/build.gradle b/build.gradle index 7439c48a5..645655aea 100644 --- a/build.gradle +++ b/build.gradle @@ -36,13 +36,6 @@ subprojects { imports { mavenBom(SpringBootPlugin.BOM_COORDINATES) } - - dependencies { -// dependency 'org.yaml:snakeyaml:2.0' -// dependency 'org.webjars:webjars-locator-core:0.53' -// dependency 'ch.qos.logback:logback-classic:1.2.13+' // CVE-2023-6378 -// dependency 'ch.qos.logback:logback-core:1.2.13+' // CVE-2023-6378 - } } } diff --git a/service/build.gradle b/service/build.gradle index 13cec07f2..53d055282 100644 --- a/service/build.gradle +++ b/service/build.gradle @@ -36,7 +36,6 @@ jib { } ext { - jersey_version = "2.36" parquet_mr_version = "1.13.1" hadoop_version = "3.3.6" } @@ -77,7 +76,7 @@ dependencies { implementation group: 'org.broadinstitute.dsde.workbench', name: 'sam-client_2.13', version: '0.1-752b4f3' implementation group: 'org.broadinstitute.dsde.workbench', name: 'leonardo-client_2.13', version: '1.3.6-22ee00b' implementation 'com.squareup.okhttp3:okhttp:4.12.0' // required by Sam client - implementation "bio.terra:datarepo-client:1.564.0-SNAPSHOT" + implementation "bio.terra:datarepo-jakarta-client:1.557.0-SNAPSHOT" implementation "bio.terra:workspace-manager-client:0.254.983-SNAPSHOT" implementation "bio.terra:java-pfb-library:0.15.0" implementation project(path: ':client') @@ -105,10 +104,10 @@ dependencies { implementation "org.apache.hadoop:hadoop-common:$hadoop_version", withoutHadoopExcludes implementation "org.apache.hadoop:hadoop-mapreduce-client-core:$hadoop_version", withoutHadoopExcludes - // hk2 is required to use WSM client, but not correctly exposed by the client - implementation "org.glassfish.jersey.inject:jersey-hk2:$jersey_version" - implementation "org.glassfish.jersey.core:jersey-client:$jersey_version" - implementation "org.glassfish.jersey.core:jersey-common:$jersey_version" + // Jersey libs, required by WSM client (and maybe other clients?), version managed by Spring + implementation "org.glassfish.jersey.inject:jersey-hk2" + implementation "org.glassfish.jersey.core:jersey-client" + implementation "org.glassfish.jersey.core:jersey-common" annotationProcessor 'org.springframework.boot:spring-boot-configuration-processor' runtimeOnly 'org.webjars.npm:swagger-ui-dist:5.9.0' @@ -126,18 +125,6 @@ dependencies { implementation('org.apache.commons:commons-compress:1.24.0') { because("CVE-2023-42503") } - - - // this is redundant (and ineffective) without an explicit override in build.gradle - // dependencyManagement, but left here for documentation purposes -// implementation('ch.qos.logback:logback-classic:1.2.13+') { -// because("CVE-2023-6378") -// } - // this is redundant (and ineffective) without an explicit override in build.gradle - // dependencyManagement, but left here for documentation purposes -// implementation('ch.qos.logback:logback-core:1.2.13+') { -// because("CVE-2023-6378") -// } } } @@ -254,9 +241,9 @@ testing { implementation 'org.springframework.boot:spring-boot-starter-test' implementation 'org.springframework.boot:spring-boot-starter-jdbc' implementation 'org.liquibase:liquibase-core:4.25.1' - implementation "org.glassfish.jersey.core:jersey-client:$jersey_version" - implementation "org.glassfish.jersey.core:jersey-common:$jersey_version" - implementation "org.glassfish.jersey.inject:jersey-hk2:$jersey_version" + implementation "org.glassfish.jersey.core:jersey-client" + implementation "org.glassfish.jersey.core:jersey-common" + implementation "org.glassfish.jersey.inject:jersey-hk2" } } } diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/datarepo/HttpDataRepoClientFactory.java b/service/src/main/java/org/databiosphere/workspacedataservice/datarepo/HttpDataRepoClientFactory.java index cd788fd44..fea385cd7 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/datarepo/HttpDataRepoClientFactory.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/datarepo/HttpDataRepoClientFactory.java @@ -2,7 +2,7 @@ import bio.terra.datarepo.api.RepositoryApi; import bio.terra.datarepo.client.ApiClient; -import javax.ws.rs.client.Client; +import jakarta.ws.rs.client.Client; import org.apache.commons.lang3.StringUtils; import org.databiosphere.workspacedataservice.sam.TokenContextUtil; import org.databiosphere.workspacedataservice.shared.model.BearerToken; From 16ec73d05434178e87a73a4f5f0ce039c6649e58 Mon Sep 17 00:00:00 2001 From: David An Date: Thu, 21 Dec 2023 16:48:28 -0500 Subject: [PATCH 06/14] global exception handler, pact test fixes --- .../controller/GlobalExceptionHandler.java | 43 +++++++++++++++++++ .../pact/SamPactTest.java | 5 ++- 2 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java new file mode 100644 index 000000000..39f74d7c3 --- /dev/null +++ b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java @@ -0,0 +1,43 @@ +package org.databiosphere.workspacedataservice.controller; + +import jakarta.servlet.http.HttpServletRequest; +import java.util.Date; +import java.util.LinkedHashMap; +import java.util.Map; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; +import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; + +@ControllerAdvice +public class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + + /** + * Handler for MethodArgumentTypeMismatchException. This exception contains the real message we + * want to display inside the nested cause. This handler ditches the top-level message in favor of + * that nested one. + * + * @param ex the exception in question + * @param servletRequest the request + * @return the desired response + */ + @ExceptionHandler({MethodArgumentTypeMismatchException.class}) + public ResponseEntity> handleAllExceptions( + Exception ex, HttpServletRequest servletRequest) { + Map errorBody = new LinkedHashMap<>(); + + errorBody.put("timestamp", new Date()); + errorBody.put("status", HttpStatus.BAD_REQUEST.value()); + errorBody.put("error", HttpStatus.BAD_REQUEST.getReasonPhrase()); + // MethodArgumentTypeMismatchException nested exception contains + // the real message we want to display + if (ex.getCause() != null && ex.getCause().getCause() != null) { + errorBody.put("message", ex.getCause().getCause().getMessage()); + } + errorBody.put("path", servletRequest.getRequestURI()); + + return ResponseEntity.badRequest().body(errorBody); + } +} diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/pact/SamPactTest.java b/service/src/test/java/org/databiosphere/workspacedataservice/pact/SamPactTest.java index 37590055e..c1823fd6d 100644 --- a/service/src/test/java/org/databiosphere/workspacedataservice/pact/SamPactTest.java +++ b/service/src/test/java/org/databiosphere/workspacedataservice/pact/SamPactTest.java @@ -48,7 +48,7 @@ public RequestResponsePact statusApiPact(PactDslWithProvider builder) { .method("GET") .willRespondWith() .status(200) - .body("{\"ok\": true}") + .body("{\"ok\": true, \"systems\": {}}") .toPact(); } @@ -61,7 +61,7 @@ public RequestResponsePact downStatusApiPact(PactDslWithProvider builder) { .method("GET") .willRespondWith() .status(500) - .body("{\"ok\": false}") + .body("{\"ok\": false, \"systems\": {}}") .toPact(); } @@ -131,6 +131,7 @@ public RequestResponsePact userStatusPact(PactDslWithProvider builder) { new PactDslJsonBody() .stringType("userSubjectId") .stringType("userEmail") + .booleanType("adminEnabled") .booleanType("enabled"); return builder .given("user status info request with access token") From 56a407f30667a79aba53e092d560fc11b21f1a5d Mon Sep 17 00:00:00 2001 From: David An Date: Thu, 21 Dec 2023 17:00:01 -0500 Subject: [PATCH 07/14] disable problem details? --- .../controller/GlobalExceptionHandler.java | 2 ++ service/src/main/resources/application.yml | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java index 39f74d7c3..deab3f373 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java @@ -28,6 +28,8 @@ public ResponseEntity> handleAllExceptions( Exception ex, HttpServletRequest servletRequest) { Map errorBody = new LinkedHashMap<>(); + // TODO AJ-1157: what shape should we return here? This will depend on problem details being + // enabled or disabled. errorBody.put("timestamp", new Date()); errorBody.put("status", HttpStatus.BAD_REQUEST.value()); errorBody.put("error", HttpStatus.BAD_REQUEST.getReasonPhrase()); diff --git a/service/src/main/resources/application.yml b/service/src/main/resources/application.yml index 36f15feb3..8d883cd7e 100644 --- a/service/src/main/resources/application.yml +++ b/service/src/main/resources/application.yml @@ -53,6 +53,10 @@ spring: password: ${env.wds.db.password} maximum-pool-size: 7 minimum-idle: 7 + mvc: + problemdetails: + # TODO AJ-1157: can we turn this back on? + enabled: false servlet: multipart.max-request-size: 5GB multipart.max-file-size: 5GB From a319bfc78b5c9d0185d0003f37f8a1480f52b9cf Mon Sep 17 00:00:00 2001 From: David An Date: Fri, 22 Dec 2023 15:12:40 -0500 Subject: [PATCH 08/14] handle multiply-nested error messages --- .../controller/GlobalExceptionHandler.java | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java index deab3f373..d770c4c11 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java @@ -1,9 +1,12 @@ package org.databiosphere.workspacedataservice.controller; import jakarta.servlet.http.HttpServletRequest; +import java.util.ArrayList; import java.util.Date; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; +import org.apache.commons.lang3.StringUtils; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ControllerAdvice; @@ -33,13 +36,27 @@ public ResponseEntity> handleAllExceptions( errorBody.put("timestamp", new Date()); errorBody.put("status", HttpStatus.BAD_REQUEST.value()); errorBody.put("error", HttpStatus.BAD_REQUEST.getReasonPhrase()); - // MethodArgumentTypeMismatchException nested exception contains - // the real message we want to display - if (ex.getCause() != null && ex.getCause().getCause() != null) { - errorBody.put("message", ex.getCause().getCause().getMessage()); - } errorBody.put("path", servletRequest.getRequestURI()); + // MethodArgumentTypeMismatchException nested exceptions contains + // the real message we want to display. Gather all the nested messages and display the last one. + List errorMessages = gatherNestedErrorMessages(ex, new ArrayList<>()); + if (!errorMessages.isEmpty()) { + errorBody.put("messages", errorMessages.get(errorMessages.size() - 1)); + } else { + errorBody.put("messages", "Unexpected error: " + ex.getClass().getName()); + } + return ResponseEntity.badRequest().body(errorBody); } + + private List gatherNestedErrorMessages(Throwable t, List accumulator) { + if (StringUtils.isNotBlank(t.getMessage())) { + accumulator.add(t.getMessage()); + } + if (t.getCause() == null) { + return accumulator; + } + return gatherNestedErrorMessages(t.getCause(), accumulator); + } } From e77c57a3db9fe05297fa0754a8a93051d0d66780 Mon Sep 17 00:00:00 2001 From: David An Date: Fri, 22 Dec 2023 15:31:57 -0500 Subject: [PATCH 09/14] fully disable problem details --- .../controller/GlobalExceptionHandler.java | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java index d770c4c11..1a8bfb743 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java @@ -7,16 +7,60 @@ import java.util.List; import java.util.Map; import org.apache.commons.lang3.StringUtils; +import org.jetbrains.annotations.NotNull; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.ProblemDetail; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.context.request.ServletWebRequest; +import org.springframework.web.context.request.WebRequest; import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler; @ControllerAdvice public class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + /** + * Override to explicitly translate Problem Details structures back to {@link + * org.databiosphere.workspacedata.model.ErrorResponse} structures. This ensures backwards + * compatibility for clients. + * + *

Even though {@code }spring.mvc.problemdetails.enabled} is set to false in config, many + * exception classes extend {@link org.springframework.web.server.ResponseStatusException}, and + * that exception serializes to a problem detail in the response. + * + * @param body the body to use for the response + * @param headers the headers to use for the response + * @param statusCode the status code to use for the response + * @param request the current request + * @return the {@code ResponseEntity} instance to use + */ + @NotNull + @Override + protected ResponseEntity createResponseEntity( + Object body, + @NotNull HttpHeaders headers, + @NotNull HttpStatusCode statusCode, + @NotNull WebRequest request) { + if (body instanceof ProblemDetail problemDetail) { + Map errorBody = new LinkedHashMap<>(); + errorBody.put("timestamp", new Date()); + errorBody.put("status", problemDetail.getStatus()); + errorBody.put("error", problemDetail.getTitle()); + errorBody.put("message", problemDetail.getDetail()); + + if (request instanceof ServletWebRequest servletWebRequest) { + errorBody.put("path", servletWebRequest.getRequest().getRequestURI()); + } + return new ResponseEntity<>(errorBody, headers, statusCode); + } + + return super.createResponseEntity(body, headers, statusCode, request); + } + /** * Handler for MethodArgumentTypeMismatchException. This exception contains the real message we * want to display inside the nested cause. This handler ditches the top-level message in favor of @@ -42,9 +86,9 @@ public ResponseEntity> handleAllExceptions( // the real message we want to display. Gather all the nested messages and display the last one. List errorMessages = gatherNestedErrorMessages(ex, new ArrayList<>()); if (!errorMessages.isEmpty()) { - errorBody.put("messages", errorMessages.get(errorMessages.size() - 1)); + errorBody.put("message", errorMessages.get(errorMessages.size() - 1)); } else { - errorBody.put("messages", "Unexpected error: " + ex.getClass().getName()); + errorBody.put("message", "Unexpected error: " + ex.getClass().getName()); } return ResponseEntity.badRequest().body(errorBody); From 251a1cf3a3b511bc49bd7b65766f284793b2cfea Mon Sep 17 00:00:00 2001 From: David An Date: Sat, 23 Dec 2023 09:32:31 -0500 Subject: [PATCH 10/14] fixes to prometheus-related config --- service/src/main/resources/application.yml | 4 ++++ .../workspacedataservice/metrics/MetricsConfigTest.java | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/service/src/main/resources/application.yml b/service/src/main/resources/application.yml index d3e1adb45..0cc779346 100644 --- a/service/src/main/resources/application.yml +++ b/service/src/main/resources/application.yml @@ -39,6 +39,10 @@ management: info: env: enabled: true + prometheus: + metrics: + export: + enabled: true info: app: diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/metrics/MetricsConfigTest.java b/service/src/test/java/org/databiosphere/workspacedataservice/metrics/MetricsConfigTest.java index f0bbc0788..6f72a0866 100644 --- a/service/src/test/java/org/databiosphere/workspacedataservice/metrics/MetricsConfigTest.java +++ b/service/src/test/java/org/databiosphere/workspacedataservice/metrics/MetricsConfigTest.java @@ -21,7 +21,7 @@ import org.springframework.test.web.servlet.MockMvc; @DirtiesContext -@SpringBootTest(properties = "management.metrics.export.prometheus.enabled=true") +@SpringBootTest(properties = "management.prometheus.metrics.export.enabled=true") @AutoConfigureMockMvc class MetricsConfigTest { @Autowired private BuildProperties buildProperties; From 96144de4e4a294b66bfc84f0a911a2bdf3890bec Mon Sep 17 00:00:00 2001 From: David An Date: Sat, 23 Dec 2023 09:41:19 -0500 Subject: [PATCH 11/14] unit test for nested messages in exception handler --- .../controller/GlobalExceptionHandler.java | 7 +- .../GlobalExceptionHandlerTest.java | 75 +++++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 service/src/test/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandlerTest.java diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java index 1a8bfb743..c9a5790e0 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java @@ -1,5 +1,6 @@ package org.databiosphere.workspacedataservice.controller; +import com.google.common.annotations.VisibleForTesting; import jakarta.servlet.http.HttpServletRequest; import java.util.ArrayList; import java.util.Date; @@ -74,9 +75,6 @@ protected ResponseEntity createResponseEntity( public ResponseEntity> handleAllExceptions( Exception ex, HttpServletRequest servletRequest) { Map errorBody = new LinkedHashMap<>(); - - // TODO AJ-1157: what shape should we return here? This will depend on problem details being - // enabled or disabled. errorBody.put("timestamp", new Date()); errorBody.put("status", HttpStatus.BAD_REQUEST.value()); errorBody.put("error", HttpStatus.BAD_REQUEST.getReasonPhrase()); @@ -94,7 +92,8 @@ public ResponseEntity> handleAllExceptions( return ResponseEntity.badRequest().body(errorBody); } - private List gatherNestedErrorMessages(Throwable t, List accumulator) { + @VisibleForTesting + List gatherNestedErrorMessages(@NotNull Throwable t, @NotNull List accumulator) { if (StringUtils.isNotBlank(t.getMessage())) { accumulator.add(t.getMessage()); } diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandlerTest.java b/service/src/test/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandlerTest.java new file mode 100644 index 000000000..a9b0fc9f4 --- /dev/null +++ b/service/src/test/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandlerTest.java @@ -0,0 +1,75 @@ +package org.databiosphere.workspacedataservice.controller; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; + +@SpringBootTest +class GlobalExceptionHandlerTest { + @Autowired private GlobalExceptionHandler globalExceptionHandler; + + @Test + void gatherNestedErrorMessagesNullMessage() { + Throwable input = new Throwable(); + List actual = + globalExceptionHandler.gatherNestedErrorMessages(input, new ArrayList<>()); + assertThat(actual).isEmpty(); + } + + @Test + void gatherNestedErrorMessagesEmptyMessage() { + Throwable input = new Throwable(""); + List actual = + globalExceptionHandler.gatherNestedErrorMessages(input, new ArrayList<>()); + assertThat(actual).isEmpty(); + } + + @Test + void gatherNestedErrorMessagesBlankMessage() { + Throwable input = new Throwable(" "); + List actual = + globalExceptionHandler.gatherNestedErrorMessages(input, new ArrayList<>()); + assertThat(actual).isEmpty(); + } + + @Test + void gatherNestedErrorMessagesOneMessage() { + Throwable input = new Throwable("one"); + List actual = + globalExceptionHandler.gatherNestedErrorMessages(input, new ArrayList<>()); + assertThat(actual).size().isEqualTo(1); + assertEquals(actual.get(0), "one"); + } + + @Test + void gatherNestedErrorMessagesTwoMessages() { + Throwable input = new Throwable("one"); + Throwable nested = new Throwable("two"); + input.initCause(nested); + List actual = + globalExceptionHandler.gatherNestedErrorMessages(input, new ArrayList<>()); + assertThat(actual).size().isEqualTo(2); + assertEquals(actual.get(0), "one"); + assertEquals(actual.get(1), "two"); + } + + @Test + void gatherNestedErrorMessagesThreeMessages() { + Throwable input = new Throwable("one"); + Throwable nested = new Throwable("two"); + Throwable third = new Throwable("three"); + nested.initCause(third); + input.initCause(nested); + List actual = + globalExceptionHandler.gatherNestedErrorMessages(input, new ArrayList<>()); + assertThat(actual).size().isEqualTo(3); + assertEquals(actual.get(0), "one"); + assertEquals(actual.get(1), "two"); + assertEquals(actual.get(2), "three"); + } +} From 1ba051a1b758c08a99d9a377695a075ebdc5cae4 Mon Sep 17 00:00:00 2001 From: David An Date: Wed, 27 Dec 2023 14:10:29 -0500 Subject: [PATCH 12/14] Spring Boot 3.2.1 --- build.gradle | 5 ++++- client/swagger.gradle | 6 +++--- service/build.gradle | 14 +++++++------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/build.gradle b/build.gradle index 645655aea..83e45ecad 100644 --- a/build.gradle +++ b/build.gradle @@ -1,7 +1,7 @@ import org.springframework.boot.gradle.plugin.SpringBootPlugin plugins { - id 'org.springframework.boot' version '3.0.13' apply false + id 'org.springframework.boot' version '3.2.1' apply false id 'io.spring.dependency-management' version '1.1.4' apply false id 'com.google.cloud.tools.jib' version '3.2.1' apply false id "org.sonarqube" version "4.4.1.3373" apply false @@ -36,6 +36,9 @@ subprojects { imports { mavenBom(SpringBootPlugin.BOM_COORDINATES) } + dependencies { + dependency 'org.liquibase:liquibase-core:4.25.1' + } } } diff --git a/client/swagger.gradle b/client/swagger.gradle index e4a666a38..05a28cdac 100644 --- a/client/swagger.gradle +++ b/client/swagger.gradle @@ -1,9 +1,9 @@ dependencies { //Need to include libraries for generated code to work - api 'com.google.code.gson:gson:2.10.1' + api 'com.google.code.gson:gson' api 'io.gsonfire:gson-fire:1.9.0' - api 'com.squareup.okhttp3:okhttp:4.12.0' - api 'com.squareup.okhttp3:logging-interceptor:4.12.0' + api 'com.squareup.okhttp3:okhttp' + api 'com.squareup.okhttp3:logging-interceptor' } openApiValidate { diff --git a/service/build.gradle b/service/build.gradle index 59a644662..99862105d 100644 --- a/service/build.gradle +++ b/service/build.gradle @@ -56,17 +56,17 @@ dependencies { implementation 'org.springframework.boot:spring-boot-starter-actuator' implementation 'io.micrometer:micrometer-registry-prometheus' implementation 'org.springframework.integration:spring-integration-jdbc' - implementation 'org.aspectj:aspectjweaver:1.8.9' // required by spring-retry, not used directly + implementation 'org.aspectj:aspectjweaver' // required by spring-retry, not used directly implementation 'com.google.guava:guava:32.1.3-jre' implementation 'org.postgresql:postgresql' implementation 'org.webjars:webjars-locator-core' // versioned by spring dependency management - implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-csv:2.13.5' + implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-csv' implementation 'io.sentry:sentry-logback:6.34.0' - implementation 'org.liquibase:liquibase-core:4.25.1' + implementation 'org.liquibase:liquibase-core' implementation 'javax.cache:cache-api' implementation 'org.ehcache:ehcache:3.10.8:jakarta' implementation 'org.hashids:hashids:1.0.3' - implementation 'jakarta.ws.rs:jakarta.ws.rs-api:3.1.0' + implementation 'jakarta.ws.rs:jakarta.ws.rs-api' implementation 'com.google.mug:mug:6.6' // required by openapi-generated models and api interfaces @@ -76,7 +76,7 @@ dependencies { // Terra libraries implementation group: 'org.broadinstitute.dsde.workbench', name: 'sam-client_2.13', version: '0.1-752b4f3' implementation group: 'org.broadinstitute.dsde.workbench', name: 'leonardo-client_2.13', version: '1.3.6-22ee00b' - implementation 'com.squareup.okhttp3:okhttp:4.12.0' // required by Sam client + implementation 'com.squareup.okhttp3:okhttp' // required by Sam client implementation "bio.terra:datarepo-jakarta-client:1.557.0-SNAPSHOT" implementation "bio.terra:workspace-manager-client:0.254.983-SNAPSHOT" implementation "bio.terra:java-pfb-library:0.15.0" @@ -113,7 +113,7 @@ dependencies { annotationProcessor 'org.springframework.boot:spring-boot-configuration-processor' runtimeOnly 'org.webjars.npm:swagger-ui-dist:5.9.0' testImplementation 'org.springframework.boot:spring-boot-starter-test' - testImplementation 'org.junit.jupiter:junit-jupiter:5.8.2' + testImplementation 'org.junit.jupiter:junit-jupiter' testImplementation project(':client') testImplementation 'au.com.dius.pact.consumer:junit5:4.6.1' testImplementation 'org.junit-pioneer:junit-pioneer:2.1.0' @@ -241,7 +241,7 @@ testing { implementation project(':client') implementation 'org.springframework.boot:spring-boot-starter-test' implementation 'org.springframework.boot:spring-boot-starter-jdbc' - implementation 'org.liquibase:liquibase-core:4.25.1' + implementation 'org.liquibase:liquibase-core' implementation "org.glassfish.jersey.core:jersey-client" implementation "org.glassfish.jersey.core:jersey-common" implementation "org.glassfish.jersey.inject:jersey-hk2" From 44ddb7dccc105e115215ad63b0b567590d01a1b2 Mon Sep 17 00:00:00 2001 From: David An Date: Wed, 27 Dec 2023 14:46:03 -0500 Subject: [PATCH 13/14] exception handler cleanup --- .../controller/GlobalExceptionHandler.java | 31 ++++++++++++------- .../GlobalExceptionHandlerTest.java | 12 +++---- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java index c9a5790e0..f743a1e9e 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java @@ -12,6 +12,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; +import org.springframework.http.MediaType; import org.springframework.http.ProblemDetail; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ControllerAdvice; @@ -24,6 +25,12 @@ @ControllerAdvice public class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + private final String ERROR = "error"; + private final String MESSAGE = "message"; + private final String PATH = "path"; + private final String STATUS = "status"; + private final String TIMESTAMP = "timestamp"; + /** * Override to explicitly translate Problem Details structures back to {@link * org.databiosphere.workspacedata.model.ErrorResponse} structures. This ensures backwards @@ -48,13 +55,13 @@ protected ResponseEntity createResponseEntity( @NotNull WebRequest request) { if (body instanceof ProblemDetail problemDetail) { Map errorBody = new LinkedHashMap<>(); - errorBody.put("timestamp", new Date()); - errorBody.put("status", problemDetail.getStatus()); - errorBody.put("error", problemDetail.getTitle()); - errorBody.put("message", problemDetail.getDetail()); + errorBody.put(ERROR, problemDetail.getTitle()); + errorBody.put(MESSAGE, problemDetail.getDetail()); + errorBody.put(STATUS, problemDetail.getStatus()); + errorBody.put(TIMESTAMP, new Date()); if (request instanceof ServletWebRequest servletWebRequest) { - errorBody.put("path", servletWebRequest.getRequest().getRequestURI()); + errorBody.put(PATH, servletWebRequest.getRequest().getRequestURI()); } return new ResponseEntity<>(errorBody, headers, statusCode); } @@ -75,21 +82,21 @@ protected ResponseEntity createResponseEntity( public ResponseEntity> handleAllExceptions( Exception ex, HttpServletRequest servletRequest) { Map errorBody = new LinkedHashMap<>(); - errorBody.put("timestamp", new Date()); - errorBody.put("status", HttpStatus.BAD_REQUEST.value()); - errorBody.put("error", HttpStatus.BAD_REQUEST.getReasonPhrase()); - errorBody.put("path", servletRequest.getRequestURI()); + errorBody.put(ERROR, HttpStatus.BAD_REQUEST.getReasonPhrase()); + errorBody.put(PATH, servletRequest.getRequestURI()); + errorBody.put(STATUS, HttpStatus.BAD_REQUEST.value()); + errorBody.put(TIMESTAMP, new Date()); // MethodArgumentTypeMismatchException nested exceptions contains // the real message we want to display. Gather all the nested messages and display the last one. List errorMessages = gatherNestedErrorMessages(ex, new ArrayList<>()); if (!errorMessages.isEmpty()) { - errorBody.put("message", errorMessages.get(errorMessages.size() - 1)); + errorBody.put(MESSAGE, errorMessages.get(errorMessages.size() - 1)); } else { - errorBody.put("message", "Unexpected error: " + ex.getClass().getName()); + errorBody.put(MESSAGE, "Unexpected error: " + ex.getClass().getName()); } - return ResponseEntity.badRequest().body(errorBody); + return ResponseEntity.badRequest().contentType(MediaType.APPLICATION_JSON).body(errorBody); } @VisibleForTesting diff --git a/service/src/test/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandlerTest.java b/service/src/test/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandlerTest.java index a9b0fc9f4..0ab00249d 100644 --- a/service/src/test/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandlerTest.java +++ b/service/src/test/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandlerTest.java @@ -43,7 +43,7 @@ void gatherNestedErrorMessagesOneMessage() { List actual = globalExceptionHandler.gatherNestedErrorMessages(input, new ArrayList<>()); assertThat(actual).size().isEqualTo(1); - assertEquals(actual.get(0), "one"); + assertEquals("one", actual.get(0)); } @Test @@ -54,8 +54,8 @@ void gatherNestedErrorMessagesTwoMessages() { List actual = globalExceptionHandler.gatherNestedErrorMessages(input, new ArrayList<>()); assertThat(actual).size().isEqualTo(2); - assertEquals(actual.get(0), "one"); - assertEquals(actual.get(1), "two"); + assertEquals("one", actual.get(0)); + assertEquals("two", actual.get(1)); } @Test @@ -68,8 +68,8 @@ void gatherNestedErrorMessagesThreeMessages() { List actual = globalExceptionHandler.gatherNestedErrorMessages(input, new ArrayList<>()); assertThat(actual).size().isEqualTo(3); - assertEquals(actual.get(0), "one"); - assertEquals(actual.get(1), "two"); - assertEquals(actual.get(2), "three"); + assertEquals("one", actual.get(0)); + assertEquals("two", actual.get(1)); + assertEquals("three", actual.get(2)); } } From dffe5d1987355dcae030a78a83afe7d7d1777a6d Mon Sep 17 00:00:00 2001 From: David An Date: Wed, 27 Dec 2023 14:55:29 -0500 Subject: [PATCH 14/14] static fields --- .../controller/GlobalExceptionHandler.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java index f743a1e9e..30e643b1f 100644 --- a/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java +++ b/service/src/main/java/org/databiosphere/workspacedataservice/controller/GlobalExceptionHandler.java @@ -25,18 +25,18 @@ @ControllerAdvice public class GlobalExceptionHandler extends ResponseEntityExceptionHandler { - private final String ERROR = "error"; - private final String MESSAGE = "message"; - private final String PATH = "path"; - private final String STATUS = "status"; - private final String TIMESTAMP = "timestamp"; + private static final String ERROR = "error"; + private static final String MESSAGE = "message"; + private static final String PATH = "path"; + private static final String STATUS = "status"; + private static final String TIMESTAMP = "timestamp"; /** * Override to explicitly translate Problem Details structures back to {@link * org.databiosphere.workspacedata.model.ErrorResponse} structures. This ensures backwards * compatibility for clients. * - *

Even though {@code }spring.mvc.problemdetails.enabled} is set to false in config, many + *

Even though {@code spring.mvc.problemdetails.enabled} is set to false in config, many * exception classes extend {@link org.springframework.web.server.ResponseStatusException}, and * that exception serializes to a problem detail in the response. *