Fix NetworkMock issue resulting in request lock. (#1121)

This commit fixes a wrong usage of the NetworkMock::ReturnHttpResponse
which does not have any sleep time by default and resulted in optimal
cases in callback trigger prior to the Network::Send mock call
returning which caught the OlpClient PendingUrlRequest with no valid
request id set and thus resulted in a lock in waiting for the callback
which never came.

Some other tests need additional increasing of wait timeout as the
delay introduced in NetworkMock response increases the overall
execution.

Additionaly this adds some more debug logs to PendingUrlRequests class
to see what callback is added or cancelled and changes the
ApiLookupClientTest.LookupApiAsync to really use the async API as this
did not happen due to copy-paste code from LookupApi test.

Resolves: OLPEDGE-2409

Signed-off-by: Andrei Popescu <[email protected]>

Author: Andrei Popescu

olp-cpp-sdk-core/src/client/OlpClient.cpp

@@ -161,13 +161,15 @@ void ExecuteSingleRequest(const std::shared_ptr<http::Network>& network,
     return CancellationToken([=] { network->Cancel(id); });
   };
 
-  pending_request->ExecuteOrCancelled(make_request, [&]() {
+  auto cancelled_func = [&]() {
     OLP_SDK_LOG_DEBUG_F(kLogTag,
                         "ExecuteSingleRequest - already cancelled, url='%s'",
                         request.GetUrl().c_str());
     callback(PendingUrlRequest::kInvalidRequestId,
              ToHttpResponse(kCancelledErrorResponse));
-  });
+  };
+
+  pending_request->ExecuteOrCancelled(make_request, cancelled_func);
 }
 
 NetworkCallbackType GetRetryCallback(

olp-cpp-sdk-core/src/client/PendingUrlRequests.cpp

@@ -37,6 +37,12 @@ size_t PendingUrlRequest::Append(NetworkAsyncCallback callback) {
   // NOTE: You should not append anything if this request was cancelled
   std::lock_guard<std::mutex> lock(mutex_);
   const auto callback_id = callbacks_.size();
+
+  OLP_SDK_LOG_DEBUG_F(
+      kLogTag,
+      "PendingUrlRequest::Append, callback_id=%zu, request_id=%" PRIu64,
+      callback_id, http_request_id_);
+
   callbacks_.emplace(callback_id, std::move(callback));
   return callback_id;
 }
@@ -52,6 +58,11 @@ bool PendingUrlRequest::ExecuteOrCancelled(const ExecuteFuncType& func,
 bool PendingUrlRequest::Cancel(size_t callback_id) {
   std::lock_guard<std::mutex> lock(mutex_);
 
+  OLP_SDK_LOG_DEBUG_F(
+      kLogTag,
+      "PendingUrlRequest::Cancel, callback_id=%zu, request_id=%" PRIu64,
+      callback_id, http_request_id_);
+
   auto it = callbacks_.find(callback_id);
   if (it == callbacks_.end()) {
     // Unknown callback_id

olp-cpp-sdk-core/src/client/PendingUrlRequests.h

@@ -79,6 +79,7 @@ class PendingUrlRequest {
   bool IsCancelled() const { return context_.IsCancelled(); }
 
   /// Cancels current network request and waits for response.
+  /// Default wait time is 60 seconds.
   bool CancelAndWait(
       std::chrono::milliseconds timeout = std::chrono::seconds(60)) {
     // Cancel Network call, wait for the Network callback

tests/common/mocks/NetworkMock.cpp

@@ -105,7 +105,6 @@ GenerateNetworkMockActions(std::shared_ptr<std::promise<void>> pre_signal,
 ///
 /// NetworkMock Actions
 ///
-
 NetworkCallback ReturnHttpResponse(http::NetworkResponse response,
                                    const std::string& response_body,
                                    const http::Headers& headers,
@@ -117,11 +116,15 @@ NetworkCallback ReturnHttpResponse(http::NetworkResponse response,
              http::Network::HeaderCallback header_callback,
              http::Network::DataCallback) mutable {
     std::thread([=]() {
+      std::this_thread::sleep_for(delay);
+
       for (const auto& header : headers) {
         header_callback(header.first, header.second);
       }
-      *payload << response_body;
-      std::this_thread::sleep_for(delay);
+
+      payload->seekp(0, std::ios_base::end);
+      payload->write(response_body.c_str(), response_body.size());
+      payload->seekp(0);
       callback(response);
     })
         .detach();

tests/common/mocks/NetworkMock.h

@@ -105,7 +105,7 @@ GenerateNetworkMockActions(std::shared_ptr<std::promise<void>> pre_signal,
 NetworkCallback ReturnHttpResponse(
     olp::http::NetworkResponse response, const std::string& response_body,
     const olp::http::Headers& headers = {},
-    std::chrono::nanoseconds delay = std::chrono::nanoseconds(0),
+    std::chrono::nanoseconds delay = std::chrono::milliseconds(50),
     olp::http::RequestId request_id = 5);
 
 inline olp::http::NetworkResponse GetResponse(int status) {