diff --git a/providers/flagd/src/sync/grpc/grpc_sync.cpp b/providers/flagd/src/sync/grpc/grpc_sync.cpp index 2ef45f0..0a49b9a 100644 --- a/providers/flagd/src/sync/grpc/grpc_sync.cpp +++ b/providers/flagd/src/sync/grpc/grpc_sync.cpp @@ -236,7 +236,11 @@ GrpcSync::StreamResult GrpcSync::ProcessStream( continue; } - UpdateFlags(raw); + absl::Status status = UpdateFlags(raw); + if (!status.ok()) { + LOG(ERROR) << "Failed to update flags: " << status; + continue; + } if (!connected) { connected = true; diff --git a/providers/flagd/src/sync/sync.cpp b/providers/flagd/src/sync/sync.cpp index 8297e67..99df01f 100644 --- a/providers/flagd/src/sync/sync.cpp +++ b/providers/flagd/src/sync/sync.cpp @@ -6,6 +6,7 @@ #include #include "absl/log/log.h" +#include "absl/strings/str_cat.h" #include "embedded_schemas.h" using Json = nlohmann::json; @@ -38,13 +39,13 @@ class FlagSync::Validator { validator.set_root_schema(Json::parse(schema::schemas.at("flagd.json"))); } - bool Validate(const Json& json) const { + absl::Status Validate(const Json& json) const { try { validator.validate(json); - return true; + return absl::OkStatus(); } catch (const std::exception& e) { - LOG(ERROR) << "Flag configuration validation failed: " << e.what(); - return false; + return absl::InvalidArgumentError( + absl::StrCat("Flag configuration validation failed: ", e.what())); } } }; @@ -58,12 +59,11 @@ FlagSync::FlagSync() FlagSync::~FlagSync() = default; -void FlagSync::UpdateFlags(const nlohmann::json& new_json) { +absl::Status FlagSync::UpdateFlags(const nlohmann::json& new_json) { if (validator_) { - if (!validator_->Validate(new_json)) { - // Validation failed, do not update flags. - LOG(ERROR) << "Flag configuration validation failed, skipping update."; - return; + absl::Status status = validator_->Validate(new_json); + if (!status.ok()) { + return status; } } @@ -83,6 +83,8 @@ void FlagSync::UpdateFlags(const nlohmann::json& new_json) { current_flags_ = std::move(new_flags_snapshot); global_metadata_ = std::move(new_metadata_snapshot); } + + return absl::OkStatus(); } void FlagSync::ClearFlags() { diff --git a/providers/flagd/src/sync/sync.h b/providers/flagd/src/sync/sync.h index f0cfd4b..34d42cf 100644 --- a/providers/flagd/src/sync/sync.h +++ b/providers/flagd/src/sync/sync.h @@ -22,7 +22,7 @@ class FlagSync { std::shared_ptr GetMetadata() const; protected: - void UpdateFlags(const nlohmann::json& new_json); + absl::Status UpdateFlags(const nlohmann::json& new_json); void ClearFlags(); private: diff --git a/providers/flagd/tests/evaluator/evaluator_test.cpp b/providers/flagd/tests/evaluator/evaluator_test.cpp index c3a60f1..1e7a0e2 100644 --- a/providers/flagd/tests/evaluator/evaluator_test.cpp +++ b/providers/flagd/tests/evaluator/evaluator_test.cpp @@ -16,8 +16,8 @@ class TestableSync : public flagd::FlagSync { } absl::Status Shutdown() override { return absl::OkStatus(); } - void TriggerUpdate(const nlohmann::json& new_json) { - this->UpdateFlags(new_json); + absl::Status TriggerUpdate(const nlohmann::json& new_json) { + return this->UpdateFlags(new_json); } }; @@ -46,7 +46,7 @@ TEST_F(EvaluatorTest, ResolveBooleanSuccess) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -63,7 +63,7 @@ TEST_F(EvaluatorTest, ResolveBooleanFlagNotFound) { nlohmann::json flags = nlohmann::json::parse(R"({ "flags": {} })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -89,7 +89,7 @@ TEST_F(EvaluatorTest, ResolveBooleanTypeMismatch) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -123,7 +123,7 @@ TEST_F(EvaluatorTest, ResolveBooleanMetadata) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -154,7 +154,7 @@ TEST_F(EvaluatorTest, ResolveBooleanVariantNotFound) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -183,7 +183,7 @@ TEST_F(EvaluatorTest, ResolveStringSuccess) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -209,7 +209,7 @@ TEST_F(EvaluatorTest, ResolveIntegerSuccess) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -235,7 +235,7 @@ TEST_F(EvaluatorTest, ResolveDoubleSuccess) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -266,7 +266,7 @@ TEST_F(EvaluatorTest, ResolveObjectSuccess) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -302,7 +302,7 @@ TEST_F(EvaluatorTest, ResolveStringTypeMismatch) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -327,7 +327,7 @@ TEST_F(EvaluatorTest, ResolveIntegerTypeMismatch) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -352,7 +352,7 @@ TEST_F(EvaluatorTest, ResolveDoubleTypeMismatch) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -371,7 +371,7 @@ class EvaluatorDefaultVariantTest TEST_P(EvaluatorDefaultVariantTest, ResolveBooleanReturnsDefault) { nlohmann::json flags_json = nlohmann::json::parse(R"({"flags":)" + GetParam() + "}"); - sync_->TriggerUpdate(flags_json); + EXPECT_TRUE(sync_->TriggerUpdate(flags_json).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -421,7 +421,7 @@ TEST_F(EvaluatorTest, ResolveBooleanDisabled) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -453,7 +453,7 @@ TEST_F(EvaluatorTest, ResolveStringTargetingSuccess) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); // Match targeting rule openfeature::EvaluationContext ctx_blue = @@ -504,7 +504,7 @@ TEST_F(EvaluatorTest, ResolveIntegerComplexTargeting) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder() .WithAttribute("env", "prod") @@ -538,7 +538,7 @@ TEST_F(EvaluatorTest, ResolveObjectNestedStructure) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -587,7 +587,7 @@ TEST_F(EvaluatorTest, ResolveBooleanFlagdSpecialVars) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder().build(); @@ -612,7 +612,7 @@ TEST_F(EvaluatorTest, ResolveBooleanTypeMismatchInTargeting) { } })"); - sync_->TriggerUpdate(flags); + EXPECT_TRUE(sync_->TriggerUpdate(flags).ok()); openfeature::EvaluationContext ctx = openfeature::EvaluationContext::Builder() .WithAttribute("color", "blue") diff --git a/providers/flagd/tests/smoke/openfeature.cpp b/providers/flagd/tests/smoke/openfeature.cpp index 174aeaf..ae01984 100644 --- a/providers/flagd/tests/smoke/openfeature.cpp +++ b/providers/flagd/tests/smoke/openfeature.cpp @@ -16,7 +16,9 @@ class MockSync : public flagd::FlagSync { } absl::Status Shutdown() override { return absl::OkStatus(); } - void SetFlags(const nlohmann::json& flags) { this->UpdateFlags(flags); } + absl::Status SetFlags(const nlohmann::json& flags) { + return this->UpdateFlags(flags); + } }; class FlagdOpenFeatureTest : public ::testing::Test { @@ -54,7 +56,7 @@ TEST_F(FlagdOpenFeatureTest, BooleanEvaluation) { } } })"); - sync_->SetFlags(flags); + EXPECT_TRUE(sync_->SetFlags(flags).ok()); EXPECT_TRUE(client_->GetBooleanValue("bool-flag", false)); EXPECT_FALSE(client_->GetBooleanValue("non-existent", false)); @@ -73,7 +75,7 @@ TEST_F(FlagdOpenFeatureTest, StringEvaluation) { } } })"); - sync_->SetFlags(flags); + EXPECT_TRUE(sync_->SetFlags(flags).ok()); EXPECT_EQ(client_->GetStringValue("string-flag", "default"), "value2"); } @@ -91,7 +93,7 @@ TEST_F(FlagdOpenFeatureTest, IntegerEvaluation) { } } })"); - sync_->SetFlags(flags); + EXPECT_TRUE(sync_->SetFlags(flags).ok()); EXPECT_EQ(client_->GetIntegerValue("int-flag", 0), 1); } @@ -109,7 +111,7 @@ TEST_F(FlagdOpenFeatureTest, DoubleEvaluation) { } } })"); - sync_->SetFlags(flags); + EXPECT_TRUE(sync_->SetFlags(flags).ok()); EXPECT_DOUBLE_EQ(client_->GetDoubleValue("double-flag", 0.0), 2.2); } @@ -128,7 +130,7 @@ TEST_F(FlagdOpenFeatureTest, ObjectEvaluation) { } } })"); - sync_->SetFlags(flags); + EXPECT_TRUE(sync_->SetFlags(flags).ok()); openfeature::Value val = client_->GetObjectValue("obj-flag", openfeature::Value()); @@ -159,7 +161,7 @@ TEST_F(FlagdOpenFeatureTest, EvaluationContextTargeting) { } } })"); - sync_->SetFlags(flags); + EXPECT_TRUE(sync_->SetFlags(flags).ok()); // Without context, should return defaultVariant "red" EXPECT_EQ(client_->GetStringValue("targeting-flag", "default"), "red-value"); diff --git a/providers/flagd/tests/smoke/sync.cpp b/providers/flagd/tests/smoke/sync.cpp index 8af8adb..d24e00e 100644 --- a/providers/flagd/tests/smoke/sync.cpp +++ b/providers/flagd/tests/smoke/sync.cpp @@ -18,8 +18,8 @@ class MockSync : public flagd::FlagSync { (override)); MOCK_METHOD(absl::Status, Shutdown, (), (override)); - void TriggerUpdate(const nlohmann::json& new_json) { - this->UpdateFlags(new_json); + absl::Status TriggerUpdate(const nlohmann::json& new_json) { + return this->UpdateFlags(new_json); } }; @@ -55,7 +55,7 @@ TEST_F(FlagSyncTest, HelperMethodsUpdateAndRetrieveFlags) { } })"_json; - sync_.TriggerUpdate(expected_flags); + EXPECT_TRUE(sync_.TriggerUpdate(expected_flags).ok()); result_ptr = sync_.GetFlags(); @@ -82,7 +82,7 @@ TEST_F(FlagSyncTest, ThreadSafetyReadersAndWriters) { const int k_iterations = 5000; std::atomic start_flag{false}; - auto writer_func = [&]() { + auto writer_func = [&] { while (!start_flag.load()); for (int i = 0; i < k_iterations; ++i) { @@ -99,11 +99,11 @@ TEST_F(FlagSyncTest, ThreadSafetyReadersAndWriters) { "metadata": {} })"_json; update["flags"]["myFlag"]["variants"]["iteration"] = i; - sync_.TriggerUpdate(update); + EXPECT_TRUE(sync_.TriggerUpdate(update).ok()); } }; - auto reader_func = [&]() { + auto reader_func = [&] { while (!start_flag.load()); for (int i = 0; i < k_iterations; ++i) { diff --git a/providers/flagd/tests/sync_test.cpp b/providers/flagd/tests/sync_test.cpp index 0d29935..6e4e897 100644 --- a/providers/flagd/tests/sync_test.cpp +++ b/providers/flagd/tests/sync_test.cpp @@ -15,8 +15,8 @@ class TestableSync : public FlagSync { } absl::Status Shutdown() override { return absl::OkStatus(); } - void TriggerUpdate(const nlohmann::json& new_json) { - this->UpdateFlags(new_json); + absl::Status TriggerUpdate(const nlohmann::json& new_json) { + return this->UpdateFlags(new_json); } }; @@ -39,7 +39,7 @@ TEST_F(SyncTest, ValidatorAcceptsValidJson) { } })"_json; - sync_.TriggerUpdate(valid_json); + EXPECT_TRUE(sync_.TriggerUpdate(valid_json).ok()); std::shared_ptr flags = sync_.GetFlags(); EXPECT_TRUE(flags->contains("my-flag")); @@ -51,7 +51,9 @@ TEST_F(SyncTest, ValidatorRejectsInvalidJson) { "something": "else" })"_json; - sync_.TriggerUpdate(invalid_json); + absl::Status status = sync_.TriggerUpdate(invalid_json); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); std::shared_ptr flags = sync_.GetFlags(); EXPECT_TRUE(flags->empty()); @@ -72,7 +74,9 @@ TEST_F(SyncTest, ValidatorRejectsInvalidType) { } })"_json; - sync_.TriggerUpdate(invalid_json); + absl::Status status = sync_.TriggerUpdate(invalid_json); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); std::shared_ptr flags = sync_.GetFlags(); EXPECT_TRUE(flags->empty()); @@ -89,7 +93,9 @@ TEST_F(SyncTest, ValidatorRejectsMissingVariants) { } })"_json; - sync_.TriggerUpdate(invalid_json); + absl::Status status = sync_.TriggerUpdate(invalid_json); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); std::shared_ptr flags = sync_.GetFlags(); EXPECT_TRUE(flags->empty()); @@ -109,7 +115,9 @@ TEST_F(SyncTest, ValidatorRejectsMalformedFlag) { } })"_json; - sync_.TriggerUpdate(invalid_json); + absl::Status status = sync_.TriggerUpdate(invalid_json); + EXPECT_FALSE(status.ok()); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); std::shared_ptr flags = sync_.GetFlags(); EXPECT_TRUE(flags->empty()); @@ -123,7 +131,7 @@ TEST_F(SyncTest, MetadataIsExtracted) { } })"_json; - sync_.TriggerUpdate(json_with_metadata); + EXPECT_TRUE(sync_.TriggerUpdate(json_with_metadata).ok()); std::shared_ptr metadata = sync_.GetMetadata(); EXPECT_EQ((*metadata)["foo"], "bar");