C++ migration: make all methods of FSTRemoteStore delegate to C++ (#2337)

Author: var-const

Firestore/Source/Remote/FSTRemoteStore.mm

@@ -31,6 +31,7 @@
 
 #include "Firestore/core/src/firebase/firestore/auth/user.h"
 #include "Firestore/core/src/firebase/firestore/model/document_key.h"
+#include "Firestore/core/src/firebase/firestore/model/document_key_set.h"
 #include "Firestore/core/src/firebase/firestore/model/mutation_batch.h"
 #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h"
 #include "Firestore/core/src/firebase/firestore/remote/online_state_tracker.h"
@@ -52,7 +53,6 @@
 using firebase::firestore::model::DocumentKeySet;
 using firebase::firestore::model::OnlineState;
 using firebase::firestore::model::SnapshotVersion;
-using firebase::firestore::model::DocumentKeySet;
 using firebase::firestore::model::TargetId;
 using firebase::firestore::remote::Datastore;
 using firebase::firestore::remote::WatchStream;
@@ -75,9 +75,6 @@
 #pragma mark - FSTRemoteStore
 
 @implementation FSTRemoteStore {
-  /** The client-side proxy for interacting with the backend. */
-  std::shared_ptr<Datastore> _datastore;
-
   std::unique_ptr<RemoteStore> _remoteStore;
 }
 
@@ -86,12 +83,8 @@ - (instancetype)initWithLocalStore:(FSTLocalStore *)localStore
                        workerQueue:(AsyncQueue *)queue
                 onlineStateHandler:(std::function<void(OnlineState)>)onlineStateHandler {
   if (self = [super init]) {
-    _datastore = std::move(datastore);
-    _datastore->Start();
-
-    _remoteStore = absl::make_unique<RemoteStore>(localStore, _datastore.get(), queue,
+    _remoteStore = absl::make_unique<RemoteStore>(localStore, std::move(datastore), queue,
                                                   std::move(onlineStateHandler));
-    _remoteStore->set_is_network_enabled(false);
   }
   return self;
 }
@@ -101,75 +94,27 @@ - (void)setSyncEngine:(id<FSTRemoteSyncer>)syncEngine {
 }
 
 - (void)start {
-  // For now, all setup is handled by enableNetwork(). We might expand on this in the future.
-  [self enableNetwork];
+  _remoteStore->Start();
 }
 
 #pragma mark Online/Offline state
 
 - (void)enableNetwork {
-  _remoteStore->set_is_network_enabled(true);
-
-  if (_remoteStore->CanUseNetwork()) {
-    // Load any saved stream token from persistent storage
-    _remoteStore->write_stream().SetLastStreamToken([_remoteStore->local_store() lastStreamToken]);
-
-    if (_remoteStore->ShouldStartWatchStream()) {
-      _remoteStore->StartWatchStream();
-    } else {
-      _remoteStore->online_state_tracker().UpdateState(OnlineState::Unknown);
-    }
-
-    // This will start the write stream if necessary.
-    [self fillWritePipeline];
-  }
+  _remoteStore->EnableNetwork();
 }
 
 - (void)disableNetwork {
-  _remoteStore->set_is_network_enabled(false);
-  [self disableNetworkInternal];
-
-  // Set the OnlineState to Offline so get()s return from cache, etc.
-  _remoteStore->online_state_tracker().UpdateState(OnlineState::Offline);
-}
-
-/** Disables the network, setting the OnlineState to the specified targetOnlineState. */
-- (void)disableNetworkInternal {
-  _remoteStore->watch_stream().Stop();
-  _remoteStore->write_stream().Stop();
-
-  if (!_remoteStore->write_pipeline().empty()) {
-    LOG_DEBUG("Stopping write stream with %s pending writes",
-              _remoteStore->write_pipeline().size());
-    _remoteStore->write_pipeline().clear();
-  }
-
-  _remoteStore->CleanUpWatchStreamState();
+  _remoteStore->DisableNetwork();
 }
 
 #pragma mark Shutdown
 
 - (void)shutdown {
-  LOG_DEBUG("FSTRemoteStore %s shutting down", (__bridge void *)self);
-  _remoteStore->set_is_network_enabled(false);
-  [self disableNetworkInternal];
-  // Set the OnlineState to Unknown (rather than Offline) to avoid potentially triggering
-  // spurious listener events with cached data, etc.
-  _remoteStore->online_state_tracker().UpdateState(OnlineState::Unknown);
-  _datastore->Shutdown();
+  _remoteStore->Shutdown();
 }
 
 - (void)credentialDidChange {
-  if (_remoteStore->CanUseNetwork()) {
-    // 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);
-    _remoteStore->set_is_network_enabled(false);
-    [self disableNetworkInternal];
-    _remoteStore->online_state_tracker().UpdateState(OnlineState::Unknown);
-    [self enableNetwork];
-  }
+  _remoteStore->HandleCredentialChange();
 }
 
 #pragma mark Watch Stream
@@ -201,7 +146,7 @@ - (void)addBatchToWritePipeline:(FSTMutationBatch *)batch {
 }
 
 - (FSTTransaction *)transaction {
-  return [FSTTransaction transactionWithDatastore:_datastore.get()];
+  return _remoteStore->CreateTransaction();
 }
 
 @end

Firestore/core/src/firebase/firestore/remote/remote_store.h

@@ -43,6 +43,7 @@
 @class FSTMutationBatch;
 @class FSTMutationBatchResult;
 @class FSTQueryData;
+@class FSTTransaction;
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -112,48 +113,72 @@ class RemoteStore : public TargetMetadataProvider,
                     public WriteStreamCallback {
  public:
   RemoteStore(FSTLocalStore* local_store,
-              Datastore* datastore,
+              std::shared_ptr<Datastore> datastore,
               util::AsyncQueue* worker_queue,
               std::function<void(model::OnlineState)> online_state_handler);
 
-  // TODO(varconst): remove the getters and setters
-
-  id<FSTRemoteSyncer> sync_engine() {
-    return sync_engine_;
-  }
   void set_sync_engine(id<FSTRemoteSyncer> sync_engine) {
     sync_engine_ = sync_engine;
   }
 
-  FSTLocalStore* local_store() {
-    return local_store_;
-  }
+  /**
+   * Starts up the remote store, creating streams, restoring state from
+   * `FSTLocalStore`, etc.
+   */
+  void Start();
 
-  OnlineStateTracker& online_state_tracker() {
-    return online_state_tracker_;
-  }
+  /**
+   * Shuts down the remote store, tearing down connections and otherwise
+   * cleaning up.
+   */
+  void Shutdown();
 
-  void set_is_network_enabled(bool value) {
-    is_network_enabled_ = value;
-  }
+  /**
+   * Temporarily disables the network. The network can be re-enabled using
+   * 'EnableNetwork'.
+   */
+  void DisableNetwork();
 
-  WatchStream& watch_stream() {
-    return *watch_stream_;
-  }
-  WriteStream& write_stream() {
-    return *write_stream_;
-  }
+  /**
+   * Re-enables the network. Only to be called as the counterpart to
+   * 'DisableNetwork'.
+   */
+  void EnableNetwork();
 
-  std::vector<FSTMutationBatch*>& write_pipeline() {
-    return write_pipeline_;
-  }
+  /**
+   * Tells the `RemoteStore` that the currently authenticated user has changed.
+   *
+   * 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 HandleCredentialChange();
 
   /** Listens to the target identified by the given `FSTQueryData`. */
   void Listen(FSTQueryData* query_data);
 
   /** Stops listening to the target with the given target ID. */
   void StopListening(model::TargetId target_id);
 
+  /**
+   * Attempts to fill our write pipeline with writes from the `FSTLocalStore`.
+   *
+   * Called internally to bootstrap or refill the write pipeline and by
+   * `FSTSyncEngine` whenever there are new mutations to process.
+   *
+   * Starts the write stream if necessary.
+   */
+  void FillWritePipeline();
+
+  /**
+   * Queues additional writes to be sent to the write stream, sending them
+   * immediately if the write stream is established.
+   */
+  void AddToWritePipeline(FSTMutationBatch* batch);
+
+  /** Returns a new transaction backed by this remote store. */
+  FSTTransaction* CreateTransaction();
+
   model::DocumentKeySet GetRemoteKeysForTarget(
       model::TargetId target_id) const override;
   FSTQueryData* GetQueryDataForTarget(model::TargetId target_id) const override;
@@ -171,37 +196,9 @@ class RemoteStore : public TargetMetadataProvider,
       model::SnapshotVersion commit_version,
       std::vector<FSTMutationResult*> mutation_results) override;
 
-  // TODO(varconst): make the following methods private.
-
-  bool CanUseNetwork() const;
-
-  void StartWatchStream();
-
-  /**
-   * Returns true if the network is enabled, the watch stream has not yet been
-   * started and there are active watch targets.
-   */
-  bool ShouldStartWatchStream() const;
-
-  void CleanUpWatchStreamState();
-
-  /**
-   * Attempts to fill our write pipeline with writes from the `FSTLocalStore`.
-   *
-   * Called internally to bootstrap or refill the write pipeline and by
-   * `FSTSyncEngine` whenever there are new mutations to process.
-   *
-   * Starts the write stream if necessary.
-   */
-  void FillWritePipeline();
-
-  /**
-   * Queues additional writes to be sent to the write stream, sending them
-   * immediately if the write stream is established.
-   */
-  void AddToWritePipeline(FSTMutationBatch* batch);
-
  private:
+  void DisableNetworkInternal();
+
   void SendWatchRequest(FSTQueryData* query_data);
   void SendUnwatchRequest(model::TargetId target_id);
 
@@ -231,14 +228,29 @@ class RemoteStore : public TargetMetadataProvider,
   void HandleHandshakeError(const util::Status& status);
   void HandleWriteError(const util::Status& status);
 
+  bool CanUseNetwork() const;
+
+  void StartWatchStream();
+
+  /**
+   * Returns true if the network is enabled, the watch stream has not yet been
+   * started and there are active watch targets.
+   */
+  bool ShouldStartWatchStream() const;
+
+  void CleanUpWatchStreamState();
+
   id<FSTRemoteSyncer> sync_engine_ = nil;
 
   /**
    * The local store, used to fill the write pipeline with outbound mutations
-   * and resolve existence filter mismatches. Immutable after initialization.
+   * and resolve existence filter mismatches.
    */
   FSTLocalStore* local_store_ = nil;
 
+  /** The client-side proxy for interacting with the backend. */
+  std::shared_ptr<Datastore> datastore_;
+
   /**
    * A mapping of watched targets that the client cares about tracking and the
    * user has explicitly called a 'listen' for this target.
@@ -253,8 +265,8 @@ class RemoteStore : public TargetMetadataProvider,
   OnlineStateTracker online_state_tracker_;
 
   /**
-   * Set to true by `EnableNetwork` and false by `DisableNetworkInternal` and
-   * indicates the user-preferred network state.
+   * Set to true by `EnableNetwork` and false by `DisableNetwork` and indicates
+   * the user-preferred network state.
    */
   bool is_network_enabled_ = false;