diff --git a/src/libs/services/scanner/bench/CMakeLists.txt b/src/libs/services/scanner/bench/CMakeLists.txt index 78d75165..d939cb1d 100644 --- a/src/libs/services/scanner/bench/CMakeLists.txt +++ b/src/libs/services/scanner/bench/CMakeLists.txt @@ -1,5 +1,6 @@ add_executable(bench-scanner + IgnoreRules.cpp Lyrics.cpp Scanner.cpp TrackMetadataParser.cpp diff --git a/src/libs/services/scanner/bench/IgnoreRules.cpp b/src/libs/services/scanner/bench/IgnoreRules.cpp new file mode 100644 index 00000000..a5c2ee74 --- /dev/null +++ b/src/libs/services/scanner/bench/IgnoreRules.cpp @@ -0,0 +1,104 @@ +/* + * Copyright (C) 2025 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 . + */ + +#include + +#include + +#include "IgnoreRules.hpp" + +namespace lms::scanner::benchmarks +{ + namespace + { + std::filesystem::path makeDeepPath(std::size_t depth) + { + std::filesystem::path path; + for (std::size_t i{}; i < depth; ++i) + path /= "artist_" + std::to_string(i); + + path /= "track.flac"; + return path; + } + + std::string makeManyRules(std::size_t count) + { + std::string content; + for (std::size_t i{}; i < count; ++i) + content += "*.rule_" + std::to_string(i) + "\n"; + + return content; + } + + void BM_IgnoreRules_isIgnored_ShallowNoMatch(benchmark::State& state) + { + const IgnoreRules rules{ "*.nfo\ncovers/\n" }; + + for (auto _ : state) + benchmark::DoNotOptimize(rules.isIgnored("track.flac", IgnoreRules::IsDirectory{ false })); + } + + void BM_IgnoreRules_isIgnored_ShallowMatch(benchmark::State& state) + { + const IgnoreRules rules{ "*.nfo\ncovers/\n" }; + + for (auto _ : state) + benchmark::DoNotOptimize(rules.isIgnored("track.nfo", IgnoreRules::IsDirectory{ false })); + } + + // Worst case for the ancestor walk: no rule ever matches, so every + // ancestor of a deep path gets fully evaluated on every call. + void BM_IgnoreRules_isIgnored_DeepPath_NoMatch(benchmark::State& state) + { + const IgnoreRules rules{ "*.nfo\ncovers/\n" }; + const std::filesystem::path path{ makeDeepPath(static_cast(state.range(0))) }; + + for (auto _ : state) + benchmark::DoNotOptimize(rules.isIgnored(path, IgnoreRules::IsDirectory{ false })); + } + + // Best case: the leaf itself matches, so the ancestor walk exits on its first iteration. + void BM_IgnoreRules_isIgnored_DeepPath_LeafMatch(benchmark::State& state) + { + const IgnoreRules rules{ "*.flac\n" }; + const std::filesystem::path path{ makeDeepPath(static_cast(state.range(0))) }; + + for (auto _ : state) + benchmark::DoNotOptimize(rules.isIgnored(path, IgnoreRules::IsDirectory{ false })); + } + + // Combines a deep path with a large rule set: every level of the + // ancestor walk pays the full per-rule fnmatch fold. + void BM_IgnoreRules_isIgnored_DeepPath_ManyRules_NoMatch(benchmark::State& state) + { + const IgnoreRules rules{ makeManyRules(static_cast(state.range(1))) }; + const std::filesystem::path path{ makeDeepPath(static_cast(state.range(0))) }; + + for (auto _ : state) + benchmark::DoNotOptimize(rules.isIgnored(path, IgnoreRules::IsDirectory{ false })); + } + } // namespace + + BENCHMARK(BM_IgnoreRules_isIgnored_ShallowNoMatch); + BENCHMARK(BM_IgnoreRules_isIgnored_ShallowMatch); + BENCHMARK(BM_IgnoreRules_isIgnored_DeepPath_NoMatch)->Arg(4)->Arg(8); + BENCHMARK(BM_IgnoreRules_isIgnored_DeepPath_LeafMatch)->Arg(4)->Arg(8); + BENCHMARK(BM_IgnoreRules_isIgnored_DeepPath_ManyRules_NoMatch)->Args({ 4, 10 })->Args({ 8, 20 }); + +} // namespace lms::scanner::benchmarks diff --git a/src/libs/services/scanner/impl/IgnoreRules.cpp b/src/libs/services/scanner/impl/IgnoreRules.cpp index b0d7d9d2..81e347af 100644 --- a/src/libs/services/scanner/impl/IgnoreRules.cpp +++ b/src/libs/services/scanner/impl/IgnoreRules.cpp @@ -21,6 +21,7 @@ #include +#include #include namespace lms::scanner @@ -79,9 +80,28 @@ namespace lms::scanner bool IgnoreRules::isIgnored(const std::filesystem::path& relativePath, IsDirectory isDir) const { + assert(relativePath.is_relative()); + if (_rules.empty()) return false; + std::filesystem::path currentPath{ relativePath }; + IsDirectory currentIsDir{ isDir }; + + while (!currentPath.empty()) + { + if (matchesRules(currentPath, currentIsDir)) + return true; + + currentPath = currentPath.parent_path(); + currentIsDir = IsDirectory{ true }; + } + + return false; + } + + bool IgnoreRules::matchesRules(const std::filesystem::path& relativePath, IsDirectory isDir) const + { bool ignored{}; for (const Rule& rule : _rules) { diff --git a/src/libs/services/scanner/impl/IgnoreRules.hpp b/src/libs/services/scanner/impl/IgnoreRules.hpp index 1cef472a..52699829 100644 --- a/src/libs/services/scanner/impl/IgnoreRules.hpp +++ b/src/libs/services/scanner/impl/IgnoreRules.hpp @@ -56,6 +56,8 @@ namespace lms::scanner bool operator==(const Rule&) const = default; }; + bool matchesRules(const std::filesystem::path& relativePath, IsDirectory isDir) const; + std::vector _rules; public: diff --git a/src/libs/services/scanner/impl/steps/ScanStepCheckForRemovedFiles.cpp b/src/libs/services/scanner/impl/steps/ScanStepCheckForRemovedFiles.cpp index aef28aa0..07c7c100 100644 --- a/src/libs/services/scanner/impl/steps/ScanStepCheckForRemovedFiles.cpp +++ b/src/libs/services/scanner/impl/steps/ScanStepCheckForRemovedFiles.cpp @@ -19,6 +19,7 @@ #include "ScanStepCheckForRemovedFiles.hpp" +#include #include #include #include @@ -107,17 +108,22 @@ namespace lms::scanner return false; } - const auto isInActiveLibrary{ [&](const MediaLibraryInfo& lib) { - if (!core::pathUtils::isPathInRootPath(p, lib.rootDirectory)) - return false; - return lib.ignoreRules.isEmpty() || !lib.ignoreRules.isIgnored(std::filesystem::relative(p, lib.rootDirectory), IgnoreRules::IsDirectory{ false }); - } }; - if (std::none_of(std::cbegin(_settings.mediaLibraries), std::cend(_settings.mediaLibraries), isInActiveLibrary)) + // media library root paths never overlap: at most one library can own this path + const auto itOwningLibrary{ std::find_if(std::cbegin(_settings.mediaLibraries), std::cend(_settings.mediaLibraries), [&](const MediaLibraryInfo& lib) { + return core::pathUtils::isPathInRootPath(p, lib.rootDirectory); + }) }; + if (itOwningLibrary == std::cend(_settings.mediaLibraries)) { LMS_LOG(DBUPDATER, DEBUG, "Removing " << p << ": out of media directory"); return false; } + if (itOwningLibrary->ignoreRules.isIgnored(std::filesystem::relative(p, itOwningLibrary->rootDirectory), IgnoreRules::IsDirectory{ false })) + { + LMS_LOG(DBUPDATER, DEBUG, "Removing " << p << ": ignored by .lmsignore rules"); + return false; + } + if (!_scanners.select(p)) { LMS_LOG(DBUPDATER, DEBUG, "Removing " << p << ": file format no longer handled"); diff --git a/src/libs/services/scanner/test/CMakeLists.txt b/src/libs/services/scanner/test/CMakeLists.txt index b603b4ab..619bcea7 100644 --- a/src/libs/services/scanner/test/CMakeLists.txt +++ b/src/libs/services/scanner/test/CMakeLists.txt @@ -3,7 +3,7 @@ include(GoogleTest) add_executable(test-scanner ArtistInfo.cpp AudioFileUtils.cpp - IgnoreFilter.cpp + IgnoreRules.cpp Lyrics.cpp PlayList.cpp ScannerStats.cpp diff --git a/src/libs/services/scanner/test/IgnoreFilter.cpp b/src/libs/services/scanner/test/IgnoreRules.cpp similarity index 78% rename from src/libs/services/scanner/test/IgnoreFilter.cpp rename to src/libs/services/scanner/test/IgnoreRules.cpp index 920aae65..01155845 100644 --- a/src/libs/services/scanner/test/IgnoreFilter.cpp +++ b/src/libs/services/scanner/test/IgnoreRules.cpp @@ -30,6 +30,13 @@ namespace lms::scanner::tests EXPECT_FALSE(f.isIgnored("track.flac", IgnoreRules::IsDirectory{ false })); } + TEST(IgnoreRules, EmptyPath_NeverIgnored) + { + const IgnoreRules f{ "*.mp3\ncovers/\n" }; + EXPECT_FALSE(f.isIgnored("", IgnoreRules::IsDirectory{ false })); + EXPECT_FALSE(f.isIgnored("", IgnoreRules::IsDirectory{ true })); + } + TEST(IgnoreRules, CommentsAndBlanksOnly) { const IgnoreRules f{ "# this is a comment\n\n# another comment\n" }; @@ -79,7 +86,7 @@ namespace lms::scanner::tests const IgnoreRules f{ "covers*/\n" }; EXPECT_TRUE(f.isIgnored("covers", IgnoreRules::IsDirectory{ true })); EXPECT_TRUE(f.isIgnored("covers_2024", IgnoreRules::IsDirectory{ true })); - EXPECT_TRUE(f.isIgnored("jazz/covers_hq", IgnoreRules::IsDirectory{ true })); // unanchored — matches at any depth + EXPECT_TRUE(f.isIgnored("jazz/covers_hq", IgnoreRules::IsDirectory{ true })); // unanchored: matches at any depth EXPECT_FALSE(f.isIgnored("notcovers", IgnoreRules::IsDirectory{ true })); EXPECT_FALSE(f.isIgnored("covers_2024", IgnoreRules::IsDirectory{ false })); // dirOnly } @@ -112,29 +119,51 @@ namespace lms::scanner::tests TEST(IgnoreRules, DirOnly_AnchoredOnlyMatchesRoot) { const IgnoreRules f{ "/untagged/\n" }; - EXPECT_TRUE(f.isIgnored("untagged", IgnoreRules::IsDirectory{ true })); // root level — match - EXPECT_FALSE(f.isIgnored("jazz/untagged", IgnoreRules::IsDirectory{ true })); // nested — no match - EXPECT_FALSE(f.isIgnored("untagged", IgnoreRules::IsDirectory{ false })); // file, not dir — no match + EXPECT_TRUE(f.isIgnored("untagged", IgnoreRules::IsDirectory{ true })); // root level: match + EXPECT_FALSE(f.isIgnored("jazz/untagged", IgnoreRules::IsDirectory{ true })); // nested: no match + EXPECT_FALSE(f.isIgnored("untagged", IgnoreRules::IsDirectory{ false })); // file, not dir: no match } TEST(IgnoreRules, DirOnly_UnanchoredMatchesAnyDepth) { const IgnoreRules f{ "covers/\n" }; - EXPECT_TRUE(f.isIgnored("covers", IgnoreRules::IsDirectory{ true })); // root — match - EXPECT_TRUE(f.isIgnored("jazz/covers", IgnoreRules::IsDirectory{ true })); // nested — also match - EXPECT_TRUE(f.isIgnored("a/b/c/covers", IgnoreRules::IsDirectory{ true })); // deep — also match - EXPECT_FALSE(f.isIgnored("covers", IgnoreRules::IsDirectory{ false })); // file, not dir — no match + EXPECT_TRUE(f.isIgnored("covers", IgnoreRules::IsDirectory{ true })); // root: match + EXPECT_TRUE(f.isIgnored("jazz/covers", IgnoreRules::IsDirectory{ true })); // nested: also match + EXPECT_TRUE(f.isIgnored("a/b/c/covers", IgnoreRules::IsDirectory{ true })); // deep: also match + EXPECT_FALSE(f.isIgnored("covers", IgnoreRules::IsDirectory{ false })); // file, not dir: no match } TEST(IgnoreRules, FullPath_ExactDir) { const IgnoreRules f{ "jazz/covers\n" }; EXPECT_TRUE(f.isIgnored("jazz/covers", IgnoreRules::IsDirectory{ true })); - EXPECT_FALSE(f.isIgnored("jazz/covers/foo", IgnoreRules::IsDirectory{ true })); // never reached in practice: scanner prunes jazz/covers/ first - EXPECT_FALSE(f.isIgnored("foo/jazz/covers", IgnoreRules::IsDirectory{ true })); // never reached in practice: scanner prunes jazz/covers/ first + EXPECT_TRUE(f.isIgnored("jazz/covers/foo", IgnoreRules::IsDirectory{ true })); // descendant of an ignored dir + EXPECT_FALSE(f.isIgnored("foo/jazz/covers", IgnoreRules::IsDirectory{ true })); // unrelated path structure, no prefix matches EXPECT_FALSE(f.isIgnored("rock/covers", IgnoreRules::IsDirectory{ true })); } + TEST(IgnoreRules, AncestorDirIgnored_MatchesDescendantFile) + { + const IgnoreRules f{ "import\n" }; + EXPECT_TRUE(f.isIgnored("import/song.mp3", IgnoreRules::IsDirectory{ false })); + EXPECT_TRUE(f.isIgnored("import/sub/song.mp3", IgnoreRules::IsDirectory{ false })); + EXPECT_FALSE(f.isIgnored("other/song.mp3", IgnoreRules::IsDirectory{ false })); + } + + TEST(IgnoreRules, AncestorDirIgnored_AnchoredRuleMatchesNestedFile) + { + const IgnoreRules f{ "/import/\n" }; + EXPECT_TRUE(f.isIgnored("import/song.mp3", IgnoreRules::IsDirectory{ false })); + EXPECT_FALSE(f.isIgnored("jazz/import/song.mp3", IgnoreRules::IsDirectory{ false })); // anchored to root only + } + + TEST(IgnoreRules, AncestorDirIgnored_NegationDoesNotReinclude) + { + const IgnoreRules f{ "jazz/\n!jazz/keep.flac\n" }; + EXPECT_TRUE(f.isIgnored("jazz/keep.flac", IgnoreRules::IsDirectory{ false })); // ancestor dir ignored: negation deeper inside has no effect + EXPECT_TRUE(f.isIgnored("jazz", IgnoreRules::IsDirectory{ true })); + } + TEST(IgnoreRules, FullPath_GlobInDir) { const IgnoreRules f{ "jazz/*.nfo\n" };