From 71f5d99c4450f5ad951f6792e8e27c8800de1b9e Mon Sep 17 00:00:00 2001 From: Ghislain MARY Date: Thu, 19 Apr 2018 09:47:18 +0200 Subject: [PATCH] Retry sending IMDN messages that have not been delivered when the SIP network gets up. --- src/chat/chat-message/imdn-message.cpp | 4 ++++ src/chat/chat-message/imdn-message.h | 1 + src/chat/chat-room/chat-room-p.h | 1 + src/chat/chat-room/chat-room.cpp | 10 ++++++++++ src/chat/chat-room/chat-room.h | 2 ++ src/chat/notification/imdn.cpp | 25 +++++++++++++++++++++++-- src/chat/notification/imdn.h | 6 +++++- src/core/core.h | 1 + tester/group_chat_tester.c | 25 ++++++++++++++++++++++--- 9 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/chat/chat-message/imdn-message.cpp b/src/chat/chat-message/imdn-message.cpp index 76daa18a2..9e849560d 100644 --- a/src/chat/chat-message/imdn-message.cpp +++ b/src/chat/chat-message/imdn-message.cpp @@ -40,6 +40,8 @@ void ImdnMessagePrivate::setState (ChatMessage::State newState, bool force) { for (const auto &message : context.displayedMessages) message->getPrivate()->updateInDb(); static_pointer_cast(context.chatRoom)->getPrivate()->getImdnHandler()->onImdnMessageDelivered(q->getSharedFromThis()); + } else if (newState == ChatMessage::State::NotDelivered) { + // TODO: Maybe we should retry sending the IMDN message if we get an error here } } @@ -56,6 +58,8 @@ ImdnMessage::ImdnMessage ( const list &nonDeliveredMessages ) : ImdnMessage(Context(chatRoom, nonDeliveredMessages)) {} +ImdnMessage::ImdnMessage (const std::shared_ptr &message) : ImdnMessage(message->getPrivate()->context) {} + ImdnMessage::ImdnMessage (const Context &context) : NotificationMessage(*new ImdnMessagePrivate(context)) { L_D(); diff --git a/src/chat/chat-message/imdn-message.h b/src/chat/chat-message/imdn-message.h index 3cc4c0e23..243b7ae66 100644 --- a/src/chat/chat-message/imdn-message.h +++ b/src/chat/chat-message/imdn-message.h @@ -64,6 +64,7 @@ private: const std::shared_ptr &chatRoom, const std::list &nonDeliveredMessages ); + ImdnMessage (const std::shared_ptr &message); ImdnMessage (const Context &context); L_DECLARE_PRIVATE(ImdnMessage); diff --git a/src/chat/chat-room/chat-room-p.h b/src/chat/chat-room/chat-room-p.h index dd6604b07..dfd7c8fa9 100644 --- a/src/chat/chat-room/chat-room-p.h +++ b/src/chat/chat-room/chat-room-p.h @@ -63,6 +63,7 @@ public: const std::list> &displayedMessages ); std::shared_ptr createImdnMessage (const std::list &nonDeliveredMessages); + std::shared_ptr createImdnMessage (const std::shared_ptr &message); std::shared_ptr createIsComposingMessage (); std::list> findChatMessages (const std::string &messageId) const; diff --git a/src/chat/chat-room/chat-room.cpp b/src/chat/chat-room/chat-room.cpp index 44af4294e..30e0a8843 100644 --- a/src/chat/chat-room/chat-room.cpp +++ b/src/chat/chat-room/chat-room.cpp @@ -123,6 +123,10 @@ shared_ptr ChatRoomPrivate::createImdnMessage (const list(new ImdnMessage(q->getSharedFromThis(), nonDeliveredMessages)); } +shared_ptr ChatRoomPrivate::createImdnMessage (const shared_ptr &message) { + return shared_ptr(new ImdnMessage(message)); +} + shared_ptr ChatRoomPrivate::createIsComposingMessage () { L_Q(); return shared_ptr(new IsComposingMessage(q->getSharedFromThis(), *isComposingHandler.get(), isComposing)); @@ -318,6 +322,12 @@ ChatRoom::ChatRoom (ChatRoomPrivate &p, const shared_ptr &core, const Chat d->isComposingHandler.reset(new IsComposing(core->getCCore(), d)); } +ChatRoom::~ChatRoom () { + L_D(); + + d->imdnHandler.reset(); +} + // ----------------------------------------------------------------------------- const ChatRoomId &ChatRoom::getChatRoomId () const { diff --git a/src/chat/chat-room/chat-room.h b/src/chat/chat-room/chat-room.h index acde23615..9c361df82 100644 --- a/src/chat/chat-room/chat-room.h +++ b/src/chat/chat-room/chat-room.h @@ -37,6 +37,8 @@ public: L_OVERRIDE_SHARED_FROM_THIS(ChatRoom); + ~ChatRoom (); + const ChatRoomId &getChatRoomId () const override; const IdentityAddress &getPeerAddress () const override; diff --git a/src/chat/notification/imdn.cpp b/src/chat/notification/imdn.cpp index 23644f2b9..6cccf52f0 100644 --- a/src/chat/notification/imdn.cpp +++ b/src/chat/notification/imdn.cpp @@ -22,7 +22,7 @@ #include "chat/chat-message/chat-message-p.h" #include "chat/chat-message/imdn-message.h" #include "chat/chat-room/chat-room-p.h" -#include "core/core.h" +#include "core/core-p.h" #include "logger/logger.h" #include "xml/imdn.h" #include "xml/linphone-imdn.h" @@ -37,10 +37,13 @@ LINPHONE_BEGIN_NAMESPACE // ----------------------------------------------------------------------------- -Imdn::Imdn (ChatRoom *chatRoom) : chatRoom(chatRoom) {} +Imdn::Imdn (ChatRoom *chatRoom) : chatRoom(chatRoom) { + chatRoom->getCore()->getPrivate()->registerListener(this); +} Imdn::~Imdn () { stopTimer(); + chatRoom->getCore()->getPrivate()->unregisterListener(this); } // ----------------------------------------------------------------------------- @@ -76,11 +79,29 @@ void Imdn::notifyDisplay (const shared_ptr &message) { // ----------------------------------------------------------------------------- void Imdn::onImdnMessageDelivered (const std::shared_ptr &message) { + // If an IMDN has been successfully delivered, remove it from the list so that + // it does not get sent again sentImdnMessages.remove(message); } // ----------------------------------------------------------------------------- +void Imdn::onNetworkReachable (bool sipNetworkReachable, bool mediaNetworkReachable) { + if (sipNetworkReachable) { + // When the SIP network gets up, retry sending every IMDN message that has not + // successfully been delivered + auto messages = sentImdnMessages; + sentImdnMessages.clear(); + for (const auto &message : messages) { + auto imdnMessage = chatRoom->getPrivate()->createImdnMessage(message); + sentImdnMessages.push_back(imdnMessage); + imdnMessage->send(); + } + } +} + +// ----------------------------------------------------------------------------- + string Imdn::createXml (const string &id, time_t timestamp, Imdn::Type imdnType, LinphoneReason reason) { char *datetime = linphone_timestamp_to_rfc3339_string(timestamp); Xsd::Imdn::Imdn imdn(id, datetime); diff --git a/src/chat/notification/imdn.h b/src/chat/notification/imdn.h index de467f859..0d1b00940 100644 --- a/src/chat/notification/imdn.h +++ b/src/chat/notification/imdn.h @@ -22,6 +22,7 @@ #include "linphone/utils/general.h" +#include "core/core-listener.h" #include "utils/background-task.h" #include "private.h" @@ -34,7 +35,7 @@ class ChatMessage; class ChatRoom; class ImdnMessage; -class Imdn { +class Imdn : public CoreListener { public: enum class Type { Delivery, @@ -58,6 +59,9 @@ public: void onImdnMessageDelivered (const std::shared_ptr &message); + // CoreListener + void onNetworkReachable (bool sipNetworkReachable, bool mediaNetworkReachable) override; + static std::string createXml (const std::string &id, time_t time, Imdn::Type imdnType, LinphoneReason reason); static void parse (const std::shared_ptr &chatMessage); diff --git a/src/core/core.h b/src/core/core.h index bcb458a87..1588eb4d4 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -51,6 +51,7 @@ class LINPHONE_PUBLIC Core : public Object { friend class ClientGroupChatRoom; friend class ClientGroupChatRoomPrivate; friend class ClientGroupToBasicChatRoomPrivate; + friend class Imdn; friend class LocalConferenceEventHandlerPrivate; friend class MainDb; friend class MainDbChatMessageKey; diff --git a/tester/group_chat_tester.c b/tester/group_chat_tester.c index ca6eed11d..98e714a89 100644 --- a/tester/group_chat_tester.c +++ b/tester/group_chat_tester.c @@ -3144,7 +3144,7 @@ end: linphone_core_manager_destroy(chloe); } -static void aggregated_imdn_for_group_chat_room (void) { +static void aggregated_imdn_for_group_chat_room_base (bool_t read_while_offline) { LinphoneCoreManager *marie = linphone_core_manager_create("marie_rc"); LinphoneCoreManager *pauline = linphone_core_manager_create("pauline_rc"); LinphoneCoreManager *chloe = linphone_core_manager_create("chloe_rc"); @@ -3196,9 +3196,19 @@ static void aggregated_imdn_for_group_chat_room (void) { // Mark the messages as read on Marie's and Pauline's sides linphone_chat_room_mark_as_read(marieCr); - linphone_chat_room_mark_as_read(paulineCr); - BC_ASSERT_TRUE(wait_for_list(coresList, &chloe->stat.number_of_LinphoneMessageDisplayed, initialChloeStats.number_of_LinphoneMessageDisplayed + 1, 1000)); + if (read_while_offline) { + linphone_core_set_network_reachable(pauline->lc, FALSE); + linphone_chat_room_mark_as_read(paulineCr); + wait_for_list(coresList, 0, 1, 2000); + linphone_core_set_network_reachable(pauline->lc, TRUE); + } else { + linphone_chat_room_mark_as_read(paulineCr); + } + BC_ASSERT_TRUE(wait_for_list(coresList, &chloe->stat.number_of_LinphoneMessageDisplayed, initialChloeStats.number_of_LinphoneMessageDisplayed + 1, 3000)); BC_ASSERT_EQUAL(chloe->stat.number_of_LinphoneMessageDeliveredToUser, 0, int, "%d"); + if (read_while_offline) { + wait_for_list(coresList, 0, 1, 2000); // To prevent memory leak + } linphone_chat_message_unref(chloeMessage3); linphone_chat_message_unref(chloeMessage2); @@ -3217,6 +3227,14 @@ end: linphone_core_manager_destroy(chloe); } +static void aggregated_imdn_for_group_chat_room (void) { + aggregated_imdn_for_group_chat_room_base(FALSE); +} + +static void aggregated_imdn_for_group_chat_room_read_while_offline (void) { + aggregated_imdn_for_group_chat_room_base(TRUE); +} + static void find_one_to_one_chat_room (void) { LinphoneCoreManager *marie = linphone_core_manager_create("marie_rc"); LinphoneCoreManager *pauline = linphone_core_manager_create("pauline_rc"); @@ -3404,6 +3422,7 @@ test_t group_chat_tests[] = { TEST_NO_TAG("Unique one-to-one chatroom re-created from the party that deleted it, with inactive devices", group_chat_room_unique_one_to_one_chat_room_recreated_from_message_2), TEST_NO_TAG("IMDN for group chat room", imdn_for_group_chat_room), TEST_NO_TAG("Aggregated IMDN for group chat room", aggregated_imdn_for_group_chat_room), + TEST_NO_TAG("Aggregated IMDN for group chat room read while offline", aggregated_imdn_for_group_chat_room_read_while_offline), TEST_NO_TAG("Find one to one chat room", find_one_to_one_chat_room), TEST_NO_TAG("New device after group chat room creation", group_chat_room_new_device_after_creation) };