From e87e65ff30d84e18005fa864c73e069c8eae32cf Mon Sep 17 00:00:00 2001 From: timparsons Date: Wed, 3 May 2023 19:48:33 -0400 Subject: [PATCH] [BI-1778] Adding validations for matching location and year for all rows with same environment Also updated unit tests to cover this, and expanded unit test support to test for import preview in addition to import commit --- pom.xml | 5 + .../processors/ExperimentProcessor.java | 102 +++++++-- .../breedinginsight/utilities/Utilities.java | 8 +- .../importer/ExperimentFileImportTest.java | 210 ++++++++++++++---- 4 files changed, 255 insertions(+), 70 deletions(-) diff --git a/pom.xml b/pom.xml index 79a4d83d3..53c76ed6e 100644 --- a/pom.xml +++ b/pom.xml @@ -289,6 +289,11 @@ junit-jupiter-engine test + + org.junit.jupiter + junit-jupiter-params + test + io.micronaut.test micronaut-test-junit5 diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index c18bbc3b3..d85b80329 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -91,6 +91,8 @@ public class ExperimentProcessor implements Processor { private static final String BLANK_FIELD_EXPERIMENT = "Field is blank when creating a new experiment"; private static final String BLANK_FIELD_ENV = "Field is blank when creating a new environment"; private static final String BLANK_FIELD_OBS = "Field is blank when creating new observations"; + private static final String ENV_LOCATION_MISMATCH = "All locations must be the same for a given environment"; + private static final String ENV_YEAR_MISMATCH = "All years must be the same for a given environment"; @Property(name = "brapi.server.reference-source") private String BRAPI_REFERENCE_SOURCE; @@ -228,7 +230,7 @@ public Map process( prepareDataForValidation(importRows, phenotypeCols, mappedBrAPIImport); - validateFields(importRows, validationErrors, mappedBrAPIImport, referencedTraits, program, phenotypeCols); + validateFields(importRows, validationErrors, mappedBrAPIImport, referencedTraits, program, phenotypeCols, commit); if (validationErrors.hasErrors()) { throw new ValidatorException(validationErrors); @@ -540,7 +542,7 @@ private String getObservationHash(String observationUnitName, String variableNam } private ValidationErrors validateFields(List importRows, ValidationErrors validationErrors, Map mappedBrAPIImport, List referencedTraits, Program program, - List> phenotypeCols) throws MissingRequiredInfoException, ApiException { + List> phenotypeCols, boolean commit) throws MissingRequiredInfoException, ApiException { //fetching any existing observations for any OUs in the import Map existingObsByObsHash = fetchExistingObservations(referencedTraits, program); Map colVarMap = referencedTraits.stream().collect(Collectors.toMap(Trait::getObservationVariableName, Function.identity())); @@ -560,7 +562,7 @@ private ValidationErrors validateFields(List importRows, Validation throw new MissingRequiredInfoException(MISSING_OBS_UNIT_ID_ERROR); } - validateConditionallyRequired(validationErrors, rowNum, importRow); + validateConditionallyRequired(validationErrors, rowNum, importRow, program, commit); validateObservationUnits(validationErrors, uniqueStudyAndObsUnit, rowNum, importRow); validateObservations(validationErrors, rowNum, importRow, phenotypeCols, colVarMap, existingObsByObsHash); } @@ -659,7 +661,7 @@ private void validateUniqueObsUnits( } } - private void validateConditionallyRequired(ValidationErrors validationErrors, int rowNum, ExperimentObservation importRow) { + private void validateConditionallyRequired(ValidationErrors validationErrors, int rowNum, ExperimentObservation importRow, Program program, boolean commit) { ImportObjectState expState = this.trialByNameNoScope.get(importRow.getExpTitle()) .getState(); ImportObjectState envState = this.studyByNameNoScope.get(importRow.getEnv()).getState(); @@ -677,8 +679,21 @@ private void validateConditionallyRequired(ValidationErrors validationErrors, in validateRequiredCell(importRow.getExpUnit(), Columns.EXP_UNIT, errorMessage, validationErrors, rowNum); validateRequiredCell(importRow.getExpType(), Columns.EXP_TYPE, errorMessage, validationErrors, rowNum); validateRequiredCell(importRow.getEnv(), Columns.ENV, errorMessage, validationErrors, rowNum); - validateRequiredCell(importRow.getEnvLocation(), Columns.ENV_LOCATION, errorMessage, validationErrors, rowNum); - validateRequiredCell(importRow.getEnvYear(), Columns.ENV_YEAR, errorMessage, validationErrors, rowNum); + if(validateRequiredCell(importRow.getEnvLocation(), Columns.ENV_LOCATION, errorMessage, validationErrors, rowNum)) { + if(!Utilities.removeProgramKeyAndUnknownAdditionalData(this.studyByNameNoScope.get(importRow.getEnv()).getBrAPIObject().getLocationName(), program.getKey()).equals(importRow.getEnvLocation())) { + addRowError(Columns.ENV_LOCATION, ENV_LOCATION_MISMATCH, validationErrors, rowNum); + } + } + if(validateRequiredCell(importRow.getEnvYear(), Columns.ENV_YEAR, errorMessage, validationErrors, rowNum)) { + String studyYear = this.studyByNameNoScope.get(importRow.getEnv()).getBrAPIObject().getSeasons().get(0); + String rowYear = importRow.getEnvYear(); + if(commit) { + rowYear = this.yearToSeasonDbId(importRow.getEnvYear(), program.getId()); + } + if(!studyYear.equals(rowYear)) { + addRowError(Columns.ENV_YEAR, ENV_YEAR_MISMATCH, validationErrors, rowNum); + } + } validateRequiredCell(importRow.getExpUnitId(), Columns.EXP_UNIT_ID, errorMessage, validationErrors, rowNum); validateRequiredCell(importRow.getExpReplicateNo(), Columns.REP_NUM, errorMessage, validationErrors, rowNum); validateRequiredCell(importRow.getExpBlockNo(), Columns.BLOCK_NUM, errorMessage, validationErrors, rowNum); @@ -691,10 +706,12 @@ private void validateConditionallyRequired(ValidationErrors validationErrors, in } } - private void validateRequiredCell(String value, String columnHeader, String errorMessage, ValidationErrors validationErrors, int rowNum) { + private boolean validateRequiredCell(String value, String columnHeader, String errorMessage, ValidationErrors validationErrors, int rowNum) { if (StringUtils.isBlank(value)) { addRowError(columnHeader, errorMessage, validationErrors, rowNum); + return false; } + return true; } private void addRowError(String field, String errorMessage, ValidationErrors validationErrors, int rowNum) { @@ -1091,15 +1108,20 @@ private Map> initializeObserva String refSource = String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName()); if (existingObsUnits.size() == rowByObsUnitId.size()) { existingObsUnits.forEach(brAPIObservationUnit -> { + processAndCacheObservationUnit(brAPIObservationUnit, refSource, program, ret, rowByObsUnitId); + BrAPIExternalReference idRef = Utilities.getExternalReference(brAPIObservationUnit.getExternalReferences(), refSource) .orElseThrow(() -> new InternalServerException("An ObservationUnit ID was not found in any of the external references")); + ExperimentObservation row = rowByObsUnitId.get(idRef.getReferenceID()); - row.setExpUnitId(Utilities.removeProgramKeyAndUnknownAdditionalData(brAPIObservationUnit.getObservationUnitName(), program.getKey())); - ret.put(createObservationUnitKey(row), - new PendingImportObject<>(ImportObjectState.EXISTING, - brAPIObservationUnit, - UUID.fromString(idRef.getReferenceID()))); + row.setExpTitle(Utilities.removeProgramKey(brAPIObservationUnit.getTrialName(), program.getKey())); + row.setEnv(Utilities.removeProgramKeyAndUnknownAdditionalData(brAPIObservationUnit.getStudyName(), program.getKey())); + row.setEnvLocation(Utilities.removeProgramKey(brAPIObservationUnit.getLocationName(), program.getKey())); }); + } else { + List missingIds = new ArrayList<>(rowByObsUnitId.keySet()); + missingIds.removeAll(existingObsUnits.stream().map(BrAPIObservationUnit::getObservationUnitDbId).collect(Collectors.toList())); + throw new IllegalStateException("Observation Units not found for ObsUnitId(s): " + String.join(COMMA_DELIMITER, missingIds)); } return ret; @@ -1138,6 +1160,13 @@ private Map> initializeStudyByNameNoScop return studyByName; } + try { + initializeStudiesForExistingObservationUnits(program, studyByName); + } catch (ApiException e) { + log.error("Error fetching studies: " + Utilities.generateApiExceptionLogMessage(e), e); + throw new InternalServerException(e.toString(), e); + } + List existingStudies; Optional> trial = getTrialPIO(experimentImportRows); @@ -1156,6 +1185,29 @@ private Map> initializeStudyByNameNoScop return studyByName; } + private void initializeStudiesForExistingObservationUnits(Program program, Map> studyByName) throws ApiException { + Set studyDbIds = this.observationUnitByNameNoScope.values() + .stream() + .map(pio -> pio.getBrAPIObject() + .getStudyDbId()) + .collect(Collectors.toSet()); + + List studies = fetchStudiesByDbId(studyDbIds, program); + studies.forEach(study -> { + processAndCacheStudy(study, program, studyByName); + }); + } + + private List fetchStudiesByDbId(Set studyDbIds, Program program) throws ApiException { + List studies = brAPIStudyDAO.getStudiesByStudyDbId(studyDbIds, program); + if(studies.size() != studyDbIds.size()) { + List missingIds = new ArrayList<>(studyDbIds); + missingIds.removeAll(studies.stream().map(BrAPIStudy::getStudyDbId).collect(Collectors.toList())); + throw new IllegalStateException("Study not found for studyDbId(s): " + String.join(COMMA_DELIMITER, missingIds)); + } + return studies; + } + private Map> initializeUniqueLocationNames(Program program, List experimentImportRows) { Map> locationByName = new HashMap<>(); @@ -1279,17 +1331,24 @@ private Map> initializeExistingGermp return existingGermplasmByGID; } - private void processAndCacheStudy(BrAPIStudy existingStudy, Program program, Map> studyByName) { - //Swap season DbId with year String - String seasonId = existingStudy.getSeasons().get(0); - existingStudy.setSeasons(List.of(this.seasonDbIdToYear(seasonId, program.getId()))); + private void processAndCacheObservationUnit(BrAPIObservationUnit brAPIObservationUnit, String refSource, Program program, Map> observationUnitByName, + Map rowByObsUnitId) { + BrAPIExternalReference idRef = Utilities.getExternalReference(brAPIObservationUnit.getExternalReferences(), refSource) + .orElseThrow(() -> new InternalServerException("An ObservationUnit ID was not found in any of the external references")); - existingStudy.setStudyName(Utilities.removeProgramKeyAndUnknownAdditionalData(existingStudy.getStudyName(), program.getKey())); + ExperimentObservation row = rowByObsUnitId.get(idRef.getReferenceID()); + row.setExpUnitId(Utilities.removeProgramKeyAndUnknownAdditionalData(brAPIObservationUnit.getObservationUnitName(), program.getKey())); + observationUnitByName.put(createObservationUnitKey(row), + new PendingImportObject<>(ImportObjectState.EXISTING, + brAPIObservationUnit, + UUID.fromString(idRef.getReferenceID()))); + } + private void processAndCacheStudy(BrAPIStudy existingStudy, Program program, Map> studyByName) { BrAPIExternalReference xref = Utilities.getExternalReference(existingStudy.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.STUDIES.getName())) .orElseThrow(() -> new IllegalStateException("External references wasn't found for study (dbid): " + existingStudy.getStudyDbId())); studyByName.put( - existingStudy.getStudyName(), + Utilities.removeProgramKeyAndUnknownAdditionalData(existingStudy.getStudyName(), program.getKey()), new PendingImportObject<>(ImportObjectState.EXISTING, existingStudy, UUID.fromString(xref.getReferenceID()))); } @@ -1341,12 +1400,7 @@ private void initializeTrialsForExistingObservationUnits(Program program, Map fetchTrialDbidsForStudies(Set studyDbIds, Program program) throws ApiException { Set trialDbIds = new HashSet<>(); - List studies = brAPIStudyDAO.getStudiesByStudyDbId(studyDbIds, program); - if(studies.size() != studyDbIds.size()) { - List missingIds = new ArrayList<>(trialDbIds); - missingIds.removeAll(studies.stream().map(BrAPIStudy::getStudyDbId).collect(Collectors.toList())); - throw new IllegalStateException("Study not found for studyDbId(s): " + String.join(COMMA_DELIMITER, missingIds)); - } + List studies = fetchStudiesByDbId(studyDbIds, program); studies.forEach(study -> { if (StringUtils.isBlank(study.getTrialDbId())) { throw new IllegalStateException("TrialDbId is not set for an existing Study: " + study.getStudyDbId()); diff --git a/src/main/java/org/breedinginsight/utilities/Utilities.java b/src/main/java/org/breedinginsight/utilities/Utilities.java index 5384a8241..59a4be0e9 100644 --- a/src/main/java/org/breedinginsight/utilities/Utilities.java +++ b/src/main/java/org/breedinginsight/utilities/Utilities.java @@ -80,13 +80,13 @@ public static String appendProgramKey( String original, String programKey ){ * @return */ public static String removeProgramKey(String original, String programKey, String additionalKeyData) { + String keyValue; if(StringUtils.isNotBlank(additionalKeyData)) { - String keyValue = String.format(" [%s-%s]", programKey, additionalKeyData); - return original.replace(keyValue, ""); + keyValue = String.format(" [%s-%s]", programKey, additionalKeyData); } else { - String keyValue = String.format(" [%s]", programKey); - return original.replace(keyValue, ""); + keyValue = String.format(" [%s]", programKey); } + return original.replace(keyValue, ""); } /** diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 6cd02e483..d63a0a515 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -68,7 +68,10 @@ import org.breedinginsight.utilities.Utilities; import org.jooq.DSLContext; import org.junit.jupiter.api.*; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.junit.platform.commons.util.StringUtils; +import org.opentest4j.AssertionFailedError; import javax.inject.Inject; import java.io.ByteArrayOutputStream; @@ -77,6 +80,8 @@ import java.io.IOException; import java.time.OffsetDateTime; import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; @@ -329,10 +334,11 @@ public void importNewEnvExistingExpNoObsSuccess() { assertRowSaved(newEnv, program, null); } - @Test + @ParameterizedTest + @ValueSource(booleans = {true, false}) @SneakyThrows - public void verifyMissingDataThrowsError() { - Program program = createProgram("Missing Req Cols", "MISS", "MISS", BRAPI_REFERENCE_SOURCE, createGermplasm(1), null); + public void verifyMissingDataThrowsError(boolean commit) { + Program program = createProgram("Missing Req Cols "+(commit ? "C" : "P"), "MISS"+(commit ? "C" : "P"), "MISS"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(1), null); Map base = new HashMap<>(); base.put(Columns.GERMPLASM_GID, "1"); @@ -349,43 +355,43 @@ public void verifyMissingDataThrowsError() { Map noGID = new HashMap<>(base); noGID.remove(Columns.GERMPLASM_GID); - uploadAndVerifyFailure(program, writeDataToFile(List.of(noGID), null), Columns.GERMPLASM_GID); + uploadAndVerifyFailure(program, writeDataToFile(List.of(noGID), null), Columns.GERMPLASM_GID, commit); Map noExpTitle = new HashMap<>(base); noExpTitle.remove(Columns.EXP_TITLE); - uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpTitle), null), Columns.EXP_TITLE); + uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpTitle), null), Columns.EXP_TITLE, commit); Map noExpUnit = new HashMap<>(base); noExpUnit.remove(Columns.EXP_UNIT); - uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpUnit), null), Columns.EXP_UNIT); + uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpUnit), null), Columns.EXP_UNIT, commit); Map noExpType = new HashMap<>(base); noExpType.remove(Columns.EXP_TYPE); - uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpType), null), Columns.EXP_TYPE); + uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpType), null), Columns.EXP_TYPE, commit); Map noEnv = new HashMap<>(base); noEnv.remove(Columns.ENV); - uploadAndVerifyFailure(program, writeDataToFile(List.of(noEnv), null), Columns.ENV); + uploadAndVerifyFailure(program, writeDataToFile(List.of(noEnv), null), Columns.ENV, commit); Map noEnvLoc = new HashMap<>(base); noEnvLoc.remove(Columns.ENV_LOCATION); - uploadAndVerifyFailure(program, writeDataToFile(List.of(noEnvLoc), null), Columns.ENV_LOCATION); + uploadAndVerifyFailure(program, writeDataToFile(List.of(noEnvLoc), null), Columns.ENV_LOCATION, commit); Map noExpUnitId = new HashMap<>(base); noExpUnitId.remove(Columns.EXP_UNIT_ID); - uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpUnitId), null), Columns.EXP_UNIT_ID); + uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpUnitId), null), Columns.EXP_UNIT_ID, commit); Map noExpRep = new HashMap<>(base); noExpRep.remove(Columns.REP_NUM); - uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpRep), null), Columns.REP_NUM); + uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpRep), null), Columns.REP_NUM, commit); Map noExpBlock = new HashMap<>(base); noExpBlock.remove(Columns.BLOCK_NUM); - uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpBlock), null), Columns.BLOCK_NUM); + uploadAndVerifyFailure(program, writeDataToFile(List.of(noExpBlock), null), Columns.BLOCK_NUM, commit); Map noEnvYear = new HashMap<>(base); noEnvYear.remove(Columns.ENV_YEAR); - uploadAndVerifyFailure(program, writeDataToFile(List.of(noEnvYear), null), Columns.ENV_YEAR); + uploadAndVerifyFailure(program, writeDataToFile(List.of(noEnvYear), null), Columns.ENV_YEAR, commit); } @Test @@ -424,11 +430,88 @@ public void importNewExpWithObsVar() { assertRowSaved(newExp, program, traits); } - @Test + @ParameterizedTest + @ValueSource(booleans = {true, false}) + @SneakyThrows + public void verifyDiffYearSameEnvThrowsError(boolean commit) { + Program program = createProgram("Diff Years "+(commit ? "C" : "P"), "YEARS"+(commit ? "C" : "P"), "YEARS"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(2), null); + + List> rows = new ArrayList<>(); + Map row = new HashMap<>(); + row.put(Columns.GERMPLASM_GID, "1"); + row.put(Columns.TEST_CHECK, "T"); + row.put(Columns.EXP_TITLE, "Different Years"); + row.put(Columns.EXP_UNIT, "Plot"); + row.put(Columns.EXP_TYPE, "Phenotyping"); + row.put(Columns.ENV, "Diff Year"); + row.put(Columns.ENV_LOCATION, "Location A"); + row.put(Columns.ENV_YEAR, "2023"); + row.put(Columns.EXP_UNIT_ID, "a-1"); + row.put(Columns.REP_NUM, "1"); + row.put(Columns.BLOCK_NUM, "1"); + rows.add(row); + + row = new HashMap<>(); + row.put(Columns.GERMPLASM_GID, "2"); + row.put(Columns.TEST_CHECK, "T"); + row.put(Columns.EXP_TITLE, "Different Years"); + row.put(Columns.EXP_UNIT, "Plot"); + row.put(Columns.EXP_TYPE, "Phenotyping"); + row.put(Columns.ENV, "Diff Year"); + row.put(Columns.ENV_LOCATION, "Location A"); + row.put(Columns.ENV_YEAR, "2022"); + row.put(Columns.EXP_UNIT_ID, "a-2"); + row.put(Columns.REP_NUM, "1"); + row.put(Columns.BLOCK_NUM, "2"); + rows.add(row); + + uploadAndVerifyFailure(program, writeDataToFile(rows, null), Columns.ENV_YEAR, commit); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + @SneakyThrows + public void verifyDiffLocSameEnvThrowsError(boolean commit) { + Program program = createProgram("Diff Locations "+(commit ? "C" : "P"), "LOCS"+(commit ? "C" : "P"), "LOCS"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(2), null); + + List> rows = new ArrayList<>(); + Map row = new HashMap<>(); + row.put(Columns.GERMPLASM_GID, "1"); + row.put(Columns.TEST_CHECK, "T"); + row.put(Columns.EXP_TITLE, "Different Years"); + row.put(Columns.EXP_UNIT, "Plot"); + row.put(Columns.EXP_TYPE, "Phenotyping"); + row.put(Columns.ENV, "Diff Year"); + row.put(Columns.ENV_LOCATION, "Location A"); + row.put(Columns.ENV_YEAR, "2023"); + row.put(Columns.EXP_UNIT_ID, "a-1"); + row.put(Columns.REP_NUM, "1"); + row.put(Columns.BLOCK_NUM, "1"); + rows.add(row); + + row = new HashMap<>(); + row.put(Columns.GERMPLASM_GID, "2"); + row.put(Columns.TEST_CHECK, "T"); + row.put(Columns.EXP_TITLE, "Different Years"); + row.put(Columns.EXP_UNIT, "Plot"); + row.put(Columns.EXP_TYPE, "Phenotyping"); + row.put(Columns.ENV, "Diff Year"); + row.put(Columns.ENV_LOCATION, "Location B"); + row.put(Columns.ENV_YEAR, "2023"); + row.put(Columns.EXP_UNIT_ID, "a-2"); + row.put(Columns.REP_NUM, "1"); + row.put(Columns.BLOCK_NUM, "2"); + rows.add(row); + + uploadAndVerifyFailure(program, writeDataToFile(rows, null), Columns.ENV_LOCATION, commit); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) @SneakyThrows - public void importNewExpWithObs() { + public void importNewExpWithObs(boolean commit) { List traits = createTraits(1); - Program program = createProgram("New Exp with Observations", "EXPOBS", "EXPOBS", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Program program = createProgram("New Exp with Observations "+(commit ? "C" : "P"), "NEXOB"+(commit ? "C" : "P"), "NEXOB"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); Map newExp = new HashMap<>(); newExp.put(Columns.GERMPLASM_GID, "1"); newExp.put(Columns.TEST_CHECK, "T"); @@ -445,7 +528,7 @@ public void importNewExpWithObs() { newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), "1"); - JsonObject result = importTestUtils.uploadAndFetch(writeDataToFile(List.of(newExp), traits), null, true, client, program, mappingId); + JsonObject result = importTestUtils.uploadAndFetch(writeDataToFile(List.of(newExp), traits), null, commit, client, program, mappingId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -455,14 +538,20 @@ public void importNewExpWithObs() { assertEquals("NEW", row.getAsJsonObject("location").get("state").getAsString()); assertEquals("NEW", row.getAsJsonObject("study").get("state").getAsString()); assertEquals("NEW", row.getAsJsonObject("observationUnit").get("state").getAsString()); - assertRowSaved(newExp, program, traits); + + if(commit) { + assertRowSaved(newExp, program, traits); + } else { + assertValidPreviewRow(newExp, row, program, traits); + } } - @Test + @ParameterizedTest + @ValueSource(booleans = {true, false}) @SneakyThrows - public void verifyFailureImportNewExpWithInvalidObs() { + public void verifyFailureImportNewExpWithInvalidObs(boolean commit) { List traits = createTraits(1); - Program program = createProgram("Invalid Observations", "INVOBS", "INVOBS", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Program program = createProgram("Invalid Observations "+(commit ? "C" : "P"), "INVOB"+(commit ? "C" : "P"), "INVOB"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); Map newExp = new HashMap<>(); newExp.put(Columns.GERMPLASM_GID, "1"); newExp.put(Columns.TEST_CHECK, "T"); @@ -479,13 +568,14 @@ public void verifyFailureImportNewExpWithInvalidObs() { newExp.put(Columns.COLUMN, "1"); newExp.put(traits.get(0).getObservationVariableName(), "Red"); - uploadAndVerifyFailure(program, writeDataToFile(List.of(newExp), traits), traits.get(0).getObservationVariableName()); + uploadAndVerifyFailure(program, writeDataToFile(List.of(newExp), traits), traits.get(0).getObservationVariableName(), commit); } - @Test + @ParameterizedTest + @ValueSource(booleans = {true, false}) @SneakyThrows - public void verifyFailureNewOuExistingEnv() { - Program program = createProgram("New OU Exising Env", "FAILOU", "FAILOU", BRAPI_REFERENCE_SOURCE, createGermplasm(1), null); + public void verifyFailureNewOuExistingEnv(boolean commit) { + Program program = createProgram("New OU Exising Env "+(commit ? "C" : "P"), "FLOU"+(commit ? "C" : "P"), "FLOU"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(1), null); Map newExp = new HashMap<>(); newExp.put(Columns.GERMPLASM_GID, "1"); newExp.put(Columns.TEST_CHECK, "T"); @@ -508,7 +598,7 @@ public void verifyFailureNewOuExistingEnv() { newOU.put(Columns.ROW, "1"); newOU.put(Columns.COLUMN, "2"); - Flowable> call = importTestUtils.uploadDataFile(writeDataToFile(List.of(newOU), null), null, true, client, program, mappingId); + Flowable> call = importTestUtils.uploadDataFile(writeDataToFile(List.of(newOU), null), null, commit, client, program, mappingId); HttpResponse response = call.blockingFirst(); assertEquals(HttpStatus.ACCEPTED, response.getStatus()); @@ -585,11 +675,13 @@ public void importNewObsVarExisingOu() { assertRowSaved(newObsVar, program, traits); } - @Test + + @ParameterizedTest + @ValueSource(booleans = {true, false}) @SneakyThrows - public void importNewObsExisingOu() { + public void importNewObsExisingOu(boolean commit) { List traits = createTraits(1); - Program program = createProgram("New Obs Existing OU", "OUOBS", "OUOBS", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Program program = createProgram("New Obs Existing OU "+(commit ? "C" : "P"), "OUOBS"+(commit ? "C" : "P"), "OUOBS"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); Map newExp = new HashMap<>(); newExp.put(Columns.GERMPLASM_GID, "1"); newExp.put(Columns.TEST_CHECK, "T"); @@ -633,7 +725,7 @@ public void importNewObsExisingOu() { newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceID()); newObservation.put(traits.get(0).getObservationVariableName(), "1"); - JsonObject result = importTestUtils.uploadAndFetch(writeDataToFile(List.of(newObservation), traits), null, true, client, program, mappingId); + JsonObject result = importTestUtils.uploadAndFetch(writeDataToFile(List.of(newObservation), traits), null, commit, client, program, mappingId); JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); assertEquals(1, previewRows.size()); @@ -643,14 +735,19 @@ public void importNewObsExisingOu() { assertEquals("EXISTING", row.getAsJsonObject("location").get("state").getAsString()); assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); - assertRowSaved(newObservation, program, traits); + if(commit) { + assertRowSaved(newObservation, program, traits); + } else { + assertValidPreviewRow(newObservation, row, program, traits); + } } - @Test + @ParameterizedTest + @ValueSource(booleans = {true, false}) @SneakyThrows - public void verifyFailureImportNewObsExisingOuWithExistingObs() { + public void verifyFailureImportNewObsExisingOuWithExistingObs(boolean commit) { List traits = createTraits(1); - Program program = createProgram("New Obs Existing Obs", "EXOBS", "EXOBS", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Program program = createProgram("New Obs Existing Obs "+(commit ? "C" : "P"), "FEXOB"+(commit ? "C" : "P"), "FEXOB"+(commit ? "C" : "P"), BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); Map newExp = new HashMap<>(); newExp.put(Columns.GERMPLASM_GID, "1"); newExp.put(Columns.TEST_CHECK, "T"); @@ -695,7 +792,7 @@ public void verifyFailureImportNewObsExisingOuWithExistingObs() { newObservation.put(Columns.OBS_UNIT_ID, ouIdXref.get().getReferenceID()); newObservation.put(traits.get(0).getObservationVariableName(), "2"); - uploadAndVerifyFailure(program, writeDataToFile(List.of(newObservation), traits), traits.get(0).getObservationVariableName()); + uploadAndVerifyFailure(program, writeDataToFile(List.of(newObservation), traits), traits.get(0).getObservationVariableName(), commit); } /* @@ -885,7 +982,7 @@ private Map assertRowSaved(Map expected, Program return ret; } - private Map assertValidPreviewRow(Map expected, JsonObject actual, Program program, List traits) { + private Map assertValidPreviewRow(Map expected, JsonObject actual, Program program, List traits) throws ApiException { Map ret = new HashMap<>(); assertNotNull(actual.get("trial")); @@ -911,7 +1008,11 @@ private Map assertValidPreviewRow(Map expected, List observations = null; if(traits != null) { assertNotNull(actual.get("observations")); - observations = gson.fromJson(actual.get("observations"), new TypeToken>(){}.getType()); + observations = StreamSupport.stream(actual.getAsJsonArray("observations") + .spliterator(), false) + .map(obs -> gson.fromJson(obs.getAsJsonObject() + .getAsJsonObject("brAPIObject"), BrAPIObservation.class)) + .collect(Collectors.toList()); ret.put("observations", observations); } @@ -932,10 +1033,21 @@ private Map assertValidPreviewRow(Map expected, assertEquals(expected.get(Columns.EXP_TYPE), trial.getAdditionalInfo().get(BrAPIAdditionalInfoFields.EXPERIMENT_TYPE).getAsString()); assertEquals(expected.get(Columns.EXP_TYPE), study.getStudyType()); assertEquals(expected.get(Columns.ENV), Utilities.removeProgramKeyAndUnknownAdditionalData(study.getStudyName(), program.getKey())); - assertEquals(expected.get(Columns.ENV_LOCATION), study.getLocationName()); - assertEquals(expected.get(Columns.ENV_LOCATION), location.getName()); - //TODO figure out how to get the actual season value -// assertEquals(expected.getInt(Columns.ENV_YEAR), Integer.parseInt(study.getSeasons().get(0))); + assertEquals(expected.get(Columns.ENV_LOCATION), Utilities.removeProgramKey(study.getLocationName(), program.getKey())); + assertEquals(expected.get(Columns.ENV_LOCATION), Utilities.removeProgramKey(location.getName(), program.getKey())); + + /* + added this try block because the year can come back as either the seasonDbId + or the actual year value depending on if the test is appending data to an existing experiment/environment + or creating a new environment as part of the upload + */ + try { + assertEquals(expected.get(Columns.ENV_YEAR), study.getSeasons().get(0)); + } catch (AssertionFailedError error) { + String expectedYearId = yearToSeasonDbId((String)expected.get(Columns.ENV_YEAR), program.getId()); + assertEquals(expectedYearId, study.getSeasons().get(0)); + } + assertEquals(expected.get(Columns.EXP_UNIT_ID), Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getObservationUnitName(), program.getKey())); BrAPIObservationUnitLevelRelationship rep = null; @@ -977,6 +1089,20 @@ private Map assertValidPreviewRow(Map expected, return ret; } + private String yearToSeasonDbId(String year, UUID programId) throws ApiException { + List seasons = this.seasonDAO.getSeasonsByYear(year, programId); + + for (BrAPISeason season : seasons) { + if (null == season.getSeasonName() || season.getSeasonName() + .isBlank() || season.getSeasonName() + .equals(year)) { + return season.getSeasonDbId(); + } + } + + return null; + } + private Program createProgram(String name, String abbv, String key, String referenceSource, List germplasm, List traits) throws ApiException, DoesNotExistException, ValidatorException, BadRequestException { SpeciesEntity validSpecies = speciesDAO.findAll().get(0); SpeciesRequest speciesRequest = SpeciesRequest.builder() @@ -1074,7 +1200,7 @@ private List createTraits(int numToCreate) { return traits; } - private JsonObject uploadAndVerifyFailure(Program program, File file, String expectedColumnError) throws InterruptedException, IOException { + private JsonObject uploadAndVerifyFailure(Program program, File file, String expectedColumnError, boolean commit) throws InterruptedException, IOException { Flowable> call = importTestUtils.uploadDataFile(file, null, true, client, program, mappingId); HttpResponse response = call.blockingFirst(); assertEquals(HttpStatus.ACCEPTED, response.getStatus());