From 5554428aaff5ab64faad46040f631938b7032264 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 28 Jan 2022 13:20:03 -0800 Subject: [PATCH 1/7] Inferring FromServices --- src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs | 13 +++++++++++++ .../ApiBehaviorApplicationModelProvider.cs | 11 ++++++++--- .../InferParameterBindingInfoConvention.cs | 12 +++++++++++- src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt | 1 - src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt | 3 +++ .../ApiBehaviorApplicationModelProviderTest.cs | 5 +++-- .../InferParameterBindingInfoConventionTest.cs | 12 ++++++++---- 7 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs b/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs index 252345fe112b..467778f1ac86 100644 --- a/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs +++ b/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs @@ -3,8 +3,10 @@ using System.Collections; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.DependencyInjection; namespace Microsoft.AspNetCore.Mvc; @@ -39,12 +41,23 @@ public Func InvalidModelStateResponseFactory /// When enabled, the following sources are inferred: /// Parameters that appear as route values, are assumed to be bound from the path (). /// Parameters of type and are assumed to be bound from form. + /// Parameters that are complex () and are registered in the DI Container () are assumed to be bound from the services , unless this + /// option is explicitly disabled . /// Parameters that are complex () are assumed to be bound from the body (). /// All other parameters are assumed to be bound from the query. /// /// public bool SuppressInferBindingSourcesForParameters { get; set; } + /// + /// When , determines if a action parameter will be injected from the DI container. + /// Parameters can be explicitly marked with an attribute that implements with or without this option set. + /// + /// + /// False by default.action method arguments will be resolved from a DI container if possible. + /// + public bool DisableImplicitFromServiceParameters { get; set; } + /// /// Gets or sets a value that determines if an multipart/form-data consumes action constraint is added to parameters /// that are bound from form data. diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs index 496187323817..b576a7460429 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Linq; @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -17,7 +18,8 @@ public ApiBehaviorApplicationModelProvider( IOptions apiBehaviorOptions, IModelMetadataProvider modelMetadataProvider, IClientErrorFactory clientErrorFactory, - ILoggerFactory loggerFactory) + ILoggerFactory loggerFactory, + IServiceProvider serviceProvider) { var options = apiBehaviorOptions.Value; @@ -47,7 +49,10 @@ public ApiBehaviorApplicationModelProvider( if (!options.SuppressInferBindingSourcesForParameters) { - ActionModelConventions.Add(new InferParameterBindingInfoConvention(modelMetadataProvider)); + var convention = options.DisableImplicitFromServiceParameters ? + new InferParameterBindingInfoConvention(modelMetadataProvider) : + new InferParameterBindingInfoConvention(modelMetadataProvider, serviceProvider.GetService()); + ActionModelConventions.Add(convention); } } diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index fa8c9fd2245b..3ba9315ae5f0 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Routing.Template; using Resources = Microsoft.AspNetCore.Mvc.Core.Resources; +using Microsoft.Extensions.DependencyInjection; namespace Microsoft.AspNetCore.Mvc.ApplicationModels; @@ -23,15 +24,19 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels; public class InferParameterBindingInfoConvention : IActionModelConvention { private readonly IModelMetadataProvider _modelMetadataProvider; + private readonly IServiceProviderIsService? _serviceProviderIsService; /// /// Initializes a new instance of . /// /// The model metadata provider. + /// The service to determine if the a type is available from the . public InferParameterBindingInfoConvention( - IModelMetadataProvider modelMetadataProvider) + IModelMetadataProvider modelMetadataProvider, + IServiceProviderIsService? serviceProviderIsService = null) { _modelMetadataProvider = modelMetadataProvider ?? throw new ArgumentNullException(nameof(modelMetadataProvider)); + _serviceProviderIsService = serviceProviderIsService; } /// @@ -95,6 +100,11 @@ internal BindingSource InferBindingSourceForParameter(ParameterModel parameter) { if (IsComplexTypeParameter(parameter)) { + if (_serviceProviderIsService?.IsService(parameter.ParameterType) == true) + { + return BindingSource.Services; + } + return BindingSource.Body; } diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt index 6db0756253e8..230dcfa930ad 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt @@ -301,7 +301,6 @@ Microsoft.AspNetCore.Mvc.ApplicationModels.IFilterModel Microsoft.AspNetCore.Mvc.ApplicationModels.IFilterModel.Filters.get -> System.Collections.Generic.IList! Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention.Apply(Microsoft.AspNetCore.Mvc.ApplicationModels.ActionModel! action) -> void -Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention.InferParameterBindingInfoConvention(Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider) -> void Microsoft.AspNetCore.Mvc.ApplicationModels.InvalidModelStateFilterConvention Microsoft.AspNetCore.Mvc.ApplicationModels.InvalidModelStateFilterConvention.Apply(Microsoft.AspNetCore.Mvc.ApplicationModels.ActionModel! action) -> void Microsoft.AspNetCore.Mvc.ApplicationModels.InvalidModelStateFilterConvention.InvalidModelStateFilterConvention() -> void diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt index d326535ca4d1..9a9ed1c93dcf 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt @@ -1,4 +1,7 @@ #nullable enable +Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServiceParameters.get -> bool +Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServiceParameters.set -> void +Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention.InferParameterBindingInfoConvention(Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider, Microsoft.Extensions.DependencyInjection.IServiceProviderIsService? serviceProviderIsService = null) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateDisplayMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.DisplayMetadataProviderContext! context) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateValidationMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadataProviderContext! context) -> void diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs index 34d9196d0c32..3c045e225199 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Reflection; @@ -163,7 +163,8 @@ private static ApiBehaviorApplicationModelProvider GetProvider( optionsAccessor, new EmptyModelMetadataProvider(), Mock.Of(), - loggerFactory); + loggerFactory, + Mock.Of()); } private class TestApiController : ControllerBase diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs index 70832d7ec47e..96894d3f5e6d 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.ComponentModel; @@ -6,7 +6,9 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Binders; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using Moq; namespace Microsoft.AspNetCore.Mvc.ApplicationModels; @@ -732,10 +734,12 @@ public void PreservesBindingSourceInference_ForParameterWithRequestPredicateAndP } private static InferParameterBindingInfoConvention GetConvention( - IModelMetadataProvider modelMetadataProvider = null) + IModelMetadataProvider modelMetadataProvider = null, + IServiceProviderIsService serviceProviderIsService = null) { modelMetadataProvider = modelMetadataProvider ?? new EmptyModelMetadataProvider(); - return new InferParameterBindingInfoConvention(modelMetadataProvider); + serviceProviderIsService = serviceProviderIsService ?? Mock.Of(s => s.IsService(It.IsAny()) == false); + return new InferParameterBindingInfoConvention(modelMetadataProvider, serviceProviderIsService); } private static ApplicationModelProviderContext GetContext( @@ -804,7 +808,7 @@ private class ParameterBindingController public IActionResult ComplexTypeModel(TestModel model) => null; [HttpPut("put-action/{id}")] - public IActionResult SimpleTypeModel(ConvertibleFromString model) => null; + public IActionResult SimpleTypeModel(ConvertibleFromString model) => null [HttpPost("form-file")] public IActionResult FormFileParameter(IFormFile formFile) => null; From 2ea927de2099d9c3dc15c282671c9c3efaeb1f42 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 1 Feb 2022 13:25:14 -0800 Subject: [PATCH 2/7] Adding unit tests --- .../InferParameterBindingInfoConvention.cs | 2 ++ ...ApiBehaviorApplicationModelProviderTest.cs | 11 ++++++++++ ...InferParameterBindingInfoConventionTest.cs | 22 ++++++++++++++++++- .../Mvc.FunctionalTests/ApiBehaviorTest.cs | 15 +++++++++++++ .../Controllers/ContactApiController.cs | 3 +++ 5 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index 3ba9315ae5f0..b710fb795001 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -39,6 +39,8 @@ public InferParameterBindingInfoConvention( _serviceProviderIsService = serviceProviderIsService; } + internal bool IsInferForServiceParametersEnabled => _serviceProviderIsService != null; + /// /// Called to determine whether the action should apply. /// diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs index 3c045e225199..b6db51e0084e 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs @@ -138,6 +138,17 @@ public void Constructor_DoesNotAddInferParameterBindingInfoConvention_IfSuppress Assert.Empty(provider.ActionModelConventions.OfType()); } + [Fact] + public void Constructor_DoesNotInferServicesParameterBindingInfoConvention_IfSuppressInferBindingSourcesForParametersIsSet() + { + // Arrange + var provider = GetProvider(new ApiBehaviorOptions { DisableImplicitFromServiceParameters = true }); + + // Act & Assert + var convention = (InferParameterBindingInfoConvention)Assert.Single(provider.ActionModelConventions, c => c is InferParameterBindingInfoConvention); + Assert.False(convention.IsInferForServiceParametersEnabled); + } + [Fact] public void Constructor_DoesNotSpecifyDefaultErrorType_IfSuppressMapClientErrorsIsSet() { diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs index 96894d3f5e6d..de561eed5c29 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs @@ -479,6 +479,24 @@ public void InferBindingSourceForParameter_ReturnsBodyForCollectionOfComplexType Assert.Same(BindingSource.Body, result); } + [Fact] + public void InferBindingSourceForParameter_ReturnsServicesForComplexTypesRegisteredInDI() + { + // Arrange + var actionName = nameof(ParameterBindingController.ServiceParameter); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + // Using any built-in type defined in the Test action + var serviceProvider = Mock.Of(s => s.IsService(typeof(IApplicationModelProvider)) == true); + var convention = GetConvention(serviceProviderIsService: serviceProvider); + + // Act + var result = convention.InferBindingSourceForParameter(parameter); + + // Assert + Assert.True(convention.IsInferForServiceParametersEnabled); + Assert.Same(BindingSource.Services, result); + } + [Fact] public void PreservesBindingSourceInference_ForFromQueryParameter_WithDefaultName() { @@ -808,7 +826,7 @@ private class ParameterBindingController public IActionResult ComplexTypeModel(TestModel model) => null; [HttpPut("put-action/{id}")] - public IActionResult SimpleTypeModel(ConvertibleFromString model) => null + public IActionResult SimpleTypeModel(ConvertibleFromString model) => null; [HttpPost("form-file")] public IActionResult FormFileParameter(IFormFile formFile) => null; @@ -875,6 +893,8 @@ public IActionResult SimpleTypeModel(ConvertibleFromString model) => null public IActionResult CollectionOfSimpleTypes(IList parameter) => null; public IActionResult CollectionOfComplexTypes(IList parameter) => null; + + public IActionResult ServiceParameter(IApplicationModelProvider parameter) => null; } [ApiController] diff --git a/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs b/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs index db20a20dcdd4..98c6cb955587 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs @@ -145,6 +145,21 @@ private async Task ActionsWithApiBehaviorInferFromBodyParameters(string action) Assert.Equal(input.Name, result.Name); } + [Fact] + public async Task ActionsWithApiBehavior_InferFromServicesParameters() + { + // Arrange + var id = 1; + var url = $"/contact/ActionWithInferredFromServicesParameter/{id}"; + var response = await Client.GetAsync(url); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + var result = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); + Assert.NotNull(result); + Assert.Equal(id, result.ContactId); + } + [Fact] public async Task ActionsWithApiBehavior_InferQueryAndRouteParameters() { diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs index 8979663f44d0..2ed6a57d9037 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs @@ -83,6 +83,9 @@ public ActionResult ActionWithInferredModelBinderTypeWithExplicitModelNa return foo; } + [HttpGet("[action]/{id}")] + public ActionResult ActionWithInferredFromServicesParameter(int id, ContactsRepository repository) => repository.GetContact(id) ?? new Contact() { ContactId = id }; + [HttpGet("[action]")] public ActionResult ActionReturningStatusCodeResult() { From 60a3ea7a03fec082e82653d4c720f71c02081e49 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 2 Feb 2022 10:23:27 -0800 Subject: [PATCH 3/7] Removing API change --- .../InferParameterBindingInfoConvention.cs | 12 +++++++++++- src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt | 1 + src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt | 1 - 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index b710fb795001..ed1aa0fa1caa 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -31,7 +31,7 @@ public class InferParameterBindingInfoConvention : IActionModelConvention /// /// The model metadata provider. /// The service to determine if the a type is available from the . - public InferParameterBindingInfoConvention( + internal InferParameterBindingInfoConvention( IModelMetadataProvider modelMetadataProvider, IServiceProviderIsService? serviceProviderIsService = null) { @@ -39,6 +39,16 @@ public InferParameterBindingInfoConvention( _serviceProviderIsService = serviceProviderIsService; } + /// + /// Initializes a new instance of . + /// + /// The model metadata provider. + public InferParameterBindingInfoConvention( + IModelMetadataProvider modelMetadataProvider) + { + _modelMetadataProvider = modelMetadataProvider ?? throw new ArgumentNullException(nameof(modelMetadataProvider)); + } + internal bool IsInferForServiceParametersEnabled => _serviceProviderIsService != null; /// diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt index 230dcfa930ad..6db0756253e8 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt @@ -301,6 +301,7 @@ Microsoft.AspNetCore.Mvc.ApplicationModels.IFilterModel Microsoft.AspNetCore.Mvc.ApplicationModels.IFilterModel.Filters.get -> System.Collections.Generic.IList! Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention.Apply(Microsoft.AspNetCore.Mvc.ApplicationModels.ActionModel! action) -> void +Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention.InferParameterBindingInfoConvention(Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider) -> void Microsoft.AspNetCore.Mvc.ApplicationModels.InvalidModelStateFilterConvention Microsoft.AspNetCore.Mvc.ApplicationModels.InvalidModelStateFilterConvention.Apply(Microsoft.AspNetCore.Mvc.ApplicationModels.ActionModel! action) -> void Microsoft.AspNetCore.Mvc.ApplicationModels.InvalidModelStateFilterConvention.InvalidModelStateFilterConvention() -> void diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt index 9a9ed1c93dcf..e6420a9158d0 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt @@ -1,7 +1,6 @@ #nullable enable Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServiceParameters.get -> bool Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServiceParameters.set -> void -Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention.InferParameterBindingInfoConvention(Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider, Microsoft.Extensions.DependencyInjection.IServiceProviderIsService? serviceProviderIsService = null) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateDisplayMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.DisplayMetadataProviderContext! context) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateValidationMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadataProviderContext! context) -> void From 7d89db95291dedf2c9c839360ca59664c9ebcdb0 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 7 Feb 2022 14:05:40 -0800 Subject: [PATCH 4/7] API Review feedback --- src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs | 11 ++++------- .../ApiBehaviorApplicationModelProvider.cs | 7 ++++--- .../InferParameterBindingInfoConvention.cs | 18 ++++++++++-------- src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt | 5 +++-- .../ApiBehaviorApplicationModelProviderTest.cs | 2 +- 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs b/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs index 467778f1ac86..79fd2ae15793 100644 --- a/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs +++ b/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs @@ -42,7 +42,7 @@ public Func InvalidModelStateResponseFactory /// Parameters that appear as route values, are assumed to be bound from the path (). /// Parameters of type and are assumed to be bound from form. /// Parameters that are complex () and are registered in the DI Container () are assumed to be bound from the services , unless this - /// option is explicitly disabled . + /// option is explicitly disabled . /// Parameters that are complex () are assumed to be bound from the body (). /// All other parameters are assumed to be bound from the query. /// @@ -50,13 +50,10 @@ public Func InvalidModelStateResponseFactory public bool SuppressInferBindingSourcesForParameters { get; set; } /// - /// When , determines if a action parameter will be injected from the DI container. - /// Parameters can be explicitly marked with an attribute that implements with or without this option set. + /// Gets or sets a value that determines if parameters are inferred to be from services. + /// This property is only applicable when is . /// - /// - /// False by default.action method arguments will be resolved from a DI container if possible. - /// - public bool DisableImplicitFromServiceParameters { get; set; } + public bool DisableImplicitFromServicesParameters { get; set; } /// /// Gets or sets a value that determines if an multipart/form-data consumes action constraint is added to parameters diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs index b576a7460429..f127b1699395 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs @@ -49,9 +49,10 @@ public ApiBehaviorApplicationModelProvider( if (!options.SuppressInferBindingSourcesForParameters) { - var convention = options.DisableImplicitFromServiceParameters ? - new InferParameterBindingInfoConvention(modelMetadataProvider) : - new InferParameterBindingInfoConvention(modelMetadataProvider, serviceProvider.GetService()); + var serviceProviderIsService = serviceProvider.GetService(); + var convention = options.DisableImplicitFromServicesParameters || serviceProviderIsService == null ? + new InferParameterBindingInfoConvention(modelMetadataProvider) : + new InferParameterBindingInfoConvention(modelMetadataProvider, serviceProviderIsService); ActionModelConventions.Add(convention); } } diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index ed1aa0fa1caa..bf9a2081f143 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -16,7 +16,8 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels; /// The goal of this convention is to make intuitive and easy to document inferences. The rules are: /// /// A previously specified is never overwritten. -/// A complex type parameter () is assigned . +/// A complex type parameter (), registered in the DI container, is assigned . +/// A complex type parameter (), not registered in the DI container, is assigned . /// Parameter with a name that appears as a route value in ANY route template is assigned . /// All other parameters are . /// @@ -30,25 +31,26 @@ public class InferParameterBindingInfoConvention : IActionModelConvention /// Initializes a new instance of . /// /// The model metadata provider. - /// The service to determine if the a type is available from the . - internal InferParameterBindingInfoConvention( - IModelMetadataProvider modelMetadataProvider, - IServiceProviderIsService? serviceProviderIsService = null) + public InferParameterBindingInfoConvention( + IModelMetadataProvider modelMetadataProvider) { _modelMetadataProvider = modelMetadataProvider ?? throw new ArgumentNullException(nameof(modelMetadataProvider)); - _serviceProviderIsService = serviceProviderIsService; } /// /// Initializes a new instance of . /// /// The model metadata provider. + /// The service to determine if the a type is available from the . public InferParameterBindingInfoConvention( - IModelMetadataProvider modelMetadataProvider) + IModelMetadataProvider modelMetadataProvider, + IServiceProviderIsService serviceProviderIsService) + : this(modelMetadataProvider) { - _modelMetadataProvider = modelMetadataProvider ?? throw new ArgumentNullException(nameof(modelMetadataProvider)); + _serviceProviderIsService = serviceProviderIsService ?? throw new ArgumentNullException(nameof(serviceProviderIsService)); } + internal bool IsInferForServiceParametersEnabled => _serviceProviderIsService != null; /// diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt index e6420a9158d0..2c04e5427635 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt @@ -1,6 +1,7 @@ #nullable enable -Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServiceParameters.get -> bool -Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServiceParameters.set -> void +Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServicesParameters.get -> bool +Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServicesParameters.set -> void +Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention.InferParameterBindingInfoConvention(Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider, Microsoft.Extensions.DependencyInjection.IServiceProviderIsService! serviceProviderIsService) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateDisplayMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.DisplayMetadataProviderContext! context) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateValidationMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadataProviderContext! context) -> void diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs index b6db51e0084e..224198672ee1 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs @@ -142,7 +142,7 @@ public void Constructor_DoesNotAddInferParameterBindingInfoConvention_IfSuppress public void Constructor_DoesNotInferServicesParameterBindingInfoConvention_IfSuppressInferBindingSourcesForParametersIsSet() { // Arrange - var provider = GetProvider(new ApiBehaviorOptions { DisableImplicitFromServiceParameters = true }); + var provider = GetProvider(new ApiBehaviorOptions { DisableImplicitFromServicesParameters = true }); // Act & Assert var convention = (InferParameterBindingInfoConvention)Assert.Single(provider.ActionModelConventions, c => c is InferParameterBindingInfoConvention); From 7549c8d364e0e23805022fa8d22a863152ef0132 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 7 Feb 2022 14:07:08 -0800 Subject: [PATCH 5/7] Remove empty line --- .../src/ApplicationModels/InferParameterBindingInfoConvention.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index bf9a2081f143..ab1e73960dbf 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -50,7 +50,6 @@ public InferParameterBindingInfoConvention( _serviceProviderIsService = serviceProviderIsService ?? throw new ArgumentNullException(nameof(serviceProviderIsService)); } - internal bool IsInferForServiceParametersEnabled => _serviceProviderIsService != null; /// From 368e50e086d0a750205558b2d1cecf24c3ca7ae1 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 7 Feb 2022 14:25:28 -0800 Subject: [PATCH 6/7] nit: feeback --- .../ApplicationModels/ApiBehaviorApplicationModelProvider.cs | 2 +- .../ApplicationModels/InferParameterBindingInfoConvention.cs | 4 ++-- .../WebSites/BasicWebSite/Controllers/ContactApiController.cs | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs index f127b1699395..1872faebf0c6 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs @@ -50,7 +50,7 @@ public ApiBehaviorApplicationModelProvider( if (!options.SuppressInferBindingSourcesForParameters) { var serviceProviderIsService = serviceProvider.GetService(); - var convention = options.DisableImplicitFromServicesParameters || serviceProviderIsService == null ? + var convention = options.DisableImplicitFromServicesParameters || serviceProviderIsService is null ? new InferParameterBindingInfoConvention(modelMetadataProvider) : new InferParameterBindingInfoConvention(modelMetadataProvider, serviceProviderIsService); ActionModelConventions.Add(convention); diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index ab1e73960dbf..11258a03b138 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -4,8 +4,8 @@ using System.Linq; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Routing.Template; -using Resources = Microsoft.AspNetCore.Mvc.Core.Resources; using Microsoft.Extensions.DependencyInjection; +using Resources = Microsoft.AspNetCore.Mvc.Core.Resources; namespace Microsoft.AspNetCore.Mvc.ApplicationModels; @@ -113,7 +113,7 @@ internal BindingSource InferBindingSourceForParameter(ParameterModel parameter) { if (IsComplexTypeParameter(parameter)) { - if (_serviceProviderIsService?.IsService(parameter.ParameterType) == true) + if (_serviceProviderIsService?.IsService(parameter.ParameterType) is true) { return BindingSource.Services; } diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs index 2ed6a57d9037..18cdc433222c 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs @@ -84,7 +84,8 @@ public ActionResult ActionWithInferredModelBinderTypeWithExplicitModelNa } [HttpGet("[action]/{id}")] - public ActionResult ActionWithInferredFromServicesParameter(int id, ContactsRepository repository) => repository.GetContact(id) ?? new Contact() { ContactId = id }; + public ActionResult ActionWithInferredFromServicesParameter(int id, ContactsRepository repository) + => repository.GetContact(id) ?? new Contact() { ContactId = id }; [HttpGet("[action]")] public ActionResult ActionReturningStatusCodeResult() From 24614056f6ac2538b4fa795818f7f86af400afd2 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 7 Feb 2022 14:27:43 -0800 Subject: [PATCH 7/7] Clean up usings --- src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs b/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs index 79fd2ae15793..8b18b7928d7f 100644 --- a/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs +++ b/src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs @@ -3,7 +3,6 @@ using System.Collections; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.Extensions.DependencyInjection;