clickmanager-plugin

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

  1. x-click-token-support
  2. Merge into trunk
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.
lp:~diegosarmentero/clickmanager-plugin/x-click-token-support updated
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.

lp:~diegosarmentero/clickmanager-plugin/x-click-token-support updated
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

lp:~diegosarmentero/clickmanager-plugin/x-click-token-support updated
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 :

PASSED: Continuous integration, rev:30
http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-ci/13/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-amd64-ci/13
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-armhf-ci/13

Click here to trigger a rebuild:
http://s-jenkins:8080/job/clickmanager-plugin-ci/13/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:33
http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-ci/14/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-amd64-ci/14/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-armhf-ci/14/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/clickmanager-plugin-ci/14/rebuild

review: Needs Fixing (continuous-integration)
lp:~diegosarmentero/clickmanager-plugin/x-click-token-support updated
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 :

PASSED: Continuous integration, rev:35
http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-ci/15/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-amd64-ci/15
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-armhf-ci/15

Click here to trigger a rebuild:
http://s-jenkins:8080/job/clickmanager-plugin-ci/15/rebuild

review: Approve (continuous-integration)
lp:~diegosarmentero/clickmanager-plugin/x-click-token-support updated
37. By Diego Sarmentero

forward error signal from network

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:37
http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-ci/16/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-amd64-ci/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-armhf-ci/16

Click here to trigger a rebuild:
http://s-jenkins:8080/job/clickmanager-plugin-ci/16/rebuild

review: Approve (continuous-integration)
lp:~diegosarmentero/clickmanager-plugin/x-click-token-support updated
38. By Diego Sarmentero

removing qcritical message

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:38
http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-ci/17/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-amd64-ci/17
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-armhf-ci/17

Click here to trigger a rebuild:
http://s-jenkins:8080/job/clickmanager-plugin-ci/17/rebuild

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
lp:~diegosarmentero/clickmanager-plugin/x-click-token-support updated
39. By Diego Sarmentero

deleting reply after use

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:39
http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-ci/18/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-amd64-ci/18
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-armhf-ci/18

Click here to trigger a rebuild:
http://s-jenkins:8080/job/clickmanager-plugin-ci/18/rebuild

review: Approve (continuous-integration)
lp:~diegosarmentero/clickmanager-plugin/x-click-token-support updated
40. By Diego Sarmentero

change method signature

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:40
http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-ci/19/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-amd64-ci/19
    SUCCESS: http://jenkins.qa.ubuntu.com/job/clickmanager-plugin-saucy-armhf-ci/19

Click here to trigger a rebuild:
http://s-jenkins:8080/job/clickmanager-plugin-ci/19/rebuild

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
1=== modified file 'src/application.cpp'
2--- src/application.cpp 2013-09-30 14:51:49 +0000
3+++ src/application.cpp 2013-10-03 11:37:38 +0000
4@@ -36,6 +36,7 @@
5 this->m_selected = false;
6 this->m_icon_url = "";
7 this->m_binary_filesize = 0;
8+ this->m_click_url = "";
9 }
10
11 void Application::initializeApplication(QString packagename, QString title, QString version)
12
13=== modified file 'src/application.h'
14--- src/application.h 2013-09-25 13:30:26 +0000
15+++ src/application.h 2013-10-03 11:37:38 +0000
16@@ -65,6 +65,7 @@
17 bool updateState() { return this->m_update_state; }
18 bool selected() { return this->m_selected; }
19 QString getError() { return this->m_error; }
20+ const QString& getClickUrl() const { return this->m_click_url; }
21
22 void initializeApplication(QString packagename, QString title, QString version);
23 void setRemoteVersion(QString& version);
24@@ -74,6 +75,7 @@
25 void setBinaryFilesize(int size) { this->m_binary_filesize = size; emit this->binaryFilesizeChanged(); }
26 void setIconUrl(QString icon) { this->m_icon_url = icon; emit this->iconUrlChanged(); }
27 void setError(QString error);
28+ void setClickUrl(const QString& url) { this->m_click_url = url; }
29
30 private:
31 QString m_packagename;
32@@ -83,6 +85,7 @@
33 QString m_dbuspath;
34 QString m_icon_url;
35 QString m_error;
36+ QString m_click_url;
37 int m_binary_filesize;
38 bool m_update;
39 bool m_update_state;
40
41=== modified file 'src/clickmanager.cpp'
42--- src/clickmanager.cpp 2013-09-25 20:47:03 +0000
43+++ src/clickmanager.cpp 2013-10-03 11:37:38 +0000
44@@ -41,6 +41,10 @@
45 this, SLOT(processUpdates()));
46 QObject::connect(&(this->m_network), SIGNAL(updatesNotFound()),
47 this, SIGNAL(updatesNotFound()));
48+ QObject::connect(&(this->m_network), SIGNAL(errorOccurred()),
49+ this, SIGNAL(errorFound()));
50+ QObject::connect(&(this->m_network), SIGNAL(clickTokenObtained(Application*, const QString&)),
51+ this, SLOT(clickTokenReceived(Application*,const QString&)));
52 QObject::connect(&(this->m_network), SIGNAL(downloadUrlFound(QString,QString)),
53 this, SLOT(downloadUrlObtained(QString,QString)));
54 QObject::connect(&(this->downloader), SIGNAL(downloadCreated(QString,QString)),
55@@ -70,6 +74,8 @@
56
57 void ClickManager::checkUpdates()
58 {
59+ this->m_model.clear();
60+ emit this->modelChanged();
61 this->m_service.getCredentials();
62 }
63
64@@ -105,6 +111,7 @@
65 }
66
67 emit this->modelChanged();
68+ emit this->updateAvailableFound();
69 }
70
71 void ClickManager::startDownload(QString packagename)
72@@ -116,10 +123,11 @@
73 void ClickManager::downloadUrlObtained(QString packagename, QString url)
74 {
75 if(this->m_token.isValid()) {
76- QString authHeader = this->m_token.signUrl(url, QStringLiteral("GET"));
77+ QString authHeader = this->m_token.signUrl(url, QStringLiteral("HEAD"), true);
78 qDebug() << "Download Url:" << url;
79- this->m_apps[packagename]->setError("");
80- this->downloader.startDownload(packagename, url, authHeader);
81+ Application* app = this->m_apps[packagename];
82+ app->setClickUrl(url);
83+ this->m_network.getClickToken(app, url, authHeader);
84 } else {
85 Application* app = this->m_apps[packagename];
86 app->setError("Invalid User Token");
87@@ -135,8 +143,15 @@
88
89 void ClickManager::downloadNotCreated(QString packagename, QString error)
90 {
91+ qDebug() << "Download not creeated";
92 Application* app = this->m_apps[packagename];
93 app->setError(error);
94 }
95
96+void ClickManager::clickTokenReceived(Application* app, const QString& clickToken)
97+{
98+ app->setError("");
99+ this->downloader.startDownload(app->getPackageName(), app->getClickUrl(), clickToken);
100+}
101+
102 }
103
104=== modified file 'src/clickmanager.h'
105--- src/clickmanager.h 2013-09-26 11:08:24 +0000
106+++ src/clickmanager.h 2013-10-03 11:37:38 +0000
107@@ -58,6 +58,8 @@
108 void modelChanged();
109 void updatesNotFound();
110 void credentialsNotFound();
111+ void updateAvailableFound();
112+ void errorFound();
113
114 public:
115 ClickManager(QQuickItem *parent = 0);
116@@ -76,6 +78,7 @@
117 void downloadNotCreated(QString packagename, QString error);
118 void handleCredentialsFound(Token token);
119 void handleCredentialsNotFound();
120+ void clickTokenReceived(Application* app, const QString& clickToken);
121
122 private:
123 QHash<QString, Application*> m_apps;
124
125=== modified file 'src/download/downloader.cpp'
126--- src/download/downloader.cpp 2013-09-25 20:47:03 +0000
127+++ src/download/downloader.cpp 2013-10-03 11:37:38 +0000
128@@ -19,6 +19,7 @@
129 #include "downloader.h"
130 #include <QVariantMap>
131 #include "metatypes.h"
132+#include "network/network.h"
133
134 #define DOWNLOAD_COMMAND "post-download-command"
135 #define APP_ID "app_id"
136@@ -35,7 +36,7 @@
137 this->manager = new DownloadManager("com.canonical.applications.Downloader", "/", QDBusConnection::sessionBus(), 0);
138 }
139
140-void Downloader::startDownload(QString& packagename, QString& url, QString& authHeader)
141+void Downloader::startDownload(QString packagename, const QString& url, const QString& authHeader)
142 {
143 QVariantMap vmap;
144 QStringList args;
145@@ -43,7 +44,7 @@
146 vmap[DOWNLOAD_COMMAND] = args;
147 vmap[APP_ID] = packagename;
148 StringMap map;
149- map["Authorization"] = authHeader;
150+ map[X_CLICK_TOKEN] = authHeader;
151 DownloadStruct down = DownloadStruct(url, vmap, map);
152 QDBusPendingReply<QDBusObjectPath> reply = this->manager->createDownload(down);
153 reply.waitForFinished();
154
155=== modified file 'src/download/downloader.h'
156--- src/download/downloader.h 2013-09-25 13:07:12 +0000
157+++ src/download/downloader.h 2013-10-03 11:37:38 +0000
158@@ -32,7 +32,7 @@
159 public:
160 explicit Downloader(QObject *parent = 0);
161
162- void startDownload(QString& packagename, QString& url, QString& authHeader);
163+ void startDownload(QString packagename, const QString& url, const QString& authHeader);
164
165 signals:
166 void downloadCreated(QString packagename, QString dbuspath);
167
168=== modified file 'src/network/network.cpp'
169--- src/network/network.cpp 2013-09-26 16:46:57 +0000
170+++ src/network/network.cpp 2013-10-03 11:37:38 +0000
171@@ -21,6 +21,7 @@
172 #include <QJsonObject>
173 #include <QJsonArray>
174 #include <QJsonValue>
175+#include <QByteArray>
176 #include <QUrl>
177 #include <QDebug>
178
179@@ -75,16 +76,21 @@
180 qDebug() << "Network::OnReply from " << reply->url()
181 << " status: " << httpStatus;
182
183- QByteArray payload = reply->readAll();
184-
185- if (payload.isEmpty()) {
186- emit this->errorOccurred();
187- return;
188- }
189-
190- QJsonDocument document = QJsonDocument::fromJson(payload);
191-
192 if (httpStatus == 200 || httpStatus == 201) {
193+ if (reply->hasRawHeader(X_CLICK_TOKEN)) {
194+ Application* app = qobject_cast<Application*>(reply->request().originatingObject());
195+ if(app != NULL) {
196+ QString header(reply->rawHeader(X_CLICK_TOKEN));
197+ emit this->clickTokenObtained(app, header);
198+ }
199+ reply->deleteLater();
200+ return;
201+ }
202+
203+ QByteArray payload = reply->readAll();
204+ QJsonDocument document = QJsonDocument::fromJson(payload);
205+ reply->deleteLater();
206+
207 if (document.isArray()) {
208 QJsonArray array = document.array();
209 int i;
210@@ -123,10 +129,20 @@
211
212 }
213
214-void Network::getResourceUrl(QString packagename)
215+void Network::getResourceUrl(const QString& packagename)
216 {
217 this->_request->setUrl(QUrl(URL_PACKAGE + packagename));
218 this->_nam->get(*this->_request);
219 }
220
221+void Network::getClickToken(Application* app, const QString& url, const QString& authHeader)
222+{
223+ QUrl query(url);
224+ query.setQuery(authHeader);
225+ QNetworkRequest request;
226+ request.setUrl(query);
227+ request.setOriginatingObject(app);
228+ this->_nam->head(request);
229+}
230+
231 }
232
233=== modified file 'src/network/network.h'
234--- src/network/network.h 2013-09-24 14:23:15 +0000
235+++ src/network/network.h 2013-10-03 11:37:38 +0000
236@@ -25,6 +25,8 @@
237 #include <QHash>
238 #include "application.h"
239
240+#define X_CLICK_TOKEN "X-Click-Token"
241+
242 namespace ClickPlugin {
243
244 class Network : public QObject
245@@ -34,13 +36,15 @@
246 explicit Network(QObject *parent = 0);
247
248 void checkForNewVersions(QHash<QString, Application*> &apps);
249- void getResourceUrl(QString packagename);
250+ void getResourceUrl(const QString& packagename);
251+ void getClickToken(Application* app, const QString& url, const QString& authHeader);
252
253 signals:
254 void updatesFound();
255 void updatesNotFound();
256 void errorOccurred();
257 void downloadUrlFound(QString packagename, QString url);
258+ void clickTokenObtained(Application* app, const QString& clickToken);
259
260 private slots:
261 void onReply(QNetworkReply*);
262
263=== modified file 'tests/fakedownloader.cpp'
264--- tests/fakedownloader.cpp 2013-09-25 20:47:03 +0000
265+++ tests/fakedownloader.cpp 2013-10-03 11:37:38 +0000
266@@ -26,7 +26,7 @@
267 {
268 }
269
270-void FakeDownloader::startDownload(QString& packagename, QString& url, QString authHeader)
271+void FakeDownloader::startDownload(QString packagename, QString url, const QString& authHeader)
272 {
273 if(authHeader.isEmpty()) {
274 emit this->downloadNotCreated(packagename, QString("DOWNLOAD ERROR"));
275
276=== modified file 'tests/fakedownloader.h'
277--- tests/fakedownloader.h 2013-09-25 20:47:03 +0000
278+++ tests/fakedownloader.h 2013-10-03 11:37:38 +0000
279@@ -29,7 +29,7 @@
280 public:
281 explicit FakeDownloader(QObject *parent = 0);
282
283- void startDownload(QString& packagename, QString& url, QString authHeader);
284+ void startDownload(QString packagename, QString url, const QString& authHeader);
285
286 signals:
287 void downloadCreated(QString packagename, QString dbuspath);
288
289=== modified file 'tests/fakenetwork.cpp'
290--- tests/fakenetwork.cpp 2013-09-25 20:47:03 +0000
291+++ tests/fakenetwork.cpp 2013-10-03 11:37:38 +0000
292@@ -40,4 +40,10 @@
293 emit this->downloadUrlFound(packagename, "http://canonical.com");
294 }
295
296+void FakeNetwork::getClickToken(Application* app, const QString& url, const QString& authHeader)
297+{
298+ QString fakeHeader("x-click-token-header");
299+ emit this->clickTokenObtained(app, fakeHeader);
300+}
301+
302 }
303
304=== modified file 'tests/fakenetwork.h'
305--- tests/fakenetwork.h 2013-09-25 20:47:03 +0000
306+++ tests/fakenetwork.h 2013-10-03 11:37:38 +0000
307@@ -34,13 +34,14 @@
308
309 void checkForNewVersions(QHash<QString, Application*> &apps);
310 void getResourceUrl(QString packagename);
311+ void getClickToken(Application* app, const QString& url, const QString& authHeader);
312
313 signals:
314 void updatesFound();
315 void updatesNotFound();
316 void errorOccurred();
317 void downloadUrlFound(QString packagename, QString url);
318-
319+ void clickTokenObtained(Application* app, const QString& clickToken);
320 };
321
322 }
323
324=== modified file 'tests/testclickmanager.cpp'
325--- tests/testclickmanager.cpp 2013-09-25 20:47:03 +0000
326+++ tests/testclickmanager.cpp 2013-10-03 11:37:38 +0000
327@@ -33,7 +33,7 @@
328 this->m_signal_counter++;
329 }
330
331-void TestClickManager::testCheckUpdates()
332+void TestClickManager::testCheckUpdatesModelSignal()
333 {
334 this->m_signal_counter = 0;
335 ClickManager manager;
336@@ -47,6 +47,23 @@
337 QCOMPARE(app->getTitle(), QString("XDA Developers App"));
338 QCOMPARE(app->updateRequired(), true);
339 QCOMPARE(app->getPackageName(), QString("com.ubuntu.developer.xda-app"));
340+ QCOMPARE(this->m_signal_counter, 2);
341+}
342+
343+void TestClickManager::testCheckUpdatesUpdateSignal()
344+{
345+ this->m_signal_counter = 0;
346+ ClickManager manager;
347+ this->connect(&(manager), SIGNAL(updateAvailableFound()),
348+ this, SLOT(slotModelChanged()));
349+ QCOMPARE(manager.m_apps.size(), 0);
350+ manager.checkUpdates();
351+ QCOMPARE(manager.m_apps.size(), 4);
352+ QCOMPARE(manager.m_model.size(), 1);
353+ Application* app = manager.m_model[0].value<Application*>();
354+ QCOMPARE(app->getTitle(), QString("XDA Developers App"));
355+ QCOMPARE(app->updateRequired(), true);
356+ QCOMPARE(app->getPackageName(), QString("com.ubuntu.developer.xda-app"));
357 QCOMPARE(this->m_signal_counter, 1);
358 }
359
360
361=== modified file 'tests/testclickmanager.h'
362--- tests/testclickmanager.h 2013-09-25 20:47:03 +0000
363+++ tests/testclickmanager.h 2013-10-03 11:37:38 +0000
364@@ -35,7 +35,8 @@
365 private slots:
366 void cleanup();
367
368- void testCheckUpdates();
369+ void testCheckUpdatesModelSignal();
370+ void testCheckUpdatesUpdateSignal();
371 void testStartDownload();
372 void testTokenObtained();
373 void testDownloadNotCreated();

Subscribers

People subscribed via source and target branches