CUE songs are now properly updated in library - you can delete a CUE sheet, add it, you can change section markers in it etc. and everything should work as expected

Song now knows it's cue path (if any)

data/data.qrc

@@ -294,6 +294,7 @@
         <file>schema/jamendo.sql</file>
         <file>schema/schema-23.sql</file>
         <file>schema/schema-24.sql</file>
+        <file>schema/schema-25.sql</file>
         <file>pythonstartup.py</file>
     </qresource>
 </RCC>

data/schema/device-schema.sql

@@ -49,7 +49,9 @@ CREATE TABLE device_%deviceid_songs (
 
   skipcount INTEGER NOT NULL DEFAULT 0,
   score INTEGER NOT NULL DEFAULT 0,
-  beginning NOT NULL DEFAULT 0
+  beginning NOT NULL DEFAULT 0,
+
+  cue_path TEXT
 );
 
 CREATE INDEX idx_device_%deviceid_songs_album ON device_%deviceid_songs (album);

data/schema/jamendo.sql

@@ -36,7 +36,9 @@ CREATE TABLE jamendo.songs (
   effective_compilation NOT NULL DEFAULT 0,
   skipcount NOT NULL DEFAULT 0,
   score NOT NULL DEFAULT 0,
-  beginning NOT NULL DEFAULT 0
+  beginning NOT NULL DEFAULT 0,
+
+  cue_path TEXT
 );
 
 CREATE VIRTUAL TABLE jamendo.songs_fts USING fts3(

data/schema/schema-25.sql

@@ -0,0 +1,3 @@
+ALTER TABLE %allsongstables ADD COLUMN cue_path TEXT;
+
+UPDATE schema_version SET version=25;

src/core/database.cpp

@@ -31,7 +31,7 @@
 #include <QVariant>
 
 const char* Database::kDatabaseFilename = "clementine.db";
-const int Database::kSchemaVersion = 24;
+const int Database::kSchemaVersion = 25;
 const char* Database::kMagicAllSongsTables = "%allsongstables";
 
 int Database::sNextConnectionId = 1;

src/core/song.cpp

@@ -93,7 +93,8 @@ const QStringList Song::kColumns = QStringList()
     << "mtime" << "ctime" << "filesize" << "sampler" << "art_automatic"
     << "art_manual" << "filetype" << "playcount" << "lastplayed" << "rating"
     << "forced_compilation_on" << "forced_compilation_off"
-    << "effective_compilation" << "skipcount" << "score" << "beginning" << "length";
+    << "effective_compilation" << "skipcount" << "score" << "beginning" << "length"
+    << "cue_path";
 
 const QString Song::kColumnSpec = Song::kColumns.join(", ");
 const QString Song::kBindSpec = Prepend(":", Song::kColumns).join(", ");
@@ -506,6 +507,8 @@ void Song::InitFromQuery(const SqlRow& q, int col) {
   d->beginning_ = q.value(col + 32).isNull() ? 0 : q.value(col + 32).toInt();
   set_length(toint(col + 33));
 
+  d->cue_path_ = tostr(col + 34);
+
   #undef tostr
   #undef toint
   #undef tofloat
@@ -964,6 +967,8 @@ void Song::BindToQuery(QSqlQuery *query) const {
   query->bindValue(":beginning", d->beginning_);
   query->bindValue(":length", intval(length()));
 
+  query->bindValue(":cue_path", d->cue_path_);
+
   #undef intval
   #undef notnullintval
 }
@@ -1057,7 +1062,8 @@ bool Song::IsMetadataEqual(const Song& other) const {
          d->samplerate_ == other.d->samplerate_ &&
          d->sampler_ == other.d->sampler_ &&
          d->art_automatic_ == other.d->art_automatic_ &&
-         d->art_manual_ == other.d->art_manual_;
+         d->art_manual_ == other.d->art_manual_ &&
+         d->cue_path_ == other.d->cue_path_;
 }
 
 void Song::SetTextFrame(const QString& id, const QString& value,

src/core/song.h

@@ -171,6 +171,9 @@ class Song {
   int lastplayed() const { return d->lastplayed_; }
   int score() const { return d->score_; }
 
+  const QString& cue_path() const { return d->cue_path_; }
+  bool has_cue() const { return !d->cue_path_.isEmpty(); }
+
   int beginning() const { return d->beginning_; }
   int end() const { return d->end_; }
 
@@ -241,6 +244,7 @@ class Song {
   void set_skipcount(int v) { d->skipcount_ = v; }
   void set_lastplayed(int v) { d->lastplayed_ = v; }
   void set_score(int v) { d->score_ = qBound(0, v, 100); }
+  void set_cue_path(const QString& v) { d->cue_path_ = v; }
 
   // Setters that should only be used by tests
   void set_filename(const QString& v) { d->filename_ = v; }
@@ -312,6 +316,9 @@ class Song {
     int filesize_;
     FileType filetype_;
 
+    // If the song has a CUE, this contains it's path.
+    QString cue_path_;
+
     // Filenames to album art for this song.
     QString art_automatic_; // Guessed by LibraryWatcher
     QString art_manual_;    // Set by the user - should take priority

src/library/librarybackend.cpp

@@ -574,6 +574,23 @@ Song LibraryBackend::GetSongByFilename(const QString& filename, int beginning) {
   return song;
 }
 
+SongList LibraryBackend::GetSongsByFilename(const QString& filename) {
+  LibraryQuery query;
+  query.SetColumnSpec("%songs_table.ROWID, " + Song::kColumnSpec);
+  query.AddWhere("filename", filename);
+
+  SongList songlist;
+  if (ExecQuery(&query)) {
+    while(query.Next()) {
+      Song song;
+      song.InitFromQuery(query);
+
+      songlist << song;
+    }
+  }
+  return songlist;
+}
+
 bool LibraryBackend::HasCompilations(const QueryOptions& opt) {
   LibraryQuery query(opt);
   query.SetColumnSpec("%songs_table.ROWID");

src/library/librarybackend.h

@@ -82,6 +82,9 @@ class LibraryBackendInterface : public QObject {
 
   virtual Song GetSongById(int id) = 0;
 
+  // Returns all sections of a song with the given filename. If there's just one section
+  // the resulting list will have it's size equal to 1.
+  virtual SongList GetSongsByFilename(const QString& filename) = 0;
   // Returns a section of a song with the given filename and beginning. If the section
   // is not present in library, returns invalid song.
   // Using default beginning value is suitable when searching for single-section songs.
@@ -140,6 +143,7 @@ class LibraryBackend : public LibraryBackendInterface {
   SongList GetSongsByForeignId(const QStringList& ids, const QString& table,
                                const QString& column);
 
+  SongList GetSongsByFilename(const QString& filename);
   Song GetSongByFilename(const QString& filename, int beginning = 0);
 
   void AddDirectory(const QString& path);

src/library/librarywatcher.cpp

@@ -26,9 +26,10 @@
 #include <QtDebug>
 #include <QThread>
 #include <QDateTime>
-#include <QTimer>
+#include <QHash>
-#include <QSettings>
 #include <QSet>
+#include <QSettings>
+#include <QTimer>
 
 #include <taglib/fileref.h>
 #include <taglib/tag.h>
@@ -286,15 +287,13 @@ void LibraryWatcher::ScanSubdirectory(
   foreach (const QString& file, files_on_disk) {
     if (stop_requested_) return;
 
+    // associated cue
     QString matching_cue = NoExtensionPart(file) + ".cue";
-    QDateTime cue_last_modified = QFileInfo(matching_cue).lastModified();
-
-    uint cue_mtime = cue_last_modified.isValid()
-                         ? cue_last_modified.toTime_t()
-                         : 0;
 
     Song matching_song;
     if (FindSongByPath(songs_in_db, file, &matching_song)) {
+      uint matching_cue_mtime = GetMtimeForCue(matching_cue);
+
       // The song is in the database and still on disk.
       // Check the mtime to see if it's been changed since it was added.
       QFileInfo file_info(file);
@@ -306,8 +305,16 @@ void LibraryWatcher::ScanSubdirectory(
         continue;
       }
 
+      // cue sheet's path from library (if any)
+      QString song_cue = matching_song.cue_path();
+      uint song_cue_mtime = GetMtimeForCue(song_cue);
+
+      bool cue_deleted = song_cue_mtime == 0 &&  matching_song.has_cue();
+      bool cue_added = matching_cue_mtime != 0 && !matching_song.has_cue();
+
       // watch out for cue songs which have their mtime equal to qMax(media_file_mtime, cue_sheet_mtime)
-      bool changed = matching_song.mtime() != qMax(file_info.lastModified().toTime_t(), cue_mtime);
+      bool changed = (matching_song.mtime() != qMax(file_info.lastModified().toTime_t(), song_cue_mtime))
+                      || cue_deleted || cue_added;
 
       // Also want to look to see whether the album art has changed
       QString image = ImageForSong(file, album_art);
@@ -317,71 +324,25 @@ void LibraryWatcher::ScanSubdirectory(
       }
 
       // the song's changed - reread the metadata from file
-      // TODO: problem if cue gets deleted or added before an update
       if (changed) {
         qDebug() << file << "changed";
 
-        // cue associated?
+        // if cue associated...
-        if(cue_mtime) {
+        if(!cue_deleted && (matching_song.has_cue() || cue_added)) {
-          QFile cue(matching_cue);
+          UpdateCueAssociatedSongs(file, path, matching_cue, image, t);
-          cue.open(QIODevice::ReadOnly);
+        // if no cue or it's about to lose it...
-
-          foreach(Song cue_song, cue_parser_->Load(&cue, matching_cue, path)) {
-            // update every song that's in the cue and library
-            Song matching_section = backend_->GetSongByFilename(cue_song.filename(), cue_song.beginning());
-            if(matching_section.is_valid()) {
-              cue_song.set_directory_id(t->dir());
-              PreserveUserSetData(file, image, matching_section, &cue_song, t);
-            }
-          }
         } else {
-          Song song_on_disk;
+          UpdateNonCueAssociatedSong(file, matching_song, image, cue_deleted, t);
-          song_on_disk.InitFromFile(file, t->dir());
-
-          if (!song_on_disk.is_valid())
-            continue;
-
-          PreserveUserSetData(file, image, matching_song, &song_on_disk, t);
         }
       }
     } else {
       // The song is on disk but not in the DB
-      SongList song_list;
+      SongList song_list = ScanNewFile(file, path, matching_cue, &cues_processed);
-
-      // don't process the same cue many times
-      if(cues_processed.contains(matching_cue))
-        continue;
-
-      // it's a cue - create virtual tracks
-      if(cue_mtime) {
-        QFile cue(matching_cue);
-        cue.open(QIODevice::ReadOnly);
-
-        // ignore FILEs pointing to other media files
-        foreach(const Song& cue_song, cue_parser_->Load(&cue, matching_cue, path)) {
-          if(cue_song.filename() == file) {
-            song_list << cue_song;
-          }
-        }
 
       if(song_list.isEmpty()) {
         continue;
       }
 
-        cues_processed << matching_cue;
-
-      // it's a normal media file
-      } else {
-        Song song;
-        song.InitFromFile(file, -1);
-
-        if (!song.is_valid()) {
-          continue;
-        }
-
-        song_list << song;
-      }
-
       qDebug() << file << "created";
       // choose an image for the song(s)
       QString image = ImageForSong(file, album_art);
@@ -426,6 +387,108 @@ void LibraryWatcher::ScanSubdirectory(
   }
 }
 
+void LibraryWatcher::UpdateCueAssociatedSongs(const QString& file, const QString& path,
+                                              const QString& matching_cue, const QString& image,
+                                              ScanTransaction* t) {
+  QFile cue(matching_cue);
+  cue.open(QIODevice::ReadOnly);
+
+  SongList old_sections = backend_->GetSongsByFilename(file);
+
+  QHash<int, Song> sections_map;
+  foreach(const Song& song, old_sections) {
+    if(song.is_valid()) {
+      sections_map[song.beginning()] = song;
+    }
+  }
+
+  QSet<int> used_ids;
+
+  // update every song that's in the cue and library
+  foreach(Song cue_song, cue_parser_->Load(&cue, matching_cue, path)) {
+    cue_song.set_directory_id(t->dir());
+
+    Song matching = sections_map[cue_song.beginning()];
+    // a new section
+    if(!matching.is_valid()) {
+      t->new_songs << cue_song;
+    // changed section
+    } else {
+      PreserveUserSetData(file, image, matching, &cue_song, t);
+      used_ids.insert(matching.id());
+    }
+  }
+
+  // sections that are now missing
+  foreach(const Song& matching, old_sections) {
+    if(!used_ids.contains(matching.id())) {
+      t->deleted_songs << matching;
+    }
+  }
+}
+
+
+void LibraryWatcher::UpdateNonCueAssociatedSong(const QString& file, const Song& matching_song,
+                                                const QString& image, bool cue_deleted,
+                                                ScanTransaction* t) {
+  // if a cue got deleted, we turn it's first section into the new
+  // 'raw' (cueless) song and we just remove the rest of the sections
+  // from the library
+  if(cue_deleted) {
+    foreach(const Song& song, backend_->GetSongsByFilename(file)) {
+      if(!song.IsMetadataEqual(matching_song) && song.is_valid()) {
+        t->deleted_songs << song;
+      }
+    }
+  }
+
+  Song song_on_disk;
+  song_on_disk.InitFromFile(file, t->dir());
+
+  if(song_on_disk.is_valid()) {
+    PreserveUserSetData(file, image, matching_song, &song_on_disk, t);
+  }
+}
+
+SongList LibraryWatcher::ScanNewFile(const QString& file, const QString& path,
+                                     const QString& matching_cue, QSet<QString>* cues_processed) {
+  SongList song_list;
+
+  uint matching_cue_mtime = GetMtimeForCue(matching_cue);
+  // if it's a cue - create virtual tracks
+  if(matching_cue_mtime) {
+    // don't process the same cue many times
+    if(cues_processed->contains(matching_cue))
+      return song_list;
+
+    QFile cue(matching_cue);
+    cue.open(QIODevice::ReadOnly);
+
+    // ignore FILEs pointing to other media files
+    foreach(const Song& cue_song, cue_parser_->Load(&cue, matching_cue, path)) {
+      if(cue_song.filename() == file) {
+        song_list << cue_song;
+      }
+    }
+
+    if(!song_list.isEmpty()) {
+      *cues_processed << matching_cue;
+    }
+
+  // it's a normal media file
+  } else {
+    Song song;
+    song.InitFromFile(file, -1);
+
+    if (song.is_valid()) {
+      song_list << song;
+    }
+
+  }
+
+  return song_list;
+}
+
 void LibraryWatcher::PreserveUserSetData(const QString& file, const QString& image,
                                          const Song& matching_song, Song* out, ScanTransaction* t) {
   out->set_id(matching_song.id());
@@ -449,6 +512,19 @@ void LibraryWatcher::PreserveUserSetData(const QString& file, const QString& ima
   }
 }
 
+uint LibraryWatcher::GetMtimeForCue(const QString& cue_path) {
+  // slight optimisation
+  if(cue_path.isEmpty()) {
+    return 0;
+  }
+
+  QDateTime cue_last_modified = QFileInfo(cue_path).lastModified();
+
+  return cue_last_modified.isValid()
+             ? cue_last_modified.toTime_t()
+             : 0;
+}
+
 void LibraryWatcher::AddWatch(QFileSystemWatcher* w, const QString& path) {
   if (!QFile::exists(path))
     return;

src/library/librarywatcher.h

@@ -133,8 +133,27 @@ class LibraryWatcher : public QObject {
   QString PickBestImage(const QStringList& images);
   QString ImageForSong(const QString& path, QMap<QString, QStringList>& album_art);
   void AddWatch(QFileSystemWatcher* w, const QString& path);
+  uint GetMtimeForCue(const QString& cue_path);
+
+  // Updates the sections of a cue associated and altered (according to mtime)
+  // media file during a scan.
+  void UpdateCueAssociatedSongs(const QString& file, const QString& path,
+                                const QString& matching_cue, const QString& image,
+                                ScanTransaction* t);
+  // Updates a single non-cue associated and altered (according to mtime) song
+  // during a scan.
+  void UpdateNonCueAssociatedSong(const QString& file, const Song& matching_song,
+                                  const QString& image, bool cue_deleted,
+                                  ScanTransaction* t)  ;
+  // Updates a new song with some metadata taken from it's equivalent old 
+  // song (for example rating and score).
   void PreserveUserSetData(const QString& file, const QString& image,
                            const Song& matching_song, Song* out, ScanTransaction* t);
+  // Scans a single media file that's present on the disk but not yet in the library.
+  // It may result in a multiple files added to the library when the media file
+  // has many sections (like a CUE related media file).
+  SongList ScanNewFile(const QString& file, const QString& path,
+                       const QString& matching_cue, QSet<QString>* cues_processed);
 
  private:
   // One of these gets stored for each Directory we're watching

src/playlistparsers/cueparser.cpp

@@ -220,6 +220,7 @@ SongList CueParser::Load(QIODevice* device, const QString& playlist_path, const
       if(cue_mtime.isValid()) {
         song.set_mtime(qMax(cue_mtime.toTime_t(), song.mtime()));
       }
+      song.set_cue_path(playlist_path);
 
       // overwrite the stuff, we may have read from the file or library, using
       // the current .cue metadata

src/scripting/python/librarybackend.sip

@@ -56,6 +56,7 @@ public:
   SongList GetSongsByForeignId(const QStringList& ids, const QString& table,
                                const QString& column);
 
+  SongList GetSongsByFilename(const QString& filename);
   Song GetSongByFilename(const QString& filename, int beginning);
 
   void AddDirectory(const QString& path);

src/scripting/python/song.sip

@@ -55,6 +55,9 @@ public:
   int lastplayed() const;
   int score() const;
 
+  const QString& cue_path() const;
+  bool has_cue() const;
+
   int beginning() const;
   int end() const;
 
@@ -128,6 +131,7 @@ public:
   void set_filename(const QString& v);
   void set_basefilename(const QString& v);
   void set_directory_id(int v);
+  void set_cue_path(const QString& v);
 
   // Comparison functions
   bool IsMetadataEqual(const Song& other) const;

tests/cueparser_test.cpp

@@ -41,7 +41,7 @@ TEST_F(CueParserTest, ParsesASong) {
   QFile file(":testdata/onesong.cue");
   file.open(QIODevice::ReadOnly);
 
-  SongList song_list = parser_.Load(&file, "", QDir(""));
+  SongList song_list = parser_.Load(&file, "CUEPATH", QDir(""));
 
   // one song
   ASSERT_EQ(1, song_list.size());
@@ -54,6 +54,7 @@ TEST_F(CueParserTest, ParsesASong) {
   ASSERT_EQ("", first_song.album());
   ASSERT_EQ(1, first_song.beginning());
   ASSERT_EQ(1, first_song.track());
+  ASSERT_EQ("CUEPATH", first_song.cue_path());
 }
 
 TEST_F(CueParserTest, ParsesTwoSongs) {
@@ -153,7 +154,7 @@ TEST_F(CueParserTest, AcceptsMultipleFileBasedCues) {
   QFile file(":testdata/manyfiles.cue");
   file.open(QIODevice::ReadOnly);
 
-  SongList song_list = parser_.Load(&file, "", QDir(""));
+  SongList song_list = parser_.Load(&file, "CUEPATH", QDir(""));
 
   // five songs
   ASSERT_EQ(5, song_list.size());
@@ -173,6 +174,7 @@ TEST_F(CueParserTest, AcceptsMultipleFileBasedCues) {
   ASSERT_EQ(1, first_song.beginning());
   ASSERT_EQ(second_song.beginning() - first_song.beginning(), first_song.length());
   ASSERT_EQ(-1, first_song.track());
+  ASSERT_EQ("CUEPATH", first_song.cue_path());
 
   ASSERT_TRUE(second_song.filename().endsWith("files/longer_one.mp3"));
   ASSERT_EQ("A1Song2", second_song.title());
@@ -190,6 +192,7 @@ TEST_F(CueParserTest, AcceptsMultipleFileBasedCues) {
   ASSERT_EQ(0, third_song.beginning());
   ASSERT_EQ(fourth_song.beginning() - third_song.beginning(), third_song.length());
   ASSERT_EQ(-1, third_song.track());
+  ASSERT_EQ("CUEPATH", third_song.cue_path());
 
   ASSERT_TRUE(fourth_song.filename().endsWith("files/longer_two_p1.mp3"));
   ASSERT_EQ("A2P1Song2", fourth_song.title());
@@ -206,6 +209,7 @@ TEST_F(CueParserTest, AcceptsMultipleFileBasedCues) {
   ASSERT_EQ("Artist Two", fifth_song.albumartist());
   ASSERT_EQ(1, fifth_song.beginning());
   ASSERT_EQ(-1, fifth_song.track());
+  ASSERT_EQ("CUEPATH", fifth_song.cue_path());
 }
 
 TEST_F(CueParserTest, SkipsBrokenSongsInMultipleFileBasedCues) {

tests/mock_librarybackend.h

@@ -51,6 +51,7 @@ class MockLibraryBackend : public LibraryBackendInterface {
 
   MOCK_METHOD1(GetSongById, Song(int));
 
+  MOCK_METHOD1(GetSongsByFilename, SongList(const QString&));
   MOCK_METHOD2(GetSongByFilename, Song(const QString&, int));
 
   MOCK_METHOD1(AddDirectory, void(const QString&));