From 498c6f8e2b833cdebd5ac09a73b654b26fcad49a Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 17 Mar 2024 23:37:21 +0100 Subject: [PATCH 1/5] Added option to silent useless warnings --- .clang-tidy | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index 9018ceaa..43c1aa0e 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -5,7 +5,9 @@ HeaderFilterRegex: '' AnalyzeTemporaryDtors: false FormatStyle: none User: '' -CheckOptions: +CheckOptions: + - key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic + value: '1' - key: readability-identifier-naming.IgnoreClassesWithAllMemberVariablesBeingPublic value: '1' - key: cppcoreguidelines-avoid-do-while.IgnoreMacros From 7315063a30d45621138b073e4d46835d5856cd37 Mon Sep 17 00:00:00 2001 From: emeric Date: Tue, 19 Mar 2024 22:48:50 +0100 Subject: [PATCH 2/5] Fixed bad merge --- src/libs/core/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/core/CMakeLists.txt b/src/libs/core/CMakeLists.txt index 9bfcb211..c8b5ff50 100644 --- a/src/libs/core/CMakeLists.txt +++ b/src/libs/core/CMakeLists.txt @@ -38,7 +38,7 @@ target_link_libraries(lmscore PUBLIC Wt::Wt ) -install(TARGETS lmscore DESTINATION lib ${CMAKE_INSTALL_LIBDIR}) +install(TARGETS lmscore DESTINATION ${CMAKE_INSTALL_LIBDIR}) if(BUILD_TESTING) add_subdirectory(test) From 717f71254eff4c9aa65565b2c54128833455c6d9 Mon Sep 17 00:00:00 2001 From: emeric Date: Tue, 19 Mar 2024 22:49:50 +0100 Subject: [PATCH 3/5] Fixed build when CMAKE_BUILD_TYPE is set to None --- CMakeLists.txt | 4 ++++ src/libs/database/CMakeLists.txt | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0da122b0..2f06fbc8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -82,6 +82,10 @@ if (BUILD_BENCHMARKS) message(STATUS "Building benchmarks") endif() +if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug") + add_definitions(-DNDEBUG) +endif() + add_subdirectory(src) install(DIRECTORY approot DESTINATION share/lms) diff --git a/src/libs/database/CMakeLists.txt b/src/libs/database/CMakeLists.txt index 30fc9a7c..b5e5a194 100644 --- a/src/libs/database/CMakeLists.txt +++ b/src/libs/database/CMakeLists.txt @@ -23,8 +23,8 @@ add_library(lmsdatabase SHARED impl/Utils.cpp ) -if (CMAKE_BUILD_TYPE MATCHES "Debug") -target_sources(lmsdatabase PRIVATE impl/TransactionChecker.cpp) +if (CMAKE_BUILD_TYPE STREQUAL "Debug") + target_sources(lmsdatabase PRIVATE impl/TransactionChecker.cpp) endif() target_include_directories(lmsdatabase INTERFACE From ac3643709fa1fb40d8dc46a3e819c82f3ec27e8e Mon Sep 17 00:00:00 2001 From: emeric Date: Fri, 22 Mar 2024 19:41:41 +0100 Subject: [PATCH 4/5] Added missing include --- src/libs/metadata/impl/TagLibTagReader.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libs/metadata/impl/TagLibTagReader.cpp b/src/libs/metadata/impl/TagLibTagReader.cpp index 7f26f601..340d6a11 100644 --- a/src/libs/metadata/impl/TagLibTagReader.cpp +++ b/src/libs/metadata/impl/TagLibTagReader.cpp @@ -19,6 +19,8 @@ #include "TagLibTagReader.hpp" +#include + #include #include #include From 1630b7cb6ffec4c2d0dc97bc061a45ecf1cb8772 Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 23 Mar 2024 15:55:29 +0100 Subject: [PATCH 5/5] Scan tracker: now handle releases and artists --- src/libs/database/impl/Artist.cpp | 38 +- src/libs/database/impl/Release.cpp | 28 +- src/libs/database/include/database/Artist.hpp | 1 + src/libs/database/include/database/IdType.hpp | 66 ++-- .../database/include/database/Release.hpp | 1 + src/libs/database/test/Artist.cpp | 142 ++++++-- src/libs/database/test/Release.cpp | 90 +++++ .../subsonic/impl/entrypoints/Searching.cpp | 324 ++++++++++++------ 8 files changed, 514 insertions(+), 176 deletions(-) diff --git a/src/libs/database/impl/Artist.cpp b/src/libs/database/impl/Artist.cpp index 3c0bf85e..d0a1d0b1 100644 --- a/src/libs/database/impl/Artist.cpp +++ b/src/libs/database/impl/Artist.cpp @@ -40,6 +40,7 @@ namespace lms::db { session.checkReadTransaction(); + // TODO remove distinct and use group by auto query{ session.getDboSession().query("SELECT DISTINCT " + std::string{ itemToSelect } + " FROM artist a") }; if (params.sortMethod == ArtistSortMethod::LastWritten || params.writtenAfter.isValid() @@ -64,13 +65,13 @@ namespace lms::db std::vector clauses; std::vector sortClauses; - for (std::string_view keyword : params.keywords) + for (const std::string_view keyword : params.keywords) { clauses.push_back("a.name LIKE ? ESCAPE '" ESCAPE_CHAR_STR "'"); query.bind("%" + utils::escapeLikeKeyword(keyword) + "%"); } - for (std::string_view keyword : params.keywords) + for (const std::string_view keyword : params.keywords) { sortClauses.push_back("a.sort_name LIKE ? ESCAPE '" ESCAPE_CHAR_STR "'"); query.bind("%" + utils::escapeLikeKeyword(keyword) + "%"); @@ -153,7 +154,7 @@ namespace lms::db Wt::Dbo::Query createQuery(Session& session, const Artist::FindParameters& params) { std::string_view itemToSelect; - + if constexpr (std::is_same_v) itemToSelect = "a.id"; else if constexpr (std::is_same_v>) @@ -184,6 +185,31 @@ namespace lms::db return session.getDboSession().query("SELECT COUNT(*) FROM artist"); } + void Artist::find(Session& session, ArtistId& lastRetrievedArtist, std::size_t count, const std::function& func, MediaLibraryId library) + { + session.checkReadTransaction(); + + auto query{ session.getDboSession().query>("SELECT a FROM artist a") + .orderBy("a.id") + .where("a.id > ?").bind(lastRetrievedArtist) + .limit(static_cast(count)) }; + + if (library.isValid()) + { + query.join("track t ON t.id = t_a_l.track_id"); + query.join("track_artist_link t_a_l ON t_a_l.artist_id = a.id"); + query.where("t.media_library_id = ?").bind(library); + } + + auto collection{ query.resultList() }; + + for (auto itResult{ collection.begin() }; itResult != collection.end(); ++itResult) + { + func(*itResult); + lastRetrievedArtist = (*itResult)->getId(); + } + } + std::vector Artist::find(Session& session, std::string_view name) { session.checkReadTransaction(); @@ -286,7 +312,7 @@ namespace lms::db .groupBy("a.id") .orderBy("COUNT(*) DESC, RANDOM()") }; - for (TrackArtistLinkType type : artistLinkTypes) + for (const TrackArtistLinkType type : artistLinkTypes) query.bind(type); return utils::execQuery(query, range); @@ -304,7 +330,7 @@ namespace lms::db where.And(WhereClause("a.id = ?")).bind(getId().toString()); { WhereClause clusterClause; - for (ClusterTypeId clusterTypeId : clusterTypeIds) + for (const ClusterTypeId clusterTypeId : clusterTypeIds) clusterClause.Or(WhereClause("c_type.id = ?")).bind(clusterTypeId.toString()); where.And(clusterClause); @@ -320,7 +346,7 @@ namespace lms::db Wt::Dbo::collection> queryRes = query; std::map> clustersByType; - for (Cluster::pointer cluster : queryRes) + for (const Cluster::pointer& cluster : queryRes) { if (clustersByType[cluster->getType()->getId()].size() < size) clustersByType[cluster->getType()->getId()].push_back(cluster); diff --git a/src/libs/database/impl/Release.cpp b/src/libs/database/impl/Release.cpp index f13769f1..6547ef0f 100644 --- a/src/libs/database/impl/Release.cpp +++ b/src/libs/database/impl/Release.cpp @@ -87,7 +87,7 @@ namespace lms::db .where("s_r.sync_state <> ?").bind(SyncState::PendingRemove); } - if (params.artist.isValid() + if (params.artist.isValid() || params.sortMethod == ReleaseSortMethod::ArtistNameThenName) { query.join("artist a ON a.id = t_a_l.artist_id") @@ -293,6 +293,30 @@ namespace lms::db return utils::execQuery(query, range); } + void Release::find(Session& session, ReleaseId& lastRetrievedRelease, std::size_t count, const std::function& func, MediaLibraryId library) + { + session.checkReadTransaction(); + + auto query{ session.getDboSession().query>("SELECT r FROM release r") + .orderBy("r.id") + .where("r.id > ?").bind(lastRetrievedRelease) + .limit(static_cast(count)) }; + + if (library.isValid()) + { + query.join("track t ON t.release_id = r.id"); + query.where("t.media_library_id = ?").bind(library); + } + + auto collection{ query.resultList() }; + + for (auto itResult{ collection.begin() }; itResult != collection.end(); ++itResult) + { + func(*itResult); + lastRetrievedRelease = (*itResult)->getId(); + } + } + RangeResults Release::find(Session& session, const FindParameters& params) { session.checkReadTransaction(); @@ -382,7 +406,7 @@ namespace lms::db return dates.front(); } - + std::optional Release::getYear() const { return getYear(false); diff --git a/src/libs/database/include/database/Artist.hpp b/src/libs/database/include/database/Artist.hpp index 9fb728c3..27994799 100644 --- a/src/libs/database/include/database/Artist.hpp +++ b/src/libs/database/include/database/Artist.hpp @@ -86,6 +86,7 @@ namespace lms::db static pointer find(Session& session, const core::UUID& MBID); static pointer find(Session& session, ArtistId id); static std::vector find(Session& session, std::string_view name); // exact match on name field + static void find(Session& session, ArtistId& lastRetrievedArtist, std::size_t count, const std::function& func, MediaLibraryId library = {}); static RangeResults find(Session& session, const FindParameters& parameters); static void find(Session& session, const FindParameters& parameters, std::function func); static RangeResults findIds(Session& session, const FindParameters& parameters); diff --git a/src/libs/database/include/database/IdType.hpp b/src/libs/database/include/database/IdType.hpp index 678728f7..972c9c46 100644 --- a/src/libs/database/include/database/IdType.hpp +++ b/src/libs/database/include/database/IdType.hpp @@ -26,44 +26,44 @@ namespace lms::db { - class IdType - { - public: - using ValueType = Wt::Dbo::dbo_default_traits::IdType; + class IdType + { + public: + using ValueType = Wt::Dbo::dbo_default_traits::IdType; - IdType() = default; - IdType(ValueType id) : _id {id} { assert(isValid()); } + IdType() = default; + IdType(ValueType id) : _id{ id } { assert(isValid()); } - bool isValid() const { return _id != Wt::Dbo::dbo_default_traits::invalidId(); } - std::string toString() const { assert(isValid()); return std::to_string(_id); } + bool isValid() const { return _id != Wt::Dbo::dbo_default_traits::invalidId(); } + std::string toString() const { assert(isValid()); return std::to_string(_id); } - ValueType getValue() const { return _id; } + ValueType getValue() const { return _id; } + auto operator<=>(const IdType& other) const = default; \ - auto operator<=>(const IdType& other) const = default; - - private: - Wt::Dbo::dbo_default_traits::IdType _id {Wt::Dbo::dbo_default_traits::invalidId()}; - }; + private: + Wt::Dbo::dbo_default_traits::IdType _id{ Wt::Dbo::dbo_default_traits::invalidId() }; + }; #define LMS_DECLARE_IDTYPE(name) \ - namespace lms::db { \ - class name : public IdType \ - { \ - public: \ - using IdType::IdType; \ - };\ - } \ - namespace std \ - { \ - template<> \ - class hash \ - { \ - public: \ - size_t operator()(lms::db::name id) const \ - { \ - return std::hash()(id.getValue()); \ - } \ - }; \ - } // ns std + namespace lms::db { \ + class name : public IdType \ + { \ + public: \ + using IdType::IdType; \ + auto operator<=>(const name& other) const = default; \ + };\ + } \ + namespace std \ + { \ + template<> \ + class hash \ + { \ + public: \ + size_t operator()(lms::db::name id) const \ + { \ + return std::hash()(id.getValue()); \ + } \ + }; \ + } // ns std } // namespace lms::db diff --git a/src/libs/database/include/database/Release.hpp b/src/libs/database/include/database/Release.hpp index 30f4deae..f12a7397 100644 --- a/src/libs/database/include/database/Release.hpp +++ b/src/libs/database/include/database/Release.hpp @@ -122,6 +122,7 @@ namespace lms::db static pointer find(Session& session, const core::UUID& MBID); static std::vector find(Session& session, const std::string& name, const std::filesystem::path& releaseDirectory); static pointer find(Session& session, ReleaseId id); + static void find(Session& session, ReleaseId& lastRetrievedRelease, std::size_t count, const std::function& func, MediaLibraryId library = {}); static RangeResults find(Session& session, const FindParameters& parameters); static void find(Session& session, const FindParameters& parameters, std::function func); static RangeResults findIds(Session& session, const FindParameters& parameters); diff --git a/src/libs/database/test/Artist.cpp b/src/libs/database/test/Artist.cpp index 356f7242..bff637df 100644 --- a/src/libs/database/test/Artist.cpp +++ b/src/libs/database/test/Artist.cpp @@ -77,6 +77,96 @@ namespace lms::db::tests } } + TEST_F(DatabaseFixture, Artist_findByRangedIdBased) + { + ScopedTrack track1{ session, "MyTrackFile1" }; + ScopedTrack track2{ session, "MyTrackFile2" }; + ScopedTrack track3{ session, "MyTrackFile3" }; + ScopedArtist artist1{ session, "MyArtist1" }; + ScopedArtist artist2{ session, "MyArtist2" }; + ScopedArtist artist3{ session, "MyArtist3" }; + ScopedMediaLibrary library{ session }; + ScopedMediaLibrary otherLibrary{ session }; + + { + auto transaction{ session.createWriteTransaction() }; + track2.get().modify()->setMediaLibrary(library.get()); + TrackArtistLink::create(session, track1.get(), artist1.get(), TrackArtistLinkType::Artist); + TrackArtistLink::create(session, track2.get(), artist2.get(), TrackArtistLinkType::Artist); + TrackArtistLink::create(session, track3.get(), artist3.get(), TrackArtistLinkType::Artist); + } + + { + auto transaction{ session.createReadTransaction() }; + + ArtistId lastRetrievedId; + std::vector visitedArtists; + Artist::find(session, lastRetrievedId, 10, [&](const Artist::pointer& artist) + { + visitedArtists.push_back(artist); + }); + ASSERT_EQ(visitedArtists.size(), 3); + EXPECT_EQ(visitedArtists[0]->getId(), artist1.getId()); + EXPECT_EQ(visitedArtists[1]->getId(), artist2.getId()); + EXPECT_EQ(visitedArtists[2]->getId(), artist3.getId()); + EXPECT_EQ(lastRetrievedId, artist3.getId()); + } + + { + auto transaction{ session.createReadTransaction() }; + + ArtistId lastRetrievedId{ artist1.getId() }; + std::vector visitedArtists; + Artist::find(session, lastRetrievedId, 1, [&](const Artist::pointer& artist) + { + visitedArtists.push_back(artist); + }); + ASSERT_EQ(visitedArtists.size(), 1); + EXPECT_EQ(visitedArtists[0]->getId(), artist2.getId()); + EXPECT_EQ(lastRetrievedId, artist2.getId()); + } + + { + auto transaction{ session.createReadTransaction() }; + + ArtistId lastRetrievedId{ artist1.getId() }; + std::vector visitedArtists; + Artist::find(session, lastRetrievedId, 0, [&](const Artist::pointer& artist) + { + visitedArtists.push_back(artist); + }); + ASSERT_EQ(visitedArtists.size(), 0); + EXPECT_EQ(lastRetrievedId, artist1.getId()); + } + + { + auto transaction{ session.createReadTransaction() }; + + ArtistId lastRetrievedId; + std::vector visitedArtists; + Artist::find(session, lastRetrievedId, 10, [&](const Artist::pointer& artist) + { + visitedArtists.push_back(artist); + }, otherLibrary.getId()); + ASSERT_EQ(visitedArtists.size(), 0); + EXPECT_EQ(lastRetrievedId, ArtistId{}); + } + + { + auto transaction{ session.createReadTransaction() }; + + ArtistId lastRetrievedId; + std::vector visitedArtists; + Artist::find(session, lastRetrievedId, 10, [&](const Artist::pointer& artist) + { + visitedArtists.push_back(artist); + }, library.getId()); + ASSERT_EQ(visitedArtists.size(), 1); + EXPECT_EQ(visitedArtists[0]->getId(), artist2.getId()); + EXPECT_EQ(lastRetrievedId, artist2.getId()); + } + } + TEST_F(DatabaseFixture, MultipleArtists) { { @@ -409,46 +499,46 @@ namespace lms::db::tests EXPECT_EQ(artists.front()->getId(), artist1.getId()); EXPECT_EQ(Artist::find(session, R"(MyArtistFoo)").size(), 0); } -{ - const auto artists{ Artist::find(session, R"(%MyArtist)") }; - ASSERT_TRUE(artists.size() == 1); - EXPECT_EQ(artists.front()->getId(), artist2.getId()); + { + const auto artists{ Artist::find(session, R"(%MyArtist)") }; + ASSERT_TRUE(artists.size() == 1); + EXPECT_EQ(artists.front()->getId(), artist2.getId()); EXPECT_EQ(Artist::find(session, R"(FooMyArtist)").size(), 0); + } + { + const auto artists{ Artist::find(session, R"(%_MyArtist)") }; + ASSERT_TRUE(artists.size() == 1); + ASSERT_EQ(artists.front()->getId(), artist3.getId()); + EXPECT_EQ(Artist::find(session, R"(%CMyArtist)").size(), 0); + } } + { - const auto artists{ Artist::find(session, R"(%_MyArtist)") }; - ASSERT_TRUE(artists.size() == 1); - ASSERT_EQ(artists.front()->getId(), artist3.getId()); - EXPECT_EQ(Artist::find(session, R"(%CMyArtist)").size(), 0); -} + auto transaction{ session.createReadTransaction() }; + { + const auto artists{ Artist::findIds(session, Artist::FindParameters {}.setKeywords({"MyArtist"})) }; + EXPECT_EQ(artists.results.size(), 6); } { - auto transaction{ session.createReadTransaction() }; -{ - const auto artists{ Artist::findIds(session, Artist::FindParameters {}.setKeywords({"MyArtist"})) }; - EXPECT_EQ(artists.results.size(), 6); - } - - { - const auto artists{ Artist::findIds(session, Artist::FindParameters {}.setKeywords({"MyArtist%"}).setSortMethod(ArtistSortMethod::ByName)) }; - ASSERT_EQ(artists.results.size(), 2); + const auto artists{ Artist::findIds(session, Artist::FindParameters {}.setKeywords({"MyArtist%"}).setSortMethod(ArtistSortMethod::ByName)) }; + ASSERT_EQ(artists.results.size(), 2); EXPECT_EQ(artists.results[0], artist1.getId()); - EXPECT_EQ(artists.results[1], artist4.getId()); - } + EXPECT_EQ(artists.results[1], artist4.getId()); + } - { - const auto artists{ Artist::findIds(session, Artist::FindParameters {}.setKeywords({"%MyArtist"}).setSortMethod(ArtistSortMethod::ByName)) }; + { + const auto artists{ Artist::findIds(session, Artist::FindParameters {}.setKeywords({"%MyArtist"}).setSortMethod(ArtistSortMethod::ByName)) }; ASSERT_EQ(artists.results.size(), 2); EXPECT_EQ(artists.results[0], artist2.getId()); - EXPECT_EQ(artists.results[1], artist5.getId()); + EXPECT_EQ(artists.results[1], artist5.getId()); } { const auto artists{ Artist::findIds(session, Artist::FindParameters {}.setKeywords({"_MyArtist"}).setSortMethod(ArtistSortMethod::ByName)) }; - ASSERT_EQ(artists.results.size(), 1); - EXPECT_EQ(artists.results[0], artist3.getId()); - } + ASSERT_EQ(artists.results.size(), 1); + EXPECT_EQ(artists.results[0], artist3.getId()); + } } } diff --git a/src/libs/database/test/Release.cpp b/src/libs/database/test/Release.cpp index 483544bf..22540e51 100644 --- a/src/libs/database/test/Release.cpp +++ b/src/libs/database/test/Release.cpp @@ -74,6 +74,96 @@ namespace lms::db::tests } } + TEST_F(DatabaseFixture, Release_findByRangedIdBased) + { + ScopedTrack track1{ session, "MyTrackFile1" }; + ScopedTrack track2{ session, "MyTrackFile2" }; + ScopedTrack track3{ session, "MyTrackFile3" }; + ScopedRelease release1{ session, "MyRelease1" }; + ScopedRelease release2{ session, "MyRelease2" }; + ScopedRelease release3{ session, "MyRelease3" }; + ScopedMediaLibrary library{ session }; + ScopedMediaLibrary otherLibrary{ session }; + + { + auto transaction{ session.createWriteTransaction() }; + track2.get().modify()->setMediaLibrary(library.get()); + track1.get().modify()->setRelease(release1.get()); + track2.get().modify()->setRelease(release2.get()); + track3.get().modify()->setRelease(release3.get()); + } + + { + auto transaction{ session.createReadTransaction() }; + + ReleaseId lastRetrievedId; + std::vector visitedReleases; + Release::find(session, lastRetrievedId, 10, [&](const Release::pointer& release) + { + visitedReleases.push_back(release); + }); + ASSERT_EQ(visitedReleases.size(), 3); + EXPECT_EQ(visitedReleases[0]->getId(), release1.getId()); + EXPECT_EQ(visitedReleases[1]->getId(), release2.getId()); + EXPECT_EQ(visitedReleases[2]->getId(), release3.getId()); + EXPECT_EQ(lastRetrievedId, release3.getId()); + } + + { + auto transaction{ session.createReadTransaction() }; + + ReleaseId lastRetrievedId{ release1.getId() }; + std::vector visitedReleases; + Release::find(session, lastRetrievedId, 1, [&](const Release::pointer& release) + { + visitedReleases.push_back(release); + }); + ASSERT_EQ(visitedReleases.size(), 1); + EXPECT_EQ(visitedReleases[0]->getId(), release2.getId()); + EXPECT_EQ(lastRetrievedId, release2.getId()); + } + + { + auto transaction{ session.createReadTransaction() }; + + ReleaseId lastRetrievedId{ release1.getId() }; + std::vector visitedReleases; + Release::find(session, lastRetrievedId, 0, [&](const Release::pointer& release) + { + visitedReleases.push_back(release); + }); + ASSERT_EQ(visitedReleases.size(), 0); + EXPECT_EQ(lastRetrievedId, release1.getId()); + } + + { + auto transaction{ session.createReadTransaction() }; + + ReleaseId lastRetrievedId; + std::vector visitedReleases; + Release::find(session, lastRetrievedId, 10, [&](const Release::pointer& release) + { + visitedReleases.push_back(release); + }, otherLibrary.getId()); + ASSERT_EQ(visitedReleases.size(), 0); + EXPECT_EQ(lastRetrievedId, ReleaseId{}); + } + + { + auto transaction{ session.createReadTransaction() }; + + ReleaseId lastRetrievedId; + std::vector visitedReleases; + Release::find(session, lastRetrievedId, 10, [&](const Release::pointer& release) + { + visitedReleases.push_back(release); + }, library.getId()); + ASSERT_EQ(visitedReleases.size(), 1); + EXPECT_EQ(visitedReleases[0]->getId(), release2.getId()); + EXPECT_EQ(lastRetrievedId, release2.getId()); + } + } + TEST_F(DatabaseFixture, Release_singleTrack) { ScopedRelease release{ session, "MyRelease" }; diff --git a/src/libs/subsonic/impl/entrypoints/Searching.cpp b/src/libs/subsonic/impl/entrypoints/Searching.cpp index b9a68b91..502dd28d 100644 --- a/src/libs/subsonic/impl/entrypoints/Searching.cpp +++ b/src/libs/subsonic/impl/entrypoints/Searching.cpp @@ -23,6 +23,7 @@ #include #include +#include "core/Random.hpp" #include "database/Artist.hpp" #include "database/Release.hpp" #include "database/Session.hpp" @@ -59,7 +60,6 @@ namespace lms::api::subsonic ObjectId extractLastRetrievedObjectId(const ScanInfo& info); void setObjectId(const ScanInfo& info, ObjectId lastRetrievedId); - void cleanOutdatedScanEntries(); private: using ClockType = std::chrono::steady_clock; @@ -70,6 +70,7 @@ namespace lms::api::subsonic ObjectId objectId; }; + static constexpr std::size_t maxScanCount{ 50 }; static constexpr ClockType::duration maxEntryDuration{ std::chrono::seconds{30} }; std::mutex _mutex; @@ -81,12 +82,15 @@ namespace lms::api::subsonic { ObjectId res; - const std::scoped_lock lock{ _mutex }; - auto it{ _ongoingScans.find(scanInfo) }; - if (it != _ongoingScans.end()) { - res = it->second.objectId; - _ongoingScans.erase(it); + const std::scoped_lock lock{ _mutex }; + + auto it{ _ongoingScans.find(scanInfo) }; + if (it != _ongoingScans.end()) + { + res = it->second.objectId; + _ongoingScans.erase(it); + } } return res; @@ -94,18 +98,212 @@ namespace lms::api::subsonic template void ScanTracker::setObjectId(const ScanInfo& scanInfo, ObjectId lastRetrievedId) - { - const std::scoped_lock lock{ _mutex }; - _ongoingScans[scanInfo] = { ClockType::now(), lastRetrievedId }; - } - - template - void ScanTracker::cleanOutdatedScanEntries() { const ClockType::time_point now{ ClockType::now() }; const std::scoped_lock lock{ _mutex }; + + // clean outdated scan entries; we do this to not have to flush everything each time we add/remove entries in the database std::erase_if(_ongoingScans, [&](const auto& entry) { return now > entry.second.timePoint + maxEntryDuration; }); + // prevent the cache size from going out of control + if (_ongoingScans.size() == maxScanCount) + _ongoingScans.erase(core::random::pickRandom(_ongoingScans)); + + _ongoingScans[scanInfo] = { now, lastRetrievedId }; + } + + void findRequestedArtists(RequestContext& context, bool id3, const std::vector& keywords, MediaLibraryId mediaLibrary, const User::pointer& user, Response::Node& searchResultNode) + { + static ScanTracker currentScansInProgress; + + const std::size_t artistCount{ getParameterAs(context.parameters, "artistCount").value_or(20) }; + if (artistCount == 0) + return; + + if (artistCount > defaultMaxCountSize) + throw ParameterValueTooHighGenericError{ "artistCount", defaultMaxCountSize }; + + const std::size_t artistOffset{ getParameterAs(context.parameters, "artistOffset").value_or(0) }; + + ArtistId lastRetrievedId; + auto findArtists{ [&] + { + Artist::FindParameters params; + params.setKeywords(keywords); + params.setRange(Range{ artistOffset, artistCount }); + params.setMediaLibrary(mediaLibrary); + + Artist::find(context.dbSession, params, [&](const Artist::pointer& artist) + { + searchResultNode.addArrayChild("artist", createArtistNode(context, artist, user, id3)); + lastRetrievedId = artist->getId(); + }); + } }; + + if (!keywords.empty()) + { + findArtists(); + } + else + { + ScanTracker::ScanInfo scanInfo + { + .clientAddress = context.clientInfo.ipAddress, + .clientName = context.clientInfo.name, + .userName = context.clientInfo.user, + .library = mediaLibrary, + .offset = artistOffset + }; + + if (ArtistId cachedLastRetrievedId{ currentScansInProgress.extractLastRetrievedObjectId(scanInfo) }; cachedLastRetrievedId.isValid()) + { + Artist::find(context.dbSession, cachedLastRetrievedId, artistCount, [&](const Artist::pointer& artist) + { + searchResultNode.addArrayChild("artist", createArtistNode(context, artist, user, id3)); + }, mediaLibrary); + lastRetrievedId = cachedLastRetrievedId; + } + else + { + findArtists(); + } + + if (lastRetrievedId.isValid()) + { + scanInfo.offset = artistOffset + artistCount; + currentScansInProgress.setObjectId(scanInfo, lastRetrievedId); + } + } + } + + void findRequestedAlbums(RequestContext& context, bool id3, const std::vector& keywords, MediaLibraryId mediaLibrary, const User::pointer& user, Response::Node& searchResultNode) + { + static ScanTracker currentScansInProgress; + + const std::size_t albumCount{ getParameterAs(context.parameters, "albumCount").value_or(20) }; + if (albumCount == 0) + return; + + if (albumCount > defaultMaxCountSize) + throw ParameterValueTooHighGenericError{ "albumCount", defaultMaxCountSize }; + + const std::size_t albumOffset{ getParameterAs(context.parameters, "albumOffset").value_or(0) }; + + ReleaseId lastRetrievedId; + + auto findReleases{ [&] + { + Release::FindParameters params; + params.setKeywords(keywords); + params.setRange(Range{ albumOffset, albumCount }); + params.setMediaLibrary(mediaLibrary); + + Release::find(context.dbSession, params, [&](const Release::pointer& release) + { + searchResultNode.addArrayChild("album", createAlbumNode(context, release, user, id3)); + lastRetrievedId = release->getId(); + }); + } }; + + if (!keywords.empty()) + { + findReleases(); + } + else + { + ScanTracker::ScanInfo scanInfo + { + .clientAddress = context.clientInfo.ipAddress, + .clientName = context.clientInfo.name, + .userName = context.clientInfo.user, + .library = mediaLibrary, + .offset = albumOffset + }; + + if (ReleaseId cachedLastRetrievedId{ currentScansInProgress.extractLastRetrievedObjectId(scanInfo) }; cachedLastRetrievedId.isValid()) + { + Release::find(context.dbSession, cachedLastRetrievedId, albumCount, [&](const Release::pointer& release) + { + searchResultNode.addArrayChild("album", createAlbumNode(context, release, user, id3)); + }, mediaLibrary); + lastRetrievedId = cachedLastRetrievedId; + } + else + { + findReleases(); + } + + if (lastRetrievedId.isValid()) + { + scanInfo.offset = albumOffset + albumCount; + currentScansInProgress.setObjectId(scanInfo, lastRetrievedId); + } + } + } + + void findRequestedTracks(RequestContext& context, const std::vector& keywords, MediaLibraryId mediaLibrary, const User::pointer& user, Response::Node& searchResultNode) + { + static ScanTracker currentScansInProgress; + + const std::size_t songCount{ getParameterAs(context.parameters, "songCount").value_or(20) }; + if (songCount == 0) + return; + + if (songCount > defaultMaxCountSize) + throw ParameterValueTooHighGenericError{ "songCount", defaultMaxCountSize }; + + const std::size_t songOffset{ getParameterAs(context.parameters, "songOffset").value_or(0) }; + + TrackId lastRetrievedId; + + auto findTracks{ [&] + { + Track::FindParameters params; + params.setKeywords(keywords); + params.setRange(Range{ songOffset, songCount }); + params.setMediaLibrary(mediaLibrary); + + Track::find(context.dbSession, params, [&](const Track::pointer& track) + { + searchResultNode.addArrayChild("song", createSongNode(context, track, user)); + lastRetrievedId = track->getId(); + }); + } }; + + if (!keywords.empty()) + { + findTracks(); + } + else + { + ScanTracker::ScanInfo scanInfo + { + .clientAddress = context.clientInfo.ipAddress, + .clientName = context.clientInfo.name, + .userName = context.clientInfo.user, + .library = mediaLibrary, + .offset = songOffset + }; + + if (TrackId cachedLastRetrievedId{ currentScansInProgress.extractLastRetrievedObjectId(scanInfo) }; cachedLastRetrievedId.isValid()) + { + Track::find(context.dbSession, cachedLastRetrievedId, songCount, [&](const Track::pointer& track) + { + searchResultNode.addArrayChild("song", createSongNode(context, track, user)); + }, mediaLibrary); + lastRetrievedId = cachedLastRetrievedId; + } + else + { + findTracks(); + } + + if (lastRetrievedId.isValid()) + { + scanInfo.offset = songOffset + songCount; + currentScansInProgress.setObjectId(scanInfo, lastRetrievedId); + } + } } } @@ -118,21 +316,8 @@ namespace lms::api::subsonic std::string_view query{ queryString }; // Optional params - const std::size_t artistCount{ getParameterAs(context.parameters, "artistCount").value_or(20) }; - const std::size_t artistOffset{ getParameterAs(context.parameters, "artistOffset").value_or(0) }; - const std::size_t albumCount{ getParameterAs(context.parameters, "albumCount").value_or(20) }; - const std::size_t albumOffset{ getParameterAs(context.parameters, "albumOffset").value_or(0) }; - const std::size_t songCount{ getParameterAs(context.parameters, "songCount").value_or(20) }; - const std::size_t songOffset{ getParameterAs(context.parameters, "songOffset").value_or(0) }; const MediaLibraryId mediaLibrary{ getParameterAs(context.parameters, "musicFolderId").value_or(MediaLibraryId{}) }; - if (artistCount > defaultMaxCountSize) - throw ParameterValueTooHighGenericError{ "artistCount", defaultMaxCountSize }; - if (albumCount > defaultMaxCountSize) - throw ParameterValueTooHighGenericError{ "albumCount", defaultMaxCountSize }; - if (songCount > defaultMaxCountSize) - throw ParameterValueTooHighGenericError{ "songCount", defaultMaxCountSize }; - // Symfonium adds extra "" if (context.clientInfo.name == "Symfonium") query = core::stringUtils::stringTrim(query, "\""); @@ -142,7 +327,7 @@ namespace lms::api::subsonic keywords = core::stringUtils::splitString(query, ' '); Response response{ Response::createOkResponse(context.serverProtocolVersion) }; - Response::Node& searchResult2Node{ response.createNode(id3 ? "searchResult3" : "searchResult2") }; + Response::Node& searchResultNode{ response.createNode(id3 ? "searchResult3" : "searchResult2") }; auto transaction{ context.dbSession.createReadTransaction() }; @@ -150,88 +335,9 @@ namespace lms::api::subsonic if (!user) throw UserNotAuthorizedError{}; - if (artistCount > 0) - { - Artist::FindParameters params; - params.setKeywords(keywords); - params.setRange(Range{ artistOffset, artistCount }); - params.setMediaLibrary(mediaLibrary); - - Artist::find(context.dbSession, params, [&](const Artist::pointer& artist) - { - searchResult2Node.addArrayChild("artist", createArtistNode(context, artist, user, id3)); - }); - } - - if (albumCount > 0) - { - Release::FindParameters params; - params.setKeywords(keywords); - params.setRange(Range{ albumOffset, albumCount }); - params.setMediaLibrary(mediaLibrary); - - Release::find(context.dbSession, params, [&](const Release::pointer& release) - { - searchResult2Node.addArrayChild("album", createAlbumNode(context, release, user, id3)); - }); - } - - if (songCount > 0) - { - static ScanTracker currentScansInProgress; - currentScansInProgress.cleanOutdatedScanEntries(); - - TrackId lastRetrievedId; - - auto findTracks{ [&] - { - Track::FindParameters params; - params.setKeywords(keywords); - params.setRange(Range{ songOffset, songCount }); - params.setMediaLibrary(mediaLibrary); - - Track::find(context.dbSession, params, [&](const Track::pointer& track) - { - searchResult2Node.addArrayChild("song", createSongNode(context, track, user)); - lastRetrievedId = track->getId(); - }); - } }; - - if (!keywords.empty()) - { - findTracks(); - } - else - { - ScanTracker::ScanInfo scanInfo - { - .clientAddress = context.clientInfo.ipAddress, - .clientName = context.clientInfo.name, - .userName = context.clientInfo.user, - .library = mediaLibrary, - .offset = songOffset - }; - - if (TrackId cachedLastRetrievedId{ currentScansInProgress.extractLastRetrievedObjectId(scanInfo) }; cachedLastRetrievedId.isValid()) - { - Track::find(context.dbSession, cachedLastRetrievedId, songCount, [&](const Track::pointer& track) - { - searchResult2Node.addArrayChild("song", createSongNode(context, track, user)); - }, mediaLibrary); - lastRetrievedId = cachedLastRetrievedId; - } - else - { - findTracks(); - } - - if (lastRetrievedId.isValid()) - { - scanInfo.offset = songOffset + songCount; - currentScansInProgress.setObjectId(scanInfo, lastRetrievedId); - } - } - } + findRequestedArtists(context, id3, keywords, mediaLibrary, user, searchResultNode); + findRequestedAlbums(context, id3, keywords, mediaLibrary, user, searchResultNode); + findRequestedTracks(context, keywords, mediaLibrary, user, searchResultNode); return response; }