Sort artists using sortname tag + search by name also in this tag. Fixes #44

src/libs/database/impl/Artist.cpp

@@ -80,7 +80,7 @@ getQuery(Session& session,
 		const std::set<IdType>& clusterIds,
 		const std::vector<std::string>& keywords,
 		std::optional<TrackArtistLink::Type> 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)
+	if (!keywords.empty())
-		where.And(WhereClause("a.name LIKE ?")).bind("%%" + keyword + "%%");
+	{
+		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::pointer>
-Artist::getAll(Session& session, NameSortMethod sortMethod, std::optional<std::size_t> offset, std::optional<std::size_t> size)
+Artist::getAll(Session& session, SortMethod sortMethod, std::optional<std::size_t> offset, std::optional<std::size_t> size)
 {
 	session.checkSharedLocked();
 
@@ -181,7 +192,7 @@ Artist::getAllIdsWithClusters(Session& session, std::optional<std::size_t> limit
 }
 
 std::vector<Artist::pointer>
-Artist::getByClusters(Session& session, const std::set<IdType>& clusters, NameSortMethod sortMethod)
+Artist::getByClusters(Session& session, const std::set<IdType>& clusters, SortMethod sortMethod)
 {
 	assert(!clusters.empty());
 
@@ -195,7 +206,7 @@ Artist::getByFilter(Session& session,
 		const std::set<IdType>& clusters,
 		const std::vector<std::string>& keywords,
 		std::optional<TrackArtistLink::Type> linkType,
-		NameSortMethod sortMethod,
+		SortMethod sortMethod,
 		std::optional<std::size_t> offset,
 		std::optional<std::size_t> size,
 		bool& moreResults)

src/libs/database/include/database/Artist.hpp

@@ -45,7 +45,7 @@ class Artist : public Wt::Dbo::Dbo<Artist>
 {
 	public:
 
-		enum class NameSortMethod
+		enum class SortMethod
 		{
 			None,
 			ByName,
@@ -60,21 +60,21 @@ class Artist : public Wt::Dbo::Dbo<Artist>
 		// Accessors
 		static pointer			getByMBID(Session& session, const UUID& MBID);
 		static pointer			getById(Session& session, IdType id);
-		static std::vector<pointer>	getByName(Session& session, const std::string& name);
+		static std::vector<pointer>	getByName(Session& session, const std::string& name);		// exact match on name field
 		static std::vector<pointer> 	getByClusters(Session& session,
 								const std::set<IdType>& clusters,		// at least one track that belongs to  these clusters
-								NameSortMethod sortMethod
+								SortMethod sortMethod
 								);
 		static std::vector<pointer> 	getByFilter(Session& session,
 								const std::set<IdType>& clusters,		// if non empty, at least one artist that belongs to these clusters
-								const std::vector<std::string>& keywords,	// if non empty, name must match all of these keywords
+								const std::vector<std::string>& keywords,	// if non empty, name must match all of these keywords (name + sort name fields)
 								std::optional<TrackArtistLink::Type> linkType, 	// if set, only artists that have produced at least one track with this link type
-								NameSortMethod sortMethod,
+								SortMethod sortMethod,
 								std::optional<std::size_t> offset,
 								std::optional<std::size_t> size,
 								bool& moreExpected);
 
-		static std::vector<pointer>	getAll(Session& session, NameSortMethod sortMethod, std::optional<std::size_t> offset = {}, std::optional<std::size_t> size = {});
+		static std::vector<pointer>	getAll(Session& session, SortMethod sortMethod, std::optional<std::size_t> offset = {}, std::optional<std::size_t> size = {});
 		static std::vector<IdType>	getAllIds(Session& session);
 		static std::vector<pointer>	getAllOrphans(Session& session); // No track related
 		static std::vector<pointer>	getLastAdded(Session& session, Wt::WDateTime after, std::optional<std::size_t> size = {});

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

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)

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