notes-app

Merge lp:~osomon/notes-app/provider-cleanup into lp:notes-app

  1. provider-cleanup
  2. Merge into trunk
Proposed by Olivier Tilloy
Status: Merged
Approved by: Olivier Tilloy
Approved revision: 155
Merged at revision: 157
Proposed branch: lp:~osomon/notes-app/provider-cleanup
Merge into: lp:notes-app
Diff against target: 187 lines (+38/-21)
3 files modified
src/NotesApp/Plugins/cachingprovider.cpp (+13/-8)
tests/unittests/image.h (+9/-1)
tests/unittests/tst_CachingProviderTests.cpp (+16/-12)
To merge this branch: bzr merge lp:~osomon/notes-app/provider-cleanup
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Ugo Riboni (community) Approve
Review via email: mp+174143@code.launchpad.net

Commit message

Various minor fixes and cleanup of the caching image provider.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:154
http://jenkins.qa.ubuntu.com/job/notes-app-ci/115/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/995
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notes-app-saucy-amd64-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notes-app-saucy-armhf-ci/10
        deb: http://jenkins.qa.ubuntu.com/job/notes-app-saucy-armhf-ci/10/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notes-app-saucy-i386-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy/999
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy/999/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-saucy/851

Click here to trigger a rebuild:
http://s-jenkins:8080/job/notes-app-ci/115/rebuild

review: Approve (continuous-integration)
lp:~osomon/notes-app/provider-cleanup updated
155. By Olivier Tilloy

Merge the latest changes from trunk.

Revision history for this message
Ugo Riboni (uriboni) wrote :

The code looks ok for me. If jenkins approves and tests pass on the device, feel free to top approve

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

PASSED: Continuous integration, rev:155
http://jenkins.qa.ubuntu.com/job/notes-app-ci/125/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-saucy/1496
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notes-app-saucy-amd64-ci/20
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notes-app-saucy-armhf-ci/20
        deb: http://jenkins.qa.ubuntu.com/job/notes-app-saucy-armhf-ci/20/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notes-app-saucy-i386-ci/20
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy/1500
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-saucy/1500/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-saucy/1270

Click here to trigger a rebuild:
http://s-jenkins:8080/job/notes-app-ci/125/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/NotesApp/Plugins/cachingprovider.cpp'
2--- src/NotesApp/Plugins/cachingprovider.cpp 2013-06-10 11:48:01 +0000
3+++ src/NotesApp/Plugins/cachingprovider.cpp 2013-07-24 14:53:25 +0000
4@@ -17,7 +17,7 @@
5 #include "cachingprovider.h"
6
7 #include <QDebug>
8-#include <QFile>
9+#include <QFileInfo>
10 #include <QEventLoop>
11 #include <QImageReader>
12 #include <QNetworkRequest>
13@@ -89,7 +89,7 @@
14
15 QImage CachingProvider::requestImage(const QString &id, QSize *size, const QSize& requestedSize)
16 {
17- QUrl realUrl = QUrl(QString(id).replace("image://cache/", ""));
18+ QUrl url(id);
19 bool canCache = true;
20
21 QDir dataLocation(QStandardPaths::writableLocation(QStandardPaths::DataLocation));
22@@ -100,8 +100,8 @@
23 }
24 }
25
26- QString hash = QString(QCryptographicHash::hash(realUrl.toString().toUtf8(), QCryptographicHash::Md5).toHex());
27- QString extension = QFileInfo(realUrl.path()).suffix();
28+ QString hash = QString(QCryptographicHash::hash(url.toString().toUtf8(), QCryptographicHash::Md5).toHex());
29+ QString extension = QFileInfo(url.path()).suffix();
30 QFileInfo cacheFile = dataLocation.filePath(hash + "." + extension);
31
32 QImage image;
33@@ -115,10 +115,10 @@
34 } else needDownload = true;
35
36 if (needDownload) {
37- if (realUrl.scheme() != "file") {
38- image = downloadImage(realUrl);
39+ if (url.scheme() != "file") {
40+ image = downloadImage(url);
41 } else {
42- image.load(realUrl.path());
43+ image.load(url.path());
44 }
45
46 if (canCache && !image.isNull()) {
47@@ -128,5 +128,10 @@
48 }
49 }
50
51- return image;
52+ *size = image.size();
53+ if (!image.isNull() && requestedSize.isValid() && (image.size() != requestedSize)) {
54+ return image.scaled(requestedSize, Qt::IgnoreAspectRatio, Qt::SmoothTransformation);
55+ } else {
56+ return image;
57+ }
58 }
59
60=== modified file 'tests/unittests/image.h'
61--- tests/unittests/image.h 2013-05-14 15:46:59 +0000
62+++ tests/unittests/image.h 2013-07-24 14:53:25 +0000
63@@ -14,6 +14,11 @@
64 * along with this program. If not, see <http://www.gnu.org/licenses/>.
65 */
66
67+#ifndef __IMAGE_H__
68+#define __IMAGE_H__
69+
70+#include <QSize>
71+
72 const unsigned char image_png[] = {
73 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d,
74 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01,
75@@ -29,4 +34,7 @@
76 0x59, 0xe7, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42,
77 0x60, 0x82
78 };
79-unsigned int image_png_len = 146;
80+const unsigned int image_png_len = 146;
81+const QSize image_png_size(1, 1);
82+
83+#endif // __IMAGE_H__
84
85=== modified file 'tests/unittests/tst_CachingProviderTests.cpp'
86--- tests/unittests/tst_CachingProviderTests.cpp 2013-05-15 12:50:49 +0000
87+++ tests/unittests/tst_CachingProviderTests.cpp 2013-07-24 14:53:25 +0000
88@@ -15,17 +15,15 @@
89 */
90
91 #include <QtTest>
92-#include <QProcess>
93-#include <QStringList>
94-#include <QImage>
95+#include <QObject>
96 #include <QDir>
97 #include <QStandardPaths>
98 #include <QNetworkAccessManager>
99
100 #include "cachingprovider.h"
101+#include "image.h"
102
103 #define BASEURL "http://localhost:8888"
104-#define BASECACHEURL "image://cache/" BASEURL
105
106 class CachingProviderTests : public QObject
107 {
108@@ -58,11 +56,12 @@
109
110 CachingProvider provider;
111 QSize size;
112- QImage image = provider.requestImage(QString(BASECACHEURL "%1").arg(path), &size, size);
113+ QImage image = provider.requestImage(QString(BASEURL "%1").arg(path), &size, size);
114
115 /* Verify that the image we get back is good and that the file was cached with the
116 * correct hash for the URL we requested. */
117 QVERIFY(!image.isNull());
118+ QCOMPARE(size, image_png_size);
119 QCOMPARE(cache.entryList().count(), 1);
120 QCOMPARE(cache.entryList().first(), QString("2c9fc02d059d33853dff5622ee332ab4.png"));
121 QCOMPARE(QNetworkAccessManager::log().count(), 1);
122@@ -77,9 +76,10 @@
123 QSize size;
124
125 /* Verify that the second time we request the same image we get something valid back */
126- provider.requestImage(QString(BASECACHEURL "%1").arg(path), &size, size);
127- QImage image = provider.requestImage(QString(BASECACHEURL "%1").arg(path), &size, size);
128+ provider.requestImage(QString(BASEURL "%1").arg(path), &size, size);
129+ QImage image = provider.requestImage(QString(BASEURL "%1").arg(path), &size, size);
130 QVERIFY(!image.isNull());
131+ QCOMPARE(size, image_png_size);
132
133 /* Verify that the server only received the first request (as the second one came
134 * from the cache) and not two of them */
135@@ -95,8 +95,9 @@
136 QSize size;
137
138 // Verify that we get a valid image back
139- QImage image = provider.requestImage(QString(BASECACHEURL "%1").arg(path), &size, size);
140+ QImage image = provider.requestImage(QString(BASEURL "%1").arg(path), &size, size);
141 QVERIFY(!image.isNull());
142+ QCOMPARE(size, image_png_size);
143 QCOMPARE(QNetworkAccessManager::log().count(), 4);
144 QCOMPARE(QNetworkAccessManager::log().first(), path);
145 QCOMPARE(QNetworkAccessManager::log().last(), QString("/image.png"));
146@@ -104,8 +105,9 @@
147
148 // Verify that the second time we request the same image it is fullfilled directly from
149 // the cache and thus the server log doesn't change at all
150- image = provider.requestImage(QString(BASECACHEURL "%1").arg(path), &size, size);
151+ image = provider.requestImage(QString(BASEURL "%1").arg(path), &size, size);
152 QVERIFY(!image.isNull());
153+ QCOMPARE(size, image_png_size);
154 QCOMPARE(QNetworkAccessManager::log().count(), 4);
155 QCOMPARE(QNetworkAccessManager::log().first(), path);
156 QCOMPARE(QNetworkAccessManager::log().last(), QString("/image.png"));
157@@ -118,8 +120,9 @@
158 CachingProvider provider;
159 QSize size;
160
161- QImage image = provider.requestImage(QString(BASECACHEURL "%1").arg(path), &size, size);
162+ QImage image = provider.requestImage(QString(BASEURL "%1").arg(path), &size, size);
163 QVERIFY(image.isNull());
164+ QVERIFY(size.isNull());
165 QCOMPARE(cache.entryList().count(), 0);
166 }
167
168@@ -129,7 +132,7 @@
169
170 CachingProvider provider;
171 QSize size;
172- QImage image = provider.requestImage(QString(BASECACHEURL "%1").arg(path), &size, size);
173+ QImage image = provider.requestImage(QString(BASEURL "%1").arg(path), &size, size);
174
175 /* Corrupt the image in the cache so that it will fail to load at next request */
176 QFile cachedFile;
177@@ -140,8 +143,9 @@
178 QNetworkAccessManager::log().clear();
179
180 /* Verify that the corrupt image will be downloaded and re-cached */
181- image = provider.requestImage(QString(BASECACHEURL "%1").arg(path), &size, size);
182+ image = provider.requestImage(QString(BASEURL "%1").arg(path), &size, size);
183 QVERIFY(!image.isNull());
184+ QCOMPARE(size, image_png_size);
185 QCOMPARE(QNetworkAccessManager::log().count(), 1);
186 QCOMPARE(QNetworkAccessManager::log().first(), path);
187 QCOMPARE(cache.entryList().first(), QString("2c9fc02d059d33853dff5622ee332ab4.png"));

Subscribers

People subscribed via source and target branches