refactor: object list special filter queries [DHIS2-17200] (#19046)

* fix: non-standard object list queries [DHIS2-17200]

* fix: users may use standard pager

* refactor: special filters as pre-stage query for ids

* chore: revert now unnecessary change

* refactor: use query filters for OU special params

* fix: failing integration test causes

* refactor: parameter objects for getObject and getObjectList

* fix: dependency issues

* fix: params in tests

* fix: don't order count query, use getFieldName() for properties in criteria API

* fix: fields parameter and filter and order parameter mapping

* fix: strip regex from path variables for path

* fix: filter parameter splitting

* refactor: always use AbstractCrudController with list params parameter

* refactor: always use object list parameter for read-only CRUD controller

* fix: broken superclass and import

* fix: add missing @EntityType definitions when/where used

* fix: metadata import service

* fix: filters on getObject

* fix: dimension controller issues

* fix: maven dependencies

* fix: attempt to disable dependency analzye run for e2e tests

* fix: sonar issues and add tests

* fix: always add sorting and cache hints in JPA query engine

* fix: more sonar issues

* refactor: user special filters as UID pre-query, checked exceptions in filter creation

* fix: query user IDs

* fix: user query for IDs and always false detection

* fix: sonar TODOs and issues

* fix: in operator with empty value list is always false

* chore: cleanup OU special filters

* fix: code scanning alert no. 9132: User-controlled data in arithmetic expression

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

.github/workflows/run-api-tests.yml

@@ -41,13 +41,13 @@ jobs:
             # build and publish multi-arch images using Jib. Image is used for api tests in
             # this workflow and can be pulled from Dockerhub by devs to run locally, ...
             mvn clean verify --threads 2C --batch-mode --no-transfer-progress \
-              -DskipTests --update-snapshots --file dhis-2/pom.xml \
+              -DskipTests -Dmdep.analyze.skip --update-snapshots --file dhis-2/pom.xml \
               --projects dhis-web-server --also-make --activate-profiles jibBuild \
               -Djib.to.image=$CORE_IMAGE_NAME -Djib.container.labels=DHIS2_BUILD_REVISION=${{github.event.pull_request.head.sha}},DHIS2_BUILD_BRANCH=${{github.head_ref}}
           else
             # only build image for running api tests in this workflow i.e. master, 2.39, ...
             mvn clean verify --threads 2C --batch-mode --no-transfer-progress \
-              -DskipTests --update-snapshots --file dhis-2/pom.xml \
+              -DskipTests -Dmdep.analyze.skip --update-snapshots --file dhis-2/pom.xml \
               --projects dhis-web-server --also-make --activate-profiles jibDockerBuild \
               -Djib.to.image=$CORE_IMAGE_NAME
           fi

dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/IdentifiableObjectManager.java

@@ -182,9 +182,6 @@ <T extends IdentifiableObject> List<T> getByCode(
   @CheckForNull
   <T extends IdentifiableObject> T search(@Nonnull Class<T> type, @Nonnull String query);
 
-  @Nonnull
-  <T extends IdentifiableObject> List<T> filter(@Nonnull Class<T> type, @Nonnull String query);
-
   @Nonnull
   <T extends IdentifiableObject> List<T> getAll(@Nonnull Class<T> type);
 

dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/OpenApi.java

@@ -469,6 +469,7 @@ enum Status {
    */
   @Target(ElementType.TYPE)
   @Retention(RetentionPolicy.RUNTIME)
+  @Inherited
   @interface Shared {
 
     /**

dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/collection/CollectionUtils.java

@@ -214,6 +214,7 @@ private static <T> Set<T> union(@Nonnull Set<T>... more) {
    * @return a string, or null if no matches.
    */
   public static String popStartsWith(Collection<String> collection, String prefix) {
+    if (collection == null || collection.isEmpty()) return null;
     Iterator<String> iterator = collection.iterator();
 
     while (iterator.hasNext()) {

dhis-2/dhis-services/dhis-service-field-filtering/src/main/java/org/hisp/dhis/fieldfiltering/FieldPreset.java → dhis-2/dhis-api/src/main/java/org/hisp/dhis/fieldfiltering/FieldPreset.java

@@ -52,7 +52,7 @@ public enum FieldPreset {
 
   FieldPreset(String name, List<String> fields) {
     this.name = name;
-    this.fields = fields;
+    this.fields = List.copyOf(fields);
   }
 
   public String getName() {

dhis-2/dhis-api/src/main/java/org/hisp/dhis/interpretation/MentionUtils.java

@@ -31,7 +31,6 @@
 import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
-import java.util.ListIterator;
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -62,17 +61,4 @@ public static Set<User> getMentionedUsers(String text, UserService userService)
     }
     return users;
   }
-
-  public static List<String> removeCustomFilters(List<String> filters) {
-    List<String> mentions = new ArrayList<String>();
-    ListIterator<String> filterIterator = filters.listIterator();
-    while (filterIterator.hasNext()) {
-      String[] filterSplit = filterIterator.next().split(":");
-      if (filterSplit[1].equals("in") && filterSplit[0].equals("mentions")) {
-        mentions.add(filterSplit[2]);
-        filterIterator.remove();
-      }
-    }
-    return mentions;
-  }
 }

dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageConversation.java

@@ -41,6 +41,7 @@
 import org.apache.commons.lang3.StringUtils;
 import org.hisp.dhis.common.BaseIdentifiableObject;
 import org.hisp.dhis.common.DxfNamespaces;
+import org.hisp.dhis.common.UID;
 import org.hisp.dhis.schema.annotation.PropertyRange;
 import org.hisp.dhis.user.User;
 
@@ -192,12 +193,12 @@ public boolean isRead(User user) {
     return false;
   }
 
-  public boolean markRead(User user) {
-    for (UserMessage userMessage : userMessages) {
-      if (userMessage.getUser() != null && userMessage.getUser().equals(user)) {
-        boolean read = userMessage.isRead();
+  public boolean markRead(UID user) {
+    for (UserMessage msg : userMessages) {
+      if (msg.getUser() != null && msg.getUser().getUid().equals(user.getValue())) {
+        boolean read = msg.isRead();
 
-        userMessage.setRead(true);
+        msg.setRead(true);
 
         return !read;
       }

dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitService.java

@@ -133,14 +133,6 @@ public interface OrganisationUnitService extends OrganisationUnitDataIntegrityPr
    */
   List<OrganisationUnit> getOrganisationUnitsByUid(@Nonnull Collection<String> uids);
 
-  /**
-   * Returns a list of OrganisationUnits based on the given params.
-   *
-   * @param params the params.
-   * @return a list of OrganisationUnits.
-   */
-  List<OrganisationUnit> getOrganisationUnitsByQuery(OrganisationUnitQueryParams params);
-
   /**
    * Returns an OrganisationUnit with a given name.
    *

dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserDetails.java

@@ -28,6 +28,7 @@
 package org.hisp.dhis.user;
 
 import java.util.Collection;
+import java.util.List;
 import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -250,6 +251,14 @@ static UserDetails createUserDetails(
 
   void setId(Long id);
 
+  default boolean canIssueUserRole(UserRole role, boolean canGrantOwnUserRole) {
+    if (role == null) return false;
+    if (isSuper()) return true;
+    if (hasAnyAuthorities(List.of(Authorities.ALL))) return true;
+    if (!canGrantOwnUserRole && getUserRoleIds().contains(role.getUid())) return false;
+    return getAllAuthorities().containsAll(role.getAuthorities());
+  }
+
   default boolean isInUserHierarchy(String orgUnitPath) {
     return isInUserHierarchy(orgUnitPath, getUserOrgUnitIds());
   }

dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserService.java

@@ -43,6 +43,7 @@
 import org.hisp.dhis.common.IdentifiableObject;
 import org.hisp.dhis.common.UID;
 import org.hisp.dhis.dataset.DataSet;
+import org.hisp.dhis.feedback.ConflictException;
 import org.hisp.dhis.feedback.ErrorCode;
 import org.hisp.dhis.feedback.ErrorReport;
 import org.hisp.dhis.feedback.ForbiddenException;
@@ -235,6 +236,17 @@ static boolean hasTwoFactorSecretForApproval(User user) {
    */
   List<User> getUsers(UserQueryParams params, @Nullable List<String> orders);
 
+  /**
+   * Returns a list of users based on the given query parameters. If the specified list of orders
+   * are empty, default order of last name and first name will be applied.
+   *
+   * @param params the user query parameters.
+   * @param orders the already validated order strings (e.g. email:asc).
+   * @return a List of users.
+   */
+  List<UID> getUserIds(UserQueryParams params, @Nullable List<String> orders)
+      throws ConflictException;
+
   /**
    * Returns the number of users based on the given query parameters.
    *
@@ -408,12 +420,9 @@ static boolean hasTwoFactorSecretForApproval(User user) {
   int countDataSetUserRoles(DataSet dataSet);
 
   /**
-   * Filters the given collection of user roles based on whether the current user is allowed to
-   * issue it.
-   *
-   * @param userRoles the collection of user roles.
+   * @return IDs of the roles the current user can issue
    */
-  void canIssueFilter(Collection<UserRole> userRoles);
+  List<UID> getRolesCurrentUserCanIssue();
 
   /**
    * Validate that the current user are allowed to create or modify properties of the given user.

dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserStore.java

@@ -65,6 +65,8 @@ public interface UserStore extends IdentifiableObjectStore<User> {
    */
   List<User> getUsers(UserQueryParams params, @Nullable List<String> orders);
 
+  List<UID> getUserIds(UserQueryParams params, @Nullable List<String> orders);
+
   /**
    * Returns the number of users based on the given query parameters.
    *

dhis-2/dhis-services/dhis-service-core/pom.xml

@@ -30,6 +30,7 @@
     <dependency>
       <groupId>org.hisp.dhis</groupId>
       <artifactId>dhis-service-node</artifactId>
+      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.hisp.dhis</groupId>

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/common/DefaultIdentifiableObjectManager.java

@@ -39,9 +39,7 @@
 import jakarta.persistence.EntityManager;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Date;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -409,34 +407,6 @@ public <T extends IdentifiableObject> T search(@Nonnull Class<T> type, @Nonnull
     return object;
   }
 
-  @Nonnull
-  @Override
-  @Transactional(readOnly = true)
-  public <T extends IdentifiableObject> List<T> filter(
-      @Nonnull Class<T> type, @Nonnull String query) {
-    Set<T> uniqueObjects = new HashSet<>();
-
-    T uidObject = get(type, query);
-
-    if (uidObject != null) {
-      uniqueObjects.add(uidObject);
-    }
-
-    T codeObject = getByCode(type, query);
-
-    if (codeObject != null) {
-      uniqueObjects.add(codeObject);
-    }
-
-    uniqueObjects.addAll(getLikeName(type, query, false));
-
-    List<T> objects = new ArrayList<>(uniqueObjects);
-
-    Collections.sort(objects);
-
-    return objects;
-  }
-
   @Nonnull
   @Override
   @Transactional(readOnly = true)

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/organisationunit/DefaultOrganisationUnitService.java

@@ -163,12 +163,6 @@ public List<OrganisationUnit> getOrganisationUnitsByUid(@Nonnull Collection<Stri
     return organisationUnitStore.getByUid(new HashSet<>(uids));
   }
 
-  @Override
-  @Transactional(readOnly = true)
-  public List<OrganisationUnit> getOrganisationUnitsByQuery(OrganisationUnitQueryParams params) {
-    return organisationUnitStore.getOrganisationUnits(params);
-  }
-
   @Override
   @Transactional(readOnly = true)
   public OrganisationUnit getOrganisationUnit(String uid) {

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/Criteria.java

@@ -61,7 +61,7 @@ public Criteria add(Criterion... criterions) {
     return this;
   }
 
-  public Criteria add(Collection<Criterion> criterions) {
+  public Criteria add(Collection<? extends Criterion> criterions) {
     this.criterions.addAll(criterions);
     return this;
   }

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/Criterion.java

@@ -30,4 +30,13 @@
 /**
  * @author Morten Olav Hansen <[email protected]>
  */
-public interface Criterion {}
+public interface Criterion {
+
+  /**
+   * @return true, when the condition cannot match any rows, e.g. an in-operator with an empty
+   *     collection to test against
+   */
+  default boolean isAlwaysFalse() {
+    return false;
+  }
+}