From a538e9a3b95bac8cff63b54786bcea09c1a467ea Mon Sep 17 00:00:00 2001 From: HMS17 Date: Fri, 30 Oct 2026 09:17:41 -0400 Subject: [PATCH] [BI-3053] Nominal observations saved incorrectly to db --- .../experiment/ExperimentUtilities.java | 51 +++++++++++++++++-- .../process/ImportTableProcess.java | 15 ++++-- .../PopulateNewPendingImportObjectsStep.java | 5 +- .../ValidatePendingImportObjectsStep.java | 19 ++----- 4 files changed, 67 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java index 44383d2ba..945c4afcb 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java @@ -46,6 +46,7 @@ import org.breedinginsight.brapps.importer.services.processors.experiment.create.model.ProcessedPhenotypeData; import org.breedinginsight.brapps.importer.services.processors.experiment.model.EntityNotFoundException; import org.breedinginsight.brapps.importer.services.processors.experiment.model.ExpImportProcessConstants; +import org.breedinginsight.dao.db.enums.DataType; import org.breedinginsight.model.Program; import org.breedinginsight.model.Scale; import org.breedinginsight.model.Trait; @@ -518,22 +519,32 @@ public static boolean isObservationMatched(ProcessedPhenotypeData phenotypeData, Integer rowNum) { Map> timeStampColByPheno = phenotypeData.getTimeStampColByPheno(); + Optional obsTraitOptional = phenotypeData.getReferencedTraits().stream().filter(trait -> trait.getObservationVariableName().equals(phenoCol.name())).findFirst(); + Trait obsTrait = obsTraitOptional.orElse(null); + if (timeStampColByPheno.isEmpty() || !timeStampColByPheno.containsKey(phenoCol.name())) { - return isValueMatched(pendingData, observationHash, value); + return isValueMatched(pendingData, observationHash, value, obsTrait); } else { String importObsTimestamp = timeStampColByPheno.get(phenoCol.name()).getString(rowNum); - return isTimestampMatched(pendingData, observationHash, importObsTimestamp) && isValueMatched(pendingData, observationHash, value); + return isTimestampMatched(pendingData, observationHash, importObsTimestamp) && isValueMatched(pendingData, observationHash, value, obsTrait); } } // TODO: will have different pending data objects between workflows so not totally reusable as-is // could probably just pass in actual underlying maps - public static boolean isValueMatched(PendingData pendingData, String observationHash, String value) { + public static boolean isValueMatched(PendingData pendingData, String observationHash, String value, Trait trait) { Map existingObsByObsHash = pendingData.getExistingObsByObsHash(); if (!existingObsByObsHash.containsKey(observationHash) || existingObsByObsHash.get(observationHash).getValue() == null) { return value == null; } + + //Nominal observation with different casing should not be treated as a different value + //On commit nominal observations will be matched with casing stored in database, so there will be no change + if (trait != null && trait.getScale().getDataType() == DataType.NOMINAL) { + return existingObsByObsHash.get(observationHash).getValue().equalsIgnoreCase(value); + } + return existingObsByObsHash.get(observationHash).getValue().equals(value); } @@ -634,6 +645,40 @@ public static boolean validCategory(List catego return categoryValues.contains(value.toLowerCase()); } + //Ensures nominal category is saved with the same casing as defined in ontology + public static void matchToDBCategory(List categories, String value, BrAPIObservation pendingObservation) { + for (BrAPIScaleValidValuesCategories category : categories) { + if (category.getValue().equalsIgnoreCase(value)) { + pendingObservation.setValue(category.getValue()); + } + } + } + + //Helper method for repeated code used in both create and overwrite workflows + public static void obsValidationHelper(CaseInsensitiveMap colVarMap, + Column phenoCol, + int rowNum, + ValidationErrors validationErrors, + boolean commit, + Map> observationByHash, + String importHash, + Map> timeStampColByPheno){ + + ExperimentUtilities.validateObservationValue(colVarMap.get(phenoCol.name()), phenoCol.getString(rowNum), phenoCol.name(), validationErrors, rowNum); + + //Match casing for nominal values to value stored in database + if(commit && colVarMap.get(phenoCol.name()).getScale().getDataType() == DataType.NOMINAL) { + ExperimentUtilities.matchToDBCategory(colVarMap.get(phenoCol.name()).getScale().getCategories(), phenoCol.getString(rowNum), observationByHash.get(importHash).getBrAPIObject()); + } + + //Timestamp validation + if(timeStampColByPheno.containsKey(phenoCol.name())) { + Column timeStampCol = timeStampColByPheno.get(phenoCol.name()); + ExperimentUtilities.validateTimeStampValue(timeStampCol.getString(rowNum), timeStampCol.name(), validationErrors, rowNum); + } + + } + public static boolean isNAObservation(String value){ return value.equalsIgnoreCase("NA"); } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java index 9aa92fca7..69b0426b9 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/middleware/process/ImportTableProcess.java @@ -49,6 +49,7 @@ import org.breedinginsight.brapps.importer.services.processors.experiment.service.ObservationVariableService; import org.breedinginsight.brapps.importer.services.processors.experiment.service.StudyService; import org.breedinginsight.brapps.importer.services.processors.experiment.validator.field.FieldValidator; +import org.breedinginsight.dao.db.enums.DataType; import org.breedinginsight.dao.db.tables.pojos.TraitEntity; import org.breedinginsight.model.Program; import org.breedinginsight.model.Trait; @@ -253,7 +254,7 @@ public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext BrAPIObservation observation = gson.fromJson(gson.toJson(observationByObsHash.get(observationHash)), BrAPIObservation.class); // Is there a change to the prior data? - if (isChanged(cellData, observation, cell.timestamp, tsColByPheno.containsKey(phenoColumnName))) { + if (isChanged(cellData, observation, cell.timestamp, tsColByPheno.containsKey(phenoColumnName), traitByPhenoColName.get(column.name()))) { // Is prior data protected? /** @@ -337,6 +338,11 @@ public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext // Construct a pending observation Optional> pendingProcessedData = Optional.ofNullable(processedData.constructPendingObservation()); + //Ensures nominal category is saved with the same casing as defined in ontology + if (pendingProcessedData.isPresent() && context.getImportContext().isCommit() && traitByPhenoColName.get(column.name()).getScale().getDataType() == DataType.NOMINAL) { + ExperimentUtilities.matchToDBCategory(traitByPhenoColName.get(column.name()).getScale().getCategories(), column.getString(rowNum), pendingProcessedData.get().getBrAPIObject()); + } + // Set the new pending observation in the pending import for the row pendingProcessedData.ifPresent(observation -> mappedImportRow.getObservations().add(observation)); @@ -363,9 +369,12 @@ public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext } } - private boolean isChanged(String cellData, BrAPIObservation observation, String newTimestamp, boolean timestampColumnPresent) { + private boolean isChanged(String cellData, BrAPIObservation observation, String newTimestamp, boolean timestampColumnPresent, Trait trait) { if (!cellData.isBlank() && !cellData.equals(observation.getValue())){ - return true; + //Nominal categories are case-insensitive since will be matched to database casing + if (!(trait != null && trait.getScale().getDataType() == DataType.NOMINAL) || !cellData.equalsIgnoreCase(observation.getValue())) { + return true; + } } // Only check timestamp if the TS: column was present in the uploaded file and there's a valid timestamp. if (timestampColumnPresent && !StringUtils.isBlank(newTimestamp)) { diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateNewPendingImportObjectsStep.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateNewPendingImportObjectsStep.java index f2b6ba133..3cf30a9a2 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateNewPendingImportObjectsStep.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/PopulateNewPendingImportObjectsStep.java @@ -604,7 +604,10 @@ private void fetchOrCreateObservationPIO(ProcessedPhenotypeData phenotypeData, // prior observation with updated value newObservation = gson.fromJson(gson.toJson(existingObsByObsHash.get(key)), BrAPIObservation.class); - if (!isValueMatched(pendingData, key, value)){ + Optional obsTraitOptional = phenotypeData.getReferencedTraits().stream().filter(trait -> trait.getObservationVariableName().equals(column.name())).findFirst(); + Trait obsTrait = obsTraitOptional.orElse(null); + + if (!isValueMatched(pendingData, key, value, obsTrait)){ newObservation.setValue(value); } else if (!StringUtils.isBlank(timeStampValue) && !isTimestampMatched(pendingData, key, timeStampValue)) { DateTimeFormatter formatter = DateTimeFormatter.ISO_INSTANT; diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/ValidatePendingImportObjectsStep.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/ValidatePendingImportObjectsStep.java index 1ed8f9f97..5cf05d3f2 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/ValidatePendingImportObjectsStep.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/ValidatePendingImportObjectsStep.java @@ -399,14 +399,7 @@ private void validateObservations(PendingData pendingData, } else if (existingObsByObsHash.containsKey(importHash) && !ExperimentUtilities.isObservationMatched(phenotypeData, pendingData, importHash, importObsValue, phenoCol, rowNum)) { // different data means validations still need to happen - // TODO consider moving these two calls into a separate method since called twice together - ExperimentUtilities.validateObservationValue(colVarMap.get(phenoCol.name()), phenoCol.getString(rowNum), phenoCol.name(), validationErrors, rowNum); - - //Timestamp validation - if(timeStampColByPheno.containsKey(phenoCol.name())) { - Column timeStampCol = timeStampColByPheno.get(phenoCol.name()); - ExperimentUtilities.validateTimeStampValue(timeStampCol.getString(rowNum), timeStampCol.name(), validationErrors, rowNum); - } + ExperimentUtilities.obsValidationHelper(colVarMap, phenoCol, rowNum, validationErrors, commit, observationByHash, importHash, timeStampColByPheno); // add a change log entry when updating the value of an observation // only will update and thereby need change log entry if no error @@ -416,7 +409,7 @@ private void validateObservations(PendingData pendingData, String timestamp = formatter.format(OffsetDateTime.now()); String reason = importRow.getOverwriteReason() != null ? importRow.getOverwriteReason() : ""; String prior = ""; - if (ExperimentUtilities.isValueMatched(pendingData, importHash, importObsValue)) { + if (ExperimentUtilities.isValueMatched(pendingData, importHash, importObsValue, colVarMap.get(phenoCol.name()))) { prior.concat(existingObsByObsHash.get(importHash).getValue()); } if (timeStampColByPheno.containsKey(phenoCol.name()) && ExperimentUtilities.isTimestampMatched(pendingData, importHash, timeStampColByPheno.get(phenoCol.name()).getString(rowNum))) { @@ -455,13 +448,7 @@ private void validateObservations(PendingData pendingData, } else if(!existingObsByObsHash.containsKey(importHash) && (StringUtils.isBlank(phenoCol.getString(rowNum)))) { observationByHash.get(importHash).setState(ImportObjectState.EXISTING); } else { - ExperimentUtilities.validateObservationValue(colVarMap.get(phenoCol.name()), phenoCol.getString(rowNum), phenoCol.name(), validationErrors, rowNum); - - //Timestamp validation - if(timeStampColByPheno.containsKey(phenoCol.name())) { - Column timeStampCol = timeStampColByPheno.get(phenoCol.name()); - ExperimentUtilities.validateTimeStampValue(timeStampCol.getString(rowNum), timeStampCol.name(), validationErrors, rowNum); - } + ExperimentUtilities.obsValidationHelper(colVarMap, phenoCol, rowNum, validationErrors, commit, observationByHash, importHash, timeStampColByPheno); } }); }