Subsonic API: now returns 404 when the requested cover art is not found (but a client list has been added to serve them a default cover art)

conf/lms.conf

@@ -61,7 +61,10 @@ api-subsonic = true;
 
 # Use this list to make the reported server version to 1.12.0 depending on the client's name
 # Main usage is to make auto detections for the 'p' (password) parameter work
-api-subsonic-report-old-server-protocol = ("DSub");
+api-subsonic-old-server-protocol-clients = ("DSub");
+
+# List of clients for whom a default cover is served (as they do not have their own)
+api-subsonic-default-cover-clients = ("DSub", "substreamer");
 
 # List of clients for whom open subsonic extensions and extra fields are disabled
 api-open-subsonic-disabled-clients = ("DSub");

src/libs/services/cover/impl/CoverService.cpp

@@ -345,9 +345,6 @@ namespace Cover
             }
         }
 
-        if (!cover)
-            cover = getDefault(width);
-
         if (cover)
             saveToCache(cacheEntryDesc, cover);
 
@@ -397,9 +394,6 @@ namespace Cover
                 cover = getFromTrack(session, releaseInfo->firstTrackId, width, false /* no release fallback */);
         }
 
-        if (!cover)
-            cover = getDefault(width);
-
         if (cover)
             saveToCache(cacheEntryDesc, cover);
 

src/libs/services/cover/impl/CoverService.hpp

@@ -56,19 +56,17 @@ namespace Cover
                 && size == other.size;
         }
     };
-
 } // ns Cover
 
 namespace std
 {
-
     template<>
     class hash<Cover::CacheEntryDesc>
     {
     public:
         size_t operator()(const Cover::CacheEntryDesc& e) const
         {
-				size_t h {};
+            size_t h{};
             std::visit([&](auto id)
                 {
                     using IdType = std::decay_t<decltype(id)>;
@@ -86,18 +84,15 @@ namespace Cover
     class CoverService : public ICoverService
     {
     public:
-			CoverService(Database::Db& db,
+        CoverService(Database::Db& db, const std::filesystem::path& execPath, const std::filesystem::path& defaultCoverPath);
-					const std::filesystem::path& execPath,
-					const std::filesystem::path& defaultCoverPath);
 
         CoverService(const CoverService&) = delete;
         CoverService& operator=(const CoverService&) = delete;
-			CoverService(CoverService&&) = delete;
-			CoverService& operator=(CoverService&&) = delete;
 
     private:
         std::shared_ptr<Image::IEncodedImage>   getFromTrack(Database::TrackId trackId, Image::ImageSize width) override;
         std::shared_ptr<Image::IEncodedImage>   getFromRelease(Database::ReleaseId releaseId, Image::ImageSize width) override;
+        std::shared_ptr<Image::IEncodedImage>   getDefault(Image::ImageSize width) override;
         void                                    flushCache() override;
         void                                    setJpegQuality(unsigned quality) override;
 
@@ -109,7 +104,6 @@ namespace Cover
         std::multimap<std::string, std::filesystem::path>   getCoverPaths(const std::filesystem::path& directoryPath) const;
         std::unique_ptr<Image::IEncodedImage>   getFromDirectory(const std::filesystem::path& directory, Image::ImageSize width) const;
         std::unique_ptr<Image::IEncodedImage>   getFromSameNamedFile(const std::filesystem::path& filePath, Image::ImageSize width) const;
-			std::shared_ptr<Image::IEncodedImage>	getDefault(Image::ImageSize width);
 
         bool                                    checkCoverFile(const std::filesystem::path& directoryPath) const;
 
@@ -118,16 +112,16 @@ namespace Cover
         std::shared_mutex _cacheMutex;
         std::unordered_map<CacheEntryDesc, std::shared_ptr<Image::IEncodedImage>> _cache;
         std::unordered_map<Image::ImageSize, std::shared_ptr<Image::IEncodedImage>> _defaultCoverCache;
-			std::atomic<std::size_t>	_cacheMisses {};
+        std::atomic<std::size_t>    _cacheMisses{};
-			std::atomic<std::size_t>	_cacheHits {};
+        std::atomic<std::size_t>    _cacheHits{};
-			std::size_t					_cacheSize {};
+        std::size_t                 _cacheSize{};
 
         void saveToCache(const CacheEntryDesc& entryDesc, std::shared_ptr<Image::IEncodedImage> image);
         std::shared_ptr<Image::IEncodedImage> loadFromCache(const CacheEntryDesc& entryDesc);
 
         const std::filesystem::path _defaultCoverPath;
         const std::size_t _maxCacheSize;
-			static inline const std::vector<std::filesystem::path> _fileExtensions {".jpg", ".jpeg", ".png", ".bmp"}; // TODO parametrize
+        static inline const std::vector<std::filesystem::path> _fileExtensions{ ".jpg", ".jpeg", ".png", ".bmp" }; // TODO parametrize
         const std::size_t _maxFileSize;
         const std::vector<std::string> _preferredFileNames;
         unsigned _jpegQuality;

src/libs/services/cover/include/services/cover/ICoverService.hpp

@@ -41,14 +41,14 @@ namespace Cover
         virtual std::shared_ptr<Image::IEncodedImage> getFromTrack(Database::TrackId trackId, Image::ImageSize width) = 0;
         virtual std::shared_ptr<Image::IEncodedImage> getFromRelease(Database::ReleaseId releaseId, Image::ImageSize width) = 0;
 
+        virtual std::shared_ptr<Image::IEncodedImage> getDefault(Image::ImageSize width) = 0;
+
         virtual void flushCache() = 0;
 
         virtual void setJpegQuality(unsigned quality) = 0; // from 1 to 100
     };
 
-	std::unique_ptr<ICoverService> createCoverService(Database::Db& db,
+    std::unique_ptr<ICoverService> createCoverService(Database::Db& db, const std::filesystem::path& execPath, const std::filesystem::path& defaultCoverPath);
-										const std::filesystem::path& execPath,
-										const std::filesystem::path& defaultCoverPath);
 
 } // namespace CoverArt
 

src/libs/subsonic/impl/RequestContext.hpp

@@ -42,6 +42,7 @@ namespace API::Subsonic
         ClientInfo clientInfo;
         ProtocolVersion serverProtocolVersion;
         bool enableOpenSubsonic{ true };
+        bool enableDefaultCover{ };
     };
 }
 

src/libs/subsonic/impl/SubsonicResource.cpp

@@ -66,7 +66,7 @@ namespace API::Subsonic
         {
             std::unordered_map<std::string, ProtocolVersion> res;
 
-            Service<IConfig>::get()->visitStrings("api-subsonic-report-old-server-protocol",
+            Service<IConfig>::get()->visitStrings("api-subsonic-old-server-protocol-clients",
                 [&](std::string_view client)
                 {
                     res.emplace(std::string{ client }, ProtocolVersion{ 1, 12, 0 });
@@ -88,6 +88,19 @@ namespace API::Subsonic
             return res;
         }
 
+        std::unordered_set<std::string> readDefaultCoverClients()
+        {
+            std::unordered_set<std::string> res;
+
+            Service<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
@@ -266,6 +279,7 @@ namespace API::Subsonic
     SubsonicResource::SubsonicResource(Db& db)
         : _serverProtocolVersionsByClient{ readConfigProtocolVersions() }
         , _openSubsonicDisabledClients{ readOpenSubsonicDisabledClients() }
+        , _defaultCoverClients{ readDefaultCoverClients() }
         , _db{ db }
     {
     }
@@ -378,8 +392,9 @@ namespace API::Subsonic
         const ClientInfo clientInfo{ getClientInfo(parameters) };
         const Database::UserId userId{ authenticateUser(request, clientInfo) };
         bool enableOpenSubsonic{ _openSubsonicDisabledClients.find(clientInfo.name) == std::cend(_openSubsonicDisabledClients) };
+        bool enableDefaultCover{ _defaultCoverClients.find(clientInfo.name) != std::cend(_openSubsonicDisabledClients) };
 
-        return { parameters, _db.getTLSSession(), userId, clientInfo, getServerProtocolVersion(clientInfo.name), enableOpenSubsonic };
+        return { parameters, _db.getTLSSession(), userId, clientInfo, getServerProtocolVersion(clientInfo.name), enableOpenSubsonic, enableDefaultCover };
     }
 
     Database::UserId SubsonicResource::authenticateUser(const Wt::Http::Request& request, const ClientInfo& clientInfo)

src/libs/subsonic/impl/SubsonicResource.hpp

@@ -53,6 +53,8 @@ namespace API::Subsonic
 
             const std::unordered_map<std::string, ProtocolVersion> _serverProtocolVersionsByClient;
             const std::unordered_set<std::string> _openSubsonicDisabledClients;
+            const std::unordered_set<std::string> _defaultCoverClients;
+
             Database::Db& _db;
     };
 

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

@@ -207,8 +207,9 @@ namespace API::Subsonic
         // 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)
+        if (!trackId && !releaseId && !artistId)
             throw BadParameterGenericError{ "id" };
 
         std::size_t size{ getParameterAs<std::size_t>(context.parameters, "size").value_or(1024) };
@@ -219,6 +220,21 @@ namespace API::Subsonic
             cover = Service<Cover::ICoverService>::get()->getFromTrack(*trackId, size);
         else if (releaseId)
             cover = Service<Cover::ICoverService>::get()->getFromRelease(*releaseId, size);
+        else if (artistId)
+        {
+            // TODO handle a placeholder for artists
+            response.setStatus(404);
+            return;
+        }
+
+        if (!cover && context.enableDefaultCover)
+            cover = Service<Cover::ICoverService>::get()->getDefault(size);
+
+        if (!cover)
+        {
+            response.setStatus(404);
+            return;
+        }
 
         response.out().write(reinterpret_cast<const char*>(cover->getData()), cover->getDataSize());
         response.setMimeType(std::string{ cover->getMimeType() });

src/lms/ui/resource/CoverResource.cpp

@@ -35,39 +35,36 @@
 
 namespace UserInterface {
 
-CoverResource::CoverResource()
+    CoverResource::CoverResource()
-{
+    {
         LmsApp->getScannerEvents().scanComplete.connect(this, [this](const Scanner::ScanStats& stats)
             {
                 if (stats.nbChanges())
                     setChanged();
             });
-}
+    }
 
-CoverResource::~CoverResource()
+    CoverResource::~CoverResource()
-{
+    {
         beingDeleted();
-}
+    }
 
-std::string
+    std::string CoverResource::getReleaseUrl(Database::ReleaseId releaseId, Size size) const
-CoverResource::getReleaseUrl(Database::ReleaseId releaseId, Size size) const
+    {
-{
         return url() + "&releaseid=" + releaseId.toString() + "&size=" + std::to_string(static_cast<std::size_t>(size));
-}
+    }
 
-std::string
+    std::string CoverResource::getTrackUrl(Database::TrackId trackId, Size size) const
-CoverResource::getTrackUrl(Database::TrackId trackId, Size size) const
+    {
-{
         return url() + "&trackid=" + trackId.toString() + "&size=" + std::to_string(static_cast<std::size_t>(size));
-}
+    }
 
-void
+    void CoverResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Response& response)
-CoverResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Response& response)
+    {
-{
         // Retrieve parameters
-	const std::string *trackIdStr = request.getParameter("trackid");
+        const std::string* trackIdStr = request.getParameter("trackid");
-	const std::string *releaseIdStr = request.getParameter("releaseid");
+        const std::string* releaseIdStr = request.getParameter("releaseid");
-	const std::string *sizeStr = request.getParameter("size");
+        const std::string* sizeStr = request.getParameter("size");
 
         // Mandatory parameter size
         if (!sizeStr)
@@ -76,7 +73,7 @@ CoverResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons
             return;
         }
 
-	const auto size {StringUtils::readAs<std::size_t>(*sizeStr)};
+        const auto size{ StringUtils::readAs<std::size_t>(*sizeStr) };
         if (!size || *size > maxSize)
         {
             LOG(DEBUG) << "invalid size provided!";
@@ -89,7 +86,7 @@ CoverResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons
         {
             LOG(DEBUG) << "Requested cover for track " << *trackIdStr << ", size = " << *size;
 
-		const std::optional<Database::TrackId> trackId {StringUtils::readAs<Database::TrackId::ValueType>(*trackIdStr)};
+            const std::optional<Database::TrackId> trackId{ StringUtils::readAs<Database::TrackId::ValueType>(*trackIdStr) };
             if (!trackId)
             {
                 LOG(DEBUG) << "track not found";
@@ -97,16 +94,20 @@ CoverResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons
             }
 
             cover = Service<Cover::ICoverService>::get()->getFromTrack(*trackId, *size);
+            if (!cover)
+                cover = Service<Cover::ICoverService>::get()->getDefault(*size);
         }
         else if (releaseIdStr)
         {
             LOG(DEBUG) << "Requested cover for release " << *releaseIdStr << ", size = " << *size;
 
-		const std::optional<Database::ReleaseId> releaseId {StringUtils::readAs<Database::ReleaseId::ValueType>(*releaseIdStr)};
+            const std::optional<Database::ReleaseId> releaseId{ StringUtils::readAs<Database::ReleaseId::ValueType>(*releaseIdStr) };
             if (!releaseId)
                 return;
 
             cover = Service<Cover::ICoverService>::get()->getFromRelease(*releaseId, *size);
+            if (!cover)
+                cover = Service<Cover::ICoverService>::get()->getDefault(*size);
         }
         else
         {
@@ -114,9 +115,9 @@ CoverResource::handleRequest(const Wt::Http::Request& request, Wt::Http::Respons
             return;
         }
 
-	response.setMimeType(std::string {cover->getMimeType()});
+        response.setMimeType(std::string{ cover->getMimeType() });
 
-	response.out().write(reinterpret_cast<const char *>(cover->getData()), cover->getDataSize());
+        response.out().write(reinterpret_cast<const char*>(cover->getData()), cover->getDataSize());
-}
+    }
 
 } // namespace UserInterface