There's always place for more fixes

Quotient/connection.cpp

@@ -69,7 +69,6 @@ Connection::Connection(const QUrl& server, QObject* parent)
     : QObject(parent)
     , d(makeImpl<Private>(std::make_unique<ConnectionData>(server)))
 {
-    //connect(qApp, &QCoreApplication::aboutToQuit, this, &Connection::saveOlmAccount);
     d->q = this; // All d initialization should occur before this line
     setObjectName(server.toString());
 }
@@ -168,11 +167,11 @@ void Connection::loginWithToken(const QString& loginToken,
                      QString() /*password*/, loginToken, deviceId, initialDeviceName);
 }
 
-void Connection::assumeIdentity(const QString& mxId, const QString& deviceId, const QString& accessToken)
+void Connection::assumeIdentity(const QString& mxId, const QString& deviceId,
+                                const QString& accessToken)
 {
-    d->completeSetup(mxId, deviceId);
-    d->ensureHomeserver(mxId).then([this, mxId, accessToken] {
-        d->data->setToken(accessToken.toLatin1());
+    d->completeSetup(mxId, false, deviceId, accessToken);
+    d->ensureHomeserver(mxId).then([this, mxId] {
         callApi<GetTokenOwnerJob>().onResult([this, mxId](const GetTokenOwnerJob* job) {
             switch (job->error()) {
             case BaseJob::Success:
@@ -280,7 +279,7 @@ void Connection::Private::dropAccessToken()
             << qUtf8Printable(job->errorString());
     });
 
-    data->setToken({});
+    data->setAccessToken({});
 }
 
 template <typename... LoginArgTs>
@@ -289,26 +288,26 @@ void Connection::Private::loginToServer(LoginArgTs&&... loginArgs)
     q->callApi<LoginJob>(std::forward<LoginArgTs>(loginArgs)...)
         .onResult([this](const LoginJob* loginJob) {
             if (loginJob->status().good()) {
-                data->setToken(loginJob->accessToken().toLatin1());
-                completeSetup(loginJob->userId(), loginJob->deviceId());
                 saveAccessTokenToKeychain();
-                if (encryptionData)
-                    encryptionData->database.clear();
+                completeSetup(loginJob->userId(), true, loginJob->deviceId(),
+                              loginJob->accessToken());
             } else
                 emit q->loginError(loginJob->errorString(), loginJob->rawDataSample());
         });
 }
 
-void Connection::Private::completeSetup(const QString& mxId, const QString& deviceId, bool mock)
+void Connection::Private::completeSetup(const QString& mxId, bool newLogin,
+                                        const std::optional<QString>& deviceId,
+                                        const std::optional<QString>& accessToken)
 {
-    data->setIdentity(mxId, deviceId);
+    data->setIdentity(mxId, deviceId.value_or(u""_s), accessToken.value_or(u""_s).toLatin1());
     q->setObjectName(data->userId() % u'/' % data->deviceId());
     qCDebug(MAIN) << "Using server" << data->baseUrl().toDisplayString()
                   << "by user" << data->userId()
                   << "from device" << data->deviceId();
     connect(qApp, &QCoreApplication::aboutToQuit, q, &Connection::saveState);
 
-    if (!mock) {
+    if (accessToken.has_value()) {
         q->loadVersions();
         q->loadCapabilities();
         q->user()->load(); // Load the local user's profile
@@ -318,21 +317,27 @@ void Connection::Private::completeSetup(const QString& mxId, const QString& devi
 
     if (useEncryption) {
         using _impl::ConnectionEncryptionData;
-        ConnectionEncryptionData::setup(q, mock, encryptionData).then([this](bool successful) {
-            if (!successful || !encryptionData)
-                useEncryption = false;
-
-            emit q->encryptionChanged(useEncryption);
-            emit q->stateChanged();
-            emit q->ready();
-            emit q->connected();
-        });
+        if (!accessToken) {
+            // Mock connection; initialise bare bones necessary for testing
+            qInfo(E2EE) << "Using a mock pickling key";
+            encryptionData = std::make_unique<ConnectionEncryptionData>(q, PicklingKey::generate());
+            encryptionData->database.clear();
+            encryptionData->olmAccount.setupNewAccount();
+        } else
+            ConnectionEncryptionData::setup(q, encryptionData, newLogin).then([this](bool successful) {
+                if (!successful || !encryptionData)
+                    useEncryption = false;
+
+                emit q->encryptionChanged(useEncryption);
+                emit q->stateChanged();
+                emit q->ready();
+                emit q->connected();
+            });
     } else {
         qCInfo(E2EE) << "End-to-end encryption (E2EE) support is off for" << q->objectName();
         emit q->ready();
         emit q->connected();
     }
-
 }
 
 QFuture<void> Connection::Private::ensureHomeserver(const QString& userId,
@@ -1898,12 +1903,11 @@ void Connection::reloadDevices()
     }
 }
 
-Connection* Connection::makeMockConnection(const QString& mxId,
-                                           bool enableEncryption)
+Connection* Connection::makeMockConnection(const QString& mxId, bool enableEncryption)
 {
     auto* c = new Connection;
     c->enableEncryption(enableEncryption);
-    c->d->completeSetup(mxId, {}, true);
+    c->d->completeSetup(mxId);
     return c;
 }
 

Quotient/connection.h

@@ -293,6 +293,7 @@ class QUOTIENT_API Connection : public QObject {
     //! Get the domain name used for ids/aliases on the server
     QString domain() const;
     //! Check if the homeserver is known to be reachable and working
+    [[deprecated("Check the result returned by Connection::loginFlows() instead")]]
     bool isUsable() const;
     //! Get the list of supported login flows
     QVector<GetLoginFlowsJob::LoginFlow> loginFlows() const;
@@ -495,8 +496,21 @@ class QUOTIENT_API Connection : public QObject {
     void setLazyLoading(bool newValue);
 
     //! Start a pre-created job object on this connection
-    Q_INVOKABLE BaseJob* run(BaseJob* job,
-                             RunningPolicy runningPolicy = ForegroundRequest);
+    Q_INVOKABLE BaseJob* run(BaseJob* job, RunningPolicy runningPolicy = ForegroundRequest);
+
+    //! \brief Start a pre-created job on this connection and get a job handle to it
+    //!
+    //! This is a template overload for run(BaseJob*, RunningPolicy) - if you call run() on any
+    //! derived job (99% of the cases when you're going to call it), this overload will be chosen
+    //! as a more type-safe and feature-rich version. It's not Q_INVOKABLE though.
+    template <std::derived_from<BaseJob> JobT>
+        requires (!std::same_as<JobT, BaseJob>)
+    JobHandle<JobT> run(JobT* job, RunningPolicy runningPolicy = ForegroundRequest)
+    {
+        JobHandle jh { job };
+        run(static_cast<BaseJob*>(job), runningPolicy);
+        return jh;
+    }
 
     //! \brief Start a job of a given type with specified arguments and policy
     //!
@@ -512,9 +526,7 @@ class QUOTIENT_API Connection : public QObject {
     template <typename JobT, typename... JobArgTs>
     JobHandle<JobT> callApi(RunningPolicy runningPolicy, JobArgTs&&... jobArgs)
     {
-        auto job = new JobT(std::forward<JobArgTs>(jobArgs)...);
-        run(job, runningPolicy);
-        return job;
+        return run(new JobT(std::forward<JobArgTs>(jobArgs)...), runningPolicy);
     }
 
     //! \brief Start a job of a specified type with specified arguments

Quotient/connection_p.h

@@ -93,7 +93,9 @@ class Q_DECL_HIDDEN Quotient::Connection::Private {
     QFuture<void> ensureHomeserver(const QString& userId, const std::optional<LoginFlow>& flow = {});
     template <typename... LoginArgTs>
     void loginToServer(LoginArgTs&&... loginArgs);
-    void completeSetup(const QString& mxId, const QString& deviceId, bool mock = false);
+    void completeSetup(const QString& mxId, bool newLogin = true,
+                       const std::optional<QString>& deviceId = {},
+                       const std::optional<QString>& accessToken = {});
     void removeRoom(const QString& roomId);
 
     void consumeRoomData(SyncDataList&& roomDataList, bool fromCache);

Quotient/connectiondata.cpp

@@ -104,7 +104,7 @@ QUrl ConnectionData::baseUrl() const { return d->baseUrl; }
 
 HomeserverData ConnectionData::homeserverData() const
 {
-    return { d->baseUrl, d->supportedSpecVersions, d->accessToken };
+    return { d->baseUrl, d->accessToken, d->supportedSpecVersions };
 }
 
 NetworkAccessManager* ConnectionData::nam() const
@@ -124,12 +124,14 @@ void ConnectionData::setBaseUrl(QUrl baseUrl)
     }
 }
 
-void ConnectionData::setToken(QByteArray token)
+void ConnectionData::setAccessToken(QByteArray token)
 {
     d->accessToken = std::move(token);
     NetworkAccessManager::setAccessToken(d->userId, d->accessToken);
 }
 
+void ConnectionData::setToken(QByteArray accessToken) { setAccessToken(std::move(accessToken)); }
+
 const QString& ConnectionData::deviceId() const { return d->deviceId; }
 
 const QString& ConnectionData::userId() const { return d->userId; }
@@ -138,18 +140,19 @@ void ConnectionData::setDeviceId(const QString& deviceId) { d->deviceId = device
 
 void ConnectionData::setUserId(const QString& userId) { setIdentity(userId, d->deviceId); }
 
-void ConnectionData::setIdentity(const QString& userId, const QString& deviceId)
+void ConnectionData::setIdentity(const QString& userId, const QString& deviceId,
+                                 QByteArray accessToken)
 {
     if (d->baseUrl.isValid()) {
         if (d->userId != userId)
             NetworkAccessManager::dropAccount(d->userId);
         if (!userId.isEmpty()) {
-            NetworkAccessManager::addAccount(userId, d->baseUrl);
-            NetworkAccessManager::setAccessToken(userId, d->accessToken);
+            NetworkAccessManager::addAccount(userId, d->baseUrl, accessToken);
         }
     }
     d->userId = userId;
     d->deviceId = deviceId;
+    d->accessToken = std::move(accessToken);
 }
 
 void ConnectionData::setSupportedSpecVersions(QStringList versions)

Quotient/connectiondata.h

@@ -32,12 +32,14 @@ class QUOTIENT_API ConnectionData {
     Quotient::NetworkAccessManager *nam() const;
 
     void setBaseUrl(QUrl baseUrl);
+    [[deprecated("Use setAccessToken() or setIdentity() instead")]]
     void setToken(QByteArray accessToken);
     [[deprecated("Use setIdentity() instead")]]
     void setDeviceId(const QString& deviceId);
     [[deprecated("Use setIdentity() instead")]]
     void setUserId(const QString& userId);
-    void setIdentity(const QString& userId, const QString& deviceId);
+    void setIdentity(const QString& userId, const QString& deviceId, QByteArray accessToken = {});
+    void setAccessToken(QByteArray accessToken);
     void setSupportedSpecVersions(QStringList versions);
 
     QString lastEvent() const;

Quotient/connectionencryptiondata_p.cpp

@@ -34,16 +34,9 @@ inline QFuture<QKeychain::Job*> runKeychainJob(QKeychain::Job* j, const QString&
     return ft;
 }
 
-QFuture<void> setupPicklingKey(Connection* connection, bool mock,
+QFuture<void> setupPicklingKey(Connection* connection,
                                std::unique_ptr<ConnectionEncryptionData>& encryptionData)
 {
-    if (mock) {
-        qInfo(E2EE) << "Using a mock pickling key";
-        encryptionData =
-            std::make_unique<ConnectionEncryptionData>(connection, PicklingKey::generate());
-        return QtFuture::makeReadyFuture<void>();
-    }
-
     using namespace QKeychain;
     const auto keychainId = connection->userId() + "-Pickle"_L1;
     qCInfo(MAIN) << "Keychain request: app" << qAppName() << "id" << keychainId;
@@ -94,15 +87,15 @@ QFuture<void> setupPicklingKey(Connection* connection, bool mock,
         });
 }
 
-QFuture<bool> ConnectionEncryptionData::setup(Connection* connection, bool mock,
-                                              std::unique_ptr<ConnectionEncryptionData>& result)
+QFuture<bool> ConnectionEncryptionData::setup(Connection* connection,
+                                              std::unique_ptr<ConnectionEncryptionData>& result,
+                                              bool clearDatabase)
 {
-    return setupPicklingKey(connection, mock, result)
-        .then([connection, mock, &result] {
-            if (mock) {
+    return setupPicklingKey(connection, result)
+        .then([connection, &result, clearDatabase] {
+            if (clearDatabase) {
+                qCInfo(E2EE) << "Clearing the database for account" << connection->objectName();
                 result->database.clear();
-                result->olmAccount.setupNewAccount();
-                return true;
             }
             if (const auto outcome = result->database.setupOlmAccount(result->olmAccount)) {
                 if (outcome == OLM_SUCCESS) {

Quotient/connectionencryptiondata_p.h

@@ -16,8 +16,9 @@ struct DevicesList;
 namespace _impl {
     class ConnectionEncryptionData {
     public:
-        static QFuture<bool> setup(Connection* connection, bool mock,
-                                   std::unique_ptr<ConnectionEncryptionData>& result);
+        static QFuture<bool> setup(Connection* connection,
+                                   std::unique_ptr<ConnectionEncryptionData>& result,
+                                   bool clearDatabase = false);
 
         Connection* q;
         QOlmAccount olmAccount;

Quotient/networkaccessmanager.cpp

@@ -27,20 +27,19 @@ class {
         HomeserverData hsData;
     };
 
-    void addConnection(QString accountId, QUrl baseUrl)
+    void addConnection(const QString& accountId, HomeserverData hsData)
     {
-        if (baseUrl.isEmpty())
+        if (hsData.baseUrl.isEmpty())
             return;
 
         const QWriteLocker _(&namLock);
         if (auto it = std::ranges::find(connectionData, accountId, &ConnectionData::accountId);
-            it != connectionData.end()) {
-            it->hsData.baseUrl = std::move(baseUrl);
-        } else
-            connectionData.push_back(
-                { std::move(accountId), HomeserverData{ std::move(baseUrl), {}, {}} });
+            it != connectionData.end())
+            it->hsData = std::move(hsData);
+        else // Xcode doesn't like emplace_back() below for some reason (anon class?..)
+            connectionData.push_back({ accountId, std::move(hsData) });
     }
-    void addSpecVersions(QStringView accountId, QStringList versions)
+    void addSpecVersions(QStringView accountId, const QStringList& versions)
     {
         if (versions.isEmpty())
             return;
@@ -51,7 +50,7 @@ class {
                                                     "versions on an inexistent account"))
             return;
 
-        it->hsData.supportedSpecVersions = std::move(versions);
+        it->hsData.supportedSpecVersions = versions;
     }
     void dropConnection(QStringView accountId)
     {
@@ -63,7 +62,7 @@ class {
     {
         const QReadLocker _(&namLock);
         auto it = std::ranges::find(connectionData, accountId, &ConnectionData::accountId);
-        return it == connectionData.cend() ? HomeserverData{ } : it->hsData;
+        return it == connectionData.cend() ? HomeserverData{} : it->hsData;
     }
     void addIgnoredSslError(const QSslError& error)
     {
@@ -97,16 +96,23 @@ class {
 
 } // anonymous namespace
 
-void NetworkAccessManager::addAccount(QString accountId, QUrl homeserver)
+void NetworkAccessManager::addAccount(const QString& accountId, const QUrl& homeserver,
+                                      const QByteArray& accessToken)
 {
     Q_ASSERT(!accountId.isEmpty());
-    d.addConnection( accountId, std::move(homeserver) );
+    d.addConnection(accountId, { homeserver, accessToken });
+}
+
+void NetworkAccessManager::setAccessToken(const QString& userId, const QByteArray& token)
+{
+    d.setAccessToken(userId, token);
 }
 
-void NetworkAccessManager::updateAccountSpecVersions(QStringView accountId, QStringList versions)
+void NetworkAccessManager::updateAccountSpecVersions(QStringView accountId,
+                                                     const QStringList& versions)
 {
     Q_ASSERT(!accountId.isEmpty());
-    d.addSpecVersions(accountId, std::move(versions));
+    d.addSpecVersions(accountId, versions);
 }
 
 void NetworkAccessManager::dropAccount(QStringView accountId)
@@ -192,8 +198,3 @@ QStringList NetworkAccessManager::supportedSchemesImplementation() const
 {
     return QNetworkAccessManager::supportedSchemesImplementation() << u"mxc"_s;
 }
-
-void NetworkAccessManager::setAccessToken(const QString& userId, const QByteArray& token)
-{
-    d.setAccessToken(userId, token);
-}

Quotient/networkaccessmanager.h

@@ -14,8 +14,9 @@ class QUOTIENT_API NetworkAccessManager : public QNetworkAccessManager {
 public:
     using QNetworkAccessManager::QNetworkAccessManager;
 
-    static void addAccount(QString accountId, QUrl homeserver);
-    static void updateAccountSpecVersions(QStringView accountId, QStringList versions);
+    static void addAccount(const QString& accountId, const QUrl& homeserver,
+                           const QByteArray& accessToken = {});
+    static void updateAccountSpecVersions(QStringView accountId, const QStringList &versions);
     static void dropAccount(QStringView accountId);
 
     static QList<QSslError> ignoredSslErrors();