Save magnatune playlist items properly, also fix tests from before

src/CMakeLists.txt

@@ -88,6 +88,7 @@ set(CLEMENTINE-SOURCES
     libraryfilterwidget.cpp
     radioviewcontainer.cpp
     networkaccessmanager.cpp
+    magnatuneplaylistitem.cpp
 )
 
 # Header files that have Q_OBJECT in

src/libraryplaylistitem.cpp

@@ -40,7 +40,7 @@ void LibraryPlaylistItem::Reload() {
 }
 
 bool LibraryPlaylistItem::InitFromQuery(const QSqlQuery &query) {
-  // Rows from the songs table come first
+  // Rows from the songs tables come first
   song_.InitFromQuery(query);
 
   return song_.is_valid();
@@ -48,7 +48,7 @@ bool LibraryPlaylistItem::InitFromQuery(const QSqlQuery &query) {
 
 QVariant LibraryPlaylistItem::DatabaseValue(DatabaseColumn column) const {
   switch (column) {
-    case Column_LibraryId: return song_.id();
+    case Column_LibraryId:    return song_.id();
-    default:               return PlaylistItem::DatabaseValue(column);
+    default:                  return PlaylistItem::DatabaseValue(column);
   }
 }

src/libraryplaylistitem.h

@@ -26,7 +26,6 @@ class LibraryPlaylistItem : public PlaylistItem {
   LibraryPlaylistItem(const Song& song);
 
   bool InitFromQuery(const QSqlQuery &query);
-  void BindToQuery(QSqlQuery *query) const;
   void Reload();
 
   Song Metadata() const { return song_; }
@@ -36,7 +35,7 @@ class LibraryPlaylistItem : public PlaylistItem {
  protected:
   QVariant DatabaseValue(DatabaseColumn column) const;
 
- private:
+ protected:
   Song song_;
 };
 

src/magnatuneplaylistitem.cpp

@@ -0,0 +1,19 @@
+#include "magnatuneplaylistitem.h"
+
+MagnatunePlaylistItem::MagnatunePlaylistItem(const QString& type)
+  : LibraryPlaylistItem(type)
+{
+}
+
+MagnatunePlaylistItem::MagnatunePlaylistItem(const Song& song)
+  : LibraryPlaylistItem("Magnatune")
+{
+  song_ = song;
+}
+
+bool MagnatunePlaylistItem::InitFromQuery(const QSqlQuery &query) {
+  // Rows from the songs tables come first
+  song_.InitFromQuery(query, Song::kColumns.count() + 1);
+
+  return song_.is_valid();
+}

src/magnatuneplaylistitem.h

@@ -0,0 +1,14 @@
+#ifndef MAGNATUNEPLAYLISTITEM_H
+#define MAGNATUNEPLAYLISTITEM_H
+
+#include "libraryplaylistitem.h"
+
+class MagnatunePlaylistItem : public LibraryPlaylistItem {
+ public:
+  MagnatunePlaylistItem(const QString& type);
+  MagnatunePlaylistItem(const Song& song);
+
+  bool InitFromQuery(const QSqlQuery &query);
+};
+
+#endif // MAGNATUNEPLAYLISTITEM_H

src/magnatuneservice.cpp

@@ -22,6 +22,7 @@
 #include "librarybackend.h"
 #include "libraryfilterwidget.h"
 #include "networkaccessmanager.h"
+#include "magnatuneplaylistitem.h"
 
 #include <QNetworkAccessManager>
 #include <QNetworkRequest>
@@ -34,6 +35,8 @@
 
 #include <QtDebug>
 
+using boost::shared_ptr;
+
 const char* MagnatuneService::kServiceName = "Magnatune";
 const char* MagnatuneService::kSettingsGroup = "Magnatune";
 const char* MagnatuneService::kDatabaseUrl =
@@ -224,8 +227,16 @@ void MagnatuneService::ShowContextMenu(RadioItem*, const QModelIndex& index,
 }
 
 void MagnatuneService::AddToPlaylist() {
-  emit AddItemsToPlaylist(library_model_->GetChildSongs(
+  SongList songs(library_model_->GetChildSongs(
       library_sort_model_->mapToSource(context_item_)));
+
+  PlaylistItemList items;
+
+  foreach (const Song& song, songs) {
+    items << shared_ptr<PlaylistItem>(new MagnatunePlaylistItem(song));
+  }
+
+  emit AddItemsToPlaylist(items);
 }
 
 void MagnatuneService::Homepage() {

src/mainwindow.cpp

@@ -278,7 +278,7 @@ MainWindow::MainWindow(NetworkAccessManager* network, Engine::Type engine, QWidg
   connect(radio_model_, SIGNAL(StreamReady(QUrl,QUrl)), player_, SLOT(StreamReady(QUrl,QUrl)));
   connect(radio_model_, SIGNAL(StreamMetadataFound(QUrl,Song)), playlist_, SLOT(SetStreamMetadata(QUrl,Song)));
   connect(radio_model_, SIGNAL(AddItemToPlaylist(RadioItem*)), SLOT(InsertRadioItem(RadioItem*)));
-  connect(radio_model_, SIGNAL(AddItemsToPlaylist(SongList)), SLOT(InsertRadioItems(SongList)));
+  connect(radio_model_, SIGNAL(AddItemsToPlaylist(PlaylistItemList)), SLOT(InsertRadioItems(PlaylistItemList)));
   connect(radio_model_->GetLastFMService(), SIGNAL(ScrobblingEnabledChanged(bool)), SLOT(ScrobblingEnabledChanged(bool)));
   connect(radio_model_->GetLastFMService(), SIGNAL(ButtonVisibilityChanged(bool)), SLOT(LastFMButtonVisibilityChanged(bool)));
   connect(ui_.radio_view->tree(), SIGNAL(doubleClicked(QModelIndex)), SLOT(RadioDoubleClick(QModelIndex)));
@@ -627,8 +627,8 @@ void MainWindow::InsertRadioItem(RadioItem* item) {
     player_->PlayAt(first_song.row(), Engine::First, true);
 }
 
-void MainWindow::InsertRadioItems(const SongList& items) {
+void MainWindow::InsertRadioItems(const PlaylistItemList& items) {
-  QModelIndex first_song = playlist_->InsertSongs(items);
+  QModelIndex first_song = playlist_->InsertItems(items);
 
   if (first_song.isValid() && player_->GetState() != Engine::Playing)
     player_->PlayAt(first_song.row(), Engine::First, true);

src/mainwindow.h

@@ -114,7 +114,7 @@ class MainWindow : public QMainWindow {
 
   void RadioDoubleClick(const QModelIndex& index);
   void InsertRadioItem(RadioItem*);
-  void InsertRadioItems(const SongList& songs);
+  void InsertRadioItems(const PlaylistItemList& items);
   void ScrobblingEnabledChanged(bool value);
   void LastFMButtonVisibilityChanged(bool value);
   void Love();

src/networkaccessmanager.cpp

@@ -9,9 +9,10 @@
 
 #include <QtDebug>
 
-NetworkAccessManager::NetworkAccessManager(QObject* parent)
+NetworkAccessManager::NetworkAccessManager(QObject* parent,
+                                           QNetworkAccessManager* injected)
       : QObject(parent),
-        network_(new QNetworkAccessManager(this)),
+        network_(injected ? injected : new QNetworkAccessManager(this)),
         cache_(new QNetworkDiskCache(this))
 {
   cache_->setCacheDirectory(QString("%1/.config/%2/networkcache/")

src/networkaccessmanager.h

@@ -15,7 +15,7 @@ class NetworkAccessManager : public QObject {
   Q_OBJECT
 
  public:
-  NetworkAccessManager(QObject* parent = 0);
+  NetworkAccessManager(QObject* parent = 0, QNetworkAccessManager* injected = 0);
 
   // Only use this from the main thread
   QNetworkAccessManager* network() const { return network_; }

src/playlist.cpp

@@ -26,6 +26,8 @@
 #include "playlistundocommands.h"
 #include "library.h"
 #include "librarybackend.h"
+#include "magnatuneservice.h"
+#include "magnatuneplaylistitem.h"
 
 #include <QtDebug>
 #include <QMimeData>
@@ -357,6 +359,8 @@ bool Playlist::dropMimeData(const QMimeData* data, Qt::DropAction action, int ro
     // if they are we treat them differently.
     if (song_data->backend->songs_table() == Library::kSongsTable)
       InsertLibraryItems(song_data->songs, row);
+    else if (song_data->backend->songs_table() == MagnatuneService::kSongsTable)
+      InsertMagnatuneItems(song_data->songs, row);
     else
       InsertSongs(song_data->songs, row);
   } else if (const RadioMimeData* radio_data = qobject_cast<const RadioMimeData*>(data)) {
@@ -554,6 +558,14 @@ QModelIndex Playlist::InsertLibraryItems(const SongList& songs, int pos) {
   return InsertItems(items, pos);
 }
 
+QModelIndex Playlist::InsertMagnatuneItems(const SongList& songs, int pos) {
+  PlaylistItemList items;
+  foreach (const Song& song, songs) {
+    items << shared_ptr<PlaylistItem>(new MagnatunePlaylistItem(song));
+  }
+  return InsertItems(items, pos);
+}
+
 QModelIndex Playlist::InsertSongs(const SongList& songs, int pos) {
   PlaylistItemList items;
   foreach (const Song& song, songs) {

src/playlist.h

@@ -123,6 +123,7 @@ class Playlist : public QAbstractListModel {
   // Changing the playlist
   QModelIndex InsertItems(const PlaylistItemList& items, int pos = -1);
   QModelIndex InsertLibraryItems(const SongList& items, int pos = -1);
+  QModelIndex InsertMagnatuneItems(const SongList& items, int pos = -1);
   QModelIndex InsertSongs(const SongList& items, int pos = -1);
   QModelIndex InsertRadioStations(const QList<RadioItem*>& items, int pos = -1);
   QModelIndex InsertStreamUrls(const QList<QUrl>& urls, int pos = -1);

src/playlistbackend.cpp

@@ -40,12 +40,15 @@ PlaylistItemList PlaylistBackend::GetPlaylistItems(int playlist) {
 
   PlaylistItemList ret;
 
-  QSqlQuery q("SELECT songs.ROWID, " + Song::kJoinSpec + ","
+  QSqlQuery q("SELECT songs.ROWID, " + Song::JoinSpec("songs") + ","
+              "       magnatune_songs.ROWID, " + Song::JoinSpec("magnatune_songs") + ","
               "       p.type, p.url, p.title, p.artist, p.album, p.length,"
               "       p.radio_service"
               " FROM playlist_items AS p"
               " LEFT JOIN songs"
               "    ON p.library_id = songs.ROWID"
+              " LEFT JOIN magnatune_songs"
+              "    ON p.library_id = magnatune_songs.ROWID"
               " WHERE p.playlist = :playlist", db);
   q.bindValue(":playlist", playlist);
   q.exec();
@@ -53,8 +56,8 @@ PlaylistItemList PlaylistBackend::GetPlaylistItems(int playlist) {
     return ret;
 
   while (q.next()) {
-    // The song table gets joined first, plus one for the song ROWID
+    // The song tables gets joined first, plus one each for the song ROWIDs
-    const int row = Song::kColumns.count() + 1;
+    const int row = (Song::kColumns.count() + 1) * 2;
 
     shared_ptr<PlaylistItem> item(
         PlaylistItem::NewFromType(q.value(row + 0).toString()));

src/playlistitem.cpp

@@ -18,12 +18,15 @@
 #include "songplaylistitem.h"
 #include "radioplaylistitem.h"
 #include "libraryplaylistitem.h"
+#include "magnatuneplaylistitem.h"
 
 #include <QtDebug>
 
 PlaylistItem* PlaylistItem::NewFromType(const QString& type) {
   if (type == "Library")
     return new LibraryPlaylistItem(type);
+  if (type == "Magnatune")
+    return new MagnatunePlaylistItem(type);
   if (type == "Stream" || type == "File")
     return new SongPlaylistItem(type);
   if (type == "Radio")

src/radiomodel.cpp

@@ -56,7 +56,7 @@ void RadioModel::AddService(RadioService *service) {
   connect(service, SIGNAL(StreamError(QString)), SIGNAL(StreamError(QString)));
   connect(service, SIGNAL(StreamMetadataFound(QUrl,Song)), SIGNAL(StreamMetadataFound(QUrl,Song)));
   connect(service, SIGNAL(AddItemToPlaylist(RadioItem*)), SIGNAL(AddItemToPlaylist(RadioItem*)));
-  connect(service, SIGNAL(AddItemsToPlaylist(SongList)), SIGNAL(AddItemsToPlaylist(SongList)));
+  connect(service, SIGNAL(AddItemsToPlaylist(PlaylistItemList)), SIGNAL(AddItemsToPlaylist(PlaylistItemList)));
 }
 
 RadioService* RadioModel::ServiceByName(const QString& name) {

src/radiomodel.h

@@ -21,6 +21,7 @@
 #include "simpletreemodel.h"
 #include "multiloadingindicator.h"
 #include "song.h"
+#include "playlistitem.h"
 
 class NetworkAccessManager;
 class RadioService;
@@ -69,7 +70,7 @@ class RadioModel : public SimpleTreeModel<RadioItem> {
   void StreamMetadataFound(const QUrl& original_url, const Song& song);
 
   void AddItemToPlaylist(RadioItem* item);
-  void AddItemsToPlaylist(const SongList& items);
+  void AddItemsToPlaylist(const PlaylistItemList& items);
 
  protected:
   void LazyPopulate(RadioItem* parent);

src/radioplaylistitem.cpp

@@ -41,8 +41,8 @@ RadioPlaylistItem::RadioPlaylistItem(RadioService* service, const QUrl& url,
 }
 
 bool RadioPlaylistItem::InitFromQuery(const QSqlQuery &query) {
-  // The song table gets joined first, plus one for the song ROWID
+  // The song tables gets joined first, plus one each for the song ROWIDs
-  const int row = Song::kColumns.count() + 1;
+  const int row = (Song::kColumns.count() + 1) * 2;
 
   url_ = query.value(row + 1).toString();
   title_ = query.value(row + 2).toString();

src/radioservice.h

@@ -24,6 +24,7 @@
 #include "radioitem.h"
 #include "multiloadingindicator.h"
 #include "song.h"
+#include "playlistitem.h"
 
 class RadioModel;
 class LibraryFilterWidget;
@@ -71,7 +72,7 @@ class RadioService : public QObject {
   void StreamMetadataFound(const QUrl& original_url, const Song& song);
 
   void AddItemToPlaylist(RadioItem* item);
-  void AddItemsToPlaylist(const SongList& items);
+  void AddItemsToPlaylist(const PlaylistItemList& items);
 
  private:
   RadioModel* model_;

src/song.cpp

@@ -78,10 +78,12 @@ const QStringList Song::kColumns = QStringList()
     << "effective_compilation";
 
 const QString Song::kColumnSpec = Song::kColumns.join(", ");
-const QString Song::kJoinSpec = Prepend("songs.", Song::kColumns).join(", ");
 const QString Song::kBindSpec = Prepend(":", Song::kColumns).join(", ");
 const QString Song::kUpdateSpec = Updateify(Song::kColumns).join(", ");
 
+QString Song::JoinSpec(const QString& table) {
+  return Prepend(table + ".", kColumns).join(", ");
+}
 
 
 static TagLib::String QStringToTaglibString(const QString& s);
@@ -317,7 +319,7 @@ void Song::GuessFileType(TagLib::FileRef* fileref) {
     d->filetype_ = Type_TrueAudio;
 }
 
-void Song::InitFromQuery(const QSqlQuery& q) {
+void Song::InitFromQuery(const QSqlQuery& q, int col) {
   if (!q.isValid())
     return;
 
@@ -327,43 +329,43 @@ void Song::InitFromQuery(const QSqlQuery& q) {
   #define toint(n) (q.value(n).isNull() ? -1 : q.value(n).toInt())
   #define tofloat(n) (q.value(n).isNull() ? -1 : q.value(n).toDouble())
 
-  d->id_ = toint(0);
+  d->id_ = toint(col + 0);
-  d->title_ = tostr(1);
+  d->title_ = tostr(col + 1);
-  d->album_ = tostr(2);
+  d->album_ = tostr(col + 2);
-  d->artist_ = tostr(3);
+  d->artist_ = tostr(col + 3);
-  d->albumartist_ = tostr(4);
+  d->albumartist_ = tostr(col + 4);
-  d->composer_ = tostr(5);
+  d->composer_ = tostr(col + 5);
-  d->track_ = toint(6);
+  d->track_ = toint(col + 6);
-  d->disc_ = toint(7);
+  d->disc_ = toint(col + 7);
-  d->bpm_ = tofloat(8);
+  d->bpm_ = tofloat(col + 8);
-  d->year_ = toint(9);
+  d->year_ = toint(col + 9);
-  d->genre_ = tostr(10);
+  d->genre_ = tostr(col + 10);
-  d->comment_ = tostr(11);
+  d->comment_ = tostr(col + 11);
-  d->compilation_ = q.value(12).toBool();
+  d->compilation_ = q.value(col + 12).toBool();
 
-  d->length_ = toint(13);
+  d->length_ = toint(col + 13);
-  d->bitrate_ = toint(14);
+  d->bitrate_ = toint(col + 14);
-  d->samplerate_ = toint(15);
+  d->samplerate_ = toint(col + 15);
 
-  d->directory_id_ = toint(16);
+  d->directory_id_ = toint(col + 16);
-  d->filename_ = tostr(17);
+  d->filename_ = tostr(col + 17);
   d->basefilename_ = QFileInfo(d->filename_).fileName();
-  d->mtime_ = toint(18);
+  d->mtime_ = toint(col + 18);
-  d->ctime_ = toint(19);
+  d->ctime_ = toint(col + 19);
-  d->filesize_ = toint(20);
+  d->filesize_ = toint(col + 20);
 
-  d->sampler_ = q.value(21).toBool();
+  d->sampler_ = q.value(col + 21).toBool();
 
-  d->art_automatic_ = q.value(22).toString();
+  d->art_automatic_ = q.value(col + 22).toString();
-  d->art_manual_ = q.value(23).toString();
+  d->art_manual_ = q.value(col + 23).toString();
 
-  d->filetype_ = FileType(q.value(24).toInt());
+  d->filetype_ = FileType(q.value(col + 24).toInt());
   // playcount = 25
   // lastplayed = 26
   // rating = 27
 
-  d->forced_compilation_on_ = q.value(28).toBool();
+  d->forced_compilation_on_ = q.value(col + 28).toBool();
-  d->forced_compilation_off_ = q.value(29).toBool();
+  d->forced_compilation_off_ = q.value(col + 29).toBool();
 
   // effective_compilation = 30
 

src/song.h

@@ -81,6 +81,8 @@ class Song {
   static const QString kBindSpec;
   static const QString kUpdateSpec;
 
+  static QString JoinSpec(const QString& table);
+
   // Don't change these values - they're stored in the database
   enum FileType {
     Type_Unknown = 0,
@@ -102,7 +104,7 @@ class Song {
   // Constructors
   void Init(const QString& title, const QString& artist, const QString& album, int length);
   void InitFromFile(const QString& filename, int directory_id);
-  void InitFromQuery(const QSqlQuery& query);
+  void InitFromQuery(const QSqlQuery& query, int col = 0);
   void InitFromLastFM(const lastfm::Track& track);
   void MergeFromSimpleMetaBundle(const Engine::SimpleMetaBundle& bundle);
 

src/songplaylistitem.cpp

@@ -32,8 +32,8 @@ SongPlaylistItem::SongPlaylistItem(const Song& song)
 }
 
 bool SongPlaylistItem::InitFromQuery(const QSqlQuery &query) {
-  // The song table gets joined first, plus one for the song ROWID
+  // The song tables gets joined first, plus one each for the song ROWIDs
-  const int row = Song::kColumns.count() + 1;
+  const int row = (Song::kColumns.count() + 1) * 2;
 
   QString filename(query.value(row + 1).toString());
 

tests/albumcoverfetcher_test.cpp

@@ -23,6 +23,7 @@
 #include <QSignalSpy>
 
 #include "mock_networkaccessmanager.h"
+#include "networkaccessmanager.h"
 #include "gtest/gtest.h"
 
 namespace {
@@ -31,22 +32,29 @@ class AlbumCoverFetcherTest : public ::testing::Test {
  protected:
   static void SetUpTestCase() {
     lastfm::ws::ApiKey = "foobar";
+
+    // Lastfm takes ownership of this.
+    mock_network_ = new MockNetworkAccessManager;
+    lastfm::setNetworkAccessManager(mock_network_);
   }
 
   void SetUp() {
-    // Lastfm takes ownership of this.
+    network_ = new NetworkAccessManager(NULL, mock_network_);
-    network_ = new MockNetworkAccessManager;
-    lastfm::setNetworkAccessManager(network_);
   }
 
-  static void TearDownTestCase() {
+  void TearDown() {
     delete network_;
   }
 
-  static MockNetworkAccessManager* network_;
+  static void TearDownTestCase() {
+    delete mock_network_;
+  }
+
+  static MockNetworkAccessManager* mock_network_;
+  NetworkAccessManager* network_;
 };
 
-MockNetworkAccessManager* AlbumCoverFetcherTest::network_;
+MockNetworkAccessManager* AlbumCoverFetcherTest::mock_network_;
 
 
 TEST_F(AlbumCoverFetcherTest, FetchesAlbumCover) {
@@ -58,9 +66,9 @@ TEST_F(AlbumCoverFetcherTest, FetchesAlbumCover) {
   params["album"] = "Bar";
   params["api_key"] = "foobar";
   params["method"] = "album.getInfo";
-  MockNetworkReply* get_info_reply = network_->ExpectGet("audioscrobbler", params, 200, data);
+  MockNetworkReply* get_info_reply = mock_network_->ExpectGet("audioscrobbler", params, 200, data);
   params.clear();
-  MockNetworkReply* album_reply = network_->ExpectGet("http://example.com/image.jpg", params, 200, "");
+  MockNetworkReply* album_reply = mock_network_->ExpectGet("http://example.com/image.jpg", params, 200, "");
 
   AlbumCoverFetcher fetcher(network_, NULL);
   QSignalSpy spy(&fetcher, SIGNAL(AlbumCoverFetched(quint64, const QImage&)));

tests/albumcovermanager_test.cpp

@@ -15,6 +15,7 @@
 */
 
 #include "albumcovermanager.h"
+#include "networkaccessmanager.h"
 
 #include "gtest/gtest.h"
 
@@ -23,10 +24,12 @@
 class AlbumCoverManagerTest : public ::testing::Test {
  protected:
   AlbumCoverManagerTest()
-      : manager_(&network_, NULL) {
+      : network_(NULL, &mock_network_),
+        manager_(&network_, NULL) {
   }
 
-  MockNetworkAccessManager network_;
+  MockNetworkAccessManager mock_network_;
+  NetworkAccessManager network_;
   AlbumCoverManager manager_;
 };