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
12 changes: 7 additions & 5 deletions libs/client-sdk/src/client_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@

std::future<bool> ClientImpl::IdentifyAsync(Context context) {
UpdateContextSynchronized(context);
flag_manager_.ClearSelector();
flag_manager_.LoadCache(context);
event_processor_->SendAsync(events::IdentifyEventParams{
std::chrono::system_clock::now(), std::move(context)});
Expand Down Expand Up @@ -211,7 +212,7 @@
std::unordered_map<Client::FlagKey, Value> result;
for (auto& [key, descriptor] : flag_manager_.Store().GetAll()) {
if (descriptor->item) {
result.try_emplace(key, descriptor->item->Detail().Value());

Check warning on line 215 in libs/client-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/client-sdk/src/client_impl.cpp:215:37 [bugprone-unchecked-optional-access]

unchecked access to optional value
}
}
return result;
Expand Down Expand Up @@ -244,11 +245,12 @@
}

template <typename T>
EvaluationDetail<T> ClientImpl::VariationInternal(FlagKey const& key,
Value default_value,
bool check_type,
bool detailed,
std::unordered_set<std::string>* visited) {
EvaluationDetail<T> ClientImpl::VariationInternal(

Check warning on line 248 in libs/client-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/client-sdk/src/client_impl.cpp:248:33 [readability-function-cognitive-complexity]

function 'VariationInternal' has cognitive complexity of 27 (threshold 25)
FlagKey const& key,

Check warning on line 249 in libs/client-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/client-sdk/src/client_impl.cpp:249:5 [bugprone-easily-swappable-parameters]

2 adjacent parameters of 'VariationInternal' of convertible types are easily swapped by mistake
Value default_value,
bool check_type,
bool detailed,
std::unordered_set<std::string>* visited) {
auto desc = flag_manager_.Store().Get(key);

events::FeatureEventParams event = {
Expand Down Expand Up @@ -301,7 +303,7 @@

LD_ASSERT(desc->item);

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

Check warning on line 306 in libs/client-sdk/src/client_impl.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/client-sdk/src/client_impl.cpp:306:25 [bugprone-unchecked-optional-access]

unchecked access to optional value
auto const& detail = flag.Detail();

// The Prerequisites vector represents the evaluated prerequisites of
Expand Down
4 changes: 4 additions & 0 deletions libs/client-sdk/src/flag_manager/flag_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
FlagManager::FlagManager(std::string const& sdk_key,
Logger& logger,
std::size_t max_cached_contexts,
std::shared_ptr<IPersistence> persistence)

Check warning on line 11 in libs/client-sdk/src/flag_manager/flag_manager.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/client-sdk/src/flag_manager/flag_manager.cpp:11:56 [performance-unnecessary-value-param]

the parameter 'persistence' is copied for each invocation but only used as a const reference; consider making it a const reference
: flag_updater_(flag_store_),
persistence_updater_(sdk_key,
flag_updater_,
Expand Down Expand Up @@ -37,4 +37,8 @@
persistence_updater_.LoadCached(context);
}

void FlagManager::ClearSelector() {
flag_store_.ClearSelector();
}

} // namespace launchdarkly::client_side::flag_manager
3 changes: 3 additions & 0 deletions libs/client-sdk/src/flag_manager/flag_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ class FlagManager {

void LoadCache(Context const& context);

/** Forgets the selector, leaving the stored flag data in place. */
void ClearSelector();

private:
FlagStore flag_store_;
FlagUpdater flag_updater_;
Expand Down
38 changes: 38 additions & 0 deletions libs/client-sdk/tests/fdv2_cache_initializer_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

namespace {

class TestPersistence : public IPersistence {

Check warning on line 24 in libs/client-sdk/tests/fdv2_cache_initializer_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/client-sdk/tests/fdv2_cache_initializer_test.cpp:24:7 [cppcoreguidelines-virtual-class-destructor]

destructor of 'TestPersistence' is public and non-virtual
public:
using StoreType =
std::map<std::string,
Expand Down Expand Up @@ -63,7 +63,7 @@
std::make_shared<TestPersistence>(TestPersistence::StoreType{
{kEnvironment,
{{kContextId, R"({"flagA":{"version":1,"value":"test"}})"}}}});
FlagManager flag_manager("the-key", logger, 5, persistence);

Check warning on line 66 in libs/client-sdk/tests/fdv2_cache_initializer_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/client-sdk/tests/fdv2_cache_initializer_test.cpp:66:49 [cppcoreguidelines-avoid-magic-numbers]

5 is a magic number; consider replacing it with a named constant

FDv2CacheInitializer initializer(&flag_manager.Cache(), context, logger);
auto future = initializer.Run();
Expand All @@ -71,13 +71,13 @@
auto result = future.GetResult();

// The result is a full change set carrying the cached flag.
auto* change_set = std::get_if<FDv2SourceResult::ChangeSet>(&result->value);

Check warning on line 74 in libs/client-sdk/tests/fdv2_cache_initializer_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/client-sdk/tests/fdv2_cache_initializer_test.cpp:74:66 [bugprone-unchecked-optional-access]

unchecked access to optional value
ASSERT_NE(nullptr, change_set);
EXPECT_EQ(ChangeSetType::kFull, change_set->change_set.type);
ASSERT_EQ(1u, change_set->change_set.data.size());

Check warning on line 77 in libs/client-sdk/tests/fdv2_cache_initializer_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/client-sdk/tests/fdv2_cache_initializer_test.cpp:77:15 [readability-uppercase-literal-suffix]

integer literal has suffix 'u', which is not uppercase
EXPECT_EQ("flagA", change_set->change_set.data[0].key);
EXPECT_EQ(Value("test"),
change_set->change_set.data[0].item.item->Detail().Value());

Check warning on line 80 in libs/client-sdk/tests/fdv2_cache_initializer_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

libs/client-sdk/tests/fdv2_cache_initializer_test.cpp:80:15 [bugprone-unchecked-optional-access]

unchecked access to optional value

// A delta against unverified cached data could silently corrupt the store,
// so the cache supplies no selector.
Expand Down Expand Up @@ -120,6 +120,44 @@
EXPECT_EQ(ChangeSetType::kNone, change_set->change_set.type);
}

TEST(FDv2CacheInitializerTest, ReadsTheContextItWasBuiltFor) {
auto first = ContextBuilder().Kind("user", "first").Build();
auto second = ContextBuilder().Kind("user", "second").Build();
auto logger = launchdarkly::logging::NullLogger();
auto persistence =
std::make_shared<TestPersistence>(TestPersistence::StoreType{
{kEnvironment,
{{PersistenceEncodeKey(first.CanonicalKey()),
R"({"flagA":{"version":1,"value":"first-value"}})"}}}});
FlagManager flag_manager("the-key", logger, 5, persistence);

// Initialize for the context that has nothing cached.
FDv2CacheInitializer second_initializer(&flag_manager.Cache(), second,
logger);
auto second_future = second_initializer.Run();
ASSERT_TRUE(second_future.IsFinished());
auto second_result = second_future.GetResult();

// Another context's data in the same cache is not a hit.
auto* second_change_set =
std::get_if<FDv2SourceResult::ChangeSet>(&second_result->value);
ASSERT_NE(nullptr, second_change_set);
EXPECT_EQ(ChangeSetType::kNone, second_change_set->change_set.type);

// Initialize for the context whose data is cached.
FDv2CacheInitializer first_initializer(&flag_manager.Cache(), first,
logger);
auto first_future = first_initializer.Run();
ASSERT_TRUE(first_future.IsFinished());
auto first_result = first_future.GetResult();

// That context's cached data comes back as a full data set.
auto* first_change_set =
std::get_if<FDv2SourceResult::ChangeSet>(&first_result->value);
ASSERT_NE(nullptr, first_change_set);
EXPECT_EQ(ChangeSetType::kFull, first_change_set->change_set.type);
}

// The orchestrator needs to tell cache initializers apart from network ones,
// so that a miss with nothing else configured still starts the SDK.
TEST(FDv2CacheInitializerTest, FactoryIdentifiesItselfAsReadingTheCache) {
Expand Down
74 changes: 74 additions & 0 deletions libs/client-sdk/tests/flag_persistence_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,80 @@ TEST(FlagPersistenceTests, CanLoadCache) {
EXPECT_EQ("test", store.Get("flagA")->item->Detail().Value().AsString());
}

TEST(FlagPersistenceTests, LoadingACacheMissRetainsTheExistingData) {
auto first = ContextBuilder().Kind("user", "first").Build();
auto second = ContextBuilder().Kind("user", "second").Build();
auto store = FlagStore();
auto updater = FlagUpdater(store);
auto persistence =
std::make_shared<TestPersistence>(TestPersistence::StoreType());
auto logger = launchdarkly::logging::NullLogger();

FlagPersistence flag_persistence("the-key", updater, store, persistence,
logger, 5);

// Put the first context's flag data in the store.
flag_persistence.Apply(
first,
FlagChangeSet{
ChangeSetType::kFull,
{FlagChange{
"flagA",
ItemDescriptor{EvaluationResult{
1, std::nullopt, false, false, std::nullopt,
EvaluationDetailInternal{Value("first-value"), std::nullopt,
std::nullopt}}}}},
Selector{}},
/* from_cache= */ false);

// Load a context that has nothing cached.
flag_persistence.LoadCached(second);

// The flag data already in the store stays there.
ASSERT_TRUE(store.Get("flagA"));
EXPECT_EQ(Value("first-value"), store.Get("flagA")->item->Detail().Value());
}

TEST(FlagPersistenceTests, LoadingACacheHitReplacesTheExistingData) {
auto first = ContextBuilder().Kind("user", "first").Build();
auto second = ContextBuilder().Kind("user", "second").Build();
auto store = FlagStore();
auto updater = FlagUpdater(store);
auto logger = launchdarkly::logging::NullLogger();

auto persistence =
std::make_shared<TestPersistence>(TestPersistence::StoreType{
{"LaunchDarkly_rUTcjlHPv6Vegd27YmtGYkEGkEUGaEbn5M0JYTFQUpA=",
{{PersistenceEncodeKey(second.CanonicalKey()),
R"({"flagB":{"version":1,"value":"second-value"}})"}}}});

FlagPersistence flag_persistence("the-key", updater, store, persistence,
logger, 5);

// Put the first context's flag data in the store.
flag_persistence.Apply(
first,
FlagChangeSet{
ChangeSetType::kFull,
{FlagChange{
"flagA",
ItemDescriptor{EvaluationResult{
1, std::nullopt, false, false, std::nullopt,
EvaluationDetailInternal{Value("first-value"), std::nullopt,
std::nullopt}}}}},
Selector{}},
/* from_cache= */ false);

// Load a context that has cached data.
flag_persistence.LoadCached(second);

// The cached data takes the place of what the store held.
EXPECT_FALSE(store.Get("flagA"));
ASSERT_TRUE(store.Get("flagB"));
EXPECT_EQ(Value("second-value"),
store.Get("flagB")->item->Detail().Value());
}

TEST(FlagPersistenceTests, EvictsContextsBeyondMax) {
auto store = FlagStore();
auto updater = FlagUpdater(store);
Expand Down
Loading