diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs index d27f38d104..a693accb8b 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs @@ -31,6 +31,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding private int? _hashCode; private IReadOnlyList? _boundProperties; private IReadOnlyDictionary? _parameterMapping; + private IReadOnlyDictionary? _boundConstructorPropertyMapping; private Exception? _recordTypeValidatorsOnPropertiesError; private bool _recordTypeConstructorDetailsCalculated; @@ -153,6 +154,21 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } } + /// + /// A mapping from properties to their corresponding constructor parameter on a record type. + /// This is the inverse mapping of . + /// + internal IReadOnlyDictionary BoundConstructorPropertyMapping + { + get + { + Debug.Assert(BoundConstructor != null, "This API can be only called for types with bound constructors."); + CalculateRecordTypeConstructorDetails(); + + return _boundConstructorPropertyMapping; + } + } + /// /// Gets instance for a constructor of a record type that is used during binding and validation. /// @@ -492,18 +508,19 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } } - [MemberNotNull(nameof(_parameterMapping))] + [MemberNotNull(nameof(_parameterMapping), nameof(_boundConstructorPropertyMapping))] private void CalculateRecordTypeConstructorDetails() { if (_recordTypeConstructorDetailsCalculated) { Debug.Assert(_parameterMapping != null); + Debug.Assert(_boundConstructorPropertyMapping != null); return; } - var boundParameters = BoundConstructor!.BoundConstructorParameters!; var parameterMapping = new Dictionary(); + var propertyMapping = new Dictionary(); foreach (var parameter in boundParameters) { @@ -514,6 +531,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding if (property != null) { parameterMapping[parameter] = property; + propertyMapping[property] = parameter; if (property.PropertyHasValidators) { @@ -529,6 +547,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding _recordTypeConstructorDetailsCalculated = true; _parameterMapping = parameterMapping; + _boundConstructorPropertyMapping = propertyMapping; } /// diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Validation/ClientValidatorCache.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Validation/ClientValidatorCache.cs index 2697daec71..41faf4d8a3 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Validation/ClientValidatorCache.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Validation/ClientValidatorCache.cs @@ -1,10 +1,11 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation { @@ -14,6 +15,15 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation public IReadOnlyList GetValidators(ModelMetadata metadata, IClientModelValidatorProvider validatorProvider) { + if (metadata.MetadataKind == ModelMetadataKind.Property && + metadata.ContainerMetadata?.BoundConstructor != null && + metadata.ContainerMetadata.BoundConstructorPropertyMapping.TryGetValue(metadata, out var parameter)) + { + // "metadata" typically points to properties. When working with record types, we want to read validation details from the + // constructor parameter instead. So let's switch it out. + metadata = parameter; + } + if (_cacheEntries.TryGetValue(metadata, out var entry)) { return GetValidatorsFromEntry(entry, metadata, validatorProvider); @@ -107,7 +117,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation var validators = new IClientModelValidator[count]; var clientValidatorIndex = 0; - for (int i = 0; i < items.Count; i++) + for (var i = 0; i < items.Count; i++) { var validator = items[i].Validator; if (validator != null) diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Validation/ClientValidatorCacheTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Validation/ClientValidatorCacheTest.cs index bb9c6f5c0c..7d4a8aeb38 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Validation/ClientValidatorCacheTest.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Validation/ClientValidatorCacheTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.ComponentModel.DataAnnotations; @@ -64,6 +64,47 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation Assert.NotSame(validator2, Assert.Single(validators2.OfType())); // not cached } + [Fact] + public void GetValidators_ReadsValidatorsFromCorrespondingRecordTypeParameter() + { + // Arrange + var cache = new ClientValidatorCache(); + var modelMetadataProvider = new TestModelMetadataProvider(); + var metadata = modelMetadataProvider.GetMetadataForType(typeof(TestRecordType)); + var property = metadata.Properties[nameof(TestRecordType.Property1)]; + var parameter = metadata.BoundConstructor.BoundConstructorParameters.First(f => f.Name == nameof(TestRecordType.Property1)); + var validatorProvider = new ProviderWithNonReusableValidators(); + + // Act + var validators = cache.GetValidators(property, validatorProvider); + + // Assert + var validator1 = Assert.Single(validators.OfType()); + var validator2 = Assert.Single(validators.OfType()); + Assert.Contains(validator1.Attribute, parameter.ValidatorMetadata); // Copied by provider + Assert.Contains(validator2.Attribute, parameter.ValidatorMetadata); // Copied by provider + } + + [Fact] + public void GetValidators_ReadsValidatorsFromProperty_IfRecordTypeDoesNotHaveCorrespondingParameter() + { + // Arrange + var cache = new ClientValidatorCache(); + var modelMetadataProvider = new TestModelMetadataProvider(); + var metadata = modelMetadataProvider.GetMetadataForType(typeof(TestRecordTypeWithProperty)); + var property = metadata.Properties[nameof(TestRecordTypeWithProperty.Property2)]; + var validatorProvider = new ProviderWithNonReusableValidators(); + + // Act + var validators = cache.GetValidators(property, validatorProvider); + + // Assert + var validator1 = Assert.Single(validators.OfType()); + var validator2 = Assert.Single(validators.OfType()); + Assert.Contains(validator1.Attribute, property.ValidatorMetadata); // Copied by provider + Assert.Contains(validator2.Attribute, property.ValidatorMetadata); // Copied by provider + } + private class TypeWithProperty { [Required] @@ -71,6 +112,15 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation public string Property1 { get; set; } } + private record TestRecordType([Required][StringLength(10)] string Property1); + + private record TestRecordTypeWithProperty([Required][StringLength(10)] string Property1) + { + [Required] + [StringLength(10)] + public string Property2 { get; set; } + } + private class ProviderWithNonReusableValidators : IClientModelValidatorProvider { public void CreateValidators(ClientValidatorProviderContext context) @@ -101,4 +151,4 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation } } } -} \ No newline at end of file +} diff --git a/src/Mvc/test/Mvc.FunctionalTests/HtmlGenerationTest.cs b/src/Mvc/test/Mvc.FunctionalTests/HtmlGenerationTest.cs index 98416f442a..dfaaa2fb5f 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/HtmlGenerationTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/HtmlGenerationTest.cs @@ -299,6 +299,32 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests #endif } + [Fact] + public async Task ClientValidators_AreGeneratedDuringInitialRender() + { + // Arrange + var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/Customer/HtmlGeneration_Customer/CustomerWithRecords"); + + // Act + var response = await Client.SendAsync(request); + + // Assert + var document = await response.GetHtmlDocumentAsync(); + + var numberInput = document.RequiredQuerySelector("input[id=Number]"); + Assert.Equal("true", numberInput.GetAttribute("data-val")); + Assert.Equal("The field Number must be between 1 and 100.", numberInput.GetAttribute("data-val-range")); + Assert.Equal("The Number field is required.", numberInput.GetAttribute("data-val-required")); + + var passwordInput = document.RequiredQuerySelector("input[id=Password]"); + Assert.Equal("true", passwordInput.GetAttribute("data-val")); + Assert.Equal("The Password field is required.", passwordInput.GetAttribute("data-val-required")); + + var addressInput = document.RequiredQuerySelector("input[id=Address]"); + Assert.Equal("true", addressInput.GetAttribute("data-val")); + Assert.Equal("The Address field is required.", addressInput.GetAttribute("data-val-required")); + } + [Fact] public async Task ValidationTagHelpers_UsingRecords() { @@ -310,7 +336,8 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests new KeyValuePair("Name", string.Empty), new KeyValuePair("Email", string.Empty), new KeyValuePair("PhoneNumber", string.Empty), - new KeyValuePair("Password", string.Empty) + new KeyValuePair("Password", string.Empty), + new KeyValuePair("Address", string.Empty), }; request.Content = new FormUrlEncodedContent(nameValueCollection); @@ -331,6 +358,9 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests validation = document.QuerySelector("span[data-valmsg-for=Password]"); Assert.Equal("The Password field is required.", validation.TextContent); + + validation = document.QuerySelector("span[data-valmsg-for=Address]"); + Assert.Equal("The Address field is required.", validation.TextContent); } [Fact] diff --git a/src/Mvc/test/WebSites/HtmlGenerationWebSite/Areas/Customer/Controllers/HtmlGeneration_CustomerController.cs b/src/Mvc/test/WebSites/HtmlGenerationWebSite/Areas/Customer/Controllers/HtmlGeneration_CustomerController.cs index e79741b381..b1d021a62a 100644 --- a/src/Mvc/test/WebSites/HtmlGenerationWebSite/Areas/Customer/Controllers/HtmlGeneration_CustomerController.cs +++ b/src/Mvc/test/WebSites/HtmlGenerationWebSite/Areas/Customer/Controllers/HtmlGeneration_CustomerController.cs @@ -13,9 +13,15 @@ namespace HtmlGenerationWebSite.Areas.Customer.Controllers return View("Customer"); } + [HttpGet] + public IActionResult CustomerWithRecords() + { + return View("CustomerWithRecords"); + } + public IActionResult CustomerWithRecords(Models.CustomerRecord customer) { return View("CustomerWithRecords"); } } -} \ No newline at end of file +} diff --git a/src/Mvc/test/WebSites/HtmlGenerationWebSite/Models/CustomerRecord.cs b/src/Mvc/test/WebSites/HtmlGenerationWebSite/Models/CustomerRecord.cs index 3e2c6df9e9..d131f08484 100644 --- a/src/Mvc/test/WebSites/HtmlGenerationWebSite/Models/CustomerRecord.cs +++ b/src/Mvc/test/WebSites/HtmlGenerationWebSite/Models/CustomerRecord.cs @@ -25,5 +25,9 @@ namespace HtmlGenerationWebSite.Models string Email, string Key - ); -} \ No newline at end of file + ) + { + [Required] + public string Address { get; set; } + } +} diff --git a/src/Mvc/test/WebSites/HtmlGenerationWebSite/Views/Shared/CustomerWithRecords.cshtml b/src/Mvc/test/WebSites/HtmlGenerationWebSite/Views/Shared/CustomerWithRecords.cshtml index 13dddfc081..1edd936847 100644 --- a/src/Mvc/test/WebSites/HtmlGenerationWebSite/Views/Shared/CustomerWithRecords.cshtml +++ b/src/Mvc/test/WebSites/HtmlGenerationWebSite/Views/Shared/CustomerWithRecords.cshtml @@ -37,10 +37,15 @@ Male Female + +
+ + +
- \ No newline at end of file + diff --git a/src/Tools/dotnet-watch/test/BrowserLaunchTests.cs b/src/Tools/dotnet-watch/test/BrowserLaunchTests.cs index 77e9a65da6..aa13d6de60 100644 --- a/src/Tools/dotnet-watch/test/BrowserLaunchTests.cs +++ b/src/Tools/dotnet-watch/test/BrowserLaunchTests.cs @@ -9,7 +9,7 @@ using Xunit.Abstractions; namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests { - public class BrowserLaunchTests + public class BrowserLaunchTests : IDisposable { private readonly WatchableApp _app; @@ -64,5 +64,10 @@ namespace Microsoft.DotNet.Watcher.Tools.FunctionalTests // Verify we launched the browser. await _app.Process.GetOutputLineStartsWithAsync(launchBrowserMessage, TimeSpan.FromMinutes(2)); } + + public void Dispose() + { + _app.Dispose(); + } } }