diff --git a/Firestore/Source/API/FIRPipelineBridge.mm b/Firestore/Source/API/FIRPipelineBridge.mm index ac3091249e0..ef09a86e1da 100644 --- a/Firestore/Source/API/FIRPipelineBridge.mm +++ b/Firestore/Source/API/FIRPipelineBridge.mm @@ -20,6 +20,7 @@ #include +#import "Firestore/Source/API/FIRCollectionReference+Internal.h" #import "Firestore/Source/API/FIRDocumentReference+Internal.h" #import "Firestore/Source/API/FIRFieldPath+Internal.h" #import "Firestore/Source/API/FIRFirestore+Internal.h" @@ -39,6 +40,7 @@ #include "Firestore/core/src/api/pipeline_result.h" #include "Firestore/core/src/api/pipeline_snapshot.h" #include "Firestore/core/src/api/stages.h" +#include "Firestore/core/src/util/comparison.h" #include "Firestore/core/src/util/error_apple.h" #include "Firestore/core/src/util/status.h" #include "Firestore/core/src/util/string_apple.h" @@ -57,12 +59,12 @@ using firebase::firestore::api::Field; using firebase::firestore::api::FindNearestStage; using firebase::firestore::api::FunctionExpr; -using firebase::firestore::api::GenericStage; using firebase::firestore::api::LimitStage; using firebase::firestore::api::MakeFIRTimestamp; using firebase::firestore::api::OffsetStage; using firebase::firestore::api::Ordering; using firebase::firestore::api::Pipeline; +using firebase::firestore::api::RawStage; using firebase::firestore::api::RemoveFieldsStage; using firebase::firestore::api::ReplaceWith; using firebase::firestore::api::Sample; @@ -73,6 +75,7 @@ using firebase::firestore::api::Where; using firebase::firestore::model::FieldPath; using firebase::firestore::nanopb::SharedMessage; +using firebase::firestore::util::ComparisonResult; using firebase::firestore::util::MakeCallback; using firebase::firestore::util::MakeNSString; using firebase::firestore::util::MakeString; @@ -80,6 +83,13 @@ NS_ASSUME_NONNULL_BEGIN +inline std::string EnsureLeadingSlash(const std::string &path) { + if (!path.empty() && path[0] == '/') { + return path; + } + return "/" + path; +} + @implementation FIRExprBridge @end @@ -216,10 +226,19 @@ @implementation FIRCollectionSourceStageBridge { std::shared_ptr collection_source; } -- (id)initWithPath:(NSString *)path { +- (id)initWithRef:(FIRCollectionReference *)ref firestore:(FIRFirestore *)db { self = [super init]; if (self) { - collection_source = std::make_shared(MakeString(path)); + if (ref.firestore.databaseID.CompareTo(db.databaseID) != ComparisonResult::Same) { + ThrowInvalidArgument( + "Invalid CollectionReference. The project ID (\"%s\") or the database (\"%s\") does not " + "match " + "the project ID (\"%s\") and database (\"%s\") of the target database of this Pipeline.", + ref.firestore.databaseID.project_id(), ref.firestore.databaseID.database_id(), + db.databaseID.project_id(), db.databaseID.project_id()); + } + collection_source = + std::make_shared(EnsureLeadingSlash(MakeString(ref.path))); } return self; } @@ -270,12 +289,21 @@ @implementation FIRDocumentsSourceStageBridge { std::shared_ptr cpp_document_source; } -- (id)initWithDocuments:(NSArray *)documents { +- (id)initWithDocuments:(NSArray *)documents firestore:(FIRFirestore *)db { self = [super init]; if (self) { std::vector cpp_documents; - for (NSString *doc in documents) { - cpp_documents.push_back(MakeString(doc)); + for (FIRDocumentReference *doc in documents) { + if (doc.firestore.databaseID.CompareTo(db.databaseID) != ComparisonResult::Same) { + ThrowInvalidArgument("Invalid DocumentReference. The project ID (\"%s\") or the database " + "(\"%s\") does not match " + "the project ID (\"%s\") and database (\"%s\") of the target database " + "of this Pipeline.", + doc.firestore.databaseID.project_id(), + doc.firestore.databaseID.database_id(), db.databaseID.project_id(), + db.databaseID.project_id()); + } + cpp_documents.push_back(EnsureLeadingSlash(MakeString(doc.path))); } cpp_document_source = std::make_shared(std::move(cpp_documents)); } @@ -754,12 +782,12 @@ - (id)initWithField:(FIRExprBridge *)field indexField:(NSString *_Nullable)index @end -@implementation FIRGenericStageBridge { +@implementation FIRRawStageBridge { NSString *_name; NSArray *_params; NSDictionary *_Nullable _options; Boolean isUserDataRead; - std::shared_ptr cpp_generic_stage; + std::shared_ptr cpp_generic_stage; } - (id)initWithName:(NSString *)name @@ -787,8 +815,8 @@ - (id)initWithName:(NSString *)name cpp_options[MakeString(key)] = [_options[key] cppExprWithReader:reader]; } } - cpp_generic_stage = std::make_shared(MakeString(_name), std::move(cpp_params), - std::move(cpp_options)); + cpp_generic_stage = std::make_shared(MakeString(_name), std::move(cpp_params), + std::move(cpp_options)); } isUserDataRead = YES; diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRPipelineBridge.h b/Firestore/Source/Public/FirebaseFirestore/FIRPipelineBridge.h index 7b8ebf80e9b..58cf3237194 100644 --- a/Firestore/Source/Public/FirebaseFirestore/FIRPipelineBridge.h +++ b/Firestore/Source/Public/FirebaseFirestore/FIRPipelineBridge.h @@ -70,7 +70,7 @@ NS_SWIFT_SENDABLE NS_SWIFT_NAME(CollectionSourceStageBridge) @interface FIRCollectionSourceStageBridge : FIRStageBridge -- (id)initWithPath:(NSString *)path; +- (id)initWithRef:(FIRCollectionReference *)ref firestore:(FIRFirestore *)db; @end @@ -94,7 +94,7 @@ NS_SWIFT_SENDABLE NS_SWIFT_NAME(DocumentsSourceStageBridge) @interface FIRDocumentsSourceStageBridge : FIRStageBridge -- (id)initWithDocuments:(NSArray *)documents; +- (id)initWithDocuments:(NSArray *)documents firestore:(FIRFirestore *)db; @end @@ -195,8 +195,8 @@ NS_SWIFT_NAME(UnnestStageBridge) @end NS_SWIFT_SENDABLE -NS_SWIFT_NAME(GenericStageBridge) -@interface FIRGenericStageBridge : FIRStageBridge +NS_SWIFT_NAME(RawStageBridge) +@interface FIRRawStageBridge : FIRStageBridge - (id)initWithName:(NSString *)name params:(NSArray *)params options:(NSDictionary *_Nullable)options; diff --git a/Firestore/Swift/Source/SwiftAPI/Pipeline/Pipeline.swift b/Firestore/Swift/Source/SwiftAPI/Pipeline/Pipeline.swift index 4e49c97301b..cb839239994 100644 --- a/Firestore/Swift/Source/SwiftAPI/Pipeline/Pipeline.swift +++ b/Firestore/Swift/Source/SwiftAPI/Pipeline/Pipeline.swift @@ -699,7 +699,7 @@ public struct Pipeline: @unchecked Sendable { /// ```swift /// // let pipeline: Pipeline = ... /// // Example: Assuming a hypothetical backend stage "customFilterV2". - /// let genericPipeline = pipeline.genericStage( + /// let genericPipeline = pipeline.rawStage( /// name: "customFilterV2", /// params: [Field("userScore"), 80], // Ordered parameters. /// options: ["mode": "strict", "logLevel": 2] // Optional named parameters. @@ -712,10 +712,10 @@ public struct Pipeline: @unchecked Sendable { /// - params: An array of ordered, `Sendable` parameters for the stage. /// - options: Optional dictionary of named, `Sendable` parameters. /// - Returns: A new `Pipeline` object with this stage appended. - public func genericStage(name: String, params: [Sendable], - options: [String: Sendable]? = nil) -> Pipeline { + public func rawStage(name: String, params: [Sendable], + options: [String: Sendable]? = nil) -> Pipeline { return Pipeline( - stages: stages + [GenericStage(name: name, params: params, options: options)], + stages: stages + [RawStage(name: name, params: params, options: options)], db: db ) } diff --git a/Firestore/Swift/Source/SwiftAPI/Pipeline/PipelineSnapshot.swift b/Firestore/Swift/Source/SwiftAPI/Pipeline/PipelineSnapshot.swift index e25191b8ad2..a260cc55cee 100644 --- a/Firestore/Swift/Source/SwiftAPI/Pipeline/PipelineSnapshot.swift +++ b/Firestore/Swift/Source/SwiftAPI/Pipeline/PipelineSnapshot.swift @@ -25,7 +25,7 @@ public struct PipelineSnapshot: Sendable { public let pipeline: Pipeline /// An array of all the results in the `PipelineSnapshot`. - let results_cache: [PipelineResult] + public let results: [PipelineResult] /// The time at which the pipeline producing this result was executed. public let executionTime: Timestamp @@ -36,10 +36,6 @@ public struct PipelineSnapshot: Sendable { self.bridge = bridge self.pipeline = pipeline executionTime = self.bridge.execution_time - results_cache = self.bridge.results.map { PipelineResult($0) } - } - - public func results() -> [PipelineResult] { - return results_cache + results = self.bridge.results.map { PipelineResult($0) } } } diff --git a/Firestore/Swift/Source/SwiftAPI/Pipeline/PipelineSource.swift b/Firestore/Swift/Source/SwiftAPI/Pipeline/PipelineSource.swift index da0b5b5b1b4..6a0026340a2 100644 --- a/Firestore/Swift/Source/SwiftAPI/Pipeline/PipelineSource.swift +++ b/Firestore/Swift/Source/SwiftAPI/Pipeline/PipelineSource.swift @@ -21,8 +21,12 @@ public struct PipelineSource: @unchecked Sendable { } public func collection(_ path: String) -> Pipeline { - let normalizedPath = path.hasPrefix("/") ? path : "/" + path - return Pipeline(stages: [CollectionSource(collection: normalizedPath)], db: db) + return Pipeline(stages: [CollectionSource(collection: db.collection(path), db: db)], db: db) + } + + public func collection(_ ref: CollectionReference) -> Pipeline { + let collectionStage = CollectionSource(collection: ref, db: db) + return Pipeline(stages: [collectionStage], db: db) } public func collectionGroup(_ collectionId: String) -> Pipeline { @@ -37,13 +41,13 @@ public struct PipelineSource: @unchecked Sendable { } public func documents(_ docs: [DocumentReference]) -> Pipeline { - let paths = docs.map { $0.path.hasPrefix("/") ? $0.path : "/" + $0.path } - return Pipeline(stages: [DocumentsSource(paths: paths)], db: db) + return Pipeline(stages: [DocumentsSource(docs: docs, db: db)], db: db) } public func documents(_ paths: [String]) -> Pipeline { - let normalizedPaths = paths.map { $0.hasPrefix("/") ? $0 : "/" + $0 } - return Pipeline(stages: [DocumentsSource(paths: normalizedPaths)], db: db) + let docs = paths.map { db.document($0) } + let documentsStage = DocumentsSource(docs: docs, db: db) + return Pipeline(stages: [documentsStage], db: db) } public func create(from query: Query) -> Pipeline { diff --git a/Firestore/Swift/Source/SwiftAPI/Stages.swift b/Firestore/Swift/Source/SwiftAPI/Stages.swift index 65796af8471..13079a3148b 100644 --- a/Firestore/Swift/Source/SwiftAPI/Stages.swift +++ b/Firestore/Swift/Source/SwiftAPI/Stages.swift @@ -33,11 +33,13 @@ class CollectionSource: Stage { let name: String = "collection" let bridge: StageBridge - private var collection: String + private var collection: CollectionReference + private let db: Firestore - init(collection: String) { + init(collection: CollectionReference, db: Firestore) { self.collection = collection - bridge = CollectionSourceStageBridge(path: collection) + self.db = db + bridge = CollectionSourceStageBridge(ref: collection, firestore: db) } } @@ -70,12 +72,14 @@ class DatabaseSource: Stage { class DocumentsSource: Stage { let name: String = "documents" let bridge: StageBridge - private var references: [String] + private var docs: [DocumentReference] + private let db: Firestore // Initialize with an array of String paths - init(paths: [String]) { - references = paths - bridge = DocumentsSourceStageBridge(documents: paths) + init(docs: [DocumentReference], db: Firestore) { + self.docs = docs + self.db = db + bridge = DocumentsSourceStageBridge(documents: docs, firestore: db) } } @@ -337,7 +341,7 @@ class Unnest: Stage { } @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) -class GenericStage: Stage { +class RawStage: Stage { let name: String let bridge: StageBridge private var params: [Sendable] @@ -349,6 +353,6 @@ class GenericStage: Stage { self.options = options let bridgeParams = params.map { Helper.sendableToExpr($0).toBridge() } let bridgeOptions = options?.mapValues { Helper.sendableToExpr($0).toBridge() } - bridge = GenericStageBridge(name: name, params: bridgeParams, options: bridgeOptions) + bridge = RawStageBridge(name: name, params: bridgeParams, options: bridgeOptions) } } diff --git a/Firestore/Swift/Tests/Integration/PipelineApiTests.swift b/Firestore/Swift/Tests/Integration/PipelineApiTests.swift index 25ca1f09a6d..f712cceca1f 100644 --- a/Firestore/Swift/Tests/Integration/PipelineApiTests.swift +++ b/Firestore/Swift/Tests/Integration/PipelineApiTests.swift @@ -263,22 +263,22 @@ final class PipelineTests: FSTIntegrationTestCase { // ... } } - func testGenericStage() async throws { + func testRawStage() async throws { // Assume we don't have a built-in "where" stage, the customer could still - // add this stage by calling genericStage, passing the name of the stage "where", + // add this stage by calling rawStage, passing the name of the stage "where", // and providing positional argument values. _ = db.pipeline().collection("books") - .genericStage(name: "where", - params: [Field("published").lt(1900)]) + .rawStage(name: "where", + params: [Field("published").lt(1900)]) .select("title", "author") // In cases where the stage also supports named argument values, then these can be // provided with a third argument that maps the argument name to value. // Note that these named arguments are always optional in the stage definition. _ = db.pipeline().collection("books") - .genericStage(name: "where", - params: [Field("published").lt(1900)], - options: ["someOptionalParamName": "the argument value for this param"]) + .rawStage(name: "where", + params: [Field("published").lt(1900)], + options: ["someOptionalParamName": "the argument value for this param"]) .select("title", "author") } diff --git a/Firestore/Swift/Tests/Integration/PipelineTests.swift b/Firestore/Swift/Tests/Integration/PipelineTests.swift index 7bfdc8525a1..9816716a1e9 100644 --- a/Firestore/Swift/Tests/Integration/PipelineTests.swift +++ b/Firestore/Swift/Tests/Integration/PipelineTests.swift @@ -14,8 +14,10 @@ * limitations under the License. */ +import FirebaseCore // For FirebaseApp management import FirebaseFirestore import Foundation +import XCTest // For XCTFail, XCTAssertEqual etc. private let bookDocs: [String: [String: Any]] = [ "book1": [ @@ -24,9 +26,10 @@ private let bookDocs: [String: [String: Any]] = [ "genre": "Science Fiction", "published": 1979, "rating": 4.2, - "tags": ["comedy", "space", "adventure"], // Array literal - "awards": ["hugo": true, "nebula": false], // Dictionary literal - "nestedField": ["level.1": ["level.2": true]], // Nested dictionary literal + "tags": ["comedy", "space", "adventure"], + "awards": ["hugo": true, "nebula": false, "others": ["unknown": ["year": 1980]]], // Corrected + "nestedField": ["level.1": ["level.2": true]], + "embedding": VectorValue([10, 1, 1, 1, 1, 1, 1, 1, 1, 1]), ], "book2": [ "title": "Pride and Prejudice", @@ -36,6 +39,7 @@ private let bookDocs: [String: [String: Any]] = [ "rating": 4.5, "tags": ["classic", "social commentary", "love"], "awards": ["none": true], + "embedding": VectorValue([1, 10, 1, 1, 1, 1, 1, 1, 1, 1]), // Added ], "book3": [ "title": "One Hundred Years of Solitude", @@ -45,6 +49,7 @@ private let bookDocs: [String: [String: Any]] = [ "rating": 4.3, "tags": ["family", "history", "fantasy"], "awards": ["nobel": true, "nebula": false], + "embedding": VectorValue([1, 1, 10, 1, 1, 1, 1, 1, 1, 1]), ], "book4": [ "title": "The Lord of the Rings", @@ -54,6 +59,9 @@ private let bookDocs: [String: [String: Any]] = [ "rating": 4.7, "tags": ["adventure", "magic", "epic"], "awards": ["hugo": false, "nebula": false], + "remarks": NSNull(), // Added + "cost": Double.nan, // Added + "embedding": VectorValue([1, 1, 1, 10, 1, 1, 1, 1, 1, 1]), // Added ], "book5": [ "title": "The Handmaid's Tale", @@ -63,6 +71,7 @@ private let bookDocs: [String: [String: Any]] = [ "rating": 4.1, "tags": ["feminism", "totalitarianism", "resistance"], "awards": ["arthur c. clarke": true, "booker prize": false], + "embedding": VectorValue([1, 1, 1, 1, 10, 1, 1, 1, 1, 1]), // Added ], "book6": [ "title": "Crime and Punishment", @@ -72,6 +81,7 @@ private let bookDocs: [String: [String: Any]] = [ "rating": 4.3, "tags": ["philosophy", "crime", "redemption"], "awards": ["none": true], + "embedding": VectorValue([1, 1, 1, 1, 1, 10, 1, 1, 1, 1]), // Added ], "book7": [ "title": "To Kill a Mockingbird", @@ -81,6 +91,7 @@ private let bookDocs: [String: [String: Any]] = [ "rating": 4.2, "tags": ["racism", "injustice", "coming-of-age"], "awards": ["pulitzer": true], + "embedding": VectorValue([1, 1, 1, 1, 1, 1, 10, 1, 1, 1]), // Added ], "book8": [ "title": "1984", @@ -90,6 +101,7 @@ private let bookDocs: [String: [String: Any]] = [ "rating": 4.2, "tags": ["surveillance", "totalitarianism", "propaganda"], "awards": ["prometheus": true], + "embedding": VectorValue([1, 1, 1, 1, 1, 1, 1, 10, 1, 1]), // Added ], "book9": [ "title": "The Great Gatsby", @@ -99,6 +111,7 @@ private let bookDocs: [String: [String: Any]] = [ "rating": 4.0, "tags": ["wealth", "american dream", "love"], "awards": ["none": true], + "embedding": VectorValue([1, 1, 1, 1, 1, 1, 1, 1, 10, 1]), // Added ], "book10": [ "title": "Dune", @@ -108,9 +121,46 @@ private let bookDocs: [String: [String: Any]] = [ "rating": 4.6, "tags": ["politics", "desert", "ecology"], "awards": ["hugo": true, "nebula": true], + "embedding": VectorValue([1, 1, 1, 1, 1, 1, 1, 1, 1, 10]), // Added ], ] +func expectResults(_ snapshot: PipelineSnapshot, + expectedCount: Int, + file: StaticString = #file, + line: UInt = #line) { + XCTAssertEqual( + snapshot.results.count, + expectedCount, + "Snapshot results count mismatch", + file: file, + line: line + ) +} + +func expectResults(_ snapshot: PipelineSnapshot, + expectedIDs: [String], + file: StaticString = #file, + line: UInt = #line) { + let results = snapshot.results + XCTAssertEqual( + results.count, + expectedIDs.count, + "Snapshot document IDs count mismatch. Expected \(expectedIDs.count), got \(results.count). Actual IDs: \(results.map { $0.id })", + file: file, + line: line + ) + + let actualIDs = results.map { $0.id! }.sorted() + XCTAssertEqual( + actualIDs, + expectedIDs.sorted(), + "Snapshot document IDs mismatch. Expected (sorted): \(expectedIDs.sorted()), got (sorted): \(actualIDs)", + file: file, + line: line + ) +} + @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) class PipelineIntegrationTests: FSTIntegrationTestCase { override func setUp() { @@ -118,17 +168,6 @@ class PipelineIntegrationTests: FSTIntegrationTestCase { super.setUp() } - func testCount() async throws { - try await firestore().collection("foo").document("bar").setData(["foo": "bar", "x": 42]) - let snapshot = try await firestore() - .pipeline() - .collection("/foo") - .where(Field("foo").eq(Constant("bar"))) - .execute() - - print(snapshot) - } - func testEmptyResults() async throws { let collRef = collectionRef( withDocuments: bookDocs @@ -141,7 +180,7 @@ class PipelineIntegrationTests: FSTIntegrationTestCase { .limit(0) .execute() - XCTAssertTrue(snapshot.results().isEmpty) + expectResults(snapshot, expectedCount: 0) } func testFullResults() async throws { @@ -155,14 +194,299 @@ class PipelineIntegrationTests: FSTIntegrationTestCase { .collection(collRef.path) .execute() - let results = snapshot.results() - XCTAssertEqual(results.count, 10) + // expectResults(snapshot, expectedCount: 10) // This is implicitly checked by expectedIDs + // version + expectResults( + snapshot, + expectedIDs: [ + "book1", "book10", "book2", "book3", "book4", + "book5", "book6", "book7", "book8", "book9", + ] + ) + } + + func testReturnsExecutionTime() async throws { + let collRef = collectionRef(withDocuments: bookDocs) + let db = collRef.firestore + + let pipeline = db.pipeline().collection(collRef.path) + let snapshot = try await pipeline.execute() + + XCTAssertEqual(snapshot.results.count, bookDocs.count, "Should fetch all documents") + + let executionTimeValue = snapshot.executionTime.dateValue().timeIntervalSince1970 + + XCTAssertGreaterThan(executionTimeValue, 0, "Execution time should be positive and not zero") + } + + func testReturnsExecutionTimeForEmptyQuery() async throws { + let collRef = + collectionRef(withDocuments: bookDocs) // Using bookDocs is fine, limit(0) makes it empty + let db = collRef.firestore + + let pipeline = db.pipeline().collection(collRef.path).limit(0) + let snapshot = try await pipeline.execute() + + expectResults(snapshot, expectedCount: 0) + + let executionTimeValue = snapshot.executionTime.dateValue().timeIntervalSince1970 + XCTAssertGreaterThan(executionTimeValue, 0, "Execution time should be positive and not zero") + } + + func testReturnsCreateAndUpdateTimeForEachDocument() async throws { + let collRef = collectionRef(withDocuments: bookDocs) + let db = collRef.firestore + let pipeline = db.pipeline().collection(collRef.path) + var snapshot = try await pipeline.execute() + + XCTAssertEqual( + snapshot.results.count, + bookDocs.count, + "Initial fetch should return all documents" + ) + for doc in snapshot.results { + XCTAssertNotNil( + doc.createTime, + "Document \(String(describing: doc.id)) should have createTime" + ) + XCTAssertNotNil( + doc.updateTime, + "Document \(String(describing: doc.id)) should have updateTime" + ) + if let createTime = doc.createTime, let updateTime = doc.updateTime { + let createTimestamp = createTime.dateValue().timeIntervalSince1970 + let updateTimestamp = updateTime.dateValue().timeIntervalSince1970 + + XCTAssertEqual(createTimestamp, + updateTimestamp, + "Initial createTime and updateTime should be equal for \(String(describing: doc.id))") + } + } + + // Update documents + let batch = db.batch() + for doc in snapshot.results { + batch + .updateData( + ["newField": "value"], + forDocument: doc.ref! + ) + } + + try await batch.commit() + + snapshot = try await pipeline.execute() + XCTAssertEqual( + snapshot.results.count, + bookDocs.count, + "Fetch after update should return all documents" + ) + + for doc in snapshot.results { + XCTAssertNotNil( + doc.createTime, + "Document \(String(describing: doc.id)) should still have createTime after update" + ) + XCTAssertNotNil( + doc.updateTime, + "Document \(String(describing: doc.id)) should still have updateTime after update" + ) + if let createTime = doc.createTime, let updateTime = doc.updateTime { + let createTimestamp = createTime.dateValue().timeIntervalSince1970 + let updateTimestamp = updateTime.dateValue().timeIntervalSince1970 + + XCTAssertLessThan(createTimestamp, + updateTimestamp, + "updateTime (\(updateTimestamp)) should be after createTime (\(createTimestamp)) for \(String(describing: doc.id))") + } + } + } + + func testReturnsExecutionTimeForAggregateQuery() async throws { + let collRef = collectionRef(withDocuments: bookDocs) + let db = collRef.firestore + + let pipeline = db.pipeline() + .collection(collRef.path) + .aggregate(Field("rating").avg().as("avgRating")) + let snapshot = try await pipeline.execute() + + XCTAssertEqual(snapshot.results.count, 1, "Aggregate query should return a single result") + + let executionTimeValue = snapshot.executionTime.dateValue().timeIntervalSince1970 + XCTAssertGreaterThan(executionTimeValue, 0, "Execution time should be positive") + } + + func testTimestampsAreNilForAggregateQueryResults() async throws { + let collRef = collectionRef(withDocuments: bookDocs) + let db = collRef.firestore + + let pipeline = db.pipeline() + .collection(collRef.path) + .aggregate( + [Field("rating").avg().as("avgRating")], + groups: ["genre"] + ) // Make sure 'groupBy' and 'average' are correct + let snapshot = try await pipeline.execute() + + // There are 8 unique genres in bookDocs + XCTAssertEqual(snapshot.results.count, 8, "Should return one result per genre") + + for doc in snapshot.results { + XCTAssertNil( + doc.createTime, + "createTime should be nil for aggregate result (docID: \(String(describing: doc.id)), data: \(doc.data))" + ) + XCTAssertNil( + doc.updateTime, + "updateTime should be nil for aggregate result (docID: \(String(describing: doc.id)), data: \(doc.data))" + ) + } + } + + func testSupportsCollectionReferenceAsSource() async throws { + let collRef = collectionRef(withDocuments: bookDocs) + let db = collRef.firestore + + let pipeline = db.pipeline().collection(collRef) + let snapshot = try await pipeline.execute() + + expectResults(snapshot, expectedCount: bookDocs.count) + } + + func testSupportsListOfDocumentReferencesAsSource() async throws { + let collRef = collectionRef(withDocuments: bookDocs) + let db = collRef.firestore + + let docRefs: [DocumentReference] = [ + collRef.document("book1"), + collRef.document("book2"), + collRef.document("book3"), + ] + let pipeline = db.pipeline().documents(docRefs) + let snapshot = try await pipeline.execute() + + expectResults(snapshot, expectedIDs: ["book1", "book2", "book3"]) + } + + func testSupportsListOfDocumentPathsAsSource() async throws { + let collRef = collectionRef(withDocuments: bookDocs) + let db = collRef.firestore + + let docPaths: [String] = [ + collRef.document("book1").path, + collRef.document("book2").path, + collRef.document("book3").path, + ] + let pipeline = db.pipeline().documents(docPaths) + let snapshot = try await pipeline.execute() + + expectResults(snapshot, expectedIDs: ["book1", "book2", "book3"]) + } + + func testRejectsCollectionReferenceFromAnotherDB() async throws { + let db1 = firestore() // Primary DB + + let db2 = Firestore.firestore(app: db1.app, database: "db2") + + let collRefDb2 = db2.collection("foo") + + XCTAssertTrue(FSTNSExceptionUtil.testForException({ + _ = db1.pipeline().collection(collRefDb2) + }, reasonContains: "Invalid CollectionReference")) + } + + func testRejectsDocumentReferenceFromAnotherDB() async throws { + let db1 = firestore() // Primary DB - let actualIDs = Set(results.map { $0.id }) - let expectedIDs = Set([ - "book1", "book2", "book3", "book4", "book5", - "book6", "book7", "book8", "book9", "book10", + let db2 = Firestore.firestore(app: db1.app, database: "db2") + + let docRefDb2 = db2.collection("foo").document("bar") + + XCTAssertTrue(FSTNSExceptionUtil.testForException({ + _ = db1.pipeline().documents([docRefDb2]) + }, reasonContains: "Invalid DocumentReference")) + } + + func testSupportsCollectionGroupAsSource() async throws { + let db = firestore() + + let rootCollForTest = collectionRef() + + let randomSubCollectionId = String(UUID().uuidString.prefix(8)) + + // Create parent documents first to ensure they exist before creating subcollections. + let doc1Ref = rootCollForTest.document("book1").collection(randomSubCollectionId) + .document("translation") + try await doc1Ref.setData(["order": 1]) + + let doc2Ref = rootCollForTest.document("book2").collection(randomSubCollectionId) + .document("translation") + try await doc2Ref.setData(["order": 2]) + + let pipeline = db.pipeline() + .collectionGroup(randomSubCollectionId) + .sort(Field("order").ascending()) + + let snapshot = try await pipeline.execute() + + // Assert that only the two documents from the targeted subCollectionId are fetched, in the + // correct order. + expectResults(snapshot, expectedIDs: [doc1Ref.documentID, doc2Ref.documentID]) + } + + func testSupportsDatabaseAsSource() async throws { + let db = firestore() + let testRootCol = collectionRef() // Provides a unique root path for this test + + let randomIDValue = UUID().uuidString.prefix(8) + + // Document 1 + let collADocRef = testRootCol.document("docA") // Using specific IDs for clarity in debugging + try await collADocRef.setData(["order": 1, "randomId": randomIDValue, "name": "DocInCollA"]) + + // Document 2 + let collBDocRef = testRootCol.document("docB") // Using specific IDs for clarity in debugging + try await collBDocRef.setData(["order": 2, "randomId": randomIDValue, "name": "DocInCollB"]) + + // Document 3 (control, should not be fetched by the main query due to different randomId) + let collCDocRef = testRootCol.document("docC") + try await collCDocRef.setData([ + "order": 3, + "randomId": "\(UUID().uuidString)", + "name": "DocInCollC", + ]) + + // Document 4 (control, no randomId, should not be fetched) + let collDDocRef = testRootCol.document("docD") + try await collDDocRef.setData(["order": 4, "name": "DocInCollDNoRandomId"]) + + // Document 5 (control, correct randomId but in a sub-sub-collection to test depth) + // This also helps ensure the database() query scans deeply. + let subSubCollDocRef = testRootCol.document("parentForSubSub").collection("subSubColl") + .document("docE") + try await subSubCollDocRef.setData([ + "order": 0, + "randomId": randomIDValue, + "name": "DocInSubSubColl", ]) - XCTAssertEqual(actualIDs, expectedIDs) + + let pipeline = db.pipeline() + .database() // Source is the entire database + .where(Field("randomId").eq(randomIDValue)) + .sort(Ascending("order")) + let snapshot = try await pipeline.execute() + + // We expect 3 documents: docA, docB, and docE (from sub-sub-collection) + XCTAssertEqual( + snapshot.results.count, + 3, + "Should fetch the three documents with the correct randomId" + ) + // Order should be docE (order 0), docA (order 1), docB (order 2) + expectResults( + snapshot, + expectedIDs: [subSubCollDocRef.documentID, collADocRef.documentID, collBDocRef.documentID] + ) } } diff --git a/Firestore/core/src/api/aggregate_expressions.cc b/Firestore/core/src/api/aggregate_expressions.cc index fb58918833c..8509dfda59a 100644 --- a/Firestore/core/src/api/aggregate_expressions.cc +++ b/Firestore/core/src/api/aggregate_expressions.cc @@ -25,7 +25,7 @@ namespace api { google_firestore_v1_Value AggregateFunction::to_proto() const { google_firestore_v1_Value result; result.which_value_type = google_firestore_v1_Value_function_value_tag; - + result.function_value = google_firestore_v1_Function{}; result.function_value.name = nanopb::MakeBytesArray(name_); result.function_value.args_count = static_cast(params_.size()); result.function_value.args = nanopb::MakeArray( diff --git a/Firestore/core/src/api/stages.cc b/Firestore/core/src/api/stages.cc index afa943c8cb0..b3dfd2c55a8 100644 --- a/Firestore/core/src/api/stages.cc +++ b/Firestore/core/src/api/stages.cc @@ -84,7 +84,7 @@ google_firestore_v1_Pipeline_Stage DocumentsSource::to_proto() const { result.name = nanopb::MakeBytesArray("documents"); - result.args_count = documents_.size(); + result.args_count = static_cast(documents_.size()); result.args = nanopb::MakeArray(result.args_count); for (size_t i = 0; i < documents_.size(); ++i) { @@ -393,7 +393,7 @@ google_firestore_v1_Pipeline_Stage Unnest::to_proto() const { return result; } -GenericStage::GenericStage( +RawStage::RawStage( std::string name, std::vector> params, std::unordered_map> options) @@ -402,7 +402,7 @@ GenericStage::GenericStage( options_(std::move(options)) { } -google_firestore_v1_Pipeline_Stage GenericStage::to_proto() const { +google_firestore_v1_Pipeline_Stage RawStage::to_proto() const { google_firestore_v1_Pipeline_Stage result; result.name = nanopb::MakeBytesArray(name_); diff --git a/Firestore/core/src/api/stages.h b/Firestore/core/src/api/stages.h index a3c7303ae92..7af3683e99b 100644 --- a/Firestore/core/src/api/stages.h +++ b/Firestore/core/src/api/stages.h @@ -293,12 +293,12 @@ class Unnest : public Stage { absl::optional index_field_; }; -class GenericStage : public Stage { +class RawStage : public Stage { public: - GenericStage(std::string name, - std::vector> params, - std::unordered_map> options); - ~GenericStage() override = default; + RawStage(std::string name, + std::vector> params, + std::unordered_map> options); + ~RawStage() override = default; google_firestore_v1_Pipeline_Stage to_proto() const override; private: