From 075f5ce6931838971706ac8ace940ee457129db7 Mon Sep 17 00:00:00 2001 From: Junbo Liang <68558268+junbo75@users.noreply.github.com> Date: Mon, 24 Jan 2022 14:20:33 -0800 Subject: [PATCH] Reimplement the GCC fix for the AWS jobs using forward declare (#7088) * Revert the recent AWSApiRequestJob and ServiceRequestJob and use forward declaring to fix the GCC issue Signed-off-by: Junbo Liang <68558268+junbo75@users.noreply.github.com> --- .../Code/Include/Framework/AWSApiRequestJob.h | 149 ++++++++-------- .../Include/Framework/ServiceRequestJob.h | 161 +++++++++--------- 2 files changed, 162 insertions(+), 148 deletions(-) diff --git a/Gems/AWSCore/Code/Include/Framework/AWSApiRequestJob.h b/Gems/AWSCore/Code/Include/Framework/AWSApiRequestJob.h index 816e9bc4b4..e70aa37f58 100644 --- a/Gems/AWSCore/Code/Include/Framework/AWSApiRequestJob.h +++ b/Gems/AWSCore/Code/Include/Framework/AWSApiRequestJob.h @@ -177,6 +177,11 @@ namespace AWSCore using OnSuccessFunction = AZStd::function; using OnFailureFunction = AZStd::function; + class Function; + + template + static AwsApiRequestJob* Create(OnSuccessFunction onSuccess, OnFailureFunction onFailure = OnFailureFunction{}, IConfig* config = GetDefaultConfig()); + static Config* GetDefaultConfig() { static AwsApiJobConfigHolder s_configHolder{}; @@ -188,10 +193,6 @@ namespace AWSCore { } - RequestType request; - ResultType result; - ErrorType error; - /// Override AZ:Job defined method to reset request state when /// the job object is reused. void Reset(bool isClearDependent) override @@ -209,35 +210,11 @@ namespace AWSCore return m_wasSuccess; } + RequestType request; + ResultType result; + ErrorType error; + protected: - - /// Constructor for creating AwsApiRequestJob Jobs that can handle queued responses - /// for OnSuccess, OnFailure, and DoCleanup - AwsApiRequestJob(OnSuccessFunction onSuccess, - OnFailureFunction onFailure, - IConfig* config = GetDefaultConfig() - ) : AwsApiClientJobType(false, config) - , m_queueOnSuccess{ true } - , m_onSuccess{ onSuccess } - , m_queueOnFailure{ true } - , m_onFailure{ onFailure } - , m_queueDelete{ true } - { - } - - bool m_wasSuccess{ false }; - - // Flag and optional function call to queue for onSuccess events - bool m_queueOnSuccess{ false }; - OnSuccessFunction m_onSuccess{}; - - // Flag and optional function call to queue for onFailure events - bool m_queueOnFailure{ false }; - OnFailureFunction m_onFailure{}; - - // Flag to queue the delete during the DoCleanup calls - bool m_queueDelete{ false }; - void Process() override { @@ -295,56 +272,86 @@ namespace AWSCore /// Called when request has completed successfully. virtual void OnSuccess() { - if (m_queueOnSuccess) - { - AZStd::function callbackHandler = [this]() - { - if (m_onSuccess) - { - m_onSuccess(this); - } - delete this; - }; - AZ::TickBus::QueueFunction(callbackHandler); - } } /// Called when the request fails. virtual void OnFailure() { - if (m_queueOnFailure) - { - AZStd::function callbackHandler = [this]() - { - if (m_onFailure) - { - m_onFailure(this); - } - delete this; - }; - AZ::TickBus::QueueFunction(callbackHandler); - } } - /// Called when request can't process and still requires cleanup (Specifically for our derived class Function which does not use auto delete) + /// Called when request can't process and still requires cleanup (Specifically for the derived class AwsApiRequestJob::Function which does not use auto delete) virtual void DoCleanup() { - if (m_queueDelete) - { - AZStd::function callbackHandler = [this]() - { - delete this; - }; - AZ::TickBus::QueueFunction(callbackHandler); - } } - public: - template - static AwsApiRequestJob* Create(OnSuccessFunction onSuccess, OnFailureFunction onFailure = OnFailureFunction{}, IConfig* config = GetDefaultConfig()) - { - return azcreate(AwsApiRequestJob, (onSuccess, onFailure, config), Allocator); - } + bool m_wasSuccess{ false }; }; + /// A specialization of AwsApiRequestJob that lets you provide functions + /// that are called on success or failure of the request. + template + class AwsApiRequestJob::Function + : public AwsApiRequestJob + { + public: + // To use a different allocator, extend this class and use this macro. + AZ_CLASS_ALLOCATOR(Function, AZ::SystemAllocator, 0); + + Function(OnSuccessFunction onSuccess, OnFailureFunction onFailure = OnFailureFunction{}, IConfig* config = GetDefaultConfig()) + : AwsApiRequestJobType( + false, config) // No auto delete - we need to perform our callbacks on the main thread so we queue them through tickbus + , m_onSuccess{ onSuccess } + , m_onFailure{ onFailure } + { + } + + private: + void OnSuccess() override + { + AZStd::function callbackHandler = [this]() + { + if (m_onSuccess) + { + m_onSuccess(this); + } + delete this; + }; + AZ::TickBus::QueueFunction(callbackHandler); + } + + void OnFailure() override + { + AZStd::function callbackHandler = [this]() + { + if (m_onFailure) + { + m_onFailure(this); + } + delete this; + }; + AZ::TickBus::QueueFunction(callbackHandler); + } + + // Code doesn't use auto delete - this allows code to make sure things get cleaned up in cases where success or failure can't be + // called. + void DoCleanup() override + { + AZStd::function callbackHandler = [this]() + { + delete this; + }; + AZ::TickBus::QueueFunction(callbackHandler); + } + + OnSuccessFunction m_onSuccess; + OnFailureFunction m_onFailure; + }; + + template + template + AwsApiRequestJob* AwsApiRequestJob::Create( + OnSuccessFunction onSuccess, OnFailureFunction onFailure, IConfig* config) + { + return azcreate(Function, (onSuccess, onFailure, config), Allocator); + } } // namespace AWSCore diff --git a/Gems/AWSCore/Code/Include/Framework/ServiceRequestJob.h b/Gems/AWSCore/Code/Include/Framework/ServiceRequestJob.h index d3eac4983a..0c2429b9bb 100644 --- a/Gems/AWSCore/Code/Include/Framework/ServiceRequestJob.h +++ b/Gems/AWSCore/Code/Include/Framework/ServiceRequestJob.h @@ -150,6 +150,11 @@ namespace AWSCore using OnSuccessFunction = AZStd::function; using OnFailureFunction = AZStd::function; + class Function; + + template + static ServiceRequestJob* Create(OnSuccessFunction onSuccess, OnFailureFunction onFailure = OnFailureFunction{}, IConfig* config = GetDefaultConfig()); + static Config* GetDefaultConfig() { static AwsApiJobConfigHolder s_configHolder{}; @@ -212,45 +217,6 @@ namespace AWSCore } protected: - /// The URL created by appending the API path to the service URL. - /// The path may contain {param} format parameters. The - /// RequestType::parameters.BuildRequest method is responsible - /// for replacing these parts of the url. - const Aws::String& m_requestUrl; - - /// Constructor for creating ServiceRequestJob Jobs that can handle queued responses - /// for OnSuccess, OnFailure, and DoCleanup - ServiceRequestJob(OnSuccessFunction onSuccess, - OnFailureFunction onFailure, - IConfig* config = GetDefaultConfig() - ) : ServiceClientJobType{ false, config } - , m_requestUrl{ config->GetRequestUrl() } - , m_queueOnSuccess{ true } - , m_onSuccess{ onSuccess } - , m_queueOnFailure{ true } - , m_onFailure{ onFailure } - , m_queueDelete{ true } - { - } - - // Flag and optional function call to queue for onSuccess events - bool m_queueOnSuccess{ false }; - OnSuccessFunction m_onSuccess{}; - - // Flag and optional function call to queue for onFailure events - bool m_queueOnFailure{ false }; - OnFailureFunction m_onFailure{}; - - // Flag to queue the delete during the DoCleanup calls - bool m_queueDelete{ false }; - - std::shared_ptr m_AWSAuthSigner{ nullptr }; - - // Passed in configuration contains the AWS Credentials to use. If this request requires credentials - // check in the constructor and set this bool to indicate if we're not valid before placing the credentials - // in the m_AWSAuthSigner - bool m_missingCredentials{ false }; - /// Called to prepare the request. By default no changes /// are made to the parameters object. Override to defer the preparation /// of parameters until running on the job's worker thread, @@ -270,50 +236,31 @@ namespace AWSCore /// Called when a request completes without error. virtual void OnSuccess() { - if (m_queueOnSuccess) - { - AZStd::function callbackHandler = [this]() - { - if (m_onSuccess) - { - m_onSuccess(this); - } - delete this; - }; - AZ::TickBus::QueueFunction(callbackHandler); - } } /// Called when an error occurs. virtual void OnFailure() { - if (m_queueOnFailure) - { - AZStd::function callbackHandler = [this]() - { - if (m_onFailure) - { - m_onFailure(this); - } - delete this; - }; - AZ::TickBus::QueueFunction(callbackHandler); - } } /// Provided so derived functions that do not auto delete can clean up virtual void DoCleanup() { - if (m_queueDelete) - { - AZStd::function callbackHandler = [this]() - { - delete this; - }; - AZ::TickBus::QueueFunction(callbackHandler); - } } + /// The URL created by appending the API path to the service URL. + /// The path may contain {param} format parameters. The + /// RequestType::parameters.BuildRequest method is responsible + /// for replacing these parts of the url. + const Aws::String& m_requestUrl; + + std::shared_ptr m_AWSAuthSigner{ nullptr }; + + // Passed in configuration contains the AWS Credentials to use. If this request requires credentials + // check in the constructor and set this bool to indicate if we're not valid before placing the credentials + // in the m_AWSAuthSigner + bool m_missingCredentials{ false }; + private: bool BuildRequest(RequestBuilder& request) override { @@ -658,13 +605,73 @@ namespace AWSCore AZ_Printf(logRequestsChannel, "Response Body:\n"); PrintRequestOutput(responseContent); } - public: - template - static ServiceRequestJob* Create(OnSuccessFunction onSuccess, OnFailureFunction onFailure = OnFailureFunction{}, IConfig* config = GetDefaultConfig()) - { - return azcreate(ServiceRequestJob, (onSuccess, onFailure, config), Allocator); - } }; + + /// A derived class that calls lambda functions on job completion. + template + class ServiceRequestJob::Function + : public ServiceRequestJob + { + public: + // To use a different allocator, extend this class and use this macro. + AZ_CLASS_ALLOCATOR(Function, AZ::SystemAllocator, 0); + + Function(OnSuccessFunction onSuccess, OnFailureFunction onFailure = OnFailureFunction{}, IConfig* config = GetDefaultConfig()) + : ServiceRequestJob(false, config) // No auto delete - The Function class will handle it with the DoCleanup() function + , m_onSuccess{ onSuccess } + , m_onFailure{ onFailure } + { + } + + private: + void OnSuccess() override + { + AZStd::function callbackHandler = [this]() + { + if (m_onSuccess) + { + m_onSuccess(this); + } + delete this; + }; + AZ::TickBus::QueueFunction(callbackHandler); + } + + void OnFailure() override + { + AZStd::function callbackHandler = [this]() + { + if (m_onFailure) + { + m_onFailure(this); + } + delete this; + }; + AZ::TickBus::QueueFunction(callbackHandler); + } + + // Code doesn't use auto delete - this ensure things get cleaned up in cases when code can't call success or failure + void DoCleanup() override + { + AZStd::function callbackHandler = [this]() + { + delete this; + }; + AZ::TickBus::QueueFunction(callbackHandler); + } + + OnSuccessFunction m_onSuccess; + OnFailureFunction m_onFailure; + + }; + + template + template + ServiceRequestJob* ServiceRequestJob::Create( + OnSuccessFunction onSuccess, OnFailureFunction onFailure, IConfig* config) + { + return azcreate(Function, (onSuccess, onFailure, config), Allocator); + } } // namespace AWSCore