This is an automated email from the ASF dual-hosted git repository.
wangdan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git
The following commit(s) were added to refs/heads/master by this push:
new fdbd6ebb4 feat(utils): update `get_last_component()` to support
`std::string_view` (#2299)
fdbd6ebb4 is described below
commit fdbd6ebb460b97d39c027837217c1a41de1b3671
Author: Dan Wang <[email protected]>
AuthorDate: Mon Oct 13 11:39:39 2025 +0800
feat(utils): update `get_last_component()` to support `std::string_view`
(#2299)
Update `get_last_component()` to support `std::string_view`, making it
adaptable
to different use cases such as `std::string_view`, `std::string`, and
C-style strings.
This enhancement allows the function to be used in various scenarios,
including parsing
log file names, replica data directory names, and ZooKeeper node names.
Additionally, the implementation of `get_last_component()` has been
refactored to
leverage `std::string_view`’s built-in methods for cleaner and more
efficient code.
---
src/replica/log_file.cpp | 119 ++++++++++++------------
src/replica/log_file.h | 4 +-
src/replica/replica_stub.cpp | 17 ++--
src/replica/replica_stub.h | 6 +-
src/replica/test/log_file_test.cpp | 115 ++++++++++++++++++++++-
src/replica/test/replica_dir_test.cpp | 47 ++++++++--
src/replica/test/replica_disk_migrate_test.cpp | 4 +-
src/utils/strings.cpp | 18 +---
src/utils/strings.h | 3 +-
src/utils/test/utils.cpp | 122 +++++++++++++++++++++++--
src/zookeeper/lock_struct.cpp | 4 +-
11 files changed, 348 insertions(+), 111 deletions(-)
diff --git a/src/replica/log_file.cpp b/src/replica/log_file.cpp
index ab88a3e2e..9ebb16e09 100644
--- a/src/replica/log_file.cpp
+++ b/src/replica/log_file.cpp
@@ -26,10 +26,11 @@
#include "log_file.h"
-#include <inttypes.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
+#include <fmt/core.h>
+#include <cinttypes>
+#include <cstdio>
+#include <cstring>
+#include <string_view>
#include <utility>
#include <vector>
@@ -46,6 +47,7 @@
#include "utils/fmt_logging.h"
#include "utils/latency_tracer.h"
#include "utils/ports.h"
+#include "utils/string_conv.h"
#include "utils/strings.h"
namespace dsn {
@@ -55,90 +57,91 @@ class task_tracker;
namespace replication {
log_file::~log_file() { close(); }
-/*static */ log_file_ptr log_file::open_read(const char *path, /*out*/
error_code &err)
+
+#define LOG_FILE_NAME_VALIDATOR(expr, message, path)
\
+ LOG_AND_RETURN_NOT_TRUE(WARNING, expr, dsn::ERR_INVALID_PARAMETERS,
message, path);
+
+#define VALIDATE_LOG_FILE_NAME(validator, expr, path)
\
+ validator(expr,
\
+ "invalid log file name (its format should be
log.<index>.<start_offset>): " \
+ "path = {}",
\
+ path)
+
+#define RETURN_IF_LOG_FILE_NAME_INVALID(expr, path)
\
+ VALIDATE_LOG_FILE_NAME(LOG_FILE_NAME_VALIDATOR, expr, path)
+
+#define CHECK_LOG_FILE_NAME(expr, path) VALIDATE_LOG_FILE_NAME(CHECK, expr,
path)
+
+/*static*/ error_code
+log_file::parse_log_file_name(const char *path, int &index, int64_t
&start_offset)
{
- char splitters[] = {'\\', '/', 0};
- std::string name = utils::get_last_component(std::string(path), splitters);
+ const auto file_name = dsn::utils::get_last_component(path, "\\/");
- // log.index.start_offset
- if (name.length() < strlen("log.") || name.substr(0, strlen("log.")) !=
std::string("log.")) {
- err = ERR_INVALID_PARAMETERS;
- LOG_WARNING("invalid log path {}", path);
- return nullptr;
- }
+ // The format of the log file name: log.<index>.<start_offset>
+ constexpr std::string_view kLogPrefix("log.");
- auto pos = name.find_first_of('.');
- CHECK(pos != std::string::npos, "invalid log_file, name = {}", name);
- auto pos2 = name.find_first_of('.', pos + 1);
- if (pos2 == std::string::npos) {
- err = ERR_INVALID_PARAMETERS;
- LOG_WARNING("invalid log path {}", path);
- return nullptr;
- }
+ // TODO(wangdan): from C++20, consider using
std::string_view::starts_with() instead.
+ RETURN_IF_LOG_FILE_NAME_INVALID(file_name.rfind(kLogPrefix, 0) == 0, path);
- /* so the log file format is log.index_str.start_offset_str */
- std::string index_str = name.substr(pos + 1, pos2 - pos - 1);
- std::string start_offset_str = name.substr(pos2 + 1);
- if (index_str.empty() || start_offset_str.empty()) {
- err = ERR_INVALID_PARAMETERS;
- LOG_WARNING("invalid log path {}", path);
- return nullptr;
- }
+ const auto begin = kLogPrefix.size();
+ const auto end = file_name.find_first_of('.', begin);
+ RETURN_IF_LOG_FILE_NAME_INVALID(end != std::string::npos, path);
- char *p = nullptr;
- int index = static_cast<int>(strtol(index_str.c_str(), &p, 10));
- if (*p != 0) {
- err = ERR_INVALID_PARAMETERS;
- LOG_WARNING("invalid log path {}", path);
- return nullptr;
- }
- int64_t start_offset =
static_cast<int64_t>(strtoll(start_offset_str.c_str(), &p, 10));
- if (*p != 0) {
- err = ERR_INVALID_PARAMETERS;
- LOG_WARNING("invalid log path {}", path);
- return nullptr;
+ const auto index_str = file_name.substr(begin, end - begin);
+ const auto start_offset_str = file_name.substr(end + 1);
+ RETURN_IF_LOG_FILE_NAME_INVALID(!index_str.empty() &&
!start_offset_str.empty(), path);
+ RETURN_IF_LOG_FILE_NAME_INVALID(dsn::buf2numeric(index_str, index), path);
+ RETURN_IF_LOG_FILE_NAME_INVALID(dsn::buf2numeric(start_offset_str,
start_offset), path);
+
+ return ERR_OK;
+}
+
+/*static*/ log_file_ptr log_file::open_read(const char *path, /*out*/
error_code &err)
+{
+ int index{0};
+ int64_t start_offset{0};
+ err = parse_log_file_name(path, index, start_offset);
+ if (err != ERR_OK) {
+ return {};
}
disk_file *hfile = file::open(path, file::FileOpenType::kReadOnly);
- if (!hfile) {
+ if (hfile == nullptr) {
err = ERR_FILE_OPERATION_FAILED;
LOG_WARNING("open log file {} failed", path);
- return nullptr;
+ return {};
}
- auto lf = new log_file(path, hfile, index, start_offset, true);
+ log_file_ptr lf(new log_file(path, hfile, index, start_offset, true));
lf->reset_stream();
+
blob hdr_blob;
err = lf->read_next_log_block(hdr_blob);
if (err == ERR_INVALID_DATA || err == ERR_INCOMPLETE_DATA || err ==
ERR_HANDLE_EOF ||
err == ERR_FILE_OPERATION_FAILED) {
- std::string removed = std::string(path) + ".removed";
+ const auto new_path = fmt::format("{}.removed", path);
LOG_ERROR("read first log entry of file {} failed, err = {}. Rename
the file to {}",
path,
err,
- removed);
- delete lf;
- lf = nullptr;
+ new_path);
- // rename file on failure
- dsn::utils::filesystem::rename_path(path, removed);
+ // Rename file on failure.
+ dsn::utils::filesystem::rename_path(path, new_path);
- return nullptr;
+ return {};
}
binary_reader reader(std::move(hdr_blob));
lf->read_file_header(reader);
if (!lf->is_right_header()) {
- std::string removed = std::string(path) + ".removed";
- LOG_ERROR("invalid log file header of file {}. Rename the file to {}",
path, removed);
- delete lf;
- lf = nullptr;
+ const auto new_path = fmt::format("{}.removed", path);
+ LOG_ERROR("invalid log file header of file {}. Rename the file to {}",
path, new_path);
- // rename file on failure
- dsn::utils::filesystem::rename_path(path, removed);
+ // Rename file on failure.
+ dsn::utils::filesystem::rename_path(path, new_path);
err = ERR_INVALID_DATA;
- return nullptr;
+ return {};
}
err = ERR_OK;
diff --git a/src/replica/log_file.h b/src/replica/log_file.h
index 74700310e..57c1115f6 100644
--- a/src/replica/log_file.h
+++ b/src/replica/log_file.h
@@ -209,10 +209,12 @@ public:
const disk_file *file_handle() const { return _handle; }
private:
+ static error_code parse_log_file_name(const char *path, int &index,
int64_t &start_offset);
+
// make private, user should create log_file through open_read() or
open_write()
log_file(const char *path, disk_file *handle, int index, int64_t
start_offset, bool is_read);
-private:
+ friend class ParseLogFileNameTest;
friend class mock_log_file;
uint32_t _crc32;
diff --git a/src/replica/replica_stub.cpp b/src/replica/replica_stub.cpp
index 5d64de07f..7f0b6cfed 100644
--- a/src/replica/replica_stub.cpp
+++ b/src/replica/replica_stub.cpp
@@ -2247,24 +2247,23 @@ replica *replica_stub::new_replica(gpid gpid,
return new_replica(gpid, app, restore_if_necessary,
is_duplication_follower, {});
}
-/*static*/ std::string replica_stub::get_replica_dir_name(const std::string
&dir)
+/*static*/ std::string_view
replica_stub::get_replica_dir_name(std::string_view dir)
{
- static const char splitters[] = {'\\', '/', 0};
- return utils::get_last_component(dir, splitters);
+ return utils::get_last_component(dir, "\\/");
}
-/* static */ bool
-replica_stub::parse_replica_dir_name(const std::string &dir_name, gpid &pid,
std::string &app_type)
+/*static*/ bool
+replica_stub::parse_replica_dir_name(std::string_view dir_name, gpid &pid,
std::string &app_type)
{
std::vector<uint32_t> ids(2, 0);
size_t begin = 0;
for (auto &id : ids) {
- size_t end = dir_name.find('.', begin);
+ const size_t end = dir_name.find('.', begin);
if (end == std::string::npos) {
return false;
}
- if (!buf2uint32(std::string_view(dir_name.data() + begin, end -
begin), id)) {
+ if (!buf2uint32(dir_name.substr(begin, end - begin), id)) {
return false;
}
@@ -2278,9 +2277,7 @@ replica_stub::parse_replica_dir_name(const std::string
&dir_name, gpid &pid, std
pid.set_app_id(static_cast<int32_t>(ids[0]));
pid.set_partition_index(static_cast<int32_t>(ids[1]));
- // TODO(wangdan): the 3rd parameter `count` does not support default
argument for CentOS 7
- // (gcc 7.3.1). After CentOS 7 is deprecated, consider dropping
std::string::npos.
- app_type.assign(dir_name, begin, std::string::npos);
+ app_type = dir_name.substr(begin);
return true;
}
diff --git a/src/replica/replica_stub.h b/src/replica/replica_stub.h
index 57070215b..10f831949 100644
--- a/src/replica/replica_stub.h
+++ b/src/replica/replica_stub.h
@@ -34,6 +34,7 @@
#include <map>
#include <memory>
#include <string>
+#include <string_view>
#include <tuple>
#include <unordered_map>
#include <utility>
@@ -394,11 +395,10 @@ private:
// Get the replica dir name from a potentially longer path (`dir` could be
an absolute
// or relative path).
- static std::string get_replica_dir_name(const std::string &dir);
+ static std::string_view get_replica_dir_name(std::string_view dir);
// Parse app id, partition id and app type from the replica dir name.
- static bool
- parse_replica_dir_name(const std::string &dir_name, gpid &pid, std::string
&app_type);
+ static bool parse_replica_dir_name(std::string_view dir_name, gpid &pid,
std::string &app_type);
// Load an existing replica which is located in `dn` with `replica_dir`.
Usually each
// different `dn` represents a unique disk. `replica_dir` is the absolute
path of the
diff --git a/src/replica/test/log_file_test.cpp
b/src/replica/test/log_file_test.cpp
index c4abb02b2..bb2b2581d 100644
--- a/src/replica/test/log_file_test.cpp
+++ b/src/replica/test/log_file_test.cpp
@@ -15,7 +15,8 @@
// specific language governing permissions and limitations
// under the License.
-#include <stddef.h>
+#include <cstddef>
+#include <cstdint>
#include <memory>
#include <string>
#include <vector>
@@ -30,8 +31,113 @@
#include "utils/error_code.h"
#include "utils/filesystem.h"
-namespace dsn {
-namespace replication {
+namespace dsn::replication {
+
+struct parse_log_file_name_case
+{
+ const char *path;
+ error_code expected_err;
+ int expected_index;
+ int64_t expected_start_offset;
+};
+
+class ParseLogFileNameTest : public
testing::TestWithParam<parse_log_file_name_case>
+{
+public:
+ static void test_parse_log_file_name()
+ {
+ const auto &test_case = GetParam();
+
+ int actual_index{0};
+ int64_t actual_start_offset{0};
+ ASSERT_EQ(test_case.expected_err,
+ log_file::parse_log_file_name(test_case.path, actual_index,
actual_start_offset));
+
+ if (test_case.expected_err != ERR_OK) {
+ return;
+ }
+
+ EXPECT_EQ(test_case.expected_index, actual_index);
+ EXPECT_EQ(test_case.expected_start_offset, actual_start_offset);
+ }
+};
+
+TEST_P(ParseLogFileNameTest, ParseLogFileName) { test_parse_log_file_name(); }
+
+const std::vector<parse_log_file_name_case> parse_log_file_name_tests{
+ // Empty file name.
+ {"", ERR_INVALID_PARAMETERS, 0, 0},
+
+ // Invalid prefix.
+ {".", ERR_INVALID_PARAMETERS, 0, 0},
+ {"g", ERR_INVALID_PARAMETERS, 0, 0},
+ {".g", ERR_INVALID_PARAMETERS, 0, 0},
+ {".gol", ERR_INVALID_PARAMETERS, 0, 0},
+ {"lo", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log_", ERR_INVALID_PARAMETERS, 0, 0},
+ {"logs.", ERR_INVALID_PARAMETERS, 0, 0},
+
+ // No field.
+ {"log.", ERR_INVALID_PARAMETERS, 0, 0},
+
+ // Only one field.
+ {"log.0", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log_0", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.012", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log_012", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.1", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log_1", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.123", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log_123", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log..0", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log..1", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log..123", ERR_INVALID_PARAMETERS, 0, 0},
+
+ // Empty fields.
+ {"log._", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log..", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log...", ERR_INVALID_PARAMETERS, 0, 0},
+
+ // Invalid splitters.
+ {"log.0_0", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log_0.0", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.0_1", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log_0.1", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.1_0", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log_1.0", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.1_1", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log_1.1", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.123_456", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log_123.456", ERR_INVALID_PARAMETERS, 0, 0},
+
+ // Invalid characters.
+ {"log.123.abc", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.123.a12", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.123.1a2", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.123.12a", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.abc.123", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.a12.123", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.1a2.123", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.12a.123", ERR_INVALID_PARAMETERS, 0, 0},
+
+ // Too many fields.
+ {"log.123.456.789", ERR_INVALID_PARAMETERS, 0, 0},
+
+ // Numbers that overflow.
+ {"log.2147483648.123", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.123.9223372036854775808", ERR_INVALID_PARAMETERS, 0, 0},
+ {"log.2147483648.9223372036854775808", ERR_INVALID_PARAMETERS, 0, 0},
+
+ // Valid fields.
+ {"log.123.456", ERR_OK, 123, 456},
+ {"log.2147483647.123", ERR_OK, 2147483647, 123},
+ {"log.123.9223372036854775807", ERR_OK, 123, 9223372036854775807},
+};
+
+INSTANTIATE_TEST_SUITE_P(LogFileTest,
+ ParseLogFileNameTest,
+ testing::ValuesIn(parse_log_file_name_tests));
class log_file_test : public replica_test_base
{
@@ -99,5 +205,4 @@ TEST_P(log_file_test, commit_log_blocks)
ASSERT_EQ(tsk->get_aio_context()->file_offset, appender->start_offset() -
_start_offset);
}
-} // namespace replication
-} // namespace dsn
+} // namespace dsn::replication
diff --git a/src/replica/test/replica_dir_test.cpp
b/src/replica/test/replica_dir_test.cpp
index 3bc64d74f..12ff39411 100644
--- a/src/replica/test/replica_dir_test.cpp
+++ b/src/replica/test/replica_dir_test.cpp
@@ -16,6 +16,7 @@
// under the License.
#include <string>
+#include <string_view>
#include <vector>
#include "common/gpid.h"
@@ -26,22 +27,35 @@ namespace dsn::replication {
struct get_replica_dir_name_case
{
- std::string path;
- std::string expected_replica_dir_name;
+ const char *path;
+ std::string_view expected_replica_dir_name;
};
class GetReplicaDirNameTest : public
testing::TestWithParam<get_replica_dir_name_case>
{
public:
+ template <typename TPath>
static void test_get_replica_dir_name()
{
const auto &test_case = GetParam();
- const auto &actual_replica_dir_name =
replica_stub::get_replica_dir_name(test_case.path);
+
+ const TPath path(test_case.path);
+ const auto actual_replica_dir_name =
replica_stub::get_replica_dir_name(path);
EXPECT_EQ(test_case.expected_replica_dir_name,
actual_replica_dir_name);
}
};
-TEST_P(GetReplicaDirNameTest, GetReplicaDirName) {
test_get_replica_dir_name(); }
+TEST_P(GetReplicaDirNameTest, GetReplicaDirNameCString)
+{
+ test_get_replica_dir_name<const char *>();
+}
+
+TEST_P(GetReplicaDirNameTest, GetReplicaDirNameString) {
test_get_replica_dir_name<std::string>(); }
+
+TEST_P(GetReplicaDirNameTest, GetReplicaDirNameStringView)
+{
+ test_get_replica_dir_name<std::string_view>();
+}
const std::vector<get_replica_dir_name_case> get_replica_dir_name_tests{
// Linux absolute path and non-empty dir name.
@@ -68,7 +82,7 @@ INSTANTIATE_TEST_SUITE_P(ReplicaDirTest,
struct parse_replica_dir_name_case
{
- std::string replica_dir_name;
+ const char *replica_dir_name;
bool ok;
gpid expected_pid;
std::string expected_app_type;
@@ -77,15 +91,17 @@ struct parse_replica_dir_name_case
class ParseReplicaDirNameTest : public
testing::TestWithParam<parse_replica_dir_name_case>
{
public:
+ template <typename TDirName>
static void test_parse_replica_dir_name()
{
const auto &test_case = GetParam();
+ const TDirName replica_dir_name(test_case.replica_dir_name);
gpid actual_pid;
std::string actual_app_type;
- ASSERT_EQ(test_case.ok,
- replica_stub::parse_replica_dir_name(
- test_case.replica_dir_name, actual_pid,
actual_app_type));
+ ASSERT_EQ(
+ test_case.ok,
+ replica_stub::parse_replica_dir_name(replica_dir_name, actual_pid,
actual_app_type));
if (!test_case.ok) {
return;
}
@@ -95,7 +111,20 @@ public:
}
};
-TEST_P(ParseReplicaDirNameTest, ParseReplicaDirName) {
test_parse_replica_dir_name(); }
+TEST_P(ParseReplicaDirNameTest, ParseReplicaDirNameCString)
+{
+ test_parse_replica_dir_name<const char *>();
+}
+
+TEST_P(ParseReplicaDirNameTest, ParseReplicaDirNameString)
+{
+ test_parse_replica_dir_name<std::string>();
+}
+
+TEST_P(ParseReplicaDirNameTest, ParseReplicaDirNameStringView)
+{
+ test_parse_replica_dir_name<std::string_view>();
+}
const std::vector<parse_replica_dir_name_case> parse_replica_dir_name_tests{
// Empty dir name.
diff --git a/src/replica/test/replica_disk_migrate_test.cpp
b/src/replica/test/replica_disk_migrate_test.cpp
index df410925e..63298e147 100644
--- a/src/replica/test/replica_disk_migrate_test.cpp
+++ b/src/replica/test/replica_disk_migrate_test.cpp
@@ -165,7 +165,9 @@ TEST_P(replica_disk_migrate_test, on_migrate_replica)
request.origin_disk = "tag_1";
request.target_disk = "tag_2";
stub->on_disk_migrate(fake_migrate_rpc);
- get_replica(request.pid)->tracker()->wait_outstanding_tasks();
+ auto rep = get_replica(request.pid);
+ ASSERT_TRUE(rep != nullptr);
+ rep->tracker()->wait_outstanding_tasks();
ASSERT_EQ(response.err, ERR_OK);
}
diff --git a/src/utils/strings.cpp b/src/utils/strings.cpp
index 85e32936c..240753645 100644
--- a/src/utils/strings.cpp
+++ b/src/utils/strings.cpp
@@ -159,22 +159,10 @@ error_s pattern_match(const std::string &str,
return error_s::ok();
}
-std::string get_last_component(const std::string &input, const char
splitters[])
+std::string_view get_last_component(std::string_view str, std::string_view
splitters)
{
- int index = -1;
- const char *s = splitters;
-
- while (*s != 0) {
- auto pos = input.find_last_of(*s);
- if (pos != std::string::npos && (static_cast<int>(pos) > index))
- index = static_cast<int>(pos);
- s++;
- }
-
- if (index != -1)
- return input.substr(index + 1);
- else
- return input;
+ const auto pos = str.find_last_of(splitters);
+ return (pos == std::string_view::npos) ? str : str.substr(pos + 1);
}
namespace {
diff --git a/src/utils/strings.h b/src/utils/strings.h
index 8972e78ec..1a5008b24 100644
--- a/src/utils/strings.h
+++ b/src/utils/strings.h
@@ -31,6 +31,7 @@
#include <list>
#include <map>
#include <string>
+#include <string_view>
#include <unordered_set>
#include <vector>
@@ -132,7 +133,7 @@ std::string kv_map_to_string(const std::map<std::string,
std::string> &kv_map,
std::string
replace_string(std::string subject, const std::string &search, const
std::string &replace);
-std::string get_last_component(const std::string &input, const char
splitters[]);
+std::string_view get_last_component(std::string_view str, std::string_view
splitters);
char *trim_string(char *s);
diff --git a/src/utils/test/utils.cpp b/src/utils/test/utils.cpp
index d1888a7f5..6ec6fdc20 100644
--- a/src/utils/test/utils.cpp
+++ b/src/utils/test/utils.cpp
@@ -29,6 +29,7 @@
#include <map>
#include <set>
#include <string>
+#include <string_view>
#include <tuple>
#include <unordered_set>
#include <utility>
@@ -49,15 +50,124 @@
namespace dsn::utils {
-TEST(core, get_last_component)
+struct get_last_component_case
{
- ASSERT_EQ("a", get_last_component("a", "/"));
- ASSERT_EQ("b", get_last_component("a/b", "/"));
- ASSERT_EQ("b", get_last_component("a//b", "/"));
- ASSERT_EQ("", get_last_component("a/", "/"));
- ASSERT_EQ("c", get_last_component("a/b_c", "/_"));
+ const char *str;
+ const char *splitters;
+ std::string_view expected_result;
+};
+
+class GetLastComponentTest : public
testing::TestWithParam<get_last_component_case>
+{
+protected:
+ template <typename TStr>
+ void test_get_last_component()
+ {
+ const auto &test_case = GetParam();
+
+ const TStr str(test_case.str);
+ const auto actual_result = get_last_component(str,
test_case.splitters);
+ EXPECT_EQ(test_case.expected_result, actual_result);
+ }
+};
+
+TEST_P(GetLastComponentTest, GetLastComponentCString) {
test_get_last_component<const char *>(); }
+
+TEST_P(GetLastComponentTest, GetLastComponentString) {
test_get_last_component<std::string>(); }
+
+TEST_P(GetLastComponentTest, GetLastComponentStringView)
+{
+ test_get_last_component<std::string_view>();
}
+const std::vector<get_last_component_case> get_last_component_tests = {
+ // Empty string.
+ {"", "", ""},
+ {"", "/", ""},
+ {"", "\\/", ""},
+ {"/", "", "/"},
+
+ // The only character is a splitter.
+ {"/", "/", ""},
+ {"/", "\\/", ""},
+
+ // The only character is not a splitter.
+ {"a", "/", "a"},
+ {"a", "\\/", "a"},
+ {"/", "\\", "/"},
+
+ // There is not any splitter in multiple characters.
+ {"aa", "/", "aa"},
+ {"aa", "\\/", "aa"},
+ {"ab", "/", "ab"},
+ {"ab", "\\/", "ab"},
+ {"abc", "/", "abc"},
+ {"abc", "\\/", "abc"},
+
+ // There is only one splitter in multiple characters.
+ {"a/", "/", ""},
+ {"/a", "/", "a"},
+ {"a/", "\\/", ""},
+ {"/a", "\\/", "a"},
+ {"aa/", "/", ""},
+ {"a/a", "/", "a"},
+ {"/aa", "/", "aa"},
+ {"aa/", "\\/", ""},
+ {"a/a", "\\/", "a"},
+ {"/aa", "\\/", "aa"},
+ {"ab/", "/", ""},
+ {"a/b", "/", "b"},
+ {"/ab", "/", "ab"},
+ {"abc/", "\\/", ""},
+ {"ab/c", "\\/", "c"},
+ {"a/bc", "\\/", "bc"},
+ {"/abc", "\\/", "abc"},
+ {"abc\\", "\\/", ""},
+ {"ab\\c", "\\/", "c"},
+ {"a\\bc", "\\/", "bc"},
+ {"\\abc", "\\/", "abc"},
+
+ // There are adjacent splitters in multiple characters.
+ {"a//", "/", ""},
+ {"a/\\", "\\/", ""},
+ {"//a", "/", "a"},
+ {"\\/a", "\\/", "a"},
+ {"aa//", "/", ""},
+ {"aa\\/", "/\\", ""},
+ {"a//a", "/", "a"},
+ {"a/\\a", "/\\", "a"},
+ {"//aa", "/", "aa"},
+ {"\\/aa", "/\\", "aa"},
+ {"ab//", "/", ""},
+ {"ab/\\", "\\/", ""},
+ {"a//b", "/", "b"},
+ {"a\\/b", "\\/", "b"},
+ {"//ab", "/", "ab"},
+ {"/\\ab", "\\/", "ab"},
+ {"abc//", "/", ""},
+ {"abc\\/", "\\/", ""},
+ {"ab//c", "/", "c"},
+ {"ab/\\c", "\\/", "c"},
+ {"a//bc", "/", "bc"},
+ {"a\\/bc", "/\\", "bc"},
+ {"//abc", "/", "abc"},
+ {"\\/abc", "/\\", "abc"},
+
+ // There are multiple splitters in multiple characters.
+ {"\\a/", "/\\", ""},
+ {"a\\a/", "/\\", ""},
+ {"/aa\\", "/\\", ""},
+ {"a/b\\", "/\\", ""},
+ {"\\ab/", "/\\", ""},
+ {"a/b/c", "/", "c"},
+ {"a\\b/c", "/\\", "c"},
+ {"ab/cd\\efg", "/\\", "efg"},
+};
+
+INSTANTIATE_TEST_SUITE_P(StringTest,
+ GetLastComponentTest,
+ testing::ValuesIn(get_last_component_tests));
+
TEST(core, crc)
{
char buffer[24];
diff --git a/src/zookeeper/lock_struct.cpp b/src/zookeeper/lock_struct.cpp
index 7ef0aee1a..2f0c0073d 100644
--- a/src/zookeeper/lock_struct.cpp
+++ b/src/zookeeper/lock_struct.cpp
@@ -30,6 +30,7 @@
#include <functional>
#include <memory>
#include <string>
+#include <string_view>
#include <type_traits>
#include <utility>
#include <vector>
@@ -613,8 +614,7 @@ void lock_struct::after_create_locknode(lock_struct_ptr
_this,
return;
}
- char splitter[] = {'/', 0};
- _this->_myself._node_seq_name = utils::get_last_component(*path, splitter);
+ _this->_myself._node_seq_name = utils::get_last_component(*path, "/");
_this->_myself._sequence_id =
parse_seq_path(_this->_myself._node_seq_name);
CHECK_NE_MSG(_this->_myself._sequence_id, -1, "invalid seq path created");
LOG_INFO("create seq/ephe node in dir({}) ok, my_sequence_id({})",
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
