Mila/count

.changeset/hot-insects-wink.md

@@ -0,0 +1,5 @@
+---
+'@firebase/firestore': minor
+---
+
+Release count query for internal use.

packages/firestore/src/api/aggregate.ts

@@ -22,16 +22,6 @@ import { cast } from '../util/input_validation';
 
 import { ensureFirestoreConfigured, Firestore } from './database';
 
-export {
-  AggregateField,
-  AggregateFieldType,
-  AggregateQuerySnapshot,
-  aggregateQuerySnapshotEqual,
-  AggregateSpec,
-  AggregateSpecData,
-  count
-} from '../lite-api/aggregate';
-
 /**
  * Executes the query and returns the results as a `AggregateQuerySnapshot` from the
  * server. Returns an error if the network is not available.

packages/firestore/src/core/firestore_client.ts

@@ -17,14 +17,17 @@
 
 import { GetOptions } from '@firebase/firestore-types';
 
-import { AggregateField, AggregateQuerySnapshot } from '../api/aggregate';
 import { LoadBundleTask } from '../api/bundle';
 import {
   CredentialChangeListener,
   CredentialsProvider
 } from '../api/credentials';
 import { User } from '../auth/user';
-import { getCount } from '../lite-api/aggregate';
+import {
+  AggregateField,
+  AggregateQuerySnapshot,
+  getCount
+} from '../lite-api/aggregate';
 import { Query as LiteQuery } from '../lite-api/reference';
 import { LocalStore } from '../local/local_store';
 import {
@@ -513,21 +516,20 @@ export function firestoreClientRunCountQuery(
     AggregateQuerySnapshot<{ count: AggregateField<number> }>
   >();
   client.asyncQueue.enqueueAndForget(async () => {
-    const remoteStore = await getRemoteStore(client);
-    if (!canUseNetwork(remoteStore)) {
-      deferred.reject(
-        new FirestoreError(
-          Code.UNAVAILABLE,
-          'Failed to get count result because the client is offline.'
-        )
-      );
-    } else {
-      try {
-        const result = await getCount(query);
-        deferred.resolve(result);
-      } catch (e) {
-        deferred.reject(e as Error);
+    try {
+      const remoteStore = await getRemoteStore(client);
+      if (!canUseNetwork(remoteStore)) {
+        deferred.reject(
+          new FirestoreError(
+            Code.UNAVAILABLE,
+            'Failed to get count result because the client is offline.'
+          )
+        );
       }
+      const result = await getCount(query);
+      deferred.resolve(result);
+    } catch (e) {
+      deferred.reject(e as Error);
     }
   });
   return deferred.promise;

packages/firestore/src/lite-api/aggregate.ts

@@ -136,12 +136,12 @@ export function getCount(
 /**
  * Compares two `AggregateQuerySnapshot` instances for equality.
  * Two `AggregateQuerySnapshot` instances are considered "equal" if they have
- * the same underlying query, the same metadata, and the same data.
+ * the same underlying query, and the same data.
  *
  * @param left - The `AggregateQuerySnapshot` to compare.
  * @param right - The `AggregateQuerySnapshot` to compare.
  *
- * @returns true if the AggregateQuerySnapshos are equal.
+ * @returns true if the AggregateQuerySnapshots are equal.
  */
 export function aggregateQuerySnapshotEqual<T extends AggregateSpec>(
   left: AggregateQuerySnapshot<T>,

packages/firestore/src/platform/node/grpc_connection.ts

@@ -86,6 +86,8 @@ export class GrpcConnection implements Connection {
   private cachedStub: GeneratedGrpcStub | null = null;
 
   get shouldResourcePathBeIncludedInRequest(): boolean {
+    // Both `invokeRPC()` and `invokeStreamingRPC()` ignore their `path` arguments, and expect
+    // the "path" to be part of the given `request`.
     return true;
   }
 

packages/firestore/src/remote/rest_connection.ts

@@ -56,6 +56,8 @@ export abstract class RestConnection implements Connection {
   private readonly databaseRoot: string;
 
   get shouldResourcePathBeIncludedInRequest(): boolean {
+    // Both `invokeRPC()` and `invokeStreamingRPC()` use their `path` arguments to determine
+    // where to run the query, and expect the `request` to NOT specify the "path".
     return false;
   }
 

packages/firestore/test/integration/api_internal/aggregation.test.ts

@@ -27,51 +27,57 @@ import {
   query,
   terminate,
   where,
-  writeBatch
+  writeBatch,
+  DocumentData,
+  QueryDocumentSnapshot
 } from '../util/firebase_export';
 import {
   apiDescribe,
-  postConverter,
   withEmptyTestCollection,
   withTestCollection,
-  withTestDb,
-  skipTestUnlessUsingEmulator
+  withTestDb
 } from '../util/helpers';
+import { USE_EMULATOR } from '../util/settings';
 
-apiDescribe('Count query', (persistence: boolean) => {
-  it('can run count query getCountFromServer', () => {
-    const testDocs = {
-      a: { author: 'authorA', title: 'titleA' },
-      b: { author: 'authorB', title: 'titleB' }
-    };
-    return skipTestUnlessUsingEmulator(() =>
-      withTestCollection(persistence, testDocs, async coll => {
+(USE_EMULATOR ? apiDescribe : apiDescribe.skip)(
+  'Count quries',
+  (persistence: boolean) => {
+    it('can run count query getCountFromServer', () => {
+      const testDocs = {
+        a: { author: 'authorA', title: 'titleA' },
+        b: { author: 'authorB', title: 'titleB' }
+      };
+      return withTestCollection(persistence, testDocs, async coll => {
         const snapshot = await getCountFromServer(coll);
         expect(snapshot.data().count).to.equal(2);
-      })
-    );
-  });
+      });
+    });
 
-  it('count query supports withConverter', () => {
-    const testDocs = {
-      a: { author: 'authorA', title: 'titleA' },
-      b: { author: 'authorB', title: 'titleB' }
-    };
-    return skipTestUnlessUsingEmulator(() =>
-      withTestCollection(persistence, testDocs, async coll => {
+    it("count query doesn't use converter", () => {
+      const testDocs = {
+        a: { author: 'authorA', title: 'titleA' },
+        b: { author: 'authorB', title: 'titleB' }
+      };
+      const throwingConverter = {
+        toFirestore(obj: never): DocumentData {
+          throw new Error('should never be called');
+        },
+        fromFirestore(snapshot: QueryDocumentSnapshot): never {
+          throw new Error('should never be called');
+        }
+      };
+      return withTestCollection(persistence, testDocs, async coll => {
         const query_ = query(
           coll,
           where('author', '==', 'authorA')
-        ).withConverter(postConverter);
+        ).withConverter(throwingConverter);
         const snapshot = await getCountFromServer(query_);
         expect(snapshot.data().count).to.equal(1);
-      })
-    );
-  });
+      });
+    });
 
-  it('count query supports collection groups', () => {
-    return skipTestUnlessUsingEmulator(() =>
-      withTestDb(persistence, async db => {
+    it('count query supports collection groups', () => {
+      return withTestDb(persistence, async db => {
         const collectionGroupId = doc(collection(db, 'aggregateQueryTest')).id;
         const docPaths = [
           `${collectionGroupId}/cg-doc1`,
@@ -89,36 +95,32 @@ apiDescribe('Count query', (persistence: boolean) => {
           collectionGroup(db, collectionGroupId)
         );
         expect(snapshot.data().count).to.equal(2);
-      })
-    );
-  });
+      });
+    });
 
-  it('getCountFromServer fails if firestore is terminated', () => {
-    return withEmptyTestCollection(persistence, async (coll, firestore) => {
-      await terminate(firestore);
-      expect(() => getCountFromServer(coll)).to.throw(
-        'The client has already been terminated.'
-      );
+    it('getCountFromServer fails if firestore is terminated', () => {
+      return withEmptyTestCollection(persistence, async (coll, firestore) => {
+        await terminate(firestore);
+        expect(() => getCountFromServer(coll)).to.throw(
+          'The client has already been terminated.'
+        );
+      });
     });
-  });
 
-  it("terminate doesn't crash when there is count query in flight", () => {
-    return skipTestUnlessUsingEmulator(() =>
-      withEmptyTestCollection(persistence, async (coll, firestore) => {
+    it("terminate doesn't crash when there is count query in flight", () => {
+      return withEmptyTestCollection(persistence, async (coll, firestore) => {
         void getCountFromServer(coll);
         await terminate(firestore);
-      })
-    );
-  });
+      });
+    });
 
-  it('getCountFromServer fails if user is offline', () => {
-    return skipTestUnlessUsingEmulator(() =>
-      withEmptyTestCollection(persistence, async (coll, firestore) => {
+    it('getCountFromServer fails if user is offline', () => {
+      return withEmptyTestCollection(persistence, async (coll, firestore) => {
         await disableNetwork(firestore);
         await expect(getCountFromServer(coll)).to.be.eventually.rejectedWith(
           'Failed to get count result because the client is offline'
         );
-      })
-    );
-  });
-});
+      });
+    });
+  }
+);

packages/firestore/test/integration/util/helpers.ts

@@ -32,9 +32,7 @@ import {
   PrivateSettings,
   SnapshotListenOptions,
   newTestFirestore,
-  QueryDocumentSnapshot,
-  newTestApp,
-  FirestoreError
+  newTestApp
 } from './firebase_export';
 import {
   ALT_PROJECT_ID,
@@ -238,22 +236,6 @@ export async function withNamedTestDbsOrSkipUnlessUsingEmulator(
     }
   }
 }
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export function skipTestUnlessUsingEmulator<
-  T extends (...params: any[]) => Promise<void>
->(fn: T, ...params: Parameters<T>): Promise<void> {
-  /**
-   * This is a wrapper to execute test functions that can only run on emulator till
-   * production test environment is ready.
-   */
-  if (!USE_EMULATOR) {
-    return Promise.resolve();
-  }
-
-  return fn(...params).catch((error: FirestoreError) => {
-    throw error;
-  });
-}
 
 export function withTestDoc(
   persistence: boolean,
@@ -341,24 +323,3 @@ export function withTestCollectionSettings(
     }
   );
 }
-
-export class Post {
-  constructor(
-    readonly title: string,
-    readonly author: string,
-    readonly ref: DocumentReference | null = null
-  ) {}
-  byline(): string {
-    return this.title + ', by ' + this.author;
-  }
-}
-
-export const postConverter = {
-  toFirestore(post: Post): DocumentData {
-    return { title: post.title, author: post.author };
-  },
-  fromFirestore(snapshot: QueryDocumentSnapshot): Post {
-    const data = snapshot.data();
-    return new Post(data.title, data.author, snapshot.ref);
-  }
-};

packages/firestore/test/lite/helpers.ts

@@ -35,8 +35,7 @@ import { QueryDocumentSnapshot } from '../../src/lite-api/snapshot';
 import { AutoId } from '../../src/util/misc';
 import {
   DEFAULT_PROJECT_ID,
-  DEFAULT_SETTINGS,
-  USE_EMULATOR
+  DEFAULT_SETTINGS
 } from '../integration/util/settings';
 
 let appCount = 0;
@@ -102,21 +101,6 @@ export function withTestCollection(
   });
 }
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export function skipTestUnlessUsingEmulator<
-  T extends (...params: any[]) => Promise<void>
->(fn: T, ...params: Parameters<T>): Promise<void> {
-  /**
-   * This is a wrapper to execute test functions that can only run on emulator till
-   * production test environment is ready.
-   */
-  if (!USE_EMULATOR) {
-    return Promise.resolve();
-  }
-
-  return fn(...params);
-}
-
 // Used for testing the FirestoreDataConverter.
 export class Post {
   constructor(