Http request counter (#1738)

* Add counter to request about the retry number using the Context

sdk/core/azure-core/CHANGELOG.md

@@ -14,6 +14,7 @@
 - Moved `NullBodyStream` to internal usage only. It is not meant for public use.
 - Removed `LimitBodyStream`.
 - Introduced `Azure::Core::CaseInsensitiveMap` which is now used to store headers in `Azure::Core::Http::Request` and `Azure::Core::Http::RawResponse`.
+- Removed `StartTry()` from `Azure::Core::Http::Request`.
 
 ### Bug Fixes
 

sdk/core/azure-core/inc/azure/core/http/http.hpp

@@ -29,6 +29,9 @@
 namespace Azure { namespace Core { namespace Test {
   class TestHttp_getters_Test;
   class TestHttp_query_parameter_Test;
+  class TestHttp_RequestStartTry_Test;
+  class TestURL_getters_Test;
+  class TestURL_query_parameter_Test;
 }}} // namespace Azure::Core::Test
 #endif
 
@@ -445,6 +448,9 @@ namespace Azure { namespace Core { namespace Http {
     // make tests classes friends to validate set Retry
     friend class Azure::Core::Test::TestHttp_getters_Test;
     friend class Azure::Core::Test::TestHttp_query_parameter_Test;
+    friend class Azure::Core::Test::TestHttp_RequestStartTry_Test;
+    friend class Azure::Core::Test::TestURL_getters_Test;
+    friend class Azure::Core::Test::TestURL_query_parameter_Test;
 #endif
 
   private:
@@ -464,6 +470,10 @@ namespace Azure { namespace Core { namespace Http {
     // adapter will decide chunk size.
     int64_t m_uploadChunkSize = 0;
 
+    // Expected to be called by a Retry policy to reset all headers set after this function was
+    // previously called
+    void StartTry();
+
   public:
     /**
      * @brief Construct an #Azure::Core::Http::Request.
@@ -583,9 +593,6 @@ namespace Azure { namespace Core { namespace Http {
      * @brief Get URL.
      */
     Url const& GetUrl() const { return this->m_url; }
-    // Expected to be called by a Retry policy to reset all headers set after this function was
-    // previously called
-    void StartTry();
   };
 
   /**

sdk/core/azure-core/inc/azure/core/http/policy.hpp

@@ -216,6 +216,19 @@ namespace Azure { namespace Core { namespace Http {
         Context const& ctx,
         Request& request,
         NextHttpPolicy nextHttpPolicy) const override;
+
+    /**
+     * @brief Get the Retry Count from the context.
+     *
+     * @remark The sentinel `-1` is returned if there is no information in the \p Context about
+     * #RetryPolicy is trying to send a request. Then `0` is returned for the first try of sending a
+     * request by the #RetryPolicy. Any subsequent retry will be referenced with a number greater
+     * than 0.
+     *
+     * @param context The context used to call send request.
+     * @return A positive number indicating the current intent to send the request.
+     */
+    static int GetRetryNumber(Context const& context);
   };
 
   /**

sdk/core/azure-core/src/http/retry_policy.cpp

@@ -122,15 +122,49 @@ bool ShouldRetryOnResponse(
 
   return true;
 }
+
+static constexpr char RetryKey[] = "AzureSdkRetryPolicyCounter";
+
+/**
+ * @brief Creates a new #Context node from \p parent with the information about the retrying while
+ * sending an Http request.
+ *
+ * @param parent The parent context for the new created.
+ * @return Context with information about retry counter.
+ */
+Context inline CreateRetryContext(Context const& parent)
+{
+  // First try as default
+  int retryCount = 0;
+  if (parent.HasKey(RetryKey))
+  {
+    retryCount = parent[RetryKey].Get<int>() + 1;
+  }
+  return parent.WithValue(RetryKey, retryCount);
+}
 } // namespace
 
+int Azure::Core::Http::RetryPolicy::GetRetryNumber(Context const& context)
+{
+  if (!context.HasKey(RetryKey))
+  {
+    // Context with no data abut sending request with retry policy = -1
+    // First try = 0
+    // Second try = 1
+    // third try = 2
+    // ...
+    return -1;
+  }
+  return context[RetryKey].Get<int>();
+}
+
 std::unique_ptr<RawResponse> Azure::Core::Http::RetryPolicy::Send(
     Context const& ctx,
     Request& request,
     NextHttpPolicy nextHttpPolicy) const
 {
   auto const shouldLog = Logging::Internal::ShouldLog(Logging::LogLevel::Informational);
-
+  auto retryContext = CreateRetryContext(ctx);
   for (RetryNumber attempt = 1;; ++attempt)
   {
     Delay retryAfter{};
@@ -140,7 +174,7 @@ std::unique_ptr<RawResponse> Azure::Core::Http::RetryPolicy::Send(
 
     try
     {
-      auto response = nextHttpPolicy.Send(ctx, request);
+      auto response = nextHttpPolicy.Send(retryContext, request);
 
       // If we are out of retry attempts, if a response is non-retriable (or simply 200 OK, i.e
       // doesn't need to be retried), then ShouldRetry returns false.
@@ -179,5 +213,8 @@ std::unique_ptr<RawResponse> Azure::Core::Http::RetryPolicy::Send(
 
     // Restore the original query parameters before next retry
     request.GetUrl().SetQueryParameters(std::move(originalQueryParameters));
+
+    // Update retry number
+    retryContext = CreateRetryContext(retryContext);
   }
 }

sdk/core/azure-core/test/ut/policy.cpp

@@ -23,6 +23,58 @@ public:
     return nullptr;
   }
 };
+
+// A policy to test retry state
+static int retryCounterState = 0;
+struct TestRetryPolicySharedState : public Azure::Core::Http::HttpPolicy
+{
+  std::unique_ptr<HttpPolicy> Clone() const override
+  {
+    return std::make_unique<TestRetryPolicySharedState>(*this);
+  }
+
+  std::unique_ptr<Azure::Core::Http::RawResponse> Send(
+      Azure::Core::Context const& ctx,
+      Azure::Core::Http::Request& request,
+      Azure::Core::Http::NextHttpPolicy nextHttpPolicy) const override
+  {
+    EXPECT_EQ(retryCounterState, Azure::Core::Http::RetryPolicy::GetRetryNumber(ctx));
+    retryCounterState += 1;
+    return nextHttpPolicy.Send(ctx, request);
+  }
+};
+
+class SuccessAfter : public Azure::Core::Http::HttpPolicy {
+private:
+  int m_successAfter; // Always success
+
+public:
+  std::unique_ptr<Azure::Core::Http::HttpPolicy> Clone() const override
+  {
+    return std::make_unique<SuccessAfter>(*this);
+  }
+
+  SuccessAfter(int successAfter = 1) : m_successAfter(successAfter) {}
+
+  std::unique_ptr<Azure::Core::Http::RawResponse> Send(
+      Azure::Core::Context const& context,
+      Azure::Core::Http::Request&,
+      Azure::Core::Http::NextHttpPolicy) const override
+  {
+    auto retryNumber = Azure::Core::Http::RetryPolicy::GetRetryNumber(context);
+    if (retryNumber == m_successAfter)
+    {
+      auto response = std::make_unique<Azure::Core::Http::RawResponse>(
+          1, 1, Azure::Core::Http::HttpStatusCode::Ok, "All Fine");
+      return response;
+    }
+
+    auto retryResponse = std::make_unique<Azure::Core::Http::RawResponse>(
+        1, 1, Azure::Core::Http::HttpStatusCode::ServiceUnavailable, "retry please :)");
+    return retryResponse;
+  }
+};
+
 } // namespace
 
 TEST(Policy, throwWhenNoTransportPolicy)
@@ -88,3 +140,44 @@ TEST(Policy, ValuePolicy)
   ASSERT_EQ(headers, decltype(headers)({{"hdrkey1", "HdrVal1"}, {"hdrkey2", "HdrVal2"}}));
   ASSERT_EQ(queryParams, decltype(queryParams)({{"QryKey1", "QryVal1"}, {"QryKey2", "QryVal2"}}));
 }
+
+TEST(Policy, RetryPolicyCounter)
+{
+  using namespace Azure::Core;
+  using namespace Azure::Core::Http;
+  using namespace Azure::Core::Internal::Http;
+
+  // Check when there's no info about retry on the context
+  auto initialContext = GetApplicationContext();
+  EXPECT_EQ(-1, RetryPolicy::GetRetryNumber(initialContext));
+
+  // Pipeline with retry test
+  std::vector<std::unique_ptr<HttpPolicy>> policies;
+  RetryOptions opt;
+  policies.push_back(std::make_unique<RetryPolicy>(opt));
+  policies.push_back(std::make_unique<TestRetryPolicySharedState>());
+  policies.push_back(std::make_unique<SuccessAfter>());
+
+  HttpPipeline pipeline(policies);
+  Request request(HttpMethod::Get, Url("url"));
+  pipeline.Send(initialContext, request);
+}
+
+TEST(Policy, RetryPolicyRetryCycle)
+{
+  using namespace Azure::Core;
+  using namespace Azure::Core::Http;
+  using namespace Azure::Core::Internal::Http;
+
+  // Pipeline with retry test
+  std::vector<std::unique_ptr<HttpPolicy>> policies;
+  RetryOptions opt;
+  opt.RetryDelay = std::chrono::milliseconds(10);
+  policies.push_back(std::make_unique<RetryPolicy>(opt));
+  policies.push_back(std::make_unique<TestRetryPolicySharedState>());
+  policies.push_back(std::make_unique<SuccessAfter>(3));
+
+  HttpPipeline pipeline(policies);
+  Request request(HttpMethod::Get, Url("url"));
+  pipeline.Send(GetApplicationContext(), request);
+}

sdk/core/azure-core/test/ut/url.cpp

@@ -5,11 +5,18 @@
 
 #include <azure/core/http/http.hpp>
 
+namespace Azure { namespace Core { namespace Test {
+
+  class TestURL : public ::testing::Test {
+  };
+
+}}} // namespace Azure::Core::Test
+
 using namespace Azure::Core;
 
 namespace Azure { namespace Core { namespace Test {
 
-  TEST(URL, getters)
+  TEST(TestURL, getters)
   {
     Http::HttpMethod httpMethod = Http::HttpMethod::Get;
     Http::Url url("http://test.url.com");
@@ -79,7 +86,7 @@ namespace Azure { namespace Core { namespace Test {
     EXPECT_FALSE(headers.count("newheader"));
   }
 
-  TEST(URL, query_parameter)
+  TEST(TestURL, query_parameter)
   {
     Http::HttpMethod httpMethod = Http::HttpMethod::Put;
     Http::Url url("http://test.com");