From 9e345ee4e55e8d6c28d5bc9b7983845c8f9c7de3 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Mon, 28 Oct 2024 19:05:36 -0400 Subject: [PATCH 1/3] [BI-2344] - used full unique germplasmName full germplasmName includes programKey and GID --- .../v2/services/BrAPIGermplasmService.java | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java index 4824993b6..fab72f768 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java @@ -82,10 +82,15 @@ public Optional getGermplasmByDBID(UUID programId, String germpl return germplasmDAO.getGermplasmByDBID(germplasmId, programId); } - public List> processListData(List germplasm, BrAPIListDetails germplasmList){ + public List> processListData(List germplasm, BrAPIListDetails germplasmList, Program program){ Map germplasmByName = new HashMap<>(); for (BrAPIGermplasm g: germplasm) { - germplasmByName.put(g.getGermplasmName(), g); + // Use the full, unique germplasmName with programKey and accessionNumber (GID) for 2 reasons: + // 1. the BrAPI list items are full names, and + // 2. germplasmNames alone are not unique within a program, this led to unexpected behavior, see BI-2344. + String uniqueGermplasmName = String.format("%s [%s-%s]", g.getGermplasmName(), program.getKey(), g.getAccessionNumber()); + g.setGermplasmName(uniqueGermplasmName); // For later. + germplasmByName.put(uniqueGermplasmName, g); } List> processedData = new ArrayList<>(); @@ -107,8 +112,6 @@ public List> processListData(List germplasm, 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); @@ -217,7 +220,7 @@ private BrAPIGermplasm cloneBrAPIGermplasm(BrAPIGermplasm germplasm) { return (BrAPIGermplasm) gson.fromJson(gson.toJson(germplasm), BrAPIGermplasm.class); } - public DownloadFile exportGermplasm(UUID programId, FileType fileExtension) throws IllegalArgumentException, ApiException, IOException { + public DownloadFile exportGermplasm(UUID programId, FileType fileExtension) throws IllegalArgumentException, ApiException, IOException, DoesNotExistException { List columns = GermplasmFileColumns.getOrderedColumns(); //Retrieve germplasm list data @@ -238,7 +241,8 @@ public DownloadFile exportGermplasm(UUID programId, FileType fileExtension) thro StreamedFile downloadFile; //Convert list data to List> data to pass into file writer - List> processedData = processListData(germplasm, null); + Program program = programService.getById(programId).orElseThrow(() -> new DoesNotExistException("Could not find program: " + programId)); + List> processedData = processListData(germplasm, null, program); if (fileExtension == FileType.CSV){ downloadFile = CSVWriter.writeToDownload(columns, processedData, fileExtension); @@ -249,7 +253,7 @@ public DownloadFile exportGermplasm(UUID programId, FileType fileExtension) thro return new DownloadFile(fileName, downloadFile); } - public DownloadFile exportGermplasmList(UUID programId, String listId, FileType fileExtension) throws IllegalArgumentException, ApiException, IOException { + public DownloadFile exportGermplasmList(UUID programId, String listId, FileType fileExtension) throws IllegalArgumentException, ApiException, IOException, DoesNotExistException { List columns = GermplasmFileColumns.getOrderedColumns(); //Retrieve germplasm list data @@ -260,15 +264,12 @@ public DownloadFile exportGermplasmList(UUID programId, String listId, FileType List germplasm = germplasmDAO.getGermplasmByRawName(germplasmNames, programId); String listName = listData.getListName(); - Optional optionalProgram = programService.getById(programId); - if (optionalProgram.isPresent()) { - Program program = optionalProgram.get(); - listName = removeAppendedKey(listName, program.getKey()); - } + Program program = programService.getById(programId).orElseThrow(() -> new DoesNotExistException("Could not find program: " + programId)); + listName = removeAppendedKey(listName, program.getKey()); String fileName = createFileName(listData, listName); StreamedFile downloadFile; //Convert list data to List> data to pass into file writer - List> processedData = processListData(germplasm, listData); + List> processedData = processListData(germplasm, listData, program); if (fileExtension == FileType.CSV){ downloadFile = CSVWriter.writeToDownload(columns, processedData, fileExtension); From 216a696da044bf9781df9766a7512361d523aec9 Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Tue, 29 Oct 2024 11:05:34 -0400 Subject: [PATCH 2/3] [BI-2344] - strip programKey and accessionNumber from file output --- .../brapi/v2/services/BrAPIGermplasmService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java index fab72f768..69f1e1a0a 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java @@ -117,7 +117,8 @@ public List> processListData(List germplasm, HashMap row = new HashMap<>(); row.put("GID", Integer.valueOf(germplasmEntry.getAccessionNumber())); - row.put("Germplasm Name", germplasmEntry.getGermplasmName()); + // Strip programKey and accessionNumber from germplasmName for the file output. + row.put("Germplasm Name", Utilities.removeProgramKeyAnyAccession(germplasmEntry.getGermplasmName(), program.getKey())); row.put("Breeding Method", germplasmEntry.getAdditionalInfo().get(BrAPIAdditionalInfoFields.GERMPLASM_BREEDING_METHOD).getAsString()); String source = germplasmEntry.getSeedSource(); row.put("Source", source); From 987997b91fbf9288c4ae271cd1b91b11efc3bbec Mon Sep 17 00:00:00 2001 From: mlm483 <128052931+mlm483@users.noreply.github.com> Date: Tue, 29 Oct 2024 14:43:47 -0400 Subject: [PATCH 3/3] [BI-2344] - updated comment --- .../brapi/v2/services/BrAPIGermplasmService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java index 69f1e1a0a..8023327d7 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java @@ -89,7 +89,7 @@ public List> processListData(List germplasm, // 1. the BrAPI list items are full names, and // 2. germplasmNames alone are not unique within a program, this led to unexpected behavior, see BI-2344. String uniqueGermplasmName = String.format("%s [%s-%s]", g.getGermplasmName(), program.getKey(), g.getAccessionNumber()); - g.setGermplasmName(uniqueGermplasmName); // For later. + g.setGermplasmName(uniqueGermplasmName); // Mutate the germplasmName in place for later use. germplasmByName.put(uniqueGermplasmName, g); }