feat: add a finally callback to NetworkRequests (#2350)

Co-authored-by: Rasmus Karlsson <rasmus.karlsson@pajlada.com>

src/common/NetworkCommon.hpp

@@ -12,6 +12,7 @@ class NetworkResult;
 using NetworkSuccessCallback = std::function<Outcome(NetworkResult)>;
 using NetworkErrorCallback = std::function<void(NetworkResult)>;
 using NetworkReplyCreatedCallback = std::function<void(QNetworkReply *)>;
+using NetworkFinallyCallback = std::function<void()>;
 
 enum class NetworkRequestType {
     Get,

src/common/NetworkPrivate.cpp

@@ -132,6 +132,7 @@ void loadUncached(const std::shared_ptr<NetworkData> &data)
                 data->timer_, &QTimer::timeout, worker, [reply, data]() {
                     qCDebug(chatterinoCommon) << "Aborted!";
                     reply->abort();
+
                     if (data->onError_)
                     {
                         postToThread([data] {
@@ -139,6 +140,13 @@ void loadUncached(const std::shared_ptr<NetworkData> &data)
                                 {}, NetworkResult::timedoutStatus));
                         });
                     }
+
+                    if (data->finally_)
+                    {
+                        postToThread([data] {
+                            data->finally_();
+                        });
+                    }
                 });
         }
 
@@ -159,17 +167,26 @@ void loadUncached(const std::shared_ptr<NetworkData> &data)
                 if (reply->error() ==
                     QNetworkReply::NetworkError::OperationCanceledError)
                 {
-                    //operation cancelled, most likely timed out
+                    // Operation cancelled, most likely timed out
                     return;
                 }
+
                 if (data->onError_)
                 {
                     auto status = reply->attribute(
                         QNetworkRequest::HttpStatusCodeAttribute);
+                    // TODO: Should this always be run on the GUI thread?
                     postToThread([data, code = status.toInt()] {
                         data->onError_(NetworkResult({}, code));
                     });
                 }
+
+                if (data->finally_)
+                {
+                    postToThread([data] {
+                        data->finally_();
+                    });
+                }
                 return;
             }
 
@@ -196,6 +213,16 @@ void loadUncached(const std::shared_ptr<NetworkData> &data)
             // log("finished {}", data->request_.url().toString());
 
             reply->deleteLater();
+
+            if (data->finally_)
+            {
+                if (data->executeConcurrently_)
+                    QtConcurrent::run([finally = std::move(data->finally_)] {
+                        finally();
+                    });
+                else
+                    data->finally_();
+            }
         };
 
         if (data->timer_ != nullptr)
@@ -271,6 +298,30 @@ void loadCached(const std::shared_ptr<NetworkData> &data)
                 });
             }
         }
+
+        if (data->finally_)
+        {
+            if (data->executeConcurrently_ || isGuiThread())
+            {
+                if (data->hasCaller_ && !data->caller_.get())
+                {
+                    return;
+                }
+
+                data->finally_();
+            }
+            else
+            {
+                postToThread([data]() {
+                    if (data->hasCaller_ && !data->caller_.get())
+                    {
+                        return;
+                    }
+
+                    data->finally_();
+                });
+            }
+        }
     }
 }
 

src/common/NetworkPrivate.hpp

@@ -44,6 +44,7 @@ struct NetworkData {
     NetworkReplyCreatedCallback onReplyCreated_;
     NetworkErrorCallback onError_;
     NetworkSuccessCallback onSuccess_;
+    NetworkFinallyCallback finally_;
 
     NetworkRequestType requestType_ = NetworkRequestType::Get;
 

src/common/NetworkRequest.cpp

@@ -79,6 +79,12 @@ NetworkRequest NetworkRequest::onSuccess(NetworkSuccessCallback cb) &&
     return std::move(*this);
 }
 
+NetworkRequest NetworkRequest::finally(NetworkFinallyCallback cb) &&
+{
+    this->data->finally_ = cb;
+    return std::move(*this);
+}
+
 NetworkRequest NetworkRequest::header(const char *headerName,
                                       const char *value) &&
 {

src/common/NetworkRequest.hpp

@@ -42,6 +42,7 @@ public:
     NetworkRequest onReplyCreated(NetworkReplyCreatedCallback cb) &&;
     NetworkRequest onError(NetworkErrorCallback cb) &&;
     NetworkRequest onSuccess(NetworkSuccessCallback cb) &&;
+    NetworkRequest finally(NetworkFinallyCallback cb) &&;
 
     NetworkRequest payload(const QByteArray &payload) &&;
     NetworkRequest cache() &&;

tests/src/NetworkRequest.cpp

@@ -67,6 +67,44 @@ TEST(NetworkRequest, Success)
     EXPECT_TRUE(NetworkManager::workerThread.isRunning());
 }
 
+TEST(NetworkRequest, FinallyCallbackOnSuccess)
+{
+    const std::vector<int> codes{200, 201, 202, 203, 204, 205, 206};
+
+    EXPECT_TRUE(NetworkManager::workerThread.isRunning());
+
+    for (const auto code : codes)
+    {
+        auto url = getStatusURL(code);
+        std::mutex mut;
+        bool requestDone = false;
+        std::condition_variable requestDoneCondition;
+
+        bool finallyCalled = false;
+
+        NetworkRequest(url)
+            .finally(
+                [&mut, &requestDone, &requestDoneCondition, &finallyCalled] {
+                    finallyCalled = true;
+
+                    {
+                        std::unique_lock lck(mut);
+                        requestDone = true;
+                    }
+                    requestDoneCondition.notify_one();
+                })
+            .execute();
+
+        // Wait for the request to finish
+        std::unique_lock lck(mut);
+        requestDoneCondition.wait(lck, [&requestDone] {
+            return requestDone;
+        });
+
+        EXPECT_TRUE(finallyCalled);
+    }
+}
+
 TEST(NetworkRequest, Error)
 {
     const std::vector<int> codes{
@@ -118,6 +156,47 @@ TEST(NetworkRequest, Error)
     EXPECT_TRUE(NetworkManager::workerThread.isRunning());
 }
 
+TEST(NetworkRequest, FinallyCallbackOnError)
+{
+    const std::vector<int> codes{
+        400, 401, 402, 403, 404, 405, 406, 407, 408, 409, 410,
+        411, 412, 413, 414, 418, 500, 501, 502, 503, 504,
+    };
+
+    EXPECT_TRUE(NetworkManager::workerThread.isRunning());
+
+    for (const auto code : codes)
+    {
+        auto url = getStatusURL(code);
+        std::mutex mut;
+        bool requestDone = false;
+        std::condition_variable requestDoneCondition;
+
+        bool finallyCalled = false;
+
+        NetworkRequest(url)
+            .finally(
+                [&mut, &requestDone, &requestDoneCondition, &finallyCalled] {
+                    finallyCalled = true;
+
+                    {
+                        std::unique_lock lck(mut);
+                        requestDone = true;
+                    }
+                    requestDoneCondition.notify_one();
+                })
+            .execute();
+
+        // Wait for the request to finish
+        std::unique_lock lck(mut);
+        requestDoneCondition.wait(lck, [&requestDone] {
+            return requestDone;
+        });
+
+        EXPECT_TRUE(finallyCalled);
+    }
+}
+
 TEST(NetworkRequest, TimeoutTimingOut)
 {
     EXPECT_TRUE(NetworkManager::workerThread.isRunning());
@@ -209,3 +288,49 @@ TEST(NetworkRequest, TimeoutNotTimingOut)
 
     EXPECT_TRUE(NetworkManager::workerThread.isRunning());
 }
+
+TEST(NetworkRequest, FinallyCallbackOnTimeout)
+{
+    EXPECT_TRUE(NetworkManager::workerThread.isRunning());
+
+    auto url = "http://httpbin.org/delay/5";
+
+    std::mutex mut;
+    bool requestDone = false;
+    std::condition_variable requestDoneCondition;
+    bool finallyCalled = false;
+    bool onSuccessCalled = false;
+    bool onErrorCalled = false;
+
+    NetworkRequest(url)
+        .timeout(1000)
+        .onSuccess([&](NetworkResult result) -> Outcome {
+            onSuccessCalled = true;
+            return Success;
+        })
+        .onError([&](NetworkResult result) {
+            onErrorCalled = true;
+            EXPECT_EQ(result.status(), NetworkResult::timedoutStatus);
+        })
+        .finally([&] {
+            finallyCalled = true;
+
+            {
+                std::unique_lock lck(mut);
+                requestDone = true;
+            }
+            requestDoneCondition.notify_one();
+        })
+        .execute();
+
+    // Wait for the request to finish
+    std::unique_lock lck(mut);
+    requestDoneCondition.wait(lck, [&requestDone] {
+        return requestDone;
+    });
+
+    EXPECT_TRUE(finallyCalled);
+    EXPECT_TRUE(onErrorCalled);
+    EXPECT_FALSE(onSuccessCalled);
+    EXPECT_TRUE(NetworkManager::workerThread.isRunning());
+}