From 1ee6c465be1f1ab18495a34df6a3a5611734213c Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Thu, 6 Aug 2026 14:29:30 +0100 Subject: [PATCH 1/3] Added Directory::findChildReleases Returns the releases held by the direct children of a directory in a single query, for callers that list a directory and need each child's release. --- src/libs/database/impl/objects/Directory.cpp | 36 +++++++++++ .../include/database/objects/Directory.hpp | 11 ++++ src/libs/database/test/Directory.cpp | 64 +++++++++++++++++++ 3 files changed, 111 insertions(+) diff --git a/src/libs/database/impl/objects/Directory.cpp b/src/libs/database/impl/objects/Directory.cpp index 822ce2736..41353f4d4 100644 --- a/src/libs/database/impl/objects/Directory.cpp +++ b/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(query, range); } + std::vector 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, 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 result; + for (const auto& [directoryId, release, releaseCount] : utils::fetchQueryResults, int>>(query)) + result.emplace_back(ChildRelease{ directoryId, release, static_cast(releaseCount) }); + + return result; + } + void Directory::setAbsolutePath(const std::filesystem::path& p) { assert(p.is_absolute()); diff --git a/src/libs/database/include/database/objects/Directory.hpp b/src/libs/database/include/database/objects/Directory.hpp index dd30df7c1..1daa6e1a4 100644 --- a/src/libs/database/include/database/objects/Directory.hpp +++ b/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 { @@ -128,6 +129,16 @@ namespace lms::db static std::vector findMismatchedLibrary(Session& session, std::optional range, const std::filesystem::path& rootPath, MediaLibraryId expectedLibraryId); static std::vector findRootDirectories(Session& session, std::optional range = std::nullopt); + struct ChildRelease + { + DirectoryId directory; + ObjectPtr release; // arbitrary one if the directory holds several + std::size_t releaseCount; + }; + // Releases held by the direct children of a directory. Resolves a whole listing in one query, + // for callers that would otherwise look up each child separately. + static std::vector findChildReleases(Session& session, DirectoryId parentDirectory); + // getters const std::filesystem::path& getAbsolutePath() const { return _absolutePath; } std::string_view getName() const { return _name; } diff --git a/src/libs/database/test/Directory.cpp b/src/libs/database/test/Directory.cpp index ab4c573de..890a3d7d6 100644 --- a/src/libs/database/test/Directory.cpp +++ b/src/libs/database/test/Directory.cpp @@ -19,6 +19,8 @@ #include "Common.hpp" +#include + #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 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 \ No newline at end of file From e81aa53aa87cc7107e205f93b5b4a7afcacdcf52 Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Thu, 6 Aug 2026 14:29:30 +0100 Subject: [PATCH 2/3] getMusicDirectory now resolves child releases in a single query The subdirectory loop looked up each child's release separately, so listing a directory cost one query per child, each in its own nested transaction. Resolve them all up front instead. --- src/libs/subsonic/impl/endpoints/Browsing.cpp | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/libs/subsonic/impl/endpoints/Browsing.cpp b/src/libs/subsonic/impl/endpoints/Browsing.cpp index a48acaab1..d37a96f06 100644 --- a/src/libs/subsonic/impl/endpoints/Browsing.cpp +++ b/src/libs/subsonic/impl/endpoints/Browsing.cpp @@ -338,8 +338,22 @@ 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 subDirectories{ Directory::find(context.getDbSession(), params) }; + + // resolve every child's release up front: one query for the whole listing. Skipped + // entirely for leaf directories, which are the common case, as there is nothing to resolve + std::unordered_map releaseByDirectory; + 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 +369,7 @@ namespace lms::api::subsonic directoryNode.addArrayChild("child", std::move(childNode)); } - }); + } } // list all tracks From 3e2258b820adb8baba143e238fc49d05eb77ad6f Mon Sep 17 00:00:00 2001 From: Jonathan Davies Date: Tue, 18 Aug 2026 19:03:05 +0100 Subject: [PATCH 3/3] Trim comments --- src/libs/database/include/database/objects/Directory.hpp | 4 +--- src/libs/subsonic/impl/endpoints/Browsing.cpp | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/libs/database/include/database/objects/Directory.hpp b/src/libs/database/include/database/objects/Directory.hpp index 1daa6e1a4..60db28424 100644 --- a/src/libs/database/include/database/objects/Directory.hpp +++ b/src/libs/database/include/database/objects/Directory.hpp @@ -132,11 +132,9 @@ namespace lms::db struct ChildRelease { DirectoryId directory; - ObjectPtr release; // arbitrary one if the directory holds several + ObjectPtr release; std::size_t releaseCount; }; - // Releases held by the direct children of a directory. Resolves a whole listing in one query, - // for callers that would otherwise look up each child separately. static std::vector findChildReleases(Session& session, DirectoryId parentDirectory); // getters diff --git a/src/libs/subsonic/impl/endpoints/Browsing.cpp b/src/libs/subsonic/impl/endpoints/Browsing.cpp index d37a96f06..026d7261b 100644 --- a/src/libs/subsonic/impl/endpoints/Browsing.cpp +++ b/src/libs/subsonic/impl/endpoints/Browsing.cpp @@ -340,9 +340,8 @@ namespace lms::api::subsonic const std::vector subDirectories{ Directory::find(context.getDbSession(), params) }; - // resolve every child's release up front: one query for the whole listing. Skipped - // entirely for leaf directories, which are the common case, as there is nothing to resolve std::unordered_map releaseByDirectory; + // skip leaf directories, which are the common case if (!subDirectories.empty()) { for (const Directory::ChildRelease& childRelease : Directory::findChildReleases(context.getDbSession(), directory->getId()))