Merge lp:~kamstrup/libunity/faves-enumerator into lp:libunity

[Mikkel Kamstrup Erlandsen]

Add Unity.LauncherFavorites API for inspecting the favorite apps in the launcher

Note that this is a read-only API intentionally. The Unity policy is that the
user alone is in control of the launcher. You can listen for changes, and the
ordering is guaranteed to match that of Unity.

[Michal Hruby (mhr3)]

Looks good, just two minor issues:

1) libunity is inconsistent in method names to get singleton instance (get_instance vs get_default), I'd rather see us using get_default which is more common in gobject world.
2) it'd be nice to add `public bool contains (string id)`, to support 'in' operator.

review: Needs Fixing

[Mikkel Kamstrup Erlandsen (kamstrup)]

> 1) libunity is inconsistent in method names to get singleton instance
> (get_instance vs get_default), I'd rather see us using get_default which is
> more common in gobject world.

I thought quite a lot about this when writing the API. The idea was that get_instance() had the semantics of a "pure" singleton, which you can only ever have precisely one of; while get_default() implies that you can have different instances, but normally just one.

But thinking about it again today I don't think API consumers is gonna appreciate this pedantry ;-) I'll rename it to get_default().

In the same spirit I think we should rename Unity.AppInfoManager.get_instance() as well - although that may be too hard an API break..?

> 2) it'd be nice to add `public bool contains (string id)`, to support 'in'
> operator.

Also thought about that one. If we could overload it with a version that takes an AppInfo then maybe, but this is too much of a Valaism to my liking. The lib is meant to have a sensible C and Python API as well. If we can fix it with some clever #defines or -custom.vala overrides then I am OK with it, but as a part of the ABI I don't like it...

[Michal Hruby (mhr3)]

> I thought quite a lot about this when writing the API. The idea was that
> get_instance() had the semantics of a "pure" singleton, which you can only
> ever have precisely one of; while get_default() implies that you can have
> different instances, but normally just one.

You can't really have "pure" singleton with gobject (g_object_new (...)), that's why everyone uses get_default.

> In the same spirit I think we should rename
> Unity.AppInfoManager.get_instance() as well - although that may be too hard an
> API break..?
>

I was thinking about that as well, but yea don't like the API break there, maybe we can add get_default and mark get_instance deprecated for now?

> > 2) it'd be nice to add `public bool contains (string id)`, to support 'in'
> > operator.
>
> Also thought about that one. If we could overload it with a version that takes
> an AppInfo then maybe, but this is too much of a Valaism to my liking. The lib
> is meant to have a sensible C and Python API as well. If we can fix it with
> some clever #defines or -custom.vala overrides then I am OK with it, but as a
> part of the ABI I don't like it...

Right, it'd be best to have that only in the vapi, unfortunately valac doesn't do -custom.vala when you're generating vapi from vala sources. The only thing we could do would be some sed :( Let's forget about that then...

[Mikkel Kamstrup Erlandsen (kamstrup)]

> > I thought quite a lot about this when writing the API. The idea was that
> > get_instance() had the semantics of a "pure" singleton, which you can only
> > ever have precisely one of; while get_default() implies that you can have
> > different instances, but normally just one.
>
> You can't really have "pure" singleton with gobject (g_object_new (...)),
> that's why everyone uses get_default.

Sure you can, just override GObject->constructor(), but that aside. We agree on the solution :-)

> > In the same spirit I think we should rename
> > Unity.AppInfoManager.get_instance() as well - although that may be too hard
> an
> > API break..?
> >
>
> I was thinking about that as well, but yea don't like the API break there,
> maybe we can add get_default and mark get_instance deprecated for now?

ok

> > > 2) it'd be nice to add `public bool contains (string id)`, to support 'in'
> > > operator.
> >
> > Also thought about that one. If we could overload it with a version that
> takes
> > an AppInfo then maybe, but this is too much of a Valaism to my liking. The
> lib
> > is meant to have a sensible C and Python API as well. If we can fix it with
> > some clever #defines or -custom.vala overrides then I am OK with it, but as
> a
> > part of the ABI I don't like it...
>
> Right, it'd be best to have that only in the vapi, unfortunately valac doesn't
> do -custom.vala when you're generating vapi from vala sources. The only thing
> we could do would be some sed :( Let's forget about that then...

We can look at it down the road if we find time for it

[lp:~kamstrup/libunity/faves-enumerator]

98. By Mikkel Kamstrup Erlandsen on 2011-12-06

Rename method LauncherFavorites.get_instance() to get_default()

99. By Mikkel Kamstrup Erlandsen on 2011-12-06

Deprecate AppInfoManager.get_instance() and rename it to get_default().

This make sure that all singletons in libunity are accessed similarly

[Mikkel Kamstrup Erlandsen (kamstrup)]

Pushed r99. Please re-review

[Michal Hruby (mhr3)]

> > You can't really have "pure" singleton with gobject (g_object_new (...)),
> > that's why everyone uses get_default.
>
> Sure you can, just override GObject->constructor(), but that aside. We agree
> on the solution :-)
>

Hmm, interesting, in that case people are just too lazy to do it I guess :)

The changes look fine, thanks.

review: Approve

=== modified file 'src/unity-appinfo-manager.vala'
--- src/unity-appinfo-manager.vala	2011-12-01 12:42:22 +0000
+++ src/unity-appinfo-manager.vala	2011-12-06 12:05:26 +0000
@@ -86,10 +86,16 @@
         }
     }
     
+    [Deprecated (replacement = "AppInfoManager.get_default")]
+    public static AppInfoManager get_instance ()
+    {
+      return AppInfoManager.get_default ();
+    }
+    
     /**
      * Get a ref to the singleton AppInfoManager
      */
-    public static AppInfoManager get_instance ()
+    public static AppInfoManager get_default ()
     {
       if (AppInfoManager.singleton == null)
         AppInfoManager.singleton = new AppInfoManager();      

=== modified file 'src/unity-launcher.vala'
--- src/unity-launcher.vala	2011-08-02 07:49:39 +0000
+++ src/unity-launcher.vala	2011-12-06 12:05:26 +0000
@@ -343,6 +343,119 @@
       props.insert ("quicklist", l._object_path);
     
     return props;
-  }  
-
+  }
+  
+  public class LauncherFavorites : Object
+  {
+    public signal void changed ();
+    
+    private static LauncherFavorites? singleton = null;
+    private Settings settings;
+    
+    /* We keep both a map and a list in order to have the correct sorting */
+    private HashTable<string,AppInfo?> fav_cache;
+    private string[] fav_list;
+    
+    private LauncherFavorites ()
+    {
+      settings = new Settings ("com.canonical.Unity.Launcher");
+      fav_cache = new HashTable<string,AppInfo?> (str_hash, str_equal);
+      reset_fav_cache ();
+      
+      settings.changed["favorites"].connect (reset_fav_cache);
+    }
+    
+    private void reset_fav_cache ()
+    {
+      fav_cache.remove_all ();
+      fav_list = settings.get_strv ("favorites");
+      foreach (string id in fav_list)
+      {
+        fav_cache.insert (id, null);
+      }
+      
+      changed ();
+    }
+    
+    /**
+     * Get the default singleton Unity.LauncherFavorites instance, creating it
+     * dynamically if necessary.
+     *
+     * @return: (transfer none): The singleton Unity.LauncherFavorites.
+     *                          If calling from C do not free this instance.
+     *
+     */
+    public static unowned LauncherFavorites get_default ()
+    {
+      if (singleton == null)
+        singleton = new LauncherFavorites ();
+        
+      return singleton;
+    }
+    
+    public bool has_app_info (AppInfo appinfo) {
+      if (appinfo.get_id () == null)
+        {
+          critical ("Can not look up favorite for AppInfo with NULL id");
+          return false;
+        }
+      
+      return has_app_id (appinfo.get_id ());
+    }
+    
+    public bool has_app_id (string app_id)
+    {
+      return fav_cache.lookup_extended (app_id, null, null);
+    }
+    
+    public AppInfo? lookup (string app_id)
+    {
+      AppInfo? appinfo;
+      bool has_id = fav_cache.lookup_extended (app_id, null, out appinfo);
+      
+      /* If we don't have the appinfo cached, pull it out of the
+       * AppInfoManager and cache it */
+      if (has_id)
+        {
+          if (appinfo != null)
+            return appinfo;
+          
+          var appman = AppInfoManager.get_default ();
+          appinfo = appman.lookup (app_id);
+          fav_cache.insert (app_id, appinfo);
+          
+          // FIXME: We really should return a dummy AppInfo for faves
+          //        where we can't find the .desktop file...
+          if (appinfo == null)
+            {
+              critical ("Can't find AppInfo for favorite '%s'", app_id);
+              return null;
+            }
+          
+          return appinfo;
+        }
+      return null;
+    }
+    
+    public string[] enumerate_ids ()
+    {
+      return fav_list;
+    }
+    
+    public AppInfo[] enumerate_app_infos ()
+    {
+      int i = 0;
+      var infos = new AppInfo[fav_cache.size ()];
+      
+      foreach (string id in fav_list)
+      {
+        var appinfo = lookup (id);
+        infos[i] = appinfo;
+        i++;
+      }
+      
+      return infos;
+    }
+  }
+  
 } /* namespace */

=== modified file 'test/Makefile.am'
--- test/Makefile.am	2011-12-02 14:37:36 +0000
+++ test/Makefile.am	2011-12-06 12:05:26 +0000
@@ -2,5 +2,6 @@
 
 EXTRA_DIST = \
   data/applications/ubuntu-about.desktop \
+  data/applications/asdasdasd.desktop \
   data/scope0.scope                      \
   data/test_desktop_file.desktop

=== added file 'test/data/applications/asdasdasd.desktop'
--- test/data/applications/asdasdasd.desktop	1970-01-01 00:00:00 +0000
+++ test/data/applications/asdasdasd.desktop	2011-12-06 12:05:26 +0000
@@ -0,0 +1,11 @@
+[Desktop Entry]
+Encoding=UTF-8
+Name=libunity bogus app
+Comment=For testing non-exisiting apps
+Exec=xterm
+Icon=distributor-logo
+Terminal=false
+Type=Application
+Categories=GNOME;Application;Core;
+StartupNotify=true
+

=== added file 'test/data/applications/testapp1.desktop'
--- test/data/applications/testapp1.desktop	1970-01-01 00:00:00 +0000
+++ test/data/applications/testapp1.desktop	2011-12-06 12:05:26 +0000
@@ -0,0 +1,11 @@
+[Desktop Entry]
+Encoding=UTF-8
+Name=libunity test app 1
+Comment=Test App 1 for testing non-exisiting apps
+Exec=xterm
+Icon=distributor-logo
+Terminal=false
+Type=Application
+Categories=GNOME;Application;Core;
+StartupNotify=true
+

=== modified file 'test/vala/Makefile.integration_tests'
--- test/vala/Makefile.integration_tests	2011-12-01 15:43:28 +0000
+++ test/vala/Makefile.integration_tests	2011-12-06 12:05:26 +0000
@@ -1,4 +1,5 @@
 check_PROGRAMS += \
+  test-launcher-integration \
   test-sound-menu \
   test-mpris-backend-client \
   test-mpris-backend-server \
@@ -11,6 +12,7 @@
 if ENABLE_INTEGRATION_TESTS
 TEST_PROGS += \
   test-lens-scope-interactions \
+  test-launcher-integration \
   test-sound-menu \
   test-mpris-backend \
   test-mpris-backend-prop-updates
@@ -27,6 +29,14 @@
 	@chmod +x $@
 
 #########################################
+##  test-launcher-integration
+#########################################
+test_launcher_integration_SOURCES = \
+  test-launcher-integration.vala
+
+test_launcher_integration_LDADD = $(test_libs)
+
+#########################################
 ##  test-sound-menu
 #########################################
 test_sound_menu_SOURCES = \

=== modified file 'test/vala/test-appinfo-manager.vala'
--- test/vala/test-appinfo-manager.vala	2011-09-27 09:48:39 +0000
+++ test/vala/test-appinfo-manager.vala	2011-12-06 12:05:26 +0000
@@ -43,14 +43,14 @@
     /* Test that we can even get a valid ref to the manager */
     internal static void test_allocation()
     {
-      var manager = AppInfoManager.get_instance();
+      var manager = AppInfoManager.get_default();
       assert (manager is AppInfoManager);
     }
 
     /* Test that we can clear an empty manager */
     internal static void test_clear_empty()
     {
-      var manager = AppInfoManager.get_instance();
+      var manager = AppInfoManager.get_default();
       manager.clear ();
       manager.clear ();
     }
@@ -58,7 +58,7 @@
     /* Test that we can clear an empty manager */
     internal static void test_sync_lookup_missing()
     {
-      var manager = AppInfoManager.get_instance();
+      var manager = AppInfoManager.get_default();
       assert (manager.lookup ("_foobar.desktop") == null);
       assert (manager.get_categories ("_foobar.desktop") == null);
       assert (manager.get_keywords ("_foobar.desktop") == null);
@@ -74,7 +74,7 @@
     
     internal static async void do_test_async_lookup_missing (MainLoop mainloop)
     {
-      var manager = AppInfoManager.get_instance();
+      var manager = AppInfoManager.get_default();
       
       try {
         AppInfo? appinfo = yield manager.lookup_async ("_foobar.desktop");
@@ -92,7 +92,7 @@
     /* Test that we can lookup something which is indeed there */
     internal static void test_sync_lookup_ok()
     {
-      var manager = AppInfoManager.get_instance();      
+      var manager = AppInfoManager.get_default();      
       
       var info = manager.lookup ("ubuntu-about.desktop");
       assert (info != null);
@@ -128,7 +128,7 @@
     
     internal static async void do_test_async_lookup_ok (MainLoop mainloop)
     {
-      var manager = AppInfoManager.get_instance();
+      var manager = AppInfoManager.get_default();
       
       try{
         var info = yield manager.lookup_async ("ubuntu-about.desktop");

=== added file 'test/vala/test-launcher-integration.vala'
--- test/vala/test-launcher-integration.vala	1970-01-01 00:00:00 +0000
+++ test/vala/test-launcher-integration.vala	2011-12-06 12:05:26 +0000
@@ -0,0 +1,176 @@
+/*
+ * Copyright (C) 2011 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by Mikkel Kamstrup Erlandsen <[e-mail]>
+ *
+ */
+
+/*
+ * Despite the relative simplicity of this API we still need to relegate it
+ * to the integration tests suite because it depends on the GSettings schema
+ * from Unity.
+ */
+
+public class Main
+{
+  public static int main (string[] args)
+  {
+    /* Prepend test search path for XDG_DATA_DIRS so we can find
+     * our sample .desktop files */
+    var datadirs = Environment.get_variable ("XDG_DATA_DIRS");
+    if (datadirs != null)
+      datadirs = Config.TESTDIR + "/data:" + datadirs;
+    else
+      datadirs = Config.TESTDIR + "/data";
+    Environment.set_variable ("XDG_DATA_DIRS", datadirs, true);
+    
+    /* Make sure we don't hose the user env with our tests... */
+    Environment.set_variable ("GSETTINGS_BACKEND", "memory", true);
+    
+    Test.init (ref args);
+
+    Test.add_data_func ("/Integration/Launcher/Favorites/UnknownApps",
+                         test_has_unknown_apps);
+    
+    Test.add_data_func ("/Integration/Launcher/Favorites/HasSampleApps",
+                         test_has_sample_apps);
+    
+    Test.add_data_func ("/Integration/Launcher/Favorites/Lookup",
+                         test_lookup);
+    
+    Test.add_data_func ("/Integration/Launcher/Favorites/EnumerateIds",
+                         test_enumerate_ids);
+    
+    Test.add_data_func ("/Integration/Launcher/Favorites/EnumerateAppInfos",
+                         test_enumerate_app_infos);
+    
+    Test.add_data_func ("/Integration/Launcher/Favorites/Changes",
+                         test_changes);
+
+    Test.run ();
+
+    return 0;
+  }
+  
+  internal static void set_up () {  
+    assert ("memory" == Environment.get_variable ("GSETTINGS_BACKEND"));
+    var settings = new Settings ("com.canonical.Unity.Launcher");
+    
+    string[] faves = { "rhythmbox.desktop", "testapp1.desktop", "ubuntu-about.desktop" };
+    settings.set_strv ("favorites", faves);
+  }
+  
+  internal static void test_has_unknown_apps ()
+  {
+    set_up ();
+    
+    var faves = Unity.LauncherFavorites.get_default ();
+    assert (!faves.has_app_id ("hulabaloola"));
+    
+    var appinfo = new DesktopAppInfo ("asdasdasd.desktop");
+    assert (appinfo != null);
+    assert (!faves.has_app_info (appinfo));
+  }
+  
+  internal static void test_has_sample_apps ()
+  {
+    set_up ();
+    
+    var faves = Unity.LauncherFavorites.get_default ();
+    assert (faves.has_app_id ("rhythmbox.desktop"));
+    assert (faves.has_app_id ("testapp1.desktop"));
+    assert (faves.has_app_id ("ubuntu-about.desktop"));
+    
+    var appinfo = new DesktopAppInfo ("rhythmbox.desktop");
+    assert (faves.has_app_info (appinfo));
+    appinfo = new DesktopAppInfo ("testapp1.desktop");
+    assert (faves.has_app_info (appinfo));
+    appinfo = new DesktopAppInfo ("ubuntu-about.desktop");
+    assert (faves.has_app_info (appinfo));
+  }
+  
+  internal static void test_lookup ()
+  {
+    set_up ();
+    
+    var faves = Unity.LauncherFavorites.get_default ();
+    var appinfo = faves.lookup ("rhythmbox.desktop");
+    assert (appinfo.get_name () == "Rhythmbox");
+    
+    appinfo = faves.lookup ("testapp1.desktop");
+    assert (appinfo.get_name () == "libunity test app 1");
+    
+    appinfo = faves.lookup ("ubuntu-about.desktop");
+    assert (appinfo.get_name () == "About Ubuntu");
+    
+    appinfo = faves.lookup ("pakupachupikamachu.desktop");
+    assert (appinfo == null);
+  }
+  
+  internal static void test_enumerate_ids ()
+  {
+    set_up ();
+    
+    var faves = Unity.LauncherFavorites.get_default ();
+    var ids = faves.enumerate_ids ();
+    
+    assert (ids.length == 3);
+    assert (ids[0] == "rhythmbox.desktop");
+    assert (ids[1] == "testapp1.desktop");
+    assert (ids[2] == "ubuntu-about.desktop");
+  }
+
+  internal static void test_enumerate_app_infos ()
+  {
+    set_up ();
+    
+    var faves = Unity.LauncherFavorites.get_default ();
+    var infos = faves.enumerate_app_infos ();
+    
+    assert (infos.length == 3);
+    assert (infos[0].get_name() == "Rhythmbox");
+    assert (infos[1].get_name() == "libunity test app 1");
+    assert (infos[2].get_name() == "About Ubuntu");
+  }
+  
+ internal static void test_changes ()
+  {
+    set_up ();
+    
+    var faves = Unity.LauncherFavorites.get_default ();
+    bool was_changed = false;
+    
+    faves.changed.connect ( () => {
+      was_changed = true;
+    });
+    
+    /* Change the faves */
+    var settings = new Settings ("com.canonical.Unity.Launcher");
+    string[] new_faves = { "rhythmbox.desktop" };
+    settings.set_strv ("favorites", new_faves);
+    
+    /* Wait for updates */
+    var ml = new MainLoop ();
+    Idle.add (() => { ml.quit(); return false; });
+    ml.run ();
+    
+    assert (was_changed == true);
+    assert (faves.enumerate_ids().length == 1);
+    assert (faves.enumerate_ids()[0] == "rhythmbox.desktop");
+  }
+  
+   
+
+}

=== modified file 'test/vala/test-launcher.vala'
--- test/vala/test-launcher.vala	2011-04-01 20:19:52 +0000
+++ test/vala/test-launcher.vala	2011-12-06 12:05:26 +0000
@@ -84,5 +84,6 @@
       
       // FIXME: We're not testing the quicklist here, that's a bit tricky
     }
+    
   }
 }