Unity Decorator Extension fails with multiple implementations

Jul 5, 2013 at 4:32 PM
Edited Jul 5, 2013 at 4:34 PM
As always, I've created this same question o SO already, will be copying it here for reference:
http://stackoverflow.com/questions/17478689/unity-decorator-extension-fails-with-multiple-implementations

I've been struggling with this problem for a couple days, and I still am not sure how to solve it.

I've created a container extension for the Unity Container to enable me to easily register decorator classes in the container. This is the implementation I currently have, which is almost identical to the one in this article:
    public class DecoratorExtension : UnityContainerExtension
    {
        private int m_order;
        private Dictionary<Type, IList<DecoratorRegistration>> m_typeStacks;
    
        protected override void Initialize()
        {
            m_typeStacks = new Dictionary<Type, IList<DecoratorRegistration>>();
            Context.Registering += AddRegistration;
            Context.Strategies.Add(new DecoratorBuildStrategy(m_typeStacks), UnityBuildStage.PreCreation);
        }
    
        private void AddRegistration(object _sender, RegisterEventArgs _e)
        {
            if (_e.TypeFrom == null || !_e.TypeFrom.IsInterface)
                return;

            GetStack(_e.TypeFrom)
                .Add(new DecoratorRegistration {Order = m_order++, Type = _e.TypeTo});
        }
    
        private IList<DecoratorRegistration> GetStack(Type _type)
        {
            if (!m_typeStacks.ContainsKey(_type))
                m_typeStacks.Add(_type, new List<DecoratorRegistration>());
    
            return m_typeStacks[_type];
        }
    }
What this does is use a list for each type, to store all type registrations for the same target type, so that I can reassemble it when Resolve is called, using this build strategy:
    internal class DecoratorBuildStrategy : BuilderStrategy
    {
        private readonly Dictionary<Type, IList<DecoratorRegistration>> m_typeStacks;
    
        internal DecoratorBuildStrategy(Dictionary<Type, IList<DecoratorRegistration>> _typeStacks)
        {
            m_typeStacks = _typeStacks;
        }
    
        public override void PreBuildUp(IBuilderContext _context)
        {
            var key = _context.OriginalBuildKey;
            if (_context.GetOverriddenResolver(key.Type) != null)
                return;
    
            // Only interfaces can use decorators.
            if (!key.Type.IsInterface)
                return;
            
            // Gets the list of types required to build the 'decorated' instance.
            // The list is reversed so that the least dependent types are built first.
            var decoratorTypes = GetDecoratorTypes(key.Type).Reverse().ToList();
            if (!decoratorTypes.Any())
                return;
    
            object value = null;
            foreach (var type in decoratorTypes)
            {
                Type typeToBuild = type;
                if (typeToBuild.IsGenericTypeDefinition)
                {
                    Type[] genericArgumentTypes = key.Type.GetGenericArguments();
                    typeToBuild = typeToBuild.MakeGenericType(genericArgumentTypes);
                }
    
                value = _context.NewBuildUp(new NamedTypeBuildKey(typeToBuild, key.Name));
    
                // An Override is created so that in the next BuildUp the already 
                // built object gets used instead of doing the BuildUp again and 
                // entering an infinite loop
                _context.AddResolverOverrides(new DependencyOverride(key.Type, value));
            }
    
            _context.Existing = value;
            _context.BuildComplete = true;
        }
    
        private IEnumerable<Type> GetDecoratorTypes(Type _type)
        {
            var typeList = m_typeStacks.GetValueOrDefault(_type) ?? new List<DecoratorRegistration>(0);
            if (!_type.IsGenericType)
                return typeList.Select(_reg => _reg.Type);
    
            // If the type is a generic type, we need to get all open generic registrations
            // alongside the closed ones
            var openGenericList = m_typeStacks
                    .GetValueOrDefault(_type.GetGenericTypeDefinition()) ?? 
                    new List<DecoratorRegistration>(0);

            // The final result is an ordered concatenation of the closed and open registrations 
            // that should be used for the type
            return typeList
                .Concat(openGenericList)
                .OrderBy(_registration => _registration.Order)
                .Select(_reg => _reg.Type);
        }
    }
This is where the DecoratorRegistration model is used. It is just a pair of type/int that represents the order of the registration. I created this to be able to mix open and closed generic registrations correctly:
    internal struct DecoratorRegistration
    {
        public int Order { get; set; }
        public Type Type { get; set; }
    }
This works wonders for the most part. The problem started when I had a class that implemented two interfaces, one which was decorated, and one that wasn't.

This is the current test case I'm trying to make work:
    private interface IAny<T> {}
    private interface IAnotherInterface {}
    private class Base<T> : IAnotherInterface, IAny<T> {}   
    private class Decorator1<T> : IAny<T>
    {
        internal readonly IAny<T> Decorated;

        public Decorator1(IAny<T> _decorated)
        {
            Decorated = _decorated;
        }
    }

    [TestMethod]
    public void DecoratorExtensionDoesNotInterfereWithNormalRegistrations()
    {
        // Arrange
        var container = new UnityContainer()
            .AddNewExtension<DecoratorExtension>()
            .RegisterType<Base<string>>(new ContainerControlledLifetimeManager())
            .RegisterType<IAny<string>, Decorator1<string>>()
            .RegisterType<IAny<string>, Base<string>>()
            .RegisterType<IAnotherInterface, Base<string>>();

        // Act
        var decorated = container.Resolve<IAny<string>>();
        var normal = container.Resolve<IAnotherInterface>();
        var anotherDecorated = container.Resolve<IAny<string>>();
        var anotherNormal = container.Resolve<IAnotherInterface>();

        // Assert
        Assert.IsInstanceOfType(normal, typeof (IAnotherInterface));
        Assert.IsInstanceOfType(decorated, typeof (Decorator1<string>));
        Assert.AreSame(normal, anotherNormal);
        Assert.AreSame(decorated, anotherDecorated);
    }
This test should make my intent clear. I wanted singleton classes, but the first call to Resolve, for either IAnotherInterface or IAny<string> results in every subsequent call to return the same thing. Thus, I get an exception:
    System.InvalidCastException: Unable to cast object of type 'Decorator1`1[System.String]' to type 'IAnotherInterface'.
on this line:
var normal = container.Resolve<IAnotherInterface>();
I'm not sure what to do here. I had to temporarily disable singletons in our project so that this could work as intended. What I wanted is that the Base<string> instance was a sintleton, but when I requested a IAny<string> it would create a NEW instance with the SAME base being decorated.

This is still using .Net 4.0, so I'm stuck with Unity 2.1 here (shouldn't matter in this case though).
Editor
Jul 7, 2013 at 8:09 PM
Edited Jul 12, 2013 at 7:29 PM
I know you've seen it, but this discussion post shows how to used named instances to create decorators.

What I'm seeing is that the extension is resolving a different type than the BuildKey that Unity has mapped to.

For example:
  • An IAny<string> is requested
  • Unity maps that to Base<string> since it was the last IAny<string> registered
  • The extension runs and creates all the decorators and returns the last decorator object. In this case it is Decorator1<string>.
Now as you say, this is usually great -- the object is created with the chain of decorators, returned and everything works. The issue occurs because ContainerControlledLifetimeManager (which is being used for singletons) stores the newly created Decorator1<string> instance with the Base<string> BuildKey. So then:
  • Unity stores Decorator1<string> with the Base<string> BuildKey
  • An IAnotherInterface is resolved
  • Unity maps that to Base<string>
  • Unity returns the Decorator1<string> instance from the ContainerControlledLifetimeManager associated with the Base<string> build key without calling the decorator extension
  • InvalidCastException
That's a problem because Decorator1<string> is actually stored in the Base<string> lifetime manager and Decorator1<string> is not an IAnotherInterface (whereas Base<string> is).

I was able to get the unit tests to pass. That said, I'm not saying it's correct in general since there are so many different scenarios that could occur.

What I did was to create another BuildStrategy that is run in the TypeMapping phase. It basically maps the OriginalBuildKey to the final BuildKey as determined by the decorator chain:
    public class DecoratorExtension : UnityContainerExtension
    {
        private int m_order;
        private Dictionary<Type, IList<DecoratorRegistration>> m_typeStacks;

        protected override void Initialize()
        {
            m_typeStacks = new Dictionary<Type, IList<DecoratorRegistration>>();
            Context.Registering += AddRegistration;
            Context.Strategies.Add(new DecoratorBuildStrategy(m_typeStacks, GetDecoratorTypes), UnityBuildStage.PreCreation);
            Context.Strategies.Add(new TypeMappingDecoratorBuildStrategy(m_typeStacks, GetDecoratorTypes), UnityBuildStage.TypeMapping);
        }

        private void AddRegistration(object _sender, RegisterEventArgs _e)
        {
            if (_e.TypeFrom == null || !_e.TypeFrom.IsInterface)
                return;

            GetStack(_e.TypeFrom)
                .Add(new DecoratorRegistration { Order = m_order++, Type = _e.TypeTo });
        }

        private IList<DecoratorRegistration> GetStack(Type _type)
        {
            if (!m_typeStacks.ContainsKey(_type))
                m_typeStacks.Add(_type, new List<DecoratorRegistration>());

            return m_typeStacks[_type];
        }

        private IEnumerable<Type> GetDecoratorTypes(Type _type)
        {
            var typeList = m_typeStacks.GetValueOrDefault(_type) ?? new List<DecoratorRegistration>(0);
            if (!_type.IsGenericType)
                return typeList.Select(_reg => _reg.Type);

            // If the type is a generic type, we need to get all open generic registrations
            // alongside the closed ones
            var openGenericList = m_typeStacks
                    .GetValueOrDefault(_type.GetGenericTypeDefinition()) ??
                    new List<DecoratorRegistration>(0);

            // The final result is an ordered concatenation of the closed and open registrations 
            // that should be used for the type
            return typeList
                .Concat(openGenericList)
                .OrderBy(_registration => _registration.Order)
                .Select(_reg => _reg.Type);
        }
    }

    public class TypeMappingDecoratorBuildStrategy : BuilderStrategy
    {
        private readonly Dictionary<Type, IList<DecoratorRegistration>> m_typeStacks;
        private readonly Func<Type, IEnumerable<Type>> getDecoratorTypes;

        internal TypeMappingDecoratorBuildStrategy(Dictionary<Type, IList<DecoratorRegistration>> _typeStacks,
            Func<Type, IEnumerable<Type>> getDecoratorTypes)
        {
            m_typeStacks = _typeStacks;
            this.getDecoratorTypes = getDecoratorTypes;
        }

        public override void PreBuildUp(IBuilderContext _context)
        {
            var key = _context.OriginalBuildKey;
            if (_context.GetOverriddenResolver(key.Type) != null)
                return;

            // Only interfaces can use decorators.
            if (!key.Type.IsInterface)
                return;

            // Gets the list of types required to build the 'decorated' instance.
            // The list is reversed so that the least dependent types are built first.
            var decoratorTypes = getDecoratorTypes(key.Type).Reverse().ToList();
            if (!decoratorTypes.Any())
                return;

            _context.BuildKey = new NamedTypeBuildKey(decoratorTypes.Last(), _context.BuildKey.Name);
        }
    }

    internal class DecoratorBuildStrategy : BuilderStrategy
    {
        private readonly Dictionary<Type, IList<DecoratorRegistration>> m_typeStacks;
        private readonly Func<Type, IEnumerable<Type>> getDecoratorTypes;
        
        internal DecoratorBuildStrategy(Dictionary<Type, IList<DecoratorRegistration>> _typeStacks,
            Func<Type, IEnumerable<Type>> getDecoratorTypes)
        {
            m_typeStacks = _typeStacks;
            this.getDecoratorTypes = getDecoratorTypes;
        }

        public override void PreBuildUp(IBuilderContext _context)
        {
            var key = _context.OriginalBuildKey;
            if (_context.GetOverriddenResolver(key.Type) != null)
                return;

            // Only interfaces can use decorators.
            if (!key.Type.IsInterface)
                return;

            // Gets the list of types required to build the 'decorated' instance.
            // The list is reversed so that the least dependent types are built first.
            var decoratorTypes = getDecoratorTypes(key.Type).Reverse().ToList();
            if (!decoratorTypes.Any())
                return;

            object value = null;
            foreach (var type in decoratorTypes)
            {
                Type typeToBuild = type;
                if (typeToBuild.IsGenericTypeDefinition)
                {
                    Type[] genericArgumentTypes = key.Type.GetGenericArguments();
                    typeToBuild = typeToBuild.MakeGenericType(genericArgumentTypes);
                }

                value = _context.NewBuildUp(new NamedTypeBuildKey(typeToBuild, key.Name));

                // An Override is created so that in the next BuildUp the already 
                // built object gets used instead of doing the BuildUp again and 
                // entering an infinite loop
                _context.AddResolverOverrides(new DependencyOverride(key.Type, value));
            }

            _context.Existing = value;
            _context.BuildComplete = true;
        }
    }

I also had to explicitly register the Decorators as singletons:
var container = new UnityContainer()
    .AddNewExtension<DecoratorExtension>()
    .RegisterType<Base<string>>(new ContainerControlledLifetimeManager())
    .RegisterType<Decorator1<string>>(new ContainerControlledLifetimeManager())
    .RegisterType<IAny<string>, Decorator1<string>>()
    .RegisterType<IAny<string>, Base<string>>()
    .RegisterType<IAnotherInterface, Base<string>>();

The unit tests pass with the above and also seem to work with a Transient registration. I hope that helps.

I'll just say that the multiple RegisterType calls to configure the decorator chain could be a bit confusing to people familiar with Unity since only the last one will result in an actual registration (last in wins). Maybe an extension method with a comment could convey the intent a bit better?
    public static class UnityExtensions
    {
        /// <summary>
        /// Register decorator type mapping.  This uses standard Unity register method and the 
        /// DecoratorContainerExtension takes care of storing the decorators/type mapping.
        /// </summary>
        public static IUnityContainer RegisterDecorator<TFrom, TTo>(this IUnityContainer container)
        {
            return container.RegisterType(typeof(TFrom), typeof(TTo));
        }

        public static IUnityContainer RegisterDecorator(this IUnityContainer container, Type from, Type to)
        {
            return container.RegisterType(from, to);
        }
    }

    var container = new UnityContainer()
        .AddNewExtension<DecoratorExtension>()
        .RegisterType(typeof(Base<>), new ContainerControlledLifetimeManager())
        .RegisterType(typeof(Decorator1<>),new ContainerControlledLifetimeManager())
        .RegisterDecorator<IAny<string>, Decorator1<string>>()
        .RegisterDecorator<IAny<string>, Base<string>>()
        .RegisterDecorator<IAnotherInterface, Base<string>>();

Just a thought.

~~
Randy Levy
entlib.support@live.com
Enterprise Library support engineer
Support How-to
Jul 8, 2013 at 6:07 PM
Ok, I can see what you did. This was something I thought about too, the fact that things were registered with the wrong buildkey.

I'll thoroughly examine this later, but it will probably take a few days because I'm resolving an issue here. And about the intent not being clear with consecutive RegisterType calls, I agree with that. My only concern with an extension method in this case is that someone could actually register a decorator without using it at all. Maybe if I could somehow enforce the usage of this extension method for decorators? It will obviously not work with the current code, but I wonder if there is a way to do that?

And, again, just wanted to say thank you Randy for the huge help. I was impressed with how you actually understood the problem and proposed a good solution so fast without even needing more details.

Will report back as soon as I can try this out a bit.
Jul 26, 2013 at 2:55 PM
Hi Randy.

I'm very sorry it took this long for me to come back to this. We had quite a few problems in a short period of time and I ended up letting this bug slide a bit, since I've changed the registrations to Transient as a quick fix.

I can confirm your code is really working. As you saw yourself, the test was wrong to begin with. The decorated instances should of course be different with these registrations, that was a mistake on my part. What I wanted to test is if the contained base class on each of the decorators was the same, and this is also working :)

I'm still curious if it is possible to abstract the decorator registrations away from the container itself in some way. What I think would be ideal is if registering decorators was only possible thought some custom api (like your extension methods). As I said earlier, I agree that the way it is working now is not that intuitive, but I don't like the fact that your extension methods use the normal RegisterType to do that same thing, as that opens up problems with people registering decorators through RegisterType directly by mistake. Is there a way I can customize the container to achieve that?

Also, when working with the normal extensions, the container overrides the last registration if another one is made for the same type right? I figure this is currently broken by my extension? I don't have a situation yet in which the same type is registered multiple times. By forcing users to use a custom api to register decorators, I could avoid the normal workflow on the container, avoiding this problem altogether.

As you may have noticed by now, I really like this concept, and would like to keep using this in our environment. I wanted to guarantee that this extension does not interfere too much with the rest of the container pipeline, so any help is appreciated. For instance, if you happen to know a more complicated test case that could be tested would you please share the idea with me?

Thanks a lot (again) man,
Juliano
Editor
Aug 1, 2013 at 4:49 AM
Edited Aug 1, 2013 at 4:49 AM
There aren't many hooks to trigger the container extension so I think you are pretty much stuck with RegisterType. Another option would be to modify Unity source to add the functionality you want. A middle ground might be to register the decorators directly with the container extension and then add the container extension to the container.

Something like:
    IUnityContainer container = new UnityContainer()
        .RegisterType(typeof(Base<>), new ContainerControlledLifetimeManager())
        .RegisterType(typeof(Decorator1<>),new ContainerControlledLifetimeManager());

    DecoratorExtension extension = new DecoratorExension(container);
    extension.RegisterDecorator<IAny<string>, Decorator1<string>>();
    extension.RegisterDecorator<IAny<string>, Base<string>>();
    extension.RegisterDecorator<IAnotherInterface, Base<string>>();

    container.AddExtension(extension);

~~
Randy Levy
entlib.support@live.com
Enterprise Library support engineer
Support How-to
Aug 24, 2013 at 12:04 AM
I'm sorry for not coming back to you earlier, thanks a lot for the insight.

I think the option of registering on the extension itself seems appropriate. I would have to 'cache' the registrations inside the extension and apply them on the event fired when it's added to the container right? Is this how the EnterpriseLibraryCoreExtension works? I'm using it right now to register a custom application block automatically from the config settings.

It's unfortunate that this is still not ideal though, because the problem persists (a normal registration can still serve as a decorator registration).