Skip to content

Investigate moving MVC Attributes from Microsoft.AspNetCore.Mvc namespace to Microsoft.AspNetCore.Http namespace #39425

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

Closed
Tracked by #39338
rafikiassumani-msft opened this issue Jan 10, 2022 · 2 comments
Assignees
Labels
old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Priority:1 Work that is critical for the release, but we could probably ship without triage-focus Add this label to flag the issue for focus at triage

Comments

@rafikiassumani-msft
Copy link
Contributor

rafikiassumani-msft commented Jan 10, 2022

With Minimal APIs we introduced different attributes such as Produces<T>, Consumes<T> as part of the Microsoft.AspNetCore.Http namespace. We would like to investigate the effort/trouble required to move the remaining MVC attributes (Consumes<T>, etc.) to Microsoft.AspNetCore.Http namespace and be consistent across both Minimal and MVC.

This could be both source and binary breaking.

@rafikiassumani-msft rafikiassumani-msft added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Jan 10, 2022
@rafikiassumani-msft rafikiassumani-msft added this to the .NET 7 Planning milestone Jan 10, 2022
@ghost
Copy link

ghost commented Jan 10, 2022

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@rafikiassumani-msft rafikiassumani-msft added triage-focus Add this label to flag the issue for focus at triage Priority:1 Work that is critical for the release, but we could probably ship without Cost:XL labels Jan 11, 2022
@BrennanConroy
Copy link
Member

Took a quick look at this and am not sure it's really feasible.

To start with we can look at the metadata defined in https://github.com/dotnet/aspnetcore/tree/main/src/Http/Http.Abstractions/src/Metadata and our goal would be to find attributes that implement these interfaces and move them from MVC to Http.
Many of the MVC attributes are defined in https://github.com/dotnet/aspnetcore/tree/main/src/Mvc/Mvc.Core/src and if we look at them we will see that nearly all of them also implement one or more MVC specific interfaces.
For example, the ConsumesAttribute

public class ConsumesAttribute :
Attribute,
IResourceFilter,
IConsumesActionConstraint,
IApiRequestMetadataProvider,
IAcceptsMetadata

implements a resource filter and an action constraint, both of which are MVC specific features. It wouldn't make sense to move those interfaces to Http as well and we can't remove those from the attribute as MVC uses them.

Other attributes have similar problems, like FromBody implements IBindingSourceMetadata and IConfigureEmptyBodyBehavior.

It looks like unless we are willing to make huge breaking changes or move a lot of MVC specific interfaces into Http we can't move the public attributes from MVC to Http.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Priority:1 Work that is critical for the release, but we could probably ship without triage-focus Add this label to flag the issue for focus at triage
Projects
None yet
Development

No branches or pull requests

2 participants