simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 155 forks source link

Help with register nested generic type #166

Closed Saborion closed 8 years ago

Saborion commented 8 years ago

I'm using commands and queries, based on interfaces, and have batch registered those. All pretty much taken from Steven's (www.cuttingedge.it) command / query blogs and the examples in the SimpleInjector documentation: interface IQuery<TResult> {} interface IQueryHandler<TQuery, TResult> where TQuery : IQuery<TResult> {}

Typically used like this in a class: private readonly IQueryHandler<GetRaceTrackQuery, RaceTrack> queryHandler; public ViewRaceTrackController(IQueryHandler<GetRaceTrack, RaceTrack> queryHandler) {}

This is registered with the container like this: container.Register(typeof(IQueryHandler<,>), new[] { typeof(IQueryHandler<,>).Assembly });

Works awesome. Now, instead of using GetRaceTrackQuery, GetXXXQuery etc, I've made a generic query, class GetByIdQuery<TEntity> : IQuery<TEntity> where TEntity : class and the query handler, class GetByIdQueryHandler<TEntity> : IQueryHandler<GetByIdQuery<TEntity>, TEntity> where TEntity : class

I've replaced the member using the query, from private readonly IQueryHandler<GetRaceTrackQuery, RaceTrack> queryHandler; to private readonly IQueryHandler<GetByIdQuery<RaceTrack>, RaceTrack> queryHandler; and updated the constructor that is being injected.

Running the application throws an exception though, stating the following:

The constructor of type ViewRaceTrackController contains the parameter with name 'queryHandler' and type IQueryHandler<GetByIdQuery<RaceTrack>, RaceTrack> that is not registered. Please ensure IQueryHandler<GetByIdQuery<RaceTrack>, RaceTrack> is registered, or change the constructor of ViewRaceTrackController.

I cannot get the compiler to accept any registration attempt on my part. How should such a registration look?

qujck commented 8 years ago

The problem is with GetByIdQueryHandler<TEntity>, none of the following declarations are valid, i.e. they do not compile:

var x = new GetByIdQueryHandler<GetRace>();
var y = new GetByIdQueryHandler<GetByIdQuery<GetRace>>();
var z = new GetByIdQueryHandler<GetRaceTrackQuery>();

It bothers me a little that the following code adds no registrations to the container and does not give any warnings (this may be a bug)

var container = new Container();
container.Register(typeof(IQueryHandler<,>), typeof(GetByIdQueryHandler<>));
container.Verify();
Saborion commented 8 years ago

Hmm... well, here is one implementation of IQueryHandler<,> that I know for a fact work, but still cannot be created the way you tried to create an instance of GetByIdQueryHandler above (since neither has an empty constructor).

public class GetHorseQueryHandler : IQueryHandler<GetHorseQuery, Horse>
{
    private readonly TrottingDbContext context;

    public GetHorseQueryHandler(TrottingDbContext context)
    {
        this.context = context;
    }

    public async Task<Horse> HandleAsync(GetHorseQuery query)
    {
        return await this.context.Horses.FindAsync(query.HorseId);
    }
}

And the implementation I try to use for GetByIdQueryHandler:

public class GetByIdQueryHandler<TEntity> : IQueryHandler<GetByIdQuery<TEntity>, TEntity> where TEntity : class
{
    private readonly TrottingDbContext context;

    public GetByIdQueryHandler(TrottingDbContext context)
    {
        this.context = context;
    }

    public async Task<TEntity> HandleAsync(GetByIdQuery<TEntity> query)
    {
        return await this.context.Set<TEntity>().FindAsync(query.Id);
    }
}

GetHorseQueryHandler works, so I assume that GetByIdQueryHandler doesn't work either because of the constraint, or because of the generic nested "GetByIdQuery<>".

dotnetjunkie commented 8 years ago

Might it be that you have 2 definitions of GetEntityById<T>?

Saborion commented 8 years ago

Pretty sure I only have that one generic query. A pretty new prototype project, not many queries in there yet. Below is the container setup, in case there's something else I'm doing wrong.

var container = new Container();
container.RegisterPerWebRequest<TrottingDbContext>();
container.RegisterPerWebRequest<PostCommitRegistrator>();

container.RegisterDecorator(typeof(ICommandHandler<>), typeof(TransactionCommandHandlerDecorator<>));
container.RegisterDecorator(typeof(ICommandHandler<>), typeof(PostCommitCommandHandlerDecorator<>));

container.Register<IPostCommitRegistrator>(() => container.GetInstance<PostCommitRegistrator>());
container.Register(typeof(IQueryHandler<,>), new[] { typeof(IQueryHandler<,>).Assembly });
container.Register(typeof(ICommandHandler<>), new[] { typeof(ICommandHandler<>).Assembly });

container.Verify();
DependencyResolver.SetResolver(new SimpleInjectorDependencyResolver(container));
dotnetjunkie commented 8 years ago

So where's the registration for the open-generic GetByIdQueryHandler<TEntity>?

TheBigRic commented 8 years ago

You're missing the registration for GetByIdQueryHandler<TEntity>.

This line:

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

will, according to the documentation and xml comments, find all non-generic implementations of the generic interface. So the GetByIdQueryHandler<T> isn't registered at all. You should add this registration:

container.Register(typeof(IQueryHandler<,>),typeof(GetByIdQueryHandler<>));

When you would another open generic implementation of IQueryHandler<T,R> you'll have to use RegisterConditional as you can read here

Saborion commented 8 years ago

Many thanks to everyone that took the time to help out. The problem was my lack of understanding of open and closed generic types. The answer posted by TheBigRic did the trick, everything works well now. Time to take a close look at open / closed generic types.