2

Closed

RouteCollectionExtensions.MapRoute does not work when defaults or constraints are of type IDictionary<string,object>

description

Using the following code:
RouteCollection routes = new RouteCollection();
routes.MapRoute(
    name: "FooRoute",
    url: "Foo/{Foo}",
    defaults: new RouteValueDictionary { { "Foo", "Bar" } }
);
Expected: routes should contain 1 route, with 1 default:
Foo = Bar

Actual: routes contains 1 route with the following defaults:
Count = 1
Keys = { "Foo" }
Value = { "Bar" }
Closed Jul 19, 2012 at 12:43 AM by hongyes
Verified

comments

kayub wrote Apr 5, 2012 at 12:38 PM

Nice catch

eilonlipton wrote Apr 9, 2012 at 10:21 PM

The MapRoute extension method is only designed to take in so-called "object dictionaries". If you want to pass in other types of dictionaries (such as an actual IDictionary<K,V>) you can instantiate the route yourself and call routes.Add(new Route(...)).

PatrickMcDonald wrote Apr 10, 2012 at 8:04 PM

I agree that we can just use routes.Add(new Route(...)), but then why do we have RouteCollection.MapRoute at all?

What about AreaRegistrationContext.MapRoute? This calls RouteCollection.MapRoute internally, so to create a route in an area with defaults or constraints of type IDictionary<TKey, TValue> we need to re-implement AreaRegistrationContext.MapRoute in our own code.

eilonlipton wrote Apr 24, 2012 at 5:04 PM

OK that's a fair argument :)

If we update MVC's RouteCollectionExtensions we should also modify Web API's RouteCollectionExtensions. They follow almost exactly the same pattern as the MVC one, but have "Http" in the API names. Can you include that in the change?

PatrickMcDonald wrote Apr 24, 2012 at 8:05 PM

Thanks Eilon.

I've added changes to the pull request that address the other RouteCollectionExtensions (in System.Web.Http and System.Web.Http.WebHost).

I took the approach I did as I wasn't sure about changing the API, however if you prefer I could add additional overloads to the MapRoute and MapHttpRoute methods and skip the casting internally in the existing methods. Let me know if you want me to do any additional refactoring.

(Also re-reading my previous comment, I hope if it didn't seem a bit "confrontational", it wasn't meant to :)

eilonlipton wrote May 2, 2012 at 5:03 PM

The updated pull request looks good! It's an interesting question about whether to add overloads or to do it more dynamically as you've done. I kind of prefer it the way you have it because it means fewer overloads, and in the end more flexibility (because you can mix and match dictionaries and objects).

We'll go ahead an accept this pull request ASAP - thanks for the contribution!

Sign in to add a comment