refactor: Simplify analytics tables SQL logic [DHIS2-18417]

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

@@ -49,7 +49,6 @@
 import org.hisp.dhis.common.IdentifiableObject;
 import org.hisp.dhis.common.IdentifiableObjectManager;
 import org.hisp.dhis.common.ValueType;
-import org.hisp.dhis.commons.util.TextUtils;
 import org.hisp.dhis.dataapproval.DataApprovalLevelService;
 import org.hisp.dhis.db.model.DataType;
 import org.hisp.dhis.db.model.IndexType;
@@ -171,19 +170,13 @@ protected void populateTableInternal(AnalyticsTablePartition partition, String f
 
     String sql = "insert into " + tableName + " (";
 
-    for (AnalyticsTableColumn col : columns) {
-      sql += quote(col.getName()) + ",";
-    }
-
-    sql = TextUtils.removeLastComma(sql) + ") select ";
+    sql += toCommaSeparated(columns, col -> quote(col.getName()));
 
-    for (AnalyticsTableColumn col : columns) {
-      sql += col.getSelectExpression() + ",";
-    }
+    sql += ") select ";
 
-    sql = TextUtils.removeLastComma(sql) + " ";
+    sql += toCommaSeparated(columns, AnalyticsTableColumn::getSelectExpression);
 
-    sql += fromClause;
+    sql += " " + fromClause;
 
     invokeTimeAndLog(sql, "Populating table: '{}'", tableName);
   }

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

@@ -42,6 +42,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
@@ -672,6 +673,19 @@ protected boolean tableIsNotEmpty(String name) {
     return jdbcTemplate.queryForRowSet(sql).next();
   }
 
+  /**
+   * Converts the given list of items to a comma-separated string, using the given mapping function
+   * to map the object to string.
+   *
+   * @param <T> the type.
+   * @param list the list.
+   * @param mapper the mapping function.
+   * @return a comma-separated string.
+   */
+  protected <T> String toCommaSeparated(List<T> list, Function<T, String> mapper) {
+    return list.stream().map(mapper).collect(Collectors.joining(","));
+  }
+
   /**
    * Quotes the given relation.
    *

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

@@ -75,6 +75,7 @@
 import org.hisp.dhis.analytics.AnalyticsAggregationType;
 import org.hisp.dhis.analytics.EventOutputType;
 import org.hisp.dhis.analytics.analyze.ExecutionPlanStore;
+import org.hisp.dhis.analytics.common.ProgramIndicatorSubqueryBuilder;
 import org.hisp.dhis.analytics.event.EventQueryParams;
 import org.hisp.dhis.analytics.event.EventQueryParams.Builder;
 import org.hisp.dhis.analytics.event.data.programindicator.DefaultProgramIndicatorSubqueryBuilder;
@@ -107,7 +108,9 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
 import org.mockito.Mock;
+import org.mockito.Spy;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.jdbc.support.rowset.ResultSetWrappingSqlRowSet;
@@ -126,11 +129,23 @@ class AbstractJdbcEventAnalyticsManagerTest extends EventAnalyticsTest {
 
   @Mock private OrganisationUnitService organisationUnitService;
 
-  private final SqlBuilder sqlBuilder = new PostgreSqlBuilder();
+  @Spy
+  private ProgramIndicatorSubqueryBuilder programIndicatorSubqueryBuilder =
+      new DefaultProgramIndicatorSubqueryBuilder(programIndicatorService);
 
-  private JdbcEventAnalyticsManager eventSubject;
+  @Spy private SqlBuilder sqlBuilder = new PostgreSqlBuilder();
 
-  private JdbcEnrollmentAnalyticsManager enrollmentSubject;
+  @Spy
+  private EventTimeFieldSqlRenderer eventTimeFieldSqlRenderer =
+      new EventTimeFieldSqlRenderer(sqlBuilder);
+
+  @Spy
+  private EnrollmentTimeFieldSqlRenderer enrollmentTimeFieldSqlRenderer =
+      new EnrollmentTimeFieldSqlRenderer(sqlBuilder);
+
+  @InjectMocks private JdbcEventAnalyticsManager eventSubject;
+
+  @InjectMocks private JdbcEnrollmentAnalyticsManager enrollmentSubject;
 
   private Program programA;
 
@@ -146,29 +161,7 @@ class AbstractJdbcEventAnalyticsManagerTest extends EventAnalyticsTest {
 
   @BeforeEach
   public void setUp() {
-    DefaultProgramIndicatorSubqueryBuilder programIndicatorSubqueryBuilder =
-        new DefaultProgramIndicatorSubqueryBuilder(programIndicatorService);
-
-    eventSubject =
-        new JdbcEventAnalyticsManager(
-            jdbcTemplate,
-            programIndicatorService,
-            programIndicatorSubqueryBuilder,
-            new EventTimeFieldSqlRenderer(sqlBuilder),
-            executionPlanStore,
-            sqlBuilder);
-
-    enrollmentSubject =
-        new JdbcEnrollmentAnalyticsManager(
-            jdbcTemplate,
-            programIndicatorService,
-            programIndicatorSubqueryBuilder,
-            new EnrollmentTimeFieldSqlRenderer(sqlBuilder),
-            executionPlanStore,
-            sqlBuilder);
-
     programA = createProgram('A');
-
     dataElementA = createDataElement('A', ValueType.INTEGER, AggregationType.SUM);
     dataElementA.setUid("fWIAEtYVEGk");
   }
@@ -241,8 +234,6 @@ void verifyGetCoordinateColumn() {
                 + colName
                 + ")::numeric, 6) || ']' as "
                 + colName));
-
-    return;
   }
 
   @Test

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

@@ -27,8 +27,6 @@
  */
 package org.hisp.dhis.analytics.event.data;
 
-import static org.hisp.dhis.analytics.common.AnalyticsDimensionsTestSupport.allValueTypeDataElements;
-import static org.hisp.dhis.analytics.common.AnalyticsDimensionsTestSupport.allValueTypeTEAs;
 import static org.hisp.dhis.analytics.common.DimensionServiceCommonTest.aggregateAllowedValueTypesPredicate;
 import static org.hisp.dhis.analytics.common.DimensionServiceCommonTest.queryDisallowedValueTypesPredicate;
 import static org.hisp.dhis.test.TestBase.injectSecurityContextNoSettings;
@@ -37,38 +35,35 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-import java.util.Collections;
 import java.util.List;
-import org.hisp.dhis.analytics.event.EnrollmentAnalyticsDimensionsService;
 import org.hisp.dhis.common.BaseIdentifiableObject;
 import org.hisp.dhis.common.PrefixedDimension;
 import org.hisp.dhis.dataelement.DataElement;
 import org.hisp.dhis.program.Program;
 import org.hisp.dhis.program.ProgramService;
-import org.hisp.dhis.security.acl.AclService;
 import org.hisp.dhis.trackedentity.TrackedEntityAttribute;
 import org.hisp.dhis.user.SystemUser;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 
+@ExtendWith(MockitoExtension.class)
 class EnrollmentAnalyticsDimensionsServiceTest {
-  private EnrollmentAnalyticsDimensionsService enrollmentAnalyticsDimensionsService;
+  @Mock private ProgramService programService;
+
+  @InjectMocks
+  private DefaultEnrollmentAnalyticsDimensionsService enrollmentAnalyticsDimensionsService;
 
   @BeforeEach
   void setup() {
     injectSecurityContextNoSettings(new SystemUser());
 
-    ProgramService programService = mock(ProgramService.class);
-
     Program program = mock(Program.class);
 
     when(programService.getProgram(any())).thenReturn(program);
-    when(program.getDataElements()).thenReturn(allValueTypeDataElements());
-    when(program.getProgramIndicators()).thenReturn(Collections.emptySet());
-    when(program.getTrackedEntityAttributes()).thenReturn(allValueTypeTEAs());
-
-    enrollmentAnalyticsDimensionsService =
-        new DefaultEnrollmentAnalyticsDimensionsService(programService, mock(AclService.class));
   }
 
   @Test

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

@@ -85,6 +85,7 @@
 import org.mockito.ArgumentCaptor;
 import org.mockito.Captor;
 import org.mockito.Mock;
+import org.mockito.Spy;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.mockito.junit.jupiter.MockitoSettings;
 import org.mockito.quality.Strictness;
@@ -108,7 +109,11 @@ class EnrollmentAnalyticsManagerTest extends EventAnalyticsTest {
 
   @Mock private ProgramIndicatorService programIndicatorService;
 
-  private final SqlBuilder sqlBuilder = new PostgreSqlBuilder();
+  @Spy private SqlBuilder sqlBuilder = new PostgreSqlBuilder();
+
+  @Spy
+  private EnrollmentTimeFieldSqlRenderer enrollmentTimeFieldSqlRenderer =
+      new EnrollmentTimeFieldSqlRenderer(sqlBuilder);
 
   @Captor private ArgumentCaptor<String> sql;
 
@@ -133,7 +138,7 @@ public void setUp() {
             jdbcTemplate,
             programIndicatorService,
             programIndicatorSubqueryBuilder,
-            new EnrollmentTimeFieldSqlRenderer(sqlBuilder),
+            enrollmentTimeFieldSqlRenderer,
             executionPlanStore,
             sqlBuilder);
   }

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

@@ -35,7 +35,7 @@
 import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CHILDREN;
 import static org.hisp.dhis.common.OrganisationUnitSelectionMode.SELECTED;
 import static org.hisp.dhis.period.RelativePeriodEnum.LAST_3_DAYS;
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import java.util.List;
 import java.util.Set;
@@ -52,7 +52,6 @@ class EnrollmentQueryHelperTest {
 
   @Test
   void testGetHeaderColumnsSamePrefixedDimension() {
-    // Given
     List<GridHeader> headers =
         List.of(
             new GridHeader("name0"),
@@ -64,10 +63,8 @@ void testGetHeaderColumnsSamePrefixedDimension() {
 
     String sql = "select name0, name1, dim.name2, pe, value, ou from table";
 
-    // When
     Set<String> headerColumns = EnrollmentQueryHelper.getHeaderColumns(headers, sql);
 
-    // Then
     String[] columns = headerColumns.toArray(String[]::new);
     assertEquals(3, columns.length);
     assertEquals("t1.\"name0\"", columns[0]);
@@ -77,7 +74,6 @@ void testGetHeaderColumnsSamePrefixedDimension() {
 
   @Test
   void testGetHeaderColumnsDifferentPrefixedDimension() {
-    // Given
     List<GridHeader> headers =
         List.of(
             new GridHeader("name0"),
@@ -89,10 +85,8 @@ void testGetHeaderColumnsDifferentPrefixedDimension() {
 
     String sql = "select name0, name1, ax.name2, pe, value, ou from table";
 
-    // When
     Set<String> headerColumns = EnrollmentQueryHelper.getHeaderColumns(headers, sql);
 
-    // Then
     String[] columns = headerColumns.toArray(String[]::new);
     assertEquals(3, columns.length);
     assertEquals("t1.\"name0\"", columns[0]);
@@ -102,7 +96,6 @@ void testGetHeaderColumnsDifferentPrefixedDimension() {
 
   @Test
   void testGetOrgUnitLevelColumnsOuMode() {
-    // Given
     OrganisationUnit organisationUnit = new OrganisationUnit("OrgTest");
     organisationUnit.setPath("/Level1/OrgTest");
 
@@ -114,18 +107,15 @@ void testGetOrgUnitLevelColumnsOuMode() {
                     ORGUNIT_DIM_ID, ORGANISATION_UNIT, List.of(organisationUnit)))
             .build();
 
-    // When
     Set<String> orgUnitColumns = EnrollmentQueryHelper.getOrgUnitLevelColumns(params);
 
-    // Then
     String[] columns = orgUnitColumns.toArray(String[]::new);
     assertEquals(1, columns.length);
     assertEquals("uidlevel2", columns[0]);
   }
 
   @Test
   void testGetOrgUnitLevelColumnsOuModeSelected() {
-    // Given
     OrganisationUnit organisationUnit = new OrganisationUnit("/Level1/Level2");
 
     EventQueryParams params =
@@ -136,16 +126,13 @@ void testGetOrgUnitLevelColumnsOuModeSelected() {
                     ORGUNIT_DIM_ID, ORGANISATION_UNIT, List.of(organisationUnit)))
             .build();
 
-    // When
     Set<String> orgUnitColumns = EnrollmentQueryHelper.getOrgUnitLevelColumns(params);
 
-    // Then
     assertEquals(0, orgUnitColumns.size());
   }
 
   @Test
   void testGetOrgUnitLevelColumnsOuModeChildren() {
-    // Given
     OrganisationUnit organisationUnit = new OrganisationUnit("/Level1/Level2");
 
     EventQueryParams params =
@@ -156,16 +143,13 @@ void testGetOrgUnitLevelColumnsOuModeChildren() {
                     ORGUNIT_DIM_ID, ORGANISATION_UNIT, List.of(organisationUnit)))
             .build();
 
-    // When
     Set<String> orgUnitColumns = EnrollmentQueryHelper.getOrgUnitLevelColumns(params);
 
-    // Then
     assertEquals(0, orgUnitColumns.size());
   }
 
   @Test
   void testGetPeriodColumns() {
-    // Given
     Period period = new Period(LAST_3_DAYS);
     period.setPeriodType(PeriodType.getPeriodTypeFromIsoString("201101"));
 
@@ -174,27 +158,22 @@ void testGetPeriodColumns() {
             .addDimension(new BaseDimensionalObject(PERIOD_DIM_ID, PERIOD, List.of(period)))
             .build();
 
-    // When
     Set<String> periodColumns = EnrollmentQueryHelper.getPeriodColumns(params);
 
-    // Then
     String[] columns = periodColumns.toArray(String[]::new);
     assertEquals(1, columns.length);
     assertEquals("t1.Monthly", columns[0]);
   }
 
   @Test
   void testGetPeriodColumnsNoPeriods() {
-    // Given
     EventQueryParams params =
         new EventQueryParams.Builder()
             .addDimension(new BaseDimensionalObject(ORGUNIT_DIM_ID, ORGANISATION_UNIT, List.of()))
             .build();
 
-    // When
     Set<String> periodColumns = EnrollmentQueryHelper.getPeriodColumns(params);
 
-    // Then
     assertEquals(0, periodColumns.size());
   }
 }