telepathy-mission-control: mcd_account_altered_by_plugin: get the new value from the McdStorage

[Simon McVittie]

Module: telepathy-mission-control
Branch: master
Commit: 2c1a41059fec4657c59078525ca2527a06443b90
URL:    http://cgit.freedesktop.org/telepathy/telepathy-mission-control/commit/?id=2c1a41059fec4657c59078525ca2527a06443b90

Author: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date:   Tue Sep 11 17:22:29 2012 +0100

mcd_account_altered_by_plugin: get the new value from the McdStorage

This means we pick up the new value a little more reliably: in
particular, this fixes processing of URISchemes, ConnectAutomatically
and Supersedes.

---

 src/mcd-account.c                                  |   65 ++++++++++++++------
 .../account-manager/backend-makes-changes.py       |   21 +++----
 2 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/src/mcd-account.c b/src/mcd-account.c
index b388cfe..3737eb1 100644
--- a/src/mcd-account.c
+++ b/src/mcd-account.c
@@ -2250,6 +2250,31 @@ mcd_account_altered_by_plugin (McdAccount *account,
     {
         guint i = 0;
         const McdDBusProp *prop = NULL;
+        GValue value = G_VALUE_INIT;
+        GError *error = NULL;
+
+        DEBUG ("%s", name);
+
+        if (tp_strdiff (name, "Parameters") &&
+            !mcd_storage_init_value_for_attribute (&value, name))
+        {
+            WARNING ("plugin wants to alter %s but I don't know what "
+                     "type that ought to be", name);
+            return;
+        }
+
+        if (!tp_strdiff (name, "Parameters"))
+        {
+            get_parameters (TP_SVC_DBUS_PROPERTIES (account), name, &value);
+        }
+        else if (!mcd_storage_get_attribute (account->priv->storage,
+                                             account->priv->unique_name,
+                                             name, &value, &error))
+        {
+            WARNING ("cannot get new value of %s: %s", name, error->message);
+            g_error_free (error);
+            return;
+        }
 
         /* find the property update handler */
         for (; prop == NULL && account_properties[i].name != NULL; i++)
@@ -2262,32 +2287,32 @@ mcd_account_altered_by_plugin (McdAccount *account,
          * then issue the change notification (DBus signals etc) for it   */
         if (prop != NULL)
         {
-            TpSvcDBusProperties *self = TP_SVC_DBUS_PROPERTIES (account);
-
-            if (prop->getprop != NULL)
+            /* poke the value back into itself with the setter: this      *
+             * extra round-trip may trigger extra actions like notifying  *
+             * the connection manager of the change, even though our own  *
+             * internal storage already has this value and needn't change */
+            if (prop->setprop != NULL)
             {
-                GValue value = G_VALUE_INIT;
-
-                prop->getprop (self, name, &value);
-
-                /* poke the value back into itself with the setter: this      *
-                 * extra round-trip may trigger extra actions like notifying  *
-                 * the connection manager of the change, even though our own  *
-                 * internal storage already has this value and needn't change */
-                if (prop->setprop != NULL)
-                  prop->setprop (self, prop->name, &value, NULL);
-                else
-                  mcd_account_changed_property (account, prop->name, &value);
-
-                g_value_unset (&value);
+                DEBUG ("Calling property setter for %s", name);
+                if (!prop->setprop (TP_SVC_DBUS_PROPERTIES (account),
+                                    prop->name, &value, &error))
+                {
+                    WARNING ("Unable to set %s: %s", name, error->message);
+                    g_error_free (error);
+                }
             }
             else
             {
-                DEBUG ("Valid DBus property %s with no get method was changed"
-                       " - cannot notify change since we cannot get its value",
-                      name);
+                DEBUG ("Emitting signal directly for %s", name);
+                mcd_account_changed_property (account, prop->name, &value);
             }
         }
+        else
+        {
+            DEBUG ("%s does not appear to be an Account property", name);
+        }
+
+        g_value_unset (&value);
     }
 }
 
diff --git a/tests/twisted/account-manager/backend-makes-changes.py b/tests/twisted/account-manager/backend-makes-changes.py
index d895dfb..a886082 100644
--- a/tests/twisted/account-manager/backend-makes-changes.py
+++ b/tests/twisted/account-manager/backend-makes-changes.py
@@ -132,9 +132,8 @@ def test(q, bus, mc, fake_accounts_service=None, **kwargs):
                 args=[account_path,
                     cs.ACCOUNT_IFACE_ADDRESSING + '.URISchemes']),
             )
-    # FIXME: doesn't work
-    #assertEquals(['xmpp'],
-    #        account.Properties.Get(cs.ACCOUNT_IFACE_ADDRESSING, 'URISchemes'))
+    assertEquals(['xmpp'],
+            account.Properties.Get(cs.ACCOUNT_IFACE_ADDRESSING, 'URISchemes'))
 
     fake_accounts_service.update_attributes(account_tail,
         {'ConnectAutomatically': True})
@@ -156,9 +155,8 @@ def test(q, bus, mc, fake_accounts_service=None, **kwargs):
                 signal='AttributeChanged',
                 args=[account_path, 'ConnectAutomatically']),
             )
-    # FIXME: doesn't work
-    #assertEquals(True,
-    #        account.Properties.Get(cs.ACCOUNT, 'ConnectAutomatically'))
+    assertEquals(True,
+            account.Properties.Get(cs.ACCOUNT, 'ConnectAutomatically'))
 
     fake_accounts_service.update_attributes(account_tail,
         {'Supersedes': [cs.ACCOUNT_PATH_PREFIX + 'ac1/game/altair']})
@@ -174,19 +172,16 @@ def test(q, bus, mc, fake_accounts_service=None, **kwargs):
                 path=account_path,
                 signal='AccountPropertyChanged',
                 interface=cs.ACCOUNT,
-                # FIXME: signal is emitted, but doesn't have the new value
-                #args=[{'Supersedes':
-                #    [cs.ACCOUNT_PATH_PREFIX + 'ac1/game/altair']}],
-                predicate=(lambda e: 'Supersedes' in e.args[0]),
+                args=[{'Supersedes':
+                    [cs.ACCOUNT_PATH_PREFIX + 'ac1/game/altair']}],
                 ),
             EventPattern('dbus-signal',
                 path=cs.TEST_DBUS_ACCOUNT_PLUGIN_PATH,
                 signal='AttributeChanged',
                 args=[account_path, 'Supersedes']),
             )
-    # FIXME: doesn't work
-    #assertEquals([cs.ACCOUNT_PATH_PREFIX + 'ac1/game/altair'],
-    #        account.Properties.Get(cs.ACCOUNT, 'Supersedes'))
+    assertEquals([cs.ACCOUNT_PATH_PREFIX + 'ac1/game/altair'],
+            account.Properties.Get(cs.ACCOUNT, 'Supersedes'))
 
     fake_accounts_service.update_attributes(account_tail,
         {'AutomaticPresenceType': cs.PRESENCE_TYPE_HIDDEN,