ubuntu-download-manager

Merge lp:~mandel/ubuntu-download-manager/correct-logging into lp:ubuntu-download-manager

  1. correct-logging
  2. Merge into trunk
Proposed by Manuel de la Peña
Status: Merged
Approved by: Alejandro J. Cura
Approved revision: 155
Merged at revision: 152
Proposed branch: lp:~mandel/ubuntu-download-manager/correct-logging
Merge into: lp:ubuntu-download-manager
Diff against target: 819 lines (+101/-111)
15 files modified
libubuntudownloadmanager/apparmor.cpp (+5/-5)
libubuntudownloadmanager/application.cpp (+1/-2)
libubuntudownloadmanager/download.cpp (+3/-2)
libubuntudownloadmanager/download_daemon.cpp (+13/-9)
libubuntudownloadmanager/download_factory.cpp (+7/-9)
libubuntudownloadmanager/download_manager.cpp (+5/-5)
libubuntudownloadmanager/download_queue.cpp (+7/-7)
libubuntudownloadmanager/downloads_db.cpp (+4/-3)
libubuntudownloadmanager/group_download.cpp (+10/-21)
libubuntudownloadmanager/libubuntudownloadmanager.pro (+1/-0)
libubuntudownloadmanager/logger.cpp (+5/-5)
libubuntudownloadmanager/logger.h (+7/-0)
libubuntudownloadmanager/single_download.cpp (+19/-30)
libubuntudownloadmanager/single_download.h (+1/-1)
libubuntudownloadmanager/system_network_info.cpp (+13/-12)
To merge this branch: bzr merge lp:~mandel/ubuntu-download-manager/correct-logging
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Alejandro J. Cura (community) Approve
dobey (community) Approve
Roberto Alsina (community) Approve
Diego Sarmentero (community) Approve
Review via email: mp+191662@code.launchpad.net

Commit message

Fix a number of logging problems found when we went to production.

Description of the change

Fix a number of logging problems found when we went to production.

To post a comment you must log in.
Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

+1

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

PASSED: Continuous integration, rev:153
http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-ci/106/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-amd64-ci/106
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-armhf-ci/97

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-download-manager-ci/106/rebuild

review: Approve (continuous-integration)
Revision history for this message
Roberto Alsina (ralsina) :
review: Approve
Revision history for this message
dobey (dobey) wrote :

+ const char* msg = message.toLatin1().data();

http://www.faqs.org/rfcs/rfc5424.html specifies the message should be UTF-8. And if not UTF-8, should be US-ASCII, not Latin1. There's no reason we shouldn't use UTF-8 here. It's in the RFC and what we should be using in Ubuntu.

review: Needs Fixing
lp:~mandel/ubuntu-download-manager/correct-logging updated
154. By Manuel de la Peña

Use utf8.

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

PASSED: Continuous integration, rev:154
http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-ci/107/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-amd64-ci/107
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-armhf-ci/98

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-download-manager-ci/107/rebuild

review: Approve (continuous-integration)
lp:~mandel/ubuntu-download-manager/correct-logging updated
155. By Manuel de la Peña

Use SHOW_TRACE insteand or TRACE.

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

looks good

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

PASSED: Continuous integration, rev:155
http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-ci/108/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-amd64-ci/108
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-download-manager-saucy-armhf-ci/99

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-download-manager-ci/108/rebuild

review: Approve (continuous-integration)
Revision history for this message
Loïc Minier (lool) wrote :

So this is already in, but I think it's unfortunate that some changes are hidden deep there both in terms of the way commits are split and the merge commit message; I've noticed that r153:
* changes level of a bunch of messages
* changes logging macros
* drops a bunch of messages

but also:
* switches to latin1 (this was reverted in r154)
* fixes support for warning level in messages sent to syslog()
* fixes path of log file
* renames onlineStateChange to onlineStateChanged

Which is not great when trying to review a larger diff like this one.

Generally ok with the changes, except I dont see how LP #1241005 is fixed since we flipped back and forth and we're back to the original approach?

I also wonder whether download-manager ought to escape things sent to syslog; what happens with e.g. logs with newlines?

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libubuntudownloadmanager/apparmor.cpp'
2--- libubuntudownloadmanager/apparmor.cpp 2013-09-25 09:12:24 +0000
3+++ libubuntudownloadmanager/apparmor.cpp 2013-10-17 18:31:10 +0000
4@@ -26,9 +26,10 @@
5 #include <QDir>
6 #include <QRegExp>
7 #include <QStandardPaths>
8-#include "./dbus_proxy.h"
9-#include "./uuid_factory.h"
10-#include "./apparmor.h"
11+#include "dbus_proxy.h"
12+#include "uuid_factory.h"
13+#include "logger.h"
14+#include "apparmor.h"
15
16 /*
17 * PRIVATE IMPLEMENTATION
18@@ -54,7 +55,7 @@
19 }
20
21 QString getUuidPath(QUuid id, QString path) {
22- qDebug() << __PRETTY_FUNCTION__ << path;
23+ TRACE << path;
24 QString idString = path + "/" + id.toString().replace(
25 QRegExp("[-{}]"), "");
26 qDebug() << "Download path is" << idString;
27@@ -132,7 +133,6 @@
28 } else {
29 // use the returned value
30 QString appId = reply.value();
31- qDebug() << "AppId is " << appId;
32
33 if (appId.isEmpty() || appId == UNCONFINED_ID) {
34 qDebug() << "UNCONFINED APP";
35
36=== modified file 'libubuntudownloadmanager/application.cpp'
37--- libubuntudownloadmanager/application.cpp 2013-09-17 10:37:08 +0000
38+++ libubuntudownloadmanager/application.cpp 2013-10-17 18:31:10 +0000
39@@ -18,7 +18,7 @@
40
41 #include <QDebug>
42 #include <QCoreApplication>
43-#include "./application.h"
44+#include "application.h"
45
46 /*
47 * PRIVATE IMPLEMENTATION
48@@ -59,7 +59,6 @@
49 void
50 Application::exit(int returnCode) {
51 Q_D(Application);
52- qDebug() << "Exit app" << returnCode;
53 d->exit(returnCode);
54 }
55
56
57=== modified file 'libubuntudownloadmanager/download.cpp'
58--- libubuntudownloadmanager/download.cpp 2013-10-09 14:45:53 +0000
59+++ libubuntudownloadmanager/download.cpp 2013-10-17 18:31:10 +0000
60@@ -18,7 +18,8 @@
61
62 #include <QDebug>
63 #include <QStringList>
64-#include "./download.h"
65+#include "logger.h"
66+#include "download.h"
67
68 /**
69 * PRIVATE IMPLEMENATION
70@@ -93,7 +94,7 @@
71 }
72
73 bool canDownload() {
74- qDebug() << __PRETTY_FUNCTION__;
75+ TRACE;
76 QNetworkInfo::NetworkMode mode = _networkInfo->currentNetworkMode();
77 switch (mode) {
78 case QNetworkInfo::UnknownMode:
79
80=== modified file 'libubuntudownloadmanager/download_daemon.cpp'
81--- libubuntudownloadmanager/download_daemon.cpp 2013-10-16 23:52:09 +0000
82+++ libubuntudownloadmanager/download_daemon.cpp 2013-10-17 18:31:10 +0000
83@@ -20,11 +20,11 @@
84 #include <QDebug>
85 #include <QSharedPointer>
86 #include <QSslCertificate>
87-#include "./application.h"
88-#include "./logger.h"
89-#include "./download_manager.h"
90-#include "./download_manager_adaptor.h"
91-#include "./download_daemon.h"
92+#include "application.h"
93+#include "logger.h"
94+#include "download_manager.h"
95+#include "download_manager_adaptor.h"
96+#include "download_daemon.h"
97
98 #define DISABLE_TIMEOUT "-disable-timeout"
99 #define SELFSIGNED_CERT "-self-signed-certs"
100@@ -71,7 +71,7 @@
101 }
102
103 void start() {
104- qDebug() << "Starting daemon";
105+ TRACE;
106 _downAdaptor = new DownloadManagerAdaptor(_downInterface);
107 bool ret = _conn->registerService(
108 "com.canonical.applications.Downloader");
109@@ -81,13 +81,13 @@
110 ret = _conn->registerObject("/", _downInterface);
111 qDebug() << ret;
112 if (!ret) {
113- qDebug() << "Could not register interface"
114+ qCritical() << "Could not register interface"
115 << _conn->connection().lastError();
116 _app->exit(-1);
117 }
118 return;
119 }
120- qDebug() << "Could not register service"
121+ qCritical() << "Could not register service"
122 << _conn->connection().lastError();
123 _app->exit(-1);
124 }
125@@ -98,8 +98,8 @@
126 }
127
128 void onDownloadManagerSizeChanged(int size) {
129+ TRACE << size;
130 bool isActive = _shutDownTimer->isActive();
131- qDebug() << "Timer is active:" << isActive << "size is:" << size;
132
133 if (isActive && size > 0) {
134 qDebug() << "Timer must be stopped because we have" << size
135@@ -158,8 +158,12 @@
136
137 // set logging
138 Logger::setupLogging();
139+#ifdef DEBUG
140+ Logger::setLogLevel(QtDebugMsg);
141+#else
142 if (qgetenv("UBUNTU_DOWNLOADER_DEBUG") != "")
143 Logger::setLogLevel(QtDebugMsg);
144+#endif
145 }
146
147 private:
148
149=== modified file 'libubuntudownloadmanager/download_factory.cpp'
150--- libubuntudownloadmanager/download_factory.cpp 2013-09-24 12:08:24 +0000
151+++ libubuntudownloadmanager/download_factory.cpp 2013-10-17 18:31:10 +0000
152@@ -17,11 +17,12 @@
153 */
154
155 #include <QPair>
156-#include "./download_adaptor.h"
157-#include "./group_download.h"
158-#include "./group_download_adaptor.h"
159-#include "./single_download.h"
160-#include "./download_factory.h"
161+#include "download_adaptor.h"
162+#include "group_download.h"
163+#include "group_download_adaptor.h"
164+#include "single_download.h"
165+#include "logger.h"
166+#include "download_factory.h"
167
168 #define OBJECT_PATH_KEY "objectpath"
169
170@@ -53,7 +54,7 @@
171 QString& dbusPath,
172 QString& rootPath,
173 bool& isConfined) {
174- qDebug() << __PRETTY_FUNCTION__ << dbusOwner << metadata;
175+ TRACE << dbusOwner << metadata;
176 if (metadata.contains(OBJECT_PATH_KEY)) {
177 // create a uuid using the string value form the metadata
178 id = QUuid(metadata[OBJECT_PATH_KEY].toString());
179@@ -83,7 +84,6 @@
180 bool isConfined = false;
181 getDownloadPath(dbusOwner, metadata, id, dbusPath, rootPath,
182 isConfined);
183- qDebug() << "Download secure data is " << id << dbusPath << rootPath;
184 Download* down = new SingleDownload(id, dbusPath, isConfined, rootPath,
185 url, metadata, headers, _networkInfo, _nam, _processFactory);
186 DownloadAdaptor* adaptor = new DownloadAdaptor(down);
187@@ -103,7 +103,6 @@
188 bool isConfined = false;
189 getDownloadPath(dbusOwner, metadata, id, dbusPath, rootPath,
190 isConfined);
191- qDebug() << "Download secure data is " << id << dbusPath << rootPath;
192 Download* down = new SingleDownload(id, dbusPath, isConfined,
193 rootPath, url, hash, algo, metadata, headers, _networkInfo, _nam,
194 _processFactory);
195@@ -124,7 +123,6 @@
196 bool isConfined = false;
197 getDownloadPath(dbusOwner, metadata, id, dbusPath, rootPath,
198 isConfined);
199- qDebug() << "Download secure data is " << id << dbusPath << rootPath;
200 Download* down = new GroupDownload(id, dbusPath, isConfined, rootPath,
201 downloads, algo, allowed3G, metadata, headers, _networkInfo,
202 _self);
203
204=== modified file 'libubuntudownloadmanager/download_manager.cpp'
205--- libubuntudownloadmanager/download_manager.cpp 2013-09-27 21:12:33 +0000
206+++ libubuntudownloadmanager/download_manager.cpp 2013-10-17 18:31:10 +0000
207@@ -18,11 +18,11 @@
208
209 #include <functional>
210 #include <QRegExp>
211-#include "./apparmor.h"
212-#include "./download_queue.h"
213-#include "./request_factory.h"
214-#include "./system_network_info.h"
215-#include "./download_manager.h"
216+#include "apparmor.h"
217+#include "download_queue.h"
218+#include "request_factory.h"
219+#include "system_network_info.h"
220+#include "download_manager.h"
221
222 /**
223 * PRIVATE IMPLEMENATION
224
225=== modified file 'libubuntudownloadmanager/download_queue.cpp'
226--- libubuntudownloadmanager/download_queue.cpp 2013-10-09 16:37:23 +0000
227+++ libubuntudownloadmanager/download_queue.cpp 2013-10-17 18:31:10 +0000
228@@ -18,7 +18,8 @@
229
230 #include <QDebug>
231 #include <QSignalMapper>
232-#include "./download_queue.h"
233+#include "logger.h"
234+#include "download_queue.h"
235
236 /*
237 * PRIVATE IMPLEMENTATION
238@@ -44,7 +45,7 @@
239 Q_Q(DownloadQueue);
240 // connect to the signals and append to the list
241 QString path = download->path();
242- qDebug() << __PRETTY_FUNCTION__ << path;
243+ TRACE << path;
244
245 _sortedPaths.append(path);
246 _downloads[path] = download;
247@@ -56,7 +57,7 @@
248
249 void remove(const QString& path) {
250 Q_Q(DownloadQueue);
251- qDebug() << __FUNCTION__ << path;
252+ TRACE << path;
253
254 Download* down = _downloads[path];
255 _sortedPaths.removeOne(path);
256@@ -87,7 +88,7 @@
257 }
258
259 void onDownloadStateChanged() {
260- qDebug() << __FUNCTION__;
261+ TRACE;
262 Q_Q(DownloadQueue);
263 // get the appdownload that emited the signal and
264 // decide what to do with it
265@@ -130,8 +131,7 @@
266 }
267
268 void onCurrentNetworkModeChanged(QNetworkInfo::NetworkMode mode) {
269- qDebug() << __PRETTY_FUNCTION__;
270- qDebug() << "Network mode changed to" << mode;
271+ TRACE << mode;
272 if (mode != QNetworkInfo::UnknownMode) {
273 updateCurrentDownload();
274 }
275@@ -139,7 +139,7 @@
276
277 private:
278 void updateCurrentDownload() {
279- qDebug() << __FUNCTION__;
280+ TRACE;
281 Q_Q(DownloadQueue);
282
283 if (!_current.isEmpty()) {
284
285=== modified file 'libubuntudownloadmanager/downloads_db.cpp'
286--- libubuntudownloadmanager/downloads_db.cpp 2013-09-25 10:56:44 +0000
287+++ libubuntudownloadmanager/downloads_db.cpp 2013-10-17 18:31:10 +0000
288@@ -25,8 +25,9 @@
289 #include <QSqlDatabase>
290 #include <QSqlQuery>
291 #include <QSqlError>
292-#include "./hash_algorithm.h"
293-#include "./downloads_db.h"
294+#include "hash_algorithm.h"
295+#include "logger.h"
296+#include "downloads_db.h"
297
298 #define SINGLE_DOWNLOAD_TABLE "CREATE TABLE SingleDownload("\
299 "uuid VARCHAR(40) PRIMARY KEY, "\
300@@ -124,7 +125,7 @@
301 }
302
303 bool init() {
304- qDebug() << __PRETTY_FUNCTION__;
305+ TRACE;
306 // create the required tables
307 qDebug() << "open the db" << _db.open();
308
309
310=== modified file 'libubuntudownloadmanager/group_download.cpp'
311--- libubuntudownloadmanager/group_download.cpp 2013-09-30 11:07:20 +0000
312+++ libubuntudownloadmanager/group_download.cpp 2013-10-17 18:31:10 +0000
313@@ -17,11 +17,12 @@
314 */
315
316 #include <QDebug>
317-#include "./download_adaptor.h"
318-#include "./hash_algorithm.h"
319-#include "./single_download.h"
320-#include "./uuid_factory.h"
321-#include "./group_download.h"
322+#include "download_adaptor.h"
323+#include "hash_algorithm.h"
324+#include "logger.h"
325+#include "single_download.h"
326+#include "uuid_factory.h"
327+#include "group_download.h"
328
329 /*
330 * PRIVATE IMPLEMENTATION
331@@ -74,18 +75,14 @@
332 // build downloads and add them to the q, it will take care of
333 // starting them etc..
334 foreach(GroupDownloadStruct download, downloads) {
335- qDebug() << "Creating download for" << download.getUrl();
336 QUrl url(download.getUrl());
337 QString hash = download.getHash();
338
339 SingleDownload* singleDownload;
340 QVariantMap downloadMetadata = QVariantMap(metadata);
341 downloadMetadata[LOCAL_PATH_KEY] = download.getLocalFile();
342- qDebug() << "Download metadata is" << downloadMetadata;
343- qDebug() << "Group metadata is" << metadata;
344
345 if (hash.isEmpty()) {
346- qDebug() << "Creating SingleDownload with no hash.";
347 singleDownload = qobject_cast<SingleDownload*>(
348 _downFactory->createDownloadForGroup(q->isConfined(),
349 q->rootPath(), url, downloadMetadata, headers));
350@@ -96,7 +93,6 @@
351 q->setLastError(QString("Invalid hash algorithm: '%1'").arg(algo));
352 }
353
354- qDebug() << "Creating SingleDownload with hash.";
355 singleDownload = qobject_cast<SingleDownload*>(
356 _downFactory->createDownloadForGroup(q->isConfined(),
357 q->rootPath(), url, hash, algo, downloadMetadata,
358@@ -122,25 +118,22 @@
359 q, SLOT(onProgress(qulonglong, qulonglong)));
360 q->connect(singleDownload, SIGNAL(finished(const QString&)),
361 q, SLOT(onFinished(const QString&)));
362- qDebug() << "Signals connected.";
363 }
364 }
365
366 void cancelDownload(bool emitSignal = true) {
367- qDebug() << __PRETTY_FUNCTION__;
368+ TRACE;
369 Q_Q(GroupDownload);
370 foreach(SingleDownload* download, _downloads) {
371 Download::State state = download->state();
372 if (state != Download::FINISH && state != Download::ERROR
373 && state != Download::CANCEL) {
374- qDebug() << "Canceling download of " << download->url();
375 download->cancel();
376 download->cancelDownload();
377 }
378 }
379
380 // loop over the finished downloads and remove the files
381- qDebug() << "Finished downloads" << _finishedDownloads;
382 foreach(const QString& path, _finishedDownloads) {
383 qDebug() << "Removing file" << path;
384 _fileManager->remove(path);
385@@ -224,7 +217,7 @@
386 private:
387 // slots to keep track of the downloads
388 void onError(const QString& error) {
389- qDebug() << __PRETTY_FUNCTION__;
390+ TRACE;
391 Q_Q(GroupDownload);
392 SingleDownload* sender = qobject_cast<SingleDownload*>(q->sender());
393 // we got an error, cancel all downloads and later remove all the
394@@ -237,22 +230,19 @@
395 void onProgress(qulonglong received, qulonglong total) {
396 Q_Q(GroupDownload);
397 SingleDownload* sender = qobject_cast<SingleDownload*>(q->sender());
398- qDebug() << __PRETTY_FUNCTION__;
399+ TRACE;
400 // TODO(mandel): the result is not real, we need to be smarter make
401 // a head request get size and name and the do all this, but atm is
402 // 'good enough' get the sender and check if we received
403 // progress from it, update its data and recalculate
404 QUrl url = sender->url();
405- qDebug() << "Progress from" << url;
406 if (_downloadsProgress.contains(url)) {
407 QPair<qulonglong, qulonglong>& data = _downloadsProgress[url];
408 data.first = received;
409 if (data.second != total) {
410- qDebug() << "Updating total!";
411 data.second = total;
412 }
413 } else {
414- qDebug() << "First progress signal";
415 _downloadsProgress[url] = QPair<qulonglong, qulonglong>(received,
416 total);
417 }
418@@ -271,13 +261,12 @@
419 }
420
421 Download* down_q = reinterpret_cast<Download*>(q);
422- qDebug() << "EMIT group progress" << totalReceived << totalTotal;
423 emit down_q->progress(totalReceived, totalTotal);
424 }
425
426 void onFinished(const QString& file) {
427 Q_Q(GroupDownload);
428- qDebug() << __PRETTY_FUNCTION__ << file;
429+ TRACE << file;
430 SingleDownload* sender = qobject_cast<SingleDownload*>(q->sender());
431 _downloadsProgress[sender->url()] = QPair<qulonglong, qulonglong>(
432 sender->totalSize(), sender->totalSize());
433
434=== modified file 'libubuntudownloadmanager/libubuntudownloadmanager.pro'
435--- libubuntudownloadmanager/libubuntudownloadmanager.pro 2013-10-09 11:53:00 +0000
436+++ libubuntudownloadmanager/libubuntudownloadmanager.pro 2013-10-17 18:31:10 +0000
437@@ -6,6 +6,7 @@
438 TEMPLATE = lib
439
440 DEFINES += APPDOWNLOADERLIB_LIBRARY
441+# DEFINES += SHOW_TRACE use if you want more debug messages
442 DEFINES += DEBUG
443
444 SOURCES += \
445
446=== modified file 'libubuntudownloadmanager/logger.cpp'
447--- libubuntudownloadmanager/logger.cpp 2013-09-27 23:43:33 +0000
448+++ libubuntudownloadmanager/logger.cpp 2013-10-17 18:31:10 +0000
449@@ -23,8 +23,7 @@
450 #include <QDebug>
451 #include <QDir>
452 #include <QStandardPaths>
453-#include "./logger.h"
454-
455+#include "logger.h"
456
457 static void *_logger = NULL;
458
459@@ -128,9 +127,8 @@
460
461 QString
462 Logger::getLogDir() {
463- QString cachePath = QStandardPaths::writableLocation(
464+ QString path = QStandardPaths::writableLocation(
465 QStandardPaths::CacheLocation);
466- QString path = cachePath + QDir::separator() + "ubuntu-download-manager";
467 qDebug() << "Logging dir is" << path;
468
469 bool wasCreated = QDir().mkpath(path);
470@@ -149,13 +147,15 @@
471 void
472 Logger::logSystemMessage(QtMsgType type, const QString& message) {
473 const char* msg = message.toUtf8().data();
474-
475 // we using %s to avoid getting a compiler error when using
476 // -Wall
477 switch (type) {
478 case QtDebugMsg:
479 syslog(LOG_DEBUG, "%s", msg);
480 break;
481+ case QtWarningMsg:
482+ syslog(LOG_WARNING, "%s", msg);
483+ break;
484 case QtCriticalMsg:
485 syslog(LOG_CRIT, "%s", msg);
486 break;
487
488=== modified file 'libubuntudownloadmanager/logger.h'
489--- libubuntudownloadmanager/logger.h 2013-09-26 08:19:53 +0000
490+++ libubuntudownloadmanager/logger.h 2013-10-17 18:31:10 +0000
491@@ -26,6 +26,13 @@
492 #include <QtGlobal>
493
494
495+#ifdef SHOW_TRACE
496+ #define TRACE qDebug() << __FILE__ ":" << __LINE__ << __PRETTY_FUNCTION__
497+#else
498+ #define TRACE if (0) qDebug()
499+#endif
500+
501+
502 class Logger : public QObject {
503 Q_OBJECT
504
505
506=== modified file 'libubuntudownloadmanager/single_download.cpp'
507--- libubuntudownloadmanager/single_download.cpp 2013-10-15 17:44:06 +0000
508+++ libubuntudownloadmanager/single_download.cpp 2013-10-17 18:31:10 +0000
509@@ -24,9 +24,10 @@
510 #include <QFile>
511 #include <QFileInfo>
512 #include <QSslError>
513-#include "./hash_algorithm.h"
514-#include "./single_download.h"
515-#include "./network_reply.h"
516+#include "hash_algorithm.h"
517+#include "logger.h"
518+#include "single_download.h"
519+#include "network_reply.h"
520
521
522 #define DATA_FILE_NAME "data.download"
523@@ -107,7 +108,7 @@
524
525 void cancelDownload() {
526 Q_Q(SingleDownload);
527- qDebug() << __PRETTY_FUNCTION__ << _url;
528+ TRACE << _url;
529
530 if (_reply != NULL) {
531 // disconnect so that we do not get useless signals
532@@ -126,7 +127,7 @@
533
534 void pauseDownload() {
535 Q_Q(SingleDownload);
536- qDebug() << __PRETTY_FUNCTION__ << _url;
537+ TRACE << _url;
538
539 if (_reply == NULL) {
540 // cannot pause because is not running
541@@ -136,7 +137,7 @@
542 return;
543 }
544
545- qDebug() << "Pausing download.";
546+ qDebug() << "Pausing download" << _url;;
547 // we need to disconnect the signals to ensure that they are not
548 // emitted due to the operation we are going to perform. We read
549 // the data in the reply and store it in a file
550@@ -165,7 +166,6 @@
551 }
552
553 qDebug() << "Resuming download.";
554- qDebug() << "Network is accessible, performing download request";
555 QNetworkRequest request = buildRequest();
556
557 // overrides the range header, we do not let clients set the range!!!
558@@ -186,9 +186,7 @@
559
560 void startDownload() {
561 Q_Q(SingleDownload);
562- qDebug() << __PRETTY_FUNCTION__ << _url;
563-
564-
565+ TRACE << _url;
566
567 if (_reply != NULL) {
568 // the download was already started, lets say that we did it
569@@ -224,7 +222,7 @@
570 }
571
572 void setThrottle(qulonglong speed) {
573- qDebug() << __PRETTY_FUNCTION__ << _url;
574+ TRACE << _url;
575
576 if (_reply != NULL)
577 _reply->setReadBufferSize(speed);
578@@ -233,7 +231,7 @@
579 // slots executed to keep track of the network reply
580 void onDownloadProgress(qint64 progress, qint64 bytesTotal) {
581 Q_Q(SingleDownload);
582- qDebug() << __PRETTY_FUNCTION__ << _url << progress << bytesTotal;
583+ TRACE << _url << progress << bytesTotal;
584
585 // write the current info we have, just in case we are killed in the
586 // middle of the download
587@@ -244,12 +242,10 @@
588 if (bytesTotal == -1) {
589 // we do not know the size of the download, simply return
590 // the same for received and for total
591- qDebug() << "EMIT progress" << received << received;
592 emit reinterpret_cast<Download*>(q)->progress(received, received);
593 return;
594 } else {
595 if (_totalSize == 0) {
596- qDebug() << "Updating total size" << bytesTotal;
597 qlonglong uBytestTotal = bytesTotal;
598 // bytesTotal is different when we have resumed because we
599 // are not counting the size that we already downloaded,
600@@ -257,7 +253,6 @@
601 // update the metadata
602 _totalSize = uBytestTotal;
603 }
604- qDebug() << "EMIT progress" << received << _totalSize;
605 emit reinterpret_cast<Download*>(q)->progress(received, _totalSize);
606 return;
607 }
608@@ -271,7 +266,7 @@
609
610 void onFinished() {
611 Q_Q(SingleDownload);
612- qDebug() << __PRETTY_FUNCTION__ << _url;
613+ TRACE << _url;
614
615 // if the hash is present we check it
616 if (!_hash.isEmpty()) {
617@@ -344,8 +339,7 @@
618 }
619
620 void onSslErrors(const QList<QSslError>& errors) {
621- qDebug() << __PRETTY_FUNCTION__ << _url;
622- qDebug() << "Found errors" << errors;
623+ TRACE << errors;
624 Q_UNUSED(errors);
625 if (!_reply->canIgnoreSslErrors(errors)) {
626 _downloading = false;
627@@ -355,13 +349,13 @@
628
629 // slots executed to keep track of the post download process
630 void onProcessError(QProcess::ProcessError error) {
631- qDebug() << __PRETTY_FUNCTION__ << error;
632+ TRACE << error;
633 Q_UNUSED(error);
634 emitError("COMMAND ERROR");
635 }
636
637 void onProcessFinished(int exitCode, QProcess::ExitStatus exitStatus) {
638- qDebug() << __PRETTY_FUNCTION__ << exitCode << exitStatus;
639+ TRACE << exitCode << exitStatus;
640 Q_Q(SingleDownload);
641 if (exitCode == 0 && exitStatus == QProcess::NormalExit) {
642 // remove the file since we are done with it
643@@ -376,15 +370,14 @@
644 }
645 }
646
647- void onOnlineStateChange(bool online) {
648- qDebug() << __PRETTY_FUNCTION__ << online;
649+ void onOnlineStateChanged(bool online) {
650+ TRACE << online;
651 Q_Q(SingleDownload);
652 _connected = online;
653 // if we are downloading and the status is correct let's call
654 // the method again, else do nothing
655 if (_connected && _downloading) {
656 Download::State state = q->state();
657- qDebug() << "We are back online and we were downloading";
658 if (state == Download::START || state == Download::RESUME) {
659 resumeDownload();
660 }
661@@ -393,7 +386,6 @@
662 // if no longer online yet we have a reply (that is, we are trying
663 // to get data from the missing connection) we pause
664 if (!_connected && _reply != NULL) {
665- qDebug() << "Lost connection and therefore pausing";
666 pauseDownload();
667 // set it to be downloading even when pause download sets it
668 // to false
669@@ -410,11 +402,10 @@
670
671 // connect to the network changed signals
672 q->connect(q->networkInfo().data(),
673- SIGNAL(onlineStateChange(bool)), q,
674- SLOT(onOnlineStateChange(bool)));
675+ SIGNAL(onlineStateChanged(bool)), q,
676+ SLOT(onOnlineStateChanged(bool)));
677
678 _filePath = saveFileName();
679- qDebug() << "Download path is" << _filePath;
680 _reply = NULL;
681 _currentData = NULL;
682
683@@ -466,7 +457,6 @@
684 QString finalPath;
685
686 if (!q->isConfined() && metadata.contains(LOCAL_PATH_KEY)) {
687- qDebug() << "App is not confined and provided a local path";
688 finalPath = metadata[LOCAL_PATH_KEY].toString();
689
690 // in this case and because the app is not confined we are
691@@ -509,7 +499,6 @@
692
693 QNetworkRequest buildRequest() {
694 Q_Q(SingleDownload);
695- qDebug() << "Building request for " << _url;
696 QNetworkRequest request = QNetworkRequest(_url);
697 QMap<QString, QString> headers = q->headers();
698 foreach(const QString& header, headers.keys()) {
699@@ -527,7 +516,7 @@
700 }
701
702 void emitError(const QString& error) {
703- qDebug() << __PRETTY_FUNCTION__ << error;
704+ TRACE << error;
705 Q_Q(SingleDownload);
706 disconnectFromReplySignals();
707 _reply->deleteLater();
708
709=== modified file 'libubuntudownloadmanager/single_download.h'
710--- libubuntudownloadmanager/single_download.h 2013-10-11 15:12:55 +0000
711+++ libubuntudownloadmanager/single_download.h 2013-10-17 18:31:10 +0000
712@@ -96,7 +96,7 @@
713
714 // private slot used to keep track of the connection
715 Q_PRIVATE_SLOT(d_func(),
716- void onOnlineStateChange(bool))
717+ void onOnlineStateChanged(bool))
718
719 private:
720 // use pimpl so that we can mantains ABI compatibility
721
722=== modified file 'libubuntudownloadmanager/system_network_info.cpp'
723--- libubuntudownloadmanager/system_network_info.cpp 2013-10-11 15:12:55 +0000
724+++ libubuntudownloadmanager/system_network_info.cpp 2013-10-17 18:31:10 +0000
725@@ -19,7 +19,8 @@
726 #include <QDebug>
727 #include <QNetworkConfigurationManager>
728 #include <QNetworkSession>
729-#include "./system_network_info.h"
730+#include "logger.h"
731+#include "system_network_info.h"
732
733 /*
734 * PRIVATE IMPLEMENTATION
735@@ -108,7 +109,7 @@
736
737 void onOnlineStateChanged(bool online) {
738 Q_Q(SystemNetworkInfo);
739- qDebug() << __PRETTY_FUNCTION__ << online;
740+ TRACE << online;
741 emit q->onlineStateChanged(online);
742 }
743
744@@ -116,66 +117,66 @@
745
746 void onCellIdChanged(int interface, const QString& id) {
747 Q_Q(SystemNetworkInfo);
748- qDebug() << __PRETTY_FUNCTION__ << interface << id;
749+ TRACE << interface << id;
750 emit q->cellIdChanged(interface, id);
751 }
752
753 void onCurrentCellDataTechnologyChanged(int interface,
754 QNetworkInfo::CellDataTechnology tech) {
755 Q_Q(SystemNetworkInfo);
756- qDebug() << __PRETTY_FUNCTION__ << interface << tech;
757+ TRACE << interface << tech;
758 emit q->currentCellDataTechnologyChanged(interface, tech);
759 }
760
761 void onCurrentMobileCountryCodeChanged(int interface, const QString& mcc) {
762 Q_Q(SystemNetworkInfo);
763- qDebug() << __PRETTY_FUNCTION__ << interface << mcc;
764+ TRACE << interface << mcc;
765 emit q->currentMobileCountryCodeChanged(interface, mcc);
766 }
767
768 void onCurrentMobileNetworkCodeChanged(int interface, const QString& mnc) {
769 Q_Q(SystemNetworkInfo);
770- qDebug() << __PRETTY_FUNCTION__ << interface << mnc;
771+ TRACE << interface << mnc;
772 emit q->currentMobileNetworkCodeChanged(interface, mnc);
773 }
774
775 void onCurrentNetworkModeChanged(QNetworkInfo::NetworkMode mode) {
776 Q_Q(SystemNetworkInfo);
777- qDebug() << __PRETTY_FUNCTION__ << mode;
778+ TRACE << mode;
779 emit q->currentNetworkModeChanged(mode);
780 }
781
782 void onLocationAreaCodeChanged(int interface, const QString& lac) {
783 Q_Q(SystemNetworkInfo);
784- qDebug() << __PRETTY_FUNCTION__ << interface << lac;
785+ TRACE << interface << lac;
786 emit q->locationAreaCodeChanged(interface, lac);
787 }
788
789 void onNetworkInterfaceCountChanged(QNetworkInfo::NetworkMode mode,
790 int count) {
791 Q_Q(SystemNetworkInfo);
792- qDebug() << __PRETTY_FUNCTION__ << mode << count;
793+ TRACE << mode << count;
794 emit q->networkInterfaceCountChanged(mode, count);
795 }
796
797 void onNetworkNameChanged(QNetworkInfo::NetworkMode mode, int interface,
798 const QString& name) {
799 Q_Q(SystemNetworkInfo);
800- qDebug() << __PRETTY_FUNCTION__ << mode << interface << name;
801+ TRACE << mode << interface << name;
802 emit q->networkNameChanged(mode, interface, name);
803 }
804
805 void onNetworkSignalStrengthChanged(QNetworkInfo::NetworkMode mode,
806 int interface, int strength) {
807 Q_Q(SystemNetworkInfo);
808- qDebug() << __PRETTY_FUNCTION__ << mode << interface << strength;
809+ TRACE << mode << interface << strength;
810 emit q->networkSignalStrengthChanged(mode, interface, strength);
811 }
812
813 void onNetworkStatusChanged(QNetworkInfo::NetworkMode mode, int interface,
814 QNetworkInfo::NetworkStatus status) {
815 Q_Q(SystemNetworkInfo);
816- qDebug() << __PRETTY_FUNCTION__ << mode << interface << status;
817+ TRACE << mode << interface << status;
818 emit q->networkStatusChanged(mode, interface, status);
819 }
820

Subscribers

People subscribed via source and target branches