sum avg (#11650)

MarkDuckworth · web-flow · commit 54d4d703b6ca · 2023-10-16T21:10:30.000-06:00
diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md
@@ -1,3 +1,6 @@
+# 10.17.0
+- [feature] Add support for sum and average aggregate queries.
+
 # 10.16.0
 - [fixed] Fixed an issue where Firestore's binary SwiftPM distribution would
   not link properly when building a target for testing. This issue affected
diff --git a/Firestore/Example/Tests/Integration/API/FIRAggregateTests.mm b/Firestore/Example/Tests/Integration/API/FIRAggregateTests.mm
diff --git a/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm b/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm
@@ -1508,14 +1508,14 @@ - (void)testMultipleInequalityInAggregateQuery {
                                      [FIRAggregateField aggregateFieldForSumOfField:@"sort"],
                                      [FIRAggregateField aggregateFieldForAverageOfField:@"v"],
                                    ]]];
-  XCTAssertEqual([snapshot valueForAggregation:[FIRAggregateField aggregateFieldForCount]],
+  XCTAssertEqual([snapshot valueForAggregateField:[FIRAggregateField aggregateFieldForCount]],
                  [NSNumber numberWithLong:3L]);
   XCTAssertEqual(
-      [[snapshot valueForAggregation:[FIRAggregateField aggregateFieldForSumOfField:@"sort"]]
+      [[snapshot valueForAggregateField:[FIRAggregateField aggregateFieldForSumOfField:@"sort"]]
           longValue],
       6L);
   XCTAssertEqual(
-      [snapshot valueForAggregation:[FIRAggregateField aggregateFieldForAverageOfField:@"v"]],
+      [snapshot valueForAggregateField:[FIRAggregateField aggregateFieldForAverageOfField:@"v"]],
       [NSNumber numberWithDouble:1.0]);
 }
 
diff --git a/Firestore/Source/API/FIRAggregateField.mm b/Firestore/Source/API/FIRAggregateField.mm
@@ -16,11 +16,11 @@
 
 #import "FIRAggregateField.h"
 
-#include <string>
-
 #import "Firestore/Source/API/FIRAggregateField+Internal.h"
 #import "Firestore/Source/API/FIRFieldPath+Internal.h"
 
+#import "Firestore/core/src/model/aggregate_field.h"
+
 using firebase::firestore::model::AggregateField;
 
 NS_ASSUME_NONNULL_BEGIN
diff --git a/Firestore/Source/API/FIRAggregateQuery+Internal.h b/Firestore/Source/API/FIRAggregateQuery+Internal.h
@@ -24,8 +24,8 @@ NS_ASSUME_NONNULL_BEGIN
 @interface FIRAggregateQuery (/* init */)
 
 - (instancetype)init NS_UNAVAILABLE;
-- (instancetype)initWithQueryAndAggregations:(FIRQuery *)query
-                                aggregations:(NSArray<FIRAggregateField *> *)aggregations
+- (instancetype)initWithQuery:(FIRQuery *)query
+              aggregateFields:(NSArray<FIRAggregateField *> *)aggregations
     NS_DESIGNATED_INITIALIZER;
 
 @end
diff --git a/Firestore/Source/API/FIRAggregateQuery.mm b/Firestore/Source/API/FIRAggregateQuery.mm
@@ -37,13 +37,13 @@ @implementation FIRAggregateQuery {
   std::unique_ptr<AggregateQuery> _aggregateQuery;
 }
 
-- (instancetype)initWithQueryAndAggregations:(FIRQuery *)query
-                                aggregations:(NSArray<FIRAggregateField *> *)aggregations {
+- (instancetype)initWithQuery:(FIRQuery *)query
+              aggregateFields:(NSArray<FIRAggregateField *> *)aggregateFields {
   if (self = [super init]) {
     _query = query;
 
     std::vector<AggregateField> _aggregateFields;
-    for (FIRAggregateField *field in aggregations) {
+    for (FIRAggregateField *field in aggregateFields) {
       _aggregateFields.push_back([field createInternalValue]);
     }
 
diff --git a/Firestore/Source/API/FIRAggregateQuerySnapshot+Internal.h b/Firestore/Source/API/FIRAggregateQuerySnapshot+Internal.h
@@ -35,26 +35,4 @@ NS_ASSUME_NONNULL_BEGIN
 
 @end
 
-// TODO(sum/avg) move the contents of this FuturePublicApi category to
-// ../Public/FirebaseFirestore/FIRAggregateQuerySnapshot.h
-@interface FIRAggregateQuerySnapshot (FuturePublicApi)
-
-/**
- * Gets the aggregation result for the specified aggregation without loss of precision. No coercion
- * of data types or values is performed.
- *
- * See the `AggregateField` class for the expected aggregration result values and types. Numeric
- * aggregation results will be boxed in an `NSNumber`.
- *
- * @param aggregation An instance of `AggregateField` that specifies which aggregation result to
- * return.
- * @return Returns the aggregation result from the server without loss of precision.
- * @warning Throws an `InvalidArgument` exception if the aggregation was not requested in the
- * `AggregateQuery`.
- * @see `AggregateField`
- */
-- (nullable id)valueForAggregation:(FIRAggregateField *)aggregation NS_SWIFT_NAME(get(_:));
-
-@end
-
 NS_ASSUME_NONNULL_END
diff --git a/Firestore/Source/API/FIRAggregateQuerySnapshot.mm b/Firestore/Source/API/FIRAggregateQuerySnapshot.mm
@@ -71,25 +71,25 @@ - (NSUInteger)hash {
 #pragma mark - Public Methods
 
 - (NSNumber *)count {
-  return (NSNumber *)[self valueForAggregation:[FIRAggregateField aggregateFieldForCount]];
+  return (NSNumber *)[self valueForAggregateField:[FIRAggregateField aggregateFieldForCount]];
 }
 
 - (FIRAggregateQuery *)query {
   return _query;
 }
 
-- (nullable id)valueForAggregation:(FIRAggregateField *)aggregation {
+- (id)valueForAggregateField:(FIRAggregateField *)aggregateField {
   FIRServerTimestampBehavior serverTimestampBehavior = FIRServerTimestampBehaviorNone;
-  AggregateAlias alias = [aggregation createAlias];
+  AggregateAlias alias = [aggregateField createAlias];
   absl::optional<google_firestore_v1_Value> fieldValue = _result.Get(alias.StringValue());
   if (!fieldValue) {
     std::string path{""};
-    if (aggregation.fieldPath) {
-      path = [aggregation.fieldPath internalValue].CanonicalString();
+    if (aggregateField.fieldPath) {
+      path = [aggregateField.fieldPath internalValue].CanonicalString();
     }
 
-    ThrowInvalidArgument("'%s(%s)' was not requested in the aggregation query.", [aggregation name],
-                         path);
+    ThrowInvalidArgument("'%s(%s)' was not requested in the aggregation query.",
+                         [aggregateField name], path);
   }
   FSTUserDataWriter *dataWriter =
       [[FSTUserDataWriter alloc] initWithFirestore:_query.query.firestore.wrapped
diff --git a/Firestore/Source/API/FIRQuery+Internal.h b/Firestore/Source/API/FIRQuery+Internal.h
@@ -47,27 +47,4 @@ NS_ASSUME_NONNULL_BEGIN
 
 @end
 
-// TODO(sum/avg) move the contents of this FuturePublicApi category to
-// ../Public/FirebaseFirestore/FIRAggregateQuerySnapshot.h
-@interface FIRQuery (FuturePublicApi)
-
-/**
- * Creates and returns a new `AggregateQuery` that aggregates the documents in the result set
- * of this query, without actually downloading the documents.
- *
- * Using an `AggregateQuery` to perform aggregations is efficient because only the final aggregation
- * values, not the documents' data, is downloaded. The query can even aggregate the documents if the
- * result set would be prohibitively large to download entirely (e.g. thousands of documents).
- *
- * @param aggregations Specifies the aggregation operations to perform on the result set of this
- * query.
- *
- * @return An `AggregateQuery` encapsulating this `Query` and `AggregateField`s, which can be used
- * to query the server for the aggregation results.
- */
-- (FIRAggregateQuery *)aggregate:(NSArray<FIRAggregateField *> *)aggregations
-    NS_SWIFT_NAME(aggregate(_:));
-
-@end
-
 NS_ASSUME_NONNULL_END
diff --git a/Firestore/Source/API/FIRQuery.mm b/Firestore/Source/API/FIRQuery.mm
@@ -485,11 +485,11 @@ - (FIRQuery *)queryEndingAtValues:(NSArray *)fieldValues {
 
 - (FIRAggregateQuery *)count {
   FIRAggregateField *countAF = [FIRAggregateField aggregateFieldForCount];
-  return [[FIRAggregateQuery alloc] initWithQueryAndAggregations:self aggregations:@[ countAF ]];
+  return [[FIRAggregateQuery alloc] initWithQuery:self aggregateFields:@[ countAF ]];
 }
 
-- (FIRAggregateQuery *)aggregate:(NSArray<FIRAggregateField *> *)aggregations {
-  return [[FIRAggregateQuery alloc] initWithQueryAndAggregations:self aggregations:aggregations];
+- (FIRAggregateQuery *)aggregate:(NSArray<FIRAggregateField *> *)aggregateFields {
+  return [[FIRAggregateQuery alloc] initWithQuery:self aggregateFields:aggregateFields];
 }
 
 #pragma mark - Private Methods
diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRAggregateField.h b/Firestore/Source/Public/FirebaseFirestore/FIRAggregateField.h
@@ -20,8 +20,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 @class FIRFieldPath;
 
-// TODO(sum/avg) move this entire file to ../Public/FirebaseFirestore when the API can be public
-
 /**
  * Represents an aggregation that can be performed by Firestore.
  */
@@ -84,7 +82,7 @@ NS_SWIFT_NAME(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.
  *
- * The result of an average operation will always be a 64-bit integer value, a double, or NaN.
+ * The result of an average operation will always be a double or NaN.
  *
  * - Averaging over zero documents or fields will result in a double value representing NaN.
  * - Averaging over NaN will result in a double value representing NaN.
@@ -99,7 +97,7 @@ NS_SWIFT_NAME(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.
  *
- * The result of an average operation will always be a 64-bit integer value, a double, or NaN.
+ * The result of an average operation will always be a double or NaN.
  *
  * - Averaging over zero documents or fields will result in a double value representing NaN.
  * - Averaging over NaN will result in a double value representing NaN.
diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRAggregateQuerySnapshot.h b/Firestore/Source/Public/FirebaseFirestore/FIRAggregateQuerySnapshot.h
@@ -19,6 +19,7 @@
 NS_ASSUME_NONNULL_BEGIN
 
 @class FIRAggregateQuery;
+@class FIRAggregateField;
 
 /**
  * The results of executing an `AggregateQuery`.
@@ -36,6 +37,22 @@ NS_SWIFT_NAME(AggregateQuerySnapshot)
 /** The number of documents in the result set of the underlying query. */
 @property(nonatomic, readonly) NSNumber* count;
 
+/**
+ * Gets the aggregate result for the specified aggregate field without loss of precision. No
+ * coercion of data types or values is performed.
+ *
+ * See the `AggregateField` class for the expected aggregate result values and types. Numeric
+ * aggregate results will be boxed in an `NSNumber`.
+ *
+ * @param aggregateField An instance of `AggregateField` that specifies which aggregate result to
+ * return.
+ * @return Returns the aggregate result from the server without loss of precision.
+ * @warning Throws an `InvalidArgument` exception if the aggregate field was not requested in the
+ * `AggregateQuery`.
+ * @see `AggregateField`
+ */
+- (id)valueForAggregateField:(FIRAggregateField*)aggregateField NS_SWIFT_NAME(get(_:));
+
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRQuery.h b/Firestore/Source/Public/FirebaseFirestore/FIRQuery.h
@@ -20,6 +20,7 @@
 #import "FIRListenerRegistration.h"
 
 @class FIRAggregateQuery;
+@class FIRAggregateField;
 @class FIRFieldPath;
 @class FIRFirestore;
 @class FIRFilter;
@@ -559,11 +560,28 @@ NS_SWIFT_NAME(Query)
  * the documents.
  *
  * Using this `AggregateQuery` to count the documents is efficient because only the final count,
- * not the documents' data, is downloaded. The query can even count the documents if the result
- * set would be prohibitively large to download entirely (e.g. thousands of documents).
+ * not the documents' data, is downloaded. This allows for counting document collections that would
+ * otherwise be too large to download (e.g. containing thousands of documents).
  */
 @property(nonatomic, readonly) FIRAggregateQuery *count;
 
+/**
+ * Creates and returns a new `AggregateQuery` that aggregates the documents in the result set
+ * of this query, without actually downloading the documents.
+ *
+ * Using an `AggregateQuery` to perform aggregations is efficient because only the final aggregation
+ * values, not the documents' data, is downloaded. This allows for aggregating document collections
+ * that would otherwise be too large to download (e.g. containing thousands of documents).
+ *
+ * @param aggregateFields Specifies the aggregate operations to perform on the result set of this
+ * query.
+ *
+ * @return An `AggregateQuery` encapsulating this `Query` and `AggregateField`s, which can be used
+ * to query the server for the aggregation results.
+ */
+- (FIRAggregateQuery *)aggregate:(NSArray<FIRAggregateField *> *)aggregateFields
+    NS_SWIFT_NAME(aggregate(_:));
+
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/Firestore/Source/Public/FirebaseFirestore/FirebaseFirestore.h b/Firestore/Source/Public/FirebaseFirestore/FirebaseFirestore.h
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#import "FIRAggregateField.h"
 #import "FIRAggregateQuery.h"
 #import "FIRAggregateQuerySnapshot.h"
 #import "FIRAggregateSource.h"
diff --git a/Firestore/Swift/Tests/Integration/AggregationIntegrationTests.swift b/Firestore/Swift/Tests/Integration/AggregationIntegrationTests.swift
diff --git a/Firestore/core/src/api/query_core.cc b/Firestore/core/src/api/query_core.cc
diff --git a/Firestore/core/src/api/query_core.h b/Firestore/core/src/api/query_core.h
diff --git a/Firestore/core/src/remote/remote_objc_bridge.cc b/Firestore/core/src/remote/remote_objc_bridge.cc

Original file line number	Diff line number	Diff line change
`@@ -37,13 +37,13 @@ @implementation FIRAggregateQuery {`
`37`	`37`	`std::unique_ptr<AggregateQuery> _aggregateQuery;`
`38`	`38`	`}`
`39`	`39`
`40`		`-- (instancetype)initWithQueryAndAggregations:(FIRQuery *)query`
`41`		`- aggregations:(NSArray<FIRAggregateField > )aggregations {`
	`40`	`+- (instancetype)initWithQuery:(FIRQuery *)query`
	`41`	`+ aggregateFields:(NSArray<FIRAggregateField > )aggregateFields {`
`42`	`42`	`if (self = [super init]) {`
`43`	`43`	`_query = query;`
`44`	`44`
`45`	`45`	`std::vector<AggregateField> _aggregateFields;`
`46`		`- for (FIRAggregateField *field in aggregations) {`
	`46`	`+ for (FIRAggregateField *field in aggregateFields) {`
`47`	`47`	`_aggregateFields.push_back([field createInternalValue]);`
`48`	`48`	`}`
`49`	`49`
Original file line number	Diff line number	Diff line change
`@@ -485,11 +485,11 @@ - (FIRQuery )queryEndingAtValues:(NSArray )fieldValues {`
`485`	`485`
`486`	`486`	`- (FIRAggregateQuery *)count {`
`487`	`487`	`FIRAggregateField *countAF = [FIRAggregateField aggregateFieldForCount];`
`488`		`- return [[FIRAggregateQuery alloc] initWithQueryAndAggregations:self aggregations:@[ countAF ]];`
	`488`	`+ return [[FIRAggregateQuery alloc] initWithQuery:self aggregateFields:@[ countAF ]];`
`489`	`489`	`}`
`490`	`490`
`491`		`-- (FIRAggregateQuery )aggregate:(NSArray<FIRAggregateField > *)aggregations {`
`492`		`- return [[FIRAggregateQuery alloc] initWithQueryAndAggregations:self aggregations:aggregations];`
	`491`	`+- (FIRAggregateQuery )aggregate:(NSArray<FIRAggregateField > *)aggregateFields {`
	`492`	`+ return [[FIRAggregateQuery alloc] initWithQuery:self aggregateFields:aggregateFields];`
`493`	`493`	`}`
`494`	`494`
`495`	`495`	`#pragma mark - Private Methods`