From 1dab0aee6a7cbc21951042c382809589124b9398 Mon Sep 17 00:00:00 2001 From: Mark McDowall Date: Mon, 27 Feb 2017 21:37:33 -0800 Subject: [PATCH] Fixed: Reduce parameters required to add a new series Fixes #1403 --- src/NzbDrone.Api/Series/SeasonResource.cs | 2 +- src/NzbDrone.Api/Series/SeriesModule.cs | 6 +- src/NzbDrone.Api/Series/SeriesResource.cs | 14 +- .../NzbDrone.Core.Test.csproj | 2 +- .../TvTests/AddSeriesFixture.cs | 132 ++++++++++++++++++ .../MetadataSource/SkyHook/SkyHookProxy.cs | 4 +- src/NzbDrone.Core/NzbDrone.Core.csproj | 3 + src/NzbDrone.Core/Tv/AddSeriesService.cs | 99 +++++++++++++ src/NzbDrone.Core/Tv/AddSeriesValidator.cs | 30 ++++ src/NzbDrone.Core/Tv/Series.cs | 17 +++ src/NzbDrone.Core/Tv/SeriesService.cs | 14 -- .../Tv/SeriesTitleSlugValidator.cs | 25 ++++ 12 files changed, 317 insertions(+), 31 deletions(-) create mode 100644 src/NzbDrone.Core.Test/TvTests/AddSeriesFixture.cs create mode 100644 src/NzbDrone.Core/Tv/AddSeriesService.cs create mode 100644 src/NzbDrone.Core/Tv/AddSeriesValidator.cs create mode 100644 src/NzbDrone.Core/Tv/SeriesTitleSlugValidator.cs diff --git a/src/NzbDrone.Api/Series/SeasonResource.cs b/src/NzbDrone.Api/Series/SeasonResource.cs index 2231502d9..4c20d8865 100644 --- a/src/NzbDrone.Api/Series/SeasonResource.cs +++ b/src/NzbDrone.Api/Series/SeasonResource.cs @@ -41,7 +41,7 @@ namespace NzbDrone.Api.Series public static List ToModel(this IEnumerable resources) { - return resources.Select(ToModel).ToList(); + return resources?.Select(ToModel).ToList() ?? new List(); } } } diff --git a/src/NzbDrone.Api/Series/SeriesModule.cs b/src/NzbDrone.Api/Series/SeriesModule.cs index 274a57bbd..0b33b9ee3 100644 --- a/src/NzbDrone.Api/Series/SeriesModule.cs +++ b/src/NzbDrone.Api/Series/SeriesModule.cs @@ -29,12 +29,14 @@ namespace NzbDrone.Api.Series { private readonly ISeriesService _seriesService; + private readonly IAddSeriesService _addSeriesService; private readonly ISeriesStatisticsService _seriesStatisticsService; private readonly ISceneMappingService _sceneMappingService; private readonly IMapCoversToLocal _coverMapper; public SeriesModule(IBroadcastSignalRMessage signalRBroadcaster, ISeriesService seriesService, + IAddSeriesService addSeriesService, ISeriesStatisticsService seriesStatisticsService, ISceneMappingService sceneMappingService, IMapCoversToLocal coverMapper, @@ -48,6 +50,7 @@ namespace NzbDrone.Api.Series : base(signalRBroadcaster) { _seriesService = seriesService; + _addSeriesService = addSeriesService; _seriesStatisticsService = seriesStatisticsService; _sceneMappingService = sceneMappingService; @@ -74,7 +77,6 @@ namespace NzbDrone.Api.Series PostValidator.RuleFor(s => s.Path).IsValidPath().When(s => s.RootFolderPath.IsNullOrWhiteSpace()); PostValidator.RuleFor(s => s.RootFolderPath).IsValidPath().When(s => s.Path.IsNullOrWhiteSpace()); - PostValidator.RuleFor(s => s.Title).NotEmpty(); PostValidator.RuleFor(s => s.TvdbId).GreaterThan(0).SetValidator(seriesExistsValidator); PutValidator.RuleFor(s => s.Path).IsValidPath(); @@ -114,7 +116,7 @@ namespace NzbDrone.Api.Series { var model = seriesResource.ToModel(); - return _seriesService.AddSeries(model).Id; + return _addSeriesService.AddSeries(model).Id; } private void UpdateSeries(SeriesResource seriesResource) diff --git a/src/NzbDrone.Api/Series/SeriesResource.cs b/src/NzbDrone.Api/Series/SeriesResource.cs index 176377a86..86de03eb7 100644 --- a/src/NzbDrone.Api/Series/SeriesResource.cs +++ b/src/NzbDrone.Api/Series/SeriesResource.cs @@ -207,19 +207,9 @@ namespace NzbDrone.Api.Series public static Core.Tv.Series ToModel(this SeriesResource resource, Core.Tv.Series series) { - series.TvdbId = resource.TvdbId; + var updatedSeries = resource.ToModel(); - series.Seasons = resource.Seasons.ToModel(); - series.Path = resource.Path; - series.ProfileId = resource.ProfileId; - - series.SeasonFolder = resource.SeasonFolder; - series.Monitored = resource.Monitored; - - series.SeriesType = resource.SeriesType; - series.RootFolderPath = resource.RootFolderPath; - series.Tags = resource.Tags; - series.AddOptions = resource.AddOptions; + series.ApplyChanges(updatedSeries); return series; } diff --git a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj index 96cea7698..3d4033dda 100644 --- a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj +++ b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj @@ -380,7 +380,7 @@ - + diff --git a/src/NzbDrone.Core.Test/TvTests/AddSeriesFixture.cs b/src/NzbDrone.Core.Test/TvTests/AddSeriesFixture.cs new file mode 100644 index 000000000..0014e59e3 --- /dev/null +++ b/src/NzbDrone.Core.Test/TvTests/AddSeriesFixture.cs @@ -0,0 +1,132 @@ +using System; +using System.Collections.Generic; +using System.IO; +using FizzWare.NBuilder; +using FluentAssertions; +using FluentValidation; +using FluentValidation.Results; +using Moq; +using NUnit.Framework; +using NzbDrone.Core.Exceptions; +using NzbDrone.Core.MetadataSource; +using NzbDrone.Core.Organizer; +using NzbDrone.Core.Tv; +using NzbDrone.Core.Test.Framework; +using NzbDrone.Core.Tv.Events; +using NzbDrone.Test.Common; + +namespace NzbDrone.Core.Test.TvTests +{ + [TestFixture] + public class AddSeriesFixture : CoreTest + { + private Series _fakeSeries; + + [SetUp] + public void Setup() + { + _fakeSeries = Builder + .CreateNew() + .With(s => s.Path = null) + .Build(); + } + + private void GivenValidSeries(int tvdbId) + { + Mocker.GetMock() + .Setup(s => s.GetSeriesInfo(tvdbId)) + .Returns(new Tuple>(_fakeSeries, new List())); + } + + private void GivenValidPath() + { + Mocker.GetMock() + .Setup(s => s.GetSeriesFolder(It.IsAny(), null)) + .Returns((c, n) => c.Title); + + Mocker.GetMock() + .Setup(s => s.Validate(It.IsAny())) + .Returns(new ValidationResult()); + } + + [Test] + public void should_be_able_to_add_a_series_without_passing_in_title() + { + var newSeries = new Series + { + TvdbId = 1, + RootFolderPath = @"C:\Test\TV" + }; + + GivenValidSeries(newSeries.TvdbId); + GivenValidPath(); + + var series = Subject.AddSeries(newSeries); + + series.Title.Should().Be(_fakeSeries.Title); + } + + [Test] + public void should_have_proper_path() + { + var newSeries = new Series + { + TvdbId = 1, + RootFolderPath = @"C:\Test\TV" + }; + + GivenValidSeries(newSeries.TvdbId); + GivenValidPath(); + + var series = Subject.AddSeries(newSeries); + + series.Path.Should().Be(Path.Combine(newSeries.RootFolderPath, _fakeSeries.Title)); + } + + [Test] + public void should_throw_if_series_validation_fails() + { + var newSeries = new Series + { + TvdbId = 1, + Path = @"C:\Test\TV\Title1" + }; + + GivenValidSeries(newSeries.TvdbId); + + Mocker.GetMock() + .Setup(s => s.Validate(It.IsAny())) + .Returns(new ValidationResult(new List + { + new ValidationFailure("Path", "Test validation failure") + })); + + Assert.Throws(() => Subject.AddSeries(newSeries)); + } + + [Test] + public void should_throw_if_series_cannot_be_found() + { + var newSeries = new Series + { + TvdbId = 1, + Path = @"C:\Test\TV\Title1" + }; + + Mocker.GetMock() + .Setup(s => s.GetSeriesInfo(newSeries.TvdbId)) + .Throws(new SeriesNotFoundException(newSeries.TvdbId)); + + Mocker.GetMock() + .Setup(s => s.Validate(It.IsAny())) + .Returns(new ValidationResult(new List + { + new ValidationFailure("Path", "Test validation failure") + })); + + Assert.Throws(() => Subject.AddSeries(newSeries)); + + ExceptionVerification.ExpectedErrors(1); + } + } +} \ No newline at end of file diff --git a/src/NzbDrone.Core/MetadataSource/SkyHook/SkyHookProxy.cs b/src/NzbDrone.Core/MetadataSource/SkyHook/SkyHookProxy.cs index 3c1ca6740..5be8e1611 100644 --- a/src/NzbDrone.Core/MetadataSource/SkyHook/SkyHookProxy.cs +++ b/src/NzbDrone.Core/MetadataSource/SkyHook/SkyHookProxy.cs @@ -157,6 +157,7 @@ namespace NzbDrone.Core.MetadataSource.SkyHook series.Actors = show.Actors.Select(MapActors).ToList(); series.Seasons = show.Seasons.Select(MapSeason).ToList(); series.Images = show.Images.Select(MapImage).ToList(); + series.Monitored = true; return series; } @@ -208,7 +209,8 @@ namespace NzbDrone.Core.MetadataSource.SkyHook return new Season { SeasonNumber = seasonResource.SeasonNumber, - Images = seasonResource.Images.Select(MapImage).ToList() + Images = seasonResource.Images.Select(MapImage).ToList(), + Monitored = seasonResource.SeasonNumber > 0 }; } diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index ff43bbca3..2c09a657b 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -1066,6 +1066,8 @@ + + @@ -1099,6 +1101,7 @@ + diff --git a/src/NzbDrone.Core/Tv/AddSeriesService.cs b/src/NzbDrone.Core/Tv/AddSeriesService.cs new file mode 100644 index 000000000..f8a049777 --- /dev/null +++ b/src/NzbDrone.Core/Tv/AddSeriesService.cs @@ -0,0 +1,99 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using FluentValidation; +using FluentValidation.Results; +using NLog; +using NzbDrone.Common.EnsureThat; +using NzbDrone.Core.Exceptions; +using NzbDrone.Core.MetadataSource; +using NzbDrone.Core.Organizer; +using NzbDrone.Core.Parser; + +namespace NzbDrone.Core.Tv +{ + public interface IAddSeriesService + { + Series AddSeries(Series newSeries); + } + + public class AddSeriesService : IAddSeriesService + { + private readonly ISeriesService _seriesService; + private readonly IProvideSeriesInfo _seriesInfo; + private readonly IBuildFileNames _fileNameBuilder; + private readonly IAddSeriesValidator _addSeriesValidator; + private readonly Logger _logger; + + public AddSeriesService(ISeriesService seriesService, + IProvideSeriesInfo seriesInfo, + IBuildFileNames fileNameBuilder, + IAddSeriesValidator addSeriesValidator, + Logger logger) + { + _seriesService = seriesService; + _seriesInfo = seriesInfo; + _fileNameBuilder = fileNameBuilder; + _addSeriesValidator = addSeriesValidator; + _logger = logger; + } + + public Series AddSeries(Series newSeries) + { + Ensure.That(newSeries, () => newSeries).IsNotNull(); + + newSeries = AddSkyhookData(newSeries); + + if (string.IsNullOrWhiteSpace(newSeries.Path)) + { + var folderName = _fileNameBuilder.GetSeriesFolder(newSeries); + newSeries.Path = Path.Combine(newSeries.RootFolderPath, folderName); + } + + newSeries.CleanTitle = newSeries.Title.CleanSeriesTitle(); + newSeries.SortTitle = SeriesTitleNormalizer.Normalize(newSeries.Title, newSeries.TvdbId); + newSeries.Added = DateTime.UtcNow; + + var validationResult = _addSeriesValidator.Validate(newSeries); + + if (!validationResult.IsValid) + { + throw new ValidationException(validationResult.Errors); + } + + _logger.Info("Adding Series {0} Path: [{1}]", newSeries, newSeries.Path); + _seriesService.AddSeries(newSeries); + + return newSeries; + } + + private Series AddSkyhookData(Series newSeries) + { + Tuple> tuple; + + try + { + tuple = _seriesInfo.GetSeriesInfo(newSeries.TvdbId); + } + catch (SeriesNotFoundException) + { + _logger.Error("tvdbid {1} was not found, it may have been removed from TheTVDB.", newSeries.TvdbId); + + throw new ValidationException(new List + { + new ValidationFailure("TvdbId", "A series with this ID was not found", newSeries.TvdbId) + }); + } + + var series = tuple.Item1; + + // If seasons were passed in on the new series use them, otherwise use the seasons from Skyhook + newSeries.Seasons = newSeries.Seasons != null && newSeries.Seasons.Any() ? newSeries.Seasons : series.Seasons; + + series.ApplyChanges(newSeries); + + return series; + } + } +} diff --git a/src/NzbDrone.Core/Tv/AddSeriesValidator.cs b/src/NzbDrone.Core/Tv/AddSeriesValidator.cs new file mode 100644 index 000000000..418815be6 --- /dev/null +++ b/src/NzbDrone.Core/Tv/AddSeriesValidator.cs @@ -0,0 +1,30 @@ +using FluentValidation; +using FluentValidation.Results; +using NzbDrone.Core.Validation.Paths; + +namespace NzbDrone.Core.Tv +{ + public interface IAddSeriesValidator + { + ValidationResult Validate(Series instance); + } + + public class AddSeriesValidator : AbstractValidator, IAddSeriesValidator + { + public AddSeriesValidator(RootFolderValidator rootFolderValidator, + SeriesPathValidator seriesPathValidator, + DroneFactoryValidator droneFactoryValidator, + SeriesAncestorValidator seriesAncestorValidator, + SeriesTitleSlugValidator seriesTitleSlugValidator) + { + RuleFor(c => c.Path).Cascade(CascadeMode.StopOnFirstFailure) + .IsValidPath() + .SetValidator(rootFolderValidator) + .SetValidator(seriesPathValidator) + .SetValidator(droneFactoryValidator) + .SetValidator(seriesAncestorValidator); + + RuleFor(c => c.TitleSlug).SetValidator(seriesTitleSlugValidator); + } + } +} diff --git a/src/NzbDrone.Core/Tv/Series.cs b/src/NzbDrone.Core/Tv/Series.cs index a3fdb986f..8542d183b 100644 --- a/src/NzbDrone.Core/Tv/Series.cs +++ b/src/NzbDrone.Core/Tv/Series.cs @@ -57,5 +57,22 @@ namespace NzbDrone.Core.Tv { return string.Format("[{0}][{1}]", TvdbId, Title.NullSafe()); } + + public void ApplyChanges(Series otherSeries) + { + TvdbId = otherSeries.TvdbId; + + Seasons = otherSeries.Seasons; + Path = otherSeries.Path; + ProfileId = otherSeries.ProfileId; + + SeasonFolder = otherSeries.SeasonFolder; + Monitored = otherSeries.Monitored; + + SeriesType = otherSeries.SeriesType; + RootFolderPath = otherSeries.RootFolderPath; + Tags = otherSeries.Tags; + AddOptions = otherSeries.AddOptions; + } } } \ No newline at end of file diff --git a/src/NzbDrone.Core/Tv/SeriesService.cs b/src/NzbDrone.Core/Tv/SeriesService.cs index 941284407..f4db2b0a4 100644 --- a/src/NzbDrone.Core/Tv/SeriesService.cs +++ b/src/NzbDrone.Core/Tv/SeriesService.cs @@ -67,20 +67,6 @@ namespace NzbDrone.Core.Tv public Series AddSeries(Series newSeries) { - Ensure.That(newSeries, () => newSeries).IsNotNull(); - - if (string.IsNullOrWhiteSpace(newSeries.Path)) - { - var folderName = _fileNameBuilder.GetSeriesFolder(newSeries); - newSeries.Path = Path.Combine(newSeries.RootFolderPath, folderName); - } - - _logger.Info("Adding Series {0} Path: [{1}]", newSeries, newSeries.Path); - - newSeries.CleanTitle = newSeries.Title.CleanSeriesTitle(); - newSeries.SortTitle = SeriesTitleNormalizer.Normalize(newSeries.Title, newSeries.TvdbId); - newSeries.Added = DateTime.UtcNow; - _seriesRepository.Insert(newSeries); _eventAggregator.PublishEvent(new SeriesAddedEvent(GetSeries(newSeries.Id))); diff --git a/src/NzbDrone.Core/Tv/SeriesTitleSlugValidator.cs b/src/NzbDrone.Core/Tv/SeriesTitleSlugValidator.cs new file mode 100644 index 000000000..97ff29095 --- /dev/null +++ b/src/NzbDrone.Core/Tv/SeriesTitleSlugValidator.cs @@ -0,0 +1,25 @@ +using FluentValidation.Validators; + +namespace NzbDrone.Core.Tv +{ + public class SeriesTitleSlugValidator : PropertyValidator + { + private readonly ISeriesService _seriesService; + + public SeriesTitleSlugValidator(ISeriesService seriesService) + : base("Title slug is in use by another series with a similar name") + { + _seriesService = seriesService; + } + + protected override bool IsValid(PropertyValidatorContext context) + { + if (context.PropertyValue == null) return true; + + dynamic instance = context.ParentContext.InstanceToValidate; + var instanceId = (int)instance.Id; + + return !_seriesService.GetAllSeries().Exists(s => s.TitleSlug.Equals(context.PropertyValue.ToString()) && s.Id != instanceId); + } + } +}