SearchQueryBuilder and BrAPIRepositoryImpl overhaul for germ optimization

Added utility methods to SearchQueryBuilder and BrAPIRepositoryImpl to allow for
proper paginating for hibernate fetch queries that don't suffocate memory.

Also added methods to run queries without pagination entirely using the SearchQueryBuilder
to prevent the use of pagination when it's not required, an issue that specifically
had to be addressed for the BI cache, but one that introduced code that is reusable for other
use cases.

Modified the GermplasmApiController's searchGermplasmPost endpoint to accomodate two code paths:
- One where no page and pageSize are supplied.  In this scenario the code will grab all germplasm
  without the use of pagination.  Good for large data grabs, but gets dangerous with excessively large
  amounts of data.  This is entirely to meet BI's current use case, which we have strongly advised they move
  off of.
- When page and/or pageSize are supplied, paginate as requested, default page size of 1000 if not requested.

Author: jloux-brapi

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/repository/BrAPIRepository.java

@@ -15,11 +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);
 
-	public Page<UUID> findAllBySearchIdsOnly(SearchQueryBuilder<T> searchQuery, Pageable pageReq);
+	public Page<T> findAllBySearchPaginatingWithFetches(SearchQueryBuilder<T> searchQuery, Pageable pageReq);
 
-	public Page<T> findAllBySearchNoPage(SearchQueryBuilder<T> searchQuery, Page<UUID> pagedIds, Pageable pageReq);
+	public List<T> findAllBySearch(SearchQueryBuilder<T> searchQuery);
 
 	public Optional<T> findById(ID id);
 

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

@@ -1,5 +1,15 @@
 package org.brapi.test.BrAPITestServer.repository;
 
+import java.io.Serializable;
+import java.util.*;
+import java.util.Map.Entry;
+import java.util.stream.Collectors;
+
+import io.swagger.model.Metadata;
+import io.swagger.model.germ.GermplasmSearchRequest;
+import javax.persistence.EntityManager;
+import javax.persistence.TypedQuery;
+
 import org.brapi.test.BrAPITestServer.model.entity.BrAPIBaseEntity;
 import org.brapi.test.BrAPITestServer.model.entity.BrAPIPrimaryEntity;
 import org.brapi.test.BrAPITestServer.model.entity.ExternalReferenceEntity;
@@ -15,53 +25,70 @@
 import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
 
-import javax.persistence.EntityManager;
-import javax.persistence.TypedQuery;
-import java.io.Serializable;
-import java.util.*;
-import java.util.Map.Entry;
-import java.util.stream.Collectors;
-
 public class BrAPIRepositoryImpl<T extends BrAPIPrimaryEntity, ID extends Serializable>
 		extends SimpleJpaRepository<T, ID> implements BrAPIRepository<T, ID> {
-	private EntityManager entityManager;
+	private final EntityManager entityManager;
 
 	public BrAPIRepositoryImpl(JpaEntityInformation<T, ID> entityInformation, EntityManager entityManager) {
 		super(entityInformation, entityManager);
 		this.entityManager = entityManager;
 	}
 
-	// This is the method that should be used for simple entities with few collections and eager loading requirements.
-	public Page<T> findAllBySearch(SearchQueryBuilder<T> searchQuery, Pageable pageReq) {
+	/**
+	 * 	Use this method to page simple entities with no lazily loaded collections or attributes.
+	 * 	WARN: Failure to do so can easily exhaust memory at scale and will cause hibernate warnings.
+	 */
+	public Page<T> findAllBySearchAndPaginate(SearchQueryBuilder<T> searchQuery, Pageable pageReq) {
 		applyUserId(searchQuery);
 		List<T> content = getPagedContent(searchQuery, pageReq);
 		Long totalCount = getTotalCount(searchQuery);
 
-		Page<T> page = new PageImpl<>(content, pageReq, totalCount);
-
-		return page;
+        return new PageImpl<>(content, pageReq, totalCount);
 	}
 
-	public Page<T> findAllBySearchNoPage(SearchQueryBuilder<T> searchQuery, Page<UUID> pagedIds, Pageable pageReq) {
+	/**
+	 * This method should be used when there is a need to page entities that are complex and have lots of lazily loaded,
+	 * one-to-many collection attributes. Call this method with the query you've built.
+	 * WARN: To avoid multiple bag fetch hibernate errors, only one collection that is one-many can be fetched at a time.
+	 * Ensure only one of these types of entity members is being fetched at time.
+	 * See {@link org.brapi.test.BrAPITestServer.service.germ.GermplasmService#findGermplasmEntities(GermplasmSearchRequest, Metadata)} for example usage.
+	 *
+	 * Once you fetch the first lazy loaded collection, if there are others you need to fetch for your entity it is
+	 * recommended to utilize the non-paging findAllBySearch() method, creating a searchQuery that inserts all the ids
+	 * of the entities found in the results from this method.  Again, follow code path of above example usage to see how this is done.
+	 *
+	 * If the need is to fetch according to an entirely different base criteria, feel free to call this method again.
+	 *
+	 * The results will be paged and ordered based on the submitted searchQuery and pageReq.
+    */
+	public Page<T> findAllBySearchPaginatingWithFetches(SearchQueryBuilder<T> searchQuery, Pageable pageReq) {
 		applyUserId(searchQuery);
 
-		List<T> entities = searchEntitiesWithIds(searchQuery, pagedIds);
+		// First grab all the ids of the entities according to the criteria of the searchQuery, paging as specified in the pageReq.
+		List<UUID> content = getPagedContentIdsOnly(searchQuery, pageReq);
+		Long totalCount = getTotalCount(searchQuery);
+
+		Page<UUID> pagedIds = new PageImpl<>(content, pageReq, totalCount);
 
-		return new PageImpl<>(entities, pageReq, pagedIds.getTotalElements());
+		// Now execute another query to fetch all the entities requested in the searchQuery, passing the pagedIds found
+		// in the previous query.  We will fetch them utilizing the ids from the paged query, avoiding the hibernate
+		// warning of paging while fetching and consuming considerably less memory.
+        return findAllBySearchUsingIds(searchQuery, pagedIds, pageReq);
 	}
 
+	private Page<T> findAllBySearchUsingIds(SearchQueryBuilder<T> searchQuery, Page<UUID> pagedIds, Pageable pageReq) {
+		List<T> entities = searchEntitiesWithIds(searchQuery, pagedIds.toList());
 
+		return new PageImpl<>(entities, pageReq, pagedIds.getSize());
+	}
 
-	// For entities that are complex and have lots of Collections and lazy loaded entities, it is more efficient to grab the IDs of the on Page,
-	// and afterward fetch these collections using the Ids, this time without paging.
-	public Page<UUID> findAllBySearchIdsOnly(SearchQueryBuilder<T> searchQuery, Pageable pageReq) {
+	/**
+	 * Use this method to run the searchQuery without pagination.  Useful for use cases where calls need to grab
+	 * every result at once, but use sparingly for use cases returning large result sets.
+	 */
+	public List<T> findAllBySearch(SearchQueryBuilder<T> searchQuery) {
 		applyUserId(searchQuery);
-		List<UUID> content = getPagedContentIdsOnly(searchQuery, pageReq);
-		Long totalCount = getTotalCount(searchQuery);
-
-		Page<UUID> page = new PageImpl<>(content, pageReq, totalCount);
-
-		return page;
+		return searchEntities(searchQuery);
 	}
 
 	public Optional<T> findById(ID id) {
@@ -98,7 +125,7 @@ public void fetchXrefs(Page<T> page, Class<T> searchClass) {
 		searchQuery.leftJoinFetch("externalReferences", "externalReferences")
 				   .appendList(page.stream().map(BrAPIBaseEntity::getId).collect(Collectors.toList()), "id");
 
-		Page<T> xrefs = findAllBySearch(searchQuery, PageRequest.of(0, page.getSize()));
+		Page<T> xrefs = findAllBySearchAndPaginate(searchQuery, PageRequest.of(0, page.getSize()));
 
 		Map<String, List<ExternalReferenceEntity>> xrefByEntity = new HashMap<>();
 		xrefs.forEach(entity -> xrefByEntity.put(entity.getId(), entity.getExternalReferences()));
@@ -127,28 +154,37 @@ private List<T> getPagedContent(SearchQueryBuilder<T> searchQuery, Pageable page
 		TypedQuery<T> query = entityManager.createQuery(searchQuery.getQuery(), searchQuery.getClazz());
 		query.setHint(QueryHints.HINT_PASS_DISTINCT_THROUGH, false);
 
-		for (Entry<String, Object> entry : searchQuery.getParams().entrySet()) {
-			query.setParameter(entry.getKey(), entry.getValue());
-		}
+		setQueryParams(query, searchQuery);
 
 		query.setFirstResult((int) pageReq.getOffset());
 		query.setMaxResults(pageReq.getPageSize());
 
-		List<T> content = query.getResultList();
-		return content;
+        return query.getResultList();
 	}
 
-	private List<T> searchEntitiesWithIds(SearchQueryBuilder<T> searchQuery, Page<UUID> ids) {
-		searchQuery.appendList(ids.stream().map(UUID::toString).collect(Collectors.toList()), "id");
+	private List<T> searchEntitiesWithIds(SearchQueryBuilder<T> searchQuery, List<UUID> ids) {
+		searchQuery.appendList(ids.stream().map(Object::toString).collect(Collectors.toList()), "id");
+
+		TypedQuery<T> query = entityManager.createQuery(searchQuery.getQuery(), searchQuery.getClazz());
+
+		setQueryParams(query, searchQuery);
 
+        return query.getResultList();
+	}
+
+	private List<T> searchEntities(SearchQueryBuilder<T> searchQuery) {
 		TypedQuery<T> query = entityManager.createQuery(searchQuery.getQuery(), searchQuery.getClazz());
 
+		setQueryParams(query, searchQuery);
+
+		return query.getResultList();
+	}
+
+	private void setQueryParams(TypedQuery<T> query, SearchQueryBuilder<T> searchQuery) {
 		for (Entry<String, Object> entry : searchQuery.getParams().entrySet()) {
 			query.setParameter(entry.getKey(), entry.getValue());
 		}
-
-		List<T> content = query.getResultList();
-		return content;	}
+	}
 
 	private List<UUID> getPagedContentIdsOnly(SearchQueryBuilder<T> searchQuery, Pageable pageReq) {
 
@@ -161,8 +197,7 @@ private List<UUID> getPagedContentIdsOnly(SearchQueryBuilder<T> searchQuery, Pag
 		query.setFirstResult((int) pageReq.getOffset());
 		query.setMaxResults(pageReq.getPageSize());
 
-		List<UUID> content = query.getResultList();
-		return content;
+        return query.getResultList();
 	}
 
 

src/main/java/org/brapi/test/BrAPITestServer/service/SearchQueryBuilder.java

@@ -1,11 +1,7 @@
 package org.brapi.test.BrAPITestServer.service;
 
 import java.math.BigDecimal;
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
 import java.time.LocalDate;
 import java.time.OffsetDateTime;
@@ -32,7 +28,7 @@ public SearchQueryBuilder(Class<T> clazz) {
 	}
 
 	public String getQuery() {
-		if (sortClause.isBlank()) {
+		if (sortClause.isEmpty()) {
 			// By default, sort on entity id to have query result remain idempotent
 			sortClause = " ORDER BY entity.id ASC ";
 		}
@@ -41,7 +37,7 @@ public String getQuery() {
 	}
 
 	public String getIdQuery() {
-		if (sortClause.isBlank()) {
+		if (sortClause.isEmpty()) {
 			// By default, sort on entity id to have query result remain idempotent
 			sortClause = " ORDER BY entity.id ASC ";
 		}
@@ -66,6 +62,15 @@ public SearchQueryBuilder<T> appendList(List<String> list, String columnName) {
 		return this;
 	}
 
+	public SearchQueryBuilder<T> appendIds(List<String> ids) {
+		String paramName = paramFilter("id");
+		if (ids != null && !ids.isEmpty()) {
+			this.whereClause += "AND " + entityPrefix("id") + " in :" + paramName + " ";
+			this.params.put(paramName, ids);
+		}
+		return this;
+	}
+
 	public SearchQueryBuilder<T> appendIntList(List<Integer> list, String columnName) {
 		String paramName = paramFilter(columnName);
 		if (list != null && !list.isEmpty()) {
@@ -242,10 +247,39 @@ public SearchQueryBuilder<T> join(String join, String name) {
 	}
 
 	public SearchQueryBuilder<T> leftJoinFetch(String join, String name) {
-		this.selectClause += "LEFT JOIN FETCH " + entityPrefix(join) + " " + paramFilter(name) + " ";
+		this.selectClause += generateLeftJoinFetch(join, name);
+		return this;
+	}
+
+	/**
+	 * Use this method to remove left join fetches from specific collection attributes so you can leverage the same query to
+	 * iterate through other lazily loaded collections on an entity you need to fetch.
+	 */
+	public SearchQueryBuilder<T> removeAndReplaceLeftJoinFetch(String join,
+															   String name,
+															   String existingJoin,
+															   String existingName) {
+
+		this.selectClause =
+				this.selectClause.replace(generateLeftJoinFetch(existingJoin, existingName), generateLeftJoinFetch(join, name));
+
 		return this;
 	}
 
+	/**
+	 * Use this method to remove left join fetches from specific collection attributes so you can leverage the same
+	 * base query criteria to add another join fetch with leftJoinFetch()
+	 */
+	public SearchQueryBuilder<T> removeLeftJoinFetch(String join, String name) {
+		this.selectClause =
+				this.selectClause.replace(generateLeftJoinFetch(join, name), "");
+		return this;
+	}
+
+	private String generateLeftJoinFetch(String join, String paramName) {
+		return "LEFT JOIN FETCH " + entityPrefix(join) + " " + paramFilter(paramName) + " ";
+	}
+
 	private String entityPrefix(String field) {
 		if (field.startsWith("*")) {
 			return field.substring(1);

src/main/java/org/brapi/test/BrAPITestServer/service/core/ListService.java

@@ -88,7 +88,7 @@ public List<ListSummary> findLists(ListSearchRequest request, Metadata metadata)
 		Pageable pageReq = PagingUtility.getPageRequest(metadata);
 		SearchQueryBuilder<ListEntity> searchQuery = buildQueryString(request);
 
-		Page<ListEntity> entityPage = listRepository.findAllBySearch(searchQuery, pageReq);
+		Page<ListEntity> entityPage = listRepository.findAllBySearchAndPaginate(searchQuery, pageReq);
 
 		List<ListSummary> data = entityPage.map(this::convertToSummary).getContent();
 		PagingUtility.calculateMetaData(metadata, entityPage);

src/main/java/org/brapi/test/BrAPITestServer/service/core/LocationService.java

@@ -83,7 +83,7 @@ public List<Location> findLocations(LocationSearchRequest request, Metadata meta
 				.appendList(request.getParentLocationDbIds(), "parentLocation.id")
 				.appendList(request.getParentLocationNames(), "parentLocation.name");
 
-		Page<LocationEntity> entityPage = locationRepository.findAllBySearch(searchQuery, pageReq);
+		Page<LocationEntity> entityPage = locationRepository.findAllBySearchAndPaginate(searchQuery, pageReq);
 
 		List<Location> data = entityPage.map(this::convertFromEntity).getContent();
 		PagingUtility.calculateMetaData(metadata, entityPage);

src/main/java/org/brapi/test/BrAPITestServer/service/core/PeopleService.java

@@ -62,7 +62,7 @@ public List<Person> findPeople(PersonSearchRequest request, Metadata metadata) {
 				.appendList(request.getMiddleNames(), "middleName").appendList(request.getPersonDbIds(), "id")
 				.appendList(request.getPhoneNumbers(), "phoneNumber").appendList(request.getUserIDs(), "userID");
 
-		Page<PersonEntity> entityPage = peopleRepository.findAllBySearch(searchQuery, pageReq);
+		Page<PersonEntity> entityPage = peopleRepository.findAllBySearchAndPaginate(searchQuery, pageReq);
 
 		List<Person> data = entityPage.map(this::convertToPerson).getContent();
 		PagingUtility.calculateMetaData(metadata, entityPage);

src/main/java/org/brapi/test/BrAPITestServer/service/core/ProgramService.java

@@ -69,7 +69,7 @@ public List<Program> findPrograms(ProgramSearchRequest request, Metadata metadat
 				.appendList(request.getObjectives(), "objective").appendList(request.getProgramDbIds(), "id")
 				.appendList(request.getProgramNames(), "name").appendEnumList(request.getProgramTypes(), "programType");
 
-		Page<ProgramEntity> page = programRepository.findAllBySearch(searchQuery, pageReq);
+		Page<ProgramEntity> page = programRepository.findAllBySearchAndPaginate(searchQuery, pageReq);
 		List<Program> programs = page.map(this::convertFromEntity).getContent();
 		PagingUtility.calculateMetaData(metadata, page);
 		return programs;

src/main/java/org/brapi/test/BrAPITestServer/service/core/SeasonService.java

@@ -44,7 +44,7 @@ public List<Season> findSeasons(String seasonDbId, String season, String seasonN
 		if (year != null)
 			searchQuery = searchQuery.appendSingle(year, "year");
 
-		Page<SeasonEntity> entityPage = seasonRepository.findAllBySearch(searchQuery, pageReq);
+		Page<SeasonEntity> entityPage = seasonRepository.findAllBySearchAndPaginate(searchQuery, pageReq);
 
 		List<Season> data = entityPage.map(this::convertFromEntity).getContent();
 		PagingUtility.calculateMetaData(metadata, entityPage);

src/main/java/org/brapi/test/BrAPITestServer/service/core/StudyService.java

@@ -149,7 +149,7 @@ public List<Study> findStudies(StudySearchRequest request, Metadata metaData) {
 				.appendList(request.getTrialDbIds(), "trial.id").appendList(request.getTrialNames(), "trial.trialName")
 				.withSort(getSortByField(request.getSortBy()), request.getSortOrder());
 
-		Page<StudyEntity> studiesPage = studyRepository.findAllBySearch(searchQuery, pageReq);
+		Page<StudyEntity> studiesPage = studyRepository.findAllBySearchAndPaginate(searchQuery, pageReq);
 		PagingUtility.calculateMetaData(metaData, studiesPage);
 
 		List<Study> studies = studiesPage.map(this::convertFromEntity).getContent();