From f70e121421a789188b0b30886c91be45dca043e1 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 20 Jun 2022 15:25:59 -0400 Subject: [PATCH 01/28] firestore.d.ts: COUNT API added --- types/firestore.d.ts | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 9bc0d6760..dbb7de04c 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -583,6 +583,15 @@ declare namespace FirebaseFirestore { */ get(documentRef: DocumentReference): Promise>; + /** + * Retrieves an aggregate query result. Holds a pessimistic lock on all + * documents that were matched by the underlying query. + * + * @param aggregateQuery An aggregate query to execute. + * @return An AggregateQuerySnapshot for the retrieved data. + */ + get(aggregateQuery: AggregateQuery): Promise>; + /** * Retrieves multiple documents from Firestore. Holds a pessimistic lock on * all returned documents. @@ -1692,6 +1701,8 @@ declare namespace FirebaseFirestore { onError?: (error: Error) => void ): () => void; + count(): AggregateQuery; + /** * Returns true if this `Query` is equal to the provided one. * @@ -2012,6 +2023,28 @@ declare namespace FirebaseFirestore { toQuery(): Query; } + export class AggregateQuerySnapshot { + private constructor(); + + readonly query: AggregateQuery; + + readonly readTime: Timestamp; + + getCount(): number; + + isEqual(other: AggregateQuerySnapshot): boolean; + } + + export class AggregateQuery { + private constructor(); + + readonly query: Query; + + get(): Promise; + + isEqual(other: AggregateQuery): boolean; + } + /** * Sentinel values that can be used when writing document fields with set(), * create() or update(). From 76cd8e7a38b6017404080116618fe91b13e36f57 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 2 Sep 2022 15:11:30 -0400 Subject: [PATCH 02/28] Work in progress --- dev/src/reference.ts | 44 ++++++++++++++++++++++++++++++++++++ dev/src/transaction.ts | 20 ++++++++++++++-- dev/system-test/firestore.ts | 32 ++++++++++++++++++++++++++ types/firestore.d.ts | 2 +- 4 files changed, 95 insertions(+), 3 deletions(-) diff --git a/dev/src/reference.ts b/dev/src/reference.ts index 78ab483c8..370f83f0d 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -2404,6 +2404,11 @@ export class Query implements firestore.Query { }, onError || console.error); } + count(): AggregateQuery { + //TODO(tomandersen) + return null as any; + } + /** * Returns a function that can be used to sort QueryDocumentSnapshots * according to the sort criteria of this query. @@ -2832,6 +2837,45 @@ export class CollectionReference } } +export class AggregateQuery implements firestore.AggregateQuery { + get query(): Query { + //TODO(tomandersen) + return null as any; + } + + get(): Promise { + //TODO(tomandersen) + return null as any; + } + + isEqual(other: FirebaseFirestore.AggregateQuery): boolean { + //TODO(tomandersen) + return null as any; + } +} + +export class AggregateQuerySnapshot implements firestore.AggregateQuerySnapshot { + get query(): firestore.AggregateQuery { + //TODO(tomandersen) + return null as any; + } + + get readTime(): firestore.Timestamp { + //TODO(tomandersen) + return null as any; + } + + getCount(): number { + //TODO(tomandersen) + return null as any; + } + + isEqual(other: FirebaseFirestore.AggregateQuerySnapshot): boolean { + //TODO(tomandersen) + return null as any; + } +} + /** * Validates the input string as a field order direction. * diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index bbb6bcd37..98df89b21 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -27,6 +27,8 @@ import {logger} from './logger'; import {FieldPath, validateFieldPath} from './path'; import {StatusCode} from './status-code'; import { + AggregateQuery, + AggregateQuerySnapshot, DocumentReference, Query, QuerySnapshot, @@ -97,6 +99,15 @@ export class Transaction implements firestore.Transaction { */ get(documentRef: DocumentReference): Promise>; + /** + * Retrieves an aggregate query result. Holds a pessimistic lock on all + * documents that were matched by the underlying query. + * + * @param aggregateQuery An aggregate query to execute. + * @return An AggregateQuerySnapshot for the retrieved data. + */ + get(aggregateQuery: AggregateQuery): Promise; + /** * Retrieve a document or a query result from the database. Holds a * pessimistic lock on all returned documents. @@ -121,8 +132,8 @@ export class Transaction implements firestore.Transaction { * ``` */ get( - refOrQuery: DocumentReference | Query - ): Promise | QuerySnapshot> { + refOrQuery: DocumentReference | Query | AggregateQuery + ): Promise | QuerySnapshot | AggregateQuerySnapshot> { if (!this._writeBatch.isEmpty) { throw new Error(READ_AFTER_WRITE_ERROR_MSG); } @@ -137,6 +148,11 @@ export class Transaction implements firestore.Transaction { return refOrQuery._get(this._transactionId); } + if (refOrQuery instanceof AggregateQuery) { + //TODO(tomandersen) + return Promise.reject(); + } + throw new Error( 'Value for argument "refOrQuery" must be a DocumentReference or a Query.' ); diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index 5d8f9ca1d..6fc8f0bb3 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -17,6 +17,7 @@ import { DocumentData, WithFieldValue, PartialWithFieldValue, + AggregateQuery, SetOptions, Settings, } from '@google-cloud/firestore'; @@ -2140,6 +2141,37 @@ describe('Query class', () => { }); }); +describe.only('Transaction class (with Emulator)', () => { + let firestore: Firestore; + let randomCol: CollectionReference; + + beforeEach(() => { + console.log("AAAAAAAAAAAAAAAA"); + firestore = new Firestore({ + host: 'localhost', + port: 8080, + projectId: 'my-cool-project' + }); + console.log("BBBBBBBBBBBBB"); + randomCol = getTestRoot(firestore); + console.log("CCCCCCCCCCCCCCCCC"); + }); + + afterEach(() => verifyInstance(firestore)); + + it('count', async () => { + console.log("AAAAAAAAAAAAAAAA"); + const ref = randomCol.doc('doc'); + console.log("AAAAAAAAAAAAAAAA"); + const count = randomCol.where('foo', '==', 'bar').count(); + console.log("AAAAAAAAAAAAAAAA"); + await ref.set({foo: 'bar'}); + console.log("AAAAAAAAAAAAAAAA"); + //const res = await firestore.runTransaction(f => f.get(count)); + //expect(res.getCount()).to.equal(1); + }); +}) + describe('Transaction class', () => { let firestore: Firestore; let randomCol: CollectionReference; diff --git a/types/firestore.d.ts b/types/firestore.d.ts index dbb7de04c..9dfd67079 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -590,7 +590,7 @@ declare namespace FirebaseFirestore { * @param aggregateQuery An aggregate query to execute. * @return An AggregateQuerySnapshot for the retrieved data. */ - get(aggregateQuery: AggregateQuery): Promise>; + get(aggregateQuery: AggregateQuery): Promise; /** * Retrieves multiple documents from Firestore. Holds a pessimistic lock on From d9f5170274c50901441adf11b6b77670b6afca8b Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Mon, 12 Sep 2022 11:09:06 -0400 Subject: [PATCH 03/28] Implement count with tests. --- dev/protos/google/firestore/v1/query.proto | 3 + dev/src/reference.ts | 172 ++++++++++++++++++--- dev/src/transaction.ts | 3 +- dev/src/types.ts | 5 + dev/system-test/firestore.ts | 41 +++-- dev/test/aggregateQuery.ts | 157 +++++++++++++++++++ dev/test/query.ts | 12 +- dev/test/util/helpers.ts | 2 +- types/firestore.d.ts | 2 +- 9 files changed, 349 insertions(+), 48 deletions(-) create mode 100644 dev/test/aggregateQuery.ts diff --git a/dev/protos/google/firestore/v1/query.proto b/dev/protos/google/firestore/v1/query.proto index 0d7a7b6bf..ddbfee7b3 100644 --- a/dev/protos/google/firestore/v1/query.proto +++ b/dev/protos/google/firestore/v1/query.proto @@ -67,6 +67,9 @@ message StructuredQuery { // Documents are required to satisfy all of the combined filters. AND = 1; + + // Documents are required to satisfy at least one of the combined filters. + OR = 2; } // The operator for combining multiple filters. diff --git a/dev/src/reference.ts b/dev/src/reference.ts index 370f83f0d..1c382db1c 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -15,7 +15,7 @@ */ import * as firestore from '@google-cloud/firestore'; -import {Duplex, Transform} from 'stream'; +import {Duplex, Readable, Transform} from 'stream'; import * as deepEqual from 'fast-deep-equal'; import * as protos from '../protos/firestore_v1_proto_api'; @@ -57,6 +57,8 @@ import {DocumentWatch, QueryWatch} from './watch'; import {validateDocumentData, WriteBatch, WriteResult} from './write-batch'; import api = protos.google.firestore.v1; +import {google} from '../protos/firestore_v1_proto_api'; +import RunAggregationQueryResponse = google.firestore.v1.RunAggregationQueryResponse; /** * The direction of a `Query.orderBy()` clause is specified as 'desc' or 'asc' @@ -2405,8 +2407,7 @@ export class Query implements firestore.Query { } count(): AggregateQuery { - //TODO(tomandersen) - return null as any; + return new AggregateQuery(this); } /** @@ -2838,41 +2839,168 @@ export class CollectionReference } export class AggregateQuery implements firestore.AggregateQuery { - get query(): Query { - //TODO(tomandersen) - return null as any; + private readonly _query: Query; + + constructor(query: Query) { + this._query = query; } - get(): Promise { - //TODO(tomandersen) - return null as any; + get query(): firestore.Query { + return this._query; } - isEqual(other: FirebaseFirestore.AggregateQuery): boolean { - //TODO(tomandersen) - return null as any; + get(): Promise { + return this._get(); + } + + /** + * Internal get() method that accepts an optional transaction id. + * + * @private + * @internal + * @param {bytes=} transactionId A transaction ID. + */ + _get(transactionId?: Uint8Array): Promise { + // Capture the error stack to preserve stack tracing across async calls. + const stack = Error().stack!; + + return new Promise((resolve, reject) => { + const stream = this._stream(transactionId); + stream.on('error', err => { + reject(wrapError(err, stack)); + }); + stream.once('data', result => { + stream.destroy(); + resolve(result); + }); + stream.on('end', () => { + reject('No AggregateQuery results'); + }); + }); + } + + /** + * Internal streaming method that accepts an optional transaction ID. + * + * @param transactionId A transaction ID. + * @private + * @internal + * @returns A stream of document results. + */ + _stream(transactionId?: Uint8Array): Readable { + const tag = requestTag(); + const firestore = this._query.firestore; + + const stream: Transform = new Transform({ + objectMode: true, + transform: (proto: RunAggregationQueryResponse, enc, callback) => { + const readTime = Timestamp.fromProto(proto.readTime!); + if (proto.result) { + const result = new AggregateQuerySnapshot( + this, + readTime, + Number(proto.result.aggregateFields!.count.integerValue!) + ); + callback(undefined, result); + } else { + callback(Error('unexpected')); + } + }, + }); + + firestore + .initializeIfNeeded(tag) + .then(async () => { + const request = this.toProto(transactionId); + const backendStream = await firestore.requestStream( + 'runAggregationQuery', + request, + tag + ); + backendStream.on('error', err => { + logger( + 'AggregateQuery._stream', + tag, + 'AggregateQuery failed with stream error:', + err + ); + stream.destroy(err); + }); + backendStream.resume(); + backendStream.pipe(stream); + }) + .catch(e => stream.destroy(e)); + + return stream; + } + + /** + * Internal method for serializing a query to its RunAggregationQuery proto + * representation with an optional transaction id or read time. + * + * @private + * @internal + * @returns Serialized JSON for the query. + */ + toProto(transactionId?: Uint8Array): api.IRunAggregationQueryRequest { + const queryProto = this._query.toProto(); + const runQueryRequest: api.IRunAggregationQueryRequest = { + parent: queryProto.parent, + structuredAggregationQuery: { + structuredQuery: queryProto.structuredQuery, + aggregations: [ + { + alias: 'count', + count: {}, + }, + ], + }, + }; + + if (transactionId instanceof Uint8Array) { + runQueryRequest.transaction = transactionId; + } + + return runQueryRequest; + } + + isEqual(other: firestore.AggregateQuery): boolean { + return ( + this === other || + (other instanceof AggregateQuery && this._query.isEqual(other._query)) + ); } } -export class AggregateQuerySnapshot implements firestore.AggregateQuerySnapshot { +export class AggregateQuerySnapshot + implements firestore.AggregateQuerySnapshot +{ + constructor( + private readonly _query: AggregateQuery, + private readonly _readTime: Timestamp, + private readonly _count: number + ) {} + get query(): firestore.AggregateQuery { - //TODO(tomandersen) - return null as any; + return this._query; } get readTime(): firestore.Timestamp { - //TODO(tomandersen) - return null as any; + return this._readTime; } getCount(): number { - //TODO(tomandersen) - return null as any; + return this._count; } - isEqual(other: FirebaseFirestore.AggregateQuerySnapshot): boolean { - //TODO(tomandersen) - return null as any; + isEqual(other: firestore.AggregateQuerySnapshot): boolean { + return ( + this === other || + (other instanceof AggregateQuerySnapshot && + this._query.isEqual(other._query) && + this._count === other._count && + this._readTime.isEqual(other._readTime)) + ); } } diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index 98df89b21..3c000c1ed 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -149,8 +149,7 @@ export class Transaction implements firestore.Transaction { } if (refOrQuery instanceof AggregateQuery) { - //TODO(tomandersen) - return Promise.reject(); + return refOrQuery.get(); } throw new Error( diff --git a/dev/src/types.ts b/dev/src/types.ts index 33bf17e9b..07dd5733b 100644 --- a/dev/src/types.ts +++ b/dev/src/types.ts @@ -65,6 +65,10 @@ export interface GapicClient { options?: CallOptions ): Duplex; runQuery(request?: api.IRunQueryRequest, options?: CallOptions): Duplex; + runAggregationQuery( + request?: api.IRunAggregationQueryRequest, + options?: CallOptions + ): Duplex; listDocuments( request: api.IListDocumentsRequest, options?: CallOptions @@ -95,6 +99,7 @@ export type FirestoreStreamingMethod = | 'listen' | 'partitionQueryStream' | 'runQuery' + | 'runAggregationQuery' | 'batchGetDocuments'; /** Type signature for the unary methods in the GAPIC layer. */ diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index 6fc8f0bb3..d863dfbf4 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -2141,36 +2141,45 @@ describe('Query class', () => { }); }); -describe.only('Transaction class (with Emulator)', () => { +describe.skip('Transaction class (with Emulator)', () => { let firestore: Firestore; let randomCol: CollectionReference; + setLogFunction(console.log); beforeEach(() => { - console.log("AAAAAAAAAAAAAAAA"); + // This cannot be hard coded to emulator. firestore = new Firestore({ host: 'localhost', - port: 8080, - projectId: 'my-cool-project' + port: 8539, + projectId: 'my-cool-project', + ssl: false, }); - console.log("BBBBBBBBBBBBB"); randomCol = getTestRoot(firestore); - console.log("CCCCCCCCCCCCCCCCC"); }); afterEach(() => verifyInstance(firestore)); - it('count', async () => { - console.log("AAAAAAAAAAAAAAAA"); - const ref = randomCol.doc('doc'); - console.log("AAAAAAAAAAAAAAAA"); + it('counts 0 document', async () => { + const count = randomCol.where('foo', '==', 'notbar').count(); + const res = await firestore.runTransaction(f => f.get(count)); + expect(res.getCount()).to.equal(0); + }); + + it('counts 1 document', async () => { + await randomCol.doc('doc').set({foo: 'bar'}); const count = randomCol.where('foo', '==', 'bar').count(); - console.log("AAAAAAAAAAAAAAAA"); - await ref.set({foo: 'bar'}); - console.log("AAAAAAAAAAAAAAAA"); - //const res = await firestore.runTransaction(f => f.get(count)); - //expect(res.getCount()).to.equal(1); + const res = await firestore.runTransaction(f => f.get(count)); + expect(res.getCount()).to.equal(1); }); -}) + + it('counts multiple documents', async () => { + await randomCol.doc('doc1').set({foo: 'bar'}); + await randomCol.doc('doc2').set({foo: 'bar'}); + const count = randomCol.where('foo', '==', 'bar').count(); + const res = await firestore.runTransaction(f => f.get(count)); + expect(res.getCount()).to.equal(2); + }); +}); describe('Transaction class', () => { let firestore: Firestore; diff --git a/dev/test/aggregateQuery.ts b/dev/test/aggregateQuery.ts new file mode 100644 index 000000000..814eac3b8 --- /dev/null +++ b/dev/test/aggregateQuery.ts @@ -0,0 +1,157 @@ +// 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 {afterEach, beforeEach, it} from 'mocha'; +import { + ApiOverride, + createInstance, + stream, + streamWithoutEnd, + verifyInstance, +} from './util/helpers'; +import {Firestore, Query, Timestamp} from '../src'; +import {expect, use} from 'chai'; +import {google} from '../protos/firestore_v1_proto_api'; +import api = google.firestore.v1; +import * as chaiAsPromised from 'chai-as-promised'; +use(chaiAsPromised); + +describe('aggregate query interface', () => { + let firestore: Firestore; + + beforeEach(() => { + // setTimeoutHandler(setImmediate); + return createInstance().then(firestoreInstance => { + firestore = firestoreInstance; + }); + }); + + afterEach(() => { + verifyInstance(firestore); + // setTimeoutHandler(setTimeout); + }); + + it('has isEqual() method', () => { + const queryA = firestore.collection('collectionId'); + const queryB = firestore.collection('collectionId'); + + const queryEquals = (equals: Query[], notEquals: Query[]) => { + for (const equal1 of equals) { + const equal1count = equal1.count(); + for (const equal2 of equals) { + const equal2count = equal2.count(); + expect(equal1count.isEqual(equal2count)).to.be.true; + expect(equal2count.isEqual(equal1count)).to.be.true; + } + + for (const notEqual of notEquals) { + const notEqual2count = notEqual.count(); + expect(equal1count.isEqual(notEqual2count)).to.be.false; + expect(notEqual2count.isEqual(equal1count)).to.be.false; + } + } + }; + + queryEquals( + [ + queryA.orderBy('foo').endBefore('a'), + queryB.orderBy('foo').endBefore('a'), + ], + [ + queryA.orderBy('foo').endBefore('b'), + queryB.orderBy('bar').endBefore('a'), + ] + ); + }); + + it('returns results', async () => { + const result: api.IRunAggregationQueryResponse = { + result: { + aggregateFields: { + count: {integerValue: '99'}, + }, + }, + readTime: {seconds: 5, nanos: 6}, + }; + const overrides: ApiOverride = { + runAggregationQuery: request => stream(result), + }; + + firestore = await createInstance(overrides); + + const query = firestore.collection('collectionId').count(); + return query.get().then(results => { + expect(results.getCount()).to.be.equal(99); + expect(results.readTime.isEqual(new Timestamp(5, 6))).to.be.true; + expect(results.query).to.be.equal(query); + }); + }); + + it('successful return without ending the stream on get()', async () => { + const result: api.IRunAggregationQueryResponse = { + result: { + aggregateFields: { + count: {integerValue: '99'}, + }, + }, + readTime: {seconds: 5, nanos: 6}, + }; + const overrides: ApiOverride = { + runAggregationQuery: request => streamWithoutEnd(result), + }; + + firestore = await createInstance(overrides); + + const query = firestore.collection('collectionId').count(); + return query.get().then(results => { + expect(results.getCount()).to.be.equal(99); + expect(results.readTime.isEqual(new Timestamp(5, 6))).to.be.true; + expect(results.query).to.be.equal(query); + }); + }); + + it('handles stream exception at initialization', () => { + const query = firestore.collection('collectionId').count(); + + query._stream = () => { + throw new Error('Expected error'); + }; + + return expect(query.get()).to.eventually.rejectedWith('Expected error'); + }); + + it('handles stream exception during initialization', async () => { + const overrides: ApiOverride = { + runAggregationQuery: () => { + return stream(new Error('Expected error')); + }, + }; + firestore = await createInstance(overrides); + + const query = firestore.collection('collectionId').count(); + await query + .get() + .then(() => { + throw new Error('Unexpected success in Promise'); + }) + .catch(err => { + expect(err.message).to.equal('Expected error'); + }); + }); + + it('handles stream exception after initialization (with get())', () => { + //Not required without retry logic. + //TODO(tomandersen) + }); +}); diff --git a/dev/test/query.ts b/dev/test/query.ts index 96fa57a54..23235e294 100644 --- a/dev/test/query.ts +++ b/dev/test/query.ts @@ -410,15 +410,15 @@ describe('query interface', () => { const queryB = firestore.collection('collectionId'); const queryEquals = (equals: Query[], notEquals: Query[]) => { - for (let i = 0; i < equals.length; ++i) { - for (const equal of equals) { - expect(equals[i].isEqual(equal)).to.be.true; - expect(equal.isEqual(equals[i])).to.be.true; + for (const equal1 of equals) { + for (const equal2 of equals) { + expect(equal1.isEqual(equal2)).to.be.true; + expect(equal2.isEqual(equal1)).to.be.true; } for (const notEqual of notEquals) { - expect(equals[i].isEqual(notEqual)).to.be.false; - expect(notEqual.isEqual(equals[i])).to.be.false; + expect(equal1.isEqual(notEqual)).to.be.false; + expect(notEqual.isEqual(equal1)).to.be.false; } } }; diff --git a/dev/test/util/helpers.ts b/dev/test/util/helpers.ts index aa8b258eb..f2d0c226b 100644 --- a/dev/test/util/helpers.ts +++ b/dev/test/util/helpers.ts @@ -24,7 +24,7 @@ import * as extend from 'extend'; import {JSONStreamIterator} from 'length-prefixed-json-stream'; import {Duplex, PassThrough} from 'stream'; import * as through2 from 'through2'; -import {firestore, google} from '../../protos/firestore_v1_proto_api'; +import {firestore} from '../../protos/firestore_v1_proto_api'; import type {grpc} from 'google-gax'; import * as proto from '../../protos/firestore_v1_proto_api'; import * as v1 from '../../src/v1'; diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 9dfd67079..0b9040350 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -2038,7 +2038,7 @@ declare namespace FirebaseFirestore { export class AggregateQuery { private constructor(); - readonly query: Query; + readonly query: Query; get(): Promise; From 6ddf4e9b3e9129b94fab67268492f9c5c7ae40fd Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Wed, 14 Sep 2022 09:38:07 -0400 Subject: [PATCH 04/28] Fix tests with better termination --- dev/src/reference.ts | 15 +++++++++------ dev/test/aggregateQuery.ts | 11 ++++++----- dev/test/query.ts | 2 +- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/dev/src/reference.ts b/dev/src/reference.ts index 1c382db1c..4a7ecd9ee 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -2286,10 +2286,10 @@ export class Query implements firestore.Query { callback(undefined, {document: lastReceivedDocument, readTime}); if (proto.done) { logger('Query._stream', tag, 'Trigger Logical Termination.'); - backendStream.unpipe(stream); - backendStream.resume(); - backendStream.end(); - stream.end(); + backendStream.unpipe(stream); + backendStream.resume(); + backendStream.end(); + stream.end(); } } else { callback(undefined, {readTime}); @@ -2906,8 +2906,7 @@ export class AggregateQuery implements firestore.AggregateQuery { callback(Error('unexpected')); } }, - }); - + }) firestore .initializeIfNeeded(tag) .then(async () => { @@ -2917,6 +2916,10 @@ export class AggregateQuery implements firestore.AggregateQuery { request, tag ); + stream.on('close', () => { + backendStream.resume() + backendStream.end(); + }) backendStream.on('error', err => { logger( 'AggregateQuery._stream', diff --git a/dev/test/aggregateQuery.ts b/dev/test/aggregateQuery.ts index 814eac3b8..c0e24d23d 100644 --- a/dev/test/aggregateQuery.ts +++ b/dev/test/aggregateQuery.ts @@ -25,21 +25,22 @@ import {expect, use} from 'chai'; import {google} from '../protos/firestore_v1_proto_api'; import api = google.firestore.v1; import * as chaiAsPromised from 'chai-as-promised'; +import {setTimeoutHandler} from "../src/backoff"; use(chaiAsPromised); describe('aggregate query interface', () => { let firestore: Firestore; beforeEach(() => { - // setTimeoutHandler(setImmediate); + setTimeoutHandler(setImmediate); return createInstance().then(firestoreInstance => { firestore = firestoreInstance; }); }); - afterEach(() => { - verifyInstance(firestore); - // setTimeoutHandler(setTimeout); + afterEach(async () => { + await verifyInstance(firestore); + setTimeoutHandler(setTimeout); }); it('has isEqual() method', () => { @@ -85,7 +86,7 @@ describe('aggregate query interface', () => { readTime: {seconds: 5, nanos: 6}, }; const overrides: ApiOverride = { - runAggregationQuery: request => stream(result), + runAggregationQuery: () => stream(result), }; firestore = await createInstance(overrides); diff --git a/dev/test/query.ts b/dev/test/query.ts index 23235e294..392425e2b 100644 --- a/dev/test/query.ts +++ b/dev/test/query.ts @@ -630,7 +630,7 @@ describe('query interface', () => { }); // Test Logical Termination on get() - it('successful return without ending the stream on get()', () => { + it('successful return without ending the stream on get()', async () => { const overrides: ApiOverride = { runQuery: request => { queryEquals(request); From eb422f91d83d16c8db416848427e1ac17567c5d6 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Wed, 14 Sep 2022 11:01:20 -0400 Subject: [PATCH 05/28] Copy Denvers API changes --- dev/src/reference.ts | 26 ++++++++++++++++---- dev/src/transaction.ts | 15 +++++++++--- dev/test/count.ts | 54 ++++++++++++++++++++++++++++++++++++++++++ types/firestore.d.ts | 53 +++++++++++++++++++++++++++++++---------- 4 files changed, 129 insertions(+), 19 deletions(-) create mode 100644 dev/test/count.ts diff --git a/dev/src/reference.ts b/dev/src/reference.ts index 4a7ecd9ee..e04297d41 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -1601,6 +1601,28 @@ export class Query implements firestore.Query { return new Query(this._firestore, options); } + /** + * Returns an `AggregateQuery` that counts the number of documents in the + * result set. + * + * @return an `AggregateQuery` that counts the number of documents in the + * result set. + */ + count(): AggregateQuery<{count: AggregateField}> { + throw new Error('not implemented'); + } + + /** + * Returns an `AggregateQuery` that performs the given aggregations. + * + * @param aggregates the aggregations to perform. + * @return an `AggregateQuery` that performs the given aggregations. + */ + aggregate(aggregates: T): AggregateQuery { + void aggregates; + throw new Error('not implemented'); + } + /** * Returns true if this `Query` is equal to the provided value. * @@ -2406,10 +2428,6 @@ export class Query implements firestore.Query { }, onError || console.error); } - count(): AggregateQuery { - return new AggregateQuery(this); - } - /** * Returns a function that can be used to sort QueryDocumentSnapshots * according to the sort criteria of this query. diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index 3c000c1ed..6917ad90d 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -106,7 +106,9 @@ export class Transaction implements firestore.Transaction { * @param aggregateQuery An aggregate query to execute. * @return An AggregateQuerySnapshot for the retrieved data. */ - get(aggregateQuery: AggregateQuery): Promise; + get( + aggregateQuery: AggregateQuery + ): Promise>; /** * Retrieve a document or a query result from the database. Holds a @@ -132,8 +134,15 @@ export class Transaction implements firestore.Transaction { * ``` */ get( - refOrQuery: DocumentReference | Query | AggregateQuery - ): Promise | QuerySnapshot | AggregateQuerySnapshot> { + refOrQuery: + | DocumentReference + | Query + | (T extends AggregateSpec ? AggregateQuery : never) + ): Promise< + | DocumentSnapshot + | QuerySnapshot + | (T extends AggregateSpec ? AggregateQuerySnapshot : never) + > { if (!this._writeBatch.isEmpty) { throw new Error(READ_AFTER_WRITE_ERROR_MSG); } diff --git a/dev/test/count.ts b/dev/test/count.ts new file mode 100644 index 000000000..3cc9f0b55 --- /dev/null +++ b/dev/test/count.ts @@ -0,0 +1,54 @@ +// 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 {Firestore} from '../src'; +import {AggregateField} from "@google-cloud/firestore"; + +export async function Demo0_NormalQuery(db: Firestore) { + const query = db.collection('games/halo/players'); + const snapshot = await query.get(); + expect(snapshot.size).to.equal(5000000); +} + +export async function Demo1_CountOfDocumentsInACollection(db: Firestore) { + const collection = db.collection('games/halo/players'); + const snapshot = await collection.count().get(); + expect(snapshot.data().count).to.equal(5000000); +} + +export async function Demo2_CountOfDocumentsInACollectionWithFilter(db: Firestore) { + const collection = db.collection('games/halo/players'); + const query = collection.where('online', '==', true); + const snapshot = await collection.count().get(); + expect(snapshot.data().count).to.equal(2000); +} + +export async function Demo3_MultipleAggregations(db: Firestore) { + const collection = db.collection('games/halo/players'); + const snapshot = await collection.aggregate({ + num_players: AggregateField.count(), + min_age: AggregateField.min('age'), + score: AggregateField.sum('score'), + }).get(); + const num_players: number = snapshot.data().num_players; + const min_age = snapshot.data().min_age ?? 0; + const total_points: number = snapshot.data().score ?? 0; + console.log( + `Found ${num_players} players, ` + + `the youngest being ${min_age} years old ` + + `with a total of ${total_points} points.` + ); +} diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 0b9040350..9309fce92 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -21,11 +21,14 @@ // Declare a global (ambient) namespace // (used when not using import statement, but just script include). declare namespace FirebaseFirestore { + // Alias for `any` but used where a Firestore field value would be provided. + 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}; /** * Similar to Typescript's `Partial`, but allows nested fields to be @@ -590,7 +593,9 @@ declare namespace FirebaseFirestore { * @param aggregateQuery An aggregate query to execute. * @return An AggregateQuerySnapshot for the retrieved data. */ - get(aggregateQuery: AggregateQuery): Promise; + get( + aggregateQuery: AggregateQuery + ): Promise>; /** * Retrieves multiple documents from Firestore. Holds a pessimistic lock on @@ -1701,7 +1706,9 @@ declare namespace FirebaseFirestore { onError?: (error: Error) => void ): () => void; - count(): AggregateQuery; + count(): AggregateQuery<{count: AggregateField}>; + + aggregate(aggregates: T): AggregateQuery; /** * Returns true if this `Query` is equal to the provided one. @@ -2023,26 +2030,48 @@ declare namespace FirebaseFirestore { toQuery(): Query; } - export class AggregateQuerySnapshot { + export class AggregateField { private constructor(); - readonly query: AggregateQuery; + static count(): AggregateField; + static min(field: string | FieldPath): AggregateField; + static max(field: string | FieldPath): AggregateField; + static sum(field: string | FieldPath): AggregateField; + static average(field: string | FieldPath): AggregateField; - readonly readTime: Timestamp; + isEqual(other: AggregateField): boolean; + } + + export type AggregateFieldType = ReturnType; + + export interface AggregateSpec { + [field: string]: AggregateFieldType; + } - getCount(): number; + export type AggregateSpecData = { + [P in keyof T]: T[P] extends AggregateField ? U : never; + }; + + export class AggregateQuery { + private constructor(); + + readonly query: Query; - isEqual(other: AggregateQuerySnapshot): boolean; + get(): Promise>; + + isEqual(other: AggregateQuery): boolean; } - export class AggregateQuery { + export class AggregateQuerySnapshot { private constructor(); - readonly query: Query; + readonly query: AggregateQuery; + + readonly readTime: Timestamp; - get(): Promise; + data(): AggregateSpecData; - isEqual(other: AggregateQuery): boolean; + isEqual(other: AggregateQuerySnapshot): boolean; } /** From 0d936e5768c9e12dbd298fdad074aae650d51bc2 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Thu, 15 Sep 2022 15:51:23 -0400 Subject: [PATCH 06/28] Implement COUNT API changes. --- dev/src/reference.ts | 121 ++++++++++++++++++++++++++++++----------- dev/src/transaction.ts | 11 ++-- types/firestore.d.ts | 5 +- 3 files changed, 98 insertions(+), 39 deletions(-) diff --git a/dev/src/reference.ts b/dev/src/reference.ts index e04297d41..a2829a42f 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -1608,8 +1608,9 @@ export class Query implements firestore.Query { * @return an `AggregateQuery` that counts the number of documents in the * result set. */ - count(): AggregateQuery<{count: AggregateField}> { - throw new Error('not implemented'); + + count(): AggregateQuery<{count: firestore.AggregateField}> { + return this.aggregate({count: AggregateField.count()}); } /** @@ -1618,9 +1619,8 @@ export class Query implements firestore.Query { * @param aggregates the aggregations to perform. * @return an `AggregateQuery` that performs the given aggregations. */ - aggregate(aggregates: T): AggregateQuery { - void aggregates; - throw new Error('not implemented'); + aggregate(aggregates: T): AggregateQuery { + return new AggregateQuery(this, aggregates); } /** @@ -2856,18 +2856,36 @@ export class CollectionReference } } -export class AggregateQuery implements firestore.AggregateQuery { +export class AggregateField implements firestore.AggregateField { + private constructor() {} + + static count(): AggregateField { + return new AggregateField(); + } + + isEqual(other: firestore.AggregateField): boolean { + return this === other || other instanceof AggregateField; + } +} + +export class AggregateQuery implements firestore.AggregateQuery { private readonly _query: Query; + private readonly _aggregates: T; - constructor(query: Query) { + constructor(query: Query, aggregates: T) { this._query = query; + this._aggregates = aggregates; } get query(): firestore.Query { return this._query; } - get(): Promise { + get aggregates(): T { + return this._aggregates; + } + + get(): Promise> { return this._get(); } @@ -2878,7 +2896,7 @@ export class AggregateQuery implements firestore.AggregateQuery { * @internal * @param {bytes=} transactionId A transaction ID. */ - _get(transactionId?: Uint8Array): Promise { + _get(transactionId?: Uint8Array): Promise> { // Capture the error stack to preserve stack tracing across async calls. const stack = Error().stack!; @@ -2914,11 +2932,15 @@ export class AggregateQuery implements firestore.AggregateQuery { transform: (proto: RunAggregationQueryResponse, enc, callback) => { const readTime = Timestamp.fromProto(proto.readTime!); if (proto.result) { - const result = new AggregateQuerySnapshot( - this, - readTime, - Number(proto.result.aggregateFields!.count.integerValue!) - ); + const data: firestore.DocumentData = {}; + const fields = proto.result.aggregateFields; + if (fields) { + const serializer = firestore._serializer!; + for (const prop of Object.keys(fields)) { + data[prop] = serializer.decodeValue(fields[prop]); + } + } + const result = new AggregateQuerySnapshot(this, readTime, data as any); //TODO(tomandersen) remove `as any` callback(undefined, result); } else { callback(Error('unexpected')); @@ -2931,6 +2953,7 @@ export class AggregateQuery implements firestore.AggregateQuery { const request = this.toProto(transactionId); const backendStream = await firestore.requestStream( 'runAggregationQuery', + /* bidirectional= */ false, request, tag ); @@ -2985,24 +3008,36 @@ export class AggregateQuery implements firestore.AggregateQuery { return runQueryRequest; } - isEqual(other: firestore.AggregateQuery): boolean { - return ( - this === other || - (other instanceof AggregateQuery && this._query.isEqual(other._query)) - ); + isEqual(other: firestore.AggregateQuery): boolean { + new AggregateQuery(null as any, {2: AggregateField.count()}) + if (this === other) { + return true; + } + if (other instanceof AggregateQuery) { + if (!this._query.isEqual(other._query)) { + return false; + } + + const thisAggregates: [string, AggregateField][] = Object.entries(this._aggregates); + const otherAggregates = other._aggregates; + + return thisAggregates.length === Object.keys(otherAggregates).length && + thisAggregates.every(([alias, field]) => field.isEqual(otherAggregates[alias])) + } + return false; } } -export class AggregateQuerySnapshot - implements firestore.AggregateQuerySnapshot +export class AggregateQuerySnapshot + implements firestore.AggregateQuerySnapshot { constructor( - private readonly _query: AggregateQuery, + private readonly _query: AggregateQuery, private readonly _readTime: Timestamp, - private readonly _count: number + private readonly _data: firestore.AggregateSpecData ) {} - get query(): firestore.AggregateQuery { + get query(): firestore.AggregateQuery { return this._query; } @@ -3011,17 +3046,37 @@ export class AggregateQuerySnapshot } getCount(): number { - return this._count; + const count = this._data.count; + if (typeof count == 'number') { + return count; + } + throw new Error('Unexpected count is ' + typeof count); } - isEqual(other: firestore.AggregateQuerySnapshot): boolean { - return ( - this === other || - (other instanceof AggregateQuerySnapshot && - this._query.isEqual(other._query) && - this._count === other._count && - this._readTime.isEqual(other._readTime)) - ); + isEqual(other: firestore.AggregateQuerySnapshot): boolean { + if (this === other) { + return true; + } + if (other instanceof AggregateQuerySnapshot) { + if (!this._query.isEqual(other._query)) { + return false; + } + + const thisData = this._data; + const thisDataKeys: string[] = Object.keys(thisData); + + const otherData = other._data; + const otherDataKeys: string[] = Object.keys(otherData); + + return thisDataKeys.length === otherDataKeys.length && thisDataKeys.every(alias => + Object.prototype.hasOwnProperty.call(otherData, alias) && thisData[alias] === otherData[alias] + ); + } + return false; + } + + data(): firestore.AggregateSpecData { + return this._data; } } diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index 6917ad90d..7fef69e12 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -106,7 +106,7 @@ export class Transaction implements firestore.Transaction { * @param aggregateQuery An aggregate query to execute. * @return An AggregateQuerySnapshot for the retrieved data. */ - get( + get( aggregateQuery: AggregateQuery ): Promise>; @@ -137,11 +137,11 @@ export class Transaction implements firestore.Transaction { refOrQuery: | DocumentReference | Query - | (T extends AggregateSpec ? AggregateQuery : never) + | (T extends firestore.AggregateSpec ? AggregateQuery : never) ): Promise< | DocumentSnapshot | QuerySnapshot - | (T extends AggregateSpec ? AggregateQuerySnapshot : never) + | (T extends firestore.AggregateSpec ? AggregateQuerySnapshot : never) > { if (!this._writeBatch.isEmpty) { throw new Error(READ_AFTER_WRITE_ERROR_MSG); @@ -158,7 +158,10 @@ export class Transaction implements firestore.Transaction { } if (refOrQuery instanceof AggregateQuery) { - return refOrQuery.get(); + //TODO(tomandersen) + const y: AggregateQuery = refOrQuery; + const x: Promise> = y.get(); + return x as any; } throw new Error( diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 9309fce92..467e3e488 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -2030,10 +2030,11 @@ declare namespace FirebaseFirestore { toQuery(): Query; } - export class AggregateField { + export class AggregateField { private constructor(); static count(): AggregateField; + //TODO(tomandersen) static min(field: string | FieldPath): AggregateField; static max(field: string | FieldPath): AggregateField; static sum(field: string | FieldPath): AggregateField; @@ -2055,7 +2056,7 @@ declare namespace FirebaseFirestore { export class AggregateQuery { private constructor(); - readonly query: Query; + readonly query: Query; get(): Promise>; From b1747ccf7dfc0d2ada412d729934bba36154f025 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Thu, 15 Sep 2022 16:56:10 -0400 Subject: [PATCH 07/28] Add comments --- dev/test/count.ts | 2 +- types/firestore.d.ts | 36 +++++++++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/dev/test/count.ts b/dev/test/count.ts index 3cc9f0b55..fc29399ad 100644 --- a/dev/test/count.ts +++ b/dev/test/count.ts @@ -15,7 +15,7 @@ import {expect} from 'chai'; import {Firestore} from '../src'; -import {AggregateField} from "@google-cloud/firestore"; +import {AggregateField} from "../src/reference"; export async function Demo0_NormalQuery(db: Firestore) { const query = db.collection('games/halo/players'); diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 467e3e488..8fa45b4aa 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -2030,10 +2030,18 @@ declare namespace FirebaseFirestore { toQuery(): Query; } - export class AggregateField { + /** + * An `AggregateField`that captures input type T. + */ + export class AggregateField { private constructor(); + /** + * Creates and returns an aggregation field that counts the documents in the result set. + * @returns An `AggregateField` object with number input type. + */ static count(): AggregateField; + //TODO(tomandersen) static min(field: string | FieldPath): AggregateField; static max(field: string | FieldPath): AggregateField; @@ -2043,12 +2051,26 @@ declare namespace FirebaseFirestore { isEqual(other: AggregateField): boolean; } + /** + * 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; }; @@ -2063,6 +2085,10 @@ declare namespace FirebaseFirestore { isEqual(other: AggregateQuery): boolean; } + + /** + * An `AggregateQuerySnapshot` contains the results of running an aggregate query. + */ export class AggregateQuerySnapshot { private constructor(); @@ -2070,6 +2096,14 @@ declare namespace FirebaseFirestore { readonly readTime: Timestamp; + /** + * 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; isEqual(other: AggregateQuerySnapshot): boolean; From 70b56599509402141cc5e3248b6fe0eacbad6405 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Thu, 15 Sep 2022 17:11:58 -0400 Subject: [PATCH 08/28] Fix linting errors --- dev/src/reference.ts | 27 ++++++++++++++++----------- dev/system-test/firestore.ts | 1 - dev/test/aggregateQuery.ts | 4 ++-- dev/test/count.ts | 2 +- dev/test/query.ts | 14 +++++++------- 5 files changed, 26 insertions(+), 22 deletions(-) diff --git a/dev/src/reference.ts b/dev/src/reference.ts index a2829a42f..c1a2c3dd6 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -1619,7 +1619,9 @@ export class Query implements firestore.Query { * @param aggregates the aggregations to perform. * @return an `AggregateQuery` that performs the given aggregations. */ - aggregate(aggregates: T): AggregateQuery { + aggregate( + aggregates: T + ): AggregateQuery { return new AggregateQuery(this, aggregates); } @@ -2308,10 +2310,10 @@ export class Query implements firestore.Query { callback(undefined, {document: lastReceivedDocument, readTime}); if (proto.done) { logger('Query._stream', tag, 'Trigger Logical Termination.'); - backendStream.unpipe(stream); - backendStream.resume(); - backendStream.end(); - stream.end(); + backendStream.unpipe(stream); + backendStream.resume(); + backendStream.end(); + stream.end(); } } else { callback(undefined, {readTime}); @@ -2868,7 +2870,9 @@ export class AggregateField implements firestore.AggregateField { } } -export class AggregateQuery implements firestore.AggregateQuery { +export class AggregateQuery + implements firestore.AggregateQuery { + private readonly _query: Query; private readonly _aggregates: T; @@ -2940,25 +2944,27 @@ export class AggregateQuery implements firest data[prop] = serializer.decodeValue(fields[prop]); } } - const result = new AggregateQuerySnapshot(this, readTime, data as any); //TODO(tomandersen) remove `as any` + //TODO(tomandersen) remove `as any` + const result = new AggregateQuerySnapshot(this, readTime, data as any); callback(undefined, result); } else { callback(Error('unexpected')); } }, - }) + }); + firestore .initializeIfNeeded(tag) .then(async () => { const request = this.toProto(transactionId); const backendStream = await firestore.requestStream( 'runAggregationQuery', - /* bidirectional= */ false, + /* bidirectional= */ false, request, tag ); stream.on('close', () => { - backendStream.resume() + backendStream.resume(); backendStream.end(); }) backendStream.on('error', err => { @@ -3009,7 +3015,6 @@ export class AggregateQuery implements firest } isEqual(other: firestore.AggregateQuery): boolean { - new AggregateQuery(null as any, {2: AggregateField.count()}) if (this === other) { return true; } diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index d863dfbf4..c8fd096fb 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -17,7 +17,6 @@ import { DocumentData, WithFieldValue, PartialWithFieldValue, - AggregateQuery, SetOptions, Settings, } from '@google-cloud/firestore'; diff --git a/dev/test/aggregateQuery.ts b/dev/test/aggregateQuery.ts index c0e24d23d..582ede285 100644 --- a/dev/test/aggregateQuery.ts +++ b/dev/test/aggregateQuery.ts @@ -25,7 +25,7 @@ import {expect, use} from 'chai'; import {google} from '../protos/firestore_v1_proto_api'; import api = google.firestore.v1; import * as chaiAsPromised from 'chai-as-promised'; -import {setTimeoutHandler} from "../src/backoff"; +import {setTimeoutHandler} from '../src/backoff'; use(chaiAsPromised); describe('aggregate query interface', () => { @@ -109,7 +109,7 @@ describe('aggregate query interface', () => { readTime: {seconds: 5, nanos: 6}, }; const overrides: ApiOverride = { - runAggregationQuery: request => streamWithoutEnd(result), + runAggregationQuery: () => streamWithoutEnd(result), }; firestore = await createInstance(overrides); diff --git a/dev/test/count.ts b/dev/test/count.ts index fc29399ad..e7e3c0c3e 100644 --- a/dev/test/count.ts +++ b/dev/test/count.ts @@ -15,7 +15,7 @@ import {expect} from 'chai'; import {Firestore} from '../src'; -import {AggregateField} from "../src/reference"; +import {AggregateField} from '../src/reference'; export async function Demo0_NormalQuery(db: Firestore) { const query = db.collection('games/halo/players'); diff --git a/dev/test/query.ts b/dev/test/query.ts index 392425e2b..96fa57a54 100644 --- a/dev/test/query.ts +++ b/dev/test/query.ts @@ -410,15 +410,15 @@ describe('query interface', () => { const queryB = firestore.collection('collectionId'); const queryEquals = (equals: Query[], notEquals: Query[]) => { - for (const equal1 of equals) { - for (const equal2 of equals) { - expect(equal1.isEqual(equal2)).to.be.true; - expect(equal2.isEqual(equal1)).to.be.true; + for (let i = 0; i < equals.length; ++i) { + for (const equal of equals) { + expect(equals[i].isEqual(equal)).to.be.true; + expect(equal.isEqual(equals[i])).to.be.true; } for (const notEqual of notEquals) { - expect(equal1.isEqual(notEqual)).to.be.false; - expect(notEqual.isEqual(equal1)).to.be.false; + expect(equals[i].isEqual(notEqual)).to.be.false; + expect(notEqual.isEqual(equals[i])).to.be.false; } } }; @@ -630,7 +630,7 @@ describe('query interface', () => { }); // Test Logical Termination on get() - it('successful return without ending the stream on get()', async () => { + it('successful return without ending the stream on get()', () => { const overrides: ApiOverride = { runQuery: request => { queryEquals(request); From a06f1105988b8fa6a7ab43c1e6d735d684679ee5 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 16 Sep 2022 10:05:26 -0400 Subject: [PATCH 09/28] Revert manual proto change --- dev/protos/google/firestore/v1/query.proto | 3 --- 1 file changed, 3 deletions(-) diff --git a/dev/protos/google/firestore/v1/query.proto b/dev/protos/google/firestore/v1/query.proto index ddbfee7b3..0d7a7b6bf 100644 --- a/dev/protos/google/firestore/v1/query.proto +++ b/dev/protos/google/firestore/v1/query.proto @@ -67,9 +67,6 @@ message StructuredQuery { // Documents are required to satisfy all of the combined filters. AND = 1; - - // Documents are required to satisfy at least one of the combined filters. - OR = 2; } // The operator for combining multiple filters. From 0ecf768834c1e68a6e0a80cc6f37181353fbf56f Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 16 Sep 2022 12:03:18 -0400 Subject: [PATCH 10/28] Add comments --- dev/src/reference.ts | 2 +- types/firestore.d.ts | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/dev/src/reference.ts b/dev/src/reference.ts index c1a2c3dd6..189b33700 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -1608,7 +1608,6 @@ export class Query implements firestore.Query { * @return an `AggregateQuery` that counts the number of documents in the * result set. */ - count(): AggregateQuery<{count: firestore.AggregateField}> { return this.aggregate({count: AggregateField.count()}); } @@ -2994,6 +2993,7 @@ export class AggregateQuery */ toProto(transactionId?: Uint8Array): api.IRunAggregationQueryRequest { const queryProto = this._query.toProto(); + //TODO(tomandersen) inspect _query to build request - this is just hard coded count right now. const runQueryRequest: api.IRunAggregationQueryRequest = { parent: queryProto.parent, structuredAggregationQuery: { diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 8fa45b4aa..48476c798 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -1706,8 +1706,19 @@ declare namespace FirebaseFirestore { onError?: (error: Error) => void ): () => void; + /** + * Returns count `AggregateQuery` based on this `Query`. + * + * @return AggregateQuery that contains count aggregate. + */ count(): AggregateQuery<{count: AggregateField}>; + /** + * Returns `AggregateQuery` for given aggregates based on this `Query`. + * + * @param aggregates Specify aliases with aggregate functions. + * @return An AggregateQuery that contains given aggregates. + */ aggregate(aggregates: T): AggregateQuery; /** @@ -2048,6 +2059,13 @@ declare namespace FirebaseFirestore { static sum(field: string | FieldPath): AggregateField; static average(field: string | FieldPath): AggregateField; + /** + * Returns true if the aggregate function in this `AggregateField` is equal to the + * provided one. + * + * @param other The `AggregateField` to compare against. + * @return true if this `AggregateField` is equal to the provided one. + */ isEqual(other: AggregateField): boolean; } @@ -2080,8 +2098,17 @@ declare namespace FirebaseFirestore { readonly query: Query; + readonly aggregates: T; + get(): Promise>; + /** + * Returns true if the query and aggregates in this `AggregateQuery` is equal to the + * provided one. + * + * @param other The `AggregateQuery` to compare against. + * @return true if this `AggregateQuery` is equal to the provided one. + */ isEqual(other: AggregateQuery): boolean; } @@ -2106,6 +2133,13 @@ declare namespace FirebaseFirestore { */ data(): AggregateSpecData; + /** + * Returns true if the query, read time, and data in this `AggregateQuerySnapshot` + * is equal to the provided one. + * + * @param other The `AggregateQuerySnapshot` to compare against. + * @return true if this `AggregateQuerySnapshot` is equal to the provided one. + */ isEqual(other: AggregateQuerySnapshot): boolean; } From f396bdaea530e36c49341cf5670761c8a2071376 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 16 Sep 2022 14:13:21 -0400 Subject: [PATCH 11/28] Fix types --- dev/src/reference.ts | 5 ++--- dev/src/transaction.ts | 15 ++++++--------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/dev/src/reference.ts b/dev/src/reference.ts index 189b33700..31798adfd 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -2935,7 +2935,7 @@ export class AggregateQuery transform: (proto: RunAggregationQueryResponse, enc, callback) => { const readTime = Timestamp.fromProto(proto.readTime!); if (proto.result) { - const data: firestore.DocumentData = {}; + const data: any = {}; const fields = proto.result.aggregateFields; if (fields) { const serializer = firestore._serializer!; @@ -2943,8 +2943,7 @@ export class AggregateQuery data[prop] = serializer.decodeValue(fields[prop]); } } - //TODO(tomandersen) remove `as any` - const result = new AggregateQuerySnapshot(this, readTime, data as any); + const result = new AggregateQuerySnapshot(this, readTime, data); callback(undefined, result); } else { callback(Error('unexpected')); diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index 7fef69e12..8b4d2935b 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -133,15 +133,15 @@ export class Transaction implements firestore.Transaction { * }); * ``` */ - get( + get( refOrQuery: | DocumentReference | Query - | (T extends firestore.AggregateSpec ? AggregateQuery : never) + | AggregateQuery ): Promise< | DocumentSnapshot | QuerySnapshot - | (T extends firestore.AggregateSpec ? AggregateQuerySnapshot : never) + | AggregateQuerySnapshot > { if (!this._writeBatch.isEmpty) { throw new Error(READ_AFTER_WRITE_ERROR_MSG); @@ -157,15 +157,12 @@ export class Transaction implements firestore.Transaction { return refOrQuery._get(this._transactionId); } - if (refOrQuery instanceof AggregateQuery) { - //TODO(tomandersen) - const y: AggregateQuery = refOrQuery; - const x: Promise> = y.get(); - return x as any; + if (refOrQuery instanceof AggregateQuery) { + return refOrQuery.get(); } throw new Error( - 'Value for argument "refOrQuery" must be a DocumentReference or a Query.' + 'Value for argument "refOrQuery" must be a DocumentReference, Query, or AggregateQuery.' ); } From 349ca1e911d67b599d6a8948f41bb3dff3695a22 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 16 Sep 2022 16:06:14 -0400 Subject: [PATCH 12/28] Implement retry --- dev/src/reference.ts | 124 ++++++++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 54 deletions(-) diff --git a/dev/src/reference.ts b/dev/src/reference.ts index 31798adfd..136dcabd5 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -20,32 +20,16 @@ import * as deepEqual from 'fast-deep-equal'; import * as protos from '../protos/firestore_v1_proto_api'; -import { - DocumentSnapshot, - DocumentSnapshotBuilder, - QueryDocumentSnapshot, -} from './document'; +import {DocumentSnapshot, DocumentSnapshotBuilder, QueryDocumentSnapshot,} from './document'; import {DocumentChange} from './document-change'; import {Firestore} from './index'; import {logger} from './logger'; import {compare} from './order'; -import { - FieldPath, - QualifiedResourcePath, - ResourcePath, - validateFieldPath, - validateResourcePath, -} from './path'; +import {FieldPath, QualifiedResourcePath, ResourcePath, validateFieldPath, validateResourcePath,} from './path'; import {Serializable, Serializer, validateUserInput} from './serializer'; import {Timestamp} from './timestamp'; import {defaultConverter} from './types'; -import { - autoId, - Deferred, - isPermanentRpcError, - requestTag, - wrapError, -} from './util'; +import {autoId, Deferred, isPermanentRpcError, requestTag, wrapError,} from './util'; import { invalidArgumentMessage, validateEnumValue, @@ -55,10 +39,7 @@ import { } from './validate'; import {DocumentWatch, QueryWatch} from './watch'; import {validateDocumentData, WriteBatch, WriteResult} from './write-batch'; - import api = protos.google.firestore.v1; -import {google} from '../protos/firestore_v1_proto_api'; -import RunAggregationQueryResponse = google.firestore.v1.RunAggregationQueryResponse; /** * The direction of a `Query.orderBy()` clause is specified as 'desc' or 'asc' @@ -2932,19 +2913,11 @@ export class AggregateQuery const stream: Transform = new Transform({ objectMode: true, - transform: (proto: RunAggregationQueryResponse, enc, callback) => { - const readTime = Timestamp.fromProto(proto.readTime!); + transform: (proto: api.IRunAggregationQueryResponse, enc, callback) => { if (proto.result) { - const data: any = {}; - const fields = proto.result.aggregateFields; - if (fields) { - const serializer = firestore._serializer!; - for (const prop of Object.keys(fields)) { - data[prop] = serializer.decodeValue(fields[prop]); - } - } - const result = new AggregateQuerySnapshot(this, readTime, data); - callback(undefined, result); + const readTime = Timestamp.fromProto(proto.readTime!); + const data = this.decodeResult(proto.result); + callback(undefined, new AggregateQuerySnapshot(this, readTime, data)); } else { callback(Error('unexpected')); } @@ -2954,34 +2927,77 @@ export class AggregateQuery firestore .initializeIfNeeded(tag) .then(async () => { + // `toProto()` might throw an exception. We rely on the behavior of an + // async function to convert this exception into the rejected Promise we + // catch below. const request = this.toProto(transactionId); - const backendStream = await firestore.requestStream( - 'runAggregationQuery', - /* bidirectional= */ false, - request, - tag - ); - stream.on('close', () => { - backendStream.resume(); - backendStream.end(); - }) - backendStream.on('error', err => { - logger( - 'AggregateQuery._stream', - tag, - 'AggregateQuery failed with stream error:', - err + + let streamActive: Deferred; + do { + streamActive = new Deferred(); + const backendStream = await firestore.requestStream( + 'runAggregationQuery', + /* bidirectional= */ false, + request, + tag ); - stream.destroy(err); - }); - backendStream.resume(); - backendStream.pipe(stream); + stream.on('close', () => { + backendStream.resume(); + backendStream.end(); + }) + backendStream.on('error', err => { + backendStream.unpipe(stream); + // If a non-transactional query failed, attempt to restart. + // Transactional queries are retried via the transaction runner. + if (!transactionId && !isPermanentRpcError(err, 'runQuery')) { + logger( + 'Query._stream', + tag, + 'Query failed with retryable stream error:', + err + ); + streamActive.resolve(/* active= */ true); + } else { + logger( + 'AggregateQuery._stream', + tag, + 'AggregateQuery failed with stream error:', + err + ); + stream.destroy(err); + streamActive.resolve(/* active= */ false); + } + }); + backendStream.resume(); + backendStream.pipe(stream); + } while (await streamActive.promise); }) .catch(e => stream.destroy(e)); return stream; } + /** + * Internal method to decode values within result. + * + * @param proto + * @private + */ + private decodeResult(proto: api.IAggregationResult): firestore.AggregateSpecData { + const data: any = {}; + const fields = proto.aggregateFields; + if (fields) { + const serializer = this._query.firestore._serializer!; + for (const prop of Object.keys(fields)) { + if (this._aggregates[prop] === undefined) { + throw new Error(`Unexpected alias [${prop}] in result aggregate result`); + } + data[prop] = serializer.decodeValue(fields[prop]); + } + } + return data; + } + /** * Internal method for serializing a query to its RunAggregationQuery proto * representation with an optional transaction id or read time. From f162f1253bac4d974b824bda4d85f652a56786eb Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 16 Sep 2022 16:23:17 -0400 Subject: [PATCH 13/28] Pass transaction --- dev/src/transaction.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index 8b4d2935b..ed46dbada 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -158,7 +158,7 @@ export class Transaction implements firestore.Transaction { } if (refOrQuery instanceof AggregateQuery) { - return refOrQuery.get(); + return refOrQuery._get(this._transactionId); } throw new Error( From 63fa8d803ee24160d8f22493a9c42a69cc48acfc Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 16 Sep 2022 16:25:09 -0400 Subject: [PATCH 14/28] Comment out test --- dev/test/count.ts | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/dev/test/count.ts b/dev/test/count.ts index e7e3c0c3e..cad3220ac 100644 --- a/dev/test/count.ts +++ b/dev/test/count.ts @@ -36,19 +36,19 @@ export async function Demo2_CountOfDocumentsInACollectionWithFilter(db: Firestor expect(snapshot.data().count).to.equal(2000); } -export async function Demo3_MultipleAggregations(db: Firestore) { - const collection = db.collection('games/halo/players'); - const snapshot = await collection.aggregate({ - num_players: AggregateField.count(), - min_age: AggregateField.min('age'), - score: AggregateField.sum('score'), - }).get(); - const num_players: number = snapshot.data().num_players; - const min_age = snapshot.data().min_age ?? 0; - const total_points: number = snapshot.data().score ?? 0; - console.log( - `Found ${num_players} players, ` + - `the youngest being ${min_age} years old ` + - `with a total of ${total_points} points.` - ); -} +// export async function Demo3_MultipleAggregations(db: Firestore) { +// const collection = db.collection('games/halo/players'); +// const snapshot = await collection.aggregate({ +// num_players: AggregateField.count(), +// min_age: AggregateField.min('age'), +// score: AggregateField.sum('score'), +// }).get(); +// const num_players: number = snapshot.data().num_players; +// const min_age = snapshot.data().min_age ?? 0; +// const total_points: number = snapshot.data().score ?? 0; +// console.log( +// `Found ${num_players} players, ` + +// `the youngest being ${min_age} years old ` + +// `with a total of ${total_points} points.` +// ); +// } From 8871496fbc1a6a0fe5d2bed29376f6945be0c908 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 16 Sep 2022 16:29:02 -0400 Subject: [PATCH 15/28] Fix test --- dev/test/transaction.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/test/transaction.ts b/dev/test/transaction.ts index 8854bc159..006979272 100644 --- a/dev/test/transaction.ts +++ b/dev/test/transaction.ts @@ -738,11 +738,11 @@ describe('transaction operations', () => { /* transactionOptions= */ {}, (transaction: InvalidApiUsage) => { expect(() => transaction.get()).to.throw( - 'Value for argument "refOrQuery" must be a DocumentReference or a Query.' + 'Value for argument "refOrQuery" must be a DocumentReference, Query, or AggregateQuery.' ); expect(() => transaction.get('foo')).to.throw( - 'Value for argument "refOrQuery" must be a DocumentReference or a Query.' + 'Value for argument "refOrQuery" must be a DocumentReference, Query, or AggregateQuery.' ); return Promise.resolve(); From 12aaf1631de4c514c8c7b8564381783c8611bc40 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 16 Sep 2022 16:29:39 -0400 Subject: [PATCH 16/28] Remove demo file --- dev/test/count.ts | 54 ----------------------------------------------- 1 file changed, 54 deletions(-) delete mode 100644 dev/test/count.ts diff --git a/dev/test/count.ts b/dev/test/count.ts deleted file mode 100644 index cad3220ac..000000000 --- a/dev/test/count.ts +++ /dev/null @@ -1,54 +0,0 @@ -// 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 {Firestore} from '../src'; -import {AggregateField} from '../src/reference'; - -export async function Demo0_NormalQuery(db: Firestore) { - const query = db.collection('games/halo/players'); - const snapshot = await query.get(); - expect(snapshot.size).to.equal(5000000); -} - -export async function Demo1_CountOfDocumentsInACollection(db: Firestore) { - const collection = db.collection('games/halo/players'); - const snapshot = await collection.count().get(); - expect(snapshot.data().count).to.equal(5000000); -} - -export async function Demo2_CountOfDocumentsInACollectionWithFilter(db: Firestore) { - const collection = db.collection('games/halo/players'); - const query = collection.where('online', '==', true); - const snapshot = await collection.count().get(); - expect(snapshot.data().count).to.equal(2000); -} - -// export async function Demo3_MultipleAggregations(db: Firestore) { -// const collection = db.collection('games/halo/players'); -// const snapshot = await collection.aggregate({ -// num_players: AggregateField.count(), -// min_age: AggregateField.min('age'), -// score: AggregateField.sum('score'), -// }).get(); -// const num_players: number = snapshot.data().num_players; -// const min_age = snapshot.data().min_age ?? 0; -// const total_points: number = snapshot.data().score ?? 0; -// console.log( -// `Found ${num_players} players, ` + -// `the youngest being ${min_age} years old ` + -// `with a total of ${total_points} points.` -// ); -// } From 999b9106d32b49fbef980d9ddd08aec35a19d790 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Fri, 16 Sep 2022 16:47:39 -0400 Subject: [PATCH 17/28] Run integration test --- dev/system-test/firestore.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index c8fd096fb..e9cb4e2bf 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -2140,20 +2140,22 @@ describe('Query class', () => { }); }); -describe.skip('Transaction class (with Emulator)', () => { +describe.only('Transaction class (with Emulator)', () => { let firestore: Firestore; let randomCol: CollectionReference; setLogFunction(console.log); beforeEach(() => { // This cannot be hard coded to emulator. - firestore = new Firestore({ - host: 'localhost', - port: 8539, - projectId: 'my-cool-project', - ssl: false, - }); - randomCol = getTestRoot(firestore); + // firestore = new Firestore({ + // host: 'localhost', + // port: 8539, + // projectId: 'my-cool-project', + // ssl: false, + // }); + // randomCol = getTestRoot(firestore); + randomCol = getTestRoot(); + firestore = randomCol.firestore; }); afterEach(() => verifyInstance(firestore)); From 4d2ee83bcdcd0e59e1222f767ffeb07bf9cdfce0 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 16 Sep 2022 20:49:46 +0000 Subject: [PATCH 18/28] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- dev/src/reference.ts | 89 ++++++++++++++++++++++++++++------------- dev/test/transaction.ts | 4 +- 2 files changed, 63 insertions(+), 30 deletions(-) diff --git a/dev/src/reference.ts b/dev/src/reference.ts index 136dcabd5..6430b62a8 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -20,16 +20,32 @@ import * as deepEqual from 'fast-deep-equal'; import * as protos from '../protos/firestore_v1_proto_api'; -import {DocumentSnapshot, DocumentSnapshotBuilder, QueryDocumentSnapshot,} from './document'; +import { + DocumentSnapshot, + DocumentSnapshotBuilder, + QueryDocumentSnapshot, +} from './document'; import {DocumentChange} from './document-change'; import {Firestore} from './index'; import {logger} from './logger'; import {compare} from './order'; -import {FieldPath, QualifiedResourcePath, ResourcePath, validateFieldPath, validateResourcePath,} from './path'; +import { + FieldPath, + QualifiedResourcePath, + ResourcePath, + validateFieldPath, + validateResourcePath, +} from './path'; import {Serializable, Serializer, validateUserInput} from './serializer'; import {Timestamp} from './timestamp'; import {defaultConverter} from './types'; -import {autoId, Deferred, isPermanentRpcError, requestTag, wrapError,} from './util'; +import { + autoId, + Deferred, + isPermanentRpcError, + requestTag, + wrapError, +} from './util'; import { invalidArgumentMessage, validateEnumValue, @@ -1600,7 +1616,7 @@ export class Query implements firestore.Query { * @return an `AggregateQuery` that performs the given aggregations. */ aggregate( - aggregates: T + aggregates: T ): AggregateQuery { return new AggregateQuery(this, aggregates); } @@ -2851,8 +2867,8 @@ export class AggregateField implements firestore.AggregateField { } export class AggregateQuery - implements firestore.AggregateQuery { - + implements firestore.AggregateQuery +{ private readonly _query: Query; private readonly _aggregates: T; @@ -2917,7 +2933,10 @@ export class AggregateQuery if (proto.result) { const readTime = Timestamp.fromProto(proto.readTime!); const data = this.decodeResult(proto.result); - callback(undefined, new AggregateQuerySnapshot(this, readTime, data)); + callback( + undefined, + new AggregateQuerySnapshot(this, readTime, data) + ); } else { callback(Error('unexpected')); } @@ -2936,33 +2955,33 @@ export class AggregateQuery do { streamActive = new Deferred(); const backendStream = await firestore.requestStream( - 'runAggregationQuery', - /* bidirectional= */ false, - request, - tag + 'runAggregationQuery', + /* bidirectional= */ false, + request, + tag ); stream.on('close', () => { backendStream.resume(); backendStream.end(); - }) + }); backendStream.on('error', err => { backendStream.unpipe(stream); // If a non-transactional query failed, attempt to restart. // Transactional queries are retried via the transaction runner. if (!transactionId && !isPermanentRpcError(err, 'runQuery')) { logger( - 'Query._stream', - tag, - 'Query failed with retryable stream error:', - err + 'Query._stream', + tag, + 'Query failed with retryable stream error:', + err ); streamActive.resolve(/* active= */ true); } else { logger( - 'AggregateQuery._stream', - tag, - 'AggregateQuery failed with stream error:', - err + 'AggregateQuery._stream', + tag, + 'AggregateQuery failed with stream error:', + err ); stream.destroy(err); streamActive.resolve(/* active= */ false); @@ -2983,14 +3002,18 @@ export class AggregateQuery * @param proto * @private */ - private decodeResult(proto: api.IAggregationResult): firestore.AggregateSpecData { + private decodeResult( + proto: api.IAggregationResult + ): firestore.AggregateSpecData { const data: any = {}; const fields = proto.aggregateFields; if (fields) { const serializer = this._query.firestore._serializer!; for (const prop of Object.keys(fields)) { if (this._aggregates[prop] === undefined) { - throw new Error(`Unexpected alias [${prop}] in result aggregate result`); + throw new Error( + `Unexpected alias [${prop}] in result aggregate result` + ); } data[prop] = serializer.decodeValue(fields[prop]); } @@ -3038,11 +3061,16 @@ export class AggregateQuery return false; } - const thisAggregates: [string, AggregateField][] = Object.entries(this._aggregates); + const thisAggregates: [string, AggregateField][] = + Object.entries(this._aggregates); const otherAggregates = other._aggregates; - return thisAggregates.length === Object.keys(otherAggregates).length && - thisAggregates.every(([alias, field]) => field.isEqual(otherAggregates[alias])) + return ( + thisAggregates.length === Object.keys(otherAggregates).length && + thisAggregates.every(([alias, field]) => + field.isEqual(otherAggregates[alias]) + ) + ); } return false; } @@ -3067,7 +3095,7 @@ export class AggregateQuerySnapshot getCount(): number { const count = this._data.count; - if (typeof count == 'number') { + if (typeof count === 'number') { return count; } throw new Error('Unexpected count is ' + typeof count); @@ -3088,8 +3116,13 @@ export class AggregateQuerySnapshot const otherData = other._data; const otherDataKeys: string[] = Object.keys(otherData); - return thisDataKeys.length === otherDataKeys.length && thisDataKeys.every(alias => - Object.prototype.hasOwnProperty.call(otherData, alias) && thisData[alias] === otherData[alias] + return ( + thisDataKeys.length === otherDataKeys.length && + thisDataKeys.every( + alias => + Object.prototype.hasOwnProperty.call(otherData, alias) && + thisData[alias] === otherData[alias] + ) ); } return false; diff --git a/dev/test/transaction.ts b/dev/test/transaction.ts index 006979272..19088425d 100644 --- a/dev/test/transaction.ts +++ b/dev/test/transaction.ts @@ -738,11 +738,11 @@ describe('transaction operations', () => { /* transactionOptions= */ {}, (transaction: InvalidApiUsage) => { expect(() => transaction.get()).to.throw( - 'Value for argument "refOrQuery" must be a DocumentReference, Query, or AggregateQuery.' + 'Value for argument "refOrQuery" must be a DocumentReference, Query, or AggregateQuery.' ); expect(() => transaction.get('foo')).to.throw( - 'Value for argument "refOrQuery" must be a DocumentReference, Query, or AggregateQuery.' + 'Value for argument "refOrQuery" must be a DocumentReference, Query, or AggregateQuery.' ); return Promise.resolve(); From 401b843b3b52c2026afc7bbc93423d2ef692f926 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Mon, 19 Sep 2022 10:15:34 -0400 Subject: [PATCH 19/28] Remove future aggregates --- types/firestore.d.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 48476c798..358983487 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -2053,12 +2053,6 @@ declare namespace FirebaseFirestore { */ static count(): AggregateField; - //TODO(tomandersen) - static min(field: string | FieldPath): AggregateField; - static max(field: string | FieldPath): AggregateField; - static sum(field: string | FieldPath): AggregateField; - static average(field: string | FieldPath): AggregateField; - /** * Returns true if the aggregate function in this `AggregateField` is equal to the * provided one. From 63ed8bbbbbaaef715afa246fdb3d3f818385ebac Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Mon, 19 Sep 2022 15:27:37 -0400 Subject: [PATCH 20/28] Fix emulator --- dev/system-test/firestore.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index e9cb4e2bf..e6734e21b 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -51,7 +51,7 @@ import { verifyInstance, } from '../test/util/helpers'; import {BulkWriter} from '../src/bulk-writer'; -import {Status} from 'google-gax/build/src/status'; +import {Status} from 'google-gax'; import {QueryPartition} from '../src/query-partition'; import {CollectionGroup} from '../src/collection-group'; @@ -2147,13 +2147,12 @@ describe.only('Transaction class (with Emulator)', () => { beforeEach(() => { // This cannot be hard coded to emulator. - // firestore = new Firestore({ + // randomCol = getTestRoot({ // host: 'localhost', - // port: 8539, + // port: 8080, // projectId: 'my-cool-project', // ssl: false, // }); - // randomCol = getTestRoot(firestore); randomCol = getTestRoot(); firestore = randomCol.firestore; }); From c58bad7d41ae9292977f7c9a57b9a007ef68d29a Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Mon, 19 Sep 2022 15:30:47 -0400 Subject: [PATCH 21/28] Enable test --- dev/system-test/firestore.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index e6734e21b..dff1c558e 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -2140,7 +2140,7 @@ describe('Query class', () => { }); }); -describe.only('Transaction class (with Emulator)', () => { +describe('Aggregates', () => { let firestore: Firestore; let randomCol: CollectionReference; setLogFunction(console.log); From dc8c3a5eae46691f62f83beb3d4afa337d7887e1 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Mon, 19 Sep 2022 15:55:05 -0400 Subject: [PATCH 22/28] Prettier --- dev/src/transaction.ts | 9 ++------- types/firestore.d.ts | 1 - 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index ed46dbada..c61f4891e 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -134,14 +134,9 @@ export class Transaction implements firestore.Transaction { * ``` */ get( - refOrQuery: - | DocumentReference - | Query - | AggregateQuery + refOrQuery: DocumentReference | Query | AggregateQuery ): Promise< - | DocumentSnapshot - | QuerySnapshot - | AggregateQuerySnapshot + DocumentSnapshot | QuerySnapshot | AggregateQuerySnapshot > { if (!this._writeBatch.isEmpty) { throw new Error(READ_AFTER_WRITE_ERROR_MSG); diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 358983487..48e62d2b8 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -2106,7 +2106,6 @@ declare namespace FirebaseFirestore { isEqual(other: AggregateQuery): boolean; } - /** * An `AggregateQuerySnapshot` contains the results of running an aggregate query. */ From dbf9d1acde6e2f3c11fb43dc0142dcb809c24d8f Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Tue, 20 Sep 2022 10:00:01 -0400 Subject: [PATCH 23/28] Cleanup --- dev/system-test/firestore.ts | 9 +-------- dev/test/aggregateQuery.ts | 5 ----- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index dff1c558e..9754a2997 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -2140,19 +2140,12 @@ describe('Query class', () => { }); }); -describe('Aggregates', () => { +describe.only('Aggregates', () => { let firestore: Firestore; let randomCol: CollectionReference; setLogFunction(console.log); beforeEach(() => { - // This cannot be hard coded to emulator. - // randomCol = getTestRoot({ - // host: 'localhost', - // port: 8080, - // projectId: 'my-cool-project', - // ssl: false, - // }); randomCol = getTestRoot(); firestore = randomCol.firestore; }); diff --git a/dev/test/aggregateQuery.ts b/dev/test/aggregateQuery.ts index 582ede285..43c08779a 100644 --- a/dev/test/aggregateQuery.ts +++ b/dev/test/aggregateQuery.ts @@ -150,9 +150,4 @@ describe('aggregate query interface', () => { expect(err.message).to.equal('Expected error'); }); }); - - it('handles stream exception after initialization (with get())', () => { - //Not required without retry logic. - //TODO(tomandersen) - }); }); From 046399c939b8a5b7cf14de240cc7eda2e999352b Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Tue, 20 Sep 2022 10:35:55 -0400 Subject: [PATCH 24/28] Add tests --- dev/system-test/firestore.ts | 48 +++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index 9754a2997..3f7eb0527 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -2152,7 +2152,14 @@ describe.only('Aggregates', () => { afterEach(() => verifyInstance(firestore)); - it('counts 0 document', async () => { + it('counts 0 document from non-existent collection', async () => { + const count = randomCol.count(); + const res = await firestore.runTransaction(f => f.get(count)); + expect(res.getCount()).to.equal(0); + }); + + it('counts 0 document from filtered empty collection', async () => { + await randomCol.doc('doc').set({foo: 'bar'}); const count = randomCol.where('foo', '==', 'notbar').count(); const res = await firestore.runTransaction(f => f.get(count)); expect(res.getCount()).to.equal(0); @@ -2160,18 +2167,53 @@ describe.only('Aggregates', () => { it('counts 1 document', async () => { await randomCol.doc('doc').set({foo: 'bar'}); - const count = randomCol.where('foo', '==', 'bar').count(); + const count = randomCol.count(); const res = await firestore.runTransaction(f => f.get(count)); expect(res.getCount()).to.equal(1); }); - it('counts multiple documents', async () => { + it('counts multiple documents with filter', async () => { await randomCol.doc('doc1').set({foo: 'bar'}); await randomCol.doc('doc2').set({foo: 'bar'}); + await randomCol.doc('doc3').set({foo: 'notbar'}); + await randomCol.doc('doc3').set({notfoo: 'bar'}); const count = randomCol.where('foo', '==', 'bar').count(); const res = await firestore.runTransaction(f => f.get(count)); expect(res.getCount()).to.equal(2); }); + + it('counts up to limit', async () => { + await randomCol.doc('doc1').set({foo: 'bar'}); + await randomCol.doc('doc2').set({foo: 'bar'}); + await randomCol.doc('doc3').set({foo: 'bar'}); + await randomCol.doc('doc4').set({foo: 'bar'}); + await randomCol.doc('doc5').set({foo: 'bar'}); + await randomCol.doc('doc6').set({foo: 'bar'}); + await randomCol.doc('doc7').set({foo: 'bar'}); + await randomCol.doc('doc8').set({foo: 'bar'}); + const count = randomCol.limit(5).count(); + const res = await firestore.runTransaction(f => f.get(count)); + expect(res.getCount()).to.equal(5); + }); + + it('counts with orderBy', async () => { + await randomCol.doc('doc1').set({foo1: 'bar1'}); + await randomCol.doc('doc2').set({foo1: 'bar2'}); + await randomCol.doc('doc3').set({foo1: 'bar3'}); + await randomCol.doc('doc4').set({foo1: 'bar4'}); + await randomCol.doc('doc5').set({foo1: 'bar5'}); + await randomCol.doc('doc6').set({foo2: 'bar6'}); + await randomCol.doc('doc7').set({foo2: 'bar7'}); + await randomCol.doc('doc8').set({foo2: 'bar8'}); + + const count1 = randomCol.orderBy('foo2').count(); + const res1 = await firestore.runTransaction(f => f.get(count1)); + expect(res1.getCount()).to.equal(3); + + const count2 = randomCol.orderBy('foo3').count(); + const res2 = await firestore.runTransaction(f => f.get(count2)); + expect(res2.getCount()).to.equal(0); + }); }); describe('Transaction class', () => { From c519991d8caaea7c9fe10d7630213236f3bc436f Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Tue, 20 Sep 2022 13:23:22 -0400 Subject: [PATCH 25/28] Changes from PR review --- dev/src/reference.ts | 24 ++++++------------------ dev/system-test/firestore.ts | 14 +++++++------- types/firestore.d.ts | 10 ---------- 3 files changed, 13 insertions(+), 35 deletions(-) diff --git a/dev/src/reference.ts b/dev/src/reference.ts index 6430b62a8..b209e74b8 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -1606,7 +1606,7 @@ export class Query implements firestore.Query { * result set. */ count(): AggregateQuery<{count: firestore.AggregateField}> { - return this.aggregate({count: AggregateField.count()}); + return this._aggregate({count: AggregateField.count()}); } /** @@ -1615,7 +1615,7 @@ export class Query implements firestore.Query { * @param aggregates the aggregations to perform. * @return an `AggregateQuery` that performs the given aggregations. */ - aggregate( + _aggregate( aggregates: T ): AggregateQuery { return new AggregateQuery(this, aggregates); @@ -2881,10 +2881,6 @@ export class AggregateQuery return this._query; } - get aggregates(): T { - return this._aggregates; - } - get(): Promise> { return this._get(); } @@ -2938,7 +2934,7 @@ export class AggregateQuery new AggregateQuerySnapshot(this, readTime, data) ); } else { - callback(Error('unexpected')); + callback(Error('RunAggregationQueryResponse is missing result')); } }, }); @@ -2968,11 +2964,11 @@ export class AggregateQuery backendStream.unpipe(stream); // If a non-transactional query failed, attempt to restart. // Transactional queries are retried via the transaction runner. - if (!transactionId && !isPermanentRpcError(err, 'runQuery')) { + if (!transactionId && !isPermanentRpcError(err, 'runAggregationQuery')) { logger( - 'Query._stream', + 'AggregateQuery._stream', tag, - 'Query failed with retryable stream error:', + 'AggregateQuery failed with retryable stream error:', err ); streamActive.resolve(/* active= */ true); @@ -3093,14 +3089,6 @@ export class AggregateQuerySnapshot return this._readTime; } - getCount(): number { - const count = this._data.count; - if (typeof count === 'number') { - return count; - } - throw new Error('Unexpected count is ' + typeof count); - } - isEqual(other: firestore.AggregateQuerySnapshot): boolean { if (this === other) { return true; diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index 3f7eb0527..c21ffbbf5 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -2155,21 +2155,21 @@ describe.only('Aggregates', () => { it('counts 0 document from non-existent collection', async () => { const count = randomCol.count(); const res = await firestore.runTransaction(f => f.get(count)); - expect(res.getCount()).to.equal(0); + expect(res.data().count).to.equal(0); }); it('counts 0 document from filtered empty collection', async () => { await randomCol.doc('doc').set({foo: 'bar'}); const count = randomCol.where('foo', '==', 'notbar').count(); const res = await firestore.runTransaction(f => f.get(count)); - expect(res.getCount()).to.equal(0); + expect(res.data().count).to.equal(0); }); it('counts 1 document', async () => { await randomCol.doc('doc').set({foo: 'bar'}); const count = randomCol.count(); const res = await firestore.runTransaction(f => f.get(count)); - expect(res.getCount()).to.equal(1); + expect(res.data().count).to.equal(1); }); it('counts multiple documents with filter', async () => { @@ -2179,7 +2179,7 @@ describe.only('Aggregates', () => { await randomCol.doc('doc3').set({notfoo: 'bar'}); const count = randomCol.where('foo', '==', 'bar').count(); const res = await firestore.runTransaction(f => f.get(count)); - expect(res.getCount()).to.equal(2); + expect(res.data().count).to.equal(2); }); it('counts up to limit', async () => { @@ -2193,7 +2193,7 @@ describe.only('Aggregates', () => { await randomCol.doc('doc8').set({foo: 'bar'}); const count = randomCol.limit(5).count(); const res = await firestore.runTransaction(f => f.get(count)); - expect(res.getCount()).to.equal(5); + expect(res.data().count).to.equal(5); }); it('counts with orderBy', async () => { @@ -2208,11 +2208,11 @@ describe.only('Aggregates', () => { const count1 = randomCol.orderBy('foo2').count(); const res1 = await firestore.runTransaction(f => f.get(count1)); - expect(res1.getCount()).to.equal(3); + expect(res1.data().count).to.equal(3); const count2 = randomCol.orderBy('foo3').count(); const res2 = await firestore.runTransaction(f => f.get(count2)); - expect(res2.getCount()).to.equal(0); + expect(res2.data().count).to.equal(0); }); }); diff --git a/types/firestore.d.ts b/types/firestore.d.ts index 48e62d2b8..d827c05e2 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -1713,14 +1713,6 @@ declare namespace FirebaseFirestore { */ count(): AggregateQuery<{count: AggregateField}>; - /** - * Returns `AggregateQuery` for given aggregates based on this `Query`. - * - * @param aggregates Specify aliases with aggregate functions. - * @return An AggregateQuery that contains given aggregates. - */ - aggregate(aggregates: T): AggregateQuery; - /** * Returns true if this `Query` is equal to the provided one. * @@ -2092,8 +2084,6 @@ declare namespace FirebaseFirestore { readonly query: Query; - readonly aggregates: T; - get(): Promise>; /** From d12c82a83acc171ea2148e7d9755d18e9239a036 Mon Sep 17 00:00:00 2001 From: Tom Andersen Date: Thu, 22 Sep 2022 11:36:50 -0400 Subject: [PATCH 26/28] Fix PR based on comments --- dev/src/reference.ts | 9 +- dev/src/transaction.ts | 7 +- dev/system-test/firestore.ts | 184 ++++++++++++++++++++++------------- dev/test/aggregateQuery.ts | 4 +- 4 files changed, 131 insertions(+), 73 deletions(-) diff --git a/dev/src/reference.ts b/dev/src/reference.ts index b209e74b8..1bd33822d 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -1615,7 +1615,7 @@ export class Query implements firestore.Query { * @param aggregates the aggregations to perform. * @return an `AggregateQuery` that performs the given aggregations. */ - _aggregate( + private _aggregate( aggregates: T ): AggregateQuery { return new AggregateQuery(this, aggregates); @@ -2964,7 +2964,10 @@ export class AggregateQuery backendStream.unpipe(stream); // If a non-transactional query failed, attempt to restart. // Transactional queries are retried via the transaction runner. - if (!transactionId && !isPermanentRpcError(err, 'runAggregationQuery')) { + if ( + !transactionId && + !isPermanentRpcError(err, 'runAggregationQuery') + ) { logger( 'AggregateQuery._stream', tag, @@ -3019,7 +3022,7 @@ export class AggregateQuery /** * Internal method for serializing a query to its RunAggregationQuery proto - * representation with an optional transaction id or read time. + * representation with an optional transaction id. * * @private * @internal diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index c61f4891e..3f01d433a 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -107,7 +107,7 @@ export class Transaction implements firestore.Transaction { * @return An AggregateQuerySnapshot for the retrieved data. */ get( - aggregateQuery: AggregateQuery + aggregateQuery: firestore.AggregateQuery ): Promise>; /** @@ -134,7 +134,10 @@ export class Transaction implements firestore.Transaction { * ``` */ get( - refOrQuery: DocumentReference | Query | AggregateQuery + refOrQuery: + | firestore.DocumentReference + | firestore.Query + | firestore.AggregateQuery ): Promise< DocumentSnapshot | QuerySnapshot | AggregateQuerySnapshot > { diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index c21ffbbf5..348045a36 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -13,15 +13,15 @@ // limitations under the License. import { - QuerySnapshot, DocumentData, - WithFieldValue, PartialWithFieldValue, + QuerySnapshot, SetOptions, Settings, + WithFieldValue, } from '@google-cloud/firestore'; -import {describe, it, before, beforeEach, afterEach} from 'mocha'; +import {afterEach, before, beforeEach, describe, it} from 'mocha'; import {expect, use} from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; import * as extend from 'extend'; @@ -54,7 +54,6 @@ import {BulkWriter} from '../src/bulk-writer'; import {Status} from 'google-gax'; import {QueryPartition} from '../src/query-partition'; import {CollectionGroup} from '../src/collection-group'; - import IBundleElement = firestore.IBundleElement; use(chaiAsPromised); @@ -2140,7 +2139,7 @@ describe('Query class', () => { }); }); -describe.only('Aggregates', () => { +describe('Aggregates', () => { let firestore: Firestore; let randomCol: CollectionReference; setLogFunction(console.log); @@ -2152,68 +2151,121 @@ describe.only('Aggregates', () => { afterEach(() => verifyInstance(firestore)); - it('counts 0 document from non-existent collection', async () => { - const count = randomCol.count(); - const res = await firestore.runTransaction(f => f.get(count)); - expect(res.data().count).to.equal(0); - }); - - it('counts 0 document from filtered empty collection', async () => { - await randomCol.doc('doc').set({foo: 'bar'}); - const count = randomCol.where('foo', '==', 'notbar').count(); - const res = await firestore.runTransaction(f => f.get(count)); - expect(res.data().count).to.equal(0); - }); - - it('counts 1 document', async () => { - await randomCol.doc('doc').set({foo: 'bar'}); - const count = randomCol.count(); - const res = await firestore.runTransaction(f => f.get(count)); - expect(res.data().count).to.equal(1); - }); - - it('counts multiple documents with filter', async () => { - await randomCol.doc('doc1').set({foo: 'bar'}); - await randomCol.doc('doc2').set({foo: 'bar'}); - await randomCol.doc('doc3').set({foo: 'notbar'}); - await randomCol.doc('doc3').set({notfoo: 'bar'}); - const count = randomCol.where('foo', '==', 'bar').count(); - const res = await firestore.runTransaction(f => f.get(count)); - expect(res.data().count).to.equal(2); - }); - - it('counts up to limit', async () => { - await randomCol.doc('doc1').set({foo: 'bar'}); - await randomCol.doc('doc2').set({foo: 'bar'}); - await randomCol.doc('doc3').set({foo: 'bar'}); - await randomCol.doc('doc4').set({foo: 'bar'}); - await randomCol.doc('doc5').set({foo: 'bar'}); - await randomCol.doc('doc6').set({foo: 'bar'}); - await randomCol.doc('doc7').set({foo: 'bar'}); - await randomCol.doc('doc8').set({foo: 'bar'}); - const count = randomCol.limit(5).count(); - const res = await firestore.runTransaction(f => f.get(count)); - expect(res.data().count).to.equal(5); - }); - - it('counts with orderBy', async () => { - await randomCol.doc('doc1').set({foo1: 'bar1'}); - await randomCol.doc('doc2').set({foo1: 'bar2'}); - await randomCol.doc('doc3').set({foo1: 'bar3'}); - await randomCol.doc('doc4').set({foo1: 'bar4'}); - await randomCol.doc('doc5').set({foo1: 'bar5'}); - await randomCol.doc('doc6').set({foo2: 'bar6'}); - await randomCol.doc('doc7').set({foo2: 'bar7'}); - await randomCol.doc('doc8').set({foo2: 'bar8'}); - - const count1 = randomCol.orderBy('foo2').count(); - const res1 = await firestore.runTransaction(f => f.get(count1)); - expect(res1.data().count).to.equal(3); - - const count2 = randomCol.orderBy('foo3').count(); - const res2 = await firestore.runTransaction(f => f.get(count2)); - expect(res2.data().count).to.equal(0); + describe('Run outside Transaction', () => { + countTests(async (q, n) => { + const res = await q.get(); + expect(res.data().count).to.equal(n); + }); + }); + + describe('Run within Transaction', () => { + countTests(async (q, n) => { + const res = await firestore.runTransaction(f => f.get(q)); + expect(res.data().count).to.equal(n); + }); }); + + function countTests( + runQueryAndExpectCount: ( + query: FirebaseFirestore.AggregateQuery<{ + count: FirebaseFirestore.AggregateField; + }>, + expectedCount: number + ) => Promise + ) { + it('counts 0 document from non-existent collection', async () => { + const count = randomCol.count(); + await runQueryAndExpectCount(count, 0); + }); + + it('counts 0 document from filtered empty collection', async () => { + await randomCol.doc('doc').set({foo: 'bar'}); + const count = randomCol.where('foo', '==', 'notbar').count(); + await runQueryAndExpectCount(count, 0); + }); + + it('counts 1 document', async () => { + await randomCol.doc('doc').set({foo: 'bar'}); + const count = randomCol.count(); + await runQueryAndExpectCount(count, 1); + }); + + it('counts 1 document', async () => { + await randomCol.doc('doc').set({foo: 'bar'}); + const count = randomCol.count(); + await runQueryAndExpectCount(count, 1); + }); + + it('counts 1 document', async () => { + await randomCol.doc('doc').set({foo: 'bar'}); + const count = randomCol.count(); + await runQueryAndExpectCount(count, 1); + }); + + it('counts multiple documents with filter', async () => { + await randomCol.doc('doc1').set({foo: 'bar'}); + await randomCol.doc('doc2').set({foo: 'bar'}); + await randomCol.doc('doc3').set({foo: 'notbar'}); + await randomCol.doc('doc3').set({notfoo: 'bar'}); + const count = randomCol.where('foo', '==', 'bar').count(); + await runQueryAndExpectCount(count, 2); + }); + + it('counts up to limit', async () => { + await randomCol.doc('doc1').set({foo: 'bar'}); + await randomCol.doc('doc2').set({foo: 'bar'}); + await randomCol.doc('doc3').set({foo: 'bar'}); + await randomCol.doc('doc4').set({foo: 'bar'}); + await randomCol.doc('doc5').set({foo: 'bar'}); + await randomCol.doc('doc6').set({foo: 'bar'}); + await randomCol.doc('doc7').set({foo: 'bar'}); + await randomCol.doc('doc8').set({foo: 'bar'}); + const count = randomCol.limit(5).count(); + await runQueryAndExpectCount(count, 5); + }); + + it('counts with orderBy', async () => { + await randomCol.doc('doc1').set({foo1: 'bar1'}); + await randomCol.doc('doc2').set({foo1: 'bar2'}); + await randomCol.doc('doc3').set({foo1: 'bar3'}); + await randomCol.doc('doc4').set({foo1: 'bar4'}); + await randomCol.doc('doc5').set({foo1: 'bar5'}); + await randomCol.doc('doc6').set({foo2: 'bar6'}); + await randomCol.doc('doc7').set({foo2: 'bar7'}); + await randomCol.doc('doc8').set({foo2: 'bar8'}); + + const count1 = randomCol.orderBy('foo2').count(); + await runQueryAndExpectCount(count1, 3); + + const count2 = randomCol.orderBy('foo3').count(); + await runQueryAndExpectCount(count2, 0); + }); + + it('counts with startAt, endAt and offset', async () => { + await randomCol.doc('doc1').set({foo: 'bar'}); + await randomCol.doc('doc2').set({foo: 'bar'}); + await randomCol.doc('doc3').set({foo: 'bar'}); + await randomCol.doc('doc4').set({foo: 'bar'}); + await randomCol.doc('doc5').set({foo: 'bar'}); + await randomCol.doc('doc6').set({foo: 'bar'}); + await randomCol.doc('doc7').set({foo: 'bar'}); + + const count1 = randomCol.startAfter(randomCol.doc('doc3')).count(); + await runQueryAndExpectCount(count1, 4); + + const count2 = randomCol.startAt(randomCol.doc('doc3')).count(); + await runQueryAndExpectCount(count2, 5); + + const count3 = randomCol.endAt(randomCol.doc('doc3')).count(); + await runQueryAndExpectCount(count3, 3); + + const count4 = randomCol.endBefore(randomCol.doc('doc3')).count(); + await runQueryAndExpectCount(count4, 2); + + const count5 = randomCol.offset(6).count(); + await runQueryAndExpectCount(count5, 1); + }); + } }); describe('Transaction class', () => { diff --git a/dev/test/aggregateQuery.ts b/dev/test/aggregateQuery.ts index 43c08779a..db0eb7c82 100644 --- a/dev/test/aggregateQuery.ts +++ b/dev/test/aggregateQuery.ts @@ -93,7 +93,7 @@ describe('aggregate query interface', () => { const query = firestore.collection('collectionId').count(); return query.get().then(results => { - expect(results.getCount()).to.be.equal(99); + expect(results.data().count).to.be.equal(99); expect(results.readTime.isEqual(new Timestamp(5, 6))).to.be.true; expect(results.query).to.be.equal(query); }); @@ -116,7 +116,7 @@ describe('aggregate query interface', () => { const query = firestore.collection('collectionId').count(); return query.get().then(results => { - expect(results.getCount()).to.be.equal(99); + expect(results.data().count).to.be.equal(99); expect(results.readTime.isEqual(new Timestamp(5, 6))).to.be.true; expect(results.query).to.be.equal(query); }); From e3136b892ca4805e5b1593e2de50834f0f58096e Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Thu, 29 Sep 2022 23:46:32 -0400 Subject: [PATCH 27/28] system-test/firestore.ts: remove unconditional call to setLogFunction(console.log) --- dev/system-test/firestore.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index 348045a36..deb9454f8 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -2142,7 +2142,6 @@ describe('Query class', () => { describe('Aggregates', () => { let firestore: Firestore; let randomCol: CollectionReference; - setLogFunction(console.log); beforeEach(() => { randomCol = getTestRoot(); From 996d86e9fc2246bacc2baec93f9424ef0bd62c6d Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 3 Oct 2022 11:20:07 -0400 Subject: [PATCH 28/28] docs: Write javadocs for COUNT API (#1783) --- dev/src/reference.ts | 187 +++++++++++++++++++++++++------------------ types/firestore.d.ts | 100 +++++++++++++---------- 2 files changed, 167 insertions(+), 120 deletions(-) diff --git a/dev/src/reference.ts b/dev/src/reference.ts index 1bd33822d..b5cac033a 100644 --- a/dev/src/reference.ts +++ b/dev/src/reference.ts @@ -1599,26 +1599,24 @@ export class Query implements firestore.Query { } /** - * Returns an `AggregateQuery` that counts the number of documents in the - * result set. + * Returns a query that counts the documents in the result set of this + * query. * - * @return an `AggregateQuery` that counts the number of documents in the - * result set. - */ - count(): AggregateQuery<{count: firestore.AggregateField}> { - return this._aggregate({count: AggregateField.count()}); - } - - /** - * Returns an `AggregateQuery` that performs the given aggregations. + * The returned query, when executed, counts the documents in the result set + * of this query without actually downloading the documents. * - * @param aggregates the aggregations to perform. - * @return an `AggregateQuery` that performs the given aggregations. + * Using the returned query to count the documents is efficient because only + * the final count, not the documents' data, is downloaded. The returned + * query can even count the documents if the result set would be + * prohibitively large to download entirely (e.g. thousands of documents). + * + * @return a query that counts the documents in the result set of this + * query. The count can be retrieved from `snapshot.data().count`, where + * `snapshot` is the `AggregateQuerySnapshot` resulting from running the + * returned query. */ - private _aggregate( - aggregates: T - ): AggregateQuery { - return new AggregateQuery(this, aggregates); + count(): AggregateQuery<{count: firestore.AggregateField}> { + return new AggregateQuery(this, {count: {}}); } /** @@ -2854,33 +2852,36 @@ export class CollectionReference } } -export class AggregateField implements firestore.AggregateField { - private constructor() {} - - static count(): AggregateField { - return new AggregateField(); - } - - isEqual(other: firestore.AggregateField): boolean { - return this === other || other instanceof AggregateField; - } -} - +/** + * A query that calculates aggregations over an underlying query. + */ export class AggregateQuery implements firestore.AggregateQuery { - private readonly _query: Query; - private readonly _aggregates: T; - - constructor(query: Query, aggregates: T) { - this._query = query; - this._aggregates = aggregates; - } + /** + * @private + * @internal + * + * @param _query The query whose aggregations will be calculated by this + * object. + * @param _aggregates The aggregations that will be performed by this query. + */ + constructor( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private readonly _query: Query, + private readonly _aggregates: T + ) {} + /** The query whose aggregations will be calculated by this object. */ get query(): firestore.Query { return this._query; } + /** + * Executes this query. + * + * @return A promise that will be resolved with the results of the query. + */ get(): Promise> { return this._get(); } @@ -2914,9 +2915,9 @@ export class AggregateQuery /** * Internal streaming method that accepts an optional transaction ID. * - * @param transactionId A transaction ID. * @private * @internal + * @param transactionId A transaction ID. * @returns A stream of document results. */ _stream(transactionId?: Uint8Array): Readable { @@ -2997,13 +2998,12 @@ export class AggregateQuery /** * Internal method to decode values within result. - * - * @param proto * @private */ private decodeResult( proto: api.IAggregationResult ): firestore.AggregateSpecData { + // eslint-disable-next-line @typescript-eslint/no-explicit-any const data: any = {}; const fields = proto.aggregateFields; if (fields) { @@ -3030,7 +3030,8 @@ export class AggregateQuery */ toProto(transactionId?: Uint8Array): api.IRunAggregationQueryRequest { const queryProto = this._query.toProto(); - //TODO(tomandersen) inspect _query to build request - this is just hard coded count right now. + //TODO(tomandersen) inspect _query to build request - this is just hard + // coded count right now. const runQueryRequest: api.IRunAggregationQueryRequest = { parent: queryProto.parent, structuredAggregationQuery: { @@ -3051,76 +3052,104 @@ export class AggregateQuery return runQueryRequest; } + /** + * Compares this object with the given object for equality. + * + * This object is considered "equal" to the other object if and only if + * `other` performs the same aggregations as this `AggregateQuery` and + * the underlying Query of `other` compares equal to that of this object + * using `Query.isEqual()`. + * + * @param other The object to compare to this object for equality. + * @return `true` if this object is "equal" to the given object, as + * defined above, or `false` otherwise. + */ isEqual(other: firestore.AggregateQuery): boolean { if (this === other) { return true; } - if (other instanceof AggregateQuery) { - if (!this._query.isEqual(other._query)) { - return false; - } - - const thisAggregates: [string, AggregateField][] = - Object.entries(this._aggregates); - const otherAggregates = other._aggregates; - - return ( - thisAggregates.length === Object.keys(otherAggregates).length && - thisAggregates.every(([alias, field]) => - field.isEqual(otherAggregates[alias]) - ) - ); + if (!(other instanceof AggregateQuery)) { + return false; } - return false; + if (!this.query.isEqual(other.query)) { + return false; + } + return deepEqual(this._aggregates, other._aggregates); } } +/** + * The results of executing an aggregation query. + */ export class AggregateQuerySnapshot implements firestore.AggregateQuerySnapshot { + /** + * @private + * @internal + * + * @param _query The query that was executed to produce this result. + * @param _readTime The time this snapshot was read. + * @param _data The results of the aggregations performed over the underlying + * query. + */ constructor( private readonly _query: AggregateQuery, private readonly _readTime: Timestamp, private readonly _data: firestore.AggregateSpecData ) {} + /** The query that was executed to produce this result. */ get query(): firestore.AggregateQuery { return this._query; } + /** The time this snapshot was read. */ get readTime(): firestore.Timestamp { return this._readTime; } + /** + * 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 results of the aggregations performed over the underlying + * query. + */ + data(): firestore.AggregateSpecData { + return this._data; + } + + /** + * Compares this object with the given object for equality. + * + * Two `AggregateQuerySnapshot` instances are considered "equal" if they + * have the same data and their underlying queries compare "equal" using + * `AggregateQuery.isEqual()`. + * + * @param other The object to compare to this object for equality. + * @return `true` if this object is "equal" to the given object, as + * defined above, or `false` otherwise. + */ isEqual(other: firestore.AggregateQuerySnapshot): boolean { if (this === other) { return true; } - if (other instanceof AggregateQuerySnapshot) { - if (!this._query.isEqual(other._query)) { - return false; - } - - const thisData = this._data; - const thisDataKeys: string[] = Object.keys(thisData); - - const otherData = other._data; - const otherDataKeys: string[] = Object.keys(otherData); - - return ( - thisDataKeys.length === otherDataKeys.length && - thisDataKeys.every( - alias => - Object.prototype.hasOwnProperty.call(otherData, alias) && - thisData[alias] === otherData[alias] - ) - ); + if (!(other instanceof AggregateQuerySnapshot)) { + return false; + } + // Since the read time is different on every read, we explicitly ignore all + // document metadata in this comparison, just like + // `DocumentSnapshot.isEqual()` does. + if (!this.query.isEqual(other.query)) { + return false; } - return false; - } - data(): firestore.AggregateSpecData { - return this._data; + return deepEqual(this._data, other._data); } } diff --git a/types/firestore.d.ts b/types/firestore.d.ts index d84dcc054..2c5ceaf7c 100644 --- a/types/firestore.d.ts +++ b/types/firestore.d.ts @@ -21,7 +21,7 @@ // Declare a global (ambient) namespace // (used when not using import statement, but just script include). declare namespace FirebaseFirestore { - // Alias for `any` but used where a Firestore field value would be provided. + /** Alias for `any` but used where a Firestore field value would be provided. */ export type DocumentFieldValue = any; /** @@ -1707,9 +1707,21 @@ declare namespace FirebaseFirestore { ): () => void; /** - * Returns count `AggregateQuery` based on this `Query`. + * Returns a query that counts the documents in the result set of this + * query. + * + * The returned query, when executed, counts the documents in the result set + * of this query without actually downloading the documents. + * + * Using the returned query to count the documents is efficient because only + * the final count, not the documents' data, is downloaded. The returned + * query can even count the documents if the result set would be + * prohibitively large to download entirely (e.g. thousands of documents). * - * @return AggregateQuery that contains count aggregate. + * @return a query that counts the documents in the result set of this + * query. The count can be retrieved from `snapshot.data().count`, where + * `snapshot` is the `AggregateQuerySnapshot` resulting from running the + * returned query. */ count(): AggregateQuery<{count: AggregateField}>; @@ -2034,94 +2046,100 @@ declare namespace FirebaseFirestore { } /** - * 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 { private constructor(); - - /** - * Creates and returns an aggregation field that counts the documents in the result set. - * @returns An `AggregateField` object with number input type. - */ - static count(): AggregateField; - - /** - * Returns true if the aggregate function in this `AggregateField` is equal to the - * provided one. - * - * @param other The `AggregateField` to compare against. - * @return true if this `AggregateField` is equal to the provided one. - */ - isEqual(other: AggregateField): boolean; } /** - * The union of all `AggregateField` types that are returned from the factory - * functions. + * The union of all `AggregateField` types that are supported by Firestore. */ - export type AggregateFieldType = ReturnType; + export type AggregateFieldType = AggregateField; /** - * 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 + * 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; }; + /** + * A query that calculates aggregations over an underlying query. + */ export class AggregateQuery { private constructor(); + /** The query whose aggregations will be calculated by this object. */ readonly query: Query; + /** + * Executes this query. + * + * @return A promise that will be resolved with the results of the query. + */ get(): Promise>; /** - * Returns true if the query and aggregates in this `AggregateQuery` is equal to the - * provided one. + * Compares this object with the given object for equality. + * + * This object is considered "equal" to the other object if and only if + * `other` performs the same aggregations as this `AggregateQuery` and + * the underlying Query of `other` compares equal to that of this object + * using `Query.isEqual()`. * - * @param other The `AggregateQuery` to compare against. - * @return true if this `AggregateQuery` is equal to the provided one. + * @param other The object to compare to this object for equality. + * @return `true` if this object is "equal" to the given object, as + * defined above, or `false` otherwise. */ isEqual(other: AggregateQuery): boolean; } /** - * An `AggregateQuerySnapshot` contains the results of running an aggregate query. + * The results of executing an aggregation query. */ export class AggregateQuerySnapshot { private constructor(); + /** The query that was executed to produce this result. */ readonly query: AggregateQuery; + /** The time this snapshot was read. */ readonly readTime: Timestamp; /** - * 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; /** - * Returns true if the query, read time, and data in this `AggregateQuerySnapshot` - * is equal to the provided one. + * Compares this object with the given object for equality. + * + * Two `AggregateQuerySnapshot` instances are considered "equal" if they + * have the same data and their underlying queries compare "equal" using + * `AggregateQuery.isEqual()`. * - * @param other The `AggregateQuerySnapshot` to compare against. - * @return true if this `AggregateQuerySnapshot` is equal to the provided one. + * @param other The object to compare to this object for equality. + * @return `true` if this object is "equal" to the given object, as + * defined above, or `false` otherwise. */ isEqual(other: AggregateQuerySnapshot): boolean; }