9

I was trying to clean up some accessability stuff in my code, and inadvertently broke Unity dependency injection. After a while I realized that I marked some public properties that I didn't really want exposed outside my DLLs to internal. Then I started getting exceptions.

So it seems that using the [Dependency] attribute in Unity only works for public properties. I suppose that makes sense since the internal and private props wouldnt be visible to the Unity assembly, but feels really dirty to have a bunch of public properties that you never want anyone to set or be able to set, other than Unity.

Is there a way to let unity set internal or private properties too?

Here is the unit test I'd like to see pass. Currently only the public prop test passes:

    [TestFixture]
public class UnityFixture
{
    [Test]
    public void UnityCanSetPublicDependency()
    {
        UnityContainer container = new UnityContainer();
        container.RegisterType<HasPublicDep, HasPublicDep>();
        container.RegisterType<TheDep, TheDep>();

        var i = container.Resolve<HasPublicDep>();
        Assert.IsNotNull(i);
        Assert.IsNotNull(i.dep);
    }

    [Test]
    public void UnityCanSetInternalDependency()
    {
        UnityContainer container = new UnityContainer();
        container.RegisterType<HasInternalDep, HasInternalDep>();
        container.RegisterType<TheDep, TheDep>();

        var i = container.Resolve<HasInternalDep>();
        Assert.IsNotNull(i);
        Assert.IsNotNull(i.dep);
    }

    [Test]
    public void UnityCanSetPrivateDependency()
    {
        UnityContainer container = new UnityContainer();
        container.RegisterType<HasPrivateDep, HasPrivateDep>();
        container.RegisterType<TheDep, TheDep>();

        var i = container.Resolve<HasPrivateDep>();
        Assert.IsNotNull(i);
        Assert.IsNotNull(i.depExposed);
    }
}

public class HasPublicDep
{
    [Dependency]
    public TheDep dep { get; set; }
}

public class HasInternalDep
{
    [Dependency]
    internal TheDep dep { get; set; }
}

public class HasPrivateDep
{
    [Dependency]
    private TheDep dep { get; set; }

    public TheDep depExposed
    {
        get { return this.dep; }
    }
}

public class TheDep
{
}

Updated:

I noticed the call stack to set the property passed from:

UnityCanSetPublicDependency()
--> Microsoft.Practices.Unity.dll
--> Microsoft.Practices.ObjectBuilder2.dll
--> HasPublicDep.TheDep.set()

So in an attempt to at least make the internal version work, I added these to my assembly's properties:

[assembly: InternalsVisibleTo("Microsoft.Practices.Unity")]
[assembly: InternalsVisibleTo("Microsoft.Practices.Unity.Configuration")]
[assembly: InternalsVisibleTo("Microsoft.Practices.ObjectBuilder2")]

However, no change. Unity/ObjectBuilder still won't set the internal property

CodingWithSpike
  • 42,906
  • 18
  • 101
  • 138

8 Answers8

6

Another solution is to use [InjectionMethod] on a method where you pass the dependency into the class.

public class MyClass {
private ILogger logger;

[InjectionMethod]
public void Init([Dependency] ILogger logger)
{
    this.logger = logger;

...etc


and calling it:

container.BuildUp<MyClass>(instanceOfMyClass);

which will call Init with the dependency from unity.

didn´t quite solve the problem, I know...but

:-) J

Johan Leino
  • 3,473
  • 1
  • 26
  • 27
5

The question itself seems to be a misunderstanding.

Regarding the core statement:

a bunch of public properties that you never want anyone to set or be able to set, other than Unity.

You would want to set them in unit tests, or how else would you pass dependency mocks? Even if you don't have unit tests, it's a strange idea to have dependencies that nothing (except some magic of Unity) can set. Do you want your code to depend so much on support tool?

Also, having public properties is not an issue at all, because your code MUST depend on interfaces, not on implementations (one of SOLID principles). If you don't follow this principle - there is no reason for you to use Unity. Surely you wouldn't declare dependencies in the interface, so the consuming class doesn't know about them.

You've already been told that it's better to use constructor injection, but property injection also has its beauty. It allows adding new dependencies with less modifications (in particular, you can avoid changing existing unit tests at all, only adding new ones).

esteuart
  • 1,323
  • 1
  • 11
  • 14
Kurtevich
  • 345
  • 8
  • 18
5

If the property is get-only, it makes more sense to use contructor injection rather than property injection.

If Unity did use reflection to set private or internal members, it would be subjected to code access security constraints. Specifically, it wouldn't work in a low-trust environment.

Kent Boogaart
  • 175,602
  • 35
  • 392
  • 393
3

UPDATE FOR Enterprise Library 5.0

As rally52rs warned might happen, the upgrade to EntLib5.0 breaks his implementation. Using the same approach as Rally did, I reflected on the new code base and worked up the following 5.0 compatible version of the InternalConstructorSelectorPolicy.

Note that my version specifically limits itself to internal constructors in the FindLongestConstructor method. On this point my code is functionally different from Rally's.

public class InternalConstructorSelectorPolicy : IConstructorSelectorPolicy, IBuilderPolicy 
{
    private IDependencyResolverPolicy CreateResolver(ParameterInfo parameter)
    {
        List<DependencyResolutionAttribute> attrs = parameter.GetCustomAttributes(false).OfType<DependencyResolutionAttribute>().ToList<DependencyResolutionAttribute>();
        if (attrs.Count > 0)
        {
            return attrs[0].CreateResolver(parameter.ParameterType);
        }
        return new NamedTypeDependencyResolverPolicy(parameter.ParameterType, null);
    }

    private SelectedConstructor CreateSelectedConstructor(IBuilderContext context, IPolicyList resolverPolicyDestination, ConstructorInfo ctor)
    {
        SelectedConstructor result = new SelectedConstructor(ctor);
        foreach (ParameterInfo param in ctor.GetParameters())
        {
            string key = Guid.NewGuid().ToString();
            IDependencyResolverPolicy policy = this.CreateResolver(param);
            resolverPolicyDestination.Set<IDependencyResolverPolicy>(policy, key);
            DependencyResolverTrackerPolicy.TrackKey(resolverPolicyDestination, context.BuildKey, key);
            result.AddParameterKey(key);
        }
        return result;
    }

    private static ConstructorInfo FindInjectionConstructor(Type typeToConstruct)
    {
        ConstructorInfo[] injectionConstructors = typeToConstruct
            .GetConstructors(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)
            .Where<ConstructorInfo>(delegate(ConstructorInfo ctor)
        {
            return ctor.IsDefined(typeof(InjectionConstructorAttribute), true);
        }).ToArray<ConstructorInfo>();
        switch (injectionConstructors.Length)
        {
            case 0:
                return null;

            case 1:
                return injectionConstructors[0];
        }
        throw new InvalidOperationException(string.Format("Multiple constructors found for {0}" , typeToConstruct.Name ));
    }

    private static ConstructorInfo FindLongestConstructor(Type typeToConstruct)
    {
        var constructors =
            Array.FindAll(
                typeToConstruct.GetConstructors(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic),
                ctor => !ctor.IsFamily && !ctor.IsPrivate);  //Filter out protected and private constructors

        Array.Sort<ConstructorInfo>(constructors, new ConstructorLengthComparer());
        switch (constructors.Length)
        {
            case 0:
                return null;

            case 1:
                return constructors[0];
        }
        int paramLength = constructors[0].GetParameters().Length;
        if (constructors[1].GetParameters().Length == paramLength)
        {
            throw new InvalidOperationException(string.Format("Ambiguous constructor found for {0}", typeToConstruct.Name));
        }
        return constructors[0];
    }

    public SelectedConstructor SelectConstructor(IBuilderContext context, IPolicyList resolverPolicyDestination)
    {
        Type typeToConstruct = context.BuildKey.Type;
        ConstructorInfo ctor = FindInjectionConstructor(typeToConstruct) ?? FindLongestConstructor(typeToConstruct);
        if (ctor != null)
        {
            return this.CreateSelectedConstructor(context, resolverPolicyDestination, ctor);
        }
        return null;
    }

    // Nested Types
    private class ConstructorLengthComparer : IComparer<ConstructorInfo>
    {
        // Methods
        public int Compare(ConstructorInfo x, ConstructorInfo y)
        {
            return (y.GetParameters().Length - x.GetParameters().Length);
        }
    }
}
Kenneth Baltrinic
  • 2,941
  • 2
  • 28
  • 45
2

Well after a lof of poking around in reflector, I figured this out. By default, the code that finds a constructor for constructor injection calls:

ConstructorInfo[] constructors = typeToConstruct.GetConstructors()

With no BindingFlags, that will only detect public constructors. With some trickery (as in copy/paste from reflector) you can make a UnityContainerExtension that does all the same stuff as the default implementation, but change the call to GetConstructors() to:

ConstructorInfo[] constructors = typeToConstruct..GetConstructors(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)

Then add the extension into the unity container. The implemented extenstion is ~100 lines of code, so I didn't paste it here. If anyone wants it, let me know...

New working test case. Note that all the Unity created classes are now internal:

[TestFixture]
public class UnityFixture
{
    [Test]
    public void UnityCanSetInternalDependency()
    {
        UnityContainer container = new UnityContainer();
        container.AddNewExtension<InternalConstructorInjectionExtension>();
        container.RegisterType<HasInternalDep, HasInternalDep>();
        container.RegisterType<TheDep, TheDep>();

        var i = container.Resolve<HasInternalDep>();
        Assert.IsNotNull(i);
        Assert.IsNotNull(i.dep);
    }
}


internal class HasInternalDep
{
    internal HasInternalDep(TheDep dep)
    {
        this.dep = dep;
    }

    internal TheDep dep { get; set; }
}

internal class TheDep
{
}

I'm sure I can make an extension to do the same to resolve non-public properties, but that code was a lot more complicated :)

CodingWithSpike
  • 42,906
  • 18
  • 101
  • 138
1

@rally25rs, although the post is more than two years old it's still ranked high (views/google etc.) so I thought I'd add my 2 cents.. I've had the same problem and eventually chose this solution: UnityContainer and internal constructor. This is meant as a comment but I can't post comments just yet.

You've probably seen and know this already, still it might be of use for anyone else viewing: The InternalsVisibleTo() attribute should never have worked - that's because Unity isn't calling your classes directly. Instead, it's using reflection and inspecting the Type. Of course, the Type isn't changed as a result of the attribute being there. To "enjoy" the benefits of internals visible etc. on the receiving side, you have to explicitly call the internal c'tor (or property).

Community
  • 1
  • 1
Jonno
  • 1,976
  • 2
  • 21
  • 21
0

This is my Internal Constructor Injector Extension class:

Big potential issue: 99% of this is a copy/paste of the Unity code from .NET reflector, from unity version 4.1.0.0. Newer versions of Unity may change the implementation and break this extension, or cause flakey errors. You're warned!

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Reflection;
using Microsoft.Practices.ObjectBuilder2;
using Microsoft.Practices.Unity;
using Microsoft.Practices.Unity.ObjectBuilder;
using Microsoft.Practices.Unity.Utility;

namespace MyApp.Unity.Configuration
{
    /// <summary>
    /// This extension changes the behavior of Unity constructor injection to allow the use of non-public constructors.
    /// By default, Unity/ObjectBuilder would call Type.GetConstructors() to get the constructors. With the default binding
    /// flags, this only returns public constructors.
    /// The code here is 99% copy/paste from Reflector's dissassembly of the default Unity/OB implementation.
    /// My only change was to add binding flags to get all constructors, not just public ones.
    /// For more info, see: Microsoft.Practices.Unity.ObjectBuilder.DefaultUnityConstructorSelectorPolicy
    /// </summary>
    public class InternalConstructorSelectorPolicy : IConstructorSelectorPolicy
    {
        protected IDependencyResolverPolicy CreateResolver(ParameterInfo param)
        {
            List<DependencyResolutionAttribute> list = new List<DependencyResolutionAttribute>(Sequence.OfType<DependencyResolutionAttribute>(param.GetCustomAttributes(false)));
            if (list.Count > 0)
            {
                return list[0].CreateResolver(param.ParameterType);
            }
            return new NamedTypeDependencyResolverPolicy(param.ParameterType, null);
        }

        private SelectedConstructor CreateSelectedConstructor(IBuilderContext context, ConstructorInfo ctor)
        {
            SelectedConstructor constructor = new SelectedConstructor(ctor);
            foreach (ParameterInfo info in ctor.GetParameters())
            {
                string buildKey = Guid.NewGuid().ToString();
                IDependencyResolverPolicy policy = this.CreateResolver(info);
                context.PersistentPolicies.Set<IDependencyResolverPolicy>(policy, buildKey);
                DependencyResolverTrackerPolicy.TrackKey(context.PersistentPolicies, context.BuildKey, buildKey);
                constructor.AddParameterKey(buildKey);
            }
            return constructor;
        }

        private ConstructorInfo FindInjectionConstructor(Type typeToConstruct)
        {
            ConstructorInfo[] infoArray = Array.FindAll<ConstructorInfo>(typeToConstruct.GetConstructors(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic), delegate(ConstructorInfo ctor)
            {
                return ctor.IsDefined(typeof(InjectionConstructorAttribute), true);
            });
            switch (infoArray.Length)
            {
                case 0:
                    return null;

                case 1:
                    return infoArray[0];
            }
            throw new InvalidOperationException(string.Format(CultureInfo.CurrentCulture, "Resources.MultipleInjectionConstructors", new object[] { typeToConstruct.Name }));
        }

        private ConstructorInfo FindLongestConstructor(Type typeToConstruct)
        {
            ConstructorInfo[] constructors = typeToConstruct.GetConstructors(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
            Array.Sort<ConstructorInfo>(constructors, new ConstructorLengthComparer());
            switch (constructors.Length)
            {
                case 0:
                    return null;

                case 1:
                    return constructors[0];
            }
            int length = constructors[0].GetParameters().Length;
            if (constructors[1].GetParameters().Length == length)
            {
                throw new InvalidOperationException(string.Format(CultureInfo.CurrentCulture, "Resources.AmbiguousInjectionConstructor", new object[] { typeToConstruct.Name, length }));
            }
            return constructors[0];
        }

        public virtual SelectedConstructor SelectConstructor(IBuilderContext context)
        {
            Type typeToConstruct = BuildKey.GetType(context.BuildKey);
            ConstructorInfo ctor = this.FindInjectionConstructor(typeToConstruct) ?? this.FindLongestConstructor(typeToConstruct);
            if (ctor != null)
            {
                return this.CreateSelectedConstructor(context, ctor);
            }
            return null;
        }

        // Nested Types
        private class ConstructorLengthComparer : IComparer<ConstructorInfo>
        {
            // Methods
            public int Compare(ConstructorInfo x, ConstructorInfo y)
            {
                return (y.GetParameters().Length - x.GetParameters().Length);
            }
        }
    }

    /// <summary>
    /// Registeres the InternalConstructorSelectorPolicy with the Unity container.
    /// </summary>
    public class InternalConstructorInjectionExtension : UnityContainerExtension
    {
        protected override void Initialize()
        {
            this.Context.Policies.SetDefault(typeof(IConstructorSelectorPolicy), new InternalConstructorSelectorPolicy());
        }
    }
}
CodingWithSpike
  • 42,906
  • 18
  • 101
  • 138
0

Based on Kent B's answer, I changed to use constructor injection, which does work for public classes. However the root issue still exists, where anything you ever want to assign or have assigned by Unity has to be public. This includes the classes themselves.

New unit test:

    [TestFixture]
public class UnityFixture
{
    [Test]
    public void UnityCanSetInternalDependency()
    {
        UnityContainer container = new UnityContainer();
        container.RegisterType<HasInternalDep, HasInternalDep>();
        container.RegisterType<TheDep, TheDep>();

        var i = container.Resolve<HasInternalDep>();
        Assert.IsNotNull(i);
        Assert.IsNotNull(i.dep);
    }
    }

internal class HasInternalDep
{
    internal HasInternalDep(TheDep dep)
    {
        this._Dep = dep;
    }

    private TheDep _Dep;
        internal TheDep dep
        {
            get { return _Dep; }
        }
}

internal class TheDep
{
}
}

With the assembly attributes:

[assembly: InternalsVisibleTo("Microsoft.Practices.Unity")]
[assembly: InternalsVisibleTo("Microsoft.Practices.Unity.Configuration")]
[assembly: InternalsVisibleTo("Microsoft.Practices.ObjectBuilder2")]

Fails with the error:

The type HasInternalDep does not have an accessible constructor.
at Microsoft.Practices.Unity.UnityContainer.DoBuildUp(Type t, String name)

So overall it seems that if you want to use Unity, you basically just have to blanket mark everything public. Really ugly for a utility/library .dll...

CodingWithSpike
  • 42,906
  • 18
  • 101
  • 138