Rearrange

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>

Author: Ravi Nadahar

bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/CacheScriptExtension.java

@@ -12,6 +12,8 @@
  */
 package org.openhab.core.automation.module.script.rulesupport.internal;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -52,6 +54,7 @@ public class CacheScriptExtension implements ScriptExtensionProvider {
     static final String SHARED_CACHE_NAME = "sharedCache";
     static final String PRIVATE_CACHE_NAME = "privateCache";
     static final String OBJECT_CACHE_NAME = "objectCache";
+    static final String LOCKING_CACHE_NAME = "lockingCache";
 
     private final Logger logger = LoggerFactory.getLogger(CacheScriptExtension.class);
     private final ScheduledExecutorService scheduler = ThreadPoolManager
@@ -60,6 +63,7 @@ public class CacheScriptExtension implements ScriptExtensionProvider {
     private final Lock cacheLock = new ReentrantLock();
     private final Map<String, Object> sharedCache = new HashMap<>();
     private final ObjectCache objectCache = new ObjectCacheImpl();
+    private final LockingCache lockingCache = new LockingCacheImpl();
     private final Map<String, Set<String>> sharedCacheKeyAccessors = new ConcurrentHashMap<>();
 
     private final Map<String, ValueCacheImpl> privateCaches = new ConcurrentHashMap<>();
@@ -79,7 +83,7 @@ public Collection<String> getPresets() {
 
     @Override
     public Collection<String> getTypes() {
-        return Set.of(PRIVATE_CACHE_NAME, SHARED_CACHE_NAME, OBJECT_CACHE_NAME);
+        return Set.of(PRIVATE_CACHE_NAME, SHARED_CACHE_NAME, OBJECT_CACHE_NAME, LOCKING_CACHE_NAME);
     }
 
     @Override
@@ -91,6 +95,8 @@ public Collection<String> getTypes() {
                 return privateCaches.computeIfAbsent(scriptIdentifier, ValueCacheImpl::new);
             case OBJECT_CACHE_NAME:
                 return objectCache;
+            case LOCKING_CACHE_NAME:
+                return lockingCache;
             default:
                 return null;
         }
@@ -102,7 +108,7 @@ public Map<String, Object> importPreset(String scriptIdentifier, String preset)
             Object privateCache = Objects
                     .requireNonNull(privateCaches.computeIfAbsent(scriptIdentifier, ValueCacheImpl::new));
             return Map.of(SHARED_CACHE_NAME, new TrackingValueCacheImpl(scriptIdentifier), PRIVATE_CACHE_NAME,
-                    privateCache, OBJECT_CACHE_NAME, objectCache);
+                    privateCache, OBJECT_CACHE_NAME, objectCache, LOCKING_CACHE_NAME, lockingCache);
         }
 
         return Map.of();
@@ -344,33 +350,6 @@ private static class LockingCacheImpl implements LockingCache {
         // All access must be guarded by "cache"
         private final Map<String, LockingCacheValue> cache = new HashMap<>();
 
-        @Override
-        public @Nullable Object put(String key, Object object) {
-            LockingCacheValue value;
-            boolean created = false;
-            synchronized (cache) {
-                value = cache.get(key);
-                if (value == null || value.removed) {
-                    created = true;
-                    value = new LockingCacheValue();
-                    value.lock.lock();
-                    cache.put(key, value);
-                }
-            }
-            Object result;
-            if (!created) {
-                value.lock.lock();
-            }
-            try {
-                result = value.object;
-                value.object = object;
-            } finally {
-                value.lock.unlock();
-            }
-
-            return result;
-        }
-
         @Override
         public @Nullable Object lockAndPut(String key, Object object) {
             LockingCacheValue value;
@@ -453,13 +432,13 @@ public Object lockAndGet(String key, Supplier<Object> supplier) {
         }
 
         @Override
-        public void unlock(String key) {
+        public @Nullable Object unlock(String key) {
             LockingCacheValue value;
             synchronized (cache) {
                 value = cache.get(key);
             }
             if (value == null) {
-                return;
+                return null;
             }
             if (value.removed) {
                 unlockAll(value);
@@ -470,6 +449,18 @@ public void unlock(String key) {
                     // Ignore
                 }
             }
+            if (value.object instanceof Cloneable c) {
+                try {
+                    Method method = c.getClass().getMethod("clone");
+                    if (method.canAccess(c)) {
+                        return method.invoke(c);
+                    }
+                } catch (NoSuchMethodException | SecurityException | IllegalAccessException
+                        | InvocationTargetException e) {
+                    // Ignore, just return null
+                }
+            }
+            return null;
         }
 
         private void unlockAll(LockingCacheValue value) {

bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/shared/LockingCache.java

@@ -29,15 +29,16 @@
 public interface LockingCache {// TODO: (Nad) Redo JavaDocs
 
     /**
-     * Add a new key-value-pair to the cache. If the key is already present, the old value is replaces by the new value.
+     * Add a new key-value-pair to the cache or replace an existing value.
+     * <p>
+     * When returning,a lock is held on the object, which <b>must</b> be released with a subsequent call
+     * to {@link #unlock(String)} with the same key. Failure to do so will result in a deadlock if this
+     * entry is attempted accessed in the future.
      *
-     * @param key a {@link String} used as key
-     * @param object the {@code Object} to store with the key
-     * @return the old object associated with this key or {@code null} if the key didn't exist
+     * @param key the {@link String} used as a key.
+     * @param object the object to store with the key
+     * @return The old object associated with the key or {@code null}.
      */
-    @Nullable
-    Object put(String key, Object object);
-
     @Nullable
     Object lockAndPut(String key, Object object);
 
@@ -51,22 +52,53 @@ public interface LockingCache {// TODO: (Nad) Redo JavaDocs
     Object remove(String key);
 
     /**
-     * Get a object from the cache
+     * Get an object from the cache.
+     * <p>
+     * When returning,a lock is held on the object, which <b>must</b> be released with a subsequent call
+     * to {@link #unlock(String)} with the same key. Failure to do so will result in a deadlock if this
+     * entry is attempted accessed in the future.
      *
-     * @param key the key of the requested object
-     * @return the object associated with the key or {@code null}.
+     * @param key the {@link String} used as a key.
+     * @return The object associated with the key or {@code null}.
      */
     @Nullable
     Object lockAndGet(String key);
 
     /**
-     * Get a serialized object from the cache or create a new key-value-pair from the given supplier
+     * Get an object from the cache or create a one for the key with the given supplier.
+     * <p>
+     * When returning,a lock is held on the object, which <b>must</b> be released with a subsequent call
+     * to {@link #unlock(String)} with the same key. Failure to do so will result in a deadlock if this
+     * entry is attempted accessed in the future.
      *
-     * @param key the key of the requested serialized object
-     * @param supplier a supplier that returns a non-null serialized object to be used if the key isn't present
-     * @return the serialized object associated with the key
+     * @param key the {@link String} used as a key.
+     * @param supplier the supplier that returns a non-{@code null} object to be used if the key isn't present.
+     * @return The object associated with the key.
      */
     Object lockAndGet(String key, Supplier<Object> supplier);
 
-    void unlock(String key);
+    /**
+     * Unlock the value for the specified key and release the local reference to the cached instance.
+     * <p>
+     * To make sure that no reference is held to the cached instance, this method should always be used
+     * like this:
+     * 
+     * <pre>
+     * <code>
+     * object = cache.lockAndGet("key");
+     * try {
+     *   ..
+     *   (do something with the object)
+     *   ..
+     * } finally {
+     *   object = cache.release("key");
+     * }
+     * </code>
+     * </pre>
+     *
+     * @param key
+     * @return
+     */
+    @Nullable
+    Object unlock(String key);
 }