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-1664] #15486

Merged
merged 4 commits into from
Oct 27, 2023
+9 −59
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
852d497
chore: split fetching all relationships from fetching paginated relat…
enricocolasante Oct 25, 2023
5d29880
chore: move handling of total count and pagination to store [TECH-1664]
enricocolasante Oct 25, 2023
96689f1
Fix review comments
enricocolasante Oct 25, 2023
3d68bce
Merge remote-tracking branch 'origin/master' into TECH-1664-paginatio…
enricocolasante Oct 26, 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
36 changes: 0 additions & 36 deletions ...r/src/main/java/org/hisp/dhis/tracker/export/relationship/DefaultRelationshipService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,10 @@
*/
package org.hisp.dhis.tracker.export.relationship;

import static org.apache.commons.collections4.CollectionUtils.isNotEmpty;
import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
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.List;
import java.util.Set;
import lombok.RequiredArgsConstructor;
import org.hisp.dhis.common.Pager;
import org.hisp.dhis.common.SlimPager;
import org.hisp.dhis.feedback.ForbiddenException;
import org.hisp.dhis.feedback.NotFoundException;
import org.hisp.dhis.program.Enrollment;
Expand Down Expand Up @@ -247,35 +240,6 @@ private RelationshipItem withNestedEntity(RelationshipItem item) {
return result;
}

/**
* This method will apply the logic related to the parameter 'totalPages=false'. This works in
* conjunction with methods in : {@link RelationshipService}
*
* <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 relationships the reference to the list of Relationships
* @return the populated SlimPager instance
*/
private Pager handleLastPageFlag(List<Relationship> relationships, PageParams pageParams) {
Integer originalPage = defaultIfNull(pageParams.getPage(), FIRST_PAGE);
Integer originalPageSize = defaultIfNull(pageParams.getPageSize(), DEFAULT_PAGE_SIZE);
boolean isLastPage = false;

if (isNotEmpty(relationships)) {
isLastPage = relationships.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.
relationships.retainAll(relationships.subList(0, originalPageSize));
}
}

return new SlimPager(originalPage, originalPageSize, isLastPage);
}

@Override
public Set<String> getOrderableFields() {
return relationshipStore.getOrderableFields();
Expand Down
14 changes: 7 additions & 7 deletions ...r/src/main/java/org/hisp/dhis/tracker/export/relationship/HibernateRelationshipStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import java.util.Set;
import java.util.function.Function;
import java.util.function.IntSupplier;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import javax.persistence.TypedQuery;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
Expand Down Expand Up @@ -121,7 +121,9 @@ public List<Relationship> getByEvent(Event event, RelationshipQueryParams queryP

@Override
public Page<Relationship> getByTrackedEntity(
TrackedEntity trackedEntity, RelationshipQueryParams queryParams, PageParams pageParams) {
TrackedEntity trackedEntity,
RelationshipQueryParams queryParams,
@Nonnull PageParams pageParams) {
TypedQuery<Relationship> relationshipTypedQuery =
getRelationshipTypedQuery(trackedEntity, queryParams, pageParams);

Expand All @@ -131,7 +133,7 @@ public Page<Relationship> getByTrackedEntity(

@Override
public Page<Relationship> getByEnrollment(
Enrollment enrollment, RelationshipQueryParams queryParams, PageParams pageParams) {
Enrollment enrollment, RelationshipQueryParams queryParams, @Nonnull PageParams pageParams) {
TypedQuery<Relationship> relationshipTypedQuery =
getRelationshipTypedQuery(enrollment, queryParams, pageParams);

Expand All @@ -141,7 +143,7 @@ public Page<Relationship> getByEnrollment(

@Override
public Page<Relationship> getByEvent(
Event event, RelationshipQueryParams queryParams, PageParams pageParams) {
Event event, RelationshipQueryParams queryParams, @Nonnull PageParams pageParams) {
TypedQuery<Relationship> relationshipTypedQuery =
getRelationshipTypedQuery(event, queryParams, pageParams);

Expand Down Expand Up @@ -232,9 +234,7 @@ private TypedQuery<Relationship> getRelationshipTypedQuery(
newJpaParameters(queryParams, pageParams, builder);

relationshipItemCriteriaQuery.orderBy(
jpaQueryParameters.getOrders().stream()
.map(o -> o.apply(root))
.collect(Collectors.toList()));
jpaQueryParameters.getOrders().stream().map(o -> o.apply(root)).toList());

TypedQuery<Relationship> relationshipTypedQuery =
getSession().createQuery(relationshipItemCriteriaQuery);
Expand Down
16 changes: 0 additions & 16 deletions ...ntegration/src/test/java/org/hisp/dhis/tracker/export/OrderAndPaginationExporterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import static org.hisp.dhis.utils.Assertions.assertIsEmpty;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;

import java.io.IOException;
Expand All @@ -51,7 +50,6 @@
import org.hisp.dhis.common.Pager;
import org.hisp.dhis.common.QueryFilter;
import org.hisp.dhis.common.QueryOperator;
import org.hisp.dhis.common.SlimPager;
import org.hisp.dhis.common.UID;
import org.hisp.dhis.feedback.BadRequestException;
import org.hisp.dhis.feedback.ForbiddenException;
Expand Down Expand Up @@ -1315,18 +1313,4 @@ private static <T extends BaseIdentifiableObject> List<String> uids(Page<T> even
private static List<String> uids(List<? extends BaseIdentifiableObject> identifiableObject) {
return identifiableObject.stream().map(BaseIdentifiableObject::getUid).toList();
}

private static void assertSlimPager(int pageNumber, int pageSize, boolean isLast, Pager pager) {
assertInstanceOf(SlimPager.class, pager, "SlimPager should be returned if totalPages=false");
SlimPager slimPager = (SlimPager) pager;
assertAll(
"pagination details",
() -> assertEquals(pageNumber, slimPager.getPage(), "number of current page"),
() -> assertEquals(pageSize, slimPager.getPageSize(), "page size"),
() ->
assertEquals(
isLast,
slimPager.isLastPage(),
isLast ? "should be the last page" : "should NOT be the last page"));
}
}
2 changes: 2 additions & 0 deletions ...isp/dhis/webapi/controller/tracker/export/relationship/RelationshipsExportController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import static org.hisp.dhis.common.OpenApi.Response.Status;
import static org.hisp.dhis.webapi.controller.tracker.ControllerSupport.RESOURCE_PATH;
import static org.hisp.dhis.webapi.controller.tracker.ControllerSupport.assertUserOrderableFieldsAreSupported;
import static org.hisp.dhis.webapi.controller.tracker.export.RequestParamsValidator.validatePaginationParameters;
import static org.hisp.dhis.webapi.controller.tracker.export.relationship.RequestParams.DEFAULT_FIELDS_PARAM;
import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;

Expand Down Expand Up @@ -97,6 +98,7 @@ public RelationshipsExportController(
@GetMapping
PagingWrapper<ObjectNode> getRelationships(RequestParams requestParams)
throws NotFoundException, BadRequestException, ForbiddenException {
validatePaginationParameters(requestParams);
RelationshipOperationParams operationParams = mapper.map(requestParams);

if (requestParams.isPaged()) {
Expand Down
Loading