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

Registration of sub types when abstraction is required. #642

Open
stevengopmo opened this Issue Dec 7, 2018 · 9 comments

Comments

2 participants
@stevengopmo
Copy link

stevengopmo commented Dec 7, 2018

Hi @dotnetjunkie and team,

I'm having a problem getting my head around a registration/integration here that I'm hoping someone can help with. Here's the scenario.

I have an abstraction

public interface IProcessor<in T> where T : ProcessFile
{
        Task<FileProcessingReport> ProcessFile(T processFile);
}

And this registration

container.Register(typeof(IProcessor<>), new[] {typeof(IProcessor<>).Assembly});
container.RegisterDecorator(typeof(IProcessor<>), typeof(ExecutionTimeDecorator<>));

So far so good. IProcessor handles ProcessFile messages (an abstraction)

    public interface IProcessor<in T> where T : ProcessFile
    {
        Task<FileProcessingReport> ProcessFile(T processFile);
    }

I use Rebus to handle messages that will eventually need to be handled by IProcessor but to get Rebus to handle the base message I have to specify the message type

    internal class ProcessFileHandler : 
        IHandleMessages<ProcessFile>, 
        IHandleMessages<IFailed<ProcessFile>>
    {
        private readonly IProcessor<ProcessFile> _processor;
        private readonly IBus _bus;

        public ProcessFileHandler(IProcessor<ProcessFile> processor, IBus bus)
        {
            _processor = processor;
            _bus = bus;
        }

        public async Task Handle(ProcessFile message)
        {
            var result = await _processor.ProcessFile(message);

            await _bus.Send(new DeliverReport(result));

        }
    }

Of course when I do this, I get an error from SI.

The constructor of type ProcessFileHandler contains the parameter with name 'processor' and type IProcessor<ProcessFile> that is not registered.
Please ensure IProcessor<ProcessFile> is registered, or change the constructor of ProcessFileHandler.
Note that there exists a registration for a different type Vortext.Ingestion.Common.Processors.IProcessor<T> while the requested type is Vortext.Ingestion.Common.Processors.IProcessor<Vortext.Ingestion.Common.Messages.ProcessFile>.'

Obviously this is because

Types that are registered by their abstraction, can only be resolved by their abstraction

So the question is how do I register IProcessor<ProcessFile> for all the ProcessFile subtypes? I'm sure this is probably obvious but I'm just not seeing it.

@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator

dotnetjunkie commented Dec 7, 2018

What implementations do you have for IProcessor<T>? I would expect your code base to have something as follows:

public class ProcessFileProcessor : IProcessor<ProcessFile> { ... }

With such an implementation, Simple Injector will automatically pick it up when you call Register(typeof(IProcessor<>), new[] {typeof(IProcessor<>).Assembly}); (assuming it lives in the same assembly as the IProcessor<T> interface).

In case you have one generic implementation:

public class ProcessorImpl<T> : IProcessor<T> { ... }

I'd expect to see a different registration (as the batch-registration will not pick up generic types):

container.Register(typeof(IProcessor<>), typeof(ProcessorImpl<>));
@stevengopmo

This comment has been minimized.

Copy link

stevengopmo commented Dec 7, 2018

I have about a dozen implementations of IProcessor<T> through an abstract implementation and you are right about SimpleInjector picking it up if my Handler class took an IProcessor<T> but my handler requires IProcessor<ProcessFile> that's why I'm getting the error.

Does that make sense? ProcessFile is an abstract class and all the inheritors get their own processor.

@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator

dotnetjunkie commented Dec 7, 2018

Can you post the definition of ProcessFile and the definition of some implementations? Is it something as follows?:

public abstract class ProcessFile { }
public class Derived1 : ProcessFile { }
public class Derived2 : ProcessFile { }

public class Derived1Processor : IProcessor<Derived1> { }
public class Derived2Processor : IProcessor<Derived2> { }
@stevengopmo

This comment has been minimized.

Copy link

stevengopmo commented Dec 7, 2018

Process file and derived

public abstract class ProcessFile
{
    protected ProcessFile(string fileFullPath, Tenant tenant)
    {
        FileFullPath = fileFullPath;
        Tenant = tenant;
    }

    public string FileFullPath { get; }
    public Tenant Tenant { get; }
    public abstract FileType FileType { get; }
}
public class ProcessInsuranceFile : ProcessFile
{
    public ProcessInsuranceFile(string fileFullPath, Tenant tenant)
        : base(fileFullPath, tenant) { }
    public override FileType FileType => FileType.Insurance;
}

IProcessor, Processor (abstract) and simple Impl

public interface IProcessor<in T> where T : ProcessFile
{
    Task<FileProcessingReport> ProcessFile(T processFile);
}

public abstract class Processor<TProcessFile, TModel> : IProcessor<TProcessFile> 
    where TProcessFile : ProcessFile
{
    protected readonly Func<FileInfo, IDelimitedFileReader<TModel>> ReaderFactory;

    protected Processor(Func<FileInfo, IDelimitedFileReader<TModel>> readerFactory)
    {
        ReaderFactory = readerFactory;
    }
    protected abstract Task<IEnumerable<(int row, string message)>> ProcessItems(
        IEnumerable<TModel> items);

    public async Task<FileProcessingReport> ProcessFile(TProcessFile processFile)
    {
        //removed for brevity
    }
}

public class InsuranceProcessor : Processor<ProcessInsuranceFile, SqlDataRecord>
{
    private readonly BulkLoadDtoDataAccess<InsuranceCompanyDto> _insuranceCompanyDataAccess;

    public Processor(BulkLoadDtoDataAccess<InsuranceCompanyDto> insuranceCompanyDataAccess,
        Func<FileInfo, BulkLoadDelimitedFileReader<InsuranceCompanyDto>> readerFactory) 
            : base(readerFactory)
    {
        _insuranceCompanyDataAccess = insuranceCompanyDataAccess;
    }

    protected override async Task<IEnumerable<(int row, string message)>> 
        ProcessItems(IEnumerable<SqlDataRecord> items)
    {
        return await _insuranceCompanyDataAccess.BulkLoad(items);
    }
@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator

dotnetjunkie commented Dec 7, 2018

This means that your Processor class implements IProcessor<ProcessInsuranceFile>.

Question: As your ProcessFileHandler depends on IProcessor<ProcessFile>, assuming there are many IProcessor<T> implementations, which implementation do you want to have injected into ProcessFileHandler?

@stevengopmo

This comment has been minimized.

Copy link

stevengopmo commented Dec 7, 2018

That's a great question. I want to have the IProcessor<T> implementation that matches the ProcessFile message implementation. For example if the message is ProcessInsuranceFile I want InsuranceProcessor (the last in the above message) to be injected.

@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator

dotnetjunkie commented Dec 7, 2018

The documentation states:

Note: Simple Injector only resolves variant implementations for collections that are registered using the Collection.Register overloads. In the screnario you are resolving a single instance using GetInstance<T> then Simple Injector will not return an assignable type, even if the exact type is not registered, because this could easily lead to ambiguity; Simple Injector will not know which implementation to select.

For more information on how to let Simple Injector handle this, please read this blog post.

@stevengopmo

This comment has been minimized.

Copy link

stevengopmo commented Dec 7, 2018

sorry Stephen, I'm really struggling with that concept. As far as I can tell Scenario #2 in that article is the one that applies to me.

In this scenario we want to configure the container in such way that when we request a single event handler for a particular event, it would return the single registered instance for that event, or is case it doesn't exist, return a compatible registered event handler.
(if you exchange event for command) and it seems like I should be using container.Collection.Register but I don't see that property in the container anywhere (I'm using 4.0.3) so maybe I need to upgrade? :)

But I guess what I fail to grasp really is why everything works if I have a handler like the following

internal class ProcessInsuranceHandler : ProcessFileHandler<ProcessInsuranceFile>, 
    IHandleMessages<ProcessInsuranceFile>, 
    IHandleMessages<IFailed<ProcessInsuranceFile>>
{
    public ProcessInsuranceHandler(
        IProcessor<ProcessInsuranceFile> processor, IBus bus) : base(processor, bus) { }

    public async Task Handle(ProcessInsuranceFile message)
    {
        await HandleMessage(message);
    }

    public async Task Handle(IFailed<ProcessInsuranceFile> message)
    {
        await HandleFailed(message);
    }
}

I guess this is because I'm being explicit about which ProcessFile message to handle?

@dotnetjunkie

This comment has been minimized.

Copy link
Collaborator

dotnetjunkie commented Dec 8, 2018

and it seems like I should be using container.Collection.Register but I don't see that property in the container anywhere (I'm using 4.0.3) so maybe I need to upgrade? :)

Always use the latest version :)

But otherwise, Container.Collection.Register in v4.1 and up replaces Container.RegisterCollection. The behavior is identical.

But I guess what I fail to grasp really is why everything works if I have a handler like the following

The Processor class implements IProcessor<ProcessInsuranceFile>. This makes Simple Injector register it by that abstraction, similar to the following registation:

container.Register<IProcessor<ProcessInsuranceFile>, Processor>();

Since the mapping is exclicit, Simple Injector can find it without help if you change your constructor argument to IProcessor<ProcessInsuranceFile>.

Another option is to inject an IEnumerable<IProcessor<ProcessFile>>. But in that case you would have to iterate the collection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment