From d14d576d99b197ab8416ceb26e28589cd9bd1283 Mon Sep 17 00:00:00 2001 From: Tobias Fella Date: Fri, 22 Nov 2024 21:53:56 +0100 Subject: [PATCH] Implement MSC 4228 Search Redirection See https://github.com/matrix-org/matrix-spec-proposals/pull/4228 for details. Since this is tricky to test without server-side support, I have added a basic implementation to the mock server in appiumtests/login-server.py 1. Start appiumtests/login-server.py 2. Start neochat with "--test --ignore-ssl-errors" options 3. Open "Explore Rooms" 4. Search for the exact string "forbidden" 5. See new error message provided by server --- appiumtests/login-server.py | 9 +++++++++ src/models/publicroomlistmodel.cpp | 29 ++++++++++++++++++++++++++++- src/models/publicroomlistmodel.h | 9 +++++++++ src/qml/ExploreRoomsPage.qml | 3 +++ src/qml/SearchPage.qml | 24 +++++++++++++++++++++--- 5 files changed, 70 insertions(+), 4 deletions(-) diff --git a/appiumtests/login-server.py b/appiumtests/login-server.py index 4856ff8db..33a593d3f 100644 --- a/appiumtests/login-server.py +++ b/appiumtests/login-server.py @@ -83,6 +83,15 @@ def create_room(): next_sync_payload = "sync_response_new_room" return response +@app.route("/_matrix/client/v3/publicRooms", methods=["POST"]) +def public_rooms(): + if request.get_json()["filter"]["generic_search_term"] == "forbidden": + data = dict() + data["errcode"] = "M_FORBIDDEN" + data["error"] = "You are not allowed to search for this. Go to https://wikipedia.org for more information" + return data, 403 + return dict() + if __name__ == "__main__": diff --git a/src/models/publicroomlistmodel.cpp b/src/models/publicroomlistmodel.cpp index cab664705..3d9c78d37 100644 --- a/src/models/publicroomlistmodel.cpp +++ b/src/models/publicroomlistmodel.cpp @@ -8,6 +8,23 @@ using namespace Quotient; +class NeoChatQueryPublicRoomsJob : public QueryPublicRoomsJob +{ +public: + explicit NeoChatQueryPublicRoomsJob(const QString &server = {}, + std::optional limit = std::nullopt, + const QString &since = {}, + const std::optional &filter = std::nullopt, + std::optional includeAllNetworks = std::nullopt, + const QString &thirdPartyInstanceId = {}) + : QueryPublicRoomsJob(server, limit, since, filter, includeAllNetworks, thirdPartyInstanceId) + { + // TODO Remove once we can use libQuotient's job directly + // This is to make libQuotient happy about results not having the "chunk" field + setExpectedKeys({}); + } +}; + PublicRoomListModel::PublicRoomListModel(QObject *parent) : QAbstractListModel(parent) { @@ -153,6 +170,8 @@ void PublicRoomListModel::next(int limit) if (m_connection == nullptr || limit < 1) { return; } + m_redirectedText.clear(); + Q_EMIT redirectedChanged(); if (job) { qCDebug(PublicRoomList) << "Other job running, ignore"; @@ -163,7 +182,7 @@ void PublicRoomListModel::next(int limit) if (m_showOnlySpaces) { roomTypes += QLatin1String("m.space"); } - job = m_connection->callApi(m_server, limit, nextBatch, QueryPublicRoomsJob::Filter{m_searchText, roomTypes}); + job = m_connection->callApi(m_server, limit, nextBatch, QueryPublicRoomsJob::Filter{m_searchText, roomTypes}); Q_EMIT searchingChanged(); connect(job, &BaseJob::finished, this, [this] { @@ -181,6 +200,9 @@ void PublicRoomListModel::next(int limit) this->beginInsertRows({}, rooms.count(), rooms.count() + job->chunk().count() - 1); rooms.append(job->chunk()); this->endInsertRows(); + } else if (job->error() == BaseJob::ContentAccessError) { + m_redirectedText = job->jsonData()[u"error"_s].toString(); + Q_EMIT redirectedChanged(); } this->job = nullptr; @@ -302,4 +324,9 @@ bool PublicRoomListModel::searching() const return job != nullptr; } +QString PublicRoomListModel::redirectedText() const +{ + return m_redirectedText; +} + #include "moc_publicroomlistmodel.cpp" diff --git a/src/models/publicroomlistmodel.h b/src/models/publicroomlistmodel.h index 6f60b81cc..2c2cecdeb 100644 --- a/src/models/publicroomlistmodel.h +++ b/src/models/publicroomlistmodel.h @@ -52,6 +52,11 @@ class PublicRoomListModel : public QAbstractListModel */ Q_PROPERTY(bool searching READ searching NOTIFY searchingChanged) + /** + * @brief The text returned by the server after redirection + */ + Q_PROPERTY(QString redirectedText READ redirectedText NOTIFY redirectedChanged) + public: /** * @brief Defines the model roles. @@ -113,6 +118,8 @@ public: */ Q_INVOKABLE void search(int limit = 50); + QString redirectedText() const; + private: QPointer m_connection = nullptr; QString m_server; @@ -135,6 +142,7 @@ private: QList rooms; Quotient::QueryPublicRoomsJob *job = nullptr; + QString m_redirectedText; Q_SIGNALS: void connectionChanged(); @@ -142,4 +150,5 @@ Q_SIGNALS: void searchTextChanged(); void showOnlySpacesChanged(); void searchingChanged(); + void redirectedChanged(); }; diff --git a/src/qml/ExploreRoomsPage.qml b/src/qml/ExploreRoomsPage.qml index 9383d63d6..794270f79 100644 --- a/src/qml/ExploreRoomsPage.qml +++ b/src/qml/ExploreRoomsPage.qml @@ -50,6 +50,8 @@ SearchPage { signal roomSelected(string roomId, string displayName, url avatarUrl, string alias, string topic, int memberCount, bool isJoined) title: i18nc("@action:title", "Explore Rooms") + customPlaceholderText: publicRoomListModel.redirectedText + customPlaceholderIcon: "data-warning" Component.onCompleted: focusSearch() @@ -93,6 +95,7 @@ SearchPage { activeFocusOnTab: false // We handle moving to this item via up/down arrows, otherwise the tab order is wacky text: i18n("Enter a Room Manually") + visible: publicRoomListModel.redirectedText.length === 0 icon.name: "compass" icon.width: Kirigami.Units.gridUnit * 2 icon.height: Kirigami.Units.gridUnit * 2 diff --git a/src/qml/SearchPage.qml b/src/qml/SearchPage.qml index 3d9d29933..cc67f7520 100644 --- a/src/qml/SearchPage.qml +++ b/src/qml/SearchPage.qml @@ -80,6 +80,17 @@ Kirigami.ScrollablePage { */ property bool showSearchButton: true + /** + * @brief Message to be shown in a custom placeholder. + * The custom placeholder will be shown if the text is not empty + */ + property alias customPlaceholderText: customPlaceholder.text + + /** + * @brief icon for the custom placeholder + */ + property string customPlaceholderIcon: "" + /** * @brief Force the search field to be focussed. */ @@ -167,18 +178,25 @@ Kirigami.ScrollablePage { Kirigami.PlaceholderMessage { id: noSearchMessage anchors.centerIn: parent - visible: searchField.text.length === 0 && listView.count === 0 + visible: searchField.text.length === 0 && listView.count === 0 && !root.showCustomPlaceholder && customPlaceholder.text.length === 0 } Kirigami.PlaceholderMessage { id: noResultMessage anchors.centerIn: parent - visible: searchField.text.length > 0 && listView.count === 0 && !root.model.searching + visible: searchField.text.length > 0 && listView.count === 0 && !root.model.searching && customPlaceholder.text.length === 0 + } + + Kirigami.PlaceholderMessage { + id: customPlaceholder + anchors.centerIn: parent + visible: searchField.text.length > 0 && listView.count === 0 && !root.model.searching && text.length > 0 + icon.name: root.customPlaceholderIcon } Kirigami.LoadingPlaceholder { anchors.centerIn: parent - visible: searchField.text.length > 0 && listView.count === 0 && root.model.searching + visible: searchField.text.length > 0 && listView.count === 0 && root.model.searching && customPlaceholder.text.length === 0 } Keys.onUpPressed: {