From bd85ecc0096ad49fc68350a5243ba74db4c9314a Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Tue, 5 Apr 2022 11:04:48 -0400 Subject: [PATCH 1/8] [BI-1339] Germplasm Details page --- .../brapi/v2/GermplasmController.java | 16 ++++++++++ .../brapi/v2/dao/BrAPIGermplasmDAO.java | 29 ++++++++++++++----- .../brapi/v2/dao/ProgramCache.java | 29 ++++++++++--------- .../v2/services/BrAPIGermplasmService.java | 8 +++++ .../brapi/v2/ProgramCacheUnitTest.java | 22 +++++++------- 5 files changed, 73 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java b/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java index 6d73d57ef..4cefdc403 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java @@ -79,4 +79,20 @@ public HttpResponse germplasmListExport( return response; } } + + @Get("/${micronaut.bi.api.version}/programs/{programId}/" + BrapiVersion.BRAPI_V2 + "/germplasm/{germplasmId}") + @Produces(MediaType.APPLICATION_JSON) + @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.ALL}) + public HttpResponse getSingleGermplasm( + @PathVariable("programId") UUID programId, + @PathVariable("germplasmId") String germplasmId) { + try { + log.debug("fetching germ id:" + germplasmId +" for program: " + programId); + return HttpResponse.ok(germplasmService.getGermplasmByUUID(programId, germplasmId)); + } catch (ApiException e) { + log.info(e.getMessage(), e); + return HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, "Error retrieving germplasm"); + } + } + } diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java index eebe17dcc..963d505f8 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java @@ -21,9 +21,11 @@ import io.micronaut.context.annotation.Context; import io.micronaut.context.annotation.Property; import io.micronaut.http.server.exceptions.InternalServerException; +import it.unimi.dsi.fastutil.Hash; import lombok.extern.slf4j.Slf4j; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.client.v2.modules.germplasm.GermplasmApi; +import org.brapi.v2.model.BrAPIExternalReference; import org.brapi.v2.model.germ.BrAPIGermplasm; import org.brapi.v2.model.germ.request.BrAPIGermplasmSearchRequest; import org.breedinginsight.brapi.v2.constants.BrAPIAdditionalInfoFields; @@ -54,7 +56,7 @@ public class BrAPIGermplasmDAO { @Property(name = "brapi.server.reference-source") private String referenceSource; - ProgramCache programGermplasmCache; + ProgramCache programGermplasmCache; @Inject public BrAPIGermplasmDAO(ProgramDAO programDAO, ImportDAO importDAO) { @@ -76,7 +78,7 @@ private void setup() { * @throws ApiException */ public List getGermplasm(UUID programId) throws ApiException { - return programGermplasmCache.get(programId); + return new ArrayList<>(programGermplasmCache.get(programId).values()); } /** @@ -87,7 +89,8 @@ public List getGermplasm(UUID programId) throws ApiException { */ public List getRawGermplasm(UUID programId) throws ApiException { Program program = new Program(programDAO.fetchOneById(programId)); - return programGermplasmCache.get(programId).stream().map(germplasm -> { + List cacheList = new ArrayList<>(programGermplasmCache.get(programId).values()); + return cacheList.stream().map(germplasm -> { germplasm.setGermplasmName(Utilities.appendProgramKey(germplasm.getDefaultDisplayName(), program.getKey(), germplasm.getAccessionNumber())); if(germplasm.getAdditionalInfo() != null && germplasm.getAdditionalInfo() .has(BrAPIAdditionalInfoFields.GERMPLASM_RAW_PEDIGREE)) { @@ -98,7 +101,7 @@ public List getRawGermplasm(UUID programId) throws ApiException }).collect(Collectors.toList()); } - private List fetchProgramGermplasm(UUID programId) throws ApiException { + private Map fetchProgramGermplasm(UUID programId) throws ApiException { GermplasmApi api = new GermplasmApi(programDAO.getCoreClient(programId)); // Set query params and make call @@ -112,8 +115,10 @@ private List fetchProgramGermplasm(UUID programId) throws ApiExc )); } - private List processGermplasmForDisplay(List programGermplasm) { + //todo change + private Map processGermplasmForDisplay(List programGermplasm) { // Process the germplasm + Map programGermplasmMap = new HashMap<>(); log.debug("processing germ for display: " + programGermplasm); Map programGermplasmByFullName = new HashMap<>(); for (BrAPIGermplasm germplasm: programGermplasm) { @@ -153,9 +158,13 @@ private List processGermplasmForDisplay(List pro } germplasm.setPedigree(newPedigreeString); } + + BrAPIExternalReference extRef = germplasm.getExternalReferences().stream().filter(reference -> referenceSource.equals(reference.getReferenceSource())).findFirst().orElseThrow(); + String germplasmId = extRef.getReferenceID(); + programGermplasmMap.put(germplasmId, germplasm); } - return programGermplasm; + return programGermplasmMap; } public List importBrAPIGermplasm(List brAPIGermplasmList, UUID programId, ImportUpload upload) throws ApiException { @@ -171,9 +180,13 @@ public List importBrAPIGermplasm(List brAPIGermp } public List getGermplasmByName(List germplasmNames, UUID programId) throws ApiException { - return programGermplasmCache.get(programId) - .stream() + return new ArrayList<>(programGermplasmCache.get(programId).values()).stream() .filter(brAPIGermplasm -> germplasmNames.contains(brAPIGermplasm.getGermplasmName())) .collect(Collectors.toList()); } + + public BrAPIGermplasm getGermplasmByUUID(String germplasmId, UUID programId) throws ApiException { + return programGermplasmCache.get(programId).get(germplasmId); + } + } diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java b/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java index c51deb36c..2e56b9730 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java @@ -31,19 +31,19 @@ import java.util.stream.Collectors; @Slf4j -public class ProgramCache { +public class ProgramCache { - private final FetchFunction> fetchMethod; + private final FetchFunction> fetchMethod; private final Map programSemaphore = new HashMap<>(); private final Cloner cloner; private final Executor executor = Executors.newCachedThreadPool(); - private final LoadingCache> cache = CacheBuilder.newBuilder() + private final LoadingCache> cache = CacheBuilder.newBuilder() .build(new CacheLoader<>() { @Override - public List load(@NotNull UUID programId) throws Exception { + public Map load(@NotNull UUID programId) throws Exception { try { - List values = fetchMethod.apply(programId); + Map values = fetchMethod.apply(programId); log.debug("cache loading complete.\nprogramId: " + programId); return values; } catch (Exception e) { @@ -54,7 +54,7 @@ public List load(@NotNull UUID programId) throws Exception { } }); - public ProgramCache(FetchFunction> fetchMethod, List keys) { + public ProgramCache(FetchFunction> fetchMethod, List keys) { this.fetchMethod = fetchMethod; this.cloner = new Cloner(); // Populate cache on start up @@ -63,12 +63,12 @@ public ProgramCache(FetchFunction> fetchMethod, List keys) { } } - public ProgramCache(FetchFunction> fetchMethod) { + public ProgramCache(FetchFunction> fetchMethod) { this.fetchMethod = fetchMethod; this.cloner = new Cloner(); } - public List get(UUID programId) throws ApiException { + public Map get(UUID programId) throws ApiException { try { // This will get current cache data, or wait for the refresh to finish if there is no cache data. // TODO: Do we want to wait for a refresh method if it is running? Returns current data right now, even if old @@ -76,14 +76,16 @@ public List get(UUID programId) throws ApiException { // If the cache is missing, refresh and get log.trace("cache miss, fetching from source.\nprogramId: " + programId); updateCache(programId); - List result = new ArrayList<>(cache.get(programId)); - result = result.stream().map(cloner::deepClone).collect(Collectors.toList()); + Map result = new HashMap<>(cache.get(programId)); + result = result.entrySet().stream().map(cloner::deepClone) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); return result; } else { log.trace("cache contains records for the program.\nprogramId: " + programId); // Most cases where the cache is populated - List result = new ArrayList<>(cache.get(programId)); - result = result.stream().map(cloner::deepClone).collect(Collectors.toList()); + Map result = new HashMap<>(cache.get(programId)); + result = result.entrySet().stream().map(cloner::deepClone) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); return result; } } catch (ExecutionException e) { @@ -120,9 +122,10 @@ private void updateCache(UUID programId) { } } + //data posted still needs to be list public List post(UUID programId, Callable> postMethod) throws Exception { List response = postMethod.call(); - updateCache(programId); + updateCache(programId); //will make get return response; } } 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 0d745846f..6f5372ba6 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java @@ -58,6 +58,14 @@ public List getGermplasm(UUID programId) throws ApiException { } } + public BrAPIGermplasm getGermplasmByUUID(UUID programId, String germplasmId) throws ApiException { + try { + return germplasmDAO.getGermplasmByUUID(germplasmId, programId); + } catch (ApiException e) { + throw new InternalServerException(e.getMessage(), e); + } + } + public List getGermplasmListsByProgramId(UUID programId, HttpRequest request) throws DoesNotExistException, ApiException { if (!programService.exists(programId)) { diff --git a/src/test/java/org/breedinginsight/brapi/v2/ProgramCacheUnitTest.java b/src/test/java/org/breedinginsight/brapi/v2/ProgramCacheUnitTest.java index fcc11b379..534a75298 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/ProgramCacheUnitTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/ProgramCacheUnitTest.java @@ -20,6 +20,7 @@ import javax.inject.Inject; import java.util.*; import java.util.concurrent.Callable; +import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; @@ -57,10 +58,11 @@ void setupNextTest() { } @SneakyThrows - public List mockFetch(UUID programId, Integer sleepTime) { + public Map mockFetch(UUID programId, Integer sleepTime) { fetchCount += 1; Thread.sleep(sleepTime); - return mockBrAPI.containsKey(programId) ? new ArrayList<>(mockBrAPI.get(programId)) : new ArrayList<>(); + //Complex to retrieve UID so using DbId as dummy + return mockBrAPI.containsKey(programId) ? new HashMap<>(mockBrAPI.get(programId).stream().collect(Collectors.toMap(BrAPIGermplasm::getGermplasmDbId, germplasm -> germplasm))) : new HashMap<>(); //fxn to return map } @SneakyThrows @@ -78,7 +80,7 @@ public List mockPost(UUID programId, List germpl @SneakyThrows public void populatedRefreshQueueSkipsRefresh() { // Make a lot of post calls and just how many times the fetch method is called - ProgramCache cache = new ProgramCache<>((UUID id) -> mockFetch(id, waitTime)); + ProgramCache cache = new ProgramCache<>((UUID id) -> mockFetch(id, waitTime)); UUID programId = UUID.randomUUID(); int numPost = 10; int currPost = 0; @@ -91,7 +93,7 @@ public void populatedRefreshQueueSkipsRefresh() { assertTrue(fetchCount < numPost, "A fetch call was made for every post. It shouldn't."); assertEquals(1, mockBrAPI.size(), "More than one program existed in mocked brapi db."); assertEquals(numPost, mockBrAPI.get(programId).size(), "Wrong number of germplasm in db"); - List cachedGermplasm = cache.get(programId); + Map cachedGermplasm = cache.get(programId); assertEquals(numPost, cachedGermplasm.size(), "Wrong number of germplasm in cache"); } @@ -99,7 +101,7 @@ public void populatedRefreshQueueSkipsRefresh() { @SneakyThrows public void programRefreshesSeparated() { // Make a lot of post calls on different programs to check that they don't wait for each other - ProgramCache cache = new ProgramCache<>((UUID id) -> mockFetch(id, waitTime)); + ProgramCache cache = new ProgramCache<>((UUID id) -> mockFetch(id, waitTime)); int numPost = 10; int currPost = 0; while (currPost < numPost) { @@ -124,7 +126,7 @@ public void programRefreshesSeparated() { public void initialGetMethodWaitsForLoad() { // Test that the get method waits for an ongoing refresh to finish when there isn't any day UUID programId = UUID.randomUUID(); - ProgramCache cache = new ProgramCache<>((UUID id) -> mockFetch(id, waitTime), List.of(programId)); + ProgramCache cache = new ProgramCache<>((UUID id) -> mockFetch(id, waitTime), List.of(programId)); cache.get(programId); // Our fetch method should have only been called once for the initial loading assertEquals(1, fetchCount, "Fetch method was called on get"); @@ -138,11 +140,11 @@ public void getMethodDoesNotWaitForRefresh() { List newList = new ArrayList<>(); newList.add(new BrAPIGermplasm()); mockBrAPI.put(programId, new ArrayList<>(newList)); - ProgramCache cache = new ProgramCache<>((UUID id) -> mockFetch(id, waitTime), List.of(programId)); + ProgramCache cache = new ProgramCache<>((UUID id) -> mockFetch(id, waitTime), List.of(programId)); Callable> postFunction = () -> mockPost(programId, new ArrayList<>(newList)); // Get waits for initial fetch - List cachedGermplasm = cache.get(programId); + Map cachedGermplasm = cache.get(programId); assertEquals(1, cachedGermplasm.size(), "Initial germplasm not as expected"); // Now post another object and call get immediately to see that it returns the old data @@ -173,10 +175,10 @@ public void refreshErrorInvalidatesCache() { ProgramCacheUnitTest mockTest = spy(this); // Start cache - ProgramCache cache = new ProgramCache<>((UUID id) -> mockTest.mockFetch(programId, waitTime), List.of(programId)); + ProgramCache cache = new ProgramCache<>((UUID id) -> mockTest.mockFetch(programId, waitTime), List.of(programId)); // Get waits for initial fetch - List cachedGermplasm = cache.get(programId); + Map cachedGermplasm = cache.get(programId); assertEquals(1, cachedGermplasm.size(), "Initial germplasm not as expected"); // Change our fetch method to throw an error now From 0829e5c826a96c169110eeb5826ce530d0f22fc8 Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Wed, 6 Apr 2022 16:50:24 -0400 Subject: [PATCH 2/8] Germplasm Retrieval and Display --- .../org/breedinginsight/brapi/v2/BrAPIV2Controller.java | 1 + .../org/breedinginsight/brapi/v2/GermplasmController.java | 3 +-- .../brapi/v2/constants/BrAPIAdditionalInfoFields.java | 1 + .../breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java | 7 ++++++- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java index f058d1307..205768784 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java @@ -33,6 +33,7 @@ import org.brapi.v2.model.core.BrAPIServerInfo; import org.brapi.v2.model.core.response.BrAPIListsListResponse; import org.brapi.v2.model.core.response.BrAPIServerInfoResponse; +import org.brapi.v2.model.germ.BrAPIGermplasm; import org.breedinginsight.api.auth.AuthenticatedUser; import org.breedinginsight.api.auth.ProgramSecured; import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; diff --git a/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java b/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java index 4cefdc403..d4d115296 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java @@ -80,7 +80,7 @@ public HttpResponse germplasmListExport( } } - @Get("/${micronaut.bi.api.version}/programs/{programId}/" + BrapiVersion.BRAPI_V2 + "/germplasm/{germplasmId}") + @Get("/${micronaut.bi.api.version}/programs/{programId}" + BrapiVersion.BRAPI_V2 + "/germplasm/{germplasmId}") @Produces(MediaType.APPLICATION_JSON) @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.ALL}) public HttpResponse getSingleGermplasm( @@ -94,5 +94,4 @@ public HttpResponse getSingleGermplasm( return HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, "Error retrieving germplasm"); } } - } diff --git a/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java b/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java index 4fab27f7d..13eb6f9b8 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java +++ b/src/main/java/org/breedinginsight/brapi/v2/constants/BrAPIAdditionalInfoFields.java @@ -19,4 +19,5 @@ public final class BrAPIAdditionalInfoFields { public static final String GERMPLASM_RAW_PEDIGREE = "rawPedigree"; + public static final String GERMPLASM_PEDIGREE_BY_NAME = "pedigreeByName"; } diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java index 963d505f8..4e61b1c0b 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java @@ -115,7 +115,6 @@ private Map fetchProgramGermplasm(UUID programId) throws )); } - //todo change private Map processGermplasmForDisplay(List programGermplasm) { // Process the germplasm Map programGermplasmMap = new HashMap<>(); @@ -144,18 +143,24 @@ private Map processGermplasmForDisplay(List parents = Arrays.asList(germplasm.getPedigree().split("/")); if (parents.size() >= 1) { if (programGermplasmByFullName.containsKey(parents.get(0))) { //problem, if parent not in germplasmbyfullname, not replaced by accession number, assumption that parents are also in file newPedigreeString = programGermplasmByFullName.get(parents.get(0)).getAccessionNumber(); + namePedigreeString = programGermplasmByFullName.get(parents.get(0)).getDefaultDisplayName(); } } if (parents.size() == 2) { if (programGermplasmByFullName.containsKey(parents.get(1))) { newPedigreeString += "/" + programGermplasmByFullName.get(parents.get(1)).getAccessionNumber(); + namePedigreeString += "/" + programGermplasmByFullName.get(parents.get(1)).getDefaultDisplayName(); } } + //For use in individual germplasm display + additionalInfo.addProperty(BrAPIAdditionalInfoFields.GERMPLASM_PEDIGREE_BY_NAME, namePedigreeString); + germplasm.setPedigree(newPedigreeString); } From a2840dfcef18358c45203e130ededa9569c1b4f2 Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Wed, 6 Apr 2022 17:04:13 -0400 Subject: [PATCH 3/8] Cleanup --- .../java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java | 1 - .../java/org/breedinginsight/brapi/v2/dao/ProgramCache.java | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java b/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java index 205768784..f058d1307 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java +++ b/src/main/java/org/breedinginsight/brapi/v2/BrAPIV2Controller.java @@ -33,7 +33,6 @@ import org.brapi.v2.model.core.BrAPIServerInfo; import org.brapi.v2.model.core.response.BrAPIListsListResponse; import org.brapi.v2.model.core.response.BrAPIServerInfoResponse; -import org.brapi.v2.model.germ.BrAPIGermplasm; import org.breedinginsight.api.auth.AuthenticatedUser; import org.breedinginsight.api.auth.ProgramSecured; import org.breedinginsight.api.auth.ProgramSecuredRoleGroup; diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java b/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java index 2e56b9730..7fcbb62da 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java @@ -122,10 +122,9 @@ private void updateCache(UUID programId) { } } - //data posted still needs to be list public List post(UUID programId, Callable> postMethod) throws Exception { List response = postMethod.call(); - updateCache(programId); //will make get + updateCache(programId); return response; } } From 55a751a09dd0c5cd2b700fb03d9ff446a8579e64 Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Thu, 7 Apr 2022 12:10:18 -0400 Subject: [PATCH 4/8] Fix null key in unit test --- .../org/breedinginsight/brapi/v2/ProgramCacheUnitTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/org/breedinginsight/brapi/v2/ProgramCacheUnitTest.java b/src/test/java/org/breedinginsight/brapi/v2/ProgramCacheUnitTest.java index 534a75298..090ddafce 100644 --- a/src/test/java/org/breedinginsight/brapi/v2/ProgramCacheUnitTest.java +++ b/src/test/java/org/breedinginsight/brapi/v2/ProgramCacheUnitTest.java @@ -61,8 +61,7 @@ void setupNextTest() { public Map mockFetch(UUID programId, Integer sleepTime) { fetchCount += 1; Thread.sleep(sleepTime); - //Complex to retrieve UID so using DbId as dummy - return mockBrAPI.containsKey(programId) ? new HashMap<>(mockBrAPI.get(programId).stream().collect(Collectors.toMap(BrAPIGermplasm::getGermplasmDbId, germplasm -> germplasm))) : new HashMap<>(); //fxn to return map + return mockBrAPI.containsKey(programId) ? new HashMap<>(mockBrAPI.get(programId).stream().collect(Collectors.toMap(germplasm -> UUID.randomUUID().toString(), germplasm -> germplasm))) : new HashMap<>(); } @SneakyThrows From 2e781eb0d6417dc099a4eff1c4048d5ee69d65ec Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Thu, 7 Apr 2022 14:24:44 -0400 Subject: [PATCH 5/8] Unit test --- .../services/BrAPIGermplasmServiceUnitTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java b/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java index 56d3caf10..9e0493760 100644 --- a/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java +++ b/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java @@ -52,6 +52,8 @@ void setup() { programService = mock(ProgramService.class); } + //TODO: Refactoring in BI-1443 needs to be done before this can be enabled due to BrAPIDAOUtil.search currently being static + /* @Test @SneakyThrows public void getGermplasmListExport() { @@ -144,4 +146,5 @@ public void getGermplasmListExport() { assertEquals("Germplasm A", resultTable.get(0, 1), "Incorrect data exported"); assertEquals("2", resultTable.get(0, 6), "Incorrect data exported"); } + */ } From ceae2578bb1a22d57c0c088bb58583e7e8800d17 Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Wed, 13 Apr 2022 11:50:49 -0400 Subject: [PATCH 6/8] Code review fixes --- .../brapi/v2/GermplasmController.java | 8 ++++--- .../brapi/v2/dao/BrAPIGermplasmDAO.java | 22 +++++++++++++++++-- .../brapi/v2/dao/ProgramCache.java | 5 +++++ .../v2/services/BrAPIGermplasmService.java | 4 ++-- 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java b/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java index d4d115296..dc2d1ddbe 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java @@ -21,6 +21,7 @@ import org.breedinginsight.brapi.v2.model.response.mappers.GermplasmQueryMapper; import org.breedinginsight.brapi.v2.services.BrAPIGermplasmService; import org.breedinginsight.model.DownloadFile; +import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.utilities.response.ResponseUtils; import javax.inject.Inject; @@ -83,13 +84,14 @@ public HttpResponse germplasmListExport( @Get("/${micronaut.bi.api.version}/programs/{programId}" + BrapiVersion.BRAPI_V2 + "/germplasm/{germplasmId}") @Produces(MediaType.APPLICATION_JSON) @ProgramSecured(roleGroups = {ProgramSecuredRoleGroup.ALL}) - public HttpResponse getSingleGermplasm( + public HttpResponse> getSingleGermplasm( @PathVariable("programId") UUID programId, @PathVariable("germplasmId") String germplasmId) { try { log.debug("fetching germ id:" + germplasmId +" for program: " + programId); - return HttpResponse.ok(germplasmService.getGermplasmByUUID(programId, germplasmId)); - } catch (ApiException e) { + Response response = new Response(germplasmService.getGermplasmByUUID(programId, germplasmId)); + return HttpResponse.ok(response); + } catch (ApiException | DoesNotExistException e) { log.info(e.getMessage(), e); return HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, "Error retrieving germplasm"); } diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java index dbb49d540..00176ccdf 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java @@ -33,6 +33,7 @@ import org.breedinginsight.brapps.importer.model.ImportUpload; import org.breedinginsight.daos.ProgramDAO; import org.breedinginsight.model.Program; +import org.breedinginsight.services.exceptions.DoesNotExistException; import org.breedinginsight.utilities.BrAPIDAOUtil; import org.breedinginsight.utilities.Utilities; @@ -101,6 +102,13 @@ public List getRawGermplasm(UUID programId) throws ApiException }).collect(Collectors.toList()); } + + /** + * Fetch formatted germplasm for this program + * @param programId + * @return Map + * @throws ApiException + */ private Map fetchProgramGermplasm(UUID programId) throws ApiException { GermplasmApi api = new GermplasmApi(programDAO.getCoreClient(programId)); @@ -115,6 +123,12 @@ private Map fetchProgramGermplasm(UUID programId) throws )); } + /** + * Process germplasm into a format for display + * @param programGermplasm + * @return Map + * @throws ApiException + */ private Map processGermplasmForDisplay(List programGermplasm) { // Process the germplasm Map programGermplasmMap = new HashMap<>(); @@ -189,8 +203,12 @@ public List getGermplasmByName(List germplasmNames, UUID .collect(Collectors.toList()); } - public BrAPIGermplasm getGermplasmByUUID(String germplasmId, UUID programId) throws ApiException { - return programGermplasmCache.get(programId).get(germplasmId); + public BrAPIGermplasm getGermplasmByUUID(String germplasmId, UUID programId) throws ApiException, DoesNotExistException { + BrAPIGermplasm germplasm = programGermplasmCache.get(programId).get(germplasmId); + if (germplasm == null) { + throw new DoesNotExistException("UUID for this germplasm does not exist"); + } + return germplasm; } } diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java b/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java index 7fcbb62da..4ad7f970f 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/ProgramCache.java @@ -30,6 +30,11 @@ import java.util.concurrent.*; import java.util.stream.Collectors; +/** + * + * @param key/id of object + * @param object + */ @Slf4j public class ProgramCache { 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 6f5372ba6..e97387c38 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java @@ -58,10 +58,10 @@ public List getGermplasm(UUID programId) throws ApiException { } } - public BrAPIGermplasm getGermplasmByUUID(UUID programId, String germplasmId) throws ApiException { + public BrAPIGermplasm getGermplasmByUUID(UUID programId, String germplasmId) throws ApiException, DoesNotExistException { try { return germplasmDAO.getGermplasmByUUID(germplasmId, programId); - } catch (ApiException e) { + } catch (ApiException | DoesNotExistException e) { throw new InternalServerException(e.getMessage(), e); } } From 66a2e2e7e8b45f57313e3d98ed7ce690fa21625a Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Wed, 13 Apr 2022 14:54:00 -0400 Subject: [PATCH 7/8] Code review fixes 2 --- .../org/breedinginsight/brapi/v2/GermplasmController.java | 5 ++++- .../brapi/v2/services/BrAPIGermplasmService.java | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java b/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java index dc2d1ddbe..ca9ad456e 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java @@ -91,9 +91,12 @@ public HttpResponse> getSingleGermplasm( log.debug("fetching germ id:" + germplasmId +" for program: " + programId); Response response = new Response(germplasmService.getGermplasmByUUID(programId, germplasmId)); return HttpResponse.ok(response); - } catch (ApiException | DoesNotExistException e) { + } catch (ApiException e) { log.info(e.getMessage(), e); return HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, "Error retrieving germplasm"); + } catch (DoesNotExistException e) { + log.info(e.getMessage(), e); + return HttpResponse.status(HttpStatus.NOT_FOUND, "Germplasm not found"); } } } 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 9c96d8d72..89573799a 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java @@ -50,7 +50,6 @@ public BrAPIGermplasmService(BrAPIListDAO brAPIListDAO, ProgramService programSe } public List getGermplasm(UUID programId) throws ApiException { - List germplasmList; try { return germplasmDAO.getGermplasm(programId); } catch (ApiException e) { @@ -61,8 +60,10 @@ public List getGermplasm(UUID programId) throws ApiException { public BrAPIGermplasm getGermplasmByUUID(UUID programId, String germplasmId) throws ApiException, DoesNotExistException { try { return germplasmDAO.getGermplasmByUUID(germplasmId, programId); - } catch (ApiException | DoesNotExistException e) { + } catch (ApiException e) { throw new InternalServerException(e.getMessage(), e); + } catch (DoesNotExistException e) { + throw e; } } From f3f5e8528211678ff368b9750f784af96994e18e Mon Sep 17 00:00:00 2001 From: HMS17 <84345306+HMS17@users.noreply.github.com> Date: Thu, 14 Apr 2022 16:37:23 -0400 Subject: [PATCH 8/8] Code review cleanup --- .../breedinginsight/brapi/v2/GermplasmController.java | 3 ++- .../breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java | 9 ++++++--- .../brapi/v2/services/BrAPIGermplasmService.java | 4 +--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java b/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java index ca9ad456e..2810f2156 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java +++ b/src/main/java/org/breedinginsight/brapi/v2/GermplasmController.java @@ -5,6 +5,7 @@ import io.micronaut.http.HttpStatus; import io.micronaut.http.MediaType; import io.micronaut.http.annotation.*; +import io.micronaut.http.server.exceptions.InternalServerException; import io.micronaut.http.server.types.files.StreamedFile; import io.micronaut.security.annotation.Secured; import io.micronaut.security.rules.SecurityRule; @@ -91,7 +92,7 @@ public HttpResponse> getSingleGermplasm( log.debug("fetching germ id:" + germplasmId +" for program: " + programId); Response response = new Response(germplasmService.getGermplasmByUUID(programId, germplasmId)); return HttpResponse.ok(response); - } catch (ApiException e) { + } catch (InternalServerException e) { log.info(e.getMessage(), e); return HttpResponse.status(HttpStatus.INTERNAL_SERVER_ERROR, "Error retrieving germplasm"); } catch (DoesNotExistException e) { diff --git a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java index 897267769..76e3ffa94 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java @@ -21,7 +21,6 @@ import io.micronaut.context.annotation.Context; import io.micronaut.context.annotation.Property; import io.micronaut.http.server.exceptions.InternalServerException; -import it.unimi.dsi.fastutil.Hash; import lombok.extern.slf4j.Slf4j; import org.brapi.client.v2.model.exceptions.ApiException; import org.brapi.client.v2.modules.germplasm.GermplasmApi; @@ -177,7 +176,7 @@ private Map processGermplasmForDisplay(List referenceSource.equals(reference.getReferenceSource())).findFirst().orElseThrow(); + BrAPIExternalReference extRef = germplasm.getExternalReferences().stream().filter(reference -> referenceSource.equals(reference.getReferenceSource())).findFirst().orElseThrow(() -> new IllegalStateException("No BI external reference found")); String germplasmId = extRef.getReferenceID(); programGermplasmMap.put(germplasmId, germplasm); } @@ -206,7 +205,11 @@ public List getGermplasmByRawName(List germplasmNames, U } public BrAPIGermplasm getGermplasmByUUID(String germplasmId, UUID programId) throws ApiException, DoesNotExistException { - BrAPIGermplasm germplasm = programGermplasmCache.get(programId).get(germplasmId); + Map cache = programGermplasmCache.get(programId); + BrAPIGermplasm germplasm = null; + if (cache != null) { + germplasm = cache.get(germplasmId); + } if (germplasm == null) { throw new DoesNotExistException("UUID for this germplasm does not exist"); } 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 89573799a..0a854517b 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java +++ b/src/main/java/org/breedinginsight/brapi/v2/services/BrAPIGermplasmService.java @@ -57,13 +57,11 @@ public List getGermplasm(UUID programId) throws ApiException { } } - public BrAPIGermplasm getGermplasmByUUID(UUID programId, String germplasmId) throws ApiException, DoesNotExistException { + public BrAPIGermplasm getGermplasmByUUID(UUID programId, String germplasmId) throws DoesNotExistException { try { return germplasmDAO.getGermplasmByUUID(germplasmId, programId); } catch (ApiException e) { throw new InternalServerException(e.getMessage(), e); - } catch (DoesNotExistException e) { - throw e; } }