From f2eecadbd3fdcce07b5c49fc13b9d0af45afbf55 Mon Sep 17 00:00:00 2001
From: Arnaud Bienner <arnaud.bienner@gmail.com>
Date: Tue, 21 Oct 2014 21:49:23 +0200
Subject: [PATCH] Don't write rating if not set Fixes issue #4128

---
 ext/libclementine-tagreader/tagreader.cpp |  8 ++++++
 tests/song_test.cpp                       | 32 +++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/ext/libclementine-tagreader/tagreader.cpp b/ext/libclementine-tagreader/tagreader.cpp
index ecb9e1ba1..5a35d680a 100644
--- a/ext/libclementine-tagreader/tagreader.cpp
+++ b/ext/libclementine-tagreader/tagreader.cpp
@@ -723,6 +723,14 @@ bool TagReader::SaveSongRatingToFile(
   if (filename.isNull()) return false;
 
   qLog(Debug) << "Saving song rating tags to" << filename;
+  if (song.rating() < 0) {
+    // The FMPS spec says unrated == "tag not present". For us, no rating
+    // results in rating being -1, so don't write anything in that case.
+    // Actually, we should also remove tag set in this case, but in
+    // Clementine it is not possible to unset rating i.e. make a song "unrated".
+    qLog(Debug) << "Unrated: do nothing";
+    return true;
+  }
 
   std::unique_ptr<TagLib::FileRef> fileref(factory_->GetFileRef(filename));
 
diff --git a/tests/song_test.cpp b/tests/song_test.cpp
index feac59d3e..f77d830c5 100644
--- a/tests/song_test.cpp
+++ b/tests/song_test.cpp
@@ -153,6 +153,38 @@ TEST_F(SongTest, FMPSPlayCountBoth) {
   EXPECT_EQ(123, song.playcount());
 }
 
+TEST_F(SongTest, FMPSUnrated) {
+  QStringList files_to_test;
+  files_to_test <<
+      ":/testdata/beep.m4a" <<
+      ":/testdata/beep.mp3" <<
+      ":/testdata/beep.flac" <<
+      ":/testdata/beep.ogg" <<
+      ":/testdata/beep.spx" <<
+      ":/testdata/beep.wav" <<
+      ":/testdata/beep.wma";
+  for (const QString& test_filename : files_to_test) {
+    TemporaryResource r(test_filename);
+    Song song = ReadSongFromFile(r.fileName());
+    // beep files don't contain rating info, so they should be considered as
+    // "unrated" i.e. rating == -1
+    EXPECT_EQ(-1, song.rating());
+    // Writing -1 i.e. "unrated" to a file shouldn't write anything
+    WriteSongRatingToFile(song, r.fileName());
+
+    // Compare files
+    QFile orig_file(test_filename);
+    orig_file.open(QIODevice::ReadOnly);
+    QByteArray orig_file_data = orig_file.readAll();
+    QFile temp_file(r.fileName());
+    temp_file.open(QIODevice::ReadOnly);
+    QByteArray temp_file_data = temp_file.readAll();
+    EXPECT_TRUE(!orig_file_data.isEmpty());
+    EXPECT_TRUE(!temp_file_data.isEmpty());
+    EXPECT_TRUE(orig_file_data == temp_file_data);
+  }
+}
+
 TEST_F(SongTest, FMPSScore) {
   TemporaryResource r(":/testdata/beep.mp3");
   {