Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: move handling of total count and pagination to store [TECH-1661] #15482

Merged
merged 8 commits into from
Oct 27, 2023
Merged

chore: move handling of total count and pagination to store [TECH-1661] #15482

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
180976d
chore: split fetching all TEs from fetching paginated TEs [TECH-1661]
enricocolasante Oct 24, 2023
d14fef9
chore: move handling of total count and pagination to store [TECH-1661]
enricocolasante Oct 25, 2023
d900679
chore: split fetching all relationships from fetching paginated relat…
enricocolasante Oct 25, 2023
d8e2f07
Merge remote-tracking branch 'origin/master' into TECH-1661-paginatio…
enricocolasante Oct 26, 2023
b729612
Merge remote-tracking branch 'origin/master' into TECH-1661-paginatio…
enricocolasante Oct 26, 2023
bac642d
Fix test
enricocolasante Oct 26, 2023
2fb6dfe
Merge remote-tracking branch 'origin/master' into TECH-1661-paginatio…
enricocolasante Oct 27, 2023
4bb1dff
Fix code review comments
enricocolasante Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 11 additions & 55 deletions ...src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 =
Expand All @@ -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,
Expand All @@ -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) {
Expand All @@ -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(
Expand Down Expand Up @@ -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();
Expand Down
66 changes: 51 additions & 15 deletions ...src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -161,7 +165,7 @@ public HibernateTrackedEntityStore(

@Override
public List<Long> getTrackedEntityIds(TrackedEntityQueryParams params) {
String sql = getQuery(params);
String sql = getQuery(params, null);
log.debug("Tracked entity query SQL: " + sql);
SqlRowSet rowSet = jdbcTemplate.queryForRowSet(sql);

Expand All @@ -176,6 +180,36 @@ public List<Long> getTrackedEntityIds(TrackedEntityQueryParams params) {
return ids;
}

@Override
public Page<Long> getTrackedEntityIds(TrackedEntityQueryParams params, PageParams pageParams) {
String sql = getQuery(params, pageParams);
log.debug("Tracked entity query SQL: " + sql);
github-advanced-security[bot] marked this conversation as resolved.
Fixed Show resolved Hide resolved
SqlRowSet rowSet = jdbcTemplate.queryForRowSet(sql);
Dismissed Show dismissed Hide dismissed

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();
Expand Down Expand Up @@ -266,11 +300,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();
Expand All @@ -287,7 +321,7 @@ private String getCountQuery(TrackedEntityQueryParams params) {
return SELECT_COUNT_INSTANCE_FROM
+ getQuerySelect(params)
+ "FROM "
+ getFromSubQuery(params, true)
+ getFromSubQuery(params, true, null)
+ getQueryRelatedTables(params)
+ " ) tecount";
}
Expand All @@ -303,7 +337,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)
Expand Down Expand Up @@ -352,7 +386,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()
Expand All @@ -378,7 +413,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();
Expand Down Expand Up @@ -980,12 +1015,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)
Expand All @@ -996,26 +1032,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 {
Expand Down
48 changes: 0 additions & 48 deletions ...ice-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntities.java
View file
Open in desktop

This file was deleted.

Loading
Loading