[BI-3053] Nominal observations saved incorrectly to db

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<String, Column<?>> timeStampColByPheno = phenotypeData.getTimeStampColByPheno();
 
+        Optional<Trait> 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<String, BrAPIObservation> 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<BrAPIScaleValidValuesCategories> catego
         return categoryValues.contains(value.toLowerCase());
     }
 
+    //Ensures nominal category is saved with the same casing as defined in ontology
+    public static void matchToDBCategory(List<BrAPIScaleValidValuesCategories> 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<String, Trait> colVarMap,
+                                           Column<?> phenoCol,
+                                           int rowNum,
+                                           ValidationErrors validationErrors,
+                                           boolean commit,
+                                           Map<String, PendingImportObject<BrAPIObservation>> observationByHash,
+                                           String importHash,
+                                           Map<String, Column<?>> 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");
     }

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<PendingImportObject<BrAPIObservation>> 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:<trait> column was present in the uploaded file and there's a valid timestamp.
         if (timestampColumnPresent && !StringUtils.isBlank(newTimestamp)) {

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<Trait> 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;

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);
             }
         });
     }