Rewrite downloader using cpr::Session

Author: adazem009

src/internal/downloader.cpp

@@ -1,44 +1,32 @@
 // SPDX-License-Identifier: Apache-2.0
 
+#include <iostream>
+
 #include "downloader.h"
 
 using namespace libscratchcpp;
 
 Downloader::Downloader()
 {
+    m_session.SetTimeout(cpr::Timeout(5000));
 }
 
-void Downloader::startDownload(const std::string &url)
+bool Downloader::download(const std::string &url)
 {
-    if (m_asyncResponse)
-        cancel();
+    m_session.SetUrl(cpr::Url(url));
+    m_response = m_session.Get();
 
-    m_asyncResponse = std::make_unique<cpr::AsyncResponse>(cpr::GetAsync(cpr::Url(url)));
-}
+    if (m_response.status_code != 200) {
+        std::cerr << "download error: " << m_response.error.message << std::endl;
 
-void Downloader::cancel()
-{
-    if (m_asyncResponse) {
-        if (!m_asyncResponse->IsCancelled())
-            (void)(m_asyncResponse->Cancel());
-        m_asyncResponse.reset();
-    }
-}
+        if (m_response.status_code != 0)
+            std::cerr << "code: " << m_response.status_code << std::endl;
 
-void Downloader::wait()
-{
-    if (m_asyncResponse) {
-        m_asyncResponse->wait();
-        m_response = m_asyncResponse->get();
-    }
-}
-
-bool Downloader::isCancelled() const
-{
-    if (!m_asyncResponse)
+        std::cerr << "URL: " << url << std::endl;
         return false;
+    }
 
-    return m_asyncResponse->IsCancelled();
+    return true;
 }
 
 const std::string &Downloader::text() const

src/internal/downloader.h

@@ -15,15 +15,11 @@ class Downloader : public IDownloader
     public:
         Downloader();
 
-        void startDownload(const std::string &url) override;
-        void cancel() override;
-        void wait() override;
-
-        bool isCancelled() const override;
+        bool download(const std::string &url) override;
         const std::string &text() const override;
 
     private:
-        std::unique_ptr<cpr::AsyncResponse> m_asyncResponse;
+        cpr::Session m_session;
         cpr::Response m_response;
 };
 

src/internal/idownloader.h

@@ -12,11 +12,7 @@ class IDownloader
     public:
         virtual ~IDownloader() { }
 
-        virtual void startDownload(const std::string &url) = 0;
-        virtual void cancel() = 0;
-        virtual void wait() = 0;
-
-        virtual bool isCancelled() const = 0;
+        virtual bool download(const std::string &url) = 0;
         virtual const std::string &text() const = 0;
 };
 

src/internal/projectdownloader.cpp

@@ -3,6 +3,7 @@
 #include <iostream>
 #include <thread>
 #include <nlohmann/json.hpp>
+#include <cpr/cpr.h>
 
 #include "projectdownloader.h"
 #include "downloaderfactory.h"
@@ -19,6 +20,8 @@ static const std::string ASSET_SUFFIX = "/get";
     m_cancelMutex.lock();                                                                                                                                                                              \
     if (m_cancel) {                                                                                                                                                                                    \
         m_downloadedAssetCount = 0;                                                                                                                                                                    \
+        m_cancelMutex.unlock();                                                                                                                                                                        \
+        std::cout << "Download aborted!" << std::endl;                                                                                                                                                 \
         return false;                                                                                                                                                                                  \
     }                                                                                                                                                                                                  \
     m_cancelMutex.unlock()
@@ -41,17 +44,13 @@ bool ProjectDownloader::downloadJson(const std::string &projectId)
 
     // Get project token
     std::cout << "Fetching project info of " << projectId << std::endl;
-    m_tokenDownloader->startDownload(PROJECT_META_PREFIX + projectId);
-    m_tokenDownloader->wait();
+    bool ret = m_tokenDownloader->download(PROJECT_META_PREFIX + projectId);
 
-    if (m_tokenDownloader->text().empty()) {
+    if (!ret) {
         std::cerr << "Could not fetch project info of " << projectId << std::endl;
         return false;
     }
 
-    if (m_tokenDownloader->isCancelled())
-        return false;
-
     CHECK_CANCEL();
 
     nlohmann::json json;
@@ -76,17 +75,13 @@ bool ProjectDownloader::downloadJson(const std::string &projectId)
 
     // Download project JSON
     std::cout << "Downloading project JSON of " << projectId << std::endl;
-    m_jsonDownloader->startDownload(PROJECT_JSON_PREFIX + projectId + "?token=" + token);
-    m_jsonDownloader->wait();
+    ret = m_jsonDownloader->download(PROJECT_JSON_PREFIX + projectId + "?token=" + token);
 
-    if (m_jsonDownloader->text().empty()) {
+    if (!ret) {
         std::cerr << "Failed to download project JSON of " << projectId << std::endl;
         return false;
     }
 
-    if (m_jsonDownloader->isCancelled())
-        return false;
-
     CHECK_CANCEL();
 
     return true;
@@ -100,16 +95,16 @@ bool ProjectDownloader::downloadAssets(const std::vector<std::string> &assetIds)
 
     auto count = assetIds.size();
     unsigned int threadCount = std::thread::hardware_concurrency();
-    unsigned int times; // how many times we should download assets simultaneously (in "groups")
+
+    // Thread count: number of assets / 5, limited to maximum number of threads
+    threadCount = std::max(1u, std::min(threadCount, static_cast<unsigned int>(std::ceil(count / 5.0))));
+
     m_assets.clear();
+    m_assets.reserve(count);
     m_downloadedAssetCount = 0;
 
-    // Calculate number of "groups"
-    if (threadCount == 0) {
-        times = count;
-        threadCount = 1;
-    } else
-        times = std::ceil(count / static_cast<double>(threadCount));
+    for (unsigned int i = 0; i < count; i++)
+        m_assets.push_back(std::string());
 
     std::cout << "Downloading " << count << " asset(s)";
 
@@ -125,33 +120,55 @@ bool ProjectDownloader::downloadAssets(const std::vector<std::string> &assetIds)
         downloaders.push_back(m_downloaderFactory->create());
 
     // Download assets
-    for (unsigned int i = 0; i < times; i++) {
-        unsigned int currentCount = std::min(threadCount, static_cast<unsigned int>(count - i * threadCount));
+    auto f = [this, &downloaders, &assetIds, count, threadCount](unsigned int thread) {
+        auto downloader = downloaders[thread];
+        unsigned int n = std::ceil(count / static_cast<double>(threadCount));
+
+        for (unsigned int i = 0; i < n; i++) {
+            unsigned int index = thread * n + i;
+
+            if (index < count) {
+                m_cancelMutex.lock();
 
-        for (unsigned int j = 0; j < currentCount; j++)
-            downloaders[j]->startDownload(ASSET_PREFIX + assetIds[i * threadCount + j] + ASSET_SUFFIX);
+                if (m_cancel)
+                    return;
 
-        for (unsigned int j = 0; j < currentCount; j++) {
-            downloaders[j]->wait();
-            assert(m_assets.size() == i * threadCount + j);
+                m_cancelMutex.unlock();
 
-            if (downloaders[j]->isCancelled())
-                return false;
+                bool ret = downloader->download(ASSET_PREFIX + assetIds[index] + ASSET_SUFFIX);
 
-            CHECK_CANCEL();
+                if (!ret) {
+                    std::cerr << "Failed to download asset: " << assetIds[index] << std::endl;
+                    m_cancelMutex.lock();
+                    m_cancel = true;
+                    m_cancelMutex.unlock();
+                    return;
+                }
 
-            m_assets.push_back(downloaders[j]->text());
-            m_downloadedAssetCount++;
+                m_assetsMutex.lock();
+                m_assets[index] = downloader->text();
+                m_downloadedAssetCount++;
+                std::cout << "Downloaded assets: " << m_downloadedAssetCount << " of " << count << std::endl;
+                m_assetsMutex.unlock();
+            }
         }
-    }
+    };
+
+    std::vector<std::thread> threads;
+
+    for (unsigned int i = 0; i < threadCount; i++)
+        threads.emplace_back(std::thread(f, i));
+
+    for (unsigned int i = 0; i < threadCount; i++)
+        threads[i].join();
+
+    CHECK_CANCEL();
 
     return true;
 }
 
 void ProjectDownloader::cancel()
 {
-    m_tokenDownloader->cancel();
-    m_jsonDownloader->cancel();
     m_cancelMutex.lock();
     m_cancel = true;
     m_cancelMutex.unlock();

src/internal/projectdownloader.h

@@ -32,6 +32,7 @@ class ProjectDownloader : public IProjectDownloader
         std::shared_ptr<IDownloader> m_tokenDownloader;
         std::shared_ptr<IDownloader> m_jsonDownloader;
         std::vector<std::string> m_assets;
+        std::mutex m_assetsMutex;
         std::atomic<unsigned int> m_downloadedAssetCount = 0;
         bool m_cancel = false;
         std::mutex m_cancelMutex;

test/mocks/downloadermock.h

@@ -8,10 +8,6 @@ using namespace libscratchcpp;
 class DownloaderMock : public IDownloader
 {
     public:
-        MOCK_METHOD(void, startDownload, (const std::string &), (override));
-        MOCK_METHOD(void, cancel, (), (override));
-        MOCK_METHOD(void, wait, (), (override));
-
-        MOCK_METHOD(bool, isCancelled, (), (const, override));
+        MOCK_METHOD(bool, download, (const std::string &), (override));
         MOCK_METHOD(const std::string &, text, (), (const, override));
 };