diff options
| -rw-r--r-- | infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/CacheKeyFactory.java | 7 | ||||
| -rw-r--r-- | infra/translate-utils/src/main/java/io/fd/honeycomb/translate/util/read/cache/DumpCacheManager.java | 40 | ||||
| -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.java | 35 | ||||
| -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 |