Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Non-Area Controllers in v3.3.3.0 #78

Closed
Bitmapped opened this Issue · 19 comments

8 participants

Bitmapped Chao waynebrantley marcelopetersen Maarten Balliauw Shad Storhaug Grímur Daníelsson Kori Francis
Bitmapped

I'm running a MVC4 site using Areas. I have a default "Home" controller at the room of the site and along with "Home" controllers in each area.

My sitemap looks (in part) like:

<mvcSiteMapNode title="Administration" controller="Home" action="Index">
      <mvcSiteMapNode title="Computers" controller="Home" action="Index" area="Computers">
        <mvcSiteMapNode title="Computers" controller="Computer" action="Index">

Under MvcSiteMapProvider v3.2.3.0, this works fine. The top level generates an "Administration" link going to ~/ and the "Computers" link goes to ~/Computers.

I upgraded to v3.3.3.0. I now get an AmbiguousControllerException complaining there are multiple "Home" controllers. I tried just specifying Area="" for the "Administration" link and a couple other ideas but nothing worked.

Is there an official solution for handling controllers outside of areas or is this a bug introduced in the new version? I noticed in the diffs that the code for DefaultControllerTypeResolver changed, so I'm guessing that's what broke this.

Deleted user

Same here.

I'm not a MVCSiteMap expert, but from what I'm seeing it starts with the FindNamespacesForArea method. At the end the method returns a non null namespacesCommon array containing 3 empty strings.

So the following if statement:

if (areaNamespaces != null)
{
namespaces = new HashSet(areaNamespaces, StringComparer.OrdinalIgnoreCase);
}
else if (ControllerBuilder.Current.DefaultNamespaces.Count > 0)
{
namespaces = ControllerBuilder.Current.DefaultNamespaces;
}

uses empty namespaces instead of my default ones (which it should use)

Then in the GetControllerTypeWithinNamespaces method everything boils down to the call to IsNamespaceMatch which returns true if the passed requestedNamespace is empty. So that's why (IMO) you get more than one match if the area is null.

The if should verify that areaNamespaces is not null AND that it contains something else than empty strings. Or the empty strings should ne have been added to the namespacesCommon array in the first place.

Chao

I've also had a problem with my areas not showing in the sitemap since upgrading to 3.3.3.0.

I've gone through each commit in between 3.2.3.0 and 3.3.3.0 and the problem is coming from a fix for issue #67 which changed FindNamespacesForArea.

I'm guessing that ELMAH.MVC interacts with the area/namespacing of your project, causing the need for the change in the method, however this fix at some point seems to rely on the changes made by ELMAH as the sitemaps are now breaking for those of us not using it.

I can see three options for moving on from here.

  1. Override the DefaultControllerTypeResolver using the previous version in your web.config declarations (shortterm solution)
  2. Have the DefaultControllerTypeResolver switch behaviour based on a setting in web.config (medium term solution, I would recommend the default being the old behaviour, only those using ELMAH.MVC then need to turn it on)
  3. Someone much cleverer than myself see if they can find out how/why ELMAH.MVC is interacting with the area/namespace issue and see if you can supply a patch (long term, fingers crossed solution). This may well be an issue that will affect other projects.
Bitmapped

Perhaps there should be a way to explicitly specify in the sitemap that a controller is not part of an area.

Chao

I think having to add a "NonArea" attribute into the sitemap is very counter intuitive, it would also seem quite tricky to add to this part of the code.

I've taken another look at it after reading what minispam said.

The change in DefaultControllerTypeResolver basically boils down to adding empty strings in the namespaces, this filters through GetControllerTypeWithinNamespaces and IsNamespaceMatch and then triggers the degenerate case:

// degenerate cases
        if (requestedNamespace == null)
        {
            return false;
        }
        else if (requestedNamespace.Length == 0)
        {
            return true;
        }

Looking at how ELMAH.MVC registers (this bit here is assumption and not tested) it appears the original bug occurred on registering a non-area route with a namespace datatoken. The fix introduced took care of this by breaking the IsNamespaceMatch test.

The original bug seems to have always been there but rarely hit, whenever a non-area namespace route is added the default namespaces were not searched for controllers, now that it returns an empty string it matches ALL controllers (inside an area is unaffected as this overrides the non-area namespace).

As such I suggest the following two changes to hopefully fix both issues:

Firstly revert the change to FindNamespacesForArea to prevent the empty strings that were affecting IsNamespaceMatch.

Secondly in ResolveControllerType replace

if (areaNamespaces != null)
{
    namespaces = new HashSet<string>(areaNamespaces, StringComparer.OrdinalIgnoreCase);
}

with

if (areaNamespaces != null)
{
    namespaces = new HashSet<string>(areaNamespaces, StringComparer.OrdinalIgnoreCase);
    if (string.isNullOrEmpty(areaName))
    {
        namespaces = namespaces.Union(ControllerBuilder.Current.DefaultNamespaces, StringComparer.OrdinalIgnoreCase);
    }
}

This allows things to be added outside of an area within their own namespace without overriding the default namespaces.

Please note as of this moment the above fix is still theoretical, I haven't had time to test it. I would appreciate it if anyone else who is in a position to do so could try it out (preferably a mix of people with and without ELMAH.MVC).

Chao chaoaretasty referenced this issue from a commit in chaoaretasty/MvcSiteMapProvider
Chao chaoaretasty Fix for Issue #78, #67
The previous fix for #67 hid rather than fixed the problem and caused new issues for non-area controllers.

This fix reverts the previous edit and amends ResolveControllerType instead.

This should now allow type resolving properly in the following cases
*A non-area route provides non-default namespaces
*Non-unique controller names appear inside and outside of an area

Problems will be caused in the following case:
*When non-area routes are to check only within a given namespace and not the defaultnamespaces

This is a much less common edgecase than either above case, it is bucking the "convention over configuration" view of MVC so comes with an expectation of more potential issues.
27812eb
waynebrantley

Using the latest code, I am still getting the above issue. I have ELMAH.MVC also and it cannot find the routes without an area. I took the step of adding the namespace to my default route and now it finds it, but the above bug still happens to me. I have 2.0.1 installed of ELMAH.MVC - I see they have 2.0.2 out. I may just remove this - as I don't really need it, can just configure ELMAH manually. Thoughts?

Bitmapped

I just upgraded to v3.3.4.0 from NuGet, which includes Pull Request #88. Non-area controllers appear to be working again, although I don't have ELMAH installed.

marcelopetersen

I'm using the latest version got from nuget, 3.3.4 and the issue continues to occur.

Found multiple controllers:Home

[AmbiguousControllerException: Found multiple controllers:Home]
MvcSiteMapProvider.DefaultControllerTypeResolver.GetControllerTypeWithinNamespaces(String area, String controller, HashSet`1 namespaces) +629

Maarten Balliauw
Owner
marcelopetersen

Yes, I've declared all my routes with namespaces

routes.MapRoute(
name: "Default",
url: "{controller}/{action}/{id}",
defaults: new
{
controller = "Home",
action = "Index",
id = UrlParameter.Optional
},
namespaces: new[] { "MyProject.WebSite.Controllers" }
);

routes.MapRoute(
name: "ProductDetailsFull",
url: "{controller}/{action}/{idProduct}/{nameProduct}",
defaults: new
{
controller = "Products",
action = "Details",
idProduct = UrlParameter.Optional,
nameProduct = UrlParameter.Optional
},
namespaces: new[] { "MyProject.WebSite.Controllers" }
);

routes.MapRoute(
name: "Administration",
url: "Administration/{controller}/{action}/{id}",
defaults: new
{
controller = "Login",
action = "Index",
id = UrlParameter.Optional
},
namespaces: new[] { "MyProject.WebSite.Areas.Administration.Controllers" }
);

Bitmapped

Have you tried not explicitly specifying the namespace for your routes? I'm not in my project and v3.3.4.0 (no ELMAH) works OK for me.

Shad Storhaug
Collaborator

@Bitmapped

I added a patch to fix compatibility issues with ELMAH, which basically just filters out Elmah.Mvc as a valid area namespace to check.

I would appreciate if you would use the code in the master branch to see if you are still having the issues described in the original post. If they are gone, I will try to urge Maarten to build another v3 Nuget release and close this issue. I am sure there are others waiting for an official ELMAH fix.

I am using ELMAH now in my project with the latest v4 build and I am not seeing any issues.

Bitmapped

I installed ELMAH earlier this year and it was working fine for me with v3.3.4. I just grabbed the new v3.3.5, which was built yesterday, and it appears to be OK as well.

Shad Storhaug
Collaborator

Well, Maarten said that he would be making another build tomorrow as we had another (related) patch, which only affects people who don't have any routes with namespaces declared in them. So I am sure you will see a v3.3.6 soon.

That said, looks like this issue is closed.

Grímur Daníelsson

I'm experiencing this issue with the latest build, 3.3.6.0 when trying to create a sitemap. I have a Admin area, routes have been defined in RouteBundle.cs and i also have Elmah.

This is how my sitemap looks like at the moment:

<mvcSiteMapNode title="Home" controller="Home" action="Index" area="">
  <mvcSiteMapNode title="Control Panel" controller="Home" action="Index" area="Admin">
    <mvcSiteMapNode title="Images" controller="Home" action="Images" />
  </mvcSiteMapNode>
</mvcSiteMapNode>

My routes:

routes.MapRoute(
                name: "Default",
                url: "{controller}/{action}/{id}",
                defaults: new { controller = "Home", action = "Index", id = UrlParameter.Optional },
                constraints: null,
                namespaces: new string[] { "Project.Web.Controllers" }

            );

            routes.MapRoute(
                name: "Admin",
                url: "Admin/{controller}/{action}/{id}",
                defaults: new { controller = "Home", action = "Index", id = UrlParameter.Optional },
                namespaces: new[] { "Project.Web.Areas.Admin.Controllers" }

            );

This is the exception i'm getting: MvcSiteMapProvider.AmbiguousControllerException: Found multiple controllers:Home

Update: Getting the same error after updating to the MvcSiteMap 4 beta. But the exception did change to: Ambiguous controller. Found multiple controller types for HomeController. Consider narrowing the places to search by adding you controller namespaces to ControllerBuilder.Current.DefaultNamespaces.

Shad Storhaug NightOwl888 reopened this
Shad Storhaug
Collaborator

This issue was happening with Elmah because Elmah registers an area with the namespace "Elmah.Mvc" (case sensitive) that was being picked up by the ControllerTypeResolver. The solution for v3 and v4 was to hard code this namespace as an exclusion (after all, Elmah is popular). In v4, we have also added a configuration setting so you can exclude any other namespaces that have this conflict in your project.

For internal DI, you can add the namespaces to the "MvcSiteMapProvider_ExcludeNamespacesForResolver" appSettings value (pipe or semicolon delimited). For external DI, you can provide this list through the areaNamespacesToIgnore parameter (IEnumerable<string>) of ControllerTypeResolverFactory in the DI configuration. This will exclude namespaces from being considered in the resolution of the controller type.

ControllerBuilder.Current.DefaultNamespaces has the opposite effect - you can include namespaces for consideration in the match.

Your issue no doubt is being caused by ControllerTypeResolver, but without knowing more about your specific configuration it is hard to say exactly what the problem might be.

You will most likely be able to work around this problem by adjusting the configuration of one or both of the settings above.

Either pull the source from GitHub, compile it with debug symbols, and step through the code in ControllerTypeResolver so you can get a better idea what is going on, or create a small demo project exhibiting the problem and post it on GitHub so we can do the same.

In the meantime, it looks like it would be very helpful to add the names of the matching types that are causing the conflict to the error message, so I will make sure that gets fixed.

Shad Storhaug
Collaborator

Ok, that took some digging because I have never used areas but I have confirmed this to be problem with your configuration, not a bug. It was easy to reproduce your issue, but hard to find the solution.

You are not registering the area route correctly. If you use the convention-based area structure, it will indeed work. The route needs to be in the /Areas/Admin/AdminAreaRegistration.cs file, like this:

using System.Web.Mvc;

namespace MvcSiteMapProvider_78.Areas.Admin
{
    public class AdminAreaRegistration : AreaRegistration
    {
        public override string AreaName
        {
            get
            {
                return "Admin";
            }
        }

        public override void RegisterArea(AreaRegistrationContext context)
        {
            context.MapRoute(
                "Admin_default",
                "Admin/{controller}/{action}/{id}",
                new { controller = "Home", action = "Index", id = UrlParameter.Optional },
                new string[] { "MvcSiteMapProvider_78.Areas.Admin.Controllers" }
            );
        }
    }
}

IMPORTANT: If you use visual studio to add your area, it will set this up for you. However, it doesn't set up the default controller for you, so be aware you may need to add it manually for the default /Admin path to work. Otherwise, it can only be resolved at /Admin/Home.

Also, it is important to put your views and controllers in the right folders:

/Areas/Admin/Controllers
/Areas/Admin/Views/[ControllerName]

Finally, you need to ensure that the line AreaRegistration.RegisterAllAreas(); is added before everything else in Application_Start in your Global.asax file in order to pick up the AreaRegistration subclass.

To see a complete example of this in action, have a look at the MvcMusicStore demo, as it is using areas already, and has 2 different home controllers.

Also, I have fixed the exception so it shows the types that are ambiguous in the error message, which will be in the next build.

Grímur Daníelsson

Thank you for the quick answer NightOwl888. How you explain it is actually like it was set up before i started trying to fix it myself. Since posting this I've gone back to the original configuration but i was still getting this error.

However when i tried it out just now everything works just fine. Got this error constantly last night so i commented the code and started working on other things. When i uncommented it this morning suddenly everything works.

So my issue seems to have been solved by itself(or i solved it unknowingly by changing some other code in the site). Thanks for taking the time to look into it.

Kori Francis

I only started seeing this issue after upgrading from MVC3 to MVC4 and changing the namespace of the project from Project.MVC3.* to Project.MVC4.*. "Found multiple controllers". I am namespacing the routes, but it's still having no effect.

Shad Storhaug
Collaborator

I am not sure if it is specific to MVC4, and I am pretty sure this behavior should be the same in MvcSiteMapProvider v3 as v4, but as I pointed out here, the area routes must be registered using the correct MVC conventions or it won't work. I couldn't find any other method that would work except that specific configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.