Split NetworkTimeouts and RedirectFollower to it's own files

- Follow redirects by default

src/CMakeLists.txt

@@ -91,6 +91,8 @@ set(SOURCES
   core/multisortfilterproxy.cpp
   core/musicstorage.cpp
   core/network.cpp
+  core/networktimeouts.cpp
+  core/redirectfollower.cpp
   core/networkproxyfactory.cpp
   core/qtfslistener.cpp
   core/settingsprovider.cpp
@@ -298,6 +300,8 @@ set(HEADERS
   core/filesystemwatcherinterface.h
   core/mergedproxymodel.h
   core/network.h
+  core/networktimeouts.h
+  core/redirectfollower.h
   core/qtfslistener.h
   core/songloader.h
   core/tagreaderclient.h

src/core/network.cpp

@@ -2,6 +2,7 @@
  * Strawberry Music Player
  * This file was part of Clementine.
  * Copyright 2010, David Sansome <me@davidsansome.com>
+ * Copyright 2018-2019, Jonas Kvinge <jonas@jkvinge.net>
  *
  * Strawberry is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -21,11 +22,11 @@
 #include "config.h"
 
 #include <QtGlobal>
-#include <QCoreApplication>
 #include <QObject>
+#include <QCoreApplication>
 #include <QStandardPaths>
-#include <QMutex>
 #include <QIODevice>
+#include <QMutex>
 #include <QString>
 #include <QUrl>
 #include <QNetworkAccessManager>
@@ -35,7 +36,6 @@
 #include <QNetworkCacheMetaData>
 #include <QAbstractNetworkCache>
 
-#include "core/closure.h"
 #include "network.h"
 
 QMutex ThreadSafeNetworkDiskCache::sMutex;
@@ -104,7 +104,13 @@ void ThreadSafeNetworkDiskCache::clear() {
 
 NetworkAccessManager::NetworkAccessManager(QObject *parent)
     : QNetworkAccessManager(parent) {
+
+#if (QT_VERSION >= QT_VERSION_CHECK(5, 9, 0))
+  setRedirectPolicy(QNetworkRequest::NoLessSafeRedirectPolicy);
+#endif
+
   setCache(new ThreadSafeNetworkDiskCache(this));
+
 }
 
 QNetworkReply *NetworkAccessManager::createRequest(Operation op, const QNetworkRequest &request, QIODevice *outgoingData) {
@@ -130,111 +136,3 @@ QNetworkReply *NetworkAccessManager::createRequest(Operation op, const QNetworkR
 
   return QNetworkAccessManager::createRequest(op, new_request, outgoingData);
 }
-
-
-NetworkTimeouts::NetworkTimeouts(int timeout_msec, QObject *parent)
-    : QObject(parent), timeout_msec_(timeout_msec) {}
-
-void NetworkTimeouts::AddReply(QNetworkReply *reply) {
-
-  if (timers_.contains(reply)) return;
-
-  connect(reply, SIGNAL(destroyed()), SLOT(ReplyFinished()));
-  connect(reply, SIGNAL(finished()), SLOT(ReplyFinished()));
-  timers_[reply] = startTimer(timeout_msec_);
-
-}
-
-void NetworkTimeouts::AddReply(RedirectFollower *reply) {
-
-  if (redirect_timers_.contains(reply)) {
-    return;
-  }
-
-  NewClosure(reply, SIGNAL(destroyed()), this, SLOT(RedirectFinished(RedirectFollower*)), reply);
-  NewClosure(reply, SIGNAL(finished()), this, SLOT(RedirectFinished(RedirectFollower*)), reply);
-  redirect_timers_[reply] = startTimer(timeout_msec_);
-
-}
-
-void NetworkTimeouts::ReplyFinished() {
-
-  QNetworkReply *reply = reinterpret_cast<QNetworkReply*>(sender());
-  if (timers_.contains(reply)) {
-    killTimer(timers_.take(reply));
-  }
-
-}
-
-void NetworkTimeouts::RedirectFinished(RedirectFollower *reply) {
-
-  if (redirect_timers_.contains(reply)) {
-    killTimer(redirect_timers_.take(reply));
-  }
-
-}
-
-void NetworkTimeouts::timerEvent(QTimerEvent *e) {
-
-  QNetworkReply *reply = timers_.key(e->timerId());
-  if (reply) {
-    reply->abort();
-  }
-
-  RedirectFollower *redirect = redirect_timers_.key(e->timerId());
-  if (redirect) {
-    redirect->abort();
-  }
-
-}
-
-
-RedirectFollower::RedirectFollower(QNetworkReply *first_reply, int max_redirects) : QObject(nullptr), current_reply_(first_reply), redirects_remaining_(max_redirects) {
-  ConnectReply(first_reply);
-}
-
-void RedirectFollower::ConnectReply(QNetworkReply *reply) {
-
-  connect(reply, SIGNAL(readyRead()), SLOT(ReadyRead()));
-  connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), SIGNAL(error(QNetworkReply::NetworkError)));
-  connect(reply, SIGNAL(downloadProgress(qint64,qint64)), SIGNAL(downloadProgress(qint64,qint64)));
-  connect(reply, SIGNAL(uploadProgress(qint64,qint64)), SIGNAL(uploadProgress(qint64,qint64)));
-  connect(reply, SIGNAL(finished()), SLOT(ReplyFinished()));
-
-}
-
-void RedirectFollower::ReadyRead() {
-
-  // Don't re-emit this signal for redirect replies.
-  if (current_reply_->attribute(QNetworkRequest::RedirectionTargetAttribute).isValid()) {
-    return;
-  }
-
-  emit readyRead();
-
-}
-
-void RedirectFollower::ReplyFinished() {
-
-  current_reply_->deleteLater();
-
-  if (current_reply_->attribute(QNetworkRequest::RedirectionTargetAttribute).isValid()) {
-    if (redirects_remaining_-- == 0) {
-      emit finished();
-      return;
-    }
-
-    const QUrl next_url = current_reply_->url().resolved(current_reply_->attribute(QNetworkRequest::RedirectionTargetAttribute).toUrl());
-
-    QNetworkRequest req(current_reply_->request());
-    req.setUrl(next_url);
-
-    current_reply_ = current_reply_->manager()->get(req);
-    ConnectReply(current_reply_);
-    return;
-  }
-
-  emit finished();
-
-}
-

src/core/network.h

@@ -2,6 +2,7 @@
  * Strawberry Music Player
  * This file was part of Clementine.
  * Copyright 2010, David Sansome <me@davidsansome.com>
+ * Copyright 2018-2019, Jonas Kvinge <jonas@jkvinge.net>
  *
  * Strawberry is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -27,22 +28,25 @@
 
 #include <QtGlobal>
 #include <QObject>
-#include <QMutex>
-#include <QIODevice>
-#include <QMap>
-#include <QByteArray>
-#include <QVariant>
-#include <QString>
-#include <QUrl>
 #include <QNetworkAccessManager>
+#include <QAbstractNetworkCache>
+#include <QIODevice>
+#include <QMutex>
+#include <QUrl>
 #include <QNetworkRequest>
 #include <QNetworkReply>
 #include <QNetworkDiskCache>
 #include <QNetworkCacheMetaData>
-#include <QAbstractNetworkCache>
-#include <QtEvents>
 
-class QTimerEvent;
+class NetworkAccessManager : public QNetworkAccessManager {
+  Q_OBJECT
+
+ public:
+  explicit NetworkAccessManager(QObject *parent = nullptr);
+
+ protected:
+  QNetworkReply *createRequest(Operation op, const QNetworkRequest &request, QIODevice *outgoingData);
+};
 
 class ThreadSafeNetworkDiskCache : public QAbstractNetworkCache {
  public:
@@ -65,80 +69,4 @@ class ThreadSafeNetworkDiskCache : public QAbstractNetworkCache {
   static QNetworkDiskCache *sCache;
 };
 
-class NetworkAccessManager : public QNetworkAccessManager {
-  Q_OBJECT
-
- public:
-  explicit NetworkAccessManager(QObject *parent = nullptr);
-
- protected:
-  QNetworkReply *createRequest(Operation op, const QNetworkRequest &request, QIODevice *outgoingData);
-};
-
-class RedirectFollower : public QObject {
-  Q_OBJECT
-
- public:
-  explicit RedirectFollower(QNetworkReply *first_reply, int max_redirects = 5);
-
-  bool hit_redirect_limit() const { return redirects_remaining_ < 0; }
-  QNetworkReply *reply() const { return current_reply_; }
-
-  // These are all forwarded to the current reply.
-  QNetworkReply::NetworkError error() const { return current_reply_->error(); }
-  QString errorString() const { return current_reply_->errorString(); }
-  QVariant attribute(QNetworkRequest::Attribute code) const { return current_reply_->attribute(code); }
-  QVariant header(QNetworkRequest::KnownHeaders header) const { return current_reply_->header(header); }
-  qint64 bytesAvailable() const { return current_reply_->bytesAvailable(); }
-  QUrl url() const { return current_reply_->url(); }
-  QByteArray readAll() { return current_reply_->readAll(); }
-  void abort() { current_reply_->abort(); }
-
-signals:
-  // These are all forwarded from the current reply.
-  void readyRead();
-  void error(QNetworkReply::NetworkError);
-  void uploadProgress(qint64 bytesSent, qint64 bytesTotal);
-  void downloadProgress(qint64 bytesReceived, qint64 bytesTotal);
-
-  // This is NOT emitted when a request that has a redirect finishes.
-  void finished();
-
- private slots:
-  void ReadyRead();
-  void ReplyFinished();
-
- private:
-  void ConnectReply(QNetworkReply *reply);
-
- private:
-  QNetworkReply *current_reply_;
-  int redirects_remaining_;
-};
-
-class NetworkTimeouts : public QObject {
-  Q_OBJECT
-
- public:
-  explicit NetworkTimeouts(int timeout_msec, QObject *parent = nullptr);
-
-  // TODO: Template this to avoid code duplication.
-  void AddReply(QNetworkReply *reply);
-  void AddReply(RedirectFollower *reply);
-  void SetTimeout(int msec) { timeout_msec_ = msec; }
-
- protected:
-  void timerEvent(QTimerEvent *e);
-
- private slots:
-  void ReplyFinished();
-  void RedirectFinished(RedirectFollower *redirect);
-
- private:
-  int timeout_msec_;
-  QMap<QNetworkReply*, int> timers_;
-  QMap<RedirectFollower*, int> redirect_timers_;
-};
-
 #endif  // NETWORK_H
-

src/core/networktimeouts.cpp

@@ -0,0 +1,90 @@
+/*
+ * Strawberry Music Player
+ * This file was part of Clementine.
+ * Copyright 2010, David Sansome <me@davidsansome.com>
+ *
+ * Strawberry is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Strawberry is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Strawberry.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "config.h"
+
+#include <QtGlobal>
+#include <QObject>
+#include <QUrl>
+#include <QNetworkRequest>
+#include <QNetworkReply>
+#include <QTimerEvent>
+
+#include "core/closure.h"
+#include "networktimeouts.h"
+#include "redirectfollower.h"
+
+NetworkTimeouts::NetworkTimeouts(int timeout_msec, QObject *parent)
+    : QObject(parent), timeout_msec_(timeout_msec) {}
+
+void NetworkTimeouts::AddReply(QNetworkReply *reply) {
+
+  if (timers_.contains(reply)) return;
+
+  connect(reply, SIGNAL(destroyed()), SLOT(ReplyFinished()));
+  connect(reply, SIGNAL(finished()), SLOT(ReplyFinished()));
+  timers_[reply] = startTimer(timeout_msec_);
+
+}
+
+void NetworkTimeouts::AddReply(RedirectFollower *reply) {
+
+  if (redirect_timers_.contains(reply)) {
+    return;
+  }
+
+  NewClosure(reply, SIGNAL(destroyed()), this, SLOT(RedirectFinished(RedirectFollower*)), reply);
+  NewClosure(reply, SIGNAL(finished()), this, SLOT(RedirectFinished(RedirectFollower*)), reply);
+  redirect_timers_[reply] = startTimer(timeout_msec_);
+
+}
+
+void NetworkTimeouts::ReplyFinished() {
+
+  QNetworkReply *reply = reinterpret_cast<QNetworkReply*>(sender());
+  if (timers_.contains(reply)) {
+    killTimer(timers_.take(reply));
+  }
+
+}
+
+void NetworkTimeouts::RedirectFinished(RedirectFollower *reply) {
+
+  if (redirect_timers_.contains(reply)) {
+    killTimer(redirect_timers_.take(reply));
+  }
+
+}
+
+void NetworkTimeouts::timerEvent(QTimerEvent *e) {
+
+  QNetworkReply *reply = timers_.key(e->timerId());
+  if (reply) {
+    reply->abort();
+  }
+
+  RedirectFollower *redirect = redirect_timers_.key(e->timerId());
+  if (redirect) {
+    redirect->abort();
+  }
+
+}
+
+

src/core/networktimeouts.h

@@ -0,0 +1,60 @@
+/*
+ * Strawberry Music Player
+ * This file was part of Clementine.
+ * Copyright 2010, David Sansome <me@davidsansome.com>
+ *
+ * Strawberry is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Strawberry is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Strawberry.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef NETWORKTIMEOUTS_H
+#define NETWORKTIMEOUTS_H
+
+#include "config.h"
+
+#include <stdbool.h>
+
+#include <QtGlobal>
+#include <QObject>
+#include <QMap>
+
+class QNetworkReply;
+class QTimerEvent;
+class RedirectFollower;
+
+class NetworkTimeouts : public QObject {
+  Q_OBJECT
+
+ public:
+  explicit NetworkTimeouts(int timeout_msec, QObject *parent = nullptr);
+
+  // TODO: Template this to avoid code duplication.
+  void AddReply(QNetworkReply *reply);
+  void AddReply(RedirectFollower *reply);
+  void SetTimeout(int msec) { timeout_msec_ = msec; }
+
+ protected:
+  void timerEvent(QTimerEvent *e);
+
+ private slots:
+  void ReplyFinished();
+  void RedirectFinished(RedirectFollower *redirect);
+
+ private:
+  int timeout_msec_;
+  QMap<QNetworkReply*, int> timers_;
+  QMap<RedirectFollower*, int> redirect_timers_;
+};
+
+#endif  // NETWORKTIMEOUTS_H

src/core/redirectfollower.cpp

@@ -0,0 +1,79 @@
+/*
+ * Strawberry Music Player
+ * This file was part of Clementine.
+ * Copyright 2010, David Sansome <me@davidsansome.com>
+ *
+ * Strawberry is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Strawberry is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Strawberry.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "config.h"
+
+#include <QtGlobal>
+#include <QObject>
+#include <QUrl>
+#include <QNetworkRequest>
+#include <QNetworkReply>
+
+#include "redirectfollower.h"
+
+RedirectFollower::RedirectFollower(QNetworkReply *first_reply, int max_redirects) : QObject(nullptr), current_reply_(first_reply), redirects_remaining_(max_redirects) {
+  ConnectReply(first_reply);
+}
+
+void RedirectFollower::ConnectReply(QNetworkReply *reply) {
+
+  connect(reply, SIGNAL(readyRead()), SLOT(ReadyRead()));
+  connect(reply, SIGNAL(error(QNetworkReply::NetworkError)), SIGNAL(error(QNetworkReply::NetworkError)));
+  connect(reply, SIGNAL(downloadProgress(qint64,qint64)), SIGNAL(downloadProgress(qint64,qint64)));
+  connect(reply, SIGNAL(uploadProgress(qint64,qint64)), SIGNAL(uploadProgress(qint64,qint64)));
+  connect(reply, SIGNAL(finished()), SLOT(ReplyFinished()));
+
+}
+
+void RedirectFollower::ReadyRead() {
+
+  // Don't re-emit this signal for redirect replies.
+  if (current_reply_->attribute(QNetworkRequest::RedirectionTargetAttribute).isValid()) {
+    return;
+  }
+
+  emit readyRead();
+
+}
+
+void RedirectFollower::ReplyFinished() {
+
+  current_reply_->deleteLater();
+
+  if (current_reply_->attribute(QNetworkRequest::RedirectionTargetAttribute).isValid()) {
+    if (redirects_remaining_-- == 0) {
+      emit finished();
+      return;
+    }
+
+    const QUrl next_url = current_reply_->url().resolved(current_reply_->attribute(QNetworkRequest::RedirectionTargetAttribute).toUrl());
+
+    QNetworkRequest req(current_reply_->request());
+    req.setUrl(next_url);
+
+    current_reply_ = current_reply_->manager()->get(req);
+    ConnectReply(current_reply_);
+    return;
+  }
+
+  emit finished();
+
+}
+

src/core/redirectfollower.h

@@ -0,0 +1,78 @@
+/*
+ * Strawberry Music Player
+ * This file was part of Clementine.
+ * Copyright 2010, David Sansome <me@davidsansome.com>
+ *
+ * Strawberry is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Strawberry is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Strawberry.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef REDIRECTFOLLOWER_H
+#define REDIRECTFOLLOWER_H
+
+#include "config.h"
+
+#include <stdbool.h>
+
+#include <QtGlobal>
+#include <QObject>
+#include <QByteArray>
+#include <QVariant>
+#include <QString>
+#include <QUrl>
+#include <QNetworkRequest>
+#include <QNetworkReply>
+
+class RedirectFollower : public QObject {
+  Q_OBJECT
+
+ public:
+  explicit RedirectFollower(QNetworkReply *first_reply, int max_redirects = 5);
+
+  bool hit_redirect_limit() const { return redirects_remaining_ < 0; }
+  QNetworkReply *reply() const { return current_reply_; }
+
+  // These are all forwarded to the current reply.
+  QNetworkReply::NetworkError error() const { return current_reply_->error(); }
+  QString errorString() const { return current_reply_->errorString(); }
+  QVariant attribute(QNetworkRequest::Attribute code) const { return current_reply_->attribute(code); }
+  QVariant header(QNetworkRequest::KnownHeaders header) const { return current_reply_->header(header); }
+  qint64 bytesAvailable() const { return current_reply_->bytesAvailable(); }
+  QUrl url() const { return current_reply_->url(); }
+  QByteArray readAll() { return current_reply_->readAll(); }
+  void abort() { current_reply_->abort(); }
+
+signals:
+  // These are all forwarded from the current reply.
+  void readyRead();
+  void error(QNetworkReply::NetworkError);
+  void uploadProgress(qint64 bytesSent, qint64 bytesTotal);
+  void downloadProgress(qint64 bytesReceived, qint64 bytesTotal);
+
+  // This is NOT emitted when a request that has a redirect finishes.
+  void finished();
+
+ private slots:
+  void ReadyRead();
+  void ReplyFinished();
+
+ private:
+  void ConnectReply(QNetworkReply *reply);
+
+ private:
+  QNetworkReply *current_reply_;
+  int redirects_remaining_;
+};
+
+#endif  // REDIRECTFOLLOWER_H

src/covermanager/albumcoverfetchersearch.cpp

@@ -38,6 +38,8 @@
 #include "core/closure.h"
 #include "core/logging.h"
 #include "core/network.h"
+#include "core/networktimeouts.h"
+#include "core/redirectfollower.h"
 #include "albumcoverfetcher.h"
 #include "albumcoverfetchersearch.h"
 #include "coverprovider.h"

src/musicbrainz/acoustidclient.cpp

@@ -45,6 +45,7 @@
 #include "acoustidclient.h"
 #include "core/closure.h"
 #include "core/network.h"
+#include "core/networktimeouts.h"
 #include "core/timeconstants.h"
 #include "core/logging.h"
 

src/musicbrainz/musicbrainzclient.cpp

@@ -46,6 +46,7 @@
 #include "core/closure.h"
 #include "core/logging.h"
 #include "core/network.h"
+#include "core/networktimeouts.h"
 #include "core/utilities.h"
 #include "musicbrainzclient.h"