Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Auth] Use result type to wrap completion handlers internally #13563

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
27 changes: 17 additions & 10 deletions FirebaseAuth/Sources/Swift/Auth/Auth.swift
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,9 @@ extension Auth: AuthInterop {
Task {
do {
let response = try await AuthBackend.call(with: request)
Auth.wrapMainAsync(callback: completion, withParam: response.signinMethods, error: nil)
Auth.wrapMainAsync(callback: completion, with: .success(response.signinMethods))
} catch {
Auth.wrapMainAsync(callback: completion, withParam: nil, error: error)
Auth.wrapMainAsync(callback: completion, with: .failure(error))
}
}
}
Expand Down Expand Up @@ -1004,9 +1004,9 @@ extension Auth: AuthInterop {
let actionCodeInfo = ActionCodeInfo(withOperation: operation,
email: email,
newEmail: response.verifiedEmail)
Auth.wrapMainAsync(callback: completion, withParam: actionCodeInfo, error: nil)
Auth.wrapMainAsync(callback: completion, with: .success(actionCodeInfo))
} catch {
Auth.wrapMainAsync(callback: completion, withParam: nil, error: error)
Auth.wrapMainAsync(callback: completion, with: .failure(error))
}
}
}
Expand Down Expand Up @@ -2223,7 +2223,11 @@ extension Auth: AuthInterop {
((AuthDataResult?, Error?) -> Void)?) -> (AuthDataResult?, Error?) -> Void {
let authDataCallback: (((AuthDataResult?, Error?) -> Void)?, AuthDataResult?, Error?) -> Void =
{ callback, result, error in
Auth.wrapMainAsync(callback: callback, withParam: result, error: error)
if let result {
Auth.wrapMainAsync(callback: callback, with: .success(result))
} else if let error {
Auth.wrapMainAsync(callback: callback, with: .failure(error))
}
}
return { authResult, error in
if let error {
Expand Down Expand Up @@ -2260,11 +2264,14 @@ extension Auth: AuthInterop {
}

class func wrapMainAsync<T: Any>(callback: ((T?, Error?) -> Void)?,
withParam param: T?,
error: Error?) -> Void {
if let callback {
DispatchQueue.main.async {
callback(param, error)
with result: Result<T, Error>) -> Void {
guard let callback else { return }
DispatchQueue.main.async {
switch result {
case let .success(success):
callback(success, nil)
case let .failure(error):
callback(nil, error)
}
}
}
Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior changes made here map to this public API:

/// Verify ownership of the second factor phone number by the current user.
/// - Parameter phoneNumber: The phone number to be verified.
/// - Parameter uiDelegate: An object used to present the SFSafariViewController. The object is
/// retained by this method until the completion block is executed.
/// - Parameter multiFactorSession: A session to identify the MFA flow. For enrollment, this
/// identifies the user trying to enroll. For sign-in, this identifies that the user already
/// passed the first factor challenge.
/// - Returns: The verification ID
@available(iOS 13, tvOS 13, macOS 10.15, watchOS 8, *)
open func verifyPhoneNumber(_ phoneNumber: String,
uiDelegate: AuthUIDelegate? = nil,
multiFactorSession: MultiFactorSession? = nil) async throws
-> String {

Is there a valid success case where the returned String should be nil (or an empty string) and the error is also non-nil? This PR enforces that it is either one or the other (non-optional String OR error).

Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ import Foundation
uiDelegate: uiDelegate,
multiFactorSession: multiFactorSession
)
Auth.wrapMainAsync(callback: completion, withParam: verificationID, error: nil)
Auth.wrapMainAsync(callback: completion, with: .success(verificationID ?? ""))
ncooke3 marked this conversation as resolved.
Show resolved Hide resolved
} catch {
Auth.wrapMainAsync(callback: completion, withParam: nil, error: error)
Auth.wrapMainAsync(callback: completion, with: .failure(error))
}
}
}
Expand Down
Loading