Refactor CustomResourceCache

* Remove the need of a lock
* clone doesn't need to handle null anymore

Author: Alexander Idelberger

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/CustomResourceCache.java

@@ -3,46 +3,38 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import io.fabric8.kubernetes.client.CustomResource;
-import java.util.Map;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
+import java.util.concurrent.ConcurrentMap;
 import java.util.function.Predicate;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
+@SuppressWarnings("rawtypes")
 public class CustomResourceCache {
 
   private static final Logger log = LoggerFactory.getLogger(CustomResourceCache.class);
 
   private final ObjectMapper objectMapper;
-  private final Map<String, CustomResource> resources = new ConcurrentHashMap<>();
-  private final Lock lock = new ReentrantLock();
+  private final ConcurrentMap<String, CustomResource> resources = new ConcurrentHashMap<>();
 
   public CustomResourceCache(ObjectMapper objectMapper) {
     this.objectMapper = objectMapper;
   }
 
   public void cacheResource(CustomResource resource) {
-    try {
-      lock.lock();
-      resources.put(KubernetesResourceUtils.getUID(resource), resource);
-    } finally {
-      lock.unlock();
-    }
+    resources.put(KubernetesResourceUtils.getUID(resource), resource);
   }
 
   public void cacheResource(CustomResource resource, Predicate<CustomResource> predicate) {
-    try {
-      lock.lock();
-      if (predicate.test(resources.get(KubernetesResourceUtils.getUID(resource)))) {
-        log.trace("Update cache after condition is true: {}", resource);
-        resources.put(resource.getMetadata().getUid(), resource);
+    resources.compute(KubernetesResourceUtils.getUID(resource), (id, prev) -> {
+      if (predicate.test(prev)) {
+        log.trace("Attempting to update cache after condition is true: {}", resource);
+        return resource;
       }
-    } finally {
-      lock.unlock();
-    }
+      return prev;
+    });
   }
 
   /**
@@ -53,24 +45,20 @@ public void cacheResource(CustomResource resource, Predicate<CustomResource> pre
    * @return
    */
   public Optional<CustomResource> getLatestResource(String uuid) {
-    return Optional.ofNullable(clone(resources.get(uuid)));
+    return Optional.ofNullable(resources.get(uuid))
+            .map(this::clone);
+  }
+
+  public CustomResource cleanup(String customResourceUid) {
+    return resources.remove(customResourceUid);
   }
 
   private CustomResource clone(CustomResource customResource) {
     try {
-      if (customResource == null) {
-        return null;
-      }
-      CustomResource clonedObject =
-          objectMapper.readValue(
+      return objectMapper.readValue(
               objectMapper.writeValueAsString(customResource), customResource.getClass());
-      return clonedObject;
     } catch (JsonProcessingException e) {
       throw new IllegalStateException(e);
     }
   }
-
-  public CustomResource cleanup(String customResourceUid) {
-    return resources.remove(customResourceUid);
-  }
 }