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

Resolving generic type with type argument that maps to multiple concrete types of other type argument fails #316

Closed
ChristianHoe opened this issue Oct 14, 2016 · 5 comments
Assignees
Milestone

Comments

@ChristianHoe
Copy link

ChristianHoe commented Oct 14, 2016

Hi,

I had to change my project to use collections and found some strange behavoir. I wrote a small sample showing the problem. Everything is registered but Simple Injector keeps complaining and I do not know why. Depending on the order of the Interfaces in line Req : IServiceRequest<ResponseA>, IServiceRequest<ResponseB> the generation of UseCase1 or UseCase2 fails.

[TestMethod]
public void _Test()
{
    var container = new SimpleInjector.Container();

    container.RegisterCollection(typeof(IService<,>), new[] { Assembly.GetExecutingAssembly() });
    container.Register(typeof(IServiceAdapter<,>), typeof(Adapter<,>));
    container.Register<UseCase1>();
    // Causes error
    container.Register<UseCase2>();
    container.Verify();
}
public interface IServiceRequest<T2> { }
public interface IService<T1, T2> where T1 : IServiceRequest<T2> { }
public interface IServiceAdapter<T1, T2> where T1 : IServiceRequest<T2> { }

// Change interface order here
public class Req : IServiceRequest<ResponseA>, IServiceRequest<ResponseB> { }

public class ResponseA { }
public class ResponseB { }

public class Service1 : IService<Req, ResponseA> { }
public class Service2 : IService<Req, ResponseB> { }

public class Adapter<T1, T2> : IServiceAdapter<T1, T2> where T1 : IServiceRequest<T2>
{
    public Adapter(IEnumerable<IService<T1, T2>> services) { }
}

class UseCase1
{
    public UseCase1(IServiceAdapter<Req, ResponseA> service) { }
}

class UseCase2
{
    public UseCase2(IServiceAdapter<Req, ResponseB> service) { }
}

The call to Verify() fails with the following exception:

The configuration is invalid. Creating the instance for type UseCase2 failed. The constructor of type UseCase2 contains the parameter with name 'service' and type IServiceAdapter<Req, ResponseB> that is not registered. Please ensure IServiceAdapter<Req, ResponseB> is registered, or change the constructor of UseCase2.

Regards,

Christian

@dotnetjunkie
Copy link
Collaborator

dotnetjunkie commented Oct 15, 2016

Okay, I'm currently looking in to this; I can't immediately see why this is going on. Running it through the debugger right now. Might be a bug. What I'm noticing is that if you change the registration of the open-generic IServiceAdapter<T1, T2> to two separate closed registrations, everything works out fine.

In the meantime, to stall some time: why do you wish to implement two IServiceRequest<T> interfaces on your Req type? It seems ambiguous to me to have to definitions of what the request should return. Can you describe why you want to do this?

@ChristianHoe
Copy link
Author

As far as I remember, you could remove the contrains from public interface IService<T1, T2> where T1 : IServiceRequest<T2> { } public interface IServiceAdapter<T1, T2> where T1 : IServiceRequest<T2> { } and everything should be fine too.

In the meantime, to stall some time: why do you wish to implement two IServiceRequest interfaces on your Req type? It seems ambiguous to me to have to definitions of what the request should return. Can you describe why you want to do this?

As you might noticed, the structure is based upon your QueryHandler example. Because we mostly deliver data from the database we skipped the command part. Not happy with your naming we renamed Query to Request and QueryHandler to Service seeing it more as messaging. Requests/Responses are data-only - the service itselft is the activ part.

Due to the havy use of time slices and other things we do not use an OR-Mapper to get everything but using database views and corresponding services. Starting with something like ReqMedia, ReqDocuments, ReqPrices, ReqBonus,... I noticed they often use the same properties. Especially in the beginning we forgot about filling some properties and mapping itself was kind of annoying und repetitive. Lukily SimpleInjector is quite flexible and allows me to say I like a service which accepts a ProduktId as input and gives me all media and the next time I like a service which accepts a ProduktId as input and gives me all prices and so on... I also start switching from using classes for the request to get-only-interfaces to get rid of the frequent property mapping.

From architectural point of view the IServiceRequest<T> is quite annoying but its really really helps the compiler to avoid invalid request response pairs.

@dotnetjunkie
Copy link
Collaborator

As you might noticed, the structure is based upon your QueryHandler example.

Yes, I noticed. After copying your code to a console application, I renamed all classes and interfaces to be able to make a mental model in my head.

we do not use an OR-Mapper to get everything but using database views and corresponding services.

Thumbs up 👍 for that.

In general, I would advise against reusing a single Req for multiple requests. From a consumer standpoint it is really weird to be able to get two completely different (unrelated) results for the same question. How can a question result into two unrelated answers? I would advise splitting your request objects with a one-to-one mapping between request and response. If you see there is a lot of duplication between 'queries', try to extract this common data into its own DTO. This DTO can be exposed as property on the query object. This allows you to compose objects out of other objects instead of using multiple inheritance. You can still add interfaces to request objects, but not multiple definitions of what a request returns.

But that said, what you are experiencing is a bug in Simple Injector. I traced it down to the internal GenericTypeBuilder class. I remember realizing this 'limitation' years back, and ignored the problem, because it might be quite hard to solve. I will however take a stab at this again. Simple Injector has the best support for generic of all containers, and I'm very keen on fixing any bugs in general, but especially bugs related to generics. I'll keep you posted about my progress.

@dotnetjunkie dotnetjunkie modified the milestones: v3.3, v3.2.3 Oct 16, 2016
@dotnetjunkie dotnetjunkie self-assigned this Oct 16, 2016
@dotnetjunkie dotnetjunkie changed the title Problem resolving generic contrain? Resolving generic type with type argument that maps to multiple concrete types of other type argument fails Oct 16, 2016
dotnetjunkie added a commit that referenced this issue Oct 16, 2016
@dotnetjunkie
Copy link
Collaborator

After some thorough investigation, the fix wasn't actually that hard.

I pushed v3.2.3 to Github and Nuget. This patch release should fix your problem.

@ChristianHoe
Copy link
Author

Thanks, works like a charm.

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

No branches or pull requests

2 participants