Menu won't render anything if current page is not in sitemap

[unklegwar]

When doing @Html.MvcSiteMap().Menu(1, 1, true), nothing renders if the current page doesn't exist in the sitemap. Why does the current page have to be in the sitemap for the menu to render? I can see how there wouldn't be a "current" node, but I'm just asking for a menu of what's in the sitemap, where I CURRENTLY am shouldn't matter.

Using route pattern ""{controller}/{action}/{id}", should it be necessary to specify preservedRouteParameters="id" in order to match a node with just "controller" and "action" (id doesn't matter, for example, when the current page is /foo/bar/1234, I only care that I'm on /foo/bar)

Thanks again.

[NightOwl888]

Thanks for the report. I agree that this behavior is broken, especially when you consider that most of the other overloads of the menu helper do not make the menu disappear when the current node is null.

This is an unfortunate side effect of fixing the SiteMap so it returns null in the case where no matching node is found. In V3, if no match was found the root node was returned. This behavior made the menu not disappear, but it also meant that there was no reasonable way to determine that no current node match was found.

On the other hand, the menu was not changed much, and perhaps it should have been. I guess the place to start would be to agree on what should happen if the current node is null. For a menu with levels to work, there must be some node returned in order for the menu to render.

@maartenba

In V3, there was an approach used that attempted to remove the last segment of the URL recursively until a hit was found. This covers the most common case where the default route is used and/or the URLs are properly made "hackable" (i.e. /somecategory, /somecategory/somepage, etc.).

That said, this code path was unreachable in V3 because the SiteMap.CurrentNode property always returned the root node in a no match scenario, meaning the current node could never be null.

Also, this logic effectively means that the current level could change in the case where the current page is not registered with the SiteMap, which could change how the menu renders. The "correct" behavior in this case is up for interpretation.

public static SiteMapNode GetCurrentNode(SiteMapProvider selectedSiteMapProvider)
{
    // get the node matching the current URL location
    var currentNode = selectedSiteMapProvider.CurrentNode;

    // if there is no node matching the current URL path, 
    // remove parts until we get a hit
    if (currentNode == null)
    {
        var url = HttpContext.Current.Request.Url.LocalPath;

        while (url.Length > 0)
        {
            // see if we can find a matching node
            currentNode = selectedSiteMapProvider.FindSiteMapNode(url);

            // if we get a hit, stop
            if (currentNode != null) break;

            // if not, remove the last path item  
            var lastSlashlocation = url.LastIndexOf("/");
            if (lastSlashlocation < 0) break; // protects us from malformed URLs
            url = url.Remove(lastSlashlocation);
        }
    }

    return currentNode;
}

Ideally, there would be a way to replace this logic in case the URL scheme is radically different than the typical "hackable" URL scheme. But unfortunately, this logic is buried in the static Menu helper and the options for injecting services here are quite limited. Fortunately, making a custom menu is an option if this logic isn't suitable.

So, it would seem that the best course of action would be to modify this slightly to mimic what the V3 SiteMap did (or rather what I suspect it was supposed to do) - that is, return the root node if all else fails.

public static ISiteMapNode GetCurrentNode(ISiteMap selectedSiteMap)
{
    return GetCurrentNode(selectedSiteMap, false);
}

public static ISiteMapNode GetCurrentNode(ISiteMap selectedSiteMap, bool returnRootNodeIfNotFound)
{
    // get the node matching the current URL location
    var currentNode = selectedSiteMap.CurrentNode;

    // if there is no node matching the current URL path, 
    // remove parts until we get a hit
    if (currentNode == null)
    {
        var url = HttpContext.Current.Request.Url.LocalPath;
        var queryString = HttpContext.Current.Request.Url.Query;

        while (url.Length > 0)
        {
            // see if we can find a matching node
            currentNode = selectedSiteMap.FindSiteMapNode(url + queryString);

            // if we get a hit, stop
            if (currentNode != null) break;

            // if not, remove the last path item  
            var lastSlashlocation = url.LastIndexOf("/");
            if (lastSlashlocation < 0) break; // protects us from malformed URLs
            url = url.Remove(lastSlashlocation);
        }
    }

    // If the current node is still null, return the root node.
    // This is the same way the SiteMap.FindSiteMapNode(rawUrl) method worked in v3.
    if (currentNode == null && returnRootNodeIfNotFound)
    {
        currentNode = selectedSiteMap.RootNode;
    }

    return currentNode;
}

Whether that is the correct course of action is up for debate.

I have also fixed the SiteMap.FindSiteMapNode(rawUrl) method so it will attempt to match "on URL" for nodes that are registered using controller and action as well as nodes that are registered by setting the Url property/attribute explicitly.

public virtual ISiteMapNode FindSiteMapNode(string rawUrl)
{
    if (rawUrl == null)
    {
        throw new ArgumentNullException("rawUrl");
    }
    rawUrl = rawUrl.Trim();
    if (rawUrl.Length == 0)
    {
        return null;
    }

    // NOTE: If the URL passed is absolute, the public facing URL will be ignored
    // and the current URL will be the absolute URL that is passed.
    var publicFacingUrl = this.urlPath.GetPublicFacingUrl(this.HttpContext);
    var currentUrl = new Uri(publicFacingUrl, rawUrl);

    // Search the internal dictionary for the URL that is registered manually.
    var node = this.FindSiteMapNodeFromUrl(currentUrl.PathAndQuery, currentUrl.AbsolutePath, currentUrl.Host, HttpContext.CurrentHandler);

    // Search for the URL by creating a context based on the new URL and matching route values.
    if (node == null)
    {
        // Create a TextWriter with null stream as a backing stream 
        // which doesn't consume resources
        using (var nullWriter = new StreamWriter(Stream.Null))
        {
            // Create a new HTTP context using the current URL.
            var currentUrlHttpContext = this.mvcContextFactory.CreateHttpContext(null, currentUrl, nullWriter);

            // Find node for the passed-in URL using the new HTTP context. This will do a
            // match based on route values and/or query string values.
            node = this.FindSiteMapNode(currentUrlHttpContext);
        }
    }

    return this.ReturnNodeIfAccessible(node);
}

Using this approach takes into consideration the current context rather than attempting to store the URLs when the SiteMap is built and then attempting to match them with a completely different context later.

Thoughts on what the best course of action for what to do when a menu with levels doesn't have a current node?

[unklegwar]

Is it just the @Html.MvcSiteMap().Menu(1, 1, true) overload that's broken? Is there another overload of Menu I can use that will render the 1st level of the sitemap, regardless of current node?

Also, is the requirement to use preservedRouteParameters to match on id tangled up in this current node business?

If your closing question was directed at me (I wasn't sure), I would think that when I'm asking to render a level of the sitemap as a menu, that it would render, regardless of current location.

[maartenba]

I think ideally, the "closest match" should be returned, and then the root node and then null.

By closest match, I mean:

• If there is a node matching the RouteData's controller, action, id and preserved route parameters, that should be the match
• Next, match controller, action
• Next, match controller
• Next, root node
• Next, null

[NightOwl888]

@unklegwar

Is there another overload of Menu I can use that will render the 1st level of the sitemap, regardless of current node?

Actually, every overload that uses levels is broken because without a current node, there is no way to determine what level the current page is at.

Also, is the requirement to use preservedRouteParameters to match on id tangled up in this current node business?

Yes. However, you can either use preservedRouteParameters or configure the route values explicitly (typically by implementing IDynamicNodeProvider or ISiteMapNodeProvider), see How to Make MvcSiteMapProvider Remember a User's Position.

If your closing question was directed at me (I wasn't sure), I would think that when I'm asking to render a level of the sitemap as a menu, that it would render, regardless of current location.

Back to the original concern - it doesn't render because there is no way to determine the current level when there is no current node.

@maartenba

Interesting idea.

However, I am not sure exactly how that could work. First of all, there is no such thing as a null root node, so returning null in the last case is moot.

There is also no such thing as a "partial" match as far as the route values are concerned. For example, which node would we return on a "controller only" match? It may match several, and the only real way to determine which one is the "default" is to analyze the configured routes to determine if there is a default action value. This logic also doesn't take into consideration any custom route values - it may be that the application doesn't have any nodes that are configured without them.

After analyzing the behavior of the URL partial matching code, it looks like this is about as close as we can get to the "right" behavior. This would use the routing infrastructure to derive the values, so any custom values would automatically be taken into consideration even if they don't correspond to what is in the URL.

In addition, it supports the fact that URLs should be "hackable", which is the recommended way to configure a web site. It also supports the default route directly. If a default action is configured, it will select the correct parent action (using the default route, it always chooses the one that corresponds to "Index"). If customizations are made to the routing the worst case scenario is that the root node will be returned on a non-matching page, which will still make it render the menu provided the root node is within its configured level.

The rest of the menu appears to have been designed to take this URL partial matching into consideration by limiting the level of the nodes according to which one is returned.

I was only referring to how the Menu finds its current node, not any of the other HTML helpers. For example, I believe that the current behavior of the SiteMapPath is correct (it doesn't render if there is no current node) because this behavior directly corresponds to what Microsoft's implementation does when there is no current node. This is true for most of the other HTML helpers as well. The menu seems to be the only oddball because of the need to determine the current node's level. Basically, the "partial matching" would only make sense to use in the menu.

FYI - I have implemented the proposed changes and put them into patch-336. @unklegwar - please check them out and let me know if this is more like the behavior you are expecting.

[unklegwar]

I just noticed the availability of the patch. I will try it out in the next few days, when I have a good time to switch gears for a bit.

[unklegwar]

Well, yes and no.
It now renders a menu when I say $Html.MvcSiteMap().Menu(1,1,true) even if I'm not on a page that's in the sitemap. So that's good.

But it still won't reliably partial match {controller}/{action}/{id} when the controller and action are supplied on a node, unless I use preservedRouteParameters="id". Is that still to be expected?

Here's an example from my sitemap:

<mvcSiteMapNode title="Data Management" controller="DataManagement" action="Index" securitycheckchildren="true" section="Main"  >
        <mvcSiteMapNode title="Dictionaries" controller="DataManagement" action="Dictionary" securityfeature="DataManagement:Dictionaries" rolecomponentrollup="DataAccess">
                    <mvcSiteMapNode   title="Process List" controller="Process" action="Index">
            <mvcSiteMapNode title="Edit Process" controller="Process" action="Edit" />

If I navigate to /Process/Edit/990088, the following seems to be the case:

1. 1st level menu renders. Good. But it doesn't know that "DataManagement" is current.
2. 2nd level menu renders correctly, as it seems to know it's SOMEWHERE in the DataManagement node. I get the "Dictionaries" item (and a few others at the same level under DataManagement, but it too is not selected.
3. No breadcrumbs, as it doesn't seem to match the "Edit Process" node as a fallback from the current Url. Since it can't match all the way to {id}, would it not be acceptable to match at least on {controller}/{action} without using preservedRouteParameters?

[NightOwl888]

But it still won't reliably partial match {controller}/{action}/{id} when the controller and action are supplied on a node, unless I use preservedRouteParameters="id". Is that still to be expected?

Yes. By default you are expected to make 1 node per route value combination (Cartesian product). You should do this for route values that help to identify the page. Exception: the page is protected by a login.

You can change the default behavior by using preservedRouteParameters, but this has some quirks. You get a breadcrumb trail, but that is pretty much it. You usually need to fix the title and make the node invisible on the rest of the HTML helpers and /sitemap.xml endpoint. And you need to be mindful of the values that are being copied from the current request because the current request could affect the URL of other visible nodes.

See this answer and How to Make MvcSiteMapProvider Remember a User's Position for a more detailed explanation.

No breadcrumbs, as it doesn't seem to match the "Edit Process" node as a fallback from the current Url. Since it can't match all the way to {id}, would it not be acceptable to match at least on {controller}/{action} without using preservedRouteParameters?

Not really. There is no guarantee that the {controller}/{action} node will have anything to do with the "current" node.

This breadcrumb behavior is the exact same behavior as Microsoft's design - it disappears when the current node is not registered in the sitemap. I think that this is the right choice.

In general, it doesn't make much sense to have "unregistered" pages in your site, but there could be edge cases where it makes sense. In those cases I agree with you that the menu should be shown as best as possible, but trying to guess what breadcrumb to show is a bit much to ask.

You would normally register all possible route value combinations using a dynamic node provider or by implementing ISiteMapNodeProvider so these "undefined" pages don't exist.

[unklegwar]

It's not that these are undefined pages. The page /Process/Edit is registered, but to register every possible id for /Process/Edit/{id} is not practical when you could have 100,000 ids for this page, and 10,000 for another, and 1,000,000 for yet another. Dynamic node providers would take forever to generate those nodes, and the sitemap structure would be huge. Add in that we cache the sitemap per customer (due to customer specific data being used to generate the map), and we have 100s of customers, each potentially with those kinds of numbers, and you can see that a node per id is out of the question. The only practical way is to ignore the id, because it makes no real difference to the page, and just see every permutation as the Process Edit page, regardless of which process. We will be using dynamic node providers to generate these "generic" entries for 18 different entities that can be edited and viewed, each with an id. But can't reasonably generate
the nodes for each id for each of those entities. It also seems greatly redundant to generate the same exact thing for every id, when the only thing varying is the id.
If preservedRouteParamaters is the only way to do it, then so be it. I don't really understand exactly what that's doing, only that it gets the nodes to match so my breadcrumbs show up. 

---

From: NightOwl888 [email protected]
To: maartenba/MvcSiteMapProvider [email protected]
Cc: unklegwar [email protected]
Sent: Thursday, July 24, 2014 2:59 AM
Subject: Re: [MvcSiteMapProvider] Menu won't render anything if current page is not in sitemap (#336)

But it still won't reliably partial match {controller}/{action}/{id} when the controller and action are supplied on a node, unless I use preservedRouteParameters="id". Is that still to be expected?
Yes. By default you are expected to make 1 node per route value combination (Cartesian product). You should do this for route values that help to identify the page. Exception: the page is protected by a login.
You can change the default behavior by using preservedRouteParameters, but this has some quirks. You get a breadcrumb trail, but that is pretty much it. You usually need to fix the title and make the node invisible on the rest of the HTML helpers and /sitemap.xml endpoint. And you need to be mindful of the values that are being copied from the current request because the current request could affect the URL of other visible nodes.
See this answer and How to Make MvcSiteMapProvider Remember a User's Position for a more detailed explanation.
No breadcrumbs, as it doesn't seem to match the "Edit Process" node as a fallback from the current Url. Since it can't match all the way to {id}, would it not be acceptable to match at least on {controller}/{action} without using preservedRouteParameters?
Not really. There is no guarantee that the {controller}/{action} node will have anything to do with the "current" node.
This breadcrumb behavior is the exact same behavior as Microsoft's design - it disappears when the current node is not registered in the sitemap. I think that this is the right choice.
In general, it doesn't make much sense to have "unregistered" pages in your site, but there could be edge cases where it makes sense. In those cases I agree with you that the menu should be shown as best as possible, but trying to guess what breadcrumb to show is a bit much to ask.
You would normally register all possible route value combinations using a dynamic node provider or by implementing ISiteMapNodeProvider so these "undefined" pages don't exist.
—
Reply to this email directly or view it on GitHub.

[NightOwl888]

Yes, for CRUD operations you would typically use preservedRouteParameters because these operations are generally behind a login page (so it doesn't make sense for them to appear in the /sitemap.xml endpoint). Also, you typically navigate to editable items by selecting them from a grid or list in the page content, so it isn't required to show them in the menu. See the code demos in How to Make MvcSiteMapProvider Remember a User's Position for examples.

I didn't mean to make it sound like loading all of those nodes is practical in every case, and using preservedRouteParameters can be done to trim the fat. You should aim to have less than 10,000 nodes total in MvcSiteMapProvider v4 in all sitemap instances combined. In v5, I plan to make it possible to cache the sitemap on disk as well as load segments of the sitemap independently so it will potentially support millions of nodes without consuming unreasonable amounts of resources or taking unreasonable amounts of time to load.

You should not need to cache the sitemap per customer unless the menu will have a completely different structure in each case. You can instead load every potential option into a single sitemap and then use visibility providers to control which options each customer will see. This will save you tons of RAM and prevent you from running up against a wall in terms of how many customers the application will support.

It also seems greatly redundant to generate the same exact thing for every id, when the only thing varying is the id.

The primary reason for this is if you want to use the built in sitemaps XML endpoint (/sitemap.xml) to submit all of your pages to search engines. If all of the URLs are registered in the sitemap, they can also be visible in the /sitemap.xml endpoint (thus to search engines), the menu HTML helper, and the sitemap HTML helper. If you would rather roll your own sitemap and sitemaps XML endpoint and you don't care to see all of the individual items in the menu, you could just use preservedRouteParameters and only use MvcSiteMapProvider for the breadcrumb trail and a high-level menu.

If preservedRouteParamaters is the only way to do it, then so be it. I don't really understand exactly what that's doing, only that it gets the nodes to match so my breadcrumbs show up.

In your case it is the only reasonable option. The way it works is it copies each of the values in the PreservedRouteParameters property from the current request into the node before attempting to match. This is why it shows up for every value. But it also means that if you have 2 nodes visible at the same time with "id" registered as a preserved route parameter, it will copy the value into both nodes before generating the URLs.

<mvcSiteMapNode title="Parent" controller="Home" action="Parent" preservedRouteParameters="id">
    <mvcSiteMapNode title="Child" controller="Home" action="Child" preservedRouteParameters="id"/>
</mvcSiteMapNode>

So in the above example, if you navigate to /Home/Child/1234 both nodes will show up in the breadcrumb. However, the URL of the parent node will be /Home/Parent/1234 because its "id" value is being copied from the current request. If you have another node that is visible at the same time (in the menu, breadcrumb, or any other MvcSiteMapProvider HTML helper) with preservedRouteParameters="id" defined, it will also get the value 1234. So you need to ensure that all visible nodes use the "id" for the same purpose. If you have a parameter with a different purpose, it must have a different name and/or not be visible in the same context or your navigation will be broken.

Also, there is no way to make the menu, sitemap, or /sitemaps.xml endpoint show a link with the URL /Home/Parent/1234 when using preservedRouteParameters - to generate that URL you need to rely on navigation outside of MvcSiteMapProvider.

Despite these issues (and the fact that you need to fix the title and visibility of the node manually) it is still worth it to use this method to get a breadcrumb trail for CRUD operations and for always matching ambient values (userId, sessionId, locale, etc.). Depending on how important it is to submit the URL to search engines and to show it in the menu, it may also make sense to use this method for other dynamic purposes as well.

[unklegwar]

So does this patch 336 get rolled into a release at some point?

[NightOwl888]

The patch is now available in v4.6.8.