[fractal/fractal-next] event: Fix replacing events
- From: Julian Sparber <jsparber src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [fractal/fractal-next] event: Fix replacing events
- Date: Tue, 14 Dec 2021 12:18:45 +0000 (UTC)
commit 3732138a7884cde438cde3a5b66d1908a2c453a9
Author: Kévin Commaille <zecakeh tedomum fr>
Date: Tue Dec 14 11:13:22 2021 +0100
event: Fix replacing events
Only store replacing events instead of all related events to avoid useless notifications.
Ignore redacted events, and forward replacing events' changes.
.../content/room_history/message_row/mod.rs | 69 +---------
src/session/room/event.rs | 144 ++++++++++++++++++---
src/session/room/timeline.rs | 131 +++++++++++++------
3 files changed, 225 insertions(+), 119 deletions(-)
---
diff --git a/src/session/content/room_history/message_row/mod.rs
b/src/session/content/room_history/message_row/mod.rs
index ad7a4a91..0f0ff813 100644
--- a/src/session/content/room_history/message_row/mod.rs
+++ b/src/session/content/room_history/message_row/mod.rs
@@ -11,8 +11,7 @@ use gtk::{
use log::warn;
use matrix_sdk::ruma::events::{
room::message::{MessageType, Relation},
- room::redaction::RoomRedactionEventContent,
- AnyMessageEventContent, AnySyncMessageEvent, AnySyncRoomEvent,
+ AnyMessageEventContent,
};
use self::{file::MessageFile, media::MessageMedia, text::MessageText};
@@ -38,7 +37,7 @@ mod imp {
pub timestamp: TemplateChild<gtk::Label>,
#[template_child]
pub content: TemplateChild<adw::Bin>,
- pub relates_to_changed_handler: RefCell<Option<SignalHandlerId>>,
+ pub content_changed_handler: RefCell<Option<SignalHandlerId>>,
pub bindings: RefCell<Vec<glib::Binding>>,
pub event: RefCell<Option<Event>>,
}
@@ -140,8 +139,8 @@ impl MessageRow {
let priv_ = imp::MessageRow::from_instance(self);
// Remove signals and bindings from the previous event
if let Some(event) = priv_.event.take() {
- if let Some(relates_to_changed_handler) = priv_.relates_to_changed_handler.take() {
- event.disconnect(relates_to_changed_handler);
+ if let Some(content_changed_handler) = priv_.content_changed_handler.take() {
+ event.disconnect(content_changed_handler);
}
while let Some(binding) = priv_.bindings.borrow_mut().pop() {
@@ -177,8 +176,8 @@ impl MessageRow {
]);
priv_
- .relates_to_changed_handler
- .replace(Some(event.connect_relates_to_changed(
+ .content_changed_handler
+ .replace(Some(event.connect_content_changed(
clone!(@weak self as obj => move |event| {
obj.update_content(event);
}),
@@ -187,63 +186,9 @@ impl MessageRow {
priv_.event.replace(Some(event));
}
- fn find_last_event(&self, event: &Event) -> Event {
- if let Some(replacement_event) = event.relates_to().iter().rev().find(|event| {
- let matrix_event = event.matrix_event();
- match matrix_event {
- Some(AnySyncRoomEvent::Message(AnySyncMessageEvent::RoomMessage(message))) => {
- message
- .content
- .relates_to
- .filter(|relation| matches!(relation, Relation::Replacement(_)))
- .is_some()
- }
- Some(AnySyncRoomEvent::Message(AnySyncMessageEvent::RoomRedaction(_))) => true,
- _ => false,
- }
- }) {
- if !replacement_event.relates_to().is_empty() {
- self.find_last_event(replacement_event)
- } else {
- replacement_event.clone()
- }
- } else {
- event.clone()
- }
- }
- /// Find the content we need to display
- fn find_content(&self, event: &Event) -> Option<AnyMessageEventContent> {
- match self.find_last_event(event).matrix_event() {
- Some(AnySyncRoomEvent::Message(message)) => Some(message.content()),
- Some(AnySyncRoomEvent::RedactedMessage(message)) => {
- if let Some(ref redaction_event) = message.unsigned().redacted_because {
- Some(AnyMessageEventContent::RoomRedaction(
- redaction_event.content.clone(),
- ))
- } else {
- Some(AnyMessageEventContent::RoomRedaction(
- RoomRedactionEventContent::new(),
- ))
- }
- }
- Some(AnySyncRoomEvent::RedactedState(state)) => {
- if let Some(ref redaction_event) = state.unsigned().redacted_because {
- Some(AnyMessageEventContent::RoomRedaction(
- redaction_event.content.clone(),
- ))
- } else {
- Some(AnyMessageEventContent::RoomRedaction(
- RoomRedactionEventContent::new(),
- ))
- }
- }
- _ => None,
- }
- }
-
fn update_content(&self, event: &Event) {
let priv_ = imp::MessageRow::from_instance(self);
- let content = self.find_content(event);
+ let content = event.content();
// TODO: create widgets for all event types
// TODO: display reaction events from event.relates_to()
diff --git a/src/session/room/event.rs b/src/session/room/event.rs
index 64141075..a8719e6b 100644
--- a/src/session/room/event.rs
+++ b/src/session/room/event.rs
@@ -1,12 +1,13 @@
-use gtk::{glib, glib::DateTime, prelude::*, subclass::prelude::*};
+use gtk::{glib, glib::clone, glib::DateTime, prelude::*, subclass::prelude::*};
use log::warn;
use matrix_sdk::{
deserialized_responses::SyncRoomEvent,
ruma::{
events::{
- room::message::MessageType, room::message::Relation, AnyMessageEventContent,
- AnyRedactedSyncMessageEvent, AnyRedactedSyncStateEvent, AnySyncMessageEvent,
- AnySyncRoomEvent, AnySyncStateEvent, Unsigned,
+ room::message::Relation,
+ room::{message::MessageType, redaction::RoomRedactionEventContent},
+ AnyMessageEventContent, AnyRedactedSyncMessageEvent, AnyRedactedSyncStateEvent,
+ AnySyncMessageEvent, AnySyncRoomEvent, AnySyncStateEvent, Unsigned,
},
identifiers::{EventId, UserId},
MilliSecondsSinceUnixEpoch,
@@ -26,6 +27,7 @@ mod imp {
use super::*;
use glib::object::WeakRef;
use glib::subclass::Signal;
+ use glib::SignalHandlerId;
use once_cell::{sync::Lazy, unsync::OnceCell};
use std::cell::{Cell, RefCell};
@@ -35,7 +37,9 @@ mod imp {
pub event: RefCell<Option<AnySyncRoomEvent>>,
/// The SDK event containing encryption information and the serialized event as `Raw`
pub pure_event: RefCell<Option<SyncRoomEvent>>,
- pub relates_to: RefCell<Vec<super::Event>>,
+ /// Events that replace this one, in the order they arrive.
+ pub replacing_events: RefCell<Vec<super::Event>>,
+ pub content_changed_handler: RefCell<Option<SignalHandlerId>>,
pub show_header: Cell<bool>,
pub room: OnceCell<WeakRef<Room>>,
}
@@ -50,7 +54,7 @@ mod imp {
impl ObjectImpl for Event {
fn signals() -> &'static [Signal] {
static SIGNALS: Lazy<Vec<Signal>> = Lazy::new(|| {
- vec![Signal::builder("relates-to-changed", &[], <()>::static_type().into()).build()]
+ vec![Signal::builder("content-changed", &[], <()>::static_type().into()).build()]
});
SIGNALS.as_ref()
}
@@ -478,22 +482,130 @@ impl Event {
}
}
- pub fn add_relates_to(&self, events: Vec<Event>) {
+ /// Whether this is a replacing `Event`.
+ ///
+ /// Replacing matrix events are:
+ ///
+ /// - `RoomRedaction`
+ /// - `RoomMessage` with `Relation::Replacement`
+ pub fn is_replacing_event(&self) -> bool {
+ match self.matrix_event() {
+ Some(AnySyncRoomEvent::Message(AnySyncMessageEvent::RoomMessage(message))) => {
+ matches!(message.content.relates_to, Some(Relation::Replacement(_)))
+ }
+ Some(AnySyncRoomEvent::Message(AnySyncMessageEvent::RoomRedaction(_))) => true,
+ _ => false,
+ }
+ }
+
+ pub fn prepend_replacing_events(&self, events: Vec<Event>) {
let priv_ = imp::Event::from_instance(self);
- priv_.relates_to.borrow_mut().extend(events);
- self.emit_by_name("relates-to-changed", &[]).unwrap();
+ priv_.replacing_events.borrow_mut().splice(..0, events);
}
- pub fn relates_to(&self) -> Vec<Event> {
+ pub fn append_replacing_events(&self, events: Vec<Event>) {
let priv_ = imp::Event::from_instance(self);
- priv_.relates_to.borrow().clone()
+ let old_replacement = self.replacement();
+
+ priv_.replacing_events.borrow_mut().extend(events);
+
+ let new_replacement = self.replacement();
+
+ // Update the signal handler to the new replacement
+ if new_replacement != old_replacement {
+ if let Some(replacement) = old_replacement {
+ if let Some(content_changed_handler) = priv_.content_changed_handler.take() {
+ replacement.disconnect(content_changed_handler);
+ }
+ }
+
+ // If the replacing event's content changed, this content changed too.
+ if let Some(replacement) = new_replacement {
+ priv_
+ .content_changed_handler
+ .replace(Some(replacement.connect_content_changed(
+ clone!(@weak self as obj => move |_| {
+ obj.emit_by_name("content-changed", &[]).unwrap();
+ }),
+ )));
+ }
+
+ self.emit_by_name("content-changed", &[]).unwrap();
+ }
}
- pub fn connect_relates_to_changed<F: Fn(&Self) + 'static>(
- &self,
- f: F,
- ) -> glib::SignalHandlerId {
- self.connect_local("relates-to-changed", true, move |values| {
+ pub fn replacing_events(&self) -> Vec<Event> {
+ let priv_ = imp::Event::from_instance(self);
+ priv_.replacing_events.borrow().clone()
+ }
+
+ /// The `Event` that replaces this one, if any.
+ ///
+ /// If this matrix event has been redacted or replaced, returns the corresponding `Event`,
+ /// otherwise returns `None`.
+ pub fn replacement(&self) -> Option<Event> {
+ self.replacing_events()
+ .iter()
+ .rev()
+ .find(|event| event.is_replacing_event() && !event.redacted())
+ .map(|event| event.clone())
+ }
+
+ /// Whether this matrix event has been redacted.
+ pub fn redacted(&self) -> bool {
+ self.replacement()
+ .filter(|event| {
+ matches!(
+ event.matrix_event(),
+ Some(AnySyncRoomEvent::Message(
+ AnySyncMessageEvent::RoomRedaction(_)
+ ))
+ )
+ })
+ .is_some()
+ }
+
+ /// The content of this matrix event.
+ pub fn original_content(&self) -> Option<AnyMessageEventContent> {
+ match self.matrix_event()? {
+ AnySyncRoomEvent::Message(message) => Some(message.content()),
+ AnySyncRoomEvent::RedactedMessage(message) => {
+ if let Some(ref redaction_event) = message.unsigned().redacted_because {
+ Some(AnyMessageEventContent::RoomRedaction(
+ redaction_event.content.clone(),
+ ))
+ } else {
+ Some(AnyMessageEventContent::RoomRedaction(
+ RoomRedactionEventContent::new(),
+ ))
+ }
+ }
+ AnySyncRoomEvent::RedactedState(state) => {
+ if let Some(ref redaction_event) = state.unsigned().redacted_because {
+ Some(AnyMessageEventContent::RoomRedaction(
+ redaction_event.content.clone(),
+ ))
+ } else {
+ Some(AnyMessageEventContent::RoomRedaction(
+ RoomRedactionEventContent::new(),
+ ))
+ }
+ }
+ _ => None,
+ }
+ }
+
+ /// The content to display for this `Event`.
+ ///
+ /// If this matrix event has been replaced, returns the replacing `Event`'s content.
+ pub fn content(&self) -> Option<AnyMessageEventContent> {
+ self.replacement()
+ .and_then(|replacement| replacement.content())
+ .or(self.original_content())
+ }
+
+ pub fn connect_content_changed<F: Fn(&Self) + 'static>(&self, f: F) -> glib::SignalHandlerId {
+ self.connect_local("content-changed", true, move |values| {
let obj = values[0].get::<Self>().unwrap();
f(&obj);
diff --git a/src/session/room/timeline.rs b/src/session/room/timeline.rs
index d6feec72..288bbb9b 100644
--- a/src/session/room/timeline.rs
+++ b/src/session/room/timeline.rs
@@ -1,3 +1,5 @@
+use std::collections::HashMap;
+
use gtk::{gio, glib, glib::clone, prelude::*, subclass::prelude::*};
use log::error;
use matrix_sdk::{
@@ -17,7 +19,7 @@ mod imp {
use glib::object::WeakRef;
use once_cell::{sync::Lazy, unsync::OnceCell};
use std::cell::{Cell, RefCell};
- use std::collections::{HashMap, VecDeque};
+ use std::collections::VecDeque;
#[derive(Debug, Default)]
pub struct Timeline {
@@ -252,7 +254,7 @@ impl Timeline {
}
}
- // Update relates_to
+ // Add relations to event
{
let list = priv_.list.borrow();
let mut relates_to_events = priv_.relates_to_events.borrow_mut();
@@ -261,30 +263,22 @@ impl Timeline {
.range(position as usize..(position + added) as usize)
.filter_map(|item| item.event())
{
- if let Some(relates_to_event_id) = event.related_matrix_event() {
- if let Some(relates_to_event) = self.event_by_id(&relates_to_event_id) {
- // FIXME: group events and set them all at once, to reduce the emission of notify
- relates_to_event.add_relates_to(vec![event.to_owned()]);
+ if let Some(relates_to) = relates_to_events.remove(&event.matrix_event_id()) {
+ let replacing_events = relates_to
+ .into_iter()
+ .map(|event_id| {
+ self.event_by_id(&event_id)
+ .expect("Previously known event has disappeared")
+ })
+ .filter(|related_event| related_event.is_replacing_event())
+ .collect();
+
+ if position != 0 || event.replacing_events().is_empty() {
+ event.append_replacing_events(replacing_events);
} else {
- // Store the new event if the `related_to` event isn't known, we will update the
`relates_to` once
- // the `related_to` event is is added to the list
- let relates_to_event =
- relates_to_events.entry(relates_to_event_id).or_default();
- relates_to_event.push(event.matrix_event_id().to_owned());
+ event.prepend_replacing_events(replacing_events);
}
}
-
- if let Some(relates_to) = relates_to_events.remove(&event.matrix_event_id()) {
- event.add_relates_to(
- relates_to
- .into_iter()
- .map(|event_id| {
- self.event_by_id(&event_id)
- .expect("Previously known event has disappeared")
- })
- .collect(),
- );
- }
}
}
@@ -294,33 +288,83 @@ impl Timeline {
.items_changed(position, removed, added);
}
- fn add_hidden_event(&self, event: Event) {
+ fn add_hidden_events(&self, events: Vec<Event>, at_front: bool) {
let priv_ = imp::Timeline::from_instance(self);
let mut relates_to_events = priv_.relates_to_events.borrow_mut();
- if let Some(relates_to_event_id) = event.related_matrix_event() {
- if let Some(relates_to_event) = self.event_by_id(&relates_to_event_id) {
- // FIXME: group events and set them all at once, to reduce the emission of notify
- relates_to_event.add_relates_to(vec![event.to_owned()]);
- } else {
- // Store the new event if the `related_to` event isn't known, we will update the
`relates_to` once
- // the `related_to` event is is added to the list
- let relates_to_event = relates_to_events.entry(relates_to_event_id).or_default();
- relates_to_event.push(event.matrix_event_id());
+ // Group events by related event
+ let mut new_relations: HashMap<EventId, Vec<Event>> = HashMap::new();
+ for event in events {
+ if let Some(relates_to) = relates_to_events.remove(&event.matrix_event_id()) {
+ let replacing_events = relates_to
+ .into_iter()
+ .map(|event_id| {
+ self.event_by_id(&event_id)
+ .expect("Previously known event has disappeared")
+ })
+ .filter(|related_event| related_event.is_replacing_event())
+ .collect();
+
+ if !at_front || event.replacing_events().is_empty() {
+ event.append_replacing_events(replacing_events);
+ } else {
+ event.prepend_replacing_events(replacing_events);
+ }
+ }
+
+ if let Some(relates_to_event) = event.related_matrix_event() {
+ let relations = new_relations.entry(relates_to_event).or_default();
+ relations.push(event);
}
}
- if let Some(relates_to) = relates_to_events.remove(&event.matrix_event_id()) {
- event.add_relates_to(
- relates_to
+ // Handle new relations
+ for (relates_to_event_id, relations) in new_relations {
+ if let Some(relates_to_event) = self.event_by_id(&relates_to_event_id) {
+ // Get the relations in relates_to_event otherwise they will be added in
+ // in items_changed and they might not be added at the right place.
+ let mut all_replacing_events: Vec<Event> = relates_to_events
+ .remove(&relates_to_event.matrix_event_id())
+ .unwrap_or_default()
.into_iter()
.map(|event_id| {
self.event_by_id(&event_id)
.expect("Previously known event has disappeared")
})
- .collect(),
- );
+ .filter(|related_event| related_event.is_replacing_event())
+ .collect();
+ let new_replacing_events: Vec<Event> = relations
+ .into_iter()
+ .filter(|event| event.is_replacing_event())
+ .collect();
+
+ if at_front {
+ all_replacing_events.splice(..0, new_replacing_events);
+ } else {
+ all_replacing_events.extend(new_replacing_events);
+ }
+
+ if !at_front || relates_to_event.replacing_events().is_empty() {
+ relates_to_event.append_replacing_events(all_replacing_events);
+ } else {
+ relates_to_event.prepend_replacing_events(all_replacing_events);
+ }
+ } else {
+ // Store the new event if the `related_to` event isn't known, we will update the
`relates_to` once
+ // the `related_to` event is is added to the list
+ let relates_to_event = relates_to_events.entry(relates_to_event_id).or_default();
+ let replacing_events_ids: Vec<EventId> = relations
+ .iter()
+ .filter(|event| event.is_replacing_event())
+ .map(|event| event.matrix_event_id())
+ .collect();
+ if at_front {
+ relates_to_event.splice(..0, replacing_events_ids);
+ } else {
+ relates_to_event.extend(replacing_events_ids);
+ }
+ }
}
}
@@ -350,6 +394,7 @@ impl Timeline {
};
let mut pending_events = priv_.pending_events.borrow_mut();
+ let mut hidden_events: Vec<Event> = vec![];
for event in batch.into_iter() {
let event_id = event.matrix_event_id();
@@ -371,7 +416,7 @@ impl Timeline {
.borrow_mut()
.insert(event_id.to_owned(), event.clone());
if event.is_hidden_event() {
- self.add_hidden_event(event);
+ hidden_events.push(event);
added -= 1;
} else {
priv_.list.borrow_mut().push_back(Item::for_event(event));
@@ -379,6 +424,8 @@ impl Timeline {
}
}
+ self.add_hidden_events(hidden_events, false);
+
index
};
@@ -404,7 +451,7 @@ impl Timeline {
let index = list.len();
if event.is_hidden_event() {
- self.add_hidden_event(event);
+ self.add_hidden_events(vec![event], false);
None
} else {
list.push_back(Item::for_event(event));
@@ -436,6 +483,7 @@ impl Timeline {
.replace(batch.last().as_ref().map(|event| event.matrix_event_id()));
{
+ let mut hidden_events: Vec<Event> = vec![];
// Extend the size of the list so that rust doesn't need to reallocate memory multiple times
priv_.list.borrow_mut().reserve(added);
@@ -446,12 +494,13 @@ impl Timeline {
.insert(event.matrix_event_id(), event.clone());
if event.is_hidden_event() {
- self.add_hidden_event(event);
+ hidden_events.push(event);
added -= 1;
} else {
priv_.list.borrow_mut().push_front(Item::for_event(event));
}
}
+ self.add_hidden_events(hidden_events, true);
}
self.items_changed(0, 0, added as u32);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
