[PictureLoader] Hand backed-off requests back to their worker instead of parking them

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker.cpp

@@ -108,6 +108,14 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture
     QUrl cachedRedirect = getCachedRedirect(url);
     if (!cachedRedirect.isEmpty()) {
         emit imageRequestSucceeded(url);
+        // The redirect target is a different host, which may itself be in 429 backoff; hand the
+        // entry back to its worker so it waits the backoff out instead of dispatching straight
+        // onto the backed-off host.
+        if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(cachedRedirect.host(),
+                                                                     QDateTime::currentDateTime())) {
+            worker->scheduleDeferredRetry();
+            return nullptr;
+        }
         return makeRequest(cachedRedirect, worker);
     }
 
@@ -136,8 +144,9 @@ QNetworkReply *CardPictureLoaderWorker::makeRequest(const QUrl &url, CardPicture
 
 void CardPictureLoaderWorker::resetRequestQuota()
 {
-    // Allowances are seeded per host on demand in processSingleRequest(), so a
+    // Allowances are seeded lazily per host in processSingleRequest() when a request is first
-    // rate-limited host never gets a fresh full quota mid-second.
+    // looked at in a new second, so a host that enters the queue mid-second now gets its reduced
+    // per-host allowance instead of falling through to the full per-second default.
     hostQuotaRemaining.clear();
 
     QDateTime now = QDateTime::currentDateTime();
@@ -191,12 +200,16 @@ bool CardPictureLoaderWorker::processSingleRequest()
     for (int i = 0; i < requestLoadQueue.size(); ++i) {
         const auto &request = requestLoadQueue.at(i);
         const QString host = request.first.host();
-        // Don't dispatch requests to a host that is currently in its 429 backoff.
+        // 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.
         if (CardPictureLoaderWorkerWork::rateLimiter().isRateLimited(host, now)) {
-            continue;
+            requestLoadQueue.removeAt(i);
+            request.second->startNextPicDownload();
+            return true;
         }
-        // Seed the allowance only now, so a host that was rate limited last second
+        // Seed the allowance now so a host that was rate limited gets its reduced
-        // doesn't get a fresh full quota the moment it is queried mid-second.
+        // allowance instead of a fresh full quota mid-second.
         if (!hostQuotaRemaining.contains(host)) {
             hostQuotaRemaining.insert(host, hostRequestQuota.value(host, MAX_REQUESTS_PER_SEC));
         }

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.cpp

@@ -22,7 +22,7 @@ static const QStringList MD5_BLACKLIST = {
     "fbc7d763c08771c260b39e2115414eeb"  // Current card back hash
 };
 
-ServerRateLimiter &CardPictureLoaderWorkerWork::rateLimiter()
+const ServerRateLimiter &CardPictureLoaderWorkerWork::rateLimiter()
 {
     return s_rateLimiter;
 }

cockatrice/src/interface/card_picture_loader/card_picture_loader_worker_work.h

@@ -44,7 +44,27 @@ public:
     CardPictureToLoad cardToDownload; ///< The card and associated URLs to try downloading
 
     /** @brief Shared per-server 429 backoff state. */
-    static ServerRateLimiter &rateLimiter();
+    static const ServerRateLimiter &rateLimiter();
+
+    /**
+     * @brief Starts downloading the next URL for this card.
+     *
+     * Skips URLs whose server is currently in 429 backoff, either waiting the
+     * backoff out or falling through to the other configured sources. Also used by
+     * the dispatch machinery to hand an entry back after it was removed from the
+     * request queue when its host turned out to be backed off.
+     */
+    void startNextPicDownload();
+
+    /**
+     * @brief Schedules a deferred retry after the relevant server backoff expires.
+     *
+     * Waits on the current URL's server when it is the reason we are blocked,
+     * otherwise on the earliest active backoff. If no servers are in backoff,
+     * concludes with failure. Otherwise resets the CardPictureToLoad indices and
+     * retries after the backoff period.
+     */
+    void scheduleDeferredRetry();
 
 public slots:
     /**
@@ -58,9 +78,6 @@ private:
 
     static ServerRateLimiter s_rateLimiter; ///< Shared per-server 429 backoff state
 
-    /** @brief Starts downloading the next URL for this card. */
-    void startNextPicDownload();
-
     /** @brief Called when all URLs have been exhausted or download failed. */
     void picDownloadFailed();
 
@@ -85,16 +102,6 @@ private:
      */
     void concludeImageLoad(const QImage &image);
 
-    /**
-     * @brief Schedules a deferred retry after the relevant server backoff expires.
-     *
-     * Waits on the current URL's server when it is the reason we are blocked,
-     * otherwise on the earliest active backoff. If no servers are in backoff,
-     * concludes with failure. Otherwise resets the CardPictureToLoad indices and
-     * retries after the backoff period.
-     */
-    void scheduleDeferredRetry();
-
 private slots:
     /** @brief Updates the picDownload setting when it changes. */
     void picDownloadChanged();