From f541da23a09ab461ee2b6814c680fdd24242ac7e Mon Sep 17 00:00:00 2001 From: Sarah McQueary Date: Tue, 19 May 2020 09:40:07 -0600 Subject: [PATCH 1/6] Add required on post validator. --- .../RequiredOnPostAttribute.cs | 48 +++++++++++++++++++ .../Server/RequestDeserializer.cs | 21 ++++---- 2 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 src/JsonApiDotNetCore/Models/CustomValidators/RequiredOnPostAttribute.cs diff --git a/src/JsonApiDotNetCore/Models/CustomValidators/RequiredOnPostAttribute.cs b/src/JsonApiDotNetCore/Models/CustomValidators/RequiredOnPostAttribute.cs new file mode 100644 index 0000000000..42288aaf1a --- /dev/null +++ b/src/JsonApiDotNetCore/Models/CustomValidators/RequiredOnPostAttribute.cs @@ -0,0 +1,48 @@ +using Microsoft.AspNetCore.Http; +using System; +using System.ComponentModel.DataAnnotations; + +namespace JsonApiDotNetCore.Models +{ + [AttributeUsage(AttributeTargets.Property)] + public sealed class RequiredOnPostAttribute : ValidationAttribute + { + private string Error { get; set; } + + /// + /// Validates that the value is not null or empty on POST operations. + /// + /// + public RequiredOnPostAttribute(string error = null) + { + Error = error; + } + protected override ValidationResult IsValid(object value, ValidationContext validationContext) + { + var httpContextAccessor = (IHttpContextAccessor)validationContext.GetService(typeof(IHttpContextAccessor)); + var request = httpContextAccessor.HttpContext.Request; + if (request.Method == "POST") + { + if (Error == null) + { + Error = string.Format("{0} is required.", validationContext.MemberName); + } + + if (value == null) + { + return new ValidationResult(Error); + } + + var propertyType = value.GetType(); + if (propertyType.Equals(typeof(System.String))) + { + if (string.IsNullOrEmpty(Convert.ToString(value))) + { + return new ValidationResult(Error); + } + } + } + return ValidationResult.Success; + } + } +} diff --git a/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs b/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs index c17ac03473..d089b904fe 100644 --- a/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs +++ b/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using JsonApiDotNetCore.Exceptions; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Internal.Contracts; @@ -36,16 +37,18 @@ protected override void AfterProcessField(IIdentifiable entity, IResourceField f { if (field is AttrAttribute attr) { - if (attr.Capabilities.HasFlag(AttrCapabilities.AllowMutate)) - { - _targetedFields.Attributes.Add(attr); - } - else - { + if (!attr.Capabilities.HasFlag(AttrCapabilities.AllowMutate)) throw new InvalidRequestBodyException( - "Changing the value of the requested attribute is not allowed.", - $"Changing the value of '{attr.PublicAttributeName}' is not allowed.", null); - } + "Changing the value of the requested attribute is not allowed.", + $"Changing the value of '{attr.PublicAttributeName}' is not allowed.", null); + + var property = attr.PropertyInfo; + var requiredOnPost = property.GetCustomAttributes(typeof(RequiredOnPostAttribute), false); + if (requiredOnPost?.FirstOrDefault() != null && attr.GetValue(entity) == null) + throw new InvalidOperationException($"Attribute {attr.PublicAttributeName} is required and therefore cannot be updated to null."); + + _targetedFields.Attributes.Add(attr); + } else if (field is RelationshipAttribute relationship) _targetedFields.Relationships.Add(relationship); From 6e202c101cc1cb1ab8b36ca0313d2616e3589262 Mon Sep 17 00:00:00 2001 From: Sarah McQueary Date: Wed, 20 May 2020 14:29:07 -0600 Subject: [PATCH 2/6] Add unit test for RequireOnPost validator. --- .../Server/RequestDeserializer.cs | 3 +- .../Server/RequestDeserializerTests.cs | 28 +++++++++++++++++++ test/UnitTests/TestModels.cs | 1 + 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs b/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs index d089b904fe..82a55df1b9 100644 --- a/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs +++ b/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs @@ -45,7 +45,8 @@ protected override void AfterProcessField(IIdentifiable entity, IResourceField f var property = attr.PropertyInfo; var requiredOnPost = property.GetCustomAttributes(typeof(RequiredOnPostAttribute), false); if (requiredOnPost?.FirstOrDefault() != null && attr.GetValue(entity) == null) - throw new InvalidOperationException($"Attribute {attr.PublicAttributeName} is required and therefore cannot be updated to null."); + throw new InvalidRequestBodyException("Changing the value of a required attribute to null is not allowed.", + $"Attribute '{attr.PublicAttributeName}' is required and therefore cannot be updated to null.", null); _targetedFields.Attributes.Add(attr); diff --git a/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs b/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs index 1094f60b37..be024a3be3 100644 --- a/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs +++ b/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs @@ -105,6 +105,34 @@ public void DeserializeRelationships_MultiplePrincipalRelationships_RegistersUpd Assert.Empty(attributesToUpdate); } + + [Fact] + public void DeserializeAttributes_PatchNullRequiredOnPostAttribute_ThrowsInvalidOperationException() + { + // Arrange + SetupFieldsManager(out _, out _); + var content = new Document + { + Data = new ResourceObject + { + Type = "testResource", + Id = "1", + Attributes = new Dictionary + { + { "requiredOnPostField", null }, + } + } + }; + var body = JsonConvert.SerializeObject(content); + + // Act, assert + var exception = Assert.Throws(() => _deserializer.Deserialize(body)); + + Assert.Equal(HttpStatusCode.UnprocessableEntity, exception.Error.StatusCode); + Assert.Equal("Failed to deserialize request body: Changing the value of a required attribute to null is not allowed.", exception.Error.Title); + Assert.Equal("Attribute 'requiredOnPostField' is required and therefore cannot be updated to null.", exception.Error.Detail); + } + private void SetupFieldsManager(out List attributesToUpdate, out List relationshipsToUpdate) { attributesToUpdate = new List(); diff --git a/test/UnitTests/TestModels.cs b/test/UnitTests/TestModels.cs index c05d99df92..8b0d9f062e 100644 --- a/test/UnitTests/TestModels.cs +++ b/test/UnitTests/TestModels.cs @@ -7,6 +7,7 @@ namespace UnitTests.TestModels public sealed class TestResource : Identifiable { [Attr] public string StringField { get; set; } + [Attr] [RequiredOnPost] public string RequiredOnPostField { get; set; } [Attr] public DateTime DateTimeField { get; set; } [Attr] public DateTime? NullableDateTimeField { get; set; } [Attr] public int IntField { get; set; } From b726c00de1a8a9a7d110430abcdc09a30bdd2df3 Mon Sep 17 00:00:00 2001 From: Sarah McQueary Date: Thu, 21 May 2020 19:06:40 -0600 Subject: [PATCH 3/6] Add new property requiredOnPost to tests using test resource. --- .../Serialization/Client/RequestSerializerTests.cs | 5 +++-- .../Serialization/Server/ResponseSerializerTests.cs | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/test/UnitTests/Serialization/Client/RequestSerializerTests.cs b/test/UnitTests/Serialization/Client/RequestSerializerTests.cs index c81f9faff8..46947f1b57 100644 --- a/test/UnitTests/Serialization/Client/RequestSerializerTests.cs +++ b/test/UnitTests/Serialization/Client/RequestSerializerTests.cs @@ -22,7 +22,7 @@ public RequestSerializerTests() public void SerializeSingle_ResourceWithDefaultTargetFields_CanBuild() { // Arrange - var entity = new TestResource { Id = 1, StringField = "value", NullableIntField = 123 }; + var entity = new TestResource { Id = 1, StringField = "value", RequiredOnPostField = "value", NullableIntField = 123 }; // Act string serialized = _serializer.Serialize(entity); @@ -33,8 +33,9 @@ public void SerializeSingle_ResourceWithDefaultTargetFields_CanBuild() ""data"":{ ""type"":""testResource"", ""id"":""1"", - ""attributes"":{ + ""attributes"":{ ""stringField"":""value"", + ""requiredOnPostField"":""value"", ""dateTimeField"":""0001-01-01T00:00:00"", ""nullableDateTimeField"":null, ""intField"":0, diff --git a/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs b/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs index d210ab4ca2..3ca3436872 100644 --- a/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs +++ b/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs @@ -16,7 +16,7 @@ public sealed class ResponseSerializerTests : SerializerTestsSetup public void SerializeSingle_ResourceWithDefaultTargetFields_CanSerialize() { // Arrange - var entity = new TestResource { Id = 1, StringField = "value", NullableIntField = 123 }; + var entity = new TestResource { Id = 1, StringField = "value", RequiredOnPostField = "value", NullableIntField = 123 }; var serializer = GetResponseSerializer(); // Act @@ -30,6 +30,7 @@ public void SerializeSingle_ResourceWithDefaultTargetFields_CanSerialize() ""id"":""1"", ""attributes"":{ ""stringField"":""value"", + ""requiredOnPostField"":""value"", ""dateTimeField"":""0001-01-01T00:00:00"", ""nullableDateTimeField"":null, ""intField"":0, @@ -50,7 +51,7 @@ public void SerializeSingle_ResourceWithDefaultTargetFields_CanSerialize() public void SerializeMany_ResourceWithDefaultTargetFields_CanSerialize() { // Arrange - var entity = new TestResource { Id = 1, StringField = "value", NullableIntField = 123 }; + var entity = new TestResource { Id = 1, StringField = "value", RequiredOnPostField = "value", NullableIntField = 123 }; var serializer = GetResponseSerializer(); // Act @@ -64,6 +65,7 @@ public void SerializeMany_ResourceWithDefaultTargetFields_CanSerialize() ""id"":""1"", ""attributes"":{ ""stringField"":""value"", + ""requiredOnPostField"":""value"", ""dateTimeField"":""0001-01-01T00:00:00"", ""nullableDateTimeField"":null, ""intField"":0, From 2ccc6e9aa765ce4cbdefb46557a2e28b5509700e Mon Sep 17 00:00:00 2001 From: Sarah McQueary Date: Tue, 26 May 2020 12:32:42 -0600 Subject: [PATCH 4/6] Add integration tests for RequiredOnPost Validator. --- .../Models/Article.cs | 1 + .../Acceptance/ManyToManyTests.cs | 4 + .../ResourceDefinitionTests.cs | 251 ++++++++++++++++++ 3 files changed, 256 insertions(+) diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs b/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs index 01b0d1e352..cac9090803 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs @@ -7,6 +7,7 @@ namespace JsonApiDotNetCoreExample.Models public sealed class Article : Identifiable { [Attr] + [RequiredOnPost] public string Name { get; set; } [HasOne] diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs index 94cffa66bb..b41a47501b 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs @@ -294,6 +294,10 @@ public async Task Can_Create_Many_To_Many() data = new { type = "articles", + attributes = new Dictionary + { + {"name", "An article with relationships"} + }, relationships = new Dictionary { { "author", new { diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/ResourceDefinitions/ResourceDefinitionTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/ResourceDefinitions/ResourceDefinitionTests.cs index 41ea2d9d45..ab2e55cb63 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/ResourceDefinitions/ResourceDefinitionTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/ResourceDefinitions/ResourceDefinitionTests.cs @@ -609,5 +609,256 @@ public async Task Cascade_Permission_Error_Delete_ToMany_Relationship() Assert.Equal("You are not allowed to update fields or relationships of locked todo items.", errorDocument.Errors[0].Title); Assert.Null(errorDocument.Errors[0].Detail); } + + [Fact] + public async Task Create_Article_With_RequiredOnPost_Name_Attribute_Succeeds() + { + // Arrange + string name = "Article Title"; + var context = _fixture.GetService(); + var author = new Author(); + context.AuthorDifferentDbContextName.Add(author); + await context.SaveChangesAsync(); + + var route = "/api/v1/articles"; + var request = new HttpRequestMessage(new HttpMethod("POST"), route); + var content = new + { + data = new + { + type = "articles", + attributes = new Dictionary + { + {"name", name} + }, + relationships = new Dictionary + { + { "author", new { + data = new + { + type = "authors", + id = author.StringId + } + } } + } + } + }; + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + request.Content.Headers.ContentType = new MediaTypeHeaderValue(HeaderConstants.MediaType); + + // Act + var response = await _fixture.Client.SendAsync(request); + + // Assert + var body = await response.Content.ReadAsStringAsync(); + Assert.Equal(HttpStatusCode.Created, response.StatusCode); + + var articleResponse = _fixture.GetDeserializer().DeserializeSingle
(body).Data; + Assert.NotNull(articleResponse); + + var persistedArticle = await _fixture.Context.Articles + .SingleAsync(a => a.Id == articleResponse.Id); + + Assert.Equal(name, persistedArticle.Name); + } + + [Fact] + public async Task Create_Article_With_RequiredOnPost_Name_Attribute_Explicitly_Null_Fails() + { + // Arrange + var context = _fixture.GetService(); + var author = new Author(); + context.AuthorDifferentDbContextName.Add(author); + await context.SaveChangesAsync(); + + var route = "/api/v1/articles"; + var request = new HttpRequestMessage(new HttpMethod("POST"), route); + var content = new + { + data = new + { + type = "articles", + attributes = new Dictionary + { + {"name", null} + }, + relationships = new Dictionary + { + { "author", new { + data = new + { + type = "authors", + id = author.StringId + } + } } + } + } + }; + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + request.Content.Headers.ContentType = new MediaTypeHeaderValue(HeaderConstants.MediaType); + + // Act + var response = await _fixture.Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.UnprocessableEntity, response.StatusCode); + } + + [Fact] + public async Task Create_Article_With_RequiredOnPost_Name_Attribute_Missing_Fails() + { + // Arrange + var context = _fixture.GetService(); + var author = new Author(); + context.AuthorDifferentDbContextName.Add(author); + await context.SaveChangesAsync(); + + var route = "/api/v1/articles"; + var request = new HttpRequestMessage(new HttpMethod("POST"), route); + var content = new + { + data = new + { + type = "articles", + relationships = new Dictionary + { + { "author", new { + data = new + { + type = "authors", + id = author.StringId + } + } } + } + } + }; + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + request.Content.Headers.ContentType = new MediaTypeHeaderValue(HeaderConstants.MediaType); + + // Act + var response = await _fixture.Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.UnprocessableEntity, response.StatusCode); + } + + + [Fact] + public async Task Update_Article_With_RequiredOnPost_Name_Attribute_Succeeds() + { + // Arrange + var name = "Article Name"; + var context = _fixture.GetService(); + var article = _articleFaker.Generate(); + context.Articles.Add(article); + await context.SaveChangesAsync(); + + var route = $"/api/v1/articles/{article.Id}"; + var request = new HttpRequestMessage(new HttpMethod("PATCH"), route); + var content = new + { + data = new + { + type = "articles", + id = article.StringId, + attributes = new Dictionary + { + {"name", name} + } + } + }; + + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + request.Content.Headers.ContentType = new MediaTypeHeaderValue(HeaderConstants.MediaType); + + // Act + var response = await _fixture.Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + _fixture.ReloadDbContext(); + var persistedArticle = await _fixture.Context.Articles + .SingleOrDefaultAsync(a => a.Id == article.Id); + + var updatedName = persistedArticle.Name; + Assert.Equal(name, updatedName); + } + + [Fact] + public async Task Update_Article_With_RequiredOnPost_Name_Attribute_Missing_Succeeds() + { + // Arrange + var context = _fixture.GetService(); + var tag = _tagFaker.Generate(); + var article = _articleFaker.Generate(); + context.Tags.Add(tag); + context.Articles.Add(article); + await context.SaveChangesAsync(); + + var route = $"/api/v1/articles/{article.Id}"; + var request = new HttpRequestMessage(new HttpMethod("PATCH"), route); + var content = new + { + data = new + { + type = "articles", + id = article.StringId, + relationships = new Dictionary + { + { "tags", new { + data = new [] { new + { + type = "tags", + id = tag.StringId + } } + } } + } + } + }; + + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + request.Content.Headers.ContentType = new MediaTypeHeaderValue(HeaderConstants.MediaType); + + // Act + var response = await _fixture.Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + [Fact] + public async Task Update_Article_With_RequiredOnPost_Name_Attribute_Explicitly_Null_Fails() + { + // Arrange + var context = _fixture.GetService(); + var article = _articleFaker.Generate(); + context.Articles.Add(article); + await context.SaveChangesAsync(); + + var route = $"/api/v1/articles/{article.Id}"; + var request = new HttpRequestMessage(new HttpMethod("PATCH"), route); + var content = new + { + data = new + { + type = "articles", + id = article.StringId, + attributes = new Dictionary + { + {"name", null} + } + } + }; + + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + request.Content.Headers.ContentType = new MediaTypeHeaderValue(HeaderConstants.MediaType); + + // Act + var response = await _fixture.Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.UnprocessableEntity, response.StatusCode); + } } } From 51afcf205330f295741e812a9f26232e4874e091 Mon Sep 17 00:00:00 2001 From: Sarah McQueary Date: Thu, 4 Jun 2020 17:23:55 -0600 Subject: [PATCH 5/6] Requested changes: -formatting -add validation error to model state -allow empty strings -documentation --- docs/usage/resources/attributes.md | 20 ++ .../Models/Article.cs | 3 +- .../Formatters/JsonApiReader.cs | 1 + .../JsonApiDotNetCore.csproj | 1 + .../RequiredOnPostAttribute.cs | 37 ++-- .../Server/Contracts/IJsonApiDeserializer.cs | 3 + .../Server/RequestDeserializer.cs | 44 ++++- .../ResourceDefinitionTests.cs | 179 +++++++++++++++--- .../Client/RequestSerializerTests.cs | 3 +- .../Server/RequestDeserializerTests.cs | 27 --- .../Server/ResponseSerializerTests.cs | 6 +- test/UnitTests/TestModels.cs | 1 - 12 files changed, 240 insertions(+), 85 deletions(-) diff --git a/docs/usage/resources/attributes.md b/docs/usage/resources/attributes.md index f5344dd32d..f241e9876c 100644 --- a/docs/usage/resources/attributes.md +++ b/docs/usage/resources/attributes.md @@ -112,3 +112,23 @@ public class Foo : Identifiable } } ``` + +# Custom Validators + +Attributes can be marked with custom validators. + +## RequiredOnPost Validator Attribute + +The 'RequiredOnPost' custom validator attribute can be marked on properties to specify if a value is required on POST requests. This allows the property to be excluded on PATCH requests, making partial patching possible. + +The 'RequiredOnPost' custom validator attribute accepts a bool to specify if empty strings are allowed on that property. The default for 'AllowEmptyStrings' is false. + +If a PATCH request contains a property assigned the 'RequiredOnPost'custom validator attribute, the requirements of the validator are verified against the patched value, which include that the value is not null and not empty if 'AllowEmptyStrings' is set to false. + +```c# +public class Person : Identifiable +{ + [RequiredOnPost] + public string FirstName { get; set; } +} +``` diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs b/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs index cac9090803..9df67889fd 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; using System.ComponentModel.DataAnnotations.Schema; using JsonApiDotNetCore.Models; @@ -7,7 +8,7 @@ namespace JsonApiDotNetCoreExample.Models public sealed class Article : Identifiable { [Attr] - [RequiredOnPost] + [RequiredOnPost(true)] public string Name { get; set; } [HasOne] diff --git a/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs b/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs index 3cab898881..4b7804cbb2 100644 --- a/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs +++ b/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs @@ -47,6 +47,7 @@ public async Task ReadAsync(InputFormatterContext context) object model; try { + _deserializer.ModelState = context.ModelState; model = _deserializer.Deserialize(body); } catch (InvalidRequestBodyException exception) diff --git a/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj b/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj index 9a1e0c0104..7786ae80b3 100644 --- a/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj +++ b/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj @@ -24,6 +24,7 @@ + diff --git a/src/JsonApiDotNetCore/Models/CustomValidators/RequiredOnPostAttribute.cs b/src/JsonApiDotNetCore/Models/CustomValidators/RequiredOnPostAttribute.cs index 42288aaf1a..fa03d29005 100644 --- a/src/JsonApiDotNetCore/Models/CustomValidators/RequiredOnPostAttribute.cs +++ b/src/JsonApiDotNetCore/Models/CustomValidators/RequiredOnPostAttribute.cs @@ -1,48 +1,53 @@ using Microsoft.AspNetCore.Http; using System; using System.ComponentModel.DataAnnotations; +using Microsoft.Extensions.DependencyInjection; namespace JsonApiDotNetCore.Models { [AttributeUsage(AttributeTargets.Property)] public sealed class RequiredOnPostAttribute : ValidationAttribute - { - private string Error { get; set; } + { + public bool AllowEmptyStrings { get; set; } /// /// Validates that the value is not null or empty on POST operations. /// - /// - public RequiredOnPostAttribute(string error = null) + /// Allow empty strings + public RequiredOnPostAttribute(bool allowEmptyStrings = false) { - Error = error; + AllowEmptyStrings = allowEmptyStrings; } protected override ValidationResult IsValid(object value, ValidationContext validationContext) { - var httpContextAccessor = (IHttpContextAccessor)validationContext.GetService(typeof(IHttpContextAccessor)); - var request = httpContextAccessor.HttpContext.Request; - if (request.Method == "POST") + var httpContextAccessor = (IHttpContextAccessor)validationContext.GetRequiredService(typeof(IHttpContextAccessor)); + if (httpContextAccessor.HttpContext.Request.Method == "POST") { - if (Error == null) + var additionaError = string.Empty; + if (!AllowEmptyStrings) { - Error = string.Format("{0} is required.", validationContext.MemberName); + additionaError = " or empty"; + } + + if (ErrorMessage == null) + { + ErrorMessage = $"The field {validationContext.MemberName} is required and cannot be null{additionaError}."; } if (value == null) { - return new ValidationResult(Error); + return new ValidationResult(ErrorMessage); } - var propertyType = value.GetType(); - if (propertyType.Equals(typeof(System.String))) + if (!AllowEmptyStrings) { - if (string.IsNullOrEmpty(Convert.ToString(value))) + if (value is string stringValue && string.IsNullOrEmpty(stringValue)) { - return new ValidationResult(Error); + return new ValidationResult(ErrorMessage); } } } - return ValidationResult.Success; + return ValidationResult.Success; } } } diff --git a/src/JsonApiDotNetCore/Serialization/Server/Contracts/IJsonApiDeserializer.cs b/src/JsonApiDotNetCore/Serialization/Server/Contracts/IJsonApiDeserializer.cs index 680391a755..ed02e1f9cb 100644 --- a/src/JsonApiDotNetCore/Serialization/Server/Contracts/IJsonApiDeserializer.cs +++ b/src/JsonApiDotNetCore/Serialization/Server/Contracts/IJsonApiDeserializer.cs @@ -1,4 +1,5 @@ using JsonApiDotNetCore.Models; +using Microsoft.AspNetCore.Mvc.ModelBinding; namespace JsonApiDotNetCore.Serialization.Server { @@ -7,6 +8,8 @@ namespace JsonApiDotNetCore.Serialization.Server /// public interface IJsonApiDeserializer { + public ModelStateDictionary ModelState { get; set; } + /// /// Deserializes JSON in to a and constructs entities /// from . diff --git a/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs b/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs index 82a55df1b9..cec049b049 100644 --- a/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs +++ b/src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs @@ -1,9 +1,9 @@ -using System; -using System.Linq; using JsonApiDotNetCore.Exceptions; using JsonApiDotNetCore.Internal; using JsonApiDotNetCore.Internal.Contracts; using JsonApiDotNetCore.Models; +using System; +using Microsoft.AspNetCore.Mvc.ModelBinding; namespace JsonApiDotNetCore.Serialization.Server { @@ -14,6 +14,8 @@ public class RequestDeserializer : BaseDocumentParser, IJsonApiDeserializer { private readonly ITargetedFields _targetedFields; + public ModelStateDictionary ModelState { get; set; } + public RequestDeserializer(IResourceContextProvider contextProvider, IResourceFactory resourceFactory, ITargetedFields targetedFields) : base(contextProvider, resourceFactory) { @@ -38,15 +40,43 @@ protected override void AfterProcessField(IIdentifiable entity, IResourceField f if (field is AttrAttribute attr) { if (!attr.Capabilities.HasFlag(AttrCapabilities.AllowMutate)) + { throw new InvalidRequestBodyException( "Changing the value of the requested attribute is not allowed.", $"Changing the value of '{attr.PublicAttributeName}' is not allowed.", null); + } + + + var requiredOnPost = Attribute.GetCustomAttribute(attr.PropertyInfo, typeof(RequiredOnPostAttribute)); + if (requiredOnPost != null) + { + var requiredOnPostAttribute = (RequiredOnPostAttribute)requiredOnPost; + var errorMessage = requiredOnPostAttribute.ErrorMessage; + if (errorMessage == null) + { + errorMessage = $"The field {attr.PropertyInfo.Name} is required and cannot be null."; + } + + if (attr.GetValue(entity) == null) + { + if (ModelState != null) + { + ModelState.AddModelError(attr.PropertyInfo.Name, errorMessage); + } + } - var property = attr.PropertyInfo; - var requiredOnPost = property.GetCustomAttributes(typeof(RequiredOnPostAttribute), false); - if (requiredOnPost?.FirstOrDefault() != null && attr.GetValue(entity) == null) - throw new InvalidRequestBodyException("Changing the value of a required attribute to null is not allowed.", - $"Attribute '{attr.PublicAttributeName}' is required and therefore cannot be updated to null.", null); + if (attr.GetValue(entity) is string stringValue && string.IsNullOrEmpty(stringValue)) + { + if (!requiredOnPostAttribute.AllowEmptyStrings) + { + errorMessage = $"The field {attr.PropertyInfo.Name} is required and cannot be null."; + if (ModelState != null) + { + ModelState.AddModelError(attr.PropertyInfo.Name, errorMessage); + } + } + } + } _targetedFields.Attributes.Add(attr); diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/ResourceDefinitions/ResourceDefinitionTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/ResourceDefinitions/ResourceDefinitionTests.cs index ab2e55cb63..1eff7503b4 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/ResourceDefinitions/ResourceDefinitionTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/ResourceDefinitions/ResourceDefinitionTests.cs @@ -82,7 +82,7 @@ public async Task Can_Create_User_With_Password() var serializer = _fixture.GetSerializer(p => new { p.Password, p.Username }); - + var httpMethod = new HttpMethod("POST"); var route = "/api/v1/users"; @@ -633,13 +633,69 @@ public async Task Create_Article_With_RequiredOnPost_Name_Attribute_Succeeds() }, relationships = new Dictionary { - { "author", new { - data = new + { "author", new { - type = "authors", - id = author.StringId - } - } } + data = new + { + type = "authors", + id = author.StringId + } + } + } + } + } + }; + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + request.Content.Headers.ContentType = new MediaTypeHeaderValue(HeaderConstants.MediaType); + + // Act + var response = await _fixture.Client.SendAsync(request); + + // Assert + var body = await response.Content.ReadAsStringAsync(); + Assert.Equal(HttpStatusCode.Created, response.StatusCode); + + var articleResponse = _fixture.GetDeserializer().DeserializeSingle
(body).Data; + Assert.NotNull(articleResponse); + + var persistedArticle = await _fixture.Context.Articles + .SingleAsync(a => a.Id == articleResponse.Id); + + Assert.Equal(name, persistedArticle.Name); + } + + [Fact] + public async Task Create_Article_With_RequiredOnPost_Name_Attribute_Empty_Succeeds() + { + // Arrange + string name = string.Empty; + var context = _fixture.GetService(); + var author = new Author(); + context.AuthorDifferentDbContextName.Add(author); + await context.SaveChangesAsync(); + + var route = "/api/v1/articles"; + var request = new HttpRequestMessage(new HttpMethod("POST"), route); + var content = new + { + data = new + { + type = "articles", + attributes = new Dictionary + { + {"name", name} + }, + relationships = new Dictionary + { + { "author", new + { + data = new + { + type = "authors", + id = author.StringId + } + } + } } } }; @@ -684,13 +740,15 @@ public async Task Create_Article_With_RequiredOnPost_Name_Attribute_Explicitly_N }, relationships = new Dictionary { - { "author", new { - data = new + { "author", new { - type = "authors", - id = author.StringId - } - } } + data = new + { + type = "authors", + id = author.StringId + } + } + } } } }; @@ -701,7 +759,13 @@ public async Task Create_Article_With_RequiredOnPost_Name_Attribute_Explicitly_N var response = await _fixture.Client.SendAsync(request); // Assert - Assert.Equal(HttpStatusCode.UnprocessableEntity, response.StatusCode); + Assert.Equal(HttpStatusCode.UnprocessableEntity, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + var errorDocument = JsonConvert.DeserializeObject(body); + Assert.Single(errorDocument.Errors); + Assert.Equal("Input validation failed.", errorDocument.Errors[0].Title); + Assert.Equal("422", errorDocument.Errors[0].Status); + Assert.Equal("The field Name is required and cannot be null.", errorDocument.Errors[0].Detail); } [Fact] @@ -722,13 +786,15 @@ public async Task Create_Article_With_RequiredOnPost_Name_Attribute_Missing_Fail type = "articles", relationships = new Dictionary { - { "author", new { - data = new + { "author", new { - type = "authors", - id = author.StringId - } - } } + data = new + { + type = "authors", + id = author.StringId + } + } + } } } }; @@ -740,6 +806,12 @@ public async Task Create_Article_With_RequiredOnPost_Name_Attribute_Missing_Fail // Assert Assert.Equal(HttpStatusCode.UnprocessableEntity, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + var errorDocument = JsonConvert.DeserializeObject(body); + Assert.Single(errorDocument.Errors); + Assert.Equal("Input validation failed.", errorDocument.Errors[0].Title); + Assert.Equal("422", errorDocument.Errors[0].Status); + Assert.Equal("The field Name is required and cannot be null.", errorDocument.Errors[0].Detail); } @@ -762,7 +834,7 @@ public async Task Update_Article_With_RequiredOnPost_Name_Attribute_Succeeds() type = "articles", id = article.StringId, attributes = new Dictionary - { + { {"name", name} } } @@ -806,13 +878,18 @@ public async Task Update_Article_With_RequiredOnPost_Name_Attribute_Missing_Succ id = article.StringId, relationships = new Dictionary { - { "tags", new { - data = new [] { new + { "tags", new { - type = "tags", - id = tag.StringId - } } - } } + data = new [] + { + new + { + type = "tags", + id = tag.StringId + } + } + } + } } } }; @@ -859,6 +936,54 @@ public async Task Update_Article_With_RequiredOnPost_Name_Attribute_Explicitly_N // Assert Assert.Equal(HttpStatusCode.UnprocessableEntity, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + var errorDocument = JsonConvert.DeserializeObject(body); + Assert.Single(errorDocument.Errors); + Assert.Equal("Input validation failed.", errorDocument.Errors[0].Title); + Assert.Equal("422", errorDocument.Errors[0].Status); + Assert.Equal("The field Name is required and cannot be null.", errorDocument.Errors[0].Detail); } + + [Fact] + public async Task Update_Article_With_RequiredOnPost_AllowEmptyString_True_Name_Attribute_Empty_Succeeds() + { + // Arrange + var context = _fixture.GetService(); + var article = _articleFaker.Generate(); + context.Articles.Add(article); + await context.SaveChangesAsync(); + + var route = $"/api/v1/articles/{article.Id}"; + var request = new HttpRequestMessage(new HttpMethod("PATCH"), route); + var content = new + { + data = new + { + type = "articles", + id = article.StringId, + attributes = new Dictionary + { + {"name", ""} + } + } + }; + + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + request.Content.Headers.ContentType = new MediaTypeHeaderValue(HeaderConstants.MediaType); + + // Act + var response = await _fixture.Client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + _fixture.ReloadDbContext(); + var persistedArticle = await _fixture.Context.Articles + .SingleOrDefaultAsync(a => a.Id == article.Id); + + var updatedName = persistedArticle.Name; + Assert.Equal("", updatedName); + } + } } diff --git a/test/UnitTests/Serialization/Client/RequestSerializerTests.cs b/test/UnitTests/Serialization/Client/RequestSerializerTests.cs index 46947f1b57..2aee20419e 100644 --- a/test/UnitTests/Serialization/Client/RequestSerializerTests.cs +++ b/test/UnitTests/Serialization/Client/RequestSerializerTests.cs @@ -22,7 +22,7 @@ public RequestSerializerTests() public void SerializeSingle_ResourceWithDefaultTargetFields_CanBuild() { // Arrange - var entity = new TestResource { Id = 1, StringField = "value", RequiredOnPostField = "value", NullableIntField = 123 }; + var entity = new TestResource { Id = 1, StringField = "value", NullableIntField = 123 }; // Act string serialized = _serializer.Serialize(entity); @@ -35,7 +35,6 @@ public void SerializeSingle_ResourceWithDefaultTargetFields_CanBuild() ""id"":""1"", ""attributes"":{ ""stringField"":""value"", - ""requiredOnPostField"":""value"", ""dateTimeField"":""0001-01-01T00:00:00"", ""nullableDateTimeField"":null, ""intField"":0, diff --git a/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs b/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs index be024a3be3..a9a3210dbe 100644 --- a/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs +++ b/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs @@ -106,33 +106,6 @@ public void DeserializeRelationships_MultiplePrincipalRelationships_RegistersUpd } - [Fact] - public void DeserializeAttributes_PatchNullRequiredOnPostAttribute_ThrowsInvalidOperationException() - { - // Arrange - SetupFieldsManager(out _, out _); - var content = new Document - { - Data = new ResourceObject - { - Type = "testResource", - Id = "1", - Attributes = new Dictionary - { - { "requiredOnPostField", null }, - } - } - }; - var body = JsonConvert.SerializeObject(content); - - // Act, assert - var exception = Assert.Throws(() => _deserializer.Deserialize(body)); - - Assert.Equal(HttpStatusCode.UnprocessableEntity, exception.Error.StatusCode); - Assert.Equal("Failed to deserialize request body: Changing the value of a required attribute to null is not allowed.", exception.Error.Title); - Assert.Equal("Attribute 'requiredOnPostField' is required and therefore cannot be updated to null.", exception.Error.Detail); - } - private void SetupFieldsManager(out List attributesToUpdate, out List relationshipsToUpdate) { attributesToUpdate = new List(); diff --git a/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs b/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs index 3ca3436872..d210ab4ca2 100644 --- a/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs +++ b/test/UnitTests/Serialization/Server/ResponseSerializerTests.cs @@ -16,7 +16,7 @@ public sealed class ResponseSerializerTests : SerializerTestsSetup public void SerializeSingle_ResourceWithDefaultTargetFields_CanSerialize() { // Arrange - var entity = new TestResource { Id = 1, StringField = "value", RequiredOnPostField = "value", NullableIntField = 123 }; + var entity = new TestResource { Id = 1, StringField = "value", NullableIntField = 123 }; var serializer = GetResponseSerializer(); // Act @@ -30,7 +30,6 @@ public void SerializeSingle_ResourceWithDefaultTargetFields_CanSerialize() ""id"":""1"", ""attributes"":{ ""stringField"":""value"", - ""requiredOnPostField"":""value"", ""dateTimeField"":""0001-01-01T00:00:00"", ""nullableDateTimeField"":null, ""intField"":0, @@ -51,7 +50,7 @@ public void SerializeSingle_ResourceWithDefaultTargetFields_CanSerialize() public void SerializeMany_ResourceWithDefaultTargetFields_CanSerialize() { // Arrange - var entity = new TestResource { Id = 1, StringField = "value", RequiredOnPostField = "value", NullableIntField = 123 }; + var entity = new TestResource { Id = 1, StringField = "value", NullableIntField = 123 }; var serializer = GetResponseSerializer(); // Act @@ -65,7 +64,6 @@ public void SerializeMany_ResourceWithDefaultTargetFields_CanSerialize() ""id"":""1"", ""attributes"":{ ""stringField"":""value"", - ""requiredOnPostField"":""value"", ""dateTimeField"":""0001-01-01T00:00:00"", ""nullableDateTimeField"":null, ""intField"":0, diff --git a/test/UnitTests/TestModels.cs b/test/UnitTests/TestModels.cs index 8b0d9f062e..c05d99df92 100644 --- a/test/UnitTests/TestModels.cs +++ b/test/UnitTests/TestModels.cs @@ -7,7 +7,6 @@ namespace UnitTests.TestModels public sealed class TestResource : Identifiable { [Attr] public string StringField { get; set; } - [Attr] [RequiredOnPost] public string RequiredOnPostField { get; set; } [Attr] public DateTime DateTimeField { get; set; } [Attr] public DateTime? NullableDateTimeField { get; set; } [Attr] public int IntField { get; set; } From 2ef4912b8987b8a1afc8794b47c6e0960300078f Mon Sep 17 00:00:00 2001 From: Sarah McQueary Date: Thu, 4 Jun 2020 17:28:29 -0600 Subject: [PATCH 6/6] unnecessary sapce --- test/UnitTests/Serialization/Client/RequestSerializerTests.cs | 2 +- test/UnitTests/Serialization/Server/RequestDeserializerTests.cs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/test/UnitTests/Serialization/Client/RequestSerializerTests.cs b/test/UnitTests/Serialization/Client/RequestSerializerTests.cs index 2aee20419e..c81f9faff8 100644 --- a/test/UnitTests/Serialization/Client/RequestSerializerTests.cs +++ b/test/UnitTests/Serialization/Client/RequestSerializerTests.cs @@ -33,7 +33,7 @@ public void SerializeSingle_ResourceWithDefaultTargetFields_CanBuild() ""data"":{ ""type"":""testResource"", ""id"":""1"", - ""attributes"":{ + ""attributes"":{ ""stringField"":""value"", ""dateTimeField"":""0001-01-01T00:00:00"", ""nullableDateTimeField"":null, diff --git a/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs b/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs index a9a3210dbe..1094f60b37 100644 --- a/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs +++ b/test/UnitTests/Serialization/Server/RequestDeserializerTests.cs @@ -105,7 +105,6 @@ public void DeserializeRelationships_MultiplePrincipalRelationships_RegistersUpd Assert.Empty(attributesToUpdate); } - private void SetupFieldsManager(out List attributesToUpdate, out List relationshipsToUpdate) { attributesToUpdate = new List();