diff --git a/src/main/java/bio/terra/service/snapshot/flight/SnapshotWorkingMapKeys.java b/src/main/java/bio/terra/service/snapshot/flight/SnapshotWorkingMapKeys.java index 3a2be898d6..1bc6627ddc 100644 --- a/src/main/java/bio/terra/service/snapshot/flight/SnapshotWorkingMapKeys.java +++ b/src/main/java/bio/terra/service/snapshot/flight/SnapshotWorkingMapKeys.java @@ -14,7 +14,6 @@ private SnapshotWorkingMapKeys() {} public static final String SNAPSHOT_EXISTS = "snapshotExists"; public static final String SNAPSHOT_HAS_GOOGLE_PROJECT = "snapshotHasGoogleProject"; public static final String SNAPSHOT_HAS_AZURE_STORAGE_ACCOUNT = "snapshotHasStorageAccount"; - public static final String SNAPSHOT_AUTH_DOMAIN_GROUPS = "snapshotAuthDomainGroups"; public static final String DATASET_EXISTS = "datasetExists"; public static final String STORAGE_ACCOUNT_RESOURCE_NAME = "storageAccountResourceName"; public static final String STORAGE_ACCOUNT_RESOURCE_TLC = "storageAccountResourceTlc"; diff --git a/src/main/java/bio/terra/service/snapshot/flight/create/AddCreatedInfoToSnapshotRequestStep.java b/src/main/java/bio/terra/service/snapshot/flight/create/AddCreatedInfoToSnapshotRequestStep.java new file mode 100644 index 0000000000..cac3fb8c65 --- /dev/null +++ b/src/main/java/bio/terra/service/snapshot/flight/create/AddCreatedInfoToSnapshotRequestStep.java @@ -0,0 +1,45 @@ +package bio.terra.service.snapshot.flight.create; + +import bio.terra.service.snapshot.flight.SnapshotWorkingMapKeys; +import bio.terra.service.snapshotbuilder.SnapshotRequestDao; +import bio.terra.stairway.FlightContext; +import bio.terra.stairway.Step; +import bio.terra.stairway.StepResult; +import bio.terra.stairway.exception.RetryException; +import java.util.UUID; + +public class AddCreatedInfoToSnapshotRequestStep implements Step { + private final SnapshotRequestDao snapshotRequestDao; + private final UUID snapshotRequestId; + private final UUID createdSnapshotId; + private final String samGroupCreatedBy; + + public AddCreatedInfoToSnapshotRequestStep( + SnapshotRequestDao snapshotRequestDao, + UUID snapshotRequestId, + UUID snapshotId, + String samGroupCreatedBy) { + this.snapshotRequestDao = snapshotRequestDao; + this.snapshotRequestId = snapshotRequestId; + this.createdSnapshotId = snapshotId; + this.samGroupCreatedBy = samGroupCreatedBy; + } + + @Override + public StepResult doStep(FlightContext context) throws InterruptedException, RetryException { + var workingMap = context.getWorkingMap(); + String samGroupName = + workingMap.get(SnapshotWorkingMapKeys.SNAPSHOT_FIRECLOUD_GROUP_NAME, String.class); + + snapshotRequestDao.updateCreatedInfo( + snapshotRequestId, createdSnapshotId, samGroupName, samGroupCreatedBy); + return StepResult.getStepResultSuccess(); + } + + @Override + public StepResult undoStep(FlightContext context) throws InterruptedException { + // remove the written information if the flight fails + snapshotRequestDao.updateCreatedInfo(snapshotRequestId, null, null, null); + return StepResult.getStepResultSuccess(); + } +} diff --git a/src/main/java/bio/terra/service/snapshot/flight/create/AddCreatedSnapshotIdToSnapshotRequestStep.java b/src/main/java/bio/terra/service/snapshot/flight/create/AddCreatedSnapshotIdToSnapshotRequestStep.java deleted file mode 100644 index f0399b3b47..0000000000 --- a/src/main/java/bio/terra/service/snapshot/flight/create/AddCreatedSnapshotIdToSnapshotRequestStep.java +++ /dev/null @@ -1,34 +0,0 @@ -package bio.terra.service.snapshot.flight.create; - -import bio.terra.service.snapshotbuilder.SnapshotRequestDao; -import bio.terra.stairway.FlightContext; -import bio.terra.stairway.Step; -import bio.terra.stairway.StepResult; -import bio.terra.stairway.exception.RetryException; -import java.util.UUID; - -public class AddCreatedSnapshotIdToSnapshotRequestStep implements Step { - private final SnapshotRequestDao snapshotRequestDao; - private final UUID snapshotRequestId; - private final UUID snapshotId; - - public AddCreatedSnapshotIdToSnapshotRequestStep( - SnapshotRequestDao snapshotRequestDao, UUID snapshotRequestId, UUID snapshotId) { - this.snapshotRequestDao = snapshotRequestDao; - this.snapshotRequestId = snapshotRequestId; - this.snapshotId = snapshotId; - } - - @Override - public StepResult doStep(FlightContext context) throws InterruptedException, RetryException { - snapshotRequestDao.updateCreatedSnapshotId(snapshotRequestId, snapshotId); - return StepResult.getStepResultSuccess(); - } - - @Override - public StepResult undoStep(FlightContext context) throws InterruptedException { - // remove the created snapshot id if the flight fails - snapshotRequestDao.updateCreatedSnapshotId(snapshotRequestId, null); - return StepResult.getStepResultSuccess(); - } -} diff --git a/src/main/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlight.java b/src/main/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlight.java index c890aa5c49..d9cd6add4b 100644 --- a/src/main/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlight.java +++ b/src/main/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlight.java @@ -434,10 +434,11 @@ public SnapshotCreateFlight(FlightMap inputParameters, Object applicationContext if (mode == SnapshotRequestContentsModel.ModeEnum.BYREQUESTID) { UUID snapshotRequestId = contents.getRequestIdSpec().getSnapshotRequestId(); - // Add created snapshot id to the snapshot request. + // On the snapshot access request, save information about the created snapshot and about + // the Sam group that was added as a DAC. addStep( - new AddCreatedSnapshotIdToSnapshotRequestStep( - snapshotRequestDao, snapshotRequestId, snapshotId)); + new AddCreatedInfoToSnapshotRequestStep( + snapshotRequestDao, snapshotRequestId, snapshotId, tdrServiceAccountEmail)); // Notify user that snapshot is ready to use. addStep( new NotifyUserOfSnapshotCreationStep( diff --git a/src/main/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotDeleteSamGroupStep.java b/src/main/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotDeleteSamGroupStep.java index 941ca1c3b9..ed9d69edbf 100644 --- a/src/main/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotDeleteSamGroupStep.java +++ b/src/main/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotDeleteSamGroupStep.java @@ -1,15 +1,14 @@ package bio.terra.service.snapshot.flight.delete; +import bio.terra.common.exception.NotFoundException; import bio.terra.service.auth.iam.IamService; import bio.terra.service.auth.iam.exception.IamNotFoundException; import bio.terra.service.job.DefaultUndoStep; -import bio.terra.service.snapshot.flight.SnapshotWorkingMapKeys; +import bio.terra.service.snapshotbuilder.SnapshotAccessRequestModel; +import bio.terra.service.snapshotbuilder.SnapshotRequestDao; import bio.terra.stairway.FlightContext; import bio.terra.stairway.StepResult; import bio.terra.stairway.exception.RetryException; -import com.fasterxml.jackson.core.type.TypeReference; -import java.util.List; -import java.util.Objects; import java.util.UUID; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -17,6 +16,7 @@ public class DeleteSnapshotDeleteSamGroupStep extends DefaultUndoStep { private static Logger logger = LoggerFactory.getLogger(DeleteSnapshotDeleteSamGroupStep.class); private final IamService iamService; + private final SnapshotRequestDao snapshotRequestDao; private final UUID snapshotId; /** @@ -27,28 +27,35 @@ public class DeleteSnapshotDeleteSamGroupStep extends DefaultUndoStep { * @param iamService * @param snapshotId */ - public DeleteSnapshotDeleteSamGroupStep(IamService iamService, UUID snapshotId) { + public DeleteSnapshotDeleteSamGroupStep( + IamService iamService, SnapshotRequestDao snapshotRequestDao, UUID snapshotId) { this.iamService = iamService; + this.snapshotRequestDao = snapshotRequestDao; this.snapshotId = snapshotId; } @Override public StepResult doStep(FlightContext context) throws InterruptedException, RetryException { - List authDomains = - context - .getWorkingMap() - .get(SnapshotWorkingMapKeys.SNAPSHOT_AUTH_DOMAIN_GROUPS, new TypeReference<>() {}); - // Only delete the Sam group if it matches the expected naming pattern - var expectedName = IamService.constructSamGroupName(snapshotId.toString()); - if (Objects.nonNull(authDomains) && authDomains.contains(expectedName)) { - try { - iamService.deleteGroup(expectedName); - } catch (IamNotFoundException ex) { - // if group does not exist, nothing to delete) - } catch (Exception ex) { - logger.error("Error deleting Sam group: {}", expectedName, ex); - } + // The request will only exist if the snapshot was created with byRequestId mode + // If it exists, the request will have the sam group name to be deleted for this snapshot + SnapshotAccessRequestModel request; + try { + request = snapshotRequestDao.getByCreatedSnapshotId(snapshotId); + } catch (NotFoundException ex) { + // If the request does not exist, nothing to delete + return StepResult.getStepResultSuccess(); } + + var samGroupName = request.samGroupName(); + try { + iamService.deleteGroup(samGroupName); + } catch (IamNotFoundException ex) { + // If group does not exist, nothing to delete + } catch (Exception ex) { + // If there is some other error, log and continue + logger.error("Error deleting Sam group: {}", samGroupName, ex); + } + return StepResult.getStepResultSuccess(); } } diff --git a/src/main/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotPopAndLockDatasetStep.java b/src/main/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotPopAndLockDatasetStep.java index 50a77ead3d..5929ae5e47 100644 --- a/src/main/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotPopAndLockDatasetStep.java +++ b/src/main/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotPopAndLockDatasetStep.java @@ -15,8 +15,6 @@ import bio.terra.stairway.Step; import bio.terra.stairway.StepResult; import bio.terra.stairway.StepStatus; -import java.util.ArrayList; -import java.util.List; import java.util.UUID; public class DeleteSnapshotPopAndLockDatasetStep implements Step { @@ -64,16 +62,6 @@ public StepResult doStep(FlightContext context) { return StepResult.getStepResultSuccess(); } - try { - List authDomains = - new ArrayList<>( - snapshotService.retrieveAuthDomains(snapshot.getId(), authenticatedUserRequest)); - map.put(SnapshotWorkingMapKeys.SNAPSHOT_AUTH_DOMAIN_GROUPS, authDomains); - } catch (Exception ex) { - // Do nothing if we can't retrieve the auth domains - // No Sam groups will be deleted - } - // Now we've confirmed the snapshot exists, let's check on the source dataset UUID datasetId = snapshot.getSourceDataset().getId(); map.put(DatasetWorkingMapKeys.DATASET_ID, datasetId); diff --git a/src/main/java/bio/terra/service/snapshot/flight/delete/SnapshotDeleteFlight.java b/src/main/java/bio/terra/service/snapshot/flight/delete/SnapshotDeleteFlight.java index 89a334b3d3..5d7d597912 100644 --- a/src/main/java/bio/terra/service/snapshot/flight/delete/SnapshotDeleteFlight.java +++ b/src/main/java/bio/terra/service/snapshot/flight/delete/SnapshotDeleteFlight.java @@ -27,6 +27,7 @@ import bio.terra.service.snapshot.flight.LockSnapshotStep; import bio.terra.service.snapshot.flight.UnlockSnapshotStep; import bio.terra.service.snapshotbuilder.SnapshotBuilderService; +import bio.terra.service.snapshotbuilder.SnapshotRequestDao; import bio.terra.service.tabulardata.google.bigquery.BigQuerySnapshotPdao; import bio.terra.stairway.Flight; import bio.terra.stairway.FlightMap; @@ -45,6 +46,7 @@ public SnapshotDeleteFlight(FlightMap inputParameters, Object applicationContext SnapshotService snapshotService = appContext.getBean(SnapshotService.class); SnapshotBuilderService snapshotBuilderService = appContext.getBean(SnapshotBuilderService.class); + SnapshotRequestDao snapshotRequestDao = appContext.getBean(SnapshotRequestDao.class); FireStoreDependencyDao dependencyDao = appContext.getBean(FireStoreDependencyDao.class); FireStoreDao fileDao = appContext.getBean(FireStoreDao.class); BigQuerySnapshotPdao bigQuerySnapshotPdao = appContext.getBean(BigQuerySnapshotPdao.class); @@ -113,7 +115,7 @@ public SnapshotDeleteFlight(FlightMap inputParameters, Object applicationContext // Now that we no longer have the resources in SAM, we can delete the underlying Sam group // that was created for snapshots byRequestId - addStep(new DeleteSnapshotDeleteSamGroupStep(iamClient, snapshotId)); + addStep(new DeleteSnapshotDeleteSamGroupStep(iamClient, snapshotRequestDao, snapshotId)); // Primary Data Deletion // Note: Must delete primary data before metadata; it relies on being able to retrieve the diff --git a/src/main/java/bio/terra/service/snapshotbuilder/SnapshotAccessRequestModel.java b/src/main/java/bio/terra/service/snapshotbuilder/SnapshotAccessRequestModel.java index 213ed143ea..d790277eae 100644 --- a/src/main/java/bio/terra/service/snapshotbuilder/SnapshotAccessRequestModel.java +++ b/src/main/java/bio/terra/service/snapshotbuilder/SnapshotAccessRequestModel.java @@ -28,7 +28,9 @@ public record SnapshotAccessRequestModel( Instant statusUpdatedDate, SnapshotAccessRequestStatus status, UUID createdSnapshotId, - String flightid) { + String flightid, + String samGroupName, + String samGroupCreatedByTerraId) { @VisibleForTesting static String generateSummaryForCriteria( @@ -137,6 +139,7 @@ public SnapshotAccessRequestResponse toApiResponse(SnapshotBuilderSettings setti .createdDate(createdDate != null ? createdDate.toString() : null) .statusUpdatedDate(statusUpdatedDate != null ? statusUpdatedDate.toString() : null) .createdSnapshotId(createdSnapshotId) - .summary(generateSummaryFromSnapshotSpecification(settings)); + .summary(generateSummaryFromSnapshotSpecification(settings)) + .authGroupName(samGroupName); } } diff --git a/src/main/java/bio/terra/service/snapshotbuilder/SnapshotRequestDao.java b/src/main/java/bio/terra/service/snapshotbuilder/SnapshotRequestDao.java index e70c81da2b..9b2606682d 100644 --- a/src/main/java/bio/terra/service/snapshotbuilder/SnapshotRequestDao.java +++ b/src/main/java/bio/terra/service/snapshotbuilder/SnapshotRequestDao.java @@ -37,7 +37,9 @@ public class SnapshotRequestDao { private static final String STATUS_UPDATED_DATE = "status_updated_date"; private static final String STATUS = "status"; private static final String FLIGHT_ID = "flightid"; - private static final String CREATED_SNAPSHOT_ID = "created_snapshot_id"; + public static final String CREATED_SNAPSHOT_ID = "created_snapshot_id"; + public static final String SAM_GROUP_NAME = "sam_group_name"; + public static final String SAM_GROUP_CREATED_BY = "sam_group_created_by"; private static final String AUTHORIZED_RESOURCES = "authorized_resources"; private static final String NOT_FOUND_MESSAGE = "Snapshot Access Request with given id does not exist."; @@ -55,7 +57,9 @@ public class SnapshotRequestDao { DaoUtils.getInstant(rs, STATUS_UPDATED_DATE), SnapshotAccessRequestStatus.valueOf(rs.getString(STATUS)), rs.getObject(CREATED_SNAPSHOT_ID, UUID.class), - rs.getString(FLIGHT_ID)); + rs.getString(FLIGHT_ID), + rs.getString(SAM_GROUP_NAME), + rs.getString(SAM_GROUP_CREATED_BY)); public SnapshotRequestDao( NamedParameterJdbcTemplate jdbcTemplate, @@ -89,6 +93,24 @@ public SnapshotAccessRequestModel getById(UUID requestId) { } } + /** + * Get the Snapshot Request associated with the given created snapshot id. + * + * @param createdSnapshotId associated with one snapshot request. + * @return the specified snapshot request or exception if it does not exist. + */ + public SnapshotAccessRequestModel getByCreatedSnapshotId(UUID createdSnapshotId) { + String sql = "SELECT * FROM snapshot_request WHERE created_snapshot_id = :created_snapshot_id"; + MapSqlParameterSource params = + new MapSqlParameterSource().addValue(CREATED_SNAPSHOT_ID, createdSnapshotId); + try { + return jdbcTemplate.queryForObject(sql, params, modelMapper); + } catch (EmptyResultDataAccessException ex) { + throw new NotFoundException( + "No snapshot access requests found for given created snapshot id", ex); + } + } + /** * Return the list of Snapshot Requests associated with the given snapshot id. * @@ -198,16 +220,21 @@ public void updateFlightId(UUID requestId, String flightId) { } @Transactional(propagation = Propagation.REQUIRED, isolation = Isolation.SERIALIZABLE) - public void updateCreatedSnapshotId(UUID requestId, UUID snapshotId) { + public void updateCreatedInfo( + UUID requestId, UUID snapshotId, String samGroupName, String groupCreatedByEmail) { String sql = """ UPDATE snapshot_request SET - created_snapshot_id = :created_snapshot_id + created_snapshot_id = :created_snapshot_id, + sam_group_name = :sam_group_name, + sam_group_created_by = :sam_group_created_by WHERE id = :id """; MapSqlParameterSource params = new MapSqlParameterSource() .addValue(CREATED_SNAPSHOT_ID, snapshotId) + .addValue(SAM_GROUP_NAME, samGroupName) + .addValue(SAM_GROUP_CREATED_BY, groupCreatedByEmail) .addValue(ID, requestId); if (jdbcTemplate.update(sql, params) == 0) { throw new NotFoundException(NOT_FOUND_MESSAGE); diff --git a/src/main/resources/api/data-repository-openapi.yaml b/src/main/resources/api/data-repository-openapi.yaml index d074abb1a5..40d8568c76 100644 --- a/src/main/resources/api/data-repository-openapi.yaml +++ b/src/main/resources/api/data-repository-openapi.yaml @@ -7543,6 +7543,8 @@ components: $ref: '#/components/schemas/UniqueIdProperty' summary: type: string + authGroupName: + type: string SnapshotAccessRequestStatus: diff --git a/src/main/resources/db/changelog.xml b/src/main/resources/db/changelog.xml index 954ac58519..b3257b695c 100644 --- a/src/main/resources/db/changelog.xml +++ b/src/main/resources/db/changelog.xml @@ -88,4 +88,5 @@ + diff --git a/src/main/resources/db/changesets/20240816_addsamgrouptorequest.yaml b/src/main/resources/db/changesets/20240816_addsamgrouptorequest.yaml new file mode 100644 index 0000000000..48b64071c3 --- /dev/null +++ b/src/main/resources/db/changesets/20240816_addsamgrouptorequest.yaml @@ -0,0 +1,15 @@ +databaseChangeLog: + - changeSet: + id: add_sam_group_to_request + author: rjohanek + changes: + - addColumn: + tableName: snapshot_request + columns: + - column: + name: sam_group_name + type: varchar(100) + - column: + name: sam_group_created_by + type: varchar(256) + diff --git a/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java b/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java index c10efbba67..5d068c6e54 100644 --- a/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java +++ b/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java @@ -621,7 +621,7 @@ void validateForByRequestIdModeApproved() { SnapshotRequestContentsModel snapshotRequestContentsModel = makeByRequestIdContentsModel(snapshotAccessRequestId); SnapshotAccessRequestModel accessRequestResponse = - SnapshotBuilderTestData.createAccessRequest(); + SnapshotBuilderTestData.createAccessRequestModelApproved(); when(snapshotRequestDao.getById(snapshotAccessRequestId)).thenReturn(accessRequestResponse); assertDoesNotThrow(() -> service.validateForByRequestIdMode(snapshotRequestContentsModel)); @@ -639,7 +639,9 @@ static SnapshotAccessRequestModel createAccessRequestWithFlightid() { null, SnapshotAccessRequestStatus.APPROVED, null, - "flightId"); + "flightId", + null, + null); } @Test @@ -660,20 +662,9 @@ void validateForByRequestIdModeNotApproved() { UUID snapshotAccessRequestId = UUID.randomUUID(); SnapshotRequestContentsModel snapshotRequestContentsModel = makeByRequestIdContentsModel(snapshotAccessRequestId); - SnapshotAccessRequestModel accessRequestResponse = - new SnapshotAccessRequestModel( - null, - null, - null, - null, - null, - null, - null, - null, - SnapshotAccessRequestStatus.SUBMITTED, - null, - null); - when(snapshotRequestDao.getById(snapshotAccessRequestId)).thenReturn(accessRequestResponse); + SnapshotAccessRequestModel accessRequestModel = + SnapshotBuilderTestData.createSnapshotAccessRequestModel(UUID.randomUUID()); + when(snapshotRequestDao.getById(snapshotAccessRequestId)).thenReturn(accessRequestModel); assertThrows( ValidationException.class, @@ -698,6 +689,8 @@ void validateForByRequestIdModeAlreadyCreated() { null, SnapshotAccessRequestStatus.APPROVED, UUID.randomUUID(), + null, + null, null); when(snapshotRequestDao.getById(snapshotAccessRequestId)).thenReturn(accessRequestResponse); @@ -740,6 +733,8 @@ void validateForByRequestIdIdModeCreatedByEmail() { null, SnapshotAccessRequestStatus.APPROVED, null, + null, + null, null); when(snapshotRequestDao.getById(snapshotAccessRequestId)).thenReturn(accessRequestResponse); assertThrows( @@ -1139,7 +1134,7 @@ void testCreateSnapshotThrowsWhenDuosClientThrows() { @Test void testCreateSnapshotWithByRequestId() { SnapshotAccessRequestModel snapshotAccessRequest = - SnapshotBuilderTestData.createAccessRequest(); + SnapshotBuilderTestData.createAccessRequestModelApproved(); UUID snapshotAccessRequestId = snapshotAccessRequest.id(); SnapshotRequestContentsModel contentsModel = makeByRequestIdContentsModel(snapshotAccessRequestId); @@ -1400,6 +1395,8 @@ void makeSnapshotFromSnapshotRequestByRequestId() { null, null, null, + null, + null, null)); when(settingsDao.getBySnapshotId(sourceSnapshotId)) .thenReturn(SnapshotBuilderTestData.SETTINGS); @@ -1449,7 +1446,7 @@ void getSourceDatasetFromSnapshotRequestHandlesByRequestId() { new SnapshotRequestModel().contents(List.of(contentsModel)); SnapshotAccessRequestModel snapshotAccessRequest = - SnapshotBuilderTestData.createAccessRequest(); + SnapshotBuilderTestData.createAccessRequestModelApproved(); Dataset dataset = new Dataset().id(datasetId).name(DATASET_NAME); Snapshot snapshot = new Snapshot().snapshotSources(List.of(new SnapshotSource().dataset(dataset))); @@ -1506,7 +1503,7 @@ void getSnapshotNameForByRequest() { when(snapshotRequestDao.getById(uuid)) .thenReturn( new SnapshotAccessRequestModel( - uuid, " a$%", null, null, null, null, null, null, null, null, null)); + uuid, " a$%", null, null, null, null, null, null, null, null, null, null, null)); assertThat(service.getSnapshotName(snapshotRequestModel), is(expectedName)); } diff --git a/src/test/java/bio/terra/service/snapshot/flight/create/AddCreatedSnapshotIdToSnapshotRequestStepTest.java b/src/test/java/bio/terra/service/snapshot/flight/create/AddCreatedInfoToSnapshotRequestStepTest.java similarity index 53% rename from src/test/java/bio/terra/service/snapshot/flight/create/AddCreatedSnapshotIdToSnapshotRequestStepTest.java rename to src/test/java/bio/terra/service/snapshot/flight/create/AddCreatedInfoToSnapshotRequestStepTest.java index 7ffd84731c..6409102deb 100644 --- a/src/test/java/bio/terra/service/snapshot/flight/create/AddCreatedSnapshotIdToSnapshotRequestStepTest.java +++ b/src/test/java/bio/terra/service/snapshot/flight/create/AddCreatedInfoToSnapshotRequestStepTest.java @@ -2,9 +2,13 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import bio.terra.common.category.Unit; +import bio.terra.service.snapshot.flight.SnapshotWorkingMapKeys; import bio.terra.service.snapshotbuilder.SnapshotRequestDao; +import bio.terra.stairway.FlightContext; +import bio.terra.stairway.FlightMap; import bio.terra.stairway.StepResult; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; @@ -16,30 +20,39 @@ @ExtendWith(MockitoExtension.class) @Tag(Unit.TAG) -class AddCreatedSnapshotIdToSnapshotRequestStepTest { +class AddCreatedInfoToSnapshotRequestStepTest { @Mock private SnapshotRequestDao snapshotRequestDao; + @Mock private FlightContext context; private static final UUID SNAPSHOT_REQUEST_ID = UUID.randomUUID(); private static final UUID CREATED_SNAPSHOT_ID = UUID.randomUUID(); - private AddCreatedSnapshotIdToSnapshotRequestStep step; + private static final String SAM_GROUP_NAME = "samGroupName"; + private static final String SAM_GROUP_CREATED_BY = "tdr@serviceaccount.com"; + private AddCreatedInfoToSnapshotRequestStep step; + private FlightMap workingMap; @BeforeEach void beforeEach() { step = - new AddCreatedSnapshotIdToSnapshotRequestStep( - snapshotRequestDao, SNAPSHOT_REQUEST_ID, CREATED_SNAPSHOT_ID); + new AddCreatedInfoToSnapshotRequestStep( + snapshotRequestDao, SNAPSHOT_REQUEST_ID, CREATED_SNAPSHOT_ID, SAM_GROUP_CREATED_BY); + workingMap = new FlightMap(); } @Test void doStep() throws InterruptedException { - StepResult result = step.doStep(null); - verify(snapshotRequestDao).updateCreatedSnapshotId(SNAPSHOT_REQUEST_ID, CREATED_SNAPSHOT_ID); + workingMap.put(SnapshotWorkingMapKeys.SNAPSHOT_FIRECLOUD_GROUP_NAME, SAM_GROUP_NAME); + when(context.getWorkingMap()).thenReturn(workingMap); + StepResult result = step.doStep(context); + verify(snapshotRequestDao) + .updateCreatedInfo( + SNAPSHOT_REQUEST_ID, CREATED_SNAPSHOT_ID, SAM_GROUP_NAME, SAM_GROUP_CREATED_BY); assertEquals(StepResult.getStepResultSuccess(), result); } @Test void undoStep() throws InterruptedException { StepResult result = step.undoStep(null); - verify(snapshotRequestDao).updateCreatedSnapshotId(SNAPSHOT_REQUEST_ID, null); + verify(snapshotRequestDao).updateCreatedInfo(SNAPSHOT_REQUEST_ID, null, null, null); assertEquals(StepResult.getStepResultSuccess(), result); } } diff --git a/src/test/java/bio/terra/service/snapshot/flight/create/CreateSnapshotAddEmailsToSamGroupStepTest.java b/src/test/java/bio/terra/service/snapshot/flight/create/CreateSnapshotAddEmailsToSamGroupStepTest.java index 5734afda3c..5fb46cdac5 100644 --- a/src/test/java/bio/terra/service/snapshot/flight/create/CreateSnapshotAddEmailsToSamGroupStepTest.java +++ b/src/test/java/bio/terra/service/snapshot/flight/create/CreateSnapshotAddEmailsToSamGroupStepTest.java @@ -10,7 +10,7 @@ import bio.terra.service.auth.iam.IamRole; import bio.terra.service.auth.iam.IamService; import bio.terra.service.snapshot.flight.SnapshotWorkingMapKeys; -import bio.terra.service.snapshotbuilder.SnapshotAccessRequestModel; +import bio.terra.service.snapshotbuilder.SnapshotBuilderTestData; import bio.terra.service.snapshotbuilder.SnapshotRequestDao; import bio.terra.stairway.FlightContext; import bio.terra.stairway.FlightMap; @@ -50,11 +50,9 @@ void setUp() { @Test void doStep() throws InterruptedException { - var researcherEmail = "researcher@gmail.com"; + var request = SnapshotBuilderTestData.createSnapshotAccessRequestModel(UUID.randomUUID()); + var researcherEmail = request.createdBy(); var emailsToAdd = List.of(researcherEmail); - var request = - new SnapshotAccessRequestModel( - null, null, null, null, null, researcherEmail, null, null, null, null, null); when(snapshotRequestDao.getById(snapshotRequestId)).thenReturn(request); assertEquals(step.doStep(flightContext), StepResult.getStepResultSuccess()); verify(iamService) diff --git a/src/test/java/bio/terra/service/snapshot/flight/create/NotifyUserOfSnapshotCreationStepTest.java b/src/test/java/bio/terra/service/snapshot/flight/create/NotifyUserOfSnapshotCreationStepTest.java index d6a5d25489..970f961127 100644 --- a/src/test/java/bio/terra/service/snapshot/flight/create/NotifyUserOfSnapshotCreationStepTest.java +++ b/src/test/java/bio/terra/service/snapshot/flight/create/NotifyUserOfSnapshotCreationStepTest.java @@ -32,7 +32,7 @@ void doStep() throws InterruptedException { var step = new NotifyUserOfSnapshotCreationStep( snapshotBuilderService, snapshotRequestDao, iamService, id); - var request = SnapshotBuilderTestData.createAccessRequest(); + var request = SnapshotBuilderTestData.createAccessRequestModelApproved(); var user = new UserIdInfo().userSubjectId("subjectId"); when(snapshotRequestDao.getById(id)).thenReturn(request); when(iamService.getUserIds(request.createdBy())).thenReturn(user); diff --git a/src/test/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlightTest.java b/src/test/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlightTest.java index 9e880fc201..c01dd3e056 100644 --- a/src/test/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlightTest.java +++ b/src/test/java/bio/terra/service/snapshot/flight/create/SnapshotCreateFlightTest.java @@ -154,7 +154,7 @@ void testSnapshotCreateFlightByRequestId() { "CreateSnapshotSetResponseStep", "CreateSnapshotJournalEntryStep", "JournalRecordUpdateEntryStep", - "AddCreatedSnapshotIdToSnapshotRequestStep", + "AddCreatedInfoToSnapshotRequestStep", "NotifyUserOfSnapshotCreationStep")); } } diff --git a/src/test/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotDeleteSamGroupStepTest.java b/src/test/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotDeleteSamGroupStepTest.java index 00599123f7..c7d62a983d 100644 --- a/src/test/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotDeleteSamGroupStepTest.java +++ b/src/test/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotDeleteSamGroupStepTest.java @@ -8,13 +8,12 @@ import static org.mockito.Mockito.when; import bio.terra.common.category.Unit; +import bio.terra.common.exception.NotFoundException; import bio.terra.service.auth.iam.IamService; -import bio.terra.service.snapshot.flight.SnapshotWorkingMapKeys; +import bio.terra.service.snapshotbuilder.SnapshotAccessRequestModel; +import bio.terra.service.snapshotbuilder.SnapshotRequestDao; import bio.terra.stairway.FlightContext; -import bio.terra.stairway.FlightMap; import bio.terra.stairway.StepStatus; -import java.util.ArrayList; -import java.util.List; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; @@ -27,36 +26,33 @@ @Tag(Unit.TAG) class DeleteSnapshotDeleteSamGroupStepTest { @Mock IamService iamService; + @Mock SnapshotRequestDao snapshotRequestDao; private UUID snapshotId; @Mock private FlightContext flightContext; private DeleteSnapshotDeleteSamGroupStep step; + private static final String EXPECTED_NAME = "samGroupName"; + private static final SnapshotAccessRequestModel SNAPSHOT_ACCESS_REQUEST_MODEL = + new SnapshotAccessRequestModel( + null, null, null, null, null, null, null, null, null, null, null, EXPECTED_NAME, null); @BeforeEach void beforeEach() { snapshotId = UUID.randomUUID(); - step = new DeleteSnapshotDeleteSamGroupStep(iamService, snapshotId); + step = new DeleteSnapshotDeleteSamGroupStep(iamService, snapshotRequestDao, snapshotId); } @Test void doStep() throws InterruptedException { - var doNotDeleteGroupName = "group1"; - var expectedName = IamService.constructSamGroupName(snapshotId.toString()); - - var flightMap = new FlightMap(); - var groups = new ArrayList<>(List.of(doNotDeleteGroupName, expectedName)); - flightMap.put(SnapshotWorkingMapKeys.SNAPSHOT_AUTH_DOMAIN_GROUPS, groups); - when(flightContext.getWorkingMap()).thenReturn(flightMap); - + when(snapshotRequestDao.getByCreatedSnapshotId(snapshotId)) + .thenReturn(SNAPSHOT_ACCESS_REQUEST_MODEL); var result = step.doStep(flightContext); - verify(iamService).deleteGroup(expectedName); - verify(iamService, never()).deleteGroup(doNotDeleteGroupName); + verify(iamService).deleteGroup(EXPECTED_NAME); assertThat(result.getStepStatus(), equalTo(StepStatus.STEP_RESULT_SUCCESS)); } @Test - void doStepNullGroups() throws InterruptedException { - var flightMap = new FlightMap(); - when(flightContext.getWorkingMap()).thenReturn(flightMap); + void doStepSnapshotNotByRequestId() throws InterruptedException { + when(snapshotRequestDao.getByCreatedSnapshotId(snapshotId)).thenThrow(NotFoundException.class); var result = step.doStep(flightContext); verify(iamService, never()).deleteGroup(any()); assertThat(result.getStepStatus(), equalTo(StepStatus.STEP_RESULT_SUCCESS)); diff --git a/src/test/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotPopAndLockDatasetStepTest.java b/src/test/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotPopAndLockDatasetStepTest.java index a1172616a6..b98b0017b7 100644 --- a/src/test/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotPopAndLockDatasetStepTest.java +++ b/src/test/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotPopAndLockDatasetStepTest.java @@ -2,10 +2,11 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import bio.terra.common.category.Unit; @@ -13,15 +14,20 @@ import bio.terra.common.iam.AuthenticatedUserRequest; import bio.terra.service.dataset.Dataset; import bio.terra.service.dataset.DatasetService; +import bio.terra.service.dataset.exception.DatasetNotFoundException; +import bio.terra.service.dataset.flight.DatasetWorkingMapKeys; +import bio.terra.service.resourcemanagement.azure.AzureStorageAccountResource; +import bio.terra.service.resourcemanagement.google.GoogleProjectResource; import bio.terra.service.snapshot.Snapshot; import bio.terra.service.snapshot.SnapshotService; import bio.terra.service.snapshot.SnapshotSource; +import bio.terra.service.snapshot.exception.SnapshotNotFoundException; import bio.terra.service.snapshot.flight.SnapshotWorkingMapKeys; import bio.terra.stairway.FlightContext; import bio.terra.stairway.FlightMap; -import bio.terra.stairway.StepStatus; -import com.fasterxml.jackson.core.type.TypeReference; +import bio.terra.stairway.StepResult; import java.util.List; +import java.util.Objects; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; @@ -38,6 +44,7 @@ class DeleteSnapshotPopAndLockDatasetStepTest { @Mock DatasetService datasetService; private Snapshot snapshot; private UUID snapshotId; + private UUID datasetId; private static final AuthenticatedUserRequest TEST_USER = AuthenticationFixtures.randomUserRequest(); private final boolean sharedLock = true; @@ -47,44 +54,77 @@ class DeleteSnapshotPopAndLockDatasetStepTest { @BeforeEach void setUp() { snapshotId = UUID.randomUUID(); + datasetId = UUID.randomUUID(); snapshot = new Snapshot() .id(snapshotId) - .snapshotSources( - List.of(new SnapshotSource().dataset(new Dataset().id(UUID.randomUUID())))); + .snapshotSources(List.of(new SnapshotSource().dataset(new Dataset().id(datasetId)))) + .projectResource(new GoogleProjectResource().googleProjectId("projectId")) + .storageAccountResource(new AzureStorageAccountResource()); step = new DeleteSnapshotPopAndLockDatasetStep( snapshotId, snapshotService, datasetService, TEST_USER, sharedLock); } + // snapshot and dataset exist @Test - void setAuthDomainGroups() { + void doStep() { when(snapshotService.retrieve(snapshotId)).thenReturn(snapshot); - when(snapshotService.retrieveAuthDomains(eq(snapshotId), any())) - .thenReturn(List.of("group1", "group2")); var flightMap = new FlightMap(); when(flightContext.getWorkingMap()).thenReturn(flightMap); - step.doStep(flightContext); + assertThat( + "Step is successful", + step.doStep(flightContext), + equalTo(StepResult.getStepResultSuccess())); FlightMap map = flightContext.getWorkingMap(); - List authDomains = - map.get(SnapshotWorkingMapKeys.SNAPSHOT_AUTH_DOMAIN_GROUPS, new TypeReference<>() {}); + assertTrue(getBoolean(map, SnapshotWorkingMapKeys.SNAPSHOT_EXISTS)); + assertTrue(getBoolean(map, SnapshotWorkingMapKeys.SNAPSHOT_HAS_GOOGLE_PROJECT)); + assertTrue(getBoolean(map, SnapshotWorkingMapKeys.SNAPSHOT_HAS_AZURE_STORAGE_ACCOUNT)); + assertEquals(datasetId, map.get(DatasetWorkingMapKeys.DATASET_ID, UUID.class)); + verify(datasetService).lock(eq(datasetId), any(), eq(sharedLock)); + assertTrue(getBoolean(map, SnapshotWorkingMapKeys.DATASET_EXISTS)); + } + + // snapshot does not exist + @Test + void doStepSnapshotNotFound() { + when(snapshotService.retrieve(snapshotId)) + .thenThrow(new SnapshotNotFoundException("not found")); + var flightMap = new FlightMap(); + when(flightContext.getWorkingMap()).thenReturn(flightMap); assertThat( - "Auth domain list is populated", authDomains, containsInAnyOrder("group1", "group2")); + "Step is successful", + step.doStep(flightContext), + equalTo(StepResult.getStepResultSuccess())); + + FlightMap map = flightContext.getWorkingMap(); + assertFalse(getBoolean(map, SnapshotWorkingMapKeys.SNAPSHOT_EXISTS)); + assertFalse(getBoolean(map, SnapshotWorkingMapKeys.DATASET_EXISTS)); + assertFalse(getBoolean(map, SnapshotWorkingMapKeys.SNAPSHOT_HAS_GOOGLE_PROJECT)); + assertFalse(getBoolean(map, SnapshotWorkingMapKeys.SNAPSHOT_HAS_AZURE_STORAGE_ACCOUNT)); } + // dataset does not exist @Test - void noAuthDomainGroups() { + void doStepDatasetNotFound() { when(snapshotService.retrieve(snapshotId)).thenReturn(snapshot); - when(snapshotService.retrieveAuthDomains(eq(snapshotId), any())).thenReturn(null); var flightMap = new FlightMap(); when(flightContext.getWorkingMap()).thenReturn(flightMap); - var result = step.doStep(flightContext); + doThrow(new DatasetNotFoundException("not found")) + .when(datasetService) + .lock(eq(datasetId), any(), eq(sharedLock)); + assertThat( + "Step is successful", + step.doStep(flightContext), + equalTo(StepResult.getStepResultSuccess())); FlightMap map = flightContext.getWorkingMap(); - List authDomains = - map.get(SnapshotWorkingMapKeys.SNAPSHOT_AUTH_DOMAIN_GROUPS, new TypeReference<>() {}); - assertNull(authDomains); - assertThat(result.getStepStatus(), equalTo(StepStatus.STEP_RESULT_SUCCESS)); + assertTrue(getBoolean(map, SnapshotWorkingMapKeys.SNAPSHOT_EXISTS)); + assertFalse(getBoolean(map, SnapshotWorkingMapKeys.DATASET_EXISTS)); + } + + static boolean getBoolean(FlightMap map, String key) { + return Objects.requireNonNull(map.get(key, Boolean.class)); } } diff --git a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotAccessRequestModelTest.java b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotAccessRequestModelTest.java index 760943e7e1..44f8311dd5 100644 --- a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotAccessRequestModelTest.java +++ b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotAccessRequestModelTest.java @@ -140,7 +140,9 @@ private SnapshotAccessRequestModel generateSnapshotAccessRequestModel() { Instant.now(), SnapshotAccessRequestStatus.SUBMITTED, UUID.randomUUID(), - "flightid"); + "flightid", + "samGroupName", + "tdr@serviceaccount.com"); } private void compareModelAndResponseFields( @@ -159,5 +161,6 @@ private void compareModelAndResponseFields( assertThat(model.flightid(), is(response.getFlightid())); assertThat(model.createdSnapshotId(), is(response.getCreatedSnapshotId())); assertThat(response.getSummary(), equalToCompressingWhiteSpace(expectedSummaryString)); + assertThat(model.samGroupName(), is(response.getAuthGroupName())); } } diff --git a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderServiceTest.java b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderServiceTest.java index c4f6e6514a..4b1b5c8a44 100644 --- a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderServiceTest.java +++ b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderServiceTest.java @@ -118,7 +118,7 @@ public void beforeEach() { @Test void createRequest() { UUID snapshotId = UUID.randomUUID(); - SnapshotAccessRequestModel model = SnapshotBuilderTestData.createAccessRequest(); + SnapshotAccessRequestModel model = SnapshotBuilderTestData.createAccessRequestModelApproved(); when(snapshotRequestDao.create( SnapshotBuilderTestData.createSnapshotAccessRequest(snapshotId), TEST_USER.getEmail())) .thenReturn(model); @@ -136,7 +136,7 @@ void createRequest() { @Test void createRequestRollsBackIfSamFails() { UUID snapshotId = UUID.randomUUID(); - SnapshotAccessRequestModel model = SnapshotBuilderTestData.createAccessRequest(); + SnapshotAccessRequestModel model = SnapshotBuilderTestData.createAccessRequestModelApproved(); SnapshotAccessRequest request = SnapshotBuilderTestData.createSnapshotAccessRequest(snapshotId); when(snapshotRequestDao.create(request, TEST_USER.getEmail())).thenReturn(model); when(iamService.createSnapshotBuilderRequestResource(eq(TEST_USER), any(), any())) @@ -439,7 +439,7 @@ void testParentQueryResult() throws Exception { @Test void testRejectRequest() { UUID id = UUID.randomUUID(); - var response = SnapshotBuilderTestData.createAccessRequest(); + var response = SnapshotBuilderTestData.createAccessRequestModelApproved(); when(snapshotRequestDao.getById(id)).thenReturn(response); when(snapshotBuilderSettingsDao.getBySnapshotId(any())) .thenReturn(SnapshotBuilderTestData.SETTINGS); @@ -451,7 +451,7 @@ void testRejectRequest() { @Test void testApproveRequest() { - var response = SnapshotBuilderTestData.createAccessRequest(); + var response = SnapshotBuilderTestData.createAccessRequestModelApproved(); UUID id = response.id(); when(snapshotRequestDao.getById(id)).thenReturn(response); when(snapshotBuilderSettingsDao.getBySnapshotId(any())) @@ -464,7 +464,7 @@ void testApproveRequest() { @Test void testGetRequest() { - var daoResponse = SnapshotBuilderTestData.createAccessRequest(); + var daoResponse = SnapshotBuilderTestData.createAccessRequestModelApproved(); UUID id = daoResponse.id(); when(snapshotRequestDao.getById(id)).thenReturn(daoResponse); when(snapshotBuilderSettingsDao.getBySnapshotId(any())) @@ -486,7 +486,7 @@ void testDeleteRequest() { void testEnumerateRequestsBySnapshot() { UUID id = UUID.randomUUID(); List daoResponse = - List.of(SnapshotBuilderTestData.createAccessRequest()); + List.of(SnapshotBuilderTestData.createAccessRequestModelApproved()); when(snapshotRequestDao.enumerateBySnapshot(id)).thenReturn(daoResponse); when(snapshotBuilderSettingsDao.getBySnapshotId(any())) .thenReturn(SnapshotBuilderTestData.SETTINGS); @@ -570,7 +570,7 @@ void createExportSnapshotLink() { @Test void notifySnapshotReady() { - var request = SnapshotBuilderTestData.createAccessRequest(); + var request = SnapshotBuilderTestData.createAccessRequestModelApproved(); when(snapshotRequestDao.getById(request.id())).thenReturn(request); var snapshot = new Snapshot().name("name"); when(snapshotService.retrieve(request.createdSnapshotId())).thenReturn(snapshot); diff --git a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java index a9b15acde8..964c4b0ba8 100644 --- a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java +++ b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java @@ -431,6 +431,8 @@ public static SnapshotAccessRequestModel createSnapshotAccessRequestModel(UUID s null, SnapshotAccessRequestStatus.SUBMITTED, null, + null, + null, null); } @@ -446,17 +448,20 @@ public static SnapshotRequestModel createSnapshotRequestByRequestId( new SnapshotRequestIdModel().snapshotRequestId(snapshotAccessRequestId)))); } - public static SnapshotAccessRequestModel createAccessRequest() { + public static SnapshotAccessRequestModel createAccessRequestModelApproved() { + SnapshotAccessRequest request = createSnapshotAccessRequest(UUID.randomUUID()); return new SnapshotAccessRequestModel( UUID.randomUUID(), + request.getName(), + request.getResearchPurposeStatement(), + request.getSourceSnapshotId(), null, - null, - UUID.randomUUID(), - null, - "email@a.com", + "user@gmail.com", + Instant.now(), + Instant.now(), + SnapshotAccessRequestStatus.APPROVED, null, null, - SnapshotAccessRequestStatus.APPROVED, null, null); } diff --git a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotRequestDaoTest.java b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotRequestDaoTest.java index 0870b085ee..27023b2920 100644 --- a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotRequestDaoTest.java +++ b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotRequestDaoTest.java @@ -20,14 +20,17 @@ import bio.terra.service.dataset.Dataset; import bio.terra.service.snapshot.Snapshot; import java.io.IOException; +import java.util.Arrays; import java.util.Set; import java.util.UUID; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.test.context.ActiveProfiles; @SpringBootTest @@ -47,6 +50,9 @@ class SnapshotRequestDaoTest { private static final String EMAIL = "user@gmail.com"; private static final String FLIGHT_ID = "flight_id"; + private static final String SAM_GROUP_NAME = "samGroupName"; + private static final String SAM_GROUP_CREATED_BY = "samGroupCreatedByUser@gmail.com"; + private static final UUID RANDOM_UUID = UUID.randomUUID(); @BeforeEach void beforeEach() throws IOException { @@ -66,14 +72,20 @@ private SnapshotAccessRequestModel createRequest(SnapshotAccessRequest snapshotA return snapshotRequestDao.create(snapshotAccessRequest, EMAIL); } - private void verifyResponseContents(SnapshotAccessRequestModel response) { + private void verifyResponseContents( + SnapshotAccessRequestModel response, String... ignoredFields) { SnapshotAccessRequestModel expected = SnapshotBuilderTestData.createSnapshotAccessRequestModel(sourceSnapshot.getId()); + // always ignore id and createdDate which are unique + String[] ignored = + Stream.concat(Arrays.stream(ignoredFields), Stream.of("id", "createdDate")) + .toArray(String[]::new); + assertThat( "Given response is the same as expected.", response, - samePropertyValuesAs(expected, "id", "createdDate", "flightid", "createdSnapshotId")); + samePropertyValuesAs(expected, ignored)); assertNotNull(response.id(), "Snapshot Access Request Response should have an id"); assertNotNull( response.createdDate(), @@ -89,7 +101,23 @@ void getById() { @Test void getByIdNotFound() { - assertThrows(NotFoundException.class, () -> snapshotRequestDao.getById(UUID.randomUUID())); + assertThrows(NotFoundException.class, () -> snapshotRequestDao.getById(RANDOM_UUID)); + } + + @Test + void getByCreatedSnapshotId() { + SnapshotAccessRequestModel response = createRequest(); + snapshotRequestDao.updateCreatedInfo( + response.id(), createdSnapshot.getId(), "name", "userEmail"); + SnapshotAccessRequestModel retrieved = + snapshotRequestDao.getByCreatedSnapshotId(createdSnapshot.getId()); + verifyResponseContents(retrieved, "createdSnapshotId", "samGroupName", "samGroupCreatedBy"); + } + + @Test + void getByCreatedSnapshotIdNotFound() { + assertThrows( + NotFoundException.class, () -> snapshotRequestDao.getByCreatedSnapshotId(RANDOM_UUID)); } @Test @@ -168,8 +196,7 @@ void createSnapshotIdNotFound() { assertThrows( NotFoundException.class, () -> - snapshotRequestDao.create( - snapshotAccessRequest.sourceSnapshotId(UUID.randomUUID()), EMAIL)); + snapshotRequestDao.create(snapshotAccessRequest.sourceSnapshotId(RANDOM_UUID), EMAIL)); } @Test @@ -192,9 +219,7 @@ void updateStatus() { void updateStatusIdNotFound() { assertThrows( NotFoundException.class, - () -> - snapshotRequestDao.updateStatus( - UUID.randomUUID(), SnapshotAccessRequestStatus.APPROVED)); + () -> snapshotRequestDao.updateStatus(RANDOM_UUID, SnapshotAccessRequestStatus.APPROVED)); } @Test @@ -205,7 +230,7 @@ void updateFlightId() { SnapshotAccessRequestModel updatedResponse = snapshotRequestDao.getById(response.id()); // only the flightId is updated - verifyResponseContents(updatedResponse); + verifyResponseContents(updatedResponse, "flightid"); assertThat( "Updated Snapshot Access Request Response should have flight id", updatedResponse.flightid(), @@ -215,39 +240,51 @@ void updateFlightId() { @Test void updateFlightIdNotFound() { assertThrows( - NotFoundException.class, - () -> snapshotRequestDao.updateFlightId(UUID.randomUUID(), FLIGHT_ID)); + NotFoundException.class, () -> snapshotRequestDao.updateFlightId(RANDOM_UUID, FLIGHT_ID)); } @Test - void updateCreatedSnapshotId() { + void updateCreatedInfo() { SnapshotAccessRequestModel response = createRequest(); verifyResponseContents(response); - snapshotRequestDao.updateCreatedSnapshotId(response.id(), createdSnapshot.getId()); + snapshotRequestDao.updateCreatedInfo( + response.id(), createdSnapshot.getId(), SAM_GROUP_NAME, SAM_GROUP_CREATED_BY); SnapshotAccessRequestModel updatedResponse = snapshotRequestDao.getById(response.id()); - // only the createdSnapshotId is updated - verifyResponseContents(updatedResponse); + // other fields remain unchanged + verifyResponseContents( + updatedResponse, "createdSnapshotId", "samGroupName", "samGroupCreatedBy"); assertThat( - "Updated Snapshot Access Request Response should have created snapshot id", + "Updated Snapshot Access Request Response should have the created snapshot id", updatedResponse.createdSnapshotId(), equalTo(createdSnapshot.getId())); + assertThat( + "Updated Snapshot Access Request Response should have saved sam group name", + updatedResponse.samGroupName(), + equalTo(SAM_GROUP_NAME)); + assertThat( + "Updated Snapshot Access Request Response should have saved sam group created by email", + updatedResponse.samGroupCreatedByTerraId(), + equalTo(SAM_GROUP_CREATED_BY)); } @Test - void updateCreatedSnapshotSourceIdNotFound() { + void updateCreatedInfoRequestIdNotFound() { assertThrows( NotFoundException.class, () -> - snapshotRequestDao.updateCreatedSnapshotId(UUID.randomUUID(), createdSnapshot.getId())); + snapshotRequestDao.updateCreatedInfo( + RANDOM_UUID, createdSnapshot.getId(), SAM_GROUP_NAME, SAM_GROUP_CREATED_BY)); } @Test - void updateCreatedSnapshotIdNotFound() { + void updateCreatedInfoSnapshotIdNotFound() { + SnapshotAccessRequestModel response = createRequest(); assertThrows( - NotFoundException.class, + DataIntegrityViolationException.class, () -> - snapshotRequestDao.updateCreatedSnapshotId(sourceSnapshot.getId(), UUID.randomUUID())); + snapshotRequestDao.updateCreatedInfo( + response.id(), RANDOM_UUID, SAM_GROUP_NAME, SAM_GROUP_CREATED_BY)); } @Test @@ -266,6 +303,6 @@ void deleteSourceSnapshot() { @Test void deleteNotFound() { - assertThrows(NotFoundException.class, () -> snapshotRequestDao.delete(UUID.randomUUID())); + assertThrows(NotFoundException.class, () -> snapshotRequestDao.delete(RANDOM_UUID)); } }