From 09b5ff7b721f0b781d6f8acefeee335d028b7b83 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Tue, 13 Mar 2018 01:59:53 -0700 Subject: [PATCH 1/2] Use ParameterInfo for getting metadata of a parameter to show the correct information in ApiExplorer [Fixes #7435] 2.1-Preview 1: IsBindingRequired and IsRequired still false with RequiredAttribute on controller parameter. --- .../DefaultApiDescriptionProvider.cs | 18 +++++++++++- .../DefaultApiDescriptionProviderTest.cs | 29 +++++++++++++++++-- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs index 23e8bb6187..ede016465c 100644 --- a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs @@ -162,7 +162,23 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer foreach (var actionParameter in context.ActionDescriptor.Parameters) { var visitor = new PseudoModelBindingVisitor(context, actionParameter); - var metadata = _modelMetadataProvider.GetMetadataForType(actionParameter.ParameterType); + + ModelMetadata metadata = null; + if (actionParameter is ControllerParameterDescriptor controllerParameterDescriptor && + _modelMetadataProvider is ModelMetadataProvider provider) + { + // The default model metadata provider derives from ModelMetadataProvider + // and can therefore supply information about attributes applied to parameters. + metadata = provider.GetMetadataForParameter(controllerParameterDescriptor.ParameterInfo); + } + else + { + // For backward compatibility, if there's a custom model metadata provider that + // only implements the older IModelMetadataProvider interface, access the more + // limited metadata information it supplies. In this scenario, validation attributes + // are not supported on parameters. + metadata = _modelMetadataProvider.GetMetadataForType(actionParameter.ParameterType); + } var bindingContext = ApiParameterDescriptionContext.GetContext( metadata, diff --git a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs index 04a1cf9d50..8ef866008a 100644 --- a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.ComponentModel.DataAnnotations; using System.Linq; using System.Reflection; using System.Text; @@ -912,6 +913,25 @@ namespace Microsoft.AspNetCore.Mvc.Description Assert.Equal(typeof(string), parameter.Type); } + [Fact] + public void GetApiDescription_ParameterDescription_IsRequired() + { + // Arrange + var action = CreateActionDescriptor(nameof(RequiredParameter)); + + // Act + var descriptions = GetApiDescriptions(action); + + // Assert + var description = Assert.Single(descriptions); + var parameter = Assert.Single(description.ParameterDescriptions); + Assert.Equal("name", parameter.Name); + Assert.Same(BindingSource.ModelBinding, parameter.Source); + Assert.Equal(typeof(string), parameter.Type); + Assert.True(parameter.ModelMetadata.IsRequired); + Assert.True(parameter.ModelMetadata.IsBindingRequired); + } + [Fact] public void GetApiDescription_ParameterDescription_SourceFromRouteData() { @@ -1472,11 +1492,12 @@ namespace Microsoft.AspNetCore.Mvc.Description action.Parameters = new List(); foreach (var parameter in action.MethodInfo.GetParameters()) { - action.Parameters.Add(new ParameterDescriptor() + action.Parameters.Add(new ControllerParameterDescriptor() { Name = parameter.Name, ParameterType = parameter.ParameterType, - BindingInfo = BindingInfo.GetBindingInfo(parameter.GetCustomAttributes().OfType()) + BindingInfo = BindingInfo.GetBindingInfo(parameter.GetCustomAttributes().OfType()), + ParameterInfo = parameter }); } @@ -1552,6 +1573,10 @@ namespace Microsoft.AspNetCore.Mvc.Description { } + private void RequiredParameter([BindRequired, Required] string name) + { + } + private void AcceptsProduct_Body([FromBody] Product product) { } From e2b6975bffca2b85e006d37860ce1828520cd9f1 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Mon, 19 Mar 2018 11:13:56 -0700 Subject: [PATCH 2/2] Marked PageArgumentBinder type as Obsolete --- .../MvcRazorPagesMvcCoreBuilderExtensions.cs | 2 ++ .../Infrastructure/PageArgumentBinder.cs | 1 + .../Internal/DefaultPageArgumentBinder.cs | 2 ++ .../Internal/ExecutorFactoryTest.cs | 9 --------- .../PageModelTest.cs | 8 -------- 5 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs index 7b976eafcd..8d6e48bb0e 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/DependencyInjection/MvcRazorPagesMvcCoreBuilderExtensions.cs @@ -118,7 +118,9 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddSingleton(); // Page model binding +#pragma warning disable CS0618 // Type or member is obsolete services.TryAddSingleton(); +#pragma warning restore CS0618 // Type or member is obsolete // Action executors services.TryAddSingleton(); diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageArgumentBinder.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageArgumentBinder.cs index 9af8eedfd4..a4d8f5cba6 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageArgumentBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageArgumentBinder.cs @@ -7,6 +7,7 @@ using Microsoft.AspNetCore.Mvc.ModelBinding; namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure { + [Obsolete("This type is obsolete and will be removed in a future version.")] public abstract class PageArgumentBinder { public async Task BindModelAsync(PageContext context, Type type, object @default, string name) diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageArgumentBinder.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageArgumentBinder.cs index 67218d4e8a..7b3310bbdd 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageArgumentBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageArgumentBinder.cs @@ -9,7 +9,9 @@ using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal { +#pragma warning disable CS0618 // Type or member is obsolete public class DefaultPageArgumentBinder : PageArgumentBinder +#pragma warning restore CS0618 // Type or member is obsolete { private readonly ParameterBinder _parameterBinder; diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/ExecutorFactoryTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/ExecutorFactoryTest.cs index f0339d2018..a5a58227e0 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/ExecutorFactoryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/ExecutorFactoryTest.cs @@ -327,14 +327,5 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Test.Internal throw new NotImplementedException(); } } - - private class MockBinder : PageArgumentBinder - { - protected override Task BindAsync(PageContext context, object value, string name, Type type) - { - var result = ModelBindingResult.Failed(); - return Task.FromResult(result); - } - } } } diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/PageModelTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/PageModelTest.cs index 11391095e5..6d0913d394 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/PageModelTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/PageModelTest.cs @@ -1942,13 +1942,5 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages throw new NotImplementedException(); } } - - private class TestPageArgumentBinder : PageArgumentBinder - { - protected override Task BindAsync(PageContext context, object value, string name, Type type) - { - return Task.FromResult(ModelBindingResult.Success(Guid.NewGuid())); - } - } } }