HONEYCOMB-348: include dump params in entity key

Change-Id: I18b2ea3c897c467740f19bf346d13240aac458ac
Signed-off-by: Marek Gradzki <mgradzki@cisco.com>

5 files changed, 70 insertions, 29 deletions

diff --git a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/CacheKeyFactory.java b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/CacheKeyFactory.java
index bf4659e89..0f045e5f7 100644
--- a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/CacheKeyFactory.java
+++ b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/CacheKeyFactory.java
@@ -17,18 +17,19 @@
 package io.fd.honeycomb.translate.util.read.cache;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
 /**
  * Provides keys for provided {@code InstanceIdentifier}
  */
-public interface CacheKeyFactory {
+public interface CacheKeyFactory<U> {
 
     /**
-     * Construct key accordingly to provided {@code InstanceIdentifier<?>}
+     * Construct key accordingly to provided {@code InstanceIdentifier<?>} and dumpParams
      */
     @Nonnull
-    String createKey(@Nonnull final InstanceIdentifier<?> actualContextIdentifier);
+    String createKey(@Nonnull final InstanceIdentifier<?> actualContextIdentifier, @Nullable final U dumpParams);
 
     /**
      * Returns type of data, for which is this factory creating keys
diff --git a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/DumpCacheManager.java b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/DumpCacheManager.java
index adcd32e0c..d3cfd415a 100644
--- a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/DumpCacheManager.java
+++ b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/DumpCacheManager.java
@@ -42,7 +42,7 @@ public final class DumpCacheManager<T, U> {
 
     private final EntityDumpExecutor<T, U> dumpExecutor;
     private final EntityDumpPostProcessingFunction<T> postProcessor;
-    private final CacheKeyFactory cacheKeyFactory;
+    private final CacheKeyFactory<U> cacheKeyFactory;
     private final Class<?> acceptOnly;
 
     private DumpCacheManager(DumpCacheManagerBuilder<T, U> builder) {
@@ -66,7 +66,7 @@ public final class DumpCacheManager<T, U> {
                                @Nonnull final ModificationCache cache, final U dumpParams)
             throws ReadFailedException {
 
-        final String entityKey = this.cacheKeyFactory.createKey(identifier);
+        final String entityKey = this.cacheKeyFactory.createKey(identifier, dumpParams);
         // 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);
@@ -120,7 +120,7 @@ public final class DumpCacheManager<T, U> {
         /**
          * Key providing unique(type-aware) keys.
          */
-        public DumpCacheManagerBuilder<T, U> withCacheKeyFactory(@Nonnull final CacheKeyFactory cacheKeyFactory) {
+        public DumpCacheManagerBuilder<T, U> withCacheKeyFactory(@Nonnull final CacheKeyFactory<U> cacheKeyFactory) {
             this.cacheKeyFactory = cacheKeyFactory;
             return this;
         }
diff --git a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactory.java b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactory.java
index ba4e7e493..3cefedb95 100644
--- a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactory.java
+++ b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactory.java
@@ -20,11 +20,13 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.opendaylight.yangtools.yang.binding.InstanceIdentifier.IdentifiableItem;
 
+import com.google.common.annotations.VisibleForTesting;
 import java.util.Collections;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.Identifiable;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
@@ -32,9 +34,11 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 /**
  * Factory providing cache keys to easier switching between scopes of caching
  */
-public final class TypeAwareIdentifierCacheKeyFactory implements CacheKeyFactory {
+public final class TypeAwareIdentifierCacheKeyFactory<U> implements CacheKeyFactory<U> {
 
     private static final String KEY_PARTS_SEPARATOR = "|";
+    @VisibleForTesting
+    protected static final String NO_PARAMS_KEY = "NO_PARAMS";
 
     // should be Set<Class<? extends DataObject & Identifiable<?>>>, but that's not possible for wildcards
     private final Set<Class<? extends DataObject>> additionalKeyTypes;
@@ -78,14 +82,15 @@ public final class TypeAwareIdentifierCacheKeyFactory implements CacheKeyFactory
     }
 
     @Override
-    public String createKey(@Nonnull final InstanceIdentifier<?> actualContextIdentifier) {
+    public String createKey(@Nonnull final InstanceIdentifier<?> actualContextIdentifier, @Nullable final U dumpParams) {
 
         checkNotNull(actualContextIdentifier, "Cannot construct key for null InstanceIdentifier");
 
         // easiest case when only simple key is needed
         if (additionalKeyTypes.isEmpty()) {
             return String
-                    .join(KEY_PARTS_SEPARATOR, type.getTypeName(), actualContextIdentifier.getTargetType().toString());
+                    .join(KEY_PARTS_SEPARATOR, type.getTypeName(), params(dumpParams),
+                        actualContextIdentifier.getTargetType().toString());
         }
 
         checkArgument(isUniqueKeyConstructable(actualContextIdentifier),
@@ -95,7 +100,15 @@ public final class TypeAwareIdentifierCacheKeyFactory implements CacheKeyFactory
         // joins unique key in form : type | additional keys | actual context
         return String
                 .join(KEY_PARTS_SEPARATOR, type.getTypeName(), additionalKeys(actualContextIdentifier),
-                        actualContextIdentifier.getTargetType().toString());
+                    params(dumpParams), actualContextIdentifier.getTargetType().toString());
+    }
+
+    private String params(final U dumpParams) {
+        if (dumpParams == null) {
+            return NO_PARAMS_KEY;
+        } else {
+            return String.valueOf(dumpParams.hashCode());
+        }
     }
 
     @Override
diff --git a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/DumpCacheManagerTest.java b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/DumpCacheManagerTest.java
index 74aef67d0..dfdcd089b 100644
--- a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/DumpCacheManagerTest.java
+++ b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/DumpCacheManagerTest.java
@@ -80,7 +80,7 @@ public class DumpCacheManagerTest {
     @Test
     public void testCaching() throws ReadFailedException {
         final IpDetailsReplyDump stage1Data = new IpDetailsReplyDump();
-        final String key = cacheKeyFactory.createKey(identifier);
+        final String key = cacheKeyFactory.createKey(identifier, NO_PARAMS);
 
 
         // executor cant return null data
@@ -134,14 +134,14 @@ public class DumpCacheManagerTest {
     @Test
     public void testSameKeyDifferentTypes() throws ReadFailedException {
         final DumpCacheManager<String, Void> stringManager =
-                new DumpCacheManager.DumpCacheManagerBuilder<String, Void>()
-                        .withExecutor((InstanceIdentifier, Void) -> "value")
-                        .acceptOnly(String.class)
-                        .build();
+            new DumpCacheManager.DumpCacheManagerBuilder<String, Void>()
+                .withExecutor((InstanceIdentifier, Void) -> "value")
+                .acceptOnly(String.class)
+                .build();
 
         final DumpCacheManager<Integer, Void> intManager = new DumpCacheManager.DumpCacheManagerBuilder<Integer, Void>()
-                .acceptOnly(Integer.class)
-                .withExecutor((InstanceIdentifier, Void) -> 3).build();
+            .acceptOnly(Integer.class)
+            .withExecutor((InstanceIdentifier, Void) -> 3).build();
 
         final Optional<String> stringDump = stringManager.getDump(identifier, cache, NO_PARAMS);
         final Optional<Integer> integerDump = intManager.getDump(identifier, cache, NO_PARAMS);
@@ -153,6 +153,21 @@ public class DumpCacheManagerTest {
 
     }
 
+    @Test
+    public void testCachingWithDifferentParams() throws ReadFailedException {
+        final DumpCacheManager<Integer, Integer> manager =
+            new DumpCacheManager.DumpCacheManagerBuilder<Integer, Integer>()
+                .withExecutor((iid, param) -> param)
+                .acceptOnly(Integer.class)
+                .build();
+
+        final Optional<Integer> dump1 = manager.getDump(identifier, cache, 1);
+        final Optional<Integer> dump2 = manager.getDump(identifier, cache, 2);
+
+        assertEquals(1, dump1.get().intValue());
+        assertEquals(2, dump2.get().intValue());
+    }
+
     private EntityDumpPostProcessingFunction<IpDetailsReplyDump> createPostProcessor() {
         return ipDetailsReplyDump -> {
             IpDetailsReplyDump modified = new IpDetailsReplyDump();
diff --git a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactoryTest.java b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactoryTest.java
index 305fba143..fa247f99b 100644
--- a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactoryTest.java
+++ b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactoryTest.java
@@ -16,6 +16,8 @@
 
 package io.fd.honeycomb.translate.util.read.cache;
 
+import static io.fd.honeycomb.translate.util.read.cache.EntityDumpExecutor.NO_PARAMS;
+import static io.fd.honeycomb.translate.util.read.cache.TypeAwareIdentifierCacheKeyFactory.NO_PARAMS_KEY;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -35,8 +37,9 @@ public class TypeAwareIdentifierCacheKeyFactoryTest {
     private InstanceIdentifier<DataObjectChild> identifierBothKeyed;
     private InstanceIdentifier<DataObjectChild> identifierOneMissing;
     private InstanceIdentifier<DataObjectChild> identifierNoneKeyed;
-    private TypeAwareIdentifierCacheKeyFactory simpleKeyFactory;
-    private TypeAwareIdentifierCacheKeyFactory complexKeyFactory;
+    private TypeAwareIdentifierCacheKeyFactory<Void> simpleKeyFactory;
+    private TypeAwareIdentifierCacheKeyFactory<Void> complexKeyFactory;
+    private TypeAwareIdentifierCacheKeyFactory<Integer> complexIntegerKeyFactory;
 
     @Before
     public void init() {
@@ -49,19 +52,27 @@ public class TypeAwareIdentifierCacheKeyFactoryTest {
                 .child(DataObjectChild.class);
 
         complexKeyFactory =
-                new TypeAwareIdentifierCacheKeyFactory(String.class, ImmutableSet.of(DataObjectParent.class));
-        simpleKeyFactory = new TypeAwareIdentifierCacheKeyFactory(String.class);
+                new TypeAwareIdentifierCacheKeyFactory<>(String.class, ImmutableSet.of(DataObjectParent.class));
+        simpleKeyFactory = new TypeAwareIdentifierCacheKeyFactory<>(String.class);
+        complexIntegerKeyFactory =
+            new TypeAwareIdentifierCacheKeyFactory<>(String.class, ImmutableSet.of(DataObjectParent.class));
     }
 
     @Test
     public void createKeyBothKeyedComplex() {
-        final String key = complexKeyFactory.createKey(identifierBothKeyed);
+        final String key = complexKeyFactory.createKey(identifierBothKeyed, NO_PARAMS);
 
         /**
          * Should pass because key constructed in this case should look like :
          * additional_scope_type[additional_scope_type_key]|cached_type
          * */
-        verifyComplexKey(key);
+        verifyComplexKey(key, NO_PARAMS_KEY);
+    }
+
+    @Test
+    public void createKeyBothKeyedComplexWithParams() {
+        final String key = complexIntegerKeyFactory.createKey(identifierBothKeyed, 123);
+        verifyComplexKey(key, "123");
     }
 
     /**
@@ -69,7 +80,7 @@ public class TypeAwareIdentifierCacheKeyFactoryTest {
      */
     @Test(expected = IllegalArgumentException.class)
     public void createKeyOneMissingComplex() {
-        complexKeyFactory.createKey(identifierOneMissing);
+        complexKeyFactory.createKey(identifierOneMissing, NO_PARAMS);
     }
 
     /**
@@ -77,12 +88,12 @@ public class TypeAwareIdentifierCacheKeyFactoryTest {
      */
     @Test(expected = IllegalArgumentException.class)
     public void createKeyNoneKeyedComplex() {
-        complexKeyFactory.createKey(identifierNoneKeyed);
+        complexKeyFactory.createKey(identifierNoneKeyed, NO_PARAMS);
     }
 
     @Test
     public void createKeyBothKeyedSimple() {
-        final String key = simpleKeyFactory.createKey(identifierBothKeyed);
+        final String key = simpleKeyFactory.createKey(identifierBothKeyed, NO_PARAMS);
 
         /**
          * Should pass because key constructed in this case should look like : cached_type
@@ -92,7 +103,7 @@ public class TypeAwareIdentifierCacheKeyFactoryTest {
 
     @Test
     public void createKeyOneMissingSimple() {
-        final String key = simpleKeyFactory.createKey(identifierOneMissing);
+        final String key = simpleKeyFactory.createKey(identifierOneMissing, NO_PARAMS);
         /**
          * Should pass because key constructed in this case should look like : cached_type
          * */
@@ -104,20 +115,21 @@ public class TypeAwareIdentifierCacheKeyFactoryTest {
      */
     @Test
     public void createKeyNoneKeyedSimple() {
-        final String key = simpleKeyFactory.createKey(identifierNoneKeyed);
+        final String key = simpleKeyFactory.createKey(identifierNoneKeyed, NO_PARAMS);
         /**
          * Should pass because key constructed in this case should look like : cached_type
          * */
         verifySimpleKey(key);
     }
 
-    private void verifyComplexKey(final String key) {
+    private void verifyComplexKey(final String key, final String params) {
         assertTrue(key.contains(String.class.getTypeName()));
         assertTrue(key.contains(DataObjectParent.class.getTypeName()));
         assertTrue(key.contains(parentKey.toString()));
         assertTrue(key.contains(DataObjectChild.class.getTypeName()));
         assertFalse(key.contains(childKey.toString()));
         assertFalse(key.contains(SuperDataObject.class.getTypeName()));
+        assertTrue(key.contains(params));
     }
 
     private void verifySimpleKey(final String key) {