Merge lp:~alecu/unity-scope-click/uninstallable-scopes into lp:unity-scope-click/devel
- uninstallable-scopes
- Merge into devel
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 |
Related bugs: |
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
Description of the change
PS Jenkins bot (ps-jenkins) wrote : | # |
- 292. By Alejandro J. Cura
-
Show debugging information when creating uri to open scope
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:292
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 293. By Alejandro J. Cura
-
Add more debugging logs
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:293
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
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.
dobey (dobey) wrote : | # |
/home/phablet/
/home/phablet/
libclickscope/
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?
dobey (dobey) : | # |
- 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
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:295
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:296
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
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::
241 + uri = "application:///" + val;
242 + }
Can you log an error here, if any?
13 +#include <streambuf>
Is this needed?
- 297. By Alejandro J. Cura
-
Removed unused includes and added error messages requested in Pawel's review
Paweł Stołowski (stolowski) wrote : | # |
Thanks, LGTM!
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.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:297
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Preview Diff
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 | } |
PASSED: Continuous integration, rev:291 jenkins. qa.ubuntu. com/job/ unity-team- unity-scope- click-devel- ci/111/ jenkins. qa.ubuntu. com/job/ unity-team- unity-scope- click-devel- utopic- amd64-ci/ 86 jenkins. qa.ubuntu. com/job/ unity-team- unity-scope- click-devel- utopic- armhf-ci/ 85 jenkins. qa.ubuntu. com/job/ unity-team- unity-scope- click-devel- utopic- armhf-ci/ 85/artifact/ work/output/ *zip*/output. zip jenkins. qa.ubuntu. com/job/ unity-team- unity-scope- click-devel- utopic- i386-ci/ 85
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/unity- team-unity- scope-click- devel-ci/ 111/rebuild
http://