From febfe518ba07fe0490e73b0917164951350466d3 Mon Sep 17 00:00:00 2001 From: emeric Date: Thu, 16 May 2019 13:49:56 +0200 Subject: [PATCH] Added unit tests on orphaned entries --- src/database/Cluster.cpp | 8 +++ src/database/Cluster.hpp | 1 + src/scanner/MediaScanner.cpp | 10 +-- test/database/DatabaseTest.cpp | 122 ++++++++++++++++++++++++++------- 4 files changed, 111 insertions(+), 30 deletions(-) diff --git a/src/database/Cluster.cpp b/src/database/Cluster.cpp index 014fec9a..c2fd8f7f 100644 --- a/src/database/Cluster.cpp +++ b/src/database/Cluster.cpp @@ -51,6 +51,14 @@ Cluster::getAll(Wt::Dbo::Session& session) return std::vector(res.begin(), res.end()); } +std::vector +Cluster::getAllOrphans(Wt::Dbo::Session& session) +{ + Wt::Dbo::collection res {session.query("SELECT DISTINCT c FROM cluster c WHERE NOT EXISTS(SELECT 1 FROM track t INNER JOIN track_cluster t_c ON t.id = t_c.track_id)")}; + + return std::vector(res.begin(), res.end()); +} + Cluster::pointer Cluster::getById(Wt::Dbo::Session& session, IdType id) { diff --git a/src/database/Cluster.hpp b/src/database/Cluster.hpp index 82f6b122..09689d33 100644 --- a/src/database/Cluster.hpp +++ b/src/database/Cluster.hpp @@ -44,6 +44,7 @@ class Cluster : public Wt::Dbo::Dbo // Find utility static std::vector getAll(Wt::Dbo::Session& session); + static std::vector getAllOrphans(Wt::Dbo::Session& session); static pointer getById(Wt::Dbo::Session& session, IdType id); // Create utility diff --git a/src/scanner/MediaScanner.cpp b/src/scanner/MediaScanner.cpp index bb9a0653..bc1744ed 100644 --- a/src/scanner/MediaScanner.cpp +++ b/src/scanner/MediaScanner.cpp @@ -649,16 +649,12 @@ MediaScanner::removeOrphanEntries() { Wt::Dbo::Transaction transaction(_db.getSession()); - // TODO better query for this // Now process orphan Cluster (no track) - auto clusters = Cluster::getAll(_db.getSession()); + auto clusters = Cluster::getAllOrphans(_db.getSession()); for (auto cluster : clusters) { - if (cluster->getCount() == 0) - { - LMS_LOG(DBUPDATER, DEBUG) << "Removing orphan cluster '" << cluster->getName() << "'"; - cluster.remove(); - } + LMS_LOG(DBUPDATER, DEBUG) << "Removing orphan cluster '" << cluster->getName() << "'"; + cluster.remove(); } } diff --git a/test/database/DatabaseTest.cpp b/test/database/DatabaseTest.cpp index 1d2ca1b9..4486c53f 100644 --- a/test/database/DatabaseTest.cpp +++ b/test/database/DatabaseTest.cpp @@ -39,11 +39,6 @@ class ScopedFileDeleter final boost::filesystem::path _path; }; -/*static void check(bool cond) -{ - if (!cond) -}*/ - #define CHECK(PRED) \ { \ if (!(PRED)) \ @@ -172,9 +167,16 @@ testSingleCluster(Wt::Dbo::Session& session) CHECK(clusters.front().id() == clusterId); CHECK(clusters.front()->getType().id() == clusterTypeId); + clusters = Cluster::getAllOrphans(session); + CHECK(clusters.size() == 1); + CHECK(clusters.front().id() == clusterId); + auto clusterTypes {ClusterType::getAll(session)}; CHECK(clusterTypes.size() == 1); CHECK(clusterTypes.front().id() == clusterTypeId); + + clusterTypes = ClusterType::getAllOrphans(session); + CHECK(clusterTypes.empty()); } { @@ -182,11 +184,18 @@ testSingleCluster(Wt::Dbo::Session& session) auto cluster {Cluster::getById(session, clusterId)}; CHECK(cluster); - CHECK(cluster->getType().id() == clusterTypeId); + cluster.remove(); + + auto clusterTypes {ClusterType::getAllOrphans(session)}; + CHECK(clusterTypes.size() == 1); + } + + { + Wt::Dbo::Transaction transaction {session}; + auto clusterType {ClusterType::getById(session, clusterTypeId)}; CHECK(clusterType); - cluster.remove(); clusterType.remove(); } } @@ -212,6 +221,11 @@ testSingleTrackSingleArtist(Wt::Dbo::Session& session) artistId = artist.id(); } + { + Wt::Dbo::Transaction transaction {session}; + CHECK(Artist::getAllOrphans(session).empty()); + } + { Wt::Dbo::Transaction transaction {session}; @@ -229,7 +243,7 @@ testSingleTrackSingleArtist(Wt::Dbo::Session& session) CHECK(artistLink->getArtist().id() == artistId); CHECK(track->getArtists(TrackArtistLink::Type::Artist).size() == 1); - CHECK(track->getArtists(TrackArtistLink::Type::ReleaseArtist).size() == 0); + CHECK(track->getArtists(TrackArtistLink::Type::ReleaseArtist).empty()); } { @@ -242,7 +256,7 @@ testSingleTrackSingleArtist(Wt::Dbo::Session& session) auto track {tracks.front()}; CHECK(track.id() == trackId); - CHECK(artist->getTracks(TrackArtistLink::Type::ReleaseArtist).size() == 0); + CHECK(artist->getTracks(TrackArtistLink::Type::ReleaseArtist).empty()); CHECK(artist->getTracks(TrackArtistLink::Type::Artist).size() == 1); track.remove(); @@ -271,6 +285,11 @@ testSingleTrackSingleArtistMultiRoles(Wt::Dbo::Session& session) artistId = artist.id(); } + { + Wt::Dbo::Transaction transaction {session}; + CHECK(Artist::getAllOrphans(session).empty()); + } + { Wt::Dbo::Transaction transaction {session}; @@ -331,6 +350,11 @@ testSingleTrackMultiArtists(Wt::Dbo::Session& session) CHECK(artist1Id != artist2Id); } + { + Wt::Dbo::Transaction transaction {session}; + CHECK(Artist::getAllOrphans(session).empty()); + } + { Wt::Dbo::Transaction transaction {session}; @@ -343,7 +367,7 @@ testSingleTrackMultiArtists(Wt::Dbo::Session& session) || (artists[0].id() == artist2Id && artists[1].id() == artist1Id)); CHECK(track->getArtists(TrackArtistLink::Type::Artist).size() == 2); - CHECK(track->getArtists(TrackArtistLink::Type::ReleaseArtist).size() == 0); + CHECK(track->getArtists(TrackArtistLink::Type::ReleaseArtist).empty()); CHECK(Artist::getAll(session).size() == 2); } @@ -361,9 +385,9 @@ testSingleTrackMultiArtists(Wt::Dbo::Session& session) CHECK(artist2->getTracks().front() == track); - CHECK(artist1->getTracks(TrackArtistLink::Type::ReleaseArtist).size() == 0); + CHECK(artist1->getTracks(TrackArtistLink::Type::ReleaseArtist).empty()); CHECK(artist1->getTracks(TrackArtistLink::Type::Artist).size() == 1); - CHECK(artist2->getTracks(TrackArtistLink::Type::ReleaseArtist).size() == 0); + CHECK(artist2->getTracks(TrackArtistLink::Type::ReleaseArtist).empty()); CHECK(artist2->getTracks(TrackArtistLink::Type::Artist).size() == 1); track.remove(); @@ -395,18 +419,32 @@ testSingleTrackSingleRelease(Wt::Dbo::Session& session) { Wt::Dbo::Transaction transaction {session}; - - auto track {Track::getById(session, trackId)}; - CHECK(track); - CHECK(track->getRelease()); - CHECK(track->getRelease().id() == releaseId); + CHECK(Release::getAllOrphans(session).empty()); auto release {Release::getById(session, releaseId)}; CHECK(release); CHECK(release->getTracks().size() == 1); CHECK(release->getTracks().front().id() == trackId); + } + { + Wt::Dbo::Transaction transaction {session}; + + auto track {Track::getById(session, trackId)}; + CHECK(track); + CHECK(track->getRelease()); + CHECK(track->getRelease().id() == releaseId); track.remove(); + } + + { + Wt::Dbo::Transaction transaction {session}; + + CHECK(Release::getAllOrphans(session).size() == 1); + auto release {Release::getById(session, releaseId)}; + CHECK(release); + CHECK(release->getTracks().empty()); + release.remove(); } } @@ -433,6 +471,11 @@ testSingleTrackSingleCluster(Wt::Dbo::Session& session) clusterId = cluster.id(); } + { + Wt::Dbo::Transaction transaction {session}; + CHECK(Cluster::getAllOrphans(session).empty()); + } + { Wt::Dbo::Transaction transaction {session}; @@ -480,6 +523,12 @@ testSingleTrackSingleReleaseSingleCluster(Wt::Dbo::Session& session) clusterId = cluster.id(); } + { + Wt::Dbo::Transaction transaction {session}; + CHECK(Cluster::getAllOrphans(session).empty()); + CHECK(Release::getAllOrphans(session).empty()); + } + { Wt::Dbo::Transaction transaction {session}; @@ -532,6 +581,13 @@ testSingleTrackSingleArtistMultiClusters(Wt::Dbo::Session& session) cluster2Id = cluster2.id(); } + { + Wt::Dbo::Transaction transaction {session}; + CHECK(Cluster::getAllOrphans(session).empty()); + CHECK(Release::getAllOrphans(session).empty()); + CHECK(Artist::getAllOrphans(session).empty()); + } + { Wt::Dbo::Transaction transaction {session}; @@ -540,7 +596,7 @@ testSingleTrackSingleArtistMultiClusters(Wt::Dbo::Session& session) CHECK(artists.front().id() == artistId); artists = Artist::getByFilter(session, {cluster2Id}); - CHECK(artists.size() == 0); + CHECK(artists.empty()); auto cluster2 {Cluster::getById(session, cluster2Id)}; auto track {Track::getById(session, trackId)}; @@ -607,6 +663,13 @@ testSingleTrackSingleArtistMultiRolesMultiClusters(Wt::Dbo::Session& session) clusterId = cluster.id(); } + { + Wt::Dbo::Transaction transaction {session}; + CHECK(Cluster::getAllOrphans(session).empty()); + CHECK(Release::getAllOrphans(session).empty()); + CHECK(Artist::getAllOrphans(session).empty()); + } + { Wt::Dbo::Transaction transaction {session}; @@ -663,6 +726,12 @@ testMultiTracksSingleArtistMultiClusters(Wt::Dbo::Session& session) clusterTypeId = clusterType.id(); } + { + Wt::Dbo::Transaction transaction {session}; + CHECK(Cluster::getAllOrphans(session).empty()); + CHECK(Artist::getAllOrphans(session).empty()); + } + { Wt::Dbo::Transaction transaction {session}; @@ -774,6 +843,13 @@ testSingleTrackSingleReleaseSingleArtistSingleCluster(Wt::Dbo::Session& session) clusterTypeId = clusterType.id(); } + { + Wt::Dbo::Transaction transaction {session}; + CHECK(Cluster::getAllOrphans(session).empty()); + CHECK(Artist::getAllOrphans(session).empty()); + CHECK(Release::getAllOrphans(session).empty()); + } + { Wt::Dbo::Transaction transaction {session}; @@ -882,11 +958,11 @@ testDatabaseEmpty(Wt::Dbo::Session& session) { Wt::Dbo::Transaction transaction {session}; - CHECK(Artist::getAll(session).size() == 0); - CHECK(Cluster::getAll(session).size() == 0); - CHECK(ClusterType::getAll(session).size() == 0); - CHECK(Release::getAll(session).size() == 0); - CHECK(Track::getAll(session).size() == 0); + CHECK(Artist::getAll(session).empty()); + CHECK(Cluster::getAll(session).empty()); + CHECK(ClusterType::getAll(session).empty()); + CHECK(Release::getAll(session).empty()); + CHECK(Track::getAll(session).empty()); } int main(int argc, char* argv[])