Merge pull request #49 from Breeding-Insight/germ-search-opts

Germplasm Search Optimizations

Author: nickpalladino

src/main/java/org/brapi/test/BrAPITestServer/controller/core/BrAPIController.java

@@ -11,7 +11,7 @@
 import io.swagger.model.core.BatchDeletesListResponseResult;
 import org.brapi.test.BrAPITestServer.auth.AuthDetails;
 import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException;
-import org.brapi.test.BrAPITestServer.exceptions.InvalidPagingException;
+import org.brapi.test.BrAPITestServer.service.PagingUtility;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.http.HttpStatus;
@@ -33,29 +33,14 @@
 
 public class BrAPIController {
 	private static final Logger log = LoggerFactory.getLogger(ServerInfoApiController.class);
-	
-	protected Metadata generateMetaDataTemplateForSearch(Integer originalRequestedPage, Integer newRequestedPage,
-			Integer originalRequestedPageSize, Integer newRequestedPageSize) throws BrAPIServerException {
-		Integer page = newRequestedPage;
-		Integer pageSize = newRequestedPageSize;
-
-		if (page == null) {
-			page = originalRequestedPage;
-		}
-		if (pageSize == null) {
-			pageSize = originalRequestedPageSize;
-		}
-
-		return generateMetaDataTemplate(page, pageSize);
-	}
 
 	protected Metadata generateMetaDataTemplate(SearchRequest request) throws BrAPIServerException {
 		return generateMetaDataTemplate(request.getPage(), request.getPageSize());
 	}
 
 	protected Metadata generateMetaDataTemplate(String pageToken, Integer pageSize) {
 		if (pageSize == null) {
-			pageSize = 1000;
+			pageSize = PagingUtility.getDefaultPageSize();
 		}
 
 		Metadata metaData = generateEmptyMetadataToken();
@@ -65,14 +50,14 @@ protected Metadata generateMetaDataTemplate(String pageToken, Integer pageSize)
 	}
 
 	protected Metadata generateMetaDataTemplate(Integer page, Integer pageSize) throws BrAPIServerException {
-		validatePaging(page, pageSize);
+		PagingUtility.validatePaging(page, pageSize);
 
 		// defaults
 		if (page == null) {
 			page = 0;
 		}
 		if (pageSize == null) {
-			pageSize = 1000;
+			pageSize = PagingUtility.getDefaultPageSize();
 		}
 
 		Metadata metaData = generateEmptyMetadata();
@@ -81,16 +66,6 @@ protected Metadata generateMetaDataTemplate(Integer page, Integer pageSize) thro
 		return metaData;
 	}
 
-	private void validatePaging(Integer page, Integer pageSize) throws BrAPIServerException {
-		boolean pageValid = (page == null) || (page >= 0);
-		if (!pageValid)
-			throw new InvalidPagingException("page");
-		boolean pageSizeValid = (pageSize == null) || (pageSize >= 1);
-		if (!pageSizeValid)
-			throw new InvalidPagingException("pageSize");
-
-	}
-
 	protected Metadata generateEmptyMetadata() {
 		Metadata metaData = new Metadata();
 		metaData.setDatafiles(new ArrayList<>());

src/main/java/org/brapi/test/BrAPITestServer/controller/germ/GermplasmApiController.java

@@ -213,12 +213,37 @@ public ResponseEntity<? extends BrAPIResponse> searchGermplasmPost(@RequestBody
 		log.debug("Request: " + request.getRequestURI());
 		validateSecurityContext(request, "ROLE_ANONYMOUS", "ROLE_USER");
 		validateAcceptHeader(request);
-		Metadata metadata = generateMetaDataTemplate(body);
 
 		String searchReqDbId = searchService.saveSearchRequest(body, SearchRequestTypes.GERMPLASM);
 		if (searchReqDbId != null) {
 			return responseAccepted(searchReqDbId);
+		}
+
+		// WARN: This code was introduced to deal with a specific use case from BI which requires all data associated with
+		// a particular program to be retreived at once.  This method of data retreival is highly unadvised and can come
+		// with serious performance deficits, such as slow response times and exhausted memory allocation.
+		// Benchmarking suggests that at around 245-275k germplasm records returned 8GB of allocated memory will fail to
+		// be enough to return a result.
+
+		// This code is a stop-gap to allow BI to continue to do this improper retreival in a way that will be efficient
+		// for their specific use case.
+
+		// To get the data in this ill-advised way, forgo sending a page or pageSize attribute in the germplasm search request
+		// to this endpoint.  This will trigger the findGermplasmWithoutPaging code, which will grab all of the data without regard
+		// to data size.
+
+		// To use the endpoint the right way, ensure one or both of the aforementioned attributes are set and the germplasm
+		// records will be retrieved and returned paginated to limit resource consumption.  This way is much more fine tuned
+		// and will result in fast retrieval times with minimal memory allocation.
+		if (body.getPage() == null && body.getPageSize() == null) {
+			log.debug("Retrieving germs without pagination");
+			List<Germplasm> data = germplasmService.findGermplasmWithoutPaging(body);
+			Metadata metadata = generateEmptyMetadata();
+			metadata.getPagination().setTotalCount(data.size());
+			return responseOK(new GermplasmListResponse(), new GermplasmListResponseResult(), data, metadata);
 		} else {
+			log.debug("Retrieving germs with pagination");
+			Metadata metadata = generateMetaDataTemplate(body);
 			List<Germplasm> data = germplasmService.findGermplasm(body, metadata);
 			return responseOK(new GermplasmListResponse(), new GermplasmListResponseResult(), data, metadata);
 		}

src/main/java/org/brapi/test/BrAPITestServer/exceptions/InvalidPagingException.java

@@ -4,9 +4,18 @@
 
 public class InvalidPagingException extends BrAPIServerException {
 	private static final long serialVersionUID = 6250184179200451757L;
+
+	private static final String pageNumExceedsTotalPages =
+			"A page was requested which exceeds total amount of data available. Please make a RQ with page < totalPages - 1. " +
+					"Page numbers start at 0, and you can find out totalPages by making a RQ with \"page\": 0";
 
 	public InvalidPagingException(String field) {
 		super(HttpStatus.BAD_REQUEST, "\'" + field + "\' value is invalid");
 	}
+
+	// This constructor should only be used when the pageNum of the RQ exceeds the total number of pages available.
+	public InvalidPagingException() {
+		super(HttpStatus.BAD_REQUEST, pageNumExceedsTotalPages);
+	}
 
 }

src/main/java/org/brapi/test/BrAPITestServer/factory/BrAPIComponent.java

@@ -4,11 +4,12 @@
 import io.swagger.model.SearchRequest;
 import io.swagger.model.core.BatchDeleteTypes;
 import jakarta.validation.Valid;
+import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException;
 
 import java.util.List;
 
 public interface BrAPIComponent<T, R extends SearchRequest> {
-    List<T> findEntities(@Valid R request, Metadata metadata);
+    List<T> findEntities(@Valid R request, Metadata metadata) throws BrAPIServerException;
     BatchDeleteTypes getBatchDeleteType();
     List<String> collectDbIds(List<T> entities);
     void deleteBatchDeleteData(List<String> dbIds);

src/main/java/org/brapi/test/BrAPITestServer/factory/core/ListComponent.java

@@ -5,6 +5,7 @@
 import io.swagger.model.core.ListSearchRequest;
 import io.swagger.model.core.ListSummary;
 import jakarta.validation.Valid;
+import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException;
 import org.brapi.test.BrAPITestServer.factory.BrAPIComponent;
 import org.brapi.test.BrAPITestServer.service.core.ListService;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -23,7 +24,8 @@ public ListComponent(ListService listService) {
     }
 
     @Override
-    public List<ListSummary> findEntities(@Valid ListSearchRequest request, Metadata metadata) {
+    public List<ListSummary> findEntities(@Valid ListSearchRequest request, Metadata metadata)
+        throws BrAPIServerException {
         return listService.findLists(request, metadata);
     }
 

src/main/java/org/brapi/test/BrAPITestServer/factory/core/TrialComponent.java

@@ -4,6 +4,7 @@
 import io.swagger.model.core.BatchDeleteTypes;
 import io.swagger.model.core.Trial;
 import io.swagger.model.core.TrialSearchRequest;
+import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException;
 import org.brapi.test.BrAPITestServer.factory.BrAPIComponent;
 import org.brapi.test.BrAPITestServer.service.core.TrialService;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -22,7 +23,8 @@ public TrialComponent(TrialService trialService) {
     }
 
     @Override
-    public List<Trial> findEntities(TrialSearchRequest request, Metadata metadata) {
+    public List<Trial> findEntities(TrialSearchRequest request, Metadata metadata)
+        throws BrAPIServerException {
         return trialService.findTrials(request, metadata);
     }
 

src/main/java/org/brapi/test/BrAPITestServer/factory/geno/PlateComponent.java

@@ -5,6 +5,7 @@
 import io.swagger.model.geno.Plate;
 import io.swagger.model.geno.PlateSearchRequest;
 import org.apache.commons.lang3.NotImplementedException;
+import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException;
 import org.brapi.test.BrAPITestServer.factory.BrAPIComponent;
 import org.brapi.test.BrAPITestServer.service.geno.PlateService;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -23,7 +24,8 @@ public PlateComponent(PlateService plateService) {
     }
 
     @Override
-    public List<Plate> findEntities(PlateSearchRequest request, Metadata metadata) {
+    public List<Plate> findEntities(PlateSearchRequest request, Metadata metadata)
+        throws BrAPIServerException  {
         return plateService.findPlates(request, metadata);
     }
 

src/main/java/org/brapi/test/BrAPITestServer/factory/geno/SampleComponent.java

@@ -4,6 +4,7 @@
 import io.swagger.model.core.BatchDeleteTypes;
 import io.swagger.model.geno.Sample;
 import io.swagger.model.geno.SampleSearchRequest;
+import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException;
 import org.brapi.test.BrAPITestServer.factory.BrAPIComponent;
 import org.brapi.test.BrAPITestServer.service.geno.SampleService;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -22,7 +23,8 @@ public SampleComponent(SampleService sampleService) {
     }
 
     @Override
-    public List<Sample> findEntities(SampleSearchRequest request, Metadata metadata) {
+    public List<Sample> findEntities(SampleSearchRequest request, Metadata metadata)
+        throws BrAPIServerException {
         return sampleService.findSamples(request, metadata);
     }
 

src/main/java/org/brapi/test/BrAPITestServer/factory/germ/GermplasmComponent.java

@@ -5,6 +5,7 @@
 import io.swagger.model.germ.Germplasm;
 import io.swagger.model.germ.GermplasmSearchRequest;
 import jakarta.validation.Valid;
+import org.brapi.test.BrAPITestServer.exceptions.BrAPIServerException;
 import org.brapi.test.BrAPITestServer.factory.BrAPIComponent;
 import org.brapi.test.BrAPITestServer.service.germ.GermplasmService;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -23,7 +24,8 @@ public GermplasmComponent(GermplasmService germplasmService) {
     }
 
     @Override
-    public List<Germplasm> findEntities(@Valid GermplasmSearchRequest request, Metadata metadata) {
+    public List<Germplasm> findEntities(@Valid GermplasmSearchRequest request, Metadata metadata)
+        throws BrAPIServerException {
         return germplasmService.findGermplasm(request, metadata);
     }
 

src/main/java/org/brapi/test/BrAPITestServer/repository/BrAPIRepository.java

@@ -4,6 +4,7 @@
 import java.util.List;
 import java.util.Optional;
 
+import org.brapi.test.BrAPITestServer.exceptions.InvalidPagingException;
 import org.brapi.test.BrAPITestServer.model.entity.BrAPIPrimaryEntity;
 import org.brapi.test.BrAPITestServer.service.SearchQueryBuilder;
 import org.springframework.data.domain.Page;
@@ -14,7 +15,11 @@
 @NoRepositoryBean
 public interface BrAPIRepository<T extends BrAPIPrimaryEntity, ID extends Serializable> extends JpaRepository<T, ID> {
 
-	public Page<T> findAllBySearch(SearchQueryBuilder<T> searchQuery, Pageable pageReq);
+	public Page<T> findAllBySearchAndPaginate(SearchQueryBuilder<T> searchQuery, Pageable pageReq) throws InvalidPagingException;
+
+	public Page<T> findAllBySearchPaginatingWithFetches(SearchQueryBuilder<T> searchQuery, Pageable pageReq) throws InvalidPagingException;
+
+	public List<T> findAllBySearch(SearchQueryBuilder<T> searchQuery);
 
 	public Optional<T> findById(ID id);
 
@@ -24,5 +29,5 @@ public interface BrAPIRepository<T extends BrAPIPrimaryEntity, ID extends Serial
 
 	public <S extends T> void refresh(S entity);
 
-	public void fetchXrefs(Page<T> page, Class<T> searchClass);
+	public void fetchXrefs(Page<T> page, Class<T> searchClass) throws InvalidPagingException;
 }