From de630e0a9f415135f029c5b12df3cdc0ae26283f Mon Sep 17 00:00:00 2001
From: David Sansome <davidsansome@gmail.com>
Date: Tue, 15 Jun 2010 14:24:17 +0000
Subject: [PATCH] Run LoadLocalDirectory in another thread so it doesn't block

---
 src/core/songloader.cpp   | 14 +++++----
 src/core/songloader.h     |  2 +-
 tests/songloader_test.cpp | 61 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 6 deletions(-)

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 <QDirIterator>
 #include <QFileInfo>
 #include <QTimer>
+#include <QtConcurrentRun>
 #include <QtDebug>
 
 #include <boost/bind.hpp>
@@ -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 <QBuffer>
+#include <QDir>
 #include <QEventLoop>
 #include <QSignalSpy>
 
 #include <boost/scoped_ptr.hpp>
+#include <cstdlib>
 
 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());
+}