[DUOS-3056][risk=no] Log ES errors instead of failing (#2287)

Author: rushtong

src/main/java/org/broadinstitute/consent/http/service/DatasetRegistrationService.java

@@ -3,6 +3,7 @@
 import com.google.cloud.storage.BlobId;
 import jakarta.ws.rs.BadRequestException;
 import jakarta.ws.rs.NotFoundException;
+import jakarta.ws.rs.core.Response;
 import java.io.IOException;
 import java.io.InputStream;
 import java.sql.SQLException;
@@ -194,7 +195,13 @@ public List<Dataset> createDatasetsFromRegistration(
 
     List<Dataset> datasets = datasetDAO.findDatasetsByIdList(createdDatasetIds);
     sendDatasetSubmittedEmails(datasets);
-    elasticSearchService.indexDatasets(datasets);
+    try (Response response = elasticSearchService.indexDatasets(datasets)) {
+      if (response.getStatus() >= 400) {
+        logWarn(String.format("Error indexing datasets from registration: %s", registration.getStudyName()));
+      }
+    } catch (Exception e) {
+      logException(e);
+    }
     return datasets;
   }
 
@@ -266,7 +273,13 @@ public Dataset updateDataset(
     }
 
     Dataset updatedDataset = datasetDAO.findDatasetById(datasetId);
-    elasticSearchService.indexDataset(updatedDataset);
+    try (Response response = elasticSearchService.indexDataset(dataset)) {
+      if (response.getStatus() >= 400) {
+        logWarn(String.format("Error indexing dataset update: %s", dataset.getName()));
+      }
+    } catch (Exception e) {
+      logException(e);
+    }
     return updatedDataset;
   }
 

src/test/java/org/broadinstitute/consent/http/service/DatasetRegistrationServiceTest.java

@@ -1,5 +1,6 @@
 package org.broadinstitute.consent.http.service;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -15,6 +16,7 @@
 
 import com.google.cloud.storage.BlobId;
 import jakarta.ws.rs.NotFoundException;
+import jakarta.ws.rs.ServerErrorException;
 import jakarta.ws.rs.core.MediaType;
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
@@ -30,6 +32,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.commons.lang3.RandomUtils;
 import org.broadinstitute.consent.http.cloudstore.GCSService;
 import org.broadinstitute.consent.http.db.DacDAO;
 import org.broadinstitute.consent.http.db.DatasetDAO;
@@ -57,14 +60,15 @@
 import org.broadinstitute.consent.http.util.gson.GsonUtil;
 import org.glassfish.jersey.media.multipart.FormDataBodyPart;
 import org.glassfish.jersey.media.multipart.FormDataContentDisposition;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Captor;
 import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
+import org.mockito.junit.jupiter.MockitoExtension;
 
-public class DatasetRegistrationServiceTest {
+@ExtendWith(MockitoExtension.class)
+class DatasetRegistrationServiceTest {
 
   private DatasetRegistrationService datasetRegistrationService;
 
@@ -89,11 +93,6 @@ public class DatasetRegistrationServiceTest {
   @Mock
   private EmailService emailService;
 
-  @BeforeEach
-  public void setUp() {
-    MockitoAnnotations.openMocks(this);
-  }
-
   private void initService() {
     datasetRegistrationService = new DatasetRegistrationService(datasetDAO, dacDAO,
         datasetServiceDAO, gcsService, elasticSearchService, studyDAO, emailService);
@@ -108,15 +107,26 @@ private void initService() {
 
   // ------------------------ test multiple dataset insert ----------------------------------- //
   @Test
-  public void testInsertCompleteDatasetRegistration() throws Exception {
+  void testInsertCompleteDatasetRegistration() throws Exception {
     User user = mock();
     DatasetRegistrationSchemaV1 schema = createRandomCompleteDatasetRegistration(user);
 
+    FormDataContentDisposition content = FormDataContentDisposition
+        .name("file")
+        .fileName("sharing_plan.txt")
+        .build();
+
+    InputStream is = new ByteArrayInputStream("HelloWorld".getBytes(StandardCharsets.UTF_8));
+    FormDataBodyPart bodyPart = mock();
+    when(bodyPart.getMediaType()).thenReturn(MediaType.TEXT_PLAIN_TYPE);
+    when(bodyPart.getContentDisposition()).thenReturn(content);
+    when(bodyPart.getValueAs(any())).thenReturn(is);
+
     initService();
 
     Map<String, FormDataBodyPart> files = Map.of("alternativeDataSharingPlan",
-        createFormDataBodyPart(), "consentGroups[0].nihInstitutionalCertificationFile",
-        createFormDataBodyPart(), "otherUnused", createFormDataBodyPart());
+        bodyPart, "consentGroups[0].nihInstitutionalCertificationFile",
+        bodyPart, "otherUnused", bodyPart);
     when(gcsService.storeDocument(any(), any(), any())).thenReturn(BlobId.of("asdf", "hjkl"),
         BlobId.of("qwer", "tyuio"));
     when(dacDAO.findById(any())).thenReturn(new Dac());
@@ -237,7 +247,7 @@ public void testInsertCompleteDatasetRegistration() throws Exception {
 
   // inserts only required fields to ensure that null fields are ok
   @Test
-  public void testInsertMinimumDatasetRegistration() throws Exception {
+  void testInsertMinimumDatasetRegistration() throws Exception {
     User user = mock();
     DatasetRegistrationSchemaV1 schema = createRandomMinimumDatasetRegistration(user);
 
@@ -288,7 +298,7 @@ public void testInsertMinimumDatasetRegistration() throws Exception {
   }
 
   @Test
-  public void testDatasetCreateRegistrationEmails() throws Exception {
+  void testDatasetCreateRegistrationEmails() throws Exception {
     User user = mock();
     DatasetRegistrationSchemaV1 schema = createRandomCompleteDatasetRegistration(user);
 
@@ -301,7 +311,7 @@ public void testDatasetCreateRegistrationEmails() throws Exception {
   }
 
   @Test
-  public void testStudyUpdateNewDatasetEmails() throws Exception {
+  void testStudyUpdateNewDatasetEmails() throws Exception {
     User user = mock();
     DatasetRegistrationSchemaV1 schema = createRandomCompleteDatasetRegistration(user);
     Study study = mock();
@@ -318,7 +328,7 @@ public void testStudyUpdateNewDatasetEmails() throws Exception {
   }
 
   @Test
-  public void testSendDatasetSubmittedEmailsExistingChairs() throws Exception {
+  void testSendDatasetSubmittedEmailsExistingChairs() throws Exception {
     User user = new User();
     user.setRoles(List.of(new UserRole(UserRoles.CHAIRPERSON.getRoleId(), UserRoles.CHAIRPERSON.getRoleName())));
     Dac dac = mock();
@@ -334,7 +344,7 @@ public void testSendDatasetSubmittedEmailsExistingChairs() throws Exception {
   }
 
   @Test
-  public void testSendDatasetSubmittedEmailsNoChairs() throws Exception {
+  void testSendDatasetSubmittedEmailsNoChairs() throws Exception {
     Dac dac = mock();
     Dataset dataset = new Dataset();
     dataset.setDacId(1);
@@ -348,7 +358,7 @@ public void testSendDatasetSubmittedEmailsNoChairs() throws Exception {
   }
 
   @Test
-  public void testCreatedDatasetsFromUpdatedStudy() {
+  void testCreatedDatasetsFromUpdatedStudy() {
     Study study = mock();
     Set<Dataset> allDatasets = Stream.of(1, 2, 3, 4, 5).map((i) -> {
       Dataset dataset = new Dataset();
@@ -373,7 +383,7 @@ public void testCreatedDatasetsFromUpdatedStudy() {
   }
 
   @Test
-  public void testCreatedDatasetsFromUpdatedStudyNoDatasets() {
+  void testCreatedDatasetsFromUpdatedStudyNoDatasets() {
     Study study = mock();
     List<DatasetUpdate> updatedDatasets = null;
     initService();
@@ -384,7 +394,7 @@ public void testCreatedDatasetsFromUpdatedStudyNoDatasets() {
   }
 
   @Test
-  public void testInsertAccessManagement() throws Exception {
+  void testInsertAccessManagement() throws Exception {
     User user = mock();
     DatasetRegistrationSchemaV1 schema = createAccessManagementRegistrationNoDacId(user);
 
@@ -406,16 +416,27 @@ public void testInsertAccessManagement() throws Exception {
 
   // test inset multiple consent groups
   @Test
-  public void testInsertMultipleDatasetRegistration() throws Exception {
+  void testInsertMultipleDatasetRegistration() throws Exception {
     User user = mock();
     DatasetRegistrationSchemaV1 schema = createRandomMultipleDatasetRegistration(user);
 
+    FormDataContentDisposition content = FormDataContentDisposition
+        .name("file")
+        .fileName("sharing_plan.txt")
+        .build();
+
+    InputStream is = new ByteArrayInputStream("HelloWorld".getBytes(StandardCharsets.UTF_8));
+    FormDataBodyPart bodyPart = mock();
+    when(bodyPart.getMediaType()).thenReturn(MediaType.TEXT_PLAIN_TYPE);
+    when(bodyPart.getContentDisposition()).thenReturn(content);
+    when(bodyPart.getValueAs(any())).thenReturn(is);
+
     initService();
 
     when(dacDAO.findById(any())).thenReturn(new Dac());
     Map<String, FormDataBodyPart> files = Map.of("alternativeDataSharingPlan",
-        createFormDataBodyPart(), "consentGroups[0].nihInstitutionalCertificationFile",
-        createFormDataBodyPart(), "otherUnused", createFormDataBodyPart());
+        bodyPart, "consentGroups[0].nihInstitutionalCertificationFile",
+        bodyPart, "otherUnused", bodyPart);
     when(gcsService.storeDocument(any(), any(), any())).thenReturn(BlobId.of("asdf", "hjkl"),
         BlobId.of("qwer", "tyuio"));
 
@@ -494,7 +515,7 @@ public void testInsertMultipleDatasetRegistration() throws Exception {
   }
 
   @Test
-  public void testRegistrationErrorsOnInvalidDacId() throws Exception {
+  void testRegistrationErrorsOnInvalidDacId() throws Exception {
 
     User user = mock();
     DatasetRegistrationSchemaV1 schema = createRandomMinimumDatasetRegistration(user);
@@ -508,7 +529,40 @@ public void testRegistrationErrorsOnInvalidDacId() throws Exception {
   }
 
   @Test
-  public void testExtractStudyProperty() {
+  void testRegistrationSucceedsWithESError() throws Exception {
+    User user = mock();
+    DatasetRegistrationSchemaV1 schema = createRandomMinimumDatasetRegistration(user);
+    when(dacDAO.findById(any())).thenReturn(new Dac());
+    when(elasticSearchService.indexDatasets(any())).thenThrow(new ServerErrorException("Timeout connecting to [elasticsearch]", 500));
+    initService();
+    assertDoesNotThrow(() -> {
+      datasetRegistrationService.createDatasetsFromRegistration(schema, user, Map.of());
+    }, "Registration Error");
+  }
+
+  @Test
+  void testUpdateDatasetSucceedsWithESError() {
+    User user = mock();
+    Dac dac = new Dac();
+    dac.setDacId(RandomUtils.nextInt(1, 100));
+    Dataset dataset = new Dataset();
+    dataset.setDataSetId(RandomUtils.nextInt(1, 100));
+    dataset.setDacId(dac.getDacId());
+    String name = RandomStringUtils.randomAlphabetic(10);
+    org.broadinstitute.consent.http.models.DatasetUpdate update = new org.broadinstitute.consent.http.models.DatasetUpdate(
+        name,
+        dac.getDacId(),
+        List.of());
+    when(datasetDAO.findDatasetById(any())).thenReturn(dataset);
+
+    initService();
+    assertDoesNotThrow(() -> {
+      datasetRegistrationService.updateDataset(dataset.getDataSetId(), user, update, Map.of());
+    }, "Update Error");
+  }
+
+  @Test
+  void testExtractStudyProperty() {
     DatasetRegistrationService.StudyPropertyExtractor extractor = new DatasetRegistrationService.StudyPropertyExtractor(
         RandomStringUtils.randomAlphabetic(10),
         PropertyType.String,
@@ -533,7 +587,7 @@ public void testExtractStudyProperty() {
   }
 
   @Test
-  public void testExtractDatasetProperty() {
+  void testExtractDatasetProperty() {
     DatasetRegistrationService.DatasetPropertyExtractor extractor = new DatasetRegistrationService.DatasetPropertyExtractor(
         RandomStringUtils.randomAlphabetic(10),
         RandomStringUtils.randomAlphabetic(10),
@@ -561,7 +615,7 @@ public void testExtractDatasetProperty() {
 
 
   @Test
-  public void testExtractStudyPropertyTyped() {
+  void testExtractStudyPropertyTyped() {
     DatasetRegistrationService.StudyPropertyExtractor extractor = new DatasetRegistrationService.StudyPropertyExtractor(
         RandomStringUtils.randomAlphabetic(10),
         PropertyType.Json,
@@ -583,7 +637,7 @@ public void testExtractStudyPropertyTyped() {
   }
 
   @Test
-  public void testExtractDatasetPropertyTyped() {
+  void testExtractDatasetPropertyTyped() {
     DatasetRegistrationService.DatasetPropertyExtractor extractor = new DatasetRegistrationService.DatasetPropertyExtractor(
         RandomStringUtils.randomAlphabetic(10),
         RandomStringUtils.randomAlphabetic(10),
@@ -643,21 +697,6 @@ private void assertContainsStudyProperty(Collection<StudyProperty> props, String
     assertEquals(value, prop.get().getValue());
   }
 
-
-  private FormDataBodyPart createFormDataBodyPart() {
-    FormDataContentDisposition content = FormDataContentDisposition
-        .name("file")
-        .fileName("sharing_plan.txt")
-        .build();
-
-    InputStream is = new ByteArrayInputStream("HelloWorld".getBytes(StandardCharsets.UTF_8));
-    FormDataBodyPart bodyPart = mock();
-    when(bodyPart.getMediaType()).thenReturn(MediaType.TEXT_PLAIN_TYPE);
-    when(bodyPart.getContentDisposition()).thenReturn(content);
-    when(bodyPart.getValueAs(any())).thenReturn(is);
-    return bodyPart;
-  }
-
   private DatasetRegistrationSchemaV1 createRandomMinimumDatasetRegistration(User user) {
     DatasetRegistrationSchemaV1 schemaV1 = new DatasetRegistrationSchemaV1();
     schemaV1.setStudyName(RandomStringUtils.randomAlphabetic(10));