From 5d07dcbc3b68f2fc12a474e8fd1896bf15194170 Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 4 Apr 2020 14:47:39 +0200 Subject: [PATCH] Handle sorting method for artists --- src/libs/database/impl/Artist.cpp | 106 +++++++++++--------- src/libs/subsonic/impl/SubsonicResource.cpp | 8 +- src/lms/ui/explore/ArtistsView.cpp | 1 + src/test/database/DatabaseTest.cpp | 37 +++---- 4 files changed, 82 insertions(+), 70 deletions(-) diff --git a/src/libs/database/impl/Artist.cpp b/src/libs/database/impl/Artist.cpp index d2571e91..3c3f3f9b 100644 --- a/src/libs/database/impl/Artist.cpp +++ b/src/libs/database/impl/Artist.cpp @@ -74,57 +74,13 @@ Artist::create(Session& session, const std::string& name, const std::optional -Artist::getAll(Session& session, std::optional offset, std::optional size) -{ - session.checkSharedLocked(); - Wt::Dbo::collection res = session.getDboSession().find() - .offset(offset ? static_cast(*offset) : -1) - .limit(size ? static_cast(*size) : -1) - .orderBy("sort_name COLLATE NOCASE"); - - return std::vector(res.begin(), res.end()); -} - -std::vector -Artist::getAllIds(Session& session) -{ - session.checkSharedLocked(); - - Wt::Dbo::collection res = session.getDboSession().query("SELECT id FROM artist"); - return std::vector(res.begin(), res.end()); -} - -std::vector -Artist::getAllOrphans(Session& session) -{ - session.checkSharedLocked(); - Wt::Dbo::collection> res {session.getDboSession().query>("SELECT DISTINCT a FROM artist a WHERE NOT EXISTS(SELECT 1 FROM track t INNER JOIN track_artist_link t_a_l ON t_a_l.artist_id = a.id WHERE t.id = t_a_l.track_id)")}; - - return std::vector(res.begin(), res.end()); -} - -std::vector -Artist::getAllIdsWithClusters(Session& session, std::optional limit) -{ - session.checkSharedLocked(); - - Wt::Dbo::collection res = session.getDboSession().query - ("SELECT DISTINCT a.id FROM artist a" - " INNER JOIN track t ON t.id = t_a_l.track_id INNER JOIN track_artist_link t_a_l ON t_a_l.artist_id = a.id" - " INNER JOIN track_cluster t_c ON t_c.track_id = t.id") - .limit(limit ? static_cast(*limit) : -1); - - return std::vector(res.begin(), res.end()); -} - - static Wt::Dbo::Query getQuery(Session& session, const std::set& clusterIds, const std::vector& keywords, - std::optional linkType) + std::optional linkType, + Artist::NameSortMethod sortMethod) { session.checkSharedLocked(); @@ -158,7 +114,17 @@ getQuery(Session& session, if (!clusterIds.empty()) oss << " GROUP BY t.id HAVING COUNT(DISTINCT c.id) = " << clusterIds.size(); - oss << " ORDER BY a.sort_name COLLATE NOCASE"; + switch (sortMethod) + { + case Artist::NameSortMethod::None: + break; + case Artist::NameSortMethod::ByName: + oss << " ORDER BY a.name COLLATE NOCASE"; + break; + case Artist::NameSortMethod::BySortName: + oss << " ORDER BY a.sort_name COLLATE NOCASE"; + break; + } Wt::Dbo::Query query = session.getDboSession().query( oss.str() ); @@ -170,6 +136,50 @@ getQuery(Session& session, return query; } +std::vector +Artist::getAll(Session& session, NameSortMethod sortMethod, std::optional offset, std::optional size) +{ + session.checkSharedLocked(); + + Wt::Dbo::collection res = getQuery(session, {}, {}, std::nullopt, sortMethod) + .limit(size ? static_cast(*size) + 1 : -1) + .offset(offset ? static_cast(*offset) : -1); + + return std::vector(res.begin(), res.end()); +} + +std::vector +Artist::getAllIds(Session& session) +{ + session.checkSharedLocked(); + + Wt::Dbo::collection res = session.getDboSession().query("SELECT id FROM artist"); + return std::vector(res.begin(), res.end()); +} + +std::vector +Artist::getAllOrphans(Session& session) +{ + session.checkSharedLocked(); + Wt::Dbo::collection> res {session.getDboSession().query>("SELECT DISTINCT a FROM artist a WHERE NOT EXISTS(SELECT 1 FROM track t INNER JOIN track_artist_link t_a_l ON t_a_l.artist_id = a.id WHERE t.id = t_a_l.track_id)")}; + + return std::vector(res.begin(), res.end()); +} + +std::vector +Artist::getAllIdsWithClusters(Session& session, std::optional limit) +{ + session.checkSharedLocked(); + + Wt::Dbo::collection res = session.getDboSession().query + ("SELECT DISTINCT a.id FROM artist a" + " INNER JOIN track t ON t.id = t_a_l.track_id INNER JOIN track_artist_link t_a_l ON t_a_l.artist_id = a.id" + " INNER JOIN track_cluster t_c ON t_c.track_id = t.id") + .limit(limit ? static_cast(*limit) : -1); + + return std::vector(res.begin(), res.end()); +} + std::vector Artist::getByClusters(Session& session, const std::set& clusters, NameSortMethod sortMethod) { @@ -191,7 +201,7 @@ Artist::getByFilter(Session& session, bool& moreResults) { session.checkSharedLocked(); - Wt::Dbo::collection collection = getQuery(session, clusters, keywords, linkType) + Wt::Dbo::collection collection = getQuery(session, clusters, keywords, linkType, sortMethod) .limit(size ? static_cast(*size) + 1 : -1) .offset(offset ? static_cast(*offset) : -1); diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp index 11391e36..0583db25 100644 --- a/src/libs/subsonic/impl/SubsonicResource.cpp +++ b/src/libs/subsonic/impl/SubsonicResource.cpp @@ -901,7 +901,7 @@ handleGetArtistsRequest(RequestContext& context) if (!user) throw UserNotAuthorizedError {}; - auto artists {Artist::getAll(context.dbSession)}; + auto artists {Artist::getAll(context.dbSession, Artist::NameSortMethod::ByName)}; for (const Artist::pointer& artist : artists) indexNode.addArrayChild("artist", artistToResponseNode(user, artist, true /* id3 */)); @@ -932,7 +932,7 @@ handleGetMusicDirectoryRequest(RequestContext& context) { directoryNode.setAttribute("name", "Music"); - auto artists {Artist::getAll(context.dbSession)}; + auto artists {Artist::getAll(context.dbSession, Artist::NameSortMethod::ByName)}; for (const Artist::pointer& artist : artists) directoryNode.addArrayChild("child", artistToResponseNode(user, artist, false /* no id3 */)); @@ -1028,7 +1028,7 @@ handleGetIndexesRequest(RequestContext& context) if (!user) throw UserNotAuthorizedError {}; - auto artists {Artist::getAll(context.dbSession)}; + auto artists {Artist::getAll(context.dbSession, Artist::NameSortMethod::ByName)}; for (const Artist::pointer& artist : artists) indexNode.addArrayChild("artist", artistToResponseNode(user, artist, false /* no id3 */)); @@ -1317,7 +1317,7 @@ handleSearchRequestCommon(RequestContext& context, bool id3) bool more; { - auto artists {Artist::getByFilter(context.dbSession, {}, keywords, {}, artistOffset, artistCount, more)}; + auto artists {Artist::getByFilter(context.dbSession, {}, keywords, std::nullopt, Artist::NameSortMethod::ByName, artistOffset, artistCount, more)}; for (const Artist::pointer& artist : artists) searchResult2Node.addArrayChild("artist", artistToResponseNode(user, artist, id3)); } diff --git a/src/lms/ui/explore/ArtistsView.cpp b/src/lms/ui/explore/ArtistsView.cpp index 2798a3a6..400b206a 100644 --- a/src/lms/ui/explore/ArtistsView.cpp +++ b/src/lms/ui/explore/ArtistsView.cpp @@ -95,6 +95,7 @@ Artists::addSome() clusterIds, searchKeywords, linkModel->getValue(_linkType->currentIndex()), + Artist::NameSortMethod::BySortName, _container->count(), 20, moreResults)}; for (const auto& artist : artists) diff --git a/src/test/database/DatabaseTest.cpp b/src/test/database/DatabaseTest.cpp index 5ad5b60b..84df0f3b 100644 --- a/src/test/database/DatabaseTest.cpp +++ b/src/test/database/DatabaseTest.cpp @@ -172,7 +172,7 @@ testSingleArtist(Session& session) { auto transaction {session.createSharedTransaction()}; - auto artists {Artist::getAll(session)}; + auto artists {Artist::getAll(session, Artist::NameSortMethod::ByName)}; CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); @@ -311,11 +311,11 @@ testSingleTrackSingleArtistMultiRoles(Session& session) { auto transaction {session.createSharedTransaction()}; bool hasMore{}; - CHECK(Artist::getByFilter(session, {}, {}, {}, {}, {}, hasMore).size() == 1); - CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Artist, {}, {}, hasMore).size() == 1); - CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::ReleaseArtist, {}, {}, hasMore).size() == 1); - CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Writer, {}, {}, hasMore).size() == 1); - CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Composer, {}, {}, hasMore).empty()); + CHECK(Artist::getByFilter(session, {}, {}, std::nullopt, Artist::NameSortMethod::ByName, {}, {}, hasMore).size() == 1); + CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Artist, Artist::NameSortMethod::ByName, {}, {}, hasMore).size() == 1); + CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::ReleaseArtist, Artist::NameSortMethod::ByName, {}, {}, hasMore).size() == 1); + CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Writer, Artist::NameSortMethod::ByName, {}, {}, hasMore).size() == 1); + CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Composer, Artist::NameSortMethod::ByName, {}, {}, hasMore).empty()); } { @@ -369,7 +369,8 @@ testSingleTrackMultiArtists(Session& session) CHECK(track->getArtists(TrackArtistLink::Type::Artist).size() == 2); CHECK(track->getArtists(TrackArtistLink::Type::ReleaseArtist).empty()); - CHECK(Artist::getAll(session).size() == 2); + CHECK(Artist::getAll(session, Artist::NameSortMethod::ByName).size() == 2); + CHECK(Artist::getAllIds(session).size() == 2); } { @@ -699,12 +700,12 @@ testSingleTrackSingleArtistMultiClusters(Session& session) { auto transaction {session.createSharedTransaction()}; - auto artists {Artist::getByClusters(session, {cluster1.getId()})}; + auto artists {Artist::getByClusters(session, {cluster1.getId()}, Artist::NameSortMethod::ByName)}; CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); - CHECK(Artist::getByClusters(session, {cluster2.getId()}).empty()); - CHECK(Artist::getByClusters(session, {cluster3.getId()}).empty()); + CHECK(Artist::getByClusters(session, {cluster2.getId()}, Artist::NameSortMethod::ByName).empty()); + CHECK(Artist::getByClusters(session, {cluster3.getId()}, Artist::NameSortMethod::ByName).empty()); cluster2.get().modify()->addTrack(track.get()); } @@ -712,19 +713,19 @@ testSingleTrackSingleArtistMultiClusters(Session& session) { auto transaction {session.createSharedTransaction()}; - auto artists {Artist::getByClusters(session, {cluster1.getId()})}; + auto artists {Artist::getByClusters(session, {cluster1.getId()}, Artist::NameSortMethod::ByName)}; CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); - artists = Artist::getByClusters(session, {cluster2.getId()}); + artists = Artist::getByClusters(session, {cluster2.getId()}, Artist::NameSortMethod::ByName); CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); - artists = Artist::getByClusters(session, {cluster1.getId(), cluster2.getId()}); + artists = Artist::getByClusters(session, {cluster1.getId(), cluster2.getId()}, Artist::NameSortMethod::ByName); CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); - CHECK(Artist::getByClusters(session, {cluster3.getId()}).empty()); + CHECK(Artist::getByClusters(session, {cluster3.getId()}, Artist::NameSortMethod::ByName).empty()); } } @@ -755,7 +756,7 @@ testSingleTrackSingleArtistMultiRolesMultiClusters(Session& session) { auto transaction {session.createSharedTransaction()}; - auto artists {Artist::getByClusters(session, {cluster.getId()})}; + auto artists {Artist::getByClusters(session, {cluster.getId()}, Artist::NameSortMethod::ByName)}; CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); } @@ -799,7 +800,7 @@ testMultiTracksSingleArtistMultiClusters(Session& session) std::set clusterIds; std::transform(std::cbegin(clusters), std::cend(clusters), std::inserter(clusterIds, std::begin(clusterIds)), [](const ScopedCluster& cluster) { return cluster.getId(); }); - auto artists {Artist::getByClusters(session, clusterIds)}; + auto artists {Artist::getByClusters(session, clusterIds, Artist::NameSortMethod::ByName)}; CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); } @@ -914,7 +915,7 @@ testSingleTrackSingleReleaseSingleArtistSingleCluster(Session& session) { auto transaction {session.createSharedTransaction()}; - auto artists {Artist::getByClusters(session, {cluster.getId()})}; + auto artists {Artist::getByClusters(session, {cluster.getId()}, Artist::NameSortMethod::ByName)}; CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); @@ -1346,7 +1347,7 @@ testDatabaseEmpty(Session& session) { auto uniqueTransaction {session.createUniqueTransaction()}; - CHECK(Artist::getAll(session).empty()); + CHECK(Artist::getAll(session, Artist::NameSortMethod::ByName).empty()); CHECK(Cluster::getAll(session).empty()); CHECK(ClusterType::getAll(session).empty()); CHECK(Release::getAll(session).empty());