From c0513e2918409fb44849ba52a8a72f1a717a2b43 Mon Sep 17 00:00:00 2001 From: Giuseppe Nespolino Date: Fri, 17 Jan 2025 13:42:24 +0100 Subject: [PATCH 1/5] fix: quotes ouIds for IN filter, resolve ou uids [DHIS2-18845] Signed-off-by: Giuseppe Nespolino --- .../org/hisp/dhis/common/InQueryFilter.java | 10 +++---- .../hisp/dhis/common/InQueryFilterTest.java | 8 +++--- .../hisp/dhis/analytics/DataQueryService.java | 11 ++++++++ .../data/DefaultDataQueryService.java | 15 +++++++++++ .../data/DimensionalObjectProvider.java | 18 +++++++++++++ .../AbstractJdbcEventAnalyticsManager.java | 7 ++--- .../data/DefaultEventDataQueryService.java | 26 ++++++++++++++++++- ...AbstractJdbcEventAnalyticsManagerTest.java | 18 +++++++++++++ 8 files changed, 100 insertions(+), 13 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java index 0df52d46f28b..cc49c8ffd83a 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java @@ -46,19 +46,19 @@ public class InQueryFilter extends QueryFilter { private final String field; - private final boolean isText; + private final boolean shouldQuote; /** * Construct a InQueryFilter using field name and the original {@link QueryFilter} * * @param field the field on which to construct the InQueryFilter * @param encodedFilter The original encodedFilter in {@link QueryFilter} - * @param isText whether this filter contains text or numeric values + * @param shouldQuote whether this filter contains text or numeric values */ - public InQueryFilter(String field, String encodedFilter, boolean isText) { + public InQueryFilter(String field, String encodedFilter, boolean shouldQuote) { super(IN, encodedFilter); this.field = field; - this.isText = isText; + this.shouldQuote = shouldQuote; } /** @@ -100,7 +100,7 @@ public String getSqlFilter() { } private String quoteIfNecessary(String item) { - return isText ? quote(item) : item; + return shouldQuote ? quote(item) : item; } private boolean hasMissingValue(List filterItems) { diff --git a/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java b/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java index b492f985d2fe..2794859657c7 100644 --- a/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java +++ b/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java @@ -64,11 +64,11 @@ void verifyNestedSqlStmtInFieldWithNullOnly() { executeTest(field, "NV", true, "(" + field + " is null and exists((select * from xy))) "); } - private void executeTest(String filterValue, boolean isText, String expected) { - executeTest("aField", filterValue, isText, expected); + private void executeTest(String filterValue, boolean shouldQuote, String expected) { + executeTest("aField", filterValue, shouldQuote, expected); } - private void executeTest(String field, String filterValue, boolean isText, String expected) { - assertEquals(new InQueryFilter(field, filterValue, isText).getSqlFilter(), expected); + private void executeTest(String field, String filterValue, boolean shouldQuote, String expected) { + assertEquals(new InQueryFilter(field, filterValue, shouldQuote).getSqlFilter(), expected); } } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java index b7ce9bdd221d..871032a2e62e 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java @@ -176,4 +176,15 @@ DimensionalObject getDimension( * @return a list of organisation units. */ List getUserOrgUnits(DataQueryParams params, String userOrgUnit); + + /** + * Based on the given parameters, this method will return a list of {@link OrganisationUnit} UIDs + * based on the given items and user organisation units. + * + * @param items the list of items that might be included into the resulting organisation unit and + * its keywords. + * @param userOrgUnits the list of organisation units associated with the current user. + * @return a list of {@link OrganisationUnit} UIDs. + */ + List getOrgUnitDimensionUid(List items, List userOrgUnits); } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java index b5511f5ebad6..4ae91c48c73d 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java @@ -363,6 +363,21 @@ private List getDimensionalObjects(DataQueryRequest request) return list; } + /** + * Based on the given parameters, this method will return a list of {@link OrganisationUnit} UIDs + * based on the given items and user organisation units. + * + * @param items the list of items that might be included into the resulting organisation unit and + * its keywords. + * @param userOrgUnits the list of organisation units associated with the current user. + * @return a list of {@link OrganisationUnit} UIDs. + */ + @Override + public List getOrgUnitDimensionUid( + List items, List userOrgUnits) { + return dimensionalObjectProducer.getOrgUnitDimensionUid(items, userOrgUnits); + } + /** * Returns a {@link DimensionalObject}. * diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java index c247113b657f..4dcce1197a36 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java @@ -423,6 +423,24 @@ public BaseDimensionalObject getOrgUnitDimension( dimensionalKeywords); } + /** + * This method will return a list of {@link OrganisationUnit} UIDs based on the given items and + * user organisation units. + * + * @param items the list of items that might be included into the resulting organisation unit and + * its keywords. + * @param userOrgUnits the list of organisation units associated with the current user. + * @return a list of {@link OrganisationUnit} UIDs. + */ + public List getOrgUnitDimensionUid( + List items, List userOrgUnits) { + return getOrgUnitDimensionItems( + items, userOrgUnits, IdScheme.UID, new ArrayList<>(), new ArrayList<>()) + .stream() + .map(DimensionalItemObject::getUid) + .toList(); + } + /** * Based on the given parameters, this method will return a list of {@link DimensionalItemObject} * of type {@link OrganisationUnit}. It also adds to the given levels and groups if certain diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java index 47952b35a9d4..8055af5b6912 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java @@ -1244,13 +1244,14 @@ private static class IdentifiableSql { } /** - * Creates a SQL statement for a single filter inside a query item. + * Creates a SQL statement for a single filter inside a query item. Made public for testing + * purposes. * * @param item the {@link QueryItem}. * @param filter the {@link QueryFilter}. * @param params the {@link EventQueryParams}. */ - private String toSql(QueryItem item, QueryFilter filter, EventQueryParams params) { + public String toSql(QueryItem item, QueryFilter filter, EventQueryParams params) { String field = item.hasAggregationType() ? getSelectSql(filter, item, params) @@ -1258,7 +1259,7 @@ private String toSql(QueryItem item, QueryFilter filter, EventQueryParams params if (IN.equals(filter.getOperator())) { InQueryFilter inQueryFilter = - new InQueryFilter(field, sqlBuilder.escape(filter.getFilter()), item.isText()); + new InQueryFilter(field, sqlBuilder.escape(filter.getFilter()), !item.isNumeric()); return inQueryFilter.getSqlFilter(); } else { diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java index 533bd5c3229e..cf13563ca647 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java @@ -37,13 +37,16 @@ import static org.hisp.dhis.analytics.util.AnalyticsUtils.illegalQueryExSupplier; import static org.hisp.dhis.analytics.util.AnalyticsUtils.throwIllegalQueryEx; import static org.hisp.dhis.common.DimensionalObject.DIMENSION_NAME_SEP; +import static org.hisp.dhis.common.DimensionalObject.OPTION_SEP; import static org.hisp.dhis.common.DimensionalObject.PERIOD_DIM_ID; import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionFromParam; import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionItemsFromParam; import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionalItemIds; +import static org.hisp.dhis.common.ValueType.ORGANISATION_UNIT; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Locale; @@ -54,6 +57,7 @@ import java.util.stream.Collectors; import lombok.Getter; import lombok.RequiredArgsConstructor; +import lombok.SneakyThrows; import org.apache.commons.lang3.StringUtils; import org.hisp.dhis.analytics.AnalyticsAggregationType; import org.hisp.dhis.analytics.DataQueryParams; @@ -318,7 +322,7 @@ private void addDimensionsToParams( if (groupableItem != null) { params.addDimension((DimensionalObject) groupableItem); } else { - groupableItem = getQueryItem(dim, pr, request.getOutputType()); + groupableItem = getQueryItem(dim, pr, request.getOutputType(), userOrgUnits); params.addItem((QueryItem) groupableItem); } @@ -511,6 +515,14 @@ private QueryItem getQueryItem( @Override public QueryItem getQueryItem(String dimensionString, Program program, EventOutputType type) { + return getQueryItem(dimensionString, program, type, Collections.emptyList()); + } + + private QueryItem getQueryItem( + String dimensionString, + Program program, + EventOutputType type, + List userOrgUnits) { String[] split = dimensionString.split(DIMENSION_NAME_SEP); if (split.length % 2 != 1) { @@ -536,6 +548,7 @@ public QueryItem getQueryItem(String dimensionString, Program program, EventOutp // FE uses HH.MM time format instead of HH:MM. This is not // compatible with db table/cell values modifyFilterWhenTimeQueryItem(queryItem, filter); + resolveOrgUnitDimensionIfNecessary(queryItem, filter, userOrgUnits); queryItem.addFilter(filter); } } @@ -543,6 +556,17 @@ public QueryItem getQueryItem(String dimensionString, Program program, EventOutp return queryItem; } + @SneakyThrows + private void resolveOrgUnitDimensionIfNecessary( + QueryItem queryItem, QueryFilter queryFilter, List userOrgUnits) { + if (queryItem.getValueType().equals(ORGANISATION_UNIT)) { + List filterItem = QueryFilter.getFilterItems(queryFilter.getFilter()); + List orgUnitDimensionUid = + dataQueryService.getOrgUnitDimensionUid(filterItem, userOrgUnits); + queryFilter.setFilter(String.join(OPTION_SEP, orgUnitDimensionUid)); + } + } + private static void modifyFilterWhenTimeQueryItem(QueryItem queryItem, QueryFilter filter) { if (queryItem.getItem() instanceof DataElement && ((DataElement) queryItem.getItem()).getValueType() == ValueType.TIME) { diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java index c78346771352..8cdef3e9a473 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java @@ -36,6 +36,7 @@ import static org.hisp.dhis.analytics.AnalyticsAggregationType.fromAggregationType; import static org.hisp.dhis.analytics.DataType.NUMERIC; import static org.hisp.dhis.common.QueryOperator.EQ; +import static org.hisp.dhis.common.QueryOperator.IN; import static org.hisp.dhis.common.QueryOperator.NE; import static org.hisp.dhis.common.QueryOperator.NEQ; import static org.hisp.dhis.common.QueryOperator.NIEQ; @@ -852,6 +853,23 @@ void testGetSelectClauseForAggregatedEnrollments() { assertEquals("select enrollment,Yearly ", select); } + @Test + void testItemsInFilterAreQuotedForOrganisationUnit() { + // Given + QueryItem queryItem = mock(QueryItem.class); + QueryFilter queryFilter = new QueryFilter(IN, "A;B;C"); + EventQueryParams params = + new EventQueryParams.Builder().withStartDate(new Date()).withEndDate(new Date()).build(); + when(queryItem.getItemName()).thenReturn("anyItem"); + when(queryItem.getValueType()).thenReturn(ValueType.ORGANISATION_UNIT); + + // When + String sql = eventSubject.toSql(queryItem, queryFilter, params).trim(); + + // Then + assertEquals("ax.\"anyItem\" in ('A','B','C')", sql); + } + @Test void testGetSelectClauseForQueryEnrollments() { // Given From edffe21ac3a18fbeb1ed0f19d28f80a3562a5065 Mon Sep 17 00:00:00 2001 From: Giuseppe Nespolino Date: Mon, 20 Jan 2025 13:41:24 +0100 Subject: [PATCH 2/5] test: added e2e test [DHIS2-18845] Signed-off-by: Giuseppe Nespolino --- .../analytics/event/query/EventQueryTest.java | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java index 6ad7d7c4b24b..11001828eae1 100644 --- a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java +++ b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java @@ -997,4 +997,68 @@ public void queryMetadataInfoForOptionSetAndOptionsWhenTenRows() throws JSONExce // Assert rows. validateRow(response, 0, List.of("Ngelehun CHC", "1")); } + + @Test + public void queryWithOrgUnitDataElement() throws JSONException { + // Given + String dimensionItems = + String.join( + ";", + "DiszpKrYNg8", + "g8upMTyEZGZ", + "LEVEL-H1KlN4QIauv", + "OU_GROUP-nlX2VoouN63", + "USER_ORGUNIT;USER_ORGUNIT_CHILDREN", + "USER_ORGUNIT_GRANDCHILDREN"); + + String dimensionOrgUnitDataElement = "Ge7Eo3FNnbl.rypjN8CV02V:IN:" + dimensionItems; + String dimensionOrgUnit = "ou:USER_ORGUNIT"; + + String dimension = dimensionOrgUnitDataElement + "," + dimensionOrgUnit; + + QueryParamsBuilder params = + new QueryParamsBuilder() + .add("dimension=" + dimension) + .add("headers=ouname,Ge7Eo3FNnbl.rypjN8CV02V") + .add("totalPages=false") + .add("displayProperty=NAME") + .add("pageSize=100") + .add("page=1") + .add("includeMetadataDetails=true") + .add("outputType=EVENT"); + + // When + ApiResponse response = analyticsEventActions.query().get("MoUd5BTQ3lY", params); + + // Then + response + .validate() + .statusCode(200) + .body("headers", hasSize(equalTo(2))) + .body("rows", hasSize(equalTo(0))) + .body("height", equalTo(0)) + .body("width", equalTo(0)) + .body("headerWidth", equalTo(2)); + + // Assert metaData. + String expectedMetaData = + "{\"pager\":{\"isLastPage\":true,\"pageSize\":100,\"page\":1},\"items\":{\"ImspTQPwCqd\":{\"uid\":\"ImspTQPwCqd\",\"code\":\"OU_525\",\"valueType\":\"TEXT\",\"name\":\"Sierra Leone\",\"dimensionItemType\":\"ORGANISATION_UNIT\",\"totalAggregationType\":\"SUM\"},\"USER_ORGUNIT\":{\"organisationUnits\":[\"ImspTQPwCqd\"]},\"ou\":{\"uid\":\"ou\",\"dimensionType\":\"ORGANISATION_UNIT\",\"name\":\"Organisation unit\"},\"Ge7Eo3FNnbl\":{\"uid\":\"Ge7Eo3FNnbl\",\"name\":\"XX MAL RDT - Case Registration\"},\"Ge7Eo3FNnbl.rypjN8CV02V\":{\"uid\":\"rypjN8CV02V\",\"aggregationType\":\"SUM\",\"valueType\":\"TEXT\",\"name\":\"XX MAL RDT TRK - Village of Residence\",\"style\":{\"icon\":\"nullapi\\/icons\\/star_medium_positive\\/icon.svg\"},\"dimensionItemType\":\"DATA_ELEMENT\",\"totalAggregationType\":\"SUM\"},\"MoUd5BTQ3lY\":{\"uid\":\"MoUd5BTQ3lY\",\"name\":\"XX MAL RDT - Case Registration\"},\"USER_ORGUNIT_CHILDREN\":{\"organisationUnits\":[\"at6UHUQatSo\",\"TEQlaapDQoK\",\"PMa2VCrupOd\",\"qhqAxPSTUXp\",\"kJq2mPyFEHo\",\"jmIPBj66vD6\",\"Vth0fbpFcsO\",\"jUb8gELQApl\",\"fdc6uOvgoji\",\"eIQbndfxQMb\",\"O6uvpzGd5pu\",\"lc3eMKXaEfw\",\"bL4ooGhyHRQ\"]},\"rypjN8CV02V\":{\"uid\":\"rypjN8CV02V\",\"aggregationType\":\"SUM\",\"valueType\":\"TEXT\",\"name\":\"XX MAL RDT TRK - Village of Residence\",\"style\":{\"icon\":\"nullapi\\/icons\\/star_medium_positive\\/icon.svg\"},\"dimensionItemType\":\"DATA_ELEMENT\",\"totalAggregationType\":\"SUM\"},\"LAST_12_MONTHS\":{\"name\":\"Last 12 months\"}},\"dimensions\":{\"pe\":[],\"ou\":[\"ImspTQPwCqd\"],\"Ge7Eo3FNnbl.rypjN8CV02V\":[]}}"; + String actualMetaData = new JSONObject((Map) response.extract("metaData")).toString(); + assertEquals(expectedMetaData, actualMetaData, false); + + // Assert headers. + validateHeader( + response, 0, "ouname", "Organisation unit name", "TEXT", "java.lang.String", false, true); + validateHeader( + response, + 1, + "Ge7Eo3FNnbl.rypjN8CV02V", + "XX MAL RDT TRK - Village of Residence", + "ORGANISATION_UNIT", + "org.hisp.dhis.organisationunit.OrganisationUnit", + false, + true); + + // no rows to assert + } } From 008c7151b521c0591f9c29b1c55ecff8180d4f64 Mon Sep 17 00:00:00 2001 From: Giuseppe Nespolino Date: Fri, 17 Jan 2025 13:42:24 +0100 Subject: [PATCH 3/5] fix: quotes ouIds for IN filter, resolve ou uids [DHIS2-18845] Signed-off-by: Giuseppe Nespolino --- .../org/hisp/dhis/common/InQueryFilter.java | 10 +++---- .../hisp/dhis/common/InQueryFilterTest.java | 8 +++--- .../hisp/dhis/analytics/DataQueryService.java | 11 ++++++++ .../data/DefaultDataQueryService.java | 15 +++++++++++ .../data/DimensionalObjectProvider.java | 18 +++++++++++++ .../AbstractJdbcEventAnalyticsManager.java | 7 ++--- .../data/DefaultEventDataQueryService.java | 26 ++++++++++++++++++- ...AbstractJdbcEventAnalyticsManagerTest.java | 18 +++++++++++++ 8 files changed, 100 insertions(+), 13 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java index 0df52d46f28b..cc49c8ffd83a 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/InQueryFilter.java @@ -46,19 +46,19 @@ public class InQueryFilter extends QueryFilter { private final String field; - private final boolean isText; + private final boolean shouldQuote; /** * Construct a InQueryFilter using field name and the original {@link QueryFilter} * * @param field the field on which to construct the InQueryFilter * @param encodedFilter The original encodedFilter in {@link QueryFilter} - * @param isText whether this filter contains text or numeric values + * @param shouldQuote whether this filter contains text or numeric values */ - public InQueryFilter(String field, String encodedFilter, boolean isText) { + public InQueryFilter(String field, String encodedFilter, boolean shouldQuote) { super(IN, encodedFilter); this.field = field; - this.isText = isText; + this.shouldQuote = shouldQuote; } /** @@ -100,7 +100,7 @@ public String getSqlFilter() { } private String quoteIfNecessary(String item) { - return isText ? quote(item) : item; + return shouldQuote ? quote(item) : item; } private boolean hasMissingValue(List filterItems) { diff --git a/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java b/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java index b492f985d2fe..2794859657c7 100644 --- a/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java +++ b/dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/InQueryFilterTest.java @@ -64,11 +64,11 @@ void verifyNestedSqlStmtInFieldWithNullOnly() { executeTest(field, "NV", true, "(" + field + " is null and exists((select * from xy))) "); } - private void executeTest(String filterValue, boolean isText, String expected) { - executeTest("aField", filterValue, isText, expected); + private void executeTest(String filterValue, boolean shouldQuote, String expected) { + executeTest("aField", filterValue, shouldQuote, expected); } - private void executeTest(String field, String filterValue, boolean isText, String expected) { - assertEquals(new InQueryFilter(field, filterValue, isText).getSqlFilter(), expected); + private void executeTest(String field, String filterValue, boolean shouldQuote, String expected) { + assertEquals(new InQueryFilter(field, filterValue, shouldQuote).getSqlFilter(), expected); } } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java index b7ce9bdd221d..871032a2e62e 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryService.java @@ -176,4 +176,15 @@ DimensionalObject getDimension( * @return a list of organisation units. */ List getUserOrgUnits(DataQueryParams params, String userOrgUnit); + + /** + * Based on the given parameters, this method will return a list of {@link OrganisationUnit} UIDs + * based on the given items and user organisation units. + * + * @param items the list of items that might be included into the resulting organisation unit and + * its keywords. + * @param userOrgUnits the list of organisation units associated with the current user. + * @return a list of {@link OrganisationUnit} UIDs. + */ + List getOrgUnitDimensionUid(List items, List userOrgUnits); } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java index b5511f5ebad6..4ae91c48c73d 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java @@ -363,6 +363,21 @@ private List getDimensionalObjects(DataQueryRequest request) return list; } + /** + * Based on the given parameters, this method will return a list of {@link OrganisationUnit} UIDs + * based on the given items and user organisation units. + * + * @param items the list of items that might be included into the resulting organisation unit and + * its keywords. + * @param userOrgUnits the list of organisation units associated with the current user. + * @return a list of {@link OrganisationUnit} UIDs. + */ + @Override + public List getOrgUnitDimensionUid( + List items, List userOrgUnits) { + return dimensionalObjectProducer.getOrgUnitDimensionUid(items, userOrgUnits); + } + /** * Returns a {@link DimensionalObject}. * diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java index c247113b657f..4dcce1197a36 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java @@ -423,6 +423,24 @@ public BaseDimensionalObject getOrgUnitDimension( dimensionalKeywords); } + /** + * This method will return a list of {@link OrganisationUnit} UIDs based on the given items and + * user organisation units. + * + * @param items the list of items that might be included into the resulting organisation unit and + * its keywords. + * @param userOrgUnits the list of organisation units associated with the current user. + * @return a list of {@link OrganisationUnit} UIDs. + */ + public List getOrgUnitDimensionUid( + List items, List userOrgUnits) { + return getOrgUnitDimensionItems( + items, userOrgUnits, IdScheme.UID, new ArrayList<>(), new ArrayList<>()) + .stream() + .map(DimensionalItemObject::getUid) + .toList(); + } + /** * Based on the given parameters, this method will return a list of {@link DimensionalItemObject} * of type {@link OrganisationUnit}. It also adds to the given levels and groups if certain diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java index 47952b35a9d4..8055af5b6912 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java @@ -1244,13 +1244,14 @@ private static class IdentifiableSql { } /** - * Creates a SQL statement for a single filter inside a query item. + * Creates a SQL statement for a single filter inside a query item. Made public for testing + * purposes. * * @param item the {@link QueryItem}. * @param filter the {@link QueryFilter}. * @param params the {@link EventQueryParams}. */ - private String toSql(QueryItem item, QueryFilter filter, EventQueryParams params) { + public String toSql(QueryItem item, QueryFilter filter, EventQueryParams params) { String field = item.hasAggregationType() ? getSelectSql(filter, item, params) @@ -1258,7 +1259,7 @@ private String toSql(QueryItem item, QueryFilter filter, EventQueryParams params if (IN.equals(filter.getOperator())) { InQueryFilter inQueryFilter = - new InQueryFilter(field, sqlBuilder.escape(filter.getFilter()), item.isText()); + new InQueryFilter(field, sqlBuilder.escape(filter.getFilter()), !item.isNumeric()); return inQueryFilter.getSqlFilter(); } else { diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java index 533bd5c3229e..cf13563ca647 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultEventDataQueryService.java @@ -37,13 +37,16 @@ import static org.hisp.dhis.analytics.util.AnalyticsUtils.illegalQueryExSupplier; import static org.hisp.dhis.analytics.util.AnalyticsUtils.throwIllegalQueryEx; import static org.hisp.dhis.common.DimensionalObject.DIMENSION_NAME_SEP; +import static org.hisp.dhis.common.DimensionalObject.OPTION_SEP; import static org.hisp.dhis.common.DimensionalObject.PERIOD_DIM_ID; import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionFromParam; import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionItemsFromParam; import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionalItemIds; +import static org.hisp.dhis.common.ValueType.ORGANISATION_UNIT; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Locale; @@ -54,6 +57,7 @@ import java.util.stream.Collectors; import lombok.Getter; import lombok.RequiredArgsConstructor; +import lombok.SneakyThrows; import org.apache.commons.lang3.StringUtils; import org.hisp.dhis.analytics.AnalyticsAggregationType; import org.hisp.dhis.analytics.DataQueryParams; @@ -318,7 +322,7 @@ private void addDimensionsToParams( if (groupableItem != null) { params.addDimension((DimensionalObject) groupableItem); } else { - groupableItem = getQueryItem(dim, pr, request.getOutputType()); + groupableItem = getQueryItem(dim, pr, request.getOutputType(), userOrgUnits); params.addItem((QueryItem) groupableItem); } @@ -511,6 +515,14 @@ private QueryItem getQueryItem( @Override public QueryItem getQueryItem(String dimensionString, Program program, EventOutputType type) { + return getQueryItem(dimensionString, program, type, Collections.emptyList()); + } + + private QueryItem getQueryItem( + String dimensionString, + Program program, + EventOutputType type, + List userOrgUnits) { String[] split = dimensionString.split(DIMENSION_NAME_SEP); if (split.length % 2 != 1) { @@ -536,6 +548,7 @@ public QueryItem getQueryItem(String dimensionString, Program program, EventOutp // FE uses HH.MM time format instead of HH:MM. This is not // compatible with db table/cell values modifyFilterWhenTimeQueryItem(queryItem, filter); + resolveOrgUnitDimensionIfNecessary(queryItem, filter, userOrgUnits); queryItem.addFilter(filter); } } @@ -543,6 +556,17 @@ public QueryItem getQueryItem(String dimensionString, Program program, EventOutp return queryItem; } + @SneakyThrows + private void resolveOrgUnitDimensionIfNecessary( + QueryItem queryItem, QueryFilter queryFilter, List userOrgUnits) { + if (queryItem.getValueType().equals(ORGANISATION_UNIT)) { + List filterItem = QueryFilter.getFilterItems(queryFilter.getFilter()); + List orgUnitDimensionUid = + dataQueryService.getOrgUnitDimensionUid(filterItem, userOrgUnits); + queryFilter.setFilter(String.join(OPTION_SEP, orgUnitDimensionUid)); + } + } + private static void modifyFilterWhenTimeQueryItem(QueryItem queryItem, QueryFilter filter) { if (queryItem.getItem() instanceof DataElement && ((DataElement) queryItem.getItem()).getValueType() == ValueType.TIME) { diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java index c78346771352..8cdef3e9a473 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java @@ -36,6 +36,7 @@ import static org.hisp.dhis.analytics.AnalyticsAggregationType.fromAggregationType; import static org.hisp.dhis.analytics.DataType.NUMERIC; import static org.hisp.dhis.common.QueryOperator.EQ; +import static org.hisp.dhis.common.QueryOperator.IN; import static org.hisp.dhis.common.QueryOperator.NE; import static org.hisp.dhis.common.QueryOperator.NEQ; import static org.hisp.dhis.common.QueryOperator.NIEQ; @@ -852,6 +853,23 @@ void testGetSelectClauseForAggregatedEnrollments() { assertEquals("select enrollment,Yearly ", select); } + @Test + void testItemsInFilterAreQuotedForOrganisationUnit() { + // Given + QueryItem queryItem = mock(QueryItem.class); + QueryFilter queryFilter = new QueryFilter(IN, "A;B;C"); + EventQueryParams params = + new EventQueryParams.Builder().withStartDate(new Date()).withEndDate(new Date()).build(); + when(queryItem.getItemName()).thenReturn("anyItem"); + when(queryItem.getValueType()).thenReturn(ValueType.ORGANISATION_UNIT); + + // When + String sql = eventSubject.toSql(queryItem, queryFilter, params).trim(); + + // Then + assertEquals("ax.\"anyItem\" in ('A','B','C')", sql); + } + @Test void testGetSelectClauseForQueryEnrollments() { // Given From 76f0fe08bd4623ac4af996c9bf186ba72fdcecbe Mon Sep 17 00:00:00 2001 From: Giuseppe Nespolino Date: Mon, 20 Jan 2025 13:41:24 +0100 Subject: [PATCH 4/5] test: added e2e test [DHIS2-18845] Signed-off-by: Giuseppe Nespolino --- .../analytics/event/query/EventQueryTest.java | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java index 6ad7d7c4b24b..11001828eae1 100644 --- a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java +++ b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java @@ -997,4 +997,68 @@ public void queryMetadataInfoForOptionSetAndOptionsWhenTenRows() throws JSONExce // Assert rows. validateRow(response, 0, List.of("Ngelehun CHC", "1")); } + + @Test + public void queryWithOrgUnitDataElement() throws JSONException { + // Given + String dimensionItems = + String.join( + ";", + "DiszpKrYNg8", + "g8upMTyEZGZ", + "LEVEL-H1KlN4QIauv", + "OU_GROUP-nlX2VoouN63", + "USER_ORGUNIT;USER_ORGUNIT_CHILDREN", + "USER_ORGUNIT_GRANDCHILDREN"); + + String dimensionOrgUnitDataElement = "Ge7Eo3FNnbl.rypjN8CV02V:IN:" + dimensionItems; + String dimensionOrgUnit = "ou:USER_ORGUNIT"; + + String dimension = dimensionOrgUnitDataElement + "," + dimensionOrgUnit; + + QueryParamsBuilder params = + new QueryParamsBuilder() + .add("dimension=" + dimension) + .add("headers=ouname,Ge7Eo3FNnbl.rypjN8CV02V") + .add("totalPages=false") + .add("displayProperty=NAME") + .add("pageSize=100") + .add("page=1") + .add("includeMetadataDetails=true") + .add("outputType=EVENT"); + + // When + ApiResponse response = analyticsEventActions.query().get("MoUd5BTQ3lY", params); + + // Then + response + .validate() + .statusCode(200) + .body("headers", hasSize(equalTo(2))) + .body("rows", hasSize(equalTo(0))) + .body("height", equalTo(0)) + .body("width", equalTo(0)) + .body("headerWidth", equalTo(2)); + + // Assert metaData. + String expectedMetaData = + "{\"pager\":{\"isLastPage\":true,\"pageSize\":100,\"page\":1},\"items\":{\"ImspTQPwCqd\":{\"uid\":\"ImspTQPwCqd\",\"code\":\"OU_525\",\"valueType\":\"TEXT\",\"name\":\"Sierra Leone\",\"dimensionItemType\":\"ORGANISATION_UNIT\",\"totalAggregationType\":\"SUM\"},\"USER_ORGUNIT\":{\"organisationUnits\":[\"ImspTQPwCqd\"]},\"ou\":{\"uid\":\"ou\",\"dimensionType\":\"ORGANISATION_UNIT\",\"name\":\"Organisation unit\"},\"Ge7Eo3FNnbl\":{\"uid\":\"Ge7Eo3FNnbl\",\"name\":\"XX MAL RDT - Case Registration\"},\"Ge7Eo3FNnbl.rypjN8CV02V\":{\"uid\":\"rypjN8CV02V\",\"aggregationType\":\"SUM\",\"valueType\":\"TEXT\",\"name\":\"XX MAL RDT TRK - Village of Residence\",\"style\":{\"icon\":\"nullapi\\/icons\\/star_medium_positive\\/icon.svg\"},\"dimensionItemType\":\"DATA_ELEMENT\",\"totalAggregationType\":\"SUM\"},\"MoUd5BTQ3lY\":{\"uid\":\"MoUd5BTQ3lY\",\"name\":\"XX MAL RDT - Case Registration\"},\"USER_ORGUNIT_CHILDREN\":{\"organisationUnits\":[\"at6UHUQatSo\",\"TEQlaapDQoK\",\"PMa2VCrupOd\",\"qhqAxPSTUXp\",\"kJq2mPyFEHo\",\"jmIPBj66vD6\",\"Vth0fbpFcsO\",\"jUb8gELQApl\",\"fdc6uOvgoji\",\"eIQbndfxQMb\",\"O6uvpzGd5pu\",\"lc3eMKXaEfw\",\"bL4ooGhyHRQ\"]},\"rypjN8CV02V\":{\"uid\":\"rypjN8CV02V\",\"aggregationType\":\"SUM\",\"valueType\":\"TEXT\",\"name\":\"XX MAL RDT TRK - Village of Residence\",\"style\":{\"icon\":\"nullapi\\/icons\\/star_medium_positive\\/icon.svg\"},\"dimensionItemType\":\"DATA_ELEMENT\",\"totalAggregationType\":\"SUM\"},\"LAST_12_MONTHS\":{\"name\":\"Last 12 months\"}},\"dimensions\":{\"pe\":[],\"ou\":[\"ImspTQPwCqd\"],\"Ge7Eo3FNnbl.rypjN8CV02V\":[]}}"; + String actualMetaData = new JSONObject((Map) response.extract("metaData")).toString(); + assertEquals(expectedMetaData, actualMetaData, false); + + // Assert headers. + validateHeader( + response, 0, "ouname", "Organisation unit name", "TEXT", "java.lang.String", false, true); + validateHeader( + response, + 1, + "Ge7Eo3FNnbl.rypjN8CV02V", + "XX MAL RDT TRK - Village of Residence", + "ORGANISATION_UNIT", + "org.hisp.dhis.organisationunit.OrganisationUnit", + false, + true); + + // no rows to assert + } } From 4a9775aa596fed01f6be808a06f759182df72ae2 Mon Sep 17 00:00:00 2001 From: Giuseppe Nespolino Date: Mon, 20 Jan 2025 13:51:39 +0100 Subject: [PATCH 5/5] test: added e2e test [DHIS2-18845] Signed-off-by: Giuseppe Nespolino --- .../org/hisp/dhis/analytics/event/query/EventQueryTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java index 11001828eae1..cc12bb525f98 100644 --- a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java +++ b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/event/query/EventQueryTest.java @@ -1008,7 +1008,8 @@ public void queryWithOrgUnitDataElement() throws JSONException { "g8upMTyEZGZ", "LEVEL-H1KlN4QIauv", "OU_GROUP-nlX2VoouN63", - "USER_ORGUNIT;USER_ORGUNIT_CHILDREN", + "USER_ORGUNIT", + "USER_ORGUNIT_CHILDREN", "USER_ORGUNIT_GRANDCHILDREN"); String dimensionOrgUnitDataElement = "Ge7Eo3FNnbl.rypjN8CV02V:IN:" + dimensionItems;