From 61e0931a827eabd0834e69ce08ec91ab58065964 Mon Sep 17 00:00:00 2001 From: emeric Date: Sat, 21 Oct 2023 14:45:39 +0200 Subject: [PATCH] Fixed output bug for values, added some optims --- src/libs/services/database/impl/Cluster.cpp | 6 ++-- src/libs/services/database/impl/Migration.cpp | 6 ++-- .../include/services/database/Cluster.hpp | 11 ++++--- src/libs/services/database/test/Cluster.cpp | 24 ++++++++------- src/libs/subsonic/impl/SubsonicResponse.cpp | 8 ++--- src/libs/subsonic/impl/SubsonicResponse.hpp | 6 ++-- .../subsonic/impl/entrypoints/Playlists.cpp | 2 +- src/libs/subsonic/impl/responses/Album.cpp | 30 +++++++------------ .../subsonic/impl/responses/ItemGenre.cpp | 4 +-- .../subsonic/impl/responses/ItemGenre.hpp | 4 +-- src/libs/subsonic/impl/responses/Song.cpp | 29 ++++++------------ 11 files changed, 58 insertions(+), 72 deletions(-) diff --git a/src/libs/services/database/impl/Cluster.cpp b/src/libs/services/database/impl/Cluster.cpp index 6acc874a..1dcbf1b7 100644 --- a/src/libs/services/database/impl/Cluster.cpp +++ b/src/libs/services/database/impl/Cluster.cpp @@ -32,11 +32,11 @@ namespace Database { namespace { - Wt::Dbo::Query createQuery(Session& session, const Cluster::FindParameters& params) + Wt::Dbo::Query createQuery(Session& session, const Cluster::FindParameters& params) { session.checkSharedLocked(); - auto query{ session.getDboSession().query("SELECT DISTINCT c.id FROM cluster c") }; + auto query{ session.getDboSession().query("SELECT DISTINCT c.id,c.name FROM cluster c") }; if (params.track.isValid() || params.release.isValid()) { @@ -74,7 +74,7 @@ namespace Database return session.getDboSession().query("SELECT COUNT(*) FROM cluster"); } - RangeResults Cluster::find(Session& session, const FindParameters& params) + RangeResults Cluster::find(Session& session, const FindParameters& params) { session.checkSharedLocked(); auto query{ createQuery(session, params) }; diff --git a/src/libs/services/database/impl/Migration.cpp b/src/libs/services/database/impl/Migration.cpp index 728caf4e..5785cce6 100644 --- a/src/libs/services/database/impl/Migration.cpp +++ b/src/libs/services/database/impl/Migration.cpp @@ -672,7 +672,7 @@ CREATE TABLE IF NOT EXISTS "track_backup" ( if (version == LMS_DATABASE_VERSION) { - LMS_LOG(DB, DEBUG) << "Lms database version " << LMS_DATABASE_VERSION << ": up to date!"; + LMS_LOG(DB, INFO) << "Lms database version " << LMS_DATABASE_VERSION << ": up to date!"; return; } else if (version > LMS_DATABASE_VERSION) @@ -683,13 +683,15 @@ CREATE TABLE IF NOT EXISTS "track_backup" ( if (version < migrationFunctions.begin()->first) throw LmsException{ outdatedMsg }; - LMS_LOG(DB, INFO) << "Migrating database from version " << version << "..."; + LMS_LOG(DB, INFO) << "Migrating database from version " << version << " to " << version + 1 << "..."; auto itMigrationFunc{ migrationFunctions.find(version) }; assert(itMigrationFunc != std::cend(migrationFunctions)); itMigrationFunc->second(session); VersionInfo::get(session).modify()->setVersion(++version); + + LMS_LOG(DB, INFO) << "Migration complete to version " << version; } } } diff --git a/src/libs/services/database/include/services/database/Cluster.hpp b/src/libs/services/database/include/services/database/Cluster.hpp index 4f416e7f..521727e6 100644 --- a/src/libs/services/database/include/services/database/Cluster.hpp +++ b/src/libs/services/database/include/services/database/Cluster.hpp @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -58,10 +59,12 @@ namespace Database { Cluster() = default; // Find utility - static std::size_t getCount(Session& session); - static RangeResults find(Session& session, const FindParameters& range); - static pointer find(Session& session, ClusterId id); - static RangeResults findOrphans(Session& session, Range range); + // As clusters only have a name, this is an optim to directly get the cluster names + using ClusterFindResult = std::tuple; + static std::size_t getCount(Session& session); + static RangeResults find(Session& session, const FindParameters& range); + static pointer find(Session& session, ClusterId id); + static RangeResults findOrphans(Session& session, Range range); // Accessors const std::string& getName() const { return _name; } diff --git a/src/libs/services/database/test/Cluster.cpp b/src/libs/services/database/test/Cluster.cpp index d9caa140..0b045fd5 100644 --- a/src/libs/services/database/test/Cluster.cpp +++ b/src/libs/services/database/test/Cluster.cpp @@ -48,13 +48,17 @@ TEST_F(DatabaseFixture, Cluster) EXPECT_EQ(Cluster::getCount(session), 1); EXPECT_EQ(cluster->getType()->getId(), clusterType.getId()); - auto clusters{ Cluster::find(session, Cluster::FindParameters {}) }; - ASSERT_EQ(clusters.results.size(), 1); - EXPECT_EQ(clusters.results.front(), cluster.getId()); + { + const auto clusters{ Cluster::find(session, Cluster::FindParameters {}) }; + ASSERT_EQ(clusters.results.size(), 1); + EXPECT_EQ(std::get(clusters.results.front()), cluster.getId()); + } - clusters = Cluster::findOrphans(session, Range{}); - ASSERT_EQ(clusters.results.size(), 1); - EXPECT_EQ(clusters.results.front(), cluster.getId()); + { + const auto clusters{ Cluster::findOrphans(session, Range{}) }; + ASSERT_EQ(clusters.results.size(), 1); + EXPECT_EQ(clusters.results.front(), cluster.getId()); + } auto clusterTypes{ ClusterType::find(session, Range {}) }; ASSERT_EQ(clusterTypes.results.size(), 1); @@ -114,7 +118,7 @@ TEST_F(DatabaseFixture, Cluster_singleTrack) auto transaction{ session.createSharedTransaction() }; auto clusters{ Cluster::find(session, Cluster::FindParameters {}.setTrack(track.getId())) }; ASSERT_EQ(clusters.results.size(), 1); - EXPECT_EQ(clusters.results.front(), cluster1.getId()); + EXPECT_EQ(std::get(clusters.results.front()), cluster1.getId()); } { @@ -317,9 +321,9 @@ TEST_F(DatabaseFixture, Cluster_singleTrackSingleReleaseSingleCluster) { auto transaction{ session.createSharedTransaction() }; - auto clusters{ Cluster::find(session, Cluster::FindParameters{}.setRelease(release.getId())) }; + const auto clusters{ Cluster::find(session, Cluster::FindParameters{}.setRelease(release.getId())) }; ASSERT_EQ(clusters.results.size(), 1); - EXPECT_EQ(clusters.results.front(), cluster.getId()); + EXPECT_EQ(std::get(clusters.results.front()), cluster.getId()); } { @@ -1106,5 +1110,3 @@ TEST_F(DatabaseFixture, MultipleTracksMultipleReleasesMultiClusters) } } } - - diff --git a/src/libs/subsonic/impl/SubsonicResponse.cpp b/src/libs/subsonic/impl/SubsonicResponse.cpp index ec6e6505..7450792e 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.cpp +++ b/src/libs/subsonic/impl/SubsonicResponse.cpp @@ -58,7 +58,7 @@ namespace API::Subsonic _attributes[key] = std::string{ value }; } - void Response::Node::addChild(Key key, Node node) + void Response::Node::addChild(Key key, Node&& node) { assert(!_value); assert(_children.find(key) == std::cend(_children)); @@ -72,7 +72,7 @@ namespace API::Subsonic _childrenArrays.emplace(key, std::vector{}); } - void Response::Node::addArrayChild(Key key, Node node) + void Response::Node::addArrayChild(Key key, Node&& node) { assert(!_value); assert(_children.find(key) == std::cend(_children)); @@ -155,7 +155,7 @@ namespace API::Subsonic return response; } - void Response::addNode(Node::Key key, Node node) + void Response::addNode(Node::Key key, Node&& node) { return _root._children["subsonic-response"].addChild(key, std::move(node)); } @@ -266,7 +266,7 @@ namespace API::Subsonic if (!first) os << ','; - os << "value:"; + os << "\"value\":"; serializeValue(os, *node._value); first = false; diff --git a/src/libs/subsonic/impl/SubsonicResponse.hpp b/src/libs/subsonic/impl/SubsonicResponse.hpp index 119630ce..9fe51a5b 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.hpp +++ b/src/libs/subsonic/impl/SubsonicResponse.hpp @@ -225,9 +225,9 @@ namespace API::Subsonic Node& createChild(Key key); Node& createArrayChild(Key key); - void addChild(Key key, Node node); + void addChild(Key key, Node&& node); void createEmptyArrayChild(Key key); - void addArrayChild(Key key, Node node); + void addArrayChild(Key key, Node&& node); void createEmptyArrayValue(Key key); void addArrayValue(Key key, std::string_view value); void addArrayValue(Key key, long long value); @@ -255,7 +255,7 @@ namespace API::Subsonic Response(Response&&) = default; Response& operator=(Response&&) = default; - void addNode(Node::Key key, Node node); + void addNode(Node::Key key, Node&& node); Node& createNode(Node::Key key); Node& createArrayNode(Node::Key key); diff --git a/src/libs/subsonic/impl/entrypoints/Playlists.cpp b/src/libs/subsonic/impl/entrypoints/Playlists.cpp index 049a8dd7..63b896a3 100644 --- a/src/libs/subsonic/impl/entrypoints/Playlists.cpp +++ b/src/libs/subsonic/impl/entrypoints/Playlists.cpp @@ -75,7 +75,7 @@ namespace API::Subsonic for (const TrackListEntry::pointer& entry : entries) playlistNode.addArrayChild("entry", createSongNode(entry->getTrack(), context.dbSession, user)); - response.addNode("playlist", playlistNode); + response.addNode("playlist", std::move(playlistNode)); return response; } diff --git a/src/libs/subsonic/impl/responses/Album.cpp b/src/libs/subsonic/impl/responses/Album.cpp index 0394e46f..0abb0b88 100644 --- a/src/libs/subsonic/impl/responses/Album.cpp +++ b/src/libs/subsonic/impl/responses/Album.cpp @@ -133,7 +133,7 @@ namespace API::Subsonic } if (const Wt::WDateTime dateTime{ Service::get()->getStarredDateTime(user->getId(), release->getId()) }; dateTime.isValid()) - albumNode.setAttribute("starred", StringUtils::toISO8601String(dateTime)); // TODO report correct date/time + albumNode.setAttribute("starred", StringUtils::toISO8601String(dateTime)); // OpenSubsonic specific fields (must always be set) if (!id3) @@ -160,33 +160,23 @@ namespace API::Subsonic params.setRelease(release->getId()); params.setClusterType(clusterType->getId()); - for (const ClusterId clusterId : Cluster::find(dbSession, params).results) - { - Cluster::pointer cluster{ Cluster::find(dbSession, clusterId) }; - if (cluster) - albumNode.addArrayValue(field, cluster->getName()); - } + for (const auto& cluster : Cluster::find(dbSession, params).results) + albumNode.addArrayValue(field, std::get(cluster)); } } }; addClusters("moods", "MOOD"); // Genres + albumNode.createEmptyArrayChild("genres"); + if (genreClusterType) { - albumNode.createEmptyArrayChild("genres"); - if (genreClusterType) - { - Cluster::FindParameters params; - params.setRelease(release->getId()); - params.setClusterType(genreClusterType->getId()); + Cluster::FindParameters params; + params.setRelease(release->getId()); + params.setClusterType(genreClusterType->getId()); - for (const ClusterId clusterId : Cluster::find(dbSession, params).results) - { - Cluster::pointer cluster{ Cluster::find(dbSession, clusterId) }; - if (cluster) - albumNode.addArrayChild("genres", createItemGenreNode(cluster)); - } - } + for (const auto& cluster : Cluster::find(dbSession, params).results) + albumNode.addArrayChild("genres", createItemGenreNode(std::get(cluster))); } albumNode.createEmptyArrayChild("artists"); diff --git a/src/libs/subsonic/impl/responses/ItemGenre.cpp b/src/libs/subsonic/impl/responses/ItemGenre.cpp index a5b3387d..70ab2b9e 100644 --- a/src/libs/subsonic/impl/responses/ItemGenre.cpp +++ b/src/libs/subsonic/impl/responses/ItemGenre.cpp @@ -23,11 +23,11 @@ namespace API::Subsonic { - Response::Node createItemGenreNode(const Database::Cluster::pointer& cluster) + Response::Node createItemGenreNode(std::string_view name) { Response::Node genreNode; - genreNode.setAttribute("name", cluster->getName()); + genreNode.setAttribute("name", name); return genreNode; } diff --git a/src/libs/subsonic/impl/responses/ItemGenre.hpp b/src/libs/subsonic/impl/responses/ItemGenre.hpp index f4c1dda9..c15a4674 100644 --- a/src/libs/subsonic/impl/responses/ItemGenre.hpp +++ b/src/libs/subsonic/impl/responses/ItemGenre.hpp @@ -19,7 +19,7 @@ #pragma once -#include "services/database/Object.hpp" +#include #include "SubsonicResponse.hpp" namespace Database @@ -29,5 +29,5 @@ namespace Database namespace API::Subsonic { - Response::Node createItemGenreNode(const Database::ObjectPtr& cluster); + Response::Node createItemGenreNode(std::string_view name); } diff --git a/src/libs/subsonic/impl/responses/Song.cpp b/src/libs/subsonic/impl/responses/Song.cpp index 8870d8a8..543737f4 100644 --- a/src/libs/subsonic/impl/responses/Song.cpp +++ b/src/libs/subsonic/impl/responses/Song.cpp @@ -221,34 +221,23 @@ namespace API::Subsonic params.setTrack(track->getId()); params.setClusterType(clusterType->getId()); - for (const ClusterId clusterId : Cluster::find(dbSession, params).results) - { - Cluster::pointer cluster {Cluster::find(dbSession, clusterId)}; - if (cluster) - trackResponse.addArrayValue(field, cluster->getName()); - } + for (const auto& cluster : Cluster::find(dbSession, params).results) + trackResponse.addArrayValue(field, std::get(cluster)); } } }; addClusters("moods", "MOOD"); // Genres + trackResponse.createEmptyArrayChild("genres"); + if (genreClusterType) { - trackResponse.createEmptyArrayChild("genres"); + Cluster::FindParameters params; + params.setTrack(track->getId()); + params.setClusterType(genreClusterType->getId()); - if (genreClusterType) - { - Cluster::FindParameters params; - params.setTrack(track->getId()); - params.setClusterType(genreClusterType->getId()); - - for (const ClusterId clusterId : Cluster::find(dbSession, params).results) - { - Cluster::pointer cluster{ Cluster::find(dbSession, clusterId) }; - if (cluster) - trackResponse.addArrayChild("genres", createItemGenreNode(cluster)); - } - } + for (const auto& cluster : Cluster::find(dbSession, params).results) + trackResponse.addArrayChild("genres", createItemGenreNode(std::get(cluster))); } trackResponse.addChild("replayGain", createReplayGainNode(track));