Skip to content

Commit

Permalink
[lipstick] Add explicit app name to notifications. Contributes to JB#…
Browse files Browse the repository at this point in the history
…51293

We have some heuristics in NotificationManager for digging out the app
name all the way from d-bus sender process binary to guessing the
.desktop file name and using what's available there, falling back to
process binary if no .desktop file available.

Since that can go wrong, especially without a .desktop file even
existing, let's expose what the notification really included.

The database schema is slightly altered. The earlier times seemed
to be enough in the past, last time 2017, so didn't bother
keeping those update paths.
  • Loading branch information
pvuorela committed Sep 25, 2020
1 parent dd8bb08 commit 016f38b
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 95 deletions.
83 changes: 48 additions & 35 deletions src/notifications/lipsticknotification.cpp
Expand Up @@ -55,53 +55,56 @@ const char *LipstickNotification::HINT_PROGRESS = "x-nemo-progress";
const char *LipstickNotification::HINT_VIBRA = "x-nemo-vibrate";
const char *LipstickNotification::HINT_VISIBILITY = "x-nemo-visibility";

LipstickNotification::LipstickNotification(const QString &appName, const QString &disambiguatedAppName, uint id,
LipstickNotification::LipstickNotification(const QString &appName, const QString &explicitAppName,
const QString &disambiguatedAppName, uint id,
const QString &appIcon, const QString &summary, const QString &body,
const QStringList &actions, const QVariantHash &hints, int expireTimeout,
QObject *parent) :
QObject(parent),
m_appName(appName),
m_disambiguatedAppName(disambiguatedAppName),
m_id(id),
m_summary(summary),
m_body(body),
m_actions(actions),
m_hints(hints),
m_expireTimeout(expireTimeout),
m_priority(hints.value(LipstickNotification::HINT_PRIORITY).toInt()),
m_timestamp(hints.value(LipstickNotification::HINT_TIMESTAMP).toDateTime().toMSecsSinceEpoch()),
m_activeProgressTimer(0)
QObject *parent)
: QObject(parent),
m_appName(appName),
m_explicitAppName(explicitAppName),
m_disambiguatedAppName(disambiguatedAppName),
m_id(id),
m_summary(summary),
m_body(body),
m_actions(actions),
m_hints(hints),
m_expireTimeout(expireTimeout),
m_priority(hints.value(LipstickNotification::HINT_PRIORITY).toInt()),
m_timestamp(hints.value(LipstickNotification::HINT_TIMESTAMP).toDateTime().toMSecsSinceEpoch()),
m_activeProgressTimer(0)
{
if (!appIcon.isEmpty()) {
m_hints.insert(LipstickNotification::HINT_APP_ICON, appIcon);
}
updateHintValues();
}

LipstickNotification::LipstickNotification(QObject *parent) :
QObject(parent),
m_id(0),
m_expireTimeout(-1),
m_priority(0),
m_timestamp(0),
m_activeProgressTimer(0)
LipstickNotification::LipstickNotification(QObject *parent)
: QObject(parent),
m_id(0),
m_expireTimeout(-1),
m_priority(0),
m_timestamp(0),
m_activeProgressTimer(0)
{
}

LipstickNotification::LipstickNotification(const LipstickNotification &notification) :
QObject(notification.parent()),
m_appName(notification.m_appName),
m_disambiguatedAppName(notification.m_disambiguatedAppName),
m_id(notification.m_id),
m_summary(notification.m_summary),
m_body(notification.m_body),
m_actions(notification.m_actions),
m_hints(notification.m_hints),
m_hintValues(notification.m_hintValues),
m_expireTimeout(notification.m_expireTimeout),
m_priority(notification.m_priority),
m_timestamp(notification.m_timestamp),
m_activeProgressTimer(0) // not caring for d-bus serialization
LipstickNotification::LipstickNotification(const LipstickNotification &notification)
: QObject(notification.parent()),
m_appName(notification.m_appName),
m_explicitAppName(notification.m_explicitAppName),
m_disambiguatedAppName(notification.m_disambiguatedAppName),
m_id(notification.m_id),
m_summary(notification.m_summary),
m_body(notification.m_body),
m_actions(notification.m_actions),
m_hints(notification.m_hints),
m_hintValues(notification.m_hintValues),
m_expireTimeout(notification.m_expireTimeout),
m_priority(notification.m_priority),
m_timestamp(notification.m_timestamp),
m_activeProgressTimer(0) // not caring for d-bus serialization
{
}

Expand All @@ -110,6 +113,11 @@ QString LipstickNotification::appName() const
return m_appName;
}

QString LipstickNotification::explicitAppName() const
{
return m_explicitAppName;
}

QString LipstickNotification::disambiguatedAppName() const
{
return m_disambiguatedAppName;
Expand All @@ -120,6 +128,11 @@ void LipstickNotification::setAppName(const QString &appName)
m_appName = appName;
}

void LipstickNotification::setExplicitAppName(const QString &appName)
{
m_explicitAppName = appName;
}

void LipstickNotification::setDisambiguatedAppName(const QString &disambiguatedAppName)
{
m_disambiguatedAppName = disambiguatedAppName;
Expand Down
10 changes: 8 additions & 2 deletions src/notifications/lipsticknotification.h
Expand Up @@ -31,6 +31,7 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject
{
Q_OBJECT
Q_PROPERTY(QString appName READ appName CONSTANT)
Q_PROPERTY(QString explicitAppName READ explicitAppName CONSTANT)
Q_PROPERTY(QString disambiguatedAppName READ disambiguatedAppName CONSTANT)
Q_PROPERTY(uint id READ id CONSTANT)
Q_PROPERTY(QString appIcon READ appIcon NOTIFY appIconChanged)
Expand Down Expand Up @@ -143,6 +144,7 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject
* Creates an object for storing information about a single notification.
*
* \param appName name of the application sending the notification
* \param explicitAppName name explicitly set on the received notification
* \param disambiguatedAppName name of the application, decorated to disambiguate names from android and native applications
* \param id the ID of the notification
* \param appIcon icon ID of the application sending the notification
Expand All @@ -153,8 +155,9 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject
* \param expireTimeout expiration timeout for the notification
* \param parent the parent QObject
*/
LipstickNotification(const QString &appName, const QString &disambiguatedAppName, uint id, const QString &appIcon,
const QString &summary, const QString &body, const QStringList &actions, const QVariantHash &hints,
LipstickNotification(const QString &appName, const QString &explicitAppName, const QString &disambiguatedAppName,
uint id, const QString &appIcon, const QString &summary, const QString &body,
const QStringList &actions, const QVariantHash &hints,
int expireTimeout, QObject *parent = 0);

/*!
Expand All @@ -166,10 +169,12 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject

//! Returns the name of the application sending the notification
QString appName() const;
QString explicitAppName() const;
QString disambiguatedAppName() const;

//! Sets the name of the application sending the notification
void setAppName(const QString &appName);
void setExplicitAppName(const QString &appName);
void setDisambiguatedAppName(const QString &disambiguatedAppName);

//! Returns the ID of the notification
Expand Down Expand Up @@ -334,6 +339,7 @@ class LIPSTICK_EXPORT LipstickNotification : public QObject

//! Name of the application sending the notification
QString m_appName;
QString m_explicitAppName;
QString m_disambiguatedAppName;

//! The ID of the notification
Expand Down
62 changes: 25 additions & 37 deletions src/notifications/notificationmanager.cpp
Expand Up @@ -307,7 +307,7 @@ uint NotificationManager::Notify(const QString &appName, uint replacesId, const
}

LipstickNotification notificationData(
appName, disambiguatedAppName, id, appIcon, summary, body, actions, hints_, expireTimeout);
appName, appName, disambiguatedAppName, id, appIcon, summary, body, actions, hints_, expireTimeout);
applyCategoryDefinition(&notificationData);
hints_ = notificationData.hints();

Expand All @@ -331,6 +331,7 @@ uint NotificationManager::Notify(const QString &appName, uint replacesId, const
}

notification->setAppName(notificationData.appName());
notification->setExplicitAppName(notificationData.explicitAppName());
notification->setDisambiguatedAppName(notificationData.disambiguatedAppName());
notification->setSummary(notificationData.summary());
notification->setBody(notificationData.body());
Expand Down Expand Up @@ -664,9 +665,10 @@ void NotificationManager::publish(const LipstickNotification *notification, uint
}

// Add the notification, its actions and its hints to the database
execSQL("INSERT INTO notifications VALUES (?, ?, ?, ?, ?, ?, ?)",
execSQL("INSERT INTO notifications VALUES (?, ?, ?, ?, ?, ?, ?, ?)",
QVariantList() << id << notification->appName() << notification->appIcon() << notification->summary()
<< notification->body() << notification->expireTimeout() << notification->disambiguatedAppName());
<< notification->body() << notification->expireTimeout() << notification->disambiguatedAppName()
<< notification->explicitAppName());

// every other is identifier and every other the localized name for it
bool everySecond = false;
Expand Down Expand Up @@ -777,55 +779,39 @@ bool NotificationManager::checkTableValidity()

const int databaseVersion(schemaVersion());

if (databaseVersion == 0) {
// Unmodified database - remove any existing notifications, which might cause problems
if (databaseVersion < 3) {
// All databases this old should have been migrated already.
qWarning() << "Removing obsolete notifications";
recreateNotificationsTable = true;
recreateActionsTable = true;
recreateHintsTable = true;
recreateExpirationTable = true;
} else if (databaseVersion == 1) {
// Check that the table schemas are as expected
recreateNotificationsTable = !verifyTableColumns("notifications",
QStringList() << "id" << "app_name" << "app_icon" << "summary"
<< "body" << "expire_timeout");
recreateActionsTable = !verifyTableColumns("actions", QStringList() << "id" << "action" << "display_name");
recreateHintsTable = !verifyTableColumns("hints", QStringList() << "id" << "hint" << "value");
recreateExpirationTable = !verifyTableColumns("expiration", QStringList() << "id" << "expire_at");

// Extend the notifications table with the disambiguated_app_name column
QSqlQuery query(*m_database);
if (query.exec("ALTER TABLE notifications ADD COLUMN disambiguated_app_name TEXT") &&
query.exec("UPDATE notifications SET disambiguated_app_name = app_name")) {
qWarning() << "Extended notifications table";
} else {
qWarning() << "Failed to extend notifications table!";
recreateNotificationsTable = true;
}
} else {
if (databaseVersion == 2) {
if (databaseVersion == 3) {
QSqlQuery query(*m_database);
if (query.exec("ALTER TABLE actions ADD COLUMN display_name TEXT")) {
qWarning() << "Extended actions table";
if (query.exec("ALTER TABLE notifications ADD COLUMN explicit_app_name TEXT")) {
qWarning() << "Extended notifications table";
} else {
qWarning() << "Failed to extend actions table!";
recreateActionsTable = true;
qWarning() << "Failed to extend notifications table!";
recreateNotificationsTable = true;
}

} else {
recreateNotificationsTable = !verifyTableColumns("notifications",
QStringList() << "id" << "app_name" << "app_icon" << "summary"
<< "body" << "expire_timeout" << "disambiguated_app_name" << "explicit_app_name");
recreateActionsTable = !verifyTableColumns("actions", QStringList() << "id" << "action" << "display_name");
}

recreateNotificationsTable = !verifyTableColumns("notifications",
QStringList() << "id" << "app_name" << "app_icon" << "summary"
<< "body" << "expire_timeout" << "disambiguated_app_name");
recreateHintsTable = !verifyTableColumns("hints", QStringList() << "id" << "hint" << "value");
recreateExpirationTable = !verifyTableColumns("expiration", QStringList() << "id" << "expire_at");
}

if (recreateNotificationsTable) {
qWarning() << "Recreating notifications table";
result &= recreateTable("notifications", "id INTEGER PRIMARY KEY, app_name TEXT, app_icon TEXT, summary TEXT, body TEXT, expire_timeout INTEGER, disambiguated_app_name TEXT");
result &= recreateTable("notifications", "id INTEGER PRIMARY KEY, app_name TEXT, app_icon TEXT, summary TEXT, "
"body TEXT, expire_timeout INTEGER, disambiguated_app_name TEXT, "
"explicit_app_name TEXT");
}
if (recreateActionsTable) {
qWarning() << "Recreating actions table";
Expand All @@ -840,8 +826,8 @@ bool NotificationManager::checkTableValidity()
result &= recreateTable("expiration", "id INTEGER PRIMARY KEY, expire_at INTEGER");
}

if (result && databaseVersion != 3) {
if (!setSchemaVersion(3)) {
if (result && databaseVersion != 4) {
if (!setSchemaVersion(4)) {
qWarning() << "Unable to set database schema version!";
}
}
Expand Down Expand Up @@ -968,6 +954,7 @@ void NotificationManager::fetchData(bool update)
QSqlRecord notificationsRecord = notificationsQuery.record();
int notificationsTableIdFieldIndex = notificationsRecord.indexOf("id");
int notificationsTableAppNameFieldIndex = notificationsRecord.indexOf("app_name");
int notificationsTableExplicitAppNameFieldIndex = notificationsRecord.indexOf("explicit_app_name");
int notificationsTableDisambiguatedAppNameFieldIndex = notificationsRecord.indexOf("disambiguated_app_name");
int notificationsTableAppIconFieldIndex = notificationsRecord.indexOf("app_icon");
int notificationsTableSummaryFieldIndex = notificationsRecord.indexOf("summary");
Expand All @@ -976,6 +963,7 @@ void NotificationManager::fetchData(bool update)
while (notificationsQuery.next()) {
const uint id = notificationsQuery.value(notificationsTableIdFieldIndex).toUInt();
QString appName = notificationsQuery.value(notificationsTableAppNameFieldIndex).toString();
QString explicitAppName = notificationsQuery.value(notificationsTableExplicitAppNameFieldIndex).toString();
QString disambiguatedAppName = notificationsQuery.value(notificationsTableDisambiguatedAppNameFieldIndex).toString();
QString appIcon = notificationsQuery.value(notificationsTableAppIconFieldIndex).toString();
QString summary = notificationsQuery.value(notificationsTableSummaryFieldIndex).toString();
Expand Down Expand Up @@ -1007,9 +995,9 @@ void NotificationManager::fetchData(bool update)
}
}

LipstickNotification *notification = new LipstickNotification(appName, disambiguatedAppName, id, appIcon,
summary, body, notificationActions, notificationHints,
expireTimeout, this);
LipstickNotification *notification = new LipstickNotification(appName, explicitAppName, disambiguatedAppName,
id, appIcon, summary, body, notificationActions,
notificationHints, expireTimeout, this);
m_notifications.insert(id, notification);

if (id > m_previousNotificationID) {
Expand Down
14 changes: 8 additions & 6 deletions tests/ut_lipsticknotification/ut_lipsticknotification.cpp
Expand Up @@ -21,6 +21,7 @@
void Ut_Notification::testGettersAndSetters()
{
QString appName = "appName1";
QString explicitAppName = "explicitAppName1";
QString disambiguatedAppName = "appName1-2-3";
uint id = 1;
QString appIcon = "appIcon1";
Expand All @@ -46,8 +47,9 @@ void Ut_Notification::testGettersAndSetters()
int expireTimeout = 1;

// Ensure that the constructor puts things in place
LipstickNotification notification(appName, disambiguatedAppName, id, appIcon, summary, body, actions, hints, expireTimeout);
LipstickNotification notification(appName, explicitAppName, disambiguatedAppName, id, appIcon, summary, body, actions, hints, expireTimeout);
QCOMPARE(notification.appName(), appName);
QCOMPARE(notification.explicitAppName(), explicitAppName);
QCOMPARE(notification.disambiguatedAppName(), disambiguatedAppName);
QCOMPARE(notification.id(), id);
QCOMPARE(notification.appIcon(), appIcon);
Expand Down Expand Up @@ -144,18 +146,18 @@ void Ut_Notification::testIcon()
hints.insert(LipstickNotification::HINT_IMAGE_PATH, imagePath);
}

LipstickNotification notification1(QString(), QString(), 0, appIcon, QString(), QString(), QStringList(), hints, 0);
LipstickNotification notification1(QString(), QString(), QString(), 0, appIcon, QString(), QString(), QStringList(), hints, 0);
QCOMPARE(notification1.appIcon(), appIcon);
QCOMPARE(notification1.hints().value(LipstickNotification::HINT_APP_ICON).toString(), appIcon);
QCOMPARE(notification1.hints().value(LipstickNotification::HINT_IMAGE_PATH).toString(), imagePath);

LipstickNotification notification2(QString(), QString(), 0, QString(), QString(), QString(), QStringList(), QVariantHash(), 0);
LipstickNotification notification2(QString(), QString(), QString(), 0, QString(), QString(), QString(), QStringList(), QVariantHash(), 0);
notification2.setHints(hints);
QCOMPARE(notification2.appIcon(), appIcon);
QCOMPARE(notification2.hints().value(LipstickNotification::HINT_APP_ICON).toString(), appIcon);
QCOMPARE(notification2.hints().value(LipstickNotification::HINT_IMAGE_PATH).toString(), imagePath);

LipstickNotification notification3(QString(), QString(), 0, QString(), QString(), QString(), QStringList(), hints, 0);
LipstickNotification notification3(QString(), QString(), QString(), 0, QString(), QString(), QString(), QStringList(), hints, 0);
QCOMPARE(notification3.appIcon(), appIcon);
QCOMPARE(notification3.hints().value(LipstickNotification::HINT_APP_ICON).toString(), appIcon);
QCOMPARE(notification3.hints().value(LipstickNotification::HINT_IMAGE_PATH).toString(), imagePath);
Expand All @@ -164,7 +166,7 @@ void Ut_Notification::testIcon()
void Ut_Notification::testSignals()
{
QVariantHash hints;
LipstickNotification notification(QString(), QString(), 0, QString(), QString(), QString(), QStringList(), hints, 0);
LipstickNotification notification(QString(), QString(), QString(), 0, QString(), QString(), QString(), QStringList(), hints, 0);
QSignalSpy summarySpy(&notification, SIGNAL(summaryChanged()));
QSignalSpy bodySpy(&notification, SIGNAL(bodyChanged()));
QSignalSpy appIconSpy(&notification, SIGNAL(appIconChanged()));
Expand Down Expand Up @@ -223,7 +225,7 @@ void Ut_Notification::testSerialization()
hints.insert(LipstickNotification::HINT_TIMESTAMP, timestamp);
int expireTimeout = 1;

LipstickNotification n1(appName, appName, id, appIcon, summary, body, actions, hints, expireTimeout);
LipstickNotification n1(appName, appName, appName, id, appIcon, summary, body, actions, hints, expireTimeout);
LipstickNotification n2;

// Transfer a Notification from n1 to n2 by serializing it to a QDBusArgument and unserializing it
Expand Down
Expand Up @@ -89,7 +89,7 @@ LipstickNotification *createNotification(uint id, int urgency = 0, QVariant prio
if (priority.isValid()) {
hints.insert(LipstickNotification::HINT_PRIORITY, priority);
}
LipstickNotification *notification = new LipstickNotification("ut_notificationfeedbackplayer", "", id, "", "", "", QStringList(), hints, -1);
LipstickNotification *notification = new LipstickNotification("ut_notificationfeedbackplayer", "", "", id, "", "", "", QStringList(), hints, -1);
notificationManagerNotification.insert(id, notification);
return notification;
}
Expand Down

0 comments on commit 016f38b

Please sign in to comment.