From 325b595f74b5b3441363391a1801a105ca60e1be Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Mon, 12 Feb 2024 09:21:19 -0800 Subject: [PATCH 01/16] snap --- .../DocumentChange+Swift.swift | 24 ++++---- Sources/FirebaseFirestore/Query+Swift.swift | 59 +++++++++++++++++++ .../QuerySnapshot+Swift.swift | 8 +++ Sources/firebase/include/FirebaseCore.hh | 17 ++++++ Sources/firebase/include/FirebaseFirestore.hh | 17 ++++++ 5 files changed, 113 insertions(+), 12 deletions(-) create mode 100644 Sources/FirebaseFirestore/Query+Swift.swift create mode 100644 Sources/FirebaseFirestore/QuerySnapshot+Swift.swift diff --git a/Sources/FirebaseFirestore/DocumentChange+Swift.swift b/Sources/FirebaseFirestore/DocumentChange+Swift.swift index 5893b0c..568fdaa 100644 --- a/Sources/FirebaseFirestore/DocumentChange+Swift.swift +++ b/Sources/FirebaseFirestore/DocumentChange+Swift.swift @@ -9,19 +9,19 @@ public typealias DocumentChange = firebase.firestore.DocumentChange public typealias DocumentChangeType = firebase.firestore.DocumentChange.`Type` extension DocumentChange { - public var type: DocumentChangeType { - swift_firebase.swift_cxx_shims.firebase.firestore.document_change_type(self) - } + public var type: DocumentChangeType { + swift_firebase.swift_cxx_shims.firebase.firestore.document_change_type(self) + } - public var document: DocumentSnapshot { - swift_firebase.swift_cxx_shims.firebase.firestore.document_change_document(self) - } + public var document: DocumentSnapshot { + swift_firebase.swift_cxx_shims.firebase.firestore.document_change_document(self) + } - public var oldIndex: UInt { - UInt(swift_firebase.swift_cxx_shims.firebase.firestore.document_change_old_index(self)) - } + public var oldIndex: UInt { + UInt(swift_firebase.swift_cxx_shims.firebase.firestore.document_change_old_index(self)) + } - public var newIndex: UInt { - UInt(swift_firebase.swift_cxx_shims.firebase.firestore.document_change_new_index(self)) - } + public var newIndex: UInt { + UInt(swift_firebase.swift_cxx_shims.firebase.firestore.document_change_new_index(self)) + } } diff --git a/Sources/FirebaseFirestore/Query+Swift.swift b/Sources/FirebaseFirestore/Query+Swift.swift new file mode 100644 index 0000000..dbe9143 --- /dev/null +++ b/Sources/FirebaseFirestore/Query+Swift.swift @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: BSD-3-Clause + +@_exported +import firebase +@_spi(Error) +import FirebaseCore + +import CxxShim + +public typealias Query = firebase.firestore.Query + +extension Query { + var firestore: Firestore { + swift_firebase.swift_cxx_shims.firebase.firestore.query_firestore(self) + } + + func getDocuments(completion: @escaping (QuerySnapshot?, Error?) -> Void) { + let future = swift_firebase.swift_cxx_shims.firebase.firestore.query_get(self, .default) + FutureSupport.runWork({ + future.Wait(firebase.FutureBase.kWaitTimeoutInfinite) + }, completion: { _ in + if future.error() == 0 { + completion(future.__resultUnsafe().pointee, nil) + } else { + let message = String(cString: future.__error_messageUnsafe()!) + completion(nil, FirebaseError(code: future.error(), message: message)) + } + /* + typealias CompletionType = (QuerySnapshot?, Error?) -> Void + withUnsafePointer(to: completion) { completion in + swift_firebase.swift_cxx_shims.firebase.future_with_result_or_error(future, { result, error, pvCompletion in + let pCompletion = pvCompletion?.assumingMemoryBound(to: CompletionType.self) + if let result { + pCompletion.pointee(result, nil) + } else { + pCompletion.pointee(nil, FirebaseError(code: error, message: nil)) + } + }) + } + */ + /* + future.withResultOrError { result, error in + completion(result, error) + } + */ + }) + } + + func getDocuments() async throws -> QuerySnapshot { + try await withCheckedThrowingContinuation { continuation in + getDocuments() { snapshot, error in + if let error { + continuation.resume(throwing: error) + } + continuation.resume(returning: snapshot ?? .init()) + } + } + } +} diff --git a/Sources/FirebaseFirestore/QuerySnapshot+Swift.swift b/Sources/FirebaseFirestore/QuerySnapshot+Swift.swift new file mode 100644 index 0000000..c4fd888 --- /dev/null +++ b/Sources/FirebaseFirestore/QuerySnapshot+Swift.swift @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: BSD-3-Clause + +@_exported +import firebase + +import CxxShim + +public typealias QuerySnapshot = firebase.firestore.QuerySnapshot diff --git a/Sources/firebase/include/FirebaseCore.hh b/Sources/firebase/include/FirebaseCore.hh index 188d19b..6ac9512 100644 --- a/Sources/firebase/include/FirebaseCore.hh +++ b/Sources/firebase/include/FirebaseCore.hh @@ -4,4 +4,21 @@ #include +/* +template +class FutureWithResultOrError { + public: + typedef void (*Callback)( + const ResultType* result, int error, void* user_data); +}; + +template inline void +future_with_result_or_error( + const ::firebase::Future& future, + FutureWithResultOrError::Callback callback, + void* user_data) { + // XXX +} +*/ + #endif diff --git a/Sources/firebase/include/FirebaseFirestore.hh b/Sources/firebase/include/FirebaseFirestore.hh index cdb574c..47f12b9 100644 --- a/Sources/firebase/include/FirebaseFirestore.hh +++ b/Sources/firebase/include/FirebaseFirestore.hh @@ -166,6 +166,23 @@ document_change_new_index(const ::firebase::firestore::DocumentChange change) { return change.new_index(); } +// MARK: Query + +inline ::firebase::firestore::Firestore * +query_firestore(::firebase::firestore::Query query) { + return query.firestore(); +} + +inline ::firebase::Future<::firebase::firestore::QuerySnapshot> +query_get(const ::firebase::firestore::Query query, + ::firebase::firestore::Source source = + ::firebase::firestore::Source::kDefault) { + return query.Get(source); +} + +// MARK: QuerySnapshot + + } // namespace swift_firebase::swift_cxx_shims::firebase::firestore #endif From c6123c04b4eecbe5af6674ff32b1afe481164f08 Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Mon, 12 Feb 2024 14:35:21 -0800 Subject: [PATCH 02/16] snapshot --- Sources/FirebaseAuth/FirebaseAuth+Swift.swift | 2 +- Sources/FirebaseAuth/FirebaseUser+Swift.swift | 2 +- Sources/FirebaseCore/FirebaseError.swift | 2 +- Sources/FirebaseCore/FutureProtocol.swift | 39 ++++++++++++++ .../DocumentReference+Swift.swift | 2 +- .../DocumentSnapshot+Swift.swift | 2 +- .../FirebaseFirestore/Firestore+Swift.swift | 2 +- Sources/FirebaseFirestore/FutureSupport.swift | 48 +++++++++++++++++ .../ListenerRegistration.swift | 2 +- Sources/FirebaseFirestore/Query+Swift.swift | 54 +++++++++++++++++-- .../FirebaseFirestore/Settings+Swift.swift | 2 +- Sources/firebase/include/FirebaseCore.hh | 40 +++++++++++++- Sources/firebase/include/FirebaseFirestore.hh | 4 +- 13 files changed, 187 insertions(+), 14 deletions(-) create mode 100644 Sources/FirebaseCore/FutureProtocol.swift create mode 100644 Sources/FirebaseFirestore/FutureSupport.swift diff --git a/Sources/FirebaseAuth/FirebaseAuth+Swift.swift b/Sources/FirebaseAuth/FirebaseAuth+Swift.swift index 6ccfed4..667acd9 100644 --- a/Sources/FirebaseAuth/FirebaseAuth+Swift.swift +++ b/Sources/FirebaseAuth/FirebaseAuth+Swift.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Error) +@_spi(Internal) import FirebaseCore import CxxShim diff --git a/Sources/FirebaseAuth/FirebaseUser+Swift.swift b/Sources/FirebaseAuth/FirebaseUser+Swift.swift index d1363cc..1e9512b 100644 --- a/Sources/FirebaseAuth/FirebaseUser+Swift.swift +++ b/Sources/FirebaseAuth/FirebaseUser+Swift.swift @@ -5,7 +5,7 @@ import Foundation @_exported import firebase -@_spi(Error) +@_spi(Internal) import FirebaseCore public typealias User = firebase.auth.User diff --git a/Sources/FirebaseCore/FirebaseError.swift b/Sources/FirebaseCore/FirebaseError.swift index 46647c6..8b619ad 100644 --- a/Sources/FirebaseCore/FirebaseError.swift +++ b/Sources/FirebaseCore/FirebaseError.swift @@ -4,7 +4,7 @@ public struct FirebaseError: Error { public let code: CInt public let message: String - @_spi(Error) + @_spi(Internal) public init(code: CInt, message: String) { self.code = code self.message = message diff --git a/Sources/FirebaseCore/FutureProtocol.swift b/Sources/FirebaseCore/FutureProtocol.swift new file mode 100644 index 0000000..861a8fd --- /dev/null +++ b/Sources/FirebaseCore/FutureProtocol.swift @@ -0,0 +1,39 @@ +@_exported +import firebase + +@_spi(Internal) +public protocol FutureProtocol { + //associatedtype ResultType + //associatedtype CompletionType + //func CallOnCompletion(_ c: CompletionType) + + func Foo() + + //func CallOnCompletion(_ completion: @escaping (UnsafeMutableRawPointer) -> Void, _ userData: UnsafeMutableRawPointer) + func CallOnCompletion(_ c: UnsafeMutableRawPointer) +} + +@_spi(Internal) +public extension FutureProtocol { + /* + func setCompletion(_ completion: @escaping (ResultType?, Error?) -> Void) { + withUnsafePointer(to: completion) { completion in + setOnCompletion({ future, pvCompletion in + // XXX + }, UnsafeMutableRawPointer(mutating: completion)) + } + } + */ + + func setCompletion(_ completion: @escaping () -> Void) { + Foo() + + /* + withUnsafePointer(to: completion) { completion in + CallOnCompletion({ pvCompletion in + // XXX + }, UnsafeMutableRawPointer(mutating: completion)) + } + */ + } +} diff --git a/Sources/FirebaseFirestore/DocumentReference+Swift.swift b/Sources/FirebaseFirestore/DocumentReference+Swift.swift index 5ca8596..e81dc22 100644 --- a/Sources/FirebaseFirestore/DocumentReference+Swift.swift +++ b/Sources/FirebaseFirestore/DocumentReference+Swift.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Error) +@_spi(Internal) import FirebaseCore import CxxShim diff --git a/Sources/FirebaseFirestore/DocumentSnapshot+Swift.swift b/Sources/FirebaseFirestore/DocumentSnapshot+Swift.swift index 78c72dc..c10d02c 100644 --- a/Sources/FirebaseFirestore/DocumentSnapshot+Swift.swift +++ b/Sources/FirebaseFirestore/DocumentSnapshot+Swift.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Error) +@_spi(Internal) import FirebaseCore import CxxShim diff --git a/Sources/FirebaseFirestore/Firestore+Swift.swift b/Sources/FirebaseFirestore/Firestore+Swift.swift index fa09727..a283178 100644 --- a/Sources/FirebaseFirestore/Firestore+Swift.swift +++ b/Sources/FirebaseFirestore/Firestore+Swift.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Error) +@_spi(Internal) import FirebaseCore import CxxShim diff --git a/Sources/FirebaseFirestore/FutureSupport.swift b/Sources/FirebaseFirestore/FutureSupport.swift new file mode 100644 index 0000000..c39489a --- /dev/null +++ b/Sources/FirebaseFirestore/FutureSupport.swift @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: BSD-3-Clause + +@_exported +import firebase + +import CxxShim +import Foundation + +internal enum FutureSupport { + static let workQueue = DispatchQueue(label: "firebase.firestore.workqueue") + + static func runWork(_ work: @escaping () -> Bool, completion: @escaping (Bool) -> Void) { + Self.workQueue.async { + let result = work() + DispatchQueue.main.async { + completion(result) + } + } + } + + /* + static func wait(future: FutureType, timeoutMilliseconds: Int32, completion: @escaping (Bool) -> Void) { + runWork({ + swift_firebase.swift_cxx_shims.firebase.future_wait(future, timeoutMilliseconds) + }, completion: completion) + } + */ +} + +/* +public protocol FutureProtocol { + func Wait(_ timeout_milliseconds: Int32) -> Bool +} + +public extension FutureProtocol { + func wait( + timeoutMilliseconds: Int32 = firebase.FutureBase.kWaitTimeoutInfinite, + completion: @escaping (Bool) -> Void + ) { + FutureSupport.workQueue.async { [self] in + let result = Wait(timeoutMilliseconds) + DispatchQueue.main.async { + completion(result) + } + } + } +} +*/ diff --git a/Sources/FirebaseFirestore/ListenerRegistration.swift b/Sources/FirebaseFirestore/ListenerRegistration.swift index fd5568e..c0564b9 100644 --- a/Sources/FirebaseFirestore/ListenerRegistration.swift +++ b/Sources/FirebaseFirestore/ListenerRegistration.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Error) +@_spi(Internal) import FirebaseCore import CxxShim diff --git a/Sources/FirebaseFirestore/Query+Swift.swift b/Sources/FirebaseFirestore/Query+Swift.swift index dbe9143..8dd9c0d 100644 --- a/Sources/FirebaseFirestore/Query+Swift.swift +++ b/Sources/FirebaseFirestore/Query+Swift.swift @@ -2,29 +2,74 @@ @_exported import firebase -@_spi(Error) +@_spi(Internal) import FirebaseCore import CxxShim public typealias Query = firebase.firestore.Query +/* +extension FutureProtocol { + func wait(completion: @escaping (Bool) -> Void) { + FutureSupport.runWork({ + Wait(firebase.FutureBase.kWaitTimeoutInfinite) + }, completion: completion) + } +} +*/ + extension Query { var firestore: Firestore { swift_firebase.swift_cxx_shims.firebase.firestore.query_firestore(self) } + /* + if let error = future.error { + completion(nil, error) + } else { + completion(future.result, nil) + } + */ func getDocuments(completion: @escaping (QuerySnapshot?, Error?) -> Void) { let future = swift_firebase.swift_cxx_shims.firebase.firestore.query_get(self, .default) - FutureSupport.runWork({ - future.Wait(firebase.FutureBase.kWaitTimeoutInfinite) - }, completion: { _ in + /* + withUnsafePointer(to: completion) { completion in + future.CallOnCompletion({ _ in + // XXX + }, UnsafeMutableRawPointer(mutating: completion)) + } + */ + + future.setCompletion({ + // XXX + }) + + /* + future.wait { result, error in + completion(result, error) + } + */ + + /* + future.wait { error in + if let error { + completion(nil, error) + } else { + completion(future.__resultUnsafe().pointee, nil) + } + } + */ + + #if false + future.wait(completion: { _ in if future.error() == 0 { completion(future.__resultUnsafe().pointee, nil) } else { let message = String(cString: future.__error_messageUnsafe()!) completion(nil, FirebaseError(code: future.error(), message: message)) } + /* typealias CompletionType = (QuerySnapshot?, Error?) -> Void withUnsafePointer(to: completion) { completion in @@ -44,6 +89,7 @@ extension Query { } */ }) + #endif } func getDocuments() async throws -> QuerySnapshot { diff --git a/Sources/FirebaseFirestore/Settings+Swift.swift b/Sources/FirebaseFirestore/Settings+Swift.swift index dac6355..d27da8e 100644 --- a/Sources/FirebaseFirestore/Settings+Swift.swift +++ b/Sources/FirebaseFirestore/Settings+Swift.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Error) +@_spi(Internal) import FirebaseCore import CxxShim diff --git a/Sources/firebase/include/FirebaseCore.hh b/Sources/firebase/include/FirebaseCore.hh index 6ac9512..c0af588 100644 --- a/Sources/firebase/include/FirebaseCore.hh +++ b/Sources/firebase/include/FirebaseCore.hh @@ -1,9 +1,10 @@ - #ifndef firebase_include_FirebaseCore_hh #define firebase_include_FirebaseCore_hh #include +namespace swift_firebase::swift_cxx_shims::firebase { + /* template class FutureWithResultOrError { @@ -21,4 +22,41 @@ future_with_result_or_error( } */ +/* +template inline bool +future_wait(const ::firebase::Future& future, int timeout_milliseconds) { + return future.Wait(timeout_milliseconds); +} +*/ + +typedef void (*CompletionType)(void*); + +// This class exists to provide protocol conformance to FutureProtocol. +template class ConformingFuture: public ::firebase::Future { + public: + typedef R ResultType; + //typedef ::firebase::Future FutureType; + //typedef ::firebase::Future::TypedCompletionCallback TypedCompletionCallback; + + ConformingFuture(const ::firebase::Future& rhs) + : ::firebase::Future(rhs) {} + + //FutureType AsFuture() { + // return *this; + //} + // + + //typedef int CompletionType; + //void CallOnCompletion(CompletionType c) {} + + void Foo() const {} + + void CallOnCompletion(void* completion) const { + // + } +} __attribute__((swift_attr("conforms_to:FirebaseCore.FutureProtocol"))); + + +} // namespace swift_firebase::swift_cxx_shims::firebase + #endif diff --git a/Sources/firebase/include/FirebaseFirestore.hh b/Sources/firebase/include/FirebaseFirestore.hh index 47f12b9..0a9df8f 100644 --- a/Sources/firebase/include/FirebaseFirestore.hh +++ b/Sources/firebase/include/FirebaseFirestore.hh @@ -5,6 +5,8 @@ #include +#include "FirebaseCore.hh" + // Functions defined in this namespace are used to get around the lack of // virtual function support currently in Swift. As that support changes // these functions will go away whenever possible. @@ -173,7 +175,7 @@ query_firestore(::firebase::firestore::Query query) { return query.firestore(); } -inline ::firebase::Future<::firebase::firestore::QuerySnapshot> +inline ::swift_firebase::swift_cxx_shims::firebase::ConformingFuture<::firebase::firestore::QuerySnapshot> query_get(const ::firebase::firestore::Query query, ::firebase::firestore::Source source = ::firebase::firestore::Source::kDefault) { From 1f5c2668612e6e677a9bebced58934dfdbfc3813 Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Mon, 12 Feb 2024 15:36:25 -0800 Subject: [PATCH 03/16] resolve the Swift conforms to nullability conformance issue --- Sources/FirebaseCore/FutureProtocol.swift | 6 +++--- Sources/firebase/include/FirebaseCore.hh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/FirebaseCore/FutureProtocol.swift b/Sources/FirebaseCore/FutureProtocol.swift index 861a8fd..eb92d3d 100644 --- a/Sources/FirebaseCore/FutureProtocol.swift +++ b/Sources/FirebaseCore/FutureProtocol.swift @@ -10,7 +10,7 @@ public protocol FutureProtocol { func Foo() //func CallOnCompletion(_ completion: @escaping (UnsafeMutableRawPointer) -> Void, _ userData: UnsafeMutableRawPointer) - func CallOnCompletion(_ c: UnsafeMutableRawPointer) + func CallOnCompletion(_ c: swift_firebase.swift_cxx_shims.firebase.CompletionType, _ userData: UnsafeMutableRawPointer?) } @_spi(Internal) @@ -28,12 +28,12 @@ public extension FutureProtocol { func setCompletion(_ completion: @escaping () -> Void) { Foo() - /* + withUnsafePointer(to: completion) { completion in CallOnCompletion({ pvCompletion in // XXX }, UnsafeMutableRawPointer(mutating: completion)) } - */ + } } diff --git a/Sources/firebase/include/FirebaseCore.hh b/Sources/firebase/include/FirebaseCore.hh index c0af588..a3802dc 100644 --- a/Sources/firebase/include/FirebaseCore.hh +++ b/Sources/firebase/include/FirebaseCore.hh @@ -51,7 +51,7 @@ template class ConformingFuture: public ::firebase::Future { void Foo() const {} - void CallOnCompletion(void* completion) const { + void CallOnCompletion(_Nonnull CompletionType, _Nullable void *userData) const { // } } __attribute__((swift_attr("conforms_to:FirebaseCore.FutureProtocol"))); From 9692d0b11de2ad28d95aa348757ecc8d45acfb04 Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Mon, 12 Feb 2024 15:56:28 -0800 Subject: [PATCH 04/16] snap --- Sources/FirebaseCore/FutureProtocol.swift | 15 ++++----------- Sources/firebase/include/FirebaseCore.hh | 16 ++-------------- 2 files changed, 6 insertions(+), 25 deletions(-) diff --git a/Sources/FirebaseCore/FutureProtocol.swift b/Sources/FirebaseCore/FutureProtocol.swift index eb92d3d..f21c807 100644 --- a/Sources/FirebaseCore/FutureProtocol.swift +++ b/Sources/FirebaseCore/FutureProtocol.swift @@ -1,16 +1,13 @@ @_exported import firebase +@_spi(Internal) +public typealias FutureCompletionType = swift_firebase.swift_cxx_shims.firebase.FutureCompletionType + @_spi(Internal) public protocol FutureProtocol { //associatedtype ResultType - //associatedtype CompletionType - //func CallOnCompletion(_ c: CompletionType) - - func Foo() - - //func CallOnCompletion(_ completion: @escaping (UnsafeMutableRawPointer) -> Void, _ userData: UnsafeMutableRawPointer) - func CallOnCompletion(_ c: swift_firebase.swift_cxx_shims.firebase.CompletionType, _ userData: UnsafeMutableRawPointer?) + func CallOnCompletion(_ completion: FutureCompletionType, _ user_data: UnsafeMutableRawPointer?) } @_spi(Internal) @@ -26,14 +23,10 @@ public extension FutureProtocol { */ func setCompletion(_ completion: @escaping () -> Void) { - Foo() - - withUnsafePointer(to: completion) { completion in CallOnCompletion({ pvCompletion in // XXX }, UnsafeMutableRawPointer(mutating: completion)) } - } } diff --git a/Sources/firebase/include/FirebaseCore.hh b/Sources/firebase/include/FirebaseCore.hh index a3802dc..eb74a07 100644 --- a/Sources/firebase/include/FirebaseCore.hh +++ b/Sources/firebase/include/FirebaseCore.hh @@ -29,7 +29,7 @@ future_wait(const ::firebase::Future& future, int timeout_millisecon } */ -typedef void (*CompletionType)(void*); +typedef void (*FutureCompletionType)(void*); // This class exists to provide protocol conformance to FutureProtocol. template class ConformingFuture: public ::firebase::Future { @@ -41,19 +41,7 @@ template class ConformingFuture: public ::firebase::Future { ConformingFuture(const ::firebase::Future& rhs) : ::firebase::Future(rhs) {} - //FutureType AsFuture() { - // return *this; - //} - // - - //typedef int CompletionType; - //void CallOnCompletion(CompletionType c) {} - - void Foo() const {} - - void CallOnCompletion(_Nonnull CompletionType, _Nullable void *userData) const { - // - } + void CallOnCompletion(_Nonnull FutureCompletionType, _Nullable void* user_data) const {} } __attribute__((swift_attr("conforms_to:FirebaseCore.FutureProtocol"))); From 00e3dfa54032d3c051041c196453c3fda30deb84 Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Mon, 12 Feb 2024 15:56:45 -0800 Subject: [PATCH 05/16] cleanup --- Sources/FirebaseCore/FutureProtocol.swift | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/Sources/FirebaseCore/FutureProtocol.swift b/Sources/FirebaseCore/FutureProtocol.swift index f21c807..0cc10d9 100644 --- a/Sources/FirebaseCore/FutureProtocol.swift +++ b/Sources/FirebaseCore/FutureProtocol.swift @@ -12,16 +12,6 @@ public protocol FutureProtocol { @_spi(Internal) public extension FutureProtocol { - /* - func setCompletion(_ completion: @escaping (ResultType?, Error?) -> Void) { - withUnsafePointer(to: completion) { completion in - setOnCompletion({ future, pvCompletion in - // XXX - }, UnsafeMutableRawPointer(mutating: completion)) - } - } - */ - func setCompletion(_ completion: @escaping () -> Void) { withUnsafePointer(to: completion) { completion in CallOnCompletion({ pvCompletion in From efefe34bee92f5ccbad07ee1b6b17c5fc9ef460f Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Mon, 12 Feb 2024 16:15:12 -0800 Subject: [PATCH 06/16] snap --- Sources/FirebaseCore/FutureProtocol.swift | 19 +++++++++++++++++-- Sources/FirebaseFirestore/Query+Swift.swift | 20 +++++++++++--------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/Sources/FirebaseCore/FutureProtocol.swift b/Sources/FirebaseCore/FutureProtocol.swift index 0cc10d9..7a700e3 100644 --- a/Sources/FirebaseCore/FutureProtocol.swift +++ b/Sources/FirebaseCore/FutureProtocol.swift @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: BSD-3-Clause + @_exported import firebase @@ -6,7 +8,10 @@ public typealias FutureCompletionType = swift_firebase.swift_cxx_shims.firebase. @_spi(Internal) public protocol FutureProtocol { - //associatedtype ResultType + associatedtype ResultType + func error() -> Int32 + func __error_messageUnsafe() -> UnsafePointer? + func __resultUnsafe() -> UnsafePointer? func CallOnCompletion(_ completion: FutureCompletionType, _ user_data: UnsafeMutableRawPointer?) } @@ -15,8 +20,18 @@ public extension FutureProtocol { func setCompletion(_ completion: @escaping () -> Void) { withUnsafePointer(to: completion) { completion in CallOnCompletion({ pvCompletion in - // XXX + let pCompletion = pvCompletion?.assumingMemoryBound(to: (() -> Void).self) + pCompletion.pointee() }, UnsafeMutableRawPointer(mutating: completion)) } } + + var resultAndError: (ResultType?, Error?) { + let error = error() + guard error == 0 else { + let message = String(cString: __error_messageUnsafe()!) + return (nil, FirebaseError(code: error, message: message)) + } + return (__resultUnsafe().pointee, nil) + } } diff --git a/Sources/FirebaseFirestore/Query+Swift.swift b/Sources/FirebaseFirestore/Query+Swift.swift index 8dd9c0d..5b341a7 100644 --- a/Sources/FirebaseFirestore/Query+Swift.swift +++ b/Sources/FirebaseFirestore/Query+Swift.swift @@ -33,16 +33,18 @@ extension Query { func getDocuments(completion: @escaping (QuerySnapshot?, Error?) -> Void) { let future = swift_firebase.swift_cxx_shims.firebase.firestore.query_get(self, .default) - /* - withUnsafePointer(to: completion) { completion in - future.CallOnCompletion({ _ in - // XXX - }, UnsafeMutableRawPointer(mutating: completion)) - } - */ - future.setCompletion({ - // XXX + let (result, error) = future.resultAndError + completion(result, error) + + /* + if future.error() == 0 { + completion(future.__resultUnsafe().pointee, nil) + } else { + let message = String(cString: future.__error_messageUnsafe()!) + completion(nil, FirebaseError(code: future.error(), message: message)) + } + */ }) /* From 10dcccad4c75f79862741caf587bf52955c47b20 Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Mon, 12 Feb 2024 16:19:26 -0800 Subject: [PATCH 07/16] cleanup --- Sources/FirebaseFirestore/Query+Swift.swift | 76 +-------------------- 1 file changed, 2 insertions(+), 74 deletions(-) diff --git a/Sources/FirebaseFirestore/Query+Swift.swift b/Sources/FirebaseFirestore/Query+Swift.swift index 5b341a7..61ae5a6 100644 --- a/Sources/FirebaseFirestore/Query+Swift.swift +++ b/Sources/FirebaseFirestore/Query+Swift.swift @@ -9,89 +9,17 @@ import CxxShim public typealias Query = firebase.firestore.Query -/* -extension FutureProtocol { - func wait(completion: @escaping (Bool) -> Void) { - FutureSupport.runWork({ - Wait(firebase.FutureBase.kWaitTimeoutInfinite) - }, completion: completion) - } -} -*/ - extension Query { var firestore: Firestore { swift_firebase.swift_cxx_shims.firebase.firestore.query_firestore(self) } - /* - if let error = future.error { - completion(nil, error) - } else { - completion(future.result, nil) - } - */ func getDocuments(completion: @escaping (QuerySnapshot?, Error?) -> Void) { let future = swift_firebase.swift_cxx_shims.firebase.firestore.query_get(self, .default) future.setCompletion({ - let (result, error) = future.resultAndError - completion(result, error) - - /* - if future.error() == 0 { - completion(future.__resultUnsafe().pointee, nil) - } else { - let message = String(cString: future.__error_messageUnsafe()!) - completion(nil, FirebaseError(code: future.error(), message: message)) - } - */ - }) - - /* - future.wait { result, error in - completion(result, error) - } - */ - - /* - future.wait { error in - if let error { - completion(nil, error) - } else { - completion(future.__resultUnsafe().pointee, nil) - } - } - */ - - #if false - future.wait(completion: { _ in - if future.error() == 0 { - completion(future.__resultUnsafe().pointee, nil) - } else { - let message = String(cString: future.__error_messageUnsafe()!) - completion(nil, FirebaseError(code: future.error(), message: message)) - } - - /* - typealias CompletionType = (QuerySnapshot?, Error?) -> Void - withUnsafePointer(to: completion) { completion in - swift_firebase.swift_cxx_shims.firebase.future_with_result_or_error(future, { result, error, pvCompletion in - let pCompletion = pvCompletion?.assumingMemoryBound(to: CompletionType.self) - if let result { - pCompletion.pointee(result, nil) - } else { - pCompletion.pointee(nil, FirebaseError(code: error, message: nil)) - } - }) - } - */ - /* - future.withResultOrError { result, error in - completion(result, error) - } - */ + let (snapshot, error) = future.resultAndError + completion(snapshot, error) }) - #endif } func getDocuments() async throws -> QuerySnapshot { From 063db41768a0b748d56a7f5f82dfca043a6a66d9 Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Mon, 12 Feb 2024 20:02:40 -0800 Subject: [PATCH 08/16] snap --- .../FireBaseUI/FireBaseUIViewController.swift | 2 +- .../FirestoreTestingViewController.swift | 4 +- Sources/FirebaseCore/FutureProtocol.swift | 18 +++++--- .../DocumentReference+Swift.swift | 36 ++++++++-------- Sources/firebase/include/FirebaseCore.hh | 41 +++++-------------- Sources/firebase/include/FirebaseFirestore.hh | 8 ++-- 6 files changed, 47 insertions(+), 62 deletions(-) diff --git a/Examples/FireBaseUI/FireBaseUIViewController.swift b/Examples/FireBaseUI/FireBaseUIViewController.swift index b4ca283..f50182d 100644 --- a/Examples/FireBaseUI/FireBaseUIViewController.swift +++ b/Examples/FireBaseUI/FireBaseUIViewController.swift @@ -246,7 +246,7 @@ internal final class FireBaseUIViewController: ViewController { let document = firestore .collection("users") .document(user.uid) - let snapshot = try await document.get() + let snapshot = try await document.getDocument() await MainActor.run { [weak self] in guard let self else { return } userDetailsLabel.text = snapshot.debugDescription diff --git a/Examples/FireBaseUI/FirestoreTestingViewController.swift b/Examples/FireBaseUI/FirestoreTestingViewController.swift index 9f08d07..34a09ec 100644 --- a/Examples/FireBaseUI/FirestoreTestingViewController.swift +++ b/Examples/FireBaseUI/FirestoreTestingViewController.swift @@ -67,8 +67,8 @@ internal final class FirestoreTestingViewController: ViewController { await buttonsEnabled(false) let document = Firestore.firestore().document(path) do { - let snapshot = try await document.get() - await displayData(data: snapshot?.debugDescription ?? "No snapshot") + let snapshot = try await document.getDocument() + await displayData(data: snapshot.debugDescription) } catch { await displayError(error: error) } diff --git a/Sources/FirebaseCore/FutureProtocol.swift b/Sources/FirebaseCore/FutureProtocol.swift index 7a700e3..69183a9 100644 --- a/Sources/FirebaseCore/FutureProtocol.swift +++ b/Sources/FirebaseCore/FutureProtocol.swift @@ -3,6 +3,8 @@ @_exported import firebase +import Foundation + @_spi(Internal) public typealias FutureCompletionType = swift_firebase.swift_cxx_shims.firebase.FutureCompletionType @@ -15,15 +17,19 @@ public protocol FutureProtocol { func CallOnCompletion(_ completion: FutureCompletionType, _ user_data: UnsafeMutableRawPointer?) } +private class CompletionWrapper { + let completion: () -> Void + init(_ completion: @escaping () -> Void) { + self.completion = completion + } +} + @_spi(Internal) public extension FutureProtocol { func setCompletion(_ completion: @escaping () -> Void) { - withUnsafePointer(to: completion) { completion in - CallOnCompletion({ pvCompletion in - let pCompletion = pvCompletion?.assumingMemoryBound(to: (() -> Void).self) - pCompletion.pointee() - }, UnsafeMutableRawPointer(mutating: completion)) - } + CallOnCompletion({ ptr in + Unmanaged.fromOpaque(ptr!).takeRetainedValue().completion() + }, Unmanaged.passRetained(CompletionWrapper(completion)).toOpaque()) } var resultAndError: (ResultType?, Error?) { diff --git a/Sources/FirebaseFirestore/DocumentReference+Swift.swift b/Sources/FirebaseFirestore/DocumentReference+Swift.swift index e81dc22..d5dca9f 100644 --- a/Sources/FirebaseFirestore/DocumentReference+Swift.swift +++ b/Sources/FirebaseFirestore/DocumentReference+Swift.swift @@ -33,27 +33,23 @@ extension DocumentReference { String(swift_firebase.swift_cxx_shims.firebase.firestore.document_path(self)) } - public func get() async throws -> DocumentSnapshot? { - typealias Promise = CheckedContinuation, any Error> - let snapshot = try await withCheckedThrowingContinuation { (continuation: Promise) in - let future = swift_firebase.swift_cxx_shims.firebase.firestore.document_get(self, .default) - withUnsafePointer(to: continuation) { continuation in - future.OnCompletion_SwiftWorkaround({ future, pvContinuation in - let pContinuation = pvContinuation?.assumingMemoryBound(to: Promise.self) - if future.pointee.error() == 0 { - pContinuation.pointee.resume(returning: .init(mutating: future.pointee.__resultUnsafe())) - } else { - let code = future.pointee.error() - let message = String(cString: future.pointee.__error_messageUnsafe()!) - pContinuation.pointee.resume(throwing: FirebaseError(code: code, message: message)) - } - }, UnsafeMutableRawPointer(mutating: continuation)) - } - future.Wait(firebase.FutureBase.kWaitTimeoutInfinite) - } + public func getDocument(completion: @escaping (DocumentSnapshot?, Error?) -> Void) { + let future = swift_firebase.swift_cxx_shims.firebase.firestore.document_get(self, .default) + future.setCompletion({ + let (snapshot, error) = future.resultAndError + completion(snapshot, error) + }) + } - guard snapshot.pointee.exists else { return nil } - return snapshot.pointee.is_valid() ? snapshot.pointee : nil + public func getDocument() async throws -> DocumentSnapshot { + try await withCheckedThrowingContinuation { continuation in + getDocument(completion: { snapshot, error in + if let error { + continuation.resume(throwing: error) + } + continuation.resume(returning: snapshot ?? .init()) + }) + } } public func addSnapshotListener(_ listener: @escaping SnapshotListenerCallback) -> ListenerRegistration { diff --git a/Sources/firebase/include/FirebaseCore.hh b/Sources/firebase/include/FirebaseCore.hh index eb74a07..e8bc582 100644 --- a/Sources/firebase/include/FirebaseCore.hh +++ b/Sources/firebase/include/FirebaseCore.hh @@ -3,31 +3,9 @@ #include -namespace swift_firebase::swift_cxx_shims::firebase { +#include -/* -template -class FutureWithResultOrError { - public: - typedef void (*Callback)( - const ResultType* result, int error, void* user_data); -}; - -template inline void -future_with_result_or_error( - const ::firebase::Future& future, - FutureWithResultOrError::Callback callback, - void* user_data) { - // XXX -} -*/ - -/* -template inline bool -future_wait(const ::firebase::Future& future, int timeout_milliseconds) { - return future.Wait(timeout_milliseconds); -} -*/ +namespace swift_firebase::swift_cxx_shims::firebase { typedef void (*FutureCompletionType)(void*); @@ -35,16 +13,19 @@ typedef void (*FutureCompletionType)(void*); template class ConformingFuture: public ::firebase::Future { public: typedef R ResultType; - //typedef ::firebase::Future FutureType; - //typedef ::firebase::Future::TypedCompletionCallback TypedCompletionCallback; + typedef ::firebase::Future FutureType; - ConformingFuture(const ::firebase::Future& rhs) - : ::firebase::Future(rhs) {} + ConformingFuture(const FutureType& rhs) : FutureType(rhs) {} - void CallOnCompletion(_Nonnull FutureCompletionType, _Nullable void* user_data) const {} + void CallOnCompletion( + _Nonnull FutureCompletionType completion, + _Nullable void* user_data) const { + OnCompletion([completion, user_data](const FutureBase&) { + completion(user_data); + }); + } } __attribute__((swift_attr("conforms_to:FirebaseCore.FutureProtocol"))); - } // namespace swift_firebase::swift_cxx_shims::firebase #endif diff --git a/Sources/firebase/include/FirebaseFirestore.hh b/Sources/firebase/include/FirebaseFirestore.hh index 0a9df8f..0fad062 100644 --- a/Sources/firebase/include/FirebaseFirestore.hh +++ b/Sources/firebase/include/FirebaseFirestore.hh @@ -45,14 +45,15 @@ document_path(const ::firebase::firestore::DocumentReference document) { return document.path(); } -inline ::firebase::Future<::firebase::firestore::DocumentSnapshot> +inline ::swift_firebase::swift_cxx_shims::firebase::ConformingFuture< + ::firebase::firestore::DocumentSnapshot> document_get(const ::firebase::firestore::DocumentReference document, ::firebase::firestore::Source source = ::firebase::firestore::Source::kDefault) { return document.Get(source); } -inline ::firebase::Future +inline ::swift_firebase::swift_cxx_shims::firebase::ConformingFuture document_set_data(::firebase::firestore::DocumentReference document, const ::firebase::firestore::MapFieldValue data, const ::firebase::firestore::SetOptions options) { @@ -175,7 +176,8 @@ query_firestore(::firebase::firestore::Query query) { return query.firestore(); } -inline ::swift_firebase::swift_cxx_shims::firebase::ConformingFuture<::firebase::firestore::QuerySnapshot> +inline ::swift_firebase::swift_cxx_shims::firebase::ConformingFuture< + ::firebase::firestore::QuerySnapshot> query_get(const ::firebase::firestore::Query query, ::firebase::firestore::Source source = ::firebase::firestore::Source::kDefault) { From 12d8b6f5c20442a93dbba3cf455b4d72e15d65c7 Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Mon, 12 Feb 2024 20:03:25 -0800 Subject: [PATCH 09/16] cleanup --- Sources/FirebaseFirestore/FutureSupport.swift | 48 ------------------- 1 file changed, 48 deletions(-) delete mode 100644 Sources/FirebaseFirestore/FutureSupport.swift diff --git a/Sources/FirebaseFirestore/FutureSupport.swift b/Sources/FirebaseFirestore/FutureSupport.swift deleted file mode 100644 index c39489a..0000000 --- a/Sources/FirebaseFirestore/FutureSupport.swift +++ /dev/null @@ -1,48 +0,0 @@ -// SPDX-License-Identifier: BSD-3-Clause - -@_exported -import firebase - -import CxxShim -import Foundation - -internal enum FutureSupport { - static let workQueue = DispatchQueue(label: "firebase.firestore.workqueue") - - static func runWork(_ work: @escaping () -> Bool, completion: @escaping (Bool) -> Void) { - Self.workQueue.async { - let result = work() - DispatchQueue.main.async { - completion(result) - } - } - } - - /* - static func wait(future: FutureType, timeoutMilliseconds: Int32, completion: @escaping (Bool) -> Void) { - runWork({ - swift_firebase.swift_cxx_shims.firebase.future_wait(future, timeoutMilliseconds) - }, completion: completion) - } - */ -} - -/* -public protocol FutureProtocol { - func Wait(_ timeout_milliseconds: Int32) -> Bool -} - -public extension FutureProtocol { - func wait( - timeoutMilliseconds: Int32 = firebase.FutureBase.kWaitTimeoutInfinite, - completion: @escaping (Bool) -> Void - ) { - FutureSupport.workQueue.async { [self] in - let result = Wait(timeoutMilliseconds) - DispatchQueue.main.async { - completion(result) - } - } - } -} -*/ From 7ab5522caa4ccf8eb9e515c4bdd3da6bedeb5a98 Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Mon, 12 Feb 2024 20:10:49 -0800 Subject: [PATCH 10/16] cleanup --- Sources/firebase/include/FirebaseFirestore.hh | 3 --- 1 file changed, 3 deletions(-) diff --git a/Sources/firebase/include/FirebaseFirestore.hh b/Sources/firebase/include/FirebaseFirestore.hh index 0fad062..9577b89 100644 --- a/Sources/firebase/include/FirebaseFirestore.hh +++ b/Sources/firebase/include/FirebaseFirestore.hh @@ -184,9 +184,6 @@ query_get(const ::firebase::firestore::Query query, return query.Get(source); } -// MARK: QuerySnapshot - - } // namespace swift_firebase::swift_cxx_shims::firebase::firestore #endif From a21611c03ba4363ea6e6ef5576410759aceb3caa Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Mon, 12 Feb 2024 20:18:12 -0800 Subject: [PATCH 11/16] add comments --- Sources/FirebaseCore/FutureProtocol.swift | 29 +++++++++++++++++------ Sources/firebase/include/FirebaseCore.hh | 5 +++- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/Sources/FirebaseCore/FutureProtocol.swift b/Sources/FirebaseCore/FutureProtocol.swift index 69183a9..2d23515 100644 --- a/Sources/FirebaseCore/FutureProtocol.swift +++ b/Sources/FirebaseCore/FutureProtocol.swift @@ -8,6 +8,11 @@ import Foundation @_spi(Internal) public typealias FutureCompletionType = swift_firebase.swift_cxx_shims.firebase.FutureCompletionType +// This protocol enables extracting common code for Future handling. Because +// C++ interop is limited for templated class types, we need to invent a +// protocol to reflect the features of a Future that should be generically +// available. This works by having a C++ annotation (see ConformingFuture) +// that specifies this protocol conformance. @_spi(Internal) public protocol FutureProtocol { associatedtype ResultType @@ -17,21 +22,22 @@ public protocol FutureProtocol { func CallOnCompletion(_ completion: FutureCompletionType, _ user_data: UnsafeMutableRawPointer?) } -private class CompletionWrapper { - let completion: () -> Void - init(_ completion: @escaping () -> Void) { - self.completion = completion - } -} - +// A home for various helper functions that make it easier to work with +// Firebase Future objects from Swift. @_spi(Internal) public extension FutureProtocol { + // Callsites retain their own reference to the Future, but they still need + // a way to know when the Future completes. This provides that mechanism. + // While the underlying Firebase `OnCompletion` method can provide a reference + // back to the Future, we don't need to expose that here. func setCompletion(_ completion: @escaping () -> Void) { CallOnCompletion({ ptr in Unmanaged.fromOpaque(ptr!).takeRetainedValue().completion() }, Unmanaged.passRetained(CompletionWrapper(completion)).toOpaque()) } + // A convenient way to access the result or error of a Future. Handles the + // messy details. var resultAndError: (ResultType?, Error?) { let error = error() guard error == 0 else { @@ -41,3 +47,12 @@ public extension FutureProtocol { return (__resultUnsafe().pointee, nil) } } + +// The Unmanaged type only works with classes, so we need a wrapper for the +// completion callback. +private class CompletionWrapper { + let completion: () -> Void + init(_ completion: @escaping () -> Void) { + self.completion = completion + } +} diff --git a/Sources/firebase/include/FirebaseCore.hh b/Sources/firebase/include/FirebaseCore.hh index e8bc582..f1ee6fc 100644 --- a/Sources/firebase/include/FirebaseCore.hh +++ b/Sources/firebase/include/FirebaseCore.hh @@ -9,7 +9,10 @@ namespace swift_firebase::swift_cxx_shims::firebase { typedef void (*FutureCompletionType)(void*); -// This class exists to provide protocol conformance to FutureProtocol. +// This class exists to provide protocol conformance to FutureProtocol. It +// also provides a method to invoke `OnCompletion` in a way that works from +// Swift. We can ignore the `FutureBase` param as the Swift caller can just +// retain the Future as part of its closure. template class ConformingFuture: public ::firebase::Future { public: typedef R ResultType; From 47741b73ade20c2c8eae4bccb544170e22d4393f Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Mon, 12 Feb 2024 20:28:35 -0800 Subject: [PATCH 12/16] fix error handling --- Sources/FirebaseFirestore/DocumentReference+Swift.swift | 3 ++- Sources/FirebaseFirestore/Query+Swift.swift | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Sources/FirebaseFirestore/DocumentReference+Swift.swift b/Sources/FirebaseFirestore/DocumentReference+Swift.swift index d5dca9f..d42158b 100644 --- a/Sources/FirebaseFirestore/DocumentReference+Swift.swift +++ b/Sources/FirebaseFirestore/DocumentReference+Swift.swift @@ -46,8 +46,9 @@ extension DocumentReference { getDocument(completion: { snapshot, error in if let error { continuation.resume(throwing: error) + } else { + continuation.resume(returning: snapshot ?? .init()) } - continuation.resume(returning: snapshot ?? .init()) }) } } diff --git a/Sources/FirebaseFirestore/Query+Swift.swift b/Sources/FirebaseFirestore/Query+Swift.swift index 61ae5a6..5554a37 100644 --- a/Sources/FirebaseFirestore/Query+Swift.swift +++ b/Sources/FirebaseFirestore/Query+Swift.swift @@ -27,8 +27,9 @@ extension Query { getDocuments() { snapshot, error in if let error { continuation.resume(throwing: error) + } else { + continuation.resume(returning: snapshot ?? .init()) } - continuation.resume(returning: snapshot ?? .init()) } } } From bcf2754e2b5f434198fd57b58aa92dcc886b66fe Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Tue, 13 Feb 2024 09:38:55 -0800 Subject: [PATCH 13/16] Some improvements per review feedback --- Sources/FirebaseCore/FutureProtocol.swift | 14 +++++++++----- Sources/FirebaseFirestore/Query+Swift.swift | 6 +++--- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Sources/FirebaseCore/FutureProtocol.swift b/Sources/FirebaseCore/FutureProtocol.swift index 2d23515..c5c31e6 100644 --- a/Sources/FirebaseCore/FutureProtocol.swift +++ b/Sources/FirebaseCore/FutureProtocol.swift @@ -6,7 +6,8 @@ import firebase import Foundation @_spi(Internal) -public typealias FutureCompletionType = swift_firebase.swift_cxx_shims.firebase.FutureCompletionType +public typealias FutureCompletionType = + swift_firebase.swift_cxx_shims.firebase.FutureCompletionType // This protocol enables extracting common code for Future handling. Because // C++ interop is limited for templated class types, we need to invent a @@ -19,7 +20,10 @@ public protocol FutureProtocol { func error() -> Int32 func __error_messageUnsafe() -> UnsafePointer? func __resultUnsafe() -> UnsafePointer? - func CallOnCompletion(_ completion: FutureCompletionType, _ user_data: UnsafeMutableRawPointer?) + func CallOnCompletion( + _ completion: FutureCompletionType, + _ user_data: UnsafeMutableRawPointer? + ) } // A home for various helper functions that make it easier to work with @@ -32,8 +36,8 @@ public extension FutureProtocol { // back to the Future, we don't need to expose that here. func setCompletion(_ completion: @escaping () -> Void) { CallOnCompletion({ ptr in - Unmanaged.fromOpaque(ptr!).takeRetainedValue().completion() - }, Unmanaged.passRetained(CompletionWrapper(completion)).toOpaque()) + Unmanaged.fromOpaque(ptr!).takeRetainedValue().completion() + }, Unmanaged.passRetained(CompletionReference(completion)).toOpaque()) } // A convenient way to access the result or error of a Future. Handles the @@ -50,7 +54,7 @@ public extension FutureProtocol { // The Unmanaged type only works with classes, so we need a wrapper for the // completion callback. -private class CompletionWrapper { +private class CompletionReference { let completion: () -> Void init(_ completion: @escaping () -> Void) { self.completion = completion diff --git a/Sources/FirebaseFirestore/Query+Swift.swift b/Sources/FirebaseFirestore/Query+Swift.swift index 5554a37..9d17aa5 100644 --- a/Sources/FirebaseFirestore/Query+Swift.swift +++ b/Sources/FirebaseFirestore/Query+Swift.swift @@ -10,11 +10,11 @@ import CxxShim public typealias Query = firebase.firestore.Query extension Query { - var firestore: Firestore { + public var firestore: Firestore { swift_firebase.swift_cxx_shims.firebase.firestore.query_firestore(self) } - func getDocuments(completion: @escaping (QuerySnapshot?, Error?) -> Void) { + public func getDocuments(completion: @escaping (QuerySnapshot?, Error?) -> Void) { let future = swift_firebase.swift_cxx_shims.firebase.firestore.query_get(self, .default) future.setCompletion({ let (snapshot, error) = future.resultAndError @@ -22,7 +22,7 @@ extension Query { }) } - func getDocuments() async throws -> QuerySnapshot { + public func getDocuments() async throws -> QuerySnapshot { try await withCheckedThrowingContinuation { continuation in getDocuments() { snapshot, error in if let error { From c7d8d475d87ee13102540b5e8d9364b88d33a737 Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Tue, 13 Feb 2024 09:49:50 -0800 Subject: [PATCH 14/16] completion callbacks should run on the main thread --- .../FirebaseFirestore/DocumentReference+Swift.swift | 9 +++++++-- Sources/FirebaseFirestore/Query+Swift.swift | 12 +++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/Sources/FirebaseFirestore/DocumentReference+Swift.swift b/Sources/FirebaseFirestore/DocumentReference+Swift.swift index d42158b..3612c69 100644 --- a/Sources/FirebaseFirestore/DocumentReference+Swift.swift +++ b/Sources/FirebaseFirestore/DocumentReference+Swift.swift @@ -33,17 +33,22 @@ extension DocumentReference { String(swift_firebase.swift_cxx_shims.firebase.firestore.document_path(self)) } + // This variant is provided for compatibility with the ObjC API. public func getDocument(completion: @escaping (DocumentSnapshot?, Error?) -> Void) { let future = swift_firebase.swift_cxx_shims.firebase.firestore.document_get(self, .default) future.setCompletion({ let (snapshot, error) = future.resultAndError - completion(snapshot, error) + DispatchQueue.main.async { + completion(snapshot, error) + } }) } public func getDocument() async throws -> DocumentSnapshot { try await withCheckedThrowingContinuation { continuation in - getDocument(completion: { snapshot, error in + let future = swift_firebase.swift_cxx_shims.firebase.firestore.document_get(self, .default) + future.setCompletion({ + let (snapshot, error) = future.resultAndError if let error { continuation.resume(throwing: error) } else { diff --git a/Sources/FirebaseFirestore/Query+Swift.swift b/Sources/FirebaseFirestore/Query+Swift.swift index 9d17aa5..3592e01 100644 --- a/Sources/FirebaseFirestore/Query+Swift.swift +++ b/Sources/FirebaseFirestore/Query+Swift.swift @@ -6,6 +6,7 @@ import firebase import FirebaseCore import CxxShim +import Foundation public typealias Query = firebase.firestore.Query @@ -14,23 +15,28 @@ extension Query { swift_firebase.swift_cxx_shims.firebase.firestore.query_firestore(self) } + // This variant is provided for compatibility with the ObjC API. public func getDocuments(completion: @escaping (QuerySnapshot?, Error?) -> Void) { let future = swift_firebase.swift_cxx_shims.firebase.firestore.query_get(self, .default) future.setCompletion({ let (snapshot, error) = future.resultAndError - completion(snapshot, error) + DispatchQueue.main.async { + completion(snapshot, error) + } }) } public func getDocuments() async throws -> QuerySnapshot { try await withCheckedThrowingContinuation { continuation in - getDocuments() { snapshot, error in + let future = swift_firebase.swift_cxx_shims.firebase.firestore.query_get(self, .default) + future.setCompletion({ + let (snapshot, error) = future.resultAndError if let error { continuation.resume(throwing: error) } else { continuation.resume(returning: snapshot ?? .init()) } - } + }) } } } From 29ee3223dcb1251fa9c0a4af339c2d995209afa8 Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Tue, 13 Feb 2024 09:57:41 -0800 Subject: [PATCH 15/16] Change CallOnCompletion to OnCompletion --- Sources/FirebaseCore/FutureProtocol.swift | 4 ++-- Sources/firebase/include/FirebaseCore.hh | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Sources/FirebaseCore/FutureProtocol.swift b/Sources/FirebaseCore/FutureProtocol.swift index c5c31e6..342b551 100644 --- a/Sources/FirebaseCore/FutureProtocol.swift +++ b/Sources/FirebaseCore/FutureProtocol.swift @@ -20,7 +20,7 @@ public protocol FutureProtocol { func error() -> Int32 func __error_messageUnsafe() -> UnsafePointer? func __resultUnsafe() -> UnsafePointer? - func CallOnCompletion( + func OnCompletion( _ completion: FutureCompletionType, _ user_data: UnsafeMutableRawPointer? ) @@ -35,7 +35,7 @@ public extension FutureProtocol { // While the underlying Firebase `OnCompletion` method can provide a reference // back to the Future, we don't need to expose that here. func setCompletion(_ completion: @escaping () -> Void) { - CallOnCompletion({ ptr in + OnCompletion({ ptr in Unmanaged.fromOpaque(ptr!).takeRetainedValue().completion() }, Unmanaged.passRetained(CompletionReference(completion)).toOpaque()) } diff --git a/Sources/firebase/include/FirebaseCore.hh b/Sources/firebase/include/FirebaseCore.hh index f1ee6fc..a34956c 100644 --- a/Sources/firebase/include/FirebaseCore.hh +++ b/Sources/firebase/include/FirebaseCore.hh @@ -20,12 +20,13 @@ template class ConformingFuture: public ::firebase::Future { ConformingFuture(const FutureType& rhs) : FutureType(rhs) {} - void CallOnCompletion( + void OnCompletion( _Nonnull FutureCompletionType completion, _Nullable void* user_data) const { - OnCompletion([completion, user_data](const FutureBase&) { - completion(user_data); - }); + ::firebase::FutureBase::OnCompletion( + [completion, user_data](const FutureBase&) { + completion(user_data); + }); } } __attribute__((swift_attr("conforms_to:FirebaseCore.FutureProtocol"))); From 7eb88a4b19ff9bebc41caa7a60bf09612c45a18d Mon Sep 17 00:00:00 2001 From: Darin Fisher Date: Wed, 14 Feb 2024 10:02:14 -0800 Subject: [PATCH 16/16] updates per code review --- Sources/FirebaseAuth/FirebaseAuth+Swift.swift | 2 +- Sources/FirebaseAuth/FirebaseUser+Swift.swift | 2 +- Sources/FirebaseCore/FirebaseError.swift | 2 +- Sources/FirebaseCore/FutureProtocol.swift | 28 +++++++++++-------- .../DocumentReference+Swift.swift | 2 +- .../DocumentSnapshot+Swift.swift | 2 +- .../FirebaseFirestore/Firestore+Swift.swift | 2 +- .../ListenerRegistration.swift | 2 +- Sources/FirebaseFirestore/Query+Swift.swift | 2 +- .../FirebaseFirestore/Settings+Swift.swift | 2 +- Sources/firebase/include/FirebaseCore.hh | 9 +++--- Sources/firebase/include/FirebaseFirestore.hh | 6 ++-- 12 files changed, 33 insertions(+), 28 deletions(-) diff --git a/Sources/FirebaseAuth/FirebaseAuth+Swift.swift b/Sources/FirebaseAuth/FirebaseAuth+Swift.swift index 667acd9..bb00c08 100644 --- a/Sources/FirebaseAuth/FirebaseAuth+Swift.swift +++ b/Sources/FirebaseAuth/FirebaseAuth+Swift.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Internal) +@_spi(FirebaseInternal) import FirebaseCore import CxxShim diff --git a/Sources/FirebaseAuth/FirebaseUser+Swift.swift b/Sources/FirebaseAuth/FirebaseUser+Swift.swift index 1e9512b..b5d2d91 100644 --- a/Sources/FirebaseAuth/FirebaseUser+Swift.swift +++ b/Sources/FirebaseAuth/FirebaseUser+Swift.swift @@ -5,7 +5,7 @@ import Foundation @_exported import firebase -@_spi(Internal) +@_spi(FirebaseInternal) import FirebaseCore public typealias User = firebase.auth.User diff --git a/Sources/FirebaseCore/FirebaseError.swift b/Sources/FirebaseCore/FirebaseError.swift index 8b619ad..4a47c47 100644 --- a/Sources/FirebaseCore/FirebaseError.swift +++ b/Sources/FirebaseCore/FirebaseError.swift @@ -4,7 +4,7 @@ public struct FirebaseError: Error { public let code: CInt public let message: String - @_spi(Internal) + @_spi(FirebaseInternal) public init(code: CInt, message: String) { self.code = code self.message = message diff --git a/Sources/FirebaseCore/FutureProtocol.swift b/Sources/FirebaseCore/FutureProtocol.swift index 342b551..b1efafa 100644 --- a/Sources/FirebaseCore/FutureProtocol.swift +++ b/Sources/FirebaseCore/FutureProtocol.swift @@ -5,16 +5,16 @@ import firebase import Foundation -@_spi(Internal) +@_spi(FirebaseInternal) public typealias FutureCompletionType = swift_firebase.swift_cxx_shims.firebase.FutureCompletionType // This protocol enables extracting common code for Future handling. Because // C++ interop is limited for templated class types, we need to invent a // protocol to reflect the features of a Future that should be generically -// available. This works by having a C++ annotation (see ConformingFuture) -// that specifies this protocol conformance. -@_spi(Internal) +// available. This works by having a C++ annotation (see swift_cxx_shims' +// Future) that specifies this protocol conformance. +@_spi(FirebaseInternal) public protocol FutureProtocol { associatedtype ResultType func error() -> Int32 @@ -26,9 +26,7 @@ public protocol FutureProtocol { ) } -// A home for various helper functions that make it easier to work with -// Firebase Future objects from Swift. -@_spi(Internal) +@_spi(FirebaseInternal) public extension FutureProtocol { // Callsites retain their own reference to the Future, but they still need // a way to know when the Future completes. This provides that mechanism. @@ -40,15 +38,21 @@ public extension FutureProtocol { }, Unmanaged.passRetained(CompletionReference(completion)).toOpaque()) } - // A convenient way to access the result or error of a Future. Handles the - // messy details. + var result: ResultType? { + __resultUnsafe().pointee + } + + var errorMessage: String? { + guard let errorMessageUnsafe = __error_messageUnsafe() else { return nil } + return String(cString: errorMessageUnsafe) + } + var resultAndError: (ResultType?, Error?) { let error = error() guard error == 0 else { - let message = String(cString: __error_messageUnsafe()!) - return (nil, FirebaseError(code: error, message: message)) + return (nil, FirebaseError(code: error, message: errorMessage!)) } - return (__resultUnsafe().pointee, nil) + return (result, nil) } } diff --git a/Sources/FirebaseFirestore/DocumentReference+Swift.swift b/Sources/FirebaseFirestore/DocumentReference+Swift.swift index 3612c69..f77f488 100644 --- a/Sources/FirebaseFirestore/DocumentReference+Swift.swift +++ b/Sources/FirebaseFirestore/DocumentReference+Swift.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Internal) +@_spi(FirebaseInternal) import FirebaseCore import CxxShim diff --git a/Sources/FirebaseFirestore/DocumentSnapshot+Swift.swift b/Sources/FirebaseFirestore/DocumentSnapshot+Swift.swift index c10d02c..2a36675 100644 --- a/Sources/FirebaseFirestore/DocumentSnapshot+Swift.swift +++ b/Sources/FirebaseFirestore/DocumentSnapshot+Swift.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Internal) +@_spi(FirebaseInternal) import FirebaseCore import CxxShim diff --git a/Sources/FirebaseFirestore/Firestore+Swift.swift b/Sources/FirebaseFirestore/Firestore+Swift.swift index a283178..6f2dfd5 100644 --- a/Sources/FirebaseFirestore/Firestore+Swift.swift +++ b/Sources/FirebaseFirestore/Firestore+Swift.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Internal) +@_spi(FirebaseInternal) import FirebaseCore import CxxShim diff --git a/Sources/FirebaseFirestore/ListenerRegistration.swift b/Sources/FirebaseFirestore/ListenerRegistration.swift index c0564b9..792a931 100644 --- a/Sources/FirebaseFirestore/ListenerRegistration.swift +++ b/Sources/FirebaseFirestore/ListenerRegistration.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Internal) +@_spi(FirebaseInternal) import FirebaseCore import CxxShim diff --git a/Sources/FirebaseFirestore/Query+Swift.swift b/Sources/FirebaseFirestore/Query+Swift.swift index 3592e01..4030265 100644 --- a/Sources/FirebaseFirestore/Query+Swift.swift +++ b/Sources/FirebaseFirestore/Query+Swift.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Internal) +@_spi(FirebaseInternal) import FirebaseCore import CxxShim diff --git a/Sources/FirebaseFirestore/Settings+Swift.swift b/Sources/FirebaseFirestore/Settings+Swift.swift index d27da8e..d529ccd 100644 --- a/Sources/FirebaseFirestore/Settings+Swift.swift +++ b/Sources/FirebaseFirestore/Settings+Swift.swift @@ -2,7 +2,7 @@ @_exported import firebase -@_spi(Internal) +@_spi(FirebaseInternal) import FirebaseCore import CxxShim diff --git a/Sources/firebase/include/FirebaseCore.hh b/Sources/firebase/include/FirebaseCore.hh index a34956c..8427ffe 100644 --- a/Sources/firebase/include/FirebaseCore.hh +++ b/Sources/firebase/include/FirebaseCore.hh @@ -13,12 +13,13 @@ typedef void (*FutureCompletionType)(void*); // also provides a method to invoke `OnCompletion` in a way that works from // Swift. We can ignore the `FutureBase` param as the Swift caller can just // retain the Future as part of its closure. -template class ConformingFuture: public ::firebase::Future { +template +class __attribute__((swift_attr("conforms_to:FirebaseCore.FutureProtocol"))) + Future : public ::firebase::Future { public: typedef R ResultType; - typedef ::firebase::Future FutureType; - ConformingFuture(const FutureType& rhs) : FutureType(rhs) {} + Future(const ::firebase::Future& rhs) : ::firebase::Future(rhs) {} void OnCompletion( _Nonnull FutureCompletionType completion, @@ -28,7 +29,7 @@ template class ConformingFuture: public ::firebase::Future { completion(user_data); }); } -} __attribute__((swift_attr("conforms_to:FirebaseCore.FutureProtocol"))); +}; } // namespace swift_firebase::swift_cxx_shims::firebase diff --git a/Sources/firebase/include/FirebaseFirestore.hh b/Sources/firebase/include/FirebaseFirestore.hh index 9577b89..874aea8 100644 --- a/Sources/firebase/include/FirebaseFirestore.hh +++ b/Sources/firebase/include/FirebaseFirestore.hh @@ -45,7 +45,7 @@ document_path(const ::firebase::firestore::DocumentReference document) { return document.path(); } -inline ::swift_firebase::swift_cxx_shims::firebase::ConformingFuture< +inline ::swift_firebase::swift_cxx_shims::firebase::Future< ::firebase::firestore::DocumentSnapshot> document_get(const ::firebase::firestore::DocumentReference document, ::firebase::firestore::Source source = @@ -53,7 +53,7 @@ document_get(const ::firebase::firestore::DocumentReference document, return document.Get(source); } -inline ::swift_firebase::swift_cxx_shims::firebase::ConformingFuture +inline ::swift_firebase::swift_cxx_shims::firebase::Future document_set_data(::firebase::firestore::DocumentReference document, const ::firebase::firestore::MapFieldValue data, const ::firebase::firestore::SetOptions options) { @@ -176,7 +176,7 @@ query_firestore(::firebase::firestore::Query query) { return query.firestore(); } -inline ::swift_firebase::swift_cxx_shims::firebase::ConformingFuture< +inline ::swift_firebase::swift_cxx_shims::firebase::Future< ::firebase::firestore::QuerySnapshot> query_get(const ::firebase::firestore::Query query, ::firebase::firestore::Source source =