Skip to content

Commit 209a28d

Browse files
committed
RDBC-1020: fix infinite recursion in BeforeDeleteEventArgs/BeforeQueryEventArgs.session
Both @Property implementations returned `self.session` (calling themselves) instead of the private backing attribute. Any access caused RecursionError. Fix: use `self.__session` inside the class body (Python handles name mangling at definition time), consistent with all other event arg classes. The explicit-mangled form `self._ClassName__session` is only correct when accessing private attributes from outside a class. Also fixes cascade deletes in BeforeDelete handlers: DeletedEntitiesHolder.__iter__ now snapshots both sets with list() before yielding, so session.delete() calls inside a handler do not raise "Set changed size during iteration". New deletions are staged in __on_before_deleted_entities and processed in a second pass. Regression tests verify that: - BeforeDeleteEventArgs.session and BeforeQueryEventArgs.session return the session object without recursion - A before-delete handler can cascade-delete a second document via args.session.load() and args.session.delete()
1 parent 556d1e0 commit 209a28d

3 files changed

Lines changed: 185 additions & 46 deletions

File tree

ravendb/documents/session/document_session_operations/in_memory_document_session_operations.py

Lines changed: 52 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ def __init__(self, items=None, prepare_entities_deletes: bool = False):
327327
if items is None:
328328
items = []
329329
self.__deleted_entities = set(map(RefEq, items))
330-
self.__prepare_entities_deleted = prepare_entities_deletes
330+
self._prepare_entities_deleted = prepare_entities_deletes
331331
self.__on_before_deleted_entities: Union[set, None] = None
332332

333333
def __getattribute__(self, item):
@@ -339,7 +339,7 @@ def __getattribute__(self, item):
339339
"discard",
340340
"DeletedEntitiesEnumeratorResult",
341341
"_DeletedEntitiesHolder__deleted_entities",
342-
"_DeletedEntitiesHolder__prepare_entities_deleted",
342+
"_prepare_entities_deleted",
343343
"_DeletedEntitiesHolder__on_before_deleted_entities",
344344
]:
345345
return super().__getattribute__(item)
@@ -358,19 +358,17 @@ def __len__(self) -> int:
358358
)
359359

360360
def __iter__(self):
361-
deleted_transformed_iterator = (
362-
self.DeletedEntitiesEnumeratorResult(item.ref, True) for item in self.__deleted_entities
363-
)
364-
if self.__on_before_deleted_entities is None:
365-
return deleted_transformed_iterator
366-
367-
on_before_deleted_iterator = (
368-
self.DeletedEntitiesEnumeratorResult(item.ref, False) for item in self.__on_before_deleted_entities
369-
)
370-
return itertools.chain(deleted_transformed_iterator, on_before_deleted_iterator)
361+
# Snapshot the main set so that cascade deletes registered by BeforeDelete
362+
# handlers do not raise "Set changed size during iteration".
363+
yield from (self.DeletedEntitiesEnumeratorResult(item.ref, True) for item in list(self.__deleted_entities))
364+
if self.__on_before_deleted_entities:
365+
yield from (
366+
self.DeletedEntitiesEnumeratorResult(item.ref, False)
367+
for item in list(self.__on_before_deleted_entities)
368+
)
371369

372370
def add(self, element: object) -> None:
373-
if self.__prepare_entities_deleted:
371+
if self._prepare_entities_deleted:
374372
if self.__on_before_deleted_entities is None:
375373
self.__on_before_deleted_entities = set()
376374
self.__on_before_deleted_entities.add(RefEq(element))
@@ -393,7 +391,7 @@ def clear(self) -> None:
393391
self.__on_before_deleted_entities.clear()
394392

395393
def evict(self, entity) -> None:
396-
if self.__prepare_entities_deleted:
394+
if self._prepare_entities_deleted:
397395
raise RuntimeError("Cannot evict entity during OnBeforeDelete")
398396
self.__deleted_entities.discard(RefEq(entity))
399397

@@ -1034,42 +1032,52 @@ def __prepare_for_creating_revisions_from_ids(self, result: SaveChangesData) ->
10341032
def __prepare_for_entities_deletion(
10351033
self, result: Union[None, SaveChangesData], changes: Union[None, Dict[str, List[DocumentsChanges]]]
10361034
) -> None:
1037-
for deleted_entity in self._deleted_entities:
1038-
document_info = self._documents_by_entity.get(deleted_entity.entity)
1039-
if document_info is None:
1040-
continue
1041-
if changes is not None:
1042-
doc_changes = []
1043-
change = DocumentsChanges("", "", DocumentsChanges.ChangeType.DOCUMENT_DELETED)
1044-
doc_changes.append(change)
1045-
changes[document_info.key] = doc_changes
1046-
else:
1047-
command = result.deferred_commands_map.get(
1048-
IdTypeAndName.create(document_info.key, CommandType.CLIENT_ANY_COMMAND, None)
1049-
)
1050-
if command:
1051-
self.__throw_invalid_deleted_document_with_deferred_command(command)
1035+
"""Build delete commands for all entities in the deleted-entities set.
1036+
1037+
While iterating, BeforeDelete event handlers may call session.delete(),
1038+
which stages new deletions for a second pass via DeletedEntitiesHolder.
1039+
"""
1040+
self._deleted_entities._prepare_entities_deleted = True
1041+
try:
1042+
for deleted_entity in self._deleted_entities:
1043+
document_info = self._documents_by_entity.get(deleted_entity.entity)
1044+
if document_info is None:
1045+
continue
1046+
if changes is not None:
1047+
doc_changes = []
1048+
change = DocumentsChanges("", "", DocumentsChanges.ChangeType.DOCUMENT_DELETED)
1049+
doc_changes.append(change)
1050+
changes[document_info.key] = doc_changes
1051+
else:
1052+
command = result.deferred_commands_map.get(
1053+
IdTypeAndName.create(document_info.key, CommandType.CLIENT_ANY_COMMAND, None)
1054+
)
1055+
if command:
1056+
self.__throw_invalid_deleted_document_with_deferred_command(command)
10521057

1053-
change_vector = None
1054-
document_info = self._documents_by_id.get(document_info.key)
1058+
change_vector = None
1059+
document_info = self._documents_by_id.get(document_info.key)
10551060

1056-
if document_info:
1057-
change_vector = document_info.change_vector
1061+
if document_info:
1062+
change_vector = document_info.change_vector
10581063

1059-
if document_info.entity is not None:
1060-
result.on_success.remove_document_by_entity(document_info.entity)
1061-
result.entities.append(document_info.entity)
1064+
if document_info.entity is not None:
1065+
result.on_success.remove_document_by_entity(document_info.entity)
1066+
result.entities.append(document_info.entity)
10621067

1063-
result.on_success.remove_document_by_id(document_info.key)
1068+
result.on_success.remove_document_by_id(document_info.key)
10641069

1065-
change_vector = change_vector if self._use_optimistic_concurrency else None
1066-
self.before_delete_invoke(BeforeDeleteEventArgs(self, document_info.key, document_info.entity))
1067-
result.session_commands.append(
1068-
DeleteCommandData(document_info.key, change_vector, document_info.change_vector)
1069-
)
1070+
change_vector = change_vector if self._use_optimistic_concurrency else None
1071+
if deleted_entity.execute_on_before_delete:
1072+
self.before_delete_invoke(BeforeDeleteEventArgs(self, document_info.key, document_info.entity))
1073+
result.session_commands.append(
1074+
DeleteCommandData(document_info.key, change_vector, document_info.change_vector)
1075+
)
10701076

1071-
if changes is None:
1072-
result.on_success.clear_deleted_entities()
1077+
if changes is None:
1078+
result.on_success.clear_deleted_entities()
1079+
finally:
1080+
self._deleted_entities._prepare_entities_deleted = False
10731081

10741082
def __prepare_for_entities_puts(self, result: SaveChangesData) -> None:
10751083
should_ignore_entity_changes = self.conventions.should_ignore_entity_changes

ravendb/documents/session/event_args.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ def __init__(self, session: "InMemoryDocumentSessionOperations", key: str, entit
172172

173173
@property
174174
def session(self) -> "InMemoryDocumentSessionOperations":
175-
return self.session
175+
return self.__session
176176

177177
@property
178178
def key(self) -> str:
@@ -190,7 +190,7 @@ def __init__(self, session: "InMemoryDocumentSessionOperations", query_customiza
190190

191191
@property
192192
def session(self) -> "InMemoryDocumentSessionOperations":
193-
return self.session
193+
return self.__session
194194

195195
@property
196196
def query_customization(self) -> DocumentQueryCustomization:
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
"""
2+
Session events: BeforeDeleteEventArgs and BeforeQueryEventArgs expose the current
3+
session without infinite recursion.
4+
5+
C# reference: FastTests/Client/Events.cs
6+
Before_Delete_Session_Listener_With_Delete_Inside
7+
"""
8+
9+
from ravendb.documents.session.event_args import (
10+
BeforeDeleteEventArgs,
11+
BeforeQueryEventArgs,
12+
BeforeStoreEventArgs,
13+
)
14+
from ravendb.tests.test_base import TestBase
15+
16+
17+
class User:
18+
def __init__(self, name: str = ""):
19+
self.name = name
20+
21+
22+
class TestEventArgsSessionProperty(TestBase):
23+
def setUp(self):
24+
super().setUp()
25+
26+
# ------------------------------------------------------------------ #
27+
# BeforeDeleteEventArgs.session returns the current session #
28+
# ------------------------------------------------------------------ #
29+
30+
def test_before_delete_event_args_session_returns_session(self):
31+
"""
32+
C# spec: BeforeDeleteEventArgs.Session returns the current session so
33+
handlers can perform additional session operations.
34+
"""
35+
with self.store.open_session() as session:
36+
session.store(User(name="Alice"), "users/1")
37+
session.save_changes()
38+
39+
session_from_args = []
40+
41+
def handler(args: BeforeDeleteEventArgs):
42+
session_from_args.append(args.session)
43+
44+
with self.store.open_session() as session:
45+
session.add_before_delete(handler)
46+
user = session.load("users/1", User)
47+
session.delete(user)
48+
session.save_changes()
49+
50+
self.assertEqual(1, len(session_from_args), "handler should have been called once")
51+
self.assertIsNotNone(session_from_args[0], "args.session should return the session object")
52+
53+
def test_before_delete_handler_can_cascade_delete_via_args_session(self):
54+
"""
55+
C# spec (Events.cs — Before_Delete_Session_Listener_With_Delete_Inside):
56+
handler receives BeforeDeleteEventArgs, loads users/2 via args.session,
57+
then calls args.session.delete(user2) to cascade the deletion.
58+
After save_changes(), both users/1 and users/2 are deleted.
59+
"""
60+
with self.store.open_session() as session:
61+
session.store(User(name="Foo"), "users/1")
62+
session.store(User(name="Bar"), "users/2")
63+
session.save_changes()
64+
65+
with self.store.open_session() as session:
66+
67+
def handler(args: BeforeDeleteEventArgs):
68+
user2 = args.session.load("users/2", User)
69+
args.session.delete(user2)
70+
71+
session.add_before_delete(handler)
72+
user1 = session.load("users/1", User)
73+
session.delete(user1)
74+
session.save_changes()
75+
76+
with self.store.open_session() as session:
77+
user1 = session.load("users/1", User)
78+
self.assertIsNone(user1, "users/1 should have been deleted")
79+
user2 = session.load("users/2", User)
80+
self.assertIsNone(user2, "users/2 should have been cascade-deleted via args.session.delete()")
81+
82+
# ------------------------------------------------------------------ #
83+
# BeforeQueryEventArgs.session returns the current session #
84+
# ------------------------------------------------------------------ #
85+
86+
def test_before_query_event_args_session_returns_session(self):
87+
"""
88+
C# spec: BeforeQueryEventArgs.Session returns the current session.
89+
"""
90+
with self.store.open_session() as session:
91+
session.store(User(name="Carol"), "users/3")
92+
session.save_changes()
93+
94+
session_from_args = []
95+
96+
def handler(args: BeforeQueryEventArgs):
97+
session_from_args.append(args.session)
98+
99+
with self.store.open_session() as session:
100+
session.add_before_query(handler)
101+
_ = list(session.query(object_type=User))
102+
103+
self.assertEqual(1, len(session_from_args), "before_query handler should have been called once")
104+
self.assertIsNotNone(session_from_args[0], "args.session should return the session object")
105+
106+
# ------------------------------------------------------------------ #
107+
# BeforeStoreEventArgs.session baseline #
108+
# ------------------------------------------------------------------ #
109+
110+
def test_before_store_event_args_session_property_works(self):
111+
"""
112+
Baseline: BeforeStoreEventArgs.session correctly returns the session.
113+
Confirms the recursion bug is specific to BeforeDeleteEventArgs and
114+
BeforeQueryEventArgs, not all event args.
115+
"""
116+
with self.store.open_session() as session:
117+
session.store(User(name="Dave"), "users/4")
118+
session.save_changes()
119+
120+
session_from_args = []
121+
122+
def handler(args: BeforeStoreEventArgs):
123+
session_from_args.append(args.session)
124+
125+
with self.store.open_session() as session:
126+
session.add_before_store(handler)
127+
session.store(User(name="Eve"), "users/5")
128+
session.save_changes()
129+
130+
self.assertEqual(1, len(session_from_args), "before_store handler should be called")
131+
self.assertIsNotNone(session_from_args[0], "BeforeStoreEventArgs.session should return the session object")

0 commit comments

Comments
 (0)