From 7386af700c3ce3928853399324f96c8f0c691d06 Mon Sep 17 00:00:00 2001 From: GriffinR Date: Fri, 6 Mar 2026 17:52:36 -0500 Subject: [PATCH] Simplify network access --- include/core/network.h | 35 +++++++++++++++-------- include/mainwindow.h | 2 +- include/ui/updatepromoter.h | 3 +- src/core/network.cpp | 57 +++++++++++++++++++++++-------------- src/mainwindow.cpp | 57 +++++++++++++++---------------------- src/ui/updatepromoter.cpp | 9 +++--- 6 files changed, 87 insertions(+), 76 deletions(-) diff --git a/include/core/network.h b/include/core/network.h index 23dc7eff..346f53fe 100644 --- a/include/core/network.h +++ b/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 #include #include +#include #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 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 diff --git a/include/mainwindow.h b/include/mainwindow.h index 8f57c511..96a0f913 100644 --- a/include/mainwindow.h +++ b/include/mainwindow.h @@ -329,7 +329,6 @@ private: #ifdef QT_NETWORK_LIB QPointer updatePromoter = nullptr; - QPointer networkAccessManager = nullptr; #endif QPointer aboutWindow = nullptr; @@ -429,6 +428,7 @@ private: void initMapList(); void initShortcuts(); void initExtraShortcuts(); + void initModuleUI(); void loadUserSettings(); void resizeWithinScreen(); void setTheme(QString); diff --git a/include/ui/updatepromoter.h b/include/ui/updatepromoter.h index 76157c4d..945f3746 100644 --- a/include/ui/updatepromoter.h +++ b/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; diff --git a/src/core/network.cpp b/src/core/network.cpp index ce0d5124..00776b82 100644 --- a/src/core/network.cpp +++ b/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::instance() { + static QPointer 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; diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 6936db9d..c0556ad1 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -55,17 +55,6 @@ #include -// 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 diff --git a/src/ui/updatepromoter.cpp b/src/ui/updatepromoter.cpp index cea5b753..a9da1de5 100644 --- a/src/ui/updatepromoter.cpp +++ b/src/ui/updatepromoter.cpp @@ -11,10 +11,9 @@ #include #include -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 {