From 88ab9a82991f003e5b8485855afbd1b44fb52050 Mon Sep 17 00:00:00 2001 From: David Sansome Date: Sun, 9 May 2010 16:53:35 +0000 Subject: [PATCH] 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 | 1 + data/schema-8.sql | 40 +++++++++++++++++ src/database.cpp | 2 +- src/librarybackend.cpp | 12 ++--- src/magnatuneservice.cpp | 34 +++++++++++--- src/magnatuneservice.h | 7 ++- src/mergedproxymodel.cpp | 19 +++++--- tests/mergedproxymodel_test.cpp | 79 ++++++++++++++++++++++++++++++++- tests/test_utils.h | 5 +++ 9 files changed, 179 insertions(+), 20 deletions(-) create mode 100644 data/schema-8.sql diff --git a/data/data.qrc b/data/data.qrc index cad3b156c..378a0cfea 100644 --- a/data/data.qrc +++ b/data/data.qrc @@ -86,5 +86,6 @@ edit-redo.png edit-undo.png magnatune.png + schema-8.sql diff --git a/data/schema-8.sql b/data/schema-8.sql new file mode 100644 index 000000000..3e4fe08b1 --- /dev/null +++ b/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; + diff --git a/src/database.cpp b/src/database.cpp index db5f4e338..83fe46eab 100644 --- a/src/database.cpp +++ b/src/database.cpp @@ -27,7 +27,7 @@ #include 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*, diff --git a/src/librarybackend.cpp b/src/librarybackend.cpp index ba165d21a..40172b3e9 100644 --- a/src/librarybackend.cpp +++ b/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) { diff --git a/src/magnatuneservice.cpp b/src/magnatuneservice.cpp index 1cf7c4a58..a8c2d156d 100644 --- a/src/magnatuneservice.cpp +++ b/src/magnatuneservice.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include @@ -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; } diff --git a/src/magnatuneservice.h b/src/magnatuneservice.h index 2fa667d7b..46178299a 100644 --- a/src/magnatuneservice.h +++ b/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_; }; diff --git a/src/mergedproxymodel.cpp b/src/mergedproxymodel.cpp index b20d3df0f..cc7b97827 100644 --- a/src/mergedproxymodel.cpp +++ b/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(sender())), + beginInsertRows(mapFromSource(GetActualSourceParent( + source_parent, static_cast(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(sender())), + beginRemoveRows(mapFromSource(GetActualSourceParent( + source_parent, static_cast(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); } diff --git a/tests/mergedproxymodel_test.cpp b/tests/mergedproxymodel_test.cpp index 6be23e812..cf6bd9bd9 100644 --- a/tests/mergedproxymodel_test.cpp +++ b/tests/mergedproxymodel_test.cpp @@ -19,6 +19,7 @@ #include "mergedproxymodel.h" #include +#include 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().isValid()); + EXPECT_EQ(0, before_spy[0][1].toInt()); + EXPECT_EQ(0, before_spy[0][2].toInt()); + EXPECT_FALSE(after_spy[0][0].value().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().isValid()); + EXPECT_EQ(0, before_spy[0][1].toInt()); + EXPECT_EQ(0, before_spy[0][2].toInt()); + EXPECT_FALSE(after_spy[0][0].value().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().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().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().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().data()); + EXPECT_EQ(0, after_spy[0][1].toInt()); + EXPECT_EQ(0, after_spy[0][2].toInt()); +} diff --git a/tests/test_utils.h b/tests/test_utils.h index 0e48e9d5b..84e5f586f 100644 --- a/tests/test_utils.h +++ b/tests/test_utils.h @@ -19,6 +19,9 @@ #include +#include +#include + 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