Skip to content
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

IRouter.GetVirtualPath alternative in EndpointRouting #23041

Open
alienwareone opened this issue Jun 17, 2020 · 9 comments
Open

IRouter.GetVirtualPath alternative in EndpointRouting #23041

alienwareone opened this issue Jun 17, 2020 · 9 comments
Labels
affected-very-few This issue impacts very few customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-routing Priority:1 Work that is critical for the release, but we could probably ship without severity-major This label is used by an internal tool

Comments

@alienwareone
Copy link

Based on this topic:
#18883

The "IRouter.RouteAsync" alternative is "DynamicRouteValueTransformer.TransformAsync".

What is the "VirtualPathData GetVirtualPath(VirtualPathContext context)" alternative for complex url generation?

For example:

public override VirtualPathData GetVirtualPath(VirtualPathContext context)
{
  var routeContext = context["context"] as new ProductListRouteContext();
  var pathSegements = new List<string>();
  if (context?.Category?.PageName != null)
  {
    pathSegments.Add(context.Category.PageName);
  }
  // Add other pathSegements based on the context/filters...
  return new new VirtualPathData(this, string.Join("/", pathSegments));
}

In my view:

Url.RouteUrl("ProductList", new { context = Model.ProductListRouteContext });

@mkArtakMSFT mkArtakMSFT added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-routing labels Jun 17, 2020
@javiercn javiercn added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Jun 17, 2020
@javiercn javiercn added this to the Next sprint planning milestone Jun 17, 2020
@ghost
Copy link

ghost commented Jul 21, 2020

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.

@SteveSandersonMS SteveSandersonMS added affected-very-few This issue impacts very few customers severity-major This label is used by an internal tool labels Oct 6, 2020 — with ASP.NET Core Issue Ranking
@alienwareone
Copy link
Author

The following issue will replace this:
#27646

@marcuslindblom
Copy link

marcuslindblom commented Jul 5, 2021

@alienwareone Did you find a solution/workaround for this issue? I'm also moving from IRouter to endpoint routing and this feels like a deal breaker.

@rynowak maybe you have some workaround or is it back to IRouter for us trying to convert to endpoint routing?

@alienwareone
Copy link
Author

@marcuslindblom I migrated to:

Sample:
https://github.com/alienwareone/ComplexOutboundParameterTransformerSample/blob/a52b3d8191a382cea9c35535bcf6075d0d3fc8cd/WebApplication2/ProductFiltersConstraint.cs

@marcuslindblom
Copy link

marcuslindblom commented Jul 9, 2021

@alienwareone I tried using the IOutboundParameterTransformer to transform the link but it appends all route data as query parameters even though i return a string without a querystring.

This transformer:

public string TransformOutbound(object? value)
{
    return "hello-from-transformer";
}

Using this tag helper:

<a asp-controller="home" asp-action="index" asp-route-id="66eb5dbc-e73b-4d8f-add5-d245ebf14683">Hello, world</a>

Generates this link: hello-from-transformer?id=66eb5dbc-e73b-4d8f-add5-d245ebf14683

Also, instead of using Constraints I use Conventions to add my transformer.

@javiercn javiercn added the jcn-p0 label Nov 5, 2021
@mkArtakMSFT mkArtakMSFT added the Priority:1 Work that is critical for the release, but we could probably ship without label Nov 23, 2021
@mkArtakMSFT mkArtakMSFT modified the milestones: Backlog, .NET 7 Planning Nov 23, 2021
@ghost
Copy link

ghost commented Nov 23, 2021

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. 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.

@mkArtakMSFT mkArtakMSFT added Priority:1 Work that is critical for the release, but we could probably ship without and removed Priority:1 Work that is critical for the release, but we could probably ship without labels Nov 10, 2022
@thurfir
Copy link

thurfir commented Nov 18, 2022

For most use cases, it is possible to override the UrlHelperFactory and implement your logic there: https://stackoverflow.com/a/71871803/4101909

However, this is far from ideal, and is sync-only.

@rjgotten
Copy link

rjgotten commented Jan 25, 2023

The "IRouter.RouteAsync" alternative is "DynamicRouteValueTransformer.TransformAsync".

Just for the record: no. It's not a suitable alternative.
At all.

DynamicRouteValueTransformer requires that you both have the dynamic route as well as the target route it should transform to, available and registered.
I.e. if you have an application with a ProductController.Index action method and want to use URLs with fancy product slugs, perhaps localized, via a dynamic transformer then your application will also have /Product/Index/{id} be publicly reachable.

The solution to that particular problem is to not use the tools MS gives you, but instead cook up your own RouteAttribute subclass which registers a catch-all pattern "{**path}" while simultaneously adding some endpoint metadata that a custom EndpointSelectorPolicy can filter on and which can manipulate the incoming route data.

That's basically how you do what DynamicRouteValueTransformer does; except correctly.

URL generation requires registering a custom IEndpointAddressScheme<> if you want to use LinkGenerator.
If you also want the in-built asp-action ; asp-controller; etc. attributes to play nice, you need to create your own IUrlHelperFactory, indeed.

In both cases, LinkGenerator and IUrlHelper it's also a synchronous-only API. 😞
Which means if you have any kind of complexities involved in your URL generation, ... sync-over-async and a healthy cursing at the short-sighted API-design it is. (I mean; really? synchronous-only? Why?)

@ghost
Copy link

ghost commented Oct 10, 2023

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-very-few This issue impacts very few customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-routing Priority:1 Work that is critical for the release, but we could probably ship without severity-major This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

10 participants