Skip to content

Commit c1abd2e

Browse files
luciano-fiandesioPhilip-Larsen-Donnelly
luciano-fiandesio
and
Philip-Larsen-Donnelly
authored
[2.43] fix: Stage.ou dimension metadata with LEVEL/OU_GROUP selectors [DHIS2-21595] (#24243)
* fix: Stage.ou dimension metadata with LEVEL/OU_GROUP selectors [DHIS2-21595] (#24110) * fix: Stage.ou dimension metadata with LEVEL/OU_GROUP selectors [DHIS2-21595] * test: verify commit signing * fix: org unit data element filters rejected with E7143 Filtering a data element by a mix of org units and levels/groups could fail with an error instead of returning data. Such filters now correctly match the listed org units together with the level/group members, while stage org unit dimensions keep their existing behaviour. * Fix e2e tests (headers) * Add GPG configuration to API analytics test workflow * fix: proper implementation of GPG signing * fix: remove need for KEY_ID * fix: align signing user * fix: use SSH instead of PGP keys * fix: temporarily disable reports push --------- Co-authored-by: Philip-Larsen-Donnelly <35666657+Philip-Larsen-Donnelly@users.noreply.github.com> Co-authored-by: Philip-Larsen-Donnelly <phil@dhis2.org> * fix formatting * fix IT test * Fix doris ci file * Fix 1 PG e2e test * fix failing e2e tests --------- Co-authored-by: Philip-Larsen-Donnelly <35666657+Philip-Larsen-Donnelly@users.noreply.github.com> Co-authored-by: Philip-Larsen-Donnelly <phil@dhis2.org>
1 parent c7f5417 commit c1abd2e

20 files changed

Lines changed: 1423 additions & 56 deletions

‎.github/workflows/run-api-analytics-tests-clickhouse.yml‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
name: Run analytics api tests [ClickHouse]
2+
env:
3+
# This is to make sure Maven don't timeout fetching dependencies. See: https://github.com/actions/virtual-environments/issues/1499
4+
MAVEN_OPTS: -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false -Dmaven.wagon.http.retryHandler.class=standard -Dmaven.wagon.http.retryHandler.count=3 -Dmaven.wagon.httpconnectionManager.ttlSeconds=125
5+
# PR → quiet (unless ci-verbose label), push/schedule → info, workflow_dispatch → user choice
6+
MAVEN_ARGS: ${{ (inputs.maven_log == 'debug' && '-X') || (github.event_name == 'pull_request' && !contains(github.event.pull_request.labels.*.name, 'ci-verbose') && '-q') || (inputs.maven_log == 'quiet' && '-q') || '' }}
7+
8+
on:
9+
pull_request:
10+
types: [opened, labeled, synchronize]
11+
schedule:
12+
- cron: "26 1 * * *" # Run at 01:36 AM
13+
workflow_dispatch:
14+
inputs:
15+
maven_log:
16+
description: "Maven log verbosity"
17+
required: false
18+
default: "quiet"
19+
type: choice
20+
options:
21+
- quiet
22+
- info
23+
- debug
24+
concurrency:
25+
group: ${{ github.workflow}}-${{ github.ref }}
26+
cancel-in-progress: true
27+
permissions:
28+
contents: read
29+
pull-requests: write
30+
31+
jobs:
32+
api-analytics-test-clickhouse:
33+
env:
34+
CORE_IMAGE_NAME: "dhis2/core-dev:local"
35+
PR_NUMBER: ${{ github.event.number }}
36+
DOCKER_CHANNEL: "dhis2/core-pr"
37+
SIERRA_LEONE_DB_PATH: "/tmp/db/sierra-leone"
38+
SIERRA_LEONE_DB_VERSION: "2.39.6"
39+
GITHUB_TOKEN: ${{ secrets.DHIS2_BOT_GITHUB_TOKEN }}
40+
SSH_SIGNING_KEY: ${{ secrets.DHIS2_BOT_SSH_SIGNING_KEY }}
41+
SSH_SIGNING_PASSPHRASE: ${{ secrets.DHIS2_BOT_SSH_SIGNING_PASSPHRASE }}
42+
43+
runs-on: ubuntu-22.04
44+
if: contains(github.event.pull_request.labels.*.name, 'run-api-analytics-tests-clickhouse') || (github.event_name == 'workflow_dispatch')
45+
steps:
46+
- uses: actions/checkout@v6
47+
48+
- name: Free disk space on runner (Ubuntu)
49+
shell: bash
50+
run: |
51+
# Remove Android and .NET libs (approx. 14GB)
52+
sudo rm -rf /usr/local/lib/android
53+
sudo rm -rf /usr/share/dotnet
54+
55+
- name: Set up JDK 17
56+
uses: actions/setup-java@v5
57+
with:
58+
java-version: 17
59+
distribution: temurin
60+
cache: maven
61+
62+
- name: Build container image
63+
run: |
64+
# only build image for running api tests in this workflow
65+
mvn clean package --threads 2C --batch-mode --no-transfer-progress \
66+
-DskipTests -Dmaven.test.skip=true --update-snapshots --file dhis-2/pom.xml \
67+
--projects dhis-web-server --also-make --activate-profiles jibDockerBuild \
68+
-Djib.to.image=$CORE_IMAGE_NAME
69+
70+
- name: Get Sierra Leone DB from cache
71+
uses: actions/cache@v5
72+
id: cache-sierra-leone-db
73+
with:
74+
path: ${{ env.SIERRA_LEONE_DB_PATH }}
75+
key: sierra-leone-db-analytics-${{ env.SIERRA_LEONE_DB_VERSION }}
76+
77+
- name: Download Sierra Leone DB
78+
if: ${{ steps.cache-sierra-leone-db.outputs.cache-hit != 'true' }}
79+
run: |
80+
mkdir -p ${{ env.SIERRA_LEONE_DB_PATH }}
81+
wget https://databases.dhis2.org/sierra-leone/${{ env.SIERRA_LEONE_DB_VERSION }}/analytics_be/dhis2-db-sierra-leone.sql.gz --no-clobber --directory-prefix ${{ env.SIERRA_LEONE_DB_PATH }}
82+
83+
- name: Run tests
84+
run: |
85+
cd dhis-2/dhis-test-e2e
86+
source setup_clickhouse_links
87+
DHIS2_IMAGE="$CORE_IMAGE_NAME" \
88+
DHIS2_E2E_TEST_DUMP_PATH="$SIERRA_LEONE_DB_PATH" \
89+
CLICKHOUSE_VERSION=24.8 \
90+
docker compose -f docker-compose.yml -f docker-compose.clickhouse.yml -f docker-compose.e2e.yml \
91+
-f docker-compose.e2e-analytics.yml -f docker-compose.clickhouse-analytics.yml \
92+
up --attach test --remove-orphans --exit-code-from test
93+
94+
# - name: Push surefire reports to repository
95+
# if: always()
96+
# run: |
97+
# cd dhis-2/dhis-test-e2e
98+
# DHIS2_IMAGE="$CORE_IMAGE_NAME" \
99+
# DHIS2_E2E_TEST_DUMP_PATH="$SIERRA_LEONE_DB_PATH" \
100+
# CLICKHOUSE_VERSION=24.8 \
101+
# TEST_TYPE=e2e-analytics \
102+
# DB_TYPE=clickhouse \
103+
# docker compose -f docker-compose.yml -f docker-compose.clickhouse.yml -f docker-compose.e2e.yml -f docker-compose.e2e-analytics.yml -f docker-compose.clickhouse-analytics.yml --profile reports run reports-processor
104+
105+
# - name: Add a comment to the pull request
106+
# if: always()
107+
# uses: peter-evans/create-or-update-comment@e8674b075228eee787fea43ef493e45ece1004c9
108+
# with:
109+
# issue-number: ${{ github.event.pull_request.number }}
110+
# body: |
111+
# Surefire reports for analytics API tests against ClickHouse have been pushed to the repository. [View reports](https://dhis2.github.io/dhis2-test-reports/reports/?component=core&testType=e2e-analytics)
112+
113+
- name: Upload logs
114+
if: failure()
115+
run: |
116+
cd dhis-2/dhis-test-e2e
117+
docker compose logs web > ~/logs.txt
118+
119+
- uses: actions/upload-artifact@v7
120+
if: failure()
121+
with:
122+
name: "tomcat_logs"
123+
path: "~/logs.txt"
124+
125+
send-slack-message:
126+
runs-on: ubuntu-22.04
127+
if: |
128+
always() &&
129+
contains(needs.*.result, 'failure') &&
130+
github.ref == 'refs/heads/master'
131+
132+
needs: [api-analytics-test-clickhouse]
133+
steps:
134+
- uses: rtCamp/action-slack-notify@v2
135+
env:
136+
SLACK_WEBHOOK: ${{ secrets.SLACK_BACKEND_WEBHOOK }}
137+
SLACK_CHANNEL: "team-backend"
138+
SLACK_MESSAGE: "Latest analytics e2e test run against ClickHouse on master failed and needs investigation :detective-duck:. \n Commit message: ${{ github.event.head_commit.message }}"
139+
SLACK_COLOR: "#ff0000"

‎.github/workflows/run-api-analytics-tests-doris.yml‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ jobs:
3333
SIERRA_LEONE_DB_PATH: "/tmp/db/sierra-leone"
3434
SIERRA_LEONE_DB_VERSION: "2.39.6"
3535
GITHUB_TOKEN: ${{ secrets.DHIS2_BOT_GITHUB_TOKEN }}
36+
SSH_SIGNING_KEY: ${{ secrets.DHIS2_BOT_SSH_SIGNING_KEY }}
37+
SSH_SIGNING_PASSPHRASE: ${{ secrets.DHIS2_BOT_SSH_SIGNING_PASSPHRASE }}
3638

3739
runs-on: ubuntu-22.04
3840
if: contains(github.event.pull_request.labels.*.name, 'run-api-analytics-tests-doris') || (github.event_name == 'workflow_dispatch')
@@ -85,26 +87,6 @@ jobs:
8587
-f docker-compose.e2e-analytics.yml -f docker-compose.doris-analytics.yml \
8688
up --remove-orphans --exit-code-from test
8789
88-
- name: Push surefire reports to repository
89-
if: always()
90-
run: |
91-
cd dhis-2/dhis-test-e2e
92-
DHIS2_IMAGE="$CORE_IMAGE_NAME" \
93-
DHIS2_E2E_TEST_DUMP_PATH="$SIERRA_LEONE_DB_PATH" \
94-
DORIS_VERSION=3.0.7 \
95-
TEST_TYPE=e2e-analytics \
96-
DB_TYPE=doris \
97-
docker compose -f docker-compose.yml -f docker-compose.doris.yml -f docker-compose.e2e.yml -f docker-compose.e2e-analytics.yml -f docker-compose.doris-analytics.yml --profile reports run reports-processor
98-
99-
- name: Add a comment to the pull request
100-
if: always()
101-
uses: peter-evans/create-or-update-comment@e8674b075228eee787fea43ef493e45ece1004c9
102-
with:
103-
issue-number: ${{ github.event.pull_request.number }}
104-
body: |
105-
Surefire reports for analytics API tests against Doris have been pushed to the repository. [View reports](https://dhis2.github.io/dhis2-test-reports/reports/?component=core&testType=e2e-analytics)
106-
107-
10890
- name: Upload logs
10991
if: failure()
11092
run: |
@@ -132,4 +114,3 @@ jobs:
132114
SLACK_CHANNEL: "team-backend"
133115
SLACK_MESSAGE: "Latest analytics e2e test run against Doris on master failed and needs investigation :detective-duck:. \n Commit message: ${{ github.event.head_commit.message }}"
134116
SLACK_COLOR: "#ff0000"
135-

‎dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DimensionalObjectProvider.java‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,18 +459,50 @@ public List<String> getOrgUnitDimensionUid(
459459
* its keywords.
460460
* @param userOrgUnits the list of organisation units associated with the current user.
461461
* @param expandGroupsAndLevels if true, expands LEVEL-X and OU_GROUP-X to their member org units.
462-
* This is needed for SQL filtering but should be false for metadata generation.
462+
* Explicit org units are kept in the result (union semantics). This is needed for SQL
463+
* filtering but should be false for metadata generation.
463464
* @return a list of {@link OrganisationUnit} UIDs.
464465
*/
465466
public List<String> getOrgUnitDimensionUid(
466467
List<String> items, List<OrganisationUnit> userOrgUnits, boolean expandGroupsAndLevels) {
468+
return getOrgUnitDimensionUid(items, userOrgUnits, expandGroupsAndLevels, false);
469+
}
470+
471+
/**
472+
* This method will return a list of {@link OrganisationUnit} UIDs based on the given items and
473+
* user organisation units.
474+
*
475+
* @param items the list of items that might be included into the resulting organisation unit and
476+
* its keywords.
477+
* @param userOrgUnits the list of organisation units associated with the current user.
478+
* @param expandGroupsAndLevels if true, expands LEVEL-X and OU_GROUP-X to their member org units.
479+
* This is needed for SQL filtering but should be false for metadata generation.
480+
* @param orgUnitsAsBoundaries if true and LEVEL-X / OU_GROUP-X selectors are present, explicit
481+
* org units only scope the expansion and are not included in the result (stage.ou dimension
482+
* semantics). If false, explicit org units are kept alongside the expanded members (union
483+
* semantics, used by org unit typed data element filters). Only relevant when
484+
* expandGroupsAndLevels is true.
485+
* @return a list of {@link OrganisationUnit} UIDs.
486+
*/
487+
public List<String> getOrgUnitDimensionUid(
488+
List<String> items,
489+
List<OrganisationUnit> userOrgUnits,
490+
boolean expandGroupsAndLevels,
491+
boolean orgUnitsAsBoundaries) {
467492
List<Integer> levels = new ArrayList<>();
468493
List<OrganisationUnitGroup> groups = new ArrayList<>();
469494

470495
List<DimensionalItemObject> ous =
471496
getOrgUnitDimensionItems(items, userOrgUnits, IdScheme.UID, levels, groups);
472497

473-
List<String> result = new ArrayList<>(ous.stream().map(DimensionalItemObject::getUid).toList());
498+
boolean hasLevelsOrGroups = !levels.isEmpty() || !groups.isEmpty();
499+
500+
List<String> result = new ArrayList<>();
501+
502+
// With boundary semantics, OUs combined with levels / groups only scope the expansion
503+
if (!(expandGroupsAndLevels && orgUnitsAsBoundaries && hasLevelsOrGroups)) {
504+
result.addAll(ous.stream().map(DimensionalItemObject::getUid).toList());
505+
}
474506

475507
if (expandGroupsAndLevels) {
476508
List<OrganisationUnit> ousList = asTypedList(ous);

‎dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManager.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1816,7 +1816,7 @@ public String toSql(QueryItem item, QueryFilter filter, EventQueryParams params)
18161816

18171817
String filterString =
18181818
item.getValueType() == ValueType.ORGANISATION_UNIT
1819-
? organisationUnitResolver.resolveOrgUnits(filter, params.getUserOrgUnits())
1819+
? organisationUnitResolver.resolveOrgUnits(filter, params.getUserOrgUnits(), item)
18201820
: filter.getFilter();
18211821

18221822
if (IN.equals(filter.getOperator())) {
@@ -3140,7 +3140,7 @@ private Condition buildInFilterCondition(
31403140
// Resolve special org unit keywords like USER_ORGUNIT if applicable
31413141
String resolvedFilter =
31423142
filterBuilder.requiresOrgUnitResolution(item)
3143-
? organisationUnitResolver.resolveOrgUnits(filter, params.getUserOrgUnits())
3143+
? organisationUnitResolver.resolveOrgUnits(filter, params.getUserOrgUnits(), item)
31443144
: filter.getFilter();
31453145

31463146
InQueryCteFilter inQueryCteFilter =
@@ -3191,7 +3191,7 @@ private String getSqlFilterValue(QueryFilter filter, QueryItem item, EventQueryP
31913191
// Resolve special org unit keywords like USER_ORGUNIT if applicable
31923192
String filterValue =
31933193
filterBuilder.requiresOrgUnitResolution(item)
3194-
? organisationUnitResolver.resolveOrgUnits(filter, params.getUserOrgUnits())
3194+
? organisationUnitResolver.resolveOrgUnits(filter, params.getUserOrgUnits(), item)
31953195
: filter.getFilter();
31963196

31973197
if ("NV".equals(filterValue)) {

‎dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/OrganisationUnitResolver.java‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,38 @@ public class OrganisationUnitResolver {
8989
/**
9090
* Resolve organisation units like ou:USER_ORGUNIT;USER_ORGUNIT_CHILDREN;LEVEL-XXX;OUGROUP-XXX
9191
* into a list of organisation unit dimension uids. Groups and levels are expanded to their member
92-
* org units for SQL filtering purposes.
92+
* org units for SQL filtering purposes. Explicit org units are kept in the result (union
93+
* semantics, used by org unit typed data element filters).
9394
*
9495
* @param queryFilter the query filter containing the organisation unit filter
9596
* @param userOrgUnits the user organisation units
9697
* @return the organisation unit dimension uids
9798
*/
9899
public String resolveOrgUnits(QueryFilter queryFilter, List<OrganisationUnit> userOrgUnits) {
100+
return resolveOrgUnits(queryFilter, userOrgUnits, false);
101+
}
102+
103+
/**
104+
* Resolve organisation units from the given query filter, applying boundary semantics when the
105+
* item is a stage.ou dimension (explicit org units only scope the LEVEL-/OU_GROUP- expansion) and
106+
* union semantics otherwise.
107+
*
108+
* @param queryFilter the query filter containing the organisation unit filter
109+
* @param userOrgUnits the user organisation units
110+
* @param item the query item the filter belongs to
111+
* @return the organisation unit dimension uids
112+
*/
113+
public String resolveOrgUnits(
114+
QueryFilter queryFilter, List<OrganisationUnit> userOrgUnits, QueryItem item) {
115+
return resolveOrgUnits(queryFilter, userOrgUnits, isStageOuDimension(item));
116+
}
117+
118+
private String resolveOrgUnits(
119+
QueryFilter queryFilter, List<OrganisationUnit> userOrgUnits, boolean orgUnitsAsBoundaries) {
99120
List<String> filterItem = QueryFilter.getFilterItems(queryFilter.getFilter());
100121
List<String> orgUnitDimensionUid =
101-
dimensionalObjectProducer.getOrgUnitDimensionUid(filterItem, userOrgUnits, true);
122+
dimensionalObjectProducer.getOrgUnitDimensionUid(
123+
filterItem, userOrgUnits, true, orgUnitsAsBoundaries);
102124

103125
// Throw E7143 if no valid org units were resolved (mirrors standard ou: dimension behavior)
104126
if (orgUnitDimensionUid.isEmpty()) {
@@ -120,7 +142,8 @@ public Map<String, MetadataItem> getMetadataItemsForOrgUnitDataElements(EventQue
120142
for (QueryItem queryItem : params.getItems()) {
121143
if (queryItem.getValueType().isOrganisationUnit()) {
122144
for (QueryFilter queryFilter : queryItem.getFilters()) {
123-
String resolveOrgUnits = resolveOrgUnits(queryFilter, params.getUserOrgUnits());
145+
String resolveOrgUnits =
146+
resolveOrgUnits(queryFilter, params.getUserOrgUnits(), queryItem);
124147
if (StringUtils.isNotBlank(resolveOrgUnits)) {
125148
orgUnitIds.addAll(Arrays.asList(resolveOrgUnits.split(OPTION_SEP)));
126149
}
@@ -162,7 +185,7 @@ private MetadataItem toMetadataItem(OrganisationUnit orgUnit, EventQueryParams p
162185
*/
163186
public List<String> resolveOrgUnits(EventQueryParams params, QueryItem item) {
164187
return item.getFilters().stream()
165-
.map(queryFilter -> resolveOrgUnits(queryFilter, params.getUserOrgUnits()))
188+
.map(queryFilter -> resolveOrgUnits(queryFilter, params.getUserOrgUnits(), item))
166189
.map(s -> s.split(DimensionConstants.OPTION_SEP))
167190
.flatMap(Arrays::stream)
168191
.distinct()
@@ -224,9 +247,12 @@ public Map<Integer, List<OrganisationUnit>> resolveOrgUnitsGroupedByLevel(
224247
public DimensionalItemObject loadOrgUnitDimensionalItem(
225248
@Nonnull String dimensionUid, @Nonnull IdScheme idScheme) {
226249
if (dimensionUid.startsWith(KEY_LEVEL)) {
250+
String levelToken = substringAfterLast(dimensionUid, SEPARATOR);
227251
OrganisationUnitLevel level =
228-
idObjectManager.getObject(
229-
OrganisationUnitLevel.class, idScheme, substringAfterLast(dimensionUid, SEPARATOR));
252+
StringUtils.isNumeric(levelToken)
253+
? organisationUnitService.getOrganisationUnitLevelByLevel(
254+
Integer.parseInt(levelToken))
255+
: idObjectManager.getObject(OrganisationUnitLevel.class, idScheme, levelToken);
230256

231257
if (level != null) {
232258
BaseDimensionalItemObject dim = new BaseDimensionalItemObject();

‎dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/QueryItemFilterBuilder.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public String extractFiltersAsSql(QueryItem item, String columnName, EventQueryP
101101
boolean needsResolution = requiresOrgUnitResolution(item);
102102
String resolvedFilter =
103103
needsResolution
104-
? organisationUnitResolver.resolveOrgUnits(f, params.getUserOrgUnits())
104+
? organisationUnitResolver.resolveOrgUnits(f, params.getUserOrgUnits(), item)
105105
: f.getFilter();
106106

107107
return buildFilterCondition(f, item, columnName, resolvedFilter);
@@ -228,7 +228,7 @@ public String extractNonNvFiltersAsSql(
228228
boolean needsResolution = requiresOrgUnitResolution(item);
229229
String resolvedFilter =
230230
needsResolution
231-
? organisationUnitResolver.resolveOrgUnits(f, params.getUserOrgUnits())
231+
? organisationUnitResolver.resolveOrgUnits(f, params.getUserOrgUnits(), item)
232232
: f.getFilter();
233233

234234
// For IN operator with mixed values, only include non-NV values

0 commit comments

Comments
 (0)