Skip to content

Commit 0295408

Browse files
weigondahlerlend
authored andcommitted
Bug#37047661 & Bug#37047661 & Bug#36982911
Bug#37047661 replace utility::starts_with with std::string.starts_wi Bug#36982911 mysql_harness::join does not handle std::string_view BUG #35496553: ML006031 is mapped to two different error messages Change-Id: I8d1549c614c40f553d43d7bdaf5f48a40a81c331
1 parent 0301460 commit 0295408

File tree

7 files changed

+116
-123
lines changed

7 files changed

+116
-123
lines changed

router/src/harness/include/mysql/harness/utility/string.h

Lines changed: 33 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828

2929
#include "harness_export.h"
3030

31-
#include <numeric> // accumulate
3231
#include <string>
3332
#include <vector>
3433

@@ -40,118 +39,56 @@ std::vector<std::string> HARNESS_EXPORT wrap_string(const std::string &to_wrap,
4039
std::size_t width,
4140
std::size_t indent_size);
4241

43-
/** @brief Checks whether string ends with the specified suffix
44-
*
45-
* Returns true if the string ends with the given suffix.
46-
*
47-
* @return bool
48-
*/
49-
bool HARNESS_EXPORT ends_with(const std::string &str,
50-
const std::string &suffix);
51-
52-
/** @brief Checks whether string starts with the specified prefix
53-
*
54-
* Returns true if the string begins with the given prefix.
55-
*
56-
* @return bool
57-
*/
58-
bool HARNESS_EXPORT starts_with(const std::string &str,
59-
const std::string &prefix);
60-
6142
HARNESS_EXPORT
6243
MY_ATTRIBUTE((format(printf, 1, 2)))
6344
std::string string_format(const char *format, ...);
6445

6546
} // namespace utility
6647

6748
namespace detail {
68-
template <class Container, class T>
69-
struct Join {
70-
static std::string impl(Container, const std::string &);
71-
};
72-
73-
template <class Container>
74-
struct Join<Container, std::string> {
75-
static std::string impl(Container cont, const std::string &delim) {
76-
if (cont.begin() == cont.end()) return {};
77-
78-
std::string o(*(cont.begin()));
79-
80-
// if T::value_type has a .size() method reallocs can be avoided
81-
// when joining the strings by calculating the size upfront
82-
{
83-
const size_t delim_size = delim.size();
84-
size_t space =
85-
std::accumulate(std::next(cont.begin()), cont.end(), o.size(),
86-
[delim_size](size_t sum, const std::string &b) {
87-
return sum + delim_size + b.size();
88-
});
89-
o.reserve(space);
90-
}
91-
92-
#if 0
93-
// once benchmarked that this is equivalent of the hand-rolled version
94-
// (number of allocs, ...) this implementation could be used.
95-
return std::accumulate(std::next(cont.begin()), cont.end(), o,
96-
[&delim](std::string a, const std::string &b) {
97-
return a.append(delim).append(b);
98-
});
99-
#else
100-
// add the first element directly
101-
auto it = std::next(cont.begin());
102-
const auto last = cont.end();
103-
104-
// all other elements with delim
105-
for (; it != last; ++it) {
106-
o += delim;
107-
108-
o += *it;
109-
}
110-
111-
return o;
112-
#endif
113-
}
114-
};
115-
116-
template <class Container>
117-
struct Join<Container, const char *> {
118-
static std::string impl(Container cont, const std::string &delim) {
119-
if (cont.begin() == cont.end()) return {};
120-
121-
return std::accumulate(std::next(cont.begin()), cont.end(),
122-
std::string(*(cont.begin())),
123-
[&delim](const std::string &a, const char *b) {
124-
return a + delim + b;
125-
});
126-
}
127-
};
49+
50+
// a simplified variant of the std::ranges::range concept
51+
//
52+
// C++20 ranges lib isn't fully available yet.
53+
54+
template <class R>
55+
concept range = requires(const R &rng) {
56+
std::begin(rng);
57+
std::end(rng);
58+
};
12859

12960
} // namespace detail
13061

13162
/**
132-
* join elements of an container into a string separated by a delimiter.
133-
*
134-
* Container MUST:
63+
* join elements of a range into a string separated by a delimiter.
13564
*
136-
* - have .begin() and end()
137-
* - ::iterator must be ForwardIterator + InputIterator
138-
* - ::value_type must be appendable to std::string
65+
* works with:
13966
*
140-
* should work with:
67+
* - std::vector, std::array, c-array, list, forward_list, deque
68+
* - and std::string, c-string, std::string_view
14169
*
142-
* - std::vector<const char *|std::string>
143-
* - std::array<const char *|std::string, N>
144-
* - std::list<const char *|std::string>
145-
*
146-
* @param cont a container
70+
* @param rng a range of strings
14771
* @param delim delimiter
148-
* @returns string of elements of container separated by delim
72+
* @returns string of elements of the range separated by delim
14973
*/
150-
template <class Container>
151-
std::string join(Container cont, const std::string &delim) {
152-
return detail::Join<Container, typename Container::value_type>::impl(cont,
153-
delim);
74+
std::string join(const detail::range auto &rng, std::string_view delim)
75+
requires(std::constructible_from<std::string, decltype(*std::begin(rng))>)
76+
{
77+
auto cur = std::begin(rng);
78+
const auto end = std::end(rng);
79+
80+
if (cur == end) return {}; // empty
81+
82+
std::string joined(*cur); // first element.
83+
84+
// append delim + element
85+
for (cur = std::next(cur); cur != end; cur = std::next(cur)) {
86+
joined.append(delim).append(*cur);
87+
}
88+
89+
return joined;
15490
}
91+
15592
/* Checks that given string belongs to the collection of strings */
15693
template <class T>
15794
constexpr bool str_in_collection(const T &t, const std::string_view &k) {

router/src/harness/tests/CMakeLists.txt

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ SET(TEST_MODULE harness)
5151

5252
SET(TESTS
5353
test_access_rights.cc
54-
test_utility_string.cc
5554
test_loader.cc
5655
test_loader_lifecycle.cc
5756
test_logging.cc
@@ -209,4 +208,12 @@ MYSQL_ADD_EXECUTABLE(linux_timestamping linux_timestamping.cc
209208
SKIP_INSTALL
210209
LINK_LIBRARIES
211210
harness_net_ts harness_stdx
211+
)
212+
213+
ADD_GOOGLETEST_FILE(test_utility_string.cc
214+
MODULE harness
215+
INCLUDE_DIRS ${MySQLRouter_SOURCE_DIR}/src/harness/include/
216+
INCLUDE_DIRS ${MySQLRouter_BINARY_DIR}/src/harness/include/
217+
EXTRA_SOURCES
218+
${CMAKE_SOURCE_DIR}/unittest/gunit/benchmark.cc
212219
)

router/src/harness/tests/test_utility_string.cc

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,16 @@
2828
#include <array>
2929
#include <deque>
3030
#include <forward_list>
31+
#include <initializer_list>
3132
#include <list>
3233
#include <set>
3334
#include <unordered_set>
3435
#include <vector>
3536

3637
#include "mysql/harness/utility/string.h"
3738

39+
#include "unittest/gunit/benchmark.h"
40+
3841
using mysql_harness::join;
3942

4043
template <typename T>
@@ -43,7 +46,16 @@ class JoinTest : public ::testing::Test {};
4346
TYPED_TEST_SUITE_P(JoinTest);
4447

4548
TYPED_TEST_P(JoinTest, many) {
46-
EXPECT_EQ(join(std::to_array<TypeParam>({"abc", "def"}), "-"), "abc-def");
49+
TypeParam c_array[]{
50+
"abc",
51+
"def",
52+
};
53+
54+
EXPECT_EQ(join(c_array, "-"), "abc-def");
55+
EXPECT_EQ(join(std::array<TypeParam, 2>{"abc", "def"}, "-"), "abc-def");
56+
EXPECT_EQ(join(std::initializer_list<const char *>{"abc", "def"}, "-"),
57+
"abc-def");
58+
4759
EXPECT_EQ(join(std::deque<TypeParam>{"abc", "def"}, "-"), "abc-def");
4860
EXPECT_EQ(join(std::forward_list<TypeParam>{"abc", "def"}, "-"), "abc-def");
4961
EXPECT_EQ(join(std::list<TypeParam>{"abc", "def"}, "-"), "abc-def");
@@ -59,7 +71,12 @@ TYPED_TEST_P(JoinTest, many) {
5971
}
6072

6173
TYPED_TEST_P(JoinTest, one) {
62-
EXPECT_EQ(join(std::to_array<TypeParam>({"abc"}), "-"), "abc");
74+
TypeParam c_array[1]{
75+
"abc",
76+
};
77+
EXPECT_EQ(join(c_array, "-"), "abc");
78+
79+
EXPECT_EQ(join(std::array<TypeParam, 1>{"abc"}, "-"), "abc");
6380
EXPECT_EQ(join(std::deque<TypeParam>{"abc"}, "-"), "abc");
6481
EXPECT_EQ(join(std::forward_list<TypeParam>{"abc"}, "-"), "abc");
6582
EXPECT_EQ(join(std::list<TypeParam>{"abc"}, "-"), "abc");
@@ -80,9 +97,53 @@ TYPED_TEST_P(JoinTest, none) {
8097

8198
REGISTER_TYPED_TEST_SUITE_P(JoinTest, many, one, none);
8299

83-
using JoinTestTypes = ::testing::Types<std::string, const char *>;
100+
using JoinTestTypes =
101+
::testing::Types<std::string, const char *, std::string_view>;
84102
INSTANTIATE_TYPED_TEST_SUITE_P(Spec, JoinTest, JoinTestTypes);
85103

104+
namespace {
105+
106+
using namespace std::string_view_literals;
107+
108+
template <class T, size_t N>
109+
constexpr auto init_bench_data() {
110+
std::array<T, N> data;
111+
112+
for (auto &el : data) {
113+
el = "fuzbuzshnuzz";
114+
}
115+
116+
return data;
117+
}
118+
119+
auto bench_ar_sv = init_bench_data<std::string_view, 1024>();
120+
auto bench_ar_cs = init_bench_data<const char *, 1024>();
121+
auto bench_ar_s = init_bench_data<std::string, 1024>();
122+
123+
void BenchJoinStdArrayStringView(size_t iter) {
124+
while ((iter--) != 0) {
125+
std::string joined = join(bench_ar_sv, ", ");
126+
}
127+
}
128+
129+
void BenchJoinStdArrayCString(size_t iter) {
130+
while ((iter--) != 0) {
131+
std::string joined = join(bench_ar_cs, ", ");
132+
}
133+
}
134+
135+
void BenchJoinStdArrayStdString(size_t iter) {
136+
while ((iter--) != 0) {
137+
std::string joined = join(bench_ar_s, ", ");
138+
}
139+
}
140+
141+
} // namespace
142+
143+
BENCHMARK(BenchJoinStdArrayStdString)
144+
BENCHMARK(BenchJoinStdArrayStringView)
145+
BENCHMARK(BenchJoinStdArrayCString)
146+
86147
int main(int argc, char *argv[]) {
87148
::testing::InitGoogleTest(&argc, argv);
88149
return RUN_ALL_TESTS();

router/src/metadata_cache/src/metadata_cache_plugin.cc

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,18 @@
3030
#include <array>
3131
#include <stdexcept>
3232
#include <string>
33-
#include <thread>
3433

35-
#include "dim.h"
3634
#include "keyring/keyring_manager.h"
3735
#include "metadata_cache.h"
3836
#include "my_thread.h" // my_thread_self_setname
3937
#include "mysql/harness/config_parser.h"
4038
#include "mysql/harness/dynamic_config.h"
4139
#include "mysql/harness/loader_config.h"
4240
#include "mysql/harness/logging/logging.h"
43-
#include "mysql/harness/utility/string.h"
4441
#include "mysqlrouter/mysql_client_thread_token.h"
4542
#include "mysqlrouter/mysql_session.h" // kSslModePreferred
4643
#include "mysqlrouter/supported_metadata_cache_options.h"
4744
#include "mysqlrouter/uri.h"
48-
#include "mysqlrouter/utils.h"
4945
#include "plugin_config.h"
5046

5147
IMPORT_LOG_FUNCTIONS()
@@ -71,10 +67,8 @@ static metadata_cache::RouterAttributes get_router_attributes(
7167

7268
const bool is_rw_split = routing_section->has("access_mode") &&
7369
routing_section->get("access_mode") == "auto";
74-
const bool is_rw = !is_rw_split && mysql_harness::utility::ends_with(
75-
destinations, "PRIMARY");
76-
const bool is_ro = !is_rw_split && mysql_harness::utility::ends_with(
77-
destinations, "SECONDARY");
70+
const bool is_rw = !is_rw_split && destinations.ends_with("PRIMARY");
71+
const bool is_ro = !is_rw_split && destinations.ends_with("SECONDARY");
7872

7973
if (protocol == "classic") {
8074
if (is_rw_split)

router/src/router/src/router_app.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1970,7 +1970,7 @@ void MySQLRouter::show_help() {
19701970

19711971
// fallback to .ini for each .conf file
19721972
const std::string conf_ext(".conf");
1973-
if (mysql_harness::utility::ends_with(file, conf_ext)) {
1973+
if (file.ends_with(conf_ext)) {
19741974
// replace .conf by .ini
19751975
std::string ini_filename =
19761976
file.substr(0, file.size() - conf_ext.size()) + ".ini";

router/tests/component/test_router_configuration_errors.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
#include <gmock/gmock.h>
2727

2828
#include "config_builder.h"
29-
#include "mysql/harness/utility/string.h"
3029
#include "router_component_test.h"
3130
#include "test/temp_directory.h"
3231

@@ -456,12 +455,12 @@ TEST_F(RouterCmdlineTest, help_output_is_sane) {
456455
if (line.empty()) {
457456
break;
458457
}
459-
if (mysql_harness::utility::starts_with(line, indent)) {
458+
if (line.starts_with(indent)) {
460459
auto file = line.substr(indent.size(), line.size());
461460
config_files.push_back(file);
462461
}
463462
}
464-
if (mysql_harness::utility::starts_with(line, "Configuration read")) {
463+
if (line.starts_with("Configuration read")) {
465464
it++; // skip next line
466465
found = true;
467466
}

router/tests/helpers/process_wrapper.h

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,14 @@
2626
#ifndef _PROCESS_WRAPPER_H_
2727
#define _PROCESS_WRAPPER_H_
2828

29-
#include "mysql/harness/string_utils.h" // split_string
30-
#include "mysql/harness/utility/string.h" // starts_with
31-
#include "process_launcher.h"
32-
#include "router_test_helpers.h"
33-
3429
#include <atomic>
35-
#include <cstring>
3630
#include <mutex>
3731
#include <optional>
3832
#include <thread>
3933

40-
using mysql_harness::Path;
34+
#include "mysql/harness/string_utils.h" // split_string
35+
#include "process_launcher.h"
36+
#include "router_test_helpers.h"
4137

4238
// test performance tweaks
4339
// shorter timeout -> faster test execution, longer timeout -> increased test
@@ -90,7 +86,6 @@ class ProcessWrapper {
9086
* of calling this method.
9187
*/
9288
std::string get_full_output() {
93-
using mysql_harness::utility::starts_with;
9489
std::vector<std::string> lines;
9590
std::string result;
9691

@@ -108,13 +103,13 @@ class ProcessWrapper {
108103
for (const std::string &line : lines) {
109104
// setrlimit sometimes fails on pb2 macos, resulting in additional,
110105
// unwanted console output that we remove here
111-
if (starts_with(line,
112-
"NOTE: core-file requested, but resource-limits say")) {
106+
if (line.starts_with(
107+
"NOTE: core-file requested, but resource-limits say")) {
113108
core_file_req_failed = true;
114109
continue;
115110
}
116111
if (core_file_req_failed &&
117-
starts_with(line, "stopping to log to the console") &&
112+
line.starts_with("stopping to log to the console") &&
118113
lines.size() == 2) {
119114
continue;
120115
}

0 commit comments

Comments
 (0)