[folks] Bug 681726 â empathy crashed with SIGABRT in g_assertion_message()

[Jeremy Whiting <jpwhiting src gnome org>]

commit d0cfa4fee7c67e744f5419f97ae08663b7afe4ff
Author: Philip Withnall <philip tecnocode co uk>
Date:   Tue Aug 28 21:48:10 2012 +0100

    Bug 681726 â empathy crashed with SIGABRT in g_assertion_message()
    
    Handle the case where calling a change_prop() method on an Individual
    doesnât find a suitable Persona to change the property on. Previously, the
    code assumed that not successfully changing the property meant an error
    mustâve been set. This was obviously wrong.
    
    This patch changes it to only try and throw an error if an error has actually
    been set. If no suitable Personas exist to write a property change to, the
    property change will fail with PropertyError.NOT_WRITEABLE.
    
    Closes: https://bugzilla.gnome.org/show_bug.cgi?id=681726

 NEWS                  |    1 +
 folks/individual.vala |  113 +++++++++++++++++++++++++++++++++----------------
 2 files changed, 77 insertions(+), 37 deletions(-)
---
diff --git a/NEWS b/NEWS
index 3b7c94b..681c1d0 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,7 @@ Bugs fixed:
 â Bug 681164 â Folks-inspect linking fails
 â Bug 680335 â empathy crashed with SIGSEGV in
   _tpf_persona_contact_weak_notify_cb()
+â Bug 681726 â empathy crashed with SIGABRT in g_assertion_message()
 
 API changes:
 â Add PropertyError.UNAVAILABLE
diff --git a/folks/individual.vala b/folks/individual.vala
index aa5715d..6bff6c7 100644
--- a/folks/individual.vala
+++ b/folks/individual.vala
@@ -160,6 +160,9 @@ public class Folks.Individual : Object,
    */
   public async void change_avatar (LoadableIcon? avatar) throws PropertyError
     {
+      /* FIXME: Once https://bugzilla.gnome.org/show_bug.cgi?id=604827 is fixed,
+       * this should be rewritten to use async delegates passed to a generic
+       * _change_single_valued_property() method. */
       if ((this._avatar != null && ((!) this._avatar).equal (avatar)) ||
           (this._avatar == null && avatar == null))
         {
@@ -169,7 +172,7 @@ public class Folks.Individual : Object,
       debug ("Setting avatar of individual '%s' to '%p'â", this.id, avatar);
 
       PropertyError? persona_error = null;
-      var avatar_changed = false;
+      var prop_changed = false;
 
       /* Try to write it to only the writeable Personas which have the
        * "avatar" property as writeable. */
@@ -188,7 +191,7 @@ public class Folks.Individual : Object,
                 {
                   yield a.change_avatar (avatar);
                   debug ("    written to writeable persona '%s'", p.uid);
-                  avatar_changed = true;
+                  prop_changed = true;
                 }
               catch (PropertyError e)
                 {
@@ -202,10 +205,17 @@ public class Folks.Individual : Object,
             }
         }
 
-      /* Failure? */
-      if (avatar_changed == false)
+      /* Failure? Changing the property failed on every suitable persona found
+       * (and potentially zero suitable personas were found). */
+      if (prop_changed == false)
         {
-          assert (persona_error != null);
+          if (persona_error == null)
+            {
+              persona_error = new PropertyError.NOT_WRITEABLE (
+                  _("Failed to change property â%sâ: No suitable personas were found."),
+                  "avatar");
+            }
+
           throw persona_error;
         }
     }
@@ -304,6 +314,9 @@ public class Folks.Individual : Object,
    */
   public async void change_alias (string alias) throws PropertyError
     {
+      /* FIXME: Once https://bugzilla.gnome.org/show_bug.cgi?id=604827 is fixed,
+       * this should be rewritten to use async delegates passed to a generic
+       * _change_single_valued_property() method. */
       if (this._alias == alias)
         {
           return;
@@ -312,7 +325,7 @@ public class Folks.Individual : Object,
       debug ("Setting alias of individual '%s' to '%s'â", this.id, alias);
 
       PropertyError? persona_error = null;
-      var alias_changed = false;
+      var prop_changed = false;
 
       /* Try to write it to only the writeable Personas which have "alias"
        * as a writeable property. */
@@ -331,7 +344,7 @@ public class Folks.Individual : Object,
                 {
                   yield a.change_alias (alias);
                   debug ("    written to writeable persona '%s'", p.uid);
-                  alias_changed = true;
+                  prop_changed = true;
                 }
               catch (PropertyError e)
                 {
@@ -345,16 +358,19 @@ public class Folks.Individual : Object,
             }
         }
 
-      /* Failure? */
-      if (alias_changed == false)
+      /* Failure? Changing the property failed on every suitable persona found
+       * (and potentially zero suitable personas were found). */
+      if (prop_changed == false)
         {
-          assert (persona_error != null);
+          if (persona_error == null)
+            {
+              persona_error = new PropertyError.NOT_WRITEABLE (
+                  _("Failed to change property â%sâ: No suitable personas were found."),
+                  "alias");
+            }
+
           throw persona_error;
         }
-
-      /* Update our copy of the alias. */
-      this._alias = alias;
-      this.notify_property ("alias");
     }
 
   private StructuredName? _structured_name = null;
@@ -400,6 +416,10 @@ public class Folks.Individual : Object,
    */
   public async void change_nickname (string nickname) throws PropertyError
     {
+      /* FIXME: Once https://bugzilla.gnome.org/show_bug.cgi?id=604827 is fixed,
+       * this should be rewritten to use async delegates passed to a generic
+       * _change_single_valued_property() method. */
+
       // Normalise null values to the empty string
       if (nickname == null)
         {
@@ -414,7 +434,7 @@ public class Folks.Individual : Object,
       debug ("Setting nickname of individual '%s' to '%s'â", this.id, nickname);
 
       PropertyError? persona_error = null;
-      var nickname_changed = false;
+      var prop_changed = false;
 
       /* Try to write it to only the writeable Personas which have "nickname"
        * as a writeable property. */
@@ -433,7 +453,7 @@ public class Folks.Individual : Object,
                 {
                   yield n.change_nickname (nickname);
                   debug ("    written to writeable persona '%s'", p.uid);
-                  nickname_changed = true;
+                  prop_changed = true;
                 }
               catch (PropertyError e)
                 {
@@ -447,16 +467,19 @@ public class Folks.Individual : Object,
             }
         }
 
-      /* Failure? */
-      if (nickname_changed == false)
+      /* Failure? Changing the property failed on every suitable persona found
+       * (and potentially zero suitable personas were found). */
+      if (prop_changed == false)
         {
-          assert (persona_error != null);
+          if (persona_error == null)
+            {
+              persona_error = new PropertyError.NOT_WRITEABLE (
+                  _("Failed to change property â%sâ: No suitable personas were found."),
+                  "nickname");
+            }
+
           throw persona_error;
         }
-
-      /* Update our copy of the nickname. */
-      this._nickname = nickname;
-      this.notify_property ("nickname");
     }
 
   private Gender _gender = Gender.UNSPECIFIED;
@@ -635,6 +658,9 @@ public class Folks.Individual : Object,
    */
   public async void change_is_favourite (bool is_favourite) throws PropertyError
     {
+      /* FIXME: Once https://bugzilla.gnome.org/show_bug.cgi?id=604827 is fixed,
+       * this should be rewritten to use async delegates passed to a generic
+       * _change_single_valued_property() method. */
       if (this._is_favourite == is_favourite)
         {
           return;
@@ -644,7 +670,7 @@ public class Folks.Individual : Object,
         is_favourite ? "TRUE" : "FALSE");
 
       PropertyError? persona_error = null;
-      var is_favourite_changed = false;
+      var prop_changed = false;
 
       /* Try to write it to only the Personas which have "is-favourite" as a
        * writeable property.
@@ -667,7 +693,7 @@ public class Folks.Individual : Object,
                 {
                   yield a.change_is_favourite (is_favourite);
                   debug ("    written to persona '%s'", p.uid);
-                  is_favourite_changed = true;
+                  prop_changed = true;
                 }
               catch (PropertyError e)
                 {
@@ -681,16 +707,19 @@ public class Folks.Individual : Object,
             }
         }
 
-      /* Failure? */
-      if (is_favourite_changed == false)
+      /* Failure? Changing the property failed on every suitable persona found
+       * (and potentially zero suitable personas were found). */
+      if (prop_changed == false)
         {
-          assert (persona_error != null);
+          if (persona_error == null)
+            {
+              persona_error = new PropertyError.NOT_WRITEABLE (
+                  _("Failed to change property â%sâ: No suitable personas were found."),
+                  "is-favourite");
+            }
+
           throw persona_error;
         }
-
-      /* Update our copy of the property. */
-      this._is_favourite = is_favourite;
-      this.notify_property ("is-favourite");
     }
 
   private HashSet<string>? _groups = null;
@@ -717,10 +746,13 @@ public class Folks.Individual : Object,
    */
   public async void change_groups (Set<string> groups) throws PropertyError
     {
+      /* FIXME: Once https://bugzilla.gnome.org/show_bug.cgi?id=604827 is fixed,
+       * this should be rewritten to use async delegates passed to a generic
+       * _change_single_valued_property() method. */
       debug ("Setting '%s' groupsâ", this.id);
 
       PropertyError? persona_error = null;
-      var groups_changed = false;
+      var prop_changed = false;
 
       /* Try to write it to only the Personas which have "groups" as a
        * writeable property. */
@@ -739,7 +771,7 @@ public class Folks.Individual : Object,
                 {
                   yield g.change_groups (groups);
                   debug ("    written to persona '%s'", p.uid);
-                  groups_changed = true;
+                  prop_changed = true;
                 }
               catch (PropertyError e)
                 {
@@ -753,10 +785,17 @@ public class Folks.Individual : Object,
             }
         }
 
-      /* Failure? */
-      if (groups_changed == false)
+      /* Failure? Changing the property failed on every suitable persona found
+       * (and potentially zero suitable personas were found). */
+      if (prop_changed == false)
         {
-          assert (persona_error != null);
+          if (persona_error == null)
+            {
+              persona_error = new PropertyError.NOT_WRITEABLE (
+                  _("Failed to change property â%sâ: No suitable personas were found."),
+                  "groups");
+            }
+
           throw persona_error;
         }
     }