diff --git a/NzbDrone.Api/ErrorManagement/ErrorPipeline.cs b/NzbDrone.Api/ErrorManagement/ErrorPipeline.cs index fe7ba61d6..e5c099fc6 100644 --- a/NzbDrone.Api/ErrorManagement/ErrorPipeline.cs +++ b/NzbDrone.Api/ErrorManagement/ErrorPipeline.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using FluentValidation; using NLog; using Nancy; using NzbDrone.Api.Extensions; @@ -25,6 +26,17 @@ namespace NzbDrone.Api.ErrorManagement return apiException.ToErrorResponse(); } + var validationException = exception as ValidationException; + + if (validationException != null) + { + _logger.Warn("Invalid request {0}", validationException.Message); + + + return validationException.Errors.AsResponse(HttpStatusCode.BadRequest); + + } + _logger.ErrorException("Unexpected error", exception); diff --git a/NzbDrone.Api/NzbDrone.Api.csproj b/NzbDrone.Api/NzbDrone.Api.csproj index 7658d8bd1..44701bd85 100644 --- a/NzbDrone.Api/NzbDrone.Api.csproj +++ b/NzbDrone.Api/NzbDrone.Api.csproj @@ -90,9 +90,11 @@ + + @@ -118,6 +120,7 @@ + diff --git a/NzbDrone.Api/NzbDroneRestModule.cs b/NzbDrone.Api/NzbDroneRestModule.cs new file mode 100644 index 000000000..81a515a5e --- /dev/null +++ b/NzbDrone.Api/NzbDroneRestModule.cs @@ -0,0 +1,16 @@ +using NzbDrone.Api.REST; +using NzbDrone.Api.Validation; + +namespace NzbDrone.Api +{ + public abstract class NzbDroneRestModule : RestModule where TResource : RestResource, new() + { + protected NzbDroneRestModule(string resource) + : base("/api/" + resource.Trim('/')) + { + PostValidator.RuleFor(r => r.Id).IsZero(); + PutValidator.RuleFor(r => r.Id).ValidId(); + } + + } +} \ No newline at end of file diff --git a/NzbDrone.Api/REST/ResourceValidator.cs b/NzbDrone.Api/REST/ResourceValidator.cs new file mode 100644 index 000000000..d9850375b --- /dev/null +++ b/NzbDrone.Api/REST/ResourceValidator.cs @@ -0,0 +1,9 @@ +using FluentValidation; + +namespace NzbDrone.Api.REST +{ + public class ResourceValidator : AbstractValidator + { + + } +} \ No newline at end of file diff --git a/NzbDrone.Api/REST/RestModule.cs b/NzbDrone.Api/REST/RestModule.cs index 6e5d9f7d5..807030972 100644 --- a/NzbDrone.Api/REST/RestModule.cs +++ b/NzbDrone.Api/REST/RestModule.cs @@ -1,13 +1,18 @@ using System; using System.Collections.Generic; +using FluentValidation; using Nancy; using NzbDrone.Api.Extensions; +using System.Linq; namespace NzbDrone.Api.REST { public abstract class RestModule : NancyModule where TResource : RestResource, new() { + protected ResourceValidator PostValidator { get; private set; } + protected ResourceValidator PutValidator { get; private set; } + protected ResourceValidator SharedValidator { get; private set; } private const string ROOT_ROUTE = "/"; private const string ID_ROUTE = "/{id}"; @@ -20,6 +25,11 @@ namespace NzbDrone.Api.REST protected RestModule(string modulePath) : base(modulePath) { + + PostValidator = new ResourceValidator(); + PutValidator = new ResourceValidator(); + SharedValidator = new ResourceValidator(); + Get[ROOT_ROUTE] = options => { EnsureImplementation(GetResourceAll); @@ -55,7 +65,7 @@ namespace NzbDrone.Api.REST return new Response { StatusCode = HttpStatusCode.OK }; }; - + } @@ -78,13 +88,21 @@ namespace NzbDrone.Api.REST { var resource = Request.Body.FromJson(); + var errors = SharedValidator.Validate(resource).Errors.ToList(); + + if (Request.Method.Equals("POST", StringComparison.InvariantCultureIgnoreCase)) { - //resource.ValidateForPost(); + errors.AddRange(PostValidator.Validate(resource).Errors); } else if (Request.Method.Equals("PUT", StringComparison.InvariantCultureIgnoreCase)) { - //resource.ValidateForPut(); + errors.AddRange(PutValidator.Validate(resource).Errors); + } + + if (errors.Any()) + { + throw new ValidationException(errors); } return resource; diff --git a/NzbDrone.Api/Series/SeriesModule.cs b/NzbDrone.Api/Series/SeriesModule.cs index a16fc1000..9b48aa990 100644 --- a/NzbDrone.Api/Series/SeriesModule.cs +++ b/NzbDrone.Api/Series/SeriesModule.cs @@ -4,15 +4,15 @@ using System.Globalization; using System.Linq; using AutoMapper; using FluentValidation; -using Nancy; using NzbDrone.Api.Extensions; using NzbDrone.Common; using NzbDrone.Core.Tv; using NzbDrone.Core.Model; +using NzbDrone.Api.Validation; namespace NzbDrone.Api.Series { - public class SeriesModule : NzbDroneApiModule//: RestModule + public class SeriesModule : NzbDroneRestModule { private readonly ISeriesService _seriesService; @@ -20,15 +20,23 @@ namespace NzbDrone.Api.Series : base("/Series") { _seriesService = seriesService; - Get["/"] = x => AllSeries(); - Get["/{id}"] = x => GetSeries((int)x.id); - Post["/"] = x => AddSeries(); - Put["/"] = x => UpdateSeries(); - Delete["/{id}"] = x => DeleteSeries((int)x.id); + GetResourceAll = AllSeries; + GetResourceById = GetSeries; + CreateResource = AddSeries; + UpdateResource = UpdateSeries; + DeleteResource = DeleteSeries; + + + SharedValidator.RuleFor(s => s.RootFolderId).ValidId(); + SharedValidator.RuleFor(s => s.QualityProfileId).ValidId(); + + + PostValidator.RuleFor(s => s.Title).NotEmpty(); + } - private Response AllSeries() + private List AllSeries() { var series = _seriesService.GetAllSeries().ToList(); var seriesStats = _seriesService.SeriesStatistics(); @@ -45,18 +53,17 @@ namespace NzbDrone.Api.Series s.NextAiring = stats.NextAiring; } - return seriesModels.AsResponse(); + return seriesModels; } - private Response GetSeries(int id) + private SeriesResource GetSeries(int id) { var series = _seriesService.GetSeries(id); var seriesModels = Mapper.Map(series); - - return seriesModels.AsResponse(); + return seriesModels; } - private Response AddSeries() + private SeriesResource AddSeries(SeriesResource seriesResource) { var newSeries = Request.Body.FromJson(); @@ -64,44 +71,40 @@ namespace NzbDrone.Api.Series //Todo: We need to create the folder if the user is adding a new series //(we can just create the folder and it won't blow up if it already exists) //We also need to remove any special characters from the filename before attempting to create it - - _seriesService.AddSeries(newSeries); - - return new Response { StatusCode = HttpStatusCode.Created }; + var series = Mapper.Map(seriesResource); + _seriesService.AddSeries(series); + return Mapper.Map(series); } - private Response UpdateSeries() + private SeriesResource UpdateSeries(SeriesResource seriesResource) { - var request = Request.Body.FromJson(); - - var series = _seriesService.GetSeries(request.Id); + var series = _seriesService.GetSeries(seriesResource.Id); - series.Monitored = request.Monitored; - series.SeasonFolder = request.SeasonFolder; - series.QualityProfileId = request.QualityProfileId; + series.Monitored = seriesResource.Monitored; + series.SeasonFolder = seriesResource.SeasonFolder; + series.QualityProfileId = seriesResource.QualityProfileId; //Todo: Do we want to force a scan when this path changes? Can we use events instead? - series.RootFolderId = request.RootFolderId; - series.FolderName = request.FolderName; + series.RootFolderId = seriesResource.RootFolderId; + series.FolderName = seriesResource.FolderName; - series.BacklogSetting = (BacklogSettingType)request.BacklogSetting; + series.BacklogSetting = (BacklogSettingType)seriesResource.BacklogSetting; - if (!String.IsNullOrWhiteSpace(request.CustomStartDate)) - series.CustomStartDate = DateTime.Parse(request.CustomStartDate, null, DateTimeStyles.RoundtripKind); + if (!String.IsNullOrWhiteSpace(seriesResource.CustomStartDate)) + series.CustomStartDate = DateTime.Parse(seriesResource.CustomStartDate, null, DateTimeStyles.RoundtripKind); else series.CustomStartDate = null; _seriesService.UpdateSeries(series); - return request.AsResponse(); + return Mapper.Map(series); } - private Response DeleteSeries(int id) + private void DeleteSeries(int id) { var deleteFiles = Convert.ToBoolean(Request.Headers["deleteFiles"].FirstOrDefault()); _seriesService.DeleteSeries(id, deleteFiles); - return new Response { StatusCode = HttpStatusCode.OK }; } } diff --git a/NzbDrone.Api/Validation/IdValidationRule.cs b/NzbDrone.Api/Validation/IdValidationRule.cs new file mode 100644 index 000000000..f2ef55a82 --- /dev/null +++ b/NzbDrone.Api/Validation/IdValidationRule.cs @@ -0,0 +1,19 @@ +using FluentValidation; +using FluentValidation.Validators; + +namespace NzbDrone.Api.Validation +{ + public static class RuleBuilderExtensions + { + public static IRuleBuilderOptions ValidId(this IRuleBuilder ruleBuilder) + { + return ruleBuilder.SetValidator(new GreaterThanValidator(0)); + } + + public static IRuleBuilderOptions IsZero(this IRuleBuilder ruleBuilder) + { + return ruleBuilder.SetValidator(new EqualValidator(0)); + } + } + +} \ No newline at end of file diff --git a/NzbDrone.Integration.Test/Client/ClientBase.cs b/NzbDrone.Integration.Test/Client/ClientBase.cs index ae7d15e89..7a61831fb 100644 --- a/NzbDrone.Integration.Test/Client/ClientBase.cs +++ b/NzbDrone.Integration.Test/Client/ClientBase.cs @@ -1,7 +1,10 @@ using System.Collections.Generic; using System.Net; using FluentAssertions; +using FluentValidation; +using FluentValidation.Results; using NLog; +using Newtonsoft.Json; using RestSharp; namespace NzbDrone.Integration.Test.Client @@ -33,6 +36,13 @@ namespace NzbDrone.Integration.Test.Client return Post(request); } + public List InvalidPost(TResource body) + { + var request = BuildRequest(); + request.AddBody(body); + return Post>(request, HttpStatusCode.BadRequest); + } + protected RestRequest BuildRequest(string command = "") { return new RestRequest(_resource + "/" + command.Trim('/')) @@ -58,9 +68,10 @@ namespace NzbDrone.Integration.Test.Client _logger.Info("{0}: {1}", request.Method, _restClient.BuildUri(request)); var response = _restClient.Execute(request); - _logger.Info("Response: {0}", response.Content); + response.StatusCode.Should().Be(statusCode); + if (response.ErrorException != null) { throw response.ErrorException; @@ -68,9 +79,6 @@ namespace NzbDrone.Integration.Test.Client response.ErrorMessage.Should().BeBlank(); - - response.StatusCode.Should().Be(statusCode); - return response.Data; } diff --git a/NzbDrone.Integration.Test/NzbDrone.Integration.Test.csproj b/NzbDrone.Integration.Test/NzbDrone.Integration.Test.csproj index 41cba8aa5..12b705c23 100644 --- a/NzbDrone.Integration.Test/NzbDrone.Integration.Test.csproj +++ b/NzbDrone.Integration.Test/NzbDrone.Integration.Test.csproj @@ -42,12 +42,20 @@ ..\packages\FluentAssertions.2.0.1\lib\net40\FluentAssertions.dll + + False + ..\packages\FluentValidation.3.4.6.0\lib\Net40\FluentValidation.dll + ..\packages\Nancy.0.16.1\lib\net40\Nancy.dll ..\packages\Nancy.Hosting.Self.0.16.1\lib\net40\Nancy.Hosting.Self.dll + + False + ..\packages\Newtonsoft.Json.5.0.3\lib\net40\Newtonsoft.Json.dll + ..\packages\NLog.2.0.1.2\lib\net40\NLog.dll diff --git a/NzbDrone.Integration.Test/SeriesTest.cs b/NzbDrone.Integration.Test/SeriesTest.cs index 5e37be243..fbd938a31 100644 --- a/NzbDrone.Integration.Test/SeriesTest.cs +++ b/NzbDrone.Integration.Test/SeriesTest.cs @@ -1,4 +1,5 @@ -using FluentAssertions; +using System.Net; +using FluentAssertions; using NUnit.Framework; using NzbDrone.Api.Series; @@ -23,10 +24,10 @@ namespace NzbDrone.Integration.Test } [Test] - [Ignore] - public void add_series_without_required_fields_should_return_400() + public void add_series_without_required_fields_should_return_badrequest() { - Series.Post(new SeriesResource()); + var errors = Series.InvalidPost(new SeriesResource()); + errors.Should().NotBeEmpty(); } } diff --git a/NzbDrone.Integration.Test/packages.config b/NzbDrone.Integration.Test/packages.config index 00794e13d..aeb45bf32 100644 --- a/NzbDrone.Integration.Test/packages.config +++ b/NzbDrone.Integration.Test/packages.config @@ -3,8 +3,10 @@ + +