Skip to content

Adding support to infer FromServices parameters #39926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Mvc/Mvc.Core/src/ApiBehaviorOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Infrastructure;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.Extensions.DependencyInjection;

namespace Microsoft.AspNetCore.Mvc;

Expand Down Expand Up @@ -39,12 +40,20 @@ public Func<ActionContext, IActionResult> InvalidModelStateResponseFactory
/// When enabled, the following sources are inferred:
/// Parameters that appear as route values, are assumed to be bound from the path (<see cref="BindingSource.Path"/>).
/// Parameters of type <see cref="IFormFile"/> and <see cref="IFormFileCollection"/> are assumed to be bound from form.
/// Parameters that are complex (<see cref="ModelMetadata.IsComplexType"/>) and are registered in the DI Container (<see cref="IServiceCollection"/>) are assumed to be bound from the services <see cref="BindingSource.Services"/>, unless this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the precedence order the same in minimal i.e is IServiceProviderIsService only used for complex types?

/// option is explicitly disabled <see cref="DisableImplicitFromServicesParameters"/>.
/// Parameters that are complex (<see cref="ModelMetadata.IsComplexType"/>) are assumed to be bound from the body (<see cref="BindingSource.Body"/>).
/// All other parameters are assumed to be bound from the query.
/// </para>
/// </summary>
public bool SuppressInferBindingSourcesForParameters { get; set; }

/// <summary>
/// Gets or sets a value that determines if parameters are inferred to be from services.
/// This property is only applicable when <see cref="SuppressInferBindingSourcesForParameters" /> is <see langword="false" />.
/// </summary>
public bool DisableImplicitFromServicesParameters { get; set; }

/// <summary>
/// Gets or sets a value that determines if an <c>multipart/form-data</c> consumes action constraint is added to parameters
/// that are bound from form data.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
// 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;
using System.Reflection;
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;

Expand All @@ -17,7 +18,8 @@ public ApiBehaviorApplicationModelProvider(
IOptions<ApiBehaviorOptions> apiBehaviorOptions,
IModelMetadataProvider modelMetadataProvider,
IClientErrorFactory clientErrorFactory,
ILoggerFactory loggerFactory)
ILoggerFactory loggerFactory,
IServiceProvider serviceProvider)
{
var options = apiBehaviorOptions.Value;

Expand Down Expand Up @@ -47,7 +49,11 @@ public ApiBehaviorApplicationModelProvider(

if (!options.SuppressInferBindingSourcesForParameters)
{
ActionModelConventions.Add(new InferParameterBindingInfoConvention(modelMetadataProvider));
var serviceProviderIsService = serviceProvider.GetService<IServiceProviderIsService>();
var convention = options.DisableImplicitFromServicesParameters || serviceProviderIsService is null ?
new InferParameterBindingInfoConvention(modelMetadataProvider) :
new InferParameterBindingInfoConvention(modelMetadataProvider, serviceProviderIsService);
ActionModelConventions.Add(convention);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Linq;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.AspNetCore.Routing.Template;
using Microsoft.Extensions.DependencyInjection;
using Resources = Microsoft.AspNetCore.Mvc.Core.Resources;

namespace Microsoft.AspNetCore.Mvc.ApplicationModels;
Expand All @@ -15,14 +16,16 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels;
/// The goal of this convention is to make intuitive and easy to document <see cref="BindingSource"/> inferences. The rules are:
/// <list type="number">
/// <item>A previously specified <see cref="BindingInfo.BindingSource" /> is never overwritten.</item>
/// <item>A complex type parameter (<see cref="ModelMetadata.IsComplexType"/>) is assigned <see cref="BindingSource.Body"/>.</item>
/// <item>A complex type parameter (<see cref="ModelMetadata.IsComplexType"/>), registered in the DI container, is assigned <see cref="BindingSource.Services"/>.</item>
/// <item>A complex type parameter (<see cref="ModelMetadata.IsComplexType"/>), not registered in the DI container, is assigned <see cref="BindingSource.Body"/>.</item>
/// <item>Parameter with a name that appears as a route value in ANY route template is assigned <see cref="BindingSource.Path"/>.</item>
/// <item>All other parameters are <see cref="BindingSource.Query"/>.</item>
/// </list>
/// </remarks>
public class InferParameterBindingInfoConvention : IActionModelConvention
{
private readonly IModelMetadataProvider _modelMetadataProvider;
private readonly IServiceProviderIsService? _serviceProviderIsService;

/// <summary>
/// Initializes a new instance of <see cref="InferParameterBindingInfoConvention"/>.
Expand All @@ -34,6 +37,21 @@ public InferParameterBindingInfoConvention(
_modelMetadataProvider = modelMetadataProvider ?? throw new ArgumentNullException(nameof(modelMetadataProvider));
}

/// <summary>
/// Initializes a new instance of <see cref="InferParameterBindingInfoConvention"/>.
/// </summary>
/// <param name="modelMetadataProvider">The model metadata provider.</param>
/// <param name="serviceProviderIsService">The service to determine if the a type is available from the <see cref="IServiceProvider"/>.</param>
public InferParameterBindingInfoConvention(
IModelMetadataProvider modelMetadataProvider,
IServiceProviderIsService serviceProviderIsService)
: this(modelMetadataProvider)
{
_serviceProviderIsService = serviceProviderIsService ?? throw new ArgumentNullException(nameof(serviceProviderIsService));
}

internal bool IsInferForServiceParametersEnabled => _serviceProviderIsService != null;

/// <summary>
/// Called to determine whether the action should apply.
/// </summary>
Expand Down Expand Up @@ -95,6 +113,11 @@ internal BindingSource InferBindingSourceForParameter(ParameterModel parameter)
{
if (IsComplexTypeParameter(parameter))
{
if (_serviceProviderIsService?.IsService(parameter.ParameterType) is true)
{
return BindingSource.Services;
}

return BindingSource.Body;
}

Expand Down
3 changes: 3 additions & 0 deletions src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#nullable enable
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
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -138,6 +138,17 @@ public void Constructor_DoesNotAddInferParameterBindingInfoConvention_IfSuppress
Assert.Empty(provider.ActionModelConventions.OfType<InferParameterBindingInfoConvention>());
}

[Fact]
public void Constructor_DoesNotInferServicesParameterBindingInfoConvention_IfSuppressInferBindingSourcesForParametersIsSet()
{
// Arrange
var provider = GetProvider(new ApiBehaviorOptions { DisableImplicitFromServicesParameters = true });

// Act & Assert
var convention = (InferParameterBindingInfoConvention)Assert.Single(provider.ActionModelConventions, c => c is InferParameterBindingInfoConvention);
Assert.False(convention.IsInferForServiceParametersEnabled);
}

[Fact]
public void Constructor_DoesNotSpecifyDefaultErrorType_IfSuppressMapClientErrorsIsSet()
{
Expand All @@ -163,7 +174,8 @@ private static ApiBehaviorApplicationModelProvider GetProvider(
optionsAccessor,
new EmptyModelMetadataProvider(),
Mock.Of<IClientErrorFactory>(),
loggerFactory);
loggerFactory,
Mock.Of<IServiceProvider>());
}

private class TestApiController : ControllerBase
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
// 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;
using System.Reflection;
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;

Expand Down Expand Up @@ -477,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<IServiceProviderIsService>(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()
{
Expand Down Expand Up @@ -732,10 +752,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<IServiceProviderIsService>(s => s.IsService(It.IsAny<Type>()) == false);
return new InferParameterBindingInfoConvention(modelMetadataProvider, serviceProviderIsService);
}

private static ApplicationModelProviderContext GetContext(
Expand Down Expand Up @@ -871,6 +893,8 @@ private class ParameterBindingController
public IActionResult CollectionOfSimpleTypes(IList<int> parameter) => null;

public IActionResult CollectionOfComplexTypes(IList<TestModel> parameter) => null;

public IActionResult ServiceParameter(IApplicationModelProvider parameter) => null;
}

[ApiController]
Expand Down
15 changes: 15 additions & 0 deletions src/Mvc/test/Mvc.FunctionalTests/ApiBehaviorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Contact>(await response.Content.ReadAsStringAsync());
Assert.NotNull(result);
Assert.Equal(id, result.ContactId);
}

[Fact]
public async Task ActionsWithApiBehavior_InferQueryAndRouteParameters()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ public ActionResult<string> ActionWithInferredModelBinderTypeWithExplicitModelNa
return foo;
}

[HttpGet("[action]/{id}")]
public ActionResult<Contact> ActionWithInferredFromServicesParameter(int id, ContactsRepository repository)
=> repository.GetContact(id) ?? new Contact() { ContactId = id };

[HttpGet("[action]")]
public ActionResult<int> ActionReturningStatusCodeResult()
{
Expand Down