From c7881dfa76656f75429e39384fbf720fb0201ef5 Mon Sep 17 00:00:00 2001 From: Sagar Upadhyaya Date: Mon, 24 Jun 2024 16:44:59 -0700 Subject: [PATCH] Addressing comments to refactor unit test Signed-off-by: Sagar Upadhyaya --- .../tier/TieredSpilloverCacheTests.java | 157 +++++++++--------- 1 file changed, 76 insertions(+), 81 deletions(-) diff --git a/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java b/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java index 39eb3bc7d142c..14dfdb67f1add 100644 --- a/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java +++ b/modules/cache-common/src/test/java/org/opensearch/cache/common/tier/TieredSpilloverCacheTests.java @@ -952,50 +952,28 @@ public void testComputeIfAbsentWithOnHeapCacheThrowingExceptionOnPut() throws Ex ICache mockOnHeapCache = mock(ICache.class); when(onHeapCacheFactory.create(any(), any(), any())).thenReturn(mockOnHeapCache); doThrow(new RuntimeException("Testing")).when(mockOnHeapCache).put(any(), any()); - CacheConfig cacheConfig = new CacheConfig.Builder().setKeyType(String.class) - .setKeyType(String.class) - .setWeigher((k, v) -> keyValueSize) - .setSettings(settings) - .setDimensionNames(dimensionNames) - .setRemovalListener(removalListener) - .setKeySerializer(new StringSerializer()) - .setValueSerializer(new StringSerializer()) - .setSettings( - Settings.builder() - .put( - CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(), - TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME - ) - .put(FeatureFlags.PLUGGABLE_CACHE, "true") - .put(settings) - .build() - ) - .setClusterSettings(clusterSettings) - .build(); + CacheConfig cacheConfig = getCacheConfig(keyValueSize, settings, removalListener); ICache.Factory mockDiskCacheFactory = new MockDiskCache.MockDiskCacheFactory(0, diskCacheSize, false); - TieredSpilloverCache tieredSpilloverCache = new TieredSpilloverCache.Builder().setCacheType( - CacheType.INDICES_REQUEST_CACHE - ) - .setRemovalListener(removalListener) - .setOnHeapCacheFactory(onHeapCacheFactory) - .setDiskCacheFactory(mockDiskCacheFactory) - .setCacheConfig(cacheConfig) - .build(); + TieredSpilloverCache tieredSpilloverCache = getTieredSpilloverCache( + onHeapCacheFactory, + mockDiskCacheFactory, + cacheConfig, + null, + removalListener + ); String value = ""; - try { - value = tieredSpilloverCache.computeIfAbsent(getICacheKey("test"), new LoadAwareCacheLoader<>() { - @Override - public boolean isLoaded() { - return false; - } + value = tieredSpilloverCache.computeIfAbsent(getICacheKey("test"), new LoadAwareCacheLoader<>() { + @Override + public boolean isLoaded() { + return false; + } - @Override - public String load(ICacheKey key) { - return "test"; - } - }); - } catch (Exception ex) {} + @Override + public String load(ICacheKey key) { + return "test"; + } + }); assertEquals("test", value); assertEquals(0, tieredSpilloverCache.completableFutureMap.size()); } @@ -1015,39 +993,20 @@ public void testComputeIfAbsentWithDiskCacheThrowingExceptionOnPut() throws Exce ) .build(); ICache.Factory onHeapCacheFactory = new OpenSearchOnHeapCache.OpenSearchOnHeapCacheFactory(); - CacheConfig cacheConfig = new CacheConfig.Builder().setKeyType(String.class) - .setKeyType(String.class) - .setWeigher((k, v) -> keyValueSize) - .setSettings(settings) - .setDimensionNames(dimensionNames) - .setRemovalListener(removalListener) - .setKeySerializer(new StringSerializer()) - .setValueSerializer(new StringSerializer()) - .setSettings( - Settings.builder() - .put( - CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(), - TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME - ) - .put(FeatureFlags.PLUGGABLE_CACHE, "true") - .put(settings) - .build() - ) - .setClusterSettings(clusterSettings) - .build(); + CacheConfig cacheConfig = getCacheConfig(keyValueSize, settings, removalListener); ICache.Factory mockDiskCacheFactory = mock(MockDiskCache.MockDiskCacheFactory.class); ICache mockDiskCache = mock(ICache.class); when(mockDiskCacheFactory.create(any(), any(), any())).thenReturn(mockDiskCache); doThrow(new RuntimeException("Test")).when(mockDiskCache).put(any(), any()); - TieredSpilloverCache tieredSpilloverCache = new TieredSpilloverCache.Builder().setCacheType( - CacheType.INDICES_REQUEST_CACHE - ) - .setRemovalListener(removalListener) - .setOnHeapCacheFactory(onHeapCacheFactory) - .setDiskCacheFactory(mockDiskCacheFactory) - .setCacheConfig(cacheConfig) - .build(); + TieredSpilloverCache tieredSpilloverCache = getTieredSpilloverCache( + onHeapCacheFactory, + mockDiskCacheFactory, + cacheConfig, + null, + removalListener + ); + String value = ""; value = tieredSpilloverCache.computeIfAbsent(getICacheKey("test"), new LoadAwareCacheLoader<>() { @Override @@ -1082,7 +1041,7 @@ public String load(ICacheKey key) { return null; } }; - verifyComputeIfAbsentThrowsException(NullPointerException.class, loadAwareCacheLoader, "Loader returned a " + "null value"); + verifyComputeIfAbsentThrowsException(NullPointerException.class, loadAwareCacheLoader, "Loader returned a null value"); } public void testConcurrencyForEvictionFlowFromOnHeapToDiskTier() throws Exception { @@ -1667,6 +1626,26 @@ public boolean isLoaded() { }; } + private TieredSpilloverCache getTieredSpilloverCache( + ICache.Factory onHeapCacheFactory, + ICache.Factory mockDiskCacheFactory, + CacheConfig cacheConfig, + List> policies, + RemovalListener, String> removalListener + ) { + TieredSpilloverCache.Builder builder = new TieredSpilloverCache.Builder().setCacheType( + CacheType.INDICES_REQUEST_CACHE + ) + .setRemovalListener(removalListener) + .setOnHeapCacheFactory(onHeapCacheFactory) + .setDiskCacheFactory(mockDiskCacheFactory) + .setCacheConfig(cacheConfig); + if (policies != null) { + builder.addPolicies(policies); + } + return builder.build(); + } + private TieredSpilloverCache initializeTieredSpilloverCache( int keyValueSize, int diskCacheSize, @@ -1709,17 +1688,34 @@ private TieredSpilloverCache intializeTieredSpilloverCache( .build(); ICache.Factory mockDiskCacheFactory = new MockDiskCache.MockDiskCacheFactory(diskDeliberateDelay, diskCacheSize, false); - TieredSpilloverCache.Builder builder = new TieredSpilloverCache.Builder().setCacheType( - CacheType.INDICES_REQUEST_CACHE - ) + return getTieredSpilloverCache(onHeapCacheFactory, mockDiskCacheFactory, cacheConfig, policies, removalListener); + } + + private CacheConfig getCacheConfig( + int keyValueSize, + Settings settings, + RemovalListener, String> removalListener + ) { + return new CacheConfig.Builder().setKeyType(String.class) + .setKeyType(String.class) + .setWeigher((k, v) -> keyValueSize) + .setSettings(settings) + .setDimensionNames(dimensionNames) .setRemovalListener(removalListener) - .setOnHeapCacheFactory(onHeapCacheFactory) - .setDiskCacheFactory(mockDiskCacheFactory) - .setCacheConfig(cacheConfig); - if (policies != null) { - builder.addPolicies(policies); - } - return builder.build(); + .setKeySerializer(new StringSerializer()) + .setValueSerializer(new StringSerializer()) + .setSettings( + Settings.builder() + .put( + CacheSettings.getConcreteStoreNameSettingForCacheType(CacheType.INDICES_REQUEST_CACHE).getKey(), + TieredSpilloverCache.TieredSpilloverCacheFactory.TIERED_SPILLOVER_CACHE_NAME + ) + .put(FeatureFlags.PLUGGABLE_CACHE, "true") + .put(settings) + .build() + ) + .setClusterSettings(clusterSettings) + .build(); } // Helper functions for extracting tier aggregated stats. @@ -1760,10 +1756,9 @@ private ImmutableCacheStats getStatsSnapshotForTier(TieredSpilloverCache t return snapshot; } - @SuppressWarnings({ "rawtypes", "unchecked" }) private void verifyComputeIfAbsentThrowsException( Class expectedException, - LoadAwareCacheLoader loader, + LoadAwareCacheLoader, String> loader, String expectedExceptionMessage ) throws InterruptedException { int onHeapCacheSize = randomIntBetween(100, 300);