Add spec test

Author: tom-andersen

packages/firestore/package.json

@@ -97,8 +97,11 @@
     "@firebase/auth": "0.20.4",
     "@rollup/plugin-alias": "3.1.9",
     "@rollup/plugin-json": "4.1.0",
+    "@types/chai": "^4.3.1",
     "@types/eslint": "7.29.0",
     "@types/json-stable-stringify": "1.0.34",
+    "chai": "^4.3.6",
+    "chai-exclude": "^2.1.0",
     "json-stable-stringify": "1.0.1",
     "protobufjs": "6.11.3",
     "rollup": "2.72.1",

packages/firestore/src/api/database.ts

@@ -56,6 +56,7 @@ import { AsyncQueue } from '../util/async_queue';
 import { newAsyncQueue } from '../util/async_queue_impl';
 import { Code, FirestoreError } from '../util/error';
 import { cast } from '../util/input_validation';
+import { logWarn } from '../util/log';
 import { Deferred } from '../util/promise';
 
 import { LoadBundleTask } from './bundle';
@@ -332,7 +333,7 @@ function setPersistenceProviders(
         if (!canFallbackFromIndexedDbError(error)) {
           throw error;
         }
-        console.warn(
+        logWarn(
           'Error enabling offline persistence. Falling back to ' +
             'persistence disabled: ' +
             error

packages/firestore/src/api/index_configuration.ts

@@ -26,6 +26,7 @@ import {
 } from '../model/field_index';
 import { Code, FirestoreError } from '../util/error';
 import { cast } from '../util/input_validation';
+import { logWarn } from '../util/log';
 
 import { ensureFirestoreConfigured, Firestore } from './database';
 
@@ -147,7 +148,7 @@ export function setIndexConfiguration(
   firestore: Firestore,
   json: string
 ): Promise<void>;
-export async function setIndexConfiguration(
+export function setIndexConfiguration(
   firestore: Firestore,
   jsonOrConfiguration: string | IndexConfiguration
 ): Promise<void> {
@@ -157,10 +158,18 @@ export async function setIndexConfiguration(
   // 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;
+    logWarn('Cannot enable indexes when persistence is disabled');
+    return Promise.resolve();
   }
+  const parsedIndexes = parseIndexes(jsonOrConfiguration);
+  return getLocalStore(client).then(localStore =>
+    localStoreConfigureFieldIndexes(localStore, parsedIndexes)
+  );
+}
 
+export function parseIndexes(
+  jsonOrConfiguration: string | IndexConfiguration
+): FieldIndex[] {
   const indexConfiguration =
     typeof jsonOrConfiguration === 'string'
       ? (tryParseJson(jsonOrConfiguration) as IndexConfiguration)
@@ -200,9 +209,7 @@ export async function setIndexConfiguration(
       );
     }
   }
-
-  const localStore = await getLocalStore(client);
-  await localStoreConfigureFieldIndexes(localStore, parsedIndexes);
+  return parsedIndexes;
 }
 
 function tryParseJson(json: string): Record<string, unknown> {

packages/firestore/src/local/indexeddb_schema.ts

@@ -28,8 +28,6 @@ import {
 import { EncodedResourcePath } from './encoded_resource_path';
 import { DbTimestampKey } from './indexeddb_sentinels';
 
-export const INDEXING_SCHEMA_VERSION = 15;
-
 /**
  * Schema Version for the Web client:
  * 1.  Initial version including Mutation Queue, Query Cache, and Remote
@@ -55,7 +53,7 @@ export const INDEXING_SCHEMA_VERSION = 15;
  * 15. Add indexing support.
  */
 
-export const SCHEMA_VERSION = INDEXING_SCHEMA_VERSION;
+export const SCHEMA_VERSION = 15;
 
 /**
  * Wrapper class to store timestamps (seconds and nanos) in IndexedDb objects.

packages/firestore/src/local/indexeddb_schema_converter.ts

@@ -45,7 +45,7 @@ import {
   DbTarget,
   DbTargetDocument,
   DbTargetGlobal,
-  INDEXING_SCHEMA_VERSION
+  SCHEMA_VERSION
 } from './indexeddb_schema';
 import {
   DbRemoteDocument as DbRemoteDocumentLegacy,
@@ -146,7 +146,7 @@ export class SchemaConverter implements SimpleDbSchemaConverter {
     debugAssert(
       fromVersion < toVersion &&
         fromVersion >= 0 &&
-        toVersion <= INDEXING_SCHEMA_VERSION,
+        toVersion <= SCHEMA_VERSION,
       `Unexpected schema upgrade from v${fromVersion} to v${toVersion}.`
     );
 

packages/firestore/src/local/local_store_impl.ts

@@ -1496,7 +1496,7 @@ export async function localStoreConfigureFieldIndexes(
   const promises: Array<PersistencePromise<void>> = [];
   return localStoreImpl.persistence.runTransaction(
     'Configure indexes',
-    'readwrite-primary',
+    'readwrite',
     transaction =>
       indexManager
         .getFieldIndexes(transaction)

packages/firestore/src/util/async_observer.ts

@@ -18,6 +18,7 @@
 import { Observer } from '../core/event_manager';
 
 import { FirestoreError } from './error';
+import { logError } from './log';
 import { EventHandler } from './misc';
 
 /*
@@ -44,7 +45,7 @@ export class AsyncObserver<T> implements Observer<T> {
     if (this.observer.error) {
       this.scheduleEvent(this.observer.error, error);
     } else {
-      console.error('Uncaught Error in snapshot listener:', error);
+      logError('Uncaught Error in snapshot listener:', error);
     }
   }
 

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

@@ -15,14 +15,11 @@
  * limitations under the License.
  */
 
-import { expect, use } from 'chai';
-import chaiAsPromised from 'chai-as-promised';
+import { expect } from 'chai';
 
 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, async db => {
@@ -82,14 +79,12 @@ apiDescribe('Index Configuration:', (persistence: boolean) => {
 
   it('bad JSON does not crash client', () => {
     return withTestDb(persistence, async db => {
-      const action: Promise<void> = setIndexConfiguration(db, '{,}');
+      const action = (): Promise<void> => setIndexConfiguration(db, '{,}');
       if (persistence) {
-        await expect(action).to.eventually.be.rejectedWith(
-          /Failed to parse JSON/
-        );
+        expect(action).to.throw(/Failed to parse JSON/);
       } else {
         // Silently do nothing. Parsing is not done and therefore no error is thrown.
-        await action;
+        await action();
       }
     });
   });

packages/firestore/test/unit/local/index_manager.test.ts

@@ -32,7 +32,6 @@ import {
 import { FieldFilter } from '../../../src/core/target';
 import { IndexType } from '../../../src/local/index_manager';
 import { IndexedDbPersistence } from '../../../src/local/indexeddb_persistence';
-import { INDEXING_SCHEMA_VERSION } from '../../../src/local/indexeddb_schema';
 import { Persistence } from '../../../src/local/persistence';
 import { documentMap } from '../../../src/model/collections';
 import { Document } from '../../../src/model/document';
@@ -73,9 +72,7 @@ describe('IndexedDbIndexManager', async () => {
 
   let persistencePromise: Promise<Persistence>;
   beforeEach(async () => {
-    persistencePromise = persistenceHelpers.testIndexedDbPersistence({
-      schemaVersion: INDEXING_SCHEMA_VERSION
-    });
+    persistencePromise = persistenceHelpers.testIndexedDbPersistence();
   });
 
   async function getIndexManager(

packages/firestore/test/unit/local/query_engine.test.ts

@@ -30,7 +30,6 @@ import { SnapshotVersion } from '../../../src/core/snapshot_version';
 import { View } from '../../../src/core/view';
 import { DocumentOverlayCache } from '../../../src/local/document_overlay_cache';
 import { IndexedDbPersistence } from '../../../src/local/indexeddb_persistence';
-import { INDEXING_SCHEMA_VERSION } from '../../../src/local/indexeddb_schema';
 import { LocalDocumentsView } from '../../../src/local/local_documents_view';
 import { MutationQueue } from '../../../src/local/mutation_queue';
 import { Persistence } from '../../../src/local/persistence';
@@ -122,9 +121,7 @@ describe('IndexedDbQueryEngine', async () => {
 
   let persistencePromise: Promise<Persistence>;
   beforeEach(async () => {
-    persistencePromise = persistenceHelpers.testIndexedDbPersistence({
-      schemaVersion: INDEXING_SCHEMA_VERSION
-    });
+    persistencePromise = persistenceHelpers.testIndexedDbPersistence();
   });
 
   genericQueryEngineTest(/* durable= */ true, () => persistencePromise);

packages/firestore/test/unit/specs/index_spec.test.ts

@@ -0,0 +1,68 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { IndexConfiguration } from '../../../src/api/index_configuration';
+import { IndexKind } from '../../../src/model/field_index';
+import * as Helpers from '../../util/helpers';
+
+import { describeSpec, specTest } from './describe_spec';
+import { client } from './spec_builder';
+
+describeSpec('Client Side Index', [], () => {
+  const config: IndexConfiguration = {
+    indexes: [
+      {
+        collectionGroup: 'restaurants',
+        queryScope: 'COLLECTION',
+        fields: [
+          {
+            fieldPath: 'price',
+            order: 'ASCENDING'
+          }
+        ]
+      }
+    ]
+  };
+  const expectedIndexes = [
+    Helpers.fieldIndex('restaurants', {
+      fields: [['price', IndexKind.ASCENDING]]
+    })
+  ];
+  specTest('Index Creation visible on all clients', ['multi-client'], () => {
+    return client(0)
+      .expectPrimaryState(true)
+      .setIndexConfiguration(config)
+      .expectIndexes(expectedIndexes)
+      .client(1)
+      .expectPrimaryState(false)
+      .expectIndexes(expectedIndexes);
+  });
+  specTest(
+    'Index Creation succeeds even if not primary',
+    ['multi-client'],
+    () => {
+      return client(0)
+        .expectPrimaryState(true)
+        .client(1)
+        .expectPrimaryState(false)
+        .setIndexConfiguration(config)
+        .expectIndexes(expectedIndexes)
+        .client(0)
+        .expectIndexes(expectedIndexes);
+    }
+  );
+});

packages/firestore/test/unit/specs/spec_builder.ts

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { IndexConfiguration } from '../../../src/api/index_configuration';
 import { ExpUserDataWriter } from '../../../src/api/reference_impl';
 import {
   LimitType,
@@ -34,6 +35,7 @@ import { TargetIdGenerator } from '../../../src/core/target_id_generator';
 import { TargetId } from '../../../src/core/types';
 import { Document } from '../../../src/model/document';
 import { DocumentKey } from '../../../src/model/document_key';
+import { FieldIndex } from '../../../src/model/field_index';
 import { JsonObject } from '../../../src/model/object_value';
 import { ResourcePath } from '../../../src/model/path';
 import {
@@ -386,6 +388,16 @@ export class SpecBuilder {
     return this;
   }
 
+  setIndexConfiguration(
+    jsonOrConfiguration: string | IndexConfiguration
+  ): this {
+    this.nextStep();
+    this.currentStep = {
+      setIndexConfiguration: jsonOrConfiguration
+    };
+    return this;
+  }
+
   // PORTING NOTE: Only used by web multi-tab tests.
   becomeHidden(): this {
     this.nextStep();
@@ -508,6 +520,15 @@ export class SpecBuilder {
     return this;
   }
 
+  /** Expects indexes to exist (in any order) */
+  expectIndexes(indexes: FieldIndex[]): this {
+    this.assertStep('Indexes expectation requires previous step');
+    const currentStep = this.currentStep!;
+    currentStep.expectedState = currentStep.expectedState || {};
+    currentStep.expectedState.indexes = indexes;
+    return this;
+  }
+
   /** Overrides the currently expected set of active targets. */
   expectActiveTargets(
     ...targets: Array<{