Refactor the function that handles HTTP results

- Ffunction extraction
- Improving code depth
- Improving readability
- Unit test enhancement

[   32s] [==========] Running 104 tests from 4 test suites.
[   32s] [----------] Global test environment set-up.
[   32s] [----------] 7 tests from HttpAsyncTest
[   32s] [ RUN      ] HttpAsyncTest.SubmitN1
[   32s] [       OK ] HttpAsyncTest.SubmitN1 (139 ms)
[   32s] [ RUN      ] HttpAsyncTest.SubmitN2
[   32s] [       OK ] HttpAsyncTest.SubmitN2 (39 ms)
[   32s] [ RUN      ] HttpAsyncTest.SubmitN3
[   34s] [       OK ] HttpAsyncTest.SubmitN3 (2009 ms)
[   34s] [ RUN      ] HttpAsyncTest.SubmitP
[   34s] [       OK ] HttpAsyncTest.SubmitP (21 ms)
[   34s] [ RUN      ] HttpAsyncTest.CancelP
[   34s] [       OK ] HttpAsyncTest.CancelP (9 ms)
[   34s] [ RUN      ] HttpAsyncTest.PauseP
[   34s] [       OK ] HttpAsyncTest.PauseP (9 ms)
[   34s] [ RUN      ] HttpAsyncTest.ResumeP
[   34s] [       OK ] HttpAsyncTest.ResumeP (10 ms)
[   34s] [----------] 7 tests from HttpAsyncTest (2243 ms total)

[   34s] [----------] Global test environment tear-down
[   34s] [==========] 104 tests from 4 test suites ran. (2457 ms total)
[   34s] [  PASSED  ] 104 tests.
[   34s] + lcov -c --ignore-errors graph --no-external -b . -d . -o capi-network-http.info

Change-Id: I8a6f231e252be99001e99cd0dd8fb4ec3370387a

diff --git a/src/http_session.c b/src/http_session.c
index bd64331efe5d8dbe2287dfb5b1a702a593e9ac4f..1a25abd0ff4b6bd3132b441305334915dc8d32d4 100644 (file)
--- a/src/http_session.c
+++ b/src/http_session.c
@@ -19,101 +19,144 @@
 
 static int generated_session_id = -1;
 
+static bool __is_auth_required(http_status_code_e status)
+{
+       return (status == HTTP_STATUS_UNAUTHORIZED || status == HTTP_STATUS_PROXY_AUTHENTICATION_REQUIRED);
+}
+
+static http_error_code_e __convert_to_http_error_code(CURLcode code)
+{
+       DBG("curl code[%d]", code);
+       switch (code) {
+       case CURLE_OK:
+               return HTTP_ERROR_NONE;
+       case CURLE_COULDNT_RESOLVE_HOST:
+               return HTTP_ERROR_COULDNT_RESOLVE_HOST;
+       case CURLE_COULDNT_CONNECT:
+               return HTTP_ERROR_COULDNT_CONNECT;
+       case CURLE_SSL_CONNECT_ERROR:
+               return HTTP_ERROR_SSL_CONNECT_ERROR;
+       case CURLE_OPERATION_TIMEDOUT:
+               return HTTP_ERROR_OPERATION_TIMEDOUT;
+       case CURLE_ABORTED_BY_CALLBACK:
+               return HTTP_ERROR_CANCELED;
+       default:
+               return HTTP_ERROR_OPERATION_FAILED;
+       }
+}
+
+static void __set_requried_auth_scheme(__http_transaction_h *transaction)
+{
+       if (transaction == NULL)
+               return;
+
+       long http_auth = _CURL_HTTP_AUTH_NONE;
+       long proxy_auth = _CURL_HTTP_AUTH_NONE;
+       curl_easy_getinfo(transaction->easy_handle, CURLINFO_HTTPAUTH_AVAIL, &http_auth);
+       curl_easy_getinfo(transaction->easy_handle, CURLINFO_PROXYAUTH_AVAIL, &proxy_auth);
+
+       bool is_proxy_auth = (proxy_auth != _CURL_HTTP_AUTH_NONE);
+       http_auth_scheme_e auth_scheme = _get_http_auth_scheme(is_proxy_auth, http_auth);
+       http_transaction_set_http_auth_scheme(transaction, auth_scheme);
+}
+
+static void _update_auth_info(__http_transaction_h *transaction)
+{
+       if (transaction == NULL)
+               return;
+
+       transaction->auth_required = TRUE;
+
+       __set_requried_auth_scheme(transaction);
+       _set_authentication_info(transaction);
+}
+
+static void _handle_success_request(__http_transaction_h *transaction)
+{
+       if (!transaction->completed_cb)
+               return;
+
+       http_status_code_e status = 0;
+       http_transaction_response_get_status_code(transaction, &status);
+       DBG("Status(%d)\n", status);
+
+       if (__is_auth_required(status))
+               _update_auth_info(transaction);
+
+       if (transaction->completed_cb)
+               transaction->completed_cb(transaction, transaction->completed_user_data);
+}
+
+static void _handle_fail_request(__http_transaction_h *transaction, http_error_code_e error)
+{
+       DBG("Error: %d", error);
+       if (!transaction)
+               return;
+
+       if (!transaction->aborted_cb)
+               return;
+
+       if (transaction->aborted_cb)
+               transaction->aborted_cb(transaction, error, transaction->aborted_user_data);
+}
+
+static void _handle_result(__http_transaction_h *transaction, CURLcode result)
+{
+       if (transaction == NULL)
+               return;
+
+       switch (result)
+       {
+       case CURLE_OK:
+               return _handle_success_request(transaction);
+       case CURLE_COULDNT_RESOLVE_HOST:
+       case CURLE_COULDNT_CONNECT:
+       case CURLE_SSL_CONNECT_ERROR:
+       case CURLE_OPERATION_TIMEDOUT:
+       case CURLE_ABORTED_BY_CALLBACK:
+       default:
+               return _handle_fail_request(transaction, __convert_to_http_error_code(result));
+       }
+}
+
 static void _handle_completed_transaction(__http_session_h *user_data)
 {
-       __http_transaction_h *transaction = NULL;
-       __http_session_h *session = user_data;
+       if (user_data == NULL)
+               return;
 
+       __http_session_h *session = user_data;
        CURLMsg* message = NULL;
        int count = 0;
-       CURL* curl_easy = NULL;
-       char* url = NULL;
-       CURLcode curl_code = CURLE_OK;
-
-       if (!session) return;
-
-       message = curl_multi_info_read(session->multi_handle, &count);
-
-       while (message != NULL) {
-               if (message->msg == CURLMSG_DONE) {
-                       curl_easy = message->easy_handle;
-                       curl_code = message->data.result;
-
-                       if (curl_easy) {
-                               curl_easy_getinfo(curl_easy, CURLINFO_PRIVATE, (char**) &transaction);
-                               curl_easy_getinfo(curl_easy, CURLINFO_EFFECTIVE_URL, &url);
-                       }
-
-                       if (!transaction) break;
-
-                       if (transaction->upload_event)
-                               _close_upload_file(transaction);
-
-                       DBG("Completed -%s: result(%d)\n", url, curl_code);
-
-                       switch (curl_code) {
-                       case CURLE_OK:
-                               if (transaction->completed_cb) {
-                                       long http_auth = _CURL_HTTP_AUTH_NONE;
-                                       long proxy_auth = _CURL_HTTP_AUTH_NONE;
-                                       http_status_code_e status = 0;
-                                       bool auth_req = FALSE;
-                                       bool proxy = FALSE;
-
-                                       http_transaction_response_get_status_code(transaction, &status);
-                                       DBG("Status(%d)\n", status);
-
-                                       if (status == HTTP_STATUS_UNAUTHORIZED || status == HTTP_STATUS_PROXY_AUTHENTICATION_REQUIRED) {
-
-                                               transaction->auth_required = auth_req = TRUE;
-
-                                               curl_easy_getinfo(transaction->easy_handle, CURLINFO_HTTPAUTH_AVAIL, &http_auth);
-                                               curl_easy_getinfo(transaction->easy_handle, CURLINFO_PROXYAUTH_AVAIL, &proxy_auth);
-
-                                               if (proxy_auth != _CURL_HTTP_AUTH_NONE)
-                                                       proxy = TRUE;
-
-                                               http_auth_scheme_e auth_scheme = _get_http_auth_scheme(proxy, http_auth);
-                                               http_transaction_set_http_auth_scheme(transaction, auth_scheme);
-                                               _set_authentication_info(transaction);
-                                       }
-                                       if (transaction->completed_cb)
-                                               transaction->completed_cb(transaction, transaction->completed_user_data);
-                               }
-
-                               break;
-                       case CURLE_COULDNT_RESOLVE_HOST:
-                               if (transaction->aborted_cb)
-                                       transaction->aborted_cb(transaction, HTTP_ERROR_COULDNT_RESOLVE_HOST, transaction->aborted_user_data);
-                               break;
-                       case CURLE_COULDNT_CONNECT:
-                               if (transaction->aborted_cb)
-                                       transaction->aborted_cb(transaction, HTTP_ERROR_COULDNT_CONNECT, transaction->aborted_user_data);
-                               break;
-                       case CURLE_SSL_CONNECT_ERROR:
-                               if (transaction->aborted_cb)
-                                       transaction->aborted_cb(transaction, HTTP_ERROR_SSL_CONNECT_ERROR, transaction->aborted_user_data);
-                               break;
-                       case CURLE_OPERATION_TIMEDOUT:
-                               if (transaction->aborted_cb)
-                                       transaction->aborted_cb(transaction, HTTP_ERROR_OPERATION_TIMEDOUT, transaction->aborted_user_data);
-                               break;
-                       case CURLE_ABORTED_BY_CALLBACK:
-                               if (transaction->aborted_cb)
-                                       transaction->aborted_cb(transaction, HTTP_ERROR_CANCELED, transaction->aborted_user_data);
-                               break;
-                       default:
-                               if (transaction->aborted_cb)
-                                       transaction->aborted_cb(transaction, HTTP_ERROR_OPERATION_FAILED, transaction->aborted_user_data);
-                               break;
-                       }
-
-                       if (session->multi_handle != NULL && curl_easy != NULL)
-                               curl_multi_remove_handle(session->multi_handle, transaction->easy_handle);
 
-               }
+       do {
                message = curl_multi_info_read(session->multi_handle, &count);
-       }
+               if (message == NULL)
+                       continue;
+
+               if (message->msg != CURLMSG_DONE)
+                       continue;
+
+               char* url = NULL;
+               __http_transaction_h *transaction = NULL;
+               CURL *curl_easy = message->easy_handle;
+               if (curl_easy) {
+                       curl_easy_getinfo(curl_easy, CURLINFO_PRIVATE, (char **)&transaction);
+                       curl_easy_getinfo(curl_easy, CURLINFO_EFFECTIVE_URL, &url);
+               }
+
+               if (!transaction)
+                       break;
+
+               if (transaction->upload_event)
+                       _close_upload_file(transaction);
+
+               DBG("Completed -%s: result(%d)\n", url, message->data.result);
+               _handle_result(transaction, message->data.result);
+
+               if (session->multi_handle != NULL && curl_easy != NULL)
+                       curl_multi_remove_handle(session->multi_handle, transaction->easy_handle);
+
+       } while (message != NULL);
 }
 
 gboolean curl_request_check(gpointer data)

diff --git a/tests/http-gtest-async.cpp b/tests/http-gtest-async.cpp
index 7a22374f9e943102f3be66efe64dec3bfb4ee846..3f0a613aeb89d5e617aff7f16e5c2662f88ce267 100755 (executable)
--- a/tests/http-gtest-async.cpp
+++ b/tests/http-gtest-async.cpp
@@ -37,6 +37,7 @@ static bool _completed_flag;
 static bool _canceled_flag;
 static bool _paused_flag;
 static bool _resumed_flag;
+static http_error_code_e _error_code;
 
 static void _completed_cb(http_transaction_h http_transaction, void *user_data);
 static void _aborted_cb(http_transaction_h http_transaction,
@@ -57,6 +58,7 @@ class HttpAsyncTest: public ::testing::Test {
                        _canceled_flag = false;
                        _paused_flag = false;
                        _resumed_flag = false;
+                       _error_code = HTTP_ERROR_NONE;
 
                        http_init();
                        http_session_create(HTTP_SESSION_MODE_NORMAL, &session);
@@ -88,6 +90,7 @@ static void _aborted_cb(http_transaction_h http_transaction,
                http_error_code_e error, void *user_data)
 {
        _completed_flag = false;
+       _error_code = error;
        if (error == HTTP_ERROR_CANCELED)
                _canceled_flag = true;
        HttpTestUtil::stopMainLoop();
@@ -120,7 +123,7 @@ static void _progress_resume_cb(http_transaction_h http_transaction,
                _resumed_flag = true;
 }
 
-TEST_F(HttpAsyncTest, SubmitN)
+TEST_F(HttpAsyncTest, SubmitN1)
 {
        http_transaction_request_set_uri(transaction, "https://untrusted-root.badssl.com");
        EXPECT_EQ(HTTP_ERROR_NONE, http_transaction_submit(transaction));
@@ -128,6 +131,32 @@ TEST_F(HttpAsyncTest, SubmitN)
        HttpTestUtil::runMainLoop();
 
        EXPECT_EQ(false, _completed_flag);
+#if !defined USE_LOCALHOST_SERVER
+       EXPECT_EQ(HTTP_ERROR_SSL_CONNECT_ERROR, _error_code);
+#endif
+}
+
+TEST_F(HttpAsyncTest, SubmitN2)
+{
+       http_transaction_request_set_uri(transaction, "123qw12qw123");
+       EXPECT_EQ(HTTP_ERROR_NONE, http_transaction_submit(transaction));
+
+       HttpTestUtil::runMainLoop();
+
+       EXPECT_EQ(false, _completed_flag);
+       EXPECT_EQ(HTTP_ERROR_COULDNT_RESOLVE_HOST, _error_code);
+}
+
+TEST_F(HttpAsyncTest, SubmitN3)
+{
+       http_transaction_set_timeout(transaction, 2);
+       http_transaction_request_set_uri(transaction, "192.168.0.248");
+       EXPECT_EQ(HTTP_ERROR_NONE, http_transaction_submit(transaction));
+
+       HttpTestUtil::runMainLoop();
+
+       EXPECT_EQ(false, _completed_flag);
+       EXPECT_EQ(HTTP_ERROR_OPERATION_TIMEDOUT, _error_code);
 }
 
 TEST_F(HttpAsyncTest, SubmitP)
@@ -147,6 +176,7 @@ TEST_F(HttpAsyncTest, CancelP)
        HttpTestUtil::runMainLoop();
 
        EXPECT_EQ(true, _canceled_flag);
+       EXPECT_EQ(HTTP_ERROR_CANCELED, _error_code);
 }
 
 TEST_F(HttpAsyncTest, PauseP)