Made some small optims when checking if files are removed

src/libs/database/impl/ArtistInfo.cpp

@@ -124,6 +124,18 @@ namespace lms::db
         });
     }
 
+    void ArtistInfo::findAbsoluteFilePath(Session& session, ArtistInfoId& lastRetrievedId, std::size_t count, const std::function<void(ArtistInfoId artistInfoId, const std::filesystem::path& absoluteFilePath)>& func)
+    {
+        session.checkReadTransaction();
+
+        auto query{ session.getDboSession()->query<std::tuple<ArtistInfoId, std::filesystem::path>>("SELECT a_i.id, a_i.absolute_file_path FROM artist_info a_i").orderBy("a_i.id").where("a_i.id > ?").bind(lastRetrievedId).limit(static_cast<int>(count)) };
+
+        utils::forEachQueryResult(query, [&](const auto& res) {
+            func(std::get<0>(res), std::get<1>(res));
+            lastRetrievedId = std::get<0>(res);
+        });
+    }
+
     Artist::pointer ArtistInfo::getArtist() const
     {
         return _artist;

src/libs/database/impl/Image.cpp

@@ -77,15 +77,15 @@ namespace lms::db
         return utils::fetchQuerySingleResult(session.getDboSession()->query<Wt::Dbo::ptr<Image>>("SELECT i from image i").where("i.absolute_file_path = ?").bind(file));
     }
 
-    void Image::find(Session& session, ImageId& lastRetrievedImage, std::size_t count, const std::function<void(const Image::pointer&)>& func)
+    void Image::find(Session& session, ImageId& lastRetrievedId, std::size_t count, const std::function<void(const Image::pointer&)>& func)
     {
         session.checkReadTransaction();
 
-        auto query{ session.getDboSession()->query<Wt::Dbo::ptr<Image>>("SELECT i from image i").orderBy("i.id").where("i.id > ?").bind(lastRetrievedImage).limit(static_cast<int>(count)) };
+        auto query{ session.getDboSession()->query<Wt::Dbo::ptr<Image>>("SELECT i from image i").orderBy("i.id").where("i.id > ?").bind(lastRetrievedId).limit(static_cast<int>(count)) };
 
         utils::forEachQueryResult(query, [&](const Image::pointer& image) {
             func(image);
-            lastRetrievedImage = image->getId();
+            lastRetrievedId = image->getId();
         });
     }
 
@@ -105,6 +105,18 @@ namespace lms::db
         });
     }
 
+    void Image::findAbsoluteFilePath(Session& session, ImageId& lastRetrievedId, std::size_t count, const std::function<void(ImageId imageId, const std::filesystem::path& absoluteFilePath)>& func)
+    {
+        session.checkReadTransaction();
+
+        auto query{ session.getDboSession()->query<std::tuple<ImageId, std::filesystem::path>>("SELECT i.id,i.absolute_file_path from image i").orderBy("i.id").where("i.id > ?").bind(lastRetrievedId).limit(static_cast<int>(count)) };
+
+        utils::forEachQueryResult(query, [&](const auto& res) {
+            func(std::get<0>(res), std::get<1>(res));
+            lastRetrievedId = std::get<0>(res);
+        });
+    }
+
     void Image::setAbsoluteFilePath(const std::filesystem::path& p)
     {
         assert(p.is_absolute());

src/libs/database/impl/PlayListFile.cpp

@@ -71,6 +71,18 @@ namespace lms::db
         });
     }
 
+    void PlayListFile::findAbsoluteFilePath(Session& session, PlayListFileId& lastRetrievedId, std::size_t count, const std::function<void(PlayListFileId playListFileId, const std::filesystem::path& absoluteFilePath)>& func)
+    {
+        session.checkReadTransaction();
+
+        auto query{ session.getDboSession()->query<std::tuple<PlayListFileId, std::filesystem::path>>("SELECT pl_f.id, pl_f.absolute_file_path FROM playlist_file pl_f").orderBy("pl_f.id").where("pl_f.id > ?").bind(lastRetrievedId).limit(static_cast<int>(count)) };
+
+        utils::forEachQueryResult(query, [&](const auto& res) {
+            func(std::get<0>(res), std::get<1>(res));
+            lastRetrievedId = std::get<0>(res);
+        });
+    }
+
     PlayListFile::pointer PlayListFile::find(Session& session, PlayListFileId id)
     {
         session.checkReadTransaction();

src/libs/database/impl/Session.cpp

@@ -427,4 +427,10 @@ namespace lms::db
         }
         LMS_LOG(DB, DEBUG, "Analyzing " << entry << ": done!");
     }
+
+    void Session::execute(std::string_view query, long long id)
+    {
+        utils::executeCommand(_session, query, id);
+    }
+
 } // namespace lms::db

src/libs/database/impl/Track.cpp

@@ -265,18 +265,30 @@ namespace lms::db
         return utils::fetchQuerySingleResult(session.getDboSession()->query<Wt::Dbo::ptr<Track>>("SELECT t from track t").where("t.id = ?").bind(id));
     }
 
-    void Track::find(Session& session, TrackId& lastRetrievedTrack, std::size_t count, const std::function<void(const Track::pointer&)>& func, MediaLibraryId library)
+    void Track::find(Session& session, TrackId& lastRetrievedId, std::size_t count, const std::function<void(const Track::pointer&)>& func, MediaLibraryId library)
     {
         session.checkReadTransaction();
 
-        auto query{ session.getDboSession()->query<Wt::Dbo::ptr<Track>>("SELECT t from track t").orderBy("t.id").where("t.id > ?").bind(lastRetrievedTrack).limit(static_cast<int>(count)) };
+        auto query{ session.getDboSession()->query<Wt::Dbo::ptr<Track>>("SELECT t from track t").orderBy("t.id").where("t.id > ?").bind(lastRetrievedId).limit(static_cast<int>(count)) };
 
         if (library.isValid())
             query.where("media_library_id = ?").bind(library);
 
         utils::forEachQueryResult(query, [&](const Track::pointer& track) {
             func(track);
-            lastRetrievedTrack = track->getId();
+            lastRetrievedId = track->getId();
+        });
+    }
+
+    void Track::findAbsoluteFilePath(Session& session, TrackId& lastRetrievedId, std::size_t count, const std::function<void(TrackId trackId, const std::filesystem::path& absoluteFilePath)>& func)
+    {
+        session.checkReadTransaction();
+
+        auto query{ session.getDboSession()->query<std::tuple<TrackId, std::filesystem::path>>("SELECT t.id,t.absolute_file_path from track t").orderBy("t.id").where("t.id > ?").bind(lastRetrievedId).limit(static_cast<int>(count)) };
+
+        utils::forEachQueryResult(query, [&](const auto& res) {
+            func(std::get<0>(res), std::get<1>(res));
+            lastRetrievedId = std::get<0>(res);
         });
     }
 

src/libs/database/impl/TrackLyrics.cpp

@@ -126,6 +126,18 @@ namespace lms::db
         return utils::execRangeQuery<TrackLyricsId>(query, range);
     }
 
+    void TrackLyrics::findAbsoluteFilePath(Session& session, TrackLyricsId& lastRetrievedId, std::size_t count, const std::function<void(TrackLyricsId trackLyricsId, const std::filesystem::path& absoluteFilePath)>& func)
+    {
+        session.checkReadTransaction();
+
+        auto query{ session.getDboSession()->query<std::tuple<TrackLyricsId, std::filesystem::path>>("SELECT t_lrc.id,t_lrc.absolute_file_path from track_lyrics t_lrc").orderBy("t_lrc.id").where("t_lrc.id > ?").bind(lastRetrievedId).limit(static_cast<int>(count)) };
+
+        utils::forEachQueryResult(query, [&](const auto& res) {
+            func(std::get<0>(res), std::get<1>(res));
+            lastRetrievedId = std::get<0>(res);
+        });
+    }
+
     TrackLyrics::SynchronizedLines TrackLyrics::getSynchronizedLines() const
     {
         SynchronizedLines synchronizedLines;

src/libs/database/impl/Utils.hpp

@@ -175,10 +175,13 @@ namespace lms::db::utils
     template<typename... Args>
     void executeCommand(Wt::Dbo::Session& session, std::string_view command, const Args&... args)
     {
-        LMS_SCOPED_TRACE_DETAILED_WITH_ARG("Database", "ExecuteCommand", "Command", command);
-
         Wt::Dbo::Call call{ session.execute(std::string{ command }) };
         (call.bind(args), ...);
+
+        {
+            LMS_SCOPED_TRACE_DETAILED_WITH_ARG("Database", "ExecuteCommand", "Command", command);
+            call.run();
+        }
     }
 
     Wt::WDateTime normalizeDateTime(const Wt::WDateTime& dateTime);

src/libs/database/include/database/ArtistInfo.hpp

@@ -53,6 +53,7 @@ namespace lms::db
         static void find(Session& session, ArtistInfoId& lastRetrievedId, std::size_t count, const std::function<void(const pointer&)>& func);
         static void findArtistNameNoLongerMatch(Session& session, std::optional<Range> range, const std::function<void(const pointer&)>& func);
         static void findWithArtistNameAmbiguity(Session& session, std::optional<Range> range, bool allowArtistMBIDFallback, const std::function<void(const pointer&)>& func);
+        static void findAbsoluteFilePath(Session& session, ArtistInfoId& lastRetrievedId, std::size_t count, const std::function<void(ArtistInfoId artistInfoId, const std::filesystem::path& absoluteFilePath)>& func);
 
         // getters
         std::size_t getScanVersion() const { return _scanVersion; }

src/libs/database/include/database/Image.hpp

@@ -69,7 +69,8 @@ namespace lms::db
         static pointer find(Session& session, const std::filesystem::path& file);
         static RangeResults<pointer> find(Session& session, const FindParameters& params);
         static void find(Session& session, const FindParameters& parameters, const std::function<void(const Image::pointer&)>& func);
-        static void find(Session& session, ImageId& lastRetrievedImage, std::size_t count, const std::function<void(const Image::pointer&)>& func);
+        static void find(Session& session, ImageId& lastRetrievedId, std::size_t count, const std::function<void(const Image::pointer&)>& func);
+        static void findAbsoluteFilePath(Session& session, ImageId& lastRetrievedId, std::size_t count, const std::function<void(ImageId imageId, const std::filesystem::path& absoluteFilePath)>& func);
 
         // getters
         const std::filesystem::path& getAbsoluteFilePath() const { return _fileAbsolutePath; }

src/libs/database/include/database/PlayListFile.hpp

@@ -49,6 +49,7 @@ namespace lms::db
         static pointer find(Session& session, PlayListFileId id);
         static pointer find(Session& session, const std::filesystem::path& path);
         static void find(Session& session, PlayListFileId& lastRetrievedId, std::size_t count, const std::function<void(const pointer&)>& func);
+        static void findAbsoluteFilePath(Session& session, PlayListFileId& lastRetrievedId, std::size_t count, const std::function<void(PlayListFileId playListFileId, const std::filesystem::path& absoluteFilePath)>& func);
 
         // getters
         const std::filesystem::path& getAbsoluteFilePath() const { return _absoluteFilePath; }

src/libs/database/include/database/Session.hpp

@@ -22,6 +22,7 @@
 #include <Wt/Dbo/Dbo.h>
 #include <Wt/Dbo/SqlConnectionPool.h>
 
+#include <span>
 #include <string>
 #include <vector>
 
@@ -106,14 +107,8 @@ namespace lms::db
         void refreshTracingLoggerStats();
 
         // returning a ptr here to ease further wrapping using operator->
-        Wt::Dbo::Session* getDboSession()
+        Wt::Dbo::Session* getDboSession() { return &_session; }
-        {
+        Db& getDb() { return _db; }
-            return &_session;
-        }
-        Db& getDb()
-        {
-            return _db;
-        }
 
         template<typename Object, typename... Args>
         typename Object::pointer create(Args&&... args)
@@ -126,7 +121,25 @@ namespace lms::db
             return res;
         }
 
+        template<typename Object>
+        void destroy(typename Object::IdType id)
+        {
+            destroy(std::span{ &id, 1 });
+        }
+
+        template<typename Object>
+        void destroy(std::span<const typename Object::IdType> ids)
+        {
+            checkWriteTransaction();
+
+            const std::string query{ std::string{ "DELETE FROM " } + _session.tableName<Object>() + " WHERE id = ?" };
+            for (typename Object::IdType id : ids)
+                execute(query, id.getValue());
+        }
+
     private:
+        void execute(std::string_view query, long long id);
+
         Db& _db;
         Wt::Dbo::Session _session;
     };

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

@@ -191,7 +191,9 @@ namespace lms::db
         static std::size_t getCount(Session& session);
         static pointer findByPath(Session& session, const std::filesystem::path& p);
         static pointer find(Session& session, TrackId id);
-        static void find(Session& session, TrackId& lastRetrievedTrack, std::size_t count, const std::function<void(const Track::pointer&)>& func, MediaLibraryId library = {});
+        static void find(Session& session, TrackId& lastRetrievedId, std::size_t count, const std::function<void(const Track::pointer&)>& func, MediaLibraryId library = {});
+        static void findAbsoluteFilePath(Session& session, TrackId& lastRetrievedId, std::size_t count, const std::function<void(TrackId trackId, const std::filesystem::path& absoluteFilePath)>& func);
+
         static bool exists(Session& session, TrackId id);
         static std::vector<pointer> findByRecordingMBID(Session& session, const core::UUID& MBID);
         static std::vector<pointer> findByMBID(Session& session, const core::UUID& MBID);

src/libs/database/include/database/TrackLyrics.hpp

@@ -82,6 +82,7 @@ namespace lms::db
         static void find(Session& session, const FindParameters& params, const std::function<void(const TrackLyrics::pointer&)>& func);
         static void find(Session& session, TrackLyricsId& lastRetrievedId, std::size_t count, const std::function<void(const TrackLyrics::pointer&)>& func);
         static RangeResults<TrackLyricsId> findOrphanIds(Session& session, std::optional<Range> range);
+        static void findAbsoluteFilePath(Session& session, TrackLyricsId& lastRetrievedId, std::size_t count, const std::function<void(TrackLyricsId trackLyricsId, const std::filesystem::path& absoluteFilePath)>& func);
 
         using SynchronizedLines = std::map<std::chrono::milliseconds, std::string>;
 

src/libs/database/test/ArtistInfo.cpp

@@ -208,4 +208,29 @@ namespace lms::db::tests
             ASSERT_TRUE(visited);
         }
     }
+
+    TEST_F(DatabaseFixture, ArtistInfo_findAbsoluteFilePath)
+    {
+        ScopedArtistInfo artistInfo{ session };
+
+        const std::filesystem::path absoluteFilePath{ "/path/to/artist.nfo" };
+        {
+            auto transaction{ session.createWriteTransaction() };
+            artistInfo.get().modify()->setAbsoluteFilePath(absoluteFilePath);
+        }
+
+        {
+            auto transaction{ session.createReadTransaction() };
+
+            ArtistInfoId lastRetrievedId;
+
+            std::filesystem::path retrievedFilePath;
+            ArtistInfo::findAbsoluteFilePath(session, lastRetrievedId, 1, [&](ArtistInfoId artistInfoId, const std::filesystem::path& filePath) {
+                EXPECT_EQ(artistInfoId, artistInfo.getId());
+                retrievedFilePath = filePath;
+            });
+
+            EXPECT_EQ(retrievedFilePath, absoluteFilePath);
+        }
+    }
 } // namespace lms::db::tests

src/libs/database/test/Image.cpp

@@ -98,4 +98,28 @@ namespace lms::db::tests
             EXPECT_EQ(results.front()->getId(), image.getId());
         }
     }
+
+    TEST_F(DatabaseFixture, Image_findAbsoluteFilePath)
+    {
+        ScopedImage image{ session, "/path/to/image" };
+
+        const std::filesystem::path absoluteFilePath{ "/path/to/image" };
+        {
+            auto transaction{ session.createWriteTransaction() };
+            image.get().modify()->setAbsoluteFilePath(absoluteFilePath);
+        }
+
+        {
+            auto transaction{ session.createReadTransaction() };
+            ImageId lastRetrievedImageId;
+            std::filesystem::path retrievedPath;
+            Image::findAbsoluteFilePath(session, lastRetrievedImageId, 1, [&](ImageId id, const std::filesystem::path& path) {
+                EXPECT_EQ(id, image.getId());
+                retrievedPath = path;
+            });
+
+            EXPECT_EQ(retrievedPath, absoluteFilePath);
+        }
+    }
+
 } // namespace lms::db::tests

src/libs/database/test/PlayListFile.cpp

@@ -85,6 +85,24 @@ namespace lms::db::tests
         }
     }
 
+    TEST_F(DatabaseFixture, PlayListFile_findAbsoluteFilePath)
+    {
+        ScopedPlayListFile playlist{ session, "/tmp/foo.m3u" };
+
+        {
+            auto transaction{ session.createReadTransaction() };
+
+            PlayListFileId lastRetrievedId;
+            std::filesystem::path retrievedFilePath;
+            PlayListFile::findAbsoluteFilePath(session, lastRetrievedId, 1, [&](PlayListFileId playListFileId, const std::filesystem::path& filePath) {
+                EXPECT_EQ(playListFileId, playlist.getId());
+                retrievedFilePath = filePath;
+            });
+
+            EXPECT_EQ(retrievedFilePath, "/tmp/foo.m3u");
+        }
+    }
+
     TEST_F(DatabaseFixture, PlayListFile_deleteTrackList)
     {
         {

src/libs/database/test/Track.cpp

@@ -151,6 +151,30 @@ namespace lms::db::tests
         }
     }
 
+    TEST_F(DatabaseFixture, Track_findAbsoluteFilePath)
+    {
+        ScopedTrack track{ session };
+        const std::filesystem::path absoluteFilePath{ "/path/to/track.mp3" };
+        {
+            auto transaction{ session.createWriteTransaction() };
+            track.get().modify()->setAbsoluteFilePath(absoluteFilePath);
+        }
+
+        {
+            auto transaction{ session.createReadTransaction() };
+
+            TrackId lastRetrievedTrackId;
+            std::vector<std::pair<TrackId, std::filesystem::path>> visitedTracks;
+            Track::findAbsoluteFilePath(session, lastRetrievedTrackId, 10, [&](TrackId trackId, const std::filesystem::path& filePath) {
+                visitedTracks.emplace_back(trackId, filePath);
+            });
+            ASSERT_EQ(visitedTracks.size(), 1);
+            EXPECT_EQ(visitedTracks[0].first, track.getId());
+            EXPECT_EQ(visitedTracks[0].second, absoluteFilePath);
+            EXPECT_EQ(lastRetrievedTrackId, track.getId());
+        }
+    }
+
     TEST_F(DatabaseFixture, Track_MediaLibrary)
     {
         ScopedTrack track{ session };

src/libs/database/test/TrackLyrics.cpp

@@ -26,6 +26,32 @@ namespace lms::db::tests
 {
     using ScopedTrackLyrics = ScopedEntity<db::TrackLyrics>;
 
+    TEST_F(DatabaseFixture, TrackLyrics_findAbsoluteFilePath)
+    {
+        ScopedTrack track{ session };
+        ScopedTrackLyrics lyrics{ session };
+
+        {
+            auto transaction{ session.createWriteTransaction() };
+
+            TrackLyrics::pointer dbLyrics{ lyrics.get() };
+
+            dbLyrics.modify()->setAbsoluteFilePath("/tmp/test.lrc");
+            dbLyrics.modify()->setTrack(track.get());
+        }
+        {
+            auto transaction{ session.createReadTransaction() };
+
+            TrackLyricsId lastRetrievedId;
+            std::filesystem::path retrievedFilePath;
+            TrackLyrics::findAbsoluteFilePath(session, lastRetrievedId, 1, [&](TrackLyricsId trackLyricsId, const std::filesystem::path& absoluteFilePath) {
+                EXPECT_EQ(trackLyricsId, lyrics.getId());
+                retrievedFilePath = absoluteFilePath;
+            });
+            EXPECT_EQ(retrievedFilePath, "/tmp/test.lrc");
+        }
+    }
+
     TEST_F(DatabaseFixture, TrackLyrics_synchronized)
     {
         using namespace std::chrono_literals;

src/libs/services/scanner/impl/steps/ScanStepCheckForRemovedFiles.cpp

@@ -74,7 +74,7 @@ namespace lms::scanner
 
         Session& session{ _db.getTLSSession() };
 
-        std::vector<typename Object::pointer> objectsToRemove;
+        std::vector<typename Object::IdType> objectIdsToRemove;
 
         typename Object::IdType lastCheckedId;
         bool endReached{};
@@ -83,39 +83,36 @@ namespace lms::scanner
             if (_abortScan)
                 break;
 
-            objectsToRemove.clear();
+            objectIdsToRemove.clear();
             {
-                constexpr std::size_t batchSize = 100;
+                constexpr std::size_t batchSize = 200;
 
                 auto transaction{ session.createReadTransaction() };
 
                 endReached = true;
-                Object::find(session, lastCheckedId, batchSize, [&](const typename Object::pointer& object) {
+                Object::findAbsoluteFilePath(session, lastCheckedId, batchSize, [&](Object::IdType objectId, const std::filesystem::path& filePath) {
                     endReached = false;
 
                     // special case for track lyrics, only check external lyrics
                     if constexpr (std::is_same_v<Object, TrackLyrics>)
                     {
-                        if (object->getAbsoluteFilePath().empty())
+                        if (filePath.empty())
                             return;
                     }
 
-                    if (!checkFile(object->getAbsoluteFilePath()))
+                    if (!checkFile(filePath))
-                        objectsToRemove.push_back(object);
+                        objectIdsToRemove.push_back(objectId);
 
                     context.currentStepStats.processedElems++;
                 });
             }
 
-            if (!objectsToRemove.empty())
+            if (!objectIdsToRemove.empty())
             {
                 auto transaction{ session.createWriteTransaction() };
 
-                for (typename Object::pointer& object : objectsToRemove)
+                session.destroy<Object>(objectIdsToRemove);
-                {
+                context.stats.deletions += objectIdsToRemove.size();
-                    object.remove();
-                    context.stats.deletions++;
-                }
             }
 
             _progressCallback(context.currentStepStats);
@@ -145,7 +142,7 @@ namespace lms::scanner
 
             if (!selectFileScanner(p))
             {
-                LMS_LOG(DBUPDATER, DEBUG, "Removing " << p.string() << ": file format no longer handled");
+                LMS_LOG(DBUPDATER, DEBUG, "Removing " << p << ": file format no longer handled");
                 return false;
             }
 
@@ -153,7 +150,7 @@ namespace lms::scanner
         }
         catch (std::filesystem::filesystem_error& e)
         {
-            LMS_LOG(DBUPDATER, ERROR, "Caught exception while checking file '" << p.string() << "': " << e.what());
+            LMS_LOG(DBUPDATER, ERROR, "Caught exception while checking file " << p << ": " << e.what());
             return false;
         }
     }

src/libs/services/scanner/impl/steps/ScanStepRemoveOrphanedDbEntries.cpp

@@ -109,7 +109,7 @@ namespace lms::scanner
     template<typename T>
     void ScanStepRemoveOrphanedDbEntries::removeOrphanedEntries(ScanContext& context)
     {
-        constexpr std::size_t batchSize = 100;
+        constexpr std::size_t batchSize = 200;
 
         using IdType = typename T::IdType;
 
@@ -130,14 +130,7 @@ namespace lms::scanner
             {
                 auto transaction{ session.createWriteTransaction() };
 
-                for (const IdType objectId : entries.results)
+                session.destroy<T>(entries.results);
-                {
-                    if (_abortScan)
-                        break;
-
-                    typename T::pointer entry{ T::find(session, objectId) };
-                    entry.remove();
-                }
             }
 
             context.currentStepStats.processedElems += entries.results.size();