From 48bd5020cd91323c402b8b8e583146ceecaaa985 Mon Sep 17 00:00:00 2001
From: David Sansome <davidsansome@gmail.com>
Date: Tue, 27 Apr 2010 17:37:26 +0000
Subject: [PATCH] Don't crash when editing a song that doesn't exist any more,
 also don't even add deleted songs to the playlist.  Fixes issue #253.

---
 src/librarybackend.cpp      | 4 ++--
 src/libraryplaylistitem.cpp | 4 +++-
 src/libraryplaylistitem.h   | 2 +-
 src/playlistitem.h          | 2 +-
 src/radioplaylistitem.cpp   | 3 ++-
 src/radioplaylistitem.h     | 2 +-
 src/song.cpp                | 3 +++
 src/songplaylistitem.cpp    | 6 +++++-
 src/songplaylistitem.h      | 2 +-
 tests/mock_playlistitem.h   | 2 +-
 10 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/src/librarybackend.cpp b/src/librarybackend.cpp
index 47a0fdb58..3733dbb05 100644
--- a/src/librarybackend.cpp
+++ b/src/librarybackend.cpp
@@ -906,8 +906,8 @@ PlaylistItemList LibraryBackend::GetPlaylistItems(int playlist) {
     if (!item)
       continue;
 
-    item->InitFromQuery(q);
-    ret << item;
+    if (item->InitFromQuery(q))
+      ret << item;
   }
 
   return ret;
diff --git a/src/libraryplaylistitem.cpp b/src/libraryplaylistitem.cpp
index d183ae888..e4b31bb7a 100644
--- a/src/libraryplaylistitem.cpp
+++ b/src/libraryplaylistitem.cpp
@@ -39,9 +39,11 @@ void LibraryPlaylistItem::Reload() {
   song_.InitFromFile(song_.filename(), song_.directory_id());
 }
 
-void LibraryPlaylistItem::InitFromQuery(const QSqlQuery &query) {
+bool LibraryPlaylistItem::InitFromQuery(const QSqlQuery &query) {
   // Rows from the songs table come first
   song_.InitFromQuery(query);
+
+  return song_.is_valid();
 }
 
 QVariant LibraryPlaylistItem::DatabaseValue(DatabaseColumn column) const {
diff --git a/src/libraryplaylistitem.h b/src/libraryplaylistitem.h
index f88d3e450..e5cb9eb98 100644
--- a/src/libraryplaylistitem.h
+++ b/src/libraryplaylistitem.h
@@ -25,7 +25,7 @@ class LibraryPlaylistItem : public PlaylistItem {
   LibraryPlaylistItem(const QString& type);
   LibraryPlaylistItem(const Song& song);
 
-  void InitFromQuery(const QSqlQuery &query);
+  bool InitFromQuery(const QSqlQuery &query);
   void BindToQuery(QSqlQuery *query) const;
   void Reload();
 
diff --git a/src/playlistitem.h b/src/playlistitem.h
index 02d835bfb..84ae4f797 100644
--- a/src/playlistitem.h
+++ b/src/playlistitem.h
@@ -47,7 +47,7 @@ class PlaylistItem {
 
   virtual Options options() const { return Default; }
 
-  virtual void InitFromQuery(const QSqlQuery& query) = 0;
+  virtual bool InitFromQuery(const QSqlQuery& query) = 0;
   void BindToQuery(QSqlQuery* query) const;
   virtual void Reload() {}
 
diff --git a/src/radioplaylistitem.cpp b/src/radioplaylistitem.cpp
index c54165079..cb049614d 100644
--- a/src/radioplaylistitem.cpp
+++ b/src/radioplaylistitem.cpp
@@ -40,7 +40,7 @@ RadioPlaylistItem::RadioPlaylistItem(RadioService* service, const QUrl& url,
   InitMetadata();
 }
 
-void RadioPlaylistItem::InitFromQuery(const QSqlQuery &query) {
+bool RadioPlaylistItem::InitFromQuery(const QSqlQuery &query) {
   // The song table gets joined first, plus one for the song ROWID
   const int row = Song::kColumns.count() + 1;
 
@@ -52,6 +52,7 @@ void RadioPlaylistItem::InitFromQuery(const QSqlQuery &query) {
   service_ = RadioModel::ServiceByName(service);
 
   InitMetadata();
+  return true;
 }
 
 QVariant RadioPlaylistItem::DatabaseValue(DatabaseColumn column) const {
diff --git a/src/radioplaylistitem.h b/src/radioplaylistitem.h
index db5c76bba..60de7b087 100644
--- a/src/radioplaylistitem.h
+++ b/src/radioplaylistitem.h
@@ -32,7 +32,7 @@ class RadioPlaylistItem : public PlaylistItem {
 
   Options options() const;
 
-  void InitFromQuery(const QSqlQuery &query);
+  bool InitFromQuery(const QSqlQuery &query);
   void BindToQuery(QSqlQuery *query) const;
 
   Song Metadata() const;
diff --git a/src/song.cpp b/src/song.cpp
index dd593f7cd..263b07b96 100644
--- a/src/song.cpp
+++ b/src/song.cpp
@@ -490,6 +490,9 @@ bool Song::Save() const {
 
   scoped_ptr<TagLib::FileRef> fileref(factory_->GetFileRef(d->filename_));
 
+  if (!fileref || fileref->isNull()) // The file probably doesn't exist
+    return false;
+
   fileref->tag()->setTitle(QStringToTaglibString(d->title_));
   fileref->tag()->setArtist(QStringToTaglibString(d->artist_));
   fileref->tag()->setAlbum(QStringToTaglibString(d->album_));
diff --git a/src/songplaylistitem.cpp b/src/songplaylistitem.cpp
index 1999c4227..7ecd80456 100644
--- a/src/songplaylistitem.cpp
+++ b/src/songplaylistitem.cpp
@@ -31,7 +31,7 @@ SongPlaylistItem::SongPlaylistItem(const Song& song)
 {
 }
 
-void SongPlaylistItem::InitFromQuery(const QSqlQuery &query) {
+bool SongPlaylistItem::InitFromQuery(const QSqlQuery &query) {
   // The song table gets joined first, plus one for the song ROWID
   const int row = Song::kColumns.count() + 1;
 
@@ -53,7 +53,11 @@ void SongPlaylistItem::InitFromQuery(const QSqlQuery &query) {
     song_.Init(title, artist, album, length);
   } else {
     song_.InitFromFile(filename, -1);
+
+    if (!song_.is_valid())
+      return false;
   }
+  return true;
 }
 
 QVariant SongPlaylistItem::DatabaseValue(DatabaseColumn column) const {
diff --git a/src/songplaylistitem.h b/src/songplaylistitem.h
index 20e6541b4..f169ec096 100644
--- a/src/songplaylistitem.h
+++ b/src/songplaylistitem.h
@@ -25,7 +25,7 @@ class SongPlaylistItem : public PlaylistItem {
   SongPlaylistItem(const QString& type);
   SongPlaylistItem(const Song& song);
 
-  void InitFromQuery(const QSqlQuery &query);
+  bool InitFromQuery(const QSqlQuery &query);
   void Reload();
 
   Song Metadata() const { return song_; }
diff --git a/tests/mock_playlistitem.h b/tests/mock_playlistitem.h
index 6ef5a6aa2..66c56576b 100644
--- a/tests/mock_playlistitem.h
+++ b/tests/mock_playlistitem.h
@@ -30,7 +30,7 @@ class MockPlaylistItem : public PlaylistItem {
   MOCK_CONST_METHOD0(options,
       Options());
   MOCK_METHOD1(InitFromQuery,
-      void(const QSqlQuery& settings));
+      bool(const QSqlQuery& settings));
   MOCK_METHOD0(Reload,
       void());
   MOCK_CONST_METHOD0(Metadata,