Merge pull request #15991 from ronso0/removeFromDisk-dont-delete-curr-history-playlist

(fix) prevent deletion of current History playlist after purging tracks

Author: ywwg

src/library/dao/playlistdao.cpp

@@ -13,7 +13,8 @@
 #include "util/math.h"
 
 PlaylistDAO::PlaylistDAO()
-        : m_pAutoDJProcessor(nullptr) {
+        : m_currentHistoryPlaylist(kInvalidPlaylistId),
+          m_pAutoDJProcessor(nullptr) {
 }
 
 void PlaylistDAO::initialize(const QSqlDatabase& database) {
@@ -277,19 +278,35 @@ bool PlaylistDAO::deleteUnlockedPlaylists(QStringList&& idStringList) {
 }
 
 bool PlaylistDAO::deleteAllUnlockedPlaylistsWithFewerTracks(
-        PlaylistDAO::HiddenType type, int minNumberOfTracks) {
+        PlaylistDAO::HiddenType type,
+        int minNumberOfTracks,
+        bool skipCurrHistory) {
+    // Note: this slot is also called after purging tracks in order to delete
+    // now empty history playlists.
+    // Though, if the current History is now also empty, we must not delete that!
+    // Else the following session log is lost -- or rather not recorded in the
+    // first place since the id passed to appendTrackToPlaylist() does not exist
+    // anymore.
+    // skipCurrHistory prevents that.
     VERIFY_OR_DEBUG_ASSERT(minNumberOfTracks > 0) {
         return false; // nothing to do, probably unintended invocation
     }
 
     QSqlQuery query(m_database);
-    query.prepare(QStringLiteral(
+    QString queryString = QStringLiteral(
             "SELECT id FROM Playlists  "
             "WHERE (SELECT count(playlist_id) FROM PlaylistTracks WHERE "
             "Playlists.ID = PlaylistTracks.playlist_id) < :length AND "
-            "Playlists.hidden = :hidden AND Playlists.locked = 0"));
+            "Playlists.hidden = :hidden AND Playlists.locked = 0");
+    if (skipCurrHistory) {
+        queryString.append(QStringLiteral(" AND Playlists.ID != :currHistoryId"));
+    }
+    query.prepare(queryString);
     query.bindValue(":hidden", static_cast<int>(type));
     query.bindValue(":length", minNumberOfTracks);
+    if (skipCurrHistory) {
+        query.bindValue(":currHistoryId", m_currentHistoryPlaylist);
+    }
     if (!query.exec()) {
         LOG_FAILED_QUERY(query);
         return false;
@@ -299,6 +316,11 @@ bool PlaylistDAO::deleteAllUnlockedPlaylistsWithFewerTracks(
     while (query.next()) {
         idStringList.append(query.value(0).toString());
     }
+
+    if (idStringList.isEmpty()) {
+        return false;
+    }
+
     qInfo() << "Prepared deletion of" << idStringList.size() << "playlists of type" << type
             << "that contain fewer than" << minNumberOfTracks << "tracks";
 
@@ -405,9 +427,29 @@ bool PlaylistDAO::removeTracksFromPlaylist(int playlistId, int startIndex) {
     return true;
 }
 
+bool PlaylistDAO::playlistExists(const int playlistId) const {
+    ScopedTransaction transaction(m_database);
+    QSqlQuery query(m_database);
+    query.prepare(QStringLiteral("SELECT id FROM Playlists WHERE id = :id"));
+    query.bindValue(":id", playlistId);
+
+    if (!query.exec()) {
+        LOG_FAILED_QUERY(query);
+        return false;
+    }
+
+    if (query.next()) {
+        // id is guaranteed to be unique, so we can return here
+        return true;
+    }
+    // not found
+    return false;
+}
+
 bool PlaylistDAO::appendTracksToPlaylist(const QList<TrackId>& trackIds, const int playlistId) {
     // qDebug() << "PlaylistDAO::appendTracksToPlaylist"
     //          << QThread::currentThread() << m_database.connectionName();
+    DEBUG_ASSERT(playlistExists(playlistId));
 
     // Start the transaction
     ScopedTransaction transaction(m_database);
@@ -991,7 +1033,8 @@ void PlaylistDAO::removeTracksFromPlaylists(const QList<TrackId>& trackIds, bool
     transaction.commit();
 
     // We may now have empty history playlists. Remove them.
-    deleteAllUnlockedPlaylistsWithFewerTracks(PlaylistDAO::PLHT_SET_LOG, 1);
+    // Note: does not delete current History playlist.
+    deleteAllUnlockedPlaylistsWithFewerTracks(PlaylistDAO::PLHT_SET_LOG, 1, true);
 
     // update the sidebar
     emit playlistContentChanged(playlistIds);

src/library/dao/playlistdao.h

@@ -47,14 +47,17 @@ class PlaylistDAO : public QObject, public virtual DAO {
     /// Needs to be called inside a transaction.
     /// @return true on success, false on error
     bool deleteAllUnlockedPlaylistsWithFewerTracks(const PlaylistDAO::HiddenType type,
-            int minNumberOfTracks);
+            int minNumberOfTracks,
+            bool skipCurrHistory = false);
     // Rename a playlist
     void renamePlaylist(const int playlistId, const QString& newName);
     // Lock or unlock a playlist
     bool setPlaylistLocked(const int playlistId, const bool locked);
     int setPlaylistsLocked(const QSet<int>& playlistIds, const bool lock);
     // Find out the state of a playlist lock
     bool isPlaylistLocked(const int playlistId) const;
+    // Check if a playlist exists
+    bool playlistExists(const int playlistId) const;
     // Append a list of tracks to a playlist
     bool appendTracksToPlaylist(const QList<TrackId>& trackIds, const int playlistId);
     // Append a track to a playlist
@@ -115,6 +118,10 @@ class PlaylistDAO : public QObject, public virtual DAO {
 
     void getPlaylistsTrackIsIn(TrackId trackId, QSet<int>* playlistSet) const;
 
+    void setCurrentHistoryPlaylistId(int id) {
+        m_currentHistoryPlaylist = id;
+    }
+
     void setAutoDJProcessor(AutoDJProcessor* pAutoDJProcessor);
 
   signals:
@@ -146,6 +153,7 @@ class PlaylistDAO : public QObject, public virtual DAO {
     void populatePlaylistMembershipCache();
 
     QMultiHash<TrackId, int> m_playlistsTrackIsIn;
+    int m_currentHistoryPlaylist;
     AutoDJProcessor* m_pAutoDJProcessor;
     DISALLOW_COPY_AND_ASSIGN(PlaylistDAO);
 };

src/library/trackset/setlogfeature.cpp

@@ -383,6 +383,7 @@ void SetlogFeature::slotGetNewPlaylist() {
                  << set_log_name;
     } else {
         m_recentTracks.clear();
+        m_playlistDao.setCurrentHistoryPlaylistId(m_currentPlaylistId);
     }
 
     // reload child model again because the 'added' signal fired by PlaylistDAO
@@ -447,6 +448,7 @@ void SetlogFeature::slotJoinWithPrevious() {
 
         // Change current setlog
         m_currentPlaylistId = previousPlaylistId;
+        m_playlistDao.setCurrentHistoryPlaylistId(m_currentPlaylistId);
     }
     qDebug() << "slotJoinWithPrevious() current:"
              << clickedPlaylistId