merge release/1.1.1 into develop

src/main/java/org/breedinginsight/brapi/v2/BrAPIGermplasmController.java

@@ -221,6 +221,7 @@ public HttpResponse<Response<DataResponse<List<BrAPIGermplasm>>>> 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})

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

src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/appendoverwrite/factory/data/OverwrittenData.java

@@ -135,7 +135,7 @@ public PendingImportObject<BrAPIObservation> 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));

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:<trait> 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;
     }
 
     /**

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

src/main/java/org/breedinginsight/brapps/importer/services/processors/germplasm/GermplasmProcessor.java

@@ -87,6 +87,8 @@ public class GermplasmProcessor implements Processor {
     List<List<BrAPIGermplasm>> 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<String, ImportPreviewStatistics> process(ImportUpload upload, List<Br
         createPostOrder();
 
         // Construct our response object
-        return getStatisticsMap(importRows);
+        return getStatisticsMap();
     }
 
     private void processNewGermplasm(Germplasm germplasm, ValidationErrors validationErrors, Map<String, ProgramBreedingMethodEntity> 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<BrAPIImport> 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<String, ImportPreviewStatistics> getStatisticsMap(List<BrAPIImport> importRows) {
+    private Map<String, ImportPreviewStatistics> 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<Integer, PendingImport> 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);

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:

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 $$;
+END $$;

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

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() {

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<Trait> traits = importTestUtils.createTraits(2);
+        Program program = createProgram("Exp with TS and additional Uploads ", "EXTSAU", "EXTSAU", BRAPI_REFERENCE_SOURCE, createGermplasm(1), traits);
+        Map<String, Object> 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<Trait> 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<BrAPIExternalReference> 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<BrAPIExternalReference> ouIdXref = Utilities.getExternalReference(ou.getExternalReferences(), String.format("%s/%s", BRAPI_REFERENCE_SOURCE, ExternalReferenceSource.OBSERVATION_UNITS.getName()));
+        assertTrue(ouIdXref.isPresent());
+
+        Map<String, Object> 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<String, String> 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.