Merge lp:~larsu/unity8/lp1236413 into lp:unity8

[Lars Karlitski]

Indicators: add TimeFormatter and use it in the messaging menu

This component can be used to turn a timestamp and a format string into a string with the corresponding time and format. The string will change whenever the timezone changes.

Indicators: add TimeFormatter and use it in the messaging menu

This component can be used to turn a timestamp and a format string into a string with the corresponding time and format. The string will change whenever the timezone changes.

This patch includes a very simply test that doesn't cover the update-on-timezone-switching. What's the recommended way of spinning up a test bus in unity8's test framework?

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:467
http://jenkins.qa.ubuntu.com/job/unity8-ci/1440/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/5075
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-touch/3016
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-saucy/2308
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-amd64-ci/463
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-armhf-ci/1440
        deb: http://jenkins.qa.ubuntu.com/job/unity8-saucy-armhf-ci/1440/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-saucy-i386-ci/1439
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-saucy/1243
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-amd64/950
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-amd64/950/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/3018
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy-armhf/3018/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-maguro/2523
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2567
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/62
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/63

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1440/rebuild

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

Unrelated to this patch since it was broken before, but "hh:mm - MMM dd" screams for proper localization

[Nick Dedekind (nick-dedekind)]

I think the TimeFormatter object screams for shared private data (for dbus management).
Each message is going to be listening to the same dbus property, so we might as well connect all formatters to the same callback using qt signal.

TimeFormatter <------
TimeFormatter <------ (emit updated) TimeFormatterPrivate <------ dbus
TimeFormatter <------

review: Needs Fixing

[Lars Karlitski (larsu)]

This is why I'm using gdbus, it does exactly that.

[Nick Dedekind (nick-dedekind)]

> This is why I'm using gdbus, it does exactly that.

But isn't the fetching of the bus and subscriptions to it's signals all a bit much for a simple time formatter? It's async, so there's a queue churning for every call to g_bus_get.
The exact same thing is going to be happening to every object at the same time. So I think the updates should be coming from a singleton.

Another note is that this should probably by in Utils rather than Indicators.

[Nick Dedekind (nick-dedekind)]

> > This is why I'm using gdbus, it does exactly that.
>
> But isn't the fetching of the bus and subscriptions to it's signals all a bit
> much for a simple time formatter? It's async, so there's a queue churning for
> every call to g_bus_get.
> The exact same thing is going to be happening to every object at the same
> time. So I think the updates should be coming from a singleton.
>
> Another note is that this should probably by in Utils rather than Indicators.

Right, discussed and thought it was light weight enough for now. Can make singleton if needed and move to Utils later.

Tested and all working.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'plugins/Unity/Indicators/CMakeLists.txt'
--- plugins/Unity/Indicators/CMakeLists.txt	2013-09-09 08:21:23 +0000
+++ plugins/Unity/Indicators/CMakeLists.txt	2013-10-16 20:25:38 +0000
@@ -36,6 +36,7 @@
     unitymenumodelcache.cpp
     unitymenumodelstack.cpp
     visibleindicatorsmodel.cpp
+    timeformatter.cpp
 )
 add_definitions(-DUNITYINDICATORS_LIBRARY)
 

=== modified file 'plugins/Unity/Indicators/Messaging/qml/MessageMenuItemFactory.qml'
--- plugins/Unity/Indicators/Messaging/qml/MessageMenuItemFactory.qml	2013-09-25 11:29:05 +0000
+++ plugins/Unity/Indicators/Messaging/qml/MessageMenuItemFactory.qml	2013-10-16 20:25:38 +0000
@@ -22,7 +22,6 @@
 import Ubuntu.Components 0.1
 import Unity.Indicators 0.1 as Indicators
 import QMenuModel 0.1 as QMenuModel
-import "utils.js" as Utils
 
 Indicators.BaseMenuItem {
     id: menuItem
@@ -65,7 +64,7 @@
             SimpleTextMessage {
                 // text
                 title: menu && menu.label ? menu.label : ""
-                time: menu ? Utils.formatDate(menu.ext.xCanonicalTime) : ""
+                time: menu ? menu.ext.xCanonicalTime : 0
                 message: menu && menu.ext.xCanonicalText ? menu.ext.xCanonicalText : ""
                 // icons
                 avatar: menu && menu.ext.icon !== undefined ? menu.ext.icon : "qrc:/indicators/artwork/messaging/default_contact.png"
@@ -90,7 +89,7 @@
 
                 // text
                 title: menu && menu.label ? menu.label : ""
-                time: menu ? Utils.formatDate(menu.ext.xCanonicalTime) : ""
+                time: menu ? menu.ext.xCanonicalTime : 0
                 message: menu && menu.ext.xCanonicalText ? menu.ext.xCanonicalText : ""
                 replyButtonText: actionsDescription && actionsDescription[0].label ? actionsDescription[0].label : "Send"
                 // icons
@@ -126,7 +125,7 @@
 
                 // text
                 title: menu && menu.label ? menu.label : ""
-                time: menu ? Utils.formatDate(menu.ext.xCanonicalTime) : ""
+                time: menu ? menu.ext.xCanonicalTime : ""
                 message: menu && menu.ext.xCanonicalText ? menu.ext.xCanonicalText : ""
                 actionButtonText: actionsDescription && actionsDescription[0].label ?  actionsDescription[0].label : "Call back"
                 replyButtonText: actionsDescription && actionsDescription[1].label ? actionsDescription[1].label : "Send"

=== modified file 'plugins/Unity/Indicators/Messaging/qml/SimpleTextMessage.qml'
--- plugins/Unity/Indicators/Messaging/qml/SimpleTextMessage.qml	2013-08-22 13:05:57 +0000
+++ plugins/Unity/Indicators/Messaging/qml/SimpleTextMessage.qml	2013-10-16 20:25:38 +0000
@@ -20,19 +20,20 @@
 
 import QtQuick 2.0
 import Ubuntu.Components 0.1
+import Unity.Indicators 0.1
 
 HeroMessage {
     id: __heroMessage
 
     property string title: ""
-    property string time: ""
+    property var time
     property string message: ""
 
     property alias footer: footerLoader.sourceComponent
 
     expandedHeight: fullMessage.y + fullMessage.height + units.gu(2)
     heroMessageHeader.titleText.text: title
-    heroMessageHeader.subtitleText.text: time
+    heroMessageHeader.subtitleText.text: timeFormatter.timeString
     heroMessageHeader.bodyText.text: message
 
     Item {
@@ -49,6 +50,12 @@
         opacity: 0.0
         enabled: false
 
+        TimeFormatter {
+            id: timeFormatter
+            time: __heroMessage.time
+            format: "hh:mm - MMM dd"
+        }
+
         Label {
             id: bodyText
             anchors {

=== modified file 'plugins/Unity/Indicators/Messaging/qml/SnapDecision.qml'
--- plugins/Unity/Indicators/Messaging/qml/SnapDecision.qml	2013-08-30 18:21:21 +0000
+++ plugins/Unity/Indicators/Messaging/qml/SnapDecision.qml	2013-10-16 20:25:38 +0000
@@ -21,13 +21,13 @@
 import QtQuick 2.0
 import Ubuntu.Components 0.1
 import Unity.Indicators 0.1 as Indicators
-import "utils.js" as Utils
+import Unity.Indicators 0.1
 
 HeroMessage {
     id: snapDecision
 
     property string title: ""
-    property string time: ""
+    property var time
     property string message: ""
 
     property bool activateEnabled: true
@@ -41,7 +41,7 @@
     heroMessageHeader.titleText.text:  title
     heroMessageHeader.subtitleText.text: message
     heroMessageHeader.subtitleText.color: "#e8e1d0"
-    heroMessageHeader.bodyText.text: time
+    heroMessageHeader.bodyText.text: timeFormatter.timeString
     heroMessageHeader.bodyText.color: "#8f8f88"
 
     signal activate
@@ -59,6 +59,12 @@
         height: units.gu(4)
         opacity: 0.0
 
+        TimeFormatter {
+            id: timeFormatter
+            time: snapDecision.time
+            format: "hh:mm - MMM dd"
+        }
+
         Button {
             text: "Message"
             color: "#bababa"

=== removed file 'plugins/Unity/Indicators/Messaging/qml/utils.js'
--- plugins/Unity/Indicators/Messaging/qml/utils.js	2013-06-27 09:09:25 +0000
+++ plugins/Unity/Indicators/Messaging/qml/utils.js	1970-01-01 00:00:00 +0000
@@ -1,27 +0,0 @@
-/*
- * Copyright 2013 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/>.
- *
- * Authors:
- *      Renato Araujo Oliveira Filho <[e-mail]>
- *      Olivier Tilloy <[e-mail]>
- */
-
-.pragma library
-
-function formatDate(timestamp) {
-    // timestamp is in microseconds since epoch
-    var date = new Date(timestamp / 1000);
-    return Qt.formatDateTime(date, "hh:mm - MMM dd");
-}

=== modified file 'plugins/Unity/Indicators/plugin.cpp'
--- plugins/Unity/Indicators/plugin.cpp	2013-09-09 09:23:15 +0000
+++ plugins/Unity/Indicators/plugin.cpp	2013-10-16 20:25:38 +0000
@@ -33,6 +33,7 @@
 #include "unitymenumodelcache.h"
 #include "unitymenumodelstack.h"
 #include "visibleindicatorsmodel.h"
+#include "timeformatter.h"
 
 static QObject* menuModelCacheSingleton(QQmlEngine* engine, QJSEngine* scriptEngine) {
   Q_UNUSED(engine);
@@ -51,6 +52,7 @@
     qmlRegisterType<RootActionState>(uri, 0, 1, "RootActionState");
     qmlRegisterType<ModelPrinter>(uri, 0, 1, "ModelPrinter");
     qmlRegisterType<VisibleIndicatorsModel>(uri, 0, 1, "VisibleIndicatorsModel");
+    qmlRegisterType<TimeFormatter>(uri, 0, 1, "TimeFormatter");
 
     qmlRegisterSingletonType<UnityMenuModelCache>(uri, 0, 1, "UnityMenuModelCache", menuModelCacheSingleton);
 

=== added file 'plugins/Unity/Indicators/timeformatter.cpp'
--- plugins/Unity/Indicators/timeformatter.cpp	1970-01-01 00:00:00 +0000
+++ plugins/Unity/Indicators/timeformatter.cpp	2013-10-16 20:25:38 +0000
@@ -0,0 +1,172 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Lars Uebernickel <[e-mail]>
+ */
+
+#include "timeformatter.h"
+
+#include <gio/gio.h>
+#include <QDateTime>
+
+struct TimeFormatterPrivate
+{
+    TimeFormatter *formatter;
+
+    QString format;
+    QString timeString;
+    qint64 time;
+
+    GDBusConnection *system_bus;
+    guint subscription_id;
+    GCancellable *cancellable;
+};
+
+static void
+timedate1_properties_changed (GDBusConnection *connection,
+                              const gchar *sender_name,
+                              const gchar *object_path,
+                              const gchar *interface_name,
+                              const gchar *signal_name,
+                              GVariant *parameters,
+                              gpointer user_data)
+{
+    Q_UNUSED(connection);
+    Q_UNUSED(sender_name);
+    Q_UNUSED(object_path);
+    Q_UNUSED(interface_name);
+    Q_UNUSED(signal_name);
+
+    TimeFormatterPrivate *priv = (TimeFormatterPrivate *)user_data;
+    GVariant *changed;
+    GVariantIter *iter;
+    const gchar *name;
+
+    if (!g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(sa{sv}as)")))
+        return;
+
+    g_variant_get (parameters, "(s@a{sv}as)", NULL, &changed, &iter);
+
+    if (g_variant_lookup (changed, "Timezone", "s", NULL)) {
+        priv->formatter->update();
+    }
+    else {
+        while (g_variant_iter_next (iter, "&s", &name)) {
+            if (g_str_equal (name, "Timezone")) {
+                priv->formatter->update();
+                break;
+            }
+        }
+    }
+
+    g_variant_unref (changed);
+    g_variant_iter_free (iter);
+}
+
+static void
+got_bus(GObject *object, GAsyncResult *result, gpointer user_data)
+{
+    Q_UNUSED(object);
+
+    TimeFormatterPrivate *priv = (TimeFormatterPrivate *)user_data;
+    GError *error = NULL;
+
+    priv->system_bus = g_bus_get_finish (result, &error);
+    if (priv->system_bus == NULL) {
+        if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+            qWarning("TimeFormatter: cannot connect to the system bus: %s", error->message);
+        g_error_free (error);
+        return;
+    }
+
+    /* Listen to the PropertiesChanged on the org.freedesktop.timedate1
+     * interface from any sender. In practice, this signal will only be sent
+     * from timedated (we can trust other processes on the system bus to behave
+     * nicely). That way, we don't have to watch timedated's well-known name
+     * and keep the process alive.
+     */
+    priv->subscription_id = g_dbus_connection_signal_subscribe (priv->system_bus,
+                                                                NULL,   /* sender */
+                                                                "org.freedesktop.DBus.Properties",
+                                                                "PropertiesChanged",
+                                                                NULL,
+                                                                "org.freedesktop.timedate1",
+                                                                G_DBUS_SIGNAL_FLAGS_NONE,
+                                                                timedate1_properties_changed,
+                                                                priv, NULL);
+}
+
+TimeFormatter::TimeFormatter(QObject *parent): QObject(parent)
+{
+    priv = new TimeFormatterPrivate;
+    priv->formatter = this;
+    priv->time = 0;
+    priv->format = "yyyy-MM-dd hh:mm";
+    priv->system_bus = NULL;
+    priv->subscription_id = 0;
+    priv->cancellable = g_cancellable_new ();
+
+    g_bus_get (G_BUS_TYPE_SYSTEM, priv->cancellable, got_bus, priv);
+}
+
+TimeFormatter::~TimeFormatter()
+{
+    if (priv->system_bus) {
+        g_dbus_connection_signal_unsubscribe (priv->system_bus, priv->subscription_id);
+        g_object_unref (priv->system_bus);
+    }
+
+    g_cancellable_cancel (priv->cancellable);
+    g_object_unref (priv->cancellable);
+}
+
+QString TimeFormatter::format() const
+{
+    return priv->format;
+}
+
+QString TimeFormatter::timeString() const
+{
+    return priv->timeString;
+}
+
+qint64 TimeFormatter::time() const
+{
+    return priv->time;
+}
+
+void TimeFormatter::setFormat(const QString &format)
+{
+    if (priv->format != format) {
+        priv->format = format;
+        Q_EMIT formatChanged(priv->format);
+        update();
+    }
+}
+
+void TimeFormatter::setTime(qint64 time)
+{
+    if (priv->time != time) {
+        priv->time = time;
+        Q_EMIT timeChanged(priv->time);
+        update();
+    }
+}
+
+void TimeFormatter::update()
+{
+    priv->timeString = QDateTime::fromMSecsSinceEpoch(priv->time / 1000).toString(priv->format);
+    Q_EMIT timeStringChanged(priv->timeString);
+}

=== added file 'plugins/Unity/Indicators/timeformatter.h'
--- plugins/Unity/Indicators/timeformatter.h	1970-01-01 00:00:00 +0000
+++ plugins/Unity/Indicators/timeformatter.h	2013-10-16 20:25:38 +0000
@@ -0,0 +1,54 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Lars Uebernickel <[e-mail]>
+ */
+
+#ifndef TIME_FORMATTER_H
+#define TIME_FORMATTER_H
+
+#include "unityindicatorsglobal.h"
+#include <QObject>
+
+class UNITYINDICATORS_EXPORT TimeFormatter : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(QString format READ format WRITE setFormat NOTIFY formatChanged)
+    Q_PROPERTY(QString timeString READ timeString NOTIFY timeStringChanged)
+    Q_PROPERTY(qint64 time READ time WRITE setTime NOTIFY timeChanged)
+
+public:
+    TimeFormatter(QObject *parent = 0);
+    ~TimeFormatter();
+
+    QString format() const;
+    QString timeString() const;
+    qint64 time() const;
+
+    void setFormat(const QString &format);
+    void setTime(qint64 time);
+
+    void update();
+
+Q_SIGNALS:
+    void formatChanged(const QString &format);
+    void timeStringChanged(const QString &timeString);
+    void timeChanged(qint64 time);
+
+private:
+    struct TimeFormatterPrivate *priv;
+};
+
+#endif

=== modified file 'tests/plugins/Unity/Indicators/CMakeLists.txt'
--- tests/plugins/Unity/Indicators/CMakeLists.txt	2013-10-11 14:32:39 +0000
+++ tests/plugins/Unity/Indicators/CMakeLists.txt	2013-10-16 20:25:38 +0000
@@ -30,4 +30,5 @@
     indicatorsmodeltest
     menucontentactivatortest
     rootactionstatetest
+    timeformattertest
     )

=== added file 'tests/plugins/Unity/Indicators/timeformattertest.cpp'
--- tests/plugins/Unity/Indicators/timeformattertest.cpp	1970-01-01 00:00:00 +0000
+++ tests/plugins/Unity/Indicators/timeformattertest.cpp	2013-10-16 20:25:38 +0000
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2013 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/>.
+ *
+ * Author: Lars Uebernickel <[e-mail]>
+ */
+
+#include "timeformatter.h"
+
+#include <QtTest>
+#include <QDebug>
+
+class TimeFormatterTest : public QObject
+{
+    Q_OBJECT
+private Q_SLOTS:
+
+    void initTestCase()
+    {
+        setenv("UNITY_TEST_ENV", "1", 1);
+    }
+
+    void cleanupTestCase()
+    {
+        unsetenv("UNITY_TEST_ENV");
+    }
+
+    void testFormat()
+    {
+        const QString format = "hh:mm dd.MM.yy";
+
+        QDateTime time = QDateTime::currentDateTime();
+
+        TimeFormatter formatter;
+        formatter.setTime(time.toMSecsSinceEpoch() * 1000);
+        formatter.setFormat(format);
+
+        QCOMPARE(formatter.timeString(), time.toString(format));
+    }
+};
+
+QTEST_GUILESS_MAIN(TimeFormatterTest)
+
+#include "timeformattertest.moc"