ASP.NET MVC: Current route values implicitly used by Url.Action

I really don't like when I work with statically-typed language like C# and still need to rely on strings or other loosely-typed constructs for things that may be strongly-typed. ASP.NET MVC went its way towards strongly-typing and now offers things like strongly-typed view models, but still lacks official built-in support for building internal links and URLs without using plain controller and action strings.

Fortunately, here is where MVC Futures comes in (available on NuGet for MVC 3 or 4). It offers a nice expression-based extension methods to build links (on HtmlHelper):

Html.ActionLink<TheController>(c => c.TheAction(theParam))

and URLs (on UrlHelper):

Url.Action<TheController>(c => c.TheAction(theParam))

It's working fine and it protects me from working with those ugly string-based methods and it is also a very convenient way to specify the target action parameters - in definitely nicer way then manually building the RouteValuesCollection required by those traditional methods.

But there is one not well-known feature of MVC (at least I haven't heard a lot about it) that for me seems to be disturbing here - MVC is reusing the route values from the current request when building URLs and no other value specified. This maybe makes some sense for the string-based methods (you don't need to build those RouteValuesCollections over and over again), but it makes no sense for the expression-based approach as the compiler enforces specifying all the parameters explicitly in order to build a lambda expression that compiles.

And here comes the nasty bug I've recently spent some hours on - passing null value is like not passing a value at all. MVC puts null under the appropriate key in the RouteValuesCollection, it then goes down to Route.GetVirtualPath method, which also takes the current route values from the RequestContext. And then the evil happens - the low-level System.Web.Routing's method ParsedRoute.Bind ignores the null value and - bang - it takes the value from the current request, if any accidentally matches by the parameter name.

It means that when trying to build an URL that passes parameter param equal to null and the current request accidentally have parameter named also param (regardless of its type or existence of any logical connection), the request's param value will be passed instead our explicitly demanded null value. And I can see no way to pass null in this case.

Actually, the bug (or feature?) exists in case of string-based URL builder methods, too. But here it is much more visible and obviously wrong.

The only way to fix that strange implicit parameter inheritance by name I know is to work around it - either by removing the name collision by renaming one of the parameters (yuck!) or by using own extension method.

I've created my own extension method to generate URLs/links that has the same signature as the buggy ones and placed it in the namespace more visible than those replaced (you'd better check your usings carefully). Here is the UrlHelper extension I have - HtmlHelper implementation generally calls UrlHelper and wraps its result in a link, so I'll omit it here. The method calls the same methods as the original method being replaced, but it tweaks the RequestContext instance: instead of passing the instance available in UrlHelper (which contains those conflicting route values from the current request), I'm creating new RequestContext reusing Route and IRouteHandler instances from the current request, leaving the actual route values empty. This way there's no possibility for the current values to "infect" our URL building process anymore. Interesting is that RouteData constructor doesn't enforce the values being set, anyway.

public static string Action<TController>(this UrlHelper helper, Expression<Action<TController>> action)
    where TController : Controller
{
    // we need to recreate RequestContext without values, to override the MVC "feature"
    // that replaces null specified in action with value inherited from current request
    var currentRouteData = helper.RequestContext.RouteData;
    var fixedRouteData = new RouteData(currentRouteData.Route, currentRouteData.RouteHandler);
    var fixedRequestContext = new RequestContext(helper.RequestContext.HttpContext, fixedRouteData);

    var valuesFromExpr = Microsoft.Web.Mvc.Internal.ExpressionHelper.GetRouteValuesFromExpression(action);
    var vpd = helper.RouteCollection.GetVirtualPathForArea(fixedRequestContext, valuesFromExpr);
    return vpd == null ? null : vpd.VirtualPath;
}

Well, it seems to be yet another example of the fact that null is rather vague and problematic thing. It can represent many different notions, depending on the context and implementation - like no value, empty value, inherited value etc. Althouth ASP.NET MVC treats null as "inherit the value" not only in this case, I'd always prefer being explicit about that kind of behaviors.

Json.NET deserialization and initialization in constructors

I've recently run into a quite interesting problem when using Json.NET library. It shows up with a static lookup collection being modified during the deserialization of some objects. Although the behavior I've encountered is documented, but for me it is breaking a principle of least astonishment a bit, so I've decided to share.

I have a class, TestClass, that I'm going to serialize and deserialize using Json.NET. It contains a simple collection of string values, that is initialized in the constructor to contain some predefined values - it's is default state. I have these values defined somewhere in the separate class in a collection marked as readonly, treated like a constant, not supposed to be modified.

Here are the tests (written in Machine.Specifications) that illustrates the issue. I'm setting TestClass state in the constructor, but I expect it to be overwritten during the deserialization, as my JSON string contains different data. In fact, deserialized values are appended to the existing collection, which occurs to be exactly the same collection as my "constants".

public static class Constants
{
    public static readonly IList<string> NotSupposedToBeModified = new List<string>()
    {
        "the first",
        "the last"
    };
}

public class TestClass
{
    public IEnumerable<string> TheCollection { get; set; }

    public TestClass()
    {
        TheCollection = Constants.NotSupposedToBeModified;
    }
}

public class DeserializingTest
{
    Because of = () =>
        result = JsonConvert.DeserializeObject<TestClass>(@"{""TheCollection"":[""other""]}");

    It should_deserialize_the_collection_correctly = () =>
        result.TheCollection.ShouldContainOnly("other");

    It should_not_modify_the_constant_collection = () =>
        Constants.NotSupposedToBeModified.ShouldContainOnly("the first", "the last");

   static TestClass result;
}

What is the result? Both tests failed:

should deserialize the collection correctly : Failed
     Should contain only: { "other" }
     entire list: { "the first", "the last", "other" }

should not modify the constant collection : Failed
     Should contain only: { "the first", "the last" }
     entire list: { "the first",  "the last", "other" }

There are three separate issues that showed up together and resulted that apparently surprising behavior:

The first one is about the constant collection defined in Constants class, being not really constant. The readonly keyword guarantees that one can not replace the collection instance, but that already-created collection instance itself still can be modified normally. It's pretty clear, but can be missed at the first sight.

The second one is even more obvious - the assignment in TestClass's constructor doesn't initialize the local collection with values from the Constants class - it just assigns the reference to exactly the same collection. So, as the assigned collection can be modified and we've just assigned it to our TestsClass instance, we already have the doors open to modify the "constant" collection by mistake.

And finally, what the Json.NET deserializer is doing here? The documentation states: "By default Json.NET will attempt to set JSON values onto existing objects and add JSON values to existing collections during deserialization.". It means that when the collection instance for TheCollection property was already created by the constructor (well, actually not created but "borrowed" from Constants class), Json.NET doesn't create a new one and just appends deserialized values to the existing collection, modifying our NotSupposedToBeModified collection.

Well, the first two pitfalls are pretty easy, but I wouldn't expect the third one. Fortunately, Json.NET provides an easy way to customize its behavior in this matter using ObjectCreationHandling option. One simple addition in DeserializeObject method and we have two green tests (even if the first two issues are still there):

result = JsonConvert.DeserializeObject<TestClass>(
     @"{""TheCollection"":[""other""]}",
     new JsonSerializerSettings() { ObjectCreationHandling = ObjectCreationHandling.Replace });