Skip to content
Draft
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
17 changes: 9 additions & 8 deletions libs/server-sdk/src/client_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
config::built::BackgroundSyncConfig const& cfg,
config::built::HttpProperties const& http_properties,
boost::asio::any_io_executor const& executor,
data_components::DataSourceStatusManager& status_manager,
std::shared_ptr<data_components::DataSourceStatusManager> status_manager,

Check warning on line 73 in libs/server-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/server-sdk/src/client_impl.cpp:73:63 [performance-unnecessary-value-param]

the parameter 'status_manager' is copied for each invocation but only used as a const reference; consider making it a const reference
Logger& logger) {
return std::make_unique<data_systems::BackgroundSync>(
endpoints, cfg, http_properties, executor, status_manager, logger);
Expand All @@ -94,7 +94,7 @@
std::vector<std::unique_ptr<data_interfaces::IFDv2InitializerFactory>>
initializer_factories;
for (auto const& initializer : cfg.initializers) {
initializer_factories.push_back(

Check warning on line 97 in libs/server-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/server-sdk/src/client_impl.cpp:97:9 [performance-inefficient-vector-operation]

'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop
std::make_unique<data_systems::FDv2PollingInitializerFactory>(
executor, logger, endpoints, http_properties, initializer));
}
Expand Down Expand Up @@ -161,10 +161,10 @@
config::built::HttpProperties const& http_properties,
Config const& config,
boost::asio::any_io_executor const& executor,
data_components::DataSourceStatusManager& status_manager,
std::shared_ptr<data_components::DataSourceStatusManager> status_manager,
Logger& logger) {
if (config.DataSystemConfig().disabled) {
return std::make_unique<data_systems::OfflineSystem>(status_manager);
return std::make_unique<data_systems::OfflineSystem>(*status_manager);
}

auto data_source_properties =
Expand All @@ -178,12 +178,12 @@
executor, status_manager, logger);
},
[&](config::built::LazyLoadConfig const& cfg) {
return MakeLazyLoadSystem(cfg, status_manager, logger);
return MakeLazyLoadSystem(cfg, *status_manager, logger);
},
[&](config::built::FDv2Config const& cfg) {
return MakeFDv2System(config.ServiceEndpoints(), cfg,
data_source_properties, executor,
status_manager, logger);
*status_manager, logger);
},
},
config.DataSystemConfig().system_);
Expand Down Expand Up @@ -220,7 +220,7 @@
bool IsFlagPresent(
std::shared_ptr<data_model::FlagDescriptor> const& flag_desc);

ClientImpl::ClientImpl(Config config, std::string const& version)

Check warning on line 223 in libs/server-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/server-sdk/src/client_impl.cpp:223:31 [performance-unnecessary-value-param]

the parameter 'config' is copied for each invocation but only used as a const reference; consider making it a const reference
: config_(config),
http_properties_(
config::builders::HttpPropertiesBuilder(config.HttpProperties())
Expand All @@ -238,7 +238,8 @@
logger_(MakeLogger(config.Logging())),
ioc_(kAsioConcurrencyHint),
work_(boost::asio::make_work_guard(ioc_)),
status_manager_(),
status_manager_(
std::make_shared<data_components::DataSourceStatusManager>()),
data_system_(MakeDataSystem(http_properties_,
config_,
ioc_.get_executor(),
Expand All @@ -251,7 +252,7 @@
big_segment_store_(
config_.BigSegments()
? std::make_shared<data_components::BigSegmentStoreWrapper>(
*config_.BigSegments(),

Check warning on line 255 in libs/server-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/server-sdk/src/client_impl.cpp:255:22 [bugprone-unchecked-optional-access]

unchecked access to optional value
ioc_.get_executor(),
logger_)
: nullptr),
Expand Down Expand Up @@ -287,10 +288,10 @@
}

std::future<bool> ClientImpl::StartAsync() {
auto pr = std::make_shared<std::promise<bool>>();

Check warning on line 291 in libs/server-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/server-sdk/src/client_impl.cpp:291:10 [readability-identifier-length]

variable name 'pr' is too short, expected at least 3 characters
auto fut = pr->get_future();

status_manager_.OnDataSourceStatusChangeEx([this, pr](auto _) {
status_manager_->OnDataSourceStatusChangeEx([this, pr](auto _) {

Check warning on line 294 in libs/server-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/server-sdk/src/client_impl.cpp:294:65 [readability-identifier-length]

parameter name '_' is too short, expected at least 3 characters
if (data_system_->Initialized()) {
pr->set_value(true);
return true; /* delete this change listener since the
Expand Down Expand Up @@ -328,14 +329,14 @@
// system to fetch them all at once up-front. This may be a no-op
// depending on the data system (e.g. if the segments are all already in
// memory.)
auto _ = data_system_->AllSegments();

Check warning on line 332 in libs/server-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/server-sdk/src/client_impl.cpp:332:10 [readability-identifier-length]

variable name '_' is too short, expected at least 3 characters

for (auto const& [key, v] : all_flags) {
if (!v || !v->item) {
continue;
}

auto const& flag = *(v->item);

Check warning on line 339 in libs/server-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/server-sdk/src/client_impl.cpp:339:29 [bugprone-unchecked-optional-access]

unchecked access to optional value

if (IsSet(options, AllFlagsState::Options::ClientSideOnly) &&
!flag.clientSideAvailability.usingEnvironmentId) {
Expand Down Expand Up @@ -531,9 +532,9 @@
}

EvaluationDetail<Value> result =
evaluator_.Evaluate(*flag_rule->item, context, event_scope);

Check warning on line 535 in libs/server-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/server-sdk/src/client_impl.cpp:535:30 [bugprone-unchecked-optional-access]

unchecked access to optional value
auto detail = PostEvaluation(key, context, default_value, result,
event_scope, flag_rule.get()->item);

Check warning on line 537 in libs/server-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/server-sdk/src/client_impl.cpp:537:47 [readability-redundant-smartptr-get]

redundant get() call on smart pointer

// Execute afterEvaluation hooks
if (executor) {
Expand Down Expand Up @@ -778,7 +779,7 @@
}

IDataSourceStatusProvider& ClientImpl::DataSourceStatus() {
return status_manager_;
return *status_manager_;
}

IBigSegmentStoreStatusProvider& ClientImpl::BigSegmentStoreStatus() {
Expand Down
2 changes: 1 addition & 1 deletion libs/server-sdk/src/client_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ class ClientImpl : public IClient {
boost::asio::executor_work_guard<boost::asio::io_context::executor_type>
work_;

data_components::DataSourceStatusManager status_manager_;
std::shared_ptr<data_components::DataSourceStatusManager> status_manager_;

// This is the main polymorphic component that constitutes the
// guts of how data is retrieved (polling, streaming, persistent stores,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <launchdarkly/data_model/sdk_data_set.hpp>

#include <functional>
#include <memory>
#include <optional>
#include <string>

Expand Down Expand Up @@ -30,12 +31,11 @@ class IDataSynchronizer {
* The data may be used to optimize the synchronization process, e.g. by
* obtaining a diff rather than a full dataset.
*
* @param destination The destination to synchronize data into. Pointer is
* invalid after the ShutdownAsync completion handler is called.
* @param destination The destination to synchronize data into.
* @param bootstrap_data Optional bootstrap data.
* Pointer is valid only for this call.
*/
virtual void StartAsync(IDestination* destination,
virtual void StartAsync(std::shared_ptr<IDestination> destination,
data_model::SDKDataSet const* bootstrap_data) = 0;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ BackgroundSync::BackgroundSync(
config::built::BackgroundSyncConfig const& background_sync_config,
config::built::HttpProperties http_properties,
boost::asio::any_io_executor ioc,
data_components::DataSourceStatusManager& status_manager,
std::shared_ptr<data_components::DataSourceStatusManager> status_manager,
Logger const& logger)
: store_(), change_notifier_(store_, store_), synchronizer_() {
: store_(),
change_notifier_(
std::make_shared<data_components::ChangeNotifier>(store_, store_)),
synchronizer_() {
std::visit(
[&](auto&& method_config) {
using T = std::decay_t<decltype(method_config)>;
Expand All @@ -34,7 +37,7 @@ BackgroundSync::BackgroundSync(
}

void BackgroundSync::Initialize() {
synchronizer_->StartAsync(&change_notifier_,
synchronizer_->StartAsync(change_notifier_,
nullptr /* no bootstrap data supported yet */);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class BackgroundSync final : public data_interfaces::IDataSystem {
config::built::BackgroundSyncConfig const& background_sync_config,
config::built::HttpProperties http_properties,
boost::asio::any_io_executor ioc,
data_components::DataSourceStatusManager& status_manager,
std::shared_ptr<data_components::DataSourceStatusManager>
status_manager,
Logger const& logger);

BackgroundSync(BackgroundSync const& item) = delete;
Expand All @@ -57,7 +58,7 @@ class BackgroundSync final : public data_interfaces::IDataSystem {

private:
data_components::MemoryStore store_;
data_components::ChangeNotifier change_notifier_;
std::shared_ptr<data_components::ChangeNotifier> change_notifier_;
// Needs to be shared to that the source can keep itself alive through
// async operations.
std::shared_ptr<data_interfaces::IDataSynchronizer> synchronizer_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
#include <launchdarkly/encoding/base_64.hpp>
#include <launchdarkly/network/http_error_messages.hpp>

#include <launchdarkly/serialization/json_flag.hpp>
#include <launchdarkly/detail/serialization/json_primitives.hpp>
#include <launchdarkly/serialization/json_flag.hpp>
#include <launchdarkly/serialization/json_sdk_data_set.hpp>
#include <launchdarkly/server_side/data_source_status.hpp>

Expand Down Expand Up @@ -65,19 +65,18 @@ std::string const& PollingDataSource::Identity() const {
PollingDataSource::PollingDataSource(
boost::asio::any_io_executor const& ioc,
Logger const& logger,
data_components::DataSourceStatusManager& status_manager,
std::shared_ptr<data_components::DataSourceStatusManager> status_manager,
config::built::ServiceEndpoints const& endpoints,
config::built::BackgroundSyncConfig::PollingConfig const&
data_source_config,
config::built::HttpProperties const& http_properties)
: logger_(logger),
status_manager_(status_manager),
status_manager_(std::move(status_manager)),
requester_(ioc, http_properties.Tls()),
polling_interval_(data_source_config.poll_interval),
request_(
MakeRequest(logger_, data_source_config, endpoints, http_properties)),
timer_(ioc),
sink_(nullptr) {
timer_(ioc) {
if (polling_interval_ < data_source_config.min_polling_interval) {
LD_LOG(logger_, LogLevel::kWarn)
<< "Polling interval too frequent, defaulting to "
Expand Down Expand Up @@ -127,7 +126,7 @@ void PollingDataSource::HandlePollResult(network::HttpResult const& res) {

if (res.IsError()) {
auto const& error_message = res.ErrorMessage();
status_manager_.SetState(
status_manager_->SetState(
DataSourceStatus::DataSourceState::kInterrupted,
DataSourceStatus::ErrorInfo::ErrorKind::kNetworkError,
error_message.has_value() ? *error_message : "unknown error");
Expand All @@ -141,7 +140,7 @@ void PollingDataSource::HandlePollResult(network::HttpResult const& res) {
auto parsed = boost::json::parse(body.value(), error_code);
if (error_code) {
LD_LOG(logger_, LogLevel::kError) << kErrorParsingPut;
status_manager_.SetError(
status_manager_->SetError(
DataSourceStatus::ErrorInfo::ErrorKind::kInvalidData,
kErrorParsingPut);
return;
Expand All @@ -150,18 +149,20 @@ void PollingDataSource::HandlePollResult(network::HttpResult const& res) {
tl::expected<data_model::SDKDataSet, JsonError>>(parsed);

if (poll_result.has_value()) {
sink_->Init(std::move(*poll_result));
status_manager_.SetState(
DataSourceStatus::DataSourceState::kValid);
if (auto sink = sink_.lock()) {
sink->Init(std::move(*poll_result));
status_manager_->SetState(
DataSourceStatus::DataSourceState::kValid);
}
return;
}
LD_LOG(logger_, LogLevel::kError) << kErrorPutInvalid;
status_manager_.SetError(
status_manager_->SetError(
DataSourceStatus::ErrorInfo::ErrorKind::kInvalidData,
kErrorPutInvalid);
return;
}
status_manager_.SetState(
status_manager_->SetState(
DataSourceStatus::DataSourceState::kInterrupted,
DataSourceStatus::ErrorInfo::ErrorKind::kUnknown,
"polling response contained no body.");
Expand All @@ -172,12 +173,12 @@ void PollingDataSource::HandlePollResult(network::HttpResult const& res) {
// parse the body.
} else {
if (network::IsRecoverableStatus(res.Status())) {
status_manager_.SetState(
status_manager_->SetState(
DataSourceStatus::DataSourceState::kInterrupted, res.Status(),
launchdarkly::network::ErrorForStatusCode(
res.Status(), "polling request", "will retry"));
} else {
status_manager_.SetState(
status_manager_->SetState(
DataSourceStatus::DataSourceState::kOff, res.Status(),
launchdarkly::network::ErrorForStatusCode(
res.Status(), "polling request", std::nullopt));
Expand Down Expand Up @@ -226,16 +227,16 @@ void PollingDataSource::StartPollingTimer() {
}

void PollingDataSource::StartAsync(
data_interfaces::IDestination* dest,
std::shared_ptr<data_interfaces::IDestination> dest,
data_model::SDKDataSet const* bootstrap_data) {
boost::ignore_unused(bootstrap_data);

sink_ = dest;

status_manager_.SetState(DataSourceStatus::DataSourceState::kInitializing);
status_manager_->SetState(DataSourceStatus::DataSourceState::kInitializing);
if (!request_.Valid()) {
LD_LOG(logger_, LogLevel::kError) << kCouldNotParseEndpoint;
status_manager_.SetState(
status_manager_->SetState(
DataSourceStatus::DataSourceState::kOff,
DataSourceStatus::ErrorInfo::ErrorKind::kNetworkError,
kCouldNotParseEndpoint);
Expand All @@ -248,7 +249,7 @@ void PollingDataSource::StartAsync(
}

void PollingDataSource::ShutdownAsync(std::function<void()> completion) {
status_manager_.SetState(DataSourceStatus::DataSourceState::kInitializing);
status_manager_->SetState(DataSourceStatus::DataSourceState::kInitializing);
timer_.cancel();
if (completion) {
boost::asio::post(timer_.get_executor(), completion);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <boost/asio/any_io_executor.hpp>

#include <chrono>
#include <memory>

namespace launchdarkly::server_side::data_systems {

Expand All @@ -21,13 +22,14 @@ class PollingDataSource
public:
PollingDataSource(boost::asio::any_io_executor const& ioc,
Logger const& logger,
data_components::DataSourceStatusManager& status_manager,
std::shared_ptr<data_components::DataSourceStatusManager>
status_manager,
config::built::ServiceEndpoints const& endpoints,
config::built::BackgroundSyncConfig::PollingConfig const&
data_source_config,
config::built::HttpProperties const& http_properties);

void StartAsync(data_interfaces::IDestination* dest,
void StartAsync(std::shared_ptr<data_interfaces::IDestination> dest,
data_model::SDKDataSet const* bootstrap_data) override;

void ShutdownAsync(std::function<void()> completion) override;
Expand All @@ -40,11 +42,8 @@ class PollingDataSource

Logger const& logger_;

// Status manager is used to report the status of the data source. It must
// outlive the source. This source performs asynchronous
// operations, so a completion handler might invoke the status manager after
// it has been destroyed.
data_components::DataSourceStatusManager& status_manager_;
// Reports the status of the data source.
std::shared_ptr<data_components::DataSourceStatusManager> status_manager_;

// Responsible for performing HTTP requests using boost::asio.
network::AsioRequester requester_;
Expand All @@ -66,7 +65,7 @@ class PollingDataSource
std::chrono::time_point<std::chrono::system_clock> last_poll_start_;

// Destination for all data obtained via polling.
data_interfaces::IDestination* sink_;
std::weak_ptr<data_interfaces::IDestination> sink_;

void StartPollingTimer();
};
Expand Down
Loading
Loading