Merge lp:~diegosarmentero/clickmanager-plugin/x-click-token-support into lp:clickmanager-plugin

Proposed by Diego Sarmentero
Status: Merged
Approved by: Diego Sarmentero
Approved revision: 40
Merged at revision: 16
Proposed branch: lp:~diegosarmentero/clickmanager-plugin/x-click-token-support
Merge into: lp:clickmanager-plugin
Diff against target: 373 lines (+90/-22)
14 files modified
src/application.cpp (+1/-0)
src/application.h (+3/-0)
src/clickmanager.cpp (+18/-3)
src/clickmanager.h (+3/-0)
src/download/downloader.cpp (+3/-2)
src/download/downloader.h (+1/-1)
src/network/network.cpp (+26/-10)
src/network/network.h (+5/-1)
tests/fakedownloader.cpp (+1/-1)
tests/fakedownloader.h (+1/-1)
tests/fakenetwork.cpp (+6/-0)
tests/fakenetwork.h (+2/-1)
tests/testclickmanager.cpp (+18/-1)
tests/testclickmanager.h (+2/-1)
To merge this branch: bzr merge lp:~diegosarmentero/clickmanager-plugin/x-click-token-support
Reviewer Review Type Date Requested Status
Manuel de la Peña (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Alejandro J. Cura (community) Approve
Review via email: mp+188667@code.launchpad.net

Commit message

- Add x-click-token support.

Description of the change

Use X-Click-Token to send a proper header to the download manager and support download that doesn't expire so quickly

To post a comment you must log in.
30. By Diego Sarmentero

link bug

31. By Diego Sarmentero

change slot name

Revision history for this message
Alejandro J. Cura (alecu) wrote :

179 + } else if (reply->hasRawHeader("X-Click-Token")) {

What's below this line makes no sense at this point of the code flow. If this method is answering for the response to the HEAD request, then the program flow should not even attempt to parse the json.

review: Needs Fixing
Revision history for this message
Alejandro J. Cura (alecu) wrote :

Please also change the variables and methods from xclickToken to clickToken, since the X only means that it's a non-standard HTTP header.

32. By Diego Sarmentero

cleanup

Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

> 179 + } else if (reply->hasRawHeader("X-Click-Token")) {
>
> What's below this line makes no sense at this point of the code flow. If this
> method is answering for the response to the HEAD request, then the program
> flow should not even attempt to parse the json.

Fixed

Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

> Please also change the variables and methods from xclickToken to clickToken,
> since the X only means that it's a non-standard HTTP header.

Fixed

33. By Diego Sarmentero

change variable name

Revision history for this message
Alejandro J. Cura (alecu) wrote :

plus one

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
34. By Diego Sarmentero

Fixing some methods signature

35. By Diego Sarmentero

removing prints

36. By Diego Sarmentero

testing

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
37. By Diego Sarmentero

forward error signal from network

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
38. By Diego Sarmentero

removing qcritical message

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Manuel de la Peña (mandel) wrote :

You are leaking the QNetworkReplies created by the QNetworkAccessManager, you should be calling reply->deleteLater() once you are done with it.

review: Needs Fixing
39. By Diego Sarmentero

deleting reply after use

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
40. By Diego Sarmentero

change method signature

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Manuel de la Peña (mandel) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/application.cpp'
--- src/application.cpp 2013-09-30 14:51:49 +0000
+++ src/application.cpp 2013-10-03 11:37:38 +0000
@@ -36,6 +36,7 @@
36 this->m_selected = false;36 this->m_selected = false;
37 this->m_icon_url = "";37 this->m_icon_url = "";
38 this->m_binary_filesize = 0;38 this->m_binary_filesize = 0;
39 this->m_click_url = "";
39}40}
4041
41void Application::initializeApplication(QString packagename, QString title, QString version)42void Application::initializeApplication(QString packagename, QString title, QString version)
4243
=== modified file 'src/application.h'
--- src/application.h 2013-09-25 13:30:26 +0000
+++ src/application.h 2013-10-03 11:37:38 +0000
@@ -65,6 +65,7 @@
65 bool updateState() { return this->m_update_state; }65 bool updateState() { return this->m_update_state; }
66 bool selected() { return this->m_selected; }66 bool selected() { return this->m_selected; }
67 QString getError() { return this->m_error; }67 QString getError() { return this->m_error; }
68 const QString& getClickUrl() const { return this->m_click_url; }
6869
69 void initializeApplication(QString packagename, QString title, QString version);70 void initializeApplication(QString packagename, QString title, QString version);
70 void setRemoteVersion(QString& version);71 void setRemoteVersion(QString& version);
@@ -74,6 +75,7 @@
74 void setBinaryFilesize(int size) { this->m_binary_filesize = size; emit this->binaryFilesizeChanged(); }75 void setBinaryFilesize(int size) { this->m_binary_filesize = size; emit this->binaryFilesizeChanged(); }
75 void setIconUrl(QString icon) { this->m_icon_url = icon; emit this->iconUrlChanged(); }76 void setIconUrl(QString icon) { this->m_icon_url = icon; emit this->iconUrlChanged(); }
76 void setError(QString error);77 void setError(QString error);
78 void setClickUrl(const QString& url) { this->m_click_url = url; }
7779
78private:80private:
79 QString m_packagename;81 QString m_packagename;
@@ -83,6 +85,7 @@
83 QString m_dbuspath;85 QString m_dbuspath;
84 QString m_icon_url;86 QString m_icon_url;
85 QString m_error;87 QString m_error;
88 QString m_click_url;
86 int m_binary_filesize;89 int m_binary_filesize;
87 bool m_update;90 bool m_update;
88 bool m_update_state;91 bool m_update_state;
8992
=== modified file 'src/clickmanager.cpp'
--- src/clickmanager.cpp 2013-09-25 20:47:03 +0000
+++ src/clickmanager.cpp 2013-10-03 11:37:38 +0000
@@ -41,6 +41,10 @@
41 this, SLOT(processUpdates()));41 this, SLOT(processUpdates()));
42 QObject::connect(&(this->m_network), SIGNAL(updatesNotFound()),42 QObject::connect(&(this->m_network), SIGNAL(updatesNotFound()),
43 this, SIGNAL(updatesNotFound()));43 this, SIGNAL(updatesNotFound()));
44 QObject::connect(&(this->m_network), SIGNAL(errorOccurred()),
45 this, SIGNAL(errorFound()));
46 QObject::connect(&(this->m_network), SIGNAL(clickTokenObtained(Application*, const QString&)),
47 this, SLOT(clickTokenReceived(Application*,const QString&)));
44 QObject::connect(&(this->m_network), SIGNAL(downloadUrlFound(QString,QString)),48 QObject::connect(&(this->m_network), SIGNAL(downloadUrlFound(QString,QString)),
45 this, SLOT(downloadUrlObtained(QString,QString)));49 this, SLOT(downloadUrlObtained(QString,QString)));
46 QObject::connect(&(this->downloader), SIGNAL(downloadCreated(QString,QString)),50 QObject::connect(&(this->downloader), SIGNAL(downloadCreated(QString,QString)),
@@ -70,6 +74,8 @@
7074
71void ClickManager::checkUpdates()75void ClickManager::checkUpdates()
72{76{
77 this->m_model.clear();
78 emit this->modelChanged();
73 this->m_service.getCredentials();79 this->m_service.getCredentials();
74}80}
7581
@@ -105,6 +111,7 @@
105 }111 }
106112
107 emit this->modelChanged();113 emit this->modelChanged();
114 emit this->updateAvailableFound();
108}115}
109116
110void ClickManager::startDownload(QString packagename)117void ClickManager::startDownload(QString packagename)
@@ -116,10 +123,11 @@
116void ClickManager::downloadUrlObtained(QString packagename, QString url)123void ClickManager::downloadUrlObtained(QString packagename, QString url)
117{124{
118 if(this->m_token.isValid()) {125 if(this->m_token.isValid()) {
119 QString authHeader = this->m_token.signUrl(url, QStringLiteral("GET"));126 QString authHeader = this->m_token.signUrl(url, QStringLiteral("HEAD"), true);
120 qDebug() << "Download Url:" << url;127 qDebug() << "Download Url:" << url;
121 this->m_apps[packagename]->setError("");128 Application* app = this->m_apps[packagename];
122 this->downloader.startDownload(packagename, url, authHeader);129 app->setClickUrl(url);
130 this->m_network.getClickToken(app, url, authHeader);
123 } else {131 } else {
124 Application* app = this->m_apps[packagename];132 Application* app = this->m_apps[packagename];
125 app->setError("Invalid User Token");133 app->setError("Invalid User Token");
@@ -135,8 +143,15 @@
135143
136void ClickManager::downloadNotCreated(QString packagename, QString error)144void ClickManager::downloadNotCreated(QString packagename, QString error)
137{145{
146 qDebug() << "Download not creeated";
138 Application* app = this->m_apps[packagename];147 Application* app = this->m_apps[packagename];
139 app->setError(error);148 app->setError(error);
140}149}
141150
151void ClickManager::clickTokenReceived(Application* app, const QString& clickToken)
152{
153 app->setError("");
154 this->downloader.startDownload(app->getPackageName(), app->getClickUrl(), clickToken);
155}
156
142}157}
143158
=== modified file 'src/clickmanager.h'
--- src/clickmanager.h 2013-09-26 11:08:24 +0000
+++ src/clickmanager.h 2013-10-03 11:37:38 +0000
@@ -58,6 +58,8 @@
58 void modelChanged();58 void modelChanged();
59 void updatesNotFound();59 void updatesNotFound();
60 void credentialsNotFound();60 void credentialsNotFound();
61 void updateAvailableFound();
62 void errorFound();
61 63
62public:64public:
63 ClickManager(QQuickItem *parent = 0);65 ClickManager(QQuickItem *parent = 0);
@@ -76,6 +78,7 @@
76 void downloadNotCreated(QString packagename, QString error);78 void downloadNotCreated(QString packagename, QString error);
77 void handleCredentialsFound(Token token);79 void handleCredentialsFound(Token token);
78 void handleCredentialsNotFound();80 void handleCredentialsNotFound();
81 void clickTokenReceived(Application* app, const QString& clickToken);
7982
80private:83private:
81 QHash<QString, Application*> m_apps;84 QHash<QString, Application*> m_apps;
8285
=== modified file 'src/download/downloader.cpp'
--- src/download/downloader.cpp 2013-09-25 20:47:03 +0000
+++ src/download/downloader.cpp 2013-10-03 11:37:38 +0000
@@ -19,6 +19,7 @@
19#include "downloader.h"19#include "downloader.h"
20#include <QVariantMap>20#include <QVariantMap>
21#include "metatypes.h"21#include "metatypes.h"
22#include "network/network.h"
2223
23#define DOWNLOAD_COMMAND "post-download-command"24#define DOWNLOAD_COMMAND "post-download-command"
24#define APP_ID "app_id"25#define APP_ID "app_id"
@@ -35,7 +36,7 @@
35 this->manager = new DownloadManager("com.canonical.applications.Downloader", "/", QDBusConnection::sessionBus(), 0);36 this->manager = new DownloadManager("com.canonical.applications.Downloader", "/", QDBusConnection::sessionBus(), 0);
36}37}
3738
38void Downloader::startDownload(QString& packagename, QString& url, QString& authHeader)39void Downloader::startDownload(QString packagename, const QString& url, const QString& authHeader)
39{40{
40 QVariantMap vmap;41 QVariantMap vmap;
41 QStringList args;42 QStringList args;
@@ -43,7 +44,7 @@
43 vmap[DOWNLOAD_COMMAND] = args;44 vmap[DOWNLOAD_COMMAND] = args;
44 vmap[APP_ID] = packagename;45 vmap[APP_ID] = packagename;
45 StringMap map;46 StringMap map;
46 map["Authorization"] = authHeader;47 map[X_CLICK_TOKEN] = authHeader;
47 DownloadStruct down = DownloadStruct(url, vmap, map);48 DownloadStruct down = DownloadStruct(url, vmap, map);
48 QDBusPendingReply<QDBusObjectPath> reply = this->manager->createDownload(down);49 QDBusPendingReply<QDBusObjectPath> reply = this->manager->createDownload(down);
49 reply.waitForFinished();50 reply.waitForFinished();
5051
=== modified file 'src/download/downloader.h'
--- src/download/downloader.h 2013-09-25 13:07:12 +0000
+++ src/download/downloader.h 2013-10-03 11:37:38 +0000
@@ -32,7 +32,7 @@
32public:32public:
33 explicit Downloader(QObject *parent = 0);33 explicit Downloader(QObject *parent = 0);
3434
35 void startDownload(QString& packagename, QString& url, QString& authHeader);35 void startDownload(QString packagename, const QString& url, const QString& authHeader);
3636
37signals:37signals:
38 void downloadCreated(QString packagename, QString dbuspath);38 void downloadCreated(QString packagename, QString dbuspath);
3939
=== modified file 'src/network/network.cpp'
--- src/network/network.cpp 2013-09-26 16:46:57 +0000
+++ src/network/network.cpp 2013-10-03 11:37:38 +0000
@@ -21,6 +21,7 @@
21#include <QJsonObject>21#include <QJsonObject>
22#include <QJsonArray>22#include <QJsonArray>
23#include <QJsonValue>23#include <QJsonValue>
24#include <QByteArray>
24#include <QUrl>25#include <QUrl>
25#include <QDebug>26#include <QDebug>
2627
@@ -75,16 +76,21 @@
75 qDebug() << "Network::OnReply from " << reply->url()76 qDebug() << "Network::OnReply from " << reply->url()
76 << " status: " << httpStatus;77 << " status: " << httpStatus;
7778
78 QByteArray payload = reply->readAll();
79
80 if (payload.isEmpty()) {
81 emit this->errorOccurred();
82 return;
83 }
84
85 QJsonDocument document = QJsonDocument::fromJson(payload);
86
87 if (httpStatus == 200 || httpStatus == 201) {79 if (httpStatus == 200 || httpStatus == 201) {
80 if (reply->hasRawHeader(X_CLICK_TOKEN)) {
81 Application* app = qobject_cast<Application*>(reply->request().originatingObject());
82 if(app != NULL) {
83 QString header(reply->rawHeader(X_CLICK_TOKEN));
84 emit this->clickTokenObtained(app, header);
85 }
86 reply->deleteLater();
87 return;
88 }
89
90 QByteArray payload = reply->readAll();
91 QJsonDocument document = QJsonDocument::fromJson(payload);
92 reply->deleteLater();
93
88 if (document.isArray()) {94 if (document.isArray()) {
89 QJsonArray array = document.array();95 QJsonArray array = document.array();
90 int i;96 int i;
@@ -123,10 +129,20 @@
123129
124}130}
125131
126void Network::getResourceUrl(QString packagename)132void Network::getResourceUrl(const QString& packagename)
127{133{
128 this->_request->setUrl(QUrl(URL_PACKAGE + packagename));134 this->_request->setUrl(QUrl(URL_PACKAGE + packagename));
129 this->_nam->get(*this->_request);135 this->_nam->get(*this->_request);
130}136}
131137
138void Network::getClickToken(Application* app, const QString& url, const QString& authHeader)
139{
140 QUrl query(url);
141 query.setQuery(authHeader);
142 QNetworkRequest request;
143 request.setUrl(query);
144 request.setOriginatingObject(app);
145 this->_nam->head(request);
146}
147
132}148}
133149
=== modified file 'src/network/network.h'
--- src/network/network.h 2013-09-24 14:23:15 +0000
+++ src/network/network.h 2013-10-03 11:37:38 +0000
@@ -25,6 +25,8 @@
25#include <QHash>25#include <QHash>
26#include "application.h"26#include "application.h"
2727
28#define X_CLICK_TOKEN "X-Click-Token"
29
28namespace ClickPlugin {30namespace ClickPlugin {
2931
30class Network : public QObject32class Network : public QObject
@@ -34,13 +36,15 @@
34 explicit Network(QObject *parent = 0);36 explicit Network(QObject *parent = 0);
3537
36 void checkForNewVersions(QHash<QString, Application*> &apps);38 void checkForNewVersions(QHash<QString, Application*> &apps);
37 void getResourceUrl(QString packagename);39 void getResourceUrl(const QString& packagename);
40 void getClickToken(Application* app, const QString& url, const QString& authHeader);
3841
39signals:42signals:
40 void updatesFound();43 void updatesFound();
41 void updatesNotFound();44 void updatesNotFound();
42 void errorOccurred();45 void errorOccurred();
43 void downloadUrlFound(QString packagename, QString url);46 void downloadUrlFound(QString packagename, QString url);
47 void clickTokenObtained(Application* app, const QString& clickToken);
4448
45private slots:49private slots:
46 void onReply(QNetworkReply*);50 void onReply(QNetworkReply*);
4751
=== modified file 'tests/fakedownloader.cpp'
--- tests/fakedownloader.cpp 2013-09-25 20:47:03 +0000
+++ tests/fakedownloader.cpp 2013-10-03 11:37:38 +0000
@@ -26,7 +26,7 @@
26{26{
27}27}
2828
29void FakeDownloader::startDownload(QString& packagename, QString& url, QString authHeader)29void FakeDownloader::startDownload(QString packagename, QString url, const QString& authHeader)
30{30{
31 if(authHeader.isEmpty()) {31 if(authHeader.isEmpty()) {
32 emit this->downloadNotCreated(packagename, QString("DOWNLOAD ERROR"));32 emit this->downloadNotCreated(packagename, QString("DOWNLOAD ERROR"));
3333
=== modified file 'tests/fakedownloader.h'
--- tests/fakedownloader.h 2013-09-25 20:47:03 +0000
+++ tests/fakedownloader.h 2013-10-03 11:37:38 +0000
@@ -29,7 +29,7 @@
29public:29public:
30 explicit FakeDownloader(QObject *parent = 0);30 explicit FakeDownloader(QObject *parent = 0);
31 31
32 void startDownload(QString& packagename, QString& url, QString authHeader);32 void startDownload(QString packagename, QString url, const QString& authHeader);
3333
34signals:34signals:
35 void downloadCreated(QString packagename, QString dbuspath);35 void downloadCreated(QString packagename, QString dbuspath);
3636
=== modified file 'tests/fakenetwork.cpp'
--- tests/fakenetwork.cpp 2013-09-25 20:47:03 +0000
+++ tests/fakenetwork.cpp 2013-10-03 11:37:38 +0000
@@ -40,4 +40,10 @@
40 emit this->downloadUrlFound(packagename, "http://canonical.com");40 emit this->downloadUrlFound(packagename, "http://canonical.com");
41}41}
4242
43void FakeNetwork::getClickToken(Application* app, const QString& url, const QString& authHeader)
44{
45 QString fakeHeader("x-click-token-header");
46 emit this->clickTokenObtained(app, fakeHeader);
47}
48
43}49}
4450
=== modified file 'tests/fakenetwork.h'
--- tests/fakenetwork.h 2013-09-25 20:47:03 +0000
+++ tests/fakenetwork.h 2013-10-03 11:37:38 +0000
@@ -34,13 +34,14 @@
3434
35 void checkForNewVersions(QHash<QString, Application*> &apps);35 void checkForNewVersions(QHash<QString, Application*> &apps);
36 void getResourceUrl(QString packagename);36 void getResourceUrl(QString packagename);
37 void getClickToken(Application* app, const QString& url, const QString& authHeader);
37 38
38signals:39signals:
39 void updatesFound();40 void updatesFound();
40 void updatesNotFound();41 void updatesNotFound();
41 void errorOccurred();42 void errorOccurred();
42 void downloadUrlFound(QString packagename, QString url);43 void downloadUrlFound(QString packagename, QString url);
43 44 void clickTokenObtained(Application* app, const QString& clickToken);
44};45};
4546
46}47}
4748
=== modified file 'tests/testclickmanager.cpp'
--- tests/testclickmanager.cpp 2013-09-25 20:47:03 +0000
+++ tests/testclickmanager.cpp 2013-10-03 11:37:38 +0000
@@ -33,7 +33,7 @@
33 this->m_signal_counter++;33 this->m_signal_counter++;
34}34}
3535
36void TestClickManager::testCheckUpdates()36void TestClickManager::testCheckUpdatesModelSignal()
37{37{
38 this->m_signal_counter = 0;38 this->m_signal_counter = 0;
39 ClickManager manager;39 ClickManager manager;
@@ -47,6 +47,23 @@
47 QCOMPARE(app->getTitle(), QString("XDA Developers App"));47 QCOMPARE(app->getTitle(), QString("XDA Developers App"));
48 QCOMPARE(app->updateRequired(), true);48 QCOMPARE(app->updateRequired(), true);
49 QCOMPARE(app->getPackageName(), QString("com.ubuntu.developer.xda-app"));49 QCOMPARE(app->getPackageName(), QString("com.ubuntu.developer.xda-app"));
50 QCOMPARE(this->m_signal_counter, 2);
51}
52
53void TestClickManager::testCheckUpdatesUpdateSignal()
54{
55 this->m_signal_counter = 0;
56 ClickManager manager;
57 this->connect(&(manager), SIGNAL(updateAvailableFound()),
58 this, SLOT(slotModelChanged()));
59 QCOMPARE(manager.m_apps.size(), 0);
60 manager.checkUpdates();
61 QCOMPARE(manager.m_apps.size(), 4);
62 QCOMPARE(manager.m_model.size(), 1);
63 Application* app = manager.m_model[0].value<Application*>();
64 QCOMPARE(app->getTitle(), QString("XDA Developers App"));
65 QCOMPARE(app->updateRequired(), true);
66 QCOMPARE(app->getPackageName(), QString("com.ubuntu.developer.xda-app"));
50 QCOMPARE(this->m_signal_counter, 1);67 QCOMPARE(this->m_signal_counter, 1);
51}68}
5269
5370
=== modified file 'tests/testclickmanager.h'
--- tests/testclickmanager.h 2013-09-25 20:47:03 +0000
+++ tests/testclickmanager.h 2013-10-03 11:37:38 +0000
@@ -35,7 +35,8 @@
35private slots:35private slots:
36 void cleanup();36 void cleanup();
3737
38 void testCheckUpdates();38 void testCheckUpdatesModelSignal();
39 void testCheckUpdatesUpdateSignal();
39 void testStartDownload();40 void testStartDownload();
40 void testTokenObtained();41 void testTokenObtained();
41 void testDownloadNotCreated();42 void testDownloadNotCreated();

Subscribers

People subscribed via source and target branches