Merge lp:~nick-dedekind/qmenumodel/qdbusactiongroup-variant-conversion into lp:qmenumodel

[Nick Dedekind]

Action updateState/activate uses parameter types to determine correct variant conversion.

Action updateState/activate uses parameter types to determine correct variant conversion.
Added tests for implicit type conversion.

[lp:~nick-dedekind/qmenumodel/qdbusactiongroup-variant-conversion]

104. By Nick Dedekind on 2014-03-20

Action updateState/activate uses parameter types to determine corect variant conversion

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:104
http://jenkins.qa.ubuntu.com/job/qmenumodel-ci/46/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qmenumodel-trusty-amd64-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qmenumodel-trusty-armhf-ci/8
        deb: http://jenkins.qa.ubuntu.com/job/qmenumodel-trusty-armhf-ci/8/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qmenumodel-ci/46/rebuild

review: Approve (continuous-integration)

Unmerged revisions

104. By Nick Dedekind on 2014-03-20

Action updateState/activate uses parameter types to determine corect variant conversion

=== modified file 'libqmenumodel/src/qdbusactiongroup.cpp'
--- libqmenumodel/src/qdbusactiongroup.cpp	2013-08-09 08:01:43 +0000
+++ libqmenumodel/src/qdbusactiongroup.cpp	2014-03-20 11:46:21 +0000
@@ -213,14 +213,41 @@
 void QDBusActionGroup::updateActionState(const QString &name, const QVariant &state)
 {
     if (m_actionGroup != NULL) {
-        g_action_group_change_action_state(m_actionGroup, name.toUtf8().data(), Converter::toGVariant(state));
+        GVariant* action_state;
+        QByteArray nameBytes = name.toUtf8();
+
+        action_state = g_action_group_get_action_state(m_actionGroup, nameBytes.data());
+        if (action_state) {
+            g_action_group_change_action_state(m_actionGroup,
+                                               nameBytes.data(),
+                                               Converter::toGVariantWithSchema(state, g_variant_get_type_string(action_state)));
+
+            g_variant_unref(action_state);
+        } else {
+            g_action_group_change_action_state(m_actionGroup, nameBytes.data(), Converter::toGVariant(state));
+        }
     }
 }
 
 void QDBusActionGroup::activateAction(const QString &name, const QVariant &parameter)
 {
     if (m_actionGroup != NULL) {
-        g_action_group_activate_action(m_actionGroup, name.toUtf8().data(), Converter::toGVariant(parameter));
+        const GVariantType* parameter_type;
+        QByteArray nameBytes = name.toUtf8();
+
+        parameter_type = g_action_group_get_action_parameter_type(m_actionGroup, nameBytes.data());
+        if (parameter_type) {
+            gchar* parameter_type_string;
+            parameter_type_string = g_variant_type_dup_string(parameter_type);
+
+            g_action_group_activate_action(m_actionGroup,
+                                           nameBytes.data(),
+                                           Converter::toGVariantWithSchema(parameter, parameter_type_string));
+
+            g_free(parameter_type_string);
+        } else {
+            g_action_group_activate_action(m_actionGroup, nameBytes.data(), Converter::toGVariant(parameter));
+        }
     }
 }
 

=== modified file 'libqmenumodel/src/qstateaction.cpp'
--- libqmenumodel/src/qstateaction.cpp	2014-02-28 12:10:02 +0000
+++ libqmenumodel/src/qstateaction.cpp	2014-03-20 11:46:21 +0000
@@ -93,9 +93,7 @@
 */
 void QStateAction::updateState(const QVariant &state)
 {
-    QVariant v = state;
-    if (v.convert(m_state.type()))
-      m_group->updateActionState(m_name, v);
+    m_group->updateActionState(m_name, state);
 }
 
 /*!
@@ -126,7 +124,7 @@
 void QStateAction::setState(const QVariant &state)
 {
     QVariant v = state;
-    if (!m_state.isValid() || (v.convert(m_state.type()) && v != m_state)) {
+    if ((m_state.isValid() != state.isValid()) || (v.convert(m_state.type()) && v != m_state)) {
         m_state = v;
         Q_EMIT stateChanged(m_state);
     }

=== modified file 'tests/client/actiongrouptest.cpp'
--- tests/client/actiongrouptest.cpp	2012-11-23 13:22:04 +0000
+++ tests/client/actiongrouptest.cpp	2014-03-20 11:46:21 +0000
@@ -154,6 +154,101 @@
     }
 
     /*
+     * Test if Action::trigger active the action over DBus
+     * using incorrect type (variant should be converted)
+     */
+    void testActiveActionIncorrectType()
+    {
+        // start model
+        m_model.start();
+        m_actionGroup.start();
+
+        // Make menu available
+        m_script.publishMenu();
+        m_script.walk(2);
+
+        // Get Action
+        QVariant action = m_model.data(m_model.index(1, 0), QMenuModel::Action);
+        QVERIFY(action.isValid());
+        QStateAction *act = m_actionGroup.action(action.toString());
+        QVERIFY(act);
+
+        // test action name
+        QCOMPARE(act->property("name").toString(), QString("Menu1Act"));
+
+        act->activate(QVariant(42.0));
+        // wait for dbus propagation
+        QTest::qWait(500);
+
+        QPair<QString, QVariant> result = m_script.popActivatedAction();
+        QCOMPARE(result.first, QString("Menu1Act"));
+        QCOMPARE(result.second.toString(), QString("42"));
+    }
+
+    /*
+     * Test updating state over DBus
+     */
+    void testUpdateActionState()
+    {
+        // start model
+        m_model.start();
+        m_actionGroup.start();
+
+        // Make menu available
+        m_script.publishMenu();
+        m_script.walk(2);
+
+        // Get Action
+        QVariant action = m_model.data(m_model.index(1, 0), QMenuModel::Action);
+        QVERIFY(action.isValid());
+        QStateAction *act = m_actionGroup.action(action.toString());
+        QVERIFY(act);
+
+        // test action name
+        QCOMPARE(act->property("name").toString(), QString("Menu1Act"));
+
+        act->updateState(QVariant("Testing1"));
+        // wait for dbus propagation
+        QTest::qWait(500);
+
+        QPair<QString, QVariant> result = m_script.popActionStateChange();
+        QCOMPARE(result.first, QString("Menu1Act"));
+        QCOMPARE(result.second.toString(), QString("Testing1"));
+    }
+
+    /*
+     * Test updating state over DBus
+     * using incorrect type (variant should be converted)
+     */
+    void testUpdateActionStateIncorrectType()
+    {
+        // start model
+        m_model.start();
+        m_actionGroup.start();
+
+        // Make menu available
+        m_script.publishMenu();
+        m_script.walk(2);
+
+        // Get Action
+        QVariant action = m_model.data(m_model.index(1, 0), QMenuModel::Action);
+        QVERIFY(action.isValid());
+        QStateAction *act = m_actionGroup.action(action.toString());
+        QVERIFY(act);
+
+        // test action name
+        QCOMPARE(act->property("name").toString(), QString("Menu1Act"));
+
+        act->updateState(QVariant(22));
+        // wait for dbus propagation
+        QTest::qWait(500);
+
+        QPair<QString, QVariant> result = m_script.popActionStateChange();
+        QCOMPARE(result.first, QString("Menu1Act"));
+        QCOMPARE(result.second.toString(), QString("22"));
+    }
+
+    /*
      * Test if Action became invalid after desappear from DBus
      */
     void testRemoveAction()
@@ -201,6 +296,7 @@
 
         // Action appear
         QVERIFY(act->isValid());
+        QVERIFY(act->state().isValid());
     }
 };
 

=== modified file 'tests/client/script_actiongrouptest.py'
--- tests/client/script_actiongrouptest.py	2013-01-31 04:22:06 +0000
+++ tests/client/script_actiongrouptest.py	2014-03-20 11:46:21 +0000
@@ -19,8 +19,8 @@
 from gi._gi import variant_type_from_string
 
 al = ActionList(MENU_OBJECT_PATH)
-al.appendItem("Menu0", "Menu0Act", actionStateType=variant_type_from_string('s'))
-al.appendItem("Menu1", "Menu1Act", actionStateType=variant_type_from_string('s'))
+al.appendItem("Menu0", "Menu0Act", actionStateType=variant_type_from_string('s'), actionState=GLib.Variant('s', 'Menu0State'))
+al.appendItem("Menu1", "Menu1Act", actionStateType=variant_type_from_string('s'), actionState=GLib.Variant('s', 'Menu0State'))
 al.removeItem("1", "Menu1Act")
 
 t = Script.create(al)

=== modified file 'tests/script/dbusmenuscript.cpp'
--- tests/script/dbusmenuscript.cpp	2012-11-23 13:22:04 +0000
+++ tests/script/dbusmenuscript.cpp	2014-03-20 11:46:21 +0000
@@ -109,3 +109,19 @@
     return qMakePair(QString(), QVariant());
 }
 
+QPair<QString, QVariant> DBusMenuScript::popActionStateChange()
+{
+    if (m_script) {
+        QDBusMessage reply = m_script->call("popActionStateChange");
+        if (reply.arguments().count() > 0) {
+            QVariant value;
+            QString name = reply.arguments()[0].toString();
+            if (reply.arguments().count() > 1) {
+                value = reply.arguments()[1];
+            }
+            return qMakePair(name, value);
+        }
+    }
+
+    return qMakePair(QString(), QVariant());
+}

=== modified file 'tests/script/dbusmenuscript.h'
--- tests/script/dbusmenuscript.h	2012-11-23 13:22:04 +0000
+++ tests/script/dbusmenuscript.h	2014-03-20 11:46:21 +0000
@@ -48,6 +48,7 @@
     void unpublishMenu();
 
     QPair<QString, QVariant> popActivatedAction();
+    QPair<QString, QVariant> popActionStateChange();
 
 private:
     QDBusInterface *m_script;

=== modified file 'tests/script/menuscript.py'
--- tests/script/menuscript.py	2013-01-31 04:22:06 +0000
+++ tests/script/menuscript.py	2014-03-20 11:46:21 +0000
@@ -74,6 +74,11 @@
     def popActivatedAction(self):
         return self._list._activatedActions.pop(0)
 
+    @dbus.service.method(dbus_interface=INTERFACE_NAME,
+                         in_signature='', out_signature='ss')
+    def popActionStateChange(self):
+        return self._list._actionStateChanges.pop(0)
+
     @staticmethod
     def create(aList):
         global bus
@@ -110,9 +115,15 @@
             parent.append_item(item)
 
             # Action
-            act = Gio.SimpleAction.new(self._kargs['actionName'], self._kargs['actionStateType'])
-            act.connect('activate', self._list._onActionActivated)
-            self._list._rootAction.insert(act)
+            if self._kargs['actionState'] == None:
+                act = Gio.SimpleAction.new(self._kargs['actionName'], self._kargs['actionStateType'])
+                act.connect('activate', self._list._onActionActivated)
+                self._list._rootAction.insert(act)
+            else:
+                act = Gio.SimpleAction.new_stateful(self._kargs['actionName'], self._kargs['actionStateType'], self._kargs['actionState'])
+                act.connect('activate', self._list._onActionActivated)
+                act.connect('change-state', self._list._onActionStateChanged)
+                self._list._rootAction.insert(act)
 
         elif self._kargs['link'] == 'section':
             section = Gio.Menu()
@@ -150,15 +161,17 @@
         self._root = None
         self._rootAction = None
         self._activatedActions = []
+        self._actionStateChanges = []
 
-    def appendItem(self, label, actionName, link=None, parentId=None,  properties=None, actionStateType=None):
+    def appendItem(self, label, actionName, link=None, parentId=None,  properties=None, actionStateType=None, actionState=None):
         self._actions.append(Action(self, 'append',
                                     parentId=parentId,
                                     label=label,
                                     actionName=actionName,
                                     link=link,
                                     properties=properties,
-                                    actionStateType=actionStateType))
+                                    actionStateType=actionStateType,
+                                    actionState=actionState))
 
     def removeItem(self, menuId, actionName=None):
         self._actions.append(Action(self, 'remove',
@@ -229,3 +242,6 @@
 
     def _onActionActivated(self, action, parameter):
         self._activatedActions.append((action.get_name(), parameter.get_string()))
+
+    def _onActionStateChanged(self, action, value):
+        self._actionStateChanges.append((action.get_name(), value.get_string()))