Simplified artwork handling: create a brand new artwork when an image changed. Simplifies subsonic element retrievals

src/libs/database/impl/objects/Artwork.cpp

@@ -20,7 +20,6 @@
 #include "database/objects/Artwork.hpp"
 
 #include <Wt/Dbo/Impl.h>
-#include <Wt/Dbo/WtSqlTraits.h>
 
 #include "database/Session.hpp"
 #include "database/objects/Image.hpp"
@@ -92,18 +91,6 @@ namespace lms::db
         return res;
     }
 
-    Wt::WDateTime Artwork::getLastWrittenTime() const
-    {
-        auto query{ session()->query<Wt::WDateTime>("SELECT MAX(COALESCE(image.file_last_write, track.file_last_write)) AS last_written_datetime FROM artwork") };
-        query.leftJoin("image ON artwork.image_id = image.id");
-        query.leftJoin("track_embedded_image ON artwork.track_embedded_image_id = track_embedded_image.id");
-        query.leftJoin("track_embedded_image_link ON track_embedded_image.id = track_embedded_image_link.track_embedded_image_id");
-        query.leftJoin("track ON track.id = track_embedded_image_link.track_id");
-        query.where("artwork.id = ?").bind(getId());
-
-        return utils::fetchQuerySingleResult(query);
-    }
-
     std::filesystem::path Artwork::getAbsoluteFilePath() const
     {
         auto query{ session()->query<std::filesystem::path>("SELECT COALESCE(image.absolute_file_path, track.absolute_file_path) AS absolute_file_path FROM artwork") };

src/libs/database/include/database/objects/Artwork.hpp

@@ -23,7 +23,6 @@
 #include <variant>
 
 #include <Wt/Dbo/Field.h>
-#include <Wt/WDateTime.h>
 
 #include "database/Object.hpp"
 #include "database/objects/ArtworkId.hpp"
@@ -50,7 +49,6 @@ namespace lms::db
         // getters
         using UnderlyingId = std::variant<std::monostate, TrackEmbeddedImageId, ImageId>;
         UnderlyingId getUnderlyingId() const;
-        Wt::WDateTime getLastWrittenTime() const;
         std::filesystem::path getAbsoluteFilePath() const;
         ObjectPtr<Image> getImage() const;
         ImageId getImageId() const;
@@ -68,7 +66,6 @@ namespace lms::db
         Artwork(ObjectPtr<Image> image);
         static pointer create(Session& session, ObjectPtr<TrackEmbeddedImage> trackEmbeddedImage);
         static pointer create(Session& session, ObjectPtr<Image> image);
-
         Wt::Dbo::ptr<TrackEmbeddedImage> _trackEmbeddedImage;
         Wt::Dbo::ptr<Image> _image;
     };

src/libs/database/test/Artwork.cpp

@@ -36,24 +36,15 @@ namespace lms::db::tests
         }
 
         ScopedImage image{ session, "/MyImage" };
+        {
+            auto transaction{ session.createWriteTransaction() };
+            image.get().modify()->setAbsoluteFilePath("/tmp/foo");
+        }
         ScopedArtwork artwork{ session, image.lockAndGet() };
 
-        const Wt::WDateTime dateTime{ Wt::WDate{ 2025, 1, 1 } };
-
         {
             auto transaction{ session.createReadTransaction() };
             EXPECT_EQ(Artwork::getCount(session), 1);
-        }
-
-        {
-            auto transaction{ session.createWriteTransaction() };
-            image.get().modify()->setLastWriteTime(dateTime);
-            image.get().modify()->setAbsoluteFilePath("/tmp/foo");
-        }
-
-        {
-            auto transaction{ session.createReadTransaction() };
-            EXPECT_EQ(artwork.get()->getLastWrittenTime(), dateTime);
             EXPECT_EQ(artwork.get()->getAbsoluteFilePath(), "/tmp/foo");
         }
     }
@@ -67,20 +58,15 @@ namespace lms::db::tests
 
         ScopedTrackEmbeddedImage image{ session };
         ScopedTrack track{ session };
-        ScopedArtwork artwork{ session, image.lockAndGet() };
-
-        const Wt::WDateTime dateTime{ Wt::WDate{ 2025, 1, 1 } };
-
         {
             auto transaction{ session.createWriteTransaction() };
             session.create<db::TrackEmbeddedImageLink>(track.get(), image.get());
-            track.get().modify()->setLastWriteTime(dateTime);
             track.get().modify()->setAbsoluteFilePath("/tmp/foo");
         }
+        ScopedArtwork artwork{ session, image.lockAndGet() };
 
         {
             auto transaction{ session.createReadTransaction() };
-            EXPECT_EQ(artwork.get()->getLastWrittenTime(), dateTime);
             EXPECT_EQ(artwork.get()->getAbsoluteFilePath(), "/tmp/foo");
         }
     }

src/libs/services/scanner/impl/scanners/ImageFileScanner.cpp

@@ -85,11 +85,20 @@ namespace lms::scanner
             }
 
             const bool added{ !image };
-            if (!image)
+            if (added)
             {
                 image = dbSession.create<db::Image>(getFilePath());
                 dbSession.create<db::Artwork>(image);
             }
+            else if (image->getLastWriteTime() != getLastWriteTime()
+                     || image->getFileSize() != getFileSize()
+                     || image->getHeight() != _parsedImageProperties->height
+                     || image->getWidth() != _parsedImageProperties->width)
+            {
+                if (auto artwork{ db::Artwork::find(dbSession, image->getId()) })
+                    artwork.remove();
+                dbSession.create<db::Artwork>(image);
+            }
 
             image.modify()->setLastWriteTime(getLastWriteTime());
             image.modify()->setFileSize(getFileSize());

src/libs/subsonic/CMakeLists.txt

@@ -34,7 +34,6 @@ add_library(lmssubsonic STATIC
 	impl/responses/ReplayGain.cpp
 	impl/responses/Song.cpp
 	impl/responses/User.cpp
-	impl/CoverArtId.cpp
 	impl/RequestContext.cpp
 	impl/ResponseFormat.cpp
 	impl/ProtocolVersion.cpp

src/libs/subsonic/impl/CoverArtId.cpp

@@ -1,79 +0,0 @@
-/*
- * 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 <cassert>
-
-#include "core/String.hpp"
-#include "database/objects/ArtworkId.hpp"
-
-namespace lms::api::subsonic
-{
-    namespace
-    {
-        constexpr char timestampSeparatorChar{ '-' };
-
-        std::string idToString(db::ArtworkId id)
-        {
-            return "art-" + id.toString();
-        }
-    } // namespace
-
-    std::string idToString(CoverArtId coverId)
-    {
-        std::string res;
-
-        // produce "art-id-timestamp"
-        res = idToString(coverId.id);
-        res += timestampSeparatorChar;
-        res += std::to_string(coverId.timestamp);
-
-        assert(!res.empty());
-        return res;
-    }
-} // namespace lms::api::subsonic
-
-// Used to parse parameters
-namespace lms::core::stringUtils
-{
-    template<>
-    std::optional<api::subsonic::CoverArtId> readAs(std::string_view str)
-    {
-        std::optional<api::subsonic::CoverArtId> res;
-
-        std::vector<std::string_view> values{ core::stringUtils::splitString(str, '-') };
-
-        // expect "art-id-timestamp"
-        if (values.size() != 3)
-            return res;
-
-        if (values[0] != "art")
-            return res;
-
-        const auto value{ core::stringUtils::readAs<db::ArtworkId::ValueType>(values[1]) };
-        const auto timestamp{ core::stringUtils::readAs<std::time_t>(values[2]) };
-        if (!value || !timestamp)
-            return std::nullopt;
-
-        res.emplace(api::subsonic::CoverArtId{ *value, *timestamp });
-
-        return res;
-    }
-} // namespace lms::core::stringUtils

src/libs/subsonic/impl/CoverArtId.hpp

@@ -1,43 +0,0 @@
-/*
- * 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 "core/String.hpp"
-#include "database/objects/ArtworkId.hpp"
-
-namespace lms::api::subsonic
-{
-    struct CoverArtId
-    {
-        db::ArtworkId id;
-        std::time_t timestamp;
-    };
-
-    std::string idToString(CoverArtId coverId);
-} // namespace lms::api::subsonic
-
-// Used to parse parameters
-namespace lms::core::stringUtils
-{
-    template<>
-    std::optional<api::subsonic::CoverArtId> readAs(std::string_view str);
-} // namespace lms::core::stringUtils

src/libs/subsonic/impl/SubsonicId.cpp

@@ -26,6 +26,11 @@ namespace lms::api::subsonic
         return "ar-" + id.toString();
     }
 
+    std::string idToString(db::ArtworkId id)
+    {
+        return "art-" + id.toString();
+    }
+
     std::string idToString(db::DirectoryId id)
     {
         return "dir-" + id.toString();
@@ -59,6 +64,19 @@ namespace lms::api::subsonic
 
 namespace lms::core::stringUtils
 {
+    template<>
+    std::optional<db::ArtworkId> readAs(std::string_view str)
+    {
+        std::vector<std::string_view> values{ core::stringUtils::splitString(str, '-') };
+        if (values.size() != 2 || values[0] != "art")
+            return std::nullopt;
+
+        if (const auto value{ core::stringUtils::readAs<db::ArtworkId::ValueType>(values[1]) })
+            return db::ArtworkId{ *value };
+
+        return std::nullopt;
+    }
+
     template<>
     std::optional<db::ArtistId> readAs(std::string_view str)
     {

src/libs/subsonic/impl/SubsonicId.hpp

@@ -20,7 +20,9 @@
 #pragma once
 
 #include "core/String.hpp"
+
 #include "database/objects/ArtistId.hpp"
+#include "database/objects/ArtworkId.hpp"
 #include "database/objects/DirectoryId.hpp"
 #include "database/objects/MediaLibraryId.hpp"
 #include "database/objects/PodcastEpisodeId.hpp"
@@ -32,6 +34,7 @@
 namespace lms::api::subsonic
 {
     std::string idToString(db::ArtistId id);
+    std::string idToString(db::ArtworkId id);
     std::string idToString(db::DirectoryId id);
     std::string idToString(db::PodcastEpisodeId id);
     std::string idToString(db::PodcastId id);
@@ -46,6 +49,9 @@ namespace lms::core::stringUtils
     template<>
     std::optional<db::ArtistId> readAs(std::string_view str);
 
+    template<>
+    std::optional<db::ArtworkId> readAs(std::string_view str);
+
     template<>
     std::optional<db::DirectoryId> readAs(std::string_view str);
 

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

@@ -39,7 +39,6 @@
 #include "services/artwork/IArtworkService.hpp"
 #include "services/transcoding/ITranscodeService.hpp"
 
-#include "CoverArtId.hpp"
 #include "ParameterParsing.hpp"
 #include "RequestContext.hpp"
 #include "SubsonicId.hpp"
@@ -359,13 +358,13 @@ namespace lms::api::subsonic
     void handleGetCoverArt(RequestContext& context, const Wt::Http::Request& /*request*/, Wt::Http::Response& response)
     {
         // Mandatory params
-        const CoverArtId coverArtId{ getMandatoryParameterAs<CoverArtId>(context.getParameters(), "id") };
+        const db::ArtworkId artworkId{ getMandatoryParameterAs<db::ArtworkId>(context.getParameters(), "id") };
 
         std::optional<std::size_t> size{ getParameterAs<std::size_t>(context.getParameters(), "size") };
         if (size)
             *size = std::clamp(*size, std::size_t{ 32 }, std::size_t{ 2048 });
 
-        std::shared_ptr<image::IEncodedImage> image{ core::Service<artwork::IArtworkService>::get()->getImage(coverArtId.id, size) };
+        std::shared_ptr<image::IEncodedImage> image{ core::Service<artwork::IArtworkService>::get()->getImage(artworkId, size) };
         if (!image)
         {
             response.setStatus(404);

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

@@ -24,7 +24,6 @@
 #include "core/String.hpp"
 #include "database/Types.hpp"
 #include "database/objects/Artist.hpp"
-#include "database/objects/Artwork.hpp"
 #include "database/objects/Directory.hpp"
 #include "database/objects/Genre.hpp"
 #include "database/objects/Grouping.hpp"
@@ -37,7 +36,6 @@
 #include "services/feedback/IFeedbackService.hpp"
 #include "services/scrobbling/IScrobblingService.hpp"
 
-#include "CoverArtId.hpp"
 #include "RequestContext.hpp"
 #include "SubsonicId.hpp"
 #include "responses/Artist.hpp"
@@ -91,11 +89,8 @@ namespace lms::api::subsonic
 
         albumNode.setAttribute("created", core::stringUtils::toISO8601String(release->getAddedTime()));
 
-        if (const auto artwork{ release->getPreferredArtwork() })
+        if (const db::ArtworkId artworkId{ release->getPreferredArtworkId() }; artworkId.isValid())
-        {
+            albumNode.setAttribute("coverArt", idToString(artworkId));
-            CoverArtId coverArtId{ artwork->getId(), artwork->getLastWrittenTime().toTime_t() };
-            albumNode.setAttribute("coverArt", idToString(coverArtId));
-        }
 
         if (const auto originalYear{ release->getOriginalYear() })
             albumNode.setAttribute("year", *originalYear);

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

@@ -24,14 +24,12 @@
 #include "core/String.hpp"
 
 #include "database/objects/Artist.hpp"
-#include "database/objects/Artwork.hpp"
 #include "database/objects/Release.hpp"
 #include "database/objects/ReleaseArtistLink.hpp"
 #include "database/objects/TrackArtistLink.hpp"
 #include "database/objects/User.hpp"
 #include "services/feedback/IFeedbackService.hpp"
 
-#include "CoverArtId.hpp"
 #include "RequestContext.hpp"
 #include "SubsonicId.hpp"
 
@@ -69,11 +67,8 @@ namespace lms::api::subsonic
 
         Response::Node artistNode{ createMinimalArtistNode(artist) };
 
-        if (const auto artwork{ artist->getPreferredArtwork() })
+        if (const db::ArtworkId artworkId{ artist->getPreferredArtworkId() }; artworkId.isValid())
-        {
+            artistNode.setAttribute("coverArt", idToString(artworkId));
-            CoverArtId coverArtId{ artwork->getId(), artwork->getLastWrittenTime().toTime_t() };
-            artistNode.setAttribute("coverArt", idToString(coverArtId));
-        }
 
         bool hasAlbums{};
         {

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

@@ -19,10 +19,9 @@
 
 #include "responses/DiscTitle.hpp"
 
-#include "database/objects/Artwork.hpp"
 #include "database/objects/Medium.hpp"
 
-#include "CoverArtId.hpp"
+#include "SubsonicId.hpp"
 
 namespace lms::api::subsonic
 {
@@ -32,11 +31,8 @@ namespace lms::api::subsonic
 
         discTitleNode.setAttribute("disc", medium->getPosition() ? *medium->getPosition() : 0);
         discTitleNode.setAttribute("title", medium->getName());
-        if (const auto artwork{ medium->getPreferredArtwork() })
+        if (const db::ArtworkId artworkId{ medium->getPreferredArtworkId() }; artworkId.isValid())
-        {
+            discTitleNode.setAttribute("coverArt", idToString(artworkId));
-            CoverArtId coverArtId{ artwork->getId(), artwork->getLastWrittenTime().toTime_t() };
-            discTitleNode.setAttribute("coverArt", idToString(coverArtId));
-        }
 
         return discTitleNode;
     }

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

@@ -21,12 +21,10 @@
 
 #include "core/Service.hpp"
 #include "core/String.hpp"
-#include "database/objects/Artwork.hpp"
 #include "database/objects/TrackList.hpp"
 #include "database/objects/User.hpp"
 #include "services/artwork/IArtworkService.hpp"
 
-#include "CoverArtId.hpp"
 #include "RequestContext.hpp"
 #include "SubsonicId.hpp"
 
@@ -49,13 +47,7 @@ namespace lms::api::subsonic
             playlistNode.setAttribute("owner", trackListUser->getLoginName());
 
         if (const db::ArtworkId artworkId{ core::Service<artwork::IArtworkService>::get()->findTrackListImage(tracklist->getId()) }; artworkId.isValid())
-        {
+            playlistNode.setAttribute("coverArt", idToString(artworkId));
-            if (const auto artwork{ db::Artwork::find(context.getDbSession(), artworkId) })
-            {
-                CoverArtId coverArtId{ artwork->getId(), artwork->getLastWrittenTime().toTime_t() };
-                playlistNode.setAttribute("coverArt", idToString(coverArtId));
-            }
-        }
 
         if (context.isOpenSubsonicEnabled())
         {

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

@@ -24,11 +24,9 @@
 #include <Wt/WDate.h>
 
 #include "core/String.hpp"
-#include "database/objects/Artwork.hpp"
 #include "database/objects/Podcast.hpp"
 #include "database/objects/PodcastEpisode.hpp"
 
-#include "CoverArtId.hpp"
 #include "RequestContext.hpp"
 #include "SubsonicId.hpp"
 
@@ -69,11 +67,8 @@ namespace lms::api::subsonic
         // estimated bitrate
         if (episode->getEnclosureLength() > 0 && episode->getDuration() > std::chrono::milliseconds::zero())
             episodeNode.setAttribute("bitrate", episode->getEnclosureLength() * 8 / std::chrono::duration_cast<std::chrono::milliseconds>(episode->getDuration()).count());
-        if (const auto artwork{ episode->getArtwork() })
+        if (const db::ArtworkId artworkId{ episode->getArtworkId() }; artworkId.isValid())
-        {
+            episodeNode.setAttribute("coverArt", idToString(artworkId));
-            CoverArtId coverArtId{ artwork->getId(), artwork->getLastWrittenTime().toTime_t() };
-            episodeNode.setAttribute("coverArt", idToString(coverArtId));
-        }
 
         // Podcast specific attributes
         // Expose the streamId only if the episode is actually downloaded
@@ -111,11 +106,8 @@ namespace lms::api::subsonic
 
         podcastNode.setAttribute("status", getStatus(podcast));
 
-        if (const auto artwork{ podcast->getArtwork() })
+        if (const db::ArtworkId artworkId{ podcast->getArtworkId() }; artworkId.isValid())
-        {
+            podcastNode.setAttribute("coverArt", idToString(artworkId));
-            CoverArtId coverArtId{ artwork->getId(), artwork->getLastWrittenTime().toTime_t() };
-            podcastNode.setAttribute("coverArt", idToString(coverArtId));
-        }
 
         if (includeEpisodes)
         {

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

@@ -30,7 +30,6 @@
 
 #include "database/Types.hpp"
 #include "database/objects/Artist.hpp"
-#include "database/objects/Artwork.hpp"
 #include "database/objects/Directory.hpp"
 #include "database/objects/Genre.hpp"
 #include "database/objects/Grouping.hpp"
@@ -45,7 +44,6 @@
 #include "services/feedback/IFeedbackService.hpp"
 #include "services/scrobbling/IScrobblingService.hpp"
 
-#include "CoverArtId.hpp"
 #include "RequestContext.hpp"
 #include "SubsonicId.hpp"
 #include "responses/Artist.hpp"
@@ -124,15 +122,9 @@ namespace lms::api::subsonic
             trackResponse.setAttribute("transcodedContentType", core::getMimeType(std::filesystem::path{ "." + fileSuffix }));
         }
 
-        auto artwork{ track->getPreferredMediaArtwork() };
+        const db::ArtworkId artworkId{ track->getPreferredMediaArtworkId().isValid() ? track->getPreferredMediaArtworkId() : track->getPreferredArtworkId() };
-        if (!artwork)
+        if (artworkId.isValid())
-            artwork = track->getPreferredArtwork();
+            trackResponse.setAttribute("coverArt", idToString(artworkId));
-
-        if (artwork)
-        {
-            CoverArtId coverArtId{ artwork->getId(), artwork->getLastWrittenTime().toTime_t() };
-            trackResponse.setAttribute("coverArt", idToString(coverArtId));
-        }
 
         std::vector<db::TrackArtistLink::pointer> artistLinks;
         std::vector<db::TrackArtistLink::pointer> trackArtistLinks;