Closed captainsafia closed 1 month ago
Implement ApiExplorer abstraction layer with no MVC dependencies 😛
How difficult would this be? I assume "quite" because it feels like a bad/unnecessary coupling. Feels like this should be the opposite; MVC depended on ModelMetadata
.
It does feel like a fairly extreme alternative over simply creating a new protected constructor however.
It's probably also far too late in the release cycle for that 😄
How difficult would this be? I assume "quite" because it feels like a bad/unnecessary coupling. Feels like this should be the opposite; MVC depended on ModelMetadata.
I've actually done some thinking about this in the past. ApiExplorer is a really useful concept but the fact that it's tied to MVC's APIs limits its potential and keeps it from being completely useable as an intermediary abstraction layer for minimal APIs and other open-source API frameworks on top of ASP.NET Core.
The easy part of the work around this is the API review required to examine the pre-existing types in ApiExplorer and figure out implementation-neutral variants of them. The hard part is replatting everything on top of those new APIs.
It's one of those things that always slips below the cost/benefit threshold, IMO. More people would need to be taking advantage of IApiDescriptionProvider
abstractions. There might be a future where the cost/benefit threshold changes and a neutral implementation becomes more compelling.
[API Review]
MVC | Minimal | |
---|---|---|
Trimmed | C,E | P,C,E |
Untrimmed | P,C,E |
internal protected
namespace Microsoft.AspNetCore.Mvc.ModelBinding;
public abstract class ModelMetadata
{
+ protected ModelMetadata(ModelMetadataIdentity identity, bool disableDynamicDependencies)
- internal virtual bool IsParseableType { get; private set; }
+ protected internal virtual bool IsParseableType { get; private set; }
}
Revised API approved!
Closing....ended up going with another approach here that did not require new API.
Background and Motivation
Background on this issue can be found in this comment.
Proposed API
Usage Examples
This API is primarily intended for consumption within the
EndpointMetadataApiDescriptionProvider
to construct theModelMetadta
implementation that is a required part of ApiExplorer metadata.https://github.com/dotnet/aspnetcore/blob/a21c9eda08cbb2d64f40c51e1b2aa1d9c0f051ef/src/Mvc/Mvc.Abstractions/src/ApiExplorer/ApiParameterDescription.cs#L7-L17
For the minimal API-based implementaton of ApiDescriptionProvider, there is a custom
EndpointModelMetadtata
class that extends the abstractModelMetadata
class that we are modifying:https://github.com/dotnet/aspnetcore/blob/6812d1df084f8d7fddc85c83d6b5ed0319993873/src/Mvc/Mvc.ApiExplorer/src/EndpointModelMetadata.cs#L12-L18
For a complete example of how these APIs are used, see https://github.com/dotnet/aspnetcore/pull/56827/commits/aa3ac70ee4d7d64e1707c9350a9590e75139afba.
Alternative Designs
ModelMetadata
s use ofParameterBindingCache
to the concreteDefaultModelMetadata
implementationRisks
The current shape of this change introduces a regression for scenarios where we generate
ModelMetadata
for responses or request bodies annotated withAccepts
. Unlike parameters, where we can take advantage of theIParameterBindingMetadata
that we introduced earlier, there is no similar abstraction layer for response types/accepts metadata. This means thatEndpointModelMetadata
generated for these will not have information about whether a type is parseable or complex.We could solve this in one of two ways:
IParameterBindingMetadata
for responses and accepts metada and introduce new constructs onModelMetadata
that consume this metadataI lean slightly towards the later option here -- it's cheaper and we can always revisit and approach #2 if we discover that there were libraries that were taking advantage of
ModelMetadata
for responses in the minimal API-based API explorer.