Detecting possible lifestyle conflicts in the Windsor container

When using the Castle Windsor container you have to make decisions about the lifestyles of your components. The common options are Singleton (which is the default) and Transient. If you are using the container within a web application you are likely to also want to use the PerWebRequest lifestyle.

When you have a complex object graph with lots of dependencies problems can arise if you have inadvertently given one or more components the wrong lifestyle. Generally speaking it is most likely to cause problems if a component has a dependent component which has a shorter lifestyle than the component it is a dependent of.

Let’s think of a concrete example using the following two classes:

public class UserContext
{
    public int SessionId {get;set;}
}

public class DoStuffService
{
    public DoStuffService(UserContext context)
    {
        ....
    }
}

In this case the UserContext class contains some information that pertains to the context in which the code is executing, namely the session id. For this reason we rightly want use the PerWebRequest lifestyle for this component (I’m not going to worry about how this component gets it’s SessionId property populated – that’s not the point).

Now, what is less obvious is that this decision implies that the DoStuffService must also have a lifestyle with a duration less than or equal to UserContext. Consider the following container configuration:

new WindsorContainer()
    .Register(
         Component.For<UserContext>().LifeStyle.PerWebRequest,
         Component.For<DoStuffService>()
        );

This is perfectly valid and you will get no complaints from the container, but what it means is that only one instance of the DoStuffService will be created by the container since the lifestyle defaults to Singleton. When it is constructed for the first and only time, we will get an instance of the UserContext injected in correctly. However, the next web request will expect a new instance of UserContext but it will not get one. i.e. UserContext has effectively become a Singleton as well.

If you are lucky, your code will fail. But the really nasty thing is that in a single user environment e.g. a development environment, Singleton and PerWebRequest can behave in exactly the same way so that you never notice a problem. Furthermore, the problem is unlikely to be as obvious as it is in this example with a much more complicated object graph.

You can see that this is not really something that the container can prevent in any generic way because there is no real way to tell whether it will in practice cause a problem. However, it is possible for us as users to decide on a policy and then to validate that the configured container conforms to that policy. A good policy seems to be that a component cannot have a lifestyle that is longer in duration than any of  it’s dependent components. Such a policy can be implemented as follows:

public static void ValidateLifestyles(this IWindsorContainer container)
{
    var errors = new ContainerLifeStyleException();
    var comparer = new LifeStyleComparer();

    foreach (var node in container.Kernel.GraphNodes)
    {
        var component = (ComponentModel)node;
        foreach (var dependent in node.Dependents)
        {
            var dependentComponent = (ComponentModel)dependent;
            if (comparer.Compare(component.LifestyleType, dependentComponent.LifestyleType) > 0)
                errors.Add(component, dependentComponent);
        }
    }

    if (errors.ContainsErrors)
        throw errors;
}

ContainerLifeStyleException is just a collection of all of the problems found along the way and LifeStyleComparer is just an implementation of IComparer<LifestyleType> so that you can decide which lifestyles are of longer duration than others .

You can then call this extension method immediately after configuring your container and any potential lifestyle problems will be reported.

Beware empty/null selectors with jQuery

I encountered a problem today with one of my JavaScript components which I use to wrap and extend the normal checkbox input control. My component uses jQuery to add an onclick event to the underlying dom element as follows:

$(this._element).click(fn);

The problem arises when the underlying element does not actually exists (never mind why). In this case this._element is null. My assumption was that if jQuery did not throw an exception here (which it rarely does) then it would probably just do nothing in this case. My assumption was wrong, as in this case it appears that jQuery will attach the click event handler to the whole document body. This is pretty obvious when you look at the jQuery code itself as the first thing it does is to make sure that you have indeed specified a selector:

jQuery.fn = jQuery.prototype = {
    init: function( selector, context ) {
        // Make sure that a selection was provided
        selector = selector || document;
....

The end result is that the click function I was trying to assign to the checkbox click was actually happening when clicking anything. I’m sure there is a good reason for this but it seems likely a slightly odd default behaviour to me and one to watch out for.

ScriptManager update

The moral of the story is that you need to always register scripts during the pre-render phase. The problems that I was talking about in my previous post only raise their heads when you start registering scripts in the load phase. The only reason that people (myself included) start doing that is because the page lifecycle is so notoriously convoluted. If I started again I would definitely use Monorail.

ScriptManager.CompositeScript issues

On the face of it the ScriptManager’s CompositeScript feature seems pretty useful. It allows you to combine multiple scripts together into a single script include at runtime. This is often a good idea since the cost in terms of latency of having a lot of individual script includes can be considerable (even if they are cached you may still have to pay the price of the 304 conditional GET). It can be a bad idea too of course because it can lead to larger overall download size and larger memory footprint of your browser cache. As always it’s a trade off.

Anyway, assume that you’ve decided that you want to do it and you choose to use the ScriptManager’s CompositeScript capability. The first problem I encountered is that there is a limit to the number of scripts that you can add to it. Though don’t expect anything as useful as a compile time error – no it will hit you at runtime with the following error:

The resource URL cannot be longer than 1024 characters. If using a CompositeScriptReference, reduce the number of ScriptReferences it contains, or combine them into a single static file and set the Path property to the location of it.

For me, this makes it pretty useless. But assuming that is not a problem, I then ran into another issue. I wanted to make my code conditional on build type such that I added a normal script reference to the script manager in a debug build to make development and debugging easier and added script references to the composite script when not in debug to get the benefits of the script combining when it matters. So I wrote code like the following:

public static void AddCompositeScript(Page page, string url)
{
    var sm = ScriptManager.GetCurrent(page);
    if (sm == null)
        throw new Exception(_messageManager.GetMessage(MessageKey.ScriptManagerUnfound));
#if DEBUG
    AddNormalScriptReference(sm, url);
#else
    sm.CompositeScript.Scripts.Add(new ScriptReference(url));
#endif
}

The problem seems to be with the page lifecycle. In a .Net page the OnLoad event fires before the OnPreRender event right? Correct. So if I register script A in the OnLoad event and script B in the OnPreRender event I know that script A will be included in the page before script B. This is the case if both script A and script B are registered as normal script references. However, if script B were registered as a Composite Script then it would end up being included on the page before script A in contrast to what the page lifecycle would have you expect. So it’s double bad because a) it seems to be wrong and b) it’s inconsistent.

My fiddler trace clearly show the composite script registered during pre-render being included in the page before other scripts included using normal script references during the the load phase.

So for me I think it’s more trouble than it’s worth. If you use content expiry on you static script files you can avoid the 304 conditional GET which basically means that once the user’s caches are primed, there is no down-side to having lots of script files. It is down to how much control you have over your user’s setup and how volatile your script files are. For me, script combining just doesn’t seem to be worth it.

Digging into YUICompressor – fo sho!

I was looking into using the YUI Compressor code to compress and combine my javascript files and specifically at the ready made msbuild task that comes with it. I wandered why the “ObfuscateJavascript” parameter appeared not to work using the following build script fragment:

<CompressorTask
            JavaScriptFiles="@(JavaScriptFiles)"
            ObfuscateJavaScript="True"
            PreserveAllSemicolons="False"
            DisableOptimizations="False"
            EncodingType="Default"
            DeleteJavaScriptFiles="False"
            LineBreakPosition="-1"
            JavaScriptOutputFile="$(UserInterfaceReleaseDir)\include\ControlScripts\ArmControlContainer.js"
            LoggingType="ALittleBit"
            ThreadCulture="en-gb" />

Digging into the code a little I can see this:

_obfuscateJavaScript = !string.IsNullOrEmpty(ObfuscateJavaScript)
    && ParseSillyTrueFalseValue(ObfuscateJavaScript.ToUpperInvariant());

ParseSillyTrueFalseValue? Well it looks like this:

private static bool ParseSillyTrueFalseValue(string value)
{
    bool result;

    if (string.IsNullOrEmpty(value))
    {
        throw new ArgumentNullException("value");
    }

    switch (value)
    {
        case "yes":
        case "y":
        case "yep":
        case "yeah":
        case "true":
        case "fosho":
        case "fo sho":
            result = true;
            break;
        default:
            result = false;
            break;
    }

    return result;
}

How bizarre? And also notice that the input is always being converted to upper case, but the switch statement only checks lower case values so it always returns false.

I’m sure that the compression code itself is a lot more solid but I think I might write my own msbuild task….

NHibernate – removing duplicates combined with paging

The criteria API for NHibernate gives us a really easy way to perform paged queries. Combine this with .Net extension methods and you can get something quite neat. I use the following extension method to page my criteria queries:

public static ICriteria Page(this ICriteria criteria, int pageIndex, int pageSize)
{
    return criteria.SetFirstResult(pageIndex*pageSize)
        .SetMaxResults(pageSize);
}

This means that I can write queries like this:

_session.CreateCriteria(typeof(IResource))
            .Page(pageIndex, pageSize)
            .List<IResource>());

Which is nice. However, when paging a result set you may encounter a problem in that some queries will generate duplicates. In the example above I am returning “resources”. Let’s assume that resources can belong to one or more “groups”. If I want to restrict my query to resources that belong to groups with some characteristic then I am potentially going to get duplicate results. If I don’t mind duplicates then I have no problem, but if I want to remove them then I have to be careful.

The easiest way to remove duplicates in NHibernate is to use a DistinctRootEntityTransformer like this:

_session.CreateCriteria(typeof(IResource))
        .CreateCriteria("Groups")
        .SetResultTransformer(new DistinctRootEntityResultTransformer())
        .List<IResource>();

The problem is that this removes duplicates in the code after the query has been executed. This is problematic if you are combining it with paging because the query will return you a certain number of records (a page full) and then the transformer will potentially remove some of those records and spoil your paging. So if you want to take advantage of the database for paging rather than doing the paging in memory (not always an option for large datasets) then you need a different strategy.

Obviously it is necessary to do the “de-duping” as part of the query. One way to do this is to use the DetachedCriteria capability in NHibernate. The idea is to create a DetachedCriteria which returns a projection of just the distinct ids of the entities that you want to retrieve (in this case resources). All of your restrictions are applied to the DetachedCriteria:

var resIds = DetachedCriteria.For<Resource>()
        .Add(Restrictions.EqProperty("id", "res.id"))
        .CreateCriteria("Groups")
        .SetProjection(Projections.Distinct(Projections.Id()));

So this gets us the list of Ids we want without any duplicates. But how to translate that into a list of domain objects with no duplicates, paged correctly.

_session.CreateCriteria(typeof(IResource), "res")
        .Page(pageIndex, pageSize)
        .Add(Subqueries.PropertyIn("res.id", resIds))                    
        .List<IResource>());

Using a correlated subquery we can restrict the query to just the distinct Ids returned by our DetachedCriteria and perform the paging all as part of the same query. Nice. The resultant sql looks something like this:

SELECT TOP 10 this_.resourceid       AS resourceid13_0_,
              this_.name             AS name13_0_,
              this_.firstname        AS firstname13_0_
FROM   rm_resource this_
WHERE  EXISTS (SELECT DISTINCT this_0_.resourceid AS y0_
               FROM   rm_resource this_0_
                      INNER JOIN rm_resourcegroup resourcegr1_
                        ON this_0_.resourceid = resourcegr1_.resourceid
               WHERE  this_0_.resourceid > @p0
                      AND this_0_.resourceid = this_.resourceid)

I’ve removed some of the bits form the select clause just to save space but you get the idea.