Add a table for the Magnatune library, and add the Magnatune database to it when the Magnatune node is expanded. Sort the Magnatune library model properly, and don't crash when adding or removing items.

data/data.qrc

@@ -86,5 +86,6 @@
         <file>edit-redo.png</file>
         <file>edit-undo.png</file>
         <file>magnatune.png</file>
+        <file>schema-8.sql</file>
     </qresource>
 </RCC>

data/schema-8.sql

@@ -0,0 +1,40 @@
+/* Schema should be kept identical to the "songs" table, even though most of
+   it isn't used by magnatune */
+CREATE TABLE magnatune_songs (
+  title TEXT,
+  album TEXT,
+  artist TEXT,
+  albumartist TEXT,
+  composer TEXT,
+  track INTEGER,
+  disc INTEGER,
+  bpm REAL,
+  year INTEGER,
+  genre TEXT,
+  comment TEXT,
+  compilation INTEGER,
+
+  length INTEGER,
+  bitrate INTEGER,
+  samplerate INTEGER,
+
+  directory INTEGER NOT NULL,
+  filename TEXT NOT NULL,
+  mtime INTEGER NOT NULL,
+  ctime INTEGER NOT NULL,
+  filesize INTEGER NOT NULL,
+  
+  sampler INTEGER NOT NULL DEFAULT 0,
+  art_automatic TEXT,
+  art_manual TEXT,
+  filetype INTEGER NOT NULL DEFAULT 0,
+  playcount INTEGER NOT NULL DEFAULT 0,
+  lastplayed INTEGER,
+  rating INTEGER,
+  forced_compilation_on INTEGER NOT NULL DEFAULT 0,
+  forced_compilation_off INTEGER NOT NULL DEFAULT 0,
+  effective_compilation NOT NULL DEFAULT 0
+);
+
+UPDATE schema_version SET version=8;
+

src/database.cpp

@@ -27,7 +27,7 @@
 #include <QVariant>
 
 const char* Database::kDatabaseFilename = "clementine.db";
-const int Database::kSchemaVersion = 7;
+const int Database::kSchemaVersion = 8;
 
 int (*Database::_sqlite3_create_function) (
     sqlite3*, const char*, int, int, void*,

src/librarybackend.cpp

@@ -228,12 +228,14 @@ void LibraryBackend::AddOrUpdateSongs(const SongList& songs) {
     // Do a sanity check first - make sure the song's directory still exists
     // This is to fix a possible race condition when a directory is removed
     // while LibraryWatcher is scanning it.
-    check_dir.bindValue(":id", song.directory_id());
-    check_dir.exec();
-    if (db_->CheckErrors(check_dir.lastError())) continue;
+    if (!dirs_table_.isEmpty()) {
+      check_dir.bindValue(":id", song.directory_id());
+      check_dir.exec();
+      if (db_->CheckErrors(check_dir.lastError())) continue;
 
-    if (!check_dir.next())
-      continue; // Directory didn't exist
+      if (!check_dir.next())
+        continue; // Directory didn't exist
+    }
 
 
     if (song.id() == -1) {

src/magnatuneservice.cpp

@@ -26,6 +26,7 @@
 #include <QNetworkReply>
 #include <QXmlStreamReader>
 #include <QtIOCompressor>
+#include <QSortFilterProxyModel>
 
 #include <QtDebug>
 
@@ -36,10 +37,20 @@ const char* MagnatuneService::kDatabaseUrl =
 MagnatuneService::MagnatuneService(RadioModel* parent)
   : RadioService(kServiceName, parent),
     root_(NULL),
-    library_backend_(new LibraryBackend(parent->db(), "songs", "", "", this)),
+    library_backend_(new LibraryBackend(parent->db(), "magnatune_songs", "", "", this)),
     library_model_(new LibraryModel(library_backend_, this)),
+    library_sort_model_(new QSortFilterProxyModel(this)),
+    total_song_count_(0),
     network_(new QNetworkAccessManager(this))
 {
+  connect(library_backend_, SIGNAL(TotalSongCountUpdated(int)),
+          SLOT(UpdateTotalSongCount(int)));
+
+  library_sort_model_->setSourceModel(library_model_);
+  library_sort_model_->setSortRole(LibraryModel::Role_SortText);
+  library_sort_model_->setDynamicSortFilter(true);
+  library_sort_model_->sort(0);
+
   library_model_->Init();
 }
 
@@ -49,7 +60,7 @@ RadioItem* MagnatuneService::CreateRootItem(RadioItem *parent) {
 
   model()->merged_model()->AddSubModel(
       model()->index(root_->row, 0, model()->ItemToIndex(parent)),
-      library_model_);
+      library_sort_model_);
 
   return root_;
 }
@@ -57,7 +68,8 @@ RadioItem* MagnatuneService::CreateRootItem(RadioItem *parent) {
 void MagnatuneService::LazyPopulate(RadioItem *item) {
   switch (item->type) {
     case RadioItem::Type_Service:
-      ReloadDatabase();
+      if (total_song_count_ == 0)
+        ReloadDatabase();
       break;
 
     default:
@@ -103,18 +115,22 @@ void MagnatuneService::ReloadDatabaseFinished() {
     return;
   }
 
+  SongList songs;
+
   QXmlStreamReader reader(&gzip);
   while (!reader.atEnd()) {
     reader.readNext();
 
     if (reader.tokenType() == QXmlStreamReader::StartElement &&
         reader.name() == "Track") {
-      ReadTrack(reader);
+      songs << ReadTrack(reader);
     }
   }
+
+  library_backend_->AddOrUpdateSongs(songs);
 }
 
-void MagnatuneService::ReadTrack(QXmlStreamReader& reader) {
+Song MagnatuneService::ReadTrack(QXmlStreamReader& reader) {
   QXmlStreamAttributes attributes = reader.attributes();
 
   Song song;
@@ -126,5 +142,11 @@ void MagnatuneService::ReadTrack(QXmlStreamReader& reader) {
   song.set_year(attributes.value("year").toString().toInt());
   song.set_filename(attributes.value("url").toString());
 
-  qDebug() << song.artist() << song.album() << song.title();
+  // We need to set these to satisfy the database constraints
+  song.set_directory_id(0);
+  song.set_mtime(0);
+  song.set_ctime(0);
+  song.set_filesize(0);
+
+  return song;
 }

src/magnatuneservice.h

@@ -22,6 +22,7 @@
 #include "radioservice.h"
 
 class QNetworkAccessManager;
+class QSortFilterProxyModel;
 
 class LibraryBackend;
 class LibraryModel;
@@ -41,16 +42,20 @@ class MagnatuneService : public RadioService {
   void StartLoading(const QUrl &url);
 
  private slots:
+  void UpdateTotalSongCount(int count) { total_song_count_ = count; }
   void ReloadDatabase();
   void ReloadDatabaseFinished();
 
  private:
-  void ReadTrack(QXmlStreamReader& reader);
+  Song ReadTrack(QXmlStreamReader& reader);
 
  private:
   RadioItem* root_;
   LibraryBackend* library_backend_;
   LibraryModel* library_model_;
+  QSortFilterProxyModel* library_sort_model_;
+
+  int total_song_count_;
 
   QNetworkAccessManager* network_;
 };

src/mergedproxymodel.cpp

@@ -121,8 +121,8 @@ QModelIndex MergedProxyModel::GetActualSourceParent(const QModelIndex& source_pa
 
 void MergedProxyModel::RowsAboutToBeInserted(const QModelIndex& source_parent,
                                              int start, int end) {
-  beginInsertRows(GetActualSourceParent(
-      source_parent, static_cast<const QAbstractItemModel*>(sender())),
+  beginInsertRows(mapFromSource(GetActualSourceParent(
+      source_parent, static_cast<const QAbstractItemModel*>(sender()))),
       start, end);
 }
 
@@ -132,8 +132,8 @@ void MergedProxyModel::RowsInserted(const QModelIndex&, int, int) {
 
 void MergedProxyModel::RowsAboutToBeRemoved(const QModelIndex& source_parent,
                                             int start, int end) {
-  beginRemoveRows(GetActualSourceParent(
-      source_parent, static_cast<const QAbstractItemModel*>(sender())),
+  beginRemoveRows(mapFromSource(GetActualSourceParent(
+      source_parent, static_cast<const QAbstractItemModel*>(sender()))),
       start, end);
 }
 
@@ -201,8 +201,14 @@ int MergedProxyModel::rowCount(const QModelIndex &parent) const {
 
   QModelIndex source_parent = mapToSource(parent);
   const QAbstractItemModel* child_model = merge_points_.key(source_parent);
-  if (child_model)
+  if (child_model) {
+    // Query the source model but disregard what it says, so it gets a chance
+    // to lazy load
+    source_parent.model()->rowCount(source_parent);
+
     return child_model->rowCount(QModelIndex());
+  }
+
   return source_parent.model()->rowCount(source_parent);
 }
 
@@ -225,7 +231,8 @@ bool MergedProxyModel::hasChildren(const QModelIndex &parent) const {
   const QAbstractItemModel* child_model = merge_points_.key(source_parent);
 
   if (child_model)
-    return child_model->hasChildren(QModelIndex());
+    return child_model->hasChildren(QModelIndex()) ||
+           source_parent.model()->hasChildren(source_parent);
   return source_parent.model()->hasChildren(source_parent);
 }
 

tests/mergedproxymodel_test.cpp

@@ -19,6 +19,7 @@
 #include "mergedproxymodel.h"
 
 #include <QStandardItemModel>
+#include <QSignalSpy>
 
 class MergedProxyModelTest : public ::testing::Test {
  protected:
@@ -67,7 +68,7 @@ TEST_F(MergedProxyModelTest, Merged) {
   QStandardItemModel submodel;
   submodel.appendRow(new QStandardItem("two"));
 
-  merged_.AddModel(source_.index(0, 0, QModelIndex()), &submodel);
+  merged_.AddSubModel(source_.index(0, 0, QModelIndex()), &submodel);
 
   ASSERT_EQ(1, merged_.rowCount(QModelIndex()));
   QModelIndex one_i = merged_.index(0, 0, QModelIndex());
@@ -82,3 +83,79 @@ TEST_F(MergedProxyModelTest, Merged) {
   EXPECT_EQ(0, merged_.rowCount(two_i));
   EXPECT_FALSE(merged_.hasChildren(two_i));
 }
+
+TEST_F(MergedProxyModelTest, SourceInsert) {
+  QSignalSpy before_spy(&merged_, SIGNAL(rowsAboutToBeInserted(QModelIndex,int,int)));
+  QSignalSpy after_spy(&merged_, SIGNAL(rowsInserted(QModelIndex,int,int)));
+
+  source_.appendRow(new QStandardItem("one"));
+
+  ASSERT_EQ(1, before_spy.count());
+  ASSERT_EQ(1, after_spy.count());
+  EXPECT_FALSE(before_spy[0][0].value<QModelIndex>().isValid());
+  EXPECT_EQ(0, before_spy[0][1].toInt());
+  EXPECT_EQ(0, before_spy[0][2].toInt());
+  EXPECT_FALSE(after_spy[0][0].value<QModelIndex>().isValid());
+  EXPECT_EQ(0, after_spy[0][1].toInt());
+  EXPECT_EQ(0, after_spy[0][2].toInt());
+}
+
+TEST_F(MergedProxyModelTest, SourceRemove) {
+  source_.appendRow(new QStandardItem("one"));
+
+  QSignalSpy before_spy(&merged_, SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int)));
+  QSignalSpy after_spy(&merged_, SIGNAL(rowsRemoved(QModelIndex,int,int)));
+
+  source_.removeRow(0, QModelIndex());
+
+  ASSERT_EQ(1, before_spy.count());
+  ASSERT_EQ(1, after_spy.count());
+  EXPECT_FALSE(before_spy[0][0].value<QModelIndex>().isValid());
+  EXPECT_EQ(0, before_spy[0][1].toInt());
+  EXPECT_EQ(0, before_spy[0][2].toInt());
+  EXPECT_FALSE(after_spy[0][0].value<QModelIndex>().isValid());
+  EXPECT_EQ(0, after_spy[0][1].toInt());
+  EXPECT_EQ(0, after_spy[0][2].toInt());
+}
+
+TEST_F(MergedProxyModelTest, SubInsert) {
+  source_.appendRow(new QStandardItem("one"));
+  QStandardItemModel submodel;
+  merged_.AddSubModel(source_.index(0, 0, QModelIndex()), &submodel);
+
+  QSignalSpy before_spy(&merged_, SIGNAL(rowsAboutToBeInserted(QModelIndex,int,int)));
+  QSignalSpy after_spy(&merged_, SIGNAL(rowsInserted(QModelIndex,int,int)));
+
+  submodel.appendRow(new QStandardItem("two"));
+
+  ASSERT_EQ(1, before_spy.count());
+  ASSERT_EQ(1, after_spy.count());
+  EXPECT_EQ("one", before_spy[0][0].value<QModelIndex>().data());
+  EXPECT_EQ(0, before_spy[0][1].toInt());
+  EXPECT_EQ(0, before_spy[0][2].toInt());
+  EXPECT_EQ("one", after_spy[0][0].value<QModelIndex>().data());
+  EXPECT_EQ(0, after_spy[0][1].toInt());
+  EXPECT_EQ(0, after_spy[0][2].toInt());
+}
+
+TEST_F(MergedProxyModelTest, SubRemove) {
+  source_.appendRow(new QStandardItem("one"));
+  QStandardItemModel submodel;
+  merged_.AddSubModel(source_.index(0, 0, QModelIndex()), &submodel);
+
+  submodel.appendRow(new QStandardItem("two"));
+
+  QSignalSpy before_spy(&merged_, SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int)));
+  QSignalSpy after_spy(&merged_, SIGNAL(rowsRemoved(QModelIndex,int,int)));
+
+  submodel.removeRow(0, QModelIndex());
+
+  ASSERT_EQ(1, before_spy.count());
+  ASSERT_EQ(1, after_spy.count());
+  EXPECT_EQ("one", before_spy[0][0].value<QModelIndex>().data());
+  EXPECT_EQ(0, before_spy[0][1].toInt());
+  EXPECT_EQ(0, before_spy[0][2].toInt());
+  EXPECT_EQ("one", after_spy[0][0].value<QModelIndex>().data());
+  EXPECT_EQ(0, after_spy[0][1].toInt());
+  EXPECT_EQ(0, after_spy[0][2].toInt());
+}

tests/test_utils.h

@@ -19,6 +19,9 @@
 
 #include <iostream>
 
+#include <QMetaType>
+#include <QModelIndex>
+
 class QNetworkRequest;
 class QString;
 class QUrl;
@@ -39,4 +42,6 @@ void PrintTo(const ::QVariant& var, std::ostream& os);
 #define EXPOSE_SIGNAL2(n, t1, t2) \
     void Emit##n(const t1& a1, const t2& a2) { emit n(a1, a2); }
 
+Q_DECLARE_METATYPE(QModelIndex);
+
 #endif  // TEST_UTILS_H