feat: Improve TEI Performance [DHIS2-14884] (#14039)

* style: formatting * feat: peer review comments [DHIS2-14884] * test: fixing tests [DHIS2-14884] * test: fixing tests [DHIS2-14884] * feat: more fields extracted from json [DHIS2-14884] * feat: more fields extracted from json [DHIS2-14884] * feat: fixing warning [DHIS2-14884] * fix: e2e tests after date fields renamed * test: merge from master [DHIS2-13779] * fix: compilation problem [DHIS2-13779] * fix: fix code after merge from master [DHIS2-14884] * fix: fix e2e tests after merge [DHIS2-14884] * fix: fix sonar warning [DHIS2-14884] * fix: fixed more tests [DHIS2-14884] * fix: fixed more tests [DHIS2-14884] * fix: merged from master [DHIS2-14884] * refactor: cleaner design [DHIS2-14884] * refactor: rollback param name [DHIS2-14884] * fix: unit tests [DHIS2-14884] fix: unit tests [DHIS2-14884] feat: orderBy using subqueries [DHIS2-14884] * fix: peer review [DHIS2-14884] * fix: peer review [DHIS2-14884] * fix: peer review [DHIS2-14884] * feat: rowcontext from json [DHIS2-14884] * feat: rowcontext from json [DHIS2-14884] * feat: rowcontext from json [DHIS2-14884] * feat: conditions are now exists subquery, left joins removed [DHIS2-14884] * feat: conditions are now exists subquery, left joins removed [DHIS2-14884] * fix: fixes some small issues (dates and SCHEDULE)[DHIS2-14884] * test: makes some e2e more consistent [DHIS2-14884] * fix: fixes some small issues (dates and SCHEDULE)[DHIS2-14884] * test: makes some e2e more consistent [DHIS2-14884] * test: fixes unit test [DHIS2-14884] * test: adds date formatter test [DHIS2-14884] * fix: missing import after merge [DHIS2-14884] * fix: missing import after merge [DHIS2-14884] * fix: fixes NPE [DHIS2-17577] --------- Co-authored-by: Maikel Arabori <[email protected]>
dhis2 · Jun 26, 2024 · 100eb77 · 100eb77
1 parent fbe637c
commit 100eb77
Show file tree

Hide file tree

Showing 37 changed files with 1,602 additions and 561 deletions.
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/collection/CollectionUtils.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/collection/CollectionUtils.java
@@ -31,6 +31,7 @@
 import static java.util.stream.Collectors.toUnmodifiableSet;
 import static lombok.AccessLevel.PRIVATE;
 
+import com.google.common.collect.ImmutableMap;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -385,4 +386,20 @@ public static <T extends IdentifiableObject> boolean containsUid(
     }
     return false;
   }
+
+  /**
+   * Merges the given maps into a single map. The order of the maps is important, as the maps are
+   * merged in the order they are provided, meaning that the last map will override any duplicate
+   * keys from the previous maps.
+   *
+   * @param maps the maps to merge
+   * @param <T> the type of the keys and values in the maps
+   * @return the merged map
+   */
+  @SafeVarargs
+  public static <T> Map<T, T> merge(Map<T, T>... maps) {
+    Map<T, T> result = new HashMap<>();
+    Stream.of(maps).forEach(result::putAll);
+    return ImmutableMap.copyOf(result);
+  }
 }
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/util/DateUtils.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/util/DateUtils.java
@@ -145,6 +145,9 @@ public class DateUtils {
   private static final DateTimeFormatter LONG_DATE_FORMAT_WITH_MILLIS =
       DateTimeFormat.forPattern("yyyy-MM-dd'T'HH:mm:ss.SSS");
 
+  private static final DateTimeFormatter LONG_DATE_FORMAT_NO_T =
+      DateTimeFormat.forPattern("yyyy-MM-dd HH:mm:ss.SSS");
+
   private static final DateTimeFormatter HTTP_DATE_FORMAT =
       DateTimeFormat.forPattern("EEE, dd MMM yyyy HH:mm:ss 'GMT'").withLocale(Locale.ENGLISH);
 
@@ -206,6 +209,16 @@ public static String toLongDate(Date date) {
     return date != null ? LONG_DATE_FORMAT.print(new DateTime(date)) : null;
   }
 
+  /**
+   * Formats a Date to the format yyyy-MM-dd HH:mm:ss.S
+   *
+   * @param date the Date to parse.
+   * @return A formatted date string.
+   */
+  public static String toLongDateNoT(Date date) {
+    return date != null ? LONG_DATE_FORMAT_NO_T.print(new DateTime(date)) : null;
+  }
+
   /**
    * Formats a Date to the format yyyy-MM-dd HH:mm:ss.
    *

diff --git a/...ces/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/QueryCreator.java b/...ces/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/QueryCreator.java
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2004-2023, University of Oslo
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ * Neither the name of the HISP project nor the names of its contributors may
+ * be used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.hisp.dhis.analytics.common;
+
+public interface QueryCreator {
+
+  Query createForSelect();
+
+  Query createForCount();
+}
diff --git a/...ervices/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/SqlQuery.java b/...ervices/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/SqlQuery.java
@@ -30,34 +30,64 @@
 import static org.springframework.util.Assert.hasText;
 import static org.springframework.util.Assert.notNull;
 
+import java.util.List;
 import java.util.Map;
+import javax.annotation.Nonnull;
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
+import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
+import org.hisp.dhis.analytics.common.params.dimension.DimensionIdentifier;
+import org.hisp.dhis.analytics.common.params.dimension.DimensionParam;
+import org.hisp.dhis.analytics.tei.query.context.sql.QueryContext;
 
 /**
  * @see org.hisp.dhis.analytics.common.Query
  * @author maikel arabori
  */
 @EqualsAndHashCode
 @Slf4j
-@Getter
 public class SqlQuery implements Query {
-  private final String statement;
 
-  private final Map<String, Object> params;
+  @Getter private final String statement;
+  private final SqlQueryContext sqlQueryContext;
 
   /**
    * @throws IllegalArgumentException if statement or params are null/empty/blank.
    */
-  public SqlQuery(String statement, Map<String, Object> params) {
+  private SqlQuery(String statement, SqlQueryContext sqlQueryContext) {
     hasText(statement, "The 'statement' must not be null/empty/blank");
-    notNull(params, "The 'params' must not be null");
+    notNull(sqlQueryContext, "The 'params' must not be null");
 
     this.statement = statement;
-    this.params = params;
+    this.sqlQueryContext = sqlQueryContext;
 
     log.debug("STATEMENT: " + statement);
-    log.debug("PARAMS: " + params);
+    log.debug("PARAMS: " + sqlQueryContext);
+  }
+
+  public static SqlQuery of(String render, QueryContext queryContext) {
+    return new SqlQuery(
+        render,
+        new SqlQueryContext(
+            queryContext.getParametersPlaceHolder(),
+            queryContext.getTeiQueryParams().getCommonParams().streamDimensions().toList()));
+  }
+
+  @Nonnull
+  @Override
+  public Map<String, Object> getParams() {
+    return sqlQueryContext.getParams();
+  }
+
+  public List<DimensionIdentifier<DimensionParam>> getDimensionIdentifiers() {
+    return sqlQueryContext.getDimensionIdentifiers();
+  }
+
+  @RequiredArgsConstructor
+  @Getter
+  private static class SqlQueryContext {
+    private final Map<String, Object> params;
+    private final List<DimensionIdentifier<DimensionParam>> dimensionIdentifiers;
   }
 }
diff --git a/...dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/SqlQueryExecutor.java b/...dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/SqlQueryExecutor.java
@@ -31,6 +31,7 @@
 
 import java.util.Optional;
 import javax.annotation.Nonnull;
+import org.hisp.dhis.analytics.common.query.jsonextractor.SqlRowSetJsonExtractorDelegator;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
@@ -54,14 +55,15 @@ public SqlQueryExecutor(@Qualifier("analyticsReadOnlyJdbcTemplate") JdbcTemplate
    * @throws IllegalArgumentException if the query argument is null.
    */
   @Override
-  public SqlQueryResult find(SqlQuery query) {
+  public SqlQueryResult find(@Nonnull SqlQuery query) {
     notNull(query, "The 'query' must not be null");
 
     SqlRowSet rowSet =
         namedParameterJdbcTemplate.queryForRowSet(
             query.getStatement(), new MapSqlParameterSource().addValues(query.getParams()));
 
-    return new SqlQueryResult(rowSet);
+    return new SqlQueryResult(
+        new SqlRowSetJsonExtractorDelegator(rowSet, query.getDimensionIdentifiers()));
   }
 
   /**

diff --git a/...ices/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/TeiListGrid.java b/...ices/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/TeiListGrid.java
@@ -27,30 +27,30 @@
  */
 package org.hisp.dhis.analytics.common;
 
+import static java.lang.String.format;
 import static java.util.Objects.isNull;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import java.sql.ResultSet;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import org.apache.commons.lang3.StringUtils;
+import lombok.extern.slf4j.Slf4j;
 import org.hisp.dhis.analytics.common.params.dimension.DimensionIdentifier;
 import org.hisp.dhis.analytics.common.params.dimension.DimensionParam;
+import org.hisp.dhis.analytics.common.query.jsonextractor.SqlRowSetJsonExtractorDelegator;
 import org.hisp.dhis.analytics.tei.TeiQueryParams;
 import org.hisp.dhis.common.Grid;
 import org.hisp.dhis.common.GridHeader;
-import org.hisp.dhis.common.ValueStatus;
 import org.hisp.dhis.common.ValueType;
-import org.hisp.dhis.event.EventStatus;
 import org.hisp.dhis.system.grid.ListGrid;
 import org.hisp.dhis.system.util.MathUtils;
 import org.springframework.jdbc.support.rowset.SqlRowSet;
 
+@Slf4j
 public class TeiListGrid extends ListGrid {
 
   private static final List<ValueType> ROUNDABLE_TYPES =
@@ -112,7 +112,8 @@ public Grid addNamedRows(SqlRowSet rs) {
           addValue(value);
           headersSet.add(columnLabel);
 
-          rowContextItems.putAll(getRowContextItem(rs, cols[i], i));
+          rowContextItems.putAll(
+              ((SqlRowSetJsonExtractorDelegator) rs).getRowContextItem(cols[i], i));
         }
       }
       if (!rowContextItems.isEmpty()) {
@@ -130,17 +131,26 @@ public Grid addNamedRows(SqlRowSet rs) {
   private Object getValueAndRoundIfNecessary(
       SqlRowSet rs, String columnLabel, boolean skipRounding) {
     ValueType valueType = getValueType(columnLabel);
+    Object value = rs.getObject(columnLabel);
     if (skipRounding || isNotRoundableType(valueType)) {
-      return rs.getObject(columnLabel);
+      return value;
+    }
+    // if roundable type we try to parse from string into double and round it
+    try {
+      return roundIfNecessary(value);
+    } catch (Exception e) {
+      log.warn(
+          format("Failed to parse value as double: %s for column: %s ", value, columnLabel), e);
+      // as a fallback we return the value as is
+      return value;
     }
-    return roundIfNecessary(rs, columnLabel);
   }
 
-  private Double roundIfNecessary(SqlRowSet rs, String columnLabel) {
-    if (isNull(rs.getObject(columnLabel))) {
+  private Double roundIfNecessary(Object value) {
+    if (isNull(value)) {
       return null;
     }
-    double doubleValue = rs.getDouble(columnLabel);
+    double doubleValue = Double.parseDouble(value.toString());
     if (teiQueryParams.getCommonParams().isSkipRounding()) {
       return doubleValue;
     }
@@ -161,49 +171,4 @@ private ValueType getValueType(String col) {
         .map(DimensionParam::getValueType)
         .orElse(ValueType.TEXT);
   }
-
-  /**
-   * The method retrieves row context content that describes the origin of the data value,
-   * indicating whether it is set, not set, or undefined. The column index is used as the map key,
-   * and the corresponding value contains information about the origin, also known as the value
-   * status.
-   *
-   * @param rs the {@link ResultSet},
-   * @param columnName the {@link String}, grid row column name
-   * @return Map of column index and value status
-   */
-  private Map<String, Object> getRowContextItem(SqlRowSet rs, String columnName, int rowIndex) {
-    Map<String, Object> rowContextItem = new HashMap<>();
-    String existIndicatorColumnLabel = columnName + EXISTS;
-    String statusIndicatorColumnLabel = columnName + STATUS;
-    String hasValueIndicatorColumnLabel = columnName + HAS_VALUE;
-
-    if (Arrays.stream(rs.getMetaData().getColumnNames())
-        .anyMatch(n -> n.equalsIgnoreCase(existIndicatorColumnLabel))) {
-
-      boolean isDefined = rs.getBoolean(existIndicatorColumnLabel);
-      boolean isSet = rs.getBoolean(hasValueIndicatorColumnLabel);
-      boolean isScheduled =
-          StringUtils.equalsIgnoreCase(
-              rs.getString(statusIndicatorColumnLabel), EventStatus.SCHEDULE.toString());
-
-      ValueStatus valueStatus = ValueStatus.SET;
-
-      if (!isDefined) {
-        valueStatus = ValueStatus.NOT_DEFINED;
-      } else if (isScheduled) {
-        valueStatus = ValueStatus.SCHEDULED;
-      } else if (!isSet) {
-        valueStatus = ValueStatus.NOT_SET;
-      }
-
-      if (valueStatus != ValueStatus.SET) {
-        Map<String, String> valueStatusMap = new HashMap<>();
-        valueStatusMap.put("valueStatus", valueStatus.getValue());
-        rowContextItem.put(Integer.toString(rowIndex), valueStatusMap);
-      }
-    }
-
-    return rowContextItem;
-  }
 }
diff --git a/...dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/ValueTypeMapping.java b/...dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/ValueTypeMapping.java
@@ -60,17 +60,11 @@ public enum ValueTypeMapping {
   DATE(ValueTypeMapping::dateConverter, LocalDate.class, LocalDateTime.class),
   TIME(s -> s, ValueType.TIME, s -> s.replace(".", ":"), "varchar"),
   BOOLEAN(
-      ValueTypeMapping::booleanConverter,
-      ValueTypeMapping::booleanSelectTransformer,
-      Boolean.class);
-
-  private static final UnaryOperator<String> BOOLEAN_SELECT_TRANSFORMER =
-      columnName ->
-          "case when "
-              + columnName
-              + " = 'true' then 1 when "
-              + columnName
-              + " = 'false' then 0 end";
+      ValueTypeMapping::booleanConverter, ValueTypeMapping::booleanJsonExtractor, Boolean.class);
+
+  private static final UnaryOperator<String> BOOLEAN_JSON_EXTRACTOR =
+      value -> value.equalsIgnoreCase("true") ? "1" : "0";
+
   private final Function<String, Object> converter;
   @Getter private final UnaryOperator<String> selectTransformer;
   private final ValueType[] valueTypes;
@@ -115,7 +109,7 @@ private static boolean isAssignableFrom(Class[] classes, ValueType valueType) {
       Class... classes) {
     this.converter = converter;
     this.valueTypes = fromClasses(classes);
-    this.selectTransformer = selectTransformer;
+    this.selectTransformer = s -> Objects.isNull(s) ? null : selectTransformer.apply(s);
     this.argumentTransformer = UnaryOperator.identity();
     this.postgresCast = name();
   }
@@ -148,8 +142,8 @@ private static boolean isTrue(String value) {
     return "1".equals(value) || "true".equalsIgnoreCase(value);
   }
 
-  private static String booleanSelectTransformer(String columnName) {
-    return BOOLEAN_SELECT_TRANSFORMER.apply(columnName);
+  private static String booleanJsonExtractor(String value) {
+    return BOOLEAN_JSON_EXTRACTOR.apply(value);
   }
 
   /**

diff --git a/...cs/src/main/java/org/hisp/dhis/analytics/common/params/dimension/DimensionIdentifier.java b/...cs/src/main/java/org/hisp/dhis/analytics/common/params/dimension/DimensionIdentifier.java
@@ -105,6 +105,10 @@ public boolean isEnrollmentDimension() {
     return hasProgram() && !hasProgramStage();
   }
 
+  public boolean isTeDimension() {
+    return !hasProgram() && !hasProgramStage();
+  }
+
   public boolean isEventDimension() {
     return hasProgram() && hasProgramStage();
   }

diff --git a/.../main/java/org/hisp/dhis/analytics/common/params/dimension/DimensionIdentifierHelper.java b/.../main/java/org/hisp/dhis/analytics/common/params/dimension/DimensionIdentifierHelper.java
@@ -36,8 +36,10 @@
 import static org.hisp.dhis.analytics.common.params.dimension.DimensionParam.StaticDimension.INCIDENTDATE;
 import static org.hisp.dhis.analytics.common.params.dimension.DimensionParam.StaticDimension.OCCURREDDATE;
 import static org.hisp.dhis.analytics.common.params.dimension.DimensionParam.StaticDimension.OUNAME;
+import static org.hisp.dhis.analytics.common.params.dimension.DimensionParamObjectType.DATA_ELEMENT;
 import static org.hisp.dhis.analytics.common.params.dimension.ElementWithOffset.emptyElementWithOffset;
 import static org.hisp.dhis.analytics.tei.query.context.QueryContextConstants.TEI_ALIAS;
+import static org.hisp.dhis.analytics.tei.query.context.sql.SqlQueryBuilders.isOfType;
 import static org.hisp.dhis.analytics.util.AnalyticsUtils.throwIllegalQueryEx;
 import static org.hisp.dhis.common.DimensionalObject.DIMENSION_IDENTIFIER_SEP;
 import static org.hisp.dhis.commons.util.TextUtils.doubleQuote;
@@ -280,4 +282,14 @@ private static String getStaticDimensionDisplayName(
       return null;
     }
   }
+
+  /**
+   * Checks if the given dimension identifier is of type data element.
+   *
+   * @param dimensionIdentifier the dimension identifier to check.
+   * @return true if the dimension identifier is of type data element, false otherwise.
+   */
+  public static boolean isDataElement(DimensionIdentifier<DimensionParam> dimensionIdentifier) {
+    return isOfType(dimensionIdentifier, DATA_ELEMENT) && dimensionIdentifier.isEventDimension();
+  }
 }