Added NoopPersistenceManager, removed NoopPersistenceStorageEngine.

At some point I added a NoopPersistenceStorageEngine which is a version of the PersistenceStorageEngine interface which takes no actions, intended for use with the special .info sync tree which does not persist it's data. However, that was the wrong level of abstraction, and it was the PersistenceManager that should have had the Noop version of it. This fixes a potential issue where .info values are returned null instead of the value they are supposed to be.

PiperOrigin-RevId: 293012252

Author: amablue

database/CMakeLists.txt

@@ -97,7 +97,7 @@ set(desktop_SRCS
     src/desktop/disconnection_desktop.cc
     src/desktop/mutable_data_desktop.cc
     src/desktop/persistence/in_memory_persistence_storage_engine.cc
-    src/desktop/persistence/noop_persistence_storage_engine.cc
+    src/desktop/persistence/noop_persistence_manager.cc
     src/desktop/persistence/persistence_manager.cc
     src/desktop/persistence/prune_forest.cc
     src/desktop/push_child_name_generator.cc

database/src/desktop/core/sync_tree.h

@@ -46,7 +46,7 @@ enum Persist {
 class SyncTree {
  public:
   SyncTree(UniquePtr<WriteTree> pending_write_tree,
-           UniquePtr<PersistenceManager> persistence_manager,
+           UniquePtr<PersistenceManagerInterface> persistence_manager,
            UniquePtr<ListenProvider> listen_provider)
       : pending_write_tree_(std::move(pending_write_tree)),
         persistence_manager_(std::move(persistence_manager)),
@@ -187,7 +187,7 @@ class SyncTree {
 
   // The persistence manager, which is used to interact with the persisted data
   // on disk (both reads and writes).
-  UniquePtr<PersistenceManager> persistence_manager_;
+  UniquePtr<PersistenceManagerInterface> persistence_manager_;
 
   // A tree that contains the SyncPoints for each location being watched in the
   // database.

database/src/desktop/persistence/noop_persistence_manager.cc

@@ -0,0 +1,111 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "database/src/desktop/persistence/noop_persistence_manager.h"
+
+#define VERIFY_INSIDE_TRANSACTION() \
+  FIREBASE_DEV_ASSERT_MESSAGE(      \
+      this->inside_transaction_,    \
+      "Transaction expected to already be in progress.")
+
+namespace firebase {
+namespace database {
+namespace internal {
+
+NoopPersistenceManager::NoopPersistenceManager() : inside_transaction_(false) {}
+
+void NoopPersistenceManager::SaveUserOverwrite(const Path& path,
+                                               const Variant& variant,
+                                               WriteId write_id) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+void NoopPersistenceManager::SaveUserMerge(const Path& path,
+                                           const CompoundWrite& children,
+                                           WriteId write_id) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+void NoopPersistenceManager::RemoveUserWrite(WriteId write_id) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+void NoopPersistenceManager::RemoveAllUserWrites() {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+void NoopPersistenceManager::ApplyUserWriteToServerCache(
+    const Path& path, const Variant& variant) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+void NoopPersistenceManager::ApplyUserWriteToServerCache(
+    const Path& path, const CompoundWrite& merge) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+std::vector<UserWriteRecord> NoopPersistenceManager::LoadUserWrites() {
+  return std::vector<UserWriteRecord>();
+}
+
+CacheNode NoopPersistenceManager::ServerCache(const QuerySpec& query_spec) {
+  return CacheNode();
+}
+
+void NoopPersistenceManager::UpdateServerCache(const QuerySpec& query_spec,
+                                               const Variant& variant) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+void NoopPersistenceManager::UpdateServerCache(const Path& path,
+                                               const CompoundWrite& children) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+void NoopPersistenceManager::SetQueryActive(const QuerySpec& query_spec) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+void NoopPersistenceManager::SetQueryInactive(const QuerySpec& query_spec) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+void NoopPersistenceManager::SetQueryComplete(const QuerySpec& query_spec) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+void NoopPersistenceManager::SetTrackedQueryKeys(
+    const QuerySpec& query_spec, const std::set<std::string>& keys) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+void NoopPersistenceManager::UpdateTrackedQueryKeys(
+    const QuerySpec& query_spec, const std::set<std::string>& added,
+    const std::set<std::string>& removed) {
+  VERIFY_INSIDE_TRANSACTION();
+}
+
+bool NoopPersistenceManager::RunInTransaction(std::function<bool()> func) {
+  FIREBASE_DEV_ASSERT_MESSAGE(!inside_transaction_,
+                              "RunInTransaction called when an existing "
+                              "transaction is already in progress.");
+  inside_transaction_ = true;
+  bool success = func();
+  inside_transaction_ = false;
+  return success;
+}
+
+}  // namespace internal
+}  // namespace database
+}  // namespace firebase

database/src/desktop/persistence/noop_persistence_manager.h

@@ -0,0 +1,132 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef FIREBASE_DATABASE_CLIENT_CPP_SRC_DESKTOP_PERSISTENCE_NOOP_PERSISTENCE_MANAGER_H_
+#define FIREBASE_DATABASE_CLIENT_CPP_SRC_DESKTOP_PERSISTENCE_NOOP_PERSISTENCE_MANAGER_H_
+
+#include <functional>
+#include <set>
+#include <string>
+#include <vector>
+
+#include "app/memory/unique_ptr.h"
+#include "app/src/mutex.h"
+#include "app/src/path.h"
+#include "database/src/common/query_spec.h"
+#include "database/src/desktop/core/cache_policy.h"
+#include "database/src/desktop/core/tracked_query_manager.h"
+#include "database/src/desktop/persistence/persistence_manager.h"
+#include "database/src/desktop/persistence/persistence_manager_interface.h"
+#include "database/src/desktop/persistence/persistence_storage_engine.h"
+#include "database/src/desktop/view/view_cache.h"
+
+namespace firebase {
+namespace database {
+namespace internal {
+
+class NoopPersistenceManager : public PersistenceManagerInterface {
+ public:
+  NoopPersistenceManager();
+
+  // Persist a user write to the storage engine.
+  //
+  // @param path The path for this write.
+  // @param variant The variant for this write.
+  // @param write_id The write id that was used for this write.
+  void SaveUserOverwrite(const Path& path, const Variant& variant,
+                         WriteId write_id) override;
+
+  // Persist a user merge to the storage engine.
+  //
+  // @param path The path for this merge.
+  // @param children The children for this merge.
+  // @param write_id The write id that was used for this merge.
+  void SaveUserMerge(const Path& path, const CompoundWrite& children,
+                     WriteId write_id) override;
+
+  // Remove the user write with the given write id.
+  //
+  // @param write_id The write id to remove.
+  void RemoveUserWrite(WriteId write_id) override;
+
+  // Remove all user writes.
+  void RemoveAllUserWrites() override;
+
+  // Applies the write to the storage engine so that it can be persisted.
+  void ApplyUserWriteToServerCache(const Path& path,
+                                   const Variant& variant) override;
+
+  // Applies the merge to the storage engine so that it can be persisted.
+  void ApplyUserWriteToServerCache(const Path& path,
+                                   const CompoundWrite& merge) override;
+
+  // Return a list of all writes that were persisted
+  //
+  // @return The list of writes
+  std::vector<UserWriteRecord> LoadUserWrites() override;
+
+  // Returns any cached variant or children as a CacheNode. The query is *not*
+  // used to filter the variant but rather to determine if it can be considered
+  // complete.
+  //
+  // @param query The query at the path
+  // @return The cached variant or an empty CacheNode if no cache is available
+  CacheNode ServerCache(const QuerySpec& query) override;
+
+  // Overwrite the server cache at the location given by the given QuerySpec.
+  void UpdateServerCache(const QuerySpec& query,
+                         const Variant& variant) override;
+
+  // Merge the given write into theserver cache at the location given by the
+  // given QuerySpec.
+  void UpdateServerCache(const Path& path,
+                         const CompoundWrite& children) override;
+
+  // Begin tracking the given QuerySpec.
+  void SetQueryActive(const QuerySpec& query) override;
+
+  // Stop tracking the given QuerySpec.
+  void SetQueryInactive(const QuerySpec& query) override;
+
+  // Inform the TrackedQueryManager to mark the tracked query as complete.
+  void SetQueryComplete(const QuerySpec& query) override;
+
+  // Inform the storage engine which keys should be tracked for a given Query.
+  void SetTrackedQueryKeys(const QuerySpec& query,
+                           const std::set<std::string>& keys) override;
+
+  // Updates the set of keys that should be tracked for a given Query.
+  void UpdateTrackedQueryKeys(const QuerySpec& query,
+                              const std::set<std::string>& added,
+                              const std::set<std::string>& removed) override;
+
+  // Run a transaction. Transactions are functions that are going to change
+  // values in the database, and they must do so effectively atomically. Two
+  // transacations cannot be run at the same time in different threads, they
+  // must be scheduled to run by Scheduler. A transaction should return true to
+  // signal that it completed successfully, or false if there was an error.
+  // Certain actions can only be performed from inside a transaction, and those
+  // functions will assert if they are not called between calls to
+  // BeingTransaction and EndTransaction.
+  bool RunInTransaction(std::function<bool()> func) override;
+
+ private:
+  bool inside_transaction_;
+};
+
+}  // namespace internal
+}  // namespace database
+}  // namespace firebase
+
+#endif  // FIREBASE_DATABASE_CLIENT_CPP_SRC_DESKTOP_PERSISTENCE_NOOP_PERSISTENCE_MANAGER_H_