From 7b0039ab855854352e863bc09884ee9b95bcf393 Mon Sep 17 00:00:00 2001 From: emeric Date: Thu, 18 Jun 2026 18:45:42 +0200 Subject: [PATCH] Removed useless redundant query --- src/libs/database/test/TrackArtistLink.cpp | 83 +++++++++++++++++++++ src/libs/subsonic/impl/responses/Artist.cpp | 14 ++-- src/libs/subsonic/impl/responses/Artist.hpp | 2 +- src/libs/subsonic/impl/responses/Song.cpp | 23 ++++-- 4 files changed, 107 insertions(+), 15 deletions(-) diff --git a/src/libs/database/test/TrackArtistLink.cpp b/src/libs/database/test/TrackArtistLink.cpp index 8e021367..8807891d 100644 --- a/src/libs/database/test/TrackArtistLink.cpp +++ b/src/libs/database/test/TrackArtistLink.cpp @@ -306,4 +306,87 @@ namespace lms::db::tests } } + TEST_F(DatabaseFixture, Track_getArtistIds_typeFilter) + { + ScopedTrack track{ session }; + ScopedArtist artist1{ session, "Artist1" }; + ScopedArtist artist2{ session, "Artist2" }; + + { + auto transaction{ session.createWriteTransaction() }; + session.create(track.get(), artist1.get(), TrackArtistLinkType::Artist, false); + session.create(track.get(), artist2.get(), TrackArtistLinkType::Artist, false); + session.create(track.get(), artist1.get(), TrackArtistLinkType::Mixer, false); + } + + { + auto transaction{ session.createReadTransaction() }; + + const auto artistIds{ track->getArtistIds({ TrackArtistLinkType::Artist }) }; + ASSERT_EQ(artistIds.size(), 2); + EXPECT_EQ(artistIds[0], artist1.getId()); + EXPECT_EQ(artistIds[1], artist2.getId()); + + const auto mixerIds{ track->getArtistIds({ TrackArtistLinkType::Mixer }) }; + ASSERT_EQ(mixerIds.size(), 1); + EXPECT_EQ(mixerIds[0], artist1.getId()); + + const auto noFilter{ track->getArtistIds({}) }; + EXPECT_EQ(noFilter.size(), 2); + } + } + + TEST_F(DatabaseFixture, Track_visitArtistLinks_orderedById) + { + ScopedTrack track{ session }; + ScopedArtist artist1{ session, "Artist1" }; + ScopedArtist artist2{ session, "Artist2" }; + + TrackArtistLinkId link1Id; + + { + auto transaction{ session.createWriteTransaction() }; + auto link1{ session.create(track.get(), artist1.get(), TrackArtistLinkType::Artist, false) }; + link1Id = link1->getId(); + session.create(track.get(), artist2.get(), TrackArtistLinkType::Artist, false); + } + + { + auto transaction{ session.createReadTransaction() }; + + std::vector links; + track->visitArtistLinks([&](const db::TrackArtistLink::pointer& link) { + links.push_back(link); + }); + + ASSERT_EQ(links.size(), 2); + EXPECT_EQ(links[0]->getArtistId(), artist1.getId()); + EXPECT_EQ(links[1]->getArtistId(), artist2.getId()); + } + + { + auto transaction{ session.createWriteTransaction() }; + TrackArtistLink::find(session, link1Id).remove(); + } + + { + auto transaction{ session.createWriteTransaction() }; + session.create(track.get(), artist1.get(), TrackArtistLinkType::Artist, false); + } + + // artist2's link has lower id, artist1's re-created link has higher id. + { + auto transaction{ session.createReadTransaction() }; + + std::vector links; + track->visitArtistLinks([&](const db::TrackArtistLink::pointer& link) { + links.push_back(link); + }); + + ASSERT_EQ(links.size(), 2); + EXPECT_EQ(links[0]->getArtistId(), artist2.getId()); + EXPECT_EQ(links[1]->getArtistId(), artist1.getId()); + } + } + } // namespace lms::db::tests \ No newline at end of file diff --git a/src/libs/subsonic/impl/responses/Artist.cpp b/src/libs/subsonic/impl/responses/Artist.cpp index 297e6ad0..43283780 100644 --- a/src/libs/subsonic/impl/responses/Artist.cpp +++ b/src/libs/subsonic/impl/responses/Artist.cpp @@ -41,17 +41,17 @@ namespace lms::api::subsonic namespace utils { - std::string joinArtistNames(const std::vector& artists) + std::string joinArtistNames(const std::vector& links) { - if (artists.size() == 1) - return artists.front()->getName(); + if (links.size() == 1) + return std::string{ links.front()->getArtistName() }; std::vector names; - names.resize(artists.size()); + names.resize(links.size()); - std::transform(std::cbegin(artists), std::cend(artists), std::begin(names), - [](const Artist::pointer& artist) { - return artist->getName(); + std::transform(std::cbegin(links), std::cend(links), std::begin(names), + [](const TrackArtistLink::pointer& link) { + return std::string{ link->getArtistName() }; }); return core::stringUtils::joinStrings(names, ", "); diff --git a/src/libs/subsonic/impl/responses/Artist.hpp b/src/libs/subsonic/impl/responses/Artist.hpp index 50612d76..bcbfe4eb 100644 --- a/src/libs/subsonic/impl/responses/Artist.hpp +++ b/src/libs/subsonic/impl/responses/Artist.hpp @@ -42,7 +42,7 @@ namespace lms::api::subsonic namespace utils { - std::string joinArtistNames(const std::vector>& artists); + std::string joinArtistNames(const std::vector>& links); std::string_view toString(db::TrackArtistLinkType type); } // namespace utils diff --git a/src/libs/subsonic/impl/responses/Song.cpp b/src/libs/subsonic/impl/responses/Song.cpp index ba5997a9..7c36bc15 100644 --- a/src/libs/subsonic/impl/responses/Song.cpp +++ b/src/libs/subsonic/impl/responses/Song.cpp @@ -134,16 +134,24 @@ namespace lms::api::subsonic trackResponse.setAttribute("coverArt", idToString(coverArtId)); } - const std::vector& artists{ track->getArtists({ db::TrackArtistLinkType::Artist }) }; - if (!artists.empty()) + std::vector artistLinks; + std::vector trackArtistLinks; + track->visitArtistLinks([&](const db::TrackArtistLink::pointer& link) { + artistLinks.push_back(link); + + if (link->getType() == db::TrackArtistLinkType::Artist) + trackArtistLinks.push_back(link); + }); + + if (!trackArtistLinks.empty()) { if (!track->getArtistDisplayName().empty()) trackResponse.setAttribute("artist", track->getArtistDisplayName()); else - trackResponse.setAttribute("artist", utils::joinArtistNames(artists)); + trackResponse.setAttribute("artist", utils::joinArtistNames(trackArtistLinks)); - if (artists.size() == 1) - trackResponse.setAttribute("artistId", idToString(artists.front()->getId())); + if (trackArtistLinks.size() == 1) + trackResponse.setAttribute("artistId", idToString(trackArtistLinks.front()->getArtistId())); } const db::Release::pointer release{ track->getRelease() }; @@ -195,7 +203,8 @@ namespace lms::api::subsonic trackResponse.createEmptyArrayChild("artists"); trackResponse.createEmptyArrayChild("contributors"); - track->visitArtistLinks([&](const db::TrackArtistLink::pointer& artistLink) { + for (const auto& artistLink : artistLinks) + { switch (artistLink->getType()) { case db::TrackArtistLinkType::Artist: @@ -204,7 +213,7 @@ namespace lms::api::subsonic default: trackResponse.addArrayChild("contributors", createContributorNode(artistLink)); } - }); + } if (release) {