From 602143d4c00d3d8d54de9418d32b8f10340a39bc Mon Sep 17 00:00:00 2001 From: Michal Maslanka Date: Tue, 27 Feb 2024 09:10:13 +0100 Subject: [PATCH 1/2] u/named_type: use fmt to format named type `named_type` was previously using a standard stream operator when being printed. This required any type wrapped with `named_type` to have `opertor<<` defined instead of relaying on `fmt::formatter` like a lot of types does. Changed the implementation of named type stream output operator to use `fmt` library. Signed-off-by: Michal Maslanka --- src/v/utils/named_type.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/v/utils/named_type.h b/src/v/utils/named_type.h index e8c597fb2e50..81627a866bc7 100644 --- a/src/v/utils/named_type.h +++ b/src/v/utils/named_type.h @@ -9,6 +9,8 @@ * by the Apache License, Version 2.0 */ #pragma once +#include + #include #include #include @@ -99,7 +101,8 @@ class base_named_type { } friend std::ostream& operator<<(std::ostream& o, const base_named_type& t) { - return o << "{" << t() << "}"; + fmt::print(o, "{{{}}}", t._value); + return o; }; friend std::istream& operator>>(std::istream& i, base_named_type& t) { @@ -173,7 +176,8 @@ class base_named_type { constexpr operator type() && { return std::move(_value); } friend std::ostream& operator<<(std::ostream& o, const base_named_type& t) { - return o << "{" << t() << "}"; + fmt::print(o, "{{{}}}", t._value); + return o; }; friend std::istream& operator>>(std::istream& i, base_named_type& t) { From 1b0d1d7ea66a5daf3fbd114da0ba36605af570ce Mon Sep 17 00:00:00 2001 From: Michal Maslanka Date: Tue, 27 Feb 2024 09:13:38 +0100 Subject: [PATCH 2/2] u/named_type: dropped wrapping curly braces Dropped curly braces that all named types were wrapped with when printed to string. The curly braces that the string representation of `named_type` are redundant and were often on purpose dropped by accessing the underlying value while printing the `named_type`. With this chance we are going to reduce the verbosity of log messages and make them easier to read. Signed-off-by: Michal Maslanka --- src/v/cloud_storage/tests/partition_manifest_test.cc | 6 +++--- src/v/cloud_storage/tests/topic_manifest_test.cc | 4 ++-- src/v/cluster/cloud_metadata/tests/cluster_manifest_test.cc | 2 +- src/v/kafka/server/tests/group_test.cc | 2 +- src/v/kafka/server/tests/member_test.cc | 2 +- src/v/utils/named_type.h | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/v/cloud_storage/tests/partition_manifest_test.cc b/src/v/cloud_storage/tests/partition_manifest_test.cc index 0b969c1157a0..861a64b45a85 100644 --- a/src/v/cloud_storage/tests/partition_manifest_test.cc +++ b/src/v/cloud_storage/tests/partition_manifest_test.cc @@ -710,7 +710,7 @@ SEASTAR_THREAD_TEST_CASE(test_no_size_bytes_segment_meta) { std::runtime_error, [](std::runtime_error ex) { return std::string(ex.what()).find( - "Missing size_bytes value in {10-1-v1.log} segment meta") + "Missing size_bytes value in 10-1-v1.log segment meta") != std::string::npos; }); } @@ -791,8 +791,8 @@ SEASTAR_THREAD_TEST_CASE(test_no_closing_bracket_meta) { [](std::runtime_error ex) { return std::string(ex.what()).find( "Failed to parse partition manifest " - "{\"b0000000/meta///-2147483648_-9223372036854775808/" - "manifest.json\"}: Missing a comma or '}' after an object " + "\"b0000000/meta///-2147483648_-9223372036854775808/" + "manifest.json\": Missing a comma or '}' after an object " "member. at offset 325") != std::string::npos; }); diff --git a/src/v/cloud_storage/tests/topic_manifest_test.cc b/src/v/cloud_storage/tests/topic_manifest_test.cc index 768a7377a297..a5367652073d 100644 --- a/src/v/cloud_storage/tests/topic_manifest_test.cc +++ b/src/v/cloud_storage/tests/topic_manifest_test.cc @@ -313,8 +313,8 @@ SEASTAR_THREAD_TEST_CASE(wrong_compaction_strategy_throws) { [](std::runtime_error ex) { return std::string(ex.what()).find( "Failed to parse topic manifest " - "{\"30000000/meta/full-test-namespace/full-test-topic/" - "topic_manifest.json\"}: Invalid compaction_strategy: " + "\"30000000/meta/full-test-namespace/full-test-topic/" + "topic_manifest.json\": Invalid compaction_strategy: " "wrong_value") != std::string::npos; }); diff --git a/src/v/cluster/cloud_metadata/tests/cluster_manifest_test.cc b/src/v/cluster/cloud_metadata/tests/cluster_manifest_test.cc index aed8991549a0..fc015c8ae0d0 100644 --- a/src/v/cluster/cloud_metadata/tests/cluster_manifest_test.cc +++ b/src/v/cluster/cloud_metadata/tests/cluster_manifest_test.cc @@ -52,7 +52,7 @@ SEASTAR_THREAD_TEST_CASE(test_basic_serialization) { cluster_metadata_manifest manifest; manifest.update(make_manifest_stream(simple_manifest_json)).get(); BOOST_CHECK_EQUAL(100, manifest.upload_time_since_epoch.count()); - auto uuid_str = "{01234567-89ab-cdef-0123-456789abcdef}"; + auto uuid_str = "01234567-89ab-cdef-0123-456789abcdef"; BOOST_CHECK_EQUAL(fmt::to_string(manifest.cluster_uuid), uuid_str); BOOST_CHECK_EQUAL(7, manifest.metadata_id()); BOOST_CHECK_EQUAL(42, manifest.controller_snapshot_offset()); diff --git a/src/v/kafka/server/tests/group_test.cc b/src/v/kafka/server/tests/group_test.cc index 9c40ef18b62b..c3f5bca653de 100644 --- a/src/v/kafka/server/tests/group_test.cc +++ b/src/v/kafka/server/tests/group_test.cc @@ -481,7 +481,7 @@ SEASTAR_THREAD_TEST_CASE(generate_member_id) { SEASTAR_THREAD_TEST_CASE(group_output) { auto g = get(); auto s = fmt::format("{}", g); - BOOST_TEST(s.find("id={g}") != std::string::npos); + BOOST_TEST(s.find("id=g") != std::string::npos); } SEASTAR_THREAD_TEST_CASE(group_state_output) { diff --git a/src/v/kafka/server/tests/member_test.cc b/src/v/kafka/server/tests/member_test.cc index d5096c9b07fe..affdc62f88dd 100644 --- a/src/v/kafka/server/tests/member_test.cc +++ b/src/v/kafka/server/tests/member_test.cc @@ -160,7 +160,7 @@ SEASTAR_THREAD_TEST_CASE(vote_for_protocols) { SEASTAR_THREAD_TEST_CASE(output_stream) { auto m = get_member(); auto s = fmt::format("{}", m); - BOOST_TEST(s.find("id={m}") != std::string::npos); + BOOST_TEST(s.find("id=m") != std::string::npos); } SEASTAR_THREAD_TEST_CASE(member_serde) { diff --git a/src/v/utils/named_type.h b/src/v/utils/named_type.h index 81627a866bc7..d1ac42fa0532 100644 --- a/src/v/utils/named_type.h +++ b/src/v/utils/named_type.h @@ -101,7 +101,7 @@ class base_named_type { } friend std::ostream& operator<<(std::ostream& o, const base_named_type& t) { - fmt::print(o, "{{{}}}", t._value); + fmt::print(o, "{}", t._value); return o; }; @@ -176,7 +176,7 @@ class base_named_type { constexpr operator type() && { return std::move(_value); } friend std::ostream& operator<<(std::ostream& o, const base_named_type& t) { - fmt::print(o, "{{{}}}", t._value); + fmt::print(o, "{}", t._value); return o; };