From 9a91a2e080a4bec6bfbec618ac28a7053b77b7da Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 28 Jul 2024 14:42:16 +0200 Subject: [PATCH] Fixed bad json musicBrainzId field in getArtistInfo2, fixes #497 --- src/libs/subsonic/impl/RequestContext.hpp | 2 + src/libs/subsonic/impl/SubsonicResource.cpp | 12 ++- src/libs/subsonic/impl/SubsonicResponse.cpp | 92 +++++++++---------- src/libs/subsonic/impl/SubsonicResponse.hpp | 2 +- .../subsonic/impl/entrypoints/Browsing.cpp | 14 ++- .../impl/entrypoints/UserManagement.cpp | 4 +- src/libs/subsonic/impl/responses/Album.cpp | 1 + src/libs/subsonic/impl/responses/Album.hpp | 2 + src/libs/subsonic/impl/responses/Artist.cpp | 1 + src/libs/subsonic/impl/responses/Artist.hpp | 2 + src/libs/subsonic/impl/responses/Genre.cpp | 14 ++- src/libs/subsonic/impl/responses/Genre.hpp | 6 +- src/libs/subsonic/impl/responses/Song.cpp | 1 + src/libs/subsonic/impl/responses/Song.hpp | 4 +- src/libs/subsonic/impl/responses/User.cpp | 21 +++-- src/libs/subsonic/impl/responses/User.hpp | 4 +- 16 files changed, 110 insertions(+), 72 deletions(-) diff --git a/src/libs/subsonic/impl/RequestContext.hpp b/src/libs/subsonic/impl/RequestContext.hpp index f90b4940..e1f6f687 100644 --- a/src/libs/subsonic/impl/RequestContext.hpp +++ b/src/libs/subsonic/impl/RequestContext.hpp @@ -27,6 +27,7 @@ #include "ClientInfo.hpp" #include "ProtocolVersion.hpp" +#include "SubsonicResponse.hpp" namespace lms::db { @@ -43,6 +44,7 @@ namespace lms::api::subsonic const db::ObjectPtr user; ClientInfo clientInfo; ProtocolVersion serverProtocolVersion; + ResponseFormat responseFormat; bool enableOpenSubsonic{ true }; bool enableDefaultCover{}; }; diff --git a/src/libs/subsonic/impl/SubsonicResource.cpp b/src/libs/subsonic/impl/SubsonicResource.cpp index 6bf949f2..1a212a00 100644 --- a/src/libs/subsonic/impl/SubsonicResource.cpp +++ b/src/libs/subsonic/impl/SubsonicResource.cpp @@ -411,6 +411,7 @@ namespace lms::api::subsonic const db::UserId userId{ authenticateUser(request, clientInfo) }; bool enableOpenSubsonic{ _openSubsonicDisabledClients.find(clientInfo.name) == std::cend(_openSubsonicDisabledClients) }; bool enableDefaultCover{ _defaultCoverClients.find(clientInfo.name) != std::cend(_openSubsonicDisabledClients) }; + const ResponseFormat format{ getParameterAs(request.getParameterMap(), "f").value_or("xml") == "json" ? ResponseFormat::json : ResponseFormat::xml }; db::User::pointer user; { @@ -422,7 +423,16 @@ namespace lms::api::subsonic throw UserNotAuthorizedError{}; } - return { parameters, _db.getTLSSession(), user, clientInfo, getServerProtocolVersion(clientInfo.name), enableOpenSubsonic, enableDefaultCover }; + return RequestContext{ + .parameters = parameters, + .dbSession = _db.getTLSSession(), + .user = user, + .clientInfo = clientInfo, + .serverProtocolVersion = getServerProtocolVersion(clientInfo.name), + .responseFormat = format, + .enableOpenSubsonic = enableOpenSubsonic, + .enableDefaultCover = enableDefaultCover + }; } db::UserId SubsonicResource::authenticateUser(const Wt::Http::Request& request, const ClientInfo& clientInfo) diff --git a/src/libs/subsonic/impl/SubsonicResponse.cpp b/src/libs/subsonic/impl/SubsonicResponse.cpp index 90ae1faf..6788aead 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.cpp +++ b/src/libs/subsonic/impl/SubsonicResponse.cpp @@ -266,74 +266,64 @@ namespace lms::api::subsonic first = false; } - if (node._value) + // Values are handled manually (using attributes) in json format + assert(!node._value); + + for (const auto& [key, childNode] : node._children) { if (!first) os << ','; - os << "\"value\":"; - serializeValue(os, *node._value); + serializeEscapedString(os, key.str()); + os << ':'; + serializeNode(os, childNode); first = false; } - else + + for (const auto& [key, childArrayNodes] : node._childrenArrays) { - for (const auto& [key, childNode] : node._children) - { - if (!first) - os << ','; + if (!first) + os << ','; + + serializeEscapedString(os, key.str()); + os << ":["; + + bool firstChild{ true }; + for (const Response::Node& childNode : childArrayNodes) + { + if (!firstChild) + os << ","; - serializeEscapedString(os, key.str()); - os << ':'; serializeNode(os, childNode); - - first = false; + firstChild = false; } + os << ']'; - for (const auto& [key, childArrayNodes] : node._childrenArrays) + first = false; + } + + for (const auto& [key, childValues] : node._childrenValues) + { + if (!first) + os << ','; + + serializeEscapedString(os, key.str()); + os << ":["; + + bool firstChild{ true }; + for (const Node::ValueType& childValue : childValues) { - if (!first) - os << ','; + if (!firstChild) + os << ","; - serializeEscapedString(os, key.str()); - os << ":["; + serializeValue(os, childValue); - bool firstChild{ true }; - for (const Response::Node& childNode : childArrayNodes) - { - if (!firstChild) - os << ","; - - serializeNode(os, childNode); - firstChild = false; - } - os << ']'; - - first = false; + firstChild = false; } + os << ']'; - for (const auto& [key, childValues] : node._childrenValues) - { - if (!first) - os << ','; - - serializeEscapedString(os, key.str()); - os << ":["; - - bool firstChild{ true }; - for (const Node::ValueType& childValue : childValues) - { - if (!firstChild) - os << ","; - - serializeValue(os, childValue); - - firstChild = false; - } - os << ']'; - - first = false; - } + first = false; } os << '}'; diff --git a/src/libs/subsonic/impl/SubsonicResponse.hpp b/src/libs/subsonic/impl/SubsonicResponse.hpp index 4c273ab5..b8cacdb6 100644 --- a/src/libs/subsonic/impl/SubsonicResponse.hpp +++ b/src/libs/subsonic/impl/SubsonicResponse.hpp @@ -27,7 +27,7 @@ #include "core/LiteralString.hpp" -#include "RequestContext.hpp" +#include "ProtocolVersion.hpp" #include "SubsonicResponseAllocator.hpp" namespace lms::api::subsonic diff --git a/src/libs/subsonic/impl/entrypoints/Browsing.cpp b/src/libs/subsonic/impl/entrypoints/Browsing.cpp index f7221b46..82c86a54 100644 --- a/src/libs/subsonic/impl/entrypoints/Browsing.cpp +++ b/src/libs/subsonic/impl/entrypoints/Browsing.cpp @@ -362,7 +362,7 @@ namespace lms::api::subsonic const auto clusters{ clusterType->getClusters() }; for (const Cluster::pointer& cluster : clusters) - genresNode.addArrayChild("genre", createGenreNode(cluster)); + genresNode.addArrayChild("genre", createGenreNode(context, cluster)); } return response; @@ -531,7 +531,17 @@ namespace lms::api::subsonic std::optional artistMBID{ artist->getMBID() }; if (artistMBID) - artistInfoNode.createChild("musicBrainzId").setValue(artistMBID->getAsString()); + { + switch (context.responseFormat) + { + case ResponseFormat::json: + artistInfoNode.setAttribute("musicBrainzId", artistMBID->getAsString()); + break; + case ResponseFormat::xml: + artistInfoNode.createChild("musicBrainzId").setValue(artistMBID->getAsString()); + break; + } + } } auto similarArtistsId{ core::Service::get()->getSimilarArtists(id, { TrackArtistLinkType::Artist, TrackArtistLinkType::ReleaseArtist }, count) }; diff --git a/src/libs/subsonic/impl/entrypoints/UserManagement.cpp b/src/libs/subsonic/impl/entrypoints/UserManagement.cpp index 3e335110..3858f7b1 100644 --- a/src/libs/subsonic/impl/entrypoints/UserManagement.cpp +++ b/src/libs/subsonic/impl/entrypoints/UserManagement.cpp @@ -35,7 +35,7 @@ namespace lms::api::subsonic throw RequestedDataNotFoundError{}; Response response{ Response::createOkResponse(context.serverProtocolVersion) }; - response.addNode("user", createUserNode(user)); + response.addNode("user", createUserNode(context, user)); return response; } @@ -47,7 +47,7 @@ namespace lms::api::subsonic auto transaction{ context.dbSession.createReadTransaction() }; User::find(context.dbSession, User::FindParameters{}, [&](const User::pointer& user) { - usersNode.addArrayChild("user", createUserNode(user)); + usersNode.addArrayChild("user", createUserNode(context, user)); }); return response; diff --git a/src/libs/subsonic/impl/responses/Album.cpp b/src/libs/subsonic/impl/responses/Album.cpp index 88d00e43..3d1fe45e 100644 --- a/src/libs/subsonic/impl/responses/Album.cpp +++ b/src/libs/subsonic/impl/responses/Album.cpp @@ -30,6 +30,7 @@ #include "services/feedback/IFeedbackService.hpp" #include "services/scrobbling/IScrobblingService.hpp" +#include "RequestContext.hpp" #include "SubsonicId.hpp" #include "responses/Artist.hpp" #include "responses/DiscTitle.hpp" diff --git a/src/libs/subsonic/impl/responses/Album.hpp b/src/libs/subsonic/impl/responses/Album.hpp index d324fe24..3b843772 100644 --- a/src/libs/subsonic/impl/responses/Album.hpp +++ b/src/libs/subsonic/impl/responses/Album.hpp @@ -33,5 +33,7 @@ namespace lms::db namespace lms::api::subsonic { + class RequestContext; + Response::Node createAlbumNode(RequestContext& context, const db::ObjectPtr& release, bool id3, const db::ObjectPtr& directory = {}); } \ No newline at end of file diff --git a/src/libs/subsonic/impl/responses/Artist.cpp b/src/libs/subsonic/impl/responses/Artist.cpp index 77481fba..2bf29e0d 100644 --- a/src/libs/subsonic/impl/responses/Artist.cpp +++ b/src/libs/subsonic/impl/responses/Artist.cpp @@ -29,6 +29,7 @@ #include "database/User.hpp" #include "services/feedback/IFeedbackService.hpp" +#include "RequestContext.hpp" #include "SubsonicId.hpp" namespace lms::api::subsonic diff --git a/src/libs/subsonic/impl/responses/Artist.hpp b/src/libs/subsonic/impl/responses/Artist.hpp index 3a8a5687..9830a102 100644 --- a/src/libs/subsonic/impl/responses/Artist.hpp +++ b/src/libs/subsonic/impl/responses/Artist.hpp @@ -36,6 +36,8 @@ namespace lms::db namespace lms::api::subsonic { + class RequestContext; + namespace utils { std::string joinArtistNames(const std::vector>& artists); diff --git a/src/libs/subsonic/impl/responses/Genre.cpp b/src/libs/subsonic/impl/responses/Genre.cpp index 308fe29f..2ae28e9f 100644 --- a/src/libs/subsonic/impl/responses/Genre.cpp +++ b/src/libs/subsonic/impl/responses/Genre.cpp @@ -21,13 +21,23 @@ #include "database/Cluster.hpp" +#include "RequestContext.hpp" + namespace lms::api::subsonic { - Response::Node createGenreNode(const db::Cluster::pointer& cluster) + Response::Node createGenreNode(RequestContext& context, const db::Cluster::pointer& cluster) { Response::Node clusterNode; - clusterNode.setValue(cluster->getName()); + switch (context.responseFormat) + { + case ResponseFormat::json: + clusterNode.setAttribute("value", cluster->getName()); + break; + case ResponseFormat::xml: + clusterNode.setValue(cluster->getName()); + break; + } clusterNode.setAttribute("songCount", cluster->getTrackCount()); clusterNode.setAttribute("albumCount", cluster->getReleasesCount()); diff --git a/src/libs/subsonic/impl/responses/Genre.hpp b/src/libs/subsonic/impl/responses/Genre.hpp index 45017625..cd34c9dd 100644 --- a/src/libs/subsonic/impl/responses/Genre.hpp +++ b/src/libs/subsonic/impl/responses/Genre.hpp @@ -30,5 +30,7 @@ namespace lms::db namespace lms::api::subsonic { - Response::Node createGenreNode(const db::ObjectPtr& cluster); -} + class RequestContext; + + Response::Node createGenreNode(RequestContext& context, const db::ObjectPtr& cluster); +} // namespace lms::api::subsonic diff --git a/src/libs/subsonic/impl/responses/Song.cpp b/src/libs/subsonic/impl/responses/Song.cpp index 9ff70831..d01205e5 100644 --- a/src/libs/subsonic/impl/responses/Song.cpp +++ b/src/libs/subsonic/impl/responses/Song.cpp @@ -35,6 +35,7 @@ #include "services/feedback/IFeedbackService.hpp" #include "services/scrobbling/IScrobblingService.hpp" +#include "RequestContext.hpp" #include "SubsonicId.hpp" #include "Utils.hpp" #include "responses/Artist.hpp" diff --git a/src/libs/subsonic/impl/responses/Song.hpp b/src/libs/subsonic/impl/responses/Song.hpp index 5b35227b..ac22ba1a 100644 --- a/src/libs/subsonic/impl/responses/Song.hpp +++ b/src/libs/subsonic/impl/responses/Song.hpp @@ -32,5 +32,7 @@ namespace lms::db namespace lms::api::subsonic { + class RequestContext; + Response::Node createSongNode(RequestContext& context, const db::ObjectPtr& track, bool id3); -} \ No newline at end of file +} // namespace lms::api::subsonic \ No newline at end of file diff --git a/src/libs/subsonic/impl/responses/User.cpp b/src/libs/subsonic/impl/responses/User.cpp index 69c8571a..557b6acb 100644 --- a/src/libs/subsonic/impl/responses/User.cpp +++ b/src/libs/subsonic/impl/responses/User.cpp @@ -19,13 +19,15 @@ #include "responses/User.hpp" +#include "database/MediaLibrary.hpp" #include "database/User.hpp" +#include "RequestContext.hpp" +#include "SubsonicId.hpp" + namespace lms::api::subsonic { - using namespace db; - - Response::Node createUserNode(const User::pointer& user) + Response::Node createUserNode(RequestContext& context, const db::User::pointer& user) { Response::Node userNode; @@ -38,14 +40,15 @@ namespace lms::api::subsonic userNode.setAttribute("playlistRole", true); userNode.setAttribute("coverArtRole", false); userNode.setAttribute("commentRole", false); - userNode.setAttribute("podcastRole", false); + userNode.setAttribute("podcastRole", false); // not supported userNode.setAttribute("streamRole", true); - userNode.setAttribute("jukeboxRole", false); - userNode.setAttribute("shareRole", false); + userNode.setAttribute("jukeboxRole", false); // not supported + userNode.setAttribute("shareRole", false); // not supported - Response::Node folder; - folder.setValue("0"); - userNode.addArrayChild("folder", std::move(folder)); + // users can access all libraries + db::MediaLibrary::find(context.dbSession, [&](const db::MediaLibrary::pointer& library) { + userNode.addArrayValue("folder", idToString(library->getId())); + }); return userNode; } diff --git a/src/libs/subsonic/impl/responses/User.hpp b/src/libs/subsonic/impl/responses/User.hpp index 9ced3725..37082684 100644 --- a/src/libs/subsonic/impl/responses/User.hpp +++ b/src/libs/subsonic/impl/responses/User.hpp @@ -30,5 +30,7 @@ namespace lms::db namespace lms::api::subsonic { - Response::Node createUserNode(const db::ObjectPtr& user); + class RequestContext; + + Response::Node createUserNode(RequestContext& context, const db::ObjectPtr& user); } \ No newline at end of file