main tests pass.

firestore/integration_test_internal/src/firestore_test.cc

@@ -20,6 +20,11 @@
 #include <future>
 #include <memory>
 #include <stdexcept>
+#include "firebase/firestore/document_snapshot.h"
+#include "firebase/firestore/field_value.h"
+#include "firebase/firestore/local_cache_settings.h"
+#include "firebase/firestore/map_field_value.h"
+#include "firebase/firestore/source.h"
 
 #if defined(__ANDROID__)
 #include "android/firestore_integration_test_android.h"
@@ -1499,6 +1504,90 @@ TEST_F(FirestoreIntegrationTest, ClearPersistenceWhileRunningFails) {
   EXPECT_EQ(await_clear_persistence.error(), Error::kErrorFailedPrecondition);
 }
 
+TEST_F(FirestoreIntegrationTest, LegacyCacheConfigForMemoryCacheWorks) {
+  auto* db = TestFirestore("legacy_memory_cache");
+  auto settings = db->settings();
+  settings.set_persistence_enabled(false);
+  db->set_settings(std::move(settings));
+
+  Await(db->Document("rooms/eros")
+            .Set(MapFieldValue{{"desc", FieldValue::String("eros")}}));
+
+  auto get_future = db->Document("rooms/eros").Get(Source::kCache);
+  const DocumentSnapshot* snapshot = Await(get_future);
+  EXPECT_EQ(get_future.status(), FutureStatus::kFutureStatusComplete);
+  EXPECT_FALSE(snapshot->is_valid());
+  snapshot->id();
+}
+
+TEST_F(FirestoreIntegrationTest, LegacyCacheConfigForPersistenceCacheWorks) {
+  auto* db = TestFirestore("legacy_persistent_cache");
+  auto settings = db->settings();
+  settings.set_persistence_enabled(true);
+  db->set_settings(std::move(settings));
+
+  Await(db->Document("rooms/eros")
+            .Set(MapFieldValue{{"desc", FieldValue::String("eros")}}));
+
+  auto get_future = db->Document("rooms/eros").Get(Source::kCache);
+  const DocumentSnapshot* snapshot = Await(get_future);
+  EXPECT_EQ(get_future.status(), FutureStatus::kFutureStatusComplete);
+  EXPECT_TRUE(snapshot->is_valid());
+  EXPECT_THAT(snapshot->GetData(),
+              ContainerEq(MapFieldValue{{"desc", FieldValue::String("eros")}}));
+}
+
+TEST_F(FirestoreIntegrationTest, NewCacheConfigForMemoryCacheWorks) {
+  auto* db = TestFirestore("new_memory_cache");
+  auto settings = db->settings();
+  settings.set_local_cache_settings(MemoryCacheSettings::Create());
+  db->set_settings(std::move(settings));
+
+  Await(db->Document("rooms/eros")
+            .Set(MapFieldValue{{"desc", FieldValue::String("eros")}}));
+
+  auto get_future = db->Document("rooms/eros").Get(Source::kCache);
+  const DocumentSnapshot* snapshot = Await(get_future);
+  EXPECT_EQ(get_future.status(), FutureStatus::kFutureStatusComplete);
+  EXPECT_FALSE(snapshot->is_valid());
+}
+
+TEST_F(FirestoreIntegrationTest, NewCacheConfigForPersistenceCacheWorks) {
+  auto* db = TestFirestore("new_persistent_cache");
+  auto settings = db->settings();
+  settings.set_local_cache_settings(
+      PersistentCacheSettings::Create().WithSizeBytes(50 * 1024 * 1024));
+  db->set_settings(std::move(settings));
+
+  Await(db->Document("rooms/eros")
+            .Set(MapFieldValue{{"desc", FieldValue::String("eros")}}));
+
+  auto get_future = db->Document("rooms/eros").Get(Source::kCache);
+  const DocumentSnapshot* snapshot = Await(get_future);
+  EXPECT_EQ(get_future.status(), FutureStatus::kFutureStatusComplete);
+  EXPECT_TRUE(snapshot->is_valid());
+  EXPECT_THAT(snapshot->GetData(),
+              ContainerEq(MapFieldValue{{"desc", FieldValue::String("eros")}}));
+}
+
+TEST_F(FirestoreIntegrationTest, CannotMixNewAndLegacyCacheConfig) {
+  {
+    auto* db = TestFirestore("mixing_1");
+    auto settings = db->settings();
+    settings.set_local_cache_settings(
+        PersistentCacheSettings::Create().WithSizeBytes(50 * 1024 * 1024));
+    EXPECT_THROW(settings.set_cache_size_bytes(0), std::logic_error);
+  }
+  {
+    auto* db = TestFirestore("mixing_2");
+    auto settings = db->settings();
+    settings.set_persistence_enabled(false);
+    EXPECT_THROW(
+        settings.set_local_cache_settings(MemoryCacheSettings::Create()),
+        std::logic_error);
+  }
+}
+
 // Note: this test only exists in C++.
 TEST_F(FirestoreIntegrationTest, DomainObjectsReferToSameFirestoreInstance) {
   EXPECT_EQ(TestFirestore(), TestFirestore()->Document("foo/bar").firestore());

firestore/src/common/settings.cc

@@ -24,6 +24,7 @@
 #include "Firestore/core/src/util/hard_assert.h"
 #include "app/meta/move.h"
 #include "firebase/firestore/local_cache_settings.h"
+#include "firestore/src/common/exception_common.h"
 
 #if !defined(__ANDROID__)
 #include "Firestore/core/src/util/executor.h"
@@ -75,7 +76,12 @@ std::shared_ptr<LocalCacheSettings> Settings::local_cache_settings() const {
 }
 
 void Settings::set_local_cache_settings(const LocalCacheSettings& cache) {
-  HARD_ASSERT(!used_legacy_cache_settings_, "");
+  if (used_legacy_cache_settings_) {
+    SimpleThrowIllegalState(
+        "Cannot mix set_local_cache_settings() with legacy cache api like "
+        "set_persistence_enabled() or set_cache_size_bytes()");
+  }
+
   if (cache.kind() == api::LocalCacheSettings::Kind::kPersistent) {
     local_cache_settings_ = std::make_shared<PersistentCacheSettings>(
         *static_cast<const PersistentCacheSettings&>(cache).settings_internal_);
@@ -86,13 +92,23 @@ void Settings::set_local_cache_settings(const LocalCacheSettings& cache) {
 }
 
 void Settings::set_persistence_enabled(bool enabled) {
-  HARD_ASSERT(local_cache_settings() == nullptr, "");
+  if (local_cache_settings_ != nullptr) {
+    SimpleThrowIllegalState(
+        "Cannot mix legacy cache api set_persistence_enabled() with new cache "
+        "api set_local_cache_settings()");
+  }
+
   persistence_enabled_ = enabled;
   used_legacy_cache_settings_ = true;
 }
 
 void Settings::set_cache_size_bytes(int64_t value) {
-  HARD_ASSERT(local_cache_settings() == nullptr, "");
+  if (local_cache_settings_ != nullptr) {
+    SimpleThrowIllegalState(
+        "Cannot mix legacy cache api set_cache_size_bytes() with new cache api "
+        "set_local_cache_settings()");
+  }
+
   cache_size_bytes_ = value;
   used_legacy_cache_settings_ = true;
 }

firestore/src/include/firebase/firestore/local_cache_settings.h

@@ -52,7 +52,6 @@ class PersistentCacheSettings final : public LocalCacheSettings {
   PersistentCacheSettings(const PersistentCacheSettingsInternal& other);
 
   PersistentCacheSettings WithSizeBytes(int64_t size) const;
-  const CoreCacheSettings& core_cache_settings() const override;
   api::LocalCacheSettings::Kind kind() const override {
     return api::LocalCacheSettings::Kind::kPersistent;
   }
@@ -62,6 +61,8 @@ class PersistentCacheSettings final : public LocalCacheSettings {
 
   PersistentCacheSettings();
 
+  const CoreCacheSettings& core_cache_settings() const override;
+
   std::unique_ptr<PersistentCacheSettingsInternal> settings_internal_;
 };
 
@@ -70,7 +71,6 @@ class MemoryCacheSettings final : public LocalCacheSettings {
   static MemoryCacheSettings Create();
   ~MemoryCacheSettings();
 
-  const CoreCacheSettings& core_cache_settings() const override;
   api::LocalCacheSettings::Kind kind() const override {
     return api::LocalCacheSettings::Kind::kMemory;
   }
@@ -81,6 +81,7 @@ class MemoryCacheSettings final : public LocalCacheSettings {
   friend class Settings;
 
   MemoryCacheSettings();
+  const CoreCacheSettings& core_cache_settings() const override;
 
   std::unique_ptr<MemoryCacheSettingsInternal> settings_internal_;
 };

firestore/src/main/firestore_main.cc

@@ -191,6 +191,9 @@ Settings FirestoreInternal::settings() const {
   result.set_ssl_enabled(from.ssl_enabled());
   result.set_persistence_enabled(from.persistence_enabled());
   result.set_cache_size_bytes(from.cache_size_bytes());
+  // TODO(wuandy): This line should be deleted when legacy cache config is
+  // removed.
+  result.used_legacy_cache_settings_ = false;
 
   return result;
 }
@@ -199,7 +202,11 @@ void FirestoreInternal::set_settings(Settings from) {
   api::Settings settings;
   settings.set_host(std::move(from.host()));
   settings.set_ssl_enabled(from.is_ssl_enabled());
-  if (!from.used_legacy_cache_settings_) {
+  // TODO(wuandy): Checking `from.local_cache_settings_` is required, because
+  // FirestoreInternal::settings() overrides used_legacy_cache_settings_. All
+  // this special logic should go away when legacy cache config is removed.
+  if (!from.used_legacy_cache_settings_ &&
+      from.local_cache_settings_ != nullptr) {
     settings.set_local_cache_settings(
         from.local_cache_settings()->core_cache_settings());
   } else {