Merge branch 'master' into DHIS2-16051_optional_program_param_TEI

dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventQueryParams.java

@@ -35,7 +35,9 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 import lombok.Getter;
+import org.apache.commons.collections4.SetUtils;
 import org.hisp.dhis.category.CategoryOptionCombo;
 import org.hisp.dhis.common.AssignedUserQueryParam;
 import org.hisp.dhis.common.IdSchemes;
@@ -123,11 +125,7 @@ class EventQueryParams {
 
   private Set<String> events = new HashSet<>();
 
-  /**
-   * Each attribute will affect the final SQL query. Some attributes are filtered on, while
-   * attributes added via {@link #orderBy(TrackedEntityAttribute, SortDirection)} will be ordered
-   * by.
-   */
+  /** Each attribute will affect the final SQL query. Some attributes are filtered on. */
   private final Map<TrackedEntityAttribute, List<QueryFilter>> attributes = new HashMap<>();
 
   /**
@@ -408,6 +406,13 @@ public List<Order> getOrder() {
     return Collections.unmodifiableList(this.order);
   }
 
+  private Map<TrackedEntityAttribute, List<QueryFilter>> getOrderAttributes() {
+    return order.stream()
+        .filter(o -> o.getField() instanceof TrackedEntityAttribute)
+        .map(o -> (TrackedEntityAttribute) o.getField())
+        .collect(Collectors.toMap(tea -> tea, tea -> List.of()));
+  }
+
   /** Order by an event field of the given {@code field} name in given sort {@code direction}. */
   public EventQueryParams orderBy(String field, SortDirection direction) {
     this.order.add(new Order(field, direction));
@@ -424,7 +429,6 @@ public EventQueryParams orderBy(DataElement de, SortDirection direction) {
   /** Order by the given tracked entity attribute {@code tea} in given sort {@code direction}. */
   public EventQueryParams orderBy(TrackedEntityAttribute tea, SortDirection direction) {
     this.order.add(new Order(tea, direction));
-    this.attributes.putIfAbsent(tea, new ArrayList<>());
     return this;
   }
 
@@ -446,6 +450,11 @@ public EventQueryParams setEvents(Set<String> events) {
     return this;
   }
 
+  /** Returns attributes that are only ordered by and not present in any filter. */
+  public Set<TrackedEntityAttribute> leftJoinAttributes() {
+    return SetUtils.difference(getOrderAttributes().keySet(), this.attributes.keySet());
+  }
+
   public Map<TrackedEntityAttribute, List<QueryFilter>> getAttributes() {
     return this.attributes;
   }

dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java

@@ -588,12 +588,16 @@ private String buildSql(
   }
 
   /**
-   * Generates a single INNER JOIN for each attribute we are searching on. We can search by a range
-   * of operators. All searching is using lower() since attribute values are case-insensitive.
+   * Generates a single INNER JOIN for each attribute we are filtering or ordering on. We can search
+   * by a range of operators. All searching is using lower() since attribute values are
+   * case-insensitive.
    */
-  private void joinAttributeValueWithoutQueryParameter(
-      StringBuilder sql, Map<TrackedEntityAttribute, List<QueryFilter>> attributes) {
-    for (Entry<TrackedEntityAttribute, List<QueryFilter>> queryItem : attributes.entrySet()) {
+  private String joinAttributeValue(EventQueryParams params) {
+    StringBuilder sql = new StringBuilder();
+
+    for (Entry<TrackedEntityAttribute, List<QueryFilter>> queryItem :
+        params.getAttributes().entrySet()) {
+
       TrackedEntityAttribute tea = queryItem.getKey();
       String teaUid = tea.getUid();
       String teaValueCol = statementBuilder.columnQuote(teaUid);
@@ -619,6 +623,8 @@ private void joinAttributeValueWithoutQueryParameter(
           getAttributeFilterQuery(
               queryItem.getValue(), teaCol, teaValueCol, tea.getValueType().isNumeric()));
     }
+
+    return sql.toString();
   }
 
   private String getAttributeFilterQuery(
@@ -674,6 +680,35 @@ private String getAttributeFilterQuery(
     return query.toString();
   }
 
+  /**
+   * Generates the LEFT JOINs used for attributes we are ordering by (If any). We use LEFT JOIN to
+   * avoid removing any rows if there is no value for a given attribute and te. The result of this
+   * LEFT JOIN is used in the sub-query projection, and ordering in the sub-query and main query.
+   *
+   * @return a SQL LEFT JOIN for attributes used for ordering, or empty string if no attributes is
+   *     used in order.
+   */
+  private String getFromSubQueryJoinOrderByAttributes(EventQueryParams params) {
+    StringBuilder joinOrderAttributes = new StringBuilder();
+
+    for (TrackedEntityAttribute orderAttribute : params.leftJoinAttributes()) {
+
+      joinOrderAttributes
+          .append(" LEFT JOIN trackedentityattributevalue AS ")
+          .append(statementBuilder.columnQuote(orderAttribute.getUid()))
+          .append(" ON ")
+          .append(statementBuilder.columnQuote(orderAttribute.getUid()))
+          .append(".trackedentityid = TE.trackedentityid ")
+          .append("AND ")
+          .append(statementBuilder.columnQuote(orderAttribute.getUid()))
+          .append(".trackedentityattributeid = ")
+          .append(orderAttribute.getId())
+          .append(SPACE);
+    }
+
+    return joinOrderAttributes.toString();
+  }
+
   private String getEventSelectQuery(
       EventQueryParams params, MapSqlParameterSource mapSqlParameterSource, User user) {
     SqlHelper hlp = new SqlHelper();
@@ -814,9 +849,11 @@ private StringBuilder getFromWhereClause(
             "left join organisationunit teou on (te.organisationunitid=teou.organisationunitid) ")
         .append("left join userinfo au on (ev.assigneduserid=au.userinfoid) ");
 
-    if (!params.getAttributes().isEmpty()) {
-      joinAttributeValueWithoutQueryParameter(fromBuilder, params.getAttributes());
-    }
+    // JOIN attributes we need to filter on.
+    fromBuilder.append(joinAttributeValue(params));
+
+    // LEFT JOIN not filterable attributes we need to sort on.
+    fromBuilder.append(getFromSubQueryJoinOrderByAttributes(params));
 
     fromBuilder.append(getCategoryOptionComboQuery(user));
 

dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/HibernateTrackedEntityStore.java

@@ -51,7 +51,6 @@
 import org.apache.commons.lang3.StringUtils;
 import org.hibernate.SessionFactory;
 import org.hisp.dhis.common.AssignedUserSelectionMode;
-import org.hisp.dhis.common.IdentifiableObject;
 import org.hisp.dhis.common.IllegalQueryException;
 import org.hisp.dhis.common.OrganisationUnitSelectionMode;
 import org.hisp.dhis.common.Pager;
@@ -300,7 +299,6 @@ private String getQuery(TrackedEntityQueryParams params, PageParams pageParams)
     return stringBuilder
         .append("FROM ")
         .append(getFromSubQuery(params, false, pageParams))
-        .append(getQueryRelatedTables(params))
         .append(getQueryOrderBy(params, false))
         .toString();
   }
@@ -317,7 +315,6 @@ private String getCountQuery(TrackedEntityQueryParams params) {
         + getQuerySelect(params)
         + "FROM "
         + getFromSubQuery(params, true, null)
-        + getQueryRelatedTables(params)
         + " ) tecount";
   }
 
@@ -333,7 +330,6 @@ private String getCountQueryWithMaxTrackedEntityLimit(TrackedEntityQueryParams p
         + getQuerySelect(params)
         + "FROM "
         + getFromSubQuery(params, true, null)
-        + getQueryRelatedTables(params)
         + (params.getProgram().getMaxTeiCountToReturn() > 0
             ? getLimitClause(params.getProgram().getMaxTeiCountToReturn() + 1)
             : "")
@@ -582,7 +578,7 @@ private String joinAttributeValue(TrackedEntityQueryParams params) {
    * avoid removing any rows if there is no value for a given attribute and te. The result of this
    * LEFT JOIN is used in the sub-query projection, and ordering in the sub-query and main query.
    *
-   * @return a SQL LEFT JOIN for attributes used for ordering, or empty string if not attributes is
+   * @return a SQL LEFT JOIN for attributes used for ordering, or empty string if no attributes is
    *     used in order.
    */
   private String getFromSubQueryJoinOrderByAttributes(TrackedEntityQueryParams params) {
@@ -907,40 +903,6 @@ private String getQueryDateConditionBetween(
         + "' ";
   }
 
-  /**
-   * Generates SQL for LEFT JOINing relevant tables with the tracked entities we have in our result.
-   * After the sub-query, we know which tracked entities we are returning, but these LEFT JOINs will
-   * add any extra information we need. For example attribute values, tet uid, tea uid, etc.
-   *
-   * @return a SQL with several LEFT JOINS, one for each relevant table to retrieve information
-   *     from.
-   */
-  private String getQueryRelatedTables(TrackedEntityQueryParams params) {
-    StringBuilder relatedTables = new StringBuilder();
-
-    relatedTables.append(
-        "LEFT JOIN trackedentitytype TET ON TET.trackedentitytypeid = TE.trackedentitytypeid ");
-
-    if (!params.getAttributes().isEmpty()) {
-      String attributeIds =
-          getCommaDelimitedString(
-              params.getAttributes().stream().map(IdentifiableObject::getId).toList());
-
-      relatedTables
-          .append("LEFT JOIN trackedentityattributevalue TEAV ")
-          .append("ON TEAV.trackedentityid = TE.trackedentityid ")
-          .append("AND TEAV.trackedentityattributeid IN (")
-          .append(attributeIds)
-          .append(") ");
-
-      relatedTables
-          .append("LEFT JOIN trackedentityattribute TEA ")
-          .append("ON TEA.trackedentityattributeid = TEAV.trackedentityattributeid ");
-    }
-
-    return relatedTables.toString();
-  }
-
   private String getLimitClause(int limit) {
     return "LIMIT " + limit;
   }

dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityQueryParams.java

@@ -60,11 +60,8 @@
 
 @ToString
 public class TrackedEntityQueryParams {
-  /**
-   * Each attribute will affect the final SQL query. Some attributes are filtered on, while
-   * attributes added via {@link #orderBy(TrackedEntityAttribute, SortDirection)} will be ordered
-   * by.
-   */
+
+  /** Each attribute will affect the final SQL query. Some attributes are filtered on. */
   private final Map<TrackedEntityAttribute, List<QueryFilter>> filters = new HashMap<>();
 
   /**
@@ -325,11 +322,6 @@ public boolean hasUniqueFilter() {
     return false;
   }
 
-  /** Returns attributes that are either ordered by or present in any filter. */
-  public Set<TrackedEntityAttribute> getAttributes() {
-    return SetUtils.union(filters.keySet(), getOrderAttributes());
-  }
-
   /** Returns attributes that are only ordered by and not present in any filter. */
   public Set<TrackedEntityAttribute> getLeftJoinAttributes() {
     return SetUtils.difference(getOrderAttributes(), filters.keySet());

dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/event/EventQueryParamsTest.java

@@ -55,16 +55,6 @@ void setUp() {
     de1 = createDataElement('a');
   }
 
-  @Test
-  void shouldAddAttributeToOrderAndAttributesWhenOrderingByAttribute() {
-    EventQueryParams params = new EventQueryParams();
-
-    params.orderBy(tea1, SortDirection.DESC);
-
-    assertEquals(List.of(new Order(tea1, SortDirection.DESC)), params.getOrder());
-    assertEquals(Map.of(tea1, List.of()), params.getAttributes());
-  }
-
   @Test
   void shouldKeepExistingAttributeFiltersWhenOrderingByAttribute() {
     EventQueryParams params = new EventQueryParams();

dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/tracker/imports/TrackerExportTests.java

@@ -438,7 +438,9 @@ public void shouldReturnFilteredEvent() {
 
   @Test
   public void shouldReturnDescOrderedEventByTEIAttribute() {
-    ApiResponse response = trackerImportExportActions.get("events?order=dIVt4l5vIOa:desc");
+    ApiResponse response =
+        trackerImportExportActions.get(
+            "events?order=dIVt4l5vIOa:desc&event=olfXZzSGacW;ZwwuwNp6gVd");
     response.validate().statusCode(200).body("instances", hasSize(equalTo(2)));
     List<String> events = response.extractList("instances.event.flatten()");
     assertEquals(
@@ -447,7 +449,9 @@ public void shouldReturnDescOrderedEventByTEIAttribute() {
 
   @Test
   public void shouldReturnAscOrderedEventByTEIAttribute() {
-    ApiResponse response = trackerImportExportActions.get("events?order=dIVt4l5vIOa:asc");
+    ApiResponse response =
+        trackerImportExportActions.get(
+            "events?order=dIVt4l5vIOa:asc&event=olfXZzSGacW;ZwwuwNp6gVd");
     response.validate().statusCode(200).body("instances", hasSize(equalTo(2)));
     List<String> events = response.extractList("instances.event.flatten()");
     assertEquals(

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/OrderAndPaginationExporterTest.java

@@ -783,7 +783,7 @@ void shouldOrderEventsByAttributeDesc() throws ForbiddenException, BadRequestExc
   }
 
   @Test
-  void shouldOrderEventsByAttributeAndFilterOutEventsWithATrackedEntityWithoutThatAttribute()
+  void shouldOrderEventsByAttributeAndNotFilterOutEventsWithATrackedEntityWithoutThatAttribute()
       throws ForbiddenException, BadRequestException {
     EventOperationParams params =
         eventParamsBuilder
@@ -798,7 +798,7 @@ void shouldOrderEventsByAttributeAndFilterOutEventsWithATrackedEntityWithoutThat
 
     List<String> events = getEvents(params);
 
-    assertEquals(List.of("D9PbzJY8bJM"), events);
+    assertEquals(List.of("D9PbzJY8bJM", "pTzf9KYMk72"), events);
   }
 
   @Test

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/event/EventExporterTest.java

@@ -844,6 +844,24 @@ void testEnrollmentOccurredAfterEqualToLastOccurredAtDate()
     assertContainsOnly(List.of("TvctPPhpD8z"), enrollments);
   }
 
+  @Test
+  void
+      shouldFilterOutEventsWithATrackedEntityWithoutThatAttributeWhenFilterAttributeHasNoQueryFilter()
+          throws ForbiddenException, BadRequestException {
+    EventOperationParams params =
+        operationParamsBuilder
+            .orgUnitUid(orgUnit.getUid())
+            .attributeFilters(Map.of("notUpdated0", List.of()))
+            .build();
+
+    List<String> trackedEntities =
+        eventService.getEvents(params).stream()
+            .map(event -> event.getEnrollment().getTrackedEntity().getUid())
+            .collect(Collectors.toList());
+
+    assertContainsOnly(List.of("dUE514NMOlo"), trackedEntities);
+  }
+
   @Test
   void testEnrollmentFilterNumericAttributes() throws ForbiddenException, BadRequestException {
     EventOperationParams params =