Redesigned HTTP downloader

- Save the data as they come rather than at once
- + some related refactoring

gpxsee.pro

@@ -43,6 +43,7 @@ HEADERS += src/common/config.h \
     src/common/greatcircle.h \
     src/common/programpaths.h \
     src/common/tifffile.h \
+    src/common/downloader.h \
     src/GUI/app.h \
     src/GUI/icons.h \
     src/GUI/gui.h \
@@ -135,7 +136,6 @@ HEADERS += src/common/config.h \
     src/map/map.h \
     src/map/maplist.h \
     src/map/onlinemap.h \
-    src/map/downloader.h \
     src/map/tile.h \
     src/map/emptymap.h \
     src/map/ozimap.h \
@@ -247,6 +247,7 @@ SOURCES += src/main.cpp \
     src/common/greatcircle.cpp \
     src/common/programpaths.cpp \
     src/common/tifffile.cpp \
+    src/common/downloader.cpp \
     src/GUI/app.cpp \
     src/GUI/gui.cpp \
     src/GUI/axisitem.cpp \
@@ -315,7 +316,6 @@ SOURCES += src/main.cpp \
     src/map/kmzmap.cpp \
     src/map/maplist.cpp \
     src/map/onlinemap.cpp \
-    src/map/downloader.cpp \
     src/map/emptymap.cpp \
     src/map/ozimap.cpp \
     src/map/polyconic.cpp \

src/GUI/app.cpp

@@ -9,7 +9,7 @@
 #include <QSurfaceFormat>
 #include "common/programpaths.h"
 #include "common/config.h"
-#include "map/downloader.h"
+#include "common/downloader.h"
 #include "map/ellipsoid.h"
 #include "map/gcs.h"
 #include "map/pcs.h"

src/GUI/gui.cpp

@@ -26,11 +26,11 @@
 #include <QStyle>
 #include <QTabBar>
 #include "common/programpaths.h"
+#include "common/downloader.h"
 #include "data/data.h"
 #include "data/poi.h"
 #include "map/maplist.h"
 #include "map/emptymap.h"
-#include "map/downloader.h"
 #include "map/crs.h"
 #include "icons.h"
 #include "keys.h"

src/common/downloader.cpp

@@ -21,16 +21,28 @@
 #define USER_AGENT \
 	APP_NAME "/" APP_VERSION " (" PLATFORM_STR "; Qt " QT_VERSION_STR ")"
 
-#define ATTR_REDIRECT QNetworkRequest::RedirectionTargetAttribute
-#define ATTR_FILE     QNetworkRequest::User
-#define ATTR_ORIGIN \
-  static_cast<QNetworkRequest::Attribute>(QNetworkRequest::User + 1)
-#define ATTR_LEVEL \
-  static_cast<QNetworkRequest::Attribute>(QNetworkRequest::User + 2)
-
 #define MAX_REDIRECT_LEVEL 5
 #define RETRIES 3
 
+#define ATTR_REDIRECT_POLICY QNetworkRequest::RedirectPolicyAttribute
+#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
+#define ATTR_HTTP2_ALLOWED QNetworkRequest::HTTP2AllowedAttribute
+#else // QT 5.15
+#define ATTR_HTTP2_ALLOWED QNetworkRequest::Http2AllowedAttribute
+#endif // QT 5.15
+
+#define TMP_SUFFIX ".download"
+
+
+static QString tmpName(const QString &origName)
+{
+	return origName + TMP_SUFFIX;
+}
+
+static QString origName(const QString &tmpName)
+{
+	return tmpName.left(tmpName.size() - (sizeof(TMP_SUFFIX) - 1));
+}
 
 Authorization::Authorization(const QString &username, const QString &password)
 {
@@ -67,93 +79,67 @@ private:
 	QBasicTimer _timer;
 };
 
-class Downloader::Redirect
-{
-public:
-	Redirect() : _level(0) {}
-	Redirect(const QUrl &origin, int level) :
-	  _origin(origin), _level(level) {}
-
-	const QUrl &origin() const {return _origin;}
-	int level() const {return _level;}
-
-private:
-	QUrl _origin;
-	int _level;
-};
-
 QNetworkAccessManager *Downloader::_manager = 0;
 int Downloader::_timeout = 30;
 bool Downloader::_http2 = true;
 
-bool Downloader::doDownload(const Download &dl,
+bool Downloader::doDownload(const Download &dl, const QByteArray &authorization)
-  const QByteArray &authorization, const Redirect *redirect)
 {
 	const QUrl &url = dl.url();
 
 	if (!url.isValid() || !(url.scheme() == QLatin1String("http")
 	  || url.scheme() == QLatin1String("https"))) {
 		qWarning("%s: Invalid URL", qPrintable(url.toString()));
-		if (redirect)
-			_errorDownloads.insert(redirect->origin(), RETRIES);
 		return false;
 	}
 
 	if (_errorDownloads.value(url) >= RETRIES)
 		return false;
-	if (_currentDownloads.contains(url) && !redirect)
+	if (_currentDownloads.contains(url))
 		return false;
 
 	QNetworkRequest request(url);
-	request.setAttribute(ATTR_FILE, QVariant(dl.file()));
+	request.setMaximumRedirectsAllowed(MAX_REDIRECT_LEVEL);
-	if (redirect) {
+	request.setAttribute(ATTR_REDIRECT_POLICY,
-		request.setAttribute(ATTR_ORIGIN, QVariant(redirect->origin()));
+	  QNetworkRequest::NoLessSafeRedirectPolicy);
-		request.setAttribute(ATTR_LEVEL, QVariant(redirect->level()));
+	request.setAttribute(ATTR_HTTP2_ALLOWED, QVariant(_http2));
-	}
 	request.setRawHeader("User-Agent", USER_AGENT);
 	if (!authorization.isNull())
 		request.setRawHeader("Authorization", authorization);
-#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
+
-	request.setAttribute(QNetworkRequest::HTTP2AllowedAttribute,
+	QFile *file = new QFile(tmpName(dl.file()));
-	  QVariant(_http2));
+	if (!file->open(QIODevice::ReadWrite)) {
-#else // QT 5.15
+		qWarning("%s: %s", qPrintable(file->fileName()),
-	request.setAttribute(QNetworkRequest::Http2AllowedAttribute,
+		  qPrintable(file->errorString()));
-	  QVariant(_http2));
+		_errorDownloads.insert(url, RETRIES);
-#endif // QT 5.15
+		return false;
+	}
 
 	Q_ASSERT(_manager);
 	QNetworkReply *reply = _manager->get(request);
-	if (reply && reply->isRunning()) {
+	file->setParent(reply);
-		_currentDownloads.insert(url);
+	_currentDownloads.insert(url, file);
+
+	if (reply->isRunning()) {
 		ReplyTimeout::setTimeout(reply, _timeout);
+		connect(reply, &QIODevice::readyRead, this, &Downloader::emitReadReady);
 		connect(reply, &QNetworkReply::finished, this, &Downloader::emitFinished);
-	} else if (reply)
+	} else {
+		readData(reply);
 		downloadFinished(reply);
-	else
+	}
-		return false;
 
 	return true;
 }
 
 void Downloader::emitFinished()
 {
-	downloadFinished(static_cast<QNetworkReply*>(sender()));
+	downloadFinished(qobject_cast<QNetworkReply*>(sender()));
 }
 
-bool Downloader::saveToDisk(const QString &filename, QIODevice *data)
+void Downloader::emitReadReady()
 {
-	QFile file(filename);
+	readData(qobject_cast<QNetworkReply*>(sender()));
-
-	if (!file.open(QIODevice::WriteOnly)) {
-		qWarning("Error writing file: %s: %s",
-		  qPrintable(filename), qPrintable(file.errorString()));
-		return false;
-	}
-
-	file.write(data->readAll());
-	file.close();
-
-	return true;
 }
 
 void Downloader::insertError(const QUrl &url, QNetworkReply::NetworkError error)
@@ -164,54 +150,27 @@ void Downloader::insertError(const QUrl &url, QNetworkReply::NetworkError error)
 		_errorDownloads.insert(url, RETRIES);
 }
 
+void Downloader::readData(QNetworkReply *reply)
+{
+	QFile *file = _currentDownloads.value(reply->request().url());
+	Q_ASSERT(file);
+	file->write(reply->readAll());
+}
+
 void Downloader::downloadFinished(QNetworkReply *reply)
 {
 	QUrl url(reply->request().url());
 	QNetworkReply::NetworkError error = reply->error();
 
+	QFile *file = _currentDownloads.value(reply->request().url());
 	if (error) {
-		QUrl origin(reply->request().attribute(ATTR_ORIGIN).toUrl());
-		if (origin.isEmpty()) {
 		insertError(url, error);
-			qWarning("Error downloading file: %s: %s",
+		qWarning("%s: %s", url.toEncoded().constData(),
-			  url.toEncoded().constData(), qPrintable(reply->errorString()));
-		} else {
-			insertError(origin, error);
-			qWarning("Error downloading file: %s -> %s: %s",
-			  origin.toEncoded().constData(), url.toEncoded().constData(),
 		  qPrintable(reply->errorString()));
-		}
+		file->remove();
 	} else {
-		QUrl location(reply->attribute(ATTR_REDIRECT).toUrl());
+		file->close();
-		QString filename(reply->request().attribute(ATTR_FILE).toString());
+		file->rename(origName(file->fileName()));
-
-		if (!location.isEmpty()) {
-			QUrl origin(reply->request().attribute(ATTR_ORIGIN).toUrl());
-			int level = reply->request().attribute(ATTR_LEVEL).toInt();
-
-			if (level >= MAX_REDIRECT_LEVEL) {
-				_errorDownloads.insert(origin, RETRIES);
-				qWarning("Error downloading file: %s: "
-				  "redirect level limit reached (redirect loop?)",
-				  origin.toEncoded().constData());
-			} else {
-				QUrl redirectUrl;
-				if (location.isRelative()) {
-					QString path = QDir::isAbsolutePath(location.path())
-					  ? location.path() : "/" + location.path();
-					redirectUrl = QUrl(url.scheme() + "://" + url.host() + path);
-				} else
-					redirectUrl = location;
-
-				Redirect redirect(origin.isEmpty() ? url : origin, level + 1);
-				Download dl(redirectUrl, filename);
-				doDownload(dl, reply->request().rawHeader("Authorization"),
-				  &redirect);
-			}
-		} else {
-			if (!saveToDisk(filename, reply))
-				_errorDownloads.insert(url, RETRIES);
-		}
 	}
 
 	_currentDownloads.remove(url);
@@ -238,3 +197,12 @@ void Downloader::enableHTTP2(bool enable)
 	_http2 = enable;
 	_manager->clearConnectionCache();
 }
+
+#ifndef QT_NO_DEBUG
+QDebug operator<<(QDebug dbg, const Download &download)
+{
+	dbg.nospace() << "Download(" << download.url() << "," << download.file()
+	  << ")";
+	return dbg.space();
+}
+#endif // QT_NO_DEBUG

src/common/downloader.h

@@ -5,9 +5,9 @@
 #include <QNetworkReply>
 #include <QUrl>
 #include <QList>
-#include <QSet>
 #include <QHash>
 
+class QFile;
 
 class Download
 {
@@ -55,18 +55,17 @@ signals:
 
 private slots:
 	void emitFinished();
-	void downloadFinished(QNetworkReply *reply);
+	void emitReadReady();
 
 private:
-	class Redirect;
 	class ReplyTimeout;
 
 	void insertError(const QUrl &url, QNetworkReply::NetworkError error);
-	bool doDownload(const Download &dl, const QByteArray &authorization,
+	bool doDownload(const Download &dl, const QByteArray &authorization);
-	  const Redirect *redirect = 0);
+	void downloadFinished(QNetworkReply *reply);
-	bool saveToDisk(const QString &filename, QIODevice *data);
+	void readData(QNetworkReply *reply);
 
-	QSet<QUrl> _currentDownloads;
+	QHash<QUrl, QFile*> _currentDownloads;
 	QHash<QUrl, int> _errorDownloads;
 
 	static QNetworkAccessManager *_manager;
@@ -74,4 +73,8 @@ private:
 	static bool _http2;
 };
 
+#ifndef QT_NO_DEBUG
+QDebug operator<<(QDebug dbg, const Download &download);
+#endif // QT_NO_DEBUG
+
 #endif // DOWNLOADER_H

src/map/mapsource.h

@@ -5,7 +5,7 @@
 #include "common/range.h"
 #include "common/rectc.h"
 #include "common/kv.h"
-#include "downloader.h"
+#include "common/downloader.h"
 #include "coordinatesystem.h"
 
 class Map;

src/map/onlinemap.cpp

@@ -3,7 +3,7 @@
 #include <QDir>
 #include "common/rectc.h"
 #include "common/programpaths.h"
-#include "downloader.h"
+#include "common/downloader.h"
 #include "osm.h"
 #include "onlinemap.h"
 

src/map/tileloader.h

@@ -3,8 +3,8 @@
 
 #include <QObject>
 #include <QString>
+#include "common/downloader.h"
 #include "tile.h"
-#include "downloader.h"
 
 class TileLoader : public QObject
 {
@@ -38,4 +38,4 @@ private:
 	bool _quadTiles;
 };
 
-#endif // TILELOADER_Honlinemap
+#endif // TILELOADER_H

src/map/wms.cpp

@@ -3,7 +3,6 @@
 #include <QEventLoop>
 #include <QXmlStreamReader>
 #include <QStringList>
-#include "downloader.h"
 #include "crs.h"
 #include "wms.h"
 
@@ -320,20 +319,6 @@ bool WMS::parseCapabilities()
 	return true;
 }
 
-bool WMS::downloadCapabilities(const QString &url)
-{
-	if (!_downloader) {
-		_downloader = new Downloader(this);
-		connect(_downloader, &Downloader::finished, this,
-		  &WMS::capabilitiesReady);
-	}
-
-	QList<Download> dl;
-	dl.append(Download(url, _path));
-
-	return _downloader->get(dl, _setup.authorization());
-}
-
 void WMS::capabilitiesReady()
 {
 	if (!QFileInfo(_path).exists()) {
@@ -348,15 +333,20 @@ void WMS::capabilitiesReady()
 }
 
 WMS::WMS(const QString &file, const WMS::Setup &setup, QObject *parent)
-  : QObject(parent), _setup(setup), _path(file), _downloader(0), _valid(false),
+  : QObject(parent), _setup(setup), _path(file), _valid(false), _ready(false)
-  _ready(false)
 {
 	QString url = QString("%1%2service=WMS&request=GetCapabilities")
 	  .arg(setup.url(), setup.url().contains('?') ? "&" : "?");
 
-	if (!QFileInfo(file).exists())
+	if (!QFileInfo(file).exists()) {
-		_valid = downloadCapabilities(url);
+		Downloader *downloader = new Downloader(this);
-	else {
+		connect(downloader, &Downloader::finished, this,
+		  &WMS::capabilitiesReady);
+
+		QList<Download> dl;
+		dl.append(Download(url, _path));
+		_valid = downloader->get(dl, _setup.authorization());
+	} else {
 		_ready = true;
 		_valid = parseCapabilities();
 	}

src/map/wms.h

@@ -6,8 +6,8 @@
 #include "common/range.h"
 #include "common/rectc.h"
 #include "common/kv.h"
+#include "common/downloader.h"
 #include "projection.h"
-#include "downloader.h"
 #include "coordinatesystem.h"
 
 class QXmlStreamReader;
@@ -109,11 +109,9 @@ private:
 	void capability(QXmlStreamReader &reader, CTX &ctx);
 	void capabilities(QXmlStreamReader &reader, CTX &ctx);
 	bool parseCapabilities();
-	bool downloadCapabilities(const QString &url);
 
 	WMS::Setup _setup;
 	QString _path;
-	Downloader *_downloader;
 	Projection _projection;
 	RangeF _scaleDenominator;
 	RectC _bbox;

src/map/wmsmap.cpp

@@ -4,7 +4,6 @@
 #include "common/wgs84.h"
 #include "common/rectc.h"
 #include "common/programpaths.h"
-#include "downloader.h"
 #include "tileloader.h"
 #include "wmsmap.h"
 

src/map/wmts.cpp

@@ -6,7 +6,6 @@
 #include <QStringList>
 #include <QtAlgorithms>
 #include <QXmlStreamReader>
-#include "downloader.h"
 #include "pcs.h"
 #include "crs.h"
 #include "wmts.h"
@@ -302,20 +301,6 @@ bool WMTS::parseCapabilities(CTX &ctx)
 	return true;
 }
 
-bool WMTS::downloadCapabilities(const QString &url)
-{
-	if (!_downloader) {
-		_downloader = new Downloader(this);
-		connect(_downloader, &Downloader::finished, this,
-		  &WMTS::capabilitiesReady);
-	}
-
-	QList<Download> dl;
-	dl.append(Download(url, _path));
-
-	return _downloader->get(dl, _setup.authorization());
-}
-
 void WMTS::capabilitiesReady()
 {
 	if (!QFileInfo(_path).exists()) {
@@ -363,7 +348,7 @@ bool WMTS::init()
 }
 
 WMTS::WMTS(const QString &file, const WMTS::Setup &setup, QObject *parent)
-  : QObject(parent), _setup(setup), _downloader(0), _valid(false), _ready(false)
+  : QObject(parent), _setup(setup), _valid(false), _ready(false)
 {
 	QUrl url(setup.rest() ? setup.url() : QString(
 	  "%1%2service=WMTS&Version=1.0.0&request=GetCapabilities").arg(setup.url(),
@@ -371,9 +356,15 @@ WMTS::WMTS(const QString &file, const WMTS::Setup &setup, QObject *parent)
 
 	_path = url.isLocalFile() ? url.toLocalFile() : file;
 
-	if (!url.isLocalFile() && !QFileInfo(file).exists())
+	if (!url.isLocalFile() && !QFileInfo(file).exists()) {
-		_valid = downloadCapabilities(url.toString());
+		Downloader *downloader = new Downloader(this);
-	else {
+		connect(downloader, &Downloader::finished, this,
+		  &WMTS::capabilitiesReady);
+
+		QList<Download> dl;
+		dl.append(Download(url.toString(), _path));
+		_valid = downloader->get(dl, _setup.authorization());
+	} else {
 		_ready = true;
 		_valid = init();
 	}

src/map/wmts.h

@@ -9,8 +9,8 @@
 #include "common/config.h"
 #include "common/rectc.h"
 #include "common/kv.h"
+#include "common/downloader.h"
 #include "projection.h"
-#include "downloader.h"
 #include "coordinatesystem.h"
 
 class QXmlStreamReader;
@@ -154,13 +154,11 @@ private:
 	void contents(QXmlStreamReader &reader, CTX &ctx);
 	void capabilities(QXmlStreamReader &reader, CTX &ctx);
 	bool parseCapabilities(CTX &ctx);
-	bool downloadCapabilities(const QString &url);
 	void createZooms(const CTX &ctx);
 	bool init();
 
 	WMTS::Setup _setup;
 	QString _path;
-	Downloader *_downloader;
 	RectC _bbox;
 	QList<Zoom> _zooms;
 	Projection _projection;