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

Multiple UrlResolvers return Multiple nodes with the same URL '' were found. SiteMap requires that sitemap nodes have unique URLs #322

Closed
Guymestef opened this issue Jun 10, 2014 · 6 comments

Comments

@Guymestef
Copy link

In SiteMap.cs/AddNodeInternal, this.siteMapChildStateFactory.CreateUrlKey(node); always return an empty string for node with a custom UrlResolver.
So when we use UrlResolver on 2 nodes, MultipleNodesWithIdenticalUrl exception is thrown.

@NightOwl888
Copy link
Collaborator

While I agree that this is a bug that needs to be fixed, the ideal way to fix it is not possible to do without a breaking change to the ISiteMapNodeUrlResolver interface, which in turn cannot be done without a major version change. The problem is that there is no way for a custom ISiteMapNodeUrlResolver to indicate whether the URL that is generated comes from the .NET routing or another source, such as a URL that is explicitly specified in the Url property, and the matching logic needs to be able to tell the difference.

However, now that I look at it again, I see that this could probably be changed not to consider whether the URL resolver is the default one:

// instead of 
bool isMvcUrl = string.IsNullOrEmpty(node.UnresolvedUrl) && this.UsesDefaultUrlResolver(node);

// do

bool isMvcUrl = string.IsNullOrEmpty(node.UnresolvedUrl);

At the moment, I don't recall exactly what the thinking was behind this. It probably had something to do with the fact that we really can't rely on the fact that if node.UnresolvedUrl is null or empty that the URL is in fact being resolved using .NET routing. And if it is not being resolved by MVC, the node matching logic may not function correctly. At one point the URLs were being matched using both URL matching and .NET route matching instead of one or the other, and this was leading to situations where invalid configurations would match on the URL and fail in other ways. So now they are separated all the way up to the URL resolver, but what is really needed is for the URL resolver to return a boolean whether the source of the URL came from the MVC UrlHelper class.

That said, I would not recommend using a custom URL resolver. While that extension point may have made sense in MVC 2 because there was little support in MVC 2 for customizing the URL (such as making them lowercase), it is now better to make your URL changes through .NET routing. The only situation where using a custom ISiteMapNodeUrlResolver makes sense is in the case where you need your URLs to resolve differently in MvcSiteMapProvider than they do in MVC (which I imagine would be pretty rare). You can either use the built-in Route class (and extension methods) to customize your URLs or if you need to make an advanced URL scheme, you can subclass RouteBase. See this post for an example.

@Guymestef
Copy link
Author

My custom urlResolver is inherited from MvcSiteMapProvider.Web.UrlResolver.SiteMapNodeUrlResolver. I proposed a correction just before you post your answer. But I'm not sure if it's a good correction because I don't know either why we have to do:
bool isMvcUrl = string.IsNullOrEmpty(node.UnresolvedUrl) && this.UsesDefaultUrlResolver(node);

NightOwl888 added a commit to NightOwl888/MvcSiteMapProvider that referenced this issue Jun 10, 2014
…le nodes with same URL" exception."

This reverts commit bc43bf9.
@NightOwl888
Copy link
Collaborator

I believe the code this branch will now work for you, but please check it out as I have not done any testing. If successful, let me know and i will roll it into a patch.

What changes do you want to make to the MvcSiteMapProvider.Web.UrlResolver.SiteMapNodeUrlResolver? Could you post your code?

I am a bit perplexed why you would want the URLs in MvcSiteMapProvider to resolve differently than they do in MVC when specifying @Html.Action or @Html.ActionLink, but if there is something that could be done to it to make the default URL resolver better, perhaps that change could be put in.

If you are just looking for a way to change the way the URLs look, you can use customized routes or as I already mentioned you can also inherit RouteBase if you need ultimate control. Both of these methods are fully supported by MVC and by MvcSiteMapProvider, and provide a common place for you to change your URLs.

@Guymestef
Copy link
Author

It works.
For the background, I'm migrating an ASP.MVC 4 to MVC 5 application.
On public profile pages, we have a menu with public url like: "{lang}/pro/profile/{country}/{city}/{companyName}/{id}"
The custom url resolver is used to populate country, city and companyName by adding values in RouteData, using id value.
I already inherit RouteBase to implement localized routes, but right now I try to migrate the application and try to not redo too much things.

@NightOwl888
Copy link
Collaborator

The fix is now in v4.6.6 and available on NuGet.

You might want to check out using IValueProvider as described in this post. That would allow you to look up the country, city, and companyName for a given id in the URL and have them automatically populated (either in a model or as parameters of the action method). However, if you are concerned about keeping the URLs the same as they were previously, that may not be helpful.

@Guymestef
Copy link
Author

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants