Merge lp:~dobey/unity-scope-click/sort-az into lp:unity-scope-click/devel

Proposed by dobey
Status: Merged
Approved by: dobey
Approved revision: 290
Merged at revision: 290
Proposed branch: lp:~dobey/unity-scope-click/sort-az
Merge into: lp:unity-scope-click/devel
Diff against target: 257 lines (+141/-20)
7 files modified
debian/control (+1/-0)
libclickscope/click/CMakeLists.txt (+1/-0)
libclickscope/click/interface.cpp (+38/-3)
libclickscope/click/interface.h (+1/-0)
libclickscope/tests/applications/user/com.ubuntu.developer.webapps.webapp-ubuntuone_webapp-ubuntuone_1.0.4.desktop (+0/-11)
libclickscope/tests/test_interface.cpp (+100/-1)
scope/clickapps/apps-query.cpp (+0/-5)
To merge this branch: bzr merge lp:~dobey/unity-scope-click/sort-az
Reviewer Review Type Date Requested Status
Alejandro J. Cura (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+222540@code.launchpad.net

Commit message

Sort installed apps alphabetically.
Add a test for the sort order.
Move the sorting back to find_installed_apps() to make testing easier.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~dobey/unity-scope-click/sort-az updated
286. By dobey

Use tolower on the titles when sorting, to avoid eBay sorting after Weather.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
lp:~dobey/unity-scope-click/sort-az updated
287. By dobey

Merge the fix-category branch in.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alejandro J. Cura (alecu) wrote :

The std::transform is happening twice for each comparison done during the sort; this looks bad cpu-wise;

One option to fix this is using a sorted map with the lowercased items as keys. Other option is moving the lowercasing to a method of Application, where we can cache it if needed.

Also, please move the sorting into its own function, and create unit tests for it.

review: Needs Fixing
lp:~dobey/unity-scope-click/sort-az updated
288. By dobey

Add dependency on libboost-locale-dev for building.
Move sorting of the apps list to a separate function.
Use boost::locale for sorting of apps by name.
If names are equal, sort based on package name.
Add tests for several different sorting cases.

289. By dobey

[ Rodney Dawes ]
* Remove unused method to fix failure building under gcc 4.9.
[ Rodney Dawes ]
* Handle some languages differently when submitting reviews. (LP:
  #1321154). Add runtime dependency on ubuntu-sdk-libs for the
  frameworks list. (LP: #1320975). Support hal+json in the search
  request. Fix issues in autopilot tests. Port autopilot tests to
  python3. Add README and HACKING files. (LP: #1226111). Translations
  updates. . (LP: #1321154, #1226111, #1320975)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~dobey/unity-scope-click/sort-az updated
290. By dobey

Set the language in the other sorting tests as well, to prevent failures.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alejandro J. Cura (alecu) wrote :

Looks good to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/control'
--- debian/control 2014-05-29 07:10:38 +0000
+++ debian/control 2014-06-13 16:50:42 +0000
@@ -6,6 +6,7 @@
6 dh-translations,6 dh-translations,
7 google-mock,7 google-mock,
8 intltool,8 intltool,
9 libboost-locale-dev,
9 libglib2.0-dev (>= 2.32),10 libglib2.0-dev (>= 2.32),
10 libjsoncpp-dev,11 libjsoncpp-dev,
11 libubuntu-download-manager-client-dev (>= 0.3+14.10.20140430-0ubuntu1),12 libubuntu-download-manager-client-dev (>= 0.3+14.10.20140430-0ubuntu1),
1213
=== modified file 'libclickscope/click/CMakeLists.txt'
--- libclickscope/click/CMakeLists.txt 2014-05-26 14:02:45 +0000
+++ libclickscope/click/CMakeLists.txt 2014-06-13 16:50:42 +0000
@@ -36,4 +36,5 @@
36 ${UBUNTUONE_LDFLAGS}36 ${UBUNTUONE_LDFLAGS}
37 ${UBUNTU_DOWNLOAD_MANAGER_CLIENT_LDFLAGS}37 ${UBUNTU_DOWNLOAD_MANAGER_CLIENT_LDFLAGS}
38 ${UBUNTU_DOWNLOAD_MANAGER_COMMON_LDFLAGS}38 ${UBUNTU_DOWNLOAD_MANAGER_COMMON_LDFLAGS}
39 -lboost_locale
39)40)
4041
=== modified file 'libclickscope/click/interface.cpp'
--- libclickscope/click/interface.cpp 2014-06-09 16:49:10 +0000
+++ libclickscope/click/interface.cpp 2014-06-13 16:50:42 +0000
@@ -37,6 +37,9 @@
37#include <sys/stat.h>37#include <sys/stat.h>
38#include <map>38#include <map>
3939
40#include <boost/locale/collator.hpp>
41#include <boost/locale/generator.hpp>
42
40#include <boost/property_tree/ptree.hpp>43#include <boost/property_tree/ptree.hpp>
41#include <boost/property_tree/json_parser.hpp>44#include <boost/property_tree/json_parser.hpp>
42#include <boost/property_tree/exceptions.hpp>45#include <boost/property_tree/exceptions.hpp>
@@ -153,8 +156,7 @@
153 DESKTOP_FILE_GROUP,156 DESKTOP_FILE_GROUP,
154 DESKTOP_FILE_KEY_NAME,157 DESKTOP_FILE_KEY_NAME,
155 domain);158 domain);
156 struct stat times;159
157 app.installed_time = stat(filename.c_str(), &times) == 0 ? times.st_mtime : 0;
158 app.url = "application:///" + filename;160 app.url = "application:///" + filename;
159 if (keyFile.has_key(DESKTOP_FILE_GROUP, DESKTOP_FILE_KEY_ICON)) {161 if (keyFile.has_key(DESKTOP_FILE_GROUP, DESKTOP_FILE_KEY_ICON)) {
160 app.icon_url = add_theme_scheme(keyFile.get_string(DESKTOP_FILE_GROUP,162 app.icon_url = add_theme_scheme(keyFile.get_string(DESKTOP_FILE_GROUP,
@@ -188,6 +190,39 @@
188 return app;190 return app;
189}191}
190192
193std::vector<click::Application> Interface::sort_apps(const std::vector<click::Application>& apps)
194{
195 std::vector<click::Application> result = apps;
196 boost::locale::generator gen;
197 const char* lang = getenv(click::Configuration::LANGUAGE_ENVVAR);
198 if (lang == NULL) {
199 lang = "C.UTF-8";
200 }
201 std::locale loc = gen(lang);
202 std::locale::global(loc);
203 typedef boost::locale::collator<char> coll_type;
204
205 // Sort applications alphabetically.
206 std::sort(result.begin(), result.end(), [&loc](const Application& a,
207 const Application& b) {
208 bool lesser = false;
209 int order = std::use_facet<coll_type>(loc)
210 .compare(boost::locale::collator_base::quaternary,
211 a.title, b.title);
212 if (order == 0) {
213 lesser = a.name < b.name;
214 } else {
215 // Because compare returns int, not bool, we have to check
216 // that 0 is greater than the result, which tells us the
217 // first element should be sorted priori
218 lesser = order < 0;
219 }
220 return lesser;
221 });
222
223 return result;
224}
225
191/* find_installed_apps()226/* find_installed_apps()
192 *227 *
193 * Find all of the installed apps matching @search_query in a timeout.228 * Find all of the installed apps matching @search_query in a timeout.
@@ -244,7 +279,7 @@
244 };279 };
245280
246 keyFileLocator->enumerateKeyFilesForInstalledApplications(enumerator);281 keyFileLocator->enumerateKeyFilesForInstalledApplications(enumerator);
247 return result;282 return sort_apps(result);
248}283}
249284
250/* is_non_click_app()285/* is_non_click_app()
251286
=== modified file 'libclickscope/click/interface.h'
--- libclickscope/click/interface.h 2014-05-26 14:02:45 +0000
+++ libclickscope/click/interface.h 2014-06-13 16:50:42 +0000
@@ -81,6 +81,7 @@
81 const std::string& domain);81 const std::string& domain);
82 virtual Application load_app_from_desktop(const unity::util::IniParser& keyFile,82 virtual Application load_app_from_desktop(const unity::util::IniParser& keyFile,
83 const std::string& filename);83 const std::string& filename);
84 static std::vector<Application> sort_apps(const std::vector<Application>& apps);
84 virtual std::vector<Application> find_installed_apps(const std::string& search_query);85 virtual std::vector<Application> find_installed_apps(const std::string& search_query);
8586
86 static bool is_non_click_app(const QString& filename);87 static bool is_non_click_app(const QString& filename);
8788
=== removed file 'libclickscope/tests/applications/user/com.ubuntu.developer.webapps.webapp-ubuntuone_webapp-ubuntuone_1.0.4.desktop'
--- libclickscope/tests/applications/user/com.ubuntu.developer.webapps.webapp-ubuntuone_webapp-ubuntuone_1.0.4.desktop 2014-02-10 12:23:35 +0000
+++ libclickscope/tests/applications/user/com.ubuntu.developer.webapps.webapp-ubuntuone_webapp-ubuntuone_1.0.4.desktop 1970-01-01 00:00:00 +0000
@@ -1,11 +0,0 @@
1[Desktop Entry]
2Type=Application
3Terminal=false
4Exec=aa-exec-click -p com.ubuntu.developer.webapps.webapp-ubuntuone_webapp-ubuntuone_1.0.4 -- webbrowser-app --enable-back-forward --webappUrlPatterns=https?://one.ubuntu.com/*,https?://login.ubuntu.com/* --webapp https://one.ubuntu.com/
5Name=Ubuntu One
6Icon=/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.developer.webapps.webapp-ubuntuone/./ubuntuone.png
7X-Ubuntu-Touch=true
8X-Ubuntu-Single-Instance=true
9Path=/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.developer.webapps.webapp-ubuntuone
10X-Ubuntu-Old-Icon=./ubuntuone.png
11X-Ubuntu-Application-ID=com.ubuntu.developer.webapps.webapp-ubuntuone_webapp-ubuntuone_1.0.4
120
=== modified file 'libclickscope/tests/test_interface.cpp'
--- libclickscope/tests/test_interface.cpp 2014-06-09 16:42:23 +0000
+++ libclickscope/tests/test_interface.cpp 2014-06-13 16:50:42 +0000
@@ -53,7 +53,6 @@
53// Maintaining this list here will become tedious over time.53// Maintaining this list here will become tedious over time.
54static const std::vector<click::Application> non_desktop_applications =54static const std::vector<click::Application> non_desktop_applications =
55{55{
56 {"com.ubuntu.developer.webapps.webapp-ubuntuone", "Ubuntu One", 0.0, "/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.developer.webapps.webapp-ubuntuone/./ubuntuone.png", "application:///com.ubuntu.developer.webapps.webapp-ubuntuone_webapp-ubuntuone_1.0.4.desktop", "", ""},
57 {"com.ubuntu.stock-ticker-mobile", "Stock Ticker", 0.0, "/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.stock-ticker-mobile/icons/stock_icon_48.png", "application:///com.ubuntu.stock-ticker-mobile_stock-ticker-mobile_0.3.7.66.desktop", "", ""},56 {"com.ubuntu.stock-ticker-mobile", "Stock Ticker", 0.0, "/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.stock-ticker-mobile/icons/stock_icon_48.png", "application:///com.ubuntu.stock-ticker-mobile_stock-ticker-mobile_0.3.7.66.desktop", "", ""},
58 {"", "Weather", 0.0, "/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.weather/./weather64.png", "application:///com.ubuntu.weather_weather_1.0.168.desktop", "", ""},57 {"", "Weather", 0.0, "/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.weather/./weather64.png", "application:///com.ubuntu.weather_weather_1.0.168.desktop", "", ""},
59 {"com.ubuntu.developer.webapps.webapp-twitter", "Twitter", 0.0, "/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.developer.webapps.webapp-twitter/./twitter.png", "application:///com.ubuntu.developer.webapps.webapp-twitter_webapp-twitter_1.0.5.desktop", "", ""},58 {"com.ubuntu.developer.webapps.webapp-twitter", "Twitter", 0.0, "/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.developer.webapps.webapp-twitter/./twitter.png", "application:///com.ubuntu.developer.webapps.webapp-twitter_webapp-twitter_1.0.5.desktop", "", ""},
@@ -182,6 +181,106 @@
182 EXPECT_TRUE(results.empty());181 EXPECT_TRUE(results.empty());
183}182}
184183
184TEST(ClickInterface, testFindAppsInDirSorted)
185{
186 QSharedPointer<click::KeyFileLocator> keyFileLocator(
187 new click::KeyFileLocator(
188 testing::systemApplicationsDirectoryForTesting(),
189 testing::userApplicationsDirectoryForTesting()));
190
191 click::Interface iface(keyFileLocator);
192
193 auto results = iface.find_installed_apps("ock");
194
195 const std::vector<click::Application> expected_results = {
196 {"com.ubuntu.clock", "Clock", 0.0, "/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.clock/./clock64.png", "application:///com.ubuntu.clock_clock_1.0.300.desktop", "", ""},
197 {"com.ubuntu.stock-ticker-mobile", "Stock Ticker", 0.0, "/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.stock-ticker-mobile/icons/stock_icon_48.png", "application:///com.ubuntu.stock-ticker-mobile_stock-ticker-mobile_0.3.7.66.desktop", "", ""},
198 };
199 EXPECT_EQ(expected_results, results);
200}
201
202TEST(ClickInterface, testSortApps)
203{
204 std::vector<click::Application> apps = {
205 {"", "Sudoku", 0.0, "", "", "", ""},
206 {"", "eBay", 0.0, "", "", "", ""},
207 {"", "Facebook", 0.0, "", "", "", ""},
208 {"", "Messaging", 0.0, "", "", "", ""},
209 {"", "Contacts", 0.0, "", "", "", ""},
210 };
211
212 std::vector<click::Application> expected = {
213 {"", "Contacts", 0.0, "", "", "", ""},
214 {"", "eBay", 0.0, "", "", "", ""},
215 {"", "Facebook", 0.0, "", "", "", ""},
216 {"", "Messaging", 0.0, "", "", "", ""},
217 {"", "Sudoku", 0.0, "", "", "", ""},
218 };
219
220 ASSERT_EQ(setenv(Configuration::LANGUAGE_ENVVAR, "en_US.UTF-8", 1), 0);
221 EXPECT_EQ(expected, click::Interface::sort_apps(apps));
222 ASSERT_EQ(unsetenv(Configuration::LANGUAGE_ENVVAR), 0);
223}
224
225TEST(ClickInterface, testSortAppsWithDuplicates)
226{
227 std::vector<click::Application> apps = {
228 {"com.sudoku.sudoku", "Sudoku", 0.0, "", "", "", ""},
229 {"com.canonical.sudoku", "Sudoku", 0.0, "", "", "", ""},
230 };
231
232 std::vector<click::Application> expected = {
233 {"com.canonical.sudoku", "Sudoku", 0.0, "", "", "", ""},
234 {"com.sudoku.sudoku", "Sudoku", 0.0, "", "", "", ""},
235 };
236
237 ASSERT_EQ(setenv(Configuration::LANGUAGE_ENVVAR, "en_US.UTF-8", 1), 0);
238 EXPECT_EQ(expected, click::Interface::sort_apps(apps));
239 ASSERT_EQ(unsetenv(Configuration::LANGUAGE_ENVVAR), 0);
240}
241
242TEST(ClickInterface, testSortAppsWithAccents)
243{
244 std::vector<click::Application> apps = {
245 {"", "Robots", 0.0, "", "", "", ""},
246 {"", "Æon", 0.0, "", "", "", ""},
247 {"", "Contacts", 0.0, "", "", "", ""},
248 {"", "Über", 0.0, "", "", "", ""},
249 };
250
251 std::vector<click::Application> expected = {
252 {"", "Æon", 0.0, "", "", "", ""},
253 {"", "Contacts", 0.0, "", "", "", ""},
254 {"", "Robots", 0.0, "", "", "", ""},
255 {"", "Über", 0.0, "", "", "", ""},
256 };
257
258 ASSERT_EQ(setenv(Configuration::LANGUAGE_ENVVAR, "en_US.UTF-8", 1), 0);
259 EXPECT_EQ(expected, click::Interface::sort_apps(apps));
260 ASSERT_EQ(unsetenv(Configuration::LANGUAGE_ENVVAR), 0);
261}
262
263TEST(ClickInterface, testSortAppsMixedCharsets)
264{
265 std::vector<click::Application> apps = {
266 {"", "Robots", 0.0, "", "", "", ""},
267 {"", "汉字", 0.0, "", "", "", ""},
268 {"", "漢字", 0.0, "", "", "", ""},
269 {"", "Über", 0.0, "", "", "", ""},
270 };
271
272 std::vector<click::Application> expected = {
273 {"", "汉字", 0.0, "", "", "", ""},
274 {"", "漢字", 0.0, "", "", "", ""},
275 {"", "Robots", 0.0, "", "", "", ""},
276 {"", "Über", 0.0, "", "", "", ""},
277 };
278
279 ASSERT_EQ(setenv(Configuration::LANGUAGE_ENVVAR, "zh_CN.UTF-8", 1), 0);
280 EXPECT_EQ(expected, click::Interface::sort_apps(apps));
281 ASSERT_EQ(unsetenv(Configuration::LANGUAGE_ENVVAR), 0);
282}
283
185TEST(ClickInterface, testFindAppByKeyword)284TEST(ClickInterface, testFindAppByKeyword)
186{285{
187 QSharedPointer<click::KeyFileLocator> keyFileLocator(286 QSharedPointer<click::KeyFileLocator> keyFileLocator(
188287
=== modified file 'scope/clickapps/apps-query.cpp'
--- scope/clickapps/apps-query.cpp 2014-06-10 20:40:33 +0000
+++ scope/clickapps/apps-query.cpp 2014-06-13 16:50:42 +0000
@@ -204,11 +204,6 @@
204 auto localResults = clickInterfaceInstance().find_installed_apps(204 auto localResults = clickInterfaceInstance().find_installed_apps(
205 query);205 query);
206206
207 // Sort applications so that newest come first.
208 std::sort(localResults.begin(), localResults.end(), [](const Application& a, const Application& b) {
209 return a.installed_time > b.installed_time;
210 });
211
212 push_local_results(207 push_local_results(
213 searchReply,208 searchReply,
214 localResults,209 localResults,

Subscribers

People subscribed via source and target branches

to all changes: