diff --git a/api/stats/attribute.h b/api/stats/attribute.h index fdedc74082..dd21a985df 100644 --- a/api/stats/attribute.h +++ b/api/stats/attribute.h @@ -74,8 +74,11 @@ class RTC_EXPORT Attribute { bool is_sequence() const; bool is_string() const; - std::string ValueToString() const; - std::string ValueToJson() const; + // Converts the attribute to a string that is parseable as a JSON object. + std::string ToString() const; + // TODO(https://crbug.com/15164): Use ToString() instead and delete these. + std::string ValueToString() const { return ToString(); } + std::string ValueToJson() const { return ToString(); } bool operator==(const Attribute& other) const; bool operator!=(const Attribute& other) const; diff --git a/api/stats/rtc_stats_member.h b/api/stats/rtc_stats_member.h index 2f01a1822f..1c9679f2f1 100644 --- a/api/stats/rtc_stats_member.h +++ b/api/stats/rtc_stats_member.h @@ -65,13 +65,6 @@ class RTCStatsMemberInterface { bool operator!=(const RTCStatsMemberInterface& other) const { return !(*this == other); } - virtual std::string ValueToString() const = 0; - // This is the same as ValueToString except for kInt64 and kUint64 types, - // where the value is represented as a double instead of as an integer. - // Since JSON stores numbers as floating point numbers, very large integers - // cannot be accurately represented, so we prefer to display them as doubles - // instead. - virtual std::string ValueToJson() const = 0; virtual const RTCStatsMemberInterface* member_ptr() const { return this; } template @@ -97,8 +90,10 @@ class RTCStatsMember : public RTCStatsMemberInterface { Type type() const override { return StaticType(); } bool is_sequence() const override; bool is_string() const override; - std::string ValueToString() const override; - std::string ValueToJson() const override; + // TODO(https://crbug.com/webrtc/15164): Delete both in favor of + // Attribute::ToString(). + std::string ValueToString() const; + std::string ValueToJson() const; template inline T value_or(U default_value) const { diff --git a/pc/peer_connection_rampup_tests.cc b/pc/peer_connection_rampup_tests.cc index 6535a7c27c..cc645a0ea7 100644 --- a/pc/peer_connection_rampup_tests.cc +++ b/pc/peer_connection_rampup_tests.cc @@ -313,7 +313,9 @@ class PeerConnectionRampUpTest : public ::testing::Test { return 0; } std::string selected_ice_id = - transport_stats[0]->selected_candidate_pair_id.ValueToString(); + transport_stats[0] + ->GetAttribute(transport_stats[0]->selected_candidate_pair_id) + .ToString(); // Use the selected ICE candidate pair ID to get the appropriate ICE stats. const RTCIceCandidatePairStats ice_candidate_pair_stats = stats->Get(selected_ice_id)->cast_to(); diff --git a/pc/rtc_stats_integrationtest.cc b/pc/rtc_stats_integrationtest.cc index 762dad6fcd..002f9d34b5 100644 --- a/pc/rtc_stats_integrationtest.cc +++ b/pc/rtc_stats_integrationtest.cc @@ -228,9 +228,10 @@ class RTCStatsVerifier { template void TestAttributeIsUndefined(const RTCStatsMember& field) { + Attribute attribute = stats_->GetAttribute(field); EXPECT_FALSE(field.has_value()) - << stats_->type() << "." << stats_->GetAttribute(field).name() << "[" - << stats_->id() << "] was defined (" << field.ValueToString() << ")."; + << stats_->type() << "." << attribute.name() << "[" << stats_->id() + << "] was defined (" << attribute.ToString() << ")."; MarkAttributeTested(field, !field.has_value()); } @@ -246,7 +247,7 @@ class RTCStatsVerifier { bool is_positive = field.value() > T(0); EXPECT_TRUE(is_positive) << stats_->type() << "." << attribute.name() << "[" << stats_->id() - << "] was not positive (" << attribute.ValueToString() << ")."; + << "] was not positive (" << attribute.ToString() << ")."; MarkAttributeTested(field, is_positive); } @@ -262,7 +263,7 @@ class RTCStatsVerifier { bool is_non_negative = field.value() >= T(0); EXPECT_TRUE(is_non_negative) << stats_->type() << "." << attribute.name() << "[" << stats_->id() - << "] was not non-negative (" << attribute.ValueToString() << ")."; + << "] was not non-negative (" << attribute.ToString() << ")."; MarkAttributeTested(field, is_non_negative); } @@ -323,8 +324,7 @@ class RTCStatsVerifier { << stats_->type() << "." << attribute.name() << " is not a reference to an " "existing dictionary of type " - << expected_type << " (value: " - << (field.has_value() ? attribute.ValueToString() : "null") << ")."; + << expected_type << " (value: " << attribute.ToString() << ")."; MarkAttributeTested(field, valid_reference); } diff --git a/stats/attribute.cc b/stats/attribute.cc index 76956e25b8..fab948b1bd 100644 --- a/stats/attribute.cc +++ b/stats/attribute.cc @@ -10,8 +10,13 @@ #include "api/stats/attribute.h" +#include + #include "absl/types/variant.h" +#include "rtc_base/arraysize.h" #include "rtc_base/checks.h" +#include "rtc_base/string_encode.h" +#include "rtc_base/strings/string_builder.h" namespace webrtc { @@ -30,6 +35,82 @@ struct VisitIsSequence { } }; +// Converts the attribute to string in a JSON-compatible way. +struct VisitToString { + template || std::is_same_v || + std::is_same_v || std::is_same_v, + bool> = true> + std::string ValueToString(const T& value) { + return rtc::ToString(value); + } + // Convert 64-bit integers to doubles before converting to string because JSON + // represents all numbers as floating points with ~15 digits of precision. + template || + std::is_same_v || + std::is_same_v, + bool> = true> + std::string ValueToString(const T& value) { + char buf[32]; + const int len = std::snprintf(&buf[0], arraysize(buf), "%.16g", + static_cast(value)); + RTC_DCHECK_LE(len, arraysize(buf)); + return std::string(&buf[0], len); + } + + // Vector attributes. + template + std::string operator()(const RTCStatsMember>* attribute) { + rtc::StringBuilder sb; + sb << "["; + const char* separator = ""; + constexpr bool element_is_string = std::is_same::value; + for (const T& element : attribute->value()) { + sb << separator; + if (element_is_string) { + sb << "\""; + } + sb << ValueToString(element); + if (element_is_string) { + sb << "\""; + } + separator = ","; + } + sb << "]"; + return sb.Release(); + } + // Map attributes. + template + std::string operator()( + const RTCStatsMember>* attribute) { + rtc::StringBuilder sb; + sb << "{"; + const char* separator = ""; + constexpr bool element_is_string = std::is_same::value; + for (const auto& pair : attribute->value()) { + sb << separator; + sb << "\"" << pair.first << "\":"; + if (element_is_string) { + sb << "\""; + } + sb << ValueToString(pair.second); + if (element_is_string) { + sb << "\""; + } + separator = ","; + } + sb << "}"; + return sb.Release(); + } + // Simple attributes. + template + std::string operator()(const RTCStatsMember* attribute) { + return ValueToString(attribute->value()); + } +}; + struct VisitIsEqual { template bool operator()(const RTCStatsMember* attribute) { @@ -69,14 +150,11 @@ bool Attribute::is_string() const { attribute_); } -std::string Attribute::ValueToString() const { - return absl::visit([](const auto* attr) { return attr->ValueToString(); }, - attribute_); -} - -std::string Attribute::ValueToJson() const { - return absl::visit([](const auto* attr) { return attr->ValueToJson(); }, - attribute_); +std::string Attribute::ToString() const { + if (!has_value()) { + return "null"; + } + return absl::visit(VisitToString(), attribute_); } bool Attribute::operator==(const Attribute& other) const { diff --git a/stats/rtc_stats.cc b/stats/rtc_stats.cc index ea87379b95..e7d72ee3a3 100644 --- a/stats/rtc_stats.cc +++ b/stats/rtc_stats.cc @@ -54,8 +54,7 @@ std::string RTCStats::ToJson() const { if (attribute.holds_alternative()) { sb << "\""; } - sb << absl::visit([](const auto* attr) { return attr->ValueToJson(); }, - attribute.as_variant()); + sb << attribute.ToString(); if (attribute.holds_alternative()) { sb << "\""; } diff --git a/stats/rtc_stats_member.cc b/stats/rtc_stats_member.cc index d7a3f6ccd2..a976a20046 100644 --- a/stats/rtc_stats_member.cc +++ b/stats/rtc_stats_member.cc @@ -17,6 +17,9 @@ namespace webrtc { namespace { +// TODO(https://crbug.com/webrtc/15164): Delete all stringified functions in +// favor of Attribute::ToString(). + // Produces "[a,b,c]". Works for non-vector `RTCStatsMemberInterface::Type` // types. template diff --git a/stats/rtc_stats_unittest.cc b/stats/rtc_stats_unittest.cc index 3ea36a6099..fd90692875 100644 --- a/stats/rtc_stats_unittest.cc +++ b/stats/rtc_stats_unittest.cc @@ -459,45 +459,50 @@ TEST(RTCStatsTest, IsString) { EXPECT_FALSE(stats.m_map_string_double.is_string()); } -TEST(RTCStatsTest, ValueToString) { +TEST(RTCStatsTest, AttributeToString) { RTCTestStats stats("statsId", Timestamp::Micros(42)); stats.m_bool = true; - EXPECT_EQ("true", stats.m_bool.ValueToString()); + EXPECT_EQ("true", stats.GetAttribute(stats.m_bool).ToString()); stats.m_string = "foo"; - EXPECT_EQ("foo", stats.m_string.ValueToString()); + EXPECT_EQ("foo", stats.GetAttribute(stats.m_string).ToString()); stats.m_int32 = -32; - EXPECT_EQ("-32", stats.m_int32.ValueToString()); + EXPECT_EQ("-32", stats.GetAttribute(stats.m_int32).ToString()); stats.m_uint32 = 32; - EXPECT_EQ("32", stats.m_uint32.ValueToString()); + EXPECT_EQ("32", stats.GetAttribute(stats.m_uint32).ToString()); stats.m_int64 = -64; - EXPECT_EQ("-64", stats.m_int64.ValueToString()); + EXPECT_EQ("-64", stats.GetAttribute(stats.m_int64).ToString()); stats.m_uint64 = 64; - EXPECT_EQ("64", stats.m_uint64.ValueToString()); + EXPECT_EQ("64", stats.GetAttribute(stats.m_uint64).ToString()); stats.m_double = 0.5; - EXPECT_EQ("0.5", stats.m_double.ValueToString()); + EXPECT_EQ("0.5", stats.GetAttribute(stats.m_double).ToString()); stats.m_sequence_bool = {true, false}; - EXPECT_EQ("[true,false]", stats.m_sequence_bool.ValueToString()); + EXPECT_EQ("[true,false]", + stats.GetAttribute(stats.m_sequence_bool).ToString()); stats.m_sequence_int32 = {-32, 32}; - EXPECT_EQ("[-32,32]", stats.m_sequence_int32.ValueToString()); + EXPECT_EQ("[-32,32]", stats.GetAttribute(stats.m_sequence_int32).ToString()); stats.m_sequence_uint32 = {64, 32}; - EXPECT_EQ("[64,32]", stats.m_sequence_uint32.ValueToString()); + EXPECT_EQ("[64,32]", stats.GetAttribute(stats.m_sequence_uint32).ToString()); stats.m_sequence_int64 = {-64, 32}; - EXPECT_EQ("[-64,32]", stats.m_sequence_int64.ValueToString()); + EXPECT_EQ("[-64,32]", stats.GetAttribute(stats.m_sequence_int64).ToString()); stats.m_sequence_uint64 = {16, 32}; - EXPECT_EQ("[16,32]", stats.m_sequence_uint64.ValueToString()); + EXPECT_EQ("[16,32]", stats.GetAttribute(stats.m_sequence_uint64).ToString()); stats.m_sequence_double = {0.5, 0.25}; - EXPECT_EQ("[0.5,0.25]", stats.m_sequence_double.ValueToString()); + EXPECT_EQ("[0.5,0.25]", + stats.GetAttribute(stats.m_sequence_double).ToString()); stats.m_sequence_string = {"foo", "bar"}; - EXPECT_EQ("[\"foo\",\"bar\"]", stats.m_sequence_string.ValueToString()); + EXPECT_EQ("[\"foo\",\"bar\"]", + stats.GetAttribute(stats.m_sequence_string).ToString()); stats.m_map_string_uint64 = std::map(); stats.m_map_string_uint64->emplace("foo", 32); stats.m_map_string_uint64->emplace("bar", 64); - EXPECT_EQ("{bar:64,foo:32}", stats.m_map_string_uint64.ValueToString()); + EXPECT_EQ("{\"bar\":64,\"foo\":32}", + stats.GetAttribute(stats.m_map_string_uint64).ToString()); stats.m_map_string_double = std::map(); stats.m_map_string_double->emplace("foo", 0.5); stats.m_map_string_double->emplace("bar", 0.25); - EXPECT_EQ("{bar:0.25,foo:0.5}", stats.m_map_string_double.ValueToString()); + EXPECT_EQ("{\"bar\":0.25,\"foo\":0.5}", + stats.GetAttribute(stats.m_map_string_double).ToString()); } // Death tests. diff --git a/test/pc/e2e/analyzer/video/video_quality_metrics_reporter.cc b/test/pc/e2e/analyzer/video/video_quality_metrics_reporter.cc index dbcf80e882..87f1590cb0 100644 --- a/test/pc/e2e/analyzer/video/video_quality_metrics_reporter.cc +++ b/test/pc/e2e/analyzer/video/video_quality_metrics_reporter.cc @@ -63,7 +63,9 @@ void VideoQualityMetricsReporter::OnStatsReports( } RTC_DCHECK_EQ(transport_stats.size(), 1); std::string selected_ice_id = - transport_stats[0]->selected_candidate_pair_id.ValueToString(); + transport_stats[0] + ->GetAttribute(transport_stats[0]->selected_candidate_pair_id) + .ToString(); // Use the selected ICE candidate pair ID to get the appropriate ICE stats. const RTCIceCandidatePairStats ice_candidate_pair_stats = report->Get(selected_ice_id)->cast_to();