fix: Bugs and missing features [DHIS2-18368]

dhis2 · Jan 15, 2025 · a7a4bc3 · a7a4bc3
1 parent 73b0276
commit a7a4bc3
Show file tree

Hide file tree

Showing 11 changed files with 432 additions and 196 deletions.
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/analytics/OptionSetSelection.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/analytics/OptionSetSelection.java
@@ -28,19 +28,23 @@
 package org.hisp.dhis.analytics;
 
 import java.util.Set;
-import lombok.Builder;
+import lombok.AllArgsConstructor;
 import lombok.Getter;
 
-@Builder
+@AllArgsConstructor
 @Getter
 public class OptionSetSelection {
+  private String qualifiedUid;
   private String optionSetUid;
   private Set<String> options;
   private OptionSetSelectionMode optionSetSelectionMode;
 
   @Override
   public String toString() {
     return "OptionSetSelection{"
+        + "qualifiedUid='"
+        + qualifiedUid
+        + '\''
         + "optionSetUid='"
         + optionSetUid
         + '\''

diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/DimensionalObjectUtils.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/DimensionalObjectUtils.java
@@ -394,7 +394,7 @@ public static List<String> getDimensionItemsFromParam(String param) {
     }
 
     if (param.split(DIMENSION_NAME_SEP).length > 1) {
-      // Extracts dimension items by removing dimension name and separator
+      // Extracts dimension items by removing dimension name and separator.
       String dimensionItems = param.substring(param.indexOf(DIMENSION_NAME_SEP) + 1);
 
       return Arrays.asList(DX_REGEX_PATTERN.split(dimensionItems));

diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/option/OptionSet.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/option/OptionSet.java
@@ -139,6 +139,17 @@ public List<String> getOptionCodes() {
         .collect(Collectors.toList());
   }
 
+  public Set<String> getOptionUids() {
+    if (options != null) {
+      return options.stream()
+          .filter(Objects::nonNull)
+          .map(Option::getUid)
+          .collect(Collectors.toSet());
+    }
+
+    return Set.of();
+  }
+
   public Set<String> getOptionCodesAsSet() {
     return options.stream()
         .filter(Objects::nonNull)

diff --git a/...service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java b/...service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultDataQueryService.java
@@ -30,7 +30,6 @@
 import static java.util.Objects.requireNonNull;
 import static org.apache.commons.collections4.CollectionUtils.addIgnoreNull;
 import static org.apache.commons.collections4.CollectionUtils.isNotEmpty;
-import static org.apache.commons.lang3.StringUtils.isNotBlank;
 import static org.apache.commons.lang3.StringUtils.isNotEmpty;
 import static org.hisp.dhis.analytics.AnalyticsAggregationType.fromAggregationType;
 import static org.hisp.dhis.analytics.DataQueryParams.DISPLAY_NAME_ATTRIBUTEOPTIONCOMBO;
@@ -45,34 +44,23 @@
 import static org.hisp.dhis.common.DimensionalObject.ATTRIBUTEOPTIONCOMBO_DIM_ID;
 import static org.hisp.dhis.common.DimensionalObject.CATEGORYOPTIONCOMBO_DIM_ID;
 import static org.hisp.dhis.common.DimensionalObject.DATA_X_DIM_ID;
-import static org.hisp.dhis.common.DimensionalObject.DIMENSION_IDENTIFIER_SEP;
 import static org.hisp.dhis.common.DimensionalObject.LATITUDE_DIM_ID;
 import static org.hisp.dhis.common.DimensionalObject.LONGITUDE_DIM_ID;
-import static org.hisp.dhis.common.DimensionalObject.OPTION_SEP;
 import static org.hisp.dhis.common.DimensionalObject.ORGUNIT_DIM_ID;
 import static org.hisp.dhis.common.DimensionalObject.ORGUNIT_GROUP_DIM_ID;
 import static org.hisp.dhis.common.DimensionalObject.PERIOD_DIM_ID;
 import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionFromParam;
 import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionItemsFromParam;
 import static org.hisp.dhis.common.DimensionalObjectUtils.getDimensionalItemIds;
-import static org.hisp.dhis.common.DimensionalObjectUtils.getFirstIdentifier;
 import static org.hisp.dhis.common.DimensionalObjectUtils.getItemsFromParam;
-import static org.hisp.dhis.common.DimensionalObjectUtils.getOptionSetSelectionMode;
-import static org.hisp.dhis.common.DimensionalObjectUtils.getOptionsParam;
-import static org.hisp.dhis.common.DimensionalObjectUtils.getSecondIdentifier;
-import static org.hisp.dhis.common.DimensionalObjectUtils.getThirdIdentifier;
-import static org.hisp.dhis.common.DimensionalObjectUtils.getValueFromDimensionParam;
 import static org.hisp.dhis.common.IdScheme.UID;
 import static org.hisp.dhis.feedback.ErrorCode.E7125;
 import static org.hisp.dhis.util.ObjectUtils.firstNonNull;
 
 import java.util.ArrayList;
 import java.util.Date;
-import java.util.HashMap;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
-import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -81,9 +69,6 @@
 import org.hisp.dhis.analytics.AnalyticsSecurityManager;
 import org.hisp.dhis.analytics.DataQueryParams;
 import org.hisp.dhis.analytics.DataQueryService;
-import org.hisp.dhis.analytics.OptionSetSelection;
-import org.hisp.dhis.analytics.OptionSetSelectionCriteria;
-import org.hisp.dhis.analytics.OptionSetSelectionMode;
 import org.hisp.dhis.analytics.OrgUnitField;
 import org.hisp.dhis.category.CategoryOptionCombo;
 import org.hisp.dhis.common.AnalyticalObject;
@@ -97,8 +82,6 @@
 import org.hisp.dhis.common.IdentifiableObjectManager;
 import org.hisp.dhis.common.IllegalQueryException;
 import org.hisp.dhis.feedback.ErrorMessage;
-import org.hisp.dhis.option.Option;
-import org.hisp.dhis.option.OptionSet;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
 import org.hisp.dhis.setting.UserSettings;
 import org.hisp.dhis.user.User;
@@ -117,6 +100,8 @@ public class DefaultDataQueryService implements DataQueryService {
 
   private final AnalyticsSecurityManager securityManager;
 
+  private final OptionSetFacade optionSetFacade;
+
   // -------------------------------------------------------------------------
   // DataQueryService implementation
   // -------------------------------------------------------------------------
@@ -132,7 +117,8 @@ public DataQueryParams getFromRequest(DataQueryRequest request) {
 
     if (isNotEmpty(request.getDimension())) {
       params.addDimensions(getDimensionalObjects(request));
-      params.withOptionSetSelectionCriteria(getOptionSetSelectionCriteria(request.getDimension()));
+      params.withOptionSetSelectionCriteria(
+          optionSetFacade.getOptionSetSelectionCriteria(request.getDimension()));
     }
 
     if (isNotEmpty(request.getFilter())) {
@@ -190,89 +176,6 @@ public DataQueryParams getFromRequest(DataQueryRequest request) {
         .build();
   }
 
-  /**
-   * Creates a {@link OptionSetSelectionCriteria} object based on the given collection of
-   * dimensions.
-   *
-   * @param dimensions the collection of dimensions.
-   * @return the {@link OptionSetSelectionCriteria} object.
-   */
-  private OptionSetSelectionCriteria getOptionSetSelectionCriteria(Set<String> dimensions) {
-    Map<String, OptionSetSelection> optionSetSelections = new HashMap<>();
-
-    for (String dimension : dimensions) {
-      String dimValue = getValueFromDimensionParam(dimension);
-
-      if (hasOptionSet(dimValue)) {
-        OptionSetSelectionMode mode = getOptionSetSelectionMode(dimValue);
-        String dimIdentifier = getDimensionIdentifier(dimValue);
-        OptionSetSelection.OptionSetSelectionBuilder optionSetSelectionBuilder =
-            OptionSetSelection.builder().optionSetSelectionMode(mode).optionSetUid(dimIdentifier);
-        String optionsParam = getOptionsParam(dimValue);
-
-        optionSetSelectionBuilder.options(extractOptions(optionsParam));
-        optionSetSelections.put(dimIdentifier, optionSetSelectionBuilder.build());
-      }
-    }
-
-    return new OptionSetSelectionCriteria(optionSetSelections);
-  }
-
-  /**
-   * Extracts the dimension uid based on the given argument and internal rules, depending on the
-   * composition of the value.
-   *
-   * @param composedDimension ie: WSGAb5XwJ3Y.QFX1FLWBwtq.R3ShQczKnI9[l8S7SjnQ58G;x7H1HjJ0R64]
-   * @return the respective dimension uid.
-   */
-  private static String getDimensionIdentifier(String composedDimension) {
-    String dimIdentifier = getThirdIdentifier(composedDimension);
-
-    if (dimIdentifier == null) {
-      dimIdentifier =
-          getFirstIdentifier(composedDimension)
-              + DIMENSION_IDENTIFIER_SEP
-              + getSecondIdentifier(composedDimension);
-    } else {
-      dimIdentifier =
-          getSecondIdentifier(composedDimension) + DIMENSION_IDENTIFIER_SEP + dimIdentifier;
-    }
-    return dimIdentifier;
-  }
-
-  /**
-   * Extracts the options uids specified in the URL param, if any.
-   *
-   * @param options the URL param options.
-   * @return the options uids found, or empty.
-   */
-  private Set<String> extractOptions(String options) {
-    if (isNotBlank(options)) {
-      Set<String> optionSet = new LinkedHashSet<>();
-
-      for (String uid : options.split(OPTION_SEP)) {
-        Option option = this.idObjectManager.get(Option.class, uid);
-        if (option != null) {
-          optionSet.add(option.getUid());
-        }
-      }
-
-      return optionSet;
-    }
-
-    return Set.of();
-  }
-
-  private boolean hasOptionSet(String param) {
-    String uid = getThirdIdentifier(param);
-
-    if (uid == null) {
-      uid = getSecondIdentifier(param);
-    }
-
-    return uid != null && idObjectManager.exists(OptionSet.class, uid);
-  }
-
   @Override
   @Transactional(readOnly = true)
   public DataQueryParams getFromAnalyticalObject(AnalyticalObject object) {

diff --git a/...is-service-analytics/src/main/java/org/hisp/dhis/analytics/data/JdbcAnalyticsManager.java b/...is-service-analytics/src/main/java/org/hisp/dhis/analytics/data/JdbcAnalyticsManager.java
@@ -29,7 +29,6 @@
 
 import static java.lang.String.join;
 import static org.apache.commons.collections4.CollectionUtils.size;
-import static org.apache.commons.lang3.StringUtils.EMPTY;
 import static org.apache.commons.lang3.time.DateUtils.addYears;
 import static org.hisp.dhis.analytics.AggregationType.AVERAGE;
 import static org.hisp.dhis.analytics.AggregationType.COUNT;
@@ -42,14 +41,16 @@
 import static org.hisp.dhis.analytics.DataQueryParams.LEVEL_PREFIX;
 import static org.hisp.dhis.analytics.DataQueryParams.VALUE_ID;
 import static org.hisp.dhis.analytics.DataType.TEXT;
+import static org.hisp.dhis.analytics.data.OptionSetFacade.addWhereClauseForOptions;
+import static org.hisp.dhis.analytics.data.OptionSetFacade.getAggregatedOptionValueClause;
+import static org.hisp.dhis.analytics.data.OptionSetFacade.getOptionSetSelectionMode;
 import static org.hisp.dhis.analytics.data.SubexpressionPeriodOffsetUtils.getParamsWithOffsetPeriods;
 import static org.hisp.dhis.analytics.util.AnalyticsUtils.throwIllegalQueryEx;
 import static org.hisp.dhis.analytics.util.AnalyticsUtils.withExceptionHandling;
-import static org.hisp.dhis.common.DimensionalObject.DIMENSION_IDENTIFIER_SEP;
 import static org.hisp.dhis.common.DimensionalObject.DIMENSION_SEP;
 import static org.hisp.dhis.common.IdentifiableObjectUtils.getUids;
 import static org.hisp.dhis.common.collection.CollectionUtils.concat;
-import static org.hisp.dhis.common.collection.CollectionUtils.isNotEmpty;
+import static org.hisp.dhis.system.util.SqlUtils.quote;
 import static org.hisp.dhis.util.DateUtils.toMediumDate;
 import static org.hisp.dhis.util.SqlExceptionUtils.ERR_MSG_SILENT_FALLBACK;
 import static org.hisp.dhis.util.SqlExceptionUtils.relationDoesNotExist;
@@ -62,7 +63,6 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Future;
@@ -91,7 +91,6 @@
 import org.hisp.dhis.commons.util.DebugUtils;
 import org.hisp.dhis.commons.util.SqlHelper;
 import org.hisp.dhis.commons.util.TextUtils;
-import org.hisp.dhis.dataelement.DataElement;
 import org.hisp.dhis.db.sql.SqlBuilder;
 import org.hisp.dhis.feedback.ErrorCode;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
@@ -349,7 +348,10 @@ private String getSelectClause(DataQueryParams params) {
     String sql = "select " + getCommaDelimitedQuotedDimensionColumns(params.getDimensions()) + ", ";
 
     sql += getValueClause(params);
-    sql += getAggregatedOptionValueClause(params);
+
+    if (hasAggregation(params)) {
+      sql += getAggregatedOptionValueClause(params);
+    }
 
     return sql;
   }
@@ -378,37 +380,11 @@ private boolean hasAggregation(DataQueryParams params) {
       return params.isAggregation();
     }
 
-    Optional<OptionSetSelectionMode> optionSetSelectionMode = Optional.empty();
-
-    for (DimensionalItemObject de : params.getDataElements()) {
-      if (params.hasOptionSetSelections()) {
-        OptionSetSelectionMode setSelectionMode =
-            params
-                .getOptionSetSelectionCriteria()
-                .getOptionSetSelections()
-                .get(
-                    de.getUid()
-                        + DIMENSION_IDENTIFIER_SEP
-                        + ((DataElement) de).getOptionSet().getUid())
-                .getOptionSetSelectionMode();
-        optionSetSelectionMode = Optional.of(setSelectionMode);
-        break;
-      }
-    }
-
-    OptionSetSelectionMode mode = optionSetSelectionMode.orElse(OptionSetSelectionMode.AGGREGATED);
+    OptionSetSelectionMode mode = getOptionSetSelectionMode(params);
 
     return params.isAggregation() && mode == OptionSetSelectionMode.AGGREGATED;
   }
 
-  protected String getAggregatedOptionValueClause(DataQueryParams params) {
-    if (params.hasOptionSetInDimensionItemsTypeDataElement() && hasAggregation(params)) {
-      return ", count(" + params.getValueColumn() + ") as valuecount ";
-    }
-
-    return EMPTY;
-  }
-
   /**
    * Returns an aggregate clause for the numeric value column.
    *
@@ -506,36 +482,12 @@ protected String getWhereClause(DataQueryParams params, AnalyticsTableType table
 
     getWhereClauseDimensions(params, sqlHelper, sql);
     getWhereClauseFilters(params, sqlHelper, sql);
-    getWhereClauseOptions(params, sqlHelper, sql);
     getWhereClauseDataApproval(params, sqlHelper, sql);
     getWhereClauseRestrictions(params, sqlHelper, sql, tableType);
 
     return sql.toString();
   }
 
-  /** Add where clause for option set selection. */
-  private void getWhereClauseOptions(
-      DataQueryParams params, SqlHelper sqlHelper, StringBuilder sql) {
-    if (params.hasOptionSetSelections()) {
-      params
-          .getOptionSetSelectionCriteria()
-          .getOptionSetSelections()
-          .forEach(
-              (key, value) -> {
-                Set<String> options = value.getOptions();
-                if (isNotEmpty(options)) {
-                  sql.append(" ")
-                      .append(sqlHelper.whereAnd())
-                      .append(" ")
-                      .append(quote("optionvalueuid"))
-                      .append(" in ('")
-                      .append(String.join("','", options))
-                      .append("') ");
-                }
-              });
-    }
-  }
-
   /** Add where clause dimensions. */
   private void getWhereClauseDimensions(
       DataQueryParams params, SqlHelper sqlHelper, StringBuilder sql) {
@@ -545,6 +497,8 @@ private void getWhereClauseDimensions(
         String items = sqlBuilder.singleQuotedCommaDelimited(getUids(dim.getItems()));
 
         sql.append(sqlHelper.whereAnd() + " " + col + " in (" + items + ") ");
+
+        addWhereClauseForOptions(params, sqlHelper, sqlBuilder, sql, items);
       }
     }
   }
@@ -1031,11 +985,7 @@ private Map<String, Object> getKeyValueMap(DataQueryParams params, String sql, i
       } else // NUMERIC
       {
         Double value = rowSet.getDouble(VALUE_ID);
-        if (params.hasOptionSetInDimensionItemsTypeDataElement()) {
-          map.put(key.toString() + counter, value);
-        } else {
-          map.put(key.toString(), value);
-        }
+        map.put(key.toString(), value);
       }
     }