Skip to content

Commit e5648c6

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 4cd66c4 commit e5648c6

3 files changed

Lines changed: 180 additions & 42 deletions

File tree

ravendb/documents/session/document_session_operations/in_memory_document_session_operations.py

Lines changed: 47 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -358,16 +358,15 @@ 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+
for item in list(self.__deleted_entities):
364+
yield self.DeletedEntitiesEnumeratorResult(item.ref, True)
365+
# Entities added by BeforeDelete handlers (via __on_before_deleted_entities)
366+
# are processed in a second pass once the main set is exhausted.
367+
if self.__on_before_deleted_entities:
368+
for item in list(self.__on_before_deleted_entities):
369+
yield self.DeletedEntitiesEnumeratorResult(item.ref, False)
371370

372371
def add(self, element: object) -> None:
373372
if self.__prepare_entities_deleted:
@@ -1034,42 +1033,50 @@ def __prepare_for_creating_revisions_from_ids(self, result: SaveChangesData) ->
10341033
def __prepare_for_entities_deletion(
10351034
self, result: Union[None, SaveChangesData], changes: Union[None, Dict[str, List[DocumentsChanges]]]
10361035
) -> 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)
1036+
# Allow BeforeDelete handlers to call session.delete() without corrupting
1037+
# the iteration: new deletions are staged in __on_before_deleted_entities
1038+
# and processed in the second pass of DeletedEntitiesHolder.__iter__.
1039+
self._deleted_entities._DeletedEntitiesHolder__prepare_entities_deleted = True
1040+
try:
1041+
for deleted_entity in self._deleted_entities:
1042+
document_info = self._documents_by_entity.get(deleted_entity.entity)
1043+
if document_info is None:
1044+
continue
1045+
if changes is not None:
1046+
doc_changes = []
1047+
change = DocumentsChanges("", "", DocumentsChanges.ChangeType.DOCUMENT_DELETED)
1048+
doc_changes.append(change)
1049+
changes[document_info.key] = doc_changes
1050+
else:
1051+
command = result.deferred_commands_map.get(
1052+
IdTypeAndName.create(document_info.key, CommandType.CLIENT_ANY_COMMAND, None)
1053+
)
1054+
if command:
1055+
self.__throw_invalid_deleted_document_with_deferred_command(command)
10521056

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

1056-
if document_info:
1057-
change_vector = document_info.change_vector
1060+
if document_info:
1061+
change_vector = document_info.change_vector
10581062

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)
1063+
if document_info.entity is not None:
1064+
result.on_success.remove_document_by_entity(document_info.entity)
1065+
result.entities.append(document_info.entity)
10621066

1063-
result.on_success.remove_document_by_id(document_info.key)
1067+
result.on_success.remove_document_by_id(document_info.key)
10641068

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-
)
1069+
change_vector = change_vector if self._use_optimistic_concurrency else None
1070+
if deleted_entity.execute_on_before_delete:
1071+
self.before_delete_invoke(BeforeDeleteEventArgs(self, document_info.key, document_info.entity))
1072+
result.session_commands.append(
1073+
DeleteCommandData(document_info.key, change_vector, document_info.change_vector)
1074+
)
10701075

1071-
if changes is None:
1072-
result.on_success.clear_deleted_entities()
1076+
if changes is None:
1077+
result.on_success.clear_deleted_entities()
1078+
finally:
1079+
self._deleted_entities._DeletedEntitiesHolder__prepare_entities_deleted = False
10731080

10741081
def __prepare_for_entities_puts(self, result: SaveChangesData) -> None:
10751082
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)