Merge pull request #486 from Breeding-Insight/release/1.1.1

Merge release/1.1.1 into main

Author: nickpalladino

.env.template

@@ -31,6 +31,10 @@ BRAPI_READ_TIMEOUT=60m
 # Max number of records to POST to the BrAPI service per request
 POST_CHUNK_SIZE=1000
 
+# Request cache records paginating through available records per program by CACHE_BRAPI_FETCH_PAGE_SIZE
+CACHE_PAGINATE_GERMPLASM=false
+CACHE_BRAPI_FETCH_PAGE_SIZE=65000
+
 # BrAPI Server Variables
 BRAPI_SERVER_PORT=8083
 

docker-compose.yml

@@ -71,7 +71,7 @@ services:
     networks:
       - backend
   bidb:
-    image: postgres:11.4
+    image: postgres:17.5
     container_name: bidb
     environment:
       - POSTGRES_DB=${DB_NAME}

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

@@ -222,6 +222,26 @@ public HttpResponse<Response<DataResponse<List<BrAPIGermplasm>>>> getGermplasm(
         }
     }
 
+    @Get("/programs/{programId}/germplasm/lists/{listDbId}/export{?fileExtension}")
+    @Produces(value = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet")
+    @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES})
+    public HttpResponse<StreamedFile> germplasmListExport(
+            @PathVariable("programId") UUID programId, @PathVariable("listDbId") String listDbId, @QueryValue(defaultValue = "XLSX") String fileExtension) {
+        String downloadErrorMessage = "An error occurred while generating the download file. Contact the development team at bidevteam@cornell.edu.";
+        try {
+            FileType extension = Enum.valueOf(FileType.class, fileExtension);
+            DownloadFile germplasmListFile = germplasmService.exportGermplasmList(programId, listDbId, extension);
+            HttpResponse<StreamedFile> germplasmListExport = HttpResponse.ok(germplasmListFile.getStreamedFile()).header(HttpHeaders.CONTENT_DISPOSITION, "attachment;filename="+germplasmListFile.getFileName()+extension.getExtension());
+            return germplasmListExport;
+        }
+        catch (Exception e) {
+            log.info(e.getMessage(), e);
+            e.printStackTrace();
+            HttpResponse response = HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, downloadErrorMessage).contentType(MediaType.TEXT_PLAIN).body(downloadErrorMessage);
+            return response;
+        }
+    }
+
     @Get("/programs/{programId}/germplasm/export{?fileExtension,list}")
     @Produces(value = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet")
     @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.PROGRAM_SCOPED_ROLES})

src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java

@@ -65,6 +65,9 @@ public class BrAPIGermplasmDAO {
     @Property(name = "micronaut.bi.api.run-scheduled-tasks")
     private boolean runScheduledTasks;
 
+    @Property(name = "brapi.paginate.germplasm")
+    private boolean paginateGermplasm;
+
     private final ProgramCache<BrAPIGermplasm> programGermplasmCache;
 
     private final BrAPIEndpointProvider brAPIEndpointProvider;
@@ -141,11 +144,26 @@ private Map<String, BrAPIGermplasm> fetchProgramGermplasm(UUID programId) throws
         BrAPIGermplasmSearchRequest germplasmSearch = new BrAPIGermplasmSearchRequest();
         germplasmSearch.externalReferenceIDs(List.of(programId.toString()));
         germplasmSearch.externalReferenceSources(List.of(Utilities.generateReferenceSource(referenceSource, ExternalReferenceSource.PROGRAMS)));
-        return processGermplasmForDisplay(brAPIDAOUtil.search(
-                api::searchGermplasmPost,
-                api::searchGermplasmSearchResultsDbIdGet,
-                germplasmSearch
-        ), program.getKey());
+
+        if (paginateGermplasm) {
+            log.debug("Fetching germplasm with pagination to BrAPI");
+            return processGermplasmForDisplay(brAPIDAOUtil.search(
+                            api::searchGermplasmPost,
+                            api::searchGermplasmSearchResultsDbIdGet,
+                            germplasmSearch),
+                    program.getKey());
+        } else {
+            log.debug("Fetching germplasm without pagination to BrAPI");
+            return processGermplasmForDisplay(brAPIDAOUtil.searchNoPaging(
+                    api::searchGermplasmPost,
+                    api::searchGermplasmSearchResultsDbIdGet,
+                    germplasmSearch),
+                    program.getKey());
+        }
+    }
+
+    public void repopulateGermplasmCacheForProgram(UUID programId) {
+        programGermplasmCache.populate(programId);
     }
 
     /**
@@ -296,11 +314,8 @@ public List<BrAPIGermplasm> createBrAPIGermplasm(List<BrAPIGermplasm> postBrAPIG
         var program = programDAO.fetchOneById(programId);
         try {
             if (!postBrAPIGermplasmList.isEmpty()) {
-                Callable<Map<String, BrAPIGermplasm>> postFunction = () -> {
                     List<BrAPIGermplasm> postResponse = brAPIDAOUtil.post(postBrAPIGermplasmList, upload, api::germplasmPost, importDAO::update);
-                    return processGermplasmForDisplay(postResponse, program.getKey());
-                };
-                return programGermplasmCache.post(programId, postFunction);
+                    return new ArrayList<>(processGermplasmForDisplay(postResponse, program.getKey()).values());
             }
             return new ArrayList<>();
         } catch (Exception e) {

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

@@ -284,7 +284,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?
                             /**
@@ -394,14 +394,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";
@@ -332,7 +334,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,
@@ -359,6 +361,10 @@ private void processNewGermplasm(Germplasm germplasm, ValidationErrors validatio
 
         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);
@@ -383,6 +389,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
@@ -408,17 +417,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;
     }
 
@@ -521,20 +539,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,
@@ -631,10 +646,13 @@ 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);
+                // Now that we have finished uploading, fetch all the data posted to BrAPI to the cache so it is up-to-date.
+                brAPIGermplasmDAO.repopulateGermplasmCacheForProgram(program.getId());
             } catch (ApiException e) {
                 throw new InternalServerException(e.toString(), e);
             }