Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide the consumer type in factories #109

Open
cabralRodrigo opened this issue Mar 25, 2021 · 10 comments
Open

Provide the consumer type in factories #109

cabralRodrigo opened this issue Mar 25, 2021 · 10 comments

Comments

@cabralRodrigo
Copy link

First of all, love the project. I've been really excited for source generators and this project just uses then so well.

Now, I'm trying to inject a logging interface in my application.
The interface is:

public interface ILogger
{
    void Info(...);
    void Error(...);
}

And my implementation is:

public class Logger<T> : ILogger
{
    public Logger()
    {
        //uses T to get the actual logger from some library
    }

    public void Info(...);
    public void Error(...);
}

As you can see, the consumer type is used in the implementation but not in the interface.
I know that default (at least for microsoft) is to use something like ILogger<T>, but all my projects uses ILogger (and I personally thinks that is better).

I don't know if currently is possible to use StrongInject to resolve this.
One possible solution is to provide the consumer type (the type begin resolved) in the factory methods and in the IFactory interface.

I'm thinking something like this:

public interface ILogger { }

public class Logger<T> : ILogger { }

public class A { public A(ILogger logger) { } }

public class B { public B(C c) { } }

public class C { public C(ILogger logger) { } }

public partial class Container : IContainer<A>, IContainer<B>
{
    [Factory]
    private static ILogger Create<TConsumer>() => new Logger<TConsumer>();
}

var container = new Container();
container.Resolve<A>(); // Here TConsumer is 'A'
container.Resolve<B>() // Here TConsumer is 'C'

Or maybe something like this:

public interface ILogger { }

public class Logger : ILogger { public Logger(Type type) { } }

public class A { public A(ILogger logger) { } }

public class B { public B(C c) { } }

public class C { public C(ILogger logger) { } }

public partial class Container : IContainer<A>, IContainer<B>
{
    [Factory]
    private static ILogger Create(Type constumerType) => new Logger(constumerType);
}

var container = new Container();
container.Resolve<A>(); // Here the parameter 'constumerType' is 'A'
container.Resolve<B>() // Here is 'C'

I understand that this is probably a breaking change and if it's not something that you're not willing to do.

Thanks for the wonderful project 😄

@YairHalberstadt
Copy link
Owner

Hi

I can definitely see the use case here.

Currently in StrongInject there's no way to see how a type is going to be used when you resolve it in order to modify how it's resolved.

I don't know the best way to support this functionality. I think what I'll do is I'll mull over this for a few days, and see if I can think of anything elegant, and not overly complex. If I do, I'll post the design here for further discussion.

@YairHalberstadt
Copy link
Owner

Sorry it took me so long to get back to you - I've been distracted by other things and then forgot about this.

What I'm thinking of doing is adding a type:

namespace StrongInject
{
    public class ResolutionInfo
    {
        public Type ResolvedAsDependencyOf { get; }
    }
}

Then ONLY for InstancePerDependency dependencies you can have a parameter of type ResolutionInfo. If no registrations exist for that type, then StrongInject will create it with the correct type.

So you could do:

[Factory(Scope.InstancePerDependency)] ILogger CreateLogger(ResolutionInfo rInfo) => new Logger(rInfo.ResolvedAsDependencyOf);

WDYT?

@trampster
Copy link

trampster commented Jul 21, 2021

For my usecase with Serilog I need to create a new logger from an existing one.

ILogger logger = _generalLogger.ForContext(typeof(parent));

So would look something like:

[Factory(Scope.InstancePerDependency)] ILogger CreateLogger(ResolutionInfo rInfo, ILogger logger) => logger.ForContext(typeof(rInfo.ResolvedAsDependencyOf));

But I worry that this would be circular.

My generic logger is currently created as follows

      [Factory(Scope.SingleInstance)]
      public static ILogger CreateLogger(ILogService logService)
      {
         return new LoggerConfiguration()
            .Enrich.FromLogContext()
            .WriteTo.File(Path.Combine(logService.GetLogFolder(), "log.txt"), fileSizeLimitBytes: 1024000, rollingInterval: RollingInterval.Day,
               outputTemplate: "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level}] ({SourceContext}) {Message}{NewLine}{Exception}", rollOnFileSizeLimit: true, hooks: new LoggingFileLifecycleCycleHooks())
            .WriteTo.AndroidLog()
            .MinimumLevel.Debug()
            .CreateLogger();
      }

I think I can solve this by creating a ILoggerResolver, which holds the generic ILogger, and then make a factory that looks like this:

[Factory(Scope.InstancePerDependency)] ILogger CreateLogger(ResolutionInfo rInfo, ILoggerResolver resolver) => resolver.GenericLogger.ForContext(typeof(rInfo.ResolvedAsDependencyOf));

So I think your design is good 👍

@YairHalberstadt
Copy link
Owner

@trampster you could do that by making it a decorator instead.

[Decorator] ILogger CreateLogger(ResolutionInfo rInfo, ILogger logger) => logger.ForContext(typeof(rInfo.ResolvedAsDependencyOf));

@AtomicBlom
Copy link

This is exactly my use case as well, using a decorator seems perfectly reasonable.

@YairHalberstadt
Copy link
Owner

I think there's some further design questions that need to be answered.

If I have the following code:

public class A{ public (IB b){} }
public class B : IB { public B(ILog log){} }
public class Log : ILog { public Log(ResolutionInfo info){} }

What would you expect ResolutionInfo to contain? I presume typeof(B) or typeof(IB) (and not typeof(ILog)) but which one?

@AtomicBlom
Copy link

I would expect it to be typeof(B). In the case of logging with Serilog, you'd want the logger to be the concrete implementation, because when you're looking for the SourceContext to hunt down a bug you're going to want to know which concrete implementation exactly it was.

@YairHalberstadt
Copy link
Owner

Some further cases which I don't think are obvious what to do.

If I have:

public record A(ResolutionInfo info);
public record B(A[] as);

Should ResolutionInfo contain typeof(A[]) or typeof(B). If we say typeof(B)`, what about the following:

public record A(ResolutionInfo info);
public record B(List<A> as);

[Register(typeof(A)]
[Register(typeof(B))]
public class Container : IContainer<B>
{
    [Factory] List<T> CreateList<T>(T[] ts) => ts.ToArray();
}

By the same logic, ResolutionInfo ought to contain typeof(B) not typeof(List<A>), but there's no way for StrongInject to know that List<A> can be ignored as just a container type rather than the actual consumer.

@YairHalberstadt
Copy link
Owner

Note that you can work around this issue by using Microsoft's ILogger<T> interface as suggested at #117 (comment), but I understand that can be a hurdle to change to.

@AtomicBlom
Copy link

Sorry, I saw this and got stuck on an answer for it and forgot to come back to it.

It's a really good question that I don't have an answer to because I've never actually encountered a situation where it was a problem.

There are a couple of things that I would note that may make this harder or simpler, I'm not sure, and that's the case that I've never needed the concept that ResolutionInfo provides directly inside a dependancy. In general, I've always tried to keep my code unaware of the container as possible unless it's doing nested resolution, and as such I personally would only need it to be available for [Factory] methods (and probably decorators)

I honestly wish I had a better answer, but I'm afraid I don't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants