Merge lp:~alecu/unity-scope-click/uninstallable-scopes into lp:unity-scope-click/devel

Proposed by Alejandro J. Cura
Status: Merged
Approved by: dobey
Approved revision: 297
Merged at revision: 293
Proposed branch: lp:~alecu/unity-scope-click/uninstallable-scopes
Merge into: lp:unity-scope-click/devel
Prerequisite: lp:~alecu/unity-scope-click/a-few-renames
Diff against target: 851 lines (+419/-85)
10 files modified
libclickscope/click/interface.cpp (+56/-6)
libclickscope/click/interface.h (+9/-0)
libclickscope/click/package.cpp (+5/-4)
libclickscope/click/package.h (+9/-0)
libclickscope/click/preview.cpp (+39/-17)
libclickscope/tests/fake_json.h (+72/-0)
libclickscope/tests/test_interface.cpp (+96/-0)
scope/clickstore/store-query.cpp (+36/-24)
scope/clickstore/store-query.h (+5/-3)
scope/tests/test_query.cpp (+92/-31)
To merge this branch: bzr merge lp:~alecu/unity-scope-click/uninstallable-scopes
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
dobey (community) Approve
Paweł Stołowski (community) Approve
Review via email: mp+223005@code.launchpad.net

Commit message

- label installed packages as such in results
- tweak icon size in surfacing to align with added subtitle
- allow launching installed scopes from package preview
- no longer filter out installed packages in store scope

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
292. By Alejandro J. Cura

Show debugging information when creating uri to open scope

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

Add more debugging logs

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 :

Clicking on the "Search" button is not working due to bug #1329890 in unity-scope-shell
Once that's fixed it should work with no changes needed here.

Revision history for this message
dobey (dobey) wrote :

/home/phablet/uninstallable-scopes/libclickscope/click/interface.cpp:406:56: error: converting to ‘std::unordered_set<click::Package>’ from initializer list would use explicit constructor ‘std::unordered_set<_Value, _Hash, _Pred, _Alloc>::unordered_set(std::unordered_set<_Value, _Hash, _Pred, _Alloc>::size_type, const hasher&, const key_equal&, const allocator_type&) [with _Value = click::Package; _Hash = std::hash<click::Package>; _Pred = std::equal_to<click::Package>; _Alloc = std::allocator<click::Package>; std::unordered_set<_Value, _Hash, _Pred, _Alloc>::size_type = unsigned int; std::unordered_set<_Value, _Hash, _Pred, _Alloc>::hasher = std::hash<click::Package>; std::unordered_set<_Value, _Hash, _Pred, _Alloc>::key_equal = std::equal_to<click::Package>; std::unordered_set<_Value, _Hash, _Pred, _Alloc>::allocator_type = std::allocator<click::Package>]’
                 callback({}, InterfaceError::ParseError);
                                                        ^
/home/phablet/uninstallable-scopes/libclickscope/click/interface.cpp:410:51: error: converting to ‘std::unordered_set<click::Package>’ from initializer list would use explicit constructor ‘std::unordered_set<_Value, _Hash, _Pred, _Alloc>::unordered_set(std::unordered_set<_Value, _Hash, _Pred, _Alloc>::size_type, const hasher&, const key_equal&, const allocator_type&) [with _Value = click::Package; _Hash = std::hash<click::Package>; _Pred = std::equal_to<click::Package>; _Alloc = std::allocator<click::Package>; std::unordered_set<_Value, _Hash, _Pred, _Alloc>::size_type = unsigned int; std::unordered_set<_Value, _Hash, _Pred, _Alloc>::hasher = std::hash<click::Package>; std::unordered_set<_Value, _Hash, _Pred, _Alloc>::key_equal = std::equal_to<click::Package>; std::unordered_set<_Value, _Hash, _Pred, _Alloc>::allocator_type = std::allocator<click::Package>]’
             callback({}, InterfaceError::CallError);
                                                   ^
libclickscope/click/CMakeFiles/clickscope.dir/build.make:126: recipe for target 'libclickscope/click/CMakeFiles/clickscope.dir/interface.cpp.o' failed

These compilation errors with gcc 4.9 seem to be a result of the changes in this branch. Can we have some plan to fix them before we get upgraded to 4.9 again, even though this compiles ok under 4.8?

review: Needs Information
Revision history for this message
dobey (dobey) :
review: Needs Information
294. By Alejandro J. Cura

Merged from prereq branch

295. By Alejandro J. Cura

Move the hashing inside the Package class

296. By Alejandro J. Cura

Handle multiple apps and scopes in the same package, caps for ✔ INSTALLED label, misc code renamings

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Paweł Stołowski (stolowski) wrote :

619 +#include <unordered_set>

This doesn't seem to be required in the context of this file?

240 + if (error == click::InterfaceError::NoError) {
241 + uri = "application:///" + val;
242 + }

Can you log an error here, if any?

13 +#include <streambuf>

Is this needed?

review: Needs Fixing
297. By Alejandro J. Cura

Removed unused includes and added error messages requested in Pawel's review

Revision history for this message
Paweł Stołowski (stolowski) wrote :

Thanks, LGTM!

review: Approve
Revision history for this message
dobey (dobey) wrote :

I still have a concern about the version not being used in the operator== for Package, as in its current state it can result in the store showing "INSTALLED" for a newer version of the package than is installed, which, when the new preview design is implemented, can result in the change log showing disparate information in the preview, resulting in the app running without any new changes, when the update has not been installed. However, this is a new issue, primarily with a design that is not yet implemented, so I don't think we should block this branch on that.

Also, the po/POTFILES.in will need updated, but I will do that in a separate branch.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libclickscope/click/interface.cpp'
2--- libclickscope/click/interface.cpp 2014-06-13 20:44:52 +0000
3+++ libclickscope/click/interface.cpp 2014-06-16 14:09:25 +0000
4@@ -33,9 +33,11 @@
5 #include <QStandardPaths>
6 #include <QTimer>
7
8+#include <cstdio>
9 #include <list>
10 #include <sys/stat.h>
11 #include <map>
12+#include <sstream>
13
14 #include <boost/locale/collator.hpp>
15 #include <boost/locale/generator.hpp>
16@@ -47,7 +49,6 @@
17
18 #include <unity/UnityExceptions.h>
19 #include <unity/util/IniParser.h>
20-#include <sstream>
21
22 #include "interface.h"
23 #include <click/key_file_locator.h>
24@@ -368,10 +369,16 @@
25
26 BOOST_FOREACH(ptree::value_type &sv, pt.get_child("hooks"))
27 {
28- // FIXME: "primary app" for a package is not defined, we just
29- // use first one here:
30- manifest.first_app_name = sv.first;
31- break;
32+ // FIXME: "primary app or scope" for a package is not defined,
33+ // we just use first one here:
34+ auto app_name = sv.second.get("desktop", "");
35+ if (manifest.first_app_name.empty() && !app_name.empty()) {
36+ manifest.first_app_name = sv.first;
37+ }
38+ auto scope_id = sv.second.get("scope", "");
39+ if (manifest.first_scope_id.empty() && !scope_id.empty()) {
40+ manifest.first_scope_id = manifest.name; // need to change this for more than one scope per click
41+ }
42 }
43 qDebug() << "adding manifest: " << manifest.name.c_str() << manifest.version.c_str() << manifest.first_app_name.c_str();
44
45@@ -398,20 +405,63 @@
46 });
47 }
48
49+PackageSet package_names_from_stdout(const std::string& stdout_data)
50+{
51+ const char TAB='\t', NEWLINE='\n';
52+ std::istringstream iss(stdout_data);
53+ PackageSet installed_packages;
54+
55+ while (iss.peek() != EOF) {
56+ Package p;
57+ std::getline(iss, p.name, TAB);
58+ std::getline(iss, p.version, NEWLINE);
59+ if (iss.eof() || p.name.empty() || p.version.empty()) {
60+ throw std::runtime_error("Error encountered parsing 'click list' output");
61+ }
62+ installed_packages.insert(p);
63+ }
64+
65+ return installed_packages;
66+}
67+
68+void Interface::get_installed_packages(std::function<void(PackageSet, InterfaceError)> callback)
69+{
70+ std::string command = "click list";
71+ qDebug() << "Running command:" << command.c_str();
72+ run_process(command, [callback](int code, const std::string& stdout_data, const std::string& stderr_data) {
73+ if (code == 0) {
74+ try {
75+ PackageSet package_names = package_names_from_stdout(stdout_data);
76+ callback(package_names, InterfaceError::NoError);
77+ } catch (...) {
78+ qWarning() << "Can't parse 'click list' output: " << QString::fromStdString(stdout_data);
79+ callback({}, InterfaceError::ParseError);
80+ }
81+ } else {
82+ qWarning() << "Error" << code << "running 'click list': " << QString::fromStdString(stderr_data);
83+ callback({}, InterfaceError::CallError);
84+ }
85+ });
86+}
87+
88 void Interface::get_manifest_for_app(const std::string &app_id,
89 std::function<void(Manifest, InterfaceError)> callback)
90 {
91 std::string command = "click info " + app_id;
92 qDebug() << "Running command:" << command.c_str();
93- run_process(command, [callback](int code, const std::string& stdout_data, const std::string&) {
94+ run_process(command, [callback, app_id](int code, const std::string& stdout_data, const std::string& stderr_data) {
95 if (code == 0) {
96 try {
97 Manifest manifest = manifest_from_json(stdout_data);
98 callback(manifest, InterfaceError::NoError);
99 } catch (...) {
100+ qWarning() << "Can't parse 'click info" << QString::fromStdString(app_id)
101+ << "' output: " << QString::fromStdString(stdout_data);
102 callback(Manifest(), InterfaceError::ParseError);
103 }
104 } else {
105+ qWarning() << "Error" << code << "running 'click info" << QString::fromStdString(app_id)
106+ << "': " << QString::fromStdString(stderr_data);
107 callback(Manifest(), InterfaceError::CallError);
108 }
109 });
110
111=== modified file 'libclickscope/click/interface.h'
112--- libclickscope/click/interface.h 2014-06-13 20:44:52 +0000
113+++ libclickscope/click/interface.h 2014-06-16 14:09:25 +0000
114@@ -38,6 +38,7 @@
115 #include <unordered_set>
116
117 #include "application.h"
118+#include "package.h"
119
120 namespace click
121 {
122@@ -59,7 +60,14 @@
123 std::string name;
124 std::string version;
125 std::string first_app_name;
126+ std::string first_scope_id;
127 bool removable = false;
128+ bool has_any_apps() const {
129+ return !first_app_name.empty();
130+ }
131+ bool has_any_scopes() const {
132+ return !first_scope_id.empty();
133+ }
134 };
135
136 enum class InterfaceError {NoError, CallError, ParseError};
137@@ -89,6 +97,7 @@
138 static bool is_icon_identifier(const std::string &icon_id);
139 static std::string add_theme_scheme(const std::string &filename);
140 virtual void get_manifests(std::function<void(ManifestList, InterfaceError)> callback);
141+ virtual void get_installed_packages(std::function<void(PackageSet, InterfaceError)> callback);
142 virtual void get_manifest_for_app(const std::string &app_id, std::function<void(Manifest, InterfaceError)> callback);
143 virtual void get_dotdesktop_filename(const std::string &app_id,
144 std::function<void(std::string filename, InterfaceError)> callback);
145
146=== modified file 'libclickscope/click/package.cpp'
147--- libclickscope/click/package.cpp 2014-06-12 21:05:25 +0000
148+++ libclickscope/click/package.cpp 2014-06-16 14:09:25 +0000
149@@ -41,10 +41,11 @@
150 }
151
152 bool operator==(const Package& lhs, const Package& rhs) {
153- return lhs.name == rhs.name &&
154- lhs.title == rhs.title &&
155- lhs.price == rhs.price &&
156- lhs.icon_url == rhs.icon_url;
157+ // We can't include the version in the comparison here, because this
158+ // comparison is used by the sorted_set, that we use to compare a package
159+ // installed locally on the device with a (possibly updated) package available in the store.
160+ return lhs.name == rhs.name;
161+
162 }
163
164 bool operator==(const PackageDetails& lhs, const PackageDetails& rhs) {
165
166=== modified file 'libclickscope/click/package.h'
167--- libclickscope/click/package.h 2014-06-13 19:58:45 +0000
168+++ libclickscope/click/package.h 2014-06-16 14:09:25 +0000
169@@ -92,9 +92,18 @@
170 std::string url;
171 std::string version;
172 void matches (std::string query, std::function<bool> callback);
173+
174+ struct hash_name {
175+ public :
176+ size_t operator()(const Package &package ) const
177+ {
178+ return std::hash<std::string>()(package.name);
179+ }
180+ };
181 };
182
183 typedef std::vector<Package> Packages;
184+typedef std::unordered_set<Package, Package::hash_name> PackageSet;
185
186 Package package_from_json_node(const Json::Value& item);
187 Packages package_list_from_json(const std::string& json);
188
189=== modified file 'libclickscope/click/preview.cpp'
190--- libclickscope/click/preview.cpp 2014-06-12 21:05:25 +0000
191+++ libclickscope/click/preview.cpp 2014-06-16 14:09:25 +0000
192@@ -36,6 +36,7 @@
193 #include <boost/algorithm/string/replace.hpp>
194
195 #include <unity/UnityExceptions.h>
196+#include <unity/scopes/CannedQuery.h>
197 #include <unity/scopes/PreviewReply.h>
198 #include <unity/scopes/Variant.h>
199 #include <unity/scopes/VariantBuilder.h>
200@@ -500,14 +501,23 @@
201 scopes::PreviewWidgetList widgets;
202 scopes::PreviewWidget buttons("buttons", "actions");
203 scopes::VariantBuilder builder;
204+
205+ std::string open_label = _("Open");
206+
207+ if (!manifest.has_any_apps() && manifest.has_any_scopes()) {
208+ open_label = _("Search");
209+ }
210+
211 if (!uri.empty())
212 {
213 builder.add_tuple(
214 {
215 {"id", scopes::Variant(click::Preview::Actions::OPEN_CLICK)},
216- {"label", scopes::Variant(_("Open"))},
217+ {"label", scopes::Variant(open_label)},
218 {"uri", scopes::Variant(uri)}
219 });
220+ qDebug() << "Adding button" << QString::fromStdString(open_label) << "-"
221+ << QString::fromStdString(uri);
222 }
223 if (manifest.removable)
224 {
225@@ -523,30 +533,42 @@
226 return widgets;
227 }
228
229-void InstalledPreview::getApplicationUri(const Manifest& /*manifest*/, std::function<void(const std::string&)> callback)
230+void InstalledPreview::getApplicationUri(const Manifest& manifest, std::function<void(const std::string&)> callback)
231 {
232- std::string uri;
233 QString app_url = QString::fromStdString(result.uri());
234
235 // asynchronously get application uri based on app name, if the uri is not application://.
236 // this can happen if the app was just installed and we have its http uri from the Result.
237 if (!app_url.startsWith("application:///")) {
238 const std::string name = result["name"].get_string();
239- qt::core::world::enter_with_task([this, name, callback] ()
240- {
241- click::Interface().get_dotdesktop_filename(name,
242- [callback] (std::string val, click::InterfaceError error) {
243- std::string uri;
244- if (error == click::InterfaceError::NoError) {
245- uri = "application:///" + val;
246- }
247- callback(uri);
248- }
249- );
250- });
251+
252+ if (manifest.has_any_apps()) {
253+ qt::core::world::enter_with_task([this, name, callback] ()
254+ {
255+ click::Interface().get_dotdesktop_filename(name,
256+ [callback, name] (std::string val, click::InterfaceError error) {
257+ std::string uri;
258+ if (error == click::InterfaceError::NoError) {
259+ uri = "application:///" + val;
260+ } else {
261+ qWarning() << "Can't get .desktop filename for"
262+ << QString::fromStdString(name);
263+ }
264+ callback(uri);
265+ }
266+ );
267+ });
268+ } else {
269+ if (manifest.has_any_scopes()) {
270+ unity::scopes::CannedQuery cq(manifest.first_scope_id);
271+ auto scope_uri = cq.to_uri();
272+ qDebug() << "Found uri for scope" << QString::fromStdString(manifest.first_scope_id)
273+ << "-" << QString::fromStdString(scope_uri);
274+ callback(scope_uri);
275+ }
276+ }
277 } else {
278- uri = app_url.toStdString();
279- callback(uri);
280+ callback(result.uri());
281 }
282 }
283
284
285=== modified file 'libclickscope/tests/fake_json.h'
286--- libclickscope/tests/fake_json.h 2014-05-26 14:27:31 +0000
287+++ libclickscope/tests/fake_json.h 2014-06-16 14:09:25 +0000
288@@ -176,4 +176,76 @@
289 }
290 )foo";
291
292+const std::string FAKE_JSON_MANIFEST_ONE_APP = R"foo(
293+ {
294+ "_removable": 1,
295+ "name": "com.example.fake-app",
296+ "version": "0.1",
297+ "hooks": {
298+ "fake-app": {
299+ "apparmor": "fake-app.json",
300+ "desktop": "fake-app.desktop"
301+ }
302+ }
303+ }
304+)foo";
305+
306+const std::string FAKE_JSON_MANIFEST_ONE_SCOPE = R"foo(
307+ {
308+ "_removable": 1,
309+ "name": "com.example.fake-scope",
310+ "version": "0.1",
311+ "hooks": {
312+ "fake-scope": {
313+ "apparmor": "scope-security.json",
314+ "scope": "fake-scope"
315+ }
316+ }
317+ }
318+)foo";
319+
320+const std::string FAKE_JSON_MANIFEST_ONE_APP_ONE_SCOPE = R"foo(
321+ {
322+ "_removable": 1,
323+ "name": "com.example.fake-1app-1scope",
324+ "version": "0.1",
325+ "hooks": {
326+ "fake-app": {
327+ "apparmor": "fake-app.json",
328+ "desktop": "fake-app.desktop"
329+ },
330+ "fake-scope": {
331+ "apparmor": "scope-security.json",
332+ "scope": "fake-scope"
333+ }
334+ }
335+ }
336+)foo";
337+
338+const std::string FAKE_JSON_MANIFEST_TWO_APPS_TWO_SCOPES = R"foo(
339+ {
340+ "_removable": 1,
341+ "name": "com.example.fake-2apps-2scopes",
342+ "version": "0.1",
343+ "hooks": {
344+ "fake-app1": {
345+ "apparmor": "fake-app1.json",
346+ "desktop": "fake-app1.desktop"
347+ },
348+ "fake-app2": {
349+ "apparmor": "fake-app2.json",
350+ "desktop": "fake-app2.desktop"
351+ },
352+ "fake-scope1": {
353+ "apparmor": "scope-security1.json",
354+ "scope": "fake-scope1"
355+ },
356+ "fake-scope2": {
357+ "apparmor": "scope-security1.json",
358+ "scope": "fake-scope2"
359+ }
360+ }
361+ }
362+)foo";
363+
364 #endif // FAKE_JSON_H
365
366=== modified file 'libclickscope/tests/test_interface.cpp'
367--- libclickscope/tests/test_interface.cpp 2014-06-13 20:44:52 +0000
368+++ libclickscope/tests/test_interface.cpp 2014-06-16 14:09:25 +0000
369@@ -119,6 +119,7 @@
370 public:
371 MOCK_METHOD2(manifest_callback, void(Manifest, InterfaceError));
372 MOCK_METHOD2(manifests_callback, void(ManifestList, InterfaceError));
373+ MOCK_METHOD2(installed_callback, void(PackageSet, InterfaceError));
374 };
375
376 }
377@@ -447,6 +448,40 @@
378 EXPECT_FALSE(iface.show_desktop_apps());
379 }
380
381+TEST(ClickInterface, testManifestFromJsonOneApp)
382+{
383+ Manifest m = manifest_from_json(FAKE_JSON_MANIFEST_ONE_APP);
384+ ASSERT_EQ(m.first_app_name, "fake-app");
385+ ASSERT_TRUE(m.has_any_apps());
386+ ASSERT_FALSE(m.has_any_scopes());
387+}
388+
389+TEST(ClickInterface, testManifestFromJsonOneScope)
390+{
391+ Manifest m = manifest_from_json(FAKE_JSON_MANIFEST_ONE_SCOPE);
392+ ASSERT_EQ(m.first_scope_id, "com.example.fake-scope");
393+ ASSERT_FALSE(m.has_any_apps());
394+ ASSERT_TRUE(m.has_any_scopes());
395+}
396+
397+TEST(ClickInterface, testManifestFromJsonOneAppOneScope)
398+{
399+ Manifest m = manifest_from_json(FAKE_JSON_MANIFEST_ONE_APP_ONE_SCOPE);
400+ ASSERT_EQ(m.first_app_name, "fake-app");
401+ ASSERT_EQ(m.first_scope_id, "com.example.fake-1app-1scope");
402+ ASSERT_TRUE(m.has_any_apps());
403+ ASSERT_TRUE(m.has_any_scopes());
404+}
405+
406+TEST(ClickInterface, testManifestFromJsonTwoAppsTwoScopes)
407+{
408+ Manifest m = manifest_from_json(FAKE_JSON_MANIFEST_TWO_APPS_TWO_SCOPES);
409+ ASSERT_EQ(m.first_app_name, "fake-app1");
410+ ASSERT_EQ(m.first_scope_id, "com.example.fake-2apps-2scopes");
411+ ASSERT_TRUE(m.has_any_apps());
412+ ASSERT_TRUE(m.has_any_scopes());
413+}
414+
415 TEST(ClickInterface, testGetManifestForAppCorrectCommand)
416 {
417 FakeClickInterface iface;
418@@ -584,3 +619,64 @@
419 ASSERT_TRUE(manifests.size() == expected.size());
420 });
421 }
422+
423+TEST(ClickInterface, testGetInstalledPackagesCorrectCommand)
424+{
425+ FakeClickInterface iface;
426+ std::string command = "click list";
427+ EXPECT_CALL(iface, run_process(command, _)).
428+ Times(1);
429+ iface.get_installed_packages([](PackageSet, InterfaceError){});
430+}
431+
432+TEST_F(ClickInterfaceTest, testGetInstalledPackagesParseError)
433+{
434+ FakeClickInterface iface;
435+ EXPECT_CALL(iface, run_process(_, _)).
436+ Times(1).
437+ WillOnce(Invoke([&](const std::string&,
438+ std::function<void(int, const std::string&,
439+ const std::string&)> callback){
440+ callback(0, "valid.package\t1.0\nINVALID LINE", "");
441+ }));
442+ EXPECT_CALL(*this, installed_callback(_, InterfaceError::ParseError));
443+ iface.get_installed_packages([this](PackageSet package_names, InterfaceError error){
444+ installed_callback(package_names, error);
445+ });
446+}
447+
448+TEST_F(ClickInterfaceTest, testGetInstalledPackagesCommandFailed)
449+{
450+ FakeClickInterface iface;
451+ EXPECT_CALL(iface, run_process(_, _)).
452+ Times(1).
453+ WillOnce(Invoke([&](const std::string&,
454+ std::function<void(int, const std::string&,
455+ const std::string&)> callback){
456+ callback(-1, "", "CRITICAL: FAIL");
457+ }));
458+ EXPECT_CALL(*this, installed_callback(_, InterfaceError::CallError));
459+ iface.get_installed_packages([this](PackageSet package_names, InterfaceError error){
460+ installed_callback(package_names, error);
461+ });
462+}
463+
464+TEST_F(ClickInterfaceTest, testGetInstalledPackagesParsed)
465+{
466+ FakeClickInterface iface;
467+ std::string sample_stdout = "ABC\t0.1\nDEF\t0.2\n";
468+ PackageSet expected{{"ABC", "0.1"}, {"DEF", "0.2"}};
469+
470+ EXPECT_CALL(iface, run_process(_, _)).
471+ Times(1).
472+ WillOnce(Invoke([&](const std::string&,
473+ std::function<void(int, const std::string&,
474+ const std::string&)> callback){
475+ callback(0, sample_stdout, "");
476+ }));
477+ iface.get_installed_packages([expected](PackageSet package_names, InterfaceError error){
478+ ASSERT_EQ(error, InterfaceError::NoError);
479+ ASSERT_EQ(package_names, expected);
480+ });
481+}
482+
483
484=== modified file 'scope/clickstore/store-query.cpp'
485--- scope/clickstore/store-query.cpp 2014-06-12 21:05:25 +0000
486+++ scope/clickstore/store-query.cpp 2014-06-16 14:09:25 +0000
487@@ -48,6 +48,8 @@
488
489 #include <click/click-i18n.h>
490
491+using namespace click;
492+
493 namespace
494 {
495
496@@ -60,10 +62,10 @@
497 },
498 "components" : {
499 "title" : "title",
500+ "subtitle": "subtitle",
501 "art" : {
502 "field": "art",
503- "aspect-ratio": 1.6,
504- "fill-mode": "fit"
505+ "aspect-ratio": 1.13
506 }
507 }
508 }
509@@ -82,7 +84,7 @@
510 "mascot" : {
511 "field": "art"
512 },
513- "subtitle": "publisher"
514+ "subtitle": "subtitle"
515 }
516 }
517 )";
518@@ -119,9 +121,7 @@
519 impl->search_operation.cancel();
520 }
521
522-namespace
523-{
524-click::Interface& clickInterfaceInstance()
525+click::Interface& click::Query::clickInterfaceInstance()
526 {
527 static QSharedPointer<click::KeyFileLocator> keyFileLocator(new click::KeyFileLocator());
528 static click::Interface iface(keyFileLocator);
529@@ -129,8 +129,6 @@
530 return iface;
531 }
532
533-}
534-
535 bool click::Query::push_result(scopes::SearchReplyProxy const& searchReply, const scopes::CategorisedResult &res)
536 {
537 return searchReply->push(res);
538@@ -158,7 +156,7 @@
539 }
540
541 void click::Query::add_available_apps(scopes::SearchReplyProxy const& searchReply,
542- const std::set<std::string>& locallyInstalledApps,
543+ const PackageSet& installedPackages,
544 const std::string& categoryTemplate)
545 {
546 scopes::CategoryRenderer categoryRenderer(categoryTemplate);
547@@ -166,7 +164,7 @@
548
549 run_under_qt([=]()
550 {
551- auto search_cb = [this, searchReply, category, locallyInstalledApps](click::Packages packages) {
552+ auto search_cb = [this, searchReply, category, installedPackages](Packages packages) {
553 qDebug("search callback");
554
555 // handle packages data
556@@ -174,16 +172,20 @@
557 qDebug() << "pushing result" << QString::fromStdString(p.name);
558 try {
559 scopes::CategorisedResult res(category);
560- // TODO: mark as installed
561- if (locallyInstalledApps.count(p.name) > 0) {
562- qDebug() << "already installed" << QString::fromStdString(p.name);
563- continue;
564- }
565 res.set_title(p.title);
566 res.set_art(p.icon_url);
567 res.set_uri(p.url);
568 res[click::Query::ResultKeys::NAME] = p.name;
569- res[click::Query::ResultKeys::INSTALLED] = false;
570+ auto installed = installedPackages.find(p);
571+ if (installed != installedPackages.end()) {
572+ res[click::Query::ResultKeys::INSTALLED] = true;
573+ res["subtitle"] = _("✔ INSTALLED");
574+ res[click::Query::ResultKeys::VERSION] = installed->version;
575+ } else {
576+ res[click::Query::ResultKeys::INSTALLED] = false;
577+ // TODO: get the real price from the webservice (upcoming branch)
578+ res["subtitle"] = _("FREE");
579+ }
580
581 this->push_result(searchReply, res);
582 } catch(const std::exception& e){
583@@ -201,6 +203,23 @@
584 });
585 }
586
587+PackageSet click::Query::get_installed_packages()
588+{
589+ std::promise<PackageSet> installed_promise;
590+ std::future<PackageSet> installed_future = installed_promise.get_future();
591+
592+ run_under_qt([&]()
593+ {
594+ clickInterfaceInstance().get_installed_packages(
595+ [&installed_promise](PackageSet installedPackages, InterfaceError){
596+ installed_promise.set_value(installedPackages);
597+ });
598+ });
599+
600+ return installed_future.get();
601+}
602+
603+
604 void click::Query::run(scopes::SearchReplyProxy const& searchReply)
605 {
606 auto query = impl->query.query_string();
607@@ -208,12 +227,6 @@
608 if (query.empty()) {
609 categoryTemplate = CATEGORY_APPS_DISPLAY;
610 }
611- auto localResults = clickInterfaceInstance().find_installed_apps(query);
612-
613- std::set<std::string> locallyInstalledApps;
614- for(const auto& app : localResults) {
615- locallyInstalledApps.insert(app.name);
616- }
617
618 static const std::string no_net_hint("no-internet");
619 if (impl->meta.contains_hint(no_net_hint))
620@@ -223,8 +236,7 @@
621 {
622 return;
623 }
624-
625 }
626
627- add_available_apps(searchReply, locallyInstalledApps, categoryTemplate);
628+ add_available_apps(searchReply, get_installed_packages(), categoryTemplate);
629 }
630
631=== modified file 'scope/clickstore/store-query.h'
632--- scope/clickstore/store-query.h 2014-05-27 06:57:52 +0000
633+++ scope/clickstore/store-query.h 2014-06-16 14:09:25 +0000
634@@ -36,8 +36,7 @@
635 namespace scopes = unity::scopes;
636
637 #include <QSharedPointer>
638-#include <set>
639-
640+#include <click/interface.h>
641
642 namespace click
643 {
644@@ -78,7 +77,10 @@
645 virtual void run(scopes::SearchReplyProxy const& reply) override;
646
647 protected:
648- virtual void add_available_apps(const scopes::SearchReplyProxy &searchReply, const std::set<std::string> &locallyInstalledApps, const std::string &category);
649+ virtual void add_available_apps(const scopes::SearchReplyProxy &searchReply,
650+ const PackageSet &installedPackages, const std::string &category);
651+ virtual click::Interface& clickInterfaceInstance();
652+ virtual PackageSet get_installed_packages();
653 virtual bool push_result(const scopes::SearchReplyProxy &searchReply, scopes::CategorisedResult const& res);
654 virtual void finished(const scopes::SearchReplyProxy &searchReply);
655 virtual scopes::Category::SCPtr register_category(scopes::SearchReplyProxy const& searchReply,
656
657=== modified file 'scope/tests/test_query.cpp'
658--- scope/tests/test_query.cpp 2014-06-12 21:05:25 +0000
659+++ scope/tests/test_query.cpp 2014-06-16 14:09:25 +0000
660@@ -47,6 +47,7 @@
661 #include <unity/scopes/SearchReply.h>
662
663 using namespace ::testing;
664+using namespace click;
665
666 namespace
667 {
668@@ -98,18 +99,20 @@
669
670 }
671 void wrap_add_available_apps(const scopes::SearchReplyProxy &searchReply,
672- const std::set<std::string> &locallyInstalledApps,
673+ const PackageSet &installedPackages,
674 const std::string& categoryTemplate)
675 {
676- add_available_apps(searchReply, locallyInstalledApps, categoryTemplate);
677+ add_available_apps(searchReply, installedPackages, categoryTemplate);
678 }
679 MOCK_METHOD2(push_result, bool(scopes::SearchReplyProxy const&, scopes::CategorisedResult const&));
680+ MOCK_METHOD0(clickInterfaceInstance, click::Interface&());
681 MOCK_METHOD1(finished, void(scopes::SearchReplyProxy const&));
682 MOCK_METHOD5(register_category, scopes::Category::SCPtr(const scopes::SearchReplyProxy &searchReply,
683 const std::string &id,
684 const std::string &title,
685 const std::string &icon,
686 const scopes::CategoryRenderer &renderer_template));
687+ using click::Query::get_installed_packages; // allow tests to access protected method
688 };
689
690 class MockQueryRun : public MockQueryBase {
691@@ -121,11 +124,12 @@
692 }
693 MOCK_METHOD3(add_available_apps,
694 void(scopes::SearchReplyProxy const&searchReply,
695- const std::set<std::string> &locallyInstalledApps,
696+ const PackageSet &locallyInstalledApps,
697 const std::string& categoryTemplate));
698 MOCK_METHOD3(push_local_results, void(scopes::SearchReplyProxy const &replyProxy,
699 std::vector<click::Application> const &apps,
700 std::string& categoryTemplate));
701+ MOCK_METHOD0(get_installed_packages, PackageSet());
702 };
703
704 class FakeCategory : public scopes::Category
705@@ -144,7 +148,7 @@
706 {
707 MockIndex mock_index;
708 scopes::SearchMetadata metadata("en_EN", "phone");
709- std::set<std::string> no_installed_packages;
710+ PackageSet no_installed_packages;
711 const unity::scopes::CannedQuery query("foo.scope", FAKE_QUERY, "");
712 MockQuery q(query, mock_index, metadata);
713 EXPECT_CALL(mock_index, do_search(FAKE_QUERY, _)).Times(1);
714@@ -163,7 +167,7 @@
715 };
716 MockIndex mock_index(packages);
717 scopes::SearchMetadata metadata("en_EN", "phone");
718- std::set<std::string> no_installed_packages;
719+ PackageSet no_installed_packages;
720 const unity::scopes::CannedQuery query("foo.scope", FAKE_QUERY, "");
721 MockQuery q(query, mock_index, metadata);
722 EXPECT_CALL(mock_index, do_search(FAKE_QUERY, _));
723@@ -185,7 +189,7 @@
724 };
725 MockIndex mock_index(packages);
726 scopes::SearchMetadata metadata("en_EN", "phone");
727- std::set<std::string> no_installed_packages;
728+ PackageSet no_installed_packages;
729 const unity::scopes::CannedQuery query("foo.scope", FAKE_QUERY, "");
730 MockQuery q(query, mock_index, metadata);
731 EXPECT_CALL(mock_index, do_search(FAKE_QUERY, _));
732@@ -206,37 +210,94 @@
733 };
734 MockIndex mock_index(packages);
735 scopes::SearchMetadata metadata("en_EN", "phone");
736+ PackageSet no_installed_packages;
737 const unity::scopes::CannedQuery query("foo.scope", FAKE_QUERY, "");
738 MockQueryRun q(query, mock_index, metadata);
739 auto reply = scopes::SearchReplyProxy();
740- EXPECT_CALL(q, add_available_apps(reply, _, _));
741+ EXPECT_CALL(q, get_installed_packages()).WillOnce(Return(no_installed_packages));
742+ EXPECT_CALL(q, add_available_apps(reply, no_installed_packages, _));
743
744 q.run(reply);
745 }
746
747 MATCHER_P(HasPackageName, n, "") { return arg[click::Query::ResultKeys::NAME].get_string() == n; }
748-
749-TEST(QueryTest, testDuplicatesFilteredOnPackageName)
750-{
751- click::Packages packages {
752- {"org.example.app1", "app title1", 0.0, "icon", "uri"},
753- {"org.example.app2", "app title2", 0.0, "icon", "uri"}
754- };
755- MockIndex mock_index(packages);
756- scopes::SearchMetadata metadata("en_EN", "phone");
757- std::set<std::string> one_installed_package {
758- "org.example.app2"
759- };
760- const unity::scopes::CannedQuery query("foo.scope", FAKE_QUERY, "");
761- MockQuery q(query, mock_index, metadata);
762- EXPECT_CALL(mock_index, do_search(FAKE_QUERY, _));
763-
764- scopes::CategoryRenderer renderer("{}");
765- auto ptrCat = std::make_shared<FakeCategory>("id", "", "", renderer);
766- EXPECT_CALL(q, register_category(_, _, _, _, _)).WillOnce(Return(ptrCat));
767-
768- scopes::SearchReplyProxy reply;
769- auto expected_name = packages.front().name;
770- EXPECT_CALL(q, push_result(_, HasPackageName(expected_name)));
771- q.wrap_add_available_apps(reply, one_installed_package, FAKE_CATEGORY_TEMPLATE);
772+MATCHER_P(IsInstalled, b, "") { return arg[click::Query::ResultKeys::INSTALLED].get_bool() == b; }
773+
774+TEST(QueryTest, testDuplicatesNotFilteredAnymore)
775+{
776+ click::Packages packages {
777+ {"org.example.app1", "app title1", 0.0, "icon", "uri"},
778+ {"org.example.app2", "app title2", 0.0, "icon", "uri"}
779+ };
780+ MockIndex mock_index(packages);
781+ scopes::SearchMetadata metadata("en_EN", "phone");
782+ PackageSet one_installed_package {
783+ {"org.example.app2", "0.2"}
784+ };
785+ const unity::scopes::CannedQuery query("foo.scope", FAKE_QUERY, "");
786+ MockQuery q(query, mock_index, metadata);
787+ EXPECT_CALL(mock_index, do_search(FAKE_QUERY, _));
788+
789+ scopes::CategoryRenderer renderer("{}");
790+ auto ptrCat = std::make_shared<FakeCategory>("id", "", "", renderer);
791+ EXPECT_CALL(q, register_category(_, _, _, _, _)).WillOnce(Return(ptrCat));
792+
793+ scopes::SearchReplyProxy reply;
794+ auto expected_name1 = packages.front().name;
795+ EXPECT_CALL(q, push_result(_, HasPackageName(expected_name1)));
796+ auto expected_name2 = packages.back().name;
797+ EXPECT_CALL(q, push_result(_, HasPackageName(expected_name2)));
798+ q.wrap_add_available_apps(reply, one_installed_package, FAKE_CATEGORY_TEMPLATE);
799+}
800+
801+TEST(QueryTest, testInstalledPackagesFlaggedAsSuch)
802+{
803+ click::Packages packages {
804+ {"org.example.app1", "app title1", 0.0, "icon", "uri"},
805+ {"org.example.app2", "app title2", 0.0, "icon", "uri"}
806+ };
807+ MockIndex mock_index(packages);
808+ scopes::SearchMetadata metadata("en_EN", "phone");
809+ PackageSet one_installed_package {
810+ {"org.example.app2", "0.2"}
811+ };
812+ const unity::scopes::CannedQuery query("foo.scope", FAKE_QUERY, "");
813+ MockQuery q(query, mock_index, metadata);
814+ EXPECT_CALL(mock_index, do_search(FAKE_QUERY, _));
815+
816+ scopes::CategoryRenderer renderer("{}");
817+ auto ptrCat = std::make_shared<FakeCategory>("id", "", "", renderer);
818+ EXPECT_CALL(q, register_category(_, _, _, _, _)).WillOnce(Return(ptrCat));
819+
820+ scopes::SearchReplyProxy reply;
821+ EXPECT_CALL(q, push_result(_, IsInstalled(true)));
822+ EXPECT_CALL(q, push_result(_, IsInstalled(false)));
823+ q.wrap_add_available_apps(reply, one_installed_package, FAKE_CATEGORY_TEMPLATE);
824+}
825+
826+class FakeInterface : public click::Interface
827+{
828+public:
829+ MOCK_METHOD1(get_installed_packages, void(std::function<void(PackageSet, click::InterfaceError)> callback));
830+};
831+
832+TEST(QueryTest, testGetInstalledPackages)
833+{
834+ click::Packages uninstalled_packages {
835+ {"name", "title", 0.0, "icon", "uri"}
836+ };
837+ MockIndex mock_index(uninstalled_packages);
838+ scopes::SearchMetadata metadata("en_EN", "phone");
839+ const unity::scopes::CannedQuery query("foo.scope", FAKE_QUERY, "");
840+ MockQuery q(query, mock_index, metadata);
841+ PackageSet installed_packages{{"package_1", "0.1"}};
842+
843+ FakeInterface fake_interface;
844+ EXPECT_CALL(q, clickInterfaceInstance()).WillOnce(ReturnRef(fake_interface));
845+ EXPECT_CALL(fake_interface, get_installed_packages(_)).WillOnce(Invoke(
846+ [&](std::function<void(PackageSet, click::InterfaceError)> callback){
847+ callback(installed_packages, click::InterfaceError::NoError);
848+ }));
849+
850+ ASSERT_EQ(q.get_installed_packages(), installed_packages);
851 }

Subscribers

People subscribed via source and target branches

to all changes: