[Fix] Implement efficient directory query in handleGetMusicDirectoryRequest()

src/libs/database/impl/objects/Directory.cpp

@@ -24,6 +24,7 @@
 #include "database/Session.hpp"
 #include "database/Types.hpp"
 #include "database/objects/MediaLibrary.hpp"
+#include "database/objects/Release.hpp"
 
 #include "Utils.hpp"
 #include "traits/IdTypeTraits.hpp"
@@ -225,6 +226,41 @@ namespace lms::db
         return utils::execRangeQuery<Directory::pointer>(query, range);
     }
 
+    std::vector<Directory::ChildRelease> Directory::findChildReleases(Session& session, DirectoryId parentDirectory)
+    {
+        session.checkReadTransaction();
+
+        // Resolves the whole listing at once.
+        //
+        // Read the joins from the track outwards, since tracks are what tie the two together (a
+        // directory has no direct link to a release):
+        //   track -> directory restricts to tracks whose directory is a child of parentDirectory
+        //   track -> release turns the track's release_id into a release row we can return
+        //
+        // Both are INNER JOINs, so a child directory holding no track, or only tracks belonging to
+        // no release, produces no row at all. Such directories are absent from the result rather
+        // than present with an empty release.
+        //
+        // GROUP BY then collapses the many tracks of a directory down to one row per directory.
+        //
+        // The count is returned rather than acted on here so each caller can apply its own rule:
+        // subsonic treats any release as the directory's album, while the folder view only links
+        // straight to a release when the directory holds exactly one.
+        auto query{ session.getDboSession()->query<std::tuple<DirectoryId, Wt::Dbo::ptr<Release>, int>>(
+            "SELECT t.directory_id, r, COUNT(DISTINCT t.release_id)"
+            " FROM track t"
+            " INNER JOIN directory d ON d.id = t.directory_id"
+            " INNER JOIN release r ON r.id = t.release_id") };
+        query.where("d.parent_directory_id = ?").bind(parentDirectory);
+        query.groupBy("t.directory_id");
+
+        std::vector<ChildRelease> result;
+        for (const auto& [directoryId, release, releaseCount] : utils::fetchQueryResults<std::tuple<DirectoryId, Wt::Dbo::ptr<Release>, int>>(query))
+            result.emplace_back(ChildRelease{ directoryId, release, static_cast<std::size_t>(releaseCount) });
+
+        return result;
+    }
+
     void Directory::setAbsolutePath(const std::filesystem::path& p)
     {
         assert(p.is_absolute());

src/libs/database/include/database/objects/Directory.hpp

@@ -43,6 +43,7 @@ namespace lms::db
 {
     class Session;
     class MediaLibrary;
+    class Release;
 
     class Directory final : public Object<Directory, DirectoryId>
     {
@@ -128,6 +129,14 @@ namespace lms::db
         static std::vector<DirectoryId> findMismatchedLibrary(Session& session, std::optional<Range> range, const std::filesystem::path& rootPath, MediaLibraryId expectedLibraryId);
         static std::vector<pointer> findRootDirectories(Session& session, std::optional<Range> range = std::nullopt);
 
+        struct ChildRelease
+        {
+            DirectoryId directory;
+            ObjectPtr<Release> release; 
+            std::size_t releaseCount;
+        };
+        static std::vector<ChildRelease> findChildReleases(Session& session, DirectoryId parentDirectory);
+
         // getters
         const std::filesystem::path& getAbsolutePath() const { return _absolutePath; }
         std::string_view getName() const { return _name; }

src/libs/database/test/Directory.cpp

@@ -19,6 +19,8 @@
 
 #include "Common.hpp"
 
+#include <unordered_map>
+
 #include "database/objects/Directory.hpp"
 #include "database/objects/Medium.hpp"
 
@@ -340,4 +342,66 @@ namespace lms::db::tests
             EXPECT_EQ(visitedDirectories[0], dir1.getId());
         }
     }
+
+    TEST_F(DatabaseFixture, Directory_findChildReleases)
+    {
+        ScopedDirectory root{ session, "/root" };
+        ScopedDirectory singleRelease{ session, "/root/single" };
+        ScopedDirectory multiRelease{ session, "/root/multi" };
+        ScopedDirectory noRelease{ session, "/root/none" };
+        ScopedDirectory notAChild{ session, "/elsewhere" };
+        ScopedRelease release1{ session, "Release1" };
+        ScopedRelease release2{ session, "Release2" };
+        ScopedRelease release3{ session, "Release3" };
+        ScopedTrack track1{ session };
+        ScopedTrack track2{ session };
+        ScopedTrack track3{ session };
+        ScopedTrack trackWithoutRelease{ session };
+        ScopedTrack trackElsewhere{ session };
+
+        {
+            auto transaction{ session.createWriteTransaction() };
+
+            singleRelease.get().modify()->setParent(root.get());
+            multiRelease.get().modify()->setParent(root.get());
+            noRelease.get().modify()->setParent(root.get());
+
+            track1.get().modify()->setDirectory(singleRelease.get());
+            track1.get().modify()->setRelease(release1.get());
+
+            track2.get().modify()->setDirectory(multiRelease.get());
+            track2.get().modify()->setRelease(release2.get());
+            track3.get().modify()->setDirectory(multiRelease.get());
+            track3.get().modify()->setRelease(release3.get());
+
+            trackWithoutRelease.get().modify()->setDirectory(noRelease.get());
+            trackElsewhere.get().modify()->setDirectory(notAChild.get());
+            trackElsewhere.get().modify()->setRelease(release1.get());
+        }
+
+        {
+            auto transaction{ session.createReadTransaction() };
+
+            const auto results{ Directory::findChildReleases(session, root.getId()) };
+            ASSERT_EQ(results.size(), 2);
+
+            std::unordered_map<DirectoryId::ValueType, Directory::ChildRelease> byDirectory;
+            for (const Directory::ChildRelease& childRelease : results)
+                byDirectory.emplace(childRelease.directory.getValue(), childRelease);
+
+            const Directory::ChildRelease& single{ byDirectory.at(singleRelease.getId().getValue()) };
+            EXPECT_EQ(single.releaseCount, 1);
+            ASSERT_NE(single.release, Release::pointer{});
+            EXPECT_EQ(single.release->getId(), release1.getId());
+
+            const Directory::ChildRelease& multi{ byDirectory.at(multiRelease.getId().getValue()) };
+            EXPECT_EQ(multi.releaseCount, 2);
+            ASSERT_NE(multi.release, Release::pointer{});
+
+            // a directory whose tracks have no release is not reported at all
+            EXPECT_EQ(byDirectory.count(noRelease.getId().getValue()), 0);
+            // nor is a directory that is not a child of the requested parent
+            EXPECT_EQ(byDirectory.count(notAChild.getId().getValue()), 0);
+        }
+    }
 } // namespace lms::db::tests

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

@@ -338,8 +338,21 @@ namespace lms::api::subsonic
             params.setParentDirectory(directory->getId());
             params.setSortMethod(DirectorySortMethod::Name);
 
-            Directory::find(context.getDbSession(), params, [&](const Directory::pointer& subDirectory) {
-                const Release::pointer release{ getReleaseFromDirectory(context.getDbSession(), subDirectory->getId()) };
+            const std::vector<Directory::pointer> subDirectories{ Directory::find(context.getDbSession(), params) };
+
+            std::unordered_map<DirectoryId::ValueType, Release::pointer> releaseByDirectory;
+            // skip leaf directories, which are the common case
+            if (!subDirectories.empty())
+            {
+                for (const Directory::ChildRelease& childRelease : Directory::findChildReleases(context.getDbSession(), directory->getId()))
+                    releaseByDirectory.emplace(childRelease.directory.getValue(), childRelease.release);
+            }
+
+            for (const Directory::pointer& subDirectory : subDirectories)
+            {
+                Release::pointer release;
+                if (const auto it{ releaseByDirectory.find(subDirectory->getId().getValue()) }; it != std::cend(releaseByDirectory))
+                    release = it->second;
 
                 if (release)
                 {
@@ -355,7 +368,7 @@ namespace lms::api::subsonic
 
                     directoryNode.addArrayChild("child", std::move(childNode));
                 }
-            });
+            }
         }
 
         // list all tracks