From 2faaf9271c18b0455a056eaceb6fccc9853bf50b Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 21 Sep 2023 13:22:41 -0400 Subject: [PATCH 01/22] update ExperimentObservation.java to have fields overwrite and overwriteReason --- .../brapps/importer/model/config/ImportFieldTypeEnum.java | 1 + .../experimentObservation/ExperimentObservation.java | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/config/ImportFieldTypeEnum.java b/src/main/java/org/breedinginsight/brapps/importer/model/config/ImportFieldTypeEnum.java index bb176bd0d..d54b509ef 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/config/ImportFieldTypeEnum.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/config/ImportFieldTypeEnum.java @@ -21,6 +21,7 @@ @Getter public enum ImportFieldTypeEnum { + BOOLEAN("BOOLEAN"), TEXT("TEXT"), NUMERICAL("NUMERICAL"), INTEGER("INTEGER"), diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java index f45cd83a3..20eb95745 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java @@ -47,6 +47,13 @@ @ImportConfigMetadata(id = "ExperimentImport", name = "Experiment Import", description = "This import is used to create Observation Unit and Experiment data") public class ExperimentObservation implements BrAPIImport { + @ImportFieldType(type = ImportFieldTypeEnum.BOOLEAN) + @ImportFieldMetadata(id = "overwrite", name = "Overwrite", description = "Boolean flag to overwrite existing observation") + private boolean overwrite; + + @ImportFieldType(type = ImportFieldTypeEnum.TEXT, collectTime = ImportCollectTimeEnum.UPLOAD) + @ImportFieldMetadata(id="overwriteReason", name="Overwrite Reason", description="Description of the reason for overwriting existing observations") + private String overwriteReason; @ImportFieldType(type = ImportFieldTypeEnum.TEXT) @ImportFieldMetadata(id = "germplasmName", name = Columns.GERMPLASM_NAME, description = "Name of germplasm") From 38231a9264680731ba2992ddb2441461ca0aacde Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 2 Oct 2023 15:42:23 -0400 Subject: [PATCH 02/22] add count of existing observations to preview --- .../processors/ExperimentProcessor.java | 61 +++++++++++++++---- 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index e5ec4a5c5..4fa572cec 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 @@ -44,6 +44,7 @@ import org.breedinginsight.brapi.v2.dao.BrAPIGermplasmDAO; import org.breedinginsight.brapps.importer.daos.*; import org.breedinginsight.brapps.importer.model.ImportUpload; +import org.breedinginsight.brapps.importer.model.base.AdditionalInfo; import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; import org.breedinginsight.brapps.importer.model.imports.PendingImport; import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation; @@ -77,6 +78,8 @@ import java.util.function.Supplier; import java.util.stream.Collectors; +import static org.breedinginsight.brapps.importer.services.FileMappingUtil.EXPERIMENT_TEMPLATE_NAME; + @Slf4j @Prototype public class ExperimentProcessor implements Processor { @@ -127,7 +130,7 @@ public class ExperimentProcessor implements Processor { private Map> observationUnitByNameNoScope = null; private final Map> observationByHash = new HashMap<>(); - + private Map existingObsByObsHash = new HashMap<>(); // existingGermplasmByGID is populated by getExistingBrapiData(), but not updated by the getNewBrapiData() method private Map> existingGermplasmByGID = null; @@ -240,7 +243,7 @@ public Map process( log.debug("done processing experiment import"); // Construct our response object - return generateStatisticsMap(importRows); + return generateStatisticsMap(importRows, phenotypeCols); } @Override @@ -550,7 +553,7 @@ private String getObservationHash(String observationUnitName, String variableNam private ValidationErrors validateFields(List importRows, ValidationErrors validationErrors, Map mappedBrAPIImport, List referencedTraits, Program program, List> phenotypeCols, boolean commit) throws MissingRequiredInfoException, ApiException { //fetching any existing observations for any OUs in the import - Map existingObsByObsHash = fetchExistingObservations(referencedTraits, program); + this.existingObsByObsHash = fetchExistingObservations(referencedTraits, program); CaseInsensitiveMap colVarMap = new CaseInsensitiveMap<>(); for ( Trait trait: referencedTraits) { colVarMap.put(trait.getObservationVariableName(),trait); @@ -572,7 +575,7 @@ private ValidationErrors validateFields(List importRows, Validation validateConditionallyRequired(validationErrors, rowNum, importRow, program, commit); validateObservationUnits(validationErrors, uniqueStudyAndObsUnit, rowNum, importRow); - validateObservations(validationErrors, rowNum, importRow, phenotypeCols, colVarMap, existingObsByObsHash); + validateObservations(validationErrors, rowNum, importRows.size(), importRow, phenotypeCols, colVarMap, commit); } return validationErrors; } @@ -625,22 +628,45 @@ private Map fetchExistingObservations(List refe .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); } - private void validateObservations(ValidationErrors validationErrors, int rowNum, ExperimentObservation importRow, List> phenotypeCols, CaseInsensitiveMap colVarMap, Map existingObservations) { + private void validateObservations(ValidationErrors validationErrors, + int rowNum, + int numRows, + ExperimentObservation importRow, + List> phenotypeCols, + Map colVarMap, + boolean commit) { phenotypeCols.forEach(phenoCol -> { var importHash = getImportObservationHash(importRow, phenoCol.name()); - if(existingObservations.containsKey(importHash) && StringUtils.isNotBlank(phenoCol.getString(rowNum)) && !existingObservations.get(importHash).getValue().equals(phenoCol.getString(rowNum))) { + + // error if import observation data already exists and user has not selected to overwrite + if(commit && !importRow.isOverwrite() && + this.existingObsByObsHash.containsKey(importHash) && + StringUtils.isNotBlank(phenoCol.getString(numRows - rowNum - 1)) && + !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum - 1))) { addRowError( phenoCol.name(), String.format("Value already exists for ObsUnitId: %s, Phenotype: %s", importRow.getObsUnitID(), phenoCol.name()), validationErrors, rowNum ); - } else if(existingObservations.containsKey(importHash) && (StringUtils.isBlank(phenoCol.getString(rowNum)) || existingObservations.get(importHash).getValue().equals(phenoCol.getString(rowNum)))) { - BrAPIObservation existingObs = existingObservations.get(importHash); + + // preview case where observation has already been committed and the import row ObsVar data differs from what + // had been saved prior to import + } else if (this.existingObsByObsHash.containsKey(importHash) && + StringUtils.isNotBlank(phenoCol.getString(numRows - rowNum -1)) && + !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum -1))) { + observationByHash.get(importHash).setState(ImportObjectState.EXISTING); + + // preview case where observation has already been committed and import ObsVar data is either empty or the + // same as has been committed prior to import + } else if(this.existingObsByObsHash.containsKey(importHash) && + (StringUtils.isBlank(phenoCol.getString(numRows - rowNum -1)) || + this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum -1)))) { + BrAPIObservation existingObs = this.existingObsByObsHash.get(importHash); existingObs.setObservationVariableName(phenoCol.name()); observationByHash.get(importHash).setState(ImportObjectState.EXISTING); observationByHash.get(importHash).setBrAPIObject(existingObs); } else { - validateObservationValue(colVarMap.get(phenoCol.name()), phenoCol.getString(rowNum), phenoCol.name(), validationErrors, rowNum); + validateObservationValue(colVarMap.get(phenoCol.name()), phenoCol.getString(numRows - rowNum -1), phenoCol.name(), validationErrors, rowNum); //Timestamp validation if(timeStampColByPheno.containsKey(phenoCol.name())) { @@ -734,7 +760,7 @@ private void addIfNotNull(HashSet set, String setValue) { } } - private Map generateStatisticsMap(List importRows) { + private Map generateStatisticsMap(List importRows, List> phenotypeCols) { // Data for stats. HashSet environmentNameCounter = new HashSet<>(); // set of unique environment names HashSet obsUnitsIDCounter = new HashSet<>(); // set of unique observation unit ID's @@ -757,6 +783,15 @@ private Map generateStatisticsMap(List preview != null && preview.getState() == ImportObjectState.EXISTING && + !StringUtils.isBlank(preview.getBrAPIObject() + .getValue())) + .count() + ); + ImportPreviewStatistics environmentStats = ImportPreviewStatistics.builder() .newObjectCount(environmentNameCounter.size()) .build(); @@ -769,12 +804,16 @@ private Map generateStatisticsMap(List Date: Mon, 2 Oct 2023 19:07:10 -0400 Subject: [PATCH 03/22] create BrAPIObservationDAO#updateBrAPIObservation --- .../constants/BrAPIAdditionalInfoFields.java | 1 + .../importer/daos/BrAPIObservationDAO.java | 25 +++++++++++++++++++ .../processors/ExperimentProcessor.java | 6 +++-- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java b/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java index 821c39a98..27723e9d5 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java +++ b/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java @@ -47,6 +47,7 @@ public final class BrAPIAdditionalInfoFields { public static final String MALE_PARENT_UNKNOWN = "maleParentUnknown"; public static final String TREATMENTS = "treatments"; public static final String GID = "gid"; + public static final String CHANGELOG = "changeLog"; public static final String ENV_YEAR = "envYear"; public static final String GERMPLASM_UUID = "germplasmId"; public static final String SAMPLE_ORGANISM = "organism"; diff --git a/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationDAO.java b/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationDAO.java index 7083cae0e..9c9ed184b 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationDAO.java +++ b/src/main/java/org/breedinginsight/brapps/importer/daos/BrAPIObservationDAO.java @@ -16,6 +16,7 @@ */ package org.breedinginsight.brapps.importer.daos; +import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.tuple.Pair; import org.brapi.client.v2.ApiResponse; import org.brapi.client.v2.model.exceptions.ApiException; @@ -24,11 +25,13 @@ import org.brapi.v2.model.pheno.BrAPIObservation; import org.brapi.v2.model.pheno.request.BrAPIObservationSearchRequest; import org.brapi.v2.model.pheno.response.BrAPIObservationListResponse; +import org.brapi.v2.model.pheno.response.BrAPIObservationSingleResponse; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.daos.ProgramDAO; import org.breedinginsight.model.Program; import org.breedinginsight.services.brapi.BrAPIEndpointProvider; import org.breedinginsight.utilities.BrAPIDAOUtil; +import org.breedinginsight.utilities.Utilities; import org.jetbrains.annotations.NotNull; import javax.inject.Inject; @@ -38,6 +41,7 @@ import static org.brapi.v2.model.BrAPIWSMIMEDataTypes.APPLICATION_JSON; @Singleton +@Slf4j public class BrAPIObservationDAO { private ProgramDAO programDAO; @@ -113,4 +117,25 @@ public List createBrAPIObservations(List brA return brAPIDAOUtil.post(brAPIObservationList, upload, api::observationsPost, importDAO::update); } + public BrAPIObservation updateBrAPIObservation(String dbId, BrAPIObservation observation, UUID programId) throws ApiException { + ObservationsApi api = brAPIEndpointProvider.get(programDAO.getCoreClient(programId), ObservationsApi.class); + ApiResponse response; + BrAPIObservation updatedObservation = null; + try { + response = api.observationsObservationDbIdPut(dbId, observation); + if (response != null) { + BrAPIObservationSingleResponse body = response.getBody(); + if (body == null) { + throw new ApiException("Response is missing body", 0, response.getHeaders(), null); + } + updatedObservation = body.getResult(); + if (updatedObservation == null) { + throw new ApiException("Response body is missing result", 0, response.getHeaders(), response.getBody().toString()); + } + } + } catch (ApiException e) { + log.error(Utilities.generateApiExceptionLogMessage(e)); + } + return updatedObservation; + } } 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 4fa572cec..24a70f82b 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 @@ -280,6 +280,7 @@ public void postBrapiData(Map mappedBrAPIImport, Program .getMutationsByObjectId(obsVarDatasetByName, BrAPIListSummary::getListDbId); List newObservationUnits = ProcessorData.getNewObjects(this.observationUnitByNameNoScope); + // filter out observations with no 'value' so they will not be saved List newObservations = ProcessorData.getNewObjects(this.observationByHash) .stream() @@ -654,7 +655,8 @@ private void validateObservations(ValidationErrors validationErrors, } else if (this.existingObsByObsHash.containsKey(importHash) && StringUtils.isNotBlank(phenoCol.getString(numRows - rowNum -1)) && !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum -1))) { - observationByHash.get(importHash).setState(ImportObjectState.EXISTING); + observationByHash.get(importHash).getBrAPIObject().setValue(phenoCol.getString(numRows - rowNum -1)); + observationByHash.get(importHash).setState(ImportObjectState.MUTATED); // preview case where observation has already been committed and import ObsVar data is either empty or the // same as has been committed prior to import @@ -786,7 +788,7 @@ private Map generateStatisticsMap(List preview != null && preview.getState() == ImportObjectState.EXISTING && + .filter(preview -> preview != null && (preview.getState() == ImportObjectState.EXISTING || preview.getState() == ImportObjectState.MUTATED) && !StringUtils.isBlank(preview.getBrAPIObject() .getValue())) .count() From 099023291915b5eb7c8010a2db22373fdb32e576 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 2 Oct 2023 21:58:18 -0400 Subject: [PATCH 04/22] add change log to additional info of mutated observation --- .../processors/ExperimentProcessor.java | 94 +++++++++++++------ 1 file changed, 65 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 24a70f82b..c659e88e3 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 @@ -16,6 +16,9 @@ */ package org.breedinginsight.brapps.importer.services.processors; +import com.google.gson.JsonArray; +import com.google.gson.Gson; +import com.google.gson.JsonObject; import com.google.gson.JsonElement; import com.google.gson.JsonObject; import io.micronaut.context.annotation.Property; @@ -27,6 +30,7 @@ import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.collections4.map.CaseInsensitiveMap; +import org.brapi.client.v2.JSON; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.v2.model.BrAPIExternalReference; import org.brapi.v2.model.core.*; @@ -46,6 +50,7 @@ import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.model.base.AdditionalInfo; import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; +import org.breedinginsight.brapps.importer.model.imports.ChangeLogEntry; import org.breedinginsight.brapps.importer.model.imports.PendingImport; import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation; import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation.Columns; @@ -95,6 +100,9 @@ public class ExperimentProcessor implements Processor { private static final String TIMESTAMP_REGEX = "^"+TIMESTAMP_PREFIX+"\\s*"; private static final String COMMA_DELIMITER = ","; private static final String BLANK_FIELD = "Required field is blank"; + 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"; @@ -136,6 +144,8 @@ public class ExperimentProcessor implements Processor { // Associates timestamp columns to associated phenotype column name for ease of storage private final Map> timeStampColByPheno = new HashMap<>(); + private final Gson gson; + @Inject public ExperimentProcessor(DSLContext dsl, @@ -159,6 +169,7 @@ public ExperimentProcessor(DSLContext dsl, this.brAPIListDAO = brAPIListDAO; this.ontologyService = ontologyService; this.fileMappingUtil = fileMappingUtil; + this.gson = new JSON().getGson(); } @Override @@ -235,7 +246,7 @@ public Map process( prepareDataForValidation(importRows, phenotypeCols, mappedBrAPIImport); - validateFields(importRows, validationErrors, mappedBrAPIImport, referencedTraits, program, phenotypeCols, commit); + validateFields(importRows, validationErrors, mappedBrAPIImport, referencedTraits, program, phenotypeCols, commit, user); if (validationErrors.hasErrors()) { throw new ValidatorException(validationErrors); @@ -552,7 +563,7 @@ private String getObservationHash(String observationUnitName, String variableNam } private ValidationErrors validateFields(List importRows, ValidationErrors validationErrors, Map mappedBrAPIImport, List referencedTraits, Program program, - List> phenotypeCols, boolean commit) throws MissingRequiredInfoException, ApiException { + List> phenotypeCols, boolean commit, User user) throws MissingRequiredInfoException, ApiException { //fetching any existing observations for any OUs in the import this.existingObsByObsHash = fetchExistingObservations(referencedTraits, program); CaseInsensitiveMap colVarMap = new CaseInsensitiveMap<>(); @@ -576,7 +587,7 @@ private ValidationErrors validateFields(List importRows, Validation validateConditionallyRequired(validationErrors, rowNum, importRow, program, commit); validateObservationUnits(validationErrors, uniqueStudyAndObsUnit, rowNum, importRow); - validateObservations(validationErrors, rowNum, importRows.size(), importRow, phenotypeCols, colVarMap, commit); + validateObservations(validationErrors, rowNum, importRows.size(), importRow, phenotypeCols, colVarMap, commit, user); } return validationErrors; } @@ -605,8 +616,8 @@ private Map fetchExistingObservations(List refe observationUnitByNameNoScope.values().forEach(ou -> { if(StringUtils.isNotBlank(ou.getBrAPIObject().getObservationUnitDbId())) { ouDbIds.add(ou.getBrAPIObject().getObservationUnitDbId()); - ouNameByDbId.put(ou.getBrAPIObject().getObservationUnitDbId(), Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getBrAPIObject().getObservationUnitName(), program.getKey())); } + ouNameByDbId.put(ou.getBrAPIObject().getObservationUnitDbId(), Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getBrAPIObject().getObservationUnitName(), program.getKey())); }); for (Trait referencedTrait : referencedTraits) { @@ -635,7 +646,8 @@ private void validateObservations(ValidationErrors validationErrors, ExperimentObservation importRow, List> phenotypeCols, Map colVarMap, - boolean commit) { + boolean commit, + User user) { phenotypeCols.forEach(phenoCol -> { var importHash = getImportObservationHash(importRow, phenoCol.name()); @@ -658,6 +670,27 @@ private void validateObservations(ValidationErrors validationErrors, observationByHash.get(importHash).getBrAPIObject().setValue(phenoCol.getString(numRows - rowNum -1)); observationByHash.get(importHash).setState(ImportObjectState.MUTATED); + // add a change log entry when updating the value of an observation + if (commit) { + BrAPIObservation pendingObservation = observationByHash.get(importHash).getBrAPIObject(); + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd:hh-mm-ssZ"); + String timestamp = formatter.format(OffsetDateTime.now()); + ChangeLogEntry change = new ChangeLogEntry(existingObsByObsHash.get(importHash).getValue(), + importRow.getOverwriteReason(), + user.getId(), + timestamp + ); + if (pendingObservation.getAdditionalInfo().isJsonNull()) { + pendingObservation.setAdditionalInfo(new JsonObject()); + pendingObservation.getAdditionalInfo().add(BrAPIAdditionalInfoFields.CHANGELOG, new JsonArray()); + } + + if (pendingObservation.getAdditionalInfo() != null && !pendingObservation.getAdditionalInfo().has(BrAPIAdditionalInfoFields.CHANGELOG)) { + pendingObservation.getAdditionalInfo().add(BrAPIAdditionalInfoFields.CHANGELOG, new JsonArray()); + } + pendingObservation.getAdditionalInfo().get(BrAPIAdditionalInfoFields.CHANGELOG).getAsJsonArray().add(gson.toJson(change)); + } + // preview case where observation has already been committed and import ObsVar data is either empty or the // same as has been committed prior to import } else if(this.existingObsByObsHash.containsKey(importHash) && @@ -708,7 +741,12 @@ private void validateConditionallyRequired(ValidationErrors validationErrors, in .getState(); ImportObjectState envState = this.studyByNameNoScope.get(importRow.getEnv()).getState(); - String errorMessage = BLANK_FIELD; + String errorMessage = BLANK_FIELD_EXPERIMENT; + if (expState == ImportObjectState.EXISTING && envState == ImportObjectState.NEW) { + errorMessage = BLANK_FIELD_ENV; + } else if(expState == ImportObjectState.EXISTING && envState == ImportObjectState.EXISTING) { + errorMessage = BLANK_FIELD_OBS; + } if(expState == ImportObjectState.NEW || envState == ImportObjectState.NEW) { validateRequiredCell(importRow.getGid(), Columns.GERMPLASM_GID, errorMessage, validationErrors, rowNum); @@ -875,20 +913,18 @@ private PendingImportObject fetchOrCreateObsUnitPIO(Progra } - private PendingImportObject fetchOrCreateObservationPIO(Program program, - User user, - ExperimentObservation importRow, - String variableName, - String value, - String timeStampValue, - boolean commit, - String seasonDbId, - PendingImportObject obsUnitPIO) { + private void fetchOrCreateObservationPIO(Program program, + User user, + ExperimentObservation importRow, + String variableName, + String value, + String timeStampValue, + boolean commit, + String seasonDbId, + PendingImportObject obsUnitPIO) { PendingImportObject pio; String key = getImportObservationHash(importRow, variableName); - if (this.observationByHash.containsKey(key)) { - pio = observationByHash.get(key); - } else { + if (!this.observationByHash.containsKey(key)) { PendingImportObject trialPIO = this.trialByNameNoScope.get(importRow.getExpTitle()); UUID trialID = trialPIO.getId(); PendingImportObject studyPIO = this.studyByNameNoScope.get(importRow.getEnv()); @@ -906,13 +942,17 @@ private PendingImportObject fetchOrCreateObservationPIO(Progra pio = new PendingImportObject<>(ImportObjectState.NEW, newObservation); this.observationByHash.put(key, pio); } - return pio; } private void addObsVarsToDatasetDetails(PendingImportObject pio, List referencedTraits, Program program) { BrAPIListDetails details = pio.getBrAPIObject(); referencedTraits.forEach(trait -> { - String id = trait.getRawObservationVariableName(); + String id = Utilities.appendProgramKey(trait.getObservationVariableName(), program.getKey()); + // Don't append the key if connected to a brapi service operating with legacy data(no appended program key) + if (trait.getFullName() == null) { + id = trait.getObservationVariableName(); + } + if (!details.getData().contains(id) && ImportObjectState.EXISTING != pio.getState()) { details.getData().add(id); } @@ -968,7 +1008,7 @@ private PendingImportObject fetchOrCreateStudyPIO(Program program, b // It is assumed that the study has only one season, And that the Years and not // the dbId's are stored in getSeason() list. - String year = newStudy.getSeasons().get(0); + String year = newStudy.getSeasons().get(0); // It is assumed that the study has only one season if (commit) { if(StringUtils.isNotBlank(year)) { String seasonID = this.yearToSeasonDbId(year, program.getId()); @@ -978,7 +1018,6 @@ private PendingImportObject fetchOrCreateStudyPIO(Program program, b addYearToStudyAdditionalInfo(program, newStudy, year); } - pio = new PendingImportObject<>(ImportObjectState.NEW, newStudy, id); this.studyByNameNoScope.put(importRow.getEnv(), pio); } @@ -1016,16 +1055,13 @@ private void addYearToStudyAdditionalInfo(Program program, BrAPIStudy study, Str } } - private PendingImportObject fetchOrCreateLocationPIO(ExperimentObservation importRow) { + private void fetchOrCreateLocationPIO(ExperimentObservation importRow) { PendingImportObject pio; - if (locationByName.containsKey((importRow.getEnvLocation()))) { - pio = locationByName.get(importRow.getEnvLocation()); - } else { + if (! locationByName.containsKey((importRow.getEnvLocation()))) { ProgramLocation newLocation = importRow.constructProgramLocation(); pio = new PendingImportObject<>(ImportObjectState.NEW, newLocation, UUID.randomUUID()); this.locationByName.put(importRow.getEnvLocation(), pio); } - return pio; } private PendingImportObject fetchOrCreateTrialPIO(Program program, User user, boolean commit, ExperimentObservation importRow, Supplier expNextVal) throws UnprocessableEntityException { @@ -1381,7 +1417,7 @@ private Map> initializeObsVarDatas BrAPIListDetails dataSetDetails = brAPIListDAO .getListById(existingDatasets.get(0).getListDbId(), program.getId()) .getResult(); - processAndCacheObsVarDataset(dataSetDetails, program, obsVarDatasetByName); + processAndCacheObsVarDataset(dataSetDetails, obsVarDatasetByName); } catch (ApiException e) { log.error(Utilities.generateApiExceptionLogMessage(e), e); throw new InternalServerException(e.toString(), e); @@ -1405,7 +1441,7 @@ private Optional> getTrialPIO(List> obsVarDatasetByName) { + private void processAndCacheObsVarDataset(BrAPIListDetails existingList, Map> obsVarDatasetByName) { BrAPIExternalReference xref = Utilities.getExternalReference(existingList.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.DATASET.getName())) .orElseThrow(() -> new IllegalStateException("External references wasn't found for list (dbid): " + existingList.getListDbId())); From ad3d566848dc32b1296c67e21aac2d1c571a188c Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 2 Oct 2023 22:20:00 -0400 Subject: [PATCH 05/22] update mutated observations in postBrapiData --- .../services/processors/ExperimentProcessor.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index c659e88e3..ea9258f55 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 @@ -270,6 +270,9 @@ public void postBrapiData(Map mappedBrAPIImport, Program Map mutatedTrialsById = ProcessorData .getMutationsByObjectId(trialByNameNoScope, BrAPITrial::getTrialDbId); + Map mutatedObservationByDbId = ProcessorData + .getMutationsByObjectId(observationByHash, observation -> observation.getObservationDbId()); + List newLocations = ProcessorData.getNewObjects(this.locationByName) .stream() .map(location -> ProgramLocationRequest.builder() @@ -388,6 +391,19 @@ public void postBrapiData(Map mappedBrAPIImport, Program throw new InternalServerException(e.getMessage(), e); } }); + + mutatedObservationByDbId.forEach((id, observation) -> { + try { + brAPIObservationDAO.updateBrAPIObservation(id, observation, program.getId()); + } catch (ApiException e) { + log.error("Error updating observation: " + Utilities.generateApiExceptionLogMessage(e), e); + throw new InternalServerException("Error saving experiment import", e); + } catch (Exception e) { + log.error("Error updating observation: ", e); + throw new InternalServerException(e.getMessage(), e); + } + }); + log.debug("experiment import complete"); } From d1e8fff33a76837e5918ff9c848fa90967336ec3 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 3 Oct 2023 13:19:04 -0400 Subject: [PATCH 06/22] add check for NPE --- .../importer/services/processors/ExperimentProcessor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index ea9258f55..87733e5a9 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 @@ -691,8 +691,9 @@ private void validateObservations(ValidationErrors validationErrors, BrAPIObservation pendingObservation = observationByHash.get(importHash).getBrAPIObject(); DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd:hh-mm-ssZ"); String timestamp = formatter.format(OffsetDateTime.now()); + String reason = importRow.getOverwriteReason() != null ? importRow.getOverwriteReason() : ""; ChangeLogEntry change = new ChangeLogEntry(existingObsByObsHash.get(importHash).getValue(), - importRow.getOverwriteReason(), + reason, user.getId(), timestamp ); From 72dc5f79c53c50410724615e401cbb9f37862e97 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 4 Oct 2023 16:06:17 -0400 Subject: [PATCH 07/22] count mutated observations --- .../processors/ExperimentProcessor.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 87733e5a9..aba667ba3 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 @@ -683,7 +683,7 @@ private void validateObservations(ValidationErrors validationErrors, } else if (this.existingObsByObsHash.containsKey(importHash) && StringUtils.isNotBlank(phenoCol.getString(numRows - rowNum -1)) && !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum -1))) { - observationByHash.get(importHash).getBrAPIObject().setValue(phenoCol.getString(numRows - rowNum -1)); + //observationByHash.get(importHash).getBrAPIObject().setValue(phenoCol.getString(numRows - rowNum -1)); observationByHash.get(importHash).setState(ImportObjectState.MUTATED); // add a change log entry when updating the value of an observation @@ -843,12 +843,22 @@ private Map generateStatisticsMap(List preview != null && (preview.getState() == ImportObjectState.EXISTING || preview.getState() == ImportObjectState.MUTATED) && + .filter(preview -> preview != null && preview.getState() == ImportObjectState.EXISTING && !StringUtils.isBlank(preview.getBrAPIObject() .getValue())) .count() ); + int numMutatedObservations = Math.toIntExact( + this.observationByHash.values() + .stream() + .filter(preview -> preview != null && preview.getState() == ImportObjectState.MUTATED && + !StringUtils.isBlank(preview.getBrAPIObject() + .getValue())) + .count() + ); + + ImportPreviewStatistics environmentStats = ImportPreviewStatistics.builder() .newObjectCount(environmentNameCounter.size()) .build(); @@ -864,13 +874,17 @@ private Map generateStatisticsMap(List Date: Sat, 7 Oct 2023 12:19:58 -0400 Subject: [PATCH 08/22] change overwrite field type to String --- .../ExperimentObservation.java | 4 ++-- .../importer/services/MappingManager.java | 19 ++++++++++++++----- .../processors/ExperimentProcessor.java | 3 ++- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java index 20eb95745..290c046ad 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentObservation.java @@ -47,9 +47,9 @@ @ImportConfigMetadata(id = "ExperimentImport", name = "Experiment Import", description = "This import is used to create Observation Unit and Experiment data") public class ExperimentObservation implements BrAPIImport { - @ImportFieldType(type = ImportFieldTypeEnum.BOOLEAN) + @ImportFieldType(type = ImportFieldTypeEnum.BOOLEAN, collectTime = ImportCollectTimeEnum.UPLOAD) @ImportFieldMetadata(id = "overwrite", name = "Overwrite", description = "Boolean flag to overwrite existing observation") - private boolean overwrite; + private String overwrite; @ImportFieldType(type = ImportFieldTypeEnum.TEXT, collectTime = ImportCollectTimeEnum.UPLOAD) @ImportFieldMetadata(id="overwriteReason", name="Overwrite Reason", description="Description of the reason for overwriting existing observations") diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java b/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java index 5a3a2289f..a64705452 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java @@ -53,6 +53,12 @@ public class MappingManager { private ImportConfigManager configManager; + public static String wrongDataTypeMsg = "Column name \"%s\" must be integer type, but non-integer type provided."; + public static String blankRequiredField = "Required field \"%s\" cannot contain empty values"; + public static String missingColumn = "Column name \"%s\" does not exist in file"; + public static String missingUserInput = "User input, \"%s\" is required"; + public static String wrongUserInputDataType = "User input, \"%s\" must be an %s"; + @Inject MappingManager(ImportConfigManager configManager) { this.configManager = configManager; @@ -339,7 +345,7 @@ private void mapUserInputField(Object parent, Field field, Map u // Only supports user input at the top level of an object at the moment. No nested objects. Map String fieldId = metadata.id(); - if (!userInput.containsKey(fieldId) && required != null) { + if ((userInput == null || !userInput.containsKey(fieldId)) && required != null) { throw new UnprocessableEntityException(importService.getMissingUserInputMsg(metadata.name())); } else if (required != null && userInput.containsKey(fieldId) && userInput.get(fieldId).toString().isBlank()) { @@ -369,12 +375,15 @@ private void checkFieldType(ImportFieldTypeEnum expectedType, String column, Str private Boolean isCorrectType(ImportFieldTypeEnum expectedType, String value) { if (!value.isBlank()) { - if (expectedType == ImportFieldTypeEnum.INTEGER) { - try { + try { + if (expectedType == ImportFieldTypeEnum.INTEGER) { Integer d = Integer.parseInt(value); - } catch (NumberFormatException nfe) { - return false; } + if (expectedType == ImportFieldTypeEnum.BOOLEAN) { + Boolean b = Boolean.parseBoolean(value); + } + } catch (NumberFormatException nfe) { + return false; } } return true; 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 aba667ba3..d4f496762 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 @@ -668,7 +668,7 @@ private void validateObservations(ValidationErrors validationErrors, var importHash = getImportObservationHash(importRow, phenoCol.name()); // error if import observation data already exists and user has not selected to overwrite - if(commit && !importRow.isOverwrite() && + if(commit && importRow.getOverwrite().equals("false") && this.existingObsByObsHash.containsKey(importHash) && StringUtils.isNotBlank(phenoCol.getString(numRows - rowNum - 1)) && !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum - 1))) { @@ -684,6 +684,7 @@ private void validateObservations(ValidationErrors validationErrors, StringUtils.isNotBlank(phenoCol.getString(numRows - rowNum -1)) && !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum -1))) { //observationByHash.get(importHash).getBrAPIObject().setValue(phenoCol.getString(numRows - rowNum -1)); + //this.existingObsByObsHash.get(importHash).setObservationVariableName(phenoCol.name()); observationByHash.get(importHash).setState(ImportObjectState.MUTATED); // add a change log entry when updating the value of an observation From cc944731a6aef40ef84a40bcce31e4067d4c82a4 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 17 Oct 2023 14:11:55 -0400 Subject: [PATCH 09/22] [BI-1830] init observation pio for updated value --- .../processors/ExperimentProcessor.java | 98 +++++++++++++------ 1 file changed, 68 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index d4f496762..498b6c8ee 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 @@ -16,8 +16,8 @@ */ package org.breedinginsight.brapps.importer.services.processors; -import com.google.gson.JsonArray; import com.google.gson.Gson; +import com.google.gson.JsonArray; import com.google.gson.JsonObject; import com.google.gson.JsonElement; import com.google.gson.JsonObject; @@ -68,6 +68,7 @@ import org.breedinginsight.services.exceptions.UnprocessableEntityException; import org.breedinginsight.services.exceptions.ValidatorException; import org.breedinginsight.utilities.Utilities; +import org.checkerframework.checker.nullness.Opt; import org.jooq.DSLContext; import tech.tablesaw.api.Table; import tech.tablesaw.columns.Column; @@ -146,7 +147,6 @@ public class ExperimentProcessor implements Processor { private final Map> timeStampColByPheno = new HashMap<>(); private final Gson gson; - @Inject public ExperimentProcessor(DSLContext dsl, BrAPITrialDAO brapiTrialDAO, @@ -490,7 +490,7 @@ private String getVariableNameFromColumn(Column column) { return column.name(); } - private void initNewBrapiData(List importRows, List> phenotypeCols, Program program, User user, List referencedTraits, boolean commit) { + private void initNewBrapiData(List importRows, List> phenotypeCols, Program program, User user, List referencedTraits, boolean commit) throws ApiException { String expSequenceName = program.getExpSequence(); if (expSequenceName == null) { @@ -554,7 +554,7 @@ private void initNewBrapiData(List importRows, List> phen } //column.name() gets phenotype name String seasonDbId = this.yearToSeasonDbId(importRow.getEnvYear(), program.getId()); - fetchOrCreateObservationPIO(program, user, importRow, column.name(), column.getString(rowNum), dateTimeValue, commit, seasonDbId, obsUnitPIO); + fetchOrCreateObservationPIO(program, user, importRow, column.name(), column.getString(rowNum), dateTimeValue, commit, seasonDbId, obsUnitPIO, referencedTraits); } } } @@ -581,7 +581,6 @@ private String getObservationHash(String observationUnitName, String variableNam private ValidationErrors validateFields(List importRows, ValidationErrors validationErrors, Map mappedBrAPIImport, List referencedTraits, Program program, List> phenotypeCols, boolean commit, User user) throws MissingRequiredInfoException, ApiException { //fetching any existing observations for any OUs in the import - this.existingObsByObsHash = fetchExistingObservations(referencedTraits, program); CaseInsensitiveMap colVarMap = new CaseInsensitiveMap<>(); for ( Trait trait: referencedTraits) { colVarMap.put(trait.getObservationVariableName(),trait); @@ -596,10 +595,10 @@ private ValidationErrors validateFields(List importRows, Validation validateTestOrCheck(importRow, validationErrors, rowNum); //Check if existing environment. If so, ObsUnitId must be assigned - if ((mappedImportRow.getStudy().getState() == ImportObjectState.EXISTING) - && (StringUtils.isBlank(importRow.getObsUnitID()))) { - throw new MissingRequiredInfoException(MISSING_OBS_UNIT_ID_ERROR); - } +// if ((mappedImportRow.getStudy().getState() == ImportObjectState.EXISTING) +// && (StringUtils.isBlank(importRow.getObsUnitID()))) { +// throw new MissingRequiredInfoException(MISSING_OBS_UNIT_ID_ERROR); +// } validateConditionallyRequired(validationErrors, rowNum, importRow, program, commit); validateObservationUnits(validationErrors, uniqueStudyAndObsUnit, rowNum, importRow); @@ -629,12 +628,24 @@ private Map fetchExistingObservations(List refe .map(PendingImportObject::getBrAPIObject) .collect(Collectors.toMap(BrAPIStudy::getStudyDbId, brAPIStudy -> Utilities.removeProgramKeyAndUnknownAdditionalData(brAPIStudy.getStudyName(), program.getKey()))); - observationUnitByNameNoScope.values().forEach(ou -> { - if(StringUtils.isNotBlank(ou.getBrAPIObject().getObservationUnitDbId())) { - ouDbIds.add(ou.getBrAPIObject().getObservationUnitDbId()); + studyNameByDbId.keySet().stream().forEach(studyDbId -> { + try { + brAPIObservationUnitDAO.getObservationUnitsForStudyDbId(studyDbId, program).forEach(ou -> { + if(StringUtils.isNotBlank(ou.getObservationUnitDbId())) { + ouDbIds.add(ou.getObservationUnitDbId()); + } + ouNameByDbId.put(ou.getObservationUnitDbId(), Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getObservationUnitName(), program.getKey())); + }); + } catch (ApiException e) { + throw new RuntimeException(e); } - ouNameByDbId.put(ou.getBrAPIObject().getObservationUnitDbId(), Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getBrAPIObject().getObservationUnitName(), program.getKey())); }); +// observationUnitByNameNoScope.values().forEach(ou -> { +// if(StringUtils.isNotBlank(ou.getBrAPIObject().getObservationUnitDbId())) { +// ouDbIds.add(ou.getBrAPIObject().getObservationUnitDbId()); +// } +// ouNameByDbId.put(ou.getBrAPIObject().getObservationUnitDbId(), Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getBrAPIObject().getObservationUnitName(), program.getKey())); +// }); for (Trait referencedTrait : referencedTraits) { variableDbIds.add(referencedTrait.getObservationVariableDbId()); @@ -685,7 +696,7 @@ private void validateObservations(ValidationErrors validationErrors, !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum -1))) { //observationByHash.get(importHash).getBrAPIObject().setValue(phenoCol.getString(numRows - rowNum -1)); //this.existingObsByObsHash.get(importHash).setObservationVariableName(phenoCol.name()); - observationByHash.get(importHash).setState(ImportObjectState.MUTATED); + //observationByHash.get(importHash).setState(ImportObjectState.MUTATED); // add a change log entry when updating the value of an observation if (commit) { @@ -795,7 +806,7 @@ private void validateConditionallyRequired(ValidationErrors validationErrors, in addRowError(Columns.OBS_UNIT_ID, "ObsUnitID cannot be specified when creating a new environment", validationErrors, rowNum); } } else { - validateRequiredCell(importRow.getObsUnitID(), Columns.OBS_UNIT_ID, errorMessage, validationErrors, rowNum); + //validateRequiredCell(importRow.getObsUnitID(), Columns.OBS_UNIT_ID, errorMessage, validationErrors, rowNum); } } @@ -914,7 +925,7 @@ private PendingImportObject getGidPOI(ExperimentObservation impo return null; } - private PendingImportObject fetchOrCreateObsUnitPIO(Program program, boolean commit, String envSeqValue, ExperimentObservation importRow) { + private PendingImportObject fetchOrCreateObsUnitPIO(Program program, boolean commit, String envSeqValue, ExperimentObservation importRow) throws ApiException { PendingImportObject pio; String key = createObservationUnitKey(importRow); if (this.observationUnitByNameNoScope.containsKey(key)) { @@ -935,10 +946,17 @@ private PendingImportObject fetchOrCreateObsUnitPIO(Progra .get(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID).getAsString()); } PendingImportObject studyPIO = this.studyByNameNoScope.get(importRow.getEnv()); - UUID studyID = studyPIO.getId(); - UUID id = UUID.randomUUID(); - BrAPIObservationUnit newObservationUnit = importRow.constructBrAPIObservationUnit(program, envSeqValue, commit, germplasmName, BRAPI_REFERENCE_SOURCE, trialID, datasetId, studyID, id); - pio = new PendingImportObject<>(ImportObjectState.NEW, newObservationUnit, id); + + List existingOUs = brAPIObservationUnitDAO.getObservationUnitsForStudyDbId(studyPIO.getBrAPIObject().getStudyDbId(), program); + List matchingOU = existingOUs.stream().filter(ou -> importRow.getExpUnitId().equals(Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getObservationUnitName(), program.getKey()))).collect(Collectors.toList()); + if (!matchingOU.isEmpty()) { + pio = new PendingImportObject<>(ImportObjectState.EXISTING, matchingOU.get(0)); + } else { + UUID studyID = studyPIO.getId(); + UUID id = UUID.randomUUID(); + BrAPIObservationUnit newObservationUnit = importRow.constructBrAPIObservationUnit(program, envSeqValue, commit, germplasmName, BRAPI_REFERENCE_SOURCE, trialID, datasetId, studyID, id); + pio = new PendingImportObject<>(ImportObjectState.NEW, newObservationUnit, id); + } this.observationUnitByNameNoScope.put(key, pio); } return pio; @@ -946,23 +964,43 @@ private PendingImportObject fetchOrCreateObsUnitPIO(Progra private void fetchOrCreateObservationPIO(Program program, - User user, - ExperimentObservation importRow, - String variableName, - String value, - String timeStampValue, - boolean commit, - String seasonDbId, - PendingImportObject obsUnitPIO) { + User user, + ExperimentObservation importRow, + String variableName, + String value, + String timeStampValue, + boolean commit, + String seasonDbId, + PendingImportObject obsUnitPIO, + List referencedTraits) throws ApiException { PendingImportObject pio; + BrAPIObservation newObservation; String key = getImportObservationHash(importRow, variableName); - if (!this.observationByHash.containsKey(key)) { + existingObsByObsHash = fetchExistingObservations(referencedTraits, program); + if (existingObsByObsHash.containsKey(key)) { + if (StringUtils.isNotBlank(value) && + !existingObsByObsHash.get(key).getValue().equals(value)) { + + // prior observation with updated value + newObservation = gson.fromJson(gson.toJson(existingObsByObsHash.get(key)), BrAPIObservation.class); + newObservation.setValue(value); + pio = new PendingImportObject<>(ImportObjectState.MUTATED, newObservation); + } else { + + // prior observation + pio = new PendingImportObject<>(ImportObjectState.EXISTING, existingObsByObsHash.get(key)); + } + + observationByHash.put(key, pio); + } else if (!this.observationByHash.containsKey(key)){ + + // new observation PendingImportObject trialPIO = this.trialByNameNoScope.get(importRow.getExpTitle()); UUID trialID = trialPIO.getId(); PendingImportObject studyPIO = this.studyByNameNoScope.get(importRow.getEnv()); UUID studyID = studyPIO.getId(); UUID id = UUID.randomUUID(); - BrAPIObservation newObservation = importRow.constructBrAPIObservation(value, variableName, seasonDbId, obsUnitPIO.getBrAPIObject(), commit, program, user, BRAPI_REFERENCE_SOURCE, trialID, studyID, obsUnitPIO.getId(), id); + newObservation = importRow.constructBrAPIObservation(value, variableName, seasonDbId, obsUnitPIO.getBrAPIObject(), commit, program, user, BRAPI_REFERENCE_SOURCE, trialID, studyID, obsUnitPIO.getId(), id); //NOTE: Can't parse invalid timestamp value, so have to skip if invalid. // Validation error should be thrown for offending value, but that doesn't happen until later downstream if (timeStampValue != null && !timeStampValue.isBlank() && (validDateValue(timeStampValue) || validDateTimeValue(timeStampValue))) { From 7e4f17ab86673dc873a44b903fac5377ed7a1648 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 18 Oct 2023 13:38:44 -0400 Subject: [PATCH 10/22] [BI-1830] refactor --- .../services/processors/ExperimentProcessor.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 498b6c8ee..89d42a263 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 @@ -946,15 +946,16 @@ private PendingImportObject fetchOrCreateObsUnitPIO(Progra .get(BrAPIAdditionalInfoFields.OBSERVATION_DATASET_ID).getAsString()); } PendingImportObject studyPIO = this.studyByNameNoScope.get(importRow.getEnv()); + UUID studyID = studyPIO.getId(); + UUID id = UUID.randomUUID(); + BrAPIObservationUnit newObservationUnit = importRow.constructBrAPIObservationUnit(program, envSeqValue, commit, germplasmName, BRAPI_REFERENCE_SOURCE, trialID, datasetId, studyID, id); - List existingOUs = brAPIObservationUnitDAO.getObservationUnitsForStudyDbId(studyPIO.getBrAPIObject().getStudyDbId(), program); - List matchingOU = existingOUs.stream().filter(ou -> importRow.getExpUnitId().equals(Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getObservationUnitName(), program.getKey()))).collect(Collectors.toList()); - if (!matchingOU.isEmpty()) { - pio = new PendingImportObject<>(ImportObjectState.EXISTING, matchingOU.get(0)); + // check for existing units if this is an existing study + if (studyPIO.getBrAPIObject().getStudyDbId() != null) { + List existingOUs = brAPIObservationUnitDAO.getObservationUnitsForStudyDbId(studyPIO.getBrAPIObject().getStudyDbId(), program); + List matchingOU = existingOUs.stream().filter(ou -> importRow.getExpUnitId().equals(Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getObservationUnitName(), program.getKey()))).collect(Collectors.toList()); + pio = !matchingOU.isEmpty() ? new PendingImportObject<>(ImportObjectState.EXISTING, matchingOU.get(0)) : new PendingImportObject<>(ImportObjectState.NEW, newObservationUnit, id); } else { - UUID studyID = studyPIO.getId(); - UUID id = UUID.randomUUID(); - BrAPIObservationUnit newObservationUnit = importRow.constructBrAPIObservationUnit(program, envSeqValue, commit, germplasmName, BRAPI_REFERENCE_SOURCE, trialID, datasetId, studyID, id); pio = new PendingImportObject<>(ImportObjectState.NEW, newObservationUnit, id); } this.observationUnitByNameNoScope.put(key, pio); From c9d5241d6a9b36ee2540f7074052a97989e81b87 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 18 Oct 2023 19:34:11 -0400 Subject: [PATCH 11/22] remove comments --- .../importer/services/processors/ExperimentProcessor.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 89d42a263..322f5241d 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 @@ -694,9 +694,6 @@ private void validateObservations(ValidationErrors validationErrors, } else if (this.existingObsByObsHash.containsKey(importHash) && StringUtils.isNotBlank(phenoCol.getString(numRows - rowNum -1)) && !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum -1))) { - //observationByHash.get(importHash).getBrAPIObject().setValue(phenoCol.getString(numRows - rowNum -1)); - //this.existingObsByObsHash.get(importHash).setObservationVariableName(phenoCol.name()); - //observationByHash.get(importHash).setState(ImportObjectState.MUTATED); // add a change log entry when updating the value of an observation if (commit) { @@ -717,7 +714,7 @@ private void validateObservations(ValidationErrors validationErrors, if (pendingObservation.getAdditionalInfo() != null && !pendingObservation.getAdditionalInfo().has(BrAPIAdditionalInfoFields.CHANGELOG)) { pendingObservation.getAdditionalInfo().add(BrAPIAdditionalInfoFields.CHANGELOG, new JsonArray()); } - pendingObservation.getAdditionalInfo().get(BrAPIAdditionalInfoFields.CHANGELOG).getAsJsonArray().add(gson.toJson(change)); + pendingObservation.getAdditionalInfo().get(BrAPIAdditionalInfoFields.CHANGELOG).getAsJsonArray().add(gson.toJsonTree(change).getAsJsonObject()); } // preview case where observation has already been committed and import ObsVar data is either empty or the From d8b8c741822464a3eb79a6622eb5c7ad5bcf8694 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Sun, 22 Oct 2023 22:56:07 -0400 Subject: [PATCH 12/22] code cleanup --- .../processors/ExperimentProcessor.java | 37 +++++++------------ 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 322f5241d..906f73db7 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java @@ -234,7 +234,7 @@ public Map process( } } - List referencedTraits = verifyTraits(program.getId(), phenotypeCols, timestampCols, validationErrors); + List referencedTraits = verifyTraits(program.getId(), phenotypeCols, timestampCols); //Now know timestamps all valid phenotypes, can associate with phenotype column name for easy retrieval for (Column tsColumn : timestampCols) { @@ -254,7 +254,7 @@ public Map process( log.debug("done processing experiment import"); // Construct our response object - return generateStatisticsMap(importRows, phenotypeCols); + return generateStatisticsMap(importRows); } @Override @@ -271,7 +271,7 @@ public void postBrapiData(Map mappedBrAPIImport, Program .getMutationsByObjectId(trialByNameNoScope, BrAPITrial::getTrialDbId); Map mutatedObservationByDbId = ProcessorData - .getMutationsByObjectId(observationByHash, observation -> observation.getObservationDbId()); + .getMutationsByObjectId(observationByHash, BrAPIObservation::getObservationDbId); List newLocations = ProcessorData.getNewObjects(this.locationByName) .stream() @@ -432,7 +432,7 @@ private void prepareDataForValidation(List importRows, List verifyTraits(UUID programId, List> phenotypeCols, List> timestampCols, ValidationErrors validationErrors) { + private List verifyTraits(UUID programId, List> phenotypeCols, List> timestampCols) { Set varNames = phenotypeCols.stream() .map(Column::name) .collect(Collectors.toSet()); @@ -578,8 +578,8 @@ private String getObservationHash(String observationUnitName, String variableNam return DigestUtils.sha256Hex(concat); } - private ValidationErrors validateFields(List importRows, ValidationErrors validationErrors, Map mappedBrAPIImport, List referencedTraits, Program program, - List> phenotypeCols, boolean commit, User user) throws MissingRequiredInfoException, ApiException { + private void validateFields(List importRows, ValidationErrors validationErrors, Map mappedBrAPIImport, List referencedTraits, Program program, + List> phenotypeCols, boolean commit, User user) { //fetching any existing observations for any OUs in the import CaseInsensitiveMap colVarMap = new CaseInsensitiveMap<>(); for ( Trait trait: referencedTraits) { @@ -593,7 +593,7 @@ private ValidationErrors validateFields(List importRows, Validation validateGermplasm(importRow, validationErrors, rowNum, mappedImportRow.getGermplasm()); } validateTestOrCheck(importRow, validationErrors, rowNum); - + //TODO: providing obs unit ID does not supersede import row inout data as expected and needs to be fixed //Check if existing environment. If so, ObsUnitId must be assigned // if ((mappedImportRow.getStudy().getState() == ImportObjectState.EXISTING) // && (StringUtils.isBlank(importRow.getObsUnitID()))) { @@ -604,7 +604,6 @@ private ValidationErrors validateFields(List importRows, Validation validateObservationUnits(validationErrors, uniqueStudyAndObsUnit, rowNum, importRow); validateObservations(validationErrors, rowNum, importRows.size(), importRow, phenotypeCols, colVarMap, commit, user); } - return validationErrors; } private void validateObservationUnits(ValidationErrors validationErrors, Set uniqueStudyAndObsUnit, int rowNum, ExperimentObservation importRow) { @@ -628,7 +627,7 @@ private Map fetchExistingObservations(List refe .map(PendingImportObject::getBrAPIObject) .collect(Collectors.toMap(BrAPIStudy::getStudyDbId, brAPIStudy -> Utilities.removeProgramKeyAndUnknownAdditionalData(brAPIStudy.getStudyName(), program.getKey()))); - studyNameByDbId.keySet().stream().forEach(studyDbId -> { + studyNameByDbId.keySet().forEach(studyDbId -> { try { brAPIObservationUnitDAO.getObservationUnitsForStudyDbId(studyDbId, program).forEach(ou -> { if(StringUtils.isNotBlank(ou.getObservationUnitDbId())) { @@ -640,12 +639,6 @@ private Map fetchExistingObservations(List refe throw new RuntimeException(e); } }); -// observationUnitByNameNoScope.values().forEach(ou -> { -// if(StringUtils.isNotBlank(ou.getBrAPIObject().getObservationUnitDbId())) { -// ouDbIds.add(ou.getBrAPIObject().getObservationUnitDbId()); -// } -// ouNameByDbId.put(ou.getBrAPIObject().getObservationUnitDbId(), Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getBrAPIObject().getObservationUnitName(), program.getKey())); -// }); for (Trait referencedTrait : referencedTraits) { variableDbIds.add(referencedTrait.getObservationVariableDbId()); @@ -803,6 +796,7 @@ private void validateConditionallyRequired(ValidationErrors validationErrors, in addRowError(Columns.OBS_UNIT_ID, "ObsUnitID cannot be specified when creating a new environment", validationErrors, rowNum); } } else { + //TODO: include this step once user-supplied obs unit id correctly supersedes other row data //validateRequiredCell(importRow.getObsUnitID(), Columns.OBS_UNIT_ID, errorMessage, validationErrors, rowNum); } } @@ -826,7 +820,7 @@ private void addIfNotNull(HashSet set, String setValue) { } } - private Map generateStatisticsMap(List importRows, List> phenotypeCols) { + private Map generateStatisticsMap(List importRows) { // Data for stats. HashSet environmentNameCounter = new HashSet<>(); // set of unique environment names HashSet obsUnitsIDCounter = new HashSet<>(); // set of unique observation unit ID's @@ -892,8 +886,8 @@ private Map generateStatisticsMap(List pi } }); } - private PendingImportObject fetchOrCreateDatasetPIO(ExperimentObservation importRow, Program program, List referencedTraits) { + private void fetchOrCreateDatasetPIO(ExperimentObservation importRow, Program program, List referencedTraits) { PendingImportObject pio; PendingImportObject trialPIO = trialByNameNoScope.get(importRow.getExpTitle()); String name = String.format("Observation Dataset [%s-%s]", @@ -1057,7 +1051,6 @@ private PendingImportObject fetchOrCreateDatasetPIO(Experiment obsVarDatasetByName.put(name, pio); } addObsVarsToDatasetDetails(pio, referencedTraits, program); - return pio; } private PendingImportObject fetchOrCreateStudyPIO(Program program, boolean commit, String expSequenceValue, ExperimentObservation importRow, Supplier envNextVal) { @@ -1413,9 +1406,7 @@ private void initializeStudiesForExistingObservationUnits(Program program, Map studies = fetchStudiesByDbId(studyDbIds, program); - studies.forEach(study -> { - processAndCacheStudy(study, program, studyByName); - }); + studies.forEach(study -> processAndCacheStudy(study, program, studyByName)); } private List fetchStudiesByDbId(Set studyDbIds, Program program) throws ApiException { From f274c7be2e0bedd8986bf0c4cde43cc0d8fc6630 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 23 Oct 2023 13:57:16 -0400 Subject: [PATCH 13/22] revert row indexing --- .../processors/ExperimentProcessor.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 906f73db7..b94545d91 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 @@ -602,7 +602,7 @@ private void validateFields(List importRows, ValidationErrors valid validateConditionallyRequired(validationErrors, rowNum, importRow, program, commit); validateObservationUnits(validationErrors, uniqueStudyAndObsUnit, rowNum, importRow); - validateObservations(validationErrors, rowNum, importRows.size(), importRow, phenotypeCols, colVarMap, commit, user); + validateObservations(validationErrors, rowNum, importRow, phenotypeCols, colVarMap, commit, user); } } @@ -662,7 +662,6 @@ private Map fetchExistingObservations(List refe private void validateObservations(ValidationErrors validationErrors, int rowNum, - int numRows, ExperimentObservation importRow, List> phenotypeCols, Map colVarMap, @@ -674,8 +673,8 @@ private void validateObservations(ValidationErrors validationErrors, // error if import observation data already exists and user has not selected to overwrite if(commit && importRow.getOverwrite().equals("false") && this.existingObsByObsHash.containsKey(importHash) && - StringUtils.isNotBlank(phenoCol.getString(numRows - rowNum - 1)) && - !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum - 1))) { + StringUtils.isNotBlank(phenoCol.getString(rowNum)) && + !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(rowNum))) { addRowError( phenoCol.name(), String.format("Value already exists for ObsUnitId: %s, Phenotype: %s", importRow.getObsUnitID(), phenoCol.name()), @@ -685,8 +684,8 @@ private void validateObservations(ValidationErrors validationErrors, // preview case where observation has already been committed and the import row ObsVar data differs from what // had been saved prior to import } else if (this.existingObsByObsHash.containsKey(importHash) && - StringUtils.isNotBlank(phenoCol.getString(numRows - rowNum -1)) && - !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum -1))) { + StringUtils.isNotBlank(phenoCol.getString(rowNum)) && + !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(rowNum))) { // add a change log entry when updating the value of an observation if (commit) { @@ -713,14 +712,14 @@ private void validateObservations(ValidationErrors validationErrors, // preview case where observation has already been committed and import ObsVar data is either empty or the // same as has been committed prior to import } else if(this.existingObsByObsHash.containsKey(importHash) && - (StringUtils.isBlank(phenoCol.getString(numRows - rowNum -1)) || - this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(numRows - rowNum -1)))) { + (StringUtils.isBlank(phenoCol.getString(rowNum)) || + this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(rowNum)))) { BrAPIObservation existingObs = this.existingObsByObsHash.get(importHash); existingObs.setObservationVariableName(phenoCol.name()); observationByHash.get(importHash).setState(ImportObjectState.EXISTING); observationByHash.get(importHash).setBrAPIObject(existingObs); } else { - validateObservationValue(colVarMap.get(phenoCol.name()), phenoCol.getString(numRows - rowNum -1), phenoCol.name(), validationErrors, rowNum); + validateObservationValue(colVarMap.get(phenoCol.name()), phenoCol.getString(rowNum), phenoCol.name(), validationErrors, rowNum); //Timestamp validation if(timeStampColByPheno.containsKey(phenoCol.name())) { From 9a9c33552e712d2dd1afea34ce00a0d5d23daf7e Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 23 Oct 2023 16:46:31 -0400 Subject: [PATCH 14/22] create ChangeLogEntry class --- .../model/imports/ChangeLogEntry.java | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 src/main/java/org/breedinginsight/brapps/importer/model/imports/ChangeLogEntry.java diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/ChangeLogEntry.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/ChangeLogEntry.java new file mode 100644 index 000000000..e2ec109ed --- /dev/null +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/ChangeLogEntry.java @@ -0,0 +1,39 @@ +/* + * 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.model.imports; + +import lombok.Getter; +import lombok.Setter; + +import java.util.UUID; + +@Getter +@Setter +public class ChangeLogEntry { + private String originalValue; + private String reasonForChange; + private UUID changedBy; + private String dateOfChange; + + public ChangeLogEntry(String originalValue, String reasonForChange, UUID changedBy, String dateOfChange) { + this.originalValue = originalValue; + this.reasonForChange = reasonForChange; + this.changedBy = changedBy; + this.dateOfChange = dateOfChange; + } +} From ec839719aed30ab3748a2e05f3bbc0c265ddf182 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 7 Nov 2023 14:50:10 -0500 Subject: [PATCH 15/22] check for adding new OU to existing environment --- .../model/imports/BrAPIImportService.java | 2 +- .../ExperimentImportService.java | 2 +- .../germplasm/GermplasmImportService.java | 2 +- .../sample/SampleSubmissionImportService.java | 2 +- .../importer/services/MappingManager.java | 2 +- .../processors/ExperimentProcessor.java | 17 ++++++++++++----- .../importer/services/processors/Processor.java | 2 +- .../services/processors/ProcessorManager.java | 4 +--- .../importer/ExperimentFileImportTest.java | 3 ++- 9 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java index 2bf412955..1d520371c 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java @@ -49,5 +49,5 @@ default String getWrongUserInputDataTypeMsg(String fieldName, String typeName) { return String.format("User input, \"%s\" must be an %s", fieldName, typeName); } ImportPreviewResponse process(List brAPIImports, Table data, Program program, ImportUpload upload, User user, Boolean commit) - throws UnprocessableEntityException, DoesNotExistException, ValidatorException, ApiException, MissingRequiredInfoException; + throws Exception; } diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentImportService.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentImportService.java index aa425a2da..d438da95c 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentImportService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentImportService.java @@ -70,7 +70,7 @@ public String getMissingColumnMsg(String columnName) { @Override public ImportPreviewResponse process(List brAPIImports, Table data, Program program, ImportUpload upload, User user, Boolean commit) - throws UnprocessableEntityException, ValidatorException, ApiException, MissingRequiredInfoException { + throws Exception { ImportPreviewResponse response = null; List processors = List.of(experimentProcessorProvider.get()); diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/germplasm/GermplasmImportService.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/germplasm/GermplasmImportService.java index e4084eae4..e7b128ff5 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/germplasm/GermplasmImportService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/germplasm/GermplasmImportService.java @@ -66,7 +66,7 @@ public String getImportTypeId() { @Override public ImportPreviewResponse process(List brAPIImports, Table data, Program program, ImportUpload upload, User user, Boolean commit) - throws UnprocessableEntityException, DoesNotExistException, ValidatorException, ApiException, MissingRequiredInfoException { + throws Exception { ImportPreviewResponse response = null; List processors = List.of(germplasmProcessorProvider.get()); diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/sample/SampleSubmissionImportService.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/sample/SampleSubmissionImportService.java index 637fa7ce3..84d8f7499 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/sample/SampleSubmissionImportService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/sample/SampleSubmissionImportService.java @@ -69,7 +69,7 @@ public ImportPreviewResponse process(List brAPIImports, Program program, ImportUpload upload, User user, - Boolean commit) throws UnprocessableEntityException, DoesNotExistException, ValidatorException, ApiException, MissingRequiredInfoException { + Boolean commit) throws Exception { List processors = List.of(sampleProcessorProvider.get()); return processorManagerProvider.get().process(brAPIImports, processors, data, program, upload, user, commit); } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java b/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java index a64705452..fda6d4ef8 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java @@ -351,7 +351,7 @@ private void mapUserInputField(Object parent, Field field, Map u else if (required != null && userInput.containsKey(fieldId) && userInput.get(fieldId).toString().isBlank()) { throw new UnprocessableEntityException(importService.getMissingUserInputMsg(metadata.name())); } - else if (userInput.containsKey(fieldId)) { + else if (userInput != null && userInput.containsKey(fieldId)) { String value = userInput.get(fieldId).toString(); if (!isCorrectType(type.type(), value)) { throw new UnprocessableEntityException(importService.getWrongUserInputDataTypeMsg(metadata.name(), type.type().toString().toLowerCase())); 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 b94545d91..f529b792d 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 @@ -30,6 +30,7 @@ import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.collections4.map.CaseInsensitiveMap; +import org.apache.commons.lang3.StringUtils; import org.brapi.client.v2.JSON; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.v2.model.BrAPIExternalReference; @@ -64,6 +65,7 @@ import org.breedinginsight.services.OntologyService; import org.breedinginsight.services.ProgramLocationService; import org.breedinginsight.services.exceptions.DoesNotExistException; +import org.breedinginsight.services.exceptions.UnprocessableEntityException; import org.breedinginsight.services.exceptions.MissingRequiredInfoException; import org.breedinginsight.services.exceptions.UnprocessableEntityException; import org.breedinginsight.services.exceptions.ValidatorException; @@ -91,6 +93,7 @@ public class ExperimentProcessor implements Processor { private static final String NAME = "Experiment"; + private static final String EXISTING_ENV = "Cannot create a new observation unit for existing environment: "; private static final String MISSING_OBS_UNIT_ID_ERROR = "Experiment Units are missing Observation Unit Id.

" + "If you’re trying to add these units to the experiment, please create a new environment" + " with all appropriate experiment units (NOTE: this will generate new Observation Unit Ids " + @@ -216,7 +219,7 @@ public Map process( Table data, Program program, User user, - boolean commit) throws ValidatorException, MissingRequiredInfoException, ApiException { + boolean commit) throws UnprocessableEntityException, ApiException, ValidatorException { log.debug("processing experiment import"); ValidationErrors validationErrors = new ValidationErrors(); @@ -490,7 +493,7 @@ private String getVariableNameFromColumn(Column column) { return column.name(); } - private void initNewBrapiData(List importRows, List> phenotypeCols, Program program, User user, List referencedTraits, boolean commit) throws ApiException { + private void initNewBrapiData(List importRows, List> phenotypeCols, Program program, User user, List referencedTraits, boolean commit) throws UnprocessableEntityException, ApiException { String expSequenceName = program.getExpSequence(); if (expSequenceName == null) { @@ -671,7 +674,7 @@ private void validateObservations(ValidationErrors validationErrors, var importHash = getImportObservationHash(importRow, phenoCol.name()); // error if import observation data already exists and user has not selected to overwrite - if(commit && importRow.getOverwrite().equals("false") && + if(commit && "false".equals(importRow.getOverwrite() == null ? "false" : importRow.getOverwrite()) && this.existingObsByObsHash.containsKey(importHash) && StringUtils.isNotBlank(phenoCol.getString(rowNum)) && !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(rowNum))) { @@ -915,7 +918,7 @@ private PendingImportObject getGidPOI(ExperimentObservation impo return null; } - private PendingImportObject fetchOrCreateObsUnitPIO(Program program, boolean commit, String envSeqValue, ExperimentObservation importRow) throws ApiException { + private PendingImportObject fetchOrCreateObsUnitPIO(Program program, boolean commit, String envSeqValue, ExperimentObservation importRow) throws UnprocessableEntityException, ApiException { PendingImportObject pio; String key = createObservationUnitKey(importRow); if (this.observationUnitByNameNoScope.containsKey(key)) { @@ -944,7 +947,11 @@ private PendingImportObject fetchOrCreateObsUnitPIO(Progra if (studyPIO.getBrAPIObject().getStudyDbId() != null) { List existingOUs = brAPIObservationUnitDAO.getObservationUnitsForStudyDbId(studyPIO.getBrAPIObject().getStudyDbId(), program); List matchingOU = existingOUs.stream().filter(ou -> importRow.getExpUnitId().equals(Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getObservationUnitName(), program.getKey()))).collect(Collectors.toList()); - pio = !matchingOU.isEmpty() ? new PendingImportObject<>(ImportObjectState.EXISTING, matchingOU.get(0)) : new PendingImportObject<>(ImportObjectState.NEW, newObservationUnit, id); + if (matchingOU.isEmpty()) { + throw new UnprocessableEntityException(EXISTING_ENV + Utilities.removeProgramKeyAndUnknownAdditionalData(studyPIO.getBrAPIObject().getStudyName(), program.getKey())); + } else { + pio = new PendingImportObject<>(ImportObjectState.EXISTING, matchingOU.get(0)); + } } else { pio = new PendingImportObject<>(ImportObjectState.NEW, newObservationUnit, id); } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/Processor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/Processor.java index e4c2e067a..38a3bf8ae 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/Processor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/Processor.java @@ -55,7 +55,7 @@ public interface Processor { Map process(ImportUpload upload, List importRows, Map mappedBrAPIImport, Table data, Program program, User user, boolean commit) - throws ValidatorException, MissingRequiredInfoException, ApiException; + throws Exception; /** * Given mapped brapi import with updates from prior dependencies, check if have everything needed diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ProcessorManager.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ProcessorManager.java index 1f61dabe8..1961b33d1 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ProcessorManager.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ProcessorManager.java @@ -18,7 +18,6 @@ import io.micronaut.context.annotation.Prototype; import lombok.extern.slf4j.Slf4j; -import org.brapi.client.v2.model.exceptions.ApiException; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; import org.breedinginsight.brapps.importer.model.imports.PendingImport; @@ -27,7 +26,6 @@ import org.breedinginsight.brapps.importer.services.ImportStatusService; import org.breedinginsight.model.Program; import org.breedinginsight.model.User; -import org.breedinginsight.services.exceptions.MissingRequiredInfoException; import org.breedinginsight.services.exceptions.ValidatorException; import tech.tablesaw.api.Table; @@ -53,7 +51,7 @@ public ProcessorManager(ImportStatusService statusService) { this.statusService = statusService; } - public ImportPreviewResponse process(List importRows, List processors, Table data, Program program, ImportUpload upload, User user, boolean commit) throws ValidatorException, ApiException, MissingRequiredInfoException { + public ImportPreviewResponse process(List importRows, List processors, Table data, Program program, ImportUpload upload, User user, boolean commit) throws Exception { this.processors = processors; diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 748299daf..af2759f58 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -86,6 +86,7 @@ @TestInstance(TestInstance.Lifecycle.PER_CLASS) @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class ExperimentFileImportTest extends BrAPITest { + private static final String OVERWRITE = "overwrite"; private FannyPack securityFp; private String mappingId; @@ -603,7 +604,7 @@ public void verifyFailureNewOuExistingEnv(boolean commit) { JsonObject result = JsonParser.parseString(upload.body()).getAsJsonObject().getAsJsonObject("result"); assertEquals(422, result.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); - assertTrue(result.getAsJsonObject("progress").get("message").getAsString().startsWith("Experiment Units are missing Observation Unit Id.")); + assertTrue(result.getAsJsonObject("progress").get("message").getAsString().startsWith("Cannot create a new observation unit for existing environment:")); } @Test From bb66651370369b7c046100bde9c9bfb46c5ec0f0 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Wed, 8 Nov 2023 15:40:31 -0500 Subject: [PATCH 16/22] [BI-1830] format pios for display --- .../processors/ExperimentProcessor.java | 36 ++++++++++-------- .../breedinginsight/utilities/Utilities.java | 38 +++++++++++++++++++ 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index f529b792d..deabe9901 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 @@ -76,6 +76,7 @@ import tech.tablesaw.columns.Column; import javax.inject.Inject; +import java.lang.reflect.Field; import java.math.BigDecimal; import java.math.BigInteger; import java.time.OffsetDateTime; @@ -93,7 +94,10 @@ public class ExperimentProcessor implements Processor { private static final String NAME = "Experiment"; - private static final String EXISTING_ENV = "Cannot create a new observation unit for existing environment: "; + private static final String EXISTING_ENV = "Cannot create new observation unit %s for existing environment %s." + + "If you’re trying to add these units to the experiment, please create a new environment" + + " with all appropriate experiment units (NOTE: this will generate new Observation Unit Ids " + + "for each experiment unit)."; private static final String MISSING_OBS_UNIT_ID_ERROR = "Experiment Units are missing Observation Unit Id.

" + "If you’re trying to add these units to the experiment, please create a new environment" + " with all appropriate experiment units (NOTE: this will generate new Observation Unit Ids " + @@ -948,9 +952,10 @@ private PendingImportObject fetchOrCreateObsUnitPIO(Progra List existingOUs = brAPIObservationUnitDAO.getObservationUnitsForStudyDbId(studyPIO.getBrAPIObject().getStudyDbId(), program); List matchingOU = existingOUs.stream().filter(ou -> importRow.getExpUnitId().equals(Utilities.removeProgramKeyAndUnknownAdditionalData(ou.getObservationUnitName(), program.getKey()))).collect(Collectors.toList()); if (matchingOU.isEmpty()) { - throw new UnprocessableEntityException(EXISTING_ENV + Utilities.removeProgramKeyAndUnknownAdditionalData(studyPIO.getBrAPIObject().getStudyName(), program.getKey())); + throw new UnprocessableEntityException(String.format(EXISTING_ENV, importRow.getExpUnitId(), + Utilities.removeProgramKeyAndUnknownAdditionalData(studyPIO.getBrAPIObject().getStudyName(), program.getKey()))); } else { - pio = new PendingImportObject<>(ImportObjectState.EXISTING, matchingOU.get(0)); + pio = new PendingImportObject<>(ImportObjectState.EXISTING, (BrAPIObservationUnit) Utilities.formatBrapiObjForDisplay(matchingOU.get(0), BrAPIObservationUnit.class, program)); } } else { pio = new PendingImportObject<>(ImportObjectState.NEW, newObservationUnit, id); @@ -961,16 +966,17 @@ private PendingImportObject fetchOrCreateObsUnitPIO(Progra } + private void fetchOrCreateObservationPIO(Program program, - User user, - ExperimentObservation importRow, - String variableName, - String value, - String timeStampValue, - boolean commit, - String seasonDbId, - PendingImportObject obsUnitPIO, - List referencedTraits) throws ApiException { + User user, + ExperimentObservation importRow, + String variableName, + String value, + String timeStampValue, + boolean commit, + String seasonDbId, + PendingImportObject obsUnitPIO, + List referencedTraits) throws ApiException { PendingImportObject pio; BrAPIObservation newObservation; String key = getImportObservationHash(importRow, variableName); @@ -982,11 +988,11 @@ private void fetchOrCreateObservationPIO(Program program, // prior observation with updated value newObservation = gson.fromJson(gson.toJson(existingObsByObsHash.get(key)), BrAPIObservation.class); newObservation.setValue(value); - pio = new PendingImportObject<>(ImportObjectState.MUTATED, newObservation); + pio = new PendingImportObject<>(ImportObjectState.MUTATED, (BrAPIObservation) Utilities.formatBrapiObjForDisplay(newObservation, BrAPIObservation.class, program)); } else { // prior observation - pio = new PendingImportObject<>(ImportObjectState.EXISTING, existingObsByObsHash.get(key)); + pio = new PendingImportObject<>(ImportObjectState.EXISTING, (BrAPIObservation) Utilities.formatBrapiObjForDisplay(existingObsByObsHash.get(key), BrAPIObservation.class, program)); } observationByHash.put(key, pio); @@ -1566,7 +1572,7 @@ private void processAndCacheStudy(BrAPIStudy existingStudy, Program program, Map .orElseThrow(() -> new IllegalStateException("External references wasn't found for study (dbid): " + existingStudy.getStudyDbId())); studyByName.put( Utilities.removeProgramKeyAndUnknownAdditionalData(existingStudy.getStudyName(), program.getKey()), - new PendingImportObject<>(ImportObjectState.EXISTING, existingStudy, UUID.fromString(xref.getReferenceID()))); + new PendingImportObject<>(ImportObjectState.EXISTING, (BrAPIStudy) Utilities.formatBrapiObjForDisplay(existingStudy, BrAPIStudy.class, program), UUID.fromString(xref.getReferenceID()))); } private void initializeTrialsForExistingObservationUnits(Program program, Map> trialByName) { diff --git a/src/main/java/org/breedinginsight/utilities/Utilities.java b/src/main/java/org/breedinginsight/utilities/Utilities.java index 7aa390635..4027c1b5d 100644 --- a/src/main/java/org/breedinginsight/utilities/Utilities.java +++ b/src/main/java/org/breedinginsight/utilities/Utilities.java @@ -20,10 +20,12 @@ import org.apache.commons.lang3.StringUtils; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.v2.model.BrAPIExternalReference; +import org.brapi.v2.model.pheno.BrAPIObservationUnit; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.model.Program; import org.flywaydb.core.api.migration.Context; +import java.lang.reflect.Field; import java.sql.ResultSet; import java.sql.Statement; import java.util.ArrayList; @@ -115,6 +117,42 @@ public static String removeProgramKey(String original, String programKey) { return removeProgramKey(original, programKey, null); } + /** + * Remove program key from fields visible on the front end. Mutates the original object and returns it. + * + * @param brapiInstance Object, an instance of a BrAPI Object + * @param brapiClass Class, the BrAPI class + * @param program + * @return Object, BrAPI instance formatted for display + */ + public static Object formatBrapiObjForDisplay(Object brapiInstance, Class brapiClass, Program program) throws RuntimeException { + List displayFields = new ArrayList<>(List.of( + "trialName", + "studyName", + "germplasmName", + "locationName", + "observationUnitName", + "observationVariableName")); + List fields = List.of(brapiClass.getDeclaredFields()); + for (Field field : fields) { + if (displayFields.contains(field.getName())) { + try { + field.setAccessible(true); + + // remove key of form [%s-%s] + String valueSansKeyAndInfo = removeProgramKeyAndUnknownAdditionalData((String) field.get(brapiInstance), program.getKey()); + + //remove key of form [%s] + String valueSansKey = removeProgramKey(valueSansKeyAndInfo, program.getKey()); + field.set(brapiInstance, valueSansKey); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); + } + } + } + return brapiInstance; + } + public static String removeProgramKeyAndUnknownAdditionalData(String original, String programKey) { String keyValueRegEx = String.format(" \\[%s\\-.*\\]", programKey); String stripped = original.replaceAll(keyValueRegEx, ""); From 800234cbc3cc53c1ee97ab6281a783aaccb55cc9 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 9 Nov 2023 11:22:23 -0500 Subject: [PATCH 17/22] map season year to study pio --- .../importer/services/processors/ExperimentProcessor.java | 8 +++++++- .../java/org/breedinginsight/utilities/Utilities.java | 6 +++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index deabe9901..fd6af0504 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 @@ -324,7 +324,7 @@ public void postBrapiData(Map mappedBrAPIImport, Program } List createdLocations = new ArrayList<>(locationService.create(actingUser, program.getId(), newLocations)); - // set the DbId to the for each newly created trial + // set the DbId to the for each newly created location for (ProgramLocation createdLocation : createdLocations) { String createdLocationName = createdLocation.getName(); this.locationByName.get(createdLocationName) @@ -1570,6 +1570,12 @@ private void processAndCacheObservationUnit(BrAPIObservationUnit brAPIObservatio 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())); + // map season dbid to year + String seasonDbId = existingStudy.getSeasons().get(0); // It is assumed that the study has only one season + if(StringUtils.isNotBlank(seasonDbId)) { + String seasonYear = this.seasonDbIdToYear(seasonDbId, program.getId()); + existingStudy.setSeasons(Collections.singletonList(seasonYear)); + } studyByName.put( Utilities.removeProgramKeyAndUnknownAdditionalData(existingStudy.getStudyName(), program.getKey()), new PendingImportObject<>(ImportObjectState.EXISTING, (BrAPIStudy) Utilities.formatBrapiObjForDisplay(existingStudy, BrAPIStudy.class, program), UUID.fromString(xref.getReferenceID()))); diff --git a/src/main/java/org/breedinginsight/utilities/Utilities.java b/src/main/java/org/breedinginsight/utilities/Utilities.java index 4027c1b5d..4b26bdefa 100644 --- a/src/main/java/org/breedinginsight/utilities/Utilities.java +++ b/src/main/java/org/breedinginsight/utilities/Utilities.java @@ -139,11 +139,11 @@ public static Object formatBrapiObjForDisplay(Object brapiInstance, Class brapiC try { field.setAccessible(true); - // remove key of form [%s-%s] + // remove either of possible key formats, [%s-%s] and [%s] String valueSansKeyAndInfo = removeProgramKeyAndUnknownAdditionalData((String) field.get(brapiInstance), program.getKey()); - - //remove key of form [%s] String valueSansKey = removeProgramKey(valueSansKeyAndInfo, program.getKey()); + + // set the value without key or additional info field.set(brapiInstance, valueSansKey); } catch (IllegalAccessException e) { throw new RuntimeException(e); From 41dd3bdf235e62b22746a4b3399451b5c518cc54 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Thu, 9 Nov 2023 18:05:12 -0500 Subject: [PATCH 18/22] [BI-1830] update experiment import test --- .../brapps/importer/ExperimentFileImportTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index af2759f58..f8a022f2b 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -604,7 +604,7 @@ public void verifyFailureNewOuExistingEnv(boolean commit) { JsonObject result = JsonParser.parseString(upload.body()).getAsJsonObject().getAsJsonObject("result"); assertEquals(422, result.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); - assertTrue(result.getAsJsonObject("progress").get("message").getAsString().startsWith("Cannot create a new observation unit for existing environment:")); + assertTrue(result.getAsJsonObject("progress").get("message").getAsString().startsWith("Cannot create a new observation unit")); } @Test From c7740711bffc18bae8b8107350bbf7bf804415fa Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Fri, 10 Nov 2023 10:15:22 -0500 Subject: [PATCH 19/22] update test --- .../brapps/importer/ExperimentFileImportTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index f8a022f2b..e6b64ca49 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -604,7 +604,7 @@ public void verifyFailureNewOuExistingEnv(boolean commit) { JsonObject result = JsonParser.parseString(upload.body()).getAsJsonObject().getAsJsonObject("result"); assertEquals(422, result.getAsJsonObject("progress").get("statuscode").getAsInt(), "Returned data: " + result); - assertTrue(result.getAsJsonObject("progress").get("message").getAsString().startsWith("Cannot create a new observation unit")); + assertTrue(result.getAsJsonObject("progress").get("message").getAsString().startsWith("Cannot create new observation unit")); } @Test From 6890a6bbb6cd544b96566219b2e5a50c69fbf02a Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 21 Nov 2023 12:29:39 -0500 Subject: [PATCH 20/22] add fixes --- .../model/imports/BrAPIImportService.java | 5 --- .../ExperimentImportService.java | 9 ++--- .../germplasm/GermplasmImportService.java | 11 ++---- .../importer/services/MappingManager.java | 14 +++---- .../processors/ExperimentProcessor.java | 39 ++++++++++++------- .../services/processors/Processor.java | 1 - .../breedinginsight/utilities/Utilities.java | 1 - 7 files changed, 39 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java index 1d520371c..0079c13b9 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java @@ -17,15 +17,10 @@ package org.breedinginsight.brapps.importer.model.imports; -import org.brapi.client.v2.model.exceptions.ApiException; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.model.response.ImportPreviewResponse; import org.breedinginsight.model.Program; import org.breedinginsight.model.User; -import org.breedinginsight.services.exceptions.DoesNotExistException; -import org.breedinginsight.services.exceptions.MissingRequiredInfoException; -import org.breedinginsight.services.exceptions.UnprocessableEntityException; -import org.breedinginsight.services.exceptions.ValidatorException; import tech.tablesaw.api.Table; import java.util.List; diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentImportService.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentImportService.java index d438da95c..cd795564a 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentImportService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/experimentObservation/ExperimentImportService.java @@ -18,18 +18,15 @@ package org.breedinginsight.brapps.importer.model.imports.experimentObservation; import lombok.extern.slf4j.Slf4j; -import org.brapi.client.v2.model.exceptions.ApiException; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; import org.breedinginsight.brapps.importer.model.imports.BrAPIImportService; -import org.breedinginsight.brapps.importer.model.imports.germplasm.GermplasmImport; import org.breedinginsight.brapps.importer.model.response.ImportPreviewResponse; -import org.breedinginsight.brapps.importer.services.processors.*; +import org.breedinginsight.brapps.importer.services.processors.ExperimentProcessor; +import org.breedinginsight.brapps.importer.services.processors.Processor; +import org.breedinginsight.brapps.importer.services.processors.ProcessorManager; import org.breedinginsight.model.Program; import org.breedinginsight.model.User; -import org.breedinginsight.services.exceptions.MissingRequiredInfoException; -import org.breedinginsight.services.exceptions.UnprocessableEntityException; -import org.breedinginsight.services.exceptions.ValidatorException; import tech.tablesaw.api.Table; import javax.inject.Inject; diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/germplasm/GermplasmImportService.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/germplasm/GermplasmImportService.java index e7b128ff5..b4eac6b96 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/germplasm/GermplasmImportService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/germplasm/GermplasmImportService.java @@ -18,24 +18,21 @@ package org.breedinginsight.brapps.importer.model.imports.germplasm; import lombok.extern.slf4j.Slf4j; -import org.brapi.client.v2.model.exceptions.ApiException; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; import org.breedinginsight.brapps.importer.model.imports.BrAPIImportService; import org.breedinginsight.brapps.importer.model.response.ImportPreviewResponse; -import org.breedinginsight.brapps.importer.services.processors.*; +import org.breedinginsight.brapps.importer.services.processors.GermplasmProcessor; +import org.breedinginsight.brapps.importer.services.processors.Processor; +import org.breedinginsight.brapps.importer.services.processors.ProcessorManager; import org.breedinginsight.model.Program; import org.breedinginsight.model.User; -import org.breedinginsight.services.exceptions.DoesNotExistException; -import org.breedinginsight.services.exceptions.MissingRequiredInfoException; -import org.breedinginsight.services.exceptions.UnprocessableEntityException; -import org.breedinginsight.services.exceptions.ValidatorException; import tech.tablesaw.api.Table; import javax.inject.Inject; import javax.inject.Provider; import javax.inject.Singleton; -import java.util.*; +import java.util.List; @Singleton @Slf4j diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java b/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java index fda6d4ef8..a3535d13c 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/MappingManager.java @@ -375,14 +375,14 @@ private void checkFieldType(ImportFieldTypeEnum expectedType, String column, Str private Boolean isCorrectType(ImportFieldTypeEnum expectedType, String value) { if (!value.isBlank()) { - try { - if (expectedType == ImportFieldTypeEnum.INTEGER) { - Integer d = Integer.parseInt(value); - } - if (expectedType == ImportFieldTypeEnum.BOOLEAN) { - Boolean b = Boolean.parseBoolean(value); + if (expectedType == ImportFieldTypeEnum.INTEGER) { + try { + Integer.parseInt(value); + } catch (NumberFormatException nfe) { + return false; } - } catch (NumberFormatException nfe) { + } + if (expectedType == ImportFieldTypeEnum.BOOLEAN && !String.valueOf(Boolean.parseBoolean(value)).equals(value)) { return false; } } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index fd6af0504..5c795f123 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 @@ -19,8 +19,6 @@ import com.google.gson.Gson; import com.google.gson.JsonArray; import com.google.gson.JsonObject; -import com.google.gson.JsonElement; -import com.google.gson.JsonObject; import io.micronaut.context.annotation.Property; import io.micronaut.context.annotation.Prototype; import io.micronaut.http.HttpStatus; @@ -49,7 +47,6 @@ import org.breedinginsight.brapi.v2.dao.BrAPIGermplasmDAO; import org.breedinginsight.brapps.importer.daos.*; import org.breedinginsight.brapps.importer.model.ImportUpload; -import org.breedinginsight.brapps.importer.model.base.AdditionalInfo; import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; import org.breedinginsight.brapps.importer.model.imports.ChangeLogEntry; import org.breedinginsight.brapps.importer.model.imports.PendingImport; @@ -70,31 +67,26 @@ import org.breedinginsight.services.exceptions.UnprocessableEntityException; import org.breedinginsight.services.exceptions.ValidatorException; import org.breedinginsight.utilities.Utilities; -import org.checkerframework.checker.nullness.Opt; import org.jooq.DSLContext; import tech.tablesaw.api.Table; import tech.tablesaw.columns.Column; import javax.inject.Inject; -import java.lang.reflect.Field; import java.math.BigDecimal; import java.math.BigInteger; import java.time.OffsetDateTime; import java.time.format.DateTimeFormatter; import java.time.format.DateTimeParseException; import java.util.*; -import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; -import static org.breedinginsight.brapps.importer.services.FileMappingUtil.EXPERIMENT_TEMPLATE_NAME; - @Slf4j @Prototype public class ExperimentProcessor implements Processor { private static final String NAME = "Experiment"; - private static final String EXISTING_ENV = "Cannot create new observation unit %s for existing environment %s." + + private static final String EXISTING_ENV = "Cannot create new observation unit %s for existing environment %s.

" + "If you’re trying to add these units to the experiment, please create a new environment" + " with all appropriate experiment units (NOTE: this will generate new Observation Unit Ids " + "for each experiment unit)."; @@ -107,7 +99,6 @@ public class ExperimentProcessor implements Processor { private static final String TIMESTAMP_PREFIX = "TS:"; private static final String TIMESTAMP_REGEX = "^"+TIMESTAMP_PREFIX+"\\s*"; private static final String COMMA_DELIMITER = ","; - private static final String BLANK_FIELD = "Required field is blank"; 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"; @@ -401,7 +392,16 @@ public void postBrapiData(Map mappedBrAPIImport, Program mutatedObservationByDbId.forEach((id, observation) -> { try { - brAPIObservationDAO.updateBrAPIObservation(id, observation, program.getId()); + BrAPIObservation updatedObs = brAPIObservationDAO.updateBrAPIObservation(id, observation, program.getId()); + if (!observation.getValue().equals(updatedObs.getValue()) || !observation.getObservationTimeStamp().equals(updatedObs.getObservationTimeStamp())) { + String message; + if(!observation.getValue().equals(updatedObs.getValue())) { + message = String.format("Updated observation, %s, from BrAPI service does not match requested update %s.", updatedObs.getValue(), observation.getValue()); + } else { + message = String.format("Updated observation timestamp, %s, from BrAPI service does not match requested update timestamp %s.", updatedObs.getObservationTimeStamp(), observation.getObservationTimeStamp()); + } + throw new Exception(message); + } } catch (ApiException e) { log.error("Error updating observation: " + Utilities.generateApiExceptionLogMessage(e), e); throw new InternalServerException("Error saving experiment import", e); @@ -671,7 +671,7 @@ private void validateObservations(ValidationErrors validationErrors, int rowNum, ExperimentObservation importRow, List> phenotypeCols, - Map colVarMap, + CaseInsensitiveMap colVarMap, boolean commit, User user) { phenotypeCols.forEach(phenoCol -> { @@ -705,6 +705,8 @@ private void validateObservations(ValidationErrors validationErrors, user.getId(), timestamp ); + + // create the changelog field in additonalinfo if it does not already exist if (pendingObservation.getAdditionalInfo().isJsonNull()) { pendingObservation.setAdditionalInfo(new JsonObject()); pendingObservation.getAdditionalInfo().add(BrAPIAdditionalInfoFields.CHANGELOG, new JsonArray()); @@ -713,6 +715,8 @@ private void validateObservations(ValidationErrors validationErrors, if (pendingObservation.getAdditionalInfo() != null && !pendingObservation.getAdditionalInfo().has(BrAPIAdditionalInfoFields.CHANGELOG)) { pendingObservation.getAdditionalInfo().add(BrAPIAdditionalInfoFields.CHANGELOG, new JsonArray()); } + + // add a new entry to the changelog pendingObservation.getAdditionalInfo().get(BrAPIAdditionalInfoFields.CHANGELOG).getAsJsonArray().add(gson.toJsonTree(change).getAsJsonObject()); } @@ -981,13 +985,20 @@ private void fetchOrCreateObservationPIO(Program program, BrAPIObservation newObservation; String key = getImportObservationHash(importRow, variableName); existingObsByObsHash = fetchExistingObservations(referencedTraits, program); + DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'"); + String formattedTimeStampValue = formatter.format(OffsetDateTime.parse(timeStampValue)); if (existingObsByObsHash.containsKey(key)) { if (StringUtils.isNotBlank(value) && - !existingObsByObsHash.get(key).getValue().equals(value)) { + (!existingObsByObsHash.get(key).getValue().equals(value) || + !existingObsByObsHash.get(key).getObservationTimeStamp().equals(formattedTimeStampValue))){ // prior observation with updated value newObservation = gson.fromJson(gson.toJson(existingObsByObsHash.get(key)), BrAPIObservation.class); - newObservation.setValue(value); + if (!existingObsByObsHash.get(key).getValue().equals(value)){ + newObservation.setValue(value); + } else if (!existingObsByObsHash.get(key).getObservationTimeStamp().equals(formattedTimeStampValue)) { + newObservation.setObservationTimeStamp(OffsetDateTime.parse(formattedTimeStampValue)); + } pio = new PendingImportObject<>(ImportObjectState.MUTATED, (BrAPIObservation) Utilities.formatBrapiObjForDisplay(newObservation, BrAPIObservation.class, program)); } else { diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/Processor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/Processor.java index 38a3bf8ae..820626676 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/Processor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/Processor.java @@ -23,7 +23,6 @@ import org.breedinginsight.brapps.importer.model.response.ImportPreviewStatistics; import org.breedinginsight.model.Program; import org.breedinginsight.model.User; -import org.breedinginsight.services.exceptions.MissingRequiredInfoException; import org.breedinginsight.services.exceptions.ValidatorException; import tech.tablesaw.api.Table; diff --git a/src/main/java/org/breedinginsight/utilities/Utilities.java b/src/main/java/org/breedinginsight/utilities/Utilities.java index 4b26bdefa..c885eaa97 100644 --- a/src/main/java/org/breedinginsight/utilities/Utilities.java +++ b/src/main/java/org/breedinginsight/utilities/Utilities.java @@ -20,7 +20,6 @@ import org.apache.commons.lang3.StringUtils; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.v2.model.BrAPIExternalReference; -import org.brapi.v2.model.pheno.BrAPIObservationUnit; import org.breedinginsight.brapps.importer.services.ExternalReferenceSource; import org.breedinginsight.model.Program; import org.flywaydb.core.api.migration.Context; From 44e7b50fbf462c74c9323bf11528cc22ce1d5c78 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Mon, 27 Nov 2023 17:31:23 -0500 Subject: [PATCH 21/22] update observation timestamps --- .../processors/ExperimentProcessor.java | 55 +++++++++++++------ 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java index 5c795f123..e32e42cb0 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 @@ -72,6 +72,7 @@ import tech.tablesaw.columns.Column; import javax.inject.Inject; +import javax.validation.Valid; import java.math.BigDecimal; import java.math.BigInteger; import java.time.OffsetDateTime; @@ -393,7 +394,7 @@ public void postBrapiData(Map mappedBrAPIImport, Program mutatedObservationByDbId.forEach((id, observation) -> { try { BrAPIObservation updatedObs = brAPIObservationDAO.updateBrAPIObservation(id, observation, program.getId()); - if (!observation.getValue().equals(updatedObs.getValue()) || !observation.getObservationTimeStamp().equals(updatedObs.getObservationTimeStamp())) { + if (!observation.getValue().equals(updatedObs.getValue()) || !observation.getObservationTimeStamp().isEqual(updatedObs.getObservationTimeStamp())) { String message; if(!observation.getValue().equals(updatedObs.getValue())) { message = String.format("Updated observation, %s, from BrAPI service does not match requested update %s.", updatedObs.getValue(), observation.getValue()); @@ -675,7 +676,9 @@ private void validateObservations(ValidationErrors validationErrors, boolean commit, User user) { phenotypeCols.forEach(phenoCol -> { - var importHash = getImportObservationHash(importRow, phenoCol.name()); + String importHash = getImportObservationHash(importRow, phenoCol.name()); + String importObsValue = phenoCol.getString(rowNum); + String importObsTimestamp = timeStampColByPheno.get(phenoCol.name()).getString(rowNum); // error if import observation data already exists and user has not selected to overwrite if(commit && "false".equals(importRow.getOverwrite() == null ? "false" : importRow.getOverwrite()) && @@ -690,9 +693,7 @@ private void validateObservations(ValidationErrors validationErrors, // preview case where observation has already been committed and the import row ObsVar data differs from what // had been saved prior to import - } else if (this.existingObsByObsHash.containsKey(importHash) && - StringUtils.isNotBlank(phenoCol.getString(rowNum)) && - !this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(rowNum))) { + } else if (existingObsByObsHash.containsKey(importHash) && !isObservationMatched(importHash, importObsValue, importObsTimestamp)) { // add a change log entry when updating the value of an observation if (commit) { @@ -700,13 +701,21 @@ private void validateObservations(ValidationErrors validationErrors, DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd:hh-mm-ssZ"); String timestamp = formatter.format(OffsetDateTime.now()); String reason = importRow.getOverwriteReason() != null ? importRow.getOverwriteReason() : ""; - ChangeLogEntry change = new ChangeLogEntry(existingObsByObsHash.get(importHash).getValue(), + String prior = ""; + if (isValueMatched(importHash, importObsValue)) { + prior.concat(existingObsByObsHash.get(importHash).getValue()); + } + if (isTimestampMatched(importHash, importObsTimestamp)) { + prior = prior.isEmpty() ? prior : prior.concat(" "); + prior.concat(existingObsByObsHash.get(importHash).getObservationTimeStamp().toString()); + } + ChangeLogEntry change = new ChangeLogEntry(prior, reason, user.getId(), timestamp ); - // create the changelog field in additonalinfo if it does not already exist + // create the changelog field in additional info if it does not already exist if (pendingObservation.getAdditionalInfo().isJsonNull()) { pendingObservation.setAdditionalInfo(new JsonObject()); pendingObservation.getAdditionalInfo().add(BrAPIAdditionalInfoFields.CHANGELOG, new JsonArray()); @@ -722,9 +731,8 @@ private void validateObservations(ValidationErrors validationErrors, // preview case where observation has already been committed and import ObsVar data is either empty or the // same as has been committed prior to import - } else if(this.existingObsByObsHash.containsKey(importHash) && - (StringUtils.isBlank(phenoCol.getString(rowNum)) || - this.existingObsByObsHash.get(importHash).getValue().equals(phenoCol.getString(rowNum)))) { + } else if(existingObsByObsHash.containsKey(importHash) && (StringUtils.isBlank(phenoCol.getString(rowNum)) || + isObservationMatched(importHash, importObsValue, importObsTimestamp))) { BrAPIObservation existingObs = this.existingObsByObsHash.get(importHash); existingObs.setObservationVariableName(phenoCol.name()); observationByHash.get(importHash).setState(ImportObjectState.EXISTING); @@ -969,7 +977,24 @@ private PendingImportObject fetchOrCreateObsUnitPIO(Progra return pio; } + boolean isTimestampMatched(String observationHash, String timeStamp) { + OffsetDateTime priorStamp = existingObsByObsHash.get(observationHash).getObservationTimeStamp(); + if (priorStamp == null) { + return timeStamp == null; + } + return priorStamp.isEqual(OffsetDateTime.parse(timeStamp)); + } + + boolean isValueMatched(String observationHash, String value) { + if (existingObsByObsHash.get(observationHash).getValue() == null) { + return value == null; + } + return existingObsByObsHash.get(observationHash).getValue().equals(value); + } + boolean isObservationMatched(String observationHash, String value, String timeStamp) { + return isTimestampMatched(observationHash, timeStamp) && isValueMatched(observationHash, value); + } private void fetchOrCreateObservationPIO(Program program, User user, @@ -985,18 +1010,16 @@ private void fetchOrCreateObservationPIO(Program program, BrAPIObservation newObservation; String key = getImportObservationHash(importRow, variableName); existingObsByObsHash = fetchExistingObservations(referencedTraits, program); - DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'"); + DateTimeFormatter formatter = DateTimeFormatter.ISO_INSTANT; String formattedTimeStampValue = formatter.format(OffsetDateTime.parse(timeStampValue)); if (existingObsByObsHash.containsKey(key)) { - if (StringUtils.isNotBlank(value) && - (!existingObsByObsHash.get(key).getValue().equals(value) || - !existingObsByObsHash.get(key).getObservationTimeStamp().equals(formattedTimeStampValue))){ + if (StringUtils.isNotBlank(value) && !isObservationMatched(key, value, timeStampValue)){ // prior observation with updated value newObservation = gson.fromJson(gson.toJson(existingObsByObsHash.get(key)), BrAPIObservation.class); - if (!existingObsByObsHash.get(key).getValue().equals(value)){ + if (!isValueMatched(key, value)){ newObservation.setValue(value); - } else if (!existingObsByObsHash.get(key).getObservationTimeStamp().equals(formattedTimeStampValue)) { + } else if (!isTimestampMatched(key, timeStampValue)) { newObservation.setObservationTimeStamp(OffsetDateTime.parse(formattedTimeStampValue)); } pio = new PendingImportObject<>(ImportObjectState.MUTATED, (BrAPIObservation) Utilities.formatBrapiObjForDisplay(newObservation, BrAPIObservation.class, program)); From 4471a0abed9971d8715fab6e666ad6ca86064342 Mon Sep 17 00:00:00 2001 From: dmeidlin <14339308+dmeidlin@users.noreply.github.com> Date: Tue, 5 Dec 2023 14:20:07 -0500 Subject: [PATCH 22/22] fix NPEs --- .../model/imports/BrAPIImportService.java | 5 ++++ .../sample/SampleSubmissionImportService.java | 5 ---- .../processors/ExperimentProcessor.java | 30 +++++++++++-------- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java index 0079c13b9..1d520371c 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/BrAPIImportService.java @@ -17,10 +17,15 @@ package org.breedinginsight.brapps.importer.model.imports; +import org.brapi.client.v2.model.exceptions.ApiException; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.model.response.ImportPreviewResponse; import org.breedinginsight.model.Program; import org.breedinginsight.model.User; +import org.breedinginsight.services.exceptions.DoesNotExistException; +import org.breedinginsight.services.exceptions.MissingRequiredInfoException; +import org.breedinginsight.services.exceptions.UnprocessableEntityException; +import org.breedinginsight.services.exceptions.ValidatorException; import tech.tablesaw.api.Table; import java.util.List; diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/imports/sample/SampleSubmissionImportService.java b/src/main/java/org/breedinginsight/brapps/importer/model/imports/sample/SampleSubmissionImportService.java index 84d8f7499..434626e68 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/imports/sample/SampleSubmissionImportService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/imports/sample/SampleSubmissionImportService.java @@ -18,7 +18,6 @@ package org.breedinginsight.brapps.importer.model.imports.sample; import lombok.extern.slf4j.Slf4j; -import org.brapi.client.v2.model.exceptions.ApiException; import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.brapps.importer.model.imports.BrAPIImport; import org.breedinginsight.brapps.importer.model.imports.BrAPIImportService; @@ -28,10 +27,6 @@ import org.breedinginsight.brapps.importer.services.processors.SampleSubmissionProcessor; import org.breedinginsight.model.Program; import org.breedinginsight.model.User; -import org.breedinginsight.services.exceptions.DoesNotExistException; -import org.breedinginsight.services.exceptions.MissingRequiredInfoException; -import org.breedinginsight.services.exceptions.UnprocessableEntityException; -import org.breedinginsight.services.exceptions.ValidatorException; import tech.tablesaw.api.Table; import javax.inject.Inject; 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 e32e42cb0..d35da62d8 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 @@ -562,7 +562,7 @@ private void initNewBrapiData(List importRows, List> phen } //column.name() gets phenotype name String seasonDbId = this.yearToSeasonDbId(importRow.getEnvYear(), program.getId()); - fetchOrCreateObservationPIO(program, user, importRow, column.name(), column.getString(rowNum), dateTimeValue, commit, seasonDbId, obsUnitPIO, referencedTraits); + fetchOrCreateObservationPIO(program, user, importRow, column, rowNum, dateTimeValue, commit, seasonDbId, obsUnitPIO, referencedTraits); } } } @@ -678,7 +678,6 @@ private void validateObservations(ValidationErrors validationErrors, phenotypeCols.forEach(phenoCol -> { String importHash = getImportObservationHash(importRow, phenoCol.name()); String importObsValue = phenoCol.getString(rowNum); - String importObsTimestamp = timeStampColByPheno.get(phenoCol.name()).getString(rowNum); // error if import observation data already exists and user has not selected to overwrite if(commit && "false".equals(importRow.getOverwrite() == null ? "false" : importRow.getOverwrite()) && @@ -693,7 +692,7 @@ private void validateObservations(ValidationErrors validationErrors, // preview case where observation has already been committed and the import row ObsVar data differs from what // had been saved prior to import - } else if (existingObsByObsHash.containsKey(importHash) && !isObservationMatched(importHash, importObsValue, importObsTimestamp)) { + } else if (existingObsByObsHash.containsKey(importHash) && !isObservationMatched(importHash, importObsValue, phenoCol, rowNum)) { // add a change log entry when updating the value of an observation if (commit) { @@ -705,7 +704,7 @@ private void validateObservations(ValidationErrors validationErrors, if (isValueMatched(importHash, importObsValue)) { prior.concat(existingObsByObsHash.get(importHash).getValue()); } - if (isTimestampMatched(importHash, importObsTimestamp)) { + if (timeStampColByPheno.containsKey(phenoCol.name()) && isTimestampMatched(importHash, timeStampColByPheno.get(phenoCol.name()).getString(rowNum))) { prior = prior.isEmpty() ? prior : prior.concat(" "); prior.concat(existingObsByObsHash.get(importHash).getObservationTimeStamp().toString()); } @@ -732,7 +731,7 @@ private void validateObservations(ValidationErrors validationErrors, // preview case where observation has already been committed and import ObsVar data is either empty or the // same as has been committed prior to import } else if(existingObsByObsHash.containsKey(importHash) && (StringUtils.isBlank(phenoCol.getString(rowNum)) || - isObservationMatched(importHash, importObsValue, importObsTimestamp))) { + isObservationMatched(importHash, importObsValue, phenoCol, rowNum))) { BrAPIObservation existingObs = this.existingObsByObsHash.get(importHash); existingObs.setObservationVariableName(phenoCol.name()); observationByHash.get(importHash).setState(ImportObjectState.EXISTING); @@ -992,15 +991,20 @@ boolean isValueMatched(String observationHash, String value) { return existingObsByObsHash.get(observationHash).getValue().equals(value); } - boolean isObservationMatched(String observationHash, String value, String timeStamp) { - return isTimestampMatched(observationHash, timeStamp) && isValueMatched(observationHash, value); + boolean isObservationMatched(String observationHash, String value, Column phenoCol, Integer rowNum) { + if (timeStampColByPheno.isEmpty() || !timeStampColByPheno.containsKey(phenoCol.name())) { + return isValueMatched(observationHash, value); + } else { + String importObsTimestamp = timeStampColByPheno.get(phenoCol.name()).getString(rowNum); + return isTimestampMatched(observationHash, importObsTimestamp) && isValueMatched(observationHash, value); + } } private void fetchOrCreateObservationPIO(Program program, User user, ExperimentObservation importRow, - String variableName, - String value, + Column column, + Integer rowNum, String timeStampValue, boolean commit, String seasonDbId, @@ -1008,18 +1012,20 @@ private void fetchOrCreateObservationPIO(Program program, List referencedTraits) throws ApiException { PendingImportObject pio; BrAPIObservation newObservation; + String variableName = column.name(); + String value = column.getString(rowNum); String key = getImportObservationHash(importRow, variableName); existingObsByObsHash = fetchExistingObservations(referencedTraits, program); - DateTimeFormatter formatter = DateTimeFormatter.ISO_INSTANT; - String formattedTimeStampValue = formatter.format(OffsetDateTime.parse(timeStampValue)); if (existingObsByObsHash.containsKey(key)) { - if (StringUtils.isNotBlank(value) && !isObservationMatched(key, value, timeStampValue)){ + if (StringUtils.isNotBlank(value) && !isObservationMatched(key, value, column, rowNum)){ // prior observation with updated value newObservation = gson.fromJson(gson.toJson(existingObsByObsHash.get(key)), BrAPIObservation.class); if (!isValueMatched(key, value)){ newObservation.setValue(value); } else if (!isTimestampMatched(key, timeStampValue)) { + DateTimeFormatter formatter = DateTimeFormatter.ISO_INSTANT; + String formattedTimeStampValue = formatter.format(OffsetDateTime.parse(timeStampValue)); newObservation.setObservationTimeStamp(OffsetDateTime.parse(formattedTimeStampValue)); } pio = new PendingImportObject<>(ImportObjectState.MUTATED, (BrAPIObservation) Utilities.formatBrapiObjForDisplay(newObservation, BrAPIObservation.class, program));