From bc03408b995e6a7316b4d6fa939be2a37d804529 Mon Sep 17 00:00:00 2001 From: David Roman Date: Mon, 11 Mar 2024 20:49:18 +0100 Subject: [PATCH 1/6] Fix assertion Closes: #431 --- src/libs/database/impl/Utils.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/database/impl/Utils.hpp b/src/libs/database/impl/Utils.hpp index 29be679c..83c7e3df 100644 --- a/src/libs/database/impl/Utils.hpp +++ b/src/libs/database/impl/Utils.hpp @@ -62,11 +62,11 @@ namespace Database::Utils // TODO may optim by not actually requesting the last one res.moreResults = true; res.results.pop_back(); + res.range.offset = range->offset; } else res.moreResults = false; - res.range.offset = range->offset; res.range.size = res.results.size(); return res; From 181a5fadd365e99b92d653117f15bfc81e43ddba Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 10 Mar 2024 15:27:45 +0100 Subject: [PATCH 2/6] Ignore vscode stuff for now --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 761aaa87..b8617e34 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ CMakeCache.txt CMakeFiles/ build/ +.vscode/ \ No newline at end of file From d876a2df0c02da59b07fd1854853c70d82d7e5fa Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 10 Mar 2024 15:28:16 +0100 Subject: [PATCH 3/6] Switched to LiteralString for keys in subsonic responses --- src/libs/subsonic/impl/SubsonicResponse.cpp | 22 ++++++++++----------- src/libs/subsonic/impl/SubsonicResponse.hpp | 14 ++----------- 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/src/libs/subsonic/impl/SubsonicResponse.cpp b/src/libs/subsonic/impl/SubsonicResponse.cpp index c5d971e3..70153e2c 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.cpp +++ b/src/libs/subsonic/impl/SubsonicResponse.cpp @@ -193,13 +193,13 @@ namespace API::Subsonic for (const auto& [key, value] : node._attributes) { if (std::holds_alternative(value)) - res.put("." + std::string{ key.get() }, std::get(value)); + res.put("." + std::string{ key.str() }, std::get(value)); else if (std::holds_alternative(value)) - res.put("." + std::string{ key.get() }, std::get(value)); + res.put("." + std::string{ key.str() }, std::get(value)); else if (std::holds_alternative(value)) - res.put("." + std::string{ key.get() }, std::get(value)); + res.put("." + std::string{ key.str() }, std::get(value)); else if (std::holds_alternative(value)) - res.put("." + std::string{ key.get() }, std::get(value)); + res.put("." + std::string{ key.str() }, std::get(value)); } auto valueToPropertyTree = [](const Node::ValueType& value) @@ -221,19 +221,19 @@ namespace API::Subsonic { for (const auto& [key, childNode] : node._children) { - res.add_child(std::string{ key.get() }, nodeToPropertyTree(childNode)); + res.add_child(std::string{ key.str() }, nodeToPropertyTree(childNode)); } for (const auto& [key, childArrayNodes] : node._childrenArrays) { for (const Node& childNode : childArrayNodes) - res.add_child(std::string{ key.get() }, nodeToPropertyTree(childNode)); + res.add_child(std::string{ key.str() }, nodeToPropertyTree(childNode)); } for (const auto& [key, childArrayValues] : node._childrenValues) { for (const Response::Node::ValueType& value : childArrayValues) - res.add_child(std::string{ key.get() }, valueToPropertyTree(value)); + res.add_child(std::string{ key.str() }, valueToPropertyTree(value)); } } @@ -255,7 +255,7 @@ namespace API::Subsonic if (!first) os << ','; - serializeEscapedString(os, key.get()); + serializeEscapedString(os, key.str()); os << ':'; serializeValue(os, value); @@ -279,7 +279,7 @@ namespace API::Subsonic if (!first) os << ','; - serializeEscapedString(os, key.get()); + serializeEscapedString(os, key.str()); os << ':'; serializeNode(os, childNode); @@ -291,7 +291,7 @@ namespace API::Subsonic if (!first) os << ','; - serializeEscapedString(os, key.get()); + serializeEscapedString(os, key.str()); os << ":["; bool firstChild{ true }; @@ -313,7 +313,7 @@ namespace API::Subsonic if (!first) os << ','; - serializeEscapedString(os, key.get()); + serializeEscapedString(os, key.str()); os << ":["; bool firstChild{ true }; diff --git a/src/libs/subsonic/impl/SubsonicResponse.hpp b/src/libs/subsonic/impl/SubsonicResponse.hpp index 60690dae..5ff50686 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.hpp +++ b/src/libs/subsonic/impl/SubsonicResponse.hpp @@ -25,6 +25,7 @@ #include #include +#include "utils/LiteralString.hpp" #include "RequestContext.hpp" namespace API::Subsonic @@ -205,18 +206,7 @@ namespace API::Subsonic class Node { public: - class Key - { - public: - template - constexpr Key(const char (&str)[N]) : _str{ str } {} - constexpr std::string_view get() const { return _str; } - - bool constexpr operator<(const Key& other) const { return _str < other._str; } - - private: - const std::string_view _str; - }; + using Key = LiteralString; void setAttribute(Key key, std::string_view value); From 58848dfae3f88b03578a5c1e75a9d5b73460b10f Mon Sep 17 00:00:00 2001 From: David Roman Date: Mon, 11 Mar 2024 19:25:30 +0100 Subject: [PATCH 4/6] make tests optional --- CMakeLists.txt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 369ef896..0da122b0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,11 +10,16 @@ if (UNIX) set(CMAKE_SHARED_LINKER_FLAGS "-Wl,--no-undefined") endif () -include(CTest) +option(ENABLE_TESTS "Enable tests" ON) + +if(ENABLE_TESTS) + include(CTest) + find_package(GTest REQUIRED) +endif() + find_package(PkgConfig REQUIRED) find_package(Threads REQUIRED) find_package(Filesystem REQUIRED) -find_package(GTest REQUIRED) find_package(Boost REQUIRED COMPONENTS system program_options) find_package(Wt REQUIRED COMPONENTS Wt Dbo DboSqlite3 HTTP) pkg_check_modules(Taglib REQUIRED IMPORTED_TARGET taglib) From 4ea6d0dd092faae31909a44effb1a124ff5b2018 Mon Sep 17 00:00:00 2001 From: David Roman Date: Mon, 11 Mar 2024 19:27:08 +0100 Subject: [PATCH 5/6] use cmake lib dir instead of hardcoded name --- src/libs/av/CMakeLists.txt | 2 +- src/libs/database/CMakeLists.txt | 2 +- src/libs/image/CMakeLists.txt | 2 +- src/libs/metadata/CMakeLists.txt | 2 +- src/libs/services/auth/CMakeLists.txt | 2 +- src/libs/services/cover/CMakeLists.txt | 2 +- src/libs/services/feedback/CMakeLists.txt | 2 +- src/libs/services/recommendation/CMakeLists.txt | 2 +- src/libs/services/scanner/CMakeLists.txt | 2 +- src/libs/services/scrobbling/CMakeLists.txt | 2 +- src/libs/som/CMakeLists.txt | 2 +- src/libs/subsonic/CMakeLists.txt | 2 +- src/libs/utils/CMakeLists.txt | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/libs/av/CMakeLists.txt b/src/libs/av/CMakeLists.txt index fee1201e..e616260d 100644 --- a/src/libs/av/CMakeLists.txt +++ b/src/libs/av/CMakeLists.txt @@ -26,5 +26,5 @@ target_link_libraries(lmsav PRIVATE PkgConfig::LIBAV ) -install(TARGETS lmsav DESTINATION lib) +install(TARGETS lmsav DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/src/libs/database/CMakeLists.txt b/src/libs/database/CMakeLists.txt index 7188f977..0b1eb84e 100644 --- a/src/libs/database/CMakeLists.txt +++ b/src/libs/database/CMakeLists.txt @@ -42,7 +42,7 @@ target_link_libraries(lmsdatabase PUBLIC Wt::Dbo ) -install(TARGETS lmsdatabase DESTINATION lib) +install(TARGETS lmsdatabase DESTINATION ${CMAKE_INSTALL_LIBDIR}) if(BUILD_TESTING) add_subdirectory(test) diff --git a/src/libs/image/CMakeLists.txt b/src/libs/image/CMakeLists.txt index cb855563..eb48565e 100644 --- a/src/libs/image/CMakeLists.txt +++ b/src/libs/image/CMakeLists.txt @@ -34,5 +34,5 @@ else () message(FATAL_ERROR "Invalid IMAGE_LIBRARY provided") endif() -install(TARGETS lmsimage DESTINATION lib) +install(TARGETS lmsimage DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/src/libs/metadata/CMakeLists.txt b/src/libs/metadata/CMakeLists.txt index 8a11a7cf..8b3cb54c 100644 --- a/src/libs/metadata/CMakeLists.txt +++ b/src/libs/metadata/CMakeLists.txt @@ -28,5 +28,5 @@ target_link_libraries(lmsmetadata PUBLIC std::filesystem ) -install(TARGETS lmsmetadata DESTINATION lib) +install(TARGETS lmsmetadata DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/src/libs/services/auth/CMakeLists.txt b/src/libs/services/auth/CMakeLists.txt index 0a2eb6a5..edb9c4c2 100644 --- a/src/libs/services/auth/CMakeLists.txt +++ b/src/libs/services/auth/CMakeLists.txt @@ -35,5 +35,5 @@ if (USE_PAM) target_link_libraries(lmsauth PRIVATE ${PAM_LIBRARIES}) endif (USE_PAM) -install(TARGETS lmsauth DESTINATION lib) +install(TARGETS lmsauth DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/src/libs/services/cover/CMakeLists.txt b/src/libs/services/cover/CMakeLists.txt index 41169c5d..6dc17262 100644 --- a/src/libs/services/cover/CMakeLists.txt +++ b/src/libs/services/cover/CMakeLists.txt @@ -24,5 +24,5 @@ target_link_libraries(lmsservice-cover PUBLIC std::filesystem ) -install(TARGETS lmsservice-cover DESTINATION lib) +install(TARGETS lmsservice-cover DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/src/libs/services/feedback/CMakeLists.txt b/src/libs/services/feedback/CMakeLists.txt index 02e63624..b7634e77 100644 --- a/src/libs/services/feedback/CMakeLists.txt +++ b/src/libs/services/feedback/CMakeLists.txt @@ -26,4 +26,4 @@ target_link_libraries(lmsfeedback PUBLIC lmsdatabase ) -install(TARGETS lmsfeedback DESTINATION lib) +install(TARGETS lmsfeedback DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/src/libs/services/recommendation/CMakeLists.txt b/src/libs/services/recommendation/CMakeLists.txt index 8244679b..603a66b0 100644 --- a/src/libs/services/recommendation/CMakeLists.txt +++ b/src/libs/services/recommendation/CMakeLists.txt @@ -26,5 +26,5 @@ target_link_libraries(lmsrecommendation PRIVATE std::filesystem ) -install(TARGETS lmsrecommendation DESTINATION lib) +install(TARGETS lmsrecommendation DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/src/libs/services/scanner/CMakeLists.txt b/src/libs/services/scanner/CMakeLists.txt index 8a0508e9..78d49e73 100644 --- a/src/libs/services/scanner/CMakeLists.txt +++ b/src/libs/services/scanner/CMakeLists.txt @@ -29,5 +29,5 @@ target_link_libraries(lmsscanner PUBLIC Wt::Wt ) -install(TARGETS lmsscanner DESTINATION lib) +install(TARGETS lmsscanner DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/src/libs/services/scrobbling/CMakeLists.txt b/src/libs/services/scrobbling/CMakeLists.txt index 2f78000a..db065828 100644 --- a/src/libs/services/scrobbling/CMakeLists.txt +++ b/src/libs/services/scrobbling/CMakeLists.txt @@ -26,7 +26,7 @@ target_link_libraries(lmsscrobbling PUBLIC lmsdatabase ) -install(TARGETS lmsscrobbling DESTINATION lib) +install(TARGETS lmsscrobbling DESTINATION ${CMAKE_INSTALL_LIBDIR}) if(BUILD_TESTING) add_subdirectory(test) diff --git a/src/libs/som/CMakeLists.txt b/src/libs/som/CMakeLists.txt index 87a2d53f..eeb971d6 100644 --- a/src/libs/som/CMakeLists.txt +++ b/src/libs/som/CMakeLists.txt @@ -17,7 +17,7 @@ target_link_libraries(lmssom PUBLIC set_property(TARGET lmssom PROPERTY POSITION_INDEPENDENT_CODE ON) -install(TARGETS lmssom DESTINATION lib) +install(TARGETS lmssom DESTINATION ${CMAKE_INSTALL_LIBDIR}) if(BUILD_TESTING) add_subdirectory(test) diff --git a/src/libs/subsonic/CMakeLists.txt b/src/libs/subsonic/CMakeLists.txt index 07394e94..41afbbb8 100644 --- a/src/libs/subsonic/CMakeLists.txt +++ b/src/libs/subsonic/CMakeLists.txt @@ -56,5 +56,5 @@ target_link_libraries(lmssubsonic PUBLIC Wt::Wt ) -install(TARGETS lmssubsonic DESTINATION lib) +install(TARGETS lmssubsonic DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/src/libs/utils/CMakeLists.txt b/src/libs/utils/CMakeLists.txt index 7d00aaa1..35368f15 100644 --- a/src/libs/utils/CMakeLists.txt +++ b/src/libs/utils/CMakeLists.txt @@ -38,7 +38,7 @@ target_link_libraries(lmsutils PUBLIC Wt::Wt ) -install(TARGETS lmsutils DESTINATION lib) +install(TARGETS lmsutils DESTINATION ${CMAKE_INSTALL_LIBDIR}) if(BUILD_TESTING) add_subdirectory(test) From 9da781cff6666e32cf24be5a2a2042b113acbf84 Mon Sep 17 00:00:00 2001 From: emeric Date: Tue, 12 Mar 2024 13:40:32 +0100 Subject: [PATCH 6/6] Fixed badly reported offset when hitting the end of the collection --- src/libs/database/impl/Utils.hpp | 6 +-- src/libs/database/test/Artist.cpp | 64 ++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/libs/database/impl/Utils.hpp b/src/libs/database/impl/Utils.hpp index 83c7e3df..e49fd6b6 100644 --- a/src/libs/database/impl/Utils.hpp +++ b/src/libs/database/impl/Utils.hpp @@ -53,7 +53,10 @@ namespace Database::Utils RangeResults res; if (range) + { + res.range.offset = range->offset; applyRange(query, Range{ range->offset, range->size + 1 }); + } auto collection{ query.resultList() }; res.results.assign(collection.begin(), collection.end()); @@ -62,10 +65,7 @@ namespace Database::Utils // TODO may optim by not actually requesting the last one res.moreResults = true; res.results.pop_back(); - res.range.offset = range->offset; } - else - res.moreResults = false; res.range.size = res.results.size(); diff --git a/src/libs/database/test/Artist.cpp b/src/libs/database/test/Artist.cpp index 3030706c..878d0f24 100644 --- a/src/libs/database/test/Artist.cpp +++ b/src/libs/database/test/Artist.cpp @@ -56,7 +56,7 @@ TEST_F(DatabaseFixture, Artist) EXPECT_EQ(artists.results.front(), artist.getId()); } - + { auto transaction{ session.createReadTransaction() }; @@ -78,6 +78,66 @@ TEST_F(DatabaseFixture, Artist) } } +TEST_F(DatabaseFixture, MultipleArtists) +{ + { + auto transaction{ session.createReadTransaction() }; + + auto artists{ Artist::findIds(session, Artist::FindParameters {}) }; + ASSERT_EQ(artists.results.size(), 0); + ASSERT_FALSE(artists.moreResults); + ASSERT_EQ(artists.range.offset, 0); + ASSERT_EQ(artists.range.size, 0); + } + + ScopedArtist artist1{ session, "MyArtist1" }; + ScopedArtist artist2{ session, "MyArtist2" }; + ScopedArtist artist3{ session, "MyArtist3" }; + + { + auto transaction{ session.createReadTransaction() }; + + auto artists{ Artist::findIds(session, Artist::FindParameters{}) }; + ASSERT_EQ(artists.results.size(), 3); + ASSERT_FALSE(artists.moreResults); + ASSERT_EQ(artists.range.offset, 0); + ASSERT_EQ(artists.range.size, 3); + } + + { + auto transaction{ session.createReadTransaction() }; + + auto artists{ Artist::findIds(session, Artist::FindParameters{}.setRange(Range{0,1})) }; + ASSERT_EQ(artists.results.size(), 1); + ASSERT_TRUE(artists.moreResults); + ASSERT_EQ(artists.range.offset, 0); + ASSERT_EQ(artists.range.size, 1); + EXPECT_EQ(artists.results[0], artist1.getId()); + } + + { + auto transaction{ session.createReadTransaction() }; + + auto artists{ Artist::findIds(session, Artist::FindParameters{}.setRange(Range{1,1})) }; + ASSERT_EQ(artists.results.size(), 1); + ASSERT_TRUE(artists.moreResults); + ASSERT_EQ(artists.range.offset, 1); + ASSERT_EQ(artists.range.size, 1); + EXPECT_EQ(artists.results[0], artist2.getId()); + } + + { + auto transaction{ session.createReadTransaction() }; + + auto artists{ Artist::findIds(session, Artist::FindParameters{}.setRange(Range{2,1})) }; + ASSERT_EQ(artists.results.size(), 1); + ASSERT_FALSE(artists.moreResults); + ASSERT_EQ(artists.range.offset, 2); + ASSERT_EQ(artists.range.size, 1); + EXPECT_EQ(artists.results[0], artist3.getId()); + } +} + TEST_F(DatabaseFixture, Artist_singleTrack) { ScopedTrack track{ session, "MyTrack" }; @@ -174,7 +234,7 @@ TEST_F(DatabaseFixture, Artist_singleTrack_mediaLibrary) ASSERT_EQ(artists.results.size(), 1); EXPECT_EQ(artists.results.front(), artist.getId()); } - { + { auto transaction{ session.createReadTransaction() }; auto artists{ Artist::findIds(session, Artist::FindParameters{}.setMediaLibrary(otherLibrary->getId())) }; EXPECT_EQ(artists.results.size(), 0);