Merge pull request #2393 from uklotzde/purge_tracks_from_playlists

lp1845837: Speed up purging of tracks

src/library/baseplaylistfeature.cpp

@@ -74,29 +74,45 @@ BasePlaylistFeature::BasePlaylistFeature(QObject* parent,
             this, SLOT(slotExportTrackFiles()));
 
     m_pAnalyzePlaylistAction = new QAction(tr("Analyze entire Playlist"), this);
-    connect(m_pAnalyzePlaylistAction, SIGNAL(triggered()),
-            this, SLOT(slotAnalyzePlaylist()));
-
-    connect(&m_playlistDao, SIGNAL(added(int)),
-            this, SLOT(slotPlaylistTableChanged(int)));
-
-    connect(&m_playlistDao, SIGNAL(deleted(int)),
-            this, SLOT(slotPlaylistTableChanged(int)));
-
-    connect(&m_playlistDao, SIGNAL(renamed(int,QString)),
-            this, SLOT(slotPlaylistTableRenamed(int,QString)));
-
-    connect(&m_playlistDao, SIGNAL(changed(int)),
-            this, SLOT(slotPlaylistContentChanged(int)));
-
-    connect(&m_playlistDao, SIGNAL(lockChanged(int)),
-            this, SLOT(slotPlaylistTableChanged(int)));
+    connect(m_pAnalyzePlaylistAction,
+            &QAction::triggered,
+            this,
+            &BasePlaylistFeature::slotAnalyzePlaylist);
+
+    connect(&m_playlistDao,
+            &PlaylistDAO::added,
+            this,
+            &BasePlaylistFeature::slotPlaylistTableChanged);
+
+    connect(&m_playlistDao,
+            &PlaylistDAO::deleted,
+            this,
+            &BasePlaylistFeature::slotPlaylistTableChanged);
+
+    connect(&m_playlistDao,
+            &PlaylistDAO::renamed,
+            this,
+            &BasePlaylistFeature::slotPlaylistTableRenamed);
+
+    connect(&m_playlistDao,
+            &PlaylistDAO::tracksChanged,
+            this,
+            &BasePlaylistFeature::slotPlaylistContentChanged);
+
+    connect(&m_playlistDao,
+            &PlaylistDAO::lockChanged,
+            this,
+            &BasePlaylistFeature::slotPlaylistTableChanged);
 
     Library* pLibrary = static_cast<Library*>(parent);
-    connect(pLibrary, SIGNAL(trackSelected(TrackPointer)),
-            this, SLOT(slotTrackSelected(TrackPointer)));
-    connect(pLibrary, SIGNAL(switchToView(const QString&)),
-            this, SLOT(slotResetSelectedTrack()));
+    connect(pLibrary,
+            &Library::trackSelected,
+            this,
+            &BasePlaylistFeature::slotTrackSelected);
+    connect(pLibrary,
+            &Library::switchToView,
+            this,
+            &BasePlaylistFeature::slotResetSelectedTrack);
 }
 
 BasePlaylistFeature::~BasePlaylistFeature() {

src/library/baseplaylistfeature.h

@@ -47,8 +47,8 @@ class BasePlaylistFeature : public LibraryFeature {
     virtual void htmlLinkClicked(const QUrl& link);
 
     virtual void slotPlaylistTableChanged(int playlistId) = 0;
-    virtual void slotPlaylistContentChanged(int playlistId) = 0;
-    virtual void slotPlaylistTableRenamed(int playlistId, QString a_strName) = 0;
+    virtual void slotPlaylistContentChanged(QSet<int> playlistIds) = 0;
+    virtual void slotPlaylistTableRenamed(int playlistId, QString newName) = 0;
     void slotCreatePlaylist();
 
   protected slots:

src/library/dao/playlistdao.cpp

@@ -243,7 +243,7 @@ bool PlaylistDAO::isPlaylistLocked(const int playlistId) const {
     return false;
 }
 
-bool PlaylistDAO::removeTracksFromPlaylist(const int playlistId, const int startIndex) {
+bool PlaylistDAO::removeTracksFromPlaylist(int playlistId, int startIndex) {
     // Retain the first track if it is loaded in a deck
     ScopedTransaction transaction(m_database);
     QSqlQuery query(m_database);
@@ -256,7 +256,7 @@ bool PlaylistDAO::removeTracksFromPlaylist(const int playlistId, const int start
         return false;
     }
     transaction.commit();
-    emit(changed(playlistId));
+    emit tracksChanged(QSet<int>{playlistId});
     return true;
 }
 
@@ -298,7 +298,7 @@ bool PlaylistDAO::appendTracksToPlaylist(const QList<TrackId>& trackIds, const i
         // TODO(XXX) don't emit if the track didn't add successfully.
         emit(trackAdded(playlistId, trackId, insertPosition++));
     }
-    emit(changed(playlistId));
+    emit tracksChanged(QSet<int>{playlistId});
     return true;
 }
 
@@ -406,18 +406,23 @@ void PlaylistDAO::removeHiddenTracks(const int playlistId) {
     }
 
     while (query.next()) {
-    	int position = query.value(query.record().indexOf("position")).toInt();
-    	removeTracksFromPlaylistInner(playlistId, position);
+        int position = query.value(query.record().indexOf("position")).toInt();
+        removeTracksFromPlaylistInner(playlistId, position);
     }
 
     transaction.commit();
-    emit(changed(playlistId));
+    emit tracksChanged(QSet<int>{playlistId});
 }
 
 
-void PlaylistDAO::removeTrackFromPlaylist(const int playlistId, const TrackId& trackId) {
+void PlaylistDAO::removeTracksFromPlaylistById(int playlistId, TrackId trackId) {
     ScopedTransaction transaction(m_database);
+    removeTracksFromPlaylistByIdInner(playlistId, trackId);
+    transaction.commit();
+    emit tracksChanged(QSet<int>{playlistId});
+}
 
+void PlaylistDAO::removeTracksFromPlaylistByIdInner(int playlistId, TrackId trackId) {
     QSqlQuery query(m_database);
     query.prepare("SELECT position FROM PlaylistTracks WHERE playlist_id=:id "
                 "AND track_id=:track_id");
@@ -431,38 +436,33 @@ void PlaylistDAO::removeTrackFromPlaylist(const int playlistId, const TrackId& t
     }
 
     while (query.next()) {
-    	int position = query.value(query.record().indexOf("position")).toInt();
+        int position = query.value(query.record().indexOf("position")).toInt();
         removeTracksFromPlaylistInner(playlistId, position);
-
     }
-
-    transaction.commit();
-    emit(changed(playlistId));
 }
 
 
-void PlaylistDAO::removeTrackFromPlaylist(const int playlistId, const int position) {
+void PlaylistDAO::removeTrackFromPlaylist(int playlistId, int position) {
     // qDebug() << "PlaylistDAO::removeTrackFromPlaylist"
     //          << QThread::currentThread() << m_database.connectionName();
     ScopedTransaction transaction(m_database);
     removeTracksFromPlaylistInner(playlistId, position);
     transaction.commit();
-    emit(changed(playlistId));
+    emit tracksChanged(QSet<int>{playlistId});
 }
 
-void PlaylistDAO::removeTracksFromPlaylist(const int playlistId, QList<int>& positions) {
+void PlaylistDAO::removeTracksFromPlaylist(int playlistId, QList<int> positions) {
     // get positions in reversed order
     qSort(positions.begin(), positions.end(), qGreater<int>());
 
     //qDebug() << "PlaylistDAO::removeTrackFromPlaylist"
     //         << QThread::currentThread() << m_database.connectionName();
     ScopedTransaction transaction(m_database);
-    QSqlQuery query(m_database);
-    foreach (int position , positions) {
-    	removeTracksFromPlaylistInner(playlistId, position);
+    for (const auto position : qAsConst(positions)) {
+        removeTracksFromPlaylistInner(playlistId, position);
     }
     transaction.commit();
-    emit(changed(playlistId));
+    emit tracksChanged(QSet<int>{playlistId});
 }
 
 void PlaylistDAO::removeTracksFromPlaylistInner(int playlistId, int position) {
@@ -550,7 +550,7 @@ bool PlaylistDAO::insertTrackIntoPlaylist(TrackId trackId, const int playlistId,
 
     m_playlistsTrackIsIn.insert(trackId, playlistId);
     emit(trackAdded(playlistId, trackId, position));
-    emit(changed(playlistId));
+    emit tracksChanged(QSet<int>{playlistId});
     return true;
 }
 
@@ -611,7 +611,7 @@ int PlaylistDAO::insertTracksIntoPlaylist(const QList<TrackId>& trackIds,
         // TODO(XXX) The position is wrong if any track failed to insert.
         emit(trackAdded(playlistId, trackId, insertPositon++));
     }
-    emit(changed(playlistId));
+    emit tracksChanged(QSet<int>{playlistId});
     return tracksAdded;
 }
 
@@ -724,7 +724,7 @@ bool PlaylistDAO::copyPlaylistTracks(const int sourcePlaylistID, const int targe
         m_playlistsTrackIsIn.insert(copiedTrackId, targetPlaylistID);
         emit(trackAdded(targetPlaylistID, copiedTrackId, copiedPosition));
     }
-    emit(changed(targetPlaylistID));
+    emit tracksChanged(QSet<int>{targetPlaylistID});
     return true;
 }
 
@@ -750,14 +750,22 @@ int PlaylistDAO::getMaxPosition(const int playlistId) const {
 void PlaylistDAO::removeTracksFromPlaylists(const QList<TrackId>& trackIds) {
     // copy the hash, because there is no guarantee that "it" is valid after remove
     QMultiHash<TrackId, int> playlistsTrackIsInCopy = m_playlistsTrackIsIn;
-    for (const auto& trackId: trackIds) {
+    QSet<int> playlistIds;
+
+    ScopedTransaction transaction(m_database);
+    for (const auto& trackId : trackIds) {
         for (auto it = playlistsTrackIsInCopy.constBegin();
                 it != playlistsTrackIsInCopy.constEnd(); ++it) {
             if (it.key() == trackId) {
-                removeTrackFromPlaylist(it.value(), trackId);
+                const auto playlistId = it.value();
+                removeTracksFromPlaylistByIdInner(playlistId, trackId);
+                playlistIds.insert(playlistId);
             }
         }
     }
+    transaction.commit();
+
+    emit tracksChanged(playlistIds);
 }
 
 int PlaylistDAO::tracksInPlaylist(const int playlistId) const {
@@ -830,7 +838,7 @@ void PlaylistDAO::moveTrack(const int playlistId, const int oldPosition, const i
         qDebug() << query.lastError();
     }
 
-    emit(changed(playlistId));
+    emit tracksChanged(QSet<int>{playlistId});
 }
 
 void PlaylistDAO::searchForDuplicateTrack(const int fromPosition,
@@ -994,7 +1002,7 @@ void PlaylistDAO::shuffleTracks(const int playlistId, const QList<int>& position
     }
 
     transaction.commit();
-    emit(changed(playlistId));
+    emit tracksChanged(QSet<int>{playlistId});
 }
 
 bool PlaylistDAO::isTrackInPlaylist(TrackId trackId, const int playlistId) const {

src/library/dao/playlistdao.h

@@ -91,9 +91,9 @@ class PlaylistDAO : public QObject, public virtual DAO {
     // removes all hidden and purged Tracks from the playlist
     void removeHiddenTracks(const int playlistId);
     // Remove a track from a playlist
-    void removeTrackFromPlaylist(const int playlistId, const TrackId& trackId);
-    void removeTrackFromPlaylist(const int playlistId, const int position);
-    void removeTracksFromPlaylist(const int playlistId, QList<int>& positions);
+    void removeTrackFromPlaylist(int playlistId, int position);
+    void removeTracksFromPlaylist(int playlistId, QList<int> positions);
+    void removeTracksFromPlaylistById(int playlistId, TrackId trackId);
     // Insert a track into a specific position in a playlist
     bool insertTrackIntoPlaylist(TrackId trackId, int playlistId, int position);
     // Inserts a list of tracks into playlist
@@ -124,15 +124,16 @@ class PlaylistDAO : public QObject, public virtual DAO {
   signals:
     void added(int playlistId);
     void deleted(int playlistId);
-    void changed(int playlistId);
+    void renamed(int playlistId, QString newName);
+    void lockChanged(int playlistId);
     void trackAdded(int playlistId, TrackId trackId, int position);
     void trackRemoved(int playlistId, TrackId trackId, int position);
-    void renamed(int playlistId, QString a_strName);
-    void lockChanged(int playlistId);
+    void tracksChanged(QSet<int> playlistIds); // added/removed/reordered
 
   private:
-    bool removeTracksFromPlaylist(const int playlistId, const int startIndex);
+    bool removeTracksFromPlaylist(int playlistId, int startIndex);
     void removeTracksFromPlaylistInner(int playlistId, int position);
+    void removeTracksFromPlaylistByIdInner(int playlistId, TrackId trackId);
     void searchForDuplicateTrack(const int fromPosition,
                                  const int toPosition,
                                  TrackId trackID,

src/library/playlistfeature.cpp

@@ -232,23 +232,24 @@ void PlaylistFeature::slotPlaylistTableChanged(int playlistId) {
     }
 }
 
-void PlaylistFeature::slotPlaylistContentChanged(int playlistId) {
+void PlaylistFeature::slotPlaylistContentChanged(QSet<int> playlistIds) {
     if (!m_pPlaylistTableModel) {
         return;
     }
 
-    //qDebug() << "slotPlaylistContentChanged() playlistId:" << playlistId;
-    enum PlaylistDAO::HiddenType type = m_playlistDao.getHiddenType(playlistId);
-    if (type == PlaylistDAO::PLHT_NOT_HIDDEN ||
-        type == PlaylistDAO::PLHT_UNKNOWN) { // In case of a deleted Playlist
-        updateChildModel(playlistId);
+    for (const auto playlistId : qAsConst(playlistIds)) {
+        enum PlaylistDAO::HiddenType type = m_playlistDao.getHiddenType(playlistId);
+        if (type == PlaylistDAO::PLHT_NOT_HIDDEN ||
+            type == PlaylistDAO::PLHT_UNKNOWN) { // In case of a deleted Playlist
+            updateChildModel(playlistId);
+        }
     }
 }
 
-
-
-void PlaylistFeature::slotPlaylistTableRenamed(int playlistId,
-                                               QString /* a_strName */) {
+void PlaylistFeature::slotPlaylistTableRenamed(
+        int playlistId,
+        QString newName) {
+    Q_UNUSED(newName);
     if (!m_pPlaylistTableModel) {
         return;
     }

src/library/playlistfeature.h

@@ -35,9 +35,9 @@ class PlaylistFeature : public BasePlaylistFeature {
     void onRightClickChild(const QPoint& globalPos, QModelIndex index);
 
   private slots:
-    void slotPlaylistTableChanged(int playlistId);
-    void slotPlaylistContentChanged(int playlistId);
-    void slotPlaylistTableRenamed(int playlistId, QString a_strName);
+    void slotPlaylistTableChanged(int playlistId) override;
+    void slotPlaylistContentChanged(QSet<int> playlistIds) override;
+    void slotPlaylistTableRenamed(int playlistId, QString newName) override;
 
  protected:
     QList<BasePlaylistFeature::IdAndLabel> createPlaylistLabels() override;

src/library/playlisttablemodel.cpp

@@ -68,8 +68,10 @@ void PlaylistTableModel::setTableModel(int playlistId) {
     setDefaultSort(fieldIndex(ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_POSITION), Qt::AscendingOrder);
     setSort(defaultSortColumn(), defaultSortOrder());
 
-    connect(&m_pTrackCollection->getPlaylistDAO(), SIGNAL(changed(int)),
-            this, SLOT(playlistChanged(int)));
+    connect(&m_pTrackCollection->getPlaylistDAO(),
+            &PlaylistDAO::tracksChanged,
+            this,
+            &PlaylistTableModel::playlistsChanged);
 }
 
 int PlaylistTableModel::addTracks(const QModelIndex& index,
@@ -130,7 +132,9 @@ void PlaylistTableModel::removeTracks(const QModelIndexList& indices) {
         trackPositions.append(trackPosition);
     }
 
-    m_pTrackCollection->getPlaylistDAO().removeTracksFromPlaylist(m_iPlaylistId, trackPositions);
+    m_pTrackCollection->getPlaylistDAO().removeTracksFromPlaylist(
+            m_iPlaylistId,
+            std::move(trackPositions));
 }
 
 void PlaylistTableModel::moveTrack(const QModelIndex& sourceIndex,
@@ -259,8 +263,8 @@ TrackModel::CapabilitiesFlags PlaylistTableModel::getCapabilities() const {
     return caps;
 }
 
-void PlaylistTableModel::playlistChanged(int playlistId) {
-    if (playlistId == m_iPlaylistId) {
+void PlaylistTableModel::playlistsChanged(QSet<int> playlistIds) {
+    if (playlistIds.contains(m_iPlaylistId)) {
         select(); // Repopulate the data model.
     }
 }

src/library/playlisttablemodel.h

@@ -33,7 +33,7 @@ class PlaylistTableModel : public BaseSqlTableModel {
     CapabilitiesFlags getCapabilities() const final;
 
   private slots:
-    void playlistChanged(int playlistId);
+    void playlistsChanged(QSet<int> playlistIds);
 
   private:
     int m_iPlaylistId;