From 5e179dc568f24fe0c1127344f33f52963c58ab38 Mon Sep 17 00:00:00 2001 From: Sagar Upadhyaya Date: Tue, 16 Jul 2024 15:35:33 -0700 Subject: [PATCH] Adding a UT for more coverage Signed-off-by: Sagar Upadhyaya --- .../cache/store/disk/EhcacheDiskCache.java | 15 +++-- .../store/disk/EhCacheDiskCacheTests.java | 57 +++++++++++++++++++ 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java b/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java index 8875c0617d652..8270acdaf0024 100644 --- a/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java +++ b/plugins/cache-ehcache/src/main/java/org/opensearch/cache/store/disk/EhcacheDiskCache.java @@ -126,7 +126,7 @@ public class EhcacheDiskCache implements ICache { 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 @@ -135,7 +135,7 @@ public class EhcacheDiskCache implements ICache { Map, CompletableFuture, V>>> completableFutureMap = new ConcurrentHashMap<>(); @SuppressForbidden(reason = "Ehcache uses File.io") - private EhcacheDiskCache(Builder builder) { + EhcacheDiskCache(Builder 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"); @@ -162,7 +162,7 @@ private EhcacheDiskCache(Builder 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()) { @@ -189,6 +189,11 @@ private EhcacheDiskCache(Builder builder) { } } + // Package private for testing + PersistentCacheManager getCacheManager() { + return this.cacheManager; + } + @SuppressWarnings({ "rawtypes" }) private Cache buildCache(Duration expireAfterAccess, Builder builder) { // Creating the cache requires permissions specified in plugin-security.policy @@ -269,7 +274,7 @@ Map, CompletableFuture, 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) () -> { @@ -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); diff --git a/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java b/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java index 1bc30e295ab97..073bec0710ba2 100644 --- a/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java +++ b/plugins/cache-ehcache/src/test/java/org/opensearch/cache/store/disk/EhCacheDiskCacheTests.java @@ -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 { @@ -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 builder) { + super(builder); + } + + @Override + PersistentCacheManager buildCacheManager() { + PersistentCacheManager cacheManager = mock(PersistentCacheManager.class); + return cacheManager; + } + } + + private EhcacheDiskCache.Builder createDummyBuilder(String storagePath) throws IOException { + Settings settings = Settings.builder().build(); + MockRemovalListener removalListener = new MockRemovalListener<>(); + ToLongBiFunction, String> weigher = getWeigher(); + try (NodeEnvironment env = newNodeEnvironment(settings)) { + if (storagePath == null || storagePath.isBlank()) { + storagePath = env.nodePaths()[0].path.toString() + "/request_cache"; + } + return (EhcacheDiskCache.Builder) new EhcacheDiskCache.Builder().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 getRandomDimensions(List dimensionNames) { Random rand = Randomness.get(); int bound = 3;