jbogard / MediatR.Extensions.Microsoft.DependencyInjection

MediatR extensions for Microsoft.Extensions.DependencyInjection
MIT License
326 stars 90 forks source link

Question / Request - Ignoring abstract types in ServiceRegistrar #121

Open bhehe opened 2 years ago

bhehe commented 2 years ago

I'm in the process of upgrading our use of MediatR from 9.x to 10.x and have bumped into an issue where the original author has unfortunately cloned/tweaked code from the ServiceRegistrar class and it no longer compiles. Yes, yes, I'm not fond of the clone/tweak angle myself and there's no guarantee of compatibility when someone does that. ;-)

From the only clues I see left behind, they allude to the reason for having done that to stem from a need to filter out abstract classes when scanning & registering and it appears their primary alteration is simply using typeInfo.IsConcrete() in multiple locations. I'm still analyzing the code and comparing to the current 10.x code to identify the exact delta(s) and better understand their changes.

What I wanted to ask here was if this seems fundamentally off and shouldn't have been necessary? We're using an approach for DI registration where we have multiple things leveraging discovery/scanning based DI registration approaches (i.e. Scrutor) and have a master list of assemblies and in some of those there are base classes, etc. for MediatR related needs.

Secondly, if this seems like something that can/should be included I'd like to propose adding the IsConcrete() checks - seems like something that shouldn't hurt and shouldn't adversely impact performance since we're already inspecting instances of the TypeInfo.

My goal is to alleviate the need for the cloned/tweaked version of the code, otherwise I'm left needing to make a 10.x compatible version of or just find a better/different way to handle the registration behavior.

bhehe commented 2 years ago

Just to follow up, it appears the original author was also aiming to add 'filtering' support and that seems to have been a contributing reason for this cloning/tweaking of the cited code. On review, it looks like 10.x now has this capability.

As it stands, I believe I'm able to throw away that cloned/tweaked code and just align onto the MediatR ServiceRegistrar and use it unaltered, as I've not found any blockers otherwise and I'm not seeing any issue with hitting abstract base types either.