unity-scope-click

Merge lp:~stolowski/unity-scope-click/network-cache into lp:unity-scope-click/devel

  1. network-cache
  2. Merge into devel
Proposed by Paweł Stołowski
Status: Merged
Approved by: Alejandro J. Cura
Approved revision: 397
Merged at revision: 394
Proposed branch: lp:~stolowski/unity-scope-click/network-cache
Merge into: lp:unity-scope-click/devel
Diff against target: 119 lines (+22/-10)
3 files modified
libclickscope/click/network_access_manager.cpp (+8/-0)
libclickscope/click/webclient.cpp (+7/-3)
libclickscope/tests/test_webclient.cpp (+7/-7)
To merge this branch: bzr merge lp:~stolowski/unity-scope-click/network-cache
Reviewer Review Type Date Requested Status
Michał Sawicz Needs Information
Alejandro J. Cura (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+230332@code.launchpad.net

Commit message

Enable QNetworkDiskCache for http GET requests.

Description of the change

Enable QNetworkDiskCache for http GET requests.

To post a comment you must log in.
lp:~stolowski/unity-scope-click/network-cache updated
397. By Paweł Stołowski

Updated tests.

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

Code looks good; now I desperately want jenkins to hurry up and give me a package to test the massive improvement in responsiveness that this MP promises ;-)

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

PASSED: Continuous integration, rev:396
http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-ci/259/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-amd64-ci/234
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/233
        deb: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-armhf-ci/233/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-team-unity-scope-click-devel-utopic-i386-ci/233

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

review: Approve (continuous-integration)
Revision history for this message
Alejandro J. Cura (alecu) wrote :

After testing on the device, I'm very happy with the results.

review: Approve
Revision history for this message
Michał Sawicz (saviq) wrote :

Would it make sense to check for network and do AlwaysCache when offline? Otherwise this will try and call up to the server anyway?

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

On Mon, Aug 11, 2014 at 7:34 PM, Michał Sawicz
<email address hidden> wrote:
> Would it make sense to check for network and do AlwaysCache when offline? Otherwise this will try and call up to the server anyway?

Good point, we should definitely investigate the behavior when
offline, and also we should check if the cached entries are skipped if
the language is changed.
I don't think though that those points are blockers for landing this branch.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'libclickscope/click/network_access_manager.cpp'
--- libclickscope/click/network_access_manager.cpp 2014-05-20 19:33:41 +0000
+++ libclickscope/click/network_access_manager.cpp 2014-08-11 15:38:29 +0000
@@ -28,6 +28,8 @@
28 */28 */
2929
30#include "network_access_manager.h"30#include "network_access_manager.h"
31#include <QNetworkDiskCache>
32#include <QStandardPaths>
3133
32click::network::Reply::Reply(QNetworkReply* reply) : reply(reply)34click::network::Reply::Reply(QNetworkReply* reply) : reply(reply)
33{35{
@@ -91,6 +93,12 @@
91QNetworkAccessManager& networkAccessManagerInstance()93QNetworkAccessManager& networkAccessManagerInstance()
92{94{
93 static QNetworkAccessManager nam;95 static QNetworkAccessManager nam;
96 if (!nam.cache())
97 {
98 QNetworkDiskCache* cache = new QNetworkDiskCache(&nam);
99 cache->setCacheDirectory(QString("%1/unity-scope-click/network").arg(QStandardPaths::writableLocation(QStandardPaths::CacheLocation)));
100 nam.setCache(cache);
101 }
94 return nam;102 return nam;
95}103}
96}104}
97105
=== modified file 'libclickscope/click/webclient.cpp'
--- libclickscope/click/webclient.cpp 2014-06-17 13:56:26 +0000
+++ libclickscope/click/webclient.cpp 2014-08-11 15:38:29 +0000
@@ -99,6 +99,8 @@
99 request->setRawHeader(ACCEPT_LANGUAGE_HEADER.c_str(),99 request->setRawHeader(ACCEPT_LANGUAGE_HEADER.c_str(),
100 Configuration().get_accept_languages().c_str());100 Configuration().get_accept_languages().c_str());
101101
102 request->setAttribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::PreferCache);
103
102 for (const auto& kv : headers) {104 for (const auto& kv : headers) {
103 QByteArray header_name(kv.first.c_str(), kv.first.length());105 QByteArray header_name(kv.first.c_str(), kv.first.length());
104 QByteArray header_value(kv.second.c_str(), kv.second.length());106 QByteArray header_value(kv.second.c_str(), kv.second.length());
@@ -109,9 +111,11 @@
109111
110 auto doConnect = [=]() {112 auto doConnect = [=]() {
111 QByteArray verb(method.c_str(), method.length());113 QByteArray verb(method.c_str(), method.length());
112 auto reply = impl->network_access_manager->sendCustomRequest(*request,114 //
113 verb,115 // for 'get' use get method of access manager explicitly as sendCustomRequest disables the use of cache.
114 buffer.data());116 auto reply = (method == "GET" && buffer->size() == 0) ?
117 impl->network_access_manager->get(*request) :
118 impl->network_access_manager->sendCustomRequest(*request, verb, buffer.data());
115 responsePtr->setReply(reply);119 responsePtr->setReply(reply);
116 };120 };
117121
118122
=== modified file 'libclickscope/tests/test_webclient.cpp'
--- libclickscope/tests/test_webclient.cpp 2014-05-23 18:45:01 +0000
+++ libclickscope/tests/test_webclient.cpp 2014-08-11 15:38:29 +0000
@@ -95,7 +95,7 @@
9595
96 click::web::Client wc(namPtr);96 click::web::Client wc(namPtr);
9797
98 EXPECT_CALL(nam, sendCustomRequest(IsCorrectUrl(QString("http://fake-server/fake/api/path")), _, _))98 EXPECT_CALL(nam, get(IsCorrectUrl(QString("http://fake-server/fake/api/path"))))
99 .Times(1)99 .Times(1)
100 .WillOnce(Return(replyPtr));100 .WillOnce(Return(replyPtr));
101101
@@ -116,7 +116,7 @@
116 params.add("a", "1");116 params.add("a", "1");
117 params.add("b", "2");117 params.add("b", "2");
118118
119 EXPECT_CALL(nam, sendCustomRequest(IsCorrectUrl(QString("http://fake-server/fake/api/path?a=1&b=2")), _, _))119 EXPECT_CALL(nam, get(IsCorrectUrl(QString("http://fake-server/fake/api/path?a=1&b=2"))))
120 .Times(1)120 .Times(1)
121 .WillOnce(Return(replyPtr));121 .WillOnce(Return(replyPtr));
122122
@@ -163,7 +163,7 @@
163163
164 click::web::Client wc(namPtr);164 click::web::Client wc(namPtr);
165165
166 EXPECT_CALL(nam, sendCustomRequest(IsCorrectCookieHeader("CookieCookieCookie"), _, _))166 EXPECT_CALL(nam, get(IsCorrectCookieHeader("CookieCookieCookie")))
167 .Times(1)167 .Times(1)
168 .WillOnce(Return(replyPtr));168 .WillOnce(Return(replyPtr));
169169
@@ -287,7 +287,7 @@
287287
288 click::web::Client wc(namPtr);288 click::web::Client wc(namPtr);
289289
290 EXPECT_CALL(nam, sendCustomRequest(_, _, _))290 EXPECT_CALL(nam, get(_))
291 .Times(1)291 .Times(1)
292 .WillOnce(Return(replyPtr));292 .WillOnce(Return(replyPtr));
293293
@@ -306,7 +306,7 @@
306306
307 click::web::Client wc(namPtr);307 click::web::Client wc(namPtr);
308308
309 EXPECT_CALL(nam, sendCustomRequest(_, _, _))309 EXPECT_CALL(nam, get(_))
310 .Times(1)310 .Times(1)
311 .WillOnce(Return(replyPtr));311 .WillOnce(Return(replyPtr));
312 EXPECT_CALL(*reply, errorString())312 EXPECT_CALL(*reply, errorString())
@@ -335,7 +335,7 @@
335335
336 click::web::Client wc(namPtr);336 click::web::Client wc(namPtr);
337337
338 EXPECT_CALL(nam, sendCustomRequest(_, _, _))338 EXPECT_CALL(nam, get(_))
339 .Times(1)339 .Times(1)
340 .WillOnce(Return(replyPtr));340 .WillOnce(Return(replyPtr));
341341
@@ -357,7 +357,7 @@
357357
358 ASSERT_EQ(setenv(click::Configuration::LANGUAGE_ENVVAR,358 ASSERT_EQ(setenv(click::Configuration::LANGUAGE_ENVVAR,
359 "en_US.UTF-8", 1), 0);359 "en_US.UTF-8", 1), 0);
360 EXPECT_CALL(nam, sendCustomRequest(IsCorrectAcceptLanguageHeader("en-US, en"), _, _))360 EXPECT_CALL(nam, get(IsCorrectAcceptLanguageHeader("en-US, en")))
361 .Times(1)361 .Times(1)
362 .WillOnce(Return(replyPtr));362 .WillOnce(Return(replyPtr));
363363

Subscribers

People subscribed via source and target branches

to all changes: