Fix updating compilations

Fixes #288

src/collection/collectionbackend.cpp

@@ -904,28 +904,30 @@ void CollectionBackend::UpdateCompilations() {
   while (q.next()) {
     QString artist = q.value(0).toString();
     QString album = q.value(1).toString();
-    QString filename = q.value(2).toString();
+    QUrl url = QUrl::fromEncoded(q.value(2).toString().toUtf8());
     bool compilation_detected = q.value(3).toBool();
 
     // Ignore songs that don't have an album field set
     if (album.isEmpty()) continue;
 
     // Find the directory the song is in
-    int last_separator = filename.lastIndexOf('/');
+    QString directory = url.toString(QUrl::PreferLocalFile|QUrl::RemoveFilename|QUrl::StripTrailingSlash);
-    if (last_separator == -1) continue;
 
-    CompilationInfo &info = compilation_info[album];
+    CompilationInfo &info = compilation_info[directory + album];
+    info.urls << url;
+    info.directory = directory;
+    info.album = album;
     info.artists.insert(artist);
-    info.directories.insert(filename.left(last_separator));
+    if (compilation_detected) info.has_compilation_detected++;
-    if (compilation_detected) info.has_compilation_detected = true;
+    else info.has_not_compilation_detected++;
-    else info.has_not_compilation_detected = true;
   }
 
   // Now mark the songs that we think are in compilations
-  QSqlQuery update(db);
-  update.prepare(QString("UPDATE %1 SET compilation_detected = :compilation_detected, compilation_effective = ((compilation OR :compilation_detected OR compilation_on) AND NOT compilation_off) + 0 WHERE album = :album AND unavailable = 0").arg(songs_table_));
   QSqlQuery find_songs(db);
-  find_songs.prepare(QString("SELECT ROWID, " + Song::kColumnSpec + " FROM %1 WHERE album = :album AND compilation_detected = :compilation_detected AND unavailable = 0").arg(songs_table_));
+  find_songs.prepare(QString("SELECT ROWID, " + Song::kColumnSpec + " FROM %1 WHERE url = :url AND compilation_detected = :compilation_detected AND unavailable = 0").arg(songs_table_));
+
+  QSqlQuery update_songs(db);
+  update_songs.prepare(QString("UPDATE %1 SET compilation_detected = :compilation_detected, compilation_effective = ((compilation OR :compilation_detected OR compilation_on) AND NOT compilation_off) + 0 WHERE url = :url AND unavailable = 0").arg(songs_table_));
 
   SongList deleted_songs;
   SongList added_songs;
@@ -935,17 +937,18 @@ void CollectionBackend::UpdateCompilations() {
   QMap<QString, CompilationInfo>::const_iterator it = compilation_info.constBegin();
   for (; it != compilation_info.constEnd(); ++it) {
     const CompilationInfo &info = it.value();
-    QString album(it.key());
 
     // If there were more 'effective album artists' than there were directories for this album then it's a compilation.
 
-    if (info.artists.count() > info.directories.count()) {
+    for (const QUrl &url : info.urls) {
-      if (info.has_not_compilation_detected)
+      if (info.artists.count() > 1) {  // This directory+album is a compilation.
-        UpdateCompilations(find_songs, update, deleted_songs, added_songs, album, 1);
+        if (info.has_not_compilation_detected > 0)  // Run updates if any of the songs is not marked as compilations.
-    }
+          UpdateCompilations(find_songs, update_songs, deleted_songs, added_songs, url, true);
-    else {
+      }
-      if (info.has_compilation_detected)
+      else {
-        UpdateCompilations(find_songs, update, deleted_songs, added_songs, album, 0);
+        if (info.has_compilation_detected > 0)
+          UpdateCompilations(find_songs, update_songs, deleted_songs, added_songs, url, false);
+      }
     }
   }
 
@@ -955,27 +958,28 @@ void CollectionBackend::UpdateCompilations() {
     emit SongsDeleted(deleted_songs);
     emit SongsDiscovered(added_songs);
   }
+
 }
 
-void CollectionBackend::UpdateCompilations(QSqlQuery &find_songs, QSqlQuery &update, SongList &deleted_songs, SongList &added_songs, const QString &album, int compilation_detected) {
+void CollectionBackend::UpdateCompilations(QSqlQuery &find_songs, QSqlQuery &update_songs, SongList &deleted_songs, SongList &added_songs, const QUrl &url, const bool compilation_detected) {
 
-  // Get songs that were already in that album, so we can tell the model they've been updated
+  // Get song, so we can tell the model its updated
-  find_songs.bindValue(":album", album);
+  find_songs.bindValue(":url", url.toString());
   find_songs.bindValue(":compilation_detected", int(!compilation_detected));
   find_songs.exec();
   while (find_songs.next()) {
     Song song;
     song.InitFromQuery(find_songs, true);
     deleted_songs << song;
-    song.set_compilation_detected(true);
+    song.set_compilation_detected(compilation_detected);
     added_songs << song;
   }
 
-  // Mark this album
+  // Update the song
-  update.bindValue(":compilation_detected", compilation_detected);
+  update_songs.bindValue(":compilation_detected", int(compilation_detected));
-  update.bindValue(":album", album);
+  update_songs.bindValue(":url", url);
-  update.exec();
+  update_songs.exec();
-  db_->CheckErrors(update);
+  db_->CheckErrors(update_songs);
 
 }
 

src/collection/collectionbackend.h

@@ -225,16 +225,18 @@ class CollectionBackend : public CollectionBackendInterface {
 
  private:
   struct CompilationInfo {
-    CompilationInfo() : has_compilation_detected(false), has_not_compilation_detected(false) {}
+    CompilationInfo() : has_compilation_detected(0), has_not_compilation_detected(0) {}
 
+    QString directory;
+    QString album;
+    QList<QUrl> urls;
     QSet<QString> artists;
-    QSet<QString> directories;
 
-    bool has_compilation_detected;
+    int has_compilation_detected;
-    bool has_not_compilation_detected;
+    int has_not_compilation_detected;
   };
 
-  void UpdateCompilations(QSqlQuery &find_songs, QSqlQuery &update, SongList &deleted_songs, SongList &added_songs, const QString &album, int compilation_detected);
+  void UpdateCompilations(QSqlQuery &find_songs, QSqlQuery &update_songs, SongList &deleted_songs, SongList &added_songs, const QUrl &url, const bool compilation_detected);
   AlbumList GetAlbums(const QString &artist, const QString &album_artist, bool compilation = false, const QueryOptions &opt = QueryOptions());
   AlbumList GetAlbums(const QString &artist, bool compilation, const QueryOptions &opt = QueryOptions());
   SubdirectoryList SubdirsInDirectory(int id, QSqlDatabase &db);

src/collection/collectionmodel.cpp

@@ -449,7 +449,6 @@ void CollectionModel::SongsDeleted(const SongList &songs) {
           break;
         }
       }
-
       beginRemoveRows(idx, node->row, node->row);
       node->parent->Delete(node->row);
       song_nodes_.remove(song.id());

src/core/song.cpp

@@ -304,7 +304,7 @@ int Song::year() const { return d->year_; }
 int Song::originalyear() const { return d->originalyear_; }
 int Song::effective_originalyear() const { return d->originalyear_ < 0 ? d->year_ : d->originalyear_; }
 const QString &Song::genre() const { return d->genre_; }
-bool Song::is_compilation() const { return (d->compilation_ || d->compilation_detected_ || d->compilation_on_) && ! d->compilation_off_; }
+bool Song::compilation() const { return d->compilation_; }
 const QString &Song::composer() const { return d->composer_; }
 const QString &Song::performer() const { return d->performer_; }
 const QString &Song::grouping() const { return d->grouping_; }
@@ -332,6 +332,10 @@ int Song::playcount() const { return d->playcount_; }
 int Song::skipcount() const { return d->skipcount_; }
 int Song::lastplayed() const { return d->lastplayed_; }
 
+bool Song::compilation_detected() const { return d->compilation_detected_; }
+bool Song::compilation_off() const { return d->compilation_off_; }
+bool Song::compilation_on() const { return d->compilation_on_; }
+
 const QUrl &Song::art_automatic() const { return d->art_automatic_; }
 const QUrl &Song::art_manual() const { return d->art_manual_; }
 bool Song::has_manually_unset_cover() const { return d->art_manual_.path() == kManuallyUnsetCover; }
@@ -350,6 +354,7 @@ bool Song::is_collection_song() const { return d->source_ == Source_Collection;
 bool Song::is_metadata_good() const { return !d->title_.isEmpty() && !d->album_.isEmpty() && !d->artist_.isEmpty() && !d->url_.isEmpty() && d->end_ > 0; }
 bool Song::is_stream() const { return d->source_ == Source_Stream || d->source_ == Source_Tidal || d->source_ == Source_Subsonic || d->source_ == Source_Qobuz; }
 bool Song::is_cdda() const { return d->source_ == Source_CDDA; }
+bool Song::is_compilation() const { return (d->compilation_ || d->compilation_detected_ || d->compilation_on_) && !d->compilation_off_; }
 
 bool Song::art_automatic_is_valid() const {
   return (

src/core/song.h

@@ -200,7 +200,7 @@ class Song {
   int year() const;
   int originalyear() const;
   const QString &genre() const;
-  bool is_compilation() const;
+  bool compilation() const;
   const QString &composer() const;
   const QString &performer() const;
   const QString &grouping() const;
@@ -232,6 +232,10 @@ class Song {
   int skipcount() const;
   int lastplayed() const;
 
+  bool compilation_detected() const;
+  bool compilation_off() const;
+  bool compilation_on() const;
+
   const QUrl &art_automatic() const;
   const QUrl &art_manual() const;
 
@@ -249,6 +253,7 @@ class Song {
   bool is_metadata_good() const;
   bool art_automatic_is_valid() const;
   bool art_manual_is_valid() const;
+  bool is_compilation() const;
 
   // Playlist views are special because you don't want to fill in album artists automatically for compilations, but you do for normal albums:
   const QString &playlist_albumartist() const;

src/dialogs/edittagdialog.cpp

@@ -914,9 +914,7 @@ void EditTagDialog::SongSaveComplete(TagReaderReply *reply, const QString &filen
     emit Error(message);
   }
   else if (song.directory_id() != -1) {
-    SongList songs;
+    app_->collection_backend()->AddOrUpdateSongs(SongList() << song);
-    songs << song;
-    app_->collection_backend()->AddOrUpdateSongs(songs);
   }
 
   if (pending_ <= 0) AcceptFinished();