firebase-ios-sdk/1499: Restart streams on any token change. (#1692)

[Port of firebase/firebase-js-sdk#1120]

Fixes #1499.

This reworks our "user listener" into a "credential change listener" that
fires on any token change, even if the User did not change. This allows
us to restart our streams (but not switch mutation queues, etc.) on token
changes.

Author: mikelehen

Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm

@@ -250,7 +250,7 @@ - (void)runTimer:(FSTTimerID)timerID {
 - (void)changeUser:(const User &)user {
   _currentUser = user;
   [self.dispatchQueue dispatchSync:^{
-    [self.syncEngine userDidChange:user];
+    [self.syncEngine credentialDidChangeWithUser:user];
   }];
 }
 

Firestore/Source/Core/FSTFirestoreClient.mm

@@ -128,8 +128,8 @@ - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
     auto userPromise = std::make_shared<std::promise<User>>();
 
     __weak typeof(self) weakSelf = self;
-    auto userChangeListener = [initialized = false, userPromise, weakSelf,
-                               workerDispatchQueue](User user) mutable {
+    auto credentialChangeListener = [initialized = false, userPromise, weakSelf,
+                                     workerDispatchQueue](User user) mutable {
       typeof(self) strongSelf = weakSelf;
       if (!strongSelf) return;
 
@@ -138,14 +138,14 @@ - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
         userPromise->set_value(user);
       } else {
         [workerDispatchQueue dispatchAsync:^{
-          [strongSelf userDidChange:user];
+          [strongSelf credentialDidChangeWithUser:user];
         }];
       }
     };
 
-    _credentialsProvider->SetUserChangeListener(userChangeListener);
+    _credentialsProvider->SetCredentialChangeListener(credentialChangeListener);
 
-    // Defer initialization until we get the current user from the userChangeListener. This is
+    // Defer initialization until we get the current user from the credentialChangeListener. This is
     // guaranteed to be synchronously dispatched onto our worker queue, so we will be initialized
     // before any subsequently queued work runs.
     [_workerDispatchQueue dispatchAsync:^{
@@ -159,6 +159,7 @@ - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
 - (void)initializeWithUser:(const User &)user usePersistence:(BOOL)usePersistence {
   // Do all of our initialization on our own dispatch queue.
   [self.workerDispatchQueue verifyIsCurrentQueue];
+  LOG_DEBUG("Initializing. Current user: %s", user.uid());
 
   // Note: The initialization work must all be synchronous (we can't dispatch more work) since
   // external write/listen operations could get queued to run before that subsequent work
@@ -213,11 +214,11 @@ - (void)initializeWithUser:(const User &)user usePersistence:(BOOL)usePersistenc
   [_remoteStore start];
 }
 
-- (void)userDidChange:(const User &)user {
+- (void)credentialDidChangeWithUser:(const User &)user {
   [self.workerDispatchQueue verifyIsCurrentQueue];
 
-  LOG_DEBUG("User Changed: %s", user.uid());
-  [self.syncEngine userDidChange:user];
+  LOG_DEBUG("Credential Changed. Current user: %s", user.uid());
+  [self.syncEngine credentialDidChangeWithUser:user];
 }
 
 - (void)applyChangedOnlineState:(FSTOnlineState)onlineState {
@@ -245,7 +246,7 @@ - (void)enableNetworkWithCompletion:(nullable FSTVoidErrorBlock)completion {
 
 - (void)shutdownWithCompletion:(nullable FSTVoidErrorBlock)completion {
   [self.workerDispatchQueue dispatchAsync:^{
-    self->_credentialsProvider->SetUserChangeListener(nullptr);
+    self->_credentialsProvider->SetCredentialChangeListener(nullptr);
 
     [self.remoteStore shutdown];
     [self.persistence shutdown];

Firestore/Source/Core/FSTSyncEngine.h

@@ -100,7 +100,7 @@ NS_ASSUME_NONNULL_BEGIN
                    updateBlock:(FSTTransactionBlock)updateBlock
                     completion:(FSTVoidIDErrorBlock)completion;
 
-- (void)userDidChange:(const firebase::firestore::auth::User &)user;
+- (void)credentialDidChangeWithUser:(const firebase::firestore::auth::User &)user;
 
 /** Applies an FSTOnlineState change to the sync engine and notifies any views of the change. */
 - (void)applyChangedOnlineState:(FSTOnlineState)onlineState;

Firestore/Source/Core/FSTSyncEngine.mm

@@ -560,15 +560,18 @@ - (void)removeLimboTargetForKey:(const DocumentKey &)key {
   return _limboTargetsByKey;
 }
 
-- (void)userDidChange:(const User &)user {
+- (void)credentialDidChangeWithUser:(const firebase::firestore::auth::User &)user {
+  BOOL userChanged = (_currentUser != user);
   _currentUser = user;
 
-  // Notify local store and emit any resulting events from swapping out the mutation queue.
-  FSTMaybeDocumentDictionary *changes = [self.localStore userDidChange:user];
-  [self emitNewSnapshotsWithChanges:changes remoteEvent:nil];
+  if (userChanged) {
+    // Notify local store and emit any resulting events from swapping out the mutation queue.
+    FSTMaybeDocumentDictionary *changes = [self.localStore userDidChange:user];
+    [self emitNewSnapshotsWithChanges:changes remoteEvent:nil];
+  }
 
   // Notify remote store so it can restart its streams.
-  [self.remoteStore userDidChange:user];
+  [self.remoteStore credentialDidChange];
 }
 
 - (firebase::firestore::model::DocumentKeySet)remoteKeysForTarget:(FSTBoxedTargetID *)targetId {

Firestore/Source/Remote/FSTRemoteStore.h

@@ -129,7 +129,7 @@ NS_ASSUME_NONNULL_BEGIN
  * In response the remote store tears down streams and clears up any tracked operations that should
  * not persist across users. Restarts the streams if appropriate.
  */
-- (void)userDidChange:(const firebase::firestore::auth::User &)user;
+- (void)credentialDidChange;
 
 /** Listens to the target identified by the given FSTQueryData. */
 - (void)listenToTargetWithQueryData:(FSTQueryData *)queryData;

Firestore/Source/Remote/FSTRemoteStore.mm

@@ -209,12 +209,12 @@ - (void)shutdown {
   [self.onlineStateTracker updateState:FSTOnlineStateUnknown];
 }
 
-- (void)userDidChange:(const User &)user {
-  LOG_DEBUG("FSTRemoteStore %s changing users: %s", (__bridge void *)self, user.uid());
+- (void)credentialDidChange {
   if ([self isNetworkEnabled]) {
     // Tear down and re-create our network streams. This will ensure we get a fresh auth token
     // for the new user and re-fill the write pipeline with new mutations from the LocalStore
     // (since mutations are per-user).
+    LOG_DEBUG("FSTRemoteStore %s restarting streams for new credential", (__bridge void *)self);
     [self disableNetworkInternal];
     [self.onlineStateTracker updateState:FSTOnlineStateUnknown];
     [self enableNetwork];

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

@@ -20,7 +20,7 @@ namespace firebase {
 namespace firestore {
 namespace auth {
 
-CredentialsProvider::CredentialsProvider() : user_change_listener_(nullptr) {
+CredentialsProvider::CredentialsProvider() : change_listener_(nullptr) {
 }
 
 CredentialsProvider::~CredentialsProvider() {

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

@@ -33,8 +33,8 @@ namespace auth {
 // `TokenErrorListener` is a listener that gets a token or an error.
 using TokenListener = std::function<void(util::StatusOr<Token>)>;
 
-// Listener notified with a User change.
-using UserChangeListener = std::function<void(User user)>;
+// Listener notified with a credential change.
+using CredentialChangeListener = std::function<void(User user)>;
 
 /**
  * Provides methods for getting the uid and token for the current user and
@@ -56,22 +56,24 @@ class CredentialsProvider {
   virtual void InvalidateToken() = 0;
 
   /**
-   * Sets the listener to be notified of user changes (sign-in / sign-out). It
-   * is immediately called once with the initial user.
+   * Sets the listener to be notified of credential changes (sign-in /
+   * sign-out, token changes). It is immediately called once with the initial
+   * user.
    *
    * Call with nullptr to remove previous listener.
    */
-  virtual void SetUserChangeListener(UserChangeListener listener) = 0;
+  virtual void SetCredentialChangeListener(
+      CredentialChangeListener changeListener) = 0;
 
  protected:
   /**
-   * A listener to be notified of user changes (sign-in / sign-out). It is
-   * immediately called once with the initial user.
+   * A listener to be notified of credential changes (sign-in / sign-out, token
+   * changes). It is immediately called once with the initial user.
    *
    * Note that this block will be called back on an arbitrary thread that is not
    * the normal Firestore worker thread.
    */
-  UserChangeListener user_change_listener_;
+  CredentialChangeListener change_listener_;
 };
 
 }  // namespace auth

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

@@ -28,10 +28,10 @@ void EmptyCredentialsProvider::GetToken(TokenListener completion) {
   }
 }
 
-void EmptyCredentialsProvider::SetUserChangeListener(
-    UserChangeListener listener) {
-  if (listener) {
-    listener(User::Unauthenticated());
+void EmptyCredentialsProvider::SetCredentialChangeListener(
+    CredentialChangeListener changeListener) {
+  if (changeListener) {
+    changeListener(User::Unauthenticated());
   }
 }
 

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

@@ -28,7 +28,8 @@ class EmptyCredentialsProvider : public CredentialsProvider {
  public:
   void GetToken(TokenListener completion) override;
   void InvalidateToken() override;
-  void SetUserChangeListener(UserChangeListener listener) override;
+  void SetCredentialChangeListener(
+      CredentialChangeListener changeListener) override;
 };
 
 }  // namespace auth