Skip to content

Commit 067af68

Browse files
committed
fix summary counter JSON serialization
1 parent 0f510fa commit 067af68

File tree

5 files changed

+87
-17
lines changed

5 files changed

+87
-17
lines changed

libs/common/include/events/client_events.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ struct FeatureEventParams {
1919
std::string key;
2020
Context context;
2121
EvaluationResult eval_result;
22+
Value default_;
2223
};
2324

2425
struct FeatureEventBase {

libs/common/include/events/detail/summarizer.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,11 @@ class Summarizer {
5252

5353
struct State {
5454
Value default_;
55-
std::unordered_set<std::string> context_kinds_;
55+
std::unordered_set<std::string> context_kinds;
5656
std::unordered_map<Summarizer::VariationKey,
5757
Summarizer::VariationSummary,
5858
Summarizer::VariationKey::Hash>
59-
counters_;
59+
counters;
6060

6161
State(Value defaultVal);
6262
};

libs/common/src/events/summarizer.cpp

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,30 +30,28 @@ void Summarizer::update(client::FeatureEventParams const& event) {
3030
// TODO(cwaldren): Value::null() should be replaced with the default value
3131
// from the evaluation.
3232

33-
auto default_value = Value::null();
3433
auto feature_state_iterator =
35-
features_.try_emplace(event.key, std::move(default_value)).first;
34+
features_.try_emplace(event.key, event.default_).first;
3635

37-
feature_state_iterator->second.context_kinds_.insert(kinds.begin(),
38-
kinds.end());
36+
feature_state_iterator->second.context_kinds.insert(kinds.begin(),
37+
kinds.end());
3938

4039
decltype(std::begin(
41-
feature_state_iterator->second.counters_)) summary_counter;
40+
feature_state_iterator->second.counters)) summary_counter;
4241

4342
if (FlagNotFound(event)) {
44-
auto key = VariationKey();
4543
summary_counter =
46-
feature_state_iterator->second.counters_
47-
.try_emplace(std::move(key),
44+
feature_state_iterator->second.counters
45+
.try_emplace(VariationKey(),
4846
feature_state_iterator->second.default_)
4947
.first;
5048

5149
} else {
5250
auto key = VariationKey(event.eval_result.version(),
5351
event.eval_result.detail().variation_index());
5452
summary_counter =
55-
feature_state_iterator->second.counters_
56-
.try_emplace(std::move(key), event.eval_result.detail().value())
53+
feature_state_iterator->second.counters
54+
.try_emplace(key, event.eval_result.detail().value())
5755
.first;
5856
}
5957

@@ -78,5 +76,5 @@ void Summarizer::VariationSummary::Increment() {
7876
}
7977

8078
Summarizer::State::State(Value defaultVal)
81-
: default_(std::move(defaultVal)), context_kinds_() {}
79+
: default_(std::move(defaultVal)), context_kinds() {}
8280
} // namespace launchdarkly::events::detail

libs/common/src/serialization/events/json_events.cpp

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,44 @@ void tag_invoke(boost::json::value_from_tag const&,
9191
boost::json::value& json_value,
9292
Summarizer::VariationSummary const& state) {}
9393

94+
/*
95+
* version (number, optional): the value of version for the counter; if version
96+
has no value, this property should be omitted entirely rather than writing a
97+
null value.
98+
99+
variation (number, optional): the value of variation for the counter; if
100+
variation has no value, this property should be omitted entirely rather than
101+
writing a null value.
102+
103+
value (any JSON type): the value of value for the counter.
104+
105+
count (number, required): the value of count for the counter.
106+
107+
unknown (boolean, optional): this should be true if version has no value;
108+
otherwise it should be omitted (do not write an explicit false).
109+
*/
94110
void tag_invoke(boost::json::value_from_tag const&,
95111
boost::json::value& json_value,
96112
Summarizer::State const& state) {
97113
auto& obj = json_value.emplace_object();
98114
obj.emplace("default", boost::json::value_from(state.default_));
99-
obj.emplace("contextKinds", boost::json::value_from(state.context_kinds_));
115+
obj.emplace("contextKinds", boost::json::value_from(state.context_kinds));
116+
boost::json::array counters;
117+
for (auto const& kvp : state.counters) {
118+
boost::json::object counter;
119+
if (kvp.first.version) {
120+
counter.emplace("version", *kvp.first.version);
121+
} else {
122+
counter.emplace("unknown", true);
123+
}
124+
if (kvp.first.variation) {
125+
counter.emplace("variation", *kvp.first.variation);
126+
}
127+
counter.emplace("value", boost::json::value_from(kvp.second.value));
128+
counter.emplace("count", kvp.second.count);
129+
counters.push_back(std::move(counter));
130+
}
131+
obj.emplace("counters", std::move(counters));
100132
}
101133
void tag_invoke(boost::json::value_from_tag const&,
102134
boost::json::value& json_value,

libs/common/tests/event_processor_test.cpp

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@
88
#include "events/client_events.hpp"
99
#include "events/detail/asio_event_processor.hpp"
1010
#include "events/detail/summarizer.hpp"
11+
#include "serialization/events/json_events.hpp"
1112

1213
using namespace launchdarkly::events::detail;
13-
// class EventProcessorTests : public ::testing::Test {};
1414

1515
static std::chrono::system_clock::time_point ZeroTime() {
1616
return std::chrono::system_clock::time_point{};
@@ -41,6 +41,7 @@ TEST(SummarizerTests, SummaryCounterUpdates) {
4141
auto const feature_version = 1;
4242
auto const context = ContextBuilder().kind("cat", "shadow").build();
4343
auto const feature_value = Value(3);
44+
auto const feature_default = Value(1);
4445
auto const feature_variation = 0;
4546

4647
auto const event = FeatureEventParams{
@@ -54,6 +55,7 @@ TEST(SummarizerTests, SummaryCounterUpdates) {
5455
EvaluationReason("FALLTHROUGH", std::nullopt, std::nullopt,
5556
std::nullopt, std::nullopt, false,
5657
std::nullopt))),
58+
feature_default,
5759
};
5860

5961
auto const num_events = 10;
@@ -65,14 +67,51 @@ TEST(SummarizerTests, SummaryCounterUpdates) {
6567
auto const& cat_food = features.find(feature_key);
6668
ASSERT_TRUE(cat_food != features.end());
6769

68-
auto const& counter = cat_food->second.counters_.find(
70+
auto const& counter = cat_food->second.counters.find(
6971
Summarizer::VariationKey(feature_version, feature_variation));
70-
ASSERT_TRUE(counter != cat_food->second.counters_.end());
72+
ASSERT_TRUE(counter != cat_food->second.counters.end());
7173

7274
ASSERT_EQ(counter->second.value.as_double(), feature_value.as_double());
7375
ASSERT_EQ(counter->second.count, num_events);
7476
}
7577

78+
TEST(SummarizerTests, JsonSerialization) {
79+
using namespace launchdarkly::events::client;
80+
using namespace launchdarkly;
81+
Summarizer summarizer;
82+
83+
auto const feature_key = "cat-food-amount";
84+
auto const feature_version = 1;
85+
auto const context = ContextBuilder().kind("cat", "shadow").build();
86+
auto const feature_value = Value(3);
87+
auto const feature_default = Value(1);
88+
auto const feature_variation = 0;
89+
90+
auto const event = FeatureEventParams{
91+
ZeroTime(),
92+
feature_key,
93+
context,
94+
EvaluationResult(
95+
feature_version, std::nullopt, false, false, std::nullopt,
96+
EvaluationDetailInternal(
97+
feature_value, feature_variation,
98+
EvaluationReason("FALLTHROUGH", std::nullopt, std::nullopt,
99+
std::nullopt, std::nullopt, false,
100+
std::nullopt))),
101+
feature_default,
102+
};
103+
104+
auto const num_events = 10;
105+
for (size_t i = 0; i < num_events; i++) {
106+
summarizer.update(event);
107+
}
108+
109+
auto json = boost::json::value_from(Summary{summarizer, ZeroTime()});
110+
auto expected = boost::json::parse(
111+
R"({"kind":"summary","startDate":0,"endDate":0,"features":{"cat-food-amount":{"default":1E0,"contextKinds":["cat"],"counters":[{"version":1,"variation":0,"value":3E0,"count":10}]}}})");
112+
ASSERT_EQ(json, expected);
113+
}
114+
76115
// This test is a temporary test that exists only to ensure the event processor
77116
// compiles; it should be replaced by more robust tests (and contract tests.)
78117
TEST(EventProcessorTests, ProcessorCompiles) {

0 commit comments

Comments
 (0)