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

[Christopher Townsend]

* 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)

[Libertine CI Bot (libertine-ci-bot)]

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)

[Larry Price (larryprice)]

lgtm

review: Approve

[lp:~townsend/libertine-scope/release-1.3.2]

57. By Christopher Townsend on 2016-08-01

Change version to 1.3.2.

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2016-07-21 16:07:09 +0000
+++ CMakeLists.txt	2016-08-01 13:24:59 +0000
@@ -2,7 +2,7 @@
 cmake_policy(SET CMP0048 NEW)
 
 project(libertine-scope
-        VERSION 1.3.1
+        VERSION 1.3.2
         LANGUAGES CXX)
 
 # We require at least g++ 4.9, to avoid ABI breakage with earlier versions.
 
=== modified file 'debian/changelog'
--- debian/changelog	2016-07-22 13:48:17 +0000
+++ debian/changelog	2016-08-01 13:24:59 +0000
@@ -1,3 +1,14 @@
+libertine-scope (1.3.2-0ubuntu1) UNRELEASED; urgency=medium
+
+  [ Larry Price ]
+  * 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)
+
+ -- Chris Townsend <[e-mail]>  Fri, 29 Jul 2016 12:35:35 -0400
+
 libertine-scope (1.3.1+16.10.20160722-0ubuntu1) yakkety; urgency=medium
 
   [ Chris Townsend ]
 
=== modified file 'scope/apps/action.cpp'
--- scope/apps/action.cpp	2016-07-19 20:20:12 +0000
+++ scope/apps/action.cpp	2016-08-01 13:24:59 +0000
@@ -29,14 +29,30 @@
 namespace usc = unity::scopes;
 
 
+namespace
+{
+static usc::ActivationResponse
+sendToResults(usc::FilterState const& filter_state)
+{
+  usc::CannedQuery query(FULLY_QUALIFIED_APPS_SCOPE);
+  query.set_filter_state(filter_state);
+  return usc::ActivationResponse(query);
+}
+}
+
+
 Action::
-Action(usc::Result const& result,
-       usc::ActionMetadata const& metadata,
-       std::string const& action_id, 
-       std::shared_ptr<HiddenApps> hidden)
-    : usc::ActivationQueryBase(result, metadata),
-      action_id_(action_id),
-      hidden_(hidden)
+Action(unity::scopes::Result const&         result,
+       unity::scopes::ActionMetadata const& metadata,
+       std::string const&                   action_id,
+       Action::OpenUriAction                open_action,
+       std::shared_ptr<HiddenApps>          hidden,
+       unity::scopes::FilterState const&    filter_state)
+  : usc::ActivationQueryBase(result, metadata)
+  , action_id_(action_id)
+  , open_action_(open_action)
+  , hidden_(hidden)
+  , filter_state_(filter_state)
 {
 }
 
@@ -46,22 +62,17 @@
 {
   if (action_id_ == "open")
   {
-    url_dispatch_send(result().uri().c_str() , NULL, NULL);
-    return usc::ActivationResponse(usc::ActivationResponse::Status::NotHandled);
+    open_action_(result().uri());
   }
   else if (action_id_ == "hide")
   {
     hidden_->add(QString::fromStdString(result()["app_id"].get_string()));
-
-    usc::CannedQuery cq(SCOPE_PKG);
-    return usc::ActivationResponse(cq);
+    return sendToResults(filter_state_);
   }
   else if (action_id_ == "show")
   {
     hidden_->remove(QString::fromStdString(result()["app_id"].get_string()));
-
-    usc::CannedQuery cq(SCOPE_PKG);
-    return usc::ActivationResponse(cq);
+    return sendToResults(filter_state_);
   }
   return usc::ActivationResponse(usc::ActivationResponse::Status::NotHandled);
 }
 
=== modified file 'scope/apps/action.h'
--- scope/apps/action.h	2016-07-12 18:26:07 +0000
+++ scope/apps/action.h	2016-08-01 13:24:59 +0000
@@ -15,31 +15,37 @@
  * * Authored by:
  *	Kyle Nitzsche <[e-mail]>
  */
-
 #ifndef SCOPE_ACTION_H_
 #define SCOPE_ACTION_H_
 
-#include "scope/apps/scope.h"
+
 #include <unity/scopes/ActionMetadata.h>
 #include <unity/scopes/ActivationQueryBase.h>
 #include <unity/scopes/ActivationResponse.h>
 #include <unity/scopes/Result.h>
 
+
 class HiddenApps;
 
 class Action : public unity::scopes::ActivationQueryBase {
-  public:
-    Action(unity::scopes::Result const&         result,
-           unity::scopes::ActionMetadata const& metadata,
-           std::string const&                   action_id, 
-           std::shared_ptr<HiddenApps>          hidden);
-
-    virtual ~Action() = default;
-    virtual unity::scopes::ActivationResponse activate() override;
-
-  private:
-    std::string action_id_;
-    std::string cache_dir_;
-    std::shared_ptr<HiddenApps> hidden_;
+public:
+  typedef std::function<void(std::string const&)> OpenUriAction;
+
+  explicit Action(unity::scopes::Result const&         result,
+                  unity::scopes::ActionMetadata const& metadata,
+                  std::string const&                   action_id,
+                  OpenUriAction                        open_action,
+                  std::shared_ptr<HiddenApps>          hidden,
+                  unity::scopes::FilterState const&    filterState);
+
+  virtual ~Action() = default;
+  virtual unity::scopes::ActivationResponse activate() override;
+
+private:
+  std::string action_id_;
+  std::string cache_dir_;
+  OpenUriAction open_action_;
+  std::shared_ptr<HiddenApps> hidden_;
+  unity::scopes::FilterState filter_state_;
 };
 #endif
 
=== modified file 'scope/apps/config.h.in'
--- scope/apps/config.h.in	2016-07-19 20:20:12 +0000
+++ scope/apps/config.h.in	2016-08-01 13:24:59 +0000
@@ -16,10 +16,8 @@
 #ifndef LIBERTINE_SCOPE_CONFIG_H_
 #define LIBERTINE_SCOPE_CONFIG_H_
 
-const std::string SCOPE_PKG = "@PACKAGE_NAME@";
+const std::string FULLY_QUALIFIED_APPS_SCOPE = "@FULLY_QUALIFIED_NAME@";
 const std::string ROOT_DEPT_ID = "root_dept";
 const std::string HIDDEN_DEPT_ID = "hidden_dept";
 
 #endif // LIBERTINE_SCOPE_CONFIG_H_
-
-
 
=== modified file 'scope/apps/libertine.cpp'
--- scope/apps/libertine.cpp	2016-07-12 18:26:07 +0000
+++ scope/apps/libertine.cpp	2016-08-01 13:24:59 +0000
@@ -91,6 +91,5 @@
 Libertine::UPtr Libertine::
 from_libertine_cli()
 {
-  return Libertine::UPtr(new LibertineCli());
+  return std::make_shared<LibertineCli>();
 }
-
 
=== modified file 'scope/apps/libertine.h'
--- scope/apps/libertine.h	2016-01-13 00:38:37 +0000
+++ scope/apps/libertine.h	2016-08-01 13:24:59 +0000
@@ -29,7 +29,7 @@
 class Libertine
 {
 public:
-  using UPtr = std::unique_ptr<Libertine>;
+  using UPtr = std::shared_ptr<Libertine>;
   using ContainerList = std::vector<std::unique_ptr<Container>>;
 
   /**
 
=== modified file 'scope/apps/query.cpp'
--- scope/apps/query.cpp	2016-07-19 13:57:43 +0000
+++ scope/apps/query.cpp	2016-08-01 13:24:59 +0000
@@ -121,7 +121,7 @@
       std::shared_ptr<HiddenApps> hidden,
       std::shared_ptr<Blacklist> blacklist)
   : usc::SearchQueryBase(query, metadata)
-  , libertine_(libertine_factory())
+  , libertine_factory_(libertine_factory)
   , hidden_(hidden)
   , blacklist_(blacklist)
 {
@@ -135,14 +135,14 @@
 
 
 QStringList Query::
-make_filters(usc::SearchReplyProxy const& reply) const
+make_filters(usc::SearchReplyProxy const& reply, Libertine::UPtr libertine) const
 {
   auto filter_state = query().filter_state();
   QStringList excludes_by_filter;
   std::list<usc::FilterBase::SCPtr> app_filters;
 
   //make exclude scope filter for apps
-  for (auto const& container: libertine_->get_container_list())
+  for (auto const& container: libertine->get_container_list())
   {
     usc::OptionSelectorFilter::SPtr filter{usc::OptionSelectorFilter::create(container->id(),
                                                                              EXCLUDED_APPS_FILTER_TITLE + container->name(),
@@ -208,18 +208,20 @@
     register_departments(reply);
   }
 
+  auto libertine = libertine_factory_();
+
   // only provide filters in root department
   QStringList excludes_by_filter;
   if (query().department_id().empty())
   {
-    excludes_by_filter = make_filters(reply);
+    excludes_by_filter = make_filters(reply, libertine);
   }
 
   QRegExp search_query(QString::fromStdString(query().query_string()), Qt::CaseInsensitive);
   bool has_no_apps = true,
        all_filtered = true;
 
-  for (auto const& container: libertine_->get_container_list())
+  for (auto const& container: libertine->get_container_list())
   {
     auto category = reply->register_category(container->id(),
                                              container->name(),
 
=== modified file 'scope/apps/query.h'
--- scope/apps/query.h	2016-07-12 18:26:07 +0000
+++ scope/apps/query.h	2016-08-01 13:24:59 +0000
@@ -50,11 +50,11 @@
 
 private:
   QStringList get_hidden_department() const;
-  QStringList make_filters(unity::scopes::SearchReplyProxy const& reply) const;
+  QStringList make_filters(unity::scopes::SearchReplyProxy const& reply, Libertine::UPtr libertine) const;
   void show_hint(unity::scopes::SearchReplyProxy const& reply, std::string const& reason) const;
   void parse_blacklist(const std::string& data_dir);
 
-  Libertine::UPtr libertine_;
+  Libertine::Factory libertine_factory_;
   std::shared_ptr<HiddenApps> hidden_;
   std::shared_ptr<Blacklist> blacklist_;
 };
 
=== modified file 'scope/apps/scope.cpp'
--- scope/apps/scope.cpp	2016-07-12 18:26:07 +0000
+++ scope/apps/scope.cpp	2016-08-01 13:24:59 +0000
@@ -29,6 +29,16 @@
 namespace usc = unity::scopes;
 
 
+namespace
+{
+static void
+open_application(std::string const& app_uri)
+{
+  url_dispatch_send(app_uri.c_str(), NULL, NULL);
+}
+}
+
+
 Scope::
 Scope(Libertine::Factory const& libertine_factory)
 : libertine_factory_(libertine_factory)
@@ -56,6 +66,7 @@
 search(usc::CannedQuery const&    query,
        usc::SearchMetadata const& metadata)
 {
+  filter_state_ = query.filter_state();
   return usc::SearchQueryBase::UPtr(new Query(query,
                                               metadata,
                                               libertine_factory_,
@@ -81,7 +92,9 @@
   return usc::ActivationQueryBase::UPtr(new Action(result,
                                                    metadata,
                                                    action_id,
-                                                   std::make_shared<HiddenApps>(cache_directory())));
+                                                   open_application,
+                                                   std::make_shared<HiddenApps>(cache_directory()),
+                                                   filter_state_));
 }
 
 
 
=== modified file 'scope/apps/scope.h'
--- scope/apps/scope.h	2016-07-12 18:26:07 +0000
+++ scope/apps/scope.h	2016-08-01 13:24:59 +0000
@@ -78,6 +78,7 @@
 
 private:
   Libertine::Factory libertine_factory_;
+  unity::scopes::FilterState filter_state_;
 };
 
 #endif // LIBERTINE_SCOPE_SCOPE_H_
 
=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2016-07-12 18:26:07 +0000
+++ tests/CMakeLists.txt	2016-08-01 13:24:59 +0000
@@ -7,6 +7,7 @@
   add_executable(${test_name}_exe
     fake_container.cpp
     fake_libertine.cpp
+    mock_hidden_apps.h
     ${test_name}.cpp
   )
 
@@ -25,5 +26,6 @@
 create_test(test_query)
 create_test(test_hidden_apps)
 create_test(test_blacklist)
+create_test(test_action)
 
 file(COPY data DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
 
=== added file 'tests/mock_hidden_apps.h'
--- tests/mock_hidden_apps.h	1970-01-01 00:00:00 +0000
+++ tests/mock_hidden_apps.h	2016-08-01 13:24:59 +0000
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2016 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, version 3, as published by the
+ * Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include "scope/apps/hidden_apps.h"
+#include <gmock/gmock.h>
+
+
+class MockHiddenApps : public HiddenApps
+{
+public:
+  MockHiddenApps()
+    : HiddenApps("")
+  {
+  }
+
+  MOCK_CONST_METHOD1(app_is_hidden, bool(QString const&));
+  MOCK_CONST_METHOD0(empty, bool());
+  MOCK_METHOD1(remove, void(QString const&));
+  MOCK_METHOD1(add, void(QString const&));
+};
 
=== added file 'tests/test_action.cpp'
--- tests/test_action.cpp	1970-01-01 00:00:00 +0000
+++ tests/test_action.cpp	2016-08-01 13:24:59 +0000
@@ -0,0 +1,82 @@
+/*
+ * Copyright 2016 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, version 3, as published by the
+ * Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include "scope/apps/action.h"
+#include "tests/mock_hidden_apps.h"
+
+#include <unity/scopes/ActionMetadata.h>
+#include <unity/scopes/testing/Result.h>
+#include <gmock/gmock.h>
+
+
+namespace
+{
+TEST(ActionTest, OpenSendsDispatchURI)
+{
+  unity::scopes::testing::Result result;
+  result.set_uri("app://this/is/my/app");
+  unity::scopes::ActionMetadata metadata("en_US", "phone");
+  auto hidden = std::make_shared<testing::NiceMock<MockHiddenApps> >();
+  unity::scopes::FilterState filter_state;
+
+  std::string uri;
+
+  Action action(result, metadata, "open", [&](std::string const& app_uri) {
+    uri = app_uri;
+  }, hidden, filter_state);
+
+  auto response = action.activate();
+  EXPECT_EQ(unity::scopes::ActivationResponse::Status::NotHandled, response.status());
+  EXPECT_EQ(result.uri(), uri);
+}
+
+
+TEST(ActionTest, ShowRemovesAppFromHidden)
+{
+  unity::scopes::testing::Result result;
+  unity::scopes::ActionMetadata metadata("en_US", "phone");
+  unity::scopes::FilterState filter_state;
+  auto hidden = std::make_shared<testing::NiceMock<MockHiddenApps> >();
+
+  QString app_id = "something.desktop";
+  result["app_id"] = app_id.toStdString();
+  EXPECT_CALL(*hidden, remove(app_id));
+
+  Action action(result, metadata, "show", Action::OpenUriAction{}, hidden, filter_state);
+
+  auto response = action.activate();
+  EXPECT_EQ(filter_state.serialize(), response.query().filter_state().serialize());
+  EXPECT_EQ(unity::scopes::ActivationResponse::Status::PerformQuery, response.status());
+}
+
+
+TEST(ActionTest, HideAddsAppToHidden)
+{
+  unity::scopes::testing::Result result;
+  unity::scopes::ActionMetadata metadata("en_US", "phone");
+  unity::scopes::FilterState filter_state;
+  auto hidden = std::make_shared<testing::NiceMock<MockHiddenApps> >();
+
+  QString app_id = "something.desktop";
+  result["app_id"] = app_id.toStdString();
+  EXPECT_CALL(*hidden, add(app_id));
+
+  Action action(result, metadata, "hide", Action::OpenUriAction{}, hidden, filter_state);
+
+  auto response = action.activate();
+  EXPECT_EQ(filter_state.serialize(), response.query().filter_state().serialize());
+  EXPECT_EQ(unity::scopes::ActivationResponse::Status::PerformQuery, response.status());
+}
+}
 
=== modified file 'tests/test_query.cpp'
--- tests/test_query.cpp	2016-07-12 18:26:07 +0000
+++ tests/test_query.cpp	2016-08-01 13:24:59 +0000
@@ -17,6 +17,7 @@
 #include "scope/apps/query.h"
 #include "scope/apps/config.h"
 #include "tests/fake_libertine.h"
+#include "tests/mock_hidden_apps.h"
 #include <unity/scopes/SearchMetadata.h>
 #include <unity/scopes/CannedQuery.h>
 #include <unity/scopes/SearchReplyProxyFwd.h>
@@ -67,18 +68,6 @@
 };
 
 
-class MockHiddenApps : public HiddenApps
-{
-public:
-  MockHiddenApps()
-    : HiddenApps("")
-  {
-  }
-
-  MOCK_CONST_METHOD1(app_is_hidden, bool(QString const&));
-  MOCK_CONST_METHOD0(empty, bool());
-};
-
 class MockBlacklist : public Blacklist
 {
 public: