Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,6 @@ _deps/

# Docker artifacts
*.tar

# Testing credentials file
testing_credentials.json
12 changes: 12 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ target_link_libraries(dbps_server_lib PUBLIC OpenSSL::SSL OpenSSL::Crypto)
# Client components library (depends on httplib, nlohmann/json, and cppcodec)
add_library(dbps_client_lib STATIC
src/client/dbps_api_client.cpp
src/client/http_client_base.cpp
src/client/httplib_client.cpp
src/client/httplib_pool_registry.cpp
src/client/httplib_pooled_client.cpp
Expand Down Expand Up @@ -364,6 +365,14 @@ if(BUILD_TESTS)
gtest_main
)

# Http client interface tests
add_executable(http_client_base_test src/client/http_client_base_test.cpp)
target_link_libraries(http_client_base_test
dbps_client_lib
dbps_common_lib
gtest_main
)

# Remote DBPA tests
add_executable(dbpa_remote_test src/common/dbpa_remote_test.cpp)
target_link_libraries(dbpa_remote_test
Expand Down Expand Up @@ -393,6 +402,7 @@ if(BUILD_SHARED_LIBS)
src/common/dbps_remote_shared_lib_wrapper.cpp
src/common/dbpa_remote.cpp
src/client/dbps_api_client.cpp
src/client/http_client_base.cpp
src/client/httplib_client.cpp
src/common/json_request.cpp
)
Expand Down Expand Up @@ -496,6 +506,7 @@ if(BUILD_TESTS)
dbpa_local_test
httplib_pool_registry_test
httplib_pooled_client_test
http_client_base_test
COMMENT "Building all tests"
)

Expand All @@ -517,6 +528,7 @@ if(BUILD_TESTS)
gtest_discover_tests(dbpa_local_test)
gtest_discover_tests(httplib_pool_registry_test)
gtest_discover_tests(httplib_pooled_client_test)
gtest_discover_tests(http_client_base_test)
endif()

# All target (everything)
Expand Down
4 changes: 2 additions & 2 deletions src/client/dbps_api_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,12 @@ void DecryptApiResponse::SetJsonRequest(const DecryptJsonRequest& request) { jso
bool DecryptApiResponse::HasJsonRequest() const { return json_request_.has_value(); }
const JsonRequest& DecryptApiResponse::GetJsonRequest() const { return json_request_.value(); }

DBPSApiClient::DBPSApiClient(std::shared_ptr<HttpClientInterface> http_client)
DBPSApiClient::DBPSApiClient(std::shared_ptr<HttpClientBase> http_client)
: http_client_(std::move(http_client)) {
}

std::string DBPSApiClient::HealthCheck() {
auto response = http_client_->Get("/healthz");
auto response = http_client_->Get("/healthz", false);

if (!response.error_message.empty()) {
return "Error: " + response.error_message;
Expand Down
6 changes: 3 additions & 3 deletions src/client/dbps_api_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include "../common/enum_utils.h"
#include "../common/json_request.h"
#include "tcb/span.hpp"
#include "http_client_interface.h"
#include "http_client_base.h"

using namespace dbps::external;
using namespace dbps::enum_utils;
Expand Down Expand Up @@ -143,7 +143,7 @@ class DBPSApiClient {
* The HTTP client is expected to be thread-safe.
* @param http_client Custom HTTP client implementation
*/
explicit DBPSApiClient(std::shared_ptr<HttpClientInterface> http_client);
explicit DBPSApiClient(std::shared_ptr<HttpClientBase> http_client);

/**
* Destructor
Expand Down Expand Up @@ -223,5 +223,5 @@ class DBPSApiClient {
);

private:
const std::shared_ptr<HttpClientInterface> http_client_;
const std::shared_ptr<HttpClientBase> http_client_;
};
31 changes: 21 additions & 10 deletions src/client/dbps_api_client_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
#include <algorithm>
#include "tcb/span.hpp"
#include "dbps_api_client.h"
#include "http_client_interface.h"
#include "http_client_base.h"
#include "../common/enums.h"
#include <nlohmann/json.hpp>
#include <gtest/gtest.h>
Expand Down Expand Up @@ -56,8 +56,14 @@ bool CompareJsonStrings(const std::string& json1, const std::string& json2, cons
}

// Mock HTTP client for testing
class MockHttpClient : public HttpClientInterface {
class MockHttpClient : public HttpClientBase {
public:
MockHttpClient()
: HttpClientBase(
"mock://",
ClientCredentials{{"client_id", "test_client_AAAA"}, {"api_key", "test_key_AAAA"}}) {
}

// Mock responses for different endpoints
void SetMockResponse(const std::string& endpoint, const HttpResponse& response) {
mock_responses_[endpoint] = response;
Expand All @@ -67,16 +73,21 @@ class MockHttpClient : public HttpClientInterface {
mock_post_responses_[endpoint] = {expected_body, response};
}

// Implement the interface methods
HttpResponse Get(const std::string& endpoint) override {
protected:
HttpResponse DoGet(const std::string& endpoint, const HeaderList& headers) override {
(void)headers;
auto it = mock_responses_.find(endpoint);
if (it != mock_responses_.end()) {
return it->second;
}
return HttpResponse(404, "", "Mock endpoint not found: " + endpoint);
}

HttpResponse Post(const std::string& endpoint, const std::string& json_body) override {
HttpResponse DoPost(const std::string& endpoint, const std::string& json_body, const HeaderList& headers) override {
(void)headers;
if (endpoint == "/token") {
return HttpResponse(200, R"({"token":"mock_jwt","token_type":"Bearer","expires_at":1766138275})");
}
auto it = mock_post_responses_.find(endpoint);
if (it != mock_post_responses_.end()) {
if (CompareJsonStrings(it->second.first, json_body, {"debug"})) {
Expand Down Expand Up @@ -299,7 +310,7 @@ TEST(DBPSApiClient, EncryptWithValidData) {
})";

mock_client->SetMockPostResponse("/encrypt", expected_request,
HttpClientInterface::HttpResponse(200, mock_response));
HttpClientBase::HttpResponse(200, mock_response));

// Create DBPSApiClient with mock client
DBPSApiClient client(std::move(mock_client));
Expand Down Expand Up @@ -391,7 +402,7 @@ TEST(DBPSApiClient, DecryptWithValidData) {
})";

mock_client->SetMockPostResponse("/decrypt", expected_request,
HttpClientInterface::HttpResponse(200, mock_response));
HttpClientBase::HttpResponse(200, mock_response));

// Create DBPSApiClient with mock client
DBPSApiClient client(std::move(mock_client));
Expand Down Expand Up @@ -534,7 +545,7 @@ TEST(DBPSApiClient, EncryptWithInvalidJsonResponse) {
})";

mock_client->SetMockPostResponse("/encrypt", expected_request,
HttpClientInterface::HttpResponse(200, mock_response));
HttpClientBase::HttpResponse(200, mock_response));

// Create DBPSApiClient with mock client
DBPSApiClient client(std::move(mock_client));
Expand Down Expand Up @@ -600,7 +611,7 @@ TEST(DBPSApiClient, DecryptWithInvalidJsonResponse) {
})";

mock_client->SetMockPostResponse("/decrypt", expected_request,
HttpClientInterface::HttpResponse(200, mock_response));
HttpClientBase::HttpResponse(200, mock_response));

// Create DBPSApiClient with mock client
DBPSApiClient client(std::move(mock_client));
Expand Down Expand Up @@ -677,7 +688,7 @@ TEST(DBPSApiClient, EncryptWithEncodingAttributes) {
})";

mock_client->SetMockPostResponse("/encrypt", expected_request,
HttpClientInterface::HttpResponse(200, mock_response));
HttpClientBase::HttpResponse(200, mock_response));

// Create DBPSApiClient with mock client
DBPSApiClient client(std::move(mock_client));
Expand Down
193 changes: 193 additions & 0 deletions src/client/http_client_base.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

#include "http_client_base.h"

#include <chrono>

#include "json_request.h"

HttpClientBase::HeaderList HttpClientBase::DefaultJsonGetHeaders() {
HeaderList headers;
headers.insert({"Accept", kJsonContentType});
Comment thread
argmarco-tkd marked this conversation as resolved.
headers.insert({"User-Agent", kDefaultUserAgent});
return headers;
}

HttpClientBase::HeaderList HttpClientBase::DefaultJsonPostHeaders() {
HeaderList headers;
headers.insert({"Content-Type", kJsonContentType});
headers.insert({"Accept", kJsonContentType});
headers.insert({"User-Agent", kDefaultUserAgent});
return headers;
}

HttpClientBase::HttpResponse HttpClientBase::Get(const std::string& endpoint, bool auth_required) {
Comment thread
argmarco-tkd marked this conversation as resolved.
// Lambda to build the request and make the actual call.
const auto attempt = [&]() -> HttpResponse {
auto headers = DefaultJsonGetHeaders();
if (auth_required) {
auto auth_error = AddAuthorizationHeader(headers);
if (!auth_error.empty()) {
return HttpResponse(0, "", auth_error);
}
}
return DoGet(endpoint, headers);
};

// First attempt
auto result = attempt();

// If we got 401 Unauthorized and auth was required, invalidate token and retry once
// This handles cases where the cached token expired between validation and use
if (auth_required && result.status_code == 401) {
InvalidateCachedToken();
result = attempt(); // Second (final) attempt with fresh token
}
return result;
}

HttpClientBase::HttpResponse HttpClientBase::Post(const std::string& endpoint,
Comment thread
argmarco-tkd marked this conversation as resolved.
const std::string& json_body,
bool auth_required) {
// Lambda to build the request and make the actual call.
const auto attempt = [&]() -> HttpResponse {
auto headers = DefaultJsonPostHeaders();
if (auth_required) {
auto auth_error = AddAuthorizationHeader(headers);
if (!auth_error.empty()) {
return HttpResponse(0, "", auth_error);
}
}
return DoPost(endpoint, json_body, headers);
};

// First attempt
auto result = attempt();

// If we got 401 Unauthorized and auth was required, invalidate token and retry once
// This handles cases where the cached token expired between validation and use
if (auth_required && result.status_code == 401) {
InvalidateCachedToken();
result = attempt(); // Second (final) attempt with fresh token
}
return result;
}

std::string HttpClientBase::AddAuthorizationHeader(HeaderList& headers) {
// Get the valid token or an error message.
std::string token_or_error;
auto token_opt = EnsureValidToken(token_or_error);
if (!token_opt.has_value()) {
return token_or_error;
}

// Replace any existing Authorization header with: "<token_type> <token>".
// Return an empty string if successful, otherwise return the error message.
headers.erase(kAuthorizationHeader);
std::string auth_value = token_opt->token_type;
if (auth_value.back() != ' ') {
auth_value.push_back(' ');
}
auth_value += token_opt->token;
headers.insert({kAuthorizationHeader, auth_value});
Comment thread
argmarco-tkd marked this conversation as resolved.
return "";
}

std::optional<HttpClientBase::TokenWithExpiration> HttpClientBase::EnsureValidToken(std::string& error) {

const auto now_epoch_seconds = []() -> std::int64_t {
const auto now = std::chrono::system_clock::now();
const auto secs = std::chrono::duration_cast<std::chrono::seconds>(now.time_since_epoch()).count();
return static_cast<std::int64_t>(secs);
};

// Adds padding to the expiration time to "expire" the token early and prevent going too close to the expiration time.
const auto is_token_valid_at = [&](std::int64_t now) -> bool {
return cached_token_.has_value() &&
!cached_token_->token.empty() &&
cached_token_->expires_at > (now + kTokenExpirySkewSeconds);
Comment thread
argmarco-tkd marked this conversation as resolved.
};

error.clear();
const auto now = now_epoch_seconds();

{
std::unique_lock<std::mutex> lock(token_mutex_);
if (is_token_valid_at(now)) {
return cached_token_;
}
while (token_fetch_in_progress_) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we need to set token_fetch_in_progress_ = true before this line.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, no. It's correct as it is. The while (token_fetch_in_progress_) forces other threads to continue on that loop until the token fetch finishes. The flip to token_fetch_in_progress_=False happens a few lines below.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it. gave it another pass at the function, it makes sense. thanks.

token_cv_.wait(lock);
if (is_token_valid_at(now_epoch_seconds())) {
return cached_token_;
}
}
token_fetch_in_progress_ = true;
} // Release token_mutex_ here

// Fetch token without holding token_mutex_
// - avoids blocking other threads from entering EnsureValidToken() and waiting on token_cv_
// - keeps the critical section small (network I/O can be slow)
// We re-acquire token_mutex_ below to update cached_token_, clear token_fetch_in_progress_, and notify waiters.

std::optional<TokenWithExpiration> fetched = FetchToken(error);

{
std::lock_guard<std::mutex> lock(token_mutex_);
token_fetch_in_progress_ = false;
if (fetched.has_value()) {
cached_token_ = fetched;
}
}
token_cv_.notify_all();
return fetched;
}

std::optional<HttpClientBase::TokenWithExpiration> HttpClientBase::FetchToken(std::string& error) {
error.clear();
TokenRequest token_req;
token_req.credential_values_ = credentials_;

// IMPORTANT: call DoPost directly (authless) to avoid recursion.
auto http_resp = DoPost("/token", token_req.ToJson(), DefaultJsonPostHeaders());
if (!http_resp.error_message.empty() || http_resp.status_code != 200) {
error = http_resp.error_message + " (status code: " + std::to_string(http_resp.status_code) + ")";
return std::nullopt;
}

TokenResponse token_resp;
token_resp.Parse(http_resp.result);
if (!token_resp.IsValid()) {
error = token_resp.GetValidationError();
if (error.empty()) {
error = "While reading token response, found an invalid token response: " + http_resp.result;
}
return std::nullopt;
}

TokenWithExpiration result;
result.token = token_resp.token_.value();
result.token_type = token_resp.token_type_.value();
result.expires_at = token_resp.expires_at_.value();
return result;
}

void HttpClientBase::InvalidateCachedToken() {
std::lock_guard<std::mutex> lock(token_mutex_);
cached_token_ = std::nullopt;
}
Loading