Task: Move all item specific instances in the item

This is solely replacing direct members access with a pair of
getters/setters

src/metadata_services/MetadataParser.cpp

@@ -99,13 +99,13 @@ parser::Task::Status MetadataParser::run( parser::Task& task )
         if ( res == false ) // playlist addition may fail due to constraint violation
             return parser::Task::Status::Fatal;
 
-        assert( task.file != nullptr );
+        assert( task.item().file() != nullptr );
         task.markStepCompleted( parser::Task::ParserStep::Completed );
         task.saveParserStep();
         return parser::Task::Status::Success;
     }
 
-    if ( task.file == nullptr )
+    if ( task.item().file() == nullptr )
     {
         assert( task.item().media() == nullptr );
         // Try to create Media & File
@@ -121,10 +121,11 @@ parser::Task::Status MetadataParser::run( parser::Task& task )
                 return parser::Task::Status::Fatal;
             }
             // For now, assume all media are made of a single file
-            task.file = task.item().media()->addFile( *task.fileFs, task.parentFolder->id(),
-                                             task.parentFolderFs->device()->isRemovable(),
-                                             File::Type::Main );
-            if ( task.file == nullptr )
+            task.item().setFile( task.item().media()->addFile( *task.item().fileFs(),
+                                                               task.item().parentFolder()->id(),
+                                             task.item().parentFolderFs()->device()->isRemovable(),
+                                             File::Type::Main ) );
+            if ( task.item().file() == nullptr )
             {
                 LOG_ERROR( "Failed to add file ", mrl, " to media #", task.item().media()->id() );
                 return parser::Task::Status::Fatal;
@@ -144,7 +145,7 @@ parser::Task::Status MetadataParser::run( parser::Task& task )
                 LOG_ERROR( "File ", mrl, " no longer present in DB, aborting");
                 return parser::Task::Status::Fatal;
             }
-            task.file = fileInDB;
+            task.item().setFile( fileInDB );
             task.item().setMedia( fileInDB->media() );
             if ( task.item().media() == nullptr ) // Without a media, we cannot go further
                 return parser::Task::Status::Fatal;
@@ -161,8 +162,8 @@ parser::Task::Status MetadataParser::run( parser::Task& task )
         return parser::Task::Status::Fatal;
     }
 
-    if ( task.parentPlaylist != nullptr )
-        task.parentPlaylist->add( task.item().media()->id(), task.parentPlaylistIndex );
+    if ( task.item().parentPlaylist() != nullptr )
+        task.item().parentPlaylist()->add( task.item().media()->id(), task.item().parentPlaylistIndex() );
 
     if ( alreadyInParser == true )
     {
@@ -216,7 +217,7 @@ parser::Task::Status MetadataParser::run( parser::Task& task )
             return parser::Task::Status::Fatal;
     }
 
-    if ( task.file->isDeleted() == true || task.item().media()->isDeleted() == true )
+    if ( task.item().file()->isDeleted() == true || task.item().media()->isDeleted() == true )
         return parser::Task::Status::Fatal;
 
     task.markStepCompleted( parser::Task::ParserStep::MetadataAnalysis );
@@ -242,8 +243,10 @@ bool MetadataParser::addPlaylistMedias( parser::Task& task ) const
         LOG_ERROR( "Failed to create playlist ", mrl, " to the media library" );
         return false;
     }
-    task.file = playlistPtr->addFile( *task.fileFs, task.parentFolder->id(), task.parentFolderFs->device()->isRemovable() );
-    if ( task.file == nullptr )
+    task.item().setFile( playlistPtr->addFile( *task.item().fileFs(),
+                                               task.item().parentFolder()->id(),
+                                               task.item().parentFolderFs()->device()->isRemovable() ) );
+    if ( task.item().file() == nullptr )
     {
         LOG_ERROR( "Failed to add playlist file ", mrl );
         return false;
@@ -465,7 +468,7 @@ std::shared_ptr<Album> MetadataParser::findAlbum( parser::Task& task, std::share
     }
 
     if ( m_previousAlbum != nullptr && albumName == m_previousAlbum->title() &&
-         m_previousFolderId != 0 && task.file->folderId() == m_previousFolderId )
+         m_previousFolderId != 0 && task.item().file()->folderId() == m_previousFolderId )
         return m_previousAlbum;
     m_previousAlbum.reset();
     m_previousFolderId = 0;
@@ -551,7 +554,7 @@ std::shared_ptr<Album> MetadataParser::findAlbum( parser::Task& task, std::share
         }
 
         // Assume album files will be in the same folder.
-        auto newFileFolder = utils::file::directory( task.file->mrl() );
+        auto newFileFolder = utils::file::directory( task.item().file()->mrl() );
         auto trackFiles = tracks[0]->files();
         bool differentFolder = false;
         for ( auto& f : trackFiles )
@@ -611,7 +614,7 @@ std::shared_ptr<Album> MetadataParser::findAlbum( parser::Task& task, std::share
     {
         LOG_WARN( "Multiple candidates for album ", albumName, ". Selecting first one out of luck" );
     }
-    m_previousFolderId = task.file->folderId();
+    m_previousFolderId = task.item().file()->folderId();
     m_previousAlbum = albums[0];
     return albums[0];
 }

src/parser/ParserWorker.cpp

@@ -173,7 +173,7 @@ void ParserWorker::mainloop()
         {
             LOG_INFO( "Executing ", serviceName, " task on ", task->item().mrl() );
             auto chrono = std::chrono::steady_clock::now();
-            if ( ( task->file != nullptr && task->file->isDeleted() )
+            if ( ( task->item().file() != nullptr && task->item().file()->isDeleted() )
                  || ( task->item().media() != nullptr && task->item().media()->isDeleted() ) )
                 status = parser::Task::Status::Fatal;
             else

src/parser/Task.cpp

@@ -53,6 +53,7 @@ Task::Task( MediaLibraryPtr ml, sqlite::Row& row )
     , m_ml( ml )
 {
     std::string mrl;
+    unsigned int parentPlaylistIndex;
     row >> m_id
         >> m_step
         >> m_retryCount
@@ -62,6 +63,7 @@ Task::Task( MediaLibraryPtr ml, sqlite::Row& row )
         >> m_parentPlaylistId
         >> parentPlaylistIndex;
     m_item = Item{ std::move( mrl ) };
+    m_item.setParentPlaylistIndex( parentPlaylistIndex );
 }
 
 Task::Task( MediaLibraryPtr ml, std::shared_ptr<fs::IFile> fileFs,
@@ -69,16 +71,12 @@ Task::Task( MediaLibraryPtr ml, std::shared_ptr<fs::IFile> fileFs,
             std::shared_ptr<fs::IDirectory> parentFolderFs,
             std::shared_ptr<Playlist> parentPlaylist,
             unsigned int parentPlaylistIndex )
-    : fileFs( std::move( fileFs ) )
-    , parentFolder( std::move( parentFolder ) )
-    , parentFolderFs( std::move( parentFolderFs ) )
-    , parentPlaylist( std::move( parentPlaylist ) )
-    , parentPlaylistIndex( parentPlaylistIndex )
-    , currentService( 0 )
+    : currentService( 0 )
     , m_ml( ml )
     , m_step( ParserStep::None )
     , m_fileId( 0 )
-    , m_item( this->fileFs->mrl() )
+    , m_item( std::move( fileFs ), std::move( parentFolder ),
+              std::move( parentFolderFs ), std::move( parentPlaylist ), parentPlaylistIndex )
 {
 }
 
@@ -120,12 +118,12 @@ void Task::startParserStep()
 bool Task::updateFileId()
 {
     assert( m_fileId == 0 );
-    assert( file != nullptr && file->id() != 0 );
+    assert( m_item.file() != nullptr && m_item.file()->id() != 0 );
     static const std::string req = "UPDATE " + policy::TaskTable::Name + " SET "
             "file_id = ? WHERE id_task = ?";
-    if ( sqlite::Tools::executeUpdate( m_ml->getConn(), req, file->id(), m_id ) == false )
+    if ( sqlite::Tools::executeUpdate( m_ml->getConn(), req, m_item.file()->id(), m_id ) == false )
         return false;
-    m_fileId = file->id();
+    m_fileId = m_item.file()->id();
     return true;
 }
 
@@ -141,6 +139,22 @@ Task::Item& Task::item()
 
 Task::Item::Item( std::string mrl )
     : m_mrl( std::move( mrl ) )
+    , m_duration( 0 )
+    , m_parentPlaylistIndex( 0 )
+{
+}
+
+Task::Item::Item( std::shared_ptr<fs::IFile> fileFs,
+                  std::shared_ptr<Folder> parentFolder,
+                  std::shared_ptr<fs::IDirectory> parentFolderFs,
+                  std::shared_ptr<Playlist> parentPlaylist, unsigned int parentPlaylistIndex  )
+    : m_mrl( fileFs->mrl() )
+    , m_duration( 0 )
+    , m_fileFs( std::move( fileFs ) )
+    , m_parentFolder( std::move( parentFolder ) )
+    , m_parentFolderFs( std::move( parentFolderFs ) )
+    , m_parentPlaylist( std::move( parentPlaylist ) )
+    , m_parentPlaylistIndex( parentPlaylistIndex )
 {
 }
 
@@ -207,6 +221,66 @@ void Task::Item::setMedia( std::shared_ptr<Media> media )
     m_media = std::move( media );
 }
 
+std::shared_ptr<File> Task::Item::file()
+{
+    return m_file;
+}
+
+void Task::Item::setFile(std::shared_ptr<File> file)
+{
+    m_file = std::move( file );
+}
+
+std::shared_ptr<Folder> Task::Item::parentFolder()
+{
+    return m_parentFolder;
+}
+
+void Task::Item::setParentFolder( std::shared_ptr<Folder> parentFolder )
+{
+    m_parentFolder = std::move( parentFolder );
+}
+
+std::shared_ptr<fs::IFile> Task::Item::fileFs()
+{
+    return m_fileFs;
+}
+
+void Task::Item::setFileFs( std::shared_ptr<fs::IFile> fileFs )
+{
+    m_fileFs = std::move( fileFs );
+}
+
+std::shared_ptr<fs::IDirectory> Task::Item::parentFolderFs()
+{
+    return m_parentFolderFs;
+}
+
+void Task::Item::setParentFolderFs( std::shared_ptr<fs::IDirectory> parentDirectoryFs )
+{
+    m_parentFolderFs = std::move( parentDirectoryFs );
+}
+
+std::shared_ptr<Playlist> Task::Item::parentPlaylist()
+{
+    return m_parentPlaylist;
+}
+
+void Task::Item::setParentPlaylist( std::shared_ptr<Playlist> parentPlaylist )
+{
+    m_parentPlaylist = std::move( parentPlaylist );
+}
+
+unsigned int Task::Item::parentPlaylistIndex() const
+{
+    return m_parentPlaylistIndex;
+}
+
+void Task::Item::setParentPlaylistIndex( unsigned int parentPlaylistIndex )
+{
+    m_parentPlaylistIndex = parentPlaylistIndex;
+}
+
 bool Task::restoreLinkedEntities()
 {
     LOG_INFO("Restoring linked entities of task ", m_id);
@@ -221,7 +295,7 @@ bool Task::restoreLinkedEntities()
     // First of all, we need to know if the file has been created already
     // ie. have we run the MetadataParser service, at least partially
     if ( m_fileId != 0 )
-        file = File::fetch( m_ml, m_fileId );
+        m_item.setFile( File::fetch( m_ml, m_fileId ) );
 
     // We might re-create tasks without mrl to ease the handling of files on
     // external storage.
@@ -230,12 +304,12 @@ bool Task::restoreLinkedEntities()
         // but we expect those to be created from an existing file after a
         // partial/failed migration. If we don't have a file nor an mrl, we
         // can't really process it.
-        if ( file == nullptr )
+        if ( m_item.file() == nullptr )
         {
             assert( !"Can't process a file without a file nor an mrl" );
             return false;
         }
-        auto folder = Folder::fetch( m_ml, file->folderId() );
+        auto folder = Folder::fetch( m_ml, m_item.file()->folderId() );
         if ( folder == nullptr )
         {
             assert( !"Can't file the folder associated with a file" );
@@ -246,11 +320,11 @@ bool Task::restoreLinkedEntities()
         }
         if ( folder->isPresent() == false )
         {
-            LOG_WARN( "Postponing rescan of removable file ", file->rawMrl(),
+            LOG_WARN( "Postponing rescan of removable file ", m_item.file()->rawMrl(),
                       " until the device containing it is present again" );
             return false;
         }
-        setMrl( file->mrl() );
+        setMrl( m_item.file()->mrl() );
     }
     auto fsFactory = m_ml->fsFactoryForMrl( m_item.mrl() );
     if ( fsFactory == nullptr )
@@ -258,7 +332,7 @@ bool Task::restoreLinkedEntities()
 
     try
     {
-        parentFolderFs = fsFactory->createDirectory( utils::file::directory( m_item.mrl() ) );
+        m_item.setParentFolderFs( fsFactory->createDirectory( utils::file::directory( m_item.mrl() ) ) );
     }
     catch ( const std::system_error& ex )
     {
@@ -268,7 +342,7 @@ bool Task::restoreLinkedEntities()
 
     try
     {
-        auto files = parentFolderFs->files();
+        auto files = m_item.parentFolderFs()->files();
         auto it = std::find_if( begin( files ), end( files ), [this]( std::shared_ptr<fs::IFile> f ) {
             return f->mrl() == m_item.mrl();
         });
@@ -277,13 +351,13 @@ bool Task::restoreLinkedEntities()
             LOG_ERROR( "Failed to restore fs::IFile associated with ", m_item.mrl() );
             return false;
         }
-        fileFs = *it;
+        m_item.setFileFs( *it );
     }
     catch ( const std::system_error& ex )
     {
         // If we never found the file yet, we can delete the task. It will be
         // recreated upon next discovery
-        if ( file == nullptr )
+        if ( m_item.file() == nullptr )
         {
             LOG_WARN( "Failed to restore file system instances for mrl ", m_item.mrl(), "."
                       " Removing the task until it gets detected again." );
@@ -301,12 +375,12 @@ bool Task::restoreLinkedEntities()
         return false;
     }
 
-    if ( file != nullptr )
-        m_item.setMedia( file->media() );
+    if ( m_item.file() != nullptr )
+        m_item.setMedia( m_item.file()->media() );
 
-    parentFolder = Folder::fetch( m_ml, m_parentFolderId );
+    m_item.setParentFolder( Folder::fetch( m_ml, m_parentFolderId ) );
     if ( m_parentPlaylistId != 0 )
-        parentPlaylist = Playlist::fetch( m_ml, m_parentPlaylistId );
+        m_item.setParentPlaylist( Playlist::fetch( m_ml, m_parentPlaylistId ) );
 
     return true;
 }
@@ -374,15 +448,19 @@ Task::create( MediaLibraryPtr ml, std::shared_ptr<fs::IFile> fileFs,
               std::shared_ptr<Folder> parentFolder, std::shared_ptr<fs::IDirectory> parentFolderFs,
               std::pair<std::shared_ptr<Playlist>, unsigned int> parentPlaylist )
 {
+    auto parentFolderId = parentFolder->id();
+    auto parentPlaylistId = parentPlaylist.first != nullptr ? parentPlaylist.first->id() : 0;
+    auto parentPlaylistIndex = parentPlaylist.second;
+
     std::shared_ptr<Task> self = std::make_shared<Task>( ml, std::move( fileFs ),
         std::move( parentFolder ), std::move( parentFolderFs ),
         std::move( parentPlaylist.first ), parentPlaylist.second );
     const std::string req = "INSERT INTO " + policy::TaskTable::Name +
         "(mrl, parent_folder_id, parent_playlist_id, parent_playlist_index) "
         "VALUES(?, ?, ?, ?)";
-    if ( insert( ml, self, req, self->m_item.mrl(), self->parentFolder->id(), sqlite::ForeignKey(
-                 self->parentPlaylist ? self->parentPlaylist->id() : 0 ),
-                 self->parentPlaylistIndex ) == false )
+    if ( insert( ml, self, req, self->m_item.mrl(), parentFolderId,
+                 sqlite::ForeignKey( parentPlaylistId ),
+                 parentPlaylistIndex ) == false )
         return nullptr;
     return self;
 }

src/parser/Task.h

@@ -94,6 +94,9 @@ public:
     public:
         Item() = default;
         Item( std::string mrl );
+        Item( std::shared_ptr<fs::IFile> fileFs,
+              std::shared_ptr<Folder> folder, std::shared_ptr<fs::IDirectory> folderFs,
+              std::shared_ptr<Playlist> parentPlaylist, unsigned int parentPlaylistIndex );
         enum class Metadata : uint8_t
         {
             Title,
@@ -162,6 +165,24 @@ public:
         std::shared_ptr<Media> media();
         void setMedia( std::shared_ptr<Media> media );
 
+        std::shared_ptr<File> file();
+        void setFile( std::shared_ptr<File> file );
+
+        std::shared_ptr<Folder> parentFolder();
+        void setParentFolder(std::shared_ptr<Folder> parentFolder );
+
+        std::shared_ptr<fs::IFile> fileFs();
+        void setFileFs( std::shared_ptr<fs::IFile> fileFs );
+
+        std::shared_ptr<fs::IDirectory> parentFolderFs();
+        void setParentFolderFs( std::shared_ptr<fs::IDirectory> parentFolderFs );
+
+        std::shared_ptr<Playlist> parentPlaylist();
+        void setParentPlaylist( std::shared_ptr<Playlist> parentPlaylist );
+
+        unsigned int parentPlaylistIndex() const;
+        void setParentPlaylistIndex( unsigned int parentPlaylistIndex );
+
     private:
         std::string m_mrl;
         std::unordered_map<Metadata, std::string> m_metadata;
@@ -169,6 +190,12 @@ public:
         std::vector<Track> m_tracks;
         int64_t m_duration;
         std::shared_ptr<Media> m_media;
+        std::shared_ptr<File> m_file;
+        std::shared_ptr<fs::IFile> m_fileFs;
+        std::shared_ptr<Folder> m_parentFolder;
+        std::shared_ptr<fs::IDirectory> m_parentFolderFs;
+        std::shared_ptr<Playlist> m_parentPlaylist;
+        unsigned int m_parentPlaylistIndex;
     };
 
     static_assert( std::is_move_assignable<Item>::value, "Item must be move assignable" );
@@ -211,12 +238,6 @@ public:
     bool restoreLinkedEntities();
     void setMrl( std::string mrl );
 
-    std::shared_ptr<File>           file;
-    std::shared_ptr<fs::IFile>      fileFs;
-    std::shared_ptr<Folder>         parentFolder;
-    std::shared_ptr<fs::IDirectory> parentFolderFs;
-    std::shared_ptr<Playlist>       parentPlaylist;
-    unsigned int                    parentPlaylistIndex;
     unsigned int                    currentService;
 
     static void createTable( sqlite::Connection* dbConnection );