RDBC-839 changing the semantic - find_identity_property to get_identity_property_name, fixes in EntityToJson

Author: poissoncorp

ravendb/documents/conventions.py

@@ -5,7 +5,7 @@
 from abc import abstractmethod, ABC
 from datetime import timedelta, datetime
 from enum import Enum
-from typing import Dict, List, Tuple, Callable, Union, Optional, Generic, Type
+from typing import Dict, List, Tuple, Callable, Union, Optional, Generic, Type, Any, TYPE_CHECKING
 
 import inflect
 
@@ -25,6 +25,11 @@
 
 _T = TypeVar("_T")
 
+if TYPE_CHECKING:
+    from ravendb.documents.session.document_session_operations.in_memory_document_session_operations import (
+        InMemoryDocumentSessionOperations,
+    )
+
 
 class DocumentConventions(object):
     @classmethod
@@ -63,7 +68,8 @@ def __init__(self):
 
         # Utilities
         self.document_id_generator: Optional[Callable[[str, object], str]] = None
-        self._find_identity_property = lambda q: q.__name__ == "key"
+        self._get_identity_property_name: Callable[[Type[Any]], str] = lambda type_: "Id"
+        self._id_property_name_cache: Dict[Type, str] = {}
         self._find_python_class: Optional[Callable[[str, Dict], str]] = None
         self._find_collection_name: Callable[[Type], str] = self.default_get_collection_name
         self._find_collection_name_for_dict: Callable[[str], str] = self.default_get_collection_name_for_dict
@@ -106,7 +112,7 @@ def find_collection_name(self) -> Callable[[type], str]:
 
     @find_collection_name.setter
     def find_collection_name(self, value) -> None:
-        self.__assert_not_frozen()
+        self._assert_not_frozen()
         self._find_collection_name = value
 
     @property
@@ -122,7 +128,7 @@ def __default(key: str, doc: Dict) -> Optional[str]:
 
     @find_python_class.setter
     def find_python_class(self, value: Callable[[str, Dict], str]):
-        self.__assert_not_frozen()
+        self._assert_not_frozen()
         self._find_python_class = value
 
     def get_python_class(self, key: str, document: Dict) -> str:
@@ -134,7 +140,7 @@ def transform_class_collection_name_to_document_id_prefix(self) -> Callable[[str
 
     @transform_class_collection_name_to_document_id_prefix.setter
     def transform_class_collection_name_to_document_id_prefix(self, value: Callable[[str], str]) -> None:
-        self.__assert_not_frozen()
+        self._assert_not_frozen()
         self._transform_class_collection_name_to_document_id_prefix = value
 
     @property
@@ -167,7 +173,7 @@ def find_python_class_name(self) -> Callable[[type], str]:
 
     @find_python_class_name.setter
     def find_python_class_name(self, value) -> None:
-        self.__assert_not_frozen()
+        self._assert_not_frozen()
         self._find_python_class_name = value
 
     @property
@@ -176,7 +182,7 @@ def should_ignore_entity_changes(self) -> ShouldIgnoreEntityChanges:
 
     @should_ignore_entity_changes.setter
     def should_ignore_entity_changes(self, value: ShouldIgnoreEntityChanges) -> None:
-        self.__assert_not_frozen()
+        self._assert_not_frozen()
         self._should_ignore_entity_changes = value
 
     @property
@@ -185,7 +191,7 @@ def load_balancer_context_seed(self) -> int:
 
     @load_balancer_context_seed.setter
     def load_balancer_context_seed(self, value: int):
-        self.__assert_not_frozen()
+        self._assert_not_frozen()
         self._load_balancer_context_seed = value
 
     @property
@@ -194,7 +200,7 @@ def load_balance_behavior(self):
 
     @load_balance_behavior.setter
     def load_balance_behavior(self, value: LoadBalanceBehavior):
-        self.__assert_not_frozen()
+        self._assert_not_frozen()
         self._load_balance_behavior = value
 
     @property
@@ -203,7 +209,7 @@ def read_balance_behavior(self) -> ReadBalanceBehavior:
 
     @read_balance_behavior.setter
     def read_balance_behavior(self, value: ReadBalanceBehavior):
-        self.__assert_not_frozen()
+        self._assert_not_frozen()
         self._read_balance_behavior = value
 
     @property
@@ -216,7 +222,7 @@ def disable_atomic_document_writes_in_cluster_wide_transaction(self) -> bool:
 
     @disable_atomic_document_writes_in_cluster_wide_transaction.setter
     def disable_atomic_document_writes_in_cluster_wide_transaction(self, value: bool):
-        self.__assert_not_frozen()
+        self._assert_not_frozen()
         self._disable_atomic_document_writes_in_cluster_wide_transaction = value
 
     @staticmethod
@@ -355,7 +361,7 @@ def range_field_name(field_name, type_name):
 
         return field_name
 
-    def __assert_not_frozen(self) -> None:
+    def _assert_not_frozen(self) -> None:
         if self._frozen:
             raise RuntimeError(
                 "Conventions has been frozen after documentStore.initialize()" " and no changes can be applied to them"
@@ -370,7 +376,7 @@ def clone(self) -> DocumentConventions:
         cloned._save_enums_as_integers = self._save_enums_as_integers
         cloned.identity_parts_separator = self.identity_parts_separator
         cloned.disable_topology_updates = self.disable_topology_updates
-        cloned._find_identity_property = self._find_identity_property
+        cloned._get_identity_property_name = self._get_identity_property_name
 
         cloned.document_id_generator = self.document_id_generator
 
@@ -384,6 +390,19 @@ def clone(self) -> DocumentConventions:
         cloned._read_balance_behavior = self._read_balance_behavior
         cloned._load_balance_behavior = self._load_balance_behavior
         self._max_http_cache_size = self._max_http_cache_size
+        return cloned
+
+    def get_identity_property_name(self, object_type: Type[Any]) -> Optional[str]:
+        # Check the cache first
+        if object_type in self._id_property_name_cache:
+            return self._id_property_name_cache[object_type]
+
+        id_property_name = self._get_identity_property_name(object_type)
+
+        # Cache the result
+        self._id_property_name_cache[object_type] = id_property_name
+
+        return id_property_name
 
     def update_from(self, configuration: ClientConfiguration):
         if configuration.disabled and self._original_configuration is None:
@@ -479,7 +498,7 @@ def update_from(self, configuration: ClientConfiguration):
 
     @staticmethod
     def default_transform_collection_name_to_document_id_prefix(collection_name: str) -> str:
-        upper_count = len(list(filter(str.isupper, collection_name)))
+        upper_count = len(list(filter(str.isupper, [char for char in collection_name])))
         if upper_count <= 1:
             return collection_name.lower()
 

ravendb/documents/identity/hilo.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 import datetime
 from threading import Lock
-from typing import Any, Union, Optional, Iterable, Dict, Tuple, Callable
+from typing import Any, Union, Optional, Iterable, Dict, Tuple, Callable, Type
 
 from typing import TYPE_CHECKING
 
@@ -15,22 +15,27 @@
 
 class GenerateEntityIdOnTheClient:
     def __init__(self, conventions: "DocumentConventions", generate_id: Callable[[Any], str]):
-        self.__conventions = conventions
-        self.__generate_id = generate_id
+        self._conventions = conventions
+        self._generate_id = generate_id
+
+    def get_identity_property_name(self, object_type: Type[Any]) -> str:
+        return self._conventions.get_identity_property_name(object_type)
 
     def try_get_id_from_instance(self, entity: Union[object, dict]) -> Tuple[bool, Union[str, None]]:
         if not entity:
             raise ValueError("Entity cannot be None")
-        identity_property = "Id"  # todo: make sure it's ok, create get_identity_property within conventions if not
+
+        identity_property = self.get_identity_property_name(entity.__class__)
+
         value = (entity if isinstance(entity, dict) else entity.__dict__).get(identity_property, None)
         if isinstance(value, str):
             return True, value
         return False, None
 
     def get_or_generate_document_id(self, entity) -> str:
-        key = self.try_get_id_from_instance(entity)[1]
-        if key is None:
-            key = self.__generate_id(entity)
+        success, key = self.try_get_id_from_instance(entity)
+        if not success:
+            key = self._generate_id(entity)
 
         if key and key.startswith("/"):
             raise ValueError(f"Cannot use value '{key}' as a document id because it begins with a '/'")
@@ -43,22 +48,22 @@ def generate_document_key_for_storage(self, entity: object) -> str:
         return key
 
     def try_set_identity(self, entity: object, key: str, is_projection: bool = False) -> None:
-        self.__try_set_identity_internal(entity, key, is_projection)
+        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: get_identity_property...
+    def _try_set_identity_internal(self, entity: Union[object, dict], key: str, is_projection: bool = False) -> None:
+        identity_property_name = self._conventions.get_identity_property_name(entity.__class__)
 
-        if identity_property is None:
+        if identity_property_name is None:
             return
 
-        if is_projection and entity.__getattribute__(identity_property):
+        if is_projection and entity.__getattribute__(identity_property_name):
             # identity property was already set
             return
 
         if isinstance(entity, dict):
-            entity[identity_property] = key
+            entity[identity_property_name] = key
             return
-        entity.__setattr__(identity_property, key)
+        entity.__setattr__(identity_property_name, key)
 
 
 class MultiDatabaseHiLoGenerator: