From 0aff916a0bb0e33a0f85ae72428a3a81ee8a6e5a Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Wed, 28 Jan 2026 15:00:18 +0000 Subject: [PATCH 1/3] Implement class Evaluation Context Signed-off-by: NeaguGeorgiana23 --- openfeature/BUILD | 3 + openfeature/evaluation_context.cpp | 78 ++++++++++++ openfeature/evaluation_context.h | 51 +++++++- test/BUILD | 11 ++ test/evaluation_context_test.cpp | 188 +++++++++++++++++++++++++++++ 5 files changed, 329 insertions(+), 2 deletions(-) create mode 100644 openfeature/evaluation_context.cpp create mode 100644 test/evaluation_context_test.cpp diff --git a/openfeature/BUILD b/openfeature/BUILD index f05a514..2fecde8 100644 --- a/openfeature/BUILD +++ b/openfeature/BUILD @@ -1,3 +1,5 @@ +load("@rules_cc//cc:defs.bzl", "cc_library") + package( default_visibility = ["//visibility:public"], ) @@ -23,6 +25,7 @@ cc_library( cc_library( name = "evaluation_context", hdrs = ["evaluation_context.h"], + srcs = ["evaluation_context.cpp"], include_prefix = "openfeature", ) diff --git a/openfeature/evaluation_context.cpp b/openfeature/evaluation_context.cpp new file mode 100644 index 0000000..12de76d --- /dev/null +++ b/openfeature/evaluation_context.cpp @@ -0,0 +1,78 @@ +#include "evaluation_context.h" + +namespace openfeature { + +EvaluationContext::EvaluationContext(std::optional targeting_key, + std::map attributes) + : targeting_key_(std::move(targeting_key)), + attributes_(std::move(attributes)) {} + +std::optional EvaluationContext::GetTargetingKey() const { + if (!targeting_key_.has_value()) { + return std::nullopt; + } + return targeting_key_; +} + +const std::any* EvaluationContext::GetValue(std::string_view key) const { + auto it = attributes_.find(std::string(key)); + if (it != attributes_.end()) { + return &it->second; + } + return nullptr; +} + +const std::map& EvaluationContext::GetAttributes() + const { + return attributes_; +} + +EvaluationContext EvaluationContext::merge( + std::initializer_list contexts) { + Builder builder; + + // Merge Attributes from all contexts (higher precedence overwrites lower). + for (const EvaluationContext* ctx : contexts) { + if (ctx) { + for (const auto& pair : ctx->GetAttributes()) { + builder.withAttribute(pair.first, pair.second); + } + } + } + + // Find the first valid targeting key from highest to lowest precedence. + // We iterate through the list backwards to check the highest precedence + // context first. + std::vector reversed(contexts); + std::reverse(reversed.begin(), reversed.end()); + + for (const EvaluationContext* ctx : reversed) { + if (ctx) { + auto key = ctx->GetTargetingKey(); + if (key.has_value() && !key->empty()) { + builder.withTargetingKey(std::string(key.value())); + break; + } + } + } + + return builder.build(); +} + +EvaluationContext::Builder& EvaluationContext::Builder::withTargetingKey( + std::string key) { + this->targeting_key_ = std::move(key); + return *this; +} + +EvaluationContext::Builder& EvaluationContext::Builder::withAttribute( + std::string key, std::any value) { + this->attributes_.insert_or_assign(std::move(key), std::move(value)); + return *this; +} + +EvaluationContext EvaluationContext::Builder::build() const { + return EvaluationContext(targeting_key_.value_or(""), attributes_); +} + +} // namespace openfeature diff --git a/openfeature/evaluation_context.h b/openfeature/evaluation_context.h index c351569..367b1c2 100644 --- a/openfeature/evaluation_context.h +++ b/openfeature/evaluation_context.h @@ -1,12 +1,59 @@ #ifndef CPP_SDK_INCLUDE_OPENFEATURE_EVALUATION_CONTEXT_H_ #define CPP_SDK_INCLUDE_OPENFEATURE_EVALUATION_CONTEXT_H_ +#include +#include +#include +#include +#include +#include +#include + namespace openfeature { -// EvaluationContext provides ambient information for the purposes of flag -// evaluation https://openfeature.dev/specification/sections/evaluation-context +// EvaluationContext provides information for the purposes of flag +// evaluation. class EvaluationContext { public: + // The Builder class is the only way to construct an EvaluationContext. + class Builder; + + EvaluationContext() = delete; + + std::optional GetTargetingKey() const; + + // Get a specific attribute value. + // Returns nullptr if key does not exist. + const std::any* GetValue(std::string_view key) const; + + // Get all attributes + const std::map& GetAttributes() const; + + // It takes a list of pointers to contexts to avoid unnecessary copies. + // The order of contexts in the initializer list determines precedence. + static EvaluationContext merge( + std::initializer_list contexts); + + private: + EvaluationContext(std::optional targeting_key, + std::map attributes); + + std::optional targeting_key_; + std::map attributes_; +}; + +class EvaluationContext::Builder { + public: + // Builder methods return a reference to self to allow for chaining. + Builder& withTargetingKey(std::string key); + Builder& withAttribute(std::string key, std::any value); + + // The build() method creates the final, immutable EvaluationContext object. + EvaluationContext build() const; + + private: + std::optional targeting_key_; + std::map attributes_; }; } // namespace openfeature diff --git a/test/BUILD b/test/BUILD index bddb116..7984dcd 100644 --- a/test/BUILD +++ b/test/BUILD @@ -1,3 +1,5 @@ +load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test") + cc_library( name = "mock_feature_provider", testonly = True, @@ -46,3 +48,12 @@ cc_test( "@googletest//:gtest_main", ], ) + +cc_test( + name = "evaluation_context_test", + srcs = ["evaluation_context_test.cpp"], + deps = [ + "//openfeature:evaluation_context", + "@googletest//:gtest_main", + ], +) diff --git a/test/evaluation_context_test.cpp b/test/evaluation_context_test.cpp new file mode 100644 index 0000000..0013f20 --- /dev/null +++ b/test/evaluation_context_test.cpp @@ -0,0 +1,188 @@ +#include "openfeature/evaluation_context.h" + +#include +#include + +#include +#include + +using namespace openfeature; + +class EvaluationContextTest : public ::testing::Test { + protected: + // Helper to cast std::any safely for assertions. + template + T AnyCast(const std::any* value) { + if (!value) throw std::runtime_error("Value is nullptr"); + return std::any_cast(*value); + } +}; + +// Test that a context built with no properties returns expected defaults. +TEST_F(EvaluationContextTest, DefaultBuilderCreatesEmptyContext) { + EvaluationContext ctx = EvaluationContext::Builder().build(); + + // Based on implementation, a missing key in builder becomes "" in + // constructor. + auto key = ctx.GetTargetingKey(); + ASSERT_TRUE(key.has_value()); + EXPECT_TRUE(key->empty()); + + EXPECT_TRUE(ctx.GetAttributes().empty()); +} + +// Test setting and retrieving the targeting key. +TEST_F(EvaluationContextTest, BuilderSetsTargetingKey) { + std::string expected_key = "user-12345"; + EvaluationContext ctx = + EvaluationContext::Builder().withTargetingKey(expected_key).build(); + + auto key = ctx.GetTargetingKey(); + ASSERT_TRUE(key.has_value()); + EXPECT_EQ(key.value(), expected_key); +} + +// Test setting and retrieving various attribute types. +TEST_F(EvaluationContextTest, BuilderSetsAttributesOfVariousTypes) { + EvaluationContext ctx = EvaluationContext::Builder() + .withAttribute("str_attr", std::string("test")) + .withAttribute("int_attr", 42) + .withAttribute("bool_attr", true) + .withAttribute("double_attr", 3.14) + .build(); + + const auto& attrs = ctx.GetAttributes(); + EXPECT_EQ(attrs.size(), 4); + + const std::any* str_val = ctx.GetValue("str_attr"); + ASSERT_NE(str_val, nullptr); + EXPECT_EQ(std::any_cast(*str_val), "test"); + + const std::any* int_val = ctx.GetValue("int_attr"); + ASSERT_NE(int_val, nullptr); + EXPECT_EQ(std::any_cast(*int_val), 42); + + const std::any* bool_val = ctx.GetValue("bool_attr"); + ASSERT_NE(bool_val, nullptr); + EXPECT_TRUE(std::any_cast(*bool_val)); + + const std::any* double_val = ctx.GetValue("double_attr"); + ASSERT_NE(double_val, nullptr); + EXPECT_DOUBLE_EQ(std::any_cast(*double_val), 3.14); +} + +// Test behavior when requesting a non-existent attribute. +TEST_F(EvaluationContextTest, GetValueReturnsNullForMissingKey) { + EvaluationContext ctx = + EvaluationContext::Builder().withAttribute("exists", 1).build(); + + EXPECT_NE(ctx.GetValue("exists"), nullptr); + EXPECT_EQ(ctx.GetValue("does_not_exist"), nullptr); +} + +// Test that setting the same attribute key twice overwrites the previous value +// within the same builder chain. +TEST_F(EvaluationContextTest, BuilderOverwritesDuplicateKeys) { + EvaluationContext ctx = EvaluationContext::Builder() + .withAttribute("key", 100) + .withAttribute("key", 200) + .build(); + + const std::any* val = ctx.GetValue("key"); + ASSERT_NE(val, nullptr); + EXPECT_EQ(std::any_cast(*val), 200); +} + +// Test merging attributes with precedence. +TEST_F(EvaluationContextTest, MergeAttributesWithPrecedence) { + EvaluationContext ctx1 = EvaluationContext::Builder() + .withAttribute("common", 1) + .withAttribute("ctx1", std::string("A")) + .build(); + + EvaluationContext ctx2 = EvaluationContext::Builder() + .withAttribute("common", 2) + .withAttribute("ctx2", std::string("B")) + .build(); + + EvaluationContext merged = EvaluationContext::merge({&ctx1, &ctx2}); + + EXPECT_EQ(merged.GetAttributes().size(), 3); + + EXPECT_EQ(std::any_cast(*merged.GetValue("common")), 2); + EXPECT_EQ(std::any_cast(*merged.GetValue("ctx1")), "A"); + EXPECT_EQ(std::any_cast(*merged.GetValue("ctx2")), "B"); +} + +// Test that the last context in the list with a valid and non-empty targeting +// key remains as the final result. +TEST_F(EvaluationContextTest, MergeTargetingKeyWithPrecedence) { + EvaluationContext ctx_no_key = EvaluationContext::Builder().build(); + EvaluationContext ctx_key_a = + EvaluationContext::Builder().withTargetingKey("KeyA").build(); + EvaluationContext ctx_key_b = + EvaluationContext::Builder().withTargetingKey("KeyB").build(); + + EvaluationContext res1 = EvaluationContext::merge({&ctx_key_a, &ctx_key_b}); + EXPECT_EQ(res1.GetTargetingKey().value(), "KeyB"); + + EvaluationContext res2 = + EvaluationContext::merge({&ctx_key_a, &ctx_no_key, &ctx_no_key}); + EXPECT_EQ(res2.GetTargetingKey().value(), "KeyA"); + + EvaluationContext res3 = + EvaluationContext::merge({&ctx_no_key, &ctx_key_b, &ctx_no_key}); + EXPECT_EQ(res3.GetTargetingKey().value(), "KeyB"); +} + +// Test Merging: Complex scenario with attributes and keys. +TEST_F(EvaluationContextTest, MergeComplexScenario) { + EvaluationContext base = EvaluationContext::Builder() + .withTargetingKey("base-user") + .withAttribute("env", std::string("prod")) + .withAttribute("region", std::string("us-east")) + .build(); + + EvaluationContext request = + EvaluationContext::Builder() + .withTargetingKey("req-user") + .withAttribute("region", std::string("us-west")) + .withAttribute("request_id", 123) + .build(); + + EvaluationContext merged = EvaluationContext::merge({&base, &request}); + + EXPECT_EQ(merged.GetTargetingKey().value(), "req-user"); + + EXPECT_EQ(std::any_cast(*merged.GetValue("env")), "prod"); + EXPECT_EQ(std::any_cast(*merged.GetValue("region")), "us-west"); + EXPECT_EQ(std::any_cast(*merged.GetValue("request_id")), 123); +} + +// The merged context should only reflect non-null inputs. +TEST_F(EvaluationContextTest, MergeIgnoresNullPointers) { + EvaluationContext ctx = + EvaluationContext::Builder().withTargetingKey("valid").build(); + + EvaluationContext merged = EvaluationContext::merge({nullptr, &ctx, nullptr}); + + EXPECT_EQ(merged.GetTargetingKey().value(), "valid"); +} + +// Test that string literals and std::string are stored and retrieved correctly. +TEST_F(EvaluationContextTest, StoresStringCorrectly) { + EvaluationContext ctx_char = + EvaluationContext::Builder().withAttribute("k", "v").build(); + + const std::any* val_char = ctx_char.GetValue("k"); + + EXPECT_TRUE(val_char->type() == typeid(const char*) || + val_char->type() == typeid(char const*)); + + EvaluationContext ctx_str = + EvaluationContext::Builder().withAttribute("k", std::string("v")).build(); + + const std::any* val_str = ctx_str.GetValue("k"); + EXPECT_EQ(val_str->type(), typeid(std::string)); + EXPECT_EQ(std::any_cast(*val_str), "v"); +} From 319928e202bfe8b31489ca059e3dc86b26083c70 Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Fri, 30 Jan 2026 11:26:53 +0000 Subject: [PATCH 2/3] Fix tests that were broken due to EvaluationContext not having a default constructor. Signed-off-by: NeaguGeorgiana23 --- .bazelrc | 7 +++++++ openfeature/client_api.cpp | 4 +++- openfeature/evaluation_context.cpp | 7 ++++++- openfeature/evaluation_context.h | 2 ++ openfeature/global_context_manager.cpp | 3 +++ openfeature/global_context_manager.h | 2 +- test/BUILD | 5 +++++ test/client_api_test.cpp | 12 ++++++------ test/evaluation_context_test.cpp | 11 +++++------ test/feature_provider_status_manager_test.cpp | 2 +- test/global_context_manager_test.cpp | 6 +++--- test/noop_provider_test.cpp | 2 +- test/openfeature_api_test.cpp | 10 +++++++++- test/provider_repository_test.cpp | 2 +- 14 files changed, 53 insertions(+), 22 deletions(-) diff --git a/.bazelrc b/.bazelrc index 4413f02..dc68efa 100644 --- a/.bazelrc +++ b/.bazelrc @@ -4,3 +4,10 @@ # as "No source files found in compile args" when running # `bazel run @hedron_compile_commands//:refresh_all`. build --features=-parse_headers + +# Fix for "relocation refers to local symbol in discarded section" +build --copt=-fno-asynchronous-unwind-tables +build --linkopt=-Wl,--gc-sections + +# Optional: Use a faster, more modern linker +# build --linkopt=-fuse-ld=lld diff --git a/openfeature/client_api.cpp b/openfeature/client_api.cpp index 93ca102..9b654b3 100644 --- a/openfeature/client_api.cpp +++ b/openfeature/client_api.cpp @@ -9,7 +9,9 @@ namespace openfeature { ClientAPI::ClientAPI(ProviderRepository& repository, std::string_view domain) - : provider_repository_(repository), domain_(domain) {} + : provider_repository_(repository), + domain_(domain), + evaluation_context_(EvaluationContext::Builder().build()) {} Metadata ClientAPI::GetMetadata() { return Metadata{domain_}; } diff --git a/openfeature/evaluation_context.cpp b/openfeature/evaluation_context.cpp index 12de76d..1982a35 100644 --- a/openfeature/evaluation_context.cpp +++ b/openfeature/evaluation_context.cpp @@ -71,8 +71,13 @@ EvaluationContext::Builder& EvaluationContext::Builder::withAttribute( return *this; } +EvaluationContext::Builder& EvaluationContext::Builder::withAttribute( + std::string key, const char* value) { + return this->withAttribute(std::move(key), std::string(value)); +} + EvaluationContext EvaluationContext::Builder::build() const { - return EvaluationContext(targeting_key_.value_or(""), attributes_); + return EvaluationContext(targeting_key_, attributes_); } } // namespace openfeature diff --git a/openfeature/evaluation_context.h b/openfeature/evaluation_context.h index 367b1c2..11da501 100644 --- a/openfeature/evaluation_context.h +++ b/openfeature/evaluation_context.h @@ -47,6 +47,8 @@ class EvaluationContext::Builder { // Builder methods return a reference to self to allow for chaining. Builder& withTargetingKey(std::string key); Builder& withAttribute(std::string key, std::any value); + // Overload for const char* to ensure implicit conversion to std::string + Builder& withAttribute(std::string key, const char* value); // The build() method creates the final, immutable EvaluationContext object. EvaluationContext build() const; diff --git a/openfeature/global_context_manager.cpp b/openfeature/global_context_manager.cpp index c356e22..69efc79 100644 --- a/openfeature/global_context_manager.cpp +++ b/openfeature/global_context_manager.cpp @@ -4,6 +4,9 @@ namespace openfeature { +GlobalContextManager::GlobalContextManager() + : global_evaluation_context_(EvaluationContext::Builder().build()) {} + GlobalContextManager& GlobalContextManager::GetInstance() { static GlobalContextManager instance; return instance; diff --git a/openfeature/global_context_manager.h b/openfeature/global_context_manager.h index 53a24f2..89e430c 100644 --- a/openfeature/global_context_manager.h +++ b/openfeature/global_context_manager.h @@ -24,7 +24,7 @@ class GlobalContextManager { EvaluationContext GetGlobalEvaluationContext() const; private: - GlobalContextManager() = default; + GlobalContextManager(); EvaluationContext global_evaluation_context_; mutable std::shared_mutex context_mutex_; }; diff --git a/test/BUILD b/test/BUILD index 66d21af..97e762c 100644 --- a/test/BUILD +++ b/test/BUILD @@ -64,6 +64,11 @@ cc_test( srcs = ["evaluation_context_test.cpp"], deps = [ "//openfeature:evaluation_context", + "@googletest//:gtest_main", + ], +) + +cc_test( name = "openfeature_api_test", srcs = ["openfeature_api_test.cpp"], deps = [ diff --git a/test/client_api_test.cpp b/test/client_api_test.cpp index 8c5039a..94731f6 100644 --- a/test/client_api_test.cpp +++ b/test/client_api_test.cpp @@ -23,7 +23,7 @@ class ClientAPITest : public ::testing::Test { void SetUp() override { // Reset the Global Context to a clean state before each test. GlobalContextManager::GetInstance().SetGlobalEvaluationContext( - EvaluationContext{}); + EvaluationContext::Builder().build()); } ProviderRepository repo_; }; @@ -46,7 +46,7 @@ TEST_F(ClientAPITest, GetProviderStatusDefaultsToReady) { // Test setting and getting the EvaluationContext. TEST_F(ClientAPITest, SetAndGetEvaluationContext) { ClientAPI client(repo_, "test-domain"); - EvaluationContext ctx; + EvaluationContext ctx = EvaluationContext::Builder().build(); // Verify we can set the context without error. EXPECT_NO_THROW(client.SetEvaluationContext(ctx)); @@ -66,7 +66,7 @@ TEST_F(ClientAPITest, GetBooleanValueReturnsDefaultWithNoopProvider) { // Test GetBooleanValue with an EvaluationContext passed in. TEST_F(ClientAPITest, GetBooleanValueWithContextReturnsDefault) { ClientAPI client(repo_, "test-domain"); - EvaluationContext ctx; + EvaluationContext ctx = EvaluationContext::Builder().build(); std::string flag_key = "my-boolean-flag"; EXPECT_TRUE(client.GetBooleanValue(flag_key, true, ctx)); @@ -75,14 +75,14 @@ TEST_F(ClientAPITest, GetBooleanValueWithContextReturnsDefault) { // Test context merging logic indirectly. TEST_F(ClientAPITest, GetBooleanValueSafeWithMergedContexts) { - EvaluationContext global_ctx; + EvaluationContext global_ctx = EvaluationContext::Builder().build(); GlobalContextManager::GetInstance().SetGlobalEvaluationContext(global_ctx); ClientAPI client(repo_, "test-domain"); - EvaluationContext client_ctx; + EvaluationContext client_ctx = EvaluationContext::Builder().build(); client.SetEvaluationContext(client_ctx); - EvaluationContext invocation_ctx; + EvaluationContext invocation_ctx = EvaluationContext::Builder().build(); // This call forces a merge of Global + Client + Invocation contexts. // We expect the NoopProvider to handle the result gracefully (return diff --git a/test/evaluation_context_test.cpp b/test/evaluation_context_test.cpp index 0013f20..d72c57f 100644 --- a/test/evaluation_context_test.cpp +++ b/test/evaluation_context_test.cpp @@ -1,6 +1,5 @@ #include "openfeature/evaluation_context.h" -#include #include #include @@ -25,8 +24,7 @@ TEST_F(EvaluationContextTest, DefaultBuilderCreatesEmptyContext) { // Based on implementation, a missing key in builder becomes "" in // constructor. auto key = ctx.GetTargetingKey(); - ASSERT_TRUE(key.has_value()); - EXPECT_TRUE(key->empty()); + EXPECT_FALSE(key.has_value()); EXPECT_TRUE(ctx.GetAttributes().empty()); } @@ -173,11 +171,12 @@ TEST_F(EvaluationContextTest, MergeIgnoresNullPointers) { TEST_F(EvaluationContextTest, StoresStringCorrectly) { EvaluationContext ctx_char = EvaluationContext::Builder().withAttribute("k", "v").build(); - const std::any* val_char = ctx_char.GetValue("k"); - EXPECT_TRUE(val_char->type() == typeid(const char*) || - val_char->type() == typeid(char const*)); + // Verify it is stored as std::string, not const char* + ASSERT_NE(val_char, nullptr); + EXPECT_EQ(val_char->type(), typeid(std::string)); + EXPECT_EQ(std::any_cast(*val_char), "v"); EvaluationContext ctx_str = EvaluationContext::Builder().withAttribute("k", std::string("v")).build(); diff --git a/test/feature_provider_status_manager_test.cpp b/test/feature_provider_status_manager_test.cpp index 362ec4f..21e736f 100644 --- a/test/feature_provider_status_manager_test.cpp +++ b/test/feature_provider_status_manager_test.cpp @@ -29,7 +29,7 @@ class FeatureProviderStatusManagerTest : public Test { std::shared_ptr mock_provider_; std::unique_ptr manager_; - EvaluationContext ctx_; + EvaluationContext ctx_ = EvaluationContext::Builder().build(); }; TEST_F(FeatureProviderStatusManagerTest, CreateWithNullProviderReturnsError) { diff --git a/test/global_context_manager_test.cpp b/test/global_context_manager_test.cpp index 751fb36..dde462f 100644 --- a/test/global_context_manager_test.cpp +++ b/test/global_context_manager_test.cpp @@ -15,7 +15,7 @@ class GlobalContextManagerTest : public ::testing::Test { // Reset to a clean state before every test. void SetUp() override { GlobalContextManager::GetInstance().SetGlobalEvaluationContext( - EvaluationContext{}); + EvaluationContext::Builder().build()); } }; @@ -28,7 +28,7 @@ TEST_F(GlobalContextManagerTest, ReturnsSameInstance) { TEST_F(GlobalContextManagerTest, SetAndGetContext) { GlobalContextManager& manager = GlobalContextManager::GetInstance(); - EvaluationContext input_ctx; + EvaluationContext input_ctx = EvaluationContext::Builder().build(); EXPECT_NO_THROW(manager.SetGlobalEvaluationContext(input_ctx)); @@ -50,7 +50,7 @@ TEST_F(GlobalContextManagerTest, ThreadSafetyStressTest) { // Writer Thread: Continuously updates the context. std::thread writer([&]() { while (!stop) { - EvaluationContext ctx; + EvaluationContext ctx = EvaluationContext::Builder().build(); // In a real scenario, we would populate ctx with different data here. manager.SetGlobalEvaluationContext(ctx); std::this_thread::sleep_for(std::chrono::milliseconds(1)); diff --git a/test/noop_provider_test.cpp b/test/noop_provider_test.cpp index 3b9b8b3..e3e880e 100644 --- a/test/noop_provider_test.cpp +++ b/test/noop_provider_test.cpp @@ -9,7 +9,7 @@ using namespace openfeature; class NoopProviderTest : public ::testing::Test { protected: NoopProvider provider_; - EvaluationContext ctx_; + EvaluationContext ctx_ = EvaluationContext::Builder().build(); }; // Test to verify the metadata returned by the provider. diff --git a/test/openfeature_api_test.cpp b/test/openfeature_api_test.cpp index f2b9e5a..cc42717 100644 --- a/test/openfeature_api_test.cpp +++ b/test/openfeature_api_test.cpp @@ -20,7 +20,7 @@ class OpenFeatureAPITest : public ::testing::Test { void SetUp() override {} void TearDown() override { api.Shutdown(); - api.SetEvaluationContext(EvaluationContext{}); + api.SetEvaluationContext(EvaluationContext ::Builder().build()); } OpenFeatureAPI& api = OpenFeatureAPI::GetInstance(); @@ -131,12 +131,15 @@ TEST_F(OpenFeatureAPITest, SetProviderAsyncDoesNotBlock) { std::future init_started_future = init_can_start.get_future(); std::promise init_can_complete; std::future init_can_complete_future = init_can_complete.get_future(); + std::promise init_has_finished; EXPECT_CALL(*mock_provider, Init(_)).WillOnce([&](const auto&) { init_can_start.set_value(); init_can_complete_future.wait(); + init_has_finished.set_value(); return absl::OkStatus(); }); + EXPECT_CALL(*mock_provider, Shutdown()).WillOnce(Return(absl::OkStatus())); api.SetProvider(mock_provider); @@ -147,6 +150,7 @@ TEST_F(OpenFeatureAPITest, SetProviderAsyncDoesNotBlock) { // Allow the init to complete. init_can_complete.set_value(); + init_has_finished.get_future().wait(); } // Test the asynchronous SetProvider for a named provider to ensure it doesn't @@ -159,11 +163,14 @@ TEST_F(OpenFeatureAPITest, SetNamedProviderAsyncDoesNotBlock) { std::future init_started_future = init_can_start.get_future(); std::promise init_can_complete; std::future init_can_complete_future = init_can_complete.get_future(); + std::promise init_has_finished; EXPECT_CALL(*mock_provider, Init(_)).WillOnce([&](const auto&) { init_can_start.set_value(); init_can_complete_future.wait(); + init_has_finished.set_value(); return absl::OkStatus(); + ; }); EXPECT_CALL(*mock_provider, Shutdown()).WillOnce(Return(absl::OkStatus())); api.SetProvider(domain, mock_provider); @@ -175,6 +182,7 @@ TEST_F(OpenFeatureAPITest, SetNamedProviderAsyncDoesNotBlock) { // Allow the init to complete. init_can_complete.set_value(); + init_has_finished.get_future().wait(); } // Test that GetClient returns a valid default ClientAPI instance. diff --git a/test/provider_repository_test.cpp b/test/provider_repository_test.cpp index 4be25b0..d7ef4c8 100644 --- a/test/provider_repository_test.cpp +++ b/test/provider_repository_test.cpp @@ -17,7 +17,7 @@ using ::testing::Return; class ProviderRepositoryTest : public ::testing::Test { protected: ProviderRepository repo; - EvaluationContext ctx; + EvaluationContext ctx = EvaluationContext::Builder().build(); }; // Test to verify the constructor initializes with a NoopProvider. From ceaca64c7c12d08ad8fd29c9635b948a21b72146 Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Mon, 2 Feb 2026 09:45:54 +0000 Subject: [PATCH 3/3] Resolve comments. Signed-off-by: NeaguGeorgiana23 --- openfeature/evaluation_context.cpp | 18 ++++----- openfeature/evaluation_context.h | 8 ++-- test/evaluation_context_test.cpp | 59 +++++++++++++++--------------- 3 files changed, 42 insertions(+), 43 deletions(-) diff --git a/openfeature/evaluation_context.cpp b/openfeature/evaluation_context.cpp index 1982a35..741aea0 100644 --- a/openfeature/evaluation_context.cpp +++ b/openfeature/evaluation_context.cpp @@ -27,15 +27,15 @@ const std::map& EvaluationContext::GetAttributes() return attributes_; } -EvaluationContext EvaluationContext::merge( +EvaluationContext EvaluationContext::Merge( std::initializer_list contexts) { Builder builder; // Merge Attributes from all contexts (higher precedence overwrites lower). for (const EvaluationContext* ctx : contexts) { - if (ctx) { + if (ctx != nullptr) { for (const auto& pair : ctx->GetAttributes()) { - builder.withAttribute(pair.first, pair.second); + builder.WithAttribute(pair.first, pair.second); } } } @@ -47,10 +47,10 @@ EvaluationContext EvaluationContext::merge( std::reverse(reversed.begin(), reversed.end()); for (const EvaluationContext* ctx : reversed) { - if (ctx) { + if (ctx != nullptr) { auto key = ctx->GetTargetingKey(); if (key.has_value() && !key->empty()) { - builder.withTargetingKey(std::string(key.value())); + builder.WithTargetingKey(std::string(key.value())); break; } } @@ -59,21 +59,21 @@ EvaluationContext EvaluationContext::merge( return builder.build(); } -EvaluationContext::Builder& EvaluationContext::Builder::withTargetingKey( +EvaluationContext::Builder& EvaluationContext::Builder::WithTargetingKey( std::string key) { this->targeting_key_ = std::move(key); return *this; } -EvaluationContext::Builder& EvaluationContext::Builder::withAttribute( +EvaluationContext::Builder& EvaluationContext::Builder::WithAttribute( std::string key, std::any value) { this->attributes_.insert_or_assign(std::move(key), std::move(value)); return *this; } -EvaluationContext::Builder& EvaluationContext::Builder::withAttribute( +EvaluationContext::Builder& EvaluationContext::Builder::WithAttribute( std::string key, const char* value) { - return this->withAttribute(std::move(key), std::string(value)); + return this->WithAttribute(std::move(key), std::string(value)); } EvaluationContext EvaluationContext::Builder::build() const { diff --git a/openfeature/evaluation_context.h b/openfeature/evaluation_context.h index 11da501..51c876f 100644 --- a/openfeature/evaluation_context.h +++ b/openfeature/evaluation_context.h @@ -31,7 +31,7 @@ class EvaluationContext { // It takes a list of pointers to contexts to avoid unnecessary copies. // The order of contexts in the initializer list determines precedence. - static EvaluationContext merge( + static EvaluationContext Merge( std::initializer_list contexts); private: @@ -45,10 +45,10 @@ class EvaluationContext { class EvaluationContext::Builder { public: // Builder methods return a reference to self to allow for chaining. - Builder& withTargetingKey(std::string key); - Builder& withAttribute(std::string key, std::any value); + Builder& WithTargetingKey(std::string key); + Builder& WithAttribute(std::string key, std::any value); // Overload for const char* to ensure implicit conversion to std::string - Builder& withAttribute(std::string key, const char* value); + Builder& WithAttribute(std::string key, const char* value); // The build() method creates the final, immutable EvaluationContext object. EvaluationContext build() const; diff --git a/test/evaluation_context_test.cpp b/test/evaluation_context_test.cpp index d72c57f..37cec61 100644 --- a/test/evaluation_context_test.cpp +++ b/test/evaluation_context_test.cpp @@ -33,7 +33,7 @@ TEST_F(EvaluationContextTest, DefaultBuilderCreatesEmptyContext) { TEST_F(EvaluationContextTest, BuilderSetsTargetingKey) { std::string expected_key = "user-12345"; EvaluationContext ctx = - EvaluationContext::Builder().withTargetingKey(expected_key).build(); + EvaluationContext::Builder().WithTargetingKey(expected_key).build(); auto key = ctx.GetTargetingKey(); ASSERT_TRUE(key.has_value()); @@ -43,10 +43,10 @@ TEST_F(EvaluationContextTest, BuilderSetsTargetingKey) { // Test setting and retrieving various attribute types. TEST_F(EvaluationContextTest, BuilderSetsAttributesOfVariousTypes) { EvaluationContext ctx = EvaluationContext::Builder() - .withAttribute("str_attr", std::string("test")) - .withAttribute("int_attr", 42) - .withAttribute("bool_attr", true) - .withAttribute("double_attr", 3.14) + .WithAttribute("str_attr", std::string("test")) + .WithAttribute("int_attr", 42) + .WithAttribute("bool_attr", true) + .WithAttribute("double_attr", 3.14) .build(); const auto& attrs = ctx.GetAttributes(); @@ -72,7 +72,7 @@ TEST_F(EvaluationContextTest, BuilderSetsAttributesOfVariousTypes) { // Test behavior when requesting a non-existent attribute. TEST_F(EvaluationContextTest, GetValueReturnsNullForMissingKey) { EvaluationContext ctx = - EvaluationContext::Builder().withAttribute("exists", 1).build(); + EvaluationContext::Builder().WithAttribute("exists", 1).build(); EXPECT_NE(ctx.GetValue("exists"), nullptr); EXPECT_EQ(ctx.GetValue("does_not_exist"), nullptr); @@ -82,8 +82,8 @@ TEST_F(EvaluationContextTest, GetValueReturnsNullForMissingKey) { // within the same builder chain. TEST_F(EvaluationContextTest, BuilderOverwritesDuplicateKeys) { EvaluationContext ctx = EvaluationContext::Builder() - .withAttribute("key", 100) - .withAttribute("key", 200) + .WithAttribute("key", 100) + .WithAttribute("key", 200) .build(); const std::any* val = ctx.GetValue("key"); @@ -94,16 +94,16 @@ TEST_F(EvaluationContextTest, BuilderOverwritesDuplicateKeys) { // Test merging attributes with precedence. TEST_F(EvaluationContextTest, MergeAttributesWithPrecedence) { EvaluationContext ctx1 = EvaluationContext::Builder() - .withAttribute("common", 1) - .withAttribute("ctx1", std::string("A")) + .WithAttribute("common", 1) + .WithAttribute("ctx1", std::string("A")) .build(); EvaluationContext ctx2 = EvaluationContext::Builder() - .withAttribute("common", 2) - .withAttribute("ctx2", std::string("B")) + .WithAttribute("common", 2) + .WithAttribute("ctx2", std::string("B")) .build(); - EvaluationContext merged = EvaluationContext::merge({&ctx1, &ctx2}); + EvaluationContext merged = EvaluationContext::Merge({&ctx1, &ctx2}); EXPECT_EQ(merged.GetAttributes().size(), 3); @@ -117,38 +117,38 @@ TEST_F(EvaluationContextTest, MergeAttributesWithPrecedence) { TEST_F(EvaluationContextTest, MergeTargetingKeyWithPrecedence) { EvaluationContext ctx_no_key = EvaluationContext::Builder().build(); EvaluationContext ctx_key_a = - EvaluationContext::Builder().withTargetingKey("KeyA").build(); + EvaluationContext::Builder().WithTargetingKey("KeyA").build(); EvaluationContext ctx_key_b = - EvaluationContext::Builder().withTargetingKey("KeyB").build(); + EvaluationContext::Builder().WithTargetingKey("KeyB").build(); - EvaluationContext res1 = EvaluationContext::merge({&ctx_key_a, &ctx_key_b}); + EvaluationContext res1 = EvaluationContext::Merge({&ctx_key_a, &ctx_key_b}); EXPECT_EQ(res1.GetTargetingKey().value(), "KeyB"); EvaluationContext res2 = - EvaluationContext::merge({&ctx_key_a, &ctx_no_key, &ctx_no_key}); + EvaluationContext::Merge({&ctx_key_a, &ctx_no_key, &ctx_no_key}); EXPECT_EQ(res2.GetTargetingKey().value(), "KeyA"); EvaluationContext res3 = - EvaluationContext::merge({&ctx_no_key, &ctx_key_b, &ctx_no_key}); + EvaluationContext::Merge({&ctx_no_key, &ctx_key_b, &ctx_no_key}); EXPECT_EQ(res3.GetTargetingKey().value(), "KeyB"); } // Test Merging: Complex scenario with attributes and keys. TEST_F(EvaluationContextTest, MergeComplexScenario) { EvaluationContext base = EvaluationContext::Builder() - .withTargetingKey("base-user") - .withAttribute("env", std::string("prod")) - .withAttribute("region", std::string("us-east")) + .WithTargetingKey("base-user") + .WithAttribute("env", std::string("prod")) + .WithAttribute("region", std::string("us-east")) .build(); EvaluationContext request = EvaluationContext::Builder() - .withTargetingKey("req-user") - .withAttribute("region", std::string("us-west")) - .withAttribute("request_id", 123) + .WithTargetingKey("req-user") + .WithAttribute("region", std::string("us-west")) + .WithAttribute("request_id", 123) .build(); - EvaluationContext merged = EvaluationContext::merge({&base, &request}); + EvaluationContext merged = EvaluationContext::Merge({&base, &request}); EXPECT_EQ(merged.GetTargetingKey().value(), "req-user"); @@ -160,9 +160,9 @@ TEST_F(EvaluationContextTest, MergeComplexScenario) { // The merged context should only reflect non-null inputs. TEST_F(EvaluationContextTest, MergeIgnoresNullPointers) { EvaluationContext ctx = - EvaluationContext::Builder().withTargetingKey("valid").build(); + EvaluationContext::Builder().WithTargetingKey("valid").build(); - EvaluationContext merged = EvaluationContext::merge({nullptr, &ctx, nullptr}); + EvaluationContext merged = EvaluationContext::Merge({nullptr, &ctx, nullptr}); EXPECT_EQ(merged.GetTargetingKey().value(), "valid"); } @@ -170,7 +170,7 @@ TEST_F(EvaluationContextTest, MergeIgnoresNullPointers) { // Test that string literals and std::string are stored and retrieved correctly. TEST_F(EvaluationContextTest, StoresStringCorrectly) { EvaluationContext ctx_char = - EvaluationContext::Builder().withAttribute("k", "v").build(); + EvaluationContext::Builder().WithAttribute("k", "v").build(); const std::any* val_char = ctx_char.GetValue("k"); // Verify it is stored as std::string, not const char* @@ -179,8 +179,7 @@ TEST_F(EvaluationContextTest, StoresStringCorrectly) { EXPECT_EQ(std::any_cast(*val_char), "v"); EvaluationContext ctx_str = - EvaluationContext::Builder().withAttribute("k", std::string("v")).build(); - + EvaluationContext::Builder().WithAttribute("k", std::string("v")).build(); const std::any* val_str = ctx_str.GetValue("k"); EXPECT_EQ(val_str->type(), typeid(std::string)); EXPECT_EQ(std::any_cast(*val_str), "v");