Subsonic API: added timestamps into coverart ids, Made use of image ids to save a lookup, ref #558

src/libs/subsonic/impl/CoverArtId.cpp

@@ -0,0 +1,97 @@
+/*
+ * Copyright (C) 2024 Emeric Poupon
+ *
+ * This file is part of LMS.
+ *
+ * LMS is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * LMS is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with LMS.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "CoverArtId.hpp"
+
+#include "SubsonicId.hpp"
+#include "core/String.hpp"
+
+namespace lms::api::subsonic
+{
+    namespace
+    {
+        constexpr char timestampSeparatorChar{ ':' };
+    }
+
+    std::string idToString(db::ImageId id)
+    {
+        return "im-" + id.toString();
+    }
+
+    std::string idToString(CoverArtId coverId)
+    {
+        // produce "id:timestamp"
+        std::string res{ std::visit([](auto&& id) {
+            return idToString(id);
+        },
+            coverId.id) };
+
+        res += timestampSeparatorChar;
+        res += std::to_string(coverId.timestamp);
+
+        return res;
+    }
+} // namespace lms::api::subsonic
+
+// Used to parse parameters
+namespace lms::core::stringUtils
+{
+    template<>
+    std::optional<db::ImageId> readAs(std::string_view str)
+    {
+        std::vector<std::string_view> values{ core::stringUtils::splitString(str, '-') };
+        if (values.size() != 2)
+            return std::nullopt;
+
+        if (values[0] != "im")
+            return std::nullopt;
+
+        if (const auto value{ core::stringUtils::readAs<db::ReleaseId::ValueType>(values[1]) })
+            return db::ImageId{ *value };
+
+        return std::nullopt;
+    }
+
+    template<>
+    std::optional<api::subsonic::CoverArtId> readAs(std::string_view str)
+    {
+        // expect "id:timestamp"
+        auto timeStampSeparator{ str.find_last_of(api::subsonic::timestampSeparatorChar) };
+        if (timeStampSeparator == std::string_view::npos)
+            return std::nullopt;
+
+        std::string_view strId{ str.substr(0, timeStampSeparator) };
+        std::string_view strTimestamp{ str.substr(timeStampSeparator + 1) };
+
+        api::subsonic::CoverArtId cover;
+        if (const auto imagetId{ readAs<db::ImageId>(strId) })
+            cover.id = *imagetId;
+        else if (const auto trackId{ readAs<db::TrackId>(strId) })
+            cover.id = *trackId;
+        else
+            return std::nullopt;
+
+        if (const auto timestamp{ readAs<std::time_t>(strTimestamp) })
+            cover.timestamp = *timestamp;
+        else
+            return std::nullopt;
+
+        return cover;
+    }
+} // namespace lms::core::stringUtils

src/libs/subsonic/impl/CoverArtId.hpp

@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2024 Emeric Poupon
+ *
+ * This file is part of LMS.
+ *
+ * LMS is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * LMS is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with LMS.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include <ctime>
+#include <variant>
+
+#include "core/String.hpp"
+#include "database/ImageId.hpp"
+#include "database/TrackId.hpp"
+
+namespace lms::api::subsonic
+{
+    struct CoverArtId
+    {
+        std::variant<db::ImageId, db::TrackId> id;
+        std::time_t timestamp;
+    };
+
+    std::string idToString(CoverArtId coverId);
+    std::string idToString(db::ImageId imageId);
+} // namespace lms::api::subsonic
+
+// Used to parse parameters
+namespace lms::core::stringUtils
+{
+    template<>
+    std::optional<db::ImageId> readAs(std::string_view str);
+
+    template<>
+    std::optional<api::subsonic::CoverArtId> readAs(std::string_view str);
+} // namespace lms::core::stringUtils

src/libs/subsonic/impl/RequestContext.hpp

@@ -47,6 +47,5 @@ namespace lms::api::subsonic
         ProtocolVersion serverProtocolVersion;
         ResponseFormat responseFormat;
         bool enableOpenSubsonic{ true };
-        bool enableDefaultCover{};
     };
 } // namespace lms::api::subsonic

src/libs/subsonic/impl/SubsonicResource.cpp

@@ -85,19 +85,6 @@ namespace lms::api::subsonic
             return res;
         }
 
-        std::unordered_set<std::string> readDefaultCoverClients()
-        {
-            std::unordered_set<std::string> res;
-
-            core::Service<core::IConfig>::get()->visitStrings("api-subsonic-default-cover-clients",
-                [&](std::string_view client) {
-                    res.emplace(std::string{ client });
-                },
-                { "DSub", "substreamer" });
-
-            return res;
-        }
-
         std::string parameterMapToDebugString(const Wt::Http::ParameterMap& parameterMap)
         {
             auto censorValue = [](const std::string& type, const std::string& value) -> std::string {
@@ -307,7 +294,6 @@ namespace lms::api::subsonic
     SubsonicResource::SubsonicResource(db::Db& db)
         : _serverProtocolVersionsByClient{ readConfigProtocolVersions() }
         , _openSubsonicDisabledClients{ readOpenSubsonicDisabledClients() }
-        , _defaultReleaseCoverClients{ readDefaultCoverClients() }
         , _supportUserPasswordAuthentication{ core::Service<core::IConfig>::get()->getBool("api-subsonic-support-user-password-auth", true) }
         , _db{ db }
     {
@@ -425,7 +411,6 @@ namespace lms::api::subsonic
         const Wt::Http::ParameterMap& parameters{ request.getParameterMap() };
         const ClientInfo clientInfo{ getClientInfo(request) };
         bool enableOpenSubsonic{ !_openSubsonicDisabledClients.contains(clientInfo.name) };
-        bool enableDefaultCover{ _defaultReleaseCoverClients.contains(clientInfo.name) };
         const ResponseFormat format{ getParameterAs<std::string>(request.getParameterMap(), "f").value_or("xml") == "json" ? ResponseFormat::json : ResponseFormat::xml };
 
         return RequestContext{
@@ -437,7 +422,6 @@ namespace lms::api::subsonic
             .serverProtocolVersion = getServerProtocolVersion(clientInfo.name),
             .responseFormat = format,
             .enableOpenSubsonic = enableOpenSubsonic,
-            .enableDefaultCover = enableDefaultCover
         };
     }
 

src/libs/subsonic/impl/SubsonicResource.hpp

@@ -51,7 +51,6 @@ namespace lms::api::subsonic
 
         const std::unordered_map<std::string, ProtocolVersion> _serverProtocolVersionsByClient;
         const std::unordered_set<std::string> _openSubsonicDisabledClients;
-        const std::unordered_set<std::string> _defaultReleaseCoverClients;
         const bool _supportUserPasswordAuthentication;
 
         db::Db& _db;

src/libs/subsonic/impl/endpoints/MediaRetrieval.cpp

@@ -24,7 +24,6 @@
 #include "av/TranscodingParameters.hpp"
 #include "av/TranscodingResourceHandlerCreator.hpp"
 #include "av/Types.hpp"
-#include "core/FileResourceHandlerCreator.hpp"
 #include "core/ILogger.hpp"
 #include "core/IResourceHandler.hpp"
 #include "core/String.hpp"
@@ -35,6 +34,7 @@
 #include "database/User.hpp"
 #include "services/artwork/IArtworkService.hpp"
 
+#include "CoverArtId.hpp"
 #include "ParameterParsing.hpp"
 #include "RequestContext.hpp"
 #include "SubsonicId.hpp"
@@ -326,36 +326,26 @@ namespace lms::api::subsonic
     void handleGetCoverArt(RequestContext& context, const Wt::Http::Request& /*request*/, Wt::Http::Response& response)
     {
         // Mandatory params
-        const auto trackId{ getParameterAs<TrackId>(context.parameters, "id") };
-        const auto releaseId{ getParameterAs<ReleaseId>(context.parameters, "id") };
-        const auto artistId{ getParameterAs<ArtistId>(context.parameters, "id") };
-
-        if (!trackId && !releaseId && !artistId)
-            throw BadParameterGenericError{ "id" };
+        const CoverArtId coverArtId{ getMandatoryParameterAs<CoverArtId>(context.parameters, "id") };
 
         std::optional<std::size_t> size{ getParameterAs<std::size_t>(context.parameters, "size") };
         if (size)
             *size = core::utils::clamp(*size, std::size_t{ 32 }, std::size_t{ 2048 });
 
-        std::shared_ptr<image::IEncodedImage> cover;
-        if (trackId)
-            cover = core::Service<cover::IArtworkService>::get()->getTrackImage(*trackId, size);
-        else if (releaseId)
-            cover = core::Service<cover::IArtworkService>::get()->getReleaseCover(*releaseId, size);
-        else if (artistId)
-            cover = core::Service<cover::IArtworkService>::get()->getArtistImage(*artistId, size);
+        std::shared_ptr<image::IEncodedImage> image;
+        if (const db::TrackId * trackId{ std::get_if<db::TrackId>(&coverArtId.id) })
+            image = core::Service<cover::IArtworkService>::get()->getTrackImage(*trackId, size);
+        else if (const db::ImageId * imageId{ std::get_if<db::ImageId>(&coverArtId.id) })
+            image = core::Service<cover::IArtworkService>::get()->getImage(*imageId, size);
 
-        if (!cover && context.enableDefaultCover && !artistId)
-            cover = core::Service<cover::IArtworkService>::get()->getDefaultReleaseCover();
-
-        if (!cover)
+        if (!image)
         {
             response.setStatus(404);
             return;
         }
 
-        response.out().write(reinterpret_cast<const char*>(cover->getData().data()), cover->getData().size());
-        response.setMimeType(std::string{ cover->getMimeType() });
+        response.out().write(reinterpret_cast<const char*>(image->getData().data()), image->getData().size());
+        response.setMimeType(std::string{ image->getMimeType() });
     }
 
 } // namespace lms::api::subsonic

src/libs/subsonic/impl/responses/Album.cpp

@@ -32,6 +32,7 @@
 #include "services/feedback/IFeedbackService.hpp"
 #include "services/scrobbling/IScrobblingService.hpp"
 
+#include "CoverArtId.hpp"
 #include "RequestContext.hpp"
 #include "SubsonicId.hpp"
 #include "responses/Artist.hpp"
@@ -84,9 +85,10 @@ namespace lms::api::subsonic
         }
 
         albumNode.setAttribute("created", core::stringUtils::toISO8601String(release->getLastWritten()));
-        if (release->getImage())
+        if (const auto image{ release->getImage() })
         {
-            albumNode.setAttribute("coverArt", idToString(release->getId()));
+            const CoverArtId coverArtId{ image->getId(), image->getLastWriteTime().toTime_t() };
+            albumNode.setAttribute("coverArt", idToString(coverArtId));
         }
         else
         {
@@ -96,7 +98,8 @@ namespace lms::api::subsonic
             params.setRange(db::Range{ 0, 1 });
 
             db::Track::find(context.dbSession, params, [&](const db::Track::pointer& track) {
-                albumNode.setAttribute("coverArt", idToString(track->getId()));
+                const CoverArtId coverArtId{ track->getId(), track->getLastWriteTime().toTime_t() };
+                albumNode.setAttribute("coverArt", idToString(coverArtId));
             });
         }
         if (const auto year{ release->getYear() })

src/libs/subsonic/impl/responses/Artist.cpp

@@ -29,6 +29,7 @@
 #include "database/User.hpp"
 #include "services/feedback/IFeedbackService.hpp"
 
+#include "CoverArtId.hpp"
 #include "RequestContext.hpp"
 #include "SubsonicId.hpp"
 
@@ -94,8 +95,11 @@ namespace lms::api::subsonic
 
         artistNode.setAttribute("id", idToString(artist->getId()));
         artistNode.setAttribute("name", artist->getName());
-        if (artist->getImage())
-            artistNode.setAttribute("coverArt", idToString(artist->getId()));
+        if (const auto image{ artist->getImage() })
+        {
+            const CoverArtId coverArtId{ image->getId(), image->getLastWriteTime().toTime_t() };
+            artistNode.setAttribute("coverArt", idToString(coverArtId));
+        }
 
         const std::size_t count{ Release::getCount(context.dbSession, Release::FindParameters{}.setArtist(artist->getId())) };
         artistNode.setAttribute("albumCount", count);

src/libs/subsonic/impl/responses/Playlist.cpp

@@ -23,6 +23,7 @@
 #include "database/TrackList.hpp"
 #include "database/User.hpp"
 
+#include "CoverArtId.hpp"
 #include "SubsonicId.hpp"
 
 namespace lms::api::subsonic
@@ -50,7 +51,8 @@ namespace lms::api::subsonic
         params.setSortMethod(TrackSortMethod::TrackList);
 
         db::Track::find(session, params, [&](const db::Track::pointer& track) {
-            playlistNode.setAttribute("coverArt", idToString(track->getId()));
+            const CoverArtId coverArtId{ track->getId(), track->getLastWriteTime().toTime_t() };
+            playlistNode.setAttribute("coverArt", idToString(coverArtId));
         });
 
         return playlistNode;

src/libs/subsonic/impl/responses/Song.cpp

@@ -36,6 +36,7 @@
 #include "services/feedback/IFeedbackService.hpp"
 #include "services/scrobbling/IScrobblingService.hpp"
 
+#include "CoverArtId.hpp"
 #include "RequestContext.hpp"
 #include "SubsonicId.hpp"
 #include "responses/Artist.hpp"
@@ -109,9 +110,18 @@ namespace lms::api::subsonic
         const Release::pointer release{ track->getRelease() };
 
         if (track->hasCover())
-            trackResponse.setAttribute("coverArt", idToString(track->getId()));
-        else if (release && release->getImage())
-            trackResponse.setAttribute("coverArt", idToString(release->getId()));
+        {
+            const CoverArtId coverArtId{ track->getId(), track->getLastWriteTime().toTime_t() };
+            trackResponse.setAttribute("coverArt", idToString(coverArtId));
+        }
+        else if (release)
+        {
+            if (const db::Image::pointer image{ release->getImage() })
+            {
+                const CoverArtId coverArtId{ image->getId(), image->getLastWriteTime().toTime_t() };
+                trackResponse.setAttribute("coverArt", idToString(coverArtId));
+            }
+        }
 
         const std::vector<Artist::pointer>& artists{ track->getArtists({ TrackArtistLinkType::Artist }) };
         if (!artists.empty())