Skip to content

JADNC: Required Input validation disabled for partial patching / relationships #781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c4c27aa
disable validator if partial patch
sasman0001 Jun 9, 2020
ead6930
Create required validator that can be diabled to allow for partial pa…
sasman0001 Jun 9, 2020
a4c688d
formatting
sasman0001 Jun 9, 2020
7d3f57a
Remove unneeded reference.
sasman0001 Jun 9, 2020
11bd7d9
package reference
sasman0001 Jun 9, 2020
c02ef34
Requested changes:
sasman0001 Jun 10, 2020
9e40b88
Add version wildcard
sasman0001 Jun 10, 2020
a0de349
expect possible null in HttpContextExtension method
sasman0001 Jun 10, 2020
86989e6
change parameter order and naming for consistency
sasman0001 Jun 10, 2020
50ae2e4
Requested changes
sasman0001 Jun 10, 2020
102e636
Remove unneeded null check.
sasman0001 Jun 10, 2020
3917539
add null checks
sasman0001 Jun 10, 2020
cfbc9e3
generate fake name for author
sasman0001 Jun 10, 2020
0bcebe3
Moved logic to RequestDeserializer, overriding methods in BaseDocumen…
sasman0001 Jun 10, 2020
6a682ed
remove references
sasman0001 Jun 10, 2020
c2f2a69
formatting
sasman0001 Jun 10, 2020
81e764e
back to var
sasman0001 Jun 10, 2020
126a1a8
Requested changes:
sasman0001 Jun 11, 2020
a1596d5
formatting
sasman0001 Jun 11, 2020
d82f193
formatting
sasman0001 Jun 11, 2020
6882b19
change access modifiers
sasman0001 Jun 11, 2020
96cbd84
Requested changes:
sasman0001 Jun 15, 2020
945691d
refactor
sasman0001 Jun 15, 2020
4496d72
remoev comma.
sasman0001 Jun 15, 2020
4bc23b7
Merge remote-tracking branch 'upstream/master' into input_validation_…
sasman0001 Jun 16, 2020
7eeb6c5
Cleanup
Jun 16, 2020
44a9d92
removed unneeded code
Jun 16, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@
<BogusVersion>29.0.1</BogusVersion>
<MoqVersion>4.13.1</MoqVersion>
</PropertyGroup>
</Project>
</Project>
4 changes: 2 additions & 2 deletions benchmarks/Serialization/JsonApiDeserializerBenchmarks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using JsonApiDotNetCore.Models;
using JsonApiDotNetCore.Serialization;
using JsonApiDotNetCore.Serialization.Server;
using Microsoft.AspNetCore.Http;
using Newtonsoft.Json;

namespace Benchmarks.Serialization
Expand Down Expand Up @@ -38,8 +39,7 @@ public JsonApiDeserializerBenchmarks()
var options = new JsonApiOptions();
IResourceGraph resourceGraph = DependencyFactory.CreateResourceGraph(options);
var targetedFields = new TargetedFields();

_jsonApiDeserializer = new RequestDeserializer(resourceGraph, new DefaultResourceFactory(new ServiceContainer()), targetedFields);
_jsonApiDeserializer = new RequestDeserializer(resourceGraph, new DefaultResourceFactory(new ServiceContainer()), targetedFields, new HttpContextAccessor());
}

[Benchmark]
Expand Down
8 changes: 8 additions & 0 deletions docs/usage/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,12 @@ If you would like to use ASP.NET Core ModelState validation into your controller
```c#
options.ValidateModelState = true;
```
You will need to use the JsonApiDotNetCore 'IsRequiredAttribute' instead of the built-in 'RequiredAttribute' because it contains modifications to enable partial patching.

```c#
public class Person : Identifiable<int>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can inherit from the non-generic Identifiable, which is easier to understand for newcomers (see my example). Also this is a nice opportunity to show usage of the AllowEmptyStrings parameter so users will know it exists. Can you use my example from earlier comments instead?

Never mind, I'll change this.

{
[IsRequired]
public string FirstName { get; set; }
}
```
1 change: 1 addition & 0 deletions src/Examples/JsonApiDotNetCoreExample/Models/Article.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ namespace JsonApiDotNetCoreExample.Models
public sealed class Article : Identifiable
{
[Attr]
[IsRequired(AllowEmptyStrings = true)]
public string Name { get; set; }

[HasOne]
Expand Down
1 change: 1 addition & 0 deletions src/Examples/JsonApiDotNetCoreExample/Models/Author.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace JsonApiDotNetCoreExample.Models
public sealed class Author : Identifiable
{
[Attr]
[IsRequired(AllowEmptyStrings = true)]
public string Name { get; set; }

[HasMany]
Expand Down
12 changes: 12 additions & 0 deletions src/JsonApiDotNetCore/Extensions/HttpContextExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,17 @@ internal static void SetJsonApiRequest(this HttpContext httpContext)
{
httpContext.Items["IsJsonApiRequest"] = bool.TrueString;
}

internal static void DisableValidator(this HttpContext httpContext, string propertyName, string model)
{
var itemKey = $"JsonApiDotNetCore_DisableValidation_{model}_{propertyName}";
httpContext.Items[itemKey] = true;
}

internal static bool IsValidatorDisabled(this HttpContext httpContext, string propertyName, string model)
{
return httpContext.Items.ContainsKey($"JsonApiDotNetCore_DisableValidation_{model}_{propertyName}") ||
httpContext.Items.ContainsKey($"JsonApiDotNetCore_DisableValidation_{model}_Relation");
}
}
}
2 changes: 1 addition & 1 deletion src/JsonApiDotNetCore/JsonApiDotNetCore.csproj
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<VersionPrefix>4.0.0</VersionPrefix>
<TargetFramework>$(NetCoreAppVersion)</TargetFramework>
Expand Down
24 changes: 24 additions & 0 deletions src/JsonApiDotNetCore/Models/IsRequiredAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
using System.ComponentModel.DataAnnotations;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;
using JsonApiDotNetCore.Extensions;

namespace JsonApiDotNetCore.Models
{
public sealed class IsRequiredAttribute : RequiredAttribute
{
private bool _isDisabled;

public override bool IsValid(object value)
{
return _isDisabled || base.IsValid(value);
}

protected override ValidationResult IsValid(object value, ValidationContext validationContext)
{
var httpContextAccessor = (IHttpContextAccessor)validationContext.GetRequiredService(typeof(IHttpContextAccessor));
_isDisabled = httpContextAccessor.HttpContext.IsValidatorDisabled(validationContext.MemberName, validationContext.ObjectType.Name);
return _isDisabled ? ValidationResult.Success : base.IsValid(value, validationContext);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ protected object Deserialize(string body)
/// <param name="attributeValues">Attributes and their values, as in the serialized content</param>
/// <param name="attributes">Exposed attributes for <paramref name="entity"/></param>
/// <returns></returns>
protected IIdentifiable SetAttributes(IIdentifiable entity, Dictionary<string, object> attributeValues, List<AttrAttribute> attributes)
protected virtual IIdentifiable SetAttributes(IIdentifiable entity, Dictionary<string, object> attributeValues, List<AttrAttribute> attributes)
{
if (attributeValues == null || attributeValues.Count == 0)
return entity;
Expand All @@ -86,14 +86,15 @@ protected IIdentifiable SetAttributes(IIdentifiable entity, Dictionary<string, o

return entity;
}

/// <summary>
/// Sets the relationships on a parsed entity
/// </summary>
/// <param name="entity">The parsed entity</param>
/// <param name="relationshipsValues">Relationships and their values, as in the serialized content</param>
/// <param name="relationshipAttributes">Exposed relationships for <paramref name="entity"/></param>
/// <returns></returns>
protected IIdentifiable SetRelationships(IIdentifiable entity, Dictionary<string, RelationshipEntry> relationshipsValues, List<RelationshipAttribute> relationshipAttributes)
protected virtual IIdentifiable SetRelationships(IIdentifiable entity, Dictionary<string, RelationshipEntry> relationshipsValues, List<RelationshipAttribute> relationshipAttributes)
{
if (relationshipsValues == null || relationshipsValues.Count == 0)
return entity;
Expand All @@ -108,7 +109,6 @@ protected IIdentifiable SetRelationships(IIdentifiable entity, Dictionary<string
SetHasOneRelationship(entity, entityProperties, hasOneAttribute, relationshipData);
else
SetHasManyRelationship(entity, (HasManyAttribute)attr, relationshipData);

}
return entity;
}
Expand Down
45 changes: 43 additions & 2 deletions src/JsonApiDotNetCore/Serialization/Server/RequestDeserializer.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
using System;
using JsonApiDotNetCore.Exceptions;
using JsonApiDotNetCore.Internal;
using JsonApiDotNetCore.Internal.Contracts;
using JsonApiDotNetCore.Models;
using Microsoft.AspNetCore.Http;
using System.Collections.Generic;
using System.Reflection;
using JsonApiDotNetCore.Extensions;
using System.Net.Http;

namespace JsonApiDotNetCore.Serialization.Server
{
Expand All @@ -12,11 +16,13 @@ namespace JsonApiDotNetCore.Serialization.Server
public class RequestDeserializer : BaseDocumentParser, IJsonApiDeserializer
{
private readonly ITargetedFields _targetedFields;
private readonly IHttpContextAccessor _httpContextAccessor;

public RequestDeserializer(IResourceContextProvider contextProvider, IResourceFactory resourceFactory, ITargetedFields targetedFields)
public RequestDeserializer(IResourceContextProvider contextProvider, IResourceFactory resourceFactory, ITargetedFields targetedFields, IHttpContextAccessor httpContextAccessor)
: base(contextProvider, resourceFactory)
{
_targetedFields = targetedFields;
_httpContextAccessor = httpContextAccessor;
}

/// <inheritdoc/>
Expand Down Expand Up @@ -50,5 +56,40 @@ protected override void AfterProcessField(IIdentifiable entity, IResourceField f
else if (field is RelationshipAttribute relationship)
_targetedFields.Relationships.Add(relationship);
}

protected override IIdentifiable SetAttributes(IIdentifiable entity, Dictionary<string, object> attributeValues, List<AttrAttribute> attributes)
{
if (_httpContextAccessor.HttpContext.Request.Method == HttpMethod.Patch.Method)
{
foreach (AttrAttribute attr in attributes)
{
if (attr.PropertyInfo.GetCustomAttribute<IsRequiredAttribute>() != null)
{
bool disableValidator = attributeValues == null || attributeValues.Count == 0 ||
!attributeValues.TryGetValue(attr.PublicAttributeName, out _);

if (disableValidator)
{
_httpContextAccessor.HttpContext.DisableValidator(attr.PropertyInfo.Name, entity.GetType().Name);
}
}
}
}

return base.SetAttributes(entity, attributeValues, attributes);
}

protected override IIdentifiable SetRelationships(IIdentifiable entity, Dictionary<string, RelationshipEntry> relationshipsValues, List<RelationshipAttribute> relationshipAttributes)
{
// If there is a relationship included in the data of the POST or PATCH, then the 'IsRequired' attribute will be disabled for any
// property within that object. For instance, a new article is posted and has a relationship included to an author. In this case,
// the author name (which has the 'IsRequired' attribute) will not be included in the POST. Unless disabled, the POST will fail.
foreach (RelationshipAttribute attr in relationshipAttributes)
{
_httpContextAccessor.HttpContext.DisableValidator("Relation", attr.PropertyInfo.Name);
}

return base.SetRelationships(entity, relationshipsValues, relationshipAttributes);
}
}
}
21 changes: 15 additions & 6 deletions test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,23 @@ namespace JsonApiDotNetCoreExampleTests.Acceptance
[Collection("WebHostCollection")]
public sealed class ManyToManyTests
{
private readonly Faker<Article> _articleFaker = new Faker<Article>()
.RuleFor(a => a.Name, f => f.Random.AlphaNumeric(10))
.RuleFor(a => a.Author, f => new Author());
private readonly TestFixture<TestStartup> _fixture;

private readonly Faker<Author> _authorFaker;
private readonly Faker<Article> _articleFaker;
private readonly Faker<Tag> _tagFaker;

private readonly TestFixture<TestStartup> _fixture;

public ManyToManyTests(TestFixture<TestStartup> fixture)
{
_fixture = fixture;

_authorFaker = new Faker<Author>()
.RuleFor(a => a.Name, f => f.Random.Words(2));

_articleFaker = new Faker<Article>()
.RuleFor(a => a.Name, f => f.Random.AlphaNumeric(10))
.RuleFor(a => a.Author, f => _authorFaker.Generate());

_tagFaker = new Faker<Tag>()
.CustomInstantiator(f => new Tag(_fixture.GetService<AppDbContext>()))
.RuleFor(a => a.Name, f => f.Random.AlphaNumeric(10));
Expand Down Expand Up @@ -282,7 +287,7 @@ public async Task Can_Create_Many_To_Many()
// Arrange
var context = _fixture.GetService<AppDbContext>();
var tag = _tagFaker.Generate();
var author = new Author();
var author = _authorFaker.Generate();
context.Tags.Add(tag);
context.AuthorDifferentDbContextName.Add(author);
await context.SaveChangesAsync();
Expand All @@ -294,6 +299,10 @@ public async Task Can_Create_Many_To_Many()
data = new
{
type = "articles",
attributes = new Dictionary<string, object>
{
{"name", "An article with relationships"}
},
relationships = new Dictionary<string, dynamic>
{
{ "author", new {
Expand Down
Loading