From 4606503818b03c1bbc0b3d9f408ff236b9c2303b Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Wed, 13 May 2020 21:27:39 +0200 Subject: [PATCH] Fixed: Performance issue when scanning large root folder --- src/NzbDrone.Common/Disk/DiskProviderBase.cs | 7 + src/NzbDrone.Common/Disk/IDiskProvider.cs | 1 + .../DiskScanServiceTests/ScanFixture.cs | 139 +++++++++++++++--- .../MediaFiles/DiskScanService.cs | 27 ++-- 4 files changed, 139 insertions(+), 35 deletions(-) diff --git a/src/NzbDrone.Common/Disk/DiskProviderBase.cs b/src/NzbDrone.Common/Disk/DiskProviderBase.cs index 727e788e9..6ac2eb4d8 100644 --- a/src/NzbDrone.Common/Disk/DiskProviderBase.cs +++ b/src/NzbDrone.Common/Disk/DiskProviderBase.cs @@ -142,6 +142,13 @@ namespace NzbDrone.Common.Disk } } + public bool FolderEmpty(string path) + { + Ensure.That(path, () => path).IsValidPath(); + + return Directory.EnumerateDirectories(path).Empty(); + } + public string[] GetDirectories(string path) { Ensure.That(path, () => path).IsValidPath(); diff --git a/src/NzbDrone.Common/Disk/IDiskProvider.cs b/src/NzbDrone.Common/Disk/IDiskProvider.cs index 554079f78..1668e5ba7 100644 --- a/src/NzbDrone.Common/Disk/IDiskProvider.cs +++ b/src/NzbDrone.Common/Disk/IDiskProvider.cs @@ -21,6 +21,7 @@ namespace NzbDrone.Common.Disk bool FileExists(string path); bool FileExists(string path, StringComparison stringComparison); bool FolderWritable(string path); + bool FolderEmpty(string path); string[] GetDirectories(string path); string[] GetFiles(string path, SearchOption searchOption); long GetFolderSize(string path); diff --git a/src/NzbDrone.Core.Test/MediaFiles/DiskScanServiceTests/ScanFixture.cs b/src/NzbDrone.Core.Test/MediaFiles/DiskScanServiceTests/ScanFixture.cs index 795f5587e..f35f7f01d 100644 --- a/src/NzbDrone.Core.Test/MediaFiles/DiskScanServiceTests/ScanFixture.cs +++ b/src/NzbDrone.Core.Test/MediaFiles/DiskScanServiceTests/ScanFixture.cs @@ -5,9 +5,12 @@ using FizzWare.NBuilder; using Moq; using NUnit.Framework; using NzbDrone.Common.Disk; +using NzbDrone.Common.Extensions; +using NzbDrone.Core.Configuration; using NzbDrone.Core.MediaFiles; using NzbDrone.Core.MediaFiles.MovieImport; using NzbDrone.Core.Movies; +using NzbDrone.Core.RootFolders; using NzbDrone.Core.Test.Framework; using NzbDrone.Test.Common; @@ -17,28 +20,58 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests public class ScanFixture : CoreTest { private Movie _movie; + private string _rootFolder; + private string _otherMovieFolder; [SetUp] public void Setup() { + _rootFolder = @"C:\Test\Movies".AsOsAgnostic(); + _otherMovieFolder = @"C:\Test\Movies\OtherMovie".AsOsAgnostic(); + var movieFolder = @"C:\Test\Movies\Movie".AsOsAgnostic(); + _movie = Builder.CreateNew() - .With(s => s.Path = @"C:\Test\Movies\Movie".AsOsAgnostic()) + .With(s => s.Path = movieFolder) .Build(); + Mocker.GetMock() + .Setup(s => s.FolderExists(It.IsAny())) + .Returns(false); + Mocker.GetMock() .Setup(s => s.GetParentFolder(It.IsAny())) .Returns((string path) => Directory.GetParent(path).FullName); + + Mocker.GetMock() + .Setup(s => s.GetBestRootFolderPath(It.IsAny())) + .Returns(_rootFolder); } - private void GivenParentFolderExists() + private void GivenRootFolder(params string[] subfolders) { Mocker.GetMock() - .Setup(s => s.FolderExists(It.IsAny())) + .Setup(s => s.FolderExists(_rootFolder)) .Returns(true); Mocker.GetMock() - .Setup(s => s.GetDirectories(It.IsAny())) - .Returns(new string[] { @"C:\Test\Movies\Movie2".AsOsAgnostic() }); + .Setup(s => s.GetDirectories(_rootFolder)) + .Returns(subfolders); + + Mocker.GetMock() + .Setup(s => s.FolderEmpty(_rootFolder)) + .Returns(subfolders.Empty()); + + foreach (var folder in subfolders) + { + Mocker.GetMock() + .Setup(s => s.FolderExists(folder)) + .Returns(true); + } + } + + private void GivenMovieFolder() + { + GivenRootFolder(_movie.Path); } private void GivenFiles(IEnumerable files) @@ -55,6 +88,12 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests ExceptionVerification.ExpectedWarns(1); + Mocker.GetMock() + .Verify(v => v.GetFiles(_movie.Path, SearchOption.AllDirectories), Times.Never()); + + Mocker.GetMock() + .Verify(v => v.CreateFolder(_movie.Path), Times.Never()); + Mocker.GetMock() .Verify(v => v.Clean(It.IsAny(), It.IsAny>()), Times.Never()); } @@ -62,26 +101,44 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests [Test] public void should_not_scan_if_movie_root_folder_is_empty() { - Mocker.GetMock() - .Setup(s => s.FolderExists(It.IsAny())) - .Returns(true); - - Mocker.GetMock() - .Setup(s => s.GetDirectories(It.IsAny())) - .Returns(new string[0]); + GivenRootFolder(); Subject.Scan(_movie); ExceptionVerification.ExpectedWarns(1); + Mocker.GetMock() + .Verify(v => v.GetFiles(_movie.Path, SearchOption.AllDirectories), Times.Never()); + + Mocker.GetMock() + .Verify(v => v.CreateFolder(_movie.Path), Times.Never()); + Mocker.GetMock() - .Verify(v => v.Clean(It.IsAny(), new List()), Times.Never()); + .Verify(v => v.Clean(It.IsAny(), It.IsAny>()), Times.Never()); + + Mocker.GetMock() + .Verify(v => v.GetImportDecisions(It.IsAny>(), _movie), Times.Never()); + } + + [Test] + public void should_create_if_movie_folder_does_not_exist_but_create_folder_enabled() + { + GivenRootFolder(_otherMovieFolder); + + Mocker.GetMock() + .Setup(s => s.CreateEmptyMovieFolders) + .Returns(true); + + Subject.Scan(_movie); + + Mocker.GetMock() + .Verify(v => v.CreateFolder(_movie.Path), Times.Once()); } [Test] public void should_not_scan_extras_subfolder() { - GivenParentFolderExists(); + GivenMovieFolder(); GivenFiles(new List { @@ -98,10 +155,42 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 1), _movie), Times.Once()); } + [Test] + public void should_not_create_if_movie_folder_does_not_exist_and_create_folder_disabled() + { + GivenRootFolder(_otherMovieFolder); + + Mocker.GetMock() + .Setup(s => s.CreateEmptyMovieFolders) + .Returns(false); + + Subject.Scan(_movie); + + Mocker.GetMock() + .Verify(v => v.CreateFolder(_movie.Path), Times.Never()); + } + + [Test] + public void should_clean_but_not_import_if_movie_folder_does_not_exist() + { + GivenRootFolder(_otherMovieFolder); + + Subject.Scan(_movie); + + Mocker.GetMock() + .Verify(v => v.FolderExists(_movie.Path), Times.Once()); + + Mocker.GetMock() + .Verify(v => v.Clean(It.IsAny(), It.IsAny>()), Times.Once()); + + Mocker.GetMock() + .Verify(v => v.GetImportDecisions(It.IsAny>(), _movie), Times.Never()); + } + [Test] public void should_not_scan_AppleDouble_subfolder() { - GivenParentFolderExists(); + GivenMovieFolder(); GivenFiles(new List { @@ -119,9 +208,10 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests [Test] public void should_scan_extras_movie_and_subfolders() { - GivenParentFolderExists(); _movie.Path = @"C:\Test\Movies\Extras".AsOsAgnostic(); + GivenMovieFolder(); + GivenFiles(new List { Path.Combine(_movie.Path, "Extras", "file1.mkv").AsOsAgnostic(), @@ -141,7 +231,7 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests [Test] public void should_not_scan_subfolders_that_start_with_period() { - GivenParentFolderExists(); + GivenMovieFolder(); GivenFiles(new List { @@ -160,7 +250,7 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests [Test] public void should_not_scan_subfolder_of_season_folder_that_starts_with_a_period() { - GivenParentFolderExists(); + GivenMovieFolder(); GivenFiles(new List { @@ -180,7 +270,7 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests [Test] public void should_not_scan_Synology_eaDir() { - GivenParentFolderExists(); + GivenMovieFolder(); GivenFiles(new List { @@ -197,7 +287,7 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests [Test] public void should_not_scan_thumb_folder() { - GivenParentFolderExists(); + GivenMovieFolder(); GivenFiles(new List { @@ -214,9 +304,10 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests [Test] public void should_scan_dotHack_folder() { - GivenParentFolderExists(); _movie.Path = @"C:\Test\TV\.hack".AsOsAgnostic(); + GivenMovieFolder(); + GivenFiles(new List { Path.Combine(_movie.Path, "Season 1", "file1.mkv").AsOsAgnostic(), @@ -230,9 +321,9 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests } [Test] - public void should_find_files_at_root_of_series_folder() + public void should_find_files_at_root_of_movie_folder() { - GivenParentFolderExists(); + GivenMovieFolder(); GivenFiles(new List { @@ -249,7 +340,7 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests [Test] public void should_exclude_osx_metadata_files() { - GivenParentFolderExists(); + GivenMovieFolder(); GivenFiles(new List { diff --git a/src/NzbDrone.Core/MediaFiles/DiskScanService.cs b/src/NzbDrone.Core/MediaFiles/DiskScanService.cs index 7db9628a4..139986b78 100644 --- a/src/NzbDrone.Core/MediaFiles/DiskScanService.cs +++ b/src/NzbDrone.Core/MediaFiles/DiskScanService.cs @@ -75,23 +75,28 @@ namespace NzbDrone.Core.MediaFiles { var rootFolder = _rootFolderService.GetBestRootFolderPath(movie.Path); - if (!_diskProvider.FolderExists(rootFolder)) - { - _logger.Warn("Movies' root folder ({0}) doesn't exist.", rootFolder); - _eventAggregator.PublishEvent(new MovieScanSkippedEvent(movie, MovieScanSkippedReason.RootFolderDoesNotExist)); - return; - } + var movieFolderExists = _diskProvider.FolderExists(movie.Path); - if (_diskProvider.GetDirectories(rootFolder).Empty()) + if (!movieFolderExists) { - _logger.Warn("Movies' root folder ({0}) is empty.", rootFolder); - _eventAggregator.PublishEvent(new MovieScanSkippedEvent(movie, MovieScanSkippedReason.RootFolderIsEmpty)); - return; + if (!_diskProvider.FolderExists(rootFolder)) + { + _logger.Warn("Movie's root folder ({0}) doesn't exist.", rootFolder); + _eventAggregator.PublishEvent(new MovieScanSkippedEvent(movie, MovieScanSkippedReason.RootFolderDoesNotExist)); + return; + } + + if (_diskProvider.FolderEmpty(rootFolder)) + { + _logger.Warn("Movie's root folder ({0}) is empty.", rootFolder); + _eventAggregator.PublishEvent(new MovieScanSkippedEvent(movie, MovieScanSkippedReason.RootFolderIsEmpty)); + return; + } } _logger.ProgressInfo("Scanning disk for {0}", movie.Title); - if (!_diskProvider.FolderExists(movie.Path)) + if (!movieFolderExists) { if (_configService.CreateEmptyMovieFolders) {