From 1a96899336f2f1a916207796b482ca4112909ebf Mon Sep 17 00:00:00 2001 From: James Graham Date: Tue, 9 Apr 2024 18:35:16 +0000 Subject: [PATCH] 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 | 21 ++--------- src/linkpreviewer.cpp | 34 ++++++------------ src/linkpreviewer.h | 24 ++++++------- src/models/messagecontentmodel.cpp | 58 ++++++++++++++++++++++-------- src/models/messagecontentmodel.h | 3 +- src/neochatconnection.cpp | 18 ++++++++++ src/neochatconnection.h | 6 ++++ 7 files changed, 93 insertions(+), 71 deletions(-) diff --git a/autotests/linkpreviewertest.cpp b/autotests/linkpreviewertest.cpp index 487b58355..7dddc7fe0 100644 --- a/autotests/linkpreviewertest.cpp +++ b/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(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(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(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()); diff --git a/src/linkpreviewer.cpp b/src/linkpreviewer.cpp index 78e75b935..77406950e 100644 --- a/src/linkpreviewer.cpp +++ b/src/linkpreviewer.cpp @@ -8,34 +8,22 @@ #include #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(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(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(this->parent()); + if (conn == nullptr) { + return; + } GetUrlPreviewJob *job = conn->callApi(m_url); connect(job, &BaseJob::success, this, [this, job, conn]() { diff --git a/src/linkpreviewer.h b/src/linkpreviewer.h index 18554f540..27f0e23a2 100644 --- a/src/linkpreviewer.h +++ b/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(); diff --git a/src/models/messagecontentmodel.cpp b/src/models/messagecontentmodel.cpp index d7123470f..77dfbff5e 100644 --- a/src/models/messagecontentmodel.cpp +++ b/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 @@ -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(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(m_event)) { + if (LinkPreviewer::hasPreviewableLinks(event)) { + m_linkPreviewer = dynamic_cast(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) { diff --git a/src/models/messagecontentmodel.h b/src/models/messagecontentmodel.h index 6561d1e4a..0f46c3be7 100644 --- a/src/models/messagecontentmodel.h +++ b/src/models/messagecontentmodel.h @@ -93,9 +93,10 @@ private: QList m_components; void updateComponents(bool isEditing = false); - LinkPreviewer *m_linkPreviewer = nullptr; + QPointer m_linkPreviewer; ItineraryModel *m_itineraryModel = nullptr; + void updateLinkPreviewer(); void updateItineraryModel(); bool m_emptyItinerary = false; }; diff --git a/src/neochatconnection.cpp b/src/neochatconnection.cpp index 694d78d6f..9bf4fab53 100644 --- a/src/neochatconnection.cpp +++ b/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" diff --git a/src/neochatconnection.h b/src/neochatconnection.h index af03f74e2..056766de0 100644 --- a/src/neochatconnection.h +++ b/src/neochatconnection.h @@ -9,6 +9,8 @@ #include #include +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 m_linkPreviewers; };