Add RetrySettings for authentication (#1251)

olp::authentication::AuthenticationSettings and
olp::authentication::Settingsstructs are extended with RetrySettings.

AuthentificationClientImpl now uses user defined retry settings
instead of hardcoded values.

Fixed double token request bug in TokenProvider.

Relates-To: OLPEDGE-2634, OLPSUP-15460

Signed-off-by: Mykola Malik <[email protected]>

Author: mykolamalik

olp-cpp-sdk-authentication/include/olp/authentication/AuthenticationSettings.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2020 HERE Europe B.V.
+ * Copyright (C) 2020-2021 HERE Europe B.V.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@
 #include <string>
 
 #include <olp/authentication/AuthenticationApi.h>
+#include <olp/core/client/RetrySettings.h>
 #include <olp/core/http/NetworkProxySettings.h>
 #include <boost/optional.hpp>
 
@@ -89,6 +90,12 @@ struct AUTHENTICATION_API AuthenticationSettings {
    * authentication server.
    */
   bool use_system_time{true};
+
+  /**
+   * @brief A collection of settings that controls how failed requests should be
+   * treated.
+   */
+  client::RetrySettings retry_settings;
 };
 
 }  // namespace authentication

olp-cpp-sdk-authentication/include/olp/authentication/Settings.h

@@ -23,6 +23,7 @@
 #include <memory>
 #include <string>
 
+#include <olp/core/client/RetrySettings.h>
 #include <olp/core/http/NetworkProxySettings.h>
 
 #include "AuthenticationApi.h"
@@ -98,6 +99,12 @@ struct AUTHENTICATION_API Settings {
    * authentication server.
    */
   bool use_system_time{true};
+
+  /**
+   * @brief A collection of settings that controls how failed requests should be
+   * treated.
+   */
+  client::RetrySettings retry_settings;
 };
 
 }  // namespace authentication

olp-cpp-sdk-authentication/include/olp/authentication/TokenProvider.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2019-2020 HERE Europe B.V.
+ * Copyright (C) 2019-2021 HERE Europe B.V.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -119,9 +119,9 @@ class TokenProvider {
 
     /// @copydoc TokenProvider::operator()()
     std::string operator()() const {
-      return GetResponse().IsSuccessful()
-                 ? GetResponse().GetResult().GetAccessToken()
-                 : "";
+      auto response = GetResponse();
+      return response.IsSuccessful() ? response.GetResult().GetAccessToken()
+                                     : "";
     }
 
     /// @copydoc TokenProvider::GetErrorResponse()

olp-cpp-sdk-authentication/src/AuthenticationClientImpl.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2020 HERE Europe B.V.
+ * Copyright (C) 2020-2021 HERE Europe B.V.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -99,19 +99,17 @@ constexpr auto kOperator = "operator";
 constexpr auto kErrorWrongTimestamp = 401204;
 constexpr auto kLogTag = "AuthenticationClient";
 
-constexpr auto kDefaultRetryCount = 3;
-constexpr auto kDefaultRetryTime = std::chrono::milliseconds(200);
-
 bool HasWrongTimestamp(olp::authentication::SignInResult& result) {
   const auto& error_response = result.GetErrorResponse();
   const auto status = result.GetStatus();
   return status == olp::http::HttpStatusCode::UNAUTHORIZED &&
          error_response.code == kErrorWrongTimestamp;
 }
 
-void RetryDelay(size_t retry) {
-  client::ExponentialBackdownStrategy retry_delay;
-  std::this_thread::sleep_for(retry_delay(kDefaultRetryTime, retry));
+void RetryDelay(const client::RetrySettings& retry_settings, size_t retry) {
+  std::this_thread::sleep_for(retry_settings.backdown_strategy(
+      std::chrono::milliseconds(retry_settings.initial_backdown_period),
+      retry));
 }
 
 client::OlpClient::RequestBodyType GenerateAppleSignInBody(
@@ -284,7 +282,9 @@ client::CancellationToken AuthenticationClientImpl::SignInClient(
 
     SignInResult response;
 
-    for (auto retry = 0; retry < kDefaultRetryCount; ++retry) {
+    const auto& retry_settings = settings_.retry_settings;
+
+    for (auto retry = 0; retry < retry_settings.max_attempts; ++retry) {
       if (context.IsCancelled()) {
         return client::ApiError::Cancelled();
       }
@@ -310,8 +310,8 @@ client::CancellationToken AuthenticationClientImpl::SignInClient(
 
       response = ParseAuthResponse(status, auth_response.response);
 
-      if (client::DefaultRetryCondition(auth_response)) {
-        RetryDelay(retry);
+      if (retry_settings.retry_condition(auth_response)) {
+        RetryDelay(retry_settings, retry);
         continue;
       }
 
@@ -492,7 +492,9 @@ client::CancellationToken AuthenticationClientImpl::HandleUserRequest(
 
     SignInUserResult response;
 
-    for (auto retry = 0; retry < kDefaultRetryCount; ++retry) {
+    const auto& retry_settings = settings_.retry_settings;
+
+    for (auto retry = 0; retry < retry_settings.max_attempts; ++retry) {
       if (context.IsCancelled()) {
         return client::ApiError::Cancelled();
       }
@@ -517,8 +519,8 @@ client::CancellationToken AuthenticationClientImpl::HandleUserRequest(
 
       response = ParseUserAuthResponse(status, auth_response.response);
 
-      if (client::DefaultRetryCondition(auth_response)) {
-        RetryDelay(retry);
+      if (retry_settings.retry_condition(auth_response)) {
+        RetryDelay(retry_settings, retry);
         continue;
       }
 
@@ -560,10 +562,12 @@ client::CancellationToken AuthenticationClientImpl::SignUpHereUser(
   std::string url = settings_.token_endpoint_url;
   url.append(kUserEndpoint);
   http::NetworkRequest request(url);
-  http::NetworkSettings network_settings;
-  if (settings_.network_proxy_settings) {
-    network_settings.WithProxySettings(settings_.network_proxy_settings.get());
-  }
+  auto network_settings =
+      http::NetworkSettings()
+          .WithTransferTimeout(settings_.retry_settings.timeout)
+          .WithConnectionTimeout(settings_.retry_settings.timeout)
+          .WithProxySettings(settings_.network_proxy_settings.get_value_or({}));
+
   request.WithVerb(http::NetworkRequest::HttpVerb::POST);
 
   auto auth_header = GenerateAuthorizationHeader(
@@ -637,10 +641,12 @@ client::CancellationToken AuthenticationClientImpl::SignOut(
   std::string url = settings_.token_endpoint_url;
   url.append(kSignoutEndpoint);
   http::NetworkRequest request(url);
-  http::NetworkSettings network_settings;
-  if (settings_.network_proxy_settings) {
-    network_settings.WithProxySettings(settings_.network_proxy_settings.get());
-  }
+  auto network_settings =
+      http::NetworkSettings()
+          .WithTransferTimeout(settings_.retry_settings.timeout)
+          .WithConnectionTimeout(settings_.retry_settings.timeout)
+          .WithProxySettings(settings_.network_proxy_settings.get_value_or({}));
+
   request.WithVerb(http::NetworkRequest::HttpVerb::POST);
   request.WithHeader(http::kAuthorizationHeader,
                      GenerateBearerHeader(access_token));

olp-cpp-sdk-authentication/src/AuthenticationClientUtils.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2020 HERE Europe B.V.
+ * Copyright (C) 2020-2021 HERE Europe B.V.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -264,8 +264,7 @@ client::OlpClient CreateOlpClient(
   settings.network_request_handler = auth_settings.network_request_handler;
   settings.authentication_settings = authentication_settings;
   settings.proxy_settings = auth_settings.network_proxy_settings;
-  settings.retry_settings.backdown_strategy =
-      client::ExponentialBackdownStrategy();
+  settings.retry_settings = auth_settings.retry_settings;
 
   if (!retry) {
     settings.retry_settings.max_attempts = 0;

olp-cpp-sdk-authentication/src/TokenEndpoint.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2019-2020 HERE Europe B.V.
+ * Copyright (C) 2019-2021 HERE Europe B.V.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -45,6 +45,7 @@ AuthenticationSettings ConvertSettings(Settings settings) {
   auth_settings.network_request_handler = settings.network_request_handler;
   auth_settings.token_endpoint_url = settings.token_endpoint_url;
   auth_settings.use_system_time = settings.use_system_time;
+  auth_settings.retry_settings = settings.retry_settings;
   return auth_settings;
 }
 }  // namespace

olp-cpp-sdk-core/CMakeLists.txt

@@ -81,6 +81,7 @@ set(OLP_SDK_CLIENT_HEADERS
     ./include/olp/core/client/OlpClientSettings.h
     ./include/olp/core/client/OlpClientSettingsFactory.h
     ./include/olp/core/client/PendingRequests.h
+    ./include/olp/core/client/RetrySettings.h
     ./include/olp/core/client/TaskContext.h
 )
 
@@ -253,11 +254,11 @@ set(OLP_SDK_CLIENT_SOURCES
     ./src/client/HRN.cpp
     ./src/client/OlpClient.cpp
     ./src/client/OlpClientFactory.cpp
-    ./src/client/OlpClientSettings.cpp
     ./src/client/OlpClientSettingsFactory.cpp
     ./src/client/PendingRequests.cpp
     ./src/client/PendingUrlRequests.h
     ./src/client/PendingUrlRequests.cpp
+    ./src/client/RetrySettings.cpp
     ./src/client/Tokenizer.h
 )
 

olp-cpp-sdk-core/include/olp/core/client/OlpClientSettings.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2019-2020 HERE Europe B.V.
+ * Copyright (C) 2019-2021 HERE Europe B.V.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -26,11 +26,11 @@
 
 #include <boost/optional.hpp>
 
-#include <olp/core/client/BackdownStrategy.h>
 #include <olp/core/client/CancellationToken.h>
 #include <olp/core/client/DefaultLookupEndpointProvider.h>
 #include <olp/core/client/HRN.h>
 #include <olp/core/client/HttpResponse.h>
+#include <olp/core/client/RetrySettings.h>
 #include <olp/core/http/Network.h>
 
 namespace olp {
@@ -61,11 +61,6 @@ using NetworkAsyncCallback = std::function<void(HttpResponse)>;
  */
 using NetworkAsyncCancel = std::function<void()>;
 
-/**
- * @brief The default retry condition that disables retries.
- */
-CORE_API bool DefaultRetryCondition(const olp::client::HttpResponse& response);
-
 /**
  * @brief A set of settings that manages the `TokenProviderCallback` and
  * `TokenProviderCancelCallback` functions.
@@ -146,63 +141,6 @@ struct CORE_API AuthenticationSettings {
   boost::optional<TokenProviderCancelCallback> cancel;
 };
 
-/**
- * @brief A collection of settings that controls how failed requests should be
- * treated by the Data SDK.
- *
- * For example, it specifies whether the failed request should be retried, how
- * long Data SDK needs to wait for the next retry attempt, the number of maximum
- * retries, and so on.
- *
- * You can customize all of these settings. The settings are used internally by
- * the `OlpClient` class.
- */
-struct CORE_API RetrySettings {
-  /**
-   * @brief Calculates the number of retry timeouts based on
-   * the initial backdown duration and retries count.
-   */
-  using BackdownStrategy = std::function<std::chrono::milliseconds(
-      std::chrono::milliseconds, size_t)>;
-
-  /**
-   * @brief Checks whether the retry is desired.
-   *
-   * @see `HttpResponse` for more details.
-   */
-  using RetryCondition = std::function<bool(const HttpResponse&)>;
-
-  /**
-   * @brief The number of attempts.
-   *
-   * The default value is 3.
-   */
-  int max_attempts = 3;
-
-  /**
-   * @brief The connection timeout limit (in seconds).
-   */
-  int timeout = 60;
-
-  /**
-   * @brief The period between the error and the first retry attempt (in
-   * milliseconds).
-   */
-  int initial_backdown_period = 200;
-
-  /**
-   * @brief The backdown strategy.
-   *
-   * Defines the delay between retries on a failed request.
-   */
-  BackdownStrategy backdown_strategy = ExponentialBackdownStrategy();
-
-  /**
-   * @brief Evaluates responses to determine if the retry should be attempted.
-   */
-  RetryCondition retry_condition = DefaultRetryCondition;
-};
-
 /**
  * @brief Settings to provide URLs for API lookup requests.
  */