Implement configureFieldIndexes with tests

packages/firestore/src/api/credentials.ts

@@ -473,24 +473,25 @@ export class FirebaseAppCheckTokenProvider
     asyncQueue: AsyncQueue,
     changeListener: CredentialChangeListener<string>
   ): void {
-    const onTokenChanged: (tokenResult: AppCheckTokenResult) => Promise<void> =
-      tokenResult => {
-        if (tokenResult.error != null) {
-          logDebug(
-            'FirebaseAppCheckTokenProvider',
-            `Error getting App Check token; using placeholder token instead. Error: ${tokenResult.error.message}`
-          );
-        }
-        const tokenUpdated = tokenResult.token !== this.latestAppCheckToken;
-        this.latestAppCheckToken = tokenResult.token;
+    const onTokenChanged: (
+      tokenResult: AppCheckTokenResult
+    ) => Promise<void> = tokenResult => {
+      if (tokenResult.error != null) {
         logDebug(
           'FirebaseAppCheckTokenProvider',
-          `Received ${tokenUpdated ? 'new' : 'existing'} token.`
+          `Error getting App Check token; using placeholder token instead. Error: ${tokenResult.error.message}`
         );
-        return tokenUpdated
-          ? changeListener(tokenResult.token)
-          : Promise.resolve();
-      };
+      }
+      const tokenUpdated = tokenResult.token !== this.latestAppCheckToken;
+      this.latestAppCheckToken = tokenResult.token;
+      logDebug(
+        'FirebaseAppCheckTokenProvider',
+        `Received ${tokenUpdated ? 'new' : 'existing'} token.`
+      );
+      return tokenUpdated
+        ? changeListener(tokenResult.token)
+        : Promise.resolve();
+    };
 
     this.tokenListener = (tokenResult: AppCheckTokenResult) => {
       asyncQueue.enqueueRetryable(() => onTokenChanged(tokenResult));

packages/firestore/src/api/index_configuration.ts

@@ -15,7 +15,9 @@
  * limitations under the License.
  */
 
+import { getLocalStore } from '../core/firestore_client';
 import { fieldPathFromDotSeparatedString } from '../lite-api/user_data_reader';
+import { localStoreConfigureFieldIndexes } from '../local/local_store_impl';
 import {
   FieldIndex,
   IndexKind,
@@ -145,22 +147,26 @@ export function setIndexConfiguration(
   firestore: Firestore,
   json: string
 ): Promise<void>;
-export function setIndexConfiguration(
+export async function setIndexConfiguration(
   firestore: Firestore,
   jsonOrConfiguration: string | IndexConfiguration
 ): Promise<void> {
   firestore = cast(firestore, Firestore);
-  ensureFirestoreConfigured(firestore);
+  const client = ensureFirestoreConfigured(firestore);
+
+  // PORTING NOTE: We don't return an error if the user has not enabled
+  // persistence since `enableIndexeddbPersistence()` can fail on the Web.
+  if (!client.offlineComponents?.indexBackfillerScheduler) {
+    console.warn('Cannot enable indexes when persistence is disabled');
+    return;
+  }
 
   const indexConfiguration =
     typeof jsonOrConfiguration === 'string'
       ? (tryParseJson(jsonOrConfiguration) as IndexConfiguration)
       : jsonOrConfiguration;
   const parsedIndexes: FieldIndex[] = [];
 
-  // PORTING NOTE: We don't return an error if the user has not enabled
-  // persistence since `enableIndexeddbPersistence()` can fail on the Web.
-
   if (Array.isArray(indexConfiguration.indexes)) {
     for (const index of indexConfiguration.indexes) {
       const collectionGroup = tryGetString(index, 'collectionGroup');
@@ -195,8 +201,8 @@ export function setIndexConfiguration(
     }
   }
 
-  // TODO(indexing): Configure indexes
-  return Promise.resolve();
+  const localStore = await getLocalStore(client);
+  await localStoreConfigureFieldIndexes(localStore, parsedIndexes);
 }
 
 function tryParseJson(json: string): Record<string, unknown> {
@@ -205,7 +211,7 @@ function tryParseJson(json: string): Record<string, unknown> {
   } catch (e) {
     throw new FirestoreError(
       Code.INVALID_ARGUMENT,
-      'Failed to parse JSON:' + (e as Error)?.message
+      'Failed to parse JSON: ' + (e as Error)?.message
     );
   }
 }

packages/firestore/src/local/index_backfiller.ts

@@ -25,7 +25,6 @@ import { debugAssert } from '../util/assert';
 import { AsyncQueue, DelayedOperation, TimerId } from '../util/async_queue';
 import { logDebug } from '../util/log';
 
-import { INDEXING_ENABLED } from './indexeddb_schema';
 import { ignoreIfPrimaryLeaseLoss, LocalStore } from './local_store';
 import { LocalWriteResult } from './local_store_impl';
 import { Persistence, Scheduler } from './persistence';
@@ -60,9 +59,7 @@ export class IndexBackfillerScheduler implements Scheduler {
       this.task === null,
       'Cannot start an already started IndexBackfillerScheduler'
     );
-    if (INDEXING_ENABLED) {
-      this.schedule(INITIAL_BACKFILL_DELAY_MS);
-    }
+    this.schedule(INITIAL_BACKFILL_DELAY_MS);
   }
 
   stop(): void {

packages/firestore/src/local/indexeddb_schema.ts

@@ -28,9 +28,6 @@ import {
 import { EncodedResourcePath } from './encoded_resource_path';
 import { DbTimestampKey } from './indexeddb_sentinels';
 
-// TODO(indexing): Remove this constant
-export const INDEXING_ENABLED = false;
-
 export const INDEXING_SCHEMA_VERSION = 15;
 
 /**
@@ -58,7 +55,7 @@ export const INDEXING_SCHEMA_VERSION = 15;
  * 15. Add indexing support.
  */
 
-export const SCHEMA_VERSION = INDEXING_ENABLED ? INDEXING_SCHEMA_VERSION : 14;
+export const SCHEMA_VERSION = INDEXING_SCHEMA_VERSION;
 
 /**
  * Wrapper class to store timestamps (seconds and nanos) in IndexedDb objects.

packages/firestore/src/local/local_store_impl.ts

@@ -39,6 +39,8 @@ import {
 import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import {
+  FieldIndex,
+  fieldIndexSemanticComparator,
   INITIAL_LARGEST_BATCH_ID,
   newIndexOffsetSuccessorFromReadTime
 } from '../model/field_index';
@@ -57,6 +59,7 @@ import {
 } from '../protos/firestore_bundle_proto';
 import { RemoteEvent, TargetChange } from '../remote/remote_event';
 import { fromVersion, JsonProtoSerializer } from '../remote/serializer';
+import { diffArrays } from '../util/array';
 import { debugAssert, debugCast, hardAssert } from '../util/assert';
 import { ByteString } from '../util/byte_string';
 import { logDebug } from '../util/log';
@@ -1483,3 +1486,37 @@ export async function localStoreSaveNamedQuery(
     }
   );
 }
+
+export async function localStoreConfigureFieldIndexes(
+  localStore: LocalStore,
+  newFieldIndexes: FieldIndex[]
+): Promise<void> {
+  const localStoreImpl = debugCast(localStore, LocalStoreImpl);
+  const indexManager = localStoreImpl.indexManager;
+  const promises: Array<PersistencePromise<void>> = [];
+  return localStoreImpl.persistence.runTransaction(
+    'Configure indexes',
+    'readwrite-primary',
+    transaction =>
+      indexManager
+        .getFieldIndexes(transaction)
+        .next(oldFieldIndexes =>
+          diffArrays(
+            oldFieldIndexes,
+            newFieldIndexes,
+            fieldIndexSemanticComparator,
+            fieldIndex => {
+              promises.push(
+                indexManager.addFieldIndex(transaction, fieldIndex)
+              );
+            },
+            fieldIndex => {
+              promises.push(
+                indexManager.deleteFieldIndex(transaction, fieldIndex)
+              );
+            }
+          )
+        )
+        .next(() => PersistencePromise.waitFor(promises))
+  );
+}

packages/firestore/src/local/query_engine.ts

@@ -43,7 +43,6 @@ import { Iterable } from '../util/misc';
 import { SortedSet } from '../util/sorted_set';
 
 import { IndexManager, IndexType } from './index_manager';
-import { INDEXING_ENABLED } from './indexeddb_schema';
 import { LocalDocumentsView } from './local_documents_view';
 import { PersistencePromise } from './persistence_promise';
 import { PersistenceTransaction } from './persistence_transaction';
@@ -134,10 +133,6 @@ export class QueryEngine {
     transaction: PersistenceTransaction,
     query: Query
   ): PersistencePromise<DocumentMap | null> {
-    if (!INDEXING_ENABLED) {
-      return PersistencePromise.resolve<DocumentMap | null>(null);
-    }
-
     if (queryMatchesAllDocuments(query)) {
       // Queries that match all documents don't benefit from using
       // key-based lookups. It is more efficient to scan all documents in a

packages/firestore/src/util/array.ts

@@ -55,3 +55,59 @@ export function findIndex<A>(
   }
   return null;
 }
+
+/**
+ * Compares two array for equality using comparator. The method computes the
+ * intersection and invokes `onAdd` for every element that is in `after` but not
+ * `before`. `onRemove` is invoked for every element in `before` but missing
+ * from `after`.
+ *
+ * The method creates a copy of both `before` and `after` and runs in O(n log
+ * n), where n is the size of the two lists.
+ *
+ * @param before - The elements that exist in the original array.
+ * @param after - The elements to diff against the original array.
+ * @param comparator - The comparator for the elements in before and after.
+ * @param onAdd - A function to invoke for every element that is part of `
+ * after` but not `before`.
+ * @param onRemove - A function to invoke for every element that is part of
+ * `before` but not `after`.
+ */
+export function diffArrays<T>(
+  before: T[],
+  after: T[],
+  comparator: (l: T, r: T) => number,
+  onAdd: (entry: T) => void,
+  onRemove: (entry: T) => void
+): void {
+  before = [...before];
+  after = [...after];
+  before.sort(comparator);
+  after.sort(comparator);
+
+  const bLen = before.length;
+  const aLen = after.length;
+  let a = 0;
+  let b = 0;
+  while (a < aLen && b < bLen) {
+    const cmp = comparator(before[b], after[a]);
+    if (cmp < 0) {
+      // The element was removed if the next element in our ordered
+      // walkthrough is only in `before`.
+      onRemove(before[b++]);
+    } else if (cmp > 0) {
+      // The element was added if the next element in our ordered walkthrough
+      // is only in `after`.
+      onAdd(after[a++]);
+    } else {
+      a++;
+      b++;
+    }
+  }
+  while (a < aLen) {
+    onAdd(after[a++]);
+  }
+  while (b < bLen) {
+    onRemove(before[b++]);
+  }
+}

packages/firestore/test/integration/api/index_configuration.test.ts

@@ -15,14 +15,17 @@
  * limitations under the License.
  */
 
-import { expect } from 'chai';
+import { expect, use } from 'chai';
+import chaiAsPromised from 'chai-as-promised';
 
 import { _setIndexConfiguration as setIndexConfiguration } from '../util/firebase_export';
 import { apiDescribe, withTestDb } from '../util/helpers';
 
+use(chaiAsPromised);
+
 apiDescribe('Index Configuration:', (persistence: boolean) => {
   it('supports JSON', () => {
-    return withTestDb(persistence, db => {
+    return withTestDb(persistence, async db => {
       return setIndexConfiguration(
         db,
         '{\n' +
@@ -59,7 +62,7 @@ apiDescribe('Index Configuration:', (persistence: boolean) => {
   });
 
   it('supports schema', () => {
-    return withTestDb(persistence, db => {
+    return withTestDb(persistence, async db => {
       return setIndexConfiguration(db, {
         indexes: [
           {
@@ -79,14 +82,20 @@ apiDescribe('Index Configuration:', (persistence: boolean) => {
 
   it('bad JSON does not crash client', () => {
     return withTestDb(persistence, async db => {
-      expect(() => setIndexConfiguration(db, '{,}')).to.throw(
-        'Failed to parse JSON'
-      );
+      const action: Promise<void> = setIndexConfiguration(db, '{,}');
+      if (persistence) {
+        await expect(action).to.eventually.be.rejectedWith(
+          /Failed to parse JSON/
+        );
+      } else {
+        // Silently do nothing. Parsing is not done and therefore no error is thrown.
+        await action;
+      }
     });
   });
 
   it('bad index does not crash client', () => {
-    return withTestDb(persistence, db => {
+    return withTestDb(persistence, async db => {
       return setIndexConfiguration(
         db,
         '{\n' +