Fix bugs in LevelDbDocumentOverlayCache where it failed to check for equality of the DocumentKey (#9381)

Author: dconeybe

Firestore/core/src/local/leveldb_document_overlay_cache.cc

@@ -66,6 +66,10 @@ absl::optional<Overlay> LevelDbDocumentOverlayCache::GetOverlay(
 
   LevelDbDocumentOverlayKey decoded_key;
   HARD_ASSERT(decoded_key.Decode(it->key()));
+  if (decoded_key.document_key() != key) {
+    return absl::nullopt;
+  }
+
   return ParseOverlay(decoded_key, it->value());
 }
 
@@ -193,7 +197,11 @@ void LevelDbDocumentOverlayCache::DeleteOverlay(const model::DocumentKey& key) {
   for (it->Seek(leveldb_key_prefix);
        it->Valid() && absl::StartsWith(it->key(), leveldb_key_prefix);
        it->Next()) {
-    db_->current_transaction()->Delete(it->key());
+    LevelDbDocumentOverlayKey decoded_key;
+    HARD_ASSERT(decoded_key.Decode(it->key()));
+    if (decoded_key.document_key() == key) {
+      db_->current_transaction()->Delete(it->key());
+    }
   }
 }
 

Firestore/core/test/unit/local/document_overlay_cache_test.cc

@@ -152,6 +152,25 @@ TEST_P(DocumentOverlayCacheTest, CanReadSavedOverlays) {
   });
 }
 
+TEST_P(DocumentOverlayCacheTest, GetOverlayExactlyMatchesTheGivenDocumentKey) {
+  this->persistence_->Run("Test", [&] {
+    this->SaveOverlaysWithSetMutations(1, {"coll/doc1/sub/doc2"});
+
+    EXPECT_FALSE(
+        this->cache_->GetOverlay(DocumentKey::FromPathString("coll/d")));
+    EXPECT_FALSE(
+        this->cache_->GetOverlay(DocumentKey::FromPathString("coll/doc1")));
+    EXPECT_FALSE(
+        this->cache_->GetOverlay(DocumentKey::FromPathString("coll/doc1ZZ")));
+
+    const DocumentKey document_key =
+        DocumentKey::FromPathString("coll/doc1/sub/doc2");
+    auto overlay_opt = this->cache_->GetOverlay(document_key);
+    ASSERT_TRUE(overlay_opt);
+    EXPECT_EQ(overlay_opt->key(), document_key);
+  });
+}
+
 TEST_P(DocumentOverlayCacheTest, SavingOverlayOverwrites) {
   this->persistence_->Run("Test", [&] {
     Mutation m1 = PatchMutation("coll/doc1", Map("foo", "bar"));
@@ -288,6 +307,34 @@ TEST_P(DocumentOverlayCacheTest, UpdateDocumentOverlay) {
   });
 }
 
+TEST_P(DocumentOverlayCacheTest, SaveDoesntAffectSubCollections) {
+  this->persistence_->Run("Test", [&] {
+    Mutation mutation1 =
+        PatchMutation("coll/doc/subcoll/subdoc", Map("foo", "bar1"));
+    Mutation mutation2 = PatchMutation("coll/doc", Map("foo", "bar2"));
+    this->SaveOverlaysWithMutations(1, {mutation1});
+    this->SaveOverlaysWithMutations(2, {mutation2});
+
+    // Verify that `GetOverlay()` returns the correct mutations.
+    {
+      auto overlay_opt = this->cache_->GetOverlay(
+          DocumentKey::FromPathString("coll/doc/subcoll/subdoc"));
+      EXPECT_TRUE(overlay_opt);
+      if (overlay_opt) {
+        EXPECT_EQ(overlay_opt.value().mutation(), mutation1);
+      }
+    }
+    {
+      auto overlay_opt =
+          this->cache_->GetOverlay(DocumentKey::FromPathString("coll/doc"));
+      EXPECT_TRUE(overlay_opt);
+      if (overlay_opt) {
+        EXPECT_EQ(overlay_opt.value().mutation(), mutation2);
+      }
+    }
+  });
+}
+
 }  // namespace
 }  // namespace local
 }  // namespace firestore