telepathy: Allow for updated Tpf.Personas properties to update the cache
authorPhilip Withnall <philip@tecnocode.co.uk>
Fri, 22 Jun 2012 18:09:57 +0000 (19:09 +0100)
committerPhilip Withnall <philip@tecnocode.co.uk>
Thu, 19 Jul 2012 03:53:27 +0000 (04:53 +0100)
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
backends/telepathy/lib/tpf-persona-store.vala
backends/telepathy/lib/tpf-persona.vala
folks/individual-aggregator.vala

diff --git a/NEWS b/NEWS
index f916082..76ef52c 100644 (file)
--- 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
 =========================================================
index 4a80c51..cc2bfd1 100644 (file)
@@ -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;
     }
 
   /**
index 234621b..8f994b3 100644 (file)
@@ -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 ();
         }
     }
 
index 7774928..5e01949 100644 (file)
@@ -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