diff --git a/src/core/songloader.cpp b/src/core/songloader.cpp index 5c6a2e1cc..97ec0f45e 100644 --- a/src/core/songloader.cpp +++ b/src/core/songloader.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -55,7 +56,8 @@ SongLoader::Result SongLoader::LoadLocal() { // inside right away. QString filename = url_.toLocalFile(); if (QFileInfo(filename).isDir()) { - return LoadLocalDirectory(filename); + QtConcurrent::run(this, &SongLoader::LoadLocalDirectory, filename); + return WillLoadAsync; } // It's a local file, so check if it looks like a playlist. @@ -74,13 +76,14 @@ SongLoader::Result SongLoader::LoadLocal() { // Not a playlist, so just assume it's a song Song song; song.InitFromFile(filename, -1); - songs_ << song; + if (song.is_valid()) + songs_ << song; } return Success; } -SongLoader::Result SongLoader::LoadLocalDirectory(const QString& filename) { +void SongLoader::LoadLocalDirectory(const QString& filename) { QDirIterator it(filename, QDir::Files | QDir::NoDotAndDotDot | QDir::Readable, QDirIterator::Subdirectories); @@ -88,10 +91,11 @@ SongLoader::Result SongLoader::LoadLocalDirectory(const QString& filename) { QString path = it.next(); Song song; song.InitFromFile(path, -1); - songs_ << song; + if (song.is_valid()) + songs_ << song; } - return Success; + emit LoadFinished(true); } SongLoader::Result SongLoader::LoadRemote() { diff --git a/src/core/songloader.h b/src/core/songloader.h index f3a9aa40c..e9e7d595b 100644 --- a/src/core/songloader.h +++ b/src/core/songloader.h @@ -60,8 +60,8 @@ private: }; Result LoadLocal(); - Result LoadLocalDirectory(const QString& filename); Result LoadRemote(); + void LoadLocalDirectory(const QString& filename); // GStreamer callbacks static void TypeFound(GstElement* typefind, uint probability, GstCaps* caps, void* self); diff --git a/tests/songloader_test.cpp b/tests/songloader_test.cpp index c7aca9f70..520fb469b 100644 --- a/tests/songloader_test.cpp +++ b/tests/songloader_test.cpp @@ -22,10 +22,12 @@ #include "engines/gstengine.h" #include +#include #include #include #include +#include class SongLoaderTest : public ::testing::Test { public: @@ -44,6 +46,8 @@ protected: loader_.reset(new SongLoader); } + void LoadLocalDirectory(const QString& dir); + static const char* kRemoteUrl; static GstEngine* sGstEngine; @@ -175,3 +179,60 @@ TEST_F(SongLoaderTest, LoadRemotePlainText) { ASSERT_EQ(1, spy.count()); EXPECT_EQ(false, spy[0][0].toBool()); } + +TEST_F(SongLoaderTest, LoadLocalDirectory) { + // Make a directory and shove some files in it + // Use QTemporaryFile to get a filename, delete the file and make a directory + // in its place with the same name. + QByteArray dir(QString(QDir::tempPath() + "/songloader_testdir-XXXXXX").toLocal8Bit()); + ASSERT_TRUE(mkdtemp(dir.data())); + + QFile resource(":/testdata/beep.mp3"); + resource.open(QIODevice::ReadOnly); + QByteArray data(resource.readAll()); + + // Write 3 MP3 files + for (int i=0 ; i<3 ; ++i) { + QFile mp3(QString("%1/%2.mp3").arg(QString(dir)).arg(i)); + mp3.open(QIODevice::WriteOnly); + mp3.write(data); + } + + // And one file that isn't an MP3 + QFile somethingelse(dir + "/somethingelse.foo"); + somethingelse.open(QIODevice::WriteOnly); + somethingelse.write("I'm not an MP3!"); + somethingelse.close(); + + // The actual test happens in another function so we can always clean up if + // it asserts + LoadLocalDirectory(QString(dir)); + + QFile::remove(QString(dir) + "/0.mp3"); + QFile::remove(QString(dir) + "/1.mp3"); + QFile::remove(QString(dir) + "/2.mp3"); + QFile::remove(QString(dir) + "/somethingelse.foo"); + rmdir(dir.constData()); +} + +void SongLoaderTest::LoadLocalDirectory(const QString &filename) { + // Load the directory + SongLoader::Result ret = loader_->Load(QUrl::fromLocalFile(filename)); + ASSERT_EQ(SongLoader::WillLoadAsync, ret); + + QSignalSpy spy(loader_.get(), SIGNAL(LoadFinished(bool))); + + // Start an event loop to wait for it to read the directory + QEventLoop loop; + QObject::connect(loader_.get(), SIGNAL(LoadFinished(bool)), + &loop, SLOT(quit())); + loop.exec(QEventLoop::ExcludeUserInputEvents); + + // Check the signal was emitted with Success + ASSERT_EQ(1, spy.count()); + EXPECT_EQ(true, spy[0][0].toBool()); + + // Check it loaded three files + ASSERT_EQ(3, loader_->songs().count()); + EXPECT_EQ("Beep mp3", loader_->songs()[2].title()); +}