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
1=== modified file 'libclickscope/click/network_access_manager.cpp'
2--- libclickscope/click/network_access_manager.cpp 2014-05-20 19:33:41 +0000
3+++ libclickscope/click/network_access_manager.cpp 2014-08-11 15:38:29 +0000
4@@ -28,6 +28,8 @@
5 */
6
7 #include "network_access_manager.h"
8+#include <QNetworkDiskCache>
9+#include <QStandardPaths>
10
11 click::network::Reply::Reply(QNetworkReply* reply) : reply(reply)
12 {
13@@ -91,6 +93,12 @@
14 QNetworkAccessManager& networkAccessManagerInstance()
15 {
16 static QNetworkAccessManager nam;
17+ if (!nam.cache())
18+ {
19+ QNetworkDiskCache* cache = new QNetworkDiskCache(&nam);
20+ cache->setCacheDirectory(QString("%1/unity-scope-click/network").arg(QStandardPaths::writableLocation(QStandardPaths::CacheLocation)));
21+ nam.setCache(cache);
22+ }
23 return nam;
24 }
25 }
26
27=== modified file 'libclickscope/click/webclient.cpp'
28--- libclickscope/click/webclient.cpp 2014-06-17 13:56:26 +0000
29+++ libclickscope/click/webclient.cpp 2014-08-11 15:38:29 +0000
30@@ -99,6 +99,8 @@
31 request->setRawHeader(ACCEPT_LANGUAGE_HEADER.c_str(),
32 Configuration().get_accept_languages().c_str());
33
34+ request->setAttribute(QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::PreferCache);
35+
36 for (const auto& kv : headers) {
37 QByteArray header_name(kv.first.c_str(), kv.first.length());
38 QByteArray header_value(kv.second.c_str(), kv.second.length());
39@@ -109,9 +111,11 @@
40
41 auto doConnect = [=]() {
42 QByteArray verb(method.c_str(), method.length());
43- auto reply = impl->network_access_manager->sendCustomRequest(*request,
44- verb,
45- buffer.data());
46+ //
47+ // for 'get' use get method of access manager explicitly as sendCustomRequest disables the use of cache.
48+ auto reply = (method == "GET" && buffer->size() == 0) ?
49+ impl->network_access_manager->get(*request) :
50+ impl->network_access_manager->sendCustomRequest(*request, verb, buffer.data());
51 responsePtr->setReply(reply);
52 };
53
54
55=== modified file 'libclickscope/tests/test_webclient.cpp'
56--- libclickscope/tests/test_webclient.cpp 2014-05-23 18:45:01 +0000
57+++ libclickscope/tests/test_webclient.cpp 2014-08-11 15:38:29 +0000
58@@ -95,7 +95,7 @@
59
60 click::web::Client wc(namPtr);
61
62- EXPECT_CALL(nam, sendCustomRequest(IsCorrectUrl(QString("http://fake-server/fake/api/path")), _, _))
63+ EXPECT_CALL(nam, get(IsCorrectUrl(QString("http://fake-server/fake/api/path"))))
64 .Times(1)
65 .WillOnce(Return(replyPtr));
66
67@@ -116,7 +116,7 @@
68 params.add("a", "1");
69 params.add("b", "2");
70
71- EXPECT_CALL(nam, sendCustomRequest(IsCorrectUrl(QString("http://fake-server/fake/api/path?a=1&b=2")), _, _))
72+ EXPECT_CALL(nam, get(IsCorrectUrl(QString("http://fake-server/fake/api/path?a=1&b=2"))))
73 .Times(1)
74 .WillOnce(Return(replyPtr));
75
76@@ -163,7 +163,7 @@
77
78 click::web::Client wc(namPtr);
79
80- EXPECT_CALL(nam, sendCustomRequest(IsCorrectCookieHeader("CookieCookieCookie"), _, _))
81+ EXPECT_CALL(nam, get(IsCorrectCookieHeader("CookieCookieCookie")))
82 .Times(1)
83 .WillOnce(Return(replyPtr));
84
85@@ -287,7 +287,7 @@
86
87 click::web::Client wc(namPtr);
88
89- EXPECT_CALL(nam, sendCustomRequest(_, _, _))
90+ EXPECT_CALL(nam, get(_))
91 .Times(1)
92 .WillOnce(Return(replyPtr));
93
94@@ -306,7 +306,7 @@
95
96 click::web::Client wc(namPtr);
97
98- EXPECT_CALL(nam, sendCustomRequest(_, _, _))
99+ EXPECT_CALL(nam, get(_))
100 .Times(1)
101 .WillOnce(Return(replyPtr));
102 EXPECT_CALL(*reply, errorString())
103@@ -335,7 +335,7 @@
104
105 click::web::Client wc(namPtr);
106
107- EXPECT_CALL(nam, sendCustomRequest(_, _, _))
108+ EXPECT_CALL(nam, get(_))
109 .Times(1)
110 .WillOnce(Return(replyPtr));
111
112@@ -357,7 +357,7 @@
113
114 ASSERT_EQ(setenv(click::Configuration::LANGUAGE_ENVVAR,
115 "en_US.UTF-8", 1), 0);
116- EXPECT_CALL(nam, sendCustomRequest(IsCorrectAcceptLanguageHeader("en-US, en"), _, _))
117+ EXPECT_CALL(nam, get(IsCorrectAcceptLanguageHeader("en-US, en")))
118 .Times(1)
119 .WillOnce(Return(replyPtr));
120

Subscribers

People subscribed via source and target branches

to all changes: