Merge pull request #268 from redknightlois/RDBC-1018

RDBC-1018/1019/1023 Fix change tracking metadata aliasing, type-coercion, add per-entity change API

Author: poissoncorp

ravendb/documents/session/document_info.py

@@ -44,4 +44,5 @@ def get_new_document_info(cls, document: Dict) -> DocumentInfo:
         if not change_vector or not isinstance(change_vector, str):
             raise ValueError(f"Document {key} must have a Change Vector")
 
-        return cls(key=key, document=document, metadata=metadata, entity=None, change_vector=change_vector)
+        # Shallow-copy metadata so mutations on this DocumentInfo don't alias the original document dict
+        return cls(key=key, document=document, metadata=dict(metadata), entity=None, change_vector=change_vector)

ravendb/documents/session/document_session.py

@@ -803,6 +803,12 @@ def graph_query(self, object_type: type, query: str):  # -> GraphDocumentQuery:
         def what_changed(self) -> Dict[str, List[DocumentsChanges]]:
             return self._session._what_changed()
 
+        def what_changed_for(self, entity: object) -> List[DocumentsChanges]:
+            return self._session._what_changed_for(entity)
+
+        def get_tracked_entities(self) -> Dict[str, dict]:
+            return self._session._get_tracked_entities()
+
         def exists(self, key: str) -> bool:
             if key is None:
                 raise ValueError("Key cannot be None")

ravendb/documents/session/document_session_operations/in_memory_document_session_operations.py

@@ -1191,6 +1191,51 @@ def _what_changed(self) -> Dict[str, List[DocumentsChanges]]:
 
         return changes
 
+    def _what_changed_for(self, entity: object) -> List[DocumentsChanges]:
+        if (doc_info := self._documents_by_entity.get(entity)) is None:
+            return []
+        if entity in self._deleted_entities:
+            return [
+                DocumentsChanges(
+                    field_old_value="",
+                    field_new_value="",
+                    change=DocumentsChanges.ChangeType.DOCUMENT_DELETED,
+                )
+            ]
+        _update_metadata_modifications(doc_info.metadata_instance, doc_info.metadata)
+        new_obj = self.entity_to_json.convert_entity_to_json(entity, doc_info)
+        changes: Dict[str, List[DocumentsChanges]] = {}
+        if not self._entity_changed(new_obj, doc_info, changes):
+            return []
+        return [
+            DocumentsChanges(
+                field_old_value=d["old_value"],
+                field_new_value=d["new_value"],
+                change=d["change"],
+                field_name=d["field_name"],
+                field_path=d["field_path"],
+            )
+            for d in changes.get(doc_info.key, [])
+        ]
+
+    def _get_tracked_entities(self) -> Dict[str, dict]:
+        result = {}
+        for entity_result in self._documents_by_entity:
+            doc_info = entity_result.value
+            result[doc_info.key] = {
+                "id": doc_info.key,
+                "entity": entity_result.key,
+                "is_deleted": self.is_deleted(doc_info.key),
+            }
+        for key in self._known_missing_ids:
+            if key not in result:
+                result[key] = {
+                    "id": key,
+                    "entity": None,
+                    "is_deleted": True,
+                }
+        return result
+
     def __get_all_entities_changes(self, changes: Dict[str, List[DocumentsChanges]]) -> None:
         for key, value in self._documents_by_id.items():
             _update_metadata_modifications(value.metadata_instance, value.metadata)

ravendb/json/json_operation.py

@@ -82,7 +82,7 @@ def compare_json(
             old_prop = original_json[prop]
 
             if isinstance(new_prop, (int, float, bool, str)):
-                if new_prop == old_prop or JsonOperation.compare_values(old_prop, new_prop):
+                if JsonOperation.compare_values(old_prop, new_prop):
                     continue
                 if changes is None:
                     return True
@@ -193,7 +193,10 @@ def compare_json_array(field_path: str, key: str, old_collection, new_collection
                             DocumentsChanges.ChangeType.ARRAY_VALUE_CHANGED,
                         )
             elif isinstance(new_collection_item, (int, float, bool, str)):
-                if not str(old_collection_item) == str(new_collection_item):
+                if (
+                    type(old_collection_item) is not type(new_collection_item)
+                    or old_collection_item != new_collection_item
+                ):
                     if changes is not None:
                         JsonOperation.new_change(
                             JsonOperation.add_index_field_path(field_path, position),

ravendb/tests/session_tests/test_change_tracking_metadata.py

@@ -0,0 +1,242 @@
+"""C# ref: FastTests/Client/WhatChanged.cs — WhatChanged_should_be_idempotent_operation (RavenDB-9150)"""
+
+from ravendb.tests.test_base import TestBase
+
+
+class User:
+    def __init__(self, name: str = "", age: int = 0):
+        self.name = name
+        self.age = age
+
+
+class TestRavenDBWhatChangedIdempotent(TestBase):
+    def setUp(self):
+        super().setUp()
+
+    def test_what_changed_should_be_idempotent_operation(self):
+        """No modifications — two calls return the same empty result."""
+        with self.store.open_session() as session:
+            session.store(User(name="Alice", age=30), "users/1")
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            session.load("users/1", User)
+
+            result_1 = session.advanced.what_changed()
+            result_2 = session.advanced.what_changed()
+
+            self.assertEqual(result_1, result_2)
+
+    def test_what_changed_should_be_idempotent_operation_with_changes(self):
+        """With modifications — two calls return the same non-empty result."""
+        with self.store.open_session() as session:
+            session.store(User(name="user1"), "users/2")
+            session.store(User(name="user2", age=1), "users/3")
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            user1 = session.load("users/2", User)
+            user2 = session.load("users/3", User)
+
+            user1.age = 10
+            session.delete(user2)
+
+            result_1 = session.advanced.what_changed()
+            result_2 = session.advanced.what_changed()
+
+            self.assertEqual(2, len(result_1))
+            self.assertEqual(len(result_1), len(result_2))
+
+    def test_what_changed_detects_metadata_modification(self):
+        with self.store.open_session() as session:
+            session.store(User(name="Bob"), "users/4")
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            user = session.load("users/4", User)
+            meta = session.advanced.get_metadata_for(user)
+            meta["@custom-tag"] = "v1"
+
+            changes = session.advanced.what_changed()
+            self.assertIn("users/4", changes)
+
+    def test_what_changed_and_save_changes_agree_on_metadata_write(self):
+        with self.store.open_session() as session:
+            session.store(User(name="Carol"), "users/5")
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            user = session.load("users/5", User)
+            meta = session.advanced.get_metadata_for(user)
+            meta["@custom-tag"] = "v2"
+
+            pending = session.advanced.what_changed()
+            self.assertIn("users/5", pending)
+
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            reloaded_meta = session.advanced.get_metadata_for(session.load("users/5", User))
+            self.assertEqual("v2", reloaded_meta.get("@custom-tag"))
+
+
+class TestRavenDBWhatChangedMetadataOps(TestBase):
+    def setUp(self):
+        super().setUp()
+
+    def test_has_changed_returns_true_for_metadata_only_modification(self):
+        with self.store.open_session() as session:
+            session.store(User(name="Alice"), "users/1")
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            user = session.load("users/1", User)
+            meta = session.advanced.get_metadata_for(user)
+            meta["@custom"] = "v1"
+
+            self.assertTrue(session.advanced.has_changed(user))
+
+    def test_has_changed_returns_false_when_no_modification(self):
+        with self.store.open_session() as session:
+            session.store(User(name="Bob"), "users/2")
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            session.load("users/2", User)
+            user = session.load("users/2", User)
+
+            self.assertFalse(session.advanced.has_changed(user))
+
+    def test_has_changes_returns_true_for_metadata_only_modification(self):
+        with self.store.open_session() as session:
+            session.store(User(name="Carol"), "users/3")
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            user = session.load("users/3", User)
+            meta = session.advanced.get_metadata_for(user)
+            meta["@custom"] = "v1"
+
+            self.assertTrue(session.advanced.has_changes())
+
+    def test_has_changes_returns_false_when_no_modification(self):
+        with self.store.open_session() as session:
+            session.store(User(name="Dave"), "users/4")
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            session.load("users/4", User)
+            self.assertFalse(session.advanced.has_changes())
+
+    def test_what_changed_detects_metadata_key_deletion(self):
+        with self.store.open_session() as session:
+            session.store(User(name="Eve"), "users/5")
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            user = session.load("users/5", User)
+            meta = session.advanced.get_metadata_for(user)
+            meta["@custom"] = "to-be-deleted"
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            user = session.load("users/5", User)
+            meta = session.advanced.get_metadata_for(user)
+            self.assertEqual("to-be-deleted", meta.get("@custom"))
+            del meta["@custom"]
+
+            changes = session.advanced.what_changed()
+            self.assertIn("users/5", changes)
+
+    def test_metadata_deletion_is_detected_and_persisted(self):
+        with self.store.open_session() as session:
+            session.store(User(name="Frank"), "users/6")
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            user = session.load("users/6", User)
+            meta = session.advanced.get_metadata_for(user)
+            meta["@tag"] = "remove-me"
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            user = session.load("users/6", User)
+            meta = session.advanced.get_metadata_for(user)
+            del meta["@tag"]
+
+            self.assertIn("users/6", session.advanced.what_changed())
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            meta = session.advanced.get_metadata_for(session.load("users/6", User))
+            self.assertNotIn("@tag", meta)
+
+
+class Doc:
+    def __init__(self, name: str = ""):
+        self.name = name
+
+
+class TestRavenDBWhatChangedMetadataMutations(TestBase):
+    def setUp(self):
+        super().setUp()
+        with self.store.open_session() as session:
+            d = Doc()
+            session.store(d, "d/1")
+            meta = session.advanced.get_metadata_for(d)
+            meta["Test-A"] = ["a", "a", "a"]
+            meta["Test-C"] = ["c", "c", "c"]
+            session.save_changes()
+
+    def test_metadata_array_value_change_detected(self):
+        """meta["Test-A"] = ["b","a","c"] -> 2 ARRAY_VALUE_CHANGED entries."""
+        with self.store.open_session() as session:
+            d = session.load("d/1", Doc)
+            meta = session.advanced.get_metadata_for(d)
+            meta["Test-A"] = ["b", "a", "c"]
+
+            changes = session.advanced.what_changed()
+
+        self.assertIn("d/1", changes)
+        self.assertEqual(2, len(changes["d/1"]))
+
+    def test_metadata_key_removal_detected(self):
+        """meta.Remove("Test-A") -> 1 REMOVED_FIELD entry."""
+        with self.store.open_session() as session:
+            d = session.load("d/1", Doc)
+            meta = session.advanced.get_metadata_for(d)
+            meta.pop("Test-A")
+
+            changes = session.advanced.what_changed()
+
+        self.assertIn("d/1", changes)
+        change_types = [str(c["change"]) for c in changes["d/1"]]
+        self.assertIn("removed_field", change_types)
+
+    def test_metadata_remove_two_add_two_detected(self):
+        """Remove Test-A, Test-C; add Test-B, Test-D -> 4 entries."""
+        with self.store.open_session() as session:
+            d = session.load("d/1", Doc)
+            meta = session.advanced.get_metadata_for(d)
+            meta.pop("Test-A")
+            meta.pop("Test-C")
+            meta["Test-B"] = ["b", "b", "b"]
+            meta["Test-D"] = ["d", "d", "d"]
+
+            changes = session.advanced.what_changed()
+
+        self.assertIn("d/1", changes)
+        self.assertEqual(4, len(changes["d/1"]))
+
+    def test_metadata_remove_one_add_one_detected(self):
+        """Remove Test-A; add Test-B -> 2 entries."""
+        with self.store.open_session() as session:
+            d = session.load("d/1", Doc)
+            meta = session.advanced.get_metadata_for(d)
+            meta.pop("Test-A")
+            meta["Test-B"] = ["b", "b", "b"]
+
+            changes = session.advanced.what_changed()
+
+        self.assertIn("d/1", changes)
+        self.assertEqual(2, len(changes["d/1"]))