Simplify network access

include/core/network.h

@@ -4,18 +4,18 @@
 /*
     The two classes defined here provide a simplified interface for Qt's network classes QNetworkAccessManager and QNetworkReply.
 
-    With the Qt classes, the workflow for a GET is roughly: generate a QNetworkRequest, give this request to QNetworkAccessManager::get,
+    With the Qt classes, the workflow for a GET is roughly: create a QNetworkAccessManager, generate a QNetworkRequest, give this request to the manager,
     connect the returned object to QNetworkReply::finished, and in the slot of that connection handle the various HTTP headers and attributes,
     then manage errors or process the webpage's body.
 
     These classes handle generating the QNetworkRequest with a given URL and manage the HTTP headers in the reply. They will automatically
     respect rate limits and return cached data if the webpage hasn't changed since previous requests. Instead of interacting with a QNetworkReply,
-    callers interact with a simplified NetworkReplyData.
+    callers interact with a simplified NetworkReplyData. Per Qt's manual, a single QNetworkAccessManager instance is sufficient for a whole application,
+    and this manager will be created internally when the first network request is made.
     Example that logs Porymap's description on GitHub:
 
-    NetworkAccessManager * manager = new NetworkAccessManager(this);
-    NetworkReplyData * reply = manager->get("https://api.github.com/repos/huderlem/porymap");
-    connect(reply, &NetworkReplyData::finished, [reply] () {
+    NetworkReplyData * reply = Network::get("https://api.github.com/repos/huderlem/porymap");
+    connect(reply, &NetworkReplyData::received, [reply] () {
         if (!reply->errorString().isEmpty()) {
             logError(QString("Failed to read description: %1").arg(reply->errorString()));
         } else {
@@ -31,6 +31,7 @@
 #include <QNetworkRequest>
 #include <QNetworkReply>
 #include <QDateTime>
+#include <QPointer>
 #endif
 
 #ifdef QT_NETWORK_LIB
@@ -45,8 +46,9 @@ public:
     QByteArray body() const { return m_body; }
     QString errorString() const { return m_error; }
     QDateTime retryAfter() const { return m_retryAfter; }
-    bool isFinished() const { return m_finished; }
+    bool isReceived() const { return m_received; }
 
+    friend class Network;
     friend class NetworkAccessManager;
 
 private:
@@ -55,15 +57,15 @@ private:
     QByteArray m_body;
     QString m_error;
     QDateTime m_retryAfter;
-    bool m_finished;
+    bool m_received = false;
 
     void finish() {
-        m_finished = true;
-        emit finished();
+        m_received = true;
+        emit received();
     };
 
 signals:
-    void finished();
+    void received();
 };
 
 class NetworkAccessManager : public QNetworkAccessManager
@@ -73,8 +75,10 @@ class NetworkAccessManager : public QNetworkAccessManager
 public:
     NetworkAccessManager(QObject * parent = nullptr);
     ~NetworkAccessManager();
-    NetworkReplyData * get(const QString &url);
-    NetworkReplyData * get(const QUrl &url);
+
+    static QPointer<NetworkAccessManager> instance();
+
+    friend class Network;
 
 private:
     // For a more complex cache we could implement a QAbstractCache for the manager
@@ -88,6 +92,13 @@ private:
     const QNetworkRequest getRequest(const QUrl &url);
 };
 
+class Network
+{
+public:
+    static NetworkReplyData * get(const QString &url);
+    static NetworkReplyData * get(const QUrl &url);
+};
+
 #endif // QT_NETWORK_LIB
 
 #endif // NETWORK_H

include/mainwindow.h

@@ -329,7 +329,6 @@ private:
 
 #ifdef QT_NETWORK_LIB
     QPointer<UpdatePromoter> updatePromoter = nullptr;
-    QPointer<NetworkAccessManager> networkAccessManager = nullptr;
 #endif
 
     QPointer<AboutPorymap> aboutWindow = nullptr;
@@ -429,6 +428,7 @@ private:
     void initMapList();
     void initShortcuts();
     void initExtraShortcuts();
+    void initModuleUI();
     void loadUserSettings();
     void resizeWithinScreen();
     void setTheme(QString);

include/ui/updatepromoter.h

@@ -18,7 +18,7 @@ class UpdatePromoter : public QDialog
     Q_OBJECT
 
 public:
-    explicit UpdatePromoter(QWidget *parent, NetworkAccessManager *manager);
+    explicit UpdatePromoter(QWidget *parent);
     ~UpdatePromoter();
 
     void checkForUpdates();
@@ -26,7 +26,6 @@ public:
 
 private:
     Ui::UpdatePromoter *ui;
-    NetworkAccessManager *const manager;
     QPushButton * button_Downloads;
     QPushButton * button_Retry;
 

src/core/network.cpp

@@ -17,29 +17,25 @@ NetworkAccessManager::~NetworkAccessManager() {
     qDeleteAll(this->cache);
 }
 
-const QNetworkRequest NetworkAccessManager::getRequest(const QUrl &url) {
-    QNetworkRequest request(url);
-
-    // Set User-Agent to porymap/#.#.#
-    request.setHeader(QNetworkRequest::UserAgentHeader, QString("%1/%2").arg(QCoreApplication::applicationName())
-                                                                        .arg(QCoreApplication::applicationVersion()));
-
-    // If we've made a successful request in this session already, set the If-None-Match header.
-    // We'll only get a full response from the server if the data has changed since this last request.
-    // This helps to avoid hitting rate limits.
-    auto cacheEntry = this->cache.value(url, nullptr);
-    if (cacheEntry)
-        request.setHeader(QNetworkRequest::IfNoneMatchHeader, cacheEntry->eTag);
-
-    return request;
+QPointer<NetworkAccessManager> NetworkAccessManager::instance() {
+    static QPointer<NetworkAccessManager> manager = nullptr;
+    if (!manager) manager = new NetworkAccessManager(qApp);
+    return manager;
 }
 
-NetworkReplyData * NetworkAccessManager::get(const QString &url) {
-    return this->get(QUrl(url));
+NetworkReplyData * Network::get(const QString &url) {
+    return Network::get(QUrl(url));
 }
 
-NetworkReplyData * NetworkAccessManager::get(const QUrl &url) {
-    NetworkReplyData * data = new NetworkReplyData();
+NetworkReplyData * Network::get(const QUrl &url) {
+    auto manager = NetworkAccessManager::instance();
+    if (!manager) {
+        Q_ASSERT("Failed to create NetworkAccessManager");
+        return nullptr;
+    }
+
+    // Caller's responsibility to delete
+    auto data = new NetworkReplyData();
     data->m_url = url;
 
     // If we are rate-limited, don't send a new request.
@@ -56,15 +52,32 @@ NetworkReplyData * NetworkAccessManager::get(const QUrl &url) {
         porymapConfig.rateLimitTimes.remove(url);
     }
 
-    QNetworkReply * reply = QNetworkAccessManager::get(this->getRequest(url));
-    connect(reply, &QNetworkReply::finished, [this, reply, data] {
-        this->processReply(reply, data);
+    QNetworkReply * reply = manager->get(manager->getRequest(url));
+    QObject::connect(reply, &QNetworkReply::finished, [manager, reply, data] {
+        manager->processReply(reply, data);
         data->finish();
     });
 
     return data;
 }
 
+const QNetworkRequest NetworkAccessManager::getRequest(const QUrl &url) {
+    QNetworkRequest request(url);
+
+    // Set User-Agent to porymap/#.#.#
+    request.setHeader(QNetworkRequest::UserAgentHeader, QString("%1/%2").arg(QCoreApplication::applicationName())
+                                                                        .arg(QCoreApplication::applicationVersion()));
+
+    // If we've made a successful request in this session already, set the If-None-Match header.
+    // We'll only get a full response from the server if the data has changed since this last request.
+    // This helps to avoid hitting rate limits.
+    auto cacheEntry = this->cache.value(url, nullptr);
+    if (cacheEntry)
+        request.setHeader(QNetworkRequest::IfNoneMatchHeader, cacheEntry->eTag);
+
+    return request;
+}
+
 void NetworkAccessManager::processReply(QNetworkReply * reply, NetworkReplyData * data) {
     if (!reply || !reply->isFinished())
         return;

src/mainwindow.cpp

@@ -55,17 +55,6 @@
 
 #include <math.h>
 
-// We only publish release binaries for Windows and macOS.
-// This is relevant for the update promoter, which alerts users of a new release.
-#if defined(Q_OS_WIN) || defined(Q_OS_MACOS)
-#define RELEASE_PLATFORM
-#endif
-#if defined(QT_NETWORK_LIB) && defined(RELEASE_PLATFORM)
-#define USE_UPDATE_PROMOTER
-#endif
-
-
-
 MainWindow::MainWindow(QWidget *parent) :
     QMainWindow(parent),
     ui(new Ui::MainWindow),
@@ -187,32 +176,37 @@ void MainWindow::setWindowDisabled(bool disabled) {
 void MainWindow::initWindow() {
     porymapConfig = PorymapConfig();
     porymapConfig.load();
-    this->initLogStatusBar();
-    this->initCustomUI();
-    this->initExtraSignals();
-    this->initEditor();
-    this->initMiscHeapObjects();
-    this->initMapList();
-    this->initShortcuts();
+    initLogStatusBar();
+    initCustomUI();
+    initExtraSignals();
+    initEditor();
+    initMiscHeapObjects();
+    initMapList();
+    initShortcuts();
+    initModuleUI();
+    setWindowDisabled(true);
+}
 
-    QStringList missingModules;
+void MainWindow::initModuleUI() {
+        QStringList missingModules;
 
-#ifndef USE_UPDATE_PROMOTER
+// Fully hide "Check for Updates" on platforms we don't publish releases for
+#if !(defined(Q_OS_WIN) || defined(Q_OS_MACOS))
     ui->actionCheck_for_Updates->setVisible(false);
-#ifdef RELEASE_PLATFORM
-    // Only report the network module missing if we would
-    // have otherwise used it (we don't on non-release platforms).
-    missingModules.append(" 'network'");
 #endif
+
+#ifndef QT_NETWORK_LIB
+    ui->actionCheck_for_Updates->setEnabled(false);
+    missingModules.append(" 'network'");
 #endif
 
 #ifndef QT_CHARTS_LIB
-    ui->pushButton_SummaryChart->setVisible(false);
+    ui->pushButton_SummaryChart->setEnabled(false);
     missingModules.append(" 'charts'");
 #endif
 
 #ifndef QT_QML_LIB
-    ui->actionPlugins->setVisible(false);
+    ui->actionPlugins->setEnabled(false);
     missingModules.append(" 'qml'");
 #endif
 
@@ -221,8 +215,6 @@ void MainWindow::initWindow() {
                             .arg(missingModules.length() > 1 ? "s" : "")
                             .arg(missingModules.join(",")));
     }
-
-    setWindowDisabled(true);
 }
 
 void MainWindow::initShortcuts() {
@@ -397,13 +389,10 @@ void MainWindow::on_actionCheck_for_Updates_triggered() {
     checkForUpdates(true);
 }
 
-#ifdef USE_UPDATE_PROMOTER
+#ifdef QT_NETWORK_LIB
 void MainWindow::checkForUpdates(bool requestedByUser) {
-    if (!this->networkAccessManager)
-        this->networkAccessManager = new NetworkAccessManager(this);
-
     if (!this->updatePromoter) {
-        this->updatePromoter = new UpdatePromoter(this, this->networkAccessManager);
+        this->updatePromoter = new UpdatePromoter(this);
         connect(this->updatePromoter, &UpdatePromoter::changedPreferences, [this] {
             if (this->preferenceEditor)
                 this->preferenceEditor->updateFields();
@@ -3065,7 +3054,7 @@ void MainWindow::on_actionPreferences_triggered() {
 void MainWindow::togglePreferenceSpecificUi() {
     ui->actionOpen_Project_in_Text_Editor->setEnabled(!porymapConfig.textEditorOpenFolder.isEmpty());
 
-#ifdef USE_UPDATE_PROMOTER
+#ifdef QT_NETWORK_LIB
     if (this->updatePromoter)
         this->updatePromoter->updatePreferences();
 #endif

src/ui/updatepromoter.cpp

@@ -11,10 +11,9 @@
 #include <QDesktopServices>
 #include <QTimer>
 
-UpdatePromoter::UpdatePromoter(QWidget *parent, NetworkAccessManager *manager)
+UpdatePromoter::UpdatePromoter(QWidget *parent)
     : QDialog(parent),
-      ui(new Ui::UpdatePromoter),
-      manager(manager)
+      ui(new Ui::UpdatePromoter)
 {
     ui->setupUi(this);
 
@@ -71,8 +70,8 @@ void UpdatePromoter::checkForUpdates() {
 
 void UpdatePromoter::get(const QUrl &url) {
     this->visitedUrls.insert(url);
-    auto reply = this->manager->get(url);
-    connect(reply, &NetworkReplyData::finished, [this, reply] () {
+    auto reply = Network::get(url);
+    connect(reply, &NetworkReplyData::received, [this, reply] () {
         if (!reply->errorString().isEmpty()) {
             this->error(reply->errorString(), reply->retryAfter());
         } else {