From 18b220b241a24150d3ef01cf0d24edcaa8a2bcbc Mon Sep 17 00:00:00 2001 From: emeric Date: Mon, 15 Apr 2024 18:50:00 +0200 Subject: [PATCH] Optimm to get all artists of a track in a single query --- src/libs/database/impl/TrackArtistLink.cpp | 30 ++++++--- .../include/database/TrackArtistLink.hpp | 1 + src/libs/database/test/Artist.cpp | 64 +++++++++++++++++++ .../subsonic/impl/responses/Contributor.cpp | 4 +- .../subsonic/impl/responses/Contributor.hpp | 3 +- src/libs/subsonic/impl/responses/Song.cpp | 11 ++-- 6 files changed, 95 insertions(+), 18 deletions(-) diff --git a/src/libs/database/impl/TrackArtistLink.cpp b/src/libs/database/impl/TrackArtistLink.cpp index 5667dd40..81ebe025 100644 --- a/src/libs/database/impl/TrackArtistLink.cpp +++ b/src/libs/database/impl/TrackArtistLink.cpp @@ -39,19 +39,17 @@ namespace lms::db if (params.linkType) query.where("t_a_l.type = ?").bind(*params.linkType); - if (params.track.isValid() || params.release.isValid()) - query.join("track t ON t.id = t_a_l.track_id"); + if (params.track.isValid()) + query.where("t_a_l.track_id = ?").bind(params.track); if (params.artist.isValid()) - query.join("artist a ON a.id = t_a_l.artist_id"); + query.where("t_a_l.artist_id = ?").bind(params.artist); if (params.release.isValid()) + { + query.join("track t ON t.id = t_a_l.track_id"); query.where("t.release_id = ?").bind(params.release); - - if (params.track.isValid()) - query.where("t.id = ?").bind(params.track); - - query.groupBy("t_a_l.id"); + } return query; } @@ -81,6 +79,22 @@ namespace lms::db return utils::fetchQuerySingleResult(session.getDboSession()->find().where("id = ?").bind(id)); } + void TrackArtistLink::find(Session& session, TrackId trackId, const std::function& artist)>& func) + { + session.checkReadTransaction(); + + using ResultType = std::tuple < Wt::Dbo::ptr, Wt::Dbo::ptr>; + + const auto query{ session.getDboSession()->query("SELECT t_a_l, a FROM track_artist_link t_a_l") + .join("artist a ON t_a_l.artist_id = a.id") + .where("t_a_l.track_id = ?").bind(trackId) }; + + utils::forEachQueryResult(query, [&](const ResultType& result) + { + func(std::get>(result), std::get>(result)); + }); + } + void TrackArtistLink::find(Session& session, const FindParameters& parameters, const std::function& func) { const auto query{ createQuery(session, parameters) }; diff --git a/src/libs/database/include/database/TrackArtistLink.hpp b/src/libs/database/include/database/TrackArtistLink.hpp index e3fe7580..216fbf33 100644 --- a/src/libs/database/include/database/TrackArtistLink.hpp +++ b/src/libs/database/include/database/TrackArtistLink.hpp @@ -62,6 +62,7 @@ namespace lms::db TrackArtistLink() = default; TrackArtistLink(ObjectPtr track, ObjectPtr artist, TrackArtistLinkType type, std::string_view subType); + static void find(Session& session, TrackId trackId, const std::function&)>&); static void find(Session& session, const FindParameters& parameters, const std::function&); static pointer find(Session& session, TrackArtistLinkId linkId); static pointer create(Session& session, ObjectPtr track, ObjectPtr artist, TrackArtistLinkType type, std::string_view subType = {}); diff --git a/src/libs/database/test/Artist.cpp b/src/libs/database/test/Artist.cpp index 7d06a95d..bd486544 100644 --- a/src/libs/database/test/Artist.cpp +++ b/src/libs/database/test/Artist.cpp @@ -394,6 +394,29 @@ namespace lms::db::tests EXPECT_TRUE(types.contains(TrackArtistLinkType::Writer)); EXPECT_FALSE(types.contains(TrackArtistLinkType::Composer)); } + + { + auto transaction{ session.createReadTransaction() }; + + std::vector visitedLinks; + TrackArtistLink::find(session, TrackArtistLink::FindParameters{}.setTrack(track.getId()), [&](const TrackArtistLink::pointer& link) + { + visitedLinks.push_back(link); + }); + ASSERT_EQ(visitedLinks.size(), 3); + EXPECT_EQ(visitedLinks[0]->getArtist()->getId(), artist.getId()); + EXPECT_EQ(visitedLinks[1]->getArtist()->getId(), artist.getId()); + EXPECT_EQ(visitedLinks[2]->getArtist()->getId(), artist.getId()); + + auto containsType = [&](TrackArtistLinkType type) + { + return std::any_of(std::cbegin(visitedLinks), std::cend(visitedLinks), [type](const TrackArtistLink::pointer& link) { return link->getType() == type;}); + }; + + EXPECT_TRUE(containsType(TrackArtistLinkType::Artist)); + EXPECT_TRUE(containsType(TrackArtistLinkType::ReleaseArtist)); + EXPECT_TRUE(containsType(TrackArtistLinkType::Writer)); + } } TEST_F(DatabaseFixture, Artist_singleTrackMultiArtists) @@ -453,6 +476,47 @@ namespace lms::db::tests tracks = Track::findIds(session, Track::FindParameters{}.setArtist(artist2->getId(), { TrackArtistLinkType::Artist })); EXPECT_EQ(tracks.results.size(), 1); } + + { + auto transaction{ session.createReadTransaction() }; + + std::vector visitedLinks; + TrackArtistLink::find(session, TrackArtistLink::FindParameters{}.setTrack(track.getId()), [&](const TrackArtistLink::pointer& link) + { + visitedLinks.push_back(link); + }); + ASSERT_EQ(visitedLinks.size(), 2); + EXPECT_EQ(visitedLinks[0]->getArtist()->getId(), artist1.getId()); + EXPECT_EQ(visitedLinks[1]->getArtist()->getId(), artist2.getId()); + } + + { + auto transaction{ session.createReadTransaction() }; + + std::vector visitedLinks; + TrackArtistLink::find(session, TrackArtistLink::FindParameters{}.setArtist(artist2.getId()), [&](const TrackArtistLink::pointer& link) + { + visitedLinks.push_back(link); + }); + ASSERT_EQ(visitedLinks.size(), 1); + EXPECT_EQ(visitedLinks[0]->getArtist()->getId(), artist2.getId()); + EXPECT_EQ(visitedLinks[0]->getTrack()->getId(), track.getId()); + } + + { + auto transaction{ session.createReadTransaction() }; + + std::vector> visitedEntries; + TrackArtistLink::find(session, track.getId(), [&](const TrackArtistLink::pointer& link, const Artist::pointer& artist) + { + visitedEntries.push_back(std::make_pair(link, artist)); + }); + ASSERT_EQ(visitedEntries.size(), 2); + EXPECT_EQ(visitedEntries[0].first->getArtist()->getId(), artist1.getId()); + EXPECT_EQ(visitedEntries[0].second->getId(), artist1.getId()); + EXPECT_EQ(visitedEntries[1].first->getArtist()->getId(), artist2.getId()); + EXPECT_EQ(visitedEntries[1].second->getId(), artist2.getId()); + } } TEST_F(DatabaseFixture, Artist_findByName) diff --git a/src/libs/subsonic/impl/responses/Contributor.cpp b/src/libs/subsonic/impl/responses/Contributor.cpp index 576e99b2..ad2dc2b0 100644 --- a/src/libs/subsonic/impl/responses/Contributor.cpp +++ b/src/libs/subsonic/impl/responses/Contributor.cpp @@ -26,14 +26,14 @@ namespace lms::api::subsonic { - Response::Node createContributorNode(const db::ObjectPtr& trackArtistLink) + Response::Node createContributorNode(const db::ObjectPtr& trackArtistLink, const db::ObjectPtr& artist) { Response::Node contributorNode; contributorNode.setAttribute("role", utils::toString(trackArtistLink->getType())); if (!trackArtistLink->getSubType().empty()) contributorNode.setAttribute("subRole", trackArtistLink->getSubType()); - contributorNode.addChild("artist", createArtistNode(trackArtistLink->getArtist())); + contributorNode.addChild("artist", createArtistNode(artist)); return contributorNode; } diff --git a/src/libs/subsonic/impl/responses/Contributor.hpp b/src/libs/subsonic/impl/responses/Contributor.hpp index 14b54dde..6c89ed01 100644 --- a/src/libs/subsonic/impl/responses/Contributor.hpp +++ b/src/libs/subsonic/impl/responses/Contributor.hpp @@ -24,10 +24,11 @@ namespace lms::db { + class Artist; class TrackArtistLink; } namespace lms::api::subsonic { - Response::Node createContributorNode(const db::ObjectPtr& trackArtistLink); + Response::Node createContributorNode(const db::ObjectPtr& trackArtistLink, const db::ObjectPtr& artist); } \ No newline at end of file diff --git a/src/libs/subsonic/impl/responses/Song.cpp b/src/libs/subsonic/impl/responses/Song.cpp index f7a8a1aa..af5c6f9f 100644 --- a/src/libs/subsonic/impl/responses/Song.cpp +++ b/src/libs/subsonic/impl/responses/Song.cpp @@ -156,25 +156,22 @@ namespace lms::api::subsonic } { - TrackArtistLink::FindParameters params; - params.setTrack(track->getId()); - trackResponse.createEmptyArrayChild("albumartists"); trackResponse.createEmptyArrayChild("artists"); trackResponse.createEmptyArrayChild("contributors"); - TrackArtistLink::find(context.dbSession, params, [&](const TrackArtistLink::pointer& link) + TrackArtistLink::find(context.dbSession, track->getId(), [&](const TrackArtistLink::pointer& link, const Artist::pointer& artist) { switch (link->getType()) { case TrackArtistLinkType::Artist: - trackResponse.addArrayChild("artists", createArtistNode(link->getArtist())); + trackResponse.addArrayChild("artists", createArtistNode(artist)); break; case TrackArtistLinkType::ReleaseArtist: - trackResponse.addArrayChild("albumartists", createArtistNode(link->getArtist())); + trackResponse.addArrayChild("albumartists", createArtistNode(artist)); break; default: - trackResponse.addArrayChild("contributors", createContributorNode(link)); + trackResponse.addArrayChild("contributors", createContributorNode(link, artist)); } }); }