Set default retry policy for C SDK

Azure · Nov 11, 2016 · 757c377 · 757c377
1 parent 79c0302
commit 757c377
Show file tree

Hide file tree

Showing 7 changed files with 89 additions and 33 deletions.
diff --git a/c/iothub_client/devdoc/requirement_docs/iothubclient_ll_requirements.md b/c/iothub_client/devdoc/requirement_docs/iothubclient_ll_requirements.md
@@ -185,6 +185,8 @@ extern IOTHUB_CLIENT_LL_HANDLE IoTHubClient_LL_Create(const IOTHUB_CLIENT_CONFIG
 
 **SRS_IOTHUBCLIENT_LL_02_008: [** Otherwise, `IoTHubClient_LL_Create` shall succeed and return a `non-NULL` handle.** ]** 
 
+**SRS_IOTHUBCLIENT_LL_25_124: [** `IoTHubClient_LL_Create` shall set the default retry policy as Exponential backoff with jitter and if succeed and return a `non-NULL` handle.** ]**
+
 
 
 ## IoTHubClient_LL_CreateWithTransport
@@ -217,6 +219,8 @@ extern  IOTHUB_CLIENT_LL_HANDLE IoTHubClient_LL_CreateWithTransport(IOTHUB_CLIEN
 
 **SRS_IOTHUBCLIENT_LL_17_007: [** If the _Register function fails, this function shall fail and return `NULL`.** ]** 
 
+**SRS_IOTHUBCLIENT_LL_25_125: [** `IoTHubClient_LL_CreateWithTransport` shall set the default retry policy as Exponential backoff with jitter and if succeed and return a `non-NULL` handle.** ]**
+
 
 
 ## IoTHubClient_LL_Destroy
@@ -342,6 +346,7 @@ extern IOTHUB_CLIENT_RESULT IoTHubClient_LL_GetSendStatus(IOTHUB_CLIENT_LL_HANDL
 extern IOTHUB_CLIENT_RESULT IoTHubClient_LL_SetConnectionStatusCallback(IOTHUB_CLIENT_LL_HANDLE iotHubClientHandle, IOTHUB_CLIENT_CONNECTION_STATUS_CALLBACK connectionStatusCallback, void* userContextCallback);
 ```
 **SRS_IOTHUBCLIENT_LL_25_111: [**IoTHubClient_LL_SetConnectionStatusCallback shall return IOTHUB_CLIENT_INVALID_ARG if called with NULL parameter iotHubClientHandle**]**
+
 **SRS_IOTHUBCLIENT_LL_25_112: [**IoTHubClient_LL_SetConnectionStatusCallback shall return IOTHUB_CLIENT_OK and save the callback and userContext as a member of the handle.**]**
 
 ###IotHubClient_LL_ConnectionStatusCallBack
@@ -359,17 +364,21 @@ IotHubClient_LL_ConnectionStatusCallBack is a function that is only called by th
 extern IOTHUB_CLIENT_RESULT IoTHubClient_LL_SetRetryPolicy(IOTHUB_CLIENT_LL_HANDLE iotHubClientHandle, IOTHUB_CLIENT_RETRY_POLICY retryPolicy, size_t retryTimeoutLimitinSeconds);
 ```
 **SRS_IOTHUBCLIENT_LL_25_116: [**IoTHubClient_LL_SetRetryPolicy shall return IOTHUB_CLIENT_INVALID_ARG if called with NULL iotHubClientHandle**]**
-**SRS_IOTHUBCLIENT_LL_25_117: [**For any policy other then IOTHUB_CLIENT_RETRY_NONE if retryTimeoutLimitinSeconds is zero then IoTHubClient_LL_SetRetryPolicy shall return IOTHUB_CLIENT_INVALID_ARG**]**
+
 **SRS_IOTHUBCLIENT_LL_25_118: [**IoTHubClient_LL_SetRetryPolicy shall save connection retry policies specified by the user to retryPolicy in struct IOTHUB_CLIENT_LL_HANDLE_DATA**]**
+
 **SRS_IOTHUBCLIENT_LL_25_119: [**IoTHubClient_LL_SetRetryPolicy shall save retryTimeoutLimitinSeconds in seconds to retryTimeout in struct IOTHUB_CLIENT_LL_HANDLE_DATA**]**
 
 ###IoTHubClient_LL_GetRetryPolicy
 ```c
 extern IOTHUB_CLIENT_RESULT IoTHubClient_LL_GetRetryPolicy(IOTHUB_CLIENT_LL_HANDLE iotHubClientHandle, IOTHUB_CLIENT_RETRY_POLICY* retryPolicy, size_t* retryTimeoutLimitinSeconds);
 ```
 **SRS_IOTHUBCLIENT_LL_25_120: [**IoTHubClient_LL_GetRetryPolicy shall return IOTHUB_CLIENT_INVALID_ARG if called with NULL iotHubClientHandle or retryPolicy or retryTimeoutLimitinSeconds parameters**]**
+
 **SRS_IOTHUBCLIENT_LL_25_121: [**IoTHubClient_LL_GetRetryPolicy shall retrieve connection retry policy from retryPolicy in struct IOTHUB_CLIENT_LL_HANDLE_DATA**]**
+
 **SRS_IOTHUBCLIENT_LL_25_122: [**IoTHubClient_LL_GetRetryPolicy shall retrieve retryTimeoutLimit in seconds from retryTimeoutinSeconds in struct IOTHUB_CLIENT_LL_HANDLE_DATA**]**
+
 **SRS_IOTHUBCLIENT_LL_25_123: [**If user did not set the policy and timeout values by calling IoTHubClient_LL_SetRetryPolicy then IoTHubClient_LL_GetRetryPolicy shall return default values**]**
 
 

diff --git a/c/iothub_client/src/iothub_client_ll.c b/c/iothub_client/src/iothub_client_ll.c
@@ -523,6 +523,13 @@ IOTHUB_CLIENT_LL_HANDLE IoTHubClient_LL_Create(const IOTHUB_CLIENT_CONFIG* confi
                             handleData->currentMessageTimeout = 0;
                             handleData->current_device_twin_timeout = 0;
                             result = handleData;
+                            /*Codes_SRS_IOTHUBCLIENT_LL_25_124: [ `IoTHubClient_LL_Create` shall set the default retry policy as Exponential backoff with jitter and if succeed and return a `non-NULL` handle. ]*/
+                            if (IoTHubClient_LL_SetRetryPolicy(handleData, IOTHUB_CLIENT_RETRY_EXPONENTIAL_BACKOFF_WITH_JITTER, 0) != IOTHUB_CLIENT_OK)
+                            {
+                                LogError("Setting default retry policy in transport failed");
+                                IoTHubClient_LL_Destroy(handleData);
+                                result = NULL;
+                            }
                         }
                     }
                 }
@@ -666,6 +673,13 @@ IOTHUB_CLIENT_LL_HANDLE IoTHubClient_LL_CreateWithTransport(const IOTHUB_CLIENT_
                                 handleData->currentMessageTimeout = 0;
                                 handleData->current_device_twin_timeout = 0;
                                 result = handleData;
+                                /*Codes_SRS_IOTHUBCLIENT_LL_25_125: [ `IoTHubClient_LL_CreateWithTransport` shall set the default retry policy as Exponential backoff with jitter and if succeed and return a `non-NULL` handle. ]*/
+                                if (IoTHubClient_LL_SetRetryPolicy(handleData, IOTHUB_CLIENT_RETRY_EXPONENTIAL_BACKOFF_WITH_JITTER, 0) != IOTHUB_CLIENT_OK)
+                                {
+                                    LogError("Setting default retry policy in transport failed");
+                                    IoTHubClient_LL_Destroy(handleData);
+                                    result = NULL;
+                                }
                             }
                         }
                     }
@@ -1183,13 +1197,7 @@ IOTHUB_CLIENT_RESULT IoTHubClient_LL_SetRetryPolicy(IOTHUB_CLIENT_LL_HANDLE iotH
         }
         else
         {
-            /*Codes_SRS_IOTHUBCLIENT_LL_25_117: [**For any policy other then IOTHUB_CLIENT_RETRY_NONE if retryTimeoutLimitInSeconds is zero then IoTHubClient_LL_SetRetryPolicy shall return IOTHUB_CLIENT_INVALID_ARG]*/
-            if (retryPolicy != IOTHUB_CLIENT_RETRY_NONE && retryTimeoutLimitInSeconds == 0)
-            {
-                result = IOTHUB_CLIENT_INVALID_ARG;
-                LOG_ERROR_RESULT;
-            }
-            else if (handleData->IoTHubTransport_SetRetryPolicy(handleData->transportHandle, retryPolicy, retryTimeoutLimitInSeconds) != 0)
+            if (handleData->IoTHubTransport_SetRetryPolicy(handleData->transportHandle, retryPolicy, retryTimeoutLimitInSeconds) != 0)
             {
                 result = IOTHUB_CLIENT_ERROR;
                 LOG_ERROR_RESULT;

diff --git a/c/iothub_client/src/iothubtransport_amqp_common.c b/c/iothub_client/src/iothubtransport_amqp_common.c
@@ -1924,8 +1924,8 @@ int IoTHubTransport_AMQP_Common_SetRetryPolicy(TRANSPORT_LL_HANDLE handle, IOTHU
     (void)retryPolicy;
     (void)retryTimeoutLimitInSeconds;
 
-    LogError("Not implemented for AMQP");
-    result = __LINE__;
+    LogInfo("Retry Policy is currently not available for AMQP");
+    result = 0;
     return result;
 }
 

diff --git a/c/iothub_client/src/iothubtransport_mqtt_common.c b/c/iothub_client/src/iothubtransport_mqtt_common.c
@@ -479,11 +479,11 @@ static bool CanRetry(RETRY_LOGIC *retryLogic)
 
                 if (retryLogic->fnRetryPolicy != NULL && (retryLogic->fnRetryPolicy(&permit, &delay, retryLogic) == 0))
                 {
-                    if ((permit == true) && retryLogic->retryTimeoutLimitInSeconds >= (delay + get_difftime(now, retryLogic->start)))
+                    if ((permit == true) && ((retryLogic->retryTimeoutLimitInSeconds == 0) || retryLogic->retryTimeoutLimitInSeconds >= (delay + get_difftime(now, retryLogic->start))))
                     {
                         retryLogic->delayFromLastConnectToRetry = delay;
 
-                        LogInfo("Evaluated delay %d at %d attempt\n", delay, retryLogic->retrycount);
+                        LogInfo("Evaluated delay %d at %d attempt to retry\n", delay, retryLogic->retrycount);
 
                         if (retryLogic->delayFromLastConnectToRetry <= ERROR_TIME_FOR_RETRY_SECS)
                         {

diff --git a/c/iothub_client/src/iothubtransporthttp.c b/c/iothub_client/src/iothubtransporthttp.c
@@ -2218,8 +2218,8 @@ static int IoTHubTransportHttp_SetRetryPolicy(TRANSPORT_LL_HANDLE handle, IOTHUB
     (void)retryPolicy;
     (void)retryTimeoutLimitInSeconds;
 
-    LogError("Not implemented for HTTP");
-    result = __LINE__;
+    LogInfo("Retry Policy is not currently not available for HTTP");
+    result = 0;
     return result;
 }
 

diff --git a/c/iothub_client/tests/iothubclient_ll_ut/iothubclient_ll_ut.c b/c/iothub_client/tests/iothubclient_ll_ut/iothubclient_ll_ut.c
@@ -309,7 +309,7 @@ static IOTHUB_DEVICE_HANDLE my_FAKE_IoTHubTransport_Register(TRANSPORT_LL_HANDLE
     return (IOTHUB_DEVICE_HANDLE)my_gballoc_malloc(1);
 }
 
-static void my_FAKE_IoTHubTransport_Unregister(IOTHUB_DEVICE_HANDLE handle)
+static void my_FAKE_IoTHubTransport_Unregister(TRANSPORT_LL_HANDLE handle)
 {
     my_gballoc_free(handle);
 }
@@ -572,6 +572,8 @@ static void setup_iothubclient_ll_create_mocks()
         .IgnoreArgument(2)
         .IgnoreArgument(3)
         .IgnoreArgument(4);
+
+    STRICT_EXPECTED_CALL(FAKE_IoTHubTransport_SetRetryPolicy(TEST_TRANSPORT_LL_HANDLE, TEST_RETRY_POLICY, 0));
 }
 
 static void setup_iothubclient_ll_sendreportedstate_mocks()
@@ -678,10 +680,15 @@ static void setup_iothubclient_ll_createwithtransport_mocks()
         .IgnoreArgument(2)
         .IgnoreArgument(3)
         .IgnoreArgument(4);
+    STRICT_EXPECTED_CALL(FAKE_IoTHubTransport_SetRetryPolicy(TEST_TRANSPORT_LL_HANDLE, TEST_RETRY_POLICY, 0))
+        .IgnoreArgument(1)
+        .IgnoreArgument(2)
+        .IgnoreArgument(3);
 #ifndef DONT_USE_UPLOADTOBLOB
     STRICT_EXPECTED_CALL(gballoc_free(IGNORED_PTR_ARG))
         .IgnoreArgument(1);
 #endif
+
 }
 
 /*Tests_SRS_IOTHUBCLIENT_LL_05_001: [IoTHubClient_LL_CreateFromConnectionString shall obtain the version string by a call to IoTHubClient_GetVersionString.]*/
@@ -721,7 +728,7 @@ TEST_FUNCTION(IoTHubClient_LL_CreateFromConnectionString_with_DeviceKey_fail)
     umock_c_negative_tests_snapshot();
 
     // act
-    size_t calls_cannot_fail[] = { 0, 10, 12, 13, 16, 19, 21, 24, 27, 28, 29, 30, 31, 32, 33, 36, 37, 38, 39, 40, 41, 42, 43, 44 };
+    size_t calls_cannot_fail[] = { 0, 10, 12, 13, 16, 19, 21, 24, 27, 28, 29, 30, 31, 32, 33, 37, 38, 39, 40, 41, 42, 43, 44, 45 };
     size_t count = umock_c_negative_tests_call_count();
     for (size_t index = 0; index < count; index++)
     {
@@ -1223,7 +1230,7 @@ TEST_FUNCTION(IoTHubClient_LL_CreateWithTransport_fail)
 
     // act
 #ifndef DONT_USE_UPLOADTOBLOB
-    size_t calls_cannot_fail[] = { 1, 2, 6, 7, 8, 10 };
+    size_t calls_cannot_fail[] = { 1, 2, 6, 7, 8, 11};
 #endif
     size_t count = umock_c_negative_tests_call_count();
     for (size_t index = 0; index < count; index++)
@@ -1764,8 +1771,7 @@ TEST_FUNCTION(IoTHubClient_LL_SetRetryPolicy_with_NULL_iotHubClientHandle_fails)
 
 }
 
-/*Tests_SRS_IOTHUBCLIENT_LL_25_117: [**For any policy other then IOTHUB_CLIENT_RETRY_NONE if retryTimeoutLimitinSeconds is zero then IoTHubClient_LL_SetRetryPolicy shall return IOTHUB_CLIENT_INVALID_ARG]*/
-TEST_FUNCTION(IoTHubClient_LL_SetRetryPolicy_Retrytimeout_0_fails)
+TEST_FUNCTION(IoTHubClient_LL_SetRetryPolicy_Retrytimeout_0_pass)
 {
     ///arrange
     IOTHUB_CLIENT_LL_HANDLE handle = IoTHubClient_LL_Create(&TEST_CONFIG);
@@ -1774,23 +1780,15 @@ TEST_FUNCTION(IoTHubClient_LL_SetRetryPolicy_Retrytimeout_0_fails)
     ///act
     for (int policy = 0; policy <= 6; policy++)
     {
-        if (policy == 0)
-        {
-            STRICT_EXPECTED_CALL(FAKE_IoTHubTransport_SetRetryPolicy(IGNORED_PTR_ARG, (IOTHUB_CLIENT_RETRY_POLICY)policy, 0))
+        STRICT_EXPECTED_CALL(FAKE_IoTHubTransport_SetRetryPolicy(IGNORED_PTR_ARG, (IOTHUB_CLIENT_RETRY_POLICY)policy, 0))
                 .IgnoreArgument(1);
-        }
+
         IOTHUB_CLIENT_RESULT result = IoTHubClient_LL_SetRetryPolicy(handle, (IOTHUB_CLIENT_RETRY_POLICY)policy, 0);
         ///assert
-        if (policy != 0)
-        {
-            ASSERT_ARE_EQUAL(IOTHUB_CLIENT_RESULT, IOTHUB_CLIENT_INVALID_ARG, result);
-            ASSERT_ARE_EQUAL(char_ptr, umock_c_get_expected_calls(), umock_c_get_actual_calls());
-        }
-        else
-        {
-            ASSERT_ARE_EQUAL(IOTHUB_CLIENT_RESULT, IOTHUB_CLIENT_OK, result);
-            ASSERT_ARE_EQUAL(char_ptr, umock_c_get_expected_calls(), umock_c_get_actual_calls());
-        }
+
+        ASSERT_ARE_EQUAL(IOTHUB_CLIENT_RESULT, IOTHUB_CLIENT_OK, result);
+        ASSERT_ARE_EQUAL(char_ptr, umock_c_get_expected_calls(), umock_c_get_actual_calls());
+
     }
 
     ///cleanup

diff --git a/c/iothub_client/tests/iothubtransport_mqtt_common_ut/iothubtransport_mqtt_common_ut.c b/c/iothub_client/tests/iothubtransport_mqtt_common_ut/iothubtransport_mqtt_common_ut.c
@@ -2379,6 +2379,47 @@ TEST_FUNCTION(IoTHubTransport_MQTT_Common_DoWork_Retry_Policy_First_connect_succ
     IoTHubTransport_MQTT_Common_Destroy(handle);
 }
 
+TEST_FUNCTION(IoTHubTransport_MQTT_Common_DoWork_Retry_Policy_First_Connect_Failed_Retry_Success_For_RetryTimeOut_0)
+{
+    // arrange
+    IOTHUBTRANSPORT_CONFIG config = { 0 };
+    SetupIothubTransportConfig(&config, TEST_DEVICE_ID, TEST_DEVICE_KEY, TEST_IOTHUB_NAME, TEST_IOTHUB_SUFFIX, TEST_PROTOCOL_GATEWAY_HOSTNAME);
+
+    TRANSPORT_LL_HANDLE handle = IoTHubTransport_MQTT_Common_Create(&config, get_IO_transport);
+    IoTHubTransport_MQTT_Common_SetRetryPolicy(handle, TEST_RETRY_POLICY, 0);
+    EXPECTED_CALL(get_time(IGNORED_PTR_ARG)).SetReturn(TEST_BIG_TIME_T);
+    IoTHubTransport_MQTT_Common_DoWork(handle, TEST_IOTHUB_CLIENT_LL_HANDLE);
+    CONNECT_ACK connack;
+    connack.isSessionPresent = false;
+    connack.returnCode = CONN_REFUSED_SERVER_UNAVAIL;
+
+    umock_c_reset_all_calls();
+    STRICT_EXPECTED_CALL(mqtt_client_disconnect(TEST_MQTT_CLIENT_HANDLE))
+        .IgnoreArgument(1);
+    // setup retry-setup mocks
+    EXPECTED_CALL(get_time(IGNORED_PTR_ARG)).SetReturn(TEST_BIG_TIME_T);
+    STRICT_EXPECTED_CALL(get_difftime(IGNORED_NUM_ARG, IGNORED_NUM_ARG))
+        .IgnoreArgument(1)
+        .IgnoreArgument(2)
+        .SetReturn(TEST_DIFF_GREATER_THAN_ERROR);
+    /* Evaluate retry Logic */
+    /* Check retry */
+    // As retry timeout is Zero, try forever.
+    setup_initialize_reconnection_mocks();
+    STRICT_EXPECTED_CALL(mqtt_client_dowork(TEST_MQTT_CLIENT_HANDLE))
+        .IgnoreArgument(1);
+
+    // act
+    g_fnMqttOperationCallback(TEST_MQTT_CLIENT_HANDLE, MQTT_CLIENT_ON_CONNACK, &connack, g_callbackCtx);
+    IoTHubTransport_MQTT_Common_DoWork(handle, TEST_IOTHUB_CLIENT_LL_HANDLE);
+
+    //assert
+    ASSERT_ARE_EQUAL(char_ptr, umock_c_get_expected_calls(), umock_c_get_actual_calls());
+
+    //cleanup
+    IoTHubTransport_MQTT_Common_Destroy(handle);
+}
+
 TEST_FUNCTION(IoTHubTransport_MQTT_Common_DoWork_Retry_Policy_First_Connect_Failed_Retry_Success)
 {
     // arrange