[cpp-ue4] Improved retry system to use Unreal's FHttpRetrySystem (#9382)

* Revert "[cpp-ue4] Added the possibility to retry requests easily with AsyncRetry method on the response and SetAutoRetryCount on the request" * [cpp-ue4] Improved retry system to use Unreal's FHttpRetrySystem * [cpp-ue4] Updated style guide link * update samples Co-authored-by: William Cheng <wing328hk@gmail.com>
OpenAPITools · May 6, 2021 · 343d7eb · 343d7eb
1 parent 32b2ea3
commit 343d7eb
Show file tree

Hide file tree

Showing 13 changed files with 394 additions and 380 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -48,6 +48,7 @@ Code change should conform to the programming style guide of the respective lang
 - C#: https://msdn.microsoft.com/en-us/library/vstudio/ff926074.aspx
 - C++: https://google.github.io/styleguide/cppguide.html
 - C++ (Tizen): https://wiki.tizen.org/Native_Platform_Coding_Idiom_and_Style_Guide#C.2B.2B_Coding_Style
+- C++ (Unreal Engine 4): https://docs.unrealengine.com/en-US/ProductionPipelines/DevelopmentSetup/CodingStandard/index.html
 - Clojure: https://github.com/bbatsov/clojure-style-guide
 - Crystal: https://crystal-lang.org/reference/conventions/coding_style.html
 - Dart: https://www.dartlang.org/guides/language/effective-dart/style

diff --git a/modules/openapi-generator/src/main/resources/cpp-ue4/api-header.mustache b/modules/openapi-generator/src/main/resources/cpp-ue4/api-header.mustache
@@ -15,9 +15,19 @@ public:
 	{{classname}}();
 	~{{classname}}();
 
+	/* Sets the URL Endpoint. 
+	* Note: several fallback endpoints can be configured in request retry policies, see Request::SetShouldRetry */
 	void SetURL(const FString& Url);
+
+	/* Adds global header params to all requests */
 	void AddHeaderParam(const FString& Key, const FString& Value);
 	void ClearHeaderParams();
+
+	/* Sets the retry manager to the user-defined retry manager. User must manage the lifetime of the retry manager.
+	* If no retry manager is specified and a request needs retries, a default retry manager will be used. 
+	* See also: Request::SetShouldRetry */
+	void SetHttpRetryManager(FHttpRetrySystem::FManager& RetryManager);
+	FHttpRetrySystem::FManager& GetHttpRetryManager();
 
 	{{#operations}}{{#operation}}class {{operationIdCamelCase}}Request;
 	class {{operationIdCamelCase}}Response;
@@ -27,15 +37,17 @@ public:
     {{#operations}}{{#operation}}{{#description}}/* {{{description}}} */
 	{{/description}}bool {{operationIdCamelCase}}(const {{operationIdCamelCase}}Request& Request, const F{{operationIdCamelCase}}Delegate& Delegate = F{{operationIdCamelCase}}Delegate()) const;
     {{/operation}}{{/operations}}
-
 private:
-    {{#operations}}{{#operation}}void On{{operationIdCamelCase}}Response(FHttpRequestPtr HttpRequest, FHttpResponsePtr HttpResponse, bool bSucceeded, F{{operationIdCamelCase}}Delegate Delegate, int AutoRetryCount) const;
+    {{#operations}}{{#operation}}void On{{operationIdCamelCase}}Response(FHttpRequestPtr HttpRequest, FHttpResponsePtr HttpResponse, bool bSucceeded, F{{operationIdCamelCase}}Delegate Delegate) const;
     {{/operation}}{{/operations}}
+	FHttpRequestRef CreateHttpRequest(const Request& Request) const;
 	bool IsValid() const;
 	void HandleResponse(FHttpResponsePtr HttpResponse, bool bSucceeded, Response& InOutResponse) const;
 
 	FString Url;
 	TMap<FString,FString> AdditionalHeaderParams;
+	mutable FHttpRetrySystem::FManager* RetryManager = nullptr;
+	mutable TUniquePtr<HttpRetryManager> DefaultRetryManager;
 };
 
 {{#cppNamespaceDeclarations}}

diff --git a/modules/openapi-generator/src/main/resources/cpp-ue4/api-source.mustache b/modules/openapi-generator/src/main/resources/cpp-ue4/api-source.mustache
@@ -45,6 +45,40 @@ bool {{classname}}::IsValid() const
 	return true;
 }
 
+void {{classname}}::SetHttpRetryManager(FHttpRetrySystem::FManager& InRetryManager)
+{
+	if(RetryManager != &GetHttpRetryManager())
+	{
+		DefaultRetryManager.Reset();
+		RetryManager = &InRetryManager;
+	}
+}
+
+FHttpRetrySystem::FManager& {{classname}}::GetHttpRetryManager()
+{
+	return *RetryManager;
+}
+
+FHttpRequestRef {{classname}}::CreateHttpRequest(const Request& Request) const
+{
+	if (!Request.GetRetryParams().IsSet())
+	{
+		return FHttpModule::Get().CreateRequest();
+	}
+	else
+	{
+		if (!RetryManager)
+		{
+			// Create default retry manager if none was specified
+			DefaultRetryManager = MakeUnique<HttpRetryManager>(6, 60);
+			RetryManager = DefaultRetryManager.Get();
+		}
+
+		const HttpRetryParams& Params = Request.GetRetryParams().GetValue();
+		return RetryManager->CreateRequest(Params.RetryLimitCountOverride, Params.RetryTimeoutRelativeSecondsOverride, Params.RetryResponseCodes, Params.RetryVerbs, Params.RetryDomains);
+	}
+}
+
 void {{classname}}::HandleResponse(FHttpResponsePtr HttpResponse, bool bSucceeded, Response& InOutResponse) const
 {
 	InOutResponse.SetHttpResponse(HttpResponse);
@@ -96,7 +130,7 @@ bool {{classname}}::{{operationIdCamelCase}}(const {{operationIdCamelCase}}Reque
 	if (!IsValid())
 		return false;
 
-	FHttpRequestRef HttpRequest = FHttpModule::Get().CreateRequest();
+	FHttpRequestRef HttpRequest = CreateHttpRequest(Request);
 	HttpRequest->SetURL(*(Url + Request.ComputePath()));
 
 	for(const auto& It : AdditionalHeaderParams)
@@ -106,26 +140,15 @@ bool {{classname}}::{{operationIdCamelCase}}(const {{operationIdCamelCase}}Reque
 
 	Request.SetupHttpRequest(HttpRequest);
 
-	HttpRequest->OnProcessRequestComplete().BindRaw(this, &{{classname}}::On{{operationIdCamelCase}}Response, Delegate, Request.GetAutoRetryCount());
+	HttpRequest->OnProcessRequestComplete().BindRaw(this, &{{classname}}::On{{operationIdCamelCase}}Response, Delegate);
 	return HttpRequest->ProcessRequest();
 }
 
-void {{classname}}::On{{operationIdCamelCase}}Response(FHttpRequestPtr HttpRequest, FHttpResponsePtr HttpResponse, bool bSucceeded, F{{operationIdCamelCase}}Delegate Delegate, int AutoRetryCount) const
+void {{classname}}::On{{operationIdCamelCase}}Response(FHttpRequestPtr HttpRequest, FHttpResponsePtr HttpResponse, bool bSucceeded, F{{operationIdCamelCase}}Delegate Delegate) const
 {
 	{{operationIdCamelCase}}Response Response;
-	Response.SetHttpRequest(HttpRequest);
-
 	HandleResponse(HttpResponse, bSucceeded, Response);
-
-	if(!Response.IsSuccessful() && AutoRetryCount > 0)
-	{
-		HttpRequest->OnProcessRequestComplete().BindRaw(this, &{{classname}}::On{{operationIdCamelCase}}Response, Delegate, AutoRetryCount - 1);
-		Response.AsyncRetry();
-	}
-	else
-	{
-		Delegate.ExecuteIfBound(Response);
-	}
+	Delegate.ExecuteIfBound(Response);
 }
 
 {{/operation}}

diff --git a/modules/openapi-generator/src/main/resources/cpp-ue4/model-base-header.mustache b/modules/openapi-generator/src/main/resources/cpp-ue4/model-base-header.mustache
@@ -5,13 +5,40 @@
 #include "Interfaces/IHttpResponse.h"
 #include "Serialization/JsonWriter.h"
 #include "Dom/JsonObject.h"
+#include "HttpRetrySystem.h"
+#include "Containers/Ticker.h"
 
 {{#cppNamespaceDeclarations}}
 namespace {{this}} 
 {
 {{/cppNamespaceDeclarations}}
 
 typedef TSharedRef<TJsonWriter<>> JsonWriter;
+using namespace FHttpRetrySystem;
+
+struct {{dllapi}} HttpRetryManager : public FManager, public FTickerObjectBase
+{
+	using FManager::FManager;
+
+	bool Tick(float DeltaTime) final;
+};
+
+struct {{dllapi}} HttpRetryParams
+{
+	HttpRetryParams(
+		const FRetryLimitCountSetting& InRetryLimitCountOverride = FRetryLimitCountSetting(),
+		const FRetryTimeoutRelativeSecondsSetting& InRetryTimeoutRelativeSecondsOverride = FRetryTimeoutRelativeSecondsSetting(),
+		const FRetryResponseCodes& InRetryResponseCodes = FRetryResponseCodes(),
+		const FRetryVerbs& InRetryVerbs = FRetryVerbs(),
+		const FRetryDomainsPtr& InRetryDomains = FRetryDomainsPtr()
+	);
+
+	FRetryLimitCountSetting              RetryLimitCountOverride;
+	FRetryTimeoutRelativeSecondsSetting  RetryTimeoutRelativeSecondsOverride;
+	FRetryResponseCodes					 RetryResponseCodes;
+	FRetryVerbs                          RetryVerbs;
+	FRetryDomainsPtr					 RetryDomains;
+};
 
 class {{dllapi}} Model
 { 
@@ -28,11 +55,12 @@ public:
 	virtual void SetupHttpRequest(const FHttpRequestRef& HttpRequest) const = 0;
 	virtual FString ComputePath() const = 0;
 
-	void SetAutoRetryCount(int InCount) { AutoRetryCount = InCount; }
-	int GetAutoRetryCount() const { return AutoRetryCount; }
+	/* Enables retry and optionally sets a retry policy for this request */
+	void SetShouldRetry(const HttpRetryParams& Params = HttpRetryParams()) { RetryParams = Params; }
+	const TOptional<HttpRetryParams>& GetRetryParams() const { return RetryParams; }
 
 private:
-	int AutoRetryCount = 0;
+	TOptional<HttpRetryParams> RetryParams;
 };
 
 class {{dllapi}} Response
@@ -44,8 +72,6 @@ public:
 	void SetSuccessful(bool InSuccessful) { Successful = InSuccessful; }
 	bool IsSuccessful() const { return Successful; }
 
-	void AsyncRetry() const;
-
 	virtual void SetHttpResponseCode(EHttpResponseCodes::Type InHttpResponseCode);
 	EHttpResponseCodes::Type GetHttpResponseCode() const { return ResponseCode; }
 
@@ -55,15 +81,11 @@ public:
 	void SetHttpResponse(const FHttpResponsePtr& InHttpResponse) { HttpResponse = InHttpResponse; }
 	const FHttpResponsePtr& GetHttpResponse() const { return HttpResponse; }
 
-	void SetHttpRequest(const FHttpRequestPtr& InHttpRequest) { HttpRequest = InHttpRequest; }
-	const FHttpRequestPtr& GetHttpRequest() const { return HttpRequest; }
-
 private:
 	bool Successful;
 	EHttpResponseCodes::Type ResponseCode;
 	FString ResponseString;
 	FHttpResponsePtr HttpResponse;
-	FHttpRequestPtr HttpRequest;
 };
 
 {{#cppNamespaceDeclarations}}

diff --git a/modules/openapi-generator/src/main/resources/cpp-ue4/model-base-source.mustache b/modules/openapi-generator/src/main/resources/cpp-ue4/model-base-source.mustache
@@ -1,13 +1,30 @@
 {{>licenseInfo}}
 #include "{{modelNamePrefix}}BaseModel.h"
 
-#include "Async/Async.h"
-
 {{#cppNamespaceDeclarations}}
 namespace {{this}} 
 {
 {{/cppNamespaceDeclarations}}
 
+bool HttpRetryManager::Tick(float DeltaTime)
+{
+	FManager::Update();
+	return true;
+}
+
+HttpRetryParams::HttpRetryParams(const FRetryLimitCountSetting& InRetryLimitCountOverride /*= FRetryLimitCountSetting()*/, 
+    const FRetryTimeoutRelativeSecondsSetting& InRetryTimeoutRelativeSecondsOverride /*= FRetryTimeoutRelativeSecondsSetting()*/, 
+    const FRetryResponseCodes& InRetryResponseCodes /*= FRetryResponseCodes()*/, 
+    const FRetryVerbs& InRetryVerbs /*= FRetryVerbs()*/, 
+    const FRetryDomainsPtr& InRetryDomains /*= FRetryDomainsPtr() */)
+    : RetryLimitCountOverride(InRetryLimitCountOverride)
+    , RetryTimeoutRelativeSecondsOverride(InRetryTimeoutRelativeSecondsOverride)
+    , RetryResponseCodes(InRetryResponseCodes)
+    , RetryVerbs(InRetryVerbs)
+    , RetryDomains(InRetryDomains)
+{
+}
+
 void Response::SetHttpResponseCode(EHttpResponseCodes::Type InHttpResponseCode)
 {
     ResponseCode = InHttpResponseCode;
@@ -18,15 +35,6 @@ void Response::SetHttpResponseCode(EHttpResponseCodes::Type InHttpResponseCode)
     }
 }
 
-void Response::AsyncRetry() const
-{
-    // Unfortunately, it is currently usafe to call ProcessRequest() directly here.
-    // This is because the HttpManager will remove all references to this HttpRequest in FHttpManager::Tick including the new request we just added, instead of removing just one. 
-    // This will lead to the request's destruction and eventually a crash.
-    // The only solution is therefore to ensure we are taking an extra reference to the request, and that the request is added after the queue is flushed.
-    Async(EAsyncExecution::TaskGraph, [AddRef = FHttpRequestPtr(GetHttpRequest())](){ AddRef->ProcessRequest(); });
-}
-
 {{#cppNamespaceDeclarations}}
 }
 {{/cppNamespaceDeclarations}}
diff --git a/samples/client/petstore/cpp-ue4/Private/OpenAPIBaseModel.cpp b/samples/client/petstore/cpp-ue4/Private/OpenAPIBaseModel.cpp
@@ -12,11 +12,28 @@
 
 #include "OpenAPIBaseModel.h"
 
-#include "Async/Async.h"
-
 namespace OpenAPI 
 {
 
+bool HttpRetryManager::Tick(float DeltaTime)
+{
+	FManager::Update();
+	return true;
+}
+
+HttpRetryParams::HttpRetryParams(const FRetryLimitCountSetting& InRetryLimitCountOverride /*= FRetryLimitCountSetting()*/, 
+    const FRetryTimeoutRelativeSecondsSetting& InRetryTimeoutRelativeSecondsOverride /*= FRetryTimeoutRelativeSecondsSetting()*/, 
+    const FRetryResponseCodes& InRetryResponseCodes /*= FRetryResponseCodes()*/, 
+    const FRetryVerbs& InRetryVerbs /*= FRetryVerbs()*/, 
+    const FRetryDomainsPtr& InRetryDomains /*= FRetryDomainsPtr() */)
+    : RetryLimitCountOverride(InRetryLimitCountOverride)
+    , RetryTimeoutRelativeSecondsOverride(InRetryTimeoutRelativeSecondsOverride)
+    , RetryResponseCodes(InRetryResponseCodes)
+    , RetryVerbs(InRetryVerbs)
+    , RetryDomains(InRetryDomains)
+{
+}
+
 void Response::SetHttpResponseCode(EHttpResponseCodes::Type InHttpResponseCode)
 {
     ResponseCode = InHttpResponseCode;
@@ -27,13 +44,4 @@ void Response::SetHttpResponseCode(EHttpResponseCodes::Type InHttpResponseCode)
     }
 }
 
-void Response::AsyncRetry() const
-{
-    // Unfortunately, it is currently usafe to call ProcessRequest() directly here.
-    // This is because the HttpManager will remove all references to this HttpRequest in FHttpManager::Tick including the new request we just added, instead of removing just one. 
-    // This will lead to the request's destruction and eventually a crash.
-    // The only solution is therefore to ensure we are taking an extra reference to the request, and that the request is added after the queue is flushed.
-    Async(EAsyncExecution::TaskGraph, [AddRef = FHttpRequestPtr(GetHttpRequest())](){ AddRef->ProcessRequest(); });
-}
-
 }