From 72925cebeac7100cc72e938bf52fd6e3ba9410f5 Mon Sep 17 00:00:00 2001 From: teleivo Date: Fri, 14 Feb 2025 12:01:01 +0100 Subject: [PATCH] fix: cannot get TE if there are no tracker programs (#19930) --- .../org/hisp/dhis/tracker/export/Page.java | 4 ++ .../DefaultTrackedEntityService.java | 6 ++- .../HibernateTrackedEntityStore.java | 54 ++++++++++++++----- .../TrackedEntityOperationParamsMapper.java | 23 ++++---- .../TrackedEntityQueryParams.java | 35 +++++++----- .../aggregates/TrackedEntityAggregate.java | 2 +- .../aggregates/TrackedEntityStore.java | 5 +- ...rackedEntityOperationParamsMapperTest.java | 2 +- .../TrackedEntityServiceTest.java | 36 +++++++++++++ 9 files changed, 124 insertions(+), 43 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/Page.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/Page.java index 5bc98966cd35..2eec6e65b81c 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/Page.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/Page.java @@ -50,6 +50,10 @@ public class Page { private final Integer prevPage; private final Integer nextPage; + public static Page empty() { + return new Page<>(List.of(), 0, 0, 0L, null, null); + } + /** * Create a new page based on an existing one but with given {@code items}. Page related counts * will not be changed so make sure the given {@code items} match the previous page size. diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java index 4173b2b53ae6..0d8fe261e860 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java @@ -375,10 +375,12 @@ private List getTrackedEntities( for (TrackedEntity trackedEntity : trackedEntities) { if (operationParams.getTrackedEntityParams().isIncludeProgramOwners()) { trackedEntity.setProgramOwners( - getTrackedEntityProgramOwners(trackedEntity, queryParams.getProgram())); + getTrackedEntityProgramOwners( + trackedEntity, queryParams.getEnrolledInTrackerProgram())); } trackedEntity.setTrackedEntityAttributeValues( - getTrackedEntityAttributeValues(trackedEntity, queryParams.getProgram())); + getTrackedEntityAttributeValues( + trackedEntity, queryParams.getEnrolledInTrackerProgram())); } trackedEntityAuditService.addTrackedEntityAudit(SEARCH, user.getUsername(), trackedEntities); return trackedEntities; diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityStore.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityStore.java index f21dada9d10a..a52aeed64a53 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityStore.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityStore.java @@ -148,6 +148,13 @@ public HibernateTrackedEntityStore( } public List getTrackedEntityIds(TrackedEntityQueryParams params) { + // A TE which is not enrolled can only be accessed by a user that is able to enroll it into a + // tracker program. Return an empty result if there are no tracker programs or the user does + // not have access to one. + if (!params.hasEnrolledInTrackerProgram() && params.getAccessibleTrackerPrograms().isEmpty()) { + return List.of(); + } + String sql = getQuery(params, null); SqlRowSet rowSet = jdbcTemplate.queryForRowSet(sql); @@ -163,6 +170,13 @@ public List getTrackedEntityIds(TrackedEntityQueryPara public Page getTrackedEntityIds( TrackedEntityQueryParams params, PageParams pageParams) { + // A TE which is not enrolled can only be accessed by a user that is able to enroll it into a + // tracker program. Return an empty result if there are no tracker programs or the user does + // not have access to one. + if (!params.hasEnrolledInTrackerProgram() && params.getAccessibleTrackerPrograms().isEmpty()) { + return Page.empty(); + } + String sql = getQuery(params, pageParams); SqlRowSet rowSet = jdbcTemplate.queryForRowSet(sql); @@ -206,12 +220,26 @@ private void checkMaxTrackedEntityCountReached( } } - public Long getTrackedEntityCount(TrackedEntityQueryParams params) { + private Long getTrackedEntityCount(TrackedEntityQueryParams params) { + // A TE which is not enrolled can only be accessed by a user that is able to enroll it into a + // tracker program. Return an empty result if there are no tracker programs or the user does + // not have access to one. + if (!params.hasEnrolledInTrackerProgram() && params.getAccessibleTrackerPrograms().isEmpty()) { + return 0L; + } + String sql = getCountQuery(params); return jdbcTemplate.queryForObject(sql, Long.class); } public int getTrackedEntityCountWithMaxTrackedEntityLimit(TrackedEntityQueryParams params) { + // A TE which is not enrolled can only be accessed by a user that is able to enroll it into a + // tracker program. Return an empty result if there are no tracker programs or the user does + // not have access to one. + if (!params.hasEnrolledInTrackerProgram() && params.getAccessibleTrackerPrograms().isEmpty()) { + return 0; + } + String sql = getCountQueryWithMaxTrackedEntityLimit(params); return jdbcTemplate.queryForObject(sql, Integer.class); } @@ -306,8 +334,8 @@ private String getCountQueryWithMaxTrackedEntityLimit(TrackedEntityQueryParams p + getQuerySelect(params) + "FROM " + getFromSubQuery(params, true, null) - + (params.getProgram().getMaxTeiCountToReturn() > 0 - ? getLimitClause(params.getProgram().getMaxTeiCountToReturn() + 1) + + (params.getEnrolledInTrackerProgram().getMaxTeiCountToReturn() > 0 + ? getLimitClause(params.getEnrolledInTrackerProgram().getMaxTeiCountToReturn() + 1) : "") + " ) tecount"; } @@ -439,10 +467,10 @@ private String joinPrograms(TrackedEntityQueryParams params) { trackedEntity.append(" INNER JOIN program P "); trackedEntity.append(" ON P.trackedentitytypeid = TE.trackedentitytypeid "); - if (!params.hasProgram()) { + if (!params.hasEnrolledInTrackerProgram()) { trackedEntity .append("AND P.programid IN (") - .append(getCommaDelimitedString(getIdentifiers(params.getPrograms()))) + .append(getCommaDelimitedString(getIdentifiers(params.getAccessibleTrackerPrograms()))) .append(")"); } @@ -472,7 +500,7 @@ private String getFromSubQueryTrackedEntityConditions( .append(whereAnd.whereAnd()) .append("TE.trackedentitytypeid = ") .append(params.getTrackedEntityType().getId()); - } else if (!params.hasProgram()) { + } else if (!params.hasEnrolledInTrackerProgram()) { trackedEntity .append(whereAnd.whereAnd()) .append("TE.trackedentitytypeid in (") @@ -594,10 +622,10 @@ private String getFromSubQueryJoinOrderByAttributes(TrackedEntityQueryParams par * @return a SQL INNER JOIN for program owner, or a LEFT JOIN if no program is specified. */ private String getFromSubQueryJoinProgramOwnerConditions(TrackedEntityQueryParams params) { - if (params.hasProgram()) { + if (params.hasEnrolledInTrackerProgram()) { return " INNER JOIN trackedentityprogramowner PO " + " ON PO.programid = " - + params.getProgram().getId() + + params.getEnrolledInTrackerProgram().getId() + " AND PO.trackedentityid = TE.trackedentityid " + " AND P.programid = PO.programid"; } @@ -662,7 +690,7 @@ private void handleOrganisationUnits(TrackedEntityQueryParams params) { } private String getOwnerOrgUnit(TrackedEntityQueryParams params) { - if (params.hasProgram()) { + if (params.hasEnrolledInTrackerProgram()) { return "PO.organisationunitid "; } @@ -736,10 +764,10 @@ private String getFromSubQueryJoinEnrollmentConditions(TrackedEntityQueryParams ON %1$s.trackedentityid = TE.trackedentityid """; - return !params.hasProgram() + return !params.hasEnrolledInTrackerProgram() ? join.formatted(ENROLLMENT_ALIAS) : join.concat(" AND %1$s.programid = %2$s") - .formatted(ENROLLMENT_ALIAS, params.getProgram().getId()); + .formatted(ENROLLMENT_ALIAS, params.getEnrolledInTrackerProgram().getId()); } return ""; @@ -757,7 +785,7 @@ private String getFromSubQueryEnrollmentConditions( SqlHelper whereAnd, TrackedEntityQueryParams params) { StringBuilder program = new StringBuilder(); - if (!params.hasProgram()) { + if (!params.hasEnrolledInTrackerProgram()) { return ""; } @@ -774,7 +802,7 @@ private String getFromSubQueryEnrollmentConditions( program .append("WHERE EN.trackedentityid = TE.trackedentityid ") .append("AND EN.programid = ") - .append(params.getProgram().getId()) + .append(params.getEnrolledInTrackerProgram().getId()) .append(SPACE); if (params.hasEnrollmentStatus()) { diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapper.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapper.java index d00577d98149..010c4e898628 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapper.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapper.java @@ -114,8 +114,8 @@ public TrackedEntityQueryParams map( program, requestedTrackedEntityType, operationParams, orgUnits, params); params - .setProgram(program) - .setPrograms(programs) + .setEnrolledInTrackerProgram(program) + .setAccessibleTrackerPrograms(programs) .setProgramStage(programStage) .setEnrollmentStatus(operationParams.getEnrollmentStatus()) .setFollowUp(operationParams.getFollowUp()) @@ -297,8 +297,9 @@ private void validateGlobalSearchParameters(TrackedEntityQueryParams params, Use private List getSearchableAttributeIds(TrackedEntityQueryParams params) { List searchableAttributeIds = new ArrayList<>(); - if (params.hasProgram()) { - searchableAttributeIds.addAll(UID.of(params.getProgram().getSearchableAttributeIds())); + if (params.hasEnrolledInTrackerProgram()) { + searchableAttributeIds.addAll( + UID.of(params.getEnrolledInTrackerProgram().getSearchableAttributeIds())); } if (params.hasTrackedEntityType()) { @@ -306,7 +307,7 @@ private List getSearchableAttributeIds(TrackedEntityQueryParams params) { UID.of(params.getTrackedEntityType().getSearchableAttributeIds())); } - if (!params.hasProgram() && !params.hasTrackedEntityType()) { + if (!params.hasEnrolledInTrackerProgram() && !params.hasTrackedEntityType()) { searchableAttributeIds.addAll( trackedEntityAttributeService.getAllSystemWideUniqueTrackedEntityAttributes().stream() .map(UID::of) @@ -346,13 +347,13 @@ private int getMaxTeiLimit(TrackedEntityQueryParams params) { } } - if (params.hasProgram()) { - maxTeiLimit = params.getProgram().getMaxTeiCountToReturn(); + if (params.hasEnrolledInTrackerProgram()) { + maxTeiLimit = params.getEnrolledInTrackerProgram().getMaxTeiCountToReturn(); if (!params.hasTrackedEntities() && isProgramMinAttributesViolated(params)) { throw new IllegalQueryException( "At least " - + params.getProgram().getMinAttributesRequiredToSearch() + + params.getEnrolledInTrackerProgram().getMinAttributesRequiredToSearch() + " attributes should be mentioned in the search criteria."); } } @@ -405,9 +406,11 @@ private boolean isProgramMinAttributesViolated(TrackedEntityQueryParams params) return false; } - return (!params.hasFilters() && params.getProgram().getMinAttributesRequiredToSearch() > 0) + return (!params.hasFilters() + && params.getEnrolledInTrackerProgram().getMinAttributesRequiredToSearch() > 0) || (params.hasFilters() - && params.getFilters().size() < params.getProgram().getMinAttributesRequiredToSearch()); + && params.getFilters().size() + < params.getEnrolledInTrackerProgram().getMinAttributesRequiredToSearch()); } private void checkIfMaxTeiLimitIsReached(TrackedEntityQueryParams params, int maxTeiLimit) { diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityQueryParams.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityQueryParams.java index 4a3f7f3db180..4d646c41471e 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityQueryParams.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityQueryParams.java @@ -68,11 +68,17 @@ public class TrackedEntityQueryParams { */ private Set orgUnits = new HashSet<>(); - /** Program for which instances in the response must be enrolled in. */ - private Program program; + /** + * Tracker program the tracked entity must be enrolled in. This should not be set when {@link + * #accessibleTrackerPrograms} is set. The user must have data read access to this program. + */ + private Program enrolledInTrackerProgram; - /** Programs to fetch. */ - private List programs = List.of(); + /** + * Tracker programs the user has data read access to. This should not be set when {@link + * #enrolledInTrackerProgram} is set. + */ + private List accessibleTrackerPrograms = List.of(); /** Status of a tracked entities enrollment into a given program. */ private EnrollmentStatus enrollmentStatus; @@ -172,8 +178,8 @@ public boolean hasOrganisationUnits() { } /** Indicates whether these parameters specify a program. */ - public boolean hasProgram() { - return program != null; + public boolean hasEnrolledInTrackerProgram() { + return enrolledInTrackerProgram != null; } /** Indicates whether these parameters specify an enrollment status. */ @@ -308,21 +314,22 @@ public TrackedEntityQueryParams setOrgUnits(Set accessibleOrgU return this; } - public Program getProgram() { - return program; + public Program getEnrolledInTrackerProgram() { + return enrolledInTrackerProgram; } - public TrackedEntityQueryParams setProgram(Program program) { - this.program = program; + public TrackedEntityQueryParams setEnrolledInTrackerProgram(Program enrolledInTrackerProgram) { + this.enrolledInTrackerProgram = enrolledInTrackerProgram; return this; } - public List getPrograms() { - return programs; + public List getAccessibleTrackerPrograms() { + return accessibleTrackerPrograms; } - public TrackedEntityQueryParams setPrograms(List programs) { - this.programs = programs; + public TrackedEntityQueryParams setAccessibleTrackerPrograms( + List accessibleTrackerPrograms) { + this.accessibleTrackerPrograms = accessibleTrackerPrograms; return this; } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/aggregates/TrackedEntityAggregate.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/aggregates/TrackedEntityAggregate.java index 4bd0bce98f0b..be77fb4ee155 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/aggregates/TrackedEntityAggregate.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/aggregates/TrackedEntityAggregate.java @@ -241,7 +241,7 @@ public List find( Stream teUidStream = trackedEntities.keySet().parallelStream(); - if (user.isPresent() && queryParams.hasProgram()) { + if (user.isPresent() && queryParams.hasEnrolledInTrackerProgram()) { teUidStream = teUidStream.filter(ownedTeis::containsKey); } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/aggregates/TrackedEntityStore.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/aggregates/TrackedEntityStore.java index 8a8c986649dc..b248b7b0bdff 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/aggregates/TrackedEntityStore.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/aggregates/TrackedEntityStore.java @@ -183,9 +183,10 @@ private Multimap getOwnedTrackedEntitiesPartitioned( String sql; - if (ctx.getQueryParams().hasProgram()) { + if (ctx.getQueryParams().hasEnrolledInTrackerProgram()) { sql = getTrackedEntitiesOwnershipSqlForSpecificProgram(skipUserScopeValidation); - paramSource.addValue("programUid", ctx.getQueryParams().getProgram().getUid()); + paramSource.addValue( + "programUid", ctx.getQueryParams().getEnrolledInTrackerProgram().getUid()); } else if (checkForOwnership) { sql = getTrackedEntitiesOwnershipSqlForAllPrograms(skipUserScopeValidation); } else { diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapperTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapperTest.java index 7a902e11770e..adec72ac5316 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapperTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapperTest.java @@ -361,7 +361,7 @@ void testMappingProgram() throws BadRequestException, ForbiddenException { TrackedEntityQueryParams params = mapper.map(operationParams, user); - assertEquals(program, params.getProgram()); + assertEquals(program, params.getEnrolledInTrackerProgram()); } @Test diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java index c85911e4bd98..619ef29435f2 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java @@ -102,6 +102,8 @@ import org.hisp.dhis.trackedentity.TrackedEntityTypeAttribute; import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue; import org.hisp.dhis.tracker.acl.TrackedEntityProgramOwnerService; +import org.hisp.dhis.tracker.export.Page; +import org.hisp.dhis.tracker.export.PageParams; import org.hisp.dhis.tracker.export.trackedentity.TrackedEntityOperationParams.TrackedEntityOperationParamsBuilder; import org.hisp.dhis.tracker.trackedentityattributevalue.TrackedEntityAttributeValueService; import org.hisp.dhis.user.User; @@ -258,6 +260,7 @@ void setUp() { new TrackedEntityTypeAttribute(trackedEntityTypeA, teaB))); trackedEntityTypeA.getSharing().setOwner(user); trackedEntityTypeA.setPublicAccess(AccessStringHelper.FULL); + trackedEntityTypeA.setMinAttributesRequiredToSearch(0); manager.save(trackedEntityTypeA, false); CategoryCombo defaultCategoryCombo = manager.getByName(CategoryCombo.class, "default"); @@ -1896,6 +1899,39 @@ void shouldFailWhenRequestingSingleTEAndTETNotAccessible() { exception.getMessage()); } + @Test + void shouldReturnEmptyResultIfUserHasNoAccessToAnyTrackerProgram() + throws ForbiddenException, BadRequestException, NotFoundException { + injectSecurityContextUser(getAdminUser()); + makeProgramMetadataAccessibleOnly(programA); + makeProgramMetadataAccessibleOnly(programB); + makeProgramMetadataAccessibleOnly(programC); + injectSecurityContextUser(user); + + TrackedEntityOperationParams operationParams = + TrackedEntityOperationParams.builder().trackedEntityType(trackedEntityTypeA).build(); + + assertIsEmpty(trackedEntityService.getTrackedEntities(operationParams)); + } + + @Test + void shouldReturnEmptyPageIfUserHasNoAccessToAnyTrackerProgram() + throws ForbiddenException, BadRequestException, NotFoundException { + injectSecurityContextUser(getAdminUser()); + makeProgramMetadataAccessibleOnly(programA); + makeProgramMetadataAccessibleOnly(programB); + makeProgramMetadataAccessibleOnly(programC); + injectSecurityContextUser(user); + + TrackedEntityOperationParams operationParams = + TrackedEntityOperationParams.builder().trackedEntityType(trackedEntityTypeA).build(); + + Page trackedEntities = + trackedEntityService.getTrackedEntities(operationParams, new PageParams(1, 3, false)); + + assertIsEmpty(trackedEntities.getItems()); + } + @Test void shouldFailWhenRequestingSingleTEAndNoMetadataAccessToAnyProgram() { injectAdminIntoSecurityContext();