From cbc260646f9feeba7d4aa7d4563021855ad612ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= <Bruebach.Lukas@bdosecurity.de>
Date: Fri, 22 Nov 2024 20:03:02 +0100
Subject: [PATCH] Address comments.

---
 cockatrice/src/client/ui/picture_loader.cpp | 14 +++++++-------
 cockatrice/src/client/ui/picture_loader.h   |  7 ++++++-
 cockatrice/src/settings/cache_settings.cpp  |  2 +-
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/cockatrice/src/client/ui/picture_loader.cpp b/cockatrice/src/client/ui/picture_loader.cpp
index c363c861f..0e80c3d8b 100644
--- a/cockatrice/src/client/ui/picture_loader.cpp
+++ b/cockatrice/src/client/ui/picture_loader.cpp
@@ -147,7 +147,7 @@ PictureLoaderWorker::PictureLoaderWorker()
     networkManager->setRedirectPolicy(QNetworkRequest::ManualRedirectPolicy);
     connect(networkManager, SIGNAL(finished(QNetworkReply *)), this, SLOT(picDownloadFinished(QNetworkReply *)));
 
-    cacheFilePath = SettingsCache::instance().getRedirectCachePath() + "cacheSettings.ini";
+    cacheFilePath = SettingsCache::instance().getRedirectCachePath() + REDIRECT_CACHE_FILENAME;
     loadRedirectCache();
     cleanStaleEntries();
 
@@ -512,9 +512,9 @@ void PictureLoaderWorker::loadRedirectCache()
     int size = settings.beginReadArray("redirects");
     for (int i = 0; i < size; ++i) {
         settings.setArrayIndex(i);
-        QUrl originalUrl = settings.value("original").toUrl();
-        QUrl redirectUrl = settings.value("redirect").toUrl();
-        QDateTime timestamp = settings.value("timestamp").toDateTime();
+        QUrl originalUrl = settings.value(REDIRECT_ORIGINAL_URL).toUrl();
+        QUrl redirectUrl = settings.value(REDIRECT_URL).toUrl();
+        QDateTime timestamp = settings.value(REDIRECT_TIMESTAMP).toDateTime();
 
         if (originalUrl.isValid() && redirectUrl.isValid()) {
             redirectCache[originalUrl] = qMakePair(redirectUrl, timestamp);
@@ -532,9 +532,9 @@ void PictureLoaderWorker::saveRedirectCache() const
     int index = 0;
     for (auto it = redirectCache.cbegin(); it != redirectCache.cend(); ++it) {
         settings.setArrayIndex(index++);
-        settings.setValue("original", it.key());
-        settings.setValue("redirect", it.value().first);
-        settings.setValue("timestamp", it.value().second);
+        settings.setValue(REDIRECT_ORIGINAL_URL, it.key());
+        settings.setValue(REDIRECT_URL, it.value().first);
+        settings.setValue(REDIRECT_TIMESTAMP, it.value().second);
     }
     settings.endArray();
 }
diff --git a/cockatrice/src/client/ui/picture_loader.h b/cockatrice/src/client/ui/picture_loader.h
index 58e60acce..d8d8aea13 100644
--- a/cockatrice/src/client/ui/picture_loader.h
+++ b/cockatrice/src/client/ui/picture_loader.h
@@ -11,6 +11,11 @@ class QNetworkAccessManager;
 class QNetworkReply;
 class QThread;
 
+#define REDIRECT_ORIGINAL_URL "original"
+#define REDIRECT_URL "redirect"
+#define REDIRECT_TIMESTAMP "timestamp"
+#define REDIRECT_CACHE_FILENAME "cache.ini"
+
 class PictureToLoad
 {
 private:
@@ -85,7 +90,7 @@ private:
     QNetworkAccessManager *networkManager;
     QHash<QUrl, QPair<QUrl, QDateTime>> redirectCache; // Stores redirect and timestamp
     QString cacheFilePath;                             // Path to persistent storage
-    int CacheTTLInDays = 30;
+    static constexpr int CacheTTLInDays = 30;
     QList<PictureToLoad> cardsToDownload;
     PictureToLoad cardBeingLoaded;
     PictureToLoad cardBeingDownloaded;
diff --git a/cockatrice/src/settings/cache_settings.cpp b/cockatrice/src/settings/cache_settings.cpp
index 176f74f9b..27b60fb6a 100644
--- a/cockatrice/src/settings/cache_settings.cpp
+++ b/cockatrice/src/settings/cache_settings.cpp
@@ -1030,7 +1030,7 @@ void SettingsCache::loadPaths()
     replaysPath = getSafeConfigPath("paths/replays", dataPath + "/replays/");
     themesPath = getSafeConfigPath("paths/themes", dataPath + "/themes/");
     picsPath = getSafeConfigPath("paths/pics", dataPath + "/pics/");
-    redirectCachePath = getSafeConfigPath("paths/redirects", dataPath + "/pics/redirects/");
+    redirectCachePath = getSafeConfigPath("paths/redirects", getCachePath() + "/redirects/");
     // this has never been exposed as an user-configurable setting
     if (picsPath.endsWith("/")) {
         customPicsPath = getSafeConfigPath("paths/custompics", picsPath + "CUSTOM/");