From 11c3fe71e4af268b5313c1408150b5822ba274a0 Mon Sep 17 00:00:00 2001 From: Pekka Vuorela Date: Tue, 28 Nov 2017 11:48:50 +0200 Subject: [PATCH] [transfer-engine] Add progress info to notifications. Contributes to JB#31692 Extra categories are now also removed in favor of just defining the properties explicitly. Switched to using freedesktop.org categories just because they exist, no actual expected effect as of now. --- data/data.pro | 6 - .../x-nemo.transfer.complete.conf | 3 - .../x-nemo.transfer.conf | 4 - .../x-nemo.transfer.error.conf | 2 - declarative/declarativetransfermodel.cpp | 2 +- lib/mediatransferinterface.h | 1 - lib/transferdbrecord.h | 2 - lib/transfermethodinfo.cpp | 2 +- rpm/transfer-engine-qt5.spec | 1 - src/dbmanager.cpp | 95 ++++++++++- src/dbmanager.h | 5 + src/transferengine.cpp | 152 +++++++++++------- src/transferengine_p.h | 4 +- transfer-engine.pro | 2 +- 14 files changed, 195 insertions(+), 86 deletions(-) delete mode 100644 data/data.pro delete mode 100644 data/notificationcategories/x-nemo.transfer.complete.conf delete mode 100644 data/notificationcategories/x-nemo.transfer.conf delete mode 100644 data/notificationcategories/x-nemo.transfer.error.conf diff --git a/data/data.pro b/data/data.pro deleted file mode 100644 index fd30c3a..0000000 --- a/data/data.pro +++ /dev/null @@ -1,6 +0,0 @@ -TEMPLATE = aux - -notification_categories.path = /usr/share/lipstick/notificationcategories -notification_categories.files = notificationcategories/*.conf - -INSTALLS += notification_categories diff --git a/data/notificationcategories/x-nemo.transfer.complete.conf b/data/notificationcategories/x-nemo.transfer.complete.conf deleted file mode 100644 index 7daa297..0000000 --- a/data/notificationcategories/x-nemo.transfer.complete.conf +++ /dev/null @@ -1,3 +0,0 @@ -appIcon=icon-lock-transfer -x-nemo-icon=icon-lock-transfer -urgency=1 diff --git a/data/notificationcategories/x-nemo.transfer.conf b/data/notificationcategories/x-nemo.transfer.conf deleted file mode 100644 index 244af6e..0000000 --- a/data/notificationcategories/x-nemo.transfer.conf +++ /dev/null @@ -1,4 +0,0 @@ -appIcon=icon-lock-transfer -x-nemo-icon=icon-lock-transfer -transient=true -urgency=1 diff --git a/data/notificationcategories/x-nemo.transfer.error.conf b/data/notificationcategories/x-nemo.transfer.error.conf deleted file mode 100644 index 83024c0..0000000 --- a/data/notificationcategories/x-nemo.transfer.error.conf +++ /dev/null @@ -1,2 +0,0 @@ -appIcon=icon-lock-information -urgency=2 diff --git a/declarative/declarativetransfermodel.cpp b/declarative/declarativetransfermodel.cpp index 6d17eb8..cdb7c4d 100644 --- a/declarative/declarativetransfermodel.cpp +++ b/declarative/declarativetransfermodel.cpp @@ -416,7 +416,7 @@ QSqlDatabase TransferModel::database() TransferDatabase database; database.setDatabaseName(absDbPath); database.setConnectOptions(QLatin1String("QSQLITE_OPEN_READONLY")); // sanity check - thread_database.setLocalData(database); + thread_database.setLocalData(database); } QSqlDatabase &database = thread_database.localData(); diff --git a/lib/mediatransferinterface.h b/lib/mediatransferinterface.h index a61fe81..cd020fb 100644 --- a/lib/mediatransferinterface.h +++ b/lib/mediatransferinterface.h @@ -53,7 +53,6 @@ class MediaTransferInterface: public QObject MediaItem *mediaItem(); - virtual QString displayName() const = 0; virtual QUrl serviceIcon() const = 0; virtual bool cancelEnabled() const = 0; diff --git a/lib/transferdbrecord.h b/lib/transferdbrecord.h index c61b0e7..bd45f7b 100644 --- a/lib/transferdbrecord.h +++ b/lib/transferdbrecord.h @@ -32,9 +32,7 @@ class TransferDBRecord { - public: - enum TransferDBRecordField { TransferID = 0, TransferType, diff --git a/lib/transfermethodinfo.cpp b/lib/transfermethodinfo.cpp index e2396d7..2922747 100644 --- a/lib/transfermethodinfo.cpp +++ b/lib/transfermethodinfo.cpp @@ -182,7 +182,7 @@ QVariant TransferMethodInfo::value(int index) const return accountId; case AccountIcon: return accountIcon; - default: + default: return QVariant(); } } diff --git a/rpm/transfer-engine-qt5.spec b/rpm/transfer-engine-qt5.spec index 70f9956..e35bf77 100644 --- a/rpm/transfer-engine-qt5.spec +++ b/rpm/transfer-engine-qt5.spec @@ -36,7 +36,6 @@ Obsoletes: nemo-transferengine <= 0.0.19 %{_bindir}/nemo-transfer-engine %{_datadir}/dbus-1/services/org.nemo.transferengine.service %{_datadir}/translations/nemo-transfer-engine_eng_en.qm -%{_datadir}/lipstick/notificationcategories/* %package -n libnemotransferengine-qt5 Summary: Transfer engine library. diff --git a/src/dbmanager.cpp b/src/dbmanager.cpp index 26a5eaf..d8edcaa 100644 --- a/src/dbmanager.cpp +++ b/src/dbmanager.cpp @@ -81,7 +81,8 @@ "strip_metadata INTEGER,\n" \ "scale_percent REAL,\n" \ "cancel_supported INTEGER,\n" \ - "restart_supported INTEGER\n" \ + "restart_supported INTEGER,\n" \ + "notification_id INTEGER\n" \ ");\n" // Cascade trigger i.e. when transfer is removed and it has metadata or callbacks, this @@ -95,7 +96,7 @@ "END;\n" // Update the following version if database schema changes. -#define USER_VERSION 1 +#define USER_VERSION 2 #define PRAGMA_USER_VERSION QString("PRAGMA user_version=%1").arg(USER_VERSION) class DbManagerPrivate { @@ -242,11 +243,28 @@ DbManager::DbManager(): // Create database schema if db didn't exist if (!dbExists) { - if(!d->createDatabaseSchema()) { + if (!d->createDatabaseSchema()) { qCritical("DbManager::DbManager: Failed to create DB schema. Can't continue!"); } } else { // Database exists, check the schema version + if (d->userVersion() == 1) { + // For this we get away with DeclarativeTransferModel directly reading database without + // update because notification_id is the last column + QSqlQuery query; + if (query.exec("ALTER TABLE transfers ADD COLUMN notification_id INTEGER")) { + qWarning() << "Extended transfers table"; + + if (!query.exec(PRAGMA_USER_VERSION)) { + qWarning() << "DbManager pragma user_version update:" + << query.lastError().text() << ":" << query.lastError().databaseText(); + } + } else { + qWarning() << "Failed to extend transfers table!" + << query.lastError().text() << ":" << query.lastError().databaseText(); + } + } + if (d->userVersion() != USER_VERSION) { d->deleteOldTables(); d->createDatabaseSchema(); @@ -408,9 +426,11 @@ int DbManager::createTransferEntry(const MediaItem *mediaItem) Q_D(DbManager); QSqlQuery query; query.prepare("INSERT INTO transfers (transfer_type, timestamp, status, progress, display_name, application_icon, thumbnail_icon, " - "service_icon, url, resource_name, mime_type, file_size, plugin_id, account_id, strip_metadata, scale_percent, cancel_supported, restart_supported)" - "VALUES (:transfer_type, :timestamp, :status, :progress, :display_name, :application_icon, :thumbnail_icon, :service_icon, " - ":url, :resource_name, :mime_type, :file_size, :plugin_id, :account_id, :strip_metadata, :scale_percent, :cancel_supported, :restart_supported)"); + " service_icon, url, resource_name, mime_type, file_size, plugin_id, account_id, strip_metadata, scale_percent, " + " cancel_supported, restart_supported, notification_id)" + "VALUES (:transfer_type, :timestamp, :status, :progress, :display_name, :application_icon, :thumbnail_icon, " + " :service_icon, :url, :resource_name, :mime_type, :file_size, :plugin_id, :account_id, :strip_metadata, :scale_percent, " + " :cancel_supported, :restart_supported, :notification_id)"); query.bindValue(":transfer_type", mediaItem->value(MediaItem::TransferType)); query.bindValue(":status", TransferEngineData::NotStarted); query.bindValue(":timestamp", d->currentDateTime()); @@ -429,9 +449,10 @@ int DbManager::createTransferEntry(const MediaItem *mediaItem) query.bindValue(":scale_percent", mediaItem->value(MediaItem::ScalePercent)); query.bindValue(":cancel_supported", mediaItem->value(MediaItem::CancelSupported)); query.bindValue(":restart_supported", mediaItem->value(MediaItem::RestartSupported)); + query.bindValue(":notification_id", 0); if (!query.exec()) { - qWarning() << "DbManager::createTransfereEntry: Failed to execute SQL query. Couldn't create an entry!" + qWarning() << "DbManager::createTransferEntry: Failed to execute SQL query. Couldn't create an entry!" << query.lastError().text() << ": " << query.lastError().databaseText(); return -1; @@ -771,6 +792,66 @@ TransferEngineData::TransferStatus DbManager::transferStatus(int key) const return TransferEngineData::Unknown; } } + +/*! + Returns the transfer progress or -1 if unavailable + */ +qreal DbManager::transferProgress(int key) const +{ + QString queryStr = QString("SELECT progress FROM transfers WHERE transfer_id='%1';").arg(QString::number(key)); + + QSqlQuery query; + if (!query.exec(queryStr)) { + qWarning() << "DbManager::transferStatus: Failed to execute SQL query. Couldn't get the progress!" + << query.lastError().text() << ": " + << query.lastError().databaseText(); + return -1; + } + + if (query.isActive() && query.isSelect()) { + query.first(); + return static_cast(query.value(0).toReal()); + } else { + return -1; + } +} + +int DbManager::notificationId(int key) +{ + QString queryStr = QString("SELECT notification_id FROM transfers WHERE transfer_id='%1';").arg(QString::number(key)); + + QSqlQuery query; + if (!query.exec(queryStr)) { + qWarning() << "DbManager::transferStatus: Failed to execute SQL query. Couldn't get the notification id!" + << query.lastError().text() << ": " + << query.lastError().databaseText(); + return 0; + } + + if (query.isActive() && query.isSelect()) { + query.first(); + return static_cast(query.value(0).toReal()); + } else { + return 0; + } +} + +bool DbManager::setNotificationId(int key, int notificationId) +{ + QString queryStr = QString("UPDATE transfers SET notification_id='%1' WHERE transfer_id='%2';") + .arg(QString::number(notificationId)).arg(QString::number(key)); + + QSqlQuery query; + if (!query.exec(queryStr)) { + qWarning() << "Failed to execute SQL query. Couldn't update the notification id!" + << query.lastError().text() << ": " + << query.lastError().databaseText(); + return false; + } + query.finish(); + return true; +} + /*! Reads the callback method names from the database for the transfer with \a key. The method names are set to the output arguments \a cancelMethod and \a restartMethod. diff --git a/src/dbmanager.h b/src/dbmanager.h index dbe621b..457e392 100644 --- a/src/dbmanager.h +++ b/src/dbmanager.h @@ -77,6 +77,11 @@ class DbManager TransferEngineData::TransferStatus transferStatus(int key) const; + qreal transferProgress(int key) const; + + int notificationId(int key); + bool setNotificationId(int key, int notificationId); + bool callbackMethods(int key, QString &cancelMethod, QString &restartMethod) const; MediaItem * mediaItem(int key) const; diff --git a/src/transferengine.cpp b/src/transferengine.cpp index 859e48c..791f32a 100644 --- a/src/transferengine.cpp +++ b/src/transferengine.cpp @@ -52,9 +52,11 @@ #define ACTIVITY_MONITOR_TIMEOUT 1*60*1000 // 1 minute in ms #define TRANSFER_EXPIRATION_THRESHOLD 3*60 // 3 minutes in seconds -#define TRANSFER_EVENT_CATEGORY "x-nemo.transfer" -#define TRANSFER_COMPLETE_EVENT_CATEGORY "x-nemo.transfer.complete" -#define TRANSFER_ERROR_EVENT_CATEGORY "x-nemo.transfer.error" +#define TRANSFER_EVENT_CATEGORY "transfer" +#define TRANSFER_COMPLETE_EVENT_CATEGORY "transfer.complete" +#define TRANSFER_ERROR_EVENT_CATEGORY "transfer.error" + +#define TRANSFER_PROGRESS_HINT "x-nemo-progress" TransferEngineSignalHandler * TransferEngineSignalHandler::instance() { @@ -299,7 +301,9 @@ void TransferEnginePrivate::recoveryCheck() void TransferEnginePrivate::sendNotification(TransferEngineData::TransferType type, TransferEngineData::TransferStatus status, - const QString &fileName) + qreal progress, + const QString &fileName, + int transferId) { if (!m_notificationsEnabled || fileName.isEmpty()) { return; @@ -310,6 +314,10 @@ void TransferEnginePrivate::sendNotification(TransferEngineData::TransferType ty QString summary; QString previewBody; QString previewSummary; + bool useProgress = false; + Notification::Urgency urgency = Notification::Normal; + QString appIcon = QStringLiteral("icon-lock-information"); + QString icon = QStringLiteral("x-nemo-icon=icon-lock-transfer"); // TODO: explicit grouping of transfer notifications is now removed, as grouping // will now be performed by lipstick. We may need to reinstate group summary @@ -325,18 +333,8 @@ void TransferEnginePrivate::sendNotification(TransferEngineData::TransferType ty // - For downloads // - For failed uploads, downloads and syncs - QList nList = Notification::notifications(); - Notification *existing = 0; - foreach (QObject *obj, nList) { - if (Notification *n = qobject_cast(obj)) { - if (n->summary() == fileName || n->previewSummary() == fileName) { - // This existing notification is for this file - existing = n; - break; - } - } - } + int notificationId = DbManager::instance()->notificationId(transferId); if (status == TransferEngineData::TransferFinished) { switch(type) { @@ -363,8 +361,11 @@ void TransferEnginePrivate::sendNotification(TransferEngineData::TransferType ty qWarning() << "TransferEnginePrivate::sendNotification: unknown state"; break; } + } else if (status == TransferEngineData::TransferInterrupted) { - category = TRANSFER_ERROR_EVENT_CATEGORY; + urgency = Notification::Critical; + appIcon = QStringLiteral("icon-lock-information"); + icon.clear(); switch (type) { case TransferEngineData::Upload: @@ -390,37 +391,58 @@ void TransferEnginePrivate::sendNotification(TransferEngineData::TransferType ty summary = fileName; previewSummary = summary; previewBody = body; - } else if (status == TransferEngineData::TransferCanceled) { - // Exit, no banners or events when user has canceled a transfer - // Remove any existing notification - if (existing) { - existing->close(); + } else if (status == TransferEngineData::TransferStarted) { + if (type == TransferEngineData::Upload || type == TransferEngineData::Download) { + category = TRANSFER_EVENT_CATEGORY; + + if (type == TransferEngineData::Upload) { + //: Notification for ongoing upload + //% "File uploading" + body = qtTrId("transferengine-no-file-uploading"); + } else { + //: Notification for ongoing file download + //% "File downloading" + body = qtTrId("transferengine-no-file-downloading"); + } + + summary = fileName; + + if (progress > 0) + useProgress = true; } + + } else if (status == TransferEngineData::TransferCanceled && notificationId > 0) { + // Exit, no banners or events when user has canceled a transfer + // Remove any existing notification + Notification notification; + notification.setReplacesId(notificationId); + notification.close(); + DbManager::instance()->setNotificationId(transferId, 0); + notificationId = 0; } if (!category.isEmpty()) { Notification notification; - if (!existing) { - // Create a new notification - if (m_settings.status() != QSettings::NoError) { - qWarning() << Q_FUNC_INFO << "Failed to read settings!" << m_settings.status(); - } else { - m_settings.beginGroup("transfers"); - const QString service = m_settings.value("service").toString(); - const QString path = m_settings.value("path").toString(); - const QString iface = m_settings.value("interface").toString(); - const QString method = m_settings.value("method").toString(); - m_settings.endGroup(); - - if (!service.isEmpty() && !path.isEmpty() && !iface.isEmpty() && !method.isEmpty()) { - notification.setRemoteActions(QVariantList() << Notification::remoteAction("default", "", service, path, iface, method) - << Notification::remoteAction("app", "", service, path, iface, method)); - } - } + if (notificationId > 0) { + notification.setReplacesId(notificationId); + } - existing = ¬ification; + if (m_settings.status() != QSettings::NoError) { + qWarning() << Q_FUNC_INFO << "Failed to read settings!" << m_settings.status(); + } else { + m_settings.beginGroup("transfers"); + const QString service = m_settings.value("service").toString(); + const QString path = m_settings.value("path").toString(); + const QString iface = m_settings.value("interface").toString(); + const QString method = m_settings.value("method").toString(); + m_settings.endGroup(); + + if (!service.isEmpty() && !path.isEmpty() && !iface.isEmpty() && !method.isEmpty()) { + notification.setRemoteActions(QVariantList() << Notification::remoteAction("default", "", service, path, iface, method) + << Notification::remoteAction("app", "", service, path, iface, method)); + } } //: Group name for notifications of successful transfers @@ -430,17 +452,24 @@ void TransferEnginePrivate::sendNotification(TransferEngineData::TransferType ty //% "Warnings" const QString errorsGroup(qtTrId("transferengine-notification_errors_group")); - // Update the notification - existing->setCategory(category); - existing->setAppName(category == TRANSFER_ERROR_EVENT_CATEGORY ? errorsGroup : transfersGroup); - existing->setSummary(summary); - existing->setBody(body); - existing->setPreviewSummary(previewSummary); - existing->setPreviewBody(previewBody); - existing->publish(); - } + notification.setCategory(category); + notification.setAppName(category == TRANSFER_ERROR_EVENT_CATEGORY ? errorsGroup : transfersGroup); + notification.setSummary(summary); + notification.setBody(body); + notification.setPreviewSummary(previewSummary); + notification.setPreviewBody(previewBody); + notification.setUrgency(urgency); - qDeleteAll(nList); + if (useProgress) { + notification.setHintValue(TRANSFER_PROGRESS_HINT, static_cast(progress)); + } + notification.publish(); + int newId = notification.replacesId(); + + if (newId != notificationId) { + DbManager::instance()->setNotificationId(transferId, newId); + } + } } int TransferEnginePrivate::uploadMediaItem(MediaItem *mediaItem, @@ -449,7 +478,7 @@ int TransferEnginePrivate::uploadMediaItem(MediaItem *mediaItem, { Q_Q(TransferEngine); - if (muif == 0) { + if (mediaItem == 0) { qWarning() << "TransferEngine::uploadMediaItem invalid MediaItem"; return -1; } @@ -586,7 +615,7 @@ void TransferEnginePrivate::uploadItemStatusChanged(MediaTransferInterface::Tran // If the flow ends up here, we are not interested in any signals the same object // might emit. Let's just disconnect them. muif->disconnect(); - sendNotification(type, tStatus, mediaFileOrResourceName(muif->mediaItem())); + sendNotification(type, tStatus, muif->progress(), mediaFileOrResourceName(muif->mediaItem()), key); ok = DbManager::instance()->updateTransferStatus(key, tStatus); if (m_plugins.remove(muif) == 0) { qWarning() << "TransferEnginePrivate::uploadItemStatusChanged: Failed to remove media upload object from the map!"; @@ -623,7 +652,7 @@ void TransferEnginePrivate::updateProgress(qreal progress) m_activityMonitor->newActivity(key); Q_Q(TransferEngine); - emit q->progressChanged(key, progress); + q->updateTransferProgress(key, progress); } void TransferEnginePrivate::pluginInfoReady() @@ -673,7 +702,6 @@ TransferEngineData::TransferType TransferEnginePrivate::transferType(int transfe } } - void TransferEnginePrivate::callbackCall(int transferId, CallbackMethodType method) { // Get DBus callback information. Callback list must contain at least @@ -1145,7 +1173,7 @@ void TransferEngine::finishTransfer(int transferId, int status, const QString &r transferStatus == TransferEngineData::TransferCanceled || transferStatus == TransferEngineData::TransferInterrupted) { DbManager::instance()->updateTransferStatus(transferId, transferStatus); - d->sendNotification(type, transferStatus, fileName); + d->sendNotification(type, transferStatus, DbManager::instance()->transferProgress(transferId), fileName, transferId); d->m_activityMonitor->activityFinished(transferId); emit statusChanged(transferId, status); @@ -1183,13 +1211,25 @@ void TransferEngine::updateTransferProgress(int transferId, double progress) d->exitSafely(); TransferEngineData::TransferType type = d->transferType(transferId); - if (type == TransferEngineData::Undefined || type == TransferEngineData::Upload) { + if (type != TransferEngineData::Download && type != TransferEngineData::Upload) { return; } + MediaItem *mediaItem = DbManager::instance()->mediaItem(transferId); + if (!mediaItem) { + qWarning() << "TransferEngine::finishTransfer: Failed to fetch MediaItem"; + return; + } + QString fileName = d->mediaFileOrResourceName(mediaItem); + + int oldProgressPercentage = DbManager::instance()->transferProgress(transferId) * 100; if (DbManager::instance()->updateProgress(transferId, progress)) { d->m_activityMonitor->newActivity(transferId); emit progressChanged(transferId, progress); + + if (oldProgressPercentage != (progress * 100)) { + d->sendNotification(type, DbManager::instance()->transferStatus(transferId), progress, fileName, transferId); + } } else { qWarning() << "TransferEngine::updateTransferProgress: Failed to update progress for " << transferId; } @@ -1313,7 +1353,7 @@ void TransferEngine::cancelTransfer(int transferId) } } /*! - DBus adaptor calls this method to enable or disable transfer speicific notifications + DBus adaptor calls this method to enable or disable transfer specific notifications based on \a enable argument. */ void TransferEngine::enableNotifications(bool enable) diff --git a/src/transferengine_p.h b/src/transferengine_p.h index b3fcfb6..d38dcbb 100644 --- a/src/transferengine_p.h +++ b/src/transferengine_p.h @@ -105,7 +105,9 @@ class TransferEnginePrivate: QObject void recoveryCheck(); void sendNotification(TransferEngineData::TransferType type, TransferEngineData::TransferStatus status, - const QString &fileName); + qreal progress, + const QString &fileName, + int transferId); int uploadMediaItem(MediaItem *mediaItem, MediaTransferInterface *muif, const QVariantMap &userData); diff --git a/transfer-engine.pro b/transfer-engine.pro index 7eefb8b..1451fd5 100644 --- a/transfer-engine.pro +++ b/transfer-engine.pro @@ -1,5 +1,5 @@ TEMPLATE = subdirs -SUBDIRS = lib src declarative tests data +SUBDIRS = lib src declarative tests src.depends = lib tests.depends = lib