Merge lp:~townsend/libertine-scope/release-1.3.2 into lp:libertine-scope/release

Proposed by Christopher Townsend
Status: Merged
Approved by: Larry Price
Approved revision: 56
Merged at revision: 57
Proposed branch: lp:~townsend/libertine-scope/release-1.3.2
Merge into: lp:libertine-scope/release
Diff against target: 494 lines (+203/-57)
15 files modified
CMakeLists.txt (+1/-1)
debian/changelog (+11/-0)
scope/apps/action.cpp (+26/-15)
scope/apps/action.h (+21/-15)
scope/apps/config.h.in (+1/-3)
scope/apps/libertine.cpp (+1/-2)
scope/apps/libertine.h (+1/-1)
scope/apps/query.cpp (+7/-5)
scope/apps/query.h (+2/-2)
scope/apps/scope.cpp (+14/-1)
scope/apps/scope.h (+1/-0)
tests/CMakeLists.txt (+2/-0)
tests/mock_hidden_apps.h (+32/-0)
tests/test_action.cpp (+82/-0)
tests/test_query.cpp (+1/-12)
To merge this branch: bzr merge lp:~townsend/libertine-scope/release-1.3.2
Reviewer Review Type Date Requested Status
Larry Price Approve
Libertine CI Bot continuous-integration Pending
Review via email: mp+301524@code.launchpad.net

This proposal supersedes a proposal from 2016-07-29.

Commit message

* Use full scope name in canned queries to properly return from Show/Hide actions.
* Initialize libertine object in Query::run() to allow constructor to execute in time. (LP: #1595944)
* Showing/hiding applications should retain filter state. (LP: #1592407)

To post a comment you must log in.
Revision history for this message
Libertine CI Bot (libertine-ci-bot) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:56
https://jenkins.canonical.com/libertine/job/lp-libertine-scope-ci/63/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/libertine/job/build/224
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=default/182
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=default/182
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=default/182
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=vivid+overlay,testname=default/182
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=xenial+overlay,testname=default/182
    SUCCESS: https://jenkins.canonical.com/libertine/job/test-0-autopkgtest/label=i386,release=yakkety,testname=default/182
    None: https://jenkins.canonical.com/libertine/job/lp-generic-update-mp/170/console
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-0-fetch/227
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=vivid+overlay/212
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=xenial+overlay/212
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-1-sourcepkg/release=yakkety/212
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/205
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=vivid+overlay/205/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/205
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=xenial+overlay/205/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/205
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=amd64,release=yakkety/205/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/205
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=vivid+overlay/205/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/205
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=xenial+overlay/205/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/205
        deb: https://jenkins.canonical.com/libertine/job/build-2-binpkg/arch=i386,release=yakkety/205/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/libertine/job/lp-libertine-scope-ci/63/rebuild

review: Approve (continuous-integration)
Revision history for this message
Larry Price (larryprice) wrote :

lgtm

review: Approve
57. By Christopher Townsend

Change version to 1.3.2.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2016-07-21 16:07:09 +0000
3+++ CMakeLists.txt 2016-08-01 13:24:59 +0000
4@@ -2,7 +2,7 @@
5 cmake_policy(SET CMP0048 NEW)
6
7 project(libertine-scope
8- VERSION 1.3.1
9+ VERSION 1.3.2
10 LANGUAGES CXX)
11
12 # We require at least g++ 4.9, to avoid ABI breakage with earlier versions.
13
14=== modified file 'debian/changelog'
15--- debian/changelog 2016-07-22 13:48:17 +0000
16+++ debian/changelog 2016-08-01 13:24:59 +0000
17@@ -1,3 +1,14 @@
18+libertine-scope (1.3.2-0ubuntu1) UNRELEASED; urgency=medium
19+
20+ [ Larry Price ]
21+ * Use full scope name in canned queries to properly return from Show/Hide
22+ actions.
23+ * Initialize libertine object in Query::run() to allow constructor to execute
24+ in time. (LP: #1595944)
25+ * Showing/hiding applications should retain filter state. (LP: #1592407)
26+
27+ -- Chris Townsend <christopher.townsend@canonical.com> Fri, 29 Jul 2016 12:35:35 -0400
28+
29 libertine-scope (1.3.1+16.10.20160722-0ubuntu1) yakkety; urgency=medium
30
31 [ Chris Townsend ]
32
33=== modified file 'scope/apps/action.cpp'
34--- scope/apps/action.cpp 2016-07-19 20:20:12 +0000
35+++ scope/apps/action.cpp 2016-08-01 13:24:59 +0000
36@@ -29,14 +29,30 @@
37 namespace usc = unity::scopes;
38
39
40+namespace
41+{
42+static usc::ActivationResponse
43+sendToResults(usc::FilterState const& filter_state)
44+{
45+ usc::CannedQuery query(FULLY_QUALIFIED_APPS_SCOPE);
46+ query.set_filter_state(filter_state);
47+ return usc::ActivationResponse(query);
48+}
49+}
50+
51+
52 Action::
53-Action(usc::Result const& result,
54- usc::ActionMetadata const& metadata,
55- std::string const& action_id,
56- std::shared_ptr<HiddenApps> hidden)
57- : usc::ActivationQueryBase(result, metadata),
58- action_id_(action_id),
59- hidden_(hidden)
60+Action(unity::scopes::Result const& result,
61+ unity::scopes::ActionMetadata const& metadata,
62+ std::string const& action_id,
63+ Action::OpenUriAction open_action,
64+ std::shared_ptr<HiddenApps> hidden,
65+ unity::scopes::FilterState const& filter_state)
66+ : usc::ActivationQueryBase(result, metadata)
67+ , action_id_(action_id)
68+ , open_action_(open_action)
69+ , hidden_(hidden)
70+ , filter_state_(filter_state)
71 {
72 }
73
74@@ -46,22 +62,17 @@
75 {
76 if (action_id_ == "open")
77 {
78- url_dispatch_send(result().uri().c_str() , NULL, NULL);
79- return usc::ActivationResponse(usc::ActivationResponse::Status::NotHandled);
80+ open_action_(result().uri());
81 }
82 else if (action_id_ == "hide")
83 {
84 hidden_->add(QString::fromStdString(result()["app_id"].get_string()));
85-
86- usc::CannedQuery cq(SCOPE_PKG);
87- return usc::ActivationResponse(cq);
88+ return sendToResults(filter_state_);
89 }
90 else if (action_id_ == "show")
91 {
92 hidden_->remove(QString::fromStdString(result()["app_id"].get_string()));
93-
94- usc::CannedQuery cq(SCOPE_PKG);
95- return usc::ActivationResponse(cq);
96+ return sendToResults(filter_state_);
97 }
98 return usc::ActivationResponse(usc::ActivationResponse::Status::NotHandled);
99 }
100
101=== modified file 'scope/apps/action.h'
102--- scope/apps/action.h 2016-07-12 18:26:07 +0000
103+++ scope/apps/action.h 2016-08-01 13:24:59 +0000
104@@ -15,31 +15,37 @@
105 * * Authored by:
106 * Kyle Nitzsche <kyle.nitzsche@canonical.com>
107 */
108-
109 #ifndef SCOPE_ACTION_H_
110 #define SCOPE_ACTION_H_
111
112-#include "scope/apps/scope.h"
113+
114 #include <unity/scopes/ActionMetadata.h>
115 #include <unity/scopes/ActivationQueryBase.h>
116 #include <unity/scopes/ActivationResponse.h>
117 #include <unity/scopes/Result.h>
118
119+
120 class HiddenApps;
121
122 class Action : public unity::scopes::ActivationQueryBase {
123- public:
124- Action(unity::scopes::Result const& result,
125- unity::scopes::ActionMetadata const& metadata,
126- std::string const& action_id,
127- std::shared_ptr<HiddenApps> hidden);
128-
129- virtual ~Action() = default;
130- virtual unity::scopes::ActivationResponse activate() override;
131-
132- private:
133- std::string action_id_;
134- std::string cache_dir_;
135- std::shared_ptr<HiddenApps> hidden_;
136+public:
137+ typedef std::function<void(std::string const&)> OpenUriAction;
138+
139+ explicit Action(unity::scopes::Result const& result,
140+ unity::scopes::ActionMetadata const& metadata,
141+ std::string const& action_id,
142+ OpenUriAction open_action,
143+ std::shared_ptr<HiddenApps> hidden,
144+ unity::scopes::FilterState const& filterState);
145+
146+ virtual ~Action() = default;
147+ virtual unity::scopes::ActivationResponse activate() override;
148+
149+private:
150+ std::string action_id_;
151+ std::string cache_dir_;
152+ OpenUriAction open_action_;
153+ std::shared_ptr<HiddenApps> hidden_;
154+ unity::scopes::FilterState filter_state_;
155 };
156 #endif
157
158=== modified file 'scope/apps/config.h.in'
159--- scope/apps/config.h.in 2016-07-19 20:20:12 +0000
160+++ scope/apps/config.h.in 2016-08-01 13:24:59 +0000
161@@ -16,10 +16,8 @@
162 #ifndef LIBERTINE_SCOPE_CONFIG_H_
163 #define LIBERTINE_SCOPE_CONFIG_H_
164
165-const std::string SCOPE_PKG = "@PACKAGE_NAME@";
166+const std::string FULLY_QUALIFIED_APPS_SCOPE = "@FULLY_QUALIFIED_NAME@";
167 const std::string ROOT_DEPT_ID = "root_dept";
168 const std::string HIDDEN_DEPT_ID = "hidden_dept";
169
170 #endif // LIBERTINE_SCOPE_CONFIG_H_
171-
172-
173
174=== modified file 'scope/apps/libertine.cpp'
175--- scope/apps/libertine.cpp 2016-07-12 18:26:07 +0000
176+++ scope/apps/libertine.cpp 2016-08-01 13:24:59 +0000
177@@ -91,6 +91,5 @@
178 Libertine::UPtr Libertine::
179 from_libertine_cli()
180 {
181- return Libertine::UPtr(new LibertineCli());
182+ return std::make_shared<LibertineCli>();
183 }
184-
185
186=== modified file 'scope/apps/libertine.h'
187--- scope/apps/libertine.h 2016-01-13 00:38:37 +0000
188+++ scope/apps/libertine.h 2016-08-01 13:24:59 +0000
189@@ -29,7 +29,7 @@
190 class Libertine
191 {
192 public:
193- using UPtr = std::unique_ptr<Libertine>;
194+ using UPtr = std::shared_ptr<Libertine>;
195 using ContainerList = std::vector<std::unique_ptr<Container>>;
196
197 /**
198
199=== modified file 'scope/apps/query.cpp'
200--- scope/apps/query.cpp 2016-07-19 13:57:43 +0000
201+++ scope/apps/query.cpp 2016-08-01 13:24:59 +0000
202@@ -121,7 +121,7 @@
203 std::shared_ptr<HiddenApps> hidden,
204 std::shared_ptr<Blacklist> blacklist)
205 : usc::SearchQueryBase(query, metadata)
206- , libertine_(libertine_factory())
207+ , libertine_factory_(libertine_factory)
208 , hidden_(hidden)
209 , blacklist_(blacklist)
210 {
211@@ -135,14 +135,14 @@
212
213
214 QStringList Query::
215-make_filters(usc::SearchReplyProxy const& reply) const
216+make_filters(usc::SearchReplyProxy const& reply, Libertine::UPtr libertine) const
217 {
218 auto filter_state = query().filter_state();
219 QStringList excludes_by_filter;
220 std::list<usc::FilterBase::SCPtr> app_filters;
221
222 //make exclude scope filter for apps
223- for (auto const& container: libertine_->get_container_list())
224+ for (auto const& container: libertine->get_container_list())
225 {
226 usc::OptionSelectorFilter::SPtr filter{usc::OptionSelectorFilter::create(container->id(),
227 EXCLUDED_APPS_FILTER_TITLE + container->name(),
228@@ -208,18 +208,20 @@
229 register_departments(reply);
230 }
231
232+ auto libertine = libertine_factory_();
233+
234 // only provide filters in root department
235 QStringList excludes_by_filter;
236 if (query().department_id().empty())
237 {
238- excludes_by_filter = make_filters(reply);
239+ excludes_by_filter = make_filters(reply, libertine);
240 }
241
242 QRegExp search_query(QString::fromStdString(query().query_string()), Qt::CaseInsensitive);
243 bool has_no_apps = true,
244 all_filtered = true;
245
246- for (auto const& container: libertine_->get_container_list())
247+ for (auto const& container: libertine->get_container_list())
248 {
249 auto category = reply->register_category(container->id(),
250 container->name(),
251
252=== modified file 'scope/apps/query.h'
253--- scope/apps/query.h 2016-07-12 18:26:07 +0000
254+++ scope/apps/query.h 2016-08-01 13:24:59 +0000
255@@ -50,11 +50,11 @@
256
257 private:
258 QStringList get_hidden_department() const;
259- QStringList make_filters(unity::scopes::SearchReplyProxy const& reply) const;
260+ QStringList make_filters(unity::scopes::SearchReplyProxy const& reply, Libertine::UPtr libertine) const;
261 void show_hint(unity::scopes::SearchReplyProxy const& reply, std::string const& reason) const;
262 void parse_blacklist(const std::string& data_dir);
263
264- Libertine::UPtr libertine_;
265+ Libertine::Factory libertine_factory_;
266 std::shared_ptr<HiddenApps> hidden_;
267 std::shared_ptr<Blacklist> blacklist_;
268 };
269
270=== modified file 'scope/apps/scope.cpp'
271--- scope/apps/scope.cpp 2016-07-12 18:26:07 +0000
272+++ scope/apps/scope.cpp 2016-08-01 13:24:59 +0000
273@@ -29,6 +29,16 @@
274 namespace usc = unity::scopes;
275
276
277+namespace
278+{
279+static void
280+open_application(std::string const& app_uri)
281+{
282+ url_dispatch_send(app_uri.c_str(), NULL, NULL);
283+}
284+}
285+
286+
287 Scope::
288 Scope(Libertine::Factory const& libertine_factory)
289 : libertine_factory_(libertine_factory)
290@@ -56,6 +66,7 @@
291 search(usc::CannedQuery const& query,
292 usc::SearchMetadata const& metadata)
293 {
294+ filter_state_ = query.filter_state();
295 return usc::SearchQueryBase::UPtr(new Query(query,
296 metadata,
297 libertine_factory_,
298@@ -81,7 +92,9 @@
299 return usc::ActivationQueryBase::UPtr(new Action(result,
300 metadata,
301 action_id,
302- std::make_shared<HiddenApps>(cache_directory())));
303+ open_application,
304+ std::make_shared<HiddenApps>(cache_directory()),
305+ filter_state_));
306 }
307
308
309
310=== modified file 'scope/apps/scope.h'
311--- scope/apps/scope.h 2016-07-12 18:26:07 +0000
312+++ scope/apps/scope.h 2016-08-01 13:24:59 +0000
313@@ -78,6 +78,7 @@
314
315 private:
316 Libertine::Factory libertine_factory_;
317+ unity::scopes::FilterState filter_state_;
318 };
319
320 #endif // LIBERTINE_SCOPE_SCOPE_H_
321
322=== modified file 'tests/CMakeLists.txt'
323--- tests/CMakeLists.txt 2016-07-12 18:26:07 +0000
324+++ tests/CMakeLists.txt 2016-08-01 13:24:59 +0000
325@@ -7,6 +7,7 @@
326 add_executable(${test_name}_exe
327 fake_container.cpp
328 fake_libertine.cpp
329+ mock_hidden_apps.h
330 ${test_name}.cpp
331 )
332
333@@ -25,5 +26,6 @@
334 create_test(test_query)
335 create_test(test_hidden_apps)
336 create_test(test_blacklist)
337+create_test(test_action)
338
339 file(COPY data DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
340
341=== added file 'tests/mock_hidden_apps.h'
342--- tests/mock_hidden_apps.h 1970-01-01 00:00:00 +0000
343+++ tests/mock_hidden_apps.h 2016-08-01 13:24:59 +0000
344@@ -0,0 +1,32 @@
345+/*
346+ * Copyright 2016 Canonical Ltd.
347+ *
348+ * This program is free software: you can redistribute it and/or modify it under
349+ * the terms of the GNU General Public License, version 3, as published by the
350+ * Free Software Foundation.
351+ *
352+ * This program is distributed in the hope that it will be useful,
353+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
354+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
355+ * GNU General Public License for more details.
356+ *
357+ * You should have received a copy of the GNU General Public License
358+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
359+ */
360+#include "scope/apps/hidden_apps.h"
361+#include <gmock/gmock.h>
362+
363+
364+class MockHiddenApps : public HiddenApps
365+{
366+public:
367+ MockHiddenApps()
368+ : HiddenApps("")
369+ {
370+ }
371+
372+ MOCK_CONST_METHOD1(app_is_hidden, bool(QString const&));
373+ MOCK_CONST_METHOD0(empty, bool());
374+ MOCK_METHOD1(remove, void(QString const&));
375+ MOCK_METHOD1(add, void(QString const&));
376+};
377
378=== added file 'tests/test_action.cpp'
379--- tests/test_action.cpp 1970-01-01 00:00:00 +0000
380+++ tests/test_action.cpp 2016-08-01 13:24:59 +0000
381@@ -0,0 +1,82 @@
382+/*
383+ * Copyright 2016 Canonical Ltd.
384+ *
385+ * This program is free software: you can redistribute it and/or modify it under
386+ * the terms of the GNU General Public License, version 3, as published by the
387+ * Free Software Foundation.
388+ *
389+ * This program is distributed in the hope that it will be useful,
390+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
391+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
392+ * GNU General Public License for more details.
393+ *
394+ * You should have received a copy of the GNU General Public License
395+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
396+ */
397+#include "scope/apps/action.h"
398+#include "tests/mock_hidden_apps.h"
399+
400+#include <unity/scopes/ActionMetadata.h>
401+#include <unity/scopes/testing/Result.h>
402+#include <gmock/gmock.h>
403+
404+
405+namespace
406+{
407+TEST(ActionTest, OpenSendsDispatchURI)
408+{
409+ unity::scopes::testing::Result result;
410+ result.set_uri("app://this/is/my/app");
411+ unity::scopes::ActionMetadata metadata("en_US", "phone");
412+ auto hidden = std::make_shared<testing::NiceMock<MockHiddenApps> >();
413+ unity::scopes::FilterState filter_state;
414+
415+ std::string uri;
416+
417+ Action action(result, metadata, "open", [&](std::string const& app_uri) {
418+ uri = app_uri;
419+ }, hidden, filter_state);
420+
421+ auto response = action.activate();
422+ EXPECT_EQ(unity::scopes::ActivationResponse::Status::NotHandled, response.status());
423+ EXPECT_EQ(result.uri(), uri);
424+}
425+
426+
427+TEST(ActionTest, ShowRemovesAppFromHidden)
428+{
429+ unity::scopes::testing::Result result;
430+ unity::scopes::ActionMetadata metadata("en_US", "phone");
431+ unity::scopes::FilterState filter_state;
432+ auto hidden = std::make_shared<testing::NiceMock<MockHiddenApps> >();
433+
434+ QString app_id = "something.desktop";
435+ result["app_id"] = app_id.toStdString();
436+ EXPECT_CALL(*hidden, remove(app_id));
437+
438+ Action action(result, metadata, "show", Action::OpenUriAction{}, hidden, filter_state);
439+
440+ auto response = action.activate();
441+ EXPECT_EQ(filter_state.serialize(), response.query().filter_state().serialize());
442+ EXPECT_EQ(unity::scopes::ActivationResponse::Status::PerformQuery, response.status());
443+}
444+
445+
446+TEST(ActionTest, HideAddsAppToHidden)
447+{
448+ unity::scopes::testing::Result result;
449+ unity::scopes::ActionMetadata metadata("en_US", "phone");
450+ unity::scopes::FilterState filter_state;
451+ auto hidden = std::make_shared<testing::NiceMock<MockHiddenApps> >();
452+
453+ QString app_id = "something.desktop";
454+ result["app_id"] = app_id.toStdString();
455+ EXPECT_CALL(*hidden, add(app_id));
456+
457+ Action action(result, metadata, "hide", Action::OpenUriAction{}, hidden, filter_state);
458+
459+ auto response = action.activate();
460+ EXPECT_EQ(filter_state.serialize(), response.query().filter_state().serialize());
461+ EXPECT_EQ(unity::scopes::ActivationResponse::Status::PerformQuery, response.status());
462+}
463+}
464
465=== modified file 'tests/test_query.cpp'
466--- tests/test_query.cpp 2016-07-12 18:26:07 +0000
467+++ tests/test_query.cpp 2016-08-01 13:24:59 +0000
468@@ -17,6 +17,7 @@
469 #include "scope/apps/query.h"
470 #include "scope/apps/config.h"
471 #include "tests/fake_libertine.h"
472+#include "tests/mock_hidden_apps.h"
473 #include <unity/scopes/SearchMetadata.h>
474 #include <unity/scopes/CannedQuery.h>
475 #include <unity/scopes/SearchReplyProxyFwd.h>
476@@ -67,18 +68,6 @@
477 };
478
479
480-class MockHiddenApps : public HiddenApps
481-{
482-public:
483- MockHiddenApps()
484- : HiddenApps("")
485- {
486- }
487-
488- MOCK_CONST_METHOD1(app_is_hidden, bool(QString const&));
489- MOCK_CONST_METHOD0(empty, bool());
490-};
491-
492 class MockBlacklist : public Blacklist
493 {
494 public:

Subscribers

People subscribed via source and target branches