From b85553af34aadde2bc5c06b4499bbb7c06526bcb Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 2 Oct 2022 14:41:55 +0200 Subject: [PATCH] Listenbrainz: fixed bad date time when importing feedbacks --- src/libs/services/scrobbling/CMakeLists.txt | 2 + .../impl/listenbrainz/Exception.hpp | 29 ++++ .../impl/listenbrainz/FeedbackTypes.cpp | 30 ++++ .../impl/listenbrainz/FeedbackTypes.hpp | 5 + .../impl/listenbrainz/FeedbacksParser.cpp | 94 +++++++++++++ .../impl/listenbrainz/FeedbacksParser.hpp | 38 +++++ .../listenbrainz/FeedbacksSynchronizer.cpp | 130 +++--------------- .../impl/listenbrainz/ListensParser.hpp | 2 + 8 files changed, 221 insertions(+), 109 deletions(-) create mode 100644 src/libs/services/scrobbling/impl/listenbrainz/Exception.hpp create mode 100644 src/libs/services/scrobbling/impl/listenbrainz/FeedbackTypes.cpp create mode 100644 src/libs/services/scrobbling/impl/listenbrainz/FeedbacksParser.cpp create mode 100644 src/libs/services/scrobbling/impl/listenbrainz/FeedbacksParser.hpp diff --git a/src/libs/services/scrobbling/CMakeLists.txt b/src/libs/services/scrobbling/CMakeLists.txt index 04137ab7..087a3236 100644 --- a/src/libs/services/scrobbling/CMakeLists.txt +++ b/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 diff --git a/src/libs/services/scrobbling/impl/listenbrainz/Exception.hpp b/src/libs/services/scrobbling/impl/listenbrainz/Exception.hpp new file mode 100644 index 00000000..6481bba7 --- /dev/null +++ b/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 . + */ + +#include "services/scrobbling/Exception.hpp" + +namespace Scrobbling::ListenBrainz +{ + class Exception : public Scrobbling::Exception + { + public: + using Scrobbling::Exception::Exception; + }; +} diff --git a/src/libs/services/scrobbling/impl/listenbrainz/FeedbackTypes.cpp b/src/libs/services/scrobbling/impl/listenbrainz/FeedbackTypes.cpp new file mode 100644 index 00000000..0b4d5168 --- /dev/null +++ b/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 . + */ + +#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(feedback.score); + return os; + } +} // Scrobbling::ListenBrainz diff --git a/src/libs/services/scrobbling/impl/listenbrainz/FeedbackTypes.hpp b/src/libs/services/scrobbling/impl/listenbrainz/FeedbackTypes.hpp index 782e523d..cb45d992 100644 --- a/src/libs/services/scrobbling/impl/listenbrainz/FeedbackTypes.hpp +++ b/src/libs/services/scrobbling/impl/listenbrainz/FeedbackTypes.hpp @@ -19,6 +19,8 @@ #pragma once +#include +#include #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 diff --git a/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksParser.cpp b/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksParser.cpp new file mode 100644 index 00000000..359e90e6 --- /dev/null +++ b/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 . + */ + +#include "FeedbacksParser.hpp" + +#include +#include +#include +#include +//#include + +#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 recordingMBID {UUID::fromString(static_cast(feedbackObj.get("recording_mbid")))}; + if (!recordingMBID) + throw Exception {"MBID not found!"}; + + return Feedback + { + Wt::WDateTime::fromTime_t(static_cast(feedbackObj.get("created"))), + *recordingMBID, + static_cast(static_cast(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 diff --git a/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksParser.hpp b/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksParser.hpp new file mode 100644 index 00000000..4b5644d6 --- /dev/null +++ b/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 . + */ + +#include + +#include "FeedbackTypes.hpp" + +namespace Scrobbling::ListenBrainz +{ + class FeedbacksParser + { + public: + struct Result + { + std::size_t feedbackCount {}; // >= feedbacks.size() + std::vector feedbacks; + }; + + static Result parse(std::string_view msgBody); + }; + +} // Scrobbling::ListenBrainz diff --git a/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp b/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp index 649303f3..78e3d5e8 100644 --- a/src/libs/services/scrobbling/impl/listenbrainz/FeedbacksSynchronizer.cpp +++ b/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 parseTotalFeedbackCount(std::string_view msgBody) { @@ -76,75 +59,6 @@ namespace return std::nullopt; } } - - Feedback - parseFeedback(const Wt::Json::Object& feedbackObj) - { - try - { - const std::optional recordingMBID {UUID::fromString(static_cast(feedbackObj.get("recording_mbid")))}; - if (!recordingMBID) - throw MBIDNotFoundException {}; - - return Feedback - { - Wt::WDateTime::fromTime_t(static_cast(feedbackObj.get("created"))), - *recordingMBID, - static_cast(static_cast(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 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 - || 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) + const std::size_t fetchedFeedbackCount {processGetFeedbacks(msgBodyCopy, context)}; + if (fetchedFeedbackCount == 0 // no more thing available on server + || context.fetchedFeedbackCount >= context.feedbackCount // we may miss something, but we will get it next time + || context.fetchedFeedbackCount >= _maxSyncFeedbackCount) { 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"; - context.fetchedFeedbackCount += parseResult.totalFeedbackCount; + LOG(DEBUG) << "Parsed " << parseResult.feedbackCount << " feedbacks, found " << parseResult.feedbacks.size() << " usable entries"; + 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 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(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++; } } diff --git a/src/libs/services/scrobbling/impl/listenbrainz/ListensParser.hpp b/src/libs/services/scrobbling/impl/listenbrainz/ListensParser.hpp index 506290b9..33563b93 100644 --- a/src/libs/services/scrobbling/impl/listenbrainz/ListensParser.hpp +++ b/src/libs/services/scrobbling/impl/listenbrainz/ListensParser.hpp @@ -19,6 +19,8 @@ #pragma once +#include + #include "ListenTypes.hpp" namespace Scrobbling::ListenBrainz