Merge pull request #447 from Breeding-Insight/feature/BI-2578

[BI-2578][BI-2489] - Optimize BrAPI Germplasm Search

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
 

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,22 @@ 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());
+        }
     }
 
     /**

src/main/java/org/breedinginsight/utilities/BrAPIDAOUtil.java

@@ -60,45 +60,59 @@ public class BrAPIDAOUtil {
     private final Duration searchTimeout;
     private final int pageSize;
     private final int postGroupSize;
+    private final int brapiFetchPageSize;
     private final ProgramService programService;
 
     @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;
     }
 
     public <T, U extends BrAPISearchRequestParametersPaging, V> List<V> search(Function<U, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchMethod,
                                                                                Function3<String, Integer, Integer, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchGetMethod,
                                                                                U searchBody
     ) throws ApiException {
-        return searchInternal(searchMethod, searchGetMethod, null, searchBody);
+        return searchInternal(searchMethod, searchGetMethod, null, searchBody, true);
     }
 
     public <T, U extends BrAPISearchRequestParametersPaging, V> List<V> search(Function<U, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchMethod,
                                                                                Function4<BrAPIWSMIMEDataTypes, String, Integer, Integer, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchGetMethod,
                                                                                U searchBody
     ) throws ApiException {
-        return searchInternal(searchMethod, null, searchGetMethod, searchBody);
+        return searchInternal(searchMethod, null, searchGetMethod, searchBody, true);
+    }
+
+    public <T, U extends BrAPISearchRequestParametersPaging, V> List<V> searchNoPaging(Function<U, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchMethod,
+                                                                               Function3<String, Integer, Integer, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchGetMethod,
+                                                                               U searchBody
+    ) throws ApiException {
+        return searchInternal(searchMethod, searchGetMethod, null, searchBody, false);
     }
 
     private <T, U extends BrAPISearchRequestParametersPaging, V> List<V> searchInternal(Function<U, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchMethod,
                                                                                         Function3<String, Integer, Integer, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchGetMethod,
                                                                                         Function4<BrAPIWSMIMEDataTypes, String, Integer, Integer, ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>>> searchGetMethodWithMimeType,
-                                                                                U searchBody) throws ApiException {
+                                                                                        U searchBody, boolean sendPaging) throws ApiException {
         try {
             List<V> 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) {
+                // 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(brapiFetchPageSize);
+            }
+
             ApiResponse<Pair<Optional<T>, Optional<BrAPIAcceptedSearchResponse>>> response = searchMethod.apply(searchBody);
             if (response.getBody().getLeft().isPresent()) {
                 BrAPIResponse listResponse = (BrAPIResponse) response.getBody().getLeft().get();
@@ -108,7 +122,7 @@ private <T, U extends BrAPISearchRequestParametersPaging, V> List<V> 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 +151,7 @@ private <T, U extends BrAPISearchRequestParametersPaging, V> List<V> searchInter
                         BrAPIResponse listResponse = (BrAPIResponse) searchGetResponse.getBody().getLeft().get();
                         listResult = getListResult(searchGetResponse);
 
-                        if(hasMorePages(listResponse)) {
+                        if(sendPaging && hasMorePages(listResponse)) {
                             currentPage++;
                             int totalPages = listResponse.getMetadata()
                                     .getPagination()

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:

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

src/test/java/org/breedinginsight/daos/BrAPIDAOUtilUnitTest.java

@@ -70,7 +70,7 @@ public ApiResponse<Pair<Optional<BrAPIGermplasmListResponse>, Optional<BrAPIAcce
     @BeforeEach
     void setup() {
         //Create instance of DAO
-        brAPIDAOUtil = new BrAPIDAOUtil(1000, Duration.of(10, ChronoUnit.MINUTES), 1, 100, programService());
+        brAPIDAOUtil = new BrAPIDAOUtil(1000, Duration.of(10, ChronoUnit.MINUTES), 1, 100, 65000, programService());
 
         //Set the page size field
         Field pageSize = BrAPIDAOUtil.class.getDeclaredField("pageSize");

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");

src/test/java/org/breedinginsight/services/geno/impl/GigwaGenotypeServiceImplIntegrationTest.java

@@ -201,7 +201,7 @@ ProgramService programService() {
 
     @MockBean(BrAPIDAOUtil.class)
     BrAPIDAOUtil brAPIDAOUtil() {
-        return spy(new BrAPIDAOUtil(1000, Duration.of(10, ChronoUnit.MINUTES), 1000, 100, programService()));
+        return spy(new BrAPIDAOUtil(1000, Duration.of(10, ChronoUnit.MINUTES), 1000, 100, 65000, programService()));
     }
 
     @MockBean(SimpleStorageService.class)