diff --git a/src/JsonApiDotNetCore/Configuration/JsonApiApplicationBuilder.cs b/src/JsonApiDotNetCore/Configuration/JsonApiApplicationBuilder.cs index 7aeffd5518..eb742cacd4 100644 --- a/src/JsonApiDotNetCore/Configuration/JsonApiApplicationBuilder.cs +++ b/src/JsonApiDotNetCore/Configuration/JsonApiApplicationBuilder.cs @@ -17,6 +17,7 @@ using JsonApiDotNetCore.Services; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; @@ -103,6 +104,7 @@ public void ConfigureMvc() if (_options.ValidateModelState) { _mvcBuilder.AddDataAnnotations(); + _services.AddSingleton(); } } @@ -163,7 +165,7 @@ private void AddMiddlewareLayer() _services.TryAddSingleton(); _services.AddSingleton(sp => sp.GetRequiredService()); _services.AddSingleton(); - _services.AddScoped(); + _services.AddSingleton(); _services.AddScoped(); _services.AddScoped(); _services.AddScoped(); diff --git a/src/JsonApiDotNetCore/Configuration/JsonApiMetadataProvider.cs b/src/JsonApiDotNetCore/Configuration/JsonApiMetadataProvider.cs new file mode 100644 index 0000000000..449035ac2f --- /dev/null +++ b/src/JsonApiDotNetCore/Configuration/JsonApiMetadataProvider.cs @@ -0,0 +1,42 @@ +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; +using Microsoft.Extensions.Options; + +namespace JsonApiDotNetCore.Configuration +{ + /// + /// Custom implementation of to support json:api partial patching. + /// + internal class JsonApiModelMetadataProvider : DefaultModelMetadataProvider + { + private readonly JsonApiValidationFilter _jsonApiValidationFilter; + + /// + public JsonApiModelMetadataProvider(ICompositeMetadataDetailsProvider detailsProvider, IRequestScopedServiceProvider serviceProvider) + : base(detailsProvider) + { + _jsonApiValidationFilter = new JsonApiValidationFilter(serviceProvider); + } + + /// + public JsonApiModelMetadataProvider(ICompositeMetadataDetailsProvider detailsProvider, IOptions optionsAccessor, IRequestScopedServiceProvider serviceProvider) + : base(detailsProvider, optionsAccessor) + { + _jsonApiValidationFilter = new JsonApiValidationFilter(serviceProvider); + } + + /// + protected override ModelMetadata CreateModelMetadata(DefaultMetadataDetails entry) + { + var metadata = (DefaultModelMetadata)base.CreateModelMetadata(entry); + + if (metadata.ValidationMetadata.IsRequired == true) + { + metadata.ValidationMetadata.PropertyValidationFilter = _jsonApiValidationFilter; + } + + return metadata; + } + } +} diff --git a/src/JsonApiDotNetCore/Configuration/JsonApiValidationFilter.cs b/src/JsonApiDotNetCore/Configuration/JsonApiValidationFilter.cs new file mode 100644 index 0000000000..b4af0ea6f5 --- /dev/null +++ b/src/JsonApiDotNetCore/Configuration/JsonApiValidationFilter.cs @@ -0,0 +1,59 @@ +using System; +using System.Linq; +using JsonApiDotNetCore.Middleware; +using JsonApiDotNetCore.Resources; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; +using Microsoft.Extensions.DependencyInjection; + +namespace JsonApiDotNetCore.Configuration +{ + /// + /// Validation filter that blocks ASP.NET Core ModelState validation on data according to the json:api spec. + /// + internal sealed class JsonApiValidationFilter : IPropertyValidationFilter + { + private readonly IRequestScopedServiceProvider _serviceProvider; + + public JsonApiValidationFilter(IRequestScopedServiceProvider serviceProvider) + { + _serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider)); + } + + /// + public bool ShouldValidateEntry(ValidationEntry entry, ValidationEntry parentEntry) + { + var request = _serviceProvider.GetRequiredService(); + + if (IsId(entry.Key)) + { + return true; + } + + var isTopResourceInPrimaryRequest = string.IsNullOrEmpty(parentEntry.Key) && request.Kind == EndpointKind.Primary; + if (!isTopResourceInPrimaryRequest) + { + return false; + } + + var httpContextAccessor = _serviceProvider.GetRequiredService(); + if (httpContextAccessor.HttpContext.Request.Method == HttpMethods.Patch) + { + var targetedFields = _serviceProvider.GetRequiredService(); + return IsFieldTargeted(entry, targetedFields); + } + + return true; + } + + private static bool IsId(string key) + { + return key == nameof(Identifiable.Id) || key.EndsWith("." + nameof(Identifiable.Id), StringComparison.Ordinal); + } + + private static bool IsFieldTargeted(ValidationEntry entry, ITargetedFields targetedFields) + { + return targetedFields.Attributes.Any(attribute => attribute.Property.Name == entry.Key); + } + } +} diff --git a/src/JsonApiDotNetCore/Configuration/ServiceCollectionExtensions.cs b/src/JsonApiDotNetCore/Configuration/ServiceCollectionExtensions.cs index ffa5fd340e..1c1e77f436 100644 --- a/src/JsonApiDotNetCore/Configuration/ServiceCollectionExtensions.cs +++ b/src/JsonApiDotNetCore/Configuration/ServiceCollectionExtensions.cs @@ -1,10 +1,8 @@ using System; using System.Collections.Generic; -using System.ComponentModel.Design; using System.Linq; using System.Reflection; using JsonApiDotNetCore.Errors; -using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Serialization.Building; using JsonApiDotNetCore.Serialization.Client.Internal; using JsonApiDotNetCore.Services; diff --git a/src/JsonApiDotNetCore/Middleware/HttpContextExtensions.cs b/src/JsonApiDotNetCore/Middleware/HttpContextExtensions.cs index 1e11c9758f..ccafcae28c 100644 --- a/src/JsonApiDotNetCore/Middleware/HttpContextExtensions.cs +++ b/src/JsonApiDotNetCore/Middleware/HttpContextExtensions.cs @@ -6,7 +6,6 @@ namespace JsonApiDotNetCore.Middleware public static class HttpContextExtensions { private const string _isJsonApiRequestKey = "JsonApiDotNetCore_IsJsonApiRequest"; - private const string _disableRequiredValidatorKey = "JsonApiDotNetCore_DisableRequiredValidator"; /// /// Indicates whether the currently executing HTTP request is being handled by JsonApiDotNetCore. @@ -25,24 +24,5 @@ internal static void RegisterJsonApiRequest(this HttpContext httpContext) httpContext.Items[_isJsonApiRequestKey] = bool.TrueString; } - - internal static void DisableRequiredValidator(this HttpContext httpContext, string propertyName, string model) - { - if (httpContext == null) throw new ArgumentNullException(nameof(httpContext)); - if (propertyName == null) throw new ArgumentNullException(nameof(propertyName)); - if (model == null) throw new ArgumentNullException(nameof(model)); - - var itemKey = $"{_disableRequiredValidatorKey}_{model}_{propertyName}"; - httpContext.Items[itemKey] = true; - } - - internal static bool IsRequiredValidatorDisabled(this HttpContext httpContext, string propertyName, string model) - { - if (httpContext == null) throw new ArgumentNullException(nameof(httpContext)); - if (propertyName == null) throw new ArgumentNullException(nameof(propertyName)); - if (model == null) throw new ArgumentNullException(nameof(model)); - - return httpContext.Items.ContainsKey($"{_disableRequiredValidatorKey}_{model}_{propertyName}"); - } } } diff --git a/src/JsonApiDotNetCore/Resources/Annotations/IsRequiredAttribute.cs b/src/JsonApiDotNetCore/Resources/Annotations/IsRequiredAttribute.cs deleted file mode 100644 index 0d031aa7d2..0000000000 --- a/src/JsonApiDotNetCore/Resources/Annotations/IsRequiredAttribute.cs +++ /dev/null @@ -1,117 +0,0 @@ -using System; -using System.Collections; -using System.ComponentModel.DataAnnotations; -using System.Linq; -using JsonApiDotNetCore.Configuration; -using JsonApiDotNetCore.Middleware; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; - -namespace JsonApiDotNetCore.Resources.Annotations -{ - /// - /// Used with model state validation as a replacement for the built-in to support partial updates. - /// - public sealed class IsRequiredAttribute : RequiredAttribute - { - private const string _isSelfReferencingResourceKey = "JsonApiDotNetCore_IsSelfReferencingResource"; - - public override bool RequiresValidationContext => true; - - /// - protected override ValidationResult IsValid(object value, ValidationContext validationContext) - { - if (validationContext == null) throw new ArgumentNullException(nameof(validationContext)); - - var request = validationContext.GetRequiredService(); - var httpContextAccessor = validationContext.GetRequiredService(); - - if (ShouldSkipValidationForResource(validationContext, request, httpContextAccessor.HttpContext) || - ShouldSkipValidationForProperty(validationContext, httpContextAccessor.HttpContext)) - { - return ValidationResult.Success; - } - - return base.IsValid(value, validationContext); - } - - private bool ShouldSkipValidationForResource(ValidationContext validationContext, IJsonApiRequest request, - HttpContext httpContext) - { - if (request.Kind == EndpointKind.Primary) - { - // 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. - - if (validationContext.ObjectType != request.PrimaryResource.ResourceType) - { - return true; - } - - if (validationContext.ObjectInstance is IIdentifiable identifiable) - { - if (identifiable.StringId != request.PrimaryId) - { - return true; - } - - var isSelfReferencingResource = (bool?) httpContext.Items[_isSelfReferencingResourceKey]; - - if (isSelfReferencingResource == null) - { - // When processing a request, the first time we get here is for the top-level resource. - // Subsequent validations for related resources inspect the cache to know that their validation can be skipped. - - isSelfReferencingResource = IsSelfReferencingResource(identifiable, validationContext); - httpContext.Items[_isSelfReferencingResourceKey] = isSelfReferencingResource; - } - - if (isSelfReferencingResource.Value) - { - return true; - } - } - } - - return false; - } - - private bool IsSelfReferencingResource(IIdentifiable identifiable, ValidationContext validationContext) - { - var provider = validationContext.GetRequiredService(); - var relationships = provider.GetResourceContext(validationContext.ObjectType).Relationships; - - foreach (var relationship in relationships) - { - if (relationship is HasOneAttribute hasOne) - { - var relationshipValue = (IIdentifiable) hasOne.GetValue(identifiable); - if (IdentifiableComparer.Instance.Equals(identifiable, relationshipValue)) - { - return true; - } - } - - if (relationship is HasManyAttribute hasMany) - { - var collection = (IEnumerable) hasMany.GetValue(identifiable); - - if (collection != null && collection.OfType().Any(resource => - IdentifiableComparer.Instance.Equals(identifiable, resource))) - { - return true; - } - } - } - - return false; - } - - private bool ShouldSkipValidationForProperty(ValidationContext validationContext, HttpContext httpContext) - { - return httpContext.IsRequiredValidatorDisabled(validationContext.MemberName, - validationContext.ObjectType.Name); - } - } -} diff --git a/src/JsonApiDotNetCore/Serialization/BaseDeserializer.cs b/src/JsonApiDotNetCore/Serialization/BaseDeserializer.cs index 5e2e94a757..61d6ec1408 100644 --- a/src/JsonApiDotNetCore/Serialization/BaseDeserializer.cs +++ b/src/JsonApiDotNetCore/Serialization/BaseDeserializer.cs @@ -68,7 +68,7 @@ protected object DeserializeBody(string body) /// The parsed resource. /// Attributes and their values, as in the serialized content. /// Exposed attributes for . - protected virtual IIdentifiable SetAttributes(IIdentifiable resource, IDictionary attributeValues, IReadOnlyCollection attributes) + protected IIdentifiable SetAttributes(IIdentifiable resource, IDictionary attributeValues, IReadOnlyCollection attributes) { if (resource == null) throw new ArgumentNullException(nameof(resource)); if (attributes == null) throw new ArgumentNullException(nameof(attributes)); diff --git a/src/JsonApiDotNetCore/Serialization/RequestDeserializer.cs b/src/JsonApiDotNetCore/Serialization/RequestDeserializer.cs index cc8250ba57..6c4a26796e 100644 --- a/src/JsonApiDotNetCore/Serialization/RequestDeserializer.cs +++ b/src/JsonApiDotNetCore/Serialization/RequestDeserializer.cs @@ -1,10 +1,7 @@ using System; -using System.Collections.Generic; using System.Net.Http; -using System.Reflection; using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Errors; -using JsonApiDotNetCore.Middleware; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Resources.Annotations; using JsonApiDotNetCore.Serialization.Objects; @@ -67,29 +64,5 @@ protected override void AfterProcessField(IIdentifiable resource, ResourceFieldA else if (field is RelationshipAttribute relationship) _targetedFields.Relationships.Add(relationship); } - - protected override IIdentifiable SetAttributes(IIdentifiable resource, IDictionary attributeValues, IReadOnlyCollection attributes) - { - if (resource == null) throw new ArgumentNullException(nameof(resource)); - if (attributes == null) throw new ArgumentNullException(nameof(attributes)); - - if (_httpContextAccessor.HttpContext.Request.Method == HttpMethod.Patch.Method) - { - foreach (AttrAttribute attr in attributes) - { - if (attr.Property.GetCustomAttribute() != null) - { - bool disableValidator = attributeValues == null || !attributeValues.ContainsKey(attr.PublicName); - - if (disableValidator) - { - _httpContextAccessor.HttpContext.DisableRequiredValidator(attr.Property.Name, resource.GetType().Name); - } - } - } - } - - return base.SetAttributes(resource, attributeValues, attributes); - } } } diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/ModelStateValidationTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/ModelStateValidationTests.cs index e85da62d5b..5f2d74715a 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/ModelStateValidationTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/ModelStateValidationTests.cs @@ -410,6 +410,71 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.Errors[0].Source.Pointer.Should().Be("/data/attributes/name"); } + [Fact] + public async Task When_patching_resource_with_invalid_ID_it_must_fail() + { + // Arrange + var directory = new SystemDirectory + { + Name = "Projects", + IsCaseSensitive = true + }; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Directories.Add(directory); + await dbContext.SaveChangesAsync(); + }); + + var content = new + { + data = new + { + type = "systemDirectories", + id = -1, + attributes = new Dictionary + { + ["name"] = "Repositories" + }, + relationships = new Dictionary + { + ["subdirectories"] = new + { + data = new[] + { + new + { + type = "systemDirectories", + id = -1 + } + } + } + } + } + }; + + string requestBody = JsonConvert.SerializeObject(content); + string route = "/systemDirectories/-1"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecutePatchAsync(route, requestBody); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.UnprocessableEntity); + + responseDocument.Errors.Should().HaveCount(2); + + responseDocument.Errors[0].StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + responseDocument.Errors[0].Title.Should().Be("Input validation failed."); + responseDocument.Errors[0].Detail.Should().Be("The field Id must match the regular expression '^[0-9]+$'."); + responseDocument.Errors[0].Source.Pointer.Should().Be("/data/attributes/id"); + + responseDocument.Errors[1].StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity); + responseDocument.Errors[1].Title.Should().Be("Input validation failed."); + responseDocument.Errors[1].Detail.Should().Be("The field Id must match the regular expression '^[0-9]+$'."); + responseDocument.Errors[1].Source.Pointer.Should().Be("/data/attributes/Subdirectories[0].Id"); + } + [Fact] public async Task When_patching_resource_with_valid_attribute_value_it_must_succeed() { diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/SystemDirectory.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/SystemDirectory.cs index e60b4bd864..9c851fe832 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/SystemDirectory.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/SystemDirectory.cs @@ -7,13 +7,17 @@ namespace JsonApiDotNetCoreExampleTests.IntegrationTests.ModelStateValidation { public sealed class SystemDirectory : Identifiable { + [Required] + [RegularExpression("^[0-9]+$")] + public override int Id { get; set; } + [Attr] - [IsRequired] + [Required] [RegularExpression(@"^[\w\s]+$")] public string Name { get; set; } [Attr] - [IsRequired] + [Required] public bool? IsCaseSensitive { get; set; } [Attr] diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/SystemFile.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/SystemFile.cs index bb2e27a6fc..a7890730f2 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/SystemFile.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/SystemFile.cs @@ -7,12 +7,12 @@ namespace JsonApiDotNetCoreExampleTests.IntegrationTests.ModelStateValidation public sealed class SystemFile : Identifiable { [Attr] - [IsRequired] + [Required] [MinLength(1)] public string FileName { get; set; } [Attr] - [IsRequired] + [Required] [Range(typeof(long), "0", "9223372036854775807")] public long SizeInBytes { get; set; } } diff --git a/test/UnitTests/Serialization/Client/RequestSerializerTests.cs b/test/UnitTests/Serialization/Client/RequestSerializerTests.cs index 0089f70262..e5a2d696ce 100644 --- a/test/UnitTests/Serialization/Client/RequestSerializerTests.cs +++ b/test/UnitTests/Serialization/Client/RequestSerializerTests.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.ComponentModel.Design; using System.Text.RegularExpressions; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Serialization.Building; diff --git a/test/UnitTests/Serialization/Common/ResourceObjectBuilderTests.cs b/test/UnitTests/Serialization/Common/ResourceObjectBuilderTests.cs index 1e884c28f1..58d380cee9 100644 --- a/test/UnitTests/Serialization/Common/ResourceObjectBuilderTests.cs +++ b/test/UnitTests/Serialization/Common/ResourceObjectBuilderTests.cs @@ -1,9 +1,7 @@ using System; using System.Collections; using System.Collections.Generic; -using System.ComponentModel.Design; using System.Linq; -using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Serialization.Building; using JsonApiDotNetCore.Serialization.Objects; using UnitTests.TestModels;