From 78acc1080025d7a5b7fa56a0a8346b914a0c96a2 Mon Sep 17 00:00:00 2001 From: Vladyslav Tsymbal Date: Mon, 6 Jul 2026 15:07:19 +0300 Subject: [PATCH] Add Delegate interface for OlpClient and implement mock for testing Added a delegate interface for OlpClient to allow for better testing and mocking of the client behavior. Implemented a mock version of the delegate to facilitate unit tests. Solves the issue of testing OlpClient without relying on actual network calls. Relates-To: HERESDK-12772 Signed-off-by: Vladyslav Tsymbal --- .../include/olp/core/client/OlpClient.h | 67 +++++++++- olp-cpp-sdk-core/src/client/OlpClient.cpp | 119 ++++++++++++------ .../tests/client/OlpClientTest.cpp | 109 ++++++++++++++++ 3 files changed, 253 insertions(+), 42 deletions(-) diff --git a/olp-cpp-sdk-core/include/olp/core/client/OlpClient.h b/olp-cpp-sdk-core/include/olp/core/client/OlpClient.h index a472d1798..b391689f8 100644 --- a/olp-cpp-sdk-core/include/olp/core/client/OlpClient.h +++ b/olp-cpp-sdk-core/include/olp/core/client/OlpClient.h @@ -45,6 +45,56 @@ class CORE_API OlpClient { using RequestBodyType = std::shared_ptr>; OlpClient(); + /** + * @brief Abstract delegate interface for the `OlpClient` implementation. + * + * Implement this interface to inject a custom (e.g. test/mock) + * implementation into `OlpClient` via the + * `OlpClient(std::shared_ptr)` constructor. The production + * implementation is created automatically by the other constructors. + */ + class CORE_API Delegate { + public: + virtual ~Delegate() = default; + + /// @copydoc OlpClient::SetBaseUrl + virtual void SetBaseUrl(const std::string& base_url) = 0; + /// @copydoc OlpClient::GetBaseUrl + virtual std::string GetBaseUrl() const = 0; + /// @copydoc OlpClient::GetMutableDefaultHeaders + virtual ParametersType& GetMutableDefaultHeaders() = 0; + /// @copydoc OlpClient::GetSettings + virtual const OlpClientSettings& GetSettings() const = 0; + + /// @copydoc OlpClient::CallApi(const std::string&, const std::string&, + /// const ParametersType&, const ParametersType&, + /// const ParametersType&, const RequestBodyType&, + /// const std::string&, const NetworkAsyncCallback&) + virtual CancellationToken CallApi( + const std::string& path, const std::string& method, + const ParametersType& query_params, const ParametersType& header_params, + const ParametersType& form_params, const RequestBodyType& post_body, + const std::string& content_type, + const NetworkAsyncCallback& callback) const = 0; + + /// @copydoc OlpClient::CallApi(std::string, std::string, ParametersType, + /// ParametersType, ParametersType, RequestBodyType, std::string, + /// CancellationContext) + virtual HttpResponse CallApi(std::string path, std::string method, + ParametersType query_params, + ParametersType header_params, + ParametersType form_params, + RequestBodyType post_body, + std::string content_type, + CancellationContext context) const = 0; + + /// @copydoc OlpClient::CallApiStream + virtual HttpResponse CallApiStream( + std::string path, std::string method, ParametersType query_params, + ParametersType header_params, http::Network::DataCallback data_callback, + RequestBodyType post_body, std::string content_type, + CancellationContext context) const = 0; + }; /** * @brief Creates the `OlpClient` instance. @@ -53,7 +103,19 @@ class CORE_API OlpClient { * @param base_url The base URL to be used for all outgoing requests. */ OlpClient(const OlpClientSettings& settings, std::string base_url); - virtual ~OlpClient(); + + /** + * @brief Creates the `OlpClient` instance with a custom delegate. + * + * Use this constructor to inject a test or mock implementation. + * The supplied @p delegate must not be null. + * + * @param delegate A non-null shared pointer to a custom `Delegate` + * implementation. + */ + explicit OlpClient(std::shared_ptr delegate); + + ~OlpClient(); /// A copy constructor. OlpClient(const OlpClient&); @@ -180,8 +242,7 @@ class CORE_API OlpClient { CancellationContext context) const; private: - class OlpClientImpl; - std::shared_ptr impl_; + std::shared_ptr impl_; }; } // namespace client diff --git a/olp-cpp-sdk-core/src/client/OlpClient.cpp b/olp-cpp-sdk-core/src/client/OlpClient.cpp index 6375bb158..68811dfba 100644 --- a/olp-cpp-sdk-core/src/client/OlpClient.cpp +++ b/olp-cpp-sdk-core/src/client/OlpClient.cpp @@ -456,32 +456,41 @@ bool IsPending(const PendingUrlRequestPtr& request) { } // namespace -class OlpClient::OlpClientImpl { +class OlpClientImpl : public OlpClient::Delegate { public: OlpClientImpl(); + using ParametersType = OlpClient::ParametersType; + using RequestBodyType = OlpClient::RequestBodyType; + OlpClientImpl(const OlpClientSettings& settings, std::string base_url); - ~OlpClientImpl() = default; + ~OlpClientImpl() override = default; - void SetBaseUrl(const std::string& base_url); - std::string GetBaseUrl() const; + void SetBaseUrl(const std::string& base_url) override; + std::string GetBaseUrl() const override; - ParametersType& GetMutableDefaultHeaders(); - const OlpClientSettings& GetSettings() const { return settings_; } + ParametersType& GetMutableDefaultHeaders() override; + const OlpClientSettings& GetSettings() const override { return settings_; } - CancellationToken CallApi(const std::string& path, const std::string& method, - const ParametersType& query_params, - const ParametersType& header_params, - const ParametersType& form_params, - const RequestBodyType& post_body, - const std::string& content_type, - const NetworkAsyncCallback& callback) const; + CancellationToken CallApi( + const std::string& path, const std::string& method, + const ParametersType& query_params, const ParametersType& header_params, + const ParametersType& form_params, const RequestBodyType& post_body, + const std::string& content_type, + const NetworkAsyncCallback& callback) const override; HttpResponse CallApi(std::string path, std::string method, ParametersType query_params, - ParametersType header_params, - http::Network::DataCallback data_callback, + ParametersType header_params, ParametersType form_params, RequestBodyType post_body, std::string content_type, - CancellationContext context) const; + CancellationContext context) const override; + + HttpResponse CallApiStream(std::string path, std::string method, + ParametersType query_params, + ParametersType header_params, + http::Network::DataCallback data_callback, + RequestBodyType post_body, + std::string content_type, + CancellationContext context) const override; std::shared_ptr CreateRequest( const std::string& path, const std::string& method, @@ -489,8 +498,15 @@ class OlpClient::OlpClientImpl { const RequestBodyType& post_body, const std::string& content_type) const; porting::optional AddBearer(bool query_empty, - http::NetworkRequest& request, - CancellationContext& context) const; + http::NetworkRequest& request, + CancellationContext& context) const; + + HttpResponse CallApiImpl(std::string path, std::string method, + ParametersType query_params, + ParametersType header_params, + http::Network::DataCallback data_callback, + RequestBodyType post_body, std::string content_type, + CancellationContext context) const; private: using MutexType = std::shared_mutex; @@ -504,29 +520,26 @@ class OlpClient::OlpClientImpl { bool ValidateBaseUrl() const; }; -OlpClient::OlpClientImpl::OlpClientImpl() +OlpClientImpl::OlpClientImpl() : pending_requests_{std::make_shared()} {} -OlpClient::OlpClientImpl::OlpClientImpl(const OlpClientSettings& settings, - std::string base_url) +OlpClientImpl::OlpClientImpl(const OlpClientSettings& settings, + std::string base_url) : base_url_{std::move(base_url)}, settings_{settings}, pending_requests_{std::make_shared()} {} -void OlpClient::OlpClientImpl::SetBaseUrl(const std::string& base_url) { +void OlpClientImpl::SetBaseUrl(const std::string& base_url) { base_url_.lockedAssign(base_url); } -std::string OlpClient::OlpClientImpl::GetBaseUrl() const { - return base_url_.lockedCopy(); -} +std::string OlpClientImpl::GetBaseUrl() const { return base_url_.lockedCopy(); } -OlpClient::ParametersType& -OlpClient::OlpClientImpl::GetMutableDefaultHeaders() { +OlpClient::ParametersType& OlpClientImpl::GetMutableDefaultHeaders() { return default_headers_; } -porting::optional OlpClient::OlpClientImpl::AddBearer( +porting::optional OlpClientImpl::AddBearer( bool query_empty, http::NetworkRequest& request, CancellationContext& context) const { const auto& settings = settings_.authentication_settings; @@ -566,14 +579,14 @@ porting::optional OlpClient::OlpClientImpl::AddBearer( return porting::none; } -bool OlpClient::OlpClientImpl::ValidateBaseUrl() const { +bool OlpClientImpl::ValidateBaseUrl() const { return base_url_.locked([](const std::string& base_url) { return base_url == "" || (base_url.find(kHttpPrefix) != std::string::npos || base_url.find(kHttpsPrefix) != std::string::npos); }); } -std::shared_ptr OlpClient::OlpClientImpl::CreateRequest( +std::shared_ptr OlpClientImpl::CreateRequest( const std::string& path, const std::string& method, const OlpClient::ParametersType& query_params, const OlpClient::ParametersType& header_params, @@ -612,7 +625,7 @@ std::shared_ptr OlpClient::OlpClientImpl::CreateRequest( return network_request; } -CancellationToken OlpClient::OlpClientImpl::CallApi( +CancellationToken OlpClientImpl::CallApi( const std::string& path, const std::string& method, const OlpClient::ParametersType& query_params, const OlpClient::ParametersType& header_params, @@ -706,7 +719,33 @@ CancellationToken OlpClient::OlpClientImpl::CallApi( return cancellation_token; } -HttpResponse OlpClient::OlpClientImpl::CallApi( +HttpResponse OlpClientImpl::CallApi(std::string path, std::string method, + OlpClient::ParametersType query_params, + OlpClient::ParametersType header_params, + OlpClient::ParametersType /*form_params*/, + OlpClient::RequestBodyType post_body, + std::string content_type, + CancellationContext context) const { + return CallApiImpl(std::move(path), std::move(method), + std::move(query_params), std::move(header_params), nullptr, + std::move(post_body), std::move(content_type), + std::move(context)); +} + +HttpResponse OlpClientImpl::CallApiStream( + std::string path, std::string method, + OlpClient::ParametersType query_params, + OlpClient::ParametersType header_params, + http::Network::DataCallback data_callback, + OlpClient::RequestBodyType post_body, std::string content_type, + CancellationContext context) const { + return CallApiImpl(std::move(path), std::move(method), + std::move(query_params), std::move(header_params), + std::move(data_callback), std::move(post_body), + std::move(content_type), std::move(context)); +} + +HttpResponse OlpClientImpl::CallApiImpl( std::string path, std::string method, OlpClient::ParametersType query_params, OlpClient::ParametersType header_params, @@ -831,6 +870,8 @@ HttpResponse OlpClient::OlpClientImpl::CallApi( OlpClient::OlpClient() : impl_(std::make_shared()) {} OlpClient::OlpClient(const OlpClientSettings& settings, std::string base_url) : impl_(std::make_shared(settings, std::move(base_url))) {} +OlpClient::OlpClient(std::shared_ptr delegate) + : impl_(std::move(delegate)) {} OlpClient::~OlpClient() = default; OlpClient::OlpClient(const OlpClient&) = default; OlpClient& OlpClient::operator=(const OlpClient&) = default; @@ -864,14 +905,14 @@ CancellationToken OlpClient::CallApi( HttpResponse OlpClient::CallApi(std::string path, std::string method, ParametersType query_params, ParametersType header_params, - ParametersType /*form_params*/, + ParametersType form_params, RequestBodyType post_body, std::string content_type, CancellationContext context) const { return impl_->CallApi(std::move(path), std::move(method), std::move(query_params), std::move(header_params), - nullptr, std::move(post_body), std::move(content_type), - std::move(context)); + std::move(form_params), std::move(post_body), + std::move(content_type), std::move(context)); } HttpResponse OlpClient::CallApiStream(std::string path, std::string method, @@ -881,10 +922,10 @@ HttpResponse OlpClient::CallApiStream(std::string path, std::string method, RequestBodyType post_body, std::string content_type, CancellationContext context) const { - return impl_->CallApi(std::move(path), std::move(method), - std::move(query_params), std::move(header_params), - std::move(data_callback), std::move(post_body), - std::move(content_type), std::move(context)); + return impl_->CallApiStream(std::move(path), std::move(method), + std::move(query_params), std::move(header_params), + std::move(data_callback), std::move(post_body), + std::move(content_type), std::move(context)); } } // namespace client diff --git a/olp-cpp-sdk-core/tests/client/OlpClientTest.cpp b/olp-cpp-sdk-core/tests/client/OlpClientTest.cpp index 2f557af10..fe70fb38b 100644 --- a/olp-cpp-sdk-core/tests/client/OlpClientTest.cpp +++ b/olp-cpp-sdk-core/tests/client/OlpClientTest.cpp @@ -2179,3 +2179,112 @@ TEST_P(OlpClientTest, UrlWithoutProtocol) { } } // namespace + +// ---- Delegate injection tests ----------------------------------------------- + +namespace { + +/// A minimal mock of OlpClient::Delegate for testing the injection path. +class MockDelegate : public olp::client::OlpClient::Delegate { + public: + using ParametersType = olp::client::OlpClient::ParametersType; + using RequestBodyType = olp::client::OlpClient::RequestBodyType; + + void SetBaseUrl(const std::string& url) override { base_url_ = url; } + std::string GetBaseUrl() const override { return base_url_; } + ParametersType& GetMutableDefaultHeaders() override { return headers_; } + const olp::client::OlpClientSettings& GetSettings() const override { + return settings_; + } + + MOCK_METHOD(olp::client::CancellationToken, CallApi, + (const std::string& path, const std::string& method, + const ParametersType& query_params, + const ParametersType& header_params, + const ParametersType& form_params, const RequestBodyType& body, + const std::string& content_type, + const olp::client::NetworkAsyncCallback& callback), + (const, override)); + + MOCK_METHOD(olp::client::HttpResponse, CallApi, + (std::string path, std::string method, ParametersType query, + ParametersType headers, ParametersType form, + RequestBodyType body, std::string content, + olp::client::CancellationContext ctx), + (const, override)); + + MOCK_METHOD(olp::client::HttpResponse, CallApiStream, + (std::string path, std::string method, ParametersType query, + ParametersType headers, olp::http::Network::DataCallback cb, + RequestBodyType body, std::string content, + olp::client::CancellationContext ctx), + (const, override)); + + private: + std::string base_url_; + ParametersType headers_; + olp::client::OlpClientSettings settings_; +}; + +/// Helper: a consumer function that accepts OlpClient by value and calls +/// the sync overload. This validates that no slicing or vtable issues arise. +static olp::client::HttpResponse ConsumeByValue(olp::client::OlpClient client, + const std::string& path) { + return client.CallApi(path, "GET", {}, {}, {}, nullptr, {}, + olp::client::CancellationContext{}); +} + +} // namespace + +class OlpClientDelegateTest : public ::testing::Test {}; + +/// Verify that injecting a custom Delegate reaches the mock. +TEST_F(OlpClientDelegateTest, CustomDelegateIsInvoked) { + auto delegate = std::make_shared(); + + const olp::client::HttpResponse kFakeResponse{200, "hello"}; + EXPECT_CALL(*delegate, + CallApi(::testing::_, ::testing::_, ::testing::_, ::testing::_, + ::testing::_, ::testing::_, ::testing::_, ::testing::_)) + .WillOnce(::testing::Return(kFakeResponse)); + + olp::client::OlpClient client(delegate); + auto response = client.CallApi("/test", "GET", {}, {}, {}, nullptr, {}, + olp::client::CancellationContext{}); + + EXPECT_EQ(response.GetStatus(), kFakeResponse.GetStatus()); +} + +/// Verify that the default (production) implementation is still used when +/// no custom delegate is injected, and the existing behaviour is preserved. +TEST_F(OlpClientDelegateTest, DefaultImplStillWorks) { + // Build a client with no network handler — the default impl should return + // OFFLINE_ERROR, exactly as before the refactoring. + olp::client::OlpClientSettings settings; + settings.network_request_handler = nullptr; + olp::client::OlpClient client(settings, "https://example.com"); + + auto response = client.CallApi("/ping", "GET", {}, {}, {}, nullptr, {}, + olp::client::CancellationContext{}); + EXPECT_EQ(response.GetStatus(), + static_cast(olp::http::ErrorCode::OFFLINE_ERROR)); +} + +/// Verify that a consumer that accepts OlpClient by value still routes calls +/// through the injected delegate, demonstrating no need for a separate +/// consumer-side interface. +TEST_F(OlpClientDelegateTest, PassByValueUsesInjectedDelegate) { + auto delegate = std::make_shared(); + + const olp::client::HttpResponse kFakeResponse{201, "created"}; + EXPECT_CALL(*delegate, + CallApi(::testing::_, ::testing::_, ::testing::_, ::testing::_, + ::testing::_, ::testing::_, ::testing::_, ::testing::_)) + .WillOnce(::testing::Return(kFakeResponse)); + + olp::client::OlpClient client(delegate); + // Pass by value — shared_ptr delegate is shared, so the mock is still live. + auto response = ConsumeByValue(client, "/resource"); + + EXPECT_EQ(response.GetStatus(), kFakeResponse.GetStatus()); +}