From 8921c4ecf9f037210253ac5434e8cab8070629f8 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 5 May 2023 12:34:06 -0700 Subject: [PATCH 1/4] Make enum for EvaluationReason::Kind --- .../common/include/data/evaluation_reason.hpp | 39 +++++++------ .../serialization/json_evaluation_reason.hpp | 9 +++ libs/common/src/data/evaluation_reason.cpp | 33 +++++++++-- .../serialization/json_evaluation_reason.cpp | 58 +++++++++++++++++-- libs/common/tests/evaluation_reason_test.cpp | 4 +- libs/common/tests/evaluation_result_test.cpp | 4 +- libs/common/tests/event_summarizer_test.cpp | 11 ++-- 7 files changed, 123 insertions(+), 35 deletions(-) diff --git a/libs/common/include/data/evaluation_reason.hpp b/libs/common/include/data/evaluation_reason.hpp index e43bd2f1a..aaac4fbaa 100644 --- a/libs/common/include/data/evaluation_reason.hpp +++ b/libs/common/include/data/evaluation_reason.hpp @@ -12,23 +12,28 @@ namespace launchdarkly { */ class EvaluationReason { public: + enum class Kind { + // The flag was off and therefore returned its configured off value. + kOff = 0, + // The flag was on but the context did not match any targets or rules. + kFallthrough = 1, + // The context key was specifically targeted for this flag. + kTargetMatch = 2, + // The context matched one of the flag's rules. + kRuleMatch = 3, + // The flag was considered off because it had at least one prerequisite + // flag that either was off or did not return the desired variation. + kPrerequisiteFailed = 4, + // The flag could not be evaluated, e.g. because it does not exist or + // due to an unexpected error. + kError = 5 + }; + friend std::ostream& operator<<(std::ostream& out, Kind const& kind); + /** - * The general category of the reason: - * - * - `"OFF"`: The flag was off and therefore returned its configured off - * value. - * - `"FALLTHROUGH"`: The flag was on but the context did not match any - * targets or rules. - * - `"TARGET_MATCH"`: The context key was specifically targeted for this - * flag. - * - `"RULE_MATCH"`: the context matched one of the flag"s rules. - * - `"PREREQUISITE_FAILED"`: The flag was considered off because it had at - * least one prerequisite flag that either was off or did not return the - * desired variation. - * - `"ERROR"`: The flag could not be evaluated, e.g. because it does not - * exist or due to an unexpected error. + * @return The general category of the reason. */ - [[nodiscard]] std::string const& kind() const; + [[nodiscard]] Kind const& kind() const; /** * A further description of the error condition, if the kind was `"ERROR"`. @@ -78,7 +83,7 @@ class EvaluationReason { */ [[nodiscard]] std::optional big_segment_status() const; - EvaluationReason(std::string kind, + EvaluationReason(Kind kind, std::optional error_kind, std::optional rule_index, std::optional rule_id, @@ -92,7 +97,7 @@ class EvaluationReason { EvaluationReason const& reason); private: - std::string kind_; + Kind kind_; std::optional error_kind_; std::optional rule_index_; std::optional rule_id_; diff --git a/libs/common/include/serialization/json_evaluation_reason.hpp b/libs/common/include/serialization/json_evaluation_reason.hpp index 2ff9d7f58..7cc4b4dd6 100644 --- a/libs/common/include/serialization/json_evaluation_reason.hpp +++ b/libs/common/include/serialization/json_evaluation_reason.hpp @@ -18,6 +18,15 @@ tl::expected tag_invoke( unused, boost::json::value const& json_value); +tl::expected tag_invoke( + boost::json::value_to_tag< + tl::expected> const& unused, + boost::json::value const& json_value); + +void tag_invoke(boost::json::value_from_tag const& unused, + boost::json::value& json_value, + EvaluationReason::Kind const& kind); + void tag_invoke(boost::json::value_from_tag const& unused, boost::json::value& json_value, EvaluationReason const& reason); diff --git a/libs/common/src/data/evaluation_reason.cpp b/libs/common/src/data/evaluation_reason.cpp index 1d08381c5..05f2dd89e 100644 --- a/libs/common/src/data/evaluation_reason.cpp +++ b/libs/common/src/data/evaluation_reason.cpp @@ -2,7 +2,32 @@ namespace launchdarkly { -std::string const& EvaluationReason::kind() const { +std::ostream& operator<<(std::ostream& out, + EvaluationReason::Kind const& kind) { + switch (kind) { + case EvaluationReason::Kind::kOff: + out << "OFF"; + break; + case EvaluationReason::Kind::kFallthrough: + out << "FALLTHROUGH"; + break; + case EvaluationReason::Kind::kTargetMatch: + out << "TARGET_MATCH"; + break; + case EvaluationReason::Kind::kRuleMatch: + out << "RULE_MATCH"; + break; + case EvaluationReason::Kind::kPrerequisiteFailed: + out << "PREREQUISITE_FAILED"; + break; + case EvaluationReason::Kind::kError: + out << "ERROR"; + break; + } + return out; +} + +EvaluationReason::Kind const& EvaluationReason::kind() const { return kind_; } @@ -31,14 +56,14 @@ std::optional EvaluationReason::big_segment_status() const { } EvaluationReason::EvaluationReason( - std::string kind, + Kind kind, std::optional error_kind, std::optional rule_index, std::optional rule_id, std::optional prerequisite_key, bool in_experiment, std::optional big_segment_status) - : kind_(std::move(kind)), + : kind_(kind), error_kind_(std::move(error_kind)), rule_index_(rule_index), rule_id_(std::move(rule_id)), @@ -47,7 +72,7 @@ EvaluationReason::EvaluationReason( big_segment_status_(std::move(big_segment_status)) {} EvaluationReason::EvaluationReason(std::string error_kind) - : EvaluationReason("ERROR", + : EvaluationReason(Kind::kError, error_kind, std::nullopt, std::nullopt, diff --git a/libs/common/src/serialization/json_evaluation_reason.cpp b/libs/common/src/serialization/json_evaluation_reason.cpp index a658fde19..b03ded743 100644 --- a/libs/common/src/serialization/json_evaluation_reason.cpp +++ b/libs/common/src/serialization/json_evaluation_reason.cpp @@ -1,7 +1,39 @@ #include "serialization/json_evaluation_reason.hpp" #include "serialization/value_mapping.hpp" +#include + namespace launchdarkly { + +tl::expected tag_invoke( + boost::json::value_to_tag< + tl::expected> const& unused, + boost::json::value const& json_value) { + if (!json_value.is_string()) { + return tl::unexpected(JsonError::kSchemaFailure); + } + auto const& str = json_value.as_string(); + if (str == "OFF") { + return EvaluationReason::Kind::kOff; + } + if (str == "FALLTHROUGH") { + return EvaluationReason::Kind::kFallthrough; + } + if (str == "TARGET_MATCH") { + return EvaluationReason::Kind::kTargetMatch; + } + if (str == "RULE_MATCH") { + return EvaluationReason::Kind::kRuleMatch; + } + if (str == "PREREQUISITE_FAILED") { + return EvaluationReason::Kind::kPrerequisiteFailed; + } + if (str == "ERROR") { + return EvaluationReason::Kind::kError; + } + return tl::make_unexpected(JsonError::kSchemaFailure); +} + tl::expected tag_invoke( boost::json::value_to_tag> const& unused, @@ -11,9 +43,16 @@ tl::expected tag_invoke( auto& json_obj = json_value.as_object(); auto kind_iter = json_obj.find("kind"); - auto kind = ValueAsOpt(kind_iter, json_obj.end()); - if (!kind.has_value()) { - return tl::unexpected(JsonError::kSchemaFailure); + if (kind_iter == json_obj.end()) { + return tl::make_unexpected(JsonError::kSchemaFailure); + } + + auto kind = boost::json::value_to< + tl::expected>( + kind_iter->value()); + + if (!kind) { + return tl::make_unexpected(kind.error()); } auto* error_kind_iter = json_obj.find("errorKind"); @@ -38,7 +77,7 @@ tl::expected tag_invoke( auto big_segment_status = ValueAsOpt(big_segment_status_iter, json_obj.end()); - return EvaluationReason{std::string(kind.value()), + return EvaluationReason{*kind, error_kind, rule_index, rule_id, @@ -48,12 +87,19 @@ tl::expected tag_invoke( } return tl::unexpected(JsonError::kSchemaFailure); } - +void tag_invoke(boost::json::value_from_tag const& unused, + boost::json::value& json_value, + EvaluationReason::Kind const& kind) { + auto& str = json_value.emplace_string(); + std::ostringstream oss; + oss << kind; + str = oss.str(); +} void tag_invoke(boost::json::value_from_tag const& unused, boost::json::value& json_value, EvaluationReason const& reason) { auto& obj = json_value.emplace_object(); - obj.emplace("kind", reason.kind()); + obj.emplace("kind", boost::json::value_from(reason.kind())); if (auto error_kind = reason.error_kind()) { obj.emplace("errorKind", *error_kind); } diff --git a/libs/common/tests/evaluation_reason_test.cpp b/libs/common/tests/evaluation_reason_test.cpp index 21677fd07..b294724b8 100644 --- a/libs/common/tests/evaluation_reason_test.cpp +++ b/libs/common/tests/evaluation_reason_test.cpp @@ -20,7 +20,7 @@ TEST(EvaluationReasonsTests, FromJsonAllFields) { "\"bigSegmentStatus\":\"STORE_ERROR\"" "}")); - EXPECT_EQ("OFF", reason.value().kind()); + EXPECT_EQ(EvaluationReason::Kind::kOff, reason.value().kind()); EXPECT_EQ("ERROR_KIND", reason.value().error_kind()); EXPECT_EQ(12, reason.value().rule_index()); EXPECT_EQ("RULE_ID", reason.value().rule_id()); @@ -36,7 +36,7 @@ TEST(EvaluationReasonsTests, FromMinimalJson) { "\"kind\":\"RULE_MATCH\"" "}")); - EXPECT_EQ("RULE_MATCH", reason.value().kind()); + EXPECT_EQ(EvaluationReason::Kind::kRuleMatch, reason.value().kind()); EXPECT_EQ(std::nullopt, reason.value().error_kind()); EXPECT_EQ(std::nullopt, reason.value().rule_index()); EXPECT_EQ(std::nullopt, reason.value().rule_id()); diff --git a/libs/common/tests/evaluation_result_test.cpp b/libs/common/tests/evaluation_result_test.cpp index 3a5e8c8cb..4f84b1416 100644 --- a/libs/common/tests/evaluation_result_test.cpp +++ b/libs/common/tests/evaluation_result_test.cpp @@ -5,6 +5,7 @@ #include "data/evaluation_result.hpp" #include "serialization/json_evaluation_result.hpp" +using launchdarkly::EvaluationReason; using launchdarkly::EvaluationResult; // NOLINTBEGIN bugprone-unchecked-optional-access @@ -46,7 +47,8 @@ TEST(EvaluationResultTests, FromJsonAllFields) { .as_object()["item"] .as_int()); EXPECT_EQ(84, evaluation_result.value().detail().variation_index()); - EXPECT_EQ("OFF", evaluation_result.value().detail().reason()->get().kind()); + EXPECT_EQ(EvaluationReason::Kind::kOff, + evaluation_result.value().detail().reason()->get().kind()); EXPECT_EQ("ERROR_KIND", evaluation_result.value().detail().reason()->get().error_kind()); EXPECT_EQ(12, diff --git a/libs/common/tests/event_summarizer_test.cpp b/libs/common/tests/event_summarizer_test.cpp index cf07cd5fb..1ee5b2db2 100644 --- a/libs/common/tests/event_summarizer_test.cpp +++ b/libs/common/tests/event_summarizer_test.cpp @@ -78,9 +78,9 @@ static FeatureEventParams FeatureEventFromParams(EvaluationParams params, params.feature_default, params.feature_version, params.feature_variation, - launchdarkly::EvaluationReason("FALLTHROUGH", std::nullopt, - std::nullopt, std::nullopt, std::nullopt, - false, std::nullopt), + launchdarkly::EvaluationReason( + launchdarkly::EvaluationReason::Kind::kFallthrough, std::nullopt, + std::nullopt, std::nullopt, std::nullopt, false, std::nullopt), false, std::nullopt, }; @@ -309,8 +309,9 @@ TEST(SummarizerTests, MissingFlagCreatesCounterUsingDefaultValue) { std::nullopt, std::nullopt, - EvaluationReason("ERROR", "FLAG_NOT_FOUND", std::nullopt, std::nullopt, - std::nullopt, false, std::nullopt), + EvaluationReason(EvaluationReason::Kind::kError, "FLAG_NOT_FOUND", + std::nullopt, std::nullopt, std::nullopt, false, + std::nullopt), false, std::nullopt, From f59bfdcf2472f23ac8d0966afedd1a8c82ace81d Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 5 May 2023 13:04:03 -0700 Subject: [PATCH 2/4] Make ErrorKind into an enum --- apps/hello-cpp/main.cpp | 4 +- libs/client-sdk/src/api.cpp | 17 ++-- .../common/include/data/evaluation_detail.hpp | 8 +- .../common/include/data/evaluation_reason.hpp | 30 +++++-- .../serialization/json_evaluation_reason.hpp | 9 ++ libs/common/src/data/evaluation_detail.cpp | 7 +- libs/common/src/data/evaluation_reason.cpp | 83 ++++++++++++------- .../serialization/json_evaluation_reason.cpp | 53 +++++++++++- libs/common/tests/evaluation_reason_test.cpp | 5 +- libs/common/tests/evaluation_result_test.cpp | 4 +- libs/common/tests/event_summarizer_test.cpp | 3 +- 11 files changed, 164 insertions(+), 59 deletions(-) diff --git a/apps/hello-cpp/main.cpp b/apps/hello-cpp/main.cpp index b72f52eb0..e4ed2a0a9 100644 --- a/apps/hello-cpp/main.cpp +++ b/apps/hello-cpp/main.cpp @@ -62,7 +62,9 @@ int main() { auto value = client.BoolVariationDetail("my-bool-flag", false); LD_LOG(logger, LogLevel::kInfo) << "Value was: " << *value; - LD_LOG(logger, LogLevel::kInfo) << "Reason was: " << value.Reason(); + if (auto reason = value.Reason()) { + LD_LOG(logger, LogLevel::kInfo) << "Reason was: " << *reason; + } // Sit around. std::cout << "Press enter to exit" << std::endl; diff --git a/libs/client-sdk/src/api.cpp b/libs/client-sdk/src/api.cpp index ef7133fd9..c607807b7 100644 --- a/libs/client-sdk/src/api.cpp +++ b/libs/client-sdk/src/api.cpp @@ -135,7 +135,8 @@ EvaluationDetail Client::VariationInternal(FlagKey const& key, "Returning default value"; // TODO: SC-199918 - auto error_reason = EvaluationReason("CLIENT_NOT_READY"); + auto error_reason = + EvaluationReason(EvaluationReason::ErrorKind::kClientNotReady); if (eval_reasons_available_) { event.reason = error_reason; } @@ -147,7 +148,8 @@ EvaluationDetail Client::VariationInternal(FlagKey const& key, LD_LOG(logger_, LogLevel::kInfo) << "Unknown feature flag " << key << "; returning default value"; - auto error_reason = EvaluationReason("FLAG_NOT_FOUND"); + auto error_reason = + EvaluationReason(EvaluationReason::ErrorKind::kFlagNotFound); if (eval_reasons_available_) { event.reason = error_reason; } @@ -168,7 +170,8 @@ EvaluationDetail Client::VariationInternal(FlagKey const& key, if (check_type && default_value.type() != Value::Type::kNull && detail.value().type() != default_value.type()) { - auto error_reason = EvaluationReason("WRONG_TYPE"); + auto error_reason = + EvaluationReason(EvaluationReason::ErrorKind::kWrongType); if (eval_reasons_available_) { event.reason = error_reason; } @@ -191,14 +194,8 @@ EvaluationDetail Client::VariationInternal(FlagKey const& key, event_processor_->AsyncSend(std::move(event)); - // TODO: this isn't a valid error, figure out how to handle if reason is - // missing. - EvaluationReason returned_reason("UNKNOWN"); - if (detail.reason()) { - returned_reason = detail.reason()->get(); - } return EvaluationDetail(detail.value(), detail.variation_index(), - returned_reason); + detail.reason()); } EvaluationDetail Client::BoolVariationDetail(Client::FlagKey const& key, diff --git a/libs/common/include/data/evaluation_detail.hpp b/libs/common/include/data/evaluation_detail.hpp index 62be5b723..44ceed8f8 100644 --- a/libs/common/include/data/evaluation_detail.hpp +++ b/libs/common/include/data/evaluation_detail.hpp @@ -25,7 +25,7 @@ class EvaluationDetail { */ EvaluationDetail(T value, std::optional variation_index, - EvaluationReason reason); + std::optional reason); /** * Constructs an EvaluationDetail representing an error and a default @@ -33,7 +33,7 @@ class EvaluationDetail { * @param error_kind Kind of the error. * @param default_value Default value. */ - EvaluationDetail(std::string error_kind, T default_value); + EvaluationDetail(EvaluationReason::ErrorKind error_kind, T default_value); /** * @return A reference to the variation value. For convenience, the * @@ -50,7 +50,7 @@ class EvaluationDetail { /** * @return A reference to the reason for the results. */ - [[nodiscard]] EvaluationReason const& Reason() const; + [[nodiscard]] std::optional const& Reason() const; /** * @return A reference to the variation value. @@ -60,6 +60,6 @@ class EvaluationDetail { private: T value_; std::optional variation_index_; - EvaluationReason reason_; + std::optional reason_; }; } // namespace launchdarkly diff --git a/libs/common/include/data/evaluation_reason.hpp b/libs/common/include/data/evaluation_reason.hpp index aaac4fbaa..b93fdd396 100644 --- a/libs/common/include/data/evaluation_reason.hpp +++ b/libs/common/include/data/evaluation_reason.hpp @@ -30,15 +30,35 @@ class EvaluationReason { }; friend std::ostream& operator<<(std::ostream& out, Kind const& kind); + enum class ErrorKind { + // The SDK was not yet fully initialized and cannot evaluate flags. + kClientNotReady = 0, + // The application did not pass valid context attributes to the SDK + // evaluation method. + kUserNotSpecified = 1, + // No flag existed with the specified flag key. + kFlagNotFound = 2, + // The application requested an evaluation result of one type but the + // resulting flag variation value was of a different type. + kWrongType = 3, + // The flag had invalid properties. + kMalformedFlag = 4, + // An unexpected error happened that stopped evaluation. + kException = 5, + }; + + friend std::ostream& operator<<(std::ostream& out, ErrorKind const& kind); + /** * @return The general category of the reason. */ [[nodiscard]] Kind const& kind() const; /** - * A further description of the error condition, if the kind was `"ERROR"`. + * A further description of the error condition, if the Kind was + * Kind::kError. */ - [[nodiscard]] std::optional error_kind() const; + [[nodiscard]] std::optional error_kind() const; /** * The index of the matched rule (0 for the first), if the kind was @@ -84,21 +104,21 @@ class EvaluationReason { [[nodiscard]] std::optional big_segment_status() const; EvaluationReason(Kind kind, - std::optional error_kind, + std::optional error_kind, std::optional rule_index, std::optional rule_id, std::optional prerequisite_key, bool in_experiment, std::optional big_segment_status); - explicit EvaluationReason(std::string error_kind); + explicit EvaluationReason(ErrorKind error_kind); friend std::ostream& operator<<(std::ostream& out, EvaluationReason const& reason); private: Kind kind_; - std::optional error_kind_; + std::optional error_kind_; std::optional rule_index_; std::optional rule_id_; std::optional prerequisite_key_; diff --git a/libs/common/include/serialization/json_evaluation_reason.hpp b/libs/common/include/serialization/json_evaluation_reason.hpp index 7cc4b4dd6..80b6bf9cb 100644 --- a/libs/common/include/serialization/json_evaluation_reason.hpp +++ b/libs/common/include/serialization/json_evaluation_reason.hpp @@ -23,10 +23,19 @@ tl::expected tag_invoke( tl::expected> const& unused, boost::json::value const& json_value); +tl::expected tag_invoke( + boost::json::value_to_tag< + tl::expected> const& unused, + boost::json::value const& json_value); + void tag_invoke(boost::json::value_from_tag const& unused, boost::json::value& json_value, EvaluationReason::Kind const& kind); +void tag_invoke(boost::json::value_from_tag const& unused, + boost::json::value& json_value, + EvaluationReason::ErrorKind const& kind); + void tag_invoke(boost::json::value_from_tag const& unused, boost::json::value& json_value, EvaluationReason const& reason); diff --git a/libs/common/src/data/evaluation_detail.cpp b/libs/common/src/data/evaluation_detail.cpp index f5aafcd30..5d1f11d78 100644 --- a/libs/common/src/data/evaluation_detail.cpp +++ b/libs/common/src/data/evaluation_detail.cpp @@ -8,13 +8,14 @@ template EvaluationDetail::EvaluationDetail( T value, std::optional variation_index, - launchdarkly::EvaluationReason reason) + std::optional reason) : value_(std::move(value)), variation_index_(variation_index), reason_(std::move(reason)) {} template -EvaluationDetail::EvaluationDetail(std::string error_kind, T default_value) +EvaluationDetail::EvaluationDetail(EvaluationReason::ErrorKind error_kind, + T default_value) : value_(std::move(default_value)), variation_index_(std::nullopt), reason_(error_kind) {} @@ -25,7 +26,7 @@ T const& EvaluationDetail::Value() const { } template -EvaluationReason const& EvaluationDetail::Reason() const { +std::optional const& EvaluationDetail::Reason() const { return reason_; } diff --git a/libs/common/src/data/evaluation_reason.cpp b/libs/common/src/data/evaluation_reason.cpp index 05f2dd89e..5eec6a53e 100644 --- a/libs/common/src/data/evaluation_reason.cpp +++ b/libs/common/src/data/evaluation_reason.cpp @@ -2,36 +2,12 @@ namespace launchdarkly { -std::ostream& operator<<(std::ostream& out, - EvaluationReason::Kind const& kind) { - switch (kind) { - case EvaluationReason::Kind::kOff: - out << "OFF"; - break; - case EvaluationReason::Kind::kFallthrough: - out << "FALLTHROUGH"; - break; - case EvaluationReason::Kind::kTargetMatch: - out << "TARGET_MATCH"; - break; - case EvaluationReason::Kind::kRuleMatch: - out << "RULE_MATCH"; - break; - case EvaluationReason::Kind::kPrerequisiteFailed: - out << "PREREQUISITE_FAILED"; - break; - case EvaluationReason::Kind::kError: - out << "ERROR"; - break; - } - return out; -} - EvaluationReason::Kind const& EvaluationReason::kind() const { return kind_; } -std::optional EvaluationReason::error_kind() const { +std::optional EvaluationReason::error_kind() + const { return error_kind_; } @@ -57,7 +33,7 @@ std::optional EvaluationReason::big_segment_status() const { EvaluationReason::EvaluationReason( Kind kind, - std::optional error_kind, + std::optional error_kind, std::optional rule_index, std::optional rule_id, std::optional prerequisite_key, @@ -71,7 +47,7 @@ EvaluationReason::EvaluationReason( in_experiment_(in_experiment), big_segment_status_(std::move(big_segment_status)) {} -EvaluationReason::EvaluationReason(std::string error_kind) +EvaluationReason::EvaluationReason(ErrorKind error_kind) : EvaluationReason(Kind::kError, error_kind, std::nullopt, @@ -115,4 +91,55 @@ bool operator==(EvaluationReason const& lhs, EvaluationReason const& rhs) { bool operator!=(EvaluationReason const& lhs, EvaluationReason const& rhs) { return !(lhs == rhs); } + +std::ostream& operator<<(std::ostream& out, + EvaluationReason::Kind const& kind) { + switch (kind) { + case EvaluationReason::Kind::kOff: + out << "OFF"; + break; + case EvaluationReason::Kind::kFallthrough: + out << "FALLTHROUGH"; + break; + case EvaluationReason::Kind::kTargetMatch: + out << "TARGET_MATCH"; + break; + case EvaluationReason::Kind::kRuleMatch: + out << "RULE_MATCH"; + break; + case EvaluationReason::Kind::kPrerequisiteFailed: + out << "PREREQUISITE_FAILED"; + break; + case EvaluationReason::Kind::kError: + out << "ERROR"; + break; + } + return out; +} + +std::ostream& operator<<(std::ostream& out, + EvaluationReason::ErrorKind const& kind) { + switch (kind) { + case EvaluationReason::ErrorKind::kClientNotReady: + out << "CLIENT_NOT_READY"; + break; + case EvaluationReason::ErrorKind::kUserNotSpecified: + out << "USER_NOT_SPECIFIED"; + break; + case EvaluationReason::ErrorKind::kFlagNotFound: + out << "FLAG_NOT_FOUND"; + break; + case EvaluationReason::ErrorKind::kWrongType: + out << "WRONG_TYPE"; + break; + case EvaluationReason::ErrorKind::kMalformedFlag: + out << "MALFORMED_FLAG"; + break; + case EvaluationReason::ErrorKind::kException: + out << "EXCEPTION"; + break; + } + return out; +} + } // namespace launchdarkly diff --git a/libs/common/src/serialization/json_evaluation_reason.cpp b/libs/common/src/serialization/json_evaluation_reason.cpp index b03ded743..aa3dbaab2 100644 --- a/libs/common/src/serialization/json_evaluation_reason.cpp +++ b/libs/common/src/serialization/json_evaluation_reason.cpp @@ -34,6 +34,35 @@ tl::expected tag_invoke( return tl::make_unexpected(JsonError::kSchemaFailure); } +tl::expected tag_invoke( + boost::json::value_to_tag< + tl::expected> const& unused, + boost::json::value const& json_value) { + if (!json_value.is_string()) { + return tl::unexpected(JsonError::kSchemaFailure); + } + auto const& str = json_value.as_string(); + if (str == "CLIENT_NOT_READY") { + return EvaluationReason::ErrorKind::kClientNotReady; + } + if (str == "USER_NOT_SPECIFIED") { + return EvaluationReason::ErrorKind::kUserNotSpecified; + } + if (str == "FLAG_NOT_FOUND") { + return EvaluationReason::ErrorKind::kFlagNotFound; + } + if (str == "WRONG_TYPE") { + return EvaluationReason::ErrorKind::kWrongType; + } + if (str == "MALFORMED_FLAG") { + return EvaluationReason::ErrorKind::kMalformedFlag; + } + if (str == "EXCEPTION") { + return EvaluationReason::ErrorKind::kException; + } + return tl::make_unexpected(JsonError::kSchemaFailure); +} + tl::expected tag_invoke( boost::json::value_to_tag> const& unused, @@ -56,8 +85,16 @@ tl::expected tag_invoke( } auto* error_kind_iter = json_obj.find("errorKind"); - auto error_kind = - ValueAsOpt(error_kind_iter, json_obj.end()); + std::optional error_kind; + if (error_kind_iter != json_obj.end()) { + auto parsed = boost::json::value_to< + tl::expected>( + error_kind_iter->value()); + if (!parsed) { + return tl::make_unexpected(parsed.error()); + } + error_kind = parsed.value(); + } auto* rule_index_iter = json_obj.find("ruleIndex"); auto rule_index = ValueAsOpt(rule_index_iter, json_obj.end()); @@ -95,13 +132,23 @@ void tag_invoke(boost::json::value_from_tag const& unused, oss << kind; str = oss.str(); } + +void tag_invoke(boost::json::value_from_tag const& unused, + boost::json::value& json_value, + EvaluationReason::ErrorKind const& kind) { + auto& str = json_value.emplace_string(); + std::ostringstream oss; + oss << kind; + str = oss.str(); +} + void tag_invoke(boost::json::value_from_tag const& unused, boost::json::value& json_value, EvaluationReason const& reason) { auto& obj = json_value.emplace_object(); obj.emplace("kind", boost::json::value_from(reason.kind())); if (auto error_kind = reason.error_kind()) { - obj.emplace("errorKind", *error_kind); + obj.emplace("errorKind", boost::json::value_from(*error_kind)); } if (auto big_segment_status = reason.big_segment_status()) { obj.emplace("bigSegmentStatus", *big_segment_status); diff --git a/libs/common/tests/evaluation_reason_test.cpp b/libs/common/tests/evaluation_reason_test.cpp index b294724b8..53abd5aeb 100644 --- a/libs/common/tests/evaluation_reason_test.cpp +++ b/libs/common/tests/evaluation_reason_test.cpp @@ -12,7 +12,7 @@ TEST(EvaluationReasonsTests, FromJsonAllFields) { boost::json::value_to>( boost::json::parse("{" "\"kind\":\"OFF\"," - "\"errorKind\":\"ERROR_KIND\"," + "\"errorKind\":\"MALFORMED_FLAG\"," "\"ruleIndex\":12," "\"ruleId\":\"RULE_ID\"," "\"prerequisiteKey\":\"PREREQ_KEY\"," @@ -21,7 +21,8 @@ TEST(EvaluationReasonsTests, FromJsonAllFields) { "}")); EXPECT_EQ(EvaluationReason::Kind::kOff, reason.value().kind()); - EXPECT_EQ("ERROR_KIND", reason.value().error_kind()); + EXPECT_EQ(EvaluationReason::ErrorKind::kMalformedFlag, + reason.value().error_kind()); EXPECT_EQ(12, reason.value().rule_index()); EXPECT_EQ("RULE_ID", reason.value().rule_id()); EXPECT_EQ("PREREQ_KEY", reason.value().prerequisite_key()); diff --git a/libs/common/tests/evaluation_result_test.cpp b/libs/common/tests/evaluation_result_test.cpp index 4f84b1416..0f7e2ce9c 100644 --- a/libs/common/tests/evaluation_result_test.cpp +++ b/libs/common/tests/evaluation_result_test.cpp @@ -24,7 +24,7 @@ TEST(EvaluationResultTests, FromJsonAllFields) { "\"variation\": 84," "\"reason\": {" "\"kind\":\"OFF\"," - "\"errorKind\":\"ERROR_KIND\"," + "\"errorKind\":\"MALFORMED_FLAG\"," "\"ruleIndex\":12," "\"ruleId\":\"RULE_ID\"," "\"prerequisiteKey\":\"PREREQ_KEY\"," @@ -49,7 +49,7 @@ TEST(EvaluationResultTests, FromJsonAllFields) { EXPECT_EQ(84, evaluation_result.value().detail().variation_index()); EXPECT_EQ(EvaluationReason::Kind::kOff, evaluation_result.value().detail().reason()->get().kind()); - EXPECT_EQ("ERROR_KIND", + EXPECT_EQ(EvaluationReason::ErrorKind::kMalformedFlag, evaluation_result.value().detail().reason()->get().error_kind()); EXPECT_EQ(12, evaluation_result.value().detail().reason()->get().rule_index()); diff --git a/libs/common/tests/event_summarizer_test.cpp b/libs/common/tests/event_summarizer_test.cpp index 1ee5b2db2..1eb51dd95 100644 --- a/libs/common/tests/event_summarizer_test.cpp +++ b/libs/common/tests/event_summarizer_test.cpp @@ -309,7 +309,8 @@ TEST(SummarizerTests, MissingFlagCreatesCounterUsingDefaultValue) { std::nullopt, std::nullopt, - EvaluationReason(EvaluationReason::Kind::kError, "FLAG_NOT_FOUND", + EvaluationReason(EvaluationReason::Kind::kError, + EvaluationReason::ErrorKind::kFlagNotFound, std::nullopt, std::nullopt, std::nullopt, false, std::nullopt), From 28266baf5b58738aedebaea41977ec30777a1227 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 5 May 2023 13:40:05 -0700 Subject: [PATCH 3/4] make operator<< use tag_invoke implementation --- libs/common/src/data/evaluation_reason.cpp | 44 +----------- .../serialization/json_evaluation_reason.cpp | 69 ++++++++++++++----- 2 files changed, 55 insertions(+), 58 deletions(-) diff --git a/libs/common/src/data/evaluation_reason.cpp b/libs/common/src/data/evaluation_reason.cpp index 5eec6a53e..0834d4806 100644 --- a/libs/common/src/data/evaluation_reason.cpp +++ b/libs/common/src/data/evaluation_reason.cpp @@ -1,5 +1,5 @@ #include "data/evaluation_reason.hpp" - +#include "serialization/json_evaluation_reason.hpp" namespace launchdarkly { EvaluationReason::Kind const& EvaluationReason::kind() const { @@ -94,51 +94,13 @@ bool operator!=(EvaluationReason const& lhs, EvaluationReason const& rhs) { std::ostream& operator<<(std::ostream& out, EvaluationReason::Kind const& kind) { - switch (kind) { - case EvaluationReason::Kind::kOff: - out << "OFF"; - break; - case EvaluationReason::Kind::kFallthrough: - out << "FALLTHROUGH"; - break; - case EvaluationReason::Kind::kTargetMatch: - out << "TARGET_MATCH"; - break; - case EvaluationReason::Kind::kRuleMatch: - out << "RULE_MATCH"; - break; - case EvaluationReason::Kind::kPrerequisiteFailed: - out << "PREREQUISITE_FAILED"; - break; - case EvaluationReason::Kind::kError: - out << "ERROR"; - break; - } + out << boost::json::value_from(kind).as_string(); return out; } std::ostream& operator<<(std::ostream& out, EvaluationReason::ErrorKind const& kind) { - switch (kind) { - case EvaluationReason::ErrorKind::kClientNotReady: - out << "CLIENT_NOT_READY"; - break; - case EvaluationReason::ErrorKind::kUserNotSpecified: - out << "USER_NOT_SPECIFIED"; - break; - case EvaluationReason::ErrorKind::kFlagNotFound: - out << "FLAG_NOT_FOUND"; - break; - case EvaluationReason::ErrorKind::kWrongType: - out << "WRONG_TYPE"; - break; - case EvaluationReason::ErrorKind::kMalformedFlag: - out << "MALFORMED_FLAG"; - break; - case EvaluationReason::ErrorKind::kException: - out << "EXCEPTION"; - break; - } + out << boost::json::value_from(kind).as_string(); return out; } diff --git a/libs/common/src/serialization/json_evaluation_reason.cpp b/libs/common/src/serialization/json_evaluation_reason.cpp index aa3dbaab2..1a991c6a4 100644 --- a/libs/common/src/serialization/json_evaluation_reason.cpp +++ b/libs/common/src/serialization/json_evaluation_reason.cpp @@ -34,6 +34,32 @@ tl::expected tag_invoke( return tl::make_unexpected(JsonError::kSchemaFailure); } +void tag_invoke(boost::json::value_from_tag const& unused, + boost::json::value& json_value, + EvaluationReason::Kind const& kind) { + auto& str = json_value.emplace_string(); + switch (kind) { + case EvaluationReason::Kind::kOff: + str = "OFF"; + break; + case EvaluationReason::Kind::kFallthrough: + str = "FALLTHROUGH"; + break; + case EvaluationReason::Kind::kTargetMatch: + str = "TARGET_MATCH"; + break; + case EvaluationReason::Kind::kRuleMatch: + str = "RULE_MATCH"; + break; + case EvaluationReason::Kind::kPrerequisiteFailed: + str = "PREREQUISITE_FAILED"; + break; + case EvaluationReason::Kind::kError: + str = "ERROR"; + break; + } +} + tl::expected tag_invoke( boost::json::value_to_tag< tl::expected> const& unused, @@ -63,6 +89,32 @@ tl::expected tag_invoke( return tl::make_unexpected(JsonError::kSchemaFailure); } +void tag_invoke(boost::json::value_from_tag const& unused, + boost::json::value& json_value, + EvaluationReason::ErrorKind const& kind) { + auto& str = json_value.emplace_string(); + switch (kind) { + case EvaluationReason::ErrorKind::kClientNotReady: + str = "CLIENT_NOT_READY"; + break; + case EvaluationReason::ErrorKind::kUserNotSpecified: + str = "USER_NOT_SPECIFIED"; + break; + case EvaluationReason::ErrorKind::kFlagNotFound: + str = "FLAG_NOT_FOUND"; + break; + case EvaluationReason::ErrorKind::kWrongType: + str = "WRONG_TYPE"; + break; + case EvaluationReason::ErrorKind::kMalformedFlag: + str = "MALFORMED_FLAG"; + break; + case EvaluationReason::ErrorKind::kException: + str = "EXCEPTION"; + break; + } +} + tl::expected tag_invoke( boost::json::value_to_tag> const& unused, @@ -124,23 +176,6 @@ tl::expected tag_invoke( } return tl::unexpected(JsonError::kSchemaFailure); } -void tag_invoke(boost::json::value_from_tag const& unused, - boost::json::value& json_value, - EvaluationReason::Kind const& kind) { - auto& str = json_value.emplace_string(); - std::ostringstream oss; - oss << kind; - str = oss.str(); -} - -void tag_invoke(boost::json::value_from_tag const& unused, - boost::json::value& json_value, - EvaluationReason::ErrorKind const& kind) { - auto& str = json_value.emplace_string(); - std::ostringstream oss; - oss << kind; - str = oss.str(); -} void tag_invoke(boost::json::value_from_tag const& unused, boost::json::value& json_value, From 4908939eb86b3fef9f43c21d1b027923887cc5e5 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Fri, 5 May 2023 14:17:51 -0700 Subject: [PATCH 4/4] remove useless std::move --- libs/common/src/data/evaluation_reason.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/common/src/data/evaluation_reason.cpp b/libs/common/src/data/evaluation_reason.cpp index 0834d4806..bddd7c1f9 100644 --- a/libs/common/src/data/evaluation_reason.cpp +++ b/libs/common/src/data/evaluation_reason.cpp @@ -40,7 +40,7 @@ EvaluationReason::EvaluationReason( bool in_experiment, std::optional big_segment_status) : kind_(kind), - error_kind_(std::move(error_kind)), + error_kind_(error_kind), rule_index_(rule_index), rule_id_(std::move(rule_id)), prerequisite_key_(std::move(prerequisite_key)),