From b92e492f408098d8a08511532caa714edcceade2 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 28 Jul 2022 13:07:24 -0700 Subject: [PATCH 01/34] Mila/count update proto (#6482) * update firestore.proto and query.proto * add aggregation_result.proto --- .../firestore/v1/aggregation_result.proto | 42 ++++++++++ .../google/firestore/v1/firestore.proto | 80 +++++++++++++++++++ .../protos/google/firestore/v1/query.proto | 51 ++++++++++++ 3 files changed, 173 insertions(+) create mode 100644 packages/firestore/src/protos/google/firestore/v1/aggregation_result.proto diff --git a/packages/firestore/src/protos/google/firestore/v1/aggregation_result.proto b/packages/firestore/src/protos/google/firestore/v1/aggregation_result.proto new file mode 100644 index 00000000000..538e3fef5e4 --- /dev/null +++ b/packages/firestore/src/protos/google/firestore/v1/aggregation_result.proto @@ -0,0 +1,42 @@ +// 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. + +syntax = "proto3"; + +package google.firestore.v1; + +import "google/firestore/v1/document.proto"; + +option csharp_namespace = "Google.Cloud.Firestore.V1"; +option go_package = "google.golang.org/genproto/googleapis/firestore/v1;firestore"; +option java_multiple_files = true; +option java_outer_classname = "AggregationResultProto"; +option java_package = "com.google.firestore.v1"; +option objc_class_prefix = "GCFS"; +option php_namespace = "Google\\Cloud\\Firestore\\V1"; +option ruby_package = "Google::Cloud::Firestore::V1"; + +// The result of a single bucket from a Firestore aggregation query. +// +// The keys of `aggregate_fields` are the same for all results in an aggregation +// query, unlike document queries which can have different fields present for +// each result. +message AggregationResult { + // The result of the aggregation functions, ex: `COUNT(*) AS total_docs`. + // + // The key is the [alias][google.firestore.v1.StructuredAggregationQuery.Aggregation.alias] + // assigned to the aggregation function on input and the size of this map + // equals the number of aggregation functions in the query. + map aggregate_fields = 2; +} diff --git a/packages/firestore/src/protos/google/firestore/v1/firestore.proto b/packages/firestore/src/protos/google/firestore/v1/firestore.proto index b149a7634eb..aefbe71699f 100644 --- a/packages/firestore/src/protos/google/firestore/v1/firestore.proto +++ b/packages/firestore/src/protos/google/firestore/v1/firestore.proto @@ -19,6 +19,7 @@ package google.firestore.v1; import "google/api/annotations.proto"; import "google/api/client.proto"; import "google/api/field_behavior.proto"; +import "google/firestore/v1/aggregation_result.proto"; import "google/firestore/v1/common.proto"; import "google/firestore/v1/document.proto"; import "google/firestore/v1/query.proto"; @@ -133,6 +134,29 @@ service Firestore { }; } + // Runs an aggregation query. + // + // Rather than producing [Document][google.firestore.v1.Document] results like [Firestore.RunQuery][google.firestore.v1.Firestore.RunQuery], + // this API allows running an aggregation to produce a series of + // [AggregationResult][google.firestore.v1.AggregationResult] server-side. + // + // High-Level Example: + // + // ``` + // -- Return the number of documents in table given a filter. + // SELECT COUNT(*) FROM ( SELECT * FROM k where a = true ); + // ``` + rpc RunAggregationQuery(RunAggregationQueryRequest) returns (stream RunAggregationQueryResponse) { + option (google.api.http) = { + post: "/v1/{parent=projects/*/databases/*/documents}:runAggregationQuery" + body: "*" + additional_bindings { + post: "/v1/{parent=projects/*/databases/*/documents/*/**}:runAggregationQuery" + body: "*" + } + }; + } + // Partitions a query by returning partition cursors that can be used to run // the query in parallel. The returned partition cursors are split points that // can be used by RunQuery as starting/end points for the query results. @@ -522,6 +546,62 @@ message RunQueryResponse { int32 skipped_results = 4; } +// The request for [Firestore.RunAggregationQuery][google.firestore.v1.Firestore.RunAggregationQuery]. +message RunAggregationQueryRequest { + // Required. The parent resource name. In the format: + // `projects/{project_id}/databases/{database_id}/documents` or + // `projects/{project_id}/databases/{database_id}/documents/{document_path}`. + // For example: + // `projects/my-project/databases/my-database/documents` or + // `projects/my-project/databases/my-database/documents/chatrooms/my-chatroom` + string parent = 1 [(google.api.field_behavior) = REQUIRED]; + + // The query to run. + oneof query_type { + // An aggregation query. + StructuredAggregationQuery structured_aggregation_query = 2; + } + + // The consistency mode for the query, defaults to strong consistency. + oneof consistency_selector { + // Run the aggregation within an already active transaction. + // + // The value here is the opaque transaction ID to execute the query in. + bytes transaction = 4; + + // Starts a new transaction as part of the query, defaulting to read-only. + // + // The new transaction ID will be returned as the first response in the + // stream. + TransactionOptions new_transaction = 5; + + // Executes the query at the given timestamp. + // + // Requires: + // + // * Cannot be more than 270 seconds in the past. + google.protobuf.Timestamp read_time = 6; + } +} + +// The response for [Firestore.RunAggregationQuery][google.firestore.v1.Firestore.RunAggregationQuery]. +message RunAggregationQueryResponse { + // A single aggregation result. + // + // Not present when reporting partial progress or when the query produced + // zero results. + AggregationResult result = 1; + + // The transaction that was started as part of this request. + // + // Only present on the first response when the request requested to start + // a new transaction. + bytes transaction = 2; + + // The time at which the aggregate value is valid for. + google.protobuf.Timestamp read_time = 3; +} + // The request for [Firestore.PartitionQuery][google.firestore.v1.Firestore.PartitionQuery]. message PartitionQueryRequest { // Required. The parent resource name. In the format: diff --git a/packages/firestore/src/protos/google/firestore/v1/query.proto b/packages/firestore/src/protos/google/firestore/v1/query.proto index 304499847c4..f2d2f21fb79 100644 --- a/packages/firestore/src/protos/google/firestore/v1/query.proto +++ b/packages/firestore/src/protos/google/firestore/v1/query.proto @@ -287,6 +287,57 @@ message StructuredQuery { google.protobuf.Int32Value limit = 5; } +message StructuredAggregationQuery { + // Defines a aggregation that produces a single result. + message Aggregation { + // Count of documents that match the query. + // + // The `COUNT(*)` aggregation function operates on the entire document + // so it does not require a field reference. + message Count { + // Optional. Optional constraint on the maximum number of documents to count. + // + // This provides a way to set an upper bound on the number of documents + // to scan, limiting latency and cost. + // + // High-Level Example: + // + // ``` + // SELECT COUNT_UP_TO(1000) FROM ( SELECT * FROM k ); + // ``` + // + // Requires: + // + // * Must be greater than zero when present. + int32 up_to = 1; + } + + // The type of aggregation to perform, required. + oneof operator { + // Count aggregator. + Count count = 1; + } + + // Required. The name of the field to store the result of the aggregation into. + // + // Requires: + // + // * Must be present. + // * Must be unique across all aggregation aliases. + // * Conform to existing [document field name][google.firestore.v1.Document.fields] limitations. + string alias = 7; + } + + // The base query to aggregate over. + oneof query_type { + // Nested structured query. + StructuredQuery structured_query = 1; + } + + // Optional. Series of aggregations to apply on top of the `structured_query`. + repeated Aggregation aggregations = 3; +} + // A position in a query result set. message Cursor { // The values that represent a position, in the order they appear in From 598e94820e94b89374c3b7561e8df7307b9f51f5 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 29 Jul 2022 12:50:01 -0700 Subject: [PATCH 02/34] compile protos and upload the generated json file (#6491) --- packages/firestore/src/protos/protos.json | 140 ++++++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/packages/firestore/src/protos/protos.json b/packages/firestore/src/protos/protos.json index fa1bbd289f4..b6401193bab 100644 --- a/packages/firestore/src/protos/protos.json +++ b/packages/firestore/src/protos/protos.json @@ -919,6 +919,15 @@ "ruby_package": "Google::Cloud::Firestore::V1" }, "nested": { + "AggregationResult": { + "fields": { + "aggregateFields": { + "keyType": "string", + "type": "Value", + "id": 2 + } + } + }, "DocumentMask": { "fields": { "fieldPaths": { @@ -1269,6 +1278,29 @@ } ] }, + "RunAggregationQuery": { + "requestType": "RunAggregationQueryRequest", + "responseType": "RunAggregationQueryResponse", + "responseStream": true, + "options": { + "(google.api.http).post": "/v1/{parent=projects/*/databases/*/documents}:runAggregationQuery", + "(google.api.http).body": "*", + "(google.api.http).additional_bindings.post": "/v1/{parent=projects/*/databases/*/documents/*/**}:runAggregationQuery", + "(google.api.http).additional_bindings.body": "*" + }, + "parsedOptions": [ + { + "(google.api.http)": { + "post": "/v1/{parent=projects/*/databases/*/documents}:runAggregationQuery", + "body": "*", + "additional_bindings": { + "post": "/v1/{parent=projects/*/databases/*/documents/*/**}:runAggregationQuery", + "body": "*" + } + } + } + ] + }, "PartitionQuery": { "requestType": "PartitionQueryRequest", "responseType": "PartitionQueryResponse", @@ -1760,6 +1792,63 @@ } } }, + "RunAggregationQueryRequest": { + "oneofs": { + "queryType": { + "oneof": [ + "structuredAggregationQuery" + ] + }, + "consistencySelector": { + "oneof": [ + "transaction", + "newTransaction", + "readTime" + ] + } + }, + "fields": { + "parent": { + "type": "string", + "id": 1, + "options": { + "(google.api.field_behavior)": "REQUIRED" + } + }, + "structuredAggregationQuery": { + "type": "StructuredAggregationQuery", + "id": 2 + }, + "transaction": { + "type": "bytes", + "id": 4 + }, + "newTransaction": { + "type": "TransactionOptions", + "id": 5 + }, + "readTime": { + "type": "google.protobuf.Timestamp", + "id": 6 + } + } + }, + "RunAggregationQueryResponse": { + "fields": { + "result": { + "type": "AggregationResult", + "id": 1 + }, + "transaction": { + "type": "bytes", + "id": 2 + }, + "readTime": { + "type": "google.protobuf.Timestamp", + "id": 3 + } + } + }, "PartitionQueryRequest": { "oneofs": { "queryType": { @@ -2296,6 +2385,57 @@ } } }, + "StructuredAggregationQuery": { + "oneofs": { + "queryType": { + "oneof": [ + "structuredQuery" + ] + } + }, + "fields": { + "structuredQuery": { + "type": "StructuredQuery", + "id": 1 + }, + "aggregations": { + "rule": "repeated", + "type": "Aggregation", + "id": 3 + } + }, + "nested": { + "Aggregation": { + "oneofs": { + "operator": { + "oneof": [ + "count" + ] + } + }, + "fields": { + "count": { + "type": "Count", + "id": 1 + }, + "alias": { + "type": "string", + "id": 7 + } + }, + "nested": { + "Count": { + "fields": { + "upTo": { + "type": "int32", + "id": 1 + } + } + } + } + } + } + }, "Cursor": { "fields": { "values": { From 7f42490c0f3af8f65a0b122e37470692f9c55ae8 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 4 Aug 2022 09:57:10 -0700 Subject: [PATCH 03/34] Mila/count add api interface (#6499) --- common/api-review/firestore.api.md | 30 +++++++++ packages/firestore/src/api.ts | 9 +++ packages/firestore/src/api/aggregate.ts | 25 +++++++ packages/firestore/src/lite-api/aggregate.ts | 69 ++++++++++++++++++++ 4 files changed, 133 insertions(+) create mode 100644 packages/firestore/src/api/aggregate.ts create mode 100644 packages/firestore/src/lite-api/aggregate.ts diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index db36b60482c..e77f2353747 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -17,6 +17,30 @@ export type AddPrefixToKeys { + // (undocumented) + readonly query: Query; + // (undocumented) + readonly type = "AggregateQuery"; +} + +// @public (undocumented) +export function aggregateQueryEqual(left: AggregateQuery, right: AggregateQuery): boolean; + +// @public (undocumented) +export class AggregateQuerySnapshot { + // (undocumented) + getCount(): number | null; + // (undocumented) + readonly query: AggregateQuery; + // (undocumented) + readonly type = "AggregateQuerySnapshot"; +} + +// @public (undocumented) +export function aggregateQuerySnapshotEqual(left: AggregateQuerySnapshot, right: AggregateQuerySnapshot): boolean; + // @public export function arrayRemove(...elements: unknown[]): FieldValue; @@ -69,6 +93,9 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por mockUserToken?: EmulatorMockTokenOptions | string; }): void; +// @public (undocumented) +export function countQuery(query: Query): AggregateQuery; + // @public export function deleteDoc(reference: DocumentReference): Promise; @@ -209,6 +236,9 @@ export class GeoPoint { }; } +// @public (undocumented) +export function getAggregateFromServerDirect(query: AggregateQuery): Promise; + // @public export function getDoc(reference: DocumentReference): Promise>; diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 8e774dbfb30..54c93346d21 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -15,6 +15,15 @@ * limitations under the License. */ +export { + AggregateQuery, + AggregateQuerySnapshot, + aggregateQueryEqual, + aggregateQuerySnapshotEqual, + countQuery, + getAggregateFromServerDirect +} from './api/aggregate'; + export { FieldPath, documentId } from './api/field_path'; export { diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts new file mode 100644 index 00000000000..1aa50bb75ea --- /dev/null +++ b/packages/firestore/src/api/aggregate.ts @@ -0,0 +1,25 @@ +/** + * @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. + */ + +export { + AggregateQuery, + AggregateQuerySnapshot, + aggregateQueryEqual, + aggregateQuerySnapshotEqual, + countQuery, + getAggregateFromServerDirect +} from '../lite-api/aggregate'; diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts new file mode 100644 index 00000000000..0afd82a5996 --- /dev/null +++ b/packages/firestore/src/lite-api/aggregate.ts @@ -0,0 +1,69 @@ +/** + * @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 { DocumentData, Query, queryEqual } from './reference'; + +export class AggregateQuery { + readonly type = 'AggregateQuery'; + readonly query: Query; + + /** @hideconstructor */ + constructor(query: Query) { + this.query = query; + } +} + +export class AggregateQuerySnapshot { + readonly type = 'AggregateQuerySnapshot'; + readonly query: AggregateQuery; + + /** @hideconstructor */ + constructor(query: AggregateQuery, readonly _count: number) { + this.query = query; + } + + getCount(): number | null { + return this._count; + } +} + +export function countQuery(query: Query): AggregateQuery { + return new AggregateQuery(query); +} + +export function getAggregateFromServerDirect( + query: AggregateQuery +): Promise { + return Promise.resolve(new AggregateQuerySnapshot(query, 42)); +} + +export function aggregateQueryEqual( + left: AggregateQuery, + right: AggregateQuery +): boolean { + return queryEqual(left.query, right.query); +} + +export function aggregateQuerySnapshotEqual( + left: AggregateQuerySnapshot, + right: AggregateQuerySnapshot +): boolean { + return ( + aggregateQueryEqual(left.query, right.query) && + left.getCount() === right.getCount() + ); +} From 981e6f11ea8e8a07597eb3ddba6de843551b021b Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 4 Aug 2022 11:24:01 -0700 Subject: [PATCH 04/34] create sample count test cases (#6506) --- .../test/integration/api/count.test.ts | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 packages/firestore/test/integration/api/count.test.ts diff --git a/packages/firestore/test/integration/api/count.test.ts b/packages/firestore/test/integration/api/count.test.ts new file mode 100644 index 00000000000..1a4bac61ced --- /dev/null +++ b/packages/firestore/test/integration/api/count.test.ts @@ -0,0 +1,53 @@ +/** + * @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 { expect } from 'chai'; +import { + countQuery, + getAggregateFromServerDirect, + query +} from '../util/firebase_export'; +import { apiDescribe, withTestCollection } from '../util/helpers'; + +apiDescribe('Aggregation COUNT query:', (persistence: boolean) => { + it('empty collection count equals to 0', () => { + const testDocs = {}; + return withTestCollection(persistence, testDocs, collection => { + const countQuery_ = countQuery(query(collection)); + return getAggregateFromServerDirect(countQuery_).then(snapshot => { + expect(snapshot.getCount()).to.equal(0); + }); + }); + }); + + it('test collection count equals to 6', () => { + const testDocs = { + a: { k: 'a' }, + b: { k: 'b' }, + c: { k: 'c' }, + d: { k: 'd' }, + e: { k: 'e' }, + f: { k: 'f' } + }; + return withTestCollection(persistence, testDocs, collection => { + const countQuery_ = countQuery(query(collection)); + return getAggregateFromServerDirect(countQuery_).then(snapshot => { + expect(snapshot.getCount()).to.equal(6); + }); + }); + }); +}); From bc61df742f69d0693ae6e1a8a2a9905cc0174a98 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 24 Aug 2022 13:41:09 -0700 Subject: [PATCH 05/34] Mila/count implement count query (#6528) --- common/api-review/firestore.api.md | 13 +- packages/firestore/src/lite-api/aggregate.ts | 63 +++++++- .../src/protos/firestore_proto_api.ts | 30 ++++ packages/firestore/src/remote/datastore.ts | 32 +++- .../firestore/src/remote/rest_connection.ts | 2 +- packages/firestore/src/remote/serializer.ts | 21 +++ .../test/integration/api/count.test.ts | 53 ------- .../firestore/test/lite/integration.test.ts | 137 ++++++++++++++++++ 8 files changed, 281 insertions(+), 70 deletions(-) delete mode 100644 packages/firestore/test/integration/api/count.test.ts diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index e77f2353747..35e21d31786 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -17,10 +17,9 @@ export type AddPrefixToKeys { - // (undocumented) - readonly query: Query; +// @public +export class AggregateQuery { + readonly query: Query; // (undocumented) readonly type = "AggregateQuery"; } @@ -28,7 +27,7 @@ export class AggregateQuery { // @public (undocumented) export function aggregateQueryEqual(left: AggregateQuery, right: AggregateQuery): boolean; -// @public (undocumented) +// @public export class AggregateQuerySnapshot { // (undocumented) getCount(): number | null; @@ -93,8 +92,8 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por mockUserToken?: EmulatorMockTokenOptions | string; }): void; -// @public (undocumented) -export function countQuery(query: Query): AggregateQuery; +// @public +export function countQuery(query: Query): AggregateQuery; // @public export function deleteDoc(reference: DocumentReference): Promise; diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts index 0afd82a5996..49770bb8293 100644 --- a/packages/firestore/src/lite-api/aggregate.ts +++ b/packages/firestore/src/lite-api/aggregate.ts @@ -15,40 +15,89 @@ * limitations under the License. */ -import { DocumentData, Query, queryEqual } from './reference'; +import { Value } from '../protos/firestore_proto_api'; +import { invokeRunAggregationQueryRpc } from '../remote/datastore'; +import { hardAssert } from '../util/assert'; +import { cast } from '../util/input_validation'; -export class AggregateQuery { +import { getDatastore } from './components'; +import { Firestore } from './database'; +import { Query, queryEqual } from './reference'; +import { LiteUserDataWriter } from './reference_impl'; + +/** + * An `AggregateQuery` computes some aggregation statistics from the result set of + * a base `Query`. + */ +export class AggregateQuery { readonly type = 'AggregateQuery'; - readonly query: Query; + /** + * The query on which you called `countQuery` in order to get this `AggregateQuery`. + */ + readonly query: Query; /** @hideconstructor */ - constructor(query: Query) { + constructor(query: Query) { this.query = query; } } +/** + * An `AggregateQuerySnapshot` contains results of a `AggregateQuery`. + */ export class AggregateQuerySnapshot { readonly type = 'AggregateQuerySnapshot'; readonly query: AggregateQuery; /** @hideconstructor */ - constructor(query: AggregateQuery, readonly _count: number) { + constructor(query: AggregateQuery, private readonly _count: number) { this.query = query; } + /** + * @returns The result of a document count aggregation. Returns null if no count aggregation is + * available in the result. + */ getCount(): number | null { return this._count; } } -export function countQuery(query: Query): AggregateQuery { +/** + * Creates an `AggregateQuery` counting the number of documents matching this query. + * + * @returns An `AggregateQuery` object that can be used to count the number of documents in + * the result set of this query. + */ +export function countQuery(query: Query): AggregateQuery { return new AggregateQuery(query); } export function getAggregateFromServerDirect( query: AggregateQuery ): Promise { - return Promise.resolve(new AggregateQuerySnapshot(query, 42)); + const firestore = cast(query.query.firestore, Firestore); + const datastore = getDatastore(firestore); + const userDataWriter = new LiteUserDataWriter(firestore); + + return invokeRunAggregationQueryRpc(datastore, query).then(result => { + hardAssert( + result[0] !== undefined, + 'Aggregation fields are missing from result.' + ); + + const counts = Object.entries(result[0]) + .filter(([key, value]) => key === 'count_alias') + .map(([key, value]) => userDataWriter.convertValue(value as Value)); + + const count = counts[0]; + hardAssert( + typeof count === 'number', + 'Count aggeragte field value is not a number: ' + count + ); + + return Promise.resolve(new AggregateQuerySnapshot(query, count)); + }); } export function aggregateQueryEqual( diff --git a/packages/firestore/src/protos/firestore_proto_api.ts b/packages/firestore/src/protos/firestore_proto_api.ts index 8634f2a7d4a..e7dfe0a88b2 100644 --- a/packages/firestore/src/protos/firestore_proto_api.ts +++ b/packages/firestore/src/protos/firestore_proto_api.ts @@ -334,6 +334,32 @@ export declare namespace firestoreV1ApiClientInterfaces { readTime?: string; skippedResults?: number; } + interface RunAggregationQueryRequest { + parent?: string; + structuredAggregationQuery?: StructuredAggregationQuery; + transaction?: string; + newTransaction?: TransactionOptions; + readTime?: string; + } + interface RunAggregationQueryResponse { + result?: AggregationResult; + transaction?: string; + readTime?: string; + } + interface AggregationResult { + aggregateFields?: ApiClientObjectMap; + } + interface StructuredAggregationQuery { + structuredQuery?: StructuredQuery; + aggregations?: Aggregation[]; + } + interface Aggregation { + count?: Count; + alias?: string; + } + interface Count { + upTo?: number; + } interface Status { code?: number; message?: string; @@ -479,6 +505,10 @@ export declare type RunQueryRequest = firestoreV1ApiClientInterfaces.RunQueryRequest; export declare type RunQueryResponse = firestoreV1ApiClientInterfaces.RunQueryResponse; +export declare type RunAggregationQueryRequest = + firestoreV1ApiClientInterfaces.RunAggregationQueryRequest; +export declare type RunAggregationQueryResponse = + firestoreV1ApiClientInterfaces.RunAggregationQueryResponse; export declare type Status = firestoreV1ApiClientInterfaces.Status; export declare type StructuredQuery = firestoreV1ApiClientInterfaces.StructuredQuery; diff --git a/packages/firestore/src/remote/datastore.ts b/packages/firestore/src/remote/datastore.ts index 0d96661f6b1..febae1187dc 100644 --- a/packages/firestore/src/remote/datastore.ts +++ b/packages/firestore/src/remote/datastore.ts @@ -18,14 +18,18 @@ import { CredentialsProvider } from '../api/credentials'; import { User } from '../auth/user'; import { Query, queryToTarget } from '../core/query'; +import { AggregateQuery } from '../lite-api/aggregate'; import { Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { Mutation } from '../model/mutation'; import { BatchGetDocumentsRequest as ProtoBatchGetDocumentsRequest, BatchGetDocumentsResponse as ProtoBatchGetDocumentsResponse, + RunAggregationQueryRequest as ProtoRunAggregationQueryRequest, + RunAggregationQueryResponse as ProtoRunAggregationQueryResponse, RunQueryRequest as ProtoRunQueryRequest, - RunQueryResponse as ProtoRunQueryResponse + RunQueryResponse as ProtoRunQueryResponse, + Value as ProtoValue } from '../protos/firestore_proto_api'; import { debugAssert, debugCast, hardAssert } from '../util/assert'; import { AsyncQueue } from '../util/async_queue'; @@ -45,7 +49,8 @@ import { JsonProtoSerializer, toMutation, toName, - toQueryTarget + toQueryTarget, + toRunAggregationQueryRequest } from './serializer'; /** @@ -232,6 +237,29 @@ export async function invokeRunQueryRpc( ); } +export async function invokeRunAggregationQueryRpc( + datastore: Datastore, + aggregateQuery: AggregateQuery +): Promise { + const datastoreImpl = debugCast(datastore, DatastoreImpl); + const request = toRunAggregationQueryRequest( + datastoreImpl.serializer, + queryToTarget(aggregateQuery.query._query) + ); + const response = await datastoreImpl.invokeStreamingRPC< + ProtoRunAggregationQueryRequest, + ProtoRunAggregationQueryResponse + >('RunAggregationQuery', request.parent!, { + structuredAggregationQuery: request.structuredAggregationQuery + }); + return ( + response + // Omit RunAggregationQueryResponse that only contain readTimes. + .filter(proto => !!proto.result) + .map(proto => proto.result!.aggregateFields!) + ); +} + export function newPersistentWriteStream( datastore: Datastore, queue: AsyncQueue, diff --git a/packages/firestore/src/remote/rest_connection.ts b/packages/firestore/src/remote/rest_connection.ts index 9c318486620..67fffbdde92 100644 --- a/packages/firestore/src/remote/rest_connection.ts +++ b/packages/firestore/src/remote/rest_connection.ts @@ -37,6 +37,7 @@ const RPC_NAME_URL_MAPPING: StringMap = {}; RPC_NAME_URL_MAPPING['BatchGetDocuments'] = 'batchGet'; RPC_NAME_URL_MAPPING['Commit'] = 'commit'; RPC_NAME_URL_MAPPING['RunQuery'] = 'runQuery'; +RPC_NAME_URL_MAPPING['RunAggregationQuery'] = 'runAggregationQuery'; const RPC_URL_VERSION = 'v1'; @@ -78,7 +79,6 @@ export abstract class RestConnection implements Connection { const headers = {}; this.modifyHeadersForRequest(headers, authToken, appCheckToken); - return this.performRPCRequest(rpcName, url, headers, req).then( response => { logDebug(LOG_TAG, 'Received: ', response); diff --git a/packages/firestore/src/remote/serializer.ts b/packages/firestore/src/remote/serializer.ts index e3c5de5c5ed..21091d55a36 100644 --- a/packages/firestore/src/remote/serializer.ts +++ b/packages/firestore/src/remote/serializer.ts @@ -77,6 +77,7 @@ import { OrderDirection as ProtoOrderDirection, Precondition as ProtoPrecondition, QueryTarget as ProtoQueryTarget, + RunAggregationQueryRequest as ProtoRunAggregationQueryRequest, Status as ProtoStatus, Target as ProtoTarget, TargetChangeTargetChangeType as ProtoTargetChangeTargetChangeType, @@ -852,6 +853,26 @@ export function toQueryTarget( return result; } +export function toRunAggregationQueryRequest( + serializer: JsonProtoSerializer, + target: Target +): ProtoRunAggregationQueryRequest { + const queryTarget = toQueryTarget(serializer, target); + + return { + structuredAggregationQuery: { + aggregations: [ + { + count: {}, + alias: 'count_alias' + } + ], + structuredQuery: queryTarget.structuredQuery + }, + parent: queryTarget.parent + }; +} + export function convertQueryTargetToQuery(target: ProtoQueryTarget): Query { let path = fromQueryPath(target.parent!); diff --git a/packages/firestore/test/integration/api/count.test.ts b/packages/firestore/test/integration/api/count.test.ts deleted file mode 100644 index 1a4bac61ced..00000000000 --- a/packages/firestore/test/integration/api/count.test.ts +++ /dev/null @@ -1,53 +0,0 @@ -/** - * @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 { expect } from 'chai'; -import { - countQuery, - getAggregateFromServerDirect, - query -} from '../util/firebase_export'; -import { apiDescribe, withTestCollection } from '../util/helpers'; - -apiDescribe('Aggregation COUNT query:', (persistence: boolean) => { - it('empty collection count equals to 0', () => { - const testDocs = {}; - return withTestCollection(persistence, testDocs, collection => { - const countQuery_ = countQuery(query(collection)); - return getAggregateFromServerDirect(countQuery_).then(snapshot => { - expect(snapshot.getCount()).to.equal(0); - }); - }); - }); - - it('test collection count equals to 6', () => { - const testDocs = { - a: { k: 'a' }, - b: { k: 'b' }, - c: { k: 'c' }, - d: { k: 'd' }, - e: { k: 'e' }, - f: { k: 'f' } - }; - return withTestCollection(persistence, testDocs, collection => { - const countQuery_ = countQuery(query(collection)); - return getAggregateFromServerDirect(countQuery_).then(snapshot => { - expect(snapshot.getCount()).to.equal(6); - }); - }); - }); -}); diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts index deaa9a0236f..398175c195f 100644 --- a/packages/firestore/test/lite/integration.test.ts +++ b/packages/firestore/test/lite/integration.test.ts @@ -20,6 +20,12 @@ import { initializeApp } from '@firebase/app'; import { expect, use } from 'chai'; import chaiAsPromised from 'chai-as-promised'; +import { + countQuery, + getAggregateFromServerDirect, + aggregateQueryEqual, + aggregateQuerySnapshotEqual +} from '../../src/lite-api/aggregate'; import { Bytes } from '../../src/lite-api/bytes'; import { Firestore, @@ -2038,3 +2044,134 @@ describe('withConverter() support', () => { }); }); }); + +describe('countQuery()', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' }, + { author: 'authorB', title: 'titleD' }, + { author: 'authorB', title: 'titleE' } + ]; + + it('AggregateQuery and AggregateQuerySnapshot inherits the original query', () => { + return withTestCollection(async coll => { + const query_ = query(coll); + const countQuery_ = countQuery(query_); + expect(countQuery_.query).to.equal(query_); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.query).to.equal(countQuery_); + expect(snapshot.query.query).to.equal(query_); + }); + }); + + it('empty test collection count', () => { + return withTestCollection(async coll => { + const countQuery_ = countQuery(query(coll)); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(0); + }); + }); + + it('test collection count with 5 docs', () => { + return withTestCollectionAndInitialData(testDocs, async collection => { + const countQuery_ = countQuery(query(collection)); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(5); + }); + }); + + it('test collection count with filter', () => { + return withTestCollectionAndInitialData(testDocs, async collection => { + const query_ = query(collection, where('author', '==', 'authorA')); + const countQuery_ = countQuery(query_); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(2); + }); + }); + + it('test collection count with filter and a small limit size', () => { + return withTestCollectionAndInitialData(testDocs, async collection => { + const query_ = query( + collection, + where('author', '==', 'authorA'), + limit(1) + ); + const countQuery_ = countQuery(query_); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(1); + }); + }); + + it('test collection count with filter and a large limit size', () => { + return withTestCollectionAndInitialData(testDocs, async collection => { + const query_ = query( + collection, + where('author', '==', 'authorA'), + limit(3) + ); + const countQuery_ = countQuery(query_); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(2); + }); + }); + + it('test collection count with converter on query', () => { + return withTestCollectionAndInitialData(testDocs, async collection => { + const query_ = query( + collection, + where('author', '==', 'authorA') + ).withConverter(postConverter); + const countQuery_ = countQuery(query_); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(2); + }); + }); + + it('aggregateQueryEqual on same queries', () => { + return withTestCollectionAndInitialData(testDocs, async collection => { + const query1 = query(collection, where('author', '==', 'authorA')); + const query2 = query(collection, where('author', '==', 'authorA')); + const countQuery1 = countQuery(query1); + const countQuery2 = countQuery(query2); + expect(aggregateQueryEqual(countQuery1, countQuery2)).to.be.true; + }); + }); + + it('aggregateQueryEqual on different queries', () => { + return withTestCollectionAndInitialData(testDocs, async collection => { + const query1 = query(collection, where('author', '==', 'authorA')); + const query2 = query(collection, where('author', '==', 'authorB')); + const countQuery1 = countQuery(query1); + const countQuery2 = countQuery(query2); + expect(aggregateQueryEqual(countQuery1, countQuery2)).to.be.false; + }); + }); + + it('aggregateQuerySnapshotEqual on same queries', () => { + return withTestCollectionAndInitialData(testDocs, async collection => { + const query1 = query(collection, where('author', '==', 'authorA')); + const query2 = query(collection, where('author', '==', 'authorA')); + const countQuery1A = countQuery(query1); + const countQuery1B = countQuery(query1); + const countQuery2 = countQuery(query2); + const snapshot1A = await getAggregateFromServerDirect(countQuery1A); + const snapshot1B = await getAggregateFromServerDirect(countQuery1B); + const snapshot2 = await getAggregateFromServerDirect(countQuery2); + expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot1B)).to.be.true; + expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot2)).to.be.true; + }); + }); + + it('aggregateQuerySnapshotEqual on different queries', () => { + return withTestCollectionAndInitialData(testDocs, async collection => { + const query1 = query(collection, where('author', '==', 'authorA')); + const query2 = query(collection, where('author', '==', 'authorB')); + const countQuery1 = countQuery(query1); + const countQuery2 = countQuery(query2); + const snapshot1 = await getAggregateFromServerDirect(countQuery1); + const snapshot2 = await getAggregateFromServerDirect(countQuery2); + expect(aggregateQuerySnapshotEqual(snapshot1, snapshot2)).to.be.false; + }); + }); +}); From c21304b80507161766589ed223a069422e62f272 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 25 Aug 2022 18:02:05 -0700 Subject: [PATCH 06/34] add test case for terminated firestore --- packages/firestore/test/lite/integration.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts index 398175c195f..ebd9259d565 100644 --- a/packages/firestore/test/lite/integration.test.ts +++ b/packages/firestore/test/lite/integration.test.ts @@ -2174,4 +2174,14 @@ describe('countQuery()', () => { expect(aggregateQuerySnapshotEqual(snapshot1, snapshot2)).to.be.false; }); }); + + it('count query on a terminated Firestore', () => { + return withTestCollection(async collection => { + await terminate(collection.firestore); + const countQuery_ = countQuery(query(collection)); + expect(() => getAggregateFromServerDirect(countQuery_)).to.throw( + 'The client has already been terminated' + ); + }); + }); }); From a3242cedaa528339ce912daf05259c24b8bfccc2 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 30 Aug 2022 10:59:12 -0700 Subject: [PATCH 07/34] Mila/count add tests (#6566) --- .../firestore/test/lite/integration.test.ts | 172 ++++++++++++++++-- 1 file changed, 160 insertions(+), 12 deletions(-) diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts index ebd9259d565..87426196342 100644 --- a/packages/firestore/test/lite/integration.test.ts +++ b/packages/firestore/test/lite/integration.test.ts @@ -2046,20 +2046,12 @@ describe('withConverter() support', () => { }); describe('countQuery()', () => { - const testDocs = [ - { author: 'authorA', title: 'titleA' }, - { author: 'authorA', title: 'titleB' }, - { author: 'authorB', title: 'titleC' }, - { author: 'authorB', title: 'titleD' }, - { author: 'authorB', title: 'titleE' } - ]; - it('AggregateQuery and AggregateQuerySnapshot inherits the original query', () => { return withTestCollection(async coll => { const query_ = query(coll); const countQuery_ = countQuery(query_); - expect(countQuery_.query).to.equal(query_); const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(countQuery_.query).to.equal(query_); expect(snapshot.query).to.equal(countQuery_); expect(snapshot.query.query).to.equal(query_); }); @@ -2073,15 +2065,25 @@ describe('countQuery()', () => { }); }); - it('test collection count with 5 docs', () => { + it('test collection count with 3 docs', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' } + ]; return withTestCollectionAndInitialData(testDocs, async collection => { const countQuery_ = countQuery(query(collection)); const snapshot = await getAggregateFromServerDirect(countQuery_); - expect(snapshot.getCount()).to.equal(5); + expect(snapshot.getCount()).to.equal(3); }); }); it('test collection count with filter', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' } + ]; return withTestCollectionAndInitialData(testDocs, async collection => { const query_ = query(collection, where('author', '==', 'authorA')); const countQuery_ = countQuery(query_); @@ -2091,6 +2093,11 @@ describe('countQuery()', () => { }); it('test collection count with filter and a small limit size', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' } + ]; return withTestCollectionAndInitialData(testDocs, async collection => { const query_ = query( collection, @@ -2104,6 +2111,11 @@ describe('countQuery()', () => { }); it('test collection count with filter and a large limit size', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' } + ]; return withTestCollectionAndInitialData(testDocs, async collection => { const query_ = query( collection, @@ -2116,7 +2128,87 @@ describe('countQuery()', () => { }); }); + it('count with order by', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: null }, + { author: 'authorB' } + ]; + return withTestCollectionAndInitialData(testDocs, async collection => { + const query_ = query(collection, orderBy('title')); + const countQuery_ = countQuery(query_); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(3); + }); + }); + + it('count with order by and startAt', () => { + const testDocs = [ + { id: 3, author: 'authorA', title: 'titleA' }, + { id: 1, author: 'authorA', title: 'titleB' }, + { id: 2, author: 'authorB', title: 'titleC' }, + { id: null, author: 'authorB', title: 'titleD' } + ]; + return withTestCollectionAndInitialData(testDocs, async collection => { + const query_ = query(collection, orderBy('id'), startAt(2)); + const countQuery_ = countQuery(query_); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(2); + }); + }); + + it('count with order by and startAfter', () => { + const testDocs = [ + { id: 3, author: 'authorA', title: 'titleA' }, + { id: 1, author: 'authorA', title: 'titleB' }, + { id: 2, author: 'authorB', title: 'titleC' }, + { id: null, author: 'authorB', title: 'titleD' } + ]; + return withTestCollectionAndInitialData(testDocs, async collection => { + const query_ = query(collection, orderBy('id'), startAfter(2)); + const countQuery_ = countQuery(query_); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(1); + }); + }); + + it('count with order by and endAt', () => { + const testDocs = [ + { id: 3, author: 'authorA', title: 'titleA' }, + { id: 1, author: 'authorA', title: 'titleB' }, + { id: 2, author: 'authorB', title: 'titleC' }, + { id: null, author: 'authorB', title: 'titleD' } + ]; + return withTestCollectionAndInitialData(testDocs, async collection => { + const query_ = query(collection, orderBy('id'), startAt(1), endAt(2)); + const countQuery_ = countQuery(query_); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(2); + }); + }); + + it('count with order by and endBefore', () => { + const testDocs = [ + { id: 3, author: 'authorA', title: 'titleA' }, + { id: 1, author: 'authorA', title: 'titleB' }, + { id: 2, author: 'authorB', title: 'titleC' }, + { id: null, author: 'authorB', title: 'titleD' } + ]; + return withTestCollectionAndInitialData(testDocs, async collection => { + const query_ = query(collection, orderBy('id'), startAt(1), endBefore(2)); + const countQuery_ = countQuery(query_); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(1); + }); + }); + it('test collection count with converter on query', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' } + ]; return withTestCollectionAndInitialData(testDocs, async collection => { const query_ = query( collection, @@ -2128,7 +2220,33 @@ describe('countQuery()', () => { }); }); + it('count query with collection groups', () => { + return withTestDb(async db => { + const collectionGroupId = doc(collection(db, 'countTest')).id; + const docPaths = [ + `${collectionGroupId}/cg-doc1`, + `abc/123/${collectionGroupId}/cg-doc2`, + `zzz${collectionGroupId}/cg-doc3`, + `abc/123/zzz${collectionGroupId}/cg-doc4`, + `abc/123/zzz/${collectionGroupId}` + ]; + const batch = writeBatch(db); + for (const docPath of docPaths) { + batch.set(doc(db, docPath), { x: 1 }); + } + await batch.commit(); + const countQuery_ = countQuery(collectionGroup(db, collectionGroupId)); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(2); + }); + }); + it('aggregateQueryEqual on same queries', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' } + ]; return withTestCollectionAndInitialData(testDocs, async collection => { const query1 = query(collection, where('author', '==', 'authorA')); const query2 = query(collection, where('author', '==', 'authorA')); @@ -2139,6 +2257,11 @@ describe('countQuery()', () => { }); it('aggregateQueryEqual on different queries', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' } + ]; return withTestCollectionAndInitialData(testDocs, async collection => { const query1 = query(collection, where('author', '==', 'authorA')); const query2 = query(collection, where('author', '==', 'authorB')); @@ -2149,6 +2272,11 @@ describe('countQuery()', () => { }); it('aggregateQuerySnapshotEqual on same queries', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' } + ]; return withTestCollectionAndInitialData(testDocs, async collection => { const query1 = query(collection, where('author', '==', 'authorA')); const query2 = query(collection, where('author', '==', 'authorA')); @@ -2164,6 +2292,11 @@ describe('countQuery()', () => { }); it('aggregateQuerySnapshotEqual on different queries', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' } + ]; return withTestCollectionAndInitialData(testDocs, async collection => { const query1 = query(collection, where('author', '==', 'authorA')); const query2 = query(collection, where('author', '==', 'authorB')); @@ -2180,8 +2313,23 @@ describe('countQuery()', () => { await terminate(collection.firestore); const countQuery_ = countQuery(query(collection)); expect(() => getAggregateFromServerDirect(countQuery_)).to.throw( - 'The client has already been terminated' + 'The client has already been terminated.' ); }); }); + + it('terminate Firestore while calling count query', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' } + ]; + return withTestCollectionAndInitialData(testDocs, async collection => { + const countQuery_ = countQuery(query(collection)); + const promise = getAggregateFromServerDirect(countQuery_); + await terminate(collection.firestore); + const snapshot = await promise; + expect(snapshot.getCount()).to.equal(3); + }); + }); }); From 6da4f4fd51302696267234ee463fee5b6531ad2f Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 1 Sep 2022 12:57:54 -0700 Subject: [PATCH 08/34] Mila/count export aggregate to api (#6575) --- packages/firestore/src/api/aggregate.ts | 17 +++++++- .../firestore/src/core/firestore_client.ts | 11 +++++ packages/firestore/src/lite-api/aggregate.ts | 2 +- .../src/platform/node/grpc_connection.ts | 4 ++ packages/firestore/src/remote/connection.ts | 8 ++++ packages/firestore/src/remote/datastore.ts | 9 ++-- .../firestore/src/remote/rest_connection.ts | 4 ++ .../test/integration/api/aggregation.test.ts | 41 ++++++++++++++++++ .../firestore/test/lite/integration.test.ts | 42 +++++++++---------- 9 files changed, 111 insertions(+), 27 deletions(-) create mode 100644 packages/firestore/test/integration/api/aggregation.test.ts diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts index 1aa50bb75ea..7155daf7514 100644 --- a/packages/firestore/src/api/aggregate.ts +++ b/packages/firestore/src/api/aggregate.ts @@ -15,11 +15,24 @@ * limitations under the License. */ +import { firestoreClientRunAggregationQuery } from '../core/firestore_client'; +import { AggregateQuery, AggregateQuerySnapshot } from '../lite-api/aggregate'; +import { cast } from '../util/input_validation'; + +import { ensureFirestoreConfigured, Firestore } from './database'; + export { AggregateQuery, AggregateQuerySnapshot, aggregateQueryEqual, aggregateQuerySnapshotEqual, - countQuery, - getAggregateFromServerDirect + countQuery } from '../lite-api/aggregate'; + +export function getAggregateFromServerDirect( + query: AggregateQuery +): Promise { + const firestore = cast(query.query.firestore, Firestore); + const client = ensureFirestoreConfigured(firestore); + return firestoreClientRunAggregationQuery(client, query); +} diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 68cb92b9a73..50caf25e496 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -17,12 +17,14 @@ import { GetOptions } from '@firebase/firestore-types'; +import { AggregateQuery, AggregateQuerySnapshot } from '../api'; import { LoadBundleTask } from '../api/bundle'; import { CredentialChangeListener, CredentialsProvider } from '../api/credentials'; import { User } from '../auth/user'; +import { getAggregate } from '../lite-api/aggregate'; import { LocalStore } from '../local/local_store'; import { localStoreExecuteQuery, @@ -501,6 +503,15 @@ export function firestoreClientTransaction( return deferred.promise; } +export function firestoreClientRunAggregationQuery( + client: FirestoreClient, + query: AggregateQuery +): Promise { + return client.asyncQueue.enqueue(() => { + return getAggregate(query); + }); +} + async function readDocumentFromCache( localStore: LocalStore, docKey: DocumentKey, diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts index 49770bb8293..3519f17ccd6 100644 --- a/packages/firestore/src/lite-api/aggregate.ts +++ b/packages/firestore/src/lite-api/aggregate.ts @@ -73,7 +73,7 @@ export function countQuery(query: Query): AggregateQuery { return new AggregateQuery(query); } -export function getAggregateFromServerDirect( +export function getAggregate( query: AggregateQuery ): Promise { const firestore = cast(query.query.firestore, Firestore); diff --git a/packages/firestore/src/platform/node/grpc_connection.ts b/packages/firestore/src/platform/node/grpc_connection.ts index f8b15440ffa..e561c1981aa 100644 --- a/packages/firestore/src/platform/node/grpc_connection.ts +++ b/packages/firestore/src/platform/node/grpc_connection.ts @@ -85,6 +85,10 @@ export class GrpcConnection implements Connection { // We cache stubs for the most-recently-used token. private cachedStub: GeneratedGrpcStub | null = null; + get shouldResourcePathBeIncludedInRequest(): boolean { + return true; + } + constructor(protos: grpc.GrpcObject, private databaseInfo: DatabaseInfo) { // eslint-disable-next-line @typescript-eslint/no-explicit-any this.firestore = (protos as any)['google']['firestore']['v1']; diff --git a/packages/firestore/src/remote/connection.ts b/packages/firestore/src/remote/connection.ts index 19a54d37c73..b6d8828944a 100644 --- a/packages/firestore/src/remote/connection.ts +++ b/packages/firestore/src/remote/connection.ts @@ -84,6 +84,14 @@ export interface Connection { appCheckToken: Token | null ): Stream; + /** + * Returns whether or not the implementation requires that the "path" of the resource + * (a document or a collection) be present in the request message. If true, then the + * request message must include the path. If false, then the request message must NOT + * include the path. + */ + get shouldResourcePathBeIncludedInRequest(): boolean; + // TODO(mcg): subscribe to connection state changes. } diff --git a/packages/firestore/src/remote/datastore.ts b/packages/firestore/src/remote/datastore.ts index febae1187dc..827f05142d4 100644 --- a/packages/firestore/src/remote/datastore.ts +++ b/packages/firestore/src/remote/datastore.ts @@ -246,12 +246,15 @@ export async function invokeRunAggregationQueryRpc( datastoreImpl.serializer, queryToTarget(aggregateQuery.query._query) ); + + const parent = request.parent; + if (!datastoreImpl.connection.shouldResourcePathBeIncludedInRequest) { + delete request.parent; + } const response = await datastoreImpl.invokeStreamingRPC< ProtoRunAggregationQueryRequest, ProtoRunAggregationQueryResponse - >('RunAggregationQuery', request.parent!, { - structuredAggregationQuery: request.structuredAggregationQuery - }); + >('RunAggregationQuery', parent!, request); return ( response // Omit RunAggregationQueryResponse that only contain readTimes. diff --git a/packages/firestore/src/remote/rest_connection.ts b/packages/firestore/src/remote/rest_connection.ts index 67fffbdde92..b2e4e5b057f 100644 --- a/packages/firestore/src/remote/rest_connection.ts +++ b/packages/firestore/src/remote/rest_connection.ts @@ -55,6 +55,10 @@ export abstract class RestConnection implements Connection { protected readonly baseUrl: string; private readonly databaseRoot: string; + get shouldResourcePathBeIncludedInRequest(): boolean { + return false; + } + constructor(private readonly databaseInfo: DatabaseInfo) { this.databaseId = databaseInfo.databaseId; const proto = databaseInfo.ssl ? 'https' : 'http'; diff --git a/packages/firestore/test/integration/api/aggregation.test.ts b/packages/firestore/test/integration/api/aggregation.test.ts new file mode 100644 index 00000000000..23613c5dc6d --- /dev/null +++ b/packages/firestore/test/integration/api/aggregation.test.ts @@ -0,0 +1,41 @@ +/** + * @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 { expect } from 'chai'; + +import { + countQuery, + getAggregateFromServerDirect, + query +} from '../util/firebase_export'; +import { apiDescribe, withTestCollection } from '../util/helpers'; + +apiDescribe('Aggregation query', (persistence: boolean) => { + it('can run count query getAggregateFromServerDirect', () => { + const testDocs = { + a: { k: 'a', sort: 1 }, + b: { k: 'b', sort: 2 }, + c: { k: 'c', sort: 2 } + }; + return withTestCollection(persistence, testDocs, async coll => { + const query_ = query(coll); + const countQuery_ = countQuery(query_); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(3); + }); + }); +}); diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts index 87426196342..a439fe71e8d 100644 --- a/packages/firestore/test/lite/integration.test.ts +++ b/packages/firestore/test/lite/integration.test.ts @@ -22,7 +22,7 @@ import chaiAsPromised from 'chai-as-promised'; import { countQuery, - getAggregateFromServerDirect, + getAggregate, aggregateQueryEqual, aggregateQuerySnapshotEqual } from '../../src/lite-api/aggregate'; @@ -2050,7 +2050,7 @@ describe('countQuery()', () => { return withTestCollection(async coll => { const query_ = query(coll); const countQuery_ = countQuery(query_); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(countQuery_.query).to.equal(query_); expect(snapshot.query).to.equal(countQuery_); expect(snapshot.query.query).to.equal(query_); @@ -2060,7 +2060,7 @@ describe('countQuery()', () => { it('empty test collection count', () => { return withTestCollection(async coll => { const countQuery_ = countQuery(query(coll)); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(0); }); }); @@ -2073,7 +2073,7 @@ describe('countQuery()', () => { ]; return withTestCollectionAndInitialData(testDocs, async collection => { const countQuery_ = countQuery(query(collection)); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(3); }); }); @@ -2087,7 +2087,7 @@ describe('countQuery()', () => { return withTestCollectionAndInitialData(testDocs, async collection => { const query_ = query(collection, where('author', '==', 'authorA')); const countQuery_ = countQuery(query_); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(2); }); }); @@ -2105,7 +2105,7 @@ describe('countQuery()', () => { limit(1) ); const countQuery_ = countQuery(query_); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(1); }); }); @@ -2123,7 +2123,7 @@ describe('countQuery()', () => { limit(3) ); const countQuery_ = countQuery(query_); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(2); }); }); @@ -2138,7 +2138,7 @@ describe('countQuery()', () => { return withTestCollectionAndInitialData(testDocs, async collection => { const query_ = query(collection, orderBy('title')); const countQuery_ = countQuery(query_); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(3); }); }); @@ -2153,7 +2153,7 @@ describe('countQuery()', () => { return withTestCollectionAndInitialData(testDocs, async collection => { const query_ = query(collection, orderBy('id'), startAt(2)); const countQuery_ = countQuery(query_); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(2); }); }); @@ -2168,7 +2168,7 @@ describe('countQuery()', () => { return withTestCollectionAndInitialData(testDocs, async collection => { const query_ = query(collection, orderBy('id'), startAfter(2)); const countQuery_ = countQuery(query_); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(1); }); }); @@ -2183,7 +2183,7 @@ describe('countQuery()', () => { return withTestCollectionAndInitialData(testDocs, async collection => { const query_ = query(collection, orderBy('id'), startAt(1), endAt(2)); const countQuery_ = countQuery(query_); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(2); }); }); @@ -2198,7 +2198,7 @@ describe('countQuery()', () => { return withTestCollectionAndInitialData(testDocs, async collection => { const query_ = query(collection, orderBy('id'), startAt(1), endBefore(2)); const countQuery_ = countQuery(query_); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(1); }); }); @@ -2215,7 +2215,7 @@ describe('countQuery()', () => { where('author', '==', 'authorA') ).withConverter(postConverter); const countQuery_ = countQuery(query_); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(2); }); }); @@ -2236,7 +2236,7 @@ describe('countQuery()', () => { } await batch.commit(); const countQuery_ = countQuery(collectionGroup(db, collectionGroupId)); - const snapshot = await getAggregateFromServerDirect(countQuery_); + const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(2); }); }); @@ -2283,9 +2283,9 @@ describe('countQuery()', () => { const countQuery1A = countQuery(query1); const countQuery1B = countQuery(query1); const countQuery2 = countQuery(query2); - const snapshot1A = await getAggregateFromServerDirect(countQuery1A); - const snapshot1B = await getAggregateFromServerDirect(countQuery1B); - const snapshot2 = await getAggregateFromServerDirect(countQuery2); + const snapshot1A = await getAggregate(countQuery1A); + const snapshot1B = await getAggregate(countQuery1B); + const snapshot2 = await getAggregate(countQuery2); expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot1B)).to.be.true; expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot2)).to.be.true; }); @@ -2302,8 +2302,8 @@ describe('countQuery()', () => { const query2 = query(collection, where('author', '==', 'authorB')); const countQuery1 = countQuery(query1); const countQuery2 = countQuery(query2); - const snapshot1 = await getAggregateFromServerDirect(countQuery1); - const snapshot2 = await getAggregateFromServerDirect(countQuery2); + const snapshot1 = await getAggregate(countQuery1); + const snapshot2 = await getAggregate(countQuery2); expect(aggregateQuerySnapshotEqual(snapshot1, snapshot2)).to.be.false; }); }); @@ -2312,7 +2312,7 @@ describe('countQuery()', () => { return withTestCollection(async collection => { await terminate(collection.firestore); const countQuery_ = countQuery(query(collection)); - expect(() => getAggregateFromServerDirect(countQuery_)).to.throw( + expect(() => getAggregate(countQuery_)).to.throw( 'The client has already been terminated.' ); }); @@ -2326,7 +2326,7 @@ describe('countQuery()', () => { ]; return withTestCollectionAndInitialData(testDocs, async collection => { const countQuery_ = countQuery(query(collection)); - const promise = getAggregateFromServerDirect(countQuery_); + const promise = getAggregate(countQuery_); await terminate(collection.firestore); const snapshot = await promise; expect(snapshot.getCount()).to.equal(3); From fe871ceedb3aa236ce29834a0d7ba65a1624ca12 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 2 Sep 2022 09:41:45 -0700 Subject: [PATCH 09/34] change int32 to int64 (#6577) --- packages/firestore/src/protos/google/firestore/v1/query.proto | 2 +- packages/firestore/src/protos/protos.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/protos/google/firestore/v1/query.proto b/packages/firestore/src/protos/google/firestore/v1/query.proto index f2d2f21fb79..1bb2b6cdd01 100644 --- a/packages/firestore/src/protos/google/firestore/v1/query.proto +++ b/packages/firestore/src/protos/google/firestore/v1/query.proto @@ -309,7 +309,7 @@ message StructuredAggregationQuery { // Requires: // // * Must be greater than zero when present. - int32 up_to = 1; + google.protobuf.Int64Value up_to = 1; } // The type of aggregation to perform, required. diff --git a/packages/firestore/src/protos/protos.json b/packages/firestore/src/protos/protos.json index b6401193bab..093e22c6451 100644 --- a/packages/firestore/src/protos/protos.json +++ b/packages/firestore/src/protos/protos.json @@ -2427,7 +2427,7 @@ "Count": { "fields": { "upTo": { - "type": "int32", + "type": "google.protobuf.Int64Value", "id": 1 } } From 507e9848f44d888a7c3ac2ccb25de9d50b3ad9e9 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 6 Sep 2022 22:07:45 -0700 Subject: [PATCH 10/34] check client offline state and test (#6579) --- .../firestore/src/core/firestore_client.ts | 22 +++++++++++++++++-- .../test/integration/api/aggregation.test.ts | 18 +++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 50caf25e496..478f7329221 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -40,6 +40,7 @@ import { toByteStreamReader } from '../platform/byte_stream_reader'; import { newSerializer, newTextEncoder } from '../platform/serializer'; import { Datastore } from '../remote/datastore'; import { + canUseNetwork, RemoteStore, remoteStoreDisableNetwork, remoteStoreEnableNetwork, @@ -507,9 +508,26 @@ export function firestoreClientRunAggregationQuery( client: FirestoreClient, query: AggregateQuery ): Promise { - return client.asyncQueue.enqueue(() => { - return getAggregate(query); + const deferred = new Deferred(); + client.asyncQueue.enqueueAndForget(async () => { + const remoteStore = await getRemoteStore(client); + if (!canUseNetwork(remoteStore)) { + deferred.reject( + new FirestoreError( + Code.UNAVAILABLE, + 'Failed to get aggregate result because the client is offline.' + ) + ); + } else { + try { + const result = await getAggregate(query); + deferred.resolve(result); + } catch (e) { + deferred.reject(e as Error); + } + } }); + return deferred.promise; } async function readDocumentFromCache( diff --git a/packages/firestore/test/integration/api/aggregation.test.ts b/packages/firestore/test/integration/api/aggregation.test.ts index 23613c5dc6d..4a5804e0bed 100644 --- a/packages/firestore/test/integration/api/aggregation.test.ts +++ b/packages/firestore/test/integration/api/aggregation.test.ts @@ -19,6 +19,7 @@ import { expect } from 'chai'; import { countQuery, + disableNetwork, getAggregateFromServerDirect, query } from '../util/firebase_export'; @@ -38,4 +39,21 @@ apiDescribe('Aggregation query', (persistence: boolean) => { expect(snapshot.getCount()).to.equal(3); }); }); + + it('getAggregateFromServerDirect fails if user is offline', () => { + const testDocs = {}; + return withTestCollection( + persistence, + testDocs, + async (collection, firestore) => { + await disableNetwork(firestore); + const countQuery_ = countQuery(collection); + await expect( + getAggregateFromServerDirect(countQuery_) + ).to.be.eventually.rejectedWith( + 'Failed to get aggregate result because the client is offline' + ); + } + ); + }); }); From 98189d5b18bdcf0958c62dcf833f0c22be786214 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 8 Sep 2022 08:57:34 -0700 Subject: [PATCH 11/34] Mila/count non lite api tests (#6581) --- .../test/integration/api/aggregation.test.ts | 97 +++++++++++++++++-- .../test/integration/util/helpers.ts | 31 +++++- .../firestore/test/lite/integration.test.ts | 86 ++++++++-------- 3 files changed, 157 insertions(+), 57 deletions(-) diff --git a/packages/firestore/test/integration/api/aggregation.test.ts b/packages/firestore/test/integration/api/aggregation.test.ts index 4a5804e0bed..9dedef43090 100644 --- a/packages/firestore/test/integration/api/aggregation.test.ts +++ b/packages/firestore/test/integration/api/aggregation.test.ts @@ -18,33 +18,112 @@ import { expect } from 'chai'; import { + collection, + collectionGroup, countQuery, + doc, disableNetwork, getAggregateFromServerDirect, - query + query, + terminate, + where, + writeBatch } from '../util/firebase_export'; -import { apiDescribe, withTestCollection } from '../util/helpers'; +import { + apiDescribe, + postConverter, + withEmptyTestCollection, + withTestCollection, + withTestDb +} from '../util/helpers'; apiDescribe('Aggregation query', (persistence: boolean) => { it('can run count query getAggregateFromServerDirect', () => { const testDocs = { - a: { k: 'a', sort: 1 }, - b: { k: 'b', sort: 2 }, - c: { k: 'c', sort: 2 } + a: { author: 'authorA', title: 'titleA' }, + b: { author: 'authorB', title: 'titleB' } }; return withTestCollection(persistence, testDocs, async coll => { + const countQuery_ = countQuery(coll); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(2); + }); + }); + + it('aggregateQuery.query equals to original query', () => { + return withEmptyTestCollection(persistence, async coll => { const query_ = query(coll); + const aggregateQuery_ = countQuery(query_); + expect(aggregateQuery_.query).to.be.equal(query_); + }); + }); + + it('aggregateQuerySnapshot.query equals to aggregateQuery', () => { + return withEmptyTestCollection(persistence, async coll => { + const aggregateQuery_ = countQuery(coll); + const snapshot = await getAggregateFromServerDirect(aggregateQuery_); + expect(snapshot.query).to.be.equal(aggregateQuery_); + }); + }); + + it('aggregate query supports withConverter', () => { + const testDocs = { + a: { author: 'authorA', title: 'titleA' }, + b: { author: 'authorB', title: 'titleB' } + }; + return withTestCollection(persistence, testDocs, async coll => { + const query_ = query( + coll, + where('author', '==', 'authorA') + ).withConverter(postConverter); const countQuery_ = countQuery(query_); const snapshot = await getAggregateFromServerDirect(countQuery_); - expect(snapshot.getCount()).to.equal(3); + expect(snapshot.getCount()).to.equal(1); + }); + }); + + it('aggregate query supports collection groups', () => { + return withTestDb(persistence, async db => { + const collectionGroupId = doc(collection(db, 'aggregateQueryTest')).id; + const docPaths = [ + `${collectionGroupId}/cg-doc1`, + `abc/123/${collectionGroupId}/cg-doc2`, + `zzz${collectionGroupId}/cg-doc3`, + `abc/123/zzz${collectionGroupId}/cg-doc4`, + `abc/123/zzz/${collectionGroupId}` + ]; + const batch = writeBatch(db); + for (const docPath of docPaths) { + batch.set(doc(db, docPath), { x: 1 }); + } + await batch.commit(); + const countQuery_ = countQuery(collectionGroup(db, collectionGroupId)); + const snapshot = await getAggregateFromServerDirect(countQuery_); + expect(snapshot.getCount()).to.equal(2); + }); + }); + + it('aggregate query fails if firestore is terminated', () => { + return withEmptyTestCollection(persistence, async (coll, firestore) => { + await terminate(firestore); + const countQuery_ = countQuery(coll); + expect(() => getAggregateFromServerDirect(countQuery_)).to.throw( + 'The client has already been terminated.' + ); + }); + }); + + it("terminate doesn't crash when there is aggregate query in flight", () => { + return withEmptyTestCollection(persistence, async (coll, firestore) => { + const countQuery_ = countQuery(coll); + void getAggregateFromServerDirect(countQuery_); + await terminate(firestore); }); }); it('getAggregateFromServerDirect fails if user is offline', () => { - const testDocs = {}; - return withTestCollection( + return withEmptyTestCollection( persistence, - testDocs, async (collection, firestore) => { await disableNetwork(firestore); const countQuery_ = countQuery(collection); diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index 0a8dd3a5108..6a653445945 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -31,7 +31,8 @@ import { setDoc, PrivateSettings, SnapshotListenOptions, - newTestFirestore + newTestFirestore, + QueryDocumentSnapshot } from './firebase_export'; import { ALT_PROJECT_ID, @@ -254,6 +255,13 @@ export function withTestCollection( return withTestCollectionSettings(persistence, DEFAULT_SETTINGS, docs, fn); } +export function withEmptyTestCollection( + persistence: boolean, + fn: (collection: CollectionReference, db: Firestore) => Promise +): Promise { + return withTestCollection(persistence, {}, fn); +} + // TODO(mikelehen): Once we wipe the database between tests, we can probably // return the same collection every time. export function withTestCollectionSettings( @@ -280,3 +288,24 @@ 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); + } +}; diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts index a439fe71e8d..8af92d36a19 100644 --- a/packages/firestore/test/lite/integration.test.ts +++ b/packages/firestore/test/lite/integration.test.ts @@ -2059,7 +2059,7 @@ describe('countQuery()', () => { it('empty test collection count', () => { return withTestCollection(async coll => { - const countQuery_ = countQuery(query(coll)); + const countQuery_ = countQuery(coll); const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(0); }); @@ -2071,8 +2071,8 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const countQuery_ = countQuery(query(collection)); + return withTestCollectionAndInitialData(testDocs, async coll => { + const countQuery_ = countQuery(coll); const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(3); }); @@ -2084,8 +2084,8 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query_ = query(collection, where('author', '==', 'authorA')); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, where('author', '==', 'authorA')); const countQuery_ = countQuery(query_); const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(2); @@ -2098,12 +2098,8 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query_ = query( - collection, - where('author', '==', 'authorA'), - limit(1) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, where('author', '==', 'authorA'), limit(1)); const countQuery_ = countQuery(query_); const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(1); @@ -2116,12 +2112,8 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query_ = query( - collection, - where('author', '==', 'authorA'), - limit(3) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, where('author', '==', 'authorA'), limit(3)); const countQuery_ = countQuery(query_); const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(2); @@ -2135,8 +2127,8 @@ describe('countQuery()', () => { { author: 'authorB', title: null }, { author: 'authorB' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query_ = query(collection, orderBy('title')); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('title')); const countQuery_ = countQuery(query_); const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(3); @@ -2150,8 +2142,8 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query_ = query(collection, orderBy('id'), startAt(2)); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAt(2)); const countQuery_ = countQuery(query_); const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(2); @@ -2165,8 +2157,8 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query_ = query(collection, orderBy('id'), startAfter(2)); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAfter(2)); const countQuery_ = countQuery(query_); const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(1); @@ -2180,8 +2172,8 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query_ = query(collection, orderBy('id'), startAt(1), endAt(2)); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAt(1), endAt(2)); const countQuery_ = countQuery(query_); const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(2); @@ -2195,8 +2187,8 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query_ = query(collection, orderBy('id'), startAt(1), endBefore(2)); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAt(1), endBefore(2)); const countQuery_ = countQuery(query_); const snapshot = await getAggregate(countQuery_); expect(snapshot.getCount()).to.equal(1); @@ -2209,9 +2201,9 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { + return withTestCollectionAndInitialData(testDocs, async coll => { const query_ = query( - collection, + coll, where('author', '==', 'authorA') ).withConverter(postConverter); const countQuery_ = countQuery(query_); @@ -2247,9 +2239,9 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query1 = query(collection, where('author', '==', 'authorA')); - const query2 = query(collection, where('author', '==', 'authorA')); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query1 = query(coll, where('author', '==', 'authorA')); + const query2 = query(coll, where('author', '==', 'authorA')); const countQuery1 = countQuery(query1); const countQuery2 = countQuery(query2); expect(aggregateQueryEqual(countQuery1, countQuery2)).to.be.true; @@ -2262,9 +2254,9 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query1 = query(collection, where('author', '==', 'authorA')); - const query2 = query(collection, where('author', '==', 'authorB')); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query1 = query(coll, where('author', '==', 'authorA')); + const query2 = query(coll, where('author', '==', 'authorB')); const countQuery1 = countQuery(query1); const countQuery2 = countQuery(query2); expect(aggregateQueryEqual(countQuery1, countQuery2)).to.be.false; @@ -2277,9 +2269,9 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query1 = query(collection, where('author', '==', 'authorA')); - const query2 = query(collection, where('author', '==', 'authorA')); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query1 = query(coll, where('author', '==', 'authorA')); + const query2 = query(coll, where('author', '==', 'authorA')); const countQuery1A = countQuery(query1); const countQuery1B = countQuery(query1); const countQuery2 = countQuery(query2); @@ -2297,9 +2289,9 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const query1 = query(collection, where('author', '==', 'authorA')); - const query2 = query(collection, where('author', '==', 'authorB')); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query1 = query(coll, where('author', '==', 'authorA')); + const query2 = query(coll, where('author', '==', 'authorB')); const countQuery1 = countQuery(query1); const countQuery2 = countQuery(query2); const snapshot1 = await getAggregate(countQuery1); @@ -2309,9 +2301,9 @@ describe('countQuery()', () => { }); it('count query on a terminated Firestore', () => { - return withTestCollection(async collection => { - await terminate(collection.firestore); - const countQuery_ = countQuery(query(collection)); + return withTestCollection(async coll => { + await terminate(coll.firestore); + const countQuery_ = countQuery(coll); expect(() => getAggregate(countQuery_)).to.throw( 'The client has already been terminated.' ); @@ -2324,10 +2316,10 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async collection => { - const countQuery_ = countQuery(query(collection)); + return withTestCollectionAndInitialData(testDocs, async coll => { + const countQuery_ = countQuery(coll); const promise = getAggregate(countQuery_); - await terminate(collection.firestore); + await terminate(coll.firestore); const snapshot = await promise; expect(snapshot.getCount()).to.equal(3); }); From 0ebab69126ec499929bc9dcba14f1b0cacee5a40 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 12 Sep 2022 20:52:46 -0700 Subject: [PATCH 12/34] Mila/count update api surface (#6589) --- packages/firebase/compat/index.d.ts | 6 +- packages/firestore-types/index.d.ts | 4 +- packages/firestore/lite/index.ts | 10 + packages/firestore/src/api.ts | 10 +- packages/firestore/src/api/aggregate.ts | 32 ++-- .../firestore/src/core/firestore_client.ts | 19 +- packages/firestore/src/lite-api/aggregate.ts | 137 +++++++++----- packages/firestore/src/lite-api/reference.ts | 7 +- packages/firestore/src/remote/connection.ts | 2 +- packages/firestore/src/remote/datastore.ts | 7 +- .../test/integration/api/aggregation.test.ts | 76 +++----- .../firestore/test/lite/integration.test.ts | 179 +++++++----------- .../test/unit/remote/datastore.test.ts | 2 + .../test/unit/specs/spec_test_components.ts | 2 + .../prune-dts/tests/firestore.output.d.ts | 5 +- 15 files changed, 251 insertions(+), 247 deletions(-) diff --git a/packages/firebase/compat/index.d.ts b/packages/firebase/compat/index.d.ts index a946c38ec9a..c5c2803b767 100644 --- a/packages/firebase/compat/index.d.ts +++ b/packages/firebase/compat/index.d.ts @@ -8227,11 +8227,15 @@ declare namespace firebase.storage { } declare namespace firebase.firestore { + + /** Alias dynamic document field value types to any */ + export type DocumentFieldValue = any; + /** * Document data (for use with `DocumentReference.set()`) consists of fields * mapped to values. */ - export type DocumentData = { [field: string]: any }; + export type DocumentData = { [field: string]: DocumentFieldValue }; /** * Update data (for use with `DocumentReference.update()`) consists of field diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts index d198fd78866..603bb9c22c3 100644 --- a/packages/firestore-types/index.d.ts +++ b/packages/firestore-types/index.d.ts @@ -17,7 +17,9 @@ import { EmulatorMockTokenOptions } from '@firebase/util'; -export type DocumentData = { [field: string]: any }; +export type DocumentFieldValue = any; + +export type DocumentData = { [field: string]: DocumentFieldValue }; export type UpdateData = { [fieldPath: string]: any }; diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index ad7ec7d8294..ec31973fe25 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -29,6 +29,16 @@ registerFirestore(); export { FirestoreSettings as Settings } from '../src/lite-api/settings'; + +export { + AggregateField, + AggregateSpec, + AggregateSpecData, + AggregateQuerySnapshot, + getCount, + aggregateQuerySnapshotEqual +} from '../src/lite-api/aggregate'; + export { Firestore as Firestore, EmulatorMockTokenOptions, diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 54c93346d21..30aa59b6e39 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -16,12 +16,12 @@ */ export { - AggregateQuery, + AggregateField, + AggregateSpec, + AggregateSpecData, AggregateQuerySnapshot, - aggregateQueryEqual, - aggregateQuerySnapshotEqual, - countQuery, - getAggregateFromServerDirect + getCountFromServer, + aggregateQuerySnapshotEqual } from './api/aggregate'; export { FieldPath, documentId } from './api/field_path'; diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts index 7155daf7514..2d342b4d4d2 100644 --- a/packages/firestore/src/api/aggregate.ts +++ b/packages/firestore/src/api/aggregate.ts @@ -14,25 +14,33 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -import { firestoreClientRunAggregationQuery } from '../core/firestore_client'; -import { AggregateQuery, AggregateQuerySnapshot } from '../lite-api/aggregate'; +import { Query } from '../api'; +import { firestoreClientRunCountQuery } from '../core/firestore_client'; +import { AggregateField, AggregateQuerySnapshot } from '../lite-api/aggregate'; import { cast } from '../util/input_validation'; import { ensureFirestoreConfigured, Firestore } from './database'; export { - AggregateQuery, + AggregateField, + AggregateSpec, + AggregateSpecData, AggregateQuerySnapshot, - aggregateQueryEqual, - aggregateQuerySnapshotEqual, - countQuery + aggregateQuerySnapshotEqual } from '../lite-api/aggregate'; -export function getAggregateFromServerDirect( - query: AggregateQuery -): Promise { - const firestore = cast(query.query.firestore, Firestore); +/** + * Executes the query and returns the results as a `AggregateQuerySnapshot` from the + * server. Returns an error if the network is not available. + * + * @param query - The `Query` to execute. + * + * @returns A `Promise` that will be resolved with the results of the query. + */ +export function getCountFromServer( + query: Query +): Promise }>> { + const firestore = cast(query.firestore, Firestore); const client = ensureFirestoreConfigured(firestore); - return firestoreClientRunAggregationQuery(client, query); + return firestoreClientRunCountQuery(client, query); } diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 478f7329221..3730568478f 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -17,14 +17,15 @@ import { GetOptions } from '@firebase/firestore-types'; -import { AggregateQuery, AggregateQuerySnapshot } from '../api'; +import { AggregateField, AggregateQuerySnapshot } from '../api/aggregate'; import { LoadBundleTask } from '../api/bundle'; import { CredentialChangeListener, CredentialsProvider } from '../api/credentials'; import { User } from '../auth/user'; -import { getAggregate } from '../lite-api/aggregate'; +import { getCount } from '../lite-api/aggregate'; +import { Query as LiteQuery } from '../lite-api/reference'; import { LocalStore } from '../local/local_store'; import { localStoreExecuteQuery, @@ -504,23 +505,25 @@ export function firestoreClientTransaction( return deferred.promise; } -export function firestoreClientRunAggregationQuery( +export function firestoreClientRunCountQuery( client: FirestoreClient, - query: AggregateQuery -): Promise { - const deferred = new Deferred(); + query: LiteQuery +): Promise }>> { + const deferred = new Deferred< + AggregateQuerySnapshot<{ count: AggregateField }> + >(); client.asyncQueue.enqueueAndForget(async () => { const remoteStore = await getRemoteStore(client); if (!canUseNetwork(remoteStore)) { deferred.reject( new FirestoreError( Code.UNAVAILABLE, - 'Failed to get aggregate result because the client is offline.' + 'Failed to get count result because the client is offline.' ) ); } else { try { - const result = await getAggregate(query); + const result = await getCount(query); deferred.resolve(result); } catch (e) { deferred.reject(e as Error); diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts index 3519f17ccd6..a4156f823bd 100644 --- a/packages/firestore/src/lite-api/aggregate.ts +++ b/packages/firestore/src/lite-api/aggregate.ts @@ -15,6 +15,8 @@ * limitations under the License. */ +import { deepEqual } from '@firebase/util'; + import { Value } from '../protos/firestore_proto_api'; import { invokeRunAggregationQueryRpc } from '../remote/datastore'; import { hardAssert } from '../util/assert'; @@ -26,61 +28,87 @@ import { Query, queryEqual } from './reference'; import { LiteUserDataWriter } from './reference_impl'; /** - * An `AggregateQuery` computes some aggregation statistics from the result set of - * a base `Query`. + * An `AggregateField`that captures input type T. */ -export class AggregateQuery { - readonly type = 'AggregateQuery'; - /** - * The query on which you called `countQuery` in order to get this `AggregateQuery`. - */ - readonly query: Query; +// eslint-disable-next-line @typescript-eslint/no-unused-vars +export class AggregateField { + type = 'AggregateField'; +} - /** @hideconstructor */ - constructor(query: Query) { - this.query = query; - } +/** + * Creates and returns an aggregation field that counts the documents in the result set. + * @returns An `AggregateField` object with number input type. + */ +export function count(): AggregateField { + return new AggregateField(); } /** - * An `AggregateQuerySnapshot` contains results of a `AggregateQuery`. + * The union of all `AggregateField` types that are returned from the factory + * functions. + */ +type AggregateFieldType = ReturnType; + +/** + * A type whose values are all `AggregateField` objects. + * This is used as an argument to the "getter" functions, and the snapshot will + * map the same names to the corresponding values. + */ +export interface AggregateSpec { + [field: string]: AggregateFieldType; +} + +/** + * A type whose keys are taken from an `AggregateSpec` type, and whose values + * are the result of the aggregation performed by the corresponding + * `AggregateField` from the input `AggregateSpec`. + */ +export type AggregateSpecData = { + [P in keyof T]: T[P] extends AggregateField ? U : never; +}; + +/** + * An `AggregateQuerySnapshot` contains the results of running an aggregate query. */ -export class AggregateQuerySnapshot { +export class AggregateQuerySnapshot { readonly type = 'AggregateQuerySnapshot'; - readonly query: AggregateQuery; /** @hideconstructor */ - constructor(query: AggregateQuery, private readonly _count: number) { - this.query = query; - } + constructor( + readonly query: Query, + private readonly _data: AggregateSpecData + ) {} /** - * @returns The result of a document count aggregation. Returns null if no count aggregation is - * available in the result. + * The results of the requested aggregations. The keys of the returned object + * will be the same as those of the `AggregateSpec` object specified to the + * aggregation method, and the values will be the corresponding aggregation + * result. + * + * @returns The aggregation statistics result of running a query. */ - getCount(): number | null { - return this._count; + data(): AggregateSpecData { + return this._data; } } /** - * Creates an `AggregateQuery` counting the number of documents matching this query. + * Counts the number of documents in the result set of the given query, ignoring + * any locally-cached data and any locally-pending writes and simply surfacing + * whatever the server returns. If the server cannot be reached then the + * returned promise will be rejected. + * + * @param query - The `Query` to execute. * - * @returns An `AggregateQuery` object that can be used to count the number of documents in - * the result set of this query. + * @returns An `AggregateQuerySnapshot` that contains the number of documents. */ -export function countQuery(query: Query): AggregateQuery { - return new AggregateQuery(query); -} - -export function getAggregate( - query: AggregateQuery -): Promise { - const firestore = cast(query.query.firestore, Firestore); +export function getCount( + query: Query +): Promise }>> { + const firestore = cast(query.firestore, Firestore); const datastore = getDatastore(firestore); const userDataWriter = new LiteUserDataWriter(firestore); - - return invokeRunAggregationQueryRpc(datastore, query).then(result => { + return invokeRunAggregationQueryRpc(datastore, query._query).then(result => { hardAssert( result[0] !== undefined, 'Aggregation fields are missing from result.' @@ -90,29 +118,36 @@ export function getAggregate( .filter(([key, value]) => key === 'count_alias') .map(([key, value]) => userDataWriter.convertValue(value as Value)); - const count = counts[0]; + const countValue = counts[0]; + hardAssert( - typeof count === 'number', - 'Count aggeragte field value is not a number: ' + count + typeof countValue === 'number', + 'Count aggregate field value is not a number: ' + countValue ); - return Promise.resolve(new AggregateQuerySnapshot(query, count)); + return Promise.resolve( + new AggregateQuerySnapshot<{ count: AggregateField }>(query, { + count: countValue + }) + ); }); } -export function aggregateQueryEqual( - left: AggregateQuery, - right: AggregateQuery -): boolean { - return queryEqual(left.query, right.query); -} - -export function aggregateQuerySnapshotEqual( - left: AggregateQuerySnapshot, - right: AggregateQuerySnapshot +/** + * 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. + * + * @param left - The `AggregateQuerySnapshot` to compare. + * @param right - The `AggregateQuerySnapshot` to compare. + * + * @returns true if the AggregateQuerySnapshos are equal. + */ +export function aggregateQuerySnapshotEqual( + left: AggregateQuerySnapshot, + right: AggregateQuerySnapshot ): boolean { return ( - aggregateQueryEqual(left.query, right.query) && - left.getCount() === right.getCount() + queryEqual(left.query, right.query) && deepEqual(left.data(), right.data()) ); } diff --git a/packages/firestore/src/lite-api/reference.ts b/packages/firestore/src/lite-api/reference.ts index 7ade1ec0714..11e6d0a140f 100644 --- a/packages/firestore/src/lite-api/reference.ts +++ b/packages/firestore/src/lite-api/reference.ts @@ -40,14 +40,17 @@ import { FieldValue } from './field_value'; import { FirestoreDataConverter } from './snapshot'; import { NestedUpdateFields, Primitive } from './types'; +/** Alias dynamic document field value types to any */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type DocumentFieldValue = any; + /** * Document data (for use with {@link @firebase/firestore/lite#(setDoc:1)}) consists of fields mapped to * values. */ export interface DocumentData { /** A mapping between a field and its value. */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - [field: string]: any; + [field: string]: DocumentFieldValue; } /** diff --git a/packages/firestore/src/remote/connection.ts b/packages/firestore/src/remote/connection.ts index b6d8828944a..717f2e9dbd2 100644 --- a/packages/firestore/src/remote/connection.ts +++ b/packages/firestore/src/remote/connection.ts @@ -90,7 +90,7 @@ export interface Connection { * request message must include the path. If false, then the request message must NOT * include the path. */ - get shouldResourcePathBeIncludedInRequest(): boolean; + readonly shouldResourcePathBeIncludedInRequest: boolean; // TODO(mcg): subscribe to connection state changes. } diff --git a/packages/firestore/src/remote/datastore.ts b/packages/firestore/src/remote/datastore.ts index 827f05142d4..64661485218 100644 --- a/packages/firestore/src/remote/datastore.ts +++ b/packages/firestore/src/remote/datastore.ts @@ -18,7 +18,6 @@ import { CredentialsProvider } from '../api/credentials'; import { User } from '../auth/user'; import { Query, queryToTarget } from '../core/query'; -import { AggregateQuery } from '../lite-api/aggregate'; import { Document } from '../model/document'; import { DocumentKey } from '../model/document_key'; import { Mutation } from '../model/mutation'; @@ -239,12 +238,12 @@ export async function invokeRunQueryRpc( export async function invokeRunAggregationQueryRpc( datastore: Datastore, - aggregateQuery: AggregateQuery + query: Query ): Promise { const datastoreImpl = debugCast(datastore, DatastoreImpl); const request = toRunAggregationQueryRequest( datastoreImpl.serializer, - queryToTarget(aggregateQuery.query._query) + queryToTarget(query) ); const parent = request.parent; @@ -254,7 +253,7 @@ export async function invokeRunAggregationQueryRpc( const response = await datastoreImpl.invokeStreamingRPC< ProtoRunAggregationQueryRequest, ProtoRunAggregationQueryResponse - >('RunAggregationQuery', parent!, request); + >('RunAggregationQuery', parent!, request, /*expectedResponseCount=*/ 1); return ( response // Omit RunAggregationQueryResponse that only contain readTimes. diff --git a/packages/firestore/test/integration/api/aggregation.test.ts b/packages/firestore/test/integration/api/aggregation.test.ts index 9dedef43090..3065166dfe2 100644 --- a/packages/firestore/test/integration/api/aggregation.test.ts +++ b/packages/firestore/test/integration/api/aggregation.test.ts @@ -20,14 +20,13 @@ import { expect } from 'chai'; import { collection, collectionGroup, - countQuery, doc, disableNetwork, - getAggregateFromServerDirect, query, terminate, where, - writeBatch + writeBatch, + getCountFromServer } from '../util/firebase_export'; import { apiDescribe, @@ -37,36 +36,19 @@ import { withTestDb } from '../util/helpers'; -apiDescribe('Aggregation query', (persistence: boolean) => { - it('can run count query getAggregateFromServerDirect', () => { +apiDescribe('Count query', (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 countQuery_ = countQuery(coll); - const snapshot = await getAggregateFromServerDirect(countQuery_); - expect(snapshot.getCount()).to.equal(2); + const snapshot = await getCountFromServer(coll); + expect(snapshot.data().count).to.equal(2); }); }); - it('aggregateQuery.query equals to original query', () => { - return withEmptyTestCollection(persistence, async coll => { - const query_ = query(coll); - const aggregateQuery_ = countQuery(query_); - expect(aggregateQuery_.query).to.be.equal(query_); - }); - }); - - it('aggregateQuerySnapshot.query equals to aggregateQuery', () => { - return withEmptyTestCollection(persistence, async coll => { - const aggregateQuery_ = countQuery(coll); - const snapshot = await getAggregateFromServerDirect(aggregateQuery_); - expect(snapshot.query).to.be.equal(aggregateQuery_); - }); - }); - - it('aggregate query supports withConverter', () => { + it('count query supports withConverter', () => { const testDocs = { a: { author: 'authorA', title: 'titleA' }, b: { author: 'authorB', title: 'titleB' } @@ -76,13 +58,12 @@ apiDescribe('Aggregation query', (persistence: boolean) => { coll, where('author', '==', 'authorA') ).withConverter(postConverter); - const countQuery_ = countQuery(query_); - const snapshot = await getAggregateFromServerDirect(countQuery_); - expect(snapshot.getCount()).to.equal(1); + const snapshot = await getCountFromServer(query_); + expect(snapshot.data().count).to.equal(1); }); }); - it('aggregate query supports collection groups', () => { + it('count query supports collection groups', () => { return withTestDb(persistence, async db => { const collectionGroupId = doc(collection(db, 'aggregateQueryTest')).id; const docPaths = [ @@ -97,42 +78,35 @@ apiDescribe('Aggregation query', (persistence: boolean) => { batch.set(doc(db, docPath), { x: 1 }); } await batch.commit(); - const countQuery_ = countQuery(collectionGroup(db, collectionGroupId)); - const snapshot = await getAggregateFromServerDirect(countQuery_); - expect(snapshot.getCount()).to.equal(2); + const snapshot = await getCountFromServer( + collectionGroup(db, collectionGroupId) + ); + expect(snapshot.data().count).to.equal(2); }); }); - it('aggregate query fails if firestore is terminated', () => { + it('getCountFromServer fails if firestore is terminated', () => { return withEmptyTestCollection(persistence, async (coll, firestore) => { await terminate(firestore); - const countQuery_ = countQuery(coll); - expect(() => getAggregateFromServerDirect(countQuery_)).to.throw( + expect(() => getCountFromServer(coll)).to.throw( 'The client has already been terminated.' ); }); }); - it("terminate doesn't crash when there is aggregate query in flight", () => { + it("terminate doesn't crash when there is count query in flight", () => { return withEmptyTestCollection(persistence, async (coll, firestore) => { - const countQuery_ = countQuery(coll); - void getAggregateFromServerDirect(countQuery_); + void getCountFromServer(coll); await terminate(firestore); }); }); - it('getAggregateFromServerDirect fails if user is offline', () => { - return withEmptyTestCollection( - persistence, - async (collection, firestore) => { - await disableNetwork(firestore); - const countQuery_ = countQuery(collection); - await expect( - getAggregateFromServerDirect(countQuery_) - ).to.be.eventually.rejectedWith( - 'Failed to get aggregate result because the client is offline' - ); - } - ); + 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' + ); + }); }); }); diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts index 8af92d36a19..2412e330015 100644 --- a/packages/firestore/test/lite/integration.test.ts +++ b/packages/firestore/test/lite/integration.test.ts @@ -20,12 +20,7 @@ import { initializeApp } from '@firebase/app'; import { expect, use } from 'chai'; import chaiAsPromised from 'chai-as-promised'; -import { - countQuery, - getAggregate, - aggregateQueryEqual, - aggregateQuerySnapshotEqual -} from '../../src/lite-api/aggregate'; +import { aggregateQuerySnapshotEqual, getCount } from '../../src/lite-api/aggregate'; import { Bytes } from '../../src/lite-api/bytes'; import { Firestore, @@ -2046,39 +2041,47 @@ describe('withConverter() support', () => { }); describe('countQuery()', () => { - it('AggregateQuery and AggregateQuerySnapshot inherits the original query', () => { + it('AggregateQuerySnapshot inherits the original query', () => { return withTestCollection(async coll => { const query_ = query(coll); - const countQuery_ = countQuery(query_); - const snapshot = await getAggregate(countQuery_); - expect(countQuery_.query).to.equal(query_); - expect(snapshot.query).to.equal(countQuery_); - expect(snapshot.query.query).to.equal(query_); + const snapshot = await getCount(query_); + expect(snapshot.query).to.equal(query_); }); }); - it('empty test collection count', () => { + it('run count query on empty test collection', () => { return withTestCollection(async coll => { - const countQuery_ = countQuery(coll); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(0); + const snapshot = await getCount(coll); + expect(snapshot.data().count).to.equal(0); }); }); - it('test collection count with 3 docs', () => { + it('run count query on test collection with 3 docs', () => { const testDocs = [ { author: 'authorA', title: 'titleA' }, { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; return withTestCollectionAndInitialData(testDocs, async coll => { - const countQuery_ = countQuery(coll); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(3); + const snapshot = await getCount(coll); + expect(snapshot.data().count).to.equal(3); + }); + }); + + it('run count query fails on invalid collection reference', () => { + return withTestDb(async db => { + const queryForRejection = collection(db, '__badpath__'); + try { + await getCount(queryForRejection); + } catch (e) { + expect((e as Error)?.message).to.equal( + 'Request failed with error: Bad Request' + ); + } }); }); - it('test collection count with filter', () => { + it('count query supports filter', () => { const testDocs = [ { author: 'authorA', title: 'titleA' }, { author: 'authorA', title: 'titleB' }, @@ -2086,13 +2089,12 @@ describe('countQuery()', () => { ]; return withTestCollectionAndInitialData(testDocs, async coll => { const query_ = query(coll, where('author', '==', 'authorA')); - const countQuery_ = countQuery(query_); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(2); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); }); }); - it('test collection count with filter and a small limit size', () => { + it('count query supports filter and a small limit size', () => { const testDocs = [ { author: 'authorA', title: 'titleA' }, { author: 'authorA', title: 'titleB' }, @@ -2100,13 +2102,12 @@ describe('countQuery()', () => { ]; return withTestCollectionAndInitialData(testDocs, async coll => { const query_ = query(coll, where('author', '==', 'authorA'), limit(1)); - const countQuery_ = countQuery(query_); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(1); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(1); }); }); - it('test collection count with filter and a large limit size', () => { + it('count query supports filter and a large limit size', () => { const testDocs = [ { author: 'authorA', title: 'titleA' }, { author: 'authorA', title: 'titleB' }, @@ -2114,13 +2115,12 @@ describe('countQuery()', () => { ]; return withTestCollectionAndInitialData(testDocs, async coll => { const query_ = query(coll, where('author', '==', 'authorA'), limit(3)); - const countQuery_ = countQuery(query_); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(2); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); }); }); - it('count with order by', () => { + it('count query supports order by', () => { const testDocs = [ { author: 'authorA', title: 'titleA' }, { author: 'authorA', title: 'titleB' }, @@ -2129,13 +2129,12 @@ describe('countQuery()', () => { ]; return withTestCollectionAndInitialData(testDocs, async coll => { const query_ = query(coll, orderBy('title')); - const countQuery_ = countQuery(query_); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(3); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(3); }); }); - it('count with order by and startAt', () => { + it('count query supports order by and startAt', () => { const testDocs = [ { id: 3, author: 'authorA', title: 'titleA' }, { id: 1, author: 'authorA', title: 'titleB' }, @@ -2144,13 +2143,12 @@ describe('countQuery()', () => { ]; return withTestCollectionAndInitialData(testDocs, async coll => { const query_ = query(coll, orderBy('id'), startAt(2)); - const countQuery_ = countQuery(query_); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(2); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); }); }); - it('count with order by and startAfter', () => { + it('count query supports order by and startAfter', () => { const testDocs = [ { id: 3, author: 'authorA', title: 'titleA' }, { id: 1, author: 'authorA', title: 'titleB' }, @@ -2159,13 +2157,12 @@ describe('countQuery()', () => { ]; return withTestCollectionAndInitialData(testDocs, async coll => { const query_ = query(coll, orderBy('id'), startAfter(2)); - const countQuery_ = countQuery(query_); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(1); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(1); }); }); - it('count with order by and endAt', () => { + it('count query supports order by and endAt', () => { const testDocs = [ { id: 3, author: 'authorA', title: 'titleA' }, { id: 1, author: 'authorA', title: 'titleB' }, @@ -2174,13 +2171,12 @@ describe('countQuery()', () => { ]; return withTestCollectionAndInitialData(testDocs, async coll => { const query_ = query(coll, orderBy('id'), startAt(1), endAt(2)); - const countQuery_ = countQuery(query_); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(2); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); }); }); - it('count with order by and endBefore', () => { + it('count query supports order by and endBefore', () => { const testDocs = [ { id: 3, author: 'authorA', title: 'titleA' }, { id: 1, author: 'authorA', title: 'titleB' }, @@ -2189,13 +2185,12 @@ describe('countQuery()', () => { ]; return withTestCollectionAndInitialData(testDocs, async coll => { const query_ = query(coll, orderBy('id'), startAt(1), endBefore(2)); - const countQuery_ = countQuery(query_); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(1); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(1); }); }); - it('test collection count with converter on query', () => { + it('count query supports converter', () => { const testDocs = [ { author: 'authorA', title: 'titleA' }, { author: 'authorA', title: 'titleB' }, @@ -2206,13 +2201,12 @@ describe('countQuery()', () => { coll, where('author', '==', 'authorA') ).withConverter(postConverter); - const countQuery_ = countQuery(query_); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(2); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); }); }); - it('count query with collection groups', () => { + it('count query supports collection groups', () => { return withTestDb(async db => { const collectionGroupId = doc(collection(db, 'countTest')).id; const docPaths = [ @@ -2227,43 +2221,14 @@ describe('countQuery()', () => { batch.set(doc(db, docPath), { x: 1 }); } await batch.commit(); - const countQuery_ = countQuery(collectionGroup(db, collectionGroupId)); - const snapshot = await getAggregate(countQuery_); - expect(snapshot.getCount()).to.equal(2); - }); - }); - - it('aggregateQueryEqual on same queries', () => { - const testDocs = [ - { author: 'authorA', title: 'titleA' }, - { author: 'authorA', title: 'titleB' }, - { author: 'authorB', title: 'titleC' } - ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query1 = query(coll, where('author', '==', 'authorA')); - const query2 = query(coll, where('author', '==', 'authorA')); - const countQuery1 = countQuery(query1); - const countQuery2 = countQuery(query2); - expect(aggregateQueryEqual(countQuery1, countQuery2)).to.be.true; - }); - }); - - it('aggregateQueryEqual on different queries', () => { - const testDocs = [ - { author: 'authorA', title: 'titleA' }, - { author: 'authorA', title: 'titleB' }, - { author: 'authorB', title: 'titleC' } - ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query1 = query(coll, where('author', '==', 'authorA')); - const query2 = query(coll, where('author', '==', 'authorB')); - const countQuery1 = countQuery(query1); - const countQuery2 = countQuery(query2); - expect(aggregateQueryEqual(countQuery1, countQuery2)).to.be.false; + const snapshot = await getCount( + query(collectionGroup(db, collectionGroupId)) + ); + expect(snapshot.data().count).to.equal(2); }); }); - it('aggregateQuerySnapshotEqual on same queries', () => { + it('aggregateQuerySnapshotEqual on same queries be truthy', () => { const testDocs = [ { author: 'authorA', title: 'titleA' }, { author: 'authorA', title: 'titleB' }, @@ -2272,56 +2237,50 @@ describe('countQuery()', () => { return withTestCollectionAndInitialData(testDocs, async coll => { const query1 = query(coll, where('author', '==', 'authorA')); const query2 = query(coll, where('author', '==', 'authorA')); - const countQuery1A = countQuery(query1); - const countQuery1B = countQuery(query1); - const countQuery2 = countQuery(query2); - const snapshot1A = await getAggregate(countQuery1A); - const snapshot1B = await getAggregate(countQuery1B); - const snapshot2 = await getAggregate(countQuery2); + const snapshot1A = await getCount(query1); + const snapshot1B = await getCount(query1); + const snapshot2 = await getCount(query2); expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot1B)).to.be.true; expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot2)).to.be.true; }); }); - it('aggregateQuerySnapshotEqual on different queries', () => { + it('aggregateQuerySnapshotEqual on different queries be falsy', () => { const testDocs = [ { author: 'authorA', title: 'titleA' }, { author: 'authorA', title: 'titleB' }, - { author: 'authorB', title: 'titleC' } + { author: 'authorB', title: 'titleC' }, + { author: 'authorB', title: 'titleD' } ]; return withTestCollectionAndInitialData(testDocs, async coll => { const query1 = query(coll, where('author', '==', 'authorA')); const query2 = query(coll, where('author', '==', 'authorB')); - const countQuery1 = countQuery(query1); - const countQuery2 = countQuery(query2); - const snapshot1 = await getAggregate(countQuery1); - const snapshot2 = await getAggregate(countQuery2); + const snapshot1 = await getCount(query1); + const snapshot2 = await getCount(query2); expect(aggregateQuerySnapshotEqual(snapshot1, snapshot2)).to.be.false; }); }); - it('count query on a terminated Firestore', () => { + it('count query fails on a terminated Firestore', () => { return withTestCollection(async coll => { await terminate(coll.firestore); - const countQuery_ = countQuery(coll); - expect(() => getAggregate(countQuery_)).to.throw( + expect(() => getCount(coll)).to.throw( 'The client has already been terminated.' ); }); }); - it('terminate Firestore while calling count query', () => { + it('terminate Firestore not effect count query in flight', () => { const testDocs = [ { author: 'authorA', title: 'titleA' }, { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; return withTestCollectionAndInitialData(testDocs, async coll => { - const countQuery_ = countQuery(coll); - const promise = getAggregate(countQuery_); + const promise = getCount(coll); await terminate(coll.firestore); const snapshot = await promise; - expect(snapshot.getCount()).to.equal(3); + expect(snapshot.data().count).to.equal(3); }); }); }); diff --git a/packages/firestore/test/unit/remote/datastore.test.ts b/packages/firestore/test/unit/remote/datastore.test.ts index f57fcf6f38d..795bf59edaf 100644 --- a/packages/firestore/test/unit/remote/datastore.test.ts +++ b/packages/firestore/test/unit/remote/datastore.test.ts @@ -65,6 +65,8 @@ describe('Datastore', () => { ): Stream { throw new Error('MockConnection.openStream() must be replaced'); } + + shouldResourcePathBeIncludedInRequest: boolean = false; } class MockAuthCredentialsProvider extends EmptyAuthCredentialsProvider { diff --git a/packages/firestore/test/unit/specs/spec_test_components.ts b/packages/firestore/test/unit/specs/spec_test_components.ts index ab63a4576b5..1436efa3fcd 100644 --- a/packages/firestore/test/unit/specs/spec_test_components.ts +++ b/packages/firestore/test/unit/specs/spec_test_components.ts @@ -253,6 +253,8 @@ export class MockConnection implements Connection { constructor(private queue: AsyncQueue) {} + shouldResourcePathBeIncludedInRequest: boolean = false; + /** * Tracks the currently active watch targets as detected by the mock watch * stream, as a mapping from target ID to query Target. diff --git a/repo-scripts/prune-dts/tests/firestore.output.d.ts b/repo-scripts/prune-dts/tests/firestore.output.d.ts index 5b4945c9b27..ef6ec6cd71e 100644 --- a/repo-scripts/prune-dts/tests/firestore.output.d.ts +++ b/repo-scripts/prune-dts/tests/firestore.output.d.ts @@ -341,12 +341,15 @@ export declare interface DocumentChange { * The type of a `DocumentChange` may be 'added', 'removed', or 'modified'. */ export declare type DocumentChangeType = 'added' | 'removed' | 'modified'; + +/** Alias dynamic document field value types to any */ +export declare type DocumentFieldValue = any; /** * Document data (for use with {@link setDoc}) consists of fields mapped to * values. */ export declare interface DocumentData { - [field: string]: any; + [field: string]: DocumentFieldValue; } /** * Returns a special sentinel `FieldPath` to refer to the ID of a document. From 4b273fec9156948377970041ce9c1efb5ea20002 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 13 Sep 2022 14:22:53 -0700 Subject: [PATCH 13/34] remove DocumentFieldValue --- packages/firebase/compat/index.d.ts | 6 +----- packages/firestore-types/index.d.ts | 4 +--- packages/firestore/lite/index.ts | 1 - packages/firestore/src/lite-api/reference.ts | 7 ++----- packages/firestore/test/lite/integration.test.ts | 5 ++++- repo-scripts/prune-dts/tests/firestore.output.d.ts | 4 +--- 6 files changed, 9 insertions(+), 18 deletions(-) diff --git a/packages/firebase/compat/index.d.ts b/packages/firebase/compat/index.d.ts index c5c2803b767..a946c38ec9a 100644 --- a/packages/firebase/compat/index.d.ts +++ b/packages/firebase/compat/index.d.ts @@ -8227,15 +8227,11 @@ declare namespace firebase.storage { } declare namespace firebase.firestore { - - /** Alias dynamic document field value types to any */ - export type DocumentFieldValue = any; - /** * Document data (for use with `DocumentReference.set()`) consists of fields * mapped to values. */ - export type DocumentData = { [field: string]: DocumentFieldValue }; + export type DocumentData = { [field: string]: any }; /** * Update data (for use with `DocumentReference.update()`) consists of field diff --git a/packages/firestore-types/index.d.ts b/packages/firestore-types/index.d.ts index 603bb9c22c3..d198fd78866 100644 --- a/packages/firestore-types/index.d.ts +++ b/packages/firestore-types/index.d.ts @@ -17,9 +17,7 @@ import { EmulatorMockTokenOptions } from '@firebase/util'; -export type DocumentFieldValue = any; - -export type DocumentData = { [field: string]: DocumentFieldValue }; +export type DocumentData = { [field: string]: any }; export type UpdateData = { [fieldPath: string]: any }; diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index ec31973fe25..c85a9e89cfc 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -29,7 +29,6 @@ registerFirestore(); export { FirestoreSettings as Settings } from '../src/lite-api/settings'; - export { AggregateField, AggregateSpec, diff --git a/packages/firestore/src/lite-api/reference.ts b/packages/firestore/src/lite-api/reference.ts index 11e6d0a140f..7ade1ec0714 100644 --- a/packages/firestore/src/lite-api/reference.ts +++ b/packages/firestore/src/lite-api/reference.ts @@ -40,17 +40,14 @@ import { FieldValue } from './field_value'; import { FirestoreDataConverter } from './snapshot'; import { NestedUpdateFields, Primitive } from './types'; -/** Alias dynamic document field value types to any */ -// eslint-disable-next-line @typescript-eslint/no-explicit-any -export type DocumentFieldValue = any; - /** * Document data (for use with {@link @firebase/firestore/lite#(setDoc:1)}) consists of fields mapped to * values. */ export interface DocumentData { /** A mapping between a field and its value. */ - [field: string]: DocumentFieldValue; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [field: string]: any; } /** diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts index 2412e330015..1512bb7bd75 100644 --- a/packages/firestore/test/lite/integration.test.ts +++ b/packages/firestore/test/lite/integration.test.ts @@ -20,7 +20,10 @@ import { initializeApp } from '@firebase/app'; import { expect, use } from 'chai'; import chaiAsPromised from 'chai-as-promised'; -import { aggregateQuerySnapshotEqual, getCount } from '../../src/lite-api/aggregate'; +import { + aggregateQuerySnapshotEqual, + getCount +} from '../../src/lite-api/aggregate'; import { Bytes } from '../../src/lite-api/bytes'; import { Firestore, diff --git a/repo-scripts/prune-dts/tests/firestore.output.d.ts b/repo-scripts/prune-dts/tests/firestore.output.d.ts index ef6ec6cd71e..2eeacb4e23c 100644 --- a/repo-scripts/prune-dts/tests/firestore.output.d.ts +++ b/repo-scripts/prune-dts/tests/firestore.output.d.ts @@ -342,14 +342,12 @@ export declare interface DocumentChange { */ export declare type DocumentChangeType = 'added' | 'removed' | 'modified'; -/** Alias dynamic document field value types to any */ -export declare type DocumentFieldValue = any; /** * Document data (for use with {@link setDoc}) consists of fields mapped to * values. */ export declare interface DocumentData { - [field: string]: DocumentFieldValue; + [field: string]: any; } /** * Returns a special sentinel `FieldPath` to refer to the ID of a document. From 576d2027b3d1b7643bf81d1c38f59b0d52cf12ba Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 13 Sep 2022 14:46:00 -0700 Subject: [PATCH 14/34] fix lint error: Symbol not found for identifier --- common/api-review/firestore-lite.api.md | 40 ++++++++++++++++++++ common/api-review/firestore.api.md | 39 ++++++++++++------- packages/firestore/lite/index.ts | 4 +- packages/firestore/src/api.ts | 8 ++-- packages/firestore/src/api/aggregate.ts | 6 ++- packages/firestore/src/lite-api/aggregate.ts | 2 +- 6 files changed, 78 insertions(+), 21 deletions(-) diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index 312db6e00ea..52b8947344c 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -17,6 +17,38 @@ export type AddPrefixToKeys { + // (undocumented) + type: string; +} + +// @public +export type AggregateFieldType = ReturnType; + +// @public +export class AggregateQuerySnapshot { + data(): AggregateSpecData; + // (undocumented) + readonly query: Query; + // (undocumented) + readonly type = "AggregateQuerySnapshot"; +} + +// @public +export function aggregateQuerySnapshotEqual(left: AggregateQuerySnapshot, right: AggregateQuerySnapshot): boolean; + +// @public +export interface AggregateSpec { + // (undocumented) + [field: string]: AggregateFieldType; +} + +// @public +export type AggregateSpecData = { + [P in keyof T]: T[P] extends AggregateField ? U : never; +}; + // @public export function arrayRemove(...elements: unknown[]): FieldValue; @@ -63,6 +95,9 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por mockUserToken?: EmulatorMockTokenOptions | string; }): void; +// @public +export function count(): AggregateField; + // @public export function deleteDoc(reference: DocumentReference): Promise; @@ -169,6 +204,11 @@ export class GeoPoint { }; } +// @public +export function getCount(query: Query): Promise; +}>>; + // @public export function getDoc(reference: DocumentReference): Promise>; diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index bab6b7de379..6fa63aa0d90 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -18,27 +18,36 @@ export type AddPrefixToKeys; +export class AggregateField { // (undocumented) - readonly type = "AggregateQuery"; + type: string; } -// @public (undocumented) -export function aggregateQueryEqual(left: AggregateQuery, right: AggregateQuery): boolean; +// @public +export type AggregateFieldType = ReturnType; // @public -export class AggregateQuerySnapshot { +export class AggregateQuerySnapshot { + data(): AggregateSpecData; // (undocumented) - getCount(): number | null; - // (undocumented) - readonly query: AggregateQuery; + readonly query: Query; // (undocumented) readonly type = "AggregateQuerySnapshot"; } -// @public (undocumented) -export function aggregateQuerySnapshotEqual(left: AggregateQuerySnapshot, right: AggregateQuerySnapshot): boolean; +// @public +export function aggregateQuerySnapshotEqual(left: AggregateQuerySnapshot, right: AggregateQuerySnapshot): boolean; + +// @public +export interface AggregateSpec { + // (undocumented) + [field: string]: AggregateFieldType; +} + +// @public +export type AggregateSpecData = { + [P in keyof T]: T[P] extends AggregateField ? U : never; +}; // @public export function arrayRemove(...elements: unknown[]): FieldValue; @@ -93,7 +102,7 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por }): void; // @public -export function countQuery(query: Query): AggregateQuery; +export function count(): AggregateField; // @public export function deleteDoc(reference: DocumentReference): Promise; @@ -235,8 +244,10 @@ export class GeoPoint { }; } -// @public (undocumented) -export function getAggregateFromServerDirect(query: AggregateQuery): Promise; +// @public +export function getCountFromServer(query: Query): Promise; +}>>; // @public export function getDoc(reference: DocumentReference): Promise>; diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index c85a9e89cfc..4da055235e3 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -31,11 +31,13 @@ export { FirestoreSettings as Settings } from '../src/lite-api/settings'; export { AggregateField, + AggregateFieldType, AggregateSpec, AggregateSpecData, AggregateQuerySnapshot, getCount, - aggregateQuerySnapshotEqual + aggregateQuerySnapshotEqual, + count } from '../src/lite-api/aggregate'; export { diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 4b7104a664b..310b346829f 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -17,11 +17,13 @@ export { AggregateField, + AggregateFieldType, + AggregateQuerySnapshot, AggregateSpec, AggregateSpecData, - AggregateQuerySnapshot, - getCountFromServer, - aggregateQuerySnapshotEqual + aggregateQuerySnapshotEqual, + count, + getCountFromServer } from './api/aggregate'; export { FieldPath, documentId } from './api/field_path'; diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts index 2d342b4d4d2..67ef39609d8 100644 --- a/packages/firestore/src/api/aggregate.ts +++ b/packages/firestore/src/api/aggregate.ts @@ -23,10 +23,12 @@ import { ensureFirestoreConfigured, Firestore } from './database'; export { AggregateField, + AggregateFieldType, + AggregateQuerySnapshot, + aggregateQuerySnapshotEqual, AggregateSpec, AggregateSpecData, - AggregateQuerySnapshot, - aggregateQuerySnapshotEqual + count } from '../lite-api/aggregate'; /** diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts index a4156f823bd..5fb9d7a7255 100644 --- a/packages/firestore/src/lite-api/aggregate.ts +++ b/packages/firestore/src/lite-api/aggregate.ts @@ -47,7 +47,7 @@ export function count(): AggregateField { * The union of all `AggregateField` types that are returned from the factory * functions. */ -type AggregateFieldType = ReturnType; +export type AggregateFieldType = ReturnType; /** * A type whose values are all `AggregateField` objects. From dd693c6576c6a34bca080af8d1aa8fa3b022bbb4 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 13 Sep 2022 15:11:43 -0700 Subject: [PATCH 15/34] format to pass lint check --- packages/firestore/lite/index.ts | 8 ++++---- packages/firestore/src/api.ts | 2 +- packages/firestore/src/api/aggregate.ts | 1 + packages/firestore/src/remote/rest_connection.ts | 1 + repo-scripts/prune-dts/tests/firestore.output.d.ts | 1 - 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index 4da055235e3..044e320a7ce 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -32,12 +32,12 @@ export { FirestoreSettings as Settings } from '../src/lite-api/settings'; export { AggregateField, AggregateFieldType, - AggregateSpec, - AggregateSpecData, AggregateQuerySnapshot, - getCount, aggregateQuerySnapshotEqual, - count + AggregateSpec, + AggregateSpecData, + count, + getCount } from '../src/lite-api/aggregate'; export { diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 310b346829f..9740b9aa21f 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -19,9 +19,9 @@ export { AggregateField, AggregateFieldType, AggregateQuerySnapshot, + aggregateQuerySnapshotEqual, AggregateSpec, AggregateSpecData, - aggregateQuerySnapshotEqual, count, getCountFromServer } from './api/aggregate'; diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts index 67ef39609d8..0f4cbcb6c6f 100644 --- a/packages/firestore/src/api/aggregate.ts +++ b/packages/firestore/src/api/aggregate.ts @@ -14,6 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { Query } from '../api'; import { firestoreClientRunCountQuery } from '../core/firestore_client'; import { AggregateField, AggregateQuerySnapshot } from '../lite-api/aggregate'; diff --git a/packages/firestore/src/remote/rest_connection.ts b/packages/firestore/src/remote/rest_connection.ts index b2e4e5b057f..ee8b7ebcc39 100644 --- a/packages/firestore/src/remote/rest_connection.ts +++ b/packages/firestore/src/remote/rest_connection.ts @@ -83,6 +83,7 @@ export abstract class RestConnection implements Connection { const headers = {}; this.modifyHeadersForRequest(headers, authToken, appCheckToken); + return this.performRPCRequest(rpcName, url, headers, req).then( response => { logDebug(LOG_TAG, 'Received: ', response); diff --git a/repo-scripts/prune-dts/tests/firestore.output.d.ts b/repo-scripts/prune-dts/tests/firestore.output.d.ts index 2eeacb4e23c..5b4945c9b27 100644 --- a/repo-scripts/prune-dts/tests/firestore.output.d.ts +++ b/repo-scripts/prune-dts/tests/firestore.output.d.ts @@ -341,7 +341,6 @@ export declare interface DocumentChange { * The type of a `DocumentChange` may be 'added', 'removed', or 'modified'. */ export declare type DocumentChangeType = 'added' | 'removed' | 'modified'; - /** * Document data (for use with {@link setDoc}) consists of fields mapped to * values. From 83fbc8d1a84298f14960075ec07d886545596632 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 14 Sep 2022 09:57:52 -0700 Subject: [PATCH 16/34] remove the exports of aggregate query functions/types --- common/api-review/firestore-lite.api.md | 40 ------------------- common/api-review/firestore.api.md | 40 ------------------- packages/firestore/lite/index.ts | 11 ----- packages/firestore/src/api.ts | 11 ----- .../test/integration/api/aggregation.test.ts | 2 +- 5 files changed, 1 insertion(+), 103 deletions(-) diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index 52b8947344c..312db6e00ea 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -17,38 +17,6 @@ export type AddPrefixToKeys { - // (undocumented) - type: string; -} - -// @public -export type AggregateFieldType = ReturnType; - -// @public -export class AggregateQuerySnapshot { - data(): AggregateSpecData; - // (undocumented) - readonly query: Query; - // (undocumented) - readonly type = "AggregateQuerySnapshot"; -} - -// @public -export function aggregateQuerySnapshotEqual(left: AggregateQuerySnapshot, right: AggregateQuerySnapshot): boolean; - -// @public -export interface AggregateSpec { - // (undocumented) - [field: string]: AggregateFieldType; -} - -// @public -export type AggregateSpecData = { - [P in keyof T]: T[P] extends AggregateField ? U : never; -}; - // @public export function arrayRemove(...elements: unknown[]): FieldValue; @@ -95,9 +63,6 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por mockUserToken?: EmulatorMockTokenOptions | string; }): void; -// @public -export function count(): AggregateField; - // @public export function deleteDoc(reference: DocumentReference): Promise; @@ -204,11 +169,6 @@ export class GeoPoint { }; } -// @public -export function getCount(query: Query): Promise; -}>>; - // @public export function getDoc(reference: DocumentReference): Promise>; diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 6fa63aa0d90..3b6538d867b 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -17,38 +17,6 @@ export type AddPrefixToKeys { - // (undocumented) - type: string; -} - -// @public -export type AggregateFieldType = ReturnType; - -// @public -export class AggregateQuerySnapshot { - data(): AggregateSpecData; - // (undocumented) - readonly query: Query; - // (undocumented) - readonly type = "AggregateQuerySnapshot"; -} - -// @public -export function aggregateQuerySnapshotEqual(left: AggregateQuerySnapshot, right: AggregateQuerySnapshot): boolean; - -// @public -export interface AggregateSpec { - // (undocumented) - [field: string]: AggregateFieldType; -} - -// @public -export type AggregateSpecData = { - [P in keyof T]: T[P] extends AggregateField ? U : never; -}; - // @public export function arrayRemove(...elements: unknown[]): FieldValue; @@ -101,9 +69,6 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por mockUserToken?: EmulatorMockTokenOptions | string; }): void; -// @public -export function count(): AggregateField; - // @public export function deleteDoc(reference: DocumentReference): Promise; @@ -244,11 +209,6 @@ export class GeoPoint { }; } -// @public -export function getCountFromServer(query: Query): Promise; -}>>; - // @public export function getDoc(reference: DocumentReference): Promise>; diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index 044e320a7ce..ad7ec7d8294 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -29,17 +29,6 @@ registerFirestore(); export { FirestoreSettings as Settings } from '../src/lite-api/settings'; -export { - AggregateField, - AggregateFieldType, - AggregateQuerySnapshot, - aggregateQuerySnapshotEqual, - AggregateSpec, - AggregateSpecData, - count, - getCount -} from '../src/lite-api/aggregate'; - export { Firestore as Firestore, EmulatorMockTokenOptions, diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 9740b9aa21f..544d275fc30 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -15,17 +15,6 @@ * limitations under the License. */ -export { - AggregateField, - AggregateFieldType, - AggregateQuerySnapshot, - aggregateQuerySnapshotEqual, - AggregateSpec, - AggregateSpecData, - count, - getCountFromServer -} from './api/aggregate'; - export { FieldPath, documentId } from './api/field_path'; export { diff --git a/packages/firestore/test/integration/api/aggregation.test.ts b/packages/firestore/test/integration/api/aggregation.test.ts index 3065166dfe2..ba0fef3003d 100644 --- a/packages/firestore/test/integration/api/aggregation.test.ts +++ b/packages/firestore/test/integration/api/aggregation.test.ts @@ -16,6 +16,7 @@ */ import { expect } from 'chai'; +import { getCountFromServer } from '../../../src/api/aggregate'; import { collection, @@ -26,7 +27,6 @@ import { terminate, where, writeBatch, - getCountFromServer } from '../util/firebase_export'; import { apiDescribe, From 26a0bd2053782eb14a556bd60728936a9e0b8726 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 14 Sep 2022 10:04:02 -0700 Subject: [PATCH 17/34] Update aggregation.test.ts --- packages/firestore/test/integration/api/aggregation.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/integration/api/aggregation.test.ts b/packages/firestore/test/integration/api/aggregation.test.ts index ba0fef3003d..da60abb5e1e 100644 --- a/packages/firestore/test/integration/api/aggregation.test.ts +++ b/packages/firestore/test/integration/api/aggregation.test.ts @@ -26,7 +26,7 @@ import { query, terminate, where, - writeBatch, + writeBatch } from '../util/firebase_export'; import { apiDescribe, From 9cced282a489b2151a573d7de1cc6867618040a0 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 14 Sep 2022 15:18:10 -0700 Subject: [PATCH 18/34] skip count query test cases if not using emulator --- .../test/integration/api/aggregation.test.ts | 97 ++++--- .../test/integration/util/helpers.ts | 24 +- packages/firestore/test/lite/helpers.ts | 19 +- .../firestore/test/lite/integration.test.ts | 273 ++++++++++-------- 4 files changed, 250 insertions(+), 163 deletions(-) diff --git a/packages/firestore/test/integration/api/aggregation.test.ts b/packages/firestore/test/integration/api/aggregation.test.ts index da60abb5e1e..a992f972a17 100644 --- a/packages/firestore/test/integration/api/aggregation.test.ts +++ b/packages/firestore/test/integration/api/aggregation.test.ts @@ -16,8 +16,8 @@ */ import { expect } from 'chai'; -import { getCountFromServer } from '../../../src/api/aggregate'; +import { getCountFromServer } from '../../../src/api/aggregate'; import { collection, collectionGroup, @@ -33,7 +33,8 @@ import { postConverter, withEmptyTestCollection, withTestCollection, - withTestDb + withTestDb, + skipTestUnlessUsingEmulator } from '../util/helpers'; apiDescribe('Count query', (persistence: boolean) => { @@ -42,10 +43,12 @@ apiDescribe('Count query', (persistence: boolean) => { 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); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollection(persistence, testDocs, async coll => { + const snapshot = await getCountFromServer(coll); + expect(snapshot.data().count).to.equal(2); + }) + ); }); it('count query supports withConverter', () => { @@ -53,36 +56,40 @@ apiDescribe('Count query', (persistence: boolean) => { a: { author: 'authorA', title: 'titleA' }, b: { author: 'authorB', title: 'titleB' } }; - return withTestCollection(persistence, testDocs, async coll => { - const query_ = query( - coll, - where('author', '==', 'authorA') - ).withConverter(postConverter); - const snapshot = await getCountFromServer(query_); - expect(snapshot.data().count).to.equal(1); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollection(persistence, testDocs, async coll => { + const query_ = query( + coll, + where('author', '==', 'authorA') + ).withConverter(postConverter); + const snapshot = await getCountFromServer(query_); + expect(snapshot.data().count).to.equal(1); + }) + ); }); it('count query supports collection groups', () => { - return withTestDb(persistence, async db => { - const collectionGroupId = doc(collection(db, 'aggregateQueryTest')).id; - const docPaths = [ - `${collectionGroupId}/cg-doc1`, - `abc/123/${collectionGroupId}/cg-doc2`, - `zzz${collectionGroupId}/cg-doc3`, - `abc/123/zzz${collectionGroupId}/cg-doc4`, - `abc/123/zzz/${collectionGroupId}` - ]; - const batch = writeBatch(db); - for (const docPath of docPaths) { - batch.set(doc(db, docPath), { x: 1 }); - } - await batch.commit(); - const snapshot = await getCountFromServer( - collectionGroup(db, collectionGroupId) - ); - expect(snapshot.data().count).to.equal(2); - }); + return skipTestUnlessUsingEmulator(() => + withTestDb(persistence, async db => { + const collectionGroupId = doc(collection(db, 'aggregateQueryTest')).id; + const docPaths = [ + `${collectionGroupId}/cg-doc1`, + `abc/123/${collectionGroupId}/cg-doc2`, + `zzz${collectionGroupId}/cg-doc3`, + `abc/123/zzz${collectionGroupId}/cg-doc4`, + `abc/123/zzz/${collectionGroupId}` + ]; + const batch = writeBatch(db); + for (const docPath of docPaths) { + batch.set(doc(db, docPath), { x: 1 }); + } + await batch.commit(); + const snapshot = await getCountFromServer( + collectionGroup(db, collectionGroupId) + ); + expect(snapshot.data().count).to.equal(2); + }) + ); }); it('getCountFromServer fails if firestore is terminated', () => { @@ -95,18 +102,22 @@ apiDescribe('Count query', (persistence: boolean) => { }); it("terminate doesn't crash when there is count query in flight", () => { - return withEmptyTestCollection(persistence, async (coll, firestore) => { - void getCountFromServer(coll); - await terminate(firestore); - }); + return skipTestUnlessUsingEmulator(() => + withEmptyTestCollection(persistence, async (coll, firestore) => { + void getCountFromServer(coll); + await terminate(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' - ); - }); + return skipTestUnlessUsingEmulator(() => + 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' + ); + }) + ); }); }); diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index c5195515bc5..9be611ce4f9 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -16,6 +16,7 @@ */ import { isIndexedDBAvailable } from '@firebase/util'; +import { Code } from '../../../src/util/error'; import { collection, @@ -33,7 +34,8 @@ import { SnapshotListenOptions, newTestFirestore, QueryDocumentSnapshot, - newTestApp + newTestApp, + FirestoreError } from './firebase_export'; import { ALT_PROJECT_ID, @@ -238,6 +240,26 @@ export async function withNamedTestDbsOrSkipUnlessUsingEmulator( } } +export function skipTestUnlessUsingEmulator< + T extends (...params: any[]) => any +>(fn: T, ...params: Parameters): Promise { + /** + * This is a wrapper function to execute test cases that can only run on emulator till + * production test environment is ready. + */ + if (!USE_EMULATOR) { + return Promise.resolve(); + } + + return fn(...params).catch((error: FirestoreError) => { + if (error.name === 'FirebaseError') { + throw error; + } else { + throw new FirestoreError(Code.UNKNOWN, error.toString()); + } + }); +} + export function withTestDoc( persistence: boolean, fn: (doc: DocumentReference, db: Firestore) => Promise diff --git a/packages/firestore/test/lite/helpers.ts b/packages/firestore/test/lite/helpers.ts index e51c7c606ab..b710da6289e 100644 --- a/packages/firestore/test/lite/helpers.ts +++ b/packages/firestore/test/lite/helpers.ts @@ -35,7 +35,8 @@ import { QueryDocumentSnapshot } from '../../src/lite-api/snapshot'; import { AutoId } from '../../src/util/misc'; import { DEFAULT_PROJECT_ID, - DEFAULT_SETTINGS + DEFAULT_SETTINGS, + USE_EMULATOR } from '../integration/util/settings'; let appCount = 0; @@ -101,6 +102,22 @@ export function withTestCollection( }); } +export function skipTestUnlessUsingEmulator< + T extends (...params: any[]) => any +>(fn: T, ...params: Parameters): Promise { + /** + * This is a wrapper function to execute test cases that can only run on emulator till + * production test environment is ready. + */ + if (!USE_EMULATOR) { + return Promise.resolve(); + } + + return fn(...params).catch((error: any) => { + throw error; + }); +} + // Used for testing the FirestoreDataConverter. export class Post { constructor( diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts index ea3e54a12bc..9c4826f2c08 100644 --- a/packages/firestore/test/lite/integration.test.ts +++ b/packages/firestore/test/lite/integration.test.ts @@ -90,6 +90,7 @@ import { Post, postConverter, postConverterMerge, + skipTestUnlessUsingEmulator, withTestCollection, withTestCollectionAndInitialData, withTestDb, @@ -2120,18 +2121,22 @@ describe('withConverter() support', () => { describe('countQuery()', () => { it('AggregateQuerySnapshot inherits the original query', () => { - return withTestCollection(async coll => { - const query_ = query(coll); - const snapshot = await getCount(query_); - expect(snapshot.query).to.equal(query_); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollection(async coll => { + const query_ = query(coll); + const snapshot = await getCount(query_); + expect(snapshot.query).to.equal(query_); + }) + ); }); it('run count query on empty test collection', () => { - return withTestCollection(async coll => { - const snapshot = await getCount(coll); - expect(snapshot.data().count).to.equal(0); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollection(async coll => { + const snapshot = await getCount(coll); + expect(snapshot.data().count).to.equal(0); + }) + ); }); it('run count query on test collection with 3 docs', () => { @@ -2140,23 +2145,27 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const snapshot = await getCount(coll); - expect(snapshot.data().count).to.equal(3); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const snapshot = await getCount(coll); + expect(snapshot.data().count).to.equal(3); + }) + ); }); it('run count query fails on invalid collection reference', () => { - return withTestDb(async db => { - const queryForRejection = collection(db, '__badpath__'); - try { - await getCount(queryForRejection); - } catch (e) { - expect((e as Error)?.message).to.equal( - 'Request failed with error: Bad Request' - ); - } - }); + return skipTestUnlessUsingEmulator(() => + withTestDb(async db => { + const queryForRejection = collection(db, '__badpath__'); + try { + await getCount(queryForRejection); + } catch (e) { + expect((e as Error)?.message).to.equal( + 'Request failed with error: Bad Request' + ); + } + }) + ); }); it('count query supports filter', () => { @@ -2165,11 +2174,13 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, where('author', '==', 'authorA')); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(2); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, where('author', '==', 'authorA')); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); + }) + ); }); it('count query supports filter and a small limit size', () => { @@ -2178,11 +2189,13 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, where('author', '==', 'authorA'), limit(1)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(1); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, where('author', '==', 'authorA'), limit(1)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(1); + }) + ); }); it('count query supports filter and a large limit size', () => { @@ -2191,11 +2204,13 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, where('author', '==', 'authorA'), limit(3)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(2); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, where('author', '==', 'authorA'), limit(3)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); + }) + ); }); it('count query supports order by', () => { @@ -2205,11 +2220,13 @@ describe('countQuery()', () => { { author: 'authorB', title: null }, { author: 'authorB' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, orderBy('title')); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(3); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('title')); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(3); + }) + ); }); it('count query supports order by and startAt', () => { @@ -2219,11 +2236,13 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, orderBy('id'), startAt(2)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(2); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAt(2)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); + }) + ); }); it('count query supports order by and startAfter', () => { @@ -2233,11 +2252,13 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, orderBy('id'), startAfter(2)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(1); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAfter(2)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(1); + }) + ); }); it('count query supports order by and endAt', () => { @@ -2247,11 +2268,13 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, orderBy('id'), startAt(1), endAt(2)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(2); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAt(1), endAt(2)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); + }) + ); }); it('count query supports order by and endBefore', () => { @@ -2261,11 +2284,13 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, orderBy('id'), startAt(1), endBefore(2)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(1); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAt(1), endBefore(2)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(1); + }) + ); }); it('count query supports converter', () => { @@ -2274,36 +2299,40 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query( - coll, - where('author', '==', 'authorA') - ).withConverter(postConverter); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(2); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query( + coll, + where('author', '==', 'authorA') + ).withConverter(postConverter); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); + }) + ); }); it('count query supports collection groups', () => { - return withTestDb(async db => { - const collectionGroupId = doc(collection(db, 'countTest')).id; - const docPaths = [ - `${collectionGroupId}/cg-doc1`, - `abc/123/${collectionGroupId}/cg-doc2`, - `zzz${collectionGroupId}/cg-doc3`, - `abc/123/zzz${collectionGroupId}/cg-doc4`, - `abc/123/zzz/${collectionGroupId}` - ]; - const batch = writeBatch(db); - for (const docPath of docPaths) { - batch.set(doc(db, docPath), { x: 1 }); - } - await batch.commit(); - const snapshot = await getCount( - query(collectionGroup(db, collectionGroupId)) - ); - expect(snapshot.data().count).to.equal(2); - }); + return skipTestUnlessUsingEmulator(() => + withTestDb(async db => { + const collectionGroupId = doc(collection(db, 'countTest')).id; + const docPaths = [ + `${collectionGroupId}/cg-doc1`, + `abc/123/${collectionGroupId}/cg-doc2`, + `zzz${collectionGroupId}/cg-doc3`, + `abc/123/zzz${collectionGroupId}/cg-doc4`, + `abc/123/zzz/${collectionGroupId}` + ]; + const batch = writeBatch(db); + for (const docPath of docPaths) { + batch.set(doc(db, docPath), { x: 1 }); + } + await batch.commit(); + const snapshot = await getCount( + query(collectionGroup(db, collectionGroupId)) + ); + expect(snapshot.data().count).to.equal(2); + }) + ); }); it('aggregateQuerySnapshotEqual on same queries be truthy', () => { @@ -2312,15 +2341,17 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query1 = query(coll, where('author', '==', 'authorA')); - const query2 = query(coll, where('author', '==', 'authorA')); - const snapshot1A = await getCount(query1); - const snapshot1B = await getCount(query1); - const snapshot2 = await getCount(query2); - expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot1B)).to.be.true; - expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot2)).to.be.true; - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const query1 = query(coll, where('author', '==', 'authorA')); + const query2 = query(coll, where('author', '==', 'authorA')); + const snapshot1A = await getCount(query1); + const snapshot1B = await getCount(query1); + const snapshot2 = await getCount(query2); + expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot1B)).to.be.true; + expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot2)).to.be.true; + }) + ); }); it('aggregateQuerySnapshotEqual on different queries be falsy', () => { @@ -2330,22 +2361,26 @@ describe('countQuery()', () => { { author: 'authorB', title: 'titleC' }, { author: 'authorB', title: 'titleD' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const query1 = query(coll, where('author', '==', 'authorA')); - const query2 = query(coll, where('author', '==', 'authorB')); - const snapshot1 = await getCount(query1); - const snapshot2 = await getCount(query2); - expect(aggregateQuerySnapshotEqual(snapshot1, snapshot2)).to.be.false; - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const query1 = query(coll, where('author', '==', 'authorA')); + const query2 = query(coll, where('author', '==', 'authorB')); + const snapshot1 = await getCount(query1); + const snapshot2 = await getCount(query2); + expect(aggregateQuerySnapshotEqual(snapshot1, snapshot2)).to.be.false; + }) + ); }); it('count query fails on a terminated Firestore', () => { - return withTestCollection(async coll => { - await terminate(coll.firestore); - expect(() => getCount(coll)).to.throw( - 'The client has already been terminated.' - ); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollection(async coll => { + await terminate(coll.firestore); + expect(() => getCount(coll)).to.throw( + 'The client has already been terminated.' + ); + }) + ); }); it('terminate Firestore not effect count query in flight', () => { @@ -2354,11 +2389,13 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return withTestCollectionAndInitialData(testDocs, async coll => { - const promise = getCount(coll); - await terminate(coll.firestore); - const snapshot = await promise; - expect(snapshot.data().count).to.equal(3); - }); + return skipTestUnlessUsingEmulator(() => + withTestCollectionAndInitialData(testDocs, async coll => { + const promise = getCount(coll); + await terminate(coll.firestore); + const snapshot = await promise; + expect(snapshot.data().count).to.equal(3); + }) + ); }); }); From f490bfeca503124ba2a6434ad1e0f57628cbf159 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 14 Sep 2022 16:07:33 -0700 Subject: [PATCH 19/34] update the type of skipTestUnlessUsingEmulator --- packages/firestore/test/integration/util/helpers.ts | 11 +++-------- packages/firestore/test/lite/helpers.ts | 8 +++----- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index 9be611ce4f9..fd523f37e70 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -16,7 +16,6 @@ */ import { isIndexedDBAvailable } from '@firebase/util'; -import { Code } from '../../../src/util/error'; import { collection, @@ -241,10 +240,10 @@ export async function withNamedTestDbsOrSkipUnlessUsingEmulator( } export function skipTestUnlessUsingEmulator< - T extends (...params: any[]) => any + T extends (...params: Parameters) => Promise >(fn: T, ...params: Parameters): Promise { /** - * This is a wrapper function to execute test cases that can only run on emulator till + * This is a wrapper to execute test functions that can only run on emulator till * production test environment is ready. */ if (!USE_EMULATOR) { @@ -252,11 +251,7 @@ export function skipTestUnlessUsingEmulator< } return fn(...params).catch((error: FirestoreError) => { - if (error.name === 'FirebaseError') { - throw error; - } else { - throw new FirestoreError(Code.UNKNOWN, error.toString()); - } + throw error; }); } diff --git a/packages/firestore/test/lite/helpers.ts b/packages/firestore/test/lite/helpers.ts index b710da6289e..9dd55da5089 100644 --- a/packages/firestore/test/lite/helpers.ts +++ b/packages/firestore/test/lite/helpers.ts @@ -103,19 +103,17 @@ export function withTestCollection( } export function skipTestUnlessUsingEmulator< - T extends (...params: any[]) => any + T extends (...params: Parameters) => Promise >(fn: T, ...params: Parameters): Promise { /** - * This is a wrapper function to execute test cases that can only run on emulator till + * 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: any) => { - throw error; - }); + return fn(...params); } // Used for testing the FirestoreDataConverter. From 078d5819221f706d557f0a4e80c75f7681b9784a Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 14 Sep 2022 18:30:30 -0700 Subject: [PATCH 20/34] roll back to any type for skipTestUnlessUsingEmulator --- packages/firestore/test/integration/util/helpers.ts | 4 ++-- packages/firestore/test/lite/helpers.ts | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index fd523f37e70..3e1337466fe 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -238,9 +238,9 @@ export async function withNamedTestDbsOrSkipUnlessUsingEmulator( } } } - +// eslint-disable-next-line @typescript-eslint/no-explicit-any export function skipTestUnlessUsingEmulator< - T extends (...params: Parameters) => Promise + T extends (...params: any[]) => Promise >(fn: T, ...params: Parameters): Promise { /** * This is a wrapper to execute test functions that can only run on emulator till diff --git a/packages/firestore/test/lite/helpers.ts b/packages/firestore/test/lite/helpers.ts index 9dd55da5089..935bbc608a0 100644 --- a/packages/firestore/test/lite/helpers.ts +++ b/packages/firestore/test/lite/helpers.ts @@ -102,8 +102,9 @@ export function withTestCollection( }); } +// eslint-disable-next-line @typescript-eslint/no-explicit-any export function skipTestUnlessUsingEmulator< - T extends (...params: Parameters) => Promise + T extends (...params: any[]) => Promise >(fn: T, ...params: Parameters): Promise { /** * This is a wrapper to execute test functions that can only run on emulator till From a9ee57a05c9d566e2a7ee071372928df7c75440d Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 14 Sep 2022 19:12:11 -0700 Subject: [PATCH 21/34] move aggregation test file into api_internal folder --- .../test/integration/{api => api_internal}/aggregation.test.ts | 1 + 1 file changed, 1 insertion(+) rename packages/firestore/test/integration/{api => api_internal}/aggregation.test.ts (97%) diff --git a/packages/firestore/test/integration/api/aggregation.test.ts b/packages/firestore/test/integration/api_internal/aggregation.test.ts similarity index 97% rename from packages/firestore/test/integration/api/aggregation.test.ts rename to packages/firestore/test/integration/api_internal/aggregation.test.ts index a992f972a17..100059a97b4 100644 --- a/packages/firestore/test/integration/api/aggregation.test.ts +++ b/packages/firestore/test/integration/api_internal/aggregation.test.ts @@ -15,6 +15,7 @@ * limitations under the License. */ +/** This file is emporarily staying in api_internal folder till aggregate queries are public. */ import { expect } from 'chai'; import { getCountFromServer } from '../../../src/api/aggregate'; From dbbc2f8213de90953ef198a57d57e3faaabf32e1 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 15 Sep 2022 13:06:33 -0700 Subject: [PATCH 22/34] resolve comments --- packages/firestore/src/api/aggregate.ts | 10 - .../firestore/src/core/firestore_client.ts | 34 +- packages/firestore/src/lite-api/aggregate.ts | 4 +- .../src/platform/node/grpc_connection.ts | 2 + .../firestore/src/remote/rest_connection.ts | 2 + .../api_internal/aggregation.test.ts | 106 +++--- .../test/integration/util/helpers.ts | 37 --- .../firestore/test/lite/integration.test.ts | 303 +++++++++--------- 8 files changed, 222 insertions(+), 276 deletions(-) diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts index 0f4cbcb6c6f..501b5437573 100644 --- a/packages/firestore/src/api/aggregate.ts +++ b/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. diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 3730568478f..ce74bdf7f8a 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/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 }> >(); 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; diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts index 5fb9d7a7255..0fb0c15da94 100644 --- a/packages/firestore/src/lite-api/aggregate.ts +++ b/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( left: AggregateQuerySnapshot, diff --git a/packages/firestore/src/platform/node/grpc_connection.ts b/packages/firestore/src/platform/node/grpc_connection.ts index e561c1981aa..b5d77a11bf7 100644 --- a/packages/firestore/src/platform/node/grpc_connection.ts +++ b/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; } diff --git a/packages/firestore/src/remote/rest_connection.ts b/packages/firestore/src/remote/rest_connection.ts index ee8b7ebcc39..40ab436787a 100644 --- a/packages/firestore/src/remote/rest_connection.ts +++ b/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; } diff --git a/packages/firestore/test/integration/api_internal/aggregation.test.ts b/packages/firestore/test/integration/api_internal/aggregation.test.ts index 100059a97b4..88d6caf17fd 100644 --- a/packages/firestore/test/integration/api_internal/aggregation.test.ts +++ b/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' ); - }) - ); - }); -}); + }); + }); + } +); diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index 3e1337466fe..c48aff1dcac 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/packages/firestore/test/integration/util/helpers.ts @@ -238,22 +238,6 @@ export async function withNamedTestDbsOrSkipUnlessUsingEmulator( } } } -// eslint-disable-next-line @typescript-eslint/no-explicit-any -export function skipTestUnlessUsingEmulator< - T extends (...params: any[]) => Promise ->(fn: T, ...params: Parameters): Promise { - /** - * 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 +325,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); - } -}; diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts index 9c4826f2c08..34eba10b1f8 100644 --- a/packages/firestore/test/lite/integration.test.ts +++ b/packages/firestore/test/lite/integration.test.ts @@ -83,7 +83,8 @@ import { runTransaction } from '../../src/lite-api/transaction'; import { writeBatch } from '../../src/lite-api/write_batch'; import { DEFAULT_PROJECT_ID, - DEFAULT_SETTINGS + DEFAULT_SETTINGS, + USE_EMULATOR } from '../integration/util/settings'; import { @@ -2119,53 +2120,41 @@ describe('withConverter() support', () => { }); }); -describe('countQuery()', () => { +(USE_EMULATOR ? describe : describe.skip)('Count quries', () => { it('AggregateQuerySnapshot inherits the original query', () => { - return skipTestUnlessUsingEmulator(() => - withTestCollection(async coll => { - const query_ = query(coll); - const snapshot = await getCount(query_); - expect(snapshot.query).to.equal(query_); - }) - ); + return withTestCollection(async coll => { + const query_ = query(coll); + const snapshot = await getCount(query_); + expect(snapshot.query).to.equal(query_); + }); }); - it('run count query on empty test collection', () => { - return skipTestUnlessUsingEmulator(() => - withTestCollection(async coll => { - const snapshot = await getCount(coll); - expect(snapshot.data().count).to.equal(0); - }) - ); + it('run count query on empty collection', () => { + return withTestCollection(async coll => { + const snapshot = await getCount(coll); + expect(snapshot.data().count).to.equal(0); + }); }); - it('run count query on test collection with 3 docs', () => { + it('run count query on collection with 3 docs', () => { const testDocs = [ { author: 'authorA', title: 'titleA' }, { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const snapshot = await getCount(coll); - expect(snapshot.data().count).to.equal(3); - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const snapshot = await getCount(coll); + expect(snapshot.data().count).to.equal(3); + }); }); it('run count query fails on invalid collection reference', () => { - return skipTestUnlessUsingEmulator(() => - withTestDb(async db => { - const queryForRejection = collection(db, '__badpath__'); - try { - await getCount(queryForRejection); - } catch (e) { - expect((e as Error)?.message).to.equal( - 'Request failed with error: Bad Request' - ); - } - }) - ); + return withTestDb(async db => { + const queryForRejection = collection(db, '__badpath__'); + await expect(getCount(queryForRejection)).to.eventually.be.rejectedWith( + 'Request failed with error: Bad Request' + ); + }); }); it('count query supports filter', () => { @@ -2174,13 +2163,11 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, where('author', '==', 'authorA')); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(2); - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, where('author', '==', 'authorA')); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); + }); }); it('count query supports filter and a small limit size', () => { @@ -2189,13 +2176,11 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, where('author', '==', 'authorA'), limit(1)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(1); - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, where('author', '==', 'authorA'), limit(1)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(1); + }); }); it('count query supports filter and a large limit size', () => { @@ -2204,13 +2189,11 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, where('author', '==', 'authorA'), limit(3)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(2); - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, where('author', '==', 'authorA'), limit(3)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); + }); }); it('count query supports order by', () => { @@ -2220,13 +2203,11 @@ describe('countQuery()', () => { { author: 'authorB', title: null }, { author: 'authorB' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, orderBy('title')); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(3); - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('title')); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(3); + }); }); it('count query supports order by and startAt', () => { @@ -2236,13 +2217,11 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, orderBy('id'), startAt(2)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(2); - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAt(2)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); + }); }); it('count query supports order by and startAfter', () => { @@ -2252,13 +2231,11 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, orderBy('id'), startAfter(2)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(1); - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAfter(2)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(1); + }); }); it('count query supports order by and endAt', () => { @@ -2268,13 +2245,11 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, orderBy('id'), startAt(1), endAt(2)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(2); - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAt(1), endAt(2)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); + }); }); it('count query supports order by and endBefore', () => { @@ -2284,55 +2259,55 @@ describe('countQuery()', () => { { id: 2, author: 'authorB', title: 'titleC' }, { id: null, author: 'authorB', title: 'titleD' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query(coll, orderBy('id'), startAt(1), endBefore(2)); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(1); - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query(coll, orderBy('id'), startAt(1), endBefore(2)); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(1); + }); }); - it('count query supports converter', () => { + it("count query doesn't use converter", () => { const testDocs = [ { author: 'authorA', title: 'titleA' }, { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const query_ = query( - coll, - where('author', '==', 'authorA') - ).withConverter(postConverter); - const snapshot = await getCount(query_); - expect(snapshot.data().count).to.equal(2); - }) - ); + 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 withTestCollectionAndInitialData(testDocs, async coll => { + const query_ = query( + coll, + where('author', '==', 'authorA') + ).withConverter(throwingConverter); + const snapshot = await getCount(query_); + expect(snapshot.data().count).to.equal(2); + }); }); it('count query supports collection groups', () => { - return skipTestUnlessUsingEmulator(() => - withTestDb(async db => { - const collectionGroupId = doc(collection(db, 'countTest')).id; - const docPaths = [ - `${collectionGroupId}/cg-doc1`, - `abc/123/${collectionGroupId}/cg-doc2`, - `zzz${collectionGroupId}/cg-doc3`, - `abc/123/zzz${collectionGroupId}/cg-doc4`, - `abc/123/zzz/${collectionGroupId}` - ]; - const batch = writeBatch(db); - for (const docPath of docPaths) { - batch.set(doc(db, docPath), { x: 1 }); - } - await batch.commit(); - const snapshot = await getCount( - query(collectionGroup(db, collectionGroupId)) - ); - expect(snapshot.data().count).to.equal(2); - }) - ); + return withTestDb(async db => { + const collectionGroupId = doc(collection(db, 'countTest')).id; + const docPaths = [ + `${collectionGroupId}/cg-doc1`, + `abc/123/${collectionGroupId}/cg-doc2`, + `zzz${collectionGroupId}/cg-doc3`, + `abc/123/zzz${collectionGroupId}/cg-doc4`, + `abc/123/zzz/${collectionGroupId}` + ]; + const batch = writeBatch(db); + for (const docPath of docPaths) { + batch.set(doc(db, docPath), { x: 1 }); + } + await batch.commit(); + const snapshot = await getCount(collectionGroup(db, collectionGroupId)); + expect(snapshot.data().count).to.equal(2); + }); }); it('aggregateQuerySnapshotEqual on same queries be truthy', () => { @@ -2341,17 +2316,33 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const query1 = query(coll, where('author', '==', 'authorA')); - const query2 = query(coll, where('author', '==', 'authorA')); - const snapshot1A = await getCount(query1); - const snapshot1B = await getCount(query1); - const snapshot2 = await getCount(query2); - expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot1B)).to.be.true; - expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot2)).to.be.true; - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query1 = query(coll, where('author', '==', 'authorA')); + const query2 = query(coll, where('author', '==', 'authorA')); + const snapshot1A = await getCount(query1); + const snapshot1B = await getCount(query1); + const snapshot2 = await getCount(query2); + expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot1B)).to.be.true; + expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot2)).to.be.true; + }); + }); + + it('aggregateQuerySnapshotEqual on same queries with different documents size be falsy', () => { + const testDocs = [ + { author: 'authorA', title: 'titleA' }, + { author: 'authorA', title: 'titleB' }, + { author: 'authorB', title: 'titleC' } + ]; + return withTestCollectionAndInitialData(testDocs, async coll => { + const query1 = query(coll, where('author', '==', 'authorA')); + const snapshot1A = await getCount(query1); + await addDoc(coll, { author: 'authorA', title: 'titleD' }); + const query2 = query(coll, where('author', '==', 'authorA')); + const snapshot1B = await getCount(query1); + const snapshot2 = await getCount(query2); + expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot1B)).to.be.false; + expect(aggregateQuerySnapshotEqual(snapshot1A, snapshot2)).to.be.false; + }); }); it('aggregateQuerySnapshotEqual on different queries be falsy', () => { @@ -2361,26 +2352,22 @@ describe('countQuery()', () => { { author: 'authorB', title: 'titleC' }, { author: 'authorB', title: 'titleD' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const query1 = query(coll, where('author', '==', 'authorA')); - const query2 = query(coll, where('author', '==', 'authorB')); - const snapshot1 = await getCount(query1); - const snapshot2 = await getCount(query2); - expect(aggregateQuerySnapshotEqual(snapshot1, snapshot2)).to.be.false; - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const query1 = query(coll, where('author', '==', 'authorA')); + const query2 = query(coll, where('author', '==', 'authorB')); + const snapshot1 = await getCount(query1); + const snapshot2 = await getCount(query2); + expect(aggregateQuerySnapshotEqual(snapshot1, snapshot2)).to.be.false; + }); }); it('count query fails on a terminated Firestore', () => { - return skipTestUnlessUsingEmulator(() => - withTestCollection(async coll => { - await terminate(coll.firestore); - expect(() => getCount(coll)).to.throw( - 'The client has already been terminated.' - ); - }) - ); + return withTestCollection(async coll => { + await terminate(coll.firestore); + expect(() => getCount(coll)).to.throw( + 'The client has already been terminated.' + ); + }); }); it('terminate Firestore not effect count query in flight', () => { @@ -2389,13 +2376,11 @@ describe('countQuery()', () => { { author: 'authorA', title: 'titleB' }, { author: 'authorB', title: 'titleC' } ]; - return skipTestUnlessUsingEmulator(() => - withTestCollectionAndInitialData(testDocs, async coll => { - const promise = getCount(coll); - await terminate(coll.firestore); - const snapshot = await promise; - expect(snapshot.data().count).to.equal(3); - }) - ); + return withTestCollectionAndInitialData(testDocs, async coll => { + const promise = getCount(coll); + await terminate(coll.firestore); + const snapshot = await promise; + expect(snapshot.data().count).to.equal(3); + }); }); }); From c12d5a00515961a4dd8b9bdb84fad5075f1230bf Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 15 Sep 2022 13:24:29 -0700 Subject: [PATCH 23/34] reformat to pass lint check --- packages/firestore/test/integration/util/helpers.ts | 4 +--- packages/firestore/test/lite/integration.test.ts | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/firestore/test/integration/util/helpers.ts b/packages/firestore/test/integration/util/helpers.ts index c48aff1dcac..79dbacaafa0 100644 --- a/packages/firestore/test/integration/util/helpers.ts +++ b/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, diff --git a/packages/firestore/test/lite/integration.test.ts b/packages/firestore/test/lite/integration.test.ts index 34eba10b1f8..823d0b524b6 100644 --- a/packages/firestore/test/lite/integration.test.ts +++ b/packages/firestore/test/lite/integration.test.ts @@ -91,7 +91,6 @@ import { Post, postConverter, postConverterMerge, - skipTestUnlessUsingEmulator, withTestCollection, withTestCollectionAndInitialData, withTestDb, @@ -2120,6 +2119,7 @@ describe('withConverter() support', () => { }); }); +// eslint-disable-next-line no-restricted-properties (USE_EMULATOR ? describe : describe.skip)('Count quries', () => { it('AggregateQuerySnapshot inherits the original query', () => { return withTestCollection(async coll => { From 74116a410e9a27fff9e389af6f25d7e594a59246 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 15 Sep 2022 14:27:10 -0700 Subject: [PATCH 24/34] add changeset as requested by github --- .changeset/light-ties-rhyme.md | 5 +++++ packages/firestore/test/lite/helpers.ts | 15 --------------- 2 files changed, 5 insertions(+), 15 deletions(-) create mode 100644 .changeset/light-ties-rhyme.md diff --git a/.changeset/light-ties-rhyme.md b/.changeset/light-ties-rhyme.md new file mode 100644 index 00000000000..9c590641483 --- /dev/null +++ b/.changeset/light-ties-rhyme.md @@ -0,0 +1,5 @@ +--- +'@firebase/firestore': minor +--- + +Internal release of count query diff --git a/packages/firestore/test/lite/helpers.ts b/packages/firestore/test/lite/helpers.ts index 935bbc608a0..c8629790e25 100644 --- a/packages/firestore/test/lite/helpers.ts +++ b/packages/firestore/test/lite/helpers.ts @@ -102,21 +102,6 @@ export function withTestCollection( }); } -// eslint-disable-next-line @typescript-eslint/no-explicit-any -export function skipTestUnlessUsingEmulator< - T extends (...params: any[]) => Promise ->(fn: T, ...params: Parameters): Promise { - /** - * 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( From de9dd9a33b5db285e9cc683eba69ea9bab038317 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 15 Sep 2022 15:11:28 -0700 Subject: [PATCH 25/34] remove the changeset --- .changeset/light-ties-rhyme.md | 5 ----- packages/firestore/test/lite/helpers.ts | 3 +-- 2 files changed, 1 insertion(+), 7 deletions(-) delete mode 100644 .changeset/light-ties-rhyme.md diff --git a/.changeset/light-ties-rhyme.md b/.changeset/light-ties-rhyme.md deleted file mode 100644 index 9c590641483..00000000000 --- a/.changeset/light-ties-rhyme.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@firebase/firestore': minor ---- - -Internal release of count query diff --git a/packages/firestore/test/lite/helpers.ts b/packages/firestore/test/lite/helpers.ts index c8629790e25..e51c7c606ab 100644 --- a/packages/firestore/test/lite/helpers.ts +++ b/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; From 3a6cc2797deee3ee625e828c2660b7f57c4771b1 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 15 Sep 2022 16:14:02 -0700 Subject: [PATCH 26/34] add the changeset back --- .changeset/hot-insects-wink.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/hot-insects-wink.md diff --git a/.changeset/hot-insects-wink.md b/.changeset/hot-insects-wink.md new file mode 100644 index 00000000000..ea0df51532f --- /dev/null +++ b/.changeset/hot-insects-wink.md @@ -0,0 +1,5 @@ +--- +'@firebase/firestore': minor +--- + +Release count query for internal use. From 2380959ee1b7aca22f888c250d1c2edad9e64d85 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 16 Sep 2022 10:17:40 -0700 Subject: [PATCH 27/34] export count qury --- common/api-review/firestore-lite.api.md | 40 +++++++++++++++++++ common/api-review/firestore.api.md | 40 +++++++++++++++++++ packages/firestore/lite/index.ts | 11 +++++ packages/firestore/src/api.ts | 11 +++++ packages/firestore/src/api/aggregate.ts | 10 +++++ .../{api_internal => api}/aggregation.test.ts | 11 +++-- 6 files changed, 117 insertions(+), 6 deletions(-) rename packages/firestore/test/integration/{api_internal => api}/aggregation.test.ts (95%) diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index 312db6e00ea..52b8947344c 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -17,6 +17,38 @@ export type AddPrefixToKeys { + // (undocumented) + type: string; +} + +// @public +export type AggregateFieldType = ReturnType; + +// @public +export class AggregateQuerySnapshot { + data(): AggregateSpecData; + // (undocumented) + readonly query: Query; + // (undocumented) + readonly type = "AggregateQuerySnapshot"; +} + +// @public +export function aggregateQuerySnapshotEqual(left: AggregateQuerySnapshot, right: AggregateQuerySnapshot): boolean; + +// @public +export interface AggregateSpec { + // (undocumented) + [field: string]: AggregateFieldType; +} + +// @public +export type AggregateSpecData = { + [P in keyof T]: T[P] extends AggregateField ? U : never; +}; + // @public export function arrayRemove(...elements: unknown[]): FieldValue; @@ -63,6 +95,9 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por mockUserToken?: EmulatorMockTokenOptions | string; }): void; +// @public +export function count(): AggregateField; + // @public export function deleteDoc(reference: DocumentReference): Promise; @@ -169,6 +204,11 @@ export class GeoPoint { }; } +// @public +export function getCount(query: Query): Promise; +}>>; + // @public export function getDoc(reference: DocumentReference): Promise>; diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 3b6538d867b..6fa63aa0d90 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -17,6 +17,38 @@ export type AddPrefixToKeys { + // (undocumented) + type: string; +} + +// @public +export type AggregateFieldType = ReturnType; + +// @public +export class AggregateQuerySnapshot { + data(): AggregateSpecData; + // (undocumented) + readonly query: Query; + // (undocumented) + readonly type = "AggregateQuerySnapshot"; +} + +// @public +export function aggregateQuerySnapshotEqual(left: AggregateQuerySnapshot, right: AggregateQuerySnapshot): boolean; + +// @public +export interface AggregateSpec { + // (undocumented) + [field: string]: AggregateFieldType; +} + +// @public +export type AggregateSpecData = { + [P in keyof T]: T[P] extends AggregateField ? U : never; +}; + // @public export function arrayRemove(...elements: unknown[]): FieldValue; @@ -69,6 +101,9 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por mockUserToken?: EmulatorMockTokenOptions | string; }): void; +// @public +export function count(): AggregateField; + // @public export function deleteDoc(reference: DocumentReference): Promise; @@ -209,6 +244,11 @@ export class GeoPoint { }; } +// @public +export function getCountFromServer(query: Query): Promise; +}>>; + // @public export function getDoc(reference: DocumentReference): Promise>; diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index ad7ec7d8294..f84751d3426 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -27,6 +27,17 @@ import { registerFirestore } from './register'; registerFirestore(); +export { + AggregateField, + AggregateFieldType, + AggregateSpec, + AggregateSpecData, + AggregateQuerySnapshot, + aggregateQuerySnapshotEqual, + count, + getCount +} from '../src/lite-api/aggregate'; + export { FirestoreSettings as Settings } from '../src/lite-api/settings'; export { diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 544d275fc30..000b03245cd 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -15,6 +15,17 @@ * limitations under the License. */ +export { + AggregateField, + AggregateFieldType, + AggregateSpec, + AggregateSpecData, + AggregateQuerySnapshot, + aggregateQuerySnapshotEqual, + count, + getCountFromServer +} from './api/aggregate'; + export { FieldPath, documentId } from './api/field_path'; export { diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts index 501b5437573..7ce80598484 100644 --- a/packages/firestore/src/api/aggregate.ts +++ b/packages/firestore/src/api/aggregate.ts @@ -22,6 +22,16 @@ import { cast } from '../util/input_validation'; import { ensureFirestoreConfigured, Firestore } from './database'; +export { + AggregateField, + AggregateFieldType, + AggregateSpec, + AggregateSpecData, + AggregateQuerySnapshot, + count, + aggregateQuerySnapshotEqual +} 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. diff --git a/packages/firestore/test/integration/api_internal/aggregation.test.ts b/packages/firestore/test/integration/api/aggregation.test.ts similarity index 95% rename from packages/firestore/test/integration/api_internal/aggregation.test.ts rename to packages/firestore/test/integration/api/aggregation.test.ts index 88d6caf17fd..0c0e9db0b15 100644 --- a/packages/firestore/test/integration/api_internal/aggregation.test.ts +++ b/packages/firestore/test/integration/api/aggregation.test.ts @@ -15,21 +15,20 @@ * limitations under the License. */ -/** This file is emporarily staying in api_internal folder till aggregate queries are public. */ import { expect } from 'chai'; -import { getCountFromServer } from '../../../src/api/aggregate'; import { collection, collectionGroup, - doc, disableNetwork, + doc, + DocumentData, + getCountFromServer, query, + QueryDocumentSnapshot, terminate, where, - writeBatch, - DocumentData, - QueryDocumentSnapshot + writeBatch } from '../util/firebase_export'; import { apiDescribe, From 597d1fb9b4cbe19fc91d5f9df8ac97cf94ab7053 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 16 Sep 2022 11:53:25 -0700 Subject: [PATCH 28/34] add changeset --- .changeset/wicked-tomatoes-grow.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/wicked-tomatoes-grow.md diff --git a/.changeset/wicked-tomatoes-grow.md b/.changeset/wicked-tomatoes-grow.md new file mode 100644 index 00000000000..b2a363e3774 --- /dev/null +++ b/.changeset/wicked-tomatoes-grow.md @@ -0,0 +1,5 @@ +--- +'@firebase/firestore': minor +--- + +Expose getCountFromServer() and Lite SDK getCount()to enable Firestore count-only aggregate queries From e2cb6a176571a068eb9c3910b3318bfa22b083a5 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 23 Sep 2022 11:20:00 -0700 Subject: [PATCH 29/34] update the firestore-client with datastore from client --- packages/firestore/src/api/aggregate.ts | 4 +- .../firestore/src/core/count_query_runner.ts | 67 +++++++++++++++++++ .../firestore/src/core/firestore_client.ts | 22 +++--- packages/firestore/src/lite-api/aggregate.ts | 29 +------- 4 files changed, 86 insertions(+), 36 deletions(-) create mode 100644 packages/firestore/src/core/count_query_runner.ts diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts index 7ce80598484..ce03d4de909 100644 --- a/packages/firestore/src/api/aggregate.ts +++ b/packages/firestore/src/api/aggregate.ts @@ -21,6 +21,7 @@ import { AggregateField, AggregateQuerySnapshot } from '../lite-api/aggregate'; import { cast } from '../util/input_validation'; import { ensureFirestoreConfigured, Firestore } from './database'; +import { ExpUserDataWriter } from './reference_impl'; export { AggregateField, @@ -45,5 +46,6 @@ export function getCountFromServer( ): Promise }>> { const firestore = cast(query.firestore, Firestore); const client = ensureFirestoreConfigured(firestore); - return firestoreClientRunCountQuery(client, query); + const userDataWriter = new ExpUserDataWriter(firestore); + return firestoreClientRunCountQuery(client, query, userDataWriter); } diff --git a/packages/firestore/src/core/count_query_runner.ts b/packages/firestore/src/core/count_query_runner.ts new file mode 100644 index 00000000000..d02970967d3 --- /dev/null +++ b/packages/firestore/src/core/count_query_runner.ts @@ -0,0 +1,67 @@ +/** + * @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 { AbstractUserDataWriter, Query } from '../api'; +import { AggregateField, AggregateQuerySnapshot } from '../lite-api/aggregate'; +import { Value } from '../protos/firestore_proto_api'; +import { Datastore, invokeRunAggregationQueryRpc } from '../remote/datastore'; +import { hardAssert } from '../util/assert'; + +/** + * CountQueryRunner encapsulates the logic needed to run the count aggregation + * queries. + */ +export class CountQueryRunner { + constructor( + private readonly query: Query, + private readonly datastore: Datastore, + private readonly userDataWriter: AbstractUserDataWriter + ) {} + + run(): Promise }>> { + return invokeRunAggregationQueryRpc(this.datastore, this.query._query).then( + result => { + hardAssert( + result[0] !== undefined, + 'Aggregation fields are missing from result.' + ); + + const counts = Object.entries(result[0]) + .filter(([key, value]) => key === 'count_alias') + .map(([key, value]) => + this.userDataWriter.convertValue(value as Value) + ); + + const countValue = counts[0]; + + hardAssert( + typeof countValue === 'number', + 'Count aggregate field value is not a number: ' + countValue + ); + + return Promise.resolve( + new AggregateQuerySnapshot<{ count: AggregateField }>( + this.query, + { + count: countValue + } + ) + ); + } + ); + } +} diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index 6b9e410c53c..f6957b6b300 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -16,18 +16,17 @@ */ import { GetOptions } from '@firebase/firestore-types'; - +import { AbstractUserDataWriter } from '../api'; +import { + AggregateField, + AggregateQuerySnapshot +} from '../api/aggregate'; import { LoadBundleTask } from '../api/bundle'; import { CredentialChangeListener, CredentialsProvider } from '../api/credentials'; import { User } from '../auth/user'; -import { - AggregateField, - AggregateQuerySnapshot, - getCount -} from '../lite-api/aggregate'; import { Query as LiteQuery } from '../lite-api/reference'; import { LocalStore } from '../local/local_store'; import { @@ -68,6 +67,7 @@ import { OfflineComponentProvider, OnlineComponentProvider } from './component_provider'; +import { CountQueryRunner } from './count_query_runner'; import { DatabaseId, DatabaseInfo } from './database_info'; import { addSnapshotsInSyncListener, @@ -510,7 +510,8 @@ export function firestoreClientTransaction( export function firestoreClientRunCountQuery( client: FirestoreClient, - query: LiteQuery + query: LiteQuery, + userDataWriter: AbstractUserDataWriter ): Promise }>> { const deferred = new Deferred< AggregateQuerySnapshot<{ count: AggregateField }> @@ -526,7 +527,12 @@ export function firestoreClientRunCountQuery( ) ); } else { - const result = await getCount(query); + const datastore = await getDatastore(client); + const result = new CountQueryRunner( + query, + datastore, + userDataWriter + ).run(); deferred.resolve(result); } } catch (e) { diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts index 0fb0c15da94..6e6516df2be 100644 --- a/packages/firestore/src/lite-api/aggregate.ts +++ b/packages/firestore/src/lite-api/aggregate.ts @@ -16,10 +16,7 @@ */ import { deepEqual } from '@firebase/util'; - -import { Value } from '../protos/firestore_proto_api'; -import { invokeRunAggregationQueryRpc } from '../remote/datastore'; -import { hardAssert } from '../util/assert'; +import { CountQueryRunner } from '../core/count_query_runner'; import { cast } from '../util/input_validation'; import { getDatastore } from './components'; @@ -108,29 +105,7 @@ export function getCount( const firestore = cast(query.firestore, Firestore); const datastore = getDatastore(firestore); const userDataWriter = new LiteUserDataWriter(firestore); - return invokeRunAggregationQueryRpc(datastore, query._query).then(result => { - hardAssert( - result[0] !== undefined, - 'Aggregation fields are missing from result.' - ); - - const counts = Object.entries(result[0]) - .filter(([key, value]) => key === 'count_alias') - .map(([key, value]) => userDataWriter.convertValue(value as Value)); - - const countValue = counts[0]; - - hardAssert( - typeof countValue === 'number', - 'Count aggregate field value is not a number: ' + countValue - ); - - return Promise.resolve( - new AggregateQuerySnapshot<{ count: AggregateField }>(query, { - count: countValue - }) - ); - }); + return new CountQueryRunner(query, datastore, userDataWriter).run(); } /** From 79670bcadc9f7cf97fde6b119a8318eadedd3196 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 26 Sep 2022 11:30:22 -0400 Subject: [PATCH 30/34] fix circular dependency between count_query_runner.ts and lite-api/aggregate.ts --- packages/firestore/lite/index.ts | 10 ++- packages/firestore/src/api/aggregate.ts | 10 ++- .../firestore/src/core/count_query_runner.ts | 2 +- packages/firestore/src/lite-api/aggregate.ts | 66 +-------------- .../firestore/src/lite-api/aggregate_types.ts | 83 +++++++++++++++++++ 5 files changed, 97 insertions(+), 74 deletions(-) create mode 100644 packages/firestore/src/lite-api/aggregate_types.ts diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index f84751d3426..a863fab6c5a 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -27,16 +27,18 @@ import { registerFirestore } from './register'; registerFirestore(); +export { + aggregateQuerySnapshotEqual, + getCount +} from '../src/lite-api/aggregate'; export { AggregateField, AggregateFieldType, AggregateSpec, AggregateSpecData, AggregateQuerySnapshot, - aggregateQuerySnapshotEqual, - count, - getCount -} from '../src/lite-api/aggregate'; + count +} from '../src/lite-api/aggregate_types'; export { FirestoreSettings as Settings } from '../src/lite-api/settings'; diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts index ce03d4de909..8f76e156b6b 100644 --- a/packages/firestore/src/api/aggregate.ts +++ b/packages/firestore/src/api/aggregate.ts @@ -17,21 +17,23 @@ import { Query } from '../api'; import { firestoreClientRunCountQuery } from '../core/firestore_client'; -import { AggregateField, AggregateQuerySnapshot } from '../lite-api/aggregate'; +import { AggregateField, AggregateQuerySnapshot } from '../lite-api/aggregate_types'; import { cast } from '../util/input_validation'; import { ensureFirestoreConfigured, Firestore } from './database'; import { ExpUserDataWriter } from './reference_impl'; +export { + aggregateQuerySnapshotEqual +} from '../lite-api/aggregate'; export { AggregateField, AggregateFieldType, AggregateSpec, AggregateSpecData, AggregateQuerySnapshot, - count, - aggregateQuerySnapshotEqual -} from '../lite-api/aggregate'; + count +} from '../lite-api/aggregate_types'; /** * Executes the query and returns the results as a `AggregateQuerySnapshot` from the diff --git a/packages/firestore/src/core/count_query_runner.ts b/packages/firestore/src/core/count_query_runner.ts index d02970967d3..1bf792e362a 100644 --- a/packages/firestore/src/core/count_query_runner.ts +++ b/packages/firestore/src/core/count_query_runner.ts @@ -16,7 +16,7 @@ */ import { AbstractUserDataWriter, Query } from '../api'; -import { AggregateField, AggregateQuerySnapshot } from '../lite-api/aggregate'; +import { AggregateField, AggregateQuerySnapshot } from '../lite-api/aggregate_types'; import { Value } from '../protos/firestore_proto_api'; import { Datastore, invokeRunAggregationQueryRpc } from '../remote/datastore'; import { hardAssert } from '../util/assert'; diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts index 6e6516df2be..b7b9f0fa2ac 100644 --- a/packages/firestore/src/lite-api/aggregate.ts +++ b/packages/firestore/src/lite-api/aggregate.ts @@ -19,76 +19,12 @@ import { deepEqual } from '@firebase/util'; import { CountQueryRunner } from '../core/count_query_runner'; import { cast } from '../util/input_validation'; +import { AggregateField, AggregateQuerySnapshot, AggregateSpec } from './aggregate_types'; import { getDatastore } from './components'; import { Firestore } from './database'; import { Query, queryEqual } from './reference'; import { LiteUserDataWriter } from './reference_impl'; -/** - * An `AggregateField`that captures input type T. - */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -export class AggregateField { - type = 'AggregateField'; -} - -/** - * Creates and returns an aggregation field that counts the documents in the result set. - * @returns An `AggregateField` object with number input type. - */ -export function count(): AggregateField { - return new AggregateField(); -} - -/** - * The union of all `AggregateField` types that are returned from the factory - * functions. - */ -export type AggregateFieldType = ReturnType; - -/** - * A type whose values are all `AggregateField` objects. - * This is used as an argument to the "getter" functions, and the snapshot will - * map the same names to the corresponding values. - */ -export interface AggregateSpec { - [field: string]: AggregateFieldType; -} - -/** - * A type whose keys are taken from an `AggregateSpec` type, and whose values - * are the result of the aggregation performed by the corresponding - * `AggregateField` from the input `AggregateSpec`. - */ -export type AggregateSpecData = { - [P in keyof T]: T[P] extends AggregateField ? U : never; -}; - -/** - * An `AggregateQuerySnapshot` contains the results of running an aggregate query. - */ -export class AggregateQuerySnapshot { - readonly type = 'AggregateQuerySnapshot'; - - /** @hideconstructor */ - constructor( - readonly query: Query, - private readonly _data: AggregateSpecData - ) {} - - /** - * The results of the requested aggregations. The keys of the returned object - * will be the same as those of the `AggregateSpec` object specified to the - * aggregation method, and the values will be the corresponding aggregation - * result. - * - * @returns The aggregation statistics result of running a query. - */ - data(): AggregateSpecData { - return this._data; - } -} - /** * Counts the number of documents in the result set of the given query, ignoring * any locally-cached data and any locally-pending writes and simply surfacing diff --git a/packages/firestore/src/lite-api/aggregate_types.ts b/packages/firestore/src/lite-api/aggregate_types.ts new file mode 100644 index 00000000000..b7243f1062c --- /dev/null +++ b/packages/firestore/src/lite-api/aggregate_types.ts @@ -0,0 +1,83 @@ +/** + * @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 { Query } from './reference'; + +/** + * An `AggregateField`that captures input type T. + */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +export class AggregateField { + type = 'AggregateField'; +} + +/** + * Creates and returns an aggregation field that counts the documents in the result set. + * @returns An `AggregateField` object with number input type. + */ +export function count(): AggregateField { + return new AggregateField(); +} + +/** + * The union of all `AggregateField` types that are returned from the factory + * functions. + */ +export type AggregateFieldType = ReturnType; + +/** + * A type whose values are all `AggregateField` objects. + * This is used as an argument to the "getter" functions, and the snapshot will + * map the same names to the corresponding values. + */ +export interface AggregateSpec { + [field: string]: AggregateFieldType; +} + +/** + * A type whose keys are taken from an `AggregateSpec` type, and whose values + * are the result of the aggregation performed by the corresponding + * `AggregateField` from the input `AggregateSpec`. + */ +export type AggregateSpecData = { + [P in keyof T]: T[P] extends AggregateField ? U : never; +}; + +/** + * An `AggregateQuerySnapshot` contains the results of running an aggregate query. + */ +export class AggregateQuerySnapshot { + readonly type = 'AggregateQuerySnapshot'; + + /** @hideconstructor */ + constructor( + readonly query: Query, + private readonly _data: AggregateSpecData + ) {} + + /** + * The results of the requested aggregations. The keys of the returned object + * will be the same as those of the `AggregateSpec` object specified to the + * aggregation method, and the values will be the corresponding aggregation + * result. + * + * @returns The aggregation statistics result of running a query. + */ + data(): AggregateSpecData { + return this._data; + } +} From c3675d49d76954087a6674ed731404e7885a61a3 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 26 Sep 2022 11:25:10 -0700 Subject: [PATCH 31/34] fix comments --- .changeset/wicked-tomatoes-grow.md | 2 +- packages/firestore/lite/index.ts | 1 + packages/firestore/src/api.ts | 11 +++++++---- packages/firestore/src/api/aggregate.ts | 17 +++++------------ .../firestore/src/core/count_query_runner.ts | 7 +++++-- packages/firestore/src/core/firestore_client.ts | 5 +++-- packages/firestore/src/lite-api/aggregate.ts | 7 ++++++- 7 files changed, 28 insertions(+), 22 deletions(-) diff --git a/.changeset/wicked-tomatoes-grow.md b/.changeset/wicked-tomatoes-grow.md index b2a363e3774..ed507b055f5 100644 --- a/.changeset/wicked-tomatoes-grow.md +++ b/.changeset/wicked-tomatoes-grow.md @@ -2,4 +2,4 @@ '@firebase/firestore': minor --- -Expose getCountFromServer() and Lite SDK getCount()to enable Firestore count-only aggregate queries +Added `getCountFromServer()` (`getCount()` in the Lite SDK), which fetches the number of documents in the result set without actually downloading the documents. diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index a863fab6c5a..07451f0889b 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -31,6 +31,7 @@ export { aggregateQuerySnapshotEqual, getCount } from '../src/lite-api/aggregate'; + export { AggregateField, AggregateFieldType, diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 000b03245cd..14c6c4b4cf1 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -15,16 +15,19 @@ * limitations under the License. */ +export { + aggregateQuerySnapshotEqual, + getCountFromServer +} from './api/aggregate'; + export { AggregateField, AggregateFieldType, AggregateSpec, AggregateSpecData, AggregateQuerySnapshot, - aggregateQuerySnapshotEqual, - count, - getCountFromServer -} from './api/aggregate'; + count +} from './lite-api/aggregate_types'; export { FieldPath, documentId } from './api/field_path'; diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts index 8f76e156b6b..bd632d0ad6d 100644 --- a/packages/firestore/src/api/aggregate.ts +++ b/packages/firestore/src/api/aggregate.ts @@ -17,23 +17,16 @@ import { Query } from '../api'; import { firestoreClientRunCountQuery } from '../core/firestore_client'; -import { AggregateField, AggregateQuerySnapshot } from '../lite-api/aggregate_types'; +import { + AggregateField, + AggregateQuerySnapshot +} from '../lite-api/aggregate_types'; import { cast } from '../util/input_validation'; import { ensureFirestoreConfigured, Firestore } from './database'; import { ExpUserDataWriter } from './reference_impl'; -export { - aggregateQuerySnapshotEqual -} from '../lite-api/aggregate'; -export { - AggregateField, - AggregateFieldType, - AggregateSpec, - AggregateSpecData, - AggregateQuerySnapshot, - count -} from '../lite-api/aggregate_types'; +export { aggregateQuerySnapshotEqual } from '../lite-api/aggregate'; /** * Executes the query and returns the results as a `AggregateQuerySnapshot` from the diff --git a/packages/firestore/src/core/count_query_runner.ts b/packages/firestore/src/core/count_query_runner.ts index 1bf792e362a..3161a270fc1 100644 --- a/packages/firestore/src/core/count_query_runner.ts +++ b/packages/firestore/src/core/count_query_runner.ts @@ -16,7 +16,10 @@ */ import { AbstractUserDataWriter, Query } from '../api'; -import { AggregateField, AggregateQuerySnapshot } from '../lite-api/aggregate_types'; +import { + AggregateField, + AggregateQuerySnapshot +} from '../lite-api/aggregate_types'; import { Value } from '../protos/firestore_proto_api'; import { Datastore, invokeRunAggregationQueryRpc } from '../remote/datastore'; import { hardAssert } from '../util/assert'; @@ -25,7 +28,7 @@ import { hardAssert } from '../util/assert'; * CountQueryRunner encapsulates the logic needed to run the count aggregation * queries. */ -export class CountQueryRunner { +export class CountQueryRunner { constructor( private readonly query: Query, private readonly datastore: Datastore, diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index f6957b6b300..2c2c0af1771 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -16,11 +16,12 @@ */ import { GetOptions } from '@firebase/firestore-types'; -import { AbstractUserDataWriter } from '../api'; + import { + AbstractUserDataWriter, AggregateField, AggregateQuerySnapshot -} from '../api/aggregate'; +} from '../api'; import { LoadBundleTask } from '../api/bundle'; import { CredentialChangeListener, diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts index b7b9f0fa2ac..9532cec855c 100644 --- a/packages/firestore/src/lite-api/aggregate.ts +++ b/packages/firestore/src/lite-api/aggregate.ts @@ -16,10 +16,15 @@ */ import { deepEqual } from '@firebase/util'; + import { CountQueryRunner } from '../core/count_query_runner'; import { cast } from '../util/input_validation'; -import { AggregateField, AggregateQuerySnapshot, AggregateSpec } from './aggregate_types'; +import { + AggregateField, + AggregateQuerySnapshot, + AggregateSpec +} from './aggregate_types'; import { getDatastore } from './components'; import { Firestore } from './database'; import { Query, queryEqual } from './reference'; From bb87268ebb27a6f20e0b929712540f6d8a6c4c9d Mon Sep 17 00:00:00 2001 From: wu-hui <53845758+wu-hui@users.noreply.github.com> Date: Mon, 26 Sep 2022 13:47:56 -0400 Subject: [PATCH 32/34] Fix document change leads to time travel across multiple tabs (#6619) * Fix document change leads to time travel across multiple tabs * Changeset --- .changeset/nine-cherries-swim.md | 5 +++ .../firestore/src/local/local_store_impl.ts | 4 +- .../test/unit/specs/listen_spec.test.ts | 41 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 .changeset/nine-cherries-swim.md diff --git a/.changeset/nine-cherries-swim.md b/.changeset/nine-cherries-swim.md new file mode 100644 index 00000000000..ca0744abcf3 --- /dev/null +++ b/.changeset/nine-cherries-swim.md @@ -0,0 +1,5 @@ +--- +'@firebase/firestore': patch +--- + +Fix a time travel issue across multiple tabs diff --git a/packages/firestore/src/local/local_store_impl.ts b/packages/firestore/src/local/local_store_impl.ts index 19c7df1f5c7..ae1760f5930 100644 --- a/packages/firestore/src/local/local_store_impl.ts +++ b/packages/firestore/src/local/local_store_impl.ts @@ -1273,7 +1273,9 @@ function setMaxReadTime( collectionGroup: string, changedDocs: SortedMap ): void { - let readTime = SnapshotVersion.min(); + let readTime = + localStoreImpl.collectionGroupReadTime.get(collectionGroup) || + SnapshotVersion.min(); changedDocs.forEach((_, doc) => { if (doc.readTime.compareTo(readTime) > 0) { readTime = doc.readTime; diff --git a/packages/firestore/test/unit/specs/listen_spec.test.ts b/packages/firestore/test/unit/specs/listen_spec.test.ts index d6cc4e7fffd..854266735cd 100644 --- a/packages/firestore/test/unit/specs/listen_spec.test.ts +++ b/packages/firestore/test/unit/specs/listen_spec.test.ts @@ -909,6 +909,47 @@ describeSpec('Listens:', [], () => { } ); + // Reproduces: https://github.com/firebase/firebase-js-sdk/issues/6511 + specTest( + 'Secondary client raises latency compensated snapshot from primary mutation', + ['multi-client'], + () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { key: '1' }); + const docAMutated = doc('collection/a', 1500, { + key: '2' + }).setHasLocalMutations(); + + return ( + client(0) + .becomeVisible() + .expectPrimaryState(true) + .userListens(query1) + .watchAcksFull(query1, 1000, docA) + .expectEvents(query1, { added: [docA] }) + .userUnlistens(query1) + .watchRemoves(query1) + .client(1) + .userListens(query1) + .expectEvents(query1, { added: [docA], fromCache: true }) + .client(0) + .expectListen(query1, { resumeToken: 'resume-token-1000' }) + .watchAcksFull(query1, 1500, docA) + .client(1) + .expectEvents(query1, {}) + .client(0) + .userSets('collection/a', { key: '2' }) + .client(1) + // Without the fix for 6511, this would raise two snapshots, first one as expected and + // second one travels back in time and raise the old stale document. + .expectEvents(query1, { + modified: [docAMutated], + hasPendingWrites: true + }) + ); + } + ); + specTest( 'Mirror queries from same secondary client', ['multi-client'], From fa07ffac2a390344f82bd174003fe771f1b82eae Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 27 Sep 2022 12:20:11 -0400 Subject: [PATCH 33/34] update changeset --- .changeset/wicked-tomatoes-grow.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/wicked-tomatoes-grow.md b/.changeset/wicked-tomatoes-grow.md index ed507b055f5..da2aeae0a17 100644 --- a/.changeset/wicked-tomatoes-grow.md +++ b/.changeset/wicked-tomatoes-grow.md @@ -1,5 +1,6 @@ --- '@firebase/firestore': minor +'firebase': minor --- Added `getCountFromServer()` (`getCount()` in the Lite SDK), which fetches the number of documents in the result set without actually downloading the documents. From 88d026653c3fe3017a6c7708c1ef5f6f310b0cab Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 27 Sep 2022 17:13:42 -0400 Subject: [PATCH 34/34] Firestore: Re-write API docs for COUNT API (#6632) --- common/api-review/firestore-lite.api.md | 8 +-- common/api-review/firestore.api.md | 8 +-- packages/firestore/lite/index.ts | 3 +- packages/firestore/src/api.ts | 3 +- packages/firestore/src/api/aggregate.ts | 21 ++++++-- packages/firestore/src/lite-api/aggregate.ts | 26 +++++---- .../firestore/src/lite-api/aggregate_types.ts | 54 ++++++++++--------- 7 files changed, 65 insertions(+), 58 deletions(-) diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index 52b8947344c..b336e0d0c8b 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -19,19 +19,16 @@ export type AddPrefixToKeys { - // (undocumented) type: string; } // @public -export type AggregateFieldType = ReturnType; +export type AggregateFieldType = AggregateField; // @public export class AggregateQuerySnapshot { data(): AggregateSpecData; - // (undocumented) readonly query: Query; - // (undocumented) readonly type = "AggregateQuerySnapshot"; } @@ -95,9 +92,6 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por mockUserToken?: EmulatorMockTokenOptions | string; }): void; -// @public -export function count(): AggregateField; - // @public export function deleteDoc(reference: DocumentReference): Promise; diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 6fa63aa0d90..46ac63239c6 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -19,19 +19,16 @@ export type AddPrefixToKeys { - // (undocumented) type: string; } // @public -export type AggregateFieldType = ReturnType; +export type AggregateFieldType = AggregateField; // @public export class AggregateQuerySnapshot { data(): AggregateSpecData; - // (undocumented) readonly query: Query; - // (undocumented) readonly type = "AggregateQuerySnapshot"; } @@ -101,9 +98,6 @@ export function connectFirestoreEmulator(firestore: Firestore, host: string, por mockUserToken?: EmulatorMockTokenOptions | string; }): void; -// @public -export function count(): AggregateField; - // @public export function deleteDoc(reference: DocumentReference): Promise; diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index 07451f0889b..5e3faa48e31 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -37,8 +37,7 @@ export { AggregateFieldType, AggregateSpec, AggregateSpecData, - AggregateQuerySnapshot, - count + AggregateQuerySnapshot } from '../src/lite-api/aggregate_types'; export { FirestoreSettings as Settings } from '../src/lite-api/settings'; diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 14c6c4b4cf1..f05db09f568 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -25,8 +25,7 @@ export { AggregateFieldType, AggregateSpec, AggregateSpecData, - AggregateQuerySnapshot, - count + AggregateQuerySnapshot } from './lite-api/aggregate_types'; export { FieldPath, documentId } from './api/field_path'; diff --git a/packages/firestore/src/api/aggregate.ts b/packages/firestore/src/api/aggregate.ts index bd632d0ad6d..a4713ca4149 100644 --- a/packages/firestore/src/api/aggregate.ts +++ b/packages/firestore/src/api/aggregate.ts @@ -29,12 +29,25 @@ import { ExpUserDataWriter } from './reference_impl'; export { aggregateQuerySnapshotEqual } 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. + * Calculates the number of documents in the result set of the given query, + * without actually downloading the documents. * - * @param query - The `Query` to execute. + * Using this function to count the documents is efficient because only the + * final count, not the documents' data, is downloaded. This function can even + * count the documents if the result set would be prohibitively large to + * download entirely (e.g. thousands of documents). * - * @returns A `Promise` that will be resolved with the results of the query. + * The result received from the server is presented, unaltered, without + * considering any local state. That is, documents in the local cache are not + * taken into consideration, neither are local modifications not yet + * synchronized with the server. Previously-downloaded results, if any, are not + * used: every request using this source necessarily involves a round trip to + * the server. + * + * @param query - The query whose result set size to calculate. + * @returns A Promise that will be resolved with the count; the count can be + * retrieved from `snapshot.data().count`, where `snapshot` is the + * `AggregateQuerySnapshot` to which the returned Promise resolves. */ export function getCountFromServer( query: Query diff --git a/packages/firestore/src/lite-api/aggregate.ts b/packages/firestore/src/lite-api/aggregate.ts index 9532cec855c..390c6af5ef8 100644 --- a/packages/firestore/src/lite-api/aggregate.ts +++ b/packages/firestore/src/lite-api/aggregate.ts @@ -31,14 +31,18 @@ import { Query, queryEqual } from './reference'; import { LiteUserDataWriter } from './reference_impl'; /** - * Counts the number of documents in the result set of the given query, ignoring - * any locally-cached data and any locally-pending writes and simply surfacing - * whatever the server returns. If the server cannot be reached then the - * returned promise will be rejected. + * Calculates the number of documents in the result set of the given query, + * without actually downloading the documents. * - * @param query - The `Query` to execute. + * Using this function to count the documents is efficient because only the + * final count, not the documents' data, is downloaded. This function can even + * count the documents if the result set would be prohibitively large to + * download entirely (e.g. thousands of documents). * - * @returns An `AggregateQuerySnapshot` that contains the number of documents. + * @param query - The query whose result set size to calculate. + * @returns A Promise that will be resolved with the count; the count can be + * retrieved from `snapshot.data().count`, where `snapshot` is the + * `AggregateQuerySnapshot` to which the returned Promise resolves. */ export function getCount( query: Query @@ -51,13 +55,15 @@ export function getCount( /** * Compares two `AggregateQuerySnapshot` instances for equality. + * * Two `AggregateQuerySnapshot` instances are considered "equal" if they have - * the same underlying query, and the same data. + * underlying queries that compare equal, and the same data. * - * @param left - The `AggregateQuerySnapshot` to compare. - * @param right - The `AggregateQuerySnapshot` to compare. + * @param left - The first `AggregateQuerySnapshot` to compare. + * @param right - The second `AggregateQuerySnapshot` to compare. * - * @returns true if the AggregateQuerySnapshots are equal. + * @returns `true` if the objects are "equal", as defined above, or `false` + * otherwise. */ export function aggregateQuerySnapshotEqual( left: AggregateQuerySnapshot, diff --git a/packages/firestore/src/lite-api/aggregate_types.ts b/packages/firestore/src/lite-api/aggregate_types.ts index b7243f1062c..d71a36e86eb 100644 --- a/packages/firestore/src/lite-api/aggregate_types.ts +++ b/packages/firestore/src/lite-api/aggregate_types.ts @@ -18,64 +18,66 @@ import { Query } from './reference'; /** - * An `AggregateField`that captures input type T. + * Represents an aggregation that can be performed by Firestore. */ // eslint-disable-next-line @typescript-eslint/no-unused-vars export class AggregateField { + /** A type string to uniquely identify instances of this class. */ type = 'AggregateField'; } /** - * Creates and returns an aggregation field that counts the documents in the result set. - * @returns An `AggregateField` object with number input type. + * The union of all `AggregateField` types that are supported by Firestore. */ -export function count(): AggregateField { - return new AggregateField(); -} +export type AggregateFieldType = AggregateField; /** - * The union of all `AggregateField` types that are returned from the factory - * functions. - */ -export type AggregateFieldType = ReturnType; - -/** - * A type whose values are all `AggregateField` objects. - * This is used as an argument to the "getter" functions, and the snapshot will - * map the same names to the corresponding values. + * A type whose property values are all `AggregateField` objects. */ export interface AggregateSpec { [field: string]: AggregateFieldType; } /** - * A type whose keys are taken from an `AggregateSpec` type, and whose values - * are the result of the aggregation performed by the corresponding - * `AggregateField` from the input `AggregateSpec`. + * A type whose keys are taken from an `AggregateSpec`, and whose values are the + * result of the aggregation performed by the corresponding `AggregateField` + * from the input `AggregateSpec`. */ export type AggregateSpecData = { [P in keyof T]: T[P] extends AggregateField ? U : never; }; /** - * An `AggregateQuerySnapshot` contains the results of running an aggregate query. + * The results of executing an aggregation query. */ export class AggregateQuerySnapshot { + /** A type string to uniquely identify instances of this class. */ readonly type = 'AggregateQuerySnapshot'; + /** + * The underlying query over which the aggregations recorded in this + * `AggregateQuerySnapshot` were performed. + */ + readonly query: Query; + /** @hideconstructor */ constructor( - readonly query: Query, + query: Query, private readonly _data: AggregateSpecData - ) {} + ) { + this.query = query; + } /** - * The results of the requested aggregations. The keys of the returned object - * will be the same as those of the `AggregateSpec` object specified to the - * aggregation method, and the values will be the corresponding aggregation - * result. + * Returns the results of the aggregations performed over the underlying + * query. + * + * The keys of the returned object will be the same as those of the + * `AggregateSpec` object specified to the aggregation method, and the values + * will be the corresponding aggregation result. * - * @returns The aggregation statistics result of running a query. + * @returns The results of the aggregations performed over the underlying + * query. */ data(): AggregateSpecData { return this._data;