Merge lp:~jonas-drange/ubuntu-settings-components/buffer-printer-state-events into lp:~phablet-team/ubuntu-settings-components/printer-components

[Jonas G. Drange]

* adds PrinterSignalHandler that is responsible for handling (some) printer signals.
* uses PrinterSignalHandler in PrinterModel so that i only will update printers a minimum number of times

[lp:~jonas-drange/ubuntu-settings-components/buffer-printer-state-events]

254. By Jonas G. Drange on 2017-02-16

undoes spurious change

[Andrew Hayzen (ahayzen)]

Looks good and resolves the issue, one question about a possible racy situation - don't think it would happen often (if at all) but maybe we should protect?

review: Needs Information

[Andrew Hayzen (ahayzen)]

As discussed the signals will get queued [0], so this won't be racy :-)

LGTM \o/

0 - https://wiki.qt.io/Threads_Events_QObjects#Signals_and_slots_across_threads
(...an automatic connection (the default) means that if the thread the receiver is living in is the same as the current thread, a direct connection is used; otherwise, a queued connection is used...)

review: Approve

=== modified file 'plugins/Ubuntu/Settings/Printers/CMakeLists.txt'
--- plugins/Ubuntu/Settings/Printers/CMakeLists.txt	2017-02-16 14:10:27 +0000
+++ plugins/Ubuntu/Settings/Printers/CMakeLists.txt	2017-02-16 14:10:27 +0000
@@ -31,6 +31,7 @@
 
     printer/printer.cpp
     printer/printerjob.cpp
+    printer/printersignalhandler.cpp
     printers/printers.cpp
 
     enums.h

=== modified file 'plugins/Ubuntu/Settings/Printers/models/printermodel.cpp'
--- plugins/Ubuntu/Settings/Printers/models/printermodel.cpp	2017-02-16 14:10:27 +0000
+++ plugins/Ubuntu/Settings/Printers/models/printermodel.cpp	2017-02-16 14:10:27 +0000
@@ -31,12 +31,14 @@
     QObject::connect(m_backend, &PrinterBackend::printerAdded,
                      this, &PrinterModel::printerAdded);
     QObject::connect(m_backend, &PrinterBackend::printerModified,
-                     this, &PrinterModel::printerModified);
+                     &m_signalHandler, &PrinterSignalHandler::onPrinterModified);
     QObject::connect(m_backend, &PrinterBackend::printerStateChanged,
-                     this, &PrinterModel::printerModified);
+                     &m_signalHandler, &PrinterSignalHandler::onPrinterModified);
     QObject::connect(m_backend, &PrinterBackend::printerDeleted,
                      this, &PrinterModel::printerDeleted);
 
+    connect(&m_signalHandler, SIGNAL(printerModified(const QString&)),
+            this, SLOT(printerModified(const QString&)));
     connect(m_backend, SIGNAL(printerLoaded(QSharedPointer<Printer>)),
             this, SLOT(printerLoaded(QSharedPointer<Printer>)));
 
@@ -77,18 +79,11 @@
     addPrinter(printer, CountChangeSignal::Emit);
 }
 
-void PrinterModel::printerModified(
-    const QString &text, const QString &printerUri,
-    const QString &printerName, uint printerState,
-    const QString &printerStateReason, bool acceptingJobs)
+void PrinterModel::printerModified(const QString &printerName)
 {
-    Q_UNUSED(text);
-    Q_UNUSED(printerUri);
-    Q_UNUSED(printerState);
-    Q_UNUSED(printerStateReason);
-    Q_UNUSED(acceptingJobs);
-
-    m_backend->requestPrinter(printerName);
+    // These signals might be emitted of a now deleted printer.
+    if (getPrinterByName(printerName))
+        m_backend->requestPrinter(printerName);
 }
 
 void PrinterModel::printerAdded(

=== modified file 'plugins/Ubuntu/Settings/Printers/models/printermodel.h'
--- plugins/Ubuntu/Settings/Printers/models/printermodel.h	2017-02-16 14:10:27 +0000
+++ plugins/Ubuntu/Settings/Printers/models/printermodel.h	2017-02-16 14:10:27 +0000
@@ -21,13 +21,13 @@
 
 #include "models/jobmodel.h"
 #include "printer/printer.h"
+#include "printer/printersignalhandler.h"
 
 #include <QAbstractListModel>
 #include <QByteArray>
 #include <QModelIndex>
 #include <QObject>
 #include <QSortFilterProxyModel>
-#include <QTimer>
 #include <QVariant>
 
 class PRINTERS_DECL_EXPORT PrinterModel : public QAbstractListModel
@@ -105,12 +105,11 @@
     PrinterBackend *m_backend;
 
     QList<QSharedPointer<Printer>> m_printers;
+    PrinterSignalHandler m_signalHandler;
 
 private Q_SLOTS:
     void printerLoaded(QSharedPointer<Printer> printer);
-    void printerModified(const QString &text, const QString &printerUri,
-        const QString &printerName, uint printerState,
-        const QString &printerStateReason, bool acceptingJobs);
+    void printerModified(const QString &printerName);
     void printerAdded(const QString &text, const QString &printerUri,
         const QString &printerName, uint printerState,
         const QString &printerStateReason, bool acceptingJobs);

=== added file 'plugins/Ubuntu/Settings/Printers/printer/printersignalhandler.cpp'
--- plugins/Ubuntu/Settings/Printers/printer/printersignalhandler.cpp	1970-01-01 00:00:00 +0000
+++ plugins/Ubuntu/Settings/Printers/printer/printersignalhandler.cpp	2017-02-16 14:10:27 +0000
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2017 Canonical, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "printersignalhandler.h"
+
+PrinterSignalHandler::PrinterSignalHandler(int triggerEventDelay,
+                                           QObject *parent)
+    : QObject(parent)
+{
+    m_timer.setInterval(triggerEventDelay);
+    connect(&m_timer, SIGNAL(timeout()), this, SLOT(process()));
+}
+
+PrinterSignalHandler::~PrinterSignalHandler()
+{
+}
+
+void PrinterSignalHandler::process()
+{
+    Q_FOREACH(auto printer, m_unprocessed) {
+        Q_EMIT printerModified(printer);
+    }
+    m_unprocessed.clear();
+    m_timer.stop();
+}
+
+void PrinterSignalHandler::onPrinterModified(
+    const QString &text, const QString &printerUri,
+    const QString &printerName, uint printerState,
+    const QString &printerStateReason, bool acceptingJobs)
+{
+
+    Q_UNUSED(text);
+    Q_UNUSED(printerUri);
+    Q_UNUSED(printerState);
+    Q_UNUSED(printerStateReason);
+    Q_UNUSED(acceptingJobs);
+
+    m_unprocessed << printerName;
+    m_timer.start();
+}
+
+void PrinterSignalHandler::onPrinterStateChanged(
+    const QString &text, const QString &printerUri,
+    const QString &printerName, uint printerState,
+    const QString &printerStateReason, bool acceptingJobs)
+{
+    Q_UNUSED(text);
+    Q_UNUSED(printerUri);
+    Q_UNUSED(printerState);
+    Q_UNUSED(printerStateReason);
+    Q_UNUSED(acceptingJobs);
+
+    m_unprocessed << printerName;
+    m_timer.start();
+}

=== added file 'plugins/Ubuntu/Settings/Printers/printer/printersignalhandler.h'
--- plugins/Ubuntu/Settings/Printers/printer/printersignalhandler.h	1970-01-01 00:00:00 +0000
+++ plugins/Ubuntu/Settings/Printers/printer/printersignalhandler.h	2017-02-16 14:10:27 +0000
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2017 Canonical, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef USC_PRINTER_SIGNAL_HANDLER_H
+#define USC_PRINTER_SIGNAL_HANDLER_H
+
+#include "printers_global.h"
+
+#include <QObject>
+#include <QSet>
+#include <QTimer>
+
+class PRINTERS_DECL_EXPORT PrinterSignalHandler : public QObject
+{
+    Q_OBJECT
+    QTimer m_timer;
+    QSet<QString> m_unprocessed;
+public:
+    explicit PrinterSignalHandler(int triggerEventDelay = 500,
+                                  QObject *parent = Q_NULLPTR);
+    ~PrinterSignalHandler();
+
+public Q_SLOTS:
+    void onPrinterModified(
+        const QString &text, const QString &printerUri,
+        const QString &printerName, uint printerState,
+        const QString &printerStateReason, bool acceptingJobs
+    );
+    void onPrinterStateChanged(
+        const QString &text, const QString &printerUri,
+        const QString &printerName, uint printerState,
+        const QString &printerStateReason, bool acceptingJobs
+    );
+
+private Q_SLOTS:
+    void process();
+
+Q_SIGNALS:
+    void printerModified(const QString &printerName);
+};
+
+#endif // USC_PRINTER_SIGNAL_HANDLER_H

=== modified file 'tests/unittests/Printers/CMakeLists.txt'
--- tests/unittests/Printers/CMakeLists.txt	2017-02-16 14:10:27 +0000
+++ tests/unittests/Printers/CMakeLists.txt	2017-02-16 14:10:27 +0000
@@ -40,3 +40,7 @@
 add_executable(testPrintersJobFilter tst_jobfilter.cpp ${MOCK_SOURCES})
 target_link_libraries(testPrintersJobFilter UbuntuSettingsPrintersQml Qt5::Test Qt5::Gui)
 add_test(tst_jobfilter testPrintersJobFilter)
+
+add_executable(testPrintersSignalHandler tst_signalhandler.cpp ${MOCK_SOURCES})
+target_link_libraries(testPrintersSignalHandler UbuntuSettingsPrintersQml Qt5::Test Qt5::Gui)
+add_test(tst_signalhandler testPrintersSignalHandler)

=== added file 'tests/unittests/Printers/tst_signalhandler.cpp'
--- tests/unittests/Printers/tst_signalhandler.cpp	1970-01-01 00:00:00 +0000
+++ tests/unittests/Printers/tst_signalhandler.cpp	2017-02-16 14:10:27 +0000
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2017 Canonical, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "printer/printersignalhandler.h"
+
+#include <QDebug>
+#include <QObject>
+#include <QSignalSpy>
+#include <QTest>
+
+class TestSignalHandler : public QObject
+{
+    Q_OBJECT
+private Q_SLOTS:
+    void testEmptyCount()
+    {
+        PrinterSignalHandler handler(500);
+        QSignalSpy modifiedSpy(&handler, SIGNAL(printerModified(const QString&)));
+
+        for (int i = 0; i < 500; i++) {
+            handler.onPrinterStateChanged("spam!", "ipp://bar/baz", "printer-a", 0, "none", true);
+        }
+
+        modifiedSpy.wait(1000);
+        QCOMPARE(modifiedSpy.count(), 1);
+    }
+};
+
+QTEST_GUILESS_MAIN(TestSignalHandler)
+#include "tst_signalhandler.moc"
+