diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp index 8c81d641d..e7e31f5a2 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -55,7 +56,14 @@ CardPictureLoader::CardPictureLoader() : QObject(nullptr) CardPictureLoader::~CardPictureLoader() { - worker->deleteLater(); + if (worker) { + // Capture the thread first: shutdownThread() blocks until the worker has been freed by the + // finished() -> deleteLater chain, after which the worker pointer must not be dereferenced. + QThread *pictureLoaderThread = worker->workerThread(); + worker->shutdownThread(); + worker = nullptr; + delete pictureLoaderThread; + } } void CardPictureLoader::getCardBackPixmap(QPixmap &pixmap, QSize size) @@ -295,7 +303,15 @@ void CardPictureLoader::clearPixmapCache() void CardPictureLoader::clearNetworkCache() { - getInstance().worker->clearNetworkCache(); + auto &worker = *getInstance().worker; + // The disk cache and redirect cache are owned by the worker thread; clearing them from the + // UI thread would race with the worker's cache reads/writes. Block until the worker thread + // has executed the clear so the "Cached card pictures have been reset." message is truthful. + if (worker.isRunning()) { + QMetaObject::invokeMethod(&worker, "clearNetworkCache", Qt::BlockingQueuedConnection); + } else { + worker.clearNetworkCache(); + } } void CardPictureLoader::cacheCardPixmaps(const QList &cards) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp index 4a2caaab4..daac1bff5 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp @@ -16,15 +16,16 @@ #include #include -static constexpr int MAX_REQUESTS_PER_SEC = 10; -static constexpr int MIN_HOST_QUOTA = 1; ///< Floor for the per-host request allowance +static constexpr int MAX_REQUESTS_PER_SEC = DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT; +static constexpr int MIN_HOST_QUOTA = DownloadSettings::MIN_HOST_REQUEST_LIMIT; static constexpr qint64 QUOTA_RECOVER_MS = 60000; ///< Idle time before a reduced quota starts recovering static constexpr int DISPATCH_INTERVAL_MS = 100; ///< Pacing between individual network requests static constexpr qint64 QUOTA_RESET_INTERVAL_MS = 1000; ///< Interval at which the request quota resets CardPictureLoaderWorker::CardPictureLoaderWorker() : QObject(nullptr), picDownload(SettingsCache::instance().downloads().getPicDownload()), - requestQuota(MAX_REQUESTS_PER_SEC) + requestQuota(MAX_REQUESTS_PER_SEC), + hostRequestLimits(SettingsCache::instance().downloads().getHostRequestLimits()) { networkManager = new QNetworkAccessManager(this); // We need a timeout to ensure requests don't hang indefinitely in case of @@ -59,6 +60,9 @@ CardPictureLoaderWorker::CardPictureLoaderWorker() localLoader = new CardPictureLoaderLocal(this); pictureLoaderThread = new QThread; + // The worker object frees itself once its thread finishes, so no event loop is left + // running and the QThread is never destroyed while still executing. + connect(pictureLoaderThread, &QThread::finished, this, &QObject::deleteLater); pictureLoaderThread->start(QThread::LowPriority); moveToThread(pictureLoaderThread); @@ -74,12 +78,36 @@ CardPictureLoaderWorker::CardPictureLoaderWorker() connect(&dispatchTimer, &QTimer::timeout, this, &CardPictureLoaderWorker::dispatchQueuedRequest); dispatchTimer.setInterval(DISPATCH_INTERVAL_MS); + + connect(&SettingsCache::instance().downloads(), &DownloadSettings::hostRequestLimitsChanged, this, + [this] { hostRequestLimits = SettingsCache::instance().downloads().getHostRequestLimits(); }); } CardPictureLoaderWorker::~CardPictureLoaderWorker() { saveRedirectCache(); - pictureLoaderThread->deleteLater(); +} + +void CardPictureLoaderWorker::shutdownThread() +{ + // The finished() -> deleteLater chain (wired in the constructor) frees this worker as soon as + // its event loop exits, so nothing - not even a member read - may run once wait() returns. + // QThread::quit() and QThread::wait() are thread-safe and may be called from the owning thread. + QThread *thread = pictureLoaderThread; + if (thread) { + thread->quit(); + thread->wait(); + } +} + +QThread *CardPictureLoaderWorker::workerThread() const +{ + return pictureLoaderThread; +} + +bool CardPictureLoaderWorker::isRunning() const +{ + return pictureLoaderThread != nullptr && pictureLoaderThread->isRunning(); } void CardPictureLoaderWorker::queueRequest(const QUrl &url, CardPictureLoaderWorkerWork *worker) @@ -138,17 +166,24 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture void CardPictureLoaderWorker::resetRequestQuota() { requestQuota = MAX_REQUESTS_PER_SEC; + // Allowances are seeded per host on demand in processSingleRequest(), so a + // rate-limited host never gets a fresh full quota mid-second. + hostQuotaRemaining.clear(); QDateTime now = QDateTime::currentDateTime(); - for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end(); ++it) { - if (!hostLast429.contains(it.key()) || now.msecsTo(hostLast429.value(it.key())) < -QUOTA_RECOVER_MS) { - it.value() = qMin(MAX_REQUESTS_PER_SEC, it.value() + 1); + for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end();) { + // An unlocked host has no per-host allowance; drop any stale entry instead of + // recovering it towards the UNLIMITED_HOST_QUOTA sentinel, which would poison it. + if (hostAllowanceCeiling(it.key()) == DownloadSettings::UNLIMITED_HOST_QUOTA) { + it = hostRequestQuota.erase(it); + continue; } - } - - for (const auto &request : requestLoadQueue) { - const QString host = request.first.host(); - hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC)); + if (!hostLast429.contains(it.key()) || now.msecsTo(hostLast429.value(it.key())) < -QUOTA_RECOVER_MS) { + // Recover towards the host's effective allowance ceiling, which may be + // lowered by the user's per-host request limits. + it.value() = qMin(hostAllowanceCeiling(it.key()), it.value() + 1); + } + ++it; } processQueuedRequests(); @@ -169,6 +204,27 @@ void CardPictureLoaderWorker::processQueuedRequests() void CardPictureLoaderWorker::dispatchQueuedRequest() { + if (requestLoadQueue.isEmpty()) { + dispatchTimer.stop(); + return; + } + + // Unlocked hosts (developer cap UNLIMITED_HOST_QUOTA) skip the dispatch pacing and the + // global per-second quota: dispatch every queued request for them back-to-back, bounded + // only by their 429 backoff window and Qt's per-host connection pool. + QDateTime now = QDateTime::currentDateTime(); + for (int i = 0; i < requestLoadQueue.size();) { + const auto &request = requestLoadQueue.at(i); + const QString host = request.first.host(); + if (hostAllowanceCeiling(host) == DownloadSettings::UNLIMITED_HOST_QUOTA && + !CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) { + makeRequest(request.first, request.second); + requestLoadQueue.removeAt(i); + } else { + ++i; + } + } + if (requestLoadQueue.isEmpty() || requestQuota <= 0) { dispatchTimer.stop(); return; @@ -184,10 +240,22 @@ void CardPictureLoaderWorker::dispatchQueuedRequest() bool CardPictureLoaderWorker::processSingleRequest() { + QDateTime now = QDateTime::currentDateTime(); for (int i = 0; i < requestLoadQueue.size(); ++i) { const auto &request = requestLoadQueue.at(i); - QString host = request.first.host(); - int allowance = hostQuotaRemaining.value(host, MAX_REQUESTS_PER_SEC); + const QString host = request.first.host(); + // Don't dispatch requests to a host that is currently in its 429 backoff. + if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) { + continue; + } + const int ceiling = hostAllowanceCeiling(host); + // Seed the allowance only now, so a host that was rate limited last second + // doesn't get a fresh full quota the moment it is queried mid-second. Clamp + // against the ceiling so a lowered user cap applies from this second onward. + if (!hostQuotaRemaining.contains(host)) { + hostQuotaRemaining.insert(host, qMin(ceiling, hostRequestQuota.value(host, ceiling))); + } + int allowance = hostQuotaRemaining.value(host); if (allowance > 0) { hostQuotaRemaining.insert(host, allowance - 1); makeRequest(request.first, request.second); @@ -198,9 +266,25 @@ bool CardPictureLoaderWorker::processSingleRequest() return false; } +int CardPictureLoaderWorker::hostAllowanceCeiling(const QString &host) const +{ + const int devCap = DownloadSettings::getDeveloperHostCaps().value(host, MAX_REQUESTS_PER_SEC); + if (devCap == DownloadSettings::UNLIMITED_HOST_QUOTA && !hostRequestLimits.contains(host)) { + return DownloadSettings::UNLIMITED_HOST_QUOTA; + } + const int requested = hostRequestLimits.value(host, devCap); + return SettingsCache::instance().downloads().clampHostRequestLimit(host, requested); +} + void CardPictureLoaderWorker::onHostRateLimited(const QString &host) { - hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC) / 2)); + const int ceiling = hostAllowanceCeiling(host); + if (ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA) { + // Unlocked hosts have no per-host allowance to halve; the shared backoff + // window tracked by the rate limiter still paces them. + return; + } + hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, ceiling) / 2)); hostLast429.insert(host, QDateTime::currentDateTime()); } diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h index 9f7fd9437..96405252f 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.h @@ -74,10 +74,37 @@ public: */ void onHostRateLimited(const QString &host); - /** @brief Clears the network cache and redirect cache. */ - void clearNetworkCache(); + /** + * @brief Stops the worker thread and releases it. + * + * Called from the owning thread (CardPictureLoader) on its way out. QThread::quit() posts an + * exit request to the worker's event loop and QThread::wait() blocks until the loop has + * returned and the thread finished. Only QThread members are touched here, so this method is + * safe to call from the owning thread. The worker object itself is freed by the finished() -> + * deleteLater chain (see the constructor); the QThread object is deleted afterwards by the + * owner (CardPictureLoader::~CardPictureLoader), not by this method. + */ + void shutdownThread(); + + /** @return Whether the worker's thread is currently running. */ + bool isRunning() const; + + /** + * @brief Returns the worker's QThread. + * @return The worker thread + * + * Only meaningful while the worker object is alive; capture it before calling shutdownThread(). + */ + QThread *workerThread() const; public slots: + /** + * @brief Clears the network cache and redirect cache. + * + * Runs on the worker thread; invoke it via a queued call when coming from another thread, + * since both caches are owned by the worker thread. + */ + void clearNetworkCache(); /** * @brief Makes a network request for the given URL using the specified worker. * @param url URL to load @@ -125,12 +152,21 @@ private: QTimer requestTimer; ///< Timer to reset the request quota QTimer dispatchTimer; ///< Timer pacing individual network requests QHash hostRequestQuota; ///< Sustained per-host request allowance + QHash hostRequestLimits; ///< User-set per-host request allowances QHash hostQuotaRemaining; ///< Per-host allowance left in the current second QHash hostLast429; ///< When each host was last rate limited CardPictureLoaderLocal *localLoader; ///< Loader for local images QSet currentlyLoading; ///< Deduplication: contains pixmapCacheKey currently being loaded + /** + * @brief Effective per-host allowance ceiling for a host. + * @param host The host to look up + * @return The allowance ceiling in requests/second, or DownloadSettings::UNLIMITED_HOST_QUOTA + * when the developer unlocked the host and no user limit is set for it. + */ + [[nodiscard]] int hostAllowanceCeiling(const QString &host) const; + /** @brief Returns cached redirect URL for the given original URL, if available. */ [[nodiscard]] QUrl getCachedRedirect(const QUrl &originalUrl) const; diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp index 66c56337c..072a919d7 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp @@ -22,6 +22,11 @@ static const QStringList MD5_BLACKLIST = { "fbc7d763c08771c260b39e2115414eeb" // Current card back hash }; +ServerRateLimiter &CardPictureLoaderWorkerWork::rateLimiter() +{ + return s_rateLimiter; +} + CardPictureLoaderWorkerWork::CardPictureLoaderWorkerWork(const CardPictureLoaderWorker *worker, const ExactCard &toLoad) : QObject(nullptr), cardToDownload(CardPictureToLoad(toLoad)), picDownload(SettingsCache::instance().downloads().getPicDownload()) diff --git a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h index 1e56a4373..8490cb3ac 100644 --- a/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h +++ b/cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h @@ -43,6 +43,9 @@ public: CardPictureToLoad cardToDownload; ///< The card and associated URLs to try downloading + /** @brief Shared per-server 429 backoff state. */ + static ServerRateLimiter &rateLimiter(); + public slots: /** * @brief Handles a finished network reply for the card image. diff --git a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp index f3eac05b8..6223cd480 100644 --- a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp +++ b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.cpp @@ -11,11 +11,14 @@ #include #include #include +#include #include #include #include #include +static constexpr int UNLOCKED_HOST_LIMIT_MAX = 50; ///< Upper bound for rate limits on hosts unlocked by the developer + DeckEditorSettingsPage::DeckEditorSettingsPage() { picDownloadCheckBox.setChecked(SettingsCache::instance().downloads().getPicDownload()); @@ -65,11 +68,16 @@ DeckEditorSettingsPage::DeckEditorSettingsPage() aRemove->setIcon(themePixmap(QStringLiteral("icons/decrement"))); connect(aRemove, &QAction::triggered, this, &DeckEditorSettingsPage::actRemoveURL); + aRateLimit = new QAction(this); + aRateLimit->setIcon(themePixmap(QStringLiteral("icons/cogwheel"))); + connect(aRateLimit, &QAction::triggered, this, &DeckEditorSettingsPage::actAdjustRateLimit); + auto *urlToolBar = new QToolBar; urlToolBar->setOrientation(Qt::Vertical); urlToolBar->addAction(aAdd); urlToolBar->addAction(aRemove); urlToolBar->addAction(aEdit); + urlToolBar->addAction(aRateLimit); urlToolBar->setSizePolicy(QSizePolicy::Preferred, QSizePolicy::MinimumExpanding); auto *urlListLayout = new QHBoxLayout; @@ -164,6 +172,54 @@ void DeckEditorSettingsPage::storeSettings() SettingsCache::instance().downloads().setDownloadUrls(downloadUrls); } +void DeckEditorSettingsPage::actAdjustRateLimit() +{ + if (urlList->currentItem() == nullptr) { + QMessageBox::information(this, tr("Adjust Rate Limit"), tr("Select a URL in the list first.")); + return; + } + + const QString host = QUrl(urlList->currentItem()->text()).host(); + if (host.isEmpty()) { + QMessageBox::information(this, tr("Adjust Rate Limit"), tr("The selected URL does not have a valid host.")); + return; + } + + const QHash &devCaps = DownloadSettings::getDeveloperHostCaps(); + const QHash currentLimits = SettingsCache::instance().downloads().getHostRequestLimits(); + const int devCap = devCaps.value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT); + const bool unlocked = devCap == DownloadSettings::UNLIMITED_HOST_QUOTA; + + bool ok = false; + int minimum; + int maximum; + int defaultValue; + if (unlocked) { + minimum = 0; // 0 means "unlimited" + maximum = UNLOCKED_HOST_LIMIT_MAX; + defaultValue = currentLimits.value(host, 0); + } else { + minimum = DownloadSettings::MIN_HOST_REQUEST_LIMIT; + maximum = devCap; + defaultValue = currentLimits.value(host, devCap); + } + + const int value = QInputDialog::getInt(this, tr("Adjust Rate Limit for %1").arg(host), + tr("Requests per second (developer maximum is %1):").arg(maximum), + defaultValue, minimum, maximum, 1, &ok); + if (!ok) { + return; + } + + QHash limits = currentLimits; + if (unlocked ? value == 0 : value == devCap) { + limits.remove(host); + } else { + limits.insert(host, value); + } + SettingsCache::instance().downloads().setHostRequestLimits(limits); +} + void DeckEditorSettingsPage::urlListChanged(const QModelIndex &, int, int, const QModelIndex &, int) { storeSettings(); @@ -244,4 +300,5 @@ void DeckEditorSettingsPage::retranslateUi() aAdd->setText(tr("Add New URL")); aEdit->setText(tr("Edit URL")); aRemove->setText(tr("Remove URL")); -} \ No newline at end of file + aRateLimit->setText(tr("Adjust Rate Limit")); +} diff --git a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h index 5db009c8a..57de5699e 100644 --- a/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h +++ b/cockatrice/src/interface/widgets/settings_page/deck_editor_settings_page.h @@ -27,6 +27,7 @@ private slots: void actAddURL(); void actRemoveURL(); void actEditURL(); + void actAdjustRateLimit(); void resetDownloadedURLsButtonClicked(); private: @@ -34,7 +35,7 @@ private: QLabel urlLinkLabel; QCheckBox picDownloadCheckBox; QListWidget *urlList; - QAction *aAdd, *aEdit, *aRemove; + QAction *aAdd, *aEdit, *aRemove, *aRateLimit; QCheckBox mcDownloadSpoilersCheckBox; QLabel msDownloadSpoilersLabel; QGroupBox *mpGeneralGroupBox; diff --git a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp b/libcockatrice_settings/libcockatrice/settings/download_settings.cpp index cfa1c054e..5293e390b 100644 --- a/libcockatrice_settings/libcockatrice/settings/download_settings.cpp +++ b/libcockatrice_settings/libcockatrice/settings/download_settings.cpp @@ -9,6 +9,22 @@ const QStringList DownloadSettings::DEFAULT_DOWNLOAD_URLS = { "https://gatherer.wizards.com/Handlers/Image.ashx?multiverseid=!set:muid!&type=card", "https://gatherer.wizards.com/Handlers/Image.ashx?name=!name!&type=card"}; +// Developer-set ceilings for the per-host request allowance. Users may lower a host's +// allowance via the download settings, but can never raise it above these values. Hosts +// not listed default to DEFAULT_HOST_REQUEST_LIMIT. A cap of UNLIMITED_HOST_QUOTA marks a +// host that is never throttled per host and skips the dispatch pacing (429 backoff still applies). +const QHash DownloadSettings::DEVELOPER_HOST_CAPS = { + // The Scryfall API enforces 10 requests/second; stay one under so a burst can't trip 429s. + {"api.scryfall.com", 9}, + // The Scryfall image CDN has no documented per-client rate limit. + {"cards.scryfall.io", UNLIMITED_HOST_QUOTA}, +}; + +const QHash &DownloadSettings::getDeveloperHostCaps() +{ + return DEVELOPER_HOST_CAPS; +} + DownloadSettings::DownloadSettings(const QString &settingPath, QObject *parent = nullptr) : SettingsManager(settingPath + "downloads.ini", "downloads", QString(), parent) { @@ -50,3 +66,32 @@ void DownloadSettings::setDownloadSpoilerStatus(bool _spoilerStatus) setValue(_spoilerStatus, "downloadSpoilers"); emit downloadSpoilerStatusChanged(); } + +QHash DownloadSettings::getHostRequestLimits() const +{ + const QVariantMap stored = getValue("hostRequestLimits").toMap(); + QHash hostRequestLimits; + for (auto it = stored.cbegin(); it != stored.cend(); ++it) { + hostRequestLimits.insert(it.key(), it.value().toInt()); + } + return hostRequestLimits; +} + +void DownloadSettings::setHostRequestLimits(const QHash &hostRequestLimits) +{ + QVariantMap stored; + for (auto it = hostRequestLimits.cbegin(); it != hostRequestLimits.cend(); ++it) { + stored.insert(it.key(), it.value()); + } + setValue(stored, "hostRequestLimits"); + emit hostRequestLimitsChanged(); +} + +int DownloadSettings::clampHostRequestLimit(const QString &host, int requested) const +{ + const int devCap = DEVELOPER_HOST_CAPS.value(host, DEFAULT_HOST_REQUEST_LIMIT); + if (devCap == UNLIMITED_HOST_QUOTA) { + return qMax(MIN_HOST_REQUEST_LIMIT, requested); + } + return qBound(MIN_HOST_REQUEST_LIMIT, requested, devCap); +} diff --git a/libcockatrice_settings/libcockatrice/settings/download_settings.h b/libcockatrice_settings/libcockatrice/settings/download_settings.h index a3a6f4ca9..9fcf9e61d 100644 --- a/libcockatrice_settings/libcockatrice/settings/download_settings.h +++ b/libcockatrice_settings/libcockatrice/settings/download_settings.h @@ -9,14 +9,34 @@ #include "settings_manager.h" +#include + class DownloadSettings : public SettingsManager { Q_OBJECT friend class SettingsCache; static const QStringList DEFAULT_DOWNLOAD_URLS; + static const QHash DEVELOPER_HOST_CAPS; public: + /** @brief Per-host request allowance (requests/second) when no developer cap applies. */ + static constexpr int DEFAULT_HOST_REQUEST_LIMIT = 10; + /** @brief Floor for any per-host request allowance. */ + static constexpr int MIN_HOST_REQUEST_LIMIT = 1; + /** @brief Developer cap marking a host as never throttled per host or by the dispatch pacing. */ + static constexpr int UNLIMITED_HOST_QUOTA = -1; + + /** + * @brief Developer-set per-host allowance ceilings (requests/second), keyed by host. + * + * Hosts not present default to DEFAULT_HOST_REQUEST_LIMIT. An entry of + * UNLIMITED_HOST_QUOTA marks a host that users may still lower, but that is never + * throttled per host by default. Users can never raise a host's allowance above its + * developer cap. + */ + static const QHash &getDeveloperHostCaps(); + explicit DownloadSettings(const QString &, QObject *); QStringList getAllURLs() const; @@ -27,9 +47,23 @@ public: [[nodiscard]] bool getDownloadSpoilersStatus() const; void setDownloadSpoilerStatus(bool _spoilerStatus); + /** @brief User-set per-host request allowances (requests/second). Missing hosts use the developer default. */ + QHash getHostRequestLimits() const; + void setHostRequestLimits(const QHash &hostRequestLimits); + + /** + * @brief Clamps the user's requested allowance for a host against its developer cap. + * @param host The host to clamp for + * @param requested The user-requested allowance in requests/second + * @return The effective allowance. Users may lower a host's allowance but never raise it + * above the developer cap; hosts with UNLIMITED_HOST_QUOTA have no upper bound. + */ + [[nodiscard]] int clampHostRequestLimit(const QString &host, int requested) const; + signals: void picDownloadChanged(); void downloadSpoilerStatusChanged(); + void hostRequestLimitsChanged(); }; #endif // COCKATRICE_DOWNLOADSETTINGS_H diff --git a/tests/settings/settings_defaults_test.cpp b/tests/settings/settings_defaults_test.cpp index eda778ae9..932ddb99b 100644 --- a/tests/settings/settings_defaults_test.cpp +++ b/tests/settings/settings_defaults_test.cpp @@ -334,6 +334,38 @@ TEST_F(SettingsDefaultsTest, Download_DownloadSpoilersStatus_Default) ASSERT_EQ(s.getDownloadSpoilersStatus(), false); } +TEST_F(SettingsDefaultsTest, Download_HostRequestLimits_Default) +{ + DownloadSettings s(settingsPath, nullptr); + ASSERT_TRUE(s.getHostRequestLimits().isEmpty()); +} + +TEST_F(SettingsDefaultsTest, Download_HostRequestLimits_SetAndGet) +{ + DownloadSettings s(settingsPath, nullptr); + s.setHostRequestLimits({{"api.scryfall.com", 5}}); + const QHash limits = s.getHostRequestLimits(); + ASSERT_EQ(limits.size(), 1); + ASSERT_EQ(limits.value("api.scryfall.com"), 5); +} + +TEST_F(SettingsDefaultsTest, Download_HostRequestLimits_StackedHostCaps) +{ + DownloadSettings s(settingsPath, nullptr); + // The developer cap for the Scryfall API lowers the ceiling to 9; a user can + // reduce it further but can never raise it above the cap. + ASSERT_EQ(s.clampHostRequestLimit("api.scryfall.com", 9), 9); + ASSERT_EQ(s.clampHostRequestLimit("api.scryfall.com", 20), 9); + ASSERT_EQ(s.clampHostRequestLimit("api.scryfall.com", 5), 5); + ASSERT_EQ(s.clampHostRequestLimit("api.scryfall.com", 0), 1); + // Hosts without a developer cap fall back to the global default ceiling. + ASSERT_EQ(s.clampHostRequestLimit("gatherer.wizards.com", 10), 10); + ASSERT_EQ(s.clampHostRequestLimit("gatherer.wizards.com", 20), 10); + // The Scryfall CDN is unlocked: no upper bound (values are only floored). + ASSERT_EQ(s.clampHostRequestLimit("cards.scryfall.io", 20), 20); + ASSERT_EQ(s.clampHostRequestLimit("cards.scryfall.io", 0), 1); +} + // --- AppearanceSettings --- TEST_F(SettingsDefaultsTest, Appearance_ThemeName_Default)