Refactor IndexBackfiller constructor

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/remote/RemoteStoreTest.java

@@ -21,7 +21,6 @@
 import com.google.firebase.database.collection.ImmutableSortedSet;
 import com.google.firebase.firestore.auth.User;
 import com.google.firebase.firestore.core.OnlineState;
-import com.google.firebase.firestore.local.IndexBackfiller;
 import com.google.firebase.firestore.local.LocalStore;
 import com.google.firebase.firestore.local.MemoryPersistence;
 import com.google.firebase.firestore.local.Persistence;
@@ -79,9 +78,7 @@ public ImmutableSortedSet<DocumentKey> getRemoteKeysForTarget(int targetId) {
     QueryEngine queryEngine = new QueryEngine();
     Persistence persistence = MemoryPersistence.createEagerGcMemoryPersistence();
     persistence.start();
-    IndexBackfiller indexBackfiller = new IndexBackfiller(persistence, new AsyncQueue());
-    LocalStore localStore =
-        new LocalStore(persistence, indexBackfiller, queryEngine, User.UNAUTHENTICATED);
+    LocalStore localStore = new LocalStore(persistence, queryEngine, User.UNAUTHENTICATED);
     RemoteStore remoteStore =
         new RemoteStore(callback, localStore, datastore, testQueue, connectivityMonitor);
 

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

@@ -14,6 +14,8 @@
 
 package com.google.firebase.firestore.core;
 
+import static com.google.firebase.firestore.util.Assert.hardAssertNonNull;
+
 import android.content.Context;
 import androidx.annotation.Nullable;
 import com.google.firebase.firestore.FirebaseFirestoreSettings;
@@ -101,7 +103,7 @@ Context getContext() {
   }
 
   public Persistence getPersistence() {
-    return persistence;
+    return hardAssertNonNull(persistence, "persistence not initialized yet");
   }
 
   @Nullable
@@ -115,29 +117,37 @@ public IndexBackfiller getIndexBackfiller() {
   }
 
   public LocalStore getLocalStore() {
-    return localStore;
+    return hardAssertNonNull(localStore, "localStore not initialized yet");
   }
 
   public SyncEngine getSyncEngine() {
-    return syncEngine;
+    return hardAssertNonNull(syncEngine, "syncEngine not initialized yet");
   }
 
   public RemoteStore getRemoteStore() {
-    return remoteStore;
+    return hardAssertNonNull(remoteStore, "remoteStore not initialized yet");
   }
 
   public EventManager getEventManager() {
-    return eventManager;
+    return hardAssertNonNull(eventManager, "eventManager not initialized yet");
   }
 
   protected ConnectivityMonitor getConnectivityMonitor() {
-    return connectivityMonitor;
+    return hardAssertNonNull(connectivityMonitor, "connectivityMonitor not initialized yet");
   }
 
   public void initialize(Configuration configuration) {
+    /**
+     * The order in which components are created is important.
+     *
+     * <p>The implementation of abstract createX methods (e.g. createRemoteStore) will call the getX
+     * methods (e.g. getLocalStore). Consequently, creating components out of order will cause
+     * createX method to fail because a dependency is null.
+     *
+     * <p>To catch incorrect order, all getX methods have runtime check for null.
+     */
     persistence = createPersistence(configuration);
     persistence.start();
-    indexBackfiller = createIndexBackfiller(configuration);
     localStore = createLocalStore(configuration);
     connectivityMonitor = createConnectivityMonitor(configuration);
     remoteStore = createRemoteStore(configuration);
@@ -146,6 +156,7 @@ public void initialize(Configuration configuration) {
     localStore.start();
     remoteStore.start();
     garbageCollectionScheduler = createGarbageCollectionScheduler(configuration);
+    indexBackfiller = createIndexBackfiller(configuration);
   }
 
   protected abstract Scheduler createGarbageCollectionScheduler(Configuration configuration);

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

@@ -54,8 +54,7 @@ protected EventManager createEventManager(Configuration configuration) {
 
   @Override
   protected LocalStore createLocalStore(Configuration configuration) {
-    return new LocalStore(
-        getPersistence(), getIndexBackfiller(), new QueryEngine(), configuration.getInitialUser());
+    return new LocalStore(getPersistence(), new QueryEngine(), configuration.getInitialUser());
   }
 
   @Override

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

@@ -35,7 +35,7 @@ protected Scheduler createGarbageCollectionScheduler(Configuration configuration
 
   @Override
   protected IndexBackfiller createIndexBackfiller(Configuration configuration) {
-    return new IndexBackfiller(getPersistence(), configuration.getAsyncQueue());
+    return new IndexBackfiller(getPersistence(), configuration.getAsyncQueue(), getLocalStore());
   }
 
   @Override

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

@@ -14,10 +14,9 @@
 
 package com.google.firebase.firestore.local;
 
-import static com.google.firebase.firestore.util.Assert.hardAssert;
-
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
+import com.google.common.base.Supplier;
 import com.google.firebase.firestore.model.Document;
 import com.google.firebase.firestore.model.DocumentKey;
 import com.google.firebase.firestore.model.FieldIndex.IndexOffset;
@@ -41,21 +40,27 @@ public class IndexBackfiller {
 
   private final Scheduler scheduler;
   private final Persistence persistence;
-  private LocalDocumentsView localDocumentsView;
-  private IndexManager indexManager;
+  private final Supplier<IndexManager> indexManagerOfCurrentUser;
+  private final Supplier<LocalDocumentsView> localDocumentsViewOfCurrentUser;
   private int maxDocumentsToProcess = MAX_DOCUMENTS_TO_PROCESS;
 
-  public IndexBackfiller(Persistence persistence, AsyncQueue asyncQueue) {
-    this.persistence = persistence;
-    this.scheduler = new Scheduler(asyncQueue);
+  public IndexBackfiller(Persistence persistence, AsyncQueue asyncQueue, LocalStore localStore) {
+    this(
+        persistence,
+        asyncQueue,
+        localStore::getIndexManagerForCurrentUser,
+        localStore::getLocalDocumentsForCurrentUser);
   }
 
-  public void setLocalDocumentsView(LocalDocumentsView localDocumentsView) {
-    this.localDocumentsView = localDocumentsView;
-  }
-
-  public void setIndexManager(IndexManager indexManager) {
-    this.indexManager = indexManager;
+  public IndexBackfiller(
+      Persistence persistence,
+      AsyncQueue asyncQueue,
+      Supplier<IndexManager> indexManagerOfCurrentUser,
+      Supplier<LocalDocumentsView> localDocumentsViewOfCurrentUser) {
+    this.persistence = persistence;
+    this.scheduler = new Scheduler(asyncQueue);
+    this.indexManagerOfCurrentUser = indexManagerOfCurrentUser;
+    this.localDocumentsViewOfCurrentUser = localDocumentsViewOfCurrentUser;
   }
 
   public class Scheduler implements com.google.firebase.firestore.local.Scheduler {
@@ -97,13 +102,12 @@ public Scheduler getScheduler() {
 
   /** Runs a single backfill operation and returns the number of documents processed. */
   public int backfill() {
-    hardAssert(localDocumentsView != null, "setLocalDocumentsView() not called");
-    hardAssert(indexManager != null, "setIndexManager() not called");
     return persistence.runTransaction("Backfill Indexes", () -> this.writeIndexEntries());
   }
 
   /** Writes index entries until the cap is reached. Returns the number of documents processed. */
   private int writeIndexEntries() {
+    IndexManager indexManager = indexManagerOfCurrentUser.get();
     Set<String> processedCollectionGroups = new HashSet<>();
     int documentsRemaining = maxDocumentsToProcess;
     while (documentsRemaining > 0) {
@@ -123,6 +127,8 @@ private int writeIndexEntries() {
    */
   private int writeEntriesForCollectionGroup(
       String collectionGroup, int documentsRemainingUnderCap) {
+    IndexManager indexManager = indexManagerOfCurrentUser.get();
+    LocalDocumentsView localDocumentsView = localDocumentsViewOfCurrentUser.get();
     // Use the earliest offset of all field indexes to query the local cache.
     IndexOffset existingOffset = indexManager.getMinOffset(collectionGroup);
 

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

@@ -114,9 +114,6 @@ public final class LocalStore implements BundleCallback {
   /** Manages the list of active field and collection indices. */
   private IndexManager indexManager;
 
-  /** Manages field index backfill. */
-  private final @Nullable IndexBackfiller indexBackfiller;
-
   /** The set of all mutations that have been sent but not yet been applied to the backend. */
   private MutationQueue mutationQueue;
 
@@ -150,16 +147,11 @@ public final class LocalStore implements BundleCallback {
   /** Used to generate targetIds for queries tracked locally. */
   private final TargetIdGenerator targetIdGenerator;
 
-  public LocalStore(
-      Persistence persistence,
-      @Nullable IndexBackfiller indexBackfiller,
-      QueryEngine queryEngine,
-      User initialUser) {
+  public LocalStore(Persistence persistence, QueryEngine queryEngine, User initialUser) {
     hardAssert(
         persistence.isStarted(), "LocalStore was passed an unstarted persistence implementation");
     this.persistence = persistence;
     this.queryEngine = queryEngine;
-    this.indexBackfiller = indexBackfiller;
 
     targetCache = persistence.getTargetCache();
     bundleCache = persistence.getBundleCache();
@@ -184,10 +176,6 @@ private void initializeUserComponents(User user) {
 
     remoteDocuments.setIndexManager(indexManager);
     queryEngine.initialize(localDocuments, indexManager);
-    if (indexBackfiller != null) {
-      indexBackfiller.setIndexManager(indexManager);
-      indexBackfiller.setLocalDocumentsView(localDocuments);
-    }
   }
 
   public void start() {
@@ -204,6 +192,14 @@ private void startMutationQueue() {
     persistence.runTransaction("Start MutationQueue", () -> mutationQueue.start());
   }
 
+  public IndexManager getIndexManagerForCurrentUser() {
+    return indexManager;
+  }
+
+  public LocalDocumentsView getLocalDocumentsForCurrentUser() {
+    return localDocuments;
+  }
+
   // PORTING NOTE: no shutdown for LocalStore or persistence components on Android.
 
   public ImmutableSortedMap<DocumentKey, Document> handleUserChange(User user) {

firebase-firestore/src/main/java/com/google/firebase/firestore/util/Assert.java

@@ -32,6 +32,23 @@ public static void hardAssert(boolean condition, String messageFormat, Object...
     }
   }
 
+  /**
+   * Triggers a hard assertion. The null check is guaranteed to be checked at runtime. If the object
+   * is null an AssertionError will be thrown. The string messageFormat will be formatted with the
+   * provided args using {@link String#format(String, Object...)}.
+   *
+   * @param obj The object to be compared with null
+   * @param messageFormat The message to throw if the condition is false, formatted using {@link
+   *     String#format(String, Object...)}.
+   * @param args The args to pass to String.format
+   */
+  public static <T> T hardAssertNonNull(T obj, String messageFormat, Object... args) {
+    if (obj == null) {
+      throw fail(messageFormat, args);
+    }
+    return obj;
+  }
+
   /**
    * Throws an AssertionError with the provided message. The string messageFormat will be formatted
    * with the provided args using {@link String#format(String, Object...)}. The method returns an

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

@@ -83,9 +83,9 @@ public void setUp() {
             persistence.getMutationQueue(User.UNAUTHENTICATED, indexManager),
             documentOverlayCache,
             indexManager);
-    backfiller = new IndexBackfiller(persistence, new AsyncQueue());
-    backfiller.setIndexManager(indexManager);
-    backfiller.setLocalDocumentsView(localDocumentsView);
+    backfiller =
+        new IndexBackfiller(
+            persistence, new AsyncQueue(), () -> indexManager, () -> localDocumentsView);
   }
 
   @After

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

@@ -125,10 +125,9 @@ public void setUp() {
 
     localStorePersistence = getPersistence();
     queryEngine = new CountingQueryEngine(new QueryEngine());
-    indexBackfiller = new IndexBackfiller(localStorePersistence, new AsyncQueue());
-    localStore =
-        new LocalStore(localStorePersistence, indexBackfiller, queryEngine, User.UNAUTHENTICATED);
+    localStore = new LocalStore(localStorePersistence, queryEngine, User.UNAUTHENTICATED);
     localStore.start();
+    indexBackfiller = new IndexBackfiller(localStorePersistence, new AsyncQueue(), localStore);
   }
 
   @After

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

@@ -32,7 +32,6 @@
 import com.google.firebase.firestore.model.Document;
 import com.google.firebase.firestore.model.MutableDocument;
 import com.google.firebase.firestore.model.mutation.Mutation;
-import com.google.firebase.firestore.util.AsyncQueue;
 import java.util.Collections;
 import java.util.List;
 import org.junit.After;
@@ -53,9 +52,7 @@ public void setUp() {
     // Setup persistence to version 12, which is before Overlay.
     persistence =
         PersistenceTestHelpers.createSQLitePersistenceForVersion("test-data-migration", 12);
-    IndexBackfiller indexBackfiller = new IndexBackfiller(persistence, new AsyncQueue());
-    localStore =
-        new LocalStore(persistence, indexBackfiller, new QueryEngine(), User.UNAUTHENTICATED);
+    localStore = new LocalStore(persistence, new QueryEngine(), User.UNAUTHENTICATED);
     localStore.start();
   }
 
@@ -92,9 +89,7 @@ public void testCreateOverlayFromSet() {
     // Switch to new persistence and run migrations
     this.persistence.shutdown();
     persistence = PersistenceTestHelpers.createSQLitePersistence("test-data-migration");
-    IndexBackfiller indexBackfiller = new IndexBackfiller(persistence, new AsyncQueue());
-    localStore =
-        new LocalStore(persistence, indexBackfiller, new QueryEngine(), User.UNAUTHENTICATED);
+    localStore = new LocalStore(persistence, new QueryEngine(), User.UNAUTHENTICATED);
     localStore.start();
 
     assertEquals(
@@ -119,9 +114,7 @@ public void testSkipsIfAlreadyMigrated() {
     // Switch to new persistence and run migrations
     this.persistence.shutdown();
     persistence = PersistenceTestHelpers.createSQLitePersistence("test-data-migration");
-    IndexBackfiller indexBackfiller = new IndexBackfiller(persistence, new AsyncQueue());
-    localStore =
-        new LocalStore(persistence, indexBackfiller, new QueryEngine(), User.UNAUTHENTICATED);
+    localStore = new LocalStore(persistence, new QueryEngine(), User.UNAUTHENTICATED);
     localStore.start();
 
     assertEquals(
@@ -137,9 +130,7 @@ public void testSkipsIfAlreadyMigrated() {
     // Switch to new persistence and run migrations which should be a no-op.
     this.persistence.shutdown();
     persistence = PersistenceTestHelpers.createSQLitePersistence("test-data-migration");
-    indexBackfiller = new IndexBackfiller(persistence, new AsyncQueue());
-    localStore =
-        new LocalStore(persistence, indexBackfiller, new QueryEngine(), User.UNAUTHENTICATED);
+    localStore = new LocalStore(persistence, new QueryEngine(), User.UNAUTHENTICATED);
     localStore.start();
 
     SQLiteOverlayMigrationManager migrationManager =
@@ -159,9 +150,7 @@ public void testCreateOverlayFromDelete() {
     // Switch to new persistence and run migrations
     this.persistence.shutdown();
     persistence = PersistenceTestHelpers.createSQLitePersistence("test-data-migration");
-    IndexBackfiller indexBackfiller = new IndexBackfiller(persistence, new AsyncQueue());
-    localStore =
-        new LocalStore(persistence, indexBackfiller, new QueryEngine(), User.UNAUTHENTICATED);
+    localStore = new LocalStore(persistence, new QueryEngine(), User.UNAUTHENTICATED);
     localStore.start();
 
     assertEquals(
@@ -191,9 +180,7 @@ public void testCreateOverlayFromPatches() {
     // Switch to new persistence and run migrations
     this.persistence.shutdown();
     persistence = PersistenceTestHelpers.createSQLitePersistence("test-data-migration");
-    IndexBackfiller indexBackfiller = new IndexBackfiller(persistence, new AsyncQueue());
-    localStore =
-        new LocalStore(persistence, indexBackfiller, new QueryEngine(), User.UNAUTHENTICATED);
+    localStore = new LocalStore(persistence, new QueryEngine(), User.UNAUTHENTICATED);
     localStore.start();
 
     DocumentOverlayCache overlay = persistence.getDocumentOverlayCache(User.UNAUTHENTICATED);
@@ -218,18 +205,14 @@ public void testCreateOverlayForDifferentUsers() {
     writeMutation(setMutation("foo/bar", map("foo", "set-by-unauthenticated")));
 
     // Switch to a different user
-    IndexBackfiller indexBackfiller = new IndexBackfiller(persistence, new AsyncQueue());
-    localStore =
-        new LocalStore(persistence, indexBackfiller, new QueryEngine(), new User("another_user"));
+    localStore = new LocalStore(persistence, new QueryEngine(), new User("another_user"));
     localStore.start();
     writeMutation(setMutation("foo/bar", map("foo", "set-by-another_user")));
 
     // Switch to new persistence and run migrations
     this.persistence.shutdown();
     persistence = PersistenceTestHelpers.createSQLitePersistence("test-data-migration");
-    indexBackfiller = new IndexBackfiller(persistence, new AsyncQueue());
-    localStore =
-        new LocalStore(persistence, indexBackfiller, new QueryEngine(), User.UNAUTHENTICATED);
+    localStore = new LocalStore(persistence, new QueryEngine(), User.UNAUTHENTICATED);
     localStore.start();
 
     assertEquals(