Change CredentialsProvider::TokenListener to use StatusOr<Token>

Firestore/Source/Remote/FSTDatastore.mm

@@ -305,20 +305,18 @@ - (void)invokeRPCWithFactory:(GRPCProtoCall * (^)(void))rpcFactory
   // TODO(mikelehen): We should force a refresh if the previous RPC failed due to an expired token,
   // but I'm not sure how to detect that right now. http://b/32762461
   _credentials->GetToken(
-      /*force_refresh=*/false,
-      [self, rpcFactory, errorHandler](Token result, const int64_t error_code,
-                                       const absl::string_view error_msg) {
-        NSError *error = util::WrapNSError(error_code, error_msg);
+      /*force_refresh=*/false, [self, rpcFactory, errorHandler](util::StatusOr<Token> result) {
         [self.workerDispatchQueue dispatchAsyncAllowingSameQueue:^{
-          if (error) {
-            errorHandler(error);
+          if (!result.ok()) {
+            errorHandler(util::MakeNSError(result.status()));
           } else {
             GRPCProtoCall *rpc = rpcFactory();
+            const Token &token = result.ValueOrDie();
             [FSTDatastore
                 prepareHeadersForRPC:rpc
                           databaseID:&self.databaseInfo->database_id()
-                               token:(result.user().is_authenticated() ? result.token()
-                                                                       : absl::string_view())];
+                               token:(token.user().is_authenticated() ? token.token()
+                                                                      : absl::string_view())];
             [rpc start];
           }
         }];

Firestore/Source/Remote/FSTStream.mm

@@ -265,17 +265,15 @@ - (void)startWithDelegate:(id)delegate {
   _delegate = delegate;
 
   _credentials->GetToken(
-      /*force_refresh=*/false,
-      [self](Token result, const int64_t error_code, const absl::string_view error_msg) {
-        NSError *error = util::WrapNSError(error_code, error_msg);
+      /*force_refresh=*/false, [self](util::StatusOr<Token> result) {
         [self.workerDispatchQueue dispatchAsyncAllowingSameQueue:^{
-          [self resumeStartWithToken:result error:error];
+          [self resumeStartWithToken:result];
         }];
       });
 }
 
 /** Add an access token to our RPC, after obtaining one from the credentials provider. */
-- (void)resumeStartWithToken:(const Token &)token error:(NSError *)error {
+- (void)resumeStartWithToken:(const util::StatusOr<Token> &)result {
   [self.workerDispatchQueue verifyIsCurrentQueue];
 
   if (self.state == FSTStreamStateStopped) {
@@ -287,16 +285,17 @@ - (void)resumeStartWithToken:(const Token &)token error:(NSError *)error {
 
   // TODO(mikelehen): We should force a refresh if the previous RPC failed due to an expired token,
   // but I'm not sure how to detect that right now. http://b/32762461
-  if (error) {
+  if (!result.ok()) {
     // RPC has not been started yet, so just invoke higher-level close handler.
-    [self handleStreamClose:error];
+    [self handleStreamClose:util::MakeNSError(result.status())];
     return;
   }
 
   self.requestsWriter = [[FSTBufferedWriter alloc] init];
   _rpc = [self createRPCWithRequestsWriter:self.requestsWriter];
   [_rpc setResponseDispatchQueue:self.workerDispatchQueue.queue];
 
+  const Token &token = result.ValueOrDie();
   [FSTDatastore
       prepareHeadersForRPC:_rpc
                 databaseID:&self.databaseInfo->database_id()

Firestore/core/src/firebase/firestore/auth/credentials_provider.h

@@ -23,19 +23,15 @@
 #include "Firestore/core/include/firebase/firestore/firestore_errors.h"
 #include "Firestore/core/src/firebase/firestore/auth/token.h"
 #include "Firestore/core/src/firebase/firestore/auth/user.h"
+#include "Firestore/core/src/firebase/firestore/util/statusor.h"
 #include "absl/strings/string_view.h"
 
 namespace firebase {
 namespace firestore {
 namespace auth {
 
 // `TokenErrorListener` is a listener that gets a token or an error.
-// token: An auth token as a string, or nullptr if error occurred.
-// error_code: The error code if one occurred, or else FirestoreErrorCode::Ok.
-// error_msg: The error if one occurred, or else nullptr.
-typedef std::function<void(
-    Token token, const int64_t error_code, const absl::string_view error_msg)>
-    TokenListener;
+typedef std::function<void(util::StatusOr<Token>)> TokenListener;
 
 // Listener notified with a User change.
 typedef std::function<void(User user)> UserChangeListener;

Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc

@@ -26,8 +26,9 @@ void EmptyCredentialsProvider::GetToken(bool force_refresh,
                                         TokenListener completion) {
   UNUSED(force_refresh);
   if (completion) {
-    // Invalid token will force the GRPC fallback to use default settings.
-    completion(Token::Invalid(), FirestoreErrorCode::Ok, "");
+    // Unauthenticated token will force the GRPC fallback to use default
+    // settings.
+    completion(Token::Unauthenticated());
   }
 }
 

Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm

@@ -23,6 +23,9 @@
 #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h"
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 
+// NB: This is also defined in Firestore/Source/Public/FIRFirestoreErrors.h
+NSString* const FIRFirestoreErrorDomain = @"FIRFirestoreErrorDomain";
+
 namespace firebase {
 namespace firestore {
 namespace auth {
@@ -84,27 +87,38 @@
   int initial_user_counter = contents_->user_counter;
 
   std::weak_ptr<Contents> weak_contents = contents_;
-  void (^get_token_callback)(NSString*, NSError*) = ^(
-      NSString* _Nullable token, NSError* _Nullable error) {
-    std::shared_ptr<Contents> contents = weak_contents.lock();
-    if (!contents) {
-      return;
-    }
-
-    std::unique_lock<std::mutex> lock(contents->mutex);
-    if (initial_user_counter != contents->user_counter) {
-      // Cancel the request since the user changed while the request was
-      // outstanding so the response is likely for a previous user (which
-      // user, we can't be sure).
-      completion(Token::Invalid(), FirestoreErrorCode::Aborted,
-                 "getToken aborted due to user change.");
-    } else {
-      completion(
-          Token{util::MakeStringView(token), contents->current_user},
-          error == nil ? FirestoreErrorCode::Ok : error.code,
-          error == nil ? "" : util::MakeStringView(error.localizedDescription));
-    }
-  };
+  void (^get_token_callback)(NSString*, NSError*) =
+      ^(NSString* _Nullable token, NSError* _Nullable error) {
+        std::shared_ptr<Contents> contents = weak_contents.lock();
+        if (!contents) {
+          return;
+        }
+
+        std::unique_lock<std::mutex> lock(contents->mutex);
+        if (initial_user_counter != contents->user_counter) {
+          // Cancel the request since the user changed while the request was
+          // outstanding so the response is likely for a previous user (which
+          // user, we can't be sure).
+          completion(util::Status(FirestoreErrorCode::Aborted,
+                                  "getToken aborted due to user change."));
+        } else {
+          if (error == nil) {
+            if (token != nil) {
+              completion(
+                  Token{util::MakeStringView(token), contents->current_user});
+            } else {
+              completion(Token::Unauthenticated());
+            }
+          } else {
+            FirestoreErrorCode error_code = FirestoreErrorCode::Unknown;
+            if (error.domain == FIRFirestoreErrorDomain) {
+              error_code = static_cast<FirestoreErrorCode>(error.code);
+            }
+            completion(util::Status(
+                error_code, util::MakeStringView(error.localizedDescription)));
+          }
+        }
+      };
 
   [contents_->app getTokenForcingRefresh:force_refresh
                             withCallback:get_token_callback];

Firestore/core/src/firebase/firestore/auth/token.cc

@@ -21,15 +21,13 @@ namespace firestore {
 namespace auth {
 
 Token::Token(const absl::string_view token, const User& user)
-    : token_(token), user_(user), is_valid_(true) {
+    : token_(token), user_(user) {
 }
 
-Token::Token() : token_(), user_(User::Unauthenticated()), is_valid_(false) {
-}
-
-const Token& Token::Invalid() {
-  static const Token kInvalidToken;
-  return kInvalidToken;
+const Token& Token::Unauthenticated() {
+  static const Token kUnauthenticatedToken(absl::string_view(),
+                                           User::Unauthenticated());
+  return kUnauthenticatedToken;
 }
 
 }  // namespace auth

Firestore/core/src/firebase/firestore/auth/token.h

@@ -46,7 +46,7 @@ class Token {
 
   /** The actual raw token. */
   const std::string& token() const {
-    FIREBASE_ASSERT(is_valid_);
+    FIREBASE_ASSERT(user_.is_authenticated());
     return token_;
   }
 
@@ -59,25 +59,17 @@ class Token {
   }
 
   /**
-   * Whether the token is a valid one.
+   * Returns a token for an unauthenticated user.
    *
-   * ## Portability notes: Invalid token is the equivalent of nil in the iOS
-   * token implementation. We use value instead of pointer for Token instance in
-   * the C++ migration.
+   * ## Portability notes: An unauthenticated token is the equivalent of
+   * nil/null in the iOS/TypeScript token implementation. We use a reference
+   * instead of a pointer for Token instances in the C++ migration.
    */
-  bool is_valid() const {
-    return is_valid_;
-  }
-
-  /** Returns an invalid token. */
-  static const Token& Invalid();
+  static const Token& Unauthenticated();
 
  private:
-  Token();
-
   const std::string token_;
   const User user_;
-  const bool is_valid_;
 };
 
 }  // namespace auth

Firestore/core/src/firebase/firestore/util/error_apple.h

@@ -24,6 +24,7 @@
 
 #include "Firestore/Source/Public/FIRFirestoreErrors.h"  // for FIRFirestoreErrorDomain
 #include "Firestore/core/include/firebase/firestore/firestore_errors.h"
+#include "Firestore/core/src/firebase/firestore/util/status.h"
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 #include "absl/strings/string_view.h"
 
@@ -32,7 +33,7 @@ namespace firestore {
 namespace util {
 
 // Translates a set of error_code and error_msg to an NSError.
-inline NSError* WrapNSError(const int64_t error_code,
+inline NSError* MakeNSError(const int64_t error_code,
                             const absl::string_view error_msg) {
   if (error_code == FirestoreErrorCode::Ok) {
     return nil;
@@ -43,6 +44,10 @@ inline NSError* WrapNSError(const int64_t error_code,
              userInfo:@{NSLocalizedDescriptionKey : WrapNSString(error_msg)}];
 }
 
+inline NSError* MakeNSError(const util::Status& status) {
+  return MakeNSError(status.code(), status.error_message());
+}
+
 }  // namespace util
 }  // namespace firestore
 }  // namespace firebase

Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cc

@@ -16,6 +16,7 @@
 
 #include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h"
 
+#include "Firestore/core/src/firebase/firestore/util/statusor.h"
 #include "gtest/gtest.h"
 
 namespace firebase {
@@ -25,11 +26,8 @@ namespace auth {
 #define UNUSED(x) (void)(x)
 
 TEST(CredentialsProvider, Typedef) {
-  TokenListener token_listener = [](Token token, const int64_t error_code,
-                                    const absl::string_view error_msg) {
+  TokenListener token_listener = [](util::StatusOr<Token> token) {
     UNUSED(token);
-    UNUSED(error_code);
-    UNUSED(error_msg);
   };
   EXPECT_NE(nullptr, token_listener);
   EXPECT_TRUE(token_listener);

Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc

@@ -16,6 +16,7 @@
 
 #include "Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h"
 
+#include "Firestore/core/src/firebase/firestore/util/statusor.h"
 #include "gtest/gtest.h"
 
 namespace firebase {
@@ -25,14 +26,13 @@ namespace auth {
 TEST(EmptyCredentialsProvider, GetToken) {
   EmptyCredentialsProvider credentials_provider;
   credentials_provider.GetToken(
-      /*force_refresh=*/true, [](Token token, const int64_t error_code,
-                                 const absl::string_view error_msg) {
-        EXPECT_FALSE(token.is_valid());
+      /*force_refresh=*/true, [](util::StatusOr<Token> result) {
+        EXPECT_TRUE(result.ok());
+        const Token& token = result.ValueOrDie();
+        EXPECT_ANY_THROW(token.token());
         const User& user = token.user();
         EXPECT_EQ("", user.uid());
         EXPECT_FALSE(user.is_authenticated());
-        EXPECT_EQ(FirestoreErrorCode::Ok, error_code);
-        EXPECT_EQ("", error_msg);
       });
 }
 

Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm

@@ -17,9 +17,11 @@
 #include "Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h"
 
 #import <FirebaseCore/FIRApp.h>
+
 #import <FirebaseCore/FIRAppInternal.h>
 #import <FirebaseCore/FIROptionsInternal.h>
 
+#include "Firestore/core/src/firebase/firestore/util/statusor.h"
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 #include "Firestore/core/test/firebase/firestore/testutil/app_testing.h"
 
@@ -29,43 +31,49 @@
 namespace firestore {
 namespace auth {
 
-FIRApp* AppWithFakeUid(NSString* _Nullable uid) {
+FIRApp* AppWithFakeUidAndToken(NSString* _Nullable uid,
+                               NSString* _Nullable token) {
   FIRApp* app = testutil::AppForUnitTesting();
   app.getUIDImplementation = ^NSString* {
     return uid;
   };
+  app.getTokenImplementation = ^(BOOL, FIRTokenCallback callback) {
+    callback(token, nil);
+  };
   return app;
 }
 
+FIRApp* AppWithFakeUid(NSString* _Nullable uid) {
+  return AppWithFakeUidAndToken(uid, uid == nil ? nil : @"default token");
+}
+
 TEST(FirebaseCredentialsProviderTest, GetTokenUnauthenticated) {
   FIRApp* app = AppWithFakeUid(nil);
 
   FirebaseCredentialsProvider credentials_provider(app);
   credentials_provider.GetToken(
-      /*force_refresh=*/true, [](Token token, const int64_t error_code,
-                                 const absl::string_view error_msg) {
-        EXPECT_EQ("", token.token());
+      /*force_refresh=*/true, [](util::StatusOr<Token> result) {
+        EXPECT_TRUE(result.ok());
+        const Token& token = result.ValueOrDie();
+        EXPECT_ANY_THROW(token.token());
         const User& user = token.user();
         EXPECT_EQ("", user.uid());
         EXPECT_FALSE(user.is_authenticated());
-        EXPECT_EQ(FirestoreErrorCode::Ok, error_code) << error_code;
-        EXPECT_EQ("", error_msg) << error_msg;
       });
 }
 
 TEST(FirebaseCredentialsProviderTest, GetToken) {
-  FIRApp* app = AppWithFakeUid(@"fake uid");
+  FIRApp* app = AppWithFakeUidAndToken(@"fake uid", @"token for fake uid");
 
   FirebaseCredentialsProvider credentials_provider(app);
   credentials_provider.GetToken(
-      /*force_refresh=*/true, [](Token token, const int64_t error_code,
-                                 const absl::string_view error_msg) {
-        EXPECT_EQ("", token.token());
+      /*force_refresh=*/true, [](util::StatusOr<Token> result) {
+        EXPECT_TRUE(result.ok());
+        const Token& token = result.ValueOrDie();
+        EXPECT_EQ("token for fake uid", token.token());
         const User& user = token.user();
         EXPECT_EQ("fake uid", user.uid());
         EXPECT_TRUE(user.is_authenticated());
-        EXPECT_EQ(FirestoreErrorCode::Ok, error_code) << error_code;
-        EXPECT_EQ("", error_msg) << error_msg;
       });
 }
 

Firestore/core/test/firebase/firestore/auth/token_test.cc

@@ -26,14 +26,12 @@ TEST(Token, Getter) {
   Token token("token", User("abc"));
   EXPECT_EQ("token", token.token());
   EXPECT_EQ(User("abc"), token.user());
-  EXPECT_TRUE(token.is_valid());
 }
 
-TEST(Token, InvalidToken) {
-  const Token& token = Token::Invalid();
+TEST(Token, UnauthenticatedToken) {
+  const Token& token = Token::Unauthenticated();
   EXPECT_ANY_THROW(token.token());
   EXPECT_EQ(User::Unauthenticated(), token.user());
-  EXPECT_FALSE(token.is_valid());
 }
 
 }  // namespace auth