Abstract the QSettings usage out of Playlist and PlaylistSequence, create a mock for it, and add a basic playlist test

src/CMakeLists.txt

@@ -75,6 +75,7 @@ set(CLEMENTINE-SOURCES
     tracksliderslider.cpp
     stickyslider.cpp
     commandlineoptions.cpp
+    settingsprovider.cpp
 )
 
 # Header files that have Q_OBJECT in

src/playlist.cpp

@@ -37,8 +37,9 @@
 const char* Playlist::kRowsMimetype = "application/x-clementine-playlist-rows";
 const char* Playlist::kSettingsGroup = "Playlist";
 
-Playlist::Playlist(QObject *parent) :
+Playlist::Playlist(QObject *parent, SettingsProvider* settings)
-    QAbstractListModel(parent),
+  : QAbstractListModel(parent),
+    settings_(settings ? settings : new DefaultSettingsProvider),
     current_is_paused_(false),
     current_virtual_index_(-1),
     is_shuffled_(false),
@@ -47,6 +48,8 @@ Playlist::Playlist(QObject *parent) :
     playlist_sequence_(NULL),
     ignore_sorting_(false)
 {
+  settings_->set_group(kSettingsGroup);
+
   connect(this, SIGNAL(rowsInserted(const QModelIndex&, int, int)), SIGNAL(PlaylistChanged()));
   connect(this, SIGNAL(rowsRemoved(const QModelIndex&, int, int)), SIGNAL(PlaylistChanged()));
 }
@@ -623,18 +626,15 @@ void Playlist::SetCurrentIsPaused(bool paused) {
 }
 
 void Playlist::Save() const {
-  QSettings s;
+  settings_->beginWriteArray("items", items_.count());
-  s.beginGroup(kSettingsGroup);
-
-  s.beginWriteArray("items", items_.count());
   for (int i=0 ; i<items_.count() ; ++i) {
-    s.setArrayIndex(i);
+    settings_->setArrayIndex(i);
-    s.setValue("type", items_.at(i)->type_string());
+    settings_->setValue("type", items_.at(i)->type_string());
-    items_.at(i)->Save(s);
+    items_.at(i)->Save(*settings_.get());
   }
-  s.endArray();
+  settings_->endArray();
 
-  s.setValue("last_index", last_played_index());
+  settings_->setValue("last_index", last_played_index());
 }
 
 void Playlist::Restore() {
@@ -642,27 +642,24 @@ void Playlist::Restore() {
   items_.clear();
   virtual_items_.clear();
 
-  QSettings s;
+  int count = settings_->beginReadArray("items");
-  s.beginGroup(kSettingsGroup);
-
-  int count = s.beginReadArray("items");
   for (int i=0 ; i<count ; ++i) {
-    s.setArrayIndex(i);
+    settings_->setArrayIndex(i);
-    QString type(s.value("type").toString());
+    QString type(settings_->value("type").toString());
 
     PlaylistItem* item = PlaylistItem::NewFromType(type);
     if (!item)
       continue;
 
-    item->Restore(s);
+    item->Restore(*settings_.get());
     items_ << item;
     virtual_items_ << virtual_items_.count();
   }
-  s.endArray();
+  settings_->endArray();
 
   reset();
 
-  last_played_item_ = index(s.value("last_index", -1).toInt(), 0, QModelIndex());
+  last_played_item_ = index(settings_->value("last_index", -1).toInt(), 0, QModelIndex());
 }
 
 bool Playlist::removeRows(int row, int count, const QModelIndex& parent) {

src/playlist.h

@@ -24,6 +24,7 @@
 #include "song.h"
 #include "radioitem.h"
 #include "playlistsequence.h"
+#include "settingsprovider.h"
 
 class RadioService;
 
@@ -31,7 +32,7 @@ class Playlist : public QAbstractListModel {
   Q_OBJECT
 
  public:
-  Playlist(QObject* parent = 0);
+  Playlist(QObject* parent = 0, SettingsProvider* settings = NULL);
   ~Playlist();
 
   enum Column {
@@ -77,7 +78,6 @@ class Playlist : public QAbstractListModel {
 
   // Persistence
   void Save() const;
-  void SaveLast() const;
   void Restore();
 
   // Accessors
@@ -153,6 +153,8 @@ class Playlist : public QAbstractListModel {
   int NextVirtualIndex(int i) const;
 
  private:
+  boost::scoped_ptr<SettingsProvider> settings_;
+
   QList<PlaylistItem*> items_;
   QList<int> virtual_items_; // Contains the indices into items_ in the order
                              // that they will be played.

src/playlistitem.h

@@ -20,9 +20,8 @@
 #include <QStandardItem>
 #include <QUrl>
 
-class QSettings;
-
 class Song;
+class SettingsProvider;
 
 class PlaylistItem {
  public:
@@ -51,8 +50,8 @@ class PlaylistItem {
 
   virtual Options options() const { return Default; }
 
-  virtual void Save(QSettings& settings) const = 0;
+  virtual void Save(SettingsProvider& settings) const = 0;
-  virtual void Restore(const QSettings& settings) = 0;
+  virtual void Restore(const SettingsProvider& settings) = 0;
   virtual void Reload() {}
 
   virtual Song Metadata() const = 0;

src/playlistsequence.cpp

@@ -23,8 +23,9 @@
 
 const char* PlaylistSequence::kSettingsGroup = "PlaylistSequence";
 
-PlaylistSequence::PlaylistSequence(QWidget *parent)
+PlaylistSequence::PlaylistSequence(QWidget *parent, SettingsProvider *settings)
   : QWidget(parent),
+    settings_(settings ? settings : new DefaultSettingsProvider),
     repeat_menu_(new QMenu(this)),
     shuffle_menu_(new QMenu(this)),
     loading_(false),
@@ -33,6 +34,8 @@ PlaylistSequence::PlaylistSequence(QWidget *parent)
 {
   ui_.setupUi(this);
 
+  settings_->set_group(kSettingsGroup);
+
   QActionGroup* repeat_group = new QActionGroup(this);
   repeat_group->addAction(ui_.action_repeat_off);
   repeat_group->addAction(ui_.action_repeat_track);
@@ -55,23 +58,17 @@ PlaylistSequence::PlaylistSequence(QWidget *parent)
 }
 
 void PlaylistSequence::Load() {
-  QSettings s;
-  s.beginGroup(kSettingsGroup);
-
   loading_ = true; // Stops these setter functions calling Save()
-  SetShuffleMode(ShuffleMode(s.value("shuffle_mode", Shuffle_Off).toInt()));
+  SetShuffleMode(ShuffleMode(settings_->value("shuffle_mode", Shuffle_Off).toInt()));
-  SetRepeatMode(RepeatMode(s.value("repeat_mode", Repeat_Off).toInt()));
+  SetRepeatMode(RepeatMode(settings_->value("repeat_mode", Repeat_Off).toInt()));
   loading_ = false;
 }
 
 void PlaylistSequence::Save() {
   if (loading_) return;
 
-  QSettings s;
+  settings_->setValue("shuffle_mode", shuffle_mode_);
-  s.beginGroup(kSettingsGroup);
+  settings_->setValue("repeat_mode", repeat_mode_);
-
-  s.setValue("shuffle_mode", shuffle_mode_);
-  s.setValue("repeat_mode", repeat_mode_);
 }
 
 void PlaylistSequence::RepeatActionTriggered(QAction* action) {

src/playlistsequence.h

@@ -20,6 +20,7 @@
 #include <QWidget>
 
 #include "ui_playlistsequence.h"
+#include "settingsprovider.h"
 
 class QMenu;
 
@@ -27,7 +28,7 @@ class PlaylistSequence : public QWidget {
   Q_OBJECT
 
  public:
-  PlaylistSequence(QWidget *parent = 0);
+  PlaylistSequence(QWidget *parent = 0, SettingsProvider* settings = 0);
 
   enum RepeatMode {
     Repeat_Off = 0,
@@ -67,6 +68,7 @@ class PlaylistSequence : public QWidget {
 
  private:
   Ui::PlaylistSequence ui_;
+  boost::scoped_ptr<SettingsProvider> settings_;
 
   QMenu* repeat_menu_;
   QMenu* shuffle_menu_;

src/radioplaylistitem.cpp

@@ -17,6 +17,7 @@
 #include "radioplaylistitem.h"
 #include "radioservice.h"
 #include "radiomodel.h"
+#include "settingsprovider.h"
 
 #include <QSettings>
 #include <QApplication>
@@ -36,14 +37,14 @@ RadioPlaylistItem::RadioPlaylistItem(RadioService* service, const QUrl& url,
   InitMetadata();
 }
 
-void RadioPlaylistItem::Save(QSettings& settings) const {
+void RadioPlaylistItem::Save(SettingsProvider& settings) const {
   settings.setValue("service", service_->name());
   settings.setValue("url", url_.toString());
   settings.setValue("title", title_);
   settings.setValue("artist", artist_);
 }
 
-void RadioPlaylistItem::Restore(const QSettings& settings) {
+void RadioPlaylistItem::Restore(const SettingsProvider& settings) {
   service_ = RadioModel::ServiceByName(settings.value("service").toString());
   url_ = settings.value("url").toString();
   title_ = settings.value("title").toString();

src/radioplaylistitem.h

@@ -33,8 +33,8 @@ class RadioPlaylistItem : public PlaylistItem {
   Type type() const { return Type_Radio; }
   Options options() const;
 
-  void Save(QSettings& settings) const;
+  void Save(SettingsProvider& settings) const;
-  void Restore(const QSettings& settings);
+  void Restore(const SettingsProvider& settings);
 
   Song Metadata() const;
 

src/settingsprovider.cpp

@@ -0,0 +1,53 @@
+/* This file is part of Clementine.
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "settingsprovider.h"
+
+SettingsProvider::SettingsProvider() {
+}
+
+DefaultSettingsProvider::DefaultSettingsProvider() {
+}
+
+void DefaultSettingsProvider::set_group(const char *group) {
+  backend_.beginGroup(group);
+}
+
+QVariant DefaultSettingsProvider::value(
+    const QString &key, const QVariant &default_value) const {
+  return backend_.value(key, default_value);
+}
+
+void DefaultSettingsProvider::setValue(
+    const QString &key, const QVariant &value) {
+  backend_.setValue(key, value);
+}
+
+int DefaultSettingsProvider::beginReadArray(const QString& prefix) {
+  return backend_.beginReadArray(prefix);
+}
+
+void DefaultSettingsProvider::beginWriteArray(const QString& prefix, int size) {
+  backend_.beginWriteArray(prefix, size);
+}
+
+void DefaultSettingsProvider::setArrayIndex(int i) {
+  backend_.setArrayIndex(i);
+}
+
+void DefaultSettingsProvider::endArray() {
+  backend_.endArray();
+}

src/settingsprovider.h

@@ -0,0 +1,59 @@
+/* This file is part of Clementine.
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef SETTINGSPROVIDER_H
+#define SETTINGSPROVIDER_H
+
+#include <QVariant>
+#include <QSettings>
+
+#include <boost/scoped_ptr.hpp>
+
+class SettingsProvider {
+ public:
+  SettingsProvider();
+  virtual ~SettingsProvider() {}
+
+  virtual void set_group(const char* group) = 0;
+
+  virtual QVariant value(const QString& key,
+                         const QVariant& default_value = QVariant()) const = 0;
+  virtual void setValue(const QString& key, const QVariant& value) = 0;
+  virtual int beginReadArray(const QString& prefix) = 0;
+  virtual void beginWriteArray(const QString& prefix, int size = -1) = 0;
+  virtual void setArrayIndex(int i) = 0;
+  virtual void endArray() = 0;
+};
+
+class DefaultSettingsProvider : public SettingsProvider {
+ public:
+  DefaultSettingsProvider();
+
+  void set_group(const char* group);
+
+  QVariant value(const QString& key,
+                 const QVariant& default_value = QVariant()) const;
+  void setValue(const QString& key, const QVariant& value);
+  int beginReadArray(const QString& prefix);
+  void beginWriteArray(const QString& prefix, int size = -1);
+  void setArrayIndex(int i);
+  void endArray();
+
+ private:
+  QSettings backend_;
+};
+
+#endif // SETTINGSPROVIDER_H

src/songplaylistitem.cpp

@@ -15,6 +15,7 @@
 */
 
 #include "songplaylistitem.h"
+#include "settingsprovider.h"
 
 #include <QtDebug>
 #include <QFile>
@@ -29,7 +30,7 @@ SongPlaylistItem::SongPlaylistItem(const Song& song)
 {
 }
 
-void SongPlaylistItem::Save(QSettings& settings) const {
+void SongPlaylistItem::Save(SettingsProvider& settings) const {
   settings.setValue("filename", song_.filename());
   settings.setValue("art_automatic", song_.art_automatic());
   settings.setValue("art_manual", song_.art_manual());
@@ -41,12 +42,12 @@ void SongPlaylistItem::Save(QSettings& settings) const {
   }
 }
 
-void SongPlaylistItem::SaveFile(QSettings& settings) const {
+void SongPlaylistItem::SaveFile(SettingsProvider& settings) const {
   settings.setValue("stream", false);
   settings.setValue("library_directory", song_.directory_id());
 }
 
-void SongPlaylistItem::SaveStream(QSettings& settings) const {
+void SongPlaylistItem::SaveStream(SettingsProvider& settings) const {
   settings.setValue("stream", true);
   settings.setValue("title", song_.title());
   settings.setValue("artist", song_.artist());
@@ -54,7 +55,7 @@ void SongPlaylistItem::SaveStream(QSettings& settings) const {
   settings.setValue("length", song_.length());
 }
 
-void SongPlaylistItem::Restore(const QSettings& settings) {
+void SongPlaylistItem::Restore(const SettingsProvider& settings) {
   song_.set_art_automatic(settings.value("art_automatic").toString());
   song_.set_art_manual(settings.value("art_manual").toString());
 
@@ -66,14 +67,14 @@ void SongPlaylistItem::Restore(const QSettings& settings) {
   }
 }
 
-void SongPlaylistItem::RestoreFile(const QSettings& settings) {
+void SongPlaylistItem::RestoreFile(const SettingsProvider& settings) {
   QString filename(settings.value("filename").toString());
 
   int directory_id(settings.value("library_directory", -1).toInt());
   song_.InitFromFile(filename, directory_id);
 }
 
-void SongPlaylistItem::RestoreStream(const QSettings& settings) {
+void SongPlaylistItem::RestoreStream(const SettingsProvider& settings) {
   QString filename(settings.value("filename").toString());
   song_.set_filename(filename);
   song_.set_filetype(Song::Type_Stream);

src/songplaylistitem.h

@@ -27,8 +27,8 @@ class SongPlaylistItem : public PlaylistItem {
 
   Type type() const { return Type_Song; }
 
-  void Save(QSettings& settings) const;
+  void Save(SettingsProvider& settings) const;
-  void Restore(const QSettings& settings);
+  void Restore(const SettingsProvider& settings);
   void Reload();
 
   Song Metadata() const { return song_; }
@@ -36,11 +36,11 @@ class SongPlaylistItem : public PlaylistItem {
   QUrl Url() const;
 
  private:
-  void SaveFile(QSettings& settings) const;
+  void SaveFile(SettingsProvider& settings) const;
-  void SaveStream(QSettings& settings) const;
+  void SaveStream(SettingsProvider& settings) const;
 
-  void RestoreFile(const QSettings& settings);
+  void RestoreFile(const SettingsProvider& settings);
-  void RestoreStream(const QSettings& settings);
+  void RestoreStream(const SettingsProvider& settings);
   Song song_;
 };
 

tests/CMakeLists.txt

@@ -95,3 +95,4 @@ add_test_file(library_test.cpp false)
 add_test_file(albumcovermanager_test.cpp true)
 add_test_file(songplaylistitem_test.cpp false)
 add_test_file(translations_test.cpp false)
+add_test_file(playlist_test.cpp true)

tests/mock_settingsprovider.h

@@ -0,0 +1,57 @@
+/* This file is part of Clementine.
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef MOCK_SETTINGSPROVIDER_H
+#define MOCK_SETTINGSPROVIDER_H
+
+#include "settingsprovider.h"
+
+#include <gmock/gmock.h>
+
+class MockSettingsProvider : public SettingsProvider {
+ public:
+  MOCK_METHOD1(set_group,
+      void(const char* group));
+  MOCK_CONST_METHOD2(value,
+      QVariant(const QString& key, const QVariant& default_value));
+  MOCK_METHOD2(setValue,
+      void(const QString& key, const QVariant& value));
+  MOCK_METHOD1(beginReadArray,
+      int(const QString& prefix));
+  MOCK_METHOD2(beginWriteArray,
+      void(const QString& prefix, int size));
+  MOCK_METHOD1(setArrayIndex,
+      void(int i));
+  MOCK_METHOD0(endArray,
+      void());
+};
+
+class DummySettingsProvider : public SettingsProvider {
+ public:
+  DummySettingsProvider() {}
+
+  void set_group(const char *group) {}
+
+  QVariant value(const QString&, const QVariant& = QVariant()) const { return QVariant(); }
+  void setValue(const QString&, const QVariant&) {}
+  int beginReadArray(const QString&) { return 0; }
+  void beginWriteArray(const QString&, int = -1) {}
+  void setArrayIndex(int) {}
+  void endArray() {}
+};
+
+#endif // MOCK_SETTINGSPROVIDER_H
+

tests/playlist_test.cpp

@@ -0,0 +1,50 @@
+/* This file is part of Clementine.
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "test_utils.h"
+#include "gtest/gtest.h"
+
+#include "playlist.h"
+#include "mock_settingsprovider.h"
+
+#include <QtDebug>
+
+#include <boost/scoped_ptr.hpp>
+
+namespace {
+
+class PlaylistTest : public ::testing::Test {
+ protected:
+  PlaylistTest()
+    : playlist_(new Playlist(NULL, new DummySettingsProvider)),
+      sequence_(new PlaylistSequence(NULL, new DummySettingsProvider))
+  {
+  }
+
+  void SetUp() {
+    playlist_->set_sequence(sequence_.get());
+  }
+
+  boost::scoped_ptr<Playlist> playlist_;
+  boost::scoped_ptr<PlaylistSequence> sequence_;
+};
+
+TEST_F(PlaylistTest, Basic) {
+  EXPECT_EQ(0, playlist_->rowCount(QModelIndex()));
+}
+
+}
+