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

Route Data related bug dependent on first request #154

Closed
mdsharpe opened this issue Apr 16, 2013 · 20 comments
Closed

Route Data related bug dependent on first request #154

mdsharpe opened this issue Apr 16, 2013 · 20 comments

Comments

@mdsharpe
Copy link

Hi,

I am attempting to use the MvcSiteMapProvider with an MVC 4 site, and am having a problem in a section of the site which has a Project Listing page and child Project Detail page.

When you access the Detail page, the ID of the entity selected from the Listing is inserted into the URL e.g.:
~/Project/2/Detail

I have not made the Detail page a Dynamic node, as I do not want the SiteMap to provide any enhanced functionality. Essentially I want the Site Map Provider to ignore the ProjectID URL parameter.

Unfortunately, I am having the following problem:
If the app starts with the first request being for the Listing page, then the user can click through to any project and the SiteMap matches the Detail page just fine.
If the app starts with a request for the Detail page, then the SiteMap only matches the Detail page if the URL contains the ProjectID of the first request, i.e. if you visit any other Project then the SiteMap currentnode is null.

I have tried to disable caching, enable long caching, and have even been digging around in the source code with no success. The inconsistent behaviour suggests it is a bug. Any help would be greatly appreciated.

Regards,
Matt Sharpe

@NightOwl888
Copy link
Collaborator

Thanks for the report. If there is a bug, I hope we can track it down.

First of all, which control are you seeing the inconsistent behavior in (SiteMapPath, Menu, other)?

Secondly, what exactly do you mean by "first request"? The site map tree should be built the same way regardless of the URL that is hit to build it.

Third, could you try using the code from the v4 branch. We have done a lot of refactoring in the new design and found and fixed several bugs along the way.

Finally, if the problem still exists in the current version, could you post some code, or better yet put a sample project on GitHub to demonstrate the problem? I take it by your URL you are not using the default route - seeing it as well as your XML could prove helpful.

@mdsharpe
Copy link
Author

Thank you very much for your quick response!

  1. In this particular project I am using my own Breadcrumbs partial view, and am seeing a problem in that SiteMap.CurrentNode is null. I suspect it would affect any control that accessed the SiteMap.
  2. By first request I mean the first request that hits the server and starts up the app domain. This may be after a rebuild, after restarting IIS, or even just after saving Web.config.
  3. I will try this. Do you publish v4 builds or would I have to get the source and build it myself? Is there a way to get v4 using NuGet?

Thanks.

@NightOwl888
Copy link
Collaborator

v4 is feature complete, but we are still working on the Nuget packaging. You will need to pull it from GitHub and compile it yourself - this can be done using the Visual Studio IDE. You indicated you are using MVC4, so you will need to set the compiler constants (Properties -> Build tab) to:

MVC4;NET40

Note that the second constant will change automatically when you change the .NET version on the Application tab.

I just noticed that we don't have this set up right for .NET 4.5, so for now just compile in .NET 4.0 and everything should work fine. I have been using it this way on an MVC 4 site for some time and have not had any issues.

v4 is set up to use a dependency injection container, but by default it uses a built-in container that defaults to using an XML file named ~/Mvc.sitemap as its source when no configuration values are supplied. To configure it in another way (including farming out the entire configuration to an external dependency injection container), you can set any of the settings found in the /DI/ConfigurationSettings.cs file in the appsettings section of the Web.config.

Do also note that there is a new schema that requires changes to the namespace section of your XML file, but other than that should work with a v3 XML configuration just fine. Setting the security trimming is now done directly in the .sitemap file instead of Web.config. See the MvcMusicStore project for a sample of how to set it up.

@NightOwl888
Copy link
Collaborator

FYI - I just pushed a patch to fix compiling the project under .NET 4.5.

@mdsharpe
Copy link
Author

Unfortunately I'm having a pretty hard time swapping v4 into my project.
Is it still possible to use it with the web.config configuration technique?

@mdsharpe
Copy link
Author

Okay so I have got v4 going in my project now. The behaviour has indeed changed, as follows:

  • If the app starts with the first request being for the Listing page, then the user can click through to any project and the SiteMap matches the Detail page, however the projectId for all child nodes is 0.
  • If the app starts with a request for the Detail page, then when viewing any Project the SiteMap matches the Detail page but the URL contains the ProjectID of the first request.

@NightOwl888
Copy link
Collaborator

Nope - the new design is not based on the .NET framework implementation of SiteMapProvider, so the configuration section must be removed entirely.

Also, I noticed when trying to make the new version work on MvcMusicStore that .NET doesn't care whether you have commented the calls to the old asp:Menu framework in your views. If they are not completely removed from the views, .NET will scan for a SiteMapProvider and when not found, will throw an exception. If you are using Razor, this won't be a problem, though.

The MvcMusicStore demo should now run on any machine with no configuration - we changed the back end to be a mock database instead of a real one so no connection string is necessary. If you can get that running, then you can compare the configurations and then resolve the issues with your own app.

Just double-check your Web.config (both of them) to ensure the right namespaces are in place. You shouldn't need anything in Global.asax, as the default implementation uses WebActivator to launch (as long as you have no MvcSitemapProvider_... settings in your Web.config). Also, double-check the namespaces in your custom partial views against the ones inside the MvcSiteMapProvider/Web/Html/DisplayTemplates/.

@NightOwl888
Copy link
Collaborator

Oops - a day late and a dollar short :).

Could you post your routes and XML?

@mdsharpe
Copy link
Author

Better - I have prepared a repro project:

https://skydrive.live.com/redir?resid=B1F44579DED4C374!246&authkey=!ALanOcfoxQluN2c

The project is using the v4 PreRelase NuGet package, however I have confirmed that behaviour is the same with the latest build from source.

@mdsharpe
Copy link
Author

OK, I don't want to speak too soon but I may have resolved the problem. I am back on v3 and have configured my route and XML as follows, and it's now working as I need it to:

            context.MapRoute(
                "Project_default",
                "Project/{projectId}/{controller}/{action}/{id}",
                new { controller = "Home", action = "Index", id = UrlParameter.Optional },
                new { projectId = @"\d+" });
<mvcSiteMapNode title="Project"
                            area="Project"
                            controller="Home"
                            action="Index"
                            projectId="0"
                            preservedRouteParameters="projectId">
                <mvcSiteMapNode title="Summary"
                                area="Project"
                                controller="Home"
                                action="Summary"
                                inheritedRouteParameters="projectId"
                                preservedRouteParameters="projectId">

@NightOwl888
Copy link
Collaborator

That's great news. I was wondering what the preserved route parameters were for, now I think I have an answer :).

I tried your changes using v4 and your demo project, but they don't seem to be working right. Is this all you did, or was there more?

@mdsharpe
Copy link
Author

Hmm, I just tried it on my demo project too, and I can see that it doesn't work - the site map nodes always have projectId as 0.
I'm afraid at this time I probably won't spend any longer looking into it for v4 - I've already spent a lot of time on this problem - and will be sticking with v3 for now.
Thank you very much for your help. I will be keeping an eye on the progress of MvcSiteMapProvider and look forward to future releases :-)
Matt

@NightOwl888
Copy link
Collaborator

No problem. At least you have identified a bug that we need to fix before v4 can be released. Good luck!

@NightOwl888
Copy link
Collaborator

@maartenba

This is a pretty serious bug - I'd call it a show stopper. I suspect it has something to do with the fact that the v3 provider is magically provided with route data in its Attributes collection. For v4, I couldn't figure out what was supplying this data (ASP.NET, MVC, Routing, etc), but whatever is doing this doesn't appear to be part of MvcSiteMapProvider.

I'd appreciate if you could take a look and let me know how you think the current behavior can be mimicked. If you step through the code in the SiteMapPath in v3, you can see that the Attributes collection for each node is supplied with area, controller, action, and id parameters. However, in v4 they are missing. The id is somehow magically filled in even if it is not the current node - right now I don't have any theory how that could be done.

It would help a lot if I could locate the code that is producing this data and copy it outright, but its location eludes me.

@maartenba
Copy link
Owner

DefaultSiteMapProvider.AttributesToRouteValues is taking part in this in v3. (line 1547)

inheritedRouteParameters and preservedRouteParameters are also used to specify which route values must be cloned from the parent node or the current HttpContext.

Can we add another visitor on top which uses these sources to specify the data on the node?

@NightOwl888
Copy link
Collaborator

Ok, I see where the attributes are being populated with route data in v3. Question: This was clearly a kludge before brought about because Attributes was the only way to add custom data to the node. Is there any reason why all of the route data can't be put into the route values collection? This could potentially make the comparison whether we have a "match" much easier by only comparing 1 collection instead of 2. Or is there a specific reason why they are separate?

However, I don't see where the values are being brought in from HttpContext (specifically, the id or custom values of the route which are present in v3). Also, that would have to happen per request, wouldn't it? I mean, HttpContext only deals with the current request. Everything during the builder stage (including visitors) is pre-cache, so it only would be done during application start, not per-request.

Yes, we can add as many visitors (or more builders) as needed using the composite pattern, but I haven't thought of a good way to inherit parent values yet. Ideally, we would be able to inherit almost any XML attribute, but does it also make sense to inherit in dynamic nodes and nodes that are defined by attributes?

@maartenba
Copy link
Owner

The reason for having two ha just grown in the product historically. It all started off with attributes and then made sense to have route values in there (but did not drop attributes because of backwards compatibility).

Can’t track the source of the data down either :(

https://github.com/maartenba/MvcSiteMapProvider/blob/master/src/MvcSiteMapProvider/MvcSiteMapProvider/MvcSiteMapNode.cs (GetRouteData method) seems to do some work https://github.com/maartenba/MvcSiteMapProvider/blob/master/src/MvcSiteMapProvider/MvcSiteMapProvider/Web/Html/SiteMapNodeModelMapper.cs seems to do some work there but not in Attributes.

From: NightOwl888 [mailto:[email protected]]
Sent: woensdag 17 april 2013 21:29
To: maartenba/MvcSiteMapProvider
Cc: Maarten Balliauw
Subject: Re: [MvcSiteMapProvider] Route Data related bug dependent on first request (#154)

Ok, I see where the attributes are being populated with route data in v3. Question: This was clearly a kludge before brought about because Attributes was the only way to add custom data to the node. Is there any reason why all of the route data can't be put into the route values collection? This could potentially make the comparison whether we have a "match" much easier by only comparing 1 collection instead of 2. Or is there a specific reason why they are separate?
However, I don't see where the values are being brought in from HttpContext (specifically, the id or custom values of the route which are present in v3). Also, that would have to happen per request, wouldn't it? I mean, HttpContext only deals with the current request. Everything during the builder stage (including visitors) is pre-cache, so it only would be done during application start, not per-request.
Yes, we can add as many visitors (or more builders) as needed using the composite pattern, but I haven't thought of a good way to inherit parent values yet. Ideally, we would be able to inherit almost any XML attribute, but does it also make sense to inherit in dynamic nodes and nodes that are defined by attributes?

Reply to this email directly or #154 (comment) view it on GitHub. https://github.com/notifications/beacon/OgHlEtub9rJzfLSOAmMnQFIeJ_sZ_OU0ggvYxGrn2MCcEnhnX5KoBf-wOp5F9jk1.gif

@NightOwl888
Copy link
Collaborator

@maartenba

I have been giving this some thought and I think I know what is wrong:

  1. The attributes collection and route values collection is being populated from the XML, not the current context like I assumed.
  2. The inheritance logic needs to be moved to a common location (services that are called from the SiteMapNodeFactory), as step 1 depends on this logic. This will ensure that inheritance works consistently across all builders, including dynamic nodes.
  3. The inheritance logic should be always on for route values and have an "inheritedRouteValuesToIgnore" attribute, rather than an "inheritedRouteParameters" attribute.
  4. The inheritance logic for all other attributes should be off by default and have an attribute "attributesToInherit" for which ones to include. The logic needs to take into consideration the properties that are not stored in the attributes collection.
  5. The logic that gets the preserved route parameters and puts them into the route values collection is missing from v4. This logic needs to run in the Get accessor for the collection so it does not affect, nor is affected by the cache.
  6. The comparison logic for matching the routes must only depend on route values, not on attributes.
  7. The comparison logic for matching the routes needs to be fixed so it understands that an exact match takes priority over a match with optional parameters. Essentially, it needs to follow the matching convention that MVC uses.

The reason why this was so hard to track down was because it is a combination of issues that causes the problem.

I will work on fixing this as soon as I can. Let me know if you can think of anything else that I may have missed.

@maartenba
Copy link
Owner

Logic seems okay.

NightOwl888 added a commit that referenced this issue Jun 1, 2013
…e preserve functionality from SiteMapNodeUrlResolver to SiteMapNode. Fixed DynamicNode so it will merge preserved route params correctly.
@NightOwl888
Copy link
Collaborator

This issue is now fixed in v4.

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

3 participants