chore: move handling of total count and pagination to store [TECH-166…

…1] (#15482) * chore: split fetching all TEs from fetching paginated TEs [TECH-1661] Update dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/OrderAndPaginationExporterTest.java chore: split fetching all TEs from fetching paginated TEs [TECH-1661] chore: split fetching all TEs from fetching paginated TEs [TECH-1661] * chore: move handling of total count and pagination to store [TECH-1661] * chore: split fetching all relationships from fetching paginated relationships [TECH-1664] * Fix test * Fix code review comments
dhis2 · Oct 27, 2023 · 5b62825 · 5b62825
1 parent 6225ec4
commit 5b62825
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 246 deletions.
diff --git a/...dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java b/...dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java
@@ -266,8 +266,6 @@ private List<Event> fetchEvents(EventQueryParams queryParams, PageParams pagePar
         sql,
         mapSqlParameterSource,
         resultSet -> {
-          log.debug("Event query SQL: " + sql);
-
           Set<String> notes = new HashSet<>();
 
           while (resultSet.next()) {
@@ -543,8 +541,6 @@ private int getEventCount(EventQueryParams params) {
 
     sql = sql.replaceFirst("limit \\d+ offset \\d+", "");
 
-    log.debug("Event query count SQL: " + sql);
-
     return jdbcTemplate.queryForObject(sql, mapSqlParameterSource, Integer.class);
   }
 

diff --git a/...ervice-tracker/src/main/java/org/hisp/dhis/tracker/export/relationship/Relationships.java b/...ervice-tracker/src/main/java/org/hisp/dhis/tracker/export/relationship/Relationships.java
diff --git a/...src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java b/...src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java
@@ -27,14 +27,10 @@
  */
 package org.hisp.dhis.tracker.export.trackedentity;
 
-import static org.apache.commons.collections4.CollectionUtils.isNotEmpty;
-import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
 import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ALL;
 import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CHILDREN;
 import static org.hisp.dhis.common.OrganisationUnitSelectionMode.DESCENDANTS;
 import static org.hisp.dhis.common.OrganisationUnitSelectionMode.SELECTED;
-import static org.hisp.dhis.common.Pager.DEFAULT_PAGE_SIZE;
-import static org.hisp.dhis.common.SlimPager.FIRST_PAGE;
 
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -53,8 +49,6 @@
 import org.hisp.dhis.common.AuditType;
 import org.hisp.dhis.common.BaseIdentifiableObject;
 import org.hisp.dhis.common.IllegalQueryException;
-import org.hisp.dhis.common.Pager;
-import org.hisp.dhis.common.SlimPager;
 import org.hisp.dhis.feedback.BadRequestException;
 import org.hisp.dhis.feedback.ForbiddenException;
 import org.hisp.dhis.feedback.NotFoundException;
@@ -311,7 +305,6 @@ private RelationshipItem withNestedEntity(
   public List<TrackedEntity> getTrackedEntities(TrackedEntityOperationParams operationParams)
       throws ForbiddenException, NotFoundException, BadRequestException {
     TrackedEntityQueryParams queryParams = mapper.map(operationParams);
-    queryParams.setSkipPaging(true);
     final List<Long> ids = getTrackedEntityIds(queryParams);
 
     List<TrackedEntity> trackedEntities =
@@ -333,15 +326,11 @@ public Page<TrackedEntity> getTrackedEntities(
       TrackedEntityOperationParams operationParams, PageParams pageParams)
       throws BadRequestException, ForbiddenException, NotFoundException {
     TrackedEntityQueryParams queryParams = mapper.map(operationParams);
-    queryParams.setPage(pageParams.getPage());
-    queryParams.setPageSize(pageParams.getPageSize());
-    queryParams.setTotalPages(pageParams.isPageTotal());
-    queryParams.setSkipPaging(false);
-    final List<Long> ids = getTrackedEntityIds(queryParams);
+    final Page<Long> ids = getTrackedEntityIds(queryParams, pageParams);
 
     List<TrackedEntity> trackedEntities =
         this.trackedEntityAggregate.find(
-            ids, operationParams.getTrackedEntityParams(), queryParams);
+            ids.getItems(), operationParams.getTrackedEntityParams(), queryParams);
 
     mapRelationshipItems(
         trackedEntities,
@@ -350,16 +339,7 @@ public Page<TrackedEntity> getTrackedEntities(
 
     addSearchAudit(trackedEntities, queryParams.getUser());
 
-    Pager pager;
-
-    if (pageParams.isPageTotal()) {
-      int count = getTrackedEntityCount(queryParams, true, true);
-      pager = new Pager(pageParams.getPage(), count, pageParams.getPageSize());
-    } else {
-      pager = handleLastPageFlag(queryParams, trackedEntities);
-    }
-
-    return Page.of(trackedEntities, pager);
+    return Page.of(trackedEntities, ids.getPager());
   }
 
   public List<Long> getTrackedEntityIds(TrackedEntityQueryParams params) {
@@ -370,6 +350,14 @@ public List<Long> getTrackedEntityIds(TrackedEntityQueryParams params) {
     return trackedEntityStore.getTrackedEntityIds(params);
   }
 
+  public Page<Long> getTrackedEntityIds(TrackedEntityQueryParams params, PageParams pageParams) {
+    decideAccess(params);
+    validate(params);
+    validateSearchScope(params);
+
+    return trackedEntityStore.getTrackedEntityIds(params, pageParams);
+  }
+
   public void decideAccess(TrackedEntityQueryParams params) {
     if (params.isOrganisationUnitMode(ALL)
         && !currentUserService.currentUserIsAuthorized(
@@ -769,38 +757,6 @@ private void addTrackedEntityAudit(TrackedEntity trackedEntity, String user) {
     }
   }
 
-  /**
-   * This method will apply the logic related to the parameter 'totalPages=false'. This works in
-   * conjunction with the method: {@link
-   * TrackedEntityStore#getTrackedEntityIds(TrackedEntityQueryParams)}
-   *
-   * <p>This is needed because we need to query (pageSize + 1) at DB level. The resulting query will
-   * allow us to evaluate if we are in the last page or not. And this is what his method does,
-   * returning the respective Pager object.
-   *
-   * @param params the request params
-   * @param trackedEntityList the reference to the list of Tracked Entities
-   * @return the populated SlimPager instance
-   */
-  private Pager handleLastPageFlag(
-      TrackedEntityQueryParams params, List<TrackedEntity> trackedEntityList) {
-    Integer originalPage = defaultIfNull(params.getPage(), FIRST_PAGE);
-    Integer originalPageSize = defaultIfNull(params.getPageSize(), DEFAULT_PAGE_SIZE);
-    boolean isLastPage = false;
-
-    if (isNotEmpty(trackedEntityList)) {
-      isLastPage = trackedEntityList.size() <= originalPageSize;
-      if (!isLastPage) {
-        // Get the same number of elements of the pageSize, forcing
-        // the removal of the last additional element added at querying
-        // time.
-        trackedEntityList.retainAll(trackedEntityList.subList(0, originalPageSize));
-      }
-    }
-
-    return new SlimPager(originalPage, originalPageSize, isLastPage);
-  }
-
   @Override
   public Set<String> getOrderableFields() {
     return trackedEntityStore.getOrderableFields();

diff --git a/...src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityStore.java b/...src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityStore.java
@@ -43,6 +43,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
+import java.util.function.IntSupplier;
 import java.util.stream.Collectors;
 import javax.persistence.criteria.CriteriaBuilder;
 import javax.persistence.criteria.Predicate;
@@ -54,6 +55,7 @@
 import org.hisp.dhis.common.IdentifiableObject;
 import org.hisp.dhis.common.IllegalQueryException;
 import org.hisp.dhis.common.OrganisationUnitSelectionMode;
+import org.hisp.dhis.common.Pager;
 import org.hisp.dhis.common.QueryFilter;
 import org.hisp.dhis.common.hibernate.SoftDeleteHibernateObjectStore;
 import org.hisp.dhis.commons.collection.CollectionUtils;
@@ -68,6 +70,8 @@
 import org.hisp.dhis.trackedentity.TrackedEntity;
 import org.hisp.dhis.trackedentity.TrackedEntityAttribute;
 import org.hisp.dhis.tracker.export.Order;
+import org.hisp.dhis.tracker.export.Page;
+import org.hisp.dhis.tracker.export.PageParams;
 import org.hisp.dhis.user.CurrentUserService;
 import org.hisp.dhis.util.DateUtils;
 import org.springframework.context.ApplicationEventPublisher;
@@ -161,8 +165,7 @@ public HibernateTrackedEntityStore(
 
   @Override
   public List<Long> getTrackedEntityIds(TrackedEntityQueryParams params) {
-    String sql = getQuery(params);
-    log.debug("Tracked entity query SQL: " + sql);
+    String sql = getQuery(params, null);
     SqlRowSet rowSet = jdbcTemplate.queryForRowSet(sql);
 
     checkMaxTrackedEntityCountReached(params, rowSet);
@@ -176,6 +179,35 @@ public List<Long> getTrackedEntityIds(TrackedEntityQueryParams params) {
     return ids;
   }
 
+  @Override
+  public Page<Long> getTrackedEntityIds(TrackedEntityQueryParams params, PageParams pageParams) {
+    String sql = getQuery(params, pageParams);
+    SqlRowSet rowSet = jdbcTemplate.queryForRowSet(sql);
+
+    checkMaxTrackedEntityCountReached(params, rowSet);
+
+    List<Long> ids = new ArrayList<>();
+
+    while (rowSet.next()) {
+      ids.add(rowSet.getLong("trackedentityid"));
+    }
+
+    IntSupplier teCount = () -> getTrackedEntityCount(params);
+    return getPage(pageParams, ids, teCount);
+  }
+
+  private Page<Long> getPage(PageParams pageParams, List<Long> teIds, IntSupplier enrollmentCount) {
+    if (pageParams.isPageTotal()) {
+      Pager pager =
+          new Pager(pageParams.getPage(), enrollmentCount.getAsInt(), pageParams.getPageSize());
+      return Page.of(teIds, pager);
+    }
+
+    Pager pager = new Pager(pageParams.getPage(), 0, pageParams.getPageSize());
+    pager.force(pageParams.getPage(), pageParams.getPageSize());
+    return Page.of(teIds, pager);
+  }
+
   @Override
   public Set<String> getOrderableFields() {
     return ORDERABLE_FIELDS.keySet();
@@ -201,14 +233,12 @@ private void checkMaxTrackedEntityCountReached(
   @Override
   public int getTrackedEntityCount(TrackedEntityQueryParams params) {
     String sql = getCountQuery(params);
-    log.debug("Tracked entity count SQL: " + sql);
     return jdbcTemplate.queryForObject(sql, Integer.class);
   }
 
   @Override
   public int getTrackedEntityCountWithMaxTrackedEntityLimit(TrackedEntityQueryParams params) {
     String sql = getCountQueryWithMaxTrackedEntityLimit(params);
-    log.debug("Tracked entity count SQL: " + sql);
     return jdbcTemplate.queryForObject(sql, Integer.class);
   }
 
@@ -266,11 +296,11 @@ public int getTrackedEntityCountWithMaxTrackedEntityLimit(TrackedEntityQueryPara
    * @param params params defining the query
    * @return SQL string
    */
-  private String getQuery(TrackedEntityQueryParams params) {
+  private String getQuery(TrackedEntityQueryParams params, PageParams pageParams) {
     StringBuilder stringBuilder = new StringBuilder(getQuerySelect(params));
     return stringBuilder
         .append("FROM ")
-        .append(getFromSubQuery(params, false))
+        .append(getFromSubQuery(params, false, pageParams))
         .append(getQueryRelatedTables(params))
         .append(getQueryOrderBy(params, false))
         .toString();
@@ -287,7 +317,7 @@ private String getCountQuery(TrackedEntityQueryParams params) {
     return SELECT_COUNT_INSTANCE_FROM
         + getQuerySelect(params)
         + "FROM "
-        + getFromSubQuery(params, true)
+        + getFromSubQuery(params, true, null)
         + getQueryRelatedTables(params)
         + " ) tecount";
   }
@@ -303,7 +333,7 @@ private String getCountQueryWithMaxTrackedEntityLimit(TrackedEntityQueryParams p
     return SELECT_COUNT_INSTANCE_FROM
         + getQuerySelect(params)
         + "FROM "
-        + getFromSubQuery(params, true)
+        + getFromSubQuery(params, true, null)
         + getQueryRelatedTables(params)
         + (params.getProgram().getMaxTeiCountToReturn() > 0
             ? getLimitClause(params.getProgram().getMaxTeiCountToReturn() + 1)
@@ -352,7 +382,8 @@ private String getQuerySelect(TrackedEntityQueryParams params) {
    *
    * @return an SQL sub-query
    */
-  private String getFromSubQuery(TrackedEntityQueryParams params, boolean isCountQuery) {
+  private String getFromSubQuery(
+      TrackedEntityQueryParams params, boolean isCountQuery, PageParams pageParams) {
     SqlHelper whereAnd = new SqlHelper(true);
     StringBuilder fromSubQuery =
         new StringBuilder()
@@ -378,7 +409,7 @@ private String getFromSubQuery(TrackedEntityQueryParams params, boolean isCountQ
       fromSubQuery
           .append(getQueryOrderBy(params, true))
           // LIMIT, OFFSET
-          .append(getFromSubQueryLimitAndOffset(params));
+          .append(getFromSubQueryLimitAndOffset(params, pageParams));
     }
 
     return fromSubQuery.append(") TE ").toString();
@@ -980,12 +1011,13 @@ private String getQueryOrderBy(TrackedEntityQueryParams params, boolean innerOrd
    *
    * @return a SQL LIMIT and OFFSET clause, or empty string if no LIMIT can be deducted.
    */
-  private String getFromSubQueryLimitAndOffset(TrackedEntityQueryParams params) {
+  private String getFromSubQueryLimitAndOffset(
+      TrackedEntityQueryParams params, PageParams pageParams) {
     StringBuilder limitOffset = new StringBuilder();
     int limit = params.getMaxTeLimit();
     int teQueryLimit = systemSettingManager.getIntSetting(SettingKey.TRACKED_ENTITY_MAX_LIMIT);
 
-    if (limit == 0 && !params.isPaging()) {
+    if (limit == 0 && pageParams == null) {
       if (teQueryLimit > 0) {
         return limitOffset
             .append(LIMIT)
@@ -996,26 +1028,26 @@ private String getFromSubQueryLimitAndOffset(TrackedEntityQueryParams params) {
       }
 
       return limitOffset.toString();
-    } else if (limit == 0 && params.isPaging()) {
+    } else if (limit == 0 && pageParams != null) {
       return limitOffset
           .append(LIMIT)
           .append(SPACE)
-          .append(params.getPageSizeWithDefault())
+          .append(pageParams.getPageSize())
           .append(SPACE)
           .append(OFFSET)
           .append(SPACE)
-          .append(params.getOffset())
+          .append((pageParams.getPage() - 1) * pageParams.getPageSize())
           .append(SPACE)
           .toString();
-    } else if (params.isPaging()) {
+    } else if (pageParams != null) {
       return limitOffset
           .append(LIMIT)
           .append(SPACE)
-          .append(Math.min(limit + 1, params.getPageSizeWithDefault()))
+          .append(Math.min(limit + 1, pageParams.getPageSize()))
           .append(SPACE)
           .append(OFFSET)
           .append(SPACE)
-          .append(params.getOffset())
+          .append((pageParams.getPage() - 1) * pageParams.getPageSize())
           .append(SPACE)
           .toString();
     } else {