From e6374e595bbcbde3beb92696cb6441c80cdda98b Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Fri, 14 Mar 2025 14:14:34 -0400 Subject: [PATCH 1/6] [BI-2578] Support for unpaginated RQs to BrAPI server for cache loading --- .../brapi/v2/dao/BrAPIGermplasmDAO.java | 2 +- .../utilities/BrAPIDAOUtil.java | 23 ++++++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) 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 75226babf..9e83615a2 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java +++ b/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java @@ -141,7 +141,7 @@ private Map 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( + return processGermplasmForDisplay(brAPIDAOUtil.searchNoPaging( api::searchGermplasmPost, api::searchGermplasmSearchResultsDbIdGet, germplasmSearch diff --git a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java index b730660e3..7fb115a53 100644 --- a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java +++ b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java @@ -79,26 +79,37 @@ public List search(Funct Function3, Optional>>> searchGetMethod, U searchBody ) throws ApiException { - return searchInternal(searchMethod, searchGetMethod, null, searchBody); + return searchInternal(searchMethod, searchGetMethod, null, searchBody, true); } public List search(Function, Optional>>> searchMethod, Function4, Optional>>> searchGetMethod, U searchBody ) throws ApiException { - return searchInternal(searchMethod, null, searchGetMethod, searchBody); + return searchInternal(searchMethod, null, searchGetMethod, searchBody, true); + } + + public List searchNoPaging(Function, Optional>>> searchMethod, + Function3, Optional>>> searchGetMethod, + U searchBody + ) throws ApiException { + return searchInternal(searchMethod, searchGetMethod, null, searchBody, false); } private List searchInternal(Function, Optional>>> searchMethod, Function3, Optional>>> searchGetMethod, Function4, Optional>>> searchGetMethodWithMimeType, - U searchBody) throws ApiException { + U searchBody, boolean sendPaging) throws ApiException { try { List listResult = new ArrayList<>(); //NOTE: Because of the way Breedbase implements BrAPI searches, the page size is initially set to an //arbitrary, large value to ensure that in the event that a 202 response is returned, the searchDbId //stored will refer to all records of the BrAPI variable. - searchBody.pageSize(10000000); + + if (sendPaging) { + searchBody.pageSize(65000); + } + ApiResponse, Optional>> response = searchMethod.apply(searchBody); if (response.getBody().getLeft().isPresent()) { BrAPIResponse listResponse = (BrAPIResponse) response.getBody().getLeft().get(); @@ -108,7 +119,7 @@ private List searchInter pagination params are handled for POST search endpoints or the corresponding endpoints in Breedbase are changed or updated */ - if(hasMorePages(listResponse)) { + if(sendPaging && hasMorePages(listResponse)) { int currentPage = listResponse.getMetadata().getPagination().getCurrentPage() + 1; int totalPages = listResponse.getMetadata().getPagination().getTotalPages(); @@ -137,7 +148,7 @@ private List searchInter BrAPIResponse listResponse = (BrAPIResponse) searchGetResponse.getBody().getLeft().get(); listResult = getListResult(searchGetResponse); - if(hasMorePages(listResponse)) { + if(sendPaging && hasMorePages(listResponse)) { currentPage++; int totalPages = listResponse.getMetadata() .getPagination() From 9c0724471b8d648ddf1029661d0c69060ef27403 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Mon, 17 Mar 2025 16:25:09 -0400 Subject: [PATCH 2/6] Add comment about configurable variable on brapi test server side --- .../java/org/breedinginsight/utilities/BrAPIDAOUtil.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java index 7fb115a53..3d4925414 100644 --- a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java +++ b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java @@ -102,11 +102,11 @@ private List searchInter U searchBody, boolean sendPaging) throws ApiException { try { List listResult = new ArrayList<>(); - //NOTE: Because of the way Breedbase implements BrAPI searches, the page size is initially set to an - //arbitrary, large value to ensure that in the event that a 202 response is returned, the searchDbId - //stored will refer to all records of the BrAPI variable. if (sendPaging) { + // This should be set to whatever the maximum allowable value is configured in the brapi test server, + // perhaps it should be configurable on bi side as well. + // For reference, that prop name is paging.page-size.max-allowed searchBody.pageSize(65000); } From 14c1ee3baaa293dd0c37cef6c0fde10b2eabbadd Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Wed, 19 Mar 2025 14:24:09 -0400 Subject: [PATCH 3/6] [BI-2578] Add config props for max rq page size and pagination flipper for germs --- .env.template | 4 ++++ .../brapi/v2/dao/BrAPIGermplasmDAO.java | 24 +++++++++++++++---- .../utilities/BrAPIDAOUtil.java | 5 +++- src/main/resources/application.yml | 13 ++++++++++ 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/.env.template b/.env.template index abe859391..1a481aae6 100644 --- a/.env.template +++ b/.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 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 9e83615a2..a8feaecf6 100644 --- a/src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIGermplasmDAO.java +++ b/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 programGermplasmCache; private final BrAPIEndpointProvider brAPIEndpointProvider; @@ -141,11 +144,22 @@ private Map 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.searchNoPaging( - 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()); + } } /** diff --git a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java index 3d4925414..15dc57236 100644 --- a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java +++ b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java @@ -62,6 +62,9 @@ public class BrAPIDAOUtil { private final int postGroupSize; private final ProgramService programService; + @Property(name = "brapi.cache.fetch-page-size") + private int brapiFetchPageSize; + @Inject public BrAPIDAOUtil(@Property(name = "brapi.search.wait-time") int searchWaitTime, @Property(name = "brapi.read-timeout") Duration searchTimeout, @@ -107,7 +110,7 @@ private List searchInter // This should be set to whatever the maximum allowable value is configured in the brapi test server, // perhaps it should be configurable on bi side as well. // For reference, that prop name is paging.page-size.max-allowed - searchBody.pageSize(65000); + searchBody.pageSize(brapiFetchPageSize); } ApiResponse, Optional>> response = searchMethod.apply(searchBody); diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 3a98c3fb6..820d0cbac 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -173,6 +173,19 @@ brapi: search: wait-time: 1000 post-group-size: ${POST_CHUNK_SIZE:1000} + paginate: + # These props are used as flippers for specific cache entities to be fetched all at once or paginated. + # If the props are set to true, there's a possibility the BrAPI server could run out of memory grabbing all the + # data and assigning it to entities in memory given a program that has a large amount of those types of entities. + # At that point these flippers should be set to true. + germplasm: ${CACHE_PAGINATE_GERMPLASM:false} + cache: + # This prop sets how many program cache entity records can be fetched at a time from BrAPI. Today, if over 65000 is used, + # a SQL error can happen for programs that have more than this page size, because to fetch these certain entities + # the same amount of IDs must be passed for any collections that need to be fetched in the entity, and SQL has a max of ~65355 params. + # The ProgramCache will iterate through all pages of an entity to retrieve all the data from BrAPI. + fetch-page-size: ${CACHE_BRAPI_FETCH_PAGE_SIZE:65000} + email: relay-server: From 51c9f7f8d295be6275dea44ed0f3d898b5f2e02d Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Mon, 31 Mar 2025 12:25:35 -0400 Subject: [PATCH 4/6] Add test server template props to bi-api --- .../resources/brapi/properties/application.properties | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/resources/brapi/properties/application.properties b/src/main/resources/brapi/properties/application.properties index 28a097f3a..3733fcd37 100644 --- a/src/main/resources/brapi/properties/application.properties +++ b/src/main/resources/brapi/properties/application.properties @@ -36,3 +36,13 @@ spring.mvc.dispatch-options-request=true security.oidc_discovery_url=https://example.com/auth/.well-known/openid-configuration security.enabled=false +security.issuer_url=http://example.com/issuerurl + +# This should either be set in accordance with a maximum number of SQL parameters (on JOIN FETCHES of collections, +# if there is more than one collection the IDs of each entity need to be passed through as parameters, and there is a SQL +# maximum of 65535. See GermplasmService.findGermplasmEntities()), +# whatever returns in a reasonable amount of time, +# or if you want to limit for the sake of server efficiency. +paging.page-size.max-allowed=65000 + +paging.page-size.default=1000 From bb359fdeef3993f915672355b65e050add7562e2 Mon Sep 17 00:00:00 2001 From: jloux-brapi Date: Wed, 2 Apr 2025 16:40:43 -0400 Subject: [PATCH 5/6] Fix unit test errors related to new property not loading --- .../java/org/breedinginsight/utilities/BrAPIDAOUtil.java | 6 +++--- .../java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java | 2 +- .../geno/impl/GigwaGenotypeServiceImplIntegrationTest.java | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java index 15dc57236..cab44d951 100644 --- a/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java +++ b/src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java @@ -60,21 +60,21 @@ public class BrAPIDAOUtil { private final Duration searchTimeout; private final int pageSize; private final int postGroupSize; + private final int brapiFetchPageSize; private final ProgramService programService; - @Property(name = "brapi.cache.fetch-page-size") - private int brapiFetchPageSize; - @Inject public BrAPIDAOUtil(@Property(name = "brapi.search.wait-time") int searchWaitTime, @Property(name = "brapi.read-timeout") Duration searchTimeout, @Property(name = "brapi.page-size") int pageSize, @Property(name = "brapi.post-group-size") int postGroupSize, + @Property(name = "brapi.cache.fetch-page-size") int brapiFetchPageSize, ProgramService programService) { this.searchWaitTime = searchWaitTime; this.searchTimeout = searchTimeout; this.pageSize = pageSize; this.postGroupSize = postGroupSize; + this.brapiFetchPageSize = brapiFetchPageSize; this.programService = programService; } diff --git a/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java b/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java index 427ed6d88..d77971817 100644 --- a/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java +++ b/src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java @@ -70,7 +70,7 @@ public ApiResponse, Optional Date: Wed, 2 Apr 2025 16:59:49 -0400 Subject: [PATCH 6/6] Fix unit test failure related to searchNoPaging function --- .../services/BrAPIGermplasmServiceUnitTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java b/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java index 8f906c169..9d5f7b792 100644 --- a/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java +++ b/src/test/java/org/breedinginsight/services/BrAPIGermplasmServiceUnitTest.java @@ -141,9 +141,9 @@ public void getGermplasmListExport() { when(programDAO.getAll()).thenReturn(Arrays.asList(Program.builder().id(testProgramId).name("Test Program").active(true).build())); when(programDAO.fetchOneById(any(UUID.class))).thenReturn(testProgram); when(programDAO.get(any(UUID.class))).thenReturn(Arrays.asList(testProgram)); - when(brAPIDAOUtil.search(any(Function.class), - any(Function3.class), - any(BrAPIGermplasmSearchRequest.class))).thenReturn(germplasm); + when(brAPIDAOUtil.searchNoPaging(any(Function.class), + any(Function3.class), + any(BrAPIGermplasmSearchRequest.class))).thenReturn(germplasm); //Create germplasm cache of stub data Method setupMethod = BrAPIGermplasmDAO.class.getDeclaredMethod("setup");