From 6f648cf22a7d63d9c22358e6352801d4dcadfb9f Mon Sep 17 00:00:00 2001 From: mb Date: Thu, 25 Jul 2024 09:52:37 +0200 Subject: [PATCH] fix: Use internal IDs for messages (closes #89) --- bindings/prose-sdk-ffi/src/client.rs | 12 +- bindings/prose-sdk-ffi/src/prose_sdk_ffi.udl | 10 +- .../prose-sdk-ffi/src/types/client_event.rs | 8 +- bindings/prose-sdk-ffi/src/types/message.rs | 6 +- bindings/prose-sdk-ffi/src/uniffi_api.rs | 19 +- bindings/prose-sdk-js/src/delegate.rs | 10 +- bindings/prose-sdk-js/src/types/message.rs | 19 +- bindings/prose-sdk-js/src/types/room.rs | 6 +- .../src/app/deps/app_dependencies.rs | 4 +- .../prose-core-client/src/app/dtos/message.rs | 6 +- crates/prose-core-client/src/app/dtos/mod.rs | 2 +- .../event_handlers/messages_event_handler.rs | 233 ++++----- .../src/app/services/room.rs | 128 +++-- .../prose-core-client/src/client_builder.rs | 19 +- crates/prose-core-client/src/client_event.rs | 8 +- .../services/encryption_domain_service.rs | 4 +- .../impls/encryption_domain_service.rs | 6 +- .../src/domain/messaging/models/message.rs | 224 +++++--- .../src/domain/messaging/models/message_id.rs | 17 +- .../domain/messaging/models/message_like.rs | 58 +- .../domain/messaging/models/message_parser.rs | 20 +- .../src/domain/messaging/models/mod.rs | 4 +- .../messaging/models/send_message_request.rs | 6 +- .../messaging/repos/messages_repository.rs | 28 +- .../impls/message_archive_domain_service.rs | 52 +- .../messaging/services/message_id_provider.rs | 44 ++ .../src/domain/messaging/services/mod.rs | 2 + .../src/domain/rooms/models/room.rs | 11 + .../messaging/caching_message_repository.rs | 74 ++- .../src/infra/messaging/message_record.rs | 42 +- .../src/infra/platform_dependencies.rs | 19 +- .../src/infra/xmpp/util/message_ext.rs | 50 ++ .../src/test/message_builder.rs | 93 ++-- .../src/test/mock_app_dependencies.rs | 28 +- .../tests/message_parsing/message_parser.rs | 48 +- .../tests/messages_event_handler.rs | 184 +++++-- crates/prose-core-client/tests/room.rs | 65 ++- crates/prose-utils/src/id_string_macro.rs | 11 +- .../prose-core-client-cli/src/type_display.rs | 12 +- .../src/type_selection.rs | 12 +- tests/prose-core-integration-tests/src/lib.rs | 8 +- .../src/tests/client/catchup_unread.rs | 35 +- .../src/tests/client/helpers/test_client.rs | 51 +- .../src/tests/client/message_handling.rs | 495 ++++++++++++++++++ .../src/tests/client/message_styling.rs | 20 +- .../src/tests/client/mod.rs | 1 + .../src/tests/client/muc.rs | 177 +------ .../src/tests/client/muc_omemo.rs | 26 +- .../src/tests/client/omemo.rs | 58 +- .../src/tests/client/reactions.rs | 5 +- .../src/tests/messages_repository.rs | 106 +++- 51 files changed, 1739 insertions(+), 847 deletions(-) create mode 100644 crates/prose-core-client/src/domain/messaging/services/message_id_provider.rs create mode 100644 tests/prose-core-integration-tests/src/tests/client/message_handling.rs diff --git a/bindings/prose-sdk-ffi/src/client.rs b/bindings/prose-sdk-ffi/src/client.rs index 8f92d5e1..cfb07de2 100644 --- a/bindings/prose-sdk-ffi/src/client.rs +++ b/bindings/prose-sdk-ffi/src/client.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use parking_lot::{Mutex, RwLock}; use tracing::info; -use prose_core_client::dtos::{Availability, Emoji, MessageRemoteId, UserProfile}; +use prose_core_client::dtos::{Availability, Emoji, MessageId, UserProfile}; use prose_core_client::infra::encryption::{EncryptionKeysRepository, SessionRepository}; use prose_core_client::infra::general::OsRngProvider; use prose_core_client::{ @@ -139,7 +139,7 @@ impl Client { pub async fn load_latest_messages( &self, _from: JID, - _since: Option, + _since: Option, _load_from_server: bool, ) -> Result, ClientError> { todo!("Use Room API"); @@ -153,7 +153,7 @@ impl Client { pub async fn load_messages_with_ids( &self, _conversation: JID, - _ids: Vec, + _ids: Vec, ) -> Result, ClientError> { todo!("Use Room API"); // let messages = self @@ -172,7 +172,7 @@ impl Client { pub async fn update_message( &self, _conversation: JID, - _id: MessageRemoteId, + _id: MessageId, _body: String, ) -> Result<(), ClientError> { todo!("Use Room API"); @@ -185,7 +185,7 @@ impl Client { pub async fn toggle_reaction_to_message( &self, _conversation: JID, - _id: MessageRemoteId, + _id: MessageId, _emoji: Emoji, ) -> Result<(), ClientError> { todo!("Use Room API"); @@ -198,7 +198,7 @@ impl Client { pub async fn retract_message( &self, _conversation: JID, - _id: MessageRemoteId, + _id: MessageId, ) -> Result<(), ClientError> { todo!("Use Room API"); // self.client diff --git a/bindings/prose-sdk-ffi/src/prose_sdk_ffi.udl b/bindings/prose-sdk-ffi/src/prose_sdk_ffi.udl index 7f80d093..7560b858 100644 --- a/bindings/prose-sdk-ffi/src/prose_sdk_ffi.udl +++ b/bindings/prose-sdk-ffi/src/prose_sdk_ffi.udl @@ -7,9 +7,7 @@ typedef string PathBuf; [Custom] typedef string Url; [Custom] -typedef string MessageRemoteId; -[Custom] -typedef string MessageServerId; +typedef string MessageId; [Custom] typedef string Emoji; [Custom] @@ -55,9 +53,9 @@ interface ClientEvent { ConnectionStatusChanged(ConnectionEvent event); ContactChanged(JID jid); AvatarChanged(JID jid); - MessagesAppended(JID conversation, sequence message_ids); - MessagesUpdated(JID conversation, sequence message_ids); - MessagesDeleted(JID conversation, sequence message_ids); + MessagesAppended(JID conversation, sequence message_ids); + MessagesUpdated(JID conversation, sequence message_ids); + MessagesDeleted(JID conversation, sequence message_ids); }; [Enum] diff --git a/bindings/prose-sdk-ffi/src/types/client_event.rs b/bindings/prose-sdk-ffi/src/types/client_event.rs index a8e99811..3bc63b3f 100644 --- a/bindings/prose-sdk-ffi/src/types/client_event.rs +++ b/bindings/prose-sdk-ffi/src/types/client_event.rs @@ -4,7 +4,7 @@ // License: Mozilla Public License v2.0 (MPL v2.0) use crate::uniffi_types::JID; -use prose_core_client::dtos::MessageRemoteId; +use prose_core_client::dtos::MessageId; use prose_core_client::ConnectionEvent; pub enum ClientEvent { @@ -23,19 +23,19 @@ pub enum ClientEvent { /// One or many messages were either received or sent. MessagesAppended { conversation: JID, - message_ids: Vec, + message_ids: Vec, }, /// One or many messages were received that affected earlier messages (e.g. a reaction). MessagesUpdated { conversation: JID, - message_ids: Vec, + message_ids: Vec, }, /// A message was deleted. MessagesDeleted { conversation: JID, - message_ids: Vec, + message_ids: Vec, }, } diff --git a/bindings/prose-sdk-ffi/src/types/message.rs b/bindings/prose-sdk-ffi/src/types/message.rs index dc2f343d..4079a79d 100644 --- a/bindings/prose-sdk-ffi/src/types/message.rs +++ b/bindings/prose-sdk-ffi/src/types/message.rs @@ -5,7 +5,7 @@ use chrono::{DateTime as ChronoDateTime, Utc}; -use prose_core_client::dtos::{Emoji, Message as ProseMessage, MessageRemoteId, MessageServerId}; +use prose_core_client::dtos::{Emoji, Message as ProseMessage, MessageId}; use crate::types::JID; @@ -19,8 +19,7 @@ pub struct Reaction { #[derive(uniffi::Record)] pub struct Message { - pub id: Option, - pub stanza_id: Option, + pub id: MessageId, pub from: Option, pub body: String, pub timestamp: DateTime, @@ -34,7 +33,6 @@ impl From for Message { fn from(value: ProseMessage) -> Self { Message { id: value.id, - stanza_id: value.stanza_id, from: value.from.id.to_user_id().map(|id| id.into_inner().into()), body: todo!(), timestamp: value.timestamp, diff --git a/bindings/prose-sdk-ffi/src/uniffi_api.rs b/bindings/prose-sdk-ffi/src/uniffi_api.rs index 9cd9aed3..007896cd 100644 --- a/bindings/prose-sdk-ffi/src/uniffi_api.rs +++ b/bindings/prose-sdk-ffi/src/uniffi_api.rs @@ -10,7 +10,8 @@ pub use std::path::PathBuf; pub use jid::{BareJid, Error as JidParseError, FullJid}; pub use prose_core_client::dtos::{ - Address, Availability, Emoji, MessageRemoteId, MessageServerId, Url, UserProfile, UserStatus, + Address, Availability, Emoji, MessageId, MessageRemoteId, MessageServerId, Url, UserProfile, + UserStatus, }; pub use prose_core_client::ConnectionEvent; pub use prose_xmpp::ConnectionError; @@ -20,19 +21,7 @@ pub use crate::{ account_bookmarks_client::AccountBookmarksClient, client::*, logger::*, ClientError, }; -impl UniffiCustomTypeConverter for MessageRemoteId { - type Builtin = String; - - fn into_custom(val: Self::Builtin) -> uniffi::Result { - Ok(val.into()) - } - - fn from_custom(obj: Self) -> Self::Builtin { - obj.into_inner() - } -} - -impl UniffiCustomTypeConverter for MessageServerId { +impl UniffiCustomTypeConverter for MessageId { type Builtin = String; fn into_custom(val: Self::Builtin) -> uniffi::Result { @@ -102,7 +91,7 @@ pub mod uniffi_types { client::Client, types::{parse_jid, AccountBookmark, DateTime, Message, Reaction, JID}, Availability, ClientError, ConnectionError, Contact, Emoji, FullJid, JidParseError, - MessageRemoteId, MessageServerId, PathBuf, Url, UserProfile, + MessageId, PathBuf, Url, UserProfile, }; } diff --git a/bindings/prose-sdk-js/src/delegate.rs b/bindings/prose-sdk-js/src/delegate.rs index d37081a6..63f6d109 100644 --- a/bindings/prose-sdk-js/src/delegate.rs +++ b/bindings/prose-sdk-js/src/delegate.rs @@ -6,7 +6,7 @@ use tracing::warn; use wasm_bindgen::prelude::*; -use prose_core_client::dtos::MessageRemoteId; +use prose_core_client::dtos::{MessageId, MessageRemoteId}; use prose_core_client::{ClientDelegate, ClientEvent, ClientRoomEventType, ConnectionEvent}; use prose_xmpp::ConnectionError; @@ -223,6 +223,14 @@ impl JSValueConvertible for Vec { } } +impl JSValueConvertible for Vec { + fn into_js_array(self) -> Vec { + self.into_iter() + .map(|id| JsValue::from(id.into_inner())) + .collect() + } +} + impl ClientDelegate for Delegate { fn handle_event(&self, client: prose_core_client::Client, event: ClientEvent) { match self.handle_event_throwing(client, event) { diff --git a/bindings/prose-sdk-js/src/types/message.rs b/bindings/prose-sdk-js/src/types/message.rs index 07a8d3f7..587ecb13 100644 --- a/bindings/prose-sdk-js/src/types/message.rs +++ b/bindings/prose-sdk-js/src/types/message.rs @@ -41,8 +41,7 @@ pub struct Body { #[wasm_bindgen] pub struct Message { - id: Option, - stanza_id: Option, + id: String, from: MessageSender, body: Body, timestamp: js_sys::Date, @@ -96,8 +95,7 @@ impl From for Message { .collect_into_js_array(); Self { - id: value.id.map(|id| id.to_string()), - stanza_id: value.stanza_id.map(ArchiveID::from), + id: value.id.to_string(), from: value.from.into(), body: Body { raw: value.body.raw, @@ -128,18 +126,13 @@ impl From for Message { #[wasm_bindgen] impl Message { #[wasm_bindgen(getter)] - pub fn id(&self) -> Option { - self.id.as_ref().map(|id| id.to_string()) + pub fn id(&self) -> String { + self.id.to_string() } #[wasm_bindgen(setter)] - pub fn set_id(&mut self, id: Option) { - self.id = id.clone().map(Into::into) - } - - #[wasm_bindgen(getter, js_name = "archiveId")] - pub fn stanza_id(&self) -> Option { - self.stanza_id.clone() + pub fn set_id(&mut self, id: String) { + self.id = id.into() } #[wasm_bindgen(getter, js_name = "from")] diff --git a/bindings/prose-sdk-js/src/types/room.rs b/bindings/prose-sdk-js/src/types/room.rs index 5a254b15..f1138fc0 100644 --- a/bindings/prose-sdk-js/src/types/room.rs +++ b/bindings/prose-sdk-js/src/types/room.rs @@ -8,7 +8,7 @@ use tracing::debug; use wasm_bindgen::prelude::wasm_bindgen; use wasm_bindgen::{JsError, JsValue}; -use prose_core_client::dtos::{MessageRemoteId, RoomEnvelope, RoomState as SdkRoomState}; +use prose_core_client::dtos::{MessageId, RoomEnvelope, RoomState as SdkRoomState}; use prose_core_client::services::{ DirectMessage, Generic, Group, PrivateChannel, PublicChannel, Room as SdkRoom, }; @@ -337,9 +337,9 @@ macro_rules! base_room_impl { &self, message_ids: &StringArray, ) -> Result { - let message_ids: Vec = Vec::::try_from(message_ids)? + let message_ids: Vec = Vec::::try_from(message_ids)? .into_iter() - .map(|id| MessageRemoteId::from(id)) + .map(|id| MessageId::from(id)) .collect(); let messages = self diff --git a/crates/prose-core-client/src/app/deps/app_dependencies.rs b/crates/prose-core-client/src/app/deps/app_dependencies.rs index 02f9c122..571fc892 100644 --- a/crates/prose-core-client/src/app/deps/app_dependencies.rs +++ b/crates/prose-core-client/src/app/deps/app_dependencies.rs @@ -27,7 +27,7 @@ use crate::domain::general::services::RequestHandlingService; use crate::domain::messaging::repos::{ DraftsRepository, MessagesRepository, OfflineMessagesRepository, }; -use crate::domain::messaging::services::MessageArchiveDomainService; +use crate::domain::messaging::services::{MessageArchiveDomainService, MessageIdProvider}; use crate::domain::messaging::services::{ MessageArchiveService, MessageMigrationDomainService, MessagingService, }; @@ -68,6 +68,7 @@ pub type DynIDProvider = Arc; pub type DynLocalRoomSettingsRepository = Arc; pub type DynMessageArchiveDomainService = Arc; pub type DynMessageArchiveService = Arc; +pub type DynMessageIdProvider = Arc; pub type DynMessageMigrationDomainService = Arc; pub type DynMessagesRepository = Arc; pub type DynMessagingService = Arc; @@ -107,6 +108,7 @@ pub struct AppDependencies { pub drafts_repo: DynDraftsRepository, pub encryption_domain_service: DynEncryptionDomainService, pub id_provider: DynIDProvider, + pub message_id_provider: DynMessageIdProvider, pub local_room_settings_repo: DynLocalRoomSettingsRepository, pub message_archive_service: DynMessageArchiveService, pub messages_repo: DynMessagesRepository, diff --git a/crates/prose-core-client/src/app/dtos/message.rs b/crates/prose-core-client/src/app/dtos/message.rs index 79f86521..3e40a93d 100644 --- a/crates/prose-core-client/src/app/dtos/message.rs +++ b/crates/prose-core-client/src/app/dtos/message.rs @@ -5,13 +5,13 @@ use chrono::{DateTime, Utc}; +use crate::domain::messaging::models::MessageId; use crate::domain::shared::models::ParticipantId; -use crate::dtos::{Attachment, Avatar, Body, Emoji, Mention, MessageRemoteId, MessageServerId}; +use crate::dtos::{Attachment, Avatar, Body, Emoji, Mention}; #[derive(Debug, Clone, PartialEq)] pub struct Message { - pub id: Option, - pub stanza_id: Option, + pub id: MessageId, pub from: MessageSender, pub body: Body, pub timestamp: DateTime, diff --git a/crates/prose-core-client/src/app/dtos/mod.rs b/crates/prose-core-client/src/app/dtos/mod.rs index ee5caa20..4123912a 100644 --- a/crates/prose-core-client/src/app/dtos/mod.rs +++ b/crates/prose-core-client/src/app/dtos/mod.rs @@ -28,7 +28,7 @@ pub use crate::domain::{ general::models::SoftwareVersion, messaging::models::{ Attachment, AttachmentType, Body, Emoji, EncryptedPayload, EncryptionKey, Mention, - MessageRemoteId, MessageServerId, Thumbnail, + MessageId, MessageRemoteId, MessageServerId, Thumbnail, }, rooms::models::{Participant, PublicRoomInfo, RoomAffiliation, RoomState}, shared::models::{ diff --git a/crates/prose-core-client/src/app/event_handlers/messages_event_handler.rs b/crates/prose-core-client/src/app/event_handlers/messages_event_handler.rs index d26a138a..b0b6f2f3 100644 --- a/crates/prose-core-client/src/app/event_handlers/messages_event_handler.rs +++ b/crates/prose-core-client/src/app/event_handlers/messages_event_handler.rs @@ -3,8 +3,6 @@ // Copyright: 2023, Marc Bauer // License: Mozilla Public License v2.0 (MPL v2.0) -use std::ops::Deref; - use anyhow::Result; use async_trait::async_trait; use chrono::TimeDelta; @@ -18,13 +16,12 @@ use prose_xmpp::stanza::Message; use crate::app::deps::{ DynAppContext, DynClientEventDispatcher, DynConnectedRoomsReadOnlyRepository, - DynEncryptionDomainService, DynMessagesRepository, DynOfflineMessagesRepository, - DynSidebarDomainService, DynTimeProvider, + DynEncryptionDomainService, DynMessageIdProvider, DynMessagesRepository, + DynOfflineMessagesRepository, DynSidebarDomainService, DynTimeProvider, }; use crate::app::event_handlers::{MessageEvent, MessageEventType, ServerEvent, ServerEventHandler}; use crate::domain::messaging::models::{ - MessageLike, MessageLikeError, MessageLikeId, MessageLikePayload, MessageParser, - MessageTargetId, + MessageId, MessageLike, MessageLikeError, MessageLikePayload, MessageParser, MessageTargetId, }; use crate::domain::rooms::models::Room; use crate::domain::shared::models::{AccountId, ConnectionState, RoomId, UserEndpointId}; @@ -41,6 +38,8 @@ pub struct MessagesEventHandler { #[inject] encryption_domain_service: DynEncryptionDomainService, #[inject] + message_id_provider: DynMessageIdProvider, + #[inject] messages_repo: DynMessagesRepository, #[inject] sidebar_domain_service: DynSidebarDomainService, @@ -81,63 +80,42 @@ impl ServerEventHandler for MessagesEventHandler { } } -enum ReceivedMessage { - Message(Message), - Carbon(Forwarded), +impl MessageEventType { + fn message(&self) -> Option<&Message> { + match self { + MessageEventType::Received(message) => Some(message), + MessageEventType::Sent(message) => Some(message), + MessageEventType::Sync(Carbon::Received(carbon)) => carbon.stanza.as_deref(), + MessageEventType::Sync(Carbon::Sent(carbon)) => carbon.stanza.as_deref(), + } + } } -enum SentMessage { +enum MessageOrCarbon { Message(Message), Carbon(Forwarded), } -impl ReceivedMessage { - pub fn from(&self) -> Option { - let message = match &self { - ReceivedMessage::Message(message) => Some(message), - ReceivedMessage::Carbon(message) => message.stanza.as_ref().map(|m| m.deref()), - }; - - let Some(message) = message else { return None }; - - let Some(from) = message.from.clone() else { - return None; - }; - - if message.is_groupchat_message() { - let Ok(from) = from.try_into_full() else { - error!("Expected FullJid in received groupchat message"); - return None; - }; - UserEndpointId::Occupant(from.into()) - } else { - match from.try_into_full() { - Ok(full) => UserEndpointId::UserResource(full.into()), - Err(bare) => UserEndpointId::User(bare.into()), - } +impl MessageOrCarbon { + fn message(&self) -> Option<&Message> { + match self { + Self::Message(message) => Some(message), + Self::Carbon(carbon) => carbon.stanza.as_deref(), } - .into() } -} - -impl SentMessage { - pub fn room_id(&self) -> Option { - let message = match self { - SentMessage::Message(message) => Some(message), - SentMessage::Carbon(message) => message.stanza.as_ref().map(|m| m.deref()), - }; - let Some(message) = message else { return None }; + pub fn from(&self) -> Option { + self.message().and_then(|m| m.sender()) + } - let Some(to) = message.to.clone() else { - return None; - }; + pub fn room_id(&self) -> Option { + self.message().and_then(|m| m.room_id()) + } - match message.type_ { - MessageType::Groupchat => RoomId::Muc(to.into_bare().into()), - _ => RoomId::User(to.into_bare().into()), - } - .into() + pub fn remote_id(&self) -> Option { + self.message() + .and_then(|m| m.id.clone()) + .map(MessageRemoteId::from) } } @@ -145,6 +123,22 @@ impl MessagesEventHandler { async fn handle_message_event(&self, event: MessageEvent) -> Result<()> { let account = self.ctx.connected_account()?; + // Skip known messages… + if let Some((Some(room_id), Some(server_id))) = event + .r#type + .message() + .map(|msg| (msg.room_id(), msg.server_id())) + { + if self + .messages_repo + .contains(&account, &room_id, &server_id) + .await? + { + // We've seen this message already… + return Ok(()); + } + } + match event.r#type { MessageEventType::Received(mut message) => { // When we send a message to a MUC room we'll receive the same message from @@ -176,24 +170,24 @@ impl MessagesEventHandler { message.from = message.to.take(); message.to = Some(room_id.into_bare().into()); return self - .handle_sent_message(account, SentMessage::Message(message)) + .handle_sent_message(account, MessageOrCarbon::Message(message)) .await; } } } - self.handle_received_message(account, ReceivedMessage::Message(message)) + self.handle_received_message(account, MessageOrCarbon::Message(message)) .await? } MessageEventType::Sync(Carbon::Received(message)) => { - self.handle_received_message(account, ReceivedMessage::Carbon(message)) + self.handle_received_message(account, MessageOrCarbon::Carbon(message)) .await? } MessageEventType::Sync(Carbon::Sent(message)) => { - self.handle_sent_message(account, SentMessage::Carbon(message)) + self.handle_sent_message(account, MessageOrCarbon::Carbon(message)) .await? } MessageEventType::Sent(message) => { - self.handle_sent_message(account, SentMessage::Message(message)) + self.handle_sent_message(account, MessageOrCarbon::Message(message)) .await? } } @@ -203,7 +197,7 @@ impl MessagesEventHandler { async fn handle_received_message( &self, account: AccountId, - message: ReceivedMessage, + message: MessageOrCarbon, ) -> Result<()> { let Some(from) = message.from() else { error!("Received message from unknown sender."); @@ -227,6 +221,7 @@ impl MessagesEventHandler { .unwrap_or_else(|| TimeDelta::zero()); let parser = MessageParser::new( + self.message_id_provider.new_id(), room.clone(), server_time, self.encryption_domain_service.clone(), @@ -234,8 +229,8 @@ impl MessagesEventHandler { ); let parsed_message: Result = match message { - ReceivedMessage::Message(message) => parser.parse_message(message).await, - ReceivedMessage::Carbon(carbon) => parser.parse_forwarded_message(carbon).await, + MessageOrCarbon::Message(message) => parser.parse_message(message).await, + MessageOrCarbon::Carbon(carbon) => parser.parse_forwarded_message(carbon).await, }; let message = match parsed_message { @@ -252,17 +247,16 @@ impl MessagesEventHandler { }; if message.payload.is_message() { - match self + _ = self .sidebar_domain_service .handle_received_message(&room_id, &message) .await - { - Ok(_) => (), - Err(err) => error!( - "Could not insert sidebar item for message. {}", - err.to_string() - ), - } + .inspect_err(|err| { + error!( + "Could not insert sidebar item for message. {}", + err.to_string() + ) + }); } let Some(room) = room else { @@ -282,7 +276,11 @@ impl MessagesEventHandler { Ok(()) } - async fn handle_sent_message(&self, account: AccountId, message: SentMessage) -> Result<()> { + async fn handle_sent_message( + &self, + account: AccountId, + message: MessageOrCarbon, + ) -> Result<()> { let Some(room_id) = &message.room_id() else { error!("Sent message to unknown recipient."); return Ok(()); @@ -293,7 +291,19 @@ impl MessagesEventHandler { return Ok(()); }; + let existing_message_id = match message.remote_id() { + Some(remote_id) => { + self.messages_repo + .resolve_remote_id_to_message_id(&account, &room_id, &remote_id) + .await? + } + None => None, + }; + + let is_update = existing_message_id.is_some(); + let parser = MessageParser::new( + existing_message_id.unwrap_or_else(|| self.message_id_provider.new_id()), Some(room.clone()), room.features .local_time_to_server_time(self.time_provider.now()), @@ -302,8 +312,8 @@ impl MessagesEventHandler { ); let mut parsed_message = match message { - SentMessage::Message(message) => parser.parse_message(message).await, - SentMessage::Carbon(carbon) => parser.parse_forwarded_message(carbon).await, + MessageOrCarbon::Message(message) => parser.parse_message(message).await, + MessageOrCarbon::Carbon(carbon) => parser.parse_forwarded_message(carbon).await, }?; // Usually for sent messages the `from` would be our connected JID and the `to` would be @@ -318,6 +328,15 @@ impl MessagesEventHandler { // take our connected jid and plug it into the `from`. parsed_message.from = ParticipantId::User(account.to_user_id()); + // We had this message saved before without a StanzaId, so we'll save it again with + // the StanzaId but won't dispatch an event since this part is irrelevant for the UI. + if is_update { + self.messages_repo + .append(&account, &room_id, &[parsed_message]) + .await?; + return Ok(()); + } + self.save_message_and_dispatch_event(&account, room, parsed_message) .await?; Ok(()) @@ -329,42 +348,15 @@ impl MessagesEventHandler { room: Room, message: MessageLike, ) -> Result<()> { - let is_message_update = if let Some(message_id) = message.id.original_id() { - self.messages_repo - .contains(account, &room.room_id, message_id) - .await - .unwrap_or(false) - } else { - false - }; - let messages = [message]; self.messages_repo .append(&account, &room.room_id, &messages) .await?; let [message] = messages; - if is_message_update { - let message_id = if let Some(target_id) = message.target { - self.resolve_message_target_id(account, &room.room_id, &message.id, target_id) - .await - } else { - None - } - .unwrap_or_else(|| message.id.id().clone()); - - self.client_event_dispatcher.dispatch_room_event( - room.clone(), - ClientRoomEventType::MessagesUpdated { - message_ids: vec![message_id], - }, - ); - return Ok(()); - } - let event_type = if let Some(target) = message.target { let Some(message_id) = self - .resolve_message_target_id(account, &room.room_id, &message.id, target) + .resolve_message_target_id(account, &room.room_id, target) .await else { return Ok(()); @@ -381,7 +373,7 @@ impl MessagesEventHandler { } } else { ClientRoomEventType::MessagesAppended { - message_ids: vec![message.id.id().as_ref().into()], + message_ids: vec![message.id.clone()], } }; @@ -395,27 +387,30 @@ impl MessagesEventHandler { &self, account: &AccountId, room_id: &RoomId, - message_id: &MessageLikeId, target_id: MessageTargetId, - ) -> Option { - match target_id { - MessageTargetId::RemoteId(id) => Some(id), - MessageTargetId::ServerId(stanza_id) => { - match self - .messages_repo - .resolve_message_id(account, &room_id, &stanza_id) + ) -> Option { + let result = match &target_id { + MessageTargetId::RemoteId(remote_id) => { + self.messages_repo + .resolve_remote_id_to_message_id(account, &room_id, remote_id) .await - { - Ok(Some(id)) => Some(id), - Ok(None) => { - warn!("Not dispatching event for message with id '{}'. Failed to look up targeted MessageId from StanzaId '{}'.", message_id, stanza_id); - None - } - Err(err) => { - error!("Not dispatching event for message with id '{}'. Encountered error while looking up StanzaId '{}': {}", message_id, stanza_id, err.to_string()); - None - } - } + } + MessageTargetId::ServerId(server_id) => { + self.messages_repo + .resolve_server_id_to_message_id(account, &room_id, server_id) + .await + } + }; + + match result { + Ok(Some(id)) => Some(id), + Ok(None) => { + warn!("Not dispatching event for message. Failed to look up targeted MessageId from {:?}.", target_id); + None + } + Err(err) => { + error!("Not dispatching event for message. Encountered error while looking up {:?}: {}", target_id, err.to_string()); + None } } } diff --git a/crates/prose-core-client/src/app/services/room.rs b/crates/prose-core-client/src/app/services/room.rs index 60b32d85..86119b1a 100644 --- a/crates/prose-core-client/src/app/services/room.rs +++ b/crates/prose-core-client/src/app/services/room.rs @@ -16,19 +16,19 @@ use itertools::Itertools; use tracing::{debug, error, info, warn}; use prose_markup::MarkdownParser; -use prose_xmpp::{IDProvider, TimeProvider}; +use prose_xmpp::TimeProvider; use crate::app::deps::{ DynAppContext, DynClientEventDispatcher, DynDraftsRepository, DynEncryptionDomainService, - DynIDProvider, DynMessageArchiveService, DynMessagesRepository, DynMessagingService, + DynMessageArchiveService, DynMessageIdProvider, DynMessagesRepository, DynMessagingService, DynRoomAttributesService, DynRoomParticipationService, DynSidebarDomainService, DynSyncedRoomSettingsService, DynTimeProvider, DynUserInfoDomainService, }; use crate::domain::messaging::models::{ - send_message_request, ArchivedMessageRef, Emoji, Message, MessageLike, MessageLikeBody, - MessageLikeError, MessageParser, MessageRemoteId, MessageTargetId, + send_message_request, ArchivedMessageRef, Emoji, Message, MessageId, MessageLike, + MessageLikeBody, MessageLikeError, MessageParser, MessageRemoteId, MessageTargetId, }; -use crate::domain::messaging::models::{MessageLikeId, MessageLikePayload, SendMessageRequest}; +use crate::domain::messaging::models::{MessageLikePayload, SendMessageRequest}; use crate::domain::rooms::models::{Room as DomainRoom, RoomAffiliation, RoomSpec}; use crate::domain::settings::models::SyncedRoomSettings; use crate::domain::shared::models::{ @@ -41,6 +41,7 @@ use crate::dtos::{ MessageServerId, ParticipantBasicInfo, Reaction as ReactionDTO, RoomState, SendMessageRequest as SendMessageRequestDTO, UserId, HTML, }; +use crate::infra::xmpp::util::MessageExt; use crate::{ClientEvent, ClientRoomEventType}; pub struct Room { @@ -88,13 +89,13 @@ pub struct RoomInner { pub(crate) ctx: DynAppContext, pub(crate) drafts_repo: DynDraftsRepository, pub(crate) encryption_domain_service: DynEncryptionDomainService, - pub(crate) id_provider: DynIDProvider, pub(crate) message_archive_service: DynMessageArchiveService, + pub(crate) message_id_provider: DynMessageIdProvider, pub(crate) message_repo: DynMessagesRepository, pub(crate) messaging_service: DynMessagingService, pub(crate) participation_service: DynRoomParticipationService, - pub(crate) synced_room_settings_service: DynSyncedRoomSettingsService, pub(crate) sidebar_domain_service: DynSidebarDomainService, + pub(crate) synced_room_settings_service: DynSyncedRoomSettingsService, pub(crate) time_provider: DynTimeProvider, pub(crate) user_info_domain_service: DynUserInfoDomainService, } @@ -248,8 +249,9 @@ impl Room { &self.ctx.connected_account()?, &self.data.room_id, &[MessageLike { - id: MessageLikeId::new(Some(message_id.clone())), - stanza_id: None, + id: message_id.clone(), + remote_id: Some(message_id.to_string().into()), + server_id: None, target: None, to: None, from: self.ctx.connected_id()?.into_user_id().into(), @@ -275,7 +277,7 @@ impl Room { pub async fn update_message( &self, - id: MessageRemoteId, + id: MessageId, request: SendMessageRequestDTO, ) -> Result<()> { if request.is_empty() { @@ -283,6 +285,16 @@ impl Room { return Ok(()); } + let account = self.ctx.connected_account()?; + + let Some(target_id) = self + .message_repo + .resolve_message_id_to_remote_id(&account, &self.data.room_id, &id) + .await? + else { + bail!("Failed to resolve message id '{id}' to a server id") + }; + let (payload, body) = if let Some(body) = request.body { let parser = MarkdownParser::new(body.text.as_ref()); let html = HTML::new(parser.convert_to_html()); @@ -327,12 +339,13 @@ impl Room { // Save the unencrypted message so that we can look it up later… self.message_repo .append( - &self.ctx.connected_account()?, + &account, &self.data.room_id, &[MessageLike { - id: MessageLikeId::new(Some(message_id.clone())), - stanza_id: None, - target: Some(id.clone().into()), + id: message_id.clone(), + remote_id: Some(message_id.to_string().into()), + server_id: None, + target: Some(target_id.clone().into()), to: None, from: self.ctx.connected_id()?.into_user_id().into(), timestamp: self.time_provider.now(), @@ -342,7 +355,7 @@ impl Room { .await?; self.messaging_service - .update_message(&self.data.room_id, &id, request) + .update_message(&self.data.room_id, &target_id, request) .await?; self.client_event_dispatcher.dispatch_room_event( @@ -355,11 +368,7 @@ impl Room { Ok(()) } - pub async fn toggle_reaction_to_message( - &self, - id: MessageRemoteId, - emoji: Emoji, - ) -> Result<()> { + pub async fn toggle_reaction_to_message(&self, id: MessageId, emoji: Emoji) -> Result<()> { let account = self.ctx.connected_account()?; let messages = self .message_repo @@ -379,8 +388,11 @@ impl Room { match &self.data.room_id { RoomId::User(room_id) => { + let Some(remote_id) = &message.remote_id else { + bail!("Cannot react to message for which we do not have a RemoteId.") + }; self.messaging_service - .react_to_chat_message(room_id, &id, &all_emojis) + .react_to_chat_message(room_id, remote_id, &all_emojis) .await } RoomId::Muc(room_id) => { @@ -400,7 +412,7 @@ impl Room { .await } - pub async fn load_messages_with_ids(&self, ids: &[MessageRemoteId]) -> Result> { + pub async fn load_messages_with_ids(&self, ids: &[MessageId]) -> Result> { let account = self.ctx.connected_account()?; let messages = self .message_repo @@ -471,7 +483,7 @@ impl Room { }) } - pub async fn set_last_read_message(&self, id: &MessageRemoteId) -> Result<()> { + pub async fn set_last_read_message(&self, id: &MessageId) -> Result<()> { let account = self.ctx.connected_account()?; let mut messages = self @@ -485,7 +497,7 @@ impl Room { let message = messages.swap_remove(0); - if let Some(stanza_id) = message.stanza_id { + if let Some(stanza_id) = message.server_id { return self .set_last_read_message_ref( &account, @@ -569,7 +581,40 @@ impl Room { // and we want to push them into `messages` in the order 6, 5, 4, 3, 2, 1 which is // why we need to iterate over each page in reverse… for archive_message in page.messages.into_iter().rev() { + let inner_message = archive_message.forwarded.stanza.as_ref(); + + let is_our_message = inner_message + .and_then(|m| m.sender()) + .map(|s| self.data.is_current_user(&account, &s.to_participant_id())) + .unwrap_or_default(); + + let message_id = if is_our_message { + if let Some(remote_id) = inner_message.and_then(|m| m.id.clone()) { + self.message_repo + .resolve_remote_id_to_message_id( + &account, + &self.data.room_id, + &MessageRemoteId::from(remote_id), + ) + .await + .unwrap_or_default() + } else { + None + } + } else { + self.message_repo + .resolve_server_id_to_message_id( + &account, + &self.data.room_id, + &MessageServerId::from(archive_message.id.as_ref()), + ) + .await + .unwrap_or_default() + } + .unwrap_or_else(|| self.message_id_provider.new_id()); + let parsed_message = match MessageParser::new( + message_id, Some(self.data.clone()), Default::default(), self.encryption_domain_service.clone(), @@ -599,11 +644,12 @@ impl Room { if parsed_message.payload.is_message() { num_text_messages += 1; - if let Some(message_id) = parsed_message.id.original_id().cloned() { - text_message_ids.push(MessageTargetId::RemoteId(message_id)) + + if let Some(remote_id) = parsed_message.remote_id.clone() { + text_message_ids.push(MessageTargetId::RemoteId(remote_id)) } - if let Some(stanza_id) = parsed_message.stanza_id.as_ref() { - text_message_ids.push(MessageTargetId::ServerId(stanza_id.clone())) + if let Some(stanza_id) = parsed_message.server_id.clone() { + text_message_ids.push(MessageTargetId::ServerId(stanza_id)) } } @@ -718,8 +764,7 @@ impl Room { message.server_id.is_some() && message.server_id == last_read_message_id; message_dtos.push(MessageDTO { - id: message.remote_id, - stanza_id: message.server_id, + id: message.id, from, body: message.body, timestamp: message.timestamp, @@ -786,7 +831,7 @@ impl Room { } async fn show_system_message(&self, message: impl Into) -> Result<()> { - let id = MessageLikeId::new(Some(self.id_provider.new_id().into())); + let id = self.message_id_provider.new_id(); let message = message.into(); self.message_repo @@ -795,7 +840,8 @@ impl Room { &self.data.room_id, &[MessageLike { id: id.clone(), - stanza_id: None, + remote_id: Some(id.to_string().into()), + server_id: None, target: None, to: None, from: ParticipantId::User("prose-bot@prose.org".parse()?), @@ -817,7 +863,7 @@ impl Room { self.client_event_dispatcher.dispatch_room_event( self.data.clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![id.id().clone()], + message_ids: vec![id.into()], }, ); @@ -831,7 +877,7 @@ impl Room { ) -> Result { let Some((message, fallback, mentions)) = body else { return Ok(SendMessageRequest { - id: MessageRemoteId::from(self.id_provider.new_id()), + id: self.message_id_provider.new_id(), body: None, attachments, }); @@ -863,7 +909,7 @@ impl Room { }; Ok(SendMessageRequest { - id: MessageRemoteId::from(self.id_provider.new_id()), + id: self.message_id_provider.new_id(), body: Some(send_message_request::Body { payload, mentions }), attachments, }) @@ -904,7 +950,7 @@ impl Room { message_ref: Option, send_message_changed_events: bool, ) -> Result<()> { - let mut updated_stanza_ids = vec![]; + let mut updated_server_ids = vec![]; self.update_synced_settings(|settings| { if settings.last_read_message == message_ref { @@ -912,28 +958,28 @@ impl Room { } if let Some(former_message_ref) = settings.last_read_message.take() { - updated_stanza_ids.push(former_message_ref.stanza_id); + updated_server_ids.push(former_message_ref.stanza_id); } if let Some(stanza_id) = message_ref.as_ref().map(|r| r.stanza_id.clone()) { - updated_stanza_ids.push(stanza_id); + updated_server_ids.push(stanza_id); } settings.last_read_message = message_ref; }) .await; - if updated_stanza_ids.is_empty() { + if updated_server_ids.is_empty() { return Ok(()); } if send_message_changed_events { let mut updated_message_ids = vec![]; - for id in updated_stanza_ids { + for id in updated_server_ids { if let Some(message_id) = self .message_repo - .resolve_message_id(&account, &self.data.room_id, &id) + .resolve_server_id_to_message_id(&account, &self.data.room_id, &id) .await? { updated_message_ids.push(message_id); diff --git a/crates/prose-core-client/src/client_builder.rs b/crates/prose-core-client/src/client_builder.rs index 2812bdee..60495b74 100644 --- a/crates/prose-core-client/src/client_builder.rs +++ b/crates/prose-core-client/src/client_builder.rs @@ -10,8 +10,8 @@ use prose_xmpp::client::ConnectorProvider; use prose_xmpp::{ns, IDProvider, SystemTimeProvider, TimeProvider, UUIDProvider}; use crate::app::deps::{ - AppConfig, AppContext, AppDependencies, DynEncryptionService, DynIDProvider, DynRngProvider, - DynTimeProvider, DynUserDeviceIdProvider, + AppConfig, AppContext, AppDependencies, DynEncryptionService, DynIDProvider, + DynMessageIdProvider, DynRngProvider, DynTimeProvider, DynUserDeviceIdProvider, }; use crate::app::event_handlers::{ BlockListEventHandler, BookmarksEventHandler, ConnectionEventHandler, ContactListEventHandler, @@ -24,6 +24,7 @@ use crate::app::services::{ use crate::client::ClientInner; use crate::domain::encryption::services::{RandUserDeviceIdProvider, UserDeviceIdProvider}; use crate::domain::general::models::{Capabilities, Feature, SoftwareVersion}; +use crate::domain::messaging::services::{MessageIdProvider, WrappingMessageIdProvider}; use crate::domain::user_info::models::PROSE_IM_NODE; use crate::domain::user_info::repos::AvatarRepository; use crate::infra::general::{NanoIDProvider, OsRngProvider, RngProvider}; @@ -49,6 +50,7 @@ pub struct ClientBuilder { store: S, time_provider: DynTimeProvider, user_device_id_provider: DynUserDeviceIdProvider, + message_id_provider: DynMessageIdProvider, } impl ClientBuilder { @@ -66,6 +68,7 @@ impl ClientBuilder ClientBuilder { store, time_provider: self.time_provider, user_device_id_provider: self.user_device_id_provider, + message_id_provider: self.message_id_provider, } } } @@ -110,6 +114,7 @@ impl ClientBuilder { store: self.store, time_provider: self.time_provider, user_device_id_provider: self.user_device_id_provider, + message_id_provider: self.message_id_provider, } } } @@ -132,6 +137,7 @@ impl ClientBuilder { store: self.store, time_provider: self.time_provider, user_device_id_provider: self.user_device_id_provider, + message_id_provider: self.message_id_provider, } } } @@ -167,6 +173,14 @@ impl ClientBuilder { self } + pub fn set_message_id_provider( + mut self, + id_provider: P, + ) -> Self { + self.message_id_provider = Arc::new(id_provider); + self + } + pub fn set_time_provider(mut self, time_provider: T) -> Self { self.time_provider = Arc::new(time_provider); self @@ -263,6 +277,7 @@ impl ClientBuilder, A, DynE ctx: AppContext::new(capabilities, self.software_version, self.app_config), encryption_service: self.encryption_service, id_provider: self.id_provider, + message_id_provider: self.message_id_provider, rng_provider: self.rng_provider, server_event_handler_queue: server_event_handler_queue.clone(), short_id_provider: self.short_id_provider, diff --git a/crates/prose-core-client/src/client_event.rs b/crates/prose-core-client/src/client_event.rs index f5ad56b7..1e60ede3 100644 --- a/crates/prose-core-client/src/client_event.rs +++ b/crates/prose-core-client/src/client_event.rs @@ -8,7 +8,7 @@ use std::fmt::{Debug, Formatter}; use prose_xmpp::ConnectionError; use crate::app::dtos::RoomEnvelope; -use crate::domain::messaging::models::MessageRemoteId; +use crate::domain::messaging::models::MessageId; use crate::domain::shared::models::UserId; #[derive(Clone, PartialEq)] @@ -46,13 +46,13 @@ pub enum ClientEvent { #[derive(Debug, Clone, PartialEq)] pub enum ClientRoomEventType { /// One or many messages were either received or sent. - MessagesAppended { message_ids: Vec }, + MessagesAppended { message_ids: Vec }, /// One or many messages were received that affected earlier messages (e.g. a reaction). - MessagesUpdated { message_ids: Vec }, + MessagesUpdated { message_ids: Vec }, /// A message was deleted. - MessagesDeleted { message_ids: Vec }, + MessagesDeleted { message_ids: Vec }, /// The room went offline, came back online and contains new messages. MessagesNeedReload, diff --git a/crates/prose-core-client/src/domain/encryption/services/encryption_domain_service.rs b/crates/prose-core-client/src/domain/encryption/services/encryption_domain_service.rs index 81c4478c..009f9c91 100644 --- a/crates/prose-core-client/src/domain/encryption/services/encryption_domain_service.rs +++ b/crates/prose-core-client/src/domain/encryption/services/encryption_domain_service.rs @@ -9,7 +9,7 @@ use async_trait::async_trait; use prose_wasm_utils::{SendUnlessWasm, SyncUnlessWasm}; use crate::domain::encryption::models::{DecryptionContext, DeviceId, DeviceInfo, DeviceList}; -use crate::domain::messaging::models::{EncryptedPayload, KeyTransportPayload, MessageRemoteId}; +use crate::domain::messaging::models::{EncryptedPayload, KeyTransportPayload, MessageId}; use crate::domain::shared::models::{RoomId, UserId}; #[derive(Debug, thiserror::Error)] @@ -51,7 +51,7 @@ pub trait EncryptionDomainService: SendUnlessWasm + SyncUnlessWasm { &self, sender_id: &UserId, room_id: &RoomId, - message_id: Option<&MessageRemoteId>, + message_id: Option<&MessageId>, payload: EncryptedPayload, context: Option, ) -> Result; diff --git a/crates/prose-core-client/src/domain/encryption/services/impls/encryption_domain_service.rs b/crates/prose-core-client/src/domain/encryption/services/impls/encryption_domain_service.rs index c50930f1..3ab2e0f3 100644 --- a/crates/prose-core-client/src/domain/encryption/services/impls/encryption_domain_service.rs +++ b/crates/prose-core-client/src/domain/encryption/services/impls/encryption_domain_service.rs @@ -29,10 +29,10 @@ use crate::domain::encryption::models::{ use crate::domain::encryption::services::encryption_domain_service::{ DecryptionError, EncryptionError, }; -use crate::domain::messaging::models::MessageLikePayload; use crate::domain::messaging::models::{EncryptedPayload, KeyTransportPayload}; +use crate::domain::messaging::models::{MessageId, MessageLikePayload}; use crate::domain::shared::models::{AccountId, UserId}; -use crate::dtos::{EncryptionKey, MessageRemoteId, PreKeyId, RoomId}; +use crate::dtos::{EncryptionKey, PreKeyId, RoomId}; use crate::util::join_all; use super::super::EncryptionDomainService as EncryptionDomainServiceTrait; @@ -288,7 +288,7 @@ impl EncryptionDomainServiceTrait for EncryptionDomainService { &self, sender_id: &UserId, room_id: &RoomId, - message_id: Option<&MessageRemoteId>, + message_id: Option<&MessageId>, payload: EncryptedPayload, context: Option, ) -> Result { diff --git a/crates/prose-core-client/src/domain/messaging/models/message.rs b/crates/prose-core-client/src/domain/messaging/models/message.rs index db8f32c7..b47be5e6 100644 --- a/crates/prose-core-client/src/domain/messaging/models/message.rs +++ b/crates/prose-core-client/src/domain/messaging/models/message.rs @@ -11,6 +11,7 @@ use tracing::{error, info, warn}; use prose_utils::id_string; +use crate::domain::messaging::models::message_id::MessageId; use crate::domain::shared::models::ParticipantId; use crate::dtos::{Attachment, MessageRemoteId, MessageServerId, HTML}; @@ -32,6 +33,7 @@ pub struct Body { #[derive(Clone, Debug, PartialEq)] pub struct Message { + pub id: MessageId, pub remote_id: Option, pub server_id: Option, pub from: ParticipantId, @@ -84,74 +86,70 @@ impl Message { messages: impl IntoIterator, ) -> Vec { let mut messages_map = IndexMap::new(); - let mut stanza_to_id_map = HashMap::new(); + let mut remote_id_to_id_map = HashMap::new(); + let mut server_id_to_id_map = HashMap::new(); let mut modifiers: Vec = vec![]; for msg in messages.into_iter() { - match msg.payload { + let message = match msg.payload { MessageLikePayload::Message { body, attachments, encryption_info, is_transient: is_private, - } => { - let message_id = msg.id.clone(); - - let message = Message { - remote_id: message_id.into_original_id(), - server_id: msg.stanza_id, - from: msg.from.into(), - body: Body { - raw: body.raw, - html: body.html, - }, - timestamp: msg.timestamp.into(), - is_read: false, - is_edited: false, - is_delivered: false, - is_transient: is_private, - is_encrypted: encryption_info.is_some(), - reactions: vec![], - attachments, - mentions: body.mentions, - }; - - if let Some(stanza_id) = &message.server_id { - stanza_to_id_map.insert(stanza_id.clone(), msg.id.id().clone()); - }; - - messages_map.insert(msg.id.id().clone(), Some(message)); + } => Message { + id: msg.id, + remote_id: msg.remote_id, + server_id: msg.server_id, + from: msg.from.into(), + body: Body { + raw: body.raw, + html: body.html, + }, + timestamp: msg.timestamp.into(), + is_read: false, + is_edited: false, + is_delivered: false, + is_transient: is_private, + is_encrypted: encryption_info.is_some(), + reactions: vec![], + attachments, + mentions: body.mentions, + }, + MessageLikePayload::Error { message: error } => Message { + id: msg.id, + remote_id: msg.remote_id, + server_id: msg.server_id, + from: msg.from.into(), + body: Body { + raw: error.clone(), + html: HTML::new(error), + }, + timestamp: msg.timestamp.into(), + is_read: false, + is_edited: false, + is_delivered: false, + is_transient: false, + is_encrypted: false, + reactions: vec![], + attachments: vec![], + mentions: vec![], + }, + _ => { + modifiers.push(msg); + continue; } - MessageLikePayload::Error { message: error } => { - let message_id = msg.id.clone(); - - let message = Message { - remote_id: message_id.into_original_id(), - server_id: msg.stanza_id, - from: msg.from.into(), - body: Body { - raw: error.clone(), - html: HTML::new(error), - }, - timestamp: msg.timestamp.into(), - is_read: false, - is_edited: false, - is_delivered: false, - is_transient: false, - is_encrypted: false, - reactions: vec![], - attachments: vec![], - mentions: vec![], - }; + }; - if let Some(stanza_id) = &message.server_id { - stanza_to_id_map.insert(stanza_id.clone(), msg.id.id().clone()); - }; + if let Some(remote_id) = message.remote_id.clone() { + remote_id_to_id_map.insert(remote_id, message.id.clone()); + } - messages_map.insert(msg.id.id().clone(), Some(message)); - } - _ => modifiers.push(msg), + if let Some(stanza_id) = message.server_id.clone() { + server_id_to_id_map.insert(stanza_id, message.id.clone()); } + + messages_map.insert(message.id.clone(), Some(message)); } for modifier in modifiers.into_iter() { @@ -161,17 +159,23 @@ impl Message { }; let message_id = match target_id { - MessageTargetId::RemoteId(ref id) => id, + MessageTargetId::RemoteId(remote_id) => { + let Some(id) = remote_id_to_id_map.get(&remote_id) else { + info!("Could not resolve RemoteId '{remote_id}' to a MessageId"); + continue; + }; + id.clone() + } MessageTargetId::ServerId(stanza_id) => { - let Some(id) = stanza_to_id_map.get(&stanza_id) else { + let Some(id) = server_id_to_id_map.get(&stanza_id) else { info!("Could not resolve StanzaId '{stanza_id}' to a MessageId"); continue; }; - id + id.clone() } }; - let Some(Some(message)) = messages_map.get_mut(message_id) else { + let Some(Some(message)) = messages_map.get_mut(&message_id) else { warn!("Ignoring message modifier targeting unknown message '{message_id}'."); continue; }; @@ -239,7 +243,7 @@ impl Message { } } MessageLikePayload::Retraction => { - messages_map.insert(message_id.clone(), None); + messages_map.insert(message_id, None); } } } @@ -262,6 +266,78 @@ mod tests { use super::*; + #[test] + fn test_reduces_messages_with_duplicate_remote_ids() { + let messages = vec![ + MessageLike { + id: "id1".into(), + remote_id: Some("mid1".into()), + server_id: Some("sid1".into()), + target: None, + to: Some(bare!("a@prose.org")), + from: user_id!("b@prose.org").into(), + timestamp: Utc.with_ymd_and_hms(2023, 04, 07, 16, 00, 00).unwrap(), + payload: MessageLikePayload::message("Message 1"), + }, + MessageLike { + id: "id2".into(), + remote_id: Some("mid1".into()), + server_id: Some("sid1".into()), + target: None, + to: Some(bare!("a@prose.org")), + from: user_id!("b@prose.org").into(), + timestamp: Utc.with_ymd_and_hms(2023, 04, 07, 16, 00, 01).unwrap(), + payload: MessageLikePayload::message("Message 2"), + }, + ]; + + let reduced_message = Message::reducing_messages(messages); + + assert_eq!( + vec![ + Message { + id: "id1".into(), + remote_id: Some("mid1".into()), + server_id: Some("sid1".into()), + from: user_id!("b@prose.org").into(), + body: Body { + raw: "Message 1".to_string(), + html: "

Message 1

".to_string().into(), + }, + timestamp: Utc.with_ymd_and_hms(2023, 04, 07, 16, 00, 00).unwrap(), + is_read: false, + is_edited: false, + is_delivered: false, + is_transient: false, + is_encrypted: false, + reactions: vec![], + attachments: vec![], + mentions: vec![] + }, + Message { + id: "id2".into(), + remote_id: Some("mid1".into()), + server_id: Some("sid1".into()), + from: user_id!("b@prose.org").into(), + body: Body { + raw: "Message 2".to_string(), + html: "

Message 2

".to_string().into(), + }, + timestamp: Utc.with_ymd_and_hms(2023, 04, 07, 16, 00, 01).unwrap(), + is_read: false, + is_edited: false, + is_delivered: false, + is_transient: false, + is_encrypted: false, + reactions: vec![], + attachments: vec![], + mentions: vec![] + } + ], + reduced_message, + ) + } + #[test] fn test_toggle_reaction() { let mut message = MessageBuilder::new_with_index(1).build_message(); @@ -374,8 +450,9 @@ mod tests { fn test_reduces_emojis() { let messages = [ MessageLike { - id: "1".into(), - stanza_id: Some("stanza-id-1".into()), + id: "id1".into(), + remote_id: Some("1".into()), + server_id: Some("stanza-id-1".into()), target: None, to: Some(bare!("a@prose.org")), from: user_id!("b@prose.org").into(), @@ -395,8 +472,9 @@ mod tests { }, }, MessageLike { - id: "2".into(), - stanza_id: None, + id: "id2".into(), + remote_id: Some("2".into()), + server_id: None, target: Some(MessageTargetId::RemoteId("1".into())), to: Some(bare!("a@prose.org")), from: user_id!("b@prose.org").into(), @@ -409,8 +487,9 @@ mod tests { }, }, MessageLike { - id: "3".into(), - stanza_id: None, + id: "id3".into(), + remote_id: Some("3".into()), + server_id: None, target: Some(MessageTargetId::RemoteId("1".into())), to: Some(bare!("a@prose.org")), from: user_id!("c@prose.org").into(), @@ -423,8 +502,9 @@ mod tests { }, }, MessageLike { - id: "4".into(), - stanza_id: None, + id: "id4".into(), + remote_id: Some("4".into()), + server_id: None, target: Some(MessageTargetId::RemoteId("1".into())), to: Some(bare!("a@prose.org")), from: user_id!("b@prose.org").into(), @@ -437,8 +517,9 @@ mod tests { }, }, MessageLike { - id: "5".into(), - stanza_id: None, + id: "id5".into(), + remote_id: Some("5".into()), + server_id: None, target: Some(MessageTargetId::ServerId("stanza-id-1".into())), to: Some(bare!("a@prose.org")), from: user_id!("b@prose.org").into(), @@ -455,6 +536,7 @@ mod tests { let reduced_message = Message::reducing_messages(messages).pop().unwrap(); assert_eq!( Message { + id: "id1".into(), remote_id: Some("1".into()), server_id: Some("stanza-id-1".into()), from: user_id!("b@prose.org").into(), diff --git a/crates/prose-core-client/src/domain/messaging/models/message_id.rs b/crates/prose-core-client/src/domain/messaging/models/message_id.rs index 56018c8d..99de54c4 100644 --- a/crates/prose-core-client/src/domain/messaging/models/message_id.rs +++ b/crates/prose-core-client/src/domain/messaging/models/message_id.rs @@ -7,11 +7,20 @@ use serde::{Deserialize, Serialize}; use prose_utils::id_string; -// The ID assigned by a client sending the message. It is not guaranteed to be unique. -id_string!(MessageRemoteId); +id_string!( + /// The ID assigned by a client sending the message. It is not guaranteed to be unique. + MessageRemoteId +); -// The ID assigned by the server to the message. -id_string!(MessageServerId); +id_string!( + /// The ID assigned by the server to the message. + MessageServerId +); + +id_string!( + /// The ID assigned to the message by us locally. + MessageId +); #[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] #[serde(tag = "type", content = "id")] diff --git a/crates/prose-core-client/src/domain/messaging/models/message_like.rs b/crates/prose-core-client/src/domain/messaging/models/message_like.rs index bdb6e915..cf1beeb8 100644 --- a/crates/prose-core-client/src/domain/messaging/models/message_like.rs +++ b/crates/prose-core-client/src/domain/messaging/models/message_like.rs @@ -3,34 +3,29 @@ // Copyright: 2023, Marc Bauer // License: Mozilla Public License v2.0 (MPL v2.0) -use std::fmt::{Debug, Display}; +use std::fmt::Debug; -use anyhow::Result; use chrono::{DateTime, Utc}; use jid::BareJid; use serde::{Deserialize, Serialize}; -use uuid::Uuid; use prose_xmpp::stanza::message; +use crate::domain::messaging::models::message_id::MessageId; use crate::domain::messaging::models::{Attachment, Mention, MessageTargetId}; use crate::domain::shared::models::{ParticipantId, HTML}; use crate::dtos::DeviceId; use super::{MessageRemoteId, MessageServerId}; -/// An ID that can act as a placeholder in the rare cases when a message doesn't have an ID. Since -/// our DataCache backends require some ID for each message we simply generate one. -#[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] -pub struct MessageLikeId(MessageRemoteId); - /// A type that describes permanent messages, i.e. messages that need to be replayed to restore /// the complete history of a conversation. Note that ephemeral messages like chat states are /// handled differently. #[derive(Debug, PartialEq, Clone)] pub struct MessageLike { - pub id: MessageLikeId, - pub stanza_id: Option, + pub id: MessageId, + pub remote_id: Option, + pub server_id: Option, pub target: Option, pub to: Option, pub from: ParticipantId, @@ -109,46 +104,3 @@ impl Payload { } } } - -impl MessageLikeId { - pub fn new(id: Option) -> Self { - if let Some(id) = id { - return MessageLikeId(id); - } - return MessageLikeId(format!("!!{}", Uuid::new_v4().to_string()).into()); - } - - /// Returns either the original message ID or the generated one. - pub fn id(&self) -> &MessageRemoteId { - &self.0 - } - - /// Returns the original message ID or None if we contain a generated ID. - pub fn into_original_id(self) -> Option { - if self.0.as_ref().starts_with("!!") { - return None; - } - return Some(self.0); - } - - pub fn original_id(&self) -> Option<&MessageRemoteId> { - if self.0.as_ref().starts_with("!!") { - return None; - } - return Some(&self.0); - } -} - -impl std::str::FromStr for MessageLikeId { - type Err = (); - - fn from_str(s: &str) -> Result { - Ok(MessageLikeId(s.to_string().into())) - } -} - -impl Display for MessageLikeId { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - Display::fmt(&self.0, f) - } -} diff --git a/crates/prose-core-client/src/domain/messaging/models/message_parser.rs b/crates/prose-core-client/src/domain/messaging/models/message_parser.rs index 7a957cc2..f21bbe6f 100644 --- a/crates/prose-core-client/src/domain/messaging/models/message_parser.rs +++ b/crates/prose-core-client/src/domain/messaging/models/message_parser.rs @@ -17,7 +17,7 @@ use crate::app::deps::DynEncryptionDomainService; use crate::domain::encryption::models::DecryptionContext; use crate::domain::messaging::models::message_like::Payload; use crate::domain::messaging::models::{ - EncryptedMessage, MessageLike, MessageLikeBody, MessageLikeEncryptionInfo, MessageLikeId, + EncryptedMessage, MessageId, MessageLike, MessageLikeBody, MessageLikeEncryptionInfo, MessageServerId, MessageTargetId, StanzaParseError, }; use crate::domain::rooms::models::Room; @@ -29,6 +29,7 @@ use crate::infra::xmpp::type_conversions::stanza_error::StanzaErrorExt; use crate::infra::xmpp::util::MessageExt; pub struct MessageParser { + message_id: MessageId, room: Option, timestamp: DateTime, encryption_domain_service: DynEncryptionDomainService, @@ -37,12 +38,14 @@ pub struct MessageParser { impl MessageParser { pub fn new( + next_message_id: MessageId, room: Option, now: DateTime, encryption_domain_service: DynEncryptionDomainService, decryption_context: Option, ) -> Self { Self { + message_id: next_message_id, room, timestamp: now.round_subsecs(0), encryption_domain_service, @@ -54,7 +57,7 @@ impl MessageParser { impl MessageParser { pub async fn parse_mam_message(self, mam_message: ArchivedMessage) -> Result { let mut parsed_message = self.parse_forwarded_message(mam_message.forwarded).await?; - parsed_message.stanza_id = Some(MessageServerId::from(mam_message.id.into_inner())); + parsed_message.server_id = Some(MessageServerId::from(mam_message.id.into_inner())); Ok(parsed_message) } @@ -106,12 +109,12 @@ impl MessageParser { .unwrap_or(self.timestamp); let message = message.into_inner(); - let id = MessageLikeId::new(message.id.map(Into::into)); let to = message.to.map(|jid| jid.into_bare()); Ok(MessageLike { - id, - stanza_id, + id: self.message_id, + remote_id: message.id.map(MessageRemoteId::from), + server_id: stanza_id, target, to, from: user_id.map(ParticipantId::User).unwrap_or(participant_id), @@ -327,18 +330,13 @@ impl MessageParser { if let (Some(sender_id), Some(omemo_element)) = (sender_id, message.omemo_element()) { let sender = DeviceId::from(omemo_element.header.sid); let encrypted_message = EncryptedMessage::from(omemo_element); - let message_id = message - .id - .as_ref() - .map(|id| MessageRemoteId::from(id.clone())); - let decryption_result = match encrypted_message { EncryptedMessage::Message(message) => { self.encryption_domain_service .decrypt_message( sender_id, room_id, - message_id.as_ref(), + Some(&self.message_id), message, self.decryption_context.clone(), ) diff --git a/crates/prose-core-client/src/domain/messaging/models/mod.rs b/crates/prose-core-client/src/domain/messaging/models/mod.rs index 617de795..632c956f 100644 --- a/crates/prose-core-client/src/domain/messaging/models/mod.rs +++ b/crates/prose-core-client/src/domain/messaging/models/mod.rs @@ -11,10 +11,10 @@ pub(crate) use error::StanzaParseError; pub use mention::Mention; #[allow(unused_imports)] // Reaction is required in unit tests pub use message::{Body, Emoji, Message, Reaction}; -pub use message_id::{MessageRemoteId, MessageServerId, MessageTargetId}; +pub use message_id::{MessageId, MessageRemoteId, MessageServerId, MessageTargetId}; pub use message_like::{ Body as MessageLikeBody, EncryptionInfo as MessageLikeEncryptionInfo, MessageLike, - MessageLikeId, Payload as MessageLikePayload, + Payload as MessageLikePayload, }; pub use message_parser::{MessageLikeError, MessageParser}; pub use message_ref::{ArchivedMessageRef, MessageRef}; diff --git a/crates/prose-core-client/src/domain/messaging/models/send_message_request.rs b/crates/prose-core-client/src/domain/messaging/models/send_message_request.rs index 0bc0ba29..38885d5c 100644 --- a/crates/prose-core-client/src/domain/messaging/models/send_message_request.rs +++ b/crates/prose-core-client/src/domain/messaging/models/send_message_request.rs @@ -3,14 +3,14 @@ // Copyright: 2024, Marc Bauer // License: Mozilla Public License v2.0 (MPL v2.0) -use super::EncryptedPayload; use crate::domain::shared::models::{Markdown, StyledMessage}; -use super::{Attachment, Mention, MessageRemoteId}; +use super::{Attachment, Mention}; +use super::{EncryptedPayload, MessageId}; #[derive(Debug, Clone, PartialEq)] pub struct SendMessageRequest { - pub id: MessageRemoteId, + pub id: MessageId, pub body: Option, pub attachments: Vec, } diff --git a/crates/prose-core-client/src/domain/messaging/repos/messages_repository.rs b/crates/prose-core-client/src/domain/messaging/repos/messages_repository.rs index 92713485..7670266b 100644 --- a/crates/prose-core-client/src/domain/messaging/repos/messages_repository.rs +++ b/crates/prose-core-client/src/domain/messaging/repos/messages_repository.rs @@ -10,7 +10,7 @@ use chrono::{DateTime, Utc}; use prose_wasm_utils::{SendUnlessWasm, SyncUnlessWasm}; use crate::domain::messaging::models::{ - ArchivedMessageRef, MessageLike, MessageRemoteId, MessageServerId, MessageTargetId, + ArchivedMessageRef, MessageId, MessageLike, MessageRemoteId, MessageServerId, MessageTargetId, }; use crate::domain::shared::models::{AccountId, RoomId}; @@ -23,14 +23,14 @@ pub trait MessagesRepository: SendUnlessWasm + SyncUnlessWasm { &self, account: &AccountId, room_id: &RoomId, - id: &MessageRemoteId, + id: &MessageId, ) -> Result>; /// Returns all parts (MessageLike) that make up all messages in `ids`. Sorted chronologically. async fn get_all( &self, account: &AccountId, room_id: &RoomId, - ids: &[MessageRemoteId], + ids: &[MessageId], ) -> Result>; /// Returns all messages that target any IDs contained in `targeted_id` and are newer /// than `newer_than`. @@ -45,7 +45,7 @@ pub trait MessagesRepository: SendUnlessWasm + SyncUnlessWasm { &self, account: &AccountId, room_id: &RoomId, - id: &MessageRemoteId, + id: &MessageServerId, ) -> Result; async fn append( &self, @@ -55,13 +55,25 @@ pub trait MessagesRepository: SendUnlessWasm + SyncUnlessWasm { ) -> Result<()>; async fn clear_cache(&self, account: &AccountId) -> Result<()>; - /// Attempts to look up the message identified by `stanza_id` and returns - /// its `id` if it was found. - async fn resolve_message_id( + async fn resolve_server_id_to_message_id( &self, account: &AccountId, room_id: &RoomId, - stanza_id: &MessageServerId, + server_id: &MessageServerId, + ) -> Result>; + + async fn resolve_remote_id_to_message_id( + &self, + account: &AccountId, + room_id: &RoomId, + remote_id: &MessageRemoteId, + ) -> Result>; + + async fn resolve_message_id_to_remote_id( + &self, + account: &AccountId, + room_id: &RoomId, + id: &MessageId, ) -> Result>; /// Returns the latest message, if available, that has a `stanza_id` set and was received diff --git a/crates/prose-core-client/src/domain/messaging/services/impls/message_archive_domain_service.rs b/crates/prose-core-client/src/domain/messaging/services/impls/message_archive_domain_service.rs index ebee8f70..f4543510 100644 --- a/crates/prose-core-client/src/domain/messaging/services/impls/message_archive_domain_service.rs +++ b/crates/prose-core-client/src/domain/messaging/services/impls/message_archive_domain_service.rs @@ -11,17 +11,17 @@ use tracing::{error, info}; use prose_proc_macros::DependenciesStruct; use prose_xmpp::TimeProvider; +use super::super::MessageArchiveDomainService as MessageArchiveDomainServiceTrait; use crate::app::deps::{ DynAppContext, DynEncryptionDomainService, DynLocalRoomSettingsRepository, - DynMessageArchiveService, DynMessagesRepository, DynTimeProvider, + DynMessageArchiveService, DynMessageIdProvider, DynMessagesRepository, DynTimeProvider, }; use crate::domain::encryption::models::DecryptionContext; use crate::domain::messaging::models::{MessageLike, MessageLikeError, MessageParser}; use crate::domain::messaging::services::MessagePage; use crate::domain::rooms::models::Room; -use crate::dtos::MessageServerId; - -use super::super::MessageArchiveDomainService as MessageArchiveDomainServiceTrait; +use crate::dtos::{AccountId, MessageRemoteId, MessageServerId}; +use crate::infra::xmpp::util::MessageExt; #[derive(DependenciesStruct)] pub struct MessageArchiveDomainService { @@ -29,6 +29,7 @@ pub struct MessageArchiveDomainService { encryption_domain_service: DynEncryptionDomainService, local_room_settings_repo: DynLocalRoomSettingsRepository, message_archive_service: DynMessageArchiveService, + message_id_provider: DynMessageIdProvider, message_repo: DynMessagesRepository, time_provider: DynTimeProvider, } @@ -87,7 +88,7 @@ impl MessageArchiveDomainServiceTrait for MessageArchiveDomainService { .map(|m| MessageServerId::from(m.id.as_ref())); let mut is_last_page = page.is_last; - self.parse_message_page(room, page, &mut messages, &context) + self.parse_message_page(&account, room, page, &mut messages, &context) .await; while !is_last_page { @@ -106,7 +107,7 @@ impl MessageArchiveDomainServiceTrait for MessageArchiveDomainService { .map(|m| MessageServerId::from(m.id.as_ref())); is_last_page = page.is_last; - self.parse_message_page(room, page, &mut messages, &context) + self.parse_message_page(&account, room, page, &mut messages, &context) .await; } @@ -138,13 +139,52 @@ impl MessageArchiveDomainServiceTrait for MessageArchiveDomainService { impl MessageArchiveDomainService { async fn parse_message_page( &self, + account: &AccountId, room: &Room, page: MessagePage, messages: &mut Vec, context: &DecryptionContext, ) { for archive_message in page.messages { + let inner_message = archive_message.forwarded.stanza.as_ref(); + + let is_our_message = inner_message + .and_then(|m| m.sender()) + .map(|s| room.is_current_user(&account, &s.to_participant_id())) + .unwrap_or_default(); + + let message_id = if is_our_message { + if let Some(remote_id) = archive_message + .forwarded + .stanza + .as_ref() + .and_then(|m| m.id.clone()) + { + self.message_repo + .resolve_remote_id_to_message_id( + &account, + &room.room_id, + &MessageRemoteId::from(remote_id), + ) + .await + .unwrap_or_default() + } else { + None + } + } else { + self.message_repo + .resolve_server_id_to_message_id( + &account, + &room.room_id, + &MessageServerId::from(archive_message.id.as_ref()), + ) + .await + .unwrap_or_default() + } + .unwrap_or_else(|| self.message_id_provider.new_id()); + let parsed_message = match MessageParser::new( + message_id, Some(room.clone()), Default::default(), self.encryption_domain_service.clone(), diff --git a/crates/prose-core-client/src/domain/messaging/services/message_id_provider.rs b/crates/prose-core-client/src/domain/messaging/services/message_id_provider.rs new file mode 100644 index 00000000..6fb422cf --- /dev/null +++ b/crates/prose-core-client/src/domain/messaging/services/message_id_provider.rs @@ -0,0 +1,44 @@ +// prose-core-client/prose-core-client +// +// Copyright: 2024, Marc Bauer +// License: Mozilla Public License v2.0 (MPL v2.0) + +use crate::dtos::MessageId; +use prose_xmpp::{IDProvider, UUIDProvider}; + +pub trait MessageIdProvider: Send + Sync { + fn new_id(&self) -> MessageId; +} + +pub struct WrappingMessageIdProvider { + id_provider: T, +} + +impl WrappingMessageIdProvider { + pub fn new(id_provider: T) -> Self { + Self { id_provider } + } +} + +impl WrappingMessageIdProvider { + pub fn uuid() -> Self { + Self { + id_provider: UUIDProvider::new(), + } + } +} + +#[cfg(feature = "test")] +impl WrappingMessageIdProvider { + pub fn incrementing(prefix: &str) -> Self { + Self { + id_provider: prose_xmpp::test::IncrementingIDProvider::new(prefix), + } + } +} + +impl MessageIdProvider for WrappingMessageIdProvider { + fn new_id(&self) -> MessageId { + MessageId::from(self.id_provider.new_id()) + } +} diff --git a/crates/prose-core-client/src/domain/messaging/services/mod.rs b/crates/prose-core-client/src/domain/messaging/services/mod.rs index 1242f782..8845b701 100644 --- a/crates/prose-core-client/src/domain/messaging/services/mod.rs +++ b/crates/prose-core-client/src/domain/messaging/services/mod.rs @@ -5,12 +5,14 @@ pub use message_archive_domain_service::MessageArchiveDomainService; pub use message_archive_service::{MessageArchiveService, MessagePage}; +pub use message_id_provider::{MessageIdProvider, WrappingMessageIdProvider}; pub use message_migration_domain_service::MessageMigrationDomainService; pub use messaging_service::MessagingService; pub mod impls; mod message_archive_domain_service; mod message_archive_service; +mod message_id_provider; mod message_migration_domain_service; mod messaging_service; diff --git a/crates/prose-core-client/src/domain/rooms/models/room.rs b/crates/prose-core-client/src/domain/rooms/models/room.rs index 3a6daf34..6f0a7601 100644 --- a/crates/prose-core-client/src/domain/rooms/models/room.rs +++ b/crates/prose-core-client/src/domain/rooms/models/room.rs @@ -202,6 +202,17 @@ impl Room { self.inner.details.write().statistics.needs_update = true; } + pub fn is_current_user(&self, account: &AccountId, participant: &ParticipantId) -> bool { + if self.room_id.is_muc_room() { + // We're generally trying to resolve OccupantIDs into UserIDs if possible. + // So the sender could be either/or depending on the room configuration. + Some(participant) == self.occupant_id().map(ParticipantId::Occupant).as_ref() + || participant == &ParticipantId::User(account.to_user_id()) + } else { + participant == &ParticipantId::User(account.to_user_id()) + } + } + pub async fn update_statistics_if_needed( &self, account: &AccountId, diff --git a/crates/prose-core-client/src/infra/messaging/caching_message_repository.rs b/crates/prose-core-client/src/infra/messaging/caching_message_repository.rs index 986fe44b..918a53a5 100644 --- a/crates/prose-core-client/src/infra/messaging/caching_message_repository.rs +++ b/crates/prose-core-client/src/infra/messaging/caching_message_repository.rs @@ -12,7 +12,7 @@ use chrono::{DateTime, Utc}; use prose_store::prelude::*; use crate::domain::messaging::models::{ - ArchivedMessageRef, MessageLike, MessageRemoteId, MessageServerId, MessageTargetId, + ArchivedMessageRef, MessageId, MessageLike, MessageRemoteId, MessageServerId, MessageTargetId, }; use crate::domain::messaging::repos::MessagesRepository; use crate::domain::shared::models::{AccountId, RoomId}; @@ -37,7 +37,7 @@ impl MessagesRepository for CachingMessageRepository { &self, account: &AccountId, room_id: &RoomId, - id: &MessageRemoteId, + id: &MessageId, ) -> Result> { Ok(self.get_all(account, room_id, &[id.clone()]).await?) } @@ -46,7 +46,7 @@ impl MessagesRepository for CachingMessageRepository { &self, account: &AccountId, room_id: &RoomId, - ids: &[MessageRemoteId], + ids: &[MessageId], ) -> Result> { let tx = self .store @@ -54,8 +54,8 @@ impl MessagesRepository for CachingMessageRepository { .await?; let collection = tx.readable_collection(MessageRecord::collection())?; - let stanza_id_target_idx = collection.index(&MessageRecord::stanza_id_target_idx())?; - let message_id_target_idx = collection.index(&MessageRecord::message_id_target_idx())?; + let stanza_id_target_idx = collection.index(&MessageRecord::server_id_target_idx())?; + let message_id_target_idx = collection.index(&MessageRecord::remote_id_target_idx())?; let message_id_idx = collection.index(&MessageRecord::message_id_idx())?; let mut messages: Vec = vec![]; @@ -77,7 +77,7 @@ impl MessagesRepository for CachingMessageRepository { .map(MessageLike::from), ); - if let Some(stanza_id) = message.as_ref().and_then(|m| m.stanza_id.as_ref()) { + if let Some(stanza_id) = message.as_ref().and_then(|m| m.server_id.as_ref()) { messages.extend( &mut stanza_id_target_idx .get_all_values::( @@ -113,8 +113,8 @@ impl MessagesRepository for CachingMessageRepository { .await?; let collection = tx.readable_collection(MessageRecord::collection())?; - let stanza_idx = collection.index(&MessageRecord::stanza_id_target_idx())?; - let message_idx = collection.index(&MessageRecord::message_id_target_idx())?; + let stanza_idx = collection.index(&MessageRecord::server_id_target_idx())?; + let message_idx = collection.index(&MessageRecord::remote_id_target_idx())?; let mut messages: Vec = vec![]; for id in targeted_ids { @@ -155,17 +155,15 @@ impl MessagesRepository for CachingMessageRepository { &self, account: &AccountId, room_id: &RoomId, - id: &MessageRemoteId, + id: &MessageServerId, ) -> Result { let tx = self .store .transaction_for_reading(&[MessageRecord::collection()]) .await?; let collection = tx.readable_collection(MessageRecord::collection())?; - let idx = collection.index(&MessageRecord::message_id_idx())?; - let flag = idx - .contains_key(&(account.clone(), room_id.clone(), id.clone())) - .await?; + let idx = collection.index(&MessageRecord::server_id_idx())?; + let flag = idx.contains_key(&(account, room_id, id)).await?; Ok(flag) } @@ -205,22 +203,58 @@ impl MessagesRepository for CachingMessageRepository { Ok(()) } - async fn resolve_message_id( + async fn resolve_server_id_to_message_id( &self, account: &AccountId, room_id: &RoomId, - stanza_id: &MessageServerId, - ) -> Result> { + server_id: &MessageServerId, + ) -> Result> { let tx = self .store .transaction_for_reading(&[MessageRecord::collection()]) .await?; let collection = tx.readable_collection(MessageRecord::collection())?; - let stanza_idx = collection.index(&MessageRecord::stanza_id_idx())?; + let stanza_idx = collection.index(&MessageRecord::server_id_idx())?; let message = stanza_idx - .get::<_, MessageRecord>(&(account.clone(), room_id.clone(), stanza_id.clone())) + .get::<_, MessageRecord>(&(account, room_id, server_id)) + .await?; + Ok(message.map(|m| m.message_id)) + } + + async fn resolve_remote_id_to_message_id( + &self, + account: &AccountId, + room_id: &RoomId, + remote_id: &MessageRemoteId, + ) -> Result> { + let tx = self + .store + .transaction_for_reading(&[MessageRecord::collection()]) + .await?; + let collection = tx.readable_collection(MessageRecord::collection())?; + let remote_id_idx = collection.index(&MessageRecord::remote_id_idx())?; + let message = remote_id_idx + .get::<_, MessageRecord>(&(account, room_id, remote_id)) + .await?; + Ok(message.map(|m| m.message_id)) + } + + async fn resolve_message_id_to_remote_id( + &self, + account: &AccountId, + room_id: &RoomId, + id: &MessageId, + ) -> Result> { + let tx = self + .store + .transaction_for_reading(&[MessageRecord::collection()]) + .await?; + let collection = tx.readable_collection(MessageRecord::collection())?; + let message_id_idx = collection.index(&MessageRecord::message_id_idx())?; + let message = message_id_idx + .get::<_, MessageRecord>(&(account, room_id, id)) .await?; - Ok(message.and_then(|m| m.message_id.into_original_id())) + Ok(message.and_then(|m| m.remote_id)) } async fn get_last_received_message( @@ -251,7 +285,7 @@ impl MessagesRepository for CachingMessageRepository { return (result, is_placeholder); } - let Some(stanza_id) = message.stanza_id else { + let Some(stanza_id) = message.server_id else { return (result, is_placeholder); }; diff --git a/crates/prose-core-client/src/infra/messaging/message_record.rs b/crates/prose-core-client/src/infra/messaging/message_record.rs index a469f2b5..6bd20268 100644 --- a/crates/prose-core-client/src/infra/messaging/message_record.rs +++ b/crates/prose-core-client/src/infra/messaging/message_record.rs @@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize}; use prose_store::prelude::*; use crate::domain::messaging::models::{ - MessageLike, MessageLikeId, MessageLikePayload, MessageTargetId, + MessageId, MessageLike, MessageLikePayload, MessageTargetId, }; use crate::domain::shared::models::AccountId; use crate::dtos::{MessageRemoteId, MessageServerId, ParticipantId, RoomId}; @@ -20,10 +20,11 @@ pub struct MessageRecord { pub id: String, pub account: AccountId, pub room_id: RoomId, - pub message_id: MessageLikeId, - pub message_id_target: Option, - pub stanza_id: Option, - pub stanza_id_target: Option, + pub message_id: MessageId, + pub remote_id: Option, + pub remote_id_target: Option, + pub server_id: Option, + pub server_id_target: Option, pub to: Option, pub from: ParticipantId, pub timestamp: DateTime, @@ -33,25 +34,28 @@ pub struct MessageRecord { mod columns { pub const ACCOUNT: &str = "account"; pub const ROOM_ID: &str = "room_id"; - pub const STANZA_ID: &str = "stanza_id"; - pub const STANZA_ID_TARGET: &str = "stanza_id_target"; pub const MESSAGE_ID: &str = "message_id"; - pub const MESSAGE_ID_TARGET: &str = "message_id_target"; + pub const SERVER_ID: &str = "server_id"; + pub const SERVER_ID_TARGET: &str = "server_id_target"; + pub const REMOTE_ID: &str = "remote_id"; + pub const REMOTE_ID_TARGET: &str = "remote_id_target"; pub const TIMESTAMP: &str = "timestamp"; } define_entity!(MessageRecord, "messages", account_idx => { columns: [columns::ACCOUNT], unique: false }, room_idx => { columns: [columns::ACCOUNT, columns::ROOM_ID], unique: false }, - // Can't be unique, because stanzaId might be nil… - stanza_id_idx => { columns: [columns::ACCOUNT, columns::ROOM_ID, columns::STANZA_ID], unique: false }, - stanza_id_target_idx => { columns: [columns::ACCOUNT, columns::ROOM_ID, columns::STANZA_ID_TARGET], unique: false }, message_id_idx => { columns: [columns::ACCOUNT, columns::ROOM_ID, columns::MESSAGE_ID], unique: true }, - message_id_target_idx => { columns: [columns::ACCOUNT, columns::ROOM_ID, columns::MESSAGE_ID_TARGET], unique: false }, + // Can't be unique, because stanzaId might be nil… + server_id_idx => { columns: [columns::ACCOUNT, columns::ROOM_ID, columns::SERVER_ID], unique: false }, + server_id_target_idx => { columns: [columns::ACCOUNT, columns::ROOM_ID, columns::SERVER_ID_TARGET], unique: false }, + // Can't be unique, because remote ids are not guaranteed to be unique… + remote_id_idx => { columns: [columns::ACCOUNT, columns::ROOM_ID, columns::REMOTE_ID], unique: false }, + remote_id_target_idx => { columns: [columns::ACCOUNT, columns::ROOM_ID, columns::REMOTE_ID_TARGET], unique: false }, timestamp_idx => { columns: [columns::ACCOUNT, columns::ROOM_ID, columns::TIMESTAMP], unique: false } ); -impl KeyType for MessageLikeId { +impl KeyType for MessageId { fn to_raw_key(&self) -> RawKey { RawKey::Text(self.to_string()) } @@ -84,9 +88,10 @@ impl MessageRecord { room_id, id, message_id: value.id, - message_id_target, - stanza_id: value.stanza_id, - stanza_id_target, + remote_id: value.remote_id, + remote_id_target: message_id_target, + server_id: value.server_id, + server_id_target: stanza_id_target, to: value.to, from: value.from, timestamp: value.timestamp, @@ -97,7 +102,7 @@ impl MessageRecord { impl From for MessageLike { fn from(value: MessageRecord) -> Self { - let target = match (value.stanza_id_target, value.message_id_target) { + let target = match (value.server_id_target, value.remote_id_target) { (Some(id), _) => Some(MessageTargetId::ServerId(id)), (_, Some(id)) => Some(MessageTargetId::RemoteId(id)), (None, None) => None, @@ -105,7 +110,8 @@ impl From for MessageLike { Self { id: value.message_id, - stanza_id: value.stanza_id, + remote_id: value.remote_id, + server_id: value.server_id, target, to: value.to, from: value.from, diff --git a/crates/prose-core-client/src/infra/platform_dependencies.rs b/crates/prose-core-client/src/infra/platform_dependencies.rs index 084e6ce3..7d957f54 100644 --- a/crates/prose-core-client/src/infra/platform_dependencies.rs +++ b/crates/prose-core-client/src/infra/platform_dependencies.rs @@ -10,8 +10,8 @@ use prose_store::prelude::*; use crate::app::deps::{ AppContext, AppDependencies, DynAvatarRepository, DynClientEventDispatcher, - DynEncryptionService, DynIDProvider, DynRngProvider, DynServerEventHandlerQueue, - DynTimeProvider, DynUserDeviceIdProvider, + DynEncryptionService, DynIDProvider, DynMessageIdProvider, DynRngProvider, + DynServerEventHandlerQueue, DynTimeProvider, DynUserDeviceIdProvider, }; use crate::app::services::RoomInner; use crate::domain::contacts::services::impls::{ @@ -61,6 +61,7 @@ pub(crate) struct PlatformDependencies { pub ctx: AppContext, pub encryption_service: DynEncryptionService, pub id_provider: DynIDProvider, + pub message_id_provider: DynMessageIdProvider, pub rng_provider: DynRngProvider, pub server_event_handler_queue: DynServerEventHandlerQueue, pub short_id_provider: DynIDProvider, @@ -70,7 +71,7 @@ pub(crate) struct PlatformDependencies { pub xmpp: Arc, } -const DB_VERSION: u32 = 29; +const DB_VERSION: u32 = 30; pub async fn open_store(driver: D) -> Result, D::Error> { let versions_changed = Arc::new(AtomicBool::new(false)); @@ -202,6 +203,11 @@ pub async fn open_store(driver: D) -> Result, D::Error> { create_collection::(&tx)?; } + if event.old_version < 30 { + tx.delete_collection(MessageRecord::collection())?; + create_collection::(&tx)?; + } + Ok(()) }) .await?; @@ -226,6 +232,7 @@ impl From for AppDependencies { let ctx = Arc::new(d.ctx); let drafts_repo = Arc::new(DraftsRepository::new(d.store.clone())); let id_provider = d.id_provider; + let message_id_provider = d.message_id_provider; let messages_repo = Arc::new(CachingMessageRepository::new(d.store.clone())); let time_provider = d.time_provider; let user_device_repo = Arc::new(CachingUserDeviceRepository::new( @@ -282,6 +289,7 @@ impl From for AppDependencies { encryption_domain_service: encryption_domain_service.clone(), local_room_settings_repo: local_room_settings_repo.clone(), message_archive_service: d.xmpp.clone(), + message_id_provider: message_id_provider.clone(), message_repo: messages_repo.clone(), time_provider: time_provider.clone(), }; @@ -350,7 +358,7 @@ impl From for AppDependencies { let ctx = ctx.clone(); let drafts_repo = drafts_repo.clone(); let encryption_domain_service = encryption_domain_service.clone(); - let id_provider = id_provider.clone(); + let message_id_provider = message_id_provider.clone(); let message_repo = messages_repo.clone(); let sidebar_domain_service = sidebar_domain_service.clone(); let time_provider = time_provider.clone(); @@ -365,7 +373,7 @@ impl From for AppDependencies { data: data.clone(), drafts_repo: drafts_repo.clone(), encryption_domain_service: encryption_domain_service.clone(), - id_provider: id_provider.clone(), + message_id_provider: message_id_provider.clone(), message_archive_service: xmpp.clone(), message_repo: message_repo.clone(), messaging_service: xmpp.clone(), @@ -391,6 +399,7 @@ impl From for AppDependencies { drafts_repo, encryption_domain_service, id_provider, + message_id_provider, local_room_settings_repo, message_archive_service: d.xmpp.clone(), messages_repo, diff --git a/crates/prose-core-client/src/infra/xmpp/util/message_ext.rs b/crates/prose-core-client/src/infra/xmpp/util/message_ext.rs index 1aa551ea..bb8a0878 100644 --- a/crates/prose-core-client/src/infra/xmpp/util/message_ext.rs +++ b/crates/prose-core-client/src/infra/xmpp/util/message_ext.rs @@ -14,6 +14,8 @@ use prose_xmpp::stanza::Message; use crate::domain::messaging::models::send_message_request::{Body, Payload}; use crate::domain::messaging::models::Attachment; +use crate::domain::shared::models::UserEndpointId; +use crate::dtos::{MessageServerId, RoomId}; pub trait MessageExt { /// Returns unique attachments. Either SIMS or OOB. @@ -28,6 +30,17 @@ pub trait MessageExt { fn is_groupchat_message(&self) -> bool; fn set_message_body(self, body: Option) -> Self; fn set_omemo_payload(self, payload: impl Into) -> Self; + + /// Returns the value of the `from` attribute converted to a `UserEndpointId`, depending on + /// the message type (groupchat or chat). + fn sender(&self) -> Option; + + /// Returns the value of the `to` attribute converted to a `RoomId`, depending on the + /// message type (groupchat or chat) + fn room_id(&self) -> Option; + + /// Returns the value of the `stanza-id` element converted to a `MessageServerId`. + fn server_id(&self) -> Option; } impl MessageExt for Message { @@ -124,6 +137,43 @@ impl MessageExt for Message { ); self } + + fn sender(&self) -> Option { + let Some(from) = self.from.clone() else { + return None; + }; + + if self.is_groupchat_message() { + let Ok(from) = from.try_into_full() else { + error!("Expected FullJid in received groupchat message"); + return None; + }; + UserEndpointId::Occupant(from.into()) + } else { + match from.try_into_full() { + Ok(full) => UserEndpointId::UserResource(full.into()), + Err(bare) => UserEndpointId::User(bare.into()), + } + } + .into() + } + + fn room_id(&self) -> Option { + let Some(to) = self.to.clone() else { + return None; + }; + + if self.is_groupchat_message() { + Some(RoomId::Muc(to.into_bare().into())) + } else { + Some(RoomId::User(to.into_bare().into())) + } + } + + fn server_id(&self) -> Option { + self.stanza_id() + .map(|sid| MessageServerId::from(sid.id.as_ref())) + } } #[cfg(test)] diff --git a/crates/prose-core-client/src/test/message_builder.rs b/crates/prose-core-client/src/test/message_builder.rs index 7a1737b5..e4ff94f4 100644 --- a/crates/prose-core-client/src/test/message_builder.rs +++ b/crates/prose-core-client/src/test/message_builder.rs @@ -3,8 +3,6 @@ // Copyright: 2023, Marc Bauer // License: Mozilla Public License v2.0 (MPL v2.0) -use std::str::FromStr; - use chrono::{DateTime, Duration, Utc}; use jid::BareJid; use minidom::Element; @@ -18,8 +16,8 @@ use prose_xmpp::stanza::message::{Forwarded, MucUser, Reactions}; use prose_xmpp::test::BareJidTestAdditions; use crate::domain::messaging::models::{ - Body, Message, MessageLike, MessageLikeBody, MessageLikeId, MessageLikePayload, - MessageRemoteId, MessageServerId, Reaction, + Body, Message, MessageId, MessageLike, MessageLikeBody, MessageLikePayload, MessageRemoteId, + MessageServerId, Reaction, }; use crate::domain::shared::models::AnonOccupantId; use crate::dtos::{ @@ -27,23 +25,15 @@ use crate::dtos::{ }; use crate::test::mock_data; -impl From for MessageLikeId -where - T: Into, -{ - fn from(s: T) -> MessageLikeId { - MessageLikeId::from_str(&s.into()).unwrap() - } -} - pub struct MessageBuilder { - id: MessageRemoteId, + id: MessageId, + remote_id: Option, stanza_id: Option, from: ParticipantId, from_anon: Option, from_name: Option, to: BareJid, - target_message_idx: Option, + target_message_id: Option, payload: MessageLikePayload, timestamp: DateTime, is_read: bool, @@ -53,7 +43,11 @@ pub struct MessageBuilder { } impl MessageBuilder { - pub fn id_for_index(idx: u32) -> MessageRemoteId { + pub fn id_for_index(idx: u32) -> MessageId { + format!("msg-{}", idx).into() + } + + pub fn remote_id_for_index(idx: u32) -> MessageRemoteId { format!("msg-{}", idx).into() } @@ -78,6 +72,15 @@ impl MessageLikePayload { } } +impl From for MessageLikePayload +where + T: Into, +{ + fn from(value: T) -> Self { + MessageLikePayload::message(value) + } +} + impl MessageLikeBody { pub fn text(body: impl Into) -> Self { let body = body.into(); @@ -101,22 +104,24 @@ impl MessageBuilder { mock_data::reference_date() + Duration::minutes(idx.into()), MessageLikePayload::message(format!("Message {}", idx)), ) - .set_stanza_id(Some(Self::stanza_id_for_index(idx))) + .set_remote_id(Some(Self::remote_id_for_index(idx))) + .set_server_id(Some(Self::stanza_id_for_index(idx))) } pub fn new_with_id( - id: impl Into, + id: impl Into, timestamp: DateTime, payload: MessageLikePayload, ) -> Self { MessageBuilder { id: id.into(), + remote_id: None, stanza_id: None, from: ParticipantId::User(BareJid::ours().into()), from_anon: None, from_name: None, to: BareJid::theirs(), - target_message_idx: None, + target_message_id: None, payload, timestamp, is_read: false, @@ -126,7 +131,17 @@ impl MessageBuilder { } } - pub fn set_stanza_id(mut self, stanza_id: Option) -> Self { + pub fn set_id(mut self, id: impl Into) -> Self { + self.id = id.into(); + self + } + + pub fn set_remote_id(mut self, remote_id: Option) -> Self { + self.remote_id = remote_id; + self + } + + pub fn set_server_id(mut self, stanza_id: Option) -> Self { self.stanza_id = stanza_id; self } @@ -151,13 +166,13 @@ impl MessageBuilder { self } - pub fn set_payload(mut self, payload: MessageLikePayload) -> Self { - self.payload = payload; + pub fn set_payload(mut self, payload: impl Into) -> Self { + self.payload = payload.into(); self } pub fn set_target_message_idx(mut self, idx: u32) -> Self { - self.target_message_idx = Some(idx); + self.target_message_id = Some(Self::remote_id_for_index(idx)); self } @@ -174,7 +189,8 @@ impl MessageBuilder { }; Message { - remote_id: Some(self.id), + id: self.id, + remote_id: self.remote_id, server_id: self.stanza_id, from: self.from, body: Body { @@ -199,8 +215,7 @@ impl MessageBuilder { }; MessageDTO { - id: Some(self.id), - stanza_id: self.stanza_id, + id: self.id, from: MessageSender { id: self.from, name: self @@ -245,11 +260,10 @@ impl MessageBuilder { pub fn build_message_like(self) -> MessageLike { MessageLike { - id: MessageLikeId::new(Some(self.id)), - stanza_id: self.stanza_id, - target: self - .target_message_idx - .map(|idx| Self::id_for_index(idx).into()), + id: self.id, + remote_id: self.remote_id, + server_id: self.stanza_id, + target: self.target_message_id.map(Into::into), to: Some(self.to), from: self.from, timestamp: self.timestamp, @@ -309,7 +323,12 @@ impl MessageBuilder { pub fn build_message_stanza(self) -> prose_xmpp::stanza::Message { let mut message = prose_xmpp::stanza::Message::new() - .set_id(self.id.as_ref().into()) + .set_id( + self.remote_id + .map(|id| id.to_string()) + .unwrap_or_else(|| self.id.to_string()) + .into(), + ) .set_type(match self.from { ParticipantId::User(_) => MessageType::Chat, ParticipantId::Occupant(_) => MessageType::Groupchat, @@ -330,11 +349,11 @@ impl MessageBuilder { MessageLikePayload::Message { body, .. } => message = message.set_body(body.raw), MessageLikePayload::Reaction { emojis } => { message = message.set_message_reactions(Reactions { - id: Self::id_for_index( - self.target_message_idx.expect("Missing target_message_idx"), - ) - .as_ref() - .into(), + id: self + .target_message_id + .expect("Missing target_message_idx") + .as_ref() + .into(), reactions: emojis.into_iter().map(Into::into).collect(), }) } diff --git a/crates/prose-core-client/src/test/mock_app_dependencies.rs b/crates/prose-core-client/src/test/mock_app_dependencies.rs index 6d1bc3bc..79b79989 100644 --- a/crates/prose-core-client/src/test/mock_app_dependencies.rs +++ b/crates/prose-core-client/src/test/mock_app_dependencies.rs @@ -16,9 +16,9 @@ use prose_xmpp::test::IncrementingIDProvider; use crate::app::deps::{ AppContext, AppDependencies, DynAppContext, DynBookmarksService, DynClientEventDispatcher, DynDraftsRepository, DynEncryptionDomainService, DynIDProvider, DynMessageArchiveService, - DynMessagesRepository, DynMessagingService, DynRngProvider, DynRoomAttributesService, - DynRoomParticipationService, DynSidebarDomainService, DynSyncedRoomSettingsService, - DynTimeProvider, DynUserInfoDomainService, + DynMessageIdProvider, DynMessagesRepository, DynMessagingService, DynRngProvider, + DynRoomAttributesService, DynRoomParticipationService, DynSidebarDomainService, + DynSyncedRoomSettingsService, DynTimeProvider, DynUserInfoDomainService, }; use crate::app::event_handlers::{MockClientEventDispatcherTrait, ServerEventHandlerQueue}; use crate::app::services::RoomInner; @@ -40,6 +40,7 @@ use crate::domain::messaging::services::mocks::{ MockMessageArchiveDomainService, MockMessageArchiveService, MockMessageMigrationDomainService, MockMessagingService, }; +use crate::domain::messaging::services::WrappingMessageIdProvider; use crate::domain::rooms::repos::mocks::{ MockConnectedRoomsReadOnlyRepository, MockConnectedRoomsReadWriteRepository, }; @@ -127,6 +128,10 @@ pub struct MockAppDependencies { pub id_provider: DynIDProvider, pub local_room_settings_repo: MockLocalRoomSettingsRepository, pub message_archive_service: MockMessageArchiveService, + #[derivative(Default( + value = "Arc::new(WrappingMessageIdProvider::incrementing(\"msg-id\"))" + ))] + pub message_id_provider: DynMessageIdProvider, pub messages_repo: MockMessagesRepository, pub messaging_service: MockMessagingService, pub offline_message_repo: MockOfflineMessagesRepository, @@ -177,7 +182,7 @@ impl From for AppDependencies { let ctx = ctx.clone(); let drafts_repo = drafts_repo.clone(); let encryption_domain_service = encryption_domain_service.clone(); - let id_provider = mock.id_provider.clone(); + let message_id_provider = mock.message_id_provider.clone(); let message_archive_service = message_archive_service.clone(); let message_repo = messages_repo.clone(); let messaging_service = messaging_service.clone(); @@ -196,7 +201,7 @@ impl From for AppDependencies { data: data.clone(), drafts_repo: drafts_repo.clone(), encryption_domain_service: encryption_domain_service.clone(), - id_provider: id_provider.clone(), + message_id_provider: message_id_provider.clone(), message_archive_service: message_archive_service.clone(), message_repo: message_repo.clone(), messaging_service: messaging_service.clone(), @@ -221,6 +226,7 @@ impl From for AppDependencies { drafts_repo, encryption_domain_service, id_provider: mock.id_provider, + message_id_provider: mock.message_id_provider, local_room_settings_repo: Arc::new(mock.local_room_settings_repo), message_archive_service, messages_repo, @@ -364,8 +370,10 @@ pub struct MockRoomFactoryDependencies { pub ctx: AppContext, pub drafts_repo: MockDraftsRepository, pub encryption_domain_service: MockEncryptionDomainService, - #[derivative(Default(value = "Arc::new(IncrementingIDProvider::new(Default::default()))"))] - pub id_provider: DynIDProvider, + #[derivative(Default( + value = "Arc::new(WrappingMessageIdProvider::incrementing(\"msg-id\"))" + ))] + pub message_id_provider: DynMessageIdProvider, pub message_archive_service: MockMessageArchiveService, pub message_repo: MockMessagesRepository, pub messaging_service: MockMessagingService, @@ -383,7 +391,7 @@ pub struct MockSealedRoomFactoryDependencies { pub ctx: DynAppContext, pub drafts_repo: DynDraftsRepository, pub encryption_domain_service: DynEncryptionDomainService, - pub id_provider: DynIDProvider, + pub message_id_provider: DynMessageIdProvider, pub message_archive_service: DynMessageArchiveService, pub message_repo: DynMessagesRepository, pub messaging_service: DynMessagingService, @@ -403,7 +411,7 @@ impl From for MockSealedRoomFactoryDependencies { ctx: Arc::new(value.ctx), drafts_repo: Arc::new(value.drafts_repo), encryption_domain_service: Arc::new(value.encryption_domain_service), - id_provider: value.id_provider, + message_id_provider: value.message_id_provider, message_archive_service: Arc::new(value.message_archive_service), message_repo: Arc::new(value.message_repo), messaging_service: Arc::new(value.messaging_service), @@ -427,7 +435,7 @@ impl From for RoomFactory { data: data.clone(), drafts_repo: value.drafts_repo.clone(), encryption_domain_service: value.encryption_domain_service.clone(), - id_provider: value.id_provider.clone(), + message_id_provider: value.message_id_provider.clone(), message_archive_service: value.message_archive_service.clone(), message_repo: value.message_repo.clone(), messaging_service: value.messaging_service.clone(), diff --git a/crates/prose-core-client/tests/message_parsing/message_parser.rs b/crates/prose-core-client/tests/message_parsing/message_parser.rs index 912b8847..c75d5502 100644 --- a/crates/prose-core-client/tests/message_parsing/message_parser.rs +++ b/crates/prose-core-client/tests/message_parsing/message_parser.rs @@ -49,6 +49,7 @@ async fn test_parse_chat_message() -> Result<()> { .add_references([reference]); let parsed_message = MessageParser::new( + "local-id-1".into(), None, Default::default(), Arc::new(MockEncryptionDomainService::new()), @@ -59,8 +60,9 @@ async fn test_parse_chat_message() -> Result<()> { assert_eq!( MessageLike { - id: "message-id-1".into(), - stanza_id: None, + id: "local-id-1".into(), + remote_id: Some("message-id-1".into()), + server_id: None, target: None, to: Some(bare!("me@prose.org")), from: ParticipantId::User(user_id!("them@prose.org")), @@ -99,6 +101,7 @@ async fn test_parse_groupchat_message() -> Result<()> { }); let parsed_message = MessageParser::new( + "local-id-1".into(), None, Default::default(), Arc::new(MockEncryptionDomainService::new()), @@ -109,8 +112,9 @@ async fn test_parse_groupchat_message() -> Result<()> { assert_eq!( MessageLike { - id: "message-id-1".into(), - stanza_id: Some("dekEV-gtF2hrg_iekCjPAlON".into()), + id: "local-id-1".into(), + remote_id: Some("message-id-1".into()), + server_id: Some("dekEV-gtF2hrg_iekCjPAlON".into()), target: None, to: Some(bare!("me@prose.org")), from: ParticipantId::Occupant(occupant_id!("room@groups.prose.org/them")), @@ -152,6 +156,7 @@ async fn test_parse_sent_carbon_message() -> Result<()> { }); let parsed_message = MessageParser::new( + "local-id-1".into(), None, Default::default(), Arc::new(MockEncryptionDomainService::new()), @@ -162,8 +167,9 @@ async fn test_parse_sent_carbon_message() -> Result<()> { assert_eq!( MessageLike { - id: "message-id-1".into(), - stanza_id: Some("Qiuahv1eo3C222uKhOqjPiW0".into()), + id: "local-id-1".into(), + remote_id: Some("message-id-1".into()), + server_id: Some("Qiuahv1eo3C222uKhOqjPiW0".into()), target: None, to: Some(bare!("them@prose.org")), from: ParticipantId::User(user_id!("me@prose.org")), @@ -208,6 +214,7 @@ async fn test_parse_mam_groupchat_message() -> Result<()> { }; let parsed_message = MessageParser::new( + "local-id-1".into(), None, Default::default(), Arc::new(MockEncryptionDomainService::new()), @@ -218,8 +225,9 @@ async fn test_parse_mam_groupchat_message() -> Result<()> { assert_eq!( MessageLike { - id: "message-id-1".into(), - stanza_id: Some("FbGQI-iEUNysr8pdD2PP9mmU".into()), + id: "local-id-1".into(), + remote_id: Some("message-id-1".into()), + server_id: Some("FbGQI-iEUNysr8pdD2PP9mmU".into()), target: None, to: Some(bare!("me@prose.org")), from: ParticipantId::Occupant(occupant_id!("room@groups.prose.org/them")), @@ -269,6 +277,7 @@ async fn test_parse_mam_groupchat_message_with_real_jid() -> Result<()> { }; let parsed_message = MessageParser::new( + "local-id-1".into(), None, Default::default(), Arc::new(MockEncryptionDomainService::new()), @@ -280,8 +289,9 @@ async fn test_parse_mam_groupchat_message_with_real_jid() -> Result<()> { assert_eq!( parsed_message, MessageLike { - id: "message-id-1".into(), - stanza_id: Some("FbGQI-iEUNysr8pdD2PP9mmU".into()), + id: "local-id-1".into(), + remote_id: Some("message-id-1".into()), + server_id: Some("FbGQI-iEUNysr8pdD2PP9mmU".into()), target: None, to: Some(bare!("me@prose.org")), from: ParticipantId::User(user_id!("them@prose.org")), @@ -325,6 +335,7 @@ async fn test_parse_mam_chat_message() -> Result<()> { }; let parsed_message = MessageParser::new( + "local-id-1".into(), None, Default::default(), Arc::new(MockEncryptionDomainService::new()), @@ -335,8 +346,9 @@ async fn test_parse_mam_chat_message() -> Result<()> { assert_eq!( MessageLike { - id: "message-id-1".into(), - stanza_id: Some("bne6LtG1ev_jIb1oYNA7nxip".into()), + id: "local-id-1".into(), + remote_id: Some("message-id-1".into()), + server_id: Some("bne6LtG1ev_jIb1oYNA7nxip".into()), target: None, to: Some(bare!("me@prose.org")), from: ParticipantId::User(user_id!("them@prose.org")), @@ -377,6 +389,7 @@ async fn test_parse_delayed_message() -> Result<()> { .set_body("Hello"); let parsed_message = MessageParser::new( + "local-id-1".into(), None, Default::default(), Arc::new(MockEncryptionDomainService::new()), @@ -387,8 +400,9 @@ async fn test_parse_delayed_message() -> Result<()> { assert_eq!( MessageLike { - id: "message-id-1".into(), - stanza_id: None, + id: "local-id-1".into(), + remote_id: Some("message-id-1".into()), + server_id: None, target: None, to: Some(bare!("me@prose.org")), from: ParticipantId::User(user_id!("them@prose.org")), @@ -422,6 +436,7 @@ async fn test_message_with_attachment_and_empty_body() -> Result<()> { }]); let parsed_message = MessageParser::new( + "local-id-1".into(), None, Default::default(), Arc::new(MockEncryptionDomainService::new()), @@ -432,8 +447,9 @@ async fn test_message_with_attachment_and_empty_body() -> Result<()> { assert_eq!( MessageLike { - id: "message-id-1".into(), - stanza_id: None, + id: "local-id-1".into(), + remote_id: Some("message-id-1".into()), + server_id: None, target: None, to: Some(bare!("me@prose.org")), from: ParticipantId::User(user_id!("them@prose.org")), diff --git a/crates/prose-core-client/tests/messages_event_handler.rs b/crates/prose-core-client/tests/messages_event_handler.rs index cfd3575e..cf9dac8e 100644 --- a/crates/prose-core-client/tests/messages_event_handler.rs +++ b/crates/prose-core-client/tests/messages_event_handler.rs @@ -18,14 +18,19 @@ use prose_core_client::domain::connection::models::ConnectionProperties; use prose_core_client::domain::messaging::models::{ MessageLike, MessageLikeBody, MessageLikePayload, }; +use prose_core_client::domain::messaging::services::WrappingMessageIdProvider; use prose_core_client::domain::rooms::models::{Room, RoomInfo}; use prose_core_client::domain::shared::models::{ MucId, OccupantId, RoomId, RoomType, UserId, UserResourceId, }; -use prose_core_client::dtos::{Availability, MessageRemoteId, MessageServerId, ParticipantId}; +use prose_core_client::dtos::{ + Availability, MessageId, MessageRemoteId, MessageServerId, ParticipantId, +}; +use prose_core_client::test::mock_data::account_jid; use prose_core_client::test::{ConstantTimeProvider, MockAppDependencies}; use prose_core_client::{muc_id, occupant_id, user_id, user_resource_id, ClientRoomEventType}; use prose_xmpp::mods::chat::Carbon; +use prose_xmpp::stanza::message::stanza_id::StanzaId; use prose_xmpp::stanza::message::{Forwarded, Reactions}; use prose_xmpp::stanza::muc::MucUser; use prose_xmpp::stanza::Message; @@ -34,10 +39,22 @@ use prose_xmpp::{bare, full, jid}; #[tokio::test] async fn test_receiving_message_adds_item_to_sidebar_if_needed() -> Result<()> { let mut deps = MockAppDependencies::default(); + deps.message_id_provider = Arc::new(WrappingMessageIdProvider::incrementing("msg-id")); let mut seq = Sequence::new(); let room = Room::group(muc_id!("group@conference.prose.org")).with_name("Group Name"); + deps.messages_repo + .expect_contains() + .once() + .in_sequence(&mut seq) + .with( + predicate::always(), + predicate::always(), + predicate::eq(MessageServerId::from("message-id")), + ) + .return_once(|_, _, _| Box::pin(async { Ok(false) })); + { let room = room.clone(); deps.connected_rooms_repo @@ -76,17 +93,6 @@ async fn test_receiving_message_adds_item_to_sidebar_if_needed() -> Result<()> { ) .return_once(|_, _| Box::pin(async { Ok(()) })); - deps.messages_repo - .expect_contains() - .once() - .in_sequence(&mut seq) - .with( - predicate::always(), - predicate::always(), - predicate::eq(MessageRemoteId::from("message-id")), - ) - .return_once(|_, _, _| Box::pin(async { Ok(false) })); - deps.messages_repo .expect_append() .once() @@ -100,7 +106,7 @@ async fn test_receiving_message_adds_item_to_sidebar_if_needed() -> Result<()> { .with( predicate::eq(room), predicate::eq(ClientRoomEventType::MessagesAppended { - message_ids: vec!["message-id".into()], + message_ids: vec!["msg-id-1".into()], }), ) .return_once(|_, _| ()); @@ -111,7 +117,11 @@ async fn test_receiving_message_adds_item_to_sidebar_if_needed() -> Result<()> { r#type: MessageEventType::Received( Message::default() .set_type(MessageType::Groupchat) - .set_id("message-id".into()) + .set_to(bare!("group@conference.prose.org")) + .set_stanza_id(StanzaId { + id: "message-id".into(), + by: bare!("group@conference.prose.org").into(), + }) .set_from(jid!("group@conference.prose.org/user")) .set_body("Hello World"), ), @@ -124,6 +134,7 @@ async fn test_receiving_message_adds_item_to_sidebar_if_needed() -> Result<()> { #[tokio::test] async fn test_receiving_message_from_new_contact_creates_room() -> Result<()> { let mut deps = MockAppDependencies::default(); + deps.message_id_provider = Arc::new(WrappingMessageIdProvider::incrementing("msg-id")); let mut seq = Sequence::new(); let room = Room::direct_message(user_id!("jane.doe@prose.org"), Availability::Unavailable); @@ -159,7 +170,7 @@ async fn test_receiving_message_from_new_contact_creates_room() -> Result<()> { .with( predicate::always(), predicate::always(), - predicate::eq(MessageRemoteId::from("message-id")), + predicate::eq(MessageServerId::from("message-id")), ) .return_once(|_, _, _| Box::pin(async { Ok(false) })); @@ -176,7 +187,7 @@ async fn test_receiving_message_from_new_contact_creates_room() -> Result<()> { .with( predicate::eq(room), predicate::eq(ClientRoomEventType::MessagesAppended { - message_ids: vec!["message-id".into()], + message_ids: vec!["msg-id-1".into()], }), ) .return_once(|_, _| ()); @@ -187,7 +198,11 @@ async fn test_receiving_message_from_new_contact_creates_room() -> Result<()> { r#type: MessageEventType::Received( Message::default() .set_type(MessageType::Chat) - .set_id("message-id".into()) + .set_to(account_jid()) + .set_stanza_id(StanzaId { + id: "message-id".into(), + by: bare!("jane.doe@prose.org").into(), + }) .set_from(jid!("jane.doe@prose.org")) .set_body("Hello World"), ), @@ -200,6 +215,7 @@ async fn test_receiving_message_from_new_contact_creates_room() -> Result<()> { #[tokio::test] async fn test_parses_user_id_from_in_sent_groupchat_message() -> Result<()> { let mut deps = MockAppDependencies::default(); + deps.message_id_provider = Arc::new(WrappingMessageIdProvider::incrementing("msg-id")); let mut seq = Sequence::new(); *deps.ctx.connection_properties.write() = Some(ConnectionProperties { @@ -212,9 +228,13 @@ async fn test_parses_user_id_from_in_sent_groupchat_message() -> Result<()> { let room = Room::group(muc_id!("room@conference.prose.org")); - let sent_message = prose_xmpp::stanza::Message::new() + let sent_message = Message::new() .set_type(MessageType::Groupchat) .set_id("message-id".into()) + .set_stanza_id(StanzaId { + id: "stanza-id".into(), + by: bare!("room@conference.prose.org").into(), + }) .set_from(full!("from@prose.org/res")) .set_to(bare!("room@conference.prose.org")) .set_body("Hello World") @@ -222,8 +242,9 @@ async fn test_parses_user_id_from_in_sent_groupchat_message() -> Result<()> { .set_markable(); let expected_saved_message = MessageLike { - id: "message-id".into(), - stanza_id: None, + id: "msg-id-1".into(), + remote_id: Some("message-id".into()), + server_id: Some("stanza-id".into()), target: None, to: Some(bare!("room@conference.prose.org")), from: ParticipantId::User(user_id!("from@prose.org")), // Resource should be dropped @@ -260,11 +281,21 @@ async fn test_parses_user_id_from_in_sent_groupchat_message() -> Result<()> { .with( predicate::always(), predicate::always(), - predicate::eq(MessageRemoteId::from("message-id")), + predicate::eq(MessageServerId::from("stanza-id")), ) .once() .return_once(|_, _, _| Box::pin(async { Ok(false) })); + deps.messages_repo + .expect_resolve_remote_id_to_message_id() + .with( + predicate::always(), + predicate::always(), + predicate::eq(MessageRemoteId::from("message-id")), + ) + .once() + .return_once(|_, _, _| Box::pin(async { Ok(None) })); + deps.messages_repo .expect_append() .once() @@ -283,7 +314,7 @@ async fn test_parses_user_id_from_in_sent_groupchat_message() -> Result<()> { .with( predicate::eq(room), predicate::eq(ClientRoomEventType::MessagesAppended { - message_ids: vec!["message-id".into()], + message_ids: vec!["msg-id-1".into()], }), ) .return_once(|_, _| ()); @@ -302,6 +333,7 @@ async fn test_parses_user_id_from_in_sent_groupchat_message() -> Result<()> { async fn test_parses_private_message_in_muc_room() -> Result<()> { let mut deps = MockAppDependencies::default(); let mut seq = Sequence::new(); + deps.message_id_provider = Arc::new(WrappingMessageIdProvider::incrementing("msg-id")); *deps.ctx.connection_properties.write() = Some(ConnectionProperties { connection_timestamp: Default::default(), @@ -313,17 +345,22 @@ async fn test_parses_private_message_in_muc_room() -> Result<()> { let room = Room::group(muc_id!("room@conference.prose.org")); - let received_message = prose_xmpp::stanza::Message::new() + let received_message = Message::new() .set_type(MessageType::Chat) .set_id("message-id".into()) + .set_stanza_id(StanzaId { + id: "stanza-id".into(), + by: bare!("room@conference.prose.org").into(), + }) .set_from(full!("room@conference.prose.org/other-user")) .set_to(full!("user@prose.org/res")) .set_body("Private Message") .add_payload(MucUser::new()); let expected_saved_message = MessageLike { - id: "message-id".into(), - stanza_id: None, + id: "msg-id-1".into(), + remote_id: Some("message-id".into()), + server_id: Some("stanza-id".into()), target: None, to: Some(bare!("user@prose.org")), from: ParticipantId::Occupant(occupant_id!("room@conference.prose.org/other-user")), @@ -384,7 +421,7 @@ async fn test_parses_private_message_in_muc_room() -> Result<()> { .with( predicate::eq(room), predicate::eq(ClientRoomEventType::MessagesAppended { - message_ids: vec!["message-id".into()], + message_ids: vec!["msg-id-1".into()], }), ) .return_once(|_, _| ()); @@ -402,6 +439,7 @@ async fn test_parses_private_message_in_muc_room() -> Result<()> { #[tokio::test] async fn test_dispatches_messages_appended_for_new_received_message() -> Result<()> { let mut deps = MockAppDependencies::default(); + deps.message_id_provider = Arc::new(WrappingMessageIdProvider::incrementing("msg-id")); let room = Room::group(muc_id!("user@prose.org")); @@ -432,7 +470,7 @@ async fn test_dispatches_messages_appended_for_new_received_message() -> Result< .with( predicate::eq(room), predicate::eq(ClientRoomEventType::MessagesAppended { - message_ids: vec!["message-id".into()], + message_ids: vec!["msg-id-1".into()], }), ) .return_once(|_, _| ()); @@ -442,7 +480,11 @@ async fn test_dispatches_messages_appended_for_new_received_message() -> Result< .handle_event(ServerEvent::Message(MessageEvent { r#type: MessageEventType::Received( Message::default() - .set_id("message-id".into()) + .set_to(account_jid()) + .set_stanza_id(StanzaId { + id: "stanza-id".into(), + by: bare!("user@prose.org").into(), + }) .set_from(jid!("user@prose.org")) .set_body("Hello World"), ), @@ -455,6 +497,7 @@ async fn test_dispatches_messages_appended_for_new_received_message() -> Result< #[tokio::test] async fn test_dispatches_messages_appended_for_sent_carbon() -> Result<()> { let mut deps = MockAppDependencies::default(); + deps.message_id_provider = Arc::new(WrappingMessageIdProvider::incrementing("msg-id")); *deps.ctx.connection_properties.write() = Some(ConnectionProperties { connection_timestamp: Default::default(), @@ -481,11 +524,21 @@ async fn test_dispatches_messages_appended_for_sent_carbon() -> Result<()> { .with( predicate::always(), predicate::always(), - predicate::eq(MessageRemoteId::from("message-id")), + predicate::eq(MessageServerId::from("Qiuahv1eo3C222uKhOqjPiW0")), ) .once() .return_once(|_, _, _| Box::pin(async { Ok(false) })); + deps.messages_repo + .expect_resolve_remote_id_to_message_id() + .with( + predicate::always(), + predicate::always(), + predicate::eq(MessageRemoteId::from("message-id")), + ) + .once() + .return_once(|_, _, _| Box::pin(async { Ok(None) })); + deps.messages_repo .expect_append() .return_once(|_, _, _| Box::pin(async { Ok(()) })); @@ -496,7 +549,7 @@ async fn test_dispatches_messages_appended_for_sent_carbon() -> Result<()> { .with( predicate::eq(room), predicate::eq(ClientRoomEventType::MessagesAppended { - message_ids: vec!["message-id".into()], + message_ids: vec!["msg-id-1".into()], }), ) .return_once(|_, _| ()); @@ -514,7 +567,7 @@ async fn test_dispatches_messages_appended_for_sent_carbon() -> Result<()> { .set_to(bare!("user@prose.org")) .set_body("Hello World") .set_chat_state(Some(ChatState::Active)) - .set_stanza_id(prose_xmpp::stanza::message::stanza_id::StanzaId { + .set_stanza_id(StanzaId { id: "Qiuahv1eo3C222uKhOqjPiW0".into(), by: bare!("user@prose.org").into(), }), @@ -531,6 +584,7 @@ async fn test_dispatches_messages_appended_for_sent_carbon() -> Result<()> { #[tokio::test] async fn test_dispatches_messages_appended_for_muc_carbon() -> Result<()> { let mut deps = MockAppDependencies::default(); + deps.message_id_provider = Arc::new(WrappingMessageIdProvider::incrementing("msg-id")); let room = Room::mock(RoomInfo { room_id: RoomId::Muc(muc_id!("room@groups.prose.org")), @@ -558,11 +612,21 @@ async fn test_dispatches_messages_appended_for_muc_carbon() -> Result<()> { .with( predicate::always(), predicate::always(), - predicate::eq(MessageRemoteId::from("message-id")), + predicate::eq(MessageServerId::from("Qiuahv1eo3C222uKhOqjPiW0")), ) .once() .return_once(|_, _, _| Box::pin(async { Ok(false) })); + deps.messages_repo + .expect_resolve_remote_id_to_message_id() + .with( + predicate::always(), + predicate::always(), + predicate::eq(MessageRemoteId::from("message-id")), + ) + .once() + .return_once(|_, _, _| Box::pin(async { Ok(None) })); + deps.messages_repo .expect_append() .return_once(|_, _, _| Box::pin(async { Ok(()) })); @@ -573,7 +637,7 @@ async fn test_dispatches_messages_appended_for_muc_carbon() -> Result<()> { .with( predicate::eq(room), predicate::eq(ClientRoomEventType::MessagesAppended { - message_ids: vec!["message-id".into()], + message_ids: vec!["msg-id-1".into()], }), ) .return_once(|_, _| ()); @@ -605,11 +669,6 @@ async fn test_dispatches_messages_updated_for_existing_received_message() -> Res let room = Room::group(muc_id!("user@prose.org")); - deps.sidebar_domain_service - .expect_handle_received_message() - .once() - .return_once(|_, _| Box::pin(async { Ok(()) })); - { let room = room.clone(); deps.connected_rooms_repo @@ -626,17 +685,6 @@ async fn test_dispatches_messages_updated_for_existing_received_message() -> Res .expect_append() .return_once(|_, _, _| Box::pin(async { Ok(()) })); - deps.client_event_dispatcher - .expect_dispatch_room_event() - .once() - .with( - predicate::eq(room), - predicate::eq(ClientRoomEventType::MessagesUpdated { - message_ids: vec!["message-id".into()], - }), - ) - .return_once(|_, _| ()); - let event_handler = MessagesEventHandler::from(&deps.into_deps()); event_handler .handle_event(ServerEvent::Message(MessageEvent { @@ -644,6 +692,11 @@ async fn test_dispatches_messages_updated_for_existing_received_message() -> Res Message::default() .set_id("message-id".into()) .set_from(jid!("user@prose.org")) + .set_to(account_jid()) + .set_stanza_id(StanzaId { + id: "stanza-id".into(), + by: bare!("user@prose.org").into(), + }) .set_body("Hello World"), ), })) @@ -677,15 +730,13 @@ async fn test_looks_up_message_id_when_dispatching_message_event() -> Result<()> .return_once(|_, _, _| Box::pin(async { Ok(()) })); deps.messages_repo - .expect_resolve_message_id() + .expect_resolve_server_id_to_message_id() .with( predicate::always(), predicate::eq(RoomId::Muc(muc_id!("group@prose.org"))), predicate::eq(MessageServerId::from("stanza-id-100")), ) - .return_once(|_, _, _| { - Box::pin(async { Ok(Some(MessageRemoteId::from("message-id-100"))) }) - }); + .return_once(|_, _, _| Box::pin(async { Ok(Some(MessageId::from("message-id-100"))) })); deps.client_event_dispatcher .expect_dispatch_room_event() @@ -706,6 +757,11 @@ async fn test_looks_up_message_id_when_dispatching_message_event() -> Result<()> .set_id("message-id".into()) .set_type(MessageType::Groupchat) .set_from(jid!("group@prose.org/user")) + .set_to(bare!("group@prose.org")) + .set_stanza_id(StanzaId { + id: "stanza-id".into(), + by: bare!("group@prose.org").into(), + }) .set_message_reactions(Reactions { id: "stanza-id-100".to_string(), reactions: vec!["🙃".into()], @@ -737,26 +793,34 @@ async fn test_looks_up_message_id_for_sent_groupchat_messages_when_dispatching_m .with( predicate::always(), predicate::always(), - predicate::eq(MessageRemoteId::from("message-id")), + predicate::eq(MessageServerId::from("stanza-id")), ) .once() .return_once(|_, _, _| Box::pin(async { Ok(false) })); + deps.messages_repo + .expect_resolve_remote_id_to_message_id() + .with( + predicate::always(), + predicate::always(), + predicate::eq(MessageRemoteId::from("message-id")), + ) + .once() + .return_once(|_, _, _| Box::pin(async { Ok(None) })); + deps.messages_repo .expect_append() .once() .return_once(|_, _, _| Box::pin(async { Ok(()) })); deps.messages_repo - .expect_resolve_message_id() + .expect_resolve_server_id_to_message_id() .with( predicate::always(), predicate::eq(RoomId::Muc(muc_id!("group@prose.org"))), predicate::eq(MessageServerId::from("stanza-id-100")), ) - .return_once(|_, _, _| { - Box::pin(async { Ok(Some(MessageRemoteId::from("message-id-100"))) }) - }); + .return_once(|_, _, _| Box::pin(async { Ok(Some(MessageId::from("message-id-100"))) })); deps.client_event_dispatcher .expect_dispatch_room_event() @@ -775,6 +839,10 @@ async fn test_looks_up_message_id_for_sent_groupchat_messages_when_dispatching_m r#type: MessageEventType::Sent( Message::default() .set_id("message-id".into()) + .set_stanza_id(StanzaId { + id: "stanza-id".into(), + by: bare!("user@prose.org").into(), + }) .set_type(MessageType::Groupchat) .set_from(full!("from@prose.org/res")) .set_to(jid!("group@prose.org")) diff --git a/crates/prose-core-client/tests/room.rs b/crates/prose-core-client/tests/room.rs index 0df6b7d8..fe8d0a17 100644 --- a/crates/prose-core-client/tests/room.rs +++ b/crates/prose-core-client/tests/room.rs @@ -3,17 +3,17 @@ // Copyright: 2023, Marc Bauer // License: Mozilla Public License v2.0 (MPL v2.0) -use std::iter; - use anyhow::Result; use chrono::{TimeZone, Utc}; use mockall::predicate; use pretty_assertions::assert_eq; +use std::iter; +use std::sync::Arc; use prose_core_client::domain::messaging::models::{ MessageLikeBody, MessageLikePayload, MessageTargetId, Reaction, }; -use prose_core_client::domain::messaging::services::MessagePage; +use prose_core_client::domain::messaging::services::{MessagePage, WrappingMessageIdProvider}; use prose_core_client::domain::rooms::models::{RegisteredMember, Room, RoomAffiliation}; use prose_core_client::domain::rooms::services::RoomFactory; use prose_core_client::domain::shared::models::{CachePolicy, MucId, OccupantId, RoomId, UserId}; @@ -116,6 +116,7 @@ async fn test_load_messages_with_ids_resolves_real_jids() -> Result<()> { #[tokio::test] async fn test_load_latest_messages_resolves_real_jids() -> Result<()> { let mut deps = MockRoomFactoryDependencies::default(); + deps.message_id_provider = Arc::new(WrappingMessageIdProvider::incrementing("msg-id")); let internals = Room::group(muc_id!("room@conference.prose.org")) .with_members([RegisteredMember { @@ -187,6 +188,11 @@ async fn test_load_latest_messages_resolves_real_jids() -> Result<()> { }) }); + deps.message_repo + .expect_resolve_server_id_to_message_id() + .times(4) + .returning(|_, _, _| Box::pin(async { Ok(None) })); + deps.message_repo .expect_append() .once() @@ -195,28 +201,32 @@ async fn test_load_latest_messages_resolves_real_jids() -> Result<()> { let room = RoomFactory::from(deps).build(internals).to_generic_room(); assert_eq!( - room.load_latest_messages().await?, MessageResultSet { messages: vec![ MessageBuilder::new_with_index(1) + .set_id("msg-id-4") .set_from(user_id!("a@prose.org")) .set_from_name("Aron Doe") .build_message_dto(), MessageBuilder::new_with_index(2) + .set_id("msg-id-3") .set_from(occupant_id!("room@conference.prose.org/b")) .set_from_name("Bernhard Doe") .build_message_dto(), MessageBuilder::new_with_index(3) + .set_id("msg-id-2") .set_from(user_id!("c@prose.org")) .set_from_name("Carl Doe") .build_message_dto(), MessageBuilder::new_with_index(4) + .set_id("msg-id-1") .set_from(occupant_id!("room@conference.prose.org/denise_doe")) .set_from_name("Denise Doe") .build_message_dto(), ], last_message_id: None - } + }, + room.load_latest_messages().await?, ); Ok(()) @@ -261,7 +271,7 @@ async fn test_toggle_reaction_in_direct_message() -> Result<()> { .once() .with( predicate::eq(user_id!("user@prose.org")), - predicate::eq(MessageBuilder::id_for_index(1)), + predicate::eq(MessageBuilder::remote_id_for_index(1)), predicate::eq(vec!["🍻".into(), "✅".into()]), ) .return_once(|_, _, _| Box::pin(async { Ok(()) })); @@ -360,6 +370,7 @@ async fn test_renames_channel_in_sidebar() -> Result<()> { #[tokio::test] async fn test_fills_result_set_when_loading_messages() -> Result<()> { let mut deps = MockRoomFactoryDependencies::default(); + deps.message_id_provider = Arc::new(WrappingMessageIdProvider::incrementing("msg-id")); deps.ctx.config.message_page_size = 5; @@ -481,6 +492,11 @@ async fn test_fills_result_set_when_loading_messages() -> Result<()> { .expect_get_user_info() .returning(|_, _| Box::pin(async { Ok(None) })); + deps.message_repo + .expect_resolve_server_id_to_message_id() + .times(10) + .returning(|_, _, _| Box::pin(async { Ok(None) })); + deps.message_repo .expect_append() .returning(|_, _, _| Box::pin(async { Ok(()) })); @@ -496,6 +512,7 @@ async fn test_fills_result_set_when_loading_messages() -> Result<()> { assert_eq!( vec![ MessageBuilder::new_with_index(90) + .set_id("msg-id-10") .set_from(user_id!("a@prose.org")) .set_from_name("A") .set_payload(MessageLikePayload::Message { @@ -513,6 +530,7 @@ async fn test_fills_result_set_when_loading_messages() -> Result<()> { },]) .build_message_dto(), MessageBuilder::new_with_index(92) + .set_id("msg-id-8") .set_from(user_id!("b@prose.org")) .set_from_name("B") .set_payload(MessageLikePayload::Message { @@ -523,6 +541,7 @@ async fn test_fills_result_set_when_loading_messages() -> Result<()> { }) .build_message_dto(), MessageBuilder::new_with_index(93) + .set_id("msg-id-7") .set_from(user_id!("a@prose.org")) .set_from_name("A") .set_payload(MessageLikePayload::Message { @@ -533,6 +552,7 @@ async fn test_fills_result_set_when_loading_messages() -> Result<()> { }) .build_message_dto(), MessageBuilder::new_with_index(94) + .set_id("msg-id-6") .set_from(user_id!("a@prose.org")) .set_from_name("A") .set_payload(MessageLikePayload::Message { @@ -543,6 +563,7 @@ async fn test_fills_result_set_when_loading_messages() -> Result<()> { }) .build_message_dto(), MessageBuilder::new_with_index(101) + .set_id("msg-id-4") .set_from(user_id!("a@prose.org")) .set_from_name("A") .set_payload(MessageLikePayload::Message { @@ -557,6 +578,7 @@ async fn test_fills_result_set_when_loading_messages() -> Result<()> { }]) .build_message_dto(), MessageBuilder::new_with_index(102) + .set_id("msg-id-3") .set_from(user_id!("b@prose.org")) .set_from_name("B") .set_payload(MessageLikePayload::Message { @@ -580,6 +602,7 @@ async fn test_fills_result_set_when_loading_messages() -> Result<()> { #[tokio::test] async fn test_stops_at_max_message_pages_to_load() -> Result<()> { let mut deps = MockRoomFactoryDependencies::default(); + deps.message_id_provider = Arc::new(WrappingMessageIdProvider::incrementing("msg-id")); deps.ctx.config.message_page_size = 5; deps.ctx.config.max_message_pages_to_load = 2; @@ -645,6 +668,11 @@ async fn test_stops_at_max_message_pages_to_load() -> Result<()> { .expect_get_user_info() .returning(|_, _| Box::pin(async { Ok(None) })); + deps.message_repo + .expect_resolve_server_id_to_message_id() + .times(10) + .returning(|_, _, _| Box::pin(async { Ok(None) })); + deps.message_repo .expect_append() .returning(|_, _, _| Box::pin(async { Ok(()) })); @@ -662,6 +690,7 @@ async fn test_stops_at_max_message_pages_to_load() -> Result<()> { assert_eq!( vec![MessageBuilder::new_with_index(100) + .set_id("msg-id-1") .set_from(user_id!("a@prose.org")) .set_from_name("A") .set_payload(MessageLikePayload::Message { @@ -722,6 +751,11 @@ async fn test_stops_at_last_page() -> Result<()> { .expect_get_user_info() .returning(|_, _| Box::pin(async { Ok(None) })); + deps.message_repo + .expect_resolve_server_id_to_message_id() + .times(8) + .returning(|_, _, _| Box::pin(async { Ok(None) })); + deps.message_repo .expect_append() .returning(|_, _, _| Box::pin(async { Ok(()) })); @@ -742,6 +776,7 @@ async fn test_stops_at_last_page() -> Result<()> { async fn test_resolves_targeted_messages_when_loading_messages() -> Result<()> { let mut deps = MockRoomFactoryDependencies::default(); deps.ctx.config.max_message_pages_to_load = 1; + deps.message_id_provider = Arc::new(WrappingMessageIdProvider::incrementing("msg-id")); deps.message_archive_service .expect_load_messages_before() @@ -759,7 +794,6 @@ async fn test_resolves_targeted_messages_when_loading_messages() -> Result<()> { .set_from(user_id!("user@prose.org")) .build_archived_message("q1", None), MessageBuilder::new_with_index(3) - .set_from(user_id!("user@prose.org")) .set_target_message_idx(2) .set_from(user_id!("a@prose.org")) .set_payload(MessageLikePayload::Reaction { @@ -786,13 +820,13 @@ async fn test_resolves_targeted_messages_when_loading_messages() -> Result<()> { predicate::always(), predicate::eq(RoomId::from(muc_id!("room@conference.prose.org"))), predicate::eq(vec![ - MessageBuilder::id_for_index(5).into(), + MessageBuilder::remote_id_for_index(5).into(), MessageBuilder::stanza_id_for_index(5).into(), - MessageBuilder::id_for_index(4).into(), + MessageBuilder::remote_id_for_index(4).into(), MessageBuilder::stanza_id_for_index(4).into(), - MessageBuilder::id_for_index(2).into(), + MessageBuilder::remote_id_for_index(2).into(), MessageBuilder::stanza_id_for_index(2).into(), - MessageBuilder::id_for_index(1).into(), + MessageBuilder::remote_id_for_index(1).into(), MessageBuilder::stanza_id_for_index(1).into(), ]), predicate::eq(Utc.with_ymd_and_hms(2024, 02, 23, 0, 0, 0).unwrap()), @@ -846,6 +880,11 @@ async fn test_resolves_targeted_messages_when_loading_messages() -> Result<()> { .expect_get_user_info() .returning(|_, _| Box::pin(async { Ok(None) })); + deps.message_repo + .expect_resolve_server_id_to_message_id() + .times(5) + .returning(|_, _, _| Box::pin(async { Ok(None) })); + deps.message_repo .expect_append() .returning(|_, _, _| Box::pin(async { Ok(()) })); @@ -861,6 +900,7 @@ async fn test_resolves_targeted_messages_when_loading_messages() -> Result<()> { assert_eq!( vec![ MessageBuilder::new_with_index(1) + .set_id("msg-id-5") .set_from(user_id!("user@prose.org")) .set_from_name("User") .set_reactions([ @@ -875,6 +915,7 @@ async fn test_resolves_targeted_messages_when_loading_messages() -> Result<()> { ]) .build_message_dto(), MessageBuilder::new_with_index(2) + .set_id("msg-id-4") .set_from(user_id!("user@prose.org")) .set_from_name("User") .set_reactions([Reaction { @@ -883,6 +924,7 @@ async fn test_resolves_targeted_messages_when_loading_messages() -> Result<()> { }]) .build_message_dto(), MessageBuilder::new_with_index(4) + .set_id("msg-id-2") .set_from(user_id!("user@prose.org")) .set_from_name("User") .set_reactions([Reaction { @@ -891,6 +933,7 @@ async fn test_resolves_targeted_messages_when_loading_messages() -> Result<()> { }]) .build_message_dto(), MessageBuilder::new_with_index(5) + .set_id("msg-id-1") .set_from(user_id!("user@prose.org")) .set_from_name("User") .set_timestamp(Utc.with_ymd_and_hms(2024, 02, 23, 0, 0, 0).unwrap()) diff --git a/crates/prose-utils/src/id_string_macro.rs b/crates/prose-utils/src/id_string_macro.rs index f1727e05..8bd773ab 100644 --- a/crates/prose-utils/src/id_string_macro.rs +++ b/crates/prose-utils/src/id_string_macro.rs @@ -5,7 +5,8 @@ #[macro_export] macro_rules! id_string { - ($t:ident) => { + ($(#[$meta:meta])* $t:ident) => { + $(#[$meta])* #[derive(Debug, Eq, PartialEq, Hash, Clone, serde::Serialize, serde::Deserialize)] #[serde(transparent)] pub struct $t(String); @@ -32,8 +33,14 @@ macro_rules! id_string { } } + impl std::borrow::Borrow for $t { + fn borrow(&self) -> &str { + &self.0 + } + } + impl std::str::FromStr for $t { - type Err = (); + type Err = std::convert::Infallible; fn from_str(s: &str) -> Result { Ok($t(s.to_string())) diff --git a/examples/prose-core-client-cli/src/type_display.rs b/examples/prose-core-client-cli/src/type_display.rs index 31c456b3..05509a3d 100644 --- a/examples/prose-core-client-cli/src/type_display.rs +++ b/examples/prose-core-client-cli/src/type_display.rs @@ -205,11 +205,7 @@ impl Display for MessageEnvelope { f, "{} | {:<36} | {:<20} | {} attachments | {} mentions | {}{}", self.0.timestamp.format("%Y/%m/%d %H:%M:%S"), - self.0 - .id - .as_ref() - .map(|id| id.clone().into_inner()) - .unwrap_or("".to_string()), + self.0.id, self.0.from.id.to_opaque_identifier().truncate_to(20), self.0.attachments.len(), self.0.mentions.len(), @@ -231,11 +227,7 @@ impl Display for CompactMessageEnvelope { f, "{} | {:<36} | {:<20} | {}", self.0.timestamp.format("%Y/%m/%d %H:%M:%S"), - self.0 - .id - .as_ref() - .map(|id| id.clone().into_inner()) - .unwrap_or("".to_string()), + self.0.id, self.0.from.id.to_opaque_identifier().truncate_to(20), self.0.body.html.to_string().truncate_to(40), ) diff --git a/examples/prose-core-client-cli/src/type_selection.rs b/examples/prose-core-client-cli/src/type_selection.rs index 5a73b135..2202e647 100644 --- a/examples/prose-core-client-cli/src/type_selection.rs +++ b/examples/prose-core-client-cli/src/type_selection.rs @@ -6,14 +6,14 @@ use std::iter; use std::path::{Path, PathBuf}; -use anyhow::{anyhow, Result}; +use anyhow::Result; use dialoguer::theme::ColorfulTheme; use dialoguer::{Input, MultiSelect, Select}; use jid::BareJid; use prose_core_client::dtos::{ - DeviceId, Message, MessageRemoteId, MessageServerId, ParticipantInfo, PublicRoomInfo, - RoomEnvelope, SidebarItem, UserId, + DeviceId, Message, MessageId, MessageServerId, ParticipantInfo, PublicRoomInfo, RoomEnvelope, + SidebarItem, UserId, }; use prose_core_client::services::{Generic, Room}; use prose_core_client::Client; @@ -120,13 +120,11 @@ pub async fn select_participant(room: &Room) -> Option { select_item_from_list(room.participants(), |p| ParticipantEnvelope(p.clone())) } -pub async fn select_message(room: &Room) -> Result> { +pub async fn select_message(room: &Room) -> Result> { let messages = load_messages(room, 1).await?; let message = select_item_from_list(messages, |message| CompactMessageEnvelope(message.clone())); - Ok(message - .map(|m| m.id.ok_or(anyhow!("Selected message does not have an ID"))) - .transpose()?) + Ok(message.map(|m| m.id)) } pub async fn select_public_channel(client: &Client) -> Result> { diff --git a/tests/prose-core-integration-tests/src/lib.rs b/tests/prose-core-integration-tests/src/lib.rs index 6ec28983..9aff958b 100644 --- a/tests/prose-core-integration-tests/src/lib.rs +++ b/tests/prose-core-integration-tests/src/lib.rs @@ -3,10 +3,16 @@ // Copyright: 2023, Marc Bauer // License: Mozilla Public License v2.0 (MPL v2.0) +use tracing::Level; + #[cfg(not(target_arch = "wasm32"))] #[ctor::ctor] fn init() { - let _ = tracing_subscriber::fmt().with_test_writer().try_init(); + let _ = tracing_subscriber::fmt() + .with_test_writer() + // Set this to Level::DEBUG to log SQL queries… + .with_max_level(Level::INFO) + .try_init(); } #[cfg(test)] diff --git a/tests/prose-core-integration-tests/src/tests/client/catchup_unread.rs b/tests/prose-core-integration-tests/src/tests/client/catchup_unread.rs index f5f3579d..c7d83f83 100644 --- a/tests/prose-core-integration-tests/src/tests/client/catchup_unread.rs +++ b/tests/prose-core-integration-tests/src/tests/client/catchup_unread.rs @@ -162,6 +162,7 @@ async fn test_rounds_timestamps() -> Result<()> { event!(client, ClientEvent::SidebarChanged); } client.receive_next().await; + client.bump_message_id(); // Check that we have one SidebarItem with an unread_count of 1 let sidebar_items = client.sidebar.sidebar_items().await; @@ -181,13 +182,14 @@ async fn test_rounds_timestamps() -> Result<()> { "# ); + client.bump_message_id(); event!(client, ClientEvent::SidebarChanged); room_event!( client, user_id.clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec!["id-2".into()] + message_ids: vec![client.get_last_message_id()] } ) } @@ -233,13 +235,14 @@ async fn test_rounds_timestamps() -> Result<()> { "# ); + client.bump_message_id(); event!(client, ClientEvent::SidebarChanged); room_event!( client, user_id.clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec!["id-3".into()] + message_ids: vec![client.get_last_message_id()] } ) } @@ -275,25 +278,25 @@ async fn test_rounds_timestamps() -> Result<()> { MessageBuilder::new_with_id( "id-1", Utc.with_ymd_and_hms(2024, 04, 05, 10, 00, 01).unwrap(), - MessageLikePayload::message("Hello"), + MessageLikePayload::message("hello 1"), ) - .set_stanza_id(Some("stanza-id-1".into())) + .set_server_id(Some("stanza-id-1".into())) .set_from(user_id!("other@prose.org")) .build_archived_message("", None), MessageBuilder::new_with_id( "id-2", Utc.with_ymd_and_hms(2024, 04, 05, 10, 00, 01).unwrap(), - MessageLikePayload::message("Hello"), + MessageLikePayload::message("hello 2"), ) - .set_stanza_id(Some("stanza-id-2".into())) + .set_server_id(Some("stanza-id-2".into())) .set_from(user_id!("other@prose.org")) .build_archived_message("", None), MessageBuilder::new_with_id( "id-3", Utc.with_ymd_and_hms(2024, 04, 05, 10, 01, 00).unwrap(), - MessageLikePayload::message("Hello"), + MessageLikePayload::message("hello 3"), ) - .set_stanza_id(Some("stanza-id-3".into())) + .set_server_id(Some("stanza-id-3".into())) .set_from(user_id!("other@prose.org")) .build_archived_message("", None), ]; @@ -343,7 +346,7 @@ async fn test_rounds_timestamps() -> Result<()> { let unread_messages = room.load_unread_messages().await?.messages; assert_eq!(1, unread_messages.len()); - assert_eq!(Some("id-3".into()), unread_messages[0].id); + assert_eq!("hello 3", unread_messages[0].body.raw); Ok(()) } @@ -491,14 +494,17 @@ async fn test_loads_unread_messages() -> Result<()> { &room_id, &[ MessageBuilder::new_with_index(1) + .set_payload("hello 1") .set_from(occupant_id!("room@conf.prose.org/friend")) .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 25, 10, 00, 00).unwrap()) .build_message_like(), MessageBuilder::new_with_index(2) + .set_payload("hello 2") .set_from(occupant_id!("room@conf.prose.org/friend")) .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 26, 10, 00, 00).unwrap()) .build_message_like(), MessageBuilder::new_with_index(3) + .set_payload("hello 3") .set_from(occupant_id!("room@conf.prose.org/friend")) .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 26, 11, 00, 00).unwrap()) .build_message_like(), @@ -533,13 +539,10 @@ async fn test_loads_unread_messages() -> Result<()> { let unread_messages = room.load_unread_messages().await?; assert_eq!( - vec![ - MessageBuilder::stanza_id_for_index(2), - MessageBuilder::stanza_id_for_index(3), - ], + vec!["hello 2", "hello 3",], unread_messages .into_iter() - .filter_map(|message| message.stanza_id) + .map(|message| message.body.raw) .collect::>() ); @@ -877,8 +880,8 @@ async fn test_set_unread_message_saves_settings() -> Result<()> { .build_message_like(), ]; - messages[2].stanza_id = None; - messages[3].stanza_id = None; + messages[2].server_id = None; + messages[3].server_id = None; let message_repo = CachingMessageRepository::new(store.clone()); message_repo.append(&account, &room_id, &messages).await?; diff --git a/tests/prose-core-integration-tests/src/tests/client/helpers/test_client.rs b/tests/prose-core-integration-tests/src/tests/client/helpers/test_client.rs index 43afc1ee..ec95cea3 100644 --- a/tests/prose-core-integration-tests/src/tests/client/helpers/test_client.rs +++ b/tests/prose-core-integration-tests/src/tests/client/helpers/test_client.rs @@ -15,7 +15,8 @@ use regex::Regex; use prose_core_client::app::deps::AppConfig; use prose_core_client::domain::encryption::services::IncrementingUserDeviceIdProvider; -use prose_core_client::dtos::{DeviceId, RoomEnvelope, RoomId, UserId}; +use prose_core_client::domain::messaging::services::WrappingMessageIdProvider; +use prose_core_client::dtos::{DeviceId, MessageId, RoomEnvelope, RoomId, UserId}; use prose_core_client::infra::encryption::{EncryptionKeysRepository, SessionRepository}; use prose_core_client::infra::general::mocks::StepRngProvider; use prose_core_client::test::ConstantTimeProvider; @@ -37,6 +38,7 @@ pub struct TestClient { pub(super) client: Client, connector: Connector, pub(crate) id_provider: IncrementingOffsettingIDProvider, + pub(crate) message_id_provider: IncrementingOffsettingIDProvider, pub(super) short_id_provider: IncrementingIDProvider, messages: TestMessageQueue, context: Mutex>>, @@ -115,6 +117,7 @@ impl TestClientBuilder { let client = Client::builder() .set_connector_provider(connector.provider()) .set_id_provider(IncrementingIDProvider::new("id")) + .set_message_id_provider(WrappingMessageIdProvider::incrementing("msg-id")) .set_short_id_provider(IncrementingIDProvider::new("short-id")) .set_rng_provider(StepRngProvider::default()) .set_avatar_repository(FsAvatarRepository::new(&path).unwrap()) @@ -131,6 +134,7 @@ impl TestClientBuilder { // We'll just mirror the used ID provider here… connector, id_provider: IncrementingOffsettingIDProvider::new("id"), + message_id_provider: IncrementingOffsettingIDProvider::new("msg-id"), short_id_provider: IncrementingIDProvider::new("short-id"), messages, context: Default::default(), @@ -193,6 +197,7 @@ macro_rules! any_event( impl TestClient { pub fn send(&self, xml: impl Into, file: &str, line: u32) { self.id_provider.apply_offset(); + self.message_id_provider.apply_offset(); let mut xml = xml.into(); @@ -219,7 +224,16 @@ impl TestClient { } self.id_provider.set_offset(highest_offset); + self.apply_ctx(&mut xml); + + if xml.contains("{{MSG_ID}}") { + self.push_ctx([("MSG_ID", self.message_id_provider.id_with_offset(1))]); + self.apply_ctx(&mut xml); + self.pop_ctx(); + self.message_id_provider.set_offset(1); + }; + self.messages.send(xml, file, line); } @@ -247,6 +261,23 @@ impl TestClient { } self.apply_ctx(&mut xml); + + if xml.contains("{{LAST_MSG_ID}}") { + self.push_ctx([( + "LAST_MSG_ID", + self.message_id_provider.last_id_with_offset(1), + )]); + self.apply_ctx(&mut xml); + self.pop_ctx(); + }; + + if xml.contains("{{MSG_ID}}") { + self.push_ctx([("MSG_ID", self.message_id_provider.id_with_offset(1))]); + self.apply_ctx(&mut xml); + self.pop_ctx(); + self.message_id_provider.set_offset(1); + }; + self.messages.receive(xml, file, line); } @@ -348,6 +379,24 @@ impl TestClient { pub fn get_next_id(&self) -> String { self.id_provider.next_id() } + + pub fn get_last_message_id(&self) -> MessageId { + self.message_id_provider.last_id_with_offset(1).into() + } + + pub fn get_next_message_id_with_offset(&self, offset: i64) -> MessageId { + assert!(offset > 0); + self.message_id_provider.id_with_offset(offset).into() + } + + pub fn get_next_message_id(&self) -> MessageId { + self.message_id_provider.next_id().into() + } + + pub fn bump_message_id(&self) { + self.message_id_provider.set_offset(1); + self.message_id_provider.apply_offset(); + } } impl TestClient { diff --git a/tests/prose-core-integration-tests/src/tests/client/message_handling.rs b/tests/prose-core-integration-tests/src/tests/client/message_handling.rs new file mode 100644 index 00000000..11c07ac4 --- /dev/null +++ b/tests/prose-core-integration-tests/src/tests/client/message_handling.rs @@ -0,0 +1,495 @@ +// prose-core-client/prose-core-integration-tests +// +// Copyright: 2024, Marc Bauer +// License: Mozilla Public License v2.0 (MPL v2.0) + +use crate::tests::client::helpers::{StartDMStrategy, TestClient}; +use crate::tests::store; +use crate::{event, recv, room_event, send}; +use anyhow::Result; +use chrono::{DateTime, Duration, TimeZone, Utc}; +use itertools::Itertools; +use minidom::Element; +use pretty_assertions::assert_eq; +use prose_core_client::domain::messaging::repos::MessagesRepository; +use prose_core_client::domain::shared::models::AnonOccupantId; +use prose_core_client::dtos::{ + AccountId, MucId, RoomId, SendMessageRequest, SendMessageRequestBody, UserId, +}; +use prose_core_client::infra::messaging::CachingMessageRepository; +use prose_core_client::test::MessageBuilder; +use prose_core_client::{account_id, muc_id, user_id, ClientEvent, ClientRoomEventType}; +use prose_proc_macros::mt_test; +use prose_xmpp::stanza::Message; +use prose_xmpp::{bare, TimeProvider}; +use xmpp_parsers::mam::QueryId; + +#[mt_test] +async fn test_receives_message_with_same_id_twice() -> Result<()> { + let client = TestClient::new().await; + client + .expect_login(user_id!("user@prose.org"), "secret") + .await?; + + let other_user_id = user_id!("other@prose.org"); + client.push_ctx([("OTHER_USER_ID", other_user_id.to_string())]); + + let room = client + .start_dm(other_user_id.clone()) + .await? + .to_generic_room(); + + let message1_id = client.get_next_message_id_with_offset(1); + let message2_id = client.get_next_message_id_with_offset(2); + + { + recv!( + client, + r#" + + Message 1 + + "# + ); + + event!(client, ClientEvent::SidebarChanged); + room_event!( + client, + room.jid().clone(), + ClientRoomEventType::MessagesAppended { + message_ids: vec![message1_id.clone()] + } + ); + } + client.receive_next().await; + + { + recv!( + client, + r#" + + Message 2 + + "# + ); + + event!(client, ClientEvent::SidebarChanged); + room_event!( + client, + room.jid().clone(), + ClientRoomEventType::MessagesAppended { + message_ids: vec![message2_id.clone()] + } + ); + } + client.receive_next().await; + + let messages = room + .load_messages_with_ids(&[message1_id, message2_id]) + .await? + .into_iter() + .map(|msg| msg.body.raw) + .collect::>(); + + assert_eq!(vec!["Message 1", "Message 2"], messages); + + Ok(()) +} + +#[mt_test] +async fn test_updates_existing_messages_on_catchup() -> Result<()> { + let store = store().await.expect("Failed to set up store."); + let client = TestClient::builder().set_store(store.clone()).build().await; + + let account = account_id!("user@prose.org"); + let user_id = user_id!("other@prose.org"); + let room_id = RoomId::User(user_id.clone()); + + let message_repo = CachingMessageRepository::new(store.clone()); + message_repo + .append( + &account, + &room_id, + &[ + MessageBuilder::new_with_index(1) + .set_from(account.to_user_id()) + .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 25, 10, 00, 00).unwrap()) + .set_id("custom-msg-id-1") + .set_server_id(None) + .set_remote_id(Some("remote-id-1".into())) + .build_message_like(), + MessageBuilder::new_with_index(2) + .set_from(user_id.clone()) + .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 25, 10, 00, 10).unwrap()) + .set_id("custom-msg-id-2") + .set_server_id(Some("server-id-2".into())) + .set_remote_id(Some("remote-id-2".into())) + .build_message_like(), + ], + ) + .await?; + + client.expect_login(account.to_user_id(), "secret").await?; + + let join_dm_strategy = StartDMStrategy::default().with_catch_up_handler({ + let account = account.clone(); + move |client, user_id| { + client.expect_catchup_with_config( + user_id, + client.time_provider.now() + - Duration::seconds(client.app_config.max_catchup_duration_secs), + vec![ + MessageBuilder::new_with_index(1) + .set_from(account.to_user_id()) + .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 26, 10, 00, 00).unwrap()) + .set_remote_id(Some("remote-id-1".into())) + .set_server_id(Some("server-id-1".into())) + .build_archived_message("", None), + MessageBuilder::new_with_index(2) + .set_from(user_id.clone()) + .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 26, 10, 10, 00).unwrap()) + .set_remote_id(Some("remote-id-2".into())) + .set_server_id(Some("server-id-2".into())) + .build_archived_message("", None), + MessageBuilder::new_with_index(3) + .set_from(user_id.clone()) + .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 26, 10, 20, 00).unwrap()) + .set_remote_id(Some("remote-id-3".into())) + .set_server_id(Some("server-id-3".into())) + .build_archived_message("", None), + ], + ) + } + }); + + let new_message_id = client.get_next_message_id(); + + client + .start_dm_with_strategy(user_id.clone(), join_dm_strategy) + .await?; + + let messages = message_repo + .get_messages_after(&account, &room_id, DateTime::::MIN_UTC) + .await? + .into_iter() + .map(|msg| (msg.id, msg.server_id, msg.timestamp)) + .sorted_by(|l, r| l.2.cmp(&r.2)) + .collect::>(); + + assert_eq!( + vec![ + ( + "custom-msg-id-1".into(), + Some("server-id-1".into()), + Utc.with_ymd_and_hms(2024, 04, 26, 10, 00, 00).unwrap() + ), + ( + "custom-msg-id-2".into(), + Some("server-id-2".into()), + Utc.with_ymd_and_hms(2024, 04, 26, 10, 10, 00).unwrap() + ), + ( + new_message_id, + Some("server-id-3".into()), + Utc.with_ymd_and_hms(2024, 04, 26, 10, 20, 00).unwrap() + ), + ], + messages + ); + + Ok(()) +} + +#[mt_test] +async fn test_updates_existing_messages_when_loading_from_mam() -> Result<()> { + let store = store().await.expect("Failed to set up store."); + let client = TestClient::builder().set_store(store.clone()).build().await; + + let account = account_id!("user@prose.org"); + let user_id = user_id!("other@prose.org"); + let room_id = RoomId::User(user_id.clone()); + + client.push_ctx([("OTHER_USER_ID", user_id.to_string())]); + + let message_repo = CachingMessageRepository::new(store.clone()); + message_repo + .append( + &account, + &room_id, + &[ + // A message by us without a StanzaId + MessageBuilder::new_with_index(1) + .set_from(account.to_user_id()) + .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 25, 10, 00, 00).unwrap()) + .set_id("custom-msg-id-1") + .set_server_id(None) + .set_remote_id(Some("remote-id-1".into())) + .build_message_like(), + // A message by them with a StanzaId + MessageBuilder::new_with_index(2) + .set_from(user_id.clone()) + .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 25, 10, 00, 10).unwrap()) + .set_id("custom-msg-id-2") + .set_server_id(Some("server-id-2".into())) + .set_remote_id(Some("remote-id-2".into())) + .build_message_like(), + ], + ) + .await?; + + client.expect_login(account.to_user_id(), "secret").await?; + + let room = client.start_dm(user_id.clone()).await?.to_generic_room(); + + send!( + client, + r#" + + + + + urn:xmpp:mam:2 + + + {{OTHER_USER_ID}} + + + + 100 + + + + + "# + ); + + let query_id = QueryId(client.id_provider.id_with_offset(1)); + + let received_messages = vec![ + // This should update our message + MessageBuilder::new_with_index(1) + .set_from(account.to_user_id()) + .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 26, 10, 00, 00).unwrap()) + .set_remote_id(Some("remote-id-1".into())) + .set_server_id(Some("server-id-1".into())) + .build_archived_message("", None), + // This should update their message + MessageBuilder::new_with_index(2) + .set_from(user_id.clone()) + .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 26, 10, 10, 00).unwrap()) + .set_remote_id(Some("remote-id-2".into())) + .set_server_id(Some("server-id-2".into())) + .build_archived_message("", None), + // This should create a new message + MessageBuilder::new_with_index(3) + .set_from(user_id.clone()) + .set_timestamp(Utc.with_ymd_and_hms(2024, 04, 26, 10, 20, 00).unwrap()) + .set_remote_id(Some("remote-id-3".into())) + .set_server_id(Some("server-id-3".into())) + .build_archived_message("", None), + ]; + + for mut archived_message in received_messages.into_iter() { + archived_message.query_id = Some(query_id.clone()); + + let message = Message::new().set_archived_message(archived_message); + client.receive_element(Element::from(message), file!(), line!()); + } + + recv!( + client, + r#" + + + + + + "# + ); + + let new_message_id = client.get_next_message_id(); + + _ = room.load_latest_messages().await?; + + let messages = message_repo + .get_messages_after(&account, &room_id, DateTime::::MIN_UTC) + .await? + .into_iter() + .map(|msg| (msg.id, msg.server_id, msg.timestamp)) + .sorted_by(|l, r| l.2.cmp(&r.2)) + .collect::>(); + + assert_eq!( + vec![ + ( + "custom-msg-id-1".into(), + Some("server-id-1".into()), + Utc.with_ymd_and_hms(2024, 04, 26, 10, 00, 00).unwrap() + ), + ( + "custom-msg-id-2".into(), + Some("server-id-2".into()), + Utc.with_ymd_and_hms(2024, 04, 26, 10, 10, 00).unwrap() + ), + ( + new_message_id, + Some("server-id-3".into()), + Utc.with_ymd_and_hms(2024, 04, 26, 10, 20, 00).unwrap() + ), + ], + messages + ); + + Ok(()) +} + +#[mt_test] +async fn test_sends_and_updates_message_to_muc_room() -> Result<()> { + let store = store().await.expect("Failed to set up store."); + let client = TestClient::builder().set_store(store.clone()).build().await; + + client + .expect_login(user_id!("user@prose.org"), "secret") + .await?; + + let room_id = muc_id!("room@conference.prose.org"); + let occupant_id = client.build_occupant_id(&room_id); + let anon_occupant_id = AnonOccupantId::from("anon-occupant-id"); + + client + .join_room(room_id.clone(), anon_occupant_id.clone()) + .await?; + + client.push_ctx([ + ("OCCUPANT_ID", occupant_id.to_string()), + ("ROOM_ID", room_id.to_string()), + ("ANON_OCCUPANT_ID", anon_occupant_id.to_string()), + ]); + + let message_id = client.get_next_message_id(); + + send!( + client, + r#" + + Hello + Hello + + + + + "# + ); + + room_event!( + client, + room_id.clone(), + ClientRoomEventType::MessagesAppended { + message_ids: vec![message_id.clone().into()] + } + ); + + let room = client.get_room(room_id.clone()).await.to_generic_room(); + room.send_message(SendMessageRequest { + body: Some(SendMessageRequestBody { + text: "Hello".into(), + }), + attachments: vec![], + }) + .await?; + + recv!( + client, + r#" + + Hello + + + + + + + "# + ); + + client.receive_next().await; + + let messages = CachingMessageRepository::new(store) + .get( + &bare!("user@prose.org").into(), + &room_id.clone().into(), + &message_id, + ) + .await?; + + assert_eq!(1, messages.len()); + assert_eq!( + Some("opZdWmO7r50ee_aGKnWvBMbK".into()), + messages[0].server_id, + ); + + client.push_ctx([("INITIAL_MESSAGE_ID", message_id.to_string())]); + + send!( + client, + r#" + + Hello World + Hello World + + + + "# + ); + + room_event!( + client, + room_id.clone(), + ClientRoomEventType::MessagesUpdated { + message_ids: vec![message_id.clone().into()] + } + ); + + room.update_message( + message_id.clone().into(), + SendMessageRequest { + body: Some(SendMessageRequestBody { + text: "Hello World".into(), + }), + attachments: vec![], + }, + ) + .await?; + + let messages = room + .load_messages_with_ids(&[message_id.clone().into()]) + .await?; + assert_eq!(1, messages.len()); + assert_eq!("

Hello World

", messages[0].body.html.as_ref()); + + recv!( + client, + r#" + + Hello World + + + + + + "# + ); + + client.receive_next().await; + + let messages = room + .load_messages_with_ids(&[message_id.clone().into()]) + .await?; + assert_eq!(1, messages.len()); + assert_eq!("

Hello World

", messages[0].body.html.as_ref()); + + client.pop_ctx(); + client.pop_ctx(); + + Ok(()) +} diff --git a/tests/prose-core-integration-tests/src/tests/client/message_styling.rs b/tests/prose-core-integration-tests/src/tests/client/message_styling.rs index c10174a3..8ed6ebba 100644 --- a/tests/prose-core-integration-tests/src/tests/client/message_styling.rs +++ b/tests/prose-core-integration-tests/src/tests/client/message_styling.rs @@ -28,7 +28,7 @@ async fn test_send_markdown_message() -> Result<()> { send!( client, r#" - + Some *bold*, _italic_, ~strikethrough~ and *_bold italic_* text. Some **bold**, _italic_, ~~strikethrough~~ and **_bold italic_** text. @@ -42,7 +42,7 @@ async fn test_send_markdown_message() -> Result<()> { client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -82,20 +82,20 @@ async fn test_receive_markdown_message() -> Result<()> { "# ); + let message_id = client.get_next_message_id(); + event!(client, ClientEvent::SidebarChanged); room_event!( client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec!["my-message-id".into()] + message_ids: vec![message_id.clone()] } ); client.receive_next().await; - let messages = room - .load_messages_with_ids(&["my-message-id".into()]) - .await?; + let messages = room.load_messages_with_ids(&[message_id]).await?; let message = messages.first().unwrap(); @@ -137,20 +137,20 @@ And another newline "# ); + let message_id = client.get_next_message_id(); + event!(client, ClientEvent::SidebarChanged); room_event!( client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec!["my-message-id".into()] + message_ids: vec![message_id.clone()] } ); client.receive_next().await; - let messages = room - .load_messages_with_ids(&["my-message-id".into()]) - .await?; + let messages = room.load_messages_with_ids(&[message_id]).await?; let message = messages.first().unwrap(); diff --git a/tests/prose-core-integration-tests/src/tests/client/mod.rs b/tests/prose-core-integration-tests/src/tests/client/mod.rs index 2e6e0250..dae30930 100644 --- a/tests/prose-core-integration-tests/src/tests/client/mod.rs +++ b/tests/prose-core-integration-tests/src/tests/client/mod.rs @@ -6,6 +6,7 @@ mod catchup_unread; mod contact_list; mod helpers; +mod message_handling; mod message_styling; mod muc; mod muc_omemo; diff --git a/tests/prose-core-integration-tests/src/tests/client/muc.rs b/tests/prose-core-integration-tests/src/tests/client/muc.rs index 2e5c21e6..31c93483 100644 --- a/tests/prose-core-integration-tests/src/tests/client/muc.rs +++ b/tests/prose-core-integration-tests/src/tests/client/muc.rs @@ -10,11 +10,8 @@ use pretty_assertions::assert_eq; use super::helpers::{JoinRoomStrategy, TestClient}; use crate::{event, recv, room_event, send}; use itertools::Itertools; -use prose_core_client::domain::shared::models::AnonOccupantId; use prose_core_client::domain::sidebar::models::BookmarkType; -use prose_core_client::dtos::{ - MucId, ParticipantId, SendMessageRequest, SendMessageRequestBody, UserId, -}; +use prose_core_client::dtos::{MucId, ParticipantId, UserId}; use prose_core_client::{muc_id, user_id, ClientEvent, ClientRoomEventType}; use prose_proc_macros::mt_test; use prose_xmpp::bare; @@ -556,6 +553,8 @@ async fn test_receives_chat_states() -> Result<()> { composing_users[0].id ); + let message_id = client.get_next_message_id(); + recv!( client, r#" @@ -580,7 +579,7 @@ async fn test_receives_chat_states() -> Result<()> { client, room_id.clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec!["message-id-2".into()] + message_ids: vec![message_id.clone()] } ); @@ -589,173 +588,9 @@ async fn test_receives_chat_states() -> Result<()> { let composing_users = room.load_composing_users().await?; assert!(composing_users.is_empty()); - let messages = room - .load_messages_with_ids(&["message-id-2".into()]) - .await?; - assert_eq!(1, messages.len()); - assert_eq!(Some("stanza-id".into()), messages[0].stanza_id); - - Ok(()) -} - -#[mt_test] -async fn test_sends_and_updates_message_to_muc_room() -> Result<()> { - let client = TestClient::new().await; - - client - .expect_login(user_id!("user@prose.org"), "secret") - .await?; - - let room_id = muc_id!("room@conference.prose.org"); - let occupant_id = client.build_occupant_id(&room_id); - let anon_occupant_id = AnonOccupantId::from("anon-occupant-id"); - - client - .join_room(room_id.clone(), anon_occupant_id.clone()) - .await?; - - client.push_ctx([ - ("OCCUPANT_ID", occupant_id.to_string()), - ("ROOM_ID", room_id.to_string()), - ("ANON_OCCUPANT_ID", anon_occupant_id.to_string()), - ]); - - let message_id = client.get_next_id(); - - send!( - client, - r#" - - Hello - Hello - - - - - "# - ); - - room_event!( - client, - room_id.clone(), - ClientRoomEventType::MessagesAppended { - message_ids: vec![message_id.clone().into()] - } - ); - - let room = client.get_room(room_id.clone()).await.to_generic_room(); - room.send_message(SendMessageRequest { - body: Some(SendMessageRequestBody { - text: "Hello".into(), - }), - attachments: vec![], - }) - .await?; - - recv!( - client, - r#" - - Hello - - - - - - - "# - ); - - room_event!( - client, - room_id.clone(), - ClientRoomEventType::MessagesUpdated { - message_ids: vec![message_id.clone().into()] - } - ); - - client.receive_next().await; - - let messages = room - .load_messages_with_ids(&[message_id.clone().into()]) - .await?; + let messages = room.load_messages_with_ids(&[message_id]).await?; assert_eq!(1, messages.len()); - assert_eq!("

Hello

", messages[0].body.html.as_ref()); - assert_eq!( - Some("opZdWmO7r50ee_aGKnWvBMbK".into()), - messages[0].stanza_id, - ); - - client.push_ctx([("INITIAL_MESSAGE_ID", message_id.to_string())]); - - send!( - client, - r#" - - Hello World - Hello World - - - - "# - ); - - room_event!( - client, - room_id.clone(), - ClientRoomEventType::MessagesUpdated { - message_ids: vec![message_id.clone().into()] - } - ); - - room.update_message( - message_id.clone().into(), - SendMessageRequest { - body: Some(SendMessageRequestBody { - text: "Hello World".into(), - }), - attachments: vec![], - }, - ) - .await?; - - let messages = room - .load_messages_with_ids(&[message_id.clone().into()]) - .await?; - assert_eq!(1, messages.len()); - assert_eq!("

Hello World

", messages[0].body.html.as_ref()); - - recv!( - client, - r#" - - Hello World - - - - - - "# - ); - - room_event!( - client, - room_id.clone(), - ClientRoomEventType::MessagesUpdated { - message_ids: vec![message_id.clone().into()] - } - ); - - client.receive_next().await; - - let messages = room - .load_messages_with_ids(&[message_id.clone().into()]) - .await?; - assert_eq!(1, messages.len()); - assert_eq!("

Hello World

", messages[0].body.html.as_ref()); - - client.pop_ctx(); - client.pop_ctx(); + assert_eq!("Hello World", messages[0].body.raw); Ok(()) } diff --git a/tests/prose-core-integration-tests/src/tests/client/muc_omemo.rs b/tests/prose-core-integration-tests/src/tests/client/muc_omemo.rs index 6579fe08..ce25c6b6 100644 --- a/tests/prose-core-integration-tests/src/tests/client/muc_omemo.rs +++ b/tests/prose-core-integration-tests/src/tests/client/muc_omemo.rs @@ -283,20 +283,20 @@ async fn test_decrypts_message_from_private_nonanonymous_muc_room() -> Result<() "# ); + let message_id = client.get_next_message_id(); + event!(client, ClientEvent::SidebarChanged); room_event!( client, room_id, ClientRoomEventType::MessagesAppended { - message_ids: vec!["my-message-id".into()] + message_ids: vec![message_id.clone()] } ); client.receive_next().await; - let messages = room - .load_messages_with_ids(&["my-message-id".into()]) - .await?; + let messages = room.load_messages_with_ids(&[message_id]).await?; assert_eq!(messages.len(), 1); assert_eq!( messages.first().unwrap().body.html.as_ref(), @@ -584,10 +584,12 @@ async fn test_encrypts_message_in_private_nonanonymous_muc_room() -> Result<()> Some(DeviceBundle::test(account_id!("user2@prose.org"), 200).await), ); + let message_id = client.get_next_message_id(); + send!( client, r#" - + [This message is OMEMO encrypted]
@@ -610,7 +612,7 @@ async fn test_encrypts_message_in_private_nonanonymous_muc_room() -> Result<()> client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -622,12 +624,10 @@ async fn test_encrypts_message_in_private_nonanonymous_muc_room() -> Result<()> }) .await?; - let message_id = client.get_last_id(); - recv!( client, r#" - + [This message is OMEMO encrypted]
@@ -648,14 +648,6 @@ async fn test_encrypts_message_in_private_nonanonymous_muc_room() -> Result<()> "# ); - room_event!( - client, - room_id, - ClientRoomEventType::MessagesUpdated { - message_ids: vec![message_id.clone().into()] - } - ); - client.receive_next().await; let messages = room.load_messages_with_ids(&[message_id.into()]).await?; diff --git a/tests/prose-core-integration-tests/src/tests/client/omemo.rs b/tests/prose-core-integration-tests/src/tests/client/omemo.rs index bcdbfe22..f65e5558 100644 --- a/tests/prose-core-integration-tests/src/tests/client/omemo.rs +++ b/tests/prose-core-integration-tests/src/tests/client/omemo.rs @@ -143,7 +143,7 @@ async fn test_does_not_start_session_when_sending_message_in_non_encrypted_room( send!( client, r#" - + Hello World Hello World @@ -157,7 +157,7 @@ async fn test_does_not_start_session_when_sending_message_in_non_encrypted_room( client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -256,7 +256,7 @@ async fn test_start_session_when_sending_message_in_encrypted_room() -> Result<( send!( client, r#" - + [This message is OMEMO encrypted]
@@ -279,7 +279,7 @@ async fn test_start_session_when_sending_message_in_encrypted_room() -> Result<( client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -296,7 +296,7 @@ async fn test_start_session_when_sending_message_in_encrypted_room() -> Result<( send!( client, r#" - + [This message is OMEMO encrypted]
@@ -319,7 +319,7 @@ async fn test_start_session_when_sending_message_in_encrypted_room() -> Result<( client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -363,7 +363,7 @@ async fn test_starts_session_for_new_devices_when_sending() -> Result<()> { send!( client, r#" - + [This message is OMEMO encrypted]
@@ -384,7 +384,7 @@ async fn test_starts_session_for_new_devices_when_sending() -> Result<()> { client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -425,7 +425,7 @@ async fn test_starts_session_for_new_devices_when_sending() -> Result<()> { send!( client, r#" - + [This message is OMEMO encrypted]
@@ -447,7 +447,7 @@ async fn test_starts_session_for_new_devices_when_sending() -> Result<()> { client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -496,7 +496,7 @@ async fn test_marks_disappeared_devices_as_inactive_and_reappeared_as_active() - send!( client, r#" - + [This message is OMEMO encrypted]
@@ -518,7 +518,7 @@ async fn test_marks_disappeared_devices_as_inactive_and_reappeared_as_active() - client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -574,7 +574,7 @@ async fn test_marks_disappeared_devices_as_inactive_and_reappeared_as_active() - send!( client, r#" - + [This message is OMEMO encrypted]
@@ -596,7 +596,7 @@ async fn test_marks_disappeared_devices_as_inactive_and_reappeared_as_active() - client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -647,7 +647,7 @@ async fn test_marks_disappeared_devices_as_inactive_and_reappeared_as_active() - send!( client, r#" - + [This message is OMEMO encrypted]
@@ -670,7 +670,7 @@ async fn test_marks_disappeared_devices_as_inactive_and_reappeared_as_active() - client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -738,7 +738,7 @@ async fn test_marks_own_disappeared_devices_as_inactive() -> Result<()> { send!( client, r#" - + [This message is OMEMO encrypted]
@@ -761,7 +761,7 @@ async fn test_marks_own_disappeared_devices_as_inactive() -> Result<()> { client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -796,7 +796,7 @@ async fn test_marks_own_disappeared_devices_as_inactive() -> Result<()> { send!( client, r#" - + [This message is OMEMO encrypted]
@@ -818,7 +818,7 @@ async fn test_marks_own_disappeared_devices_as_inactive() -> Result<()> { client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec![client.get_last_id().into()] + message_ids: vec![client.get_last_message_id()] } ); @@ -905,20 +905,20 @@ async fn test_starts_session_and_decrypts_received_messages() -> Result<()> { "# ); + let message_id = client.get_next_message_id(); + event!(client, ClientEvent::SidebarChanged); room_event!( client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec!["my-message-id".into()] + message_ids: vec![message_id.clone()] } ); client.receive_next().await; - let messages = room - .load_messages_with_ids(&["my-message-id".into()]) - .await?; + let messages = room.load_messages_with_ids(&[message_id]).await?; assert_eq!(messages.len(), 1); assert_eq!( messages.first().unwrap().body.html.as_ref(), @@ -957,7 +957,7 @@ async fn test_starts_session_and_decrypts_received_messages() -> Result<()> { recv!( client, r#" - + [This message is OMEMO encrypted] {{ENCRYPTED_PAYLOAD}} @@ -984,12 +984,14 @@ async fn test_starts_session_and_decrypts_received_messages() -> Result<()> { "# ); + let message_id = client.get_next_message_id(); + event!(client, ClientEvent::SidebarChanged); room_event!( client, room.jid().clone(), ClientRoomEventType::MessagesAppended { - message_ids: vec!["other-message-id".into()] + message_ids: vec![message_id.clone()] } ); @@ -997,9 +999,7 @@ async fn test_starts_session_and_decrypts_received_messages() -> Result<()> { // Second message should not contain a pre-key, thus the bundle shouldn't be published again. - let messages = room - .load_messages_with_ids(&["other-message-id".into()]) - .await?; + let messages = room.load_messages_with_ids(&[message_id]).await?; assert_eq!(messages.len(), 1); assert_eq!( messages.first().unwrap().body.html.as_ref(), diff --git a/tests/prose-core-integration-tests/src/tests/client/reactions.rs b/tests/prose-core-integration-tests/src/tests/client/reactions.rs index 37c86745..1990f2ce 100644 --- a/tests/prose-core-integration-tests/src/tests/client/reactions.rs +++ b/tests/prose-core-integration-tests/src/tests/client/reactions.rs @@ -24,6 +24,7 @@ async fn test_reactions() -> Result<()> { .await?; let room_id = muc_id!("room@conf.prose.org"); + let message_id = client.get_next_message_id(); let messages = vec![ Element::from_pretty_printed_xml(r#" @@ -90,9 +91,9 @@ async fn test_reactions() -> Result<()> { .await?; let room = client.get_room(room_id).await.to_generic_room(); - let messages = room.load_messages_with_ids(&["id-1".into()]).await?; + let messages = room.load_messages_with_ids(&[message_id]).await?; - assert_eq!(messages.len(), 1); + assert_eq!(1, messages.len()); let message = messages.get(0).unwrap(); diff --git a/tests/prose-core-integration-tests/src/tests/messages_repository.rs b/tests/prose-core-integration-tests/src/tests/messages_repository.rs index 3c3564e3..03a0c2c8 100644 --- a/tests/prose-core-integration-tests/src/tests/messages_repository.rs +++ b/tests/prose-core-integration-tests/src/tests/messages_repository.rs @@ -23,7 +23,9 @@ async fn test_can_insert_same_message_twice() -> Result<()> { let repo = CachingMessageRepository::new(store().await?); let room_id = RoomId::from(user_id!("a@prose.org")); - let message = MessageBuilder::new_with_index(123).build_message_like(); + let message = MessageBuilder::new_with_index(123) + .set_id("my-message-id") + .build_message_like(); repo.append( &account_id!("account@prose.org"), @@ -40,12 +42,8 @@ async fn test_can_insert_same_message_twice() -> Result<()> { assert_eq!( vec![message.clone()], - repo.get_all( - &account_id!("account@prose.org"), - &room_id, - &[message.id.clone().into_original_id().unwrap()] - ) - .await?, + repo.get_all(&account_id!("account@prose.org"), &room_id, &[message.id]) + .await?, ); Ok(()) @@ -244,9 +242,9 @@ async fn test_load_only_messages_targeting() -> Result<()> { &account_id!("account@prose.org"), &room_id, &[ - MessageTargetId::RemoteId(MessageBuilder::id_for_index(1)), + MessageTargetId::RemoteId(MessageBuilder::remote_id_for_index(1)), MessageTargetId::ServerId(MessageBuilder::stanza_id_for_index(1)), - MessageTargetId::RemoteId(MessageBuilder::id_for_index(2)), + MessageTargetId::RemoteId(MessageBuilder::remote_id_for_index(2)), MessageTargetId::ServerId(MessageBuilder::stanza_id_for_index(2)), ], &Utc.with_ymd_and_hms(2024, 02, 22, 0, 0, 0).unwrap() @@ -291,7 +289,9 @@ async fn test_load_only_messages_targeting_sort_order() -> Result<()> { repo.get_messages_targeting( &account_id!("account@prose.org"), &room_id, - &[MessageTargetId::RemoteId(MessageBuilder::id_for_index(100))], + &[MessageTargetId::RemoteId( + MessageBuilder::remote_id_for_index(100) + )], &DateTime::::default() ) .await? @@ -301,7 +301,7 @@ async fn test_load_only_messages_targeting_sort_order() -> Result<()> { } #[async_test] -async fn test_resolves_message_id() -> Result<()> { +async fn test_resolves_server_id_to_message_id() -> Result<()> { let repo = CachingMessageRepository::new(store().await?); let room_id = RoomId::from(user_id!("a@prose.org")); @@ -312,7 +312,7 @@ async fn test_resolves_message_id() -> Result<()> { assert_eq!( Some(MessageBuilder::id_for_index(101)), - repo.resolve_message_id( + repo.resolve_server_id_to_message_id( &account_id!("account@prose.org"), &room_id, &MessageBuilder::stanza_id_for_index(101) @@ -322,7 +322,7 @@ async fn test_resolves_message_id() -> Result<()> { assert_eq!( None, - repo.resolve_message_id( + repo.resolve_server_id_to_message_id( &account_id!("account@prose.org"), &room_id, &MessageBuilder::stanza_id_for_index(1) @@ -333,6 +333,72 @@ async fn test_resolves_message_id() -> Result<()> { Ok(()) } +#[async_test] +async fn test_resolves_remote_id_to_message_id() -> Result<()> { + let repo = CachingMessageRepository::new(store().await?); + + let room_id = RoomId::from(user_id!("a@prose.org")); + let message = MessageBuilder::new_with_index(101).build_message_like(); + + repo.append(&account_id!("account@prose.org"), &room_id, &[message]) + .await?; + + assert_eq!( + Some(MessageBuilder::id_for_index(101)), + repo.resolve_remote_id_to_message_id( + &account_id!("account@prose.org"), + &room_id, + &MessageBuilder::remote_id_for_index(101) + ) + .await? + ); + + assert_eq!( + None, + repo.resolve_remote_id_to_message_id( + &account_id!("account@prose.org"), + &room_id, + &MessageBuilder::remote_id_for_index(1) + ) + .await? + ); + + Ok(()) +} + +#[async_test] +async fn test_resolves_message_id_to_remote_id() -> Result<()> { + let repo = CachingMessageRepository::new(store().await?); + + let room_id = RoomId::from(user_id!("a@prose.org")); + let message = MessageBuilder::new_with_index(101).build_message_like(); + + repo.append(&account_id!("account@prose.org"), &room_id, &[message]) + .await?; + + assert_eq!( + Some(MessageBuilder::remote_id_for_index(101)), + repo.resolve_message_id_to_remote_id( + &account_id!("account@prose.org"), + &room_id, + &MessageBuilder::id_for_index(101) + ) + .await? + ); + + assert_eq!( + None, + repo.resolve_message_id_to_remote_id( + &account_id!("account@prose.org"), + &room_id, + &MessageBuilder::id_for_index(1) + ) + .await? + ); + + Ok(()) +} + #[async_test] async fn test_get_messages_after() -> Result<()> { let repo = CachingMessageRepository::new(store().await?); @@ -383,9 +449,9 @@ async fn test_get_messages_after() -> Result<()> { assert_eq!( vec![ - MessageBuilder::id_for_index(1), - MessageBuilder::id_for_index(2), - MessageBuilder::id_for_index(3) + MessageBuilder::remote_id_for_index(1), + MessageBuilder::remote_id_for_index(2), + MessageBuilder::remote_id_for_index(3) ], repo.get_messages_after( &account_id!("a@prose.org"), @@ -394,12 +460,12 @@ async fn test_get_messages_after() -> Result<()> { ) .await? .into_iter() - .map(|m| m.id.into_original_id().unwrap()) + .map(|m| m.remote_id.unwrap()) .collect::>() ); assert_eq!( - vec![MessageBuilder::id_for_index(3)], + vec![MessageBuilder::remote_id_for_index(3)], repo.get_messages_after( &account_id!("a@prose.org"), &muc_id!("room2@prose.org").into(), @@ -407,7 +473,7 @@ async fn test_get_messages_after() -> Result<()> { ) .await? .into_iter() - .map(|m| m.id.into_original_id().unwrap()) + .map(|m| m.remote_id.unwrap()) .collect::>() ); @@ -536,7 +602,7 @@ async fn test_loads_latest_received_message() -> Result<()> { &[ MessageBuilder::new_with_index(2) .set_timestamp(Utc.with_ymd_and_hms(2024, 5, 1, 0, 0, 0).unwrap()) - .set_stanza_id(None) + .set_server_id(None) .build_message_like(), MessageBuilder::new_with_index(3) .set_timestamp(Utc.with_ymd_and_hms(2024, 4, 1, 0, 0, 0).unwrap())