diff --git a/src/libs/database/impl/Artist.cpp b/src/libs/database/impl/Artist.cpp index 3c3f3f9b..8f310180 100644 --- a/src/libs/database/impl/Artist.cpp +++ b/src/libs/database/impl/Artist.cpp @@ -80,7 +80,7 @@ getQuery(Session& session, const std::set& clusterIds, const std::vector& keywords, std::optional linkType, - Artist::NameSortMethod sortMethod) + Artist::SortMethod sortMethod) { session.checkSharedLocked(); @@ -89,8 +89,19 @@ getQuery(Session& session, std::ostringstream oss; oss << "SELECT DISTINCT a FROM artist a"; - for (auto keyword : keywords) - where.And(WhereClause("a.name LIKE ?")).bind("%%" + keyword + "%%"); + if (!keywords.empty()) + { + WhereClause whereKeywordsName; + WhereClause whereKeywordsSortName; + + for (auto keyword : keywords) + { + whereKeywordsName.And(WhereClause("a.name LIKE ?")).bind("%%" + keyword + "%%"); + whereKeywordsSortName.And(WhereClause("a.sort_name LIKE ?")).bind("%%" + keyword + "%%"); + } + + where.And(whereKeywordsName.Or(whereKeywordsSortName)); + } if (!clusterIds.empty() || linkType) { @@ -116,12 +127,12 @@ getQuery(Session& session, switch (sortMethod) { - case Artist::NameSortMethod::None: + case Artist::SortMethod::None: break; - case Artist::NameSortMethod::ByName: + case Artist::SortMethod::ByName: oss << " ORDER BY a.name COLLATE NOCASE"; break; - case Artist::NameSortMethod::BySortName: + case Artist::SortMethod::BySortName: oss << " ORDER BY a.sort_name COLLATE NOCASE"; break; } @@ -137,7 +148,7 @@ getQuery(Session& session, } std::vector -Artist::getAll(Session& session, NameSortMethod sortMethod, std::optional offset, std::optional size) +Artist::getAll(Session& session, SortMethod sortMethod, std::optional offset, std::optional size) { session.checkSharedLocked(); @@ -181,7 +192,7 @@ Artist::getAllIdsWithClusters(Session& session, std::optional limit } std::vector -Artist::getByClusters(Session& session, const std::set& clusters, NameSortMethod sortMethod) +Artist::getByClusters(Session& session, const std::set& clusters, SortMethod sortMethod) { assert(!clusters.empty()); @@ -195,7 +206,7 @@ Artist::getByFilter(Session& session, const std::set& clusters, const std::vector& keywords, std::optional linkType, - NameSortMethod sortMethod, + SortMethod sortMethod, std::optional offset, std::optional size, bool& moreResults) diff --git a/src/libs/database/include/database/Artist.hpp b/src/libs/database/include/database/Artist.hpp index 44fa339c..f6f5df7b 100644 --- a/src/libs/database/include/database/Artist.hpp +++ b/src/libs/database/include/database/Artist.hpp @@ -45,7 +45,7 @@ class Artist : public Wt::Dbo::Dbo { public: - enum class NameSortMethod + enum class SortMethod { None, ByName, @@ -60,21 +60,21 @@ class Artist : public Wt::Dbo::Dbo // Accessors static pointer getByMBID(Session& session, const UUID& MBID); static pointer getById(Session& session, IdType id); - static std::vector getByName(Session& session, const std::string& name); + static std::vector getByName(Session& session, const std::string& name); // exact match on name field static std::vector getByClusters(Session& session, const std::set& clusters, // at least one track that belongs to these clusters - NameSortMethod sortMethod + SortMethod sortMethod ); static std::vector getByFilter(Session& session, const std::set& clusters, // if non empty, at least one artist that belongs to these clusters - const std::vector& keywords, // if non empty, name must match all of these keywords + const std::vector& keywords, // if non empty, name must match all of these keywords (name + sort name fields) std::optional linkType, // if set, only artists that have produced at least one track with this link type - NameSortMethod sortMethod, + SortMethod sortMethod, std::optional offset, std::optional size, bool& moreExpected); - static std::vector getAll(Session& session, NameSortMethod sortMethod, std::optional offset = {}, std::optional size = {}); + static std::vector getAll(Session& session, SortMethod sortMethod, std::optional offset = {}, std::optional size = {}); static std::vector getAllIds(Session& session); static std::vector getAllOrphans(Session& session); // No track related static std::vector getLastAdded(Session& session, Wt::WDateTime after, std::optional size = {}); diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp index 0583db25..80c69a85 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, Artist::NameSortMethod::ByName)}; + auto artists {Artist::getAll(context.dbSession, Artist::SortMethod::BySortName)}; 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, Artist::NameSortMethod::ByName)}; + auto artists {Artist::getAll(context.dbSession, Artist::SortMethod::BySortName)}; 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, Artist::NameSortMethod::ByName)}; + auto artists {Artist::getAll(context.dbSession, Artist::SortMethod::BySortName)}; 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, std::nullopt, Artist::NameSortMethod::ByName, artistOffset, artistCount, more)}; + auto artists {Artist::getByFilter(context.dbSession, {}, keywords, std::nullopt, Artist::SortMethod::BySortName, 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 400b206a..28d1bfc9 100644 --- a/src/lms/ui/explore/ArtistsView.cpp +++ b/src/lms/ui/explore/ArtistsView.cpp @@ -95,7 +95,7 @@ Artists::addSome() clusterIds, searchKeywords, linkModel->getValue(_linkType->currentIndex()), - Artist::NameSortMethod::BySortName, + Artist::SortMethod::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 84df0f3b..68e19288 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, Artist::NameSortMethod::ByName)}; + auto artists {Artist::getAll(session, Artist::SortMethod::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, {}, {}, 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()); + CHECK(Artist::getByFilter(session, {}, {}, std::nullopt, Artist::SortMethod::ByName, {}, {}, hasMore).size() == 1); + CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Artist, Artist::SortMethod::ByName, {}, {}, hasMore).size() == 1); + CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::ReleaseArtist, Artist::SortMethod::ByName, {}, {}, hasMore).size() == 1); + CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Writer, Artist::SortMethod::ByName, {}, {}, hasMore).size() == 1); + CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Composer, Artist::SortMethod::ByName, {}, {}, hasMore).empty()); } { @@ -369,7 +369,7 @@ testSingleTrackMultiArtists(Session& session) CHECK(track->getArtists(TrackArtistLink::Type::Artist).size() == 2); CHECK(track->getArtists(TrackArtistLink::Type::ReleaseArtist).empty()); - CHECK(Artist::getAll(session, Artist::NameSortMethod::ByName).size() == 2); + CHECK(Artist::getAll(session, Artist::SortMethod::ByName).size() == 2); CHECK(Artist::getAllIds(session).size() == 2); } @@ -386,6 +386,65 @@ testSingleTrackMultiArtists(Session& session) } } +static +void +testSingleArtistSearchByName(Session& session) +{ + ScopedArtist artist {session, "AAA"}; + + { + auto transaction {session.createUniqueTransaction()}; + artist.get().modify()->setSortName("ZZZ"); + } + + { + auto transaction {session.createSharedTransaction()}; + + bool more {}; + CHECK(Artist::getByFilter(session, {}, {"N"}, std::nullopt, Artist::SortMethod::ByName, std::nullopt, std::nullopt, more).empty()); + + const auto artistsByAAA {Artist::Artist::getByFilter(session, {}, {"A"}, std::nullopt, Artist::SortMethod::ByName, std::nullopt, std::nullopt, more)}; + CHECK(artistsByAAA.size() == 1); + CHECK(artistsByAAA.front().id() == artist.getId()); + + const auto artistsByZZZ {Artist::Artist::getByFilter(session, {}, {"Z"}, std::nullopt, Artist::SortMethod::ByName, std::nullopt, std::nullopt, more)}; + CHECK(artistsByZZZ.size() == 1); + CHECK(artistsByZZZ.front().id() == artist.getId()); + + CHECK(Artist::getByName(session, "NNN").empty()); + } +} + +static +void +testMultiArtistsSortMethod(Session& session) +{ + ScopedArtist artistA {session, "artistA"}; + ScopedArtist artistB {session, "artistB"}; + + { + auto transaction {session.createUniqueTransaction()}; + + artistA.get().modify()->setSortName("sortNameB"); + artistB.get().modify()->setSortName("sortNameA"); + } + + { + auto transaction {session.createSharedTransaction()}; + + auto allArtistsByName {Artist::getAll(session, Artist::SortMethod::ByName)}; + auto allArtistsBySortName {Artist::getAll(session, Artist::SortMethod::BySortName)}; + + CHECK(allArtistsByName.size() == 2); + CHECK(allArtistsByName.front().id() == artistA.getId()); + CHECK(allArtistsByName.back().id() == artistB.getId()); + + CHECK(allArtistsBySortName.size() == 2); + CHECK(allArtistsBySortName.front().id() == artistB.getId()); + CHECK(allArtistsBySortName.back().id() == artistA.getId()); + } +} + static void testSingleTrackSingleRelease(Session& session) @@ -700,12 +759,12 @@ testSingleTrackSingleArtistMultiClusters(Session& session) { auto transaction {session.createSharedTransaction()}; - auto artists {Artist::getByClusters(session, {cluster1.getId()}, Artist::NameSortMethod::ByName)}; + auto artists {Artist::getByClusters(session, {cluster1.getId()}, Artist::SortMethod::ByName)}; CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); - CHECK(Artist::getByClusters(session, {cluster2.getId()}, Artist::NameSortMethod::ByName).empty()); - CHECK(Artist::getByClusters(session, {cluster3.getId()}, Artist::NameSortMethod::ByName).empty()); + CHECK(Artist::getByClusters(session, {cluster2.getId()}, Artist::SortMethod::ByName).empty()); + CHECK(Artist::getByClusters(session, {cluster3.getId()}, Artist::SortMethod::ByName).empty()); cluster2.get().modify()->addTrack(track.get()); } @@ -713,19 +772,19 @@ testSingleTrackSingleArtistMultiClusters(Session& session) { auto transaction {session.createSharedTransaction()}; - auto artists {Artist::getByClusters(session, {cluster1.getId()}, Artist::NameSortMethod::ByName)}; + auto artists {Artist::getByClusters(session, {cluster1.getId()}, Artist::SortMethod::ByName)}; CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); - artists = Artist::getByClusters(session, {cluster2.getId()}, Artist::NameSortMethod::ByName); + artists = Artist::getByClusters(session, {cluster2.getId()}, Artist::SortMethod::ByName); CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); - artists = Artist::getByClusters(session, {cluster1.getId(), cluster2.getId()}, Artist::NameSortMethod::ByName); + artists = Artist::getByClusters(session, {cluster1.getId(), cluster2.getId()}, Artist::SortMethod::ByName); CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); - CHECK(Artist::getByClusters(session, {cluster3.getId()}, Artist::NameSortMethod::ByName).empty()); + CHECK(Artist::getByClusters(session, {cluster3.getId()}, Artist::SortMethod::ByName).empty()); } } @@ -756,7 +815,7 @@ testSingleTrackSingleArtistMultiRolesMultiClusters(Session& session) { auto transaction {session.createSharedTransaction()}; - auto artists {Artist::getByClusters(session, {cluster.getId()}, Artist::NameSortMethod::ByName)}; + auto artists {Artist::getByClusters(session, {cluster.getId()}, Artist::SortMethod::ByName)}; CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); } @@ -800,7 +859,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, Artist::NameSortMethod::ByName)}; + auto artists {Artist::getByClusters(session, clusterIds, Artist::SortMethod::ByName)}; CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); } @@ -915,7 +974,7 @@ testSingleTrackSingleReleaseSingleArtistSingleCluster(Session& session) { auto transaction {session.createSharedTransaction()}; - auto artists {Artist::getByClusters(session, {cluster.getId()}, Artist::NameSortMethod::ByName)}; + auto artists {Artist::getByClusters(session, {cluster.getId()}, Artist::SortMethod::ByName)}; CHECK(artists.size() == 1); CHECK(artists.front().id() == artist.getId()); @@ -1347,7 +1406,7 @@ testDatabaseEmpty(Session& session) { auto uniqueTransaction {session.createUniqueTransaction()}; - CHECK(Artist::getAll(session, Artist::NameSortMethod::ByName).empty()); + CHECK(Artist::getAll(session, Artist::SortMethod::ByName).empty()); CHECK(Cluster::getAll(session).empty()); CHECK(ClusterType::getAll(session).empty()); CHECK(Release::getAll(session).empty()); @@ -1398,6 +1457,9 @@ int main() RUN_TEST(testSingleTrackSingleArtistMultiRoles); RUN_TEST(testSingleTrackMultiArtists); + RUN_TEST(testSingleArtistSearchByName); + RUN_TEST(testMultiArtistsSortMethod); + RUN_TEST(testSingleTrackSingleRelease); RUN_TEST(testSingleTrackSingleCluster);