Merge lp:~mandel/ubuntu-system-settings/donot-query-on-error into lp:ubuntu-system-settings

[Manuel de la Peña]

Sign the query in order to get a possible auth error from the server and stop the user from trying to do updates with invalid creds.

Sign the query in order to get a possible auth error from the server and stop the user from trying to do updates with invalid creds.

[lp:~mandel/ubuntu-system-settings/donot-query-on-error]

1513. By Manuel de la Peña on 2015-08-27

Improve the ui used to show the lack of creds.

[Ken VanDine (ken-vandine)]

This looks great, one suggestion:

In PageComponent.qml, handle onCredentialsDeleted the same as onCredentialsNotFound. This will show the prompt to signin to U1 under the system update.

[Ken VanDine (ken-vandine)]

Perfect, thanks!

review: Approve

[lp:~mandel/ubuntu-system-settings/donot-query-on-error]

1514. By Manuel de la Peña on 2015-08-27

Move the network to its correct location.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1512
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/2363/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3880/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-amd64-ci/138/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-i386-ci/547/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3877/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/2363/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1514
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/2366/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/3883
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-amd64-ci/141
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-i386-ci/550
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/3187
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3880
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/3880/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22865

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/2366/rebuild

review: Needs Fixing (continuous-integration)

=== modified file 'plugins/system-update/PageComponent.qml'
--- plugins/system-update/PageComponent.qml	2015-06-29 13:06:41 +0000
+++ plugins/system-update/PageComponent.qml	2015-08-27 10:19:03 +0000
@@ -205,8 +205,7 @@
         }
 
         onCredentialsDeleted: {
-            credentialsNotification.visible = false;
-            uoaConfig.exec();
+            credentialsNotification.visible = true;
         }
 
         onSystemUpdateDownloaded: {

=== modified file 'plugins/system-update/download_tracker.cpp'
--- plugins/system-update/download_tracker.cpp	2015-02-26 18:59:51 +0000
+++ plugins/system-update/download_tracker.cpp	2015-08-27 10:19:03 +0000
@@ -23,7 +23,7 @@
 #include <ubuntu/download_manager/error.h>
 
 #include "download_tracker.h"
-#include "network/network.h"
+#include "network.h"
 
 namespace {
     const QString DOWNLOAD_COMMAND = "post-download-command";

=== removed directory 'plugins/system-update/network'
=== renamed file 'plugins/system-update/network/network.cpp' => 'plugins/system-update/network.cpp'
--- plugins/system-update/network/network.cpp	2015-08-27 10:19:03 +0000
+++ plugins/system-update/network.cpp	2015-08-27 10:19:03 +0000
@@ -29,7 +29,6 @@
 
 namespace {
     const QString URL_APPS = "https://search.apps.ubuntu.com/api/v1/click-metadata";
-    const QString APPS_DATA = "APPS_DATA";
     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")
@@ -58,6 +57,49 @@
     return result;
 }
 
+bool Network::replyIsValid(QNetworkReply *reply)
+{
+    auto statusAttr = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute);
+    if (!statusAttr.isValid()) {
+        Q_EMIT errorOccurred();
+        return false;
+    }
+
+    int httpStatus = statusAttr.toInt();
+    qWarning() << "HTTP Status: " << httpStatus;
+
+    if (httpStatus == 401 || httpStatus == 403) {
+        qWarning() << "Emitting credetials error.";
+        Q_EMIT credentialError();
+        return false;
+    }
+
+    return true;
+}
+
+void Network::onTokenRequestFinished(Update* app, QNetworkReply* r)
+{
+    // the scoped pointer will take care of calling the deleteLater when leaving the slot
+    QScopedPointer<QNetworkReply, QScopedPointerDeleteLater> reply(r);
+
+    // check if the reply is valid, this method will emit all the required signals to propagate errors to the rest
+    // of the plugin
+    if (!replyIsValid(r)) {
+        qWarning() << "Reply is not valid.";
+        return;
+    }
+
+    if (reply->hasRawHeader(X_CLICK_TOKEN)) {
+        if (app != nullptr) {
+            QString header(reply->rawHeader(X_CLICK_TOKEN));
+            Q_EMIT clickTokenObtained(app, header);
+        }
+        return;
+    }
+
+    Q_EMIT errorOccurred();
+}
+
 std::string Network::architectureFromDpkg()
 {
     QString program("dpkg");
@@ -91,6 +133,7 @@
 
 void Network::checkForNewVersions(QHash<QString, Update*> &apps)
 {
+    qWarning() << __PRETTY_FUNCTION__;
     m_apps = apps;
     QJsonObject serializer;
     QJsonArray array;
@@ -107,18 +150,19 @@
     QByteArray content = doc.toJson();
 
     QString urlApps = getUrlApps();
+    QString authHeader = m_token.signUrl(urlApps, QStringLiteral("POST"), true);
+    QUrl url(urlApps);
+    url.setQuery(authHeader);
+
     QNetworkRequest request;
     request.setHeader(QNetworkRequest::ContentTypeHeader, "application/json");
     request.setRawHeader(QByteArray("X-Ubuntu-Frameworks"), QByteArray::fromStdString(frameworks.str()));
     request.setRawHeader(QByteArray("X-Ubuntu-Architecture"), QByteArray::fromStdString(getArchitecture()));
-    request.setUrl(QUrl(urlApps));
-
-    RequestObject* reqObject = new RequestObject(QString(APPS_DATA));
-    request.setOriginatingObject(reqObject);
+    request.setUrl(url);
 
     auto reply = m_nam.post(request, content);
 
-    connect(reply, &QNetworkReply::finished, this, &Network::onReplyFinished);
+    connect(reply, &QNetworkReply::finished, this, &Network::onUpdatesCheckFinished);
     connect(reply, &QNetworkReply::sslErrors, this, &Network::onReplySslErrors);
     connect(reply, static_cast<void(QNetworkReply::*)
             (QNetworkReply::NetworkError)>(&QNetworkReply::error),
@@ -139,66 +183,49 @@
     return command;
 }
 
-void Network::onReplyFinished()
+void Network::onUpdatesCheckFinished()
 {
     // the scoped pointer will take care of calling the deleteLater when leaving the slot
-    QScopedPointer<QNetworkReply, QScopedPointerDeleteLater> reply(qobject_cast<QNetworkReply*>(sender()));
-    auto statusAttr = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute);
-    if (!statusAttr.isValid()) {
-        Q_EMIT errorOccurred();
-        return;
-    }
-
-    int httpStatus = statusAttr.toInt();
-
-    if (httpStatus == 401 || httpStatus == 403) {
-        Q_EMIT credentialError();
-        return;
-    }
-
-    if (httpStatus == 200 || httpStatus == 201) {
-        if (reply->hasRawHeader(X_CLICK_TOKEN)) {
-            auto app = qobject_cast<Update*>(reply->request().originatingObject());
-            if (app != nullptr) {
-                QString header(reply->rawHeader(X_CLICK_TOKEN));
-                Q_EMIT clickTokenObtained(app, header);
-            }
-            return;
-        }
-
-        QByteArray payload = reply->readAll();
-        QJsonDocument document = QJsonDocument::fromJson(payload);
-
-        auto state = qobject_cast<RequestObject*>(reply->request().originatingObject());
-        if (state != nullptr && state->operation.contains(APPS_DATA) && document.isArray()) {
-            QJsonArray array = document.array();
-            bool updates = false;
-            for (int i = 0; i < array.size(); i++) {
-                auto object = array.at(i).toObject();
-                auto name = object["name"].toString();
-                auto version = object["version"].toString();
-                auto icon_url = object["icon_url"].toString();
-                auto url = object["download_url"].toString();
-                auto download_sha512 = object["download_sha512"].toString();
-                auto size = object["binary_filesize"].toInt();
-                if (m_apps.contains(name)) {
-                    m_apps[name]->setRemoteVersion(version);
-                    if (m_apps[name]->updateRequired()) {
-                        m_apps[name]->setIconUrl(icon_url);
-                        m_apps[name]->setDownloadUrl(url);
-                        m_apps[name]->setBinaryFilesize(size);
-                        m_apps[name]->setDownloadSha512(download_sha512);
-                        updates = true;
-                    }
+    auto r = qobject_cast<QNetworkReply*>(sender());
+
+    // check for http error status and emit all the required signals
+    if (!replyIsValid(r)) {
+        qWarning() << "Reply is not valid.";
+        return;
+    }
+
+    QScopedPointer<QNetworkReply, QScopedPointerDeleteLater> reply(r);
+
+    auto document = QJsonDocument::fromJson(reply->readAll());
+
+    if (document.isArray()) {
+        QJsonArray array = document.array();
+        bool updates = false;
+        for (int i = 0; i < array.size(); i++) {
+            auto object = array.at(i).toObject();
+            auto name = object["name"].toString();
+            auto version = object["version"].toString();
+            auto icon_url = object["icon_url"].toString();
+            auto url = object["download_url"].toString();
+            auto download_sha512 = object["download_sha512"].toString();
+            auto size = object["binary_filesize"].toInt();
+            if (m_apps.contains(name)) {
+                m_apps[name]->setRemoteVersion(version);
+                if (m_apps[name]->updateRequired()) {
+                    m_apps[name]->setIconUrl(icon_url);
+                    m_apps[name]->setDownloadUrl(url);
+                    m_apps[name]->setBinaryFilesize(size);
+                    m_apps[name]->setDownloadSha512(download_sha512);
+                    updates = true;
                 }
             }
-            if (updates) {
-                Q_EMIT updatesFound();
-                return;
-            } else {
-                Q_EMIT updatesNotFound();
-                return;
-            }
+        }
+        if (updates) {
+            Q_EMIT updatesFound();
+            return;
+        } else {
+            Q_EMIT updatesNotFound();
+            return;
         }
     }
 
@@ -229,20 +256,31 @@
     reply->deleteLater();
 }
 
-void Network::getClickToken(Update *app, const QString &url,
-                            const QString &authHeader)
+void Network::getClickToken(Update *app, const QString &url)
 {
+    qWarning() << __PRETTY_FUNCTION__;
+    if (!m_token.isValid()) {
+        app->setError("Invalid User Token");
+        return;
+    }
+
+    QString authHeader = m_token.signUrl(app->downloadUrl(), QStringLiteral("HEAD"), true);
+    app->setClickUrl(app->downloadUrl());
+
     QProcessEnvironment environment = QProcessEnvironment::systemEnvironment();
     QString signUrl = environment.value("CLICK_TOKEN_URL", url);
     QUrl query(signUrl);
     query.setQuery(authHeader);
     QNetworkRequest request;
     request.setUrl(query);
-    request.setOriginatingObject(app);
 
     auto reply = m_nam.head(request);
 
-    connect(reply, &QNetworkReply::finished, this, &Network::onReplyFinished);
+    connect(reply, &QNetworkReply::finished, this, [=](){
+        auto reply = qobject_cast<QNetworkReply*>(sender());
+        onTokenRequestFinished(app, reply);
+    });
+
     connect(reply, &QNetworkReply::sslErrors, this, &Network::onReplySslErrors);
     connect(reply, static_cast<void(QNetworkReply::*)
                     (QNetworkReply::NetworkError)>(&QNetworkReply::error),

=== renamed file 'plugins/system-update/network/network.h' => 'plugins/system-update/network.h'
--- plugins/system-update/network/network.h	2015-08-27 10:19:03 +0000
+++ plugins/system-update/network.h	2015-08-27 10:19:03 +0000
@@ -19,29 +19,18 @@
 #ifndef NETWORK_H
 #define NETWORK_H
 
+#include <token.h>
 #include <QObject>
 #include <QtNetwork/QNetworkAccessManager>
 #include <QtNetwork/QNetworkReply>
 #include <QHash>
-#include "../update.h"
+
+#include "update.h"
 
 #define X_CLICK_TOKEN "X-Click-Token"
 
 namespace UpdatePlugin {
 
-class RequestObject : public QObject
-{
-    Q_OBJECT
-public:
-    explicit RequestObject(QString oper, QObject *parent = 0) :
-        QObject(parent)
-    {
-        operation = oper;
-    }
-
-    QString operation;
-};
-
 class Network : public QObject
 {
     Q_OBJECT
@@ -49,11 +38,14 @@
     explicit Network(QObject *parent = 0);
 
     void checkForNewVersions(QHash<QString, Update*> &apps);
-    void getClickToken(Update *app, const QString &url,
-                       const QString &authHeader);
+    void getClickToken(Update *app, const QString &url);
+
     virtual std::vector<std::string> getAvailableFrameworks();
     virtual std::string getArchitecture();
 
+    void setUbuntuOneToken(UbuntuOne::Token token) { m_token = token; }
+    UbuntuOne::Token getUbuntuOneToken() { return m_token; }
+
 Q_SIGNALS:
     void updatesFound();
     void updatesNotFound();
@@ -64,16 +56,19 @@
     void credentialError();
 
 private Q_SLOTS:
-    void onReplyFinished();
+    void onUpdatesCheckFinished();
     void onReplySslErrors(const QList<QSslError> & errors);
     void onReplyError(QNetworkReply::NetworkError code);
 
 private:
     QNetworkAccessManager m_nam;
     QHash<QString, Update*> m_apps;
+    UbuntuOne::Token m_token;
 
     QString getUrlApps();
     QString getFrameworksDir();
+    bool replyIsValid(QNetworkReply *reply);
+    void onTokenRequestFinished(Update* update, QNetworkReply* reply);
 
 protected:
     virtual std::string architectureFromDpkg();

=== modified file 'plugins/system-update/plugin/CMakeLists.txt'
--- plugins/system-update/plugin/CMakeLists.txt	2014-10-22 19:37:42 +0000
+++ plugins/system-update/plugin/CMakeLists.txt	2015-08-27 10:19:03 +0000
@@ -1,7 +1,7 @@
 SET (CMAKE_AUTOMOC ON)
 include_directories(${CMAKE_CURRENT_BINARY_DIR})
 
-add_library(update-plugin SHARED update-plugin.h update-plugin.cpp ../network/network.cpp ../network/network.h
+add_library(update-plugin SHARED update-plugin.h update-plugin.cpp ../network.cpp ../network.h
 ../update.cpp ../update.h ../update_manager.cpp ../update_manager.h ../system_update.cpp ../system_update.h
 ../download_tracker.h ../download_tracker.cpp)
 

=== modified file 'plugins/system-update/update.h'
--- plugins/system-update/update.h	2014-10-07 17:54:46 +0000
+++ plugins/system-update/update.h	2015-08-27 10:19:03 +0000
@@ -18,8 +18,7 @@
  *
 */
 
-#ifndef UPDATE_H
-#define UPDATE_H
+#pragma once
 
 #include <QObject>
 #include <QtQml>
@@ -144,5 +143,3 @@
 };
 
 }
-
-#endif // UPDATE_H

=== modified file 'plugins/system-update/update_manager.cpp'
--- plugins/system-update/update_manager.cpp	2015-08-27 10:19:03 +0000
+++ plugins/system-update/update_manager.cpp	2015-08-27 10:19:03 +0000
@@ -18,15 +18,19 @@
  *
 */
 
-#include "update_manager.h"
-#include <QString>
-#include <QStringList>
+
+#include <QDBusInterface>
 #include <QJsonDocument>
 #include <QJsonObject>
 #include <QJsonArray>
 #include <QJsonValue>
 #include <QProcessEnvironment>
-#include <QDBusInterface>
+#include <QString>
+#include <QStringList>
+#include <ssoservice.h>
+
+#include "update_manager.h"
+#include "network.h"
 
 using namespace UbuntuOne;
 
@@ -164,7 +168,7 @@
 
 void UpdateManager::handleCredentialsFound(Token token)
 {
-    m_token = token;
+    m_network.setUbuntuOneToken(token);
     QStringList args("list");
     args << "--manifest";
     QString command = getClickCommand();
@@ -332,13 +336,7 @@
 
 void UpdateManager::downloadApp(Update *app)
 {
-    if (m_token.isValid()) {
-        QString authHeader = m_token.signUrl(app->downloadUrl(), QStringLiteral("HEAD"), true);
-        app->setClickUrl(app->downloadUrl());
-        m_network.getClickToken(app, app->downloadUrl(), authHeader);
-    } else {
-        app->setError("Invalid User Token");
-    }
+    m_network.getClickToken(app, app->downloadUrl());
 }
 
 void UpdateManager::clickTokenReceived(Update *app, const QString &clickToken)

=== modified file 'plugins/system-update/update_manager.h'
--- plugins/system-update/update_manager.h	2015-08-27 10:19:03 +0000
+++ plugins/system-update/update_manager.h	2015-08-27 10:19:03 +0000
@@ -18,8 +18,7 @@
  *
 */
 
-#ifndef UPDATEMANAGER_H
-#define UPDATEMANAGER_H
+#pragma once
 
 #include <QObject>
 #include <QtQml>
@@ -40,7 +39,7 @@
 #else
 #include <ssoservice.h>
 #include <QProcess>
-#include "network/network.h"
+#include "network.h"
 #include "system_update.h"
 #endif
 
@@ -112,8 +111,8 @@
     QHash<QString, Update*> get_apps() { return m_apps; }
     QVariantList get_model() { return m_model; }
     int get_downloadMode() { return m_downloadMode; }
-    void set_token(Token& t) { m_token = t; }
-    Token get_token() { return m_token; }
+    void set_token(Token& t) { m_network.setUbuntuOneToken(t); }
+    Token get_token() { return m_network.getUbuntuOneToken(); }
     void setCheckintUpdates(int value) { m_checkingUpdates = value; }
     void setCheckSystemUpdates(int value) { m_systemCheckingUpdate = value; }
     void setCheckClickUpdates(int value) { m_clickCheckingUpdate = value; }
@@ -149,7 +148,6 @@
     QHash<QString, Update*> m_apps;
     int m_downloadMode;
     QVariantList m_model;
-    Token m_token;
     QString m_latestDownload;
 
 #ifdef TESTS
@@ -174,5 +172,3 @@
 };
 
 }
-
-#endif // UPDATEMANAGER_H

=== modified file 'tests/plugins/system-update/CMakeLists.txt'
--- tests/plugins/system-update/CMakeLists.txt	2015-04-30 19:35:35 +0000
+++ tests/plugins/system-update/CMakeLists.txt	2015-08-27 10:19:03 +0000
@@ -28,7 +28,7 @@
 add_executable(tst-network
     tst_network.cpp
     ../../../plugins/system-update/update.cpp
-    ../../../plugins/system-update/network/network.cpp
+    ../../../plugins/system-update/network.cpp
 )
 
 # set the path to the library folder

=== modified file 'tests/plugins/system-update/fakenetwork.cpp'
--- tests/plugins/system-update/fakenetwork.cpp	2014-02-26 11:26:01 +0000
+++ tests/plugins/system-update/fakenetwork.cpp	2015-08-27 10:19:03 +0000
@@ -40,10 +40,9 @@
     emit this->downloadUrlFound(packagename, "http://canonical.com");
 }
 
-void FakeNetwork::getClickToken(Update* app, const QString& url, const QString& authHeader)
+void FakeNetwork::getClickToken(Update* app, const QString& url)
 {
     Q_UNUSED(url);
-    Q_UNUSED(authHeader);
     QString fakeHeader("x-click-token-header");
     emit this->clickTokenObtained(app, fakeHeader);
 }

=== modified file 'tests/plugins/system-update/fakenetwork.h'
--- tests/plugins/system-update/fakenetwork.h	2015-08-27 10:19:03 +0000
+++ tests/plugins/system-update/fakenetwork.h	2015-08-27 10:19:03 +0000
@@ -19,9 +19,12 @@
 #ifndef FAKENETWORK_H
 #define FAKENETWORK_H
 
+#include <token.h>
+
 #include <QObject>
 #include <QHash>
 #include <QString>
+
 #include "update.h"
 
 namespace UpdatePlugin {
@@ -34,8 +37,10 @@
 
     void checkForNewVersions(QHash<QString, Update*> &apps);
     void getResourceUrl(const QString& packagename);
-    void getClickToken(Update* app, const QString& url, const QString& authHeader);
-    
+    void getClickToken(Update* app, const QString& url);
+    void setUbuntuOneToken(UbuntuOne::Token t) { m_token = t; }
+    UbuntuOne::Token getUbuntuOneToken() { return m_token; }
+
 signals:
     void updatesFound();
     void updatesNotFound();
@@ -45,6 +50,9 @@
     void credentialError();
     void downloadUrlFound(const QString& packagename, const QString& url);
     void clickTokenObtained(Update* app, const QString& clickToken);
+
+private:
+    UbuntuOne::Token m_token;
 };
 
 }

=== modified file 'tests/plugins/system-update/tst_network.cpp'
--- tests/plugins/system-update/tst_network.cpp	2015-04-30 14:16:22 +0000
+++ tests/plugins/system-update/tst_network.cpp	2015-08-27 10:19:03 +0000
@@ -22,7 +22,7 @@
 #include <QObject>
 #include <QTest>
 #include <QString>
-#include "network/network.h"
+#include "network.h"
 #include "update.h"
 
 using namespace UpdatePlugin;