Subsonic API: do not block the whole application while handling a getArtist. But this is really best effort as it is way too long

src/libs/subsonic/impl/entrypoints/Browsing.cpp

@@ -26,6 +26,7 @@
 #include "services/database/Track.hpp"
 #include "services/database/User.hpp"
 #include "services/recommendation/IRecommendationService.hpp"
+#include "utils/Logger.hpp"
 #include "utils/Random.hpp"
 #include "utils/Service.hpp"
 #include "responses/Album.hpp"
@@ -94,52 +95,79 @@ namespace API::Subsonic
 
             Response::Node& artistsNode{ response.createNode(id3 ? "artists" : "indexes") };
             artistsNode.setAttribute("ignoredArticles", "");
-            artistsNode.setAttribute("lastModified", reportedDummyDateULong);
+            artistsNode.setAttribute("lastModified", reportedDummyDateULong); // TODO report last file write?
-
-            auto transaction{ context.dbSession.createSharedTransaction() };
-
-            User::pointer user{ User::find(context.dbSession, context.userId) };
-            if (!user)
-                throw UserNotAuthorizedError{};
 
             Artist::FindParameters parameters;
-            parameters.setSortMethod(ArtistSortMethod::BySortName);
-            switch (user->getSubsonicArtistListMode())
             {
-            case SubsonicArtistListMode::AllArtists:
+                auto transaction{ context.dbSession.createSharedTransaction() };
-                break;
+
-            case SubsonicArtistListMode::ReleaseArtists:
+                User::pointer user{ User::find(context.dbSession, context.userId) };
-                parameters.setLinkType(TrackArtistLinkType::ReleaseArtist);
+                if (!user)
-                break;
+                    throw UserNotAuthorizedError{};
-            case SubsonicArtistListMode::TrackArtists:
+
-                parameters.setLinkType(TrackArtistLinkType::Artist);
+                parameters.setSortMethod(ArtistSortMethod::BySortName);
-                break;
+                switch (user->getSubsonicArtistListMode())
+                {
+                case SubsonicArtistListMode::AllArtists:
+                    break;
+                case SubsonicArtistListMode::ReleaseArtists:
+                    parameters.setLinkType(TrackArtistLinkType::ReleaseArtist);
+                    break;
+                case SubsonicArtistListMode::TrackArtists:
+                    parameters.setLinkType(TrackArtistLinkType::Artist);
+                    break;
+                }
             }
 
-            std::map<char, std::vector<Artist::pointer>> artistsSortedByFirstChar;
+            // This endpoint does not scale: make sort lived transactions in order not to block the whole application
-            const RangeResults<ArtistId> artists{ Artist::find(context.dbSession, parameters) };
+
-            for (const ArtistId artistId : artists.results)
+            // first pass: dispatch the artists by first letter
+            LMS_LOG(API_SUBSONIC, DEBUG) << "GetArtists: fetching all artists...";
+            std::map<char, std::vector<ArtistId>> artistsSortedByFirstChar;
+            std::size_t currentArtistOffset{0};
+            constexpr std::size_t batchSize{ 100 };
+            bool hasMoreArtists{ true };
+            while (hasMoreArtists)
             {
-                const Artist::pointer artist{ Artist::find(context.dbSession, artistId) };
+                auto transaction{ context.dbSession.createSharedTransaction() };
-                const std::string& sortName{ artist->getSortName() };
 
-                char sortChar;
+                parameters.setRange(Range{ currentArtistOffset, batchSize });
-                if (sortName.empty() || !std::isalpha(sortName[0]))
+                const RangeResults<ArtistId> artists{ Artist::find(context.dbSession, parameters) };
-                    sortChar = '?';
+                for (const ArtistId artistId : artists.results)
-                else
+                {
-                    sortChar = std::toupper(sortName[0]);
+                    const Artist::pointer artist{ Artist::find(context.dbSession, artistId) };
+                    std::string_view sortName{ artist->getSortName() };
 
-                artistsSortedByFirstChar[sortChar].push_back(artist);
+                    char sortChar;
+                    if (sortName.empty() || !std::isalpha(sortName[0]))
+                        sortChar = '?';
+                    else
+                        sortChar = std::toupper(sortName[0]);
+
+                    artistsSortedByFirstChar[sortChar].push_back(artistId);
+                }
+
+                hasMoreArtists = artists.moreResults;
+                currentArtistOffset += artists.results.size();
             }
 
-
+            // second pass: add each artist
-            for (const auto& [sortChar, artists] : artistsSortedByFirstChar)
+            LMS_LOG(API_SUBSONIC, DEBUG) << "GetArtists: constructing response...";
+            for (const auto& [sortChar, artistIds] : artistsSortedByFirstChar)
             {
                 Response::Node& indexNode{ artistsNode.createArrayChild("index") };
                 indexNode.setAttribute("name", std::string{ sortChar });
 
-                for (const Artist::pointer& artist : artists)
+                for (const ArtistId artistId : artistIds)
-                    indexNode.addArrayChild("artist", createArtistNode(artist, context.dbSession, user, id3));
+                {
+                    auto transaction{ context.dbSession.createSharedTransaction() };
+                    User::pointer user{ User::find(context.dbSession, context.userId) };
+                    if (!user)
+                        throw UserNotAuthorizedError{};
+
+                    if (const Artist::pointer artist{ Artist::find(context.dbSession, artistId) })
+                        indexNode.addArrayChild("artist", createArtistNode(artist, context.dbSession, user, id3));
+                }
             }
 
             return response;