Skip to content

Commit 295d617

Browse files
larshelgelarshelge
authored
refactor: Simplify analytics tables SQL logic [DHIS2-18417] (#19386)
1 parent befb55b commit 295d617

File tree

7 files changed

+71
-76
lines changed

7 files changed

+71
-76
lines changed

‎dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
import org.hisp.dhis.common.IdentifiableObject;
5050
import org.hisp.dhis.common.IdentifiableObjectManager;
5151
import org.hisp.dhis.common.ValueType;
52-
import org.hisp.dhis.commons.util.TextUtils;
5352
import org.hisp.dhis.dataapproval.DataApprovalLevelService;
5453
import org.hisp.dhis.db.model.DataType;
5554
import org.hisp.dhis.db.model.IndexType;
@@ -171,19 +170,13 @@ protected void populateTableInternal(AnalyticsTablePartition partition, String f
171170

172171
String sql = "insert into " + tableName + " (";
173172

174-
for (AnalyticsTableColumn col : columns) {
175-
sql += quote(col.getName()) + ",";
176-
}
177-
178-
sql = TextUtils.removeLastComma(sql) + ") select ";
173+
sql += toCommaSeparated(columns, col -> quote(col.getName()));
179174

180-
for (AnalyticsTableColumn col : columns) {
181-
sql += col.getSelectExpression() + ",";
182-
}
175+
sql += ") select ";
183176

184-
sql = TextUtils.removeLastComma(sql) + " ";
177+
sql += toCommaSeparated(columns, AnalyticsTableColumn::getSelectExpression);
185178

186-
sql += fromClause;
179+
sql += " " + fromClause;
187180

188181
invokeTimeAndLog(sql, "Populating table: '{}'", tableName);
189182
}

‎dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractJdbcTableManager.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import java.util.List;
4343
import java.util.Map;
4444
import java.util.Set;
45+
import java.util.function.Function;
4546
import java.util.stream.Collectors;
4647
import lombok.RequiredArgsConstructor;
4748
import lombok.extern.slf4j.Slf4j;
@@ -672,6 +673,19 @@ protected boolean tableIsNotEmpty(String name) {
672673
return jdbcTemplate.queryForRowSet(sql).next();
673674
}
674675

676+
/**
677+
* Converts the given list of items to a comma-separated string, using the given mapping function
678+
* to map the object to string.
679+
*
680+
* @param <T> the type.
681+
* @param list the list.
682+
* @param mapper the mapping function.
683+
* @return a comma-separated string.
684+
*/
685+
protected <T> String toCommaSeparated(List<T> list, Function<T, String> mapper) {
686+
return list.stream().map(mapper).collect(Collectors.joining(","));
687+
}
688+
675689
/**
676690
* Quotes the given relation.
677691
*

‎dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java‎

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@
7575
import org.hisp.dhis.analytics.AnalyticsAggregationType;
7676
import org.hisp.dhis.analytics.EventOutputType;
7777
import org.hisp.dhis.analytics.analyze.ExecutionPlanStore;
78+
import org.hisp.dhis.analytics.common.ProgramIndicatorSubqueryBuilder;
7879
import org.hisp.dhis.analytics.event.EventQueryParams;
7980
import org.hisp.dhis.analytics.event.EventQueryParams.Builder;
8081
import org.hisp.dhis.analytics.event.data.programindicator.DefaultProgramIndicatorSubqueryBuilder;
@@ -107,7 +108,9 @@
107108
import org.junit.jupiter.api.BeforeEach;
108109
import org.junit.jupiter.api.Test;
109110
import org.junit.jupiter.api.extension.ExtendWith;
111+
import org.mockito.InjectMocks;
110112
import org.mockito.Mock;
113+
import org.mockito.Spy;
111114
import org.mockito.junit.jupiter.MockitoExtension;
112115
import org.springframework.jdbc.core.JdbcTemplate;
113116
import org.springframework.jdbc.support.rowset.ResultSetWrappingSqlRowSet;
@@ -126,11 +129,23 @@ class AbstractJdbcEventAnalyticsManagerTest extends EventAnalyticsTest {
126129

127130
@Mock private OrganisationUnitService organisationUnitService;
128131

129-
private final SqlBuilder sqlBuilder = new PostgreSqlBuilder();
132+
@Spy
133+
private ProgramIndicatorSubqueryBuilder programIndicatorSubqueryBuilder =
134+
new DefaultProgramIndicatorSubqueryBuilder(programIndicatorService);
130135

131-
private JdbcEventAnalyticsManager eventSubject;
136+
@Spy private SqlBuilder sqlBuilder = new PostgreSqlBuilder();
132137

133-
private JdbcEnrollmentAnalyticsManager enrollmentSubject;
138+
@Spy
139+
private EventTimeFieldSqlRenderer eventTimeFieldSqlRenderer =
140+
new EventTimeFieldSqlRenderer(sqlBuilder);
141+
142+
@Spy
143+
private EnrollmentTimeFieldSqlRenderer enrollmentTimeFieldSqlRenderer =
144+
new EnrollmentTimeFieldSqlRenderer(sqlBuilder);
145+
146+
@InjectMocks private JdbcEventAnalyticsManager eventSubject;
147+
148+
@InjectMocks private JdbcEnrollmentAnalyticsManager enrollmentSubject;
134149

135150
private Program programA;
136151

@@ -146,29 +161,7 @@ class AbstractJdbcEventAnalyticsManagerTest extends EventAnalyticsTest {
146161

147162
@BeforeEach
148163
public void setUp() {
149-
DefaultProgramIndicatorSubqueryBuilder programIndicatorSubqueryBuilder =
150-
new DefaultProgramIndicatorSubqueryBuilder(programIndicatorService);
151-
152-
eventSubject =
153-
new JdbcEventAnalyticsManager(
154-
jdbcTemplate,
155-
programIndicatorService,
156-
programIndicatorSubqueryBuilder,
157-
new EventTimeFieldSqlRenderer(sqlBuilder),
158-
executionPlanStore,
159-
sqlBuilder);
160-
161-
enrollmentSubject =
162-
new JdbcEnrollmentAnalyticsManager(
163-
jdbcTemplate,
164-
programIndicatorService,
165-
programIndicatorSubqueryBuilder,
166-
new EnrollmentTimeFieldSqlRenderer(sqlBuilder),
167-
executionPlanStore,
168-
sqlBuilder);
169-
170164
programA = createProgram('A');
171-
172165
dataElementA = createDataElement('A', ValueType.INTEGER, AggregationType.SUM);
173166
dataElementA.setUid("fWIAEtYVEGk");
174167
}
@@ -241,8 +234,6 @@ void verifyGetCoordinateColumn() {
241234
+ colName
242235
+ ")::numeric, 6) || ']' as "
243236
+ colName));
244-
245-
return;
246237
}
247238

248239
@Test

‎dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/EnrollmentAnalyticsDimensionsServiceTest.java‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@
2727
*/
2828
package org.hisp.dhis.analytics.event.data;
2929

30-
import static org.hisp.dhis.analytics.common.AnalyticsDimensionsTestSupport.allValueTypeDataElements;
31-
import static org.hisp.dhis.analytics.common.AnalyticsDimensionsTestSupport.allValueTypeTEAs;
3230
import static org.hisp.dhis.analytics.common.DimensionServiceCommonTest.aggregateAllowedValueTypesPredicate;
3331
import static org.hisp.dhis.analytics.common.DimensionServiceCommonTest.queryDisallowedValueTypesPredicate;
3432
import static org.hisp.dhis.test.TestBase.injectSecurityContextNoSettings;
@@ -37,38 +35,35 @@
3735
import static org.mockito.Mockito.mock;
3836
import static org.mockito.Mockito.when;
3937

40-
import java.util.Collections;
4138
import java.util.List;
42-
import org.hisp.dhis.analytics.event.EnrollmentAnalyticsDimensionsService;
4339
import org.hisp.dhis.common.BaseIdentifiableObject;
4440
import org.hisp.dhis.common.PrefixedDimension;
4541
import org.hisp.dhis.dataelement.DataElement;
4642
import org.hisp.dhis.program.Program;
4743
import org.hisp.dhis.program.ProgramService;
48-
import org.hisp.dhis.security.acl.AclService;
4944
import org.hisp.dhis.trackedentity.TrackedEntityAttribute;
5045
import org.hisp.dhis.user.SystemUser;
5146
import org.junit.jupiter.api.BeforeEach;
5247
import org.junit.jupiter.api.Test;
48+
import org.junit.jupiter.api.extension.ExtendWith;
49+
import org.mockito.InjectMocks;
50+
import org.mockito.Mock;
51+
import org.mockito.junit.jupiter.MockitoExtension;
5352

53+
@ExtendWith(MockitoExtension.class)
5454
class EnrollmentAnalyticsDimensionsServiceTest {
55-
private EnrollmentAnalyticsDimensionsService enrollmentAnalyticsDimensionsService;
55+
@Mock private ProgramService programService;
56+
57+
@InjectMocks
58+
private DefaultEnrollmentAnalyticsDimensionsService enrollmentAnalyticsDimensionsService;
5659

5760
@BeforeEach
5861
void setup() {
5962
injectSecurityContextNoSettings(new SystemUser());
6063

61-
ProgramService programService = mock(ProgramService.class);
62-
6364
Program program = mock(Program.class);
6465

6566
when(programService.getProgram(any())).thenReturn(program);
66-
when(program.getDataElements()).thenReturn(allValueTypeDataElements());
67-
when(program.getProgramIndicators()).thenReturn(Collections.emptySet());
68-
when(program.getTrackedEntityAttributes()).thenReturn(allValueTypeTEAs());
69-
70-
enrollmentAnalyticsDimensionsService =
71-
new DefaultEnrollmentAnalyticsDimensionsService(programService, mock(AclService.class));
7267
}
7368

7469
@Test

‎dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/EnrollmentAnalyticsManagerTest.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
import org.mockito.ArgumentCaptor;
8686
import org.mockito.Captor;
8787
import org.mockito.Mock;
88+
import org.mockito.Spy;
8889
import org.mockito.junit.jupiter.MockitoExtension;
8990
import org.mockito.junit.jupiter.MockitoSettings;
9091
import org.mockito.quality.Strictness;
@@ -108,7 +109,11 @@ class EnrollmentAnalyticsManagerTest extends EventAnalyticsTest {
108109

109110
@Mock private ProgramIndicatorService programIndicatorService;
110111

111-
private final SqlBuilder sqlBuilder = new PostgreSqlBuilder();
112+
@Spy private SqlBuilder sqlBuilder = new PostgreSqlBuilder();
113+
114+
@Spy
115+
private EnrollmentTimeFieldSqlRenderer enrollmentTimeFieldSqlRenderer =
116+
new EnrollmentTimeFieldSqlRenderer(sqlBuilder);
112117

113118
@Captor private ArgumentCaptor<String> sql;
114119

@@ -133,7 +138,7 @@ public void setUp() {
133138
jdbcTemplate,
134139
programIndicatorService,
135140
programIndicatorSubqueryBuilder,
136-
new EnrollmentTimeFieldSqlRenderer(sqlBuilder),
141+
enrollmentTimeFieldSqlRenderer,
137142
executionPlanStore,
138143
sqlBuilder);
139144
}

‎dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/EnrollmentQueryHelperTest.java‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CHILDREN;
3636
import static org.hisp.dhis.common.OrganisationUnitSelectionMode.SELECTED;
3737
import static org.hisp.dhis.period.RelativePeriodEnum.LAST_3_DAYS;
38-
import static org.junit.jupiter.api.Assertions.*;
38+
import static org.junit.jupiter.api.Assertions.assertEquals;
3939

4040
import java.util.List;
4141
import java.util.Set;
@@ -52,7 +52,6 @@ class EnrollmentQueryHelperTest {
5252

5353
@Test
5454
void testGetHeaderColumnsSamePrefixedDimension() {
55-
// Given
5655
List<GridHeader> headers =
5756
List.of(
5857
new GridHeader("name0"),
@@ -64,10 +63,8 @@ void testGetHeaderColumnsSamePrefixedDimension() {
6463

6564
String sql = "select name0, name1, dim.name2, pe, value, ou from table";
6665

67-
// When
6866
Set<String> headerColumns = EnrollmentQueryHelper.getHeaderColumns(headers, sql);
6967

70-
// Then
7168
String[] columns = headerColumns.toArray(String[]::new);
7269
assertEquals(3, columns.length);
7370
assertEquals("t1.\"name0\"", columns[0]);
@@ -77,7 +74,6 @@ void testGetHeaderColumnsSamePrefixedDimension() {
7774

7875
@Test
7976
void testGetHeaderColumnsDifferentPrefixedDimension() {
80-
// Given
8177
List<GridHeader> headers =
8278
List.of(
8379
new GridHeader("name0"),
@@ -89,10 +85,8 @@ void testGetHeaderColumnsDifferentPrefixedDimension() {
8985

9086
String sql = "select name0, name1, ax.name2, pe, value, ou from table";
9187

92-
// When
9388
Set<String> headerColumns = EnrollmentQueryHelper.getHeaderColumns(headers, sql);
9489

95-
// Then
9690
String[] columns = headerColumns.toArray(String[]::new);
9791
assertEquals(3, columns.length);
9892
assertEquals("t1.\"name0\"", columns[0]);
@@ -102,7 +96,6 @@ void testGetHeaderColumnsDifferentPrefixedDimension() {
10296

10397
@Test
10498
void testGetOrgUnitLevelColumnsOuMode() {
105-
// Given
10699
OrganisationUnit organisationUnit = new OrganisationUnit("OrgTest");
107100
organisationUnit.setPath("/Level1/OrgTest");
108101

@@ -114,18 +107,15 @@ void testGetOrgUnitLevelColumnsOuMode() {
114107
ORGUNIT_DIM_ID, ORGANISATION_UNIT, List.of(organisationUnit)))
115108
.build();
116109

117-
// When
118110
Set<String> orgUnitColumns = EnrollmentQueryHelper.getOrgUnitLevelColumns(params);
119111

120-
// Then
121112
String[] columns = orgUnitColumns.toArray(String[]::new);
122113
assertEquals(1, columns.length);
123114
assertEquals("uidlevel2", columns[0]);
124115
}
125116

126117
@Test
127118
void testGetOrgUnitLevelColumnsOuModeSelected() {
128-
// Given
129119
OrganisationUnit organisationUnit = new OrganisationUnit("/Level1/Level2");
130120

131121
EventQueryParams params =
@@ -136,16 +126,13 @@ void testGetOrgUnitLevelColumnsOuModeSelected() {
136126
ORGUNIT_DIM_ID, ORGANISATION_UNIT, List.of(organisationUnit)))
137127
.build();
138128

139-
// When
140129
Set<String> orgUnitColumns = EnrollmentQueryHelper.getOrgUnitLevelColumns(params);
141130

142-
// Then
143131
assertEquals(0, orgUnitColumns.size());
144132
}
145133

146134
@Test
147135
void testGetOrgUnitLevelColumnsOuModeChildren() {
148-
// Given
149136
OrganisationUnit organisationUnit = new OrganisationUnit("/Level1/Level2");
150137

151138
EventQueryParams params =
@@ -156,16 +143,13 @@ void testGetOrgUnitLevelColumnsOuModeChildren() {
156143
ORGUNIT_DIM_ID, ORGANISATION_UNIT, List.of(organisationUnit)))
157144
.build();
158145

159-
// When
160146
Set<String> orgUnitColumns = EnrollmentQueryHelper.getOrgUnitLevelColumns(params);
161147

162-
// Then
163148
assertEquals(0, orgUnitColumns.size());
164149
}
165150

166151
@Test
167152
void testGetPeriodColumns() {
168-
// Given
169153
Period period = new Period(LAST_3_DAYS);
170154
period.setPeriodType(PeriodType.getPeriodTypeFromIsoString("201101"));
171155

@@ -174,27 +158,22 @@ void testGetPeriodColumns() {
174158
.addDimension(new BaseDimensionalObject(PERIOD_DIM_ID, PERIOD, List.of(period)))
175159
.build();
176160

177-
// When
178161
Set<String> periodColumns = EnrollmentQueryHelper.getPeriodColumns(params);
179162

180-
// Then
181163
String[] columns = periodColumns.toArray(String[]::new);
182164
assertEquals(1, columns.length);
183165
assertEquals("t1.Monthly", columns[0]);
184166
}
185167

186168
@Test
187169
void testGetPeriodColumnsNoPeriods() {
188-
// Given
189170
EventQueryParams params =
190171
new EventQueryParams.Builder()
191172
.addDimension(new BaseDimensionalObject(ORGUNIT_DIM_ID, ORGANISATION_UNIT, List.of()))
192173
.build();
193174

194-
// When
195175
Set<String> periodColumns = EnrollmentQueryHelper.getPeriodColumns(params);
196176

197-
// Then
198177
assertEquals(0, periodColumns.size());
199178
}
200179
}

0 commit comments

Comments
 (0)