Remove Indexing flag (#3226)

Author: schmidt-sebastian

firebase-firestore/firebase-firestore.gradle

@@ -105,13 +105,6 @@ android.libraryVariants.all { variant ->
     file("local.properties").withInputStream { localProps.load(it) }
     } catch (FileNotFoundException e) {
     }
-
-    // TODO(Indexing): Delete below once indexing is shipped.
-    if (localProps['firestoreEnableIndexing']) {
-        variant.buildConfigField("boolean", "ENABLE_INDEXING", "true")
-    } else {
-        variant.buildConfigField("boolean", "ENABLE_INDEXING", "false")
-    }
 }
 
 configurations.all {

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java

@@ -19,29 +19,15 @@
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import com.google.android.gms.tasks.Task;
 import com.google.android.gms.tasks.Tasks;
-import com.google.firebase.firestore.local.Persistence;
 import com.google.firebase.firestore.testutil.Assert;
 import com.google.firebase.firestore.testutil.IntegrationTestUtil;
 import java.util.concurrent.ExecutionException;
 import org.junit.After;
-import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
 @RunWith(AndroidJUnit4.class)
 public class IndexingTest {
-  /** Current state of indexing support. Used for restoring after test run. */
-  private static final boolean supportsIndexing = Persistence.INDEXING_SUPPORT_ENABLED;
-
-  @BeforeClass
-  public static void beforeClass() {
-    Persistence.INDEXING_SUPPORT_ENABLED = true;
-  }
-
-  @BeforeClass
-  public static void afterClass() {
-    Persistence.INDEXING_SUPPORT_ENABLED = supportsIndexing;
-  }
 
   @After
   public void tearDown() {
@@ -115,6 +101,4 @@ public void testBadIndexDoesNotCrashClient() {
                     + "  \"fieldOverrides\": []\n"
                     + "}"));
   }
-
-  // TODO(indexing): Add tests that validate that indices are active
 }

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/IntegrationTestUtil.java

@@ -38,7 +38,6 @@
 import com.google.firebase.firestore.QuerySnapshot;
 import com.google.firebase.firestore.auth.User;
 import com.google.firebase.firestore.core.DatabaseInfo;
-import com.google.firebase.firestore.local.Persistence;
 import com.google.firebase.firestore.model.DatabaseId;
 import com.google.firebase.firestore.testutil.provider.FirestoreProvider;
 import com.google.firebase.firestore.util.AsyncQueue;
@@ -255,8 +254,6 @@ public static FirebaseFirestore testFirestore(
     // This unfortunately is a global setting that affects existing Firestore clients.
     Logger.setLogLevel(logLevel);
 
-    Persistence.INDEXING_SUPPORT_ENABLED = true;
-
     Context context = ApplicationProvider.getApplicationContext();
     DatabaseId databaseId = DatabaseId.forDatabase(projectId, DatabaseId.DEFAULT_DATABASE_ID);
 

firebase-firestore/src/main/java/com/google/firebase/firestore/FirebaseFirestore.java

@@ -52,6 +52,7 @@
 import com.google.firebase.firestore.util.Function;
 import com.google.firebase.firestore.util.Logger;
 import com.google.firebase.firestore.util.Logger.Level;
+import com.google.firebase.firestore.util.Preconditions;
 import com.google.firebase.inject.Deferred;
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
@@ -306,13 +307,8 @@ public FirebaseApp getApp() {
   @VisibleForTesting
   Task<Void> setIndexConfiguration(String json) {
     ensureClientConfigured();
-
-    if (!settings.isPersistenceEnabled()) {
-      throw new IllegalStateException("Cannot enable indexes when persistence is disabled");
-    }
-
-    // Preconditions.checkState(BuildConfig.ENABLE_INDEXING, "Indexing support is not yet
-    // available.");
+    Preconditions.checkState(
+        settings.isPersistenceEnabled(), "Cannot enable indexes when persistence is disabled");
 
     List<FieldIndex> parsedIndexes = new ArrayList<>();
 

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

@@ -40,7 +40,7 @@ public abstract class ComponentProvider {
   private RemoteStore remoteStore;
   private EventManager eventManager;
   private ConnectivityMonitor connectivityMonitor;
-  private IndexBackfiller indexBackfiller;
+  @Nullable private IndexBackfiller indexBackfiller;
   @Nullable private Scheduler garbageCollectionScheduler;
 
   /** Configuration options for the component provider. */
@@ -109,6 +109,7 @@ public Scheduler getGarbageCollectionScheduler() {
     return garbageCollectionScheduler;
   }
 
+  @Nullable
   public IndexBackfiller getIndexBackfiller() {
     return indexBackfiller;
   }

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

@@ -74,9 +74,9 @@ public final class FirestoreClient {
   private RemoteStore remoteStore;
   private SyncEngine syncEngine;
   private EventManager eventManager;
-  private IndexBackfiller indexBackfiller;
 
   // LRU-related
+  @Nullable private Scheduler indexBackfillScheduler;
   @Nullable private Scheduler gcScheduler;
 
   public FirestoreClient(
@@ -160,9 +160,8 @@ public Task<Void> terminate() {
           if (gcScheduler != null) {
             gcScheduler.stop();
           }
-
-          if (Persistence.INDEXING_SUPPORT_ENABLED) {
-            indexBackfiller.getScheduler().stop();
+          if (indexBackfillScheduler != null) {
+            indexBackfillScheduler.stop();
           }
         });
   }
@@ -282,14 +281,15 @@ private void initialize(Context context, User user, FirebaseFirestoreSettings se
     remoteStore = provider.getRemoteStore();
     syncEngine = provider.getSyncEngine();
     eventManager = provider.getEventManager();
-    indexBackfiller = provider.getIndexBackfiller();
+    IndexBackfiller indexBackfiller = provider.getIndexBackfiller();
 
     if (gcScheduler != null) {
       gcScheduler.start();
     }
 
-    if (Persistence.INDEXING_SUPPORT_ENABLED && settings.isPersistenceEnabled()) {
-      indexBackfiller.getScheduler().start();
+    if (indexBackfiller != null) {
+      indexBackfillScheduler = indexBackfiller.getScheduler();
+      indexBackfillScheduler.start();
     }
   }
 

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

@@ -42,8 +42,9 @@ protected Scheduler createGarbageCollectionScheduler(Configuration configuration
   }
 
   @Override
+  @Nullable
   protected IndexBackfiller createIndexBackfiller(Configuration configuration) {
-    return new IndexBackfiller(getPersistence(), configuration.getAsyncQueue());
+    return null;
   }
 
   @Override

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

@@ -14,6 +14,7 @@
 
 package com.google.firebase.firestore.core;
 
+import com.google.firebase.firestore.local.IndexBackfiller;
 import com.google.firebase.firestore.local.LocalSerializer;
 import com.google.firebase.firestore.local.LruDelegate;
 import com.google.firebase.firestore.local.LruGarbageCollector;
@@ -32,6 +33,11 @@ protected Scheduler createGarbageCollectionScheduler(Configuration configuration
     return gc.newScheduler(configuration.getAsyncQueue(), getLocalStore());
   }
 
+  @Override
+  protected IndexBackfiller createIndexBackfiller(Configuration configuration) {
+    return new IndexBackfiller(getPersistence(), configuration.getAsyncQueue());
+  }
+
   @Override
   protected Persistence createPersistence(Configuration configuration) {
     LocalSerializer serializer =

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

@@ -22,7 +22,7 @@
 import java.util.Arrays;
 
 /**
- * OrderedCodeWriter is a minimal-allocation implmentation of the writing behavior defined by the
+ * OrderedCodeWriter is a minimal-allocation implementation of the writing behavior defined by the
  * backend.
  */
 public class OrderedCodeWriter {

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

@@ -72,13 +72,11 @@ public Scheduler(AsyncQueue asyncQueue) {
 
     @Override
     public void start() {
-      hardAssert(Persistence.INDEXING_SUPPORT_ENABLED, "Indexing support not enabled");
       scheduleBackfill();
     }
 
     @Override
     public void stop() {
-      hardAssert(Persistence.INDEXING_SUPPORT_ENABLED, "Indexing support not enabled");
       if (backfillTask != null) {
         backfillTask.cancel();
       }

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

@@ -115,7 +115,7 @@ public final class LocalStore implements BundleCallback {
   private IndexManager indexManager;
 
   /** Manages field index backfill. */
-  private final IndexBackfiller indexBackfiller;
+  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;
@@ -152,7 +152,7 @@ public final class LocalStore implements BundleCallback {
 
   public LocalStore(
       Persistence persistence,
-      IndexBackfiller indexBackfiller,
+      @Nullable IndexBackfiller indexBackfiller,
       QueryEngine queryEngine,
       User initialUser) {
     hardAssert(
@@ -168,15 +168,18 @@ public LocalStore(
     localDocuments =
         new LocalDocumentsView(remoteDocuments, mutationQueue, documentOverlayCache, indexManager);
     this.queryEngine = queryEngine;
-    this.indexBackfiller = indexBackfiller;
     queryEngine.initialize(localDocuments, indexManager);
 
     localViewReferences = new ReferenceSet();
     persistence.getReferenceDelegate().setInMemoryPins(localViewReferences);
 
     remoteDocuments.setIndexManager(indexManager);
-    indexBackfiller.setIndexManager(indexManager);
-    indexBackfiller.setLocalDocumentsView(localDocuments);
+
+    this.indexBackfiller = indexBackfiller;
+    if (indexBackfiller != null) {
+      indexBackfiller.setIndexManager(indexManager);
+      indexBackfiller.setLocalDocumentsView(localDocuments);
+    }
 
     queryDataByTarget = new SparseArray<>();
     targetIdByTarget = new HashMap<>();
@@ -218,8 +221,10 @@ public ImmutableSortedMap<DocumentKey, Document> handleUserChange(User user) {
 
     // TODO(indexing): Add spec tests that test these components change after a user change
     remoteDocuments.setIndexManager(indexManager);
-    indexBackfiller.setIndexManager(indexManager);
-    indexBackfiller.setLocalDocumentsView(localDocuments);
+    if (indexBackfiller != null) {
+      indexBackfiller.setIndexManager(indexManager);
+      indexBackfiller.setLocalDocumentsView(localDocuments);
+    }
 
     // Union the old/new changed keys.
     ImmutableSortedSet<DocumentKey> changedKeys = DocumentKey.emptyKeySet();

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

@@ -14,7 +14,6 @@
 
 package com.google.firebase.firestore.local;
 
-import com.google.firebase.firestore.BuildConfig;
 import com.google.firebase.firestore.auth.User;
 import com.google.firebase.firestore.util.Supplier;
 
@@ -53,10 +52,6 @@ public abstract class Persistence {
   /** Constant string to indicate a data migration is required to support overlays. */
   public static String DATA_MIGRATION_BUILD_OVERLAYS = "BUILD_OVERLAYS";
 
-  /** Temporary setting for enabling indexing-specific code paths while in development. */
-  // TODO(Indexing): Remove this.
-  public static boolean INDEXING_SUPPORT_ENABLED = BuildConfig.ENABLE_INDEXING;
-
   // Local subclasses only, please.
   Persistence() {}
 

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

@@ -96,11 +96,6 @@ final class SQLiteIndexManager implements IndexManager {
 
   @Override
   public void start() {
-    if (!Persistence.INDEXING_SUPPORT_ENABLED) {
-      started = true;
-      return;
-    }
-
     Map<Integer, FieldIndex.IndexState> indexStates = new HashMap<>();
 
     // Fetch all index states if persisted for the user. These states contain per user information
@@ -228,7 +223,6 @@ public void deleteFieldIndex(FieldIndex index) {
   @Override
   public void updateIndexEntries(ImmutableSortedMap<DocumentKey, Document> documents) {
     hardAssert(started, "IndexManager not started");
-    if (!Persistence.INDEXING_SUPPORT_ENABLED) return;
 
     for (Map.Entry<DocumentKey, Document> entry : documents) {
       Collection<FieldIndex> fieldIndexes = getFieldIndexes(entry.getKey().getCollectionGroup());
@@ -509,10 +503,6 @@ private Object[] fillBounds(
   public FieldIndex getFieldIndex(Target target) {
     hardAssert(started, "IndexManager not started");
 
-    if (!Persistence.INDEXING_SUPPORT_ENABLED) {
-      return null;
-    }
-
     TargetIndexMatcher targetIndexMatcher = new TargetIndexMatcher(target);
     String collectionGroup =
         target.getCollectionGroup() != null

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

@@ -28,7 +28,6 @@
 import com.google.firebase.firestore.proto.Target;
 import com.google.firebase.firestore.util.Consumer;
 import com.google.firebase.firestore.util.Logger;
-import com.google.firebase.firestore.util.Preconditions;
 import com.google.protobuf.InvalidProtocolBufferException;
 import java.util.ArrayList;
 import java.util.List;
@@ -49,10 +48,7 @@ class SQLiteSchema {
    * The version of the schema. Increase this by one for each migration added to runMigrations
    * below.
    */
-  static final int VERSION = 15;
-
-  // TODO(indexing): Remove this constant and increment VERSION to enable indexing support
-  static final int INDEXING_SUPPORT_VERSION = VERSION + 1;
+  static final int VERSION = 16;
 
   /**
    * The batch size for data migrations.
@@ -77,11 +73,7 @@ void runSchemaUpgrades() {
   }
 
   void runSchemaUpgrades(int fromVersion) {
-    int toVersion = VERSION;
-    if (Persistence.INDEXING_SUPPORT_ENABLED) {
-      toVersion = INDEXING_SUPPORT_VERSION;
-    }
-    runSchemaUpgrades(fromVersion, toVersion);
+    runSchemaUpgrades(fromVersion, VERSION);
   }
 
   /**
@@ -183,6 +175,10 @@ void runSchemaUpgrades(int fromVersion, int toVersion) {
       ensureReadTime();
     }
 
+    if (fromVersion < 16 && toVersion >= 16) {
+      createFieldIndex();
+    }
+
     /*
      * Adding a new schema upgrade? READ THIS FIRST!
      *
@@ -196,11 +192,6 @@ void runSchemaUpgrades(int fromVersion, int toVersion) {
      *    that existing values have been properly maintained. Calculate them again, if applicable.
      */
 
-    if (fromVersion < INDEXING_SUPPORT_VERSION && toVersion >= INDEXING_SUPPORT_VERSION) {
-      Preconditions.checkState(Persistence.INDEXING_SUPPORT_ENABLED);
-      createFieldIndex();
-    }
-
     Logger.debug(
         "SQLiteSchema",
         "Migration from version %s to %s took %s milliseconds",

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

@@ -47,9 +47,7 @@
 import java.util.Map;
 import java.util.Set;
 import org.junit.After;
-import org.junit.AfterClass;
 import org.junit.Before;
-import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TestName;
@@ -60,18 +58,6 @@
 @RunWith(RobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
 public class IndexBackfillerTest {
-  /** Current state of indexing support. Used for restoring after test run. */
-  private static final boolean supportsIndexing = Persistence.INDEXING_SUPPORT_ENABLED;
-
-  @BeforeClass
-  public static void beforeClass() {
-    Persistence.INDEXING_SUPPORT_ENABLED = true;
-  }
-
-  @AfterClass
-  public static void afterClass() {
-    Persistence.INDEXING_SUPPORT_ENABLED = supportsIndexing;
-  }
 
   @Rule public TestName name = new TestName();