merge: resolve master conflict after dhis2#24803

dhis2#24803 already landed the fuller nativeSynchronizedQuery fix for
HibernateJobConfigurationStore, including the FileResource query space.
Drop the temporary local synchronizedNativeQuery subset from this branch
and keep the PR's ehcache hot-region predefinitions.

Author: netroms

dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks.yaml

@@ -50,6 +50,7 @@ checks:
   - program_rules/program_rules_no_priority.yaml
   - program_rules/program_rules_inconsistent_program_program_stage.yaml
   - analytical_objects/visualizations_not_used_1year.yaml
+  - analytical_objects/visualizations_wrong_sort_order.yaml
   - analytical_objects/maps_not_used_1year.yaml
   - analytical_objects/dashboards_not_used_1year.yaml
   - analytical_objects/dashboards_empty.yaml

dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks/analytical_objects/visualizations_wrong_sort_order.yaml

@@ -0,0 +1,84 @@
+# Copyright (c) 2004-2022, 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.
+#
+---
+name:  visualizations_wrong_sort_order
+description: Visualizations with possibly wrong sort order of data dimension items
+section: Visualizations
+section_order: 2
+summary_sql: >-
+    WITH visualizations_wrong_sort_order AS (
+    SELECT DISTINCT visualizationid, sort_order, expected_order
+    FROM (
+    SELECT
+    visualizationid,
+    sort_order,
+    row_number() OVER (PARTITION BY visualizationid ORDER BY sort_order)
+    + (MIN(sort_order) OVER (PARTITION BY visualizationid) - 1) AS expected_order
+    FROM visualization_datadimensionitems
+    ) AS foo
+    WHERE sort_order != expected_order
+    ORDER BY visualizationid, sort_order
+    )
+    SELECT
+    COUNT(*) AS value,
+    100 * (SELECT COUNT(*) FROM (SELECT DISTINCT visualizationid FROM visualizations_wrong_sort_order) AS bar) /
+    NULLIF((SELECT COUNT(*) FROM visualization), 0) AS percent
+    FROM visualizations_wrong_sort_order;
+details_sql: >-
+    SELECT
+    a.uid,
+    a.name,
+    b.sort_order || ' != ' || b.expected_order AS comment
+    FROM visualization a
+    INNER JOIN (
+    SELECT DISTINCT visualizationid, sort_order, expected_order
+    FROM (
+    SELECT
+    visualizationid,
+    sort_order,
+    row_number() OVER (PARTITION BY visualizationid ORDER BY sort_order)
+    + (MIN(sort_order) OVER (PARTITION BY visualizationid) - 1) AS expected_order
+    FROM visualization_datadimensionitems
+    ) AS foo
+    WHERE sort_order != expected_order
+    ORDER BY visualizationid, sort_order
+    ) b ON a.visualizationid = b.visualizationid;
+details_id_type: visualizations
+severity: CRITICAL
+introduction: >
+    The data dimension items within a visualization are ordered using a sort order column. If
+    this sort order contains gaps (for example 0, 1, 3, 4 instead of 0, 1, 2, 3), the visualization
+    will fail to open with an internal server error: the sort order is used as a list index when
+    the visualization is loaded, and a gap causes the missing position to be reconstructed as an
+    empty entry, which then causes an error when the visualization's data dimension items are
+    processed. This check identifies any visualizations where the sort order of the data dimension
+    items contains gaps.
+recommendation: >
+    To correct the sort order of a visualization's data dimension items, an administrator can
+    directly update the sort_order column of the affected rows in the visualization_datadimensionitems
+    table so that it forms a contiguous sequence starting at 0, with no gaps or duplicates, for each
+    affected visualization.

dhis-2/dhis-services/dhis-service-administration/src/test/java/org/hisp/dhis/dataintegrity/DataIntegrityYamlReaderTest.java

@@ -93,7 +93,7 @@ void testReadDataIntegrityYaml() {
 
     List<DataIntegrityCheck> checks = new ArrayList<>();
     readYaml(checks, "data-integrity-checks.yaml", "data-integrity-checks", CLASS_PATH);
-    assertEquals(95, checks.size());
+    assertEquals(96, checks.size());
 
     // Names should be unique
     List<String> allNames = checks.stream().map(DataIntegrityCheck::getName).toList();

dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/query/jsonextractor/SqlRowSetJsonExtractorDelegator.java

@@ -92,11 +92,42 @@ public class SqlRowSetJsonExtractorDelegator extends SqlRowSetDelegator {
     OBJECT_MAPPER.findAndRegisterModules();
   }
 
+  /** Package-private so that a test can count how often the document is actually parsed. */
   @SneakyThrows
-  private List<JsonEnrollment> parseEnrollmentsFromJson(String json) {
+  List<JsonEnrollment> parseEnrollmentsFromJson(String json) {
     return OBJECT_MAPPER.readValue(json, new TypeReference<>() {});
   }
 
+  /**
+   * Returns the parsed {@code enrollments} document for the row the cursor is on, parsing it at
+   * most once per row.
+   *
+   * <p>Every requested column used to trigger its own {@code readValue} of the whole document, from
+   * two independent call sites ({@link #getObject(String)} and {@link #getRowContextItem(String,
+   * int)}), so a line list with ten data-element dimensions parsed the same string around twenty
+   * times per row.
+   *
+   * <p>The memo is keyed on the document itself rather than on the cursor position, which is what
+   * makes a stale hit impossible rather than merely unlikely: a hit requires the current row's
+   * document to be equal to the memoised one, and equal input cannot produce a different parse
+   * result. {@link String#equals} short-circuits on identity, which is the case a forward cursor
+   * hits on every column after the first.
+   *
+   * <p>The returned list is shared between the callers within a row. Nothing downstream mutates it
+   * - the extractors only stream over it, and {@code getItemBasedOnOffset} sorts the stream, not
+   * the source.
+   */
+  private List<JsonEnrollment> getEnrollments() {
+    String json = super.getString("enrollments");
+
+    if (memoisedEnrollments == null || !Objects.equals(memoisedJson, json)) {
+      memoisedEnrollments = parseEnrollmentsFromJson(json);
+      memoisedJson = json;
+    }
+
+    return memoisedEnrollments;
+  }
+
   private static final Comparator<JsonEnrollment> ENR_ENROLLMENT_DATE_COMPARATOR =
       comparing(JsonEnrollment::getEnrollmentDate, nullsFirst(naturalOrder())).reversed();
 
@@ -107,6 +138,14 @@ private List<JsonEnrollment> parseEnrollmentsFromJson(String json) {
 
   private final List<String> existingColumnsInRowSet;
 
+  /**
+   * The {@code enrollments} document of the row the cursor is on, and its parse. Per-instance state
+   * on a per-request object; this class is not shared between threads and must not become so.
+   */
+  private String memoisedJson;
+
+  private List<JsonEnrollment> memoisedEnrollments;
+
   public SqlRowSetJsonExtractorDelegator(
       SqlRowSet sqlRowSet, List<DimensionIdentifier<DimensionParam>> dimensionIdentifiers) {
     super(sqlRowSet);
@@ -179,7 +218,7 @@ public Object getObject(String columnLabel) throws InvalidResultSetAccessExcepti
       return super.getObject(canonicalLabel);
     }
     // if the column is not present in the rowset, we check if it is present in the json string
-    List<JsonEnrollment> enrollments = parseEnrollmentsFromJson(super.getString("enrollments"));
+    List<JsonEnrollment> enrollments = getEnrollments();
 
     DimensionIdentifier<DimensionParam> dimensionIdentifier = dimIdByKey.get(canonicalLabel);
     if (dimensionIdentifier == null) {
@@ -350,8 +389,7 @@ public Map<String, Object> getRowContextItem(String columnName, int rowIndex) {
     }
 
     JsonEvent event =
-        getJsonEnrollment(
-                parseEnrollmentsFromJson(super.getString("enrollments")), dimensionIdentifier)
+        getJsonEnrollment(getEnrollments(), dimensionIdentifier)
             .map(jEnr -> getJsonEvent(dimensionIdentifier, jEnr))
             .orElse(null);
 

dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/EventAnalyticsUtils.java

@@ -145,12 +145,13 @@ public static Map<String, Object> getAggregatedEventDataMapping(Grid grid) {
    * Adds values.
    *
    * @param identifiers the list of list of identifiers.
-   * @param grid the input {@link Grid}.
+   * @param valueMap the value map of the input {@link Grid}, as returned by {@link
+   *     #getAggregatedEventDataMapping(Grid)}. It depends only on the input grid, so callers which
+   *     invoke this method repeatedly for the same grid must build it once and reuse it.
    * @param outputGrid the output {@link Grid}.
    */
-  public static void addValues(List<List<String>> identifiers, Grid grid, Grid outputGrid) {
-    Map<String, Object> valueMap = getAggregatedEventDataMapping(grid);
-
+  public static void addValues(
+      List<List<String>> identifiers, Map<String, Object> valueMap, Grid outputGrid) {
     boolean hasValues = false;
 
     for (List<String> idList : identifiers) {

dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/EventAggregateService.java

@@ -49,6 +49,7 @@
 import static org.hisp.dhis.analytics.common.ColumnHeader.PROGRAM_STATUS;
 import static org.hisp.dhis.analytics.event.EventAnalyticsUtils.addValues;
 import static org.hisp.dhis.analytics.event.EventAnalyticsUtils.generateEventDataPermutations;
+import static org.hisp.dhis.analytics.event.EventAnalyticsUtils.getAggregatedEventDataMapping;
 import static org.hisp.dhis.analytics.event.LabelMapper.getEnrollmentDateLabel;
 import static org.hisp.dhis.analytics.event.LabelMapper.getIncidentDateLabel;
 import static org.hisp.dhis.analytics.event.LabelMapper.getOrgUnitLabel;
@@ -636,6 +637,10 @@ private Grid generateOutputGrid(
           outputGrid.addHeader(new GridHeader(display, display, NUMBER, false, false));
         });
 
+    // The value map is a pure function of the input grid, which is not modified below. Build it
+    // once here instead of once per row permutation.
+    Map<String, Object> valueMap = getAggregatedEventDataMapping(grid);
+
     for (Map<String, EventAnalyticsDimensionalItem> rowCombination : rowPermutations) {
       outputGrid.addRow();
       List<List<String>> ids = new ArrayList<>();
@@ -663,7 +668,7 @@ private Grid generateOutputGrid(
       }
 
       addValuesInOutputGrid(rowDimensions, outputGrid, displayObjects, params);
-      addValues(ids, grid, outputGrid);
+      addValues(ids, valueMap, outputGrid);
     }
 
     return getGridWithRows(grid, outputGrid);
@@ -684,19 +689,19 @@ private static Grid getGridWithRows(Grid grid, Grid outputGrid) {
    * empty.
    *
    * @param rowDimensions the list of row dimensions.
-   * @param grid the {@link Grid}.
+   * @param outputGrid the output {@link Grid}.
    * @param displayObjects the map of display objects.
    * @param params the {@link EventQueryParams}.
    */
   private static void addValuesInOutputGrid(
       List<String> rowDimensions,
-      Grid grid,
+      Grid outputGrid,
       Map<String, EventAnalyticsDimensionalItem> displayObjects,
       EventQueryParams params) {
     if (!displayObjects.isEmpty()) {
       rowDimensions.forEach(
           dimension ->
-              grid.addValue(
+              outputGrid.addValue(
                   displayObjects.get(dimension).getDisplayProperty(params.getDisplayProperty())));
     }
   }