Linkpreviewer Improvements

- Have LinkPreviewers stored in NeoChatConnection so that they don't have to be reloaded everytime the MessageContentModel is refreshed
- This means the link is never changed (it will be swiched for a new previewer with the new link)
- LinkPreviewers are stored by URL so they can be re-used by any event with the same URL

BUG: 484927  (because the offending code is ripped out)

autotests/linkpreviewertest.cpp

@@ -32,8 +32,6 @@ private Q_SLOTS:
 
     void linkPreviewsReject_data();
     void linkPreviewsReject();
-
-    void editedLink();
 };
 
 void LinkPreviewerTest::initTestCase()
@@ -59,7 +57,7 @@ void LinkPreviewerTest::linkPreviewsMatch()
     QFETCH(QUrl, testOutputLink);
 
     auto event = TestUtils::loadEventFromFile<RoomMessageEvent>(eventSource);
-    auto linkPreviewer = LinkPreviewer(room, event.get());
+    auto linkPreviewer = LinkPreviewer(LinkPreviewer::linkPreview(event.get()), connection);
 
     QCOMPARE(linkPreviewer.empty(), false);
     QCOMPARE(linkPreviewer.url(), testOutputLink);
@@ -79,22 +77,7 @@ void LinkPreviewerTest::linkPreviewsReject()
     QFETCH(QString, eventSource);
 
     auto event = TestUtils::loadEventFromFile<RoomMessageEvent>(eventSource);
-    auto linkPreviewer = LinkPreviewer(room, event.get());
-
-    QCOMPARE(linkPreviewer.empty(), true);
-    QCOMPARE(linkPreviewer.url(), QUrl());
-}
-
-void LinkPreviewerTest::editedLink()
-{
-    room->syncNewEvents(QStringLiteral("test-linkpreviewerintial-sync.json"));
-    auto event = eventCast<const RoomMessageEvent>(room->messageEvents().at(0).get());
-    auto linkPreviewer = LinkPreviewer(room, event);
-
-    QCOMPARE(linkPreviewer.empty(), false);
-    QCOMPARE(linkPreviewer.url(), QUrl("https://kde.org"_ls));
-
-    room->syncNewEvents(QStringLiteral("test-linkpreviewerreplace-sync.json"));
+    auto linkPreviewer = LinkPreviewer(LinkPreviewer::linkPreview(event.get()), connection);
 
     QCOMPARE(linkPreviewer.empty(), true);
     QCOMPARE(linkPreviewer.url(), QUrl());

src/linkpreviewer.cpp

@@ -8,34 +8,22 @@
 #include <Quotient/events/roommessageevent.h>
 
 #include "neochatconfig.h"
-#include "neochatroom.h"
+#include "neochatconnection.h"
 #include "utils.h"
 
 using namespace Quotient;
 
-LinkPreviewer::LinkPreviewer(const NeoChatRoom *room, const Quotient::RoomMessageEvent *event, QObject *parent)
+LinkPreviewer::LinkPreviewer(const QUrl &url, QObject *parent)
     : QObject(parent)
-    , m_currentRoom(room)
-    , m_event(event)
     , m_loaded(false)
-    , m_url(linkPreview(event))
+    , m_url(url)
 {
-    connect(this, &LinkPreviewer::urlChanged, this, &LinkPreviewer::emptyChanged);
+    Q_ASSERT(dynamic_cast<Connection *>(this->parent()));
 
-    if (m_event != nullptr && m_currentRoom != nullptr) {
-        loadUrlPreview();
-        connect(m_currentRoom, &NeoChatRoom::urlPreviewEnabledChanged, this, &LinkPreviewer::loadUrlPreview);
-        // Make sure that we react to edits
-        connect(m_currentRoom, &NeoChatRoom::replacedEvent, this, [this](const Quotient::RoomEvent *newEvent) {
-            if (m_event->id() == newEvent->id()) {
-                m_event = eventCast<const Quotient::RoomMessageEvent>(newEvent);
-                m_url = linkPreview(m_event);
-                Q_EMIT urlChanged();
-                loadUrlPreview();
-            }
-        });
-    }
+    connect(this, &LinkPreviewer::urlChanged, this, &LinkPreviewer::emptyChanged);
     connect(NeoChatConfig::self(), &NeoChatConfig::ShowLinkPreviewChanged, this, &LinkPreviewer::loadUrlPreview);
+
+    loadUrlPreview();
 }
 
 bool LinkPreviewer::loaded() const
@@ -65,14 +53,14 @@ QUrl LinkPreviewer::url() const
 
 void LinkPreviewer::loadUrlPreview()
 {
-    if (!m_currentRoom || !NeoChatConfig::showLinkPreview() || !m_currentRoom->urlPreviewEnabled()) {
-        return;
-    }
     if (m_url.scheme() == QStringLiteral("https")) {
         m_loaded = false;
         Q_EMIT loadedChanged();
 
-        auto conn = m_currentRoom->connection();
+        auto conn = dynamic_cast<Connection *>(this->parent());
+        if (conn == nullptr) {
+            return;
+        }
         GetUrlPreviewJob *job = conn->callApi<GetUrlPreviewJob>(m_url);
 
         connect(job, &BaseJob::success, this, [this, job, conn]() {

src/linkpreviewer.h

@@ -60,7 +60,8 @@ class LinkPreviewer : public QObject
     Q_PROPERTY(bool empty READ empty NOTIFY emptyChanged)
 
 public:
-    explicit LinkPreviewer(const NeoChatRoom *room = nullptr, const Quotient::RoomMessageEvent *event = nullptr, QObject *parent = nullptr);
+    LinkPreviewer() = default;
+    explicit LinkPreviewer(const QUrl &url, QObject *parent = nullptr);
 
     [[nodiscard]] QUrl url() const;
     [[nodiscard]] bool loaded() const;
@@ -76,18 +77,6 @@ public:
      */
     static bool hasPreviewableLinks(const Quotient::RoomMessageEvent *event);
 
-private:
-    const NeoChatRoom *m_currentRoom;
-    const Quotient::RoomMessageEvent *m_event;
-
-    bool m_loaded;
-    QString m_title = QString();
-    QString m_description = QString();
-    QUrl m_imageSource = QUrl();
-    QUrl m_url;
-
-    void loadUrlPreview();
-
     /**
      * @brief Return the link to be previewed from the given event.
      *
@@ -96,6 +85,15 @@ private:
      */
     static QUrl linkPreview(const Quotient::RoomMessageEvent *event);
 
+private:
+    bool m_loaded;
+    QString m_title = QString();
+    QString m_description = QString();
+    QUrl m_imageSource = QUrl();
+    QUrl m_url;
+
+    void loadUrlPreview();
+
 Q_SIGNALS:
     void loadedChanged();
     void titleChanged();

src/models/messagecontentmodel.cpp

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
 
 #include "messagecontentmodel.h"
+#include "neochatconfig.h"
 
 #include <QImageReader>
 
@@ -23,6 +24,7 @@
 #include "filetype.h"
 #include "itinerarymodel.h"
 #include "linkpreviewer.h"
+#include "neochatconnection.h"
 #include "neochatroom.h"
 #include "texthandler.h"
 
@@ -92,23 +94,11 @@ MessageContentModel::MessageContentModel(const Quotient::RoomEvent *event, NeoCh
                 endResetModel();
             }
         });
+        connect(m_room, &NeoChatRoom::urlPreviewEnabledChanged, this, &MessageContentModel::updateLinkPreviewer);
+        connect(NeoChatConfig::self(), &NeoChatConfig::ShowLinkPreviewChanged, this, &MessageContentModel::updateLinkPreviewer);
     }
 
-    if (const auto event = eventCast<const Quotient::RoomMessageEvent>(m_event)) {
-        if (LinkPreviewer::hasPreviewableLinks(event)) {
-            m_linkPreviewer = new LinkPreviewer(m_room, event, this);
-
-            connect(m_linkPreviewer, &LinkPreviewer::loadedChanged, [this]() {
-                if (m_linkPreviewer->loaded()) {
-                    // HACK: Because DelegateChooser can't switch the delegate on dataChanged it has to think there is a new delegate.
-                    beginResetModel();
-                    m_components[m_components.size() - 1].type = MessageComponentType::LinkPreview;
-                    endResetModel();
-                }
-            });
-        }
-    }
-
+    updateLinkPreviewer();
     updateComponents();
 }
 
@@ -319,6 +309,44 @@ void MessageContentModel::updateComponents(bool isEditing)
     endResetModel();
 }
 
+void MessageContentModel::updateLinkPreviewer()
+{
+    if (m_room == nullptr || m_event == nullptr) {
+        if (m_linkPreviewer != nullptr) {
+            m_linkPreviewer->disconnect(this);
+            m_linkPreviewer = nullptr;
+            updateComponents();
+        }
+        return;
+    }
+    if (!m_room->urlPreviewEnabled()) {
+        if (m_linkPreviewer != nullptr) {
+            m_linkPreviewer->disconnect(this);
+            m_linkPreviewer = nullptr;
+            updateComponents();
+        }
+        return;
+    }
+
+    if (const auto event = eventCast<const Quotient::RoomMessageEvent>(m_event)) {
+        if (LinkPreviewer::hasPreviewableLinks(event)) {
+            m_linkPreviewer = dynamic_cast<NeoChatConnection *>(m_room->connection())->previewerForLink(LinkPreviewer::linkPreview(event));
+            updateComponents();
+
+            if (m_linkPreviewer != nullptr) {
+                connect(m_linkPreviewer, &LinkPreviewer::loadedChanged, [this]() {
+                    if (m_linkPreviewer != nullptr && m_linkPreviewer->loaded()) {
+                        // HACK: Because DelegateChooser can't switch the delegate on dataChanged it has to think there is a new delegate.
+                        beginResetModel();
+                        m_components[m_components.size() - 1].type = MessageComponentType::LinkPreview;
+                        endResetModel();
+                    }
+                });
+            }
+        }
+    }
+}
+
 void MessageContentModel::updateItineraryModel()
 {
     if (m_room == nullptr || m_event == nullptr) {

src/models/messagecontentmodel.h

@@ -93,9 +93,10 @@ private:
     QList<MessageComponent> m_components;
     void updateComponents(bool isEditing = false);
 
-    LinkPreviewer *m_linkPreviewer = nullptr;
+    QPointer<LinkPreviewer> m_linkPreviewer;
     ItineraryModel *m_itineraryModel = nullptr;
 
+    void updateLinkPreviewer();
     void updateItineraryModel();
     bool m_emptyItinerary = false;
 };

src/neochatconnection.cpp

@@ -9,6 +9,8 @@
 #include "controller.h"
 #include "jobs/neochatchangepasswordjob.h"
 #include "jobs/neochatdeactivateaccountjob.h"
+#include "linkpreviewer.h"
+#include "neochatconfig.h"
 #include "neochatroom.h"
 #include "roommanager.h"
 #include "spacehierarchycache.h"
@@ -477,4 +479,20 @@ QString NeoChatConnection::accountDataJsonString(const QString &type) const
     return QString::fromUtf8(QJsonDocument(accountDataJson(type)).toJson());
 }
 
+LinkPreviewer *NeoChatConnection::previewerForLink(const QUrl &link)
+{
+    if (!NeoChatConfig::showLinkPreview()) {
+        return nullptr;
+    }
+
+    auto previewer = m_linkPreviewers.value(link, nullptr);
+    if (previewer != nullptr) {
+        return previewer;
+    }
+
+    previewer = new LinkPreviewer(link, this);
+    m_linkPreviewers[link] = previewer;
+    return previewer;
+}
+
 #include "moc_neochatconnection.cpp"

src/neochatconnection.h

@@ -9,6 +9,8 @@
 #include <QCoroTask>
 #include <Quotient/connection.h>
 
+class LinkPreviewer;
+
 class NeoChatConnection : public Quotient::Connection
 {
     Q_OBJECT
@@ -147,6 +149,8 @@ public:
 
     bool isOnline() const;
 
+    LinkPreviewer *previewerForLink(const QUrl &link);
+
 Q_SIGNALS:
     void labelChanged();
     void directChatNotificationsChanged();
@@ -166,4 +170,6 @@ private:
     void connectSignals();
 
     int m_badgeNotificationCount = 0;
+
+    QHash<QUrl, LinkPreviewer *> m_linkPreviewers;
 };