z4kn4fein / stashbox

A lightweight, fast, and portable dependency injection framework for .NET-based solutions.
https://z4kn4fein.github.io/stashbox
MIT License
140 stars 10 forks source link

parent-child resolution (egg-chicken) #44

Closed bonesoul closed 4 years ago

bonesoul commented 5 years ago

let's say we have an egg-chicken case we would like to handle as follows;

   public interface IEgg
    {
        IChicken Chicken { get; set; }
    }

    public interface IChicken
    {
        IEgg Egg { get; set; }
    }

    public class Chicken : IChicken
    {
        public IEgg Egg { get; set; }
    }

    public class Egg : IEgg
    {
        public Egg(IChicken chicken)
        {
            Chicken = chicken;
        }

        public IChicken Chicken
        {
            get;
            set;
        }
    }

with unity ioc it's possible with;

public void BuildObjectGraphWithCircularDependency()
{
    var container = new UnityContainer();

    container.RegisterType<IChicken, Chicken>(new PerResolveLifetimeManager(), new InjectionProperty("Egg"));
    container.RegisterType<IEgg, Egg>();

    var chicken = container.Resolve<IChicken>();

    Assert.AreSame(chicken, chicken.Egg.Chicken);
}

( code from http://sharpfellows.com/post/Unity-IoC-Container- )

is this also possible with stashbox?

z4kn4fein commented 5 years ago

Hi, it's possible but not in this form, you can use the .WithCircularDependencyWithLazy() configuration option, however that needs to declare one of your circular dependendecies as Lazy<T>, and that doesn't require property injection, it also works with constructor arguments. But i'd recommend to avoid circular dependencies if possible. Do you have an exact issue that could be solved only with this?

bonesoul commented 5 years ago

Basically i want to have a parent - child relation between these 2 interfaces;

namespace PsnBooster.Core.Psn.Server
{
    public interface IPsnServer
    {
        int Id { get; }

        IPAddress IpAddress { get; }

        ILatencyCheck LatencyCheck { get; }
    }
}
    public interface ILatencyCheck
    {
        IPsnServer Server { get; }

        long Latency { get; }

        IPStatus LastStatus { get; }

        event EventHandler Executed;

        void Reset();

        Task Execute();
    }
z4kn4fein commented 5 years ago

Hi, sorry for the late reply, i added a sample test for your case here. Currently this behavior can be achieved only with Lazy<T> is this an issue for you?

Tobriand commented 5 years ago

@z4kn4fein Will this also work without using SingletonLifetime() - e.g. with PerResolutionRequestLifetime() instead? When I've used a SingletonLifetime, I run into issues if the object should be created only once (and reused), but it has dependencies that have yet to be declared, since SingletonLifetime seems to create an instance on registration, not on consumption.

z4kn4fein commented 5 years ago

Hey,

Could you give me please a bit more detailed explanation or a sample configuration about what you want to achieve? Basically the singleton lifetime creates the underlying instance only when the first resolution request arrives to that registration and not at registration time. Could you please send me the configuration where you experienced different behaviors?

Thanks!

Tobriand commented 5 years ago

Hmm... I think the behaviour I was attributing to time-of-instantiation might actually be attributed to slightly unexpected behaviour of singletons within a scope. Which is to say, user error.

Specifically, my test was providing a dependency for TheSingleton in a scope, and expecting it to be consumed. Of course, it wasn't.

The actual error I got was

System.ArgumentException
  HResult=0x80070057
  Message=Expression of type 'System.Object' cannot be used for assignment to type 'TheSingletonInstance'
  Source=System.Linq.Expressions
  StackTrace:
   at System.Linq.Expressions.Expression.Assign(Expression left, Expression right)
   at Stashbox.BuildUp.Expressions.ExpressionBuilder.FillMembersExpression(IContainerContext containerContext, MemberInformation[] injectionMembers, RegistrationContext registrationContext, ResolutionContext resolutionContext, Expression instance)
   at Stashbox.BuildUp.Expressions.ExpressionBuilder.CreateFillExpression(IContainerContext containerContext, IServiceRegistration serviceRegistration, Expression instance, ResolutionContext resolutionContext, Type serviceType)
   at Stashbox.BuildUp.ObjectBuilders.FactoryObjectBuilder.GetExpressionInternal(IContainerContext containerContext, IServiceRegistration serviceRegistration, ResolutionContext resolutionContext, Type resolveType)
   at Stashbox.BuildUp.ObjectBuilderBase.BuildDisposalTrackingAndFinalizerExpression(IContainerContext containerContext, IServiceRegistration serviceRegistration, ResolutionContext resolutionContext, Type resolveType)
   at Stashbox.BuildUp.ObjectBuilderBase.GetExpression(IContainerContext containerContext, IServiceRegistration serviceRegistration, ResolutionContext resolutionContext, Type resolveType)
   at Stashbox.Registration.ServiceRegistration.ConstructExpression(IContainerContext containerContext, ResolutionContext resolutionContext, Type resolveType)
   at Stashbox.Registration.ServiceRegistration.GetExpression(IContainerContext containerContext, ResolutionContext resolutionContext, Type resolveType)
   at Stashbox.ResolutionScope.Activate(ResolutionContext resolutionContext, Type type, Object name)
   at Stashbox.ResolutionScope.Resolve(Type typeFrom, Boolean nullResultAllowed, Object[] dependencyOverrides)
   at DependencyResolverProxy.Resolve(Type typeFrom, Boolean nullResultAllowed, Object[] dependencyOverrides) in DependencyResolverProxy.cs:line 56

But as I say, thinking about it, it's correct that something goes wrong along these lines. And it probably isn't related to this question. Having a slightly more in depth error message (maybe along the lines of "I couldn't find a resolvable definition of TheSingletonInstance within the scope ; one or more definitions are present, but have dependencies that are undefined.") would be a wholly different issue to consider addressing!

z4kn4fein commented 5 years ago

Yep, singleton with scoped dependencies are not working as you expect, those dependencies will be singletons also because the singleton will be constructed only once. But you are right, the container should warn the user if this happens, I'll put this on the roadmap.

However, the exception you sent me looks a bit strange and opens a different issue in my mind, could you please share with me the configuration which caused this?

Tobriand commented 5 years ago

Took a while, but here it is. Turns out playing with lifetimes seems to have some strange effects in general!

This is basically the same as the code posted that replicates the other issue already raised, except that it applies a different scope, and goes wrong without a child container thrown into the mix.


        class DoResolveAttribute : Attribute { }
        interface ITier1 { }
        class Tier1 : ITier1 {[DoResolve] public ITier2 Inner { get; set; }[DoResolve] public TierBase OtherInner { get; set; } }
        interface ITier2 { }
        class Tier2 : ITier2 { public Tier2(string name) { Name = name; } public string Name { get; set; } }
        abstract class TierBase { public int Id { get; protected set; } }
        class Tier3 : TierBase { public Tier3(int id) { Id = id; }[DoResolve] public ITier2 Inner { get; set; } }

        public abstract class PrivateArgs 
        {
            public object[] ArgList { get; protected set; }
            public abstract Type Target { get; }
        }
        public class PrivateArgs<T> : PrivateArgs
        {
            static readonly Type _Target = typeof(T);

            public PrivateArgs(params object[] args) : base()
            {
                ArgList = args;
            }

            public static PrivateArgs<T> Get(params object[] args)
            {
                var res = new PrivateArgs<T>();
                res.ArgList = args;
                return res;
            }

            public override Type Target { get; } = _Target;
        }

        /// <summary>
        /// 
        /// </summary>
        [TestMethod]
        public void ContextEstablishedInChildContainersCanBeAccessedWhenUsingAParentScopeConstruction()
        {
            StashboxContainer sb1 = CreateContainer(c => c.WithSingletonLifetime());
            StashboxContainer sb2 = CreateContainer(c => c.WithSingletonLifetime());

            // This works
            sb1.PutInstanceInScope(typeof(PrivateArgs<ITier2>), PrivateArgs<ITier2>.Get("Bob"));
            sb1.PutInstanceInScope(typeof(PrivateArgs<TierBase>), PrivateArgs<TierBase>.Get(5));
            ITier1 renderer = (ITier1)sb1.Resolve(typeof(ITier1));

            using (var scope = sb2.BeginScope())
            {
                scope.PutInstanceInScope(typeof(PrivateArgs<ITier2>), PrivateArgs<ITier2>.Get("Bob"));
                scope.PutInstanceInScope(typeof(PrivateArgs<TierBase>), PrivateArgs<TierBase>.Get(5));
                ITier1 renderer2 = (ITier1)scope.Resolve(typeof(ITier1));
            }

        }

        private static StashboxContainer CreateContainer(Action<RegistrationConfigurator> scopeConfig = null)
        {
            var sb = new Stashbox.StashboxContainer(
                            config => config
                                .WithUnknownTypeResolution(ctx => ctx.WhenHas<DoResolveAttribute>())
                                .WithMemberInjectionWithoutAnnotation(
                                    Stashbox.Configuration.Rules.AutoMemberInjectionRules.PropertiesWithPublicSetter,
                                    ti => ti.CustomAttributes.OfType<DoResolveAttribute>().Any()
                                    )
                            );

            var allTypes = new (Type, Type)[]
                {
                    (typeof(ITier1), typeof(Tier1)),
                    (typeof(ITier2), typeof(Tier2)),
                    (typeof(TierBase), typeof(Tier3)),
                }.ToList();

            foreach (var type in allTypes)
            {
                var tbuilt = type.Item2;
                var tinterface = type.Item1;

                var targs = typeof(PrivateArgs<>).MakeGenericType(tinterface);
                sb.Register(tinterface, tbuilt,
                    c => 
                            c
                            .WithFactory(d =>
                            {
                                PrivateArgs argContainer = d.Resolve(targs) as PrivateArgs;
                                object[] args = argContainer?.ArgList;
                                object res = null;
                                try { res = Activator.CreateInstance(tbuilt, args ?? new object[0]); }
                                catch { }

                                return res;
                            })
                            .WithAutoMemberInjection(
                                Stashbox.Configuration.Rules.AutoMemberInjectionRules.PropertiesWithPublicSetter,
                                ti => ti.CustomAttributes.OfType<DoResolveAttribute>().Any())
                            // Simple extension method allowing conditional configuration inline
                            .ApplyIf(scopeConfig != null, scopeConfig)
                    );
            }

            return sb;
        }
z4kn4fein commented 5 years ago

I see the issue here, singletons by design are resolved in the root resolution scope, however, you put their dependencies into a new scope, so when the factories are being hit, this expression d.Resolve(targs) as PrivateArgs; is evaluated as null, because those PrivateArgs are not in the root scope. But the container assumes that the result of the factory is valid so it tries to inject the evaluated null anyway. There is a type mismatch however so that needs to be fixed.

Tobriand commented 5 years ago

Yeah... I guess what I'm actually after is scoped + inheritance (which might be how it works in any event). I.e. it gets resolved as a scoped instance, then if scopes are nested, gets reused rather than created once per scope.

I'll check what happens using the above - given the failure is pretty much by design, it will probably still fail, but it might be clearer why :)

z4kn4fein commented 4 years ago

Closing this now due to inactivity, please open a new issue if something comes up!