AlbumTrack: Store artist as a foreign key to an actual Artist entity

Instead of storing a simple string

include/IAlbumTrack.h

@@ -41,7 +41,7 @@ class IAlbumTrack
          * "Artist 1 featuring Artist 2 and also artist 3 and a whole bunch of people"
          * @return
          */
-        virtual const std::string& artist() const = 0;
+        virtual ArtistPtr artist() const = 0;
         virtual const std::string& genre() = 0;
         virtual unsigned int trackNumber() = 0;
         virtual std::shared_ptr<IAlbum> album() = 0;

src/AlbumTrack.cpp

@@ -22,6 +22,7 @@
 
 #include "AlbumTrack.h"
 #include "Album.h"
+#include "Artist.h"
 #include "Media.h"
 #include "database/SqliteTools.h"
 #include "logging/Logger.h"
@@ -35,7 +36,7 @@ AlbumTrack::AlbumTrack(DBConnection dbConnection, sqlite::Row& row )
 {
     row >> m_id
         >> m_mediaId
-        >> m_artist
+        >> m_artistId
         >> m_genre
         >> m_trackNumber
         >> m_albumId
@@ -61,19 +62,25 @@ unsigned int AlbumTrack::id() const
     return m_id;
 }
 
-const std::string&AlbumTrack::artist() const
+ArtistPtr AlbumTrack::artist() const
 {
-    return m_artist;
+    auto lock = m_artist.lock();
+    if ( m_artist.isCached() == false && m_artistId != 0 )
+    {
+        m_artist = Artist::fetch( m_dbConnection, m_artistId );
+    }
+    return m_artist.get();
 }
 
-bool AlbumTrack::setArtist( const std::string& artist )
+bool AlbumTrack::setArtist( std::shared_ptr<Artist> artist )
 {
     static const std::string req = "UPDATE " + policy::AlbumTrackTable::Name +
-            " SET artist = ? WHERE id_track = ?";
-    if ( artist == m_artist )
+            " SET artist_id = ? WHERE id_track = ?";
+    if ( artist->id() == m_artistId )
         return true;
-    if ( sqlite::Tools::executeUpdate( m_dbConnection, req, artist, m_id ) == false )
+    if ( sqlite::Tools::executeUpdate( m_dbConnection, req, artist->id(), m_id ) == false )
         return false;
+    m_artistId = artist->id();
     m_artist = artist;
     return true;
 }
@@ -83,7 +90,7 @@ bool AlbumTrack::createTable( DBConnection dbConnection )
     static const std::string req = "CREATE TABLE IF NOT EXISTS " + policy::AlbumTrackTable::Name + "("
                 "id_track INTEGER PRIMARY KEY AUTOINCREMENT,"
                 "media_id INTEGER,"
-                "artist TEXT,"
+                "artist_id UNSIGNED INTEGER,"
                 "genre TEXT,"
                 "track_number UNSIGNED INTEGER,"
                 "album_id UNSIGNED INTEGER NOT NULL,"
@@ -91,7 +98,9 @@ bool AlbumTrack::createTable( DBConnection dbConnection )
                 "disc_number UNSIGNED INTEGER,"
                 "is_present BOOLEAN NOT NULL DEFAULT 1,"
                 "FOREIGN KEY (media_id) REFERENCES " + policy::MediaTable::Name + "(id_media)"
-                    " ON DELETE CASCADE, "
+                    " ON DELETE CASCADE,"
+                "FOREIGN KEY (artist_id) REFERENCES " + policy::ArtistTable::Name + "(id_artist)"
+                    " ON DELETE CASCADE,"
                 "FOREIGN KEY (album_id) REFERENCES Album(id_album) "
                     " ON DELETE CASCADE"
             ")";

src/AlbumTrack.h

@@ -29,9 +29,11 @@
 #include "IAlbumTrack.h"
 #include "IMediaLibrary.h"
 #include "database/DatabaseHelpers.h"
+#include "utils/Cache.h"
 
 class Album;
 class AlbumTrack;
+class Artist;
 class Media;
 
 namespace policy
@@ -51,8 +53,8 @@ class AlbumTrack : public IAlbumTrack, public DatabaseHelpers<AlbumTrack, policy
         AlbumTrack(Media* media, unsigned int trackNumber, unsigned int albumId , unsigned int discNumber);
 
         virtual unsigned int id() const override;
-        virtual const std::string& artist() const override;
-        bool setArtist( const std::string& artist );
+        virtual ArtistPtr artist() const override;
+        bool setArtist( std::shared_ptr<Artist> artist );
         virtual const std::string& genre() override;
         bool setGenre( const std::string& genre );
         virtual unsigned int trackNumber() override;
@@ -69,7 +71,7 @@ class AlbumTrack : public IAlbumTrack, public DatabaseHelpers<AlbumTrack, policy
         DBConnection m_dbConnection;
         unsigned int m_id;
         unsigned int m_mediaId;
-        std::string m_artist;
+        unsigned int m_artistId;
         std::string m_genre;
         unsigned int m_trackNumber;
         unsigned int m_albumId;
@@ -78,6 +80,7 @@ class AlbumTrack : public IAlbumTrack, public DatabaseHelpers<AlbumTrack, policy
         bool m_isPresent;
 
         std::weak_ptr<Album> m_album;
+        mutable Cache<std::shared_ptr<Artist>> m_artist;
 
         friend struct policy::AlbumTrackTable;
 };

test/unittest/AlbumTrackTests.cpp

@@ -51,16 +51,23 @@ TEST_F( AlbumTracks, Artist )
     auto f = ml->addFile( "track1.mp3" );
     auto track = album->addTrack( f, 1, 0 );
 
-    ASSERT_EQ( track->artist(), "" );
-    track->setArtist( "artist" );
-    ASSERT_EQ( track->artist(), "artist" );
+    auto artist = track->artist();
+    ASSERT_EQ( nullptr, artist );
+    auto newArtist = ml->createArtist( "Dream Seaotter" );
+    track->setArtist( newArtist );
+    artist = track->artist();
+    ASSERT_NE( nullptr, artist );
+    ASSERT_EQ( artist->name(), newArtist->name() );
 
     Reload();
 
     // Don't reuse the "track" and "f" variable, their type differ
     auto file = ml->media( f->id() );
     auto albumTrack = file->albumTrack();
-    ASSERT_EQ( albumTrack->artist(), "artist" );
+    artist = albumTrack->artist();
+    ASSERT_NE( nullptr, artist );
+    ASSERT_EQ( newArtist->name(), artist->name() );
+    ASSERT_EQ( newArtist->id(), artist->id() );
 }
 
 TEST_F( AlbumTracks, SetReleaseYear )