[folks] telepathy: Allow for updated Tpf.Personas properties to update the cache

[Philip Withnall <pwithnall src gnome org>]

commit 36ecd01a58b952c40c985166aa74ededda7fcefc
Author: Philip Withnall <philip tecnocode co uk>
Date:   Fri Jun 22 19:09:57 2012 +0100

    telepathy: Allow for updated Tpf.Personas properties to update the cache
    
    Currently, the cache is only written when going offline, which means that any
    properties of Tpf.Personas which are updated at runtime arenât written to the
    cache unless the client explicitly goes offline before being closed. This
    doesnât often happen, meaning the property updates are lost and the cache
    becomes stale.
    
    This commit keeps track of the clean/dirty state of the cache and writes it
    out when PersonaStore.flush() is called. It also adds a new method,
    IndividualAggregator.unprepare(), which ensures all the persona stores handled
    by the aggregator are flushed. Clients should call this method before closing
    their main loop.
    
    (Calling flush() in the finalise function of the PersonaStore doesnât work
    because Tpf.PersonaStores are often never finalised due to the implementation
    of Tpf.PersonaStore.dup_for_account(). Furthermore, calling flush() in the
    finalise function of the IndividualAggregator doesnât work because the client
    will typically quit the main loop immediately afterwards, which will cancel
    the asynchronous flush call.)
    
    New API:
     â IndividualAggregator.unprepare()
    
    Helps: https://bugzilla.gnome.org/show_bug.cgi?id=660128

 NEWS                                          |    2 +
 backends/telepathy/lib/tpf-persona-store.vala |   72 +++++++++++++++++++-----
 backends/telepathy/lib/tpf-persona.vala       |   24 ++++++++
 folks/individual-aggregator.vala              |   43 +++++++++++++++
 4 files changed, 126 insertions(+), 15 deletions(-)
---
diff --git a/NEWS b/NEWS
index f916082..76ef52c 100644
--- a/NEWS
+++ b/NEWS
@@ -11,10 +11,12 @@ Bugs fixed:
 â Bug 678696 â Add API for EDS Google system groups
 â Bug 669158 â Readonly/Offline issues
 â Bug 675223 â Shouldn't warn if CM does not implement any contact list
+â Bug 660128 â Most contacts don't have an avatar
 
 API changes:
 â Add AntiLinkable interface and implement it on Kf.Persona and Edsf.Persona
 â Add Edsf.Persona.in_google_personal_group
+â Add IndividualAggregator.unprepare()
 
 Overview of changes from libfolks 0.7.1 to libfolks 0.7.2
 =========================================================
diff --git a/backends/telepathy/lib/tpf-persona-store.vala b/backends/telepathy/lib/tpf-persona-store.vala
index 4a80c51..cc2bfd1 100644
--- a/backends/telepathy/lib/tpf-persona-store.vala
+++ b/backends/telepathy/lib/tpf-persona-store.vala
@@ -77,6 +77,10 @@ public class Tpf.PersonaStore : Folks.PersonaStore
   private Debug _debug;
   private PersonaStoreCache _cache;
   private Cancellable? _load_cache_cancellable = null;
+  /* Whether data in memory is dirty and needs flushing to the cache at some
+   * point. For example, this will be true if a contact has updated their avatar
+   * while we've been online. */
+  private bool _cache_needs_update = false;
 
   /* marshalled from ContactInfo.SupportedFields */
   internal HashSet<string> _supported_fields;
@@ -378,6 +382,7 @@ public class Tpf.PersonaStore : Folks.PersonaStore
       this._personas = new HashMap<string, Persona> ();
       this._personas_ro = this._personas.read_only_view;
       this._persona_set = new HashSet<Persona> ();
+      this._cache_needs_update = false;
 
       if (this._conn != null)
         {
@@ -657,26 +662,26 @@ public class Tpf.PersonaStore : Folks.PersonaStore
               /* Call reset immediately, otherwise TpConnection's invalidation
                * will cause all contacts to weak notify. See bug #675141 */
               var old_personas = this._persona_set;
+              var old_cache_needs_update = this._cache_needs_update;
               this._reset ();
 
-              /* Only store/load the cache if the account is enabled and valid;
-               * otherwise, the PersonaStore will get removed and the cache
-               * deleted later anyway. */
-              if (this.account.enabled && this.account.valid)
+              if (old_cache_needs_update)
                 {
-                  this._store_cache.begin (old_personas, (o, r) =>
-                    {
-                      this._store_cache.end (r);
+                  this._set_cache_needs_update ();
+                }
+
+              this._store_cache.begin (old_personas, (o, r) =>
+                {
+                  this._store_cache.end (r);
 
-                      if (this.account.enabled && this.account.valid)
+                  if (this.account.enabled && this.account.valid)
+                    {
+                      this._load_cache.begin (old_personas, (o2, r2) =>
                         {
-                          this._load_cache.begin (old_personas, (o2, r2) =>
-                            {
-                              this._load_cache.end (r2);
-                            });
-                        }
-                    });
-                }
+                          this._load_cache.end (r2);
+                        });
+                    }
+                });
             }
 
           /* If the persona store starts offline, we've reached a quiescent
@@ -896,15 +901,52 @@ public class Tpf.PersonaStore : Folks.PersonaStore
       this.notify_property ("always-writeable-properties");
     }
 
+  public override async void flush ()
+    {
+      debug ("Flushing Tpf.PersonaStore %p (â%sâ).", this, this.id);
+
+      /* Store the cache if it needs an update. */
+      yield this._store_cache (this._persona_set);
+    }
+
+  /**
+   * Called when a contact property is set which is not accessible when either
+   * the contact is offline or we're offline. For example, a contact's avatar.
+   * This will cause the cache to be stored when the PersonaStore is destroyed.
+   */
+  internal void _set_cache_needs_update ()
+    {
+      debug ("Setting cache as needing an update for Tpf.PersonaStore " +
+          "%p (â%sâ).", this, this.id);
+      this._cache_needs_update = true;
+    }
+
   /**
    * When we're about to disconnect, store the current set of personas to the
    * cache file so that we can access them once offline.
    */
   private async void _store_cache (HashSet<Persona> old_personas)
     {
+      /* Only store/load the cache if the account is enabled and valid;
+       * otherwise, the PersonaStore will get removed and the cache
+       * deleted later anyway. */
+      if (!this.account.enabled || !this.account.valid)
+        {
+          debug ("Skipping storing cache for Tpf.PersonaStore %p (â%sâ) as " +
+              "its TpAccount is disabled or invalid.", this, this.id);
+          return;
+        }
+      else if (this._cache_needs_update == false)
+        {
+          debug ("Skipping storing cache for Tpf.PersonaStore %p (â%sâ) as " +
+              "it doesnât need an update.", this, this.id);
+          return;
+        }
+
       debug ("Storing cache for Tpf.PersonaStore %p ('%s').", this, this.id);
 
       yield this._cache.store_objects (old_personas);
+      this._cache_needs_update = false;
     }
 
   /**
diff --git a/backends/telepathy/lib/tpf-persona.vala b/backends/telepathy/lib/tpf-persona.vala
index 234621b..8f994b3 100644
--- a/backends/telepathy/lib/tpf-persona.vala
+++ b/backends/telepathy/lib/tpf-persona.vala
@@ -341,6 +341,9 @@ public class Tpf.Persona : Folks.Persona,
 
       this._is_favourite = is_favourite;
       this.notify_property ("is-favourite");
+
+      /* Mark the cache as needing to be updated. */
+      ((Tpf.PersonaStore) this.store)._set_cache_needs_update ();
     }
 
   private HashSet<EmailFieldDetails> _email_addresses =
@@ -522,6 +525,9 @@ public class Tpf.Persona : Folks.Persona,
       if (changed == true)
         {
           this.notify_property ("groups");
+
+          /* Mark the cache as needing to be updated. */
+          ((Tpf.PersonaStore) this.store)._set_cache_needs_update ();
         }
     }
 
@@ -742,6 +748,9 @@ public class Tpf.Persona : Folks.Persona,
               {
                 this._alias = this.contact.alias;
                 this.notify_property ("alias");
+
+                /* Mark the cache as needing to be updated. */
+                ((Tpf.PersonaStore) this.store)._set_cache_needs_update ();
               }
           });
 
@@ -842,6 +851,7 @@ public class Tpf.Persona : Folks.Persona,
 
   private void _contact_notify_contact_info ()
     {
+      var changed = false;
       var new_birthday_str = "";
       var new_full_name = "";
       var new_email_addresses = new HashSet<EmailFieldDetails> (
@@ -918,6 +928,7 @@ public class Tpf.Persona : Folks.Persona,
                 {
                   this._birthday = d.to_utc ();
                   this.notify_property ("birthday");
+                  changed = true;
                 }
             }
           else
@@ -932,6 +943,7 @@ public class Tpf.Persona : Folks.Persona,
             {
               this._birthday = null;
               this.notify_property ("birthday");
+              changed = true;
             }
         }
 
@@ -941,12 +953,14 @@ public class Tpf.Persona : Folks.Persona,
           this._email_addresses = new_email_addresses;
           this._email_addresses_ro = new_email_addresses.read_only_view;
           this.notify_property ("email-addresses");
+          changed = true;
         }
 
       if (new_full_name != this._full_name)
         {
           this._full_name = new_full_name;
           this.notify_property ("full-name");
+          changed = true;
         }
 
       if (!Folks.Internal.equal_sets<PhoneFieldDetails> (new_phone_numbers,
@@ -955,6 +969,7 @@ public class Tpf.Persona : Folks.Persona,
           this._phone_numbers = new_phone_numbers;
           this._phone_numbers_ro = new_phone_numbers.read_only_view;
           this.notify_property ("phone-numbers");
+          changed = true;
         }
 
       if (!Folks.Internal.equal_sets<UrlFieldDetails> (new_urls, this._urls))
@@ -962,6 +977,13 @@ public class Tpf.Persona : Folks.Persona,
           this._urls = new_urls;
           this._urls_ro = new_urls.read_only_view;
           this.notify_property ("urls");
+          changed = true;
+        }
+
+      if (changed == true)
+        {
+          /* Mark the cache as needing to be updated. */
+          ((Tpf.PersonaStore) this.store)._set_cache_needs_update ();
         }
     }
 
@@ -1148,6 +1170,8 @@ public class Tpf.Persona : Folks.Persona,
         {
           this._avatar = (LoadableIcon) icon;
           this.notify_property ("avatar");
+          /* Mark the persona cache as needing to be updated. */
+          ((Tpf.PersonaStore) this.store)._set_cache_needs_update ();
         }
     }
 
diff --git a/folks/individual-aggregator.vala b/folks/individual-aggregator.vala
index 7774928..5e01949 100644
--- a/folks/individual-aggregator.vala
+++ b/folks/individual-aggregator.vala
@@ -565,6 +565,49 @@ public class Folks.IndividualAggregator : Object
     }
 
   /**
+   * Clean up and release resources used by the aggregator.
+   *
+   * This will disconnect the aggregator cleanly from any resources it or its
+   * persona stores are using. It is recommended to call this method before
+   * finalising the individual aggregator, but calling it is not required. If
+   * this method is not called then, for example, unsaved changes in backends
+   * may not be flushed.
+   *
+   * Concurrent calls to this function from different threads will block until
+   * preparation has completed. However, concurrent calls to this function from
+   * a single thread might not, i.e. the first call will block but subsequent
+   * calls might return before the first one. (Though they will be safe in every
+   * other respect.)
+   *
+   * @since UNRELEASED
+   * @throws GLib.Error if unpreparing the backend-specific services failed â
+   * this will be a backend-specific error
+   */
+  public async void unprepare () throws GLib.Error
+    {
+      lock (this._is_prepared)
+        {
+          if (!this._is_prepared || this._prepare_pending)
+            {
+              return;
+            }
+
+          try
+            {
+              /* Flush any PersonaStores which need it. */
+              foreach (var p in this._stores.values)
+                {
+                  yield p.flush ();
+                }
+            }
+          finally
+            {
+              this._prepare_pending = false;
+            }
+        }
+    }
+
+  /**
    * Get all matches for a given { link Individual}.
    *
    * @param matchee the individual to find matches for