Merge lp:~mhr3/libunity/remote-scope-sources into lp:libunity

[Michal Hruby]

Fix sources filter when using remote scopes.

[Mikkel Kamstrup Erlandsen (kamstrup)]

68 - public OptionsFilter sources { get; internal set; }
69 + public OptionsFilter sources { get; private set; }

Technicaly this is an ABI break. Although only in internal ABI. If it's just a cleanup I suggest we leave it as is to not trigger the alarm bells of the packaging system.

$ nm -D src/.libs/libunity.so | grep set_sources
0001d931 T unity_lens_set_sources_display_name
000483ac T unity_scope_proxy_set_sources
$ nm -D /usr/lib/libunity.so | grep set_sources
00017140 T unity_lens_set_sources_display_name
00037c80 T unity_scope_proxy_set_sources
0002d350 T unity_scope_set_sources

review: Needs Fixing

[lp:~mhr3/libunity/remote-scope-sources]

113. By Michal Hruby on 2012-01-23

Revert ABI break

[Michal Hruby (mhr3)]

Reverted, unfortunately "internal set;" marks the property as writeable, which is not desired.

[Mikkel Kamstrup Erlandsen (kamstrup)]

Point taken. If you want to remove it take it up with Didier. It's unlikely to be an issue; we just need to make sure nobody panics ;-)

review: Approve

=== modified file 'src/unity-lens-filters.vala'
--- src/unity-lens-filters.vala	2012-01-17 19:36:24 +0000
+++ src/unity-lens-filters.vala	2012-01-23 13:23:30 +0000
@@ -211,7 +211,7 @@
     
     if (element != null)
     {
-      var free_it = (owned) element.data;
+      element.data = null;
       options.delete_link (element);
 
       this.changed ();
@@ -245,6 +245,8 @@
     VariantIter iter;
     array.get("a(sssb)", out iter);
 
+    string[] option_ids = {};
+
     for (var i = 0; i < iter.n_children(); i++)
     {
       string b_id;
@@ -255,6 +257,25 @@
       iter.next("(sssb)", out b_id, out b_name, out b_icon_hint, out b_active);
 
       find_and_update_option(b_id, b_name, b_icon_hint, b_active);
+      option_ids += b_id;
+    }
+
+    if (options.length () != option_ids.length)
+    {
+      // something got removed, remove it from the List (and don't use
+      // remove_option() cause it emits changed()
+      unowned List<FilterOption> l = options;
+      while (l != null)
+      {
+        unowned string id = l.data.id;
+        unowned List<FilterOption> to_remove = l;
+        l = l.next;
+        if (!(id in option_ids))
+        {
+          to_remove.data = null;
+          options.delete_link (to_remove);
+        }
+      }
     }
   }
 

=== modified file 'src/unity-scope-proxy-remote.vala'
--- src/unity-scope-proxy-remote.vala	2012-01-06 13:04:27 +0000
+++ src/unity-scope-proxy-remote.vala	2012-01-23 13:23:30 +0000
@@ -278,6 +278,7 @@
 
     /* extract the sources */
     sources.update (scope_info.sources);
+    this.notify_property ("sources");
 
     if (!synchronized)
     {

=== modified file 'src/unity-scope.vala'
--- src/unity-scope.vala	2012-01-09 11:19:46 +0000
+++ src/unity-scope.vala	2012-01-23 13:23:30 +0000
@@ -63,8 +63,7 @@
    
   public Scope (string dbus_path_)
   {
-    Object (dbus_path: dbus_path_,
-            sources: new CheckOptionFilter ("sources", "Sources", null, true));
+    Object (dbus_path: dbus_path_);
   }
 
   construct

=== modified file 'test/vala/test-lens.vala'
--- test/vala/test-lens.vala	2012-01-06 14:17:45 +0000
+++ test/vala/test-lens.vala	2012-01-23 13:23:30 +0000
@@ -463,23 +463,92 @@
     bool got_search_changed = false;
     local_scope.sources.add_option ("id1", "Source1", null);
     local_scope.sources.add_option ("id2", "Source2", null);
+
     var ml = new MainLoop ();
-
     // wait a bit for the model to update
-    Timeout.add (500, () => { ml.quit (); return false; });
+    Idle.add (() => { ml.quit (); return false; });
+    ml.run ();
+    ml = new MainLoop ();
+
+    ulong sig_id = local_scope.search_changed.connect ((lens_search) =>
+    {
+      assert (lens_search.search_string.has_prefix ("sources-test"));
+      lens_search.finished ();
+    });
+    // do a search, so we can sync with the remote scope
+    call_lens_search ("sources-test", () => { ml.quit (); });
+    ml.run ();
+    ml = new MainLoop ();
+    // after this the sources *have* to be updated
+    Idle.add (() => { ml.quit (); return false; });
     ml.run ();
 
     bool found1 = false;
     bool found2 = false;
+    bool remote1 = false;
+    bool remote2 = false;
     foreach (var filter_option in exported_lens.get_sources_internal ().options)
     {
       if (filter_option.id.has_suffix ("id1") && filter_option.id != "id1")
         found1 = true;
       else if (filter_option.id.has_suffix ("id2") && filter_option.id != "id2")
         found2 = true;
+      else if (filter_option.id.has_suffix ("id1-remote"))
+        remote1 = true;
+      else if (filter_option.id.has_suffix ("id2-remote"))
+        remote2 = true;
     }
 
     assert (found1);
     assert (found2);
+    if (remote_scope_test)
+    {
+      assert (remote1);
+      assert (remote2);
+    }
+
+    // =============== PART 2 of the test =============== //
+    // we'll now remove one source
+    local_scope.sources.remove_option ("id1");
+
+    ml = new MainLoop ();
+    // wait a bit for the model to update
+    Idle.add (() => { ml.quit (); return false; });
+    ml.run ();
+    ml = new MainLoop ();
+    // do another search to synchronize
+    call_lens_search ("sources-test-2", () => { ml.quit (); });
+    ml.run ();
+    ml = new MainLoop ();
+    // after this the sources *have* to be updated
+    Idle.add (() => { ml.quit (); return false; });
+    ml.run ();
+
+    found1 = false;
+    found2 = false;
+    remote1 = false;
+    remote2 = false;
+    foreach (var filter_option in exported_lens.get_sources_internal ().options)
+    {
+      if (filter_option.id.has_suffix ("id1") && filter_option.id != "id1")
+        found1 = true;
+      else if (filter_option.id.has_suffix ("id2") && filter_option.id != "id2")
+        found2 = true;
+      else if (filter_option.id.has_suffix ("id1-remote"))
+        remote1 = true;
+      else if (filter_option.id.has_suffix ("id2-remote"))
+        remote2 = true;
+    }
+    
+    // make the the id1 sources are gone
+    assert (found1 == false);
+    assert (found2);
+    if (remote_scope_test)
+    {
+      assert (remote1 == false);
+      assert (remote2);
+    }
+
+    SignalHandler.disconnect (local_scope, sig_id);
   }
 }

=== modified file 'test/vala/test-remote-scope.vala'
--- test/vala/test-remote-scope.vala	2011-12-08 14:57:22 +0000
+++ test/vala/test-remote-scope.vala	2012-01-23 13:23:30 +0000
@@ -31,6 +31,7 @@
     Test.add_data_func ("/Integration/Scope/TwoSearches", test_scope_two_searches);
     Test.add_data_func ("/Integration/Scope/ModelSync", test_scope_model_sync);
     Test.add_data_func ("/Integration/Scope/ReplyHint", test_scope_reply_hint);
+    Test.add_data_func ("/Integration/Scope/Sources", test_scope_sources);
     Test.add_data_func ("/Integration/Scope/Finalize", test_scope_finalize);
 
     Test.run ();
@@ -228,6 +229,47 @@
     bus.flush_sync ();
   }
 
+  public static void test_scope_sources ()
+  {
+    // we expect that the lens will call the search method
+    assert (exported_scope != null);
+
+    var ml = new MainLoop ();
+
+    exported_scope.sources.add_option ("id1-remote", "Remote 1", null);
+    exported_scope.sources.add_option ("id2-remote", "Remote 2", null);
+
+    bool remove_option = false;
+    // we're using the search for synchronization
+    ulong sig_id = exported_scope.search_changed.connect ((lens_search, search_type, cancellable) =>
+    {
+      assert (lens_search.search_string.has_prefix ("sources-test"));
+      if (remove_option)
+      {
+        exported_scope.sources.remove_option ("id1-remote");
+      }
+      // wait a while to emit the finished signal (so the info_changed
+      // is emitted)
+      Idle.add (() => { lens_search.finished (); ml.quit (); return false; });
+    });
+
+    Timeout.add (2000, () => { assert_not_reached (); });
+    ml.run ();
+
+    // first part of the test passed, now let's try to remove a source
+
+    remove_option = true;
+    // waiting for another search to finish
+    ml = new MainLoop ();
+    Timeout.add (2000, () => { assert_not_reached (); });
+    ml.run ();
+
+    SignalHandler.disconnect (exported_scope, sig_id);
+
+    var bus = Bus.get_sync (BusType.SESSION);
+    bus.flush_sync ();
+  }
+
   public static void test_scope_finalize ()
   {
     // wait for the lens to vanish