Merge pull request #211 from poissoncorp/RDBC-812

RDBC-812 Don't send 'Id' property to the server

Author: poissoncorp

ravendb/documents/identity/hilo.py

@@ -46,7 +46,7 @@ def try_set_identity(self, entity: object, key: str, is_projection: bool = False
         self.__try_set_identity_internal(entity, key, is_projection)
 
     def __try_set_identity_internal(self, entity: Union[object, dict], key: str, is_projection: bool = False) -> None:
-        identity_property = "Id"  # todo: make sure it's ok, create get_identity_property...
+        identity_property = "Id"  # todo: get_identity_property...
 
         if identity_property is None:
             return
@@ -55,7 +55,7 @@ def __try_set_identity_internal(self, entity: Union[object, dict], key: str, is_
             # identity property was already set
             return
 
-        if type(entity) == dict:
+        if isinstance(entity, dict):
             entity[identity_property] = key
             return
         entity.__setattr__(identity_property, key)

ravendb/documents/session/entity_to_json.py

@@ -38,7 +38,7 @@ def convert_entity_to_json(self, entity: object, document_info: DocumentInfo) ->
             self._session.before_conversion_to_document_invoke(
                 BeforeConversionToDocumentEventArgs(document_info.key, entity, self._session)
             )
-        document = EntityToJson._convert_entity_to_json_internal(self, entity, document_info)
+        document = EntityToJson._convert_entity_to_json_internal(self, entity, document_info, True)
         if document_info is not None:
             self._session.after_conversion_to_document_invoke(
                 AfterConversionToDocumentEventArgs(self._session, document_info.key, entity, document)
@@ -192,9 +192,10 @@ def convert_to_entity_static(
                 BeforeConversionToEntityEventArgs(session_hook, key, object_type, document_deepcopy)
             )
 
-        # 1.1 Check if passed object type (or extracted from metadata) is a dictionary
-        if object_type == dict or type_from_metadata == "builtins.dict":
+        # 1.1 Check if passed object type (or extracted from metadata) is a dictionary and if document isn't a dict
+        if object_type == dict or (object_type is None and type_from_metadata == "builtins.dict"):
             EntityToJson._invoke_after_conversion_to_entity_event(session_hook, key, object_type, document_deepcopy)
+            # todo: document_deepcopy["Id"] = metadata.get("@id", None) - consider adding id property
             return document_deepcopy
 
         # 1.2 If there's no object type in metadata
@@ -211,8 +212,12 @@ def convert_to_entity_static(
         # 2. There was a type in the metadata
         else:
             object_from_metadata = Utils.import_class(type_from_metadata)
+
+            # 2.0 Check if the user wants to cast dict to type
+            if object_from_metadata == dict and object_type != dict:
+                pass
             # 2.1 Import was successful
-            if object_from_metadata is not None:
+            elif object_from_metadata is not None:
                 # 2.1.1 Set object_type to successfully imported type/ from metadata inherits from passed object_type
                 if object_type is None or Utils.is_inherit(object_type, object_from_metadata):
                     object_type = object_from_metadata

ravendb/tests/issue_tests/test_issue_198.py

@@ -1,3 +1,5 @@
+from ravendb.infrastructure.orders import Company
+from ravendb.primitives import constants
 from ravendb.tests.test_base import TestBase
 
 
@@ -22,6 +24,28 @@ def test_create_document_with_server_generated_guid(self):
         with self.store.open_session() as session:
             product = session.load(guid_from_server)
             self.assertEqual("Test", product.name)
+            metadata_id = session.advanced.get_metadata_for(product)["@id"]
+            identity = session.advanced.get_document_id(product)
+            id_prop = product.Id
+
+            self.assertEqual(id_prop, identity)
+            self.assertEqual(id_prop, metadata_id)
+            self.assertEqual(id_prop, guid_from_server)
+
+        with self.store.open_session() as session:
+            product = session.load(guid_from_server)
+            product.Id = ""
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            product = session.load(guid_from_server)
+            metadata_id1 = session.advanced.get_metadata_for(product)[constants.Documents.Metadata.ID]
+            identity1 = session.advanced.get_document_id(product)
+            id_prop1 = product.Id
+
+            self.assertEqual(id_prop, id_prop1)
+            self.assertEqual(id_prop, metadata_id1)
+            self.assertEqual(id_prop, identity1)
 
     def test_can_create_multiple_documents_with_server_generated_guid(self):
         with self.store.open_session() as session:
@@ -45,3 +69,40 @@ def test_can_create_multiple_documents_with_server_generated_guid(self):
 
             product3 = session.load(guid_from_server3)
             self.assertEqual("Test3", product3.name)
+
+    def test_preserve_id_field(self):
+        with self.store.open_session() as session:
+            company = Company(Id="companies/222", name="Apple Inc.")
+            session.store(company)
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            company = session.load("companies/222", Company)
+            self.assertEqual("companies/222", company.Id)
+            self.assertEqual("Apple Inc.", company.name)
+
+    def test_docs_loaded_as_dictionary_dont_have_id_entry_and_its_id_is_available_via_metadata(self):
+        with self.store.open_session() as session:
+            company = Company(Id="companies/1", name="A Inc.")
+            company_dict = {"Id": "companies/dict", "name": "Dict Inc."}
+            session.store(company)
+            session.store(company_dict)
+            session.save_changes()
+
+        with self.store.open_session() as session:
+            company = session.load("companies/1", Company)
+            company_dict = session.load("companies/dict", dict)
+
+            self.assertNotIn("Id", company_dict)
+            self.assertEqual("companies/dict", session.advanced.get_document_id(company_dict))
+            self.assertEqual("companies/dict", session.advanced.get_metadata_for(company_dict)["@id"])
+
+            self.assertEqual("companies/1", company.Id)
+            self.assertEqual("companies/1", session.advanced.get_document_id(company))
+            self.assertEqual("companies/1", session.advanced.get_metadata_for(company)["@id"])
+
+        with self.store.open_session() as session:
+            company_dict_as_company_object = session.load("companies/dict", Company)
+            self.assertEqual("companies/dict", company_dict_as_company_object.Id)
+            self.assertEqual("companies/dict", session.advanced.get_document_id(company_dict_as_company_object))
+            self.assertEqual("companies/dict", session.advanced.get_metadata_for(company_dict_as_company_object)["@id"])

ravendb/tests/jvm_migrated_tests/crud_tests/test_basic_document.py

@@ -13,7 +13,7 @@ def setUp(self):
         super(TestBasicDocument, self).setUp()
 
     def test_get(self):
-        dummy = {"name": None, "age": None, "Id": None}
+        dummy = {"name": None, "age": None}
         with self.store.open_session() as session:
             user1 = User("jacus", None)
             user2 = User("arek", None)

ravendb/tests/jvm_migrated_tests/issues_tests/test_RDBC_387.py

@@ -25,5 +25,5 @@ def test_should_store_single_id(self):
             self.assertIsNone(session_user.get("id", None))
             self.assertIsNone(bulk_insert_user.get("id", None))
 
-            self.assertIsNotNone(session_user.get("Id", None))
+            self.assertIsNone(session_user.get("Id", None))
             self.assertIsNone(bulk_insert_user.get("Id", None))

ravendb/tests/jvm_migrated_tests/server_tests/patching/test_advanced_patching.py

@@ -46,7 +46,7 @@ def to_json(self) -> Dict[str, Any]:
     @classmethod
     def from_json(cls, json_dict: Dict[str, Any]) -> CustomType:
         return cls(
-            json_dict["Id"],
+            json_dict["Id"] if "Id" in json_dict else None,
             json_dict["owner"],
             json_dict["value"],
             json_dict["comments"],

ravendb/tools/utils.py

@@ -450,7 +450,16 @@ def fill_with_nested_object_types(entity: object, nested_object_types: Dict[str,
     @staticmethod
     def initialize_object(obj: dict, object_type: Type[_T], convert_to_snake_case: bool = None) -> _T:
         initialize_dict, set_needed = Utils.make_initialize_dict(obj, object_type.__init__, convert_to_snake_case)
-        o = object_type(**initialize_dict)
+        try:
+            o = object_type(**initialize_dict)
+        except Exception as e:
+            if "Id" not in initialize_dict:
+                initialize_dict["Id"] = None
+                o = object_type(**initialize_dict)
+            else:
+                raise TypeError(
+                    f"Couldn't initialize object of type '{object_type.__name__}' using dict '{obj}'"
+                ) from e
         if set_needed:
             for key, value in obj.items():
                 setattr(o, key, value)