mirror of
https://github.com/Cockatrice/Cockatrice.git
synced 2026-09-21 17:15:09 -07:00
[PictureLoader] Add user-configurable per-host request caps (#7287)
* [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. * [PictureLoader] Let unlocked hosts skip dispatch pacing; adjust limits per URL Two refinements to the per-host request caps: - Unlocked hosts (UNLIMITED_HOST_QUOTA, e.g. cards.scryfall.io) no longer wait on the 100ms dispatch pacing or consume the global per-second quota. dispatchQueuedRequest fires their queued requests back-to-back, bounded only by their 429 backoff window and Qt's per-host connection pool, so an unthrottled CDN is not artificially slowed. - The deck editor download settings page replaces the static grid of one spinbox per known host with an "Adjust Rate Limit" toolbar action on the URL list. It picks the host out of the selected URL and clamps the entry against the developer cap table (including for user-added URLs). Also fixes a review finding: resetRequestQuota could write the UNLIMITED_HOST_QUOTA sentinel (-1) into the sustained per-host quota when a host became unlocked mid-run, permanently poisoning its allowance. Stale entries for unlocked hosts are now dropped, and the per-second seed is clamped against the effective ceiling so a lowered limit applies immediately. * [PictureLoader] Cap unlocked host bursts and adapt them to 429s * [PictureLoader] Store per-host limits readably and show them per URL * [PictureLoader] Make dispatch and rate-limit bookkeeping key on the real host Addresses ZeldaZach's round-4 review nits: - Dispatch now resolves the cached-redirect chain before the in-flight gate, so a redirect learned after a URL was queued can no longer bypass the MAX_IN_FLIGHT_PER_HOST cap and drain the whole queue onto the redirect target, which may carry its own developer cap. processSingleRequest does the same so the allowance math keys on the host that is actually hit. - The per-host in-flight slot is released when the reply is destroyed (with the worker as the connection context) rather than on a 'finished' connection bound to the work object, so an aborted reply or a work object deleted while a reply is pending can never permanently shrink the fast path's concurrency. - storeSettings only prunes limits for hosts with neither a URL nor a developer cap, so throttles on redirect targets (api.scryfall.com -> cards.scryfall.io) survive URL removal. - Unlocked hosts are offered 0..UNLOCKED_HOST_LIMIT_MAX (50) in the rate limit dialog, matching clampHostRequestLimit() and the documented hand-editable range, so values written into downloads.ini are no longer silently rewritten on the next edit. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
BruebachL
GitHub
parent
3d5eb84d81
commit
14acf3bf64
7 changed files with 496 additions and 34 deletions
|
|
@ -16,14 +16,15 @@
|
|||
#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())
|
||||
: QObject(nullptr), picDownload(SettingsCache::instance().downloads().getPicDownload()),
|
||||
hostRequestLimits(SettingsCache::instance().downloads().getHostRequestLimits())
|
||||
{
|
||||
networkManager = new QNetworkAccessManager(this);
|
||||
// We need a timeout to ensure requests don't hang indefinitely in case of
|
||||
|
|
@ -73,6 +74,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()
|
||||
|
|
@ -135,8 +139,21 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture
|
|||
|
||||
QNetworkReply *reply = networkManager->get(req);
|
||||
|
||||
// Connect reply handling
|
||||
connect(reply, &QNetworkReply::finished, worker, [reply, worker] { worker->handleNetworkReply(reply); });
|
||||
// Track in-flight replies per host so the unlocked fast path can bound how many requests it
|
||||
// issues at once, instead of creating replies that time out before Qt opens a connection.
|
||||
const QString host = url.host();
|
||||
hostInFlight.insert(host, hostInFlight.value(host) + 1);
|
||||
|
||||
// Release the in-flight slot when the reply is destroyed, not when it emits `finished`, and use
|
||||
// the worker (not the work object) as the context object: a reply can go away without ever
|
||||
// finishing (aborted, or a work object deleted while a reply is still pending), and a connection
|
||||
// bound to that work object's lifetime would then never run, permanently shrinking the fast
|
||||
// path's concurrency until it wedges. This way the slot is released exactly once.
|
||||
connect(reply, &QObject::destroyed, this,
|
||||
[this, host] { hostInFlight.insert(host, qMax(0, hostInFlight.value(host) - 1)); });
|
||||
|
||||
// Connect reply handling; the work object is the context so its handler dies with it.
|
||||
connect(reply, &QNetworkReply::finished, worker, [worker, reply] { worker->handleNetworkReply(reply); });
|
||||
|
||||
return reply;
|
||||
}
|
||||
|
|
@ -144,10 +161,23 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture
|
|||
void CardPictureLoaderWorker::resetRequestQuota()
|
||||
{
|
||||
QDateTime now = QDateTime::currentDateTime();
|
||||
for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end(); ++it) {
|
||||
for (auto it = hostRequestQuota.begin(); it != hostRequestQuota.end();) {
|
||||
if (!hostLast429.contains(it.key()) || now.msecsTo(hostLast429.value(it.key())) < -QUOTA_RECOVER_MS) {
|
||||
it.value() = qMin(MAX_REQUESTS_PER_SEC, it.value() + 1);
|
||||
if (hostAllowanceCeiling(it.key()) == DownloadSettings::UNLIMITED_HOST_QUOTA) {
|
||||
// A developer-unlocked host that fell back after a 429 recovers towards the default
|
||||
// allowance; once it gets there it becomes unlocked (fast-path) again.
|
||||
if (it.value() + 1 >= DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT) {
|
||||
it = hostRequestQuota.erase(it);
|
||||
continue;
|
||||
}
|
||||
it.value() += 1;
|
||||
} else {
|
||||
// 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;
|
||||
}
|
||||
|
||||
// Forget the per-second allowances; each host's allowance is re-seeded lazily from its
|
||||
|
|
@ -177,8 +207,53 @@ void CardPictureLoaderWorker::dispatchQueuedRequest()
|
|||
return;
|
||||
}
|
||||
|
||||
if (!processSingleRequest()) {
|
||||
// No queued host currently has allowance left in this second; wait for the quota reset.
|
||||
QDateTime now = QDateTime::currentDateTime();
|
||||
bool dispatched = false;
|
||||
// Set while an unlocked host still has queued work blocked only by the in-flight cap; the
|
||||
// timer must keep running so it gets another try as soon as a slot frees. A host blocked by
|
||||
// its 429 backoff instead waits for the next quota-reset tick to restart the dispatcher.
|
||||
bool unlockedCapped = false;
|
||||
|
||||
// Unlocked hosts (developer cap UNLIMITED_HOST_QUOTA) skip the pacing and the per-host
|
||||
// allowance: dispatch their queued requests back-to-back, bounded by their 429 backoff and the
|
||||
// per-host in-flight cap so a large burst can't queue replies that time out before Qt opens a
|
||||
// connection for them.
|
||||
for (int i = 0; i < requestLoadQueue.size();) {
|
||||
const auto &request = requestLoadQueue.at(i);
|
||||
// Dispatch decisions must key on the host the request will actually go to, not the URL that
|
||||
// merely redirects to it: a redirect learned after this URL was queued would otherwise
|
||||
// bypass the in-flight cap and drain the whole queue onto the target host unchecked.
|
||||
const QUrl resolvedUrl = resolveCachedRedirect(request.first);
|
||||
const QString host = resolvedUrl.host();
|
||||
if (isUnlockedHost(host)) {
|
||||
if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) {
|
||||
++i;
|
||||
continue;
|
||||
}
|
||||
if (hostInFlight.value(host) < MAX_IN_FLIGHT_PER_HOST) {
|
||||
makeRequest(resolvedUrl, request.second);
|
||||
requestLoadQueue.removeAt(i);
|
||||
dispatched = true;
|
||||
continue;
|
||||
}
|
||||
unlockedCapped = true;
|
||||
}
|
||||
++i;
|
||||
}
|
||||
|
||||
if (requestLoadQueue.isEmpty()) {
|
||||
dispatchTimer.stop();
|
||||
requestTimer.stop();
|
||||
return;
|
||||
}
|
||||
|
||||
if (processSingleRequest()) {
|
||||
dispatched = true;
|
||||
}
|
||||
|
||||
// Keep the timer running while there is progress to make or unlocked work waiting on a free
|
||||
// in-flight slot; otherwise no host has allowance left this second, so wait for the quota reset.
|
||||
if (!dispatched && !unlockedCapped) {
|
||||
dispatchTimer.stop();
|
||||
}
|
||||
}
|
||||
|
|
@ -221,8 +296,12 @@ bool CardPictureLoaderWorker::processSingleRequest()
|
|||
{
|
||||
QDateTime now = QDateTime::currentDateTime();
|
||||
for (int i = 0; i < requestLoadQueue.size(); ++i) {
|
||||
const auto &request = requestLoadQueue.at(i);
|
||||
const QString host = request.first.host();
|
||||
// Copy the entry: takeAt(i) below erases within the list this reference points into.
|
||||
const auto request = requestLoadQueue.at(i);
|
||||
// Resolve cached redirects so the rate-limit and allowance arithmetic keys on the host the
|
||||
// request will actually hit (see resolveCachedRedirect).
|
||||
const QUrl resolvedUrl = resolveCachedRedirect(request.first);
|
||||
const QString host = resolvedUrl.host();
|
||||
// Don't dispatch requests to a host that is currently in its 429 backoff; hand the entry
|
||||
// back to its worker so it can wait the backoff out or fall through to another source,
|
||||
// instead of leaving it parked in the queue with no reply pending. Only applies to
|
||||
|
|
@ -233,25 +312,39 @@ bool CardPictureLoaderWorker::processSingleRequest()
|
|||
// The queued URL is usually a cached-redirect target whose host differs from
|
||||
// cardToDownload.getCurrentUrl(), so scheduleDeferredRetry() (which waits out the
|
||||
// blocked host's deadline) is used instead of startNextPicDownload() looping on the
|
||||
// original host. Keep scanning so one backed-off entry doesn't monopolize the tick.
|
||||
// original host.
|
||||
auto entry = requestLoadQueue.takeAt(i);
|
||||
--i;
|
||||
entry.second->scheduleDeferredRetry(host);
|
||||
if (host != entry.first.host()) {
|
||||
// A cached redirect target is what is blocked, which the work object would not
|
||||
// discover from its own URL; wait out that specific host (with jitter) instead.
|
||||
entry.second->scheduleDeferredRetry(host);
|
||||
} else {
|
||||
entry.second->startNextPicDownload();
|
||||
}
|
||||
return true;
|
||||
}
|
||||
// Unlocked hosts are handled by dispatchQueuedRequest's fast path, bounded by the in-flight
|
||||
// cap; they must not fall through to the per-host allowance arithmetic below.
|
||||
if (isUnlockedHost(host)) {
|
||||
continue;
|
||||
}
|
||||
// Seed the allowance now so a host that was rate limited gets its reduced
|
||||
// allowance instead of a fresh full quota mid-second.
|
||||
int ceiling = hostAllowanceCeiling(host);
|
||||
if (ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA) {
|
||||
// A 429 dropped this unlocked host out of the fast path and installed a concrete
|
||||
// allowance; pace it against that allowance until the recovery loop unlocks it again.
|
||||
ceiling = hostRequestQuota.value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT);
|
||||
}
|
||||
// Seed the allowance lazily so a host that enters the queue mid-second gets its reduced
|
||||
// per-host allowance, clamped against the ceiling so a lowered user cap applies from this
|
||||
// second onward.
|
||||
if (!hostQuotaRemaining.contains(host)) {
|
||||
hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC));
|
||||
hostQuotaRemaining.insert(host, qMin(ceiling, hostRequestQuota.value(host, ceiling)));
|
||||
}
|
||||
int allowance = hostQuotaRemaining.value(host);
|
||||
if (allowance > 0) {
|
||||
hostQuotaRemaining.insert(host, allowance - 1);
|
||||
auto entry = requestLoadQueue.takeAt(i);
|
||||
// The allowance is only spent when a request is actually issued: makeRequest() returns
|
||||
// nullptr when the cached redirect target is in backoff and it hands the entry back.
|
||||
if (makeRequest(entry.first, entry.second)) {
|
||||
hostQuotaRemaining.insert(host, allowance - 1);
|
||||
}
|
||||
makeRequest(resolvedUrl, entry.second);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
|
@ -267,9 +360,30 @@ bool CardPictureLoaderWorker::requestTouchesNetwork(const QUrl &url) const
|
|||
return !useNetworkCache;
|
||||
}
|
||||
|
||||
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);
|
||||
}
|
||||
|
||||
bool CardPictureLoaderWorker::isUnlockedHost(const QString &host) const
|
||||
{
|
||||
return hostAllowanceCeiling(host) == DownloadSettings::UNLIMITED_HOST_QUOTA && !hostRequestQuota.contains(host);
|
||||
}
|
||||
|
||||
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);
|
||||
// An unlocked host has no per-host allowance to halve. Install one instead so it drops out of
|
||||
// the unlocked fast path and is paced like a throttled host; the recovery loop in
|
||||
// resetRequestQuota() then walks it back up and unlocks it again.
|
||||
const int base =
|
||||
ceiling == DownloadSettings::UNLIMITED_HOST_QUOTA ? DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT : ceiling;
|
||||
hostRequestQuota.insert(host, qMax(MIN_HOST_QUOTA, hostRequestQuota.value(host, base) / 2));
|
||||
hostLast429.insert(host, QDateTime::currentDateTime());
|
||||
}
|
||||
|
||||
|
|
@ -327,6 +441,22 @@ QUrl CardPictureLoaderWorker::getCachedRedirect(const QUrl &originalUrl) const
|
|||
return {};
|
||||
}
|
||||
|
||||
QUrl CardPictureLoaderWorker::resolveCachedRedirect(const QUrl &url) const
|
||||
{
|
||||
// Follow the whole cached-redirect chain so dispatch keys on the host that is really hit. The
|
||||
// depth bound keeps a corrupt or self-referencing cache entry from spinning us forever.
|
||||
QUrl resolved = url;
|
||||
int depth = 0;
|
||||
while (depth++ < MAX_REDIRECT_CHAIN_DEPTH) {
|
||||
QUrl target = getCachedRedirect(resolved);
|
||||
if (target.isEmpty() || target == resolved) {
|
||||
break;
|
||||
}
|
||||
resolved = target;
|
||||
}
|
||||
return resolved;
|
||||
}
|
||||
|
||||
void CardPictureLoaderWorker::loadRedirectCache()
|
||||
{
|
||||
QSettings settings(cacheFilePath, QSettings::IniFormat);
|
||||
|
|
|
|||
|
|
@ -124,19 +124,53 @@ 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
|
||||
QHash<QString, int> hostInFlight; ///< Network replies currently in flight, per host
|
||||
|
||||
/** @brief Maximum concurrent in-flight network replies per host. */
|
||||
static constexpr int MAX_IN_FLIGHT_PER_HOST = 6;
|
||||
|
||||
/** @brief Bound on how many cached-redirect hops dispatch resolution will follow. */
|
||||
static constexpr int MAX_REDIRECT_CHAIN_DEPTH = 10;
|
||||
|
||||
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 Whether a host may skip dispatch pacing and per-host allowance entirely.
|
||||
*
|
||||
* A host is unlocked while it has no user limit and no reduced allowance installed by a 429.
|
||||
* A 429 drops it out of the fast path until resetRequestQuota() walks the allowance back up.
|
||||
*/
|
||||
[[nodiscard]] bool isUnlockedHost(const QString &host) const;
|
||||
|
||||
/** @brief Returns cached redirect URL for the given original URL, if available. */
|
||||
[[nodiscard]] QUrl getCachedRedirect(const QUrl &originalUrl) const;
|
||||
|
||||
/** @brief Whether a request for this URL would actually touch the network, rather than being served from the disk
|
||||
/** @brief Whether a request for this URL would actually touch the network, rather than being served from the disk
|
||||
* cache. */
|
||||
[[nodiscard]] bool requestTouchesNetwork(const QUrl &url) const;
|
||||
|
||||
/**
|
||||
* @brief Follows the cached-redirect chain to the URL that will actually be requested.
|
||||
* @param url The URL to resolve
|
||||
* @return The final URL after chasing cached redirects, or @p url itself if none lead elsewhere
|
||||
*
|
||||
* Dispatch decisions (unlocked-host fast path, 429 backoff, in-flight cap) must key on the host
|
||||
* a request really goes to, not the URL that merely redirects to it.
|
||||
*/
|
||||
[[nodiscard]] QUrl resolveCachedRedirect(const QUrl &url) const;
|
||||
|
||||
/** @brief Loads redirect cache from disk. */
|
||||
void loadRedirectCache();
|
||||
|
||||
|
|
|
|||
|
|
@ -10,7 +10,9 @@
|
|||
#include <QInputDialog>
|
||||
#include <QLineEdit>
|
||||
#include <QMessageBox>
|
||||
#include <QSet>
|
||||
#include <QToolBar>
|
||||
#include <QUrl>
|
||||
#include <libcockatrice/settings/download_settings.h>
|
||||
#include <libcockatrice/settings/paths_settings.h>
|
||||
#include <libcockatrice/settings/personal_settings.h>
|
||||
|
|
@ -51,7 +53,9 @@ DeckEditorSettingsPage::DeckEditorSettingsPage()
|
|||
urlList->setDragDropMode(QAbstractItemView::InternalMove);
|
||||
connect(urlList->model(), &QAbstractItemModel::rowsMoved, this, &DeckEditorSettingsPage::urlListChanged);
|
||||
|
||||
urlList->addItems(SettingsCache::instance().downloads().getAllURLs());
|
||||
for (const QString &url : SettingsCache::instance().downloads().getAllURLs()) {
|
||||
addUrlItem(url);
|
||||
}
|
||||
|
||||
aAdd = new QAction(this);
|
||||
aAdd->setIcon(themePixmap(QStringLiteral("icons/increment")));
|
||||
|
|
@ -65,11 +69,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;
|
||||
|
|
@ -117,7 +126,9 @@ void DeckEditorSettingsPage::resetDownloadedURLsButtonClicked()
|
|||
{
|
||||
SettingsCache::instance().downloads().resetToDefaultURLs();
|
||||
urlList->clear();
|
||||
urlList->addItems(SettingsCache::instance().downloads().getAllURLs());
|
||||
for (const QString &url : SettingsCache::instance().downloads().getAllURLs()) {
|
||||
addUrlItem(url);
|
||||
}
|
||||
QMessageBox::information(this, tr("Success"), tr("Download URLs have been reset."));
|
||||
}
|
||||
|
||||
|
|
@ -126,7 +137,7 @@ void DeckEditorSettingsPage::actAddURL()
|
|||
bool ok;
|
||||
QString msg = QInputDialog::getText(this, tr("Add URL"), tr("URL:"), QLineEdit::Normal, QString(), &ok);
|
||||
if (ok) {
|
||||
urlList->addItem(msg);
|
||||
addUrlItem(msg);
|
||||
storeSettings();
|
||||
}
|
||||
}
|
||||
|
|
@ -141,12 +152,14 @@ void DeckEditorSettingsPage::actRemoveURL()
|
|||
|
||||
void DeckEditorSettingsPage::actEditURL()
|
||||
{
|
||||
if (urlList->currentItem()) {
|
||||
QString oldText = urlList->currentItem()->text();
|
||||
QListWidgetItem *item = urlList->currentItem();
|
||||
if (item) {
|
||||
const QString oldText = urlForItem(item);
|
||||
bool ok;
|
||||
QString msg = QInputDialog::getText(this, tr("Edit URL"), tr("URL:"), QLineEdit::Normal, oldText, &ok);
|
||||
if (ok) {
|
||||
urlList->currentItem()->setText(msg);
|
||||
item->setData(Qt::UserRole, msg);
|
||||
item->setText(urlLabel(msg));
|
||||
storeSettings();
|
||||
}
|
||||
}
|
||||
|
|
@ -158,10 +171,133 @@ void DeckEditorSettingsPage::storeSettings()
|
|||
|
||||
QStringList downloadUrls;
|
||||
for (int i = 0; i < urlList->count(); i++) {
|
||||
qInfo() << "Priority" << i << ":" << urlList->item(i)->text();
|
||||
downloadUrls << urlList->item(i)->text();
|
||||
const QString url = urlForItem(urlList->item(i));
|
||||
qInfo() << "Priority" << i << ":" << url;
|
||||
downloadUrls << url;
|
||||
}
|
||||
SettingsCache::instance().downloads().setDownloadUrls(downloadUrls);
|
||||
|
||||
// Drop per-host limits whose host is no longer referenced by any configured URL, so removing
|
||||
// a URL doesn't leave a stale throttle behind that reactivates if the host is re-added.
|
||||
QSet<QString> usedHosts;
|
||||
for (const QString &url : downloadUrls) {
|
||||
const QString host = QUrl(url).host();
|
||||
if (!host.isEmpty()) {
|
||||
usedHosts.insert(host);
|
||||
}
|
||||
}
|
||||
QHash<QString, int> limits = SettingsCache::instance().downloads().getHostRequestLimits();
|
||||
bool limitsChanged = false;
|
||||
for (auto it = limits.begin(); it != limits.end();) {
|
||||
// Prune only limits for hosts that are neither referenced by a configured URL nor carry a
|
||||
// developer cap. Capped hosts are often redirect targets (e.g. api.scryfall.com redirects
|
||||
// to cards.scryfall.io) that never appear in the URL list, yet they are exactly the hosts
|
||||
// the throttle applies to, so dropping them when a URL is removed would silently re-enable
|
||||
// free-running traffic to a rate-sensitive server.
|
||||
if (!usedHosts.contains(it.key()) && !DownloadSettings::getDeveloperHostCaps().contains(it.key())) {
|
||||
it = limits.erase(it);
|
||||
limitsChanged = true;
|
||||
} else {
|
||||
++it;
|
||||
}
|
||||
}
|
||||
if (limitsChanged) {
|
||||
SettingsCache::instance().downloads().setHostRequestLimits(limits);
|
||||
}
|
||||
|
||||
refreshUrlItems();
|
||||
}
|
||||
|
||||
QListWidgetItem *DeckEditorSettingsPage::addUrlItem(const QString &url)
|
||||
{
|
||||
auto *item = new QListWidgetItem(urlLabel(url));
|
||||
item->setData(Qt::UserRole, url);
|
||||
urlList->addItem(item);
|
||||
return item;
|
||||
}
|
||||
|
||||
QString DeckEditorSettingsPage::urlForItem(const QListWidgetItem *item) const
|
||||
{
|
||||
return item->data(Qt::UserRole).toString();
|
||||
}
|
||||
|
||||
QString DeckEditorSettingsPage::urlLabel(const QString &url) const
|
||||
{
|
||||
const QString host = QUrl(url).host();
|
||||
if (host.isEmpty()) {
|
||||
return url;
|
||||
}
|
||||
|
||||
const QHash<QString, int> limits = SettingsCache::instance().downloads().getHostRequestLimits();
|
||||
const int devCap =
|
||||
DownloadSettings::getDeveloperHostCaps().value(host, DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT);
|
||||
if (devCap == DownloadSettings::UNLIMITED_HOST_QUOTA && !limits.contains(host)) {
|
||||
return tr("%1 (unlimited)").arg(url);
|
||||
}
|
||||
|
||||
const int requested = limits.value(
|
||||
host, devCap == DownloadSettings::UNLIMITED_HOST_QUOTA ? DownloadSettings::DEFAULT_HOST_REQUEST_LIMIT : devCap);
|
||||
const int effective = SettingsCache::instance().downloads().clampHostRequestLimit(host, requested);
|
||||
return tr("%1 (%2/s)").arg(url).arg(effective);
|
||||
}
|
||||
|
||||
void DeckEditorSettingsPage::refreshUrlItems()
|
||||
{
|
||||
for (int i = 0; i < urlList->count(); ++i) {
|
||||
QListWidgetItem *item = urlList->item(i);
|
||||
item->setText(urlLabel(urlForItem(item)));
|
||||
}
|
||||
}
|
||||
|
||||
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(urlForItem(urlList->currentItem())).host();
|
||||
if (host.isEmpty()) {
|
||||
QMessageBox::information(this, tr("Adjust Rate Limit"), tr("The selected URL does not have a valid host."));
|
||||
return;
|
||||
}
|
||||
|
||||
const QHash<QString, int> &devCaps = DownloadSettings::getDeveloperHostCaps();
|
||||
const QHash<QString, int> 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;
|
||||
QString prompt;
|
||||
if (unlocked) {
|
||||
minimum = 0; // 0 means "unlimited"
|
||||
maximum = DownloadSettings::UNLOCKED_HOST_LIMIT_MAX;
|
||||
defaultValue = currentLimits.value(host, 0);
|
||||
prompt = tr("Requests per second (0 = unlimited, fastest; up to %1):").arg(maximum);
|
||||
} else {
|
||||
minimum = DownloadSettings::MIN_HOST_REQUEST_LIMIT;
|
||||
maximum = devCap;
|
||||
defaultValue = currentLimits.value(host, devCap);
|
||||
prompt = tr("Requests per second (developer maximum is %1):").arg(maximum);
|
||||
}
|
||||
|
||||
const int value = QInputDialog::getInt(this, tr("Adjust Rate Limit for %1").arg(host), prompt, defaultValue,
|
||||
minimum, maximum, 1, &ok);
|
||||
if (!ok) {
|
||||
return;
|
||||
}
|
||||
|
||||
QHash<QString, int> limits = currentLimits;
|
||||
if (unlocked ? value == 0 : value == devCap) {
|
||||
limits.remove(host);
|
||||
} else {
|
||||
limits.insert(host, value);
|
||||
}
|
||||
SettingsCache::instance().downloads().setHostRequestLimits(limits);
|
||||
refreshUrlItems();
|
||||
}
|
||||
|
||||
void DeckEditorSettingsPage::urlListChanged(const QModelIndex &, int, int, const QModelIndex &, int)
|
||||
|
|
@ -244,4 +380,8 @@ void DeckEditorSettingsPage::retranslateUi()
|
|||
aAdd->setText(tr("Add New URL"));
|
||||
aEdit->setText(tr("Edit URL"));
|
||||
aRemove->setText(tr("Remove URL"));
|
||||
}
|
||||
aRateLimit->setText(tr("Adjust Rate Limit"));
|
||||
|
||||
// The per-URL rate limit suffixes are translated, so refresh them when the language changes.
|
||||
refreshUrlItems();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
@ -46,6 +47,18 @@ private:
|
|||
QLabel infoOnSpoilersLabel;
|
||||
QPushButton *mpSpoilerPathButton;
|
||||
QPushButton *updateNowButton;
|
||||
|
||||
/** @brief Adds a list item for the given URL, storing the raw URL alongside its displayed label. */
|
||||
QListWidgetItem *addUrlItem(const QString &url);
|
||||
|
||||
/** @brief Returns the raw URL stored on a list item. */
|
||||
[[nodiscard]] QString urlForItem(const QListWidgetItem *item) const;
|
||||
|
||||
/** @brief Returns the display label for a URL, including its current effective rate limit. */
|
||||
[[nodiscard]] QString urlLabel(const QString &url) const;
|
||||
|
||||
/** @brief Refreshes the displayed label of every URL item after limits or settings change. */
|
||||
void refreshUrlItems();
|
||||
};
|
||||
|
||||
#endif // COCKATRICE_DECK_EDITOR_SETTINGS_PAGE_H
|
||||
|
|
|
|||
|
|
@ -12,6 +12,22 @@ const QStringList DownloadSettings::DEFAULT_DOWNLOAD_URLS = {
|
|||
const QString DownloadSettings::SCRYFALL_NAMED_LOCALIZED_URL =
|
||||
"https://api.scryfall.com/cards/named?fuzzy=!localizedName!&lang=!sflang!&format=image&face=!prop:side!";
|
||||
|
||||
// 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<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)
|
||||
{
|
||||
|
|
@ -65,3 +81,57 @@ void DownloadSettings::setDownloadSpoilerStatus(bool _spoilerStatus)
|
|||
setValue(_spoilerStatus, "downloadSpoilers");
|
||||
emit downloadSpoilerStatusChanged();
|
||||
}
|
||||
|
||||
QHash<QString, int> DownloadSettings::getHostRequestLimits() const
|
||||
{
|
||||
auto settings = getSettings();
|
||||
if (!defaultGroup.isEmpty()) {
|
||||
settings.beginGroup(defaultGroup);
|
||||
}
|
||||
settings.beginGroup("hostRequestLimits");
|
||||
|
||||
QHash<QString, int> hostRequestLimits;
|
||||
const QStringList hosts = settings.childKeys();
|
||||
for (const QString &host : hosts) {
|
||||
hostRequestLimits.insert(host, settings.value(host).toInt());
|
||||
}
|
||||
|
||||
settings.endGroup();
|
||||
if (!defaultGroup.isEmpty()) {
|
||||
settings.endGroup();
|
||||
}
|
||||
return hostRequestLimits;
|
||||
}
|
||||
|
||||
void DownloadSettings::setHostRequestLimits(const QHash<QString, int> &hostRequestLimits)
|
||||
{
|
||||
auto settings = getSettings();
|
||||
if (!defaultGroup.isEmpty()) {
|
||||
settings.beginGroup(defaultGroup);
|
||||
}
|
||||
|
||||
// Drop the legacy single-key form (an opaque @Variant blob) written by earlier builds so each
|
||||
// host is stored as a plain, hand-editable key in its own subgroup.
|
||||
settings.remove("hostRequestLimits");
|
||||
settings.beginGroup("hostRequestLimits");
|
||||
settings.remove(QString());
|
||||
for (auto it = hostRequestLimits.cbegin(); it != hostRequestLimits.cend(); ++it) {
|
||||
settings.setValue(it.key(), it.value());
|
||||
}
|
||||
settings.endGroup();
|
||||
|
||||
if (!defaultGroup.isEmpty()) {
|
||||
settings.endGroup();
|
||||
}
|
||||
settings.sync();
|
||||
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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -9,6 +9,8 @@
|
|||
|
||||
#include "settings_manager.h"
|
||||
|
||||
#include <QHash>
|
||||
|
||||
class DownloadSettings : public SettingsManager
|
||||
{
|
||||
Q_OBJECT
|
||||
|
|
@ -16,8 +18,35 @@ class DownloadSettings : public SettingsManager
|
|||
|
||||
static const QStringList DEFAULT_DOWNLOAD_URLS;
|
||||
static const QString SCRYFALL_NAMED_LOCALIZED_URL;
|
||||
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 Upper bound offered to the user when lowering an unlocked host's allowance.
|
||||
*
|
||||
* Unlocked hosts have no developer cap, so `clampHostRequestLimit` puts no upper bound on
|
||||
* them; this only bounds what the settings dialog offers, and matches the widest paced
|
||||
* allowance a user is documented to be able to hand-edit in `downloads.ini`. Choosing 0
|
||||
* below this restores the "unlimited" fast path.
|
||||
*/
|
||||
static constexpr int UNLOCKED_HOST_LIMIT_MAX = 50;
|
||||
/** @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<QString, int> &getDeveloperHostCaps();
|
||||
|
||||
explicit DownloadSettings(const QString &, QObject *);
|
||||
|
||||
QStringList getAllURLs() const;
|
||||
|
|
@ -29,9 +58,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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue