From 96c00ff019b3fe41582e3e25b30d4bdfaa6c9a09 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois-Simon=20Fauteux-Chapleau?=
<francois-simon.fauteux-chapleau@savoirfairelinux.com>
Date: Thu, 22 Aug 2024 11:22:16 -0400
Subject: [PATCH] interaction: set body and transferStatus of DATA_TRANSFER
messages
This patch adds code in the interaction::Info::init function so that the
"body" and "transferStatus" fields are always set when an Info struct is
constructed for a message of type DATA_TRANSFER.
This removes some code duplication in conversationmodel.cpp, where these
fields were being set as an extra step after construction in three
different places.
It also fixes a bug in the ConversationModelPimpl::slotMessageUpdated
function, which did *not* set the "body" of DATA_TRANSFER messages. The
body was therefore empty instead of containing a file path, which is
what caused the image preview bug described in the following issue:
GitLab: #1671
The patch also reverts a change that was made in the
MessageListModel::update function by commit
d2eba1d91ebd25362364699200421912bd77bb66. This change was a workaround
for the above bug, but it is no longer necessary (and it broke message
deletion, which relies on the body of the deleted message being set to
the empty string).
GitLab: #1825
Change-Id: I5848b93a12c1ef7b3735c5c6db6b32a9bbc4041d
---
src/libclient/api/interaction.h | 41 ++++++++++++++++---
src/libclient/conversationmodel.cpp | 61 +++--------------------------
src/libclient/messagelistmodel.cpp | 7 +---
3 files changed, 43 insertions(+), 66 deletions(-)
diff --git a/src/libclient/api/interaction.h b/src/libclient/api/interaction.h
index f4ff6074c..5c184305e 100644
--- a/src/libclient/api/interaction.h
+++ b/src/libclient/api/interaction.h
@@ -20,9 +20,11 @@
#include <QString>
#include <QObject>
+#include <QFileInfo>
#include <ctime>
#include "typedefs.h"
+#include "../dbus/configurationmanager.h"
namespace lrc {
@@ -454,7 +456,10 @@ struct Info
return status == Status::SUCCESS || status == Status::DISPLAYED;
}
- void init(const MapStringString& message, const QString& accountURI)
+ void init(const MapStringString& message,
+ const QString& accountURI,
+ const QString& accountId,
+ const QString& conversationId)
{
type = to_type(message["type"]);
if (message.contains("react-to") && type == Type::TEXT) {
@@ -482,16 +487,42 @@ struct Info
duration = message["duration"].toInt() / 1000;
if (message.contains("confId"))
confId = message["confId"];
+ } else if (type == Type::DATA_TRANSFER) {
+ QString path;
+ qlonglong bytesProgress, totalSize;
+ ConfigurationManager::instance().fileTransferInfo(accountId,
+ conversationId,
+ message["fileId"],
+ path,
+ totalSize,
+ bytesProgress);
+ QFileInfo fi(path);
+ body = fi.isSymLink() ? fi.symLinkTarget() : path;
+ transferStatus = bytesProgress == 0 ? TransferStatus::TRANSFER_AWAITING_HOST
+ : bytesProgress == totalSize ? TransferStatus::TRANSFER_FINISHED
+ : TransferStatus::TRANSFER_ONGOING;
+
}
commit = message;
}
- Info(const MapStringString& message, const QString& accountURI)
+ // NOTE: The `accountId` and `conversationId` arguments are only used for messages of
+ // type DATA_TRANSFER. They can therefore be omitted if the caller knows that `message`
+ // is of a different type. They must be provided otherwise, as failure to do so would
+ // result in the `body` and `transferStatus` fields of the returned Info struct to
+ // contain incorrect information whenever `message` is of type DATA_TRANSFER.
+ Info(const MapStringString& message,
+ const QString& accountURI,
+ const QString& accountId = "",
+ const QString& conversationId = "")
{
- init(message, accountURI);
+ init(message, accountURI, accountId, conversationId);
}
- Info(const SwarmMessage& msg, const QString& accountUri)
+ Info(const SwarmMessage& msg,
+ const QString& accountUri,
+ const QString& accountId,
+ const QString& conversationId)
{
MapStringString msgBody;
for (auto it = msg.body.cbegin(); it != msg.body.cend(); ++it) {
@@ -499,7 +530,7 @@ struct Info
const auto& value = it.value();
msgBody.insert(key, value);
}
- init(msgBody, accountUri);
+ init(msgBody, accountUri, accountId, conversationId);
parentId = msg.linearizedParent;
type = to_type(msg.type);
for (const auto& edition : msg.editions)
diff --git a/src/libclient/conversationmodel.cpp b/src/libclient/conversationmodel.cpp
index bfcf74251..6c182309e 100644
--- a/src/libclient/conversationmodel.cpp
+++ b/src/libclient/conversationmodel.cpp
@@ -2285,31 +2285,14 @@ ConversationModelPimpl::slotSwarmLoaded(uint32_t requestId,
auto& conversation = getConversationForUid(conversationId).get();
for (const auto& message : messages) {
QString msgId = message.id;
- auto msg = interaction::Info(message, linked.owner.profileInfo.uri);
+ auto msg = interaction::Info(message, linked.owner.profileInfo.uri, accountId, conversationId);
auto downloadFile = false;
if (msg.type == interaction::Type::INITIAL) {
allLoaded = true;
} else if (msg.type == interaction::Type::DATA_TRANSFER) {
QString fileId = message.body.value("fileId");
- QString path;
- qlonglong bytesProgress, totalSize;
- linked.owner.dataTransferModel->fileTransferInfo(accountId,
- conversationId,
- fileId,
- path,
- totalSize,
- bytesProgress);
- QFileInfo fi(path);
- if (fi.isSymLink()) {
- msg.body = fi.symLinkTarget();
- } else {
- msg.body = path;
- }
- msg.transferStatus = bytesProgress == 0 ? interaction::TransferStatus::TRANSFER_AWAITING_HOST
- : bytesProgress == totalSize ? interaction::TransferStatus::TRANSFER_FINISHED
- : interaction::TransferStatus::TRANSFER_ONGOING;
linked.owner.dataTransferModel->registerTransferId(fileId, msgId);
- downloadFile = (bytesProgress == 0);
+ downloadFile = (msg.transferStatus == interaction::TransferStatus::TRANSFER_AWAITING_HOST);
}
// If message is loaded, insert message at beginning
@@ -2351,25 +2334,12 @@ ConversationModelPimpl::slotMessagesFound(uint32_t requestId,
QMap<QString, interaction::Info> messageDetailedInformation;
if (requestId == mediaResearchRequestId) {
Q_FOREACH (const MapStringString& msg, messageIds) {
- auto intInfo = interaction::Info(msg, "");
- if (intInfo.type == interaction::Type::DATA_TRANSFER) {
- auto fileId = msg["fileId"];
-
- QString path;
- qlonglong bytesProgress, totalSize;
- linked.owner.dataTransferModel->fileTransferInfo(accountId,
- conversationId,
- fileId,
- path,
- totalSize,
- bytesProgress);
- intInfo.body = path;
- }
+ auto intInfo = interaction::Info(msg, "", accountId, conversationId);
messageDetailedInformation[msg["id"]] = std::move(intInfo);
}
} else if (requestId == msgResearchRequestId) {
Q_FOREACH (const MapStringString& msg, messageIds) {
- auto intInfo = interaction::Info(msg, "");
+ auto intInfo = interaction::Info(msg, "", accountId, conversationId);
if (intInfo.type == interaction::Type::TEXT) {
messageDetailedInformation[msg["id"]] = std::move(intInfo);
}
@@ -2395,33 +2365,14 @@ ConversationModelPimpl::slotMessageReceived(const QString& accountId,
}
}
QString msgId = message.id;
- auto msg = interaction::Info(message, linked.owner.profileInfo.uri);
+ auto msg = interaction::Info(message, linked.owner.profileInfo.uri, accountId, conversationId);
if (msg.type == interaction::Type::CALL) {
msg.body = interaction::getCallInteractionString(msg.authorUri
== linked.owner.profileInfo.uri,
msg);
} else if (msg.type == interaction::Type::DATA_TRANSFER) {
- // save data transfer interaction to db and assosiate daemon id with interaction id,
- // conversation id and db id
QString fileId = message.body.value("fileId");
- QString path;
- qlonglong bytesProgress, totalSize;
- linked.owner.dataTransferModel->fileTransferInfo(accountId,
- conversationId,
- fileId,
- path,
- totalSize,
- bytesProgress);
- QFileInfo fi(path);
- if (fi.isSymLink()) {
- msg.body = fi.symLinkTarget();
- } else {
- msg.body = path;
- }
- msg.transferStatus = bytesProgress == 0 ? interaction::TransferStatus::TRANSFER_AWAITING_HOST
- : bytesProgress == totalSize ? interaction::TransferStatus::TRANSFER_FINISHED
- : interaction::TransferStatus::TRANSFER_ONGOING;
linked.owner.dataTransferModel->registerTransferId(fileId, msgId);
}
@@ -2473,7 +2424,7 @@ ConversationModelPimpl::slotMessageUpdated(const QString& accountId,
try {
auto& conversation = getConversationForUid(conversationId).get();
QString msgId = message.id;
- auto msg = interaction::Info(message, linked.owner.profileInfo.uri);
+ auto msg = interaction::Info(message, linked.owner.profileInfo.uri, accountId, conversationId);
if (!conversation.interactions->update(msgId, msg)) {
qDebug() << "Message not found or cannot be reparented.";
diff --git a/src/libclient/messagelistmodel.cpp b/src/libclient/messagelistmodel.cpp
index 75c4299f3..be4ae6c49 100644
--- a/src/libclient/messagelistmodel.cpp
+++ b/src/libclient/messagelistmodel.cpp
@@ -201,12 +201,7 @@ MessageListModel::update(const QString& id, const interaction::Info& interaction
return true;
}
}
- // TODO: look into why this update with an empty body is broadcasted just
- // after loading the messages. This is a workaround to avoid the empty
- // file transfer path. Until then, don't update the body if it's empty.
- if (!interaction.body.isEmpty()) {
- current.body = interaction.body;
- }
+ current.body = interaction.body;
current.commit = interaction.commit;
current.previousBodies = interaction.previousBodies;
current.parsedBody = interaction.parsedBody;
--
GitLab