From e97f6a8209a7f960cdf26492edd06a9fe4714d2c Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Fri, 7 Jun 2024 11:29:57 -0400 Subject: [PATCH 1/9] use spring hateoas to build test links; also use @MockBean at the class level to bulk mock components that don't need stubbing --- build.gradle | 1 + .../controller/SnapshotValidationTest.java | 27 +++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/build.gradle b/build.gradle index 8523b7aa91..b0a40ca465 100644 --- a/build.gradle +++ b/build.gradle @@ -220,6 +220,7 @@ dependencies { implementation 'com.squareup.okhttp3:okhttp' implementation 'org.springframework.boot:spring-boot-starter-actuator' + implementation 'org.springframework.hateoas:spring-hateoas' implementation 'io.micrometer:micrometer-registry-prometheus' implementation 'com.fasterxml.jackson.core:jackson-core:2.15.3' diff --git a/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java b/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java index f4b76ca2ac..aacb26738b 100644 --- a/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java +++ b/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java @@ -6,6 +6,8 @@ import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; +import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.linkTo; +import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.methodOn; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -13,6 +15,7 @@ import bio.terra.common.TestUtils; import bio.terra.common.category.Unit; import bio.terra.common.iam.AuthenticatedUserRequestFactory; +import bio.terra.controller.SnapshotsApi; import bio.terra.model.ErrorModel; import bio.terra.model.SnapshotRequestAssetModel; import bio.terra.model.SnapshotRequestContentsModel; @@ -46,7 +49,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.MockBean; -import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.test.context.ActiveProfiles; @@ -63,19 +65,21 @@ SnapshotsApiController.class, SnapshotRequestValidator.class }) +@MockBean({ + JobService.class, + SnapshotService.class, + IamService.class, + FileService.class, + AuthenticatedUserRequestFactory.class, + SnapshotBuilderService.class +}) @Tag(Unit.TAG) @WebMvcTest class SnapshotValidationTest { @Autowired private MockMvc mvc; - @MockBean private JobService jobService; - @MockBean private SnapshotService snapshotService; - @MockBean private IamService iamService; @MockBean private IngestRequestValidator ingestRequestValidator; - @MockBean private FileService fileService; - @MockBean private AuthenticatedUserRequestFactory authenticatedUserRequestFactory; - @MockBean private SnapshotBuilderService snapshotBuilderService; - @SpyBean private ApplicationConfiguration applicationConfiguration; + @MockBean private ApplicationConfiguration applicationConfiguration; private SnapshotRequestModel snapshotByAssetRequest; @@ -93,9 +97,10 @@ void setup() { private ErrorModel expectBadSnapshotCreateRequest(SnapshotRequestModel snapshotRequest) throws Exception { + var uri = linkTo(methodOn(SnapshotsApi.class).createSnapshot(snapshotRequest)).toUri(); MvcResult result = mvc.perform( - post("/api/repository/v1/snapshots") + post(uri) .contentType(MediaType.APPLICATION_JSON) .content(TestUtils.mapToJson(snapshotRequest))) .andExpect(status().is4xxClientError()) @@ -318,10 +323,10 @@ private void checkValidationErrorModel(ErrorModel errorModel, String[] messageCo * * We check to see if the code is wrapped in quotes to prevent matching on substrings. */ - List> expectedMatches = + var expectedMatches = Arrays.stream(messageCodes) .map(code -> containsString("'" + code + "'")) - .collect(Collectors.toList()); + .toList(); assertThat("Detail codes are right", details, containsInAnyOrder(expectedMatches)); } } From 41e863d811392a0c8f473cec16872a4d76a7d352 Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Fri, 7 Jun 2024 14:36:18 -0400 Subject: [PATCH 2/9] spotless --- .../bio/terra/app/controller/SnapshotValidationTest.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java b/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java index aacb26738b..7ffec189a4 100644 --- a/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java +++ b/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java @@ -35,10 +35,8 @@ import java.util.Collections; import java.util.List; import java.util.UUID; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.commons.lang3.StringUtils; -import org.hamcrest.Matcher; import org.hamcrest.Matchers; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; @@ -324,9 +322,7 @@ private void checkValidationErrorModel(ErrorModel errorModel, String[] messageCo * We check to see if the code is wrapped in quotes to prevent matching on substrings. */ var expectedMatches = - Arrays.stream(messageCodes) - .map(code -> containsString("'" + code + "'")) - .toList(); + Arrays.stream(messageCodes).map(code -> containsString("'" + code + "'")).toList(); assertThat("Detail codes are right", details, containsInAnyOrder(expectedMatches)); } } From b5c6834aba8ce500e4c46d0c7ced4716ebdc5212 Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Fri, 7 Jun 2024 15:24:52 -0400 Subject: [PATCH 3/9] move unit test configuration so it's included in test configuration --- .../configuration}/UnitTestConfiguration.java | 3 +-- .../dataset/DatasetRequestValidatorTest.java | 18 ++++++++++-------- .../snapshot/SnapshotRequestValidatorTest.java | 16 +++++++++------- 3 files changed, 20 insertions(+), 17 deletions(-) rename src/test/java/bio/terra/{common/fixtures => app/configuration}/UnitTestConfiguration.java (90%) diff --git a/src/test/java/bio/terra/common/fixtures/UnitTestConfiguration.java b/src/test/java/bio/terra/app/configuration/UnitTestConfiguration.java similarity index 90% rename from src/test/java/bio/terra/common/fixtures/UnitTestConfiguration.java rename to src/test/java/bio/terra/app/configuration/UnitTestConfiguration.java index 3c19309ef9..1ce19aa498 100644 --- a/src/test/java/bio/terra/common/fixtures/UnitTestConfiguration.java +++ b/src/test/java/bio/terra/app/configuration/UnitTestConfiguration.java @@ -1,6 +1,5 @@ -package bio.terra.common.fixtures; +package bio.terra.app.configuration; -import bio.terra.app.configuration.ApplicationConfiguration; import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; diff --git a/src/test/java/bio/terra/service/dataset/DatasetRequestValidatorTest.java b/src/test/java/bio/terra/service/dataset/DatasetRequestValidatorTest.java index bf696f4d95..69a35b1be1 100644 --- a/src/test/java/bio/terra/service/dataset/DatasetRequestValidatorTest.java +++ b/src/test/java/bio/terra/service/dataset/DatasetRequestValidatorTest.java @@ -17,6 +17,7 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import bio.terra.app.configuration.UnitTestConfiguration; import bio.terra.app.controller.ApiValidationExceptionHandler; import bio.terra.app.controller.DatasetsApiController; import bio.terra.app.controller.GlobalExceptionHandler; @@ -25,7 +26,6 @@ import bio.terra.app.controller.converters.SqlSortDirectionDescDefaultConverter; import bio.terra.common.TestUtils; import bio.terra.common.category.Unit; -import bio.terra.common.fixtures.UnitTestConfiguration; import bio.terra.common.iam.AuthenticatedUserRequestFactory; import bio.terra.model.AssetModel; import bio.terra.model.AssetTableModel; @@ -71,25 +71,27 @@ SqlSortDirectionDescDefaultConverter.class, UnitTestConfiguration.class }) +@MockBean({ + JobService.class, + DatasetService.class, + IamService.class, + FileService.class, + AuthenticatedUserRequestFactory.class, + SnapshotBuilderService.class, +}) @WebMvcTest @Tag(Unit.TAG) class DatasetRequestValidatorTest { @Autowired private MockMvc mvc; - @MockBean private JobService jobService; - @MockBean private DatasetService datasetService; - @MockBean private IamService iamService; - @MockBean private FileService fileService; - @MockBean private AuthenticatedUserRequestFactory authenticatedUserRequestFactory; - @MockBean private SnapshotBuilderService snapshotBuilderService; @MockBean private IngestRequestValidator ingestRequestValidator; @MockBean private AssetModelValidator assetModelValidator; @MockBean private DatasetSchemaUpdateValidator datasetSchemaUpdateValidator; @MockBean private DataDeletionRequestValidator dataDeletionRequestValidator; @BeforeEach - void setup() throws Exception { + void setup() { when(ingestRequestValidator.supports(any())).thenReturn(true); when(dataDeletionRequestValidator.supports(any())).thenReturn(true); when(assetModelValidator.supports(any())).thenReturn(true); diff --git a/src/test/java/bio/terra/service/snapshot/SnapshotRequestValidatorTest.java b/src/test/java/bio/terra/service/snapshot/SnapshotRequestValidatorTest.java index a7fbbd3296..cefab1a5ec 100644 --- a/src/test/java/bio/terra/service/snapshot/SnapshotRequestValidatorTest.java +++ b/src/test/java/bio/terra/service/snapshot/SnapshotRequestValidatorTest.java @@ -6,11 +6,11 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import bio.terra.app.configuration.ApplicationConfiguration; +import bio.terra.app.configuration.UnitTestConfiguration; import bio.terra.app.controller.ApiValidationExceptionHandler; import bio.terra.app.controller.GlobalExceptionHandler; import bio.terra.app.controller.SnapshotsApiController; import bio.terra.common.category.Unit; -import bio.terra.common.fixtures.UnitTestConfiguration; import bio.terra.common.iam.AuthenticatedUserRequestFactory; import bio.terra.model.JobModel; import bio.terra.service.auth.iam.IamService; @@ -39,17 +39,19 @@ GlobalExceptionHandler.class, UnitTestConfiguration.class }) +@MockBean({ + SnapshotService.class, + IamService.class, + FileService.class, + ApplicationConfiguration.class, + AuthenticatedUserRequestFactory.class, + SnapshotBuilderService.class +}) @WebMvcTest @Tag(Unit.TAG) class SnapshotRequestValidatorTest { @Autowired private MockMvc mvc; @MockBean private JobService jobService; - @MockBean private SnapshotService snapshotService; - @MockBean private IamService iamService; - @MockBean private FileService fileService; - @MockBean private ApplicationConfiguration applicationConfiguration; - @MockBean private AuthenticatedUserRequestFactory authenticatedUserRequestFactory; - @MockBean private SnapshotBuilderService snapshotBuilderService; @MockBean private IngestRequestValidator ingestRequestValidator; @MockBean private AssetModelValidator assetModelValidator; From f8a73847f80d82df380994b3208000b77ba626be Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Tue, 11 Jun 2024 12:20:59 -0400 Subject: [PATCH 4/9] add Primary to objectmapper bean to fix an issue with spring-hateoas in tests --- .../bio/terra/app/configuration/ApplicationConfiguration.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/bio/terra/app/configuration/ApplicationConfiguration.java b/src/main/java/bio/terra/app/configuration/ApplicationConfiguration.java index 4aabd298bd..c3d313eff4 100644 --- a/src/main/java/bio/terra/app/configuration/ApplicationConfiguration.java +++ b/src/main/java/bio/terra/app/configuration/ApplicationConfiguration.java @@ -385,6 +385,8 @@ public NamedParameterJdbcTemplate synapseJdbcTemplate( return new NamedParameterJdbcTemplate(ds); } + // Use Primary to fix an issue with unqualified ObjectMapper injections in spring-hateoas. + @Primary @Bean("objectMapper") public ObjectMapper objectMapper() { return new ObjectMapper() From 75bd59e507ab0b241c42093a8442682c0162debf Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Tue, 11 Jun 2024 14:09:21 -0400 Subject: [PATCH 5/9] revert toList() change, it breaks containsInAnyOrder --- ...ataRepositoryServiceApiControllerTest.java | 54 +++++++++++++------ 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java b/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java index e21ed859a4..cf31e3b68f 100644 --- a/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java +++ b/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java @@ -2,6 +2,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; +import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.linkTo; +import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.methodOn; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.options; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -20,6 +22,7 @@ import bio.terra.model.DRSPassportRequestModel; import bio.terra.service.filedata.DrsService; import bio.terra.service.filedata.exception.DrsObjectNotFoundException; +import java.net.URI; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -35,12 +38,9 @@ @ContextConfiguration(classes = {DataRepositoryServiceApiController.class}) @Tag(Unit.TAG) @WebMvcTest +@MockBean(ApplicationConfiguration.class) class DataRepositoryServiceApiControllerTest { - private static final String GET_DRS_OBJECT_ENDPOINT = "/ga4gh/drs/v1/objects/{object_id}"; - private static final String GET_DRS_OBJECT_ACCESS_ENDPOINT = - "/ga4gh/drs/v1/objects/{object_id}/access/{access_id}"; - // Test fixtures private static final String DRS_ID = "foo"; private static final String DRS_ACCESS_ID = "bar"; @@ -54,7 +54,6 @@ class DataRepositoryServiceApiControllerTest { @Autowired private MockMvc mvc; - @MockBean private ApplicationConfiguration applicationConfiguration; @MockBean private DrsService drsService; @MockBean private AuthenticatedUserRequestFactory authenticatedUserRequestFactory; @@ -66,45 +65,68 @@ void setUp() { when(authenticatedUserRequestFactory.from(any())).thenReturn(TEST_USER); } + private static URI createGetUri() { + return linkTo(methodOn(DataRepositoryServiceApiController.class).getObject(DRS_ID, false)) + .toUri(); + } + + private static URI createAccessUri() { + return linkTo( + methodOn(DataRepositoryServiceApiController.class) + .getAccessURL(DRS_ID, DRS_ACCESS_ID, null)) + .toUri(); + } + @Test void testUnknownDrsIdWithGetFlow() throws Exception { when(drsService.lookupObjectByDrsId(TEST_USER, DRS_ID, false)) .thenThrow(DrsObjectNotFoundException.class); - mvc.perform(get(GET_DRS_OBJECT_ENDPOINT, DRS_ID)).andExpect(status().isNotFound()); + mvc.perform(get(createGetUri())).andExpect(status().isNotFound()); when(drsService.getAccessUrlForObjectId(TEST_USER, DRS_ID, DRS_ACCESS_ID, null)) .thenThrow(DrsObjectNotFoundException.class); - mvc.perform(get(GET_DRS_OBJECT_ACCESS_ENDPOINT, DRS_ID, DRS_ACCESS_ID)) - .andExpect(status().isNotFound()); + mvc.perform(get(createAccessUri())).andExpect(status().isNotFound()); } @Test void testKnownDrsIdWithGetFlow() throws Exception { when(drsService.lookupObjectByDrsId(TEST_USER, DRS_ID, false)).thenReturn(DRS_OBJECT); - mvc.perform(get(GET_DRS_OBJECT_ENDPOINT, DRS_ID)) + mvc.perform(get(createGetUri())) .andExpect(status().isOk()) .andExpect(jsonPath("$.id").value(DRS_ID)); when(drsService.getAccessUrlForObjectId(TEST_USER, DRS_ID, DRS_ACCESS_ID, null)) .thenReturn(DRS_ACCESS_URL_OBJECT); - mvc.perform(get(GET_DRS_OBJECT_ACCESS_ENDPOINT, DRS_ID, DRS_ACCESS_ID)) + mvc.perform(get(createAccessUri())) .andExpect(status().isOk()) .andExpect(jsonPath("$.url").value(DRS_ACCESS_URL)); } + private static URI createPostObjectUri() { + return linkTo(methodOn(DataRepositoryServiceApiController.class).postObject(DRS_ID, PASSPORT)) + .toUri(); + } + + private static URI createPostAccessUri() { + return linkTo( + methodOn(DataRepositoryServiceApiController.class) + .postAccessURL(DRS_ID, DRS_ACCESS_ID, PASSPORT, null)) + .toUri(); + } + @Test void testUnknownDrsIdWithPostFlow() throws Exception { when(drsService.lookupObjectByDrsIdPassport(DRS_ID, PASSPORT)) .thenThrow(DrsObjectNotFoundException.class); mvc.perform( - post(GET_DRS_OBJECT_ENDPOINT, DRS_ID) + post(createPostObjectUri()) .contentType(MediaType.APPLICATION_JSON) .content(TestUtils.mapToJson(PASSPORT))); when(drsService.postAccessUrlForObjectId(DRS_ID, DRS_ACCESS_ID, PASSPORT, null)) .thenThrow(DrsObjectNotFoundException.class); mvc.perform( - post(GET_DRS_OBJECT_ACCESS_ENDPOINT, DRS_ID, DRS_ACCESS_ID) + post(createPostAccessUri()) .contentType(MediaType.APPLICATION_JSON) .content(TestUtils.mapToJson(PASSPORT))) .andExpect(status().isNotFound()); @@ -114,7 +136,7 @@ void testUnknownDrsIdWithPostFlow() throws Exception { void testKnownDrsIdWithPostFlow() throws Exception { when(drsService.lookupObjectByDrsIdPassport(DRS_ID, PASSPORT)).thenReturn(DRS_OBJECT); mvc.perform( - post(GET_DRS_OBJECT_ENDPOINT, DRS_ID) + post(createPostObjectUri()) .contentType(MediaType.APPLICATION_JSON) .content(TestUtils.mapToJson(PASSPORT))) .andExpect(status().isOk()) @@ -123,7 +145,7 @@ void testKnownDrsIdWithPostFlow() throws Exception { when(drsService.postAccessUrlForObjectId(DRS_ID, DRS_ACCESS_ID, PASSPORT, null)) .thenReturn(DRS_ACCESS_URL_OBJECT); mvc.perform( - post(GET_DRS_OBJECT_ACCESS_ENDPOINT, DRS_ID, DRS_ACCESS_ID) + post(createPostAccessUri()) .contentType(MediaType.APPLICATION_JSON) .content(TestUtils.mapToJson(PASSPORT))) .andExpect(status().isOk()) @@ -134,13 +156,13 @@ void testKnownDrsIdWithPostFlow() throws Exception { void testUnknownDrsIdWithOptionsFlow() throws Exception { when(drsService.lookupAuthorizationsByDrsId(DRS_ID)) .thenThrow(DrsObjectNotFoundException.class); - mvc.perform(options(GET_DRS_OBJECT_ENDPOINT, DRS_ID)).andExpect(status().isNotFound()); + mvc.perform(options(createGetUri())).andExpect(status().isNotFound()); } @Test void testKnownDrsIdWithOptionsFlow() throws Exception { when(drsService.lookupAuthorizationsByDrsId(DRS_ID)).thenReturn(PASSPORT_AUTHORIZATIONS); - mvc.perform(options(GET_DRS_OBJECT_ENDPOINT, DRS_ID)) + mvc.perform(options(createGetUri())) .andExpect(status().isOk()) .andExpect(jsonPath("$.passport_auth_issuers[0]").value(PASSPORT_ISSUER)); } From 03daa1f14e6f5f7cdc42352a3f400c041b2d68e2 Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Tue, 11 Jun 2024 14:09:42 -0400 Subject: [PATCH 6/9] revert toList() change, it breaks containsInAnyOrder --- .../bio/terra/app/controller/SnapshotValidationTest.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java b/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java index 7ffec189a4..a7532cab9f 100644 --- a/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java +++ b/src/test/java/bio/terra/app/controller/SnapshotValidationTest.java @@ -35,8 +35,10 @@ import java.util.Collections; import java.util.List; import java.util.UUID; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.commons.lang3.StringUtils; +import org.hamcrest.Matcher; import org.hamcrest.Matchers; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; @@ -321,8 +323,10 @@ private void checkValidationErrorModel(ErrorModel errorModel, String[] messageCo * * We check to see if the code is wrapped in quotes to prevent matching on substrings. */ - var expectedMatches = - Arrays.stream(messageCodes).map(code -> containsString("'" + code + "'")).toList(); + List> expectedMatches = + Arrays.stream(messageCodes) + .map(code -> containsString("'" + code + "'")) + .collect(Collectors.toList()); assertThat("Detail codes are right", details, containsInAnyOrder(expectedMatches)); } } From 8a28cf826579b0d0b44ec6c70c51e2592faa743a Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Wed, 12 Jun 2024 09:33:13 -0400 Subject: [PATCH 7/9] back up incorrect change --- .../terra/service/snapshot/SnapshotRequestValidatorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/bio/terra/service/snapshot/SnapshotRequestValidatorTest.java b/src/test/java/bio/terra/service/snapshot/SnapshotRequestValidatorTest.java index 2270be27c0..7e8ab0c085 100644 --- a/src/test/java/bio/terra/service/snapshot/SnapshotRequestValidatorTest.java +++ b/src/test/java/bio/terra/service/snapshot/SnapshotRequestValidatorTest.java @@ -42,7 +42,6 @@ UnitTestConfiguration.class }) @MockBean({ - SnapshotService.class, IamService.class, FileService.class, ApplicationConfiguration.class, @@ -54,6 +53,7 @@ class SnapshotRequestValidatorTest { @Autowired private MockMvc mvc; @MockBean private JobService jobService; + @MockBean private SnapshotService snapshotService; @MockBean private IngestRequestValidator ingestRequestValidator; @MockBean private AssetModelValidator assetModelValidator; From aa8e72923de1d232cddf800bfd686fd4b9996cc6 Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Wed, 10 Jul 2024 14:55:35 -0400 Subject: [PATCH 8/9] create helper methods --- ...ataRepositoryServiceApiControllerTest.java | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java b/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java index cf31e3b68f..e9baaeab2e 100644 --- a/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java +++ b/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java @@ -30,6 +30,7 @@ import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.web.servlet.MockMvc; @@ -65,16 +66,20 @@ void setUp() { when(authenticatedUserRequestFactory.from(any())).thenReturn(TEST_USER); } + private static URI createUri(ResponseEntity object) { + return linkTo(object).toUri(); + } + + private static DataRepositoryServiceApiController getDrsApi() { + return methodOn(DataRepositoryServiceApiController.class); + } + private static URI createGetUri() { - return linkTo(methodOn(DataRepositoryServiceApiController.class).getObject(DRS_ID, false)) - .toUri(); + return createUri(getDrsApi().getObject(DRS_ID, false)); } private static URI createAccessUri() { - return linkTo( - methodOn(DataRepositoryServiceApiController.class) - .getAccessURL(DRS_ID, DRS_ACCESS_ID, null)) - .toUri(); + return createUri(getDrsApi().getAccessURL(DRS_ID, DRS_ACCESS_ID, null)); } @Test @@ -103,15 +108,11 @@ void testKnownDrsIdWithGetFlow() throws Exception { } private static URI createPostObjectUri() { - return linkTo(methodOn(DataRepositoryServiceApiController.class).postObject(DRS_ID, PASSPORT)) - .toUri(); + return createUri(getDrsApi().postObject(DRS_ID, PASSPORT)); } private static URI createPostAccessUri() { - return linkTo( - methodOn(DataRepositoryServiceApiController.class) - .postAccessURL(DRS_ID, DRS_ACCESS_ID, PASSPORT, null)) - .toUri(); + return createUri(getDrsApi().postAccessURL(DRS_ID, DRS_ACCESS_ID, PASSPORT, null)); } @Test From 0e1bd798b19bd0e46c05b27cd9f161be7d7c9f9a Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Wed, 10 Jul 2024 15:21:47 -0400 Subject: [PATCH 9/9] convert test code with more complex cases --- ...ataRepositoryServiceApiControllerTest.java | 10 +- .../controller/DatasetsApiControllerTest.java | 111 ++++++++++-------- 2 files changed, 66 insertions(+), 55 deletions(-) diff --git a/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java b/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java index e9baaeab2e..9e156e1dbc 100644 --- a/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java +++ b/src/test/java/bio/terra/app/controller/DataRepositoryServiceApiControllerTest.java @@ -70,16 +70,16 @@ private static URI createUri(ResponseEntity object) { return linkTo(object).toUri(); } - private static DataRepositoryServiceApiController getDrsApi() { + private static DataRepositoryServiceApiController getApi() { return methodOn(DataRepositoryServiceApiController.class); } private static URI createGetUri() { - return createUri(getDrsApi().getObject(DRS_ID, false)); + return createUri(getApi().getObject(DRS_ID, false)); } private static URI createAccessUri() { - return createUri(getDrsApi().getAccessURL(DRS_ID, DRS_ACCESS_ID, null)); + return createUri(getApi().getAccessURL(DRS_ID, DRS_ACCESS_ID, null)); } @Test @@ -108,11 +108,11 @@ void testKnownDrsIdWithGetFlow() throws Exception { } private static URI createPostObjectUri() { - return createUri(getDrsApi().postObject(DRS_ID, PASSPORT)); + return createUri(getApi().postObject(DRS_ID, PASSPORT)); } private static URI createPostAccessUri() { - return createUri(getDrsApi().postAccessURL(DRS_ID, DRS_ACCESS_ID, PASSPORT, null)); + return createUri(getApi().postAccessURL(DRS_ID, DRS_ACCESS_ID, PASSPORT, null)); } @Test diff --git a/src/test/java/bio/terra/app/controller/DatasetsApiControllerTest.java b/src/test/java/bio/terra/app/controller/DatasetsApiControllerTest.java index baa1070a6d..1a7df54919 100644 --- a/src/test/java/bio/terra/app/controller/DatasetsApiControllerTest.java +++ b/src/test/java/bio/terra/app/controller/DatasetsApiControllerTest.java @@ -11,6 +11,8 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; +import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.linkTo; +import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.methodOn; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -57,6 +59,7 @@ import bio.terra.service.dataset.exception.DatasetNotFoundException; import bio.terra.service.filedata.FileService; import bio.terra.service.job.JobService; +import java.net.URI; import java.util.List; import java.util.Set; import java.util.UUID; @@ -71,6 +74,7 @@ import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.web.servlet.MockMvc; @@ -80,13 +84,13 @@ @ContextConfiguration(classes = {DatasetsApiController.class, GlobalExceptionHandler.class}) @Tag(Unit.TAG) @WebMvcTest +@MockBean(FileService.class) class DatasetsApiControllerTest { @Autowired private MockMvc mvc; @MockBean private JobService jobService; @MockBean private DatasetRequestValidator datasetRequestValidator; @MockBean private DatasetService datasetService; @MockBean private IamService iamService; - @MockBean private FileService fileService; @MockBean private AuthenticatedUserRequestFactory authenticatedUserRequestFactory; @MockBean private AssetModelValidator assetModelValidator; @MockBean private IngestRequestValidator ingestRequestValidator; @@ -95,16 +99,8 @@ class DatasetsApiControllerTest { private static final AuthenticatedUserRequest TEST_USER = AuthenticationFixtures.randomUserRequest(); - private static final String DATASET_ID_ENDPOINT = "/api/repository/v1/datasets/{id}"; - private static final String LOCK_DATASET_ENDPOINT = DATASET_ID_ENDPOINT + "/lock"; - private static final String UNLOCK_DATASET_ENDPOINT = DATASET_ID_ENDPOINT + "/unlock"; - private static final String DATASET_INGEST_ENDPOINT = DATASET_ID_ENDPOINT + "/ingest"; private static final DatasetRequestAccessIncludeModel INCLUDE = DatasetRequestAccessIncludeModel.NONE; - private static final String QUERY_DATA_ENDPOINT = DATASET_ID_ENDPOINT + "/data/{table}"; - - private static final String QUERY_COLUMN_STATISTICS_ENDPOINT = - QUERY_DATA_ENDPOINT + "/statistics/{column}"; private static final SqlSortDirectionAscDefault DIRECTION = SqlSortDirectionAscDefault.ASC; private static final UUID DATASET_ID = UUID.randomUUID(); @@ -128,6 +124,18 @@ void setUp() { .path("/path/to/controlfile.json"); } + private static URI createUri(ResponseEntity object) { + return linkTo(object).toUri(); + } + + private static DatasetsApiController getApi() { + return methodOn(DatasetsApiController.class); + } + + private static MockHttpServletRequestBuilder createGetRequest() { + return get(createUri(getApi().retrieveDataset(DATASET_ID, List.of(INCLUDE)))); + } + @Test void testRetrieveDataset() throws Exception { DatasetModel expected = new DatasetModel().id(DATASET_ID); @@ -135,8 +143,7 @@ void testRetrieveDataset() throws Exception { .thenReturn(expected); String actualJson = - mvc.perform( - get(DATASET_ID_ENDPOINT, DATASET_ID).queryParam("include", String.valueOf(INCLUDE))) + mvc.perform(createGetRequest()) .andExpect(status().isOk()) .andReturn() .getResponse() @@ -151,8 +158,7 @@ void testRetrieveDataset() throws Exception { @Test void testRetrieveDatasetNotFound() throws Exception { mockNotFound(); - mvc.perform(get(DATASET_ID_ENDPOINT, DATASET_ID).queryParam("include", String.valueOf(INCLUDE))) - .andExpect(status().isNotFound()); + mvc.perform(createGetRequest()).andExpect(status().isNotFound()); verifyNoInteractions(iamService); } @@ -164,8 +170,7 @@ void testRetrieveDatasetForbidden() throws Exception { .verifyAuthorizations( TEST_USER, IamResourceType.DATASET, DATASET_ID.toString(), List.of(iamAction)); - mvc.perform(get(DATASET_ID_ENDPOINT, DATASET_ID).queryParam("include", String.valueOf(INCLUDE))) - .andExpect(status().isForbidden()); + mvc.perform(createGetRequest()).andExpect(status().isForbidden()); verifyAuthorizationsCall(List.of(iamAction)); } @@ -175,6 +180,12 @@ private void mockPatchDatasetIamActions() { .thenReturn(DATASET_PATCH_ACTIONS); } + private static MockHttpServletRequestBuilder createPatchRequest() { + return patch(createUri(getApi().patchDataset(DATASET_ID, DATASET_PATCH_REQUEST))) + .contentType(MediaType.APPLICATION_JSON) + .content(TestUtils.mapToJson(DATASET_PATCH_REQUEST)); + } + @Test void patchDataset() throws Exception { mockValidators(); @@ -186,10 +197,7 @@ void patchDataset() throws Exception { .thenReturn(patchedDatasetSummary); String actualJson = - mvc.perform( - patch(DATASET_ID_ENDPOINT, DATASET_ID) - .contentType(MediaType.APPLICATION_JSON) - .content(TestUtils.mapToJson(DATASET_PATCH_REQUEST))) + mvc.perform(createPatchRequest()) .andExpect(status().isOk()) .andReturn() .getResponse() @@ -208,11 +216,7 @@ void patchDataset_notFound() throws Exception { mockPatchDatasetIamActions(); mockNotFound(); - mvc.perform( - patch(DATASET_ID_ENDPOINT, DATASET_ID) - .contentType(MediaType.APPLICATION_JSON) - .content(TestUtils.mapToJson(DATASET_PATCH_REQUEST))) - .andExpect(status().isNotFound()); + mvc.perform(createPatchRequest()).andExpect(status().isNotFound()); verifyNoInteractions(iamService); verify(datasetService, never()).patch(DATASET_ID, DATASET_PATCH_REQUEST, TEST_USER); @@ -227,11 +231,7 @@ void patchDataset_forbidden() throws Exception { .verifyAuthorizations( TEST_USER, IamResourceType.DATASET, DATASET_ID.toString(), DATASET_PATCH_ACTIONS); - mvc.perform( - patch(DATASET_ID_ENDPOINT, DATASET_ID) - .contentType(MediaType.APPLICATION_JSON) - .content(TestUtils.mapToJson(DATASET_PATCH_REQUEST))) - .andExpect(status().isForbidden()); + mvc.perform(createPatchRequest()).andExpect(status().isForbidden()); verify(datasetService, never()).patch(DATASET_ID, DATASET_PATCH_REQUEST, TEST_USER); } @@ -275,13 +275,22 @@ private static Stream testQueryDatasetDataById() { private static Stream testQueryDatasetColumnStatistics() { return Stream.of( arguments( - post(QUERY_COLUMN_STATISTICS_ENDPOINT, DATASET_ID, "good_table", "good_column") + post(createUri( + getApi() + .queryDatasetColumnStatisticsById( + DATASET_ID, + "good_table", + "good_column", + new QueryColumnStatisticsRequestModel()))) .contentType(MediaType.APPLICATION_JSON) .content( TestUtils.mapToJson(new QueryColumnStatisticsRequestModel().filter(FILTER)))), arguments( - get(QUERY_COLUMN_STATISTICS_ENDPOINT, DATASET_ID, "good_table", "good_column") - .queryParam("filter", FILTER))); + get( + createUri( + getApi() + .lookupDatasetColumnStatisticsById( + DATASET_ID, "good_table", "good_column", FILTER))))); } @ParameterizedTest @@ -434,7 +443,10 @@ private void mockValidators() { private static MockHttpServletRequestBuilder postQueryDataRequest( String tableName, String columnName) { - return post(QUERY_DATA_ENDPOINT, DATASET_ID, tableName) + URI uri = + createUri( + getApi().queryDatasetDataById(DATASET_ID, tableName, new QueryDataRequestModel())); + return post(uri) .contentType(MediaType.APPLICATION_JSON) .content( TestUtils.mapToJson( @@ -447,7 +459,10 @@ private static MockHttpServletRequestBuilder postQueryDataRequest( } private static MockHttpServletRequestBuilder getRequest(String tableName, String columnName) { - return get(QUERY_DATA_ENDPOINT, DATASET_ID, tableName) + URI uri = + createUri( + getApi().queryDatasetDataById(DATASET_ID, tableName, new QueryDataRequestModel())); + return get(uri) .queryParam("limit", String.valueOf(LIMIT)) .queryParam("offset", String.valueOf(OFFSET)) .queryParam("sort", columnName) @@ -462,7 +477,7 @@ void lockDataset() throws Exception { mockValidators(); var response = - mvc.perform(put(LOCK_DATASET_ENDPOINT, DATASET_ID)) + mvc.perform(put(createUri(getApi().lockDataset(DATASET_ID)))) .andExpect(status().is2xxSuccessful()) .andReturn() .getResponse() @@ -485,7 +500,7 @@ void unlockDataset() throws Exception { var response = mvc.perform( - put(UNLOCK_DATASET_ENDPOINT, DATASET_ID) + put(createUri(getApi().unlockDataset(DATASET_ID, unlockRequest))) .contentType(MediaType.APPLICATION_JSON) .content(TestUtils.mapToJson(unlockRequest))) .andExpect(status().is2xxSuccessful()) @@ -498,17 +513,19 @@ void unlockDataset() throws Exception { verify(datasetService).manualUnlock(TEST_USER, DATASET_ID, unlockRequest); } + private MockHttpServletRequestBuilder createIngestDatasetRequest() { + return post(createUri(getApi().ingestDataset(DATASET_ID, ingestRequestModel))) + .contentType(MediaType.APPLICATION_JSON) + .content(TestUtils.mapToJson(ingestRequestModel)); + } + @Test void ingestDataset_forbidden() throws Exception { IamAction iamAction = IamAction.INGEST_DATA; mockForbidden(iamAction); mockValidators(); - mvc.perform( - post(DATASET_INGEST_ENDPOINT, DATASET_ID) - .contentType(MediaType.APPLICATION_JSON) - .content(TestUtils.mapToJson(ingestRequestModel))) - .andExpect(status().isForbidden()); + mvc.perform(createIngestDatasetRequest()).andExpect(status().isForbidden()); verifyAuthorizationCall(iamAction); } @@ -541,10 +558,7 @@ void ingestDataset_updateStrategy_supported( when(jobService.retrieveJob(jobId, TEST_USER)).thenReturn(expectedJob); String actualJson = - mvc.perform( - post(DATASET_INGEST_ENDPOINT, DATASET_ID) - .contentType(MediaType.APPLICATION_JSON) - .content(TestUtils.mapToJson(ingestRequestModel))) + mvc.perform(createIngestDatasetRequest()) .andExpect(status().isAccepted()) .andReturn() .getResponse() @@ -569,13 +583,10 @@ void ingestDataset_updateStrategy_unsupported( mockValidators(); when(datasetService.retrieveDatasetSummary(DATASET_ID)) .thenReturn(new DatasetSummaryModel().cloudPlatform(platform)); + ingestRequestModel.setUpdateStrategy(updateStrategy); String actualJson = - mvc.perform( - post(DATASET_INGEST_ENDPOINT, DATASET_ID) - .contentType(MediaType.APPLICATION_JSON) - .content( - TestUtils.mapToJson(ingestRequestModel.updateStrategy(updateStrategy)))) + mvc.perform(createIngestDatasetRequest()) .andExpect(status().isBadRequest()) .andReturn() .getResponse()