From fc9279921cfacbb96f2ecf1b15e560d928a6327b Mon Sep 17 00:00:00 2001 From: Federico Andres Lois Date: Thu, 26 Feb 2026 17:40:49 -0300 Subject: [PATCH] RDBC-1034 execute_indexes([]) and IndexCreation.create_indexes([]) are no-ops; fix PutIndexesOperation, GetIndexesOperation, and EnableIndexOperation - execute_indexes([]) and IndexCreation.create_indexes([]) return early, preventing a spurious "Could not create indexes in one shot" warning log - PutIndexesOperation raises ValueError for empty args (matches C# ArgumentNullException) - PutIndexesOperation.set_response: parse JSON before checking for "Error" key; the old code did a string substring search then subscripted the raw string, causing server errors to be silently swallowed as TypeError - GetIndexesOperation URL: fixed missing "=" in "&pageSize={n}" parameter (was "&pageSize{n}"), which caused the server to silently ignore pagination - EnableIndexOperation: add RaftCommand base class and rename raft_unique_request_id to get_raft_unique_request_id, matching C# IRaftCommand and DisableIndexOperation --- ravendb/documents/indexes/index_creation.py | 2 + ravendb/documents/operations/indexes.py | 33 ++--- ravendb/documents/store/definition.py | 2 + ravendb/tests/issue_tests/test_RDBC_1034.py | 133 ++++++++++++++++++ .../index_tests/test_index_operation.py | 70 +++++++++ 5 files changed, 219 insertions(+), 21 deletions(-) create mode 100644 ravendb/tests/issue_tests/test_RDBC_1034.py diff --git a/ravendb/documents/indexes/index_creation.py b/ravendb/documents/indexes/index_creation.py index 6904abea..c0e584f0 100644 --- a/ravendb/documents/indexes/index_creation.py +++ b/ravendb/documents/indexes/index_creation.py @@ -17,6 +17,8 @@ def create_indexes( store: "DocumentStore", conventions: Optional["DocumentConventions"] = None, ) -> None: + if not indexes: + return if conventions is None: conventions = store.conventions diff --git a/ravendb/documents/operations/indexes.py b/ravendb/documents/operations/indexes.py index ace915d6..d8c5b46b 100644 --- a/ravendb/documents/operations/indexes.py +++ b/ravendb/documents/operations/indexes.py @@ -28,8 +28,7 @@ class PutIndexesOperation(MaintenanceOperation): def __init__(self, *indexes_to_add: IndexDefinition): if len(indexes_to_add) == 0: - raise ValueError("Invalid indexes_to_add") - + raise ValueError("indexes_to_add cannot be empty") super(PutIndexesOperation, self).__init__() self._indexes_to_add = indexes_to_add self.__all_java_script_indexes = True # todo: set it in the command @@ -67,9 +66,12 @@ def create_request(self, node: ServerNode) -> requests.Request: return request def set_response(self, response: str, from_cache: bool) -> None: - self.result = json.loads(response) # todo: PutIndexResult instead of dict - if "Error" in response: - raise ErrorResponseException(response["Error"]) + if response is None: + self._throw_invalid_response() + result = json.loads(response) # todo: PutIndexResult instead of dict + if "Error" in result: + raise ErrorResponseException(result["Error"]) + self.result = result class GetIndexNamesOperation(MaintenanceOperation): @@ -176,7 +178,7 @@ def __init__(self, index_name: str, cluster_wide: bool = False): def get_command(self, conventions) -> VoidRavenCommand: return self.__EnableIndexCommand(self.__index_name, self.__cluster_wide) - class __EnableIndexCommand(VoidRavenCommand): + class __EnableIndexCommand(VoidRavenCommand, RaftCommand): def __init__(self, index_name: str, cluster_wide: bool): if index_name is None: raise ValueError("index_name cannot be None") @@ -192,7 +194,7 @@ def create_request(self, server_node) -> requests.Request: f"&clusterWide={self.__cluster_wide}", ) - def raft_unique_request_id(self) -> str: + def get_raft_unique_request_id(self) -> str: return RaftIdGenerator.new_id() @@ -440,7 +442,7 @@ def create_request(self, server_node) -> requests.Request: return requests.Request( "GET", f"{server_node.url}/databases/{server_node.database}" - f"/indexes?start={self.__start}&pageSize{self.__page_size} ", + f"/indexes?start={self.__start}&pageSize={self.__page_size}", ) def set_response(self, response: str, from_cache: bool) -> None: @@ -501,7 +503,7 @@ def get_command(self, conventions: "DocumentConventions") -> VoidRavenCommand: def __filter_auto_indexes(self): for name in self.__index_names: - if name.startswith("auto/"): + if name.lower().startswith("auto/"): raise ValueError("Index list contains Auto-Indexes. Lock Mode is not set for Auto-Indexes") class __SetIndexesLockCommand(VoidRavenCommand, RaftCommand): @@ -614,18 +616,7 @@ def is_read_request(self) -> bool: def create_request(self, server_node: ServerNode) -> requests.Request: request = requests.Request("POST") request.url = f"{server_node.url}/databases/{server_node.database}/indexes/has-changed" - request.data = { - "Configuration": self.__index.configuration, - "Fields": self.__index.fields, - "LockMode": self.__index.lock_mode, - "Maps": self.__index.maps, - "Name": self.__index.name, - "OutputReduceToCollection": self.__index.output_reduce_to_collection, - "Priority": self.__index.priority, - "Reduce": self.__index.reduce, - "SourceType": self.__index.source_type, - "Type": self.__index.type, - } + request.data = self.__index.to_json() return request def set_response(self, response: str, from_cache: bool) -> None: diff --git a/ravendb/documents/store/definition.py b/ravendb/documents/store/definition.py index d1bbd7e8..059b6461 100644 --- a/ravendb/documents/store/definition.py +++ b/ravendb/documents/store/definition.py @@ -475,6 +475,8 @@ def execute_index(self, task: "AbstractIndexCreationTask", database: Optional[st def execute_indexes(self, tasks: "List[AbstractIndexCreationTask]", database: Optional[str] = None) -> None: self.assert_initialized() + if not tasks: + return indexes_to_add = IndexCreation.create_indexes_to_add(tasks, self.conventions) self.maintenance.for_database(self.get_effective_database(database)).send(PutIndexesOperation(*indexes_to_add)) diff --git a/ravendb/tests/issue_tests/test_RDBC_1034.py b/ravendb/tests/issue_tests/test_RDBC_1034.py new file mode 100644 index 00000000..51f18abf --- /dev/null +++ b/ravendb/tests/issue_tests/test_RDBC_1034.py @@ -0,0 +1,133 @@ +""" +RDBC-1034: store.execute_indexes([]) should be a no-op, not raise ValueError. + +C# reference: FastTests/Client/Indexing/IndexesFromClient.cs + CreateIndexes_Should_Not_Throw_When_Indexes_List_Is_Empty (RavenDB-24077) +""" + +import logging +import unittest + +from ravendb.documents.operations.indexes import ( + EnableIndexOperation, + GetIndexesOperation, + PutIndexesOperation, +) +from ravendb.http.server_node import ServerNode +from ravendb.http.topology import RaftCommand +from ravendb.tests.test_base import TestBase + + +class TestExecuteIndexesEmptyUnit(unittest.TestCase): + """Unit tests — no server required.""" + + def test_put_indexes_operation_empty_args_raises(self): + # PutIndexesOperation with no args should raise ValueError (matches C# ArgumentNullException) + with self.assertRaises(ValueError): + PutIndexesOperation() + + def test_put_indexes_set_response_parses_error_from_dict(self): + # set_response must parse the JSON payload before checking for an "Error" field. + # A raw substring search can misclassify successful payloads and break on dict-style access. + from ravendb.documents.operations.indexes import PutIndexesOperation + from ravendb.documents.indexes.definitions import IndexDefinition + import json + + idx = IndexDefinition() + idx.name = "TestIndex" + idx.maps = {"from d in docs select new { d.Name }"} + op = PutIndexesOperation(idx) + + class FakeConventions: + pass + + cmd = op.get_command(FakeConventions()) + + # A valid-looking success response must not raise + success_payload = json.dumps([{"Index": "TestIndex", "RaftCommandIndex": 1}]) + cmd.set_response(success_payload, False) + self.assertEqual(cmd.result[0]["Index"], "TestIndex") + + # A response that contains "Error" only as a substring in index names must not raise + harmless_payload = json.dumps([{"Index": "ErrorTracker", "RaftCommandIndex": 2}]) + cmd.set_response(harmless_payload, False) + self.assertEqual(cmd.result[0]["Index"], "ErrorTracker") + + def test_get_indexes_operation_url_includes_page_size(self): + # The URL must include &pageSize=N, not the malformed &pageSizeN + node = ServerNode("http://localhost:8080", "TestDb") + + op = GetIndexesOperation(0, 25) + + class FakeConventions: + pass + + cmd = op.get_command(FakeConventions()) + req = cmd.create_request(node) + self.assertIn("pageSize=25", req.url) + self.assertNotIn("pageSize25", req.url) + self.assertFalse(req.url.endswith(" "), "URL must not have trailing space") + + def test_enable_index_command_is_raft_command(self): + # C# EnableIndexCommand implements IRaftCommand; Python must match. + op = EnableIndexOperation("MyIndex") + + class FakeConventions: + pass + + cmd = op.get_command(FakeConventions()) + self.assertIsInstance(cmd, RaftCommand) + self.assertTrue( + callable(getattr(cmd, "get_raft_unique_request_id", None)), + "EnableIndexCommand must implement get_raft_unique_request_id", + ) + # Two calls must return different IDs (each request gets its own Raft slot) + id1 = cmd.get_raft_unique_request_id() + id2 = cmd.get_raft_unique_request_id() + self.assertNotEqual(id1, id2) + + def test_create_indexes_empty_list_does_not_log(self): + from ravendb.documents.indexes.index_creation import IndexCreation + + class FakeStore: + class FakeMaintenance: + def send(self, op): + raise AssertionError("send() should not be called for empty list") + + maintenance = FakeMaintenance() + conventions = None + + with self.assertLogs(level=logging.WARNING) as cm: + logging.warning("sentinel") # ensure assertLogs doesn't fail on empty + IndexCreation.create_indexes([], FakeStore()) + + self.assertFalse( + any("Could not create indexes" in line for line in cm.output), + "Empty-list call must not emit a 'Could not create indexes' log entry", + ) + + +class TestExecuteIndexesEmpty(TestBase): + """Integration tests — require a live server.""" + + def setUp(self): + super().setUp() + self.store = self.get_document_store() + + def tearDown(self): + super().tearDown() + self.store.close() + + def test_execute_indexes_empty_list_is_noop(self): + # Should NOT raise + self.store.execute_indexes([]) + + def test_index_creation_create_indexes_empty_is_noop(self): + from ravendb.documents.indexes.index_creation import IndexCreation + + # IndexCreation.create_indexes with empty list should not raise + IndexCreation.create_indexes([], self.store) + + +if __name__ == "__main__": + unittest.main() diff --git a/ravendb/tests/jvm_migrated_tests/index_tests/test_index_operation.py b/ravendb/tests/jvm_migrated_tests/index_tests/test_index_operation.py index 3a0b78c1..0f1bb2c6 100644 --- a/ravendb/tests/jvm_migrated_tests/index_tests/test_index_operation.py +++ b/ravendb/tests/jvm_migrated_tests/index_tests/test_index_operation.py @@ -17,12 +17,15 @@ GetIndexErrorsOperation, GetIndexesOperation, GetIndexOperation, + ResetIndexOperation, SetIndexesLockOperation, SetIndexesPriorityOperation, GetTermsOperation, IndexHasChangedOperation, ) +from ravendb.documents.operations.statistics import GetStatisticsOperation + from ravendb.tests.test_base import TestBase, UserWithId @@ -177,3 +180,70 @@ def test_has_index_changed(self): self.assertFalse(self.store.maintenance.send(IndexHasChangedOperation(index))) index.maps = ("from users",) self.assertTrue(self.store.maintenance.send(IndexHasChangedOperation(index))) + + def test_has_index_changed_additional_sources(self): + index = UsersIndex() + self.store.maintenance.send(PutIndexesOperation(index)) + self.assertFalse(self.store.maintenance.send(IndexHasChangedOperation(index))) + index.additional_sources = {"Helper.cs": "public static string Format(string s) => s;"} + self.assertTrue(self.store.maintenance.send(IndexHasChangedOperation(index))) + + def test_can_reset_index(self): + index = UsersIndex() + self.store.maintenance.send(PutIndexesOperation(index)) + with self.store.open_session() as session: + session.store(UserWithId("John")) + session.save_changes() + self.wait_for_indexing(self.store, self.store.database) + + stats = self.store.maintenance.send(GetStatisticsOperation()) + self.assertEqual(0, len(stats.stale_indexes)) + + # Stop the indexer so the index stays stale after reset, allowing a deterministic + # stale-indexes assertion without racing against the re-indexing cycle. + self.store.maintenance.send(StopIndexingOperation()) + self.store.maintenance.send(ResetIndexOperation(index.name)) + + stats = self.store.maintenance.send(GetStatisticsOperation()) + self.assertEqual(1, len(stats.stale_indexes)) + self.assertEqual(index.name, stats.stale_indexes[0].name) + + def test_get_definition_for_non_existent_index_returns_none(self): + index = UsersIndex() + self.store.maintenance.send(PutIndexesOperation(index)) + + result = self.store.maintenance.send(GetIndexOperation("does-not-exist")) + self.assertIsNone(result) + + result = self.store.maintenance.send(GetIndexOperation(index.name)) + self.assertIsNotNone(result) + self.assertEqual(index.name, result.name) + self.assertEqual(index.maps, result.maps) + + def test_set_lock_mode_on_auto_index_raises(self): + with self.store.open_session() as session: + session.store(UserWithId("Jane")) + session.save_changes() + with self.store.open_session() as session: + list(session.query(object_type=UserWithId).where_equals("name", "Jane")) + self.wait_for_indexing(self.store, self.store.database) + auto_index_names = [ + name + for name in self.store.maintenance.send(GetIndexNamesOperation(0, 25)) + if name.lower().startswith("auto/") + ] + self.assertGreater(len(auto_index_names), 0) + auto_index_name = auto_index_names[0] + + stats = self.store.maintenance.send(GetIndexStatisticsOperation(auto_index_name)) + self.assertEqual(IndexLockMode.UNLOCK, stats.lock_mode) + self.assertEqual(IndexPriority.NORMAL, stats.priority) + + with self.assertRaises(ValueError): + self.store.maintenance.send(SetIndexesLockOperation(IndexLockMode.LOCKED_IGNORE, auto_index_name)) + + # priority can still be set on auto-indexes even though lock mode cannot + self.store.maintenance.send(SetIndexesPriorityOperation(IndexPriority.LOW, auto_index_name)) + stats = self.store.maintenance.send(GetIndexStatisticsOperation(auto_index_name)) + self.assertEqual(IndexLockMode.UNLOCK, stats.lock_mode) + self.assertEqual(IndexPriority.LOW, stats.priority)