diff --git a/src/libs/services/database/impl/StarredArtist.cpp b/src/libs/services/database/impl/StarredArtist.cpp index 0427c80b..0508cef8 100644 --- a/src/libs/services/database/impl/StarredArtist.cpp +++ b/src/libs/services/database/impl/StarredArtist.cpp @@ -53,6 +53,17 @@ namespace Database return session.getDboSession().find().where("id = ?").bind(id).resultValue(); } + StarredArtist::pointer StarredArtist::find(Session& session, ArtistId artistId, UserId userId) + { + session.checkSharedLocked(); + return session.getDboSession().query>("SELECT s_a from starred_artist s_a") + .join("user u ON u.id = s_a.user_id") + .where("s_a.artist_id = ?").bind(artistId) + .where("s_a.user_id = ?").bind(userId) + .where("s_a.backend = u.feedback_backend") + .resultValue(); + } + StarredArtist::pointer StarredArtist::find(Session& session, ArtistId artistId, UserId userId, FeedbackBackend backend) { session.checkSharedLocked(); diff --git a/src/libs/services/database/impl/StarredRelease.cpp b/src/libs/services/database/impl/StarredRelease.cpp index 7645d3f9..39c83274 100644 --- a/src/libs/services/database/impl/StarredRelease.cpp +++ b/src/libs/services/database/impl/StarredRelease.cpp @@ -53,6 +53,17 @@ namespace Database return session.getDboSession().find().where("id = ?").bind(id).resultValue(); } + StarredRelease::pointer StarredRelease::find(Session& session, ReleaseId releaseId, UserId userId) + { + session.checkSharedLocked(); + return session.getDboSession().query>("SELECT s_r from starred_release s_r") + .join("user u ON u.id = s_r.user_id") + .where("s_r.release_id = ?").bind(releaseId) + .where("s_r.user_id = ?").bind(userId) + .where("s_r.backend = u.feedback_backend") + .resultValue(); + } + StarredRelease::pointer StarredRelease::find(Session& session, ReleaseId releaseId, UserId userId, FeedbackBackend backend) { session.checkSharedLocked(); diff --git a/src/libs/services/database/impl/StarredTrack.cpp b/src/libs/services/database/impl/StarredTrack.cpp index 78112ded..1a8252e3 100644 --- a/src/libs/services/database/impl/StarredTrack.cpp +++ b/src/libs/services/database/impl/StarredTrack.cpp @@ -53,6 +53,17 @@ namespace Database return session.getDboSession().find().where("id = ?").bind(id).resultValue(); } + StarredTrack::pointer StarredTrack::find(Session& session, TrackId trackId, UserId userId) + { + session.checkSharedLocked(); + return session.getDboSession().query>("SELECT s_t from starred_track s_t") + .join("user u ON u.id = s_t.user_id") + .where("s_t.track_id = ?").bind(trackId) + .where("s_t.user_id = ?").bind(userId) + .where("s_t.backend = u.feedback_backend") + .resultValue(); + } + StarredTrack::pointer StarredTrack::find(Session& session, TrackId trackId, UserId userId, FeedbackBackend backend) { session.checkSharedLocked(); @@ -63,6 +74,15 @@ namespace Database .resultValue(); } + bool StarredTrack::exists(Session& session, TrackId trackId, UserId userId, FeedbackBackend backend) + { + return session.getDboSession().query("SELECT 1 from starred_track") + .where("track_id = ?").bind(trackId) + .where("user_id = ?").bind(userId) + .where("backend = ?").bind(backend) + .resultValue() == 1; + } + RangeResults StarredTrack::find(Session& session, const FindParameters& params) { session.checkSharedLocked(); diff --git a/src/libs/services/database/include/services/database/StarredArtist.hpp b/src/libs/services/database/include/services/database/StarredArtist.hpp index 1d02eaf5..6d53225c 100644 --- a/src/libs/services/database/include/services/database/StarredArtist.hpp +++ b/src/libs/services/database/include/services/database/StarredArtist.hpp @@ -42,6 +42,7 @@ namespace Database // Search utility static std::size_t getCount(Session& session); static pointer find(Session& session, StarredArtistId id); + static pointer find(Session& session, ArtistId artistId, UserId userId); // current backend static pointer find(Session& session, ArtistId artistId, UserId userId, FeedbackBackend backend); // Accessors diff --git a/src/libs/services/database/include/services/database/StarredRelease.hpp b/src/libs/services/database/include/services/database/StarredRelease.hpp index 23d4116e..f42866d4 100644 --- a/src/libs/services/database/include/services/database/StarredRelease.hpp +++ b/src/libs/services/database/include/services/database/StarredRelease.hpp @@ -42,6 +42,7 @@ namespace Database // Search utility static std::size_t getCount(Session& session); static pointer find(Session& session, StarredReleaseId id); + static pointer find(Session& session, ReleaseId releaseId, UserId userId); // current feedback backend static pointer find(Session& session, ReleaseId releaseId, UserId userId, FeedbackBackend backend); // Accessors diff --git a/src/libs/services/database/include/services/database/StarredTrack.hpp b/src/libs/services/database/include/services/database/StarredTrack.hpp index 5db11aaa..fa85f41d 100644 --- a/src/libs/services/database/include/services/database/StarredTrack.hpp +++ b/src/libs/services/database/include/services/database/StarredTrack.hpp @@ -55,7 +55,9 @@ namespace Database // Search utility static std::size_t getCount(Session& session); static pointer find(Session& session, StarredTrackId id); + static pointer find(Session& session, TrackId trackId, UserId userId); // current feedback backend static pointer find(Session& session, TrackId trackId, UserId userId, FeedbackBackend backend); + static bool exists(Session& session, TrackId trackId, UserId userId, FeedbackBackend backend); static RangeResults find(Session& session, const FindParameters& findParams); // Accessors diff --git a/src/libs/services/database/test/StarredArtist.cpp b/src/libs/services/database/test/StarredArtist.cpp index 8be5df9b..55e7d9da 100644 --- a/src/libs/services/database/test/StarredArtist.cpp +++ b/src/libs/services/database/test/StarredArtist.cpp @@ -62,6 +62,29 @@ TEST_F(DatabaseFixture, StarredArtist) artists = Artist::findIds(session, Artist::FindParameters{}.setStarringUser(user2.getId(), FeedbackBackend::Internal)); EXPECT_EQ(artists.results.size(), 0); } + + { + auto transaction{ session.createUniqueTransaction() }; + user.get().modify()->setFeedbackBackend(FeedbackBackend::ListenBrainz); + } + + { + auto transaction{ session.createSharedTransaction() }; + + auto gotArtist{ StarredArtist::find(session, artist->getId(), user->getId()) }; + EXPECT_EQ(gotArtist, Artist::pointer{}); + } + + { + auto transaction{ session.createUniqueTransaction() }; + user.get().modify()->setFeedbackBackend(FeedbackBackend::Internal); + } + + { + auto transaction{ session.createUniqueTransaction() }; + auto gotArtist{ StarredArtist::find(session, artist->getId(), user->getId()) }; + EXPECT_EQ(gotArtist->getId(), starredArtist->getId()); + } } TEST_F(DatabaseFixture, StarredArtist_PendingDestroy) diff --git a/src/libs/services/database/test/StarredRelease.cpp b/src/libs/services/database/test/StarredRelease.cpp index e595a416..91b3871e 100644 --- a/src/libs/services/database/test/StarredRelease.cpp +++ b/src/libs/services/database/test/StarredRelease.cpp @@ -62,6 +62,18 @@ TEST_F(DatabaseFixture, StarredRelease) releases = Release::find(session, Release::FindParameters{}.setStarringUser(user2.getId(), FeedbackBackend::Internal)); EXPECT_EQ(releases.results.size(), 0); } + + { + auto transaction{ session.createUniqueTransaction() }; + user.get().modify()->setFeedbackBackend(FeedbackBackend::ListenBrainz); + } + + { + auto transaction{ session.createSharedTransaction() }; + + auto gotRelease{ StarredRelease::find(session, release->getId(), user->getId()) }; + EXPECT_EQ(gotRelease, StarredRelease::pointer{}); + } } TEST_F(DatabaseFixture, Starredrelease_PendingDestroy) diff --git a/src/libs/services/database/test/StarredTrack.cpp b/src/libs/services/database/test/StarredTrack.cpp index a471215e..cd7f45f2 100644 --- a/src/libs/services/database/test/StarredTrack.cpp +++ b/src/libs/services/database/test/StarredTrack.cpp @@ -26,98 +26,110 @@ using ScopedStarredTrack = ScopedEntity; TEST_F(DatabaseFixture, StarredTrack) { - ScopedTrack track {session, "MyTrack"}; - ScopedUser user {session, "MyUser"}; - ScopedUser user2 {session, "MyUser2"}; + ScopedTrack track{ session, "MyTrack" }; + ScopedUser user{ session, "MyUser" }; + ScopedUser user2{ session, "MyUser2" }; { - auto transaction {session.createSharedTransaction()}; + auto transaction{ session.createSharedTransaction() }; - auto starredTrack {StarredTrack::find(session, track->getId(), user->getId(), FeedbackBackend::Internal)}; + auto starredTrack{ StarredTrack::find(session, track->getId(), user->getId(), FeedbackBackend::Internal) }; EXPECT_FALSE(starredTrack); EXPECT_EQ(StarredTrack::getCount(session), 0); - auto tracks {Track::findIds(session, Track::FindParameters {})}; + auto tracks{ Track::findIds(session, Track::FindParameters {}) }; EXPECT_EQ(tracks.results.size(), 1); } - ScopedStarredTrack starredTrack {session, track.lockAndGet(), user.lockAndGet(), FeedbackBackend::Internal}; + ScopedStarredTrack starredTrack{ session, track.lockAndGet(), user.lockAndGet(), FeedbackBackend::Internal }; { - auto transaction {session.createSharedTransaction()}; + auto transaction{ session.createSharedTransaction() }; - auto gotTrack {StarredTrack::find(session, track->getId(), user->getId(), FeedbackBackend::Internal)}; + auto gotTrack{ StarredTrack::find(session, track->getId(), user->getId(), FeedbackBackend::Internal) }; EXPECT_EQ(gotTrack->getId(), starredTrack->getId()); EXPECT_EQ(StarredTrack::getCount(session), 1); } { - auto transaction {session.createSharedTransaction()}; + auto transaction{ session.createSharedTransaction() }; - auto tracks {Track::findIds(session, Track::FindParameters {})}; + auto tracks{ Track::findIds(session, Track::FindParameters {}) }; EXPECT_EQ(tracks.results.size(), 1); - tracks = Track::findIds(session, Track::FindParameters {}.setStarringUser(user.getId(), FeedbackBackend::Internal)); + tracks = Track::findIds(session, Track::FindParameters{}.setStarringUser(user.getId(), FeedbackBackend::Internal)); EXPECT_EQ(tracks.results.size(), 1); - tracks = Track::findIds(session, Track::FindParameters {}.setStarringUser(user2.getId(), FeedbackBackend::Internal)); + tracks = Track::findIds(session, Track::FindParameters{}.setStarringUser(user2.getId(), FeedbackBackend::Internal)); EXPECT_EQ(tracks.results.size(), 0); } + + { + auto transaction{ session.createUniqueTransaction() }; + user.get().modify()->setFeedbackBackend(FeedbackBackend::ListenBrainz); + } + + { + auto transaction{ session.createSharedTransaction() }; + + auto gotRelease{ StarredTrack::find(session, track->getId(), user->getId()) }; + EXPECT_EQ(gotRelease, StarredTrack::pointer{}); + } } TEST_F(DatabaseFixture, Starredtrack_PendingDestroy) { - ScopedTrack track {session, "MyTrack"}; - ScopedUser user {session, "MyUser"}; - ScopedStarredTrack starredTrack {session, track.lockAndGet(), user.lockAndGet(), FeedbackBackend::Internal}; + ScopedTrack track{ session, "MyTrack" }; + ScopedUser user{ session, "MyUser" }; + ScopedStarredTrack starredTrack{ session, track.lockAndGet(), user.lockAndGet(), FeedbackBackend::Internal }; { - auto transaction {session.createUniqueTransaction()}; + auto transaction{ session.createUniqueTransaction() }; - auto tracks {Track::findIds(session, Track::FindParameters {}.setStarringUser(user.getId(), FeedbackBackend::Internal))}; + auto tracks{ Track::findIds(session, Track::FindParameters {}.setStarringUser(user.getId(), FeedbackBackend::Internal)) }; EXPECT_EQ(tracks.results.size(), 1); starredTrack.get().modify()->setSyncState(SyncState::PendingRemove); - tracks = Track::findIds(session, Track::FindParameters {}.setStarringUser(user.getId(), FeedbackBackend::Internal)); + tracks = Track::findIds(session, Track::FindParameters{}.setStarringUser(user.getId(), FeedbackBackend::Internal)); EXPECT_EQ(tracks.results.size(), 0); } } TEST_F(DatabaseFixture, StarredTrack_dateTime) { - ScopedTrack track1 {session, "MyTrack1"}; - ScopedTrack track2 {session, "MyTrack2"}; - ScopedUser user {session, "MyUser"}; + ScopedTrack track1{ session, "MyTrack1" }; + ScopedTrack track2{ session, "MyTrack2" }; + ScopedUser user{ session, "MyUser" }; - ScopedStarredTrack starredTrack1 {session, track1.lockAndGet(), user.lockAndGet(), FeedbackBackend::Internal}; - ScopedStarredTrack starredTrack2 {session, track2.lockAndGet(), user.lockAndGet(), FeedbackBackend::Internal}; + ScopedStarredTrack starredTrack1{ session, track1.lockAndGet(), user.lockAndGet(), FeedbackBackend::Internal }; + ScopedStarredTrack starredTrack2{ session, track2.lockAndGet(), user.lockAndGet(), FeedbackBackend::Internal }; - const Wt::WDateTime dateTime {Wt::WDate {1950, 1, 2}, Wt::WTime {12, 30, 1}}; + const Wt::WDateTime dateTime{ Wt::WDate {1950, 1, 2}, Wt::WTime {12, 30, 1} }; { - auto transaction {session.createSharedTransaction()}; + auto transaction{ session.createSharedTransaction() }; - auto tracks {Track::findIds(session, Track::FindParameters {}.setStarringUser(user.getId(), FeedbackBackend::Internal))}; + auto tracks{ Track::findIds(session, Track::FindParameters {}.setStarringUser(user.getId(), FeedbackBackend::Internal)) }; EXPECT_EQ(tracks.results.size(), 2); } { - auto transaction {session.createUniqueTransaction()}; + auto transaction{ session.createUniqueTransaction() }; starredTrack1.get().modify()->setDateTime(dateTime); starredTrack2.get().modify()->setDateTime(dateTime.addSecs(-1)); - auto tracks {Track::findIds(session, Track::FindParameters {}.setStarringUser(user.getId(), FeedbackBackend::Internal).setSortMethod(TrackSortMethod::StarredDateDesc))}; + auto tracks{ Track::findIds(session, Track::FindParameters {}.setStarringUser(user.getId(), FeedbackBackend::Internal).setSortMethod(TrackSortMethod::StarredDateDesc)) }; ASSERT_EQ(tracks.results.size(), 2); EXPECT_EQ(tracks.results[0], starredTrack1->getTrack()->getId()); EXPECT_EQ(tracks.results[1], starredTrack2->getTrack()->getId()); } { - auto transaction {session.createUniqueTransaction()}; + auto transaction{ session.createUniqueTransaction() }; starredTrack1.get().modify()->setDateTime(dateTime); starredTrack2.get().modify()->setDateTime(dateTime.addSecs(1)); - auto tracks {Track::findIds(session, Track::FindParameters {}.setStarringUser(user.getId(), FeedbackBackend::Internal).setSortMethod(TrackSortMethod::StarredDateDesc))}; + auto tracks{ Track::findIds(session, Track::FindParameters {}.setStarringUser(user.getId(), FeedbackBackend::Internal).setSortMethod(TrackSortMethod::StarredDateDesc)) }; ASSERT_EQ(tracks.results.size(), 2); EXPECT_EQ(tracks.results[0], starredTrack2->getTrack()->getId()); EXPECT_EQ(tracks.results[1], starredTrack1->getTrack()->getId()); diff --git a/src/libs/services/feedback/impl/FeedbackService.impl.hpp b/src/libs/services/feedback/impl/FeedbackService.impl.hpp index f16be52c..b9220ffd 100644 --- a/src/libs/services/feedback/impl/FeedbackService.impl.hpp +++ b/src/libs/services/feedback/impl/FeedbackService.impl.hpp @@ -82,28 +82,20 @@ namespace Feedback template bool FeedbackService::isStarred(UserId userId, ObjIdType objId) { - const auto backend{ getUserFeedbackBackend(userId) }; - if (!backend) - return false; - Session& session{ _db.getTLSSession() }; auto transaction{ session.createSharedTransaction() }; - typename StarredObjType::pointer starredObj{ StarredObjType::find(session, objId, userId, *backend) }; + typename StarredObjType::pointer starredObj{ StarredObjType::find(session, objId, userId) }; return starredObj && (starredObj->getSyncState() != SyncState::PendingRemove); } template Wt::WDateTime FeedbackService::getStarredDateTime(UserId userId, ObjIdType objId) { - const auto backend{ getUserFeedbackBackend(userId) }; - if (!backend) - return {}; - Session& session{ _db.getTLSSession() }; auto transaction{ session.createSharedTransaction() }; - typename StarredObjType::pointer starredObj{ StarredObjType::find(session, objId, userId, *backend) }; + typename StarredObjType::pointer starredObj{ StarredObjType::find(session, objId, userId) }; if (starredObj && (starredObj->getSyncState() != SyncState::PendingRemove)) return starredObj->getDateTime(); diff --git a/src/libs/services/feedback/impl/listenbrainz/FeedbacksSynchronizer.cpp b/src/libs/services/feedback/impl/listenbrainz/FeedbacksSynchronizer.cpp index 1cd2c64a..6a9e91fa 100644 --- a/src/libs/services/feedback/impl/listenbrainz/FeedbacksSynchronizer.cpp +++ b/src/libs/services/feedback/impl/listenbrainz/FeedbacksSynchronizer.cpp @@ -451,9 +451,7 @@ namespace Feedback::ListenBrainz } trackId = tracks.front()->getId(); - - const StarredTrack::pointer starredTrack{ StarredTrack::find(session, trackId, context.userId, Database::FeedbackBackend::ListenBrainz) }; - needImport = !starredTrack; + needImport = !StarredTrack::exists(session, trackId, context.userId, Database::FeedbackBackend::ListenBrainz); // don't update starred date time // no need to update state if it was found as not synchronized