From 8a0b0669600820f1ed3a749e54ec378780f0162a Mon Sep 17 00:00:00 2001 From: Sylvain Berfini Date: Fri, 24 Nov 2017 10:44:20 +0100 Subject: [PATCH] Changes in transient messages in chat room, replaced by transient event logs --- coreapi/private.h | 1 - coreapi/tester_utils.h | 1 - src/c-wrapper/api/c-chat-room.cpp | 8 --- src/chat/chat-message/chat-message.cpp | 46 ++++++------- src/chat/chat-message/chat-message.h | 1 - src/chat/chat-room/chat-room-p.h | 19 ++---- src/chat/chat-room/chat-room.cpp | 66 +++---------------- .../chat-room/real-time-text-chat-room.cpp | 2 +- src/chat/chat-room/server-group-chat-room-p.h | 1 - .../chat-room/server-group-chat-room-stub.cpp | 2 - .../file-transfer-chat-message-modifier.cpp | 6 -- tester/message_tester.c | 14 ++-- 12 files changed, 43 insertions(+), 124 deletions(-) diff --git a/coreapi/private.h b/coreapi/private.h index efe81bb52..fc2bc54e3 100644 --- a/coreapi/private.h +++ b/coreapi/private.h @@ -489,7 +489,6 @@ LinphoneChatRoom *_linphone_client_group_chat_room_new (LinphoneCore *core, cons LinphoneChatRoom *_linphone_server_group_chat_room_new (LinphoneCore *core, LinphonePrivate::SalCallOp *op); void linphone_chat_room_release(LinphoneChatRoom *cr); void linphone_chat_room_set_call(LinphoneChatRoom *cr, LinphoneCall *call); -bctbx_list_t * linphone_chat_room_get_transient_messages(const LinphoneChatRoom *cr); LinphoneChatRoomCbs * linphone_chat_room_cbs_new (void); /**/ diff --git a/coreapi/tester_utils.h b/coreapi/tester_utils.h index 2dade201e..862d1043a 100644 --- a/coreapi/tester_utils.h +++ b/coreapi/tester_utils.h @@ -93,7 +93,6 @@ LINPHONE_PUBLIC mblk_t *_linphone_call_stats_get_received_rtcp (const LinphoneCa LINPHONE_PUBLIC LinphoneQualityReporting *linphone_call_log_get_quality_reporting(LinphoneCallLog *call_log); LINPHONE_PUBLIC reporting_session_report_t **linphone_quality_reporting_get_reports(LinphoneQualityReporting *qreporting); -LINPHONE_PUBLIC bctbx_list_t * linphone_chat_room_get_transient_messages(const LinphoneChatRoom *cr); LINPHONE_PUBLIC LinphoneChatRoom * linphone_core_find_chat_room (const LinphoneCore *lc, const LinphoneAddress *peerAddr, const LinphoneAddress *localAddr); LINPHONE_PUBLIC MSList* linphone_core_fetch_friends_from_db(LinphoneCore *lc, LinphoneFriendList *list); diff --git a/src/c-wrapper/api/c-chat-room.cpp b/src/c-wrapper/api/c-chat-room.cpp index 7f329f082..3291afc31 100644 --- a/src/c-wrapper/api/c-chat-room.cpp +++ b/src/c-wrapper/api/c-chat-room.cpp @@ -71,10 +71,6 @@ void linphone_chat_room_release (LinphoneChatRoom *cr) { L_GET_PRIVATE_FROM_C_OBJECT(cr)->release(); } -void linphone_chat_room_remove_transient_message (LinphoneChatRoom *cr, LinphoneChatMessage *msg) { - L_GET_PRIVATE_FROM_C_OBJECT(cr)->removeTransientMessage(L_GET_CPP_PTR_FROM_C_OBJECT(msg)); -} - void linphone_chat_room_send_message (LinphoneChatRoom *cr, const char *msg) { L_GET_PRIVATE_FROM_C_OBJECT(cr)->sendMessage(L_GET_CPP_PTR_FROM_C_OBJECT(cr)->createMessage(msg)); } @@ -174,10 +170,6 @@ void linphone_chat_room_set_call (LinphoneChatRoom *cr, LinphoneCall *call) { L_GET_PRIVATE_FROM_C_OBJECT(cr, RealTimeTextChatRoom)->call = call; } -bctbx_list_t *linphone_chat_room_get_transient_messages (const LinphoneChatRoom *cr) { - return L_GET_RESOLVED_C_LIST_FROM_CPP_LIST(L_GET_PRIVATE_FROM_C_OBJECT(cr)->getTransientMessages()); -} - void linphone_chat_room_mark_as_read (LinphoneChatRoom *cr) { L_GET_CPP_PTR_FROM_C_OBJECT(cr)->markAsRead(); } diff --git a/src/chat/chat-message/chat-message.cpp b/src/chat/chat-message/chat-message.cpp index 6364db82a..8b9a27411 100644 --- a/src/chat/chat-message/chat-message.cpp +++ b/src/chat/chat-message/chat-message.cpp @@ -518,7 +518,7 @@ void ChatMessagePrivate::send () { if (result == ChatMessageModifier::Result::Error) { sal_error_info_set((SalErrorInfo *)op->get_error_info(), SalReasonNotAcceptable, "SIP", errorCode, "Unable to encrypt IM", nullptr); q->updateState(ChatMessage::State::NotDelivered); - q->store(); + store(); return; } else if (result == ChatMessageModifier::Result::Suspended) { currentSendStep |= ChatMessagePrivate::Step::Encryption; @@ -585,10 +585,32 @@ void ChatMessagePrivate::store() { shared_ptr eventLog = chatEvent.lock(); if (eventLog) { q->getChatRoom()->getCore()->getPrivate()->mainDb->updateEvent(eventLog); + + if (direction == ChatMessage::Direction::Incoming) { + if (!hasFileTransferContent()) { + // Incoming message doesn't have any download waiting anymore, we can remove it's event from the transients + q->getChatRoom()->getPrivate()->removeTransientEvent(eventLog); + } + } else { + if (state == ChatMessage::State::Delivered || state == ChatMessage::State::NotDelivered) { + // Once message has reached this state it won't change anymore so we can remove the event from the transients + q->getChatRoom()->getPrivate()->removeTransientEvent(eventLog); + } + } } else { eventLog = make_shared(time, q->getSharedFromThis()); chatEvent = eventLog; q->getChatRoom()->getCore()->getPrivate()->mainDb->addEvent(eventLog); + + if (direction == ChatMessage::Direction::Incoming) { + if (hasFileTransferContent()) { + // Keep the event in the transient list, message storage can be updated in near future + q->getChatRoom()->getPrivate()->addTransientEvent(eventLog); + } + } else { + // Keep event in transient to be able to store in database state changes + q->getChatRoom()->getPrivate()->addTransientEvent(eventLog); + } } } @@ -760,32 +782,10 @@ void ChatMessage::removeCustomHeader (const string &headerName) { d->customHeaders.erase(headerName); } -void ChatMessage::store () { - L_D(); - - if (d->storageId != 0) { - /* The message has already been stored (probably because of file transfer), update it */ - // TODO: history. - // linphone_chat_message_store_update(L_GET_C_BACK_PTR(this)); - } else { - /* Store the new message */ - // TODO: history. - // linphone_chat_message_store(L_GET_C_BACK_PTR(this)); - } -} - void ChatMessage::updateState (State state) { L_D(); d->setState(state); - // TODO: history. - // linphone_chat_message_store_state(L_GET_C_BACK_PTR(this)); - - if (state == State::Delivered || state == State::NotDelivered) { - shared_ptr chatRoom = getChatRoom(); - if (chatRoom) - chatRoom->getPrivate()->moveTransientMessageToWeakMessages(getSharedFromThis()); - } } void ChatMessage::send () { diff --git a/src/chat/chat-message/chat-message.h b/src/chat/chat-message/chat-message.h index 952b3abd3..7fdf0e61d 100644 --- a/src/chat/chat-message/chat-message.h +++ b/src/chat/chat-message/chat-message.h @@ -65,7 +65,6 @@ public: void sendDisplayNotification (); void setImdnMessageId (const std::string &imdnMessageId); void setIsSecured (bool isSecured); - void store (); // ----- TODO: Remove me. std::shared_ptr getChatRoom () const; diff --git a/src/chat/chat-room/chat-room-p.h b/src/chat/chat-room/chat-room-p.h index a4fd64b20..853c2f557 100644 --- a/src/chat/chat-room/chat-room-p.h +++ b/src/chat/chat-room/chat-room-p.h @@ -25,6 +25,7 @@ #include "chat/notification/is-composing.h" #include "chat-room.h" #include "object/object-p.h" +#include "event-log/event-log.h" // ============================================================================= @@ -37,14 +38,8 @@ public: static int createChatMessageFromDb (void *data, int argc, char **argv, char **colName); - void addTransientMessage (const std::shared_ptr &msg); - void addWeakMessage (const std::shared_ptr &msg); - std::list> getTransientMessages () const { - return transientMessages; - } - - void moveTransientMessageToWeakMessages (const std::shared_ptr &msg); - void removeTransientMessage (const std::shared_ptr &msg); + void addTransientEvent (const std::shared_ptr &log); + void removeTransientEvent (const std::shared_ptr &log); void release (); @@ -55,12 +50,8 @@ public: void sendIsComposingNotification (); int createChatMessageFromDb (int argc, char **argv, char **colName); - std::shared_ptr getTransientMessage (unsigned int storageId) const; - std::shared_ptr getWeakMessage (unsigned int storageId) const; std::list> findMessages (const std::string &messageId); - virtual void storeOrUpdateMessage (const std::shared_ptr &msg); - virtual LinphoneReason messageReceived (SalOp *op, const SalMessage *msg); void realtimeTextReceived (uint32_t character, LinphoneCall *call); @@ -84,9 +75,7 @@ public: ChatRoom::State state = ChatRoom::State::None; bool isComposing = false; std::list
remoteIsComposing; - std::list> transientMessages; - - std::list> weakMessages; + std::list> transientEvents; // TODO: Remove me. Must be present only in rtt chat room. std::shared_ptr pendingMessage; diff --git a/src/chat/chat-room/chat-room.cpp b/src/chat/chat-room/chat-room.cpp index 652c30d83..1cda16c65 100644 --- a/src/chat/chat-room/chat-room.cpp +++ b/src/chat/chat-room/chat-room.cpp @@ -42,32 +42,16 @@ int ChatRoomPrivate::createChatMessageFromDb (void *data, int argc, char **argv, // ----------------------------------------------------------------------------- -void ChatRoomPrivate::addTransientMessage (const shared_ptr &msg) { - auto iter = find(transientMessages.begin(), transientMessages.end(), msg); - if (iter == transientMessages.end()) - transientMessages.push_back(msg); +void ChatRoomPrivate::addTransientEvent (const shared_ptr &log) { + auto iter = find(transientEvents.begin(), transientEvents.end(), log); + if (iter == transientEvents.end()) + transientEvents.push_back(log); } -void ChatRoomPrivate::addWeakMessage (const shared_ptr &msg) { - weak_ptr weakptr(msg); - weakMessages.push_back(weakptr); -} - -void ChatRoomPrivate::moveTransientMessageToWeakMessages (const shared_ptr &msg) { - auto iter = find(transientMessages.begin(), transientMessages.end(), msg); - if (iter != transientMessages.end()) { - /* msg is not transient anymore, we can remove it from our transient list and unref it */ - addWeakMessage(msg); - removeTransientMessage(msg); - } else { - /* msg has already been removed from the transient messages, do nothing */ - } -} - -void ChatRoomPrivate::removeTransientMessage (const shared_ptr &msg) { - auto iter = find(transientMessages.begin(), transientMessages.end(), msg); - if (iter != transientMessages.end()) { - transientMessages.erase(iter); +void ChatRoomPrivate::removeTransientEvent (const shared_ptr &log) { + auto iter = find(transientEvents.begin(), transientEvents.end(), log); + if (iter != transientEvents.end()) { + transientEvents.erase(iter); } } @@ -75,12 +59,6 @@ void ChatRoomPrivate::removeTransientMessage (const shared_ptr &msg void ChatRoomPrivate::release () { isComposingHandler->stopTimers(); - - for (auto &message : weakMessages) - message.lock()->cancelFileTransfer(); - - for (auto &message : transientMessages) - message->cancelFileTransfer(); } // ----------------------------------------------------------------------------- @@ -175,42 +153,16 @@ int ChatRoomPrivate::createChatMessageFromDb (int argc, char **argv, char **colN return 0; } -shared_ptr ChatRoomPrivate::getTransientMessage (unsigned int storageId) const { - for (auto &message : transientMessages) { - if (message->getPrivate()->getStorageId() == storageId) - return message; - } - return nullptr; -} - -std::shared_ptr ChatRoomPrivate::getWeakMessage (unsigned int storageId) const { - for (auto &message : weakMessages) { - try { - shared_ptr msg(message); - if (msg->getPrivate()->getStorageId() == storageId) - return msg; - } catch(const std::bad_weak_ptr& e) {} - } - return nullptr; -} - list > ChatRoomPrivate::findMessages (const string &messageId) { // TODO: history. return list>(); } -void ChatRoomPrivate::storeOrUpdateMessage (const shared_ptr &msg) { - msg->store(); -} - void ChatRoomPrivate::sendMessage (const shared_ptr &msg) { L_Q(); // TODO: Check direction. - /* Add to transient list */ - addTransientMessage(msg); - msg->getPrivate()->setTime(ms_time(0)); msg->getPrivate()->send(); @@ -222,8 +174,6 @@ void ChatRoomPrivate::sendMessage (const shared_ptr &msg) { cb(cr, L_GET_C_BACK_PTR(event)); } - storeOrUpdateMessage(msg); - if (isComposing) isComposing = false; isComposingHandler->stopIdleTimer(); diff --git a/src/chat/chat-room/real-time-text-chat-room.cpp b/src/chat/chat-room/real-time-text-chat-room.cpp index 6242b51b1..2f3d7a853 100644 --- a/src/chat/chat-room/real-time-text-chat-room.cpp +++ b/src/chat/chat-room/real-time-text-chat-room.cpp @@ -77,7 +77,7 @@ void RealTimeTextChatRoomPrivate::realtimeTextReceived (uint32_t character, Linp pendingMessage->getPrivate()->setDirection(ChatMessage::Direction::Incoming); if (lp_config_get_int(cCore->config, "misc", "store_rtt_messages", 1) == 1) - storeOrUpdateMessage(pendingMessage); + pendingMessage->getPrivate()->store(); chatMessageReceived(pendingMessage); pendingMessage = nullptr; diff --git a/src/chat/chat-room/server-group-chat-room-p.h b/src/chat/chat-room/server-group-chat-room-p.h index 2b003fdac..18c0830da 100644 --- a/src/chat/chat-room/server-group-chat-room-p.h +++ b/src/chat/chat-room/server-group-chat-room-p.h @@ -47,7 +47,6 @@ public: void update (SalCallOp *op); void dispatchMessage (const IdentityAddress &fromAddr, const Content &content); - void storeOrUpdateMessage (const std::shared_ptr &msg) override; LinphoneReason messageReceived (SalOp *op, const SalMessage *msg) override; void setConferenceAddress (const IdentityAddress &confAddr); diff --git a/src/chat/chat-room/server-group-chat-room-stub.cpp b/src/chat/chat-room/server-group-chat-room-stub.cpp index 1a3999e9f..e0ffc5ef9 100644 --- a/src/chat/chat-room/server-group-chat-room-stub.cpp +++ b/src/chat/chat-room/server-group-chat-room-stub.cpp @@ -57,8 +57,6 @@ void ServerGroupChatRoomPrivate::update (SalCallOp *) {} void ServerGroupChatRoomPrivate::dispatchMessage (const IdentityAddress &, const Content &) {} -void ServerGroupChatRoomPrivate::storeOrUpdateMessage (const shared_ptr &) {} - LinphoneReason ServerGroupChatRoomPrivate::messageReceived (SalOp *, const SalMessage *) { return LinphoneReasonNone; } diff --git a/src/chat/modifier/file-transfer-chat-message-modifier.cpp b/src/chat/modifier/file-transfer-chat-message-modifier.cpp index 377be8e81..6016ad50c 100644 --- a/src/chat/modifier/file-transfer-chat-message-modifier.cpp +++ b/src/chat/modifier/file-transfer-chat-message-modifier.cpp @@ -401,9 +401,6 @@ void FileTransferChatMessageModifier::processIoErrorUpload (const belle_sip_io_e lError() << "I/O Error during file upload of msg [" << this << "]"; chatMessage->updateState(ChatMessage::State::NotDelivered); releaseHttpRequest(); - - if (chatRoom) - chatRoom->getPrivate()->removeTransientMessage(chatMessage); } static void _chat_message_process_auth_requested_upload (void *data, belle_sip_auth_event *event) { @@ -415,9 +412,6 @@ void FileTransferChatMessageModifier::processAuthRequestedUpload (const belle_si lError() << "Error during file upload: auth requested for msg [" << this << "]"; chatMessage->updateState(ChatMessage::State::NotDelivered); releaseHttpRequest(); - - if (chatRoom) - chatRoom->getPrivate()->removeTransientMessage(chatMessage); } int FileTransferChatMessageModifier::uploadFile () { diff --git a/tester/message_tester.c b/tester/message_tester.c index 77242f3ef..2f7732039 100644 --- a/tester/message_tester.c +++ b/tester/message_tester.c @@ -416,15 +416,15 @@ static void text_message_with_send_error(void) { linphone_chat_room_send_chat_message(chat_room,msg); /* check transient msg list: the msg should be in it, and should be the only one */ - BC_ASSERT_EQUAL((unsigned int)bctbx_list_size(linphone_chat_room_get_transient_messages(chat_room)), 1, unsigned int, "%u"); - BC_ASSERT_PTR_EQUAL(bctbx_list_nth_data(linphone_chat_room_get_transient_messages(chat_room),0), msg); + /*BC_ASSERT_EQUAL((unsigned int)bctbx_list_size(linphone_chat_room_get_transient_messages(chat_room)), 1, unsigned int, "%u"); + BC_ASSERT_PTR_EQUAL(bctbx_list_nth_data(linphone_chat_room_get_transient_messages(chat_room),0), msg);*/ BC_ASSERT_TRUE(wait_for(pauline->lc,marie->lc,&marie->stat.number_of_LinphoneMessageNotDelivered,1)); /*BC_ASSERT_EQUAL(marie->stat.number_of_LinphoneMessageInProgress,1, int, "%d");*/ BC_ASSERT_EQUAL(pauline->stat.number_of_LinphoneMessageReceived,0, int, "%d"); /* the msg should have been discarded from transient list after an error */ - BC_ASSERT_EQUAL((unsigned int)bctbx_list_size(linphone_chat_room_get_transient_messages(chat_room)), 0, unsigned int, "%u"); + //BC_ASSERT_EQUAL((unsigned int)bctbx_list_size(linphone_chat_room_get_transient_messages(chat_room)), 0, unsigned int, "%u"); sal_set_send_error(linphone_core_get_sal(marie->lc), 0); @@ -449,8 +449,8 @@ static void text_message_with_external_body(void) { linphone_chat_room_send_chat_message(chat_room,msg); /* check transient msg list: the msg should be in it, and should be the only one */ - BC_ASSERT_EQUAL((unsigned int)bctbx_list_size(linphone_chat_room_get_transient_messages(chat_room)), 1, unsigned int, "%u"); - BC_ASSERT_PTR_EQUAL(bctbx_list_nth_data(linphone_chat_room_get_transient_messages(chat_room),0), msg); + /*BC_ASSERT_EQUAL((unsigned int)bctbx_list_size(linphone_chat_room_get_transient_messages(chat_room)), 1, unsigned int, "%u"); + BC_ASSERT_PTR_EQUAL(bctbx_list_nth_data(linphone_chat_room_get_transient_messages(chat_room),0), msg);*/ BC_ASSERT_TRUE(wait_for(pauline->lc,marie->lc,&marie->stat.number_of_LinphoneMessageReceived,1)); BC_ASSERT_TRUE(wait_for(pauline->lc,marie->lc,&pauline->stat.number_of_LinphoneMessageDelivered,1)); @@ -458,7 +458,7 @@ static void text_message_with_external_body(void) { BC_ASSERT_EQUAL(pauline->stat.number_of_LinphoneMessageInProgress,1, int, "%d"); BC_ASSERT_EQUAL(marie->stat.number_of_LinphoneMessageExtBodyReceived,1, int, "%d"); - BC_ASSERT_EQUAL((unsigned int)bctbx_list_size(linphone_chat_room_get_transient_messages(chat_room)), 0, unsigned int, "%u"); + //BC_ASSERT_EQUAL((unsigned int)bctbx_list_size(linphone_chat_room_get_transient_messages(chat_room)), 0, unsigned int, "%u"); linphone_core_manager_destroy(marie); linphone_core_manager_destroy(pauline); @@ -2385,7 +2385,6 @@ void file_and_text_message(void) { test_t message_tests[] = { TEST_NO_TAG("File + Text message", file_and_text_message), TEST_NO_TAG("Text message", text_message), - TEST_NO_TAG("Text message within call dialog", text_message_within_call_dialog), TEST_NO_TAG("Text message with credentials from auth callback", text_message_with_credential_from_auth_callback), TEST_NO_TAG("Text message with privacy", text_message_with_privacy), TEST_NO_TAG("Text message compatibility mode", text_message_compatibility_mode), @@ -2454,6 +2453,7 @@ test_t message_tests[] = { TEST_NO_TAG("IM Encryption Engine b64", im_encryption_engine_b64), TEST_NO_TAG("IM Encryption Engine b64 async", im_encryption_engine_b64_async), // Crash currently + TEST_NO_TAG("Text message within call dialog", text_message_within_call_dialog), TEST_NO_TAG("Info message", info_message), TEST_NO_TAG("Info message with body", info_message_with_body), TEST_NO_TAG("Crash during file transfer", crash_during_file_transfer),