[Suggestion] Make DefaultLinkGenerator public and non sealed

[BrightSoul]

Hi everyone, I'd like to localize URLs of my application and, in order to do that, my solution is to translate route values for action and controller before they're actually processed by the DefaultLinkGenerator. This is what I'd like to do:

public class MyLinkGenerator : DefaultLinkGenerator
{
    public override string GetPathByAddress<TAddress>(HttpContext httpContext, TAddress address, RouteValueDictionary values, RouteValueDictionary ambientValues = null, PathString? pathBase = null, FragmentString fragment = default, LinkOptions options = null)
    {
        //Fiddle with the RouteValueDictionary here

        //Then invoke the base method
        return base.GetPathByAddress(httpContext, address, values, ambientValues, pathBase, fragment, options);
    }
    //...
}

Since I don't want to change the default url generation logic, I thought this was a simple and effective solution. It's not viable though, since the DefaultLinkGenerator is a private sealed class.

Is there a particular reason why it cannot be a normal, overridable public class?

The fact it's a private sealed class did not stop me from achieving this solution, in fact I just had to wrap the DefaultLinkGenerator with my own implementation. It works but it's way uglier.

In Startup.ConfigureServices:

var serviceProvider = services.BuildServiceProvider();
var defaultLinkGenerator = serviceProvider.GetService<LinkGenerator>();
var myLinkGenerator = new LocalizedLinkGenerator(defaultLinkGenerator);
services.AddSingleton<LinkGenerator>(myLinkGenerator);

Would you please consider to make the DefaultLinkGenerator public and non-sealed?

Thanks. P.S. Overall, I've found maaaany obstacles when trying to localize URLs. Another is in issue #16965. It's almost like you didn't even consider people might want to do it.

[javiercn]

@BrightSoul thanks for contacting us.

We don't tend to make classes like that available since they are focused on implementation details and greatly limit our ability to evolve the codebase.

There is an IOutboundRouteParameterTransformer feature that I think you could probably use for this.

[BrightSoul]

@javiercn Thanks for your reply

There is an IOutboundRouteParameterTransformer feature that I think you could probably use for this.

Unfortunately this option was ruled out in this other issue. https://github.com/dotnet/aspnetcore/issues/4579

The thing is, I need to know what the CultureInfo for the current request is to change the RouteValueDictionary appropriately. It cannot be done in advance. It has to be done per-request, on the fly and just before the DefaultLinkGenerator executes.

I found no other extension point in ASP.NET Core that allows me to translate URLs.

We don't tend to make classes like that available since they are focused on implementation details and greatly limit our ability to evolve the codebase.

Why is that though? DefaultLinkGenerator derives from LinkGenerator, which is an abstract public class, so it must override all its public abstract methods. There's no need to hide those. Maybe hide other non overridden methods of DefaultLinkGenerator, such as GetUriByEndpoints, by making those internal. The class itself should be public, imho.

[BrightSoul]

Just to clarify it further. When I write this code in a Razor View:

<a asp-controller="Book" asp-action="Chapter1">1</a>

The output should be this for an english user:

<a href="/en/Book/Chapter1">1</a>

And this for an italian user:

<a href="/it/Libro/Capitolo1">1</a>

This is an application I made which shows the workarounds I had to take to make it work. https://github.com/BrightSoul/aspnetcore-localization-demo

[ghost]

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[MichalSznajder]

This is again issue related to ambient values handling with endpoint routing changed in 3.0.

[MichalSznajder]

Until @javiercn solves this in .NET 7 (please please) I have this abomination as workaround.

At the end of your Startup.ConfigureServices() add this horrendous code:

var defaultLinkGeneratorDescriptor = services.Single(s => s.ServiceType == typeof(LinkGenerator));
services.Remove(defaultLinkGeneratorDescriptor);
services.AddSingleton(typeof(LinkGenerator), serviceProvider => new CustomLinkGenerator(serviceProvider, defaultLinkGeneratorDescriptor.ImplementationType!));

It removes `DefaultLinkGenerator`` from DI and installs our own implementation. Then add this class to your project:

public class CustomLinkGenerator : LinkGenerator
{
    private readonly LinkGenerator _oldLinkGenerator;

    public CustomLinkGenerator(IServiceProvider serviceProvider, Type linkGeneratorType)
    {
        _oldLinkGenerator = (LinkGenerator)ActivatorUtilities.CreateInstance(serviceProvider, linkGeneratorType);
    }

    public override string? GetPathByAddress<TAddress>(HttpContext httpContext, TAddress address, RouteValueDictionary values,
        RouteValueDictionary? ambientValues = null, PathString? pathBase = null,
        FragmentString fragment = new(), LinkOptions? options = null)
    {
        if (ambientValues?.TryGetValue("culture", out var value) ?? false)
        {
            values["culture"] = value;
        }
        return _oldLinkGenerator.GetPathByAddress(httpContext, address, values, ambientValues, pathBase, fragment, options);
    }

  // other 3 methods are left as excercise for reader
}

This class wraps DefaultLinkGenerator by abusing DI helper and circumventing internal modifier applied to this class. CustomLinkGenerator then manually adds ambient values that we are interested in.

Maybe this would be idea for solving ambient values issues with endpoint routing: give some kind of hook for users to decide? Most complains about ambient values handling change is about multi-tenant (so urls like /tenant/Controller/Action) or things with culture as OP described.

[MartijnFLC]

@michalsznajder Thanks for this! Costed me an enormous amount of time figuring out a way to solve this in an official way in a migration from an app. Couldn't believe there just wasn't one as this problem is already quite old. I just needed a culture in the url to remain there. In https://github.com/dotnet/aspnetcore/issues/31476 @Muchiachio has a similar hack (also thanks!), without exercise, haha. I like yours better, so it's not needed to call BuildServiceProvider().

Hope in .NET 7 it's fixed/has a nice way to deal with this.

[ghost]

Thanks for contacting us. We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. 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.