HONEYCOMB-144 - Make dump cache manager thread-save

Modified to be thread save and generic 
to be usable in all plugins
Change-Id: I26c90e8c8aa13c07fa389d86a9784e92e9532bcd
Signed-off-by: Jan Srnicek <jsrnicek@cisco.com> 

7 files changed, 37 insertions, 44 deletions

diff --git a/infra/translate-api/src/main/java/io/fd/honeycomb/translate/ModificationCache.java b/infra/translate-api/src/main/java/io/fd/honeycomb/translate/ModificationCache.java
index dcf97439b..2419ffebe 100644
--- a/infra/translate-api/src/main/java/io/fd/honeycomb/translate/ModificationCache.java
+++ b/infra/translate-api/src/main/java/io/fd/honeycomb/translate/ModificationCache.java
@@ -16,18 +16,21 @@
 
 package io.fd.honeycomb.translate;
 
-import com.google.common.collect.Maps;
-import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import javax.annotation.concurrent.ThreadSafe;
 
 /**
- * Simple context class that provides transient storage during one or more read/write operations
+ * Simple context class that provides transient storage during one or more read/write operations.
+ * Internally Thread-save.
  */
+@ThreadSafe
 public class ModificationCache implements AutoCloseable {
 
-    protected final HashMap<Object, Object> map;
+    protected final Map<Object, Object> map;
 
     public ModificationCache() {
-        map = Maps.newHashMap();
+        map = new ConcurrentHashMap<>();
     }
 
     public Object get(final Object o) {
diff --git a/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/DumpCacheManager.java b/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/DumpCacheManager.java
index 1acd9de4b..272aa8473 100644
--- a/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/DumpCacheManager.java
+++ b/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/DumpCacheManager.java
@@ -19,12 +19,11 @@ package io.fd.honeycomb.translate.v3po.util.cache;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.base.Optional;
+import io.fd.honeycomb.translate.ModificationCache;
 import io.fd.honeycomb.translate.v3po.util.cache.exceptions.check.DumpCheckFailedException;
 import io.fd.honeycomb.translate.v3po.util.cache.exceptions.execution.DumpExecutionFailedException;
 import io.fd.honeycomb.translate.v3po.util.cache.noop.NoopDumpPostProcessingFunction;
-import io.fd.honeycomb.translate.ModificationCache;
 import javax.annotation.Nonnull;
-import org.openvpp.jvpp.dto.JVppReplyDump;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -32,7 +31,7 @@ import org.slf4j.LoggerFactory;
  * Manager responsible for returning Data object dumps<br> either from cache or by invoking specified {@link
  * EntityDumpExecutor}
  */
-public final class DumpCacheManager<T extends JVppReplyDump, U> {
+public final class DumpCacheManager<T, U> {
 
     private static final Logger LOG = LoggerFactory.getLogger(DumpCacheManager.class);
 
@@ -49,21 +48,21 @@ public final class DumpCacheManager<T extends JVppReplyDump, U> {
     /**
      * Returns {@link Optional<T>} of dump
      */
-    public Optional<T> getDump(@Nonnull String entityKey, @Nonnull ModificationCache cache)
+    public Optional<T> getDump(@Nonnull String entityKey, @Nonnull ModificationCache cache, final U dumpParams)
             throws DumpExecutionFailedException {
 
-        //this key binding to every log has its logic ,because every customizer have its own cache manager and if
-        //there is need for debugging/fixing some complex call with a lot of data,you can get lost in those logs
+        // this key binding to every log has its logic ,because every customizer have its own cache manager and if
+        // there is need for debugging/fixing some complex call with a lot of data,you can get lost in those logs
         LOG.debug("Loading dump for KEY[{}]", entityKey);
 
         T dump = (T) cache.get(entityKey);
 
         if (dump == null) {
             LOG.debug("Dump for KEY[{}] not present in cache,invoking dump executor", entityKey);
+            // binds and execute dump to be thread-save
+            dump = dumpExecutor.executeDump(dumpParams);
 
-            dump = dumpExecutor.executeDump();
-
-            //this is not a critical exception, so its only logged here
+            // this is not a critical exception, so its only logged here
             try {
                 dumpNonEmptyCheck.assertNotEmpty(dump);
             } catch (DumpCheckFailedException e) {
@@ -71,7 +70,7 @@ public final class DumpCacheManager<T extends JVppReplyDump, U> {
                 return Optional.absent();
             }
 
-            //no need to check if post processor active,if wasnt set,default no-op will be used
+            // no need to check if post processor active,if wasn't set,default no-op will be used
             LOG.debug("Post-processing dump for KEY[{}]", entityKey);
             dump = postProcessor.apply(dump);
 
@@ -83,14 +82,14 @@ public final class DumpCacheManager<T extends JVppReplyDump, U> {
         }
     }
 
-    public static final class DumpCacheManagerBuilder<T extends JVppReplyDump, U> {
+    public static final class DumpCacheManagerBuilder<T, U> {
 
         private EntityDumpExecutor<T, U> dumpExecutor;
         private EntityDumpNonEmptyCheck<T> dumpNonEmptyCheck;
         private EntityDumpPostProcessingFunction<T> postProcessingFunction;
 
         public DumpCacheManagerBuilder() {
-            //for cases when user does not set specific post-processor
+            // for cases when user does not set specific post-processor
             postProcessingFunction = new NoopDumpPostProcessingFunction<T>();
         }
 
diff --git a/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpExecutor.java b/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpExecutor.java
index b25c59a8a..8f3ae5596 100644
--- a/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpExecutor.java
+++ b/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpExecutor.java
@@ -17,25 +17,20 @@
 package io.fd.honeycomb.translate.v3po.util.cache;
 
 import io.fd.honeycomb.translate.v3po.util.cache.exceptions.execution.DumpExecutionFailedException;
-import org.openvpp.jvpp.dto.JVppReplyDump;
+import javax.annotation.concurrent.ThreadSafe;
 
 /**
- * Generic interface for classes that return dumps for Data objects
+ * Generic interface for classes that return dumps for Data objects.
+ * Must be implemented in Thread-save fashion.
  */
-public interface EntityDumpExecutor<T extends JVppReplyDump, U> {
+@ThreadSafe
+public interface EntityDumpExecutor<T, U> {
 
     /**
-     * Performs dump on {link T} entity
+     * Performs dump on {@link T} entity
      *
      * @return dump of specified {@link T} entity
      * @throws DumpExecutionFailedException when dump fails
      */
-    public T executeDump() throws DumpExecutionFailedException;
-
-    /**
-     * Bind dumping params for executor
-     */
-    default public void bindDumpParams(U params) {
-        throw new UnsupportedOperationException("You should override this method if you need to bind dumping params");
-    }
+    T executeDump(final U params) throws DumpExecutionFailedException;
 }
diff --git a/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpNonEmptyCheck.java b/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpNonEmptyCheck.java
index d64e312fe..d2216f64a 100644
--- a/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpNonEmptyCheck.java
+++ b/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpNonEmptyCheck.java
@@ -18,16 +18,14 @@ package io.fd.honeycomb.translate.v3po.util.cache;
 
 import io.fd.honeycomb.translate.v3po.util.cache.exceptions.check.DumpCheckFailedException;
 import io.fd.honeycomb.translate.v3po.util.cache.exceptions.check.i.DumpEmptyException;
-import org.openvpp.jvpp.dto.JVppReplyDump;
 
 /**
  * Generic interface for classes that verifies if dump of data object is non-empty
  */
-public interface EntityDumpNonEmptyCheck<T extends JVppReplyDump> {
+public interface EntityDumpNonEmptyCheck<T> {
 
     /**
      * Verifies if data are non-empty,if not throws {@link DumpEmptyException}
-     * @throws DumpEmptyException
      */
     public void assertNotEmpty(T data) throws DumpCheckFailedException;
 }
diff --git a/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpPostProcessingFunction.java b/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpPostProcessingFunction.java
index 2bc2446e9..ff3f42c2d 100644
--- a/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpPostProcessingFunction.java
+++ b/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/EntityDumpPostProcessingFunction.java
@@ -17,13 +17,12 @@
 package io.fd.honeycomb.translate.v3po.util.cache;
 
 import java.util.function.Function;
-import org.openvpp.jvpp.dto.JVppReplyDump;
 
 /**
  * Generic interface for class that are post-processing data dumped from vpp
  */
 @FunctionalInterface
-public interface EntityDumpPostProcessingFunction<T extends JVppReplyDump> extends Function<T, T> {
+public interface EntityDumpPostProcessingFunction<T> extends Function<T, T> {
 
 
     /**
diff --git a/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/noop/NoopDumpPostProcessingFunction.java b/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/noop/NoopDumpPostProcessingFunction.java
index 58bfe6f0b..f8eb9f2b5 100644
--- a/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/noop/NoopDumpPostProcessingFunction.java
+++ b/vpp-common/vpp-translate-utils/src/main/java/io/fd/honeycomb/translate/v3po/util/cache/noop/NoopDumpPostProcessingFunction.java
@@ -17,11 +17,10 @@
 package io.fd.honeycomb.translate.v3po.util.cache.noop;
 
 import io.fd.honeycomb.translate.v3po.util.cache.EntityDumpPostProcessingFunction;
-import org.openvpp.jvpp.dto.JVppReplyDump;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class NoopDumpPostProcessingFunction<T extends JVppReplyDump> implements EntityDumpPostProcessingFunction<T> {
+public class NoopDumpPostProcessingFunction<T> implements EntityDumpPostProcessingFunction<T> {
 
     private static final Logger LOG = LoggerFactory.getLogger(NoopDumpPostProcessingFunction.class);
 
diff --git a/vpp-common/vpp-translate-utils/src/test/java/io/fd/honeycomb/translate/v3po/util/cache/DumpCacheManagerTest.java b/vpp-common/vpp-translate-utils/src/test/java/io/fd/honeycomb/translate/v3po/util/cache/DumpCacheManagerTest.java
index 21b3646c4..456744f86 100644
--- a/vpp-common/vpp-translate-utils/src/test/java/io/fd/honeycomb/translate/v3po/util/cache/DumpCacheManagerTest.java
+++ b/vpp-common/vpp-translate-utils/src/test/java/io/fd/honeycomb/translate/v3po/util/cache/DumpCacheManagerTest.java
@@ -20,9 +20,9 @@ import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.when;
 
 import com.google.common.base.Optional;
+import io.fd.honeycomb.translate.ModificationCache;
 import io.fd.honeycomb.translate.v3po.util.cache.exceptions.check.i.DumpEmptyException;
 import io.fd.honeycomb.translate.v3po.util.cache.exceptions.execution.DumpExecutionFailedException;
-import io.fd.honeycomb.translate.ModificationCache;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mock;
@@ -73,7 +73,7 @@ public class DumpCacheManagerTest {
     public void testCaching() throws DumpExecutionFailedException {
 
 
-        Optional<IpDetailsReplyDump> stage1Optional = managerNegative.getDump(KEY, cache);
+        Optional<IpDetailsReplyDump> stage1Optional = managerNegative.getDump(KEY, cache, null);
 
         //this is first call so instance should be from executor
         assertEquals(false, stage1Optional.isPresent());
@@ -81,18 +81,18 @@ public class DumpCacheManagerTest {
 
         //rebind executor with other data
         IpDetailsReplyDump stage2LoadedDump = new IpDetailsReplyDump();
-        when(executor.executeDump()).thenReturn(stage2LoadedDump);
+        when(executor.executeDump(null)).thenReturn(stage2LoadedDump);
 
-        Optional<IpDetailsReplyDump> stage2Optional = managerPositive.getDump(KEY, cache);
+        Optional<IpDetailsReplyDump> stage2Optional = managerPositive.getDump(KEY, cache, null);
 
         assertEquals(true, stage2Optional.isPresent());
         assertEquals(stage2LoadedDump, stage2Optional.get());
 
         //rebind executor with other data
         IpDetailsReplyDump stage3LoadedDump = new IpDetailsReplyDump();
-        when(executor.executeDump()).thenReturn(stage3LoadedDump);
+        when(executor.executeDump(null)).thenReturn(stage3LoadedDump);
 
-        Optional<IpDetailsReplyDump> stage3Optional = managerPositive.getDump(KEY, cache);
+        Optional<IpDetailsReplyDump> stage3Optional = managerPositive.getDump(KEY, cache, null);
         assertEquals(true, stage3Optional.isPresent());
         //check if it returns instance cached from previous stage
         assertEquals(stage2LoadedDump, stage3Optional.get());
@@ -105,9 +105,9 @@ public class DumpCacheManagerTest {
         details.swIfIndex = 2;
         dump.ipDetails.add(details);
 
-        when(executor.executeDump()).thenReturn(dump);
+        when(executor.executeDump(null)).thenReturn(dump);
 
-        Optional<IpDetailsReplyDump> optionalDump = managerPositiveWithPostProcessing.getDump(KEY, cache);
+        Optional<IpDetailsReplyDump> optionalDump = managerPositiveWithPostProcessing.getDump(KEY, cache, null);
 
         assertEquals(true, optionalDump.isPresent());
         assertEquals(1, optionalDump.get().ipDetails.size());