Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -2456,7 +2456,7 @@ private void AcceptsProduct_Default([ModelBinder] Product product)
}

// This will show up as source = unknown
private void AcceptsProduct_Custom([ModelBinder(BinderType = typeof(BodyModelBinder))] Product product)
private void AcceptsProduct_Custom([ModelBinder<BodyModelBinder>] Product product)
{
}

Expand Down Expand Up @@ -2556,7 +2556,7 @@ private void FromModelBinding(int id)
{
}

private void FromCustom([ModelBinder(typeof(BodyModelBinder))] int id)
private void FromCustom([ModelBinder<BodyModelBinder>] int id)
{
}

Expand Down
14 changes: 14 additions & 0 deletions src/Mvc/Mvc.Core/src/Filters/MiddlewareFilterOfTAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.Mvc;

/// <inheritdoc />
/// <typeparam name="T">A type which configures a middleware pipeline.</typeparam>
public class MiddlewareFilterAttribute<T> : MiddlewareFilterAttribute
{
/// <summary>
/// Instantiates a new instance of <see cref="MiddlewareFilterAttribute"/>.
/// </summary>
public MiddlewareFilterAttribute() : base(typeof(T)) { }
}
24 changes: 24 additions & 0 deletions src/Mvc/Mvc.Core/src/ModelBinderOfTAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.AspNetCore.Mvc.ModelBinding;

namespace Microsoft.AspNetCore.Mvc;

/// <inheritdoc />
/// <typeparam name="TBinder">A <see cref="Type"/> which implements <see cref="IModelBinder"/>.</typeparam>
/// <remarks>
/// This is a derived generic variant of the <see cref="ModelBinderAttribute"/>.
/// Ensure that only one instance of either attribute is provided on the target.
/// </remarks>
public class ModelBinderAttribute<TBinder> : ModelBinderAttribute where TBinder : IModelBinder
{
/// <summary>
/// Initializes a new instance of <see cref="ModelBinderAttribute"/>.
/// </summary>
/// <remarks>
/// Subclass this attribute and set <see cref="BindingSource"/> if <see cref="BindingSource.Custom"/> is not
/// correct for the specified type parameter.
/// </remarks>
public ModelBinderAttribute() : base(typeof(TBinder)) { }
}
21 changes: 20 additions & 1 deletion src/Mvc/Mvc.Core/src/ModelBinding/Metadata/ModelAttributes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,25 @@ public static ModelAttributes GetAttributesForParameter(ParameterInfo parameterI

private static Type? GetMetadataType(Type type)
{
return type.GetCustomAttribute<ModelMetadataTypeAttribute>()?.MetadataType;
// GetCustomAttribute will examine the members inheritance chain
// for attributes of a particular type by default. Meaning that
// in the following scenario, the `ModelMetadataType` attribute on
// both the derived _and_ base class will be returned.
// [ModelMetadataType<BaseModel>]
// private class BaseViewModel { }
// [ModelMetadataType<DerivedModel>]
// private class DerivedViewModel : BaseViewModel { }
// To avoid this, we call `GetCustomAttributes` directly
// to avoid examining the inheritance hierarchy.
// See https://source.dot.net/#System.Private.CoreLib/src/System/Attribute.CoreCLR.cs,677
var modelMetadataTypeAttributes = type.GetCustomAttributes<ModelMetadataTypeAttribute>(inherit: false);
try
{
return modelMetadataTypeAttributes?.SingleOrDefault()?.MetadataType;
}
catch (InvalidOperationException e)
{
throw new InvalidOperationException("Only one ModelMetadataType attribute is permitted per type.", e);
}
}
}
19 changes: 19 additions & 0 deletions src/Mvc/Mvc.Core/src/ModelMetadataTypeOfTAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.Mvc;

/// <inheritdoc />
/// <typeparam name="T">The type of metadata class that is associated with a data model class.</typeparam>
/// <remarks>
/// This is a derived generic variant of the <see cref="ModelMetadataTypeAttribute"/>
/// which does not allow multiple instances on a single target.
/// Ensure that only one instance of either attribute is provided on the target.
/// </remarks>
public class ModelMetadataTypeAttribute<T> : ModelMetadataTypeAttribute
{
/// <summary>
/// Initializes a new instance of the <see cref="ModelMetadataTypeAttribute" /> class.
/// </summary>
public ModelMetadataTypeAttribute() : base(typeof(T)) { }
}
18 changes: 18 additions & 0 deletions src/Mvc/Mvc.Core/src/ProducesOfTAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.Mvc;

/// <inheritdoc />
/// <typeparam name="T">The <see cref="Type"/> of object that is going to be written in the response.</typeparam>
/// <remarks>
/// This is a derived generic variant of the <see cref="ProducesAttribute"/>.
/// Ensure that only one instance of either attribute is provided on the target.
/// </remarks>
public class ProducesAttribute<T> : ProducesAttribute
{
/// <summary>
/// Initializes an instance of <see cref="ProducesAttribute"/>.
/// </summary>
public ProducesAttribute() : base(typeof(T)) { }
}
24 changes: 24 additions & 0 deletions src/Mvc/Mvc.Core/src/ProducesResponseTypeOfTAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.Mvc;

/// <inheritdoc />
/// <typeparam name="T">The <see cref="Type"/> of object that is going to be written in the response.</typeparam>
public class ProducesResponseTypeAttribute<T> : ProducesResponseTypeAttribute
{
/// <summary>
/// Initializes an instance of <see cref="ProducesResponseTypeAttribute"/>.
/// </summary>
/// <param name="statusCode">The HTTP response status code.</param>
public ProducesResponseTypeAttribute(int statusCode) : base(typeof(T), statusCode) { }

/// <summary>
/// Initializes an instance of <see cref="ProducesResponseTypeAttribute"/>.
/// </summary>
/// <param name="statusCode">The HTTP response status code.</param>
/// <param name="contentType">The content type associated with the response.</param>
/// <param name="additionalContentTypes">Additional content types supported by the response.</param>
public ProducesResponseTypeAttribute(int statusCode, string contentType, params string[] additionalContentTypes)
: base(typeof(T), statusCode, contentType, additionalContentTypes) { }
}
15 changes: 15 additions & 0 deletions src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
#nullable enable
*REMOVED*static Microsoft.AspNetCore.Routing.ControllerLinkGeneratorExtensions.GetUriByAction(this Microsoft.AspNetCore.Routing.LinkGenerator! generator, string! action, string! controller, object? values, string? scheme, Microsoft.AspNetCore.Http.HostString host, Microsoft.AspNetCore.Http.PathString pathBase = default(Microsoft.AspNetCore.Http.PathString), Microsoft.AspNetCore.Http.FragmentString fragment = default(Microsoft.AspNetCore.Http.FragmentString), Microsoft.AspNetCore.Routing.LinkOptions? options = null) -> string?
Microsoft.AspNetCore.Mvc.MiddlewareFilterAttribute<T>
Microsoft.AspNetCore.Mvc.MiddlewareFilterAttribute<T>.MiddlewareFilterAttribute() -> void
Microsoft.AspNetCore.Mvc.ModelBinderAttribute<TBinder>
Microsoft.AspNetCore.Mvc.ModelBinderAttribute<TBinder>.ModelBinderAttribute() -> void
Microsoft.AspNetCore.Mvc.ModelMetadataTypeAttribute<T>
Microsoft.AspNetCore.Mvc.ModelMetadataTypeAttribute<T>.ModelMetadataTypeAttribute() -> void
Microsoft.AspNetCore.Mvc.ProducesAttribute<T>
Microsoft.AspNetCore.Mvc.ProducesAttribute<T>.ProducesAttribute() -> void
Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute<T>
Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute<T>.ProducesResponseTypeAttribute(int statusCode) -> void
Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute<T>.ProducesResponseTypeAttribute(int statusCode, string! contentType, params string![]! additionalContentTypes) -> void
Microsoft.AspNetCore.Mvc.ServiceFilterAttribute<TFilter>
Microsoft.AspNetCore.Mvc.ServiceFilterAttribute<TFilter>.ServiceFilterAttribute() -> void
Microsoft.AspNetCore.Mvc.TypeFilterAttribute<TFilter>
Microsoft.AspNetCore.Mvc.TypeFilterAttribute<TFilter>.TypeFilterAttribute() -> void
Microsoft.AspNetCore.Mvc.ValidationProblemDetails.Errors.set -> void
static Microsoft.AspNetCore.Routing.ControllerLinkGeneratorExtensions.GetUriByAction(this Microsoft.AspNetCore.Routing.LinkGenerator! generator, string! action, string! controller, object? values, string! scheme, Microsoft.AspNetCore.Http.HostString host, Microsoft.AspNetCore.Http.PathString pathBase = default(Microsoft.AspNetCore.Http.PathString), Microsoft.AspNetCore.Http.FragmentString fragment = default(Microsoft.AspNetCore.Http.FragmentString), Microsoft.AspNetCore.Routing.LinkOptions? options = null) -> string?
Microsoft.AspNetCore.Mvc.CreatedResult.CreatedResult() -> void
Expand Down
18 changes: 18 additions & 0 deletions src/Mvc/Mvc.Core/src/ServiceFilterOfTAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using Microsoft.AspNetCore.Mvc.Filters;

namespace Microsoft.AspNetCore.Mvc;

/// <inheritdoc />
/// <typeparam name="TFilter">The <see cref="Type"/> of filter to find.</typeparam>
[DebuggerDisplay("ServiceFilter: Type={ServiceType} Order={Order}")]
public class ServiceFilterAttribute<TFilter> : ServiceFilterAttribute where TFilter : IFilterMetadata
{
/// <summary>
/// Instantiates a new <see cref="ServiceFilterAttribute"/> instance.
/// </summary>
public ServiceFilterAttribute() : base(typeof(TFilter)) { }
}
16 changes: 16 additions & 0 deletions src/Mvc/Mvc.Core/src/TypeFilterOfTAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.AspNetCore.Mvc.Filters;

namespace Microsoft.AspNetCore.Mvc;

/// <inheritdoc />
/// <typeparam name="TFilter">The <see cref="Type"/> of filter to create.</typeparam>
public class TypeFilterAttribute<TFilter> : TypeFilterAttribute where TFilter : IFilterMetadata
{
/// <summary>
/// Instantiates a new <see cref="TypeFilterAttribute"/> instance.
/// </summary>
public TypeFilterAttribute() : base(typeof(TFilter)) { }
}
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,18 @@ public void GetAttributesForProperty_WithModelType_IncludesTypeAttributes()
attribute => Assert.IsType<ClassValidator>(attribute));
}

[Fact]
public void GetAttributeForProperty_WithModelType_HandlesMultipleAttributesOnType()
Copy link
Member

Choose a reason for hiding this comment

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

Great!

This covers ModelMetadataType. Are there other attributes that had AllowMultiple=false?

Copy link
Member Author

@captainsafia captainsafia Mar 14, 2023

Choose a reason for hiding this comment

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

ModelBinder has it, but it's behavior is a little interesting. When constructing the model-binding context, we capture all the attributes on a type, including inherited ones (ref). When constructing the actual model binding context, we pick the attribute closest to the type were evaluating and use its properties to construct the context (ref).

So unlike the ModelMetadata scenario, there isn't a place in code where we try to evaluate a single attribute on a type. We could modify the binding logic above to throw if there are multiple attributes on the same type. That would introduce a new exception in the code (although it's not possible to hit that exception without this current change).

Ditto the same situation for ProducesAttribute. Although we only expect one attribute on a particular type, all the codepaths that consume it assume that attributes can be inherited and don't strictly expect a single attribute to be defined.

So we can throw exceptions at runtime that warn about multiple attributes on the same type but that feels a little heavy handed given the existing behavior for these attributes.

Thoughts?

{
// Arrange
var modelType = typeof(InvalidBaseViewModel);
var property = modelType.GetRuntimeProperties().FirstOrDefault(p => p.Name == nameof(BaseModel.RouteValue));

// Assert
var exception = Assert.Throws<InvalidOperationException>(() => ModelAttributes.GetAttributesForProperty(modelType, property));
Assert.Equal("Only one ModelMetadataType attribute is permitted per type.", exception.Message);
}

[ClassValidator]
private class BaseModel
{
Expand Down Expand Up @@ -322,7 +334,7 @@ private class DerivedModelWithAttributes : BaseModel
{
}

[ModelMetadataType(typeof(BaseModel))]
[ModelMetadataType<BaseModel>]
private class BaseViewModel
{
[Range(0, 10)]
Expand All @@ -335,7 +347,11 @@ private class BaseViewModel
public string RouteValue { get; set; }
}

[ModelMetadataType(typeof(DerivedModel))]
[ModelMetadataType<BaseModel>]
[ModelMetadataType(typeof(BaseModel))]
private class InvalidBaseViewModel : BaseViewModel { }

[ModelMetadataType<DerivedModel>]
private class DerivedViewModel : BaseViewModel
{
[StringLength(2)]
Expand All @@ -358,7 +374,7 @@ public override bool IsValid(object value)
}
}

[ModelMetadataType(typeof(MergedAttributesMetadata))]
[ModelMetadataType<MergedAttributesMetadata>]
private class MergedAttributes
{
[Required]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ private class Person
public Address Address { get; set; }
}

[ModelBinder(BinderType = typeof(AddressModelBinder))]
[ModelBinder<AddressModelBinder>]
private class Address
{
public string Street { get; set; }
Expand Down Expand Up @@ -188,7 +188,7 @@ public async Task BinderTypeOnParameterType_WithData_EmptyPrefix_GetsBound(Bindi

private class Person3
{
[ModelBinder(BinderType = typeof(Address3ModelBinder))]
[ModelBinder<Address3ModelBinder>]
public Address3 Address { get; set; }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
namespace ApiExplorerWebSite;

[Produces("application/json", Type = typeof(Product))]
[ProducesResponseType(typeof(ErrorInfo), 500)]
[ProducesResponseType<ErrorInfo>(500)]
[Route("ApiExplorerResponseTypeOverrideOnAction")]
public class ApiExplorerResponseTypeOverrideOnActionController : Controller
{
Expand All @@ -16,15 +16,15 @@ public void GetController()
}

[HttpGet("Action")]
[Produces(typeof(Customer))]
[Produces<Customer>]
[ProducesResponseType(typeof(ErrorInfoOverride), 500)] // overriding the type specified on the server
public object GetAction()
{
return null;
}

[HttpGet("Action2")]
[ProducesResponseType(typeof(Customer), 200, "text/plain")]
[ProducesResponseType<Customer>(200, "text/plain")]
public object GetActionWithContentTypeOverride()
{
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public string Login(LoginViewModel model)
[HttpPost]
[AllowAnonymous]
[ValidateAntiForgeryToken]
[TypeFilter(typeof(RedirectAntiforgeryValidationFailedResultFilter))]
[TypeFilter<RedirectAntiforgeryValidationFailedResultFilter>]
public string LoginWithRedirectResultFilter(LoginViewModel model)
{
return "Ok";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ public User UserInfo()
return CreateUser();
}

[Produces(typeof(User))]
[Produces<User>]
public IActionResult UserInfo_ProducesWithTypeOnly()
{
return new ObjectResult(CreateUser());
}

[Produces("application/xml", Type = typeof(User))]
[Produces<User>]
public IActionResult UserInfo_ProducesWithTypeAndContentType()
{
return new ObjectResult(CreateUser());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ public class FiltersController : Controller
public IActionResult AlwaysRunResultFiltersCanRunWhenResourceFilterShortCircuit([FromBody] Product product) =>
throw new Exception("Shouldn't be executed");

[ServiceFilter(typeof(ServiceActionFilter))]
[ServiceFilter<ServiceActionFilter>]
public IActionResult ServiceFilterTest() => Content("Service filter content");

[TraceResultOutputFilter]
public IActionResult TraceResult() => new EmptyResult();

[Route("{culture}/[controller]/[action]")]
[MiddlewareFilter(typeof(LocalizationPipeline))]
[MiddlewareFilter<LocalizationPipeline>]
public IActionResult MiddlewareFilterTest()
{
return Content($"CurrentCulture:{CultureInfo.CurrentCulture.Name},CurrentUICulture:{CultureInfo.CurrentUICulture.Name}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public string FromConstraint()
}

[HttpGet]
[TypeFilter(typeof(RequestScopedFilter))]
[TypeFilter<RequestScopedFilter>]
public void FromFilter()
{
}
Expand Down