search

dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.47k stars 10.03k forks source link

MapDynamicControllerRoute doesn't work with DefaultLinkGenerator #16965

Open ollieferns opened 5 years ago

ollieferns commented 5 years ago

Describe the bug

The DefaultLink Generator (and the AnchorTagHelper) do not work when mapping dynamic routes. They do not find any routes and produce a null href.

To Reproduce

  1. create a vanilla mvc site
  2. add one dynamic route
  3. use the anchor tag helper or the LinkGen class
  4. Result is href is null.

see v simple example here.

https://github.com/ollieferns/dynamiclinkgenissue

Further technical details

Runtime Environment: OS Name: Mac OS X OS Version: 10.15 OS Platform: Darwin RID: osx.10.15-x64 Base Path: /usr/local/share/dotnet/sdk/3.1.100-preview2-014569/

Host (useful for support): Version: 3.1.0-preview2.19525.6 Commit: 5672978d91

.NET Core SDKs installed: 3.0.100 [/usr/local/share/dotnet/sdk] 3.1.100-preview1-014459 [/usr/local/share/dotnet/sdk] 3.1.100-preview2-014569 [/usr/local/share/dotnet/sdk]

.NET Core runtimes installed: Microsoft.AspNetCore.App 3.0.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.0-preview1.19508.20 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.0-preview2.19528.8 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App] Microsoft.NETCore.App 2.1.13 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 3.0.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.0-preview1.19506.1 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.0-preview2.19525.6 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]

javiercn commented 5 years ago

@ollieferns thanks for contacting us.

@rynowak can you look into this?

oferns commented 5 years ago

@javiercn @rynowak I cloned the source and had a look. The issue appears to me to be in RouteValuesAddressScheme around line 104

var metadata = endpoint.Metadata.GetMetadata<IRouteNameMetadata>();

Because the endpoint is a IDynamicEndpointMetadata not a IRouteNameMetadata it doesnt consider the route. I made IDynamicEndpointMetadata inherit from IRouteNameMetadata as a fix. It fixed my issue but made 3 tests fail. If I get to the bottom of the issue, how would I go about submitting a PR? I would rather like this fixed. In the meantime, what is the best way of referencing my altered MVC assemblies instead of the default ones for my own project?

oferns commented 5 years ago

Hi, any chance of a quick answer to this while the issue is being looked at? " In the meantime, what is the best way of referencing my altered MVC assemblies instead of the default ones for my own project?"

Is there an easy way to do this? Or do I need to add the MVC projects to my solution?

Thanks, O

ollieferns commented 4 years ago

@rynowak @javiercn could I get a response to my questions at least if there is no progress? thanks

ollieferns commented 4 years ago

@rynowak @javiercn could somebody reach out to me please regarding this issue. It seems to me to be a fairly serious issue with endpoint routing. Happy to discuss privately if required but the total radio silence is a bit unnerving when trying to convince my powers-that-be that .net core and aspnet is a viable technology choice for what we are trying to do here.

oferns commented 4 years ago

So as an update this still does not work in .NET 5.01 Preview. How depressing. No answers to my questions for 4+months and no fix.

MaxHorstmann commented 4 years ago

Same issue here, see Stack Overflow. Is there an update? Would a PR for this be considered?

mkArtakMSFT commented 4 years ago

Thanks for suggesting help, @MaxHorstmann. It's not clear at the moment what the fix would look like here as we haven't yet investigated this. @javiercn do you have something on mind about this for @MaxHorstmann to consider?

JeremyCaney commented 4 years ago

I’m really glad to hear that this is being evaluated for .NET 5.0.0. In the meanwhile, at minimum, it’d be useful to acknowledge this limitation in the documentation. It took me a bit to isolate this behavior and discover this issue.

(If I have a chance, I can look into submitting a PR for the documentation—though I’ll need to investigate the conventions for documenting feature limitations, as I’m not clear how that’s generally handled.)

ghost commented 4 years ago

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.

ghost commented 3 years ago

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.

enricoe73 commented 3 years ago

Hello, any news on this ? It's not woking also for razor pages with endpoints.MapDynamicPageRoute and the tag helper ( asp-page-handler attribute) @mkArtakMSFT

ghost commented 3 years ago

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.

oferns commented 3 years ago

(facepalm)

thilap-web commented 3 years ago

any workarounds on this ?

rjgotten commented 3 years ago

any workarounds on this ?

You have to glue your own link generation into things. To make it work you have to overwrite the default registered address schemes used by the link generator with versions that can handle whatever kind of link generation you need. GetPathByAction is implemented as an extension method on top of GetPathByAddress so you can't override it with a custom implementation directly, you have to go through the whole address scheme pipeline instead.

(I've written it myself before, but it's closed source company code. Having to write it that way blows, to say the least.)

Note also that the whole URL generation pipeline running through LinkGenerator and the address schemes is synchronous through-and-through. So if you need to pull data from a database at any one point -- which is not entirely unthinkable for data-driven localized URL generation -- you have to settle for blocking operations.

ghost commented 2 years ago

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.