Fixed bad json musicBrainzId field in getArtistInfo2, fixes #497

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<db::User> user;
         ClientInfo clientInfo;
         ProtocolVersion serverProtocolVersion;
+        ResponseFormat responseFormat;
         bool enableOpenSubsonic{ true };
         bool enableDefaultCover{};
     };

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<std::string>(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)

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\":";
+            serializeEscapedString(os, key.str());
-            serializeValue(os, *node._value);
+            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);
-
+                firstChild = false;
-                first = 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)
+                if (!firstChild)
-                    os << ',';
+                    os << ",";
 
-                serializeEscapedString(os, key.str());
+                serializeValue(os, childValue);
-                os << ":[";
 
-                bool firstChild{ true };
+                firstChild = false;
-                for (const Response::Node& childNode : childArrayNodes)
-                {
-                    if (!firstChild)
-                        os << ",";
-
-                    serializeNode(os, childNode);
-                    firstChild = false;
-                }
-                os << ']';
-
-                first = false;
             }
+            os << ']';
 
-            for (const auto& [key, childValues] : node._childrenValues)
+            first = false;
-            {
-                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;
-            }
         }
 
         os << '}';

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

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<core::UUID> 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<recommendation::IRecommendationService>::get()->getSimilarArtists(id, { TrackArtistLinkType::Artist, TrackArtistLinkType::ReleaseArtist }, count) };

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;

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"

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<db::Release>& release, bool id3, const db::ObjectPtr<db::Directory>& directory = {});
 }

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

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<db::ObjectPtr<db::Artist>>& artists);

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());
 

src/libs/subsonic/impl/responses/Genre.hpp

@@ -30,5 +30,7 @@ namespace lms::db
 
 namespace lms::api::subsonic
 {
-    Response::Node createGenreNode(const db::ObjectPtr<db::Cluster>& cluster);
+    class RequestContext;
-}
+
+    Response::Node createGenreNode(RequestContext& context, const db::ObjectPtr<db::Cluster>& cluster);
+} // namespace lms::api::subsonic

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"

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<db::Track>& track, bool id3);
-}
+} // namespace lms::api::subsonic

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(RequestContext& context, const db::User::pointer& user)
-
-    Response::Node createUserNode(const 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("jukeboxRole", false); // not supported
-        userNode.setAttribute("shareRole", false);
+        userNode.setAttribute("shareRole", false);   // not supported
 
-        Response::Node folder;
+        // users can access all libraries
-        folder.setValue("0");
+        db::MediaLibrary::find(context.dbSession, [&](const db::MediaLibrary::pointer& library) {
-        userNode.addArrayChild("folder", std::move(folder));
+            userNode.addArrayValue("folder", idToString(library->getId()));
+        });
 
         return userNode;
     }

src/libs/subsonic/impl/responses/User.hpp

@@ -30,5 +30,7 @@ namespace lms::db
 
 namespace lms::api::subsonic
 {
-    Response::Node createUserNode(const db::ObjectPtr<db::User>& user);
+    class RequestContext;
+
+    Response::Node createUserNode(RequestContext& context, const db::ObjectPtr<db::User>& user);
 }