From 5bda44a80e40f6d2f1266321fb911ae53f6d5774 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 13 Feb 2024 16:32:51 -0500 Subject: [PATCH 01/40] [BI-2046] add validator for exp import on obsUnitID for existing studies --- .../processors/ExperimentProcessor.java | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 4ea10c07a..9fcd1b501 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -118,7 +118,7 @@ public class ExperimentProcessor implements Processor { private final Map seasonDbIdToYearCache = new HashMap<>(); //These BrapiData-objects are initially populated by the getExistingBrapiData() method, - // then updated by the getNewBrapiData() method. + // then updated by the initNewBrapiData() method. private Map> trialByNameNoScope = null; private Map> locationByName = null; private Map> studyByNameNoScope = null; @@ -129,7 +129,7 @@ public class ExperimentProcessor implements Processor { private final Map> observationByHash = new HashMap<>(); private Map existingObsByObsHash = new HashMap<>(); - // existingGermplasmByGID is populated by getExistingBrapiData(), but not updated by the getNewBrapiData() method + // existingGermplasmByGID is populated by getExistingBrapiData(), but not updated by the initNewBrapiData() method private Map> existingGermplasmByGID = null; // Associates timestamp columns to associated phenotype column name for ease of storage @@ -598,13 +598,6 @@ private void validateFields(List importRows, ValidationErrors valid validateGermplasm(importRow, validationErrors, rowNum, mappedImportRow.getGermplasm()); } validateTestOrCheck(importRow, validationErrors, rowNum); - //TODO: providing obs unit ID does not supersede import row inout data as expected and needs to be fixed - //Check if existing environment. If so, ObsUnitId must be assigned -// if ((mappedImportRow.getStudy().getState() == ImportObjectState.EXISTING) -// && (StringUtils.isBlank(importRow.getObsUnitID()))) { -// throw new MissingRequiredInfoException(MISSING_OBS_UNIT_ID_ERROR); -// } - validateConditionallyRequired(validationErrors, rowNum, importRow, program, commit); validateObservationUnits(validationErrors, uniqueStudyAndObsUnit, rowNum, importRow); validateObservations(validationErrors, rowNum, importRow, phenotypeCols, colVarMap, commit, user); @@ -810,8 +803,14 @@ private void validateConditionallyRequired(ValidationErrors validationErrors, in addRowError(Columns.OBS_UNIT_ID, "ObsUnitID cannot be specified when creating a new environment", validationErrors, rowNum); } } else { - //TODO: include this step once user-supplied obs unit id correctly supersedes other row data - //validateRequiredCell(importRow.getObsUnitID(), Columns.OBS_UNIT_ID, errorMessage, validationErrors, rowNum); + //Check if existing environment. If so, ObsUnitId must be assigned + validateRequiredCell( + importRow.getObsUnitID(), + Columns.OBS_UNIT_ID, + "Experimental entities are missing ObsUnitIDs. Import cannot proceed", + validationErrors, + rowNum + ); } } @@ -1380,7 +1379,7 @@ private Map> initializeObserva BrAPIExternalReference idRef = Utilities.getExternalReference(brAPIObservationUnit.getExternalReferences(), refSource) .orElseThrow(() -> new InternalServerException("An ObservationUnit ID was not found in any of the external references")); - ExperimentObservation row = rowByObsUnitId.get(idRef.getReferenceID()); + ExperimentObservation row = rowByObsUnitId.get(idRef.getReferenceId()); row.setExpTitle(Utilities.removeProgramKey(brAPIObservationUnit.getTrialName(), program.getKey())); row.setEnv(Utilities.removeProgramKeyAndUnknownAdditionalData(brAPIObservationUnit.getStudyName(), program.getKey())); row.setEnvLocation(Utilities.removeProgramKey(brAPIObservationUnit.getLocationName(), program.getKey())); @@ -1600,12 +1599,12 @@ private void processAndCacheObservationUnit(BrAPIObservationUnit brAPIObservatio BrAPIExternalReference idRef = Utilities.getExternalReference(brAPIObservationUnit.getExternalReferences(), refSource) .orElseThrow(() -> new InternalServerException("An ObservationUnit ID was not found in any of the external references")); - ExperimentObservation row = rowByObsUnitId.get(idRef.getReferenceID()); + ExperimentObservation row = rowByObsUnitId.get(idRef.getReferenceId()); row.setExpUnitId(Utilities.removeProgramKeyAndUnknownAdditionalData(brAPIObservationUnit.getObservationUnitName(), program.getKey())); observationUnitByName.put(createObservationUnitKey(row), new PendingImportObject<>(ImportObjectState.EXISTING, brAPIObservationUnit, - UUID.fromString(idRef.getReferenceID()))); + UUID.fromString(idRef.getReferenceId()))); } private void processAndCacheStudy(BrAPIStudy existingStudy, Program program, Map> studyByName) { From ba789170a3cce9cd17c0ebbf05188647df5f962e Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 14 Feb 2024 11:17:54 -0500 Subject: [PATCH 02/40] [BI-2046] update ref to obsolete brapi-client field --- .../importer/ExperimentFileImportTest.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 7a797a815..175f8584e 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -666,7 +666,7 @@ public void importNewObsVarExisingOu() { BrAPITrial brAPITrial = brAPITrialDAO.getTrialsByName(List.of((String)newExp.get(Columns.EXP_TITLE)), program).get(0); Optional trialIdXref = Utilities.getExternalReference(brAPITrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())); assertTrue(trialIdXref.isPresent()); - BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceID()), program).get(0); + BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceId()), program).get(0); BrAPIObservationUnit ou = ouDAO.getObservationUnitsForStudyDbId(brAPIStudy.getStudyDbId(), program).get(0); Optional ouIdXref = Utilities.getExternalReference(ou.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName())); @@ -686,7 +686,7 @@ public void importNewObsVarExisingOu() { newObsVar.put(Columns.BLOCK_NUM, "1"); newObsVar.put(Columns.ROW, "1"); newObsVar.put(Columns.COLUMN, "1"); - newObsVar.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceID()); + newObsVar.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); newObsVar.put(traits.get(1).getObservationVariableName(), null); JsonObject result = importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits), null, true, client, program, mappingId); @@ -731,7 +731,7 @@ public void importNewObsExisingOu(boolean commit) { BrAPITrial brAPITrial = brAPITrialDAO.getTrialsByName(List.of((String)newExp.get(Columns.EXP_TITLE)), program).get(0); Optional trialIdXref = Utilities.getExternalReference(brAPITrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())); assertTrue(trialIdXref.isPresent()); - BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceID()), program).get(0); + BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceId()), program).get(0); BrAPIObservationUnit ou = ouDAO.getObservationUnitsForStudyDbId(brAPIStudy.getStudyDbId(), program).get(0); Optional ouIdXref = Utilities.getExternalReference(ou.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName())); @@ -751,7 +751,7 @@ public void importNewObsExisingOu(boolean commit) { newObservation.put(Columns.BLOCK_NUM, "1"); newObservation.put(Columns.ROW, "1"); newObservation.put(Columns.COLUMN, "1"); - newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceID()); + newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); newObservation.put(traits.get(0).getObservationVariableName(), "1"); JsonObject result = importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits), null, commit, client, program, mappingId); @@ -798,7 +798,7 @@ public void verifyFailureImportNewObsExisingOuWithExistingObs(boolean commit) { BrAPITrial brAPITrial = brAPITrialDAO.getTrialsByName(List.of((String)newExp.get(Columns.EXP_TITLE)), program).get(0); Optional trialIdXref = Utilities.getExternalReference(brAPITrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())); assertTrue(trialIdXref.isPresent()); - BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceID()), program).get(0); + BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceId()), program).get(0); BrAPIObservationUnit ou = ouDAO.getObservationUnitsForStudyDbId(brAPIStudy.getStudyDbId(), program).get(0); Optional ouIdXref = Utilities.getExternalReference(ou.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName())); @@ -818,7 +818,7 @@ public void verifyFailureImportNewObsExisingOuWithExistingObs(boolean commit) { newObservation.put(Columns.BLOCK_NUM, "1"); newObservation.put(Columns.ROW, "1"); newObservation.put(Columns.COLUMN, "1"); - newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceID()); + newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); newObservation.put(traits.get(0).getObservationVariableName(), "2"); uploadAndVerifyFailure(program, importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits), traits.get(0).getObservationVariableName(), commit); @@ -925,7 +925,7 @@ public void importNewObsAfterFirstExpWithObs(boolean commit) { BrAPITrial brAPITrial = brAPITrialDAO.getTrialsByName(List.of((String)newExp.get(Columns.EXP_TITLE)), program).get(0); Optional trialIdXref = Utilities.getExternalReference(brAPITrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())); assertTrue(trialIdXref.isPresent()); - BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceID()), program).get(0); + BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceId()), program).get(0); BrAPIObservationUnit ou = ouDAO.getObservationUnitsForStudyDbId(brAPIStudy.getStudyDbId(), program).get(0); Optional ouIdXref = Utilities.getExternalReference(ou.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName())); @@ -945,7 +945,7 @@ public void importNewObsAfterFirstExpWithObs(boolean commit) { newObservation.put(Columns.BLOCK_NUM, "1"); newObservation.put(Columns.ROW, "1"); newObservation.put(Columns.COLUMN, "1"); - newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceID()); + newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); newObservation.put(traits.get(0).getObservationVariableName(), "1"); newObservation.put(traits.get(1).getObservationVariableName(), "2"); @@ -999,7 +999,7 @@ public void importNewObsAfterFirstExpWithObs_blank(boolean commit) { BrAPITrial brAPITrial = brAPITrialDAO.getTrialsByName(List.of((String)newExp.get(Columns.EXP_TITLE)), program).get(0); Optional trialIdXref = Utilities.getExternalReference(brAPITrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())); assertTrue(trialIdXref.isPresent()); - BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceID()), program).get(0); + BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceId()), program).get(0); BrAPIObservationUnit ou = ouDAO.getObservationUnitsForStudyDbId(brAPIStudy.getStudyDbId(), program).get(0); Optional ouIdXref = Utilities.getExternalReference(ou.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName())); @@ -1021,7 +1021,7 @@ public void importNewObsAfterFirstExpWithObs_blank(boolean commit) { newObservation.put(Columns.BLOCK_NUM, "1"); newObservation.put(Columns.ROW, "1"); newObservation.put(Columns.COLUMN, "1"); - newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceID()); + newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); newObservation.put(traits.get(0).getObservationVariableName(), ""); newObservation.put(traits.get(1).getObservationVariableName(), "2"); @@ -1053,7 +1053,7 @@ private Map assertRowSaved(Map expected, Program Optional trialIdXref = Utilities.getExternalReference(trial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())); assertTrue(trialIdXref.isPresent()); - List studies = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceID()), program); + List studies = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceId()), program); assertFalse(studies.isEmpty()); BrAPIStudy study = null; for(BrAPIStudy s : studies) { From ef4fea6c3ce999a7f4fbb47ee7c65e8d12ed291b Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 15 Feb 2024 10:55:44 -0500 Subject: [PATCH 03/40] [BI-2046] fix typos and obsolete refs --- .../importer/ExperimentFileImportTest.java | 86 +++++++++---------- 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 175f8584e..056e8b297 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -361,7 +361,6 @@ public void importNewEnvNoObsSuccess() { assertRowSaved(newEnv, program, null); } - @ParameterizedTest @ValueSource(booleans = {true, false}) @SneakyThrows @@ -599,50 +598,49 @@ public void verifyFailureImportNewExpWithInvalidObs(boolean commit) { uploadAndVerifyFailure(program, importTestUtils.writeExperimentDataToFile(List.of(newExp), traits), traits.get(0).getObservationVariableName(), commit); } - // NO Longer needed, but may be needed in the future. -// @ParameterizedTest -// @ValueSource(booleans = {true, false}) -// @SneakyThrows -// public void verifyFailureNewOuExistingEnv(boolean commit) { -// Program program = createProgram("New OU Exising Env "+(commit ? "C" : "P"), "FLOU"+(commit ? "C" : "P"), "FLOU"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(1), null); -// Map newExp = new HashMap<>(); -// newExp.put(Columns.GERMPLASM_GID, "1"); -// newExp.put(Columns.TEST_CHECK, "T"); -// newExp.put(Columns.EXP_TITLE, "Test Exp"); -// newExp.put(Columns.EXP_UNIT, "Plot"); -// newExp.put(Columns.EXP_TYPE, "Phenotyping"); -// newExp.put(Columns.ENV, "New Env"); -// newExp.put(Columns.ENV_LOCATION, "Location A"); -// newExp.put(Columns.ENV_YEAR, "2023"); -// newExp.put(Columns.EXP_UNIT_ID, "a-1"); -// newExp.put(Columns.REP_NUM, "1"); -// newExp.put(Columns.BLOCK_NUM, "1"); -// newExp.put(Columns.ROW, "1"); -// newExp.put(Columns.COLUMN, "1"); -// -// importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newExp), null), null, true, client, program, mappingId); -// -// Map newOU = new HashMap<>(newExp); -// newOU.put(Columns.EXP_UNIT_ID, "a-2"); -// newOU.put(Columns.ROW, "1"); -// newOU.put(Columns.COLUMN, "2"); -// -// Flowable> call = importTestUtils.uploadDataFile(importTestUtils.writeExperimentDataToFile(List.of(newOU), null), null, commit, client, program, mappingId); -// HttpResponse response = call.blockingFirst(); -// assertEquals(HttpStatus.ACCEPTED, response.getStatus()); -// -// String importId = JsonParser.parseString(response.body()).getAsJsonObject().getAsJsonObject("result").get("importId").getAsString(); -// -// HttpResponse upload = importTestUtils.getUploadedFile(importId, client, program, mappingId); -// JsonObject result = JsonParser.parseString(upload.body()).getAsJsonObject().getAsJsonObject("result"); -// assertEquals(422, result.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); -// -// assertTrue(result.getAsJsonObject("progress").get("message").getAsString().startsWith("Experimental entities are missing ObsUnitIDs")); -// } + @ParameterizedTest + @ValueSource(booleans = {true, false}) + @SneakyThrows + public void verifyFailureNewOuExistingEnv(boolean commit) { + Program program = createProgram("New OU Existing Env "+(commit ? "C" : "P"), "FLOU"+(commit ? "C" : "P"), "FLOU"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(1), null); + Map newExp = new HashMap<>(); + newExp.put(Columns.GERMPLASM_GID, "1"); + newExp.put(Columns.TEST_CHECK, "T"); + newExp.put(Columns.EXP_TITLE, "Test Exp"); + newExp.put(Columns.EXP_UNIT, "Plot"); + newExp.put(Columns.EXP_TYPE, "Phenotyping"); + newExp.put(Columns.ENV, "New Env"); + newExp.put(Columns.ENV_LOCATION, "Location A"); + newExp.put(Columns.ENV_YEAR, "2023"); + newExp.put(Columns.EXP_UNIT_ID, "a-1"); + newExp.put(Columns.REP_NUM, "1"); + newExp.put(Columns.BLOCK_NUM, "1"); + newExp.put(Columns.ROW, "1"); + newExp.put(Columns.COLUMN, "1"); + + importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newExp), null), null, true, client, program, mappingId); + + Map newOU = new HashMap<>(newExp); + newOU.put(Columns.EXP_UNIT_ID, "a-2"); + newOU.put(Columns.ROW, "1"); + newOU.put(Columns.COLUMN, "2"); + + Flowable> call = importTestUtils.uploadDataFile(importTestUtils.writeExperimentDataToFile(List.of(newOU), null), null, commit, client, program, mappingId); + HttpResponse response = call.blockingFirst(); + assertEquals(HttpStatus.ACCEPTED, response.getStatus()); + + String importId = JsonParser.parseString(response.body()).getAsJsonObject().getAsJsonObject("result").get("importId").getAsString(); + + HttpResponse upload = importTestUtils.getUploadedFile(importId, client, program, mappingId); + JsonObject result = JsonParser.parseString(upload.body()).getAsJsonObject().getAsJsonObject("result"); + assertEquals(422, result.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); + + assertTrue(result.getAsJsonObject("progress").get("message").getAsString().startsWith("Experimental entities are missing ObsUnitIDs")); + } @Test @SneakyThrows - public void importNewObsVarExisingOu() { + public void importNewObsVarExistingOu() { List traits = importTestUtils.createTraits(2); Program program = createProgram("New ObsVar Existing OU", "OUVAR", "OUVAR", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); Map newExp = new HashMap<>(); @@ -708,7 +706,7 @@ public void importNewObsVarExisingOu() { @ParameterizedTest @ValueSource(booleans = {true, false}) @SneakyThrows - public void importNewObsExisingOu(boolean commit) { + public void importNewObsExistingOu(boolean commit) { List traits = importTestUtils.createTraits(1); Program program = createProgram("New Obs Existing OU "+(commit ? "C" : "P"), "OUOBS"+(commit ? "C" : "P"), "OUOBS"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); Map newExp = new HashMap<>(); @@ -774,7 +772,7 @@ public void importNewObsExisingOu(boolean commit) { @ParameterizedTest @ValueSource(booleans = {true, false}) @SneakyThrows - public void verifyFailureImportNewObsExisingOuWithExistingObs(boolean commit) { + public void verifyFailureImportNewObsExistingOuWithExistingObs(boolean commit) { List traits = importTestUtils.createTraits(1); Program program = createProgram("New Obs Existing Obs "+(commit ? "C" : "P"), "FEXOB"+(commit ? "C" : "P"), "FEXOB"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); Map newExp = new HashMap<>(); From b7836323ee4f7639ee268a7a408cd3ec9cb09653 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 26 Feb 2024 16:04:26 -0500 Subject: [PATCH 04/40] [BI-2046] set pendingObsUnitByOUId --- .../importer/services/FileMappingUtil.java | 14 +- .../processors/ExperimentProcessor.java | 258 +++++++++++++++++- .../importer/ExperimentFileImportTest.java | 64 +++++ 3 files changed, 318 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/FileMappingUtil.java b/src/main/java/org/breedinginsight/brapps/importer/services/FileMappingUtil.java index e603a7d21..a3e96164a 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/FileMappingUtil.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/FileMappingUtil.java @@ -27,10 +27,7 @@ import javax.inject.Inject; import javax.inject.Singleton; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; @Singleton public class FileMappingUtil { @@ -94,4 +91,13 @@ public List sortByField(List sortedFields, List unsortedItems, return unsortedItems; } + + public boolean isValidUUID(String id) { + try { + UUID.fromString(id); + return true; + } catch (IllegalArgumentException e) { + return false; + } + } } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 9fcd1b501..148e139ce 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -34,6 +34,7 @@ import org.brapi.v2.model.core.*; import org.brapi.v2.model.core.request.BrAPIListNewRequest; import org.brapi.v2.model.core.response.BrAPIListDetails; +import org.brapi.v2.model.geno.BrAPIReference; import org.brapi.v2.model.germ.BrAPIGermplasm; import org.brapi.v2.model.pheno.BrAPIObservation; import org.brapi.v2.model.pheno.BrAPIObservationUnit; @@ -83,7 +84,7 @@ public class ExperimentProcessor implements Processor { private static final String NAME = "Experiment"; - private static final String MISSING_OBS_UNIT_ID_ERROR = "Experimental entities are missing ObsUnitIDs"; + private static final String MISSING_OBS_UNIT_ID_ERROR = "Experimental entities are missing ObsUnitIDs. Import cannot proceed"; private static final String PREEXISTING_EXPERIMENT_TITLE = "Experiment Title already exists"; private static final String MULTIPLE_EXP_TITLES = "File contains more than one Experiment Title"; private static final String MIDNIGHT = "T00:00:00-00:00"; @@ -120,12 +121,17 @@ public class ExperimentProcessor implements Processor { //These BrapiData-objects are initially populated by the getExistingBrapiData() method, // then updated by the initNewBrapiData() method. private Map> trialByNameNoScope = null; + private Map> pendingTrialByOUId = null; private Map> locationByName = null; + private Map> pendingLocationByOUId = null; private Map> studyByNameNoScope = null; + private Map> pendingStudyByOUId = null; private Map> obsVarDatasetByName = null; + private Map> pendingObsDatasetByOUId = null; // It is assumed that there are no preexisting Observation Units for the given environment (so this will not be // initialized by getExistingBrapiData() ) private Map> observationUnitByNameNoScope = null; + private Map> pendingObsUnitByOUId = null; private final Map> observationByHash = new HashMap<>(); private Map existingObsByObsHash = new HashMap<>(); @@ -135,6 +141,9 @@ public class ExperimentProcessor implements Processor { // Associates timestamp columns to associated phenotype column name for ease of storage private final Map> timeStampColByPheno = new HashMap<>(); private final Gson gson; + private boolean hasAllReferenceUnitIds = true; + private boolean hasNoReferenceUnitIds = true; + private Set referenceOUIds = new HashSet<>(); @Inject public ExperimentProcessor(DSLContext dsl, @@ -179,12 +188,47 @@ public void getExistingBrapiData(List importRows, Program program) .map(trialImport -> (ExperimentObservation) trialImport) .collect(Collectors.toList()); - this.observationUnitByNameNoScope = initializeObservationUnits(program, experimentImportRows); - this.trialByNameNoScope = initializeTrialByNameNoScope(program, experimentImportRows); - this.studyByNameNoScope = initializeStudyByNameNoScope(program, experimentImportRows); - this.locationByName = initializeUniqueLocationNames(program, experimentImportRows); - this.obsVarDatasetByName = initializeObsVarDatasetByName(program, experimentImportRows); - this.existingGermplasmByGID = initializeExistingGermplasmByGID(program, experimentImportRows); + Map referenceUnitById = new HashMap<>(); + Map expTitleByRefUnitId = new HashMap<>(); + // check for references to Deltabreed-generated observation units + setReferenceOUIds(importRows); + + if (hasAllReferenceUnitIds) { + + // get all prior units referenced in import + try { + List referenceOUs = brAPIObservationUnitDAO.getObservationUnitsById(new ArrayList(referenceOUIds), program); + for (BrAPIObservationUnit unit: referenceOUs) { + setPendingObsUnitByOUId(program); + setPendingTrialByOUId(program); + + // set pendingStudiesByOUId + // set pendingLocationsByOUId + // set pendingObsDatasetByOUId + + BrAPIExternalReference xref = Utilities.getExternalReference( + unit.getExternalReferences(), + ExternalReferenceSource.OBSERVATION_UNITS.getName() + ).orElseThrow(() -> new IllegalStateException("No BI external reference found")); + referenceUnitById.put(xref.getReferenceId(), unit); + expTitleByRefUnitId.put(xref.getReferenceId(), unit.getTrialName()); + } + } catch (ApiException e) { + throw new InternalServerException(e.toString(), e); + } + } else if (hasNoReferenceUnitIds) { + observationUnitByNameNoScope = initializeObservationUnits(program, experimentImportRows); + trialByNameNoScope = initializeTrialByNameNoScope(program, experimentImportRows); + studyByNameNoScope = initializeStudyByNameNoScope(program, experimentImportRows); + locationByName = initializeUniqueLocationNames(program, experimentImportRows); + obsVarDatasetByName = initializeObsVarDatasetByName(program, experimentImportRows); + existingGermplasmByGID = initializeExistingGermplasmByGID(program, experimentImportRows); + } else { + + // can't proceed if you have a mix of ObsUnitId for some but not all rows + throw new HttpStatusException(HttpStatus.UNPROCESSABLE_ENTITY, MISSING_OBS_UNIT_ID_ERROR); + } + } /** @@ -495,7 +539,14 @@ private String getVariableNameFromColumn(Column column) { return column.name(); } - private void initNewBrapiData(List importRows, List> phenotypeCols, Program program, User user, List referencedTraits, boolean commit) throws ApiException, MissingRequiredInfoException { + private void initNewBrapiData( + List importRows, + List> phenotypeCols, + Program program, + User user, + List referencedTraits, + boolean commit + ) throws ApiException, MissingRequiredInfoException { String expSequenceName = program.getExpSequence(); if (expSequenceName == null) { @@ -807,7 +858,7 @@ private void validateConditionallyRequired(ValidationErrors validationErrors, in validateRequiredCell( importRow.getObsUnitID(), Columns.OBS_UNIT_ID, - "Experimental entities are missing ObsUnitIDs. Import cannot proceed", + MISSING_OBS_UNIT_ID_ERROR, validationErrors, rowNum ); @@ -1106,7 +1157,7 @@ private PendingImportObject fetchOrCreateStudyPIO( PendingImportObject pio; if (studyByNameNoScope.containsKey(importRow.getEnv())) { pio = studyByNameNoScope.get(importRow.getEnv()); - if (! commit){ + if (!commit){ addYearToStudyAdditionalInfo(program, pio.getBrAPIObject()); } } else { @@ -1173,7 +1224,13 @@ private void fetchOrCreateLocationPIO(ExperimentObservation importRow) { } } - private PendingImportObject fetchOrCreateTrialPIO(Program program, User user, boolean commit, ExperimentObservation importRow, Supplier expNextVal) throws UnprocessableEntityException { + private PendingImportObject fetchOrCreateTrialPIO( + Program program, + User user, + boolean commit, + ExperimentObservation importRow, + Supplier expNextVal + ) throws UnprocessableEntityException { PendingImportObject trialPio; if (trialByNameNoScope.containsKey(importRow.getExpTitle())) { PendingImportObject envPio; @@ -1397,6 +1454,60 @@ private Map> initializeObserva } } + /** + * Retrieves a list of pending Observation Units based on their IDs. + * + * This function retrieves Observation Units based on a list of reference Observation Unit IDs + * and the associated program. It then sets pending Observation Units for each ID. + * + * @return List of BrAPIObservationUnit: The list of reference Observation Units retrieved. + * + * @throws InternalServerException if an error occurs during the process. + */ + private List setPendingObsUnitByOUId(Program program) { + try { + // Retrieve reference Observation Units based on IDs + List referenceObsUnits = brAPIObservationUnitDAO.getObservationUnitsById( + new ArrayList(referenceOUIds), + program + ); + + // Construct the DeltaBreed observation unit source for external references + String deltabreedOUSource = String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName()); + + if (referenceObsUnits.size() == referenceOUIds.size()) { + // Iterate through reference Observation Units + referenceObsUnits.forEach(unit -> { + // Get external reference for the Observation Unit + BrAPIExternalReference unitXref = Utilities.getExternalReference(unit.getExternalReferences(), deltabreedOUSource) + .orElseThrow(() -> new InternalServerException("An ObservationUnit ID was not found in any of the external references")); + + // Set pending Observation Unit by its ID + pendingObsUnitByOUId.put( + unitXref.getReferenceId(), + new PendingImportObject( + ImportObjectState.EXISTING, unit, UUID.fromString(unitXref.getReferenceId())) + ); + }); + } else { + // Handle missing Observation Unit IDs + List missingIds = new ArrayList<>(referenceOUIds); + Set fetchedIds = referenceObsUnits.stream().map(unit -> + Utilities.getExternalReference(unit.getExternalReferences(), deltabreedOUSource) + .orElseThrow(() -> new InternalServerException("An ObservationUnit ID was not found in any of the external references")) + .getReferenceId()) + .collect(Collectors.toSet()); + missingIds.removeAll(fetchedIds); + throw new IllegalStateException("Observation Units not found for ObsUnitId(s): " + String.join(COMMA_DELIMITER, missingIds)); + } + + return referenceObsUnits; + } catch (ApiException e) { + log.error("Error fetching observation units: " + Utilities.generateApiExceptionLogMessage(e), e); + throw new InternalServerException(e.toString(), e); + } + } + private Map> initializeTrialByNameNoScope(Program program, List experimentImportRows) { Map> trialByName = new HashMap<>(); @@ -1461,12 +1572,26 @@ private void initializeStudiesForExistingObservationUnits(Program program, Map processAndCacheStudy(study, program, studyByName)); } + /** + * Fetches a list of BrAPI studies by studyDbIds belonging to a specific + * program. + * If not all studyDbIds are found in the database, it throws an + * IllegalStateException. + * + * @param studyDbIds A set of studyDbIds to fetch studies for + * @param program The program for which the studies are associated + * @return A list of BrAPIStudy objects representing the fetched studies + * @throws ApiException if an error occurs during the database fetch + * operation + * @throws IllegalStateException if not all studyDbIds are found in the database + */ private List fetchStudiesByDbId(Set studyDbIds, Program program) throws ApiException { List studies = brAPIStudyDAO.getStudiesByStudyDbId(studyDbIds, program); - if(studies.size() != studyDbIds.size()) { + if (studies.size() != studyDbIds.size()) { List missingIds = new ArrayList<>(studyDbIds); missingIds.removeAll(studies.stream().map(BrAPIStudy::getStudyDbId).collect(Collectors.toList())); - throw new IllegalStateException("Study not found for studyDbId(s): " + String.join(COMMA_DELIMITER, missingIds)); + throw new IllegalStateException( + "Study not found for studyDbId(s): " + String.join(COMMA_DELIMITER, missingIds)); } return studies; } @@ -1621,6 +1746,52 @@ private void processAndCacheStudy(BrAPIStudy existingStudy, Program program, Map new PendingImportObject<>(ImportObjectState.EXISTING, (BrAPIStudy) Utilities.formatBrapiObjForDisplay(existingStudy, BrAPIStudy.class, program), UUID.fromString(xref.getReferenceID()))); } + private void setPendingTrialByOUId(Program program) { + if(observationUnitByNameNoScope.size() > 0) { + Set trialDbIds = new HashSet<>(); + Set studyDbIds = new HashSet<>(); + + observationUnitByNameNoScope.values() + .forEach(pio -> { + BrAPIObservationUnit existingOu = pio.getBrAPIObject(); + if (StringUtils.isBlank(existingOu.getTrialDbId()) && StringUtils.isBlank(existingOu.getStudyDbId())) { + throw new IllegalStateException("TrialDbId and StudyDbId are not set for an existing ObservationUnit"); + } + + if (StringUtils.isNotBlank(existingOu.getTrialDbId())) { + trialDbIds.add(existingOu.getTrialDbId()); + } else { + studyDbIds.add(existingOu.getStudyDbId()); + } + }); + + //if the OU doesn't have the trialDbId set, then fetch the study to fetch the trialDbId + if(!studyDbIds.isEmpty()) { + try { + trialDbIds.addAll(fetchTrialDbidsForStudies(studyDbIds, program)); + } catch (ApiException e) { + log.error("Error fetching studies: " + Utilities.generateApiExceptionLogMessage(e), e); + throw new InternalServerException(e.toString(), e); + } + } + + try { + List trials = brapiTrialDAO.getTrialsByDbIds(trialDbIds, program); + if (trials.size() != trialDbIds.size()) { + List missingIds = new ArrayList<>(trialDbIds); + missingIds.removeAll(trials.stream().map(BrAPITrial::getTrialDbId).collect(Collectors.toList())); + throw new IllegalStateException("Trial not found for trialDbId(s): " + String.join(COMMA_DELIMITER, missingIds)); + } + + String trialRefSource = String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName()); + trials.forEach(trial -> processAndCachePendingTrial(trial, trialRefSource, "foo")); + } catch (ApiException e) { + log.error("Error fetching trials: " + Utilities.generateApiExceptionLogMessage(e), e); + throw new InternalServerException(e.toString(), e); + } + } + } + private void initializeTrialsForExistingObservationUnits(Program program, Map> trialByName) { if(observationUnitByNameNoScope.size() > 0) { Set trialDbIds = new HashSet<>(); @@ -1680,7 +1851,12 @@ private Set fetchTrialDbidsForStudies(Set studyDbIds, Program pr return trialDbIds; } - private void processAndCacheTrial(BrAPITrial existingTrial, Program program, String trialRefSource, Map> trialByNameNoScope) { + private void processAndCacheTrial( + BrAPITrial existingTrial, + Program program, + String trialRefSource, + Map> trialByNameNoScope + ) { //get TrialId from existingTrial BrAPIExternalReference experimentIDRef = Utilities.getExternalReference(existingTrial.getExternalReferences(), trialRefSource) @@ -1692,6 +1868,60 @@ private void processAndCacheTrial(BrAPITrial existingTrial, Program program, Str new PendingImportObject<>(ImportObjectState.EXISTING, existingTrial, experimentId)); } + /** + * Cache pending trial with reference to observation unit. + * + * This method processes the given existing trial, associates it with the + * specified + * program, retrieves the experiment ID from the trial's external references, + * then caches it with the provided organizational unit reference ID. + * + * @param existingTrial The existing trial to process and cache. + * @param trialRefSource The source to retrieve the trial's reference. + * @param referenceOUId The ID of the reference observation unit. + */ + private void processAndCachePendingTrial( + BrAPITrial existingTrial, + String trialRefSource, + String referenceOUId) { + + // Retrieve experiment ID from existingTrial + BrAPIExternalReference experimentIDRef = Utilities + .getExternalReference(existingTrial.getExternalReferences(), trialRefSource) + .orElseThrow(() -> new InternalServerException( + "An Experiment ID was not found in any of the external references")); + UUID experimentId = UUID.fromString(experimentIDRef.getReferenceId()); + + // Cache pending trial by observation unit ID + pendingTrialByOUId.put( + referenceOUId, + new PendingImportObject<>(ImportObjectState.EXISTING, existingTrial, experimentId)); + } + + /** + * Sets the reference Observation Unit IDs based on the import rows. + * Checks for references to existing observation units and populates the + * referenceOUIds list. + * Sets flags hasAllReferenceUnitIds and hasNoReferenceUnitIds accordingly. + */ + private void setReferenceOUIds(List importRows) { + for (int rowNum = 0; rowNum < importRows.size(); rowNum++) { + ExperimentObservation importRow = (ExperimentObservation) importRows.get(rowNum); + + // Check if ObsUnitID is blank + if (importRow.getObsUnitID() == null || importRow.getObsUnitID().isBlank()) { + hasAllReferenceUnitIds = false; + } else if (referenceOUIds.contains(importRow.getObsUnitID())) { + // Throw exception if ObsUnitID is repeated + throw new IllegalStateException("ObsUnitId is repeated: " + importRow.getObsUnitID()); + } else { + // Add ObsUnitID to referenceOUIds + referenceOUIds.add(importRow.getObsUnitID()); + hasNoReferenceUnitIds = false; + } + } + } + private void validateTimeStampValue(String value, String columnHeader, ValidationErrors validationErrors, int row) { if (StringUtils.isBlank(value)) { diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 056e8b297..58299ece5 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -702,6 +702,70 @@ public void importNewObsVarExistingOu() { assertRowSaved(newObsVar, program, traits); } + @Test + @SneakyThrows + public void importNewObsVarByObsUnitId() { + List traits = importTestUtils.createTraits(2); + Program program = createProgram("New ObsVar Existing OU", "OUVAR", "OUVAR", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Map newExp = new HashMap<>(); + newExp.put(Columns.GERMPLASM_GID, "1"); + newExp.put(Columns.TEST_CHECK, "T"); + newExp.put(Columns.EXP_TITLE, "Test Exp"); + newExp.put(Columns.EXP_UNIT, "Plot"); + newExp.put(Columns.EXP_TYPE, "Phenotyping"); + newExp.put(Columns.ENV, "New Env"); + newExp.put(Columns.ENV_LOCATION, "Location A"); + newExp.put(Columns.ENV_YEAR, "2023"); + newExp.put(Columns.EXP_UNIT_ID, "a-1"); + newExp.put(Columns.REP_NUM, "1"); + newExp.put(Columns.BLOCK_NUM, "1"); + newExp.put(Columns.ROW, "1"); + newExp.put(Columns.COLUMN, "1"); + newExp.put(traits.get(0).getObservationVariableName(), null); + + importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newExp), null), null, true, client, program, mappingId); + + BrAPITrial brAPITrial = brAPITrialDAO.getTrialsByName(List.of((String)newExp.get(Columns.EXP_TITLE)), program).get(0); + Optional trialIdXref = Utilities.getExternalReference(brAPITrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())); + assertTrue(trialIdXref.isPresent()); + BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceId()), program).get(0); + + BrAPIObservationUnit ou = ouDAO.getObservationUnitsForStudyDbId(brAPIStudy.getStudyDbId(), program).get(0); + Optional ouIdXref = Utilities.getExternalReference(ou.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName())); + assertTrue(ouIdXref.isPresent()); + + Map newObsVar = new HashMap<>(); +// newObsVar.put(Columns.GERMPLASM_GID, "1"); +// newObsVar.put(Columns.TEST_CHECK, "T"); +// newObsVar.put(Columns.EXP_TITLE, "Test Exp"); +// newObsVar.put(Columns.EXP_UNIT, "Plot"); +// newObsVar.put(Columns.EXP_TYPE, "Phenotyping"); +// newObsVar.put(Columns.ENV, "New Env"); +// newObsVar.put(Columns.ENV_LOCATION, "Location A"); +// newObsVar.put(Columns.ENV_YEAR, "2023"); +// newObsVar.put(Columns.EXP_UNIT_ID, "a-1"); +// newObsVar.put(Columns.REP_NUM, "1"); +// newObsVar.put(Columns.BLOCK_NUM, "1"); +// newObsVar.put(Columns.ROW, "1"); +// newObsVar.put(Columns.COLUMN, "1"); + newObsVar.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); + newObsVar.put(traits.get(1).getObservationVariableName(), null); + + JsonObject result = importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits), null, true, client, program, mappingId); + + JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); + assertEquals(1, previewRows.size()); + JsonObject row = previewRows.get(0).getAsJsonObject(); + + assertEquals("EXISTING", row.getAsJsonObject("trial").get("state").getAsString()); + assertTrue(row.getAsJsonObject("trial").get("brAPIObject").getAsJsonObject().get("additionalInfo").getAsJsonObject().has("observationDatasetId")); + assertTrue(importTestUtils.UUID_REGEX.matcher(row.getAsJsonObject("trial").get("brAPIObject").getAsJsonObject().get("additionalInfo").getAsJsonObject().get("observationDatasetId").getAsString()).matches()); + assertEquals("EXISTING", row.getAsJsonObject("location").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); + assertRowSaved(newObsVar, program, traits); + } + @ParameterizedTest @ValueSource(booleans = {true, false}) From 9917eb5fe054979a5d5fa329e8b708f10ec7c7f4 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 27 Feb 2024 15:47:36 -0500 Subject: [PATCH 05/40] [BI-2046] refactor initializing OUs --- .../processors/ExperimentProcessor.java | 122 ++++++++++-------- 1 file changed, 69 insertions(+), 53 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 148e139ce..384ec8260 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -188,47 +188,32 @@ public void getExistingBrapiData(List importRows, Program program) .map(trialImport -> (ExperimentObservation) trialImport) .collect(Collectors.toList()); - Map referenceUnitById = new HashMap<>(); - Map expTitleByRefUnitId = new HashMap<>(); // check for references to Deltabreed-generated observation units - setReferenceOUIds(importRows); + referenceOUIds = collateReferenceOUIds(importRows); if (hasAllReferenceUnitIds) { - - // get all prior units referenced in import try { - List referenceOUs = brAPIObservationUnitDAO.getObservationUnitsById(new ArrayList(referenceOUIds), program); - for (BrAPIObservationUnit unit: referenceOUs) { - setPendingObsUnitByOUId(program); - setPendingTrialByOUId(program); - - // set pendingStudiesByOUId - // set pendingLocationsByOUId - // set pendingObsDatasetByOUId - - BrAPIExternalReference xref = Utilities.getExternalReference( - unit.getExternalReferences(), - ExternalReferenceSource.OBSERVATION_UNITS.getName() - ).orElseThrow(() -> new IllegalStateException("No BI external reference found")); - referenceUnitById.put(xref.getReferenceId(), unit); - expTitleByRefUnitId.put(xref.getReferenceId(), unit.getTrialName()); - } + + // get all prior units referenced in import + pendingObsUnitByOUId = fetchReferenceObservationUnits(referenceOUIds, program); + observationUnitByNameNoScope = mapPendingObservationUnitByName(pendingObsUnitByOUId, program); } catch (ApiException e) { + log.error("Error fetching observation units: " + Utilities.generateApiExceptionLogMessage(e), e); throw new InternalServerException(e.toString(), e); } } else if (hasNoReferenceUnitIds) { observationUnitByNameNoScope = initializeObservationUnits(program, experimentImportRows); - trialByNameNoScope = initializeTrialByNameNoScope(program, experimentImportRows); - studyByNameNoScope = initializeStudyByNameNoScope(program, experimentImportRows); - locationByName = initializeUniqueLocationNames(program, experimentImportRows); - obsVarDatasetByName = initializeObsVarDatasetByName(program, experimentImportRows); - existingGermplasmByGID = initializeExistingGermplasmByGID(program, experimentImportRows); + } else { // can't proceed if you have a mix of ObsUnitId for some but not all rows throw new HttpStatusException(HttpStatus.UNPROCESSABLE_ENTITY, MISSING_OBS_UNIT_ID_ERROR); } - + trialByNameNoScope = initializeTrialByNameNoScope(program, experimentImportRows); + studyByNameNoScope = initializeStudyByNameNoScope(program, experimentImportRows); + locationByName = initializeUniqueLocationNames(program, experimentImportRows); + obsVarDatasetByName = initializeObsVarDatasetByName(program, experimentImportRows); + existingGermplasmByGID = initializeExistingGermplasmByGID(program, experimentImportRows); } /** @@ -562,6 +547,7 @@ private void initNewBrapiData( } Supplier envNextVal = () -> dsl.nextval(envSequenceName.toLowerCase()); existingObsByObsHash = fetchExistingObservations(referencedTraits, program); + for (int rowNum = 0; rowNum < importRows.size(); rowNum++) { ExperimentObservation importRow = (ExperimentObservation) importRows.get(rowNum); @@ -1454,6 +1440,28 @@ private Map> initializeObserva } } + private Map> mapPendingObservationUnitByName( + Map> pendingUnitById, + Program program + ) { + Map> pendingUnitByName = new HashMap<>(); + for (Map.Entry> entry : pendingUnitById.entrySet()) { + String studyName = Utilities.removeProgramKeyAndUnknownAdditionalData( + entry.getValue().getBrAPIObject().getStudyName(), + program.getKey() + ); + String observationUnitName = Utilities.removeProgramKeyAndUnknownAdditionalData( + entry.getValue().getBrAPIObject().getObservationUnitName(), + program.getKey() + ); + pendingUnitByName.put(createObservationUnitKey(studyName, observationUnitName), entry.getValue()); + } + return pendingUnitByName; + } + + + + /** * Retrieves a list of pending Observation Units based on their IDs. * @@ -1464,7 +1472,11 @@ private Map> initializeObserva * * @throws InternalServerException if an error occurs during the process. */ - private List setPendingObsUnitByOUId(Program program) { + private Map> fetchReferenceObservationUnits( + Set referenceOUIds, + Program program + ) throws ApiException { + Map> pendingUnitById = new HashMap<>(); try { // Retrieve reference Observation Units based on IDs List referenceObsUnits = brAPIObservationUnitDAO.getObservationUnitsById( @@ -1480,10 +1492,10 @@ private List setPendingObsUnitByOUId(Program program) { referenceObsUnits.forEach(unit -> { // Get external reference for the Observation Unit BrAPIExternalReference unitXref = Utilities.getExternalReference(unit.getExternalReferences(), deltabreedOUSource) - .orElseThrow(() -> new InternalServerException("An ObservationUnit ID was not found in any of the external references")); + .orElseThrow(() -> new IllegalStateException("External reference does not exist for Deltabreed ObservationUnit ID")); // Set pending Observation Unit by its ID - pendingObsUnitByOUId.put( + pendingUnitById.put( unitXref.getReferenceId(), new PendingImportObject( ImportObjectState.EXISTING, unit, UUID.fromString(unitXref.getReferenceId())) @@ -1494,17 +1506,17 @@ private List setPendingObsUnitByOUId(Program program) { List missingIds = new ArrayList<>(referenceOUIds); Set fetchedIds = referenceObsUnits.stream().map(unit -> Utilities.getExternalReference(unit.getExternalReferences(), deltabreedOUSource) - .orElseThrow(() -> new InternalServerException("An ObservationUnit ID was not found in any of the external references")) + .orElseThrow(() -> new InternalServerException("External reference does not exist for Deltabreed ObservationUnit ID")) .getReferenceId()) .collect(Collectors.toSet()); missingIds.removeAll(fetchedIds); throw new IllegalStateException("Observation Units not found for ObsUnitId(s): " + String.join(COMMA_DELIMITER, missingIds)); } - return referenceObsUnits; + return pendingUnitById; } catch (ApiException e) { log.error("Error fetching observation units: " + Utilities.generateApiExceptionLogMessage(e), e); - throw new InternalServerException(e.toString(), e); + throw new ApiException(e); } } @@ -1562,11 +1574,12 @@ private Map> initializeStudyByNameNoScop } private void initializeStudiesForExistingObservationUnits(Program program, Map> studyByName) throws ApiException { - Set studyDbIds = this.observationUnitByNameNoScope.values() - .stream() - .map(pio -> pio.getBrAPIObject() - .getStudyDbId()) - .collect(Collectors.toSet()); + Map> observationUnitMap = hasAllReferenceUnitIds ? pendingObsUnitByOUId : observationUnitByNameNoScope; + Set studyDbIds = observationUnitMap.values() + .stream() + .map(pio -> pio.getBrAPIObject() + .getStudyDbId()) + .collect(Collectors.toSet()); List studies = fetchStudiesByDbId(studyDbIds, program); studies.forEach(study -> processAndCacheStudy(study, program, studyByName)); @@ -1796,20 +1809,21 @@ private void initializeTrialsForExistingObservationUnits(Program program, Map 0) { Set trialDbIds = new HashSet<>(); Set studyDbIds = new HashSet<>(); - - observationUnitByNameNoScope.values() - .forEach(pio -> { - BrAPIObservationUnit existingOu = pio.getBrAPIObject(); - if (StringUtils.isBlank(existingOu.getTrialDbId()) && StringUtils.isBlank(existingOu.getStudyDbId())) { - throw new IllegalStateException("TrialDbId and StudyDbId are not set for an existing ObservationUnit"); - } - - if (StringUtils.isNotBlank(existingOu.getTrialDbId())) { - trialDbIds.add(existingOu.getTrialDbId()); - } else { - studyDbIds.add(existingOu.getStudyDbId()); - } - }); + Map> observationUnitMap = hasAllReferenceUnitIds ? pendingObsUnitByOUId : observationUnitByNameNoScope; + + observationUnitMap.values() + .forEach(pio -> { + BrAPIObservationUnit existingOu = pio.getBrAPIObject(); + if (StringUtils.isBlank(existingOu.getTrialDbId()) && StringUtils.isBlank(existingOu.getStudyDbId())) { + throw new IllegalStateException("TrialDbId and StudyDbId are not set for an existing ObservationUnit"); + } + + if (StringUtils.isNotBlank(existingOu.getTrialDbId())) { + trialDbIds.add(existingOu.getTrialDbId()); + } else { + studyDbIds.add(existingOu.getStudyDbId()); + } + }); //if the OU doesn't have the trialDbId set, then fetch the study to fetch the trialDbId if(!studyDbIds.isEmpty()) { @@ -1904,7 +1918,8 @@ private void processAndCachePendingTrial( * referenceOUIds list. * Sets flags hasAllReferenceUnitIds and hasNoReferenceUnitIds accordingly. */ - private void setReferenceOUIds(List importRows) { + private Set collateReferenceOUIds(List importRows) { + Set referenceOUIds = new HashSet<>(); for (int rowNum = 0; rowNum < importRows.size(); rowNum++) { ExperimentObservation importRow = (ExperimentObservation) importRows.get(rowNum); @@ -1920,6 +1935,7 @@ private void setReferenceOUIds(List importRows) { hasNoReferenceUnitIds = false; } } + return referenceOUIds; } private void validateTimeStampValue(String value, From 9db276a95ccf731266396403e6484ceb0194b0c7 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 27 Feb 2024 19:56:02 -0500 Subject: [PATCH 06/40] [BI-2046] get trial pio from reference obs unit --- .../processors/ExperimentProcessor.java | 58 ++++++++++++------- .../importer/ExperimentFileImportTest.java | 2 +- 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 384ec8260..e756c6954 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -1107,7 +1107,12 @@ private void addObsVarsToDatasetDetails(PendingImportObject pi } private void fetchOrCreateDatasetPIO(ExperimentObservation importRow, Program program, List referencedTraits) { PendingImportObject pio; - PendingImportObject trialPIO = trialByNameNoScope.get(importRow.getExpTitle()); + PendingImportObject trialPIO; + if (hasAllReferenceUnitIds) { + trialPIO = trialByNameNoScope.values().iterator().next(); + } else { + trialPIO = trialByNameNoScope.get(importRow.getExpTitle()); + } String name = String.format("Observation Dataset [%s-%s]", program.getKey(), trialPIO.getBrAPIObject() @@ -1218,24 +1223,35 @@ private PendingImportObject fetchOrCreateTrialPIO( Supplier expNextVal ) throws UnprocessableEntityException { PendingImportObject trialPio; - if (trialByNameNoScope.containsKey(importRow.getExpTitle())) { - PendingImportObject envPio; - trialPio = trialByNameNoScope.get(importRow.getExpTitle()); - envPio = this.studyByNameNoScope.get(importRow.getEnv()); - if (trialPio!=null && ImportObjectState.EXISTING==trialPio.getState() && (StringUtils.isBlank( importRow.getObsUnitID() )) && (envPio!=null && ImportObjectState.EXISTING==envPio.getState() ) ){ - throw new UnprocessableEntityException(PREEXISTING_EXPERIMENT_TITLE); + + // use the prior trial if observation unit IDs are supplied + if (hasAllReferenceUnitIds) { + if (trialByNameNoScope.size() != 1) { + throw new UnprocessableEntityException(MULTIPLE_EXP_TITLES); } - } else if (!trialByNameNoScope.isEmpty()) { - throw new UnprocessableEntityException(MULTIPLE_EXP_TITLES); + trialPio = trialByNameNoScope.values().iterator().next(); + + // otherwise create a new trial, but there can be only one allowed } else { - UUID id = UUID.randomUUID(); - String expSeqValue = null; - if (commit) { - expSeqValue = expNextVal.get().toString(); + if (trialByNameNoScope.containsKey(importRow.getExpTitle())) { + PendingImportObject envPio; + trialPio = trialByNameNoScope.get(importRow.getExpTitle()); + envPio = this.studyByNameNoScope.get(importRow.getEnv()); + if (trialPio!=null && ImportObjectState.EXISTING==trialPio.getState() && (StringUtils.isBlank( importRow.getObsUnitID() )) && (envPio!=null && ImportObjectState.EXISTING==envPio.getState() ) ){ + throw new UnprocessableEntityException(PREEXISTING_EXPERIMENT_TITLE); + } + } else if (!trialByNameNoScope.isEmpty()) { + throw new UnprocessableEntityException(MULTIPLE_EXP_TITLES); + } else { + UUID id = UUID.randomUUID(); + String expSeqValue = null; + if (commit) { + expSeqValue = expNextVal.get().toString(); + } + BrAPITrial newTrial = importRow.constructBrAPITrial(program, user, commit, BRAPI_REFERENCE_SOURCE, id, expSeqValue); + trialPio = new PendingImportObject<>(ImportObjectState.NEW, newTrial, id); + this.trialByNameNoScope.put(importRow.getExpTitle(), trialPio); } - BrAPITrial newTrial = importRow.constructBrAPITrial(program, user, commit, BRAPI_REFERENCE_SOURCE, id, expSeqValue); - trialPio = new PendingImportObject<>(ImportObjectState.NEW, newTrial, id); - this.trialByNameNoScope.put(importRow.getExpTitle(), trialPio); } return trialPio; } @@ -1482,11 +1498,11 @@ private Map> fetchReferenceObs List referenceObsUnits = brAPIObservationUnitDAO.getObservationUnitsById( new ArrayList(referenceOUIds), program - ); + ); // Construct the DeltaBreed observation unit source for external references String deltabreedOUSource = String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName()); - + if (referenceObsUnits.size() == referenceOUIds.size()) { // Iterate through reference Observation Units referenceObsUnits.forEach(unit -> { @@ -1574,8 +1590,7 @@ private Map> initializeStudyByNameNoScop } private void initializeStudiesForExistingObservationUnits(Program program, Map> studyByName) throws ApiException { - Map> observationUnitMap = hasAllReferenceUnitIds ? pendingObsUnitByOUId : observationUnitByNameNoScope; - Set studyDbIds = observationUnitMap.values() + Set studyDbIds = observationUnitByNameNoScope.values() .stream() .map(pio -> pio.getBrAPIObject() .getStudyDbId()) @@ -1809,9 +1824,8 @@ private void initializeTrialsForExistingObservationUnits(Program program, Map 0) { Set trialDbIds = new HashSet<>(); Set studyDbIds = new HashSet<>(); - Map> observationUnitMap = hasAllReferenceUnitIds ? pendingObsUnitByOUId : observationUnitByNameNoScope; - observationUnitMap.values() + observationUnitByNameNoScope.values() .forEach(pio -> { BrAPIObservationUnit existingOu = pio.getBrAPIObject(); if (StringUtils.isBlank(existingOu.getTrialDbId()) && StringUtils.isBlank(existingOu.getStudyDbId())) { diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 58299ece5..59607a577 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -706,7 +706,7 @@ public void importNewObsVarExistingOu() { @SneakyThrows public void importNewObsVarByObsUnitId() { List traits = importTestUtils.createTraits(2); - Program program = createProgram("New ObsVar Existing OU", "OUVAR", "OUVAR", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Program program = createProgram("New ObsVar Referring to OU by ID", "OUVAR", "VAROU", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); Map newExp = new HashMap<>(); newExp.put(Columns.GERMPLASM_GID, "1"); newExp.put(Columns.TEST_CHECK, "T"); From fd40f55bf61071f99c77299fadd378910ffbdb8f Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 29 Feb 2024 14:56:05 -0500 Subject: [PATCH 07/40] [BI-2046] update fetchOrCreate methods --- .../processors/ExperimentProcessor.java | 106 ++++++++++++------ 1 file changed, 74 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index e756c6954..adbef9852 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -234,7 +234,7 @@ public Map process( Table data, Program program, User user, - boolean commit) throws ApiException, ValidatorException, MissingRequiredInfoException { + boolean commit) throws ApiException, ValidatorException, MissingRequiredInfoException, UnprocessableEntityException { log.debug("processing experiment import"); ValidationErrors validationErrors = new ValidationErrors(); @@ -531,7 +531,7 @@ private void initNewBrapiData( User user, List referencedTraits, boolean commit - ) throws ApiException, MissingRequiredInfoException { + ) throws UnprocessableEntityException, ApiException, MissingRequiredInfoException { String expSequenceName = program.getExpSequence(); if (expSequenceName == null) { @@ -594,9 +594,23 @@ private void initNewBrapiData( dateTimeValue += MIDNIGHT; } } - //column.name() gets phenotype name - String seasonDbId = this.yearToSeasonDbId(importRow.getEnvYear(), program.getId()); - fetchOrCreateObservationPIO(program, user, importRow, column, rowNum, dateTimeValue, commit, seasonDbId, obsUnitPIO, referencedTraits); + + // get the study year either referenced from the observation unit or listed explicitly on the import row + String studyYear = hasAllReferenceUnitIds ? studyPIO.getBrAPIObject().getSeasons().get(0) : importRow.getEnvYear(); + String seasonDbId = yearToSeasonDbId(studyYear, program.getId()); + fetchOrCreateObservationPIO( + program, + user, + importRow, + column, //column.name() gets phenotype name + rowNum, + dateTimeValue, + commit, + seasonDbId, + obsUnitPIO, + studyPIO, + referencedTraits + ); } } } @@ -966,10 +980,12 @@ private PendingImportObject getGidPOI(ExperimentObservation impo return null; } - private PendingImportObject fetchOrCreateObsUnitPIO(Program program, boolean commit, String envSeqValue, ExperimentObservation importRow) throws ApiException, MissingRequiredInfoException { + private PendingImportObject fetchOrCreateObsUnitPIO(Program program, boolean commit, String envSeqValue, ExperimentObservation importRow) throws ApiException, MissingRequiredInfoException, UnprocessableEntityException { PendingImportObject pio; String key = createObservationUnitKey(importRow); - if (this.observationUnitByNameNoScope.containsKey(key)) { + if (hasAllReferenceUnitIds) { + pio = pendingObsUnitByOUId.get(importRow.getObsUnitID()); + } else if (observationUnitByNameNoScope.containsKey(key)) { pio = observationUnitByNameNoScope.get(key); } else { String germplasmName = ""; @@ -978,7 +994,7 @@ private PendingImportObject fetchOrCreateObsUnitPIO(Progra .getBrAPIObject() .getGermplasmName(); } - PendingImportObject trialPIO = this.trialByNameNoScope.get(importRow.getExpTitle()); + PendingImportObject trialPIO = trialByNameNoScope.get(importRow.getExpTitle());; UUID trialID = trialPIO.getId(); UUID datasetId = null; if (commit) { @@ -1041,12 +1057,20 @@ private void fetchOrCreateObservationPIO(Program program, boolean commit, String seasonDbId, PendingImportObject obsUnitPIO, - List referencedTraits) throws ApiException { + PendingImportObject studyPIO, + List referencedTraits) throws ApiException, UnprocessableEntityException { PendingImportObject pio; BrAPIObservation newObservation; String variableName = column.name(); String value = column.getString(rowNum); - String key = getImportObservationHash(importRow, variableName); + String key; + if (hasAllReferenceUnitIds) { + String unitName = obsUnitPIO.getBrAPIObject().getObservationUnitName(); + String studyName = studyPIO.getBrAPIObject().getStudyName(); + key = getObservationHash(unitName, variableName, studyName); + } else { + key = getImportObservationHash(importRow, variableName); + } if (existingObsByObsHash.containsKey(key)) { if (StringUtils.isNotBlank(value) && !isObservationMatched(key, value, column, rowNum)){ @@ -1071,9 +1095,11 @@ private void fetchOrCreateObservationPIO(Program program, } else if (!this.observationByHash.containsKey(key)){ // new observation - PendingImportObject trialPIO = this.trialByNameNoScope.get(importRow.getExpTitle()); + PendingImportObject trialPIO = hasAllReferenceUnitIds ? + getSingleEntryValue(trialByNameNoScope, MULTIPLE_EXP_TITLES) : trialByNameNoScope.get(importRow.getExpTitle()); + UUID trialID = trialPIO.getId(); - PendingImportObject studyPIO = this.studyByNameNoScope.get(importRow.getEnv()); + //PendingImportObject studyPIO = this.studyByNameNoScope.get(importRow.getEnv()); UUID studyID = studyPIO.getId(); UUID id = UUID.randomUUID(); newObservation = importRow.constructBrAPIObservation(value, variableName, seasonDbId, obsUnitPIO.getBrAPIObject(), commit, program, user, BRAPI_REFERENCE_SOURCE, trialID, studyID, obsUnitPIO.getId(), id); @@ -1083,7 +1109,7 @@ private void fetchOrCreateObservationPIO(Program program, newObservation.setObservationTimeStamp(OffsetDateTime.parse(timeStampValue)); } - newObservation.setStudyDbId(this.studyByNameNoScope.get(importRow.getEnv()).getId().toString()); //set as the BI ID to facilitate looking up studies when saving new observations + newObservation.setStudyDbId(this.studyByNameNoScope.get(pendingObsUnitByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getStudyName()).getId().toString()); //set as the BI ID to facilitate looking up studies when saving new observations pio = new PendingImportObject<>(ImportObjectState.NEW, newObservation); this.observationByHash.put(key, pio); @@ -1105,14 +1131,10 @@ private void addObsVarsToDatasetDetails(PendingImportObject pi } }); } - private void fetchOrCreateDatasetPIO(ExperimentObservation importRow, Program program, List referencedTraits) { + private void fetchOrCreateDatasetPIO(ExperimentObservation importRow, Program program, List referencedTraits) throws UnprocessableEntityException { PendingImportObject pio; - PendingImportObject trialPIO; - if (hasAllReferenceUnitIds) { - trialPIO = trialByNameNoScope.values().iterator().next(); - } else { - trialPIO = trialByNameNoScope.get(importRow.getExpTitle()); - } + PendingImportObject trialPIO = hasAllReferenceUnitIds ? + getSingleEntryValue(trialByNameNoScope, MULTIPLE_EXP_TITLES) : trialByNameNoScope.get(importRow.getExpTitle()); String name = String.format("Observation Dataset [%s-%s]", program.getKey(), trialPIO.getBrAPIObject() @@ -1144,15 +1166,26 @@ private PendingImportObject fetchOrCreateStudyPIO( boolean commit, String expSequenceValue, ExperimentObservation importRow, - Supplier envNextVal) { + Supplier envNextVal + ) throws UnprocessableEntityException { PendingImportObject pio; - if (studyByNameNoScope.containsKey(importRow.getEnv())) { + if (hasAllReferenceUnitIds) { + String studyName = Utilities.removeProgramKeyAndUnknownAdditionalData( + pendingObsUnitByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getStudyName(), + program.getKey() + ); + pio = studyByNameNoScope.get(studyName); + if (!commit){ + addYearToStudyAdditionalInfo(program, pio.getBrAPIObject()); + } + } else if (studyByNameNoScope.containsKey(importRow.getEnv())) { pio = studyByNameNoScope.get(importRow.getEnv()); if (!commit){ addYearToStudyAdditionalInfo(program, pio.getBrAPIObject()); } } else { - PendingImportObject trialPIO = this.trialByNameNoScope.get(importRow.getExpTitle()); + PendingImportObject trialPIO = hasAllReferenceUnitIds ? + getSingleEntryValue(trialByNameNoScope, MULTIPLE_EXP_TITLES) : trialByNameNoScope.get(importRow.getExpTitle()); UUID trialID = trialPIO.getId(); UUID id = UUID.randomUUID(); BrAPIStudy newStudy = importRow.constructBrAPIStudy(program, commit, BRAPI_REFERENCE_SOURCE, expSequenceValue, trialID, id, envNextVal); @@ -1208,10 +1241,13 @@ private void addYearToStudyAdditionalInfo(Program program, BrAPIStudy study, Str private void fetchOrCreateLocationPIO(ExperimentObservation importRow) { PendingImportObject pio; - if (! locationByName.containsKey((importRow.getEnvLocation()))) { - ProgramLocation newLocation = importRow.constructProgramLocation(); + String envLocationName = hasAllReferenceUnitIds ? + pendingObsUnitByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getLocationName() : importRow.getEnvLocation(); + if (!locationByName.containsKey((importRow.getEnvLocation()))) { + ProgramLocation newLocation = new ProgramLocation(); + newLocation.setName(envLocationName); pio = new PendingImportObject<>(ImportObjectState.NEW, newLocation, UUID.randomUUID()); - this.locationByName.put(importRow.getEnvLocation(), pio); + this.locationByName.put(envLocationName, pio); } } @@ -1226,10 +1262,7 @@ private PendingImportObject fetchOrCreateTrialPIO( // use the prior trial if observation unit IDs are supplied if (hasAllReferenceUnitIds) { - if (trialByNameNoScope.size() != 1) { - throw new UnprocessableEntityException(MULTIPLE_EXP_TITLES); - } - trialPio = trialByNameNoScope.values().iterator().next(); + trialPio = getSingleEntryValue(trialByNameNoScope, MULTIPLE_EXP_TITLES); // otherwise create a new trial, but there can be only one allowed } else { @@ -1548,8 +1581,10 @@ private Map> initializeTrialByNameNoScop .collect(Collectors.toList()); try { String trialRefSource = String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName()); - brapiTrialDAO.getTrialsByName(uniqueTrialNames, program) - .forEach(existingTrial -> processAndCacheTrial(existingTrial, program, trialRefSource, trialByName)); + + brapiTrialDAO.getTrialsByName(uniqueTrialNames, program).forEach(existingTrial -> + processAndCacheTrial(existingTrial, program, trialRefSource, trialByName) + ); } catch (ApiException e) { log.error("Error fetching trials: " + Utilities.generateApiExceptionLogMessage(e), e); throw new InternalServerException(e.toString(), e); @@ -2116,4 +2151,11 @@ private String seasonDbIdToYearFromDatabase(String seasonDbId, UUID programId) { Integer yearInt = (season == null) ? null : season.getYear(); return (yearInt == null) ? "" : yearInt.toString(); } + + private V getSingleEntryValue(Map map, String message) throws UnprocessableEntityException { + if (map.size() != 1) { + throw new UnprocessableEntityException(message); + } + return map.values().iterator().next(); + } } From 87cc4bdc454fed889388d8ab5a548f8018d9685f Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 4 Mar 2024 15:24:50 -0500 Subject: [PATCH 08/40] [BI-2046] initialize brapi maps when OUIds supplied --- .../processors/ExperimentProcessor.java | 111 ++++++++++++++---- 1 file changed, 88 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index adbef9852..c38fd0774 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -24,6 +24,7 @@ import io.micronaut.http.HttpStatus; import io.micronaut.http.exceptions.HttpStatusException; import io.micronaut.http.server.exceptions.InternalServerException; +import io.reactivex.functions.Function; import lombok.extern.slf4j.Slf4j; import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.collections4.map.CaseInsensitiveMap; @@ -46,6 +47,7 @@ import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; import org.breedinginsight.brapi.v2.dao.*; import org.breedinginsight.brapps.importer.model.ImportUpload; +import org.breedinginsight.brapps.importer.model.base.Study; import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; import org.breedinginsight.brapps.importer.model.imports.ChangeLogEntry; import org.breedinginsight.brapps.importer.model.imports.PendingImport; @@ -197,9 +199,18 @@ public void getExistingBrapiData(List importRows, Program program) // get all prior units referenced in import pendingObsUnitByOUId = fetchReferenceObservationUnits(referenceOUIds, program); observationUnitByNameNoScope = mapPendingObservationUnitByName(pendingObsUnitByOUId, program); + initializeTrialsForExistingObservationUnits(program, trialByNameNoScope); + initializeStudiesForExistingObservationUnits(program, studyByNameNoScope); + locationByName = initializeLocationByName(program, studyByNameNoScope); + + + pendingStudyByOUId = fetchStudyByOUId(referenceOUIds, pendingObsUnitByOUId, program); } catch (ApiException e) { log.error("Error fetching observation units: " + Utilities.generateApiExceptionLogMessage(e), e); throw new InternalServerException(e.toString(), e); + } catch (Exception e) { + log.error("Error processing experiment with ", e); + throw new InternalServerException(e.toString(), e); } } else if (hasNoReferenceUnitIds) { observationUnitByNameNoScope = initializeObservationUnits(program, experimentImportRows); @@ -1489,6 +1500,15 @@ private Map> initializeObserva } } + private Map> mapPendingTrialOUId( + Map> trialByName, + Program program + ) { + Map> trialByOUId = new HashMap<>(); + + + return trialByOUId; + } private Map> mapPendingObservationUnitByName( Map> pendingUnitById, Program program @@ -1508,9 +1528,6 @@ private Map> mapPendingObserva return pendingUnitByName; } - - - /** * Retrieves a list of pending Observation Units based on their IDs. * @@ -1534,13 +1551,13 @@ private Map> fetchReferenceObs ); // Construct the DeltaBreed observation unit source for external references - String deltabreedOUSource = String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName()); + String deltaBreedOUSource = String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName()); if (referenceObsUnits.size() == referenceOUIds.size()) { // Iterate through reference Observation Units referenceObsUnits.forEach(unit -> { // Get external reference for the Observation Unit - BrAPIExternalReference unitXref = Utilities.getExternalReference(unit.getExternalReferences(), deltabreedOUSource) + BrAPIExternalReference unitXref = Utilities.getExternalReference(unit.getExternalReferences(), deltaBreedOUSource) .orElseThrow(() -> new IllegalStateException("External reference does not exist for Deltabreed ObservationUnit ID")); // Set pending Observation Unit by its ID @@ -1554,7 +1571,7 @@ private Map> fetchReferenceObs // Handle missing Observation Unit IDs List missingIds = new ArrayList<>(referenceOUIds); Set fetchedIds = referenceObsUnits.stream().map(unit -> - Utilities.getExternalReference(unit.getExternalReferences(), deltabreedOUSource) + Utilities.getExternalReference(unit.getExternalReferences(), deltaBreedOUSource) .orElseThrow(() -> new InternalServerException("External reference does not exist for Deltabreed ObservationUnit ID")) .getReferenceId()) .collect(Collectors.toSet()); @@ -1580,10 +1597,8 @@ private Map> initializeTrialByNameNoScop .distinct() .collect(Collectors.toList()); try { - String trialRefSource = String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName()); - brapiTrialDAO.getTrialsByName(uniqueTrialNames, program).forEach(existingTrial -> - processAndCacheTrial(existingTrial, program, trialRefSource, trialByName) + processAndCacheTrial(existingTrial, program, trialByName) ); } catch (ApiException e) { log.error("Error fetching trials: " + Utilities.generateApiExceptionLogMessage(e), e); @@ -1604,6 +1619,9 @@ private Map> initializeStudyByNameNoScop } catch (ApiException e) { log.error("Error fetching studies: " + Utilities.generateApiExceptionLogMessage(e), e); throw new InternalServerException(e.toString(), e); + } catch (Exception e) { + log.error("Error processing studies", e); + throw new InternalServerException(e.toString(), e); } List existingStudies; @@ -1615,16 +1633,24 @@ private Map> initializeStudyByNameNoScop } UUID experimentId = trial.get().getId(); existingStudies = brAPIStudyDAO.getStudiesByExperimentID(experimentId, program); - existingStudies.forEach(existingStudy -> processAndCacheStudy(existingStudy, program, studyByName)); + for (BrAPIStudy existingStudy : existingStudies) { + processAndCacheStudy(existingStudy, program, BrAPIStudy::getStudyName, studyByName); + } } catch (ApiException e) { log.error("Error fetching studies: " + Utilities.generateApiExceptionLogMessage(e), e); throw new InternalServerException(e.toString(), e); + } catch (Exception e) { + log.error("Error processing studies: ", e); + throw new InternalServerException(e.toString(), e); } return studyByName; } - private void initializeStudiesForExistingObservationUnits(Program program, Map> studyByName) throws ApiException { + private void initializeStudiesForExistingObservationUnits( + Program program, + Map> studyByName + ) throws Exception { Set studyDbIds = observationUnitByNameNoScope.values() .stream() .map(pio -> pio.getBrAPIObject() @@ -1632,7 +1658,9 @@ private void initializeStudiesForExistingObservationUnits(Program program, Map studies = fetchStudiesByDbId(studyDbIds, program); - studies.forEach(study -> processAndCacheStudy(study, program, studyByName)); + for (BrAPIStudy study : studies) { + processAndCacheStudy(study, program, BrAPIStudy::getStudyName, studyByName); + } } /** @@ -1659,6 +1687,33 @@ private List fetchStudiesByDbId(Set studyDbIds, Program prog return studies; } + Map> initializeLocationByName( + Program program, + Map> studyByName + ) { + Map> locationByName = new HashMap<>(); + + List existingLocations = new ArrayList<>(); + if(studyByName.size() > 0) { + Set locationDbIds = studyByName.values() + .stream() + .map(study -> study.getBrAPIObject() + .getLocationDbId()) + .collect(Collectors.toSet()); + try { + existingLocations.addAll(locationService.getLocationsByDbId(locationDbIds, program.getId())); + } catch (ApiException e) { + log.error("Error fetching locations: " + Utilities.generateApiExceptionLogMessage(e), e); + throw new InternalServerException(e.toString(), e); + } + } + existingLocations.forEach(existingLocation -> locationByName.put( + existingLocation.getName(), + new PendingImportObject<>(ImportObjectState.EXISTING, existingLocation, existingLocation.getId()) + ) + ); + return locationByName; + } private Map> initializeUniqueLocationNames(Program program, List experimentImportRows) { Map> locationByName = new HashMap<>(); @@ -1794,8 +1849,13 @@ private void processAndCacheObservationUnit(BrAPIObservationUnit brAPIObservatio brAPIObservationUnit, UUID.fromString(idRef.getReferenceId()))); } - - private void processAndCacheStudy(BrAPIStudy existingStudy, Program program, Map> studyByName) { + private PendingImportObject processAndCacheStudy( + BrAPIStudy existingStudy, + Program program, + Function getterFunction, + Map> studyMap + ) throws Exception { + PendingImportObject pendingStudy; BrAPIExternalReference xref = Utilities.getExternalReference(existingStudy.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.STUDIES.getName())) .orElseThrow(() -> new IllegalStateException("External references wasn't found for study (dbid): " + existingStudy.getStudyDbId())); // map season dbid to year @@ -1804,9 +1864,16 @@ private void processAndCacheStudy(BrAPIStudy existingStudy, Program program, Map String seasonYear = this.seasonDbIdToYear(seasonDbId, program.getId()); existingStudy.setSeasons(Collections.singletonList(seasonYear)); } - studyByName.put( - Utilities.removeProgramKeyAndUnknownAdditionalData(existingStudy.getStudyName(), program.getKey()), - new PendingImportObject<>(ImportObjectState.EXISTING, (BrAPIStudy) Utilities.formatBrapiObjForDisplay(existingStudy, BrAPIStudy.class, program), UUID.fromString(xref.getReferenceID()))); + pendingStudy = new PendingImportObject<>( + ImportObjectState.EXISTING, + (BrAPIStudy) Utilities.formatBrapiObjForDisplay(existingStudy, BrAPIStudy.class, program), + UUID.fromString(xref.getReferenceId()) + ); + studyMap.put( + Utilities.removeProgramKeyAndUnknownAdditionalData(getterFunction.apply(existingStudy), program.getKey()), + pendingStudy + ); + return pendingStudy; } private void setPendingTrialByOUId(Program program) { @@ -1892,8 +1959,7 @@ private void initializeTrialsForExistingObservationUnits(Program program, Map processAndCacheTrial(trial, program, trialRefSource, trialByName)); + trials.forEach(trial -> processAndCacheTrial(trial, program, trialByName)); } catch (ApiException e) { log.error("Error fetching trials: " + Utilities.generateApiExceptionLogMessage(e), e); throw new InternalServerException(e.toString(), e); @@ -1915,16 +1981,15 @@ private Set fetchTrialDbidsForStudies(Set studyDbIds, Program pr } private void processAndCacheTrial( - BrAPITrial existingTrial, + BrAPITrial existingTrial, Program program, - String trialRefSource, Map> trialByNameNoScope ) { //get TrialId from existingTrial - BrAPIExternalReference experimentIDRef = Utilities.getExternalReference(existingTrial.getExternalReferences(), trialRefSource) + BrAPIExternalReference experimentIDRef = Utilities.getExternalReference(existingTrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())) .orElseThrow(() -> new InternalServerException("An Experiment ID was not found in any of the external references")); - UUID experimentId = UUID.fromString(experimentIDRef.getReferenceID()); + UUID experimentId = UUID.fromString(experimentIDRef.getReferenceId()); trialByNameNoScope.put( Utilities.removeProgramKey(existingTrial.getTrialName(), program.getKey()), From 8d3c29a8e14e538196090b1fbe732199a49ef6ae Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 4 Mar 2024 17:13:33 -0500 Subject: [PATCH 09/40] [BI-2046] map trials and studies to OU Id --- .../processors/ExperimentProcessor.java | 44 +++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index c38fd0774..763c740ca 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -202,7 +202,12 @@ public void getExistingBrapiData(List importRows, Program program) initializeTrialsForExistingObservationUnits(program, trialByNameNoScope); initializeStudiesForExistingObservationUnits(program, studyByNameNoScope); locationByName = initializeLocationByName(program, studyByNameNoScope); - + for (Map.Entry> unitEntry : pendingObsUnitByOUId.entrySet()) { + String unitId = unitEntry.getKey(); + BrAPIObservationUnit unit = unitEntry.getValue().getBrAPIObject(); + mapPendingTrialByOUId(unitId, unit, trialByNameNoScope, studyByNameNoScope, pendingTrialByOUId, program); + mapPendingStudyByOUId(unitId, unit, studyByNameNoScope, pendingStudyByOUId, program); + } pendingStudyByOUId = fetchStudyByOUId(referenceOUIds, pendingObsUnitByOUId, program); } catch (ApiException e) { @@ -1500,12 +1505,43 @@ private Map> initializeObserva } } - private Map> mapPendingTrialOUId( - Map> trialByName, + private Map> mapPendingStudyByOUId( + String unitId, + BrAPIObservationUnit unit, + Map> studyByName, + Map> studyByOUId, Program program ) { - Map> trialByOUId = new HashMap<>(); + if (unit.getStudyName() != null) { + String studyName = Utilities.removeProgramKeyAndUnknownAdditionalData(unit.getStudyName(), program.getKey()); + studyByOUId.put(unitId, studyByName.get(studyName)); + } else { + throw new IllegalStateException("Observation unit missing study name: " + unitId); + } + return studyByOUId; + } + private Map> mapPendingTrialByOUId( + String unitId, + BrAPIObservationUnit unit, + Map> trialByName, + Map> studyByName, + Map> trialByOUId, + Program program + ) { + String trialName; + if (unit.getTrialName() != null) { + trialName = Utilities.removeProgramKeyAndUnknownAdditionalData(unit.getTrialName(), program.getKey()); + } else if (unit.getStudyName() != null) { + String studyName = Utilities.removeProgramKeyAndUnknownAdditionalData(unit.getStudyName(), program.getKey()); + trialName = Utilities.removeProgramKeyAndUnknownAdditionalData( + studyByName.get(studyName).getBrAPIObject().getTrialName(), + program.getKey() + ); + } else { + throw new IllegalStateException("Observation unit missing trial name and study name: " + unitId); + } + trialByOUId.put(unitId, trialByName.get(trialName)); return trialByOUId; } From 6cb0251360e0c89ed96479ab4b4ff774a6a3e528 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 4 Mar 2024 17:42:11 -0500 Subject: [PATCH 10/40] [BI-2046] map locations by OU id --- .../processors/ExperimentProcessor.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 763c740ca..61e03c3f5 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -207,6 +207,8 @@ public void getExistingBrapiData(List importRows, Program program) BrAPIObservationUnit unit = unitEntry.getValue().getBrAPIObject(); mapPendingTrialByOUId(unitId, unit, trialByNameNoScope, studyByNameNoScope, pendingTrialByOUId, program); mapPendingStudyByOUId(unitId, unit, studyByNameNoScope, pendingStudyByOUId, program); + mapPendingLocationByOUId(unitId, unit, pendingStudyByOUId, locationByName, pendingLocationByOUId); + } pendingStudyByOUId = fetchStudyByOUId(referenceOUIds, pendingObsUnitByOUId, program); @@ -1505,6 +1507,27 @@ private Map> initializeObserva } } + private Map> mapPendingLocationByOUId( + String unitId, + BrAPIObservationUnit unit, + Map> studyByOUId, + Map> locationByName, + Map> locationByOUId + ) { + if (unit.getLocationName() != null) { + locationByOUId.put(unitId, locationByName.get(unit.getLocationName())); + } else if (studyByOUId.get(unitId) != null && studyByOUId.get(unitId).getBrAPIObject().getLocationName() != null) { + locationByOUId.put( + unitId, + locationByName.get(studyByOUId.get(unitId).getBrAPIObject().getLocationName()) + ); + } else { + throw new IllegalStateException("Observation unit missing location: " + unitId); + } + + return locationByOUId; + } + private Map> mapPendingStudyByOUId( String unitId, BrAPIObservationUnit unit, From 7b3eb166dee3b06e5619ef60a565b68bc0830434 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 5 Mar 2024 12:32:53 -0500 Subject: [PATCH 11/40] [BI-2046] initialize dataset maps for existing observation units --- .../processors/ExperimentProcessor.java | 49 ++++++++++++++++++- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 61e03c3f5..7ad90a13f 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -208,7 +208,7 @@ public void getExistingBrapiData(List importRows, Program program) mapPendingTrialByOUId(unitId, unit, trialByNameNoScope, studyByNameNoScope, pendingTrialByOUId, program); mapPendingStudyByOUId(unitId, unit, studyByNameNoScope, pendingStudyByOUId, program); mapPendingLocationByOUId(unitId, unit, pendingStudyByOUId, locationByName, pendingLocationByOUId); - + initializeObsVarDatasetForExistingObservationUnit(unitId, pendingObsDatasetByOUId, obsVarDatasetByName, pendingTrialByOUId, program); } pendingStudyByOUId = fetchStudyByOUId(referenceOUIds, pendingObsUnitByOUId, program); @@ -1808,6 +1808,43 @@ private Map> initializeUniqueLocati existingLocations.forEach(existingLocation -> locationByName.put(existingLocation.getName(), new PendingImportObject<>(ImportObjectState.EXISTING, existingLocation, existingLocation.getId()))); return locationByName; } + + private Map> initializeObsVarDatasetForExistingObservationUnit( + String unitId, + Map> obsVarDatasetByOUId, + Map> obsVarDatasetByName, + Map> pendingTrialByOUId, + Program program + ) { + if (pendingTrialByOUId.get(unitId) != null && + pendingTrialByOUId.get(unitId).getBrAPIObject().getAdditionalInfo().has(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID)) { + String datasetId = pendingTrialByOUId.get(unitId).getBrAPIObject() + .getAdditionalInfo() + .get(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID) + .getAsString(); + + try { + List existingDatasets = brAPIListDAO + .getListByTypeAndExternalRef(BrAPIListTypes.OBSERVATIONVARIABLES, + program.getId(), + String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName()), + UUID.fromString(datasetId)); + if (existingDatasets == null || existingDatasets.isEmpty()) { + throw new InternalServerException("existing dataset summary not returned from brapi server"); + } + BrAPIListDetails dataSetDetails = brAPIListDAO + .getListById(existingDatasets.get(0).getListDbId(), program.getId()) + .getResult(); + processAndCacheObsVarDataset(dataSetDetails, obsVarDatasetByName); + processAndCacheObsVarDatasetByOUId(dataSetDetails, unitId, obsVarDatasetByOUId); + } catch (ApiException e) { + log.error(Utilities.generateApiExceptionLogMessage(e), e); + throw new InternalServerException(e.toString(), e); + } + } + return obsVarDatasetByOUId; + } + private Map> initializeObsVarDatasetByName(Program program, List experimentImportRows) { Map> obsVarDatasetByName = new HashMap<>(); @@ -1854,12 +1891,20 @@ private Optional> getTrialPIO(List> obsVarDatasetByOUId) { + BrAPIExternalReference xref = Utilities.getExternalReference(existingList.getExternalReferences(), + String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName())) + .orElseThrow(() -> new IllegalStateException("External references wasn't found for list (dbid): " + existingList.getListDbId())); + obsVarDatasetByOUId.put(unitId, + new PendingImportObject(ImportObjectState.EXISTING, existingList, UUID.fromString(xref.getReferenceId()))); + } private void processAndCacheObsVarDataset(BrAPIListDetails existingList, Map> obsVarDatasetByName) { BrAPIExternalReference xref = Utilities.getExternalReference(existingList.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName())) .orElseThrow(() -> new IllegalStateException("External references wasn't found for list (dbid): " + existingList.getListDbId())); obsVarDatasetByName.put(existingList.getListName(), - new PendingImportObject(ImportObjectState.EXISTING, existingList, UUID.fromString(xref.getReferenceID()))); + new PendingImportObject(ImportObjectState.EXISTING, existingList, UUID.fromString(xref.getReferenceId()))); } private Map> initializeExistingGermplasmByGID(Program program, List experimentImportRows) { Map> existingGermplasmByGID = new HashMap<>(); From 346b629bd9d7d69f08f342e829facb72664baffd Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 5 Mar 2024 13:35:26 -0500 Subject: [PATCH 12/40] [BI-2046] refactor initialization of dataset map --- .../processors/ExperimentProcessor.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 7ad90a13f..1ad614d5a 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -202,13 +202,13 @@ public void getExistingBrapiData(List importRows, Program program) initializeTrialsForExistingObservationUnits(program, trialByNameNoScope); initializeStudiesForExistingObservationUnits(program, studyByNameNoScope); locationByName = initializeLocationByName(program, studyByNameNoScope); + initializeObsVarDatasetForExistingObservationUnits(pendingTrialByOUId, program); for (Map.Entry> unitEntry : pendingObsUnitByOUId.entrySet()) { String unitId = unitEntry.getKey(); BrAPIObservationUnit unit = unitEntry.getValue().getBrAPIObject(); mapPendingTrialByOUId(unitId, unit, trialByNameNoScope, studyByNameNoScope, pendingTrialByOUId, program); mapPendingStudyByOUId(unitId, unit, studyByNameNoScope, pendingStudyByOUId, program); mapPendingLocationByOUId(unitId, unit, pendingStudyByOUId, locationByName, pendingLocationByOUId); - initializeObsVarDatasetForExistingObservationUnit(unitId, pendingObsDatasetByOUId, obsVarDatasetByName, pendingTrialByOUId, program); } pendingStudyByOUId = fetchStudyByOUId(referenceOUIds, pendingObsUnitByOUId, program); @@ -1809,16 +1809,15 @@ private Map> initializeUniqueLocati return locationByName; } - private Map> initializeObsVarDatasetForExistingObservationUnit( - String unitId, - Map> obsVarDatasetByOUId, - Map> obsVarDatasetByName, + private Map> initializeObsVarDatasetForExistingObservationUnits( Map> pendingTrialByOUId, Program program ) { - if (pendingTrialByOUId.get(unitId) != null && - pendingTrialByOUId.get(unitId).getBrAPIObject().getAdditionalInfo().has(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID)) { - String datasetId = pendingTrialByOUId.get(unitId).getBrAPIObject() + Map> obsVarDatasetByName = new HashMap<>(); + + if (pendingTrialByOUId.size() > 0 && + pendingTrialByOUId.values().iterator().next().getBrAPIObject().getAdditionalInfo().has(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID)) { + String datasetId = pendingTrialByOUId.values().iterator().next().getBrAPIObject() .getAdditionalInfo() .get(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID) .getAsString(); @@ -1836,13 +1835,12 @@ private Map> initializeObsVarDatas .getListById(existingDatasets.get(0).getListDbId(), program.getId()) .getResult(); processAndCacheObsVarDataset(dataSetDetails, obsVarDatasetByName); - processAndCacheObsVarDatasetByOUId(dataSetDetails, unitId, obsVarDatasetByOUId); } catch (ApiException e) { log.error(Utilities.generateApiExceptionLogMessage(e), e); throw new InternalServerException(e.toString(), e); } } - return obsVarDatasetByOUId; + return obsVarDatasetByName; } private Map> initializeObsVarDatasetByName(Program program, List experimentImportRows) { From 18ff6e23338855e4f02897fcd4fb35ac70603b3c Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 5 Mar 2024 14:40:20 -0500 Subject: [PATCH 13/40] [BI-2046] map obsVarDataset by OU id --- .../services/processors/ExperimentProcessor.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 1ad614d5a..f75c6aacc 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -209,6 +209,7 @@ public void getExistingBrapiData(List importRows, Program program) mapPendingTrialByOUId(unitId, unit, trialByNameNoScope, studyByNameNoScope, pendingTrialByOUId, program); mapPendingStudyByOUId(unitId, unit, studyByNameNoScope, pendingStudyByOUId, program); mapPendingLocationByOUId(unitId, unit, pendingStudyByOUId, locationByName, pendingLocationByOUId); + mapPendingObsDatasetByOUId(unitId, pendingTrialByOUId, obsVarDatasetByName, pendingObsDatasetByOUId, program); } pendingStudyByOUId = fetchStudyByOUId(referenceOUIds, pendingObsUnitByOUId, program); @@ -1809,6 +1810,20 @@ private Map> initializeUniqueLocati return locationByName; } + private Map> mapPendingObsDatasetByOUId( + String unitId, + Map> trialByOUId, + Map> obsVarDatasetByName, + Map> obsVarDatasetByOUId, + Program program + ) { + if (!trialByOUId.isEmpty() && !obsVarDatasetByName.isEmpty() && + trialByOUId.values().iterator().next().getBrAPIObject().getAdditionalInfo().has(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID)) { + obsVarDatasetByOUId.put(unitId, obsVarDatasetByName.values().iterator().next()); + } + + return obsVarDatasetByOUId; + } private Map> initializeObsVarDatasetForExistingObservationUnits( Map> pendingTrialByOUId, Program program From aa397ce6353752d4cbac9e070709eef9d517035b Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 5 Mar 2024 17:57:17 -0500 Subject: [PATCH 14/40] [BI-2046] fix NPEs --- .../processors/ExperimentProcessor.java | 66 +++++++++++++++---- 1 file changed, 53 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index f75c6aacc..fc0307668 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -47,6 +47,7 @@ import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; import org.breedinginsight.brapi.v2.dao.*; import org.breedinginsight.brapps.importer.model.ImportUpload; +import org.breedinginsight.brapps.importer.model.base.ObservationUnit; import org.breedinginsight.brapps.importer.model.base.Study; import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; import org.breedinginsight.brapps.importer.model.imports.ChangeLogEntry; @@ -122,23 +123,24 @@ public class ExperimentProcessor implements Processor { //These BrapiData-objects are initially populated by the getExistingBrapiData() method, // then updated by the initNewBrapiData() method. - private Map> trialByNameNoScope = null; - private Map> pendingTrialByOUId = null; + private Map> trialByNameNoScope = new HashMap<>(); + private Map> pendingTrialByOUId = new HashMap<>(); private Map> locationByName = null; - private Map> pendingLocationByOUId = null; - private Map> studyByNameNoScope = null; - private Map> pendingStudyByOUId = null; + private Map> pendingLocationByOUId = new HashMap<>(); + private Map> studyByNameNoScope = new HashMap<>(); + private Map> pendingStudyByOUId = new HashMap<>(); private Map> obsVarDatasetByName = null; - private Map> pendingObsDatasetByOUId = null; + private Map> pendingObsDatasetByOUId = new HashMap<>(); // It is assumed that there are no preexisting Observation Units for the given environment (so this will not be // initialized by getExistingBrapiData() ) private Map> observationUnitByNameNoScope = null; - private Map> pendingObsUnitByOUId = null; + private Map> pendingObsUnitByOUId = new HashMap<>(); private final Map> observationByHash = new HashMap<>(); private Map existingObsByObsHash = new HashMap<>(); // existingGermplasmByGID is populated by getExistingBrapiData(), but not updated by the initNewBrapiData() method private Map> existingGermplasmByGID = null; + private Map> pendingGermplasmByOUId = null; // Associates timestamp columns to associated phenotype column name for ease of storage private final Map> timeStampColByPheno = new HashMap<>(); @@ -202,7 +204,8 @@ public void getExistingBrapiData(List importRows, Program program) initializeTrialsForExistingObservationUnits(program, trialByNameNoScope); initializeStudiesForExistingObservationUnits(program, studyByNameNoScope); locationByName = initializeLocationByName(program, studyByNameNoScope); - initializeObsVarDatasetForExistingObservationUnits(pendingTrialByOUId, program); + obsVarDatasetByName = initializeObsVarDatasetForExistingObservationUnits(trialByNameNoScope, program); + existingGermplasmByGID = initializeGermplasmByGIDForExistingObservationUnits(observationUnitByNameNoScope, program); for (Map.Entry> unitEntry : pendingObsUnitByOUId.entrySet()) { String unitId = unitEntry.getKey(); BrAPIObservationUnit unit = unitEntry.getValue().getBrAPIObject(); @@ -210,9 +213,10 @@ public void getExistingBrapiData(List importRows, Program program) mapPendingStudyByOUId(unitId, unit, studyByNameNoScope, pendingStudyByOUId, program); mapPendingLocationByOUId(unitId, unit, pendingStudyByOUId, locationByName, pendingLocationByOUId); mapPendingObsDatasetByOUId(unitId, pendingTrialByOUId, obsVarDatasetByName, pendingObsDatasetByOUId, program); + mapGermplasmByOUId(unitId, unit, existingGermplasmByGID, pendingGermplasmByOUId); } - pendingStudyByOUId = fetchStudyByOUId(referenceOUIds, pendingObsUnitByOUId, program); + //pendingStudyByOUId = fetchStudyByOUId(referenceOUIds, pendingObsUnitByOUId, program); } catch (ApiException e) { log.error("Error fetching observation units: " + Utilities.generateApiExceptionLogMessage(e), e); throw new InternalServerException(e.toString(), e); @@ -1810,6 +1814,16 @@ private Map> initializeUniqueLocati return locationByName; } + private Map> mapGermplasmByOUId( + String unitId, + BrAPIObservationUnit unit, + Map> germplasmByName, + Map> germplasmByOUId + ) { + //String dbId = unit. + //germplasmByName.values().stream().filter(pio -> pio.getBrAPIObject().getGermplasmDbId()) + return germplasmByOUId; + } private Map> mapPendingObsDatasetByOUId( String unitId, Map> trialByOUId, @@ -1825,14 +1839,14 @@ private Map> mapPendingObsDatasetB return obsVarDatasetByOUId; } private Map> initializeObsVarDatasetForExistingObservationUnits( - Map> pendingTrialByOUId, + Map> trialByName, Program program ) { Map> obsVarDatasetByName = new HashMap<>(); - if (pendingTrialByOUId.size() > 0 && - pendingTrialByOUId.values().iterator().next().getBrAPIObject().getAdditionalInfo().has(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID)) { - String datasetId = pendingTrialByOUId.values().iterator().next().getBrAPIObject() + if (trialByName.size() > 0 && + trialByName.values().iterator().next().getBrAPIObject().getAdditionalInfo().has(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID)) { + String datasetId = trialByName.values().iterator().next().getBrAPIObject() .getAdditionalInfo() .get(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID) .getAsString(); @@ -1919,6 +1933,32 @@ private void processAndCacheObsVarDataset(BrAPIListDetails existingList, Map(ImportObjectState.EXISTING, existingList, UUID.fromString(xref.getReferenceId()))); } + private Map> initializeGermplasmByGIDForExistingObservationUnits( + Map> unitByName, + Program program + ) { + Map> existingGermplasmByGID = new HashMap<>(); + + List existingGermplasms = new ArrayList<>(); + if(unitByName.size() > 0) { + Set germplasmDbIds = unitByName.values().stream().map(ou -> ou.getBrAPIObject().getGermplasmDbId()).collect(Collectors.toSet()); + try { + existingGermplasms.addAll(brAPIGermplasmDAO.getGermplasmsByDBID(germplasmDbIds, program.getId())); + } catch (ApiException e) { + log.error("Error fetching germplasm: " + Utilities.generateApiExceptionLogMessage(e), e); + throw new InternalServerException(e.toString(), e); + } + } + + existingGermplasms.forEach(existingGermplasm -> { + BrAPIExternalReference xref = Utilities.getExternalReference(existingGermplasm.getExternalReferences(), String.format("%s", BRAPI_REFERENCE_SOURCE)) + .orElseThrow(() -> new IllegalStateException("External references wasn't found for germplasm (dbid): " + existingGermplasm.getGermplasmDbId())); + existingGermplasmByGID.put(existingGermplasm.getAccessionNumber(), new PendingImportObject<>(ImportObjectState.EXISTING, existingGermplasm, UUID.fromString(xref.getReferenceID()))); + }); + return existingGermplasmByGID; + } + + private Map> initializeExistingGermplasmByGID(Program program, List experimentImportRows) { Map> existingGermplasmByGID = new HashMap<>(); From 9c33f31eeadf052a65d16aa7b8644038f904fa1f Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 5 Mar 2024 18:16:06 -0500 Subject: [PATCH 15/40] [BI-2046] map germplasm by observation unit id --- .../services/processors/ExperimentProcessor.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index fc0307668..347e894b6 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -139,8 +139,8 @@ public class ExperimentProcessor implements Processor { private final Map> observationByHash = new HashMap<>(); private Map existingObsByObsHash = new HashMap<>(); // existingGermplasmByGID is populated by getExistingBrapiData(), but not updated by the initNewBrapiData() method - private Map> existingGermplasmByGID = null; - private Map> pendingGermplasmByOUId = null; + private Map> existingGermplasmByGID = new HashMap<>(); + private Map> pendingGermplasmByOUId = new HashMap<>(); // Associates timestamp columns to associated phenotype column name for ease of storage private final Map> timeStampColByPheno = new HashMap<>(); @@ -216,7 +216,6 @@ public void getExistingBrapiData(List importRows, Program program) mapGermplasmByOUId(unitId, unit, existingGermplasmByGID, pendingGermplasmByOUId); } - //pendingStudyByOUId = fetchStudyByOUId(referenceOUIds, pendingObsUnitByOUId, program); } catch (ApiException e) { log.error("Error fetching observation units: " + Utilities.generateApiExceptionLogMessage(e), e); throw new InternalServerException(e.toString(), e); @@ -1820,8 +1819,9 @@ private Map> mapGermplasmByOUId( Map> germplasmByName, Map> germplasmByOUId ) { - //String dbId = unit. - //germplasmByName.values().stream().filter(pio -> pio.getBrAPIObject().getGermplasmDbId()) + String gid = unit.getAdditionalInfo().getAsJsonObject().get(BrAPIAdditionalInfoFields.GID).getAsString(); + germplasmByOUId.put(unitId, germplasmByName.get(gid)); + return germplasmByOUId; } private Map> mapPendingObsDatasetByOUId( From e9ef80d3a50579a92c61563df284bfa4186d0e92 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 5 Mar 2024 18:18:37 -0500 Subject: [PATCH 16/40] [BI-2046] refaactor --- .../services/processors/ExperimentProcessor.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 347e894b6..5a4120e00 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -225,17 +225,17 @@ public void getExistingBrapiData(List importRows, Program program) } } else if (hasNoReferenceUnitIds) { observationUnitByNameNoScope = initializeObservationUnits(program, experimentImportRows); - + trialByNameNoScope = initializeTrialByNameNoScope(program, experimentImportRows); + studyByNameNoScope = initializeStudyByNameNoScope(program, experimentImportRows); + locationByName = initializeUniqueLocationNames(program, experimentImportRows); + obsVarDatasetByName = initializeObsVarDatasetByName(program, experimentImportRows); + existingGermplasmByGID = initializeExistingGermplasmByGID(program, experimentImportRows); + } else { // can't proceed if you have a mix of ObsUnitId for some but not all rows throw new HttpStatusException(HttpStatus.UNPROCESSABLE_ENTITY, MISSING_OBS_UNIT_ID_ERROR); } - trialByNameNoScope = initializeTrialByNameNoScope(program, experimentImportRows); - studyByNameNoScope = initializeStudyByNameNoScope(program, experimentImportRows); - locationByName = initializeUniqueLocationNames(program, experimentImportRows); - obsVarDatasetByName = initializeObsVarDatasetByName(program, experimentImportRows); - existingGermplasmByGID = initializeExistingGermplasmByGID(program, experimentImportRows); } /** From 74eeeb5f3bf30385bb26027a40083b1649a39438 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 6 Mar 2024 17:13:32 -0500 Subject: [PATCH 17/40] [BI-2046] prepare for validation using reference OU ids --- .../processors/ExperimentProcessor.java | 50 +++++++++++++------ 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 5a4120e00..7eaf38550 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -230,7 +230,7 @@ public void getExistingBrapiData(List importRows, Program program) locationByName = initializeUniqueLocationNames(program, experimentImportRows); obsVarDatasetByName = initializeObsVarDatasetByName(program, experimentImportRows); existingGermplasmByGID = initializeExistingGermplasmByGID(program, experimentImportRows); - + } else { // can't proceed if you have a mix of ObsUnitId for some but not all rows @@ -467,22 +467,42 @@ public void postBrapiData(Map mappedBrAPIImport, Program private void prepareDataForValidation(List importRows, List> phenotypeCols, Map mappedBrAPIImport) { for (int rowNum = 0; rowNum < importRows.size(); rowNum++) { ExperimentObservation importRow = (ExperimentObservation) importRows.get(rowNum); - PendingImport mappedImportRow = mappedBrAPIImport.getOrDefault(rowNum, new PendingImport()); - mappedImportRow.setTrial(this.trialByNameNoScope.get(importRow.getExpTitle())); - mappedImportRow.setLocation(this.locationByName.get(importRow.getEnvLocation())); - mappedImportRow.setStudy(this.studyByNameNoScope.get(importRow.getEnv())); - mappedImportRow.setObservationUnit(this.observationUnitByNameNoScope.get(createObservationUnitKey(importRow))); - - // loop over phenotype column observation data for current row List> observations = mappedImportRow.getObservations(); - for (Column column : phenotypeCols) { - // if value was blank won't be entry in map for this observation - observations.add(this.observationByHash.get(getImportObservationHash(importRow, getVariableNameFromColumn(column)))); - } + String observationHash; + if (hasAllReferenceUnitIds) { + mappedImportRow.setTrial(pendingTrialByOUId.get(importRow.getObsUnitID()); + mappedImportRow.setLocation(pendingLocationByOUId.get(importRow.getObsUnitID())); + mappedImportRow.setStudy(pendingStudyByOUId.get(importRow.getObsUnitID())); + mappedImportRow.setObservationUnit(pendingObsUnitByOUId.get(importRow.getObsUnitID())); + mappedImportRow.setGermplasm(pendingGermplasmByOUId.get(importRow.getObsUnitID())); + + // loop over phenotype column observation data for current row + for (Column column : phenotypeCols) { + observationHash = getObservationHash( + pendingObsUnitByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getObservationUnitName(), + getVariableNameFromColumn(column), + pendingStudyByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getStudyName() + ); - PendingImportObject germplasmPIO = getGidPOI(importRow); - mappedImportRow.setGermplasm(germplasmPIO); + // if value was blank won't be entry in map for this observation + observations.add(observationByHash.get(observationHash)); + } + + } else { + mappedImportRow.setTrial(trialByNameNoScope.get(importRow.getExpTitle())); + mappedImportRow.setLocation(locationByName.get(importRow.getEnvLocation())); + mappedImportRow.setStudy(studyByNameNoScope.get(importRow.getEnv())); + mappedImportRow.setObservationUnit(observationUnitByNameNoScope.get(createObservationUnitKey(importRow))); + mappedImportRow.setGermplasm(getGidPIO(importRow)); + + // loop over phenotype column observation data for current row + for (Column column : phenotypeCols) { + + // if value was blank won't be entry in map for this observation + observations.add(observationByHash.get(getImportObservationHash(importRow, getVariableNameFromColumn(column)))); + } + } mappedBrAPIImport.put(rowNum, mappedImportRow); } @@ -994,7 +1014,7 @@ private void validateTestOrCheck(ExperimentObservation importRow, ValidationErro } } - private PendingImportObject getGidPOI(ExperimentObservation importRow) { + private PendingImportObject getGidPIO(ExperimentObservation importRow) { if (this.existingGermplasmByGID.containsKey(importRow.getGid())) { return existingGermplasmByGID.get(importRow.getGid()); } From eaeb42394fab77cfa9366f0cd8dca557e417adf8 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 7 Mar 2024 10:50:30 -0500 Subject: [PATCH 18/40] [BI-2046] validate observations for reference observation units --- .../processors/ExperimentProcessor.java | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 7eaf38550..bf616273a 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -471,7 +471,7 @@ private void prepareDataForValidation(List importRows, List> observations = mappedImportRow.getObservations(); String observationHash; if (hasAllReferenceUnitIds) { - mappedImportRow.setTrial(pendingTrialByOUId.get(importRow.getObsUnitID()); + mappedImportRow.setTrial(pendingTrialByOUId.get(importRow.getObsUnitID())); mappedImportRow.setLocation(pendingLocationByOUId.get(importRow.getObsUnitID())); mappedImportRow.setStudy(pendingStudyByOUId.get(importRow.getObsUnitID())); mappedImportRow.setObservationUnit(pendingObsUnitByOUId.get(importRow.getObsUnitID())); @@ -687,17 +687,26 @@ private void validateFields(List importRows, ValidationErrors valid for (int rowNum = 0; rowNum < importRows.size(); rowNum++) { ExperimentObservation importRow = (ExperimentObservation) importRows.get(rowNum); PendingImport mappedImportRow = mappedBrAPIImport.get(rowNum); - if (StringUtils.isNotBlank(importRow.getGid())) { // if GID is blank, don't bother to check if it is valid. - validateGermplasm(importRow, validationErrors, rowNum, mappedImportRow.getGermplasm()); + if (hasAllReferenceUnitIds) { + validateObservations(validationErrors, rowNum, importRow, phenotypeCols, colVarMap, commit, user); + } else { + if (StringUtils.isNotBlank(importRow.getGid())) { // if GID is blank, don't bother to check if it is valid. + validateGermplasm(importRow, validationErrors, rowNum, mappedImportRow.getGermplasm()); + } + validateTestOrCheck(importRow, validationErrors, rowNum); + validateConditionallyRequired(validationErrors, rowNum, importRow, program, commit); + validateObservationUnits(validationErrors, uniqueStudyAndObsUnit, rowNum, importRow); + validateObservations(validationErrors, rowNum, importRow, phenotypeCols, colVarMap, commit, user); } - validateTestOrCheck(importRow, validationErrors, rowNum); - validateConditionallyRequired(validationErrors, rowNum, importRow, program, commit); - validateObservationUnits(validationErrors, uniqueStudyAndObsUnit, rowNum, importRow); - validateObservations(validationErrors, rowNum, importRow, phenotypeCols, colVarMap, commit, user); } } - private void validateObservationUnits(ValidationErrors validationErrors, Set uniqueStudyAndObsUnit, int rowNum, ExperimentObservation importRow) { + private void validateObservationUnits( + ValidationErrors validationErrors, + Set uniqueStudyAndObsUnit, + int rowNum, + ExperimentObservation importRow + ) { validateUniqueObsUnits(validationErrors, uniqueStudyAndObsUnit, rowNum, importRow); String key = createObservationUnitKey(importRow); @@ -759,9 +768,20 @@ private void validateObservations(ValidationErrors validationErrors, boolean commit, User user) { phenotypeCols.forEach(phenoCol -> { - String importHash = getImportObservationHash(importRow, phenoCol.name()); + String importHash; String importObsValue = phenoCol.getString(rowNum); + if (hasAllReferenceUnitIds) { + importHash = getObservationHash( + pendingObsUnitByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getObservationUnitName(), + getVariableNameFromColumn(phenoCol), + pendingStudyByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getStudyName() + ); + + } else { + importHash = getImportObservationHash(importRow, phenoCol.name()); + } + // error if import observation data already exists and user has not selected to overwrite if(commit && "false".equals(importRow.getOverwrite() == null ? "false" : importRow.getOverwrite()) && this.existingObsByObsHash.containsKey(importHash) && @@ -845,7 +865,8 @@ private void validateUniqueObsUnits( ValidationErrors validationErrors, Set uniqueStudyAndObsUnit, int rowNum, - ExperimentObservation importRow) { + ExperimentObservation importRow + ) { String envIdPlusStudyId = createObservationUnitKey(importRow); if (uniqueStudyAndObsUnit.contains(envIdPlusStudyId)) { String errorMessage = String.format("The ID (%s) is not unique within the environment(%s)", importRow.getExpUnitId(), importRow.getEnv()); From bd480f24cc4e218f70c4050f9569d25ef97bfd1c Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 7 Mar 2024 15:06:41 -0500 Subject: [PATCH 19/40] [BI-2046] add test for updating observations referenced by OU id --- .../importer/ExperimentFileImportTest.java | 105 +++++++++++++++--- 1 file changed, 91 insertions(+), 14 deletions(-) diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 59607a577..3ff0271f7 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -735,19 +735,6 @@ public void importNewObsVarByObsUnitId() { assertTrue(ouIdXref.isPresent()); Map newObsVar = new HashMap<>(); -// newObsVar.put(Columns.GERMPLASM_GID, "1"); -// newObsVar.put(Columns.TEST_CHECK, "T"); -// newObsVar.put(Columns.EXP_TITLE, "Test Exp"); -// newObsVar.put(Columns.EXP_UNIT, "Plot"); -// newObsVar.put(Columns.EXP_TYPE, "Phenotyping"); -// newObsVar.put(Columns.ENV, "New Env"); -// newObsVar.put(Columns.ENV_LOCATION, "Location A"); -// newObsVar.put(Columns.ENV_YEAR, "2023"); -// newObsVar.put(Columns.EXP_UNIT_ID, "a-1"); -// newObsVar.put(Columns.REP_NUM, "1"); -// newObsVar.put(Columns.BLOCK_NUM, "1"); -// newObsVar.put(Columns.ROW, "1"); -// newObsVar.put(Columns.COLUMN, "1"); newObsVar.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); newObsVar.put(traits.get(1).getObservationVariableName(), null); @@ -763,7 +750,58 @@ public void importNewObsVarByObsUnitId() { assertEquals("EXISTING", row.getAsJsonObject("location").get("state").getAsString()); assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); - assertRowSaved(newObsVar, program, traits); + assertRowReferencedByOUIdSaved(newObsVar, program, traits); + } + + @Test + @SneakyThrows + public void importNewObservationDataByObsUnitId() { + List traits = importTestUtils.createTraits(1); + Program program = createProgram("New Observation Referring to OU by ID", "OUDAT", "DATAOU", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Map newExp = new HashMap<>(); + newExp.put(Columns.GERMPLASM_GID, "1"); + newExp.put(Columns.TEST_CHECK, "T"); + newExp.put(Columns.EXP_TITLE, "Test Exp"); + newExp.put(Columns.EXP_UNIT, "Plot"); + newExp.put(Columns.EXP_TYPE, "Phenotyping"); + newExp.put(Columns.ENV, "New Env"); + newExp.put(Columns.ENV_LOCATION, "Location A"); + newExp.put(Columns.ENV_YEAR, "2023"); + newExp.put(Columns.EXP_UNIT_ID, "a-1"); + newExp.put(Columns.REP_NUM, "1"); + newExp.put(Columns.BLOCK_NUM, "1"); + newExp.put(Columns.ROW, "1"); + newExp.put(Columns.COLUMN, "1"); + newExp.put(traits.get(0).getObservationVariableName(), null); + + importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newExp), null), null, true, client, program, mappingId); + + BrAPITrial brAPITrial = brAPITrialDAO.getTrialsByName(List.of((String)newExp.get(Columns.EXP_TITLE)), program).get(0); + Optional trialIdXref = Utilities.getExternalReference(brAPITrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())); + assertTrue(trialIdXref.isPresent()); + BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceId()), program).get(0); + + BrAPIObservationUnit ou = ouDAO.getObservationUnitsForStudyDbId(brAPIStudy.getStudyDbId(), program).get(0); + Optional ouIdXref = Utilities.getExternalReference(ou.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName())); + assertTrue(ouIdXref.isPresent()); + + Map newObsVar = new HashMap<>(); + newObsVar.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); + newObsVar.put(traits.get(0).getObservationVariableName(), "newData"); + + JsonObject result = importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits), null, true, client, program, mappingId); + + JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); + assertEquals(1, previewRows.size()); + JsonObject row = previewRows.get(0).getAsJsonObject(); + + assertEquals("EXISTING", row.getAsJsonObject("trial").get("state").getAsString()); + assertTrue(row.getAsJsonObject("trial").get("brAPIObject").getAsJsonObject().get("additionalInfo").getAsJsonObject().has("observationDatasetId")); + assertTrue(importTestUtils.UUID_REGEX.matcher(row.getAsJsonObject("trial").get("brAPIObject").getAsJsonObject().get("additionalInfo").getAsJsonObject().get("observationDatasetId").getAsString()).matches()); + assertEquals("EXISTING", row.getAsJsonObject("location").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); + assertRowReferencedByOUIdSaved(newObsVar, program, traits); } @@ -1106,6 +1144,45 @@ public void importNewObsAfterFirstExpWithObs_blank(boolean commit) { } } + private Map assertRowReferencedByOUIdSaved(Map expected, Program program, List traits) throws ApiException { + Map ret = new HashMap<>(); + + List units = ouDAO.getObservationUnitsById(List.of((String)expected.get(Columns.OBS_UNIT_ID)), program); + assertFalse(units.isEmpty()); + + List observations = null; + if(traits != null) { + observations = observationDAO.getObservationsByObservationUnitsAndVariables( + units.stream().map(BrAPIObservationUnit::getObservationUnitDbId).collect(Collectors.toList()), + traits.stream().map(Trait::getObservationVariableDbId).collect(Collectors.toList()), + program + ); + if (expected.get(traits.get(0).getObservationVariableName()) == null) { + assertTrue(observations.isEmpty()); + } else { + assertFalse(observations.isEmpty()); + List expectedVariableObservation = new ArrayList<>(); + List actualVariableObservation = new ArrayList<>(); + observations.forEach(observation -> actualVariableObservation.add(String.format("%s:%s", Utilities.removeProgramKey(observation.getObservationVariableName(), program.getKey()), observation.getValue()))); + for(Trait trait : traits) { + if (expected.get(trait.getObservationVariableName()) != null) { + expectedVariableObservation.add(String.format("%s:%s", trait.getObservationVariableName(), expected.get(trait.getObservationVariableName()))); + } + } + if (actualVariableObservation.isEmpty()) { + assertTrue(expectedVariableObservation.isEmpty()); + } else { + assertThat("Missing Variable:Observation combo", actualVariableObservation, containsInAnyOrder(expectedVariableObservation.toArray())); + } + + } + + ret.put("observations", observations); + } + + return ret; + } + private Map assertRowSaved(Map expected, Program program, List traits) throws ApiException { Map ret = new HashMap<>(); From 97047658944dd447ed3b7be2801887156bf3e9f2 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 11 Mar 2024 13:44:32 -0400 Subject: [PATCH 20/40] [BI-2046] fix studyPIO reference when creating new observation PIO --- .../importer/services/processors/ExperimentProcessor.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index bf616273a..a47d42e9c 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -1162,7 +1162,6 @@ private void fetchOrCreateObservationPIO(Program program, getSingleEntryValue(trialByNameNoScope, MULTIPLE_EXP_TITLES) : trialByNameNoScope.get(importRow.getExpTitle()); UUID trialID = trialPIO.getId(); - //PendingImportObject studyPIO = this.studyByNameNoScope.get(importRow.getEnv()); UUID studyID = studyPIO.getId(); UUID id = UUID.randomUUID(); newObservation = importRow.constructBrAPIObservation(value, variableName, seasonDbId, obsUnitPIO.getBrAPIObject(), commit, program, user, BRAPI_REFERENCE_SOURCE, trialID, studyID, obsUnitPIO.getId(), id); @@ -1172,7 +1171,7 @@ private void fetchOrCreateObservationPIO(Program program, newObservation.setObservationTimeStamp(OffsetDateTime.parse(timeStampValue)); } - newObservation.setStudyDbId(this.studyByNameNoScope.get(pendingObsUnitByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getStudyName()).getId().toString()); //set as the BI ID to facilitate looking up studies when saving new observations + newObservation.setStudyDbId(studyPIO.getId().toString()); //set as the BI ID to facilitate looking up studies when saving new observations pio = new PendingImportObject<>(ImportObjectState.NEW, newObservation); this.observationByHash.put(key, pio); From 885ef5325316ade47c1ed08e5058a28aaf709bf1 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Fri, 15 Mar 2024 14:27:58 -0400 Subject: [PATCH 21/40] [BI-2046] correct observation hash when OU id referenced --- .../importer/services/processors/ExperimentProcessor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index a47d42e9c..6e53b37ef 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -480,6 +480,7 @@ private void prepareDataForValidation(List importRows, List column : phenotypeCols) { observationHash = getObservationHash( + pendingStudyByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getStudyName() + pendingObsUnitByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getObservationUnitName(), getVariableNameFromColumn(column), pendingStudyByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getStudyName() @@ -1130,7 +1131,7 @@ private void fetchOrCreateObservationPIO(Program program, if (hasAllReferenceUnitIds) { String unitName = obsUnitPIO.getBrAPIObject().getObservationUnitName(); String studyName = studyPIO.getBrAPIObject().getStudyName(); - key = getObservationHash(unitName, variableName, studyName); + key = getObservationHash(studyName + unitName, variableName, studyName); } else { key = getImportObservationHash(importRow, variableName); } From a39c505ae100f4299aa0bbe2f6793a69e434864a Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 27 Mar 2024 11:11:19 -0400 Subject: [PATCH 22/40] [BI-2046] set preview state for existing empty datasets --- .../processors/ExperimentProcessor.java | 25 ++++++---- .../importer/ExperimentFileImportTest.java | 47 ++++++++++++++++--- 2 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 6e53b37ef..5230f3819 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -233,7 +233,7 @@ public void getExistingBrapiData(List importRows, Program program) } else { - // can't proceed if you have a mix of ObsUnitId for some but not all rows + // can't proceed if the import has a mix of ObsUnitId for some but not all rows throw new HttpStatusException(HttpStatus.UNPROCESSABLE_ENTITY, MISSING_OBS_UNIT_ID_ERROR); } } @@ -666,6 +666,10 @@ private String createObservationUnitKey(String studyName, String obsUnitName) { return studyName + obsUnitName; } + private String getImportObservationHash(String obsUnitName, String variableName, String studyName) { + return getObservationHash(createObservationUnitKey(studyName, obsUnitName), variableName, studyName); + } + private String getImportObservationHash(ExperimentObservation importRow, String variableName) { return getObservationHash(createObservationUnitKey(importRow), variableName, importRow.getEnv()); } @@ -773,7 +777,7 @@ private void validateObservations(ValidationErrors validationErrors, String importObsValue = phenoCol.getString(rowNum); if (hasAllReferenceUnitIds) { - importHash = getObservationHash( + importHash = getImportObservationHash( pendingObsUnitByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getObservationUnitName(), getVariableNameFromColumn(phenoCol), pendingStudyByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getStudyName() @@ -832,14 +836,17 @@ private void validateObservations(ValidationErrors validationErrors, pendingObservation.getAdditionalInfo().get(BrAPIAdditionalInfoFields.CHANGELOG).getAsJsonArray().add(gson.toJsonTree(change).getAsJsonObject()); } - // preview case where observation has already been committed and import ObsVar data is either empty or the - // same as has been committed prior to import - } else if(existingObsByObsHash.containsKey(importHash) && (StringUtils.isBlank(phenoCol.getString(rowNum)) || - isObservationMatched(importHash, importObsValue, phenoCol, rowNum))) { + // preview case where observation has already been committed and import ObsVar data is the + // same as has been committed prior to import + } else if(isObservationMatched(importHash, importObsValue, phenoCol, rowNum)) { BrAPIObservation existingObs = this.existingObsByObsHash.get(importHash); existingObs.setObservationVariableName(phenoCol.name()); observationByHash.get(importHash).setState(ImportObjectState.EXISTING); observationByHash.get(importHash).setBrAPIObject(existingObs); + + // preview case where observation has already been committed and import ObsVar data is empty prior to import + } else if(!existingObsByObsHash.containsKey(importHash) && (StringUtils.isBlank(phenoCol.getString(rowNum)))) { + observationByHash.get(importHash).setState(ImportObjectState.EXISTING); } else { validateObservationValue(colVarMap.get(phenoCol.name()), phenoCol.getString(rowNum), phenoCol.name(), validationErrors, rowNum); @@ -1097,7 +1104,7 @@ boolean isTimestampMatched(String observationHash, String timeStamp) { } boolean isValueMatched(String observationHash, String value) { - if (existingObsByObsHash.get(observationHash).getValue() == null) { + if (!existingObsByObsHash.containsKey(observationHash) || existingObsByObsHash.get(observationHash).getValue() == null) { return value == null; } return existingObsByObsHash.get(observationHash).getValue().equals(value); @@ -1994,7 +2001,7 @@ private Map> initializeGermplasmByGI existingGermplasms.forEach(existingGermplasm -> { BrAPIExternalReference xref = Utilities.getExternalReference(existingGermplasm.getExternalReferences(), String.format("%s", BRAPI_REFERENCE_SOURCE)) .orElseThrow(() -> new IllegalStateException("External references wasn't found for germplasm (dbid): " + existingGermplasm.getGermplasmDbId())); - existingGermplasmByGID.put(existingGermplasm.getAccessionNumber(), new PendingImportObject<>(ImportObjectState.EXISTING, existingGermplasm, UUID.fromString(xref.getReferenceID()))); + existingGermplasmByGID.put(existingGermplasm.getAccessionNumber(), new PendingImportObject<>(ImportObjectState.EXISTING, existingGermplasm, UUID.fromString(xref.getReferenceId()))); }); return existingGermplasmByGID; } @@ -2030,7 +2037,7 @@ private Map> initializeExistingGermp existingGermplasms.forEach(existingGermplasm -> { BrAPIExternalReference xref = Utilities.getExternalReference(existingGermplasm.getExternalReferences(), String.format("%s", BRAPI_REFERENCE_SOURCE)) .orElseThrow(() -> new IllegalStateException("External references wasn't found for germplasm (dbid): " + existingGermplasm.getGermplasmDbId())); - existingGermplasmByGID.put(existingGermplasm.getAccessionNumber(), new PendingImportObject<>(ImportObjectState.EXISTING, existingGermplasm, UUID.fromString(xref.getReferenceID()))); + existingGermplasmByGID.put(existingGermplasm.getAccessionNumber(), new PendingImportObject<>(ImportObjectState.EXISTING, existingGermplasm, UUID.fromString(xref.getReferenceId()))); }); return existingGermplasmByGID; } diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 3ff0271f7..33591b431 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -757,7 +757,7 @@ public void importNewObsVarByObsUnitId() { @SneakyThrows public void importNewObservationDataByObsUnitId() { List traits = importTestUtils.createTraits(1); - Program program = createProgram("New Observation Referring to OU by ID", "OUDAT", "DATAOU", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Program program = createProgram("New Observation Referring to OU by ID", "OUDAT", "DATOU", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); Map newExp = new HashMap<>(); newExp.put(Columns.GERMPLASM_GID, "1"); newExp.put(Columns.TEST_CHECK, "T"); @@ -787,7 +787,7 @@ public void importNewObservationDataByObsUnitId() { Map newObsVar = new HashMap<>(); newObsVar.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); - newObsVar.put(traits.get(0).getObservationVariableName(), "newData"); + newObsVar.put(traits.get(0).getObservationVariableName(), "1"); JsonObject result = importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits), null, true, client, program, mappingId); @@ -921,7 +921,24 @@ public void verifyFailureImportNewObsExistingOuWithExistingObs(boolean commit) { newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); newObservation.put(traits.get(0).getObservationVariableName(), "2"); - uploadAndVerifyFailure(program, importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits), traits.get(0).getObservationVariableName(), commit); + JsonObject result = importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits), null, true, client, program, mappingId); + + JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); + assertEquals(1, previewRows.size()); + JsonObject row = previewRows.get(0).getAsJsonObject(); + + assertEquals("EXISTING", row.getAsJsonObject("trial").get("state").getAsString()); + assertTrue(row.getAsJsonObject("trial").get("brAPIObject").getAsJsonObject().get("additionalInfo").getAsJsonObject().has("observationDatasetId")); + assertTrue(importTestUtils.UUID_REGEX.matcher(row.getAsJsonObject("trial").get("brAPIObject").getAsJsonObject().get("additionalInfo").getAsJsonObject().get("observationDatasetId").getAsString()).matches()); + assertEquals("EXISTING", row.getAsJsonObject("location").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); + + if(commit) { + assertRowSaved(newObservation, program, traits); + } else { + assertValidPreviewRow(newObservation, row, program, traits); + } } /* @@ -1136,7 +1153,7 @@ public void importNewObsAfterFirstExpWithObs_blank(boolean commit) { assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); - newObservation.put(traits.get(0).getObservationVariableName(), "1"); + //newObservation.put(traits.get(0).getObservationVariableName(), "1"); if(commit) { assertRowSaved(newObservation, program, traits); } else { @@ -1152,6 +1169,7 @@ private Map assertRowReferencedByOUIdSaved(Map e List observations = null; if(traits != null) { + //observations = observationDAO.getObservationsByStudyName(List.of(units.get(0).getStudyName()), program); observations = observationDAO.getObservationsByObservationUnitsAndVariables( units.stream().map(BrAPIObservationUnit::getObservationUnitDbId).collect(Collectors.toList()), traits.stream().map(Trait::getObservationVariableDbId).collect(Collectors.toList()), @@ -1160,7 +1178,7 @@ private Map assertRowReferencedByOUIdSaved(Map e if (expected.get(traits.get(0).getObservationVariableName()) == null) { assertTrue(observations.isEmpty()); } else { - assertFalse(observations.isEmpty()); + //assertFalse(observations.isEmpty()); List expectedVariableObservation = new ArrayList<>(); List actualVariableObservation = new ArrayList<>(); observations.forEach(observation -> actualVariableObservation.add(String.format("%s:%s", Utilities.removeProgramKey(observation.getObservationVariableName(), program.getKey()), observation.getValue()))); @@ -1286,7 +1304,24 @@ private Map assertRowSaved(Map expected, Program if(traits != null) { List expectedVariableObservation = new ArrayList<>(); List actualVariableObservation = new ArrayList<>(); - observations.forEach(observation -> actualVariableObservation.add(String.format("%s:%s", Utilities.removeProgramKey(observation.getObservationVariableName(), program.getKey()), observation.getValue()))); + observations.forEach(observation -> actualVariableObservation.add( + String.format( + "%s:%s", + Utilities.removeProgramKey(observation.getObservationVariableName(), program.getKey()), + observation.getValue() + ) + )); + +// //Remove observations duplicated when re-imported with no change +// Set uniqueObservations = new HashSet<>(); +// Iterator iterator = actualVariableObservation.iterator(); +// +// while (iterator.hasNext()) { +// String item = iterator.next(); +// if (!uniqueObservations.add(item)) { +// iterator.remove(); // Remove the duplicate actual observation +// } +// } for(Trait trait : traits) { if (expected.get(trait.getObservationVariableName()) != null) { expectedVariableObservation.add(String.format("%s:%s", trait.getObservationVariableName(), expected.get(trait.getObservationVariableName()))); From 3e04828858faf59713e25fe62fe123fa4ba7571a Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 27 Mar 2024 18:53:35 -0400 Subject: [PATCH 23/40] fix test --- .../processors/ExperimentProcessor.java | 2 +- .../importer/ExperimentFileImportTest.java | 72 +++++++++---------- 2 files changed, 33 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 5230f3819..c9d861906 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -1144,7 +1144,7 @@ private void fetchOrCreateObservationPIO(Program program, } if (existingObsByObsHash.containsKey(key)) { - if (StringUtils.isNotBlank(value) && !isObservationMatched(key, value, column, rowNum)){ + if (!isObservationMatched(key, value, column, rowNum)){ // prior observation with updated value newObservation = gson.fromJson(gson.toJson(existingObsByObsHash.get(key)), BrAPIObservation.class); diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 33591b431..2e28f5efc 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -753,11 +753,12 @@ public void importNewObsVarByObsUnitId() { assertRowReferencedByOUIdSaved(newObsVar, program, traits); } - @Test + @ParameterizedTest + @ValueSource(booleans = {true, false}) @SneakyThrows - public void importNewObservationDataByObsUnitId() { + public void importNewObservationDataByObsUnitId(boolean commit) { List traits = importTestUtils.createTraits(1); - Program program = createProgram("New Observation Referring to OU by ID", "OUDAT", "DATOU", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Program program = createProgram("New Observation Referring to OU by ID"+(commit ? "C" : "P"), "OUDAT"+(commit ? "C" : "P"), "DATOU"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); Map newExp = new HashMap<>(); newExp.put(Columns.GERMPLASM_GID, "1"); newExp.put(Columns.TEST_CHECK, "T"); @@ -772,7 +773,7 @@ public void importNewObservationDataByObsUnitId() { newExp.put(Columns.BLOCK_NUM, "1"); newExp.put(Columns.ROW, "1"); newExp.put(Columns.COLUMN, "1"); - newExp.put(traits.get(0).getObservationVariableName(), null); + newExp.put(traits.get(0).getObservationVariableName(), null); // empty dataset importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newExp), null), null, true, client, program, mappingId); @@ -786,10 +787,23 @@ public void importNewObservationDataByObsUnitId() { assertTrue(ouIdXref.isPresent()); Map newObsVar = new HashMap<>(); + newObsVar.put(Columns.GERMPLASM_GID, "1"); + newObsVar.put(Columns.TEST_CHECK, "T"); + newObsVar.put(Columns.EXP_TITLE, "Test Exp"); + newObsVar.put(Columns.EXP_UNIT, "Plot"); + newObsVar.put(Columns.EXP_TYPE, "Phenotyping"); + newObsVar.put(Columns.ENV, "New Env"); + newObsVar.put(Columns.ENV_LOCATION, "Location A"); + newObsVar.put(Columns.ENV_YEAR, "2023"); + newObsVar.put(Columns.EXP_UNIT_ID, "a-1"); + newObsVar.put(Columns.REP_NUM, "1"); + newObsVar.put(Columns.BLOCK_NUM, "1"); + newObsVar.put(Columns.ROW, "1"); + newObsVar.put(Columns.COLUMN, "1"); newObsVar.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); newObsVar.put(traits.get(0).getObservationVariableName(), "1"); - JsonObject result = importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits), null, true, client, program, mappingId); + JsonObject result = importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits), null, commit, client, program, mappingId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -801,7 +815,12 @@ public void importNewObservationDataByObsUnitId() { assertEquals("EXISTING", row.getAsJsonObject("location").get("state").getAsString()); assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); - assertRowReferencedByOUIdSaved(newObsVar, program, traits); + if(commit) { + //assertRowReferencedByOUIdSaved(newObsVar, program, traits); + assertRowSaved(newObsVar, program, traits); + } else { + assertValidPreviewRow(newObsVar, row, program, traits); + } } @@ -921,24 +940,7 @@ public void verifyFailureImportNewObsExistingOuWithExistingObs(boolean commit) { newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceId()); newObservation.put(traits.get(0).getObservationVariableName(), "2"); - JsonObject result = importTestUtils.uploadAndFetch(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits), null, true, client, program, mappingId); - - JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); - assertEquals(1, previewRows.size()); - JsonObject row = previewRows.get(0).getAsJsonObject(); - - assertEquals("EXISTING", row.getAsJsonObject("trial").get("state").getAsString()); - assertTrue(row.getAsJsonObject("trial").get("brAPIObject").getAsJsonObject().get("additionalInfo").getAsJsonObject().has("observationDatasetId")); - assertTrue(importTestUtils.UUID_REGEX.matcher(row.getAsJsonObject("trial").get("brAPIObject").getAsJsonObject().get("additionalInfo").getAsJsonObject().get("observationDatasetId").getAsString()).matches()); - assertEquals("EXISTING", row.getAsJsonObject("location").get("state").getAsString()); - assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); - assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); - - if(commit) { - assertRowSaved(newObservation, program, traits); - } else { - assertValidPreviewRow(newObservation, row, program, traits); - } + uploadAndVerifyFailure(program, importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits), traits.get(0).getObservationVariableName(), commit); } /* @@ -1169,12 +1171,12 @@ private Map assertRowReferencedByOUIdSaved(Map e List observations = null; if(traits != null) { - //observations = observationDAO.getObservationsByStudyName(List.of(units.get(0).getStudyName()), program); - observations = observationDAO.getObservationsByObservationUnitsAndVariables( - units.stream().map(BrAPIObservationUnit::getObservationUnitDbId).collect(Collectors.toList()), - traits.stream().map(Trait::getObservationVariableDbId).collect(Collectors.toList()), - program - ); + observations = observationDAO.getObservationsByStudyName(List.of(units.get(0).getStudyName()), program); +// observations = observationDAO.getObservationsByObservationUnitsAndVariables( +// units.stream().map(BrAPIObservationUnit::getObservationUnitDbId).collect(Collectors.toList()), +// traits.stream().map(Trait::getObservationVariableDbId).collect(Collectors.toList()), +// program +// ); if (expected.get(traits.get(0).getObservationVariableName()) == null) { assertTrue(observations.isEmpty()); } else { @@ -1312,16 +1314,6 @@ private Map assertRowSaved(Map expected, Program ) )); -// //Remove observations duplicated when re-imported with no change -// Set uniqueObservations = new HashSet<>(); -// Iterator iterator = actualVariableObservation.iterator(); -// -// while (iterator.hasNext()) { -// String item = iterator.next(); -// if (!uniqueObservations.add(item)) { -// iterator.remove(); // Remove the duplicate actual observation -// } -// } for(Trait trait : traits) { if (expected.get(trait.getObservationVariableName()) != null) { expectedVariableObservation.add(String.format("%s:%s", trait.getObservationVariableName(), expected.get(trait.getObservationVariableName()))); From 17b08e36dffa002c2a6b115346d71b48dda556cd Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 10:32:32 -0400 Subject: [PATCH 24/40] [BI-2046] remove unused helper method --- .../brapps/importer/services/FileMappingUtil.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/FileMappingUtil.java b/src/main/java/org/breedinginsight/brapps/importer/services/FileMappingUtil.java index a3e96164a..cb93daf48 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/FileMappingUtil.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/FileMappingUtil.java @@ -91,13 +91,4 @@ public List sortByField(List sortedFields, List unsortedItems, return unsortedItems; } - - public boolean isValidUUID(String id) { - try { - UUID.fromString(id); - return true; - } catch (IllegalArgumentException e) { - return false; - } - } } From 55a0f788fc244952a7d920bda82ff8b18e220d61 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 12:45:30 -0400 Subject: [PATCH 25/40] [BI-2046] refactor --- .../processors/ExperimentProcessor.java | 31 ++++++++++++------- .../importer/ExperimentFileImportTest.java | 9 +----- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index c9d861906..7a5ef92ce 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -35,7 +35,6 @@ import org.brapi.v2.model.core.*; import org.brapi.v2.model.core.request.BrAPIListNewRequest; import org.brapi.v2.model.core.response.BrAPIListDetails; -import org.brapi.v2.model.geno.BrAPIReference; import org.brapi.v2.model.germ.BrAPIGermplasm; import org.brapi.v2.model.pheno.BrAPIObservation; import org.brapi.v2.model.pheno.BrAPIObservationUnit; @@ -47,8 +46,6 @@ import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; import org.breedinginsight.brapi.v2.dao.*; import org.breedinginsight.brapps.importer.model.ImportUpload; -import org.breedinginsight.brapps.importer.model.base.ObservationUnit; -import org.breedinginsight.brapps.importer.model.base.Study; import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; import org.breedinginsight.brapps.importer.model.imports.ChangeLogEntry; import org.breedinginsight.brapps.importer.model.imports.PendingImport; @@ -471,19 +468,20 @@ private void prepareDataForValidation(List importRows, List> observations = mappedImportRow.getObservations(); String observationHash; if (hasAllReferenceUnitIds) { - mappedImportRow.setTrial(pendingTrialByOUId.get(importRow.getObsUnitID())); - mappedImportRow.setLocation(pendingLocationByOUId.get(importRow.getObsUnitID())); - mappedImportRow.setStudy(pendingStudyByOUId.get(importRow.getObsUnitID())); - mappedImportRow.setObservationUnit(pendingObsUnitByOUId.get(importRow.getObsUnitID())); - mappedImportRow.setGermplasm(pendingGermplasmByOUId.get(importRow.getObsUnitID())); + String refOUId = importRow.getObsUnitID(); + mappedImportRow.setTrial(pendingTrialByOUId.get(refOUId)); + mappedImportRow.setLocation(pendingLocationByOUId.get(refOUId)); + mappedImportRow.setStudy(pendingStudyByOUId.get(refOUId)); + mappedImportRow.setObservationUnit(pendingObsUnitByOUId.get(refOUId)); + mappedImportRow.setGermplasm(pendingGermplasmByOUId.get(refOUId)); // loop over phenotype column observation data for current row for (Column column : phenotypeCols) { observationHash = getObservationHash( - pendingStudyByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getStudyName() + - pendingObsUnitByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getObservationUnitName(), + pendingStudyByOUId.get(refOUId).getBrAPIObject().getStudyName() + + pendingObsUnitByOUId.get(refOUId).getBrAPIObject().getObservationUnitName(), getVariableNameFromColumn(column), - pendingStudyByOUId.get(importRow.getObsUnitID()).getBrAPIObject().getStudyName() + pendingStudyByOUId.get(refOUId).getBrAPIObject().getStudyName() ); // if value was blank won't be entry in map for this observation @@ -666,6 +664,17 @@ private String createObservationUnitKey(String studyName, String obsUnitName) { return studyName + obsUnitName; } + /** + * This method is responsible for generating a hash based on the import observation unit information. + * It takes the observation unit name, variable name, and study name as input parameters. + * The observation unit key is created using the study name and observation unit name. + * The hash is generated based on the observation unit key, variable name, and study name. + * + * @param obsUnitName The name of the observation unit being imported. + * @param variableName The name of the variable associated with the observation unit. + * @param studyName The name of the study associated with the observation unit. + * @return A string representing the hash of the import observation unit information. + */ private String getImportObservationHash(String obsUnitName, String variableName, String studyName) { return getObservationHash(createObservationUnitKey(studyName, obsUnitName), variableName, studyName); } diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 2e28f5efc..9420f18f0 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -816,7 +816,6 @@ public void importNewObservationDataByObsUnitId(boolean commit) { assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); if(commit) { - //assertRowReferencedByOUIdSaved(newObsVar, program, traits); assertRowSaved(newObsVar, program, traits); } else { assertValidPreviewRow(newObsVar, row, program, traits); @@ -1155,7 +1154,6 @@ public void importNewObsAfterFirstExpWithObs_blank(boolean commit) { assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); - //newObservation.put(traits.get(0).getObservationVariableName(), "1"); if(commit) { assertRowSaved(newObservation, program, traits); } else { @@ -1172,15 +1170,10 @@ private Map assertRowReferencedByOUIdSaved(Map e List observations = null; if(traits != null) { observations = observationDAO.getObservationsByStudyName(List.of(units.get(0).getStudyName()), program); -// observations = observationDAO.getObservationsByObservationUnitsAndVariables( -// units.stream().map(BrAPIObservationUnit::getObservationUnitDbId).collect(Collectors.toList()), -// traits.stream().map(Trait::getObservationVariableDbId).collect(Collectors.toList()), -// program -// ); if (expected.get(traits.get(0).getObservationVariableName()) == null) { assertTrue(observations.isEmpty()); } else { - //assertFalse(observations.isEmpty()); + assertFalse(observations.isEmpty()); List expectedVariableObservation = new ArrayList<>(); List actualVariableObservation = new ArrayList<>(); observations.forEach(observation -> actualVariableObservation.add(String.format("%s:%s", Utilities.removeProgramKey(observation.getObservationVariableName(), program.getKey()), observation.getValue()))); From cc68f285ef4433d7b554f85fad934122df1d5a8c Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:10:50 -0400 Subject: [PATCH 26/40] [BI-2046] document exp processhelper method --- .../processors/ExperimentProcessor.java | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 7a5ef92ce..3ed5a0d01 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -882,8 +882,7 @@ private void validateUniqueObsUnits( ValidationErrors validationErrors, Set uniqueStudyAndObsUnit, int rowNum, - ExperimentObservation importRow - ) { + ExperimentObservation importRow) { String envIdPlusStudyId = createObservationUnitKey(importRow); if (uniqueStudyAndObsUnit.contains(envIdPlusStudyId)) { String errorMessage = String.format("The ID (%s) is not unique within the environment(%s)", importRow.getExpUnitId(), importRow.getEnv()); @@ -1568,6 +1567,24 @@ private Map> initializeObserva } } + /** + * Maps pending locations by Observation Unit (OU) Id based on given parameters. + * + * This method takes in a unitId, BrAPIObservationUnit unit, Maps of studyByOUId, locationByName, + * and locationByOUId. It then associates the location of the observation unit with the respective OU Id. + * If the locationName is not null for the unit, it is directly added to locationByOUId. + * If the locationName is null, it checks the studyByOUId map for a location related to the unit. + * If a location related to the unit is found, it maps that location with the respective OU Id. + * If no location is found, it throws an IllegalStateException. + * + * @param unitId the Observation Unit Id + * @param unit the BrAPIObservationUnit object + * @param studyByOUId a Map of Study by Observation Unit Id + * @param locationByName a Map of Location by Name + * @param locationByOUId a Map of Location by Observation Unit Id + * @return the updated locationByOUId map after mapping the pending locations + * @throws IllegalStateException if the Observation Unit is missing a location + */ private Map> mapPendingLocationByOUId( String unitId, BrAPIObservationUnit unit, From 068f046ab61c75186b6e0700891c41376e81af07 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:16:17 -0400 Subject: [PATCH 27/40] [BI-2046] document exp process helper method --- .../services/processors/ExperimentProcessor.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 3ed5a0d01..658241ddb 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -1666,14 +1666,13 @@ private Map> mapPendingObserva } /** - * Retrieves a list of pending Observation Units based on their IDs. + * Retrieves reference Observation Units based on a set of reference Observation Unit IDs and a Program. + * Constructs DeltaBreed observation unit source for external references and sets up pending Observation Units. * - * This function retrieves Observation Units based on a list of reference Observation Unit IDs - * and the associated program. It then sets pending Observation Units for each ID. - * - * @return List of BrAPIObservationUnit: The list of reference Observation Units retrieved. - * - * @throws InternalServerException if an error occurs during the process. + * @param referenceOUIds A set of reference Observation Unit IDs to retrieve + * @param program The Program associated with the Observation Units + * @return A Map containing pending Observation Units by their ID + * @throws ApiException if an error occurs during the process */ private Map> fetchReferenceObservationUnits( Set referenceOUIds, From e39a02def0143a5fe67cb9f9e36047f06d4a48db Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:20:11 -0400 Subject: [PATCH 28/40] [BI-2046] document exp process helper method --- .../processors/ExperimentProcessor.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 658241ddb..46f9bb95b 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -1800,17 +1800,16 @@ private void initializeStudiesForExistingObservationUnits( } /** - * Fetches a list of BrAPI studies by studyDbIds belonging to a specific - * program. - * If not all studyDbIds are found in the database, it throws an - * IllegalStateException. + * Fetches a list of BrAPI studies by their study database IDs for a given program. * - * @param studyDbIds A set of studyDbIds to fetch studies for - * @param program The program for which the studies are associated - * @return A list of BrAPIStudy objects representing the fetched studies - * @throws ApiException if an error occurs during the database fetch - * operation - * @throws IllegalStateException if not all studyDbIds are found in the database + * This method queries the BrAPIStudyDAO to retrieve studies based on the provided study database IDs and the program. + * It ensures that all requested study database IDs are found in the result set, throwing an IllegalStateException if any are missing. + * + * @param studyDbIds a Set of Strings representing the study database IDs to fetch + * @param program the Program object representing the program context in which to fetch studies + * @return a List of BrAPIStudy objects matching the provided study database IDs + * @throws ApiException if there is an issue fetching the studies + * @throws IllegalStateException if any requested study database IDs are not found in the result set */ private List fetchStudiesByDbId(Set studyDbIds, Program program) throws ApiException { List studies = brAPIStudyDAO.getStudiesByStudyDbId(studyDbIds, program); From a42b98f3de226ad10f539965eaacae17eb752592 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:23:28 -0400 Subject: [PATCH 29/40] [BI-2046] document exp process helper method --- .../services/processors/ExperimentProcessor.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 46f9bb95b..f283150a0 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -1822,10 +1822,21 @@ private List fetchStudiesByDbId(Set studyDbIds, Program prog return studies; } + /** + * Initializes a map of ProgramLocation objects by their names using the given Program and a map of BrAPIStudy objects by their names. + * + * This method takes a Program object and a map of BrAPIStudy objects by their names, retrieves the location database IDs from the studies, + * and fetches existing ProgramLocation objects based on the database IDs. It then creates a map of ProgramLocation objects by their names + * with PendingImportObject wrappers that indicate the state of the object as existing. + * + * @param program the Program object to associate with the locations + * @param studyByName a map of BrAPIStudy objects by their names + * @return a map of ProgramLocation objects by their names with PendingImportObject wrappers + * @throws InternalServerException if an error occurs during the location retrieval process + */ Map> initializeLocationByName( Program program, - Map> studyByName - ) { + Map> studyByName) { Map> locationByName = new HashMap<>(); List existingLocations = new ArrayList<>(); From b3f819cfcd092985a7c546bc52454e0c67da92cd Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:29:28 -0400 Subject: [PATCH 30/40] [BI-2046] document exp process helper method --- .../processors/ExperimentProcessor.java | 49 ++++++++++++++++--- 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index f283150a0..3137fa371 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -209,7 +209,7 @@ public void getExistingBrapiData(List importRows, Program program) mapPendingTrialByOUId(unitId, unit, trialByNameNoScope, studyByNameNoScope, pendingTrialByOUId, program); mapPendingStudyByOUId(unitId, unit, studyByNameNoScope, pendingStudyByOUId, program); mapPendingLocationByOUId(unitId, unit, pendingStudyByOUId, locationByName, pendingLocationByOUId); - mapPendingObsDatasetByOUId(unitId, pendingTrialByOUId, obsVarDatasetByName, pendingObsDatasetByOUId, program); + mapPendingObsDatasetByOUId(unitId, pendingTrialByOUId, obsVarDatasetByName, pendingObsDatasetByOUId); mapGermplasmByOUId(unitId, unit, existingGermplasmByGID, pendingGermplasmByOUId); } @@ -1896,24 +1896,49 @@ private Map> initializeUniqueLocati return locationByName; } + /** + * Maps a given germplasm to an observation unit ID in a given map of germplasm by observation unit ID. + * + * This method retrieves the Global Identifier (GID) of the provided observation unit and uses it to lookup + * the corresponding PendingImportObject in the map of germplasm by name. The found germplasm + * object is then mapped to the observation unit ID in the provided map of germplasm by observation unit ID. + * The updated map is returned after the mapping operation has been performed. + * + * @param unitId The observation unit ID to which the germplasm should be mapped. + * @param unit The BrAPIObservationUnit object representing the observation unit. + * @param germplasmByName The map of germplasm objects by name used to lookup the desired germplasm. + * @param germplasmByOUId The map of germplasm objects by observation unit ID to update with the mapping result. + * @return The updated map of germplasm objects by observation unit ID after mapping the germplasm to the provided observation unit ID. + */ private Map> mapGermplasmByOUId( String unitId, BrAPIObservationUnit unit, Map> germplasmByName, - Map> germplasmByOUId - ) { + Map> germplasmByOUId) { String gid = unit.getAdditionalInfo().getAsJsonObject().get(BrAPIAdditionalInfoFields.GID).getAsString(); germplasmByOUId.put(unitId, germplasmByName.get(gid)); return germplasmByOUId; } + + /** + * Maps the pending observation dataset by OU Id based on the given inputs. + * This function checks if the trialByOUId map is not empty, the obsVarDatasetByName map is not empty, + * and if the first entry in the trialByOUId map contains observation dataset id in its additional info. + * If the conditions are met, it adds the pending import object from the obsVarDatasetByName map to the + * obsVarDatasetByOUId map using the unitId as the key. + * + * @param unitId the unit ID based on which the mapping is done + * @param trialByOUId a map containing pending import objects with BrAPITrial as the value, mapped by OU Id + * @param obsVarDatasetByName a map containing pending import objects with BrAPIListDetails as the value, mapped by dataset name + * @param obsVarDatasetByOUId a map containing pending import objects with BrAPIListDetails as the value, mapped by OU Id + * @return the updated obsVarDatasetByOUId map after potential addition of a pending import object + */ private Map> mapPendingObsDatasetByOUId( String unitId, Map> trialByOUId, Map> obsVarDatasetByName, - Map> obsVarDatasetByOUId, - Program program - ) { + Map> obsVarDatasetByOUId) { if (!trialByOUId.isEmpty() && !obsVarDatasetByName.isEmpty() && trialByOUId.values().iterator().next().getBrAPIObject().getAdditionalInfo().has(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID)) { obsVarDatasetByOUId.put(unitId, obsVarDatasetByName.values().iterator().next()); @@ -1921,10 +1946,18 @@ private Map> mapPendingObsDatasetB return obsVarDatasetByOUId; } + + /** + * Initializes observation variable dataset for existing observation units. This function retrieves existing datasets related to observation variables for the specified trial and program, processes the dataset details, and caches the data accordingly. + * + * @param trialByName A map containing trial information indexed by trial name. + * @param program The program to which the datasets are related. + * @return A map of observation variable dataset objects indexed by dataset name. + * @throws InternalServerException If the existing dataset summary is not retrieved from the BrAPI server, or an error occurs during API communication. + */ private Map> initializeObsVarDatasetForExistingObservationUnits( Map> trialByName, - Program program - ) { + Program program) { Map> obsVarDatasetByName = new HashMap<>(); if (trialByName.size() > 0 && From 2e6161f06c1d7ae3d06608d9957e2ea34d271751 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:31:22 -0400 Subject: [PATCH 31/40] [BI-2046] delete unused helper method --- .../services/processors/ExperimentProcessor.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 3137fa371..32a6f9b78 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -2034,14 +2034,7 @@ private Optional> getTrialPIO(List> obsVarDatasetByOUId) { - BrAPIExternalReference xref = Utilities.getExternalReference(existingList.getExternalReferences(), - String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName())) - .orElseThrow(() -> new IllegalStateException("External references wasn't found for list (dbid): " + existingList.getListDbId())); - obsVarDatasetByOUId.put(unitId, - new PendingImportObject(ImportObjectState.EXISTING, existingList, UUID.fromString(xref.getReferenceId()))); - } + private void processAndCacheObsVarDataset(BrAPIListDetails existingList, Map> obsVarDatasetByName) { BrAPIExternalReference xref = Utilities.getExternalReference(existingList.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName())) From a3f16c2fd923a4372349de33fb4b5411e9582233 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:35:10 -0400 Subject: [PATCH 32/40] [BI-2046] document exp process helper method --- .../processors/ExperimentProcessor.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 32a6f9b78..16529c1ca 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -2034,7 +2034,7 @@ private Optional> getTrialPIO(List> obsVarDatasetByName) { BrAPIExternalReference xref = Utilities.getExternalReference(existingList.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName())) @@ -2042,10 +2042,21 @@ private void processAndCacheObsVarDataset(BrAPIListDetails existingList, Map(ImportObjectState.EXISTING, existingList, UUID.fromString(xref.getReferenceId()))); } + + /** + * Initializes a mapping of BrAPI Germplasm objects by Germplasm ID for existing BrAPI Observation Units. + * This method retrieves existing Germplasms associated with the provided Observation Units and creates a mapping + * using their Accession Number as the key and a PendingImportObject containing the Germplasm object and a reference ID. + * If no existing Germplasms are found, an empty mapping is returned. + * + * @param unitByName A mapping of Observation Units by name. + * @param program The BrAPI Program object to which the Germplasms belong. + * @return A mapping of BrAPI Germplasm objects by Germplasm ID for existing Observation Units. + * @throws InternalServerException If an error occurs while fetching Germplasms from the database. + */ private Map> initializeGermplasmByGIDForExistingObservationUnits( Map> unitByName, - Program program - ) { + Program program) { Map> existingGermplasmByGID = new HashMap<>(); List existingGermplasms = new ArrayList<>(); From aab081e193638fc00a9b69881286000c473a7f94 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:39:05 -0400 Subject: [PATCH 33/40] [BI-2046] delete unused helper method --- .../processors/ExperimentProcessor.java | 49 +------------------ 1 file changed, 1 insertion(+), 48 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 16529c1ca..6239e4325 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -2130,8 +2130,7 @@ private PendingImportObject processAndCacheStudy( BrAPIStudy existingStudy, Program program, Function getterFunction, - Map> studyMap - ) throws Exception { + Map> studyMap) throws Exception { PendingImportObject pendingStudy; BrAPIExternalReference xref = Utilities.getExternalReference(existingStudy.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.STUDIES.getName())) .orElseThrow(() -> new IllegalStateException("External references wasn't found for study (dbid): " + existingStudy.getStudyDbId())); @@ -2153,52 +2152,6 @@ private PendingImportObject processAndCacheStudy( return pendingStudy; } - private void setPendingTrialByOUId(Program program) { - if(observationUnitByNameNoScope.size() > 0) { - Set trialDbIds = new HashSet<>(); - Set studyDbIds = new HashSet<>(); - - observationUnitByNameNoScope.values() - .forEach(pio -> { - BrAPIObservationUnit existingOu = pio.getBrAPIObject(); - if (StringUtils.isBlank(existingOu.getTrialDbId()) && StringUtils.isBlank(existingOu.getStudyDbId())) { - throw new IllegalStateException("TrialDbId and StudyDbId are not set for an existing ObservationUnit"); - } - - if (StringUtils.isNotBlank(existingOu.getTrialDbId())) { - trialDbIds.add(existingOu.getTrialDbId()); - } else { - studyDbIds.add(existingOu.getStudyDbId()); - } - }); - - //if the OU doesn't have the trialDbId set, then fetch the study to fetch the trialDbId - if(!studyDbIds.isEmpty()) { - try { - trialDbIds.addAll(fetchTrialDbidsForStudies(studyDbIds, program)); - } catch (ApiException e) { - log.error("Error fetching studies: " + Utilities.generateApiExceptionLogMessage(e), e); - throw new InternalServerException(e.toString(), e); - } - } - - try { - List trials = brapiTrialDAO.getTrialsByDbIds(trialDbIds, program); - if (trials.size() != trialDbIds.size()) { - List missingIds = new ArrayList<>(trialDbIds); - missingIds.removeAll(trials.stream().map(BrAPITrial::getTrialDbId).collect(Collectors.toList())); - throw new IllegalStateException("Trial not found for trialDbId(s): " + String.join(COMMA_DELIMITER, missingIds)); - } - - String trialRefSource = String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName()); - trials.forEach(trial -> processAndCachePendingTrial(trial, trialRefSource, "foo")); - } catch (ApiException e) { - log.error("Error fetching trials: " + Utilities.generateApiExceptionLogMessage(e), e); - throw new InternalServerException(e.toString(), e); - } - } - } - private void initializeTrialsForExistingObservationUnits(Program program, Map> trialByName) { if(observationUnitByNameNoScope.size() > 0) { Set trialDbIds = new HashSet<>(); From b3396c8a913375f1293556bae8f74cfbe71b93f7 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:42:43 -0400 Subject: [PATCH 34/40] [BI-2046] delete unused helper method --- .../processors/ExperimentProcessor.java | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 6239e4325..b7c68fa50 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -2226,36 +2226,6 @@ private void processAndCacheTrial( new PendingImportObject<>(ImportObjectState.EXISTING, existingTrial, experimentId)); } - /** - * Cache pending trial with reference to observation unit. - * - * This method processes the given existing trial, associates it with the - * specified - * program, retrieves the experiment ID from the trial's external references, - * then caches it with the provided organizational unit reference ID. - * - * @param existingTrial The existing trial to process and cache. - * @param trialRefSource The source to retrieve the trial's reference. - * @param referenceOUId The ID of the reference observation unit. - */ - private void processAndCachePendingTrial( - BrAPITrial existingTrial, - String trialRefSource, - String referenceOUId) { - - // Retrieve experiment ID from existingTrial - BrAPIExternalReference experimentIDRef = Utilities - .getExternalReference(existingTrial.getExternalReferences(), trialRefSource) - .orElseThrow(() -> new InternalServerException( - "An Experiment ID was not found in any of the external references")); - UUID experimentId = UUID.fromString(experimentIDRef.getReferenceId()); - - // Cache pending trial by observation unit ID - pendingTrialByOUId.put( - referenceOUId, - new PendingImportObject<>(ImportObjectState.EXISTING, existingTrial, experimentId)); - } - /** * Sets the reference Observation Unit IDs based on the import rows. * Checks for references to existing observation units and populates the From ac8072b19f0096a1f0484e73c5afb61a26729c8f Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:46:06 -0400 Subject: [PATCH 35/40] [BI-2046] document exp process helper method --- .../services/processors/ExperimentProcessor.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index b7c68fa50..412b34222 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -2213,8 +2213,7 @@ private Set fetchTrialDbidsForStudies(Set studyDbIds, Program pr private void processAndCacheTrial( BrAPITrial existingTrial, Program program, - Map> trialByNameNoScope - ) { + Map> trialByNameNoScope) { //get TrialId from existingTrial BrAPIExternalReference experimentIDRef = Utilities.getExternalReference(existingTrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())) @@ -2227,10 +2226,12 @@ private void processAndCacheTrial( } /** - * Sets the reference Observation Unit IDs based on the import rows. - * Checks for references to existing observation units and populates the - * referenceOUIds list. - * Sets flags hasAllReferenceUnitIds and hasNoReferenceUnitIds accordingly. + * This function collates unique ObsUnitID values from a list of BrAPIImport objects. + * It iterates through the list and adds non-blank ObsUnitID values to a Set. It also checks for any repeated ObsUnitIDs. + * + * @param importRows a List of BrAPIImport objects containing ExperimentObservation data + * @return a Set of unique ObsUnitID strings + * @throws IllegalStateException if a repeated ObsUnitID is encountered */ private Set collateReferenceOUIds(List importRows) { Set referenceOUIds = new HashSet<>(); From 4a328778c060ada73065cc2b014337dc56849c87 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:49:24 -0400 Subject: [PATCH 36/40] [BI-2046] document exp process helper method --- .../importer/services/processors/ExperimentProcessor.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 412b34222..f4bc0a0d0 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -2418,6 +2418,14 @@ private String seasonDbIdToYearFromDatabase(String seasonDbId, UUID programId) { return (yearInt == null) ? "" : yearInt.toString(); } + /** + * Returns the single value from the given map, throwing an UnprocessableEntityException if the map does not contain exactly one entry. + * + * @param map The map from which to retrieve the single value. + * @param message The error message to include in the UnprocessableEntityException if the map does not contain exactly one entry. + * @return The single value from the map. + * @throws UnprocessableEntityException if the map does not contain exactly one entry. + */ private V getSingleEntryValue(Map map, String message) throws UnprocessableEntityException { if (map.size() != 1) { throw new UnprocessableEntityException(message); From b98da08c0df8b8309b70accf9ee700b1e529e97e Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:57:40 -0400 Subject: [PATCH 37/40] [BI-2046] clean up --- .../importer/services/processors/ExperimentProcessor.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index f4bc0a0d0..b14b935c2 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -128,8 +128,6 @@ public class ExperimentProcessor implements Processor { private Map> pendingStudyByOUId = new HashMap<>(); private Map> obsVarDatasetByName = null; private Map> pendingObsDatasetByOUId = new HashMap<>(); - // It is assumed that there are no preexisting Observation Units for the given environment (so this will not be - // initialized by getExistingBrapiData() ) private Map> observationUnitByNameNoScope = null; private Map> pendingObsUnitByOUId = new HashMap<>(); From be76c1238619748fa75fbbf179f90312fc8fc84e Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 1 Apr 2024 15:15:44 -0400 Subject: [PATCH 38/40] [BI-2046] Update expected error message in test --- .../importer/services/processors/ExperimentProcessor.java | 7 ++++--- .../brapps/importer/ExperimentFileImportTest.java | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index b14b935c2..c8757c8eb 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -1345,8 +1345,9 @@ private PendingImportObject fetchOrCreateTrialPIO( if (trialByNameNoScope.containsKey(importRow.getExpTitle())) { PendingImportObject envPio; trialPio = trialByNameNoScope.get(importRow.getExpTitle()); - envPio = this.studyByNameNoScope.get(importRow.getEnv()); - if (trialPio!=null && ImportObjectState.EXISTING==trialPio.getState() && (StringUtils.isBlank( importRow.getObsUnitID() )) && (envPio!=null && ImportObjectState.EXISTING==envPio.getState() ) ){ + envPio = studyByNameNoScope.get(importRow.getEnv()); + if (trialPio!=null && ImportObjectState.EXISTING==trialPio.getState() && + (StringUtils.isBlank( importRow.getObsUnitID() )) && (envPio!=null && ImportObjectState.EXISTING==envPio.getState() ) ){ throw new UnprocessableEntityException(PREEXISTING_EXPERIMENT_TITLE); } } else if (!trialByNameNoScope.isEmpty()) { @@ -1359,7 +1360,7 @@ private PendingImportObject fetchOrCreateTrialPIO( } BrAPITrial newTrial = importRow.constructBrAPITrial(program, user, commit, BRAPI_REFERENCE_SOURCE, id, expSeqValue); trialPio = new PendingImportObject<>(ImportObjectState.NEW, newTrial, id); - this.trialByNameNoScope.put(importRow.getExpTitle(), trialPio); + trialByNameNoScope.put(importRow.getExpTitle(), trialPio); } } return trialPio; diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 9420f18f0..6bc203d64 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -635,7 +635,7 @@ public void verifyFailureNewOuExistingEnv(boolean commit) { JsonObject result = JsonParser.parseString(upload.body()).getAsJsonObject().getAsJsonObject("result"); assertEquals(422, result.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); - assertTrue(result.getAsJsonObject("progress").get("message").getAsString().startsWith("Experimental entities are missing ObsUnitIDs")); + assertTrue(result.getAsJsonObject("progress").get("message").getAsString().startsWith("Experiment Title already exists")); } @Test From 7ea6dc49757de1af0eea413829d9c5be627f7d08 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:44:55 -0400 Subject: [PATCH 39/40] [BI-2046] remove redundant part of error message --- .../importer/services/processors/ExperimentProcessor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index c8757c8eb..e584f7ca4 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -84,7 +84,7 @@ public class ExperimentProcessor implements Processor { private static final String NAME = "Experiment"; - private static final String MISSING_OBS_UNIT_ID_ERROR = "Experimental entities are missing ObsUnitIDs. Import cannot proceed"; + private static final String MISSING_OBS_UNIT_ID_ERROR = "Experimental entities are missing ObsUnitIDs"; private static final String PREEXISTING_EXPERIMENT_TITLE = "Experiment Title already exists"; private static final String MULTIPLE_EXP_TITLES = "File contains more than one Experiment Title"; private static final String MIDNIGHT = "T00:00:00-00:00"; From 8bc7bc56b88f1b41be99391e544bbd936a35485b Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 3 Apr 2024 17:37:59 -0400 Subject: [PATCH 40/40] [BI-2046] add comment --- .../importer/services/processors/ExperimentProcessor.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index e584f7ca4..cbc7299d1 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -1346,6 +1346,8 @@ private PendingImportObject fetchOrCreateTrialPIO( PendingImportObject envPio; trialPio = trialByNameNoScope.get(importRow.getExpTitle()); envPio = studyByNameNoScope.get(importRow.getEnv()); + + // creating new units for existing experiments and environments is not possible if (trialPio!=null && ImportObjectState.EXISTING==trialPio.getState() && (StringUtils.isBlank( importRow.getObsUnitID() )) && (envPio!=null && ImportObjectState.EXISTING==envPio.getState() ) ){ throw new UnprocessableEntityException(PREEXISTING_EXPERIMENT_TITLE); @@ -2226,7 +2228,9 @@ private void processAndCacheTrial( /** * This function collates unique ObsUnitID values from a list of BrAPIImport objects. - * It iterates through the list and adds non-blank ObsUnitID values to a Set. It also checks for any repeated ObsUnitIDs. + * It iterates through the list and adds non-blank ObsUnitID values to a Set. + * It also checks for any repeated ObsUnitIDs. The instance variables hasAllReferenceUnitIds and + * hasNoReferenceUnitIds are updated. * * @param importRows a List of BrAPIImport objects containing ExperimentObservation data * @return a Set of unique ObsUnitID strings