Skip to content

RDBC-855/856 Check if pydantic objects break storing documents, Utils cleanup #228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: v5.2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/RavenClient.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ jobs:
- name: Install embedded RavenDB
run: pip install ravendb-embedded

- name: Install Pydantic
run: pip install pydantic

- name: Run certifi script
run: python ./.github/workflows/add_ca.py

Expand Down
2 changes: 1 addition & 1 deletion ravendb/documents/commands/batches.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ def set_response(self, response: str, from_cache: bool) -> None:
"Got None response from the server after doing a batch, something is very wrong."
" Probably a garbled response."
)
self.result = Utils.initialize_object(json.loads(response), self._result_class, True)
self.result = BatchCommandResult.from_json(json.loads(response))


class ClusterWideBatchCommand(SingleNodeBatchCommand):
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import json
from typing import Dict, Type, TypeVar, Optional
from typing import Dict, Type, TypeVar, Optional, Any, Union

from ravendb.primitives import constants
from ravendb.documents.conventions import DocumentConventions
Expand Down Expand Up @@ -60,11 +60,11 @@ def get_single_value(

key: str = item.get("Key")
index: int = item.get("Index")
raw: dict = item.get("Value")
raw: Dict[str, Union[Any, Dict[str, Any]]] = item.get("Value")
if not raw:
return CompareExchangeValue(key, index, None)
metadata = None
bjro = raw.get(constants.Documents.Metadata.KEY)
bjro: Dict[str, Any] = raw.get(constants.Documents.Metadata.KEY)
if bjro:
metadata = (
MetadataAsDictionary(bjro)
Expand Down
4 changes: 1 addition & 3 deletions ravendb/documents/operations/operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ def wait_for_completion(self) -> None:
raise OperationCancelledException()
elif operation_status == "Faulted":
result = status.get("Result")
exception_result: OperationExceptionResult = Utils.initialize_object(
result, OperationExceptionResult, True
)
exception_result = OperationExceptionResult.from_json(result)
schema = ExceptionDispatcher.ExceptionSchema(
self.__request_executor.url, exception_result.type, exception_result.message, exception_result.error
)
Expand Down
2 changes: 1 addition & 1 deletion ravendb/documents/queries/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def write(self, obj: object):
elif "__str__" in obj.__class__.__dict__:
self.__buffer.append(str(obj))
else:
self.__buffer.append(str(Utils.dictionarize(obj)))
self.__buffer.append(str(Utils.object_to_dict_for_hash_calculator(obj)))

def write_parameters(self, qp: "Parameters") -> None:
if qp is None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,10 @@ def before_query_invoke(self, before_query_event_args: BeforeQueryEventArgs):
def documents_by_id(self):
return self._documents_by_id

@property
def included_documents_by_id(self):
return self._included_documents_by_id

@property
def deleted_entities(self):
return self._deleted_entities
Expand Down
20 changes: 9 additions & 11 deletions ravendb/documents/session/entity_to_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,15 +172,8 @@ def convert_to_entity(
if "from_json" in object_type.__dict__ and inspect.ismethod(object_type.from_json):
# By custom defined 'from_json' serializer class method
entity = object_type.from_json(document_deepcopy)
elif is_projection:
entity = DynamicStructure(**document_deepcopy)
entity.__class__ = object_type
try:
entity = Utils.initialize_object(document_deepcopy, object_type)
except TypeError as e:
raise InvalidOperationException("Probably projection error", e)
else:
entity = Utils.convert_json_dict_to_object(document_deepcopy, object_type)
entity = Utils.convert_json_dict_to_object(document_deepcopy, object_type, is_projection)

EntityToJsonUtils.invoke_after_conversion_to_entity_event(session, key, object_type, document_deepcopy)

Expand Down Expand Up @@ -295,10 +288,15 @@ def determine_object_type(

# Passed type is not a type from metadata, neither there's no inheritance - probably projection
elif object_type_from_user is not object_type_from_metadata:
if not all([name in object_type_from_metadata.__dict__ for name in object_type_from_user.__dict__]):
# Document from database and object_type from user aren't compatible
# Check if types are compatible
incompatible_fields = Utils.check_valid_projection(object_type_from_user, object_type_from_metadata)
if incompatible_fields:
raise exceptions.InvalidOperationException(
f"Cannot covert document from type {object_type_from_metadata} to {object_type_from_user}"
f"Invalid projection. Cannot covert document "
f"from type '{object_type_from_metadata.__name__}' "
f"to type '{object_type_from_user.__name__}'. "
f"Type '{object_type_from_user.__name__}' instance has fields {incompatible_fields} "
f"that aren't on '{object_type_from_metadata.__name__}'."
)

# Projection
Expand Down
4 changes: 2 additions & 2 deletions ravendb/documents/session/operations/load_operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,11 @@ def __get_document(self, object_type: Type[_T], key: str) -> _T:
if self._session.is_deleted(key):
return Utils.get_default_value(object_type)

doc = self._session._documents_by_id.get(key)
doc = self._session.documents_by_id.get(key)
if doc is not None:
return self._session.track_entity_document_info(object_type, doc)

doc = self._session._included_documents_by_id.get(key)
doc = self._session.included_documents_by_id.get(key)
if doc is not None:
return self._session.track_entity_document_info(object_type, doc)

Expand Down
6 changes: 1 addition & 5 deletions ravendb/http/request_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,11 +438,7 @@ def __run(errors: list):

topology = Topology(
self._topology_etag,
(
self.topology_nodes
if self.topology_nodes
else list(map(lambda url_val: ServerNode(url_val, self._database_name, "!"), initial_urls))
),
(self.topology_nodes or [ServerNode(url, self._database_name, "!") for url in initial_urls]),
)

self._node_selector = NodeSelector(topology, self._thread_pool_executor)
Expand Down
29 changes: 19 additions & 10 deletions ravendb/http/server_node.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from enum import Enum
from typing import Optional, TYPE_CHECKING
from typing import Optional, TYPE_CHECKING, Any, Dict

if TYPE_CHECKING:
from ravendb.http.topology import ClusterTopology
Expand All @@ -26,8 +26,17 @@ def __init__(
self.database = database
self.cluster_tag = cluster_tag
self.server_role = server_role
self.__last_server_version_check = 0
self.__last_server_version: str = None
self._last_server_version_check = 0
self._last_server_version: Optional[str] = None

@classmethod
def from_json(cls, json_dict: Dict[str, Any]) -> "ServerNode":
return cls(
json_dict["Url"],
json_dict["Database"],
json_dict["ClusterTag"],
ServerNode.Role(json_dict["ServerRole"]) if "ServerRole" in json_dict else None,
)

def __eq__(self, other) -> bool:
if self == other:
Expand All @@ -45,7 +54,7 @@ def __hash__(self) -> int:

@property
def last_server_version(self) -> str:
return self.__last_server_version
return self._last_server_version

@classmethod
def create_from(cls, topology: "ClusterTopology"):
Expand All @@ -64,16 +73,16 @@ def create_from(cls, topology: "ClusterTopology"):
return nodes

def should_update_server_version(self) -> bool:
if self.last_server_version is None or self.__last_server_version_check > 100:
if self.last_server_version is None or self._last_server_version_check > 100:
return True

self.__last_server_version_check += 1
self._last_server_version_check += 1
return False

def update_server_version(self, server_version: str):
self.__last_server_version = server_version
self.__last_server_version_check = 0
self._last_server_version = server_version
self._last_server_version_check = 0

def discard_server_version(self) -> None:
self.__last_server_version_check = None
self.__last_server_version_check = 0
self._last_server_version_check = None
self._last_server_version_check = 0
6 changes: 5 additions & 1 deletion ravendb/http/topology.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import uuid
from abc import abstractmethod
from concurrent.futures import ThreadPoolExecutor
from typing import TYPE_CHECKING, Optional
from typing import TYPE_CHECKING, Optional, Any
from typing import Union, List, Dict

from ravendb.exceptions.exceptions import (
Expand All @@ -25,6 +25,10 @@ def __init__(self, etag: int, nodes: List[ServerNode]):
self.etag = etag
self.nodes = nodes

@classmethod
def from_json(cls, json_dict: Dict[str, Any]) -> "Topology":
return cls(json_dict["Etag"], [ServerNode.from_json(node_json_dict) for node_json_dict in json_dict["Nodes"]])


class ClusterTopology:
def __init__(self):
Expand Down
12 changes: 8 additions & 4 deletions ravendb/json/result.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
from typing import List, Any, Dict
from typing import List, Any, Dict, Optional


class BatchCommandResult:
def __init__(self, results, transaction_index):
self.results: [None, list] = results
self.transaction_index: [None, int] = transaction_index
def __init__(self, results: Optional[List[Dict]], transaction_index: Optional[int]):
self.results = results
self.transaction_index = transaction_index

@classmethod
def from_json(cls, json_dict: Dict[str, Any]) -> "BatchCommandResult":
return cls(json_dict["Results"], json_dict["TransactionIndex"] if "TransactionIndex" in json_dict else None)


class JsonArrayResult:
Expand Down
10 changes: 2 additions & 8 deletions ravendb/serverwide/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from ravendb.tools.utils import Utils


class GetDatabaseTopologyCommand(RavenCommand):
class GetDatabaseTopologyCommand(RavenCommand[Topology]):
def __init__(self, debug_tag: Optional[str] = None, application_identifier: Optional[uuid.UUID] = None):
super().__init__(Topology)
self.__debug_tag = debug_tag
Expand All @@ -33,13 +33,7 @@ def create_request(self, node: ServerNode) -> requests.Request:
def set_response(self, response: str, from_cache: bool) -> None:
if response is None:
return

# todo: that's pretty bad way to do that, replace with initialization function that take nested object types
self.result: Topology = Utils.initialize_object(json.loads(response), self._result_class, True)
node_list = []
for node in self.result.nodes:
node_list.append(Utils.initialize_object(node, ServerNode, True))
self.result.nodes = node_list
self.result = Topology.from_json(json.loads(response))


class GetClusterTopologyCommand(RavenCommand[ClusterTopologyResponse]):
Expand Down
22 changes: 22 additions & 0 deletions ravendb/tests/issue_tests/test_RDBC_855.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from datetime import datetime

from pydantic import BaseModel

from ravendb.tests.test_base import TestBase


class User(BaseModel):
name: str = None
birthday: datetime = None
Id: str = None


class TestRDBC855(TestBase):
def test_storing_pydantic_objects(self):
with self.store.open_session() as session:
session.store(User(name="Josh", birthday=datetime(1999, 1, 1), Id="users/51"))
session.save_changes()

with self.store.open_session() as session:
user = session.load("users/51", User)
self.assertEqual("Josh", user.name)
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def __init__(self):
super(MyCounterIndex, self).__init__()
self.map = (
"counters.Companies.HeartRate.Select(counter => new {\n"
" heartBeat = counter.Value,\n"
" heart_beat = counter.Value,\n"
" name = counter.Name,\n"
" user = counter.DocumentId\n"
"})"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def test_can_get_revisions_by_change_vector(self):

for i in range(10):
with self.store.open_session() as session:
user = session.load(id_, Company)
user = session.load(id_, User)
user.name = f"Fitzchak{i}"
session.save_changes()

Expand Down Expand Up @@ -134,7 +134,7 @@ def test_collection_case_sensitive_test_1(self):

for i in range(10):
with self.store.open_session() as session:
user = session.load(id_, Company)
user = session.load(id_, User)
user.name = "raven" + str(i)
session.save_changes()

Expand All @@ -159,7 +159,7 @@ def test_collection_case_sensitive_test_2(self):

for i in range(10):
with self.store.open_session() as session:
user = session.load(id_, Company)
user = session.load(id_, User)
user.name = "raven" + str(i)
session.save_changes()

Expand Down Expand Up @@ -284,7 +284,7 @@ def test_can_get_metadata_for_lazily(self):

for i in range(10):
with self.store.open_session() as session:
user = session.load(id_, Company)
user = session.load(id_, User)
user.name = f"Omer{i}"
session.save_changes()

Expand Down Expand Up @@ -319,7 +319,7 @@ def test_can_get_for_lazily(self):

for i in range(10):
with self.store.open_session() as session:
user = session.load(id_, Company)
user = session.load(id_, User)
user.name = f"Omer{i}"
session.save_changes()

Expand Down Expand Up @@ -392,7 +392,7 @@ def test_can_get_revisions_by_change_vectors_lazily(self):

for i in range(10):
with self.store.open_session() as session:
user = session.load(id_, Company)
user = session.load(id_, User)
user.name = f"Omer{i}"
session.save_changes()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
import time
import unittest
from typing import Optional

from ravendb.documents.indexes.abstract_index_creation_tasks import AbstractIndexCreationTask
from ravendb.documents.session.loaders.include import QueryIncludeBuilder
from ravendb.documents.session.misc import TransactionMode, SessionOptions
from ravendb.documents.session.query import QueryStatistics
from ravendb.infrastructure.orders import Company, Address, Employee
from ravendb.tests.test_base import TestBase
from ravendb.util.util import StartingWithOptions


class Companies_ByName(AbstractIndexCreationTask):
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from ravendb import ServerNode
from ravendb.serverwide.commands import GetDatabaseTopologyCommand
from ravendb.tests.test_base import TestBase

Expand All @@ -18,4 +19,4 @@ def test_get_topology(self):
self.assertEqual(server_node.url, self.store.urls[0])
self.assertEqual(server_node.database, self.store.database)
self.assertEqual(server_node.cluster_tag, "A")
self.assertEqual(server_node.server_role, "Member")
self.assertEqual(server_node.server_role, ServerNode.Role.MEMBER)
Loading
Loading