Listenbrainz: fixed listens parsing. fixes #262

src/libs/services/database/impl/Track.cpp

@@ -45,9 +45,13 @@ createQuery(Session& session, const Track::FindParameters& params)
 
 	auto query {session.getDboSession().query<TrackId>(params.distinct ? "SELECT DISTINCT t.id FROM track t" : "SELECT t.id FROM track t")};
 
+	assert(params.keywords.empty() || params.name.empty());
 	for (std::string_view keyword : params.keywords)
 		query.where("t.name LIKE ? ESCAPE '" ESCAPE_CHAR_STR "'").bind("%" + Utils::escapeLikeKeyword(keyword) + "%");
 
+	if (!params.name.empty())
+		query.where("t.name = ?").bind(params.name);
+
 	if (params.writtenAfter.isValid())
 		query.where("t.file_last_write > ?").bind(params.writtenAfter);
 
@@ -80,11 +84,15 @@ createQuery(Session& session, const Track::FindParameters& params)
 		query.where(oss.str());
 	}
 
-	if (params.artist.isValid())
+	if (params.artist.isValid() || !params.artistName.empty())
 	{
 		query.join("artist a ON a.id = t_a_l.artist_id")
-				.join("track_artist_link t_a_l ON t_a_l.track_id = t.id")
-				.where("a.id = ?").bind(params.artist);
+				.join("track_artist_link t_a_l ON t_a_l.track_id = t.id");
+
+		if (params.artist.isValid())
+			query.where("a.id = ?").bind(params.artist);
+		if (!params.artistName.empty())
+			query.where("a.name = ?").bind(params.artistName);
 
 		if (!params.trackArtistLinkTypes.empty())
 		{
@@ -109,6 +117,11 @@ createQuery(Session& session, const Track::FindParameters& params)
 		query.where("t.release_id IS NULL");
 	else if (params.release.isValid())
 		query.where("t.release_id = ?").bind(params.release);
+	else if (!params.releaseName.empty())
+	{
+		query.join("release r ON t.release_id = r.id");
+		query.where("r.name = ?").bind(params.releaseName);
+	}
 
 	if (params.trackList.isValid())
 	{
@@ -117,6 +130,9 @@ createQuery(Session& session, const Track::FindParameters& params)
 		query.where("t_l.id = ?").bind(params.trackList);
 	}
 
+	if (params.trackNumber)
+		query.where("t.track_number = ?").bind(*params.trackNumber);
+
 	switch (params.sortMethod)
 	{
 		case TrackSortMethod::None:
@@ -282,19 +298,6 @@ Track::find(Session& session, const FindParameters& parameters)
 	return Utils::execQuery(query, parameters.range);
 }
 
-RangeResults<TrackId>
-Track::findByNameAndReleaseName(Session& session, std::string_view trackName, std::string_view releaseName)
-{
-	session.checkSharedLocked();
-
-	auto query {session.getDboSession().query<TrackId>("SELECT t.id from track t")
-		.join("release r ON t.release_id = r.id")
-		.where("t.name = ?").bind(trackName)
-		.where("r.name = ?").bind(releaseName)};
-
-	return Utils::execQuery(query, Range {});
-}
-
 RangeResults<TrackId>
 Track::findSimilarTracks(Session& session, const std::vector<TrackId>& tracks, Range range)
 {

src/libs/services/database/include/services/database/Track.hpp

@@ -59,30 +59,38 @@ class Track : public Object<Track, TrackId>
 	public:
 		struct FindParameters
 		{
-			std::vector<ClusterId>			clusters; // if non empty, tracks that belong to these clusters
-			std::vector<std::string_view>	keywords; // if non empty, name must match all of these keywords
+			std::vector<ClusterId>			clusters;		// if non empty, tracks that belong to these clusters
+			std::vector<std::string_view>	keywords;		// if non empty, name must match all of these keywords
+			std::string						name;			// if non empty, must match this name
 			TrackSortMethod					sortMethod {TrackSortMethod::None};
 			Range							range;
 			Wt::WDateTime					writtenAfter;
 			UserId							starringUser;	// only tracks starred by this user
 			std::optional<Scrobbler>		scrobbler;		// and for this scrobbler
-			ArtistId						artist;						// only tracks that involve this user
+			ArtistId						artist;			// only tracks that involve this artist
+			std::string						artistName;		// only tracks that involve this artist name
 			EnumSet<TrackArtistLinkType>	trackArtistLinkTypes; 		//    and for these link types
 			bool							nonRelease {};	// only tracks that do not belong to a release
 			ReleaseId						release;		// matching this release
+			std::string						releaseName;	// matching this release name
 			TrackListId						trackList;		// matching this trackList
+			std::optional<int>				trackNumber;	// matching this track number
 			bool							distinct {true};
 
 			FindParameters& setClusters(const std::vector<ClusterId>& _clusters) { clusters = _clusters; return *this; }
 			FindParameters& setKeywords(const std::vector<std::string_view>& _keywords) { keywords = _keywords; return *this; }
+			FindParameters& setName(std::string_view _name) { name = _name; return *this; }
 			FindParameters& setSortMethod(TrackSortMethod _method) { sortMethod = _method; return *this; }
 			FindParameters& setRange(Range _range) { range = _range; return *this; }
 			FindParameters& setWrittenAfter(const Wt::WDateTime& _after) { writtenAfter = _after; return *this; }
 			FindParameters& setStarringUser(UserId _user, Scrobbler _scrobbler) { starringUser = _user; scrobbler = _scrobbler; return *this; }
 			FindParameters& setArtist(ArtistId _artist, EnumSet<TrackArtistLinkType> _trackArtistLinkTypes = {}) { artist = _artist; trackArtistLinkTypes = _trackArtistLinkTypes; return *this; }
+			FindParameters& setArtistName(std::string_view _artistName, EnumSet<TrackArtistLinkType> _trackArtistLinkTypes = {}) { artistName = _artistName; trackArtistLinkTypes = _trackArtistLinkTypes; return *this; }
 			FindParameters& setNonRelease(bool _nonRelease) { nonRelease = _nonRelease; return *this; }
 			FindParameters& setRelease(ReleaseId _release) { release = _release; return *this; }
+			FindParameters& setReleaseName(std::string_view _releaseName) { releaseName = _releaseName; return *this; }
 			FindParameters& setTrackList(TrackListId _trackList) { trackList = _trackList; return *this; }
+			FindParameters& setTrackNumber(int _trackNumber) { trackNumber = _trackNumber; return *this; }
 			FindParameters& setDistinct(bool _distinct) { distinct = _distinct; return *this; }
 		};
 
@@ -103,7 +111,6 @@ class Track : public Object<Track, TrackId>
 		static RangeResults<TrackId>	findSimilarTracks(Session& session, const std::vector<TrackId>& trackIds, Range range);
 
 		static RangeResults<TrackId>	find(Session& session, const FindParameters& parameters);
-		static RangeResults<TrackId>	findByNameAndReleaseName(Session& session, std::string_view trackName, std::string_view releaseName);
 		static RangeResults<PathResult>	findPaths(Session& session, Range range);
 		static RangeResults<TrackId>	findRecordingMBIDDuplicates(Session& session, Range range);
 		static RangeResults<TrackId>	findWithRecordingMBIDAndMissingFeatures(Session& session, Range range);

src/libs/services/database/test/Artist.cpp

@@ -65,6 +65,7 @@ TEST_F(DatabaseFixture, Artist_singleTrack)
 	{
 		auto transaction {session.createUniqueTransaction()};
 
+		track.get().modify()->setName("MyTrackName");
 		TrackArtistLink::create(session, track.get(), artist.get(), TrackArtistLinkType::Artist);
 	}
 
@@ -89,6 +90,23 @@ TEST_F(DatabaseFixture, Artist_singleTrack)
 		EXPECT_TRUE(track->getArtists({TrackArtistLinkType::ReleaseArtist}).empty());
 		EXPECT_EQ(track->getArtists({}).size(), 1);
 	}
+
+	{
+		auto transaction {session.createUniqueTransaction()};
+		auto tracks {Track::find(session, Track::FindParameters{}.setName("MyTrackName").setArtistName("MyArtist"))};
+		ASSERT_EQ(tracks.results.size(), 1);
+		EXPECT_EQ(tracks.results.front(), track.getId());
+	}
+	{
+		auto transaction {session.createUniqueTransaction()};
+		auto tracks {Track::find(session, Track::FindParameters{}.setName("MyTrackName").setArtistName("MyArtistFoo"))};
+		EXPECT_EQ(tracks.results.size(), 0);
+	}
+	{
+		auto transaction {session.createUniqueTransaction()};
+		auto tracks {Track::find(session, Track::FindParameters{}.setName("MyTrackNameFoo").setArtistName("MyArtist"))};
+		EXPECT_EQ(tracks.results.size(), 0);
+	}
 }
 
 TEST_F(DatabaseFixture, Artist_singleTracktMultiRoles)

src/libs/services/database/test/Release.cpp

@@ -84,18 +84,18 @@ TEST_F(DatabaseFixture, Release_singleTrack)
 
 		{
 			auto transaction {session.createUniqueTransaction()};
-			auto tracks {Track::findByNameAndReleaseName(session, "MyTrackName", "MyReleaseName")};
+			auto tracks {Track::find(session, Track::FindParameters{}.setName("MyTrackName").setReleaseName("MyReleaseName"))};
 			ASSERT_EQ(tracks.results.size(), 1);
 			EXPECT_EQ(tracks.results.front(), track.getId());
 		}
 		{
 			auto transaction {session.createUniqueTransaction()};
-			auto tracks {Track::findByNameAndReleaseName(session, "MyTrackName", "MyReleaseFoo")};
+			auto tracks {Track::find(session, Track::FindParameters{}.setName("MyTrackName").setReleaseName("MyReleaseFoo"))};
 			EXPECT_EQ(tracks.results.size(), 0);
 		}
 		{
 			auto transaction {session.createUniqueTransaction()};
-			auto tracks {Track::findByNameAndReleaseName(session, "MyTrackFoo", "MyReleaseName")};
+			auto tracks {Track::find(session, Track::FindParameters{}.setName("MyTrackFoo").setReleaseName("MyReleaseName"))};
 			EXPECT_EQ(tracks.results.size(), 0);
 		}
 	}

src/libs/services/scrobbling/CMakeLists.txt

@@ -3,6 +3,7 @@ add_library(lmsscrobbling SHARED
 	impl/internal/InternalScrobbler.cpp
 	impl/listenbrainz/FeedbacksSynchronizer.cpp
 	impl/listenbrainz/ListenBrainzScrobbler.cpp
+	impl/listenbrainz/ListensParser.cpp
 	impl/listenbrainz/ListensSynchronizer.cpp
 	impl/listenbrainz/Utils.cpp
 	impl/ScrobblingService.cpp
@@ -27,3 +28,6 @@ target_link_libraries(lmsscrobbling PUBLIC
 
 install(TARGETS lmsscrobbling DESTINATION lib)
 
+if(BUILD_TESTING)
+	add_subdirectory(test)
+endif()

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

@@ -0,0 +1,122 @@
+/*
+ * 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 "ListensParser.hpp"
+
+#include <Wt/Json/Array.h>
+#include <Wt/Json/Object.h>
+#include <Wt/Json/Value.h>
+#include <Wt/Json/Parser.h>
+
+#include "utils/Logger.hpp"
+#include "Utils.hpp"
+
+namespace
+{
+	using namespace Scrobbling::ListenBrainz;
+
+	ListensParser::Entry
+	parseListen(const Wt::Json::Object& listen)
+	{
+		ListensParser::Entry entry;
+
+		// Mandatory fields
+		const Wt::Json::Object& metadata = listen.get("track_metadata");
+		entry.trackName = static_cast<std::string>(metadata.get("track_name"));
+		entry.artistName = static_cast<std::string>(metadata.get("artist_name"));
+
+		// Optional fields
+		entry.releaseName = static_cast<std::string>(metadata.get("release_name").orIfNull(""));
+		if (listen.type("listened_at") == Wt::Json::Type::Number)
+			entry.listenedAt = Wt::WDateTime::fromTime_t(static_cast<int>(listen.get("listened_at")));
+		if (!entry.listenedAt.isValid())
+			LOG(ERROR) << "Invalid or missing 'listened_at' field!";
+
+		if (metadata.type("additional_info") == Wt::Json::Type::Object)
+		{
+			const Wt::Json::Object& additionalInfo = metadata.get("additional_info");
+			entry.recordingMBID = UUID::fromString(additionalInfo.get("recording_mbid").orIfNull(""));
+			entry.releaseMBID = UUID::fromString(additionalInfo.get("release_mbid").orIfNull(""));
+
+			int trackNumber {additionalInfo.get("tracknumber").orIfNull(-1)};
+			if (trackNumber > 0)
+				entry.trackNumber = trackNumber;
+		}
+
+		return entry;
+	}
+} // namespace
+
+namespace Scrobbling::ListenBrainz
+{
+	std::vector<ListensParser::Entry>
+	ListensParser::parse(std::string_view msgBody)
+	{
+		std::vector<Entry> entries;
+
+		try
+		{
+			Wt::Json::Object root;
+			Wt::Json::parse(std::string {msgBody}, root);
+
+			const Wt::Json::Object& payload = root.get("payload");
+			const Wt::Json::Array& listens = payload.get("listens");
+
+			LOG(DEBUG) << "Parsing " << listens.size() << " listens...";
+
+			if (listens.empty())
+				return entries;
+
+			for (const Wt::Json::Value& value : listens)
+			{
+				try
+				{
+					const Wt::Json::Object& listen = value;
+					entries.push_back(parseListen(listen));
+				}
+				catch (const Wt::WException& error)
+				{
+					LOG(ERROR) << "Cannot parse 'listen': " << error.what();
+				}
+			}
+		}
+		catch (const Wt::WException& error)
+		{
+			LOG(ERROR) << "Cannot parse 'listens': " << error.what();
+		}
+
+		return entries;
+	}
+
+	std::ostream&
+	operator<<(std::ostream& os, const ListensParser::Entry& entry)
+	{
+		os << "track name = '" << entry.trackName << "', artistName = '" << entry.artistName << "'";
+		if (entry.listenedAt.isValid())
+			os << ", listenedAt = " << entry.listenedAt.toString();
+		if (!entry.releaseName.empty())
+			os << ", releaseName = '" << entry.releaseName << "'";
+		if (entry.trackNumber)
+			os << ", trackNumber = " << *entry.trackNumber;
+		if (entry.recordingMBID)
+			os << ", recordingMBID = '" << entry.recordingMBID->getAsString() << "'";
+
+		return os;
+	}
+} // Scrobbling::ListenBrainz

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

@@ -0,0 +1,49 @@
+/*
+ * 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/>.
+ */
+
+#pragma once
+
+#include <string>
+#include <ostream>
+#include <Wt/WDateTime.h>
+
+#include "utils/UUID.hpp"
+
+namespace Scrobbling::ListenBrainz
+{
+	class ListensParser
+	{
+		public:
+			struct Entry
+			{
+				std::string trackName;
+				std::string releaseName;
+				std::string artistName;
+				std::optional<UUID> recordingMBID;
+				std::optional<UUID> releaseMBID;
+				std::optional<unsigned> trackNumber;
+				Wt::WDateTime listenedAt;
+			};
+
+			static std::vector<Entry> parse(std::string_view msgBody);
+	};
+
+	std::ostream& operator<<(std::ostream& os, const ListensParser::Entry& entry);
+
+} // Scrobbling::ListenBrainz

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

@@ -36,6 +36,7 @@
 #include "utils/IConfig.hpp"
 #include "utils/http/IClient.hpp"
 #include "utils/Service.hpp"
+#include "ListensParser.hpp"
 
 #include "Utils.hpp"
 
@@ -142,128 +143,55 @@ namespace
 	}
 
 	Database::TrackId
-	tryMatchListen(Database::Session& session, const Wt::Json::Object& metadata)
+	tryGetMatchingTrack(Database::Session& session, const ListensParser::Entry& listen)
 	{
 		using namespace Database;
 
-		//LOG(DEBUG) << "Trying to match track' " << Wt::Json::serialize(metadata) << "'";
+		auto transaction {session.createSharedTransaction()};
 
-		// first try to get the associated track using MBIDs, and then fallback on names
-		if (metadata.type("additional_info") == Wt::Json::Type::Object)
+		// first try to match using recording MBID, and then fallback on possibly ambiguous info
+		if (listen.recordingMBID)
 		{
-			const Wt::Json::Object& additionalInfo = metadata.get("additional_info");
-			if (std::optional<UUID> recordingMBID {UUID::fromString(additionalInfo.get("recording_mbid").orIfNull(""))})
+			const auto tracks {Track::findByRecordingMBID(session, *listen.recordingMBID)};
+			// if duplicated files, do not record it (let the user correct its database)
+			if (tracks.size() == 1)
 			{
-				const auto tracks {Track::findByRecordingMBID(session, *recordingMBID)};
-				// if duplicated files, do not record it (let the user correct its database)
-				if (tracks.size() == 1)
-					return tracks.front()->getId();
+				LOG(DEBUG) << "Matched listen '" << listen << "' using recording MBID";
+				return tracks.front()->getId();
+			}
+			else if (tracks.size() > 1)
+			{
+				LOG(DEBUG) << "Too many matches for listen '" << listen << "' using recording MBID!";
+				return {};
 			}
 		}
 
-		// these fields are mandatory
-		const std::string trackName {static_cast<std::string>(metadata.get("track_name"))};
-		const std::string releaseName {static_cast<std::string>(metadata.get("release_name"))};
+		assert(!listen.trackName.empty() && !listen.artistName.empty());
 
-		auto tracks {Track::findByNameAndReleaseName(session, trackName, releaseName)};
-		if (tracks.results.size() > 1)
-		{
-			tracks.results.erase(std::remove_if(std::begin(tracks.results), std::end(tracks.results),
-				[&](const TrackId trackId)
-				{
-					const Track::pointer track {Track::find(session, trackId)};
-
-					if (std::string artistName {metadata.get("artist_name").orIfNull("")}; !artistName.empty())
-					{
-						const auto& artists {track->getArtists({TrackArtistLinkType::Artist})};
-						if (std::none_of(std::begin(artists), std::end(artists), [&](const Artist::pointer& artist) { return artist->getName() == artistName; }))
-							return true;
-					}
-					if (metadata.type("additional_info") == Wt::Json::Type::Object)
-					{
-						const Wt::Json::Object& additionalInfo = metadata.get("additional_info");
-						if (track->getTrackNumber())
-						{
-							int otherTrackNumber {additionalInfo.get("tracknumber").orIfNull(-1)};
-							if (otherTrackNumber > 0 && static_cast<std::size_t>(otherTrackNumber) != *track->getTrackNumber())
-								return true;
-						}
-
-						if (auto releaseMBID {track->getRelease()->getMBID()})
-						{
-							if (std::optional<UUID> otherReleaseMBID {UUID::fromString(additionalInfo.get("release_mbid").orIfNull(""))})
-							{
-								if (otherReleaseMBID->getAsString() != releaseMBID->getAsString())
-									return true;
-							}
-						}
-					}
-
-					return false;
-				}), std::end(tracks.results));
-		}
+		// TODO check release MBID?
+		Track::FindParameters params;
+		params.setName(listen.trackName);
+		params.setReleaseName(listen.releaseName);
+		params.setArtistName(listen.artistName);
+		if (listen.trackNumber)
+			params.setTrackNumber(*listen.trackNumber);
 
+		const auto tracks {Track::find(session, params)};
+		// conservative behavior: in case of multiple matches: reject
 		if (tracks.results.size() == 1)
+		{
+			LOG(DEBUG) << "Matched listen '" << listen << "' using metadata";
 			return tracks.results.front();
+		}
+		else if (tracks.results.size() > 1)
+		{
+			LOG(DEBUG) << "Too many matches for listen '" << listen << "' using metadata";
+			return {};
+		}
 
+		LOG(DEBUG) << "No match for listen '" << listen << "'";
 		return {};
 	}
-
-	struct ParseGetListensResult
-	{
-		Wt::WDateTime oldestEntry;
-		std::size_t listenCount{};
-		std::vector<Scrobbling::TimedListen> matchedListens;
-	};
-	ParseGetListensResult
-	parseGetListens(Database::Session& session, std::string_view msgBody, Database::UserId userId)
-	{
-		ParseGetListensResult result;
-
-		try
-		{
-			Wt::Json::Object root;
-			Wt::Json::parse(std::string {msgBody}, root);
-
-			const Wt::Json::Object& payload = root.get("payload");
-			const Wt::Json::Array& listens = payload.get("listens");
-
-			LOG(DEBUG) << "Got " << listens.size() << " listens";
-
-			if (listens.empty())
-				return result;
-
-			auto transaction {session.createSharedTransaction()};
-
-			for (const Wt::Json::Value& value : listens)
-			{
-				const Wt::Json::Object& listen = value;
-				const Wt::WDateTime listenedAt {Wt::WDateTime::fromTime_t(static_cast<int>(listen.get("listened_at")))};
-				const Wt::Json::Object& metadata = listen.get("track_metadata");
-
-				if (!listenedAt.isValid())
-				{
-					LOG(ERROR) << "bad listened_at field!";
-					continue;
-				}
-
-				result.listenCount++;
-				if (!result.oldestEntry.isValid())
-					result.oldestEntry = listenedAt;
-				else if (listenedAt < result.oldestEntry)
-					result.oldestEntry = listenedAt;
-
-				if (Database::TrackId trackId {tryMatchListen(session, metadata)}; trackId.isValid())
-					result.matchedListens.emplace_back(Scrobbling::TimedListen {{userId, trackId}, listenedAt});
-			}
-		}
-		catch (const Wt::WException& error)
-		{
-			LOG(ERROR) << "Cannot parse 'get-listens' result: " << error.what();
-		}
-
-		return result;
-	}
 }
 
 namespace Scrobbling::ListenBrainz
@@ -619,16 +547,30 @@ namespace Scrobbling::ListenBrainz
 	{
 		Database::Session& session {_db.getTLSSession()};
 
-		const ParseGetListensResult parseResult {parseGetListens(session, msgBody, context.userId)};
+		context.maxDateTime = {}; // invalidate to break in case no more listens are fetched
+		std::vector<ListensParser::Entry> parsedListens {ListensParser::parse(msgBody)};
+		context.fetchedListenCount += parsedListens.size();
 
-		context.fetchedListenCount += parseResult.listenCount;
-		context.matchedListenCount += parseResult.matchedListens.size();
-		context.maxDateTime = parseResult.oldestEntry;
-
-		for (const TimedListen& listen : parseResult.matchedListens)
+		for (const ListensParser::Entry& parsedListen : parsedListens)
 		{
-			if (saveListen(listen, Database::ScrobblingState::Synchronized))
-				context.importedListenCount++;
+			// update oldest listen for the next query
+			if (!parsedListen.listenedAt.isValid())
+			{
+				LOG(DEBUG) << "Skipping entry due to invalid listenedAt";
+				continue;
+			}
+
+			if (!context.maxDateTime.isValid() || context.maxDateTime > parsedListen.listenedAt)
+				context.maxDateTime = parsedListen.listenedAt;
+
+			if (const Database::TrackId trackId {tryGetMatchingTrack(session, parsedListen)}; trackId.isValid())
+			{
+				context.matchedListenCount++;
+
+				const Scrobbling::TimedListen listen {{context.userId, trackId}, parsedListen.listenedAt};
+				if (saveListen(listen, Database::ScrobblingState::Synchronized))
+					context.importedListenCount++;
+			}
 		}
 	}
 } // namespace Scrobbling::ListenBrainz

src/libs/services/scrobbling/test/CMakeLists.txt

@@ -0,0 +1,20 @@
+
+add_executable(test-scrobbling
+	Listenbrainz.cpp
+	Scrobbling.cpp
+	)
+
+target_link_libraries(test-scrobbling PRIVATE
+	lmsutils
+	lmsscrobbling
+	GTest::GTest
+	)
+
+target_include_directories(test-scrobbling PRIVATE
+	../impl
+	)
+
+if (NOT CMAKE_CROSSCOMPILING)
+	gtest_discover_tests(test-scrobbling)
+endif()
+

src/libs/services/scrobbling/test/Listenbrainz.cpp

@@ -0,0 +1,101 @@
+/*
+ * 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 <gtest/gtest.h>
+
+#include "listenbrainz/ListensParser.hpp"
+
+using namespace Scrobbling::ListenBrainz;
+
+TEST(Listenbrainz, parseListens_empty)
+{
+	std::vector<ListensParser::Entry> entries {ListensParser::parse("")};
+	EXPECT_EQ(entries.size(), 0);
+
+	entries = ListensParser::parse(R"({"payload":{"count":0,"latest_listen_ts":1664105200,"listens":[],"user_id":"epoupon"}})");
+	EXPECT_EQ(entries.size(), 0);
+}
+
+TEST(Listenbrainz, parseListens_single_missingMBID)
+{
+	std::vector<ListensParser::Entry> entries {ListensParser::parse(R"({"payload":{"count":1,"latest_listen_ts":1663159479,"listens":[{"inserted_at":1650541124,"listened_at":1650541124,"recording_msid":"0e1418e3-b485-413a-84af-6316312cb116","track_metadata":{"additional_info":{"artist_msid":"ab5b27ad-e579-441c-ac60-d5dd9975c044","listening_from":"LMS","recording_msid":"0e1418e3-b485-413a-84af-6316312cb116","release_msid":"3f22f274-a9ee-4cb2-8dd1-f3bd18407099","tracknumber":8},"artist_name":"Broke For Free","release_name":"YEKOMS","track_name":"U2B"},"user_name":"epoupon"}],"user_id":"epoupon"}})")};
+	ASSERT_EQ(entries.size(), 1);
+	EXPECT_EQ(entries[0].trackName, "U2B");
+	EXPECT_EQ(entries[0].releaseName, "YEKOMS");
+	EXPECT_EQ(entries[0].artistName, "Broke For Free");
+	EXPECT_EQ(entries[0].recordingMBID, std::nullopt);
+	EXPECT_EQ(entries[0].releaseMBID, std::nullopt);
+	EXPECT_EQ(entries[0].trackNumber, 8);
+
+	Wt::WDateTime listenedAt;
+	listenedAt.setTime_t(1650541124);
+	EXPECT_EQ(entries[0].listenedAt, listenedAt);
+}
+
+TEST(Listenbrainz, parseListens_twoEntries)
+{
+	std::vector<ListensParser::Entry> entries {ListensParser::parse(R"({"payload":{"count":2,"latest_listen_ts":1664028167,"listens":[{"inserted_at":1664028167,"listened_at":1664028167,"recording_msid":"29c11137-e40b-4875-9ec0-9a20a4bdc2d3","track_metadata":{"additional_info":{"artist_mbids":["069a1c1f-14eb-4d36-b0a0-77dffbd67713"],"artist_msid":null,"listening_from":"LMS","recording_mbid":"46ae879f-2dbe-46d3-99ad-05c116f97a30","recording_msid":"29c11137-e40b-4875-9ec0-9a20a4bdc2d3","release_mbid":"44915500-fbb9-4060-98ce-59a57a429edc","release_msid":null,"track_mbid":"5427a943-a096-4d0b-8b9a-53aca9ed61ac","tracknumber":5},"artist_name":"Broke For Free","mbid_mapping":{"artist_mbids":["069a1c1f-14eb-4d36-b0a0-77dffbd67713"],"recording_mbid":"46ae879f-2dbe-46d3-99ad-05c116f97a30","release_mbid":"44915500-fbb9-4060-98ce-59a57a429edc"},"release_name":"Petal","track_name":"Juparo"},"user_name":"epoupon"},{"inserted_at":1664027919,"listened_at":1664027918,"recording_msid":"fe5abc47-89cd-4235-80b5-00f47cecbe01","track_metadata":{"additional_info":{"artist_mbids":["069a1c1f-14eb-4d36-b0a0-77dffbd67713"],"artist_msid":null,"listening_from":"LMS","recording_mbid":"d89d042c-8cc1-4526-9080-5bab728ee15f","recording_msid":"fe5abc47-89cd-4235-80b5-00f47cecbe01","release_mbid":"44915500-fbb9-4060-98ce-59a57a429edc","release_msid":null,"track_mbid":"9f33a17f-e33e-492f-85a4-7b2e9e09613e","tracknumber":4},"artist_name":"Broke For Free","mbid_mapping":{"artist_mbids":["069a1c1f-14eb-4d36-b0a0-77dffbd67713"],"recording_mbid":"d89d042c-8cc1-4526-9080-5bab728ee15f","release_mbid":"44915500-fbb9-4060-98ce-59a57a429edc"},"release_name":"Petal","track_name":"Melt"},"user_name":"epoupon"}],"user_id":"epoupon"}})")};
+	ASSERT_EQ(entries.size(), 2);
+	EXPECT_EQ(entries[0].trackName, "Juparo");
+	EXPECT_EQ(entries[0].releaseName, "Petal");
+	EXPECT_EQ(entries[0].artistName, "Broke For Free");
+	ASSERT_TRUE(entries[0].recordingMBID.has_value());
+	EXPECT_EQ(entries[0].recordingMBID->getAsString(), "46ae879f-2dbe-46d3-99ad-05c116f97a30");
+	ASSERT_TRUE(entries[0].releaseMBID.has_value());
+	EXPECT_EQ(entries[0].releaseMBID->getAsString(), "44915500-fbb9-4060-98ce-59a57a429edc");
+	EXPECT_EQ(entries[0].trackNumber, 5);
+
+	EXPECT_EQ(entries[1].trackName, "Melt");
+	EXPECT_EQ(entries[1].releaseName, "Petal");
+	EXPECT_EQ(entries[1].artistName, "Broke For Free");
+	ASSERT_TRUE(entries[1].recordingMBID.has_value());
+	EXPECT_EQ(entries[1].recordingMBID->getAsString(), "d89d042c-8cc1-4526-9080-5bab728ee15f");
+	ASSERT_TRUE(entries[1].releaseMBID.has_value());
+	EXPECT_EQ(entries[1].releaseMBID->getAsString(), "44915500-fbb9-4060-98ce-59a57a429edc");
+	EXPECT_EQ(entries[1].trackNumber, 4);
+}
+
+TEST(Listenbrainz, parseListens_entryNotFromLms)
+{
+	std::vector<ListensParser::Entry> entries {ListensParser::parse(R"({"payload":{"count":1,"latest_listen_ts":1664105730,"listens":[{"inserted_at":1664105730,"listened_at":1664105730,"recording_msid":"6a11ff4d-0623-4b2e-98e0-0e172f1f28d7","track_metadata":{"additional_info":{"artist_msid":null,"media_player":"BrainzPlayer","music_service":"youtube.com","music_service_name":"youtube","origin_url":"https://www.youtube.com/watch?v=EBP5vL3YWTI","recording_msid":"6a11ff4d-0623-4b2e-98e0-0e172f1f28d7","release_msid":null,"submission_client":"BrainzPlayer"},"artist_name":"Dio","brainzplayer_metadata":{"track_name":"Dio - Breathless"},"mbid_mapping":{"artist_mbids":["c55193fb-f5d2-4839-a263-4c044fca1456"],"recording_mbid":"92929526-21d7-4e75-b759-1072951664c4","release_mbid":"16cbf9ba-2e38-3893-9f23-f8567e26c18b"},"release_name":"The Last in Line","track_name":"Breathless"},"user_name":"epoupon"}],"user_id":"epoupon"}})")};
+	ASSERT_EQ(entries.size(), 1);
+	EXPECT_EQ(entries[0].trackName, "Breathless");
+	EXPECT_EQ(entries[0].releaseName, "The Last in Line");
+	EXPECT_EQ(entries[0].artistName, "Dio");
+	EXPECT_FALSE(entries[0].recordingMBID.has_value());
+	EXPECT_FALSE(entries[0].releaseMBID.has_value());
+}
+
+TEST(Listenbrainz, parseListens_multiArtists)
+{
+	std::vector<ListensParser::Entry> entries {ListensParser::parse(R"({"payload":{"count":1,"latest_listen_ts":1664106427,"listens":[{"inserted_at":1664106427,"listened_at":1664106427,"recording_msid":"b1dad0df-329b-443d-bacf-cdbebdddbfd0","track_metadata":{"additional_info":{"artist_mbids":["04ce0202-043d-4cbe-8f09-8abaf3b80c71","79311c51-9748-49df-baa1-d925fd29f4e8"],"artist_msid":null,"listening_from":"LMS","recording_mbid":"a5f380bc-0a85-4a9f-88db-d41bb9aa2a4b","recording_msid":"b1dad0df-329b-443d-bacf-cdbebdddbfd0","release_mbid":"147b4669-3d20-43f8-89c0-ba1da8b87dd3","release_msid":null,"track_mbid":"a20dd067-29b6-3d38-a0be-eeb86b4671c1","tracknumber":1},"artist_name":"Gloom","release_name":"Demovibes 9: Party, people going","track_name":"Stargazer of Disgrace"},"user_name":"epoupon"}],"user_id":"epoupon"}})")};
+	ASSERT_EQ(entries.size(), 1);
+}
+
+TEST(Listenbrainz, parseListens_minPayload)
+{
+	std::vector<ListensParser::Entry> entries {ListensParser::parse(R"({"payload":{"count":1,"latest_listen_ts":1664106427,"listens":[{"track_metadata":{"artist_name":"Gloom","track_name":"Stargazer of Disgrace"},"user_name":"epoupon"}],"user_id":"epoupon"}})")};
+	ASSERT_EQ(entries.size(), 1);
+	EXPECT_FALSE(entries[0].listenedAt.isValid());
+	EXPECT_EQ(entries[0].trackName, "Stargazer of Disgrace");
+	EXPECT_EQ(entries[0].artistName, "Gloom");
+	EXPECT_EQ(entries[0].releaseName, "");
+	EXPECT_FALSE(entries[0].recordingMBID.has_value());
+	EXPECT_FALSE(entries[0].releaseMBID.has_value());
+}

src/libs/services/scrobbling/test/Scrobbling.cpp

@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2021 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 <gtest/gtest.h>
+
+#include "utils/Logger.hpp"
+#include "utils/Service.hpp"
+#include "utils/StreamLogger.hpp"
+
+int main(int argc, char **argv)
+{
+	// log to stdout
+	Service<Logger> logger {std::make_unique<StreamLogger>(std::cout, EnumSet<Severity> {Severity::FATAL, Severity::ERROR})};
+
+	::testing::InitGoogleTest(&argc, argv);
+	return RUN_ALL_TESTS();
+}
+

src/libs/utils/impl/StreamLogger.cpp

@@ -19,14 +19,16 @@
 
 #include "utils/StreamLogger.hpp"
 
-StreamLogger::StreamLogger(std::ostream& os)
+StreamLogger::StreamLogger(std::ostream& os, EnumSet<Severity> severities)
 : _os {os}
+, _severities {severities}
 {
 }
 
 void
 StreamLogger::processLog(const Log& log)
 {
-	_os << "[" << getSeverityName(log.getSeverity()) << "] [" << getModuleName(log.getModule()) << "] " << log.getMessage() << std::endl;
+	if (_severities.contains(log.getSeverity()))
+		_os << "[" << getSeverityName(log.getSeverity()) << "] [" << getModuleName(log.getModule()) << "] " << log.getMessage() << std::endl;
 }
 

src/libs/utils/include/utils/EnumSet.hpp

@@ -20,6 +20,9 @@
 #pragma once
 
 #include <cassert>
+#include <cstdint>
+#include <initializer_list>
+#include <limits>
 #include <type_traits>
 
 template <typename T, typename underlying_type = std::uint32_t>
@@ -47,13 +50,13 @@ class EnumSet
 
 		constexpr void insert(T value)
 		{
-			assert(static_cast<size_t>(value) < sizeof(_bitfield) * 8);
+			assert(static_cast<std::size_t>(value) < sizeof(_bitfield) * 8);
 			_bitfield |= (underlying_type{ 1 } << static_cast<underlying_type>(value));
 		}
 
 		constexpr void erase(T value)
 		{
-			assert(static_cast<size_t>(value) < sizeof(_bitfield) * 8);
+			assert(static_cast<std::size_t>(value) < sizeof(_bitfield) * 8);
 			_bitfield &= ~(underlying_type{ 1 } << static_cast<underlying_type>(value));
 		}
 
@@ -64,7 +67,7 @@ class EnumSet
 
 		constexpr bool contains(T value) const
 		{
-			assert(static_cast<size_t>(value) < sizeof(_bitfield) * 8);
+			assert(static_cast<std::size_t>(value) < sizeof(_bitfield) * 8);
 			return _bitfield & (underlying_type{ 1 } << static_cast<underlying_type>(value));
 		}
 

src/libs/utils/include/utils/StreamLogger.hpp

@@ -19,16 +19,20 @@
 
 #pragma once
 
-#include "Logger.hpp"
+#include "utils/EnumSet.hpp"
+#include "utils/Logger.hpp"
 
 class StreamLogger final : public Logger
 {
 	public:
-		StreamLogger(std::ostream& oss);
+		static constexpr EnumSet<Severity> defaultSeverities {Severity::FATAL, Severity::ERROR, Severity::WARNING, Severity::INFO};
+
+		StreamLogger(std::ostream& oss, EnumSet<Severity> severities = defaultSeverities);
 
 		void processLog(const Log& log);
 
 	private:
 		std::ostream& _os;
+		const EnumSet<Severity> _severities;
 };