From 429c7f1fef51225dbd7491e1a85e9bbef8a44cce Mon Sep 17 00:00:00 2001 From: David Randolph Phillips Date: Fri, 6 Oct 2023 11:07:18 -0400 Subject: [PATCH 1/2] [BI-1890] added validateTestOrCheck() method --- .../services/processors/ExperimentProcessor.java | 11 +++++++++++ 1 file changed, 11 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 c9e7f5aa0..fde0f4d72 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 @@ -557,6 +557,7 @@ private ValidationErrors validateFields(List importRows, Validation if (StringUtils.isNotBlank(importRow.getGid())) { // if GID is blank, don't bother to check if it is valid. validateGermplasm(importRow, validationErrors, rowNum, mappedImportRow.getGermplasm()); } + validateTestOrCheck(importRow, validationErrors, rowNum); //Check if existing environment. If so, ObsUnitId must be assigned if ((mappedImportRow.getStudy().getState() == ImportObjectState.EXISTING) @@ -784,6 +785,16 @@ private void validateGermplasm(ExperimentObservation importRow, ValidationErrors } } + private void validateTestOrCheck(ExperimentObservation importRow, ValidationErrors validationErrors, int rowNum) { + String testOrCheck = importRow.getTestOrCheck(); + if ( ! ( testOrCheck==null || testOrCheck.isBlank() + || "C".equalsIgnoreCase(testOrCheck) || "CHECK".equalsIgnoreCase(testOrCheck) + || "T".equalsIgnoreCase(testOrCheck) || "TEST".equalsIgnoreCase(testOrCheck) ) + ){ + addRowError(Columns.TEST_CHECK, String.format("Invalid value (%s)", testOrCheck), validationErrors, rowNum) ; + } + } + private PendingImportObject getGidPOI(ExperimentObservation importRow) { if (this.existingGermplasmByGID.containsKey(importRow.getGid())) { return existingGermplasmByGID.get(importRow.getGid()); From 889a6bf4a5e38e1673f9a373ee0aa6707066407e Mon Sep 17 00:00:00 2001 From: David Randolph Phillips Date: Fri, 6 Oct 2023 11:22:10 -0400 Subject: [PATCH 2/2] [BI-1890] Fixed problems (not bugs) reported by InteliJ --- .../processors/ExperimentProcessor.java | 86 ++++++------------- 1 file changed, 25 insertions(+), 61 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 fde0f4d72..f5a7fe84e 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 @@ -41,7 +41,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.PendingImport; import org.breedinginsight.brapps.importer.model.imports.experimentObservation.ExperimentObservation; @@ -74,8 +73,6 @@ 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 { @@ -113,8 +110,6 @@ public class ExperimentProcessor implements Processor { // used to make the yearsToSeasonDbId() function more efficient private final Map yearToSeasonDbIdCache = new HashMap<>(); - // used to make the seasonDbIdtoYear() function more efficient - private final Map seasonDbIdToYearCache = new HashMap<>(); //These BrapiData-objects are initially populated by the getExistingBrapiData() method, // then updated by the getNewBrapiData() method. @@ -126,13 +121,13 @@ public class ExperimentProcessor implements Processor { // initialized by getExistingBrapiData() ) private Map> observationUnitByNameNoScope = null; - private Map> observationByHash = new HashMap<>(); + private final Map> observationByHash = new HashMap<>(); // existingGermplasmByGID is populated by getExistingBrapiData(), but not updated by the getNewBrapiData() method private Map> existingGermplasmByGID = null; // Associates timestamp columns to associated phenotype column name for ease of storage - private Map> timeStampColByPheno = new HashMap<>(); + private final Map> timeStampColByPheno = new HashMap<>(); @Inject public ExperimentProcessor(DSLContext dsl, @@ -220,7 +215,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 +249,7 @@ public void postBrapiData(Map mappedBrAPIImport, Program List newTrials = ProcessorData.getNewObjects(this.trialByNameNoScope); Map mutatedTrialsById = ProcessorData - .getMutationsByObjectId(trialByNameNoScope, trial -> trial.getTrialDbId()); + .getMutationsByObjectId(trialByNameNoScope, BrAPITrial::getTrialDbId); List newLocations = ProcessorData.getNewObjects(this.locationByName) .stream() @@ -274,7 +269,7 @@ public void postBrapiData(Map mappedBrAPIImport, Program return request; }).collect(Collectors.toList()); Map datasetNewDataById = ProcessorData - .getMutationsByObjectId(obsVarDatasetByName, listDetails -> listDetails.getListDbId()); + .getMutationsByObjectId(obsVarDatasetByName, BrAPIListSummary::getListDbId); List newObservationUnits = ProcessorData.getNewObjects(this.observationUnitByNameNoScope); // filter out observations with no 'value' so they will not be saved @@ -287,9 +282,7 @@ public void postBrapiData(Map mappedBrAPIImport, Program try { List createdDatasets = new ArrayList<>(brAPIListDAO.createBrAPILists(newDatasetRequests, program.getId(), upload)); - createdDatasets.forEach(summary -> { - obsVarDatasetByName.get(summary.getListName()).getBrAPIObject().setListDbId(summary.getListDbId()); - }); + createdDatasets.forEach(summary -> obsVarDatasetByName.get(summary.getListName()).getBrAPIObject().setListDbId(summary.getListDbId())); List createdTrials = new ArrayList<>(brapiTrialDAO.createBrAPITrials(newTrials, program.getId(), upload)); // set the DbId to the for each newly created trial @@ -403,7 +396,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()); @@ -543,8 +536,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) throws MissingRequiredInfoException, ApiException { + private void 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); Map colVarMap = referencedTraits.stream().collect(Collectors.toMap(Trait::getObservationVariableName, Function.identity())); @@ -569,7 +562,6 @@ private ValidationErrors validateFields(List importRows, Validation validateObservationUnits(validationErrors, uniqueStudyAndObsUnit, rowNum, importRow); validateObservations(validationErrors, rowNum, importRow, phenotypeCols, colVarMap, existingObsByObsHash); } - return validationErrors; } private void validateObservationUnits(ValidationErrors validationErrors, Set uniqueStudyAndObsUnit, int rowNum, ExperimentObservation importRow) { @@ -834,20 +826,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()); @@ -865,7 +855,6 @@ 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(); @@ -886,7 +875,7 @@ private void addObsVarsToDatasetDetails(PendingImportObject 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]", @@ -913,7 +902,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) { @@ -941,16 +929,13 @@ private PendingImportObject fetchOrCreateStudyPIO(Program program, b return pio; } - 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) { @@ -1171,7 +1156,6 @@ private Map> initializeObserva private Map> initializeTrialByNameNoScope(Program program, List experimentImportRows) { Map> trialByName = new HashMap<>(); - String programKey = program.getKey(); initializeTrialsForExistingObservationUnits(program, trialByName); @@ -1303,7 +1287,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); @@ -1327,7 +1311,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())); @@ -1574,17 +1558,6 @@ private String yearToSeasonDbId(String year, UUID programId) { return dbID; } - private String seasonDbIdToYear(String seasonDbId, UUID programId) { - String year = null; - if (this.seasonDbIdToYearCache.containsKey(seasonDbId)) { // get it from cache if possible - year = this.seasonDbIdToYearCache.get(seasonDbId); - } else { - year = this.seasonDbIdToYearFromDatabase(seasonDbId, programId); - this.seasonDbIdToYearCache.put(seasonDbId, year); - } - return year; - } - private String yearToSeasonDbIdFromDatabase(String year, UUID programId) { BrAPISeason targetSeason = null; List seasons; @@ -1611,14 +1584,5 @@ private String yearToSeasonDbIdFromDatabase(String year, UUID programId) { return (targetSeason == null) ? null : targetSeason.getSeasonDbId(); } - private String seasonDbIdToYearFromDatabase(String seasonDbId, UUID programId) { - BrAPISeason season = null; - try { - season = this.brAPISeasonDAO.getSeasonById(seasonDbId, programId); - } catch (ApiException e) { - log.error(Utilities.generateApiExceptionLogMessage(e), e); - } - Integer yearInt = (season == null) ? null : season.getYear(); - return (yearInt == null) ? "" : yearInt.toString(); - } + }