From 87fd8bef3bf8af256ca1ede377eba3dbf8efb5c2 Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 15 Sep 2025 20:27:33 +0200 Subject: [PATCH] Added a fallback to accept musicbrainzartistid ins artist.nfo files, ref #750 --- CMakeLists.txt | 1 + src/libs/metadata/CMakeLists.txt | 1 + src/libs/metadata/impl/ArtistInfo.cpp | 61 +++++++++++++++--------- src/libs/metadata/test/ArtistInfo.cpp | 18 +++++++ src/libs/services/podcast/CMakeLists.txt | 5 +- 5 files changed, 60 insertions(+), 26 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a51a50e6..3ebbd18d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -24,6 +24,7 @@ find_package(Threads REQUIRED) find_package(Filesystem REQUIRED) find_package(Boost REQUIRED COMPONENTS system program_options iostreams) find_package(Wt REQUIRED COMPONENTS Wt Dbo DboSqlite3 HTTP) +find_package(Pugixml CONFIG REQUIRED) # WT if (NOT Wt_FOUND) diff --git a/src/libs/metadata/CMakeLists.txt b/src/libs/metadata/CMakeLists.txt index ea401ef1..1c3c2967 100644 --- a/src/libs/metadata/CMakeLists.txt +++ b/src/libs/metadata/CMakeLists.txt @@ -34,6 +34,7 @@ target_include_directories(lmsmetadata PRIVATE target_link_libraries(lmsmetadata PRIVATE lmsav PkgConfig::Taglib + pugixml::pugixml ) target_link_libraries(lmsmetadata PUBLIC diff --git a/src/libs/metadata/impl/ArtistInfo.cpp b/src/libs/metadata/impl/ArtistInfo.cpp index 85b4e96b..bcba6b88 100644 --- a/src/libs/metadata/impl/ArtistInfo.cpp +++ b/src/libs/metadata/impl/ArtistInfo.cpp @@ -19,14 +19,27 @@ #include "metadata/ArtistInfo.hpp" -#include -#include +#include #include "core/ILogger.hpp" +#include "core/LiteralString.hpp" #include "core/String.hpp" namespace lms::metadata { + namespace + { + std::optional getText(const pugi::xml_node& node, const core::LiteralString& tag) + { + std::optional res; + + if (const pugi::xml_node child{ node.child(tag.c_str()) }) + res = std::string_view{ child.child_value() }; + + return res; + } + } // namespace + std::span getSupportedArtistInfoFiles() { static const std::array files{ "artist.nfo" }; @@ -35,29 +48,33 @@ namespace lms::metadata ArtistInfo parseArtistInfo(std::istream& is) { - try + ArtistInfo artistInfo; + pugi::xml_document doc; + pugi::xml_parse_result result{ doc.load(is) }; + if (!result) { - ArtistInfo artistInfo; - - boost::property_tree::ptree root; - boost::property_tree::read_xml(is, root); - - const auto& artistNode{ root.get_child("artist") }; - - artistInfo.mbid = core::UUID::fromString(core::stringUtils::stringTrim(artistNode.get_optional("musicBrainzArtistID").value_or(""))); - artistInfo.name = core::stringUtils::stringTrim(artistNode.get_optional("name").value_or("")); - artistInfo.sortName = core::stringUtils::stringTrim(artistNode.get_optional("sortname").value_or("")); - artistInfo.type = core::stringUtils::stringTrim(artistNode.get_optional("type").value_or("")); - artistInfo.gender = core::stringUtils::stringTrim(artistNode.get_optional("gender").value_or("")); - artistInfo.disambiguation = core::stringUtils::stringTrim(artistNode.get_optional("disambiguation").value_or("")); - artistInfo.biography = artistNode.get_optional("biography").value_or(""); - - return artistInfo; + LMS_LOG(METADATA, ERROR, "Cannot read artist info xml: " << result.description()); + throw ArtistInfoParseException{ result.description() }; } - catch (boost::property_tree::ptree_error& error) + + const pugi::xml_node artistNode{ doc.child("artist") }; + if (!artistNode) + throw ArtistInfoParseException{ "No element found in artist info xml" }; + { - LMS_LOG(METADATA, ERROR, "Cannot read artist xml info: " << error.what()); - throw ArtistInfoParseException{ error.what() }; + auto mbid{ getText(artistNode, "musicBrainzArtistID") }; + if (!mbid.has_value()) + mbid = getText(artistNode, "musicbrainzartistid"); // lidarr seems to put this in lowercase + artistInfo.mbid = core::UUID::fromString(core::stringUtils::stringTrim(mbid.has_value() ? *mbid : "")); } + + artistInfo.name = core::stringUtils::stringTrim(getText(artistNode, "name").value_or("")); + artistInfo.sortName = core::stringUtils::stringTrim(getText(artistNode, "sortname").value_or("")); + artistInfo.type = core::stringUtils::stringTrim(getText(artistNode, "type").value_or("")); + artistInfo.gender = core::stringUtils::stringTrim(getText(artistNode, "gender").value_or("")); + artistInfo.disambiguation = core::stringUtils::stringTrim(getText(artistNode, "disambiguation").value_or("")); + artistInfo.biography = getText(artistNode, "biography").value_or(""); + + return artistInfo; } } // namespace lms::metadata \ No newline at end of file diff --git a/src/libs/metadata/test/ArtistInfo.cpp b/src/libs/metadata/test/ArtistInfo.cpp index 4410cf2b..aa84b87e 100644 --- a/src/libs/metadata/test/ArtistInfo.cpp +++ b/src/libs/metadata/test/ArtistInfo.cpp @@ -77,6 +77,24 @@ He moved from the UK to Montreal in 1984 to become resident DJ at a number of cl ASSERT_EQ(artistInfo.biography, "DJ and producer based in London, UK. Founder of Missile Records and Planet Of Drums.\r\n\r\nHe moved from the UK to Montreal in 1984 to become resident DJ at a number of clubs. In 1987, he began working as an A&R for JSE Agency & Management in New York, managing the likes of Tommy Musto, Frankie Bones, and The KLF. He also arranged and was tour manager for artists such as Womack & Womack, Jungle Brothers, Ice-T, and Guru Josh."); } + TEST(ArtistInfo, basic_musicbrainzartistid) + { + std::istringstream is{ R"( + + Tim Taylor + 38811c52-85e3-4e2e-3319-ab7d9f2cfa5b + Taylor, Tim + Timothy Taylor +)" }; + + const ArtistInfo artistInfo{ parseArtistInfo(is) }; + + EXPECT_EQ(artistInfo.mbid, core::UUID::fromString("38811c52-85e3-4e2e-3319-ab7d9f2cfa5b")); + EXPECT_EQ(artistInfo.name, "Tim Taylor"); + ASSERT_EQ(artistInfo.sortName, "Taylor, Tim"); + ASSERT_EQ(artistInfo.disambiguation, "Timothy Taylor"); + } + TEST(ArtistInfo, trim) { std::istringstream is{ R"( diff --git a/src/libs/services/podcast/CMakeLists.txt b/src/libs/services/podcast/CMakeLists.txt index 889d2b53..54b4a77f 100644 --- a/src/libs/services/podcast/CMakeLists.txt +++ b/src/libs/services/podcast/CMakeLists.txt @@ -1,5 +1,3 @@ -pkg_check_modules(PUGIXML REQUIRED IMPORTED_TARGET pugixml) - add_library(lmspodcast STATIC impl/steps/CheckForMissingFilesStep.cpp impl/steps/ClearTmpDirectoryStep.cpp @@ -22,13 +20,12 @@ target_include_directories(lmspodcast INTERFACE target_include_directories(lmspodcast PRIVATE include impl - ${PUGIXML_INCLUDE_DIRS} ) target_link_libraries(lmspodcast PRIVATE lmscore lmsimage - PkgConfig::PUGIXML + pugixml::pugixml ) target_link_libraries(lmspodcast PUBLIC