Fixed caching for multiple self-references

src/JsonApiDotNetCore/Resources/Annotations/IsRequiredAttribute.cs

@@ -1,7 +1,5 @@
 using System;
-using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations;
-using System.Linq;
 using JsonApiDotNetCore.Configuration;
 using JsonApiDotNetCore.Middleware;
 using Microsoft.AspNetCore.Http;
@@ -14,8 +12,7 @@ namespace JsonApiDotNetCore.Resources.Annotations
     /// </summary>
     public sealed class IsRequiredAttribute : RequiredAttribute
     {
-        // TODO: We need put this cache in some context, as it is not thread-safe (parallel requests) and grows indefinitely.
-        private static readonly HashSet<IIdentifiable> _selfReferencingEntitiesCache = new HashSet<IIdentifiable>(IdentifiableComparer.Instance);
+        private const string _isSelfReferencingResourceKey = "JsonApiDotNetCore_IsSelfReferencingResource";
 
         public override bool RequiresValidationContext => true;
 
@@ -27,7 +24,7 @@ protected override ValidationResult IsValid(object value, ValidationContext vali
             var request = validationContext.GetRequiredService<IJsonApiRequest>();
             var httpContextAccessor = validationContext.GetRequiredService<IHttpContextAccessor>();
 
-            if (ShouldSkipValidationForResource(validationContext, request) ||
+            if (ShouldSkipValidationForResource(validationContext, request, httpContextAccessor.HttpContext) ||
                 ShouldSkipValidationForProperty(validationContext, httpContextAccessor.HttpContext))
             {
                 return ValidationResult.Success;
@@ -36,7 +33,8 @@ protected override ValidationResult IsValid(object value, ValidationContext vali
             return base.IsValid(value, validationContext);
         }
 
-        private bool ShouldSkipValidationForResource(ValidationContext validationContext, IJsonApiRequest request)
+        private bool ShouldSkipValidationForResource(ValidationContext validationContext, IJsonApiRequest request,
+            HttpContext httpContext)
         {
             if (request.Kind == EndpointKind.Primary)
             {
@@ -56,14 +54,18 @@ private bool ShouldSkipValidationForResource(ValidationContext validationContext
                         return true;
                     }
 
-                    if (ResourceIsSelfReferencing(validationContext, identifiable))
+                    var isSelfReferencingResource = (bool?) httpContext.Items[_isSelfReferencingResourceKey];
+
+                    if (isSelfReferencingResource == null)
                     {
-                        _selfReferencingEntitiesCache.Add(identifiable);
+                        // 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.
 
-                        return false;
+                        isSelfReferencingResource = IsSelfReferencingResource(identifiable, validationContext);
+                        httpContext.Items[_isSelfReferencingResourceKey] = isSelfReferencingResource;
                     }
 
-                    if (_selfReferencingEntitiesCache.Contains(identifiable))
+                    if (isSelfReferencingResource.Value)
                     {
                         return true;
                     }
@@ -73,7 +75,7 @@ private bool ShouldSkipValidationForResource(ValidationContext validationContext
             return false;
         }
 
-        private bool ResourceIsSelfReferencing(ValidationContext validationContext, IIdentifiable identifiable)
+        private bool IsSelfReferencingResource(IIdentifiable identifiable, ValidationContext validationContext)
         {
             var provider = validationContext.GetRequiredService<IResourceContextProvider>();
             var relationships = provider.GetResourceContext(validationContext.ObjectType).Relationships;
@@ -88,7 +90,6 @@ private bool ResourceIsSelfReferencing(ValidationContext validationContext, IIde
                         return true;
                     }
                 }
-
             }
 
             return false;

test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/Enterprise.cs

@@ -12,6 +12,10 @@ public sealed class Enterprise : Identifiable
         [RegularExpression(@"^[\w\s]+$")]
         public string CompanyName { get; set; }
 
+        [Attr]
+        [IsRequired]
+        public string CityOfResidence { get; set; }
+
         [Attr]
         [MinLength(5)]
         public string Industry { get; set; }
@@ -24,5 +28,11 @@ public sealed class Enterprise : Identifiable
 
         [HasOne]
         public Enterprise Parent { get; set; }
+
+        [HasOne]
+        public Enterprise Self { get; set; }
+
+        [HasOne]
+        public Enterprise AlsoSelf { get; set; }
     }
 }

test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/EnterprisePartner.cs

@@ -17,6 +17,5 @@ public sealed class EnterprisePartner : Identifiable
         [Attr]
         [IsRequired]
         public EnterprisePartnerClassification Classification { get; set; }
-
     }
 }

test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/ModelStateDbContext.cs

@@ -11,5 +11,16 @@ public sealed class ModelStateDbContext : DbContext
         public ModelStateDbContext(DbContextOptions<ModelStateDbContext> options) : base(options)
         {
         }
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+        {
+            modelBuilder.Entity<Enterprise>()
+                .HasOne(enterprise => enterprise.Self)
+                .WithOne();
+
+            modelBuilder.Entity<Enterprise>()
+                .HasOne(enterprise => enterprise.AlsoSelf)
+                .WithOne();
+        }
     }
 }

test/JsonApiDotNetCoreExampleTests/IntegrationTests/ModelStateValidation/ModelStateValidationTests.cs

@@ -28,7 +28,8 @@ public async Task When_posting_resource_with_omitted_required_attribute_value_it
                     type = "enterprises",
                     attributes = new Dictionary<string, object>
                     {
-                        ["industry"] = "Transport"
+                        ["industry"] = "Transport",
+                        ["cityOfResidence"] = "Cambridge"
                     }
                 }
             };
@@ -61,7 +62,8 @@ public async Task When_posting_resource_with_null_for_required_attribute_value_i
                     attributes = new Dictionary<string, object>
                     {
                         ["companyName"] = null,
-                        ["industry"] = "Transport"
+                        ["industry"] = "Transport",
+                        ["cityOfResidence"] = "Cambridge"
                     }
                 }
             };
@@ -93,7 +95,8 @@ public async Task When_posting_resource_with_invalid_attribute_value_it_must_fai
                     type = "enterprises",
                     attributes = new Dictionary<string, object>
                     {
-                        ["companyName"] = "!@#$%^&*().-"
+                        ["companyName"] = "!@#$%^&*().-",
+                        ["cityOfResidence"] = "Cambridge"
                     }
                 }
             };
@@ -125,7 +128,8 @@ public async Task When_posting_resource_with_valid_attribute_value_it_must_succe
                     type = "enterprises",
                     attributes = new Dictionary<string, object>
                     {
-                        ["companyName"] = "Massive Dynamic"
+                        ["companyName"] = "Massive Dynamic",
+                        ["cityOfResidence"] = "Cambridge"
                     }
                 }
             };
@@ -141,6 +145,7 @@ public async Task When_posting_resource_with_valid_attribute_value_it_must_succe
 
             responseDocument.SingleData.Should().NotBeNull();
             responseDocument.SingleData.Attributes["companyName"].Should().Be("Massive Dynamic");
+            responseDocument.SingleData.Attributes["cityOfResidence"].Should().Be("Cambridge");
         }
 
         [Fact]
@@ -212,7 +217,8 @@ public async Task When_posting_resource_with_annotated_relationships_it_must_suc
 
             var parent = new Enterprise
             {
-                CompanyName = "Caesars Entertainment"
+                CompanyName = "Caesars Entertainment",
+                CityOfResidence = "Las Vegas"
             };
 
             await _testContext.RunOnDatabaseAsync(async dbContext =>
@@ -230,7 +236,8 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
                     type = "enterprises",
                     attributes = new Dictionary<string, object>
                     {
-                        ["companyName"] = "Flamingo Hotel"
+                        ["companyName"] = "Flamingo Hotel",
+                        ["cityOfResidence"] = "Las Vegas"
                     },
                     relationships = new Dictionary<string, object>
                     {
@@ -285,6 +292,7 @@ public async Task When_patching_resource_with_omitted_required_attribute_value_i
             var enterprise = new Enterprise
             {
                 CompanyName = "Massive Dynamic",
+                CityOfResidence = "Cambridge",
                 Industry = "Manufacturing"
             };
 
@@ -325,7 +333,8 @@ public async Task When_patching_resource_with_null_for_required_attribute_value_
             // Arrange
             var enterprise = new Enterprise
             {
-                CompanyName = "Massive Dynamic"
+                CompanyName = "Massive Dynamic",
+                CityOfResidence = "Cambridge"
             };
 
             await _testContext.RunOnDatabaseAsync(async dbContext =>
@@ -369,7 +378,8 @@ public async Task When_patching_resource_with_invalid_attribute_value_it_must_fa
             // Arrange
             var enterprise = new Enterprise
             {
-                CompanyName = "Massive Dynamic"
+                CompanyName = "Massive Dynamic",
+                CityOfResidence = "Cambridge"
             };
 
             await _testContext.RunOnDatabaseAsync(async dbContext =>
@@ -414,6 +424,7 @@ public async Task When_patching_resource_with_valid_attribute_value_it_must_succ
             var enterprise = new Enterprise
             {
                 CompanyName = "Massive Dynamic",
+                CityOfResidence = "Cambridge",
                 Industry = "Manufacturing"
             };
 
@@ -463,6 +474,7 @@ public async Task When_patching_resource_with_annotated_relationships_it_must_su
             var enterprise = new Enterprise
             {
                 CompanyName = "Bell Medics",
+                CityOfResidence = "Cambridge",
                 MailAddress = mailAddress,
                 Partners = new List<EnterprisePartner>
                 {
@@ -475,7 +487,8 @@ public async Task When_patching_resource_with_annotated_relationships_it_must_su
                 },
                 Parent = new Enterprise
                 {
-                    CompanyName = "Global Inc"
+                    CompanyName = "Global Inc",
+                    CityOfResidence = "New York"
                 }
             };
 
@@ -495,7 +508,8 @@ public async Task When_patching_resource_with_annotated_relationships_it_must_su
 
             var otherParent = new Enterprise
             {
-                CompanyName = "World Inc"
+                CompanyName = "World Inc",
+                CityOfResidence = "New York"
             };
 
             await _testContext.RunOnDatabaseAsync(async dbContext =>
@@ -563,21 +577,23 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
         }
 
         [Fact]
-        public async Task When_patching_resource_with_self_reference_it_must_succeed()
+        public async Task When_patching_resource_with_multiple_self_references_it_must_succeed()
         {
             // Arrange
             var enterprise = new Enterprise
             {
                 CompanyName = "Bell Medics",
+                CityOfResidence = "Cambridge",
                 Parent = new Enterprise
                 {
-                    CompanyName = "Global Inc"
+                    CompanyName = "Global Inc",
+                    CityOfResidence = "New York"
                 }
             };
 
             await _testContext.RunOnDatabaseAsync(async dbContext =>
             {
-                dbContext.Enterprises.AddRange(enterprise);
+                dbContext.Enterprises.Add(enterprise);
                 await dbContext.SaveChangesAsync();
             });
 
@@ -593,7 +609,15 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
                     },
                     relationships = new Dictionary<string, object>
                     {
-                        ["parent"] = new
+                        ["self"] = new
+                        {
+                            data = new
+                            {
+                                type = "enterprises",
+                                id = enterprise.StringId
+                            }
+                        },
+                        ["alsoSelf"] = new
                         {
                             data = new
                             {
@@ -624,15 +648,18 @@ public async Task When_patching_annotated_ToOne_relationship_it_must_succeed()
             var enterprise = new Enterprise
             {
                 CompanyName = "Bell Medics",
+                CityOfResidence = "Cambridge",
                 Parent = new Enterprise
                 {
-                    CompanyName = "Global Inc"
+                    CompanyName = "Global Inc",
+                    CityOfResidence = "New York"
                 }
             };
 
             var otherParent = new Enterprise
             {
-                CompanyName = "World Inc"
+                CompanyName = "World Inc",
+                CityOfResidence = "New York"
             };
 
             await _testContext.RunOnDatabaseAsync(async dbContext =>
@@ -669,6 +696,7 @@ public async Task When_patching_annotated_ToMany_relationship_it_must_succeed()
             var enterprise = new Enterprise
             {
                 CompanyName = "Bell Medics",
+                CityOfResidence = "Cambridge",
                 Partners = new List<EnterprisePartner>
                 {
                     new EnterprisePartner