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

Exclude certain derived types from registering #121

Open
SlowLogicBoy opened this issue Jul 13, 2018 · 6 comments
Open

Exclude certain derived types from registering #121

SlowLogicBoy opened this issue Jul 13, 2018 · 6 comments

Comments

@SlowLogicBoy
Copy link

SlowLogicBoy commented Jul 13, 2018

//Class in library assembly
public class LibraryClass {}

public class MyBaseClass : LibraryClass {}

public class MyClass : MyBaseClass {}

right now it registers MyClass as MyClass, MyBaseClass and LibraryClass, I would like to exclude registering base classes from Library.
I tried doing:

  <AutoDI>
    <Type Name="Library.*" Lifetime="None"/>
  </AutoDI>

But it doesn't work and I presume it is not intended way to do this.

Maybe there is a way to do this using <Map> ?

@Keboo
Copy link
Owner

Keboo commented Jul 13, 2018

When you say, "exclude registering base classes from Library." do you mean exclude all registrations that use base classes or exclude all registrations from that derive from LibraryClass?

The first case is easy, as this is controlled with behaviors. Rather than using Default, simply specify a comma separated list of which behaviors you would like turned on. In this case the behavior you want to turn off is IncludeBaseClasses. This would mean that in you should only get mappings for each type to itself. If you want to turn off the mapping of each of the classes to itself, you can remove the IncludeClasses behavior.

For the second case, I think the issue is your Type map. In version 3.1.0 there was a breaking change that was introduced. Rather than the Name properties defaulting to being regular expressions they use a simplistic wildcard matching strategy. So as you have it written it would no map classes that are within the Library namespace (likely not what you were going for). Perhaps something like this?

<AutoDI>
  <Type Name=".Library*" Lifetime="None"/>
</AutoDI>

Assuming you don't have a namespace that starts with "Library", this would remove all types that started with the name "Library". But in your example above this does nothing to the maps for that the behaviors will all for MyBaseClass and MyClass. You can also get more explicit with the Name attribute and prefix it with "regex:" to have the value treated as a regular expression instead of a simple wildcard match.

Finally it is worth knowing that when AutoDI runs, the behaviors run, and generate a bunch of mappings. Then the explicit Map/Type values are evaluated, letting them override the mappings that the behaviors added.

Let me know if this is sufficient for your needs or if there is something better that could be done.

@SlowLogicBoy
Copy link
Author

The problem I'm having is when I request sp.GetService<LibraryClass>(); it will try to resolve MyBaseClass and MyClass, and I don't want DI to register those classes as an implementation for LibraryClass. I could set Behavior but that would affect whole assembly.

<AutoDI>
  <Type Name=".Library*" Lifetime="None"/>
</AutoDI>

This didn't work for me since MyClass still was registered as implementation of LibraryClass, but that might be my mistake so I'll look at it once more.

@Keboo Keboo changed the title Exculde certain Base Types from registering Exclude certain derived types from registering Jul 15, 2018
@Keboo
Copy link
Owner

Keboo commented Jul 15, 2018

Ah I think I understand the issue now (and no, my previous suggestion wont work). And unfortunately it is current "working as designed" (though the design may be flawed). The original intent with the base class stuff was to be able to request an [abstract] base class and have it automatically return the most derived class.

It is also worth noting that when behaviors add potential maps, if multiple maps conflict, all of the conflicting maps are removed from the final mapping result. So in your example (with the current release) the only remaining map are:
MyClass => MyClass as Transient (from the IncludeClasses behavior)
LibraryClass => MyClass as Transient (from the IncludeBaseClasses behavior).

The second one is actually a bug. It should have been considered a duplicate map because it could have been considered mapped to itself, mapped to MyBaseClass, or mapped to MyClass, but because of the way that it currently throws out duplicates, On the second potential map it detects the duplicates, and removes both. So when the third potential map comes it, it is allowed. (Buggy code here). I have created branch fix121 to keep track of this issue as well.

Regardless, there is still the outstanding question of the best way to handle this.

There are a couple potential ideas I have here:

  1. Simply add a new <behaviors> tag like the existing <map> or <type> tags. This would provide a place where additional customization could take place. Such as the addition of an exclusion list for the behaviors in question. This might be ok, but feels very inflexible.
  2. Add support for custom behaviors (Add support for custom behaviors #63). This is something I have been playing around with since, I tend to turn off any problematic behavior and declare a bunch of manual maps with my various rules (usually stuff that is pretty project specific). The idea behind the custom behaviors is that there would be some base class that you would derive from. It would get called with the complete list of types, and have access to add/remove maps. Obviously there are lots of details I have not worked out yet, and it has not risen high enough on my priority list yet.

Any feedback or other thoughts?

@Keboo Keboo added the bug label Jul 15, 2018
@Keboo Keboo added this to the 3.3.0 milestone Jul 15, 2018
@SlowLogicBoy
Copy link
Author

SlowLogicBoy commented Jul 16, 2018

I can only suggest the xml format. I would like to have something similar to how dotnet sdk handles project files:

<ItemGroup>
    <None Remove="Item/To/Remove.cs" />
</ItemGroup>

In this case it could look something like:

<behaviors>
    <behavior Name="FullClassName" Remove="BaseClass;AnotherBassClass" Behavior="IncludeClasses" />
</behaviors>

well maybe Exclude would be more appropriate than Remove, and Class instead of Name.

EDIT:
On the other hand, maybe this is not a thing that should be worked on, because this would just be Manual registration transferred to Config/Attributes, and easier way would be to just register stuff like this manually using SetupMethod ?

@Keboo
Copy link
Owner

Keboo commented Jul 16, 2018

Agreed. For the 3.3.0 release I only plan on addressing the bug with odd number of conflicting maps actually getting registered. This is clearly a bug, and fixing it will lay the foundation for doing custom behaviors which I think is the best solution. With that said, I am still considering adding some minor level of customization to the built-in behaviors because simply excluding some types feels like a use-case that should be supported.

You are correct doing it in the SetupMethod is certainly a simple way of handling. This is how I am currently handing some more complex scenarios in my own apps. Sometimes you just need to do some setup and run-time because values are not known at compile time.

@Keboo
Copy link
Owner

Keboo commented Aug 4, 2018

@SlowLogicBoy the next 3.3.0 preview nuget will have this bug addressed.
There is a slight change in behavior (to bring this DI container more in line with the MS one). Behaviors no longer ignore duplicate mappings. When a type has multiple possible resolutions, the last one that was registered is the one that will resolve. In addition, you can now request an IEnumerable and get back all of the types that are registered for that given service type.

For an example take a look at these unit tests as well as this one.

Since the bug with the registration is now address, I am going to bump the remainder of this issue to vNext (likely 3.4.0).

@Keboo Keboo added enhancement and removed bug labels Aug 4, 2018
@Keboo Keboo modified the milestones: 3.3.0, 3.4.0 Aug 4, 2018
@Keboo Keboo removed this from the 3.4.0 milestone Mar 1, 2019
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