Commit 74ca00bc authored by Hugo Beauzée-Luyssen's avatar Hugo Beauzée-Luyssen
Browse files

IMediaLibrary: Add discovery callbacks

Use a more robust hack (or uglyness, actually) to wait for discoveries &
FS reloads.
This makes the folders tests green again!
parent 42b0194d
......@@ -28,6 +28,8 @@ public:
virtual void onDiscoveryStarted( const std::string& entryPoint ) = 0;
virtual void onDiscoveryCompleted( const std::string& entryPoint ) = 0;
virtual void onReloadStarted() = 0;
virtual void onReloadCompleted() = 0;
};
class IMediaLibrary
......
......@@ -95,12 +95,16 @@ void DiscovererWorker::run()
}
else
{
if ( m_cb != nullptr )
m_cb->onReloadStarted();
for ( auto& d : m_discoverers )
{
d->reload();
if ( m_run == false )
break;
}
if ( m_cb != nullptr )
m_cb->onReloadCompleted();
}
}
LOG_INFO( "Exiting DiscovererWorker thread" );
......
......@@ -31,6 +31,7 @@ class Folders : public Tests
TEST_F( Folders, Add )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -43,6 +44,7 @@ TEST_F( Folders, Add )
TEST_F( Folders, Delete )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -67,7 +69,10 @@ TEST_F( Folders, Delete )
auto file = ml->file( filePath );
ASSERT_EQ( nullptr, file );
cbMock->prepareForReload();
Reload();
bool reloaded = cbMock->waitForReload();
ASSERT_TRUE( reloaded );
// Recheck folder deletion from DB:
f = ml->folder( folderPath );
......@@ -76,11 +81,15 @@ TEST_F( Folders, Delete )
TEST_F( Folders, Load )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
cbMock->prepareForReload();
Reload();
bool reloaded = cbMock->waitForReload();
ASSERT_TRUE( reloaded );
auto files = ml->files();
ASSERT_EQ( files.size(), 3u );
......@@ -90,6 +99,7 @@ TEST_F( Folders, Load )
TEST_F( Folders, InvalidPath )
{
cbMock->prepareForWait( 1 );
ml->discover( "/invalid/path" );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -100,6 +110,7 @@ TEST_F( Folders, InvalidPath )
TEST_F( Folders, List )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -109,7 +120,10 @@ TEST_F( Folders, List )
auto files = f->files();
ASSERT_EQ( files.size(), 2u );
cbMock->prepareForReload();
Reload();
bool reloaded = cbMock->waitForReload();
ASSERT_TRUE( reloaded );
f = ml->folder( f->path() );
files = f->files();
......@@ -118,6 +132,7 @@ TEST_F( Folders, List )
TEST_F( Folders, AbsolutePath )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -128,6 +143,7 @@ TEST_F( Folders, AbsolutePath )
TEST_F( Folders, ListFolders )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -144,7 +160,7 @@ TEST_F( Folders, ListFolders )
auto file = subFiles[0];
ASSERT_EQ( std::string{ mock::FileSystemFactory::SubFolder } + "subfile.mp4", file->mrl() );
// Now again, without cache
// Now again, without cache. No need to wait for fs discovery reload here
Reload();
f = ml->folder( f->path() );
......@@ -161,6 +177,7 @@ TEST_F( Folders, ListFolders )
TEST_F( Folders, LastModificationDate )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -180,6 +197,7 @@ TEST_F( Folders, LastModificationDate )
TEST_F( Folders, NewFolderWithFile )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -191,7 +209,11 @@ TEST_F( Folders, NewFolderWithFile )
fsMock->addFolder( mock::FileSystemFactory::Root, "newfolder/", time( nullptr ) );
fsMock->addFile( newFolder, "newfile.avi" );
// This will trigger a reload
cbMock->prepareForReload();
Reload();
bool reloaded = cbMock->waitForReload();
ASSERT_TRUE( reloaded );
ASSERT_EQ( 4u, ml->files().size() );
auto file = ml->file( newFolder + "newfile.avi" );
......@@ -201,6 +223,7 @@ TEST_F( Folders, NewFolderWithFile )
// This is expected to fail until we fix the file system modifications detection
TEST_F( Folders, NewFileInSubFolder )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -214,7 +237,10 @@ TEST_F( Folders, NewFileInSubFolder )
ml.reset();
fsMock->addFile( mock::FileSystemFactory::SubFolder, "newfile.avi" );
cbMock->prepareForReload();
Reload();
bool reloaded = cbMock->waitForReload();
ASSERT_TRUE( reloaded );
ASSERT_EQ( 4u, ml->files().size() );
auto file = ml->file( std::string( mock::FileSystemFactory::SubFolder ) + "newfile.avi" );
......@@ -227,6 +253,7 @@ TEST_F( Folders, NewFileInSubFolder )
TEST_F( Folders, RemoveFileFromDirectory )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -236,7 +263,10 @@ TEST_F( Folders, RemoveFileFromDirectory )
ml.reset();
fsMock->removeFile( mock::FileSystemFactory::SubFolder, "subfile.mp4" );
cbMock->prepareForReload();
Reload();
bool reloaded = cbMock->waitForReload();
ASSERT_TRUE( reloaded );
ASSERT_EQ( 2u, ml->files().size() );
auto file = ml->file( std::string( mock::FileSystemFactory::SubFolder ) + "subfile.mp4" );
......@@ -247,6 +277,7 @@ TEST_F( Folders, RemoveFileFromDirectory )
TEST_F( Folders, RemoveDirectory )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -256,7 +287,10 @@ TEST_F( Folders, RemoveDirectory )
ml.reset();
fsMock->removeFolder( mock::FileSystemFactory::SubFolder );
cbMock->prepareForReload();
Reload();
bool reloaded = cbMock->waitForReload();
ASSERT_TRUE( reloaded );
ASSERT_EQ( 2u, ml->files().size() );
auto file = ml->file( std::string( mock::FileSystemFactory::SubFolder ) + "subfile.mp4" );
......@@ -267,6 +301,7 @@ TEST_F( Folders, RemoveDirectory )
TEST_F( Folders, UpdateFile )
{
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -280,7 +315,11 @@ TEST_F( Folders, UpdateFile )
fsMock->files[filePath]->markAsModified();
fsMock->dirs[mock::FileSystemFactory::SubFolder]->markAsModified();
cbMock->prepareForReload();
Reload();
bool reloaded = cbMock->waitForReload();
ASSERT_TRUE( reloaded );
f = ml->file( filePath );
ASSERT_NE( nullptr, f );
// The file is expected to be deleted and re-added since it changed, so the
......@@ -292,6 +331,7 @@ TEST_F( Folders, UpdateFile )
TEST_F( Folders, CheckRemovable )
{
fsMock->dirs[mock::FileSystemFactory::SubFolder]->markRemovable();
cbMock->prepareForWait( 1 );
ml->discover( "." );
bool discovered = cbMock->wait();
ASSERT_TRUE( discovered );
......@@ -303,6 +343,7 @@ TEST_F( Folders, CheckRemovable )
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 );
......
......@@ -29,6 +29,8 @@ class ServiceCb : public IMediaLibraryCb
// IMediaLibraryCb interface
virtual void onDiscoveryStarted( const std::string& ) override {}
virtual void onDiscoveryCompleted( const std::string& ) override {}
virtual void onReloadStarted() override {}
virtual void onReloadCompleted() override {}
};
class VLCMetadataServices : public Tests
......
......@@ -19,34 +19,65 @@ public:
{
}
virtual void onDiscoveryStarted( const std::string& )
virtual void onDiscoveryStarted( const std::string& ) override
{
}
virtual void onDiscoveryCompleted( const std::string& )
virtual void onDiscoveryCompleted( const std::string& ) override
{
--m_nbDiscoveryToWait;
m_cond.notify_all();
if ( --m_nbDiscoveryToWait == 0 )
m_cond.notify_all();
}
virtual void onReloadStarted() override
{
}
virtual void onReloadCompleted() override
{
if ( --m_nbReloadExpected == 0 )
m_reloadCond.notify_all();
}
bool wait()
{
// If m_nbDiscoveryToWait is 0 when entering this function, it means the discovery
// hasn't started yet. This is only used for tests, and so far, with a single discovery module.
// Obviously, this is a hack and won't work if (when) we add another discovery module...
// Hello future self, enjoy.
auto v = 0;
m_nbDiscoveryToWait.compare_exchange_strong(v, 1);
std::unique_lock<std::mutex> lock( m_mutex );
return m_cond.wait_for( lock, std::chrono::seconds( 5 ), [this]() {
return m_nbDiscoveryToWait == 0;
} );
}
// We don't synchronously trigger the discovery, so we can't rely on started/completed being called
// Instead, we manually tell this mock how much discovery we expect. This sucks, but the alternative
// would probably be to have an extra IMediaLibraryCb member to signal that a discovery has been queue.
// however, in practice, this is a callback that says "yep, you've called IMediaLibrary::discover()"
// which is probably lame.
void prepareForWait(int nbExpected)
{
m_nbDiscoveryToWait = nbExpected;
}
void prepareForReload()
{
m_nbReloadExpected = 1;
}
bool waitForReload()
{
std::unique_lock<std::mutex> lock( m_mutex );
return m_reloadCond.wait_for( lock, std::chrono::seconds( 5 ), [this](){
return m_nbReloadExpected == 0;
});
}
private:
std::atomic_int m_nbDiscoveryToWait;
std::condition_variable m_cond;
std::mutex m_mutex;
std::atomic_int m_nbReloadExpected;
std::condition_variable m_reloadCond;
std::mutex m_reloadMutex;
};
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment