From d00c467aeaade73bb59e3bc44fa779678337ed0e Mon Sep 17 00:00:00 2001 From: Phil Shapiro Date: Tue, 20 Aug 2024 14:45:42 -0400 Subject: [PATCH] DC-958: Send email on snapshot creation complete (#1760) - Add general support for using the thurloe pubsub topic for email notifications from TDR - Add an email to be sent when snapshot creation is complete --- .../NotificationConfiguration.java | 6 ++ .../auth/iam/IamProviderInterface.java | 3 + .../terra/service/auth/iam/IamService.java | 6 ++ .../terra/service/auth/iam/sam/SamIam.java | 46 ++++------ .../notification/NotificationService.java | 54 ++++++++++++ .../service/notification/PubSubService.java | 33 +++++++ .../SnapshotReadyNotification.java | 21 +++++ .../NotifyUserOfSnapshotCreationStep.java | 37 ++++++++ .../flight/create/SnapshotCreateFlight.java | 9 +- .../SnapshotBuilderService.java | 27 +++++- src/main/resources/application.properties | 2 + .../notification/NotificationServiceTest.java | 44 ++++++++++ .../service/snapshot/SnapshotServiceTest.java | 87 ++++++------------- .../NotifyUserOfSnapshotCreationStepTest.java | 42 +++++++++ .../create/SnapshotCreateFlightTest.java | 3 +- .../SnapshotBuilderServiceTest.java | 68 ++++++++++----- .../SnapshotBuilderTestData.java | 15 ++++ .../tabulardata/google/BigQueryPdaoTest.java | 3 + 18 files changed, 392 insertions(+), 114 deletions(-) create mode 100644 src/main/java/bio/terra/app/configuration/NotificationConfiguration.java create mode 100644 src/main/java/bio/terra/service/notification/NotificationService.java create mode 100644 src/main/java/bio/terra/service/notification/PubSubService.java create mode 100644 src/main/java/bio/terra/service/notification/SnapshotReadyNotification.java create mode 100644 src/main/java/bio/terra/service/snapshot/flight/create/NotifyUserOfSnapshotCreationStep.java create mode 100644 src/test/java/bio/terra/service/notification/NotificationServiceTest.java create mode 100644 src/test/java/bio/terra/service/snapshot/flight/create/NotifyUserOfSnapshotCreationStepTest.java diff --git a/src/main/java/bio/terra/app/configuration/NotificationConfiguration.java b/src/main/java/bio/terra/app/configuration/NotificationConfiguration.java new file mode 100644 index 0000000000..261ec51346 --- /dev/null +++ b/src/main/java/bio/terra/app/configuration/NotificationConfiguration.java @@ -0,0 +1,6 @@ +package bio.terra.app.configuration; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +@ConfigurationProperties(prefix = "notification") +public record NotificationConfiguration(String projectId, String topicId) {} diff --git a/src/main/java/bio/terra/service/auth/iam/IamProviderInterface.java b/src/main/java/bio/terra/service/auth/iam/IamProviderInterface.java index 1f0136c55d..a8f77ce677 100644 --- a/src/main/java/bio/terra/service/auth/iam/IamProviderInterface.java +++ b/src/main/java/bio/terra/service/auth/iam/IamProviderInterface.java @@ -14,6 +14,7 @@ import java.util.Set; import java.util.UUID; import org.broadinstitute.dsde.workbench.client.sam.model.ManagedResourceGroupCoordinates; +import org.broadinstitute.dsde.workbench.client.sam.model.UserIdInfo; /** * This is the interface to IAM used in the main body of the repository code. Right now, the only @@ -332,4 +333,6 @@ void azureCreateManagedResourceGroup( boolean getResourceTypeAdminPermission( AuthenticatedUserRequest userReq, IamResourceType iamResourceType, IamAction action) throws InterruptedException; + + UserIdInfo getUserIds(String accessToken, String userEmail) throws InterruptedException; } diff --git a/src/main/java/bio/terra/service/auth/iam/IamService.java b/src/main/java/bio/terra/service/auth/iam/IamService.java index 8276c1599d..0783be639b 100644 --- a/src/main/java/bio/terra/service/auth/iam/IamService.java +++ b/src/main/java/bio/terra/service/auth/iam/IamService.java @@ -27,6 +27,7 @@ import java.util.concurrent.TimeUnit; import org.apache.commons.collections4.ListUtils; import org.apache.commons.collections4.map.PassiveExpiringMap; +import org.broadinstitute.dsde.workbench.client.sam.model.UserIdInfo; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -522,4 +523,9 @@ public String signUrlForBlob( AuthenticatedUserRequest userReq, String project, String path, Duration duration) { return callProvider(() -> iamProvider.signUrlForBlob(userReq, project, path, duration)); } + + public UserIdInfo getUserIds(String userEmail) { + String tdrSaAccessToken = googleCredentialsService.getApplicationDefaultAccessToken(SCOPES); + return callProvider(() -> iamProvider.getUserIds(tdrSaAccessToken, userEmail)); + } } diff --git a/src/main/java/bio/terra/service/auth/iam/sam/SamIam.java b/src/main/java/bio/terra/service/auth/iam/sam/SamIam.java index f5ac4a5d30..30379ec027 100644 --- a/src/main/java/bio/terra/service/auth/iam/sam/SamIam.java +++ b/src/main/java/bio/terra/service/auth/iam/sam/SamIam.java @@ -58,6 +58,7 @@ import org.broadinstitute.dsde.workbench.client.sam.model.RolesAndActions; import org.broadinstitute.dsde.workbench.client.sam.model.SyncReportEntry; import org.broadinstitute.dsde.workbench.client.sam.model.SystemStatus; +import org.broadinstitute.dsde.workbench.client.sam.model.UserIdInfo; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -840,6 +841,15 @@ AccessPolicyMembershipRequest createAccessPolicy(IamRole role, List emai return membership; } + @Override + public UserIdInfo getUserIds(String accessToken, String userEmail) throws InterruptedException { + return SamRetry.retry(configurationService, () -> getUserIdsInner(accessToken, userEmail)); + } + + private UserIdInfo getUserIdsInner(String accessToken, String userEmail) throws ApiException { + return samApiService.usersApi(accessToken).getUserIds(userEmail); + } + /** * Syncing a policy with SAM results in a Google group being created that is tied to that policy. * The response is an object with one key that is the policy group email and a value that is a @@ -867,7 +877,7 @@ public static ErrorReportException convertSamExToDataRepoEx(final ApiException s logger.warn("Sam client exception details: {}", samEx.getResponseBody()); // Sometimes the sam message is buried several levels down inside of the error report object. - String message = null; + String message; try { ErrorReport errorReport = objectMapper.readValue(samEx.getResponseBody(), ErrorReport.class); message = extractErrorMessage(errorReport); @@ -875,34 +885,16 @@ public static ErrorReportException convertSamExToDataRepoEx(final ApiException s message = Objects.requireNonNullElse(samEx.getMessage(), "Sam client exception"); } - switch (samEx.getCode()) { - case HttpStatusCodes.STATUS_CODE_BAD_REQUEST: - { - return new IamBadRequestException(message, samEx); - } - case HttpStatusCodes.STATUS_CODE_UNAUTHORIZED: - { - return new IamUnauthorizedException(message, samEx); - } - case HttpStatusCodes.STATUS_CODE_FORBIDDEN: - { - return new IamForbiddenException(message, samEx); - } - case HttpStatusCodes.STATUS_CODE_NOT_FOUND: - { - return new IamNotFoundException(message, samEx); - } - case HttpStatusCodes.STATUS_CODE_CONFLICT: - { - return new IamConflictException(message, samEx); - } + return switch (samEx.getCode()) { + case HttpStatusCodes.STATUS_CODE_BAD_REQUEST -> new IamBadRequestException(message, samEx); + case HttpStatusCodes.STATUS_CODE_UNAUTHORIZED -> new IamUnauthorizedException(message, samEx); + case HttpStatusCodes.STATUS_CODE_FORBIDDEN -> new IamForbiddenException(message, samEx); + case HttpStatusCodes.STATUS_CODE_NOT_FOUND -> new IamNotFoundException(message, samEx); + case HttpStatusCodes.STATUS_CODE_CONFLICT -> new IamConflictException(message, samEx); // SAM does not use a 501 NOT_IMPLEMENTED status code, so that case is skipped here // A 401 error will only occur when OpenDJ is down and should be raised as a 500 error - default: - { - return new IamInternalServerErrorException(message, samEx); - } - } + default -> new IamInternalServerErrorException(message, samEx); + }; } @VisibleForTesting diff --git a/src/main/java/bio/terra/service/notification/NotificationService.java b/src/main/java/bio/terra/service/notification/NotificationService.java new file mode 100644 index 0000000000..cbd23567a9 --- /dev/null +++ b/src/main/java/bio/terra/service/notification/NotificationService.java @@ -0,0 +1,54 @@ +package bio.terra.service.notification; + +import bio.terra.app.configuration.NotificationConfiguration; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.annotations.VisibleForTesting; +import jakarta.annotation.PostConstruct; +import java.io.IOException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Service; + +@Service +public class NotificationService { + + private static final Logger logger = LoggerFactory.getLogger(NotificationService.class); + + private final PubSubService pubSubService; + private final NotificationConfiguration notificationConfiguration; + private final ObjectMapper objectMapper; + + public NotificationService( + PubSubService pubSubService, + NotificationConfiguration notificationConfiguration, + ObjectMapper objectMapper) { + this.pubSubService = pubSubService; + this.notificationConfiguration = notificationConfiguration; + this.objectMapper = objectMapper; + } + + @VisibleForTesting + @PostConstruct + protected void createTopic() { + try { + pubSubService.createTopic( + notificationConfiguration.projectId(), notificationConfiguration.topicId()); + } catch (IOException e) { + logger.warn("Error creating notification topic", e); + } + } + + public void snapshotReady( + String subjectId, String snapshotExportLink, String snapshotName, String snapshotSummary) { + try { + pubSubService.publishMessage( + notificationConfiguration.projectId(), + notificationConfiguration.topicId(), + objectMapper.writeValueAsString( + new SnapshotReadyNotification( + subjectId, snapshotExportLink, snapshotName, snapshotSummary))); + } catch (IOException e) { + logger.warn("Error sending notification", e); + } + } +} diff --git a/src/main/java/bio/terra/service/notification/PubSubService.java b/src/main/java/bio/terra/service/notification/PubSubService.java new file mode 100644 index 0000000000..e387aff1d5 --- /dev/null +++ b/src/main/java/bio/terra/service/notification/PubSubService.java @@ -0,0 +1,33 @@ +package bio.terra.service.notification; + +import com.google.cloud.pubsub.v1.Publisher; +import com.google.cloud.pubsub.v1.TopicAdminClient; +import com.google.protobuf.ByteString; +import com.google.pubsub.v1.PubsubMessage; +import com.google.pubsub.v1.Topic; +import com.google.pubsub.v1.TopicName; +import java.io.IOException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Service; + +@Service +public class PubSubService { + private static final Logger logger = LoggerFactory.getLogger(PubSubService.class); + + public void createTopic(String projectId, String topicId) throws IOException { + try (TopicAdminClient topicAdminClient = TopicAdminClient.create()) { + TopicName topicName = TopicName.of(projectId, topicId); + if (topicAdminClient.getTopic(topicName) == null) { + Topic topic = topicAdminClient.createTopic(topicName); + logger.info("Created topic: {}", topic.getName()); + } + } + } + + public void publishMessage(String projectId, String topicId, String message) throws IOException { + TopicName topicName = TopicName.of(projectId, topicId); + var publisher = Publisher.newBuilder(topicName).build(); + publisher.publish(PubsubMessage.newBuilder().setData(ByteString.copyFromUtf8(message)).build()); + } +} diff --git a/src/main/java/bio/terra/service/notification/SnapshotReadyNotification.java b/src/main/java/bio/terra/service/notification/SnapshotReadyNotification.java new file mode 100644 index 0000000000..37bd2d712e --- /dev/null +++ b/src/main/java/bio/terra/service/notification/SnapshotReadyNotification.java @@ -0,0 +1,21 @@ +package bio.terra.service.notification; + +public record SnapshotReadyNotification( + String notificationType, + String recipientUserId, + String snapshotExportLink, + String snapshotName, + String snapshotSummary) { + public SnapshotReadyNotification( + String recipientUserId, + String snapshotExportLink, + String snapshotName, + String snapshotSummary) { + this( + "SnapshotReadyNotification", + recipientUserId, + snapshotExportLink, + snapshotName, + snapshotSummary); + } +} diff --git a/src/main/java/bio/terra/service/snapshot/flight/create/NotifyUserOfSnapshotCreationStep.java b/src/main/java/bio/terra/service/snapshot/flight/create/NotifyUserOfSnapshotCreationStep.java new file mode 100644 index 0000000000..0cf6714efd --- /dev/null +++ b/src/main/java/bio/terra/service/snapshot/flight/create/NotifyUserOfSnapshotCreationStep.java @@ -0,0 +1,37 @@ +package bio.terra.service.snapshot.flight.create; + +import bio.terra.service.auth.iam.IamService; +import bio.terra.service.job.DefaultUndoStep; +import bio.terra.service.snapshotbuilder.SnapshotBuilderService; +import bio.terra.service.snapshotbuilder.SnapshotRequestDao; +import bio.terra.stairway.FlightContext; +import bio.terra.stairway.StepResult; +import bio.terra.stairway.exception.RetryException; +import java.util.UUID; + +public class NotifyUserOfSnapshotCreationStep extends DefaultUndoStep { + private final SnapshotBuilderService snapshotBuilderService; + private final SnapshotRequestDao snapshotRequestDao; + private final IamService iamService; + private final UUID snapshotRequestId; + + public NotifyUserOfSnapshotCreationStep( + SnapshotBuilderService snapshotBuilderService, + SnapshotRequestDao snapshotRequestDao, + IamService iamService, + UUID snapshotRequestId) { + this.snapshotBuilderService = snapshotBuilderService; + this.snapshotRequestDao = snapshotRequestDao; + this.iamService = iamService; + this.snapshotRequestId = snapshotRequestId; + } + + @Override + public StepResult doStep(FlightContext flightContext) + throws InterruptedException, RetryException { + var request = snapshotRequestDao.getById(snapshotRequestId); + var user = iamService.getUserIds(request.createdBy()); + snapshotBuilderService.notifySnapshotReady(user.getUserSubjectId(), snapshotRequestId); + 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 4aeb24561e..c890aa5c49 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 @@ -432,11 +432,16 @@ public SnapshotCreateFlight(FlightMap inputParameters, Object applicationContext IamResourceType.DATASET, "A snapshot was created from this dataset.")); - // at end of flight, add created snapshot id to the snapshot request if (mode == SnapshotRequestContentsModel.ModeEnum.BYREQUESTID) { + UUID snapshotRequestId = contents.getRequestIdSpec().getSnapshotRequestId(); + // Add created snapshot id to the snapshot request. addStep( new AddCreatedSnapshotIdToSnapshotRequestStep( - snapshotRequestDao, contents.getRequestIdSpec().getSnapshotRequestId(), snapshotId)); + snapshotRequestDao, snapshotRequestId, snapshotId)); + // Notify user that snapshot is ready to use. + addStep( + new NotifyUserOfSnapshotCreationStep( + snapshotBuilderService, snapshotRequestDao, iamService, snapshotRequestId)); } } } diff --git a/src/main/java/bio/terra/service/snapshotbuilder/SnapshotBuilderService.java b/src/main/java/bio/terra/service/snapshotbuilder/SnapshotBuilderService.java index 4d19fdb2ac..19f91cd052 100644 --- a/src/main/java/bio/terra/service/snapshotbuilder/SnapshotBuilderService.java +++ b/src/main/java/bio/terra/service/snapshotbuilder/SnapshotBuilderService.java @@ -1,5 +1,6 @@ package bio.terra.service.snapshotbuilder; +import bio.terra.app.configuration.TerraConfiguration; import bio.terra.common.CloudPlatformWrapper; import bio.terra.common.exception.ApiException; import bio.terra.common.exception.BadRequestException; @@ -23,6 +24,7 @@ import bio.terra.service.dataset.Dataset; import bio.terra.service.dataset.DatasetService; import bio.terra.service.filedata.azure.AzureSynapsePdao; +import bio.terra.service.notification.NotificationService; import bio.terra.service.snapshot.Snapshot; import bio.terra.service.snapshot.SnapshotService; import bio.terra.service.snapshotbuilder.query.Query; @@ -61,9 +63,10 @@ public class SnapshotBuilderService { private final IamService iamService; private final SnapshotService snapshotService; private final BigQuerySnapshotPdao bigQuerySnapshotPdao; - private final AzureSynapsePdao azureSynapsePdao; private final QueryBuilderFactory queryBuilderFactory; + private final NotificationService notificationService; + private final TerraConfiguration terraConfiguration; public SnapshotBuilderService( SnapshotRequestDao snapshotRequestDao, @@ -72,16 +75,20 @@ public SnapshotBuilderService( IamService iamService, SnapshotService snapshotService, BigQuerySnapshotPdao bigQuerySnapshotPdao, + NotificationService notificationService, AzureSynapsePdao azureSynapsePdao, - QueryBuilderFactory queryBuilderFactory) { + QueryBuilderFactory queryBuilderFactory, + TerraConfiguration terraConfiguration) { this.snapshotRequestDao = snapshotRequestDao; this.snapshotBuilderSettingsDao = snapshotBuilderSettingsDao; this.datasetService = datasetService; this.iamService = iamService; this.snapshotService = snapshotService; this.bigQuerySnapshotPdao = bigQuerySnapshotPdao; + this.notificationService = notificationService; this.azureSynapsePdao = azureSynapsePdao; this.queryBuilderFactory = queryBuilderFactory; + this.terraConfiguration = terraConfiguration; } public SnapshotAccessRequestResponse createRequest( @@ -414,4 +421,20 @@ private SnapshotBuilderDomainOption getDomainOption( String domainId = getDomainId(conceptId, snapshot, userRequest); return getDomainOptionFromSettingsByName(domainId, snapshot.getId()); } + + public String createExportSnapshotLink(UUID snapshotId) { + return "%s/import-data?snapshotId=%s&format=tdrexport&tdrSyncPermissions=false" + .formatted(terraConfiguration.basePath(), snapshotId); + } + + public void notifySnapshotReady(String subjectId, UUID snapshotRequestId) { + var snapshotAccessRequest = snapshotRequestDao.getById(snapshotRequestId); + UUID snapshotId = snapshotAccessRequest.createdSnapshotId(); + Snapshot snapshot = snapshotService.retrieve(snapshotId); + notificationService.snapshotReady( + subjectId, + createExportSnapshotLink(snapshotId), + snapshot.getName(), + convertModelToApiResponse(snapshotAccessRequest).getSummary()); + } } diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 787dbabfe3..52cb33c4fc 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -158,3 +158,5 @@ duos.basePath=https://consent.dsde-dev.broadinstitute.org tps.basePath=https://tps.dsde-dev.broadinstitute.org sentry.dsn= sentry.environment=undefined +notification.projectId=broad-dsde-dev +notification.topicId=workbench-notifications-dev diff --git a/src/test/java/bio/terra/service/notification/NotificationServiceTest.java b/src/test/java/bio/terra/service/notification/NotificationServiceTest.java new file mode 100644 index 0000000000..f541329e0c --- /dev/null +++ b/src/test/java/bio/terra/service/notification/NotificationServiceTest.java @@ -0,0 +1,44 @@ +package bio.terra.service.notification; + +import static org.mockito.Mockito.verify; + +import bio.terra.app.configuration.NotificationConfiguration; +import bio.terra.common.category.Unit; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@Tag(Unit.TAG) +@ExtendWith(MockitoExtension.class) +class NotificationServiceTest { + @Mock private PubSubService pubSubService; + private NotificationService notificationService; + private final NotificationConfiguration configuration = + new NotificationConfiguration("project", "topic"); + + @BeforeEach + void beforeEach() { + notificationService = new NotificationService(pubSubService, configuration, new ObjectMapper()); + } + + @Test + void createTopic() throws Exception { + notificationService.createTopic(); + verify(pubSubService).createTopic(configuration.projectId(), configuration.topicId()); + } + + @Test + void snapshotReady() throws Exception { + notificationService.snapshotReady("id", "link", "name", "summary"); + verify(pubSubService) + .publishMessage( + configuration.projectId(), + configuration.topicId(), + """ + {"notificationType":"SnapshotReadyNotification","recipientUserId":"id","snapshotExportLink":"link","snapshotName":"name","snapshotSummary":"summary"}"""); + } +} diff --git a/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java b/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java index ac37a7d5bc..c10efbba67 100644 --- a/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java +++ b/src/test/java/bio/terra/service/snapshot/SnapshotServiceTest.java @@ -621,45 +621,37 @@ void validateForByRequestIdModeApproved() { SnapshotRequestContentsModel snapshotRequestContentsModel = makeByRequestIdContentsModel(snapshotAccessRequestId); SnapshotAccessRequestModel accessRequestResponse = - new SnapshotAccessRequestModel( - null, - null, - null, - null, - null, - "email@a.com", - null, - null, - SnapshotAccessRequestStatus.APPROVED, - null, - null); + SnapshotBuilderTestData.createAccessRequest(); when(snapshotRequestDao.getById(snapshotAccessRequestId)).thenReturn(accessRequestResponse); assertDoesNotThrow(() -> service.validateForByRequestIdMode(snapshotRequestContentsModel)); } + static SnapshotAccessRequestModel createAccessRequestWithFlightid() { + return new SnapshotAccessRequestModel( + null, + null, + null, + null, + null, + "email@a.com", + null, + null, + SnapshotAccessRequestStatus.APPROVED, + null, + "flightId"); + } + @Test void validateForByRequestIdModeJobFailed() { UUID snapshotAccessRequestId = UUID.randomUUID(); - String flightId = "flightId"; SnapshotRequestContentsModel snapshotRequestContentsModel = makeByRequestIdContentsModel(snapshotAccessRequestId); - SnapshotAccessRequestModel accessRequestResponse = - new SnapshotAccessRequestModel( - null, - null, - null, - null, - null, - "email@a.com", - null, - null, - SnapshotAccessRequestStatus.APPROVED, - null, - flightId); + SnapshotAccessRequestModel accessRequestResponse = createAccessRequestWithFlightid(); when(snapshotRequestDao.getById(snapshotAccessRequestId)).thenReturn(accessRequestResponse); - when(jobService.unauthRetrieveJobState(flightId)).thenReturn(FlightStatus.ERROR); + when(jobService.unauthRetrieveJobState(accessRequestResponse.flightid())) + .thenReturn(FlightStatus.ERROR); assertDoesNotThrow(() -> service.validateForByRequestIdMode(snapshotRequestContentsModel)); } @@ -718,26 +710,14 @@ void validateForByRequestIdModeAlreadyCreated() { @Test void validateForByRequestIdModeJobRunning() { UUID snapshotAccessRequestId = UUID.randomUUID(); - String flightId = "flightId"; SnapshotRequestContentsModel snapshotRequestContentsModel = makeByRequestIdContentsModel(snapshotAccessRequestId); - SnapshotAccessRequestModel accessRequestResponse = - new SnapshotAccessRequestModel( - null, - null, - null, - null, - null, - null, - null, - null, - SnapshotAccessRequestStatus.APPROVED, - null, - flightId); + SnapshotAccessRequestModel accessRequestResponse = createAccessRequestWithFlightid(); when(snapshotRequestDao.getById(snapshotAccessRequestId)).thenReturn(accessRequestResponse); // any flight status that isn't error or fatal - when(jobService.unauthRetrieveJobState(flightId)).thenReturn(FlightStatus.READY); + when(jobService.unauthRetrieveJobState(accessRequestResponse.flightid())) + .thenReturn(FlightStatus.READY); assertThrows( ValidationException.class, () -> service.validateForByRequestIdMode(snapshotRequestContentsModel)); @@ -1158,26 +1138,15 @@ void testCreateSnapshotThrowsWhenDuosClientThrows() { @Test void testCreateSnapshotWithByRequestId() { - UUID snapshotAccessRequestId = UUID.randomUUID(); + SnapshotAccessRequestModel snapshotAccessRequest = + SnapshotBuilderTestData.createAccessRequest(); + UUID snapshotAccessRequestId = snapshotAccessRequest.id(); SnapshotRequestContentsModel contentsModel = makeByRequestIdContentsModel(snapshotAccessRequestId); SnapshotRequestModel request = new SnapshotRequestModel().contents(List.of(contentsModel)); request.profileId(UUID.randomUUID()); JobBuilder jobBuilder = mock(JobBuilder.class); String jobId = mockJobService(request, jobBuilder); - SnapshotAccessRequestModel snapshotAccessRequest = - new SnapshotAccessRequestModel( - snapshotAccessRequestId, - null, - null, - UUID.randomUUID(), - null, - "email@a.com", - null, - null, - SnapshotAccessRequestStatus.APPROVED, - null, - null); when(snapshotRequestDao.getById(snapshotAccessRequestId)).thenReturn(snapshotAccessRequest); when(snapshotDao.retrieveSnapshot(snapshotAccessRequest.sourceSnapshotId())) .thenReturn( @@ -1480,14 +1449,14 @@ void getSourceDatasetFromSnapshotRequestHandlesByRequestId() { new SnapshotRequestModel().contents(List.of(contentsModel)); SnapshotAccessRequestModel snapshotAccessRequest = - new SnapshotAccessRequestModel( - null, null, null, snapshotId, null, null, null, null, null, null, null); + SnapshotBuilderTestData.createAccessRequest(); Dataset dataset = new Dataset().id(datasetId).name(DATASET_NAME); Snapshot snapshot = new Snapshot().snapshotSources(List.of(new SnapshotSource().dataset(dataset))); when(snapshotRequestDao.getById(snapshotAccessRequestId)).thenReturn(snapshotAccessRequest); - when(snapshotDao.retrieveSnapshot(snapshotId)).thenReturn(snapshot); + when(snapshotDao.retrieveSnapshot(snapshotAccessRequest.sourceSnapshotId())) + .thenReturn(snapshot); Dataset sourceDataset = service.getSourceDatasetFromSnapshotRequest(snapshotRequestModel); 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 new file mode 100644 index 0000000000..d6a5d25489 --- /dev/null +++ b/src/test/java/bio/terra/service/snapshot/flight/create/NotifyUserOfSnapshotCreationStepTest.java @@ -0,0 +1,42 @@ +package bio.terra.service.snapshot.flight.create; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import bio.terra.common.category.Unit; +import bio.terra.service.auth.iam.IamService; +import bio.terra.service.snapshotbuilder.SnapshotBuilderService; +import bio.terra.service.snapshotbuilder.SnapshotBuilderTestData; +import bio.terra.service.snapshotbuilder.SnapshotRequestDao; +import bio.terra.stairway.StepStatus; +import java.util.UUID; +import org.broadinstitute.dsde.workbench.client.sam.model.UserIdInfo; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@Tag(Unit.TAG) +@ExtendWith(MockitoExtension.class) +class NotifyUserOfSnapshotCreationStepTest { + @Mock private SnapshotBuilderService snapshotBuilderService; + @Mock private SnapshotRequestDao snapshotRequestDao; + @Mock private IamService iamService; + + @Test + void doStep() throws InterruptedException { + var id = UUID.randomUUID(); + var step = + new NotifyUserOfSnapshotCreationStep( + snapshotBuilderService, snapshotRequestDao, iamService, id); + var request = SnapshotBuilderTestData.createAccessRequest(); + var user = new UserIdInfo().userSubjectId("subjectId"); + when(snapshotRequestDao.getById(id)).thenReturn(request); + when(iamService.getUserIds(request.createdBy())).thenReturn(user); + assertThat(step.doStep(null).getStepStatus(), is(StepStatus.STEP_RESULT_SUCCESS)); + verify(snapshotBuilderService).notifySnapshotReady(user.getUserSubjectId(), id); + } +} 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 8af72efd58..9e880fc201 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,6 +154,7 @@ void testSnapshotCreateFlightByRequestId() { "CreateSnapshotSetResponseStep", "CreateSnapshotJournalEntryStep", "JournalRecordUpdateEntryStep", - "AddCreatedSnapshotIdToSnapshotRequestStep")); + "AddCreatedSnapshotIdToSnapshotRequestStep", + "NotifyUserOfSnapshotCreationStep")); } } diff --git a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderServiceTest.java b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderServiceTest.java index 48dcf89521..c4f6e6514a 100644 --- a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderServiceTest.java +++ b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderServiceTest.java @@ -1,18 +1,22 @@ package bio.terra.service.snapshotbuilder; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import bio.terra.app.configuration.TerraConfiguration; import bio.terra.common.CloudPlatformWrapper; import bio.terra.common.category.Unit; import bio.terra.common.exception.ApiException; @@ -39,6 +43,7 @@ import bio.terra.service.dataset.DatasetService; import bio.terra.service.dataset.DatasetSummary; import bio.terra.service.filedata.azure.AzureSynapsePdao; +import bio.terra.service.notification.NotificationService; import bio.terra.service.resourcemanagement.google.GoogleProjectResource; import bio.terra.service.snapshot.Snapshot; import bio.terra.service.snapshot.SnapshotService; @@ -85,12 +90,15 @@ class SnapshotBuilderServiceTest { @Mock private SnapshotService snapshotService; @Mock private DatasetService datasetService; @Mock private BigQuerySnapshotPdao bigQuerySnapshotPdao; + @Mock private NotificationService notificationService; @Mock private AzureSynapsePdao azureSynapsePdao; @Mock private QueryBuilderFactory queryBuilderFactory; private static final AuthenticatedUserRequest TEST_USER = AuthenticationFixtures.randomUserRequest(); + private static final TerraConfiguration terraConfiguration = new TerraConfiguration("basepath"); + @BeforeEach public void beforeEach() { snapshotBuilderService = @@ -101,16 +109,16 @@ public void beforeEach() { iamService, snapshotService, bigQuerySnapshotPdao, + notificationService, azureSynapsePdao, - queryBuilderFactory); + queryBuilderFactory, + terraConfiguration); } @Test void createRequest() { UUID snapshotId = UUID.randomUUID(); - SnapshotAccessRequestModel model = - new SnapshotAccessRequestModel( - null, null, null, null, null, null, null, null, null, null, null); + SnapshotAccessRequestModel model = SnapshotBuilderTestData.createAccessRequest(); when(snapshotRequestDao.create( SnapshotBuilderTestData.createSnapshotAccessRequest(snapshotId), TEST_USER.getEmail())) .thenReturn(model); @@ -128,15 +136,12 @@ void createRequest() { @Test void createRequestRollsBackIfSamFails() { UUID snapshotId = UUID.randomUUID(); - UUID snapshotRequestId = UUID.randomUUID(); - SnapshotAccessRequestModel model = - new SnapshotAccessRequestModel( - snapshotRequestId, null, null, null, null, null, null, null, null, null, null); + SnapshotAccessRequestModel model = SnapshotBuilderTestData.createAccessRequest(); SnapshotAccessRequest request = SnapshotBuilderTestData.createSnapshotAccessRequest(snapshotId); when(snapshotRequestDao.create(request, TEST_USER.getEmail())).thenReturn(model); when(iamService.createSnapshotBuilderRequestResource(eq(TEST_USER), any(), any())) .thenThrow(new ApiException("Error")); - doNothing().when(snapshotRequestDao).delete(snapshotRequestId); + doNothing().when(snapshotRequestDao).delete(model.id()); assertThrows( InternalServerErrorException.class, () -> snapshotBuilderService.createRequest(TEST_USER, request)); @@ -434,9 +439,7 @@ void testParentQueryResult() throws Exception { @Test void testRejectRequest() { UUID id = UUID.randomUUID(); - var response = - new SnapshotAccessRequestModel( - null, null, null, null, null, null, null, null, null, null, null); + var response = SnapshotBuilderTestData.createAccessRequest(); when(snapshotRequestDao.getById(id)).thenReturn(response); when(snapshotBuilderSettingsDao.getBySnapshotId(any())) .thenReturn(SnapshotBuilderTestData.SETTINGS); @@ -448,10 +451,8 @@ void testRejectRequest() { @Test void testApproveRequest() { - UUID id = UUID.randomUUID(); - var response = - new SnapshotAccessRequestModel( - id, null, null, null, null, null, null, null, null, null, null); + var response = SnapshotBuilderTestData.createAccessRequest(); + UUID id = response.id(); when(snapshotRequestDao.getById(id)).thenReturn(response); when(snapshotBuilderSettingsDao.getBySnapshotId(any())) .thenReturn(SnapshotBuilderTestData.SETTINGS); @@ -463,10 +464,8 @@ void testApproveRequest() { @Test void testGetRequest() { - UUID id = UUID.randomUUID(); - SnapshotAccessRequestModel daoResponse = - new SnapshotAccessRequestModel( - id, null, null, null, null, null, null, null, null, null, null); + var daoResponse = SnapshotBuilderTestData.createAccessRequest(); + UUID id = daoResponse.id(); when(snapshotRequestDao.getById(id)).thenReturn(daoResponse); when(snapshotBuilderSettingsDao.getBySnapshotId(any())) .thenReturn(SnapshotBuilderTestData.SETTINGS); @@ -487,9 +486,7 @@ void testDeleteRequest() { void testEnumerateRequestsBySnapshot() { UUID id = UUID.randomUUID(); List daoResponse = - List.of( - new SnapshotAccessRequestModel( - null, null, null, null, null, null, null, null, null, null, null)); + List.of(SnapshotBuilderTestData.createAccessRequest()); when(snapshotRequestDao.enumerateBySnapshot(id)).thenReturn(daoResponse); when(snapshotBuilderSettingsDao.getBySnapshotId(any())) .thenReturn(SnapshotBuilderTestData.SETTINGS); @@ -560,4 +557,29 @@ private static SnapshotBuilderService.ParentQueryResult createResult( concept.getCount(), concept.isHasChildren()); } + + @Test + void createExportSnapshotLink() { + UUID snapshotId = UUID.randomUUID(); + var link = snapshotBuilderService.createExportSnapshotLink(snapshotId); + assertThat( + link, + allOf( + containsString(terraConfiguration.basePath()), containsString(snapshotId.toString()))); + } + + @Test + void notifySnapshotReady() { + var request = SnapshotBuilderTestData.createAccessRequest(); + when(snapshotRequestDao.getById(request.id())).thenReturn(request); + var snapshot = new Snapshot().name("name"); + when(snapshotService.retrieve(request.createdSnapshotId())).thenReturn(snapshot); + when(snapshotBuilderSettingsDao.getBySnapshotId(request.sourceSnapshotId())) + .thenReturn(SnapshotBuilderTestData.SETTINGS); + String id = "id"; + snapshotBuilderService.notifySnapshotReady(id, request.id()); + verify(notificationService) + .snapshotReady( + eq(id), anyString(), eq(snapshot.getName()), eq("No snapshot specification found")); + } } diff --git a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java index 1ce91425ee..a9b15acde8 100644 --- a/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java +++ b/src/test/java/bio/terra/service/snapshotbuilder/SnapshotBuilderTestData.java @@ -445,4 +445,19 @@ public static SnapshotRequestModel createSnapshotRequestByRequestId( .requestIdSpec( new SnapshotRequestIdModel().snapshotRequestId(snapshotAccessRequestId)))); } + + public static SnapshotAccessRequestModel createAccessRequest() { + return new SnapshotAccessRequestModel( + UUID.randomUUID(), + null, + null, + UUID.randomUUID(), + null, + "email@a.com", + null, + null, + SnapshotAccessRequestStatus.APPROVED, + null, + null); + } } diff --git a/src/test/java/bio/terra/service/tabulardata/google/BigQueryPdaoTest.java b/src/test/java/bio/terra/service/tabulardata/google/BigQueryPdaoTest.java index 4ed8058c53..764aec5920 100644 --- a/src/test/java/bio/terra/service/tabulardata/google/BigQueryPdaoTest.java +++ b/src/test/java/bio/terra/service/tabulardata/google/BigQueryPdaoTest.java @@ -90,6 +90,7 @@ import java.util.UUID; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; +import org.broadinstitute.dsde.workbench.client.sam.model.UserIdInfo; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -397,6 +398,8 @@ public void createSnapshotByRequestId() throws Exception { when(samService.getGroup(any(), any())) .thenThrow(new IamNotFoundException(new Throwable("Group not found"))); when(samService.createGroup(any(), any())).thenReturn("group@firecloud.org"); + when(samService.getUserIds(any(), any())) + .thenReturn(new UserIdInfo().userSubjectId("subjectId")); Snapshot sourceSnapshot = stageOmopData(); SnapshotAccessRequestResponse approvedAccessRequest =