Skip to content

Support binding parameters to type Uri in Minimal Actions #36649

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
martincostello opened this issue Sep 17, 2021 · 3 comments · Fixed by #42217
Closed

Support binding parameters to type Uri in Minimal Actions #36649

martincostello opened this issue Sep 17, 2021 · 3 comments · Fixed by #42217
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-minimal-actions Controller-like actions for endpoint routing help wanted Up for grabs. We would accept a PR to help resolve this issue old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Priority:3 Work that is nice to have triage-focus Add this label to flag the issue for focus at triage

Comments

@martincostello
Copy link
Member

Is your feature request related to a problem? Please describe.

In MVC controllers actions, it is possible to strongly-type a parameter expected to be a URI as Uri. For example:

public IActionResult MyAction([FromQuery(Name = "redirect_uri")] Uri? redirectUri)
{
    // Implementation
}

However, attempting to replicate this with a Minimal Action endpoint results in an exception being thrown.

app.MapGet("/my-action", ([FromQuery(Name = "redirect_uri")] Uri? redirectUri) =>
{
    // Implementation
});
  Message: 
System.InvalidOperationException : No public static bool Uri.TryParse(string, out Uri) method found for redirectUri.

  Stack Trace: 
RequestDelegateFactory.BindParameterFromValue(ParameterInfo parameter, Expression valueExpression, FactoryContext factoryContext)
RequestDelegateFactory.CreateArgument(ParameterInfo parameter, FactoryContext factoryContext)
RequestDelegateFactory.CreateArguments(ParameterInfo[] parameters, FactoryContext factoryContext)
RequestDelegateFactory.CreateTargetableRequestDelegate(MethodInfo methodInfo, RequestDelegateFactoryOptions options, FactoryContext factoryContext, Expression targetExpression)
RequestDelegateFactory.Create(Delegate handler, RequestDelegateFactoryOptions options)
DelegateEndpointRouteBuilderExtensions.Map(IEndpointRouteBuilder endpoints, RoutePattern pattern, Delegate handler)
DelegateEndpointRouteBuilderExtensions.MapMethods(IEndpointRouteBuilder endpoints, String pattern, IEnumerable`1 httpMethods, Delegate handler)
DelegateEndpointRouteBuilderExtensions.MapGet(IEndpointRouteBuilder endpoints, String pattern, Delegate handler)

Describe the solution you'd like

Either of:

  1. Minimal Actions (RequestDelegateFactory) special-cases Uri to bind parameters;
  2. Minimal Actions supports extending model binding for types not owned by the application where a TryParse or BindAsync method cannot be added;
  3. The Uri type adds a TryParse() method that can be consumed by the built-in binding.

Additional context

Found using .NET SDK 6.0.100-rc.1.21458.32 to port an existing MVC application to use Minimal Actions.

@davidfowl
Copy link
Member

Yes this is because MVC supports TypeConverters natively. We could choose to do this as well but there are some concerns around that subsystem and AOT friendliness. If we did this, we would support Uri natively or support it via TypeConverter.

@davidfowl davidfowl added the feature-minimal-actions Controller-like actions for endpoint routing label Sep 17, 2021
@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 Sep 17, 2021
@rafikiassumani-msft rafikiassumani-msft added this to the .NET 7 Planning milestone Sep 17, 2021
@rafikiassumani-msft
Copy link
Contributor

We will revisit this in NET7. We might examine the following suggestion: The Uri type adds a TryParse() method that can be consumed by the built-in binding.

@rafikiassumani-msft rafikiassumani-msft added triage-focus Add this label to flag the issue for focus at triage Priority:3 Work that is nice to have Cost:S labels Jan 11, 2022
@davidfowl
Copy link
Member

Seems like this might be the only type we don't support OOTB that MVC does (with built in TypeConverters). That's based on this list here https://docs.microsoft.com/en-us/aspnet/core/mvc/models/model-binding?view=aspnetcore-6.0#simple-types without consideration for extensibility via type converters.

Also for the implementation, the type converter does this https://github.com/dotnet/runtime/blob/5605bdff4b104496941cc9e6acfc19783a52e7b8/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/UriTypeConverter.cs#L41-L49. We'll want a TryCreate overload with similar parameters.

@rafikiassumani-msft rafikiassumani-msft added the help wanted Up for grabs. We would accept a PR to help resolve this issue label Jun 14, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 16, 2022
@amcasey amcasey added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-minimal-actions Controller-like actions for endpoint routing help wanted Up for grabs. We would accept a PR to help resolve this issue old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Priority:3 Work that is nice to have triage-focus Add this label to flag the issue for focus at triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants