Merge pull request #1253 from json-api-dotnet/fix-modelstate-validation-non-required

maurei · web-flow · commit 2324b69d2c4c · 2023-02-05T21:07:25.000+01:00
Fixed: ModelState validation failed when [Range] does not include property default value
diff --git a/src/JsonApiDotNetCore/Configuration/JsonApiModelMetadataProvider.cs b/src/JsonApiDotNetCore/Configuration/JsonApiModelMetadataProvider.cs
@@ -32,11 +32,7 @@ public JsonApiModelMetadataProvider(ICompositeMetadataDetailsProvider detailsPro
     protected override ModelMetadata CreateModelMetadata(DefaultMetadataDetails entry)
     {
         var metadata = (DefaultModelMetadata)base.CreateModelMetadata(entry);
-
-        if (metadata.ValidationMetadata.IsRequired == true)
-        {
-            metadata.ValidationMetadata.PropertyValidationFilter = _jsonApiValidationFilter;
-        }
+        metadata.ValidationMetadata.PropertyValidationFilter = _jsonApiValidationFilter;
 
         return metadata;
     }
diff --git a/src/JsonApiDotNetCore/Configuration/JsonApiValidationFilter.cs b/src/JsonApiDotNetCore/Configuration/JsonApiValidationFilter.cs
@@ -1,6 +1,7 @@
 using JsonApiDotNetCore.Middleware;
 using JsonApiDotNetCore.Resources;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
 using Microsoft.Extensions.DependencyInjection;
 
@@ -23,15 +24,13 @@ public JsonApiValidationFilter(IHttpContextAccessor httpContextAccessor)
     /// <inheritdoc />
     public bool ShouldValidateEntry(ValidationEntry entry, ValidationEntry parentEntry)
     {
-        IServiceProvider serviceProvider = GetScopedServiceProvider();
-
-        var request = serviceProvider.GetRequiredService<IJsonApiRequest>();
-
-        if (IsId(entry.Key))
+        if (entry.Metadata.MetadataKind == ModelMetadataKind.Type || IsId(entry.Key))
         {
             return true;
         }
 
+        IServiceProvider serviceProvider = GetScopedServiceProvider();
+        var request = serviceProvider.GetRequiredService<IJsonApiRequest>();
         bool isTopResourceInPrimaryRequest = string.IsNullOrEmpty(parentEntry.Key) && IsAtPrimaryEndpoint(request);
 
         if (!isTopResourceInPrimaryRequest)
diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/InputValidation/ModelState/ModelStateFakers.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/InputValidation/ModelState/ModelStateFakers.cs
@@ -17,14 +17,14 @@ internal sealed class ModelStateFakers : FakerContainer
         new Faker<SystemFile>()
             .UseSeed(GetFakerSeed())
             .RuleFor(systemFile => systemFile.FileName, faker => faker.System.FileName())
+            .RuleFor(systemFile => systemFile.Attributes, faker => faker.Random.Enum(FileAttributes.Normal, FileAttributes.Hidden, FileAttributes.ReadOnly))
             .RuleFor(systemFile => systemFile.SizeInBytes, faker => faker.Random.Long(0, 1_000_000)));
 
     private readonly Lazy<Faker<SystemDirectory>> _lazySystemDirectoryFaker = new(() =>
         new Faker<SystemDirectory>()
             .UseSeed(GetFakerSeed())
-            .RuleFor(systemDirectory => systemDirectory.Name, faker => faker.Address.City())
-            .RuleFor(systemDirectory => systemDirectory.IsCaseSensitive, faker => faker.Random.Bool())
-            .RuleFor(systemDirectory => systemDirectory.SizeInBytes, faker => faker.Random.Long(0, 1_000_000)));
+            .RuleFor(systemDirectory => systemDirectory.Name, faker => Path.GetFileNameWithoutExtension(faker.System.FileName()))
+            .RuleFor(systemDirectory => systemDirectory.IsCaseSensitive, faker => faker.Random.Bool()));
 
     public Faker<SystemVolume> SystemVolume => _lazySystemVolumeFaker.Value;
     public Faker<SystemFile> SystemFile => _lazySystemFileFaker.Value;
diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/InputValidation/ModelState/ModelStateValidationTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/InputValidation/ModelState/ModelStateValidationTests.cs
@@ -166,8 +166,6 @@ public async Task Cannot_create_resource_with_multiple_violations()
                 type = "systemDirectories",
                 attributes = new
                 {
-                    isCaseSensitive = false,
-                    sizeInBytes = -1
                 }
             }
         };
@@ -192,9 +190,9 @@ public async Task Cannot_create_resource_with_multiple_violations()
         ErrorObject error2 = responseDocument.Errors[1];
         error2.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
         error2.Title.Should().Be("Input validation failed.");
-        error2.Detail.Should().Be("The field SizeInBytes must be between 0 and 9223372036854775807.");
+        error2.Detail.Should().Be("The IsCaseSensitive field is required.");
         error2.Source.ShouldNotBeNull();
-        error2.Source.Pointer.Should().Be("/data/attributes/sizeInBytes");
+        error2.Source.Pointer.Should().Be("/data/attributes/isCaseSensitive");
     }
 
     [Fact]
@@ -205,15 +203,14 @@ public async Task Does_not_exceed_MaxModelValidationErrors()
         {
             data = new
             {
-                type = "systemDirectories",
+                type = "systemFiles",
                 attributes = new
                 {
-                    sizeInBytes = -1
                 }
             }
         };
 
-        const string route = "/systemDirectories";
+        const string route = "/systemFiles";
 
         // Act
         (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecutePostAsync<Document>(route, requestBody);
@@ -232,16 +229,16 @@ public async Task Does_not_exceed_MaxModelValidationErrors()
         ErrorObject error2 = responseDocument.Errors[1];
         error2.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
         error2.Title.Should().Be("Input validation failed.");
-        error2.Detail.Should().Be("The Name field is required.");
+        error2.Detail.Should().Be("The FileName field is required.");
         error2.Source.ShouldNotBeNull();
-        error2.Source.Pointer.Should().Be("/data/attributes/directoryName");
+        error2.Source.Pointer.Should().Be("/data/attributes/fileName");
 
         ErrorObject error3 = responseDocument.Errors[2];
         error3.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
         error3.Title.Should().Be("Input validation failed.");
-        error3.Detail.Should().Be("The IsCaseSensitive field is required.");
+        error3.Detail.Should().Be("The Attributes field is required.");
         error3.Source.ShouldNotBeNull();
-        error3.Source.Pointer.Should().Be("/data/attributes/isCaseSensitive");
+        error3.Source.Pointer.Should().Be("/data/attributes/attributes");
     }
 
     [Fact]
@@ -360,30 +357,30 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
     public async Task Can_update_resource_with_omitted_required_attribute_value()
     {
         // Arrange
-        SystemDirectory existingDirectory = _fakers.SystemDirectory.Generate();
+        SystemFile existingFile = _fakers.SystemFile.Generate();
 
-        long newSizeInBytes = _fakers.SystemDirectory.Generate().SizeInBytes;
+        long? newSizeInBytes = _fakers.SystemFile.Generate().SizeInBytes;
 
         await _testContext.RunOnDatabaseAsync(async dbContext =>
         {
-            dbContext.Directories.Add(existingDirectory);
+            dbContext.Files.Add(existingFile);
             await dbContext.SaveChangesAsync();
         });
 
         var requestBody = new
         {
             data = new
             {
-                type = "systemDirectories",
-                id = existingDirectory.StringId,
+                type = "systemFiles",
+                id = existingFile.StringId,
                 attributes = new
                 {
                     sizeInBytes = newSizeInBytes
                 }
             }
         };
 
-        string route = $"/systemDirectories/{existingDirectory.StringId}";
+        string route = $"/systemFiles/{existingFile.StringId}";
 
         // Act
         (HttpResponseMessage httpResponse, string responseDocument) = await _testContext.ExecutePatchAsync<string>(route, requestBody);
diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/InputValidation/ModelState/SystemDirectory.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/InputValidation/ModelState/SystemDirectory.cs
@@ -20,10 +20,6 @@ public sealed class SystemDirectory : Identifiable<int>
     [Required]
     public bool? IsCaseSensitive { get; set; }
 
-    [Attr]
-    [Range(typeof(long), "0", "9223372036854775807")]
-    public long SizeInBytes { get; set; }
-
     [HasMany]
     public ICollection<SystemDirectory> Subdirectories { get; set; } = new List<SystemDirectory>();
 
diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/InputValidation/ModelState/SystemFile.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/InputValidation/ModelState/SystemFile.cs
@@ -15,6 +15,9 @@ public sealed class SystemFile : Identifiable<int>
 
     [Attr]
     [Required]
-    [Range(typeof(long), "0", "9223372036854775807")]
-    public long? SizeInBytes { get; set; }
+    public FileAttributes? Attributes { get; set; }
+
+    [Attr]
+    [Range(typeof(long), "1", "9223372036854775807")]
+    public long SizeInBytes { get; set; }
 }

Original file line number	Diff line number	Diff line change
`@@ -32,11 +32,7 @@ public JsonApiModelMetadataProvider(ICompositeMetadataDetailsProvider detailsPro`
`32`	`32`	`protected override ModelMetadata CreateModelMetadata(DefaultMetadataDetails entry)`
`33`	`33`	`{`
`34`	`34`	`var metadata = (DefaultModelMetadata)base.CreateModelMetadata(entry);`
`35`		`-`
`36`		`- if (metadata.ValidationMetadata.IsRequired == true)`
`37`		`- {`
`38`		`- metadata.ValidationMetadata.PropertyValidationFilter = _jsonApiValidationFilter;`
`39`		`- }`
	`35`	`+ metadata.ValidationMetadata.PropertyValidationFilter = _jsonApiValidationFilter;`
`40`	`36`
`41`	`37`	`return metadata;`
`42`	`38`	`}`
Original file line number	Diff line number	Diff line change
`@@ -166,8 +166,6 @@ public async Task Cannot_create_resource_with_multiple_violations()`
`166`	`166`	`type = "systemDirectories",`
`167`	`167`	`attributes = new`
`168`	`168`	`{`
`169`		`- isCaseSensitive = false,`
`170`		`- sizeInBytes = -1`
`171`	`169`	`}`
`172`	`170`	`}`
`173`	`171`	`};`
`@@ -192,9 +190,9 @@ public async Task Cannot_create_resource_with_multiple_violations()`
`192`	`190`	`ErrorObject error2 = responseDocument.Errors[1];`
`193`	`191`	`error2.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);`
`194`	`192`	`error2.Title.Should().Be("Input validation failed.");`
`195`		`- error2.Detail.Should().Be("The field SizeInBytes must be between 0 and 9223372036854775807.");`
	`193`	`+ error2.Detail.Should().Be("The IsCaseSensitive field is required.");`
`196`	`194`	`error2.Source.ShouldNotBeNull();`
`197`		`- error2.Source.Pointer.Should().Be("/data/attributes/sizeInBytes");`
	`195`	`+ error2.Source.Pointer.Should().Be("/data/attributes/isCaseSensitive");`
`198`	`196`	`}`
`199`	`197`
`200`	`198`	`[Fact]`
`@@ -205,15 +203,14 @@ public async Task Does_not_exceed_MaxModelValidationErrors()`
`205`	`203`	`{`
`206`	`204`	`data = new`
`207`	`205`	`{`
`208`		`- type = "systemDirectories",`
	`206`	`+ type = "systemFiles",`
`209`	`207`	`attributes = new`
`210`	`208`	`{`
`211`		`- sizeInBytes = -1`
`212`	`209`	`}`
`213`	`210`	`}`
`214`	`211`	`};`
`215`	`212`
`216`		`- const string route = "/systemDirectories";`
	`213`	`+ const string route = "/systemFiles";`
`217`	`214`
`218`	`215`	`// Act`
`219`	`216`	`(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecutePostAsync<Document>(route, requestBody);`
`@@ -232,16 +229,16 @@ public async Task Does_not_exceed_MaxModelValidationErrors()`
`232`	`229`	`ErrorObject error2 = responseDocument.Errors[1];`
`233`	`230`	`error2.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);`
`234`	`231`	`error2.Title.Should().Be("Input validation failed.");`
`235`		`- error2.Detail.Should().Be("The Name field is required.");`
	`232`	`+ error2.Detail.Should().Be("The FileName field is required.");`
`236`	`233`	`error2.Source.ShouldNotBeNull();`
`237`		`- error2.Source.Pointer.Should().Be("/data/attributes/directoryName");`
	`234`	`+ error2.Source.Pointer.Should().Be("/data/attributes/fileName");`
`238`	`235`
`239`	`236`	`ErrorObject error3 = responseDocument.Errors[2];`
`240`	`237`	`error3.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);`
`241`	`238`	`error3.Title.Should().Be("Input validation failed.");`
`242`		`- error3.Detail.Should().Be("The IsCaseSensitive field is required.");`
	`239`	`+ error3.Detail.Should().Be("The Attributes field is required.");`
`243`	`240`	`error3.Source.ShouldNotBeNull();`
`244`		`- error3.Source.Pointer.Should().Be("/data/attributes/isCaseSensitive");`
	`241`	`+ error3.Source.Pointer.Should().Be("/data/attributes/attributes");`
`245`	`242`	`}`
`246`	`243`
`247`	`244`	`[Fact]`
`@@ -360,30 +357,30 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>`
`360`	`357`	`public async Task Can_update_resource_with_omitted_required_attribute_value()`
`361`	`358`	`{`
`362`	`359`	`// Arrange`
`363`		`- SystemDirectory existingDirectory = _fakers.SystemDirectory.Generate();`
	`360`	`+ SystemFile existingFile = _fakers.SystemFile.Generate();`
`364`	`361`
`365`		`- long newSizeInBytes = _fakers.SystemDirectory.Generate().SizeInBytes;`
	`362`	`+ long? newSizeInBytes = _fakers.SystemFile.Generate().SizeInBytes;`
`366`	`363`
`367`	`364`	`await _testContext.RunOnDatabaseAsync(async dbContext =>`
`368`	`365`	`{`
`369`		`- dbContext.Directories.Add(existingDirectory);`
	`366`	`+ dbContext.Files.Add(existingFile);`
`370`	`367`	`await dbContext.SaveChangesAsync();`
`371`	`368`	`});`
`372`	`369`
`373`	`370`	`var requestBody = new`
`374`	`371`	`{`
`375`	`372`	`data = new`
`376`	`373`	`{`
`377`		`- type = "systemDirectories",`
`378`		`- id = existingDirectory.StringId,`
	`374`	`+ type = "systemFiles",`
	`375`	`+ id = existingFile.StringId,`
`379`	`376`	`attributes = new`
`380`	`377`	`{`
`381`	`378`	`sizeInBytes = newSizeInBytes`
`382`	`379`	`}`
`383`	`380`	`}`
`384`	`381`	`};`
`385`	`382`
`386`		`- string route = $"/systemDirectories/{existingDirectory.StringId}";`
	`383`	`+ string route = $"/systemFiles/{existingFile.StringId}";`
`387`	`384`
`388`	`385`	`// Act`
`389`	`386`	`(HttpResponseMessage httpResponse, string responseDocument) = await _testContext.ExecutePatchAsync<string>(route, requestBody);`