replacing FSTGetTokenResult by C++ Token implementation (firebase#805)

zxu123 · web-flow · commit 7a4a2ea10844 · 2018-02-20T12:25:39.000-05:00
* replacing Auth/FSTUser by C++ auth implementation

* address changes

* replacing FSTGetTokenResult by C++ Token implementation

* address changes

* address changes

* fix another const&amp; v.s. dispatch bug

* fix more const&amp; v.s. dispatch bug  zxu123 committed

* fix

* passing by value in callback
diff --git a/Firestore/Source/Auth/FSTCredentialsProvider.h b/Firestore/Source/Auth/FSTCredentialsProvider.h
@@ -16,53 +16,27 @@
 
 #import <Foundation/Foundation.h>
 
+#include "Firestore/core/src/firebase/firestore/auth/token.h"
 #include "Firestore/core/src/firebase/firestore/auth/user.h"
 
 NS_ASSUME_NONNULL_BEGIN
 
 @class FIRApp;
 @class FSTDispatchQueue;
 
-#pragma mark - FSTGetTokenResult
-
-/**
- * The current User and the authentication token provided by the underlying authentication
- * mechanism. This is the result of calling -[FSTCredentialsProvider getTokenForcingRefresh].
- *
- * ## Portability notes: no TokenType on iOS
- *
- * The TypeScript client supports 1st party Oauth tokens (for the Firebase Console to auth as the
- * developer) and OAuth2 tokens for the node.js sdk to auth with a service account. We don't have
- * plans to support either case on mobile so there's no TokenType here.
- */
-// TODO(mcg): Rename FSTToken, change parameter order to line up with the other platforms.
-@interface FSTGetTokenResult : NSObject
-
-- (instancetype)init NS_UNAVAILABLE;
-- (instancetype)initWithUser:(const firebase::firestore::auth::User &)user
-                       token:(NSString *_Nullable)token NS_DESIGNATED_INITIALIZER;
-
-/** The user with which the token is associated (used for persisting user state on disk, etc.). */
-@property(nonatomic, assign, readonly) const firebase::firestore::auth::User &user;
-
-/** The actual raw token. */
-@property(nonatomic, copy, nullable, readonly) NSString *token;
-
-@end
-
 #pragma mark - Typedefs
 
 /**
  * `FSTVoidTokenErrorBlock` is a block that gets a token or an error.
  *
- * @param token An auth token as a string.
+ * @param token An auth token, either valid or invalid when error occurred.
  * @param error The error if one occurred, or else `nil`.
  */
-typedef void (^FSTVoidGetTokenResultBlock)(FSTGetTokenResult *_Nullable token,
+typedef void (^FSTVoidGetTokenResultBlock)(firebase::firestore::auth::Token token,
                                            NSError *_Nullable error);
 
 /** Listener block notified with a User. */
-typedef void (^FSTVoidUserBlock)(const firebase::firestore::auth::User &user);
+typedef void (^FSTVoidUserBlock)(firebase::firestore::auth::User user);
 
 #pragma mark - FSTCredentialsProvider
 
diff --git a/Firestore/Source/Auth/FSTCredentialsProvider.mm b/Firestore/Source/Auth/FSTCredentialsProvider.mm
@@ -25,38 +25,16 @@
 #import "Firestore/Source/Util/FSTClasses.h"
 #import "Firestore/Source/Util/FSTDispatchQueue.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/string_apple.h"
 
 namespace util = firebase::firestore::util;
+using firebase::firestore::auth::Token;
 using firebase::firestore::auth::User;
 
 NS_ASSUME_NONNULL_BEGIN
 
-#pragma mark - FSTGetTokenResult
-
-@interface FSTGetTokenResult () {
-  User _user;
-}
-
-@end
-
-@implementation FSTGetTokenResult
-
-- (instancetype)initWithUser:(const User &)user token:(NSString *_Nullable)token {
-  if (self = [super init]) {
-    _user = user;
-    _token = token;
-  }
-  return self;
-}
-
-- (const User &)user {
-  return _user;
-}
-
-@end
-
 #pragma mark - FSTFirebaseCredentialsProvider
 @interface FSTFirebaseCredentialsProvider () {
   /** The current user as reported to us via our AuthStateDidChangeListener. */
@@ -141,11 +119,9 @@ - (void)getTokenForcingRefresh:(BOOL)forceRefresh
             NSError *cancelError = [NSError errorWithDomain:FIRFirestoreErrorDomain
                                                        code:FIRFirestoreErrorCodeAborted
                                                    userInfo:errorInfo];
-            completion(nil, cancelError);
+            completion(Token::Invalid(), cancelError);
           } else {
-            FSTGetTokenResult *result =
-                [[FSTGetTokenResult alloc] initWithUser:_currentUser token:token];
-            completion(result, error);
+            completion(Token(util::MakeStringView(token), _currentUser), error);
           }
         };
       };
diff --git a/Firestore/Source/Auth/FSTEmptyCredentialsProvider.mm b/Firestore/Source/Auth/FSTEmptyCredentialsProvider.mm
@@ -19,8 +19,10 @@
 #import "Firestore/Source/Util/FSTAssert.h"
 #import "Firestore/Source/Util/FSTDispatchQueue.h"
 
+#include "Firestore/core/src/firebase/firestore/auth/token.h"
 #include "Firestore/core/src/firebase/firestore/auth/user.h"
 
+using firebase::firestore::auth::Token;
 using firebase::firestore::auth::User;
 
 NS_ASSUME_NONNULL_BEGIN
@@ -29,7 +31,8 @@ @implementation FSTEmptyCredentialsProvider
 
 - (void)getTokenForcingRefresh:(BOOL)forceRefresh
                     completion:(FSTVoidGetTokenResultBlock)completion {
-  completion(nil, nil);
+  // Invalid token will force the GRPC fallback to use default settings.
+  completion(Token::Invalid(), nil);
 }
 
 - (void)setUserChangeListener:(nullable FSTVoidUserBlock)block {
diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm
@@ -103,7 +103,7 @@ - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
     __block bool initialized = false;
     __block User initialUser;
     FSTWeakify(self);
-    _credentialsProvider.userChangeListener = ^(const User &user) {
+    _credentialsProvider.userChangeListener = ^(User user) {
       FSTStrongify(self);
       if (self) {
         if (!initialized) {
diff --git a/Firestore/Source/Remote/FSTDatastore.h b/Firestore/Source/Remote/FSTDatastore.h
@@ -20,6 +20,7 @@
 
 #include "Firestore/core/src/firebase/firestore/core/database_info.h"
 #include "Firestore/core/src/firebase/firestore/model/database_id.h"
+#include "absl/strings/string_view.h"
 
 @class FSTDocumentKey;
 @class FSTDispatchQueue;
@@ -83,7 +84,7 @@ NS_ASSUME_NONNULL_BEGIN
 /** Adds headers to the RPC including any OAuth access token if provided .*/
 + (void)prepareHeadersForRPC:(GRPCCall *)rpc
                   databaseID:(const firebase::firestore::model::DatabaseId *)databaseID
-                       token:(nullable NSString *)token;
+                       token:(const absl::string_view)token;
 
 /** Looks up a list of documents in datastore. */
 - (void)lookupDocuments:(NSArray<FSTDocumentKey *> *)keys
diff --git a/Firestore/Source/Remote/FSTDatastore.mm b/Firestore/Source/Remote/FSTDatastore.mm
@@ -35,11 +35,13 @@
 
 #import "Firestore/Protos/objc/google/firestore/v1beta1/Firestore.pbrpc.h"
 
+#include "Firestore/core/src/firebase/firestore/auth/token.h"
 #include "Firestore/core/src/firebase/firestore/core/database_info.h"
 #include "Firestore/core/src/firebase/firestore/model/database_id.h"
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 
 namespace util = firebase::firestore::util;
+using firebase::firestore::auth::Token;
 using firebase::firestore::core::DatabaseInfo;
 using firebase::firestore::model::DatabaseId;
 
@@ -301,16 +303,18 @@ - (void)invokeRPCWithFactory:(GRPCProtoCall * (^)(void))rpcFactory
   // but I'm not sure how to detect that right now. http://b/32762461
   [self.credentials
       getTokenForcingRefresh:NO
-                  completion:^(FSTGetTokenResult *_Nullable result, NSError *_Nullable error) {
+                  completion:^(Token result, NSError *_Nullable error) {
                     error = [FSTDatastore firestoreErrorForError:error];
                     [self.workerDispatchQueue dispatchAsyncAllowingSameQueue:^{
                       if (error) {
                         errorHandler(error);
                       } else {
                         GRPCProtoCall *rpc = rpcFactory();
-                        [FSTDatastore prepareHeadersForRPC:rpc
-                                                databaseID:&self.databaseInfo->database_id()
-                                                     token:result.token];
+                        [FSTDatastore
+                            prepareHeadersForRPC:rpc
+                                      databaseID:&self.databaseInfo->database_id()
+                                           token:(result.is_valid() ? result.token()
+                                                                    : absl::string_view())];
                         [rpc start];
                       }
                     }];
@@ -334,8 +338,8 @@ - (FSTWriteStream *)createWriteStream {
 /** Adds headers to the RPC including any OAuth access token if provided .*/
 + (void)prepareHeadersForRPC:(GRPCCall *)rpc
                   databaseID:(const DatabaseId *)databaseID
-                       token:(nullable NSString *)token {
-  rpc.oauth2AccessToken = token;
+                       token:(const absl::string_view)token {
+  rpc.oauth2AccessToken = token.data() == nullptr ? nil : util::WrapNSString(token);
   rpc.requestHeaders[kXGoogAPIClientHeader] = [FSTDatastore googAPIClientHeaderValue];
   // This header is used to improve routing and project isolation by the backend.
   rpc.requestHeaders[kGoogleCloudResourcePrefix] =
diff --git a/Firestore/Source/Remote/FSTStream.mm b/Firestore/Source/Remote/FSTStream.mm
@@ -35,11 +35,13 @@
 
 #import "Firestore/Protos/objc/google/firestore/v1beta1/Firestore.pbrpc.h"
 
+#include "Firestore/core/src/firebase/firestore/auth/token.h"
 #include "Firestore/core/src/firebase/firestore/core/database_info.h"
 #include "Firestore/core/src/firebase/firestore/model/database_id.h"
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 
 namespace util = firebase::firestore::util;
+using firebase::firestore::auth::Token;
 using firebase::firestore::core::DatabaseInfo;
 using firebase::firestore::model::DatabaseId;
 
@@ -256,18 +258,17 @@ - (void)startWithDelegate:(id)delegate {
   FSTAssert(_delegate == nil, @"Delegate must be nil");
   _delegate = delegate;
 
-  [self.credentials
-      getTokenForcingRefresh:NO
-                  completion:^(FSTGetTokenResult *_Nullable result, NSError *_Nullable error) {
-                    error = [FSTDatastore firestoreErrorForError:error];
-                    [self.workerDispatchQueue dispatchAsyncAllowingSameQueue:^{
-                      [self resumeStartWithToken:result error:error];
-                    }];
-                  }];
+  [self.credentials getTokenForcingRefresh:NO
+                                completion:^(Token result, NSError *_Nullable error) {
+                                  error = [FSTDatastore firestoreErrorForError:error];
+                                  [self.workerDispatchQueue dispatchAsyncAllowingSameQueue:^{
+                                    [self resumeStartWithToken:result error:error];
+                                  }];
+                                }];
 }
 
 /** Add an access token to our RPC, after obtaining one from the credentials provider. */
-- (void)resumeStartWithToken:(FSTGetTokenResult *)token error:(NSError *)error {
+- (void)resumeStartWithToken:(const Token &)token error:(NSError *)error {
   if (self.state == FSTStreamStateStopped) {
     // Streams can be stopped while waiting for authorization.
     return;
@@ -289,7 +290,7 @@ - (void)resumeStartWithToken:(FSTGetTokenResult *)token error:(NSError *)error {
   _rpc = [self createRPCWithRequestsWriter:self.requestsWriter];
   [FSTDatastore prepareHeadersForRPC:_rpc
                           databaseID:&self.databaseInfo->database_id()
-                               token:token.token];
+                               token:(token.is_valid() ? token.token() : absl::string_view())];
   FSTAssert(_callbackFilter == nil, @"GRX Filter must be nil");
   _callbackFilter = [[FSTCallbackFilter alloc] initWithStream:self];
   [_rpc startWithWriteable:_callbackFilter];
diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h
@@ -31,11 +31,11 @@ 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: The error if one occurred, or else nullptr.
-typedef std::function<void(const Token& token, const absl::string_view error)>
+typedef std::function<void(Token token, const absl::string_view error)>
     TokenListener;
 
 // Listener notified with a User change.
-typedef std::function<void(const User& user)> UserChangeListener;
+typedef std::function<void(User user)> UserChangeListener;
 
 /**
  * Provides methods for getting the uid and token for the current user and
diff --git a/Firestore/core/src/firebase/firestore/auth/token.cc b/Firestore/core/src/firebase/firestore/auth/token.cc
@@ -21,7 +21,15 @@ namespace firestore {
 namespace auth {
 
 Token::Token(const absl::string_view token, const User& user)
-    : token_(token), user_(user) {
+    : token_(token), user_(user), is_valid_(true) {
+}
+
+Token::Token() : token_(), user_(User::Unauthenticated()), is_valid_(false) {
+}
+
+const Token& Token::Invalid() {
+  static const Token kInvalidToken;
+  return kInvalidToken;
 }
 
 }  // namespace auth
diff --git a/Firestore/core/src/firebase/firestore/auth/token.h b/Firestore/core/src/firebase/firestore/auth/token.h
@@ -20,6 +20,7 @@
 #include <string>
 
 #include "Firestore/core/src/firebase/firestore/auth/user.h"
+#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h"
 #include "absl/strings/string_view.h"
 
 namespace firebase {
@@ -45,6 +46,7 @@ class Token {
 
   /** The actual raw token. */
   const std::string& token() const {
+    FIREBASE_ASSERT(is_valid_);
     return token_;
   }
 
@@ -56,9 +58,26 @@ class Token {
     return user_;
   }
 
+  /**
+   * Whether the token is a valid one.
+   *
+   * ## 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.
+   */
+  bool is_valid() const {
+    return is_valid_;
+  }
+
+  /** Returns an invalid token. */
+  static const Token& Invalid();
+
  private:
+  Token();
+
   const std::string token_;
   const User user_;
+  const bool is_valid_;
 };
 
 }  // namespace auth
diff --git a/Firestore/core/src/firebase/firestore/util/string_apple.h b/Firestore/core/src/firebase/firestore/util/string_apple.h
@@ -42,7 +42,16 @@ inline NSString* WrapNSStringNoCopy(const absl::string_view str) {
   return WrapNSStringNoCopy(str.data());
 }
 
-// Creates an absl::string_view wrapper for the contents of the given NSString.
+// Translates a string_view string to the equivalent NSString by making a copy.
+inline NSString* WrapNSString(const absl::string_view str) {
+  return [[NSString alloc]
+      initWithBytes:const_cast<void*>(static_cast<const void*>(str.data()))
+             length:str.length()
+           encoding:NSUTF8StringEncoding];
+}
+
+// Creates an absl::string_view wrapper for the contents of the given
+// NSString.
 inline absl::string_view MakeStringView(NSString* str) {
   return absl::string_view(
       [str UTF8String], [str lengthOfBytesUsingEncoding:NSUTF8StringEncoding]);
diff --git a/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cc b/Firestore/core/test/firebase/firestore/auth/credentials_provider_test.cc
@@ -25,7 +25,7 @@ namespace auth {
 #define UNUSED(x) (void)(x)
 
 TEST(CredentialsProvider, Typedef) {
-  TokenListener token_listener = [](const Token& token,
+  TokenListener token_listener = [](Token token,
                                     const absl::string_view error) {
     UNUSED(token);
     UNUSED(error);
@@ -37,9 +37,7 @@ TEST(CredentialsProvider, Typedef) {
   EXPECT_EQ(nullptr, token_listener);
   EXPECT_FALSE(token_listener);
 
-  UserChangeListener user_change_listener = [](const User& user) {
-    UNUSED(user);
-  };
+  UserChangeListener user_change_listener = [](User user) { UNUSED(user); };
   EXPECT_NE(nullptr, user_change_listener);
   EXPECT_TRUE(user_change_listener);
 
diff --git a/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc b/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc
@@ -25,8 +25,7 @@ namespace auth {
 TEST(EmptyCredentialsProvider, GetToken) {
   EmptyCredentialsProvider credentials_provider;
   credentials_provider.GetToken(
-      /*force_refresh=*/true,
-      [](const Token& token, const absl::string_view error) {
+      /*force_refresh=*/true, [](Token token, const absl::string_view error) {
         EXPECT_EQ("", token.token());
         const User& user = token.user();
         EXPECT_EQ("", user.uid());
@@ -37,7 +36,7 @@ TEST(EmptyCredentialsProvider, GetToken) {
 
 TEST(EmptyCredentialsProvider, SetListener) {
   EmptyCredentialsProvider credentials_provider;
-  credentials_provider.SetUserChangeListener([](const User& user) {
+  credentials_provider.SetUserChangeListener([](User user) {
     EXPECT_EQ("", user.uid());
     EXPECT_FALSE(user.is_authenticated());
   });
diff --git a/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm b/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm
diff --git a/Firestore/core/test/firebase/firestore/auth/token_test.cc b/Firestore/core/test/firebase/firestore/auth/token_test.cc
