SONAR-6068 Make persisit iterators "reusable"

2 files changed, 54 insertions, 48 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/Cache.java b/sonar-batch/src/main/java/org/sonar/batch/index/Cache.java
index c98b88773e6..02efaab0f00 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/index/Cache.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/index/Cache.java
@@ -300,15 +300,7 @@ public class Cache<V> {
    * Lazy-loading values for given keys
    */
   public Iterable<V> values(Object firstKey, Object secondKey) {
-    try {
-      exchange.clear();
-      exchange.append(firstKey).append(secondKey).append(Key.BEFORE);
-      Exchange iteratorExchange = new Exchange(exchange);
-      KeyFilter filter = new KeyFilter().append(KeyFilter.simpleTerm(firstKey)).append(KeyFilter.simpleTerm(secondKey));
-      return new ValueIterable<V>(iteratorExchange, filter);
-    } catch (Exception e) {
-      throw failToGetValues(e);
-    }
+    return new ValueIterable<V>(exchange, firstKey, secondKey);
   }
 
   private IllegalStateException failToGetValues(Exception e) {
@@ -319,41 +311,22 @@ public class Cache<V> {
    * Lazy-loading values for a given key
    */
   public Iterable<V> values(Object firstKey) {
-    try {
-      exchange.clear();
-      exchange.append(firstKey).append(Key.BEFORE);
-      Exchange iteratorExchange = new Exchange(exchange);
-      KeyFilter filter = new KeyFilter().append(KeyFilter.simpleTerm(firstKey));
-      return new ValueIterable<V>(iteratorExchange, filter);
-    } catch (Exception e) {
-      throw failToGetValues(e);
-    }
+    return new ValueIterable<V>(exchange, firstKey);
   }
 
   /**
    * Lazy-loading values
    */
   public Iterable<V> values() {
-    try {
-      exchange.clear().append(Key.BEFORE);
-      Exchange iteratorExchange = new Exchange(exchange);
-      KeyFilter filter = new KeyFilter().append(KeyFilter.ALL);
-      return new ValueIterable<V>(iteratorExchange, filter);
-    } catch (Exception e) {
-      throw failToGetValues(e);
-    }
+    return new ValueIterable<V>(exchange);
   }
 
   public Iterable<Entry<V>> entries() {
-    exchange.clear().to(Key.BEFORE);
-    KeyFilter filter = new KeyFilter().append(KeyFilter.ALL);
-    return new EntryIterable<V>(new Exchange(exchange), filter);
+    return new EntryIterable<V>(exchange);
   }
 
   public Iterable<Entry<V>> entries(Object firstKey) {
-    exchange.clear().append(firstKey).append(Key.BEFORE);
-    KeyFilter filter = new KeyFilter().append(KeyFilter.simpleTerm(firstKey));
-    return new EntryIterable<V>(new Exchange(exchange), filter);
+    return new EntryIterable<V>(exchange, firstKey);
   }
 
   private void resetKey(Object key) {
@@ -383,15 +356,25 @@ public class Cache<V> {
   //
 
   private static class ValueIterable<T> implements Iterable<T> {
-    private final Iterator<T> iterator;
+    private final Exchange originExchange;
+    private final Object[] keys;
 
-    private ValueIterable(Exchange exchange, KeyFilter keyFilter) {
-      this.iterator = new ValueIterator<T>(exchange, keyFilter);
+    private ValueIterable(Exchange originExchange, Object... keys) {
+      this.originExchange = originExchange;
+      this.keys = keys;
     }
 
     @Override
     public Iterator<T> iterator() {
-      return iterator;
+      originExchange.clear();
+      KeyFilter filter = new KeyFilter();
+      for (Object key : keys) {
+        originExchange.append(key);
+        filter = filter.append(KeyFilter.simpleTerm(key));
+      }
+      originExchange.append(Key.BEFORE);
+      Exchange iteratorExchange = new Exchange(originExchange);
+      return new ValueIterator<T>(iteratorExchange, filter);
     }
   }
 
@@ -434,15 +417,25 @@ public class Cache<V> {
   }
 
   private static class EntryIterable<T> implements Iterable<Entry<T>> {
-    private final EntryIterator<T> it;
+    private final Exchange originExchange;
+    private final Object[] keys;
 
-    private EntryIterable(Exchange exchange, KeyFilter keyFilter) {
-      it = new EntryIterator<T>(exchange, keyFilter);
+    private EntryIterable(Exchange originExchange, Object... keys) {
+      this.originExchange = originExchange;
+      this.keys = keys;
     }
 
     @Override
     public Iterator<Entry<T>> iterator() {
-      return it;
+      originExchange.clear();
+      KeyFilter filter = new KeyFilter();
+      for (Object key : keys) {
+        originExchange.append(key);
+        filter = filter.append(KeyFilter.simpleTerm(key));
+      }
+      originExchange.append(Key.BEFORE);
+      Exchange iteratorExchange = new Exchange(originExchange);
+      return new EntryIterator<T>(iteratorExchange, filter);
     }
   }
 
diff --git a/sonar-batch/src/test/java/org/sonar/batch/index/CacheTest.java b/sonar-batch/src/test/java/org/sonar/batch/index/CacheTest.java
index 9bae728f375..0235af3dc89 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/index/CacheTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/index/CacheTest.java
@@ -25,6 +25,7 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.sonar.batch.index.Cache.Entry;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -57,11 +58,15 @@ public class CacheTest {
     assertThat(cache.get("france")).isEqualTo("paris");
     assertThat(cache.keySet()).containsOnly("france", "italy");
     assertThat(cache.keySet("france")).isEmpty();
-    assertThat(cache.values()).containsOnly("paris", "rome");
+    Iterable<String> values = cache.values();
+    assertThat(values).containsOnly("paris", "rome");
+    assertThat(values).containsOnly("paris", "rome");
     assertThat(cache.containsKey("france")).isTrue();
 
-    Cache.Entry[] entries = Iterables.toArray(cache.entries(), Cache.Entry.class);
+    Iterable<Entry<String>> iterable = cache.entries();
+    Cache.Entry[] entries = Iterables.toArray(iterable, Cache.Entry.class);
     assertThat(entries).hasSize(2);
+    assertThat(iterable).hasSize(2);
     assertThat(entries[0].key()[0]).isEqualTo("france");
     assertThat(entries[0].value()).isEqualTo("paris");
     assertThat(entries[1].key()[0]).isEqualTo("italy");
@@ -74,10 +79,10 @@ public class CacheTest {
     assertThat(cache.keySet("france")).isEmpty();
     assertThat(cache.containsKey("france")).isFalse();
     assertThat(cache.containsKey("italy")).isTrue();
-    assertThat(cache.values()).containsOnly("rome");
+    assertThat(values).containsOnly("rome");
 
     cache.clear();
-    assertThat(cache.values()).isEmpty();
+    assertThat(values).isEmpty();
   }
 
   @Test
@@ -114,8 +119,10 @@ public class CacheTest {
     assertThat(cache.values("europe")).containsOnly("paris", "rome");
     assertThat(cache.values("oceania")).isEmpty();
 
-    Cache.Entry[] allEntries = Iterables.toArray(cache.entries(), Cache.Entry.class);
+    Iterable<Entry<String>> iterable = cache.entries();
+    Cache.Entry[] allEntries = Iterables.toArray(iterable, Cache.Entry.class);
     assertThat(allEntries).hasSize(3);
+    assertThat(iterable).hasSize(3);
     assertThat(allEntries[0].key()).isEqualTo(new String[] {"asia", "china"});
     assertThat(allEntries[0].value()).isEqualTo("pekin");
     assertThat(allEntries[1].key()).isEqualTo(new String[] {"europe", "france"});
@@ -123,8 +130,10 @@ public class CacheTest {
     assertThat(allEntries[2].key()).isEqualTo(new String[] {"europe", "italy"});
     assertThat(allEntries[2].value()).isEqualTo("rome");
 
-    Cache.Entry[] subEntries = Iterables.toArray(cache.entries("europe"), Cache.Entry.class);
+    Iterable<Entry<String>> iterable2 = cache.entries("europe");
+    Cache.Entry[] subEntries = Iterables.toArray(iterable2, Cache.Entry.class);
     assertThat(subEntries).hasSize(2);
+    assertThat(iterable2).hasSize(2);
     assertThat(subEntries[0].key()).isEqualTo(new String[] {"europe", "france"});
     assertThat(subEntries[0].value()).isEqualTo("paris");
     assertThat(subEntries[1].key()).isEqualTo(new String[] {"europe", "italy"});
@@ -171,8 +180,10 @@ public class CacheTest {
     assertThat(cache.values("europe")).containsOnly("eiffel tower", "lake", "colosseum", "notre dame");
     assertThat(cache.values("europe", "france")).containsOnly("eiffel tower", "lake", "notre dame");
 
-    Cache.Entry[] allEntries = Iterables.toArray(cache.entries(), Cache.Entry.class);
+    Iterable<Entry<String>> iterable = cache.entries();
+    Cache.Entry[] allEntries = Iterables.toArray(iterable, Cache.Entry.class);
     assertThat(allEntries).hasSize(7);
+    assertThat(iterable).hasSize(7);
     assertThat(allEntries[0].key()).isEqualTo(new String[] {"america", "us", "new york"});
     assertThat(allEntries[0].value()).isEqualTo("empire state building");
     assertThat(allEntries[1].key()).isEqualTo(new String[] {"asia", "china", "pekin"});
@@ -186,8 +197,10 @@ public class CacheTest {
     assertThat(allEntries[5].key()).isEqualTo(new String[] {"europe", "italy", "rome"});
     assertThat(allEntries[5].value()).isEqualTo("colosseum");
 
-    Cache.Entry[] subEntries = Iterables.toArray(cache.entries("europe"), Cache.Entry.class);
+    Iterable<Entry<String>> iterable2 = cache.entries("europe");
+    Cache.Entry[] subEntries = Iterables.toArray(iterable2, Cache.Entry.class);
     assertThat(subEntries).hasSize(4);
+    assertThat(iterable2).hasSize(4);
     assertThat(subEntries[0].key()).isEqualTo(new String[] {"europe", "france", "annecy"});
     assertThat(subEntries[0].value()).isEqualTo("lake");
     assertThat(subEntries[1].key()).isEqualTo(new String[] {"europe", "france", "paris"});