[folks] Bug 659128 â If a persona store goes away we don't remove its personas

[Philip Withnall <pwithnall src gnome org>]

commit 7d4d0100b22159e10fe76c719bf4f38b24d35057
Author: Philip Withnall <philip tecnocode co uk>
Date:   Thu Sep 15 22:09:36 2011 +0100

    Bug 659128 â If a persona store goes away we don't remove its personas
    
    Handle removal of the address books backing Edsf.PersonaStores by notifying
    of the removal of all the personas in the store, then emitting
    PersonaStore.removed.
    
    Test included.
    
    Closes: bgo#659128

 NEWS                                     |    1 +
 backends/eds/eds-backend.vala            |   46 ++-----
 backends/eds/lib/edsf-persona-store.vala |   58 +++++++++
 tests/eds/Makefile.am                    |    6 +
 tests/eds/store-removed.vala             |  204 ++++++++++++++++++++++++++++++
 tests/lib/eds/backend.vala               |    5 +
 6 files changed, 286 insertions(+), 34 deletions(-)
---
diff --git a/NEWS b/NEWS
index dce01c5..3c1d4ec 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,7 @@ Bugs fixed:
 * Bug 657065 â Cache keeps around contacts from disabled accounts
 * Bug 658323 â Deprecate FOLKS_WRITEABLE_STORE in favour of FOLKS_PRIMARY_STORE
 * Bug 659095 â Don't distribute typelib file
+* Bug 659128 â If a persona store goes away we don't remove its personas
 
 API changes:
 * Individual.avatar is now settable using Individual.change_avatar() (not new
diff --git a/backends/eds/eds-backend.vala b/backends/eds/eds-backend.vala
index dd2aea9..b4549e5 100644
--- a/backends/eds/eds-backend.vala
+++ b/backends/eds/eds-backend.vala
@@ -158,10 +158,10 @@ public class Folks.Backends.Eds.Backend : Folks.Backend
 
       debug ("Address book source list changed.");
 
-      /* Collapse the updated list of groups down into a set of current address
-       * books we're interested in, excluding the ones currently in the
-       * backend. */
-      var new_sources = new HashMap<string, E.Source> ();
+      /* Add address books which didn't previously exist in the backend.
+       * We don't deal with removals here: see _source_list_changed_cb() in
+       * Edsf.PersonaStore for that. */
+      var added_sources = new LinkedList<E.Source> ();
 
       foreach (var g in groups)
         {
@@ -175,42 +175,20 @@ public class Folks.Backends.Eds.Backend : Folks.Backend
                   continue;
                 }
 
-              new_sources.set (s.peek_relative_uri (), s);
-            }
-        }
-
-      /* Remove address books which no longer exist from the backend. */
-      var removed_sources = new LinkedList<string> ();
-
-      foreach (var source_uri in this._persona_stores.keys)
-        {
-          if (!new_sources.has_key (source_uri))
-            {
-              removed_sources.add (source_uri);
-            }
-        }
+              var source_uri = s.peek_relative_uri ();
 
-      /* Add address books which didn't previously exist in the backend. */
-      var added_sources = new LinkedList<string> ();
-
-      foreach (var source_uri in new_sources.keys)
-        {
-          if (!this._persona_stores.has_key (source_uri))
-            {
-              added_sources.add (source_uri);
+              if (!this._persona_stores.has_key (source_uri))
+                {
+                  added_sources.add (s);
+                }
             }
         }
 
       /* Actually apply the changes to our state. We can't do this any earlier
-       * or we'll mess up the calculation of what's been added and removed. */
-      foreach (var source_uri in removed_sources)
-        {
-          this._remove_address_book (this._persona_stores.get (source_uri));
-        }
-
-      foreach (var source_uri in added_sources)
+       * or we'll mess up the calculation of what's been added. */
+      foreach (var source in added_sources)
         {
-          this._add_address_book (new_sources.get (source_uri));
+          this._add_address_book (source);
         }
     }
 
diff --git a/backends/eds/lib/edsf-persona-store.vala b/backends/eds/lib/edsf-persona-store.vala
index 3e3f73f..687e83e 100644
--- a/backends/eds/lib/edsf-persona-store.vala
+++ b/backends/eds/lib/edsf-persona-store.vala
@@ -41,6 +41,7 @@ public class Edsf.PersonaStore : Folks.PersonaStore
   private E.BookClient _addressbook;
   private E.BookClientView _ebookview;
   private string _addressbook_uri = null;
+  private E.SourceList? _source_list = null;
   private E.Source _source;
   private string _query_str;
 
@@ -234,6 +235,13 @@ public class Edsf.PersonaStore : Folks.PersonaStore
 
               this._addressbook = null;
             }
+
+          if (this._source_list != null)
+            {
+              this._source_list.changed.disconnect (
+                  this._source_list_changed_cb);
+              this._source_list = null;
+            }
         }
       catch (GLib.Error e)
         {
@@ -525,6 +533,13 @@ public class Edsf.PersonaStore : Folks.PersonaStore
 
           try
             {
+              /* Listen for removal signals for the address book. There's no
+               * need to check if we still exist in the list, as
+               * addressbook.open() will fail if we don't. */
+              E.BookClient.get_sources (out this._source_list);
+              this._source_list.changed.connect (this._source_list_changed_cb);
+
+              /* Connect to the address book. */
               this._addressbook = new E.BookClient (this._source);
 
               this._addressbook.notify["readonly"].connect (
@@ -1763,4 +1778,47 @@ public class Edsf.PersonaStore : Folks.PersonaStore
           _("Unknown error setting property â%sâ: %s"), property_name,
           error_in.message);
     }
+
+  private bool _is_in_source_list ()
+    {
+      unowned GLib.SList<weak E.SourceGroup> groups =
+          this._source_list.peek_groups ();
+
+      foreach (var g in groups)
+        {
+          foreach (var s in g.peek_sources ())
+            {
+              if (s.peek_relative_uri () == this.id)
+                {
+                  /* We've found ourself. */
+                  return true;
+                }
+            }
+        }
+
+      return false;
+    }
+
+  /* Detect removal of the address book. We can't do this in Eds.Backend because
+   * it has no way to tell the PersonaStore that it's been removed without
+   * uglifying the store's public API. */
+  private void _source_list_changed_cb (E.SourceList list)
+    {
+      /* If we can't find our source, this persona store's address book has
+       * been removed. */
+      if (this._is_in_source_list () == false)
+        {
+          /* Marshal the personas from a Collection to a Set. */
+          var removed_personas = new HashSet<Persona> ();
+          var iter = this._personas.map_iterator ();
+
+          while (iter.next () == true)
+            {
+              removed_personas.add (iter.get_value ());
+            }
+
+          this._emit_personas_changed (null, removed_personas);
+          this.removed ();
+        }
+    }
 }
diff --git a/tests/eds/Makefile.am b/tests/eds/Makefile.am
index e114dc4..287dca0 100644
--- a/tests/eds/Makefile.am
+++ b/tests/eds/Makefile.am
@@ -43,6 +43,7 @@ AM_VALAFLAGS = \
 
 # in order from least to most complex
 noinst_PROGRAMS = \
+	store-removed \
 	persona-store-tests \
 	individual-retrieval \
 	phone-details \
@@ -88,6 +89,10 @@ TESTS_ENVIRONMENT = \
 
 TESTS = $(noinst_PROGRAMS)
 
+store_removed_SOURCES = \
+	store-removed.vala \
+	$(NULL)
+
 persona_store_tests_SOURCES = \
 	persona-store-tests.vala \
 	$(NULL)
@@ -204,6 +209,7 @@ CLEANFILES = \
 
 MAINTAINERCLEANFILES = \
         $(addsuffix .c,$(noinst_PROGRAMS)) \
+        store_removed_vala.stamp \
         persona_store_tests_vala.stamp \
         individual_retrieval_vala.stamp \
         removing_contacts_vala.stamp \
diff --git a/tests/eds/store-removed.vala b/tests/eds/store-removed.vala
new file mode 100644
index 0000000..e154c04
--- /dev/null
+++ b/tests/eds/store-removed.vala
@@ -0,0 +1,204 @@
+/*
+ * Copyright (C) 2011 Philip Withnall
+ *
+ * This library is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this library.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors: Philip Withnall <philip tecnocode co uk>
+ *
+ */
+
+using EdsTest;
+using Folks;
+using Gee;
+
+public class StoreRemovedTests : Folks.TestCase
+{
+  private EdsTest.Backend _eds_backend;
+  private GLib.MainLoop _main_loop;
+  private IndividualAggregator _aggregator;
+
+  public StoreRemovedTests ()
+    {
+      base ("StoreRemoved");
+
+      this._eds_backend = new EdsTest.Backend ();
+
+      this.add_test ("single store", this.test_single_store);
+    }
+
+  public override void set_up ()
+    {
+      this._eds_backend.set_up ();
+    }
+
+  public override void tear_down ()
+    {
+      this._eds_backend.tear_down ();
+    }
+
+  public void test_single_store ()
+    {
+      this._main_loop = new GLib.MainLoop (null, false);
+
+      this._eds_backend.reset ();
+
+      /* Create a contact in the address book. */
+      var c1 = new Gee.HashMap<string, Value?> ();
+
+      Value? v = Value (typeof (string));
+      v.set_string ("Chancellor Brian Blessed");
+      c1.set ("full_name", (owned) v);
+
+      v = Value (typeof (string));
+      v.set_string ("brian example com");
+      c1.set (Edsf.Persona.email_fields[0], (owned) v);
+
+      this._eds_backend.add_contact (c1);
+
+      /* Schedule the test to start with the main loop. */
+      this._test_single_store_part1_async ();
+
+      var timeout_id = Timeout.add_seconds (5, () =>
+        {
+          critical ("Timeout reached.");
+          return false;
+        });
+
+      this._main_loop.run ();
+
+      /* We should have a single individual by now. */
+      assert (this._aggregator.individuals.size == 1);
+
+      Source.remove (timeout_id);
+
+      /* Part 2, where we remove the address book. */
+      this._test_single_store_part2_async ();
+
+      timeout_id = Timeout.add_seconds (5, () =>
+        {
+          critical ("Timeout reached.");
+          return false;
+        });
+
+      this._main_loop.run ();
+
+      /* The individual should be gone. */
+      assert (this._aggregator.individuals.size == 0);
+
+      Source.remove (timeout_id);
+    }
+
+  private async void _test_single_store_part1_async ()
+    {
+      yield this._eds_backend.commit_contacts_to_addressbook ();
+
+      var store = BackendStore.dup ();
+      yield store.prepare ();
+
+      this._aggregator = new IndividualAggregator ();
+
+      ulong signal_id = 0;
+      signal_id = this._aggregator.individuals_changed_detailed.connect (
+          (changes) =>
+        {
+          var added = changes.get_values ();
+          var removed = changes.get_keys ();
+
+          assert (added.size == 1);
+
+          /* We don't really care what the individual is, as long as it's not
+           * null. */
+          foreach (var i in added)
+            {
+              assert (i != null);
+            }
+
+          assert (removed.size == 1);
+
+          foreach (var i in removed)
+            {
+              assert (i == null);
+            }
+
+          /* Success! */
+          this._main_loop.quit ();
+          this._aggregator.disconnect (signal_id);
+        });
+
+      try
+        {
+          yield this._aggregator.prepare ();
+        }
+      catch (GLib.Error e)
+        {
+          critical ("Error when calling prepare: %s", e.message);
+        }
+    }
+
+  private async void _test_single_store_part2_async ()
+    {
+      ulong signal_id = 0;
+      signal_id = this._aggregator.individuals_changed_detailed.connect (
+          (changes) =>
+        {
+          var added = changes.get_values ();
+          var removed = changes.get_keys ();
+
+          assert (added.size == 1);
+
+          foreach (var i in added)
+            {
+              assert (i == null);
+            }
+
+          assert (removed.size == 1);
+
+          /* We don't really care what the individual is, as long as it's not
+           * null. */
+          foreach (var i in removed)
+            {
+              assert (i != null);
+            }
+
+          /* Success! */
+          this._main_loop.quit ();
+          this._aggregator.disconnect (signal_id);
+        });
+
+      /* Tear down the backend. This should remove all individuals. We check
+       * for this above. */
+      E.SourceList? source_list = null;
+      try
+        {
+          E.BookClient.get_sources (out source_list);
+          source_list.remove_source_by_uid (this._eds_backend.address_book_uid);
+        }
+      catch (GLib.Error e1)
+        {
+          critical ("Error getting source list: %s", e1.message);
+        }
+    }
+}
+
+public int main (string[] args)
+{
+  Test.init (ref args);
+
+  TestSuite root = TestSuite.get_root ();
+  root.add_suite (new StoreRemovedTests ().get_suite ());
+
+  Test.run ();
+
+  return 0;
+}
diff --git a/tests/lib/eds/backend.vala b/tests/lib/eds/backend.vala
index dd2c174..5e9d970 100644
--- a/tests/lib/eds/backend.vala
+++ b/tests/lib/eds/backend.vala
@@ -44,6 +44,11 @@ public class EdsTest.Backend
       get; set; default = "local://test";
     }
 
+  public string address_book_uid
+    {
+      get { return this._addressbook.get_source ().peek_uid (); }
+    }
+
   public Backend ()
     {
       this._contacts = new GLib.List<Gee.HashMap<string, Value?>> ();