Remove unnecessary joins

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

@@ -125,13 +125,12 @@ namespace Database
             std::ostringstream oss;
             oss << "r.id IN (SELECT DISTINCT r.id FROM release r"
                 " INNER JOIN track t ON t.release_id = r.id"
-                " INNER JOIN cluster c ON c.id = t_c.cluster_id"
                 " INNER JOIN track_cluster t_c ON t_c.track_id = t.id";
 
             WhereClause clusterClause;
             for (const ClusterId clusterId : params.clusters)
             {
-                clusterClause.Or(WhereClause("c.id = ?"));
+                clusterClause.Or(WhereClause("t_c.cluster_id = ?"));
                 query.bind(clusterId);
             }
 

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

@@ -35,11 +35,11 @@
 #include "StringViewTraits.hpp"
 #include "Utils.hpp"
 
-namespace Database {
+namespace Database
-
+{
-static
+    namespace
-Wt::Dbo::Query<TrackId>
+    {
-createQuery(Session& session, const Track::FindParameters& params)
+        Wt::Dbo::Query<TrackId> createQuery(Session& session, const Track::FindParameters& params)
         {
             session.checkSharedLocked();
 
@@ -68,13 +68,12 @@ createQuery(Session& session, const Track::FindParameters& params)
             {
                 std::ostringstream oss;
                 oss << "t.id IN (SELECT DISTINCT t.id FROM track t"
-			" INNER JOIN track_cluster t_c ON t_c.track_id = t.id"
+                    " INNER JOIN track_cluster t_c ON t_c.track_id = t.id";
-			" INNER JOIN cluster c ON c.id = t_c.cluster_id";
 
                 WhereClause clusterClause;
                 for (const ClusterId clusterId : params.clusters)
                 {
-			clusterClause.Or(WhereClause("c.id = ?"));
+                    clusterClause.Or(WhereClause("t_c.cluster_id = ?"));
                     query.bind(clusterId);
                 }
 
@@ -163,14 +162,14 @@ createQuery(Session& session, const Track::FindParameters& params)
 
             return query;
         }
+    }
 
     Track::Track(const std::filesystem::path& p)
         : _filePath{ p.string() }
     {
     }
 
-Track::pointer
+    Track::pointer Track::create(Session& session, const std::filesystem::path& p)
-Track::create(Session& session, const std::filesystem::path& p)
     {
         return session.getDboSession().add(std::unique_ptr<Track> {new Track{ p }});
     }
@@ -183,16 +182,14 @@ Track::getCount(Session& session)
         return session.getDboSession().query<int>("SELECT COUNT(*) FROM track");
     }
 
-Track::pointer
+    Track::pointer Track::findByPath(Session& session, const std::filesystem::path& p)
-Track::findByPath(Session& session, const std::filesystem::path& p)
     {
         session.checkSharedLocked();
 
         return session.getDboSession().find<Track>().where("file_path = ?").bind(p.string()).resultValue();
     }
 
-Track::pointer
+    Track::pointer Track::find(Session& session, TrackId id)
-Track::find(Session& session, TrackId id)
     {
         session.checkSharedLocked();
 
@@ -201,16 +198,14 @@ Track::find(Session& session, TrackId id)
             .resultValue();
     }
 
-bool
+    bool Track::exists(Session& session, TrackId id)
-Track::exists(Session& session, TrackId id)
     {
         session.checkSharedLocked();
 
         return session.getDboSession().query<int>("SELECT 1 from track").where("id = ?").bind(id).resultValue() == 1;
     }
 
-std::vector<Track::pointer>
+    std::vector<Track::pointer> Track::findByMBID(Session& session, const UUID& mbid)
-Track::findByMBID(Session& session, const UUID& mbid)
     {
         session.checkSharedLocked();
 
@@ -221,8 +216,7 @@ Track::findByMBID(Session& session, const UUID& mbid)
         return std::vector<Track::pointer>(res.begin(), res.end());
     }
 
-std::vector<Track::pointer>
+    std::vector<Track::pointer> Track::findByRecordingMBID(Session& session, const UUID& mbid)
-Track::findByRecordingMBID(Session& session, const UUID& mbid)
     {
         session.checkSharedLocked();
 
@@ -233,8 +227,7 @@ Track::findByRecordingMBID(Session& session, const UUID& mbid)
         return std::vector<Track::pointer>(res.begin(), res.end());
     }
 
-RangeResults<Track::PathResult>
+    RangeResults<Track::PathResult> Track::findPaths(Session& session, Range range)
-Track::findPaths(Session& session, Range range)
     {
         using QueryResultType = std::tuple<TrackId, std::string>;
         session.checkSharedLocked();
@@ -258,8 +251,7 @@ Track::findPaths(Session& session, Range range)
         return res;
     }
 
-RangeResults<TrackId>
+    RangeResults<TrackId> Track::findTrackMBIDDuplicates(Session& session, Range range)
-Track::findTrackMBIDDuplicates(Session& session, Range range)
     {
         session.checkSharedLocked();
 
@@ -269,8 +261,7 @@ Track::findTrackMBIDDuplicates(Session& session, Range range)
         return Utils::execQuery(query, range);
     }
 
-RangeResults<TrackId>
+    RangeResults<TrackId> Track::findWithRecordingMBIDAndMissingFeatures(Session& session, Range range)
-Track::findWithRecordingMBIDAndMissingFeatures(Session& session, Range range)
     {
         session.checkSharedLocked();
 
@@ -281,14 +272,12 @@ Track::findWithRecordingMBIDAndMissingFeatures(Session& session, Range range)
         return Utils::execQuery(query, range);
     }
 
-std::vector<Cluster::pointer>
+    std::vector<Cluster::pointer> Track::getClusters() const
-Track::getClusters() const
     {
         return std::vector<Cluster::pointer>(_clusters.begin(), _clusters.end());
     }
 
-std::vector<ClusterId>
+    std::vector<ClusterId> Track::getClusterIds() const
-Track::getClusterIds() const
     {
         assert(session());
 
@@ -300,8 +289,7 @@ Track::getClusterIds() const
         return std::vector<ClusterId>(res.begin(), res.end());
     }
 
-RangeResults<TrackId>
+    RangeResults<TrackId> Track::find(Session& session, const FindParameters& parameters)
-Track::find(Session& session, const FindParameters& parameters)
     {
         session.checkSharedLocked();
 
@@ -310,8 +298,7 @@ Track::find(Session& session, const FindParameters& parameters)
         return Utils::execQuery(query, parameters.range);
     }
 
-RangeResults<TrackId>
+    RangeResults<TrackId> Track::findSimilarTracks(Session& session, const std::vector<TrackId>& tracks, Range range)
-Track::findSimilarTracks(Session& session, const std::vector<TrackId>& tracks, Range range)
     {
         assert(!tracks.empty());
         session.checkSharedLocked();
@@ -341,52 +328,44 @@ Track::findSimilarTracks(Session& session, const std::vector<TrackId>& tracks, R
         return Utils::execQuery(query, range);
     }
 
-void
+    void Track::clearArtistLinks()
-Track::clearArtistLinks()
     {
         _trackArtistLinks.clear();
     }
 
-void
+    void Track::addArtistLink(const ObjectPtr<TrackArtistLink>& artistLink)
-Track::addArtistLink(const ObjectPtr<TrackArtistLink>& artistLink)
     {
         _trackArtistLinks.insert(getDboPtr(artistLink));
     }
 
-void
+    void Track::setClusters(const std::vector<ObjectPtr<Cluster>>& clusters)
-Track::setClusters(const std::vector<ObjectPtr<Cluster>>& clusters)
     {
         _clusters.clear();
         for (const ObjectPtr<Cluster>& cluster : clusters)
             _clusters.insert(getDboPtr(cluster));
     }
 
-std::optional<int>
+    std::optional<int> Track::getYear() const
-Track::getYear() const
     {
         return (_date.isValid() ? std::make_optional<int>(_date.year()) : std::nullopt);
     }
 
-std::optional<int>
+    std::optional<int> Track::getOriginalYear() const
-Track::getOriginalYear() const
     {
         return (_originalDate.isValid() ? std::make_optional<int>(_originalDate.year()) : std::nullopt);
     }
 
-std::optional<std::string>
+    std::optional<std::string> Track::getCopyright() const
-Track::getCopyright() const
     {
         return _copyright != "" ? std::make_optional<std::string>(_copyright) : std::nullopt;
     }
 
-std::optional<std::string>
+    std::optional<std::string> Track::getCopyrightURL() const
-Track::getCopyrightURL() const
     {
         return _copyrightURL != "" ? std::make_optional<std::string>(_copyrightURL) : std::nullopt;
     }
 
-std::vector<Artist::pointer>
+    std::vector<Artist::pointer> Track::getArtists(EnumSet<TrackArtistLinkType> linkTypes) const
-Track::getArtists(EnumSet<TrackArtistLinkType> linkTypes) const
     {
         assert(session());
 
@@ -421,8 +400,7 @@ Track::getArtists(EnumSet<TrackArtistLinkType> linkTypes) const
         return std::vector<Artist::pointer>(std::begin(res), std::end(res));
     }
 
-std::vector<ArtistId>
+    std::vector<ArtistId> Track::getArtistIds(EnumSet<TrackArtistLinkType> linkTypes) const
-Track::getArtistIds(EnumSet<TrackArtistLinkType> linkTypes) const
     {
         assert(self());
         assert(session());
@@ -458,14 +436,12 @@ Track::getArtistIds(EnumSet<TrackArtistLinkType> linkTypes) const
         return std::vector<ArtistId>(std::cbegin(res), std::cend(res));
     }
 
-std::vector<TrackArtistLink::pointer>
+    std::vector<TrackArtistLink::pointer> Track::getArtistLinks() const
-Track::getArtistLinks() const
     {
         return std::vector<TrackArtistLink::pointer>(_trackArtistLinks.begin(), _trackArtistLinks.end());
     }
 
-std::vector<std::vector<Cluster::pointer>>
+    std::vector<std::vector<Cluster::pointer>> Track::getClusterGroups(const std::vector<ClusterType::pointer>& clusterTypes, std::size_t size) const
-Track::getClusterGroups(const std::vector<ClusterType::pointer>& clusterTypes, std::size_t size) const
     {
         assert(self());
         assert(session());
@@ -508,8 +484,7 @@ Track::getClusterGroups(const std::vector<ClusterType::pointer>& clusterTypes, s
 
     namespace Debug
     {
-	std::ostream&
+        std::ostream& operator<<(std::ostream& os, const TrackInfo& trackInfo)
-	operator<<(std::ostream& os, const TrackInfo& trackInfo)
         {
             auto transaction{ trackInfo.session.createSharedTransaction() };
 
@@ -535,4 +510,3 @@ namespace Debug
     }
 
 } // namespace Database
-

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

@@ -154,6 +154,51 @@ TEST_F(DatabaseFixture, Cluster_singleTrack)
     }
 }
 
+TEST_F(DatabaseFixture, Cluster_singleTrackWithSeveralClusters)
+{
+    ScopedTrack track{ session, "MyTrack" };
+    ScopedClusterType clusterType{ session, "MyClusterType" };
+
+    ScopedCluster cluster1{ session, clusterType.lockAndGet(), "MyCluster1" };
+    ScopedCluster cluster2{ session, clusterType.lockAndGet(), "MyCluster2" };
+
+    const std::vector<ClusterId> clusterIds{ cluster1.getId(), cluster2.getId() };
+
+    {
+        auto transaction{ session.createSharedTransaction() };
+
+        const auto tracks{ Track::find(session, Track::FindParameters{}.setClusters(clusterIds)) };
+        EXPECT_TRUE(tracks.results.empty());
+    }
+
+    {
+        auto transaction{ session.createUniqueTransaction() };
+
+        cluster1.get().modify()->addTrack(track.get());
+    }
+
+    {
+        auto transaction{ session.createSharedTransaction() };
+
+        const auto tracks{ Track::find(session, Track::FindParameters{}.setClusters(clusterIds)) };
+        EXPECT_TRUE(tracks.results.empty());
+    }
+
+    {
+        auto transaction{ session.createUniqueTransaction() };
+
+        cluster2.get().modify()->addTrack(track.get());
+    }
+
+    {
+        auto transaction{ session.createSharedTransaction() };
+
+        const auto tracks{ Track::find(session, Track::FindParameters{}.setClusters(clusterIds)) };
+        ASSERT_FALSE(tracks.results.empty());
+        EXPECT_EQ(tracks.results.front(), track.getId());
+    }
+}
+
 TEST_F(DatabaseFixture, Cluster_multiTracks)
 {
     std::list<ScopedTrack> tracks;