resolve comments round 2

Author: Brian Chen

firebase-firestore/src/main/java/com/google/firebase/firestore/core/FirestoreClient.java

@@ -76,7 +76,7 @@ public final class FirestoreClient {
   // LRU-related
   @Nullable private Scheduler gcScheduler;
 
-  @Nullable private Scheduler indexScheduler;
+  @Nullable private Scheduler indexBackfillScheduler;
 
   public FirestoreClient(
       final Context context,
@@ -146,8 +146,8 @@ public Task<Void> terminate() {
             gcScheduler.stop();
           }
 
-          if (indexScheduler != null && Persistence.INDEXING_SUPPORT_ENABLED) {
-            indexScheduler.stop();
+          if (indexBackfillScheduler != null) {
+            indexBackfillScheduler.stop();
           }
         });
   }
@@ -262,7 +262,6 @@ private void initialize(Context context, User user, FirebaseFirestoreSettings se
     provider.initialize(configuration);
     persistence = provider.getPersistence();
     gcScheduler = provider.getGarbageCollectionScheduler();
-    indexScheduler = provider.getIndexBackfillScheduler();
     localStore = provider.getLocalStore();
     remoteStore = provider.getRemoteStore();
     syncEngine = provider.getSyncEngine();
@@ -272,8 +271,10 @@ private void initialize(Context context, User user, FirebaseFirestoreSettings se
       gcScheduler.start();
     }
 
-    if (indexScheduler != null && Persistence.INDEXING_SUPPORT_ENABLED) {
-      indexScheduler.start();
+    if (Persistence.INDEXING_SUPPORT_ENABLED && settings.isPersistenceEnabled()) {
+      indexBackfillScheduler = provider.getIndexBackfillScheduler();
+      hardAssert(indexBackfillScheduler != null, "Index backfill scheduler should not be null.");
+      indexBackfillScheduler.start();
     }
   }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/index/IndexEntry.java

@@ -18,6 +18,7 @@
  * Represents an index entry saved by the SDK in the local storage. Temporary placeholder, since
  * we'll probably serialize the indexValue right away rather than store it.
  */
+// TODO(indexing)
 public class IndexEntry {
   private final int indexId;
   private final byte[] indexValue;

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

@@ -20,9 +20,7 @@
 import com.google.firebase.firestore.util.AsyncQueue;
 import java.util.concurrent.TimeUnit;
 
-/**
- * Implements the steps for backfilling indexes.
- */
+/** Implements the steps for backfilling indexes. */
 public class IndexBackfiller {
   /** How long we wait to try running index backfill after SDK initialization. */
   private static final long INITIAL_BACKFILL_DELAY_MS = TimeUnit.SECONDS.toMillis(15);
@@ -105,14 +103,15 @@ public BackfillScheduler newScheduler(AsyncQueue asyncQueue, LocalStore localSto
     return new BackfillScheduler(asyncQueue, localStore);
   }
 
-  // TODO: Figure out which index entries to backfill.
+  // TODO(indexing): Figure out which index entries to backfill.
   public Results backfill() {
     int numIndexesWritten = 0;
     int numIndexesRemoved = 0;
     return new Results(/* hasRun= */ true, numIndexesWritten, numIndexesRemoved);
   }
 
-  public void addIndexEntry(IndexEntry entry) {
+  @VisibleForTesting
+  void addIndexEntry(IndexEntry entry) {
     persistence.execute(
         "INSERT OR IGNORE INTO index_entries ("
             + "index_id, "
@@ -125,7 +124,8 @@ public void addIndexEntry(IndexEntry entry) {
         entry.getDocumentId());
   }
 
-  public void removeIndexEntry(int indexId, String uid, String documentId) {
+  @VisibleForTesting
+  void removeIndexEntry(int indexId, String uid, String documentId) {
     persistence.execute(
         "DELETE FROM index_entries "
             + "WHERE index_id = ? "
@@ -139,7 +139,7 @@ public void removeIndexEntry(int indexId, String uid, String documentId) {
 
   @Nullable
   @VisibleForTesting
-  public IndexEntry getIndexEntry(int indexId) {
+  IndexEntry getIndexEntry(int indexId) {
     return persistence
         .query("SELECT index_value, uid, document_id FROM index_entries WHERE index_id = ?")
         .binding(indexId)

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

@@ -54,7 +54,6 @@ public static void afterClass() {
   @Before
   public void setUp() {
     persistence = PersistenceTestHelpers.createSQLitePersistence();
-    ;
     backfiller = persistence.getIndexBackfiller();
   }
 
@@ -67,15 +66,19 @@ public void tearDown() {
   public void addAndRemoveIndexEntry() {
     IndexEntry testEntry =
         new IndexEntry(1, "TEST_BLOB".getBytes(), "sample-uid", "sample-documentId");
-    backfiller.addIndexEntry(testEntry);
-    IndexEntry entry = backfiller.getIndexEntry(1);
-    assertNotNull(entry);
-    assertEquals("TEST_BLOB", new String(entry.getIndexValue()));
-    assertEquals("sample-documentId", entry.getDocumentId());
-    assertEquals("sample-uid", entry.getUid());
+    persistence.runTransaction(
+        "testAddAndRemoveIndexEntry",
+        () -> {
+          backfiller.addIndexEntry(testEntry);
+          IndexEntry entry = backfiller.getIndexEntry(1);
+          assertNotNull(entry);
+          assertEquals("TEST_BLOB", new String(entry.getIndexValue()));
+          assertEquals("sample-documentId", entry.getDocumentId());
+          assertEquals("sample-uid", entry.getUid());
 
-    backfiller.removeIndexEntry(1, "sample-uid", "sample-documentId");
-    entry = backfiller.getIndexEntry(1);
-    assertNull(entry);
+          backfiller.removeIndexEntry(1, "sample-uid", "sample-documentId");
+          entry = backfiller.getIndexEntry(1);
+          assertNull(entry);
+        });
   }
 }