Skip to content

Commit

Permalink
Adding a UT for more coverage
Browse files Browse the repository at this point in the history
Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com>
  • Loading branch information
sgup432 committed Jul 16, 2024
1 parent a611976 commit 5e179dc
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public class EhcacheDiskCache<K, V> implements ICache<K, V> {

final static int MINIMUM_MAX_SIZE_IN_BYTES = 1024 * 100; // 100KB
final static String CACHE_DATA_CLEANUP_DURING_INITIALIZATION_EXCEPTION = "Failed to delete ehcache disk cache under "
+ "path: % during initialization. Please clean this up manually and restart the process";
+ "path: %s during initialization. Please clean this up manually and restart the process";

/**
* Used in computeIfAbsent to synchronize loading of a given key. This is needed as ehcache doesn't provide a
Expand All @@ -135,7 +135,7 @@ public class EhcacheDiskCache<K, V> implements ICache<K, V> {
Map<ICacheKey<K>, CompletableFuture<Tuple<ICacheKey<K>, V>>> completableFutureMap = new ConcurrentHashMap<>();

@SuppressForbidden(reason = "Ehcache uses File.io")
private EhcacheDiskCache(Builder<K, V> builder) {
EhcacheDiskCache(Builder<K, V> builder) {
this.keyType = Objects.requireNonNull(builder.keyType, "Key type shouldn't be null");
this.valueType = Objects.requireNonNull(builder.valueType, "Value type shouldn't be null");
this.expireAfterAccess = Objects.requireNonNull(builder.getExpireAfterAcess(), "ExpireAfterAccess value shouldn't " + "be null");
Expand All @@ -162,7 +162,7 @@ private EhcacheDiskCache(Builder<K, V> builder) {
logger.info("Found older disk cache data lying around during initialization under path: {}", this.storagePath);
IOUtils.rm(ehcacheDirectory);
} catch (IOException e) {
throw new OpenSearchException(String.format(CACHE_DATA_CLEANUP_DURING_INITIALIZATION_EXCEPTION, this.storagePath));
throw new OpenSearchException(String.format(CACHE_DATA_CLEANUP_DURING_INITIALIZATION_EXCEPTION, this.storagePath), e);
}
}
if (builder.threadPoolAlias == null || builder.threadPoolAlias.isBlank()) {
Expand All @@ -189,6 +189,11 @@ private EhcacheDiskCache(Builder<K, V> builder) {
}
}

// Package private for testing
PersistentCacheManager getCacheManager() {
return this.cacheManager;
}

@SuppressWarnings({ "rawtypes" })
private Cache<ICacheKey, ByteArrayWrapper> buildCache(Duration expireAfterAccess, Builder<K, V> builder) {
// Creating the cache requires permissions specified in plugin-security.policy
Expand Down Expand Up @@ -269,7 +274,7 @@ Map<ICacheKey<K>, CompletableFuture<Tuple<ICacheKey<K>, V>>> getCompletableFutur
}

@SuppressForbidden(reason = "Ehcache uses File.io")
private PersistentCacheManager buildCacheManager() {
PersistentCacheManager buildCacheManager() {
// In case we use multiple ehCaches, we can define this cache manager at a global level.
// Creating the cache manager also requires permissions specified in plugin-security.policy
return AccessController.doPrivileged((PrivilegedAction<PersistentCacheManager>) () -> {
Expand Down Expand Up @@ -473,7 +478,7 @@ public void close() {
try {
cacheManager.destroyCache(this.diskCacheAlias);
} catch (Exception e) {
logger.error(() -> new ParameterizedMessage("Exception occurred while trying to destroy cache and " + "associated data"), e);
logger.error(() -> new ParameterizedMessage("Exception occurred while trying to destroy cache and associated data"), e);
}
// Delete all the disk cache related files/data in case it is present
Path ehcacheDirectory = Paths.get(this.storagePath);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,17 @@
import java.util.concurrent.Phaser;
import java.util.function.ToLongBiFunction;

import org.ehcache.PersistentCacheManager;

import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_LISTENER_MODE_SYNC_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_MAX_SIZE_IN_BYTES_KEY;
import static org.opensearch.cache.EhcacheDiskCacheSettings.DISK_STORAGE_PATH_KEY;
import static org.opensearch.cache.store.disk.EhcacheDiskCache.MINIMUM_MAX_SIZE_IN_BYTES;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;

@ThreadLeakFilters(filters = { EhcacheThreadLeakFilter.class })
public class EhCacheDiskCacheTests extends OpenSearchSingleNodeTestCase {
Expand Down Expand Up @@ -1118,6 +1124,57 @@ public void testEhcacheWithStorageSizeZero() throws Exception {
}
}

public void testEhcacheCloseWithDestroyCacheMethodThrowingException() throws Exception {
EhcacheDiskCache ehcacheDiskCache = new MockEhcahceDiskCache(createDummyBuilder(null));
PersistentCacheManager cacheManager = ehcacheDiskCache.getCacheManager();
doNothing().when(cacheManager).removeCache(anyString());
doNothing().when(cacheManager).close();
doThrow(new RuntimeException("test")).when(cacheManager).destroyCache(anyString());
ehcacheDiskCache.close();
}

class MockEhcahceDiskCache extends EhcacheDiskCache {

public MockEhcahceDiskCache(Builder<String, String> builder) {
super(builder);
}

@Override
PersistentCacheManager buildCacheManager() {
PersistentCacheManager cacheManager = mock(PersistentCacheManager.class);
return cacheManager;
}
}

private EhcacheDiskCache.Builder<String, String> createDummyBuilder(String storagePath) throws IOException {
Settings settings = Settings.builder().build();
MockRemovalListener<String, String> removalListener = new MockRemovalListener<>();
ToLongBiFunction<ICacheKey<String>, String> weigher = getWeigher();
try (NodeEnvironment env = newNodeEnvironment(settings)) {
if (storagePath == null || storagePath.isBlank()) {
storagePath = env.nodePaths()[0].path.toString() + "/request_cache";
}
return (EhcacheDiskCache.Builder<String, String>) new EhcacheDiskCache.Builder<String, String>().setThreadPoolAlias(
"ehcacheTest"
)
.setIsEventListenerModeSync(true)
.setStoragePath(storagePath)
.setKeyType(String.class)
.setValueType(String.class)
.setKeySerializer(new StringSerializer())
.setDiskCacheAlias("test1")
.setValueSerializer(new StringSerializer())
.setDimensionNames(List.of(dimensionName))
.setCacheType(CacheType.INDICES_REQUEST_CACHE)
.setSettings(settings)
.setExpireAfterAccess(TimeValue.MAX_VALUE)
.setMaximumWeightInBytes(CACHE_SIZE_IN_BYTES)
.setRemovalListener(removalListener)
.setWeigher(weigher)
.setStatsTrackingEnabled(false);
}
}

private List<String> getRandomDimensions(List<String> dimensionNames) {
Random rand = Randomness.get();
int bound = 3;
Expand Down

0 comments on commit 5e179dc

Please sign in to comment.