Merge lp:~dobey/unity-scope-click/really-we-want-to-sign-all-the-requests into lp:unity-scope-click

[dobey]

Bring back the refactor to sign all requests.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:440
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/733/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-amd64-ci/137/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-armhf-ci/133/console

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

review: Needs Fixing (continuous-integration)

[Charles Kerr (charlesk)]

LGTM.

review: Approve

=== modified file 'libclickscope/click/download-manager.cpp'
--- libclickscope/click/download-manager.cpp	2016-03-30 14:14:14 +0000
+++ libclickscope/click/download-manager.cpp	2016-04-01 17:25:41 +0000
@@ -154,11 +154,11 @@
                 });
     QObject::connect(response.data(), &click::web::Response::error,
                      [this, callback, package_name](QString error, int error_code) {
-                         qDebug() << QStringLiteral("Network error (%1) fetching click token for:").arg(error_code) << package_name.c_str();
+                         qWarning() << QStringLiteral("Network error (%1) fetching click token for:").arg(error_code) << package_name.c_str();
                          switch(error_code) {
                          case 401:
                          case 403:
-                             sso->invalidateCredentials();
+                             client->invalidateCredentials();
                              callback(error.toUtf8().data(), Error::CredentialsError);
                              break;
                          default:
@@ -172,7 +172,6 @@
 void DownloadManager::setCredentialsService(const QSharedPointer<click::CredentialsService>& credentialsService)
 {
     sso = credentialsService;
-    client->setCredentialsService(sso);
 }
 
 } // namespace click

=== modified file 'libclickscope/click/pay.cpp'
--- libclickscope/click/pay.cpp	2016-03-30 14:14:14 +0000
+++ libclickscope/click/pay.cpp	2016-04-01 17:25:41 +0000
@@ -228,9 +228,6 @@
 
 click::web::Cancellable Package::get_purchases(std::function<void(const PurchaseSet&)> callback)
 {
-    QSharedPointer<click::CredentialsService> sso(new click::CredentialsService());
-    client->setCredentialsService(sso);
-
     QSharedPointer<click::web::Response> response = client->call
         (get_base_url() + pay::API_ROOT + pay::PURCHASES_API_PATH, "GET", true);
 

=== modified file 'libclickscope/click/preview.cpp'
--- libclickscope/click/preview.cpp	2016-03-30 14:14:14 +0000
+++ libclickscope/click/preview.cpp	2016-04-01 17:25:41 +0000
@@ -734,8 +734,6 @@
     std::promise<bool> promise;
     auto future = promise.get_future();
     run_under_qt([this, reply, &promise]() {
-            QSharedPointer<click::CredentialsService> sso(new click::CredentialsService());
-            dm->setCredentialsService(sso);
             dm->start(download_url, download_sha512, result["name"].get_string(),
                       [this, reply, &promise] (std::string msg, DownloadManager::Error dmerr){
                           switch (dmerr)
@@ -888,8 +886,6 @@
             std::promise<bool> submit_promise;
             std::future<bool> submit_future = submit_promise.get_future();
             qt::core::world::enter_with_task([this, review, &submit_promise, widget_id]() mutable {
-                    QSharedPointer<click::CredentialsService> sso(new click::CredentialsService());
-                    client->setCredentialsService(sso);
                     if (widget_id == "rating") {
                         submit_operation = reviews->submit_review(review,
                                                               [&submit_promise](click::Reviews::Error){

=== modified file 'libclickscope/click/reviews.cpp'
--- libclickscope/click/reviews.cpp	2016-03-30 14:14:14 +0000
+++ libclickscope/click/reviews.cpp	2016-04-01 17:25:41 +0000
@@ -131,7 +131,8 @@
     click::web::CallParams params;
     params.add(click::REVIEWS_QUERY_ARGNAME, package_name.c_str());
     QSharedPointer<click::web::Response> response = client->call
-        (get_base_url() + click::REVIEWS_API_PATH, params);
+        (get_base_url() + click::REVIEWS_API_PATH, "GET", false,
+         std::map<std::string, std::string>{}, "", params);
 
     QObject::connect(response.data(), &click::web::Response::finished,
                 [=](QString reply) {

=== modified file 'libclickscope/click/ubuntuone_credentials.cpp'
--- libclickscope/click/ubuntuone_credentials.cpp	2016-03-30 14:14:14 +0000
+++ libclickscope/click/ubuntuone_credentials.cpp	2016-04-01 17:25:41 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Canonical Ltd.
+ * Copyright (C) 2014-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
@@ -29,6 +29,9 @@
 
 #include "ubuntuone_credentials.h"
 
+#include <future>
+#include <QCoreApplication>
+
 namespace u1 = UbuntuOne;
 
 click::CredentialsService::CredentialsService()
@@ -47,6 +50,43 @@
 {
 }
 
+UbuntuOne::Token click::CredentialsService::getToken()
+{
+    if (!_token.isValid()) {
+        std::promise<UbuntuOne::Token> promise;
+        auto future = promise.get_future();
+
+        auto success = QObject::connect(ssoService.data(),
+                                        &u1::SSOService::credentialsFound,
+                                        [this, &promise](const u1::Token& token) {
+                                            emit credentialsFound(_token);
+                                            promise.set_value(token);
+                                        });
+        auto notfound = QObject::connect(ssoService.data(),
+                                         &u1::SSOService::credentialsNotFound,
+                                         [this, &promise]() {
+                                             qWarning() << "No Ubuntu One token found.";
+                                             emit credentialsNotFound();
+                                             promise.set_value(u1::Token());
+                                         });
+
+        getCredentials();
+
+        std::future_status status = future.wait_for(std::chrono::milliseconds(0));
+        while (status != std::future_status::ready) {
+            QCoreApplication::processEvents();
+            qDebug() << "Processed some events, waiting to process again.";
+            status = future.wait_for(std::chrono::milliseconds(100));
+        }
+
+        _token = future.get();
+        QObject::disconnect(success);
+        QObject::disconnect(notfound);
+    }
+
+    return _token;
+}
+
 void click::CredentialsService::getCredentials()
 {
     ssoService->getCredentials();

=== modified file 'libclickscope/click/ubuntuone_credentials.h'
--- libclickscope/click/ubuntuone_credentials.h	2016-03-30 14:14:14 +0000
+++ libclickscope/click/ubuntuone_credentials.h	2016-04-01 17:25:41 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Canonical Ltd.
+ * Copyright (C) 2014-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
@@ -47,6 +47,7 @@
     
     CredentialsService& operator=(const CredentialsService&) = delete;
 
+    virtual UbuntuOne::Token getToken();
     virtual void getCredentials();
     virtual void invalidateCredentials();
 
@@ -57,7 +58,7 @@
 
 private:
     QScopedPointer<UbuntuOne::SSOService> ssoService;
-
+    UbuntuOne::Token _token;
 }; // CredentialsService
 
 } // namespace click

=== modified file 'libclickscope/click/webclient.cpp'
--- libclickscope/click/webclient.cpp	2016-03-30 14:14:14 +0000
+++ libclickscope/click/webclient.cpp	2016-04-01 17:25:41 +0000
@@ -82,7 +82,7 @@
     const std::string& iri,
     const click::web::CallParams& params)
 {
-    return call(iri, "GET", false,
+    return call(iri, "GET", true,
                 std::map<std::string, std::string>(), "", params);
 }
 
@@ -132,25 +132,21 @@
     request->setRawHeader(DEVICE_ID_HEADER.c_str(), deviceId.data());
 
     if (sign && !impl->sso.isNull()) {
-        click::utils::SmartConnect sc(responsePtr.data());
-        sc.connect(impl->sso.data(), &click::CredentialsService::credentialsFound,
-                   [=](const UbuntuOne::Token& token) {
-                       QString auth_header = token.signUrl(url.toString(),
-                                                           method.c_str());
-                       qDebug() << "Signed URL:" << request->url().toString();
-                       request->setRawHeader(AUTHORIZATION_HEADER.c_str(), auth_header.toUtf8());
-                       impl->sso.clear();
-                       doConnect();
-                   });
-        sc.connect(impl->sso.data(), &click::CredentialsService::credentialsNotFound,
-                   [=]() {
-                       impl->sso.clear();
-                       qWarning() << "Signing reuested but no credentials found. Using unsigned URL.";
-                       doConnect();
-                   });
-        // TODO: Need to handle error signal once in CredentialsService.
-        impl->sso->getCredentials();
+        auto token = impl->sso->getToken();
+        if (token.isValid()) {
+            QString auth_header = token.signUrl(url.toString(),
+                                                method.c_str());
+            qDebug() << "Signed URL:" << request->url().toString();
+            request->setRawHeader(AUTHORIZATION_HEADER.c_str(), auth_header.toUtf8());
+        } else {
+            qWarning() << "Signing reuested but returned token is invalid.";
+        }
+
+        doConnect();
     } else {
+        if (sign && impl->sso.isNull()) {
+            qCritical() << "Unable to sign request without SSO object.";
+        }
         doConnect();
     }
 

=== modified file 'libclickscope/click/webclient.h'
--- libclickscope/click/webclient.h	2016-03-30 14:14:14 +0000
+++ libclickscope/click/webclient.h	2016-04-01 17:25:41 +0000
@@ -121,7 +121,7 @@
     virtual QSharedPointer<Response> call(
         const std::string& iri,
         const std::string& method,
-        bool sign = false,
+        bool sign = true,
         const std::map<std::string, std::string>& headers = std::map<std::string, std::string>(),
         const std::string& data = "",
         const CallParams& params = CallParams());

=== modified file 'libclickscope/tests/mock_ubuntuone_credentials.h'
--- libclickscope/tests/mock_ubuntuone_credentials.h	2016-03-30 14:14:14 +0000
+++ libclickscope/tests/mock_ubuntuone_credentials.h	2016-04-01 17:25:41 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Canonical Ltd.
+ * Copyright (C) 2014-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
@@ -27,11 +27,11 @@
  * files in the program, then also delete it here.
  */
 
+#include <token.h>
 
 class MockCredentialsService : public click::CredentialsService {
  public:
-  MOCK_METHOD0(getCredentials,
-      void());
-  MOCK_METHOD0(invalidateCredentials,
-      void());
+    MOCK_METHOD0(getToken, UbuntuOne::Token());
+    MOCK_METHOD0(getCredentials, void());
+    MOCK_METHOD0(invalidateCredentials, void());
 };

=== modified file 'libclickscope/tests/mock_webclient.h'
--- libclickscope/tests/mock_webclient.h	2016-03-30 14:14:14 +0000
+++ libclickscope/tests/mock_webclient.h	2016-04-01 17:25:41 +0000
@@ -87,13 +87,13 @@
     QSharedPointer<click::web::Response> call(
         const std::string& iri,
         const click::web::CallParams& params=click::web::CallParams()) override {
-        return callImpl(iri, "GET", false,
+        return callImpl(iri, "GET", true,
                         std::map<std::string, std::string>(), "", params);
     }
     QSharedPointer<click::web::Response> call(
         const std::string& iri,
         const std::string& method,
-        bool sign = false,
+        bool sign = true,
         const std::map<std::string, std::string>& headers = std::map<std::string, std::string>(),
         const std::string& data = "",
         const click::web::CallParams& params=click::web::CallParams()) override {
@@ -102,7 +102,6 @@
 
     MOCK_METHOD1(has_header, bool(const std::string& header));
     MOCK_METHOD1(get_header, std::string(const std::string&header));
-    MOCK_METHOD0(invalidateCredentials, void());
 };
 
 }

=== modified file 'libclickscope/tests/test_download_manager.cpp'
--- libclickscope/tests/test_download_manager.cpp	2016-03-30 14:14:14 +0000
+++ libclickscope/tests/test_download_manager.cpp	2016-04-01 17:25:41 +0000
@@ -63,6 +63,7 @@
 
     virtual void SetUp()
     {
+        ssoPtr.reset(new MockCredentialsService());
         namPtr.reset(new MockNetworkAccessManager());
         clientPtr.reset(new NiceMock<MockClient>(namPtr));
         clientPtr->setCredentialsService(ssoPtr);
@@ -160,9 +161,6 @@
     LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
     auto response = responseForReply(reply.asSharedPtr());
 
-    QSharedPointer<MockCredentialsService> sso(new MockCredentialsService());
-    dmPtr->setCredentialsService(sso);
-
     EXPECT_CALL(reply.instance, errorString())
         .WillOnce(Return(QString("ERROR")));
     EXPECT_CALL(reply.instance, attribute(_)).WillOnce(Return(QVariant(401)));
@@ -172,7 +170,7 @@
     EXPECT_CALL(*clientPtr, callImpl(_, _, _, _, _, _))
             .Times(1)
             .WillOnce(Return(response));
-    EXPECT_CALL(*(sso.data()), invalidateCredentials());
+    EXPECT_CALL(*ssoPtr, invalidateCredentials());
     EXPECT_CALL(*this, start_callback("ERROR (201)",
                                       click::DownloadManager::Error::CredentialsError)).Times(1);
 

=== modified file 'libclickscope/tests/test_index.cpp'
--- libclickscope/tests/test_index.cpp	2016-03-30 14:14:14 +0000
+++ libclickscope/tests/test_index.cpp	2016-04-01 17:25:41 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Canonical Ltd.
+ * Copyright (C) 2014-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
@@ -190,6 +190,18 @@
     indexPtr->departments("departments", [](const click::DepartmentList&, const click::HighlightList&, click::Index::Error, int) {});
 }
 
+TEST_F(IndexTest, testDetailsSignsCall)
+{
+    LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
+    auto response = responseForReply(reply.asSharedPtr());
+
+    EXPECT_CALL(*clientPtr, callImpl(_, _, true, _, _, _))
+            .Times(1)
+            .WillOnce(Return(response));
+
+    indexPtr->get_details("fake-app", [](const click::PackageDetails, click::Index::Error) {});
+}
+
 TEST_F(IndexTest, testSearchSendsRightPath)
 {
     LifetimeHelper<click::network::Reply, MockNetworkReply> reply;

=== modified file 'libclickscope/tests/test_reviews.cpp'
--- libclickscope/tests/test_reviews.cpp	2016-03-30 14:14:14 +0000
+++ libclickscope/tests/test_reviews.cpp	2016-04-01 17:25:41 +0000
@@ -150,6 +150,19 @@
                                      click::Reviews::Error) {});
 }
 
+TEST_F(ReviewsTest, testFetchReviewsDoesNotSignCall)
+{
+    LifetimeHelper<click::network::Reply, MockNetworkReply> reply;
+    auto response = responseForReply(reply.asSharedPtr());
+
+    EXPECT_CALL(*clientPtr, callImpl(_, _, false, _, _, _))
+            .Times(1)
+            .WillOnce(Return(response));
+
+    reviewsPtr->fetch_reviews("", [](click::ReviewList,
+                                     click::Reviews::Error) {});
+}
+
 TEST_F(ReviewsTest, testFetchReviewsSendsQueryAsParam)
 {
     LifetimeHelper<click::network::Reply, MockNetworkReply> reply;

=== modified file 'libclickscope/tests/test_webclient.cpp'
--- libclickscope/tests/test_webclient.cpp	2016-03-30 14:14:14 +0000
+++ libclickscope/tests/test_webclient.cpp	2016-04-01 17:25:41 +0000
@@ -221,11 +221,9 @@
     click::web::Client wc(namPtr);
     wc.setCredentialsService(ssoPtr);
 
-    EXPECT_CALL(sso, getCredentials()).WillOnce(Invoke([&](){
-                UbuntuOne::Token token("token_key", "token_secret",
-                                       "consumer_key", "consumer_secret");
-                sso.credentialsFound(token);
-            }));
+    EXPECT_CALL(sso, getToken())
+        .WillOnce(Return(UbuntuOne::Token("token_key", "token_secret",
+                                          "consumer_key", "consumer_secret")));
     EXPECT_CALL(nam, sendCustomRequest(IsValidOAuthHeader(true), _, _))
             .Times(1)
             .WillOnce(Return(replyPtr));
@@ -271,9 +269,7 @@
     click::web::Client wc(namPtr);
     wc.setCredentialsService(ssoPtr);
 
-    EXPECT_CALL(sso, getCredentials()).WillOnce(Invoke([&]() {
-                sso.credentialsNotFound();
-            }));
+    EXPECT_CALL(sso, getToken()).WillOnce(Return(UbuntuOne::Token()));
     EXPECT_CALL(nam, sendCustomRequest(IsValidOAuthHeader(false), _, _))
         .Times(1)
         .WillOnce(Return(replyPtr));

=== modified file 'scope/clickapps/apps-scope.cpp'
--- scope/clickapps/apps-scope.cpp	2016-03-30 14:14:14 +0000
+++ scope/clickapps/apps-scope.cpp	2016-04-01 17:25:41 +0000
@@ -81,6 +81,8 @@
     static const int zero = 0;
     auto emptyCb = [this]()
     {
+        sso.reset(new click::CredentialsService());
+        client->setCredentialsService(sso);
         dm.reset(Ubuntu::DownloadManager::Manager::createSessionManager());
     };
 

=== modified file 'scope/clickapps/apps-scope.h'
--- scope/clickapps/apps-scope.h	2016-03-30 14:14:14 +0000
+++ scope/clickapps/apps-scope.h	2016-04-01 17:25:41 +0000
@@ -71,6 +71,7 @@
     QSharedPointer<click::Index> index;
     QSharedPointer<pay::Package> pay_package;
     QSharedPointer<Ubuntu::DownloadManager::Manager> dm;
+    QSharedPointer<click::CredentialsService> sso;
     std::shared_ptr<click::DepartmentsDb> depts_db;
 
     std::string installApplication(unity::scopes::Result const& result);

=== modified file 'scope/clickstore/store-scope.cpp'
--- scope/clickstore/store-scope.cpp	2016-03-30 14:14:14 +0000
+++ scope/clickstore/store-scope.cpp	2016-04-01 17:25:41 +0000
@@ -83,6 +83,8 @@
     static const int zero = 0;
     auto emptyCb = [this]()
     {
+        sso.reset(new click::CredentialsService());
+        client->setCredentialsService(sso);
         dm.reset(Ubuntu::DownloadManager::Manager::createSessionManager());
     };
 

=== modified file 'scope/clickstore/store-scope.h'
--- scope/clickstore/store-scope.h	2016-03-30 14:14:14 +0000
+++ scope/clickstore/store-scope.h	2016-04-01 17:25:41 +0000
@@ -74,6 +74,7 @@
     QSharedPointer<click::Index> index;
     QSharedPointer<pay::Package> pay_package;
     QSharedPointer<Ubuntu::DownloadManager::Manager> dm;
+    QSharedPointer<click::CredentialsService> sso;
     std::shared_ptr<click::DepartmentLookup> depts;
     std::shared_ptr<click::HighlightList> highlights;
     std::shared_ptr<click::DepartmentsDb> depts_db;