fix: Update code

dhis2 · Dec 2, 2024 · 41ca710 · 41ca710
1 parent 35a8daa
commit 41ca710
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 61 deletions.
diff --git a/...analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java b/...analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java
@@ -51,6 +51,7 @@
 import java.util.Map;
 import java.util.Objects;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.lang3.Validate;
 import org.hisp.dhis.analytics.AnalyticsTableHookService;
 import org.hisp.dhis.analytics.AnalyticsTableType;
 import org.hisp.dhis.analytics.AnalyticsTableUpdateParams;
@@ -492,7 +493,7 @@ private List<AnalyticsTableColumn> getColumnForDataElement(
         sqlBuilder.jsonExtractNested("eventdatavalues", dataElement.getUid(), "value");
     String selectExpression = getSelectExpression(dataElement.getValueType(), columnExpression);
     String dataFilterClause = getDataFilterClause(dataElement);
-    String sql = getSelectForInsert(dataElement, selectExpression, dataFilterClause);
+    String sql = String.format("%s as %s", selectExpression, quote(dataElement.getUid()));
     Skip skipIndex = skipIndex(dataElement.getValueType(), dataElement.hasOptionSet());
 
     if (withLegendSet) {
@@ -533,7 +534,7 @@ private List<AnalyticsTableColumn> getColumnForOrgUnitDataElement(
 
     if (isSpatialSupport()) {
       String fromType = "ou.geometry " + fromClause;
-      String geoSql = getSelectForInsert(dataElement, fromType, dataFilterClause);
+      String geoSql = getOrgUnitSelectExpression(dataElement, fromType, dataFilterClause);
 
       columns.add(
           AnalyticsTableColumn.builder()
@@ -546,7 +547,7 @@ private List<AnalyticsTableColumn> getColumnForOrgUnitDataElement(
     }
 
     String fromTypeSql = "ou.name " + fromClause;
-    String ouNameSql = getSelectForInsert(dataElement, fromTypeSql, dataFilterClause);
+    String ouNameSql = getOrgUnitSelectExpression(dataElement, fromTypeSql, dataFilterClause);
 
     columns.add(
         AnalyticsTableColumn.builder()
@@ -624,31 +625,24 @@ private List<AnalyticsTableColumn> getColumnForAttributeWithLegendSet(
   }
 
   /**
-   * Retyrns a select statement for the given select expression.
+   * Returns a select statement for the given data element with value type org unit.
    *
    * @param dataElement the data element to create the select statement for.
    * @param selectExpression the select expression.
    * @param dataFilterClause the data filter clause.
    * @return a select expression.
    */
-  private String getSelectForInsert(
+  private String getOrgUnitSelectExpression(
       DataElement dataElement, String selectExpression, String dataFilterClause) {
-    String sqlTemplate =
-        dataElement.getValueType().isOrganisationUnit()
-            ? "(select ${selectExpression} ${dataClause})${closingParentheses} as ${uid}"
-            : "${selectExpression}${closingParentheses} as ${uid}";
-
+    Validate.isTrue(dataElement.getValueType().isOrganisationUnit());
+    String prts = getClosingParentheses(selectExpression);
     return replaceQualify(
-        sqlTemplate,
+        "(select ${selectExpression} ${dataFilterClause})${closingParentheses} as ${uid}",
         Map.of(
-            "selectExpression",
-            selectExpression,
-            "dataClause",
-            dataFilterClause,
-            "closingParentheses",
-            getClosingParentheses(selectExpression),
-            "uid",
-            quote(dataElement.getUid())));
+            "selectExpression", selectExpression,
+            "dataFilterClause", dataFilterClause,
+            "closingParentheses", prts,
+            "uid", quote(dataElement.getUid())));
   }
 
   /**

diff --git a/...ytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java b/...ytics/src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerTest.java
@@ -316,10 +316,15 @@ void verifyClientSideTimestampsColumns() {
     assertThat(
         lastUpdated.getSelectExpression(),
         is(
-            "case when ev.lastupdatedatclient is not null then ev.lastupdatedatclient else ev.lastupdated end"));
+            """
+            case when ev.lastupdatedatclient is not null \
+            then ev.lastupdatedatclient else ev.lastupdated end"""));
     assertThat(
         created.getSelectExpression(),
-        is("case when ev.createdatclient is not null then ev.createdatclient else ev.created end"));
+        is(
+            """
+           case when ev.createdatclient is not null \
+           then ev.createdatclient else ev.created end"""));
   }
 
   @Test
@@ -389,13 +394,7 @@ void verifyGetTableWithDataElements() {
         case when eventdatavalues #>> '{deabcdefghW, value}' ~* '^\\d{4}-\\d{2}-\\d{2}(\\s|T)?((\\d{2}:)(\\d{2}:)?(\\d{2}))?(|.(\\d{3})|.(\\d{3})Z)?$' \
         then cast(eventdatavalues #>> '{deabcdefghW, value}' as timestamp) else null end as "deabcdefghW\"""";
     String aliasD5 =
-        "(select ou.uid from \"organisationunit\" ou where ou.uid = "
-            + "eventdatavalues #>> '{"
-            + d5.getUid()
-            + ", value}' "
-            + ") as \""
-            + d5.getUid()
-            + "\"";
+        "eventdatavalues #>> '{" + d5.getUid() + ", value}' as \"" + d5.getUid() + "\"";
     String aliasD6 =
         """
         case when eventdatavalues #>> '{deabcdefghH, value}' ~* '^(-?[0-9]+)(\\.[0-9]+)?$' \
@@ -509,10 +508,16 @@ void verifyGetTableWithTrackedEntityAttribute() {
     String aliasD1 =
         """
         eventdatavalues #>> '{deabcdefghZ, value}' as "deabcdefghZ\"""";
+    String aliasTeaUid =
+        """
+        (select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid \
+        and trackedentityattributeid=%d) as "%s\"""";
+
     String aliasTea1 =
-        "(select %s from \"organisationunit\" ou where ou.uid = (select value from "
-            + "\"trackedentityattributevalue\" where trackedentityid=en.trackedentityid and "
-            + "trackedentityattributeid=%d)) as \"%s\"";
+        """
+        (select %s from "organisationunit" ou where ou.uid = \
+        (select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid \
+        and trackedentityattributeid=%d)) as "%s\"""";
 
     AnalyticsTableUpdateParams params =
         AnalyticsTableUpdateParams.newBuilder()
@@ -544,14 +549,14 @@ void verifyGetTableWithTrackedEntityAttribute() {
             TEXT,
             toSelectExpression(aliasD1, d1.getUid()),
             Skip.SKIP) // ValueType.TEXT
-        .addColumn(
-            tea1.getUid(), TEXT, String.format(aliasTea1, "ou.uid", tea1.getId(), tea1.getUid()))
-        // Second Geometry column created from the OU column above
+        .addColumn(tea1.getUid(), TEXT, String.format(aliasTeaUid, tea1.getId(), tea1.getUid()))
+        // Org unit geometry column
         .addColumn(
             tea1.getUid() + "_geom",
             GEOMETRY,
             String.format(aliasTea1, "ou.geometry", tea1.getId(), tea1.getUid()),
             IndexType.GIST)
+        // Org unit name column
         .addColumn(
             tea1.getUid() + "_name",
             TEXT,
@@ -599,16 +604,21 @@ void verifyDataElementTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable(
     subject.populateTable(params, partition);
     verify(jdbcTemplate).execute(sql.capture());
 
-    String ouQuery =
-        "(select ou.%s from \"organisationunit\" ou where ou.uid = "
-            + "eventdatavalues #>> '{"
-            + d5.getUid()
-            + ", value}' ) as \""
-            + d5.getUid()
-            + "\"";
-
-    assertThat(sql.getValue(), containsString(String.format(ouQuery, "uid")));
-    assertThat(sql.getValue(), containsString(String.format(ouQuery, "name")));
+    String ouUidQuery =
+        String.format(
+            """
+            eventdatavalues #>> '{%s, value}' as %s""",
+            d5.getUid(), quote(d5.getUid()));
+
+    String ouNameQuery =
+        String.format(
+            """
+            (select ou.name from "organisationunit" ou where ou.uid = \
+            eventdatavalues #>> '{%s, value}' ) as %s""",
+            d5.getUid(), quote(d5.getUid()));
+
+    assertThat(sql.getValue(), containsString(ouUidQuery));
+    assertThat(sql.getValue(), containsString(ouNameQuery));
   }
 
   @Test
@@ -649,15 +659,23 @@ void verifyTeiTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() {
     subject.populateTable(params, partition);
     verify(jdbcTemplate).execute(sql.capture());
 
-    String ouQuery =
-        "(select ou.%s from \"organisationunit\" ou where ou.uid = "
-            + "(select value from \"trackedentityattributevalue\" where trackedentityid=en.trackedentityid and "
-            + "trackedentityattributeid=9999)) as \""
-            + tea.getUid()
-            + "\"";
-
-    assertThat(sql.getValue(), containsString(String.format(ouQuery, "uid")));
-    assertThat(sql.getValue(), containsString(String.format(ouQuery, "name")));
+    String ouUidQuery =
+        String.format(
+            """
+        (select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid and \
+        trackedentityattributeid=9999) as %s""",
+            quote(tea.getUid()));
+
+    String ouNameQuery =
+        String.format(
+            """
+        (select ou.name from "organisationunit" ou where ou.uid = \
+        (select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid and \
+        trackedentityattributeid=9999)) as %s""",
+            quote(tea.getUid()));
+
+    assertThat(sql.getValue(), containsString(ouUidQuery));
+    assertThat(sql.getValue(), containsString(ouNameQuery));
   }
 
   @Test
@@ -937,14 +955,23 @@ void verifyTeaTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() {
 
     verify(jdbcTemplate).execute(sql.capture());
 
-    String ouQuery =
-        """
-            (select ou.%s from \"organisationunit\" ou where ou.uid = \
-            (select value from \"trackedentityattributevalue\" where trackedentityid=en.trackedentityid and \
-            trackedentityattributeid=9999)) as %s""";
-
-    assertThat(sql.getValue(), containsString(String.format(ouQuery, "uid", quote(tea.getUid()))));
-    assertThat(sql.getValue(), containsString(String.format(ouQuery, "name", quote(tea.getUid()))));
+    String ouUidQuery =
+        String.format(
+            """
+            select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid \
+            and trackedentityattributeid=9999) as %s""",
+            quote(tea.getUid()));
+
+    String ouNameQuery =
+        String.format(
+            """
+        (select ou.name from "organisationunit" ou where ou.uid = \
+        (select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid \
+        and trackedentityattributeid=9999)) as %s""",
+            quote(tea.getUid()));
+
+    assertThat(sql.getValue(), containsString(ouUidQuery));
+    assertThat(sql.getValue(), containsString(ouNameQuery));
   }
 
   private String toSelectExpression(String template, String uid) {