Reviewing persistence implementations

Author: ilgrosso

.github/workflows/crosschecks.yml

@@ -31,7 +31,7 @@ jobs:
       fail-fast: false
       matrix:
         language: ['java']
-        java: [ '25', '28-ea' ]
+        java: [ '25', '27' ]
         os: [ubuntu-latest, windows-latest, macos-latest]
 
     steps:

core/idrepo/logic/src/test/java/org/apache/syncope/core/logic/ReportLogicTest.java

@@ -89,16 +89,6 @@ public static void unsetAuthContext() {
     @Autowired
     private ReportLogic logic;
 
-    private void checkExport(final String execKey) throws IOException {
-        ByteArrayOutputStream os = new ByteArrayOutputStream();
-
-        logic.exportExecutionResult(os, execKey);
-
-        os.close();
-        byte[] entity = os.toByteArray();
-        assertTrue(entity.length > 0);
-    }
-
     @Test
     void executeAndExport() throws Exception {
         ReportTO report = logic.read("0062ea9c-924d-4ecf-9961-4492a8cc6d1b");
@@ -123,6 +113,12 @@ void executeAndExport() throws Exception {
 
         String execKey = report.getExecutions().getFirst().getKey();
 
-        checkExport(execKey);
+        ByteArrayOutputStream os = new ByteArrayOutputStream();
+
+        logic.exportExecutionResult(os, execKey);
+
+        os.close();
+        byte[] entity = os.toByteArray();
+        assertTrue(entity.length > 0);
     }
 }

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);
             }
         }

…ementationManagerCacheEntryListener.java …ementationManagerCacheEntryListener.javacore/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/openjpa/ImplementationManagerCacheEntryListener.java renamed to core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/ImplementationManagerCacheEntryListener.java core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/openjpa/ImplementationManagerCacheEntryListener.java renamed to core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/ImplementationManagerCacheEntryListener.java

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.syncope.core.persistence.jpa.openjpa;
+package org.apache.syncope.core.persistence.jpa;
 
 import jakarta.persistence.EntityManagerFactory;
 import java.io.Serializable;