Changed schema to allow multiple releases/artists to share the same i…

…mage
epoupon · Oct 4, 2024 · 55c7dea · 55c7dea
1 parent b5d6507
commit 55c7dea
Show file tree

Hide file tree

Showing 9 changed files with 133 additions and 24 deletions.
diff --git a/src/libs/database/impl/Artist.cpp b/src/libs/database/impl/Artist.cpp
@@ -277,7 +277,7 @@ namespace lms::db
 
     ObjectPtr<Image> Artist::getImage() const
     {
-        return ObjectPtr<Image>{ _image.lock() };
+        return ObjectPtr<Image>{ _image };
     }
 
     RangeResults<ArtistId> Artist::findSimilarArtistIds(core::EnumSet<TrackArtistLinkType> artistLinkTypes, std::optional<Range> range) const

diff --git a/src/libs/database/impl/Migration.cpp b/src/libs/database/impl/Migration.cpp
@@ -35,7 +35,7 @@ namespace lms::db
 {
     namespace
     {
-        static constexpr Version LMS_DATABASE_VERSION{ 68 };
+        static constexpr Version LMS_DATABASE_VERSION{ 69 };
     }
 
     VersionInfo::VersionInfo()
@@ -809,9 +809,72 @@ SELECT
         session.getDboSession()->execute("UPDATE scan_settings SET scan_version = scan_version + 1");
     }
 
+    void migrateFromV68(Session& session)
+    {
+        // Changed the way we ref images from release and artists (several releases and artist can now share the same image)
+        session.getDboSession()->execute(R"(CREATE TABLE "release_backup" (
+"id" integer primary key autoincrement,
+"version" integer not null,
+"name" text not null,
+"sort_name" text not null,
+"mbid" text not null,
+"group_mbid" text not null,
+"total_disc" integer,
+"artist_display_name" text not null,
+"is_compilation" boolean not null,
+"image_id" bigint,
+constraint "fk_release_image" foreign key ("image_id") references "image" ("id") on delete set null deferrable initially deferred))");
+
+        // Migrate data, with the new image_id field set to null
+        session.getDboSession()->execute(R"(INSERT INTO release_backup 
+SELECT
+ id,
+ version,
+ name,
+ sort_name,
+ mbid,
+ group_mbid,
+ total_disc,
+ artist_display_name,
+ is_compilation,
+ NULL
+ FROM release
+ )");
+        session.getDboSession()->execute("DROP TABLE release");
+        session.getDboSession()->execute("ALTER TABLE release_backup RENAME TO release");
+
+        session.getDboSession()->execute(R"(CREATE TABLE IF NOT EXISTS "artist_backup" (
+  "id" integer primary key autoincrement,
+  "version" integer not null,
+  "name" text not null,
+  "sort_name" text not null,
+  "mbid" text not null,
+  "image_id" bigint,
+  constraint "fk_artist_image" foreign key ("image_id") references "image" ("id") on delete set null deferrable initially deferred
+))");
+
+        // Migrate data, with the new image_id field set to null
+        session.getDboSession()->execute(R"(INSERT INTO artist_backup 
+SELECT
+ id,
+ version,
+ name,
+ sort_name,
+ mbid,
+ NULL
+ FROM artist
+ )");
+
+        session.getDboSession()->execute("DROP TABLE artist");
+        session.getDboSession()->execute("ALTER TABLE artist_backup RENAME TO artist");
+
+        // Just increment the scan version of the settings to make the next scheduled scan rescan everything
+        session.getDboSession()->execute("UPDATE scan_settings SET scan_version = scan_version + 1");
+    }
+
     bool doDbMigration(Session& session)
     {
-        static const std::string outdatedMsg{ "Outdated database, please rebuild it (delete the .db file and restart)" };
+        constexpr std::string_view outdatedMsg{ "Outdated database, please rebuild it (delete the .db file and restart)" };
 
         ScopedNoForeignKeys noPragmaKeys{ session.getDb() };
 
@@ -853,6 +916,7 @@ SELECT
             { 65, migrateFromV65 },
             { 66, migrateFromV66 },
             { 67, migrateFromV67 },
+            { 68, migrateFromV68 },
         };
 
         bool migrationPerformed{};

diff --git a/src/libs/database/impl/Release.cpp b/src/libs/database/impl/Release.cpp
@@ -572,7 +572,7 @@ namespace lms::db
 
     ObjectPtr<Image> Release::getImage() const
     {
-        return ObjectPtr<Image>{ _image.lock() };
+        return ObjectPtr<Image>{ _image };
     }
 
     void Release::clearLabels()

diff --git a/src/libs/database/impl/Session.cpp b/src/libs/database/impl/Session.cpp
@@ -179,6 +179,7 @@ namespace lms::db
 
         auto transaction{ createWriteTransaction() };
         _session.execute("CREATE INDEX IF NOT EXISTS artist_id_idx ON artist(id)");
+        _session.execute("CREATE INDEX IF NOT EXISTS artist_image_idx ON artist(image_id)");
         _session.execute("CREATE INDEX IF NOT EXISTS artist_name_idx ON artist(name)");
         _session.execute("CREATE INDEX IF NOT EXISTS artist_sort_name_nocase_idx ON artist(sort_name COLLATE NOCASE)");
         _session.execute("CREATE INDEX IF NOT EXISTS artist_mbid_idx ON artist(mbid)");
@@ -195,11 +196,9 @@ namespace lms::db
         _session.execute("CREATE INDEX IF NOT EXISTS directory_path_idx ON directory(absolute_path)");
         _session.execute("CREATE INDEX IF NOT EXISTS directory_media_library_idx ON directory(media_library_id)");
 
-        _session.execute("CREATE INDEX IF NOT EXISTS image_artist_idx ON image(artist_id)");
         _session.execute("CREATE INDEX IF NOT EXISTS image_directory_stem_idx ON image(directory_id, stem COLLATE NOCASE)");
         _session.execute("CREATE INDEX IF NOT EXISTS image_id_idx ON image(id)");
         _session.execute("CREATE INDEX IF NOT EXISTS image_path_idx ON image(absolute_file_path)");
-        _session.execute("CREATE INDEX IF NOT EXISTS image_release_idx ON image(release_id)");
         _session.execute("CREATE INDEX IF NOT EXISTS image_stem_idx ON image(stem COLLATE NOCASE)");
 
         _session.execute("CREATE INDEX IF NOT EXISTS label_name_idx ON label(name)");
@@ -218,6 +217,7 @@ namespace lms::db
         _session.execute("CREATE INDEX IF NOT EXISTS rated_track_user_track_idx ON rated_track(user_id,track_id)");
 
         _session.execute("CREATE INDEX IF NOT EXISTS release_id_idx ON release(id)");
+        _session.execute("CREATE INDEX IF NOT EXISTS release_image_idx ON release(image_id)");
         _session.execute("CREATE INDEX IF NOT EXISTS release_mbid_idx ON release(mbid)");
         _session.execute("CREATE INDEX IF NOT EXISTS release_name_idx ON release(name)");
         _session.execute("CREATE INDEX IF NOT EXISTS release_name_nocase_idx ON release(name COLLATE NOCASE)");

diff --git a/src/libs/database/include/database/Artist.hpp b/src/libs/database/include/database/Artist.hpp
@@ -162,7 +162,7 @@ namespace lms::db
             Wt::Dbo::field(a, _sortName, "sort_name");
             Wt::Dbo::field(a, _MBID, "mbid");
 
-            Wt::Dbo::hasOne(a, _image, "artist");
+            Wt::Dbo::belongsTo(a, _image, "image", Wt::Dbo::OnDeleteSetNull);
             Wt::Dbo::hasMany(a, _trackArtistLinks, Wt::Dbo::ManyToOne, "artist");
             Wt::Dbo::hasMany(a, _starredArtists, Wt::Dbo::ManyToMany, "user_starred_artists", "", Wt::Dbo::OnDeleteCascade);
         }
@@ -179,7 +179,7 @@ namespace lms::db
         std::string _sortName;
         std::string _MBID; // Musicbrainz Identifier
 
-        Wt::Dbo::weak_ptr<Image> _image;
+        Wt::Dbo::ptr<Image> _image;
         Wt::Dbo::collection<Wt::Dbo::ptr<TrackArtistLink>> _trackArtistLinks; // Tracks involving this artist
         Wt::Dbo::collection<Wt::Dbo::ptr<StarredArtist>> _starredArtists;     // starred entries for this artist
     };

diff --git a/src/libs/database/include/database/Image.hpp b/src/libs/database/include/database/Image.hpp
@@ -87,16 +87,6 @@ namespace lms::db
         void setFileSize(std::size_t fileSize) { _fileSize = fileSize; }
         void setWidth(std::size_t width) { _width = width; }
         void setHeight(std::size_t height) { _height = height; }
-        void setArtist(const ObjectPtr<Artist>& artist)
-        {
-            _artist = getDboPtr(artist);
-            _release = nullptr;
-        }
-        void setRelease(const ObjectPtr<Release>& release)
-        {
-            _release = getDboPtr(release);
-            _artist = nullptr;
-        }
         void setDirectory(const ObjectPtr<Directory>& directory) { _directory = getDboPtr(directory); }
 
         template<class Action>
@@ -110,8 +100,8 @@ namespace lms::db
             Wt::Dbo::field(a, _width, "width");
             Wt::Dbo::field(a, _height, "height");
 
-            Wt::Dbo::belongsTo(a, _artist, "artist", Wt::Dbo::OnDeleteCascade);
-            Wt::Dbo::belongsTo(a, _release, "release", Wt::Dbo::OnDeleteCascade);
+            Wt::Dbo::hasMany(a, _artists, Wt::Dbo::ManyToOne, "image");
+            Wt::Dbo::hasMany(a, _releases, Wt::Dbo::ManyToOne, "image");
             Wt::Dbo::belongsTo(a, _directory, "directory", Wt::Dbo::OnDeleteCascade);
         }
 
@@ -127,8 +117,8 @@ namespace lms::db
         int _width{};
         int _height{};
 
-        Wt::Dbo::ptr<Artist> _artist;
-        Wt::Dbo::ptr<Release> _release;
+        Wt::Dbo::collection<Wt::Dbo::ptr<Artist>> _artists;
+        Wt::Dbo::collection<Wt::Dbo::ptr<Release>> _releases;
         Wt::Dbo::ptr<Directory> _directory;
     };
 } // namespace lms::db
diff --git a/src/libs/database/include/database/Release.hpp b/src/libs/database/include/database/Release.hpp
@@ -287,7 +287,7 @@ namespace lms::db
             Wt::Dbo::field(a, _isCompilation, "is_compilation");
             Wt::Dbo::hasMany(a, _tracks, Wt::Dbo::ManyToOne, "release");
 
-            Wt::Dbo::hasOne(a, _image, "release");
+            Wt::Dbo::belongsTo(a, _image, "image", Wt::Dbo::OnDeleteSetNull);
             Wt::Dbo::hasMany(a, _labels, Wt::Dbo::ManyToMany, "release_label", "", Wt::Dbo::OnDeleteCascade);
             Wt::Dbo::hasMany(a, _releaseTypes, Wt::Dbo::ManyToMany, "release_release_type", "", Wt::Dbo::OnDeleteCascade);
         }
@@ -310,7 +310,7 @@ namespace lms::db
         std::string _artistDisplayName;
         bool _isCompilation{}; // See https://picard-docs.musicbrainz.org/en/appendices/tag_mapping.html#compilation-itunes-5
 
-        Wt::Dbo::weak_ptr<Image> _image;
+        Wt::Dbo::ptr<Image> _image;
         Wt::Dbo::collection<Wt::Dbo::ptr<Track>> _tracks;
         Wt::Dbo::collection<Wt::Dbo::ptr<Label>> _labels;
         Wt::Dbo::collection<Wt::Dbo::ptr<ReleaseType>> _releaseTypes;

diff --git a/src/libs/database/test/Artist.cpp b/src/libs/database/test/Artist.cpp
@@ -19,8 +19,12 @@
 
 #include "Common.hpp"
 
+#include "database/Image.hpp"
+
 namespace lms::db::tests
 {
+    using ScopedImage = ScopedEntity<db::Image>;
+
     TEST_F(DatabaseFixture, Artist)
     {
         {
@@ -697,4 +701,28 @@ namespace lms::db::tests
             EXPECT_EQ(artists.results.front(), artist.getId());
         }
     }
+
+    TEST_F(DatabaseFixture, Artist_image)
+    {
+        ScopedArtist release{ session, "MyArtist" };
+
+        {
+            auto transaction{ session.createReadTransaction() };
+            EXPECT_FALSE(release.get()->getImage());
+        }
+
+        ScopedImage image{ session, "/myImage" };
+
+        {
+            auto transaction{ session.createWriteTransaction() };
+            release.get().modify()->setImage(image.get());
+        }
+
+        {
+            auto transaction{ session.createReadTransaction() };
+            auto artistImage(release.get()->getImage());
+            ASSERT_TRUE(artistImage);
+            EXPECT_EQ(artistImage->getId(), image.getId());
+        }
+    }
 } // namespace lms::db::tests
diff --git a/src/libs/database/test/Release.cpp b/src/libs/database/test/Release.cpp
@@ -19,8 +19,11 @@
 
 #include "Common.hpp"
 
+#include "database/Image.hpp"
+
 namespace lms::db::tests
 {
+    using ScopedImage = ScopedEntity<db::Image>;
     using ScopedLabel = ScopedEntity<db::Label>;
     using ScopedReleaseType = ScopedEntity<db::ReleaseType>;
 
@@ -1038,4 +1041,28 @@ namespace lms::db::tests
             EXPECT_EQ(release3->getTrackCount(), 0);
         }
     }
+
+    TEST_F(DatabaseFixture, Release_image)
+    {
+        ScopedRelease release{ session, "MyRelease" };
+
+        {
+            auto transaction{ session.createReadTransaction() };
+            EXPECT_FALSE(release.get()->getImage());
+        }
+
+        ScopedImage image{ session, "/myImage" };
+
+        {
+            auto transaction{ session.createWriteTransaction() };
+            release.get().modify()->setImage(image.get());
+        }
+
+        {
+            auto transaction{ session.createReadTransaction() };
+            auto releaseImage(release.get()->getImage());
+            ASSERT_TRUE(releaseImage);
+            EXPECT_EQ(releaseImage->getId(), image.getId());
+        }
+    }
 } // namespace lms::db::tests