From ab422ea5f97f04772a03b431295b009f3ad44f5f Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Wed, 2 Sep 2026 03:32:23 +0000 Subject: [PATCH 1/2] feat: Discard the FDv2 selector when the evaluation context changes --- libs/client-sdk/src/client_impl.cpp | 4 + .../src/flag_manager/flag_manager.cpp | 4 + .../src/flag_manager/flag_manager.hpp | 7 + .../tests/fdv2_context_switch_test.cpp | 175 ++++++++++++++++++ 4 files changed, 190 insertions(+) create mode 100644 libs/client-sdk/tests/fdv2_context_switch_test.cpp diff --git a/libs/client-sdk/src/client_impl.cpp b/libs/client-sdk/src/client_impl.cpp index 82a09db35..fd5a60e02 100644 --- a/libs/client-sdk/src/client_impl.cpp +++ b/libs/client-sdk/src/client_impl.cpp @@ -160,6 +160,10 @@ static bool IsInitializedSuccessfully(DataSourceStatus::DataSourceState state) { std::future ClientImpl::IdentifyAsync(Context context) { UpdateContextSynchronized(context); + // A selector describes one context's data, so it is never carried over. + // Any flag data already loaded stays available for evaluation until a + // full data set arrives for the new context. + flag_manager_.ClearSelector(); flag_manager_.LoadCache(context); event_processor_->SendAsync(events::IdentifyEventParams{ std::chrono::system_clock::now(), std::move(context)}); diff --git a/libs/client-sdk/src/flag_manager/flag_manager.cpp b/libs/client-sdk/src/flag_manager/flag_manager.cpp index bdeba00b8..68a0334fa 100644 --- a/libs/client-sdk/src/flag_manager/flag_manager.cpp +++ b/libs/client-sdk/src/flag_manager/flag_manager.cpp @@ -37,4 +37,8 @@ void FlagManager::LoadCache(Context const& context) { persistence_updater_.LoadCached(context); } +void FlagManager::ClearSelector() { + flag_store_.ClearSelector(); +} + } // namespace launchdarkly::client_side::flag_manager diff --git a/libs/client-sdk/src/flag_manager/flag_manager.hpp b/libs/client-sdk/src/flag_manager/flag_manager.hpp index ad09bb55e..ec08a057e 100644 --- a/libs/client-sdk/src/flag_manager/flag_manager.hpp +++ b/libs/client-sdk/src/flag_manager/flag_manager.hpp @@ -28,6 +28,13 @@ class FlagManager { void LoadCache(Context const& context); + /** + * Forgets the selector for the data currently held, leaving the data in + * place. Called when the evaluation context changes, since a selector + * describes one context's data and is never reused for another. + */ + void ClearSelector(); + private: FlagStore flag_store_; FlagUpdater flag_updater_; diff --git a/libs/client-sdk/tests/fdv2_context_switch_test.cpp b/libs/client-sdk/tests/fdv2_context_switch_test.cpp new file mode 100644 index 000000000..9999a2635 --- /dev/null +++ b/libs/client-sdk/tests/fdv2_context_switch_test.cpp @@ -0,0 +1,175 @@ +#include + +#include +#include + +#include +#include + +#include +#include +#include +#include + +using launchdarkly::Context; +using launchdarkly::ContextBuilder; +using launchdarkly::EvaluationDetailInternal; +using launchdarkly::EvaluationResult; +using launchdarkly::Value; +using launchdarkly::client_side::FlagChange; +using launchdarkly::client_side::FlagChangeSet; +using launchdarkly::client_side::ItemDescriptor; +using launchdarkly::client_side::flag_manager::FlagManager; +using launchdarkly::client_side::flag_manager::PersistenceEncodeKey; +using launchdarkly::data_model::ChangeSetType; +using launchdarkly::data_model::Selector; + +namespace { + +class TestPersistence : public IPersistence { + public: + using StoreType = + std::map>>; + + explicit TestPersistence(StoreType store) : store_(std::move(store)) {} + + void Set(std::string storageNamespace, + std::string key, + std::string data) noexcept override { + store_[storageNamespace][key] = data; + } + + void Remove(std::string storageNamespace, + std::string key) noexcept override { + store_[storageNamespace].erase(key); + } + + std::optional Read(std::string storageNamespace, + std::string key) noexcept override { + return store_[storageNamespace][key]; + } + + StoreType store_; +}; + +ItemDescriptor Flag(std::uint64_t version, Value value) { + return ItemDescriptor{ + EvaluationResult{version, std::nullopt, false, false, std::nullopt, + EvaluationDetailInternal{std::move(value), + std::nullopt, std::nullopt}}}; +} + +char const* const kEnvironment = + "LaunchDarkly_rUTcjlHPv6Vegd27YmtGYkEGkEUGaEbn5M0JYTFQUpA="; + +} // namespace + +// A selector names a state the service can compute changes against for one +// context. Sending it for another would ask for the wrong delta. +TEST(FDv2ContextSwitchTest, ClearSelectorForgetsTheBasisButKeepsTheData) { + auto logger = launchdarkly::logging::NullLogger(); + FlagManager flag_manager("the-key", logger, 5, nullptr); + + flag_manager.Updater().Apply( + ContextBuilder().Kind("user", "first").Build(), + FlagChangeSet{ChangeSetType::kFull, + {FlagChange{"flagA", Flag(1, Value("a"))}}, + Selector{Selector::State{1, "state-1"}}}, + /* from_cache= */ false); + + ASSERT_TRUE(flag_manager.Store().CurrentSelector().value.has_value()); + + flag_manager.ClearSelector(); + + EXPECT_FALSE(flag_manager.Store().CurrentSelector().value.has_value()); + ASSERT_TRUE(flag_manager.Store().Get("flagA")); + EXPECT_EQ(Value("a"), + flag_manager.Store().Get("flagA")->item->Detail().Value()); +} + +// Nothing is available to evaluate against for the new context yet, so the +// previous context's data has to stay until a full data set arrives. +TEST(FDv2ContextSwitchTest, CacheMissRetainsThePreviousContextsData) { + auto logger = launchdarkly::logging::NullLogger(); + auto persistence = + std::make_shared(TestPersistence::StoreType()); + FlagManager flag_manager("the-key", logger, 5, persistence); + + auto first = ContextBuilder().Kind("user", "first").Build(); + flag_manager.Updater().Apply( + first, + FlagChangeSet{ChangeSetType::kFull, + {FlagChange{"flagA", Flag(1, Value("first-value"))}}, + Selector{Selector::State{1, "state-1"}}}, + /* from_cache= */ false); + + auto second = ContextBuilder().Kind("user", "second").Build(); + flag_manager.ClearSelector(); + flag_manager.LoadCache(second); + + ASSERT_TRUE(flag_manager.Store().Get("flagA")); + EXPECT_EQ(Value("first-value"), + flag_manager.Store().Get("flagA")->item->Detail().Value()); +} + +TEST(FDv2ContextSwitchTest, CacheHitReplacesThePreviousContextsData) { + auto logger = launchdarkly::logging::NullLogger(); + auto second = ContextBuilder().Kind("user", "second").Build(); + auto persistence = + std::make_shared(TestPersistence::StoreType{ + {kEnvironment, + {{PersistenceEncodeKey(second.CanonicalKey()), + R"({"flagB":{"version":1,"value":"second-value"}})"}}}}); + FlagManager flag_manager("the-key", logger, 5, persistence); + + flag_manager.Updater().Apply( + ContextBuilder().Kind("user", "first").Build(), + FlagChangeSet{ChangeSetType::kFull, + {FlagChange{"flagA", Flag(1, Value("first-value"))}}, + Selector{Selector::State{1, "state-1"}}}, + /* from_cache= */ false); + + flag_manager.ClearSelector(); + flag_manager.LoadCache(second); + + EXPECT_FALSE(flag_manager.Store().Get("flagA")); + ASSERT_TRUE(flag_manager.Store().Get("flagB")); + EXPECT_EQ(Value("second-value"), + flag_manager.Store().Get("flagB")->item->Detail().Value()); +} + +// The cache initializer for a new context reports a hit or a miss for that +// context alone, whatever the store currently holds. +TEST(FDv2ContextSwitchTest, CacheInitializerReadsTheNewContext) { + auto logger = launchdarkly::logging::NullLogger(); + auto first = ContextBuilder().Kind("user", "first").Build(); + auto second = ContextBuilder().Kind("user", "second").Build(); + auto persistence = + std::make_shared(TestPersistence::StoreType{ + {kEnvironment, + {{PersistenceEncodeKey(first.CanonicalKey()), + R"({"flagA":{"version":1,"value":"first-value"}})"}}}}); + FlagManager flag_manager("the-key", logger, 5, persistence); + + using launchdarkly::client_side::data_sources::FDv2CacheInitializer; + using launchdarkly::client_side::data_sources::FDv2SourceResult; + + auto second_result = + FDv2CacheInitializer(&flag_manager.Cache(), second, logger) + .Run() + .GetResult(); + auto* second_change_set = + std::get_if(&second_result->value); + ASSERT_NE(nullptr, second_change_set); + EXPECT_EQ(ChangeSetType::kNone, second_change_set->change_set.type); + + auto first_result = + FDv2CacheInitializer(&flag_manager.Cache(), first, logger) + .Run() + .GetResult(); + auto* first_change_set = + std::get_if(&first_result->value); + ASSERT_NE(nullptr, first_change_set); + EXPECT_EQ(ChangeSetType::kFull, first_change_set->change_set.type); +} From 4993c9f5764a8115e2e21dfc0cfe590e74521c76 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 5 Oct 2026 21:28:59 -0700 Subject: [PATCH 2/2] chore: Tighten comments and fold the new tests into existing files --- libs/client-sdk/src/client_impl.cpp | 14 +- .../src/flag_manager/flag_manager.hpp | 6 +- .../tests/fdv2_cache_initializer_test.cpp | 38 ++++ .../tests/fdv2_context_switch_test.cpp | 175 ------------------ .../tests/flag_persistence_test.cpp | 74 ++++++++ 5 files changed, 119 insertions(+), 188 deletions(-) delete mode 100644 libs/client-sdk/tests/fdv2_context_switch_test.cpp diff --git a/libs/client-sdk/src/client_impl.cpp b/libs/client-sdk/src/client_impl.cpp index fd5a60e02..eca423d66 100644 --- a/libs/client-sdk/src/client_impl.cpp +++ b/libs/client-sdk/src/client_impl.cpp @@ -160,9 +160,6 @@ static bool IsInitializedSuccessfully(DataSourceStatus::DataSourceState state) { std::future ClientImpl::IdentifyAsync(Context context) { UpdateContextSynchronized(context); - // A selector describes one context's data, so it is never carried over. - // Any flag data already loaded stays available for evaluation until a - // full data set arrives for the new context. flag_manager_.ClearSelector(); flag_manager_.LoadCache(context); event_processor_->SendAsync(events::IdentifyEventParams{ @@ -248,11 +245,12 @@ void ClientImpl::FlushAsync() { } template -EvaluationDetail ClientImpl::VariationInternal(FlagKey const& key, - Value default_value, - bool check_type, - bool detailed, - std::unordered_set* visited) { +EvaluationDetail ClientImpl::VariationInternal( + FlagKey const& key, + Value default_value, + bool check_type, + bool detailed, + std::unordered_set* visited) { auto desc = flag_manager_.Store().Get(key); events::FeatureEventParams event = { diff --git a/libs/client-sdk/src/flag_manager/flag_manager.hpp b/libs/client-sdk/src/flag_manager/flag_manager.hpp index ec08a057e..1097d90c8 100644 --- a/libs/client-sdk/src/flag_manager/flag_manager.hpp +++ b/libs/client-sdk/src/flag_manager/flag_manager.hpp @@ -28,11 +28,7 @@ class FlagManager { void LoadCache(Context const& context); - /** - * Forgets the selector for the data currently held, leaving the data in - * place. Called when the evaluation context changes, since a selector - * describes one context's data and is never reused for another. - */ + /** Forgets the selector, leaving the stored flag data in place. */ void ClearSelector(); private: diff --git a/libs/client-sdk/tests/fdv2_cache_initializer_test.cpp b/libs/client-sdk/tests/fdv2_cache_initializer_test.cpp index 837bd14e4..fd9ae82f1 100644 --- a/libs/client-sdk/tests/fdv2_cache_initializer_test.cpp +++ b/libs/client-sdk/tests/fdv2_cache_initializer_test.cpp @@ -120,6 +120,44 @@ TEST(FDv2CacheInitializerTest, NoPersistenceConfiguredProducesANoneIntent) { 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::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(&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(&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) { diff --git a/libs/client-sdk/tests/fdv2_context_switch_test.cpp b/libs/client-sdk/tests/fdv2_context_switch_test.cpp deleted file mode 100644 index 9999a2635..000000000 --- a/libs/client-sdk/tests/fdv2_context_switch_test.cpp +++ /dev/null @@ -1,175 +0,0 @@ -#include - -#include -#include - -#include -#include - -#include -#include -#include -#include - -using launchdarkly::Context; -using launchdarkly::ContextBuilder; -using launchdarkly::EvaluationDetailInternal; -using launchdarkly::EvaluationResult; -using launchdarkly::Value; -using launchdarkly::client_side::FlagChange; -using launchdarkly::client_side::FlagChangeSet; -using launchdarkly::client_side::ItemDescriptor; -using launchdarkly::client_side::flag_manager::FlagManager; -using launchdarkly::client_side::flag_manager::PersistenceEncodeKey; -using launchdarkly::data_model::ChangeSetType; -using launchdarkly::data_model::Selector; - -namespace { - -class TestPersistence : public IPersistence { - public: - using StoreType = - std::map>>; - - explicit TestPersistence(StoreType store) : store_(std::move(store)) {} - - void Set(std::string storageNamespace, - std::string key, - std::string data) noexcept override { - store_[storageNamespace][key] = data; - } - - void Remove(std::string storageNamespace, - std::string key) noexcept override { - store_[storageNamespace].erase(key); - } - - std::optional Read(std::string storageNamespace, - std::string key) noexcept override { - return store_[storageNamespace][key]; - } - - StoreType store_; -}; - -ItemDescriptor Flag(std::uint64_t version, Value value) { - return ItemDescriptor{ - EvaluationResult{version, std::nullopt, false, false, std::nullopt, - EvaluationDetailInternal{std::move(value), - std::nullopt, std::nullopt}}}; -} - -char const* const kEnvironment = - "LaunchDarkly_rUTcjlHPv6Vegd27YmtGYkEGkEUGaEbn5M0JYTFQUpA="; - -} // namespace - -// A selector names a state the service can compute changes against for one -// context. Sending it for another would ask for the wrong delta. -TEST(FDv2ContextSwitchTest, ClearSelectorForgetsTheBasisButKeepsTheData) { - auto logger = launchdarkly::logging::NullLogger(); - FlagManager flag_manager("the-key", logger, 5, nullptr); - - flag_manager.Updater().Apply( - ContextBuilder().Kind("user", "first").Build(), - FlagChangeSet{ChangeSetType::kFull, - {FlagChange{"flagA", Flag(1, Value("a"))}}, - Selector{Selector::State{1, "state-1"}}}, - /* from_cache= */ false); - - ASSERT_TRUE(flag_manager.Store().CurrentSelector().value.has_value()); - - flag_manager.ClearSelector(); - - EXPECT_FALSE(flag_manager.Store().CurrentSelector().value.has_value()); - ASSERT_TRUE(flag_manager.Store().Get("flagA")); - EXPECT_EQ(Value("a"), - flag_manager.Store().Get("flagA")->item->Detail().Value()); -} - -// Nothing is available to evaluate against for the new context yet, so the -// previous context's data has to stay until a full data set arrives. -TEST(FDv2ContextSwitchTest, CacheMissRetainsThePreviousContextsData) { - auto logger = launchdarkly::logging::NullLogger(); - auto persistence = - std::make_shared(TestPersistence::StoreType()); - FlagManager flag_manager("the-key", logger, 5, persistence); - - auto first = ContextBuilder().Kind("user", "first").Build(); - flag_manager.Updater().Apply( - first, - FlagChangeSet{ChangeSetType::kFull, - {FlagChange{"flagA", Flag(1, Value("first-value"))}}, - Selector{Selector::State{1, "state-1"}}}, - /* from_cache= */ false); - - auto second = ContextBuilder().Kind("user", "second").Build(); - flag_manager.ClearSelector(); - flag_manager.LoadCache(second); - - ASSERT_TRUE(flag_manager.Store().Get("flagA")); - EXPECT_EQ(Value("first-value"), - flag_manager.Store().Get("flagA")->item->Detail().Value()); -} - -TEST(FDv2ContextSwitchTest, CacheHitReplacesThePreviousContextsData) { - auto logger = launchdarkly::logging::NullLogger(); - auto second = ContextBuilder().Kind("user", "second").Build(); - auto persistence = - std::make_shared(TestPersistence::StoreType{ - {kEnvironment, - {{PersistenceEncodeKey(second.CanonicalKey()), - R"({"flagB":{"version":1,"value":"second-value"}})"}}}}); - FlagManager flag_manager("the-key", logger, 5, persistence); - - flag_manager.Updater().Apply( - ContextBuilder().Kind("user", "first").Build(), - FlagChangeSet{ChangeSetType::kFull, - {FlagChange{"flagA", Flag(1, Value("first-value"))}}, - Selector{Selector::State{1, "state-1"}}}, - /* from_cache= */ false); - - flag_manager.ClearSelector(); - flag_manager.LoadCache(second); - - EXPECT_FALSE(flag_manager.Store().Get("flagA")); - ASSERT_TRUE(flag_manager.Store().Get("flagB")); - EXPECT_EQ(Value("second-value"), - flag_manager.Store().Get("flagB")->item->Detail().Value()); -} - -// The cache initializer for a new context reports a hit or a miss for that -// context alone, whatever the store currently holds. -TEST(FDv2ContextSwitchTest, CacheInitializerReadsTheNewContext) { - auto logger = launchdarkly::logging::NullLogger(); - auto first = ContextBuilder().Kind("user", "first").Build(); - auto second = ContextBuilder().Kind("user", "second").Build(); - auto persistence = - std::make_shared(TestPersistence::StoreType{ - {kEnvironment, - {{PersistenceEncodeKey(first.CanonicalKey()), - R"({"flagA":{"version":1,"value":"first-value"}})"}}}}); - FlagManager flag_manager("the-key", logger, 5, persistence); - - using launchdarkly::client_side::data_sources::FDv2CacheInitializer; - using launchdarkly::client_side::data_sources::FDv2SourceResult; - - auto second_result = - FDv2CacheInitializer(&flag_manager.Cache(), second, logger) - .Run() - .GetResult(); - auto* second_change_set = - std::get_if(&second_result->value); - ASSERT_NE(nullptr, second_change_set); - EXPECT_EQ(ChangeSetType::kNone, second_change_set->change_set.type); - - auto first_result = - FDv2CacheInitializer(&flag_manager.Cache(), first, logger) - .Run() - .GetResult(); - auto* first_change_set = - std::get_if(&first_result->value); - ASSERT_NE(nullptr, first_change_set); - EXPECT_EQ(ChangeSetType::kFull, first_change_set->change_set.type); -} diff --git a/libs/client-sdk/tests/flag_persistence_test.cpp b/libs/client-sdk/tests/flag_persistence_test.cpp index ae449da35..d426e81ac 100644 --- a/libs/client-sdk/tests/flag_persistence_test.cpp +++ b/libs/client-sdk/tests/flag_persistence_test.cpp @@ -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::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::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);