[folks/bgo629006-persona-add-edge-cases7: 2/2] Clean up the behavior of the add_persona_from_details() functions.

[Travis Reitter <treitter src gnome org>]

commit 7d96b351e766e15e857d022b6937abdc0be05356
Author: Travis Reitter <travis reitter collabora co uk>
Date:   Tue Sep 7 16:11:30 2010 -0700

    Clean up the behavior of the add_persona_from_details() functions.
    
    This clarifies when an add fails for a temporary reason (eg, the store is
    offline) and can be re-attempted.
    
    Fixes bgo#629006.

 NEWS                                          |    4 +++
 backends/telepathy/lib/tpf-persona-store.vala |   29 +++++++++++++++++-------
 folks/individual-aggregator.vala              |   27 ++++++++++++++++++-----
 folks/persona-store.vala                      |   20 ++++++++++++++--
 4 files changed, 62 insertions(+), 18 deletions(-)
---
diff --git a/NEWS b/NEWS
index a931cbe..bb31260 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,8 @@ API changes:
 * Added Persona::is-user
 * Added IndividualAggregator::user
 * Added PersonaStoreError.UNSUPPORTED_ON_USER
+* Added {IndividualAggregator, PersonaStoreError}.STORE_OFFLINE, used by the
+  respective add_persona_from_details() functions
 
 Bugs fixed:
 * Bug 629452 â?? [Patch] Add missing gio linking for import-tool
@@ -38,6 +40,8 @@ Bugs fixed:
 * Bug 627402 â?? Support marking FolksPersonas as "me"
 * Bug 629642 â?? individuals-changed emitted in the wrong order
 * Bug 629643 â?? do not fall back to the id if alias is empty
+* Bug 629006 â?? PersonaStore should gracefully handle offline Persona change
+  attempts
 
 Overview of changes from libfolks 0.1.16 to libfolks 0.1.17
 ===========================================================
diff --git a/backends/telepathy/lib/tpf-persona-store.vala b/backends/telepathy/lib/tpf-persona-store.vala
index 637a1d6..0dedb92 100644
--- a/backends/telepathy/lib/tpf-persona-store.vala
+++ b/backends/telepathy/lib/tpf-persona-store.vala
@@ -1186,6 +1186,15 @@ public class Tpf.PersonaStore : Folks.PersonaStore
               this.type_id, this.id, contact_id);
         }
 
+      var status = this.account.get_connection_status (null);
+      if ((status == TelepathyGLib.ConnectionStatus.DISCONNECTED) ||
+          (status == TelepathyGLib.ConnectionStatus.CONNECTING) ||
+          this.conn == null)
+        {
+          throw new PersonaStoreError.STORE_OFFLINE ("cannot create a new " +
+              "Tpf.Persona while offline");
+        }
+
       string[] contact_ids = new string[1];
       contact_ids[0] = contact_id;
 
@@ -1194,7 +1203,12 @@ public class Tpf.PersonaStore : Folks.PersonaStore
           var personas = yield create_personas_from_contact_ids (
               contact_ids);
 
-          if (personas != null && personas.length () == 1)
+          if (personas == null)
+            {
+              /* the persona already existed */
+              return null;
+            }
+          else if (personas.length () == 1)
             {
               var persona = personas.data;
 
@@ -1215,20 +1229,17 @@ public class Tpf.PersonaStore : Folks.PersonaStore
 
               return persona;
             }
-          else if (personas != null && personas.length () > 1)
+          else
             {
-              /* We ignore the case of an empty list, as it just means the
-               * contact was already in our roster */
-              warning ("requested a single persona, but got %u back",
-                  personas.length ());
+              throw new PersonaStoreError.CREATE_FAILED ("requested a single " +
+                  "persona, but got %u back", personas.length ());
             }
         }
       catch (GLib.Error e)
         {
-          warning ("failed to add a persona from details: %s", e.message);
+          throw new PersonaStoreError.CREATE_FAILED ("failed to add a " +
+              "persona from details: %s", e.message);
         }
-
-      return null;
     }
 
   /**
diff --git a/folks/individual-aggregator.vala b/folks/individual-aggregator.vala
index 50a09cb..ee1bef1 100644
--- a/folks/individual-aggregator.vala
+++ b/folks/individual-aggregator.vala
@@ -645,22 +645,30 @@ public class Folks.IndividualAggregator : Object
    * Add a new persona in the given { link PersonaStore} based on the `details`
    * provided.
    *
+   * If the target store is offline, this function will throw
+   * { link IndividualAggregatorError.STORE_OFFLINE}. It's the responsibility of
+   * the caller to cache details and re-try this function if it wishes to make
+   * offline adds work.
+   *
    * The details hash is a backend-specific mapping of key, value strings.
    * Common keys include:
    *
    *  * contact - service-specific contact ID
    *
-   * If `parent` is provided, the new persona will be appended to its ordered
-   * list of personas.
+   * If a { link Persona} with the given details already exists in the store, no
+   * error will be thrown and this function will return `null`.
    *
    * @param parent an optional { link Individual} to add the new { link Persona}
-   * to
+   * to. This persona will be appended to its ordered list of personas.
    * @param persona_store_type the { link PersonaStore.type_id} of the
    * { link PersonaStore} to use
    * @param persona_store_id the { link PersonaStore.id} of the
    * { link PersonaStore} to use
    * @param details a key-value map of details to use in creating the new
    * { link Persona}
+   * @return the new { link Persona} or `null` if the corresponding
+   * { link Persona} already existed. If non-`null`, the new { link Persona}
+   * will also be added to a new or existing { link Individual} as necessary.
    */
   public async Persona? add_persona_from_details (Individual? parent,
       string persona_store_type,
@@ -686,9 +694,16 @@ public class Folks.IndividualAggregator : Object
         }
       catch (PersonaStoreError e)
         {
-          throw new IndividualAggregatorError.ADD_FAILED (
-              "failed to add contact for store type '%s', ID '%s': %s",
-              persona_store_type, persona_store_id, e.message);
+          if (e is PersonaStoreError.STORE_OFFLINE)
+            {
+              throw new IndividualAggregatorError.STORE_OFFLINE (e.message);
+            }
+          else
+            {
+              throw new IndividualAggregatorError.ADD_FAILED (
+                  "failed to add contact for store type '%s', ID '%s': %s",
+                  persona_store_type, persona_store_id, e.message);
+            }
         }
 
       if (parent != null && persona != null)
diff --git a/folks/persona-store.vala b/folks/persona-store.vala
index b860e81..2d865ef 100644
--- a/folks/persona-store.vala
+++ b/folks/persona-store.vala
@@ -243,15 +243,29 @@ public abstract class Folks.PersonaStore : Object
    * The { link Persona} will be created by the PersonaStore backend from the
    * key-value pairs given in `details`. FIXME: These are backend-specific.
    *
+   * All additions through this function will later be emitted through the
+   * personas-changed signal to be notified of the new { link Persona}. The
+   * return value is purely for convenience, since it can be complicated to
+   * correlate the provided details with the final Persona.
+   *
+   * If the store is offline, this function will throw
+   * { link PersonaStoreError.STORE_OFFLINE}. It's the responsibility of the
+   * caller to cache details and re-try this function if it wishes to make
+   * offline adds work.
+   *
    * If the details are not recognised or are invalid,
    * { link PersonaStoreError.INVALID_ARGUMENT} will be thrown.
    *
-   * If a { link Persona} with the given details already exists in the store,
-   * `null` will be returned, but no error will be thrown.
+   * If a { link Persona} with the given details already exists in the store, no
+   * error will be thrown and this function will return `null`.
    *
    * @param details a key-value map of details to use in creating the new
    * { link Persona}
-   * @return the new { link Persona}, or `null` on failure
+   *
+   * @return the new { link Persona} or `null` if the corresponding Persona
+   * already existed. If non-`null`, the new { link Persona} will also be
+   * amongst the { link Persona}(s) in a future emission of
+   * { link PersonaStore.personas_changed}.
    */
   public abstract async Persona? add_persona_from_details (
       HashTable<string, Value?> details) throws Folks.PersonaStoreError;