From 050f3ebf4cba550664cd9f0ccebc4ffb9e828a9d Mon Sep 17 00:00:00 2001
From: Andrea Gianarda <andrea.gianarda@belledonne-communications.com>
Date: Thu, 9 Nov 2023 14:23:12 +0100
Subject: [PATCH] Improve MainDb::loadChatRooms by using a std::map instead of
an std::list
Increase timeout of Michelle's and focus's chat room deletion
function simple_conference_base: wait for full state and then
liblinphone tester timeout ms before checking volumes
video_call_expected_fps_for_specified_bandwidth: increase number of
iterations
---
src/conference/conference.h | 32 ++++++
src/db/main-db.cpp | 147 ++++++++++++-------------
src/db/main-db.h | 8 +-
tester/audio_video_conference_tester.c | 33 +++++-
tester/local_chat_tester.cpp | 14 +--
tester/video_quality_tester.c | 2 +-
6 files changed, 143 insertions(+), 93 deletions(-)
diff --git a/src/conference/conference.h b/src/conference/conference.h
index 8ca05371e4..22935b0b1c 100644
--- a/src/conference/conference.h
+++ b/src/conference/conference.h
@@ -64,6 +64,38 @@ class LINPHONE_PUBLIC Conference : public ConferenceInterface, public Conference
friend class ServerGroupChatRoom;
public:
+ struct ConferenceIdCompare {
+ bool operator()(const ConferenceId &lhs, const ConferenceId &rhs) const {
+ const auto &lhsRawPeerAddress = lhs.getPeerAddress();
+ Address lhsPeerAddress = Address();
+ if (lhsRawPeerAddress) {
+ lhsPeerAddress = lhsRawPeerAddress->getUriWithoutGruu();
+ lhsPeerAddress.removeUriParam(Conference::SecurityModeParameter);
+ }
+ const auto &lhsRawLocalAddress = lhs.getLocalAddress();
+ Address lhsLocalAddress = Address();
+ if (lhsRawLocalAddress) {
+ lhsLocalAddress = lhsRawLocalAddress->getUriWithoutGruu();
+ lhsLocalAddress.removeUriParam(Conference::SecurityModeParameter);
+ }
+
+ const auto &rhsRawPeerAddress = rhs.getPeerAddress();
+ Address rhsPeerAddress = Address();
+ if (rhsRawPeerAddress) {
+ rhsPeerAddress = rhsRawPeerAddress->getUriWithoutGruu();
+ rhsPeerAddress.removeUriParam(Conference::SecurityModeParameter);
+ }
+ const auto &rhsRawLocalAddress = rhs.getLocalAddress();
+ Address rhsLocalAddress = Address();
+ if (rhsRawLocalAddress) {
+ rhsLocalAddress = rhsRawLocalAddress->getUriWithoutGruu();
+ rhsLocalAddress.removeUriParam(Conference::SecurityModeParameter);
+ }
+
+ return (lhsPeerAddress < rhsPeerAddress) ||
+ ((lhsPeerAddress == rhsPeerAddress) && (lhsLocalAddress < rhsLocalAddress));
+ }
+ };
static constexpr int labelLength = 10;
static const std::string SecurityModeParameter;
~Conference();
diff --git a/src/db/main-db.cpp b/src/db/main-db.cpp
index 1226486203..a92528638f 100644
--- a/src/db/main-db.cpp
+++ b/src/db/main-db.cpp
@@ -4909,91 +4909,74 @@ void MainDb::disableDisplayNotificationRequired(const std::shared_ptr<const Even
// - set the creation time to the earliest one
// - assign all events preceeding the latest creation time to the kept chatroom
// - destroy the deleted chatroom from DB
-void MainDb::addChatroomToList(list<shared_ptr<AbstractChatRoom>> &chatRooms,
- const shared_ptr<AbstractChatRoom> chatRoom) const {
+void MainDb::addChatroomToList(
+ std::map<ConferenceId, std::shared_ptr<AbstractChatRoom>, Conference::ConferenceIdCompare> &chatRoomsMap,
+ const shared_ptr<AbstractChatRoom> chatRoom) const {
#ifdef HAVE_DB_STORAGE
- L_D();
const auto chatRoomConferenceId = chatRoom->getConferenceId();
- const auto chatRoomIt =
- std::find_if(chatRooms.cbegin(), chatRooms.cend(), [&chatRoomConferenceId](const auto &storedChatRoom) {
- const auto &storedChatRoomConferenceId = storedChatRoom->getConferenceId();
-
- const auto storedChatRoomConferenceIdPeerAddress = storedChatRoomConferenceId.getPeerAddress();
- const auto storedChatRoomConferenceIdPeerAddressWithoutGruu =
- (storedChatRoomConferenceIdPeerAddress && storedChatRoomConferenceIdPeerAddress->isValid())
- ? storedChatRoomConferenceId.getPeerAddress()->getUriWithoutGruu()
- : Address();
- const auto chatRoomConferenceIdPeerAddress = chatRoomConferenceId.getPeerAddress();
- const auto chatRoomConferenceIdPeerAddressWithoutGruu =
- (chatRoomConferenceIdPeerAddress && chatRoomConferenceIdPeerAddress->isValid())
- ? chatRoomConferenceId.getPeerAddress()->getUriWithoutGruu()
- : Address();
-
- const auto storedChatRoomConferenceIdLocalAddress = storedChatRoomConferenceId.getLocalAddress();
- const auto storedChatRoomConferenceIdLocalAddressWithoutGruu =
- (storedChatRoomConferenceIdLocalAddress && storedChatRoomConferenceIdLocalAddress->isValid())
- ? storedChatRoomConferenceId.getLocalAddress()->getUriWithoutGruu()
- : Address();
- const auto chatRoomConferenceIdLocalAddress = chatRoomConferenceId.getLocalAddress();
- const auto chatRoomConferenceIdLocalAddressWithoutGruu =
- (chatRoomConferenceIdLocalAddress && chatRoomConferenceIdLocalAddress->isValid())
- ? chatRoomConferenceId.getLocalAddress()->getUriWithoutGruu()
- : Address();
-
- return (storedChatRoomConferenceIdLocalAddressWithoutGruu == chatRoomConferenceIdLocalAddressWithoutGruu) &&
- (storedChatRoomConferenceIdPeerAddressWithoutGruu == chatRoomConferenceIdPeerAddressWithoutGruu);
- });
shared_ptr<AbstractChatRoom> chatRoomToAdd = nullptr;
- if (chatRoomIt == chatRooms.cend()) {
+ try {
+ auto storedChatRoom = chatRoomsMap.at(chatRoomConferenceId);
+ chatRoomToAdd = mergeChatRooms(chatRoom, storedChatRoom);
+ } catch (std::out_of_range &) {
chatRoomToAdd = chatRoom;
+ }
+ chatRoomsMap.insert_or_assign(chatRoomConferenceId, chatRoomToAdd);
+#endif
+}
+
+shared_ptr<AbstractChatRoom> MainDb::mergeChatRooms(const shared_ptr<AbstractChatRoom> chatRoom1,
+ const shared_ptr<AbstractChatRoom> chatRoom2) const {
+#ifdef HAVE_DB_STORAGE
+ L_D();
+ shared_ptr<AbstractChatRoom> chatRoomToAdd = nullptr;
+ const auto chatRoom1ConferenceId = chatRoom1->getConferenceId();
+ const auto chatRoom1CreationTime = chatRoom1->getCreationTime();
+ const auto chatRoom2ConferenceId = chatRoom2->getConferenceId();
+ const auto chatRoom2CreationTime = chatRoom2->getCreationTime();
+ lInfo() << "Chat rooms with conference id " << chatRoom1ConferenceId << " and " << chatRoom2ConferenceId
+ << " will be merged as they have the same peer address";
+ ConferenceId conferenceIdToAdd;
+ ConferenceId conferenceIdToRemove;
+ time_t creationTime = 0;
+ time_t creationTimeToDelete = 0;
+ // Update chatroom with the largest creation time and update its creation time with the lowest value.
+ if (chatRoom2CreationTime < chatRoom1CreationTime) {
+ chatRoomToAdd = chatRoom1;
+ creationTime = chatRoom2CreationTime;
+ creationTimeToDelete = chatRoom1CreationTime;
+ conferenceIdToAdd = chatRoom1ConferenceId;
+ conferenceIdToRemove = chatRoom2ConferenceId;
} else {
- auto storedChatRoom = (*chatRoomIt);
- const auto storedChatRoomConferenceId = storedChatRoom->getConferenceId();
- const auto storedChatRoomCreationTime = storedChatRoom->getCreationTime();
- const auto chatRoomCreationTime = chatRoom->getCreationTime();
- lInfo() << "Chat rooms with conference id " << chatRoomConferenceId << " and " << storedChatRoomConferenceId
- << " will be merged as they have the same peer address";
- chatRooms.erase(chatRoomIt);
- ConferenceId conferenceIdToAdd;
- ConferenceId conferenceIdToRemove;
- time_t creationTime = 0;
- time_t creationTimeToDelete = 0;
- // Update chatroom with the largest creation time and update its creation time with the lowest value.
- if (storedChatRoomCreationTime < chatRoomCreationTime) {
- chatRoomToAdd = chatRoom;
- creationTime = storedChatRoomCreationTime;
- creationTimeToDelete = chatRoomCreationTime;
- conferenceIdToAdd = chatRoomConferenceId;
- conferenceIdToRemove = storedChatRoomConferenceId;
- } else {
- chatRoomToAdd = storedChatRoom;
- creationTime = chatRoomCreationTime;
- creationTimeToDelete = storedChatRoomCreationTime;
- conferenceIdToAdd = storedChatRoomConferenceId;
- conferenceIdToRemove = chatRoomConferenceId;
- }
- chatRoomToAdd->getPrivate()->setCreationTime(creationTime);
- d->unreadChatMessageCountCache.insert(conferenceIdToAdd,
- *d->unreadChatMessageCountCache[conferenceIdToAdd] +
- *d->unreadChatMessageCountCache[conferenceIdToRemove]);
- d->unreadChatMessageCountCache.insert(conferenceIdToRemove, 0);
+ chatRoomToAdd = chatRoom2;
+ creationTime = chatRoom1CreationTime;
+ creationTimeToDelete = chatRoom2CreationTime;
+ conferenceIdToAdd = chatRoom2ConferenceId;
+ conferenceIdToRemove = chatRoom1ConferenceId;
+ }
+ chatRoomToAdd->getPrivate()->setCreationTime(creationTime);
+ d->unreadChatMessageCountCache.insert(conferenceIdToAdd, *d->unreadChatMessageCountCache[conferenceIdToAdd] +
+ *d->unreadChatMessageCountCache[conferenceIdToRemove]);
+ d->unreadChatMessageCountCache.insert(conferenceIdToRemove, 0);
- const long long &dbChatRoomToAddId = d->selectChatRoomId(conferenceIdToAdd);
- const long long &dbChatRoomToRemoveId = d->selectChatRoomId(conferenceIdToRemove);
+ const long long &dbChatRoomToAddId = d->selectChatRoomId(conferenceIdToAdd);
+ const long long &dbChatRoomToRemoveId = d->selectChatRoomId(conferenceIdToRemove);
- auto creationTimeToDeleteSoci = d->dbSession.getTimeWithSociIndicator(creationTimeToDelete);
- soci::session *session = d->dbSession.getBackendSession();
- // Move conference event that occurred before the latest chatroom was created.
- // Events such as chat messages are already stored in both chat rooms
- *session << "UPDATE conference_event SET chat_room_id = :newChatRoomid WHERE event_id IN (SELECT "
- "conference_event.event_id FROM conference_event, event WHERE event.id = conference_event.event_id "
- "AND conference_event.chat_room_id = :chatRoomId AND event.creation_time < :creationTime)",
- soci::use(dbChatRoomToAddId), soci::use(dbChatRoomToRemoveId),
- soci::use(creationTimeToDeleteSoci.first, creationTimeToDeleteSoci.second);
+ auto creationTimeToDeleteSoci = d->dbSession.getTimeWithSociIndicator(creationTimeToDelete);
+ soci::session *session = d->dbSession.getBackendSession();
+ // Move conference event that occurred before the latest chatroom was created.
+ // Events such as chat messages are already stored in both chat rooms
+ *session << "UPDATE conference_event SET chat_room_id = :newChatRoomid WHERE event_id IN (SELECT "
+ "conference_event.event_id FROM conference_event, event WHERE event.id = conference_event.event_id "
+ "AND conference_event.chat_room_id = :chatRoomId AND event.creation_time < :creationTime)",
+ soci::use(dbChatRoomToAddId), soci::use(dbChatRoomToRemoveId),
+ soci::use(creationTimeToDeleteSoci.first, creationTimeToDeleteSoci.second);
+ if (dbChatRoomToRemoveId != -1) {
*session << "DELETE FROM chat_room WHERE id = :chatRoomId", soci::use(dbChatRoomToRemoveId);
}
-
- chatRooms.push_back(chatRoomToAdd);
+ return chatRoomToAdd;
+#else
+ return nullptr;
#endif
}
@@ -5021,7 +5004,8 @@ list<shared_ptr<AbstractChatRoom>> MainDb::getChatRooms() const {
return L_DB_TRANSACTION {
L_D();
- list<shared_ptr<AbstractChatRoom>> chatRooms;
+ std::map<ConferenceId, std::shared_ptr<AbstractChatRoom>, Conference::ConferenceIdCompare> chatRoomsMap;
+
shared_ptr<Core> core = getCore();
bool serverMode = linphone_core_conference_server_enabled(core->getCCore());
@@ -5047,7 +5031,7 @@ list<shared_ptr<AbstractChatRoom>> MainDb::getChatRooms() const {
shared_ptr<AbstractChatRoom> chatRoom = core->findChatRoom(conferenceId, false);
if (chatRoom) {
- chatRooms.push_back(chatRoom);
+ chatRoomsMap.insert(std::make_pair(conferenceId, chatRoom));
continue;
}
@@ -5181,11 +5165,16 @@ list<shared_ptr<AbstractChatRoom>> MainDb::getChatRooms() const {
lDebug() << "Found chat room in DB: (peer=" << conferenceId.getPeerAddress()->toStringUriOnlyOrdered()
<< ", local=" << conferenceId.getLocalAddress()->toStringUriOnlyOrdered() << ").";
- addChatroomToList(chatRooms, chatRoom);
+ addChatroomToList(chatRoomsMap, chatRoom);
}
tr.commit();
+ std::list<std::shared_ptr<AbstractChatRoom>> chatRooms;
+ for (const auto &[conferenceId, chatRoom] : chatRoomsMap) {
+ chatRooms.push_back(chatRoom);
+ }
+
return chatRooms;
};
#else
diff --git a/src/db/main-db.h b/src/db/main-db.h
index e88fc8bebf..e601b03531 100644
--- a/src/db/main-db.h
+++ b/src/db/main-db.h
@@ -33,6 +33,7 @@
#include "chat/chat-message/chat-message.h"
#include "conference/conference-id.h"
#include "conference/conference-info.h"
+#include "conference/conference.h"
#include "core/core-accessor.h"
// =============================================================================
@@ -251,8 +252,11 @@ private:
L_DISABLE_COPY(MainDb);
void initCleanup();
- void addChatroomToList(std::list<std::shared_ptr<AbstractChatRoom>> &chatRooms,
- const std::shared_ptr<AbstractChatRoom> chatRoom) const;
+ void addChatroomToList(
+ std::map<ConferenceId, std::shared_ptr<AbstractChatRoom>, Conference::ConferenceIdCompare> &chatRoomsMap,
+ const std::shared_ptr<AbstractChatRoom> chatRoom) const;
+ std::shared_ptr<AbstractChatRoom> mergeChatRooms(const std::shared_ptr<AbstractChatRoom> chatRoom1,
+ const std::shared_ptr<AbstractChatRoom> chatRoom2) const;
};
LINPHONE_END_NAMESPACE
diff --git a/tester/audio_video_conference_tester.c b/tester/audio_video_conference_tester.c
index f34da0c012..72439e3aab 100644
--- a/tester/audio_video_conference_tester.c
+++ b/tester/audio_video_conference_tester.c
@@ -590,8 +590,39 @@ static void simple_conference_base(LinphoneCoreManager *marie,
BC_ASSERT_TRUE(wait_for_list(lcs, &marie->stat.number_of_LinphoneCallStreamsRunning,
initial_marie_stat.number_of_LinphoneCallStreamsRunning + 2, 3000));
+ bool_t marie_event_log_enabled =
+ linphone_config_get_bool(linphone_core_get_config(marie->lc), "misc", "conference_event_log_enabled", TRUE);
+ if (marie_event_log_enabled) {
+ bool_t marie_conference_server = linphone_core_conference_server_enabled(marie->lc);
+ BC_ASSERT_TRUE(wait_for_list(lcs, &marie->stat.number_of_LinphoneSubscriptionActive,
+ (initial_marie_stat.number_of_LinphoneSubscriptionActive + 1), 5000));
+ if (!marie_conference_server) {
+ BC_ASSERT_TRUE(wait_for_list(lcs, &marie->stat.number_of_NotifyFullStateReceived,
+ (initial_marie_stat.number_of_NotifyFullStateReceived + 1),
+ liblinphone_tester_sip_timeout));
+ }
+ }
+ bool_t laure_event_log_enabled =
+ linphone_config_get_bool(linphone_core_get_config(laure->lc), "misc", "conference_event_log_enabled", TRUE);
+ if (laure_event_log_enabled && marie_event_log_enabled) {
+ BC_ASSERT_TRUE(wait_for_list(lcs, &laure->stat.number_of_LinphoneSubscriptionActive,
+ (initial_laure_stat.number_of_LinphoneSubscriptionActive + 1), 5000));
+ BC_ASSERT_TRUE(wait_for_list(lcs, &laure->stat.number_of_NotifyFullStateReceived,
+ (initial_laure_stat.number_of_NotifyFullStateReceived + 1),
+ liblinphone_tester_sip_timeout));
+ }
+ bool_t pauline_event_log_enabled =
+ linphone_config_get_bool(linphone_core_get_config(pauline->lc), "misc", "conference_event_log_enabled", TRUE);
+ if (pauline_event_log_enabled && marie_event_log_enabled) {
+ BC_ASSERT_TRUE(wait_for_list(lcs, &pauline->stat.number_of_LinphoneSubscriptionActive,
+ (initial_pauline_stat.number_of_LinphoneSubscriptionActive + 1), 5000));
+ BC_ASSERT_TRUE(wait_for_list(lcs, &pauline->stat.number_of_NotifyFullStateReceived,
+ (initial_pauline_stat.number_of_NotifyFullStateReceived + 1),
+ liblinphone_tester_sip_timeout));
+ }
+
// wait a bit to ensure that should NOTIFYs be sent, they reach their destination
- wait_for_list(lcs, NULL, 0, 3000);
+ wait_for_list(lcs, NULL, 0, liblinphone_tester_sip_timeout);
// Check that laure received volumes from other participant's devices
LinphoneCall *laure_call = linphone_core_get_current_call(laure->lc);
diff --git a/tester/local_chat_tester.cpp b/tester/local_chat_tester.cpp
index 9239f51aa0..f022105b54 100644
--- a/tester/local_chat_tester.cpp
+++ b/tester/local_chat_tester.cpp
@@ -2727,9 +2727,6 @@ static void group_chat_room_with_duplications(void) {
// wait bit more to detect side effect if any
CoreManagerAssert({focus, marie, pauline, michelle, laure}).waitUntil(chrono::seconds(5), [] { return false; });
- account = linphone_core_get_default_account(laure.getLc());
- LinphoneAddress *oldLaureDeviceAddress = linphone_address_clone(linphone_account_get_contact_address(account));
-
ms_message("%s reinitializes its core", linphone_core_get_identity(laure.getLc()));
coresList = bctbx_list_remove(coresList, laure.getLc());
linphone_core_manager_reinit(laure.getCMgr());
@@ -2761,12 +2758,9 @@ static void group_chat_room_with_duplications(void) {
// Notify chat room that a participant has registered
bctbx_list_t *devices = NULL;
bctbx_list_t *specs = linphone_core_get_linphone_specs_list(laure.getLc());
- LinphoneParticipantDeviceIdentity *identity =
- linphone_factory_create_participant_device_identity(linphone_factory_get(), oldLaureDeviceAddress, "");
- linphone_participant_device_identity_set_capability_descriptor_2(identity, specs);
- devices = bctbx_list_append(devices, identity);
- identity = linphone_factory_create_participant_device_identity(linphone_factory_get(), laureDeviceAddress, "");
+ LinphoneParticipantDeviceIdentity *identity =
+ linphone_factory_create_participant_device_identity(linphone_factory_get(), laureDeviceAddress, "");
linphone_participant_device_identity_set_capability_descriptor_2(identity, specs);
devices = bctbx_list_append(devices, identity);
bctbx_list_free_with_data(specs, ms_free);
@@ -2830,7 +2824,7 @@ static void group_chat_room_with_duplications(void) {
}
BC_ASSERT_TRUE(wait_for_list(coresList, &michelle.getStats().number_of_LinphoneConferenceStateDeleted,
michelle_stat.number_of_LinphoneConferenceStateDeleted + nbChatrooms,
- liblinphone_tester_sip_timeout));
+ 3 * liblinphone_tester_sip_timeout));
BC_ASSERT_TRUE(wait_for_list(coresList, &marie.getStats().number_of_participants_removed,
marie_stat.number_of_participants_removed + nbChatrooms,
liblinphone_tester_sip_timeout));
@@ -2965,7 +2959,7 @@ static void group_chat_room_with_duplications(void) {
// wait until chatroom is deleted server side
BC_ASSERT_TRUE(
- CoreManagerAssert({focus, marie, pauline, michelle, laure}).waitUntil(chrono::seconds(90), [&focus] {
+ CoreManagerAssert({focus, marie, pauline, michelle, laure}).waitUntil(chrono::seconds(120), [&focus] {
return focus.getCore().getChatRooms().size() == 0;
}));
diff --git a/tester/video_quality_tester.c b/tester/video_quality_tester.c
index 1faff5a19a..47a389169b 100644
--- a/tester/video_quality_tester.c
+++ b/tester/video_quality_tester.c
@@ -279,7 +279,7 @@ static void video_call_expected_fps_for_specified_bandwidth(int bandwidth, int e
/*wait some time until the target fps is reached. Indeed the bandwidth measurement may take several
iterations to converge to a value big enough to allow mediastreamer2 to switch to the high fps profile*/
- for (count = 0; count < 3; count++) {
+ for (count = 0; count < 10; count++) {
/*wait for at least the first TMMBR to arrive*/
BC_ASSERT_TRUE(
wait_for_until(marie->lc, pauline->lc, &marie->stat.last_tmmbr_value_received, 1, 10000));
--
GitLab