Reviewing persistence implementations

core/idrepo/rest-cxf/src/main/java/org/apache/syncope/core/rest/cxf/ThreadLocalCleanupOutInterceptor.java

@@ -22,7 +22,6 @@
 import org.apache.cxf.message.Message;
 import org.apache.cxf.phase.AbstractPhaseInterceptor;
 import org.apache.cxf.phase.Phase;
-import org.apache.syncope.core.persistence.api.utils.FormatUtils;
 import org.identityconnectors.common.l10n.CurrentLocale;
 import org.identityconnectors.framework.impl.api.local.ThreadClassLoaderManager;
 import org.slf4j.MDC;
@@ -38,8 +37,6 @@ public ThreadLocalCleanupOutInterceptor() {
 
     @Override
     public void handleMessage(final Message message) throws Fault {
-        FormatUtils.clear();
-
         ThreadClassLoaderManager.clearInstance();
         CurrentLocale.clear();
 

core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/SyncopeAbstractSearchCondVisitor.java

@@ -50,7 +50,7 @@
 public abstract class SyncopeAbstractSearchCondVisitor
         extends AbstractSearchConditionVisitor<SearchBean, SearchCond> {
 
-    protected static final ThreadLocal<SearchCond> SEARCH_COND = new ThreadLocal<>();
+    protected SearchCond searchCond;
 
     protected static AttrCond createAttrCond(final String schema) {
         AttrCond attrCond = SearchableFields.contains(schema)
@@ -264,11 +264,11 @@ protected SearchCond visitCompound(final SearchCondition<SearchBean> sc) {
 
     @Override
     public void visit(final SearchCondition<SearchBean> sc) {
-        SEARCH_COND.set(sc.getStatement() == null ? visitCompound(sc) : visitPrimitive(sc));
+        searchCond = sc.getStatement() == null ? visitCompound(sc) : visitPrimitive(sc);
     }
 
     @Override
     public SearchCond getQuery() {
-        return SEARCH_COND.get();
+        return searchCond;
     }
 }

core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/search/SyncopePage.java

@@ -47,9 +47,15 @@ public int getTotalPages() {
 
     @Override
     public boolean equals(final Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (!(obj instanceof SyncopePage<?> other)) {
+            return false;
+        }
         return new EqualsBuilder().
                 appendSuper(super.equals(obj)).
-                append(total, total).
+                append(total, other.total).
                 build();
     }
 

core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/utils/FormatUtils.java

@@ -41,12 +41,6 @@ public final class FormatUtils {
 
     private static final String NO_CONVERSION_PATTERN = "#,##0.###";
 
-    private static final ThreadLocal<DecimalFormat> DECIMAL_FORMAT = ThreadLocal.withInitial(() -> {
-        DecimalFormat df = new DecimalFormat(NO_CONVERSION_PATTERN);
-        df.setDecimalFormatSymbols(DecimalFormatSymbols.getInstance(Locale.ENGLISH));
-        return df;
-    });
-
     public static final ZoneOffset DEFAULT_OFFSET = OffsetDateTime.now().getOffset();
 
     public static String format(final TemporalAccessor temporal) {
@@ -64,41 +58,19 @@ public static String format(final long number) {
     }
 
     public static String format(final long number, final String conversionPattern) {
-        DecimalFormat df = DECIMAL_FORMAT.get();
-
-        String previous = df.toPattern();
-        if (!previous.equals(conversionPattern)) {
-            df.applyPattern(conversionPattern);
-        }
-
-        String formatted = df.format(number);
-
-        if (!previous.equals(conversionPattern)) {
-            df.applyPattern(previous);
-        }
-
-        return formatted;
+        final DecimalFormat df = new DecimalFormat(conversionPattern);
+        df.setDecimalFormatSymbols(DecimalFormatSymbols.getInstance(Locale.ENGLISH));
+        return df.format(number);
     }
 
     public static String format(final double number) {
         return format(number, NO_CONVERSION_PATTERN);
     }
 
     public static String format(final double number, final String conversionPattern) {
-        DecimalFormat df = DECIMAL_FORMAT.get();
-
-        String previous = df.toPattern();
-        if (!previous.equals(conversionPattern)) {
-            df.applyPattern(conversionPattern);
-        }
-
-        String formatted = df.format(number);
-
-        if (!previous.equals(conversionPattern)) {
-            df.applyPattern(previous);
-        }
-
-        return formatted;
+        final DecimalFormat df = new DecimalFormat(conversionPattern);
+        df.setDecimalFormatSymbols(DecimalFormatSymbols.getInstance(Locale.ENGLISH));
+        return df.format(number);
     }
 
     public static OffsetDateTime parseDate(final String source)
@@ -141,15 +113,11 @@ public static OffsetDateTime parseDate(final String source, final String convers
     }
 
     public static Number parseNumber(final String source, final String conversionPattern) throws ParseException {
-        DecimalFormat df = DECIMAL_FORMAT.get();
-        df.applyPattern(conversionPattern);
+        final DecimalFormat df = new DecimalFormat(conversionPattern);
+        df.setDecimalFormatSymbols(DecimalFormatSymbols.getInstance(Locale.ENGLISH));
         return df.parse(source);
     }
 
-    public static void clear() {
-        DECIMAL_FORMAT.remove();
-    }
-
     private FormatUtils() {
         // private empty constructor
     }

core/persistence-common/src/main/java/org/apache/syncope/core/persistence/common/dao/AbstractAnyMatchDAO.java

@@ -23,11 +23,13 @@
 import jakarta.validation.constraints.Min;
 import java.beans.PropertyDescriptor;
 import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.time.OffsetDateTime;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Locale;
 import java.util.Optional;
 import java.util.Set;
 import java.util.regex.Pattern;
@@ -181,12 +183,6 @@ public <T extends Any> boolean matches(final T any, final SearchCond cond) {
                             orElse(null));
                 }
 
-                if (match == null) {
-                    match = cond.asLeaf(AttrCond.class).
-                            map(leaf -> matches(any, leaf, not)).
-                            orElse(null);
-                }
-
                 return BooleanUtils.toBoolean(match);
             }
             case AND -> {
@@ -299,7 +295,7 @@ protected boolean matches(
                 case LIKE, ILIKE -> {
                     if (schema.getType().isStringClass()) {
                         StringBuilder output = new StringBuilder();
-                        for (char c : cond.getExpression().toLowerCase().toCharArray()) {
+                        for (char c : cond.getExpression().toLowerCase(Locale.ROOT).toCharArray()) {
                             if (c == '%') {
                                 output.append(".*");
                             } else if (Character.isLetter(c)) {
@@ -428,11 +424,19 @@ protected boolean matches(final Any any, final AnyCond cond, final boolean not)
                 boolean foundBooleanMin = false;
                 boolean foundBooleanMax = false;
                 if (Integer.class.equals(pd.getPropertyType())) {
-                    for (Annotation annotation : pd.getPropertyType().getAnnotations()) {
-                        if (Min.class.equals(annotation.annotationType())) {
-                            foundBooleanMin = ((Min) annotation).value() == 0;
-                        } else if (Max.class.equals(annotation.annotationType())) {
-                            foundBooleanMax = ((Max) annotation).value() == 1;
+                    for (Class<?> clazz = any.getClass(); clazz != null; clazz = clazz.getSuperclass()) {
+                        try {
+                            final Field field = clazz.getDeclaredField(pd.getName());
+                            for (final Annotation annotation : field.getAnnotations()) {
+                                if (Min.class.equals(annotation.annotationType())) {
+                                    foundBooleanMin = ((Min) annotation).value() == 0;
+                                } else if (Max.class.equals(annotation.annotationType())) {
+                                    foundBooleanMax = ((Max) annotation).value() == 1;
+                                }
+                            }
+                            break;
+                        } catch (final NoSuchFieldException e) {
+                            // continue walking up the class hierarchy
                         }
                     }
                 }

core/persistence-common/src/main/java/org/apache/syncope/core/persistence/common/dao/AbstractAnySearchDAO.java

@@ -20,8 +20,10 @@
 
 import java.util.ArrayList;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -272,11 +274,17 @@ protected <T extends Any> List<T> buildResult(final List<Object> raw, final AnyT
                 map(key -> key instanceof Object[] array ? (String) (array)[0] : ((String) key)).
                 toList();
 
+        Map<String, Integer> keyPositions = new HashMap<>();
+        for (int i = 0; i < keys.size(); i++) {
+            keyPositions.put(keys.get(i), i);
+        }
+
         // sort anys according to keys' sorting, as their ordering is same as raw, e.g. the actual query results
         List<Any> anys = anyUtilsFactory.getInstance(kind).dao().findByKeys(keys).stream().
-                sorted(Comparator.comparing(any -> keys.indexOf(any.getKey()))).toList();
+                sorted(Comparator.comparingInt(any -> keyPositions.getOrDefault(any.getKey(), -1))).toList();
 
-        keys.stream().filter(key -> anys.stream().noneMatch(any -> key.equals(any.getKey()))).
+        Set<String> foundKeys = anys.stream().map(Any::getKey).collect(Collectors.toSet());
+        keys.stream().filter(key -> !foundKeys.contains(key)).
                 forEach(key -> LOG.error("Could not find {} with id {}, even if returned by native query", kind, key));
 
         return (List<T>) anys;

core/persistence-common/src/main/java/org/apache/syncope/core/persistence/common/dao/AbstractSearchDAO.java

@@ -197,7 +197,7 @@ protected List<SearchCond> buildDerAttrValueConditions(
         // Contains used identifiers in order to avoid replications
         Set<String> used = new HashSet<>();
 
-        // Create several clauses: one for eanch identifiers
+        // Create several clauses: one for each identifier
         for (int i = 0; i < identifiers.size() && !used.contains(identifiers.get(i)); i++) {
             used.add(identifiers.get(i));
 
@@ -251,7 +251,7 @@ protected CheckResult<AnyCond> check(final AnyCond cond, final Field field, fina
         PlainSchema schema = entityFactory.newEntity(PlainSchema.class);
         schema.setKey(field.getName());
         for (AttrSchemaType attrSchemaType : AttrSchemaType.values()) {
-            if (field.getType().isAssignableFrom(attrSchemaType.getType())) {
+            if (attrSchemaType.getType().isAssignableFrom(field.getType())) {
                 schema.setType(attrSchemaType);
             }
         }

core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/PersistenceContext.java

@@ -239,8 +239,9 @@ public CommonEntityManagerFactoryConf commonEMFConf(
                 + "FieldStrategies='"
                 + "java.util.Locale=org.apache.syncope.core.persistence.jpa.openjpa.LocaleValueHandler'");
 
-        jpaPropertyMap.put("openjpa.DataCache", "true");
-        jpaPropertyMap.put("openjpa.QueryCache", "true");
+        // bound the in-memory caches, otherwise they can grow unbounded on long-running installations
+        jpaPropertyMap.put("openjpa.DataCache", "true(CacheSize=1000,SoftReferenceSize=100)");
+        jpaPropertyMap.put("openjpa.QueryCache", "true(CacheSize=1000,SoftReferenceSize=0)");
 
         jpaPropertyMap.put("openjpa.RemoteCommitProvider", props.getRemoteCommitProvider());
 

core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/AbstractJPAAnySearchDAO.java

@@ -755,7 +755,7 @@ protected long doCount(
         fillWithParameters(countQuery, parameters);
 
         // 5. execute the query and return the result
-        return ((Number) countQuery.getSingleResult()).intValue();
+        return ((Number) countQuery.getSingleResult()).longValue();
     }
 
     protected abstract void parseOrderByForPlainSchema(
@@ -773,7 +773,6 @@ protected void parseOrderByForField(
             final Sort.Order clause) {
 
         item.select = defaultSV(svs).alias() + '.' + fieldName;
-        item.where = StringUtils.EMPTY;
         item.orderBy = defaultSV(svs).alias() + '.' + fieldName + ' ' + clause.getDirection().name();
     }
 

core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/dao/AbstractJPARealmSearchDAO.java

@@ -23,8 +23,10 @@
 import jakarta.persistence.Query;
 import jakarta.persistence.TypedQuery;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -56,6 +58,8 @@
 
 public abstract class AbstractJPARealmSearchDAO extends AbstractRealmSearchDAO {
 
+    protected static final int IN_CLAUSE_MAX_SIZE = 500;
+
     protected record QueryInfo(RealmSearchNode node, Set<String> plainSchemas) {
 
     }
@@ -200,13 +204,13 @@ protected Optional<QueryInfo> getQuery(final SearchCond cond, final List<Object>
                     node = cond.asLeaf(AnyCond.class)
                             .map(anyCond -> getQuery(anyCond, not, parameters))
                             .or(() -> cond.asLeaf(AttrCond.class).map(attrCond -> {
-                        CheckResult<AttrCond> checked = check(attrCond);
-                        AttrCondQuery query = getQuery(attrCond, not, checked, parameters);
-                        if (query.addPlainSchemas()) {
-                            plainSchemas.add(checked.schema().getKey());
-                        }
-                        return query.node();
-                    }));
+                                CheckResult<AttrCond> checked = check(attrCond);
+                                AttrCondQuery query = getQuery(attrCond, not, checked, parameters);
+                                if (query.addPlainSchemas()) {
+                                    plainSchemas.add(checked.schema().getKey());
+                                }
+                                return query.node();
+                            }));
                 }
 
                 if (node.isEmpty()) {
@@ -423,8 +427,8 @@ protected String buildFrom(final Set<String> plainSchemas, final OrderBySupport
     protected String buildWhere(final Set<String> bases, final QueryInfo queryInfo, final List<Object> parameters) {
         String fullPaths = bases.stream().
                 map(base -> "r.fullPath=?" + setParameter(parameters, base)
-                + " OR r.fullPath LIKE ?" + setParameter(
-                        parameters, SyncopeConstants.ROOT_REALM.equals(base) ? "/%" : base + "/%")).
+                        + " OR r.fullPath LIKE ?" + setParameter(
+                                parameters, SyncopeConstants.ROOT_REALM.equals(base) ? "/%" : base + "/%")).
                 collect(Collectors.joining(" OR "));
 
         RealmSearchNode root;
@@ -481,7 +485,6 @@ protected void parseOrderByForField(
             final Sort.Order clause) {
 
         item.select = "r." + fieldName;
-        item.where = StringUtils.EMPTY;
         item.orderBy = "r." + fieldName + ' ' + clause.getDirection().name();
     }
 
@@ -580,9 +583,27 @@ protected List<Realm> doSearch(final Set<String> bases, final SearchCond cond, f
         @SuppressWarnings("unchecked")
         List<String> keys = query.getResultList().stream().
                 map(key -> key instanceof Object[] array ? (array)[0] : key).
+                map(Object::toString).
                 toList();
 
-        return keys.stream().map(k -> entityManager.find(JPARealm.class, k)).
-                filter(Objects::nonNull).map(Realm.class::cast).toList();
+        if (keys.isEmpty()) {
+            return List.of();
+        }
+
+        Map<String, Realm> found = new HashMap<>();
+        for (int i = 0; i < keys.size(); i += IN_CLAUSE_MAX_SIZE) {
+            List<String> chunk = keys.subList(i, Math.min(i + IN_CLAUSE_MAX_SIZE, keys.size()));
+            entityManager.createQuery(
+                    "SELECT e FROM " + JPARealm.class.getSimpleName() + " e WHERE e.id IN :keys", JPARealm.class).
+                    setParameter("keys", chunk).
+                    getResultList().
+                    forEach(realm -> found.put(realm.getKey(), realm));
+        }
+
+        return keys.stream().
+                map(found::get).
+                filter(Objects::nonNull).
+                map(Realm.class::cast).
+                toList();
     }
 }