Commit fd01fb97 authored by Aleix Pol Gonzalez's avatar Aleix Pol Gonzalez 🐧

Change how we deal with Transactions

They get removed from the model through their status, they get added by
the class who requested them. Will allow for simpler code and more
semantics for the Transactions.
parent 1b2f32fd
......@@ -34,10 +34,12 @@ Kirigami.BasicListItem {
progressModel.append({app: trans.resource})
}
onTransactionCancelled: {
var id = progressModel.appAt(trans.resource)
if(id>=0)
progressModel.remove(id)
onTransactionRemoved: {
if (trans.status == Transaction.CancelledStatus) {
var id = progressModel.appAt(trans.resource)
if(id>=0)
progressModel.remove(id)
}
}
}
......
......@@ -35,6 +35,11 @@ Transaction::Transaction(QObject *parent, AbstractResource *resource,
{
}
Transaction::~Transaction()
{
Q_ASSERT(!TransactionModel::global()->contains(this));
}
AbstractResource *Transaction::resource() const
{
return m_resource;
......@@ -71,8 +76,10 @@ void Transaction::setStatus(Status status)
m_status = status;
emit statusChanged(m_status);
if (m_status == DoneStatus) {
if (m_status == DoneStatus || m_status == CancelledStatus || m_status == DoneWithErrorStatus) {
setCancellable(false);
TransactionModel::global()->removeTransaction(this);
}
}
}
......
......@@ -62,7 +62,9 @@ public:
/// Transaction is done
DoneStatus,
/// Transaction is done, but there was an error during transaction
DoneWithErrorStatus
DoneWithErrorStatus,
/// Transaction was cancelled
CancelledStatus
};
Q_ENUM(Status)
......@@ -79,6 +81,8 @@ public:
Transaction(QObject *parent, AbstractResource *resource,
Transaction::Role role, const AddonList &addons = {});
~Transaction() override;
/**
* @returns the AbstractResource which this transaction works with
*/
......
......@@ -31,8 +31,6 @@ TransactionListener::TransactionListener(QObject *parent)
, m_transaction(nullptr)
{
connect(TransactionModel::global(), &TransactionModel::transactionAdded, this, &TransactionListener::transactionAdded);
connect(TransactionModel::global(), &TransactionModel::transactionRemoved, this, &TransactionListener::transactionRemoved);
connect(TransactionModel::global(), &TransactionModel::transactionCancelled, this, &TransactionListener::transactionCancelled);
}
void TransactionListener::cancel()
......@@ -139,6 +137,10 @@ void TransactionListener::setTransaction(Transaction* trans)
void TransactionListener::transactionStatusChanged(Transaction::Status status)
{
switch (status) {
case Transaction::CancelledStatus:
setTransaction(nullptr);
emit cancelled();
break;
case Transaction::DoneStatus:
setTransaction(nullptr);
break;
......@@ -149,21 +151,6 @@ void TransactionListener::transactionStatusChanged(Transaction::Status status)
emit statusTextChanged();
}
void TransactionListener::transactionRemoved(Transaction* trans)
{
if(m_transaction == trans) {
setTransaction(nullptr);
}
}
void TransactionListener::transactionCancelled(Transaction* trans)
{
if(m_transaction == trans) {
setTransaction(nullptr);
}
emit cancelled();
}
int TransactionListener::progress() const
{
return m_transaction ? m_transaction->progress() : 0;
......
......@@ -101,6 +101,8 @@ QVariant TransactionModel::data(const QModelIndex &index, int role) const
break;
case Transaction::DoneStatus:
return i18nc("@info:status", "Done");
case Transaction::CancelledStatus:
return i18nc("@info:status", "Cancelled");
}
break;
case TransactionRole:
......@@ -162,13 +164,6 @@ void TransactionModel::addTransaction(Transaction *trans)
emit transactionAdded(trans);
}
void TransactionModel::cancelTransaction(Transaction *trans)
{
removeTransaction(trans);
emit transactionCancelled(trans);
}
void TransactionModel::removeTransaction(Transaction *trans)
{
Q_ASSERT(trans);
......
......@@ -56,7 +56,6 @@ public:
QModelIndex indexOf(AbstractResource *res) const;
void addTransaction(Transaction *trans);
void cancelTransaction(Transaction *trans);
void removeTransaction(Transaction *trans);
bool contains(Transaction* transaction) const { return m_transactions.contains(transaction); }
......@@ -69,7 +68,6 @@ Q_SIGNALS:
void startingFirstTransaction();
void lastTransactionFinished();
void transactionAdded(Transaction *trans);
void transactionCancelled(Transaction *trans);
void transactionRemoved(Transaction* trans);
void countChanged();
void progressChanged();
......
......@@ -26,7 +26,6 @@
#include <resources/StandardBackendUpdater.h>
#include <resources/SourcesModel.h>
#include <Transaction/Transaction.h>
#include <Transaction/TransactionModel.h>
#include <KAboutData>
#include <KLocalizedString>
......@@ -144,22 +143,19 @@ AbstractReviewsBackend* DummyBackend::reviewsBackend() const
return m_reviews;
}
void DummyBackend::installApplication(AbstractResource* app, const AddonList& addons)
Transaction* DummyBackend::installApplication(AbstractResource* app, const AddonList& addons)
{
TransactionModel *transModel = TransactionModel::global();
transModel->addTransaction(new DummyTransaction(qobject_cast<DummyResource*>(app), addons, Transaction::InstallRole));
return new DummyTransaction(qobject_cast<DummyResource*>(app), addons, Transaction::InstallRole);
}
void DummyBackend::installApplication(AbstractResource* app)
Transaction* DummyBackend::installApplication(AbstractResource* app)
{
TransactionModel *transModel = TransactionModel::global();
transModel->addTransaction(new DummyTransaction(qobject_cast<DummyResource*>(app), Transaction::InstallRole));
return new DummyTransaction(qobject_cast<DummyResource*>(app), Transaction::InstallRole);
}
void DummyBackend::removeApplication(AbstractResource* app)
Transaction* DummyBackend::removeApplication(AbstractResource* app)
{
TransactionModel *transModel = TransactionModel::global();
transModel->addTransaction(new DummyTransaction(qobject_cast<DummyResource*>(app), Transaction::RemoveRole));
return new DummyTransaction(qobject_cast<DummyResource*>(app), Transaction::RemoveRole);
}
void DummyBackend::checkForUpdates()
......
......@@ -44,9 +44,9 @@ public:
bool isValid() const override { return true; } // No external file dependencies that could cause runtime errors
QList<QAction*> messageActions() const override { return m_messageActions; }
void installApplication(AbstractResource* app) override;
void installApplication(AbstractResource* app, const AddonList& addons) override;
void removeApplication(AbstractResource* app) override;
Transaction* installApplication(AbstractResource* app) override;
Transaction* installApplication(AbstractResource* app, const AddonList& addons) override;
Transaction* removeApplication(AbstractResource* app) override;
bool isFetching() const override { return m_fetching; }
AbstractResource * resourceForFile(const QUrl & ) override;
void checkForUpdates() override;
......
......@@ -21,7 +21,6 @@
#include "DummyTransaction.h"
#include "DummyBackend.h"
#include "DummyResource.h"
#include <Transaction/TransactionModel.h>
#include <QTimer>
#include <QDebug>
#include <KRandom>
......@@ -55,7 +54,8 @@ void DummyTransaction::iterateTransaction()
void DummyTransaction::cancel()
{
m_iterate = false;
TransactionModel::global()->cancelTransaction(this);
setStatus(CancelledStatus);
}
void DummyTransaction::finishTransaction()
......@@ -71,8 +71,7 @@ void DummyTransaction::finishTransaction()
newState = AbstractResource::None;
break;
}
m_app->setState(newState);
m_app->setAddons(addons());
TransactionModel::global()->removeTransaction(this);
m_app->setState(newState);
deleteLater();
}
......@@ -25,7 +25,6 @@
#include <resources/ResourcesProxyModel.h>
#include <resources/AbstractBackendUpdater.h>
#include <ApplicationAddonsModel.h>
#include <Transaction/TransactionModel.h>
#include <ReviewsBackend/ReviewsModel.h>
#include <UpdateModel/UpdateModel.h>
#include <resources/ResourcesUpdatesModel.h>
......
......@@ -29,7 +29,6 @@
#include <resources/StandardBackendUpdater.h>
#include <resources/SourcesModel.h>
#include <Transaction/Transaction.h>
#include <Transaction/TransactionModel.h>
#include <appstream/OdrsReviewsBackend.h>
#include <appstream/AppStreamIntegration.h>
......@@ -955,7 +954,7 @@ AbstractReviewsBackend * FlatpakBackend::reviewsBackend() const
return m_reviews.data();
}
void FlatpakBackend::installApplication(AbstractResource *app, const AddonList &addons)
Transaction* FlatpakBackend::installApplication(AbstractResource *app, const AddonList &addons)
{
Q_UNUSED(addons);
......@@ -968,7 +967,7 @@ void FlatpakBackend::installApplication(AbstractResource *app, const AddonList &
resource->setState(AbstractResource::Installed);
integrateRemote(preferredInstallation(), remote);
}
return;
return nullptr;
}
FlatpakTransaction *transaction = nullptr;
......@@ -1003,14 +1002,15 @@ void FlatpakBackend::installApplication(AbstractResource *app, const AddonList &
updateAppState(installation, resource);
}
});
return transaction;
}
void FlatpakBackend::installApplication(AbstractResource *app)
Transaction* FlatpakBackend::installApplication(AbstractResource *app)
{
installApplication(app, {});
return installApplication(app, {});
}
void FlatpakBackend::removeApplication(AbstractResource *app)
Transaction* FlatpakBackend::removeApplication(AbstractResource *app)
{
FlatpakResource *resource = qobject_cast<FlatpakResource*>(app);
......@@ -1019,7 +1019,7 @@ void FlatpakBackend::removeApplication(AbstractResource *app)
if (m_sources->removeSource(resource->flatpakName())) {
resource->setState(AbstractResource::None);
}
return;
return nullptr;
}
FlatpakInstallation *installation = resource->installation();
......@@ -1030,6 +1030,7 @@ void FlatpakBackend::removeApplication(AbstractResource *app)
updateAppSize(installation, resource);
}
});
return transaction;
}
void FlatpakBackend::checkForUpdates()
......
......@@ -54,9 +54,9 @@ public:
bool isValid() const override;
QList<QAction*> messageActions() const override { return {}; }
void installApplication(AbstractResource* app) override;
void installApplication(AbstractResource* app, const AddonList& addons) override;
void removeApplication(AbstractResource* app) override;
Transaction* installApplication(AbstractResource* app) override;
Transaction* installApplication(AbstractResource* app, const AddonList& addons) override;
Transaction* removeApplication(AbstractResource* app) override;
bool isFetching() const override { return m_fetching; }
AbstractResource * resourceForFile(const QUrl & ) override;
void checkForUpdates() override;
......
......@@ -24,8 +24,6 @@
#include "FlatpakResource.h"
#include "FlatpakTransactionJob.h"
#include <Transaction/TransactionModel.h>
#include <QDebug>
#include <QTimer>
......@@ -44,8 +42,6 @@ FlatpakTransaction::FlatpakTransaction(FlatpakInstallation* installation, Flatpa
{
setCancellable(true);
TransactionModel::global()->addTransaction(this);
if (!delayStart) {
QTimer::singleShot(0, this, &FlatpakTransaction::start);
}
......@@ -62,7 +58,7 @@ void FlatpakTransaction::cancel()
if (m_runtime) {
m_runtimeJob->cancel();
}
TransactionModel::global()->cancelTransaction(this);
setStatus(CancelledStatus);
}
void FlatpakTransaction::setRuntime(FlatpakResource *runtime)
......@@ -164,6 +160,4 @@ void FlatpakTransaction::finishTransaction()
} else {
setStatus(DoneWithErrorStatus);
}
TransactionModel::global()->removeTransaction(this);
}
......@@ -40,7 +40,6 @@
// DiscoverCommon includes
#include "Transaction/Transaction.h"
#include "Transaction/TransactionModel.h"
#include "Category/Category.h"
// Own includes
......@@ -233,8 +232,6 @@ public:
: Transaction(parent, res, role)
, m_id(res->entry().uniqueId())
{
TransactionModel::global()->addTransaction(this);
setCancellable(false);
auto manager = res->knsBackend()->engine();
......@@ -257,43 +254,36 @@ public:
case KNS3::Entry::Updateable:
if (status() != DoneStatus) {
setStatus(DoneStatus);
TransactionModel::global()->removeTransaction(this);
}
break;
}
}
}
~KNSTransaction() override {
if (TransactionModel::global()->contains(this)) {
qWarning() << "deleting Transaction before it's done";
TransactionModel::global()->removeTransaction(this);
}
}
void cancel() override {}
private:
const QString m_id;
};
void KNSBackend::removeApplication(AbstractResource* app)
Transaction* KNSBackend::removeApplication(AbstractResource* app)
{
auto res = qobject_cast<KNSResource*>(app);
new KNSTransaction(this, res, Transaction::RemoveRole);
auto t = new KNSTransaction(this, res, Transaction::RemoveRole);
m_engine->uninstall(res->entry());
return t;
}
void KNSBackend::installApplication(AbstractResource* app)
Transaction* KNSBackend::installApplication(AbstractResource* app)
{
auto res = qobject_cast<KNSResource*>(app);
m_engine->install(res->entry());
new KNSTransaction(this, res, Transaction::InstallRole);
return new KNSTransaction(this, res, Transaction::InstallRole);
}
void KNSBackend::installApplication(AbstractResource* app, const AddonList& /*addons*/)
Transaction* KNSBackend::installApplication(AbstractResource* app, const AddonList& /*addons*/)
{
installApplication(app);
return installApplication(app);
}
int KNSBackend::updatesCount() const
......
......@@ -41,9 +41,9 @@ public:
explicit KNSBackend(QObject* parent, const QString& iconName, const QString &knsrc);
~KNSBackend() override;
void removeApplication(AbstractResource* app) override;
void installApplication(AbstractResource* app) override;
void installApplication(AbstractResource* app, const AddonList& addons) override;
Transaction* removeApplication(AbstractResource* app) override;
Transaction* installApplication(AbstractResource* app) override;
Transaction* installApplication(AbstractResource* app, const AddonList& addons) override;
int updatesCount() const override;
AbstractReviewsBackend* reviewsBackend() const override;
AbstractBackendUpdater* backendUpdater() const override;
......
......@@ -25,7 +25,6 @@
#include "utils.h"
#include "LocalFilePKResource.h"
#include <resources/AbstractResource.h>
#include <Transaction/TransactionModel.h>
#include <QDebug>
#include <QTimer>
#include <KLocalizedString>
......@@ -43,7 +42,6 @@ PKTransaction::PKTransaction(const QVector<AbstractResource*>& apps, Transaction
m_pkgnames.unite(res->allPackageNames().toSet());
}
TransactionModel::global()->addTransaction(this);
QTimer::singleShot(0, this, &PKTransaction::start);
}
......@@ -122,7 +120,7 @@ void PKTransaction::cancellableChanged()
void PKTransaction::cancel()
{
if (!m_trans) {
TransactionModel::global()->cancelTransaction(this);
setStatus(CancelledStatus);
} else if (m_trans->allowCancel()) {
m_trans->cancel();
} else {
......@@ -171,11 +169,7 @@ void PKTransaction::cleanup(PackageKit::Transaction::Exit exit, uint runtime)
}
this->submitResolve();
setStatus(Transaction::DoneStatus);
if (cancel)
TransactionModel::global()->cancelTransaction(this);
else
TransactionModel::global()->removeTransaction(this);
setStatus(Transaction::CancelledStatus);
}
void PKTransaction::proceed()
......
......@@ -29,7 +29,6 @@
#include <resources/AbstractResource.h>
#include <resources/StandardBackendUpdater.h>
#include <resources/SourcesModel.h>
#include <Transaction/TransactionModel.h>
#include <appstream/OdrsReviewsBackend.h>
#include <appstream/AppStreamIntegration.h>
......@@ -441,8 +440,9 @@ int PackageKitBackend::updatesCount() const
return m_updatesPackageId.count();
}
void PackageKitBackend::installApplication(AbstractResource* app, const AddonList& addons)
Transaction* PackageKitBackend::installApplication(AbstractResource* app, const AddonList& addons)
{
Transaction* t = nullptr;
if(!addons.addonsToInstall().isEmpty())
{
QVector<AbstractResource*> appsToInstall;
......@@ -454,27 +454,29 @@ void PackageKitBackend::installApplication(AbstractResource* app, const AddonLis
appsToInstall += m_packages.packages.value(toInstall);
Q_ASSERT(appsToInstall.last());
}
new PKTransaction(appsToInstall, Transaction::ChangeAddonsRole);
t = new PKTransaction(appsToInstall, Transaction::ChangeAddonsRole);
}
if (!addons.addonsToRemove().isEmpty()) {
QVector<AbstractResource*> appsToRemove = kTransform<QVector<AbstractResource*>>(addons.addonsToRemove(), [this](const QString& toRemove){ return m_packages.packages.value(toRemove); });
new PKTransaction(appsToRemove, Transaction::RemoveRole);
t = new PKTransaction(appsToRemove, Transaction::RemoveRole);
}
if (!app->isInstalled())
installApplication(app);
t = installApplication(app);
return t;
}
void PackageKitBackend::installApplication(AbstractResource* app)
Transaction* PackageKitBackend::installApplication(AbstractResource* app)
{
new PKTransaction({app}, Transaction::InstallRole);
return new PKTransaction({app}, Transaction::InstallRole);
}
void PackageKitBackend::removeApplication(AbstractResource* app)
Transaction* PackageKitBackend::removeApplication(AbstractResource* app)
{
Q_ASSERT(!isFetching());
new PKTransaction({app}, Transaction::RemoveRole);
return new PKTransaction({app}, Transaction::RemoveRole);
}
QSet<AbstractResource*> PackageKitBackend::upgradeablePackages() const
......
......@@ -51,9 +51,9 @@ class DISCOVERCOMMON_EXPORT PackageKitBackend : public AbstractResourcesBackend
ResultsStream* findResourceByPackageName(const QUrl& search) override;
int updatesCount() const override;
void installApplication(AbstractResource* app) override;
void installApplication(AbstractResource* app, const AddonList& addons) override;
void removeApplication(AbstractResource* app) override;
Transaction* installApplication(AbstractResource* app) override;
Transaction* installApplication(AbstractResource* app, const AddonList& addons) override;
Transaction* removeApplication(AbstractResource* app) override;
bool isValid() const override { return true; }
QSet<AbstractResource*> upgradeablePackages() const;
bool isFetching() const override;
......
......@@ -26,7 +26,6 @@
#include <resources/SourcesModel.h>
#include <Category/Category.h>
#include <Transaction/Transaction.h>
#include <Transaction/TransactionModel.h>
#include <KAboutData>
#include <KLocalizedString>
......@@ -130,26 +129,24 @@ AbstractReviewsBackend* SnapBackend::reviewsBackend() const
return m_reviews;
}
void SnapBackend::installApplication(AbstractResource* app, const AddonList& addons)
Transaction* SnapBackend::installApplication(AbstractResource* app, const AddonList& addons)
{
Q_ASSERT(addons.isEmpty());
installApplication(app);
return installApplication(app);
}
void SnapBackend::installApplication(AbstractResource* _app)
Transaction* SnapBackend::installApplication(AbstractResource* _app)
{
TransactionModel *transModel = TransactionModel::global();
auto app = qobject_cast<SnapResource*>(_app);
auto job = m_socket.snapAction(app->packageName(), SnapSocket::Install);
transModel->addTransaction(new SnapTransaction(app, job, &m_socket, Transaction::InstallRole));
return new SnapTransaction(app, job, &m_socket, Transaction::InstallRole);
}
void SnapBackend::removeApplication(AbstractResource* _app)
Transaction* SnapBackend::removeApplication(AbstractResource* _app)
{
TransactionModel *transModel = TransactionModel::global();
auto app = qobject_cast<SnapResource*>(_app);
auto job = m_socket.snapAction(app->packageName(), SnapSocket::Remove);
transModel->addTransaction(new SnapTransaction(app, job, &m_socket, Transaction::RemoveRole));
return new SnapTransaction(app, job, &m_socket, Transaction::RemoveRole);
}
QString SnapBackend::displayName() const
......
......@@ -46,9 +46,9 @@ public:
bool isValid() const override { return true; } // No external file dependencies that could cause runtime errors
QList<QAction*> messageActions() const override { return {}; }
void installApplication(AbstractResource* app) override;
void installApplication(AbstractResource* app, const AddonList& addons) override;
void removeApplication(AbstractResource* app) override;
Transaction* installApplication(AbstractResource* app) override;
Transaction* installApplication(AbstractResource* app, const AddonList& addons) override;
Transaction* removeApplication(AbstractResource* app) override;
bool isFetching() const override { return m_fetching; }
SnapSocket* socket() { return &m_socket; }
void checkForUpdates() override {}
......
......@@ -22,7 +22,6 @@
#include "SnapBackend.h"
#include "SnapResource.h"
#include "SnapSocket.h"
#include <Transaction/TransactionModel.h>
#include <QTimer>
SnapTransaction::SnapTransaction(SnapResource* app, SnapJob* job, SnapSocket* socket, Role role)
......@@ -51,7 +50,7 @@ void SnapTransaction::transactionStarted(SnapJob* job)
if (!job->isSuccessful()) {
qWarning() << "non-successful transaction" << job->statusCode();
TransactionModel::global()->removeTransaction(this);
setStatus(DoneStatus);
return;
}
......@@ -82,5 +81,4 @@ void SnapTransaction::finishTransaction()
delete m_timer;
m_app->refreshState();
setStatus(DoneStatus);
TransactionModel::global()->removeTransaction(this);
}
......@@ -25,7 +25,6 @@
#include <resources/ResourcesProxyModel.h>
#include <resources/AbstractBackendUpdater.h>
#include <ApplicationAddonsModel.h>
#include <Transaction/TransactionModel.h>
#include <ReviewsBackend/ReviewsModel.h>
#include <ScreenshotsModel.h>
#include "../libsnapclient/SnapSocket.h"
......
......@@ -66,10 +66,10 @@ void DiscoverDeclarativePlugin::registerTypes(const char* /*uri*/)
qmlRegisterUncreatableType<QAction>("org.kde.discover", 1, 0, "QAction", QStringLiteral("Use QQC Action"));
qmlRegisterUncreatableType<AbstractResource>("org.kde.discover", 1, 0, "AbstractResource", QStringLiteral("should come from the ResourcesModel"));
qmlRegisterUncreatableType<AbstractSourcesBackend>("org.kde.discover", 1, 0, "AbstractSourcesBackend", QStringLiteral("should come from the SourcesModel"));
qmlRegisterUncreatableType<Transaction>("org.kde.discover", 1, 0, "Transaction", QStringLiteral("should come from the backends"));
qmlRegisterType<Rating>();
qmlRegisterType<AbstractResourcesBackend>();
qmlRegisterType<AbstractReviewsBackend>();
qmlRegisterType<Category>();
qmlRegisterType<ResourcesModel>();
qmlRegisterType<Transaction>();
}
......@@ -73,9 +73,9 @@ AbstractResourcesBackend::AbstractResourcesBackend(QObject* parent)
{
}
void AbstractResourcesBackend::installApplication(AbstractResource* app)
Transaction* AbstractResourcesBackend::installApplication(AbstractResource* app)
{
installApplication(app, AddonList());
return installApplication(app, AddonList());
}
void AbstractResourcesBackend::integrateActions(KActionCollection*)
......
......@@ -180,31 +180,29 @@ class DISCOVERCOMMON_EXPORT AbstractResourcesBackend : public QObject
public Q_SLOTS:
/**
* This gets called when the backend should install an application.
* The AbstractResourcesBackend should create a Transaction object, which
* will provide Muon with information like the status and progress of a transaction,
* and add it to the TransactionModel with the following line:
* \code
* TransactionModel::global()->addTransaction(transaction);
* \endcode
* where transaction is the newly created Transaction.
* The AbstractResourcesBackend should create a Transaction object, is returned and
* will be included in the TransactionModel
* @param app the application to be installed
* @param addons the addons which should be installed with the application
* @returns the Transaction that keeps track of the installation process