diff --git a/src/libs/database/impl/objects/Directory.cpp b/src/libs/database/impl/objects/Directory.cpp index 822ce273..41353f4d 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 dd30df7c..60db2842 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,14 @@ 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; + std::size_t releaseCount; + }; + 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 ab4c573d..890a3d7d 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 diff --git a/src/libs/subsonic/impl/endpoints/Browsing.cpp b/src/libs/subsonic/impl/endpoints/Browsing.cpp index a48acaab..026d7261 100644 --- a/src/libs/subsonic/impl/endpoints/Browsing.cpp +++ b/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 subDirectories{ Directory::find(context.getDbSession(), params) }; + + 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())) + 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