MediaLibrary: Expose an unknown artist getter

include/IMediaLibrary.h

@@ -102,6 +102,11 @@ class IMediaLibrary
         virtual MoviePtr movie( const std::string& title ) = 0;
         virtual MoviePtr createMovie( const std::string& title ) = 0;
         virtual ArtistPtr artist( const std::string& name ) = 0;
+        /**
+         * @brief unknownArtist returns a representation of a virtual "unknown" artist.
+         * If all tracks have an associated artist, this returns nullptr.
+         */
+        virtual ArtistPtr unknownArtist() = 0;
         virtual ArtistPtr createArtist( const std::string& name ) = 0;
         virtual std::vector<ArtistPtr> artists() const = 0;
 

src/AlbumTrack.cpp

@@ -142,12 +142,9 @@ bool AlbumTrack::destroy()
 bool AlbumTrack::addArtist( ArtistPtr artist )
 {
     static const std::string req = "INSERT INTO TrackArtistRelation VALUES(?, ?)";
-    if ( m_id == 0 || artist->id() == 0 )
-    {
-        LOG_ERROR("Both artist * album need to be inserted in database before being linked together" );
-        return false;
-    }
-    return sqlite::Tools::executeRequest( m_dbConnection, req, m_id, artist->id() );
+    // If track's ID is 0, the request will fail due to table constraints
+    sqlite::ForeignKey artistForeignKey( artist != nullptr ? artist->id() : 0 );
+    return sqlite::Tools::executeRequest( m_dbConnection, req, m_id, artistForeignKey );
 }
 
 std::vector<ArtistPtr> AlbumTrack::artists() const

src/Artist.cpp

@@ -45,6 +45,12 @@ Artist::Artist( const std::string& name )
 {
 }
 
+Artist::Artist(DBConnection dbConnection)
+    : m_dbConnection( dbConnection )
+    , m_id( 0 )
+{
+}
+
 unsigned int Artist::id() const
 {
     return m_id;
@@ -72,6 +78,8 @@ bool Artist::setShortBio(const std::string &shortBio)
 
 std::vector<AlbumPtr> Artist::albums() const
 {
+    if ( m_id == 0 )
+        return {};
     static const std::string req = "SELECT alb.* FROM " + policy::AlbumTable::Name + " alb "
             "LEFT JOIN AlbumArtistRelation aar ON aar.id_album = alb.id_album "
             "WHERE aar.id_artist = ?";
@@ -80,10 +88,22 @@ std::vector<AlbumPtr> Artist::albums() const
 
 std::vector<AlbumTrackPtr> Artist::tracks() const
 {
-    static const std::string req = "SELECT tra.* FROM " + policy::AlbumTrackTable::Name + " tra "
-            "LEFT JOIN TrackArtistRelation tar ON tar.id_track = tra.id_track "
-            "WHERE tar.id_artist = ?";
-    return AlbumTrack::fetchAll( m_dbConnection, req, m_id );
+    if ( m_id )
+    {
+        static const std::string req = "SELECT tra.* FROM " + policy::AlbumTrackTable::Name + " tra "
+                "LEFT JOIN TrackArtistRelation tar ON tar.id_track = tra.id_track "
+                "WHERE tar.id_artist = ?";
+        return AlbumTrack::fetchAll( m_dbConnection, req, m_id );
+    }
+    else
+    {
+        // Not being able to rely on ForeignKey here makes me a saaaaad panda...
+        // But sqlite only accepts "IS NULL" to compare against NULL...
+        static const std::string req = "SELECT tra.* FROM " + policy::AlbumTrackTable::Name + " tra "
+                "LEFT JOIN TrackArtistRelation tar ON tar.id_track = tra.id_track "
+                "WHERE tar.id_artist IS NULL";
+        return AlbumTrack::fetchAll( m_dbConnection, req, m_id );
+    }
 }
 
 bool Artist::createTable( DBConnection dbConnection )

src/Artist.h

@@ -46,6 +46,11 @@ private:
 public:
     Artist( DBConnection dbConnection, sqlite3_stmt* stmt );
     Artist( const std::string& name );
+    /**
+     * @brief Artist Construct an empty artist, with a DB connection.
+     * This is only meant to construct the Unknown Artist virtual representation
+     */
+    Artist( DBConnection dbConnection );
 
     virtual unsigned int id() const override;
     virtual const std::string &name() const override;

src/MediaLibrary.cpp

@@ -310,6 +310,19 @@ ArtistPtr MediaLibrary::artist(const std::string &name)
     return Artist::fetchOne( m_dbConnection.get(), req, name );
 }
 
+ArtistPtr MediaLibrary::unknownArtist()
+{
+    if ( m_unknownArtist != nullptr )
+        return m_unknownArtist;
+    static const std::string req = "SELECT id_track FROM TrackArtistRelation "
+            "WHERE id_artist IS NULL LIMIT 1";
+    if ( sqlite::Tools::hasResults( m_dbConnection.get(), req ) == true )
+    {
+        m_unknownArtist = std::make_shared<Artist>( m_dbConnection.get() );
+    }
+    return m_unknownArtist;
+}
+
 ArtistPtr MediaLibrary::createArtist( const std::string& name )
 {
     return Artist::create( m_dbConnection.get(), name );
@@ -333,6 +346,9 @@ void MediaLibrary::addMetadataService(std::unique_ptr<IMetadataService> service)
 
 void MediaLibrary::reload()
 {
+    // For the sake of simplicity, just flush the unknownArtist cache, regardless of any change
+    // FIXME: Replicate this for "live" change handling, once we have it.
+    m_unknownArtist = nullptr;
     m_discoverer->reload();
 }
 

src/MediaLibrary.h

@@ -68,6 +68,7 @@ class MediaLibrary : public IMediaLibrary
         virtual MoviePtr createMovie( const std::string& title ) override;
 
         virtual ArtistPtr artist( const std::string& name ) override;
+        virtual ArtistPtr unknownArtist() override;
         virtual ArtistPtr createArtist( const std::string& name ) override;
         virtual std::vector<ArtistPtr> artists() const override;
 
@@ -90,6 +91,9 @@ class MediaLibrary : public IMediaLibrary
         std::shared_ptr<factory::IFileSystem> m_fsFactory;
         std::string m_snapshotPath;
         IMediaLibraryCb* m_callback;
+        // Unknown artist "cache". If a track has been parsed with an unknown artist, this
+        // won't change (until we reload or detect a change)
+        ArtistPtr m_unknownArtist;
 
         // This probably qualifies as a work around, but we need to keep the VLC::Instance
         // alive to be able to use the logging wrapper lambda

src/database/SqliteTools.h

@@ -168,6 +168,20 @@ class Tools
             return T::load( dbConnection, stmt.get() );
         }
 
+        template <typename... Args>
+        static bool hasResults( DBConnection dbConnection, const std::string& req, Args&&... args )
+        {
+            auto ctx = dbConnection->acquireContext();
+
+            auto stmt = prepareRequest( dbConnection, req, std::forward<Args>( args )... );
+            if ( stmt == nullptr )
+                return false;
+            int res = sqlite3_step( stmt.get() );
+            if ( res != SQLITE_ROW )
+                return false;
+            return true;
+        }
+
         template <typename... Args>
         static bool executeRequest( DBConnection dbConnection, const std::string& req, Args&&... args )
         {

src/metadata_services/vlc/VLCMetadataService.cpp

@@ -237,5 +237,7 @@ bool VLCMetadataService::handleArtist( AlbumPtr album, AlbumTrackPtr track, VLC:
                 LOG_WARN( "Failed to add artist ", artist->name(), " to album ", album->title() );
         }
     }
+    else
+        track->addArtist( nullptr );
     return true;
 }

test/ArtistTests.cpp

@@ -24,6 +24,7 @@
 
 #include "IArtist.h"
 #include "IAlbum.h"
+#include "IAlbumTrack.h"
 
 class Artists : public Tests
 {
@@ -97,3 +98,30 @@ TEST_F( Artists, GetAll )
     artists = ml->artists();
     ASSERT_EQ( artists.size(), 5u );
 }
+
+TEST_F( Artists, UnknownArtist )
+{
+    // If no song has been added, unknown artist should be null.
+    auto a = ml->unknownArtist();
+    ASSERT_EQ( a, nullptr );
+
+    auto album = ml->createAlbum( "Rise of the otters" );
+    auto t = album->addTrack( "Otters: awakening", 1 );
+    // explicitely set the artist to nullptr (aka "unknown artist")
+    auto res = t->addArtist( nullptr );
+    ASSERT_EQ( res, true );
+
+    // Now, querying unknownArtist should give out some results.
+    a = ml->unknownArtist();
+    ASSERT_NE( a, nullptr );
+    auto tracks = a->tracks();
+    ASSERT_EQ( tracks.size(), 1u );
+
+    Reload();
+
+    // Check that unknown artist tracks listing persists in DB
+    a = ml->unknownArtist();
+    ASSERT_NE( a, nullptr );
+    tracks = a->tracks();
+    ASSERT_EQ( tracks.size(), 1u );
+}