From 9ee7166550bc52b7641b19f99c5eb518472a6bf4 Mon Sep 17 00:00:00 2001 From: Preston From Date: Thu, 2 Jan 2020 17:24:57 -0700 Subject: [PATCH] Change setCompletionStatus to convertCompletionStatus So async and sync uses of Request only set the status code in one place, change setCompletionStatus to convertCompletionStatus and use the result of the function to set the status in either onComplete (async) or Perform (sync). --- inc/lift/EventLoop.h | 16 ++++++++-------- inc/lift/Request.h | 8 +++++--- src/EventLoop.cpp | 16 ++++++---------- src/Request.cpp | 36 +++++++++++++----------------------- 4 files changed, 32 insertions(+), 44 deletions(-) diff --git a/inc/lift/EventLoop.h b/inc/lift/EventLoop.h index 7d6814e..c33fa3a 100644 --- a/inc/lift/EventLoop.h +++ b/inc/lift/EventLoop.h @@ -271,16 +271,16 @@ class EventLoop { /** * @param event_loop Reference to the EventLoop that is calling onComplete (so requests that have * response wait times can be removed from the multiset of ResponseWaitTimeWrapper) + * @param completion_status RequestStatus enum indicating the status of the lift Request when it has completed + * (Success, error, etc.). * @param shared_request Shared pointer to the SharedRequest that owns this Request, so it can be used to create - * a RequestHandle and return the Request to the RequestPool if necessary. - * @param response_wait_time_timeout Bool indicating whether or not onComplete was called because - * a response wait time was exceeded (true) or not (false) - * @param finish_time Optional that will contain a uint64_t indicating the timepoint when - * the request was timed out while waiting for the response time. - * If the request received a response or timed out via cURL, this will be - * empty and we'll get the total time from the cURL handle. + * a RequestHandle and return the Request to the RequestPool if necessary. + * @param finish_time Optional that will contain a uint64_t indicating the timepoint when request was timed out + * while waiting for the response time. If the request received a response or timed out via + * cURL, this will be empty and we'll get the total time from the cURL handle. + * Default is an empty optional. */ - friend auto Request::onComplete(EventLoop& event_loop, std::shared_ptr shared_request, std::optional finish_time) -> void; + friend auto Request::onComplete(EventLoop& event_loop, RequestStatus completion_status, std::shared_ptr shared_request, std::optional finish_time) -> void; }; } // lift diff --git a/inc/lift/Request.h b/inc/lift/Request.h index 0239a69..442f9e8 100644 --- a/inc/lift/Request.h +++ b/inc/lift/Request.h @@ -415,13 +415,15 @@ class Request { /** * Converts a CURLcode into a RequestStatus. * @param curl_code The CURLcode to convert. + * @return The lift RequestStatus that corresponds to the CURLcode */ - auto setCompletionStatus( - CURLcode curl_code) -> void; + auto convertCompletionStatus(CURLcode curl_code) -> RequestStatus; /** * @param event_loop Reference to the EventLoop that is calling onComplete (so requests that have * response wait times can be removed from the multiset of ResponseWaitTimeWrapper) + * @param completion_status RequestStatus enum indicating the status of the lift Request when it has completed + * (Success, error, etc.). * @param shared_request Shared pointer to the SharedRequest that owns this Request, so it can be used to create * a RequestHandle and return the Request to the RequestPool if necessary. * @param finish_time Optional that will contain a uint64_t indicating the timepoint when request was timed out @@ -429,7 +431,7 @@ class Request { * cURL, this will be empty and we'll get the total time from the cURL handle. * Default is an empty optional. */ - auto onComplete(EventLoop& event_loop, std::shared_ptr shared_request, std::optional finish_time = std::nullopt) -> void; + auto onComplete(EventLoop& event_loop, RequestStatus completion_status, std::shared_ptr shared_request, std::optional finish_time = std::nullopt) -> void; /** * Helper function to find how many bytes are left to be downloaded for a request diff --git a/src/EventLoop.cpp b/src/EventLoop.cpp index b5712e7..b7bc886 100644 --- a/src/EventLoop.cpp +++ b/src/EventLoop.cpp @@ -220,7 +220,7 @@ auto EventLoop::stopTimedOutRequests() -> void // Call onComplete on the underlying Request object, copying in the shared_pointer // so we get a correct reference count. - shared_request->GetAsReference().onComplete(*this, shared_request, now); + shared_request->GetAsReference().onComplete(*this, RequestStatus::RESPONSE_WAIT_TIME_TIMEOUT, shared_request, now); } // If there are still items in the multiset, get the first item and use its time to reset the request timer. @@ -294,8 +294,7 @@ auto EventLoop::checkActions( auto shared_request_ptr = *shared_request_ptr_pointer; auto& shared_request_ref = shared_request_ptr->GetAsReference(); - shared_request_ref.setCompletionStatus(easy_result); - shared_request_ref.onComplete(*this, shared_request_ptr); + shared_request_ref.onComplete(*this, shared_request_ref.convertCompletionStatus(easy_result), shared_request_ptr); --m_active_request_count; } @@ -495,14 +494,11 @@ auto requests_accept_async(uv_async_t* handle) -> void if(curl_code != CURLM_OK && curl_code != CURLM_CALL_MULTI_PERFORM) { - /** - * If curl_multi_add_handle fails then notify the user that the request failed to start - * immediately. + /* + * If curl_multi_add_handle fails then notify the user that the request failed to start immediately. + * If we are calling onComplete now, move the shared_ptr into the method so it cleans itself up correctly. */ - request_handle->setCompletionStatus(CURLcode::CURLE_SEND_ERROR); - - // If we are calling onComplete now, move the shared_ptr into the method so it cleans itself up correctly. - request_handle->onComplete(*event_loop, *shared_request_on_heap); + request_handle->onComplete(*event_loop, RequestStatus::ERROR_FAILED_TO_START, *shared_request_on_heap); } else { diff --git a/src/Request.cpp b/src/Request.cpp index fd12327..be526a0 100644 --- a/src/Request.cpp +++ b/src/Request.cpp @@ -324,7 +324,7 @@ auto Request::Perform() -> bool { prepareForPerform(); auto curl_error_code = curl_easy_perform(m_curl_handle); - setCompletionStatus(curl_error_code); + m_status_code = convertCompletionStatus(curl_error_code); return (m_status_code == RequestStatus::SUCCESS); } @@ -451,52 +451,41 @@ auto Request::clearResponseBuffers() -> void m_response_data.clear(); } -auto Request::setCompletionStatus( - CURLcode curl_code) -> void +auto Request::convertCompletionStatus(CURLcode curl_code) -> RequestStatus { - #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wswitch-enum" switch (curl_code) { case CURLcode::CURLE_OK: - m_status_code = RequestStatus::SUCCESS; - break; + return RequestStatus::SUCCESS; case CURLcode::CURLE_GOT_NOTHING: - m_status_code = RequestStatus::RESPONSE_EMPTY; - break; + return RequestStatus::RESPONSE_EMPTY; case CURLcode::CURLE_OPERATION_TIMEDOUT: - m_status_code = RequestStatus::TIMEOUT; - break; + return RequestStatus::TIMEOUT; case CURLcode::CURLE_COULDNT_CONNECT: - m_status_code = RequestStatus::CONNECT_ERROR; - break; + return RequestStatus::CONNECT_ERROR; case CURLcode::CURLE_COULDNT_RESOLVE_HOST: - m_status_code = RequestStatus::CONNECT_DNS_ERROR; - break; + return RequestStatus::CONNECT_DNS_ERROR; case CURLcode::CURLE_SSL_CONNECT_ERROR: - m_status_code = RequestStatus::CONNECT_SSL_ERROR; - break; + return RequestStatus::CONNECT_SSL_ERROR; case CURLcode::CURLE_WRITE_ERROR: /** * If there is a cURL write error, but the maximum number of bytes has been written, * then we intentionally aborted, so let's set this to success. * Otherwise, there was an error in the CURL write callback. */ - m_status_code = (getRemainingDownloadBytes() == 0) + return (getRemainingDownloadBytes() == 0) ? RequestStatus::SUCCESS : RequestStatus::DOWNLOAD_ERROR; - break; case CURLcode::CURLE_SEND_ERROR: - m_status_code = RequestStatus::ERROR_FAILED_TO_START; - break; + return RequestStatus::ERROR_FAILED_TO_START; default: - m_status_code = RequestStatus::ERROR; - break; + return RequestStatus::ERROR; } #pragma GCC diagnostic pop } -auto Request::onComplete(EventLoop& event_loop, std::shared_ptr shared_request, std::optional finish_time) -> void +auto Request::onComplete(EventLoop& event_loop, RequestStatus completion_status, std::shared_ptr shared_request, std::optional finish_time) -> void { auto request_handle_ptr = RequestHandle(std::move(shared_request)); @@ -510,6 +499,7 @@ auto Request::onComplete(EventLoop& event_loop, std::shared_ptr s } setTotalTime(finish_time); + m_status_code = completion_status; if (m_on_complete_handler != nullptr) {