MCR-3283 Enable PMD rule DoubleCheckedLocking (#2407)

* MCR-3283 Start working on PMD rule DoubleCheckedLocking

* MCR-3283 Enable PMD rule DoubleCheckedLocking

mycore-base/src/main/java/org/mycore/access/MCRAccessCacheManager.java

@@ -19,7 +19,6 @@
 package org.mycore.access;
 
 import java.util.List;
-import java.util.Optional;
 import java.util.Set;
 import java.util.UUID;
 import java.util.stream.Collectors;
@@ -33,38 +32,23 @@
 
 /**
  * @author Thomas Scheffler (yagee)
- *
  */
 class MCRAccessCacheManager {
+
     private static final int CAPACITY = MCRConfiguration2.getOrThrow("MCR.Access.Cache.Size", Integer::valueOf);
 
-    private static String key = MCRAccessCacheManager.class.getCanonicalName();
+    private static final String SESSION_PERMISSION_KEY = MCRAccessCacheManager.class.getCanonicalName();
 
+    @SuppressWarnings("unchecked")
     private MCRCache<MCRPermissionHandle, Boolean> getSessionPermissionCache() {
         MCRSession session = MCRSessionMgr.getCurrentSession();
-        @SuppressWarnings("unchecked")
-        MCRCache<MCRPermissionHandle, Boolean> cache = (MCRCache<MCRPermissionHandle, Boolean>) session.get(key);
-        if (cache == null) {
-            synchronized (getCacheCreationLock(session)) {
-                cache = (MCRCache<MCRPermissionHandle, Boolean>) session.get(key);
-                if (cache == null) {
-                    cache = createCache(session);
-                    session.put(key, cache);
-                }
-            }
-        }
-        return cache;
-    }
-
-    private static Object getCacheCreationLock(MCRSession session) {
-        //in a short living scoped session, we should not lock the whole session, but a unique object
-        return Optional.of(session)
-            .map(s -> s.get(MCRScopedSession.SCOPED_HINT))
-            .orElse(session);
+        return (MCRCache<MCRPermissionHandle, Boolean>) session.computeIfAbsent(
+            SESSION_PERMISSION_KEY, k -> createCache(session));
     }
 
+    @SuppressWarnings("unchecked")
     private MCRCache<MCRPermissionHandle, Boolean> getCacheFromSession(MCRSession session) {
-        return (MCRCache<MCRPermissionHandle, Boolean>) session.get(key);
+        return (MCRCache<MCRPermissionHandle, Boolean>) session.get(SESSION_PERMISSION_KEY);
     }
 
     private MCRCache<MCRPermissionHandle, Boolean> createCache(MCRSession session) {
@@ -101,7 +85,7 @@ private void removePermissionFromCache(MCRCache<MCRPermissionHandle, Boolean> pe
             .stream()
             .filter(hdl -> hdl.id() != null)
             .filter(hdl -> ids.contains(hdl.id()))
-            .collect(Collectors.toList());
+            .toList();
         handlesToRemove.forEach(permissionCache::remove);
     }
 
@@ -119,6 +103,6 @@ private record MCRPermissionHandle(String id, String permission) {
         private MCRPermissionHandle {
             permission = permission.intern();
         }
-
     }
+
 }

mycore-base/src/main/java/org/mycore/common/MCRSession.java

@@ -45,6 +45,7 @@
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -248,6 +249,28 @@ public Object put(Object key, Object value) {
         return map.put(key, value);
     }
 
+    /**
+     * Retrieves the value associated with the given key within the session, or computes and stores a new value if
+     * absent.
+     * <p>
+     * This method ensures thread-safe lazy initialization by leveraging
+     * {@link ConcurrentHashMap#computeIfAbsent(Object, Function)}.
+     * <p>
+     * If the key is not already present in the session, the provided mapping function is used to compute the value,
+     * which is then stored in the session.
+     *
+     * @param key             the key whose associated value is to be retrieved or computed
+     * @param mappingFunction the function to compute a value if the key is absent
+     * @return the existing or newly computed value associated with the key
+     * @throws NullPointerException if the key or mapping function is null
+     */
+    public Object computeIfAbsent(Object key, Function<Object, Object> mappingFunction) {
+        if(!map.containsKey(key)) {
+            mapChanged = true;
+        }
+        return map.computeIfAbsent(key, mappingFunction);
+    }
+
     /** Returns the object that was stored in the session under the given key * */
     public Object get(Object key) {
         return map.get(key);

mycore-base/src/main/java/org/mycore/datamodel/ifs2/MCRStoreCenter.java

@@ -18,9 +18,10 @@
 
 package org.mycore.datamodel.ifs2;
 
-import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Supplier;
 import java.util.stream.Stream;
 
 public final class MCRStoreCenter {
@@ -30,7 +31,7 @@ public final class MCRStoreCenter {
     private static volatile MCRStoreCenter instance;
 
     private MCRStoreCenter() {
-        this.storeHeap = new HashMap<>();
+        this.storeHeap = new ConcurrentHashMap<>();
     }
 
     public static MCRStoreCenter instance() {
@@ -52,10 +53,23 @@ public static MCRStoreCenter instance() {
      */
     public void addStore(String id, MCRStore store) throws MCRStoreAlreadyExistsException {
         if (storeHeap.putIfAbsent(id, store) != null) {
-            throw new MCRStoreAlreadyExistsException("Could not add store with ID " + id + ", store allready exists");
+            throw new MCRStoreAlreadyExistsException("Could not add store with ID " + id + ", store already exists");
         }
     }
 
+    /**
+     * Computes a store if it does not exist yet.
+     *
+     * @param id store id
+     * @param storeSupplier the mapping function to create a store
+     * @return the original store or the newly computed one
+     * @param <T> the store
+     */
+    @SuppressWarnings("unchecked")
+    public <T extends MCRStore> T computeStoreIfAbsent(String id, Supplier<T> storeSupplier) {
+        return (T) storeHeap.computeIfAbsent(id, k -> storeSupplier.get());
+    }
+
     /**
      * Get the MyCoRe Store with the given ID from store center.
      *
@@ -107,4 +121,5 @@ public boolean removeStore(String id) {
     public void clear() {
         storeHeap.clear();
     }
+
 }

mycore-base/src/main/java/org/mycore/datamodel/ifs2/MCRStoreManager.java

@@ -18,33 +18,95 @@
 
 package org.mycore.datamodel.ifs2;
 
+import java.util.function.Supplier;
+
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.mycore.common.MCRException;
 import org.mycore.datamodel.ifs2.MCRStore.MCRStoreConfig;
 
+/**
+ * Manages the lifecycle of {@link MCRStore} instances, including creation, retrieval, and removal.
+ * <p>
+ * This class provides methods to create new stores, retrieve existing ones, and remove them from
+ * the {@link MCRStoreCenter}. It ensures proper initialization and thread-safe store access.
+ */
 public class MCRStoreManager {
-    private static final Logger LOGGER = LogManager.getLogger(MCRStoreManager.class);
 
+    private static final Logger LOGGER = LogManager.getLogger();
+
+    /**
+     * Builds a new store instance using the default configuration  and adds it to the {@link MCRStoreCenter}.
+     *
+     * @param id         the unique identifier for the store
+     * @param storeClass the class of the store to create
+     * @return the created store instance
+     * @throws ReflectiveOperationException if the store cannot be instantiated
+     */
     public static <T extends MCRStore> T createStore(String id, Class<T> storeClass)
         throws ReflectiveOperationException {
         return createStore(new MCRStoreDefaultConfig(id), storeClass);
     }
 
+    /**
+     * Builds a new store instance with the specified configuration and adds it to the {@link MCRStoreCenter}.
+     *
+     * @param config     the store configuration
+     * @param storeClass the class of the store to create
+     * @return the created store instance
+     * @throws ReflectiveOperationException if the store cannot be instantiated
+     */
     public static <T extends MCRStore> T createStore(MCRStoreConfig config, Class<T> storeClass)
         throws ReflectiveOperationException {
-        T store = storeClass.getDeclaredConstructor().newInstance();
-        store.init(config);
+        T store = buildStore(config, storeClass);
         try {
             LOGGER.info("Adding instance of {} as MCRStore '{}'.", storeClass::getSimpleName, store::getID);
             MCRStoreCenter.instance().addStore(store.getID(), store);
         } catch (Exception e) {
-            throw new MCRException("Could not create store with ID " + config.getID() + ", store allready exists", e);
+            throw new MCRException("Could not create store with ID " + config.getID() + ", store already exists", e);
         }
+        return store;
+    }
 
+    /**
+     * Builds a new store instance using the default configuration.
+     *
+     * @param id         the unique identifier for the store
+     * @param storeClass the class of the store to create
+     * @return the built store instance
+     * @throws ReflectiveOperationException if the store cannot be instantiated
+     */
+    public static <T extends MCRStore> T buildStore(String id, Class<T> storeClass)
+        throws ReflectiveOperationException {
+        return buildStore(new MCRStoreDefaultConfig(id), storeClass);
+    }
+
+    /**
+     * Builds a new store instance with the specified configuration.
+     *
+     * @param config     the store configuration
+     * @param storeClass the class of the store to create
+     * @return the built store instance
+     * @throws ReflectiveOperationException if the store cannot be instantiated
+     */
+    public static <T extends MCRStore> T buildStore(MCRStoreConfig config, Class<T> storeClass)
+        throws ReflectiveOperationException {
+        T store = storeClass.getDeclaredConstructor().newInstance();
+        store.init(config);
         return store;
     }
 
+    /**
+     * Retrieves an existing store by its ID, or creates and registers a new one if absent.
+     *
+     * @param id            the store ID
+     * @param storeSupplier the supplier function to create a store if it does not exist
+     * @return the existing or newly created store instance
+     */
+    public static <T extends MCRStore> T computeStoreIfAbsent(String id, Supplier<T> storeSupplier) {
+        return MCRStoreCenter.instance().computeStoreIfAbsent(id, storeSupplier);
+    }
+
     /**
      * Returns the store with the given id
      *
@@ -67,4 +129,5 @@ public static void removeStore(String id) {
         LOGGER.info("Remove MCRStore '{}'.", id);
         MCRStoreCenter.instance().removeStore(id);
     }
+
 }

mycore-ifs/src/main/java/org/mycore/datamodel/niofs/ifs2/MCRFileSystemUtils.java

@@ -56,11 +56,10 @@
 
 /**
  * @author Thomas Scheffler
- *
  */
 final class MCRFileSystemUtils {
 
-    private static final Logger LOGGER = LogManager.getLogger(MCRFileSystemUtils.class);
+    private static final Logger LOGGER = LogManager.getLogger();
 
     private static final String DEFAULT_CONFIG_PREFIX = "MCR.IFS.ContentStore.IFS2."; //todo: rename
 
@@ -96,27 +95,16 @@ static MCRFileCollection getFileCollection(String owner) throws IOException {
     }
 
     static MCRFileStore getStore(String base) {
-        String storeBaseDir = getBaseDir();
-
+        String baseDir = getBaseDir();
         String sid = STORE_ID_PREFIX + base;
-        storeBaseDir += File.separatorChar + base.replace("_", File.separator);
-
-        MCRFileStore store = MCRStoreManager.getStore(sid);
-        if (store == null) {
-            synchronized (MCRStoreManager.class) {
-                store = MCRStoreManager.getStore(sid);
-                if (store == null) {
-                    store = createStore(sid, storeBaseDir, base);
-                }
-            }
-        }
-        return store;
+        String storeBaseDir = baseDir + File.separatorChar + base.replace("_", File.separator);
+        return MCRStoreManager.computeStoreIfAbsent(sid, () -> buildStore(sid, storeBaseDir, base));
     }
 
-    private static MCRFileStore createStore(String sid, String storeBaseDir, String base) {
+    private static MCRFileStore buildStore(String sid, String storeBaseDir, String base) {
         try {
             configureStore(sid, storeBaseDir, base);
-            return MCRStoreManager.createStore(sid, MCRFileStore.class);
+            return MCRStoreManager.buildStore(sid, MCRFileStore.class);
         } catch (Exception ex) {
             String msg = "Could not create IFS2 file store with ID " + sid;
             throw new MCRConfigurationException(msg, ex);

mycore-oai/src/main/java/org/mycore/oai/MCROAIDataProvider.java

@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.ServiceLoader;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -73,7 +74,7 @@ public class MCROAIDataProvider extends MCRServlet {
     private static final OAIXMLOutputProcessor OAI_XML_OUTPUT_PROCESSOR = new OAIXMLOutputProcessor();
 
     static {
-        ADAPTER_MAP = new HashMap<>();
+        ADAPTER_MAP = new ConcurrentHashMap<>();
     }
 
     private String myBaseURL;
@@ -105,16 +106,16 @@ protected void doGetPost(MCRServletJob job) throws Exception {
         // handle request
         OAIResponse oaiResponse = oaiProvider.handleRequest(oaiRequest);
         // build response
-        Element xmlRespone = oaiResponse.toXML();
+        Element xmlResponse = oaiResponse.toXML();
 
         if (!(adapter instanceof MCROAIAdapter mcrAdapter) || mcrAdapter.moveNamespaceDeclarationsToRoot()) {
-            moveNamespacesUp(xmlRespone);
+            moveNamespacesUp(xmlResponse);
         }
 
         // fire
         job.getResponse().setContentType("text/xml; charset=UTF-8");
         XMLOutputter xout = new XMLOutputter(Format.getPrettyFormat(), OAI_XML_OUTPUT_PROCESSOR);
-        xout.output(addXSLStyle(new Document(xmlRespone)), job.getResponse().getOutputStream());
+        xout.output(addXSLStyle(new Document(xmlResponse)), job.getResponse().getOutputStream());
     }
 
     /**
@@ -165,28 +166,23 @@ private Document addXSLStyle(Document doc) {
 
     private OAIAdapter getOAIAdapter() {
         String oaiAdapterKey = getServletName();
-        MCROAIAdapter oaiAdapter = ADAPTER_MAP.get(oaiAdapterKey);
-        if (oaiAdapter == null) {
-            synchronized (this) {
-                // double check because of synchronize block
-                oaiAdapter = ADAPTER_MAP.get(oaiAdapterKey);
-                if (oaiAdapter == null) {
-                    String adapter = MCROAIAdapter.PREFIX + oaiAdapterKey + ".Adapter";
-                    oaiAdapter = MCRConfiguration2.getInstanceOf(MCROAIAdapter.class, adapter)
-                        .orElseGet(() -> MCRConfiguration2.getInstanceOfOrThrow(
-                            MCROAIAdapter.class, MCROAIAdapter.PREFIX + "DefaultAdapter"));
-                    oaiAdapter.init(this.myBaseURL, oaiAdapterKey);
-                    ADAPTER_MAP.put(oaiAdapterKey, oaiAdapter);
-                }
-            }
-        }
+        return ADAPTER_MAP.computeIfAbsent(oaiAdapterKey, k -> createOAIAdapter(oaiAdapterKey));
+    }
+
+    private MCROAIAdapter createOAIAdapter(String oaiAdapterKey) {
+        MCROAIAdapter oaiAdapter;
+        String adapter = MCROAIAdapter.PREFIX + oaiAdapterKey + ".Adapter";
+        oaiAdapter = MCRConfiguration2.getInstanceOf(MCROAIAdapter.class, adapter)
+            .orElseGet(() -> MCRConfiguration2.getInstanceOfOrThrow(
+                MCROAIAdapter.class, MCROAIAdapter.PREFIX + "DefaultAdapter"));
+        oaiAdapter.init(this.myBaseURL, oaiAdapterKey);
         return oaiAdapter;
     }
 
     /**
      * Moves all namespace declarations in the children of target to the target.
      *
-     * @param target the namespace are bundled here
+     * @param target the namespace is bundled here
      */
     private void moveNamespacesUp(Element target) {
         Map<String, Namespace> existingNamespaces = getNamespaceMap(target);

ruleset.xml

@@ -313,10 +313,7 @@
   <rule ref="category/java/multithreading.xml/DoNotUseThreads"/>
   -->
   <rule ref="category/java/multithreading.xml/DontCallThreadRun"/>
-  <!-- TODO: Create PR to fix this rule -->
-  <!--
   <rule ref="category/java/multithreading.xml/DoubleCheckedLocking"/>
-  -->
   <rule ref="category/java/multithreading.xml/NonThreadSafeSingleton"/>
   <rule ref="category/java/multithreading.xml/UnsynchronizedStaticFormatter"/>
   <!-- TODO: Create PR to fix this rule -->