From 4c7d2b50e936ceb1dbfea34fbfa65dd1842560ac Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 25 Jun 2026 10:13:03 +0100 Subject: [PATCH] Revert "Replace Clone with shallow_copy" This reverts commit 1ace5316ae8602fe362eb631828e0f1f65f9d1fd. --- rust/src/events/formats/mod.rs | 25 ++++---------- rust/src/events/mod.rs | 19 +++-------- rust/src/events/relations.rs | 60 ++++------------------------------ 3 files changed, 16 insertions(+), 88 deletions(-) diff --git a/rust/src/events/formats/mod.rs b/rust/src/events/formats/mod.rs index 6baada32dc..8f24ffd757 100644 --- a/rust/src/events/formats/mod.rs +++ b/rust/src/events/formats/mod.rs @@ -95,11 +95,11 @@ pub use vmsc4242::EventFormatVMSC4242; /// pyclass. /// /// The `signatures` and `unsigned` fields are kept separate from the other -/// fields as they are mutable. Note [`FormattedEvent::shallow_copy`] is -/// *shallow*: it shares the mutable `signatures`/`unsigned`/internal state -/// behind their `Arc`s (cheap, and fine for read-only uses such as bundled -/// aggregations). Use [`FormattedEvent::deep_copy`] when an independently-mutable -/// copy is required. `common_fields` and `specific_fields` are both +/// fields as they are mutable. Note the derived [`Clone`] is *shallow*: it +/// shares the mutable `signatures`/`unsigned`/internal state behind their +/// `Arc`s (cheap, and fine for read-only uses such as bundled aggregations). +/// Use [`FormattedEvent::deep_copy`] when an independently-mutable copy is +/// required. `common_fields` and `specific_fields` are both /// `#[serde(flatten)]`ed so that the serialised JSON is a single flat object /// matching the Matrix spec. /// @@ -109,7 +109,7 @@ pub use vmsc4242::EventFormatVMSC4242; /// Instead, deserialize directly from a JSON string with /// `serde_json::from_str`. See https://github.com/serde-rs/serde/issues/2230 /// for details. -#[derive(Serialize, Deserialize)] +#[derive(Clone, Serialize, Deserialize)] pub struct FormattedEvent> { /// The event's signatures. /// @@ -154,19 +154,6 @@ impl FormattedEvent { } } - /// Returns a shallow copy of this object, sharing the mutable - /// `signatures`/`unsigned`/`internal_metadata` with the original. This - /// mimics the behaviour python references (which share the underlying - /// object). - pub fn shallow_copy(&self) -> FormattedEvent { - FormattedEvent { - signatures: self.signatures.clone(), - unsigned: self.unsigned.clone(), - specific_fields: Arc::clone(&self.specific_fields), - common_fields: Arc::clone(&self.common_fields), - } - } - pub fn validate(&self) -> Result<(), Error> { match &*self.specific_fields { EventFormatEnum::V1(format) => format.validate(&self.common_fields), diff --git a/rust/src/events/mod.rs b/rust/src/events/mod.rs index 1abdeafb2a..21d56e8e7a 100644 --- a/rust/src/events/mod.rs +++ b/rust/src/events/mod.rs @@ -137,7 +137,11 @@ pub fn register_module(py: Python<'_>, m: &Bound<'_, PyModule>) -> PyResult<()> /// metadata, rejection reason, and a reference to the room version that /// produced this event). See the module-level docs for the high-level /// design. +/// +/// `Clone` is shallow (see [`FormattedEvent`]) and lets an `Event` be held by +/// value, e.g. inside [`BundledAggregations`](crate::events::relations::BundledAggregations). #[pyclass(frozen, weakref, skip_from_py_object)] +#[derive(Clone)] pub struct Event { /// The parsed event JSON. parsed_event: FormattedEvent, @@ -389,21 +393,6 @@ impl Event { Ok(new_event) } - /// Returns a shallow copy of this object, sharing the mutable - /// `signatures`/`unsigned`/`internal_metadata` with the original. This - /// mimics the behaviour python references (which share the underlying - /// object). - pub fn shallow_copy(&self) -> Event { - Event { - parsed_event: self.parsed_event.shallow_copy(), - internal_metadata: self.internal_metadata.clone(), - room_version: self.room_version, - rejected_reason: self.rejected_reason.clone(), - event_id: self.event_id.clone(), - room_id: self.room_id.clone(), - } - } - /// If this event has the `msc4354_sticky` top-level field, returns a /// `SynapseDuration` representing the sticky duration. Otherwise returns /// `None`. diff --git a/rust/src/events/relations.rs b/rust/src/events/relations.rs index b58a1b607c..2e99d36ba4 100644 --- a/rust/src/events/relations.rs +++ b/rust/src/events/relations.rs @@ -29,7 +29,8 @@ 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)] +#[pyclass(frozen, skip_from_py_object, get_all)] +#[derive(Clone)] pub struct ThreadAggregation { /// The latest event in the thread. pub latest_event: Event, @@ -39,48 +40,24 @@ pub struct ThreadAggregation { pub current_user_participated: bool, } -impl Clone for ThreadAggregation { - fn clone(&self) -> Self { - Self { - latest_event: self.latest_event.shallow_copy(), - count: self.count, - current_user_participated: self.current_user_participated, - } - } -} - #[pymethods] impl ThreadAggregation { #[new] fn new(latest_event: &Event, count: i64, current_user_participated: bool) -> Self { Self { - latest_event: latest_event.shallow_copy(), + latest_event: latest_event.clone(), count, current_user_participated, } } - - #[getter] - fn latest_event(&self) -> Event { - self.latest_event.shallow_copy() - } - - #[getter] - fn count(&self) -> i64 { - self.count - } - - #[getter] - fn current_user_participated(&self) -> bool { - self.current_user_participated - } } /// The bundled aggregations for a single event. /// /// Some values require additional processing during serialization (the edit /// and the thread's latest event are themselves serialized). -#[pyclass(frozen, skip_from_py_object)] +#[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, @@ -90,16 +67,6 @@ pub struct BundledAggregations { pub thread: Option, } -impl Clone for BundledAggregations { - fn clone(&self) -> Self { - Self { - references: self.references.clone(), - replace: self.replace.as_ref().map(Event::shallow_copy), - thread: self.thread.clone(), - } - } -} - #[pymethods] impl BundledAggregations { #[new] @@ -111,26 +78,11 @@ impl BundledAggregations { ) -> Self { Self { references, - replace: replace.map(Event::shallow_copy), + replace: replace.cloned(), thread: thread.cloned(), } } - #[getter] - fn references(&self) -> Option { - self.references.clone() - } - - #[getter] - fn replace(&self) -> Option { - self.replace.as_ref().map(Event::shallow_copy) - } - - #[getter] - fn thread(&self) -> Option { - self.thread.clone() - } - /// Whether there are any aggregations to bundle. /// /// Matches the Python `bool(self.references or self.replace or self.thread)`: