From cbf1d96b16287db5824d9f52f6d55c4f3357696b Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Fri, 7 Jun 2019 23:02:43 +0200 Subject: [PATCH] Avoid duplicate songs in the collection backend --- src/collection/collectionbackend.cpp | 129 ++++++++++++++++++++++----- src/collection/collectionbackend.h | 6 ++ 2 files changed, 112 insertions(+), 23 deletions(-) diff --git a/src/collection/collectionbackend.cpp b/src/collection/collectionbackend.cpp index 9b2777235..b8005ce26 100644 --- a/src/collection/collectionbackend.cpp +++ b/src/collection/collectionbackend.cpp @@ -405,6 +405,7 @@ void CollectionBackend::AddOrUpdateSongs(const SongList &songs) { SongList deleted_songs; for (const Song &song : 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 CollectionWatcher is scanning it. if (!dirs_table_.isEmpty()) { @@ -415,28 +416,8 @@ void CollectionBackend::AddOrUpdateSongs(const SongList &songs) { if (!check_dir.next()) continue; // Directory didn't exist } - if (song.id() == -1) { - // Create + if (song.id() != -1) { // This song exists in the DB. - // Insert the row and create a new ID - song.BindToQuery(&add_song); - add_song.exec(); - if (db_->CheckErrors(add_song)) continue; - - // Get the new ID - const int id = add_song.lastInsertId().toInt(); - - // Add to the FTS index - add_song_fts.bindValue(":id", id); - song.BindToFtsQuery(&add_song_fts); - add_song_fts.exec(); - if (db_->CheckErrors(add_song_fts)) continue; - - Song copy(song); - copy.set_id(id); - added_songs << copy; - } - else { // Get the previous song data first Song old_song(GetSongById(song.id())); if (!old_song.is_valid()) continue; @@ -454,7 +435,59 @@ void CollectionBackend::AddOrUpdateSongs(const SongList &songs) { deleted_songs << old_song; added_songs << song; + + continue; + } + else if (song.song_id() != -1) { // Song has a unique id, check if the song exists. + + // Get the previous song data first + Song old_song(GetSongBySongId(song.song_id())); + + if (old_song.is_valid() && old_song.id() != -1) { + + Song new_song = song; + new_song.set_id(old_song.id()); + // Update + new_song.BindToQuery(&update_song); + update_song.bindValue(":id", new_song.id()); + update_song.exec(); + if (db_->CheckErrors(update_song)) continue; + + new_song.BindToFtsQuery(&update_song_fts); + update_song_fts.bindValue(":id", new_song.id()); + update_song_fts.exec(); + if (db_->CheckErrors(update_song_fts)) continue; + + deleted_songs << old_song; + added_songs << new_song; + + continue; + + } + + } + + // Create new song + + // Insert the row and create a new ID + song.BindToQuery(&add_song); + add_song.exec(); + if (db_->CheckErrors(add_song)) continue; + + // Get the new ID + const int id = add_song.lastInsertId().toInt(); + + // Add to the FTS index + add_song_fts.bindValue(":id", id); + song.BindToFtsQuery(&add_song_fts); + add_song_fts.exec(); + if (db_->CheckErrors(add_song_fts)) continue; + + Song copy(song); + copy.set_id(id); + added_songs << copy; + } transaction.Commit(); @@ -571,7 +604,7 @@ QStringList CollectionBackend::GetAllArtistsWithAlbums(const QueryOptions &opt) query.AddCompilationRequirement(false); query.AddWhere("album", "", "!="); - // Albums with no 'albumartist' (extract 'artist'): + // Albums with no 'albumartist' (extract 'artist'): CollectionQuery query2(opt); query2.SetColumnSpec("DISTINCT artist"); query2.AddCompilationRequirement(false); @@ -744,6 +777,57 @@ SongList CollectionBackend::GetSongsByUrl(const QUrl &url) { } + +Song CollectionBackend::GetSongBySongId(int song_id) { + QMutexLocker l(db_->Mutex()); + QSqlDatabase db(db_->Connect()); + return GetSongBySongId(song_id, db); +} + +SongList CollectionBackend::GetSongsBySongId(const QList &song_ids) { + QMutexLocker l(db_->Mutex()); + QSqlDatabase db(db_->Connect()); + + QStringList str_song_ids; + for (int song_id : song_ids) { + str_song_ids << QString::number(song_id); + } + + return GetSongsBySongId(str_song_ids, db); +} + +SongList CollectionBackend::GetSongsBySongId(const QStringList &song_ids) { + QMutexLocker l(db_->Mutex()); + QSqlDatabase db(db_->Connect()); + + return GetSongsBySongId(song_ids, db); +} + +Song CollectionBackend::GetSongBySongId(int song_id, QSqlDatabase &db) { + SongList list = GetSongsBySongId(QStringList() << QString::number(song_id), db); + if (list.isEmpty()) return Song(); + return list.first(); +} + +SongList CollectionBackend::GetSongsBySongId(const QStringList &song_ids, QSqlDatabase &db) { + + QString in = song_ids.join(","); + + QSqlQuery q(db); + q.prepare(QString("SELECT ROWID, " + Song::kColumnSpec + " FROM %1 WHERE SONG_ID IN (%2)").arg(songs_table_, in)); + q.exec(); + if (db_->CheckErrors(q)) return SongList(); + + SongList ret; + while (q.next()) { + Song song; + song.InitFromQuery(q, true); + ret << song; + } + return ret; + +} + CollectionBackend::AlbumList CollectionBackend::GetCompilationAlbums(const QueryOptions &opt) { return GetAlbums(QString(), true, opt); } @@ -1143,4 +1227,3 @@ void CollectionBackend::DeleteAll() { emit DatabaseReset(); } - diff --git a/src/collection/collectionbackend.h b/src/collection/collectionbackend.h index f728e9fba..9724044a1 100644 --- a/src/collection/collectionbackend.h +++ b/src/collection/collectionbackend.h @@ -178,6 +178,12 @@ class CollectionBackend : public CollectionBackendInterface { void DeleteAll(); + Song GetSongBySongId(int song_id); + SongList GetSongsBySongId(const QList &song_ids); + SongList GetSongsBySongId(const QStringList &song_ids); + Song GetSongBySongId(int song_id, QSqlDatabase &db); + SongList GetSongsBySongId(const QStringList &song_ids, QSqlDatabase &db); + public slots: void LoadDirectories(); void UpdateTotalSongCount();