[Network] Add a configured QNetworkAccessManager factory (#7628)

* [Network] Add a configured QNetworkAccessManager factory

Every HTTP caller built its own QNetworkAccessManager, so the system proxy
and a transfer timeout were easy to omit. Add NetworkManagerFactory::create()
as the single place that applies the process-wide proxy configuration and a
transfer timeout, and use it at all 14 construction sites.

Per-site settings such as disk caches and manual redirect policies stay with
their callers.

* [Tests] Link libcockatrice_network into the tests that compile its headers

release_channel.cpp and card_picture_loader_worker.cpp now include
libcockatrice/network/network_manager.h, but the test targets that compile
them only linked libcockatrice_settings, so the include directory never
reached them and every platform failed with "file not found":
loader_local_matching_test and picture_loader_benchmark_test.

libcockatrice_network is an INTERFACE library, so linking it costs these
targets nothing but hands over the include path.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

cockatrice/src/client/network/interfaces/deck_stats_interface.cpp

@@ -20,11 +20,12 @@
 #include <libcockatrice/card/database/card_database_manager.h>
 #include <libcockatrice/deck_list/deck_list.h>
 #include <libcockatrice/deck_list/tree/deck_list_card_node.h>
+#include <libcockatrice/network/network_manager.h>
 #include <version_string.h>
 
 DeckStatsInterface::DeckStatsInterface(QObject *parent) : QObject(parent)
 {
-    manager = new QNetworkAccessManager(this);
+    manager = NetworkManagerFactory::create(this);
     connect(manager, &QNetworkAccessManager::finished, this, &DeckStatsInterface::queryFinished);
 }
 

cockatrice/src/client/network/interfaces/tapped_out_interface.cpp

@@ -26,11 +26,12 @@
 #include <libcockatrice/card/database/card_database_manager.h>
 #include <libcockatrice/deck_list/deck_list.h>
 #include <libcockatrice/deck_list/tree/deck_list_card_node.h>
+#include <libcockatrice/network/network_manager.h>
 #include <version_string.h>
 
 TappedOutInterface::TappedOutInterface(QObject *parent) : QObject(parent)
 {
-    manager = new QNetworkAccessManager(this);
+    manager = NetworkManagerFactory::create(this);
     connect(manager, &QNetworkAccessManager::finished, this, &TappedOutInterface::queryFinished);
 }
 

cockatrice/src/client/network/update/card_spoiler/spoiler_background_updater.cpp

@@ -27,6 +27,7 @@
 #include <functional>
 #include <libcockatrice/card/database/card_database.h>
 #include <libcockatrice/card/database/card_database_manager.h>
+#include <libcockatrice/network/network_manager.h>
 #include <libcockatrice/settings/download_settings.h>
 #include <libcockatrice/settings/paths_settings.h>
 #include <tuple>
@@ -56,7 +57,7 @@ void SpoilerBackgroundUpdater::startSpoilerDownloadProcess(QString url, bool sav
 
 void SpoilerBackgroundUpdater::downloadFromURL(QUrl url, bool saveResults)
 {
-    auto *nam = new QNetworkAccessManager(this);
+    auto *nam = NetworkManagerFactory::create(this);
     auto request = QNetworkRequest(url);
     request.setHeader(QNetworkRequest::UserAgentHeader, QString("Cockatrice %1").arg(VERSION_STRING));
     QNetworkReply *reply = nam->get(request);

cockatrice/src/client/network/update/client/release_channel.cpp

@@ -20,6 +20,7 @@
 #include <QVariant>
 #include <QVariantMap>
 #include <QtGlobal>
+#include <libcockatrice/network/network_manager.h>
 #include <optional>
 #include <qnamespace.h>
 #include <qprocessordetection.h>
@@ -62,7 +63,7 @@ std::optional<int> getProductVersionMajor()
 
 } // namespace
 
-ReleaseChannel::ReleaseChannel() : netMan(new QNetworkAccessManager(this)), response(nullptr), lastRelease(nullptr)
+ReleaseChannel::ReleaseChannel() : netMan(NetworkManagerFactory::create(this)), response(nullptr), lastRelease(nullptr)
 {
 }
 

cockatrice/src/client/network/update/client/update_downloader.cpp

@@ -9,10 +9,11 @@
 #include <QNetworkRequest>
 #include <QUrl>
 #include <QVariant>
+#include <libcockatrice/network/network_manager.h>
 
 UpdateDownloader::UpdateDownloader(QObject *parent) : QObject(parent), response(nullptr)
 {
-    netMan = new QNetworkAccessManager(this);
+    netMan = NetworkManagerFactory::create(this);
 }
 
 void UpdateDownloader::beginDownload(QUrl downloadUrl)

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp

@@ -28,6 +28,7 @@
 #include <QVariant>
 #include <QtGlobal>
 #include <compare>
+#include <libcockatrice/network/network_manager.h>
 #include <libcockatrice/settings/cache_storage_settings.h>
 #include <libcockatrice/settings/download_settings.h>
 #include <libcockatrice/settings/paths_settings.h>
@@ -46,11 +47,10 @@ CardPictureLoaderWorker::CardPictureLoaderWorker()
     : QObject(nullptr), picDownload(SettingsCache::instance().downloads().getPicDownload()),
       hostRequestLimits(SettingsCache::instance().downloads().getHostRequestLimits())
 {
-    networkManager = new QNetworkAccessManager(this);
+    networkManager = NetworkManagerFactory::create(this);
     // We need a timeout to ensure requests don't hang indefinitely in case of
     // cache corruption, see related Qt bug: https://bugreports.qt.io/browse/QTBUG-111397
     // Use Qt's default timeout (30s, as of 2023-02-22)
-    networkManager->setTransferTimeout();
     cache = new QNetworkDiskCache(this);
     cache->setCacheDirectory(SettingsCache::instance().getNetworkCachePath());
     cache->setMaximumCacheSize(1024L * 1024L *

cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_website.cpp

@@ -27,6 +27,7 @@
 #include <QVBoxLayout>
 #include <functional>
 #include <libcockatrice/card/import/card_name_normalizer.h>
+#include <libcockatrice/network/network_manager.h>
 #include <qnamespace.h>
 #include <version_string.h>
 
@@ -34,7 +35,7 @@ class QWidget;
 
 DlgLoadDeckFromWebsite::DlgLoadDeckFromWebsite(QWidget *parent) : QDialog(parent)
 {
-    nam = new QNetworkAccessManager(this);
+    nam = NetworkManagerFactory::create(this);
 
     layout = new QVBoxLayout(this);
     setLayout(layout);

cockatrice/src/interface/widgets/server/handle_public_servers.cpp

@@ -13,13 +13,14 @@
 #include <QUrl>
 #include <QVariant>
 #include <QVariantMap>
+#include <libcockatrice/network/network_manager.h>
 #include <libcockatrice/settings/servers_settings.h>
 #include <qlogging.h>
 
 #define PUBLIC_SERVERS_JSON "https://cockatrice.github.io/public-servers.json"
 
 HandlePublicServers::HandlePublicServers(QObject *parent)
-    : QObject(parent), nam(new QNetworkAccessManager(this)), reply(nullptr)
+    : QObject(parent), nam(NetworkManagerFactory::create(this)), reply(nullptr)
 {
 }
 

cockatrice/src/interface/widgets/tabs/api/archidekt/display/archidekt_api_response_deck_listings_display_widget.cpp

@@ -11,6 +11,7 @@
 #include <QNetworkAccessManager>
 #include <QNetworkRequest>
 #include <QSlider>
+#include <libcockatrice/network/network_manager.h>
 #include <libcockatrice/utility/qt_utils.h>
 #include <qnamespace.h>
 
@@ -27,8 +28,7 @@ ArchidektApiResponseDeckListingsDisplayWidget::ArchidektApiResponseDeckListingsD
 
     cardSizeSlider->enableCtrlScrollResize(flowWidget);
 
-    imageNetworkManager = new QNetworkAccessManager(this);
-    imageNetworkManager->setTransferTimeout(); // Use Qt's default timeout
+    imageNetworkManager = NetworkManagerFactory::create(this);
     imageNetworkManager->setRedirectPolicy(QNetworkRequest::ManualRedirectPolicy);
 
     // Add widgets for deck listings

cockatrice/src/interface/widgets/tabs/api/archidekt/tab_archidekt.cpp

@@ -48,6 +48,7 @@
 #include <QWidget>
 #include <QtGlobal>
 #include <libcockatrice/card/database/card_database_manager.h>
+#include <libcockatrice/network/network_manager.h>
 #include <libcockatrice/settings/cards_display_settings.h>
 #include <libcockatrice/utility/qt_utils.h>
 #include <qlogging.h>
@@ -58,8 +59,7 @@ TabArchidekt::TabArchidekt(TabSupervisor *_tabSupervisor)
     : Tab(_tabSupervisor), currentPage(1), isLoadingMore(false), isListMode(true)
 {
     // Initialize network
-    networkManager = new QNetworkAccessManager(this);
-    networkManager->setTransferTimeout();
+    networkManager = NetworkManagerFactory::create(this);
     networkManager->setRedirectPolicy(QNetworkRequest::ManualRedirectPolicy);
     connect(networkManager, &QNetworkAccessManager::finished, this, &TabArchidekt::processApiJson);
 

cockatrice/src/interface/widgets/tabs/api/commander_spellbook/handle_commander_brackets.cpp

@@ -12,12 +12,13 @@
 #include <QStringLiteral>
 #include <QUrl>
 #include <QVariant>
+#include <libcockatrice/network/network_manager.h>
 #include <libcockatrice/settings/commander_bracket_settings.h>
 
 static const QUrl COMMANDER_BRACKET_JSON_URL(QStringLiteral("https://cockatrice.github.io/commander-brackets.json"));
 
 HandleCommanderBrackets::HandleCommanderBrackets(QObject *parent)
-    : QObject(parent), nam(new QNetworkAccessManager(this)), reply(nullptr)
+    : QObject(parent), nam(NetworkManagerFactory::create(this)), reply(nullptr)
 {
 }
 

cockatrice/src/interface/widgets/tabs/api/edhrec/tab_edhrec.cpp

@@ -18,14 +18,14 @@
 #include <QRegularExpression>
 #include <QSharedPointer>
 #include <QUrl>
+#include <libcockatrice/network/network_manager.h>
 #include <qlogging.h>
 
 class TabSupervisor;
 
 TabEdhRec::TabEdhRec(TabSupervisor *_tabSupervisor) : Tab(_tabSupervisor)
 {
-    networkManager = new QNetworkAccessManager(this);
-    networkManager->setTransferTimeout(); // Use Qt's default timeout
+    networkManager = NetworkManagerFactory::create(this);
     networkManager->setRedirectPolicy(QNetworkRequest::ManualRedirectPolicy);
     connect(networkManager, &QNetworkAccessManager::finished, this, &TabEdhRec::processApiJson);
 }

cockatrice/src/interface/widgets/tabs/api/edhrec/tab_edhrec_main.cpp

@@ -32,6 +32,7 @@
 #include <libcockatrice/models/database/card/card_search_model.h>
 #include <libcockatrice/models/database/card_database_display_model.h>
 #include <libcockatrice/models/database/card_database_model.h>
+#include <libcockatrice/network/network_manager.h>
 #include <libcockatrice/settings/cards_display_settings.h>
 #include <version_string.h>
 
@@ -44,8 +45,7 @@ static bool canBeCommander(const CardInfoPtr &cardInfo)
 
 TabEdhRecMain::TabEdhRecMain(TabSupervisor *_tabSupervisor) : Tab(_tabSupervisor)
 {
-    networkManager = new QNetworkAccessManager(this);
-    networkManager->setTransferTimeout(); // Use Qt's default timeout
+    networkManager = NetworkManagerFactory::create(this);
     networkManager->setRedirectPolicy(QNetworkRequest::ManualRedirectPolicy);
     connect(networkManager, SIGNAL(finished(QNetworkReply *)), this, SLOT(processApiJson(QNetworkReply *)));
 

libcockatrice_network/libcockatrice/network/network_manager.h

@@ -0,0 +1,38 @@
+#ifndef COCKATRICE_NETWORK_NETWORK_MANAGER_H
+#define COCKATRICE_NETWORK_NETWORK_MANAGER_H
+
+#include <QNetworkAccessManager>
+#include <QNetworkProxyFactory>
+#include <mutex>
+
+/**
+ * @file network_manager.h
+ * @ingroup Network
+ * @brief Creates QNetworkAccessManagers with the application-wide baseline.
+ *
+ * Every manager in the client should start from NetworkManagerFactory::create()
+ * so features that apply to all HTTP traffic (the system proxy, a transfer
+ * timeout) cannot be forgotten in one of the many call sites. Feature-specific
+ * setup such as a disk cache or a manual redirect policy stays with the caller.
+ */
+namespace NetworkManagerFactory
+{
+/**
+ * @brief Returns a QNetworkAccessManager configured for Cockatrice.
+ *
+ * The manager is parented to @p parent. The system proxy is enabled once,
+ * process-wide, the first time this function is called. Transfers use Qt's
+ * default timeout instead of waiting forever.
+ */
+inline QNetworkAccessManager *create(QObject *parent = nullptr)
+{
+    static std::once_flag systemProxyFlag;
+    std::call_once(systemProxyFlag, [] { QNetworkProxyFactory::setUseSystemConfiguration(true); });
+
+    auto *manager = new QNetworkAccessManager(parent);
+    manager->setTransferTimeout();
+    return manager;
+}
+} // namespace NetworkManagerFactory
+
+#endif // COCKATRICE_NETWORK_NETWORK_MANAGER_H

oracle/src/oraclewizard.cpp

@@ -22,6 +22,7 @@
 #include <QWindow>
 #include <QWizardPage>
 #include <QtGlobal>
+#include <libcockatrice/network/network_manager.h>
 #include <libcockatrice/settings/cards_display_settings.h>
 #include <libcockatrice/settings/personal_settings.h>
 #include <libcockatrice/utility/local_server_name.h>
@@ -53,7 +54,7 @@ OracleWizard::OracleWizard(QWidget *parent) : QWizard(parent)
     // foreignData for any other language is never imported.
     importer->setCardLang(SettingsCache::instance().cardsDisplay().getCardLang());
 
-    nam = new QNetworkAccessManager(this);
+    nam = NetworkManagerFactory::create(this);
 
     QList<OracleWizardPage *> pages;
 

tests/CMakeLists.txt

@@ -369,6 +369,7 @@ cockatrice_add_test(
   ${VERSION_STRING_CPP}
   loader_local_matching_test.cpp
   LIBS
+  libcockatrice_network
   libcockatrice_settings
   INCLUDE_DIRS
   ${CMAKE_SOURCE_DIR}/cockatrice/src

tests/picture_loader_benchmark/CMakeLists.txt

@@ -15,8 +15,9 @@ cockatrice_add_manual_test(
   settings_cache_mock.cpp
   LIBS
   libcockatrice_card
-  libcockatrice_settings
   libcockatrice_interfaces
+  libcockatrice_network
+  libcockatrice_settings
   libcockatrice_utility
   INCLUDE_DIRS
   ${CMAKE_SOURCE_DIR}/cockatrice/src