fix(batchprint): send DBus notification with correct argument signature

batch-print/notifyclient.cpp

@@ -5,8 +5,7 @@
 #include "notifyclient.h"
 #include "errormessages.h"
 
-#include <QDBusInterface>
-#include <QDBusReply>
+#include <QDBusConnection>
 #include <QDebug>
 #include <QGuiApplication>
 
@@ -43,18 +42,13 @@ QString NotifyClient::buildBody(int total, int succeeded, const QStringList &fai
     return body;
 }
 
-void NotifyClient::notifyResult(int total, int succeeded, const QStringList &failedFiles)
+QDBusMessage NotifyClient::buildNotifyMessage(const QString &body)
 {
-    QString body = buildBody(total, succeeded, failedFiles);
-
-    QDBusInterface iface(QStringLiteral("org.freedesktop.Notifications"),
-                         QStringLiteral("/org/freedesktop/Notifications"),
-                         QStringLiteral("org.freedesktop.Notifications"));
-
-    if (!iface.isValid()) {
-        fprintf(stderr, "%s\n", body.toUtf8().constData());
-        return;
-    }
+    QDBusMessage msg = QDBusMessage::createMethodCall(
+        QStringLiteral("org.freedesktop.Notifications"),
+        QStringLiteral("/org/freedesktop/Notifications"),
+        QStringLiteral("org.freedesktop.Notifications"),
+        QStringLiteral("Notify"));
 
     QVariantList args;
     args << QStringLiteral("deepin-reader");
@@ -65,36 +59,31 @@ void NotifyClient::notifyResult(int total, int succeeded, const QStringList &fai
     args << QStringList();
     args << QVariantMap();
     args << qint32(-1);
+    msg.setArguments(args);
 
-    QDBusMessage reply = iface.call(QStringLiteral("Notify"), args);
-    if (reply.type() == QDBusMessage::ErrorMessage) {
-        fprintf(stderr, "%s\n", body.toUtf8().constData());
-    }
+    return msg;
 }
 
-void NotifyClient::notifyError(const QString &body)
+void NotifyClient::sendNotification(const QString &body)
 {
-    QDBusInterface iface(QStringLiteral("org.freedesktop.Notifications"),
-                         QStringLiteral("/org/freedesktop/Notifications"),
-                         QStringLiteral("org.freedesktop.Notifications"));
-
-    if (!iface.isValid()) {
+    QDBusConnection bus = QDBusConnection::sessionBus();
+    if (!bus.isConnected()) {
         fprintf(stderr, "%s\n", body.toUtf8().constData());
         return;
     }
 
-    QVariantList args;
-    args << QStringLiteral("deepin-reader");
-    args << quint32(0);
-    args << QStringLiteral("deepin-reader");
-    args << ErrorMessages::notifyTitle();
-    args << body;
-    args << QStringList();
-    args << QVariantMap();
-    args << qint32(-1);
-
-    QDBusMessage reply = iface.call(QStringLiteral("Notify"), args);
+    QDBusMessage reply = bus.call(buildNotifyMessage(body));
     if (reply.type() == QDBusMessage::ErrorMessage) {
         fprintf(stderr, "%s\n", body.toUtf8().constData());
     }
 }
+
+void NotifyClient::notifyResult(int total, int succeeded, const QStringList &failedFiles)
+{
+    sendNotification(buildBody(total, succeeded, failedFiles));
+}
+
+void NotifyClient::notifyError(const QString &body)
+{
+    sendNotification(body);
+}

batch-print/notifyclient.h

@@ -5,6 +5,7 @@
 #ifndef NOTIFYCLIENT_H
 #define NOTIFYCLIENT_H
 
+#include <QDBusMessage>
 #include <QString>
 #include <QStringList>
 
@@ -14,6 +15,10 @@ class NotifyClient
     static void notifyResult(int total, int succeeded, const QStringList &failedFiles);
     static void notifyError(const QString &body);
     static QString buildBody(int total, int succeeded, const QStringList &failedFiles);
+    static QDBusMessage buildNotifyMessage(const QString &body);
+
+private:
+    static void sendNotification(const QString &body);
 };
 
 #endif // NOTIFYCLIENT_H

tests/batch-print/ut_notifyclient.cpp

@@ -6,7 +6,10 @@
 #include "errormessages.h"
 
 #include <gtest/gtest.h>
+#include <QDBusArgument>
+#include <QDBusMessage>
 #include <QGuiApplication>
+#include <QVariant>
 
 static int argc = 1;
 static char *argv[] = { const_cast<char *>("ut_notifyclient"), nullptr };
@@ -121,3 +124,47 @@ TEST_F(NotifyClientTest, NotifyErrorNoDefaultPrinterDoesNotCrash) {
     NotifyClient::notifyError(ErrorMessages::noDefaultPrinter());
     SUCCEED();
 }
+
+// Regression test for the "no error prompt without printer" bug:
+// NotifyClient used to pass a QVariantList to QDBusInterface::call(), which
+// marshalled the whole argument list as ONE 'av' parameter. The deepin
+// notification daemon rejects signature 'av' with UnknownMethod, so every
+// notification silently fell back to invisible stderr. The message must
+// instead carry 8 separate arguments (signature susssasa{sv}i).
+TEST_F(NotifyClientTest, BuildNotifyMessageHasEightSeparateArguments) {
+    QDBusMessage msg = NotifyClient::buildNotifyMessage(
+        QStringLiteral("test body"));
+    QVariantList args = msg.arguments();
+    ASSERT_EQ(args.size(), 8);
+}
+
+TEST_F(NotifyClientTest, BuildNotifyMessageArgumentTypesMatchSpec) {
+    QDBusMessage msg = NotifyClient::buildNotifyMessage(
+        QStringLiteral("test body"));
+    QVariantList args = msg.arguments();
+    ASSERT_EQ(args.size(), 8);
+
+    EXPECT_EQ(args.at(0).metaType().id(), QMetaType::QString);
+    EXPECT_EQ(args.at(1).metaType().id(), QMetaType::UInt);
+    EXPECT_EQ(args.at(2).metaType().id(), QMetaType::QString);
+    EXPECT_EQ(args.at(3).metaType().id(), QMetaType::QString);
+    EXPECT_EQ(args.at(4).metaType().id(), QMetaType::QString);
+    EXPECT_EQ(args.at(5).metaType().id(), QMetaType::QStringList);
+    EXPECT_EQ(args.at(6).metaType().id(), QMetaType::QVariantMap);
+    EXPECT_EQ(args.at(7).metaType().id(), QMetaType::Int);
+    EXPECT_FALSE(args.at(0).toString().isEmpty());
+    EXPECT_EQ(args.at(4).toString(), QStringLiteral("test body"));
+    EXPECT_EQ(args.at(7).toInt(), -1);
+}
+
+TEST_F(NotifyClientTest, BuildNotifyMessageTargetsNotificationService) {
+    QDBusMessage msg = NotifyClient::buildNotifyMessage(
+        QStringLiteral("test body"));
+    EXPECT_EQ(msg.service(),
+              QStringLiteral("org.freedesktop.Notifications"));
+    EXPECT_EQ(msg.path(),
+              QStringLiteral("/org/freedesktop/Notifications"));
+    EXPECT_EQ(msg.interface(),
+              QStringLiteral("org.freedesktop.Notifications"));
+    EXPECT_EQ(msg.member(), QStringLiteral("Notify"));
+}