From 0efa5d5b4f2940e4db3f95c0729ebd7769237920 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 25 Jun 2026 10:17:21 +0100 Subject: [PATCH] Replace Clone with Py<..> references --- rust/src/events/relations.rs | 67 +++++++++++++++++++++++++++++------- rust/src/events/serialize.rs | 5 +-- 2 files changed, 58 insertions(+), 14 deletions(-) diff --git a/rust/src/events/relations.rs b/rust/src/events/relations.rs index 2e99d36ba4..472d3e4f62 100644 --- a/rust/src/events/relations.rs +++ b/rust/src/events/relations.rs @@ -23,17 +23,16 @@ //! Python handles; cloning an `Event` is cheap (it shares the underlying data //! behind `Arc`s) and the events are only ever read here. -use pyo3::{pyclass, pymethods}; +use pyo3::{pyclass, pymethods, Py, PyTraverseError, PyVisit}; use crate::events::{json_object::JsonObject, Event}; /// A thread's bundled summary: its latest event, the number of events in the /// thread, and whether the requesting user has participated. #[pyclass(frozen, skip_from_py_object, get_all)] -#[derive(Clone)] pub struct ThreadAggregation { /// The latest event in the thread. - pub latest_event: Event, + pub latest_event: Py, /// The total number of events in the thread. pub count: i64, /// Whether the requesting user has sent an event to the thread. @@ -43,13 +42,35 @@ pub struct ThreadAggregation { #[pymethods] impl ThreadAggregation { #[new] - fn new(latest_event: &Event, count: i64, current_user_participated: bool) -> Self { + fn new(latest_event: Py, count: i64, current_user_participated: bool) -> Self { Self { - latest_event: latest_event.clone(), + latest_event, count, current_user_participated, } } + + #[getter] + fn latest_event(&self) -> &Py { + &self.latest_event + } + + #[getter] + fn count(&self) -> i64 { + self.count + } + + #[getter] + fn current_user_participated(&self) -> bool { + self.current_user_participated + } + + /// The Python GC needs to know that this object references the latest + /// event. + fn __traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + visit.call(&self.latest_event)?; + Ok(()) + } } /// The bundled aggregations for a single event. @@ -57,14 +78,13 @@ impl ThreadAggregation { /// Some values require additional processing during serialization (the edit /// and the thread's latest event are themselves serialized). #[pyclass(frozen, skip_from_py_object, get_all)] -#[derive(Clone)] pub struct BundledAggregations { /// The `m.reference` aggregation (e.g. `{"chunk": [{"event_id": ...}]}`). pub references: Option, /// The edit (`m.replace`) event that applies to this event. - pub replace: Option, + pub replace: Option>, /// The thread (`m.thread`) summary for this event. - pub thread: Option, + pub thread: Option>, } #[pymethods] @@ -73,16 +93,31 @@ impl BundledAggregations { #[pyo3(signature = (references = None, replace = None, thread = None))] fn new( references: Option, - replace: Option<&Event>, - thread: Option<&ThreadAggregation>, + replace: Option>, + thread: Option>, ) -> Self { Self { references, - replace: replace.cloned(), - thread: thread.cloned(), + replace, + thread, } } + #[getter] + fn references(&self) -> Option { + self.references.clone() + } + + #[getter] + fn replace(&self) -> Option<&Py> { + self.replace.as_ref() + } + + #[getter] + fn thread(&self) -> Option<&Py> { + self.thread.as_ref() + } + /// Whether there are any aggregations to bundle. /// /// Matches the Python `bool(self.references or self.replace or self.thread)`: @@ -92,4 +127,12 @@ impl BundledAggregations { || self.replace.is_some() || self.thread.is_some() } + + /// The Python GC needs to know that this object references the latest + /// event. + fn __traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError> { + visit.call(&self.replace)?; + visit.call(&self.thread)?; + Ok(()) + } } diff --git a/rust/src/events/serialize.rs b/rust/src/events/serialize.rs index 7d782fc749..1f1e5083d7 100644 --- a/rust/src/events/serialize.rs +++ b/rust/src/events/serialize.rs @@ -362,7 +362,7 @@ fn inject_bundled_aggregations( // sender, but per MSC3925 we include the full edit. // https://spec.matrix.org/v1.5/client-server-api/#server-side-aggregation-of-mreplace-relationships let serialized = serialize_event( - replace, + replace.get(), time_now_ms, config, None, @@ -375,10 +375,11 @@ fn inject_bundled_aggregations( } if let Some(thread) = &aggregation.thread { + let thread = thread.get(); // The thread's latest event is serialized with the same bundle map, so // it may recurse further. let serialized_latest = serialize_event( - &thread.latest_event, + thread.latest_event.get(), time_now_ms, config, None,