From fb84d58b901d8a05c94a531272cbcc283308ccf9 Mon Sep 17 00:00:00 2001 From: Federico Lois Date: Fri, 19 Sep 2025 13:19:59 -0300 Subject: [PATCH] RDBC-949: Refactors is_read_request to a method Ensures `is_read_request` is consistently implemented as a method rather than a property across all commands. This change fixes potential issues where the intended behavior of checking if a command is a read request might not be correctly executed due to inconsistent implementation. Adds a test to verify the consistent implementation of `is_read_request`. --- ravendb/documents/commands/batches.py | 3 +- ravendb/documents/commands/crud.py | 1 - ravendb/documents/commands/multi_get.py | 1 - ravendb/http/request_executor.py | 4 +- .../test_platform_contracts.py | 52 +++++++++++++++++++ 5 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 ravendb/tests/raven_commands_tests/test_platform_contracts.py diff --git a/ravendb/documents/commands/batches.py b/ravendb/documents/commands/batches.py index 79d3f8ec..53538f1c 100644 --- a/ravendb/documents/commands/batches.py +++ b/ravendb/documents/commands/batches.py @@ -130,8 +130,7 @@ def __enter__(self): def __exit__(self, exc_type, exc_val, exc_tb): pass - @property - def is_read_request(self): + def is_read_request(self) -> bool: return False def create_request(self, node: ServerNode) -> requests.Request: diff --git a/ravendb/documents/commands/crud.py b/ravendb/documents/commands/crud.py index c474c3d2..ae14fe1e 100644 --- a/ravendb/documents/commands/crud.py +++ b/ravendb/documents/commands/crud.py @@ -324,7 +324,6 @@ def prepare_request_with_multiple_ids( def set_response(self, response: str, from_cache: bool) -> None: self.result = GetDocumentsResult.from_json(json.loads(response)) if response is not None else None - @property def is_read_request(self) -> bool: return True diff --git a/ravendb/documents/commands/multi_get.py b/ravendb/documents/commands/multi_get.py index 0e5c65cb..58eea887 100644 --- a/ravendb/documents/commands/multi_get.py +++ b/ravendb/documents/commands/multi_get.py @@ -248,7 +248,6 @@ def read_response(response_json: dict) -> GetResponse: return get_response - @property def is_read_request(self) -> bool: return False diff --git a/ravendb/http/request_executor.py b/ravendb/http/request_executor.py index 0658fc84..a0c00fd4 100644 --- a/ravendb/http/request_executor.py +++ b/ravendb/http/request_executor.py @@ -691,7 +691,7 @@ def choose_node_for_request(self, cmd: RavenCommand, session_info: SessionInfo) if session_info is not None and session_info.can_use_load_balance_behavior: return self._node_selector.get_node_by_session_id(session_info.session_id) - if not cmd.is_read_request: + if not cmd.is_read_request(): return self._node_selector.get_preferred_node() if self.conventions.read_balance_behavior == ReadBalanceBehavior.NONE: @@ -783,7 +783,7 @@ def _get_from_cache( if ( use_cache and command.can_cache - and command.is_read_request + and command.is_read_request() and command.response_type == RavenCommandResponseType.OBJECT ): return self._cache.get(url) diff --git a/ravendb/tests/raven_commands_tests/test_platform_contracts.py b/ravendb/tests/raven_commands_tests/test_platform_contracts.py new file mode 100644 index 00000000..538f5e8d --- /dev/null +++ b/ravendb/tests/raven_commands_tests/test_platform_contracts.py @@ -0,0 +1,52 @@ +import unittest +import importlib +import inspect +import pkgutil +import ravendb + +from ravendb.tests.test_base import TestBase + + +class TestPlatformContracts(TestBase): + def setUp(self): + super(TestPlatformContracts, self).setUp() + + def tearDown(self): + super(TestPlatformContracts, self).tearDown() + TestBase.delete_all_topology_files() + + def test_all_commands_expose_callable_is_read_request(self): + from ravendb.http.raven_command import RavenCommand + + offenders = [] + for mod in self._import_all_ravendb_modules(): + for cls in self._iter_subclasses_in_module(mod, RavenCommand): + attr = getattr(cls, "is_read_request", None) + if isinstance(attr, property): + offenders.append(f"{cls.__module__}.{cls.__qualname__}") + + if offenders: + self.fail("is_read_request must be a method on: " + ", ".join(offenders)) + + # private helpers + def _import_all_ravendb_modules(self): + for _, modname, _ in pkgutil.walk_packages(ravendb.__path__, ravendb.__name__ + "."): + if modname.startswith("ravendb.tests."): + continue + try: + yield importlib.import_module(modname) + except Exception: + continue + + def _iter_subclasses_in_module(self, mod, base_cls): + for _, obj in inspect.getmembers(mod): + if inspect.isclass(obj) and issubclass(obj, base_cls) and obj is not base_cls: + yield obj + if inspect.isclass(obj): + for _, inner in inspect.getmembers(obj, inspect.isclass): + if issubclass(inner, base_cls) and inner is not base_cls: + yield inner + + +if __name__ == "__main__": + unittest.main