Should use LinkGenerator instead of injecting IUrlHelper

[lamLeX]

Which project template is the issue about?
ApiTemplate
...

Which version of the project template is the issue about?
2.2.0
...

Which version of Visual Studio or the dotnet SDK were you using?
2.2.401
...

In Asp.net Core 2.2, a singleton service called LinkGenerator is introduced. Currently, it behaves similar to IUrlHelper. Instead of manually register the IUrlHelper in Startup.cs like this

.AddScoped(x => x
                    .GetRequiredService<IUrlHelperFactory>()
                    .GetUrlHelper(x.GetRequiredService<IActionContextAccessor>().ActionContext))

the template should use LinkGenerator instead.

References:

• https://wildermuth.com/2018/10/15/LinkGenerator-in-ASP-NET-Core-2-2
• https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.routing.linkgenerator?view=aspnetcore-2.2
• https://devblogs.microsoft.com/aspnet/asp-net-core-2-2-0-preview1-endpoint-routing/

[RehanSaeed]

Yes, I'm aware of this. I believe I tried to use it at one point but can't remember why I stopped. I'll check and come back to this issue.

[RehanSaeed]

As far as I can work out, to generate a URL using LinkGenerator you have to do this:

this.linkGenerator.GetUriByRouteValues(
    CarsControllerRoute.GetCarPage,
    new PageOptions() { First = pageOptions.First, Last = pageOptions.Last, After = endCursor }, 
    Uri.UriSchemeHttps,
    new HostString("todo-get-host-from-httpcontextaccessor"));

The alternative with IUrlHelper, a bit of code in Startup to register IUrlHelper and my extension methods is:

this.urlHelper.AbsoluteRouteUrl(
    CarsControllerRoute.GetCarPage,
    new PageOptions() { First = pageOptions.First, Last = pageOptions.Last, After = endCursor });

I'd really like to make the switch to simplify the template. If there is something I'm missing and there is a way to use the current URL's schema and host with LinkGenerator, then I'd like to learn how.

[RehanSaeed]

Feel free to reopen if you find anything to improve the situation.

[lamLeX]

@RehanSaeed
The LinkGenerator class is designed so that it can be used in normal dotnet core app when asp.net midleware pipeline is not available.
The example you gave is used when the httpcontext is not available.
When httpcontext is available, you can use something like this

this.linkGenerator.GetUriByRouteValues(
    this.httpContextAccessor.HttpContext,
    CarsControllerRoute.GetCarPage,
    new PageOptions() { First = pageOptions.First, Last = pageOptions.Last, After = endCursor });

Since we did the IOC register for HttpContextAccessor anyway, injecting it and use itexplicitly like this is clearer. However, if you want to hide the httpcontext for the sake of brevity, you could always have something to inject HttpContext similarly to what your AbsoluteRouteUrl extension does but it would deviate from any documentations available online.

[RehanSaeed]

I missed that overload. I agree that it's a bit better than the one I posted. It's a shame that you have to pass in the HttpContext as an argument, ideally there would be a way to get rid of that.

With IUrlHelper, you can actually get hold of the HttpContext from the UrlHelper instance, so I created an extension method that makes use of that, so you don't have to pass it in.

What are your thoughts, do you still think we should use it?

[lamLeX]

Sorry that for a late reply.

In my opinion, most of the case when the user code wants to generate the link, it needs to have access to HttpContext too.
Take the GetCarPageCommand for example, the GetLinkValueItem method has access to HttpContext via the injected IHttpContextAccessor.

The only scenario that hiding HttpContext from user code may come handy is when you want to implemented some helper extension method like you've done with AbsoluteRouteUrl. However, I would argue that this is a very specific API that only available in Boxed.AspNetCore library. And it makes the template less portable.
Developers who don't have accessed to decompiled code or the source code of AbsoluteRouteUrl would have trouble understanding what AbsoluteRouteUrl actually do. Therefore, I would prefer using the Asp.Net framework's LinkGenerator directly where tutorial and documentation are wildly available.
It also reduces a chunk of code that you need to maintain in your Boxed.AspNetCore library, make it more streamlined.

That's all for my feedback as a user. I'll leave the final decision for you.

[RehanSaeed]

Honestly, you have compelling arguments and I'm in two minds about it. There is no perfect solution unfortunately. So, I've raised this issue, lets see what the ASP.NET Core team say:

dotnet/aspnetcore#14192

[RehanSaeed]

After receiving a very positive response in that issue, I've switched the project to use LinkGenerator. It sounds like the next version of ASP.NET Core will default to using the current HttpContext, so we won't need to pass it in.