fix filedownload on maximized view and context menu

- move the logic for remembering local filenames from
  messagecontentmodel to neochatroom
- use it also when maximized and in context menu opening, to stop
  re-downloading
- make onFileTransferCompleted signal connection one-shot (stops memory
  leak)

src/models/mediamessagefiltermodel.cpp

@@ -30,40 +30,6 @@ bool MediaMessageFilterModel::filterAcceptsRow(int sourceRow, const QModelIndex
 
 QVariant MediaMessageFilterModel::data(const QModelIndex &index, int role) const
 {
-    if (role == SourceRole) {
-        if (mapToSource(index).data(MessageEventModel::MediaInfoRole).toMap()[QLatin1String("mimeType")].toString().contains(QLatin1String("image"))) {
-            return mapToSource(index).data(MessageEventModel::MediaInfoRole).toMap()[QStringLiteral("source")].toUrl();
-        } else if (mapToSource(index).data(MessageEventModel::MediaInfoRole).toMap()[QLatin1String("mimeType")].toString().contains(QLatin1String("video"))) {
-            auto progressInfo = mapToSource(index).data(MessageEventModel::ProgressInfoRole).value<Quotient::FileTransferInfo>();
-
-            if (progressInfo.completed()) {
-                return mapToSource(index).data(MessageEventModel::ProgressInfoRole).value<Quotient::FileTransferInfo>().localPath;
-            } else {
-                return QUrl();
-            }
-        } else {
-            return QUrl();
-        }
-    }
-    if (role == TempSourceRole) {
-        return mapToSource(index).data(MessageEventModel::MediaInfoRole).toMap()[QStringLiteral("tempInfo")].toMap()[QStringLiteral("source")].toUrl();
-    }
-    if (role == TypeRole) {
-        if (mapToSource(index).data(MessageEventModel::MediaInfoRole).toMap()[QLatin1String("mimeType")].toString().contains(QLatin1String("image"))) {
-            return MediaType::Image;
-        } else {
-            return MediaType::Video;
-        }
-    }
-    if (role == CaptionRole) {
-        return mapToSource(index).data(Qt::DisplayRole);
-    }
-    if (role == SourceWidthRole) {
-        return mapToSource(index).data(MessageEventModel::MediaInfoRole).toMap()[QStringLiteral("width")].toFloat();
-    }
-    if (role == SourceHeightRole) {
-        return mapToSource(index).data(MessageEventModel::MediaInfoRole).toMap()[QStringLiteral("height")].toFloat();
-    }
     // We need to catch this one and return true if the next media object was
     // on a different day.
     if (role == MessageEventModel::ShowSectionRole) {
@@ -80,6 +46,37 @@ QVariant MediaMessageFilterModel::data(const QModelIndex &index, int role) const
         return QVariant::fromValue<MessageContentModel *>(model);
     }
 
+    QVariantMap mediaInfo = mapToSource(index).data(MessageEventModel::MediaInfoRole).toMap();
+
+    if (role == TempSourceRole) {
+        return mediaInfo[QStringLiteral("tempInfo")].toMap()[QStringLiteral("source")].toUrl();
+    }
+    if (role == CaptionRole) {
+        return mapToSource(index).data(Qt::DisplayRole);
+    }
+    if (role == SourceWidthRole) {
+        return mediaInfo[QStringLiteral("width")].toFloat();
+    }
+    if (role == SourceHeightRole) {
+        return mediaInfo[QStringLiteral("height")].toFloat();
+    }
+
+    bool isVideo = mediaInfo[QStringLiteral("mimeType")].toString().contains(QStringLiteral("video"));
+
+    if (role == TypeRole) {
+        return (isVideo) ? MediaType::Video : MediaType::Image;
+    }
+    if (role == SourceRole) {
+        if (isVideo) {
+            auto progressInfo = mapToSource(index).data(MessageEventModel::ProgressInfoRole).value<Quotient::FileTransferInfo>();
+            if (progressInfo.completed()) {
+                return mapToSource(index).data(MessageEventModel::ProgressInfoRole).value<Quotient::FileTransferInfo>().localPath;
+            }
+        } else {
+            return mediaInfo[QStringLiteral("source")].toUrl();
+        }
+    }
+
     return sourceModel()->data(mapToSource(index), role);
 }
 

src/models/messagecontentmodel.cpp

@@ -108,24 +108,9 @@ void MessageContentModel::initializeModel()
         }
     });
     connect(m_room, &NeoChatRoom::fileTransferCompleted, this, [this](const QString &eventId) {
-        if (m_event != nullptr && eventId == m_eventId) {
+        if (m_room != nullptr && m_event != nullptr && eventId == m_eventId) {
             resetContent();
             Q_EMIT dataChanged(index(0), index(rowCount() - 1), {FileTransferInfoRole});
-
-            QString mxcUrl;
-            if (auto event = eventCast<const Quotient::RoomMessageEvent>(m_event)) {
-                if (event->hasFileContent()) {
-                    mxcUrl = event->content()->fileInfo()->url().toString();
-                }
-            } else if (auto event = eventCast<const Quotient::StickerEvent>(m_event)) {
-                mxcUrl = event->image().fileInfo()->url().toString();
-            }
-            if (mxcUrl.isEmpty()) {
-                return;
-            }
-            auto localPath = m_room->fileTransferInfo(m_eventId).localPath.toLocalFile();
-            auto config = KSharedConfig::openStateConfig(QStringLiteral("neochatdownloads"))->group(QStringLiteral("downloads"));
-            config.writePathEntry(mxcUrl.mid(6), localPath);
         }
     });
     connect(m_room, &NeoChatRoom::fileTransferFailed, this, [this](const QString &eventId) {
@@ -260,7 +245,7 @@ QVariant MessageContentModel::data(const QModelIndex &index, int role) const
         return eventHandler.getMediaInfo();
     }
     if (role == FileTransferInfoRole) {
-        return QVariant::fromValue(fileInfo());
+        return QVariant::fromValue(m_room->cachedFileTransferInfo(m_event.get()));
     }
     if (role == ItineraryModelRole) {
         return QVariant::fromValue<ItineraryModel *>(m_itineraryModel);
@@ -440,7 +425,7 @@ QList<MessageComponent> MessageContentModel::componentsForType(MessageComponentT
 
         if (m_emptyItinerary) {
             if (!m_isReply) {
-                auto fileTransferInfo = fileInfo();
+                auto fileTransferInfo = m_room->cachedFileTransferInfo(m_event.get());
 
 #ifndef Q_OS_ANDROID
                 Q_ASSERT(event->content() != nullptr && event->content()->fileInfo() != nullptr);
@@ -565,7 +550,7 @@ void MessageContentModel::updateItineraryModel()
 
     if (auto event = eventCast<const Quotient::RoomMessageEvent>(m_event)) {
         if (event->hasFileContent()) {
-            auto filePath = fileInfo().localPath;
+            auto filePath = m_room->cachedFileTransferInfo(m_event.get()).localPath;
             if (filePath.isEmpty() && m_itineraryModel != nullptr) {
                 delete m_itineraryModel;
                 m_itineraryModel = nullptr;
@@ -593,42 +578,4 @@ void MessageContentModel::updateItineraryModel()
     }
 }
 
-FileTransferInfo MessageContentModel::fileInfo() const
-{
-    if (m_room == nullptr || m_event == nullptr) {
-        return {};
-    }
-
-    QString mxcUrl;
-    int total;
-    if (auto event = eventCast<const Quotient::RoomMessageEvent>(m_event)) {
-        if (event->hasFileContent()) {
-            mxcUrl = event->content()->fileInfo()->url().toString();
-            total = event->content()->fileInfo()->payloadSize;
-        }
-    } else if (auto event = eventCast<const Quotient::StickerEvent>(m_event)) {
-        mxcUrl = event->image().fileInfo()->url().toString();
-        total = event->image().fileInfo()->payloadSize;
-    }
-    auto config = KSharedConfig::openStateConfig(QStringLiteral("neochatdownloads"))->group(QStringLiteral("downloads"));
-    if (!config.hasKey(mxcUrl.mid(6))) {
-        return m_room->fileTransferInfo(m_eventId);
-    }
-    const auto path = config.readPathEntry(mxcUrl.mid(6), QString());
-    QFileInfo info(path);
-    if (!info.isFile()) {
-        config.deleteEntry(mxcUrl);
-        return m_room->fileTransferInfo(m_eventId);
-    }
-    // TODO: we could check the hash here
-    return FileTransferInfo{
-        .status = FileTransferInfo::Completed,
-        .isUpload = false,
-        .progress = total,
-        .total = total,
-        .localDir = QUrl(info.dir().path()),
-        .localPath = QUrl::fromLocalFile(path),
-    };
-}
-
 #include "moc_messagecontentmodel.cpp"

src/models/messagecontentmodel.h

@@ -141,6 +141,4 @@ private:
 
     void updateItineraryModel();
     bool m_emptyItinerary = false;
-
-    Quotient::FileTransferInfo fileInfo() const;
 };

src/models/messageeventmodel.cpp

@@ -514,11 +514,11 @@ QVariant MessageEventModel::data(const QModelIndex &idx, int role) const
     if (role == ProgressInfoRole) {
         if (auto e = eventCast<const RoomMessageEvent>(&evt)) {
             if (e->hasFileContent()) {
-                return QVariant::fromValue(m_currentRoom->fileTransferInfo(e->id()));
+                return QVariant::fromValue(m_currentRoom->cachedFileTransferInfo(&evt));
             }
         }
-        if (auto e = eventCast<const StickerEvent>(&evt)) {
+        if (eventCast<const StickerEvent>(&evt)) {
-            return QVariant::fromValue(m_currentRoom->fileTransferInfo(e->id()));
+            return QVariant::fromValue(m_currentRoom->cachedFileTransferInfo(&evt));
         }
     }
 

src/neochatroom.cpp

@@ -69,6 +69,26 @@ NeoChatRoom::NeoChatRoom(Connection *connection, QString roomId, JoinState joinS
         setFileUploadingProgress(0);
         setHasFileUploading(false);
     });
+    connect(this, &Room::fileTransferCompleted, this, [this](QString eventId) {
+        const auto evtIt = findInTimeline(eventId);
+        if (evtIt != messageEvents().rend()) {
+            const auto m_event = evtIt->viewAs<RoomEvent>();
+            QString mxcUrl;
+            if (auto event = eventCast<const Quotient::RoomMessageEvent>(m_event)) {
+                if (event->hasFileContent()) {
+                    mxcUrl = event->content()->fileInfo()->url().toString();
+                }
+            } else if (auto event = eventCast<const Quotient::StickerEvent>(m_event)) {
+                mxcUrl = event->image().fileInfo()->url().toString();
+            }
+            if (mxcUrl.isEmpty()) {
+                return;
+            }
+            auto localPath = this->fileTransferInfo(eventId).localPath.toLocalFile();
+            auto config = KSharedConfig::openStateConfig(QStringLiteral("neochatdownloads"))->group(QStringLiteral("downloads"));
+            config.writePathEntry(mxcUrl.mid(6), localPath);
+        }
+    });
 
     connect(this, &Room::addedMessages, this, &NeoChatRoom::readMarkerLoadedChanged);
     connect(this, &Room::aboutToAddHistoricalMessages, this, &NeoChatRoom::cleanupExtraEventRange);
@@ -1370,7 +1390,7 @@ void NeoChatRoom::openEventMediaExternally(const QString &eventId)
     if (evtIt != messageEvents().rend() && is<RoomMessageEvent>(**evtIt)) {
         const auto event = evtIt->viewAs<RoomMessageEvent>();
         if (event->hasFileContent()) {
-            const auto transferInfo = fileTransferInfo(eventId);
+            const auto transferInfo = cachedFileTransferInfo(event);
             if (transferInfo.completed()) {
                 UrlHelper helper;
                 helper.openUrl(transferInfo.localPath);
@@ -1378,15 +1398,20 @@ void NeoChatRoom::openEventMediaExternally(const QString &eventId)
                 downloadFile(eventId,
                              QUrl(QStandardPaths::writableLocation(QStandardPaths::CacheLocation) + u'/'
                                   + event->id().replace(u':', u'_').replace(u'/', u'_').replace(u'+', u'_') + fileNameToDownload(eventId)));
-                connect(this, &Room::fileTransferCompleted, this, [this, eventId](QString id, QUrl localFile, FileSourceInfo fileMetadata) {
+                connect(
-                    Q_UNUSED(localFile);
+                    this,
-                    Q_UNUSED(fileMetadata);
+                    &Room::fileTransferCompleted,
-                    if (id == eventId) {
+                    this,
-                        auto transferInfo = fileTransferInfo(eventId);
+                    [this, eventId](QString id, QUrl localFile, FileSourceInfo fileMetadata) {
-                        UrlHelper helper;
+                        Q_UNUSED(localFile);
-                        helper.openUrl(transferInfo.localPath);
+                        Q_UNUSED(fileMetadata);
-                    }
+                        if (id == eventId) {
-                });
+                            auto transferInfo = fileTransferInfo(eventId);
+                            UrlHelper helper;
+                            helper.openUrl(transferInfo.localPath);
+                        }
+                    },
+                    static_cast<Qt::ConnectionType>(Qt::SingleShotConnection));
             }
         }
     }
@@ -1406,20 +1431,66 @@ void NeoChatRoom::copyEventMedia(const QString &eventId)
                 downloadFile(eventId,
                              QUrl(QStandardPaths::writableLocation(QStandardPaths::CacheLocation) + u'/'
                                   + event->id().replace(u':', u'_').replace(u'/', u'_').replace(u'+', u'_') + fileNameToDownload(eventId)));
-                connect(this, &Room::fileTransferCompleted, this, [this, eventId](QString id, QUrl localFile, FileSourceInfo fileMetadata) {
+                connect(
-                    Q_UNUSED(localFile);
+                    this,
-                    Q_UNUSED(fileMetadata);
+                    &Room::fileTransferCompleted,
-                    if (id == eventId) {
+                    this,
-                        auto transferInfo = fileTransferInfo(eventId);
+                    [this, eventId](QString id, QUrl localFile, FileSourceInfo fileMetadata) {
-                        Clipboard clipboard;
+                        Q_UNUSED(localFile);
-                        clipboard.setImage(transferInfo.localPath);
+                        Q_UNUSED(fileMetadata);
-                    }
+                        if (id == eventId) {
-                });
+                            auto transferInfo = fileTransferInfo(eventId);
+                            Clipboard clipboard;
+                            clipboard.setImage(transferInfo.localPath);
+                        }
+                    },
+                    static_cast<Qt::ConnectionType>(Qt::SingleShotConnection));
             }
         }
     }
 }
 
+FileTransferInfo NeoChatRoom::cachedFileTransferInfo(const Quotient::RoomEvent *event) const
+{
+    QString mxcUrl;
+    int total = 0;
+    if (auto evt = eventCast<const Quotient::RoomMessageEvent>(event)) {
+        if (evt->hasFileContent()) {
+            mxcUrl = evt->content()->fileInfo()->url().toString();
+            total = evt->content()->fileInfo()->payloadSize;
+        }
+    } else if (auto evt = eventCast<const Quotient::StickerEvent>(event)) {
+        mxcUrl = evt->image().fileInfo()->url().toString();
+        total = evt->image().fileInfo()->payloadSize;
+    }
+
+    FileTransferInfo transferInfo = fileTransferInfo(event->id());
+    if (transferInfo.active()) {
+        return transferInfo;
+    }
+
+    auto config = KSharedConfig::openStateConfig(QStringLiteral("neochatdownloads"))->group(QStringLiteral("downloads"));
+    if (!config.hasKey(mxcUrl.mid(6))) {
+        return transferInfo;
+    }
+
+    const auto path = config.readPathEntry(mxcUrl.mid(6), QString());
+    QFileInfo info(path);
+    if (!info.isFile()) {
+        config.deleteEntry(mxcUrl);
+        return transferInfo;
+    }
+    // TODO: we could check the hash here
+    return FileTransferInfo{
+        .status = FileTransferInfo::Completed,
+        .isUpload = false,
+        .progress = total,
+        .total = total,
+        .localDir = QUrl(info.dir().path()),
+        .localPath = QUrl::fromLocalFile(path),
+    };
+}
+
 ChatBarCache *NeoChatRoom::mainCache() const
 {
     return m_mainCache;

src/neochatroom.h

@@ -579,6 +579,14 @@ public:
      */
     Q_INVOKABLE QString invitingUserId() const;
 
+    /**
+     * @brief Return the cached file transfer information for the event.
+     *
+     * If we downloaded the file previously, return a struct with Completed status
+     * and the local file path stored in KSharedCOnfig
+     */
+    Quotient::FileTransferInfo cachedFileTransferInfo(const Quotient::RoomEvent *event) const;
+
 private:
     QSet<const Quotient::RoomEvent *> highlights;