LocalStore uses a SparseArray to track target ids, not a set (#65)

* LocalStore uses a SparseArray to track target ids, not a set, so use that instead of a set in lru code.
* Add android robolectric to memory version of the lru tests

Author: Greg Soltis

firebase-firestore/src/main/java/com/google/firebase/firestore/local/LruDelegate.java

@@ -14,8 +14,8 @@
 
 package com.google.firebase.firestore.local;
 
+import android.util.SparseArray;
 import com.google.firebase.firestore.util.Consumer;
-import java.util.Set;
 
 /**
  * Persistence layers intending to use LRU Garbage collection should implement this interface. This
@@ -37,7 +37,7 @@ interface LruDelegate {
    *
    * @return the number of targets removed.
    */
-  int removeTargets(long upperBound, Set<Integer> activeTargetIds);
+  int removeTargets(long upperBound, SparseArray<?> activeTargetIds);
 
   /**
    * Removes all unreferenced documents from the cache that have a sequence number less than or

firebase-firestore/src/main/java/com/google/firebase/firestore/local/LruGarbageCollector.java

@@ -14,10 +14,10 @@
 
 package com.google.firebase.firestore.local;
 
+import android.util.SparseArray;
 import com.google.firebase.firestore.core.ListenSequence;
 import java.util.Comparator;
 import java.util.PriorityQueue;
-import java.util.Set;
 
 /** Implements the steps for LRU garbage collection. */
 class LruGarbageCollector {
@@ -80,7 +80,7 @@ long nthSequenceNumber(int count) {
    * Removes targets with a sequence number equal to or less than the given upper bound, and removes
    * document associations with those targets.
    */
-  int removeTargets(long upperBound, Set<Integer> activeTargetIds) {
+  int removeTargets(long upperBound, SparseArray<?> activeTargetIds) {
     return delegate.removeTargets(upperBound, activeTargetIds);
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryLruReferenceDelegate.java

@@ -16,13 +16,13 @@
 
 import static com.google.firebase.firestore.util.Assert.hardAssert;
 
+import android.util.SparseArray;
 import com.google.firebase.firestore.core.ListenSequence;
 import com.google.firebase.firestore.model.DocumentKey;
 import com.google.firebase.firestore.model.MaybeDocument;
 import com.google.firebase.firestore.util.Consumer;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Set;
 
 /** Provides LRU garbage collection functionality for MemoryPersistence. */
 class MemoryLruReferenceDelegate implements ReferenceDelegate, LruDelegate {
@@ -94,7 +94,7 @@ public void setInMemoryPins(ReferenceSet inMemoryPins) {
   }
 
   @Override
-  public int removeTargets(long upperBound, Set<Integer> activeTargetIds) {
+  public int removeTargets(long upperBound, SparseArray<?> activeTargetIds) {
     return persistence.getQueryCache().removeQueries(upperBound, activeTargetIds);
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryQueryCache.java

@@ -14,6 +14,7 @@
 
 package com.google.firebase.firestore.local;
 
+import android.util.SparseArray;
 import com.google.firebase.database.collection.ImmutableSortedSet;
 import com.google.firebase.firestore.core.Query;
 import com.google.firebase.firestore.model.DocumentKey;
@@ -22,7 +23,6 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
-import java.util.Set;
 import javax.annotation.Nullable;
 
 /**
@@ -115,13 +115,13 @@ public void removeQueryData(QueryData queryData) {
    *
    * @return the number of targets removed
    */
-  int removeQueries(long upperBound, Set<Integer> activeTargetIds) {
+  int removeQueries(long upperBound, SparseArray<?> activeTargetIds) {
     int removed = 0;
     for (Iterator<Map.Entry<Query, QueryData>> it = queries.entrySet().iterator(); it.hasNext(); ) {
       Map.Entry<Query, QueryData> entry = it.next();
       int targetId = entry.getValue().getTargetId();
       long sequenceNumber = entry.getValue().getSequenceNumber();
-      if (sequenceNumber <= upperBound && !activeTargetIds.contains(targetId)) {
+      if (sequenceNumber <= upperBound && activeTargetIds.get(targetId) == null) {
         it.remove();
         removeMatchingKeysForTargetId(targetId);
         removed++;

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteLruReferenceDelegate.java

@@ -16,11 +16,11 @@
 
 import static com.google.firebase.firestore.util.Assert.hardAssert;
 
+import android.util.SparseArray;
 import com.google.firebase.firestore.core.ListenSequence;
 import com.google.firebase.firestore.model.DocumentKey;
 import com.google.firebase.firestore.model.ResourcePath;
 import com.google.firebase.firestore.util.Consumer;
-import java.util.Set;
 
 /** Provides LRU functionality for SQLite persistence. */
 class SQLiteLruReferenceDelegate implements ReferenceDelegate, LruDelegate {
@@ -103,7 +103,7 @@ public void removeReference(DocumentKey key) {
   }
 
   @Override
-  public int removeTargets(long upperBound, Set<Integer> activeTargetIds) {
+  public int removeTargets(long upperBound, SparseArray<?> activeTargetIds) {
     return persistence.getQueryCache().removeQueries(upperBound, activeTargetIds);
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteQueryCache.java

@@ -18,6 +18,7 @@
 import static com.google.firebase.firestore.util.Assert.hardAssert;
 
 import android.database.sqlite.SQLiteStatement;
+import android.util.SparseArray;
 import com.google.firebase.Timestamp;
 import com.google.firebase.database.collection.ImmutableSortedSet;
 import com.google.firebase.firestore.core.Query;
@@ -26,7 +27,6 @@
 import com.google.firebase.firestore.proto.Target;
 import com.google.firebase.firestore.util.Consumer;
 import com.google.protobuf.InvalidProtocolBufferException;
-import java.util.Set;
 import javax.annotation.Nullable;
 
 /** Cached Queries backed by SQLite. */
@@ -186,7 +186,7 @@ public void removeQueryData(QueryData queryData) {
    * present in `activeTargetIds`. Document associations for the removed targets are also removed.
    * Returns the number of targets removed.
    */
-  int removeQueries(long upperBound, Set<Integer> activeTargetIds) {
+  int removeQueries(long upperBound, SparseArray<?> activeTargetIds) {
     int[] count = new int[1];
     // SQLite has a max sql statement size, so there is technically a possibility that including a
     // an IN clause in this query to filter `activeTargetIds` could overflow. Rather than deal with
@@ -196,7 +196,7 @@ int removeQueries(long upperBound, Set<Integer> activeTargetIds) {
         .forEach(
             row -> {
               int targetId = row.getInt(0);
-              if (!activeTargetIds.contains(targetId)) {
+              if (activeTargetIds.get(targetId) == null) {
                 removeTarget(targetId);
                 count[0]++;
               }

firebase-firestore/src/test/java/com/google/firebase/firestore/local/LruGarbageCollectorTestCase.java

@@ -26,6 +26,7 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import android.util.SparseArray;
 import com.google.firebase.Timestamp;
 import com.google.firebase.firestore.auth.User;
 import com.google.firebase.firestore.core.ListenSequence;
@@ -162,7 +163,7 @@ private void removeDocumentFromTarget(DocumentKey key, int targetId) {
     queryCache.removeMatchingKeys(keySet(key), targetId);
   }
 
-  private int removeTargets(long upperBound, Set<Integer> activeTargetIds) {
+  private int removeTargets(long upperBound, SparseArray<?> activeTargetIds) {
     return persistence.runTransaction(
         "Remove queries", () -> garbageCollector.removeTargets(upperBound, activeTargetIds));
   }
@@ -283,7 +284,7 @@ public void testSequenceNumbersWithMutationsInQueries() {
 
   @Test
   public void testRemoveQueriesUpThroughSequenceNumber() {
-    Map<Integer, QueryData> activeTargetIds = new HashMap<>();
+    SparseArray<QueryData> activeTargetIds = new SparseArray<>();
     for (int i = 0; i < 100; i++) {
       QueryData queryData = addNextQuery();
       // Mark odd queries as live so we can test filtering out live queries.
@@ -295,7 +296,7 @@ public void testRemoveQueriesUpThroughSequenceNumber() {
     // GC up through 20th query, which is 20%.
     // Expect to have GC'd 10 targets, since every other target is live
     long upperBound = 20 + initialSequenceNumber;
-    int removed = removeTargets(upperBound, activeTargetIds.keySet());
+    int removed = removeTargets(upperBound, activeTargetIds);
     assertEquals(10, removed);
     // Make sure we removed the even targets with targetID <= 20.
     persistence.runTransaction(
@@ -567,8 +568,8 @@ public void testRemoveTargetsThenGC() {
         });
 
     // Finally, do the garbage collection, up to but not including the removal of middleTarget
-    Set<Integer> activeTargetIds = new HashSet<>();
-    activeTargetIds.add(oldestTarget.getTargetId());
+    SparseArray<QueryData> activeTargetIds = new SparseArray<>();
+    activeTargetIds.put(oldestTarget.getTargetId(), oldestTarget);
     int targetsRemoved = garbageCollector.removeTargets(upperBound, activeTargetIds);
     // Expect to remove newest target
     assertEquals(1, targetsRemoved);

firebase-firestore/src/test/java/com/google/firebase/firestore/local/MemoryLruGarbageCollectorTest.java

@@ -14,6 +14,12 @@
 
 package com.google.firebase.firestore.local;
 
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+
+@RunWith(RobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
 public class MemoryLruGarbageCollectorTest extends LruGarbageCollectorTestCase {
   @Override
   Persistence createPersistence() {