Fixes audacity#5970, audacity#5971

crsib · Feb 22, 2024 · a6a67b8 · a6a67b8
1 parent 10d116b
commit a6a67b8
Show file tree

Hide file tree

Showing 21 changed files with 287 additions and 220 deletions.
diff --git a/libraries/lib-cloud-audiocom/CMakeLists.txt b/libraries/lib-cloud-audiocom/CMakeLists.txt
@@ -56,6 +56,7 @@ set ( LIBRARIES
    lib-project-file-io-interface # ProjectFileIOExtension
    PUBLIC
       rapidjson::rapidjson # Protocol is JSON based
+      lib-concurrency-interface
    PRIVATE
       lib-sqlite-helpers-interface
       lib-crypto-interface

diff --git a/libraries/lib-cloud-audiocom/CloudSyncService.cpp b/libraries/lib-cloud-audiocom/CloudSyncService.cpp
@@ -273,7 +273,8 @@ CloudSyncService& CloudSyncService::Get()
 }
 
 CloudSyncService::GetProjectsFuture CloudSyncService::GetProjects(
-   std::shared_ptr<CancellationContext> context, int page, int pageSize,
+   audacity::concurrency::CancellationContextPtr context, int page,
+   int pageSize,
    std::string searchString)
 {
    using namespace audacity::network_manager;

diff --git a/libraries/lib-cloud-audiocom/CloudSyncService.h b/libraries/lib-cloud-audiocom/CloudSyncService.h
@@ -21,6 +21,8 @@
 
 #include "NetworkUtils.h"
 
+#include "concurrency/CancellationContext.h"
+
 class AudacityProject;
 
 namespace cloud::audiocom
@@ -84,7 +86,7 @@ class CLOUD_AUDIOCOM_API CloudSyncService final
 
    //! Retrieve the list of projects from the cloud
    GetProjectsFuture GetProjects(
-      std::shared_ptr<CancellationContext> context, int page, int pageSize,
+      audacity::concurrency::CancellationContextPtr context, int page, int pageSize,
       std::string searchString);
 
    //! Open the project from the cloud. This operation is asynchronous.

diff --git a/libraries/lib-cloud-audiocom/NetworkUtils.cpp b/libraries/lib-cloud-audiocom/NetworkUtils.cpp
@@ -106,55 +106,6 @@ void SetCommonHeaders(Request& request)
          common_headers::Authorization, oauthService.GetAccessToken());
 }
 
-CancellationContext::CancellationContext(Tag)
-{
-}
-
-std::shared_ptr<CancellationContext> CancellationContext::Create()
-{
-   return std::make_shared<CancellationContext>(Tag {});
-}
-
-void CancellationContext::Cancel()
-{
-   if (mCancelled.exchange(true))
-      return;
-
-   std::vector<std::function<void()>> callbacks;
-
-   {
-      auto lock = std::lock_guard { mDeferredCallbacksMutex };
-      std::swap(callbacks, mDeferredCallbacks);
-   }
-
-   std::for_each(
-      callbacks.begin(), callbacks.end(), [](auto& callback) { callback(); });
-}
-
-void CancellationContext::OnCancelled(std::function<void()> callback)
-{
-   if (!callback)
-      return;
-
-   if (mCancelled.load())
-   {
-      callback();
-      return;
-   }
-
-   auto lock = std::lock_guard { mDeferredCallbacksMutex };
-   mDeferredCallbacks.push_back(std::move(callback));
-}
-
-void CancellationContext::OnCancelled(
-   std::shared_ptr<audacity::network_manager::IResponse> response)
-{
-   if (!response)
-      return;
-
-   OnCancelled([response = std::move(response)]() { response->abort(); });
-}
-
 TransferStats& TransferStats::SetBytesTransferred(int64_t bytesTransferred)
 {
    BytesTransferred = bytesTransferred;

diff --git a/libraries/lib-cloud-audiocom/NetworkUtils.h b/libraries/lib-cloud-audiocom/NetworkUtils.h
@@ -11,13 +11,8 @@
 
 #pragma once
 
-#include <atomic>
 #include <chrono>
-#include <functional>
-#include <memory>
-#include <mutex>
 #include <string>
-#include <vector>
 
 namespace audacity::network_manager
 {
@@ -43,34 +38,6 @@ struct CLOUD_AUDIOCOM_API TransferStats final
    TransferStats& SetTransferDuration(Duration transferDuration);
 }; // struct TransferStats
 
-struct CLOUD_AUDIOCOM_API CancellationContext final
-{
-   struct Tag final
-   {
-   };
-
-public:
-   explicit CancellationContext(Tag);
-
-   CancellationContext(const CancellationContext&)            = delete;
-   CancellationContext(CancellationContext&&)                 = delete;
-   CancellationContext& operator=(const CancellationContext&) = delete;
-   CancellationContext& operator=(CancellationContext&&)      = delete;
-
-   [[nodiscard]] static std::shared_ptr<CancellationContext> Create();
-
-   void Cancel();
-
-   void OnCancelled(std::function<void()> callback);
-   void
-   OnCancelled(std::shared_ptr<audacity::network_manager::IResponse> response);
-
-private:
-   std::atomic<bool> mCancelled { false };
-
-   std::mutex mDeferredCallbacksMutex;
-   std::vector<std::function<void()>> mDeferredCallbacks;
-}; // struct CancellationContext
 
 enum class ResponseResultCode
 {

diff --git a/libraries/lib-cloud-audiocom/sync/AsynchronousOperation.h b/libraries/lib-cloud-audiocom/sync/AsynchronousOperation.h
@@ -15,6 +15,8 @@
 #include <memory>
 #include <vector>
 
+#include "concurrency/ICancellable.h"
+
 class TrackList;
 
 namespace cloud::audiocom::sync
@@ -32,14 +34,14 @@ struct ProjectUploadData final
    std::shared_ptr<TrackList> Tracks;
 };
 
-class ProjectUploadOperation /* not final */
+class ProjectUploadOperation /* not final */ :
+    public audacity::concurrency::ICancellable
 {
 public:
    virtual ~ProjectUploadOperation() = default;
 
-   virtual void Start(UploadMode mode) = 0;
+   virtual void Start(UploadMode mode)                       = 0;
    virtual void SetUploadData(const ProjectUploadData& data) = 0;
-   virtual void Cancel() = 0;
-   virtual bool IsCompleted() const = 0;
+   virtual bool IsCompleted() const                          = 0;
 }; // class AsynchronousOperation
 } // namespace cloud::audiocom::sync
diff --git a/libraries/lib-cloud-audiocom/sync/DataUploader.cpp b/libraries/lib-cloud-audiocom/sync/DataUploader.cpp
@@ -36,7 +36,7 @@ struct DataUploader::Response final
    DataUploader& Uploader;
    UploadUrls Target;
    std::function<void(ResponseResult)> Callback;
-   std::function<bool(double)> ProgressCallback;
+   std::function<void(double)> ProgressCallback;
 
    int RetriesCount { 3 };
    int RetriesLeft { 3 };
@@ -45,21 +45,24 @@ struct DataUploader::Response final
    UploadData Data;
    std::shared_ptr<IResponse> NetworkResponse;
 
-   bool UploadFailed { false };
-
    ResponseResult CurrentResult;
+   CancellationContextPtr CancelContext;
+
+   std::atomic<bool> UploadFailed { false };
 
    Response(
-      DataUploader& uploader, const UploadUrls& target, UploadData data,
+      DataUploader& uploader, CancellationContextPtr cancelContext,
+      const UploadUrls& target, UploadData data,
       std::string mimeType, std::function<void(ResponseResult)> callback,
-      std::function<bool(double)> progressCallback)
+      std::function<void(double)> progressCallback)
        : Uploader { uploader }
        , Target { target }
        , Callback { std::move(callback) }
        , ProgressCallback { std::move(progressCallback) }
        , RetriesLeft { RetriesCount }
        , MimeType { std::move(mimeType) }
        , Data { std::move(data) }
+       , CancelContext { std::move(cancelContext) }
    {
       PerformUpload();
    }
@@ -77,13 +80,15 @@ struct DataUploader::Response final
 
          NetworkResponse = NetworkManager::GetInstance().doPut(
             request, data.data(), data.size());
+         CancelContext->OnCancelled(NetworkResponse);
       }
       else
       {
          auto filePath = *std::get_if<std::string>(&Data);
 
          NetworkResponse = NetworkManager::GetInstance().doPut(
             request, CreateRequestPayloadStream(filePath));
+         CancelContext->OnCancelled(NetworkResponse);
 
       }
 
@@ -105,8 +110,7 @@ struct DataUploader::Response final
                current = 0;
             }
 
-            if (!ProgressCallback(static_cast<double>(current) / total))
-               NetworkResponse->abort();
+            ProgressCallback(static_cast<double>(current) / total);
          });
    }
 
@@ -138,7 +142,9 @@ struct DataUploader::Response final
       Data = {};
       Request request { Target.SuccessUrl };
 
-      NetworkResponse = NetworkManager::GetInstance().doPost(request, nullptr, 0);
+      NetworkResponse =
+         NetworkManager::GetInstance().doPost(request, nullptr, 0);
+      CancelContext->OnCancelled(NetworkResponse);
 
       NetworkResponse->setRequestFinishedCallback(
          [this](auto)
@@ -166,17 +172,16 @@ struct DataUploader::Response final
 
    void FailUpload()
    {
-      if (!UploadFailed)
+      if (!UploadFailed.exchange(true))
       {
          Data = {};
-
          Callback(CurrentResult);
-         UploadFailed = true;
       }
 
       Request request { Target.FailUrl };
 
       NetworkResponse = NetworkManager::GetInstance().doPost(request, nullptr, 0);
+      CancelContext->OnCancelled(NetworkResponse);
 
       NetworkResponse->setRequestFinishedCallback(
          [this](auto)
@@ -195,12 +200,6 @@ struct DataUploader::Response final
          });
    }
 
-   void Cancel()
-   {
-      if (NetworkResponse)
-         NetworkResponse->abort();
-   }
-
    void CleanUp()
    {
       BasicUI::CallAfter([this]() { Uploader.RemoveResponse(*this); });
@@ -209,7 +208,6 @@ struct DataUploader::Response final
 
 DataUploader::~DataUploader()
 {
-   CancelAll();
 }
 
 DataUploader& DataUploader::Get()
@@ -218,49 +216,44 @@ DataUploader& DataUploader::Get()
    return instance;
 }
 
-void DataUploader::CancelAll()
-{
-   ResponsesList responses;
-
-   {
-      auto lock = std::lock_guard { mResponseMutex };
-      responses = std::move(mResponses);
-   }
-
-   for (auto& response : responses)
-      response->Cancel();
-}
-
 void DataUploader::Upload(
+   CancellationContextPtr cancelContext,
    const ServiceConfig&, const UploadUrls& target, std::vector<uint8_t> data,
    std::function<void(ResponseResult)> callback,
-   std::function<bool(double)> progressCallback)
+   std::function<void(double)> progressCallback)
 {
    if (!callback)
       callback = [](auto...) {};
 
    if (!progressCallback)
       progressCallback = [](auto...) { return true; };
 
+   if (!cancelContext)
+      cancelContext = audacity::concurrency::CancellationContext::Create();
+
    auto lock = std::lock_guard { mResponseMutex };
 
    mResponses.emplace_back(std::make_unique<Response>(
-      *this, target, std::move(data),
+      *this, cancelContext, target, std::move(data),
       audacity::network_manager::common_content_types::ApplicationXOctetStream,
       std::move(callback), std::move(progressCallback)));
 }
 
 void DataUploader::Upload(
+   CancellationContextPtr cancelContext,
    const ServiceConfig& config, const UploadUrls& target, std::string filePath,
    std::function<void(ResponseResult)> callback,
-   std::function<bool(double)> progressCallback)
+   std::function<void(double)> progressCallback)
 {
    if (!callback)
       callback = [](auto...) {};
 
    if (!progressCallback)
       progressCallback = [](auto...) { return true; };
 
+   if (!cancelContext)
+      cancelContext = audacity::concurrency::CancellationContext::Create();
+
    if (!wxFileExists(audacity::ToWXString(filePath)))
    {
       if (callback)
@@ -274,7 +267,7 @@ void DataUploader::Upload(
    auto lock = std::lock_guard { mResponseMutex };
 
    mResponses.emplace_back(std::make_unique<Response>(
-      *this, target, std::move(filePath),
+      *this, cancelContext, target, std::move(filePath),
       audacity::network_manager::common_content_types::ApplicationXOctetStream,
       std::move(callback), std::move(progressCallback)));
 }