Merge lp:~dobey/unity-scope-click/lowercase-query into lp:unity-scope-click

[dobey]

Always convert query to lowercase.
Add operator override to read call params back out for testing.
Refactor tests to be able to test that query is lowercase.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:404
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/693/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-amd64-ci/97/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-armhf-ci/93/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/693/rebuild

review: Needs Fixing (continuous-integration)

[lp:~dobey/unity-scope-click/lowercase-query]

405. By dobey on 2016-02-01

[ Pawel Stolowski ]
* Support two column layouts in the previews. (LP: #1536372)
[ Rodney Dawes ]
* Avoid a race and crash by unlinking the callback inside the
  callback. (LP: #1486287)
* Only try to parse the JSON when we get a 200. (LP: #1531656)
* Remove reference to click in translated strings. (LP: #1536252)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:405
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/701/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-amd64-ci/105/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-armhf-ci/101/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/701/rebuild

review: Needs Fixing (continuous-integration)

[Charles Kerr (charlesk)]

Wily Jenkins failures not withstanding, LGTM.

review: Approve

=== modified file 'libclickscope/click/index.cpp'
--- libclickscope/click/index.cpp	2015-11-24 18:23:23 +0000
+++ libclickscope/click/index.cpp	2016-02-01 16:10:29 +0000
@@ -97,9 +97,11 @@
 
 std::string Index::build_index_query(const std::string& query, const std::string& department)
 {
+    std::string lquery{query};
+    std::transform(lquery.begin(), lquery.end(), lquery.begin(), ::tolower);
+
     std::stringstream result;
-
-    result << query;
+    result << lquery;
     if (!department.empty()) {
         result << ",department:" << department;
     }

=== modified file 'libclickscope/click/webclient.cpp'
--- libclickscope/click/webclient.cpp	2016-01-21 20:43:16 +0000
+++ libclickscope/click/webclient.cpp	2016-02-01 16:10:29 +0000
@@ -42,6 +42,11 @@
     query.addQueryItem(key.c_str(), value.c_str());
 }
 
+std::string click::web::CallParams::operator[](const std::string& key) const
+{
+    return query.queryItemValue(key.c_str()).toUtf8().data();
+}
+
 bool click::web::CallParams::operator==(const CallParams &other) const
 {
     return (this->query == other.query);

=== modified file 'libclickscope/click/webclient.h'
--- libclickscope/click/webclient.h	2016-01-21 20:43:16 +0000
+++ libclickscope/click/webclient.h	2016-02-01 16:10:29 +0000
@@ -64,6 +64,7 @@
     friend class Client;
 public:
     void add(const std::string& key, const std::string& value);
+    std::string operator[](const std::string& key) const;
     bool operator==(const CallParams &other) const;
 };
 

=== modified file 'libclickscope/tests/test_index.cpp'
--- libclickscope/tests/test_index.cpp	2015-11-24 18:23:23 +0000
+++ libclickscope/tests/test_index.cpp	2016-02-01 16:10:29 +0000
@@ -44,6 +44,10 @@
 
 namespace
 {
+    const std::string fake_arch{"fake_arch"};
+    const std::string fake_fwk_1{"fake_fwk_1"};
+    const std::string fake_fwk_2{"fake_fwk_2"};
+    std::vector<std::string> fake_frameworks{fake_fwk_1, fake_fwk_2};
 
 class MockableIndex : public click::Index {
 public:
@@ -53,8 +57,6 @@
         click::Index(client, configuration)
     {
     }
-    MOCK_METHOD0(build_headers, std::map<std::string, std::string>());
-    MOCK_METHOD2(build_index_query, std::string(const std::string&, const std::string&));
 };
 
 class MockConfiguration : public click::Configuration {
@@ -97,6 +99,12 @@
     LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
     auto response = responseForReply(reply.asSharedPtr());
 
+    EXPECT_CALL(*configPtr, get_architecture())
+        .Times(1)
+        .WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks())
+        .Times(1)
+        .WillOnce(Return(fake_frameworks));
     EXPECT_CALL(*clientPtr, callImpl(_, _, _, _, _, _))
             .Times(1)
             .WillOnce(Return(response));
@@ -104,11 +112,41 @@
     indexPtr->search("", [](click::Packages, click::Packages) {});
 }
 
+MATCHER_P(QueryContains, query, "")
+{
+    return arg["q"].find(query) != std::string::npos;
+}
+
+TEST_F(IndexTest, testSearchQueryIsLowercase)
+{
+    LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
+    auto response = responseForReply(reply.asSharedPtr());
+
+    EXPECT_CALL(*configPtr, get_architecture())
+        .Times(1)
+        .WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks())
+        .Times(1)
+        .WillOnce(Return(fake_frameworks));
+    EXPECT_CALL(*clientPtr, callImpl(_, _, _, _, _, QueryContains("foobar")))
+            .Times(1)
+            .WillOnce(Return(response));
+
+    indexPtr->search("FooBar", [](click::Packages, click::Packages) {});
+}
+
+
 TEST_F(IndexTest, testSearchSignsCall)
 {
     LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
     auto response = responseForReply(reply.asSharedPtr());
 
+    EXPECT_CALL(*configPtr, get_architecture())
+        .Times(1)
+        .WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks())
+        .Times(1)
+        .WillOnce(Return(fake_frameworks));
     EXPECT_CALL(*clientPtr, callImpl(_, _, true, _, _, _))
             .Times(1)
             .WillOnce(Return(response));
@@ -121,6 +159,12 @@
     LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
     auto response = responseForReply(reply.asSharedPtr());
 
+    EXPECT_CALL(*configPtr, get_architecture())
+        .Times(1)
+        .WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks())
+        .Times(1)
+        .WillOnce(Return(fake_frameworks));
     EXPECT_CALL(*clientPtr, callImpl(_, _, true, _, _, _))
             .Times(1)
             .WillOnce(Return(response));
@@ -133,6 +177,12 @@
     LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
     auto response = responseForReply(reply.asSharedPtr());
 
+    EXPECT_CALL(*configPtr, get_architecture())
+        .Times(1)
+        .WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks())
+        .Times(1)
+        .WillOnce(Return(fake_frameworks));
     EXPECT_CALL(*clientPtr, callImpl(_, _, true, _, _, _))
             .Times(1)
             .WillOnce(Return(response));
@@ -140,31 +190,17 @@
     indexPtr->departments("departments", [](const click::DepartmentList&, const click::HighlightList&, click::Index::Error, int) {});
 }
 
-TEST_F(IndexTest, testSearchSendsBuiltQueryAsParam)
-{
-    const std::string FAKE_BUILT_QUERY = "FAKE_QUERY,frameworks:fake-14.04,architecture:fake-arch";
-
-    LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
-    auto response = responseForReply(reply.asSharedPtr());
-
-    click::web::CallParams params;
-    params.add(click::QUERY_ARGNAME, FAKE_BUILT_QUERY);
-    EXPECT_CALL(*clientPtr, callImpl(_, _, _, _, _, params))
-            .Times(1)
-            .WillOnce(Return(response));
-
-    EXPECT_CALL(*indexPtr, build_index_query(FAKE_QUERY, ""))
-            .Times(1)
-            .WillOnce(Return(FAKE_BUILT_QUERY));
-
-    indexPtr->search(FAKE_QUERY, [](click::Packages, click::Packages) {});
-}
-
 TEST_F(IndexTest, testSearchSendsRightPath)
 {
     LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
     auto response = responseForReply(reply.asSharedPtr());
 
+    EXPECT_CALL(*configPtr, get_architecture())
+        .Times(1)
+        .WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks())
+        .Times(1)
+        .WillOnce(Return(fake_frameworks));
     EXPECT_CALL(*clientPtr, callImpl(EndsWith(click::SEARCH_PATH),
                                      _, _, _, _, _))
             .Times(1)
@@ -182,6 +218,12 @@
     EXPECT_CALL(reply.instance, readAll())
             .Times(1)
             .WillOnce(Return(fake_json));
+    EXPECT_CALL(*configPtr, get_architecture())
+        .Times(1)
+        .WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks())
+        .Times(1)
+        .WillOnce(Return(fake_frameworks));
     EXPECT_CALL(*clientPtr, callImpl(_, _, _, _, _, _))
             .Times(1)
             .WillOnce(Return(response));
@@ -203,6 +245,12 @@
     EXPECT_CALL(reply.instance, readAll())
             .Times(1)
             .WillOnce(Return(fake_json));
+    EXPECT_CALL(*configPtr, get_architecture())
+        .Times(1)
+        .WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks())
+        .Times(1)
+        .WillOnce(Return(fake_frameworks));
     EXPECT_CALL(*clientPtr, callImpl(_, _, _, _, _, _))
             .Times(1)
             .WillOnce(Return(response));
@@ -225,6 +273,12 @@
     EXPECT_CALL(reply.instance, readAll())
             .Times(1)
             .WillOnce(Return(fake_json));
+    EXPECT_CALL(*configPtr, get_architecture())
+        .Times(1)
+        .WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks())
+        .Times(1)
+        .WillOnce(Return(fake_frameworks));
     EXPECT_CALL(*clientPtr, callImpl(_, _, _, _, _, _))
             .Times(1)
             .WillOnce(Return(response));
@@ -251,6 +305,12 @@
     LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
     auto response = responseForReply(reply.asSharedPtr());
 
+    EXPECT_CALL(*configPtr, get_architecture())
+        .Times(1)
+        .WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks())
+        .Times(1)
+        .WillOnce(Return(fake_frameworks));
     EXPECT_CALL(*clientPtr, callImpl(_, _, _, _, _, _))
             .Times(1)
             .WillOnce(Return(response));
@@ -271,6 +331,12 @@
     LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
     auto response = responseForReply(reply.asSharedPtr());
 
+    EXPECT_CALL(*configPtr, get_architecture())
+        .Times(1)
+        .WillOnce(Return(fake_arch));
+    EXPECT_CALL(*configPtr, get_available_frameworks())
+        .Times(1)
+        .WillOnce(Return(fake_frameworks));
     EXPECT_CALL(*clientPtr, callImpl(_, _, _, _, _, _))
             .Times(1)
             .WillOnce(Return(response));
@@ -560,11 +626,6 @@
 
 class QueryStringTest : public ::testing::Test {
 protected:
-    const std::string fake_arch{"fake_arch"};
-    const std::string fake_fwk_1{"fake_fwk_1"};
-    const std::string fake_fwk_2{"fake_fwk_2"};
-    std::vector<std::string> fake_frameworks{fake_fwk_1, fake_fwk_2};
-
     QSharedPointer<MockClient> clientPtr;
     QSharedPointer<MockNetworkAccessManager> namPtr;
     QSharedPointer<MockConfiguration> configPtr;