From e7bf4f3e4fc059ef9ea0e0b253a1953a91fd77d8 Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Sun, 31 May 2020 13:24:05 +0200
Subject: [PATCH 01/10] ResourceResolver

Introducing the uniform way to resolve Matrix URIs and identifiers
to Room/User objects, passing an optional event id (if supplied) to
the client-defined handler. Just call ResourceResolver::visitResource()
or ResourceResolver::openResource() and you'll have that string parsed
and dispatched where you need.
---
 CMakeLists.txt           |   1 +
 lib/resourceresolver.cpp |  97 ++++++++++++++++++++++++++++++++
 lib/resourceresolver.h   | 117 +++++++++++++++++++++++++++++++++++++++
 libquotient.pri          |   2 +
 tests/quotest.cpp        |  88 +++++++++++++++++++++++++++++
 5 files changed, 305 insertions(+)
 create mode 100644 lib/resourceresolver.cpp
 create mode 100644 lib/resourceresolver.h

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5b6410f13..830751964 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -149,6 +149,7 @@ set(lib_SRCS
     lib/room.cpp
     lib/user.cpp
     lib/avatar.cpp
+    lib/resourceresolver.cpp
     lib/syncdata.cpp
     lib/settings.cpp
     lib/networksettings.cpp
diff --git a/lib/resourceresolver.cpp b/lib/resourceresolver.cpp
new file mode 100644
index 000000000..f910d6401
--- /dev/null
+++ b/lib/resourceresolver.cpp
@@ -0,0 +1,97 @@
+#include "resourceresolver.h"
+
+#include "settings.h"
+
+#include <QtCore/QRegularExpression>
+
+using namespace Quotient;
+
+QString ResourceResolver::toMatrixId(const QString& uriOrId,
+                                     QStringList uriServers)
+{
+    auto id = QUrl::fromPercentEncoding(uriOrId.toUtf8());
+    const auto MatrixScheme = "matrix:"_ls;
+    if (id.startsWith(MatrixScheme)) {
+        id.remove(0, MatrixScheme.size());
+        for (const auto& p: { std::pair { "user/"_ls, '@' },
+                              { "roomid/"_ls, '!' },
+                              { "room/"_ls, '#' } })
+            if (id.startsWith(p.first)) {
+                id.replace(0, p.first.size(), p.second);
+                break;
+            }
+        // The below assumes that /event/ cannot show up in normal Matrix ids.
+        id.replace("/event/"_ls, "/$"_ls);
+    } else {
+        const auto MatrixTo_ServerName = QStringLiteral("matrix.to");
+        if (!uriServers.contains(MatrixTo_ServerName))
+            uriServers.push_back(MatrixTo_ServerName);
+        id.remove(
+            QRegularExpression("^https://(" + uriServers.join('|') + ")/?#/"));
+    }
+    return id;
+}
+
+ResourceResolver::Result ResourceResolver::visitResource(
+    Connection* account, const QString& identifier,
+    std::function<void(User*)> userHandler,
+    std::function<void(Room*, QString)> roomEventHandler)
+{
+    const auto& normalizedId = toMatrixId(identifier);
+    auto&& [sigil, mainId, secondaryId] = parseIdentifier(normalizedId);
+            Room* room = nullptr;
+            switch (sigil) {
+        case char(-1):
+            return MalformedMatrixId;
+        case char(0):
+            return EmptyMatrixId;
+        case '@':
+            if (auto* user = account->user(mainId)) {
+                userHandler(user);
+                return Success;
+            }
+            return MalformedMatrixId;
+        case '!':
+            if ((room = account->room(mainId)))
+                break;
+            return UnknownMatrixId;
+        case '#':
+            if ((room = account->roomByAlias(mainId)))
+                break;
+            [[fallthrough]];
+        default:
+            return UnknownMatrixId;
+    }
+    roomEventHandler(room, secondaryId);
+    return Success;
+}
+
+ResourceResolver::IdentifierParts
+ResourceResolver::parseIdentifier(const QString& identifier)
+{
+    if (identifier.isEmpty())
+        return {};
+    
+    // The regex is quick and dirty, only intending to triage the id.
+    static const QRegularExpression IdRE {
+        "^(?<main>(?<sigil>.)([^/]+))(/(?<sec>[^?]+))?"
+    };
+    auto dissectedId = IdRE.match(identifier);
+    if (!dissectedId.hasMatch())
+        return { char(-1) };
+
+    const auto sigil = dissectedId.captured("sigil");
+    return { sigil.size() != 1 ? char(-1) : sigil.front().toLatin1(),
+                dissectedId.captured("main"), dissectedId.captured("sec") };
+}
+
+ResourceResolver::Result
+ResourceResolver::openResource(Connection* account, const QString& identifier,
+                               const QString& action)
+{
+    return visitResource(account, identifier,
+        [this, &action](User* u) { emit userAction(u, action); },
+        [this, &action](Room* room, const QString& eventId) {
+            emit roomAction(room, eventId, action);
+        });
+}
diff --git a/lib/resourceresolver.h b/lib/resourceresolver.h
new file mode 100644
index 000000000..794b77966
--- /dev/null
+++ b/lib/resourceresolver.h
@@ -0,0 +1,117 @@
+#pragma once
+
+#include "connection.h"
+
+#include <functional>
+
+namespace Quotient {
+
+/*! \brief Matrix resource resolver
+ * TODO: rewrite
+ * Similar to visitResource(), this class encapsulates the logic of resolving
+ * a Matrix identifier or a URI into Quotient object(s) and applying an action
+ * to the resolved object(s). Instead of using a C++ visitor pattern, it
+ * announces the request through Qt's signals passing the resolved object(s)
+ * through those (still in a typesafe way).
+ *
+ * This class is aimed primarily at clients where invoking the resolving/action
+ * and handling the action are happening in decoupled parts of the code; it's
+ * also useful to operate on Matrix identifiers and URIs from QML/JS code
+ * that cannot call visitResource due to QML/C++ interface limitations.
+ */
+class ResourceResolver : public QObject {
+    Q_OBJECT
+public:
+    enum Result : short {
+        StillResolving = -1,
+        Success = 0,
+        UnknownMatrixId,
+        MalformedMatrixId,
+        NoAccount,
+        EmptyMatrixId
+    };
+    Q_ENUM(Result)
+
+    explicit ResourceResolver(QObject* parent = nullptr) : QObject(parent)
+    { }
+
+    /*! \brief Decode a URI to a Matrix identifier (or a room/event pair)
+     *
+     * This accepts plain Matrix ids, MSC2312 URIs (aka matrix: URIs) and
+     * matrix.to URIs.
+     *
+     * \return a Matrix identifier as defined by the common identifier grammars
+     *         or a slash separated pair of Matrix identifiers if the original
+     *         uri/id pointed to an event in a room
+     */
+    static QString toMatrixId(const QString& uriOrId,
+                                 QStringList uriServers = {});
+
+    /*! \brief Resolve the resource and invoke an action on it, visitor style
+     *
+     * This template function encapsulates the logic of resolving a Matrix
+     * identifier or URI into a Quotient object (or objects) and applying an
+     * appropriate action handler from the set provided by the caller to it.
+     * A typical use case for that is opening a room or mentioning a user in
+     * response to clicking on a Matrix URI or identifier.
+     *
+     * \param account The connection used as a context to resolve the identifier
+     *
+     * \param identifier The Matrix identifier or URI. MSC2312 URIs and classic
+     *                   Matrix ID scheme are supported.
+     *
+     * \sa ResourceResolver
+     */
+    static Result
+    visitResource(Connection* account, const QString& identifier,
+                  std::function<void(User*)> userHandler,
+                  std::function<void(Room*, QString)> roomEventHandler);
+
+    /*! \brief Resolve the resource and request an action on it, signal style
+     *
+     * This method:
+     * 1. Resolves \p identifier into an actual object (Room or User), with
+     *    possible additional data such as event id, in the context of
+     *    \p account.
+     * 2. If the resolving is successful, depending on the type of the object,
+     *    emits the respective signal to which the client must connect in order
+     *    to apply the action to the resource (open a room, mention a user etc.).
+     * 3. Returns the result of resolving the resource.
+     *
+     * Note that the action can be applied either synchronously or entirely
+     * asynchronously; ResourceResolver does not restrain the client code
+     * to use either method. The resource resolving part is entirely synchronous
+     * though. If the synchronous operation is chosen, only
+     * direct connections to ResourceResolver signals must be used, and
+     * the caller should check the future's state immediately after calling
+     * openResource() to process any feedback from the resolver and/or action
+     * handler. If asynchronous operation is needed then either direct or queued
+     * connections to ResourceResolver's signals can be used and the caller
+     * must both check the ResourceFuture state right after calling openResource
+     * and also connect to ResourceFuture::ready() signal in order to process
+     * the result of resolving and action.
+     */
+    Q_INVOKABLE Result openResource(Connection* account,
+                                    const QString& identifier,
+                                    const QString& action = {});
+
+signals:
+    /// An action on a user has been requested
+    void userAction(Quotient::User* user, QString action);
+
+    /// An action on a room has been requested, with optional event id
+    void roomAction(Quotient::Room* room, QString eventId, QString action);
+
+private:
+    struct IdentifierParts {
+        char sigil;
+        QString mainId {};
+        QString secondaryId = {};
+    };
+
+    static IdentifierParts parseIdentifier(const QString& identifier);
+};
+
+} // namespace Quotient
+
+
diff --git a/libquotient.pri b/libquotient.pri
index a5a1459f1..f0057712d 100644
--- a/libquotient.pri
+++ b/libquotient.pri
@@ -34,6 +34,7 @@ HEADERS += \
     $$SRCPATH/room.h \
     $$SRCPATH/user.h \
     $$SRCPATH/avatar.h \
+    $$SRCPATH/resourceresolver.h \
     $$SRCPATH/syncdata.h \
     $$SRCPATH/quotient_common.h \
     $$SRCPATH/util.h \
@@ -90,6 +91,7 @@ SOURCES += \
     $$SRCPATH/room.cpp \
     $$SRCPATH/user.cpp \
     $$SRCPATH/avatar.cpp \
+    $$SRCPATH/resourceresolver.cpp \
     $$SRCPATH/syncdata.cpp \
     $$SRCPATH/util.cpp \
     $$SRCPATH/events/event.cpp \
diff --git a/tests/quotest.cpp b/tests/quotest.cpp
index b06665a91..68b8ebd66 100644
--- a/tests/quotest.cpp
+++ b/tests/quotest.cpp
@@ -2,6 +2,7 @@
 #include "connection.h"
 #include "room.h"
 #include "user.h"
+#include "resourceresolver.h"
 
 #include "csapi/joining.h"
 #include "csapi/leaving.h"
@@ -98,6 +99,7 @@ private slots:
     TEST_DECL(sendAndRedact)
     TEST_DECL(addAndRemoveTag)
     TEST_DECL(markDirectChat)
+    TEST_DECL(visitResources)
     // Add more tests above here
 
 public:
@@ -612,6 +614,92 @@ TEST_IMPL(markDirectChat)
                 && removedDCs.contains(connection()->user(), targetRoom->id()));
 }
 
+TEST_IMPL(visitResources)
+{
+    // Same as the two tests above, ResourceResolver emits signals
+    // synchronously so we use signal spies to intercept them instead of
+    // connecting lambdas before calling openResource(). NB: this test
+    // assumes that ResourceResolver::openResource is implemented in terms
+    // of ResourceResolver::visitResource, so the latter doesn't need a
+    // separate test.
+    static ResourceResolver rr;
+
+    // This lambda returns true in case of error, false if it's fine so far
+    auto testResourceResolver = [this, thisTest](const QStringList& uris,
+                                                 auto signal, auto* target,
+                                                 const QString& eventId = {}) {
+        int r = qRegisterMetaType<decltype(target)>();
+        Q_ASSERT(r != 0);
+        QSignalSpy spy(&rr, signal);
+        for (const auto& uri: uris) {
+            clog << "Resolving uri " << uri.toStdString() << endl;
+            rr.openResource(connection(), uri, "action");
+            if (spy.count() != 1) {
+                clog << "Wrong number of signal emissions (" << spy.count()
+                     << ')' << endl;
+                FAIL_TEST();
+            }
+            const auto& emission = spy.front();
+            Q_ASSERT(emission.count() >= 2);
+            if (emission.front().value<decltype(target)>() != target) {
+                clog << "Action on an incorrect target called" << endl;
+                FAIL_TEST();
+            }
+            if (emission.back() != "action") {
+                clog << "Action wasn't passed" << endl;
+                FAIL_TEST();
+            }
+            if (!eventId.isEmpty()) {
+                const auto passedEvtId = (emission.cend() - 2)->toString();
+                if (passedEvtId != eventId) {
+                    clog << "Event passed incorrectly (received "
+                         << passedEvtId.toStdString() << " instead of "
+                         << eventId.toStdString() << ')' << endl;
+                    FAIL_TEST();
+                }
+            }
+            spy.clear();
+        }
+        return false;
+    };
+
+    // Matrix identifiers used throughout all URI tests
+    const auto& roomId = room()->id();
+    const auto& roomAlias = room()->canonicalAlias();
+    const auto& userId = connection()->userId();
+    const auto& eventId = room()->messageEvents().back()->id();
+    Q_ASSERT(!roomId.isEmpty());
+    Q_ASSERT(!roomAlias.isEmpty());
+    Q_ASSERT(!userId.isEmpty());
+    Q_ASSERT(!eventId.isEmpty());
+
+    const QStringList roomUris {
+        roomId,
+        "matrix:roomid/" + roomId.mid(1),
+        "https://matrix.to/#/" + roomId,
+        roomAlias,
+        "matrix:room/" + roomAlias.mid(1),
+        "https://matrix.to/#/" + roomAlias,
+        "https://matrix.to#/" + roomAlias, // Just in case
+    };
+    const QStringList userUris { userId, "matrix:user/" + userId.mid(1),
+                                 "https://matrix.to/#/" + userId };
+    const QStringList eventUris {
+        "matrix:room/" + roomAlias.mid(1) + "/event/" + eventId.mid(1),
+        "https://matrix.to/#/" + roomId + '/' + eventId
+    };
+    // If any test breaks, the breaking call will return true, and further
+    // execution will be cut by ||'s short-circuiting
+    if (testResourceResolver(roomUris, &ResourceResolver::roomAction, room())
+        || testResourceResolver(userUris, &ResourceResolver::userAction,
+                                connection()->user())
+        || testResourceResolver(eventUris, &ResourceResolver::roomAction,
+                                room(), eventId))
+        return true;
+    // TODO: negative cases
+    FINISH_TEST(true);
+}
+
 void TestManager::conclude()
 {
     QString succeededRec { QString::number(succeeded.size()) % " of "

From 012f6ea6a4fa63d4e48e910f829e060c19cbcb1e Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Thu, 11 Jun 2020 22:48:26 +0200
Subject: [PATCH 02/10] Fix building with older Qt

---
 lib/resourceresolver.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/resourceresolver.cpp b/lib/resourceresolver.cpp
index f910d6401..0d5c5a20b 100644
--- a/lib/resourceresolver.cpp
+++ b/lib/resourceresolver.cpp
@@ -81,7 +81,7 @@ ResourceResolver::parseIdentifier(const QString& identifier)
         return { char(-1) };
 
     const auto sigil = dissectedId.captured("sigil");
-    return { sigil.size() != 1 ? char(-1) : sigil.front().toLatin1(),
+    return { sigil.size() != 1 ? char(-1) : sigil[0].toLatin1(),
                 dissectedId.captured("main"), dissectedId.captured("sec") };
 }
 

From 5f3e88b87748bb982858850803e8405ac88919d9 Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Fri, 12 Jun 2020 15:14:45 +0200
Subject: [PATCH 03/10] quotest: delay sync start till after joining

To ensure that the sync returns the details of the room just joined.
---
 tests/quotest.cpp | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/tests/quotest.cpp b/tests/quotest.cpp
index 68b8ebd66..7076264cb 100644
--- a/tests/quotest.cpp
+++ b/tests/quotest.cpp
@@ -217,22 +217,20 @@ void TestManager::setupAndRun()
     clog << "Access token: " << c->accessToken().toStdString() << endl;
 
     c->setLazyLoading(true);
-    c->syncLoop();
 
     clog << "Joining " << targetRoomName.toStdString() << endl;
     auto joinJob = c->joinRoom(targetRoomName);
-    // Ensure, before this test is completed, that the room has been joined
-    // and filled with some events so that other tests could use that
+    // Ensure that the room has been joined and filled with some events
+    // so that other tests could use that
     connect(joinJob, &BaseJob::success, this, [this, joinJob] {
         testSuite = new TestSuite(c->room(joinJob->roomId()), origin, this);
+        // Only start the sync after joining, to make sure the room just
+        // joined is in it
+        c->syncLoop();
         connectSingleShot(c, &Connection::syncDone, this, [this] {
-            if (testSuite->room()->timelineSize() > 0)
-                doTests();
-            else {
-                testSuite->room()->getPreviousContent();
-                connectSingleShot(testSuite->room(), &Room::addedMessages, this,
-                                  &TestManager::doTests);
-            }
+            testSuite->room()->getPreviousContent();
+            connectSingleShot(testSuite->room(), &Room::addedMessages, this,
+                              &TestManager::doTests);
         });
     });
     connect(joinJob, &BaseJob::failure, this, [this] {

From 99ae651d04fedb13eccd837f296c6a79790d37ca Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Fri, 12 Jun 2020 20:38:45 +0200
Subject: [PATCH 04/10] quotest: consolidate processing of syncDone()

---
 tests/quotest.cpp | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/tests/quotest.cpp b/tests/quotest.cpp
index 7076264cb..19aa5b859 100644
--- a/tests/quotest.cpp
+++ b/tests/quotest.cpp
@@ -227,10 +227,26 @@ void TestManager::setupAndRun()
         // Only start the sync after joining, to make sure the room just
         // joined is in it
         c->syncLoop();
-        connectSingleShot(c, &Connection::syncDone, this, [this] {
-            testSuite->room()->getPreviousContent();
-            connectSingleShot(testSuite->room(), &Room::addedMessages, this,
-                              &TestManager::doTests);
+        connect(c, &Connection::syncDone, this, [this] {
+            static int i = 0;
+            clog << "Sync " << ++i << " complete" << endl;
+            if (auto* r = testSuite->room()) {
+                clog << "Test room timeline size = " << r->timelineSize();
+                if (r->pendingEvents().empty())
+                    clog << ", pending size = " << r->pendingEvents().size();
+                clog << endl;
+            }
+            if (!running.empty()) {
+                clog << running.size() << " test(s) in the air:";
+                for (const auto& test: qAsConst(running))
+                    clog << " " << testName(test);
+                clog << endl;
+            }
+            if (i == 1) {
+                testSuite->room()->getPreviousContent();
+                connectSingleShot(testSuite->room(), &Room::addedMessages, this,
+                                  &TestManager::doTests);
+            }
         });
     });
     connect(joinJob, &BaseJob::failure, this, [this] {
@@ -262,8 +278,8 @@ void TestManager::doTests()
 
         const auto testName = metaMethod.name();
         running.push_back(testName);
-        // Some tests return the result immediately, so queue everything
-        // so that we could process all tests asynchronously.
+        // Some tests return the result immediately but we queue everything
+        // and process all tests asynchronously.
         QMetaObject::invokeMethod(testSuite, "doTest", Qt::QueuedConnection,
                                   Q_ARG(QByteArray, testName));
     }
@@ -283,20 +299,6 @@ void TestManager::doTests()
                     conclude();
                 }
             });
-
-    connect(c, &Connection::syncDone, this, [this] {
-        static int i = 0;
-        clog << "Sync " << ++i << " complete" << endl;
-        if (auto* r = testSuite->room())
-            clog << "Test room timeline size = " << r->timelineSize()
-                 << ", pending size = " << r->pendingEvents().size() << endl;
-        if (!running.empty()) {
-            clog << running.size() << " test(s) in the air:";
-            for (const auto& test: qAsConst(running))
-                clog << " " << testName(test);
-            clog << endl;
-        }
-    });
 }
 
 TEST_IMPL(loadMembers)

From 4dd51859c9ada4660e2a9cd16c3d0a99ad13761f Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Wed, 17 Jun 2020 22:13:53 +0200
Subject: [PATCH 05/10] class MatrixUri; support action=join and via=

---
 lib/quotient_common.h    |   8 +-
 lib/resourceresolver.cpp | 275 +++++++++++++++++++++++++++++----------
 lib/resourceresolver.h   | 172 ++++++++++++++++--------
 tests/quotest.cpp        |  65 +++++----
 4 files changed, 372 insertions(+), 148 deletions(-)

diff --git a/lib/quotient_common.h b/lib/quotient_common.h
index 44541b42f..446f628b3 100644
--- a/lib/quotient_common.h
+++ b/lib/quotient_common.h
@@ -14,15 +14,15 @@ enum RunningPolicy { ForegroundRequest = 0x0, BackgroundRequest = 0x1 };
 
 Q_ENUM_NS(RunningPolicy)
 
-enum ResourceResolveResult : short {
+enum UriResolveResult : short {
     StillResolving = -1,
-    Resolved = 0,
+    UriResolved = 0,
     UnknownMatrixId,
-    MalformedMatrixId,
+    MalformedUri,
     NoAccount,
     EmptyMatrixId
 };
-Q_ENUM_NS(ResourceResolveResult)
+Q_ENUM_NS(UriResolveResult)
 
 } // namespace Quotient
 /// \deprecated Use namespace Quotient instead
diff --git a/lib/resourceresolver.cpp b/lib/resourceresolver.cpp
index 0d5c5a20b..e78200616 100644
--- a/lib/resourceresolver.cpp
+++ b/lib/resourceresolver.cpp
@@ -1,97 +1,236 @@
 #include "resourceresolver.h"
 
-#include "settings.h"
+#include "connection.h"
+#include "logging.h"
 
 #include <QtCore/QRegularExpression>
 
 using namespace Quotient;
 
-QString ResourceResolver::toMatrixId(const QString& uriOrId,
-                                     QStringList uriServers)
+struct ReplacePair { QByteArray uriString; char sigil; };
+static const auto replacePairs = { ReplacePair { "user/", '@' },
+                                   { "roomid/", '!' },
+                                   { "room/", '#' } };
+
+MatrixUri::MatrixUri(QByteArray primaryId, QByteArray secondaryId, QString query)
 {
-    auto id = QUrl::fromPercentEncoding(uriOrId.toUtf8());
-    const auto MatrixScheme = "matrix:"_ls;
-    if (id.startsWith(MatrixScheme)) {
-        id.remove(0, MatrixScheme.size());
-        for (const auto& p: { std::pair { "user/"_ls, '@' },
-                              { "roomid/"_ls, '!' },
-                              { "room/"_ls, '#' } })
-            if (id.startsWith(p.first)) {
-                id.replace(0, p.first.size(), p.second);
+    if (primaryId.isEmpty())
+        primaryType_ = Empty;
+    else {
+        setScheme("matrix");
+        QString pathToBe;
+        primaryType_ = Invalid;
+        for (const auto& p: replacePairs)
+            if (primaryId[0] == p.sigil) {
+                primaryType_ = Type(p.sigil);
+                pathToBe = p.uriString + primaryId.mid(1);
                 break;
             }
-        // The below assumes that /event/ cannot show up in normal Matrix ids.
-        id.replace("/event/"_ls, "/$"_ls);
-    } else {
-        const auto MatrixTo_ServerName = QStringLiteral("matrix.to");
-        if (!uriServers.contains(MatrixTo_ServerName))
-            uriServers.push_back(MatrixTo_ServerName);
-        id.remove(
-            QRegularExpression("^https://(" + uriServers.join('|') + ")/?#/"));
+        if (!secondaryId.isEmpty())
+            pathToBe += "/event/" + secondaryId.mid(1);
+        setPath(pathToBe);
     }
-    return id;
+    setQuery(std::move(query));
 }
 
-ResourceResolver::Result ResourceResolver::visitResource(
-    Connection* account, const QString& identifier,
-    std::function<void(User*)> userHandler,
-    std::function<void(Room*, QString)> roomEventHandler)
+MatrixUri::MatrixUri(QUrl url) : QUrl(std::move(url))
 {
-    const auto& normalizedId = toMatrixId(identifier);
-    auto&& [sigil, mainId, secondaryId] = parseIdentifier(normalizedId);
-            Room* room = nullptr;
-            switch (sigil) {
-        case char(-1):
-            return MalformedMatrixId;
-        case char(0):
-            return EmptyMatrixId;
-        case '@':
-            if (auto* user = account->user(mainId)) {
-                userHandler(user);
-                return Success;
-            }
-            return MalformedMatrixId;
-        case '!':
-            if ((room = account->room(mainId)))
-                break;
-            return UnknownMatrixId;
-        case '#':
-            if ((room = account->roomByAlias(mainId)))
+    // NB: url is moved from and empty by now
+    if (isEmpty())
+        return; // primaryType_ == None
+
+    primaryType_ = Invalid;
+    if (!QUrl::isValid()) // MatrixUri::isValid() checks primaryType_
+        return;
+
+    if (scheme() == "matrix") {
+        // Check sanity as per https://github.com/matrix-org/matrix-doc/pull/2312
+        const auto& urlPath = path();
+        const auto& splitPath = urlPath.splitRef('/');
+        switch (splitPath.size()) {
+        case 2:
+            break;
+        case 4:
+            if (splitPath[2] == "event")
                 break;
             [[fallthrough]];
         default:
-            return UnknownMatrixId;
+            return; // Invalid
+        }
+
+        for (const auto& p: replacePairs)
+            if (urlPath.startsWith(p.uriString)) {
+                primaryType_ = Type(p.sigil);
+                return; // The only valid return path for matrix: URIs
+            }
+        qCWarning(MAIN) << "Invalid matrix: URI passed to MatrixUri";
+    }
+    if (scheme() == "https" && authority() == "matrix.to") {
+        // See https://matrix.org/docs/spec/appendices#matrix-to-navigation
+        static const QRegularExpression MatrixToUrlRE {
+            R"(^/(?<main>[^/?]+)(/(?<sec>[^?]+))?(\?(?<query>.+))?$)"
+        };
+        // matrix.to accepts both literal sigils (as well as & and ? used in
+        // its "query" substitute) and their %-encoded forms;
+        // so force QUrl to decode everything.
+        auto f = fragment(QUrl::FullyDecoded);
+        if (auto&& m = MatrixToUrlRE.match(f); m.hasMatch())
+            *this = MatrixUri { m.captured("main").toUtf8(),
+                                m.captured("sec").toUtf8(),
+                                m.captured("query") };
     }
-    roomEventHandler(room, secondaryId);
-    return Success;
 }
 
-ResourceResolver::IdentifierParts
-ResourceResolver::parseIdentifier(const QString& identifier)
+MatrixUri::MatrixUri(const QString &uriOrId)
+    : MatrixUri(fromUserInput(uriOrId))
+{ }
+
+MatrixUri MatrixUri::fromUserInput(const QString& uriOrId)
 {
-    if (identifier.isEmpty())
+    if (uriOrId.isEmpty())
+        return {}; // type() == None
+
+    // A quick check if uriOrId is a plain Matrix id
+    if (QStringLiteral("!@#+").contains(uriOrId[0]))
+        return MatrixUri { uriOrId.toUtf8() };
+
+    // Bare event ids cannot be resolved without a room scope but are treated as
+    // valid anyway; in the future we might expose them as, say,
+    // matrix:event/eventid
+    if (uriOrId[0] == '$')
+        return MatrixUri { "", uriOrId.toUtf8() };
+
+    return MatrixUri { QUrl::fromUserInput(uriOrId) };
+}
+
+MatrixUri::Type MatrixUri::type() const { return primaryType_; }
+
+MatrixUri::SecondaryType MatrixUri::secondaryType() const
+{
+    return path().section('/', 2, 2) == "event" ? EventId : NoSecondaryId;
+}
+
+QUrl MatrixUri::toUrl(UriForm form) const
+{
+    if (!isValid())
         return {};
-    
-    // The regex is quick and dirty, only intending to triage the id.
-    static const QRegularExpression IdRE {
-        "^(?<main>(?<sigil>.)([^/]+))(/(?<sec>[^?]+))?"
-    };
-    auto dissectedId = IdRE.match(identifier);
-    if (!dissectedId.hasMatch())
-        return { char(-1) };
-
-    const auto sigil = dissectedId.captured("sigil");
-    return { sigil.size() != 1 ? char(-1) : sigil[0].toLatin1(),
-                dissectedId.captured("main"), dissectedId.captured("sec") };
+
+    if (form == CanonicalUri)
+        return *this;
+
+    QUrl url;
+    url.setScheme("https");
+    url.setHost("matrix.to");
+    url.setPath("/");
+    auto fragment = primaryId();
+    if (const auto& secId = secondaryId(); !secId.isEmpty())
+        fragment += '/' + secId;
+    if (const auto& q = query(); !q.isEmpty())
+        fragment += '?' + q;
+    url.setFragment(fragment);
+    return url;
+}
+
+QString MatrixUri::toDisplayString(MatrixUri::UriForm form) const
+{
+    return toUrl(form).toDisplayString();
 }
 
-ResourceResolver::Result
+QString MatrixUri::primaryId() const
+{
+    if (primaryType_ == Empty || primaryType_ == Invalid)
+        return {};
+
+    const auto& idStem = path().section('/', 1, 1);
+    return idStem.isEmpty() ? idStem : primaryType_ + idStem;
+}
+
+QString MatrixUri::secondaryId() const
+{
+    const auto& idStem = path().section('/', 3);
+    return idStem.isEmpty() ? idStem : secondaryType() + idStem;
+}
+
+QString MatrixUri::action() const
+{
+    return QUrlQuery { query() }.queryItemValue("action");
+}
+
+QStringList MatrixUri::viaServers() const
+{
+    return QUrlQuery { query() }.allQueryItemValues(QStringLiteral("via"),
+                                                    QUrl::EncodeReserved);
+}
+
+bool MatrixUri::isValid() const
+{
+    return primaryType_ != Empty && primaryType_ != Invalid;
+}
+
+UriResolveResult Quotient::visitResource(
+    Connection* account, const MatrixUri& uri,
+    std::function<void(User*)> userHandler,
+    std::function<void(Room*, QString)> roomEventHandler,
+    std::function<void(Connection*, QString, QStringList)> joinHandler)
+{
+    Q_ASSERT_X(account != nullptr, __FUNCTION__,
+               "The Connection argument passed to visit/openResource must not "
+               "be nullptr");
+    if (uri.action() == "join") {
+        if (uri.type() != MatrixUri::RoomAlias
+            && uri.type() != MatrixUri::RoomId)
+            return MalformedUri;
+
+        joinHandler(account, uri.primaryId(), uri.viaServers());
+        return UriResolved;
+    }
+
+    Room* room = nullptr;
+    switch (uri.type()) {
+    case MatrixUri::Invalid:
+        return MalformedUri;
+    case MatrixUri::Empty:
+        return EmptyMatrixId;
+    case MatrixUri::UserId:
+        if (auto* user = account->user(uri.primaryId())) {
+            userHandler(user);
+            return UriResolved;
+        }
+        return MalformedUri;
+    case MatrixUri::RoomId:
+        if ((room = account->room(uri.primaryId())))
+            break;
+        return UnknownMatrixId;
+    case MatrixUri::RoomAlias:
+        if ((room = account->roomByAlias(uri.primaryId())))
+            break;
+        [[fallthrough]];
+    default:
+        return UnknownMatrixId;
+    }
+    roomEventHandler(room, uri.secondaryId());
+    return UriResolved;
+}
+
+UriResolveResult
 ResourceResolver::openResource(Connection* account, const QString& identifier,
                                const QString& action)
 {
-    return visitResource(account, identifier,
-        [this, &action](User* u) { emit userAction(u, action); },
-        [this, &action](Room* room, const QString& eventId) {
-            emit roomAction(room, eventId, action);
+    return openResource(account, MatrixUri(identifier), action);
+}
+
+UriResolveResult ResourceResolver::openResource(Connection* account,
+                                                const MatrixUri& uri,
+                                                const QString& overrideAction)
+{
+    return visitResource(
+        account, uri,
+        [this, &overrideAction](User* u) { emit userAction(u, overrideAction); },
+        [this, &overrideAction](Room* room, const QString& eventId) {
+            emit roomAction(room, eventId, overrideAction);
+        },
+        [this](Connection* account, const QString& roomAliasOrId,
+               const QStringList& viaServers) {
+            emit joinAction(account, roomAliasOrId, viaServers);
         });
 }
diff --git a/lib/resourceresolver.h b/lib/resourceresolver.h
index 794b77966..fea07e97b 100644
--- a/lib/resourceresolver.h
+++ b/lib/resourceresolver.h
@@ -1,10 +1,118 @@
 #pragma once
 
-#include "connection.h"
+#include "quotient_common.h"
+
+#include <QtCore/QUrl>
+#include <QtCore/QUrlQuery>
+#include <QtCore/QObject>
 
 #include <functional>
 
 namespace Quotient {
+class Connection;
+class Room;
+class User;
+
+/*! \brief A wrapper around a Matrix URI or identifier
+ *
+ * This class encapsulates a Matrix resource identifier, passed in either of
+ * 3 forms: a plain Matrix identifier (sigil, localpart, serverpart or, for
+ * modern event ids, sigil and base64 hash); an MSC2312 URI (aka matrix: URI);
+ * or a matrix.to URL. The input can be either encoded (serverparts with
+ * punycode, the rest with percent-encoding) or unencoded (in this case it is
+ * the caller's responsibility to resolve all possible ambiguities).
+ *
+ * The class provides functions to check the validity of the identifier,
+ * its type, and obtain components, also in either unencoded (for displaying)
+ * or encoded (for APIs) form.
+ */
+class MatrixUri : private QUrl {
+    Q_GADGET
+    Q_PROPERTY(QString primaryId READ primaryId CONSTANT)
+    Q_PROPERTY(QString secondaryId READ secondaryId CONSTANT)
+//    Q_PROPERTY(QUrlQuery query READ query CONSTANT)
+    Q_PROPERTY(QString action READ action CONSTANT)
+//    Q_PROPERTY(QStringList viaServers READ viaServers CONSTANT)
+public:
+    enum Type : char {
+        Invalid = char(-1),
+        Empty = 0x0,
+        UserId = '@',
+        RoomId = '!',
+        RoomAlias = '#',
+        Group = '+'
+    };
+    Q_ENUM(Type)
+    enum SecondaryType : char {
+        NoSecondaryId = 0x0,
+        EventId = '$'
+    };
+
+    enum UriForm : short { CanonicalUri, MatrixToUri };
+    Q_ENUM(UriForm)
+
+    /// Construct an empty Matrix URI
+    MatrixUri() = default;
+    /*! \brief Decode a user input string to a Matrix identifier
+     *
+     * Accepts plain Matrix ids, MSC2312 URIs (aka matrix: URIs) and
+     * matrix.to URLs. In case of URIs/URLs, it uses QUrl's TolerantMode
+     * parser to decode common mistakes/irregularities (see QUrl documentation
+     * for more details).
+     */
+    MatrixUri(const QString& uriOrId);
+
+    /// Construct a Matrix URI from components
+    explicit MatrixUri(QByteArray primaryId, QByteArray secondaryId = {},
+                       QString query = {});
+    /// Construct a Matrix URI from matrix.to or MSC2312 (matrix:) URI
+    explicit MatrixUri(QUrl url);
+
+    static MatrixUri fromUserInput(const QString& uriOrId);
+    static MatrixUri fromUrl(QUrl url);
+
+    /// Get the primary type of the Matrix URI (user id, room id or alias)
+    /*! Note that this does not include an event as a separate type, since
+     * events can only be addressed inside of rooms, which, in turn, are
+     * addressed either by id or alias. If you need to check whether the URI
+     * is specifically an event URI, use secondaryType() instead.
+     */
+    Type type() const;
+    SecondaryType secondaryType() const;
+    QUrl toUrl(UriForm form = CanonicalUri) const;
+    QString toDisplayString(UriForm form = CanonicalUri) const;
+    QString primaryId() const;
+    QString secondaryId() const;
+    QString action() const;
+    QStringList viaServers() const;
+    bool isValid() const;
+    using QUrl::isEmpty, QUrl::path, QUrl::query, QUrl::fragment;
+
+private:
+
+    Type primaryType_ = Empty;
+};
+
+/*! \brief Resolve the resource and invoke an action on it, visitor style
+ *
+ * This template function encapsulates the logic of resolving a Matrix
+ * identifier or URI into a Quotient object (or objects) and applying an
+ * appropriate action handler from the set provided by the caller to it.
+ * A typical use case for that is opening a room or mentioning a user in
+ * response to clicking on a Matrix URI or identifier.
+ *
+ * \param account The connection used as a context to resolve the identifier
+ *
+ * \param uri The Matrix identifier or URI; MSC2312 URIs and classic Matrix IDs
+ *            are supported
+ *
+ * \sa ResourceResolver
+ */
+UriResolveResult
+visitResource(Connection* account, const MatrixUri& uri,
+              std::function<void(User*)> userHandler,
+              std::function<void(Room*, QString)> roomEventHandler,
+              std::function<void(Connection*, QString, QStringList)> joinHandler);
 
 /*! \brief Matrix resource resolver
  * TODO: rewrite
@@ -22,51 +130,9 @@ namespace Quotient {
 class ResourceResolver : public QObject {
     Q_OBJECT
 public:
-    enum Result : short {
-        StillResolving = -1,
-        Success = 0,
-        UnknownMatrixId,
-        MalformedMatrixId,
-        NoAccount,
-        EmptyMatrixId
-    };
-    Q_ENUM(Result)
-
     explicit ResourceResolver(QObject* parent = nullptr) : QObject(parent)
     { }
 
-    /*! \brief Decode a URI to a Matrix identifier (or a room/event pair)
-     *
-     * This accepts plain Matrix ids, MSC2312 URIs (aka matrix: URIs) and
-     * matrix.to URIs.
-     *
-     * \return a Matrix identifier as defined by the common identifier grammars
-     *         or a slash separated pair of Matrix identifiers if the original
-     *         uri/id pointed to an event in a room
-     */
-    static QString toMatrixId(const QString& uriOrId,
-                                 QStringList uriServers = {});
-
-    /*! \brief Resolve the resource and invoke an action on it, visitor style
-     *
-     * This template function encapsulates the logic of resolving a Matrix
-     * identifier or URI into a Quotient object (or objects) and applying an
-     * appropriate action handler from the set provided by the caller to it.
-     * A typical use case for that is opening a room or mentioning a user in
-     * response to clicking on a Matrix URI or identifier.
-     *
-     * \param account The connection used as a context to resolve the identifier
-     *
-     * \param identifier The Matrix identifier or URI. MSC2312 URIs and classic
-     *                   Matrix ID scheme are supported.
-     *
-     * \sa ResourceResolver
-     */
-    static Result
-    visitResource(Connection* account, const QString& identifier,
-                  std::function<void(User*)> userHandler,
-                  std::function<void(Room*, QString)> roomEventHandler);
-
     /*! \brief Resolve the resource and request an action on it, signal style
      *
      * This method:
@@ -91,9 +157,12 @@ class ResourceResolver : public QObject {
      * and also connect to ResourceFuture::ready() signal in order to process
      * the result of resolving and action.
      */
-    Q_INVOKABLE Result openResource(Connection* account,
-                                    const QString& identifier,
-                                    const QString& action = {});
+    Q_INVOKABLE UriResolveResult openResource(Connection* account,
+                                                   const QString& identifier,
+                                                   const QString& action = {});
+    Q_INVOKABLE UriResolveResult
+    openResource(Connection* account, const MatrixUri& uri,
+                 const QString& overrideAction = {});
 
 signals:
     /// An action on a user has been requested
@@ -102,14 +171,9 @@ class ResourceResolver : public QObject {
     /// An action on a room has been requested, with optional event id
     void roomAction(Quotient::Room* room, QString eventId, QString action);
 
-private:
-    struct IdentifierParts {
-        char sigil;
-        QString mainId {};
-        QString secondaryId = {};
-    };
-
-    static IdentifierParts parseIdentifier(const QString& identifier);
+    /// A join action has been requested, with optional 'via' servers
+    void joinAction(Quotient::Connection* account, QString roomAliasOrId,
+                    QStringList viaServers);
 };
 
 } // namespace Quotient
diff --git a/tests/quotest.cpp b/tests/quotest.cpp
index 19aa5b859..f9e25284b 100644
--- a/tests/quotest.cpp
+++ b/tests/quotest.cpp
@@ -627,13 +627,15 @@ TEST_IMPL(visitResources)
     // This lambda returns true in case of error, false if it's fine so far
     auto testResourceResolver = [this, thisTest](const QStringList& uris,
                                                  auto signal, auto* target,
-                                                 const QString& eventId = {}) {
+                                                 QVariantList otherArgs = {}) {
         int r = qRegisterMetaType<decltype(target)>();
         Q_ASSERT(r != 0);
         QSignalSpy spy(&rr, signal);
-        for (const auto& uri: uris) {
-            clog << "Resolving uri " << uri.toStdString() << endl;
-            rr.openResource(connection(), uri, "action");
+        for (const auto& uriString: uris) {
+            MatrixUri uri { uriString };
+            clog << "Checking " << uriString.toStdString()
+                 << " -> " << uri.toDisplayString().toStdString() << endl;
+            rr.openResource(connection(), uri);
             if (spy.count() != 1) {
                 clog << "Wrong number of signal emissions (" << spy.count()
                      << ')' << endl;
@@ -642,21 +644,19 @@ TEST_IMPL(visitResources)
             const auto& emission = spy.front();
             Q_ASSERT(emission.count() >= 2);
             if (emission.front().value<decltype(target)>() != target) {
-                clog << "Action on an incorrect target called" << endl;
+                clog << "Signal emitted with an incorrect target" << endl;
                 FAIL_TEST();
             }
-            if (emission.back() != "action") {
-                clog << "Action wasn't passed" << endl;
-                FAIL_TEST();
-            }
-            if (!eventId.isEmpty()) {
-                const auto passedEvtId = (emission.cend() - 2)->toString();
-                if (passedEvtId != eventId) {
-                    clog << "Event passed incorrectly (received "
-                         << passedEvtId.toStdString() << " instead of "
-                         << eventId.toStdString() << ')' << endl;
+            if (!otherArgs.empty()) {
+                if (emission.size() < otherArgs.size() + 1) {
+                    clog << "Emission doesn't include all arguments" << endl;
                     FAIL_TEST();
                 }
+                for (auto i = 0; i < otherArgs.size(); ++i)
+                    if (otherArgs[i] != emission[i + 1]) {
+                        clog << "Mismatch in argument #" << i + 1 << endl;
+                        FAIL_TEST();
+                    }
             }
             spy.clear();
         }
@@ -674,13 +674,10 @@ TEST_IMPL(visitResources)
     Q_ASSERT(!eventId.isEmpty());
 
     const QStringList roomUris {
-        roomId,
-        "matrix:roomid/" + roomId.mid(1),
-        "https://matrix.to/#/" + roomId,
-        roomAlias,
-        "matrix:room/" + roomAlias.mid(1),
+        roomId, "matrix:roomid/" + roomId.mid(1),
+        "https://matrix.to/#/%21" + roomId.mid(1),
+        roomAlias, "matrix:room/" + roomAlias.mid(1),
         "https://matrix.to/#/" + roomAlias,
-        "https://matrix.to#/" + roomAlias, // Just in case
     };
     const QStringList userUris { userId, "matrix:user/" + userId.mid(1),
                                  "https://matrix.to/#/" + userId };
@@ -688,13 +685,37 @@ TEST_IMPL(visitResources)
         "matrix:room/" + roomAlias.mid(1) + "/event/" + eventId.mid(1),
         "https://matrix.to/#/" + roomId + '/' + eventId
     };
+    // The following URIs are not supposed to be actually joined (and even
+    // exist, as yet) - only to be syntactically correct
+    static const auto& joinRoomAlias =
+        QStringLiteral("unjoined:example.org"); // # will be added below
+    QString joinQuery { "?action=join" };
+    static const QStringList joinByAliasUris {
+        "matrix:room/" + joinRoomAlias + joinQuery,
+        "https://matrix.to/#/%23"/*`#`*/ + joinRoomAlias + joinQuery
+    };
+    static const auto& joinRoomId = QStringLiteral("!anyid:example.org");
+    static const QStringList viaServers { "matrix.org", "example.org" };
+    static const auto viaQuery =
+        std::accumulate(viaServers.cbegin(), viaServers.cend(), joinQuery,
+                        [](QString q, const QString& s) {
+                            return q + "&via=" + s;
+                        });
+    static const QStringList joinByIdUris {
+        "matrix:roomid/" + joinRoomId.mid(1) + viaQuery,
+        "https://matrix.to/#/" + joinRoomId + viaQuery
+    };
     // If any test breaks, the breaking call will return true, and further
     // execution will be cut by ||'s short-circuiting
     if (testResourceResolver(roomUris, &ResourceResolver::roomAction, room())
         || testResourceResolver(userUris, &ResourceResolver::userAction,
                                 connection()->user())
         || testResourceResolver(eventUris, &ResourceResolver::roomAction,
-                                room(), eventId))
+                                room(), { eventId })
+        || testResourceResolver(joinByAliasUris, &ResourceResolver::joinAction,
+                                connection(), { '#' + joinRoomAlias })
+        || testResourceResolver(joinByIdUris, &ResourceResolver::joinAction,
+                                connection(), { joinRoomId, viaServers }))
         return true;
     // TODO: negative cases
     FINISH_TEST(true);

From 227d7c0ba26c3eb3e7394e0a5b7cc79544db7515 Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Fri, 26 Jun 2020 18:57:52 +0200
Subject: [PATCH 06/10] quotest: unit tests for MatrixUri

---
 tests/quotest.cpp | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/tests/quotest.cpp b/tests/quotest.cpp
index f9e25284b..529154547 100644
--- a/tests/quotest.cpp
+++ b/tests/quotest.cpp
@@ -635,7 +635,7 @@ TEST_IMPL(visitResources)
             MatrixUri uri { uriString };
             clog << "Checking " << uriString.toStdString()
                  << " -> " << uri.toDisplayString().toStdString() << endl;
-            rr.openResource(connection(), uri);
+            rr.openResource(connection(), uriString);
             if (spy.count() != 1) {
                 clog << "Wrong number of signal emissions (" << spy.count()
                      << ')' << endl;
@@ -663,6 +663,27 @@ TEST_IMPL(visitResources)
         return false;
     };
 
+    // Basic tests
+    QUrl invalidUrl { "https://" };
+    invalidUrl.setAuthority("---:@@@");
+    const MatrixUri emptyUri {}, uriFromEmptyUrl {},
+        invalidMatrixUri { QStringLiteral("matrix:&invalid@") },
+        matrixUriFromInvalidUrl { invalidUrl };
+
+    for (const auto& u: { emptyUri, uriFromEmptyUrl })
+        if (u.isValid() || !u.isEmpty()) {
+            clog << "Empty Matrix URI test failed" << endl;
+            FAIL_TEST();
+        }
+    if (matrixUriFromInvalidUrl.isEmpty() || matrixUriFromInvalidUrl.isValid()) {
+        clog << "Invalid Matrix URI test failed" << endl;
+        FAIL_TEST();
+    }
+    if (invalidMatrixUri.isEmpty() || invalidMatrixUri.isValid()) {
+        clog << "Invalid sigil in a Matrix URI - test failed" << endl;
+        FAIL_TEST();
+    }
+
     // Matrix identifiers used throughout all URI tests
     const auto& roomId = room()->id();
     const auto& roomAlias = room()->canonicalAlias();
@@ -675,7 +696,7 @@ TEST_IMPL(visitResources)
 
     const QStringList roomUris {
         roomId, "matrix:roomid/" + roomId.mid(1),
-        "https://matrix.to/#/%21" + roomId.mid(1),
+        "https://matrix.to/#/%21"/*`!`*/ + roomId.mid(1),
         roomAlias, "matrix:room/" + roomAlias.mid(1),
         "https://matrix.to/#/" + roomAlias,
     };

From af329351289606f3cb1ef865cb0cbe61c1d1711b Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Sun, 19 Jul 2020 16:12:09 +0200
Subject: [PATCH 07/10] MatrixUri->Uri: Extend to non-Matrix URIs

---
 CMakeLists.txt           |   3 +-
 lib/quotient_common.h    |   8 +-
 lib/resourceresolver.cpp | 236 ---------------------------------------
 lib/resourceresolver.h   | 181 ------------------------------
 lib/uri.cpp              | 181 ++++++++++++++++++++++++++++++
 lib/uri.h                |  85 ++++++++++++++
 lib/uriresolver.cpp      | 110 ++++++++++++++++++
 lib/uriresolver.h        | 162 +++++++++++++++++++++++++++
 libquotient.pri          |   6 +-
 tests/quotest.cpp        |  24 ++--
 10 files changed, 560 insertions(+), 436 deletions(-)
 delete mode 100644 lib/resourceresolver.cpp
 delete mode 100644 lib/resourceresolver.h
 create mode 100644 lib/uri.cpp
 create mode 100644 lib/uri.h
 create mode 100644 lib/uriresolver.cpp
 create mode 100644 lib/uriresolver.h

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 830751964..808899ba5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -149,7 +149,8 @@ set(lib_SRCS
     lib/room.cpp
     lib/user.cpp
     lib/avatar.cpp
-    lib/resourceresolver.cpp
+    lib/uri.cpp
+    lib/uriresolver.cpp
     lib/syncdata.cpp
     lib/settings.cpp
     lib/networksettings.cpp
diff --git a/lib/quotient_common.h b/lib/quotient_common.h
index 446f628b3..bb05af052 100644
--- a/lib/quotient_common.h
+++ b/lib/quotient_common.h
@@ -17,10 +17,10 @@ Q_ENUM_NS(RunningPolicy)
 enum UriResolveResult : short {
     StillResolving = -1,
     UriResolved = 0,
-    UnknownMatrixId,
-    MalformedUri,
-    NoAccount,
-    EmptyMatrixId
+    CouldNotResolve,
+    IncorrectAction,
+    InvalidUri,
+    NoAccount
 };
 Q_ENUM_NS(UriResolveResult)
 
diff --git a/lib/resourceresolver.cpp b/lib/resourceresolver.cpp
deleted file mode 100644
index e78200616..000000000
--- a/lib/resourceresolver.cpp
+++ /dev/null
@@ -1,236 +0,0 @@
-#include "resourceresolver.h"
-
-#include "connection.h"
-#include "logging.h"
-
-#include <QtCore/QRegularExpression>
-
-using namespace Quotient;
-
-struct ReplacePair { QByteArray uriString; char sigil; };
-static const auto replacePairs = { ReplacePair { "user/", '@' },
-                                   { "roomid/", '!' },
-                                   { "room/", '#' } };
-
-MatrixUri::MatrixUri(QByteArray primaryId, QByteArray secondaryId, QString query)
-{
-    if (primaryId.isEmpty())
-        primaryType_ = Empty;
-    else {
-        setScheme("matrix");
-        QString pathToBe;
-        primaryType_ = Invalid;
-        for (const auto& p: replacePairs)
-            if (primaryId[0] == p.sigil) {
-                primaryType_ = Type(p.sigil);
-                pathToBe = p.uriString + primaryId.mid(1);
-                break;
-            }
-        if (!secondaryId.isEmpty())
-            pathToBe += "/event/" + secondaryId.mid(1);
-        setPath(pathToBe);
-    }
-    setQuery(std::move(query));
-}
-
-MatrixUri::MatrixUri(QUrl url) : QUrl(std::move(url))
-{
-    // NB: url is moved from and empty by now
-    if (isEmpty())
-        return; // primaryType_ == None
-
-    primaryType_ = Invalid;
-    if (!QUrl::isValid()) // MatrixUri::isValid() checks primaryType_
-        return;
-
-    if (scheme() == "matrix") {
-        // Check sanity as per https://github.com/matrix-org/matrix-doc/pull/2312
-        const auto& urlPath = path();
-        const auto& splitPath = urlPath.splitRef('/');
-        switch (splitPath.size()) {
-        case 2:
-            break;
-        case 4:
-            if (splitPath[2] == "event")
-                break;
-            [[fallthrough]];
-        default:
-            return; // Invalid
-        }
-
-        for (const auto& p: replacePairs)
-            if (urlPath.startsWith(p.uriString)) {
-                primaryType_ = Type(p.sigil);
-                return; // The only valid return path for matrix: URIs
-            }
-        qCWarning(MAIN) << "Invalid matrix: URI passed to MatrixUri";
-    }
-    if (scheme() == "https" && authority() == "matrix.to") {
-        // See https://matrix.org/docs/spec/appendices#matrix-to-navigation
-        static const QRegularExpression MatrixToUrlRE {
-            R"(^/(?<main>[^/?]+)(/(?<sec>[^?]+))?(\?(?<query>.+))?$)"
-        };
-        // matrix.to accepts both literal sigils (as well as & and ? used in
-        // its "query" substitute) and their %-encoded forms;
-        // so force QUrl to decode everything.
-        auto f = fragment(QUrl::FullyDecoded);
-        if (auto&& m = MatrixToUrlRE.match(f); m.hasMatch())
-            *this = MatrixUri { m.captured("main").toUtf8(),
-                                m.captured("sec").toUtf8(),
-                                m.captured("query") };
-    }
-}
-
-MatrixUri::MatrixUri(const QString &uriOrId)
-    : MatrixUri(fromUserInput(uriOrId))
-{ }
-
-MatrixUri MatrixUri::fromUserInput(const QString& uriOrId)
-{
-    if (uriOrId.isEmpty())
-        return {}; // type() == None
-
-    // A quick check if uriOrId is a plain Matrix id
-    if (QStringLiteral("!@#+").contains(uriOrId[0]))
-        return MatrixUri { uriOrId.toUtf8() };
-
-    // Bare event ids cannot be resolved without a room scope but are treated as
-    // valid anyway; in the future we might expose them as, say,
-    // matrix:event/eventid
-    if (uriOrId[0] == '$')
-        return MatrixUri { "", uriOrId.toUtf8() };
-
-    return MatrixUri { QUrl::fromUserInput(uriOrId) };
-}
-
-MatrixUri::Type MatrixUri::type() const { return primaryType_; }
-
-MatrixUri::SecondaryType MatrixUri::secondaryType() const
-{
-    return path().section('/', 2, 2) == "event" ? EventId : NoSecondaryId;
-}
-
-QUrl MatrixUri::toUrl(UriForm form) const
-{
-    if (!isValid())
-        return {};
-
-    if (form == CanonicalUri)
-        return *this;
-
-    QUrl url;
-    url.setScheme("https");
-    url.setHost("matrix.to");
-    url.setPath("/");
-    auto fragment = primaryId();
-    if (const auto& secId = secondaryId(); !secId.isEmpty())
-        fragment += '/' + secId;
-    if (const auto& q = query(); !q.isEmpty())
-        fragment += '?' + q;
-    url.setFragment(fragment);
-    return url;
-}
-
-QString MatrixUri::toDisplayString(MatrixUri::UriForm form) const
-{
-    return toUrl(form).toDisplayString();
-}
-
-QString MatrixUri::primaryId() const
-{
-    if (primaryType_ == Empty || primaryType_ == Invalid)
-        return {};
-
-    const auto& idStem = path().section('/', 1, 1);
-    return idStem.isEmpty() ? idStem : primaryType_ + idStem;
-}
-
-QString MatrixUri::secondaryId() const
-{
-    const auto& idStem = path().section('/', 3);
-    return idStem.isEmpty() ? idStem : secondaryType() + idStem;
-}
-
-QString MatrixUri::action() const
-{
-    return QUrlQuery { query() }.queryItemValue("action");
-}
-
-QStringList MatrixUri::viaServers() const
-{
-    return QUrlQuery { query() }.allQueryItemValues(QStringLiteral("via"),
-                                                    QUrl::EncodeReserved);
-}
-
-bool MatrixUri::isValid() const
-{
-    return primaryType_ != Empty && primaryType_ != Invalid;
-}
-
-UriResolveResult Quotient::visitResource(
-    Connection* account, const MatrixUri& uri,
-    std::function<void(User*)> userHandler,
-    std::function<void(Room*, QString)> roomEventHandler,
-    std::function<void(Connection*, QString, QStringList)> joinHandler)
-{
-    Q_ASSERT_X(account != nullptr, __FUNCTION__,
-               "The Connection argument passed to visit/openResource must not "
-               "be nullptr");
-    if (uri.action() == "join") {
-        if (uri.type() != MatrixUri::RoomAlias
-            && uri.type() != MatrixUri::RoomId)
-            return MalformedUri;
-
-        joinHandler(account, uri.primaryId(), uri.viaServers());
-        return UriResolved;
-    }
-
-    Room* room = nullptr;
-    switch (uri.type()) {
-    case MatrixUri::Invalid:
-        return MalformedUri;
-    case MatrixUri::Empty:
-        return EmptyMatrixId;
-    case MatrixUri::UserId:
-        if (auto* user = account->user(uri.primaryId())) {
-            userHandler(user);
-            return UriResolved;
-        }
-        return MalformedUri;
-    case MatrixUri::RoomId:
-        if ((room = account->room(uri.primaryId())))
-            break;
-        return UnknownMatrixId;
-    case MatrixUri::RoomAlias:
-        if ((room = account->roomByAlias(uri.primaryId())))
-            break;
-        [[fallthrough]];
-    default:
-        return UnknownMatrixId;
-    }
-    roomEventHandler(room, uri.secondaryId());
-    return UriResolved;
-}
-
-UriResolveResult
-ResourceResolver::openResource(Connection* account, const QString& identifier,
-                               const QString& action)
-{
-    return openResource(account, MatrixUri(identifier), action);
-}
-
-UriResolveResult ResourceResolver::openResource(Connection* account,
-                                                const MatrixUri& uri,
-                                                const QString& overrideAction)
-{
-    return visitResource(
-        account, uri,
-        [this, &overrideAction](User* u) { emit userAction(u, overrideAction); },
-        [this, &overrideAction](Room* room, const QString& eventId) {
-            emit roomAction(room, eventId, overrideAction);
-        },
-        [this](Connection* account, const QString& roomAliasOrId,
-               const QStringList& viaServers) {
-            emit joinAction(account, roomAliasOrId, viaServers);
-        });
-}
diff --git a/lib/resourceresolver.h b/lib/resourceresolver.h
deleted file mode 100644
index fea07e97b..000000000
--- a/lib/resourceresolver.h
+++ /dev/null
@@ -1,181 +0,0 @@
-#pragma once
-
-#include "quotient_common.h"
-
-#include <QtCore/QUrl>
-#include <QtCore/QUrlQuery>
-#include <QtCore/QObject>
-
-#include <functional>
-
-namespace Quotient {
-class Connection;
-class Room;
-class User;
-
-/*! \brief A wrapper around a Matrix URI or identifier
- *
- * This class encapsulates a Matrix resource identifier, passed in either of
- * 3 forms: a plain Matrix identifier (sigil, localpart, serverpart or, for
- * modern event ids, sigil and base64 hash); an MSC2312 URI (aka matrix: URI);
- * or a matrix.to URL. The input can be either encoded (serverparts with
- * punycode, the rest with percent-encoding) or unencoded (in this case it is
- * the caller's responsibility to resolve all possible ambiguities).
- *
- * The class provides functions to check the validity of the identifier,
- * its type, and obtain components, also in either unencoded (for displaying)
- * or encoded (for APIs) form.
- */
-class MatrixUri : private QUrl {
-    Q_GADGET
-    Q_PROPERTY(QString primaryId READ primaryId CONSTANT)
-    Q_PROPERTY(QString secondaryId READ secondaryId CONSTANT)
-//    Q_PROPERTY(QUrlQuery query READ query CONSTANT)
-    Q_PROPERTY(QString action READ action CONSTANT)
-//    Q_PROPERTY(QStringList viaServers READ viaServers CONSTANT)
-public:
-    enum Type : char {
-        Invalid = char(-1),
-        Empty = 0x0,
-        UserId = '@',
-        RoomId = '!',
-        RoomAlias = '#',
-        Group = '+'
-    };
-    Q_ENUM(Type)
-    enum SecondaryType : char {
-        NoSecondaryId = 0x0,
-        EventId = '$'
-    };
-
-    enum UriForm : short { CanonicalUri, MatrixToUri };
-    Q_ENUM(UriForm)
-
-    /// Construct an empty Matrix URI
-    MatrixUri() = default;
-    /*! \brief Decode a user input string to a Matrix identifier
-     *
-     * Accepts plain Matrix ids, MSC2312 URIs (aka matrix: URIs) and
-     * matrix.to URLs. In case of URIs/URLs, it uses QUrl's TolerantMode
-     * parser to decode common mistakes/irregularities (see QUrl documentation
-     * for more details).
-     */
-    MatrixUri(const QString& uriOrId);
-
-    /// Construct a Matrix URI from components
-    explicit MatrixUri(QByteArray primaryId, QByteArray secondaryId = {},
-                       QString query = {});
-    /// Construct a Matrix URI from matrix.to or MSC2312 (matrix:) URI
-    explicit MatrixUri(QUrl url);
-
-    static MatrixUri fromUserInput(const QString& uriOrId);
-    static MatrixUri fromUrl(QUrl url);
-
-    /// Get the primary type of the Matrix URI (user id, room id or alias)
-    /*! Note that this does not include an event as a separate type, since
-     * events can only be addressed inside of rooms, which, in turn, are
-     * addressed either by id or alias. If you need to check whether the URI
-     * is specifically an event URI, use secondaryType() instead.
-     */
-    Type type() const;
-    SecondaryType secondaryType() const;
-    QUrl toUrl(UriForm form = CanonicalUri) const;
-    QString toDisplayString(UriForm form = CanonicalUri) const;
-    QString primaryId() const;
-    QString secondaryId() const;
-    QString action() const;
-    QStringList viaServers() const;
-    bool isValid() const;
-    using QUrl::isEmpty, QUrl::path, QUrl::query, QUrl::fragment;
-
-private:
-
-    Type primaryType_ = Empty;
-};
-
-/*! \brief Resolve the resource and invoke an action on it, visitor style
- *
- * This template function encapsulates the logic of resolving a Matrix
- * identifier or URI into a Quotient object (or objects) and applying an
- * appropriate action handler from the set provided by the caller to it.
- * A typical use case for that is opening a room or mentioning a user in
- * response to clicking on a Matrix URI or identifier.
- *
- * \param account The connection used as a context to resolve the identifier
- *
- * \param uri The Matrix identifier or URI; MSC2312 URIs and classic Matrix IDs
- *            are supported
- *
- * \sa ResourceResolver
- */
-UriResolveResult
-visitResource(Connection* account, const MatrixUri& uri,
-              std::function<void(User*)> userHandler,
-              std::function<void(Room*, QString)> roomEventHandler,
-              std::function<void(Connection*, QString, QStringList)> joinHandler);
-
-/*! \brief Matrix resource resolver
- * TODO: rewrite
- * Similar to visitResource(), this class encapsulates the logic of resolving
- * a Matrix identifier or a URI into Quotient object(s) and applying an action
- * to the resolved object(s). Instead of using a C++ visitor pattern, it
- * announces the request through Qt's signals passing the resolved object(s)
- * through those (still in a typesafe way).
- *
- * This class is aimed primarily at clients where invoking the resolving/action
- * and handling the action are happening in decoupled parts of the code; it's
- * also useful to operate on Matrix identifiers and URIs from QML/JS code
- * that cannot call visitResource due to QML/C++ interface limitations.
- */
-class ResourceResolver : public QObject {
-    Q_OBJECT
-public:
-    explicit ResourceResolver(QObject* parent = nullptr) : QObject(parent)
-    { }
-
-    /*! \brief Resolve the resource and request an action on it, signal style
-     *
-     * This method:
-     * 1. Resolves \p identifier into an actual object (Room or User), with
-     *    possible additional data such as event id, in the context of
-     *    \p account.
-     * 2. If the resolving is successful, depending on the type of the object,
-     *    emits the respective signal to which the client must connect in order
-     *    to apply the action to the resource (open a room, mention a user etc.).
-     * 3. Returns the result of resolving the resource.
-     *
-     * Note that the action can be applied either synchronously or entirely
-     * asynchronously; ResourceResolver does not restrain the client code
-     * to use either method. The resource resolving part is entirely synchronous
-     * though. If the synchronous operation is chosen, only
-     * direct connections to ResourceResolver signals must be used, and
-     * the caller should check the future's state immediately after calling
-     * openResource() to process any feedback from the resolver and/or action
-     * handler. If asynchronous operation is needed then either direct or queued
-     * connections to ResourceResolver's signals can be used and the caller
-     * must both check the ResourceFuture state right after calling openResource
-     * and also connect to ResourceFuture::ready() signal in order to process
-     * the result of resolving and action.
-     */
-    Q_INVOKABLE UriResolveResult openResource(Connection* account,
-                                                   const QString& identifier,
-                                                   const QString& action = {});
-    Q_INVOKABLE UriResolveResult
-    openResource(Connection* account, const MatrixUri& uri,
-                 const QString& overrideAction = {});
-
-signals:
-    /// An action on a user has been requested
-    void userAction(Quotient::User* user, QString action);
-
-    /// An action on a room has been requested, with optional event id
-    void roomAction(Quotient::Room* room, QString eventId, QString action);
-
-    /// A join action has been requested, with optional 'via' servers
-    void joinAction(Quotient::Connection* account, QString roomAliasOrId,
-                    QStringList viaServers);
-};
-
-} // namespace Quotient
-
-
diff --git a/lib/uri.cpp b/lib/uri.cpp
new file mode 100644
index 000000000..e81933dc0
--- /dev/null
+++ b/lib/uri.cpp
@@ -0,0 +1,181 @@
+#include "uri.h"
+
+#include "logging.h"
+
+#include <QtCore/QRegularExpression>
+
+using namespace Quotient;
+
+struct ReplacePair { QByteArray uriString; char sigil; };
+static const auto replacePairs = { ReplacePair { "user/", '@' },
+                                   { "roomid/", '!' },
+                                   { "room/", '#' },
+                                   // The notation for bare event ids is not
+                                   // proposed in MSC2312 (and anywhere, as yet)
+                                   { "event/", '$' } };
+
+Uri::Uri(QByteArray primaryId, QByteArray secondaryId, QString query)
+{
+    if (primaryId.isEmpty())
+        primaryType_ = Empty;
+    else {
+        setScheme("matrix");
+        QString pathToBe;
+        primaryType_ = Invalid;
+        for (const auto& p: replacePairs)
+            if (primaryId[0] == p.sigil) {
+                primaryType_ = Type(p.sigil);
+                pathToBe = p.uriString + primaryId.mid(1);
+                break;
+            }
+        if (!secondaryId.isEmpty())
+            pathToBe += "/event/" + secondaryId.mid(1);
+        setPath(pathToBe);
+    }
+    setQuery(std::move(query));
+}
+
+Uri::Uri(QUrl url) : QUrl(std::move(url))
+{
+    // NB: don't try to use `url` from here on, it's moved-from and empty
+    if (isEmpty())
+        return; // primaryType_ == Empty
+
+    if (!QUrl::isValid()) { // MatrixUri::isValid() checks primaryType_
+        primaryType_ = Invalid;
+        return;
+    }
+
+    if (scheme() == "matrix") {
+        // Check sanity as per https://github.com/matrix-org/matrix-doc/pull/2312
+        const auto& urlPath = path();
+        const auto& splitPath = urlPath.splitRef('/');
+        switch (splitPath.size()) {
+        case 2:
+            break;
+        case 4:
+            if (splitPath[2] == "event")
+                break;
+            [[fallthrough]];
+        default:
+            return; // Invalid
+        }
+
+        for (const auto& p: replacePairs)
+            if (urlPath.startsWith(p.uriString)) {
+                primaryType_ = Type(p.sigil);
+                return; // The only valid return path for matrix: URIs
+            }
+        qCDebug(MAIN) << "The matrix: URI is not recognised:"
+                      << toDisplayString();
+        return;
+    }
+
+    primaryType_ = NonMatrix; // Default, unless overridden by the code below
+    if (scheme() == "https" && authority() == "matrix.to") {
+        // See https://matrix.org/docs/spec/appendices#matrix-to-navigation
+        static const QRegularExpression MatrixToUrlRE {
+            R"(^/(?<main>[^/?]+)(/(?<sec>[^?]+))?(\?(?<query>.+))?$)"
+        };
+        // matrix.to accepts both literal sigils (as well as & and ? used in
+        // its "query" substitute) and their %-encoded forms;
+        // so force QUrl to decode everything.
+        auto f = fragment(QUrl::FullyDecoded);
+        if (auto&& m = MatrixToUrlRE.match(f); m.hasMatch())
+            *this = Uri { m.captured("main").toUtf8(),
+                          m.captured("sec").toUtf8(), m.captured("query") };
+    }
+}
+
+Uri::Uri(const QString& uriOrId) : Uri(fromUserInput(uriOrId)) {}
+
+Uri Uri::fromUserInput(const QString& uriOrId)
+{
+    if (uriOrId.isEmpty())
+        return {}; // type() == None
+
+    // A quick check if uriOrId is a plain Matrix id
+    // Bare event ids cannot be resolved without a room scope as per the current
+    // spec but there's a movement towards making them navigable (see, e.g.,
+    // https://github.com/matrix-org/matrix-doc/pull/2644) - so treat them
+    // as valid
+    if (QStringLiteral("!@#+$").contains(uriOrId[0]))
+        return Uri { uriOrId.toUtf8() };
+
+    return Uri { QUrl::fromUserInput(uriOrId) };
+}
+
+Uri::Type Uri::type() const { return primaryType_; }
+
+Uri::SecondaryType Uri::secondaryType() const
+{
+    return path().section('/', 2, 2) == "event" ? EventId : NoSecondaryId;
+}
+
+QUrl Uri::toUrl(UriForm form) const
+{
+    if (!isValid())
+        return {};
+
+    if (form == CanonicalUri || type() == NonMatrix)
+        return *this;
+
+    QUrl url;
+    url.setScheme("https");
+    url.setHost("matrix.to");
+    url.setPath("/");
+    auto fragment = primaryId();
+    if (const auto& secId = secondaryId(); !secId.isEmpty())
+        fragment += '/' + secId;
+    if (const auto& q = query(); !q.isEmpty())
+        fragment += '?' + q;
+    url.setFragment(fragment);
+    return url;
+}
+
+QString Uri::primaryId() const
+{
+    if (primaryType_ == Empty || primaryType_ == Invalid)
+        return {};
+
+    const auto& idStem = path().section('/', 1, 1);
+    return idStem.isEmpty() ? idStem : primaryType_ + idStem;
+}
+
+QString Uri::secondaryId() const
+{
+    const auto& idStem = path().section('/', 3);
+    return idStem.isEmpty() ? idStem : secondaryType() + idStem;
+}
+
+static const auto ActionKey = QStringLiteral("action");
+
+QString Uri::action() const
+{
+    return type() == NonMatrix || !isValid()
+               ? QString()
+               : QUrlQuery { query() }.queryItemValue(ActionKey);
+}
+
+void Uri::setAction(const QString& newAction)
+{
+    if (!isValid()) {
+        qCWarning(MAIN) << "Cannot set an action on an invalid Quotient::Uri";
+        return;
+    }
+    QUrlQuery q { query() };
+    q.removeQueryItem(ActionKey);
+    q.addQueryItem(ActionKey, newAction);
+    setQuery(q);
+}
+
+QStringList Uri::viaServers() const
+{
+    return QUrlQuery { query() }.allQueryItemValues(QStringLiteral("via"),
+                                                    QUrl::EncodeReserved);
+}
+
+bool Uri::isValid() const
+{
+    return primaryType_ != Empty && primaryType_ != Invalid;
+}
diff --git a/lib/uri.h b/lib/uri.h
new file mode 100644
index 000000000..270766dd5
--- /dev/null
+++ b/lib/uri.h
@@ -0,0 +1,85 @@
+#pragma once
+
+#include "quotient_common.h"
+
+#include <QtCore/QUrl>
+#include <QtCore/QUrlQuery>
+
+namespace Quotient {
+
+/*! \brief A wrapper around a Matrix URI or identifier
+ *
+ * This class encapsulates a Matrix resource identifier, passed in either of
+ * 3 forms: a plain Matrix identifier (sigil, localpart, serverpart or, for
+ * modern event ids, sigil and base64 hash); an MSC2312 URI (aka matrix: URI);
+ * or a matrix.to URL. The input can be either encoded (serverparts with
+ * punycode, the rest with percent-encoding) or unencoded (in this case it is
+ * the caller's responsibility to resolve all possible ambiguities).
+ *
+ * The class provides functions to check the validity of the identifier,
+ * its type, and obtain components, also in either unencoded (for displaying)
+ * or encoded (for APIs) form.
+ */
+class Uri : private QUrl {
+    Q_GADGET
+public:
+    enum Type : char {
+        Invalid = char(-1),
+        Empty = 0x0,
+        UserId = '@',
+        RoomId = '!',
+        RoomAlias = '#',
+        Group = '+',
+        BareEventId = '$', // https://github.com/matrix-org/matrix-doc/pull/2644
+        NonMatrix = ':'
+    };
+    Q_ENUM(Type)
+    enum SecondaryType : char { NoSecondaryId = 0x0, EventId = '$' };
+    Q_ENUM(SecondaryType)
+
+    enum UriForm : short { CanonicalUri, MatrixToUri };
+    Q_ENUM(UriForm)
+
+    /// Construct an empty Matrix URI
+    Uri() = default;
+    /*! \brief Decode a user input string to a Matrix identifier
+     *
+     * Accepts plain Matrix ids, MSC2312 URIs (aka matrix: URIs) and
+     * matrix.to URLs. In case of URIs/URLs, it uses QUrl's TolerantMode
+     * parser to decode common mistakes/irregularities (see QUrl documentation
+     * for more details).
+     */
+    Uri(const QString& uriOrId);
+
+    /// Construct a Matrix URI from components
+    explicit Uri(QByteArray primaryId, QByteArray secondaryId = {},
+                 QString query = {});
+    /// Construct a Matrix URI from matrix.to or MSC2312 (matrix:) URI
+    explicit Uri(QUrl url);
+
+    static Uri fromUserInput(const QString& uriOrId);
+    static Uri fromUrl(QUrl url);
+
+    /// Get the primary type of the Matrix URI (user id, room id or alias)
+    /*! Note that this does not include an event as a separate type, since
+     * events can only be addressed inside of rooms, which, in turn, are
+     * addressed either by id or alias. If you need to check whether the URI
+     * is specifically an event URI, use secondaryType() instead.
+     */
+    Q_INVOKABLE Type type() const;
+    Q_INVOKABLE SecondaryType secondaryType() const;
+    Q_INVOKABLE QUrl toUrl(UriForm form = CanonicalUri) const;
+    Q_INVOKABLE QString primaryId() const;
+    Q_INVOKABLE QString secondaryId() const;
+    Q_INVOKABLE QString action() const;
+    Q_INVOKABLE void setAction(const QString& newAction);
+    Q_INVOKABLE QStringList viaServers() const;
+    Q_INVOKABLE bool isValid() const;
+    using QUrl::path, QUrl::query, QUrl::fragment;
+    using QUrl::isEmpty, QUrl::toDisplayString;
+
+private:
+    Type primaryType_ = Empty;
+};
+
+}
diff --git a/lib/uriresolver.cpp b/lib/uriresolver.cpp
new file mode 100644
index 000000000..5052890b8
--- /dev/null
+++ b/lib/uriresolver.cpp
@@ -0,0 +1,110 @@
+#include "uriresolver.h"
+
+#include "connection.h"
+#include "user.h"
+
+using namespace Quotient;
+
+UriResolveResult UriResolverBase::visitResource(Connection* account,
+                                                        const Uri& uri)
+{
+    switch (uri.type()) {
+    case Uri::NonMatrix:
+        return visitNonMatrix(uri.toUrl()) ? UriResolved : CouldNotResolve;
+    case Uri::Invalid:
+    case Uri::Empty:
+        return InvalidUri;
+    default:;
+    }
+
+    if (!account)
+        return NoAccount;
+
+    switch (uri.type()) {
+    case Uri::UserId: {
+        if (uri.action() == "join")
+            return IncorrectAction;
+        auto* user = account->user(uri.primaryId());
+        Q_ASSERT(user != nullptr);
+        visitUser(user, uri.action());
+        return UriResolved;
+    }
+    case Uri::RoomId:
+    case Uri::RoomAlias: {
+        auto* room = uri.type() == Uri::RoomId
+                         ? account->room(uri.primaryId())
+                         : account->roomByAlias(uri.primaryId());
+        if (room != nullptr) {
+            visitRoom(room, uri.secondaryId());
+            return UriResolved;
+        }
+        if (uri.action() == "join") {
+            joinRoom(account, uri.primaryId(), uri.viaServers());
+            return UriResolved;
+        }
+        [[fallthrough]];
+    }
+    default:
+        return CouldNotResolve;
+    }
+}
+
+template <typename... FnTs>
+class StaticUriDispatcher : public UriResolverBase {
+public:
+    StaticUriDispatcher(const FnTs&... fns) : fns_(fns...) {}
+
+private:
+    void visitUser(User* user, const QString& action) override
+    {
+        std::get<0>(fns_)(user, action);
+    }
+    void visitRoom(Room* room, const QString& eventId) override
+    {
+        std::get<1>(fns_)(room, eventId);
+    }
+    void joinRoom(Connection* account, const QString& roomAliasOrId,
+                  const QStringList& viaServers = {}) override
+    {
+        std::get<2>(fns_)(account, roomAliasOrId, viaServers);
+    }
+    bool visitNonMatrix(const QUrl& url) override
+    {
+        return std::get<3>(fns_)(url);
+    }
+
+    std::tuple<FnTs...> fns_;
+};
+
+UriResolveResult Quotient::visitResource(
+    Connection* account, const Uri& uri,
+    std::function<void(User*, QString)> userHandler,
+    std::function<void(Room*, QString)> roomEventHandler,
+    std::function<void(Connection*, QString, QStringList)> joinHandler,
+    std::function<bool(const QUrl&)> nonMatrixHandler)
+{
+    return StaticUriDispatcher(userHandler, roomEventHandler, joinHandler,
+                               nonMatrixHandler)
+        .visitResource(account, uri);
+}
+
+void UriDispatcher::visitUser(User *user, const QString &action)
+{
+    emit userAction(user, action);
+}
+
+void UriDispatcher::visitRoom(Room *room, const QString &eventId)
+{
+    emit roomAction(room, eventId);
+}
+
+void UriDispatcher::joinRoom(Connection *account, const QString &roomAliasOrId, const QStringList &viaServers)
+{
+    emit joinAction(account, roomAliasOrId, viaServers);
+}
+
+bool UriDispatcher::visitNonMatrix(const QUrl &url)
+{
+    emit nonMatrixAction(url);
+    return true;
+}
diff --git a/lib/uriresolver.h b/lib/uriresolver.h
new file mode 100644
index 000000000..914ddf027
--- /dev/null
+++ b/lib/uriresolver.h
@@ -0,0 +1,162 @@
+#pragma once
+
+#include "uri.h"
+
+#include <QtCore/QObject>
+
+#include <functional>
+
+namespace Quotient {
+class Connection;
+class Room;
+class User;
+
+/*! \brief Abstract class to resolve the resource and act on it
+ *
+ * This class encapsulates the logic of resolving a Matrix identifier or URI
+ * into a Quotient object (or objects) and calling an appropriate handler on it.
+ * It is a type-safe way of handling a URI with no prior context on its type
+ * in cases like, e.g., when a user clicks on a URI in the application.
+ *
+ * This class provides empty "handlers" for each type of URI to facilitate
+ * gradual implementation. Derived classes are encouraged to override as many
+ * of them as possible.
+ */
+class UriResolverBase {
+public:
+    /*! \brief Resolve the resource and dispatch an action depending on its type
+     *
+     * This method:
+     * 1. Resolves \p uri into an actual object (e.g., Room or User),
+     *    with possible additional data such as event id, in the context of
+     *    \p account.
+     * 2. If the resolving is successful, depending on the type of the object,
+     *    calls the appropriate virtual function (defined in a derived
+     *    concrete class) to perform an action on the resource (open a room,
+     *    mention a user etc.).
+     * 3. Returns the result of resolving the resource.
+     */
+    UriResolveResult visitResource(Connection* account, const Uri& uri);
+
+protected:
+    /// Called by visitResource() when the passed URI identifies a Matrix user
+    virtual void visitUser(User* user, const QString& action) {}
+    /// Called by visitResource() when the passed URI identifies a room or
+    /// an event in a room
+    virtual void visitRoom(Room* room, const QString& eventId) {}
+    /// Called by visitResource() when the passed URI has `action() == "join"`
+    /// and identifies a room that the user defined by the Connection argument
+    /// is not a member of
+    virtual void joinRoom(Connection* account, const QString& roomAliasOrId,
+                          const QStringList& viaServers = {})
+    {}
+    /// Called by visitResource() when the passed URI has `type() == NonMatrix`
+    /*!
+     * Should return true if the URI is considered resolved, false otherwise.
+     * A basic implementation in a graphical client can look like
+     * `return QDesktopServices::openUrl(url);` but it's strongly advised to
+     * ask for a user confirmation beforehand.
+     */
+    virtual bool visitNonMatrix(const QUrl& url) { return false; }
+};
+
+/*! \brief Resolve the resource and invoke an action on it, via function objects
+ *
+ * This function encapsulates the logic of resolving a Matrix identifier or URI
+ * into a Quotient object (or objects) and calling an appropriate handler on it.
+ * Unlike UriResolverBase it accepts the list of handlers from
+ * the caller; internally it's uses a minimal UriResolverBase class
+ *
+ * \param account The connection used as a context to resolve the identifier
+ *
+ * \param uri A URI that can represent a Matrix entity
+ *
+ * \param userHandler Called when the passed URI identifies a Matrix user
+ *
+ * \param roomEventHandler Called when the passed URI identifies a room or
+ *                         an event in a room
+ *
+ * \param joinHandler Called when the passed URI has `action() == "join"` and
+ *                    identifies a room that the user defined by
+ *                    the Connection argument is not a member of
+ *
+ * \param nonMatrixHandler Called when the passed URI has `type() == NonMatrix`;
+ *                         should return true if the URI is considered resolved,
+ *                         false otherwise
+ *
+ * \sa UriResolverBase, UriDispatcher
+ */
+UriResolveResult
+visitResource(Connection* account, const Uri& uri,
+              std::function<void(User*, QString)> userHandler,
+              std::function<void(Room*, QString)> roomEventHandler,
+              std::function<void(Connection*, QString, QStringList)> joinHandler,
+              std::function<bool(const QUrl&)> nonMatrixHandler);
+
+/*! \brief Check that the resource is resolvable with no action on it */
+inline UriResolveResult checkResource(Connection* account,
+                                      const Uri& uri)
+{
+    return visitResource(
+        account, uri, [](auto, auto) {}, [](auto, auto) {},
+        [](auto, auto, auto) {}, [](auto) { return false; });
+}
+
+/*! \brief Resolve the resource and invoke an action on it, via Qt signals
+ *
+ * This is an implementation of UriResolverBase that is based on
+ * QObject and uses Qt signals instead of virtual functions to provide an
+ * open-ended interface for visitors.
+ *
+ * This class is aimed primarily at clients where invoking the resolving/action
+ * and handling the action are happening in decoupled parts of the code; it's
+ * also useful to operate on Matrix identifiers and URIs from QML/JS code
+ * that cannot call resolveResource() due to QML/C++ interface limitations.
+ *
+ * This class does not restrain the client code to a certain type of
+ * connections: both direct and queued (or a mix) will work fine. One limitation
+ * caused by that is there's no way to indicate if a non-Matrix URI has been
+ * successfully resolved - a signal always returns void.
+ *
+ * Note that in case of using (non-blocking) queued connections the code that
+ * calls resolveResource() should not expect the action to be performed
+ * synchronously - the returned value is the result of resolving the URI,
+ * not acting on it.
+ */
+class UriDispatcher : public QObject, public UriResolverBase {
+    Q_OBJECT
+public:
+    explicit UriDispatcher(QObject* parent = nullptr) : QObject(parent) {}
+
+    // It's actually UriResolverBase::visitResource() but with Q_INVOKABLE
+    Q_INVOKABLE UriResolveResult resolveResource(Connection* account,
+                                                 const Uri& uri)
+    {
+        return UriResolverBase::visitResource(account, uri);
+    }
+
+signals:
+    /// An action on a user has been requested
+    void userAction(Quotient::User* user, QString action);
+
+    /// An action on a room has been requested, with optional event id
+    void roomAction(Quotient::Room* room, QString eventId);
+
+    /// A join action has been requested, with optional 'via' servers
+    void joinAction(Quotient::Connection* account, QString roomAliasOrId,
+                    QStringList viaServers);
+
+    /// An action on a non-Matrix URL has been requested
+    void nonMatrixAction(QUrl url);
+
+private:
+    void visitUser(User* user, const QString& action) override;
+    void visitRoom(Room* room, const QString& eventId) override;
+    void joinRoom(Connection* account, const QString& roomAliasOrId,
+                  const QStringList& viaServers = {}) override;
+    bool visitNonMatrix(const QUrl& url) override;
+};
+
+} // namespace Quotient
+
+
diff --git a/libquotient.pri b/libquotient.pri
index f0057712d..98fe3b039 100644
--- a/libquotient.pri
+++ b/libquotient.pri
@@ -34,7 +34,8 @@ HEADERS += \
     $$SRCPATH/room.h \
     $$SRCPATH/user.h \
     $$SRCPATH/avatar.h \
-    $$SRCPATH/resourceresolver.h \
+    $$SRCPATH/uri.h \
+    $$SRCPATH/uriresolver.h \
     $$SRCPATH/syncdata.h \
     $$SRCPATH/quotient_common.h \
     $$SRCPATH/util.h \
@@ -91,7 +92,8 @@ SOURCES += \
     $$SRCPATH/room.cpp \
     $$SRCPATH/user.cpp \
     $$SRCPATH/avatar.cpp \
-    $$SRCPATH/resourceresolver.cpp \
+    $$SRCPATH/uri.cpp \
+    $$SRCPATH/uriresolver.cpp \
     $$SRCPATH/syncdata.cpp \
     $$SRCPATH/util.cpp \
     $$SRCPATH/events/event.cpp \
diff --git a/tests/quotest.cpp b/tests/quotest.cpp
index 529154547..c2c50df2b 100644
--- a/tests/quotest.cpp
+++ b/tests/quotest.cpp
@@ -2,7 +2,7 @@
 #include "connection.h"
 #include "room.h"
 #include "user.h"
-#include "resourceresolver.h"
+#include "uriresolver.h"
 
 #include "csapi/joining.h"
 #include "csapi/leaving.h"
@@ -622,7 +622,7 @@ TEST_IMPL(visitResources)
     // assumes that ResourceResolver::openResource is implemented in terms
     // of ResourceResolver::visitResource, so the latter doesn't need a
     // separate test.
-    static ResourceResolver rr;
+    static UriDispatcher ud;
 
     // This lambda returns true in case of error, false if it's fine so far
     auto testResourceResolver = [this, thisTest](const QStringList& uris,
@@ -630,12 +630,12 @@ TEST_IMPL(visitResources)
                                                  QVariantList otherArgs = {}) {
         int r = qRegisterMetaType<decltype(target)>();
         Q_ASSERT(r != 0);
-        QSignalSpy spy(&rr, signal);
+        QSignalSpy spy(&ud, signal);
         for (const auto& uriString: uris) {
-            MatrixUri uri { uriString };
+            Uri uri { uriString };
             clog << "Checking " << uriString.toStdString()
                  << " -> " << uri.toDisplayString().toStdString() << endl;
-            rr.openResource(connection(), uriString);
+            ud.visitResource(connection(), uriString);
             if (spy.count() != 1) {
                 clog << "Wrong number of signal emissions (" << spy.count()
                      << ')' << endl;
@@ -666,7 +666,7 @@ TEST_IMPL(visitResources)
     // Basic tests
     QUrl invalidUrl { "https://" };
     invalidUrl.setAuthority("---:@@@");
-    const MatrixUri emptyUri {}, uriFromEmptyUrl {},
+    const Uri emptyUri {}, uriFromEmptyUrl {},
         invalidMatrixUri { QStringLiteral("matrix:&invalid@") },
         matrixUriFromInvalidUrl { invalidUrl };
 
@@ -710,7 +710,7 @@ TEST_IMPL(visitResources)
     // exist, as yet) - only to be syntactically correct
     static const auto& joinRoomAlias =
         QStringLiteral("unjoined:example.org"); // # will be added below
-    QString joinQuery { "?action=join" };
+    static const QString joinQuery { "?action=join" };
     static const QStringList joinByAliasUris {
         "matrix:room/" + joinRoomAlias + joinQuery,
         "https://matrix.to/#/%23"/*`#`*/ + joinRoomAlias + joinQuery
@@ -728,14 +728,14 @@ TEST_IMPL(visitResources)
     };
     // If any test breaks, the breaking call will return true, and further
     // execution will be cut by ||'s short-circuiting
-    if (testResourceResolver(roomUris, &ResourceResolver::roomAction, room())
-        || testResourceResolver(userUris, &ResourceResolver::userAction,
+    if (testResourceResolver(roomUris, &UriDispatcher::roomAction, room())
+        || testResourceResolver(userUris, &UriDispatcher::userAction,
                                 connection()->user())
-        || testResourceResolver(eventUris, &ResourceResolver::roomAction,
+        || testResourceResolver(eventUris, &UriDispatcher::roomAction,
                                 room(), { eventId })
-        || testResourceResolver(joinByAliasUris, &ResourceResolver::joinAction,
+        || testResourceResolver(joinByAliasUris, &UriDispatcher::joinAction,
                                 connection(), { '#' + joinRoomAlias })
-        || testResourceResolver(joinByIdUris, &ResourceResolver::joinAction,
+        || testResourceResolver(joinByIdUris, &UriDispatcher::joinAction,
                                 connection(), { joinRoomId, viaServers }))
         return true;
     // TODO: negative cases

From 1529f46e6dd457d059fb7e6e9cd10fa0b0399553 Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Mon, 20 Jul 2020 19:33:19 +0200
Subject: [PATCH 08/10] UriResolverBase::visitUser(): add return value

To enable reporting when the action is incorrect.
---
 lib/uriresolver.cpp | 18 ++++++++++--------
 lib/uriresolver.h   | 18 ++++++++++++------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/lib/uriresolver.cpp b/lib/uriresolver.cpp
index 5052890b8..ec30512c0 100644
--- a/lib/uriresolver.cpp
+++ b/lib/uriresolver.cpp
@@ -6,7 +6,7 @@
 using namespace Quotient;
 
 UriResolveResult UriResolverBase::visitResource(Connection* account,
-                                                        const Uri& uri)
+                                                const Uri& uri)
 {
     switch (uri.type()) {
     case Uri::NonMatrix:
@@ -26,8 +26,7 @@ UriResolveResult UriResolverBase::visitResource(Connection* account,
             return IncorrectAction;
         auto* user = account->user(uri.primaryId());
         Q_ASSERT(user != nullptr);
-        visitUser(user, uri.action());
-        return UriResolved;
+        return visitUser(user, uri.action());
     }
     case Uri::RoomId:
     case Uri::RoomAlias: {
@@ -49,15 +48,16 @@ UriResolveResult UriResolverBase::visitResource(Connection* account,
     }
 }
 
+// This template is only instantiated once, for Quotient::visitResource()
 template <typename... FnTs>
 class StaticUriDispatcher : public UriResolverBase {
 public:
     StaticUriDispatcher(const FnTs&... fns) : fns_(fns...) {}
 
 private:
-    void visitUser(User* user, const QString& action) override
+    UriResolveResult visitUser(User* user, const QString& action) override
     {
-        std::get<0>(fns_)(user, action);
+        return std::get<0>(fns_)(user, action);
     }
     void visitRoom(Room* room, const QString& eventId) override
     {
@@ -78,7 +78,7 @@ class StaticUriDispatcher : public UriResolverBase {
 
 UriResolveResult Quotient::visitResource(
     Connection* account, const Uri& uri,
-    std::function<void(User*, QString)> userHandler,
+    std::function<UriResolveResult(User*, QString)> userHandler,
     std::function<void(Room*, QString)> roomEventHandler,
     std::function<void(Connection*, QString, QStringList)> joinHandler,
     std::function<bool(const QUrl&)> nonMatrixHandler)
@@ -88,9 +88,10 @@ UriResolveResult Quotient::visitResource(
         .visitResource(account, uri);
 }
 
-void UriDispatcher::visitUser(User *user, const QString &action)
+UriResolveResult UriDispatcher::visitUser(User *user, const QString &action)
 {
     emit userAction(user, action);
+    return UriResolved;
 }
 
 void UriDispatcher::visitRoom(Room *room, const QString &eventId)
@@ -98,7 +99,8 @@ void UriDispatcher::visitRoom(Room *room, const QString &eventId)
     emit roomAction(room, eventId);
 }
 
-void UriDispatcher::joinRoom(Connection *account, const QString &roomAliasOrId, const QStringList &viaServers)
+void UriDispatcher::joinRoom(Connection* account, const QString& roomAliasOrId,
+                             const QStringList& viaServers)
 {
     emit joinAction(account, roomAliasOrId, viaServers);
 }
diff --git a/lib/uriresolver.h b/lib/uriresolver.h
index 914ddf027..9b2ced9db 100644
--- a/lib/uriresolver.h
+++ b/lib/uriresolver.h
@@ -40,7 +40,14 @@ class UriResolverBase {
 
 protected:
     /// Called by visitResource() when the passed URI identifies a Matrix user
-    virtual void visitUser(User* user, const QString& action) {}
+    /*!
+     * \return IncorrectAction if the action is not correct or not supported;
+     *         UriResolved if it is accepted; other values are disallowed
+     */
+    virtual UriResolveResult visitUser(User* user, const QString& action)
+    {
+        return IncorrectAction;
+    }
     /// Called by visitResource() when the passed URI identifies a room or
     /// an event in a room
     virtual void visitRoom(Room* room, const QString& eventId) {}
@@ -88,17 +95,16 @@ class UriResolverBase {
  */
 UriResolveResult
 visitResource(Connection* account, const Uri& uri,
-              std::function<void(User*, QString)> userHandler,
+              std::function<UriResolveResult(User*, QString)> userHandler,
               std::function<void(Room*, QString)> roomEventHandler,
               std::function<void(Connection*, QString, QStringList)> joinHandler,
               std::function<bool(const QUrl&)> nonMatrixHandler);
 
 /*! \brief Check that the resource is resolvable with no action on it */
-inline UriResolveResult checkResource(Connection* account,
-                                      const Uri& uri)
+inline UriResolveResult checkResource(Connection* account, const Uri& uri)
 {
     return visitResource(
-        account, uri, [](auto, auto) {}, [](auto, auto) {},
+        account, uri, [](auto, auto) { return UriResolved; }, [](auto, auto) {},
         [](auto, auto, auto) {}, [](auto) { return false; });
 }
 
@@ -150,7 +156,7 @@ class UriDispatcher : public QObject, public UriResolverBase {
     void nonMatrixAction(QUrl url);
 
 private:
-    void visitUser(User* user, const QString& action) override;
+    UriResolveResult visitUser(User* user, const QString& action) override;
     void visitRoom(Room* room, const QString& eventId) override;
     void joinRoom(Connection* account, const QString& roomAliasOrId,
                   const QStringList& viaServers = {}) override;

From 6d804f56e570c39ea9967c66c4bdad6f530e956e Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Tue, 21 Jul 2020 11:19:10 +0200
Subject: [PATCH 09/10] Uri: bare-sigil URIs are invalid

---
 lib/uri.cpp       | 24 +++++++++++++++++-------
 tests/quotest.cpp |  5 +++++
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/lib/uri.cpp b/lib/uri.cpp
index e81933dc0..f813794c9 100644
--- a/lib/uri.cpp
+++ b/lib/uri.cpp
@@ -7,12 +7,15 @@
 using namespace Quotient;
 
 struct ReplacePair { QByteArray uriString; char sigil; };
-static const auto replacePairs = { ReplacePair { "user/", '@' },
-                                   { "roomid/", '!' },
-                                   { "room/", '#' },
-                                   // The notation for bare event ids is not
-                                   // proposed in MSC2312 (and anywhere, as yet)
-                                   { "event/", '$' } };
+/// Defines bi-directional mapping of path prefixes and sigils
+static const auto replacePairs = {
+    ReplacePair { "user/", '@' },
+    { "roomid/", '!' },
+    { "room/", '#' },
+    // The notation for bare event ids is not proposed in MSC2312 but there's
+    // https://github.com/matrix-org/matrix-doc/pull/2644
+    { "event/", '$' }
+};
 
 Uri::Uri(QByteArray primaryId, QByteArray secondaryId, QString query)
 {
@@ -22,14 +25,21 @@ Uri::Uri(QByteArray primaryId, QByteArray secondaryId, QString query)
         setScheme("matrix");
         QString pathToBe;
         primaryType_ = Invalid;
+        if (primaryId.size() < 2) // There should be something after sigil
+            return;
         for (const auto& p: replacePairs)
             if (primaryId[0] == p.sigil) {
                 primaryType_ = Type(p.sigil);
                 pathToBe = p.uriString + primaryId.mid(1);
                 break;
             }
-        if (!secondaryId.isEmpty())
+        if (!secondaryId.isEmpty()) {
+            if (secondaryId.size() < 2) {
+                primaryType_ = Invalid;
+                return;
+            }
             pathToBe += "/event/" + secondaryId.mid(1);
+        }
         setPath(pathToBe);
     }
     setQuery(std::move(query));
diff --git a/tests/quotest.cpp b/tests/quotest.cpp
index c2c50df2b..4663d34a7 100644
--- a/tests/quotest.cpp
+++ b/tests/quotest.cpp
@@ -667,6 +667,7 @@ TEST_IMPL(visitResources)
     QUrl invalidUrl { "https://" };
     invalidUrl.setAuthority("---:@@@");
     const Uri emptyUri {}, uriFromEmptyUrl {},
+        bareSigil { QStringLiteral("#") },
         invalidMatrixUri { QStringLiteral("matrix:&invalid@") },
         matrixUriFromInvalidUrl { invalidUrl };
 
@@ -675,6 +676,10 @@ TEST_IMPL(visitResources)
             clog << "Empty Matrix URI test failed" << endl;
             FAIL_TEST();
         }
+    if (bareSigil.isValid()) {
+        clog << "Bare sigil URI test failed" << endl;
+        FAIL_TEST();
+    }
     if (matrixUriFromInvalidUrl.isEmpty() || matrixUriFromInvalidUrl.isValid()) {
         clog << "Invalid Matrix URI test failed" << endl;
         FAIL_TEST();

From bd74588539d8a5922e9f51eb691052c06c02a5ed Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Wed, 22 Jul 2020 18:38:55 +0200
Subject: [PATCH 10/10] quotest: Minor refactoring of basic URI tests

---
 tests/quotest.cpp | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/tests/quotest.cpp b/tests/quotest.cpp
index 4663d34a7..ae2726228 100644
--- a/tests/quotest.cpp
+++ b/tests/quotest.cpp
@@ -664,22 +664,19 @@ TEST_IMPL(visitResources)
     };
 
     // Basic tests
-    QUrl invalidUrl { "https://" };
-    invalidUrl.setAuthority("---:@@@");
-    const Uri emptyUri {}, uriFromEmptyUrl {},
-        bareSigil { QStringLiteral("#") },
-        invalidMatrixUri { QStringLiteral("matrix:&invalid@") },
-        matrixUriFromInvalidUrl { invalidUrl };
-
-    for (const auto& u: { emptyUri, uriFromEmptyUrl })
+    for (const auto& u: { Uri {}, Uri { QUrl {} } })
         if (u.isValid() || !u.isEmpty()) {
             clog << "Empty Matrix URI test failed" << endl;
             FAIL_TEST();
         }
-    if (bareSigil.isValid()) {
+    if (Uri { QStringLiteral("#") }.isValid()) {
         clog << "Bare sigil URI test failed" << endl;
         FAIL_TEST();
     }
+    QUrl invalidUrl { "https://" };
+    invalidUrl.setAuthority("---:@@@");
+    const Uri matrixUriFromInvalidUrl { invalidUrl },
+        invalidMatrixUri { QStringLiteral("matrix:&invalid@") };
     if (matrixUriFromInvalidUrl.isEmpty() || matrixUriFromInvalidUrl.isValid()) {
         clog << "Invalid Matrix URI test failed" << endl;
         FAIL_TEST();