From 3164f8708b96194fce4db726b094e9a7fda48818 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Blin?= <sebastien.blin@savoirfairelinux.com> Date: Mon, 25 Jul 2022 12:07:22 -0400 Subject: [PATCH] conversationmodel: fix wrong ordering for messages Change-Id: Ida8cba2ea8d386abf3e5ff2bc024069860bfcda0 --- src/libclient/conversationmodel.cpp | 21 +++++++++++++++------ src/libclient/messagelistmodel.cpp | 28 ++++++++++++++-------------- src/libclient/messagelistmodel.h | 5 ++--- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/libclient/conversationmodel.cpp b/src/libclient/conversationmodel.cpp index 281373470..85cfa5d25 100644 --- a/src/libclient/conversationmodel.cpp +++ b/src/libclient/conversationmodel.cpp @@ -1189,7 +1189,7 @@ ConversationModel::sendMessage(const QString& uid, const QString& body, const QS bool ret = false; { - std::lock_guard<std::mutex> lk(pimpl_->interactionsLocks[convId]); + std::lock_guard<std::mutex> lk(pimpl_->interactionsLocks[newConv.uid]); ret = newConv.interactions->insert(std::pair<QString, interaction::Info>(msgId, msg)) .second; } @@ -1313,7 +1313,7 @@ ConversationModel::clearHistory(const QString& uid) storage::clearHistory(pimpl_->db, uid); // Update conversation { - std::lock_guard<std::mutex> lk(pimpl_->interactionsLocks[uid]); + std::lock_guard<std::mutex> lk(pimpl_->interactionsLocks[conversation.uid]); conversation.interactions->clear(); } storage::getHistory(pimpl_->db, conversation); // will contain "Conversation started" @@ -1400,7 +1400,7 @@ ConversationModel::clearInteractionsCache(const QString& convId) auto& conversation = pimpl_->conversations.at(conversationIdx); if (!conversation.isRequest && !conversation.needsSyncing && conversation.isSwarm()) { { - std::lock_guard<std::mutex> lk(pimpl_->interactionsLocks[convId]); + std::lock_guard<std::mutex> lk(pimpl_->interactionsLocks[conversation.uid]); conversation.interactions->clear(); } conversation.allMessagesLoaded = false; @@ -2447,6 +2447,15 @@ ConversationModelPimpl::insertSwarmInteraction(const QString& interactionId, bool insertAtBegin) { std::lock_guard<std::mutex> lk(interactionsLocks[conversation.uid]); + auto itExists = conversation.interactions->find(interactionId); + if (itExists != conversation.interactions->end()) { + // Erase interaction if exists, as it will be updated via a re-insertion + itExists = conversation.interactions->erase(itExists); + if (itExists != conversation.interactions->end()) { + // next interaction doesn't have parent anymore. + conversation.parentsId[itExists->first] = interactionId; + } + } int index = conversation.interactions->indexOfMessage(interaction.parentId); if (index >= 0) { auto result = conversation.interactions->insert(index + 1, @@ -2996,7 +3005,7 @@ ConversationModelPimpl::addConversationWith(const QString& convId, storage::getHistory(db, conversation); std::vector<std::function<void(void)>> updateSlots; { - std::lock_guard<std::mutex> lk(interactionsLocks[convId]); + std::lock_guard<std::mutex> lk(interactionsLocks[conversation.uid]); for (auto& interaction : (*(conversation.interactions))) { if (interaction.second.status != interaction::Status::SENDING) { continue; @@ -4001,7 +4010,7 @@ ConversationModelPimpl::slotTransferStatusFinished(const QString& fileId, datatr auto newStatus = interaction::Status::TRANSFER_FINISHED; interaction::Info itCopy; { - std::lock_guard<std::mutex> lk(interactionsLocks[conversationId]); + std::lock_guard<std::mutex> lk(interactionsLocks[conversations[conversationIdx].uid]); auto& interactions = conversations[conversationIdx].interactions; auto it = interactions->find(interactionId); if (it != interactions->end()) { @@ -4091,7 +4100,7 @@ ConversationModelPimpl::updateTransferStatus(const QString& fileId, bool emitUpdated = false; interaction::Info itCopy; { - std::lock_guard<std::mutex> lk(interactionsLocks[conversationId]); + std::lock_guard<std::mutex> lk(interactionsLocks[conversations[conversationIdx].uid]); auto& interactions = conversations[conversationIdx].interactions; auto it = interactions->find(interactionId); if (it != interactions->end()) { diff --git a/src/libclient/messagelistmodel.cpp b/src/libclient/messagelistmodel.cpp index 8bd33ebea..ac79134b5 100644 --- a/src/libclient/messagelistmodel.cpp +++ b/src/libclient/messagelistmodel.cpp @@ -64,6 +64,16 @@ MessageListModel::find(const QString& msgId) return interactions_.end(); } +iterator +MessageListModel::erase(const iterator& it) +{ + auto index = std::distance(begin(), it); + Q_EMIT beginRemoveRows(QModelIndex(), index, index); + auto erased = interactions_.erase(it); + Q_EMIT endRemoveRows(); + return erased; +} + constIterator MessageListModel::find(const QString& msgId) const { @@ -157,9 +167,11 @@ MessageListModel::size() const } void -MessageListModel::clear(int leaveN) +MessageListModel::clear() { - interactions_.erase(interactions_.begin(), std::prev(interactions_.end(), leaveN)); + Q_EMIT beginResetModel(); + interactions_.clear(); + Q_EMIT endResetModel(); } bool @@ -446,18 +458,6 @@ MessageListModel::getRead(const QString& peer) return ""; } -void -MessageListModel::emitBeginResetModel() -{ - Q_EMIT beginResetModel(); -} - -void -MessageListModel::emitEndResetModel() -{ - Q_EMIT endResetModel(); -} - void MessageListModel::emitDataChanged(iterator it, VectorInt roles) { diff --git a/src/libclient/messagelistmodel.h b/src/libclient/messagelistmodel.h index f369a3b6d..3d36e577c 100644 --- a/src/libclient/messagelistmodel.h +++ b/src/libclient/messagelistmodel.h @@ -77,6 +77,7 @@ public: interaction::Info message, bool beginning = false); iterator find(const QString& msgId); + iterator erase(const iterator& it); constIterator find(const QString& msgId) const; QPair<iterator, bool> insert(std::pair<QString, interaction::Info> message, bool beginning = false); @@ -89,7 +90,7 @@ public: constIterator begin() const; reverseIterator rbegin(); int size() const; - void clear(int leaveN = 0); + void clear(); bool empty() const; interaction::Info at(const QString& intId) const; QPair<QString, interaction::Info> front() const; @@ -116,8 +117,6 @@ public: // Note: this is not ideal, and this class should be refactored into a proper // view model and absorb the interaction management logic to avoid exposing // these emission wrappers - void emitBeginResetModel(); - void emitEndResetModel(); void emitDataChanged(iterator it, VectorInt roles = {}); void emitDataChanged(const QString& msgId, VectorInt roles = {}); -- GitLab