Skip to content

Outbound route parameter transformer should support dependency injection #4579

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
BrightSoul opened this issue Dec 11, 2018 · 6 comments
Closed
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates question

Comments

@BrightSoul
Copy link

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

When I register an outbound route parameter transformer, I should be able to register it as a type and not an implementation. This way, I could leverage ASP.NET Core dependency injection. Infact, I might need to read configuration or the database to decide how to do transformations.

Describe the solution you'd like

I register the route parameter transformer like this.

services.AddMvc(options =>
{
    var routeParameterTransformer = new MyParameterTransformer();
    var routeConvention = new RouteTokenTransformerConvention(routeParameterTransformer);
    options.Conventions.Add(routeConvention);
}).SetCompatibilityVersion(CompatibilityVersion.Version_2_2);

Instead, I'd like to register it like this, by just providing its type.

services.AddMvc(options =>
{
    var routeParameterTransformer = typeof(MyParameterTransformer);
    var routeConvention = new RouteTokenTransformerConvention(routeParameterTransformer);
    options.Conventions.Add(routeConvention);
}).SetCompatibilityVersion(CompatibilityVersion.Version_2_2);

This way, ASP.NET Core would inject any dependencies when constructing the transformer.

Describe alternatives you've considered

I don't want to use the ServiceProvider directly.

@Eilon Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Dec 13, 2018
@mkArtakMSFT
Copy link
Contributor

Thanks for contacting us, @BrightSoul.
@JamesNK, can you please look into this? Thanks!

@JamesNK
Copy link
Member

JamesNK commented Jan 16, 2019

Hi @BrightSoul

The RouteTokenTransformerConvention type doesn't have a point where it has a service provider to resolve the type. If you look at the source code you will see that it is very simple - https://github.com/aspnet/AspNetCore/blob/ae9b3992b233b9184bdc1349bc99a780db355086/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/RouteTokenTransformerConvention.cs

To achieve what you want would require giving the type and the service provider to the constructor, and then resolving the type in the constructor. Doing that would beg the question, what is being saved over just resolving the service yourself and then calling the current constructor?

I don't think this is something that we would add. However, the class isn't sealed. You could inherit from it, and add a constructor that does what I described above.

@BrightSoul
Copy link
Author

Hey @JamesNK, thanks for your reply.

If you look at the source code you will see that it is very simple

Yeah, I thought it would be more complex at first and perhaps my expectations for an "outbound parameter transformer" were wrong.
For instance, I expected it to have a scoped life cycle and that I could use it to perform different transformations depending on the current HTTP context, e.g. for URL localization. For an english-speaking user, a link to the products page would direct him to /Products. For italian users the same link would direct them to /Prodotti.

However, as I've seen, an outbound route transformer is just for simple transformations.

What is being saved over just resolving the service yourself and then calling the current constructor?

Nothing, of course. As it is now, I could just inject its dependencies myself when I construct it.

@BrightSoul
Copy link
Author

BTW, in classic ASP.NET I've been handling URL localization by hacking a route constraint to rewrite values in the RouteValuesDictionary, which is absolutely barbaric. I wonder if ASP.NET Core has a better solution to this. A custom middleware would solve the url resolution part but how about url generation?
That's why I opened this issue: I though the route transformer could do both in one place.

Here's what I'm after:
http://www.aspitalia.com/articoli/asp.net/internaz-mvc-5-09.jpg

@JamesNK
Copy link
Member

JamesNK commented Mar 4, 2019

For instance, I expected it to have a scoped life cycle and that I could use it to perform different transformations depending on the current HTTP context, e.g. for URL localization. For an english-speaking user, a link to the products page would direct him to /Products. For italian users the same link would direct them to /Prodotti.

Unfortunately you can't do that with RouteTokenTransformerConvention. It modifies the endpoints generated at startup. By default there is a single set of them and they don't change per request.

You can access the scoped HttpContext in a parameter transformer using IHttpContextAccessor when you want to transform non-controller/action/area/page routes values:

public class SampleOutboundRouteTransformer : IOutboundParameterTransformer
{
    private readonly IHttpContextAccessor _httpContextAccessor;

    public SampleOutboundRouteTransformer(IHttpContextAccessor httpContextAccessor)
    {
        _httpContextAccessor = httpContextAccessor;
    }

    public string TransformOutbound(object value)
    {
        return value?.ToString();
    }
}
app.UseMvc(routes =>
{
    routes.MapRoute(
        name: "default",
        template: "{controller=Home}/{action=Index}/{id?}",
        defaults: null,
        constraints: new { id = new SampleOutboundRouteTransformer(routes.ServiceProvider.GetRequiredService<IHttpContextAccessor>()) });
});

This would allow id value to be transformed per request using values from HttpContext.

@mkArtakMSFT
Copy link
Contributor

Thanks for contacting us. We believe that the question you've raised have been answered. If you still feel a need to continue the discussion, feel free to reopen it and add your comments.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates question
Projects
None yet
Development

No branches or pull requests

4 participants