From 47b14168b4ffc3f14b959747fd1140d6c22f910a Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Mon, 4 Nov 2024 16:01:54 +0000 Subject: [PATCH 01/28] feat(firestore): support for aggregate queries including `sum()` & `average()` --- ...tiveFirebaseFirestoreCollectionModule.java | 78 +++++++++++++++++++ packages/firestore/lib/FirestoreAggregate.js | 35 +++++++++ packages/firestore/lib/modular/index.d.ts | 71 +++++++++++++++++ packages/firestore/lib/modular/index.js | 65 ++++++++++++++++ 4 files changed, 249 insertions(+) diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java index 982e38680c..00ea393237 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java @@ -26,6 +26,9 @@ import com.facebook.react.bridge.*; import com.google.android.gms.tasks.Tasks; import com.google.firebase.firestore.*; + +import java.util.ArrayList; + import io.invertase.firebase.common.ReactNativeFirebaseEventEmitter; import io.invertase.firebase.common.ReactNativeFirebaseModule; @@ -193,6 +196,79 @@ public void collectionCount( }); } + @ReactMethod + public void aggregateQuery( + String appName, + String databaseId, + String path, + String type, + ReadableArray filters, + ReadableArray orders, + ReadableMap options, + ReadableArray aggregateQueries + ){ + FirebaseFirestore firebaseFirestore = getFirestoreForApp(appName, databaseId); + ReactNativeFirebaseFirestoreQuery firestoreQuery = + new ReactNativeFirebaseFirestoreQuery( + appName, + databaseId, + getQueryForFirestore(firebaseFirestore, path, type), + filters, + orders, + options); + ArrayList aggregateFields = new ArrayList<>(); + + for (int i = 0; i < aggregateQueries.size(); i++) { + ReadableMap aggregateQuery = aggregateQueries.getMap(i); + String aggregateType = aggregateQuery.getString("aggregateType"); + String fieldPath = aggregateQuery.getString("fieldPath"); + if (aggregateType && fieldPath) { + switch (aggregateType) { + case "count": + aggregateFields.add(AggregateField.count(fieldPath)); + break; + case "sum": + aggregateFields.add(AggregateField.sum(fieldPath)); + break; + case "average": + aggregateFields.add(AggregateField.avg(fieldPath)); + break; + default: + break; + } + } + + AggregateQuery aggregateQuery = firestoreQuery.query.aggregate(aggregateFields); + aggregateQuery + .get(AggregateSource.SERVER) + .addOnCompleteListener( + task -> { + if (task.isSuccessful()) { + WritableMap result = Arguments.createMap(); + AggregateQuerySnapshot snapshot = task.getResult(); + aggregateFields.forEach(aggregateField -> { + switch (aggregateField.getOperator()) { + case "count": + result.putDouble(aggregateField.getFieldPath(), Long.valueOf(snapshot.getCount()).doubleValue()); + break; + case "sum": + result.putDouble(aggregateField.getFieldPath(), snapshot.getSum(aggregateField.getFieldPath())); + break; + case "average": + result.putDouble(aggregateField.getFieldPath(), snapshot.getAverage(aggregateField.getFieldPath())); + break; + default: + break; + } + } + promise.resolve(result); + } else { + rejectPromiseFirestoreException(promise, task.getException()); + } + }); + } + } + @ReactMethod public void collectionGet( String appName, @@ -214,6 +290,8 @@ public void collectionGet( orders, options); handleQueryGet(firestoreQuery, getSource(getOptions), promise); + + } private void handleQueryOnSnapshot( diff --git a/packages/firestore/lib/FirestoreAggregate.js b/packages/firestore/lib/FirestoreAggregate.js index 2b8b0ae8a0..cd1e14c163 100644 --- a/packages/firestore/lib/FirestoreAggregate.js +++ b/packages/firestore/lib/FirestoreAggregate.js @@ -15,6 +15,8 @@ * */ +import FirestoreFieldPath, { fromDotSeparatedString } from './FirestoreFieldPath'; + export class FirestoreAggregateQuery { constructor(firestore, query, collectionPath, modifiers) { this._firestore = firestore; @@ -50,3 +52,36 @@ export class FirestoreAggregateQuerySnapshot { return { count: this._data.count }; } } + +export const AggregateType = { + SUM: 'sum', + AVG: 'average', + COUNT: 'count', +}; + +export class AggregateField { + /** Indicates the aggregation operation of this AggregateField. */ + aggregateType; + fieldPath; + + /** + * Create a new AggregateField + * @param aggregateType Specifies the type of aggregation operation to perform. + * @param _internalFieldPath Optionally specifies the field that is aggregated. + * @internal + */ + constructor(aggregateType, fieldPath) { + this.aggregateType = aggregateType; + this.fieldPath = fieldPath; + } +} + +export function fieldPathFromArgument(path) { + if (path instanceof FirestoreFieldPath) { + return path; + } else if (typeof path === 'string') { + return fromDotSeparatedString(methodName, path); + } else { + throw new Error('Field path arguments must be of type `string` or `FieldPath`'); + } +} diff --git a/packages/firestore/lib/modular/index.d.ts b/packages/firestore/lib/modular/index.d.ts index 179bf65a80..f7cbc91de6 100644 --- a/packages/firestore/lib/modular/index.d.ts +++ b/packages/firestore/lib/modular/index.d.ts @@ -495,6 +495,77 @@ export function getCountFromServer >; +/** + * Specifies a set of aggregations and their aliases. + */ +interface AggregateSpec { + [field: string]: AggregateFieldType; +} + +/** + * The union of all `AggregateField` types that are supported by Firestore. + */ +export type AggregateFieldType = + | ReturnType + | ReturnType + | ReturnType; + +export function getAggregateFromServer< + AggregateSpecType extends AggregateSpec, + AppModelType, + DbModelType extends FirebaseFirestoreTypes.DocumentData, +>( + query: Query, + aggregateSpec: AggregateSpecType, +): Promise< + FirebaseFirestoreTypes.AggregateQuerySnapshot +>; + +/** + * Create an AggregateField object that can be used to compute the sum of + * a specified field over a range of documents in the result set of a query. + * @param field Specifies the field to sum across the result set. + */ +export function sum(field: string | FieldPath): AggregateField; + +/** + * Create an AggregateField object that can be used to compute the average of + * a specified field over a range of documents in the result set of a query. + * @param field Specifies the field to average across the result set. + */ +export function average(field: string | FieldPath): AggregateField; + +/** + * Create an AggregateField object that can be used to compute the count of + * documents in the result set of a query. + */ +export function count(): AggregateField; + +/** + * Represents an aggregation that can be performed by Firestore. + */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +export class AggregateField { + /** A type string to uniquely identify instances of this class. */ + readonly type = 'AggregateField'; + + /** Indicates the aggregation operation of this AggregateField. */ + readonly aggregateType: AggregateType; + + /** + * Create a new AggregateField + * @param aggregateType Specifies the type of aggregation operation to perform. + * @param _internalFieldPath Optionally specifies the field that is aggregated. + * @internal + */ + constructor( + aggregateType: AggregateType = 'count', + readonly _internalFieldPath?: InternalFieldPath, + ) { + this.aggregateType = aggregateType; + } +} + /** * Represents the task of loading a Firestore bundle. * It provides progress of bundle loading, as well as task completion and error events. diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index 46eb2d8c4c..3244f09841 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -13,6 +13,8 @@ */ import { firebase } from '../index'; +import { AggregateField, AggregateType } from '../FirestoreAggregate'; +import FirestorePath from '../FirestorePath'; /** * @param {FirebaseApp?} app @@ -192,6 +194,69 @@ export function getCountFromServer(query) { return query.count().get(); } +export async function getAggregateFromServer(query, aggregateSpec) { + const aggregateQueries = []; + for (const key in aggregateSpec) { + if (aggregateSpec.hasOwnProperty(key)) { + const value = aggregateSpec[key]; + + if (value instanceof AggregateField) { + switch (value.aggregateType) { + case AggregateType.AVG: + case AggregateType.SUM: + case AggregateType.COUNT: + const aggregateQuery = { + aggregateType: value.aggregateType, + // TODO - how is this sent over the wire? Think it is serialized automatically + field: value.fieldPath, + }; + aggregateQueries.push(aggregateQuery); + break; + default: + throw new Error( + `"AggregateField" has an an unknown "AggregateType" : ${value.aggregateType}`, + ); + } + } + } + } + + return query._firestore.native.aggregateQuery( + query._collectionPath.relativeName, + query._modifiers.type, + query._modifiers.filters, + query._modifiers.orders, + query._modifiers.options, + aggregateQueries, + ); +} + +/** + * Create an AggregateField object that can be used to compute the sum of + * a specified field over a range of documents in the result set of a query. + * @param field Specifies the field to sum across the result set. + */ +export function sum(field) { + return new AggregateField(AggregateType.SUM, FirestorePath.fromName(field)); +} + +/** + * Create an AggregateField object that can be used to compute the average of + * a specified field over a range of documents in the result set of a query. + * @param field Specifies the field to average across the result set. + */ +export function average(field) { + return new AggregateField(AggregateType.AVG, FirestorePath.fromName(field)); +} + +/** + * Create an AggregateField object that can be used to compute the count of + * documents in the result set of a query. + */ +export function count() { + return new AggregateField(AggregateType.COUNT); +} + /** * @param {Firestore} firestore * @param {ReadableStream | ArrayBuffer | string} bundleData From c92547ee91690cc344ade76ceb64bb4cf69e4fc7 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 5 Nov 2024 14:18:05 +0000 Subject: [PATCH 02/28] feat(firestore, android): working version of aggregate query --- ...tiveFirebaseFirestoreCollectionModule.java | 54 +++++++++++++------ packages/firestore/lib/FirestoreAggregate.js | 13 +++-- packages/firestore/lib/modular/index.js | 46 +++++++++------- 3 files changed, 73 insertions(+), 40 deletions(-) diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java index 00ea393237..0022ffda2a 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java @@ -17,6 +17,8 @@ * */ +import static com.google.firebase.firestore.AggregateField.average; +import static com.google.firebase.firestore.AggregateField.sum; import static io.invertase.firebase.firestore.ReactNativeFirebaseFirestoreCommon.rejectPromiseFirestoreException; import static io.invertase.firebase.firestore.ReactNativeFirebaseFirestoreSerialize.snapshotToWritableMap; import static io.invertase.firebase.firestore.UniversalFirebaseFirestoreCommon.getFirestoreForApp; @@ -205,7 +207,8 @@ public void aggregateQuery( ReadableArray filters, ReadableArray orders, ReadableMap options, - ReadableArray aggregateQueries + ReadableArray aggregateQueries, + Promise promise ){ FirebaseFirestore firebaseFirestore = getFirestoreForApp(appName, databaseId); ReactNativeFirebaseFirestoreQuery firestoreQuery = @@ -216,57 +219,76 @@ public void aggregateQuery( filters, orders, options); + ArrayList aggregateFields = new ArrayList<>(); for (int i = 0; i < aggregateQueries.size(); i++) { ReadableMap aggregateQuery = aggregateQueries.getMap(i); + String aggregateType = aggregateQuery.getString("aggregateType"); - String fieldPath = aggregateQuery.getString("fieldPath"); - if (aggregateType && fieldPath) { + String fieldPath = aggregateQuery.getString("field"); + + assert aggregateType != null; switch (aggregateType) { case "count": - aggregateFields.add(AggregateField.count(fieldPath)); + aggregateFields.add(AggregateField.count()); break; case "sum": + assert fieldPath != null; aggregateFields.add(AggregateField.sum(fieldPath)); break; case "average": - aggregateFields.add(AggregateField.avg(fieldPath)); + assert fieldPath != null; + aggregateFields.add(AggregateField.average(fieldPath)); break; default: - break; + throw new Error("Invalid AggregateType: " + aggregateType); } - } + } + AggregateQuery firestoreAggregateQuery = firestoreQuery.query.aggregate(aggregateFields.get(0), + aggregateFields.subList(1, aggregateFields.size()).toArray(new AggregateField[0])); - AggregateQuery aggregateQuery = firestoreQuery.query.aggregate(aggregateFields); - aggregateQuery + firestoreAggregateQuery .get(AggregateSource.SERVER) .addOnCompleteListener( task -> { if (task.isSuccessful()) { WritableMap result = Arguments.createMap(); AggregateQuerySnapshot snapshot = task.getResult(); - aggregateFields.forEach(aggregateField -> { - switch (aggregateField.getOperator()) { + + for (int k = 0; k < aggregateQueries.size(); k++) { + ReadableMap aggQuery = aggregateQueries.getMap(k); + String aggType = aggQuery.getString("aggregateType"); + String field = aggQuery.getString("field"); + String key = aggQuery.getString("key"); + assert key != null; + assert aggType != null; + switch (aggType) { case "count": - result.putDouble(aggregateField.getFieldPath(), Long.valueOf(snapshot.getCount()).doubleValue()); + result.putDouble(key, Long.valueOf(snapshot.getCount()).doubleValue()); break; case "sum": - result.putDouble(aggregateField.getFieldPath(), snapshot.getSum(aggregateField.getFieldPath())); + assert field != null; + Number sum = (Number) snapshot.get(sum(field)); + assert sum != null; + result.putDouble(key, sum.doubleValue()); break; case "average": - result.putDouble(aggregateField.getFieldPath(), snapshot.getAverage(aggregateField.getFieldPath())); + assert field != null; + Number average = snapshot.get(average(field)); + assert average != null; + result.putDouble(key, average.doubleValue()); break; default: - break; + throw new Error("Invalid AggregateType: " + aggType); } } + promise.resolve(result); } else { rejectPromiseFirestoreException(promise, task.getException()); } }); - } } @ReactMethod diff --git a/packages/firestore/lib/FirestoreAggregate.js b/packages/firestore/lib/FirestoreAggregate.js index cd1e14c163..a52c96c104 100644 --- a/packages/firestore/lib/FirestoreAggregate.js +++ b/packages/firestore/lib/FirestoreAggregate.js @@ -38,18 +38,23 @@ export class FirestoreAggregateQuery { this._modifiers.orders, this._modifiers.options, ) - .then(data => new FirestoreAggregateQuerySnapshot(this._query, data)); + .then(data => new FirestoreAggregateQuerySnapshot(this._query, data, true)); } } export class FirestoreAggregateQuerySnapshot { - constructor(query, data) { + constructor(query, data, isGetCountFromServer) { this._query = query; this._data = data; + this._isGetCountFromServer = isGetCountFromServer; } data() { - return { count: this._data.count }; + if (this._isGetCountFromServer) { + return { count: this._data.count }; + } else { + return { ...this._data }; + } } } @@ -80,7 +85,7 @@ export function fieldPathFromArgument(path) { if (path instanceof FirestoreFieldPath) { return path; } else if (typeof path === 'string') { - return fromDotSeparatedString(methodName, path); + return fromDotSeparatedString(path); } else { throw new Error('Field path arguments must be of type `string` or `FieldPath`'); } diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index 3244f09841..95d067f360 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -13,8 +13,12 @@ */ import { firebase } from '../index'; -import { AggregateField, AggregateType } from '../FirestoreAggregate'; -import FirestorePath from '../FirestorePath'; +import { + FirestoreAggregateQuerySnapshot, + AggregateField, + AggregateType, + fieldPathFromArgument, +} from '../FirestoreAggregate'; /** * @param {FirebaseApp?} app @@ -198,37 +202,39 @@ export async function getAggregateFromServer(query, aggregateSpec) { const aggregateQueries = []; for (const key in aggregateSpec) { if (aggregateSpec.hasOwnProperty(key)) { - const value = aggregateSpec[key]; + const aggregateField = aggregateSpec[key]; - if (value instanceof AggregateField) { - switch (value.aggregateType) { + if (aggregateField instanceof AggregateField) { + switch (aggregateField.aggregateType) { case AggregateType.AVG: case AggregateType.SUM: case AggregateType.COUNT: const aggregateQuery = { - aggregateType: value.aggregateType, - // TODO - how is this sent over the wire? Think it is serialized automatically - field: value.fieldPath, + aggregateType: aggregateField.aggregateType, + field: aggregateField.fieldPath === null ? null : aggregateField.fieldPath._toPath(), + key, }; aggregateQueries.push(aggregateQuery); break; default: throw new Error( - `"AggregateField" has an an unknown "AggregateType" : ${value.aggregateType}`, + `"AggregateField" has an an unknown "AggregateType" : ${aggregateField.aggregateType}`, ); } } } } - return query._firestore.native.aggregateQuery( - query._collectionPath.relativeName, - query._modifiers.type, - query._modifiers.filters, - query._modifiers.orders, - query._modifiers.options, - aggregateQueries, - ); + return query._firestore.native + .aggregateQuery( + query._collectionPath.relativeName, + query._modifiers.type, + query._modifiers.filters, + query._modifiers.orders, + query._modifiers.options, + aggregateQueries, + ) + .then(data => new FirestoreAggregateQuerySnapshot(query, data, false)); } /** @@ -237,7 +243,7 @@ export async function getAggregateFromServer(query, aggregateSpec) { * @param field Specifies the field to sum across the result set. */ export function sum(field) { - return new AggregateField(AggregateType.SUM, FirestorePath.fromName(field)); + return new AggregateField(AggregateType.SUM, fieldPathFromArgument(field)); } /** @@ -246,7 +252,7 @@ export function sum(field) { * @param field Specifies the field to average across the result set. */ export function average(field) { - return new AggregateField(AggregateType.AVG, FirestorePath.fromName(field)); + return new AggregateField(AggregateType.AVG, fieldPathFromArgument(field)); } /** @@ -254,7 +260,7 @@ export function average(field) { * documents in the result set of a query. */ export function count() { - return new AggregateField(AggregateType.COUNT); + return new AggregateField(AggregateType.COUNT, null); } /** From 7f01f41f9d4836b8b06e86f37905d38469999d04 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 5 Nov 2024 16:56:05 +0000 Subject: [PATCH 03/28] feat: iOS implementation of aggregate queries --- .../RNFBFirestoreCollectionModule.m | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m b/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m index 963f6fec11..4182acc5f8 100644 --- a/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m +++ b/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m @@ -216,6 +216,83 @@ - (void)invalidate { }]; } +RCT_EXPORT_METHOD(aggregateQuery + : (FIRApp *)firebaseApp + : (NSString *)databaseId + : (NSString *)path + : (NSString *)type + : (NSArray *)filters + : (NSArray *)orders + : (NSDictionary *)options + : (NSArray *)aggregateQueries + : (RCTPromiseResolveBlock)resolve + : (RCTPromiseRejectBlock)reject) { + FIRFirestore *firestore = [RNFBFirestoreCommon getFirestoreForApp:firebaseApp + databaseId:databaseId]; + FIRQuery *query = [RNFBFirestoreCommon getQueryForFirestore:firestore path:path type:type]; + + NSMutableArray *aggregateFields = + [[NSMutableArray alloc] init]; + + for (NSDictionary *aggregateQuery in aggregateQueries) { + NSString *aggregateType = aggregateQuery[@"aggregateType"]; + NSString *fieldPath = aggregateQuery[@"field"]; + assert(aggregateType); + assert(fieldPath); + + if([aggregateType isEqualToString:@"count"]){ + [aggregateFields addObject:[FIRAggregateField aggregateFieldForCount]]; + } else if([aggregateType isEqualToString:@"sum"]){ + [aggregateFields + addObject:[FIRAggregateField aggregateFieldForSumOfField:fieldPath]]; + } else if([aggregateType isEqualToString:@"average"]){ + [aggregateFields + addObject:[FIRAggregateField aggregateFieldForAverageOfField:fieldPath]]; + } else { + NSString *reason = [@"Invalid Aggregate Type: " stringByAppendingString:aggregateType]; + @throw [NSException exceptionWithName:@"RNFB Firestore: Invalid Aggregate Type" + reason:reason + userInfo:nil]; + } + } + + FIRAggregateQuery *aggregateQuery = [query aggregate:aggregateFields]; + + [aggregateQuery + aggregationWithSource:FIRAggregateSourceServer + completion:^(FIRAggregateQuerySnapshot *_Nullable snapshot, + NSError *_Nullable error) { + if (error) { + [RNFBFirestoreCommon promiseRejectFirestoreException:reject error:error]; + } else { + NSMutableDictionary *snapshotMap = [NSMutableDictionary dictionary]; + + for (NSDictionary *aggregateQuery in aggregateQueries) { + NSString *aggregateType = aggregateQuery[@"aggregateType"]; + NSString *fieldPath = aggregateQuery[@"field"]; + NSString *key = aggregateQuery[@"key"]; + assert(key); + + if([aggregateType isEqualToString:@"count"]){ + snapshotMap[key] = snapshot.count; + } else if([aggregateType isEqualToString:@"sum"]){ + NSNumber *sum = [snapshot + valueForAggregateField:[FIRAggregateField + aggregateFieldForSumOfField:fieldPath]]; + snapshotMap[key] = sum; + } else if([aggregateType isEqualToString:@"average"]){ + NSNumber *average = [snapshot + valueForAggregateField: + [FIRAggregateField + aggregateFieldForAverageOfField:fieldPath]]; + snapshotMap[key] = (average == nil ? [NSNull null] : average); + } + } + resolve(snapshotMap); + } + }]; +} + RCT_EXPORT_METHOD(collectionGet : (FIRApp *)firebaseApp : (NSString *)databaseId From 8203f964328d9c8cbb17cbdb36f9180712a567c6 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 6 Nov 2024 12:58:27 +0000 Subject: [PATCH 04/28] test: getAggregateFromServer() --- .../e2e/Aggregate/AggregateQuery.e2e.js | 161 ++++++++++++++++++ packages/firestore/lib/modular/index.js | 25 ++- 2 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js diff --git a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js new file mode 100644 index 0000000000..1a32bb07a5 --- /dev/null +++ b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js @@ -0,0 +1,161 @@ +/* + * Copyright (c) 2022-present Invertase Limited & Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this library 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. + * + */ +const COLLECTION = 'firestore'; +const { wipe } = require('../helpers'); +describe('getAggregateFromServer()', function () { + before(function () { + return wipe(); + }); + + it('throws if incorrect `query` argument', function () { + const { getAggregateFromServer, count } = firestoreModular; + const aggregateSpec = { + count: count(), + }; + try { + getAggregateFromServer(null, aggregateSpec); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + ); + } + + try { + getAggregateFromServer(undefined, aggregateSpec); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + ); + } + + try { + getAggregateFromServer('some-string', aggregateSpec); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + ); + } + return Promise.resolve(); + }); + + it('throws if incorrect `aggregateSpec` argument', function () { + const { getAggregateFromServer, collection, getFirestore, count } = firestoreModular; + + const colRef = collection(getFirestore(), `firestore`); + + try { + getAggregateFromServer(colRef, 'not an object'); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + '`getAggregateFromServer(query, *)` `aggregateSpec` muse be an object', + ); + } + + const aggregateSpec = { + count: "doesn't contain an aggregate field", + }; + + try { + getAggregateFromServer(colRef, aggregateSpec); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + '`getAggregateFromServer(query, *)` `aggregateSpec` must contain at least one `AggregateField`', + ); + } + + try { + getAggregateFromServer(colRef, aggregateSpec); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + ); + } + const aggField = count(); + aggField.aggregateType = 'change underlying type'; + + const aggregateSpec2 = { + count: aggField, + }; + + try { + getAggregateFromServer(colRef, aggregateSpec2); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql("'AggregateField' has an an unknown 'AggregateType'"); + } + return Promise.resolve(); + }); + + it('count(), average() & sum()', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, count, average, sum } = + firestoreModular; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-count/collection`); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { bar: 5, baz: 4 }), + setDoc(doc(colRef, 'two'), { bar: 5, baz: 4 }), + setDoc(doc(colRef, 'three'), { bar: 5, baz: 4 }), + ]); + + const aggregateSpec = { + ignoreThisProperty: 'not aggregate field', + countCollection: count(), + averageBar: average('bar'), + sumBaz: sum('baz'), + averageNonExisting: average('not-a-property'), + sumNotExisting: sum('not-a-property'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.countCollection.should.eql(3); + data.averageBar.should.eql(5); + data.sumBaz.should.eql(12); + // should only return the aggregate field requests + data.should.not.have.property('ignoreThisProperty'); + // average returns null on non-property + data.averageNonExisting.should.eql(null); + // sum returns 0 on non-property + data.sumNotExisting.should.eql(0); + + const colRefNoDocs = collection(firestore, `${COLLECTION}/aggregate-count/no-docs`); + + const aggregateSpecNoDocuments = { + countCollection: count(), + averageBar: average('bar'), + sumBaz: sum('baz'), + }; + + const resultNoDocs = await getAggregateFromServer(colRefNoDocs, aggregateSpecNoDocuments); + + const dataNoDocs = resultNoDocs.data(); + + dataNoDocs.countCollection.should.eql(0); + dataNoDocs.averageBar.should.eql(null); + dataNoDocs.sumBaz.should.eql(0); + }); +}); diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index 95d067f360..fd0d55135f 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -13,12 +13,14 @@ */ import { firebase } from '../index'; +import { isObject } from '@react-native-firebase/app/lib/common'; import { FirestoreAggregateQuerySnapshot, AggregateField, AggregateType, fieldPathFromArgument, } from '../FirestoreAggregate'; +import FirestoreQuery from '../FirestoreQuery'; /** * @param {FirebaseApp?} app @@ -199,11 +201,30 @@ export function getCountFromServer(query) { } export async function getAggregateFromServer(query, aggregateSpec) { + if (!(query instanceof FirestoreQuery)) { + throw new Error( + '`getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + ); + } + + if (!isObject(aggregateSpec)) { + throw new Error('`getAggregateFromServer(query, *)` `aggregateSpec` muse be an object'); + } else { + const containsOneAggregateField = Object.values(aggregateSpec).find( + value => value instanceof AggregateField, + ); + + if (!containsOneAggregateField) { + throw new Error( + '`getAggregateFromServer(query, *)` `aggregateSpec` must contain at least one `AggregateField`', + ); + } + } const aggregateQueries = []; for (const key in aggregateSpec) { if (aggregateSpec.hasOwnProperty(key)) { const aggregateField = aggregateSpec[key]; - + // we ignore any fields that are not `AggregateField` if (aggregateField instanceof AggregateField) { switch (aggregateField.aggregateType) { case AggregateType.AVG: @@ -218,7 +239,7 @@ export async function getAggregateFromServer(query, aggregateSpec) { break; default: throw new Error( - `"AggregateField" has an an unknown "AggregateType" : ${aggregateField.aggregateType}`, + `'AggregateField' has an an unknown 'AggregateType' : ${aggregateField.aggregateType}`, ); } } From 5727264d871bc4ac899cebbe25bf9c3c0ce2a5eb Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 6 Nov 2024 14:03:50 +0000 Subject: [PATCH 05/28] test: update e2e tests --- .../e2e/Aggregate/AggregateQuery.e2e.js | 259 +++++++++++++----- 1 file changed, 189 insertions(+), 70 deletions(-) diff --git a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js index 1a32bb07a5..3427f4578b 100644 --- a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js +++ b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js @@ -21,38 +21,40 @@ describe('getAggregateFromServer()', function () { return wipe(); }); - it('throws if incorrect `query` argument', function () { - const { getAggregateFromServer, count } = firestoreModular; - const aggregateSpec = { - count: count(), - }; - try { - getAggregateFromServer(null, aggregateSpec); - return Promise.reject(new Error('Did not throw an Error.')); - } catch (error) { - error.message.should.containEql( - 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', - ); - } + describe('throws exceptions for incorrect inputs', function () { + it('throws if incorrect `query` argument', function () { + const { getAggregateFromServer, count } = firestoreModular; + const aggregateSpec = { + count: count(), + }; + try { + getAggregateFromServer(null, aggregateSpec); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + ); + } - try { - getAggregateFromServer(undefined, aggregateSpec); - return Promise.reject(new Error('Did not throw an Error.')); - } catch (error) { - error.message.should.containEql( - 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', - ); - } + try { + getAggregateFromServer(undefined, aggregateSpec); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + ); + } - try { - getAggregateFromServer('some-string', aggregateSpec); - return Promise.reject(new Error('Did not throw an Error.')); - } catch (error) { - error.message.should.containEql( - 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', - ); - } - return Promise.resolve(); + try { + getAggregateFromServer('some-string', aggregateSpec); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + ); + } + return Promise.resolve(); + }); }); it('throws if incorrect `aggregateSpec` argument', function () { @@ -106,56 +108,173 @@ describe('getAggregateFromServer()', function () { return Promise.resolve(); }); - it('count(), average() & sum()', async function () { - const { getAggregateFromServer, doc, setDoc, collection, getFirestore, count, average, sum } = - firestoreModular; - const firestore = getFirestore(); + describe('count(), average() & sum()', function () { + it('no existing collection responses for average(), sum() & count()', async function () { + const { getAggregateFromServer, collection, getFirestore, count, average, sum } = + firestoreModular; + const firestore = getFirestore(); - const colRef = collection(firestore, `${COLLECTION}/aggregate-count/collection`); + const colRefNoDocs = collection(firestore, `${COLLECTION}/aggregate-count/no-docs`); - await Promise.all([ - setDoc(doc(colRef, 'one'), { bar: 5, baz: 4 }), - setDoc(doc(colRef, 'two'), { bar: 5, baz: 4 }), - setDoc(doc(colRef, 'three'), { bar: 5, baz: 4 }), - ]); + const aggregateSpecNoDocuments = { + countCollection: count(), + averageBar: average('bar'), + sumBaz: sum('baz'), + }; - const aggregateSpec = { - ignoreThisProperty: 'not aggregate field', - countCollection: count(), - averageBar: average('bar'), - sumBaz: sum('baz'), - averageNonExisting: average('not-a-property'), - sumNotExisting: sum('not-a-property'), - }; + const resultNoDocs = await getAggregateFromServer(colRefNoDocs, aggregateSpecNoDocuments); - const result = await getAggregateFromServer(colRef, aggregateSpec); + const dataNoDocs = resultNoDocs.data(); - const data = result.data(); + // average returns null, whilst sum and count return 0 + dataNoDocs.countCollection.should.eql(0); + dataNoDocs.averageBar.should.eql(null); + dataNoDocs.sumBaz.should.eql(0); + }); - data.countCollection.should.eql(3); - data.averageBar.should.eql(5); - data.sumBaz.should.eql(12); - // should only return the aggregate field requests - data.should.not.have.property('ignoreThisProperty'); - // average returns null on non-property - data.averageNonExisting.should.eql(null); - // sum returns 0 on non-property - data.sumNotExisting.should.eql(0); + it('single path using `string`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, count, average, sum } = + firestoreModular; + const firestore = getFirestore(); - const colRefNoDocs = collection(firestore, `${COLLECTION}/aggregate-count/no-docs`); + const colRef = collection(firestore, `${COLLECTION}/aggregate-count/collection`); - const aggregateSpecNoDocuments = { - countCollection: count(), - averageBar: average('bar'), - sumBaz: sum('baz'), - }; + await Promise.all([ + setDoc(doc(colRef, 'one'), { bar: 5, baz: 4 }), + setDoc(doc(colRef, 'two'), { bar: 5, baz: 4 }), + setDoc(doc(colRef, 'three'), { bar: 5, baz: 4 }), + ]); + + const aggregateSpec = { + ignoreThisProperty: 'not aggregate field', + countCollection: count(), + averageBar: average('bar'), + sumBaz: sum('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.countCollection.should.eql(3); + data.averageBar.should.eql(5); + data.sumBaz.should.eql(12); + // should only return the aggregate field requests + data.should.not.have.property('ignoreThisProperty'); + }); + + it('single path using `FieldPath`', async function () { + const { + getAggregateFromServer, + doc, + setDoc, + collection, + getFirestore, + count, + average, + sum, + FieldPath, + } = firestoreModular; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-count-field-path/collection`); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { bar: 5, baz: 4 }), + setDoc(doc(colRef, 'two'), { bar: 5, baz: 4 }), + setDoc(doc(colRef, 'three'), { bar: 5, baz: 4 }), + ]); + + const aggregateSpec = { + ignoreThisProperty: 'not aggregate field', + countCollection: count(), + averageBar: average(new FieldPath('bar')), + sumBaz: sum(new FieldPath('baz')), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.countCollection.should.eql(3); + data.averageBar.should.eql(5); + data.sumBaz.should.eql(12); + // should only return the aggregate field requests + data.should.not.have.property('ignoreThisProperty'); + }); + + it('nested object using `string`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, count, average, sum } = + firestoreModular; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-count-nested/collection`); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { foo: { bar: 5, baz: 4 } }), + setDoc(doc(colRef, 'two'), { foo: { bar: 5, baz: 4 } }), + setDoc(doc(colRef, 'three'), { foo: { bar: 5, baz: 4 } }), + ]); + + const aggregateSpec = { + ignoreThisProperty: 'not aggregate field', + countCollection: count(), + averageBar: average('foo.bar'), + sumBaz: sum('foo.baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.countCollection.should.eql(3); + data.averageBar.should.eql(5); + data.sumBaz.should.eql(12); + // should only return the aggregate field requests + data.should.not.have.property('ignoreThisProperty'); + }); + + it('nested object using `FieldPath`', async function () { + const { + getAggregateFromServer, + doc, + setDoc, + collection, + getFirestore, + count, + average, + sum, + FieldPath, + } = firestoreModular; + const firestore = getFirestore(); + + const colRef = collection( + firestore, + `${COLLECTION}/aggregate-count-nested-field-path/collection`, + ); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { foo: { bar: 5, baz: 4 } }), + setDoc(doc(colRef, 'two'), { foo: { bar: 5, baz: 4 } }), + setDoc(doc(colRef, 'three'), { foo: { bar: 5, baz: 4 } }), + ]); + + const aggregateSpec = { + ignoreThisProperty: 'not aggregate field', + countCollection: count(), + averageBar: average(new FieldPath('foo.bar')), + sumBaz: sum(new FieldPath('foo.baz')), + }; - const resultNoDocs = await getAggregateFromServer(colRefNoDocs, aggregateSpecNoDocuments); + const result = await getAggregateFromServer(colRef, aggregateSpec); - const dataNoDocs = resultNoDocs.data(); + const data = result.data(); - dataNoDocs.countCollection.should.eql(0); - dataNoDocs.averageBar.should.eql(null); - dataNoDocs.sumBaz.should.eql(0); + data.countCollection.should.eql(3); + data.averageBar.should.eql(5); + data.sumBaz.should.eql(12); + // should only return the aggregate field requests + data.should.not.have.property('ignoreThisProperty'); + }); }); }); From 988d7af665cc863f367e45dceb9fef541dadd732 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 6 Nov 2024 14:16:44 +0000 Subject: [PATCH 06/28] chore: improve typing --- packages/firestore/lib/FirestoreAggregate.js | 6 +++--- packages/firestore/lib/modular/index.d.ts | 7 +++---- packages/firestore/lib/modular/index.js | 3 ++- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/firestore/lib/FirestoreAggregate.js b/packages/firestore/lib/FirestoreAggregate.js index a52c96c104..a2437bee75 100644 --- a/packages/firestore/lib/FirestoreAggregate.js +++ b/packages/firestore/lib/FirestoreAggregate.js @@ -67,17 +67,17 @@ export const AggregateType = { export class AggregateField { /** Indicates the aggregation operation of this AggregateField. */ aggregateType; - fieldPath; + _fieldPath; /** * Create a new AggregateField * @param aggregateType Specifies the type of aggregation operation to perform. - * @param _internalFieldPath Optionally specifies the field that is aggregated. + * @param _fieldPath Optionally specifies the field that is aggregated. * @internal */ constructor(aggregateType, fieldPath) { this.aggregateType = aggregateType; - this.fieldPath = fieldPath; + this._fieldPath = fieldPath; } } diff --git a/packages/firestore/lib/modular/index.d.ts b/packages/firestore/lib/modular/index.d.ts index f7cbc91de6..df51ad2ace 100644 --- a/packages/firestore/lib/modular/index.d.ts +++ b/packages/firestore/lib/modular/index.d.ts @@ -10,6 +10,7 @@ import Query = FirebaseFirestoreTypes.Query; import FieldValue = FirebaseFirestoreTypes.FieldValue; import FieldPath = FirebaseFirestoreTypes.FieldPath; import PersistentCacheIndexManager = FirebaseFirestoreTypes.PersistentCacheIndexManager; +import AggregateQuerySnapshot = FirebaseFirestoreTypes.AggregateQuerySnapshot; /** Primitive types. */ export type Primitive = string | number | boolean | undefined | null; @@ -513,13 +514,11 @@ export type AggregateFieldType = export function getAggregateFromServer< AggregateSpecType extends AggregateSpec, AppModelType, - DbModelType extends FirebaseFirestoreTypes.DocumentData, + DbModelType extends DocumentData, >( query: Query, aggregateSpec: AggregateSpecType, -): Promise< - FirebaseFirestoreTypes.AggregateQuerySnapshot ->; +): Promise>; /** * Create an AggregateField object that can be used to compute the sum of diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index fd0d55135f..89da2cb436 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -232,7 +232,8 @@ export async function getAggregateFromServer(query, aggregateSpec) { case AggregateType.COUNT: const aggregateQuery = { aggregateType: aggregateField.aggregateType, - field: aggregateField.fieldPath === null ? null : aggregateField.fieldPath._toPath(), + field: + aggregateField._fieldPath === null ? null : aggregateField._fieldPath._toPath(), key, }; aggregateQueries.push(aggregateQuery); From 4264338dffdce5982617a2cde5b6fac7fa7c2d86 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 6 Nov 2024 14:20:47 +0000 Subject: [PATCH 07/28] chore: format --- ...tiveFirebaseFirestoreCollectionModule.java | 149 +++++++++--------- .../RNFBFirestoreCollectionModule.m | 85 +++++----- 2 files changed, 114 insertions(+), 120 deletions(-) diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java index 0022ffda2a..fe9aeb4cc1 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java @@ -28,11 +28,9 @@ import com.facebook.react.bridge.*; import com.google.android.gms.tasks.Tasks; import com.google.firebase.firestore.*; - -import java.util.ArrayList; - import io.invertase.firebase.common.ReactNativeFirebaseEventEmitter; import io.invertase.firebase.common.ReactNativeFirebaseModule; +import java.util.ArrayList; public class ReactNativeFirebaseFirestoreCollectionModule extends ReactNativeFirebaseModule { private static final String SERVICE_NAME = "FirestoreCollection"; @@ -200,25 +198,24 @@ public void collectionCount( @ReactMethod public void aggregateQuery( - String appName, - String databaseId, - String path, - String type, - ReadableArray filters, - ReadableArray orders, - ReadableMap options, - ReadableArray aggregateQueries, - Promise promise - ){ + String appName, + String databaseId, + String path, + String type, + ReadableArray filters, + ReadableArray orders, + ReadableMap options, + ReadableArray aggregateQueries, + Promise promise) { FirebaseFirestore firebaseFirestore = getFirestoreForApp(appName, databaseId); ReactNativeFirebaseFirestoreQuery firestoreQuery = - new ReactNativeFirebaseFirestoreQuery( - appName, - databaseId, - getQueryForFirestore(firebaseFirestore, path, type), - filters, - orders, - options); + new ReactNativeFirebaseFirestoreQuery( + appName, + databaseId, + getQueryForFirestore(firebaseFirestore, path, type), + filters, + orders, + options); ArrayList aggregateFields = new ArrayList<>(); @@ -229,66 +226,68 @@ public void aggregateQuery( String fieldPath = aggregateQuery.getString("field"); assert aggregateType != null; - switch (aggregateType) { - case "count": - aggregateFields.add(AggregateField.count()); - break; - case "sum": - assert fieldPath != null; - aggregateFields.add(AggregateField.sum(fieldPath)); - break; - case "average": - assert fieldPath != null; - aggregateFields.add(AggregateField.average(fieldPath)); - break; - default: - throw new Error("Invalid AggregateType: " + aggregateType); - } + switch (aggregateType) { + case "count": + aggregateFields.add(AggregateField.count()); + break; + case "sum": + assert fieldPath != null; + aggregateFields.add(AggregateField.sum(fieldPath)); + break; + case "average": + assert fieldPath != null; + aggregateFields.add(AggregateField.average(fieldPath)); + break; + default: + throw new Error("Invalid AggregateType: " + aggregateType); + } } - AggregateQuery firestoreAggregateQuery = firestoreQuery.query.aggregate(aggregateFields.get(0), - aggregateFields.subList(1, aggregateFields.size()).toArray(new AggregateField[0])); + AggregateQuery firestoreAggregateQuery = + firestoreQuery.query.aggregate( + aggregateFields.get(0), + aggregateFields.subList(1, aggregateFields.size()).toArray(new AggregateField[0])); - firestoreAggregateQuery + firestoreAggregateQuery .get(AggregateSource.SERVER) .addOnCompleteListener( - task -> { - if (task.isSuccessful()) { - WritableMap result = Arguments.createMap(); - AggregateQuerySnapshot snapshot = task.getResult(); - - for (int k = 0; k < aggregateQueries.size(); k++) { - ReadableMap aggQuery = aggregateQueries.getMap(k); - String aggType = aggQuery.getString("aggregateType"); - String field = aggQuery.getString("field"); - String key = aggQuery.getString("key"); - assert key != null; - assert aggType != null; - switch (aggType) { - case "count": - result.putDouble(key, Long.valueOf(snapshot.getCount()).doubleValue()); - break; - case "sum": - assert field != null; - Number sum = (Number) snapshot.get(sum(field)); - assert sum != null; - result.putDouble(key, sum.doubleValue()); - break; - case "average": - assert field != null; - Number average = snapshot.get(average(field)); - assert average != null; - result.putDouble(key, average.doubleValue()); - break; - default: - throw new Error("Invalid AggregateType: " + aggType); + task -> { + if (task.isSuccessful()) { + WritableMap result = Arguments.createMap(); + AggregateQuerySnapshot snapshot = task.getResult(); + + for (int k = 0; k < aggregateQueries.size(); k++) { + ReadableMap aggQuery = aggregateQueries.getMap(k); + String aggType = aggQuery.getString("aggregateType"); + String field = aggQuery.getString("field"); + String key = aggQuery.getString("key"); + assert key != null; + assert aggType != null; + switch (aggType) { + case "count": + result.putDouble(key, Long.valueOf(snapshot.getCount()).doubleValue()); + break; + case "sum": + assert field != null; + Number sum = (Number) snapshot.get(sum(field)); + assert sum != null; + result.putDouble(key, sum.doubleValue()); + break; + case "average": + assert field != null; + Number average = snapshot.get(average(field)); + assert average != null; + result.putDouble(key, average.doubleValue()); + break; + default: + throw new Error("Invalid AggregateType: " + aggType); + } } - } - promise.resolve(result); - } else { - rejectPromiseFirestoreException(promise, task.getException()); - } - }); + promise.resolve(result); + } else { + rejectPromiseFirestoreException(promise, task.getException()); + } + }); } @ReactMethod @@ -312,8 +311,6 @@ public void collectionGet( orders, options); handleQueryGet(firestoreQuery, getSource(getOptions), promise); - - } private void handleQueryOnSnapshot( diff --git a/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m b/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m index 4182acc5f8..966dd9f12d 100644 --- a/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m +++ b/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m @@ -232,7 +232,7 @@ - (void)invalidate { FIRQuery *query = [RNFBFirestoreCommon getQueryForFirestore:firestore path:path type:type]; NSMutableArray *aggregateFields = - [[NSMutableArray alloc] init]; + [[NSMutableArray alloc] init]; for (NSDictionary *aggregateQuery in aggregateQueries) { NSString *aggregateType = aggregateQuery[@"aggregateType"]; @@ -240,57 +240,54 @@ - (void)invalidate { assert(aggregateType); assert(fieldPath); - if([aggregateType isEqualToString:@"count"]){ + if ([aggregateType isEqualToString:@"count"]) { [aggregateFields addObject:[FIRAggregateField aggregateFieldForCount]]; - } else if([aggregateType isEqualToString:@"sum"]){ - [aggregateFields - addObject:[FIRAggregateField aggregateFieldForSumOfField:fieldPath]]; - } else if([aggregateType isEqualToString:@"average"]){ - [aggregateFields - addObject:[FIRAggregateField aggregateFieldForAverageOfField:fieldPath]]; + } else if ([aggregateType isEqualToString:@"sum"]) { + [aggregateFields addObject:[FIRAggregateField aggregateFieldForSumOfField:fieldPath]]; + } else if ([aggregateType isEqualToString:@"average"]) { + [aggregateFields addObject:[FIRAggregateField aggregateFieldForAverageOfField:fieldPath]]; } else { NSString *reason = [@"Invalid Aggregate Type: " stringByAppendingString:aggregateType]; @throw [NSException exceptionWithName:@"RNFB Firestore: Invalid Aggregate Type" - reason:reason - userInfo:nil]; + reason:reason + userInfo:nil]; } } - + FIRAggregateQuery *aggregateQuery = [query aggregate:aggregateFields]; - + [aggregateQuery - aggregationWithSource:FIRAggregateSourceServer - completion:^(FIRAggregateQuerySnapshot *_Nullable snapshot, - NSError *_Nullable error) { - if (error) { - [RNFBFirestoreCommon promiseRejectFirestoreException:reject error:error]; - } else { - NSMutableDictionary *snapshotMap = [NSMutableDictionary dictionary]; - - for (NSDictionary *aggregateQuery in aggregateQueries) { - NSString *aggregateType = aggregateQuery[@"aggregateType"]; - NSString *fieldPath = aggregateQuery[@"field"]; - NSString *key = aggregateQuery[@"key"]; - assert(key); - - if([aggregateType isEqualToString:@"count"]){ - snapshotMap[key] = snapshot.count; - } else if([aggregateType isEqualToString:@"sum"]){ - NSNumber *sum = [snapshot - valueForAggregateField:[FIRAggregateField - aggregateFieldForSumOfField:fieldPath]]; - snapshotMap[key] = sum; - } else if([aggregateType isEqualToString:@"average"]){ - NSNumber *average = [snapshot - valueForAggregateField: - [FIRAggregateField - aggregateFieldForAverageOfField:fieldPath]]; - snapshotMap[key] = (average == nil ? [NSNull null] : average); - } - } - resolve(snapshotMap); - } - }]; + aggregationWithSource:FIRAggregateSourceServer + completion:^(FIRAggregateQuerySnapshot *_Nullable snapshot, + NSError *_Nullable error) { + if (error) { + [RNFBFirestoreCommon promiseRejectFirestoreException:reject error:error]; + } else { + NSMutableDictionary *snapshotMap = [NSMutableDictionary dictionary]; + + for (NSDictionary *aggregateQuery in aggregateQueries) { + NSString *aggregateType = aggregateQuery[@"aggregateType"]; + NSString *fieldPath = aggregateQuery[@"field"]; + NSString *key = aggregateQuery[@"key"]; + assert(key); + + if ([aggregateType isEqualToString:@"count"]) { + snapshotMap[key] = snapshot.count; + } else if ([aggregateType isEqualToString:@"sum"]) { + NSNumber *sum = [snapshot + valueForAggregateField:[FIRAggregateField + aggregateFieldForSumOfField:fieldPath]]; + snapshotMap[key] = sum; + } else if ([aggregateType isEqualToString:@"average"]) { + NSNumber *average = [snapshot + valueForAggregateField:[FIRAggregateField + aggregateFieldForAverageOfField:fieldPath]]; + snapshotMap[key] = (average == nil ? [NSNull null] : average); + } + } + resolve(snapshotMap); + } + }]; } RCT_EXPORT_METHOD(collectionGet From fa1a1e8b8e05e50a0648208b56181bcf55f24337 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 6 Nov 2024 14:36:08 +0000 Subject: [PATCH 08/28] chore: rm assertions --- ...tiveFirebaseFirestoreCollectionModule.java | 30 +++++++------------ .../RNFBFirestoreCollectionModule.m | 3 -- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java index fe9aeb4cc1..e535b81e8c 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java @@ -31,6 +31,7 @@ import io.invertase.firebase.common.ReactNativeFirebaseEventEmitter; import io.invertase.firebase.common.ReactNativeFirebaseModule; import java.util.ArrayList; +import java.util.Objects; public class ReactNativeFirebaseFirestoreCollectionModule extends ReactNativeFirebaseModule { private static final String SERVICE_NAME = "FirestoreCollection"; @@ -221,22 +222,18 @@ public void aggregateQuery( for (int i = 0; i < aggregateQueries.size(); i++) { ReadableMap aggregateQuery = aggregateQueries.getMap(i); - String aggregateType = aggregateQuery.getString("aggregateType"); String fieldPath = aggregateQuery.getString("field"); - assert aggregateType != null; - switch (aggregateType) { + switch (Objects.requireNonNull(aggregateType)) { case "count": aggregateFields.add(AggregateField.count()); break; case "sum": - assert fieldPath != null; - aggregateFields.add(AggregateField.sum(fieldPath)); + aggregateFields.add(AggregateField.sum(Objects.requireNonNull(fieldPath))); break; case "average": - assert fieldPath != null; - aggregateFields.add(AggregateField.average(fieldPath)); + aggregateFields.add(AggregateField.average(Objects.requireNonNull(fieldPath))); break; default: throw new Error("Invalid AggregateType: " + aggregateType); @@ -260,23 +257,18 @@ public void aggregateQuery( String aggType = aggQuery.getString("aggregateType"); String field = aggQuery.getString("field"); String key = aggQuery.getString("key"); - assert key != null; - assert aggType != null; - switch (aggType) { + + switch (Objects.requireNonNull(aggType)) { case "count": - result.putDouble(key, Long.valueOf(snapshot.getCount()).doubleValue()); + result.putDouble(Objects.requireNonNull(key), Long.valueOf(snapshot.getCount()).doubleValue()); break; case "sum": - assert field != null; - Number sum = (Number) snapshot.get(sum(field)); - assert sum != null; - result.putDouble(key, sum.doubleValue()); + Number sum = (Number) snapshot.get(sum(Objects.requireNonNull(field))); + result.putDouble(Objects.requireNonNull(key), Objects.requireNonNull(sum).doubleValue()); break; case "average": - assert field != null; - Number average = snapshot.get(average(field)); - assert average != null; - result.putDouble(key, average.doubleValue()); + Number average = snapshot.get(average(Objects.requireNonNull(field))); + result.putDouble(Objects.requireNonNull(key), Objects.requireNonNull(average).doubleValue()); break; default: throw new Error("Invalid AggregateType: " + aggType); diff --git a/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m b/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m index 966dd9f12d..1895abaf66 100644 --- a/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m +++ b/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m @@ -237,8 +237,6 @@ - (void)invalidate { for (NSDictionary *aggregateQuery in aggregateQueries) { NSString *aggregateType = aggregateQuery[@"aggregateType"]; NSString *fieldPath = aggregateQuery[@"field"]; - assert(aggregateType); - assert(fieldPath); if ([aggregateType isEqualToString:@"count"]) { [aggregateFields addObject:[FIRAggregateField aggregateFieldForCount]]; @@ -269,7 +267,6 @@ - (void)invalidate { NSString *aggregateType = aggregateQuery[@"aggregateType"]; NSString *fieldPath = aggregateQuery[@"field"]; NSString *key = aggregateQuery[@"key"]; - assert(key); if ([aggregateType isEqualToString:@"count"]) { snapshotMap[key] = snapshot.count; From 8b76a9b2c09f829c598320e7cd53dff64a067567 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 6 Nov 2024 14:37:42 +0000 Subject: [PATCH 09/28] chore: format --- .../ReactNativeFirebaseFirestoreCollectionModule.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java index e535b81e8c..ce368b0e7d 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java @@ -260,15 +260,20 @@ public void aggregateQuery( switch (Objects.requireNonNull(aggType)) { case "count": - result.putDouble(Objects.requireNonNull(key), Long.valueOf(snapshot.getCount()).doubleValue()); + result.putDouble( + Objects.requireNonNull(key), + Long.valueOf(snapshot.getCount()).doubleValue()); break; case "sum": Number sum = (Number) snapshot.get(sum(Objects.requireNonNull(field))); - result.putDouble(Objects.requireNonNull(key), Objects.requireNonNull(sum).doubleValue()); + result.putDouble( + Objects.requireNonNull(key), Objects.requireNonNull(sum).doubleValue()); break; case "average": Number average = snapshot.get(average(Objects.requireNonNull(field))); - result.putDouble(Objects.requireNonNull(key), Objects.requireNonNull(average).doubleValue()); + result.putDouble( + Objects.requireNonNull(key), + Objects.requireNonNull(average).doubleValue()); break; default: throw new Error("Invalid AggregateType: " + aggType); From 9cd5ebc5fb411d751ad8769705060ab8dcd9a320 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 6 Nov 2024 15:20:21 +0000 Subject: [PATCH 10/28] feat: 'other' platform support --- .../firestore/lib/web/RNFBFirestoreModule.js | 42 +++++++++++++++++++ packages/firestore/lib/web/query.js | 2 +- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/packages/firestore/lib/web/RNFBFirestoreModule.js b/packages/firestore/lib/web/RNFBFirestoreModule.js index ac942a55b0..1a52f77737 100644 --- a/packages/firestore/lib/web/RNFBFirestoreModule.js +++ b/packages/firestore/lib/web/RNFBFirestoreModule.js @@ -11,6 +11,10 @@ import { getDoc, getDocs, getCount, + getAggregateFromServer, + count, + average, + sum, deleteDoc, setDoc, updateDoc, @@ -215,6 +219,44 @@ export default { }); }, + aggregateQuery(appName, databaseId, path, type, filters, orders, options, aggregateQueries) { + return guard(async () => { + const firestore = getCachedFirestoreInstance(appName, databaseId); + const queryRef = + type === 'collectionGroup' ? collectionGroup(firestore, path) : collection(firestore, path); + const query = buildQuery(queryRef, filters, orders, options); + const aggregateSpec = {}; + + for (let i = 0; i < aggregateQueries.length; i++) { + const aggregateQuery = aggregateQueries[i]; + const { aggregateType, field, key } = aggregateQuery; + + switch (aggregateType) { + case 'count': + aggregateSpec[key] = count(); + break; + case 'average': + aggregateSpec[key] = average(field); + break; + case 'sum': + aggregateSpec[key] = sum(field); + break; + } + } + const result = await getAggregateFromServer(query, aggregateSpec); + + const data = result.data(); + const response = {}; + for (let i = 0; i < aggregateQueries.length; i++) { + const aggregateQuery = aggregateQueries[i]; + const { key } = aggregateQuery; + response[key] = data[key]; + } + + return response; + }); + }, + /** * Get a collection from Firestore. * @param {string} appName - The app name. diff --git a/packages/firestore/lib/web/query.js b/packages/firestore/lib/web/query.js index e181e56980..eed7df8cdd 100644 --- a/packages/firestore/lib/web/query.js +++ b/packages/firestore/lib/web/query.js @@ -108,5 +108,5 @@ function getFilterConstraint(filter) { throw new Error('Invalid filter operator'); } - throw new Error('Invaldi filter.'); + throw new Error('Invalid filter.'); } From 5f54a16ccae45af914dd5074707cd625813d95dd Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 6 Nov 2024 15:40:12 +0000 Subject: [PATCH 11/28] tes: fix test scopes --- .../e2e/Aggregate/AggregateQuery.e2e.js | 96 +++++++++---------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js index 3427f4578b..12ef953641 100644 --- a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js +++ b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js @@ -55,57 +55,57 @@ describe('getAggregateFromServer()', function () { } return Promise.resolve(); }); - }); - it('throws if incorrect `aggregateSpec` argument', function () { - const { getAggregateFromServer, collection, getFirestore, count } = firestoreModular; + it('throws if incorrect `aggregateSpec` argument', function () { + const { getAggregateFromServer, collection, getFirestore, count } = firestoreModular; - const colRef = collection(getFirestore(), `firestore`); + const colRef = collection(getFirestore(), `firestore`); - try { - getAggregateFromServer(colRef, 'not an object'); - return Promise.reject(new Error('Did not throw an Error.')); - } catch (error) { - error.message.should.containEql( - '`getAggregateFromServer(query, *)` `aggregateSpec` muse be an object', - ); - } - - const aggregateSpec = { - count: "doesn't contain an aggregate field", - }; - - try { - getAggregateFromServer(colRef, aggregateSpec); - return Promise.reject(new Error('Did not throw an Error.')); - } catch (error) { - error.message.should.containEql( - '`getAggregateFromServer(query, *)` `aggregateSpec` must contain at least one `AggregateField`', - ); - } - - try { - getAggregateFromServer(colRef, aggregateSpec); - return Promise.reject(new Error('Did not throw an Error.')); - } catch (error) { - error.message.should.containEql( - 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', - ); - } - const aggField = count(); - aggField.aggregateType = 'change underlying type'; - - const aggregateSpec2 = { - count: aggField, - }; - - try { - getAggregateFromServer(colRef, aggregateSpec2); - return Promise.reject(new Error('Did not throw an Error.')); - } catch (error) { - error.message.should.containEql("'AggregateField' has an an unknown 'AggregateType'"); - } - return Promise.resolve(); + try { + getAggregateFromServer(colRef, 'not an object'); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + '`getAggregateFromServer(query, *)` `aggregateSpec` muse be an object', + ); + } + + const aggregateSpec = { + count: "doesn't contain an aggregate field", + }; + + try { + getAggregateFromServer(colRef, aggregateSpec); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + '`getAggregateFromServer(query, *)` `aggregateSpec` must contain at least one `AggregateField`', + ); + } + + try { + getAggregateFromServer(colRef, aggregateSpec); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql( + 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + ); + } + const aggField = count(); + aggField.aggregateType = 'change underlying type'; + + const aggregateSpec2 = { + count: aggField, + }; + + try { + getAggregateFromServer(colRef, aggregateSpec2); + return Promise.reject(new Error('Did not throw an Error.')); + } catch (error) { + error.message.should.containEql("'AggregateField' has an an unknown 'AggregateType'"); + } + return Promise.resolve(); + }); }); describe('count(), average() & sum()', function () { From 8db15d5c94caedf91a3b6ab934afcd04b7df1361 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 6 Nov 2024 17:04:09 +0000 Subject: [PATCH 12/28] fix: firestore lite has different name for API --- packages/firestore/lib/web/RNFBFirestoreModule.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/firestore/lib/web/RNFBFirestoreModule.js b/packages/firestore/lib/web/RNFBFirestoreModule.js index 1a52f77737..c69edb7bea 100644 --- a/packages/firestore/lib/web/RNFBFirestoreModule.js +++ b/packages/firestore/lib/web/RNFBFirestoreModule.js @@ -11,7 +11,7 @@ import { getDoc, getDocs, getCount, - getAggregateFromServer, + getAggregate, count, average, sum, @@ -243,7 +243,7 @@ export default { break; } } - const result = await getAggregateFromServer(query, aggregateSpec); + const result = await getAggregate(query, aggregateSpec); const data = result.data(); const response = {}; From 5c38c5b03c45729d6c5cfe07a929627774bf223c Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 6 Nov 2024 17:14:44 +0000 Subject: [PATCH 13/28] test: ensure exposed to end user --- packages/firestore/__tests__/firestore.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index 28e232c53f..f6b019c1e7 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -4,6 +4,7 @@ import firestore, { firebase, Filter, getFirestore, + getAggregateFromServer, addDoc, doc, collection, @@ -651,6 +652,10 @@ describe('Firestore', function () { it('`enablePersistentCacheIndexAutoCreation` is properly exposed to end user', function () { expect(enablePersistentCacheIndexAutoCreation).toBeDefined(); }); + + it('`getAggregateFromServer` is properly exposed to end user', function () { + expect(getAggregateFromServer).toBeDefined(); + }); }); describe('FirestorePersistentCacheIndexManager', function () { From 148cfb1ea6be0078ed8c4c000395a15ac17a4b85 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 7 Nov 2024 10:14:45 +0000 Subject: [PATCH 14/28] test: fix broken tests --- packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js | 10 +--------- packages/firestore/lib/modular/index.js | 2 +- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js index 12ef953641..d8ebb1921e 100644 --- a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js +++ b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js @@ -83,14 +83,6 @@ describe('getAggregateFromServer()', function () { ); } - try { - getAggregateFromServer(colRef, aggregateSpec); - return Promise.reject(new Error('Did not throw an Error.')); - } catch (error) { - error.message.should.containEql( - 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', - ); - } const aggField = count(); aggField.aggregateType = 'change underlying type'; @@ -128,7 +120,7 @@ describe('getAggregateFromServer()', function () { // average returns null, whilst sum and count return 0 dataNoDocs.countCollection.should.eql(0); - dataNoDocs.averageBar.should.eql(null); + should(dataNoDocs.averageBar).be.null(); dataNoDocs.sumBaz.should.eql(0); }); diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index 89da2cb436..54cb050cdb 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -200,7 +200,7 @@ export function getCountFromServer(query) { return query.count().get(); } -export async function getAggregateFromServer(query, aggregateSpec) { +export function getAggregateFromServer(query, aggregateSpec) { if (!(query instanceof FirestoreQuery)) { throw new Error( '`getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', From de34cc2dcb76c2bd195b7d4297fe267fabc8abfe Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 7 Nov 2024 11:19:35 +0000 Subject: [PATCH 15/28] fix(android): allow null value for average --- .../ReactNativeFirebaseFirestoreCollectionModule.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java index ce368b0e7d..113e5e16b8 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java @@ -271,9 +271,12 @@ public void aggregateQuery( break; case "average": Number average = snapshot.get(average(Objects.requireNonNull(field))); - result.putDouble( - Objects.requireNonNull(key), - Objects.requireNonNull(average).doubleValue()); + String averageKey = Objects.requireNonNull(key); + if (average == null) { + result.putNull(averageKey); + } else { + result.putDouble(averageKey, Objects.requireNonNull(average).doubleValue()); + } break; default: throw new Error("Invalid AggregateType: " + aggType); From bb76c4d802890855cb7b1f151f422bea28101715 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 7 Nov 2024 14:12:21 +0000 Subject: [PATCH 16/28] chore: fix typo --- packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js | 8 ++++---- packages/firestore/lib/modular/index.js | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js index d8ebb1921e..4adda7901d 100644 --- a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js +++ b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js @@ -32,7 +32,7 @@ describe('getAggregateFromServer()', function () { return Promise.reject(new Error('Did not throw an Error.')); } catch (error) { error.message.should.containEql( - 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + 'getAggregateFromServer(*, aggregateSpec)` `query` must be an instance of `FirestoreQuery`', ); } @@ -41,7 +41,7 @@ describe('getAggregateFromServer()', function () { return Promise.reject(new Error('Did not throw an Error.')); } catch (error) { error.message.should.containEql( - 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + 'getAggregateFromServer(*, aggregateSpec)` `query` must be an instance of `FirestoreQuery`', ); } @@ -50,7 +50,7 @@ describe('getAggregateFromServer()', function () { return Promise.reject(new Error('Did not throw an Error.')); } catch (error) { error.message.should.containEql( - 'getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + 'getAggregateFromServer(*, aggregateSpec)` `query` must be an instance of `FirestoreQuery`', ); } return Promise.resolve(); @@ -66,7 +66,7 @@ describe('getAggregateFromServer()', function () { return Promise.reject(new Error('Did not throw an Error.')); } catch (error) { error.message.should.containEql( - '`getAggregateFromServer(query, *)` `aggregateSpec` muse be an object', + '`getAggregateFromServer(query, *)` `aggregateSpec` must be an object', ); } diff --git a/packages/firestore/lib/modular/index.js b/packages/firestore/lib/modular/index.js index 54cb050cdb..4242e983eb 100644 --- a/packages/firestore/lib/modular/index.js +++ b/packages/firestore/lib/modular/index.js @@ -203,12 +203,12 @@ export function getCountFromServer(query) { export function getAggregateFromServer(query, aggregateSpec) { if (!(query instanceof FirestoreQuery)) { throw new Error( - '`getAggregateFromServer(*, aggregateSpec)` `query` muse be an instance of `FirestoreQuery`', + '`getAggregateFromServer(*, aggregateSpec)` `query` must be an instance of `FirestoreQuery`', ); } if (!isObject(aggregateSpec)) { - throw new Error('`getAggregateFromServer(query, *)` `aggregateSpec` muse be an object'); + throw new Error('`getAggregateFromServer(query, *)` `aggregateSpec` must be an object'); } else { const containsOneAggregateField = Object.values(aggregateSpec).find( value => value instanceof AggregateField, From 9f8542b7baffc85bc7ae3b988c943764e6638933 Mon Sep 17 00:00:00 2001 From: Mike Hardy Date: Thu, 7 Nov 2024 11:42:45 -0500 Subject: [PATCH 17/28] fix(firestore, android): send null errors through promise reject path having native module exceptions vs promise rejects requires JS level code to handle multiple types of error vs being able to use one style --- ...tiveFirebaseFirestoreCollectionModule.java | 50 ++++++++++++------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java index 113e5e16b8..99ff79e91d 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java @@ -31,7 +31,6 @@ import io.invertase.firebase.common.ReactNativeFirebaseEventEmitter; import io.invertase.firebase.common.ReactNativeFirebaseModule; import java.util.ArrayList; -import java.util.Objects; public class ReactNativeFirebaseFirestoreCollectionModule extends ReactNativeFirebaseModule { private static final String SERVICE_NAME = "FirestoreCollection"; @@ -223,20 +222,26 @@ public void aggregateQuery( for (int i = 0; i < aggregateQueries.size(); i++) { ReadableMap aggregateQuery = aggregateQueries.getMap(i); String aggregateType = aggregateQuery.getString("aggregateType"); + if (aggregateType == null) aggregateType = ""; String fieldPath = aggregateQuery.getString("field"); + if (fieldPath == null) { + promise.reject("firestore/invalid-argument", "fieldPath cannot be null"); + return; + } - switch (Objects.requireNonNull(aggregateType)) { + switch (aggregateType) { case "count": aggregateFields.add(AggregateField.count()); break; case "sum": - aggregateFields.add(AggregateField.sum(Objects.requireNonNull(fieldPath))); + aggregateFields.add(AggregateField.sum(fieldPath)); break; case "average": - aggregateFields.add(AggregateField.average(Objects.requireNonNull(fieldPath))); + aggregateFields.add(AggregateField.average(fieldPath)); break; default: - throw new Error("Invalid AggregateType: " + aggregateType); + promise.reject("firestore/invalid-argument", "Invalid AggregateType: " + aggregateType); + return; } } AggregateQuery firestoreAggregateQuery = @@ -255,31 +260,42 @@ public void aggregateQuery( for (int k = 0; k < aggregateQueries.size(); k++) { ReadableMap aggQuery = aggregateQueries.getMap(k); String aggType = aggQuery.getString("aggregateType"); + if (aggType == null) aggType = ""; String field = aggQuery.getString("field"); + if (field == null) { + promise.reject("firestore/invalid-argument", "field may not be null"); + return; + } String key = aggQuery.getString("key"); + if (key == null) { + promise.reject("firestore/invalid-argument", "key may not be null"); + return; + } - switch (Objects.requireNonNull(aggType)) { + switch (aggType) { case "count": - result.putDouble( - Objects.requireNonNull(key), - Long.valueOf(snapshot.getCount()).doubleValue()); + result.putDouble(key, Long.valueOf(snapshot.getCount()).doubleValue()); break; case "sum": - Number sum = (Number) snapshot.get(sum(Objects.requireNonNull(field))); - result.putDouble( - Objects.requireNonNull(key), Objects.requireNonNull(sum).doubleValue()); + Number sum = (Number) snapshot.get(sum(field)); + if (sum == null) { + promise.reject("firestore/unknown", "sum unexpectedly null"); + return; + } + result.putDouble(key, sum.doubleValue()); break; case "average": - Number average = snapshot.get(average(Objects.requireNonNull(field))); - String averageKey = Objects.requireNonNull(key); + Number average = snapshot.get(average(field)); if (average == null) { - result.putNull(averageKey); + result.putNull(key); } else { - result.putDouble(averageKey, Objects.requireNonNull(average).doubleValue()); + result.putDouble(key, average.doubleValue()); } break; default: - throw new Error("Invalid AggregateType: " + aggType); + promise.reject( + "firestore/invalid-argument", "Invalid AggregateType: " + aggType); + return; } } From f66838f9046b8f791694f871515a5174d2e47ca4 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 8 Nov 2024 10:06:51 +0000 Subject: [PATCH 18/28] test: update aggregate query to see what happens with float handling --- .../firestore/e2e/Aggregate/AggregateQuery.e2e.js | 14 +++++++------- packages/firestore/e2e/Aggregate/count.e2e.js | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js index 4adda7901d..88cb0f3469 100644 --- a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js +++ b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js @@ -17,8 +17,8 @@ const COLLECTION = 'firestore'; const { wipe } = require('../helpers'); describe('getAggregateFromServer()', function () { - before(function () { - return wipe(); + before(async function () { + return await wipe(); }); describe('throws exceptions for incorrect inputs', function () { @@ -132,9 +132,9 @@ describe('getAggregateFromServer()', function () { const colRef = collection(firestore, `${COLLECTION}/aggregate-count/collection`); await Promise.all([ - setDoc(doc(colRef, 'one'), { bar: 5, baz: 4 }), - setDoc(doc(colRef, 'two'), { bar: 5, baz: 4 }), - setDoc(doc(colRef, 'three'), { bar: 5, baz: 4 }), + setDoc(doc(colRef, 'one'), { bar: 0.4, baz: 0.1 }), + setDoc(doc(colRef, 'two'), { bar: 0.5, baz: 0.1 }), + setDoc(doc(colRef, 'three'), { bar: 0.6, baz: 0.1 }), ]); const aggregateSpec = { @@ -149,8 +149,8 @@ describe('getAggregateFromServer()', function () { const data = result.data(); data.countCollection.should.eql(3); - data.averageBar.should.eql(5); - data.sumBaz.should.eql(12); + data.averageBar.should.eql(0.5); + data.sumBaz.should.eql(0.3); // should only return the aggregate field requests data.should.not.have.property('ignoreThisProperty'); }); diff --git a/packages/firestore/e2e/Aggregate/count.e2e.js b/packages/firestore/e2e/Aggregate/count.e2e.js index 80997cb59b..126714320d 100644 --- a/packages/firestore/e2e/Aggregate/count.e2e.js +++ b/packages/firestore/e2e/Aggregate/count.e2e.js @@ -17,8 +17,8 @@ const COLLECTION = 'firestore'; const { wipe } = require('../helpers'); describe('firestore().collection().count()', function () { - before(function () { - return wipe(); + before(async function () { + return await wipe(); }); describe('v8 compatibility', function () { From 056f45caec5473406c214e130ff12593f05a7663 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 8 Nov 2024 10:07:39 +0000 Subject: [PATCH 19/28] fix: update exception handling iOS --- .../ios/RNFBFirestore/RNFBFirestoreCollectionModule.m | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m b/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m index 1895abaf66..ff520037f7 100644 --- a/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m +++ b/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m @@ -246,9 +246,12 @@ - (void)invalidate { [aggregateFields addObject:[FIRAggregateField aggregateFieldForAverageOfField:fieldPath]]; } else { NSString *reason = [@"Invalid Aggregate Type: " stringByAppendingString:aggregateType]; - @throw [NSException exceptionWithName:@"RNFB Firestore: Invalid Aggregate Type" - reason:reason - userInfo:nil]; + [RNFBFirestoreCommon + promiseRejectFirestoreException:reject + error:[NSException exceptionWithName: + @"RNFB Firestore: Invalid Aggregate Type" + reason:reason + userInfo:nil]]; } } From b4428276350ccd10b0681c63dc9dada04234e06b Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 8 Nov 2024 10:14:29 +0000 Subject: [PATCH 20/28] chore: AggregateQuerySnapshot type update --- packages/firestore/lib/index.d.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/firestore/lib/index.d.ts b/packages/firestore/lib/index.d.ts index 261250473c..bdbdc551b0 100644 --- a/packages/firestore/lib/index.d.ts +++ b/packages/firestore/lib/index.d.ts @@ -921,12 +921,16 @@ export namespace FirebaseFirestoreTypes { /** * The results of executing an aggregation query. */ - export interface AggregateQuerySnapshot { + export interface AggregateQuerySnapshot< + AggregateSpecType extends AggregateSpec, + AppModelType = DocumentData, + DbModelType extends DocumentData = DocumentData, + > { /** * The underlying query over which the aggregations recorded in this * `AggregateQuerySnapshot` were performed. */ - get query(): Query; + get query(): Query; /** * Returns the results of the aggregations performed over the underlying @@ -939,7 +943,7 @@ export namespace FirebaseFirestoreTypes { * @returns The results of the aggregations performed over the underlying * query. */ - data(): AggregateSpecData; + data(): AggregateSpecData; } /** From f5a2fe50e543b87ba4dd808e9c1f476c63d52019 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 8 Nov 2024 15:15:09 +0000 Subject: [PATCH 21/28] fix: return after promise rejection --- .../firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m b/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m index ff520037f7..9ed2aa8b7b 100644 --- a/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m +++ b/packages/firestore/ios/RNFBFirestore/RNFBFirestoreCollectionModule.m @@ -252,6 +252,7 @@ - (void)invalidate { @"RNFB Firestore: Invalid Aggregate Type" reason:reason userInfo:nil]]; + return; } } From 5bdd26a8deff5e53e779c5d0e47b0376d09add4f Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 8 Nov 2024 15:37:03 +0000 Subject: [PATCH 22/28] fix: android, fieldPath can be null for count. fix promise.reject --- ...tiveFirebaseFirestoreCollectionModule.java | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java index 99ff79e91d..727b37dc63 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java @@ -34,6 +34,7 @@ public class ReactNativeFirebaseFirestoreCollectionModule extends ReactNativeFirebaseModule { private static final String SERVICE_NAME = "FirestoreCollection"; + private final String TAG = "aaaaaaaa"; private static SparseArray collectionSnapshotListeners = new SparseArray<>(); @@ -224,10 +225,6 @@ public void aggregateQuery( String aggregateType = aggregateQuery.getString("aggregateType"); if (aggregateType == null) aggregateType = ""; String fieldPath = aggregateQuery.getString("field"); - if (fieldPath == null) { - promise.reject("firestore/invalid-argument", "fieldPath cannot be null"); - return; - } switch (aggregateType) { case "count": @@ -240,7 +237,8 @@ public void aggregateQuery( aggregateFields.add(AggregateField.average(fieldPath)); break; default: - promise.reject("firestore/invalid-argument", "Invalid AggregateType: " + aggregateType); + rejectPromiseWithCodeAndMessage( + promise, "firestore/invalid-argument", "Invalid AggregateType: " + aggregateType); return; } } @@ -262,13 +260,11 @@ public void aggregateQuery( String aggType = aggQuery.getString("aggregateType"); if (aggType == null) aggType = ""; String field = aggQuery.getString("field"); - if (field == null) { - promise.reject("firestore/invalid-argument", "field may not be null"); - return; - } String key = aggQuery.getString("key"); + if (key == null) { - promise.reject("firestore/invalid-argument", "key may not be null"); + rejectPromiseWithCodeAndMessage( + promise, "firestore/invalid-argument", "key may not be null"); return; } @@ -279,7 +275,8 @@ public void aggregateQuery( case "sum": Number sum = (Number) snapshot.get(sum(field)); if (sum == null) { - promise.reject("firestore/unknown", "sum unexpectedly null"); + rejectPromiseWithCodeAndMessage( + promise, "firestore/unknown", "sum unexpectedly null"); return; } result.putDouble(key, sum.doubleValue()); @@ -293,8 +290,10 @@ public void aggregateQuery( } break; default: - promise.reject( - "firestore/invalid-argument", "Invalid AggregateType: " + aggType); + rejectPromiseWithCodeAndMessage( + promise, + "firestore/invalid-argument", + "Invalid AggregateType: " + aggType); return; } } From 97124c9ff77b9e5b8d244b888433cd70a627b6ef Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 8 Nov 2024 15:38:47 +0000 Subject: [PATCH 23/28] chore: remove tag --- .../firestore/ReactNativeFirebaseFirestoreCollectionModule.java | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java index 727b37dc63..b44b87170f 100644 --- a/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java +++ b/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreCollectionModule.java @@ -34,7 +34,6 @@ public class ReactNativeFirebaseFirestoreCollectionModule extends ReactNativeFirebaseModule { private static final String SERVICE_NAME = "FirestoreCollection"; - private final String TAG = "aaaaaaaa"; private static SparseArray collectionSnapshotListeners = new SparseArray<>(); From eb65fd87cbf87c9f8c21d9a097dc503bbe830d4a Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 12 Nov 2024 15:05:11 +0000 Subject: [PATCH 24/28] test: edge cases for aggregate queries --- .../e2e/Aggregate/AggregateQuery.e2e.js | 349 ++++++++++++++++-- 1 file changed, 322 insertions(+), 27 deletions(-) diff --git a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js index 88cb0f3469..e88499b333 100644 --- a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js +++ b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js @@ -101,29 +101,6 @@ describe('getAggregateFromServer()', function () { }); describe('count(), average() & sum()', function () { - it('no existing collection responses for average(), sum() & count()', async function () { - const { getAggregateFromServer, collection, getFirestore, count, average, sum } = - firestoreModular; - const firestore = getFirestore(); - - const colRefNoDocs = collection(firestore, `${COLLECTION}/aggregate-count/no-docs`); - - const aggregateSpecNoDocuments = { - countCollection: count(), - averageBar: average('bar'), - sumBaz: sum('baz'), - }; - - const resultNoDocs = await getAggregateFromServer(colRefNoDocs, aggregateSpecNoDocuments); - - const dataNoDocs = resultNoDocs.data(); - - // average returns null, whilst sum and count return 0 - dataNoDocs.countCollection.should.eql(0); - should(dataNoDocs.averageBar).be.null(); - dataNoDocs.sumBaz.should.eql(0); - }); - it('single path using `string`', async function () { const { getAggregateFromServer, doc, setDoc, collection, getFirestore, count, average, sum } = firestoreModular; @@ -132,9 +109,9 @@ describe('getAggregateFromServer()', function () { const colRef = collection(firestore, `${COLLECTION}/aggregate-count/collection`); await Promise.all([ - setDoc(doc(colRef, 'one'), { bar: 0.4, baz: 0.1 }), - setDoc(doc(colRef, 'two'), { bar: 0.5, baz: 0.1 }), - setDoc(doc(colRef, 'three'), { bar: 0.6, baz: 0.1 }), + setDoc(doc(colRef, 'one'), { bar: 0.4, baz: 3 }), + setDoc(doc(colRef, 'two'), { bar: 0.5, baz: 3 }), + setDoc(doc(colRef, 'three'), { bar: 0.6, baz: 3 }), ]); const aggregateSpec = { @@ -150,7 +127,7 @@ describe('getAggregateFromServer()', function () { data.countCollection.should.eql(3); data.averageBar.should.eql(0.5); - data.sumBaz.should.eql(0.3); + data.sumBaz.should.eql(9); // should only return the aggregate field requests data.should.not.have.property('ignoreThisProperty'); }); @@ -268,5 +245,323 @@ describe('getAggregateFromServer()', function () { // should only return the aggregate field requests data.should.not.have.property('ignoreThisProperty'); }); + + describe('edge cases for aggregate query', function () { + it('no existing collection responses for average(), sum() & count()', async function () { + const { getAggregateFromServer, collection, getFirestore, count, average, sum } = + firestoreModular; + const firestore = getFirestore(); + + const colRefNoDocs = collection(firestore, `${COLLECTION}/aggregate-count/no-docs`); + + const aggregateSpecNoDocuments = { + countCollection: count(), + averageBar: average('bar'), + sumBaz: sum('baz'), + }; + + const resultNoDocs = await getAggregateFromServer(colRefNoDocs, aggregateSpecNoDocuments); + + const dataNoDocs = resultNoDocs.data(); + + // average returns null, whilst sum and count return 0 + dataNoDocs.countCollection.should.eql(0); + should(dataNoDocs.averageBar).be.null(); + dataNoDocs.sumBaz.should.eql(0); + }); + + it('sum of `0.3`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, sum } = + firestoreModular; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-count/sum-0-3`); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { bar: 0.4, baz: 0.1 }), + setDoc(doc(colRef, 'two'), { bar: 0.5, baz: 0.1 }), + setDoc(doc(colRef, 'three'), { bar: 0.6, baz: 0.1 }), + ]); + + const aggregateSpec = { + sumBaz: sum('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.sumBaz.should.eql(0.30000000000000004); + }); + + it('return JavaScript single max safe integer for `sum()`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, sum } = + firestoreModular; + const MAX_INT = Number.MAX_SAFE_INTEGER; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-count/max-int`); + + await Promise.all([setDoc(doc(colRef, 'one'), { baz: MAX_INT })]); + + const aggregateSpec = { + sumBaz: sum('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.sumBaz.should.eql(MAX_INT); + }); + + it('return JavaScript nine max safe integers for `sum()`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, sum } = + firestoreModular; + const MAX_INT = Number.MAX_SAFE_INTEGER; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-count/max-int-2`); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { baz: MAX_INT }), + setDoc(doc(colRef, 'two'), { baz: MAX_INT }), + setDoc(doc(colRef, 'three'), { baz: MAX_INT }), + setDoc(doc(colRef, 'four'), { baz: MAX_INT }), + setDoc(doc(colRef, 'five'), { baz: MAX_INT }), + setDoc(doc(colRef, 'six'), { baz: MAX_INT }), + setDoc(doc(colRef, 'seven'), { baz: MAX_INT }), + setDoc(doc(colRef, 'eight'), { baz: MAX_INT }), + setDoc(doc(colRef, 'nine'), { baz: MAX_INT }), + ]); + + const aggregateSpec = { + sumBaz: sum('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.sumBaz.should.eql(MAX_INT * 9); + }); + + it('return JavaScript single max safe number for `sum()`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, sum } = + firestoreModular; + const MAX_NUMBER = Number.MAX_VALUE; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-count/max-number`); + + await Promise.all([setDoc(doc(colRef, 'one'), { baz: MAX_NUMBER })]); + + const aggregateSpec = { + sumBaz: sum('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.sumBaz.should.eql(MAX_NUMBER); + }); + + it('returns `Infinity` for JavaScript max safe number + 1 for `sum()`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, sum } = + firestoreModular; + const MAX_NUMBER = Number.MAX_VALUE; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-count/max-number`); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { baz: MAX_NUMBER }), + setDoc(doc(colRef, 'two'), { baz: 1 }), + ]); + + const aggregateSpec = { + sumBaz: sum('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + // Doesn't add 1, just returns MAX_NUMBER + data.sumBaz.should.eql(MAX_NUMBER); + }); + + it('returns `Infinity` for JavaScript max safe number + 100 for `sum()`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, sum } = + firestoreModular; + const MAX_NUMBER = Number.MAX_VALUE; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-count/max-number-2`); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { baz: MAX_NUMBER }), + setDoc(doc(colRef, 'two'), { baz: 100 }), + ]); + + const aggregateSpec = { + sumBaz: sum('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + // Doesn't add 100, just returns MAX_NUMBER + data.sumBaz.should.eql(MAX_NUMBER); + }); + + it('returns `Infinity` for JavaScript two max safe numbers for `sum()`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, sum } = + firestoreModular; + const MAX_NUMBER = Number.MAX_VALUE; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-count/max-number-3`); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { baz: MAX_NUMBER }), + setDoc(doc(colRef, 'two'), { baz: MAX_NUMBER }), + ]); + + const aggregateSpec = { + sumBaz: sum('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + // Returns Infinity + data.sumBaz.should.eql(Infinity); + }); + + it('returns `0` for properties with `0` for `average()`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, average } = + firestoreModular; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-average/0-values`); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { baz: 0 }), + setDoc(doc(colRef, 'two'), { baz: 0 }), + ]); + + const aggregateSpec = { + averageBaz: average('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.averageBaz.should.eql(0); + }); + + it('returns `-1` for properties with `-1` for `average()`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, average } = + firestoreModular; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-average/minus-one-values`); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { baz: -1 }), + setDoc(doc(colRef, 'two'), { baz: -1 }), + ]); + + const aggregateSpec = { + averageBaz: average('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.averageBaz.should.eql(-1); + }); + + it('returns `-3` for properties with `-3` for `average()`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, average } = + firestoreModular; + const firestore = getFirestore(); + + const colRef = collection(firestore, `${COLLECTION}/aggregate-average/minus-three-values`); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { baz: -3 }), + setDoc(doc(colRef, 'two'), { baz: -3 }), + setDoc(doc(colRef, 'three'), { baz: -3 }), + ]); + + const aggregateSpec = { + averageBaz: average('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.averageBaz.should.eql(-3); + }); + + it('returns `-2` for properties with `-1`, `-2`,`-3` for `average()`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, average } = + firestoreModular; + const firestore = getFirestore(); + + const colRef = collection( + firestore, + `${COLLECTION}/aggregate-average/minus-various-values`, + ); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { baz: -1 }), + setDoc(doc(colRef, 'two'), { baz: -2 }), + setDoc(doc(colRef, 'three'), { baz: -3 }), + ]); + + const aggregateSpec = { + averageBaz: average('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.averageBaz.should.eql(-2); + }); + + it.only('returns `WHAT` for properties with `-1`, `-2`,`-3` for `average()`', async function () { + const { getAggregateFromServer, doc, setDoc, collection, getFirestore, average } = + firestoreModular; + const firestore = getFirestore(); + + const colRef = collection( + firestore, + `${COLLECTION}/aggregate-average/minus-various-float-values`, + ); + + await Promise.all([ + setDoc(doc(colRef, 'one'), { baz: -0.1 }), + setDoc(doc(colRef, 'two'), { baz: -0.2 }), + setDoc(doc(colRef, 'three'), { baz: -0.3 }), + ]); + + const aggregateSpec = { + averageBaz: average('baz'), + }; + + const result = await getAggregateFromServer(colRef, aggregateSpec); + + const data = result.data(); + + data.averageBaz.should.eql(-0.19999999999999998); + }); + }); }); }); From 01e2d945249095b0b216ac34e76da329c560139c Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 12 Nov 2024 15:11:03 +0000 Subject: [PATCH 25/28] chore: remove only() for test --- packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js index e88499b333..9aa6d2a115 100644 --- a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js +++ b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js @@ -536,7 +536,7 @@ describe('getAggregateFromServer()', function () { data.averageBaz.should.eql(-2); }); - it.only('returns `WHAT` for properties with `-1`, `-2`,`-3` for `average()`', async function () { + it('returns `WHAT` for properties with `-1`, `-2`,`-3` for `average()`', async function () { const { getAggregateFromServer, doc, setDoc, collection, getFirestore, average } = firestoreModular; const firestore = getFirestore(); From d18a67c639faec841b48e4a544ce25f5634ec2b1 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 12 Nov 2024 15:48:55 +0000 Subject: [PATCH 26/28] test: update what test produces --- packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js index 9aa6d2a115..a57a174650 100644 --- a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js +++ b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js @@ -536,7 +536,7 @@ describe('getAggregateFromServer()', function () { data.averageBaz.should.eql(-2); }); - it('returns `WHAT` for properties with `-1`, `-2`,`-3` for `average()`', async function () { + it('returns `-0.19999999999999998` for properties with `-1`, `-2`,`-3` for `average()`', async function () { const { getAggregateFromServer, doc, setDoc, collection, getFirestore, average } = firestoreModular; const firestore = getFirestore(); From a89ca19f1bb36ee347ec6520f6af401b3a4d89e6 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 13 Nov 2024 11:07:05 +0000 Subject: [PATCH 27/28] test: correct return type expected --- packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js index a57a174650..1d593ceb92 100644 --- a/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js +++ b/packages/firestore/e2e/Aggregate/AggregateQuery.e2e.js @@ -367,7 +367,7 @@ describe('getAggregateFromServer()', function () { data.sumBaz.should.eql(MAX_NUMBER); }); - it('returns `Infinity` for JavaScript max safe number + 1 for `sum()`', async function () { + it('returns `MAX_NUMBER` for JavaScript max safe number + 1 for `sum()`', async function () { const { getAggregateFromServer, doc, setDoc, collection, getFirestore, sum } = firestoreModular; const MAX_NUMBER = Number.MAX_VALUE; @@ -391,7 +391,7 @@ describe('getAggregateFromServer()', function () { data.sumBaz.should.eql(MAX_NUMBER); }); - it('returns `Infinity` for JavaScript max safe number + 100 for `sum()`', async function () { + it('returns `MAX_NUMBER` for JavaScript max safe number + 100 for `sum()`', async function () { const { getAggregateFromServer, doc, setDoc, collection, getFirestore, sum } = firestoreModular; const MAX_NUMBER = Number.MAX_VALUE; From cb54669f66683db81c6aa5d9a5e9fdaa48aaab07 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 13 Nov 2024 12:32:29 +0000 Subject: [PATCH 28/28] test: ensure aggregate fields are exposed to end user --- packages/firestore/__tests__/firestore.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/firestore/__tests__/firestore.test.ts b/packages/firestore/__tests__/firestore.test.ts index f6b019c1e7..29f661fe9b 100644 --- a/packages/firestore/__tests__/firestore.test.ts +++ b/packages/firestore/__tests__/firestore.test.ts @@ -5,6 +5,9 @@ import firestore, { Filter, getFirestore, getAggregateFromServer, + count, + average, + sum, addDoc, doc, collection, @@ -656,6 +659,18 @@ describe('Firestore', function () { it('`getAggregateFromServer` is properly exposed to end user', function () { expect(getAggregateFromServer).toBeDefined(); }); + + it('`count` is properly exposed to end user', function () { + expect(count).toBeDefined(); + }); + + it('`average` is properly exposed to end user', function () { + expect(average).toBeDefined(); + }); + + it('`sum` is properly exposed to end user', function () { + expect(sum).toBeDefined(); + }); }); describe('FirestorePersistentCacheIndexManager', function () {