tags:

views:

1353

answers:

7

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

+3  A: 

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.

HTH, Kent

Kent Boogaart
true, that probably is a cleaner solution. still annoyed that unity / ob can't use internals. oh well... thanks!
rally25rs
A: 

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...

rally25rs
+2  A: 

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 :)

rally25rs
+3  A: 

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
very cool, I wasn't aware of that attribute. thanks!
rally25rs
A: 

can you try to post the code for InternalConstructorInjectionExtension ?

thanks in advance

Is now posted in another answer here. Hope it helps you out...
rally25rs
A: 

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());
     }
    }
}
rally25rs
+2  A: 

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
I haven't moved to EntLib5 yet, so this will save me the headache whenever I do... thanks for the code!
rally25rs