From fc27a4f852d8aad6aa9e769bd988f66833fe8bff Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 7 Sep 2024 09:34:45 +0200 Subject: [PATCH] Apply all separators, not only the first one found, ref #518 --- src/libs/core/impl/String.cpp | 60 ++++++++++++++++++++------- src/libs/core/include/core/String.hpp | 2 + src/libs/core/test/String.cpp | 32 ++++++++++++++ src/libs/database/impl/Migration.cpp | 2 +- src/libs/metadata/impl/Parser.cpp | 4 +- src/libs/metadata/test/Parser.cpp | 18 ++++---- 6 files changed, 90 insertions(+), 28 deletions(-) diff --git a/src/libs/core/impl/String.cpp b/src/libs/core/impl/String.cpp index a891e766..226caa1d 100644 --- a/src/libs/core/impl/String.cpp +++ b/src/libs/core/impl/String.cpp @@ -97,6 +97,41 @@ namespace lms::core::stringUtils return res; } + + template + std::vector splitString(std::string_view str, std::span separators) + { + std::vector res; + + size_t currentPos{}; + while (currentPos < str.size()) + { + size_t nextSeparatorPos{ std::string_view::npos }; + size_t sepLen{}; + + for (const std::string_view sep : separators) + { + if (sep.empty()) + continue; + + size_t found{ str.find(sep, currentPos) }; + if (found < nextSeparatorPos) + { + nextSeparatorPos = found; + sepLen = sep.size(); + } + } + + if (nextSeparatorPos == std::string_view::npos) + break; + + res.push_back(str.substr(currentPos, nextSeparatorPos - currentPos)); + currentPos = nextSeparatorPos + sepLen; + } + + res.push_back(str.substr(currentPos)); + return res; + } } // namespace details template<> @@ -129,24 +164,17 @@ namespace lms::core::stringUtils std::vector splitString(std::string_view str, std::string_view separator) { - std::vector res; + return splitString(str, std::span(&separator, 1)); + } - if (separator.empty()) - return { str }; + std::vector splitString(std::string_view str, std::span separators) + { + return details::splitString(str, separators); + } - size_t pos{}; - size_t found{ str.find(separator) }; - - while (found != std::string_view::npos) - { - res.push_back(str.substr(pos, found - pos)); - pos = found + separator.size(); - found = str.find(separator, pos); - } - - res.push_back(str.substr(pos)); - - return res; + std::vector splitString(std::string_view str, std::span separators) + { + return details::splitString(str, separators); } std::string joinStrings(std::span strings, std::string_view delimiter) diff --git a/src/libs/core/include/core/String.hpp b/src/libs/core/include/core/String.hpp index 2cf47885..590e0034 100644 --- a/src/libs/core/include/core/String.hpp +++ b/src/libs/core/include/core/String.hpp @@ -40,6 +40,8 @@ namespace lms::core::stringUtils { [[nodiscard]] std::vector splitString(std::string_view string, char separator); [[nodiscard]] std::vector splitString(std::string_view string, std::string_view separator); + [[nodiscard]] std::vector splitString(std::string_view string, std::span separators); + [[nodiscard]] std::vector splitString(std::string_view string, std::span separators); [[nodiscard]] std::string joinStrings(std::span strings, std::string_view delimiter); [[nodiscard]] std::string joinStrings(std::span strings, std::string_view delimiter); diff --git a/src/libs/core/test/String.cpp b/src/libs/core/test/String.cpp index 142145c8..2e9789ef 100644 --- a/src/libs/core/test/String.cpp +++ b/src/libs/core/test/String.cpp @@ -79,6 +79,9 @@ namespace lms::core::stringUtils::tests TestCase tests[]{ { "", "", { "" } }, + { "//", "//", { "", "" } }, + { "//abc//", "//", { "", "abc", "" } }, + { "//abc////abc//", "//", { "", "abc", "", "abc", "" } }, { "abc", "", { "abc" } }, { "abc", "-", { "abc" } }, { "abc", "b", { "a", "c" } }, @@ -99,6 +102,35 @@ namespace lms::core::stringUtils::tests } } + TEST(StringUtils, splitString_multiStringDelim) + { + struct TestCase + { + std::string_view input; + std::vector delimiters; + std::vector expectedOutput; + }; + + TestCase tests[]{ + { "", { "" }, { "" } }, + { "abc", { "" }, { "abc" } }, + { "abc", { "b" }, { "a", "c" } }, + { "ab/cd", { "/" }, { "ab", "cd" } }, + { "ab/cd", { "/", ";" }, { "ab", "cd" } }, + { "ab;/cd", { "/", ";" }, { "ab", "", "cd" } }, + { "ab;/;cd", { "/", ";" }, { "ab", "", "", "cd" } }, + { "ab/;cd", { "/", ";" }, { "ab", "", "cd" } }, + { "ab/;/cd", { "/", ";" }, { "ab", "", "", "cd" } }, + { "ab/cd/ef", { "/", "cd" }, { "ab", "", "", "ef" } }, + }; + + for (const TestCase& test : tests) + { + const std::vector res{ splitString(test.input, test.delimiters) }; + EXPECT_EQ(res, test.expectedOutput) << "Input = '" << test.input << "'"; + } + } + TEST(StringUtils, joinStrings) { struct TestCase diff --git a/src/libs/database/impl/Migration.cpp b/src/libs/database/impl/Migration.cpp index 8fb2ae7b..f108a5dc 100644 --- a/src/libs/database/impl/Migration.cpp +++ b/src/libs/database/impl/Migration.cpp @@ -740,7 +740,7 @@ SELECT { session.getDboSession()->execute("ALTER TABLE release ADD is_compilation BOOLEAN NOT NULL DEFAULT(false)"); - // Just increment the scan version of the settings to make the next scheduled scan rescan everything + // Just increment the scan version of the settings to make the next scheduled scan rescan everything session.getDboSession()->execute("UPDATE scan_settings SET scan_version = scan_version + 1"); } diff --git a/src/libs/metadata/impl/Parser.cpp b/src/libs/metadata/impl/Parser.cpp index c9417fc6..7e25a9c1 100644 --- a/src/libs/metadata/impl/Parser.cpp +++ b/src/libs/metadata/impl/Parser.cpp @@ -46,7 +46,7 @@ namespace lms::metadata { if (value.find(tagDelimiter) != std::string_view::npos) { - for (std::string_view splitTag : core::stringUtils::splitString(value, tagDelimiter)) + for (std::string_view splitTag : core::stringUtils::splitString(value, tagDelimiters)) visitTagIfNonEmpty(splitTag); return; @@ -80,7 +80,7 @@ namespace lms::metadata { if (value.find(tagDelimiter) != std::string_view::npos) { - for (std::string_view splitTag : core::stringUtils::splitString(value, tagDelimiter)) + for (std::string_view splitTag : core::stringUtils::splitString(value, tagDelimiters)) addTagIfNonEmpty(splitTag); return; diff --git a/src/libs/metadata/test/Parser.cpp b/src/libs/metadata/test/Parser.cpp index bfbcde69..986f819c 100644 --- a/src/libs/metadata/test/Parser.cpp +++ b/src/libs/metadata/test/Parser.cpp @@ -236,8 +236,8 @@ namespace lms::metadata const TestTagReader testTags{ { { TagType::Album, { "MyAlbum" } }, - { TagType::AlbumArtist, { "AlbumArtist1 \\ AlbumArtist2" } }, - { TagType::Artist, { " This / is ; One Artist \\ Other Artist " } }, + { TagType::AlbumArtist, { "AlbumArtist1 / AlbumArtist2" } }, + { TagType::Artist, { " Artist1 / Artist2 feat. Artist3 " } }, { TagType::Genre, { "Genre1 ; Genre2" } }, { TagType::Language, { " Lang1/Lang2 / Lang3" } }, @@ -246,13 +246,14 @@ namespace lms::metadata Parser parser; static_cast(parser).setDefaultTagDelimiters(std::vector{ " ; ", "/" }); - static_cast(parser).setArtistTagDelimiters(std::vector{ " \\ ", " / " }); // The first delimiter found will be used + static_cast(parser).setArtistTagDelimiters(std::vector{ " / ", " feat. " }); std::unique_ptr track{ parser.parse(testTags) }; - ASSERT_EQ(track->artists.size(), 2); - EXPECT_EQ(track->artists[0].name, "This / is ; One Artist"); - EXPECT_EQ(track->artists[1].name, "Other Artist"); - EXPECT_EQ(track->artistDisplayName, "This / is ; One Artist, Other Artist"); // reconstruct artist display name since a custom delimiter is hit + ASSERT_EQ(track->artists.size(), 3); + EXPECT_EQ(track->artists[0].name, "Artist1"); + EXPECT_EQ(track->artists[1].name, "Artist2"); + EXPECT_EQ(track->artists[2].name, "Artist3"); + EXPECT_EQ(track->artistDisplayName, "Artist1, Artist2, Artist3"); // reconstruct artist display name since a custom delimiter is hit ASSERT_EQ(track->genres.size(), 2); EXPECT_EQ(track->genres[0], "Genre1"); EXPECT_EQ(track->genres[1], "Genre2"); @@ -269,8 +270,7 @@ namespace lms::metadata EXPECT_EQ(track->medium->release->name, "MyAlbum"); EXPECT_EQ(track->medium->release->artists[0].name, "AlbumArtist1"); EXPECT_EQ(track->medium->release->artists[1].name, "AlbumArtist2"); - EXPECT_EQ(track->medium->release->artistDisplayName, "AlbumArtist1 \\ AlbumArtist2"); // reconstruct artist display name since a custom delimiter is hit - + EXPECT_EQ(track->medium->release->artistDisplayName, "AlbumArtist1 / AlbumArtist2"); // TODO: reconstruct artist display name since a custom delimiter is hit } TEST(Parser, noArtistInArtist)