Merge lp:~alecu/unity-scope-click/fix-empty-variant-rtm into lp:unity-scope-click/rtm-14.09

[Alejandro J. Cura]

Check Variant for null value

[Alejandro J. Cura (alecu)]

This branch is a backport of this bugfix from vivid to rtm.

[dobey (dobey)]

review: Approve

=== modified file 'libclickscope/click/index.h'
--- libclickscope/click/index.h	2014-10-01 03:37:34 +0000
+++ libclickscope/click/index.h	2015-01-15 20:11:09 +0000
@@ -74,6 +74,7 @@
 
 public:
     enum class Error {NoError, CredentialsError, NetworkError};
+    Index() {}
     Index(const QSharedPointer<click::web::Client>& client,
           const QSharedPointer<Configuration> configuration=QSharedPointer<Configuration>(new Configuration()));
     virtual std::pair<Packages, Packages> package_lists_from_json(const std::string& json);

=== modified file 'libclickscope/click/preview.cpp'
--- libclickscope/click/preview.cpp	2014-10-22 19:32:37 +0000
+++ libclickscope/click/preview.cpp	2015-01-15 20:11:09 +0000
@@ -254,6 +254,22 @@
     return b.str();
 }
 
+void PreviewStrategy::run_under_qt(const std::function<void ()> &task)
+{
+    qt::core::world::enter_with_task([task]() {
+        task();
+    });
+}
+
+std::string get_string_maybe_null(scopes::Variant variant)
+{
+    if (variant.is_null()) {
+        return "";
+    } else {
+        return variant.get_string();
+    }
+}
+
 // TODO: error handling - once get_details provides errors, we can
 // return them from populateDetails and check them in the calling code
 // to decide whether to show error widgets. see bug LP: #1289541
@@ -262,15 +278,15 @@
                                                     click::Reviews::Error)> reviews_callback)
 {
 
-    std::string app_name = result["name"].get_string();
+    std::string app_name = get_string_maybe_null(result["name"]);
 
     if (app_name.empty()) {
         click::PackageDetails details;
         qDebug() << "in populateDetails(), app_name is empty";
         details.package.title = result.title();
         details.package.icon_url = result.art();
-        details.description = result["description"].get_string();
-        details.main_screenshot_url = result["main_screenshot"].get_string();
+        details.description = get_string_maybe_null(result["description"]);
+        details.main_screenshot_url = get_string_maybe_null(result["main_screenshot"]);
         details_callback(details);
         reviews_callback(click::ReviewList(), click::Reviews::Error::NoError);
     } else {
@@ -278,7 +294,7 @@
         // I think this should not be required when we switch the click::Index over
         // to using the Qt bridge. With that, the qt dependency becomes an implementation detail
         // and code using it does not need to worry about threading/event loop topics.
-        qt::core::world::enter_with_task([this, details_callback, reviews_callback, app_name]()
+        run_under_qt([this, details_callback, reviews_callback, app_name]()
             {
                 index_operation = index->get_details(app_name, [this, app_name, details_callback, reviews_callback](PackageDetails details, click::Index::Error error){
                     if(error == click::Index::Error::NoError) {
@@ -289,8 +305,8 @@
                         click::PackageDetails details;
                         details.package.title = result.title();
                         details.package.icon_url = result.art();
-                        details.description = result["description"].get_string();
-                        details.main_screenshot_url = result["main_screenshot"].get_string();
+                        details.description = get_string_maybe_null(result["description"]);
+                        details.main_screenshot_url = get_string_maybe_null(result["main_screenshot"]);
                         details_callback(details);
                     }
                     reviews_operation = reviews->fetch_reviews(app_name,

=== modified file 'libclickscope/click/preview.h'
--- libclickscope/click/preview.h	2014-10-16 21:25:16 +0000
+++ libclickscope/click/preview.h	2015-01-15 20:11:09 +0000
@@ -149,6 +149,7 @@
     virtual std::string build_other_metadata(const PackageDetails& details);
     virtual std::string build_updates_table(const PackageDetails& details);
     virtual std::string build_whats_new(const PackageDetails& details);
+    virtual void run_under_qt(const std::function<void ()> &task);
 
     scopes::Result result;
     QSharedPointer<click::web::Client> client;

=== modified file 'libclickscope/click/reviews.h'
--- libclickscope/click/reviews.h	2014-05-26 14:02:45 +0000
+++ libclickscope/click/reviews.h	2015-01-15 20:11:09 +0000
@@ -72,10 +72,11 @@
     QSharedPointer<web::Client> client;
 public:
     enum class Error {NoError, CredentialsError, NetworkError};
+    Reviews() {}
     Reviews(const QSharedPointer<click::web::Client>& client);
     virtual ~Reviews();
 
-    click::web::Cancellable fetch_reviews (const std::string& package_name,
+    virtual click::web::Cancellable fetch_reviews (const std::string& package_name,
                                            std::function<void(ReviewList, Error)> callback);
     click::web::Cancellable submit_review (const Review& review,
                                            std::function<void(Error)> callback);

=== modified file 'libclickscope/tests/test_preview.cpp'
--- libclickscope/tests/test_preview.cpp	2014-10-17 19:55:56 +0000
+++ libclickscope/tests/test_preview.cpp	2015-01-15 20:11:09 +0000
@@ -33,6 +33,8 @@
 #include <gtest/gtest.h>
 #include <click/preview.h>
 #include <fake_json.h>
+#include <click/index.h>
+#include <click/reviews.h>
 
 using namespace ::testing;
 using namespace unity::scopes;
@@ -46,23 +48,57 @@
 
 };
 
+class FakeIndex : public click::Index
+{
+public:
+    FakeIndex() {
+
+    }
+    click::web::Cancellable get_details(const std::string& /*package_name*/, std::function<void(click::PackageDetails, Error)> callback) override {
+        callback(click::PackageDetails(), Error::NetworkError);
+        return click::web::Cancellable();
+    }
+
+};
+
+class FakeReviews : public click::Reviews
+{
+public:
+    FakeReviews() {
+
+    }
+
+    click::web::Cancellable fetch_reviews(const std::string &/*package_name*/, std::function<void (click::ReviewList, Error)> callback) override {
+        callback(click::ReviewList(), Error::NoError);
+        return click::web::Cancellable();
+    }
+};
+
 class FakePreview : public click::PreviewStrategy
 {
 public:
     FakePreview(Result& result) : click::PreviewStrategy::PreviewStrategy(result)
     {
-
+        index.reset(new FakeIndex());
+        reviews.reset(new FakeReviews());
     }
 
     void run(const PreviewReplyProxy &/*reply*/)
     {
 
     }
+
+    void run_under_qt(const std::function<void()> &task) {
+        // when testing, do not actually run under qt
+        task();
+    }
+
     using click::PreviewStrategy::screenshotsWidgets;
     using click::PreviewStrategy::descriptionWidgets;
     using click::PreviewStrategy::build_other_metadata;
     using click::PreviewStrategy::build_updates_table;
     using click::PreviewStrategy::build_whats_new;
+    using click::PreviewStrategy::populateDetails;
 };
 
 class PreviewsBaseTest : public Test
@@ -103,6 +139,19 @@
     ASSERT_EQ(sources[2].get_string(), "sshot3");
 }
 
+TEST_F(PreviewStrategyTest, testEmptyResults)
+{
+    FakeResult result{vm};
+    result["name"] = "fake name";
+    FakePreview preview{result};
+    preview.populateDetails(
+                [](const click::PackageDetails &){
+                },
+                [](const click::ReviewList&, click::Reviews::Error){
+                });
+
+}
+
 class PreviewStrategyDescriptionTest : public PreviewStrategyTest
 {
 public: