maartenba / MvcSiteMapProvider

An ASP.NET MVC SiteMapProvider implementation for the ASP.NET MVC framework.
Microsoft Public License
538 stars 218 forks source link

Fixed problem with incorrect GetControllerSessionBehavior accessor #386

Closed ghost closed 9 years ago

ghost commented 9 years ago

The accessor for the "GetControllerSessionBehavior"-method in ControllerFactoryDecorator should be protected override instead of public, cause it overrides the protected internal virtual method from DefaultControllerFactory. Since it's now public it's never called and that results in the this.innerControllerFactory.GetControllerSessionBehavior() never being called.

NightOwl888 commented 9 years ago

Thanks for the pull request.

However, your solution is incorrect because GetControllerSessionBehavior is part of the public interface of IControllerFactory, therefore it must be declared as public to satisfy the interface contract.

This GetControllerSessionBehavior method is correctly calling the GetControllerSessionBehavior of the inner controller factory (the one that is configured in MVC, which may be the DefaultControllerFactory).

If you take a careful look at the DefaultControllerFactory source code, you will notice that there are 2 declarations of GetControllerSessionBehavior - 1 public and one protected virtual. The public one is the only one that the interface IControllerFactory is concerned with.

But then, you probably already noticed because you have already closed the request. Thanks for contributing, anyway.