Merge lp:~alecu/unity-scope-click/send-click-frameworks into lp:unity-scope-click

[Alejandro J. Cura]

Include supported frameworks in click search

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:170
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/355/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-amd64-ci/256
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/253
        deb: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/253/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[lp:~alecu/unity-scope-click/send-click-frameworks]

171. By Alejandro J. Cura on 2014-02-25

improved comment

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:171
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/357/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-amd64-ci/258
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/255
        deb: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/255/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Mike McCracken (mikemc)]

+1. I only have nitpicky style comments:

This is something I did recently (in my case it broke something, since my file wasn't including Qt), so it stuck out to me:
In FrameworkLocator::get_available_frameworks, this func looks like
It's trying to be all c++11, but it's using the qt foreach macro:
https://qt-project.org/doc/qt-5.0/qtcore/containers.html#the-foreach-keyword

the c++11 equivalent is the ranged for loop:
http://www.cprogramming.com/c++11/c++11-ranged-for-loop.html

So you probably want 'for (auto f : list_folder(FRAMEWORKS_FOLDER, FRAMEWORKS_PATTERN)) {...}'
(Although I haven't tried it, so it occurs to me that you might not be able to stick the call to list_folder inline like I did there...)

In queryUri (line 95 of the diff), you're using
QString::fromUtf8(ststring.c_str()) where fromStdString() is
shorter. You use fromStdString elsewhere, might as well change it here
now to be consistent.

review: Approve

[Alejandro J. Cura (alecu)]

> +1. I only have nitpicky style comments:
>
>
> This is something I did recently (in my case it broke something, since my file
> wasn't including Qt), so it stuck out to me:
> In FrameworkLocator::get_available_frameworks, this func looks like
> It's trying to be all c++11, but it's using the qt foreach macro:
> https://qt-project.org/doc/qt-5.0/qtcore/containers.html#the-foreach-keyword
>
> the c++11 equivalent is the ranged for loop:
> http://www.cprogramming.com/c++11/c++11-ranged-for-loop.html
>
> So you probably want 'for (auto f : list_folder(FRAMEWORKS_FOLDER,
> FRAMEWORKS_PATTERN)) {...}'

Good catch! TIL foreach vs ranged for.

> In queryUri (line 95 of the diff), you're using
> QString::fromUtf8(ststring.c_str()) where fromStdString() is
> shorter. You use fromStdString elsewhere, might as well change it here
> now to be consistent.

I'm getting rid of all the QNetwork mess in query.cpp in my next branch (replacing it with calls to Index.search), so I aimed for the smaller change there. For that reason, I rather keep it as is.

[lp:~alecu/unity-scope-click/send-click-frameworks]

172. By Alejandro J. Cura on 2014-02-26

Replacing Qt's foreach with c++11's ranged for

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:172
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/358/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-amd64-ci/259
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/256
        deb: http://jenkins.qa.ubuntu.com/job/unity-scope-click-trusty-armhf-ci/256/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[dobey (dobey)]

review: Approve

[lp:~alecu/unity-scope-click/send-click-frameworks]

173. By Alejandro J. Cura on 2014-03-05

merged with trunk

=== modified file 'scope/click/interface.cpp'
--- scope/click/interface.cpp	2014-02-24 14:10:10 +0000
+++ scope/click/interface.cpp	2014-03-05 13:11:23 +0000
@@ -237,6 +237,29 @@
     }
 }
 
+std::vector<std::string> FrameworkLocator::list_folder(const std::string& folder, const std::string& pattern)
+{
+    std::vector<std::string> result;
+
+    QDir dir(QString::fromStdString(folder), QString::fromStdString(pattern),
+             QDir::Unsorted, QDir::Readable | QDir::Files);
+    QStringList entries = dir.entryList();
+    for (int i = 0; i < entries.size(); ++i) {
+        QString filename = entries.at(i);
+        result.push_back(filename.toStdString());
+    }
+
+    return result;
+}
+
+std::vector<std::string> FrameworkLocator::get_available_frameworks()
+{
+    std::vector<std::string> result;
+    for (auto f: list_folder(FRAMEWORKS_FOLDER, FRAMEWORKS_PATTERN)) {
+        result.push_back(f.substr(0, f.size()-FRAMEWORKS_EXTENSION_LENGTH));
+    }
+    return result;
+}
 
 ManifestList manifest_list_from_json(const std::string& json)
 {
@@ -386,7 +409,4 @@
     });
 }
 
-
-
-
 } // namespace click

=== modified file 'scope/click/interface.h'
--- scope/click/interface.h	2014-02-24 14:04:59 +0000
+++ scope/click/interface.h	2014-03-05 13:11:23 +0000
@@ -89,6 +89,18 @@
     QSharedPointer<KeyFileLocator> keyFileLocator;
 };
 
+class FrameworkLocator
+{
+public:
+    constexpr static const char* FRAMEWORKS_FOLDER {"/usr/share/click/frameworks/"};
+    constexpr static const char* FRAMEWORKS_PATTERN {"*.framework"};
+    constexpr static const int FRAMEWORKS_EXTENSION_LENGTH = 10; // strlen(".framework")
+
+    virtual std::vector<std::string> get_available_frameworks();
+protected:
+    virtual std::vector<std::string> list_folder(const std::string &folder, const std::string &pattern);
+};
+
 } // namespace click
 
 #endif // CLICK_INTERFACE_H

=== modified file 'scope/click/query.cpp'
--- scope/click/query.cpp	2014-02-26 20:15:57 +0000
+++ scope/click/query.cpp	2014-03-05 13:11:23 +0000
@@ -56,6 +56,7 @@
 #include<QUrl>
 #include<vector>
 #include<set>
+#include<sstream>
 
 namespace
 {
@@ -311,6 +312,16 @@
 
     return iface;
 }
+
+QString frameworks_arg()
+{
+    std::stringstream frameworks;
+    for (auto f: click::FrameworkLocator().get_available_frameworks()) {
+        frameworks << ",framework:" << f;
+    }
+    return QString::fromStdString(frameworks.str());
+}
+
 }
 void click::Query::run(scopes::SearchReplyProxy const& searchReply)
 {
@@ -335,8 +346,10 @@
     {
         static const QString queryPattern(
                     "https://search.apps.ubuntu.com/api/v1/search?q=%1"
-                    ",framework:ubuntu-sdk-13.10,architecture:%2");
-        QString queryUri = queryPattern.arg(QString::fromUtf8(impl->query.c_str())).arg(architecture());
+                    "%2,architecture:%3");
+
+        QString queryUri = queryPattern.arg(QString::fromUtf8(impl->query.c_str()))
+                .arg(frameworks_arg()).arg(architecture());
 
         auto nam = getNetworkAccessManager(env);
         auto networkReply = nam->get(QNetworkRequest(QUrl(queryUri)));

=== modified file 'scope/tests/test_interface.cpp'
--- scope/tests/test_interface.cpp	2014-02-18 07:05:58 +0000
+++ scope/tests/test_interface.cpp	2014-03-05 13:11:23 +0000
@@ -182,3 +182,55 @@
 
     EXPECT_EQ(result.end(), std::find(result.begin(), result.end(), mustNotBePartOfResult));
 }
+
+class FakeFrameworkLocator : public click::FrameworkLocator
+{
+public:
+    MOCK_METHOD2(list_folder, std::vector<std::string>(
+                     const std::string& folder, const std::string& pattern));
+};
+
+TEST(FrameworkLocator, getAvailableFrameworksUsesRightFolder)
+{
+    using namespace ::testing;
+    FakeFrameworkLocator locator;
+    EXPECT_CALL(locator, list_folder(FrameworkLocator::FRAMEWORKS_FOLDER, _))
+            .Times(1).WillOnce(Return(std::vector<std::string>()));
+    locator.get_available_frameworks();
+}
+
+TEST(FrameworkLocator, getAvailableFrameworksUsesRightPattern)
+{
+    using namespace ::testing;
+    FakeFrameworkLocator locator;
+    EXPECT_CALL(locator, list_folder(_, FrameworkLocator::FRAMEWORKS_PATTERN))
+            .Times(1).WillOnce(Return(std::vector<std::string>()));
+    locator.get_available_frameworks();
+}
+
+TEST(FrameworkLocator, getAvailableFrameworksTwoResults)
+{
+    using namespace ::testing;
+
+    FakeFrameworkLocator locator;
+    std::vector<std::string> response = {"abc.framework", "def.framework"};
+    EXPECT_CALL(locator, list_folder(_, _))
+            .Times(1)
+            .WillOnce(Return(response));
+    auto frameworks = locator.get_available_frameworks();
+    std::vector<std::string> expected = {"abc", "def"};
+    EXPECT_EQ(expected, frameworks);
+}
+
+TEST(FrameworkLocator, getAvailableFrameworksNoResults)
+{
+    using namespace ::testing;
+
+    FakeFrameworkLocator locator;
+    std::vector<std::string> response = {};
+    EXPECT_CALL(locator, list_folder(_, _))
+            .Times(1)
+            .WillOnce(Return(response));
+    auto frameworks = locator.get_available_frameworks();
+    EXPECT_EQ(0, frameworks.size());
+}