diff --git a/crates/matrix-sdk-search/README.md b/crates/matrix-sdk-search/README.md index bcecdcbec14..183214ea470 100644 --- a/crates/matrix-sdk-search/README.md +++ b/crates/matrix-sdk-search/README.md @@ -34,12 +34,11 @@ The search crate accepts index operations through ```rust use matrix_sdk_search::index::{ - RoomIndex, RoomIndexOperation, + IndexableEvent, RoomIndex, RoomIndexOperation, builder::RoomIndexBuilder }; -use ruma::events::room::message::OriginalSyncRoomMessageEvent; -async fn add_event(index: &mut RoomIndex, event: OriginalSyncRoomMessageEvent) { +async fn add_event(index: &mut RoomIndex, event: IndexableEvent) { index.execute(RoomIndexOperation::Add(event)); } ``` diff --git a/crates/matrix-sdk-search/changelog.d/6710.changed.md b/crates/matrix-sdk-search/changelog.d/6710.changed.md new file mode 100644 index 00000000000..8365938a1d9 --- /dev/null +++ b/crates/matrix-sdk-search/changelog.d/6710.changed.md @@ -0,0 +1 @@ +Introduce an `IndexableEvent` type and decouple message parsing from search indexing. \ No newline at end of file diff --git a/crates/matrix-sdk-search/src/index/mod.rs b/crates/matrix-sdk-search/src/index/mod.rs index 5f4dab6e7d7..f1c157a4f70 100644 --- a/crates/matrix-sdk-search/src/index/mod.rs +++ b/crates/matrix-sdk-search/src/index/mod.rs @@ -17,9 +17,7 @@ pub mod builder; use std::{collections::HashSet, fmt}; -use ruma::{ - EventId, OwnedEventId, OwnedRoomId, RoomId, events::room::message::OriginalSyncRoomMessageEvent, -}; +use ruma::{EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedRoomId, OwnedUserId, RoomId}; use tantivy::{ Index, IndexReader, TantivyDocument, collector::TopDocs, directory::error::OpenDirectoryError, query::QueryParser, schema::Value, @@ -33,18 +31,37 @@ use crate::{ writer::SearchIndexWriter, }; +/// The subset of an event's data required to index it and later retrieve it. +/// +/// Produced by the matrix-sdk layer, which knows how to extract searchable text +/// from each event type. This crate stays agnostic to Matrix event content. +#[derive(Debug, Clone)] +pub struct IndexableEvent { + /// The event's own id (primary key). + pub event_id: OwnedEventId, + /// The id used as the deletion key: the original event id for edits, + /// otherwise the event's own id. + pub original_event_id: OwnedEventId, + /// The sender of the event. + pub sender: OwnedUserId, + /// The origin server timestamp of the event. + pub timestamp: MilliSecondsSinceUnixEpoch, + /// The text to index for this event. + pub body: String, +} + /// A struct to represent the operations on a [`RoomIndex`] #[derive(Debug, Clone)] pub enum RoomIndexOperation { /// Add this event to the index. - Add(OriginalSyncRoomMessageEvent), + Add(IndexableEvent), /// Remove all documents in the index where /// `MatrixSearchIndexSchema::deletion_key()` matches this event id. Remove(OwnedEventId), /// Replace all documents in the index where /// `MatrixSearchIndexSchema::deletion_key()` matches this event id with /// the new event. - Edit(OwnedEventId, OriginalSyncRoomMessageEvent), + Edit(OwnedEventId, IndexableEvent), /// Do nothing. Noop, } @@ -185,7 +202,7 @@ impl RoomIndex { fn add( &mut self, writer: &mut SearchIndexWriter, - event: OriginalSyncRoomMessageEvent, + event: IndexableEvent, ) -> Result<(), IndexError> { if !self.contains(&event.event_id) { writer.add(self.schema.make_doc(event.clone())?)?; @@ -342,16 +359,38 @@ mod tests { EventId, event_id, events::{ AnySyncMessageLikeEvent, - room::message::{OriginalSyncRoomMessageEvent, RoomMessageEventContentWithoutRelation}, + room::message::{ + MessageType, OriginalSyncRoomMessageEvent, Relation, + RoomMessageEventContentWithoutRelation, + }, }, room_id, user_id, }; use crate::{ error::IndexError, - index::{RoomIndex, RoomIndexOperation, builder::RoomIndexBuilder}, + index::{IndexableEvent, RoomIndex, RoomIndexOperation, builder::RoomIndexBuilder}, }; + /// Build an [`IndexableEvent`] from a text room message (tests only handle + /// text). + fn to_indexable(event: &OriginalSyncRoomMessageEvent) -> IndexableEvent { + let MessageType::Text(content) = &event.content.msgtype else { + panic!("test helper only supports text messages") + }; + let original_event_id = match &event.content.relates_to { + Some(Relation::Replacement(replacement)) => replacement.event_id.clone(), + _ => event.event_id.clone(), + }; + IndexableEvent { + event_id: event.event_id.clone(), + original_event_id, + sender: event.sender.clone(), + timestamp: event.origin_server_ts, + body: content.body.clone(), + } + } + /// Helper function to add a regular message to the index /// /// # Panic @@ -365,7 +404,7 @@ mod tests { && let Some(ev) = ev.as_original() && ev.content.relates_to.is_none() { - return index.execute(RoomIndexOperation::Add(ev.clone())); + return index.execute(RoomIndexOperation::Add(to_indexable(ev))); } panic!("Event was not a relationless OriginalSyncRoomMessageEvent.") } @@ -383,7 +422,7 @@ mod tests { event_id: &EventId, new: OriginalSyncRoomMessageEvent, ) -> Result<(), IndexError> { - index.execute(RoomIndexOperation::Edit(event_id.to_owned(), new)) + index.execute(RoomIndexOperation::Edit(event_id.to_owned(), to_indexable(&new))) } #[test] diff --git a/crates/matrix-sdk-search/src/schema.rs b/crates/matrix-sdk-search/src/schema.rs index d107f52d54b..251d4d06a4e 100644 --- a/crates/matrix-sdk-search/src/schema.rs +++ b/crates/matrix-sdk-search/src/schema.rs @@ -12,13 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -use ruma::events::room::message::{MessageType, OriginalSyncRoomMessageEvent, Relation}; use tantivy::{ DateTime, TantivyDocument, doc, schema::{DateOptions, DateTimePrecision, Field, INDEXED, STORED, STRING, Schema, TEXT}, }; -use crate::error::{IndexError, IndexSchemaError}; +use crate::{ + error::{IndexError, IndexSchemaError}, + index::IndexableEvent, +}; pub(crate) trait MatrixSearchIndexSchema { fn new() -> Self; @@ -27,7 +29,7 @@ pub(crate) trait MatrixSearchIndexSchema { fn deletion_key(&self) -> Field; fn get_field_name(&self, field: Field) -> &str; fn as_tantivy_schema(&self) -> Schema; - fn make_doc(&self, event: OriginalSyncRoomMessageEvent) -> Result; + fn make_doc(&self, event: IndexableEvent) -> Result; } #[derive(Debug, Clone)] @@ -92,29 +94,17 @@ impl MatrixSearchIndexSchema for RoomMessageSchema { self.inner.clone() } - /// Given an [`OriginalSyncRoomMessageEvent`] return a - /// [`TantivyDocument`]. - fn make_doc(&self, event: OriginalSyncRoomMessageEvent) -> Result { - let body = match &event.content.msgtype { - MessageType::Text(content) => Ok(content.body.clone()), - _ => Err(IndexError::MessageTypeNotSupported), - }?; - - let mut document = doc!( + /// Given an [`IndexableEvent`] return a [`TantivyDocument`]. + fn make_doc(&self, event: IndexableEvent) -> Result { + let document = doc!( self.event_id_field => event.event_id.to_string(), - self.body_field => body, + self.body_field => event.body, self.date_field => - DateTime::from_timestamp_millis( - event.origin_server_ts.get().into()), + DateTime::from_timestamp_millis(event.timestamp.get().into()), self.sender_field => event.sender.to_string(), + self.original_event_id_field => event.original_event_id.to_string(), ); - if let Some(Relation::Replacement(replacement_data)) = &event.content.relates_to { - document.add_text(self.original_event_id_field, replacement_data.event_id.clone()); - } else { - document.add_text(self.original_event_id_field, event.event_id); - } - Ok(document) } } diff --git a/crates/matrix-sdk/changelog.d/6710.changed.md b/crates/matrix-sdk/changelog.d/6710.changed.md new file mode 100644 index 00000000000..ba9e07d00d2 --- /dev/null +++ b/crates/matrix-sdk/changelog.d/6710.changed.md @@ -0,0 +1 @@ +Index media, stickers, polls and more message types. Move message parsing responsibility outside the search crate and into the main SDK one. \ No newline at end of file diff --git a/crates/matrix-sdk/src/search_index/mod.rs b/crates/matrix-sdk/src/search_index/mod.rs index d11a1c5f077..410823cbdde 100644 --- a/crates/matrix-sdk/src/search_index/mod.rs +++ b/crates/matrix-sdk/src/search_index/mod.rs @@ -22,16 +22,21 @@ use futures_util::future::join_all; use matrix_sdk_base::deserialized_responses::TimelineEvent; use matrix_sdk_search::{ error::IndexError, - index::{RoomIndex, RoomIndexOperation, builder::RoomIndexBuilder}, + index::{IndexableEvent, RoomIndex, RoomIndexOperation, builder::RoomIndexBuilder}, }; use ruma::{ EventId, OwnedEventId, OwnedRoomId, RoomId, events::{ AnySyncMessageLikeEvent, AnySyncTimelineEvent, + poll::{ + start::SyncPollStartEvent, + unstable_start::{SyncUnstablePollStartEvent, UnstablePollStartEventContent}, + }, room::{ - message::{OriginalSyncRoomMessageEvent, Relation, SyncRoomMessageEvent}, + message::{MessageType, OriginalSyncRoomMessageEvent, Relation, SyncRoomMessageEvent}, redaction::SyncRoomRedactionEvent, }, + sticker::SyncStickerEvent, }, room_version_rules::RedactionRules, }; @@ -233,6 +238,48 @@ async fn get_most_recent_edit( } } +/// Indexable text for a media message: its filename plus any caption. +fn media_body(filename: &str, caption: Option<&str>) -> String { + match caption { + Some(caption) => format!("{filename} {caption}"), + None => filename.to_owned(), + } +} + +/// Extract the searchable text from a room message, or `None` if its type +/// carries no text to index. +fn room_message_body(msgtype: &MessageType) -> Option { + match msgtype { + MessageType::Text(content) => Some(content.body.clone()), + MessageType::Emote(content) => Some(content.body.clone()), + MessageType::Notice(content) => Some(content.body.clone()), + MessageType::ServerNotice(content) => Some(content.body.clone()), + MessageType::Location(content) => Some(content.body.clone()), + MessageType::Image(content) => Some(media_body(content.filename(), content.caption())), + MessageType::Video(content) => Some(media_body(content.filename(), content.caption())), + MessageType::Audio(content) => Some(media_body(content.filename(), content.caption())), + MessageType::File(content) => Some(media_body(content.filename(), content.caption())), + _ => None, + } +} + +/// Build an [`IndexableEvent`] from a room message, or `None` if its type +/// carries no searchable text. +fn indexable_from_room_message(event: &OriginalSyncRoomMessageEvent) -> Option { + let body = room_message_body(&event.content.msgtype)?; + let original_event_id = match &event.content.relates_to { + Some(Relation::Replacement(replacement)) => replacement.event_id.clone(), + _ => event.event_id.clone(), + }; + Some(IndexableEvent { + event_id: event.event_id.clone(), + original_event_id, + sender: event.sender.clone(), + timestamp: event.origin_server_ts, + body, + }) +} + /// If the given [`OriginalSyncRoomMessageEvent`] is an edit we make an /// [`RoomIndexOperation::Edit`] with the new most recent version of the /// original. @@ -242,7 +289,14 @@ async fn handle_possible_edit( ) -> Option { if let Some(Relation::Replacement(replacement_data)) = &event.content.relates_to { if let Some(recent) = get_most_recent_edit(cache, &replacement_data.event_id).await { - return Some(RoomIndexOperation::Edit(replacement_data.event_id.clone(), recent)); + return Some( + indexable_from_room_message(&recent).map_or( + RoomIndexOperation::Noop, + |indexable| { + RoomIndexOperation::Edit(replacement_data.event_id.clone(), indexable) + }, + ), + ); } else { return Some(RoomIndexOperation::Noop); } @@ -262,30 +316,107 @@ async fn handle_room_message( &event.event_id, ) .await - .map(RoomIndexOperation::Add)); + .and_then(|recent| indexable_from_room_message(&recent).map(RoomIndexOperation::Add))); } None } -/// Return a [`RoomIndexOperation::Edit`] or [`RoomIndexOperation::Remove`] -/// depending on the message. +/// Return a [`RoomIndexOperation`] removing a redacted event from the index, or +/// re-adding the most recent remaining version if an edit was redacted. async fn handle_room_redaction( event: SyncRoomRedactionEvent, cache: &RoomEventCache, rules: &RedactionRules, ) -> Option { - if let Some(redacted_event_id) = event.redacts(rules) - && let Ok(Some(redacted_event)) = cache.find_event(redacted_event_id).await + let redacted_event_id = event.redacts(rules)?; + + // If the redacted event was a room message edit, re-add the most recent + // remaining version instead of just removing it. + if let Ok(Some(redacted_event)) = cache.find_event(redacted_event_id).await && let Ok(AnySyncTimelineEvent::MessageLike(AnySyncMessageLikeEvent::RoomMessage( redacted_event, ))) = redacted_event.raw().deserialize() && let Some(redacted_event) = redacted_event.as_original() + && let Some(operation) = handle_possible_edit(redacted_event, cache).await { - return handle_possible_edit(redacted_event, cache) - .await - .or(Some(RoomIndexOperation::Remove(redacted_event.event_id.clone()))); + return Some(operation); } - None + + // Otherwise remove the redacted event from the index. This covers plain + // messages, stickers and polls. + Some(RoomIndexOperation::Remove(redacted_event_id.to_owned())) +} + +/// Return a [`RoomIndexOperation::Add`] indexing a sticker's descriptive text. +fn handle_sticker(event: SyncStickerEvent) -> Option { + let event = event.as_original()?; + Some(RoomIndexOperation::Add(IndexableEvent { + event_id: event.event_id.clone(), + original_event_id: event.event_id.clone(), + sender: event.sender.clone(), + timestamp: event.origin_server_ts, + body: event.content.body.clone(), + })) +} + +/// Return a [`RoomIndexOperation::Add`] indexing an unstable poll's question +/// and answers. +/// +/// ponytail: only indexes the initial `New` poll — edits (`Replacement`) and +/// poll ends are ignored. Add edit handling if editing a poll needs to update +/// search results. +fn handle_unstable_poll_start(event: SyncUnstablePollStartEvent) -> Option { + let event = event.as_original()?; + + let UnstablePollStartEventContent::New(content) = &event.content else { + return None; + }; + + let block = &content.poll_start; + let mut body = block.question.text.clone(); + for answer in block.answers.iter() { + body.push(' '); + body.push_str(&answer.text); + } + + Some(RoomIndexOperation::Add(IndexableEvent { + event_id: event.event_id.clone(), + original_event_id: event.event_id.clone(), + sender: event.sender.clone(), + timestamp: event.origin_server_ts, + body, + })) +} + +/// Return a [`RoomIndexOperation::Add`] indexing a stable poll's question and +/// answers. +/// +/// ponytail: like [`handle_unstable_poll_start`], edits and poll ends are +/// ignored — only the initial poll is indexed. +fn handle_poll_start(event: SyncPollStartEvent) -> Option { + let event = event.as_original()?; + + // Skip poll edits, matching the unstable poll handling. + if let Some(Relation::Replacement(_)) = &event.content.relates_to { + return None; + } + + let block = &event.content.poll; + let mut body = block.question.text.find_plain()?.to_owned(); + for answer in block.answers.iter() { + if let Some(text) = answer.text.find_plain() { + body.push(' '); + body.push_str(text); + } + } + + Some(RoomIndexOperation::Add(IndexableEvent { + event_id: event.event_id.clone(), + original_event_id: event.event_id.clone(), + sender: event.sender.clone(), + timestamp: event.origin_server_ts, + body, + })) } /// Prepare a [`TimelineEvent`] into a [`RoomIndexOperation`] for search @@ -310,6 +441,11 @@ async fn parse_timeline_event( AnySyncMessageLikeEvent::RoomRedaction(event) => { handle_room_redaction(event, cache, redaction_rules).await } + AnySyncMessageLikeEvent::Sticker(event) => handle_sticker(event), + AnySyncMessageLikeEvent::PollStart(event) => handle_poll_start(event), + AnySyncMessageLikeEvent::UnstablePollStart(event) => { + handle_unstable_poll_start(event) + } _ => None, }, AnySyncTimelineEvent::State(_) => None, @@ -363,6 +499,151 @@ mod tests { assert_eq!(response[0].1, event_id, "event id doesn't match: {response:?}"); } + #[cfg(feature = "experimental-search")] + #[async_test] + async fn test_sync_media_message_is_indexed() { + use ruma::owned_mxc_uri; + + let mock_server = MatrixMockServer::new().await; + let client = mock_server.client_builder().build().await; + + client.event_cache().subscribe().unwrap(); + + let room_id = room_id!("!room_id:localhost"); + let image_id = event_id!("$image_id:localhost"); + let file_id = event_id!("$file_id:localhost"); + let user_id = user_id!("@user_id:localhost"); + + let f = EventFactory::new(); + let room = mock_server + .sync_room( + &client, + JoinedRoomBuilder::new(room_id).add_timeline_bulk(vec![ + f.image("holiday_beach.jpg".to_owned(), owned_mxc_uri!("mxc://localhost/1")) + .caption(Some("sunset over the ocean".to_owned()), None) + .event_id(image_id) + .sender(user_id) + .into_raw_sync(), + f.image("quarterly_report.pdf".to_owned(), owned_mxc_uri!("mxc://localhost/2")) + .event_id(file_id) + .sender(user_id) + .into_raw_sync(), + ]), + ) + .await; + + // The caption is indexed. + let response = room.search("sunset", 5, None).await.unwrap(); + assert_eq!(response.len(), 1, "unexpected results for caption search: {response:?}"); + assert_eq!(response[0].1, image_id, "event id doesn't match: {response:?}"); + + // The filename is indexed. + let response = room.search("holiday_beach", 5, None).await.unwrap(); + assert_eq!(response.len(), 1, "unexpected results for filename search: {response:?}"); + assert_eq!(response[0].1, image_id, "event id doesn't match: {response:?}"); + + // A media message without a caption still indexes its filename. + let response = room.search("quarterly_report", 5, None).await.unwrap(); + assert_eq!(response.len(), 1, "unexpected results for filename search: {response:?}"); + assert_eq!(response[0].1, file_id, "event id doesn't match: {response:?}"); + } + + #[cfg(feature = "experimental-search")] + #[async_test] + async fn test_sync_sticker_and_poll_are_indexed() { + use ruma::{events::room::ImageInfo, owned_mxc_uri}; + + let mock_server = MatrixMockServer::new().await; + let client = mock_server.client_builder().build().await; + + client.event_cache().subscribe().unwrap(); + + let room_id = room_id!("!room_id:localhost"); + let sticker_id = event_id!("$sticker_id:localhost"); + let poll_id = event_id!("$poll_id:localhost"); + let user_id = user_id!("@user_id:localhost"); + + let f = EventFactory::new().room(room_id).sender(user_id); + let room = mock_server + .sync_room( + &client, + JoinedRoomBuilder::new(room_id).add_timeline_bulk(vec![ + f.sticker( + "a waving cat", + ImageInfo::new(), + owned_mxc_uri!("mxc://localhost/1"), + ) + .event_id(sticker_id) + .into_raw_sync(), + f.poll_start("fallback", "favourite cheese?", vec!["comté", "gruyère"]) + .event_id(poll_id) + .into_raw_sync(), + ]), + ) + .await; + + // The sticker's description is indexed. + let response = room.search("waving", 5, None).await.unwrap(); + assert_eq!(response.len(), 1, "unexpected results for sticker search: {response:?}"); + assert_eq!(response[0].1, sticker_id, "event id doesn't match: {response:?}"); + + // The poll question is indexed. + let response = room.search("cheese", 5, None).await.unwrap(); + assert_eq!(response.len(), 1, "unexpected results for poll question search: {response:?}"); + assert_eq!(response[0].1, poll_id, "event id doesn't match: {response:?}"); + + // The poll answers are indexed. + let response = room.search("gruyère", 5, None).await.unwrap(); + assert_eq!(response.len(), 1, "unexpected results for poll answer search: {response:?}"); + assert_eq!(response[0].1, poll_id, "event id doesn't match: {response:?}"); + } + + #[cfg(feature = "experimental-search")] + #[async_test] + async fn test_sync_stable_poll_is_indexed() { + use ruma::events::{ + message::TextContentBlock, + poll::start::{PollAnswer, PollAnswers, PollContentBlock, PollStartEventContent}, + }; + + let mock_server = MatrixMockServer::new().await; + let client = mock_server.client_builder().build().await; + + client.event_cache().subscribe().unwrap(); + + let room_id = room_id!("!room_id:localhost"); + let poll_id = event_id!("$stable_poll_id:localhost"); + let user_id = user_id!("@user_id:localhost"); + + let answers: PollAnswers = vec![ + PollAnswer::new("0".to_owned(), TextContentBlock::plain("comté")), + PollAnswer::new("1".to_owned(), TextContentBlock::plain("gruyère")), + ] + .try_into() + .unwrap(); + let poll = PollContentBlock::new(TextContentBlock::plain("favourite cheese?"), answers); + let content = PollStartEventContent::new(TextContentBlock::plain("fallback"), poll); + + let f = EventFactory::new().room(room_id).sender(user_id); + let room = mock_server + .sync_room( + &client, + JoinedRoomBuilder::new(room_id) + .add_timeline_bulk(vec![f.event(content).event_id(poll_id).into_raw_sync()]), + ) + .await; + + // The poll question is indexed. + let response = room.search("cheese", 5, None).await.unwrap(); + assert_eq!(response.len(), 1, "unexpected results for poll question search: {response:?}"); + assert_eq!(response[0].1, poll_id, "event id doesn't match: {response:?}"); + + // The poll answers are indexed. + let response = room.search("gruyère", 5, None).await.unwrap(); + assert_eq!(response.len(), 1, "unexpected results for poll answer search: {response:?}"); + assert_eq!(response[0].1, poll_id, "event id doesn't match: {response:?}"); + } + #[cfg(feature = "experimental-search")] #[async_test] async fn test_search_index_edit_ordering() {