Make DolphinPackageInstaller a KJob

This means developers can use a familiar pattern here.

src/CMakeLists.txt

@@ -274,7 +274,7 @@ target_sources(dolphinstatic PRIVATE
     dolphincontextmenu.cpp
     dolphinnavigatorswidgetaction.cpp
     dolphintabbar.cpp
-    dolphinpackagemanager.cpp
+    dolphinpackageinstaller.cpp
     dolphinplacesmodelsingleton.cpp
     dolphinrecenttabsmenu.cpp
     dolphintabpage.cpp
@@ -335,7 +335,7 @@ target_sources(dolphinstatic PRIVATE
     dolphincontextmenu.h
     dolphinnavigatorswidgetaction.h
     dolphintabbar.h
-    dolphinpackagemanager.h
+    dolphinpackageinstaller.h
     dolphinplacesmodelsingleton.h
     dolphinrecenttabsmenu.h
     dolphintabpage.h

src/dolphinpackageinstaller.cpp

@@ -5,7 +5,7 @@
     SPDX-License-Identifier: LGPL-2.1-only OR LGPL-3.0-only OR LicenseRef-KDE-Accepted-LGPL
 */
 
-#include "dolphinpackagemanager.h"
+#include "dolphinpackageinstaller.h"
 
 #include <KLocalizedString>
 
@@ -18,51 +18,54 @@
 #include <QTimer>
 #include <QtAssert>
 
-DolphinPackageManager::DolphinPackageManager(QObject *parent)
+DolphinPackageInstaller::DolphinPackageInstaller(const QString &packageName,
-    : QObject(parent)
+                                                 const QUrl &fallBackInstallationPageUrl,
+                                                 std::function<bool()> isPackageInstalledCheck,
+                                                 QObject *parent)
+    : KJob(parent)
+    , m_packageName{packageName}
+    , m_fallBackInstallationPageUrl{fallBackInstallationPageUrl}
+    , m_isPackageInstalledCheck{isPackageInstalledCheck}
 {
 }
 
-void DolphinPackageManager::install(const QString &packageName, const QUrl &fallBackInstallationPageUrl, std::function<bool()> isPackageInstalledCheck)
+void DolphinPackageInstaller::start()
 {
-    Q_ASSERT_X(m_packageName.isEmpty(), "install", "Reusing a DolphinPackageManager object has not been implemented and can lead to conflicts.");
+    if (m_isPackageInstalledCheck()) {
-    if (isPackageInstalledCheck()) {
+        emitResult();
-        Q_EMIT success();
         return;
     }
 
-    m_packageName = packageName;
 #if HAVE_PACKAGEKIT
-    Q_UNUSED(fallBackInstallationPageUrl)
     PackageKit::Daemon::setHints(PackageKit::Daemon::hints() + QStringList{QStringLiteral("interactive=true")});
-    const PackageKit::Transaction *resolveTransaction = PackageKit::Daemon::resolve(packageName);
+    const PackageKit::Transaction *resolveTransaction = PackageKit::Daemon::resolve(m_packageName);
 
-    connect(resolveTransaction, &PackageKit::Transaction::errorCode, this, &DolphinPackageManager::slotInstallationFailed);
+    connect(resolveTransaction, &PackageKit::Transaction::errorCode, this, &DolphinPackageInstaller::slotInstallationFailed);
-    connect(resolveTransaction, &PackageKit::Transaction::finished, this, [this, packageName]() { // Will be disconnected if we find a package.
+    connect(resolveTransaction, &PackageKit::Transaction::finished, this, [this]() { // Will be disconnected if we find a package.
         slotInstallationFailed(PackageKit::Transaction::ErrorPackageNotFound,
-                               i18nc("@info:shell about system packages", "Could not find package %1.", packageName));
+                               i18nc("@info:shell about system packages", "Could not find package %1.", m_packageName));
     });
     connect(resolveTransaction,
             &PackageKit::Transaction::package,
             this,
             [this, resolveTransaction](PackageKit::Transaction::Info /* info */, const QString &packageId) {
                 disconnect(resolveTransaction, nullptr, this, nullptr); // We only care about the first package.
-                PackageKit::Transaction *installTransaction = PackageKit::Daemon::installPackage(packageId);
+                const PackageKit::Transaction *installTransaction = PackageKit::Daemon::installPackage(packageId);
+                connectTransactionToJobProgress(*installTransaction);
                 connect(installTransaction,
                         &PackageKit::Transaction::errorCode,
                         this,
                         [installTransaction, this](PackageKit::Transaction::Error error, const QString &details) {
-                            disconnect(installTransaction, nullptr, this, nullptr); // We only want to emit failure() or success() once.
+                            disconnect(installTransaction, nullptr, this, nullptr); // We only want to emit a result once.
                             slotInstallationFailed(error, details);
                         });
                 connect(installTransaction,
                         &PackageKit::Transaction::finished,
                         this,
                         [installTransaction, this](const PackageKit::Transaction::Exit status, uint /* runtime */) {
-                            disconnect(installTransaction, nullptr, this, nullptr); // We only want to emit failure() or success() once.
+                            disconnect(installTransaction, nullptr, this, nullptr); // We only want to emit a result once.
                             if (status == PackageKit::Transaction::ExitSuccess) {
-                                Q_EMIT success();
+                                emitResult();
-                                deleteLater();
                             } else {
                                 slotInstallationFailed(PackageKit::Transaction::ErrorUnknown,
                                                        i18nc("@info %1 is error code",
@@ -72,13 +75,11 @@ void DolphinPackageManager::install(const QString &packageName, const QUrl &fall
                         });
             });
 #else
-    Q_UNUSED(packageName)
+    QDesktopServices::openUrl(m_fallBackInstallationPageUrl);
-    QDesktopServices::openUrl(fallBackInstallationPageUrl);
     auto waitForSuccess = new QTimer(this);
-    connect(waitForSuccess, &QTimer::timeout, this, [isPackageInstalledCheck, this, waitForSuccess]() {
+    connect(waitForSuccess, &QTimer::timeout, this, [this]() {
-        if (isPackageInstalledCheck()) {
+        if (m_isPackageInstalledCheck()) {
-            Q_EMIT success();
+            emitResult();
-            deleteLater();
         }
     });
     waitForSuccess->start(3000);
@@ -86,13 +87,24 @@ void DolphinPackageManager::install(const QString &packageName, const QUrl &fall
 }
 
 #if HAVE_PACKAGEKIT
-void DolphinPackageManager::slotInstallationFailed(PackageKit::Transaction::Error error, const QString &details)
+void DolphinPackageInstaller::connectTransactionToJobProgress(const PackageKit::Transaction &transaction)
 {
-    Q_EMIT failure(xi18nc("@info:shell %1 is package name, %2 is error message, %3 is error e.g. 'ErrorNoNetwork'",
+    connect(&transaction, &PackageKit::Transaction::speedChanged, this, [this, &transaction]() {
+        emitSpeed(transaction.speed());
+    });
+    connect(&transaction, &PackageKit::Transaction::percentageChanged, this, [this, &transaction]() {
+        setPercent(transaction.percentage());
+    });
+}
+
+void DolphinPackageInstaller::slotInstallationFailed(PackageKit::Transaction::Error error, const QString &details)
+{
+    setErrorString(xi18nc("@info:shell %1 is package name, %2 is error message, %3 is error e.g. 'ErrorNoNetwork'",
                           "Installing <application>%1</application> failed: %2 (%3)<nl/>Please try installing <application>%1</application> manually instead.",
                           m_packageName,
                           details,
                           QMetaEnum::fromType<PackageKit::Transaction::Error>().valueToKey(error)));
-    deleteLater();
+    setError(error);
+    emitResult();
 }
 #endif

src/dolphinpackageinstaller.h

@@ -0,0 +1,85 @@
+/*
+    This file is part of the KDE project
+    SPDX-FileCopyrightText: 2024 Felix Ernst <felixernst@kde.org>
+
+    SPDX-License-Identifier: LGPL-2.1-only OR LGPL-3.0-only OR LicenseRef-KDE-Accepted-LGPL
+*/
+
+#ifndef dolphinpackageinstaller_H
+#define dolphinpackageinstaller_H
+
+#include "config-dolphin.h"
+
+#if HAVE_PACKAGEKIT
+#include <PackageKit/Transaction>
+#endif
+#include <KJob>
+
+#include <QUrl>
+
+/**
+ * @brief A KJob providing simple means to install a package.
+ */
+class DolphinPackageInstaller : public KJob
+{
+public:
+    /**
+     * @brief Installs a system package.
+     *
+     * @param packageName                   A name that can be resolved to a package.
+     * @param fallBackInstallationPageUrl   This url will be opened if Dolphin was installed without the PackageKit library. A good choice for this parameter
+     *                                      is an appstream url that will be opened in a software store like Discover
+     *                                      e.g. "appstream://org.kde.filelight.desktop". The user is then expected to install the package themselves and
+     *                                      KJob::result() will be emitted when it is detected that the installation finished successfully.
+     * @param isPackageInstalledCheck       A function that can be regularly checked to determine if the installation was already successful.
+     */
+    explicit DolphinPackageInstaller(const QString &packageName,
+                                     const QUrl &fallBackInstallationPageUrl,
+                                     std::function<bool()> isPackageInstalledCheck,
+                                     QObject *parent = nullptr);
+
+    /**
+     * @see KJob::start().
+     * Make sure to connect to the KJob::result() signal and show the KJob::errorString() to users there before calling this.
+     */
+    void start() override;
+
+    /** @see KJob::errorString(). */
+    inline QString errorString() const override
+    {
+        return m_errorString;
+    };
+
+private:
+    /** @see KJob::errorString(). */
+    inline void setErrorString(const QString &errorString)
+    {
+        m_errorString = errorString;
+    };
+
+#if HAVE_PACKAGEKIT
+    /**
+     * Makes sure progress signals of @p transaction are forwarded to KJob's progress signals.
+     */
+    void connectTransactionToJobProgress(const PackageKit::Transaction &transaction);
+
+private Q_SLOTS:
+    /** Creates a nice user-facing error message from its parameters and then finishes this job with an @p error. */
+    void slotInstallationFailed(PackageKit::Transaction::Error error, const QString &details);
+#endif
+
+private:
+    /** The name of the package that is supposed to be installed. */
+    const QString m_packageName;
+
+    /** @see DolphinPackageInstaller::DolphinPackageInstaller(). */
+    const QUrl m_fallBackInstallationPageUrl;
+
+    /** @see DolphinPackageInstaller::DolphinPackageInstaller(). */
+    const std::function<bool()> m_isPackageInstalledCheck;
+
+    /** @see KJob::errorString(). */
+    QString m_errorString;
+};
+
+#endif // dolphinpackageinstaller_H

src/dolphinpackagemanager.h

@@ -1,71 +0,0 @@
-/*
-    This file is part of the KDE project
-    SPDX-FileCopyrightText: 2024 Felix Ernst <felixernst@kde.org>
-
-    SPDX-License-Identifier: LGPL-2.1-only OR LGPL-3.0-only OR LicenseRef-KDE-Accepted-LGPL
-*/
-
-#ifndef DOLPHINPACKAGEMANAGER_H
-#define DOLPHINPACKAGEMANAGER_H
-
-#include "config-dolphin.h"
-
-#if HAVE_PACKAGEKIT
-#include <PackageKit/Transaction>
-#endif
-
-#include <QObject>
-
-/**
- * @brief A class providing a simple API to install packages.
- */
-class DolphinPackageManager : public QObject
-{
-    Q_OBJECT
-
-public:
-    explicit DolphinPackageManager(QObject *parent = nullptr);
-
-    /**
-     * @brief Installs a system package.
-     *
-     * @param packageName                   A name that can be resolved to a package.
-     * @param fallBackInstallationPageUrl   This url will be opened if Dolphin was installed without the PackageKit library. A good choice for this parameter
-     *                                      is an appstream url that will be opened in a software store (like Discover) e.g. "appstream://org.kde.kio.admin".
-     *                                      The user is then expected to install the package themselves and success() will be emitted once that finished.
-     * @param isPackageInstalledCheck       A function that can be regularly checked to determine if the installation was already successful.
-     *
-     * Calling this method will lead to either emitting the success() or the failure() signal. Once either is emitted this object will delete itself.
-     * This generally happens asynchronously with rare exceptions, so connect to the signals before calling this method.
-     */
-    void install(const QString &packageName, const QUrl &fallBackInstallationPageUrl, std::function<bool()> isPackageInstalledCheck);
-
-Q_SIGNALS:
-    /**
-     * Emitted when this object assumes that the package was successfully installed.
-     * This object will delete itself after emitting this signal.
-     *
-     * @note This does not always mean that the isPackageInstalledCheck of DolphinPackageManager::install() also passes.
-     */
-    void success();
-
-    /**
-     * Emitted when the installation finally failed.
-     * This object will delete itself after emitting this signal.
-     *
-     * Be sure to show the @p userFacingErrorMessage to the user or they won't know what happened and can't write useful bug reports.
-     */
-    void failure(const QString &userFacingErrorMessage);
-
-#if HAVE_PACKAGEKIT
-private Q_SLOTS:
-    /** Creates a nice user-facing error message from its parameters and then emits failure() and deletes this object. */
-    void slotInstallationFailed(PackageKit::Transaction::Error error, const QString &details);
-#endif
-
-private:
-    /** The name of the package that is currently being installed. */
-    QString m_packageName;
-};
-
-#endif // DOLPHINPACKAGEMANAGER_H

src/statusbar/statusbarspaceinfo.cpp

@@ -7,7 +7,7 @@
 #include "statusbarspaceinfo.h"
 
 #include "config-dolphin.h"
-#include "dolphinpackagemanager.h"
+#include "dolphinpackageinstaller.h"
 #include "global.h"
 #include "spaceinfoobserver.h"
 
@@ -154,7 +154,7 @@ void StatusBarSpaceInfo::updateMenu()
             vLayout->addSpacing(Dolphin::VERTICAL_SPACER_HEIGHT);
 
             auto installFilelightButton =
-                new QPushButton(QIcon::fromTheme(QStringLiteral("filelight")), i18nc("@action:button", "Install Filelight"), containerWidget);
+                new QPushButton(QIcon::fromTheme(QStringLiteral("filelight")), i18nc("@action:button", "Install Filelight…"), containerWidget);
             installFilelightButton->setFixedWidth(std::max(installFilelightButton->sizeHint().width(), installFilelightTitle->sizeHint().width()));
             auto buttonLayout = new QHBoxLayout{containerWidget};
             buttonLayout->addWidget(installFilelightButton, 0, Qt::AlignHCenter);
@@ -170,15 +170,21 @@ void StatusBarSpaceInfo::updateMenu()
 #ifdef Q_OS_WIN
             QDesktopServices::openUrl(QUrl("https://apps.kde.org/filelight"));
 #else
-            auto packageInstaller = new DolphinPackageManager(this);
+                    auto packageInstaller = new DolphinPackageInstaller(
-            connect(packageInstaller, &DolphinPackageManager::failure, this, [this, packageInstaller](const QString &userFacingErrorMessage) {
+                        FILELIGHT_PACKAGE_NAME,
-                Q_EMIT showMessage(userFacingErrorMessage, KMessageWidget::Error);
+                        QUrl("appstream://org.kde.filelight.desktop"),
-            });
+                        []() {
-            connect(packageInstaller, &DolphinPackageManager::success, this, [this, packageInstaller]() {
+                            return KService::serviceByDesktopName(QStringLiteral("org.kde.filelight"));
-                Q_EMIT showMessage(xi18nc("@info", "<application>Filelight</application> installed successfully."), KMessageWidget::Positive);
+                        },
-            });
+                        this);
-            packageInstaller->install(FILELIGHT_PACKAGE_NAME, QUrl("appstream://org.kde.filelight.desktop"), [](){ return KService::serviceByDesktopName(QStringLiteral("org.kde.filelight"));
+                    connect(packageInstaller, &KJob::result, this, [this](KJob *job) {
-            });
+                        if (job->error()) {
+                            Q_EMIT showMessage(job->errorString(), KMessageWidget::Error);
+                        } else {
+                            Q_EMIT showMessage(xi18nc("@info", "<application>Filelight</application> installed successfully."), KMessageWidget::Positive);
+                        }
+                    });
+                    packageInstaller->start();
 #endif
             });