Link folders & devices upon folders creation

Also remove the folders' isRemovable method, since this is now part of
the device properties

include/IFolder.h

@@ -36,6 +36,5 @@ public:
     virtual std::vector<MediaPtr> files() = 0;
     virtual std::vector<FolderPtr> folders() = 0;
     virtual unsigned int lastModificationDate() = 0;
-    virtual bool isRemovable() = 0;
     virtual FolderPtr parent() = 0;
 };

include/factory/IFileSystem.h

@@ -48,5 +48,12 @@ namespace factory
         /// \param fileName an absolute path to a file
         ///
         virtual std::unique_ptr<fs::IFile> createFile( const std::string& fileName ) = 0;
+        ///
+        /// \brief createDevice creates a representation of a device
+        /// \param uuid The device UUID
+        /// \return A representation of the device, or nullptr if the device is currently unavailable.
+        ///
+        virtual std::shared_ptr<fs::IDevice> createDevice( const std::string& uuid ) = 0;
+
     };
 }

src/Folder.cpp

@@ -21,6 +21,7 @@
  *****************************************************************************/
 
 #include "Folder.h"
+#include "Device.h"
 #include "Media.h"
 
 #include "database/SqliteTools.h"
@@ -40,17 +41,17 @@ Folder::Folder(DBConnection dbConnection, sqlite::Row& row )
         >> m_path
         >> m_parent
         >> m_lastModificationDate
-        >> m_isRemovable
-        >> m_isBlacklisted;
+        >> m_isBlacklisted
+        >> m_deviceId;
 }
 
-Folder::Folder( const std::string& path, time_t lastModificationDate, bool isRemovable, unsigned int parent )
+Folder::Folder( const std::string& path, time_t lastModificationDate, unsigned int parent, unsigned int deviceId )
     : m_id( 0 )
     , m_path( path )
     , m_parent( parent )
     , m_lastModificationDate( lastModificationDate )
-    , m_isRemovable( isRemovable )
     , m_isBlacklisted( false )
+    , m_deviceId( deviceId )
 {
 }
 
@@ -62,21 +63,21 @@ bool Folder::createTable(DBConnection connection)
             "path TEXT UNIQUE ON CONFLICT FAIL,"
             "id_parent UNSIGNED INTEGER,"
             "last_modification_date UNSIGNED INTEGER,"
-            "is_removable INTEGER,"
             "is_blacklisted INTEGER,"
+            "device_id UNSIGNED INTEGER,"
             "FOREIGN KEY (id_parent) REFERENCES " + policy::FolderTable::Name +
             "(id_folder) ON DELETE CASCADE"
             ")";
     return sqlite::Tools::executeRequest( connection, req );
 }
 
-std::shared_ptr<Folder> Folder::create( DBConnection connection, const std::string& path, time_t lastModificationDate, bool isRemovable, unsigned int parentId )
+std::shared_ptr<Folder> Folder::create(DBConnection connection, const std::string& path, time_t lastModificationDate, unsigned int parentId, Device& device )
 {
-    auto self = std::make_shared<Folder>( path, lastModificationDate, isRemovable, parentId );
+    auto self = std::make_shared<Folder>( path, lastModificationDate, parentId, device.id() );
     static const std::string req = "INSERT INTO " + policy::FolderTable::Name +
-            "(path, id_parent, last_modification_date, is_removable) VALUES(?, ?, ?, ?)";
+            "(path, id_parent, last_modification_date, device_id) VALUES(?, ?, ?, ?)";
     if ( insert( connection, self, req, path, sqlite::ForeignKey( parentId ),
-                         lastModificationDate, isRemovable ) == false )
+                         lastModificationDate, device.id() ) == false )
         return nullptr;
     self->m_dbConection = connection;
     return self;
@@ -139,7 +140,7 @@ bool Folder::setLastModificationDate( unsigned int lastModificationDate )
     return true;
 }
 
-bool Folder::isRemovable()
+unsigned int Folder::deviceId() const
 {
-    return m_isRemovable;
+    return m_deviceId;
 }

src/Folder.h

@@ -28,6 +28,7 @@
 #include <sqlite3.h>
 
 class Folder;
+class Device;
 
 namespace fs
 {
@@ -49,10 +50,10 @@ class Folder : public IFolder, public DatabaseHelpers<Folder, policy::FolderTabl
 {
 public:
     Folder( DBConnection dbConnection, sqlite::Row& row );
-    Folder(const std::string& path, time_t lastModificationDate, bool isRemovable, unsigned int parent );
+    Folder(const std::string& path, time_t lastModificationDate, unsigned int parent , unsigned int deviceId);
 
     static bool createTable( DBConnection connection );
-    static std::shared_ptr<Folder> create(DBConnection connection, const std::string& path, time_t lastModificationDate, bool isRemovable, unsigned int parentId );
+    static std::shared_ptr<Folder> create(DBConnection connection, const std::string& path, time_t lastModificationDate, unsigned int parentId, Device& device );
     static bool blacklist( DBConnection connection, const std::string& path );
 
     static std::shared_ptr<Folder> fromPath( DBConnection conn, const std::string& path );
@@ -64,7 +65,7 @@ public:
     virtual FolderPtr parent() override;
     virtual unsigned int lastModificationDate() override;
     bool setLastModificationDate(unsigned int lastModificationDate);
-    bool isRemovable();
+    unsigned int deviceId() const;
 
 private:
     DBConnection m_dbConection;
@@ -73,8 +74,8 @@ private:
     std::string m_path;
     unsigned int m_parent;
     unsigned int m_lastModificationDate;
-    bool m_isRemovable;
     bool m_isBlacklisted;
+    unsigned int m_deviceId;
 
     friend struct policy::FolderTable;
 };

src/discoverer/FsDiscoverer.cpp

@@ -69,13 +69,12 @@ bool FsDiscoverer::discover( const std::string &entryPoint )
     auto blist = blacklist();
     if ( isBlacklisted( fsDir->path(), blist ) == true )
         return false;
-    return addFolder( fsDir.get(), nullptr, nullptr, blist );
+    return addFolder( fsDir.get(), nullptr, blist );
 }
 
 void FsDiscoverer::reload()
 {
     //FIXME: This should probably be in a sql transaction
-    //FIXME: This shouldn't be done for "removable"/network files
     static const std::string req = "SELECT * FROM " + policy::FolderTable::Name
             + " WHERE id_parent IS NULL";
     auto rootFolders = Folder::fetchAll<Folder>( m_dbConn, req );
@@ -83,6 +82,15 @@ void FsDiscoverer::reload()
     for ( const auto& f : rootFolders )
     {
         auto folder = m_fsFactory->createDirectory( f->path() );
+        // Don't assume a folder that is a mountpoint will get its modification date updated on all platforms
+        // So first, we check if the device containing this folder is available:
+        auto deviceFs = folder->device();
+        if ( deviceFs == nullptr )
+        {
+            auto device = Device::fetch( m_dbConn, f->deviceId() );
+            device->setPresent( false );
+            continue;
+        }
         if ( folder->lastModificationDate() == f->lastModificationDate() )
             continue;
         checkSubfolders( folder.get(), f.get(), blist );
@@ -123,7 +131,7 @@ bool FsDiscoverer::checkSubfolders( fs::IDirectory* folder, Folder* parentFolder
                 continue;
             }
             LOG_INFO( "New folder detected: ", subFolderPath );
-            addFolder( subFolder.get(), parentFolder, folder, blacklist );
+            addFolder( subFolder.get(), parentFolder, blacklist );
             continue;
         }
         auto folderInDb = *it;
@@ -198,25 +206,22 @@ bool FsDiscoverer::isBlacklisted(const std::string& path, const std::vector<std:
     }) != end( blacklist );
 }
 
-bool FsDiscoverer::addFolder( fs::IDirectory* folder, Folder* parentFolder, fs::IDirectory* parent, const std::vector<std::shared_ptr<Folder>>& blacklist ) const
+bool FsDiscoverer::addFolder( fs::IDirectory* folder, Folder* parentFolder, const std::vector<std::shared_ptr<Folder>>& blacklist ) const
 {
     // Force <0> as lastModificationDate, so this folder is detected as outdated
     // by the modification checking code
     auto deviceFs = folder->device();
-    // In case this is an entry point, we always want to create a device representation.
-    // Otherwise, we only need to know if this we discovered a new mountpoint.
-    if ( parent == nullptr || parent->device() != deviceFs )
+    // We are creating a folder, there has to be a device containing it.
+    assert( deviceFs != nullptr );
+    auto device = Device::fromUuid( m_dbConn, deviceFs->uuid() );
+    if ( device == nullptr )
     {
-        auto device = Device::fromUuid( m_dbConn, deviceFs->uuid() );
-        if ( device == nullptr )
-        {
-            LOG_INFO( "Creating new device in DB ", deviceFs->uuid() );
-            device = Device::create( m_dbConn, deviceFs->uuid(), deviceFs->isRemovable() );
-        }
+        LOG_INFO( "Creating new device in DB ", deviceFs->uuid() );
+        device = Device::create( m_dbConn, deviceFs->uuid(), deviceFs->isRemovable() );
     }
 
-    auto f = Folder::create( m_dbConn, folder->path(), 0, folder->isRemovable(),
-                             parentFolder != nullptr ? parentFolder->id() : 0 );
+    auto f = Folder::create( m_dbConn, folder->path(), 0,
+                             parentFolder != nullptr ? parentFolder->id() : 0, *device );
     if ( f == nullptr )
         return false;
     checkFiles( folder, f.get() );

src/discoverer/FsDiscoverer.h

@@ -43,7 +43,7 @@ private:
     void checkFiles( fs::IDirectory *folder, Folder* parentFolder ) const;
     std::vector<std::shared_ptr<Folder>> blacklist() const;
     bool isBlacklisted( const std::string& path, const std::vector<std::shared_ptr<Folder>>& blacklist ) const;
-    bool addFolder( fs::IDirectory* folder, Folder* parentFolder, fs::IDirectory* parent , const std::vector<std::shared_ptr<Folder> >& blacklist ) const;
+    bool addFolder(fs::IDirectory* folder, Folder* parentFolder, const std::vector<std::shared_ptr<Folder> >& blacklist ) const;
 
 private:
     MediaLibrary* m_ml;

src/factory/FileSystem.cpp

@@ -27,6 +27,7 @@
 #if defined(__linux__) || defined(__APPLE__)
 # include "filesystem/unix/Directory.h"
 # include "filesystem/unix/File.h"
+# include "filesystem/unix/Device.h"
 #elif defined(_WIN32)
 # include "filesystem/win32/Directory.h"
 # include "filesystem/win32/File.h"
@@ -53,4 +54,11 @@ std::unique_ptr<fs::IFile> FileSystemFactory::createFile(const std::string& file
     return std::unique_ptr<fs::IFile>( new fs::File( fileName ) );
 }
 
+std::shared_ptr<fs::IDevice> FileSystemFactory::createDevice( const std::string& uuid )
+{
+    return fs::Device::fromUuid( uuid );
+}
+
+
+
 }

src/factory/FileSystem.h

@@ -35,6 +35,7 @@ namespace factory
     public:
         virtual std::shared_ptr<fs::IDirectory> createDirectory( const std::string& path ) override;
         virtual std::unique_ptr<fs::IFile> createFile( const std::string& fileName ) override;
+        virtual std::shared_ptr<fs::IDevice> createDevice( const std::string& uuid ) override;
 
     private:
         std::unordered_map<std::string, std::shared_ptr<fs::IDirectory>> m_dirs;

src/filesystem/unix/Device.cpp

@@ -71,6 +71,17 @@ std::shared_ptr<IDevice> Device::fromPath( const std::string& path )
     return nullptr;
 }
 
+std::shared_ptr<IDevice> Device::fromUuid( const std::string& uuid )
+{
+    for ( const auto& p : Cache )
+    {
+        const auto& d = p.second;
+        if ( d->uuid() == uuid )
+            return d;
+    }
+    return nullptr;
+}
+
 Device::DeviceMap Device::listDevices()
 {
     DeviceMap res;

src/filesystem/unix/Device.h

@@ -38,7 +38,11 @@ public:
     virtual bool isRemovable() const override;
     virtual const std::string& mountpoint() const override;
 
+    ///
+    /// \brief fromPath Returns the device that contains the given path
+    ///
     static std::shared_ptr<IDevice> fromPath( const std::string& path );
+    static std::shared_ptr<IDevice> fromUuid( const std::string& uuid );
 
 protected:
     Device( const std::string& devicePath );

test/mocks/FileSystem.h

@@ -212,9 +212,9 @@ struct FileSystemFactory : public factory::IFileSystem
 
     FileSystemFactory()
     {
-        auto rootDevice = std::make_shared<Device>( std::string{ Root }, "root" );
-        auto removableDevice = std::make_shared<Device>( std::string{ SubFolder }, "removable" );
-        removableDevice ->setRemovable( true );
+        rootDevice = std::make_shared<Device>( std::string{ Root }, "root" );
+        removableDevice = std::make_shared<Device>( std::string{ SubFolder }, "removable" );
+        removableDevice->setRemovable( true );
         dirs[Root] = std::unique_ptr<mock::Directory>( new Directory{ nullptr, Root, 123, rootDevice  } );
             addFile( Root, "video.avi" );
             addFile( Root, "audio.mp3" );
@@ -285,8 +285,19 @@ struct FileSystemFactory : public factory::IFileSystem
         return std::unique_ptr<fs::IFile>( new File( static_cast<const mock::File&>( * it->second.get() ) ) );
     }
 
+    virtual std::shared_ptr<fs::IDevice> createDevice( const std::string& uuid ) override
+    {
+        if ( uuid == "root" )
+            return rootDevice;
+        if ( uuid == "removable" )
+            return removableDevice;
+        return nullptr;
+    }
+
     std::unordered_map<std::string, std::shared_ptr<mock::File>> files;
     std::unordered_map<std::string, std::shared_ptr<mock::Directory>> dirs;
+    std::shared_ptr<Device> rootDevice;
+    std::shared_ptr<Device> removableDevice;
 };
 
 // Noop FS (basically just returns file names, and don't try to access those.)
@@ -344,6 +355,11 @@ public:
     {
         return std::unique_ptr<fs::IFile>( new NoopFile( fileName ) );
     }
+
+    virtual std::shared_ptr<fs::IDevice> createDevice( const std::string& ) override
+    {
+        return nullptr;
+    }
 };
 
 }

test/unittest/FolderTests.cpp

@@ -350,31 +350,6 @@ TEST_F( Folders, UpdateFile )
     ASSERT_NE( id, f->id() );
 }
 
-// This simply tests that the flag is properly stored in db
-TEST_F( Folders, CheckRemovable )
-{
-    fsMock->dirs[mock::FileSystemFactory::SubFolder]->markRemovable();
-    cbMock->prepareForWait( 1 );
-    ml->discover( "." );
-    bool discovered = cbMock->wait();
-    ASSERT_TRUE( discovered );
-
-    auto f = ml->folder( mock::FileSystemFactory::Root );
-    ASSERT_NE( f, nullptr );
-    ASSERT_FALSE( f->isRemovable() );
-    auto subfolder = ml->folder( mock::FileSystemFactory::SubFolder );
-    ASSERT_NE( subfolder, nullptr );
-    ASSERT_TRUE( subfolder->isRemovable() );
-
-    // No actual FS change, no need to wait for the actual FS reload
-    Reload();
-
-    f = ml->folder( mock::FileSystemFactory::Root );
-    ASSERT_FALSE( f->isRemovable() );
-    subfolder = ml->folder( mock::FileSystemFactory::SubFolder );
-    ASSERT_TRUE( subfolder->isRemovable() );
-}
-
 TEST_F( Folders, Blacklist )
 {
     cbMock->prepareForWait( 1 );