[PictureLoader] Add user-configurable per-host request caps

Picture downloads were throttled to a uniform 10 requests/second per host
with no way to tune a specific server. A rate-limited API host (Scryfall
caps at 10 req/s) can trip 429s during bursts, and CDN hosts with no rate
limit were throttled needlessly.

Introduce developer-owned per-host caps that users can only ever lower,
never raise, exposed in the download settings page:
- DownloadSettings::DEVELOPER_HOST_CAPS sets the ceiling per host
  (api.scryfall.com 9, cards.scryfall.io unlimited, others 10).
- A new hostRequestLimits setting stores user overrides in downloads.ini;
  clampHostRequestLimit() bounds them to [1, devCap] so a user can reduce
  api.scryfall.com to 5 but never raise it above 9.
- The picture worker seeds, halves on 429, and recovers its sustained
  per-host allowance against the effective ceiling instead of the global
  maximum, and skips per-host accounting entirely for unlocked hosts
  (cards.scryfall.io) while global pacing and 429 backoff still apply.
- The deck editor settings page gains one spinbox per known host, each
  clamped to its developer cap.
This commit is contained in:
Lukas Brübach 2026-09-12 16:58:10 +02:00 committed by BruebachL
parent e3820c3f34
commit da307a82b3
7 changed files with 236 additions and 6 deletions

View file

@ -16,15 +16,16 @@
#include <utility>
#include <version_string.h>
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
@ -74,6 +75,9 @@ 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()
@ -145,7 +149,9 @@ void CardPictureLoaderWorker::resetRequestQuota()
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);
// 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);
}
}
@ -190,10 +196,18 @@ bool CardPictureLoaderWorker::processSingleRequest()
if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) {
continue;
}
const int ceiling = hostAllowanceCeiling(host);
// Unlocked hosts (developer cap UNLIMITED_HOST_QUOTA) skip the per-host allowance
// entirely; only the global quota and request pacing still apply.
if (ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA) {
makeRequest(request.first, request.second);
requestLoadQueue.removeAt(i);
return true;
}
// 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.
if (!hostQuotaRemaining.contains(host)) {
hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC));
hostQuotaRemaining.insert(host, hostRequestQuota.value(host, ceiling));
}
int allowance = hostQuotaRemaining.value(host);
if (allowance > 0) {
@ -206,9 +220,24 @@ 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));
if (hostAllowanceCeiling(host) == 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, hostAllowanceCeiling(host)) / 2));
hostLast429.insert(host, QDateTime::currentDateTime());
}

View file

@ -125,12 +125,21 @@ private:
QTimer requestTimer; ///< Timer to reset the request quota
QTimer dispatchTimer; ///< Timer pacing individual network requests
QHash<QString, int> hostRequestQuota; ///< Sustained per-host request allowance
QHash<QString, int> hostRequestLimits; ///< User-set per-host request allowances
QHash<QString, int> hostQuotaRemaining; ///< Per-host allowance left in the current second
QHash<QString, QDateTime> hostLast429; ///< When each host was last rate limited
CardPictureLoaderLocal *localLoader; ///< Loader for local images
QSet<QString> 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;

View file

@ -10,12 +10,17 @@
#include <QInputDialog>
#include <QLineEdit>
#include <QMessageBox>
#include <QSet>
#include <QToolBar>
#include <QUrl>
#include <algorithm>
#include <libcockatrice/settings/download_settings.h>
#include <libcockatrice/settings/paths_settings.h>
#include <libcockatrice/settings/personal_settings.h>
#include <libcockatrice/utility/macros.h>
static constexpr int UNLOCKED_HOST_LIMIT_MAX = 50; ///< Upper bound for hosts unlocked by the developer
DeckEditorSettingsPage::DeckEditorSettingsPage()
{
picDownloadCheckBox.setChecked(SettingsCache::instance().downloads().getPicDownload());
@ -96,6 +101,53 @@ DeckEditorSettingsPage::DeckEditorSettingsPage()
&DownloadSettings::setDownloadSpoilerStatus);
connect(&mcDownloadSpoilersCheckBox, &QCheckBox::toggled, this, &DeckEditorSettingsPage::setSpoilersEnabled);
// Per-host request limit group: one spinbox per known picture host. A spinbox at its
// lower bound (0 for unlocked hosts, the developer cap for capped hosts) means "follow
// the developer default"; the worker clamps any explicit value against the developer cap.
mpRequestLimitGroupBox = new QGroupBox;
auto *requestLimitLayout = new QGridLayout;
const QHash<QString, int> &devCaps = DownloadSettings::getDeveloperHostCaps();
const QHash<QString, int> userLimits = SettingsCache::instance().downloads().getHostRequestLimits();
QSet<QString> hosts;
for (const QString &urlTemplate : SettingsCache::instance().downloads().getAllURLs()) {
hosts.insert(QUrl(urlTemplate).host());
}
const QList<QString> devHosts = devCaps.keys();
for (const QString &devHost : devHosts) {
hosts.insert(devHost);
}
QList<QString> sortedHosts(hosts.cbegin(), hosts.cend());
std::sort(sortedHosts.begin(), sortedHosts.end(),
[](const QString &a, const QString &b) { return a.localeAwareCompare(b) < 0; });
int hostRow = 1;
for (const QString &host : sortedHosts) {
const int devCap = devCaps.value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT);
const bool unlocked = devCap == DownloadSettings::UNLIMITED_HOST_QUOTA;
auto *hostLabel = new QLabel(host);
auto *spinBox = new QSpinBox;
if (unlocked) {
spinBox->setRange(0, UNLOCKED_HOST_LIMIT_MAX); // 0 means "unlimited"
spinBox->setValue(userLimits.value(host, 0));
} else {
spinBox->setRange(DownloadSettings::MIN_HOST_REQUEST_LIMIT, devCap);
spinBox->setValue(userLimits.value(host, devCap));
}
connect(spinBox, &QSpinBox::valueChanged, this, &DeckEditorSettingsPage::storeRequestLimits);
requestLimitLayout->addWidget(hostLabel, hostRow, 0);
requestLimitLayout->addWidget(spinBox, hostRow, 1);
requestLimitSpinBoxes.insert(host, spinBox);
++hostRow;
}
requestLimitLayout->addWidget(&requestLimitHelpLabel, hostRow, 0, 1, 2);
mpRequestLimitGroupBox->setLayout(requestLimitLayout);
mpGeneralGroupBox = new QGroupBox;
mpGeneralGroupBox->setLayout(lpGeneralGrid);
@ -104,6 +156,7 @@ DeckEditorSettingsPage::DeckEditorSettingsPage()
auto *lpMainLayout = new QVBoxLayout;
lpMainLayout->addWidget(mpGeneralGroupBox);
lpMainLayout->addWidget(mpRequestLimitGroupBox);
lpMainLayout->addWidget(mpSpoilerGroupBox);
setLayout(lpMainLayout);
@ -164,6 +217,24 @@ void DeckEditorSettingsPage::storeSettings()
SettingsCache::instance().downloads().setDownloadUrls(downloadUrls);
}
void DeckEditorSettingsPage::storeRequestLimits()
{
QHash<QString, int> stored;
const QHash<QString, int> &devCaps = DownloadSettings::getDeveloperHostCaps();
for (auto it = requestLimitSpinBoxes.cbegin(); it != requestLimitSpinBoxes.cend(); ++it) {
const QString host = it.key();
const int value = it.value()->value();
const int devCap = devCaps.value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT);
// Only values that differ from the developer default are persisted; the worker
// treats a missing entry as "follow the developer default".
const int developerDefault = devCap == DownloadSettings::UNLIMITED_HOST_QUOTA ? 0 : devCap;
if (value != developerDefault) {
stored.insert(host, value);
}
}
SettingsCache::instance().downloads().setHostRequestLimits(stored);
}
void DeckEditorSettingsPage::urlListChanged(const QModelIndex &, int, int, const QModelIndex &, int)
{
storeSettings();
@ -230,6 +301,9 @@ void DeckEditorSettingsPage::setSpoilersEnabled(bool anInput)
void DeckEditorSettingsPage::retranslateUi()
{
mpGeneralGroupBox->setTitle(tr("URL Download Priority"));
mpRequestLimitGroupBox->setTitle(tr("Per-Host Request Limit"));
requestLimitHelpLabel.setText(tr("Pictures per second per host. Hosts can be lowered below their developer "
"limit but never raised above it; 0 means the host is not throttled per host."));
mpSpoilerGroupBox->setTitle(tr("Spoilers"));
mcDownloadSpoilersCheckBox.setText(tr("Download Spoilers Automatically"));
mcSpoilerSaveLabel.setText(tr("Spoiler Location:"));

View file

@ -5,9 +5,11 @@
#include <QCheckBox>
#include <QGroupBox>
#include <QHash>
#include <QLabel>
#include <QListWidget>
#include <QPushButton>
#include <QSpinBox>
class DeckEditorSettingsPage : public AbstractSettingsPage
{
@ -19,6 +21,7 @@ public:
private slots:
void storeSettings();
void storeRequestLimits();
void urlListChanged(const QModelIndex &, int, int, const QModelIndex &, int);
void setSpoilersEnabled(bool);
void spoilerPathButtonClicked();
@ -40,6 +43,10 @@ private:
QGroupBox *mpGeneralGroupBox;
QGroupBox *mpSpoilerGroupBox;
QGroupBox *mpRequestLimitGroupBox;
QLabel requestLimitHelpLabel;
QHash<QString, QSpinBox *> requestLimitSpinBoxes;
QLineEdit *mpSpoilerSavePathLineEdit;
QLabel mcSpoilerSaveLabel;
QLabel lastUpdatedLabel;

View file

@ -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 (request pacing and 429 backoff still apply).
const QHash<QString, int> 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<QString, int> &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<QString, int> DownloadSettings::getHostRequestLimits() const
{
const QVariantMap stored = getValue("hostRequestLimits").toMap();
QHash<QString, int> hostRequestLimits;
for (auto it = stored.cbegin(); it != stored.cend(); ++it) {
hostRequestLimits.insert(it.key(), it.value().toInt());
}
return hostRequestLimits;
}
void DownloadSettings::setHostRequestLimits(const QHash<QString, int> &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);
}

View file

@ -9,14 +9,34 @@
#include "settings_manager.h"
#include <QHash>
class DownloadSettings : public SettingsManager
{
Q_OBJECT
friend class SettingsCache;
static const QStringList DEFAULT_DOWNLOAD_URLS;
static const QHash<QString, int> 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 (pacing still applies). */
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<QString, int> &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<QString, int> getHostRequestLimits() const;
void setHostRequestLimits(const QHash<QString, int> &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

View file

@ -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<QString, int> 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)