BI-2304 - merging into release/1.0

src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java

@@ -18,7 +18,6 @@
 package org.breedinginsight.brapi.v2.constants;
 
 public final class BrAPIAdditionalInfoFields {
-    public static final String GERMPLASM_LIST_ENTRY_NUMBERS = "listEntryNumbers";
     public static final String GERMPLASM_LIST_ID = "listId";
     public static final String GERMPLASM_RAW_PEDIGREE = "rawPedigree";
     public static final String GERMPLASM_PEDIGREE_BY_NAME = "pedigreeByName";

src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java

@@ -1,5 +1,6 @@
 package org.breedinginsight.brapi.v2.services;
 
+import com.google.gson.Gson;
 import io.micronaut.context.annotation.Property;
 import io.micronaut.http.server.exceptions.InternalServerException;
 import io.micronaut.http.server.types.files.StreamedFile;
@@ -24,14 +25,14 @@
 import org.breedinginsight.services.writers.ExcelWriter;
 import org.breedinginsight.brapi.v2.dao.BrAPIGermplasmDAO;
 import org.breedinginsight.brapps.importer.model.ImportUpload;
+import org.breedinginsight.utilities.Utilities;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
 import java.io.IOException;
 import java.time.OffsetDateTime;
 import java.time.format.DateTimeFormatter;
 import java.util.*;
-import java.util.function.ToIntFunction;
 import java.util.stream.Collectors;
 
 @Slf4j
@@ -44,6 +45,7 @@ public class BrAPIGermplasmService {
     private final BrAPIGermplasmDAO germplasmDAO;
     private final ProgramService programService;
     private final BrAPIListDAO brAPIListDAO;
+    private final Gson gson = new Gson();
 
     @Inject
     public BrAPIGermplasmService(BrAPIListDAO brAPIListDAO, ProgramService programService, BrAPIGermplasmDAO germplasmDAO) {
@@ -80,10 +82,36 @@ public Optional<BrAPIGermplasm> getGermplasmByDBID(UUID programId, String germpl
         return germplasmDAO.getGermplasmByDBID(germplasmId, programId);
     }
 
-    public List<Map<String, Object>> processListData(List<BrAPIGermplasm> germplasm, UUID germplasmListId){
+    public List<Map<String, Object>> processListData(List<BrAPIGermplasm> germplasm, BrAPIListDetails germplasmList){
+        Map<String, BrAPIGermplasm> germplasmByName = new HashMap<>();
+        for (BrAPIGermplasm g: germplasm) {
+            germplasmByName.put(g.getGermplasmName(), g);
+        }
+
         List<Map<String, Object>> processedData =  new ArrayList<>();
 
-        for (BrAPIGermplasm germplasmEntry: germplasm) {
+        // This holds the BrAPI list items or all germplasm in a program if the list is null.
+        List<String> orderedGermplasmNames = new ArrayList<>();
+        if (germplasmList == null) {
+            orderedGermplasmNames = germplasm.stream().sorted((left, right) -> {
+                Integer leftAccessionNumber = Integer.parseInt(left.getAccessionNumber());
+                Integer rightAccessionNumber = Integer.parseInt(right.getAccessionNumber());
+                return leftAccessionNumber.compareTo(rightAccessionNumber);
+            }).map(BrAPIGermplasm::getGermplasmName).collect(Collectors.toList());
+        } else {
+            orderedGermplasmNames = germplasmList.getData();
+        }
+
+        // For export, assign entry number sequentially based on BrAPI list order.
+        int entryNumber = 0;
+        for (String germplasmName: orderedGermplasmNames) {
+            // Increment entryNumber.
+            ++entryNumber;
+            // Strip program key and accession number from germplasm name.
+            germplasmName = Utilities.removeUnknownProgramKey(germplasmName);  // TODO: could move to the germplasmList != null codepath.
+            // Lookup the BrAPI germplasm in the map.
+            BrAPIGermplasm germplasmEntry = germplasmByName.get(germplasmName);
+
             HashMap<String, Object> row = new HashMap<>();
             row.put("GID", Integer.valueOf(germplasmEntry.getAccessionNumber()));
             row.put("Germplasm Name", germplasmEntry.getGermplasmName());
@@ -92,12 +120,11 @@ public List<Map<String, Object>> processListData(List<BrAPIGermplasm> germplasm,
             row.put("Source", source);
 
             // Use the entry number in the list map if generated
-            if(new UUID(0,0).compareTo(germplasmListId) == 0) {
+            if(germplasmList == null) {
                 // Not downloading a real list, use GID (https://breedinginsight.atlassian.net/browse/BI-2266).
                 row.put("Entry No", Integer.valueOf(germplasmEntry.getAccessionNumber()));
             } else {
-                row.put("Entry No", germplasmEntry.getAdditionalInfo()
-                        .getAsJsonObject(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ENTRY_NUMBERS).get(germplasmListId.toString()).getAsInt());
+                row.put("Entry No", entryNumber);
             }
 
             //If germplasm was imported with an external UID, it will be stored in external reference with same source as seed source
@@ -156,13 +183,40 @@ public List<BrAPIGermplasm> getGermplasmByList(UUID programId, String listDbId)
             // get list BrAPI germplasm variables
             List<String> germplasmNames = listResponse.getResult().getData();
             List<BrAPIGermplasm> germplasm = germplasmDAO.getGermplasmByRawName(germplasmNames, programId);
+            Map<String, BrAPIGermplasm> germplasmByName = new HashMap<>();
 
-            // set the list ID in the germplasm additional info
-             germplasm.forEach(g -> g.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ID, listId));
-            return germplasm;
+            for (BrAPIGermplasm g : germplasm) {
+                // set the list ID in the germplasm additional info
+                germplasm.forEach(x -> x.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ID, listId));
+                // Add to map.
+                germplasmByName.put(g.getGermplasmName(), g);
+            }
+
+            // Get the program key.
+            String programKey = programService.getById(programId)
+                    .orElseThrow(ApiException::new)
+                    .getKey();
+
+            // Build list from BrAPI list that preserves ordering and duplicates and assigns sequential entry numbers.
+            List<BrAPIGermplasm> germplasmList = new ArrayList<>();
+            int entryNumber = 0;
+            for (String germplasmName : germplasmNames) {
+                ++entryNumber;
+                BrAPIGermplasm listEntry = cloneBrAPIGermplasm(germplasmByName.get(Utilities.removeProgramKeyAndUnknownAdditionalData(germplasmName, programKey)));
+                // Set entry number.
+                listEntry.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_IMPORT_ENTRY_NUMBER, entryNumber);
+                germplasmList.add(listEntry);
+            }
+
+            return germplasmList;
         } else throw new ApiException();
     }
 
+    private BrAPIGermplasm cloneBrAPIGermplasm(BrAPIGermplasm germplasm) {
+        // Serialize then deserialize to deep copy.
+        return (BrAPIGermplasm) gson.fromJson(gson.toJson(germplasm), BrAPIGermplasm.class);
+    }
+
     public DownloadFile exportGermplasm(UUID programId, FileType fileExtension) throws IllegalArgumentException, ApiException, IOException {
         List<Column> columns = GermplasmFileColumns.getOrderedColumns();
 
@@ -184,7 +238,7 @@ public DownloadFile exportGermplasm(UUID programId, FileType fileExtension) thro
 
         StreamedFile downloadFile;
         //Convert list data to List<Map<String, Object>> data to pass into file writer
-        List<Map<String, Object>> processedData =  processListData(germplasm, new UUID(0,0));
+        List<Map<String, Object>> processedData =  processListData(germplasm, null);
 
         if (fileExtension == FileType.CSV){
             downloadFile = CSVWriter.writeToDownload(columns, processedData, fileExtension);
@@ -205,10 +259,6 @@ public DownloadFile exportGermplasmList(UUID programId, String listId, FileType
         List<String> germplasmNames = listData.getData();
         List<BrAPIGermplasm> germplasm = germplasmDAO.getGermplasmByRawName(germplasmNames, programId);
 
-        //processGermplasmForDisplay, numbers
-        UUID germplasmListId = getGermplasmListId(listData);
-        germplasm.sort(Comparator.comparingInt(getEntryNumber(germplasmListId)));
-
         String listName = listData.getListName();
         Optional<Program> optionalProgram = programService.getById(programId);
         if (optionalProgram.isPresent()) {
@@ -218,7 +268,7 @@ public DownloadFile exportGermplasmList(UUID programId, String listId, FileType
         String fileName = createFileName(listData, listName);
         StreamedFile downloadFile;
         //Convert list data to List<Map<String, Object>> data to pass into file writer
-        List<Map<String, Object>> processedData =  processListData(germplasm, germplasmListId);
+        List<Map<String, Object>> processedData =  processListData(germplasm, listData);
 
         if (fileExtension == FileType.CSV){
             downloadFile = CSVWriter.writeToDownload(columns, processedData, fileExtension);
@@ -254,32 +304,6 @@ private boolean hasListExternalReference(List<BrAPIExternalReference> refs) thro
         return refs.stream().anyMatch(e -> referenceSource.concat("/lists").equals(e.getReferenceSource()));
     }
 
-    private ToIntFunction<BrAPIGermplasm> getEntryNumber(UUID germplasmListId) throws IllegalArgumentException {
-        if(germplasmListId.compareTo(new UUID(0,0)) == 0) {
-            return this::getImportEntryNumber;
-        } else {
-            return g -> getGermplasmListEntryNumber(g, germplasmListId);
-        }
-    }
-
-    private Integer getImportEntryNumber(BrAPIGermplasm g) throws IllegalArgumentException {
-        if(Objects.nonNull(g.getAdditionalInfo()) &&
-                g.getAdditionalInfo().has(BrAPIAdditionalInfoFields.GERMPLASM_IMPORT_ENTRY_NUMBER)) {
-            return g.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_IMPORT_ENTRY_NUMBER).getAsInt();
-        } else {
-            throw new IllegalArgumentException();
-        }
-    }
-    private Integer getGermplasmListEntryNumber(BrAPIGermplasm g, UUID germplasmListId) throws IllegalArgumentException {
-        if(Objects.nonNull(g.getAdditionalInfo()) &&
-                g.getAdditionalInfo().has(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ENTRY_NUMBERS)) {
-            return g.getAdditionalInfo().getAsJsonObject(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ENTRY_NUMBERS)
-                .get(germplasmListId.toString()).getAsInt();
-        } else {
-            throw new IllegalArgumentException();
-        }
-    }
-
     private String createFileName(BrAPIListDetails listData, String listName) {
         //TODO change timestamp to edit date when editing functionality is added
         String timestamp;

src/main/java/org/breedinginsight/brapps/importer/model/base/Germplasm.java

@@ -185,13 +185,6 @@ public void updateBrAPIGermplasm(BrAPIGermplasm germplasm, Program program, UUID
             }
         }
 
-        // Add germplasm to the new list
-        JsonObject listEntryNumbers = germplasm.getAdditionalInfo().getAsJsonObject(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ENTRY_NUMBERS);
-        if(listEntryNumbers == null) {
-            listEntryNumbers = new JsonObject();
-            germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ENTRY_NUMBERS, listEntryNumbers);
-        }
-        listEntryNumbers.addProperty(listId.toString(), entryNo);
         germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_IMPORT_ENTRY_NUMBER, entryNo); //so the preview UI shows correctly
 
         // TODO: figure out why clear this out: brapi-server
@@ -241,9 +234,6 @@ public BrAPIGermplasm constructBrAPIGermplasm(ProgramBreedingMethodEntity breedi
         createdBy.put(BrAPIAdditionalInfoFields.CREATED_BY_USER_ID, user.getId().toString());
         createdBy.put(BrAPIAdditionalInfoFields.CREATED_BY_USER_NAME, user.getName());
         germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.CREATED_BY, createdBy);
-        Map<UUID, String> listEntryNumbers = new HashMap<>();
-        listEntryNumbers.put(listId, entryNo);
-        germplasm.putAdditionalInfoItem(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ENTRY_NUMBERS, listEntryNumbers);
         //TODO: Need to check that the acquisition date it in date format
         //brAPIGermplasm.setAcquisitionDate(pedigreeImport.getGermplasm().getAcquisitionDate());
         germplasm.setCountryOfOriginCode(getCountryOfOrigin());

src/main/java/org/breedinginsight/brapps/importer/services/processors/ExperimentProcessor.java

@@ -972,7 +972,11 @@ private void validateUniqueObsUnits(
             String errorMessage = String.format("The ID (%s) is not unique within the environment(%s)", importRow.getExpUnitId(), importRow.getEnv());
             this.addRowError(Columns.EXP_UNIT_ID, errorMessage, validationErrors, rowNum);
         } else {
-            uniqueStudyAndObsUnit.add(envIdPlusStudyId);
+            //Only want to add valid unique study-obs unit combos
+            //To avoid situations like system counting a null value as a unique combo
+            if (!envIdPlusStudyId.isBlank()) {
+                uniqueStudyAndObsUnit.add(envIdPlusStudyId);
+            }
         }
     }
 

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

@@ -16,6 +16,7 @@
  */
 package org.breedinginsight.brapps.importer.services.processors;
 
+import com.google.gson.Gson;
 import io.micronaut.context.annotation.Property;
 import io.micronaut.context.annotation.Prototype;
 import io.micronaut.http.HttpStatus;
@@ -70,6 +71,7 @@ public class GermplasmProcessor implements Processor {
     private final BrAPIListDAO brAPIListDAO;
     private final DSLContext dsl;
     private final BrAPIGermplasmDAO brAPIGermplasmDAO;
+    private final Gson gson = new Gson();
 
     Map<String, PendingImportObject<BrAPIGermplasm>> germplasmByAccessionNumber = new HashMap<>();
     Map<String, Integer> fileGermplasmByName = new HashMap<>();
@@ -263,6 +265,16 @@ public Map<String, ImportPreviewStatistics> process(ImportUpload upload, List<Br
         Map<String, Integer> entryNumberCounts = new HashMap<>();
         List<String> userProvidedEntryNumbers = new ArrayList<>();
         ValidationErrors validationErrors = new ValidationErrors();
+        // Sort importRows by entry number (if present).
+        importRows.sort((left, right) -> {
+            if (left.getGermplasm().getEntryNo() == null || right.getGermplasm().getEntryNo() == null) {
+                return 0;
+            } else {
+                Integer leftEntryNo = Integer.parseInt(left.getGermplasm().getEntryNo());
+                Integer rightEntryNo = Integer.parseInt(right.getGermplasm().getEntryNo());
+                return leftEntryNo.compareTo(rightEntryNo);
+            }
+        });
         for (int i = 0; i < importRows.size(); i++) {
             log.debug("processing germplasm row: " + (i+1));
             BrAPIImport brapiImport = importRows.get(i);
@@ -373,6 +385,8 @@ private boolean processExistingGermplasm(Germplasm germplasm, ValidationErrors v
         String gid = germplasm.getAccessionNumber();
         if (germplasmByAccessionNumber.containsKey(gid)) {
             existingGermplasm = germplasmByAccessionNumber.get(gid).getBrAPIObject();
+            // Serialize and deserialize to deep copy
+            existingGermplasm = gson.fromJson(gson.toJson(existingGermplasm), BrAPIGermplasm.class);
         } else {
             //should be caught in getExistingBrapiData
             ValidationError ve = new ValidationError("GID", String.format(missingGID, gid), HttpStatus.NOT_FOUND);

src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/ExperimentUtilities.java

@@ -169,13 +169,19 @@ public static String createObservationUnitKey(ExperimentObservation importRow) {
      *
      * This method takes in the name of a study and the name of an observation unit and concatenates them to create a unique key.
      *
+     * If one or both of the inputs is null, returns an empty string since not a valid combination
+     *
      * @param studyName The name of the study
      * @param obsUnitName The name of the observation unit
      * @return A string representing the unique key formed by concatenating the study name and observation unit name
      */
     public static String createObservationUnitKey(String studyName, String obsUnitName) {
         // Concatenate the study name and observation unit name to create the unique key
-        return studyName + obsUnitName;
+        if (studyName != null && obsUnitName != null) {
+            return studyName + obsUnitName;
+        } else {
+            return "";
+        }
     }
 
 // Module/Script-level documentation

src/main/java/org/breedinginsight/brapps/importer/services/processors/experiment/create/workflow/steps/ValidatePendingImportObjectsStep.java

@@ -290,7 +290,11 @@ private void validateUniqueObsUnits(
             String errorMessage = String.format("The ID (%s) is not unique within the environment(%s)", importRow.getExpUnitId(), importRow.getEnv());
             ExperimentUtilities.addRowError(ExperimentObservation.Columns.EXP_UNIT_ID, errorMessage, validationErrors, rowNum);
         } else {
-            uniqueStudyAndObsUnit.add(envIdPlusStudyId);
+            //Only want to add valid unique study-obs unit combos
+            //To avoid situations like system counting a null value as a unique combo
+            if (!envIdPlusStudyId.isBlank()) {
+                uniqueStudyAndObsUnit.add(envIdPlusStudyId);
+            }
         }
     }
 

src/main/java/org/breedinginsight/utilities/response/mappers/GermplasmQueryMapper.java

@@ -31,21 +31,13 @@ public class GermplasmQueryMapper extends AbstractQueryMapper {
 
     public GermplasmQueryMapper() {
         fields = Map.ofEntries(
-                Map.entry("importEntryNumber", (germplasm) ->{
+                Map.entry("importEntryNumber", (germplasm) -> {
                     String entryNumber = null;
                         if (germplasm.getAdditionalInfo() != null) {
                             // if additionalInfo contains the importEntryNumber key then return the value
                             if (germplasm.getAdditionalInfo().has(BrAPIAdditionalInfoFields.GERMPLASM_IMPORT_ENTRY_NUMBER)) {
                                 entryNumber = germplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_IMPORT_ENTRY_NUMBER).getAsString();
                             }
-
-                            // if additionalInfo has both listEntryNumbers and listId keys then return the entry number
-                            // mapped to the listId
-                            if (germplasm.getAdditionalInfo().has(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ENTRY_NUMBERS)
-                                && germplasm.getAdditionalInfo().has(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ID)) {
-                                String listId = germplasm.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ID).getAsString();
-                                entryNumber = germplasm.getAdditionalInfo().getAsJsonObject(BrAPIAdditionalInfoFields.GERMPLASM_LIST_ENTRY_NUMBERS).get(listId).getAsString();
-                            }
                         }
                     return entryNumber;
                 }),

src/main/resources/version.properties

@@ -14,5 +14,5 @@
 # limitations under the License.
 #
 
-version=v0.10.0+825
-versionInfo=https://github.com/Breeding-Insight/bi-api/commit/31fcf3ef2177339facc2b2d86cd7d0c129313570
+version=v0.10.0+827
+versionInfo=https://github.com/Breeding-Insight/bi-api/commit/e21aaa301f64d747e8d522fc012a0dcf8d258762