Fixed: Do not run ModelState validation for relationships in POST/PATCH (#831)

Fixed: crash when post/patch contains relationships

Author: Bart Koelman

src/Examples/JsonApiDotNetCoreExample/Models/Article.cs

@@ -8,7 +8,6 @@ namespace JsonApiDotNetCoreExample.Models
     public sealed class Article : Identifiable
     {
         [Attr]
-        [IsRequired(AllowEmptyStrings = true)]
         public string Caption { get; set; }
 
         [Attr]

src/Examples/JsonApiDotNetCoreExample/Models/Author.cs

@@ -11,7 +11,6 @@ public sealed class Author : Identifiable
         public string FirstName { get; set; }
 
         [Attr]
-        [IsRequired(AllowEmptyStrings = true)]
         public string LastName { get; set; }
 
         [Attr]

src/Examples/JsonApiDotNetCoreExample/Models/Tag.cs

@@ -1,4 +1,3 @@
-using System.ComponentModel.DataAnnotations;
 using JsonApiDotNetCore.Resources;
 using JsonApiDotNetCore.Resources.Annotations;
 
@@ -7,7 +6,6 @@ namespace JsonApiDotNetCoreExample.Models
     public class Tag : Identifiable
     {
         [Attr]
-        [RegularExpression(@"^\W$")]
         public string Name { get; set; }
 
         [Attr]

src/JsonApiDotNetCore/Errors/InvalidModelStateException.cs

@@ -34,10 +34,7 @@ private static IReadOnlyCollection<Error> FromModelState(ModelStateDictionary mo
 
             foreach (var (propertyName, entry) in modelState.Where(x => x.Value.Errors.Any()))
             {
-                PropertyInfo property = resourceType.GetProperty(propertyName);
-
-                string attributeName =
-                    property.GetCustomAttribute<AttrAttribute>().PublicName ?? namingStrategy.GetPropertyName(property.Name, false);
+                string attributeName = GetDisplayNameForProperty(propertyName, resourceType, namingStrategy);
 
                 foreach (var modelError in entry.Errors)
                 {
@@ -55,6 +52,19 @@ private static IReadOnlyCollection<Error> FromModelState(ModelStateDictionary mo
             return errors;
         }
 
+        private static string GetDisplayNameForProperty(string propertyName, Type resourceType,
+            NamingStrategy namingStrategy)
+        {
+            PropertyInfo property = resourceType.GetProperty(propertyName);
+            if (property != null)
+            {
+                var attrAttribute = property.GetCustomAttribute<AttrAttribute>();
+                return attrAttribute?.PublicName ?? namingStrategy.GetPropertyName(property.Name, false);
+            }
+
+            return propertyName;
+        }
+
         private static Error FromModelError(ModelError modelError, string attributeName,
             bool includeExceptionStackTraceInErrors)
         {

src/JsonApiDotNetCore/Middleware/HttpContextExtensions.cs

@@ -5,42 +5,44 @@ namespace JsonApiDotNetCore.Middleware
 {
     public static class HttpContextExtensions
     {
+        private const string _isJsonApiRequestKey = "JsonApiDotNetCore_IsJsonApiRequest";
+        private const string _disableRequiredValidatorKey = "JsonApiDotNetCore_DisableRequiredValidator";
+
         /// <summary>
         /// Indicates whether the currently executing HTTP request is being handled by JsonApiDotNetCore.
         /// </summary>
         public static bool IsJsonApiRequest(this HttpContext httpContext)
         {
             if (httpContext == null) throw new ArgumentNullException(nameof(httpContext));
 
-            string value = httpContext.Items["IsJsonApiRequest"] as string;
+            string value = httpContext.Items[_isJsonApiRequestKey] as string;
             return value == bool.TrueString;
         }
 
         internal static void RegisterJsonApiRequest(this HttpContext httpContext)
         {
             if (httpContext == null) throw new ArgumentNullException(nameof(httpContext));
 
-            httpContext.Items["IsJsonApiRequest"] = bool.TrueString;
+            httpContext.Items[_isJsonApiRequestKey] = bool.TrueString;
         }
 
-        internal static void DisableValidator(this HttpContext httpContext, string propertyName, string model)
+        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 = $"JsonApiDotNetCore_DisableValidation_{model}_{propertyName}";
+            var itemKey = $"{_disableRequiredValidatorKey}_{model}_{propertyName}";
             httpContext.Items[itemKey] = true;
         }
 
-        internal static bool IsValidatorDisabled(this HttpContext httpContext, string propertyName, string model)
+        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($"JsonApiDotNetCore_DisableValidation_{model}_{propertyName}") ||
-                   httpContext.Items.ContainsKey($"JsonApiDotNetCore_DisableValidation_{model}_Relation");
+            return httpContext.Items.ContainsKey($"{_disableRequiredValidatorKey}_{model}_{propertyName}");
         }
     }
 }

src/JsonApiDotNetCore/Resources/Annotations/IsRequiredAttribute.cs

@@ -1,5 +1,9 @@
 using System;
+using System.Collections;
+using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations;
+using System.Linq;
+using JsonApiDotNetCore.Configuration;
 using JsonApiDotNetCore.Middleware;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.DependencyInjection;
@@ -11,22 +15,104 @@ namespace JsonApiDotNetCore.Resources.Annotations
     /// </summary>
     public sealed class IsRequiredAttribute : RequiredAttribute
     {
-        private bool _isDisabled;
+        private const string _isSelfReferencingResourceKey = "JsonApiDotNetCore_IsSelfReferencingResource";
 
-        /// <inheritdoc />
-        public override bool IsValid(object value)
-        {
-            return _isDisabled || base.IsValid(value);
-        }
+        public override bool RequiresValidationContext => true;
 
         /// <inheritdoc />
         protected override ValidationResult IsValid(object value, ValidationContext validationContext)
         {
             if (validationContext == null) throw new ArgumentNullException(nameof(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);
+            var request = validationContext.GetRequiredService<IJsonApiRequest>();
+            var httpContextAccessor = validationContext.GetRequiredService<IHttpContextAccessor>();
+
+            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<IResourceContextProvider>();
+            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<IIdentifiable>().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);
         }
     }
 }

src/JsonApiDotNetCore/Hooks/Internal/IdentifiableComparer.cs renamed to src/JsonApiDotNetCore/Resources/IdentifiableComparer.cs

@@ -1,10 +1,9 @@
 using System.Collections.Generic;
-using JsonApiDotNetCore.Resources;
 
-namespace JsonApiDotNetCore.Hooks.Internal
+namespace JsonApiDotNetCore.Resources
 {
     /// <summary>
-    /// Compares `IIdentifiable` with each other based on ID
+    /// Compares `IIdentifiable` instances with each other based on StringId.
     /// </summary>
     internal sealed class IdentifiableComparer : IEqualityComparer<IIdentifiable>
     {

src/JsonApiDotNetCore/Serialization/RequestDeserializer.cs

@@ -79,34 +79,17 @@ protected override IIdentifiable SetAttributes(IIdentifiable resource, IDictiona
                 {
                     if (attr.Property.GetCustomAttribute<IsRequiredAttribute>() != null)
                     {
-                        bool disableValidator = attributeValues == null || attributeValues.Count == 0 ||
-                                                !attributeValues.TryGetValue(attr.PublicName, out _);
+                        bool disableValidator = attributeValues == null || !attributeValues.ContainsKey(attr.PublicName);
 
                         if (disableValidator)
                         {
-                            _httpContextAccessor.HttpContext.DisableValidator(attr.Property.Name, resource.GetType().Name);
+                            _httpContextAccessor.HttpContext.DisableRequiredValidator(attr.Property.Name, resource.GetType().Name);
                         }
                     }
                 }
             }
 
             return base.SetAttributes(resource, attributeValues, attributes);
         }
-
-        protected override IIdentifiable SetRelationships(IIdentifiable resource, IDictionary<string, RelationshipEntry> relationshipsValues, IReadOnlyCollection<RelationshipAttribute> relationshipAttributes)
-        {
-            if (resource == null) throw new ArgumentNullException(nameof(resource));
-            if (relationshipAttributes == null) throw new ArgumentNullException(nameof(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.Property.Name);
-            }
-
-            return base.SetRelationships(resource, relationshipsValues, relationshipAttributes);
-        }
     }
 }