Listenbrainz: fixed bad date time when importing feedbacks

src/libs/services/scrobbling/CMakeLists.txt

@@ -1,7 +1,9 @@
 
 add_library(lmsscrobbling SHARED
 	impl/internal/InternalScrobbler.cpp
+	impl/listenbrainz/FeedbacksParser.cpp
 	impl/listenbrainz/FeedbacksSynchronizer.cpp
+	impl/listenbrainz/FeedbackTypes.cpp
 	impl/listenbrainz/ListenBrainzScrobbler.cpp
 	impl/listenbrainz/ListenTypes.cpp
 	impl/listenbrainz/ListensParser.cpp

src/libs/services/scrobbling/impl/listenbrainz/Exception.hpp

@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2022 Emeric Poupon
+ *
+ * This file is part of LMS.
+ *
+ * LMS 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.
+ *
+ * LMS 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 LMS.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "services/scrobbling/Exception.hpp"
+
+namespace Scrobbling::ListenBrainz
+{
+	class Exception : public Scrobbling::Exception
+	{
+		public:
+			using Scrobbling::Exception::Exception;
+	};
+}

src/libs/services/scrobbling/impl/listenbrainz/FeedbackTypes.cpp

@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2022 Emeric Poupon
+ *
+ * This file is part of LMS.
+ *
+ * LMS 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.
+ *
+ * LMS 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 LMS.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "FeedbackTypes.hpp"
+
+namespace Scrobbling::ListenBrainz
+{
+	std::ostream&
+	operator<<(std::ostream& os, const Feedback& feedback)
+	{
+		os << "created = '" << feedback.created.toString() << "', recording MBID = '" << feedback.recordingMBID.getAsString() << "', score = " << static_cast<int>(feedback.score);
+		return os;
+	}
+} // Scrobbling::ListenBrainz

src/libs/services/scrobbling/impl/listenbrainz/FeedbackTypes.hpp

@@ -19,6 +19,8 @@
 
 #pragma once
 
+#include <ostream>
+#include <Wt/WDateTime.h>
 #include "utils/UUID.hpp"
 
 namespace Scrobbling::ListenBrainz
@@ -37,4 +39,7 @@ namespace Scrobbling::ListenBrainz
 		UUID			recordingMBID;
 		FeedbackType	score;
 	};
+
+	std::ostream& operator<<(std::ostream& os, const Feedback& feedback);
+
 } // Scrobbling::ListenBrainz

src/libs/services/scrobbling/impl/listenbrainz/FeedbacksParser.cpp

@@ -0,0 +1,94 @@
+/*
+ * Copyright (C) 2022 Emeric Poupon
+ *
+ * This file is part of LMS.
+ *
+ * LMS 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.
+ *
+ * LMS 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 LMS.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "FeedbacksParser.hpp"
+
+#include <Wt/Json/Array.h>
+#include <Wt/Json/Object.h>
+#include <Wt/Json/Value.h>
+#include <Wt/Json/Parser.h>
+//#include <Wt/Json/Serializer.h>
+
+#include "services/scrobbling/Exception.hpp"
+#include "Exception.hpp"
+#include "Utils.hpp"
+
+namespace Scrobbling::ListenBrainz
+{
+	namespace
+	{
+		Feedback
+		parseFeedback(const Wt::Json::Object& feedbackObj)
+		{
+			const std::optional<UUID> recordingMBID {UUID::fromString(static_cast<std::string>(feedbackObj.get("recording_mbid")))};
+			if (!recordingMBID)
+				throw Exception {"MBID not found!"};
+
+			return Feedback
+			{
+				Wt::WDateTime::fromTime_t(static_cast<int>(feedbackObj.get("created"))),
+					*recordingMBID,
+					static_cast<FeedbackType>(static_cast<int>(feedbackObj.get("score")))
+			};
+		}
+	}
+
+	FeedbacksParser::Result
+	FeedbacksParser::parse(std::string_view msgBody)
+	{
+		Result res;
+
+		try
+		{
+			Wt::Json::Object root;
+			Wt::Json::parse(std::string {msgBody}, root);
+
+			const Wt::Json::Array& feedbacks = root.get("feedback");
+
+			LOG(DEBUG) << "Got " << feedbacks.size() << " feedbacks";
+
+			if (feedbacks.empty())
+				return res;
+
+			res.feedbackCount = feedbacks.size();
+
+			for (const Wt::Json::Value& value : feedbacks)
+			{
+				try
+				{
+					res.feedbacks.push_back(parseFeedback(value));
+				}
+				catch (const Exception& e)
+				{
+					LOG(DEBUG) << "Cannot parse feedback: " << e.what() << ", skipping";
+				}
+				catch (const Wt::WException &e)
+				{
+					LOG(DEBUG) << "Cannot parse feedback: " << e.what() << ", skipping";
+				}
+			}
+		}
+		catch (const Wt::WException& error)
+		{
+			LOG(ERROR) << "Cannot parse 'feedback' result: " << error.what();
+		}
+
+		return res;
+	}
+} // Scrobbling::ListenBrainz

src/libs/services/scrobbling/impl/listenbrainz/FeedbacksParser.hpp

@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2022 Emeric Poupon
+ *
+ * This file is part of LMS.
+ *
+ * LMS 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.
+ *
+ * LMS 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 LMS.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <string_view>
+
+#include "FeedbackTypes.hpp"
+
+namespace Scrobbling::ListenBrainz
+{
+	class FeedbacksParser
+	{
+		public:
+			struct Result
+			{
+				std::size_t feedbackCount {}; // >= feedbacks.size()
+				std::vector<Feedback> feedbacks;
+			};
+
+			static Result parse(std::string_view msgBody);
+	};
+
+} // Scrobbling::ListenBrainz

src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp

@@ -30,11 +30,12 @@
 #include "services/database/StarredTrack.hpp"
 #include "services/database/Track.hpp"
 #include "services/database/User.hpp"
-#include "services/scrobbling/Exception.hpp"
 #include "utils/IConfig.hpp"
 #include "utils/http/IClient.hpp"
 #include "utils/Service.hpp"
 
+#include "Exception.hpp"
+#include "FeedbacksParser.hpp"
 #include "Utils.hpp"
 
 using namespace Scrobbling::ListenBrainz;
@@ -42,24 +43,6 @@ using namespace Database;
 
 namespace
 {
-	class Exception : public Scrobbling::Exception
-	{
-		public:
-			using Scrobbling::Exception::Exception;
-	};
-
-	class ParseErrorException : public Exception
-	{
-		public:
-			using Exception::Exception;
-	};
-
-	class MBIDNotFoundException : public Exception
-	{
-		public:
-			MBIDNotFoundException() : Exception {"MBID not found"} {}
-	};
-
 	std::optional<std::size_t>
 	parseTotalFeedbackCount(std::string_view msgBody)
 	{
@@ -76,75 +59,6 @@ namespace
 			return std::nullopt;
 		}
 	}
-
-	Feedback
-	parseFeedback(const Wt::Json::Object& feedbackObj)
-	{
-		try
-		{
-			const std::optional<UUID> recordingMBID {UUID::fromString(static_cast<std::string>(feedbackObj.get("recording_mbid")))};
-			if (!recordingMBID)
-				throw MBIDNotFoundException {};
-
-			return Feedback
-			{
-				Wt::WDateTime::fromTime_t(static_cast<int>(feedbackObj.get("created"))),
-				*recordingMBID,
-				static_cast<FeedbackType>(static_cast<int>(feedbackObj.get("score")))
-			};
-		}
-		catch (const Wt::WException& e)
-		{
-			LOG(DEBUG) << "Cannot parse feedback: " << e.what();
-			throw Exception {};
-		}
-	}
-
-	struct GetFeedbacksResult
-	{
-		std::size_t totalFeedbackCount{};
-		std::vector<Feedback> feedbacks;
-	};
-
-	GetFeedbacksResult
-	parseGetFeedbacks(std::string_view msgBody)
-	{
-		GetFeedbacksResult res;
-
-		try
-		{
-			Wt::Json::Object root;
-			Wt::Json::parse(std::string {msgBody}, root);
-
-			const Wt::Json::Array& feedbacks = root.get("feedback");
-
-			LOG(DEBUG) << "Got " << feedbacks.size() << " feedbacks";
-
-			if (feedbacks.empty())
-				return res;
-
-			res.totalFeedbackCount += feedbacks.size();
-
-			for (const Wt::Json::Value& value : feedbacks)
-			{
-				try
-				{
-					res.feedbacks.push_back(parseFeedback(value));
-				}
-				catch (const Exception &e)
-				{
-					LOG(DEBUG) << "Cannot parse feedback: " << e.what() << ", skipping";
-				}
-			}
-		}
-		catch (const Wt::WException& error)
-		{
-			LOG(ERROR) << "Cannot parse 'get-feedback' result: " << error.what();
-			throw ParseErrorException {error.what()};
-		}
-
-		return res;
-	}
 }
 
 namespace Scrobbling::ListenBrainz
@@ -493,24 +407,17 @@ namespace Scrobbling::ListenBrainz
 				std::string msgBodyCopy {msgBody};
 				_strand.dispatch([this, msgBodyCopy, &context]
 				{
-					try
+					const std::size_t fetchedFeedbackCount {processGetFeedbacks(msgBodyCopy, context)};
-					{
+					if (fetchedFeedbackCount == 0 // no more thing available on server
-						const std::size_t fetchedFeedbackCount {processGetFeedbacks(msgBodyCopy, context)};
+							|| context.fetchedFeedbackCount >= context.feedbackCount // we may miss something, but we will get it next time
-						if (fetchedFeedbackCount == 0 // no more thing available on server
+							|| context.fetchedFeedbackCount >= _maxSyncFeedbackCount)
-								|| context.fetchedFeedbackCount >= context.feedbackCount // we may miss something, but we will get it next time
-								|| context.fetchedFeedbackCount >= _maxSyncFeedbackCount)
-						{
-							onSyncEnded(context);
-						}
-						else
-						{
-							enqueGetFeedbacks(context);
-						}
-					}
-					catch (const Exception& e)
 					{
 						onSyncEnded(context);
 					}
+					else
+					{
+						enqueGetFeedbacks(context);
+					}
 				});
 			};
 		request.onFailureFunc = [=, &context]
@@ -524,17 +431,17 @@ namespace Scrobbling::ListenBrainz
 	std::size_t
 	FeedbacksSynchronizer::processGetFeedbacks(std::string_view msgBody, UserContext& context)
 	{
-		const GetFeedbacksResult parseResult {parseGetFeedbacks(msgBody)};
+		const FeedbacksParser::Result parseResult {FeedbacksParser::parse(msgBody)};
 
-		LOG(DEBUG) << "Parsed " << parseResult.totalFeedbackCount << " feedbacks, found " << parseResult.feedbacks.size() << " usable entries";
+		LOG(DEBUG) << "Parsed " << parseResult.feedbackCount << " feedbacks, found " << parseResult.feedbacks.size() << " usable entries";
-		context.fetchedFeedbackCount += parseResult.totalFeedbackCount;
+		context.fetchedFeedbackCount += parseResult.feedbackCount;
 
 		for (const Feedback& feedback : parseResult.feedbacks)
 		{
 			tryImportFeedback(feedback, context);
 		}
 
-		return parseResult.totalFeedbackCount;
+		return parseResult.feedbackCount;
 	}
 
 	void
@@ -550,11 +457,12 @@ namespace Scrobbling::ListenBrainz
 			const std::vector<Track::pointer> tracks {Track::findByRecordingMBID(session, feedback.recordingMBID)};
 			if (tracks.size() > 1)
 			{
-				LOG(DEBUG) << "Duplicate recording MBIDs found for '" << feedback.recordingMBID.getAsString() << "', using first entry found";
+				LOG(DEBUG) << "Too many matches for feedback '" << feedback << "': duplicate recording MBIDs found";
+				return;
 			}
 			else if (tracks.empty())
 			{
-				LOG(DEBUG) << "No track found for recording MBID '" << feedback.recordingMBID.getAsString() << "'";
+				LOG(DEBUG) << "Cannot match feedback '" << feedback << "': no track found for this recording MBID";
 				return;
 			}
 
@@ -571,6 +479,8 @@ namespace Scrobbling::ListenBrainz
 
 		if (needImport)
 		{
+			LOG(DEBUG) << "Importing feedback '" << feedback << "'";
+
 			auto transaction {session.createUniqueTransaction()};
 
 			const Track::pointer track {Track::find(session, trackId)};
@@ -583,11 +493,13 @@ namespace Scrobbling::ListenBrainz
 
 			StarredTrack::pointer starredTrack {session.create<StarredTrack>(track, user, Database::Scrobbler::ListenBrainz)};
 			starredTrack.modify()->setScrobblingState(ScrobblingState::Synchronized);
+			starredTrack.modify()->setDateTime(feedback.created);
 
 			context.importedFeedbackCount++;
 		}
 		else
 		{
+			LOG(DEBUG) << "No need to import feedback '" << feedback << "', already imported";
 			context.matchedFeedbackCount++;
 		}
 	}

src/libs/services/scrobbling/impl/listenbrainz/ListensParser.hpp

@@ -19,6 +19,8 @@
 
 #pragma once
 
+#include <string_view>
+
 #include "ListenTypes.hpp"
 
 namespace Scrobbling::ListenBrainz