From 5af44b98bbb710550d9ba8b2beedee40acb3e1c1 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 30 Jun 2025 16:20:37 -0400 Subject: [PATCH 01/13] add test for appending obs data using prior variables --- ...leSubmissionControllerIntegrationTest.java | 2 +- .../v2/ListControllerIntegrationTest.java | 2 +- .../importer/ExperimentFileImportTest.java | 200 ++++++++++++++---- .../brapps/importer/ImportTestUtils.java | 43 +++- .../SampleSubmissionFileImportTest.java | 2 +- 5 files changed, 196 insertions(+), 53 deletions(-) diff --git a/src/test/java/org/breedinginsight/api/v1/controller/SampleSubmissionControllerIntegrationTest.java b/src/test/java/org/breedinginsight/api/v1/controller/SampleSubmissionControllerIntegrationTest.java index 978bb0aa8..44a6e3425 100644 --- a/src/test/java/org/breedinginsight/api/v1/controller/SampleSubmissionControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/api/v1/controller/SampleSubmissionControllerIntegrationTest.java @@ -407,7 +407,7 @@ private String createExperiment(Program program) throws IOException, Interrupted .get(0).getAsJsonObject().get("id").getAsString(); JsonObject importResult = importTestUtils.uploadAndFetchWorkflow( - importTestUtils.writeExperimentDataToFile(List.of(makeExpImportRow("Env1")), null, false), + importTestUtils.writeExperimentDataToFile(List.of(makeExpImportRow("Env1")), null, false, false, null), null, true, client, diff --git a/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java index ca6e5c2e9..b5ee8ce0d 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/ListControllerIntegrationTest.java @@ -148,7 +148,7 @@ public void setup() { newExp.put(traits.get(0).getObservationVariableName(), "1"); JsonObject result = importTestUtils.uploadAndFetchWorkflow( - importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); } @Test diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 8275e1cea..45631d558 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -48,6 +48,7 @@ import org.breedinginsight.api.model.v1.request.SpeciesRequest; import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; import org.breedinginsight.brapi.v2.dao.*; +import org.breedinginsight.brapi.v2.services.BrAPITrialService; import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation.Columns; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.dao.db.tables.pojos.BiUserEntity; @@ -63,6 +64,8 @@ import org.breedinginsight.services.exceptions.BadRequestException; import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.services.exceptions.ValidatorException; +import org.breedinginsight.utilities.DatasetUtil; +import org.breedinginsight.utilities.FileUtil; import org.breedinginsight.utilities.Utilities; import org.jooq.DSLContext; import org.junit.jupiter.api.*; @@ -70,8 +73,10 @@ import org.junit.jupiter.params.provider.ValueSource; import org.junit.platform.commons.util.StringUtils; import org.opentest4j.AssertionFailedError; +import tech.tablesaw.api.Table; import javax.inject.Inject; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; import java.time.OffsetDateTime; @@ -80,6 +85,7 @@ import java.util.stream.StreamSupport; import static io.micronaut.http.HttpRequest.GET; +import static io.micronaut.http.HttpRequest.POST; import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.OBSERVATION_UNIT_ID_SUFFIX; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -93,6 +99,7 @@ public class ExperimentFileImportTest extends BrAPITest { private FannyPack securityFp; private String mappingId; private BiUserEntity testUser; + private Program program; @Property(name = "brapi.server.reference-source") private String BRAPI_REFERENCE_SOURCE; @@ -136,6 +143,9 @@ public class ExperimentFileImportTest extends BrAPITest { @Inject private ProgramLocationService locationService; + @Inject + private BrAPITrialService experimentService; + @Inject private BrAPIGermplasmDAO germplasmDAO; @@ -160,6 +170,7 @@ public void setup() { mappingId = (String) setupObjects.get("mappingId"); testUser = (BiUserEntity) setupObjects.get("testUser"); securityFp = (FannyPack) setupObjects.get("securityFp"); + program = (Program) setupObjects.get("program"); newExperimentWorkflowId = importTestUtils.getExperimentWorkflowId(client, 0); appendOverwriteWorkflowId = importTestUtils.getExperimentWorkflowId(client, 1); } @@ -180,6 +191,115 @@ public void setup() { - existing env that already has observation variables (existing dataset) */ + @Test + @SneakyThrows + //@Disabled + public void appendExperimentWithObsVarFromPriorDataset() { + log.debug("appendExperimentWithObsVarFromPriorDataset"); + + // Create a plot-level dataset that includes observation variable tt_test_1 + List traits = importTestUtils.createTraits(1); + Program program = createProgram("Append Exp with Prior Observations Vars", "EXPPRI", "EXPPRI", 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); + + JsonObject importResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); + String expId = importResponse + .get("preview").getAsJsonObject() + .get("rows").getAsJsonArray() + .get(0).getAsJsonObject() + .get("trial").getAsJsonObject() + .get("id").getAsString(); + + // Create sub-entity dataset that has two plant-level units but no obsvars and, therefore, no observations. + Flowable> postCall = client.exchange( + POST(String.format("/programs/%s/experiments/%s/dataset", + program.getId().toString(), expId), + "{\"name\":\"Plant\",\"repeatedMeasures\":2}") + .cookie(new NettyCookie("phylo-token", "test-registered-user")), + String.class); + HttpResponse postResponse = postCall.blockingFirst(); + + // Assert 200 response + assertEquals(HttpStatus.OK, postResponse.getStatus()); + + // Grab the system ids for the sub-entity dataset units + JsonObject result = JsonParser.parseString(postResponse.body()).getAsJsonObject().getAsJsonObject("result"); + + // Export the plant-level sub-entity dataset + String extension = "CSV"; + BrAPITrial experiment = experimentService.getTrialDataByUUID(program.getId(), UUID.fromString(expId), false); + String plantDatasetId = DatasetUtil.getDatasetIdByNameFromJson(experiment.getAdditionalInfo().getAsJsonArray("datasets"), "Plant"); + Flowable> plantExportCall = client.exchange( + GET(String.format("/programs/%s/experiments/%s/export?all=true&includeTimestamps=false&fileExtension=%s&datasetId=%s", + program.getId().toString(), expId, extension, plantDatasetId)) + .cookie(new NettyCookie("phylo-token", "test-registered-user")), byte[].class + ); + HttpResponse plantResponse = plantExportCall.blockingFirst(); + + // Make sub-entity dataset export request. + ByteArrayInputStream bodyStream = new ByteArrayInputStream(Objects.requireNonNull(plantResponse.body())); + Table exportTable = FileUtil.parseTableFromCsv(bodyStream); + + // Assert 200 response + assertEquals(HttpStatus.OK, plantResponse.getStatus()); + + // Build a request to append tt_test_1 observation data on the sub-entity dataset + String sub1ObsUnitId = exportTable.row(0).getString("Plant ObsUnitID"); + String sub2ObsUnitId = exportTable.row(1).getString("Plant ObsUnitID"); + + Map sub1 = new HashMap<>(); + sub1.put(Columns.GERMPLASM_GID, "1"); + sub1.put(Columns.TEST_CHECK, "T"); + sub1.put(Columns.EXP_TITLE, "Test Exp"); + sub1.put(Columns.EXP_UNIT, "Plot"); + sub1.put(Columns.EXP_TYPE, "Phenotyping"); + sub1.put(Columns.ENV, "New Env"); + sub1.put(Columns.ENV_LOCATION, "Location A"); + sub1.put(Columns.ENV_YEAR, "2023"); + sub1.put(Columns.EXP_UNIT_ID, "a-1"); + sub1.put(Columns.REP_NUM, "1"); + sub1.put(Columns.BLOCK_NUM, "1"); + sub1.put(Columns.ROW, "1"); + sub1.put(Columns.COLUMN, "1"); + sub1.put("Plant " + OBSERVATION_UNIT_ID_SUFFIX, sub1ObsUnitId); + sub1.put(traits.get(0).getObservationVariableName(), "1"); + + Map sub2 = new HashMap<>(); + sub2.put(Columns.GERMPLASM_GID, "1"); + sub2.put(Columns.TEST_CHECK, "T"); + sub2.put(Columns.EXP_TITLE, "Test Exp"); + sub2.put(Columns.EXP_UNIT, "Plot"); + sub2.put(Columns.EXP_TYPE, "Phenotyping"); + sub2.put(Columns.ENV, "New Env"); + sub2.put(Columns.ENV_LOCATION, "Location A"); + sub2.put(Columns.ENV_YEAR, "2023"); + sub2.put(Columns.EXP_UNIT_ID, "a-1"); + sub2.put(Columns.REP_NUM, "1"); + sub2.put(Columns.BLOCK_NUM, "1"); + sub2.put(Columns.ROW, "1"); + sub2.put(Columns.COLUMN, "1"); + sub2.put("Plant " + OBSERVATION_UNIT_ID_SUFFIX, sub2ObsUnitId); + sub2.put(traits.get(0).getObservationVariableName(), "2"); + + // Verify that the validation check returns a 400-level response since tt_test_1 is already used in the plot-level dataset + JsonObject previewResponse = importTestUtils.uploadAndFetchWorkflowPreview(importTestUtils.writeExperimentDataToFile(List.of(sub1, sub2), null, true, false, "Plant"), null, true, client, program, mappingId, appendOverwriteWorkflowId); + assertEquals(422, previewResponse.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); + } + @Test @SneakyThrows public void importNewExpNewLocNoObsSuccess() { @@ -202,7 +322,7 @@ public void importNewExpNewLocNoObsSuccess() { validRow.put(Columns.COLUMN, "1"); validRow.put(Columns.TREATMENT_FACTORS, "Test treatment factors"); - JsonObject uploadResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(validRow), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + JsonObject uploadResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(validRow), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); JsonArray previewRows = uploadResponse.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -254,7 +374,7 @@ public void importNewExpMultiNewEnvSuccess() { secondEnv.put(Columns.COLUMN, "1"); secondEnv.put(Columns.TREATMENT_FACTORS, "Test treatment factors"); - JsonObject uploadResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(firstEnv, secondEnv), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + JsonObject uploadResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(firstEnv, secondEnv), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); JsonArray previewRows = uploadResponse.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(2, previewRows.size()); @@ -295,7 +415,7 @@ public void importExistingExpAndEnvErrorMessage() { newExp.put(Columns.ROW, "1"); newExp.put(Columns.COLUMN, "1"); - importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); Map dupExp = new HashMap<>(); dupExp.put(Columns.GERMPLASM_GID, "1"); @@ -312,7 +432,7 @@ public void importExistingExpAndEnvErrorMessage() { dupExp.put(Columns.ROW, "1"); dupExp.put(Columns.COLUMN, "1"); - JsonObject expResult = importTestUtils.uploadAndFetchWorkflowNoStatusCheck(importTestUtils.writeExperimentDataToFile(List.of(dupExp), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + JsonObject expResult = importTestUtils.uploadAndFetchWorkflowNoStatusCheck(importTestUtils.writeExperimentDataToFile(List.of(dupExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); assertEquals(422, expResult.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + expResult); assertTrue(expResult.getAsJsonObject("progress").get("message").getAsString().startsWith("Experiment Title already exists")); @@ -339,7 +459,7 @@ public void importNewEnvNoObsSuccess() { newEnv.put(Columns.ROW, "1"); newEnv.put(Columns.COLUMN, "1"); - JsonObject uploadResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newEnv), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + JsonObject uploadResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newEnv), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); JsonArray previewRows = uploadResponse.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -375,52 +495,52 @@ public void verifyMissingDataThrowsError(boolean commit) { Map noGID = new HashMap<>(base); noGID.remove(Columns.GERMPLASM_GID); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noGID), null, false), Columns.GERMPLASM_GID, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noGID), null, false, false, null), Columns.GERMPLASM_GID, commit, newExperimentWorkflowId); Map noExpTitle = new HashMap<>(base); noExpTitle.remove(Columns.EXP_TITLE); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpTitle), null, false), Columns.EXP_TITLE, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpTitle), null, false, false, null), Columns.EXP_TITLE, commit, newExperimentWorkflowId); Map noExpUnit = new HashMap<>(base); noExpUnit.remove(Columns.EXP_UNIT); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpUnit), null, false), Columns.EXP_UNIT, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpUnit), null, false, false, null), Columns.EXP_UNIT, commit, newExperimentWorkflowId); Map noExpType = new HashMap<>(base); noExpType.remove(Columns.EXP_TYPE); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpType), null, false), Columns.EXP_TYPE, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpType), null, false, false, null), Columns.EXP_TYPE, commit, newExperimentWorkflowId); Map noEnv = new HashMap<>(base); noEnv.remove(Columns.ENV); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noEnv), null, false), Columns.ENV, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noEnv), null, false, false, null), Columns.ENV, commit, newExperimentWorkflowId); Map noEnvLoc = new HashMap<>(base); noEnvLoc.remove(Columns.ENV_LOCATION); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noEnvLoc), null, false), Columns.ENV_LOCATION, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noEnvLoc), null, false, false, null), Columns.ENV_LOCATION, commit, newExperimentWorkflowId); Map noExpUnitId = new HashMap<>(base); noExpUnitId.remove(Columns.EXP_UNIT_ID); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpUnitId), null, false), Columns.EXP_UNIT_ID, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpUnitId), null, false, false, null), Columns.EXP_UNIT_ID, commit, newExperimentWorkflowId); Map noExpRep = new HashMap<>(base); noExpRep.remove(Columns.REP_NUM); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpRep), null, false), Columns.REP_NUM, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpRep), null, false, false, null), Columns.REP_NUM, commit, newExperimentWorkflowId); Map noExpBlock = new HashMap<>(base); noExpBlock.remove(Columns.BLOCK_NUM); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpBlock), null, false), Columns.BLOCK_NUM, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noExpBlock), null, false, false, null), Columns.BLOCK_NUM, commit, newExperimentWorkflowId); Map noEnvYear = new HashMap<>(base); noEnvYear.remove(Columns.ENV_YEAR); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noEnvYear), null, false), Columns.ENV_YEAR, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(noEnvYear), null, false, false, null), Columns.ENV_YEAR, commit, newExperimentWorkflowId); } @Test @@ -445,7 +565,7 @@ public void importNewExpWithObsVar() { newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), null); - JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -497,7 +617,7 @@ public void verifyDiffYearSameEnvThrowsError(boolean commit) { row.put(Columns.BLOCK_NUM, "2"); rows.add(row); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(rows, null, false), Columns.ENV_YEAR, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(rows, null, false, false, null), Columns.ENV_YEAR, commit, newExperimentWorkflowId); } @@ -537,7 +657,7 @@ public void verifyDiffLocSameEnvThrowsError(boolean commit) { row.put(Columns.BLOCK_NUM, "2"); rows.add(row); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(rows, null, false), Columns.ENV_LOCATION, commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(rows, null, false, false, null), Columns.ENV_LOCATION, commit, newExperimentWorkflowId); } @ParameterizedTest @@ -563,7 +683,7 @@ public void importNewExpWithObs(boolean commit) { newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), "1"); - JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false), null, true, client, program, mappingId, newExperimentWorkflowId); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -604,7 +724,7 @@ public void verifyFailureImportNewExpWithInvalidObs(boolean commit) { newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), "Red"); - uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false), traits.get(0).getObservationVariableName(), commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailure(program, importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false, false, null), traits.get(0).getObservationVariableName(), commit, newExperimentWorkflowId); } @@ -629,14 +749,14 @@ public void verifyFailureNewOuExistingEnv(boolean commit) { newExp.put(Columns.ROW, "1"); newExp.put(Columns.COLUMN, "1"); - importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); Map newOU = new HashMap<>(newExp); newOU.put(Columns.EXP_UNIT_ID, "a-2"); newOU.put(Columns.ROW, "1"); newOU.put(Columns.COLUMN, "2"); - JsonObject result = importTestUtils.uploadAndFetchWorkflowNoStatusCheck(importTestUtils.writeExperimentDataToFile(List.of(newOU), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + JsonObject result = importTestUtils.uploadAndFetchWorkflowNoStatusCheck(importTestUtils.writeExperimentDataToFile(List.of(newOU), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); assertEquals(422, result.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); assertTrue(result.getAsJsonObject("progress").get("message").getAsString().startsWith("Experiment Title already exists")); @@ -664,7 +784,7 @@ public void importNewObsVarExistingOu() { newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), null); - importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); 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())); @@ -692,7 +812,7 @@ public void importNewObsVarExistingOu() { newObsVar.put("Plot "+OBSERVATION_UNIT_ID_SUFFIX, ouIdXref.get().getReferenceId()); newObsVar.put(traits.get(1).getObservationVariableName(), null); - JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits, true), null, true, client, program, mappingId, appendOverwriteWorkflowId); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits, true, false, null), null, true, client, program, mappingId, appendOverwriteWorkflowId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -730,7 +850,7 @@ public void importNewObsVarByObsUnitId() { newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), null); - importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); 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())); @@ -745,7 +865,7 @@ public void importNewObsVarByObsUnitId() { newObsVar.put("Plot "+OBSERVATION_UNIT_ID_SUFFIX, ouIdXref.get().getReferenceId()); newObsVar.put(traits.get(1).getObservationVariableName(), null); - JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits, true), null, true, client, program, mappingId, appendOverwriteWorkflowId); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits, true, false, null), null, true, client, program, mappingId, appendOverwriteWorkflowId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -784,7 +904,7 @@ public void importNewObservationDataByObsUnitId(boolean commit) { newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), null); // empty dataset - importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); 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())); @@ -812,7 +932,7 @@ public void importNewObservationDataByObsUnitId(boolean commit) { newObsVar.put("Plot "+OBSERVATION_UNIT_ID_SUFFIX, ouIdXref.get().getReferenceId()); newObsVar.put(traits.get(0).getObservationVariableName(), "1"); - JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits, true), null, commit, client, program, mappingId, appendOverwriteWorkflowId); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits, true, false, null), null, commit, client, program, mappingId, appendOverwriteWorkflowId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -860,7 +980,7 @@ public void verifyBlankObsInOverwriteIsNoOp(boolean commit) { newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), "1"); // Valid observation value. - importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); // Fetch the ObsUnitId to use in the overwrite upload. BrAPITrial brAPITrial = brAPITrialDAO.getTrialsByName(List.of((String)newExp.get(Columns.EXP_TITLE)), program).get(0); @@ -894,7 +1014,7 @@ public void verifyBlankObsInOverwriteIsNoOp(boolean commit) { requestBody.put("overwrite", "true"); requestBody.put("overwriteReason", "testing"); - JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits, true), requestBody, commit, client, program, mappingId, appendOverwriteWorkflowId); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObsVar), traits, true, false, null), requestBody, commit, client, program, mappingId, appendOverwriteWorkflowId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); JsonObject row = previewRows.get(0).getAsJsonObject(); @@ -930,7 +1050,7 @@ public void importNewObsExistingOu(boolean commit) { newExp.put(Columns.ROW, "1"); newExp.put(Columns.COLUMN, "1"); - importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); 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())); @@ -958,7 +1078,7 @@ public void importNewObsExistingOu(boolean commit) { newObservation.put("Plot "+OBSERVATION_UNIT_ID_SUFFIX, ouIdXref.get().getReferenceId()); newObservation.put(traits.get(0).getObservationVariableName(), "1"); - JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits, true), null, commit, client, program, mappingId, appendOverwriteWorkflowId); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits, true, false, null), null, commit, client, program, mappingId, appendOverwriteWorkflowId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -998,7 +1118,7 @@ public void verifyFailureImportNewObsExistingOuWithExistingObs(boolean commit) { newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), "1"); - importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); 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())); @@ -1026,7 +1146,7 @@ public void verifyFailureImportNewObsExistingOuWithExistingObs(boolean commit) { newObservation.put("Plot "+OBSERVATION_UNIT_ID_SUFFIX, ouIdXref.get().getReferenceId()); newObservation.put(traits.get(0).getObservationVariableName(), "2"); - uploadAndVerifyWorkflowFailureNonTabular(program, importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits, true), traits.get(0).getObservationVariableName(), commit, newExperimentWorkflowId); + uploadAndVerifyWorkflowFailureNonTabular(program, importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits, true, false, null), traits.get(0).getObservationVariableName(), commit, newExperimentWorkflowId); } /* @@ -1057,7 +1177,7 @@ public void importSecondExpAfterFirstExpWithObs() { newExpA.put(Columns.COLUMN, "1"); newExpA.put(traits.get(0).getObservationVariableName(), "1"); - JsonObject resultA = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExpA), traits, false), null, true, client, program, mappingId, newExperimentWorkflowId); + JsonObject resultA = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExpA), traits, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); JsonArray previewRowsA = resultA.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRowsA.size()); @@ -1085,7 +1205,7 @@ public void importSecondExpAfterFirstExpWithObs() { newExpB.put(Columns.COLUMN, "1"); newExpB.put(traits.get(0).getObservationVariableName(), "1"); - JsonObject resultB = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExpB), traits, false), null, true, client, program, mappingId, newExperimentWorkflowId); + JsonObject resultB = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExpB), traits, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); JsonArray previewRowsB = resultB.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRowsB.size()); @@ -1127,7 +1247,7 @@ public void importNewObsAfterFirstExpWithObs(boolean commit) { newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), "1"); - importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); 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())); @@ -1156,7 +1276,7 @@ public void importNewObsAfterFirstExpWithObs(boolean commit) { newObservation.put(traits.get(0).getObservationVariableName(), "1"); newObservation.put(traits.get(1).getObservationVariableName(), "2"); - JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits, true), null, commit, client, program, mappingId, appendOverwriteWorkflowId); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits, true, false, null), null, commit, client, program, mappingId, appendOverwriteWorkflowId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -1204,7 +1324,7 @@ public void importNewObsAfterFirstExpWithObs_blank(boolean commit) { newExp.put(traits.get(0).getObservationVariableName(), originalValue); newExp.put(traits.get(1).getObservationVariableName(), "2"); - importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); 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())); @@ -1240,7 +1360,7 @@ public void importNewObsAfterFirstExpWithObs_blank(boolean commit) { Map requestBody = new HashMap<>(); requestBody.put("overwrite", "true"); requestBody.put("overwriteReason", "testing"); - JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits, true), requestBody, commit, client, program, mappingId, appendOverwriteWorkflowId); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits, true, false, null), requestBody, commit, client, program, mappingId, appendOverwriteWorkflowId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); diff --git a/src/test/java/org/breedinginsight/brapps/importer/ImportTestUtils.java b/src/test/java/org/breedinginsight/brapps/importer/ImportTestUtils.java index eb4030150..2562a9a66 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ImportTestUtils.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ImportTestUtils.java @@ -240,16 +240,34 @@ public JsonObject uploadAndFetchWorkflow(File file, Program program, String mappingId, String workflowId) throws InterruptedException { + JsonObject result = generatePreview(file,userData, commit, client, program, mappingId, workflowId); + assertEquals(200, result.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); + return result; + } + + public JsonObject uploadAndFetchWorkflowPreview(File file, + Map userData, + Boolean commit, + RxHttpClient client, + Program program, + String mappingId, + String workflowId) throws InterruptedException { + return generatePreview(file, userData, commit, client, program, mappingId, workflowId); + } + + private JsonObject generatePreview(File file, + Map userData, + Boolean commit, + RxHttpClient client, + Program program, + String mappingId, + String workflowId) throws InterruptedException { Flowable> call = uploadWorkflowDataFile(file, userData, commit, client, program, mappingId, workflowId); HttpResponse response = call.blockingFirst(); - assertEquals(HttpStatus.ACCEPTED, response.getStatus()); - String importId = JsonParser.parseString(response.body()).getAsJsonObject().getAsJsonObject("result").get("importId").getAsString(); - HttpResponse upload = getUploadedFile(importId, client, program, mappingId); - JsonObject result = JsonParser.parseString(upload.body()).getAsJsonObject().getAsJsonObject("result"); - assertEquals(200, result.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); - return result; + + return JsonParser.parseString(upload.body()).getAsJsonObject().getAsJsonObject("result"); } public JsonObject uploadAndFetchWorkflowNoStatusCheck(File file, @@ -297,7 +315,8 @@ public List createTraits(int numToCreate) { return traits; } - public File writeExperimentDataToFile(List> data, List traits, boolean ObsUnitIDCol) throws IOException { + public File writeExperimentDataToFile(List> data, List traits, boolean ObsUnitIDCol, boolean isSubEntity, String level) throws IOException { + String obsLevel = level == null ? "Plot" : level; File file = File.createTempFile("test", ".csv"); List columns = new ArrayList<>(); @@ -307,13 +326,17 @@ public File writeExperimentDataToFile(List> data, List> data, List Date: Wed, 2 Jul 2025 17:14:49 -0400 Subject: [PATCH 02/13] add test for appending data to multiple datasets --- .../importer/ExperimentFileImportTest.java | 138 +++++++++++++++++- 1 file changed, 131 insertions(+), 7 deletions(-) diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 45631d558..89203c53c 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -193,7 +193,6 @@ public void setup() { @Test @SneakyThrows - //@Disabled public void appendExperimentWithObsVarFromPriorDataset() { log.debug("appendExperimentWithObsVarFromPriorDataset"); @@ -235,8 +234,6 @@ public void appendExperimentWithObsVarFromPriorDataset() { // Assert 200 response assertEquals(HttpStatus.OK, postResponse.getStatus()); - - // Grab the system ids for the sub-entity dataset units JsonObject result = JsonParser.parseString(postResponse.body()).getAsJsonObject().getAsJsonObject("result"); // Export the plant-level sub-entity dataset @@ -250,13 +247,13 @@ public void appendExperimentWithObsVarFromPriorDataset() { ); HttpResponse plantResponse = plantExportCall.blockingFirst(); - // Make sub-entity dataset export request. - ByteArrayInputStream bodyStream = new ByteArrayInputStream(Objects.requireNonNull(plantResponse.body())); - Table exportTable = FileUtil.parseTableFromCsv(bodyStream); - // Assert 200 response assertEquals(HttpStatus.OK, plantResponse.getStatus()); + // Parse the export table + ByteArrayInputStream bodyStream = new ByteArrayInputStream(Objects.requireNonNull(plantResponse.body())); + Table exportTable = FileUtil.parseTableFromCsv(bodyStream); + // Build a request to append tt_test_1 observation data on the sub-entity dataset String sub1ObsUnitId = exportTable.row(0).getString("Plant ObsUnitID"); String sub2ObsUnitId = exportTable.row(1).getString("Plant ObsUnitID"); @@ -300,6 +297,133 @@ public void appendExperimentWithObsVarFromPriorDataset() { assertEquals(422, previewResponse.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); } + @Test + @SneakyThrows + public void appendExperimentMultipleDatasets() { + log.debug("appendExperimentMultipleDatasets"); + + // Create a plot-level dataset + List traits = importTestUtils.createTraits(1); + Program program = createProgram("Append Exp with Multiple Datasets", "MULSET", "MULSET", 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); + + JsonObject importResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); + String expId = importResponse + .get("preview").getAsJsonObject() + .get("rows").getAsJsonArray() + .get(0).getAsJsonObject() + .get("trial").getAsJsonObject() + .get("id").getAsString(); + + // Create two sub-entity datasets that have two plant-level units + Flowable> sub1PostCall = client.exchange( + POST(String.format("/programs/%s/experiments/%s/dataset", + program.getId().toString(), expId), + "{\"name\":\"Plant\",\"repeatedMeasures\":2}") + .cookie(new NettyCookie("phylo-token", "test-registered-user")), + String.class); + HttpResponse sub1PostResponse = sub1PostCall.blockingFirst(); + Flowable> sub2PostCall = client.exchange( + POST(String.format("/programs/%s/experiments/%s/dataset", + program.getId().toString(), expId), + "{\"name\":\"Plant\",\"repeatedMeasures\":2}") + .cookie(new NettyCookie("phylo-token", "test-registered-user")), + String.class); + HttpResponse sub2PostResponse = sub2PostCall.blockingFirst(); + + // Assert 200 response + assertEquals(HttpStatus.OK, sub1PostResponse.getStatus()); + assertEquals(HttpStatus.OK, sub2PostResponse.getStatus()); + JsonObject sub1Result = JsonParser.parseString(sub1PostResponse.body()) + .getAsJsonObject() + .getAsJsonObject("result"); + JsonObject sub2Result = JsonParser.parseString(sub2PostResponse.body()) + .getAsJsonObject() + .getAsJsonObject("result"); + + // Export the plant-level sub-entity datasets + String extension = "CSV"; + BrAPITrial experiment = experimentService.getTrialDataByUUID(program.getId(), UUID.fromString(expId), false); + String plant1DatasetId = DatasetUtil.getDatasetIdByNameFromJson(experiment.getAdditionalInfo().getAsJsonArray("datasets"), "Plant"); + Flowable> plant1ExportCall = client.exchange( + GET(String.format("/programs/%s/experiments/%s/export?all=true&includeTimestamps=false&fileExtension=%s&datasetId=%s", + program.getId().toString(), expId, extension, plant1DatasetId)) + .cookie(new NettyCookie("phylo-token", "test-registered-user")), byte[].class + ); + HttpResponse plant1Response = plant1ExportCall.blockingFirst(); + + + String plant2DatasetId = DatasetUtil.getDatasetIdByNameFromJson(experiment.getAdditionalInfo().getAsJsonArray("datasets"), "Plant"); + Flowable> plant2ExportCall = client.exchange( + GET(String.format("/programs/%s/experiments/%s/export?all=true&includeTimestamps=false&fileExtension=%s&datasetId=%s", + program.getId().toString(), expId, extension, plant2DatasetId)) + .cookie(new NettyCookie("phylo-token", "test-registered-user")), byte[].class + ); + HttpResponse plant2Response = plant2ExportCall.blockingFirst(); + + // Parse the export tables + ByteArrayInputStream bodyStream1 = new ByteArrayInputStream(Objects.requireNonNull(plant1Response.body())); + Table exportTable1 = FileUtil.parseTableFromCsv(bodyStream1); + ByteArrayInputStream bodyStream2 = new ByteArrayInputStream(Objects.requireNonNull(plant2Response.body())); + Table exportTable2 = FileUtil.parseTableFromCsv(bodyStream2); + + // Build a request to append tt_test_1 observation data on observation units from two separate datasets + String sub1ObsUnitId = exportTable1.row(0).getString("Plant ObsUnitID"); + String sub2ObsUnitId = exportTable2.row(0).getString("Plant ObsUnitID"); + + Map sub1 = new HashMap<>(); + sub1.put(Columns.GERMPLASM_GID, "1"); + sub1.put(Columns.TEST_CHECK, "T"); + sub1.put(Columns.EXP_TITLE, "Test Exp"); + sub1.put(Columns.EXP_UNIT, "Plot"); + sub1.put(Columns.EXP_TYPE, "Phenotyping"); + sub1.put(Columns.ENV, "New Env"); + sub1.put(Columns.ENV_LOCATION, "Location A"); + sub1.put(Columns.ENV_YEAR, "2023"); + sub1.put(Columns.EXP_UNIT_ID, "a-1"); + sub1.put(Columns.REP_NUM, "1"); + sub1.put(Columns.BLOCK_NUM, "1"); + sub1.put(Columns.ROW, "1"); + sub1.put(Columns.COLUMN, "1"); + sub1.put("Plant " + OBSERVATION_UNIT_ID_SUFFIX, sub1ObsUnitId); + sub1.put(traits.get(0).getObservationVariableName(), "1"); + + Map sub2 = new HashMap<>(); + sub1.put(Columns.GERMPLASM_GID, "1"); + sub1.put(Columns.TEST_CHECK, "T"); + sub1.put(Columns.EXP_TITLE, "Test Exp"); + sub1.put(Columns.EXP_UNIT, "Plot"); + sub1.put(Columns.EXP_TYPE, "Phenotyping"); + sub1.put(Columns.ENV, "New Env"); + sub1.put(Columns.ENV_LOCATION, "Location A"); + sub1.put(Columns.ENV_YEAR, "2023"); + sub1.put(Columns.EXP_UNIT_ID, "a-1"); + sub1.put(Columns.REP_NUM, "1"); + sub1.put(Columns.BLOCK_NUM, "1"); + sub1.put(Columns.ROW, "1"); + sub1.put(Columns.COLUMN, "1"); + sub1.put("Plant " + OBSERVATION_UNIT_ID_SUFFIX, sub2ObsUnitId); + sub1.put(traits.get(0).getObservationVariableName(), "2"); + + // Verify that the validation check returns a 400-level response since obervation units belonging to different datasets are included + JsonObject previewResponse = importTestUtils.uploadAndFetchWorkflowPreview(importTestUtils.writeExperimentDataToFile(List.of(sub1, sub2), null, true, false, "Plant"), null, true, client, program, mappingId, appendOverwriteWorkflowId); + assertEquals(422, previewResponse.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + sub1Result + sub2Result); + } + @Test @SneakyThrows public void importNewExpNewLocNoObsSuccess() { From bb70d3d857e3b97c6dceebd3b5e8f3f7acdf5290 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 8 Jul 2025 15:14:33 -0400 Subject: [PATCH 03/13] create validation check for single dataset used in append workflow --- .../AppendOverwriteIDValidation.java | 3 + .../ObservationUnitDuplicateIDValidator.java | 3 + .../ObservationUnitIDBlankValidator.java | 3 + .../ObservationUnitIDColumnNameValidator.java | 3 + .../ObservationUnitIDFormatValidator.java | 3 + ...ObservationUnitSingleDatasetValidator.java | 91 +++++++++++++++++++ .../model/ExpImportProcessConstants.java | 3 +- .../importer/ExperimentFileImportTest.java | 51 +++++++---- 8 files changed, 140 insertions(+), 20 deletions(-) create mode 100644 src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitSingleDatasetValidator.java diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/AppendOverwriteIDValidation.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/AppendOverwriteIDValidation.java index 2692a2eaa..42fe01230 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/AppendOverwriteIDValidation.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/AppendOverwriteIDValidation.java @@ -74,6 +74,9 @@ public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext // Fetch the obs units from the BrAPi service brAPIObservationUnitReadWorkflowInitialization.execute(); + // Validate retrieved observation units + ouIdValidator.validateDynamicColumns(context); + return processNext(context); } catch (EntityNotFoundException e) { /** diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitDuplicateIDValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitDuplicateIDValidator.java index d4621d058..03ee5838c 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitDuplicateIDValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitDuplicateIDValidator.java @@ -37,6 +37,9 @@ public class ObservationUnitDuplicateIDValidator implements DynamicColumnValidator { @Override public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { + // Skip this validation if the observation units have already been fetched from the BrAPI service + if (!ctx.getAppendOverwriteWorkflowContext().getPendingObsUnitByOUId().isEmpty()) return; + if (ctx.getAppendOverwriteWorkflowContext().getObsUnitColName() == null) { throw new BadRequestException(OZEX.getValue()); } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDBlankValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDBlankValidator.java index 97e49ad34..0dddf51ca 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDBlankValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDBlankValidator.java @@ -35,6 +35,9 @@ public class ObservationUnitIDBlankValidator implements DynamicColumnValidator { @Override public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { + // Skip this validation if the observation units have already been fetched from the BrAPI service + if (!ctx.getAppendOverwriteWorkflowContext().getPendingObsUnitByOUId().isEmpty()) return; + if (ctx.getAppendOverwriteWorkflowContext().getObsUnitColName() == null) { throw new BadRequestException(OZEX.getValue()); } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java index 32b2ccbc1..e5a257236 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java @@ -41,6 +41,9 @@ public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws // Skip this validation if it has already been successfully completed if (ctx.getAppendOverwriteWorkflowContext().getObsUnitColName() != null) return; + // Skip this validation if the observation units have already been fetched from the BrAPI service + if (!ctx.getAppendOverwriteWorkflowContext().getPendingObsUnitByOUId().isEmpty()) return; + // Get the names of all the dynamic columns with observation unit ids String[] idColNames = Arrays.stream(ctx.getImportContext().getUpload().getDynamicColumnNames()) .filter(name->name.endsWith(OBSERVATION_UNIT_ID_SUFFIX)).toArray(String[]::new); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDFormatValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDFormatValidator.java index bf612a2a4..93417b2c6 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDFormatValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDFormatValidator.java @@ -40,6 +40,9 @@ public class ObservationUnitIDFormatValidator implements DynamicColumnValidator @Override public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { + // Skip this validation if the observation units have already been fetched from the BrAPI service + if (!ctx.getAppendOverwriteWorkflowContext().getPendingObsUnitByOUId().isEmpty()) return; + if (ctx.getAppendOverwriteWorkflowContext().getObsUnitColName() == null) { throw new BadRequestException(OZEX.getValue()); } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitSingleDatasetValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitSingleDatasetValidator.java new file mode 100644 index 000000000..e33667c66 --- /dev/null +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitSingleDatasetValidator.java @@ -0,0 +1,91 @@ +/* + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.observationUnitID; + +import io.micronaut.context.annotation.Property; +import lombok.extern.slf4j.Slf4j; +import org.brapi.v2.model.BrAPIExternalReference; +import org.brapi.v2.model.pheno.BrAPIObservationUnit; +import org.breedinginsight.brapps.importer.model.response.PendingImportObject; +import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; +import org.breedinginsight.services.exceptions.BadRequestException; +import org.breedinginsight.utilities.Utilities; +import tech.tablesaw.columns.Column; + +import javax.inject.Singleton; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.ErrMessage.OZEX; +import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.ErrMessage.PJZH; + +@Slf4j +@Singleton +public class ObservationUnitSingleDatasetValidator implements DynamicColumnValidator { + private final String referenceSourceBase; + + public ObservationUnitSingleDatasetValidator(@Property(name = "brapi.server.reference-source") String referenceSourceBase) { + this.referenceSourceBase = referenceSourceBase; + } + + + @Override + public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { + // Skip this validation if the observation units have not been fetched from the BrAPI service + if (ctx.getAppendOverwriteWorkflowContext().getPendingObsUnitByOUId().isEmpty()) return; + + if (ctx.getAppendOverwriteWorkflowContext().getObsUnitColName() == null) { + throw new BadRequestException(OZEX.getValue()); + } + + String idColName = ctx.getAppendOverwriteWorkflowContext().getObsUnitColName(); + Column idCol = ctx.getImportContext().getData().columns(idColName).get(0); + Map> unitPioCache = ctx.getAppendOverwriteWorkflowContext().getPendingObsUnitByOUId(); + String singleDatasetId = null; + for (int rowNum = 0; rowNum < ctx.getImportContext().getImportRows().size(); rowNum++) { + // Get the external references for the BrAPI Observation Unit stored with the given id + List refs = unitPioCache + .get(idCol.get(rowNum).toString()) + .getBrAPIObject() + .getExternalReferences(); + + // Find the id of the dataset that owns the given observation unit + String datasetId = Utilities + .getExternalReference(refs, referenceSourceBase, ExternalReferenceSource.DATASET) + .map(BrAPIExternalReference::getReferenceId) + .orElseThrow(() -> new BadRequestException(PJZH.getValue())); + + // Make sure there is only a single unique dataset used in the import + if (singleDatasetId == null) { + singleDatasetId = datasetId; + } else if (!singleDatasetId.equals(datasetId)) { + throw new BadRequestException(PJZH.getValue()); + } + } + + } + + @Override + public int getOrder() { + return 5; + } +} diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java index d4bc7f084..4ab957534 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java @@ -41,7 +41,8 @@ public enum ErrMessage { DUPLICATE_OBS_UNIT_ID("ObsUnitId is repeated"), OZEX("Missing ObsUnitID column"), VVCN("ObsUnitID is duplicated"), - BITB("Invalid or missing ObsUnitID"); + BITB("Invalid or missing ObsUnitID"), + PJZH("Required field is blank"); private String value; diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 89203c53c..d7188b474 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -319,7 +319,6 @@ public void appendExperimentMultipleDatasets() { newExp.put(Columns.BLOCK_NUM, "1"); newExp.put(Columns.ROW, "1"); newExp.put(Columns.COLUMN, "1"); - //newExp.put(traits.get(0).getObservationVariableName(), null); JsonObject importResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); String expId = importResponse @@ -357,8 +356,22 @@ public void appendExperimentMultipleDatasets() { // Export the plant-level sub-entity datasets String extension = "CSV"; + + // Get the dataset ids from the experiment additional info BrAPITrial experiment = experimentService.getTrialDataByUUID(program.getId(), UUID.fromString(expId), false); - String plant1DatasetId = DatasetUtil.getDatasetIdByNameFromJson(experiment.getAdditionalInfo().getAsJsonArray("datasets"), "Plant"); + JsonObject additionalInfo = experiment.getAdditionalInfo(); + JsonArray datasetsJsonArray = additionalInfo.getAsJsonArray("datasets"); + List subEntityDatasetIds = new ArrayList<>(); + for (JsonElement elem : datasetsJsonArray) { + JsonObject datasetObj = elem.getAsJsonObject(); + int level = datasetObj.get("level").getAsInt(); + if (level == 1) { + String id = datasetObj.get("id").getAsString(); + subEntityDatasetIds.add(id); + } + } + + String plant1DatasetId = subEntityDatasetIds.get(0); Flowable> plant1ExportCall = client.exchange( GET(String.format("/programs/%s/experiments/%s/export?all=true&includeTimestamps=false&fileExtension=%s&datasetId=%s", program.getId().toString(), expId, extension, plant1DatasetId)) @@ -367,7 +380,7 @@ public void appendExperimentMultipleDatasets() { HttpResponse plant1Response = plant1ExportCall.blockingFirst(); - String plant2DatasetId = DatasetUtil.getDatasetIdByNameFromJson(experiment.getAdditionalInfo().getAsJsonArray("datasets"), "Plant"); + String plant2DatasetId = subEntityDatasetIds.get(1); Flowable> plant2ExportCall = client.exchange( GET(String.format("/programs/%s/experiments/%s/export?all=true&includeTimestamps=false&fileExtension=%s&datasetId=%s", program.getId().toString(), expId, extension, plant2DatasetId)) @@ -403,25 +416,25 @@ public void appendExperimentMultipleDatasets() { sub1.put(traits.get(0).getObservationVariableName(), "1"); Map sub2 = new HashMap<>(); - sub1.put(Columns.GERMPLASM_GID, "1"); - sub1.put(Columns.TEST_CHECK, "T"); - sub1.put(Columns.EXP_TITLE, "Test Exp"); - sub1.put(Columns.EXP_UNIT, "Plot"); - sub1.put(Columns.EXP_TYPE, "Phenotyping"); - sub1.put(Columns.ENV, "New Env"); - sub1.put(Columns.ENV_LOCATION, "Location A"); - sub1.put(Columns.ENV_YEAR, "2023"); - sub1.put(Columns.EXP_UNIT_ID, "a-1"); - sub1.put(Columns.REP_NUM, "1"); - sub1.put(Columns.BLOCK_NUM, "1"); - sub1.put(Columns.ROW, "1"); - sub1.put(Columns.COLUMN, "1"); - sub1.put("Plant " + OBSERVATION_UNIT_ID_SUFFIX, sub2ObsUnitId); - sub1.put(traits.get(0).getObservationVariableName(), "2"); + sub2.put(Columns.GERMPLASM_GID, "1"); + sub2.put(Columns.TEST_CHECK, "T"); + sub2.put(Columns.EXP_TITLE, "Test Exp"); + sub2.put(Columns.EXP_UNIT, "Plot"); + sub2.put(Columns.EXP_TYPE, "Phenotyping"); + sub2.put(Columns.ENV, "New Env"); + sub2.put(Columns.ENV_LOCATION, "Location A"); + sub2.put(Columns.ENV_YEAR, "2023"); + sub2.put(Columns.EXP_UNIT_ID, "a-1"); + sub2.put(Columns.REP_NUM, "1"); + sub2.put(Columns.BLOCK_NUM, "1"); + sub2.put(Columns.ROW, "1"); + sub2.put(Columns.COLUMN, "1"); + sub2.put("Plant " + OBSERVATION_UNIT_ID_SUFFIX, sub2ObsUnitId); + sub2.put(traits.get(0).getObservationVariableName(), "2"); // Verify that the validation check returns a 400-level response since obervation units belonging to different datasets are included JsonObject previewResponse = importTestUtils.uploadAndFetchWorkflowPreview(importTestUtils.writeExperimentDataToFile(List.of(sub1, sub2), null, true, false, "Plant"), null, true, client, program, mappingId, appendOverwriteWorkflowId); - assertEquals(422, previewResponse.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + sub1Result + sub2Result); + assertEquals(400, previewResponse.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + sub1Result + sub2Result); } @Test From f5b26d14e1920ef1cb8f39f8876a4144eee427d1 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Fri, 11 Jul 2025 09:39:37 -0400 Subject: [PATCH 04/13] create validator for ensuring obs vars are not re-used from other datasets --- .../factory/entity/PendingDataset.java | 32 ++-- .../factory/entity/PendingEntityFactory.java | 13 +- .../AppendOverwriteVariableValidation.java | 84 ++++++++++ .../process/ImportTableProcess.java | 56 +------ .../model/AppendOverwriteWorkflowContext.java | 9 +- ...ervationVariablePriorDatasetValidator.java | 143 ++++++++++++++++++ ...ObservationVariableTimestampValidator.java | 110 ++++++++++++++ .../ObservationVariableValidator.java | 44 ++++++ .../model/ExpImportProcessConstants.java | 3 +- .../experiment/service/DatasetService.java | 14 +- .../importer/ExperimentFileImportTest.java | 1 - 11 files changed, 435 insertions(+), 74 deletions(-) create mode 100644 src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/AppendOverwriteVariableValidation.java create mode 100644 src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java create mode 100644 src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java create mode 100644 src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableValidator.java diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingDataset.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingDataset.java index 216006f23..70460a6c7 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingDataset.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingDataset.java @@ -17,9 +17,11 @@ package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.factory.entity; +import io.micronaut.context.annotation.Property; import io.micronaut.context.annotation.Prototype; import com.google.gson.JsonArray; import org.brapi.client.v2.model.exceptions.ApiException; +import org.brapi.v2.model.BrAPIExternalReference; import org.brapi.v2.model.core.BrAPIListSummary; import org.brapi.v2.model.core.request.BrAPIListNewRequest; import org.brapi.v2.model.core.response.BrAPIListDetails; @@ -27,15 +29,14 @@ import org.breedinginsight.brapi.v2.dao.BrAPIListDAO; import org.breedinginsight.brapps.importer.model.response.ImportObjectState; import org.breedinginsight.brapps.importer.model.response.PendingImportObject; +import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.brapps.importer.services.processors.experiment.ExperimentUtilities; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteWorkflowContext; import org.breedinginsight.brapps.importer.services.processors.experiment.model.ImportContext; import org.breedinginsight.brapps.importer.services.processors.experiment.service.DatasetService; -import org.breedinginsight.services.exceptions.DoesNotExistException; -import org.breedinginsight.services.exceptions.MissingRequiredInfoException; -import org.breedinginsight.services.exceptions.UnprocessableEntityException; import org.breedinginsight.utilities.DatasetUtil; +import org.breedinginsight.utilities.Utilities; import java.util.ArrayList; import java.util.List; @@ -49,16 +50,19 @@ public class PendingDataset implements ExperimentImportEntity BrAPIListDAO brAPIListDAO; DatasetService datasetService; ExperimentUtilities experimentUtilities; + String referenceSourceBase; public PendingDataset(AppendOverwriteMiddlewareContext context, BrAPIListDAO brAPIListDAO, DatasetService datasetService, - ExperimentUtilities experimentUtilities) { + ExperimentUtilities experimentUtilities, + String referenceSourceBase) { this.cache = context.getAppendOverwriteWorkflowContext(); this.importContext = context.getImportContext(); this.brAPIListDAO = brAPIListDAO; this.datasetService = datasetService; this.experimentUtilities = experimentUtilities; + this.referenceSourceBase = referenceSourceBase; } /** * Create new objects generated by the workflow in the BrAPI service. @@ -103,13 +107,21 @@ public List brapiPost(List members) throws A */ @Override public List brapiRead() throws ApiException { - // Get the id of the dataset belonging to the required exp units - JsonArray datasetsJson = cache.getTrialByNameNoScope().values().iterator().next().getBrAPIObject() - .getAdditionalInfo() - .getAsJsonArray(BrAPIAdditionalInfoFields.DATASETS); - String datasetId = DatasetUtil.getTopLevelDatasetFromJson(datasetsJson).getId().toString(); + // Get the dataset id from the xref of any observation unit in the import + List refs = cache + .getPendingObsUnitByOUId() + .values() + .stream() + .findFirst() + .get() + .getBrAPIObject() + .getExternalReferences(); + String datasetId = Utilities + .getExternalReference(refs, referenceSourceBase, ExternalReferenceSource.DATASET) + .map(BrAPIExternalReference::getReferenceId) + .get(); - // Get the dataset belonging to required exp units + // Get the dataset return List.of(datasetService.fetchDatasetById(datasetId, importContext.getProgram()).orElseThrow(ApiException::new)); } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingEntityFactory.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingEntityFactory.java index 20e827f2f..440d663cc 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingEntityFactory.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingEntityFactory.java @@ -19,6 +19,7 @@ import io.micronaut.context.annotation.Bean; import io.micronaut.context.annotation.Factory; +import io.micronaut.context.annotation.Property; import io.micronaut.context.annotation.Prototype; import org.breedinginsight.brapi.v2.dao.*; import org.breedinginsight.brapps.importer.services.processors.experiment.ExperimentUtilities; @@ -45,6 +46,7 @@ public class PendingEntityFactory { private final ProgramLocationService programLocationService; private final LocationService locationService; private final ExperimentUtilities experimentUtilities; + private final String referenceSourceBase; @Inject public PendingEntityFactory(TrialService trialService, @@ -58,7 +60,8 @@ public PendingEntityFactory(TrialService trialService, DatasetService datasetService, BrAPIObservationDAO brAPIObservationDAO, OntologyService ontologyService, ProgramLocationService programLocationService, LocationService locationService, - ExperimentUtilities experimentUtilities) { + ExperimentUtilities experimentUtilities, + @Property(name = "brapi.server.reference-source") String referenceSourceBase) { this.trialService = trialService; this.brapiTrialDAO = brapiTrialDAO; this.observationUnitDAO = observationUnitDAO; @@ -73,6 +76,7 @@ public PendingEntityFactory(TrialService trialService, this.programLocationService = programLocationService; this.locationService = locationService; this.experimentUtilities = experimentUtilities; + this.referenceSourceBase = referenceSourceBase; } public static PendingTrial pendingTrial(AppendOverwriteMiddlewareContext context, @@ -105,8 +109,9 @@ public static PendingGermplasm pendingGermplasm(AppendOverwriteMiddlewareContext public static PendingDataset pendingDataset(AppendOverwriteMiddlewareContext context, BrAPIListDAO brAPIListDAO, DatasetService datasetService, - ExperimentUtilities experimentUtilities) { - return new PendingDataset(context, brAPIListDAO, datasetService, experimentUtilities); + ExperimentUtilities experimentUtilities, + String referenceSourceBase) { + return new PendingDataset(context, brAPIListDAO, datasetService, experimentUtilities, referenceSourceBase); } public static PendingObservation pendingObservation(AppendOverwriteMiddlewareContext context, @@ -150,7 +155,7 @@ public PendingGermplasm pendingGermplasmBean(AppendOverwriteMiddlewareContext co @Bean @Prototype public PendingDataset pendingDatasetBean(AppendOverwriteMiddlewareContext context) { - return pendingDataset(context, brAPIListDAO, datasetService, experimentUtilities); + return pendingDataset(context, brAPIListDAO, datasetService, experimentUtilities, referenceSourceBase); } @Bean diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/AppendOverwriteVariableValidation.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/AppendOverwriteVariableValidation.java new file mode 100644 index 000000000..381ba1113 --- /dev/null +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/AppendOverwriteVariableValidation.java @@ -0,0 +1,84 @@ +/* + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.middleware; + +import io.micronaut.context.annotation.Prototype; +import lombok.extern.slf4j.Slf4j; +import org.brapi.client.v2.model.exceptions.ApiException; +import org.brapi.v2.model.core.BrAPITrial; +import org.brapi.v2.model.core.response.BrAPIListDetails; +import org.breedinginsight.brapps.importer.services.processors.experiment.ExperimentUtilities; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.factory.action.BrAPIReadFactory; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.factory.action.WorkflowReadInitialization; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddleware; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.MiddlewareException; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.observationVariable.ObservationVariableValidator; +import org.breedinginsight.brapps.importer.services.processors.experiment.model.EntityNotFoundException; +import org.breedinginsight.services.exceptions.BadRequestException; +import org.breedinginsight.services.exceptions.ValidatorException; +import org.breedinginsight.api.model.v1.response.ValidationErrors; + +@Slf4j +@Prototype +public class AppendOverwriteVariableValidation extends AppendOverwriteMiddleware { + private final ObservationVariableValidator obsVarValidator; + private final BrAPIReadFactory brAPIReadFactory; + WorkflowReadInitialization brAPITrialReadWorkflowInitialization; + WorkflowReadInitialization brAPIDatasetReadWorkflowInitialization; + + public AppendOverwriteVariableValidation(ObservationVariableValidator obsVarValidator, + BrAPIReadFactory brAPIReadFactory) { + this.obsVarValidator = obsVarValidator; + this.brAPIReadFactory = brAPIReadFactory; + } + + @Override + public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext context) { + brAPITrialReadWorkflowInitialization = brAPIReadFactory.trialWorkflowReadInitializationBean(context); + brAPIDatasetReadWorkflowInitialization = brAPIReadFactory.datasetWorkflowReadInitializationBean(context); + ValidationErrors validationErrors = context.getAppendOverwriteWorkflowContext().getValidationErrors(); + + try { + // Validate any dynamic columns that are phenotype variables + obsVarValidator.validateDynamicColumns(context); + + // Check for tabular errors collected during validation + if (validationErrors.hasErrors()) { + throw new ValidatorException(validationErrors); + } + + // Fetch the observation variables owned by all datasets belonging to the experiment involved in the import + brAPITrialReadWorkflowInitialization.execute(); + brAPIDatasetReadWorkflowInitialization.execute(); + + // Validate again to check that none of the phenotypes in the import belong to other datasets + obsVarValidator.validateDynamicColumns(context); + + return processNext(context); + } catch ( EntityNotFoundException e) { + // TODO: change method to handle errors with other entities besides obs units + ExperimentUtilities.addValidationErrorsForObsUnitsNotFound(e, context); + context.getAppendOverwriteWorkflowContext().setProcessError(new MiddlewareException(new ValidatorException(validationErrors))); + return this.compensate(context); + } catch (BadRequestException | ApiException | ValidatorException e) { + context.getAppendOverwriteWorkflowContext().setProcessError(new MiddlewareException(e)); + return this.compensate(context); + } + } +} diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java index 45eea09e1..d799b0631 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java @@ -107,60 +107,14 @@ public ImportTableProcess(StudyService studyService, @Override public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext context) { - log.debug("verifying traits listed in import"); - - // Get all the phenotypic columns of the import - ImportUpload upload = context.getImportContext().getUpload(); - Table data = context.getImportContext().getData(); - List phenotypeColNames = Arrays.stream(upload.getDynamicColumnNames()) - .filter(name -> !name.endsWith(OBSERVATION_UNIT_ID_SUFFIX)) - .filter(name -> !name.contains(SUB_UNIT_NUMBER)) - .collect(Collectors.toList()); - - // don't allow periods (.) or square brackets in Phenotype Column Names - for (String phenotypeColumnName: phenotypeColNames) { - if(phenotypeColumnName.contains(".") || phenotypeColumnName.contains("[") || phenotypeColumnName.contains("]")){ - String errorMsg = String.format("Observation columns may not contain periods or square brackets (see column '%s')", phenotypeColumnName); - throw new HttpStatusException(HttpStatus.UNPROCESSABLE_ENTITY, errorMsg); - } - } - List> dynamicCols = data.columns(phenotypeColNames.toArray(new String[0])); - - // Collect the columns for observation variable data - List> phenotypeCols = dynamicCols.stream().filter(col -> !col.name().startsWith(TIMESTAMP_PREFIX)).collect(Collectors.toList()); - List varNames = phenotypeCols.stream().map(Column::name).collect(Collectors.toList()); - - // Collect the columns for observation timestamps - List> timestampCols = dynamicCols.stream().filter(col -> col.name().startsWith(TIMESTAMP_PREFIX)).collect(Collectors.toList()); - Set tsNames = timestampCols.stream().map(Column::name).collect(Collectors.toSet()); - - // Construct validation errors for any timestamp columns that don't have a matching variable column - List importRows = context.getImportContext().getImportRows(); - Optional.ofNullable(context.getAppendOverwriteWorkflowContext().getValidationErrors()).orElseGet(() -> { - context.getAppendOverwriteWorkflowContext().setValidationErrors(new ValidationErrors()); - return new ValidationErrors(); - }); - ValidationErrors validationErrors = context.getAppendOverwriteWorkflowContext().getValidationErrors(); - List tsValErrs = observationVariableService.validateMatchedTimestamps(Set.copyOf(varNames), timestampCols).orElse(new ArrayList<>()); - for (int i = 0; i < importRows.size(); i++) { - int rowNum = i; - tsValErrs.forEach(validationError -> validationErrors.addError(rowNum, validationError)); - } - try { - // Stop processing the import if there are unmatched timestamp columns - if (tsValErrs.size() > 0) { - throw new UnprocessableEntityException("One or more timestamp columns do not have a matching observation variable"); - } - - //Now know timestamps all valid phenotypes, can associate with phenotype column name for easy retrieval - Map> tsColByPheno = timestampCols.stream().collect(Collectors.toMap(col -> col.name().replaceFirst(TIMESTAMP_REGEX, StringUtils.EMPTY), col -> col)); - - // Add the map to the context for use in processing import - context.getAppendOverwriteWorkflowContext().setTimeStampColByPheno(tsColByPheno); + ValidationErrors validationErrors = context.getAppendOverwriteWorkflowContext().getValidationErrors(); + Map> tsColByPheno = context.getAppendOverwriteWorkflowContext().getTimeStampColByPheno(); + List> phenotypeCols = context.getAppendOverwriteWorkflowContext().getPhenotypeCols(); + List varNames = context.getAppendOverwriteWorkflowContext().getVarNames(); + Program program = context.getImportContext().getProgram(); // Fetch the traits named in the observation variable columns - Program program = context.getImportContext().getProgram(); List traits = observationVariableService.fetchTraitsByName(Set.copyOf(varNames), program); // Map trait by phenotype column name diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/model/AppendOverwriteWorkflowContext.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/model/AppendOverwriteWorkflowContext.java index b1c7dfbc3..e42b6185a 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/model/AppendOverwriteWorkflowContext.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/model/AppendOverwriteWorkflowContext.java @@ -31,10 +31,7 @@ import org.breedinginsight.model.ProgramLocation; import tech.tablesaw.columns.Column; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; +import java.util.*; @Getter @Setter @@ -58,6 +55,10 @@ public class AppendOverwriteWorkflowContext { private MiddlewareException processError; private ValidationErrors validationErrors; + // Dynamic Columns + private List> phenotypeCols; + private List varNames; + // Cache maps keyed by name without program scope private Map> observationUnitByNameNoScope; private Map> trialByNameNoScope; diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java new file mode 100644 index 000000000..b1822df20 --- /dev/null +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java @@ -0,0 +1,143 @@ +/* + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.observationVariable; + +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import io.micronaut.context.annotation.Property; +import lombok.extern.slf4j.Slf4j; +import org.brapi.client.v2.model.exceptions.ApiException; +import org.brapi.v2.model.BrAPIExternalReference; +import org.brapi.v2.model.core.BrAPITrial; +import org.brapi.v2.model.core.response.BrAPIListDetails; +import org.breedinginsight.brapps.importer.model.response.PendingImportObject; +import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; +import org.breedinginsight.brapps.importer.services.processors.experiment.service.DatasetService; +import org.breedinginsight.services.exceptions.BadRequestException; +import org.breedinginsight.utilities.Utilities; + +import javax.inject.Singleton; +import java.util.*; + +import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.ErrMessage.JABH; + +@Slf4j +@Singleton +public class ObservationVariablePriorDatasetValidator implements DynamicColumnValidator { + private final String referenceSourceBase; + private final DatasetService datasetService; + + public ObservationVariablePriorDatasetValidator(@Property(name = "brapi.server.reference-source") String referenceSourceBase, DatasetService datasetService) { + this.referenceSourceBase = referenceSourceBase; + this.datasetService = datasetService; + } + + @Override + public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { + // Skip the validation if the dependencies have not been fetched from the BrAPI service + if(noMappings(ctx)) return; + + // Get the dataset id used for this import + String datasetId = getImportDatasetId(ctx); + + // Get the ids for the other datasets in the same experiment + Set otherIds = getExperimentDatasetIds(ctx); + otherIds.remove(datasetId); + + // Get the names of any observation variables owned by the other datasets + Set forbiddenVariables = getDatasetVariables(ctx, otherIds); + + // Check that no phenotype name used in the import already belongs to another dataset + if (forbidden(ctx, forbiddenVariables)) { + throw new BadRequestException(JABH.getValue()); + } + } + + @Override + public int getOrder() { + return 2; + } + + private Set getDatasetVariables(AppendOverwriteMiddlewareContext ctx, Set ids) throws ApiException { + Set variables = new HashSet<>(); + List varListDetails = datasetService + .fetchDatasetsByIds(ids, ctx.getImportContext().getProgram()).orElse(new ArrayList<>()); + for (BrAPIListDetails brAPIListDetails : varListDetails) { + variables.addAll(brAPIListDetails.getData()); + } + + return variables; + } + + private boolean forbidden(AppendOverwriteMiddlewareContext ctx, Set forbiddenVariables) { + List importVarNames = ctx.getAppendOverwriteWorkflowContext().getVarNames(); + for (String importVarName : importVarNames) { + if (forbiddenVariables.contains(importVarName)) return true; + } + + return false; + } + + private Set getExperimentDatasetIds(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { + JsonArray expDatasets = ctx.getAppendOverwriteWorkflowContext() + .getPendingTrialByOUId() + .values() + .stream() + .findFirst() + .orElseThrow(() -> new BadRequestException("No pending trial found")) + .getBrAPIObject() + .getAdditionalInfo() + .getAsJsonArray("datasets"); + Set datasetIds = new HashSet<>(); + for (JsonElement expDataset : expDatasets) { + String datasetId = expDataset.getAsJsonObject().get("id").getAsString(); + datasetIds.add(datasetId); + } + + return datasetIds; + } + + private String getImportDatasetId(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { + List refs = ctx.getAppendOverwriteWorkflowContext() + .getPendingObsUnitByOUId() + .values() + .stream() + .findFirst() + .orElseThrow(()->new BadRequestException("No pending obs unit found")) + .getBrAPIObject() + .getExternalReferences(); + + return Utilities + .getExternalReference(refs, referenceSourceBase, ExternalReferenceSource.DATASET) + .map(BrAPIExternalReference::getReferenceId) + .orElseThrow(() -> new BadRequestException("No dataset associated with observation unit")); + } + + private boolean noMappings(AppendOverwriteMiddlewareContext context) { + Map> trialByOUId = context + .getAppendOverwriteWorkflowContext() + .getPendingTrialByOUId(); + Map> datasetByOUId = context + .getAppendOverwriteWorkflowContext() + .getPendingObsDatasetByOUId(); + + return !(trialByOUId == null || trialByOUId.isEmpty() || datasetByOUId == null || datasetByOUId.isEmpty()); + } +} diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java new file mode 100644 index 000000000..538bf220b --- /dev/null +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java @@ -0,0 +1,110 @@ +/* + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.observationVariable; + +import io.micronaut.http.HttpStatus; +import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; +import org.breedinginsight.api.model.v1.response.ValidationError; +import org.breedinginsight.api.model.v1.response.ValidationErrors; +import org.breedinginsight.brapps.importer.model.ImportUpload; +import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; +import org.breedinginsight.brapps.importer.services.processors.experiment.service.ObservationVariableService; +import org.breedinginsight.services.exceptions.BadRequestException; +import org.breedinginsight.services.exceptions.UnprocessableEntityException; +import tech.tablesaw.api.Table; +import tech.tablesaw.columns.Column; + +import javax.inject.Singleton; +import java.util.*; +import java.util.stream.Collectors; + +import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.*; + +@Slf4j +@Singleton +public class ObservationVariableTimestampValidator implements DynamicColumnValidator { + private final ObservationVariableService observationVariableService; + + public ObservationVariableTimestampValidator(ObservationVariableService observationVariableService) { + this.observationVariableService = observationVariableService; + } + + @Override + public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { + log.debug("verifying traits listed in import"); + + // Get all the phenotypic columns of the import + ImportUpload upload = ctx.getImportContext().getUpload(); + Table data = ctx.getImportContext().getData(); + List phenotypeColNames = Arrays.stream(upload.getDynamicColumnNames()) + .filter(name -> !name.endsWith(OBSERVATION_UNIT_ID_SUFFIX)) + .filter(name -> !name.contains(SUB_UNIT_NUMBER)) + .collect(Collectors.toList()); + + // don't allow periods (.) or square brackets in Phenotype Column Names + for (String phenotypeColumnName: phenotypeColNames) { + if(phenotypeColumnName.contains(".") || phenotypeColumnName.contains("[") || phenotypeColumnName.contains("]")){ + String errorMsg = String.format("Observation columns may not contain periods or square brackets (see column '%s')", phenotypeColumnName); + throw new BadRequestException(errorMsg); + } + } + List> dynamicCols = data.columns(phenotypeColNames.toArray(new String[0])); + + // Collect the columns for observation variable data + List> phenotypeCols = dynamicCols.stream().filter(col -> !col.name().startsWith(TIMESTAMP_PREFIX)).collect(Collectors.toList()); + List varNames = phenotypeCols.stream().map(Column::name).collect(Collectors.toList()); + + // Add the phenotypes to the context for use in processing import + ctx.getAppendOverwriteWorkflowContext().setPhenotypeCols(phenotypeCols); + ctx.getAppendOverwriteWorkflowContext().setVarNames(varNames); + + // Collect the columns for observation timestamps + List> timestampCols = dynamicCols.stream().filter(col -> col.name().startsWith(TIMESTAMP_PREFIX)).collect(Collectors.toList()); + + // Construct validation errors for any timestamp columns that don't have a matching variable column + List importRows = ctx.getImportContext().getImportRows(); + ValidationErrors validationErrors = ctx.getAppendOverwriteWorkflowContext().getValidationErrors(); + List tsValErrs = observationVariableService + .validateMatchedTimestamps(Set.copyOf(varNames), timestampCols) + .orElse(new ArrayList<>()); + for (int i = 0; i < importRows.size(); i++) { + int rowNum = i; + tsValErrs.forEach(validationError -> validationErrors.addError(rowNum, validationError)); + } + + if (tsValErrs.isEmpty()) { + //Now know timestamps all valid phenotypes, can associate with phenotype column name for easy retrieval + Map> tsColByPheno = timestampCols + .stream() + .collect(Collectors + .toMap(col -> col.name().replaceFirst(TIMESTAMP_REGEX, StringUtils.EMPTY), + col -> col)); + + // Add the map to the context for use in processing import + ctx.getAppendOverwriteWorkflowContext().setTimeStampColByPheno(tsColByPheno); + } + } + + @Override + public int getOrder() { + return 1; + } +} diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableValidator.java new file mode 100644 index 000000000..ea039024b --- /dev/null +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableValidator.java @@ -0,0 +1,44 @@ +/* + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.observationVariable; + +import io.micronaut.context.annotation.Primary; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; +import org.breedinginsight.services.exceptions.BadRequestException; + +import javax.inject.Singleton; +import java.util.List; + +@Primary +@Singleton +public class ObservationVariableValidator implements DynamicColumnValidator { + private final List validators; + + public ObservationVariableValidator(List validators) { + this.validators = validators; + } + + + @Override + public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { + for (DynamicColumnValidator validator : validators) { + validator.validateDynamicColumns(ctx); + } + } +} diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java index 4ab957534..d508efff0 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java @@ -42,7 +42,8 @@ public enum ErrMessage { OZEX("Missing ObsUnitID column"), VVCN("ObsUnitID is duplicated"), BITB("Invalid or missing ObsUnitID"), - PJZH("Required field is blank"); + PJZH("Required field is blank"), + JABH("Observation variable(s) are already associated with another dataset(s) in this experiment"); private String value; diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java index 68aca9e2b..0cf7d815b 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java @@ -33,9 +33,7 @@ import javax.inject.Inject; import javax.inject.Singleton; -import java.util.List; -import java.util.Optional; -import java.util.UUID; +import java.util.*; @Singleton public class DatasetService { @@ -88,6 +86,16 @@ public Optional fetchDatasetById(String id, Program program) t return dataSetDetails; } + public Optional> fetchDatasetsByIds(Set datasetIds, Program program) throws ApiException { + List datasets = new ArrayList<>(); + for (String datasetId : datasetIds) { + Optional dataSetDetailsOptional = fetchDatasetById(datasetId, program); + dataSetDetailsOptional.ifPresent(datasets::add); + } + + return datasets.isEmpty() ? Optional.empty() : Optional.of(datasets); + } + /** * Constructs a PendingImportObject for a BrAPIListDetails dataset. * This method retrieves the external reference for the dataset from the existing list diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index d7188b474..2ed39d1dd 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -379,7 +379,6 @@ public void appendExperimentMultipleDatasets() { ); HttpResponse plant1Response = plant1ExportCall.blockingFirst(); - String plant2DatasetId = subEntityDatasetIds.get(1); Flowable> plant2ExportCall = client.exchange( GET(String.format("/programs/%s/experiments/%s/export?all=true&includeTimestamps=false&fileExtension=%s&datasetId=%s", From 3ef17424f4d5a386ef032dda38d0751e584c4244 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Fri, 11 Jul 2025 11:45:54 -0400 Subject: [PATCH 05/13] create interface for obs var validator --- .../DynamicObsUnitValidator.java} | 7 +++-- .../ObservationUnitDuplicateIDValidator.java | 3 +- .../ObservationUnitIDBlankValidator.java | 3 +- .../ObservationUnitIDColumnNameValidator.java | 3 +- .../ObservationUnitIDFormatValidator.java | 3 +- .../ObservationUnitIDValidator.java | 12 ++++---- ...ObservationUnitSingleDatasetValidator.java | 4 +-- .../DynamicObsVarValidator.java | 28 +++++++++++++++++++ ...ervationVariablePriorDatasetValidator.java | 9 +++--- ...ObservationVariableTimestampValidator.java | 5 +--- .../ObservationVariableValidator.java | 14 +++++----- 11 files changed, 56 insertions(+), 35 deletions(-) rename src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/{DynamicColumnValidator.java => observationUnitID/DynamicObsUnitValidator.java} (81%) create mode 100644 src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/DynamicObsVarValidator.java diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/DynamicColumnValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/DynamicObsUnitValidator.java similarity index 81% rename from src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/DynamicColumnValidator.java rename to src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/DynamicObsUnitValidator.java index 909f48b21..166a6fd0e 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/DynamicColumnValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/DynamicObsUnitValidator.java @@ -15,13 +15,14 @@ * limitations under the License. */ -package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns; +package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.observationUnitID; import io.micronaut.core.order.Ordered; +import org.brapi.client.v2.model.exceptions.ApiException; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; import org.breedinginsight.services.exceptions.BadRequestException; @FunctionalInterface -public interface DynamicColumnValidator extends Ordered { - void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException; +public interface DynamicObsUnitValidator extends Ordered { + void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException, ApiException; } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitDuplicateIDValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitDuplicateIDValidator.java index 03ee5838c..54a70cf57 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitDuplicateIDValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitDuplicateIDValidator.java @@ -21,7 +21,6 @@ import org.breedinginsight.api.model.v1.response.ValidationErrors; import org.breedinginsight.brapps.importer.services.processors.experiment.ExperimentUtilities; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; -import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; import org.breedinginsight.services.exceptions.BadRequestException; import tech.tablesaw.columns.Column; @@ -34,7 +33,7 @@ @Slf4j @Singleton -public class ObservationUnitDuplicateIDValidator implements DynamicColumnValidator { +public class ObservationUnitDuplicateIDValidator implements DynamicObsUnitValidator { @Override public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { // Skip this validation if the observation units have already been fetched from the BrAPI service diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDBlankValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDBlankValidator.java index 0dddf51ca..b69234a1c 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDBlankValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDBlankValidator.java @@ -21,7 +21,6 @@ import org.breedinginsight.api.model.v1.response.ValidationErrors; import org.breedinginsight.brapps.importer.services.processors.experiment.ExperimentUtilities; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; -import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; import org.breedinginsight.services.exceptions.BadRequestException; import tech.tablesaw.columns.Column; @@ -32,7 +31,7 @@ @Slf4j @Singleton -public class ObservationUnitIDBlankValidator implements DynamicColumnValidator { +public class ObservationUnitIDBlankValidator implements DynamicObsUnitValidator { @Override public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { // Skip this validation if the observation units have already been fetched from the BrAPI service diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java index e5a257236..49d86340b 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java @@ -20,7 +20,6 @@ import lombok.extern.slf4j.Slf4j; import org.breedinginsight.brapps.importer.services.processors.experiment.ExperimentUtilities; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; -import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; import org.breedinginsight.services.exceptions.BadRequestException; import javax.inject.Singleton; @@ -32,7 +31,7 @@ @Slf4j @Singleton -public class ObservationUnitIDColumnNameValidator implements DynamicColumnValidator { +public class ObservationUnitIDColumnNameValidator implements DynamicObsUnitValidator { public ObservationUnitIDColumnNameValidator() {} diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDFormatValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDFormatValidator.java index 93417b2c6..e35ffd884 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDFormatValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDFormatValidator.java @@ -20,7 +20,6 @@ import lombok.extern.slf4j.Slf4j; import org.breedinginsight.api.model.v1.response.ValidationErrors; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; -import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; import org.breedinginsight.services.exceptions.BadRequestException; import tech.tablesaw.columns.Column; import java.util.regex.Pattern; @@ -33,7 +32,7 @@ @Slf4j @Singleton -public class ObservationUnitIDFormatValidator implements DynamicColumnValidator { +public class ObservationUnitIDFormatValidator implements DynamicObsUnitValidator { private static final Pattern UUID_PATTERN = Pattern.compile( "^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$" ); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDValidator.java index 46e76384a..dc37d7e16 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDValidator.java @@ -18,8 +18,8 @@ package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.observationUnitID; import io.micronaut.context.annotation.Primary; +import org.brapi.client.v2.model.exceptions.ApiException; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; -import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; import org.breedinginsight.services.exceptions.BadRequestException; import javax.inject.Singleton; @@ -27,17 +27,17 @@ @Primary @Singleton -public class ObservationUnitIDValidator implements DynamicColumnValidator { - private final List validators; +public class ObservationUnitIDValidator implements DynamicObsUnitValidator { + private final List validators; - public ObservationUnitIDValidator(List validators) { + public ObservationUnitIDValidator(List validators) { this.validators = validators; } @Override public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) - throws BadRequestException { - for (DynamicColumnValidator validator : validators) { + throws BadRequestException, ApiException { + for (DynamicObsUnitValidator validator : validators) { validator.validateDynamicColumns(ctx); } } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitSingleDatasetValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitSingleDatasetValidator.java index e33667c66..ef01603ed 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitSingleDatasetValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitSingleDatasetValidator.java @@ -24,14 +24,12 @@ import org.breedinginsight.brapps.importer.model.response.PendingImportObject; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; -import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; import org.breedinginsight.services.exceptions.BadRequestException; import org.breedinginsight.utilities.Utilities; import tech.tablesaw.columns.Column; import javax.inject.Singleton; -import java.util.HashMap; import java.util.List; import java.util.Map; @@ -40,7 +38,7 @@ @Slf4j @Singleton -public class ObservationUnitSingleDatasetValidator implements DynamicColumnValidator { +public class ObservationUnitSingleDatasetValidator implements DynamicObsUnitValidator { private final String referenceSourceBase; public ObservationUnitSingleDatasetValidator(@Property(name = "brapi.server.reference-source") String referenceSourceBase) { diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/DynamicObsVarValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/DynamicObsVarValidator.java new file mode 100644 index 000000000..b42cbee7c --- /dev/null +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/DynamicObsVarValidator.java @@ -0,0 +1,28 @@ +/* + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.observationVariable; + +import io.micronaut.core.order.Ordered; +import org.brapi.client.v2.model.exceptions.ApiException; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; +import org.breedinginsight.services.exceptions.BadRequestException; + +@FunctionalInterface +public interface DynamicObsVarValidator extends Ordered { + void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException, ApiException; +} diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java index b1822df20..c7b6ba3da 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java @@ -28,7 +28,6 @@ import org.breedinginsight.brapps.importer.model.response.PendingImportObject; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; -import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; import org.breedinginsight.brapps.importer.services.processors.experiment.service.DatasetService; import org.breedinginsight.services.exceptions.BadRequestException; import org.breedinginsight.utilities.Utilities; @@ -40,17 +39,19 @@ @Slf4j @Singleton -public class ObservationVariablePriorDatasetValidator implements DynamicColumnValidator { +public class ObservationVariablePriorDatasetValidator implements DynamicObsVarValidator { private final String referenceSourceBase; private final DatasetService datasetService; - public ObservationVariablePriorDatasetValidator(@Property(name = "brapi.server.reference-source") String referenceSourceBase, DatasetService datasetService) { + public ObservationVariablePriorDatasetValidator( + @Property(name = "brapi.server.reference-source") String referenceSourceBase, + DatasetService datasetService) { this.referenceSourceBase = referenceSourceBase; this.datasetService = datasetService; } @Override - public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { + public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException, ApiException { // Skip the validation if the dependencies have not been fetched from the BrAPI service if(noMappings(ctx)) return; diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java index 538bf220b..17858065d 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java @@ -17,7 +17,6 @@ package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.observationVariable; -import io.micronaut.http.HttpStatus; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; import org.breedinginsight.api.model.v1.response.ValidationError; @@ -25,10 +24,8 @@ import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; -import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; import org.breedinginsight.brapps.importer.services.processors.experiment.service.ObservationVariableService; import org.breedinginsight.services.exceptions.BadRequestException; -import org.breedinginsight.services.exceptions.UnprocessableEntityException; import tech.tablesaw.api.Table; import tech.tablesaw.columns.Column; @@ -40,7 +37,7 @@ @Slf4j @Singleton -public class ObservationVariableTimestampValidator implements DynamicColumnValidator { +public class ObservationVariableTimestampValidator implements DynamicObsVarValidator { private final ObservationVariableService observationVariableService; public ObservationVariableTimestampValidator(ObservationVariableService observationVariableService) { diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableValidator.java index ea039024b..83cd9cd35 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableValidator.java @@ -18,8 +18,8 @@ package org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.observationVariable; import io.micronaut.context.annotation.Primary; +import org.brapi.client.v2.model.exceptions.ApiException; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteMiddlewareContext; -import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.validator.dynamicColumns.DynamicColumnValidator; import org.breedinginsight.services.exceptions.BadRequestException; import javax.inject.Singleton; @@ -27,17 +27,17 @@ @Primary @Singleton -public class ObservationVariableValidator implements DynamicColumnValidator { - private final List validators; +public class ObservationVariableValidator implements DynamicObsVarValidator { + private final List validators; - public ObservationVariableValidator(List validators) { + public ObservationVariableValidator(List validators) { this.validators = validators; } - @Override - public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws BadRequestException { - for (DynamicColumnValidator validator : validators) { + public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) + throws BadRequestException, ApiException { + for (DynamicObsVarValidator validator : validators) { validator.validateDynamicColumns(ctx); } } From 2302e6da90199f259fa936c9b0beed898c8e3181 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 15 Jul 2025 21:34:55 -0400 Subject: [PATCH 06/13] fix bug in obsvar validator --- .../experiment/ExperimentUtilities.java | 2 +- .../AppendOverwritePhenotypesWorkflow.java | 15 ++++++-- .../factory/entity/PendingDataset.java | 35 +++++++++++-------- .../initialize/WorkflowInitialization.java | 6 ---- .../process/ImportTableProcess.java | 24 +++++++++---- .../ObservationUnitIDColumnNameValidator.java | 4 +-- ...ervationVariablePriorDatasetValidator.java | 14 ++++++-- ...ObservationVariableTimestampValidator.java | 7 ++-- .../model/ExpImportProcessConstants.java | 5 +-- .../experiment/service/DatasetService.java | 4 +-- .../importer/ExperimentFileImportTest.java | 8 ++--- 11 files changed, 78 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java index 46769bb5b..2f22d7e9e 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java @@ -92,7 +92,7 @@ public ExperimentUtilities() { */ public boolean isInvalidMemberListForClass(List list, Class clazz) { // Check if the input list is null, empty, or contains any member that is not an instance of the specified class - return list == null || list.isEmpty() || !list.stream().allMatch(clazz::isInstance); + return (list == null) || !list.stream().allMatch(clazz::isInstance); } /** diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java index fbbc0afbd..7743e08cd 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java @@ -28,6 +28,7 @@ import org.breedinginsight.brapps.importer.services.ImportStatusService; import org.breedinginsight.brapps.importer.services.processors.experiment.ExperimentWorkflowNavigator; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.middleware.AppendOverwriteIDValidation; +import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.middleware.AppendOverwriteVariableValidation; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.middleware.commit.BrAPICommit; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.middleware.initialize.WorkflowInitialization; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.middleware.process.ImportTableProcess; @@ -46,20 +47,24 @@ @Singleton public class AppendOverwritePhenotypesWorkflow implements ExperimentWorkflow { private final ExperimentWorkflowNavigator.Workflow workflow; + private final AppendOverwriteMiddleware validationMiddleware; private final AppendOverwriteMiddleware importPreviewMiddleware; private final AppendOverwriteMiddleware brapiCommitMiddleware; private final ImportStatusService statusService; @Inject public AppendOverwritePhenotypesWorkflow(AppendOverwriteIDValidation expUnitIDValidation, + AppendOverwriteVariableValidation obsVariableValidation, WorkflowInitialization workflowInitialization, ImportTableProcess importTableProcess, BrAPICommit brAPICommit, ImportStatusService statusService){ this.statusService = statusService; this.workflow = ExperimentWorkflowNavigator.Workflow.APPEND_OVERWRITE; + this.validationMiddleware = (AppendOverwriteMiddleware) AppendOverwriteMiddleware.link( + expUnitIDValidation, + obsVariableValidation); this.importPreviewMiddleware = (AppendOverwriteMiddleware) AppendOverwriteMiddleware.link( - expUnitIDValidation, workflowInitialization, importTableProcess); this.brapiCommitMiddleware = (AppendOverwriteMiddleware) AppendOverwriteMiddleware.link(brAPICommit); @@ -115,11 +120,15 @@ public Optional process(ImportServiceContext context) { .appendOverwriteWorkflowContext(new AppendOverwriteWorkflowContext()) .build(); + // Validate the import + AppendOverwriteMiddlewareContext validatedImportContext = this.validationMiddleware.process(workflowContext); + // Process the import preview - AppendOverwriteMiddlewareContext processedPreviewContext = this.importPreviewMiddleware.process(workflowContext); + AppendOverwriteMiddlewareContext processedPreviewContext = this.importPreviewMiddleware.process(validatedImportContext); // Stop and return any errors that occurred while processing - Optional previewException = Optional.ofNullable(processedPreviewContext.getAppendOverwriteWorkflowContext().getProcessError()); + Optional previewException = Optional + .ofNullable(processedPreviewContext.getAppendOverwriteWorkflowContext().getProcessError()); if (previewException.isPresent()) { log.debug(String.format("%s in %s", previewException.get().getException().getClass().getName(), previewException.get().getLocalTransactionName())); result.ifPresent(importWorkflowResult -> importWorkflowResult.setCaughtException(Optional.ofNullable(previewException.get().getException()))); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingDataset.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingDataset.java index 70460a6c7..0a831d56c 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingDataset.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/entity/PendingDataset.java @@ -38,9 +38,7 @@ import org.breedinginsight.utilities.DatasetUtil; import org.breedinginsight.utilities.Utilities; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.stream.Collectors; @Prototype @@ -122,7 +120,10 @@ public List brapiRead() throws ApiException { .get(); // Get the dataset - return List.of(datasetService.fetchDatasetById(datasetId, importContext.getProgram()).orElseThrow(ApiException::new)); + return datasetService + .fetchDatasetById(datasetId, importContext.getProgram()) + .map(List::of) + .orElseGet(List::of); } /** @@ -242,18 +243,22 @@ public void initializeWorkflow(List members) { .collect(Collectors.toMap(pio -> pio.getBrAPIObject().getListName(),pio -> pio)); // Construct a hashmap to look up the pending dataset by the observation unit ID of a unit stored in the BrAPI service - Map> pendingObsDatasetByOUId = cache.getPendingObsUnitByOUId().entrySet().stream() - .collect(Collectors.toMap( - Map.Entry::getKey, - e -> { - if (cache.getPendingTrialByOUId().isEmpty() || - pendingDatasetByName.isEmpty() || - cache.getPendingTrialByOUId().values().iterator().next().getBrAPIObject().getAdditionalInfo().getAsJsonArray(BrAPIAdditionalInfoFields.DATASETS).isEmpty()) { - throw new IllegalStateException("There is not an observation data set for this unit: " + e.getKey()); + Map> pendingObsDatasetByOUId; + if (pendingDatasetByName.isEmpty()) { + pendingObsDatasetByOUId = Collections.emptyMap(); + } else { + pendingObsDatasetByOUId = cache.getPendingObsUnitByOUId().entrySet().stream() + .collect(Collectors.toMap( + Map.Entry::getKey, + e -> { + if (cache.getPendingTrialByOUId().isEmpty() || + cache.getPendingTrialByOUId().values().iterator().next().getBrAPIObject().getAdditionalInfo().getAsJsonArray(BrAPIAdditionalInfoFields.DATASETS).isEmpty()) { + throw new IllegalStateException("There is not an observation data set for this unit: " + e.getKey()); + } + return pendingDatasetByName.values().iterator().next(); } - return pendingDatasetByName.values().iterator().next(); - } - )); + )); + } // Add the maps to the context for use in processing import cache.setObsVarDatasetByName(pendingDatasetByName); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/initialize/WorkflowInitialization.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/initialize/WorkflowInitialization.java index 2aa889fc2..decd495a2 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/initialize/WorkflowInitialization.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/initialize/WorkflowInitialization.java @@ -39,10 +39,8 @@ @Slf4j @Prototype public class WorkflowInitialization extends AppendOverwriteMiddleware { - WorkflowReadInitialization brAPITrialReadWorkflowInitialization; WorkflowReadInitialization brAPIStudyReadWorkflowInitialization; WorkflowReadInitialization locationReadWorkflowInitialization; - WorkflowReadInitialization brAPIDatasetReadWorkflowInitialization; WorkflowReadInitialization brAPIGermplasmReadWorkflowInitialization; BrAPIReadFactory brAPIReadFactory; @@ -52,18 +50,14 @@ public WorkflowInitialization(BrAPIReadFactory brAPIReadFactory) { } @Override public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext context) { - brAPITrialReadWorkflowInitialization = brAPIReadFactory.trialWorkflowReadInitializationBean(context); brAPIStudyReadWorkflowInitialization = brAPIReadFactory.studyWorkflowReadInitializationBean(context); locationReadWorkflowInitialization = brAPIReadFactory.locationWorkflowReadInitializationBean(context); - brAPIDatasetReadWorkflowInitialization = brAPIReadFactory.datasetWorkflowReadInitializationBean(context); brAPIGermplasmReadWorkflowInitialization = brAPIReadFactory.germplasmWorkflowReadInitializationBean(context); log.debug("reading required BrAPI data from BrAPI service"); try { - brAPITrialReadWorkflowInitialization.execute(); brAPIStudyReadWorkflowInitialization.execute(); locationReadWorkflowInitialization.execute(); - brAPIDatasetReadWorkflowInitialization.execute(); brAPIGermplasmReadWorkflowInitialization.execute(); } catch (ApiException e) { context.getAppendOverwriteWorkflowContext().setProcessError(new MiddlewareException(e)); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java index d799b0631..48d4e5a76 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java @@ -68,6 +68,7 @@ import java.util.stream.Collectors; import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.*; +import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.ErrMessage.DATASET_NOT_FOUND; import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.ErrMessage.MULTIPLE_EXP_TITLES; @Slf4j @@ -130,17 +131,28 @@ public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext // Sort the traits to match the order of the headers in the import file List sortedTraits = experimentUtil.sortByField(varNames, new ArrayList<>(traits), TraitEntity::getObservationVariableName); - // Get the pending observation dataset - PendingImportObject pendingTrial = ExperimentUtilities.getSingleEntryValue(context.getAppendOverwriteWorkflowContext().getTrialByNameNoScope()).orElseThrow(()->new UnprocessableEntityException(MULTIPLE_EXP_TITLES.getValue())); - String datasetName = String.format("Observation Dataset [%s-%s]", program.getKey(), pendingTrial.getBrAPIObject().getAdditionalInfo().get(BrAPIAdditionalInfoFields.EXPERIMENT_NUMBER).getAsString()); - PendingImportObject pendingDataset = context.getAppendOverwriteWorkflowContext().getObsVarDatasetByName().get(datasetName); + // Get the pending observation dataset; there should only be a single dataset used for the import + PendingImportObject pendingTrial = ExperimentUtilities + .getSingleEntryValue(context.getAppendOverwriteWorkflowContext().getTrialByNameNoScope()) + .orElseThrow(()->new UnprocessableEntityException(MULTIPLE_EXP_TITLES.getValue())); + PendingImportObject pendingDataset = context + .getAppendOverwriteWorkflowContext() + .getPendingObsDatasetByOUId() + .values() + .stream() + .findAny() + .orElseGet(()-> new PendingImportObject(ImportObjectState.NEW, new BrAPIListDetails())); // Add new phenotypes to the pending observation dataset list (NOTE: "obsVarName [programKey]" is used instead of obsVarDbId) // TODO: Change to using actual dbIds as per the BrAPI spec, instead of namespaced obsVar names (was necessary for Breedbase) - List datasetObsVarDbIds = pendingDataset.getBrAPIObject().getData().stream().collect(Collectors.toList()); + List datasetObsVarDbIds = Optional.ofNullable(pendingDataset.getBrAPIObject().getData()) + .map(ArrayList::new) + .orElseGet(ArrayList::new); List phenoDbIds = sortedTraits.stream().map(t->Utilities.appendProgramKey(t.getObservationVariableName(), program.getKey())).collect(Collectors.toList()); phenoDbIds.removeAll(datasetObsVarDbIds); - pendingDataset.getBrAPIObject().getData().addAll(phenoDbIds); + for (String phenoDbId : phenoDbIds) { + pendingDataset.getBrAPIObject().addDataItem(phenoDbId); + } // Update pending status if (ImportObjectState.EXISTING == pendingDataset.getState()) { diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java index 49d86340b..98a9cb8f1 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationUnitID/ObservationUnitIDColumnNameValidator.java @@ -27,7 +27,7 @@ import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.ErrMessage.OZEX; import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.OBSERVATION_UNIT_ID_SUFFIX; -import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.SUB_UNIT_NUMBER; +import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.SUB_UNIT_ID; @Slf4j @Singleton @@ -57,7 +57,7 @@ public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws if (idColCount == 2) { // if sub-entity ids in import then check for presence of sub-unit # column Arrays.stream(ctx.getImportContext().getUpload().getDynamicColumnNames()) - .filter(name-> name.equals(SUB_UNIT_NUMBER)) + .filter(name-> name.equals(SUB_UNIT_ID)) .findAny() .orElseThrow(()->new BadRequestException(OZEX.getValue())); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java index c7b6ba3da..70ba472d9 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariablePriorDatasetValidator.java @@ -34,6 +34,7 @@ import javax.inject.Singleton; import java.util.*; +import java.util.stream.Collectors; import static org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants.ErrMessage.JABH; @@ -78,10 +79,16 @@ public int getOrder() { private Set getDatasetVariables(AppendOverwriteMiddlewareContext ctx, Set ids) throws ApiException { Set variables = new HashSet<>(); + String progKey = ctx.getImportContext().getProgram().getKey(); List varListDetails = datasetService .fetchDatasetsByIds(ids, ctx.getImportContext().getProgram()).orElse(new ArrayList<>()); for (BrAPIListDetails brAPIListDetails : varListDetails) { - variables.addAll(brAPIListDetails.getData()); + List priorVariablesNoScope = brAPIListDetails + .getData() + .stream() + .map((scopedVariable) -> Utilities.removeProgramKey(scopedVariable, progKey)) + .collect(Collectors.toList()); + variables.addAll(priorVariablesNoScope); } return variables; @@ -139,6 +146,9 @@ private boolean noMappings(AppendOverwriteMiddlewareContext context) { .getAppendOverwriteWorkflowContext() .getPendingObsDatasetByOUId(); - return !(trialByOUId == null || trialByOUId.isEmpty() || datasetByOUId == null || datasetByOUId.isEmpty()); + if (trialByOUId == null) return true; + if (trialByOUId.isEmpty()) return true; + if (datasetByOUId == null) return true; + return false; } } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java index 17858065d..cb9b4ac21 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/validator/dynamicColumns/observationVariable/ObservationVariableTimestampValidator.java @@ -51,9 +51,12 @@ public void validateDynamicColumns(AppendOverwriteMiddlewareContext ctx) throws // Get all the phenotypic columns of the import ImportUpload upload = ctx.getImportContext().getUpload(); Table data = ctx.getImportContext().getData(); - List phenotypeColNames = Arrays.stream(upload.getDynamicColumnNames()) + List phenotypeColNames = upload + .getDynamicColumnNamesList() + .stream() .filter(name -> !name.endsWith(OBSERVATION_UNIT_ID_SUFFIX)) - .filter(name -> !name.contains(SUB_UNIT_NUMBER)) + .filter(name -> !name.contains(SUB_UNIT_ID)) + .filter(name -> !name.contains(SUB_OBS_UNIT)) .collect(Collectors.toList()); // don't allow periods (.) or square brackets in Phenotype Column Names diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java index d508efff0..b8ea2b6af 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/model/ExpImportProcessConstants.java @@ -18,7 +18,6 @@ package org.breedinginsight.brapps.importer.services.processors.experiment.model; import com.fasterxml.jackson.annotation.JsonValue; -import io.micronaut.context.annotation.Property; import lombok.extern.slf4j.Slf4j; @Slf4j @@ -30,7 +29,8 @@ public class ExpImportProcessConstants { public static final String TIMESTAMP_REGEX = "^"+TIMESTAMP_PREFIX+"\\s*"; public static String BRAPI_REFERENCE_SOURCE; public static final String MIDNIGHT = "T00:00:00-00:00"; - public static final String SUB_UNIT_NUMBER = "Sub-Unit #"; + public static final String SUB_UNIT_ID = "Sub Unit ID"; + public static final String SUB_OBS_UNIT = "Sub-Obs Unit"; public enum ErrMessage { MULTIPLE_EXP_TITLES("File contains more than one Experiment Title"), @@ -39,6 +39,7 @@ public enum ErrMessage { UNMATCHED_COLUMN("Ontology term(s) not found: "), OBS_UNIT_NOT_FOUND("Invalid ObsUnitID"), DUPLICATE_OBS_UNIT_ID("ObsUnitId is repeated"), + DATASET_NOT_FOUND("Dataset not found"), OZEX("Missing ObsUnitID column"), VVCN("ObsUnitID is duplicated"), BITB("Invalid or missing ObsUnitID"), diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java index 0cf7d815b..d772466ca 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/DatasetService.java @@ -72,10 +72,8 @@ public Optional fetchDatasetById(String id, Program program) t program.getId(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName()), UUID.fromString(id)); - - // Check if the existing dataset summaries are returned, throw exception if not if (existingDatasets == null || existingDatasets.isEmpty()) { - throw new InternalServerException("Existing dataset summary not returned from BrAPI server"); + return Optional.empty(); } // Retrieve dataset details using the list DB ID from the existing dataset summary diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 2ed39d1dd..06b899020 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -213,9 +213,9 @@ public void appendExperimentWithObsVarFromPriorDataset() { 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(), "1"); - JsonObject importResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), null, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); + JsonObject importResponse = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), traits, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); String expId = importResponse .get("preview").getAsJsonObject() .get("rows").getAsJsonArray() @@ -293,8 +293,8 @@ public void appendExperimentWithObsVarFromPriorDataset() { sub2.put(traits.get(0).getObservationVariableName(), "2"); // Verify that the validation check returns a 400-level response since tt_test_1 is already used in the plot-level dataset - JsonObject previewResponse = importTestUtils.uploadAndFetchWorkflowPreview(importTestUtils.writeExperimentDataToFile(List.of(sub1, sub2), null, true, false, "Plant"), null, true, client, program, mappingId, appendOverwriteWorkflowId); - assertEquals(422, previewResponse.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); + JsonObject previewResponse = importTestUtils.uploadAndFetchWorkflowPreview(importTestUtils.writeExperimentDataToFile(List.of(sub1, sub2), traits, true, true, "Plant"), null, true, client, program, mappingId, appendOverwriteWorkflowId); + assertEquals(400, previewResponse.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); } @Test From f13bbe720b36a7461b0d687b23fb4f82821b3deb Mon Sep 17 00:00:00 2001 From: HMS17 Date: Tue, 2 Sep 2025 11:15:39 -0400 Subject: [PATCH 07/13] [BI-2110] - Unit Test Fix --- .../brapps/importer/ExperimentFileImportTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index ab3823856..0e0fa4af0 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -1460,7 +1460,7 @@ public void importNewObsAfterFirstExpWithObsAndTimestamps() { // In the first upload, only 1 trait should be present. List initialTraits = List.of(traits.get(0)); - importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), initialTraits, false), null, true, client, program, mappingId, newExperimentWorkflowId); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), initialTraits, false, false, null), null, true, client, program, mappingId, newExperimentWorkflowId); 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())); @@ -1491,7 +1491,7 @@ public void importNewObsAfterFirstExpWithObsAndTimestamps() { // Send overwrite parameters in request body to allow the append workflow to work normally. Map userData = Map.of("overwrite", "true", "overwriteReason", "testing"); - JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits, true), userData, true, client, program, mappingId, appendOverwriteWorkflowId); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits, true, false, null), userData, true, client, program, mappingId, appendOverwriteWorkflowId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); From 6f760ae7a02f0d3910d2bb0ddf140de89d28548d Mon Sep 17 00:00:00 2001 From: HMS17 Date: Wed, 10 Sep 2025 19:07:22 -0400 Subject: [PATCH 08/13] [BI-2110] - Validation Fix, Enabled Append when more than one germplasm --- .../experiment/ExperimentUtilities.java | 20 ++++++++++--------- .../steps/CommitPendingImportObjectsStep.java | 3 ++- ...ulateExistingPendingImportObjectsStep.java | 3 ++- .../service/ObservationUnitService.java | 3 ++- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java index 2f22d7e9e..cf6ff0cf2 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java @@ -162,26 +162,28 @@ public static List importRowsToExperimentObservations(Lis * @return a String representing the unique key for the observation unit */ public static String createObservationUnitKey(ExperimentObservation importRow) { - // Extract the environment and experimental unit ID from the ExperimentObservation object + // Extract the environment and experimental unit ID and germplasm GID from the ExperimentObservation object // and pass them to the createObservationUnitKey method - return createObservationUnitKey(importRow.getEnv(), importRow.getExpUnitId()); + return createObservationUnitKey(importRow.getEnv(), importRow.getExpUnitId(), importRow.getGermplasm().getAccessionNumber()); //todo check right one } /** * Create Observation Unit Key * - * This method takes in the name of a study and the name of an observation unit and concatenates them to create a unique key. + * This method takes in the name of a study and the name of an observation unit and the germplasm GID and concatenates them to create a unique key. + * Germplasm GID needed due to how repeated measures are created and named * - * If one or both of the inputs is null, returns an empty string since not a valid combination + * If any of the inputs are null, returns an empty string since not a valid combination * * @param studyName The name of the study * @param obsUnitName The name of the observation unit - * @return A string representing the unique key formed by concatenating the study name and observation unit name + * @param germplasmGID The GID of the germplasm + * @return A string representing the unique key formed by concatenating the study name and observation unit name and germplasm GID */ - public static String createObservationUnitKey(String studyName, String obsUnitName) { - // Concatenate the study name and observation unit name to create the unique key - if (studyName != null && obsUnitName != null) { - return studyName + obsUnitName; + public static String createObservationUnitKey(String studyName, String obsUnitName, String germplasmGID) { + // Concatenate the study name and observation unit name to create the unique key //todo needs to take in more because repeated measures in append + if (studyName != null && obsUnitName != null && germplasmGID != null) { + return studyName + obsUnitName + germplasmGID; } else { return ""; } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java index 5c318d336..0fa6a0146 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java @@ -179,7 +179,8 @@ public void process(ProcessContext processContext, ProcessedData processedData) // retrieve the BrAPI ObservationUnit from this.observationUnitByNameNoScope String createdObservationUnit_StripedStudyName = Utilities.removeProgramKeyAndUnknownAdditionalData(createdObservationUnit.getStudyName(), program.getKey()); String createdObservationUnit_StripedObsUnitName = Utilities.removeProgramKeyAndUnknownAdditionalData(createdObservationUnit.getObservationUnitName(), program.getKey()); - String createdObsUnit_key = ExperimentUtilities.createObservationUnitKey(createdObservationUnit_StripedStudyName, createdObservationUnit_StripedObsUnitName); + String createdObservationUnit_GermplasmGID = createdObservationUnit.getGermplasmDbId(); + String createdObsUnit_key = ExperimentUtilities.createObservationUnitKey(createdObservationUnit_StripedStudyName, createdObservationUnit_StripedObsUnitName, createdObservationUnit_GermplasmGID); observationUnitByNameNoScope.get(createdObsUnit_key) .getBrAPIObject() .setObservationUnitDbId(createdObservationUnit.getObservationUnitDbId()); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java index e611ec46f..7d6b33a3b 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java @@ -513,8 +513,9 @@ private Map fetchExistingObservations(List refe String studyName = studyNameByDbId.get(obs.getStudyDbId()); String variableName = variableNameByDbId.get(obs.getObservationVariableDbId()); String ouName = ouNameByDbId.get(obs.getObservationUnitDbId()); + String germplasmGID = obs.getGermplasmDbId(); - String key = ExperimentUtilities.getObservationHash(ExperimentUtilities.createObservationUnitKey(studyName, ouName), variableName, studyName); + String key = ExperimentUtilities.getObservationHash(ExperimentUtilities.createObservationUnitKey(studyName, ouName, germplasmGID), variableName, studyName); return Map.entry(key, obs); }) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java index d66507558..44f91b5d8 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java @@ -160,7 +160,8 @@ public Map> mapPendingUnitByNa pio.getBrAPIObject().getObservationUnitName(), program.getKey() ); - pendingUnitByNameNoScope.put(ExperimentUtilities.createObservationUnitKey(studyName, observationUnitName), pio); + String germplasmGID = pio.getBrAPIObject().getGermplasmDbId(); + pendingUnitByNameNoScope.put(ExperimentUtilities.createObservationUnitKey(studyName, observationUnitName, germplasmGID), pio); //todo try here dangerous } return pendingUnitByNameNoScope; From 9616246898a8ce66e0414dc60e17a745afbd9370 Mon Sep 17 00:00:00 2001 From: HMS17 Date: Mon, 15 Sep 2025 10:36:37 -0400 Subject: [PATCH 09/13] [BI-2110] - Remove unneeded todos --- .../services/processors/experiment/ExperimentUtilities.java | 4 ++-- .../processors/experiment/service/ObservationUnitService.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java index cf6ff0cf2..7aa306ecd 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java @@ -164,7 +164,7 @@ public static List importRowsToExperimentObservations(Lis public static String createObservationUnitKey(ExperimentObservation importRow) { // Extract the environment and experimental unit ID and germplasm GID from the ExperimentObservation object // and pass them to the createObservationUnitKey method - return createObservationUnitKey(importRow.getEnv(), importRow.getExpUnitId(), importRow.getGermplasm().getAccessionNumber()); //todo check right one + return createObservationUnitKey(importRow.getEnv(), importRow.getExpUnitId(), importRow.getGermplasm().getAccessionNumber()); } /** @@ -181,7 +181,7 @@ public static String createObservationUnitKey(ExperimentObservation importRow) { * @return A string representing the unique key formed by concatenating the study name and observation unit name and germplasm GID */ public static String createObservationUnitKey(String studyName, String obsUnitName, String germplasmGID) { - // Concatenate the study name and observation unit name to create the unique key //todo needs to take in more because repeated measures in append + // Concatenate the study name and observation unit name and germplasm GID to create the unique key if (studyName != null && obsUnitName != null && germplasmGID != null) { return studyName + obsUnitName + germplasmGID; } else { diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java index 44f91b5d8..e4d306e6d 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java @@ -161,7 +161,7 @@ public Map> mapPendingUnitByNa program.getKey() ); String germplasmGID = pio.getBrAPIObject().getGermplasmDbId(); - pendingUnitByNameNoScope.put(ExperimentUtilities.createObservationUnitKey(studyName, observationUnitName, germplasmGID), pio); //todo try here dangerous + pendingUnitByNameNoScope.put(ExperimentUtilities.createObservationUnitKey(studyName, observationUnitName, germplasmGID), pio); } return pendingUnitByNameNoScope; From de0a598e1fd87957cc83418ed0dd67f17dbf989d Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Thu, 6 Nov 2025 11:23:47 -0500 Subject: [PATCH 10/13] [BI-2110] Missing check on validation --- .../AppendOverwritePhenotypesWorkflow.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java index 7743e08cd..67811154b 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java @@ -123,6 +123,14 @@ public Optional process(ImportServiceContext context) { // Validate the import AppendOverwriteMiddlewareContext validatedImportContext = this.validationMiddleware.process(workflowContext); + //Stop and return any validation errors, needs to be done before processing to avoid null pointer exceptions + Optional validationErrorOptional = Optional + .ofNullable(validatedImportContext.getAppendOverwriteWorkflowContext().getValidationErrors()); + if (validationErrorOptional.isPresent() && validationErrorOptional.get().hasErrors()){ + result.ifPresent(importWorkflowResult -> importWorkflowResult.setCaughtException(Optional.of(new ValidatorException(validationErrorOptional.get())))); + return result; + } + // Process the import preview AppendOverwriteMiddlewareContext processedPreviewContext = this.importPreviewMiddleware.process(validatedImportContext); From d63ca33589be16b76323b563d1e0e5c3b5008288 Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Thu, 6 Nov 2025 11:32:05 -0500 Subject: [PATCH 11/13] [BI-2110] Added missing imports --- .../appendoverwrite/AppendOverwritePhenotypesWorkflow.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java index 67811154b..ab1990ed2 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java @@ -19,6 +19,7 @@ import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import org.breedinginsight.api.model.v1.response.ValidationErrors; import org.breedinginsight.brapps.importer.model.imports.ImportServiceContext; import org.breedinginsight.brapps.importer.model.response.ImportPreviewResponse; import org.breedinginsight.brapps.importer.model.response.ImportPreviewStatistics; @@ -37,6 +38,7 @@ import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.AppendOverwriteWorkflowContext; import org.breedinginsight.brapps.importer.services.processors.experiment.appendoverwrite.model.MiddlewareException; import org.breedinginsight.brapps.importer.services.processors.experiment.model.ImportContext; +import org.breedinginsight.services.exceptions.ValidatorException; import javax.inject.Inject; import javax.inject.Singleton; From a4bf18a4fbbd85cafbafe2ffd9f15c83fe0ae40c Mon Sep 17 00:00:00 2001 From: HMS17 Date: Tue, 18 Nov 2025 09:24:48 -0500 Subject: [PATCH 12/13] [BI-2110] - Adding GID to key generation --- .../experiment/ExperimentUtilities.java | 19 ++++++++++--------- .../steps/CommitPendingImportObjectsStep.java | 4 ++-- ...ulateExistingPendingImportObjectsStep.java | 4 ++-- .../service/ObservationUnitService.java | 3 ++- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java index 7aa306ecd..1cdfcfdaf 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java @@ -156,34 +156,35 @@ public static List importRowsToExperimentObservations(Lis } /** - * This method generates a unique key for an observation unit based on the environment and experimental unit ID. + * This method generates a unique key for an observation unit based on the environment and experimental unit ID and germplasm GID. * - * @param importRow the ExperimentObservation object containing the environment and experimental unit ID + * @param importRow the ExperimentObservation object containing the environment and experimental unit ID and germplasm GID * @return a String representing the unique key for the observation unit */ public static String createObservationUnitKey(ExperimentObservation importRow) { // Extract the environment and experimental unit ID and germplasm GID from the ExperimentObservation object // and pass them to the createObservationUnitKey method - return createObservationUnitKey(importRow.getEnv(), importRow.getExpUnitId(), importRow.getGermplasm().getAccessionNumber()); + return createObservationUnitKey(importRow.getEnv(), importRow.getExpUnitId(), importRow.getGid()); } /** * Create Observation Unit Key * - * This method takes in the name of a study and the name of an observation unit and the germplasm GID and concatenates them to create a unique key. - * Germplasm GID needed due to how repeated measures are created and named + * This method takes in the name of a study and the name of an observation unit and the germplasm name and concatenates them to create a unique key. + * Sub-observation unit name needed when repeated measures due to how they are created and named * * If any of the inputs are null, returns an empty string since not a valid combination * * @param studyName The name of the study * @param obsUnitName The name of the observation unit - * @param germplasmGID The GID of the germplasm - * @return A string representing the unique key formed by concatenating the study name and observation unit name and germplasm GID + * @param germplasmGID The germplasm gid + * @return A string representing the unique key formed by concatenating the study name and observation unit name and germplasm gid */ public static String createObservationUnitKey(String studyName, String obsUnitName, String germplasmGID) { - // Concatenate the study name and observation unit name and germplasm GID to create the unique key + // Concatenate the study name and observation unit name and germplasm gid to create the unique key if (studyName != null && obsUnitName != null && germplasmGID != null) { - return studyName + obsUnitName + germplasmGID; + String keyDelim = "@*"; + return studyName + keyDelim + obsUnitName + keyDelim + germplasmGID; } else { return ""; } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java index 0fa6a0146..4e0663066 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/CommitPendingImportObjectsStep.java @@ -179,8 +179,8 @@ public void process(ProcessContext processContext, ProcessedData processedData) // retrieve the BrAPI ObservationUnit from this.observationUnitByNameNoScope String createdObservationUnit_StripedStudyName = Utilities.removeProgramKeyAndUnknownAdditionalData(createdObservationUnit.getStudyName(), program.getKey()); String createdObservationUnit_StripedObsUnitName = Utilities.removeProgramKeyAndUnknownAdditionalData(createdObservationUnit.getObservationUnitName(), program.getKey()); - String createdObservationUnit_GermplasmGID = createdObservationUnit.getGermplasmDbId(); - String createdObsUnit_key = ExperimentUtilities.createObservationUnitKey(createdObservationUnit_StripedStudyName, createdObservationUnit_StripedObsUnitName, createdObservationUnit_GermplasmGID); + String createdObservationUnit_GID = createdObservationUnit.getAdditionalInfo().get("gid").getAsString(); + String createdObsUnit_key = ExperimentUtilities.createObservationUnitKey(createdObservationUnit_StripedStudyName, createdObservationUnit_StripedObsUnitName, createdObservationUnit_GID); observationUnitByNameNoScope.get(createdObsUnit_key) .getBrAPIObject() .setObservationUnitDbId(createdObservationUnit.getObservationUnitDbId()); diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java index 7d6b33a3b..11f83bbec 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateExistingPendingImportObjectsStep.java @@ -513,9 +513,9 @@ private Map fetchExistingObservations(List refe String studyName = studyNameByDbId.get(obs.getStudyDbId()); String variableName = variableNameByDbId.get(obs.getObservationVariableDbId()); String ouName = ouNameByDbId.get(obs.getObservationUnitDbId()); - String germplasmGID = obs.getGermplasmDbId(); + String germplasmGID = obs.getAdditionalInfo().get("gid").toString(); - String key = ExperimentUtilities.getObservationHash(ExperimentUtilities.createObservationUnitKey(studyName, ouName, germplasmGID), variableName, studyName); + String key = ExperimentUtilities.getObservationHash(ExperimentUtilities.createObservationUnitKey(studyName, ouName, germplasmGID), variableName, studyName); return Map.entry(key, obs); }) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java index e4d306e6d..3c387f061 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationUnitService.java @@ -160,7 +160,8 @@ public Map> mapPendingUnitByNa pio.getBrAPIObject().getObservationUnitName(), program.getKey() ); - String germplasmGID = pio.getBrAPIObject().getGermplasmDbId(); + String germplasmGID = pio.getBrAPIObject().getAdditionalInfo().get("gid").getAsString(); + pendingUnitByNameNoScope.put(ExperimentUtilities.createObservationUnitKey(studyName, observationUnitName, germplasmGID), pio); } From adce131e87df572de6c3f732ed89bdd34bd7fe68 Mon Sep 17 00:00:00 2001 From: HMS17 Date: Wed, 19 Nov 2025 10:34:24 -0500 Subject: [PATCH 13/13] [BI-2110] - Changed validation handling to avoid nullpointerexception --- .../AppendOverwritePhenotypesWorkflow.java | 52 +++++++++++++------ 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java index ab1990ed2..1f815c80d 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/AppendOverwritePhenotypesWorkflow.java @@ -19,6 +19,8 @@ import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; import org.breedinginsight.api.model.v1.response.ValidationErrors; import org.breedinginsight.brapps.importer.model.imports.ImportServiceContext; import org.breedinginsight.brapps.importer.model.response.ImportPreviewResponse; @@ -72,6 +74,33 @@ public AppendOverwritePhenotypesWorkflow(AppendOverwriteIDValidation expUnitIDVa this.brapiCommitMiddleware = (AppendOverwriteMiddleware) AppendOverwriteMiddleware.link(brAPICommit); } + /** + * Determines if any validation or process errors are present in the context and handles result accordingly + * + * @param context The import service context containing upload, data, program, user, commit flag, and workflow information. + * @param result the ImportWorkflowResult to be modified in response to any errors found in context + * @return Pair containing a Boolean indicating whether any errors were found, and the potentially modified ImportWorkflowResult + */ + public Pair> checkForExistingErrors(AppendOverwriteMiddlewareContext context, Optional result){ + // Stop and return any validation errors + Optional validationErrorOptional = Optional + .ofNullable(context.getAppendOverwriteWorkflowContext().getValidationErrors()); + if (validationErrorOptional.isPresent() && validationErrorOptional.get().hasErrors()){ + result.ifPresent(importWorkflowResult -> importWorkflowResult.setCaughtException(Optional.of(new ValidatorException(validationErrorOptional.get())))); + return new ImmutablePair<>(true, result); + } + + // Stop and return any errors that occurred while processing + Optional previewException = Optional + .ofNullable(context.getAppendOverwriteWorkflowContext().getProcessError()); + if (previewException.isPresent()) { + log.debug(String.format("%s in %s", previewException.get().getException().getClass().getName(), previewException.get().getLocalTransactionName())); + result.ifPresent(importWorkflowResult -> importWorkflowResult.setCaughtException(Optional.ofNullable(previewException.get().getException()))); + return new ImmutablePair<>(true, result); + } + return new ImmutablePair<>(false, result); + } + /** * Processes the import workflow based on the provided import service context. * If the provided context is not valid or if the workflow is not equal to the context workflow, returns an empty Optional. @@ -108,6 +137,8 @@ public Optional process(ImportServiceContext context) { return result; } + Pair> errors; + // Build the workflow context for processing the import ImportContext importContext = ImportContext.builder() .upload(context.getUpload()) @@ -125,25 +156,16 @@ public Optional process(ImportServiceContext context) { // Validate the import AppendOverwriteMiddlewareContext validatedImportContext = this.validationMiddleware.process(workflowContext); - //Stop and return any validation errors, needs to be done before processing to avoid null pointer exceptions - Optional validationErrorOptional = Optional - .ofNullable(validatedImportContext.getAppendOverwriteWorkflowContext().getValidationErrors()); - if (validationErrorOptional.isPresent() && validationErrorOptional.get().hasErrors()){ - result.ifPresent(importWorkflowResult -> importWorkflowResult.setCaughtException(Optional.of(new ValidatorException(validationErrorOptional.get())))); - return result; - } + //Stop and return any validation or process errors, needs to be done before processing import preview to avoid null pointer exceptions + errors = checkForExistingErrors(validatedImportContext, result); + if (errors.getLeft()) return errors.getRight(); // Process the import preview AppendOverwriteMiddlewareContext processedPreviewContext = this.importPreviewMiddleware.process(validatedImportContext); - // Stop and return any errors that occurred while processing - Optional previewException = Optional - .ofNullable(processedPreviewContext.getAppendOverwriteWorkflowContext().getProcessError()); - if (previewException.isPresent()) { - log.debug(String.format("%s in %s", previewException.get().getException().getClass().getName(), previewException.get().getLocalTransactionName())); - result.ifPresent(importWorkflowResult -> importWorkflowResult.setCaughtException(Optional.ofNullable(previewException.get().getException()))); - return result; - } + // Stop and return any validation or process errors + errors = checkForExistingErrors(validatedImportContext, result); + if (errors.getLeft()) return errors.getRight(); // Build and return the preview response ImportPreviewResponse response = new ImportPreviewResponse();