diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java index 8bf929855..850ed8138 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java @@ -221,6 +221,7 @@ public HttpResponse>>> getGermplasm( return HttpResponse.status(HttpStatus.UNPROCESSABLE_ENTITY, "Error parsing requested date format"); } } + @Get("/programs/{programId}/germplasm/lists/{listDbId}/export{?fileExtension}") @Produces(value = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet") @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES}) diff --git a/src/main/java/org/breedinginsight/brapps/importer/model/base/Germplasm.java b/src/main/java/org/breedinginsight/brapps/importer/model/base/Germplasm.java index 00d89e4db..de1cf19f1 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/model/base/Germplasm.java +++ b/src/main/java/org/breedinginsight/brapps/importer/model/base/Germplasm.java @@ -155,7 +155,19 @@ public static String constructGermplasmListName(String listName, Program program return String.format("%s [%s-germplasm]", listName, program.getKey()); } - public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID listId, boolean commit, boolean updatePedigree) { + /** + * Will mutate synonym and pedigree fields if changed and meet change criteria + * + * @param germplasm germplasm object + * @param program program + * @param listId list id + * @param commit flag indicating if commit changes should be made + * @param updatePedigree flag indicating if pedigree should be updated + * @return mutated indicator + */ + public boolean updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID listId, boolean commit, boolean updatePedigree) { + + boolean mutated = false; if (updatePedigree) { if (!StringUtils.isBlank(getFemaleParentAccessionNumber())) { @@ -170,6 +182,7 @@ public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID if (!StringUtils.isBlank(getMaleParentEntryNo())) { germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_MALE_PARENT_ENTRY_NO, getMaleParentEntryNo()); } + mutated = true; } // Append synonyms to germplasm that don't already exist @@ -181,6 +194,7 @@ public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID brapiSynonym.setSynonym(synonym); if (!existingSynonyms.contains(brapiSynonym)) { germplasm.addSynonymsItem(brapiSynonym); + mutated = true; } } } @@ -193,6 +207,8 @@ public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID if (commit) { setUpdateCommitFields(germplasm, program.getKey()); } + + return mutated; } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java index 54a1afe06..f3ce7e530 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java @@ -135,7 +135,7 @@ public PendingImportObject constructPendingObservation() { original = observation.getValue(); } - if (!isTimestampMatched()) { + if (!isTimestampMatched() && StringUtils.isNotBlank(timestamp)) { // Update the timestamp DateTimeFormatter formatter = DateTimeFormatter.ISO_INSTANT; String formattedTimeStampValue = formatter.format(observationService.parseDateTime(timestamp)); 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 45eea09e1..97ec21628 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 @@ -288,7 +288,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)) { + if (isChanged(cellData, observation, cell.timestamp, tsColByPheno.containsKey(phenoColumnName))) { // Is prior data protected? /** @@ -398,14 +398,15 @@ public AppendOverwriteMiddlewareContext process(AppendOverwriteMiddlewareContext } } - private boolean isChanged(String cellData, BrAPIObservation observation, String newTimestamp) { + private boolean isChanged(String cellData, BrAPIObservation observation, String newTimestamp, boolean timestampColumnPresent) { if (!cellData.isBlank() && !cellData.equals(observation.getValue())){ return true; } - if (StringUtils.isBlank(newTimestamp)) { - return (observation.getObservationTimeStamp()!=null); + // Only check timestamp if the TS: column was present in the uploaded file and there's a valid timestamp. + if (timestampColumnPresent && !StringUtils.isBlank(newTimestamp)) { + return !observationService.parseDateTime(newTimestamp).equals(observation.getObservationTimeStamp()); } - return !observationService.parseDateTime(newTimestamp).equals(observation.getObservationTimeStamp()); + return false; } /** diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationService.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationService.java index a6ecac2c0..de98202fa 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationService.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/service/ObservationService.java @@ -17,6 +17,7 @@ package org.breedinginsight.brapps.importer.services.processors.experiment.service; +import lombok.extern.slf4j.Slf4j; import org.apache.commons.codec.digest.DigestUtils; import org.apache.commons.lang3.StringUtils; import org.brapi.v2.model.core.BrAPISeason; @@ -42,6 +43,7 @@ import java.util.*; import java.util.stream.Collectors; +@Slf4j @Singleton public class ObservationService { private final ExperimentUtilities experimentUtilities; @@ -115,6 +117,7 @@ public OffsetDateTime parseDateTime(String dateString) { LocalDate localDate = LocalDate.parse(dateString, formatter); return localDate.atStartOfDay().atOffset(ZoneOffset.UTC); } catch (DateTimeParseException ex) { + log.error("Failed to parse timestamp: \"{}\".", dateString); // If both parsing attempts fail, return null return null; } diff --git a/src/main/java/org/breedinginsight/brapps/importer/services/processors/germplasm/GermplasmProcessor.java b/src/main/java/org/breedinginsight/brapps/importer/services/processors/germplasm/GermplasmProcessor.java index e0adba213..50ab5bafd 100644 --- a/src/main/java/org/breedinginsight/brapps/importer/services/processors/germplasm/GermplasmProcessor.java +++ b/src/main/java/org/breedinginsight/brapps/importer/services/processors/germplasm/GermplasmProcessor.java @@ -87,6 +87,8 @@ public class GermplasmProcessor implements Processor { List> postOrder = new ArrayList<>(); BrAPIListNewRequest importList = new BrAPIListNewRequest(); + private int numNewPedigreeConnections = 0; + public static String missingGIDsMsg = "The following GIDs were not found in the database: %s"; public static String missingParentalGIDsMsg = "The following parental GIDs were not found in the database: %s"; public static String missingParentalEntryNoMsg = "The following parental entry numbers were not found in the database: %s"; @@ -333,7 +335,7 @@ public Map process(ImportUpload upload, List
breedingMethods, @@ -361,6 +363,10 @@ private void processNewGermplasm(Germplasm germplasm, ValidationErrors validatio validateGermplasmName(germplasm, i+2, validationErrors); validatePedigree(germplasm, i + 2, validationErrors); + if (germplasm.pedigreeExists()) { + numNewPedigreeConnections++; + } + BrAPIGermplasm newGermplasm = germplasm.constructBrAPIGermplasm(program, breedingMethod, user, commit, BRAPI_REFERENCE_SOURCE, nextVal, importListId); newGermplasmList.add(newGermplasm); @@ -385,6 +391,9 @@ private Germplasm removeBreedingMethodBlanks(Germplasm germplasm) { private boolean processExistingGermplasm(Germplasm germplasm, ValidationErrors validationErrors, List importRows, Program program, UUID importListId, boolean commit, PendingImport mappedImportRow, int rowIndex) { BrAPIGermplasm existingGermplasm; String gid = germplasm.getAccessionNumber(); + boolean mutated = false; + boolean updatePedigree = false; + if (germplasmByAccessionNumber.containsKey(gid)) { existingGermplasm = germplasmByAccessionNumber.get(gid).getBrAPIObject(); // Serialize and deserialize to deep copy @@ -410,17 +419,26 @@ private boolean processExistingGermplasm(Germplasm germplasm, ValidationErrors v } } - if(germplasm.pedigreeExists()) { + // if no existing pedigree and file has pedigree then validate and update + if(germplasm.pedigreeExists() && !hasPedigree(existingGermplasm)) { validatePedigree(germplasm, rowIndex + 2, validationErrors); + updatePedigree = true; } - germplasm.updateBrAPIGermplasm(existingGermplasm, program, importListId, commit, true); - - updatedGermplasmList.add(existingGermplasm); - mappedImportRow.setGermplasm(new PendingImportObject<>(ImportObjectState.MUTATED, existingGermplasm)); - importList.addDataItem(existingGermplasm.getGermplasmName()); + mutated = germplasm.updateBrAPIGermplasm(existingGermplasm, program, importListId, commit, updatePedigree); + if (mutated) { + updatedGermplasmList.add(existingGermplasm); + mappedImportRow.setGermplasm(new PendingImportObject<>(ImportObjectState.MUTATED, existingGermplasm)); + if (updatePedigree) { + numNewPedigreeConnections++; + } + } else { + mappedImportRow.setGermplasm(new PendingImportObject<>(ImportObjectState.EXISTING, existingGermplasm)); + } + // add to list regardless of mutated or not + importList.addDataItem(existingGermplasm.getGermplasmName()); return true; } @@ -523,20 +541,17 @@ private boolean canUpdatePedigreeNoEqualsCheck(BrAPIGermplasm existingGermplasm, germplasm.pedigreeExists(); } - private Map getStatisticsMap(List importRows) { + private Map getStatisticsMap() { ImportPreviewStatistics germplasmStats = ImportPreviewStatistics.builder() .newObjectCount(newGermplasmList.size()) .ignoredObjectCount(germplasmByAccessionNumber.size()) .build(); - //Modified logic here to check for female parent accession number or entry no, removed check for male due to assumption that shouldn't have only male parent - int newObjectCount = newGermplasmList.stream().filter(newGermplasm -> newGermplasm != null).collect(Collectors.toList()).size(); + // TODO: numNewPedigreeConnections is global modified in existing and new flows, refactor at some point ImportPreviewStatistics pedigreeConnectStats = ImportPreviewStatistics.builder() - .newObjectCount(importRows.stream().filter(germplasmImport -> - germplasmImport.getGermplasm() != null && - (germplasmImport.getGermplasm().getFemaleParentAccessionNumber() != null || germplasmImport.getGermplasm().getFemaleParentEntryNo() != null) - ).collect(Collectors.toList()).size()).build(); + .newObjectCount(numNewPedigreeConnections) + .build(); return Map.of( "Germplasm", germplasmStats, @@ -658,7 +673,8 @@ public void postBrapiData(Map mappedBrAPIImport, Program } // Create list - if (!newGermplasmList.isEmpty() || !updatedGermplasmList.isEmpty()) { + // create & update flows both unconditionally add germplasm names to importList so use that for check + if (!importList.getData().isEmpty()) { try { // Create germplasm list brAPIListDAO.createBrAPILists(List.of(importList), program.getId(), upload); diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 8ec7aa907..d58a8d8d2 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -156,7 +156,7 @@ web: logout: url: ${web.base-url} signup: - url-timeout: 60m + url-timeout: 24h signup: url: ${web.base-url}/signup success: diff --git a/src/main/resources/brapi/sql/R__species.sql b/src/main/resources/brapi/sql/R__species.sql index ed7667322..d5d3d62ab 100644 --- a/src/main/resources/brapi/sql/R__species.sql +++ b/src/main/resources/brapi/sql/R__species.sql @@ -13,21 +13,6 @@ -- See the License for the specific language governing permissions and -- limitations under the License. --- 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. - CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; DO $$ @@ -39,6 +24,7 @@ BEGIN • Do it this way so no schema changes are required • Removed the Honey Bee special case because all systems will be starting fresh ------------------------------------------------------------------------------------------ */ + INSERT INTO crop (id, auth_user_id, crop_name) SELECT uuid_generate_v5('9a4deca9-4068-46a3-9efe-db0c181f491a'::uuid, @@ -62,4 +48,4 @@ BEGIN -- Only rewrite the row if name changed UPDATE SET crop_name = EXCLUDED.crop_name WHERE crop.crop_name IS DISTINCT FROM EXCLUDED.crop_name; -END $$; \ No newline at end of file +END $$; diff --git a/src/main/resources/version.properties b/src/main/resources/version.properties index 84ddc5ad1..56ce1b70c 100644 --- a/src/main/resources/version.properties +++ b/src/main/resources/version.properties @@ -14,7 +14,5 @@ # limitations under the License. # - -version=v1.2.0+1045 -versionInfo=https://github.com/Breeding-Insight/bi-api/commit/85abdce79bee7d43816835a1750d4e6668f52d7b - +version=v1.2.0+1055 +versionInfo=https://github.com/Breeding-Insight/bi-api/commit/0a030718b0b626689e4e19a7acf17be481ac6add \ No newline at end of file diff --git a/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java b/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java index 9b9cda353..2a0ad750f 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/GermplasmControllerIntegrationTest.java @@ -42,6 +42,7 @@ import java.util.Map; import java.util.Objects; + import static io.micronaut.http.HttpRequest.GET; import static io.micronaut.http.HttpRequest.POST; import static org.junit.jupiter.api.Assertions.*; @@ -243,6 +244,7 @@ public void getAllGermplasmListsSuccess() { } } } + @ParameterizedTest @CsvSource(value = {"CSV", "XLSX", "XLS"}) @SneakyThrows @@ -275,6 +277,7 @@ public void germplasmListExport(String extension) { int dataSize = download.rowCount(); assertEquals(3, dataSize, "Wrong number of germplasm were returned"); } + @Test @SneakyThrows public void getAllGermplasmByListSuccess() { diff --git a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java index 8275e1cea..8d31d62b7 100644 --- a/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java +++ b/src/test/java/org/breedinginsight/brapps/importer/ExperimentFileImportTest.java @@ -1173,6 +1173,82 @@ public void importNewObsAfterFirstExpWithObs(boolean commit) { } } + /* + Scenario: + - an experiment was created with observations and timestamps + - do a second upload with additional observations for the experiment, but without the timestamp column + - verify the second set of observations get uploaded successfully + */ + @Test + @SneakyThrows + public void importNewObsAfterFirstExpWithObsAndTimestamps() { + log.debug("importNewObsAfterFirstExpWithObsAndTimestamps"); + List traits = importTestUtils.createTraits(2); + Program program = createProgram("Exp with TS and additional Uploads ", "EXTSAU", "EXTSAU", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits); + Map newExp = new HashMap<>(); + newExp.put(Columns.GERMPLASM_GID, "1"); + newExp.put(Columns.TEST_CHECK, "T"); + newExp.put(Columns.EXP_TITLE, "Test Exp"); + newExp.put(Columns.EXP_UNIT, "Plot"); + newExp.put(Columns.EXP_TYPE, "Phenotyping"); + newExp.put(Columns.ENV, "New Env"); + newExp.put(Columns.ENV_LOCATION, "Location A"); + newExp.put(Columns.ENV_YEAR, "2025"); + newExp.put(Columns.EXP_UNIT_ID, "a-1"); + newExp.put(Columns.REP_NUM, "1"); + newExp.put(Columns.BLOCK_NUM, "1"); + newExp.put(Columns.ROW, "1"); + newExp.put(Columns.COLUMN, "1"); + newExp.put(traits.get(0).getObservationVariableName(), "1"); + newExp.put("TS:" + traits.get(0).getObservationVariableName(), "2019-12-19T12:14:50Z"); + + // In the first upload, only 1 trait should be present. + List initialTraits = List.of(traits.get(0)); + importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newExp), initialTraits, false), null, true, client, program, mappingId, newExperimentWorkflowId); + + BrAPITrial brAPITrial = brAPITrialDAO.getTrialsByName(List.of((String)newExp.get(Columns.EXP_TITLE)), program).get(0); + Optional trialIdXref = Utilities.getExternalReference(brAPITrial.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.TRIALS.getName())); + assertTrue(trialIdXref.isPresent()); + BrAPIStudy brAPIStudy = brAPIStudyDAO.getStudiesByExperimentID(UUID.fromString(trialIdXref.get().getReferenceId()), program).get(0); + + BrAPIObservationUnit ou = ouDAO.getObservationUnitsForStudyDbId(brAPIStudy.getStudyDbId(), program).get(0); + Optional ouIdXref = Utilities.getExternalReference(ou.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName())); + assertTrue(ouIdXref.isPresent()); + + Map newObservation = new HashMap<>(); + newObservation.put(Columns.GERMPLASM_GID, "1"); + newObservation.put(Columns.TEST_CHECK, "T"); + newObservation.put(Columns.EXP_TITLE, "Test Exp"); + newObservation.put(Columns.EXP_UNIT, "Plot"); + newObservation.put(Columns.EXP_TYPE, "Phenotyping"); + newObservation.put(Columns.ENV, "New Env"); + newObservation.put(Columns.ENV_LOCATION, "Location A"); + newObservation.put(Columns.ENV_YEAR, "2025"); + newObservation.put(Columns.EXP_UNIT_ID, "a-1"); + newObservation.put(Columns.REP_NUM, "1"); + newObservation.put(Columns.BLOCK_NUM, "1"); + newObservation.put(Columns.ROW, "1"); + newObservation.put(Columns.COLUMN, "1"); + newObservation.put("Plot "+OBSERVATION_UNIT_ID_SUFFIX, ouIdXref.get().getReferenceId()); + newObservation.put(traits.get(0).getObservationVariableName(), "1"); + newObservation.put(traits.get(1).getObservationVariableName(), "1"); + + // Send overwrite parameters in request body to allow the append workflow to work normally. + Map userData = Map.of("overwrite", "true", "overwriteReason", "testing"); + JsonObject result = importTestUtils.uploadAndFetchWorkflow(importTestUtils.writeExperimentDataToFile(List.of(newObservation), traits, true), userData, true, client, program, mappingId, appendOverwriteWorkflowId); + + JsonArray previewRows = result.get("preview").getAsJsonObject().get("rows").getAsJsonArray(); + assertEquals(1, previewRows.size()); + JsonObject row = previewRows.get(0).getAsJsonObject(); + + assertEquals("EXISTING", row.getAsJsonObject("trial").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("location").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("study").get("state").getAsString()); + assertEquals("EXISTING", row.getAsJsonObject("observationUnit").get("state").getAsString()); + assertRowSaved(newObservation, program, traits); + + } + /* Scenario: - Create an experiment with valid observations.