Firestore: testing_hooks.ts: move some logic into testing_hooks_spi.ts

packages/firestore/externs.json

@@ -36,6 +36,7 @@
     "packages/firestore/src/util/error.ts",
     "packages/firestore/src/local/indexeddb_schema.ts",
     "packages/firestore/src/local/indexeddb_schema_legacy.ts",
-    "packages/firestore/src/local/shared_client_state_schema.ts"
+    "packages/firestore/src/local/shared_client_state_schema.ts",
+    "packages/firestore/src/util/testing_hooks.ts"
   ]
 }

packages/firestore/src/api.ts

@@ -223,4 +223,8 @@ export type {
 } from './api/credentials';
 export { EmptyAuthCredentialsProvider as _EmptyAuthCredentialsProvider } from './api/credentials';
 export { EmptyAppCheckTokenProvider as _EmptyAppCheckTokenProvider } from './api/credentials';
-export { TestingHooks as _TestingHooks } from './util/testing_hooks';
+export {
+  ExistenceFilterMismatchCallback as _TestingHooksExistenceFilterMismatchCallback,
+  TestingHooks as _TestingHooks
+} from './util/testing_hooks';
+export { ExistenceFilterMismatchInfo as _TestingHooksExistenceFilterMismatchInfo } from './util/testing_hooks_spi';

packages/firestore/src/remote/watch_change.ts

@@ -38,9 +38,9 @@ import { primitiveComparator } from '../util/misc';
 import { SortedMap } from '../util/sorted_map';
 import { SortedSet } from '../util/sorted_set';
 import {
-  ExistenceFilterMismatchInfo as TestingHooksExistenceFilterMismatchInfo,
-  TestingHooks
-} from '../util/testing_hooks';
+  testingHooksSpi,
+  ExistenceFilterMismatchInfo as TestingHooksExistenceFilterMismatchInfo
+} from '../util/testing_hooks_spi';
 
 import { BloomFilter, BloomFilterError } from './bloom_filter';
 import { ExistenceFilter } from './existence_filter';
@@ -452,7 +452,7 @@ export class WatchChangeAggregator {
               purpose
             );
           }
-          TestingHooks.instance?.notifyOnExistenceFilterMismatch(
+          testingHooksSpi?.notifyOnExistenceFilterMismatch(
             createExistenceFilterMismatchInfoForTestingHooks(
               currentSize,
               watchChange.existenceFilter,

packages/firestore/src/util/testing_hooks.ts

@@ -15,47 +15,24 @@
  * limitations under the License.
  */
 
+import { Unsubscribe } from '../api/reference_impl';
+
+import {
+  setTestingHooksSpi,
+  ExistenceFilterMismatchInfo,
+  TestingHooksSpi
+} from './testing_hooks_spi';
+
 /**
- * Manages "testing hooks", hooks into the internals of the SDK to verify
- * internal state and events during integration tests. Do not use this class
- * except for testing purposes.
- *
- * There are two ways to retrieve the global singleton instance of this class:
- * 1. The `instance` property, which returns null if the global singleton
- *      instance has not been created. Use this property if the caller should
- *      "do nothing" if there are no testing hooks registered, such as when
- *      delivering an event to notify registered callbacks.
- * 2. The `getOrCreateInstance()` method, which creates the global singleton
- *      instance if it has not been created. Use this method if the instance is
- *      needed to, for example, register a callback.
+ * Testing hooks for use by Firestore's integration test suite to reach into the
+ * SDK internals to validate logic and behavior that is not visible from the
+ * public API surface.
  *
  * @internal
  */
 export class TestingHooks {
-  private readonly onExistenceFilterMismatchCallbacks = new Map<
-    Symbol,
-    ExistenceFilterMismatchCallback
-  >();
-
-  private constructor() {}
-
-  /**
-   * Returns the singleton instance of this class, or null if it has not been
-   * initialized.
-   */
-  static get instance(): TestingHooks | null {
-    return gTestingHooksSingletonInstance;
-  }
-
-  /**
-   * Returns the singleton instance of this class, creating it if is has never
-   * been created before.
-   */
-  static getOrCreateInstance(): TestingHooks {
-    if (gTestingHooksSingletonInstance === null) {
-      gTestingHooksSingletonInstance = new TestingHooks();
-    }
-    return gTestingHooksSingletonInstance;
+  private constructor() {
+    throw new Error('instances of this class should not be created');
   }
 
   /**
@@ -72,87 +49,58 @@ export class TestingHooks {
    * the first invocation of the returned function does anything; all subsequent
    * invocations do nothing.
    */
-  onExistenceFilterMismatch(
+  static onExistenceFilterMismatch(
     callback: ExistenceFilterMismatchCallback
-  ): () => void {
-    const key = Symbol();
-    this.onExistenceFilterMismatchCallbacks.set(key, callback);
-    return () => this.onExistenceFilterMismatchCallbacks.delete(key);
-  }
-
-  /**
-   * Invokes all currently-registered `onExistenceFilterMismatch` callbacks.
-   * @param info Information about the existence filter mismatch.
-   */
-  notifyOnExistenceFilterMismatch(info: ExistenceFilterMismatchInfo): void {
-    this.onExistenceFilterMismatchCallbacks.forEach(callback => callback(info));
+  ): Unsubscribe {
+    return TestingHooksSpiImpl.instance.onExistenceFilterMismatch(callback);
   }
 }
 
 /**
- * Information about an existence filter mismatch, as specified to callbacks
- * registered with `TestingUtils.onExistenceFilterMismatch()`.
+ * The signature of callbacks registered with
+ * `TestingUtils.onExistenceFilterMismatch()`.
+ *
+ * The return value, if any, is ignored.
+ *
+ * @internal
  */
-export interface ExistenceFilterMismatchInfo {
-  /** The number of documents that matched the query in the local cache. */
-  localCacheCount: number;
-
-  /**
-   * The number of documents that matched the query on the server, as specified
-   * in the ExistenceFilter message's `count` field.
-   */
-  existenceFilterCount: number;
-
-  /**
-   * The projectId used when checking documents for membership in the bloom
-   * filter.
-   */
-  projectId: string;
-
-  /**
-   * The databaseId used when checking documents for membership in the bloom
-   * filter.
-   */
-  databaseId: string;
+export type ExistenceFilterMismatchCallback = (
+  info: ExistenceFilterMismatchInfo
+) => unknown;
 
-  /**
-   * Information about the bloom filter provided by Watch in the ExistenceFilter
-   * message's `unchangedNames` field. If this property is omitted or undefined
-   * then that means that Watch did _not_ provide a bloom filter.
-   */
-  bloomFilter?: {
-    /**
-     * Whether a full requery was averted by using the bloom filter. If false,
-     * then something happened, such as a false positive, to prevent using the
-     * bloom filter to avoid a full requery.
-     */
-    applied: boolean;
+/**
+ * The implementation of `TestingHooksSpi`.
+ */
+class TestingHooksSpiImpl implements TestingHooksSpi {
+  private readonly existenceFilterMismatchCallbacksById = new Map<
+    Symbol,
+    ExistenceFilterMismatchCallback
+  >();
 
-    /** The number of hash functions used in the bloom filter. */
-    hashCount: number;
+  private constructor() {}
 
-    /** The number of bytes in the bloom filter's bitmask. */
-    bitmapLength: number;
+  static get instance(): TestingHooksSpiImpl {
+    if (!testingHooksSpiImplInstance) {
+      testingHooksSpiImplInstance = new TestingHooksSpiImpl();
+      setTestingHooksSpi(testingHooksSpiImplInstance);
+    }
+    return testingHooksSpiImplInstance;
+  }
 
-    /** The number of bits of padding in the last byte of the bloom filter. */
-    padding: number;
+  notifyOnExistenceFilterMismatch(info: ExistenceFilterMismatchInfo): void {
+    this.existenceFilterMismatchCallbacksById.forEach(callback =>
+      callback(info)
+    );
+  }
 
-    /**
-     * Tests the given string for membership in the bloom filter created from
-     * the existence filter; will be undefined if creating the bloom filter
-     * failed.
-     */
-    mightContain?: (value: string) => boolean;
-  };
+  onExistenceFilterMismatch(
+    callback: ExistenceFilterMismatchCallback
+  ): Unsubscribe {
+    const id = Symbol();
+    const callbacks = this.existenceFilterMismatchCallbacksById;
+    callbacks.set(id, callback);
+    return () => callbacks.delete(id);
+  }
 }
 
-/**
- * The signature of callbacks registered with
- * `TestingUtils.onExistenceFilterMismatch()`.
- */
-export type ExistenceFilterMismatchCallback = (
-  info: ExistenceFilterMismatchInfo
-) => void;
-
-/** The global singleton instance of `TestingHooks`. */
-let gTestingHooksSingletonInstance: TestingHooks | null = null;
+let testingHooksSpiImplInstance: TestingHooksSpiImpl | null = null;

packages/firestore/src/util/testing_hooks_spi.ts

@@ -0,0 +1,110 @@
+/**
+ * @license
+ * Copyright 2023 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.
+ */
+
+/**
+ * The global, singleton instance of TestingHooksSpi.
+ *
+ * This variable will be `null` in all cases _except_ when running from
+ * integration tests that have registered callbacks to be notified of events
+ * that happen during the test execution.
+ */
+export let testingHooksSpi: TestingHooksSpi | null = null;
+
+/**
+ * Sets the value of the `testingHooksSpi` object.
+ * @param instance the instance to set.
+ */
+export function setTestingHooksSpi(instance: TestingHooksSpi): void {
+  if (testingHooksSpi) {
+    throw new Error('a TestingHooksSpi instance is already set');
+  }
+  testingHooksSpi = instance;
+}
+
+/**
+ * The "service provider interface" for the testing hooks.
+ *
+ * The implementation of this object will handle the callbacks made by the SDK
+ * to be handled by the integration tests.
+ *
+ * This "SPI" is separated from the implementation to avoid import cycles and
+ * to enable production builds to fully tree-shake away the testing hooks logic.
+ */
+export interface TestingHooksSpi {
+  /**
+   * Invokes all callbacks registered with
+   * `TestingHooks.onExistenceFilterMismatch()` with the given info.
+   */
+  notifyOnExistenceFilterMismatch(info: ExistenceFilterMismatchInfo): void;
+}
+
+/**
+ * Information about an existence filter mismatch.
+ * @internal
+ */
+export interface ExistenceFilterMismatchInfo {
+  /** The number of documents that matched the query in the local cache. */
+  localCacheCount: number;
+
+  /**
+   * The number of documents that matched the query on the server, as specified
+   * in the ExistenceFilter message's `count` field.
+   */
+  existenceFilterCount: number;
+
+  /**
+   * The projectId used when checking documents for membership in the bloom
+   * filter.
+   */
+  projectId: string;
+
+  /**
+   * The databaseId used when checking documents for membership in the bloom
+   * filter.
+   */
+  databaseId: string;
+
+  /**
+   * Information about the bloom filter provided by Watch in the ExistenceFilter
+   * message's `unchangedNames` field. If this property is omitted or undefined
+   * then that means that Watch did _not_ provide a bloom filter.
+   */
+  bloomFilter?: {
+    /**
+     * Whether a full requery was averted by using the bloom filter. If false,
+     * then something happened, such as a false positive, to prevent using the
+     * bloom filter to avoid a full requery.
+     */
+    applied: boolean;
+
+    /** The number of hash functions used in the bloom filter. */
+    hashCount: number;
+
+    /** The number of bytes in the bloom filter's bitmask. */
+    bitmapLength: number;
+
+    /** The number of bits of padding in the last byte of the bloom filter. */
+    padding: number;
+
+    /**
+     * Tests the given string for membership in the bloom filter created from
+     * the existence filter; will be undefined if creating the bloom filter
+     * failed.
+     */
+    mightContain?: (value: string) => boolean;
+  };
+}