RDBC-644 Critical fix for DocumentInfo missing metadata bug

Author: poissoncorp

ravendb/documents/session/entity_to_json.py

@@ -54,7 +54,7 @@ def convert_entity_to_json_internal_static(
         json_node = Utils.entity_to_dict(entity, conventions.json_default_method)
         EntityToJson.write_metadata(json_node, document_info)
         if remove_identity_property:
-            EntityToJson.try_remove_identity_property(entity)
+            EntityToJson.try_remove_identity_property_json(json_node)
         return json_node
 
     @staticmethod
@@ -69,7 +69,7 @@ def _convert_entity_to_json_internal(
         json_node = Utils.entity_to_dict(entity, self._session.conventions.json_default_method)
         self.write_metadata(json_node, document_info)
         if remove_identity_property:
-            self.try_remove_identity_property(json_node)
+            self.try_remove_identity_property_json(json_node)
         return json_node
 
     # todo: refactor this method, make it more useful/simple and less ugly (like this return...[0])
@@ -100,6 +100,14 @@ def try_remove_identity_property(document):
         except AttributeError:
             return False
 
+    @staticmethod
+    def try_remove_identity_property_json(document: Dict) -> bool:
+        try:
+            del document["Id"]
+            return True
+        except KeyError:
+            return False
+
     @staticmethod
     def write_metadata(json_node: dict, document_info: DocumentInfo):
         if document_info is None:
@@ -153,7 +161,7 @@ def convert_to_entity_static(
         conventions: "DocumentConventions",
         session_hook: Optional["InMemoryDocumentSessionOperations"] = None,
     ) -> _T:
-        metadata = document.pop("@metadata")
+        metadata = document.get("@metadata")
         document_deepcopy = deepcopy(document)
         type_from_metadata = conventions.try_get_type_from_metadata(metadata)
         is_inherit = False

ravendb/json/json_operation.py

@@ -50,7 +50,9 @@ def compare_json(
 
         new_fields = new_json_props - old_json_props
         removed_fields = old_json_props - new_json_props
-        removed_fields.discard("Id")  # todo: Discuss about that condition - add/del Id causes changes
+
+        # todo: Discuss about that condition - add/del Id causes changes
+        removed_fields.discard("Id")
 
         for field in removed_fields:
             if changes is None:
@@ -64,7 +66,7 @@ def compare_json(
                 prop == constants.Documents.Metadata.LAST_MODIFIED
                 or prop == constants.Documents.Metadata.COLLECTION
                 or prop == constants.Documents.Metadata.CHANGE_VECTOR
-                or prop == constants.Documents.Metadata.KEY
+                or prop == constants.Documents.Metadata.ID
             ):
                 continue
 

ravendb/tests/jvm_migrated_tests/client_tests/session_tests/test_add_or_patch.py

@@ -1,4 +1,4 @@
-from datetime import datetime
+from datetime import datetime, timedelta
 from dataclasses import dataclass
 
 from typing import List
@@ -24,21 +24,19 @@ def test_can_add_or_patch(self):
         key = "users/1"
 
         with self.store.open_session() as session:
-            new_user = User(first_name="Hibernating", last_name="Rhinos", login_count=1)
+            new_user = User(first_name="Hibernating", last_name="Rhinos", last_login=datetime.utcnow())
             session.store(new_user, key)
             session.save_changes()
+            self.assertEqual(1, session.advanced.number_of_requests)
 
         with self.store.open_session() as session:
-            new_user = User(first_name="Hibernating", last_name="Rhinos")
-            session.advanced.add_or_increment(key, new_user, "login_count", 3)
-
+            new_user = User(first_name="Hibernating", last_name="Rhinos", last_login=datetime.utcnow())
+            new_date = datetime.utcnow() + timedelta(days=365)
+            session.advanced.add_or_patch(key, new_user, "last_login", new_date)
             session.save_changes()
 
             self.assertEqual(1, session.advanced.number_of_requests)
 
-            user = session.load(key, User)
-            self.assertEqual(4, user.login_count)
-
             session.delete(key)
             session.save_changes()
 
@@ -71,7 +69,7 @@ def test_can_add_or_patch_add_item_to_an_existing_array(self):
 
         with self.store.open_session() as session:
             user = User(first_name="Hibernating", last_name="Rhinos")
-            datetime_now = datetime.now()
+            datetime_now = datetime.utcnow()
             d2000 = datetime(
                 2000,
                 datetime_now.month,

ravendb/tests/jvm_migrated_tests/issues_tests/test_ravenDB_16301.py

@@ -16,71 +16,74 @@ def setUp(self):
         super(TestRavenDB16301, self).setUp()
 
     def test_can_use_conditional_load_lazily(self):
+        # Store 100 companies using bulk insert - it won't store identity property on the server
         with self.store.bulk_insert() as bulk_insert:
             for i in range(100):
                 bulk_insert.store_by_entity(Company())
 
-        ids: List[Result] = []
-        loads: List[Lazy[ConditionalLoadResult[Company]]] = []
+        conditional_loads: List[Lazy[ConditionalLoadResult[Company]]] = []
 
-        with self.store.open_session() as session1:
-            ids = list(
-                session1.advanced.document_query(object_type=Company)
+        with self.store.open_session() as query_modify_session:
+            id_and_cv_results = list(
+                query_modify_session.advanced.document_query(object_type=Company)
                 .wait_for_non_stale_results()
                 .select_fields_query_data(
                     Result,
                     QueryData.custom_function("o", "{ Id : id(o), change_vector : getMetadata(o)['@change-vector'] }"),
                 )
             )
 
-            all_ids = [single_id.Id for single_id in ids]
-            session1.load(all_ids, Company)
+            all_ids = [single_id.Id for single_id in id_and_cv_results]
+            query_modify_session.load(all_ids, Company)
 
+            # load 50 first companies and update them
             res_ids = all_ids[0:50]
 
-            res = session1.load(res_ids, Company)
+            res = query_modify_session.load(res_ids, Company)
 
             c = 0
             for key in res:
                 c += 1
                 res[key].phone = c
 
-            session1.save_changes()
+            query_modify_session.save_changes()
 
         with self.store.open_session() as session:
-            # load last 10
+            # load last 10 (track them in the session)
             session.load(all_ids[-10:])
             number_of_requests_per_session = session.advanced.number_of_requests
 
-            for res in ids:
-                loads.append(session.advanced.lazily.conditional_load(res.Id, res.change_vector, Company))
+            for res in id_and_cv_results:
+                conditional_loads.append(session.advanced.lazily.conditional_load(res.Id, res.change_vector, Company))
 
             session.advanced.eagerly.execute_all_pending_lazy_operations()
 
             self.assertEqual(number_of_requests_per_session + 1, session.advanced.number_of_requests)
 
             for i in range(100):
-                l = loads[i]
+                conditional_load = conditional_loads[i]
 
-                self.assertFalse(l.is_value_created)
-                conditional_load_resuilt = l.value
+                self.assertFalse(conditional_load.is_value_created)
+                conditional_load_result = conditional_load.value
 
                 if i < 50:
                     # load from server
-                    self.assertEqual(ids[i].Id, conditional_load_resuilt.entity.Id)
+                    self.assertEqual(id_and_cv_results[i].Id, conditional_load_result.entity.Id)
                 elif i < 90:
                     # not modified
-                    self.assertIsNone(conditional_load_resuilt.entity)
-                    self.assertEqual(ids[i].change_vector, conditional_load_resuilt.change_vector)
+                    self.assertEqual(id_and_cv_results[i].change_vector, conditional_load_result.change_vector)
+                    self.assertIsNone(conditional_load_result.entity)
 
                 else:
                     # tracked in session
-                    self.assertEqual(ids[i].Id, conditional_load_resuilt.entity.Id)
-                    self.assertIsNotNone(conditional_load_resuilt.entity)
-                    self.assertEqual(ids[i].change_vector, conditional_load_resuilt.change_vector)
+                    self.assertEqual(id_and_cv_results[i].Id, conditional_load_result.entity.Id)
+                    self.assertIsNotNone(conditional_load_result.entity)
+                    self.assertEqual(id_and_cv_results[i].change_vector, conditional_load_result.change_vector)
 
                 # not exist on server
-                lazy = session.advanced.lazily.conditional_load("Companies/322-A", ids[0].change_vector, Company)
+                lazy = session.advanced.lazily.conditional_load(
+                    "Companies/322-A", id_and_cv_results[0].change_vector, Company
+                )
                 load = lazy.value
                 self.assertIsNone(load.entity)
                 self.assertIsNone(load.change_vector)

ravendb/tools/utils.py

@@ -527,7 +527,7 @@ def convert_to_entity(
     ) -> Union[_T, _DynamicStructure]:
         if document is None:
             return None
-        metadata = document.pop("@metadata")
+        metadata = document.get("@metadata")
         original_document = deepcopy(document)
         type_from_metadata = conventions.try_get_type_from_metadata(metadata)
         mapper = conventions.mappers.get(object_type, None)