3 files changed, 86 insertions, 22 deletions

diff --git a/plugins/unityshell/src/FavoriteStoreGSettings.cpp b/plugins/unityshell/src/FavoriteStoreGSettings.cpp
index 239ea07a9..fbdaa341c 100644
--- a/plugins/unityshell/src/FavoriteStoreGSettings.cpp
+++ b/plugins/unityshell/src/FavoriteStoreGSettings.cpp
@@ -51,8 +51,6 @@ void on_settings_updated(GSettings* settings,
 const gchar* key,
 FavoriteStoreGSettings* self);
 
-std::string get_basename_or_path(std::string const& desktop_path);
-
 }
 
 FavoriteStoreGSettings::FavoriteStoreGSettings()
@@ -234,6 +232,7 @@ void FavoriteStoreGSettings::SaveFavorites(FavoriteList const& favorites, bool i
 int index = 0;
 // Since we don't always save the full path, we store the values we are
 // actually going to save in a different list.
+ const gchar* const* system_dirs = g_get_system_data_dirs();
 FavoriteList values;
 for (FavoriteList::const_iterator i = favorites.begin(), end = favorites.end();
 i != end; ++i, ++index)
@@ -243,7 +242,8 @@ void FavoriteStoreGSettings::SaveFavorites(FavoriteList const& favorites, bool i
 // the string that we are going to save. This way we know that the pointer
 // is valid for the lifetime of the favs array usage in the method call to
 // set the settings, and that we aren't referencing a temporary.
- FavoriteList::iterator iter = values.insert(values.end(), get_basename_or_path(*i));
+ FavoriteList::iterator iter = values.insert(values.end(),
+ impl::get_basename_or_path(*i, system_dirs));
 favs[index] = iter->c_str();
 }
 
@@ -300,36 +300,43 @@ void on_settings_updated(GSettings* settings,
 }
 }
 
-std::string get_basename_or_path(std::string const& desktop_path)
+} // anonymous namespace
+
+namespace impl
 {
- const gchar* const* dirs = g_get_system_data_dirs();
 
+std::string get_basename_or_path(std::string const& desktop_path,
+ const char* const* system_dirs)
+{
 /* We check to see if the desktop file belongs to one of the system data
 * directories. If so, then we store it's desktop id, otherwise we store
 * it's full path. We're clever like that.
 */
- for (int i = 0; dirs[i]; ++i)
+ for (int i = 0; system_dirs && system_dirs[i]; ++i)
 {
- std::string dir(dirs[i]);
- if (dir.at(dir.length()-1) == G_DIR_SEPARATOR)
- dir.append("applications/");
- else
- dir.append("/applications/");
-
- if (desktop_path.find(dir) == 0)
+ if (system_dirs[i] && system_dirs[i][0] != '\0')
 {
- // if we are in a subdirectory of system patch, the store name should
- // be subdir-filename.desktop
- std::string desktop_suffix = desktop_path.substr(dir.size());
- std::replace(desktop_suffix.begin(), desktop_suffix.end(), G_DIR_SEPARATOR, '-');
- return desktop_suffix;
+ std::string dir(system_dirs[i]);
+
+ if (dir.at(dir.length()-1) == G_DIR_SEPARATOR)
+ dir.append("applications/");
+ else
+ dir.append("/applications/");
+
+ if (desktop_path.find(dir) == 0)
+ {
+ // if we are in a subdirectory of system patch, the store name should
+ // be subdir-filename.desktop
+ std::string desktop_suffix = desktop_path.substr(dir.size());
+ std::replace(desktop_suffix.begin(), desktop_suffix.end(), G_DIR_SEPARATOR, '-');
+ return desktop_suffix;
+ }
 }
 }
 return desktop_path;
 }
 
-
-} // anonymous namespace
+} // namespace impl
 
 } // namespace internal
 } // namespace unity
diff --git a/plugins/unityshell/src/FavoriteStorePrivate.h b/plugins/unityshell/src/FavoriteStorePrivate.h
index 4b27e0c03..f02433917 100644
--- a/plugins/unityshell/src/FavoriteStorePrivate.h
+++ b/plugins/unityshell/src/FavoriteStorePrivate.h
@@ -39,6 +39,10 @@ std::vector<std::string> GetRemoved(std::list<std::string> const& old, std::list
 
 bool NeedToBeReordered(std::list<std::string> const& old, std::list<std::string> const& fresh);
 
+std::string get_basename_or_path(std::string const& desktop_path,
+ const char* const* system_dirs);
+
+
 } // namespace impl
 } // namespace internal
 } // namespace unity
diff --git a/tests/test_favorite_store_gsettings.cpp b/tests/test_favorite_store_gsettings.cpp
index deed131fd..54cac9ebd 100644
--- a/tests/test_favorite_store_gsettings.cpp
+++ b/tests/test_favorite_store_gsettings.cpp
@@ -26,15 +26,16 @@
 
 #define G_SETTINGS_ENABLE_BACKEND
 #include <gio/gsettingsbackend.h>
-#include <gtest/gtest.h>
+#include <gmock/gmock.h>
 #include <glib.h>
 
 #include "FavoriteStore.h"
 #include "FavoriteStoreGSettings.h"
+#include "FavoriteStorePrivate.h"
 #include <UnityCore/GLibWrapper.h>
 
-
 using namespace unity;
+using testing::Eq;
 
 namespace {
 
@@ -489,4 +490,56 @@ TEST_F(TestFavoriteStoreGSettings, TestFavoriteSignalsMixed3)
 EXPECT_TRUE(reordered_received);
 }
 
+TEST(TestFavoriteStorePathDetection, TestEmptyValues)
+{
+ using internal::impl::get_basename_or_path;
+ EXPECT_THAT(get_basename_or_path("/some/path/to.desktop", nullptr),
+ Eq("/some/path/to.desktop"));
+ EXPECT_THAT(get_basename_or_path("/some/path/to.desktop", { nullptr }),
+ Eq("/some/path/to.desktop"));
+ const char* const path[] = { "", nullptr };
+ EXPECT_THAT(get_basename_or_path("/some/path/to.desktop", path),
+ Eq("/some/path/to.desktop"));
+}
+
+TEST(TestFavoriteStorePathDetection, TestPathNeedsApplications)
+{
+ using internal::impl::get_basename_or_path;
+ const char* const path[] = { "/this/path",
+ "/that/path/",
+ nullptr };
+ EXPECT_THAT(get_basename_or_path("/this/path/to.desktop", path),
+ Eq("/this/path/to.desktop"));
+ EXPECT_THAT(get_basename_or_path("/that/path/to.desktop", path),
+ Eq("/that/path/to.desktop"));
+}
+
+TEST(TestFavoriteStorePathDetection, TestStripsPath)
+{
+ using internal::impl::get_basename_or_path;
+ const char* const path[] = { "/this/path",
+ "/that/path/",
+ nullptr };
+ EXPECT_THAT(get_basename_or_path("/this/path/applications/to.desktop", path),
+ Eq("to.desktop"));
+ EXPECT_THAT(get_basename_or_path("/that/path/applications/to.desktop", path),
+ Eq("to.desktop"));
+ EXPECT_THAT(get_basename_or_path("/some/path/applications/to.desktop", path),
+ Eq("/some/path/applications/to.desktop"));
+}
+
+TEST(TestFavoriteStorePathDetection, TestSubdirectory)
+{
+ using internal::impl::get_basename_or_path;
+ const char* const path[] = { "/this/path",
+ "/that/path/",
+ nullptr };
+ EXPECT_THAT(get_basename_or_path("/this/path/applications/subdir/to.desktop", path),
+ Eq("subdir-to.desktop"));
+ EXPECT_THAT(get_basename_or_path("/that/path/applications/subdir/to.desktop", path),
+ Eq("subdir-to.desktop"));
+ EXPECT_THAT(get_basename_or_path("/this/path/applications/subdir1/subdir2/to.desktop", path),
+ Eq("subdir1-subdir2-to.desktop"));
+}
+
 } // anonymous namespace