Yesterday I blogged about using form button names to select action methods on an ASP.NET MVC controller. After using that attribute for a while I realized that I can make a generalization out of that: why not use any form value in the action method selection process? This way I can avoid code like this:

[HttpPost]
public ActionResult Fruit(SampleModel model) {
    if (!ModelState.IsValid) {
        return View(model);
    }
    if (model.IsApple) {
        // Do something
        EatApple(model);
    }
    else {
        // Do something else
        SmashOthers(model);
    }
    TempData["Message"] = "Saved";
    return RedirectToAction("Index");
}

That is not too bad, but when more choices are introduced it gets worse and worse (i.e. more if-then-else’s). Solution for this is very similar with the ButtonAttribute approach: introduce an attribute that takes form element name, and list of accepted values. If there is a match the action method is selected, otherwise the search continues. Usage example:

[HttpPost]
[ActionName("Fruit")]
[FormValue("SelectedFruit", "Apple")]
public ActionResult Apple(FormValueModel model)
{
    // TODO: Validate and act on data, then redirect; this is just a sample
    EatApples(model);    
    ViewBag.Message = "Selected: " + model.SelectedFruit;
    return View(model);
}

[HttpPost]
[ActionName("Fruit")]
[FormValue("SelectedFruit", "Pear", "Orange")]
public ActionResult PearOrOrange(FormValueModel model)
{
    // TODO: Validate and act on data, then redirect; this is just a sample
    SmashOther(model);
    ViewBag.Message = "Selected: " + model.SelectedOFruit;
    return View(model);
}

Beware, that this method has two shortcomings:

  1. The order of attributes is very important. If you change it you very easily get a 404.
  2. There must be no ambiguity on selections or otherwise ASP.NET MVC will give you an error. Usually this happens if you attempt to create one action method that picks only some values, and then another without FormValue attribute that picks all the rest; when MVC framework faces this situation and input values match the first, FormValue-decorated action method, it also matches the one without decoration -> error. To fix this attributes must cover the whole set of choices.

Regardless of the shortcomings I still think this is yet another valuable tool to keep my controllers thin.

Here is the full listing for FormValueAttribute:

using System;
using System.Collections.Generic;
using System.Web.Mvc;

/// <summary>
/// Defines an action method selector that checks form values. If form item 
/// name and value matches, the action method is selected.
/// </summary>
[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]
public sealed class FormValueAttribute : ActionMethodSelectorAttribute
{
    private List<string> valuesField;

    /// <summary>
    /// Initializes a new instance of the <see cref="FormValueAttribute"/> class.
    /// </summary>
    public FormValueAttribute()
    {
    }

    /// <summary>
    /// Initializes a new instance of the <see cref="FormValueAttribute"/> class.
    /// </summary>
    /// <param name="name">The form element name.</param>
    /// <param name="values">The values, one of these should match.</param>
    public FormValueAttribute(string name, params string[] values)
    {
        if (string.IsNullOrEmpty(name))
        {
            throw new ArgumentNullException("name", "Must specify a form element name.");
        }

        if (values == null || values.Length == 0)
        {
            throw new ArgumentNullException("values", "Must specify at least one value.");
        }

        this.Name = name;
        this.valuesField = new List<string>(values);
    }

    /// <summary>
    /// Gets or sets the form field name.
    /// </summary>
    /// <value>The name.</value>
    public string Name { get; set; }

    /// <summary>
    /// Gets form item values. These are the values of the form item <see cref="Name"/> 
    /// that should match in order this action method to be selected. 
    /// </summary>
    public IList<string> Values
    {
        get
        {
            if (this.valuesField == null)
            {
                this.valuesField = new List<string>();
            }

            return this.valuesField;
        }
    }

    /// <summary>
    /// Checks whether the given controller action method is valid for execution based on 
    /// given form field <see cref="Name"/> and <see cref="Values"/>.
    /// </summary>
    /// <param name="controllerContext">Information about the controller.</param>
    /// <param name="methodInfo">Information about the method.</param>
    /// <returns>True if valid.</returns>
    public override bool IsValidForRequest(
        ControllerContext controllerContext,
        System.Reflection.MethodInfo methodInfo)
    {
        if (controllerContext == null)
        {
            throw new ArgumentNullException("controllerContext", "Context can't be null.");
        }

        if (methodInfo == null)
        {
            throw new ArgumentNullException("methodInfo", "Method info can't be null.");
        }

        this.GuardState();

        bool returnValue = false;

        // The way of getting only the name without any prefixes might 
        // be incorrect on some situation since there might be a hierarchy 
        // of values in the form Some.Other.Value
        ValueProviderResult result = controllerContext.Controller.ValueProvider.GetValue(this.Name);

        if (result != null)
        {
            // check if any of the values match
            foreach (var value in this.Values)
            {
                returnValue = string.Equals(value, result.AttemptedValue as string);

                if (returnValue)
                {
                    break;
                }
            }
        }

        return returnValue;
    }

    private void GuardState()
    {
        if (string.IsNullOrEmpty(this.Name))
        {
            throw new InvalidOperationException(
                "Form field name must be initialized to non empty value");
        }

        if (this.Values == null || this.Values.Count == 0)
        {
            throw new InvalidOperationException(
                "Form field match values must be initialized.");
        }
    }
}

Very common problem in form-heavy solutions is to be able to have one HTML form, but two different submit buttons for it. Think “Save” and “Delete”:

<div id="buttons">
    <button name="Save">Save</button>
    <button name="Delete">Delete</button>
</div>

On controller side this poses a problem: these actions do very different tasks; Save usually validates data and saves it, Delete does not validate and deletes (and so forth). They might also have different access rights. So having these on one action breaks (at least my) principles. Fortunately ASP.NET MVC has a thing called ActionMethodSelectorAttribute that you can extend. With that you can divide the logic into separate action methods, like:

[HttpPost]
[ActionName("Edit")]
[Button("Save")]
public ActionResult Save(SampleModel model)
{
    // TODO: Save and redirect, this is just a sample
    ViewBag.Message = "Save button clicked and Save action method selected.";
    return View(model);
}
[HttpPost]
[ActionName("Edit")]
[Button("Delete")]
public ActionResult Delete(SampleModel model)
{
    // TODO: Save and redirect, this is just a sample
    ViewBag.Message = "Delete button clicked and Save action method selected.";
    return View(model);
}

Of course you might want to use constants instead of magic strings for button names. Note, that in the above example the action method name is exactly the same “Edit” for both actions. The underlying ButtonAttribute for the action method selection is listed below, feel free to use it:

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Web.Mvc;

/// <summary>
/// Declares a button attribute that selects action method to run. 
/// </summary>
[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]
public sealed class ButtonAttribute : ActionMethodSelectorAttribute
{
    private List<string> namesField;

    /// <summary>
    /// Initializes a new instance of the <see cref="ButtonAttribute"/> class.
    /// </summary>
    public ButtonAttribute()
    {
    }

    /// <summary>
    /// Initializes a new instance of the <see cref="ButtonAttribute"/> class.
    /// </summary>
    /// <param name="names">The button names that should match.</param>
    public ButtonAttribute(params string[] names)
    {
        if (names == null || names.Length == 0)
        {
            throw new ArgumentNullException(
                "names", 
                "Must specify at least one button name.");
        }

        this.namesField = new List<string>(names);
    }

    /// <summary>
    /// Gets button names. These are the form field names that are accepted when
    /// selecting proper action method.
    /// </summary>
    public IList<string> Names
    {
        get
        {
            if (this.namesField == null)
            {
                this.namesField = new List<string>();
            }

            return this.namesField;
        }
    }

    /// <summary>
    /// Checks whether the given controller action method is valid for execution 
    /// based on given button name.
    /// </summary>
    /// <param name="controllerContext">Controller  context.</param>
    /// <param name="methodInfo">Method context.</param>
    /// <returns>True if valid.</returns>
    public override bool IsValidForRequest(
        ControllerContext controllerContext, 
        System.Reflection.MethodInfo methodInfo)
    {
        if (controllerContext == null)
        {
            throw new ArgumentNullException("controllerContext", "Context can't be null.");
        }

        if (methodInfo == null)
        {
            throw new ArgumentNullException("methodInfo", "Method info can't be null.");
        }

        this.GuardState();

        bool returnValue = false;

        foreach (var name in this.Names)
        {
            ValueProviderResult result = controllerContext.Controller.ValueProvider
                .GetValue(name);
            returnValue = result != null;

            if (returnValue)
            {
                break;
            }
        }

        return returnValue;
    }

    private void GuardState()
    {
        if (this.Names == null)
        {
            throw new InvalidOperationException(
                "Button name must be initialized to non empty value.");
        }
    }
}

I find myself searching for this information over and over again for each web site I develop, so I guess I must blog this topic to remember it from now on.

IIS 7 has a very powerful caching framework, but some of the defaults are in my opinion not very well thought for modern web applications. For example: by default Javascript files are not compressed with GZIP, and cache location for static content is not public by default. So, if you use T4MVC, SquishtIt, or any other means to link to your static files with a timestamp then you might want to use settings along these lines:

<system.webserver>
  <!-- Omitted Some settings for brevity-->
  <!-- ... -->
  <httpProtocol>
    <customHeaders>
      <!-- Remove notion about ASP.NET -->
      <remove 
        name="X-Powered-By" />
    </customHeaders>
  </httpProtocol>
  <staticContent>
    <!--Enable gzipping JS by changing the mime type. 
    The IIS default mime type is not gzipped-->
    <remove 
        fileExtension=".js" />
    <mimeMap 
        fileExtension=".js" 
        mimeType="text/javascript" />
    
    <!--Add a long expires cache header-->
    <clientCache 
        cacheControlMode="UseExpires" 
        httpExpires="Tue, 19 Jan 2038 03:14:07 GMT" />
  </staticContent>
  <urlCompression
        doDynamicCompression="true" 
        doStaticCompression="true"
        dynamicCompressionBeforeCache="true"/>
  <caching>
    <profiles>
      <add 
        extension=".gif" 
        policy="CacheUntilChange" 
        kernelCachePolicy="CacheUntilChange" 
        location="Any" />
      <add 
        extension=".ico" 
        policy="CacheUntilChange" 
        kernelCachePolicy="CacheUntilChange" 
        location="Any" />
      <add 
        extension=".png" 
        policy="CacheUntilChange" 
        kernelCachePolicy="CacheUntilChange" 
        location="Any" />
      <add 
        extension=".js" 
        policy="CacheUntilChange" 
        kernelCachePolicy="CacheUntilChange" 
        location="Any" />
      <add 
        extension=".css" 
        policy="CacheUntilChange" 
        kernelCachePolicy="CacheUntilChange" 
        location="Any" />
    </profiles>
  </caching>
</system.webserver>

After changes check results with YSlow! or by manually examining response headers. Remember that IIS optimizes compression by not starting it if it is not required: you have to call the site from two different IP addresses from two different machines before IIS compresses responses.

A small but annoying bug I found today (and seems that someone else is having the same problem): when using jQuery.tablesorter) plugin together with tablesorterPager, applying the pager clears some properties from the table; In this case namely zebra striping. After initialization first table page is not striped, but next ones are after the first table page change.

I found some crazy complex solutions, like creating new widgets to replace the built in “zebra”, but the good enough solution ended up being lot easier: asking tablesorter to reapply widget:

var targetTable = $("table#some");
targetTable
    .tablesorter(
        { 
            widthFixed: true,  
            widgets: ['zebra']
        })
    .tablesorterPager(
        { 
            container: $("#pager"),
            size: 10,
            positionFixed: false,
            page: 0
        });
// tableSorterPager clears zebra striping, 
// re-apply it here. For some reason does not 
// work if called directly in continuation 
// to previous jQuery functions
targetTable.trigger("applyWidgetId", "zebra");

jQuery.tablesorter is outdated and not actively developed; If I could choose, I would go for some other plugin like datatables.net or soon-to-be-released “official” jQuery UI grid. In this case I did not have a choice.

In the project I recently worked for our solution’s web interface froze every now and then. This problem had been going on for a while, but every time the problem went away by itself without any intervention. No problem, until the issue escalated even to developer workstations, making them slow.

Investigation

The problem was easily pinpointed to be at the service layer (IIS WAS hosted). Investigation was started by adding more application pools, and immediately the responsiveness of the service increased. The main reason being that worker processes were restarted, but we did not know it then. Gladly we knew that this was not a fix, just a temporary cure, there was still something badly wrong.

So my coworker used some of his parallel Linq magic to create a denial of service test that targeted our service layer. Then we ran it… and our service died in 15 seconds on database connection pool problems. To check whether this really was a connection pool issue we increased connection pool size, but with no noticeable impact. Also, on some tests the service died immediately after 2 seconds. This was clearly not a connection pool problem.

Then we started to gather some performance data. We used performance monitor and resource monitor. Here is a screenshot from task manager presenting one server after a days use:

Original image lost at some blog conversion, sorry. This image showed a lot of threads on system.

Notice anything strange? Here is the situation after worker process were restarted:

Original image lost at some blog conversion, sorry. This image showed a decent amount of threads on a system.

Clearly the amount of threads explodes during use. That was very strange since all our threads are managed by trusted Windows Process Activation Services. Except that we had some custom thread handling on one place only, a logger class called ThreadedLogger that calls (heavy) logging in a background thread, and manages those threads itself. This class was adapted from some good Stackoverflow answers):

public abstract class ThreadedLogger<T> : IDisposable
{
    private Queue<action> queue = new Queue<action>();
    private ManualResetEvent hasNewItems = new ManualResetEvent(false);
    private ManualResetEvent terminate = new ManualResetEvent(false);
    private ManualResetEvent waiting = new ManualResetEvent(false);
 
    private Thread loggingThread;
 
    protected ThreadedLogger()
    {
        this.loggingThread = new Thread(new ThreadStart(this.ProcessQueue));
        this.loggingThread.IsBackground = true;
 
        // This is performed from a bg thread, to ensure 
        // the queue is serviced from a single thread
        this.loggingThread.Start();
    }
    
    // ...
}

As seen from above, this class creates a new background thread and manages it by itself. This is intentional, because this is the only way to use a background thread without WCF or IIS ripping it down when the primary thread completes. Ironically this was implemented to increase performance. Reading the code back and forth I did not find any bugs that would explode the amount of threads. Well, it turned out that this code is almost perfect; the reason was how the code was instantiated.

When service or web site starts, logger is registered into an IoC container:

container.RegisterType<ILogger, LoggerFacade>();

Then the logger is taken into use through dependency property or constructor injection:

[Dependency]
public ILogger Logger { get; set; }

Nice and easy? Except that was a perfect way to shoot yourself in the foot with an inversion of control container. Now every time this dependency is resolved a new thread is started, and that thread is never disposed. Since most of the dependencies are registered with RegisterType() instead of RegisterInstance(), it had been used also for the logger without any further thought.

The fix?

Nice and easy: just changed the registration of logger class to singleton. This is the good part of IoC containers: they provide a Single Point of Fixâ„¢ for this kind of issues:

container.RegisterType<ILogger, LoggerFacade>(
    new ContainerControlledLifeTimeManager());