summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJan Srnicek <jsrnicek@cisco.com>2016-11-24 08:47:31 +0100
committerJan Srnicek <jsrnicek@cisco.com>2016-11-24 08:47:31 +0100
commitc70fcc07dd643654f8c436c5ea4ff8d81bf51603 (patch)
tree57770000e503d59535257208a867e780dc0b8cf8
parent8128f33de85b2e839a8ce6d18812374a63b81c66 (diff)
HONEYCOMB-289 - Type-aware support for DumpCacheManager
Standard cache key factory made type-aware Added checking for type of returned data from cache Change-Id: Ie4d31a9d2b0d25c4b2f4ea66be98060f449007b6 Signed-off-by: Jan Srnicek <jsrnicek@cisco.com>
-rw-r--r--infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/CacheKeyFactory.java7
-rw-r--r--infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/DumpCacheManager.java40
-rw-r--r--infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactory.java (renamed from infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/IdentifierCacheKeyFactory.java)64
-rw-r--r--infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/DumpCacheManagerTest.java35
-rw-r--r--infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactoryTest.java (renamed from infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/IdentifierCacheKeyFactoryTest.java)44
5 files changed, 130 insertions, 60 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 1b444ba3c..bf4659e89 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
@@ -27,5 +27,12 @@ public interface CacheKeyFactory {
/**
* Construct key accordingly to provided {@code InstanceIdentifier<?>}
*/
+ @Nonnull
String createKey(@Nonnull final InstanceIdentifier<?> actualContextIdentifier);
+
+ /**
+ * Returns type of data, for which is this factory creating keys
+ */
+ @Nonnull
+ Class<?> getCachedDataType();
}
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 f1b265dec..adcd32e0c 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
@@ -17,6 +17,8 @@
package io.fd.honeycomb.translate.util.read.cache;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+import static java.util.Objects.nonNull;
import com.google.common.base.Optional;
import io.fd.honeycomb.translate.ModificationCache;
@@ -41,11 +43,13 @@ public final class DumpCacheManager<T, U> {
private final EntityDumpExecutor<T, U> dumpExecutor;
private final EntityDumpPostProcessingFunction<T> postProcessor;
private final CacheKeyFactory cacheKeyFactory;
+ private final Class<?> acceptOnly;
private DumpCacheManager(DumpCacheManagerBuilder<T, U> builder) {
this.dumpExecutor = builder.dumpExecutor;
this.postProcessor = builder.postProcessingFunction;
this.cacheKeyFactory = builder.cacheKeyFactory;
+ this.acceptOnly = builder.acceptOnly;
}
/**
@@ -79,6 +83,12 @@ public final class DumpCacheManager<T, U> {
cache.put(entityKey, dump);
return Optional.of(dump);
} else {
+ // if specified, check whether data returned from cache can be used as result of this dump manager
+ // used as a secondary check if cache does not have any data of different type stored under the same key
+ checkState(acceptOnly.isInstance(dump),
+ "This dump manager accepts only %s as data, but %s was returned from cache",
+ acceptOnly, dump.getClass());
+
LOG.debug("Cached instance of dump was found for KEY[{}]", entityKey);
return Optional.of(dump);
}
@@ -86,18 +96,14 @@ public final class DumpCacheManager<T, U> {
public static final class DumpCacheManagerBuilder<T, U> {
- private static final CacheKeyFactory DEFAULT_CACHE_KEY_FACTORY_INSTANCE = new IdentifierCacheKeyFactory();
-
private EntityDumpExecutor<T, U> dumpExecutor;
private EntityDumpPostProcessingFunction<T> postProcessingFunction;
private CacheKeyFactory cacheKeyFactory;
+ private Class<?> acceptOnly;
public DumpCacheManagerBuilder() {
// for cases when user does not set specific post-processor
postProcessingFunction = new NoopDumpPostProcessingFunction<T>();
-
- //use no additional scopes version by default
- cacheKeyFactory = DEFAULT_CACHE_KEY_FACTORY_INSTANCE;
}
public DumpCacheManagerBuilder<T, U> withExecutor(@Nonnull final EntityDumpExecutor<T, U> executor) {
@@ -111,17 +117,37 @@ public final class DumpCacheManager<T, U> {
return this;
}
+ /**
+ * Key providing unique(type-aware) keys.
+ */
public DumpCacheManagerBuilder<T, U> withCacheKeyFactory(@Nonnull final CacheKeyFactory cacheKeyFactory) {
this.cacheKeyFactory = cacheKeyFactory;
return this;
}
+ /**
+ * If modification returns object of different type that this, throw exception to prevent processing data
+ * of different type.
+ */
+ public DumpCacheManagerBuilder<T, U> acceptOnly(@Nonnull final Class<?> acceptOnly) {
+ this.acceptOnly = acceptOnly;
+ return this;
+ }
+
public DumpCacheManager<T, U> build() {
checkNotNull(dumpExecutor, "Dump executor cannot be null");
checkNotNull(postProcessingFunction,
"Dump post-processor cannot be null cannot be null, default implementation is used when not set explicitly");
- checkNotNull(cacheKeyFactory,
- "Cache key factory cannot be null, default non-extended implementation is used when not set explicitly");
+
+ if (acceptOnly != null) {
+ cacheKeyFactory = new TypeAwareIdentifierCacheKeyFactory(acceptOnly);
+ } else if (cacheKeyFactory != null) {
+ acceptOnly = cacheKeyFactory.getCachedDataType();
+ } else {
+ throw new IllegalStateException(
+ "Invalid combination - either acceptOnly type must be defined[defined=" + nonNull(acceptOnly) +
+ "], or type-aware cache key factory[defined=" + nonNull(cacheKeyFactory) + "]");
+ }
return new DumpCacheManager<>(this);
}
diff --git a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/IdentifierCacheKeyFactory.java b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactory.java
index 51f47e137..ba4e7e493 100644
--- a/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/IdentifierCacheKeyFactory.java
+++ b/infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactory.java
@@ -32,31 +32,51 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
/**
* Factory providing cache keys to easier switching between scopes of caching
*/
-public final class IdentifierCacheKeyFactory implements CacheKeyFactory {
+public final class TypeAwareIdentifierCacheKeyFactory implements CacheKeyFactory {
private static final String KEY_PARTS_SEPARATOR = "|";
- // should be Set<Class<? extends DataObject & Identificable<?>>>, but that's not possible for wildcards
+ // should be Set<Class<? extends DataObject & Identifiable<?>>>, but that's not possible for wildcards
private final Set<Class<? extends DataObject>> additionalKeyTypes;
+ // factory must be aware of type of data, to prevent creating same key for same identifier but different data
+ private final Class<?> type;
/**
* Construct simple cache key factory
*/
- public IdentifierCacheKeyFactory() {
- this(Collections.emptySet());
+ public TypeAwareIdentifierCacheKeyFactory(@Nonnull final Class<?> type) {
+ this(type, Collections.emptySet());
}
/**
* @param additionalKeyTypes Additional types from path of cached type, that are specifying scope
*/
- public IdentifierCacheKeyFactory(@Nonnull final Set<Class<? extends DataObject>> additionalKeyTypes) {
+ public TypeAwareIdentifierCacheKeyFactory(@Nonnull final Class<?> type,
+ @Nonnull final Set<Class<? extends DataObject>> additionalKeyTypes) {
// verify that all are non-null and identifiable
+ this.type = checkNotNull(type, "Type cannot be null");
this.additionalKeyTypes = checkNotNull(additionalKeyTypes, "Additional key types can't be null").stream()
- .map(IdentifierCacheKeyFactory::verifyNotNull)
- .map(IdentifierCacheKeyFactory::verifyIsIdentifiable)
+ .map(TypeAwareIdentifierCacheKeyFactory::verifyNotNull)
+ .map(TypeAwareIdentifierCacheKeyFactory::verifyIsIdentifiable)
.collect(Collectors.toSet());
}
+ private static String bindKeyString(IdentifiableItem identifiableItem) {
+ return String.format("%s[%s]", identifiableItem.getType().getTypeName(), identifiableItem.getKey());
+ }
+
+ private static Class<? extends DataObject> verifyNotNull(final Class<? extends DataObject> type) {
+ return checkNotNull(type, "Cannot use null as key");
+ }
+
+ /**
+ * Initial check if provided scope variables are identifiable aka. can be used to create unique cache key
+ */
+ private static Class<? extends DataObject> verifyIsIdentifiable(final Class<? extends DataObject> type) {
+ checkArgument(Identifiable.class.isAssignableFrom(type), "Type %s is not Identifiable", type);
+ return type;
+ }
+
@Override
public String createKey(@Nonnull final InstanceIdentifier<?> actualContextIdentifier) {
@@ -64,18 +84,25 @@ public final class IdentifierCacheKeyFactory implements CacheKeyFactory {
// easiest case when only simple key is needed
if (additionalKeyTypes.isEmpty()) {
- return actualContextIdentifier.getTargetType().toString();
+ return String
+ .join(KEY_PARTS_SEPARATOR, type.getTypeName(), actualContextIdentifier.getTargetType().toString());
}
checkArgument(isUniqueKeyConstructable(actualContextIdentifier),
"Unable to construct unique key, required key types : %s, provided paths : %s", additionalKeyTypes,
actualContextIdentifier.getPathArguments());
+ // joins unique key in form : type | additional keys | actual context
return String
- .join(KEY_PARTS_SEPARATOR, additionalKeys(actualContextIdentifier),
+ .join(KEY_PARTS_SEPARATOR, type.getTypeName(), additionalKeys(actualContextIdentifier),
actualContextIdentifier.getTargetType().toString());
}
+ @Override
+ public Class<?> getCachedDataType() {
+ return type;
+ }
+
/**
* Verifies that all requested key parts have keys
*/
@@ -94,29 +121,12 @@ public final class IdentifierCacheKeyFactory implements CacheKeyFactory {
return pathArgument instanceof IdentifiableItem;
}
-
private String additionalKeys(final InstanceIdentifier<?> actualContextIdentifier) {
return StreamSupport.stream(actualContextIdentifier.getPathArguments().spliterator(), false)
.filter(this::isAdditionalScope)
.filter(this::isIdentifiable)
.map(IdentifiableItem.class::cast)
- .map(IdentifierCacheKeyFactory::bindKeyString)
+ .map(TypeAwareIdentifierCacheKeyFactory::bindKeyString)
.collect(Collectors.joining(KEY_PARTS_SEPARATOR));
}
-
- private static String bindKeyString(IdentifiableItem identifiableItem) {
- return String.format("%s[%s]", identifiableItem.getType().getTypeName(), identifiableItem.getKey());
- }
-
- private static Class<? extends DataObject> verifyNotNull(final Class<? extends DataObject> type) {
- return checkNotNull(type, "Cannot use null as key");
- }
-
- /**
- * Initial check if provided scope variables are identifiable aka. can be used to create unique cache key
- */
- private static Class<? extends DataObject> verifyIsIdentifiable(final Class<? extends DataObject> type) {
- checkArgument(Identifiable.class.isAssignableFrom(type), "Type %s is not Identifiable", type);
- return type;
- }
}
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 3639439ce..74aef67d0 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
@@ -18,6 +18,7 @@ package io.fd.honeycomb.translate.util.read.cache;
import static io.fd.honeycomb.translate.util.read.cache.EntityDumpExecutor.NO_PARAMS;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.when;
import com.google.common.base.Optional;
@@ -35,13 +36,9 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
public class DumpCacheManagerTest {
- private interface DataObj extends DataObject {}
-
@Mock
private EntityDumpExecutor<IpDetailsReplyDump, Void> executor;
-
private InstanceIdentifier<DataObj> identifier;
-
private DumpCacheManager<IpDetailsReplyDump, Void> managerPositive;
private DumpCacheManager<IpDetailsReplyDump, Void> managerPositiveWithPostProcessing;
private DumpCacheManager<IpDetailsReplyDump, Void> managerNegative;
@@ -54,23 +51,26 @@ public class DumpCacheManagerTest {
managerPositive =
new DumpCacheManager.DumpCacheManagerBuilder<IpDetailsReplyDump, Void>()
.withExecutor(executor)
+ .acceptOnly(IpDetailsReplyDump.class)
.build();
managerPositiveWithPostProcessing =
new DumpCacheManager.DumpCacheManagerBuilder<IpDetailsReplyDump, Void>()
.withExecutor(executor)
+ .acceptOnly(IpDetailsReplyDump.class)
.withPostProcessingFunction(createPostProcessor())
.build();
managerNegative =
new DumpCacheManager.DumpCacheManagerBuilder<IpDetailsReplyDump, Void>()
.withExecutor(executor)
+ .acceptOnly(IpDetailsReplyDump.class)
.build();
cache = new ModificationCache();
identifier = InstanceIdentifier.create(DataObj.class);
//manager uses this implementation by default, so it can be used to test behaviour
- cacheKeyFactory = new IdentifierCacheKeyFactory();
+ cacheKeyFactory = new TypeAwareIdentifierCacheKeyFactory(IpDetailsReplyDump.class);
}
@@ -131,6 +131,28 @@ public class DumpCacheManagerTest {
assertEquals(7, optionalDump.get().ipDetails.get(0).swIfIndex);
}
+ @Test
+ public void testSameKeyDifferentTypes() throws ReadFailedException {
+ final DumpCacheManager<String, Void> stringManager =
+ 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();
+
+ final Optional<String> stringDump = stringManager.getDump(identifier, cache, NO_PARAMS);
+ final Optional<Integer> integerDump = intManager.getDump(identifier, cache, NO_PARAMS);
+
+ assertTrue(stringDump.isPresent());
+ assertTrue(integerDump.isPresent());
+ assertEquals("value", stringDump.get());
+ assertEquals(3, integerDump.get().intValue());
+
+ }
+
private EntityDumpPostProcessingFunction<IpDetailsReplyDump> createPostProcessor() {
return ipDetailsReplyDump -> {
IpDetailsReplyDump modified = new IpDetailsReplyDump();
@@ -146,6 +168,9 @@ public class DumpCacheManagerTest {
};
}
+ private interface DataObj extends DataObject {
+ }
+
private static final class IpDetailsReplyDump {
List<IpDetails> ipDetails = new ArrayList<>();
diff --git a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/IdentifierCacheKeyFactoryTest.java b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactoryTest.java
index e7eae552c..305fba143 100644
--- a/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/IdentifierCacheKeyFactoryTest.java
+++ b/infra/translate-utils/src/test/java/io/fd/honeycomb/translate/util/read/cache/TypeAwareIdentifierCacheKeyFactoryTest.java
@@ -28,31 +28,15 @@ import org.opendaylight.yangtools.yang.binding.Identifiable;
import org.opendaylight.yangtools.yang.binding.Identifier;
import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
-public class IdentifierCacheKeyFactoryTest {
-
- private interface SuperDataObject extends DataObject {
- }
-
- private interface DataObjectParent extends DataObject, ChildOf<SuperDataObject>, Identifiable<DataObjectParentKey> {
- }
-
- private class DataObjectParentKey implements Identifier<DataObjectParent> {
- }
-
- private interface DataObjectChild extends DataObject, ChildOf<DataObjectParent>, Identifiable<DataObjectChildKey> {
- }
-
- private class DataObjectChildKey implements Identifier<DataObjectChild> {
- }
+public class TypeAwareIdentifierCacheKeyFactoryTest {
private DataObjectParentKey parentKey;
private DataObjectChildKey childKey;
private InstanceIdentifier<DataObjectChild> identifierBothKeyed;
private InstanceIdentifier<DataObjectChild> identifierOneMissing;
private InstanceIdentifier<DataObjectChild> identifierNoneKeyed;
-
- private IdentifierCacheKeyFactory simpleKeyFactory;
- private IdentifierCacheKeyFactory complexKeyFactory;
+ private TypeAwareIdentifierCacheKeyFactory simpleKeyFactory;
+ private TypeAwareIdentifierCacheKeyFactory complexKeyFactory;
@Before
public void init() {
@@ -64,8 +48,9 @@ public class IdentifierCacheKeyFactoryTest {
identifierNoneKeyed = InstanceIdentifier.create(SuperDataObject.class).child(DataObjectParent.class)
.child(DataObjectChild.class);
- complexKeyFactory = new IdentifierCacheKeyFactory(ImmutableSet.of(DataObjectParent.class));
- simpleKeyFactory = new IdentifierCacheKeyFactory();
+ complexKeyFactory =
+ new TypeAwareIdentifierCacheKeyFactory(String.class, ImmutableSet.of(DataObjectParent.class));
+ simpleKeyFactory = new TypeAwareIdentifierCacheKeyFactory(String.class);
}
@Test
@@ -127,6 +112,7 @@ public class IdentifierCacheKeyFactoryTest {
}
private void verifyComplexKey(final String key) {
+ assertTrue(key.contains(String.class.getTypeName()));
assertTrue(key.contains(DataObjectParent.class.getTypeName()));
assertTrue(key.contains(parentKey.toString()));
assertTrue(key.contains(DataObjectChild.class.getTypeName()));
@@ -135,10 +121,26 @@ public class IdentifierCacheKeyFactoryTest {
}
private void verifySimpleKey(final String key) {
+ assertTrue(key.contains(String.class.getTypeName()));
assertFalse(key.contains(DataObjectParent.class.getTypeName()));
assertFalse(key.contains(parentKey.toString()));
assertTrue(key.contains(DataObjectChild.class.getTypeName()));
assertFalse(key.contains(childKey.toString()));
assertFalse(key.contains(SuperDataObject.class.getTypeName()));
}
+
+ private interface SuperDataObject extends DataObject {
+ }
+
+ private interface DataObjectParent extends DataObject, ChildOf<SuperDataObject>, Identifiable<DataObjectParentKey> {
+ }
+
+ private interface DataObjectChild extends DataObject, ChildOf<DataObjectParent>, Identifiable<DataObjectChildKey> {
+ }
+
+ private class DataObjectParentKey implements Identifier<DataObjectParent> {
+ }
+
+ private class DataObjectChildKey implements Identifier<DataObjectChild> {
+ }
} \ No newline at end of file