From a9b730aaa2e44dbba7c546b0d78ad0fef4884d29 Mon Sep 17 00:00:00 2001 From: Brian Pearce Date: Fri, 15 Sep 2023 09:26:23 +0200 Subject: [PATCH] feat(chatffi): message metadata (#5766) Description --- This adds a metadata field for chat messages. Metadata can be expected to hold information about a message such as it being a reply to a previous message, or it being a token request. Motivation and Context --- Enhanced chat functionality How Has This Been Tested? --- CI / Cucumber. What process can a PR reviewer use to test or verify this change? --- Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify --------- Co-authored-by: SW van Heerden --- Cargo.lock | 2 + base_layer/chat_ffi/chat.h | 52 ++++++- base_layer/chat_ffi/src/lib.rs | 141 +++++++++++++++--- base_layer/contacts/Cargo.toml | 2 + .../examples/chat_client/src/client.rs | 24 ++- .../down.sql | 1 + .../up.sql | 1 + base_layer/contacts/proto/message.proto | 18 ++- .../contacts/src/contacts_service/error.rs | 5 +- .../contacts/src/contacts_service/service.rs | 3 +- .../src/contacts_service/storage/sqlite_db.rs | 2 +- .../storage/types/messages.rs | 34 +++-- .../src/contacts_service/types/message.rs | 81 +++++++++- .../contacts_service/types/message_builder.rs | 27 +++- .../src/contacts_service/types/mod.rs | 2 +- base_layer/contacts/src/schema.rs | 1 + integration_tests/src/chat_ffi.rs | 83 +++++++---- .../tests/features/ChatFFI.feature | 10 ++ integration_tests/tests/steps/chat_steps.rs | 98 +++++++++++- 19 files changed, 500 insertions(+), 87 deletions(-) create mode 100644 base_layer/contacts/migrations/2023-09-05-132343_add_metadata_to_messages/down.sql create mode 100644 base_layer/contacts/migrations/2023-09-05-132343_add_metadata_to_messages/up.sql diff --git a/Cargo.lock b/Cargo.lock index 3dcbc8fc60..a8b97e5cf6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5599,6 +5599,8 @@ dependencies = [ "num-traits", "prost 0.9.0", "rand 0.8.5", + "serde", + "serde_json", "tari_common", "tari_common_sqlite", "tari_common_types", diff --git a/base_layer/chat_ffi/chat.h b/base_layer/chat_ffi/chat.h index f7db268629..d8bf0dbbbd 100644 --- a/base_layer/chat_ffi/chat.h +++ b/base_layer/chat_ffi/chat.h @@ -16,6 +16,8 @@ struct ChatClientFFI; struct ChatMessages; +struct Message; + struct TariAddress; struct TransportConfig; @@ -116,20 +118,56 @@ void destroy_chat_config(struct ApplicationConfig *config); * * ## Arguments * `client` - The Client pointer + * `message` - Pointer to a Message struct + * `error_out` - Pointer to an int which will be modified + * + * ## Returns + * `()` - Does not return a value, equivalent to void in C + * + * # Safety + * The ```message``` should be destroyed after use + */ +void send_chat_message(struct ChatClientFFI *client, struct Message *message, int *error_out); + +/** + * Creates a message and returns a ptr to it + * + * ## Arguments * `receiver` - A string containing a tari address * `message` - The peer seeds config for the node * `error_out` - Pointer to an int which will be modified * * ## Returns - * `()` - Does not return a value, equivalent to void in C + * `*mut Message` - A pointer to a message object * * # Safety * The ```receiver``` should be destroyed after use */ -void send_chat_message(struct ChatClientFFI *client, - struct TariAddress *receiver, - const char *message_c_char, - int *error_out); +struct Message *create_chat_message(struct TariAddress *receiver, + const char *message, + int *error_out); + +/** + * Creates message metadata and appends it to a Message + * + * ## Arguments + * `message` - A pointer to a message + * `metadata_type` - An int8 that maps to MessageMetadataType enum + * '0' -> Reply + * '1' -> TokenRequest + * `data` - contents for the metadata in string format + * `error_out` - Pointer to an int which will be modified + * + * ## Returns + * `()` - Does not return a value, equivalent to void in C + * + * ## Safety + * `message` should be destroyed eventually + */ +void add_chat_message_metadata(struct Message *message, + const int *metadata_type, + const char *data, + int *error_out); /** * Add a contact @@ -143,9 +181,9 @@ void send_chat_message(struct ChatClientFFI *client, * `()` - Does not return a value, equivalent to void in C * * # Safety - * The ```address``` should be destroyed after use + * The ```receiver``` should be destroyed after use */ -void add_chat_contact(struct ChatClientFFI *client, struct TariAddress *receiver, int *error_out); +void add_chat_contact(struct ChatClientFFI *client, struct TariAddress *address, int *error_out); /** * Check the online status of a contact diff --git a/base_layer/chat_ffi/src/lib.rs b/base_layer/chat_ffi/src/lib.rs index 195c66fca7..75191ec682 100644 --- a/base_layer/chat_ffi/src/lib.rs +++ b/base_layer/chat_ffi/src/lib.rs @@ -50,7 +50,7 @@ use tari_common_types::tari_address::TariAddress; use tari_comms::multiaddr::Multiaddr; use tari_contacts::contacts_service::{ handle::{DEFAULT_MESSAGE_LIMIT, DEFAULT_MESSAGE_PAGE}, - types::Message, + types::{Message, MessageBuilder, MessageMetadata, MessageMetadataType}, }; use tari_p2p::{SocksAuthentication, TorControlAuthentication, TorTransportConfig, TransportConfig, TransportType}; use tari_utilities::hex; @@ -485,22 +485,16 @@ unsafe fn init_logging(log_path: PathBuf, error_out: *mut c_int) { /// /// ## Arguments /// `client` - The Client pointer -/// `receiver` - A string containing a tari address -/// `message` - The peer seeds config for the node +/// `message` - Pointer to a Message struct /// `error_out` - Pointer to an int which will be modified /// /// ## Returns /// `()` - Does not return a value, equivalent to void in C /// /// # Safety -/// The ```receiver``` should be destroyed after use +/// The ```message``` should be destroyed after use #[no_mangle] -pub unsafe extern "C" fn send_chat_message( - client: *mut ChatClientFFI, - receiver: *mut TariAddress, - message_c_char: *const c_char, - error_out: *mut c_int, -) { +pub unsafe extern "C" fn send_chat_message(client: *mut ChatClientFFI, message: *mut Message, error_out: *mut c_int) { let mut error = 0; ptr::swap(error_out, &mut error as *mut c_int); @@ -509,23 +503,119 @@ pub unsafe extern "C" fn send_chat_message( ptr::swap(error_out, &mut error as *mut c_int); } + if message.is_null() { + error = LibChatError::from(InterfaceError::NullError("message".to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + } + + (*client) + .runtime + .block_on((*client).client.send_message((*message).clone())); +} + +/// Creates a message and returns a ptr to it +/// +/// ## Arguments +/// `receiver` - A string containing a tari address +/// `message` - The peer seeds config for the node +/// `error_out` - Pointer to an int which will be modified +/// +/// ## Returns +/// `*mut Message` - A pointer to a message object +/// +/// # Safety +/// The ```receiver``` should be destroyed after use +#[no_mangle] +pub unsafe extern "C" fn create_chat_message( + receiver: *mut TariAddress, + message: *const c_char, + error_out: *mut c_int, +) -> *mut Message { + let mut error = 0; + ptr::swap(error_out, &mut error as *mut c_int); + if receiver.is_null() { error = LibChatError::from(InterfaceError::NullError("receiver".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); } - let message = match CStr::from_ptr(message_c_char).to_str() { + let message_str = match CStr::from_ptr(message).to_str() { Ok(str) => str.to_string(), Err(e) => { error = LibChatError::from(InterfaceError::InvalidArgument(e.to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); + return ptr::null_mut(); + }, + }; + + let message_out = MessageBuilder::new() + .address((*receiver).clone()) + .message(message_str) + .build(); + + Box::into_raw(Box::new(message_out)) +} + +/// Creates message metadata and appends it to a Message +/// +/// ## Arguments +/// `message` - A pointer to a message +/// `metadata_type` - An int8 that maps to MessageMetadataType enum +/// '0' -> Reply +/// '1' -> TokenRequest +/// `data` - contents for the metadata in string format +/// `error_out` - Pointer to an int which will be modified +/// +/// ## Returns +/// `()` - Does not return a value, equivalent to void in C +/// +/// ## Safety +/// `message` should be destroyed eventually +#[no_mangle] +pub unsafe extern "C" fn add_chat_message_metadata( + message: *mut Message, + metadata_type: *const c_int, + data: *const c_char, + error_out: *mut c_int, +) { + let mut error = 0; + ptr::swap(error_out, &mut error as *mut c_int); + + if message.is_null() { + error = LibChatError::from(InterfaceError::NullError("message".to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + return; + } + + let metadata_type = match MessageMetadataType::from_byte(metadata_type as u8) { + Some(t) => t, + None => { + error = LibChatError::from(InterfaceError::InvalidArgument( + "Couldn't convert byte to Metadata type".to_string(), + )) + .code; + ptr::swap(error_out, &mut error as *mut c_int); return; }, }; - (*client) - .runtime - .block_on((*client).client.send_message((*receiver).clone(), message)); + if data.is_null() { + error = LibChatError::from(InterfaceError::NullError("data".to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + return; + } + + let data: Vec = match CStr::from_ptr(data).to_str() { + Ok(str) => str.as_bytes().into(), + Err(e) => { + error = LibChatError::from(InterfaceError::InvalidArgument(e.to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + return; + }, + }; + + let metadata = MessageMetadata { metadata_type, data }; + (*message).push(metadata); } /// Add a contact @@ -539,11 +629,11 @@ pub unsafe extern "C" fn send_chat_message( /// `()` - Does not return a value, equivalent to void in C /// /// # Safety -/// The ```address``` should be destroyed after use +/// The ```receiver``` should be destroyed after use #[no_mangle] pub unsafe extern "C" fn add_chat_contact( client: *mut ChatClientFFI, - receiver: *mut TariAddress, + address: *mut TariAddress, error_out: *mut c_int, ) { let mut error = 0; @@ -554,12 +644,12 @@ pub unsafe extern "C" fn add_chat_contact( ptr::swap(error_out, &mut error as *mut c_int); } - if receiver.is_null() { + if address.is_null() { error = LibChatError::from(InterfaceError::NullError("receiver".to_string())).code; ptr::swap(error_out, &mut error as *mut c_int); } - (*client).runtime.block_on((*client).client.add_contact(&(*receiver))); + (*client).runtime.block_on((*client).client.add_contact(&(*address))); } /// Check the online status of a contact @@ -969,4 +1059,19 @@ mod test { destroy_chat_tor_transport_config(transport_config); } } + + #[test] + fn test_metadata_adding() { + let message_ptr = Box::into_raw(Box::default()); + + let data_c_str = CString::new("hello".to_string()).unwrap(); + let data_char: *const c_char = CString::into_raw(data_c_str) as *const c_char; + + let error_out = Box::into_raw(Box::new(0)); + + unsafe { add_chat_message_metadata(message_ptr, 1 as *const c_int, data_char, error_out) } + + let message = unsafe { Box::from_raw(message_ptr) }; + assert_eq!(message.metadata.len(), 1) + } } diff --git a/base_layer/contacts/Cargo.toml b/base_layer/contacts/Cargo.toml index 8c88c3766b..46b23d91c8 100644 --- a/base_layer/contacts/Cargo.toml +++ b/base_layer/contacts/Cargo.toml @@ -27,6 +27,8 @@ num-derive = "0.3.3" num-traits = "0.2.15" prost = "0.9" rand = "0.8" +serde = "1.0.136" +serde_json = "1.0.79" thiserror = "1.0.26" tokio = { version = "1.23", features = ["sync", "macros"] } tower = "0.4" diff --git a/base_layer/contacts/examples/chat_client/src/client.rs b/base_layer/contacts/examples/chat_client/src/client.rs index 1e80fff872..9fe6b475e1 100644 --- a/base_layer/contacts/examples/chat_client/src/client.rs +++ b/base_layer/contacts/examples/chat_client/src/client.rs @@ -33,7 +33,7 @@ use tari_comms::{CommsNode, NodeIdentity}; use tari_contacts::contacts_service::{ handle::ContactsServiceHandle, service::ContactOnlineStatus, - types::{Message, MessageBuilder}, + types::{Message, MessageBuilder, MessageMetadata, MessageMetadataType}, }; use tari_shutdown::Shutdown; @@ -44,9 +44,11 @@ const LOG_TARGET: &str = "contacts::chat_client"; #[async_trait] pub trait ChatClient { async fn add_contact(&self, address: &TariAddress); + fn add_metadata(&self, message: Message, metadata_type: MessageMetadataType, data: String) -> Message; async fn check_online_status(&self, address: &TariAddress) -> ContactOnlineStatus; - async fn send_message(&self, receiver: TariAddress, message: String); + fn create_message(&self, receiver: &TariAddress, message: String) -> Message; async fn get_messages(&self, sender: &TariAddress, limit: u64, page: u64) -> Vec; + async fn send_message(&self, message: Message); fn identity(&self) -> &NodeIdentity; fn shutdown(&mut self); } @@ -148,10 +150,10 @@ impl ChatClient for Client { ContactOnlineStatus::Offline } - async fn send_message(&self, receiver: TariAddress, message: String) { + async fn send_message(&self, message: Message) { if let Some(mut contacts_service) = self.contacts.clone() { contacts_service - .send_message(MessageBuilder::new().message(message).address(receiver).build()) + .send_message(message) .await .expect("Message wasn't sent"); } @@ -168,6 +170,20 @@ impl ChatClient for Client { messages } + + fn create_message(&self, receiver: &TariAddress, message: String) -> Message { + MessageBuilder::new().address(receiver.clone()).message(message).build() + } + + fn add_metadata(&self, mut message: Message, metadata_type: MessageMetadataType, data: String) -> Message { + let metadata = MessageMetadata { + metadata_type, + data: data.into_bytes(), + }; + + message.push(metadata); + message + } } pub async fn wait_for_connectivity(comms: CommsNode) -> anyhow::Result<()> { diff --git a/base_layer/contacts/migrations/2023-09-05-132343_add_metadata_to_messages/down.sql b/base_layer/contacts/migrations/2023-09-05-132343_add_metadata_to_messages/down.sql new file mode 100644 index 0000000000..edfc520ef0 --- /dev/null +++ b/base_layer/contacts/migrations/2023-09-05-132343_add_metadata_to_messages/down.sql @@ -0,0 +1 @@ +ALTER TABLE contacts drop metadata; diff --git a/base_layer/contacts/migrations/2023-09-05-132343_add_metadata_to_messages/up.sql b/base_layer/contacts/migrations/2023-09-05-132343_add_metadata_to_messages/up.sql new file mode 100644 index 0000000000..a0c48f44fa --- /dev/null +++ b/base_layer/contacts/migrations/2023-09-05-132343_add_metadata_to_messages/up.sql @@ -0,0 +1 @@ + ALTER TABLE messages ADD metadata BLOB; diff --git a/base_layer/contacts/proto/message.proto b/base_layer/contacts/proto/message.proto index e528929527..41301eb01c 100644 --- a/base_layer/contacts/proto/message.proto +++ b/base_layer/contacts/proto/message.proto @@ -6,13 +6,23 @@ package tari.contacts.chat; message Message { bytes body = 1; - bytes address = 2; - DirectionEnum direction = 3; - uint64 stored_at = 4; - bytes message_id = 5; + repeated MessageMetadata metadata = 2; + bytes address = 3; + DirectionEnum direction = 4; + uint64 stored_at = 5; + bytes message_id = 6; } enum DirectionEnum { Inbound = 0; Outbound = 1; +} + +message MessageMetadata { + MessageTypeEnum metadata_type = 1; + bytes data = 2; +} + +enum MessageTypeEnum { + TokenRequest = 0; } \ No newline at end of file diff --git a/base_layer/contacts/src/contacts_service/error.rs b/base_layer/contacts/src/contacts_service/error.rs index ea1c3c9cba..1112f675da 100644 --- a/base_layer/contacts/src/contacts_service/error.rs +++ b/base_layer/contacts/src/contacts_service/error.rs @@ -22,7 +22,6 @@ use diesel::result::Error as DieselError; use tari_common_sqlite::error::SqliteStorageError; -use tari_common_types::tari_address::TariAddressError; use tari_comms::connectivity::ConnectivityError; use tari_comms_dht::outbound::DhtOutboundError; use tari_p2p::services::liveness::error::LivenessError; @@ -48,8 +47,8 @@ pub enum ContactsServiceError { ConnectivityError(#[from] ConnectivityError), #[error("Outbound comms error: `{0}`")] OutboundCommsError(#[from] DhtOutboundError), - #[error("Error parsing address: `{source}`")] - MessageParsingError { source: TariAddressError }, + #[error("Error parsing address: `{0}`")] + MessageParsingError(String), #[error("Error decoding message: `{0}`")] MalformedMessageError(#[from] prost::DecodeError), #[error("Message source does not match authenticated origin")] diff --git a/base_layer/contacts/src/contacts_service/service.rs b/base_layer/contacts/src/contacts_service/service.rs index ee6c2929a3..47c57f6ac6 100644 --- a/base_layer/contacts/src/contacts_service/service.rs +++ b/base_layer/contacts/src/contacts_service/service.rs @@ -421,8 +421,7 @@ where T: ContactsBackend + 'static }, }; if let Some(source_public_key) = msg.authenticated_origin { - let message = - Message::try_from(msg_inner).map_err(|ta| ContactsServiceError::MessageParsingError { source: ta })?; + let message = Message::try_from(msg_inner).map_err(ContactsServiceError::MessageParsingError)?; let our_message = Message { address: TariAddress::from_public_key(&source_public_key, message.address.network()), diff --git a/base_layer/contacts/src/contacts_service/storage/sqlite_db.rs b/base_layer/contacts/src/contacts_service/storage/sqlite_db.rs index c47c566a78..aba31a0da0 100644 --- a/base_layer/contacts/src/contacts_service/storage/sqlite_db.rs +++ b/base_layer/contacts/src/contacts_service/storage/sqlite_db.rs @@ -176,7 +176,7 @@ where TContactServiceDbConnection: PooledDbConnection { if let DbValue::Message(m) = *i { - MessagesSqlInsert::from(*m).commit(&mut conn)?; + MessagesSqlInsert::try_from(*m)?.commit(&mut conn)?; } }, } diff --git a/base_layer/contacts/src/contacts_service/storage/types/messages.rs b/base_layer/contacts/src/contacts_service/storage/types/messages.rs index 3f6f9226d7..47506388bd 100644 --- a/base_layer/contacts/src/contacts_service/storage/types/messages.rs +++ b/base_layer/contacts/src/contacts_service/storage/types/messages.rs @@ -24,12 +24,13 @@ use std::convert::TryFrom; use chrono::NaiveDateTime; use diesel::{prelude::*, SqliteConnection}; +use serde_json; use tari_common_types::tari_address::TariAddress; use crate::{ contacts_service::{ error::ContactsServiceStorageError, - types::{Direction, Message}, + types::{Direction, Message, MessageMetadata}, }, schema::messages, }; @@ -40,10 +41,11 @@ use crate::{ #[diesel(primary_key(message_id))] pub struct MessagesSqlInsert { pub address: Vec, + pub message_id: Vec, pub body: Vec, - pub direction: i32, + pub metadata: Vec, pub stored_at: NaiveDateTime, - pub message_id: Vec, + pub direction: i32, } #[derive(Clone, Debug, Queryable, PartialEq, Eq, QueryableByName)] @@ -53,6 +55,7 @@ pub struct MessagesSql { pub address: Vec, pub message_id: Vec, pub body: Vec, + pub metadata: Vec, pub stored_at: NaiveDateTime, pub direction: i32, } @@ -91,6 +94,11 @@ impl TryFrom for Message { #[allow(clippy::cast_sign_loss)] fn try_from(o: MessagesSql) -> Result { let address = TariAddress::from_bytes(&o.address).map_err(|_| ContactsServiceStorageError::ConversionError)?; + let metadata: Vec = serde_json::from_str( + &String::from_utf8(o.metadata.clone()).map_err(|_| ContactsServiceStorageError::ConversionError)?, + ) + .map_err(|_| ContactsServiceStorageError::ConversionError)?; + Ok(Self { address, direction: Direction::from_byte( @@ -99,6 +107,7 @@ impl TryFrom for Message { .unwrap_or_else(|| panic!("Direction from byte {}", o.direction)), stored_at: o.stored_at.timestamp() as u64, body: o.body, + metadata, message_id: o.message_id, }) } @@ -106,14 +115,19 @@ impl TryFrom for Message { /// Conversion from a Contact to the Sql datatype form #[allow(clippy::cast_possible_wrap)] -impl From for MessagesSqlInsert { - fn from(o: Message) -> Self { - Self { +impl TryFrom for MessagesSqlInsert { + type Error = ContactsServiceStorageError; + + fn try_from(o: Message) -> Result { + let metadata = serde_json::to_string(&o.metadata).map_err(|_| ContactsServiceStorageError::ConversionError)?; + + Ok(Self { address: o.address.to_bytes().to_vec(), - direction: i32::from(o.direction.as_byte()), - stored_at: NaiveDateTime::from_timestamp_opt(o.stored_at as i64, 0).unwrap(), - body: o.body, message_id: o.message_id, - } + body: o.body, + metadata: metadata.into_bytes().to_vec(), + stored_at: NaiveDateTime::from_timestamp_opt(o.stored_at as i64, 0).unwrap(), + direction: i32::from(o.direction.as_byte()), + }) } } diff --git a/base_layer/contacts/src/contacts_service/types/message.rs b/base_layer/contacts/src/contacts_service/types/message.rs index 087eb07dad..46d9fc3767 100644 --- a/base_layer/contacts/src/contacts_service/types/message.rs +++ b/base_layer/contacts/src/contacts_service/types/message.rs @@ -24,7 +24,8 @@ use std::convert::TryFrom; use num_derive::FromPrimitive; use num_traits::FromPrimitive; -use tari_common_types::tari_address::{TariAddress, TariAddressError}; +use serde::{Deserialize, Serialize}; +use tari_common_types::tari_address::TariAddress; use tari_comms_dht::domain_message::OutboundDomainMessage; use tari_p2p::tari_message::TariMessageType; use tari_utilities::ByteArray; @@ -34,16 +35,24 @@ use crate::contacts_service::proto; #[derive(Clone, Debug, Default)] pub struct Message { pub body: Vec, + pub metadata: Vec, pub address: TariAddress, pub direction: Direction, pub stored_at: u64, pub message_id: Vec, } +impl Message { + pub fn push(&mut self, metadata: MessageMetadata) { + self.metadata.push(metadata) + } +} + #[repr(u8)] -#[derive(FromPrimitive, Debug, Copy, Clone)] +#[derive(FromPrimitive, Debug, Copy, Clone, Default, PartialEq)] pub enum Direction { Inbound = 0, + #[default] Outbound = 1, } @@ -57,19 +66,46 @@ impl Direction { } } -impl Default for Direction { - fn default() -> Self { - Self::Outbound +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +pub struct MessageMetadata { + pub metadata_type: MessageMetadataType, + pub data: Vec, +} + +#[repr(u8)] +#[derive(FromPrimitive, Debug, Copy, Clone, Default, Deserialize, Serialize, PartialEq)] +pub enum MessageMetadataType { + Reply = 0, + #[default] + TokenRequest = 1, +} + +impl MessageMetadataType { + pub fn as_byte(self) -> u8 { + self as u8 + } + + pub fn from_byte(value: u8) -> Option { + FromPrimitive::from_u8(value) } } impl TryFrom for Message { - type Error = TariAddressError; + type Error = String; fn try_from(message: proto::Message) -> Result { + let mut metadata = vec![]; + for m in message.metadata { + match MessageMetadata::try_from(m) { + Ok(md) => metadata.push(md), + Err(e) => return Err(e), + } + } + Ok(Self { body: message.body, - address: TariAddress::from_bytes(&message.address)?, + metadata, + address: TariAddress::from_bytes(&message.address).map_err(|e| e.to_string())?, // A Message from a proto::Message will always be an inbound message direction: Direction::Inbound, stored_at: message.stored_at, @@ -82,6 +118,11 @@ impl From for proto::Message { fn from(message: Message) -> Self { Self { body: message.body, + metadata: message + .metadata + .iter() + .map(|m| proto::MessageMetadata::from(m.clone())) + .collect(), address: message.address.to_bytes().to_vec(), direction: i32::from(message.direction.as_byte()), stored_at: message.stored_at, @@ -95,3 +136,29 @@ impl From for OutboundDomainMessage { Self::new(&TariMessageType::Chat, message.into()) } } + +impl TryFrom for MessageMetadata { + type Error = String; + + fn try_from(md: proto::MessageMetadata) -> Result { + if let Some(md_type) = + MessageMetadataType::from_byte(u8::try_from(md.metadata_type).map_err(|e| e.to_string())?) + { + Ok(Self { + data: md.data, + metadata_type: md_type, + }) + } else { + Err("Not a valid metadata type".into()) + } + } +} + +impl From for proto::MessageMetadata { + fn from(md: MessageMetadata) -> Self { + Self { + data: md.data, + metadata_type: i32::from(md.metadata_type.as_byte()), + } + } +} diff --git a/base_layer/contacts/src/contacts_service/types/message_builder.rs b/base_layer/contacts/src/contacts_service/types/message_builder.rs index 96e95ee3e5..638c20362a 100644 --- a/base_layer/contacts/src/contacts_service/types/message_builder.rs +++ b/base_layer/contacts/src/contacts_service/types/message_builder.rs @@ -21,10 +21,9 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. use tari_common_types::tari_address::TariAddress; -use tari_utilities::ByteArray; use uuid::Uuid; -use crate::contacts_service::types::Message; +use crate::contacts_service::types::{message::MessageMetadata, Message}; #[derive(Clone, Debug, Default)] pub struct MessageBuilder { @@ -33,7 +32,9 @@ pub struct MessageBuilder { impl MessageBuilder { pub fn new() -> Self { - let message_id = Uuid::new_v4().into_bytes().to_vec(); + // We're forcing it to a String before bytes so we can have the same representation used in + // all places. Otherwise the UUID byte format will differ if displayed somewhere. + let message_id = Uuid::new_v4().to_string().into_bytes(); Self { inner: Message { @@ -62,7 +63,27 @@ impl MessageBuilder { } } + pub fn metadata(&self, new_metadata: MessageMetadata) -> Self { + let mut metadata = self.inner.metadata.clone(); + metadata.push(new_metadata); + + Self { + inner: Message { + metadata, + ..self.inner.clone() + }, + } + } + pub fn build(&self) -> Message { self.inner.clone() } } + +impl From for MessageBuilder { + fn from(message: Message) -> Self { + Self { + inner: Message { ..message }, + } + } +} diff --git a/base_layer/contacts/src/contacts_service/types/mod.rs b/base_layer/contacts/src/contacts_service/types/mod.rs index 89b83734b5..d2bc7105af 100644 --- a/base_layer/contacts/src/contacts_service/types/mod.rs +++ b/base_layer/contacts/src/contacts_service/types/mod.rs @@ -24,7 +24,7 @@ mod contact; pub use contact::Contact; mod message; -pub use message::{Direction, Message}; +pub use message::{Direction, Message, MessageMetadata, MessageMetadataType}; mod message_builder; pub use message_builder::MessageBuilder; diff --git a/base_layer/contacts/src/schema.rs b/base_layer/contacts/src/schema.rs index 25f9d3e061..f3b921bca3 100644 --- a/base_layer/contacts/src/schema.rs +++ b/base_layer/contacts/src/schema.rs @@ -16,6 +16,7 @@ diesel::table! { address -> Binary, message_id -> Binary, body -> Binary, + metadata -> Binary, stored_at -> Timestamp, direction -> Integer, } diff --git a/integration_tests/src/chat_ffi.rs b/integration_tests/src/chat_ffi.rs index 9dd2f77f77..ecb767aabf 100644 --- a/integration_tests/src/chat_ffi.rs +++ b/integration_tests/src/chat_ffi.rs @@ -41,7 +41,10 @@ use tari_comms::{ peer_manager::{Peer, PeerFeatures}, NodeIdentity, }; -use tari_contacts::contacts_service::{service::ContactOnlineStatus, types::Message}; +use tari_contacts::contacts_service::{ + service::ContactOnlineStatus, + types::{Message, MessageMetadataType}, +}; use crate::{chat_client::test_config, get_port}; @@ -60,24 +63,26 @@ extern "C" fn callback_message_received(_state: *mut c_void) { extern "C" { pub fn create_chat_client( config: *mut c_void, - out_error: *const c_int, + error_out: *const c_int, callback_contact_status_change: unsafe extern "C" fn(*mut c_void), callback_message_received: unsafe extern "C" fn(*mut c_void), ) -> *mut ClientFFI; - pub fn send_chat_message( - client: *mut ClientFFI, - receiver: *mut c_void, - message: *const c_char, - out_error: *const c_int, - ); - pub fn add_chat_contact(client: *mut ClientFFI, address: *mut c_void, out_error: *const c_int); - pub fn check_online_status(client: *mut ClientFFI, address: *mut c_void, out_error: *const c_int) -> c_int; + pub fn create_chat_message(receiver: *mut c_void, message: *const c_char, error_out: *const c_int) -> *mut c_void; + pub fn send_chat_message(client: *mut ClientFFI, message: *mut c_void, error_out: *const c_int); + pub fn add_chat_message_metadata( + message: *mut c_void, + metadata_type: *const c_int, + data: *const c_char, + error_out: *const c_int, + ) -> *mut c_void; + pub fn add_chat_contact(client: *mut ClientFFI, address: *mut c_void, error_out: *const c_int); + pub fn check_online_status(client: *mut ClientFFI, address: *mut c_void, error_out: *const c_int) -> c_int; pub fn get_chat_messages( client: *mut ClientFFI, sender: *mut c_void, limit: *mut c_void, page: *mut c_void, - out_error: *const c_int, + error_out: *const c_int, ) -> *mut c_void; pub fn destroy_chat_client_ffi(client: *mut ClientFFI); } @@ -99,8 +104,8 @@ impl ChatClient for ChatFFI { let address_ptr = Box::into_raw(Box::new(address.to_owned())) as *mut c_void; - let out_error = Box::into_raw(Box::new(0)); - unsafe { add_chat_contact(client.0, address_ptr, out_error) } + let error_out = Box::into_raw(Box::new(0)); + unsafe { add_chat_contact(client.0, address_ptr, error_out) } } async fn check_online_status(&self, address: &TariAddress) -> ContactOnlineStatus { @@ -109,23 +114,20 @@ impl ChatClient for ChatFFI { let address_ptr = Box::into_raw(Box::new(address.clone())) as *mut c_void; let result; - let out_error = Box::into_raw(Box::new(0)); - unsafe { result = check_online_status(client.0, address_ptr, out_error) } + let error_out = Box::into_raw(Box::new(0)); + unsafe { result = check_online_status(client.0, address_ptr, error_out) } ContactOnlineStatus::from_byte(u8::try_from(result).unwrap()).expect("A valid u8 from FFI status") } - async fn send_message(&self, receiver: TariAddress, message: String) { + async fn send_message(&self, message: Message) { let client = self.ptr.lock().unwrap(); - let message_c_str = CString::new(message).unwrap(); - let message_c_char: *const c_char = CString::into_raw(message_c_str) as *const c_char; - - let receiver_ptr = Box::into_raw(Box::new(receiver)) as *mut c_void; - let out_error = Box::into_raw(Box::new(0)); + let error_out = Box::into_raw(Box::new(0)); + let message_ptr = Box::into_raw(Box::new(message)) as *mut c_void; unsafe { - send_chat_message(client.0, receiver_ptr, message_c_char, out_error); + send_chat_message(client.0, message_ptr, error_out); } } @@ -136,16 +138,45 @@ impl ChatClient for ChatFFI { let messages; unsafe { - let out_error = Box::into_raw(Box::new(0)); + let error_out = Box::into_raw(Box::new(0)); let limit = Box::into_raw(Box::new(limit)) as *mut c_void; let page = Box::into_raw(Box::new(page)) as *mut c_void; - let all_messages = get_chat_messages(client.0, address_ptr, limit, page, out_error) as *mut Vec; + let all_messages = get_chat_messages(client.0, address_ptr, limit, page, error_out) as *mut Vec; messages = (*all_messages).clone(); } messages } + fn create_message(&self, receiver: &TariAddress, message: String) -> Message { + let address_ptr = Box::into_raw(Box::new(receiver.to_owned())) as *mut c_void; + + let message_c_str = CString::new(message).unwrap(); + let message_c_char: *const c_char = CString::into_raw(message_c_str) as *const c_char; + + let error_out = Box::into_raw(Box::new(0)); + + unsafe { + let message_ptr = create_chat_message(address_ptr, message_c_char, error_out) as *mut Message; + *Box::from_raw(message_ptr) + } + } + + fn add_metadata(&self, message: Message, metadata_type: MessageMetadataType, data: String) -> Message { + let message_ptr = Box::into_raw(Box::new(message)) as *mut c_void; + let message_type = metadata_type.as_byte() as *const c_int; + + let data_c_str = CString::new(data).unwrap(); + let data_c_char: *const c_char = CString::into_raw(data_c_str) as *const c_char; + + let error_out = Box::into_raw(Box::new(0)); + + unsafe { + add_chat_message_metadata(message_ptr, message_type, data_c_char, error_out); + *Box::from_raw(message_ptr as *mut Message) + } + } + fn identity(&self) -> &NodeIdentity { &self.identity } @@ -189,14 +220,14 @@ pub async fn spawn_ffi_chat_client(name: &str, seed_peers: Vec, base_dir: let client_ptr; - let out_error = Box::into_raw(Box::new(0)); + let error_out = Box::into_raw(Box::new(0)); unsafe { *ChatCallback::instance().contact_status_change.lock().unwrap() = 0; client_ptr = create_chat_client( config_ptr, - out_error, + error_out, callback_contact_status_change, callback_message_received, ); diff --git a/integration_tests/tests/features/ChatFFI.feature b/integration_tests/tests/features/ChatFFI.feature index 3842ff156b..4a98580b69 100644 --- a/integration_tests/tests/features/ChatFFI.feature +++ b/integration_tests/tests/features/ChatFFI.feature @@ -45,3 +45,13 @@ Feature: Chat FFI messaging Given I have a seed node SEED_A When I have a chat FFI client CHAT_A connected to seed node SEED_A Then I can shutdown CHAT_A without a problem + + Scenario: Reply to message + Given I have a seed node SEED_A + When I have a chat FFI client CHAT_A connected to seed node SEED_A + When I have a chat FFI client CHAT_B connected to seed node SEED_A + When I use CHAT_A to send a message 'Hey there' to CHAT_B + When I use CHAT_B to send a reply saying 'oh hai' to CHAT_A's message 'Hey there' + Then CHAT_B will have 2 messages with CHAT_A + Then CHAT_A will have 2 messages with CHAT_B + Then CHAT_A will have a replied to message from CHAT_B with 'oh hai' diff --git a/integration_tests/tests/steps/chat_steps.rs b/integration_tests/tests/steps/chat_steps.rs index 5fbe417efa..7dc277e2b0 100644 --- a/integration_tests/tests/steps/chat_steps.rs +++ b/integration_tests/tests/steps/chat_steps.rs @@ -28,6 +28,7 @@ use tari_common_types::tari_address::TariAddress; use tari_contacts::contacts_service::{ handle::{DEFAULT_MESSAGE_LIMIT, DEFAULT_MESSAGE_PAGE}, service::ContactOnlineStatus, + types::{Direction, Message, MessageMetadata, MessageMetadataType}, }; use tari_integration_tests::{chat_client::spawn_chat_client, TariWorld}; @@ -65,7 +66,52 @@ async fn send_message_to(world: &mut TariWorld, sender: String, message: String, let receiver = world.chat_clients.get(&receiver).unwrap(); let address = TariAddress::from_public_key(receiver.identity().public_key(), Network::LocalNet); - sender.send_message(address, message).await; + let message = sender.create_message(&address, message); + + sender.send_message(message).await; +} + +#[when(regex = r"^I use (.+) to send a reply saying '(.+)' to (.*)'s message '(.*)'$")] +async fn i_reply_to_message( + world: &mut TariWorld, + sender: String, + outbound_msg: String, + receiver: String, + inbound_msg: String, +) { + let sender = world.chat_clients.get(&sender).unwrap(); + let receiver = world.chat_clients.get(&receiver).unwrap(); + let address = TariAddress::from_public_key(receiver.identity().public_key(), Network::LocalNet); + + for _ in 0..(TWO_MINUTES_WITH_HALF_SECOND_SLEEP) { + let messages: Vec = (*sender) + .get_messages(&address, DEFAULT_MESSAGE_LIMIT, DEFAULT_MESSAGE_PAGE) + .await; + + if messages.is_empty() { + tokio::time::sleep(Duration::from_millis(HALF_SECOND)).await; + continue; + } + + let inbound_chat_message = messages + .iter() + .find(|m| m.body == inbound_msg.clone().into_bytes()) + .expect("no message with that content found") + .clone(); + + let message = sender.create_message(&address, outbound_msg); + + let message = sender.add_metadata( + message, + MessageMetadataType::Reply, + String::from_utf8(inbound_chat_message.message_id).expect("bytes to uuid"), + ); + + sender.send_message(message).await; + return; + } + + panic!("Never received incoming chat message",) } #[then(expr = "{word} will have {int} message(s) with {word}")] @@ -128,3 +174,53 @@ async fn wait_for_contact_to_be_online(world: &mut TariWorld, client: String, co last_status ) } + +#[then(regex = r"^(.+) will have a replied to message from (.*) with '(.*)'$")] +async fn have_replied_message(world: &mut TariWorld, receiver: String, sender: String, inbound_reply: String) { + let receiver = world.chat_clients.get(&receiver).unwrap(); + let sender = world.chat_clients.get(&sender).unwrap(); + let address = TariAddress::from_public_key(sender.identity().public_key(), Network::LocalNet); + + for _ in 0..(TWO_MINUTES_WITH_HALF_SECOND_SLEEP) { + let messages: Vec = (*receiver) + .get_messages(&address, DEFAULT_MESSAGE_LIMIT, DEFAULT_MESSAGE_PAGE) + .await; + + // 1 message out, 1 message back = 2 + if messages.len() < 2 { + tokio::time::sleep(Duration::from_millis(HALF_SECOND)).await; + continue; + } + + let inbound_chat_message = messages + .iter() + .find(|m| m.body == inbound_reply.clone().into_bytes()) + .expect("no message with that content found") + .clone(); + + let outbound_chat_message = messages + .iter() + .find(|m| m.direction == Direction::Outbound) + .expect("no message with that direction found") + .clone(); + + let metadata: &MessageMetadata = &inbound_chat_message.metadata[0]; + + // Metadata data is a reply type + assert_eq!( + metadata.metadata_type, + MessageMetadataType::Reply, + "Metadata type is wrong" + ); + + // Metadata data contains id to original message + assert_eq!( + metadata.data, outbound_chat_message.message_id, + "Message id does not match" + ); + + return; + } + + panic!("Never received incoming chat message",) +}