Handle sorting method for artists

src/libs/database/impl/Artist.cpp

@@ -74,57 +74,13 @@ Artist::create(Session& session, const std::string& name, const std::optional<UU
 	return res;
 }
 
-std::vector<Artist::pointer>
-Artist::getAll(Session& session, std::optional<std::size_t> offset, std::optional<std::size_t> size)
-{
-	session.checkSharedLocked();
-	Wt::Dbo::collection<pointer> res = session.getDboSession().find<Artist>()
-		.offset(offset ? static_cast<int>(*offset) : -1)
-		.limit(size ? static_cast<int>(*size) : -1)
-		.orderBy("sort_name COLLATE NOCASE");
-
-	return std::vector<pointer>(res.begin(), res.end());
-}
-
-std::vector<IdType>
-Artist::getAllIds(Session& session)
-{
-	session.checkSharedLocked();
-
-	Wt::Dbo::collection<IdType> res = session.getDboSession().query<IdType>("SELECT id FROM artist");
-	return std::vector<IdType>(res.begin(), res.end());
-}
-
-std::vector<Artist::pointer>
-Artist::getAllOrphans(Session& session)
-{
-	session.checkSharedLocked();
-	Wt::Dbo::collection<Wt::Dbo::ptr<Artist>> res {session.getDboSession().query<Wt::Dbo::ptr<Artist>>("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<pointer>(res.begin(), res.end());
-}
-
-std::vector<IdType>
-Artist::getAllIdsWithClusters(Session& session, std::optional<std::size_t> limit)
-{
-	session.checkSharedLocked();
-
-	Wt::Dbo::collection<IdType> res = session.getDboSession().query<IdType>
-		("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<int>(*limit) : -1);
-
-	return std::vector<IdType>(res.begin(), res.end());
-}
-
-
 static
 Wt::Dbo::Query<Artist::pointer>
 getQuery(Session& session,
 		const std::set<IdType>& clusterIds,
 		const std::vector<std::string>& keywords,
-		std::optional<TrackArtistLink::Type> linkType)
+		std::optional<TrackArtistLink::Type> 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<Artist::pointer> query = session.getDboSession().query<Artist::pointer>( oss.str() );
 
@@ -170,6 +136,50 @@ getQuery(Session& session,
 	return query;
 }
 
+std::vector<Artist::pointer>
+Artist::getAll(Session& session, NameSortMethod sortMethod, std::optional<std::size_t> offset, std::optional<std::size_t> size)
+{
+	session.checkSharedLocked();
+
+	Wt::Dbo::collection<Artist::pointer> res = getQuery(session, {}, {}, std::nullopt, sortMethod)
+		.limit(size ? static_cast<int>(*size) + 1 : -1)
+		.offset(offset ? static_cast<int>(*offset) : -1);
+
+	return std::vector<pointer>(res.begin(), res.end());
+}
+
+std::vector<IdType>
+Artist::getAllIds(Session& session)
+{
+	session.checkSharedLocked();
+
+	Wt::Dbo::collection<IdType> res = session.getDboSession().query<IdType>("SELECT id FROM artist");
+	return std::vector<IdType>(res.begin(), res.end());
+}
+
+std::vector<Artist::pointer>
+Artist::getAllOrphans(Session& session)
+{
+	session.checkSharedLocked();
+	Wt::Dbo::collection<Wt::Dbo::ptr<Artist>> res {session.getDboSession().query<Wt::Dbo::ptr<Artist>>("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<pointer>(res.begin(), res.end());
+}
+
+std::vector<IdType>
+Artist::getAllIdsWithClusters(Session& session, std::optional<std::size_t> limit)
+{
+	session.checkSharedLocked();
+
+	Wt::Dbo::collection<IdType> res = session.getDboSession().query<IdType>
+		("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<int>(*limit) : -1);
+
+	return std::vector<IdType>(res.begin(), res.end());
+}
+
 std::vector<Artist::pointer>
 Artist::getByClusters(Session& session, const std::set<IdType>& clusters, NameSortMethod sortMethod)
 {
@@ -191,7 +201,7 @@ Artist::getByFilter(Session& session,
 		bool& moreResults)
 {
 	session.checkSharedLocked();
-	Wt::Dbo::collection<Artist::pointer> collection = getQuery(session, clusters, keywords, linkType)
+	Wt::Dbo::collection<Artist::pointer> collection = getQuery(session, clusters, keywords, linkType, sortMethod)
 		.limit(size ? static_cast<int>(*size) + 1 : -1)
 		.offset(offset ? static_cast<int>(*offset) : -1);
 

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));
 	}

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)

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, {}, {}, std::nullopt, Artist::NameSortMethod::ByName, {}, {}, hasMore).size() == 1);
-		CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Artist, {}, {}, hasMore).size() == 1);
+		CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Artist, Artist::NameSortMethod::ByName, {}, {}, hasMore).size() == 1);
-		CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::ReleaseArtist, {}, {}, hasMore).size() == 1);
+		CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::ReleaseArtist, Artist::NameSortMethod::ByName, {}, {}, hasMore).size() == 1);
-		CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Writer, {}, {}, hasMore).size() == 1);
+		CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Writer, Artist::NameSortMethod::ByName, {}, {}, hasMore).size() == 1);
-		CHECK(Artist::getByFilter(session, {}, {}, TrackArtistLink::Type::Composer, {}, {}, hasMore).empty());
+		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, {cluster2.getId()}, Artist::NameSortMethod::ByName).empty());
-		CHECK(Artist::getByClusters(session, {cluster3.getId()}).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<IdType> 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());