From 4c1e6e14aa98b0d192c0ec55b05ac438b07ecd8f Mon Sep 17 00:00:00 2001 From: Mark McDowall Date: Mon, 22 Jul 2013 17:50:37 -0700 Subject: [PATCH] EpisodeFile cleanup and deletion fixes Upgraded episodes will no longer be auto unmonitored EpsiodeFiles will be removed from db if parsing rules have changed EpisodeFiles will be removed from db if they are not in their series' folder (or subfolder) --- NzbDrone.Common/DiskProvider.cs | 22 ++++++ .../NotExistingFileSpecificationFixture.cs | 1 - .../MediaFileTableCleanupServiceFixture.cs | 74 +++++++++++++------ .../UpgradeMediaFileServiceFixture.cs | 2 +- .../HandleEpisodeFileDeletedFixture.cs | 25 +++++-- .../NotExistingFileSpecification.cs | 13 ++-- .../Events/EpisodeFileDeletedEvent.cs | 7 +- NzbDrone.Core/MediaFiles/MediaFileService.cs | 7 +- .../MediaFileTableCleanupService.cs | 39 +++++++++- .../MediaFiles/UpgradeMediaFileService.cs | 2 +- NzbDrone.Core/Tv/EpisodeService.cs | 2 +- 11 files changed, 145 insertions(+), 49 deletions(-) diff --git a/NzbDrone.Common/DiskProvider.cs b/NzbDrone.Common/DiskProvider.cs index 5604da874..ef8b1d33c 100644 --- a/NzbDrone.Common/DiskProvider.cs +++ b/NzbDrone.Common/DiskProvider.cs @@ -35,6 +35,7 @@ namespace NzbDrone.Common bool IsFileLocked(FileInfo file); string GetPathRoot(string path); void SetPermissions(string filename, string account, FileSystemRights Rights, AccessControlType ControlType); + bool IsParent(string parentfolder, string subfolder); } public class DiskProvider : IDiskProvider @@ -383,5 +384,26 @@ namespace NzbDrone.Common directorySecurity.AddAccessRule(accessRule); directoryInfo.SetAccessControl(directorySecurity); } + + public bool IsParent(string parent, string subfolder) + { + parent = parent.TrimEnd(Path.DirectorySeparatorChar); + subfolder = subfolder.TrimEnd(Path.DirectorySeparatorChar); + + var diParent = new DirectoryInfo(parent); + var diSubfolder = new DirectoryInfo(subfolder); + + while (diSubfolder.Parent != null) + { + if (diSubfolder.Parent.FullName == diParent.FullName) + { + return true; + } + + diSubfolder = diSubfolder.Parent; + } + + return false; + } } } \ No newline at end of file diff --git a/NzbDrone.Core.Test/MediaFileTests/EpisodeImportTests/NotExistingFileSpecificationFixture.cs b/NzbDrone.Core.Test/MediaFileTests/EpisodeImportTests/NotExistingFileSpecificationFixture.cs index dcac086ea..d4d9d6177 100644 --- a/NzbDrone.Core.Test/MediaFileTests/EpisodeImportTests/NotExistingFileSpecificationFixture.cs +++ b/NzbDrone.Core.Test/MediaFileTests/EpisodeImportTests/NotExistingFileSpecificationFixture.cs @@ -112,7 +112,6 @@ namespace NzbDrone.Core.Test.MediaFileTests.EpisodeImportTests } [Test] - [Explicit] public void should_return_false_if_exact_path_exists_in_db() { Mocker.GetMock() diff --git a/NzbDrone.Core.Test/MediaFileTests/MediaFileTableCleanupServiceFixture.cs b/NzbDrone.Core.Test/MediaFileTests/MediaFileTableCleanupServiceFixture.cs index 771f67540..e0d022054 100644 --- a/NzbDrone.Core.Test/MediaFileTests/MediaFileTableCleanupServiceFixture.cs +++ b/NzbDrone.Core.Test/MediaFileTests/MediaFileTableCleanupServiceFixture.cs @@ -14,27 +14,47 @@ namespace NzbDrone.Core.Test.MediaFileTests { public class MediaFileTableCleanupServiceFixture : CoreTest { - - private void GiveEpisodeFiles(IEnumerable episodeFiles) - { - Mocker.GetMock() - .Setup(c => c.GetFilesBySeries(It.IsAny())) - .Returns(episodeFiles.ToList()); - } - - private const string DeletedPath = "ANY FILE WITH THIS PATH IS CONSIDERED DELETED!"; [SetUp] public void SetUp() { + Mocker.GetMock() + .Setup(s => s.GetSeries(It.IsAny())) + .Returns(Builder.CreateNew().Build()); + Mocker.GetMock() - .Setup(e => e.FileExists(It.Is(c => c != DeletedPath))) - .Returns(true); + .Setup(e => e.FileExists(It.Is(c => c != DeletedPath))) + .Returns(true); Mocker.GetMock() - .Setup(c => c.GetEpisodesByFileId(It.IsAny())) - .Returns(new List { new Episode() }); + .Setup(c => c.GetEpisodesByFileId(It.IsAny())) + .Returns(new List {new Episode()}); + + Mocker.GetMock() + .Setup(s => s.IsParent(It.IsAny(), It.IsAny())) + .Returns(true); + } + + private void GivenEpisodeFiles(IEnumerable episodeFiles) + { + Mocker.GetMock() + .Setup(c => c.GetFilesBySeries(It.IsAny())) + .Returns(episodeFiles.ToList()); + } + + private void GivenFilesAreNotAttachedToEpisode() + { + Mocker.GetMock() + .Setup(c => c.GetEpisodesByFileId(It.IsAny())) + .Returns(new List()); + } + + private void GivenFileIsNotInSeriesFolder() + { + Mocker.GetMock() + .Setup(s => s.IsParent(It.IsAny(), It.IsAny())) + .Returns(false); } [Test] @@ -43,7 +63,7 @@ namespace NzbDrone.Core.Test.MediaFileTests var episodeFiles = Builder.CreateListOfSize(10) .Build(); - GiveEpisodeFiles(episodeFiles); + GivenEpisodeFiles(episodeFiles); Subject.Execute(new CleanMediaFileDb(0)); @@ -58,12 +78,11 @@ namespace NzbDrone.Core.Test.MediaFileTests .With(c => c.Path = DeletedPath) .Build(); - GiveEpisodeFiles(episodeFiles); + GivenEpisodeFiles(episodeFiles); Subject.Execute(new CleanMediaFileDb(0)); - Mocker.GetMock().Verify(c => c.Delete(It.Is(e => e.Path == DeletedPath)), Times.Exactly(2)); - + Mocker.GetMock().Verify(c => c.Delete(It.Is(e => e.Path == DeletedPath), false), Times.Exactly(2)); } [Test] @@ -74,19 +93,28 @@ namespace NzbDrone.Core.Test.MediaFileTests .With(c => c.Path = "ExistingPath") .Build(); - GiveEpisodeFiles(episodeFiles); + GivenEpisodeFiles(episodeFiles); GivenFilesAreNotAttachedToEpisode(); Subject.Execute(new CleanMediaFileDb(0)); - Mocker.GetMock().Verify(c => c.Delete(It.IsAny()), Times.Exactly(10)); + Mocker.GetMock().Verify(c => c.Delete(It.IsAny(), false), Times.Exactly(10)); } - private void GivenFilesAreNotAttachedToEpisode() + [Test] + public void should_delete_files_that_do_not_belong_to_the_series_path() { - Mocker.GetMock() - .Setup(c => c.GetEpisodesByFileId(It.IsAny())) - .Returns(new List()); + var episodeFiles = Builder.CreateListOfSize(10) + .Random(10) + .With(c => c.Path = "ExistingPath") + .Build(); + + GivenEpisodeFiles(episodeFiles); + GivenFileIsNotInSeriesFolder(); + + Subject.Execute(new CleanMediaFileDb(0)); + + Mocker.GetMock().Verify(c => c.Delete(It.IsAny(), false), Times.Exactly(10)); } } } diff --git a/NzbDrone.Core.Test/MediaFileTests/UpgradeMediaFileServiceFixture.cs b/NzbDrone.Core.Test/MediaFileTests/UpgradeMediaFileServiceFixture.cs index 1b469c361..321d7de35 100644 --- a/NzbDrone.Core.Test/MediaFileTests/UpgradeMediaFileServiceFixture.cs +++ b/NzbDrone.Core.Test/MediaFileTests/UpgradeMediaFileServiceFixture.cs @@ -113,7 +113,7 @@ namespace NzbDrone.Core.Test.MediaFileTests Subject.UpgradeEpisodeFile(_episodeFile, _localEpisode); - Mocker.GetMock().Verify(v => v.Delete(It.IsAny()), Times.Once()); + Mocker.GetMock().Verify(v => v.Delete(It.IsAny(), true), Times.Once()); } } } diff --git a/NzbDrone.Core.Test/TvTests/EpisodeProviderTests/HandleEpisodeFileDeletedFixture.cs b/NzbDrone.Core.Test/TvTests/EpisodeProviderTests/HandleEpisodeFileDeletedFixture.cs index 62c1d6a04..869a01929 100644 --- a/NzbDrone.Core.Test/TvTests/EpisodeProviderTests/HandleEpisodeFileDeletedFixture.cs +++ b/NzbDrone.Core.Test/TvTests/EpisodeProviderTests/HandleEpisodeFileDeletedFixture.cs @@ -58,7 +58,7 @@ namespace NzbDrone.Core.Test.TvTests.EpisodeProviderTests { GivenSingleEpisodeFile(); - Subject.Handle(new EpisodeFileDeletedEvent(_episodeFile)); + Subject.Handle(new EpisodeFileDeletedEvent(_episodeFile, false)); Mocker.GetMock() .Verify(v => v.Update(It.Is(e => e.EpisodeFileId == 0)), Times.Once()); @@ -69,14 +69,14 @@ namespace NzbDrone.Core.Test.TvTests.EpisodeProviderTests { GivenMultiEpisodeFile(); - Subject.Handle(new EpisodeFileDeletedEvent(_episodeFile)); + Subject.Handle(new EpisodeFileDeletedEvent(_episodeFile, false)); Mocker.GetMock() .Verify(v => v.Update(It.Is(e => e.EpisodeFileId == 0)), Times.Exactly(2)); } [Test] - public void should_set_monitored_to_false_if_autoUnmonitor_is_true() + public void should_set_monitored_to_false_if_autoUnmonitor_is_true_and_is_not_for_an_upgrade() { GivenSingleEpisodeFile(); @@ -84,7 +84,7 @@ namespace NzbDrone.Core.Test.TvTests.EpisodeProviderTests .SetupGet(s => s.AutoUnmonitorPreviouslyDownloadedEpisodes) .Returns(true); - Subject.Handle(new EpisodeFileDeletedEvent(_episodeFile)); + Subject.Handle(new EpisodeFileDeletedEvent(_episodeFile, false)); Mocker.GetMock() .Verify(v => v.Update(It.Is(e => e.Monitored == false)), Times.Once()); @@ -99,7 +99,22 @@ namespace NzbDrone.Core.Test.TvTests.EpisodeProviderTests .SetupGet(s => s.AutoUnmonitorPreviouslyDownloadedEpisodes) .Returns(false); - Subject.Handle(new EpisodeFileDeletedEvent(_episodeFile)); + Subject.Handle(new EpisodeFileDeletedEvent(_episodeFile, false)); + + Mocker.GetMock() + .Verify(v => v.Update(It.Is(e => e.Monitored == true)), Times.Once()); + } + + [Test] + public void should_leave_monitored_to_true_if_autoUnmonitor_is_true_and_is_for_an_upgrade() + { + GivenSingleEpisodeFile(); + + Mocker.GetMock() + .SetupGet(s => s.AutoUnmonitorPreviouslyDownloadedEpisodes) + .Returns(true); + + Subject.Handle(new EpisodeFileDeletedEvent(_episodeFile, true)); Mocker.GetMock() .Verify(v => v.Update(It.Is(e => e.Monitored == true)), Times.Once()); diff --git a/NzbDrone.Core/MediaFiles/EpisodeImport/Specifications/NotExistingFileSpecification.cs b/NzbDrone.Core/MediaFiles/EpisodeImport/Specifications/NotExistingFileSpecification.cs index f4c655366..ec335345d 100644 --- a/NzbDrone.Core/MediaFiles/EpisodeImport/Specifications/NotExistingFileSpecification.cs +++ b/NzbDrone.Core/MediaFiles/EpisodeImport/Specifications/NotExistingFileSpecification.cs @@ -20,11 +20,11 @@ namespace NzbDrone.Core.MediaFiles.EpisodeImport.Specifications public bool IsSatisfiedBy(LocalEpisode localEpisode) { -// if (_mediaFileService.Exists(localEpisode.Path)) -// { -// _logger.Trace("File is a match for an existing episode file: {0}", localEpisode.Path); -// return false; -// } + if (_mediaFileService.Exists(localEpisode.Path)) + { + _logger.Trace("File is a match for an existing episode file: {0}", localEpisode.Path); + return false; + } var existingFiles = localEpisode.Episodes.Where(e => e.EpisodeFileId > 0).Select(e => e.EpisodeFile.Value); @@ -36,9 +36,6 @@ namespace NzbDrone.Core.MediaFiles.EpisodeImport.Specifications _logger.Trace("File is a match for an existing episode file: {0}", localEpisode.Path); return false; } - - _logger.Trace("Existing filename: {0} size: {1}", Path.GetFileName(existingFile.Path), existingFile.Size); - _logger.Trace("New filename: {0} size: {1}", Path.GetFileName(localEpisode.Path), localEpisode.Size); } return true; diff --git a/NzbDrone.Core/MediaFiles/Events/EpisodeFileDeletedEvent.cs b/NzbDrone.Core/MediaFiles/Events/EpisodeFileDeletedEvent.cs index fe3a0d6b3..65548c6bb 100644 --- a/NzbDrone.Core/MediaFiles/Events/EpisodeFileDeletedEvent.cs +++ b/NzbDrone.Core/MediaFiles/Events/EpisodeFileDeletedEvent.cs @@ -1,14 +1,17 @@ -using NzbDrone.Common.Messaging; +using System; +using NzbDrone.Common.Messaging; namespace NzbDrone.Core.MediaFiles.Events { public class EpisodeFileDeletedEvent : IEvent { public EpisodeFile EpisodeFile { get; private set; } + public Boolean ForUpgrade { get; private set; } - public EpisodeFileDeletedEvent(EpisodeFile episodeFile) + public EpisodeFileDeletedEvent(EpisodeFile episodeFile, Boolean forUpgrade) { EpisodeFile = episodeFile; + ForUpgrade = forUpgrade; } } } \ No newline at end of file diff --git a/NzbDrone.Core/MediaFiles/MediaFileService.cs b/NzbDrone.Core/MediaFiles/MediaFileService.cs index 66e625357..487afd3b8 100644 --- a/NzbDrone.Core/MediaFiles/MediaFileService.cs +++ b/NzbDrone.Core/MediaFiles/MediaFileService.cs @@ -13,7 +13,7 @@ namespace NzbDrone.Core.MediaFiles { EpisodeFile Add(EpisodeFile episodeFile); void Update(EpisodeFile episodeFile); - void Delete(EpisodeFile episodeFile); + void Delete(EpisodeFile episodeFile, bool forUpgrade = false); bool Exists(string path); EpisodeFile GetFileByPath(string path); List GetFilesBySeries(int seriesId); @@ -46,10 +46,11 @@ namespace NzbDrone.Core.MediaFiles _mediaFileRepository.Update(episodeFile); } - public void Delete(EpisodeFile episodeFile) + public void Delete(EpisodeFile episodeFile, bool forUpgrade = false) { _mediaFileRepository.Delete(episodeFile); - _messageAggregator.PublishEvent(new EpisodeFileDeletedEvent(episodeFile)); + + _messageAggregator.PublishEvent(new EpisodeFileDeletedEvent(episodeFile, forUpgrade)); } public bool Exists(string path) diff --git a/NzbDrone.Core/MediaFiles/MediaFileTableCleanupService.cs b/NzbDrone.Core/MediaFiles/MediaFileTableCleanupService.cs index 949f88679..7b0a1a700 100644 --- a/NzbDrone.Core/MediaFiles/MediaFileTableCleanupService.cs +++ b/NzbDrone.Core/MediaFiles/MediaFileTableCleanupService.cs @@ -4,6 +4,7 @@ using NLog; using NzbDrone.Common; using NzbDrone.Common.Messaging; using NzbDrone.Core.MediaFiles.Commands; +using NzbDrone.Core.Parser; using NzbDrone.Core.Tv; namespace NzbDrone.Core.MediaFiles @@ -14,19 +15,29 @@ namespace NzbDrone.Core.MediaFiles private readonly IMediaFileService _mediaFileService; private readonly IDiskProvider _diskProvider; private readonly IEpisodeService _episodeService; + private readonly ISeriesService _seriesService; + private readonly IParsingService _parsingService; private readonly Logger _logger; - public MediaFileTableCleanupService(IMediaFileService mediaFileService, IDiskProvider diskProvider, IEpisodeService episodeService, Logger logger) + public MediaFileTableCleanupService(IMediaFileService mediaFileService, + IDiskProvider diskProvider, + IEpisodeService episodeService, + ISeriesService seriesService, + IParsingService parsingService, + Logger logger) { _mediaFileService = mediaFileService; _diskProvider = diskProvider; _episodeService = episodeService; + _seriesService = seriesService; + _parsingService = parsingService; _logger = logger; } public void Execute(CleanMediaFileDb message) { var seriesFile = _mediaFileService.GetFilesBySeries(message.SeriesId); + var series = _seriesService.GetSeries(message.SeriesId); foreach (var episodeFile in seriesFile) { @@ -34,14 +45,34 @@ namespace NzbDrone.Core.MediaFiles { if (!_diskProvider.FileExists(episodeFile.Path)) { - _logger.Trace("File [{0}] no longer exists on disk. removing from db", episodeFile.Path); + _logger.Trace("File [{0}] no longer exists on disk, removing from db", episodeFile.Path); _mediaFileService.Delete(episodeFile); + continue; } - if (!_episodeService.GetEpisodesByFileId(episodeFile.Id).Any()) + if (!_diskProvider.IsParent(series.Path, episodeFile.Path)) { - _logger.Trace("File [{0}] is not assigned to any episodes. removing from db", episodeFile.Path); + _logger.Trace("File [{0}] does not belong to this series, removing from db", episodeFile.Path); _mediaFileService.Delete(episodeFile); + continue; + } + + var episodes = _episodeService.GetEpisodesByFileId(episodeFile.Id); + + if (!episodes.Any()) + { + _logger.Trace("File [{0}] is not assigned to any episodes, removing from db", episodeFile.Path); + _mediaFileService.Delete(episodeFile); + continue; + } + + var localEpsiode = _parsingService.GetEpisodes(episodeFile.Path, series); + + if (localEpsiode == null || episodes.Count != localEpsiode.Episodes.Count) + { + _logger.Trace("File [{0}] parsed episodes has changed, removing from db", episodeFile.Path); + _mediaFileService.Delete(episodeFile); + continue; } } catch (Exception ex) diff --git a/NzbDrone.Core/MediaFiles/UpgradeMediaFileService.cs b/NzbDrone.Core/MediaFiles/UpgradeMediaFileService.cs index 4905705ca..b4fcf39df 100644 --- a/NzbDrone.Core/MediaFiles/UpgradeMediaFileService.cs +++ b/NzbDrone.Core/MediaFiles/UpgradeMediaFileService.cs @@ -40,7 +40,7 @@ namespace NzbDrone.Core.MediaFiles _logger.Trace("Removing existing episode file: {0}", file); _recycleBinProvider.DeleteFile(file.Path); - _mediaFileService.Delete(file); + _mediaFileService.Delete(file, true); } _logger.Trace("Moving episode file: {0}", episodeFile); diff --git a/NzbDrone.Core/Tv/EpisodeService.cs b/NzbDrone.Core/Tv/EpisodeService.cs index 2ea42bc50..d7cf318bf 100644 --- a/NzbDrone.Core/Tv/EpisodeService.cs +++ b/NzbDrone.Core/Tv/EpisodeService.cs @@ -183,7 +183,7 @@ namespace NzbDrone.Core.Tv _logger.Trace("Detaching episode {0} from file.", episode.Id); episode.EpisodeFileId = 0; - if (_configService.AutoUnmonitorPreviouslyDownloadedEpisodes) + if (!message.ForUpgrade && _configService.AutoUnmonitorPreviouslyDownloadedEpisodes) { episode.Monitored = false; }