PEPPER-923 . cleanup/refactor

broadinstitute · Nov 7, 2024 · 7a75fae · 7a75fae
1 parent 1253000
commit 7a75fae
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 40 deletions.
diff --git a/...core/src/main/java/org/broadinstitute/dsm/service/onchistory/ESParticipantIdProvider.java b/...core/src/main/java/org/broadinstitute/dsm/service/onchistory/ESParticipantIdProvider.java
@@ -28,6 +28,18 @@ public ESParticipantIdProvider(String realm, String participantIndex) {
      * @throws DSMBadRequestException when no participant ID is found for short ID
      */
     public int getParticipantIdForShortId(String shortId) {
+        //getParticipantDataForShortId does all the validations to make sure participant ID exists
+        return getParticipantDataForShortId(shortId).getDsm().get().getParticipant().get().getParticipantId().intValue();
+    }
+
+    /**
+     * Given a participant short ID return a participant ID
+     * @throws DsmInternalError for bad ES behavior
+     * @throws DSMBadRequestException when no participant ID is found for short ID
+     */
+    public ElasticSearchParticipantDto getParticipantDataForShortId(String shortId) {
+        ElasticSearchParticipantDto ptpData = elasticSearchService.getParticipantDocumentByShortId(shortId, participantIndex)
+                .orElseThrow(() -> new DSMBadRequestException("No participant found for shortId " + shortId));
         Dsm dsm = elasticSearchService.getParticipantDsmByShortId(shortId, participantIndex);
         Optional<Participant> dsmParticipant = dsm.getParticipant();
         if (dsmParticipant.isEmpty()) {
@@ -38,19 +50,10 @@ public int getParticipantIdForShortId(String shortId) {
             throw new DsmInternalError("ES returned empty dsm.participant.participantId object for shortId " + shortId);
         }
         try {
-            return participantID.intValue();
+            return ptpData;
         } catch (Exception e) {
             throw new DsmInternalError("Invalid dsm.participant.participantId for shortId " + shortId);
         }
     }
 
-    /**
-     * Given a participant short ID return a participant ID
-     * @throws DsmInternalError for bad ES behavior
-     * @throws DSMBadRequestException when no participant ID is found for short ID
-     */
-    public Optional<ElasticSearchParticipantDto> getParticipantDataForShortId(String shortId) {
-        return elasticSearchService.getParticipantDocumentByShortId(shortId, participantIndex);
-    }
-
 }
diff --git a/...core/src/main/java/org/broadinstitute/dsm/service/onchistory/OncHistoryUploadService.java b/...core/src/main/java/org/broadinstitute/dsm/service/onchistory/OncHistoryUploadService.java
@@ -15,15 +15,13 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
 import com.google.gson.JsonObject;
 import lombok.extern.slf4j.Slf4j;
 import org.broadinstitute.dsm.db.FieldSettings;
 import org.broadinstitute.dsm.db.OncHistoryDetail;
-import org.broadinstitute.dsm.db.Participant;
 import org.broadinstitute.dsm.db.dao.ddp.instance.DDPInstanceDao;
 import org.broadinstitute.dsm.db.dao.ddp.participant.ParticipantDao;
 import org.broadinstitute.dsm.db.dao.settings.FieldSettingsDao;
@@ -33,7 +31,6 @@
 import org.broadinstitute.dsm.exception.DSMBadRequestException;
 import org.broadinstitute.dsm.exception.DsmInternalError;
 import org.broadinstitute.dsm.files.parser.onchistory.OncHistoryParser;
-import org.broadinstitute.dsm.model.elastic.Dsm;
 import org.broadinstitute.dsm.model.elastic.converters.camelcase.CamelCaseConverter;
 import org.broadinstitute.dsm.model.elastic.search.ElasticSearchParticipantDto;
 import org.broadinstitute.lddp.db.SimpleResult;
@@ -141,8 +138,7 @@ protected Map<Integer, Integer> getParticipantIds(List<OncHistoryRecord> oncHist
         ParticipantDao participantDao = ParticipantDao.of();
 
         for (OncHistoryRecord rec : oncHistoryRecords) {
-            ElasticSearchParticipantDto ptpData = participantIdProvider.getParticipantDataForShortId(
-                    rec.getParticipantTextId()).orElseThrow(() -> new OncHistoryValidationException("Invalid short ID " + rec.getParticipantTextId()));
+            ElasticSearchParticipantDto ptpData = participantIdProvider.getParticipantDataForShortId(rec.getParticipantTextId());
             if (ptpData.getStatus().isPresent() && ptpData.getStatus().get().startsWith("EXITED")) {
                 exitedParticipants.add(rec.getParticipantTextId());
             }
@@ -152,7 +148,8 @@ protected Map<Integer, Integer> getParticipantIds(List<OncHistoryRecord> oncHist
                 continue;
             }
 
-            int participantId = getParticipantIdFromElasticDoc(ptpData, rec.getParticipantTextId());
+            //since participantIdProvider.getParticipantDataForShortId already made sure participant ID exists.. its ok to get it directly
+            int participantId = ptpData.getDsm().get().getParticipant().get().getParticipantId().intValue();
             try {
                 ParticipantDto participant = participantDao.get(participantId).orElseThrow();
                 rec.setDdpParticipantId(participant.getDdpParticipantId().orElseThrow());
@@ -178,27 +175,6 @@ protected Map<Integer, Integer> getParticipantIds(List<OncHistoryRecord> oncHist
         return medIds;
     }
 
-    private int getParticipantIdFromElasticDoc(ElasticSearchParticipantDto ptpData, String shortId) {
-        Optional<Dsm> dsm = ptpData.getDsm();
-        if (dsm.isEmpty()) {
-            throw new DsmInternalError("Dsm object is empty for shortId " + shortId);
-        }
-
-        Optional<Participant> dsmParticipant = dsm.get().getParticipant();
-        if (dsmParticipant.isEmpty()) {
-            throw new DsmInternalError("ES returned empty dsm.participant object for shortId " + shortId);
-        }
-        Long participantID = dsmParticipant.get().getParticipantId();
-        if (participantID == null) {
-            throw new DsmInternalError("ES returned empty dsm.participant.participantId object for shortId " + shortId);
-        }
-        try {
-            return participantID.intValue();
-        } catch (Exception e) {
-            throw new DsmInternalError("Invalid dsm.participant.participantId for shortId " + shortId);
-        }
-    }
-
     /**
      * validate row contents and convert certain columns to json additionalValues
      */

diff --git a/...m-core/src/main/java/org/broadinstitute/dsm/service/onchistory/ParticipantIdProvider.java b/...m-core/src/main/java/org/broadinstitute/dsm/service/onchistory/ParticipantIdProvider.java
@@ -11,5 +11,5 @@ public interface ParticipantIdProvider {
      */
     int getParticipantIdForShortId(String shortId);
 
-    Optional<ElasticSearchParticipantDto> getParticipantDataForShortId(String shortId);
+    ElasticSearchParticipantDto getParticipantDataForShortId(String shortId);
 }
diff --git a/.../src/test/java/org/broadinstitute/dsm/service/onchistory/OncHistoryUploadServiceTest.java b/.../src/test/java/org/broadinstitute/dsm/service/onchistory/OncHistoryUploadServiceTest.java
@@ -431,7 +431,7 @@ public int getParticipantIdForShortId(String shortId) {
         }
 
         @Override
-        public Optional<ElasticSearchParticipantDto> getParticipantDataForShortId(String shortId) {
+        public ElasticSearchParticipantDto getParticipantDataForShortId(String shortId) {
             Dsm dsm = new Dsm();
             Participant participant = new Participant();
             participant.setParticipantId(shortIdToId.get(shortId).longValue());
@@ -444,7 +444,7 @@ public Optional<ElasticSearchParticipantDto> getParticipantDataForShortId(String
             ElasticSearchParticipantDto dto = new ElasticSearchParticipantDto.Builder()
                     .withDsm(dsm)
                     .withStatus(participantStatus).build();
-            return Optional.of(dto);
+            return dto;
         }
 
     }