From 77720bd2e64c5dcf643f95b57b2391cc1a7410a6 Mon Sep 17 00:00:00 2001
From: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com>
Date: Thu, 4 Apr 2024 09:49:45 -0700
Subject: [PATCH] Addressing first set of comments

Signed-off-by: Sagar Upadhyaya <sagar.upadhyaya.121@gmail.com>
---
 .../common/tier/TieredSpilloverCache.java     | 29 +++++++++----------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCache.java b/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCache.java
index 79629f7543fc7..0442a7d35ebf6 100644
--- a/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCache.java
+++ b/modules/cache-common/src/main/java/org/opensearch/cache/common/tier/TieredSpilloverCache.java
@@ -50,7 +50,7 @@
 public class TieredSpilloverCache<K, V> implements ICache<K, V> {
 
     // Used to avoid caching stale entries in lower tiers.
-    private static final List<RemovalReason> REMOVAL_REASONS_FOR_EVICTION = List.of(RemovalReason.EVICTED, RemovalReason.CAPACITY);
+    private static final List<RemovalReason> SPILLOVER_REMOVAL_REASONS = List.of(RemovalReason.EVICTED, RemovalReason.CAPACITY);
 
     private final ICache<K, V> diskCache;
     private final ICache<K, V> onHeapCache;
@@ -74,7 +74,7 @@ public class TieredSpilloverCache<K, V> implements ICache<K, V> {
                 @Override
                 public void onRemoval(RemovalNotification<K, V> notification) {
                     try (ReleasableLock ignore = writeLock.acquire()) {
-                        if (REMOVAL_REASONS_FOR_EVICTION.contains(notification.getRemovalReason())
+                        if (SPILLOVER_REMOVAL_REASONS.contains(notification.getRemovalReason())
                             && evaluatePolicies(notification.getValue())) {
                             diskCache.put(notification.getKey(), notification.getValue());
                         } else {
@@ -165,10 +165,11 @@ public void invalidateAll() {
      * Provides an iteration over both onHeap and disk keys. This is not protected from any mutations to the cache.
      * @return An iterable over (onHeap + disk) keys
      */
-    @SuppressWarnings({ "rawtypes", "unchecked" })
+    @SuppressWarnings({ "unchecked" })
     @Override
     public Iterable<K> keys() {
-        return new ConcatenatedIterables<K>(new Iterable[] { onHeapCache.keys(), diskCache.keys() });
+        Iterable<K>[] iterables = (Iterable<K>[]) new Iterable<?>[] { onHeapCache.keys(), diskCache.keys() };
+        return new ConcatenatedIterables<K>(iterables);
     }
 
     @Override
@@ -227,7 +228,7 @@ boolean evaluatePolicies(V value) {
      * iterator supports it.
      * @param <K> Type of key.
      */
-    class ConcatenatedIterables<K> implements Iterable<K> {
+    static class ConcatenatedIterables<K> implements Iterable<K> {
 
         final Iterable<K>[] iterables;
 
@@ -245,7 +246,7 @@ public Iterator<K> iterator() {
             return new ConcatenatedIterator<>(iterators);
         }
 
-        class ConcatenatedIterator<T> implements Iterator<T> {
+        static class ConcatenatedIterator<T> implements Iterator<T> {
             private final Iterator<T>[] iterators;
             private int currentIteratorIndex;
             private Iterator<T> currentIterator;
@@ -258,18 +259,14 @@ public ConcatenatedIterator(Iterator<T>[] iterators) {
 
             @Override
             public boolean hasNext() {
-                // Check if the current iterator has next element
-                while (currentIterator.hasNext()) {
-                    return true;
-                }
-                // If the current iterator is exhausted, switch to the next iterator
-                currentIteratorIndex++;
-                if (currentIteratorIndex < iterators.length) {
+                while (!currentIterator.hasNext()) {
+                    currentIteratorIndex++;
+                    if (currentIteratorIndex == iterators.length) {
+                        return false;
+                    }
                     currentIterator = iterators[currentIteratorIndex];
-                    // Check if the switched iterator has next element
-                    return hasNext();
                 }
-                return false;
+                return true;
             }
 
             @Override