From b9548e35f4649cc64fcf5c394249d31ba3120960 Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Tue, 2 Dec 2025 04:43:24 -0600 Subject: [PATCH 1/4] feat: error feedback for config response parsing --- Makefile | 6 +- README.md | 114 +++++------------- examples/assignment_details.cpp | 15 ++- examples/bandits.cpp | 15 ++- examples/flag_assignments.cpp | 15 ++- src/config_response.cpp | 46 +++++-- src/config_response.hpp | 5 +- .../test_bandit_evaluation.cpp | 20 ++- .../test_flag_evaluation.cpp | 14 ++- .../test_flag_evaluation_details.cpp | 14 ++- .../test_flag_performance.cpp | 14 ++- test/test_allocation_evaluation_details.cpp | 16 ++- test/test_assignment_details.cpp | 16 ++- test/test_bandit_action_details.cpp | 16 ++- test/test_config_response_json.cpp | 36 ++++-- test/test_configuration.cpp | 9 +- test/test_serialized_json_assignment.cpp | 14 ++- 17 files changed, 240 insertions(+), 145 deletions(-) diff --git a/Makefile b/Makefile index 7e7c621..76867ec 100644 --- a/Makefile +++ b/Makefile @@ -91,17 +91,17 @@ examples: .PHONY: run-bandits run-bandits: examples @echo "Running bandits example..." - @$(BUILD_DIR)/bandits + @cd examples && ../$(BUILD_DIR)/bandits .PHONY: run-flags run-flags: examples @echo "Running flag_assignments example..." - @$(BUILD_DIR)/flag_assignments + @cd examples && ../$(BUILD_DIR)/flag_assignments .PHONY: run-assignment-details run-assignment-details: examples @echo "Running assignment_details example..." - @$(BUILD_DIR)/assignment_details + @cd examples && ../$(BUILD_DIR)/assignment_details # Clean build artifacts .PHONY: clean diff --git a/README.md b/README.md index 8ab5c95..9538c12 100644 --- a/README.md +++ b/README.md @@ -124,15 +124,8 @@ Other dependencies (nlohmann/json, semver, etc.) are vendored and require no ins The Eppo SDK requires configuration data containing your feature flags. This SDK is designed for offline use, so you'll load configuration directly rather than using SDK keys or polling. ```cpp -#include #include "client.hpp" -// Parse configuration from a JSON string -eppoclient::ConfigResponse parseConfiguration(const std::string& configJson) { - nlohmann::json j = nlohmann::json::parse(configJson); - return j; -} - // Your configuration as a JSON string std::string configJson = R"({ "flags": { @@ -143,9 +136,16 @@ std::string configJson = R"({ } })"; +// Parse configuration from JSON string +std::string error; +eppoclient::ConfigResponse config = eppoclient::parseConfigResponse(configJson, error); +if (!error.empty()) { + std::cerr << "Configuration parsing error: " << error << std::endl; + return 1; +} + // Create and initialize the configuration store eppoclient::ConfigurationStore configStore; -eppoclient::ConfigResponse config = parseConfiguration(configJson); configStore.setConfiguration(eppoclient::Configuration(config)); // Create the client (configStore must outlive client) @@ -267,7 +267,12 @@ int main() { // Initialize configuration eppoclient::ConfigurationStore configStore; std::string configJson = "..."; // Your JSON config string - eppoclient::ConfigResponse config = parseConfiguration(configJson); + std::string error; + eppoclient::ConfigResponse config = eppoclient::parseConfigResponse(configJson, error); + if (!error.empty()) { + std::cerr << "Configuration parsing error: " << error << std::endl; + return 1; + } configStore.setConfiguration(eppoclient::Configuration(config)); // Create loggers @@ -320,19 +325,21 @@ To use bandits, you need to load both flag configuration and bandit models: #include #include "client.hpp" -// Parse bandit models from a JSON string -eppoclient::BanditResponse parseBanditModels(const std::string& modelsJson) { - nlohmann::json j = nlohmann::json::parse(modelsJson); - return j; -} - // Your configuration and bandit models as JSON strings std::string flagConfigJson = "..."; // Your flag config JSON std::string banditModelsJson = "..."; // Your bandit models JSON -// Initialize with both flags and bandit models -eppoclient::ConfigResponse flagConfig = parseConfiguration(flagConfigJson); -eppoclient::BanditResponse banditModels = parseBanditModels(banditModelsJson); +// Parse flag configuration +std::string configError; +eppoclient::ConfigResponse flagConfig = eppoclient::parseConfigResponse(flagConfigJson, configError); +if (!configError.empty()) { + std::cerr << "Configuration parsing error: " << configError << std::endl; + return 1; +} + +// Parse bandit models (using nlohmann::json deserialization) +nlohmann::json banditJson = nlohmann::json::parse(banditModelsJson); +eppoclient::BanditResponse banditModels = banditJson; eppoclient::ConfigurationStore configStore; configStore.setConfiguration(eppoclient::Configuration(flagConfig, banditModels)); @@ -421,69 +428,6 @@ if (result.action.has_value()) { } ``` -### Complete Bandit Example - -Here's a complete example from `examples/bandits.cpp` showing bandit-powered car recommendations: - -```cpp -#include -#include -#include "client.hpp" - -int main() { - // Load configuration - eppoclient::ConfigurationStore configStore; - std::string flagConfigJson = "..."; // Your flag config JSON - std::string banditModelsJson = "..."; // Your bandit models JSON - eppoclient::ConfigResponse flagConfig = parseConfiguration(flagConfigJson); - eppoclient::BanditResponse banditModels = parseBanditModels(banditModelsJson); - configStore.setConfiguration(eppoclient::Configuration(flagConfig, banditModels)); - - // Create loggers - auto assignmentLogger = std::make_shared(); - auto banditLogger = std::make_shared(); - auto applicationLogger = std::make_shared(); - - // Create client - eppoclient::EppoClient client( - configStore, - assignmentLogger, - banditLogger, - applicationLogger - ); - - // Define subject attributes (user context) - eppoclient::ContextAttributes subjectAttributes; - // Add any relevant user attributes here - - // Define available car actions with their attributes - std::map actions; - - eppoclient::ContextAttributes toyota; - toyota.numericAttributes["speed"] = 120.0; - actions["toyota"] = toyota; - - eppoclient::ContextAttributes honda; - honda.numericAttributes["speed"] = 115.0; - actions["honda"] = honda; - - // Get bandit recommendation - eppoclient::BanditResult result = client.getBanditAction( - "car_bandit_flag", - "user-abc123", - subjectAttributes, - actions, - "car_bandit" - ); - - if (result.action.has_value()) { - std::cout << "Recommended car: " << result.action.value() << std::endl; - } - - return 0; -} -``` - ## Error Handling The Eppo SDK is built with **`-fno-exceptions`** and does not use exceptions internally. When errors occur during flag evaluation (such as missing flags, invalid parameters, or type mismatches), the SDK: @@ -637,7 +581,13 @@ Always ensure these preconditions are met to avoid assertion failures. int main() { // Initialize client with application logger eppoclient::ConfigurationStore configStore; - eppoclient::ConfigResponse config = parseConfiguration(configJson); + std::string configJson = "..."; // Your JSON config string + std::string error; + eppoclient::ConfigResponse config = eppoclient::parseConfigResponse(configJson, error); + if (!error.empty()) { + std::cerr << "Configuration parsing error: " << error << std::endl; + return 1; + } configStore.setConfiguration(eppoclient::Configuration(config)); auto applicationLogger = std::make_shared(); diff --git a/examples/assignment_details.cpp b/examples/assignment_details.cpp index 23d354b..7e2bbe8 100644 --- a/examples/assignment_details.cpp +++ b/examples/assignment_details.cpp @@ -76,10 +76,19 @@ bool loadFlagsConfiguration(const std::string& filepath, eppoclient::ConfigRespo return false; } - nlohmann::json j; - file >> j; + // Read entire file content into a string + std::string configJson((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse configuration using parseConfigResponse + std::string error; + response = eppoclient::parseConfigResponse(configJson, error); + + if (!error.empty()) { + std::cerr << "Failed to parse configuration: " << error << std::endl; + return false; + } - response = j; return true; } diff --git a/examples/bandits.cpp b/examples/bandits.cpp index 5b649f6..b461360 100644 --- a/examples/bandits.cpp +++ b/examples/bandits.cpp @@ -126,10 +126,19 @@ bool loadFlagsConfiguration(const std::string& filepath, eppoclient::ConfigRespo return false; } - nlohmann::json j; - file >> j; + // Read entire file content into a string + std::string configJson((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse configuration using parseConfigResponse + std::string error; + response = eppoclient::parseConfigResponse(configJson, error); + + if (!error.empty()) { + std::cerr << "Failed to parse configuration: " << error << std::endl; + return false; + } - response = j; return true; } diff --git a/examples/flag_assignments.cpp b/examples/flag_assignments.cpp index 59b424b..e4bdefe 100644 --- a/examples/flag_assignments.cpp +++ b/examples/flag_assignments.cpp @@ -74,10 +74,19 @@ bool loadFlagsConfiguration(const std::string& filepath, eppoclient::ConfigRespo return false; } - nlohmann::json j; - file >> j; + // Read entire file content into a string + std::string configJson((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse configuration using parseConfigResponse + std::string error; + response = eppoclient::parseConfigResponse(configJson, error); + + if (!error.empty()) { + std::cerr << "Failed to parse configuration: " << error << std::endl; + return false; + } - response = j; return true; } diff --git a/src/config_response.cpp b/src/config_response.cpp index 08efc03..bf9ba16 100644 --- a/src/config_response.cpp +++ b/src/config_response.cpp @@ -1,8 +1,5 @@ #include "config_response.hpp" -#include #include -#include -#include #include "json_utils.hpp" #include "rules.hpp" #include "time_utils.hpp" @@ -620,20 +617,31 @@ void to_json(nlohmann::json& j, const ConfigResponse& cr) { j = nlohmann::json{{"flags", cr.flags}, {"bandits", cr.bandits}}; } -void from_json(const nlohmann::json& j, ConfigResponse& cr) { - cr.flags.clear(); - cr.bandits.clear(); +ConfigResponse parseConfigResponse(const std::string& configJson, std::string& error) { + error.clear(); + ConfigResponse response; - // Parse flags - each flag independently, skip invalid ones + // Use the non-throwing version of parse (returns discarded value on error) + nlohmann::json j = nlohmann::json::parse(configJson, nullptr, false); + + if (j.is_discarded()) { + error = "Failed to parse JSON configuration string"; + return ConfigResponse(); // Return empty ConfigResponse on error + } + + std::vector allErrors; + + // Parse flags - each flag independently, collect errors if (j.contains("flags") && j["flags"].is_object()) { for (auto& [flagKey, flagJson] : j["flags"].items()) { std::string parseError; auto flag = internal::parseFlagConfiguration(flagJson, parseError); if (flag) { - cr.flags[flagKey] = *flag; + response.flags[flagKey] = *flag; + } else if (!parseError.empty()) { + allErrors.push_back("Flag '" + flagKey + "': " + parseError); } - // If parsing failed, flag is simply not included in the map } } @@ -643,20 +651,36 @@ void from_json(const nlohmann::json& j, ConfigResponse& cr) { // Each bandit entry is an array of BanditVariation if (banditJsonArray.is_array()) { std::vector variations; + int varIndex = 0; for (const auto& varJson : banditJsonArray) { std::string parseError; auto banditVar = internal::parseBanditVariation(varJson, parseError); if (banditVar) { variations.push_back(*banditVar); + } else if (!parseError.empty()) { + allErrors.push_back("Bandit '" + banditKey + "' variation[" + + std::to_string(varIndex) + "]: " + parseError); } - // If parsing failed, variation is simply not included + varIndex++; } if (!variations.empty()) { - cr.bandits[banditKey] = variations; + response.bandits[banditKey] = variations; } + } else { + allErrors.push_back("Bandit '" + banditKey + "': Expected array of variations"); } } } + + // Consolidate all errors into a single error message + if (!allErrors.empty()) { + error = "Configuration parsing encountered errors:\n"; + for (const auto& err : allErrors) { + error += " - " + err + "\n"; + } + } + + return response; } } // namespace eppoclient diff --git a/src/config_response.hpp b/src/config_response.hpp index 341a0a2..0ebe001 100644 --- a/src/config_response.hpp +++ b/src/config_response.hpp @@ -155,7 +155,10 @@ struct ConfigResponse { // serialization/deserialization for the nlohmann::json library void to_json(nlohmann::json& j, const ConfigResponse& cr); -void from_json(const nlohmann::json& j, ConfigResponse& cr); + +// Parse configuration from a JSON string +// Returns a ConfigResponse. If parsing fails, error will contain the error message. +ConfigResponse parseConfigResponse(const std::string& configJson, std::string& error); // Internal namespace for implementation details not covered by semver namespace internal { diff --git a/test/shared_test_cases/test_bandit_evaluation.cpp b/test/shared_test_cases/test_bandit_evaluation.cpp index 66c9775..52b8a6e 100644 --- a/test/shared_test_cases/test_bandit_evaluation.cpp +++ b/test/shared_test_cases/test_bandit_evaluation.cpp @@ -86,10 +86,18 @@ ConfigResponse loadBanditFlagsConfiguration(const std::string& filepath) { throw std::runtime_error("Failed to open bandit flags configuration file: " + filepath); } - json j; - file >> j; + // Read entire file content into a string + std::string configJson((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse configuration using parseConfigResponse + std::string error; + ConfigResponse response = parseConfigResponse(configJson, error); + + if (!error.empty()) { + throw std::runtime_error("Failed to parse configuration: " + error); + } - ConfigResponse response = j; return response; } @@ -222,7 +230,11 @@ TEST_CASE("UFC Bandit Test Cases - Bandit Action Selection", "[ufc][bandits]") { } // Create ConfigResponse from the flags JSON - ConfigResponse configResponse = configJson; + std::string error; + ConfigResponse configResponse = parseConfigResponse(configJson.dump(), error); + if (!error.empty()) { + throw std::runtime_error("Failed to parse configuration: " + error); + } // Create BanditResponse from the models JSON BanditResponse banditResponse = modelsJson; diff --git a/test/shared_test_cases/test_flag_evaluation.cpp b/test/shared_test_cases/test_flag_evaluation.cpp index 454fcc5..f012332 100644 --- a/test/shared_test_cases/test_flag_evaluation.cpp +++ b/test/shared_test_cases/test_flag_evaluation.cpp @@ -35,10 +35,18 @@ ConfigResponse loadFlagsConfiguration(const std::string& filepath) { throw std::runtime_error("Failed to open flags configuration file: " + filepath); } - json j; - file >> j; + // Read entire file content into a string + std::string configJson((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse configuration using parseConfigResponse + std::string error; + ConfigResponse response = parseConfigResponse(configJson, error); + + if (!error.empty()) { + throw std::runtime_error("Failed to parse configuration: " + error); + } - ConfigResponse response = j; return response; } diff --git a/test/shared_test_cases/test_flag_evaluation_details.cpp b/test/shared_test_cases/test_flag_evaluation_details.cpp index befae82..cca0cbb 100644 --- a/test/shared_test_cases/test_flag_evaluation_details.cpp +++ b/test/shared_test_cases/test_flag_evaluation_details.cpp @@ -46,10 +46,18 @@ static ConfigResponse loadFlagsConfiguration(const std::string& filepath) { throw std::runtime_error("Failed to open flags configuration file: " + filepath); } - json j; - file >> j; + // Read entire file content into a string + std::string configJson((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse configuration using parseConfigResponse + std::string error; + ConfigResponse response = parseConfigResponse(configJson, error); + + if (!error.empty()) { + throw std::runtime_error("Failed to parse configuration: " + error); + } - ConfigResponse response = j; return response; } diff --git a/test/shared_test_cases/test_flag_performance.cpp b/test/shared_test_cases/test_flag_performance.cpp index 18ccfa7..7ca53cd 100644 --- a/test/shared_test_cases/test_flag_performance.cpp +++ b/test/shared_test_cases/test_flag_performance.cpp @@ -48,10 +48,18 @@ static ConfigResponse loadFlagsConfiguration(const std::string& filepath) { throw std::runtime_error("Failed to open flags configuration file: " + filepath); } - json j; - file >> j; + // Read entire file content into a string + std::string configJson((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse configuration using parseConfigResponse + std::string error; + ConfigResponse response = parseConfigResponse(configJson, error); + + if (!error.empty()) { + throw std::runtime_error("Failed to parse configuration: " + error); + } - ConfigResponse response = j; return response; } diff --git a/test/test_allocation_evaluation_details.cpp b/test/test_allocation_evaluation_details.cpp index d1f8ab2..4b99584 100644 --- a/test/test_allocation_evaluation_details.cpp +++ b/test/test_allocation_evaluation_details.cpp @@ -16,9 +16,19 @@ ConfigResponse loadFlagsConfiguration(const std::string& filepath) { throw std::runtime_error("Failed to open flags configuration file: " + filepath); } - json j; - file >> j; - return j.get(); + // Read entire file content into a string + std::string configJson((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse configuration using parseConfigResponse + std::string error; + ConfigResponse response = parseConfigResponse(configJson, error); + + if (!error.empty()) { + throw std::runtime_error("Failed to parse configuration: " + error); + } + + return response; } } // namespace diff --git a/test/test_assignment_details.cpp b/test/test_assignment_details.cpp index c606d92..7f510b8 100644 --- a/test/test_assignment_details.cpp +++ b/test/test_assignment_details.cpp @@ -50,9 +50,19 @@ ConfigResponse loadFlagsConfiguration(const std::string& filepath) { throw std::runtime_error("Failed to open flags configuration file: " + filepath); } - json j; - file >> j; - return j.get(); + // Read entire file content into a string + std::string configJson((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse configuration using parseConfigResponse + std::string error; + ConfigResponse response = parseConfigResponse(configJson, error); + + if (!error.empty()) { + throw std::runtime_error("Failed to parse configuration: " + error); + } + + return response; } } // namespace diff --git a/test/test_bandit_action_details.cpp b/test/test_bandit_action_details.cpp index bf8266d..d172a50 100644 --- a/test/test_bandit_action_details.cpp +++ b/test/test_bandit_action_details.cpp @@ -50,9 +50,19 @@ ConfigResponse loadFlagsConfiguration(const std::string& filepath) { throw std::runtime_error("Failed to open flags configuration file: " + filepath); } - json j; - file >> j; - return j.get(); + // Read entire file content into a string + std::string configJson((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse configuration using parseConfigResponse + std::string error; + ConfigResponse response = parseConfigResponse(configJson, error); + + if (!error.empty()) { + throw std::runtime_error("Failed to parse configuration: " + error); + } + + return response; } } // namespace diff --git a/test/test_config_response_json.cpp b/test/test_config_response_json.cpp index 265b7a0..0846e3a 100644 --- a/test/test_config_response_json.cpp +++ b/test/test_config_response_json.cpp @@ -165,7 +165,9 @@ TEST_CASE("ConfigResponse deserialization - empty config", "[config_response][js json j = json::object(); j["flags"] = json::object(); - ConfigResponse response = j; + std::string error; + ConfigResponse response = parseConfigResponse(j.dump(), error); + REQUIRE(error.empty()); REQUIRE(response.flags.empty()); } @@ -189,7 +191,9 @@ TEST_CASE("ConfigResponse deserialization - single simple flag", "[config_respon } })"_json; - ConfigResponse response = j; + std::string error; + ConfigResponse response = parseConfigResponse(j.dump(), error); + REQUIRE(error.empty()); REQUIRE(response.flags.size() == 1); REQUIRE(response.flags.count("test-flag") == 1); @@ -247,7 +251,9 @@ TEST_CASE("ConfigResponse deserialization - all variation types", "[config_respo } })"_json; - ConfigResponse response = j; + std::string error; + ConfigResponse response = parseConfigResponse(j.dump(), error); + REQUIRE(error.empty()); REQUIRE(response.flags.size() == 5); REQUIRE(response.flags["string-flag"].variationType == VariationType::STRING); @@ -303,7 +309,9 @@ TEST_CASE("ConfigResponse deserialization - flag with allocations", "[config_res } })"_json; - ConfigResponse response = j; + std::string error; + ConfigResponse response = parseConfigResponse(j.dump(), error); + REQUIRE(error.empty()); REQUIRE(response.flags.size() == 1); REQUIRE(response.flags["allocated-flag"].allocations.size() == 1); @@ -325,7 +333,9 @@ TEST_CASE("ConfigResponse deserialization - from real UFC file", "[config_respon json j; file >> j; - ConfigResponse response = j; + std::string error; + ConfigResponse response = parseConfigResponse(j.dump(), error); + REQUIRE(error.empty()); // Verify some expected flags from the test file REQUIRE(response.flags.size() > 0); @@ -363,7 +373,9 @@ TEST_CASE("ConfigResponse round-trip - simple flag", "[config_response][json]") json j = original; // Deserialize - ConfigResponse deserialized = j; + std::string error; + ConfigResponse deserialized = parseConfigResponse(j.dump(), error); + REQUIRE(error.empty()); // Verify REQUIRE(deserialized.flags.size() == 1); @@ -446,7 +458,9 @@ TEST_CASE("ConfigResponse round-trip - complex flag with allocations", "[config_ json j = original; // Deserialize - ConfigResponse deserialized = j; + std::string error; + ConfigResponse deserialized = parseConfigResponse(j.dump(), error); + REQUIRE(error.empty()); // Verify structure is preserved REQUIRE(deserialized.flags.size() == 1); @@ -488,7 +502,9 @@ TEST_CASE("ConfigResponse precompute after deserialization", "[config_response][ } })"_json; - ConfigResponse response = j; + std::string error; + ConfigResponse response = parseConfigResponse(j.dump(), error); + REQUIRE(error.empty()); // Precompute should not crash REQUIRE_NOTHROW(response.precompute()); @@ -543,7 +559,9 @@ TEST_CASE("ConfigResponse usage example - deserialize from file", "[config_respo nlohmann::json j; file >> j; - ConfigResponse response = j; + std::string error; + ConfigResponse response = parseConfigResponse(j.dump(), error); + REQUIRE(error.empty()); // Verify the response is valid REQUIRE(response.flags.size() > 0); diff --git a/test/test_configuration.cpp b/test/test_configuration.cpp index 28e391c..5a1801a 100644 --- a/test/test_configuration.cpp +++ b/test/test_configuration.cpp @@ -66,11 +66,10 @@ TEST_CASE("ConfigResponse with bandits", "[configuration]") { } })"; - // Parse the JSON - json j = json::parse(jsonStr); - - // Deserialize to ConfigResponse - ConfigResponse response = j; + // Parse configuration using parseConfigResponse + std::string error; + ConfigResponse response = parseConfigResponse(jsonStr, error); + REQUIRE(error.empty()); // Verify structure CHECK(response.flags.size() == 1); diff --git a/test/test_serialized_json_assignment.cpp b/test/test_serialized_json_assignment.cpp index eb3d65c..3948228 100644 --- a/test/test_serialized_json_assignment.cpp +++ b/test/test_serialized_json_assignment.cpp @@ -39,10 +39,18 @@ ConfigResponse loadFlagsConfiguration(const std::string& filepath) { throw std::runtime_error("Failed to open flags configuration file: " + filepath); } - json j; - file >> j; + // Read entire file content into a string + std::string configJson((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse configuration using parseConfigResponse + std::string error; + ConfigResponse response = parseConfigResponse(configJson, error); + + if (!error.empty()) { + throw std::runtime_error("Failed to parse configuration: " + error); + } - ConfigResponse response = j; return response; } From 69f031966a3c09a38274217d874f6c009744c37e Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Tue, 2 Dec 2025 05:18:44 -0600 Subject: [PATCH 2/4] feat: error feedback for bandit response parsing --- CHANGELOG.md | 4 ++ README.md | 11 +++-- examples/bandits.cpp | 12 +++-- src/bandit_model.cpp | 44 ++++++++++++++----- src/bandit_model.hpp | 5 ++- .../test_bandit_evaluation.cpp | 15 +++++-- test/test_bandit_model.cpp | 11 ++--- test/test_configuration.cpp | 7 ++- 8 files changed, 77 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea7cdd8..4ca1beb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Eliminates expensive configuration copies on every flag evaluation - **BREAKING**: `Configuration` constructors now take parameters by value for better performance - `ConfigurationStore` now uses atomic operations instead of mutex internally for better performance +- **BREAKING**: Replaced `from_json(const nlohmann::json&, BanditResponse&)` with `parseBanditResponse(const std::string&, std::string&)` + - New function takes JSON string and error reference parameter for consistent error tracking + - Returns parsed `BanditResponse` with errors reported via the error parameter + - Matches the same pattern as `parseConfigResponse()` for configuration parsing ### Removed diff --git a/README.md b/README.md index 9538c12..c6a4182 100644 --- a/README.md +++ b/README.md @@ -322,7 +322,6 @@ Eppo's contextual bandits allow you to dynamically select the best variant based To use bandits, you need to load both flag configuration and bandit models: ```cpp -#include #include "client.hpp" // Your configuration and bandit models as JSON strings @@ -337,9 +336,13 @@ if (!configError.empty()) { return 1; } -// Parse bandit models (using nlohmann::json deserialization) -nlohmann::json banditJson = nlohmann::json::parse(banditModelsJson); -eppoclient::BanditResponse banditModels = banditJson; +// Parse bandit models +std::string banditError; +eppoclient::BanditResponse banditModels = eppoclient::parseBanditResponse(banditModelsJson, banditError); +if (!banditError.empty()) { + std::cerr << "Bandit models parsing error: " << banditError << std::endl; + return 1; +} eppoclient::ConfigurationStore configStore; configStore.setConfiguration(eppoclient::Configuration(flagConfig, banditModels)); diff --git a/examples/bandits.cpp b/examples/bandits.cpp index b461360..45363a5 100644 --- a/examples/bandits.cpp +++ b/examples/bandits.cpp @@ -150,10 +150,16 @@ bool loadBanditModels(const std::string& filepath, eppoclient::BanditResponse& r return false; } - nlohmann::json j; - file >> j; + std::string jsonStr((std::istreambuf_iterator(file)), std::istreambuf_iterator()); + + std::string error; + response = eppoclient::parseBanditResponse(jsonStr, error); + + if (!error.empty()) { + std::cerr << "Error parsing bandit models: " << error << std::endl; + return false; + } - response = j; return true; } diff --git a/src/bandit_model.cpp b/src/bandit_model.cpp index e0eca63..f89b0c3 100644 --- a/src/bandit_model.cpp +++ b/src/bandit_model.cpp @@ -249,30 +249,52 @@ void to_json(nlohmann::json& j, const BanditResponse& br) { j = nlohmann::json{{"bandits", br.bandits}, {"updatedAt", formatISOTimestamp(br.updatedAt)}}; } -void from_json(const nlohmann::json& j, BanditResponse& br) { - br.bandits.clear(); +BanditResponse parseBanditResponse(const std::string& banditJson, std::string& error) { + error.clear(); + BanditResponse response; - // Parse bandits - each bandit independently, skip invalid ones + // Use the non-throwing version of parse (returns discarded value on error) + nlohmann::json j = nlohmann::json::parse(banditJson, nullptr, false); + + if (j.is_discarded()) { + error = "Failed to parse JSON bandit response string"; + return BanditResponse(); // Return empty BanditResponse on error + } + + std::vector allErrors; + + // Parse bandits - each bandit independently, collect errors if (j.contains("bandits") && j["bandits"].is_object()) { for (auto& [banditKey, banditJson] : j["bandits"].items()) { std::string parseError; auto bandit = internal::parseBanditConfiguration(banditJson, parseError); if (bandit) { - br.bandits[banditKey] = *bandit; + response.bandits[banditKey] = *bandit; + } else if (!parseError.empty()) { + allErrors.push_back("Bandit '" + banditKey + "': " + parseError); } - // If parsing failed, bandit is simply not included in the map } } + // Parse updatedAt field if (j.contains("updatedAt") && j["updatedAt"].is_string()) { - std::string error; - br.updatedAt = parseISOTimestamp(j["updatedAt"].get_ref(), error); - // TODO: log error - // if (!error.empty()) { - // logger.error("BanditResponse: Invalid updatedAt: " + error); - // } + std::string parseError; + response.updatedAt = parseISOTimestamp(j["updatedAt"].get_ref(), parseError); + if (!parseError.empty()) { + allErrors.push_back("Invalid updatedAt: " + parseError); + } } + + // Consolidate all errors into a single error message + if (!allErrors.empty()) { + error = "Bandit response parsing encountered errors:\n"; + for (const auto& err : allErrors) { + error += " - " + err + "\n"; + } + } + + return response; } // BanditVariation serialization diff --git a/src/bandit_model.hpp b/src/bandit_model.hpp index f1bf630..3621b07 100644 --- a/src/bandit_model.hpp +++ b/src/bandit_model.hpp @@ -99,7 +99,10 @@ struct BanditResponse { }; void to_json(nlohmann::json& j, const BanditResponse& br); -void from_json(const nlohmann::json& j, BanditResponse& br); + +// Parse bandit response from a JSON string +// Returns a BanditResponse. If parsing fails, error will contain the error message. +BanditResponse parseBanditResponse(const std::string& banditJson, std::string& error); /** * Associates a bandit with a specific flag variation. diff --git a/test/shared_test_cases/test_bandit_evaluation.cpp b/test/shared_test_cases/test_bandit_evaluation.cpp index 52b8a6e..84f1ae5 100644 --- a/test/shared_test_cases/test_bandit_evaluation.cpp +++ b/test/shared_test_cases/test_bandit_evaluation.cpp @@ -108,10 +108,13 @@ BanditResponse loadBanditModelsConfiguration(const std::string& filepath) { throw std::runtime_error("Failed to open bandit models configuration file: " + filepath); } - json j; - file >> j; + std::string jsonStr((std::istreambuf_iterator(file)), std::istreambuf_iterator()); - BanditResponse response = j; + std::string error; + BanditResponse response = parseBanditResponse(jsonStr, error); + if (!error.empty()) { + throw std::runtime_error("Failed to parse bandit models configuration: " + error); + } return response; } @@ -237,7 +240,11 @@ TEST_CASE("UFC Bandit Test Cases - Bandit Action Selection", "[ufc][bandits]") { } // Create BanditResponse from the models JSON - BanditResponse banditResponse = modelsJson; + std::string banditError; + BanditResponse banditResponse = parseBanditResponse(modelsJson.dump(), banditError); + if (!banditError.empty()) { + throw std::runtime_error("Failed to parse bandit response: " + banditError); + } // Create configuration with both flags and bandit models Configuration combinedConfig(configResponse, banditResponse); diff --git a/test/test_bandit_model.cpp b/test/test_bandit_model.cpp index 21d332c..44f4adb 100644 --- a/test/test_bandit_model.cpp +++ b/test/test_bandit_model.cpp @@ -206,7 +206,9 @@ TEST_CASE("BanditResponse serialization", "[bandit_model]") { CHECK(j["bandits"]["bandit1"]["banditKey"] == "bandit1"); // Deserialize from JSON - BanditResponse response2 = j; + std::string error2; + BanditResponse response2 = parseBanditResponse(j.dump(), error2); + CHECK(error2.empty()); CHECK(response2.bandits.size() == 1); CHECK(response2.bandits["bandit1"].banditKey == "bandit1"); CHECK(response2.bandits["bandit1"].modelData.gamma == 0.85); @@ -256,11 +258,10 @@ TEST_CASE("Complete JSON round-trip", "[bandit_model]") { "updatedAt": "2024-01-15T10:30:00Z" })"; - // Parse JSON - json j = json::parse(jsonStr); - // Deserialize to BanditResponse - BanditResponse response = j; + std::string error; + BanditResponse response = parseBanditResponse(jsonStr, error); + CHECK(error.empty()); // Verify structure CHECK(response.bandits.size() == 1); diff --git a/test/test_configuration.cpp b/test/test_configuration.cpp index 5a1801a..43b1fea 100644 --- a/test/test_configuration.cpp +++ b/test/test_configuration.cpp @@ -140,11 +140,10 @@ TEST_CASE("BanditResponse with multiple bandits", "[configuration]") { "updatedAt": "2024-01-15T11:00:00Z" })"; - // Parse JSON - json j = json::parse(jsonStr); - // Deserialize to BanditResponse - BanditResponse response = j; + std::string error; + BanditResponse response = parseBanditResponse(jsonStr, error); + CHECK(error.empty()); // Verify structure CHECK(response.bandits.size() == 2); From ed4134baa3f5bc3b4044510171ce16ec19cb16e8 Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Tue, 2 Dec 2025 05:38:42 -0600 Subject: [PATCH 3/4] feat: parseConfiguration convenience method --- CHANGELOG.md | 8 +- README.md | 35 ++-- examples/assignment_details.cpp | 12 +- examples/bandits.cpp | 58 +++---- examples/flag_assignments.cpp | 12 +- src/bandit_model.cpp | 7 +- src/bandit_model.hpp | 9 +- src/config_response.cpp | 4 + src/config_response.hpp | 7 +- src/configuration.cpp | 30 ++++ src/configuration.hpp | 10 ++ .../test_bandit_evaluation.cpp | 105 ++++-------- .../test_flag_evaluation.cpp | 30 ++-- .../test_flag_evaluation_details.cpp | 15 +- .../test_flag_performance.cpp | 16 +- test/test_allocation_evaluation_details.cpp | 2 +- test/test_assignment_details.cpp | 2 +- test/test_bandit_action_details.cpp | 2 +- test/test_bandit_model.cpp | 4 +- test/test_config_response_json.cpp | 18 +- test/test_configuration.cpp | 161 +++++++++++++++++- test/test_serialized_json_assignment.cpp | 2 +- 22 files changed, 345 insertions(+), 204 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ca1beb..5da9a80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `ConfigurationStore(Configuration config)` - constructor accepting Configuration by value - `setConfiguration(Configuration config)` - setter accepting Configuration by value - Move constructor and move assignment operator for `Configuration` class +- `parseConfiguration()` convenience function for parsing both flag configuration and bandit models in a single call + - Takes flag config JSON, bandit models JSON, and error reference parameter + - Returns a fully constructed `Configuration` object with both parsed configurations + - Simplifies setup for applications using contextual bandits ### Changed @@ -22,10 +26,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Eliminates expensive configuration copies on every flag evaluation - **BREAKING**: `Configuration` constructors now take parameters by value for better performance - `ConfigurationStore` now uses atomic operations instead of mutex internally for better performance -- **BREAKING**: Replaced `from_json(const nlohmann::json&, BanditResponse&)` with `parseBanditResponse(const std::string&, std::string&)` - - New function takes JSON string and error reference parameter for consistent error tracking - - Returns parsed `BanditResponse` with errors reported via the error parameter - - Matches the same pattern as `parseConfigResponse()` for configuration parsing ### Removed diff --git a/README.md b/README.md index c6a4182..5d9fa3f 100644 --- a/README.md +++ b/README.md @@ -138,7 +138,7 @@ std::string configJson = R"({ // Parse configuration from JSON string std::string error; -eppoclient::ConfigResponse config = eppoclient::parseConfigResponse(configJson, error); +eppoclient::Configuration config = eppoclient::parseConfiguration(configJson, error); if (!error.empty()) { std::cerr << "Configuration parsing error: " << error << std::endl; return 1; @@ -146,7 +146,7 @@ if (!error.empty()) { // Create and initialize the configuration store eppoclient::ConfigurationStore configStore; -configStore.setConfiguration(eppoclient::Configuration(config)); +configStore.setConfiguration(config); // Create the client (configStore must outlive client) eppoclient::EppoClient client(configStore); @@ -268,12 +268,12 @@ int main() { eppoclient::ConfigurationStore configStore; std::string configJson = "..."; // Your JSON config string std::string error; - eppoclient::ConfigResponse config = eppoclient::parseConfigResponse(configJson, error); + eppoclient::Configuration config = eppoclient::parseConfiguration(configJson, error); if (!error.empty()) { std::cerr << "Configuration parsing error: " << error << std::endl; return 1; } - configStore.setConfiguration(eppoclient::Configuration(config)); + configStore.setConfiguration(config); // Create loggers auto assignmentLogger = std::make_shared(); @@ -328,24 +328,19 @@ To use bandits, you need to load both flag configuration and bandit models: std::string flagConfigJson = "..."; // Your flag config JSON std::string banditModelsJson = "..."; // Your bandit models JSON -// Parse flag configuration -std::string configError; -eppoclient::ConfigResponse flagConfig = eppoclient::parseConfigResponse(flagConfigJson, configError); -if (!configError.empty()) { - std::cerr << "Configuration parsing error: " << configError << std::endl; - return 1; -} - -// Parse bandit models -std::string banditError; -eppoclient::BanditResponse banditModels = eppoclient::parseBanditResponse(banditModelsJson, banditError); -if (!banditError.empty()) { - std::cerr << "Bandit models parsing error: " << banditError << std::endl; +std::string error; +eppoclient::Configuration config = eppoclient::parseConfiguration( + flagConfigJson, + banditModelsJson, + error +); +if (!error.empty()) { + std::cerr << "Configuration parsing error: " << error << std::endl; return 1; } eppoclient::ConfigurationStore configStore; -configStore.setConfiguration(eppoclient::Configuration(flagConfig, banditModels)); +configStore.setConfiguration(config); // Create bandit logger to track bandit actions class MyBanditLogger : public eppoclient::BanditLogger { @@ -586,12 +581,12 @@ int main() { eppoclient::ConfigurationStore configStore; std::string configJson = "..."; // Your JSON config string std::string error; - eppoclient::ConfigResponse config = eppoclient::parseConfigResponse(configJson, error); + eppoclient::Configuration config = eppoclient::parseConfiguration(configJson, error); if (!error.empty()) { std::cerr << "Configuration parsing error: " << error << std::endl; return 1; } - configStore.setConfiguration(eppoclient::Configuration(config)); + configStore.setConfiguration(config); auto applicationLogger = std::make_shared(); eppoclient::EppoClient client( diff --git a/examples/assignment_details.cpp b/examples/assignment_details.cpp index 7e2bbe8..3c1212e 100644 --- a/examples/assignment_details.cpp +++ b/examples/assignment_details.cpp @@ -69,7 +69,7 @@ class ConsoleApplicationLogger : public eppoclient::ApplicationLogger { }; // Helper function to load flags configuration from JSON file -bool loadFlagsConfiguration(const std::string& filepath, eppoclient::ConfigResponse& response) { +bool loadFlagsConfiguration(const std::string& filepath, eppoclient::Configuration& config) { std::ifstream file(filepath); if (!file.is_open()) { std::cerr << "Failed to open flags configuration file: " << filepath << std::endl; @@ -80,9 +80,9 @@ bool loadFlagsConfiguration(const std::string& filepath, eppoclient::ConfigRespo std::string configJson((std::istreambuf_iterator(file)), std::istreambuf_iterator()); - // Parse configuration using parseConfigResponse + // Parse configuration using parseConfiguration std::string error; - response = eppoclient::parseConfigResponse(configJson, error); + config = eppoclient::parseConfiguration(configJson, error); if (!error.empty()) { std::cerr << "Failed to parse configuration: " << error << std::endl; @@ -95,14 +95,14 @@ bool loadFlagsConfiguration(const std::string& filepath, eppoclient::ConfigRespo int main() { // Load the flags configuration std::cout << "Loading flags configuration..." << std::endl; - eppoclient::ConfigResponse ufc; - if (!loadFlagsConfiguration("config/flags-v1.json", ufc)) { + eppoclient::Configuration config; + if (!loadFlagsConfiguration("config/flags-v1.json", config)) { return 1; } // Create configuration store and set the configuration auto configStore = std::make_shared(); - configStore->setConfiguration(eppoclient::Configuration(ufc)); + configStore->setConfiguration(config); // Create assignment logger and application logger auto assignmentLogger = std::make_shared(); diff --git a/examples/bandits.cpp b/examples/bandits.cpp index 45363a5..ebd4c2e 100644 --- a/examples/bandits.cpp +++ b/examples/bandits.cpp @@ -118,45 +118,33 @@ class ConsoleApplicationLogger : public eppoclient::ApplicationLogger { } }; -// Helper function to load flags configuration from JSON file -bool loadFlagsConfiguration(const std::string& filepath, eppoclient::ConfigResponse& response) { - std::ifstream file(filepath); - if (!file.is_open()) { - std::cerr << "Failed to open flags configuration file: " << filepath << std::endl; +// Helper function to load complete configuration (flags + bandits) from JSON files +bool loadConfiguration(const std::string& flagsFilepath, const std::string& banditsFilepath, + eppoclient::Configuration& config) { + // Read flags configuration file + std::ifstream flagsFile(flagsFilepath); + if (!flagsFile.is_open()) { + std::cerr << "Failed to open flags configuration file: " << flagsFilepath << std::endl; return false; } + std::string flagsJson((std::istreambuf_iterator(flagsFile)), + std::istreambuf_iterator()); - // Read entire file content into a string - std::string configJson((std::istreambuf_iterator(file)), - std::istreambuf_iterator()); - - // Parse configuration using parseConfigResponse - std::string error; - response = eppoclient::parseConfigResponse(configJson, error); - - if (!error.empty()) { - std::cerr << "Failed to parse configuration: " << error << std::endl; + // Read bandit models file + std::ifstream banditsFile(banditsFilepath); + if (!banditsFile.is_open()) { + std::cerr << "Failed to open bandit models file: " << banditsFilepath << std::endl; return false; } + std::string banditsJson((std::istreambuf_iterator(banditsFile)), + std::istreambuf_iterator()); - return true; -} - -// Helper function to load bandit models from JSON file -bool loadBanditModels(const std::string& filepath, eppoclient::BanditResponse& response) { - std::ifstream file(filepath); - if (!file.is_open()) { - std::cerr << "Failed to open bandit models file: " << filepath << std::endl; - return false; - } - - std::string jsonStr((std::istreambuf_iterator(file)), std::istreambuf_iterator()); - + // Parse both configurations at once std::string error; - response = eppoclient::parseBanditResponse(jsonStr, error); + config = eppoclient::parseConfiguration(flagsJson, banditsJson, error); if (!error.empty()) { - std::cerr << "Error parsing bandit models: " << error << std::endl; + std::cerr << "Failed to parse configuration: " << error << std::endl; return false; } @@ -166,18 +154,14 @@ bool loadBanditModels(const std::string& filepath, eppoclient::BanditResponse& r int main() { // Load the bandit flags and models configuration std::cout << "Loading bandit flags and models configuration..." << std::endl; - eppoclient::ConfigResponse banditFlags; - eppoclient::BanditResponse banditModels; - if (!loadFlagsConfiguration("config/bandit-flags-v1.json", banditFlags)) { - return 1; - } - if (!loadBanditModels("config/bandit-models-v1.json", banditModels)) { + eppoclient::Configuration config; + if (!loadConfiguration("config/bandit-flags-v1.json", "config/bandit-models-v1.json", config)) { return 1; } // Create configuration store and set the configuration with both flags and bandits auto configStore = std::make_shared(); - configStore->setConfiguration(eppoclient::Configuration(banditFlags, banditModels)); + configStore->setConfiguration(config); // Create assignment logger, bandit logger, and application logger auto assignmentLogger = std::make_shared(); diff --git a/examples/flag_assignments.cpp b/examples/flag_assignments.cpp index e4bdefe..be3150a 100644 --- a/examples/flag_assignments.cpp +++ b/examples/flag_assignments.cpp @@ -67,7 +67,7 @@ class ConsoleApplicationLogger : public eppoclient::ApplicationLogger { }; // Helper function to load flags configuration from JSON file -bool loadFlagsConfiguration(const std::string& filepath, eppoclient::ConfigResponse& response) { +bool loadFlagsConfiguration(const std::string& filepath, eppoclient::Configuration& config) { std::ifstream file(filepath); if (!file.is_open()) { std::cerr << "Failed to open flags configuration file: " << filepath << std::endl; @@ -78,9 +78,9 @@ bool loadFlagsConfiguration(const std::string& filepath, eppoclient::ConfigRespo std::string configJson((std::istreambuf_iterator(file)), std::istreambuf_iterator()); - // Parse configuration using parseConfigResponse + // Parse configuration using parseConfiguration std::string error; - response = eppoclient::parseConfigResponse(configJson, error); + config = eppoclient::parseConfiguration(configJson, error); if (!error.empty()) { std::cerr << "Failed to parse configuration: " << error << std::endl; @@ -93,14 +93,14 @@ bool loadFlagsConfiguration(const std::string& filepath, eppoclient::ConfigRespo int main() { // Load the flags configuration std::cout << "Loading flags configuration..." << std::endl; - eppoclient::ConfigResponse ufc; - if (!loadFlagsConfiguration("config/flags-v1.json", ufc)) { + eppoclient::Configuration config; + if (!loadFlagsConfiguration("config/flags-v1.json", config)) { return 1; } // Create configuration store and set the configuration auto configStore = std::make_shared(); - configStore->setConfiguration(eppoclient::Configuration(ufc)); + configStore->setConfiguration(config); // Create assignment logger and application logger auto assignmentLogger = std::make_shared(); diff --git a/src/bandit_model.cpp b/src/bandit_model.cpp index f89b0c3..1daa720 100644 --- a/src/bandit_model.cpp +++ b/src/bandit_model.cpp @@ -249,6 +249,8 @@ void to_json(nlohmann::json& j, const BanditResponse& br) { j = nlohmann::json{{"bandits", br.bandits}, {"updatedAt", formatISOTimestamp(br.updatedAt)}}; } +namespace internal { + BanditResponse parseBanditResponse(const std::string& banditJson, std::string& error) { error.clear(); BanditResponse response; @@ -280,7 +282,8 @@ BanditResponse parseBanditResponse(const std::string& banditJson, std::string& e // Parse updatedAt field if (j.contains("updatedAt") && j["updatedAt"].is_string()) { std::string parseError; - response.updatedAt = parseISOTimestamp(j["updatedAt"].get_ref(), parseError); + response.updatedAt = + parseISOTimestamp(j["updatedAt"].get_ref(), parseError); if (!parseError.empty()) { allErrors.push_back("Invalid updatedAt: " + parseError); } @@ -297,6 +300,8 @@ BanditResponse parseBanditResponse(const std::string& banditJson, std::string& e return response; } +} // namespace internal + // BanditVariation serialization void to_json(nlohmann::json& j, const BanditVariation& bv) { j = nlohmann::json{{"key", bv.key}, diff --git a/src/bandit_model.hpp b/src/bandit_model.hpp index 3621b07..1ffe2ee 100644 --- a/src/bandit_model.hpp +++ b/src/bandit_model.hpp @@ -100,10 +100,6 @@ struct BanditResponse { void to_json(nlohmann::json& j, const BanditResponse& br); -// Parse bandit response from a JSON string -// Returns a BanditResponse. If parsing fails, error will contain the error message. -BanditResponse parseBanditResponse(const std::string& banditJson, std::string& error); - /** * Associates a bandit with a specific flag variation. * Used to link feature flag variations to bandit experiments. @@ -122,6 +118,11 @@ void to_json(nlohmann::json& j, const BanditVariation& bv); // Internal namespace for implementation details not covered by semver namespace internal { +// Parse bandit response from a JSON string +// Returns a BanditResponse. If parsing fails, error will contain the error message. +// INTERNAL API: Use parseConfiguration() in the parent namespace instead. +BanditResponse parseBanditResponse(const std::string& banditJson, std::string& error); + // Custom parsing functions that handle errors gracefully. // These are INTERNAL APIs and may change without notice. std::optional parseBanditNumericAttributeCoefficient( diff --git a/src/config_response.cpp b/src/config_response.cpp index bf9ba16..1b7b7b5 100644 --- a/src/config_response.cpp +++ b/src/config_response.cpp @@ -617,6 +617,8 @@ void to_json(nlohmann::json& j, const ConfigResponse& cr) { j = nlohmann::json{{"flags", cr.flags}, {"bandits", cr.bandits}}; } +namespace internal { + ConfigResponse parseConfigResponse(const std::string& configJson, std::string& error) { error.clear(); ConfigResponse response; @@ -683,4 +685,6 @@ ConfigResponse parseConfigResponse(const std::string& configJson, std::string& e return response; } +} // namespace internal + } // namespace eppoclient diff --git a/src/config_response.hpp b/src/config_response.hpp index 0ebe001..556dbf9 100644 --- a/src/config_response.hpp +++ b/src/config_response.hpp @@ -156,13 +156,14 @@ struct ConfigResponse { // serialization/deserialization for the nlohmann::json library void to_json(nlohmann::json& j, const ConfigResponse& cr); +// Internal namespace for implementation details not covered by semver +namespace internal { + // Parse configuration from a JSON string // Returns a ConfigResponse. If parsing fails, error will contain the error message. +// INTERNAL API: Use parseConfiguration() in the parent namespace instead. ConfigResponse parseConfigResponse(const std::string& configJson, std::string& error); -// Internal namespace for implementation details not covered by semver -namespace internal { - // Custom parsing functions that handle errors gracefully. // These are INTERNAL APIs and may change without notice. std::optional parseShardRange(const nlohmann::json& j, std::string& error); diff --git a/src/configuration.cpp b/src/configuration.cpp index 967142f..e68faad 100644 --- a/src/configuration.cpp +++ b/src/configuration.cpp @@ -60,4 +60,34 @@ const BanditConfiguration* Configuration::getBanditConfiguration(const std::stri return &(it->second); } +Configuration parseConfiguration(const std::string& flagConfigJson, + const std::string& banditModelsJson, std::string& error) { + error.clear(); + + // Parse flag configuration + std::string flagError; + ConfigResponse flagConfig = internal::parseConfigResponse(flagConfigJson, flagError); + if (!flagError.empty()) { + error = "Configuration parsing error: " + flagError; + return Configuration(); + } + + // Parse bandit models if provided + BanditResponse banditModels; + if (!banditModelsJson.empty()) { + std::string banditError; + banditModels = internal::parseBanditResponse(banditModelsJson, banditError); + if (!banditError.empty()) { + error = "Bandit models parsing error: " + banditError; + return Configuration(); + } + } + + return Configuration(std::move(flagConfig), std::move(banditModels)); +} + +Configuration parseConfiguration(const std::string& flagConfigJson, std::string& error) { + return parseConfiguration(flagConfigJson, "", error); +} + } // namespace eppoclient diff --git a/src/configuration.hpp b/src/configuration.hpp index bbcf4b5..83d8239 100644 --- a/src/configuration.hpp +++ b/src/configuration.hpp @@ -54,6 +54,16 @@ class Configuration { std::map> banditFlagAssociations_; }; +// Parse complete configuration from JSON strings +// Returns a Configuration object. If parsing fails, error will contain the error message. +// banditModelsJson is optional - pass an empty string to parse only flags. +Configuration parseConfiguration(const std::string& flagConfigJson, + const std::string& banditModelsJson, std::string& error); + +// Parse flag configuration only (without bandit models) +// Returns a Configuration object. If parsing fails, error will contain the error message. +Configuration parseConfiguration(const std::string& flagConfigJson, std::string& error); + } // namespace eppoclient #endif // CONFIGURATION_H diff --git a/test/shared_test_cases/test_bandit_evaluation.cpp b/test/shared_test_cases/test_bandit_evaluation.cpp index 84f1ae5..c838bbf 100644 --- a/test/shared_test_cases/test_bandit_evaluation.cpp +++ b/test/shared_test_cases/test_bandit_evaluation.cpp @@ -79,43 +79,36 @@ ContextAttributes parseContextAttributes(const json& attrJson) { return attributes; } -// Helper function to load bandit flags configuration from JSON file -ConfigResponse loadBanditFlagsConfiguration(const std::string& filepath) { - std::ifstream file(filepath); - if (!file.is_open()) { - throw std::runtime_error("Failed to open bandit flags configuration file: " + filepath); +// Helper function to load complete configuration (flags + bandits) from JSON files +Configuration loadBanditConfiguration(const std::string& flagsFilepath, + const std::string& banditsFilepath) { + // Read flags configuration file + std::ifstream flagsFile(flagsFilepath); + if (!flagsFile.is_open()) { + throw std::runtime_error("Failed to open bandit flags configuration file: " + + flagsFilepath); } + std::string flagsJson((std::istreambuf_iterator(flagsFile)), + std::istreambuf_iterator()); + + // Read bandit models file + std::ifstream banditsFile(banditsFilepath); + if (!banditsFile.is_open()) { + throw std::runtime_error("Failed to open bandit models configuration file: " + + banditsFilepath); + } + std::string banditsJson((std::istreambuf_iterator(banditsFile)), + std::istreambuf_iterator()); - // Read entire file content into a string - std::string configJson((std::istreambuf_iterator(file)), - std::istreambuf_iterator()); - - // Parse configuration using parseConfigResponse + // Parse both configurations at once std::string error; - ConfigResponse response = parseConfigResponse(configJson, error); + Configuration config = parseConfiguration(flagsJson, banditsJson, error); if (!error.empty()) { throw std::runtime_error("Failed to parse configuration: " + error); } - return response; -} - -// Helper function to load bandit models configuration from JSON file -BanditResponse loadBanditModelsConfiguration(const std::string& filepath) { - std::ifstream file(filepath); - if (!file.is_open()) { - throw std::runtime_error("Failed to open bandit models configuration file: " + filepath); - } - - std::string jsonStr((std::istreambuf_iterator(file)), std::istreambuf_iterator()); - - std::string error; - BanditResponse response = parseBanditResponse(jsonStr, error); - if (!error.empty()) { - throw std::runtime_error("Failed to parse bandit models configuration: " + error); - } - return response; + return config; } // Helper function to load a single bandit test case from JSON file @@ -208,46 +201,8 @@ TEST_CASE("UFC Bandit Test Cases - Bandit Action Selection", "[ufc][bandits]") { std::string flagsPath = "test/data/ufc/bandit-flags-v1.json"; std::string modelsPath = "test/data/ufc/bandit-models-v1.json"; - // Read flags JSON - std::ifstream flagsFile(flagsPath); - REQUIRE(flagsFile.is_open()); - json flagsJson; - flagsFile >> flagsJson; - flagsFile.close(); - - // Read models JSON - std::ifstream modelsFile(modelsPath); - REQUIRE(modelsFile.is_open()); - json modelsJson; - modelsFile >> modelsJson; - modelsFile.close(); - - // Create a JSON for ConfigResponse with flags and flattened bandits - json configJson; - configJson["flags"] = flagsJson["flags"]; - - // Parse the bandits array structure - ConfigResponse expects arrays - // The bandit-flags-v1.json has bandits as arrays which matches our structure - if (flagsJson.contains("bandits")) { - configJson["bandits"] = flagsJson["bandits"]; - } - - // Create ConfigResponse from the flags JSON - std::string error; - ConfigResponse configResponse = parseConfigResponse(configJson.dump(), error); - if (!error.empty()) { - throw std::runtime_error("Failed to parse configuration: " + error); - } - - // Create BanditResponse from the models JSON - std::string banditError; - BanditResponse banditResponse = parseBanditResponse(modelsJson.dump(), banditError); - if (!banditError.empty()) { - throw std::runtime_error("Failed to parse bandit response: " + banditError); - } - - // Create configuration with both flags and bandit models - Configuration combinedConfig(configResponse, banditResponse); + // Load configuration using the combined helper function + Configuration combinedConfig = loadBanditConfiguration(flagsPath, modelsPath); // Create client with configuration auto configStore = std::make_shared(); @@ -359,8 +314,16 @@ TEST_CASE("Load bandit models configuration", "[ufc][bandit-config]") { std::string modelsPath = "test/data/ufc/bandit-models-v1.json"; SECTION("Bandit models file exists and can be parsed") { - BanditResponse response; - REQUIRE_NOTHROW(response = loadBanditModelsConfiguration(modelsPath)); + // Read the file + std::ifstream file(modelsPath); + REQUIRE(file.is_open()); + std::string jsonStr((std::istreambuf_iterator(file)), + std::istreambuf_iterator()); + + // Parse using parseBanditResponse + std::string error; + BanditResponse response = internal::parseBanditResponse(jsonStr, error); + REQUIRE(error.empty()); // Verify we have some bandits REQUIRE(response.bandits.size() > 0); diff --git a/test/shared_test_cases/test_flag_evaluation.cpp b/test/shared_test_cases/test_flag_evaluation.cpp index f012332..622dd2c 100644 --- a/test/shared_test_cases/test_flag_evaluation.cpp +++ b/test/shared_test_cases/test_flag_evaluation.cpp @@ -29,7 +29,7 @@ struct TestCase { }; // Helper function to load flags configuration from JSON file -ConfigResponse loadFlagsConfiguration(const std::string& filepath) { +Configuration loadFlagsConfiguration(const std::string& filepath) { std::ifstream file(filepath); if (!file.is_open()) { throw std::runtime_error("Failed to open flags configuration file: " + filepath); @@ -39,15 +39,15 @@ ConfigResponse loadFlagsConfiguration(const std::string& filepath) { std::string configJson((std::istreambuf_iterator(file)), std::istreambuf_iterator()); - // Parse configuration using parseConfigResponse + // Parse configuration using parseConfiguration std::string error; - ConfigResponse response = parseConfigResponse(configJson, error); + Configuration config = parseConfiguration(configJson, error); if (!error.empty()) { throw std::runtime_error("Failed to parse configuration: " + error); } - return response; + return config; } // Helper function to parse attributes from JSON @@ -185,12 +185,9 @@ bool compareValues( TEST_CASE("UFC Test Cases - Flag Assignments", "[ufc][flags]") { // Load flags configuration std::string flagsPath = "test/data/ufc/flags-v1.json"; - ConfigResponse configResponse; + Configuration config; - REQUIRE_NOTHROW(configResponse = loadFlagsConfiguration(flagsPath)); - - // Create configuration - Configuration config(configResponse); + REQUIRE_NOTHROW(config = loadFlagsConfiguration(flagsPath)); // Create client with configuration auto configStore = std::make_shared(); @@ -308,16 +305,17 @@ TEST_CASE("Load flags configuration", "[ufc][config]") { std::string flagsPath = "test/data/ufc/flags-v1.json"; SECTION("Flags file exists and can be parsed") { - ConfigResponse response; - REQUIRE_NOTHROW(response = loadFlagsConfiguration(flagsPath)); + Configuration config; + REQUIRE_NOTHROW(config = loadFlagsConfiguration(flagsPath)); - // Verify we have some flags - REQUIRE(response.flags.size() > 0); + // Verify we have some flags (check via getFlagConfiguration) + // We know from the test data that "kill-switch" flag exists + REQUIRE(config.getFlagConfiguration("kill-switch") != nullptr); // Check for specific known flags - CHECK(response.flags.count("kill-switch") > 0); - CHECK(response.flags.count("numeric_flag") > 0); - CHECK(response.flags.count("boolean-false-assignment") > 0); + CHECK(config.getFlagConfiguration("kill-switch") != nullptr); + CHECK(config.getFlagConfiguration("numeric_flag") != nullptr); + CHECK(config.getFlagConfiguration("boolean-false-assignment") != nullptr); } } diff --git a/test/shared_test_cases/test_flag_evaluation_details.cpp b/test/shared_test_cases/test_flag_evaluation_details.cpp index cca0cbb..9f0bae2 100644 --- a/test/shared_test_cases/test_flag_evaluation_details.cpp +++ b/test/shared_test_cases/test_flag_evaluation_details.cpp @@ -40,7 +40,7 @@ struct EvaluationDetailsTestCase { }; // Helper function to load flags configuration from JSON file -static ConfigResponse loadFlagsConfiguration(const std::string& filepath) { +static Configuration loadFlagsConfiguration(const std::string& filepath) { std::ifstream file(filepath); if (!file.is_open()) { throw std::runtime_error("Failed to open flags configuration file: " + filepath); @@ -50,15 +50,15 @@ static ConfigResponse loadFlagsConfiguration(const std::string& filepath) { std::string configJson((std::istreambuf_iterator(file)), std::istreambuf_iterator()); - // Parse configuration using parseConfigResponse + // Parse configuration using parseConfiguration std::string error; - ConfigResponse response = parseConfigResponse(configJson, error); + Configuration config = parseConfiguration(configJson, error); if (!error.empty()) { throw std::runtime_error("Failed to parse configuration: " + error); } - return response; + return config; } // Helper function to parse attributes from JSON @@ -293,12 +293,9 @@ static void validateAllocationEvaluationCodes(const EvaluationDetails& evaluatio TEST_CASE("UFC Test Cases - Flag Evaluation Details", "[ufc][evaluation-details]") { // Load flags configuration std::string flagsPath = "test/data/ufc/flags-v1.json"; - ConfigResponse configResponse; + Configuration config; - REQUIRE_NOTHROW(configResponse = loadFlagsConfiguration(flagsPath)); - - // Create configuration - Configuration config(configResponse); + REQUIRE_NOTHROW(config = loadFlagsConfiguration(flagsPath)); // Create client with configuration auto configStore = std::make_shared(); diff --git a/test/shared_test_cases/test_flag_performance.cpp b/test/shared_test_cases/test_flag_performance.cpp index 7ca53cd..61033c5 100644 --- a/test/shared_test_cases/test_flag_performance.cpp +++ b/test/shared_test_cases/test_flag_performance.cpp @@ -42,7 +42,7 @@ struct TimingResult { }; // Helper function to load flags configuration from JSON file -static ConfigResponse loadFlagsConfiguration(const std::string& filepath) { +static Configuration loadFlagsConfiguration(const std::string& filepath) { std::ifstream file(filepath); if (!file.is_open()) { throw std::runtime_error("Failed to open flags configuration file: " + filepath); @@ -52,15 +52,15 @@ static ConfigResponse loadFlagsConfiguration(const std::string& filepath) { std::string configJson((std::istreambuf_iterator(file)), std::istreambuf_iterator()); - // Parse configuration using parseConfigResponse + // Parse configuration using parseConfiguration std::string error; - ConfigResponse response = parseConfigResponse(configJson, error); + Configuration config = parseConfiguration(configJson, error); if (!error.empty()) { throw std::runtime_error("Failed to parse configuration: " + error); } - return response; + return config; } // Helper function to parse attributes from JSON @@ -191,19 +191,15 @@ TEST_CASE("Performance - Flag Evaluation Timing", "[performance][flags]") { // Load flags configuration std::string flagsPath = "test/data/ufc/flags-v1.json"; std::cout << "Loading flags configuration...\n"; - ConfigResponse configResponse; + Configuration config; try { - configResponse = loadFlagsConfiguration(flagsPath); + config = loadFlagsConfiguration(flagsPath); std::cout << "Configuration loaded successfully\n"; } catch (const std::exception& e) { std::cerr << "Failed to load configuration: " << e.what() << std::endl; throw; } - // Create configuration - std::cout << "Creating configuration...\n"; - Configuration config(configResponse); - // Create client with configuration std::cout << "Creating client...\n"; diff --git a/test/test_allocation_evaluation_details.cpp b/test/test_allocation_evaluation_details.cpp index 4b99584..dc68ed0 100644 --- a/test/test_allocation_evaluation_details.cpp +++ b/test/test_allocation_evaluation_details.cpp @@ -22,7 +22,7 @@ ConfigResponse loadFlagsConfiguration(const std::string& filepath) { // Parse configuration using parseConfigResponse std::string error; - ConfigResponse response = parseConfigResponse(configJson, error); + ConfigResponse response = internal::parseConfigResponse(configJson, error); if (!error.empty()) { throw std::runtime_error("Failed to parse configuration: " + error); diff --git a/test/test_assignment_details.cpp b/test/test_assignment_details.cpp index 7f510b8..11f50f4 100644 --- a/test/test_assignment_details.cpp +++ b/test/test_assignment_details.cpp @@ -56,7 +56,7 @@ ConfigResponse loadFlagsConfiguration(const std::string& filepath) { // Parse configuration using parseConfigResponse std::string error; - ConfigResponse response = parseConfigResponse(configJson, error); + ConfigResponse response = internal::parseConfigResponse(configJson, error); if (!error.empty()) { throw std::runtime_error("Failed to parse configuration: " + error); diff --git a/test/test_bandit_action_details.cpp b/test/test_bandit_action_details.cpp index d172a50..3cfd6e1 100644 --- a/test/test_bandit_action_details.cpp +++ b/test/test_bandit_action_details.cpp @@ -56,7 +56,7 @@ ConfigResponse loadFlagsConfiguration(const std::string& filepath) { // Parse configuration using parseConfigResponse std::string error; - ConfigResponse response = parseConfigResponse(configJson, error); + ConfigResponse response = internal::parseConfigResponse(configJson, error); if (!error.empty()) { throw std::runtime_error("Failed to parse configuration: " + error); diff --git a/test/test_bandit_model.cpp b/test/test_bandit_model.cpp index 44f4adb..3775e27 100644 --- a/test/test_bandit_model.cpp +++ b/test/test_bandit_model.cpp @@ -207,7 +207,7 @@ TEST_CASE("BanditResponse serialization", "[bandit_model]") { // Deserialize from JSON std::string error2; - BanditResponse response2 = parseBanditResponse(j.dump(), error2); + BanditResponse response2 = internal::parseBanditResponse(j.dump(), error2); CHECK(error2.empty()); CHECK(response2.bandits.size() == 1); CHECK(response2.bandits["bandit1"].banditKey == "bandit1"); @@ -260,7 +260,7 @@ TEST_CASE("Complete JSON round-trip", "[bandit_model]") { // Deserialize to BanditResponse std::string error; - BanditResponse response = parseBanditResponse(jsonStr, error); + BanditResponse response = internal::parseBanditResponse(jsonStr, error); CHECK(error.empty()); // Verify structure diff --git a/test/test_config_response_json.cpp b/test/test_config_response_json.cpp index 0846e3a..313a9bd 100644 --- a/test/test_config_response_json.cpp +++ b/test/test_config_response_json.cpp @@ -166,7 +166,7 @@ TEST_CASE("ConfigResponse deserialization - empty config", "[config_response][js j["flags"] = json::object(); std::string error; - ConfigResponse response = parseConfigResponse(j.dump(), error); + ConfigResponse response = internal::parseConfigResponse(j.dump(), error); REQUIRE(error.empty()); REQUIRE(response.flags.empty()); @@ -192,7 +192,7 @@ TEST_CASE("ConfigResponse deserialization - single simple flag", "[config_respon })"_json; std::string error; - ConfigResponse response = parseConfigResponse(j.dump(), error); + ConfigResponse response = internal::parseConfigResponse(j.dump(), error); REQUIRE(error.empty()); REQUIRE(response.flags.size() == 1); @@ -252,7 +252,7 @@ TEST_CASE("ConfigResponse deserialization - all variation types", "[config_respo })"_json; std::string error; - ConfigResponse response = parseConfigResponse(j.dump(), error); + ConfigResponse response = internal::parseConfigResponse(j.dump(), error); REQUIRE(error.empty()); REQUIRE(response.flags.size() == 5); @@ -310,7 +310,7 @@ TEST_CASE("ConfigResponse deserialization - flag with allocations", "[config_res })"_json; std::string error; - ConfigResponse response = parseConfigResponse(j.dump(), error); + ConfigResponse response = internal::parseConfigResponse(j.dump(), error); REQUIRE(error.empty()); REQUIRE(response.flags.size() == 1); @@ -334,7 +334,7 @@ TEST_CASE("ConfigResponse deserialization - from real UFC file", "[config_respon file >> j; std::string error; - ConfigResponse response = parseConfigResponse(j.dump(), error); + ConfigResponse response = internal::parseConfigResponse(j.dump(), error); REQUIRE(error.empty()); // Verify some expected flags from the test file @@ -374,7 +374,7 @@ TEST_CASE("ConfigResponse round-trip - simple flag", "[config_response][json]") // Deserialize std::string error; - ConfigResponse deserialized = parseConfigResponse(j.dump(), error); + ConfigResponse deserialized = internal::parseConfigResponse(j.dump(), error); REQUIRE(error.empty()); // Verify @@ -459,7 +459,7 @@ TEST_CASE("ConfigResponse round-trip - complex flag with allocations", "[config_ // Deserialize std::string error; - ConfigResponse deserialized = parseConfigResponse(j.dump(), error); + ConfigResponse deserialized = internal::parseConfigResponse(j.dump(), error); REQUIRE(error.empty()); // Verify structure is preserved @@ -503,7 +503,7 @@ TEST_CASE("ConfigResponse precompute after deserialization", "[config_response][ })"_json; std::string error; - ConfigResponse response = parseConfigResponse(j.dump(), error); + ConfigResponse response = internal::parseConfigResponse(j.dump(), error); REQUIRE(error.empty()); // Precompute should not crash @@ -560,7 +560,7 @@ TEST_CASE("ConfigResponse usage example - deserialize from file", "[config_respo file >> j; std::string error; - ConfigResponse response = parseConfigResponse(j.dump(), error); + ConfigResponse response = internal::parseConfigResponse(j.dump(), error); REQUIRE(error.empty()); // Verify the response is valid diff --git a/test/test_configuration.cpp b/test/test_configuration.cpp index 43b1fea..1b90005 100644 --- a/test/test_configuration.cpp +++ b/test/test_configuration.cpp @@ -68,7 +68,7 @@ TEST_CASE("ConfigResponse with bandits", "[configuration]") { // Parse configuration using parseConfigResponse std::string error; - ConfigResponse response = parseConfigResponse(jsonStr, error); + ConfigResponse response = internal::parseConfigResponse(jsonStr, error); REQUIRE(error.empty()); // Verify structure @@ -142,7 +142,7 @@ TEST_CASE("BanditResponse with multiple bandits", "[configuration]") { // Deserialize to BanditResponse std::string error; - BanditResponse response = parseBanditResponse(jsonStr, error); + BanditResponse response = internal::parseBanditResponse(jsonStr, error); CHECK(error.empty()); // Verify structure @@ -173,3 +173,160 @@ TEST_CASE("BanditResponse with multiple bandits", "[configuration]") { CHECK(j2["bandits"]["bandit-1"]["modelName"] == "falcon"); CHECK(j2["bandits"]["bandit-2"]["modelName"] == "contextual"); } + +TEST_CASE("parseConfiguration convenience function", "[configuration]") { + std::string flagConfigJson = R"({ + "flags": { + "test-flag": { + "key": "test-flag", + "enabled": true, + "variationType": "STRING", + "variations": { + "control": { + "key": "control", + "value": "default" + } + }, + "allocations": [], + "totalShards": 10000 + } + } + })"; + + std::string banditModelsJson = R"({ + "bandits": { + "test-bandit": { + "banditKey": "test-bandit", + "modelName": "falcon", + "modelVersion": "v1", + "updatedAt": "2024-01-15T10:30:00Z", + "modelData": { + "gamma": 1.0, + "defaultActionScore": 0.0, + "actionProbabilityFloor": 0.0, + "coefficients": {} + } + } + } + })"; + + // Parse both configurations + std::string error; + Configuration config = parseConfiguration(flagConfigJson, banditModelsJson, error); + CHECK(error.empty()); + + // Verify flag was parsed + const FlagConfiguration* flag = config.getFlagConfiguration("test-flag"); + REQUIRE(flag != nullptr); + CHECK(flag->key == "test-flag"); + CHECK(flag->enabled == true); + + // Verify bandit was parsed + const BanditConfiguration* bandit = config.getBanditConfiguration("test-bandit"); + REQUIRE(bandit != nullptr); + CHECK(bandit->banditKey == "test-bandit"); + CHECK(bandit->modelName == "falcon"); +} + +TEST_CASE("parseConfiguration with invalid flag config", "[configuration]") { + std::string flagConfigJson = "invalid json"; + std::string banditModelsJson = R"({"bandits": {}})"; + + std::string error; + Configuration config = parseConfiguration(flagConfigJson, banditModelsJson, error); + + // Should have an error + CHECK(!error.empty()); + CHECK(error.find("Configuration parsing error") != std::string::npos); +} + +TEST_CASE("parseConfiguration with invalid bandit config", "[configuration]") { + std::string flagConfigJson = R"({"flags": {}})"; + std::string banditModelsJson = "invalid json"; + + std::string error; + Configuration config = parseConfiguration(flagConfigJson, banditModelsJson, error); + + // Should have an error + CHECK(!error.empty()); + CHECK(error.find("Bandit models parsing error") != std::string::npos); +} + +TEST_CASE("parseConfiguration without bandit models", "[configuration]") { + std::string flagConfigJson = R"({ + "flags": { + "test-flag": { + "key": "test-flag", + "enabled": true, + "variationType": "BOOLEAN", + "variations": { + "on": { + "key": "on", + "value": true + }, + "off": { + "key": "off", + "value": false + } + }, + "allocations": [], + "totalShards": 10000 + } + } + })"; + + // Parse with empty bandit models JSON + std::string error; + Configuration config = parseConfiguration(flagConfigJson, "", error); + CHECK(error.empty()); + + // Verify flag was parsed + const FlagConfiguration* flag = config.getFlagConfiguration("test-flag"); + REQUIRE(flag != nullptr); + CHECK(flag->key == "test-flag"); + CHECK(flag->enabled == true); + + // Verify no bandit configuration + const BanditConfiguration* bandit = config.getBanditConfiguration("test-bandit"); + CHECK(bandit == nullptr); +} + +TEST_CASE("parseConfiguration flags-only overload", "[configuration]") { + std::string flagConfigJson = R"({ + "flags": { + "simple-flag": { + "key": "simple-flag", + "enabled": true, + "variationType": "STRING", + "variations": { + "a": { + "key": "a", + "value": "variant-a" + }, + "b": { + "key": "b", + "value": "variant-b" + } + }, + "allocations": [], + "totalShards": 10000 + } + } + })"; + + // Parse with two-parameter overload (no bandits) + std::string error; + Configuration config = parseConfiguration(flagConfigJson, error); + CHECK(error.empty()); + + // Verify flag was parsed + const FlagConfiguration* flag = config.getFlagConfiguration("simple-flag"); + REQUIRE(flag != nullptr); + CHECK(flag->key == "simple-flag"); + CHECK(flag->enabled == true); + CHECK(flag->variations.size() == 2); + + // Verify no bandit configuration + const BanditConfiguration* bandit = config.getBanditConfiguration("any-bandit"); + CHECK(bandit == nullptr); +} diff --git a/test/test_serialized_json_assignment.cpp b/test/test_serialized_json_assignment.cpp index 3948228..faa8626 100644 --- a/test/test_serialized_json_assignment.cpp +++ b/test/test_serialized_json_assignment.cpp @@ -45,7 +45,7 @@ ConfigResponse loadFlagsConfiguration(const std::string& filepath) { // Parse configuration using parseConfigResponse std::string error; - ConfigResponse response = parseConfigResponse(configJson, error); + ConfigResponse response = internal::parseConfigResponse(configJson, error); if (!error.empty()) { throw std::runtime_error("Failed to parse configuration: " + error); From ad4b1d827ad0ada10110f8dcb01a3708cd30e8cf Mon Sep 17 00:00:00 2001 From: Greg Huels Date: Tue, 2 Dec 2025 06:19:43 -0600 Subject: [PATCH 4/4] fix windows test error --- src/bandit_model.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bandit_model.cpp b/src/bandit_model.cpp index 1daa720..9dc55fb 100644 --- a/src/bandit_model.cpp +++ b/src/bandit_model.cpp @@ -267,9 +267,9 @@ BanditResponse parseBanditResponse(const std::string& banditJson, std::string& e // Parse bandits - each bandit independently, collect errors if (j.contains("bandits") && j["bandits"].is_object()) { - for (auto& [banditKey, banditJson] : j["bandits"].items()) { + for (auto& [banditKey, banditJsonObj] : j["bandits"].items()) { std::string parseError; - auto bandit = internal::parseBanditConfiguration(banditJson, parseError); + auto bandit = internal::parseBanditConfiguration(banditJsonObj, parseError); if (bandit) { response.bandits[banditKey] = *bandit;