mcp_account_storage_set_*: return whether anything changed

The plugins are better-placed to do this than McdStorage: they know
their own storage format, after all.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=27727

2 files changed, 57 insertions, 15 deletions

diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c
index 733fbefd..e43d40e5 100644
--- a/tests/twisted/dbus-account-plugin.c
+++ b/tests/twisted/dbus-account-plugin.c
@@ -1082,7 +1082,7 @@ test_dbus_account_plugin_get (const McpAccountStorage *storage,
   return TRUE;
 }
 
-static gboolean
+static McpAccountStorageSetResult
 test_dbus_account_plugin_set_attribute (McpAccountStorage *storage,
     McpAccountManager *am,
     const gchar *account_name,
@@ -1092,6 +1092,8 @@ test_dbus_account_plugin_set_attribute (McpAccountStorage *storage,
 {
   TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
   Account *account = lookup_account (self, account_name);
+  GVariant *old;
+  guint old_flags;
 
   g_return_val_if_fail (account_name != NULL, FALSE);
   g_return_val_if_fail (attribute != NULL, FALSE);
@@ -1099,10 +1101,13 @@ test_dbus_account_plugin_set_attribute (McpAccountStorage *storage,
   DEBUG ("%s of %s", attribute, account_name);
 
   if (!self->active || account == NULL)
-    return FALSE;
+    return MCP_ACCOUNT_STORAGE_SET_RESULT_FAILED;
 
   if (value == NULL)
     {
+      if (!g_hash_table_contains (account->attributes, attribute))
+        return MCP_ACCOUNT_STORAGE_SET_RESULT_UNCHANGED;
+
       g_hash_table_remove (account->attributes, attribute);
       g_hash_table_remove (account->attribute_flags, attribute);
       g_hash_table_add (account->uncommitted_attributes, g_strdup (attribute));
@@ -1112,9 +1117,16 @@ test_dbus_account_plugin_set_attribute (McpAccountStorage *storage,
           "DeferringDeleteAttribute",
           g_variant_new_parsed ("(%o, %s)", account->path, attribute), NULL);
 
-      return TRUE;
+      return MCP_ACCOUNT_STORAGE_SET_RESULT_CHANGED;
     }
 
+  old = g_hash_table_lookup (account->attributes, attribute);
+  old_flags = GPOINTER_TO_UINT (g_hash_table_lookup (
+        account->attribute_flags, attribute));
+
+  if (old != NULL && g_variant_equal (old, value) && old_flags == flags)
+    return MCP_ACCOUNT_STORAGE_SET_RESULT_UNCHANGED;
+
   g_hash_table_insert (account->attributes, g_strdup (attribute),
       g_variant_ref (value));
   g_hash_table_insert (account->attribute_flags, g_strdup (attribute),
@@ -1127,10 +1139,10 @@ test_dbus_account_plugin_set_attribute (McpAccountStorage *storage,
       g_variant_new_parsed ("(%o, %s, %v)", account->path, attribute, value),
       NULL);
 
-  return TRUE;
+  return MCP_ACCOUNT_STORAGE_SET_RESULT_CHANGED;
 }
 
-static gboolean
+static McpAccountStorageSetResult
 test_dbus_account_plugin_set_parameter (McpAccountStorage *storage,
     McpAccountManager *am,
     const gchar *account_name,
@@ -1140,6 +1152,8 @@ test_dbus_account_plugin_set_parameter (McpAccountStorage *storage,
 {
   TestDBusAccountPlugin *self = TEST_DBUS_ACCOUNT_PLUGIN (storage);
   Account *account = lookup_account (self, account_name);
+  GVariant *old;
+  guint old_flags;
 
   g_return_val_if_fail (account_name != NULL, FALSE);
   g_return_val_if_fail (parameter != NULL, FALSE);
@@ -1147,10 +1161,14 @@ test_dbus_account_plugin_set_parameter (McpAccountStorage *storage,
   DEBUG ("%s of %s", parameter, account_name);
 
   if (!self->active || account == NULL)
-    return FALSE;
+    return MCP_ACCOUNT_STORAGE_SET_RESULT_FAILED;
 
   if (value == NULL)
     {
+      if (!g_hash_table_contains (account->parameters, parameter) &&
+          !g_hash_table_contains (account->untyped_parameters, parameter))
+        return MCP_ACCOUNT_STORAGE_SET_RESULT_UNCHANGED;
+
       g_hash_table_remove (account->parameters, parameter);
       g_hash_table_remove (account->untyped_parameters, parameter);
       g_hash_table_remove (account->parameter_flags, parameter);
@@ -1164,6 +1182,13 @@ test_dbus_account_plugin_set_parameter (McpAccountStorage *storage,
       return TRUE;
     }
 
+  old = g_hash_table_lookup (account->parameters, parameter);
+  old_flags = GPOINTER_TO_UINT (g_hash_table_lookup (
+        account->parameter_flags, parameter));
+
+  if (old != NULL && g_variant_equal (old, value) && old_flags == flags)
+    return MCP_ACCOUNT_STORAGE_SET_RESULT_UNCHANGED;
+
   g_hash_table_remove (account->untyped_parameters, parameter);
   g_hash_table_insert (account->parameters, g_strdup (parameter),
       g_variant_ref (value));
@@ -1177,7 +1202,7 @@ test_dbus_account_plugin_set_parameter (McpAccountStorage *storage,
       g_variant_new_parsed ("(%o, %s, %v)", account->path, parameter, value),
       NULL);
 
-  return TRUE;
+  return MCP_ACCOUNT_STORAGE_SET_RESULT_CHANGED;
 }
 
 static gboolean
diff --git a/tests/twisted/mcp-account-diversion.c b/tests/twisted/mcp-account-diversion.c
index e99c6144..7bba5d8f 100644
--- a/tests/twisted/mcp-account-diversion.c
+++ b/tests/twisted/mcp-account-diversion.c
@@ -111,7 +111,7 @@ _create_config (void)
   DEBUG ("created %s", file);
 }
 
-static gboolean
+static McpAccountStorageSetResult
 _set (McpAccountStorage *self,
       McpAccountManager *am,
       const gchar *account,
@@ -121,9 +121,10 @@ _set (McpAccountStorage *self,
 {
   AccountDiversionPlugin *adp = ACCOUNT_DIVERSION_PLUGIN (self);
   gchar *val_str;
+  gboolean changed;
 
   if (g_str_has_prefix (account, DONT_DIVERT))
-      return FALSE;
+    return MCP_ACCOUNT_STORAGE_SET_RESULT_FAILED;
 
   if (val == NULL)
     {
@@ -131,7 +132,10 @@ _set (McpAccountStorage *self,
       GStrv keys;
 
       if (g_key_file_remove_key (adp->keyfile, account, key, NULL))
-        adp->save = TRUE;
+        {
+          adp->save = TRUE;
+          changed = TRUE;
+        }
 
       keys = g_key_file_get_keys (adp->keyfile, account, &n, NULL);
 
@@ -142,17 +146,30 @@ _set (McpAccountStorage *self,
     }
   else
     {
-      adp->save = TRUE;
+      gchar *old;
 
       val_str = mcp_account_manager_escape_variant_for_keyfile (am, val);
-      g_key_file_set_value (adp->keyfile, account, key, val_str);
+
+      old = g_key_file_get_value (adp->keyfile, account, key, NULL);
+
+      if (tp_strdiff (old, val_str))
+        {
+          g_key_file_set_value (adp->keyfile, account, key, val_str);
+          adp->save = TRUE;
+          changed = TRUE;
+        }
+
       g_free (val_str);
+      g_free (old);
     }
 
-  return TRUE;
+  if (changed)
+    return MCP_ACCOUNT_STORAGE_SET_RESULT_CHANGED;
+  else
+    return MCP_ACCOUNT_STORAGE_SET_RESULT_UNCHANGED;
 }
 
-static gboolean
+static McpAccountStorageSetResult
 _set_attribute (McpAccountStorage *self,
       McpAccountManager *am,
       const gchar *account,
@@ -163,7 +180,7 @@ _set_attribute (McpAccountStorage *self,
   return _set (self, am, account, attribute, val, flags);
 }
 
-static gboolean
+static McpAccountStorageSetResult
 _set_parameter (McpAccountStorage *self,
       McpAccountManager *am,
       const gchar *account,